mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 15:09:42 +02:00
Backfill the two regression gaps flagged on master after the recent
hostname-title and tmux-flicker fixes shipped without server-side
assertions.
* test/server-index-title.test.ts (8 tests) — exercises WebServer's
index.html templating path: default os.hostname(), --title-hostname
override, HTML-escape against `<script>`-style breakout, ampersand
non-double-encoding, exact-once substitution, and byte-identical
template-tail invariance.
* test/tmux-window-size-query.test.ts (15 tests) — mocks
child_process.execFileSync and walks the helper through the
browser-resize-between-attaches happy path, query-then-die race,
zero/negative/empty/non-numeric output, plus argv-form/timeout
assertions to lock down the no-shell-interpolation guarantee.
* src/session.ts — extracts the inline 14-line tmux size query into
a named `queryTmuxWindowSize()` export so the test surface is a
pure function. Behavior unchanged.
* src/web/public/notification-manager.js — Browser Notification API
(layer 3) now uses `${this.originalTitle}: ${title}` so OS-level
desktop pop-ups carry the same `codeman:<host>` prefix that the
tab title and Web Push payloads already do, finishing the
hostname plumb-through started in #82.
* CLAUDE.md, README.md — document the dual-CLI env-prefix discipline
(CLAUDE_CODE_* vs OPENCODE_*), expand the xterm-zerolag-input
duplication gotcha to mention the published-package side-effect,
and note that the hostname prefix now applies uniformly to tab
title, tab-flash, and OS notifications.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* Verifies that WebServer templates the `<title>` tag in the served
|
||||
* index.html with the hostname-aware `codeman:<host>` window title
|
||||
* (feature #82). The title must:
|
||||
* - default to `codeman:<os.hostname()>` when no override is supplied
|
||||
* - honor a custom `titleHostname` passed via the constructor (CLI flag
|
||||
* `--title-hostname <host>` plumbs through to here)
|
||||
* - HTML-escape the hostname so a value like `<script>foo</script>`
|
||||
* can't break out of the title tag
|
||||
* - replace the bare `<title>Codeman</title>` literal exactly once
|
||||
* - leave the rest of the document byte-for-byte identical to the
|
||||
* template on disk
|
||||
*
|
||||
* Strategy: construct WebServer with port 0 / testMode (no network
|
||||
* activity until start()) and call the private `renderIndexHtml()`
|
||||
* method directly. The Fastify `/` and `/index.html` route handlers
|
||||
* are one-liners that call exactly this method (server.ts:539-544),
|
||||
* so testing the render function covers both endpoints without
|
||||
* needing to listen on a port.
|
||||
*
|
||||
* Port: N/A (no server start)
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { hostname as osHostname } from 'node:os';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const indexHtmlPath = join(__dirname, '..', 'src', 'web', 'public', 'index.html');
|
||||
const rawTemplate = readFileSync(indexHtmlPath, 'utf-8');
|
||||
|
||||
function render(host?: string): string {
|
||||
const server = new WebServer(0, false, true, host);
|
||||
return (server as unknown as { renderIndexHtml: () => string }).renderIndexHtml();
|
||||
}
|
||||
|
||||
describe('WebServer index.html <title> templating (#82)', () => {
|
||||
it('substitutes the bare <title>Codeman</title> with codeman:<host>', () => {
|
||||
const html = render('laptop');
|
||||
expect(html).toContain('<title>codeman:laptop</title>');
|
||||
expect(html).not.toContain('<title>Codeman</title>');
|
||||
});
|
||||
|
||||
it('defaults to os.hostname() when no titleHostname is supplied', () => {
|
||||
const html = render();
|
||||
const expected = `<title>codeman:${osHostname()}</title>`;
|
||||
expect(html).toContain(expected);
|
||||
});
|
||||
|
||||
it('treats an empty-string titleHostname as "not supplied" and falls back to os.hostname()', () => {
|
||||
// CLI normally guarantees a non-empty string, but the constructor's
|
||||
// `titleHostname || getHostname()` guard makes empty fall through —
|
||||
// pin that behavior so a future refactor doesn't accidentally ship
|
||||
// a `<title>codeman:</title>` to users.
|
||||
const html = render('');
|
||||
expect(html).toMatch(/<title>codeman:.+<\/title>/);
|
||||
expect(html).not.toContain('<title>codeman:</title>');
|
||||
});
|
||||
|
||||
it('HTML-escapes < > & in the hostname so it cannot break out of the title tag', () => {
|
||||
const html = render('<script>alert(1)</script>');
|
||||
expect(html).toContain('<title>codeman:<script>alert(1)</script></title>');
|
||||
// The raw closing </title> from the injected payload must NOT appear
|
||||
// outside the actual title element — escape-then-substitute prevents
|
||||
// an attacker-controlled hostname from terminating the tag early.
|
||||
expect(html).not.toContain('<script>alert(1)</script></title>');
|
||||
});
|
||||
|
||||
it('escapes an ampersand without double-encoding existing entities', () => {
|
||||
// The escaper replaces & first, then < and >. A hostname that already
|
||||
// contains a literal `&` should render as `&` once, not `&amp;`.
|
||||
const html = render('a&b');
|
||||
expect(html).toContain('<title>codeman:a&b</title>');
|
||||
expect(html).not.toContain('&amp;');
|
||||
});
|
||||
|
||||
it('only substitutes the <title> tag — the rest of the template is byte-for-byte identical', () => {
|
||||
const html = render('laptop');
|
||||
const beforeTitle = rawTemplate.split('<title>Codeman</title>')[0];
|
||||
const afterTitle = rawTemplate.split('<title>Codeman</title>')[1];
|
||||
expect(html.startsWith(beforeTitle)).toBe(true);
|
||||
expect(html.endsWith(afterTitle)).toBe(true);
|
||||
// Sanity check: length differs only by the title swap.
|
||||
const expectedDelta = `<title>codeman:laptop</title>`.length - `<title>Codeman</title>`.length;
|
||||
expect(html.length - rawTemplate.length).toBe(expectedDelta);
|
||||
});
|
||||
|
||||
it('replaces the <title> placeholder exactly once', () => {
|
||||
const html = render('laptop');
|
||||
// Defense against a future regression where the template gains a
|
||||
// second `<title>Codeman</title>` (e.g. inside a <noscript>) and only
|
||||
// the first gets templated — would leave a stale literal in the served
|
||||
// HTML that overrides the correct one in some renderers.
|
||||
const occurrencesOfNew = html.split('<title>codeman:laptop</title>').length - 1;
|
||||
const occurrencesOfOld = html.split('<title>Codeman</title>').length - 1;
|
||||
expect(occurrencesOfNew).toBe(1);
|
||||
expect(occurrencesOfOld).toBe(0);
|
||||
});
|
||||
|
||||
it('two WebServer instances on different hostnames render distinct titles', () => {
|
||||
const htmlA = render('host-a');
|
||||
const htmlB = render('host-b');
|
||||
expect(htmlA).toContain('<title>codeman:host-a</title>');
|
||||
expect(htmlB).toContain('<title>codeman:host-b</title>');
|
||||
expect(htmlA).not.toContain('host-b');
|
||||
expect(htmlB).not.toContain('host-a');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,163 @@
|
||||
/**
|
||||
* Covers `queryTmuxWindowSize()`, the helper extracted from `_attachToMux`
|
||||
* in PR #80 ("prevent tmux flicker on restart by matching existing window size").
|
||||
*
|
||||
* Before #80, the PTY was hardcoded to 120x40 on every attach. If a previous
|
||||
* client had resized the tmux window to e.g. 200x50, the re-attach would
|
||||
* shrink the window back to 120x40, then xterm.js would resize it again on
|
||||
* the next frame — visible flicker and one lost repaint of scrollback.
|
||||
*
|
||||
* The fix queries tmux for the actual window geometry first via
|
||||
* `tmux display -t <name> -p '#{window_width} #{window_height}'`. We cover:
|
||||
* - Happy path: tmux reports valid geometry → those numbers are used.
|
||||
* - Browser-resize-between-attaches: tmux reports a non-default size
|
||||
* (because a prior client resized it) → the helper picks that up.
|
||||
* - Query-then-die race: tmux dies between query and attach → the query
|
||||
* either throws or returns garbage; either way the helper falls back to
|
||||
* 120x40 so the attach can still proceed (the pty.spawn that follows has
|
||||
* its own try/catch for the actual failed-attach case).
|
||||
* - Defensive paths: empty output, non-numeric output, zero/negative
|
||||
* dimensions, trailing whitespace.
|
||||
* - Security: muxName is passed as an argv element (no shell), so a
|
||||
* malicious mux name can't inject options.
|
||||
*
|
||||
* Strategy: mock `node:child_process.execFileSync` and assert both the call
|
||||
* shape (argv, timeout) and the parsed return.
|
||||
*
|
||||
* Port: N/A (no server / no real tmux)
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { execFileSync } = vi.hoisted(() => ({
|
||||
execFileSync: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('node:child_process', async () => {
|
||||
const actual = await vi.importActual<typeof import('node:child_process')>('node:child_process');
|
||||
return { ...actual, execFileSync };
|
||||
});
|
||||
|
||||
import { queryTmuxWindowSize } from '../src/session.js';
|
||||
|
||||
const DEFAULT = { cols: 120, rows: 40 };
|
||||
|
||||
beforeEach(() => {
|
||||
execFileSync.mockReset();
|
||||
});
|
||||
|
||||
describe('queryTmuxWindowSize — happy path', () => {
|
||||
it('returns the geometry tmux reports', () => {
|
||||
execFileSync.mockReturnValue('200 50\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual({ cols: 200, rows: 50 });
|
||||
});
|
||||
|
||||
it('picks up a non-default size left behind by a prior client (browser-resize-between-attaches)', () => {
|
||||
// Scenario: client A attached at 220x60, resized tmux to that, then disconnected.
|
||||
// tmux keeps the last-attached geometry. Client B re-attaches and should spawn
|
||||
// its PTY at 220x60, not 120x40 — that's the whole point of #80.
|
||||
execFileSync.mockReturnValue('220 60');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual({ cols: 220, rows: 60 });
|
||||
});
|
||||
|
||||
it('tolerates trailing whitespace and newlines in tmux output', () => {
|
||||
execFileSync.mockReturnValue(' 180 45 \n\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual({ cols: 180, rows: 45 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('queryTmuxWindowSize — fallback paths', () => {
|
||||
it('falls back to 120x40 when tmux exits non-zero (process not found)', () => {
|
||||
// execFileSync throws when the child exits non-zero. Simulates `tmux` binary
|
||||
// missing or `display -t` failing because the target session doesn't exist.
|
||||
execFileSync.mockImplementation(() => {
|
||||
const err = new Error('Command failed: tmux display -t bogus') as Error & { status: number };
|
||||
err.status = 1;
|
||||
throw err;
|
||||
});
|
||||
expect(queryTmuxWindowSize('bogus')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux dies between query and parse (ETIMEDOUT / ENOENT)', () => {
|
||||
// Query-then-die race: simulates the tmux server being killed mid-call.
|
||||
execFileSync.mockImplementation(() => {
|
||||
const err = new Error('spawn ETIMEDOUT') as NodeJS.ErrnoException;
|
||||
err.code = 'ETIMEDOUT';
|
||||
throw err;
|
||||
});
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns empty output', () => {
|
||||
execFileSync.mockReturnValue('');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns whitespace-only output', () => {
|
||||
execFileSync.mockReturnValue(' \n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns non-numeric output', () => {
|
||||
execFileSync.mockReturnValue('not a size\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when only one dimension is present', () => {
|
||||
execFileSync.mockReturnValue('200\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when a dimension is zero (degenerate geometry)', () => {
|
||||
// tmux reporting `0` would crash node-pty downstream — must not propagate.
|
||||
execFileSync.mockReturnValue('0 40\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
execFileSync.mockReturnValue('120 0\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when a dimension is negative', () => {
|
||||
execFileSync.mockReturnValue('-200 -50\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns NaN-producing tokens', () => {
|
||||
execFileSync.mockReturnValue('abc def\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
});
|
||||
});
|
||||
|
||||
describe('queryTmuxWindowSize — call shape', () => {
|
||||
it('invokes tmux with display -t <name> -p ... via argv (not a shell)', () => {
|
||||
execFileSync.mockReturnValue('120 40\n');
|
||||
queryTmuxWindowSize('codeman-abc');
|
||||
expect(execFileSync).toHaveBeenCalledTimes(1);
|
||||
const [bin, argv, opts] = execFileSync.mock.calls[0];
|
||||
expect(bin).toBe('tmux');
|
||||
expect(argv).toEqual(['display', '-t', 'codeman-abc', '-p', '#{window_width} #{window_height}']);
|
||||
// execFileSync — not execSync — so muxName is never substituted into a shell string.
|
||||
expect(opts).toMatchObject({ encoding: 'utf8' });
|
||||
});
|
||||
|
||||
it('uses a bounded timeout so a hung tmux server cannot block startup forever', () => {
|
||||
execFileSync.mockReturnValue('120 40\n');
|
||||
queryTmuxWindowSize('codeman-abc');
|
||||
const [, , opts] = execFileSync.mock.calls[0];
|
||||
// Whatever the exact constant, the contract is: ≤5s so the user-visible
|
||||
// attach path can't hang on a stuck tmux server.
|
||||
expect(typeof opts?.timeout).toBe('number');
|
||||
expect(opts?.timeout).toBeGreaterThan(0);
|
||||
expect(opts?.timeout).toBeLessThanOrEqual(5000);
|
||||
});
|
||||
|
||||
it('passes a muxName that looks like a tmux flag as an argv element (no option injection)', () => {
|
||||
execFileSync.mockReturnValue('120 40\n');
|
||||
queryTmuxWindowSize('-x 1 -y 1; rm -rf');
|
||||
const [, argv] = execFileSync.mock.calls[0];
|
||||
// The whole "name" lives in a single argv slot, so tmux interprets it as a
|
||||
// target session name, not as additional flags. The `-t` flag preceding it
|
||||
// pins it as the target argument.
|
||||
expect(argv?.[2]).toBe('-x 1 -y 1; rm -rf');
|
||||
expect((argv as string[]).indexOf('-x')).toBe(-1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user