diff --git a/CLAUDE.md b/CLAUDE.md index 1515818c..58f79646 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,7 +77,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph | Task | Command | |------|---------| | Dev with TLS | `npx tsx src/index.ts web --https` | -| Override window title hostname | `npx tsx src/index.ts web --title-hostname ` (default: `os.hostname()` — tab title renders as `codeman:`) | +| Override window title hostname | `npx tsx src/index.ts web --title-hostname ` (default: `os.hostname()` — `codeman:` is used for tab title, title-flash, and OS desktop notification prefix) | | Continuous typecheck | `tsc --noEmit --watch` | | Test coverage | `npm run test:coverage` | | Dead-code sweep | `npm run knip` (config in `knip.json`) | @@ -95,8 +95,9 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph - **Package ≠ product name** — npm: `aicodeman`, product: **Codeman**. Release renames tags accordingly - **Global regex `lastIndex`** — Shared `g`-flag patterns in loops must reset `lastIndex = 0` first, or use the `execPattern()` helper in `utils/regex-patterns.ts` (resets automatically) - **`envOverrides` flow `CLAUDE_CODE_*` / `OPENCODE_*` env vars** — Set via `POST /api/sessions { envOverrides }`, stored on `Session._envOverrides`, exported by `tmux-manager.buildEnvExports()` at spawn time, persisted in `SessionState.envOverrides`. **Do NOT** write these to `/.claude/settings.local.json` — that's the old path and creates UI/disk drift +- **Dual-CLI prefix discipline** — Codeman supports both Claude Code and OpenCode (`claude-cli-resolver.ts` / `opencode-cli-resolver.ts`); env-var prefix is CLI-specific (`CLAUDE_CODE_*` vs `OPENCODE_*`) and the allowlist in `schemas.ts` enforces this. When adding settings, decide which CLI(s) it applies to and gate the env export accordingly — don't blindly forward both prefixes - **Zod `.optional()` rejects `null`** — accepts `undefined` only. When the frontend builds a request body with `JSON.stringify`, an explicit `null` field is preserved on the wire and fails validation with `INVALID_INPUT`. Convert `null` → `undefined` before stringifying (e.g. `field: value ?? undefined`), or declare the schema `.nullish()`. Real bugs caused: 0.6.4 (`durationMinutes` for ∞ respawn), and the same shape pattern hit `opusContext1mEnabled` in 0.6.3 -- **`xterm-zerolag-input` is duplicated** — the local-echo overlay lives in BOTH `packages/xterm-zerolag-input/src/` (published package) AND inline inside `src/web/public/app.js` (runtime copy used by the web UI). Any change to overlay behavior MUST be applied to both, or dev and prod diverge. Always test on mobile after touching it. +- **`xterm-zerolag-input` is duplicated** — the local-echo overlay lives in BOTH `packages/xterm-zerolag-input/src/` (published to npm as a standalone library for external consumers — see README "Published Packages") AND inline inside `src/web/public/app.js` (runtime copy the web UI actually loads, since the page ships as plain JS without a bundler). Any change to overlay behavior MUST be applied to both, or dev and prod diverge — and a public API break in the package warrants a separate version bump for `xterm-zerolag-input` in the changeset. Always test on mobile after touching it. **Import conventions**: Utils from `./utils`, types from `./types` (barrel), config from specific `./config/*` files. diff --git a/README.md b/README.md index bec7bc48..6e88561c 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ codeman web # codeman: codeman web --title-hostname dev-box # codeman:dev-box (manual override for noisy hostnames) ``` -The title is templated into the served HTML on first byte, so it's correct from the very first paint and works without JavaScript. +The title is templated into the served HTML on first byte, so it's correct from the very first paint and works without JavaScript. The same hostname prefix is applied to the tab-flash format (`⚠️ (N) codeman:`) and to OS-level desktop notifications (`codeman:: `), so cross-host alerts in the system notification center are also unambiguous. ### Smart Token Management diff --git a/src/session.ts b/src/session.ts index 05ee5e08..a483e054 100644 --- a/src/session.ts +++ b/src/session.ts @@ -121,6 +121,37 @@ const NEWLINE_SPLIT_PATTERN = /\r?\n/; // Note: Claude CLI PATH resolution moved to session-cli-builder.ts (buildClaudeEnv) +/** PTY fallback geometry when tmux can't be queried (matches pre-#80 hardcoded values). */ +const DEFAULT_PTY_COLS = 120; +const DEFAULT_PTY_ROWS = 40; +const TMUX_DISPLAY_TIMEOUT_MS = 2000; + +/** + * Ask tmux for the current window geometry of `muxName` so a re-attaching PTY + * client can spawn at the same size and avoid the resize-flicker / scrollback + * loss documented in #80. Returns `{ cols: 120, rows: 40 }` on any failure + * (tmux dead, muxName unknown, malformed output) — caller never has to + * differentiate "tmux unreachable" from "size 120x40". + * + * Argv form (execFileSync, not execSync) keeps `muxName` out of any shell so + * a hostile session name can't inject options. + */ +export function queryTmuxWindowSize(muxName: string): { cols: number; rows: number } { + try { + const sizeStr = execFileSync('tmux', ['display', '-t', muxName, '-p', '#{window_width} #{window_height}'], { + timeout: TMUX_DISPLAY_TIMEOUT_MS, + encoding: 'utf8', + }).trim(); + const [w, h] = sizeStr.split(' ').map(Number); + if (w > 0 && h > 0) { + return { cols: w, rows: h }; + } + } catch { + /* fall back below */ + } + return { cols: DEFAULT_PTY_COLS, rows: DEFAULT_PTY_ROWS }; +} + /** * Represents a JSON message from Claude CLI's stream-json output format. * Messages are newline-delimited JSON objects parsed from PTY output. @@ -947,22 +978,7 @@ export class Session extends EventEmitter { // Attach to the mux session via PTY // Query existing tmux window size so re-attach matches (avoids flicker from 120x40 default) - let ptyCols = 120; - let ptyRows = 40; - try { - const sizeStr = execFileSync( - 'tmux', - ['display', '-t', this._muxSession!.muxName, '-p', '#{window_width} #{window_height}'], - { timeout: 2000, encoding: 'utf8' } - ).trim(); - const [w, h] = sizeStr.split(' ').map(Number); - if (w > 0 && h > 0) { - ptyCols = w; - ptyRows = h; - } - } catch { - /* fall back to 120x40 */ - } + const { cols: ptyCols, rows: ptyRows } = queryTmuxWindowSize(this._muxSession!.muxName); try { this.ptyProcess = pty.spawn(mux.getAttachCommand(), mux.getAttachArgs(this._muxSession!.muxName), { name: 'xterm-256color', diff --git a/src/web/public/notification-manager.js b/src/web/public/notification-manager.js index 58801dd5..40fad292 100644 --- a/src/web/public/notification-manager.js +++ b/src/web/public/notification-manager.js @@ -3,7 +3,7 @@ * * The NotificationManager class implements five notification layers: * 1. In-app notification drawer (slide-out panel with grouped notifications) - * 2. Tab title flash (alternating "(*) Codeman" when tab is hidden) + * 2. Tab title flash (alternating "⚠️ (N) codeman:" / "codeman:" when tab is hidden; uses this.originalTitle so it tracks any per-host title) * 3. Browser Notification API (desktop push with auto-close after 8s) * 4. Web Push via service worker (OS-level notifications when tab is closed) * 5. Audio alerts (Web Audio API beep, user-opt-in) @@ -330,7 +330,7 @@ class NotificationManager { if (now - this.lastBrowserNotifTime < BROWSER_NOTIF_RATE_LIMIT_MS) return; this.lastBrowserNotifTime = now; - const notif = new Notification(`Codeman: ${title}`, { + const notif = new Notification(`${this.originalTitle}: ${title}`, { body, tag, // Groups same-tag notifications icon: '/favicon.ico', diff --git a/test/server-index-title.test.ts b/test/server-index-title.test.ts new file mode 100644 index 00000000..85deba6c --- /dev/null +++ b/test/server-index-title.test.ts @@ -0,0 +1,111 @@ +/** + * Verifies that WebServer templates the `` 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` 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 templating (#82)', () => { + it('substitutes the bare <title>Codeman with codeman:', () => { + const html = render('laptop'); + expect(html).toContain('codeman:laptop'); + expect(html).not.toContain('Codeman'); + }); + + it('defaults to os.hostname() when no titleHostname is supplied', () => { + const html = render(); + const expected = `codeman:${osHostname()}`; + 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 `codeman:` to users. + const html = render(''); + expect(html).toMatch(/codeman:.+<\/title>/); + expect(html).not.toContain('<title>codeman:'); + }); + + it('HTML-escapes < > & in the hostname so it cannot break out of the title tag', () => { + const html = render(''); + expect(html).toContain('codeman:<script>alert(1)</script>'); + // The raw closing 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(''); + }); + + 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('codeman:a&b'); + expect(html).not.toContain('&amp;'); + }); + + it('only substitutes the tag — the rest of the template is byte-for-byte identical', () => { + const html = render('laptop'); + const beforeTitle = rawTemplate.split('<title>Codeman')[0]; + const afterTitle = rawTemplate.split('Codeman')[1]; + expect(html.startsWith(beforeTitle)).toBe(true); + expect(html.endsWith(afterTitle)).toBe(true); + // Sanity check: length differs only by the title swap. + const expectedDelta = `codeman:laptop`.length - `Codeman`.length; + expect(html.length - rawTemplate.length).toBe(expectedDelta); + }); + + it('replaces the placeholder exactly once', () => { + const html = render('laptop'); + // Defense against a future regression where the template gains a + // second `<title>Codeman` (e.g. inside a