From 7c07284b95a121ff47f62a002941440ef4af52db Mon Sep 17 00:00:00 2001 From: lior Date: Tue, 28 Jul 2026 23:12:27 +0300 Subject: [PATCH 01/24] test: isolate quick-start case fixtures --- test/quick-start.test.ts | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/test/quick-start.test.ts b/test/quick-start.test.ts index 330fe5d9..9611a401 100644 --- a/test/quick-start.test.ts +++ b/test/quick-start.test.ts @@ -1,11 +1,28 @@ import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; -import { WebServer } from '../src/web/server.js'; -import { existsSync, rmSync, mkdirSync } from 'node:fs'; +import type { WebServer } from '../src/web/server.js'; +import { existsSync, rmSync, mkdirSync, mkdtempSync } from 'node:fs'; import { join } from 'node:path'; -import { homedir } from 'node:os'; +import { tmpdir } from 'node:os'; const TEST_PORT = 3099; -const CASES_DIR = join(homedir(), 'codeman-cases'); +const ORIGINAL_HOME = process.env.HOME; +const TEST_HOME = mkdtempSync(join(tmpdir(), 'codeman-quick-start-')); +const CASES_DIR = join(TEST_HOME, 'codeman-cases'); +let webServerModule: Promise | undefined; + +process.env.HOME = TEST_HOME; + +async function createTestServer(port: number): Promise { + webServerModule ??= import('../src/web/server.js'); + const { WebServer: TestWebServer } = await webServerModule; + return new TestWebServer(port, false, true); +} + +afterAll(() => { + if (ORIGINAL_HOME === undefined) delete process.env.HOME; + else process.env.HOME = ORIGINAL_HOME; + rmSync(TEST_HOME, { recursive: true, force: true }); +}); describe('Quick Start API', () => { let server: WebServer; @@ -13,7 +30,7 @@ describe('Quick Start API', () => { const createdCases: string[] = []; beforeAll(async () => { - server = new WebServer(TEST_PORT, false, true); + server = await createTestServer(TEST_PORT); await server.start(); baseUrl = `http://localhost:${TEST_PORT}`; }); @@ -147,7 +164,7 @@ describe('Session Management', () => { let baseUrl: string; beforeAll(async () => { - server = new WebServer(TEST_PORT + 1, false, true); + server = await createTestServer(TEST_PORT + 1); await server.start(); baseUrl = `http://localhost:${TEST_PORT + 1}`; }); @@ -206,7 +223,7 @@ describe('Case Management', () => { const createdCases: string[] = []; beforeAll(async () => { - server = new WebServer(TEST_PORT + 2, false, true); + server = await createTestServer(TEST_PORT + 2); await server.start(); baseUrl = `http://localhost:${TEST_PORT + 2}`; }); From 3c903b36ca60e398c7cb620376f6fd38d2265eeb Mon Sep 17 00:00:00 2001 From: lior Date: Tue, 28 Jul 2026 23:16:37 +0300 Subject: [PATCH 02/24] fix(hooks): reawaken jobs without replacing user hooks --- docs/claude-code-hooks-reference.md | 114 ++++++++++++---- src/hooks-config.ts | 193 ++++++++++++++++++++++++++-- src/web/routes/session-routes.ts | 8 +- test/hook-secret-selfheal.test.ts | 61 +++++++-- test/hooks-config.test.ts | 147 ++++++++++++++++++++- 5 files changed, 467 insertions(+), 56 deletions(-) diff --git a/docs/claude-code-hooks-reference.md b/docs/claude-code-hooks-reference.md index e11aad09..ec309406 100644 --- a/docs/claude-code-hooks-reference.md +++ b/docs/claude-code-hooks-reference.md @@ -2,14 +2,18 @@ > Official documentation for Claude Code hooks system, extracted from [code.claude.com](https://code.claude.com/docs/en/hooks). -**Last Updated**: 2026-01-24 +**Last Updated**: 2026-07-25 **Source**: [Claude Code Hooks Documentation](https://code.claude.com/docs/en/hooks) +> This is a maintained summary, not an exhaustive copy of the upstream reference. +> Check the source link for event-specific schemas before adding a new hook. + --- ## Overview Hooks are automated scripts that execute at specific events during your Claude Code session. They allow you to: + - Validate, modify, or block tool usage - Add context to prompts - Implement custom workflows @@ -21,12 +25,12 @@ Hooks are automated scripts that execute at specific events during your Claude C Hooks are configured in settings files: -| File | Scope | -|------|-------| -| `~/.claude/settings.json` | User (global) | -| `.claude/settings.json` | Project | +| File | Scope | +| ----------------------------- | -------------------------- | +| `~/.claude/settings.json` | User (global) | +| `.claude/settings.json` | Project | | `.claude/settings.local.json` | Local project (gitignored) | -| Plugin hook files | Plugin-specific | +| Plugin hook files | Plugin-specific | ### Basic Structure @@ -49,8 +53,9 @@ Hooks are configured in settings files: ``` **Key Fields**: + - `matcher`: Pattern to match tool names (case-sensitive, supports regex like `Edit|Write` or `*` for all) -- `type`: `"command"` for bash or `"prompt"` for LLM-based evaluation +- `type`: `"command"`, `"http"`, `"mcp_tool"`, `"prompt"`, or `"agent"` where the event supports it - `command`: Bash command to execute - `prompt`: LLM prompt for evaluation (prompt-based hooks only) - `timeout`: Optional timeout in seconds (default: 60) @@ -59,6 +64,10 @@ Hooks are configured in settings files: ## Hook Events +Claude Code's current event surface is broader than the detailed subset below. In +particular, `TeammateIdle` and `TaskCompleted` are supported lifecycle events used +by Codeman; they are not stale or plugin-defined event names. + ### PreToolUse **When**: After Claude creates tool parameters, before processing the tool call. @@ -66,15 +75,17 @@ Hooks are configured in settings files: **Use Cases**: Approval, denial, or modification of tool calls. **Common Matchers**: + - `Bash` - Shell commands - `Write` - File writing - `Edit` - File editing - `Read` - File reading -- `Task` - Subagent tasks +- `Agent` - Subagent tasks - `WebFetch`, `WebSearch` - Web operations - `mcp____` - MCP tools **Output Control**: + ```json { "hookSpecificOutput": { @@ -96,13 +107,14 @@ Hooks are configured in settings files: **Use Cases**: Auto-approve or deny permissions. **Output Control**: + ```json { "hookSpecificOutput": { "hookEventName": "PermissionRequest", "decision": { "behavior": "allow|deny", - "updatedInput": { }, + "updatedInput": {}, "message": "deny reason", "interrupt": false } @@ -117,6 +129,7 @@ Hooks are configured in settings files: **Use Cases**: Provide feedback, run formatters/linters, log operations. **Output Control**: + ```json { "decision": "block", @@ -128,15 +141,30 @@ Hooks are configured in settings files: } ``` +#### Asynchronous Rewake + +Command hooks can set `"asyncRewake": true` to run asynchronously and wake an +idle Claude turn when the hook exits with code 2. The hook's stderr is delivered +to Claude as a system reminder. This implies `"async": true`; ordinary async +hooks do not wake an idle turn, and their output waits for the next interaction. + +Codeman uses this on `PostToolUse(Bash)`: a self-contained Node helper extracts +the background task ID from the Bash result, watches the session transcript for +the matching completion notification, and exits 2. It does not send terminal +input, so it cannot submit a user's partially written prompt. + ### Notification **When**: When Claude Code sends notifications. **Matchers**: + - `permission_prompt` - `idle_prompt` - `auth_success` - `elicitation_dialog` +- `elicitation_complete` +- `elicitation_response` ### UserPromptSubmit @@ -145,6 +173,7 @@ Hooks are configured in settings files: **Use Cases**: Add context, validate, or block prompts. **Output Control**: + ```json { "decision": "block", @@ -165,6 +194,7 @@ Hooks are configured in settings files: **Use Cases**: **Ralph Wiggum loops** - block exit and refeed prompt. **Output Control**: + ```json { "decision": "block", @@ -173,6 +203,7 @@ Hooks are configured in settings files: ``` Or to allow exit: + ```json { "continue": true, @@ -184,15 +215,32 @@ Or to allow exit: ### SubagentStop -**When**: When a subagent (Task tool call) finishes responding. +**When**: When a subagent (Agent tool call) finishes responding. **Use Cases**: Control nested loops, verify subagent output. +### TeammateIdle + +**When**: When an agent-team teammate is about to go idle. + +**Use Cases**: Reassign work, continue a teammate loop, or notify an orchestrator. + +**Matcher Support**: None. The hook fires for every occurrence. + +### TaskCompleted + +**When**: When a task is about to be marked completed. + +**Use Cases**: Validate completion or forward team progress to an external UI. + +**Matcher Support**: None. The hook fires for every occurrence. + ### PreCompact **When**: Before a compact operation. **Matchers**: + - `manual` - Invoked from `/compact` - `auto` - Invoked from auto-compact @@ -201,6 +249,7 @@ Or to allow exit: **When**: When Claude Code starts or resumes a session. **Matchers**: + - `startup` - Fresh start - `resume` - From `--resume`, `--continue`, or `/resume` - `clear` - From `/clear` @@ -209,6 +258,7 @@ Or to allow exit: **Use Cases**: Load development context, set environment variables. **Persisting Environment Variables**: + ```bash #!/bin/bash if [ -n "$CLAUDE_ENV_FILE" ]; then @@ -219,6 +269,7 @@ exit 0 ``` **Output Control**: + ```json { "hookSpecificOutput": { @@ -233,6 +284,7 @@ exit 0 **When**: When a session ends. **Reason Values**: + - `clear` - `logout` - `prompt_input_exit` @@ -254,7 +306,7 @@ Hooks receive JSON via stdin with common fields: "permission_mode": "default", "hook_event_name": "PreToolUse", "tool_name": "Bash", - "tool_input": { }, + "tool_input": {}, "tool_use_id": "toolu_01ABC123..." } ``` @@ -262,6 +314,7 @@ Hooks receive JSON via stdin with common fields: ### Tool-Specific Input **Bash**: + ```json { "tool_name": "Bash", @@ -274,6 +327,7 @@ Hooks receive JSON via stdin with common fields: ``` **Write**: + ```json { "tool_name": "Write", @@ -285,6 +339,7 @@ Hooks receive JSON via stdin with common fields: ``` **Edit**: + ```json { "tool_name": "Edit", @@ -302,11 +357,11 @@ Hooks receive JSON via stdin with common fields: ### Exit Codes -| Code | Behavior | -|------|----------| -| 0 | Success. `stdout` processed (shown in verbose or added as context) | -| 2 | Blocking error. Only `stderr` used. Blocks tool/prompt based on event | -| Other | Non-blocking error. `stderr` shown in verbose, execution continues | +| Code | Behavior | +| ----- | --------------------------------------------------------------------- | +| 0 | Success. `stdout` processed (shown in verbose or added as context) | +| 2 | Blocking error. Only `stderr` used. Blocks tool/prompt based on event | +| Other | Non-blocking error. `stderr` shown in verbose, execution continues | ### JSON Output (Exit Code 0) @@ -323,7 +378,12 @@ Hooks receive JSON via stdin with common fields: ## Prompt-Based Hooks -For Stop and SubagentStop events, you can use LLM-based evaluation: +Prompt and agent handlers are supported by decision-oriented events including +`PreToolUse`, `PermissionRequest`, `PostToolUse`, `PostToolUseFailure`, +`PostToolBatch`, `UserPromptSubmit`, `Stop`, `SubagentStop`, `TaskCreated`, and +`TaskCompleted`. Check the upstream reference before choosing a handler type. + +For example, a Stop event can use LLM-based evaluation: ```json { @@ -344,6 +404,7 @@ For Stop and SubagentStop events, you can use LLM-based evaluation: ``` **LLM Response Format**: + ```json { "ok": true, @@ -362,17 +423,18 @@ Hooks can be defined in Skills, Agents, and Slash Commands using frontmatter: name: secure-operations hooks: PreToolUse: - - matcher: "Bash" + - matcher: 'Bash' hooks: - type: command - command: "./scripts/security-check.sh" + command: './scripts/security-check.sh' --- ``` These hooks: + - Are scoped to the component's lifecycle - Only run when that component is active -- Support: PreToolUse, PostToolUse, Stop +- Support all hook events; a subagent-scoped `Stop` is converted to `SubagentStop` --- @@ -550,11 +612,11 @@ exit 0 ## Environment Variables -| Variable | Description | -|----------|-------------| -| `CLAUDE_PROJECT_DIR` | Project root directory | -| `CLAUDE_CODE_REMOTE` | `"true"` for web, empty for CLI | -| `CLAUDE_ENV_FILE` | Path to write persistent env vars (SessionStart) | +| Variable | Description | +| -------------------- | ------------------------------------------------ | +| `CLAUDE_PROJECT_DIR` | Project root directory | +| `CLAUDE_CODE_REMOTE` | `"true"` for web, empty for CLI | +| `CLAUDE_ENV_FILE` | Path to write persistent env vars (SessionStart) | --- @@ -593,4 +655,4 @@ Use `/hooks` command to view registered hooks and make changes. --- -*Source: [Claude Code Hooks Documentation](https://code.claude.com/docs/en/hooks)* +_Source: [Claude Code Hooks Documentation](https://code.claude.com/docs/en/hooks)_ diff --git a/src/hooks-config.ts b/src/hooks-config.ts index ffc8b840..ca8b0a2f 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -16,7 +16,7 @@ * `stop`, `teammate_idle`, `task_completed` * * Hook categories: `Notification` (3 matchers), `Stop` (1), `TeammateIdle` (1), - * `TaskCompleted` (1) + * `TaskCompleted` (1), `PostToolUse` (1 self-contained background Bash rewake) * * @dependencies types (HookEventType), config/auth-config (HOOK_TIMEOUT_MS) * @consumedby web/server (session creation), session-cli-builder (env setup) @@ -40,6 +40,85 @@ import { HOOK_TIMEOUT_MS } from './config/auth-config.js'; * are independent; the map self-prunes when a path's chain goes idle. */ const settingsWriteLocks = new Map>(); +const BACKGROUND_WAKE_MARKER = 'CODEMAN_BACKGROUND_REWAKE_V1'; +const BACKGROUND_WAKE_TIMEOUT_SECONDS = 6 * 60 * 60; + +/** + * Inline Node helper for Claude Code's `asyncRewake` hook. + * + * A background Bash tool returns immediately with a task ID, then Claude writes + * its completion as a queue-operation in the transcript. Watching that durable + * record avoids injecting terminal input (which could submit a user's draft). + * The helper is embedded in settings via `node -e`, so it has no script path + * that can go stale after an install or plugin-cache cleanup. + */ +export function generateBackgroundWakeScript(): string { + return [ + "const fs = require('node:fs');", + `const ${BACKGROUND_WAKE_MARKER} = true;`, + 'let input = {};', + "try { input = JSON.parse(fs.readFileSync(0, 'utf8') || '{}'); } catch { process.exit(0); }", + 'function findTaskId(value) {', + " const idKeys = new Set(['taskId', 'task_id', 'shellId', 'shell_id', 'backgroundTaskId', 'background_task_id']);", + ' const stack = [value];', + ' const seen = new Set();', + ' while (stack.length > 0) {', + ' const current = stack.pop();', + " if (!current || typeof current !== 'object' || seen.has(current)) continue;", + ' seen.add(current);', + ' for (const [key, nested] of Object.entries(current)) {', + " if (idKeys.has(key) && typeof nested === 'string' && /^[A-Za-z0-9_-]+$/.test(nested)) return nested;", + " if (nested && typeof nested === 'object') stack.push(nested);", + ' }', + ' }', + " const serialized = JSON.stringify(value ?? '');", + ' const messageMatch = serialized.match(/Command running in background with ID:\\s*([A-Za-z0-9_-]+)/i);', + ' if (messageMatch) return messageMatch[1];', + ' const pathMatch = serialized.match(/[\\\\/]tasks[\\\\/]([A-Za-z0-9_-]+)\\.output/i);', + ' return pathMatch ? pathMatch[1] : null;', + '}', + 'const taskId = findTaskId(input.tool_response);', + "const transcriptPath = typeof input.transcript_path === 'string' ? input.transcript_path : '';", + 'if (!taskId || !transcriptPath) process.exit(0);', + 'let position = 0;', + 'try { position = Math.max(0, fs.statSync(transcriptPath).size - 262144); } catch { process.exit(0); }', + "let carry = '';", + 'function inspect(text) {', + ' for (const line of text.split(/\\r?\\n/)) {', + ' if (!line.includes(taskId)) continue;', + ' let entry;', + ' try { entry = JSON.parse(line); } catch { continue; }', + " if (entry.type !== 'queue-operation' || typeof entry.content !== 'string') continue;", + " if (!entry.content.includes('' + taskId + '')) continue;", + ' const status = entry.content.match(/(completed|failed|killed|error)<\\/status>/i);', + ' if (!status) continue;', + ' const output = entry.content.match(/([^<]+)<\\/output-file>/i);', + " const location = output ? ' Read ' + output[1] + ' and' : '';", + " console.error('Background command ' + taskId + ' ' + status[1].toLowerCase() + '.' + location + ' continue the task.');", + ' process.exit(2);', + ' }', + '}', + 'function poll() {', + ' try {', + ' const size = fs.statSync(transcriptPath).size;', + " if (size < position) { position = 0; carry = ''; }", + ' if (size > position) {', + ' const length = Math.min(size - position, 1048576);', + ' const buffer = Buffer.allocUnsafe(length);', + " const fd = fs.openSync(transcriptPath, 'r');", + ' const bytes = fs.readSync(fd, buffer, 0, length, position);', + ' fs.closeSync(fd);', + ' position += bytes;', + " carry = (carry + buffer.subarray(0, bytes).toString('utf8')).slice(-262144);", + ' inspect(carry);', + ' }', + ' } catch {}', + ' setTimeout(poll, 1000);', + '}', + 'poll();', + ].join('\n'); +} + function withSettingsLock(path: string, fn: () => Promise): Promise { const prev = settingsWriteLocks.get(path) ?? Promise.resolve(); const run = prev.then(fn, fn); // run after the prior writer, regardless of its outcome @@ -112,10 +191,91 @@ export function generateHooksConfig(): { hooks: Record } { hooks: [{ type: 'command', command: curlCmd('task_completed'), timeout: HOOK_TIMEOUT_MS }], }, ], + PostToolUse: [ + { + matcher: 'Bash', + hooks: [ + { + type: 'command', + command: 'node', + args: ['-e', generateBackgroundWakeScript()], + asyncRewake: true, + timeout: BACKGROUND_WAKE_TIMEOUT_SECONDS, + }, + ], + }, + ], }, }; } +function isCodemanHookHandler(value: unknown): boolean { + try { + const serialized = JSON.stringify(value); + return serialized.includes('/api/hook-event') || serialized.includes(BACKGROUND_WAKE_MARKER); + } catch { + return false; + } +} + +/** + * Replace only Codeman-owned command handlers while preserving user events, + * matcher entries, and sibling handlers in mixed entries. + */ +function mergeCodemanHooks(existingValue: unknown, generated: Record): Record { + const existing = + existingValue && typeof existingValue === 'object' && !Array.isArray(existingValue) + ? (existingValue as Record) + : {}; + const merged: Record = {}; + + for (const eventName of new Set([...Object.keys(existing), ...Object.keys(generated)])) { + const existingEntries = Array.isArray(existing[eventName]) ? (existing[eventName] as unknown[]) : []; + const generatedEntries = generated[eventName]; + if (!generatedEntries) { + merged[eventName] = existingEntries; + continue; + } + + const entries: unknown[] = []; + let insertedGenerated = false; + for (const entry of existingEntries) { + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { + if (!isCodemanHookHandler(entry)) entries.push(entry); + continue; + } + + const record = entry as Record; + if (!Array.isArray(record.hooks)) { + if (isCodemanHookHandler(record)) { + if (!insertedGenerated) { + entries.push(...generatedEntries); + insertedGenerated = true; + } + } else { + entries.push(entry); + } + continue; + } + + const retainedHandlers = record.hooks.filter((handler) => !isCodemanHookHandler(handler)); + const removedCodemanHandler = retainedHandlers.length !== record.hooks.length; + if (removedCodemanHandler && !insertedGenerated) { + entries.push(...generatedEntries); + insertedGenerated = true; + } + if (retainedHandlers.length > 0 || !removedCodemanHandler) { + entries.push(retainedHandlers.length === record.hooks.length ? entry : { ...record, hooks: retainedHandlers }); + } + } + + if (!insertedGenerated) entries.push(...generatedEntries); + merged[eventName] = entries; + } + + return merged; +} + /** * Remove a subset of env keys from .claude/settings.local.json.env if present. * Used during the disk→tmux-setenv migration: when the caller is actively setting @@ -237,29 +397,31 @@ export async function writeHooksConfig(casePath: string): Promise { } const hooksConfig = generateHooksConfig(); - const merged = { ...existing, ...hooksConfig }; + const merged = { + ...existing, + hooks: mergeCodemanHooks(existing.hooks, hooksConfig.hooks), + }; await writeFile(settingsPath, JSON.stringify(merged, null, 2) + '\n'); }); } /** - * Self-heal a case's hooks block so the COD-91 unconditional hook-secret gate keeps - * accepting its hook events. + * Self-heal a case's Codeman-owned hooks block. * * `writeHooksConfig` only runs when a case is first CREATED. Cases created before the * X-Codeman-Hook-Secret header was added (COD-54, 2026-06-10) keep hook curls in their * settings.local.json that POST to /api/hook-event WITHOUT the secret — which, once the - * gate requires it unconditionally (COD-91), silently 401 on a password-protected - * install. This refreshes the hooks block so those stale curls regain the header. + * gate requires it unconditionally (COD-91), silently 401 on a password-protected install. + * Older Codeman blocks also lack the background Bash async-rewake hook. Refresh either + * stale shape on launch so existing cases gain both current behaviors. * * Deliberately surgical: regenerates ONLY when settings.local.json already contains - * Codeman's own hook curls (they target `/api/hook-event`) that lack the secret header. - * No-op when the file/hooks are absent (we never impose hooks on a user who removed - * them), when the hooks aren't ours, or when the secret is already present — so it never - * clobbers a user's customizations and is cheap enough to call on every Claude spawn. + * Codeman's own hook curls (they target `/api/hook-event`) and they are stale. No-op + * when the file/hooks are absent (we never impose hooks on a user who removed them) or + * when the hooks aren't ours, so it is cheap enough to call on every Claude spawn. */ -export async function refreshStaleHookSecret(casePath: string): Promise { +export async function refreshStaleCodemanHooks(casePath: string): Promise { const settingsPath = join(casePath, '.claude', 'settings.local.json'); if (!existsSync(settingsPath)) return; await withSettingsLock(settingsPath, async () => { @@ -274,8 +436,13 @@ export async function refreshStaleHookSecret(casePath: string): Promise { // The generated curl carries this header literal (see generateHooksConfig); its // absence on our own hooks means they predate COD-54 and need regenerating. const hasSecret = hooksJson.includes('X-Codeman-Hook-Secret'); - if (!isOurs || hasSecret) return; - const merged = { ...existing, ...generateHooksConfig() }; + const hasBackgroundWake = hooksJson.includes(BACKGROUND_WAKE_MARKER); + if (!isOurs || (hasSecret && hasBackgroundWake)) return; + const generated = generateHooksConfig(); + const merged = { + ...existing, + hooks: mergeCodemanHooks(existing.hooks, generated.hooks), + }; await writeFile(settingsPath, JSON.stringify(merged, null, 2) + '\n'); }); } diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 20636f8a..0588c52b 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -65,7 +65,7 @@ import { updateCaseModel, stripCaseEnvKeys, applyStatusLineConfig, - refreshStaleHookSecret, + refreshStaleCodemanHooks, } from '../../hooks-config.js'; import { generateClaudeMd } from '../../templates/claude-md.js'; import { imageWatcher } from '../../image-watcher.js'; @@ -445,7 +445,7 @@ export function registerSessionRoutes( // unconditional hook-secret gate keeps accepting its hook events. No-op for fresh // cases (writeHooksConfig already wrote the secret) and for non-Codeman/absent hooks. if ((body.mode ?? 'claude') === 'claude') { - await refreshStaleHookSecret(workingDir).catch(() => {}); + await refreshStaleCodemanHooks(workingDir).catch(() => {}); } // Check OpenCode availability if requested @@ -2170,7 +2170,7 @@ export function registerSessionRoutes( // now-unconditional hook-secret gate keeps accepting its hook events. No-op when // the hooks aren't ours or already carry the secret. Skipped for remote cases — // resolvedCasePath is a REMOTE path that doesn't exist on the local filesystem. - await refreshStaleHookSecret(resolvedCasePath).catch(() => {}); + await refreshStaleCodemanHooks(resolvedCasePath).catch(() => {}); } // Docker cases: the workspace is a REAL host dir bind-mounted into the container. @@ -2186,7 +2186,7 @@ export function registerSessionRoutes( if (!existsSync(join(resolvedCasePath, '.claude', 'settings.local.json'))) { await writeHooksConfig(resolvedCasePath); } else { - await refreshStaleHookSecret(resolvedCasePath).catch(() => {}); + await refreshStaleCodemanHooks(resolvedCasePath).catch(() => {}); } } catch { /* non-fatal — the session still runs, hooks may be degraded */ diff --git a/test/hook-secret-selfheal.test.ts b/test/hook-secret-selfheal.test.ts index f3c54b48..892bf32e 100644 --- a/test/hook-secret-selfheal.test.ts +++ b/test/hook-secret-selfheal.test.ts @@ -1,10 +1,10 @@ /** - * COD-91 — `refreshStaleHookSecret` self-heal. + * COD-91 — `refreshStaleCodemanHooks` self-heal. * * Making the hook-event secret unconditionally required (PR #127) would silently 401 the * hook curls baked into cases created before the secret header existed (COD-54). Those * curls live in `.claude/settings.local.json` and `writeHooksConfig` only runs at case - * CREATION, so existing cases never refresh. `refreshStaleHookSecret` regenerates the + * CREATION, so existing cases never refresh. `refreshStaleCodemanHooks` regenerates the * hooks block on session spawn — but ONLY when the case already holds Codeman's own * pre-secret hook curls, never clobbering a user's customizations. * @@ -15,7 +15,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { refreshStaleHookSecret } from '../src/hooks-config.js'; +import { refreshStaleCodemanHooks } from '../src/hooks-config.js'; const SECRET_HEADER = 'X-Codeman-Hook-Secret'; @@ -41,7 +41,7 @@ function staleCodemanHooks() { }; } -describe('refreshStaleHookSecret', () => { +describe('refreshStaleCodemanHooks', () => { let dir: string; let settingsPath: string; @@ -60,7 +60,7 @@ describe('refreshStaleHookSecret', () => { settingsPath, JSON.stringify({ env: { CLAUDE_CODE_FOO: '1' }, model: 'opus', hooks: staleCodemanHooks() }, null, 2) ); - await refreshStaleHookSecret(dir); + await refreshStaleCodemanHooks(dir); const after = JSON.parse(readFileSync(settingsPath, 'utf-8')); expect(JSON.stringify(after.hooks)).toContain(SECRET_HEADER); @@ -73,11 +73,11 @@ describe('refreshStaleHookSecret', () => { it('leaves a hooks block that already carries the secret unchanged', async () => { // Seed with a current block by healing a stale one first, then re-heal: second pass must no-op. writeFileSync(settingsPath, JSON.stringify({ hooks: staleCodemanHooks() }, null, 2)); - await refreshStaleHookSecret(dir); + await refreshStaleCodemanHooks(dir); const healed = readFileSync(settingsPath, 'utf-8'); expect(healed).toContain(SECRET_HEADER); - await refreshStaleHookSecret(dir); + await refreshStaleCodemanHooks(dir); expect(readFileSync(settingsPath, 'utf-8')).toBe(healed); // byte-identical: no rewrite }); @@ -88,19 +88,60 @@ describe('refreshStaleHookSecret', () => { 2 ); writeFileSync(settingsPath, foreign); - await refreshStaleHookSecret(dir); + await refreshStaleCodemanHooks(dir); expect(readFileSync(settingsPath, 'utf-8')).toBe(foreign); }); + it('preserves user handlers and events in a mixed stale configuration', async () => { + const hooks = staleCodemanHooks(); + hooks.Stop[0].hooks.push({ + type: 'command', + command: './notify-user.sh', + timeout: 10, + }); + const customPostToolUse = { + matcher: 'Write', + hooks: [{ type: 'command', command: './format.sh' }], + }; + const customEvent = [ + { + hooks: [{ type: 'command', command: './audit.sh' }], + }, + ]; + writeFileSync( + settingsPath, + JSON.stringify( + { + hooks: { + ...hooks, + PostToolUse: [customPostToolUse], + CustomEvent: customEvent, + }, + }, + null, + 2 + ) + ); + + await refreshStaleCodemanHooks(dir); + + const after = JSON.parse(readFileSync(settingsPath, 'utf-8')); + expect(JSON.stringify(after.hooks)).toContain(SECRET_HEADER); + expect(JSON.stringify(after.hooks)).toContain('CODEMAN_BACKGROUND_REWAKE_V1'); + expect(JSON.stringify(after.hooks.Stop)).toContain('./notify-user.sh'); + expect(after.hooks.PostToolUse).toEqual(expect.arrayContaining([customPostToolUse])); + expect(after.hooks.CustomEvent).toEqual(customEvent); + }); + it('is a no-op when settings.local.json is absent (does not create one)', async () => { - await refreshStaleHookSecret(dir); + await refreshStaleCodemanHooks(dir); expect(existsSync(settingsPath)).toBe(false); }); it('leaves a malformed settings file untouched', async () => { const garbage = '{ not valid json'; writeFileSync(settingsPath, garbage); - await refreshStaleHookSecret(dir); + await refreshStaleCodemanHooks(dir); expect(readFileSync(settingsPath, 'utf-8')).toBe(garbage); }); }); diff --git a/test/hooks-config.test.ts b/test/hooks-config.test.ts index 513d14ad..ebb0b63d 100644 --- a/test/hooks-config.test.ts +++ b/test/hooks-config.test.ts @@ -9,7 +9,13 @@ import { describe, it, expect, beforeAll, beforeEach, afterAll, afterEach } from import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { generateHooksConfig, writeHooksConfig } from '../src/hooks-config.js'; +import { spawn } from 'node:child_process'; +import { + generateBackgroundWakeScript, + generateHooksConfig, + refreshStaleCodemanHooks, + writeHooksConfig, +} from '../src/hooks-config.js'; describe('generateHooksConfig', () => { it('should return an object with hooks key', () => { @@ -29,6 +35,30 @@ describe('generateHooksConfig', () => { expect(config.hooks.Stop).toHaveLength(1); }); + it('should configure a self-contained Bash background-task rewake hook', () => { + const config = generateHooksConfig(); + const postToolHooks = config.hooks.PostToolUse as Array<{ + matcher: string; + hooks: Array<{ + type: string; + command: string; + args: string[]; + asyncRewake: boolean; + timeout: number; + }>; + }>; + + expect(postToolHooks).toHaveLength(1); + expect(postToolHooks[0].matcher).toBe('Bash'); + expect(postToolHooks[0].hooks[0]).toMatchObject({ + type: 'command', + command: 'node', + asyncRewake: true, + }); + expect(postToolHooks[0].hooks[0].args).toEqual(['-e', generateBackgroundWakeScript()]); + expect(postToolHooks[0].hooks[0].timeout).toBeGreaterThanOrEqual(3600); + }); + it('should configure idle_prompt matcher', () => { const config = generateHooksConfig(); const notifHooks = config.hooks.Notification as Array<{ matcher?: string }>; @@ -157,7 +187,42 @@ describe('writeHooksConfig', () => { expect(parsed.hooks).toBeDefined(); }); - it('should overwrite existing hooks key', async () => { + it('should upgrade Codeman-owned hooks that predate background rewake', async () => { + const claudeDir = join(testDir, '.claude'); + const settingsPath = join(claudeDir, 'settings.local.json'); + mkdirSync(claudeDir, { recursive: true }); + const oldHooks = generateHooksConfig().hooks; + delete oldHooks.PostToolUse; + writeFileSync(settingsPath, JSON.stringify({ hooks: oldHooks }, null, 2)); + + await refreshStaleCodemanHooks(testDir); + + const parsed = JSON.parse(readFileSync(settingsPath, 'utf-8')); + expect(parsed.hooks.PostToolUse).toHaveLength(1); + expect(JSON.stringify(parsed.hooks.PostToolUse)).toContain('CODEMAN_BACKGROUND_REWAKE_V1'); + }); + + it('should not add rewake hooks to a user-owned hook configuration', async () => { + const claudeDir = join(testDir, '.claude'); + const settingsPath = join(claudeDir, 'settings.local.json'); + mkdirSync(claudeDir, { recursive: true }); + const userHooks = { + PostToolUse: [ + { + matcher: 'Write', + hooks: [{ type: 'command', command: './format.sh' }], + }, + ], + }; + writeFileSync(settingsPath, JSON.stringify({ hooks: userHooks }, null, 2)); + + await refreshStaleCodemanHooks(testDir); + + const parsed = JSON.parse(readFileSync(settingsPath, 'utf-8')); + expect(parsed.hooks).toEqual(userHooks); + }); + + it('should preserve user hook events while installing Codeman hooks', async () => { const claudeDir = join(testDir, '.claude'); mkdirSync(claudeDir, { recursive: true }); writeFileSync(join(claudeDir, 'settings.local.json'), JSON.stringify({ hooks: { oldHook: [] } }, null, 2)); @@ -165,7 +230,7 @@ describe('writeHooksConfig', () => { await writeHooksConfig(testDir); const parsed = JSON.parse(readFileSync(join(claudeDir, 'settings.local.json'), 'utf-8')); - expect(parsed.hooks.oldHook).toBeUndefined(); + expect(parsed.hooks.oldHook).toEqual([]); expect(parsed.hooks.Notification).toBeDefined(); }); @@ -187,6 +252,82 @@ describe('writeHooksConfig', () => { }); }); +describe('background task rewake helper', () => { + const testDir = join(tmpdir(), 'codeman-background-rewake-test-' + Date.now()); + + beforeEach(() => { + mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + }); + + function runHelper(input: Record): Promise<{ code: number | null; stderr: string }> { + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, ['-e', generateBackgroundWakeScript()], { + stdio: ['pipe', 'ignore', 'pipe'], + }); + let stderr = ''; + const timeout = setTimeout(() => { + child.kill(); + reject(new Error('background rewake helper timed out')); + }, 5000); + + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (chunk) => { + stderr += chunk; + }); + child.on('error', reject); + child.on('close', (code) => { + clearTimeout(timeout); + resolve({ code, stderr }); + }); + child.stdin.end(JSON.stringify(input)); + }); + } + + it('exits without waiting for an ordinary Bash result', async () => { + const result = await runHelper({ + transcript_path: join(testDir, 'transcript.jsonl'), + tool_response: { stdout: 'ordinary command completed' }, + }); + + expect(result.code).toBe(0); + expect(result.stderr).toBe(''); + }); + + it('exits 2 when the matching background command completes', async () => { + const transcriptPath = join(testDir, 'transcript.jsonl'); + writeFileSync(transcriptPath, ''); + + const resultPromise = runHelper({ + transcript_path: transcriptPath, + tool_response: { + stdout: 'Command running in background with ID: bg-test-1. Output is being written to: /tmp/bg-test-1.output.', + }, + }); + + await new Promise((resolve) => setTimeout(resolve, 100)); + writeFileSync( + transcriptPath, + JSON.stringify({ + type: 'queue-operation', + operation: 'enqueue', + content: + '\nbg-test-1\ncompleted\n' + + '/tmp/bg-test-1.output\n', + }) + '\n' + ); + + const result = await resultPromise; + expect(result.code).toBe(2); + expect(result.stderr).toContain('bg-test-1'); + expect(result.stderr).toContain('completed'); + expect(result.stderr).toContain('/tmp/bg-test-1.output'); + }); +}); + // ========== Hook Event API Integration Tests ========== // Port 3130 reserved for hooks integration tests From 4a4720cb6230a428c8417a5243ed2b5909655c68 Mon Sep 17 00:00:00 2001 From: lior Date: Tue, 28 Jul 2026 23:18:10 +0300 Subject: [PATCH 03/24] fix(transcripts): complete tools from user results --- src/transcript-watcher.ts | 49 ++++++++++------ test/transcript-watcher.test.ts | 99 ++++++++++++++++++++++----------- 2 files changed, 101 insertions(+), 47 deletions(-) diff --git a/src/transcript-watcher.ts b/src/transcript-watcher.ts index 8eb01644..8d44865c 100644 --- a/src/transcript-watcher.ts +++ b/src/transcript-watcher.ts @@ -40,6 +40,7 @@ interface TranscriptContentBlock { text?: string; name?: string; input?: Record; + tool_use_id?: string; content?: string; is_error?: boolean; } @@ -328,10 +329,7 @@ export class TranscriptWatcher extends EventEmitter { this.handleResultEntry(entry); break; case 'user': - // User message means new turn, reset some state - this.state.isComplete = false; - this.state.hasError = false; - this.state.errorMessage = null; + this.handleUserEntry(entry); break; case 'system': // System messages are informational @@ -360,23 +358,42 @@ export class TranscriptWatcher extends EventEmitter { this.state.currentTool = block.name; this.emit('transcript:tool_start', block.name); } else if (block.type === 'tool_result') { - // Tool completed - const wasError = block.is_error === true; - const toolName = this.state.currentTool; - this.state.toolExecuting = false; - this.state.currentTool = null; - if (toolName) { - this.emit('transcript:tool_end', toolName, wasError); - } - if (wasError && block.content) { - this.state.hasError = true; - this.state.errorMessage = String(block.content).slice(0, 200); - } + this.handleToolResult(block); } } } } + private handleUserEntry(entry: TranscriptEntry): void { + // A user-authored prompt starts a turn, while Claude tool results also use + // user entries. Reset turn state first, then close any completed tool. + this.state.isComplete = false; + this.state.hasError = false; + this.state.errorMessage = null; + + const content = entry.message?.content; + if (!Array.isArray(content)) return; + for (const block of content) { + if (block.type === 'tool_result') { + this.handleToolResult(block); + } + } + } + + private handleToolResult(block: TranscriptContentBlock): void { + const wasError = block.is_error === true; + const toolName = this.state.currentTool; + this.state.toolExecuting = false; + this.state.currentTool = null; + if (toolName) { + this.emit('transcript:tool_end', toolName, wasError); + } + if (wasError && block.content) { + this.state.hasError = true; + this.state.errorMessage = String(block.content).slice(0, 200); + } + } + private handleResultEntry(entry: TranscriptEntry): void { // Result entry indicates completion this.state.isComplete = true; diff --git a/test/transcript-watcher.test.ts b/test/transcript-watcher.test.ts index c286dd0f..badba35e 100644 --- a/test/transcript-watcher.test.ts +++ b/test/transcript-watcher.test.ts @@ -88,14 +88,16 @@ describe('TranscriptWatcher', () => { watcher.start(testFile); // Add user entry - const userEntry = { type: 'user', timestamp: new Date().toISOString(), message: { role: 'user', content: 'test' } }; + const userEntry = { + type: 'user', + timestamp: new Date().toISOString(), + message: { role: 'user', content: 'test' }, + }; appendFileSync(testFile, JSON.stringify(userEntry) + '\n'); - // Wait for processing - await new Promise(resolve => setTimeout(resolve, 100)); - - const state = watcher.getState(); - expect(state.entryCount).toBeGreaterThanOrEqual(1); + await vi.waitFor(() => { + expect(watcher.getState().entryCount).toBeGreaterThanOrEqual(1); + }); }); it('should emit transcript:complete on result entry', async () => { @@ -109,10 +111,9 @@ describe('TranscriptWatcher', () => { const resultEntry = { type: 'result', timestamp: new Date().toISOString() }; appendFileSync(testFile, JSON.stringify(resultEntry) + '\n'); - // Wait for processing - await new Promise(resolve => setTimeout(resolve, 200)); - - expect(completeHandler).toHaveBeenCalled(); + await vi.waitFor(() => { + expect(completeHandler).toHaveBeenCalled(); + }); const state = watcher.getState(); expect(state.isComplete).toBe(true); }); @@ -130,22 +131,62 @@ describe('TranscriptWatcher', () => { timestamp: new Date().toISOString(), message: { role: 'assistant', - content: [ - { type: 'tool_use', name: 'Read', input: { file_path: '/test.txt' } } - ] - } + content: [{ type: 'tool_use', name: 'Read', input: { file_path: '/test.txt' } }], + }, }; appendFileSync(testFile, JSON.stringify(assistantEntry) + '\n'); - // Wait for processing - await new Promise(resolve => setTimeout(resolve, 200)); - - expect(toolStartHandler).toHaveBeenCalledWith('Read'); + await vi.waitFor(() => { + expect(toolStartHandler).toHaveBeenCalledWith('Read'); + }); const state = watcher.getState(); expect(state.toolExecuting).toBe(true); expect(state.currentTool).toBe('Read'); }); + it('should complete a tool when Claude writes tool_result in a user entry', async () => { + writeFileSync(testFile, ''); + watcher.start(testFile); + + const toolEndHandler = vi.fn(); + watcher.on('transcript:tool_end', toolEndHandler); + + appendFileSync( + testFile, + JSON.stringify({ + type: 'assistant', + timestamp: new Date().toISOString(), + message: { + role: 'assistant', + content: [{ type: 'tool_use', name: 'Bash', input: { command: 'printf done' } }], + }, + }) + '\n' + ); + await vi.waitFor(() => { + expect(watcher.getState().toolExecuting).toBe(true); + }); + + appendFileSync( + testFile, + JSON.stringify({ + type: 'user', + timestamp: new Date().toISOString(), + message: { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 'toolu_1', content: 'done', is_error: false }], + }, + }) + '\n' + ); + + await vi.waitFor(() => { + expect(toolEndHandler).toHaveBeenCalledWith('Bash', false); + }); + expect(watcher.getState()).toMatchObject({ + toolExecuting: false, + currentTool: null, + }); + }); + it('should detect plan mode from AskUserQuestion tool', async () => { writeFileSync(testFile, ''); watcher.start(testFile); @@ -159,17 +200,14 @@ describe('TranscriptWatcher', () => { timestamp: new Date().toISOString(), message: { role: 'assistant', - content: [ - { type: 'tool_use', name: 'AskUserQuestion', input: { question: 'test?' } } - ] - } + content: [{ type: 'tool_use', name: 'AskUserQuestion', input: { question: 'test?' } }], + }, }; appendFileSync(testFile, JSON.stringify(assistantEntry) + '\n'); - // Wait for processing - await new Promise(resolve => setTimeout(resolve, 200)); - - expect(planModeHandler).toHaveBeenCalled(); + await vi.waitFor(() => { + expect(planModeHandler).toHaveBeenCalled(); + }); const state = watcher.getState(); expect(state.planModeDetected).toBe(true); }); @@ -182,15 +220,14 @@ describe('TranscriptWatcher', () => { const resultEntry = { type: 'result', timestamp: new Date().toISOString(), - error: { type: 'api_error', message: 'Rate limited' } + error: { type: 'api_error', message: 'Rate limited' }, }; appendFileSync(testFile, JSON.stringify(resultEntry) + '\n'); - // Wait for processing - await new Promise(resolve => setTimeout(resolve, 200)); - + await vi.waitFor(() => { + expect(watcher.getState().hasError).toBe(true); + }); const state = watcher.getState(); - expect(state.hasError).toBe(true); expect(state.errorMessage).toContain('Rate limited'); }); }); From 67eb5b43eb80440763ab35b04005c10c7180a2dd Mon Sep 17 00:00:00 2001 From: lior Date: Tue, 28 Jul 2026 23:20:01 +0300 Subject: [PATCH 04/24] fix(notifications): quiet lifecycle hook noise --- src/web/public/index.html | 2 +- src/web/public/notification-manager.js | 139 +++++++++++++++------- src/web/public/settings-ui.js | 7 +- test/notification-manager-noise.test.ts | 150 ++++++++++++++++++++++++ 4 files changed, 252 insertions(+), 46 deletions(-) create mode 100644 test/notification-manager-noise.test.ts diff --git a/src/web/public/index.html b/src/web/public/index.html index 224280db..3fa2501c 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1848,7 +1848,7 @@
Response complete
- + diff --git a/src/web/public/notification-manager.js b/src/web/public/notification-manager.js index 8c28e915..2d7f4d9f 100644 --- a/src/web/public/notification-manager.js +++ b/src/web/public/notification-manager.js @@ -9,7 +9,7 @@ * 5. Audio alerts (Web Audio API beep, user-opt-in) * * Features: - * - Per-event-type preferences (enabled, browser, audio, push) with v1→v4 migration + * - Per-event-type preferences (enabled, browser, audio, push) with v1→v5 migration * - Device-specific defaults (notifications disabled on mobile by default) * - 5s notification grouping window to batch rapid-fire events * - 100-notification cap with oldest eviction @@ -21,7 +21,7 @@ * @param {CodemanApp} app - Reference to the main app instance * * @dependency constants.js (STUCK_THRESHOLD_DEFAULT_MS, timing constants) - * @dependency mobile-handlers.js (MobileDetection.getDeviceType for device-specific defaults) + * @dependency mobile-handlers.js (MobileDetection stable handheld identity/device type) * @loadorder 4 of 15 — loaded after voice-input.js, before keyboard-accessory.js */ @@ -65,12 +65,19 @@ class NotificationManager { }); } - loadPreferences() { + _usesMobilePreferences() { + return ( + MobileDetection.isHandheldDevice?.() ?? + MobileDetection.getDeviceType() === 'mobile' + ); + } + + getDefaultPreferences() { const defaultEventTypes = { permission_prompt: { enabled: true, browser: true, audio: true, push: false }, elicitation_dialog: { enabled: true, browser: true, audio: true, push: false }, idle_prompt: { enabled: true, browser: true, audio: false, push: false }, - stop: { enabled: true, browser: false, audio: false, push: false }, + stop: { enabled: false, browser: false, audio: false, push: false }, session_error: { enabled: true, browser: true, audio: false, push: false }, respawn_cycle: { enabled: true, browser: false, audio: false, push: false }, token_milestone: { enabled: true, browser: false, audio: false, push: false }, @@ -80,8 +87,8 @@ class NotificationManager { }; // Device-specific defaults: mobile has notifications disabled by default - const isMobile = MobileDetection.getDeviceType() === 'mobile'; - const defaults = { + const isMobile = this._usesMobilePreferences(); + return { enabled: !isMobile, // Disabled on mobile by default browserNotifications: !isMobile, audioAlerts: false, @@ -92,51 +99,97 @@ class NotificationManager { muteInfo: false, // Per-event-type preferences eventTypes: defaultEventTypes, - _version: 4, + _version: 5, }; + } + + /** + * Apply the complete v1→v5 migration to either local or server-hydrated + * preferences. Keeping one normalization path prevents fresh browsers from + * reviving retired drawer-only hook defaults. + */ + normalizePreferences(rawPreferences) { + const defaults = this.getDefaultPreferences(); + if ( + !rawPreferences || + typeof rawPreferences !== 'object' || + Array.isArray(rawPreferences) + ) { + return defaults; + } + + const prefs = { + ...rawPreferences, + eventTypes: + rawPreferences.eventTypes && + typeof rawPreferences.eventTypes === 'object' && + !Array.isArray(rawPreferences.eventTypes) + ? Object.fromEntries( + Object.entries(rawPreferences.eventTypes).map(([key, value]) => [ + key, + value && typeof value === 'object' ? { ...value } : value, + ]) + ) + : undefined, + }; + const version = Number.isInteger(prefs._version) ? prefs._version : 0; + + // Migrate: v1 had browserNotifications defaulting to false + if (version < 2) { + prefs.browserNotifications = true; + } + // Migrate: v2 -> v3 adds eventTypes + if (version < 3) { + prefs.eventTypes = { ...defaults.eventTypes }; + } + // Migrate: v3 -> v4 adds push field to all eventTypes + if (version < 4 && prefs.eventTypes) { + for (const key of Object.keys(prefs.eventTypes)) { + if (prefs.eventTypes[key] && prefs.eventTypes[key].push === undefined) { + prefs.eventTypes[key].push = false; + } + } + } + // Migrate: v4 -> v5 removes the drawer-only Response Complete default. + // Preserve users who opted into any external delivery channel. + if (version < 5) { + const stopPref = prefs.eventTypes?.stop; + if ( + stopPref?.enabled === true && + !stopPref.browser && + !stopPref.audio && + !stopPref.push + ) { + stopPref.enabled = false; + } + } + + return { + ...defaults, + ...prefs, + eventTypes: { ...defaults.eventTypes, ...prefs.eventTypes }, + _version: 5, + }; + } + + loadPreferences() { try { const storageKey = this.getStorageKey(); const saved = localStorage.getItem(storageKey); if (saved) { - const prefs = JSON.parse(saved); - // Migrate: v1 had browserNotifications defaulting to false - if (!prefs._version || prefs._version < 2) { - prefs.browserNotifications = true; - prefs._version = 2; - } - // Migrate: v2 -> v3 adds eventTypes - if (prefs._version < 3) { - prefs.eventTypes = defaultEventTypes; - prefs._version = 3; - localStorage.setItem(storageKey, JSON.stringify(prefs)); - } - // Migrate: v3 -> v4 adds push field to all eventTypes - if (prefs._version < 4) { - if (prefs.eventTypes) { - for (const key of Object.keys(prefs.eventTypes)) { - if (prefs.eventTypes[key] && prefs.eventTypes[key].push === undefined) { - prefs.eventTypes[key].push = false; - } - } - } - prefs._version = 4; - localStorage.setItem(storageKey, JSON.stringify(prefs)); - } - // Merge with defaults to ensure all eventTypes exist - return { - ...defaults, - ...prefs, - eventTypes: { ...defaultEventTypes, ...prefs.eventTypes }, - }; + const normalized = this.normalizePreferences(JSON.parse(saved)); + localStorage.setItem(storageKey, JSON.stringify(normalized)); + return normalized; } } catch (_e) { /* ignore */ } - return defaults; + return this.getDefaultPreferences(); } // Get storage key for notification prefs (device-specific) getStorageKey() { - const isMobile = MobileDetection.getDeviceType() === 'mobile'; - return isMobile ? 'codeman-notification-prefs-mobile' : 'codeman-notification-prefs'; + return this._usesMobilePreferences() + ? 'codeman-notification-prefs-mobile' + : 'codeman-notification-prefs'; } savePreferences() { @@ -163,8 +216,10 @@ class NotificationManager { 'exit-gate': 'ralph_complete', 'subagent-spawn': 'subagent_spawn', 'subagent-complete': 'subagent_complete', - 'hook-teammate-idle': 'idle_prompt', - 'hook-task-completed': 'stop', + // Team lifecycle hooks are agent activity, not session-idle/stop alerts. + // Reuse the existing opt-in agent categories instead of making them noisy. + 'hook-teammate-idle': 'subagent_spawn', + 'hook-task-completed': 'subagent_complete', }; const eventTypeKey = categoryToEventType[category] || category; diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 9e0c0d0c..5470dda7 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -408,7 +408,7 @@ Object.assign(CodemanApp.prototype, { document.getElementById('eventIdleAudio').checked = idlePref.audio ?? false; // Response complete (stop) const stopPref = eventTypes.stop || {}; - document.getElementById('eventStopEnabled').checked = stopPref.enabled ?? true; + document.getElementById('eventStopEnabled').checked = stopPref.enabled ?? false; document.getElementById('eventStopBrowser').checked = stopPref.browser ?? false; document.getElementById('eventStopPush').checked = stopPref.push ?? false; document.getElementById('eventStopAudio').checked = stopPref.audio ?? false; @@ -1587,7 +1587,7 @@ Object.assign(CodemanApp.prototype, { audio: document.getElementById('eventSubagentAudio').checked, }, }, - _version: 4, + _version: 5, }; if (this.notificationManager) { this.notificationManager.preferences = notifPrefsToSave; @@ -2275,7 +2275,8 @@ Object.assign(CodemanApp.prototype, { if (notificationPreferences && this.notificationManager) { const localNotifPrefs = localStorage.getItem(this.notificationManager.getStorageKey()); if (!localNotifPrefs) { - this.notificationManager.preferences = notificationPreferences; + this.notificationManager.preferences = + this.notificationManager.normalizePreferences(notificationPreferences); this.notificationManager.savePreferences(); } } diff --git a/test/notification-manager-noise.test.ts b/test/notification-manager-noise.test.ts new file mode 100644 index 00000000..d5fbf896 --- /dev/null +++ b/test/notification-manager-noise.test.ts @@ -0,0 +1,150 @@ +import { readFileSync } from 'node:fs'; +import { JSDOM } from 'jsdom'; +import { afterEach, describe, expect, it } from 'vitest'; + +const SOURCE = readFileSync(new URL('../src/web/public/notification-manager.js', import.meta.url), 'utf8'); + +type EventPreference = { + enabled: boolean; + browser: boolean; + audio: boolean; + push: boolean; +}; + +type NotificationPreferences = { + enabled: boolean; + eventTypes: Record; + _version: number; +}; + +type Manager = { + preferences: NotificationPreferences; + notifications: unknown[]; + getStorageKey: () => string; + normalizePreferences: (preferences: Record) => NotificationPreferences; + notify: (notification: Record) => void; +}; + +const openWindows: JSDOM[] = []; + +function loadManager( + saved?: Record, + device: { deviceType?: string; handheld?: boolean } = {} +): { dom: JSDOM; manager: Manager } { + const dom = new JSDOM( + '
', + { + url: 'http://localhost/', + runScripts: 'outside-only', + } + ); + openWindows.push(dom); + const win = dom.window as unknown as Window & + typeof globalThis & { + MobileDetection: { + getDeviceType: () => string; + isHandheldDevice?: () => boolean; + }; + STUCK_THRESHOLD_DEFAULT_MS: number; + GROUPING_TIMEOUT_MS: number; + NOTIFICATION_LIST_CAP: number; + }; + win.MobileDetection = { + getDeviceType: () => device.deviceType ?? 'desktop', + ...(typeof device.handheld === 'boolean' ? { isHandheldDevice: () => device.handheld === true } : {}), + }; + win.STUCK_THRESHOLD_DEFAULT_MS = 600_000; + win.GROUPING_TIMEOUT_MS = 5_000; + win.NOTIFICATION_LIST_CAP = 100; + win.requestAnimationFrame = ((callback: FrameRequestCallback) => { + callback(0); + return 1; + }) as typeof requestAnimationFrame; + + if (saved) { + win.localStorage.setItem('codeman-notification-prefs', JSON.stringify(saved)); + } + + win.eval(` + ${SOURCE} + window.__testNotificationManager = NotificationManager; + `); + const NotificationManager = ( + win as unknown as { + __testNotificationManager: new (app: { sessions: Map }) => Manager; + } + ).__testNotificationManager; + const manager = new NotificationManager({ sessions: new Map() }) as Manager; + return { dom, manager }; +} + +afterEach(() => { + for (const dom of openWindows.splice(0)) dom.window.close(); +}); + +describe('notification noise defaults', () => { + it('keeps response-complete and team lifecycle drawer entries opt-in', () => { + const { manager } = loadManager(); + expect(manager.preferences.eventTypes.stop.enabled).toBe(false); + + for (const category of ['hook-stop', 'hook-teammate-idle', 'hook-task-completed']) { + manager.notify({ + urgency: 'info', + category, + sessionId: 'session-1', + sessionName: 'session', + title: category, + message: category, + }); + } + + expect(manager.notifications).toHaveLength(0); + }); + + it('migrates the old drawer-only Stop default but preserves explicit delivery', () => { + const quietV4 = { + enabled: true, + eventTypes: { + stop: { enabled: true, browser: false, audio: false, push: false }, + }, + _version: 4, + }; + const { manager: quietManager } = loadManager(quietV4); + expect(quietManager.preferences.eventTypes.stop.enabled).toBe(false); + expect(quietManager.preferences._version).toBe(5); + + const browserV4 = { + enabled: true, + eventTypes: { + stop: { enabled: true, browser: true, audio: false, push: false }, + }, + _version: 4, + }; + const { manager: browserManager } = loadManager(browserV4); + expect(browserManager.preferences.eventTypes.stop.enabled).toBe(true); + }); + + it('normalizes server-hydrated v4 preferences through the same quiet migration', () => { + const { manager } = loadManager(); + manager.preferences = manager.normalizePreferences({ + enabled: true, + eventTypes: { + stop: { enabled: true, browser: false, audio: false, push: false }, + }, + _version: 4, + }); + + expect(manager.preferences.eventTypes.stop.enabled).toBe(false); + expect(manager.preferences._version).toBe(5); + }); + + it('keeps mobile notification defaults and storage on an unfolded handheld', () => { + const { manager } = loadManager(undefined, { + deviceType: 'desktop', + handheld: true, + }); + + expect(manager.preferences.enabled).toBe(false); + expect(manager.getStorageKey()).toBe('codeman-notification-prefs-mobile'); + }); +}); From 0a039239e4052a63b5b5cdbfd338420aae9d86b6 Mon Sep 17 00:00:00 2001 From: lior Date: Wed, 29 Jul 2026 03:30:38 +0300 Subject: [PATCH 05/24] fix(sessions): preserve active terminal during launches --- src/web/public/session-ui.js | 91 ++++++++++++++++++++++++------------ test/run-mode-ui.test.ts | 30 ++++++++++++ 2 files changed, 91 insertions(+), 30 deletions(-) diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index d3cbec61..95fdd862 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -576,13 +576,43 @@ Object.assign(CodemanApp.prototype, { return startNumber; }, + /** + * Launch progress may use the terminal only on the session-less home screen. + * When another session is active, mutating the shared xterm would serialize + * launch chrome into that session's snapshot during the subsequent switch. + */ + _beginSessionLaunchStatus(message, ansiColor = '1;32') { + const ownsTerminal = !this.activeSessionId; + if (ownsTerminal) { + this.terminal.clear(); + this.terminal.writeln(`\x1b[${ansiColor}m ${message}\x1b[0m`); + this.terminal.writeln(''); + } else { + this.showToast?.(message, 'info'); + } + return ownsTerminal; + }, + + _appendSessionLaunchStatus(ownsTerminal, message, ansiColor = '90') { + if (!ownsTerminal || this.activeSessionId) return; + this.terminal.writeln(`\x1b[${ansiColor}m ${message}\x1b[0m`); + }, + + _reportSessionLaunchError(ownsTerminal, message) { + if (ownsTerminal && !this.activeSessionId) { + this.terminal.writeln(`\x1b[1;31m Error: ${message}\x1b[0m`); + } else { + this.showToast?.(message, 'error'); + } + }, + async runClaude() { const caseName = document.getElementById('quickStartCase').value || 'testcase'; const tabCount = Math.min(20, Math.max(1, parseInt(document.getElementById('tabCount').value) || 1)); - this.terminal.clear(); - this.terminal.writeln(`\x1b[1;32m Starting ${tabCount} Claude session(s) in ${caseName}...\x1b[0m`); - this.terminal.writeln(''); + const ownsLaunchTerminal = this._beginSessionLaunchStatus( + `Starting ${tabCount} Claude session(s) in ${caseName}...` + ); // Focus terminal NOW, in the synchronous user-gesture context (button click). // iOS Safari ignores programmatic focus() after any await, so this must happen // before the first async call. The keyboard opens here and stays open through @@ -665,7 +695,7 @@ Object.assign(CodemanApp.prototype, { await this._ensureCreatedSessionVisible(data.data.sessionId, data.data.session); remoteIds.push(data.data.sessionId); } - this.terminal.writeln(`\x1b[90m All ${tabCount} remote session(s) ready\x1b[0m`); + this._appendSessionLaunchStatus(ownsLaunchTerminal, `All ${tabCount} remote session(s) ready`); if (remoteIds[0]) { await this.selectSession(remoteIds[0]); this.loadQuickStartCases(); @@ -700,7 +730,7 @@ Object.assign(CodemanApp.prototype, { const modelOverride = globalSettings.claudeModel || (useOpus1m ? 'opus[1m]' : ''); // Step 1: Create all sessions in parallel - this.terminal.writeln(`\x1b[90m Creating ${tabCount} session(s)...\x1b[0m`); + this._appendSessionLaunchStatus(ownsLaunchTerminal, `Creating ${tabCount} session(s)...`); const createPromises = sessionNames.map(name => fetch('/api/sessions', { method: 'POST', @@ -741,12 +771,12 @@ Object.assign(CodemanApp.prototype, { )); // Step 3: Start all sessions in parallel (biggest speedup) - this.terminal.writeln(`\x1b[90m Starting ${tabCount} session(s) in parallel...\x1b[0m`); + this._appendSessionLaunchStatus(ownsLaunchTerminal, `Starting ${tabCount} session(s) in parallel...`); await Promise.all(sessionIds.map(id => fetch(`/api/sessions/${id}/interactive`, { method: 'POST' }) )); - this.terminal.writeln(`\x1b[90m All ${tabCount} sessions ready\x1b[0m`); + this._appendSessionLaunchStatus(ownsLaunchTerminal, `All ${tabCount} sessions ready`); // Auto-switch to the new session using selectSession (does proper refresh) if (firstSessionId) { @@ -756,7 +786,7 @@ Object.assign(CodemanApp.prototype, { this.terminal.focus(); } catch (err) { - this.terminal.writeln(`\x1b[1;31m Error: ${err.message}\x1b[0m`); + this._reportSessionLaunchError(ownsLaunchTerminal, err.message); } }, @@ -799,9 +829,10 @@ Object.assign(CodemanApp.prototype, { const caseName = document.getElementById('quickStartCase').value || 'testcase'; const shellCount = Math.min(20, Math.max(1, parseInt(document.getElementById('shellCount').value) || 1)); - this.terminal.clear(); - this.terminal.writeln(`\x1b[1;33m Starting ${shellCount} Shell session(s) in ${caseName}...\x1b[0m`); - this.terminal.writeln(''); + const ownsLaunchTerminal = this._beginSessionLaunchStatus( + `Starting ${shellCount} Shell session(s) in ${caseName}...`, + '1;33' + ); try { // Get the case path @@ -907,7 +938,7 @@ Object.assign(CodemanApp.prototype, { this.terminal.focus(); } catch (err) { - this.terminal.writeln(`\x1b[1;31m Error: ${err.message}\x1b[0m`); + this._reportSessionLaunchError(ownsLaunchTerminal, err.message); } }, @@ -918,9 +949,7 @@ Object.assign(CodemanApp.prototype, { const _runLoc = (this.cases || []).find(c => c.name === caseName)?.location; const isRemote = _runLoc === 'remote' || _runLoc === 'docker'; - this.terminal.clear(); - this.terminal.writeln(`\x1b[1;32m Starting OpenCode session in ${caseName}...\x1b[0m`); - this.terminal.writeln(''); + const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting OpenCode session in ${caseName}...`); // Focus in sync gesture context (see runClaude comment) this.terminal.focus(); @@ -930,8 +959,10 @@ Object.assign(CodemanApp.prototype, { const statusRes = await fetch('/api/opencode/status'); const status = (await statusRes.json()).data; if (!status.available) { - this.terminal.writeln('\x1b[1;31m OpenCode CLI not found.\x1b[0m'); - this.terminal.writeln('\x1b[90m Install with: curl -fsSL https://opencode.ai/install | bash\x1b[0m'); + this._reportSessionLaunchError( + ownsLaunchTerminal, + 'OpenCode CLI not found. Install with: curl -fsSL https://opencode.ai/install | bash' + ); return; } } @@ -964,7 +995,7 @@ Object.assign(CodemanApp.prototype, { this.terminal.focus(); } catch (err) { - this.terminal.writeln(`\x1b[1;31m Error: ${err.message}\x1b[0m`); + this._reportSessionLaunchError(ownsLaunchTerminal, err.message); } }, @@ -975,9 +1006,7 @@ Object.assign(CodemanApp.prototype, { const _runLoc = (this.cases || []).find(c => c.name === caseName)?.location; const isRemote = _runLoc === 'remote' || _runLoc === 'docker'; - this.terminal.clear(); - this.terminal.writeln(`\x1b[1;32m Starting Codex session in ${caseName}...\x1b[0m`); - this.terminal.writeln(''); + const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting Codex session in ${caseName}...`); this.terminal.focus(); try { @@ -985,8 +1014,10 @@ Object.assign(CodemanApp.prototype, { const statusRes = await fetch('/api/codex/status'); const status = (await statusRes.json()).data; if (!status.available) { - this.terminal.writeln('\x1b[1;31m Codex CLI not found.\x1b[0m'); - this.terminal.writeln('\x1b[90m Install with: npm install -g @openai/codex\x1b[0m'); + this._reportSessionLaunchError( + ownsLaunchTerminal, + 'Codex CLI not found. Install with: npm install -g @openai/codex' + ); return; } } @@ -1021,7 +1052,7 @@ Object.assign(CodemanApp.prototype, { this.terminal.focus(); } catch (err) { - this.terminal.writeln(`\x1b[1;31m Error: ${err.message}\x1b[0m`); + this._reportSessionLaunchError(ownsLaunchTerminal, err.message); } }, @@ -1032,9 +1063,7 @@ Object.assign(CodemanApp.prototype, { const _runLoc = (this.cases || []).find(c => c.name === caseName)?.location; const isRemote = _runLoc === 'remote' || _runLoc === 'docker'; - this.terminal.clear(); - this.terminal.writeln(`\x1b[1;32m Starting Gemini session in ${caseName}...\x1b[0m`); - this.terminal.writeln(''); + const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting Gemini session in ${caseName}...`); this.terminal.focus(); try { @@ -1042,8 +1071,10 @@ Object.assign(CodemanApp.prototype, { const statusRes = await fetch('/api/gemini/status'); const status = (await statusRes.json()).data; if (!status.available) { - this.terminal.writeln('\x1b[1;31m Gemini CLI not found.\x1b[0m'); - this.terminal.writeln('\x1b[90m Install with: npm install -g @google/gemini-cli\x1b[0m'); + this._reportSessionLaunchError( + ownsLaunchTerminal, + 'Gemini CLI not found. Install with: npm install -g @google/gemini-cli' + ); return; } } @@ -1072,7 +1103,7 @@ Object.assign(CodemanApp.prototype, { this.terminal.focus(); } catch (err) { - this.terminal.writeln(`\x1b[1;31m Error: ${err.message}\x1b[0m`); + this._reportSessionLaunchError(ownsLaunchTerminal, err.message); } }, diff --git a/test/run-mode-ui.test.ts b/test/run-mode-ui.test.ts index 0c5b25af..12efe6b8 100644 --- a/test/run-mode-ui.test.ts +++ b/test/run-mode-ui.test.ts @@ -74,6 +74,36 @@ describe('run mode UI', () => { }); describe('Run launch synchronization', () => { + it('keeps launch progress out of an active session terminal', () => { + const CodemanApp = function CodemanApp(this: any) {}; + const context = vm.createContext({ + CodemanApp, + localStorage: { getItem: () => null, setItem: () => {} }, + document: { getElementById: () => null }, + console, + }); + const sessionUi = readFileSync(resolve(import.meta.dirname, '../src/web/public/session-ui.js'), 'utf8'); + vm.runInContext(sessionUi, context, { filename: 'session-ui.js' }); + + const app = new (CodemanApp as any)(); + app.activeSessionId = 'existing-session'; + app.terminal = { + clear: vi.fn(), + writeln: vi.fn(), + }; + app.showToast = vi.fn(); + + const ownsTerminal = app._beginSessionLaunchStatus('Starting Codex session', '1;32'); + app._appendSessionLaunchStatus(ownsTerminal, 'Creating session'); + app._reportSessionLaunchError(ownsTerminal, 'Launch failed'); + + expect(ownsTerminal).toBe(false); + expect(app.terminal.clear).not.toHaveBeenCalled(); + expect(app.terminal.writeln).not.toHaveBeenCalled(); + expect(app.showToast).toHaveBeenNthCalledWith(1, 'Starting Codex session', 'info'); + expect(app.showToast).toHaveBeenNthCalledWith(2, 'Launch failed', 'error'); + }); + it('coalesces overlapping Run activations and disables the button while the request is active', async () => { const runBtn = { disabled: false, From bba3d80971ea34579357c304c2693df6f2030481 Mon Sep 17 00:00:00 2001 From: lior Date: Wed, 29 Jul 2026 09:11:53 +0300 Subject: [PATCH 06/24] test: isolate runtime state and PTY integration --- src/session.ts | 23 ++++++++++++++++------- test/setup.ts | 46 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/session.ts b/src/session.ts index 98e05940..460358ac 100644 --- a/src/session.ts +++ b/src/session.ts @@ -180,6 +180,8 @@ export function isAltScreenStripMode(mode: SessionMode): boolean { const DEFAULT_PTY_COLS = 120; const DEFAULT_PTY_ROWS = 40; const TMUX_DISPLAY_TIMEOUT_MS = 2000; +const IS_TEST_MODE = !!process.env.VITEST; +const TEST_PTY_SCRIPT = 'process.stdin.pipe(process.stdout);'; /** Delay before the in-container Claude CLI version probe (lets the container start). */ const DOCKER_CLI_VERSION_PROBE_DELAY_MS = 3000; @@ -1248,16 +1250,23 @@ export class Session extends EventEmitter { // No extra sleep — createSession() already waits for tmux readiness } - // Attach to the mux session via PTY - // Prevent tmux from letting the newest browser attach dictate global window - // size; accepted Codeman resize events update it explicitly below. - mux.setManualWindowSize?.(this._muxSession!.muxName); + // Integration tests need a live input/output transport without attaching to + // the host's tmux server or agent CLI. Production still uses the real mux. + if (!IS_TEST_MODE) { + // Prevent tmux from letting the newest browser attach dictate global window + // size; accepted Codeman resize events update it explicitly below. + mux.setManualWindowSize?.(this._muxSession!.muxName); + } // Query existing tmux window size so re-attach matches (avoids flicker from 120x40 default). // MUST go through the dedicated socket (mux.muxSocket); a bare `tmux display` hits the // default server, always fails for our socketed sessions, and silently falls back to 120x40. - const { cols: ptyCols, rows: ptyRows } = queryTmuxWindowSize(this._muxSession!.muxName, mux.muxSocket); + const { cols: ptyCols, rows: ptyRows } = IS_TEST_MODE + ? { cols: DEFAULT_PTY_COLS, rows: DEFAULT_PTY_ROWS } + : queryTmuxWindowSize(this._muxSession!.muxName, mux.muxSocket); + const attachCommand = IS_TEST_MODE ? process.execPath : mux.getAttachCommand(); + const attachArgs = IS_TEST_MODE ? ['-e', TEST_PTY_SCRIPT] : mux.getAttachArgs(this._muxSession!.muxName); try { - this.ptyProcess = pty.spawn(mux.getAttachCommand(), mux.getAttachArgs(this._muxSession!.muxName), { + this.ptyProcess = pty.spawn(attachCommand, attachArgs, { name: 'xterm-256color', cols: ptyCols, rows: ptyRows, @@ -2683,7 +2692,7 @@ export class Session extends EventEmitter { if (this.ptyProcess && (dimsChanged || options.force)) { this._ptyCols = cols; this._ptyRows = rows; - if (this._mux && this._muxSession) { + if (!IS_TEST_MODE && this._mux && this._muxSession) { this._mux.resizeWindow?.(this._muxSession.muxName, cols, rows); } this.ptyProcess.resize(cols, rows); diff --git a/test/setup.ts b/test/setup.ts index 160a1f7c..59881fd6 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -1,16 +1,36 @@ /** * @fileoverview Global test setup for Codeman tests * - * SAFETY: TmuxManager has built-in test mode detection - * (via process.env.VITEST) that makes ALL shell commands no-ops. - * This means tests CANNOT kill, create, or interact with real tmux - * sessions regardless of what the test code does. + * SAFETY: The suite gets a temporary HOME and explicitly enables runtime test + * mode before application modules load. Tests therefore cannot touch the real + * Codeman state/cases tree or launch external tmux-backed agent sessions. * * This setup file strips shell-level auth configuration that can leak from a * running Codeman instance, then handles mock/timer cleanup between tests. */ -import { afterEach, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, afterEach, vi } from 'vitest'; + +const originalHome = process.env.HOME; +const originalUserProfile = process.env.USERPROFILE; +const originalVitest = process.env.VITEST; +const originalPlaywrightBrowsersPath = process.env.PLAYWRIGHT_BROWSERS_PATH; +const testHome = mkdtempSync(join(tmpdir(), 'codeman-vitest-')); + +if (originalPlaywrightBrowsersPath === undefined && originalHome) { + process.env.PLAYWRIGHT_BROWSERS_PATH = + process.platform === 'darwin' + ? join(originalHome, 'Library', 'Caches', 'ms-playwright') + : process.platform === 'win32' + ? join(process.env.LOCALAPPDATA || originalHome, 'ms-playwright') + : join(originalHome, '.cache', 'ms-playwright'); +} +process.env.HOME = testHome; +process.env.USERPROFILE = testHome; +process.env.VITEST = 'true'; delete process.env.CODEMAN_PASSWORD; delete process.env.CODEMAN_USERNAME; @@ -23,3 +43,19 @@ afterEach(() => { vi.clearAllMocks(); vi.useRealTimers(); }); + +afterAll(() => { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + + if (originalVitest === undefined) delete process.env.VITEST; + else process.env.VITEST = originalVitest; + + if (originalPlaywrightBrowsersPath === undefined) delete process.env.PLAYWRIGHT_BROWSERS_PATH; + else process.env.PLAYWRIGHT_BROWSERS_PATH = originalPlaywrightBrowsersPath; + + rmSync(testHome, { recursive: true, force: true }); +}); From a406aef2fa4419d9c76523797dfeced9e42b91c6 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 29 Jul 2026 09:01:18 +0200 Subject: [PATCH 07/24] chore: version packages Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/553c3037.md | 9 -- .changeset/fix-claude-response-viewer.md | 9 -- .changeset/mobile-filesystem-path-picker.md | 10 -- CHANGELOG.md | 37 +++++ CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- docs/web-tabs-fixes-plan.md | 151 +++++++++++++++++++ docs/web-tabs.md | 27 +++- package-lock.json | 4 +- package.json | 23 ++- src/web/public/mobile.css | 11 ++ src/web/public/styles.css | 38 +++++ src/web/public/webview-tabs.js | 56 +++++-- src/web/webview-proxy.ts | 130 +++++++++++++++- test/webview-menu-rows.test.ts | 157 ++++++++++++++++++++ test/webview-proxy.test.ts | 124 ++++++++++++++++ 16 files changed, 732 insertions(+), 58 deletions(-) delete mode 100644 .changeset/553c3037.md delete mode 100644 .changeset/fix-claude-response-viewer.md delete mode 100644 .changeset/mobile-filesystem-path-picker.md create mode 100644 docs/web-tabs-fixes-plan.md create mode 100644 test/webview-menu-rows.test.ts diff --git a/.changeset/553c3037.md b/.changeset/553c3037.md deleted file mode 100644 index 10bdce07..00000000 --- a/.changeset/553c3037.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -'aicodeman': patch ---- - -Fix two multi-user scoping holes in the new filesystem path picker. `GET /api/filesystem/browse` and `GET /api/filesystem/preview` accept an optional `sessionId` that contributes the session's working directory as a browse root, but they resolved it straight off the session map without an ownership check, unlike the nine other session-scoped handlers in the same route file. A non-admin could therefore pin another user's working directory as a root simply by passing their session id, then list and preview files under it. Both endpoints now run `canAccessOwned` and report 404, which also avoids confirming that a session id exists. - -Separately, `Home` and `CASES_DIR` were unconditional browse roots for every caller. Per-user spaces live at `/`, which is inside `homedir()`, so the `Home` root alone exposed every other user's workspace to any authenticated user. In multi-user mode a non-admin now gets only their own space plus anything explicitly listed in `CODEMAN_FILE_PICKER_ROOTS`; `/mnt/d` is no longer offered by default, since a broad host mount should be an explicit operator decision in a multi-user deployment. Admins keep the host-wide roots, and single-user mode is unchanged. - -Both holes are regression-guarded in `test/routes/file-routes.test.ts`, verified to fail against the previous code. Multi-user mode is opt-in and off by default, so single-user installs were never affected. diff --git a/.changeset/fix-claude-response-viewer.md b/.changeset/fix-claude-response-viewer.md deleted file mode 100644 index 53cf018b..00000000 --- a/.changeset/fix-claude-response-viewer.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -'aicodeman': patch ---- - -Normalize Claude conversations in the response viewer. A Claude transcript is an append-only event log, so one logical exchange spans many JSONL rows: tool-result rows, meta/image/skill rows, compact summaries, task and team notifications, sidechains, replayed assistant snapshots, and multi-block assistant output. The viewer rendered a card per row, which produced duplicate and truncated cards that read as lost responses. Cards are now built at real human-turn boundaries, replayed assistant snapshots are deduplicated, and sidechain rows (which belong to subagents, not the main conversation) no longer leak in. An identical prompt that legitimately recurs after an assistant reply is still kept as its own turn. - -Measured over 40 real transcripts: 3108 cards became 621, duplicate cards dropped from 74 to 8 (all of them genuinely repeated turns), no assistant text was lost, and the non-`context=full` last-response text was byte-identical on every file. - -Also rebinds recovered sessions to their transcript. `reconcileSessions()` can recover a lost mux session as a `restored-` placeholder with a stale working directory, which made transcript lookup by cwd find nothing. The placeholder still carries the first eight characters of the conversation UUID, so the viewer now rebinds to the matching top-level transcript when exactly one candidate matches. diff --git a/.changeset/mobile-filesystem-path-picker.md b/.changeset/mobile-filesystem-path-picker.md deleted file mode 100644 index 9427c9ad..00000000 --- a/.changeset/mobile-filesystem-path-picker.md +++ /dev/null @@ -1,10 +0,0 @@ ---- -"aicodeman": minor ---- - -feat(mobile): browse and insert local file and folder paths - -Add a root-confined filesystem picker to Link Existing and the extended mobile -keyboard bar. Selected paths remain editable at the active prompt, supported -images/documents/text files open in a safe inline preview, and a new one-tap -action clears only the current unsent input without invoking `/clear`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d3f3fd3..5a2be4c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,42 @@ # aicodeman +## 1.9.0 + +### Minor Changes + +- 2667150: feat(mobile): browse and insert local file and folder paths + + Add a root-confined filesystem picker to Link Existing and the extended mobile + keyboard bar. Selected paths remain editable at the active prompt, supported + images/documents/text files open in a safe inline preview, and a new one-tap + action clears only the current unsent input without invoking `/clear`. + +### Patch Changes + +- 3cff98f: Fix two multi-user scoping holes in the new filesystem path picker. `GET /api/filesystem/browse` and `GET /api/filesystem/preview` accept an optional `sessionId` that contributes the session's working directory as a browse root, but they resolved it straight off the session map without an ownership check, unlike the nine other session-scoped handlers in the same route file. A non-admin could therefore pin another user's working directory as a root simply by passing their session id, then list and preview files under it. Both endpoints now run `canAccessOwned` and report 404, which also avoids confirming that a session id exists. + + Separately, `Home` and `CASES_DIR` were unconditional browse roots for every caller. Per-user spaces live at `/`, which is inside `homedir()`, so the `Home` root alone exposed every other user's workspace to any authenticated user. In multi-user mode a non-admin now gets only their own space plus anything explicitly listed in `CODEMAN_FILE_PICKER_ROOTS`; `/mnt/d` is no longer offered by default, since a broad host mount should be an explicit operator decision in a multi-user deployment. Admins keep the host-wide roots, and single-user mode is unchanged. + + Both holes are regression-guarded in `test/routes/file-routes.test.ts`, verified to fail against the previous code. Multi-user mode is opt-in and off by default, so single-user installs were never affected. + +- Web tabs: delete saved URLs from the Run dropdown, and fix images in proxied dashboards. + + **Saved URLs are now manageable from the dropdown.** Each row under "Web / URL" gains a gear and an `x`, so a URL can be edited or deleted without first opening it as a tab. Previously the only delete path ran through the gear on an open tab, which was a dead end for a URL you no longer wanted open at all. Both controls stay permanently visible rather than hover-revealed, because the same menu is used on touch, and they get a larger hit box there. Deleting leaves the dropdown open on the remaining rows, and deleting the dashboard that is currently open also closes its tab and unmounts its frame. + + **Runtime-injected images no longer 404.** A dashboard that renders its own markup from script (`card.innerHTML = ''`, `img.src = '/api/slide'`) escaped every rewrite layer at once: `` never applies to a root-absolute URL, the server-side attribute rewrite only ever sees the initial document, and `runtimeUrlShim()` patched only `fetch`, `XMLHttpRequest`, `WebSocket` and `EventSource`. Those requests landed on Codeman's own root and 404'd, with a symptom that reads as an upstream fault: the dashboard's data loaded while every image stayed broken. + + The shim now also covers the DOM URL sinks, so the request is never emitted in the first place and neither the `/api` fence in the 404 fallback nor the one in the auth middleware had to move. It wraps `innerHTML`, `outerHTML`, `insertAdjacentHTML` (including on `ShadowRoot`), `setAttribute`/`setAttributeNS`, and the `src`/`srcset`/`href`/`poster`/`data`/`action` property setters on img, source, media, video poster, script, iframe, embed, track, link, anchor, area, object and form, with a `MutationObserver` as a last net for sinks not patched above. Every rewrite routes through the same idempotent helper, which matters because unlike the server-side rewrite this one sees markup that may already be proxied, and a page re-injecting its own `outerHTML` would otherwise double-prefix. Everything is defensively guarded and marked so a double injection cannot wrap an already-wrapped setter. + + Measured against a real dashboard: 693 image elements, 0 of them under the proxy prefix and 0 of 23 in-viewport images decoded before, 693 and 23 of 23 after. Covered by a new jsdom suite over the shim's DOM half and a new frontend suite over the dropdown rows. Known remaining gaps are documented in `docs/web-tabs.md`: a root-absolute `url()` inside a stylesheet injected at runtime, and self-navigation via `location.href`, which cannot be patched because `Location.href` is unforgeable. + + Also in this release: a value-first README overhaul pointing at getcodeman.com, and the QR-auth distribution test now uses a chi-square check instead of a max-deviation threshold that failed on random variance. + +- bca56b4: Normalize Claude conversations in the response viewer. A Claude transcript is an append-only event log, so one logical exchange spans many JSONL rows: tool-result rows, meta/image/skill rows, compact summaries, task and team notifications, sidechains, replayed assistant snapshots, and multi-block assistant output. The viewer rendered a card per row, which produced duplicate and truncated cards that read as lost responses. Cards are now built at real human-turn boundaries, replayed assistant snapshots are deduplicated, and sidechain rows (which belong to subagents, not the main conversation) no longer leak in. An identical prompt that legitimately recurs after an assistant reply is still kept as its own turn. + + Measured over 40 real transcripts: 3108 cards became 621, duplicate cards dropped from 74 to 8 (all of them genuinely repeated turns), no assistant text was lost, and the non-`context=full` last-response text was byte-identical on every file. + + Also rebinds recovered sessions to their transcript. `reconcileSessions()` can recover a lost mux session as a `restored-` placeholder with a stale working directory, which made transcript lookup by cwd find nothing. The placeholder still carries the first eight characters of the conversation UUID, so the viewer now rebinds to the matching top-level transcript when exactly one candidate matches. + ## 1.8.3 ### Patch Changes diff --git a/CLAUDE.md b/CLAUDE.md index 9abf9837..0e5cc5a5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,7 +74,7 @@ When user says "COM": CI runs `npm run check:lockfile` on every push/PR, so lockfile drift fails the build even if the `version-packages` script is bypassed. -**Version**: 1.8.3 (must match `package.json`) +**Version**: 1.9.0 (must match `package.json`) ## Project Overview diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 6f954c77..ec2a5826 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -117,7 +117,7 @@ The general rule: **any new endpoint that turns a caller-supplied `sessionId` in **Two things a sandboxed frame breaks that are invisible to `curl`.** Both were found only by driving a real dashboard in a real browser, and both present identically as the dashboard's own "Failed to fetch" while the page itself renders fine: -1. **Root-absolute URLs built at runtime.** `` only governs URLs the HTML parser resolves; `fetch('/api/data')` bypasses it and lands on Codeman's root. That is how most dashboards talk to their own backend. The `Referer`-keyed 404 fallback deliberately refuses `/api`, `/ws`, `/q` (widening it there would let a request-supplied header skip auth on Codeman's own API), so the fix is `runtimeUrlShim()`: a small script injected right after `` that patches `fetch`, `XMLHttpRequest.open`, `WebSocket` and `EventSource` to rebase root-absolute and same-origin-absolute URLs into the prefix. It removes the whole class inside the iframe instead of trading security for it. ⚠️ It must be injected even when the page ships its OWN `` (an early return there silently breaks exactly the pages that need it most). +1. **Root-absolute URLs built at runtime.** `` only governs URLs the HTML parser resolves; `fetch('/api/data')` bypasses it and lands on Codeman's root. That is how most dashboards talk to their own backend. The `Referer`-keyed 404 fallback deliberately refuses `/api`, `/ws`, `/q` (widening it there would let a request-supplied header skip auth on Codeman's own API), so the fix is `runtimeUrlShim()`: a small script injected right after `` that rebases root-absolute and same-origin-absolute URLs into the prefix. It removes the whole class inside the iframe instead of trading security for it. ⚠️ It must be injected even when the page ships its OWN `` (an early return there silently breaks exactly the pages that need it most). ⚠️ **The DOM sinks are as load-bearing as `fetch`.** Patching only `fetch`/`XHR`/`WebSocket`/`EventSource` leaves `container.innerHTML = ''` and `img.src = '/api/slide'` untouched, and neither of the other layers can reach those either (`` never applies to root-absolute URLs, and `rewriteHtml()` only ever sees the INITIAL document, never markup built later by page script). The symptom is precise and easy to misdiagnose as an upstream fault: the dashboard's **data** loads while every **image** stays broken. So the shim also wraps `innerHTML`/`outerHTML`/`insertAdjacentHTML`, `setAttribute`/`setAttributeNS`, and the `src`/`srcset`/`href`/`poster`/`data`/`action` property setters, with a `MutationObserver` as a last net for sinks not patched above. Every rewrite routes through the same idempotent `rw()`, which matters because unlike the server-side rewrite this one sees markup that may ALREADY be proxied (a page re-injecting its own `outerHTML` would otherwise double-prefix). The DOM half is pinned in jsdom by `test/webview-proxy.test.ts`; `curl` cannot see any of it. 2. **CORS on same-host requests.** An opaque-origin document treats EVERY request as cross-origin, including to the very host it was served from, so its `fetch`/XHR are CORS-checked and its preflights carry `Origin: null`. Static subresources (script/css/img) are NOT CORS-checked, which is why the page renders while its API calls die with an opaque `net::ERR_FAILED`. `buildProxyCorsHeaders()` echoes the origin (omitting `allow-credentials` for `null`, which browsers reject in combination), upstream `access-control-*` headers are dropped (they describe the dashboard's origin, not the frame's), and the proxy answers preflights itself rather than relaying them. ⚠️ `registerSecurityHeaders` answers EVERY `OPTIONS` with a bare 204 before routing, and its CORS block only emits headers for localhost origins, so that short-circuit **must** exempt a valid webview capability or every preflight fails. `curl` cannot reproduce any of this because curl does not enforce CORS. **Rewrites, each load-bearing** (pure + unit-tested in `src/web/webview-proxy.ts`): drop `x-frame-options` and the CSP `frame-ancestors` directive (the point of the proxy); drop `content-encoding`/`content-length` because undici's `fetch` already decoded the body (forwarding them makes the browser gunzip plaintext); rewrite `Location` for same-origin redirects only, handing CROSS-origin redirects back unchanged so this never becomes an open relay; rebase `Set-Cookie` `Path` onto the prefix and drop `Domain`; inject `` and rebase root-absolute `src`/`href`/`action`. `resolveUpstreamUrl()` returns null on anything escaping the upstream origin. diff --git a/docs/web-tabs-fixes-plan.md b/docs/web-tabs-fixes-plan.md new file mode 100644 index 00000000..3b7d6578 --- /dev/null +++ b/docs/web-tabs-fixes-plan.md @@ -0,0 +1,151 @@ +# Web tabs: two fixes (planned + implemented 2026-07-28) + +Both found against the saved dashboard +`https://macminis-mac-mini.tailf80371.ts.net:4000` (Bio-Hacking-Dashboard). +Kept because the root-cause analysis of the second one is not obvious from the +resulting diff. + +Status: **both implemented and verified end-to-end.** The one deliberate +non-change is recorded at the bottom. + +--- + +## Bug 1: saved URLs could not be deleted from the Run dropdown + +### What happened + +The "Web / URL" section of the Run dropdown listed every saved dashboard as a +single clickable row whose only action was "open". Deleting required opening the +dashboard as a tab, clicking the tab's gear, then Delete in the modal, so a URL +you no longer wanted open at all could not be removed without first opening it. + +### What shipped + +- `renderWebviewMenuItems()` (`src/web/public/webview-tabs.js`) now renders each + saved URL as a `.run-mode-row--web` flex row: the open button, a gear + (`showWebviewModal`), and an `x` (`deleteWebviewById`). Nested buttons are + invalid HTML, hence the wrapper rather than a button inside a button. +- `deleteWebview()` split into the modal entry point, the new row entry point + `deleteWebviewById(id)`, and the shared `_confirmAndDeleteWebview(id)`. +- Both side buttons call `event.stopPropagation()` so the click does not also + open the dashboard. +- The dropdown's outside-click handler (`session-ui.js`) closes when the click + target is not inside `#runModeMenu`, and the row is gone by the time the delete + resolves, so `deleteWebviewById` re-asserts `.active` on the menu. Verified in a + browser: deleting one of several URLs leaves you looking at the rest of the list. +- CSS in `styles.css` (`.run-mode-row--web`, `.run-mode-row-btn`) plus a larger + touch target in `mobile.css`. The side buttons are permanently visible rather + than hover-revealed, because this menu is used on touch. + +No server change: `DELETE /api/webviews/:id` already existed, owner-scoped, and +already revoked the capability and broadcast `WebviewChanged`. + +--- + +## Bug 2: images did not load in a proxied dashboard + +### Reproduction (before the fix) + +``` +CAP=/open> +# A) upstream direct -> 200 image/jpeg 118150 +curl -sk "https://macminis-mac-mini.tailf80371.ts.net:4000/api/hero?slug=120-minutes-in-nature" +# B) through the proxy prefix -> 200 image/jpeg 118150 +curl -sk "https://localhost:3000/webview/$CAP/api/hero?slug=120-minutes-in-nature" +# C) what the browser ACTUALLY requested -> 404 {"errorCode":"NOT_FOUND"} +curl -sk -H "Referer: https://localhost:3000/webview/$CAP/" \ + "https://localhost:3000/api/hero?slug=120-minutes-in-nature" +# D) same shape but NOT under /api -> 200 (referer fallback rescues it) +curl -sk -H "Referer: https://localhost:3000/webview/$CAP/" "https://localhost:3000/styles.css" +``` + +The proxy itself was fine (B). The failure was entirely about which URL the +browser ended up requesting (C). + +### Root cause + +The dashboard builds its image markup at runtime with root-absolute URLs: +`c.innerHTML = ''`, `img.src = +slideSrc(...)` returning `/api/slide?owner=...`, `/api/story`, `/api/video`, and a +nested `