From 80397fe140f4c58ae6897c1b39a611451ec6f21b Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 6 Sep 2026 23:05:26 +0200 Subject: [PATCH] fix(hooks,mobile): the merge-time items from the #367 and #368 reviews #367 (UserPromptSubmit hook): `hook:prompt_submitted` went on the wire unregistered; it is now in both SSE registries (158 = 158), and the hook only lands in the run summary when the conversation actually moved, since one row per prompt would evict useful rows from the 1000-event FIFO and clutter the Summary timeline and /api/search. #368 (Add Case header submit): the pending-state dimming targeted the footer button, which the <=860px layout hides, so on a phone the only visible submit control stayed at full brightness while a clone ran. The header button now dims too, and a static test pins the header-submit contract so it cannot silently disappear again. Co-Authored-By: Claude Fable 5.1 --- CLAUDE.md | 4 +- src/web/public/constants.js | 1 + src/web/public/mobile.css | 9 ++++ src/web/routes/hook-event-routes.ts | 10 +++- src/web/sse-events.ts | 6 ++- test/create-case-modal-structure.test.ts | 59 ++++++++++++++++++++++++ 6 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 test/create-case-modal-structure.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 7db88134..e0375930 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -230,7 +230,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Owner tab layouts** (COD-359, `tab-layout*.ts` + `GET`/`PUT /api/tab-layout`): named tab GROUPS over the flat tab strip, scoped per owner (`SINGLE_USER_LAYOUT_OWNER` = `@single` when multi-user is off), persisted under the `tabLayouts` key in state.json. A layout is `{version, groups[], ungrouped[], updatedAt}` whose refs point at either a session or a saved webview (`TabRefKind`), capped at 32 groups / 512 refs. ⚠️ **BACKEND ONLY as of 1.24.1**: nothing in `src/web/public/` calls these routes yet, so a UI built on top is new frontend work, not a rewiring job. ⚠️ **`TabLayoutService` is the single mutation boundary** and every lifecycle caller (session created/removed, webview created/deleted, a legacy order PUT) describes ONE completed server action and gets AT MOST ONE versioned write; writing layout state from a route or a manager directly is what the service exists to prevent. ⚠️ The layout does not replace `PUT /api/session-order`, it PROJECTS onto it: `tab-layout-legacy-order.ts` is the pure translation both ways (`putLegacyOrder()` recomposes a global order from the owner's groups), so changing one side without the other silently desyncs the tab strip from the stored layout. ⚠️ **Reconciliation is gated on a SUCCESSFUL restore** (`markRestorationComplete` / `markRestorationFailed` / `markRestorationSkipped`, plus `assertDeletionReady()`): pruning refs against a session list that failed to load would delete live tabs, so a failed restore must leave the layout untouched. `PUT` takes exactly `{baseVersion, layout}` (any other key shape is a validation error), answers a stale `baseVersion` with the current layout rather than clobbering, and is capped at 128 KiB. Broadcasts `tab:layoutChanged`, owner-routed via `deriveTabLayoutSseHint`. -**Hook events**: Claude Code hooks trigger via `/api/hook-event`. Key events: `permission_prompt`, `elicitation_dialog`, `elicitation_complete`, `elicitation_response`, `idle_prompt`, `stop`, `teammate_idle`, `task_completed`. See `src/hooks-config.ts`; upstream hook semantics mirrored in `docs/claude-code-hooks-reference.md`. ⚠️ **Every claude session INSTALLS the hooks block into its workspace** (`applyWorkspaceHooks` in hooks-config.ts → `ensureCodemanHooks`, an add-only merge that keeps a user's own handlers), from EVERY claude create path — both interactive routes, cron fires, legacy scheduled runs, the plan-orchestrator one-shots — and from `restoreMuxSessions()` for sessions recovered on server start (that boot sweep skips a workspace that no longer exists, so a deleted repo with a surviving tmux session is never resurrected as an empty dir). Before 2026-08-15 hooks were written ONLY when Codeman created the case DIRECTORY, so a linked case / cloned repo — where most sessions actually run — had no hooks at all and every hook-driven surface was silently dead there: an AskUserQuestion dialog blocked the pane while the tab and the phone overview both read a calm `idle`, with no Approvals Inbox item, no push, no definitive `stop`/`idle_prompt` for respawn and no `stop`/`blocked` for the wait endpoints. The escape hatch is the synced `workspaceHooksEnabled` setting (App Settings → Agents & CLIs → Claude, **default ON**); OFF restores the old behavior, where a Codeman block that is already there is still refreshed when stale (COD-91) but one is never added. ⚠️ Route the decision through `applyWorkspaceHooks` rather than calling `ensureCodemanHooks` at a new site, or the setting silently stops applying to that path. ⚠️ Claude Code RE-READS `settings.local.json`, so an already-running session starts firing hooks without a restart (measured 2026-08-15) — and the notification for a blocking dialog is delayed by Claude Code (~30s), so the alert trails the dialog. ⚠️ An AskUserQuestion / plan-selection dialog arrives as **`permission_prompt`**, not `elicitation_dialog` (that one is MCP elicitation), so it renders as the RED "needs you" alert, not the yellow idle one. +**Hook events**: Claude Code hooks trigger via `/api/hook-event`. Key events: `permission_prompt`, `elicitation_dialog`, `elicitation_complete`, `elicitation_response`, `idle_prompt`, `stop`, `teammate_idle`, `task_completed`, `prompt_submitted` (UserPromptSubmit, #367: a Claude pane reports its live conversation id first-hand). See `src/hooks-config.ts`; upstream hook semantics mirrored in `docs/claude-code-hooks-reference.md`. ⚠️ **Every claude session INSTALLS the hooks block into its workspace** (`applyWorkspaceHooks` in hooks-config.ts → `ensureCodemanHooks`, an add-only merge that keeps a user's own handlers), from EVERY claude create path — both interactive routes, cron fires, legacy scheduled runs, the plan-orchestrator one-shots — and from `restoreMuxSessions()` for sessions recovered on server start (that boot sweep skips a workspace that no longer exists, so a deleted repo with a surviving tmux session is never resurrected as an empty dir). Before 2026-08-15 hooks were written ONLY when Codeman created the case DIRECTORY, so a linked case / cloned repo — where most sessions actually run — had no hooks at all and every hook-driven surface was silently dead there: an AskUserQuestion dialog blocked the pane while the tab and the phone overview both read a calm `idle`, with no Approvals Inbox item, no push, no definitive `stop`/`idle_prompt` for respawn and no `stop`/`blocked` for the wait endpoints. The escape hatch is the synced `workspaceHooksEnabled` setting (App Settings → Agents & CLIs → Claude, **default ON**); OFF restores the old behavior, where a Codeman block that is already there is still refreshed when stale (COD-91) but one is never added. ⚠️ Route the decision through `applyWorkspaceHooks` rather than calling `ensureCodemanHooks` at a new site, or the setting silently stops applying to that path. ⚠️ Claude Code RE-READS `settings.local.json`, so an already-running session starts firing hooks without a restart (measured 2026-08-15) — and the notification for a blocking dialog is delayed by Claude Code (~30s), so the alert trails the dialog. ⚠️ An AskUserQuestion / plan-selection dialog arrives as **`permission_prompt`**, not `elicitation_dialog` (that one is MCP elicitation), so it renders as the RED "needs you" alert, not the yellow idle one. **Approvals Inbox** (cross-session queue of prompts waiting on a human; `approvalsInboxEnabled`, SYNCED, default OFF: every surface is opt-in; only the store and answer endpoints run regardless, so flipping it ON shows anything already pending): `web/approval-inbox.ts` is a `sessionWaits`-style singleton fed by `/api/hook-event`, holding at most ONE item per session (a new prompt supersedes), claude-mode only, in-memory. Cards are answered via `POST /api/approvals/:id/answer`, which sends a digit / Esc / idle-prompt text through `writeViaMux` (menu answers never carry `\r`). ⚠️ `option` digits are accepted ONLY when they match options parsed from the captured pane frame, and the answer path RE-CAPTURES the pane first (a dialog that no longer parses on screen means the keystroke would land in the composer, so refuse with 409). ⚠️ Resolution on the heuristic `working` signal ALONE is restricted to `idle` items; a permission/question item gets the pane-VERIFIED variant on that same signal (`resolveIfDialogGone()` → `verifyStillAnswerable()`), so the heuristic only decides when to LOOK and the screen decides the outcome. That is what clears a dialog answered in the terminal mid-turn; the other definitive signals are `stop`, `elicitation_complete`/`elicitation_response`, exit/delete, answer, supersede and the 12h TTL. ⚠️ **Viewing a session ACKNOWLEDGES its idle item, it does not resolve it** (`POST /api/approvals/session/:sessionId/viewed` → `acknowledgedAt` → `approval:updated`): the item stays pending (still answerable, still Read My Mind context) and only stops arming the yellow tab alert. That flag is what makes the clear durable, since the view-clears-idle rule used to live in one browser's memory and `seedApprovals()` re-armed the alert on the next reload while other devices never heard about it at all; the local half is `markIdleAlertSeen()` (app.js), called from BOTH `selectSession` paths, including the already-active early return, where a click could otherwise never clear the alert. ⚠️ **Only a HUMAN opening a session acknowledges**: `selectSession(id, { auto: true })` marks the three selections the APP makes (boot restore, a solo window opening its target, the fallback after the active session is closed) and skips the acknowledgement, so a page load cannot silently spend an alert the user never saw. The flag defaults to user-initiated, so an untagged call site fails toward acknowledging rather than toward an alert nothing can clear; `test/session-select-ack-gate.test.ts` pins both the gate and the tagged call sites. Idle-only by construction (`acknowledge()` defaults to `['idle']`): looking at a permission/question dialog does not answer it. ⚠️ Same rule on the input path: `_ackDelivery` (app.js) spends the IDLE alert only, via that same `markIdleAlertSeen()`. It used to `clearPendingHooks(sessionId)` with no kind, so one keystroke wiped a RED alert on that device while the dialog was still up, the other devices stayed red, and a reload re-seeded it. ⚠️ Claude Code fires no "permission answered" hook (only `elicitation_complete`/`elicitation_response`, i.e. the question flavor), so an answered-in-the-terminal dialog would otherwise sit pending until `stop`: `GET /api/approvals` therefore runs a **staleness sweep** over the caller's own items via `verifyStillAnswerable()`, which is deliberately the conservative check the answer path uses (only an item whose ORIGINAL frame parsed options can be dropped, so an unreadable capture keeps the alert rather than losing a live one). ⚠️ **`applyCapture()` is therefore ADD-ONLY for `options`**: a re-capture that parses nothing must never erase a parse an earlier one found. Claude Code delays the Notification hook behind the dialog (measured 6s, documented ~30s), so the 600ms re-capture routinely lands on a frame the user has ALREADY answered; clearing the field there made the item permanently unsweepable, because `verifyStillAnswerable()` reads a MISSING `options` as "we never could read this dialog" and keeps such items answerable by design. The red "needs you" then survived every sweep AND every page reload, went away only on `stop` (2026-08-20: a confirmed question left a tab flowing red for ~8 minutes while the turn ran on), and the stale card still accepted an answer, typing a bare `1` into a composer with no dialog under it. Pinned by `test/approval-inbox.test.ts`. ⚠️ A frame that parses no options is CONCLUSIVE in exactly two cases, and the second one closes the late-hook hole: the item once parsed options (they cannot vanish while the dialog is up), or the frame shows Claude actively running a turn. A modal dialog BLOCKS the turn, so the two cannot coexist — measured on v2.1.237, a live-dialog frame carries neither the `… (13s` timer NOR the `esc to interrupt` footer, which the dialog replaces with `Enter to select · ↑/↓ to navigate · Esc to cancel`. Anything else stays answerable, so an unreadable capture still keeps the alert. That second signal is reached by a delayed staleness pass (`STALE_CHECK_DELAY_MS`, 3s) scheduled alongside the re-capture, because a prompt answered BEFORE the hook lands creates an item whose FIRST capture already has no dialog in it: nothing ever parsed, `stop` may have fired already, and the alert then outlived reloads until the 12h TTL. ⚠️ That pass must stay comfortably LATER than `RECAPTURE_DELAY_MS`, whose whole reason for existing is that the hook can beat Ink to the screen — resolving inside the paint window would clear the alert for a dialog that was about to appear. The frontend seeds from `GET /api/approvals` in `handleInit` **regardless of the setting**: the seed re-arms the tab-alert state machine (`setPendingHook`) unconditionally, and only populating `this.approvals` (the inbox surfaces) is gated — seeding used to be gated wholesale, which left a reloaded page with NO red tab while a permission dialog sat blocking a session (2026-08-15); `_onApprovalResolved` clears the pending-hook alert unconditionally for the same reason. ⚠️ The red/yellow tab alert itself is a STEADY border/background/dot with a pulse on top: the original keyframes swung to transparent at 0%/100%, so half of every cycle looked like a normal tab. Push Approve/Deny buttons stay gated on the setting (`sendPushNotifications` strips `actions`/`approvalId` when OFF) and are answered from `sw.js` directly so they work with no tab open. Surfaces (all gated on the setting): header bell (marker-hidden until count > 0, phones never show it) + drawer (`approvals-ui.js`), phone overview NEEDS YOU answer strips (`mobile-overview.js`). Design: `docs/approvals-inbox-plan.md`. @@ -357,7 +357,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L ### SSE Event Registry -157 event constants in `src/web/sse-events.ts` (backend) and `SSE_EVENTS` in `constants.js` (frontend). **Both must be kept in sync**, and `test/sse-registry-parity.test.ts` is the guard that pins it (currently exactly in sync, 157 = 157, no drift either direction). ⚠️ `hook:agent_working` is the one hook event with no Claude Code hook behind it — the DeepSeek status bridge reports it (see External CLI modes). The backend file's `@fileoverview` carries the per-category breakdown, including the two Web tab events. +158 event constants in `src/web/sse-events.ts` (backend) and `SSE_EVENTS` in `constants.js` (frontend). **Both must be kept in sync**, and `test/sse-registry-parity.test.ts` is the guard that pins it (currently exactly in sync, 158 = 158, no drift either direction). ⚠️ `hook:agent_working` is the one hook event with no Claude Code hook behind it — the DeepSeek status bridge reports it (see External CLI modes). The backend file's `@fileoverview` carries the per-category breakdown, including the two Web tab events. ### API Routes diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 80308150..d93888f9 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -958,6 +958,7 @@ const SSE_EVENTS = { HOOK_AGENT_WORKING: 'hook:agent_working', HOOK_TEAMMATE_IDLE: 'hook:teammate_idle', HOOK_TASK_COMPLETED: 'hook:task_completed', + HOOK_PROMPT_SUBMITTED: 'hook:prompt_submitted', // Approvals Inbox APPROVAL_PENDING: 'approval:pending', diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index 7dc2655c..b313f097 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -3355,6 +3355,15 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat font-size: 0.86rem; } + /* Add Case's pending state has to show on the header button: below 860px it is + the only submit control (the footer is hidden), and the #caseModalSubmit + .loading rule in the phone block dims a button nobody can see. Measured at + 390px before this: header opacity 1 for the whole clone, hidden footer 0.6. */ + #createCaseModal .set-head-save.loading { + opacity: 0.6; + pointer-events: none; + } + :is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-head-actions .modal-close { width: 36px; height: 36px; diff --git a/src/web/routes/hook-event-routes.ts b/src/web/routes/hook-event-routes.ts index b69e4129..fe96a0f7 100644 --- a/src/web/routes/hook-event-routes.ts +++ b/src/web/routes/hook-event-routes.ts @@ -126,6 +126,7 @@ export function registerHookEventRoutes( // Sync Claude's current conversation id. Interactive PTY mode never emits // `session_id` on stdout, so hooks are the only reliable way to learn that // the user ran `/clear` (which spins up a new conversation jsonl). + let conversationChanged = false; if (data && typeof data.session_id === 'string' && data.session_id) { const session = ctx.sessions.get(sessionId); const prevClaudeSessionId = session?.claudeSessionId; @@ -150,6 +151,7 @@ export function registerHookEventRoutes( session && (session.claudeSessionId !== prevClaudeSessionId || session.claudeSessionChain.length !== prevChainLength) ) { + conversationChanged = true; ctx.persistSessionState(session); } // Docker sessions: keep the case's resume seed following the LIVE @@ -229,9 +231,13 @@ export function registerHookEventRoutes( ...(approvalId && session?.mode !== 'deepseek' && { approvalId }), }); - // Track in run summary + // Track in run summary. `prompt_submitted` fires on EVERY prompt of every + // Claude pane; only the ones where the conversation actually moved (a /clear + // successor) carry information, and recording the rest would push a row into + // the Summary timeline and /api/search per turn and evict useful rows from + // the 1000-event FIFO (#367 merge-time fix). const summaryTracker = ctx.runSummaryTrackers.get(sessionId); - if (summaryTracker) { + if (summaryTracker && (event !== 'prompt_submitted' || conversationChanged)) { summaryTracker.recordHookEvent(event, safeData); } diff --git a/src/web/sse-events.ts b/src/web/sse-events.ts index 6042d7a1..b89c3425 100644 --- a/src/web/sse-events.ts +++ b/src/web/sse-events.ts @@ -5,7 +5,7 @@ * and referenced by the frontend (`SSE_EVENTS` in `constants.js`). * Both files MUST be kept in sync. * - * 157 event constants organized by category: + * 158 event constants organized by category: * - **Core** (1): init * - **Transport** (1): sse:heartbeat * - **Session lifecycle** (23): created, updated, deleted, terminal, idle, working, ... @@ -25,7 +25,7 @@ * - **Plan orchestration** (5): started, progress, subagent, completed, cancelled * - **Tunnel** (7): started, stopped, progress, error, qrRotated, qrRegenerated, qrAuthUsed * - **Image / attachments** (2): image:detected, attachment:detected - * - **Hooks** (9): idle_prompt, permission_prompt, elicitation_dialog, elicitation_complete, elicitation_response, stop, agent_working, teammate_idle, task_completed + * - **Hooks** (10): idle_prompt, permission_prompt, elicitation_dialog, elicitation_complete, elicitation_response, stop, agent_working, teammate_idle, task_completed, prompt_submitted * (agent_working is the odd one out: reported by the DeepSeek Harness status bridge, not by a Claude Code hook) * - **Approvals** (3): pending, updated, resolved (cross-session Approvals Inbox) * - **Orchestrator** (12): stateChanged, planProgress, planReady, phase*, verification, task*, completed, error @@ -372,6 +372,8 @@ export const HookAgentWorking = 'hook:agent_working' as const; export const HookTeammateIdle = 'hook:teammate_idle' as const; /** Claude Code hook: teammate task completed. */ export const HookTaskCompleted = 'hook:task_completed' as const; +/** UserPromptSubmit fired in a Claude pane (#367): the pane learned its live conversation id first-hand. */ +export const HookPromptSubmitted = 'hook:prompt_submitted' as const; // ─── Approvals Inbox ───────────────────────────────────────────────────────── diff --git a/test/create-case-modal-structure.test.ts b/test/create-case-modal-structure.test.ts new file mode 100644 index 00000000..fd643e19 --- /dev/null +++ b/test/create-case-modal-structure.test.ts @@ -0,0 +1,59 @@ +/** + * @fileoverview Static guard for the Add Case modal's submit controls (#368). + * + * Below 860px the shared set-* surface hides the modal footer, and for eight + * releases that footer held the only Create/Clone/Link button, so no case could + * be added from a phone and nothing failed. This pins the contract that fixed it: + * a header submit button exists after the close button, and the two JS paths + * that toggle submit state drive BOTH buttons. + */ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; + +const publicDir = resolve(import.meta.dirname, '../src/web/public'); +const html = readFileSync(resolve(publicDir, 'index.html'), 'utf8'); +const sessionUi = readFileSync(resolve(publicDir, 'session-ui.js'), 'utf8'); +const mobileCss = readFileSync(resolve(publicDir, 'mobile.css'), 'utf8'); + +function caseModal(): string { + const start = html.indexOf('