diff --git a/.changeset/fix-claude-conversation-first-hand.md b/.changeset/fix-claude-conversation-first-hand.md new file mode 100644 index 000000000..41755447f --- /dev/null +++ b/.changeset/fix-claude-conversation-first-hand.md @@ -0,0 +1,23 @@ +--- +"aicodeman": patch +--- + +fix(session): learn the live Claude conversation from the CLI's own hook + +Which conversation a pane is on was re-derived by correlating `~/.claude/history.jsonl` +against `Session.lastSubmitAt` — and `lastSubmitAt` is bumped only by input that flows +through Codeman's own write path. A user who attaches to the pane's tmux session directly +never set it, so the resolver returned at its first line for that pane's whole life and the +response viewer stayed pinned to the launch conversation, showing a pre-`/clear` transcript +indefinitely. A new `UserPromptSubmit` hook reports the live conversation id from inside the +CLI process, addressed by the pane's own `$CODEMAN_SESSION_ID`, so the id is a fact rather +than a correlation: it never consults the working directory and cannot be claimed by a +sibling pane on the same folder. A pane with such an id now skips the correlation entirely, +which strictly reduces the number of prompts eligible for cwd-based guessing. The hook also +stamps `lastSubmitAt`, so it finally means "a prompt was submitted" rather than "typed into +Codeman's web terminal". Conversations vouched for this way are persisted as a chain, whose +tail re-pins the conversation when a surviving tmux session is re-attached after a restart — +`start()` otherwise resets the id back to the launch conversation. Existing workspaces heal +on their next Claude spawn via the hooks staleness sweep. The hook's stdout is discarded with curl's own `-o /dev/null`: +Claude Code injects `UserPromptSubmit` output into the model's context, so an undiscarded +curl would paste the API envelope into the user's own prompt on every turn. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index e34e7cd54..72aa3c0a2 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -338,6 +338,8 @@ Anatomy: `.set-shell` → `.set-shell-head` (title + `.set-head-actions`) + `.se ⚠️ **A Claude pane's conversation is identified by the pane's own Enter, never by "newest entry for this cwd".** `~/.claude/history.jsonl` records every submitted prompt as `{project, sessionId, timestamp}`, and `/clear` moves the pane to a fresh `.jsonl` that nothing on the PTY announces — so the viewer has to re-derive the live conversation. Keying that off `project` alone was the bug: a cwd is shared with every other Codeman tab on it, with tabs long since closed, and with any plain `claude` the user runs in their own terminal, so the eye followed whichever of those conversations was typed into last and showed a stranger's transcript. `resolveActiveClaudeSessionIdFromHistory()` instead credits an entry to a pane only when it lands within `CLAUDE_SUBMIT_MATCH_MS` of that pane's `Session.lastSubmitAt` **and** no other pane on the same cwd submitted closer — the same last-submit correlation the Codex locator uses. With no correlated entry the pane keeps the id it has: a viewer one turn behind beats a viewer showing someone else's conversation. +⚠️ **A first-hand conversation id outranks every correlation, and the correlation must never run when one exists.** `UserPromptSubmit` and `Stop` hook payloads carry `session_id` — the pane's LIVE conversation, reported from inside the CLI process — and reach Codeman addressed by that pane's own `$CODEMAN_SESSION_ID`. That binding is a fact: it never consults `workingDir`, so it cannot be claimed by a sibling pane on the same folder, by a closed tab, or by a bare `claude` in the user's terminal. `Session.claudeSessionIdIsFirstHand` gates `resolveActiveClaudeSessionIdFromHistory()` at its first line, so the number of prompts eligible for cwd-based guessing goes DOWN, never up. There is no TTL: if hooks stop arriving the last hook-supplied id is kept forever rather than falling back to guessing, which is the same rule as the paragraph below. ⚠️ **This is also the only fix for a pane driven by attaching to its tmux session directly.** `lastSubmitAt` was bumped only by `Session.write()`/`writeViaMux()`, i.e. input that flows through Codeman, so such a pane's anchor stayed 0 and the resolver returned at `if (!submitAt)` for the pane's entire life. The hook stamps it too (`markPromptSubmitted()`), so it finally means "a prompt was submitted". ⚠️ **Only a first-hand adoption may extend `Session.claudeSessionChain`** — a correlated guess writing into the pane's permanent record is precisely the bug the paragraph above describes, made durable. The chain is persisted because `/clear` is otherwise unrecoverable: once the pane moves on, the predecessor id exists nowhere else. Its tail re-pins the conversation on a RESTORED mux attach, where the launch id is a lie (the CLI never stopped and may have `/clear`ed before the restart); a NEW pane has an empty chain and keeps the launch id unchanged. ⚠️ **The hook's stdout must stay discarded, using curl's own `-o /dev/null`** (`curlCmdSilent`): Claude Code injects a `UserPromptSubmit` hook's stdout into the model's context — the CLI's own hook reference says "Exit code 0 - stdout shown to Claude" — so an undiscarded curl pastes Codeman's `{"success":true,…}` envelope into the user's own prompt on every turn. ⚠️ **A trailing `>/dev/null` does NOT work and looks like it does**: `curlCmd` already ends `… 2>/dev/null || true`, and in `pipeline || true >/dev/null` the shell binds the redirection to `true`, which never runs on the success path. An `endsWith('>/dev/null')` assertion passes on exactly that broken form, so the test asserts the `-o` flag instead. The other events feed SSE, where their stdout is harmless — hence a separate builder rather than a change to `curlCmd`. Tests: `test/hooks-config.test.ts`, `test/routes/hook-event-routes.test.ts`, `test/routes/session-routes-claude-last-response.test.ts`. + ⚠️ **`Session.lastSubmitAt` is persisted state, not a runtime counter.** `start()` reassigns `_claudeSessionId = resumeSessionId || id` on every launch — including the re-attach path for a mux session that survived the restart — so a recovered pane always points the viewer at its *launch* conversation, even when the CLI moved on via `/clear` hours earlier. The submit anchor is the only thing that can correct that without user input, so it round-trips through `SessionState.lastSubmitAt` and is restored in `restoreMuxSessions()`. Drop it from `toState()` and recovered panes silently show the pre-`/clear` transcript until the user types again. Restoring a *stale* anchor is safe: the resolver's staleness guard rejects any candidate transcript older than the one the pane is currently on, which is exactly the shape of a respawn into a fresh conversation. ⚠️ **Claude transcripts are grouped at real human-turn boundaries, not per JSONL row.** A Claude transcript is an append-only event log, so one logical exchange spans many rows: tool-result rows, meta/image/skill rows, compact summaries, task/team notifications, sidechains, replayed assistant snapshots, and multi-block assistant output. Rendering a card per row was the bug: it produced duplicate and truncated cards that looked like the viewer had lost the response. The grouping walks to the next genuine user turn and dedups replayed assistant snapshots while preserving the tool/task/skill/compact/team metadata filtering. Related: a recovered `restored-` tmux placeholder carries a **stale cwd**, so transcript lookup by working directory finds nothing; it rebinds to the matching top-level Claude transcript UUID instead when that match is unambiguous. Tests: `test/routes/session-routes-claude-last-response.test.ts`. Purely client-side (no `renderIndexHtml` step): the template ships with `btn-response-viewer-header--hidden` and `applyHeaderVisibilitySettings()` (settings-ui.js) toggles it after settings load. Hiding must go through that marker class — the base rule is `display:inline-flex !important`, so an inline style can't override it. `showResponseViewer` is in the `displayKeys` per-device set (settings-ui.js), so it does NOT sync across devices. diff --git a/src/hooks-config.ts b/src/hooks-config.ts index 534993f0e..687ee36d3 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -366,19 +366,33 @@ export function generateHooksConfig(): { hooks: Record } { // never lands in this config and rotation needs no respawn. If the var/file is // missing the header is empty — the middleware then allows the request only on // the plain loopback bypass (tunnel down), same as pre-secret behavior. - const curlCmd = (event: HookEventType) => + const curlCmd = (event: HookEventType, options: { discardStdout?: boolean } = {}) => `HOOK_DATA=$(cat 2>/dev/null || echo '{}'); ` + `printf '{"event":"${event}","sessionId":"%s","data":%s}' "$CODEMAN_SESSION_ID" "$HOOK_DATA" | ` + // `-k`, same as the statusline exporter: CODEMAN_API_URL is loopback HTTPS with // a self-signed cert on --https/tailscale installs. Without it curl exits 60, // the `|| true` swallows it, and ALL SIX hook events die silently: respawn loses // its definitive idle signals and the wait endpoints lose stop/blocked. - `curl -sk -X POST "$CODEMAN_API_URL/api/hook-event" ` + + `curl -sk ${options.discardStdout ? '-o /dev/null ' : ''}-X POST "$CODEMAN_API_URL/api/hook-event" ` + `-H 'Content-Type: application/json' ` + `-H "X-Codeman-Hook-Secret: $(cat "$CODEMAN_HOOK_SECRET_FILE" 2>/dev/null)" ` + `--data @- ` + `2>/dev/null || true`; + // The same POST with stdout DISCARDED, via curl's own `-o`. UserPromptSubmit is + // one of the hook events whose stdout Claude Code injects into the model's + // context (the CLI's own hook reference: "Exit code 0 - stdout shown to + // Claude"), so an undiscarded curl pastes Codeman's `{"success":true,…}` + // envelope into the user's prompt on every single turn. + // ⚠️ It MUST be curl's flag, not a trailing redirect. `curlCmd` already ends + // `… 2>/dev/null || true`, and in `pipeline || true >/dev/null` the shell binds + // the redirection to `true` — which never runs on the success path — so the + // envelope still reaches stdout. Verified in dash and bash. + // ⚠️ The flag is opt-in so the other events' command text stays byte-identical: + // their stdout feeds the SSE stream harmlessly, and changing it would rewrite + // every workspace's settings file for no gain. + const curlCmdSilent = (event: HookEventType) => curlCmd(event, { discardStdout: true }); + return { hooks: { Notification: [ @@ -410,6 +424,16 @@ export function generateHooksConfig(): { hooks: Record } { hooks: [{ type: 'command', command: curlCmd('stop'), timeout: HOOK_TIMEOUT_SECONDS }], }, ], + // The pane's LIVE conversation id, reported by the CLI process itself. + // Without it the response viewer has to guess which `.jsonl` a pane + // is on after a `/clear`, and the only anchor it can guess from is an + // Enter that went THROUGH Codeman — so a user who attaches to tmux + // directly never gets one and stays pinned to the launch conversation. + UserPromptSubmit: [ + { + hooks: [{ type: 'command', command: curlCmdSilent('prompt_submitted'), timeout: HOOK_TIMEOUT_SECONDS }], + }, + ], SubagentStop: [ { hooks: [ @@ -735,9 +759,22 @@ export async function refreshStaleCodemanHooks(casePath: string): Promise // Approvals Inbox needs the elicitation_complete/elicitation_response // matchers; their absence marks a pre-inbox hooks block. const hasElicitationComplete = hooksJson.includes('elicitation_complete'); + // The UserPromptSubmit event is what gives a tmux-driven pane a first-hand + // conversation id; its absence marks a pre-prompt_submitted hooks block. + // ⚠️ No surrounding quotes: `hooksJson` is JSON.stringify'd, so the marker + // inside the command reads \"prompt_submitted\" and a quoted needle never + // matches — which would make this gate permanently false and rewrite every + // workspace's settings file on every Claude spawn. The sibling markers are + // quote-free for the same reason. + const hasPromptSubmit = hooksJson.includes('prompt_submitted'); if ( !isOurs || - (hasSecret && hasBackgroundWake && hasSubagentStopGuard && hasElicitationComplete && !hasTlsFlaglessCurl) + (hasSecret && + hasBackgroundWake && + hasSubagentStopGuard && + hasElicitationComplete && + hasPromptSubmit && + !hasTlsFlaglessCurl) ) return; const generated = generateHooksConfig(); diff --git a/src/session.ts b/src/session.ts index 8a0608aa5..f7c3085b5 100644 --- a/src/session.ts +++ b/src/session.ts @@ -152,6 +152,11 @@ const WIRE_ACTIVITY_SETTLE_MS = 15_000; /** Graceful shutdown delay when stopping session (100ms) */ const GRACEFUL_SHUTDOWN_DELAY_MS = 100; +// Conversations kept in a pane's chain. A pane that /clears repeatedly would +// otherwise grow state.json without bound; 32 covers any real session's history +// and the oldest entries are the ones whose transcripts Claude Code has pruned. +const MAX_CLAUDE_SESSION_CHAIN = 32; + // Filter out terminal focus escape sequences (focus in/out reports) // ^[[I (focus in), ^[[O (focus out), and the enable/disable sequences // eslint-disable-next-line no-control-regex @@ -427,6 +432,17 @@ export class Session extends EventEmitter { private _wireActivityAt: number; private _wireActivitySettleUntil: number; private _claudeSessionId: string | null = null; + // Set only when the id came from the CLI's own UserPromptSubmit/Stop hook + // payload, keyed on this pane's $CODEMAN_SESSION_ID. That binding is a fact, + // not a correlation: it never consults cwd, so a sibling pane on the same + // folder cannot steal it. Runtime-only — a restart must re-earn it from the + // next hook rather than trust a persisted claim. + private _claudeSessionIdIsFirstHand = false; + // Conversations this pane has been on, oldest first, current last. Grows only + // through a first-hand adoption, so it can never splice in a foreign + // conversation. Persisted, because `/clear` is otherwise unrecoverable: the + // predecessor id exists nowhere else once the pane moves on. + private _claudeSessionChain: string[] = []; private _totalCost: number = 0; private _messages: ClaudeMessage[] = []; private _lineBuffer: string = ''; @@ -648,6 +664,8 @@ export class Session extends EventEmitter { attachmentHistory?: SessionAttachmentHistoryItem[]; /** Restored wall-clock ms of the pane's last Enter (see `lastSubmitAt`). */ lastSubmitAt?: number; + /** Restored conversation chain, oldest first (see `claudeSessionChain`). */ + claudeSessionChain?: string[]; /** Restored wall-clock ms of the pane's last output (recovery only; see `_wireActivityAt`). */ lastActivityAt?: number; /** Remote execution metadata for sessions launched through SSH inside local tmux. */ @@ -704,6 +722,13 @@ export class Session extends EventEmitter { // response viewer re-derive the live conversation without waiting for the // user to type again. this._lastSubmitAt = config.lastSubmitAt ?? 0; + // Restored chain: its tail is the conversation the CLI was actually on when + // the server stopped, which outranks the launch id seeded just above. The + // FIRST-HAND flag is deliberately NOT restored — a persisted claim is not a + // fact, so the pane re-earns the guess-free path from its next hook. + this._claudeSessionChain = Array.isArray(config.claudeSessionChain) ? [...config.claudeSessionChain] : []; + const restoredConversation = this._claudeSessionChain[this._claudeSessionChain.length - 1]; + if (restoredConversation) this._claudeSessionId = restoredConversation; this._mux = config.mux || null; this._useMux = config.useMux ?? (this._mux !== null && this._mux.isAvailable()); this._muxSession = config.muxSession || null; @@ -902,6 +927,20 @@ export class Session extends EventEmitter { return this._claudeSessionId; } + /** + * True when `claudeSessionId` came from the CLI's own hook payload rather than + * from the launch config or a history correlation. The response viewer uses it + * to skip guessing entirely — see resolveActiveClaudeSessionIdFromHistory(). + */ + get claudeSessionIdIsFirstHand(): boolean { + return this._claudeSessionIdIsFirstHand; + } + + /** Conversations this pane has been on, oldest first, current last. */ + get claudeSessionChain(): readonly string[] { + return this._claudeSessionChain; + } + /** Docker execution metadata when this session runs inside a container, else undefined. */ get docker(): SessionDocker | undefined { return this._docker; @@ -959,11 +998,38 @@ export class Session extends EventEmitter { // payload). In interactive PTY mode Claude CLI emits no JSON to stdout, so // `_handleJsonMessage` never sees `session_id`; hooks are the only signal // that conveys a post-/clear conversation switch. - adoptClaudeSessionId(newId: string): void { - if (!newId || newId === this._claudeSessionId) return; + // + // `firstHand` marks an id that came from the CLI process itself — a hook + // payload whose delivery was keyed on this pane's $CODEMAN_SESSION_ID. Only + // those extend the chain: a history-correlated guess must never be able to + // write a foreign conversation into this pane's permanent record. + adoptClaudeSessionId(newId: string, options: { firstHand?: boolean } = {}): void { + if (!newId) return; + if (options.firstHand) { + this._claudeSessionIdIsFirstHand = true; + this._recordClaudeSessionInChain(newId); + } + if (newId === this._claudeSessionId) return; this._claudeSessionId = newId; } + /** + * Append to the conversation chain, oldest first. A repeat of the current tail + * is a no-op (every prompt in a conversation reports the same id), and an id + * already in the chain moves to the tail rather than duplicating, which is + * what a `/resume` back to an earlier conversation does. + */ + private _recordClaudeSessionInChain(id: string): void { + if (this._claudeSessionChain[this._claudeSessionChain.length - 1] === id) return; + const existing = this._claudeSessionChain.indexOf(id); + if (existing !== -1) this._claudeSessionChain.splice(existing, 1); + this._claudeSessionChain.push(id); + // A pane that /clears in a loop must not grow this without bound. + if (this._claudeSessionChain.length > MAX_CLAUDE_SESSION_CHAIN) { + this._claudeSessionChain.splice(0, this._claudeSessionChain.length - MAX_CLAUDE_SESSION_CHAIN); + } + } + /** The tmux session name, if the session is running inside a mux */ get muxName(): string | null { return this._muxSession?.muxName ?? null; @@ -1397,6 +1463,12 @@ export class Session extends EventEmitter { respawnBlocked: this._respawnBlocked || undefined, attachmentHistory: this.attachmentHistory.length > 0 ? this.attachmentHistory : undefined, lastSubmitAt: this._lastSubmitAt || undefined, + // Only a chain the CLI's own hooks vouched for is persisted, and only when + // the pane actually moved conversation. Its LAST entry is the live one, so + // it is also what restores `claudeSessionId` across a restart — `start()` + // resets that field to the launch id at three separate points, which is + // why a recovered pane otherwise shows its pre-/clear transcript forever. + claudeSessionChain: this._claudeSessionChain.length > 0 ? [...this._claudeSessionChain] : undefined, // envOverrides intentionally NOT on the public SessionState type — they must not // leak into SSE / GET /api/sessions broadcasts (schema allows OPENCODE_*, which // can carry secrets). For disk persistence, session-manager calls @@ -1931,6 +2003,11 @@ export class Session extends EventEmitter { }, REMOTE_CLI_VERSION_PROBE_DELAY_MS); } + // ⚠️ Hoisted, because the "third reset point" below runs unconditionally + // AFTER the mux branch and would otherwise stomp the restored conversation + // straight back to the launch id. + let restoredConversation: string | undefined; + // If mux wrapping is enabled, create or attach to a mux session if (this._useMux && this._mux) { try { @@ -1972,7 +2049,15 @@ export class Session extends EventEmitter { // over the generic `this.id` fallback, or this line clobbers it back // to the Codeman id // on every single respawn. - this._claudeSessionId = this._resumeSessionId || this._ompConfig?.resumeSessionId || this.id; + // ⚠️ A RESTORED mux session is the one case where the launch id is a + // lie: the CLI never stopped, so a `/clear` before the Codeman restart + // already moved it to a conversation `this.id` knows nothing about. The + // persisted chain's tail is that conversation, reported first-hand by + // the CLI's own hook, so it outranks the fallback here. A NEW pane has + // an empty chain and falls through to exactly today's expression. + restoredConversation = isRestored ? this._claudeSessionChain[this._claudeSessionChain.length - 1] : undefined; + this._claudeSessionId = + restoredConversation || this._resumeSessionId || this._ompConfig?.resumeSessionId || this.id; // For NEW mux sessions: wait for readiness then clean buffer // For RESTORED mux sessions: don't do anything - client will fetch buffer on tab switch @@ -2094,8 +2179,13 @@ export class Session extends EventEmitter { // unconditionally after both the mux and direct-PTY paths, so it also needs // the ompConfig fallback or it stomps the mux branch's correctly-resolved // OMP alias back to this.id on every mux/plain-reattach boot recovery - // (the "third reset point" — see DECISIONS.md). - this._claudeSessionId = this._resumeSessionId || this._ompConfig?.resumeSessionId || this.id; + // (the "third reset point" — see DECISIONS.md). For the same reason it needs + // `restoredConversation`: on a RESTORED mux attach the CLI never stopped and + // may have `/clear`ed before the restart, so the launch id is a lie and the + // chain's tail is the live conversation. Empty on every other path, which + // leaves this expression exactly as it was. + this._claudeSessionId = + restoredConversation || this._resumeSessionId || this._ompConfig?.resumeSessionId || this.id; this._pid = this.ptyProcess.pid; console.log('[Session] Interactive PTY spawned with PID:', this._pid); @@ -3185,6 +3275,16 @@ export class Session extends EventEmitter { } } + /** + * A prompt was submitted, reported by the CLI's own UserPromptSubmit hook. + * `_trackSubmit` only sees input that flows through Codeman's write path, so + * a pane the user drives by attaching to tmux directly never stamped this and + * `lastSubmitAt` stayed 0 for its whole life. + */ + markPromptSubmitted(): void { + this._lastSubmitAt = Date.now(); + } + /** * Per-client highest-applied input sequence, for exactly-once input delivery. * Keyed by the web client's stable `clientId`. Bounded so many devices over a diff --git a/src/types/api.ts b/src/types/api.ts index b63268af2..e90cb265f 100644 --- a/src/types/api.ts +++ b/src/types/api.ts @@ -110,6 +110,10 @@ export type HookEventType = | 'stop' | 'teammate_idle' | 'task_completed' + // Claude Code's UserPromptSubmit. The payload's `session_id` is the pane's + // LIVE conversation id, reported by the CLI process itself, so it survives a + // `/clear` without any cwd/timestamp correlation. + | 'prompt_submitted' // No Claude Code hook behind this one: it is the DeepSeek status bridge's // "a turn STARTED" report (see deepseek-status-shim.ts). Keep in step with // HookEventSchema in web/schemas.ts. diff --git a/src/types/session.ts b/src/types/session.ts index 3c34b49d4..d415ff9fb 100644 --- a/src/types/session.ts +++ b/src/types/session.ts @@ -648,6 +648,15 @@ export interface SessionState { * again until the pane's own Enter is known. */ lastSubmitAt?: number; + /** + * Claude conversations this pane has been on, oldest first, current last. + * Written ONLY from a first-hand `UserPromptSubmit`/`Stop` hook payload — + * never from the history correlation — so it cannot record a sibling pane's + * conversation. Persisted because `/clear` is otherwise unrecoverable: once + * the pane moves on, the predecessor id exists nowhere else, and the last + * entry is what re-pins `claudeSessionId` past `start()`'s three resets. + */ + claudeSessionChain?: string[]; /** * PTY-exit circuit breaker tripped — respawn blocked until an explicit restart * (COD-118). Runtime-only: never restored on boot (fresh server = fresh breaker). diff --git a/src/web/route-helpers.ts b/src/web/route-helpers.ts index fb02fd613..0c676bffb 100644 --- a/src/web/route-helpers.ts +++ b/src/web/route-helpers.ts @@ -422,6 +422,11 @@ export function sanitizeHookData(data: Record | null | undefine 'stop_hook_active', 'transcript_path', 'message', + // UserPromptSubmit identity fields. `prompt` is deliberately NOT here: the + // prompt text would land in the SSE broadcast, and Read My Mind already + // captures intent through transcript-watcher. + 'prompt_id', + 'source', ]; for (const key of allowedKeys) { diff --git a/src/web/routes/hook-event-routes.ts b/src/web/routes/hook-event-routes.ts index aa4f30e6b..b69e4129f 100644 --- a/src/web/routes/hook-event-routes.ts +++ b/src/web/routes/hook-event-routes.ts @@ -129,7 +129,29 @@ export function registerHookEventRoutes( if (data && typeof data.session_id === 'string' && data.session_id) { const session = ctx.sessions.get(sessionId); const prevClaudeSessionId = session?.claudeSessionId; - session?.adoptClaudeSessionId(data.session_id); + const prevChainLength = session?.claudeSessionChain.length ?? 0; + // FIRST-HAND: this payload came from the CLI process itself and reached us + // because the pane's own $CODEMAN_SESSION_ID addressed it. No cwd, no + // timestamp, nothing a sibling pane on the same folder could win — so the + // response viewer can stop guessing entirely (see + // resolveActiveClaudeSessionIdFromHistory). + session?.adoptClaudeSessionId(data.session_id, { firstHand: true }); + if (event === 'prompt_submitted') { + // Repairs `lastSubmitAt` for a pane driven straight from tmux: it was + // bumped only by input that flowed through Codeman's own write path, so + // it read 0 forever for those panes and every consumer of "when did this + // pane last submit" silently degraded. + session?.markPromptSubmitted(); + } + // Persist when the conversation actually moved: `/clear` emits no + // completion event, so without this the successor id is lost on restart + // and recovery falls back to the launch conversation. + if ( + session && + (session.claudeSessionId !== prevClaudeSessionId || session.claudeSessionChain.length !== prevChainLength) + ) { + ctx.persistSessionState(session); + } // Docker sessions: keep the case's resume seed following the LIVE // conversation (post-/clear id switches), so a container stop/reboot // relaunch resumes the right transcript. diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 9c9e7b408..8af3a8a4c 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1860,8 +1860,17 @@ export function registerSessionRoutes( session: Session, projectsDir: string ): Promise { + // A pane whose conversation id came from its OWN hook needs no correlation: + // $CODEMAN_SESSION_ID (the pane's env) -> data.session_id (the CLI's own + // stdin JSON) is a first-hand binding that never looks at cwd, so it cannot + // be stolen by a sibling pane, a closed tab, or a bare `claude` in a + // terminal. Guessing can only be worse than the fact. This is also what + // closes the hole below for a pane driven straight from tmux: it never + // reaches `if (!submitAt)`. + if (session.claudeSessionIdIsFirstHand) return null; + const submitAt = session.lastSubmitAt; - if (!submitAt) return null; // never typed through Codeman — nothing to credit + if (!submitAt) return null; // no anchor at all — nothing to credit const cached = claudeHistoryPinCache.get(session.id); if (cached && cached.submitAt === submitAt) return cached.claudeSessionId; diff --git a/src/web/schemas.ts b/src/web/schemas.ts index a78b9defb..003dd5065 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -924,6 +924,9 @@ export const HookEventSchema = z.object({ 'stop', 'teammate_idle', 'task_completed', + // Claude Code's UserPromptSubmit: a first-hand report of the pane's live + // conversation id. Keep in step with HookEventType in types/api.ts. + 'prompt_submitted', // A turn STARTED. Unlike the others this one has no Claude Code hook behind // it: it is reported by the DeepSeek Harness status shim, and exists so a // dialog answered in the terminal resolves its Approvals Inbox item at once diff --git a/src/web/server.ts b/src/web/server.ts index 0cf318d65..2272aef56 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -2794,6 +2794,10 @@ export class WebServer extends EventEmitter { // the launch conversation until the user types again, even though // the re-attached CLI is on a post-`/clear` one. lastSubmitAt: savedState?.lastSubmitAt, + // Conversations this pane provably owned, oldest first. Its tail is + // the conversation the CLI was on when the server stopped, which is + // what a re-attach must point the viewer at instead of the launch id. + claudeSessionChain: savedState?.claudeSessionChain, // The pane's last output, previous run's value. Without it every // restart restamped all sessions "now" (constructor + the attach // repaint within the same second), flattening the home screens' diff --git a/test/hooks-config.test.ts b/test/hooks-config.test.ts index 9239bcbd6..774774b3e 100644 --- a/test/hooks-config.test.ts +++ b/test/hooks-config.test.ts @@ -42,6 +42,34 @@ describe('generateHooksConfig', () => { expect(config.hooks.Stop).toHaveLength(1); }); + it('reports the live conversation id on every prompt, with stdout discarded', () => { + const config = generateHooksConfig(); + expect(config.hooks.UserPromptSubmit).toBeInstanceOf(Array); + expect(config.hooks.UserPromptSubmit).toHaveLength(1); + const command = (config.hooks.UserPromptSubmit as Array<{ hooks: Array<{ command: string }> }>)[0].hooks[0].command; + expect(command).toContain('"event":"prompt_submitted"'); + expect(command).toContain('$CODEMAN_SESSION_ID'); + // ⚠️ Claude Code injects a UserPromptSubmit hook's stdout into the model's + // context ("Exit code 0 - stdout shown to Claude"), so without this the API + // envelope is pasted into the user's own prompt on every turn. Every other + // event's stdout is harmless (it feeds SSE). + // ⚠️ Assert curl's OWN flag, not a trailing redirect: the command already + // ends `… 2>/dev/null || true`, and in `pipeline || true >/dev/null` the + // shell binds the redirect to `true`, which never runs on the success path. + // A `endsWith('>/dev/null')` assertion passes on exactly that broken form. + expect(command).toContain('curl -sk -o /dev/null -X POST'); + expect(command.trimEnd().endsWith('>/dev/null')).toBe(false); + }); + + it("leaves every other hook event's command text byte-identical", () => { + // The opt-in discard exists so the five SSE-fed events do not change shape: + // rewriting their command churns every workspace's settings.local.json. + const config = generateHooksConfig(); + const stop = (config.hooks.Stop as Array<{ hooks: Array<{ command: string }> }>)[0].hooks[0].command; + expect(stop).toContain('curl -sk -X POST'); + expect(stop).not.toContain('-o /dev/null'); + }); + it('should guard subagent stops while their background work is active', () => { const config = generateHooksConfig(); const subagentHooks = config.hooks.SubagentStop as Array<{ @@ -324,6 +352,39 @@ describe('writeHooksConfig', () => { expect(serialized).not.toContain('CODEMAN_BACKGROUND_REWAKE_V1'); }); + it('heals a hooks block written before UserPromptSubmit existed', async () => { + const claudeDir = join(testDir, '.claude'); + const settingsPath = join(claudeDir, 'settings.local.json'); + mkdirSync(claudeDir, { recursive: true }); + // An otherwise-current block from the previous release: the pane would keep + // guessing its conversation from ~/.claude/history.jsonl forever. + const hooks = generateHooksConfig().hooks; + delete hooks.UserPromptSubmit; + writeFileSync(settingsPath, JSON.stringify({ hooks }, null, 2)); + + await refreshStaleCodemanHooks(testDir); + + const parsed = JSON.parse(readFileSync(settingsPath, 'utf-8')); + expect(JSON.stringify(parsed.hooks.UserPromptSubmit)).toContain('prompt_submitted'); + }); + + it('leaves an already-current hooks block untouched', async () => { + // ⚠️ The staleness gate reads a JSON.stringify'd blob, so a marker written + // with surrounding quotes never matches and the gate is permanently false — + // which rewrites every workspace's settings.local.json on every Claude + // spawn instead of never. This asserts the no-op, which is the property a + // quoted needle silently breaks. + const claudeDir = join(testDir, '.claude'); + const settingsPath = join(claudeDir, 'settings.local.json'); + mkdirSync(claudeDir, { recursive: true }); + writeFileSync(settingsPath, JSON.stringify({ hooks: generateHooksConfig().hooks }, null, 2)); + const before = readFileSync(settingsPath, 'utf-8'); + + await refreshStaleCodemanHooks(testDir); + + expect(readFileSync(settingsPath, 'utf-8')).toBe(before); + }); + it('replaces the V2 background hook without duplicating it', async () => { const claudeDir = join(testDir, '.claude'); const settingsPath = join(claudeDir, 'settings.local.json'); diff --git a/test/mocks/mock-session.ts b/test/mocks/mock-session.ts index 18990a2e4..b43b2fcde 100644 --- a/test/mocks/mock-session.ts +++ b/test/mocks/mock-session.ts @@ -29,6 +29,32 @@ export class MockSession extends EventEmitter { terminalBuffer: string = ''; /** Mirrors Session.lastSubmitAt — the response viewer credits history entries by it. */ lastSubmitAt: number = 0; + /** Mirrors Session.claudeSessionId — the conversation the viewer reads. */ + claudeSessionId: string | null = null; + /** Mirrors Session.claudeSessionIdIsFirstHand — set only by a hook adoption. */ + claudeSessionIdIsFirstHand: boolean = false; + /** Mirrors Session.claudeSessionChain — oldest first, current last. */ + claudeSessionChain: string[] = []; + + /** Mirrors Session.adoptClaudeSessionId, including the first-hand chain rule. */ + adoptClaudeSessionId(newId: string, options: { firstHand?: boolean } = {}): void { + if (!newId) return; + if (options.firstHand) { + this.claudeSessionIdIsFirstHand = true; + if (this.claudeSessionChain[this.claudeSessionChain.length - 1] !== newId) { + const existing = this.claudeSessionChain.indexOf(newId); + if (existing !== -1) this.claudeSessionChain.splice(existing, 1); + this.claudeSessionChain.push(newId); + } + } + if (newId === this.claudeSessionId) return; + this.claudeSessionId = newId; + } + + /** Mirrors Session.markPromptSubmitted. */ + markPromptSubmitted(): void { + this.lastSubmitAt = Date.now(); + } private _muxName: string | null = null; diff --git a/test/routes/hook-event-routes.test.ts b/test/routes/hook-event-routes.test.ts index 279d2c3a8..a553ea0c8 100644 --- a/test/routes/hook-event-routes.test.ts +++ b/test/routes/hook-event-routes.test.ts @@ -115,6 +115,75 @@ describe('hook-event-routes', () => { ); }); + /** + * The pane's live conversation id, reported by the CLI process itself. This + * is what lets the response viewer stop guessing from ~/.claude/history.jsonl + * — a guess that could never run at all for a pane the user drives by + * attaching to tmux, because `lastSubmitAt` only ever saw Codeman's own + * write path. + */ + it('adopts the conversation id first-hand from a prompt_submitted hook', async () => { + const session = harness.ctx._session; + const before = session.lastSubmitAt; + + const res = await harness.app.inject({ + method: 'POST', + url: '/api/hook-event', + payload: { + event: 'prompt_submitted', + sessionId: harness.ctx._sessionId, + data: { hook_event_name: 'UserPromptSubmit', session_id: 'conv-1', source: 'user' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(session.claudeSessionId).toBe('conv-1'); + expect(session.claudeSessionIdIsFirstHand).toBe(true); + expect(session.claudeSessionChain).toEqual(['conv-1']); + expect(session.lastSubmitAt).toBeGreaterThan(before); + expect(harness.ctx.persistSessionState).toHaveBeenCalledWith(session); + }); + + it('records a /clear successor in the chain and persists it, without duplicating a repeat', async () => { + const session = harness.ctx._session; + const submit = async (conversationId: string) => + harness.app.inject({ + method: 'POST', + url: '/api/hook-event', + payload: { + event: 'prompt_submitted', + sessionId: harness.ctx._sessionId, + data: { hook_event_name: 'UserPromptSubmit', session_id: conversationId }, + }, + }); + + await submit('conv-1'); + await submit('conv-1'); // every prompt in a conversation reports the same id + await submit('conv-2'); // the user ran /clear + + expect(session.claudeSessionChain).toEqual(['conv-1', 'conv-2']); + expect(session.claudeSessionId).toBe('conv-2'); + // `/clear` emits no completion event, so the successor is lost on restart + // unless the hook itself persists it. + expect(harness.ctx.persistSessionState).toHaveBeenCalledTimes(2); + }); + + it('does not leak the prompt text into the broadcast', async () => { + await harness.app.inject({ + method: 'POST', + url: '/api/hook-event', + payload: { + event: 'prompt_submitted', + sessionId: harness.ctx._sessionId, + data: { hook_event_name: 'UserPromptSubmit', session_id: 'conv-1', prompt: 'my secret prompt' }, + }, + }); + + const broadcast = JSON.stringify(harness.ctx.broadcast.mock.calls); + expect(broadcast).not.toContain('my secret prompt'); + expect(broadcast).toContain('conv-1'); + }); + it('returns 404 for unknown session', async () => { const res = await harness.app.inject({ method: 'POST', diff --git a/test/routes/session-routes-claude-last-response.test.ts b/test/routes/session-routes-claude-last-response.test.ts index f9cd8bc24..7ac29f7ca 100644 --- a/test/routes/session-routes-claude-last-response.test.ts +++ b/test/routes/session-routes-claude-last-response.test.ts @@ -232,16 +232,18 @@ describe('GET /api/sessions/:id/last-response (claude conversation pinning)', () } /** Replaces the pre-seeded mock session with a Claude pane in WORKDIR. */ - function addPane(id: string, conversationId: string, lastSubmitAt: number) { + function addPane(id: string, conversationId: string, lastSubmitAt: number, firstHand = false) { const base = harness.ctx._session; const pane = Object.create(Object.getPrototypeOf(base)) as typeof base & { claudeSessionId: string; lastSubmitAt: number; + claudeSessionIdIsFirstHand: boolean; adoptClaudeSessionId: ReturnType; }; Object.assign(pane, base, { id, mode: 'claude', workingDir: WORKDIR, docker: undefined }); pane.claudeSessionId = conversationId; pane.lastSubmitAt = lastSubmitAt; + pane.claudeSessionIdIsFirstHand = firstHand; pane.adoptClaudeSessionId = vi.fn((newId: string) => { pane.claudeSessionId = newId; }); @@ -286,6 +288,41 @@ describe('GET /api/sessions/:id/last-response (claude conversation pinning)', () expect(pane.adoptClaudeSessionId).not.toHaveBeenCalled(); }); + /** + * The whole point of the UserPromptSubmit hook: a pane driven by attaching to + * tmux directly never bumps `lastSubmitAt` (only Codeman's own write path + * does), so before this the correlation could not run at all for it and the + * viewer stayed pinned to the launch conversation for the pane's whole life. + */ + it("trusts the pane's own hook over any history correlation", async () => { + const pane = addPane('pane-1', 'hook-conversation', 0, true); + writeTranscript('hook-conversation', 'the answer this pane gave', NOW - 60_000); + // A newer, closer entry that the correlation would otherwise have claimed. + writeTranscript('decoy-conversation', 'a stranger answer', NOW); + writeHistory([{ sessionId: 'decoy-conversation', timestamp: NOW }]); + + expect(await getLastResponse('pane-1')).toEqual({ + text: 'the answer this pane gave', + timestamp: expect.any(String), + }); + expect(pane.adoptClaudeSessionId).not.toHaveBeenCalled(); + }); + + it('never lets a correlation override a first-hand id, even a well-anchored one', async () => { + // Same shape as the /clear-following test above, which DOES adopt — the only + // difference is that this pane's id came from its own hook. + const pane = addPane('pane-1', 'before-clear', NOW, true); + writeTranscript('before-clear', 'answer before clear', NOW - 60_000); + writeTranscript('after-clear', 'answer after clear', NOW + 500); + writeHistory([{ sessionId: 'after-clear', timestamp: NOW + 120 }]); + + expect(await getLastResponse('pane-1')).toEqual({ + text: 'answer before clear', + timestamp: expect.any(String), + }); + expect(pane.adoptClaudeSessionId).not.toHaveBeenCalled(); + }); + it('credits a shared-cwd entry to the pane whose Enter is closest to it', async () => { const near = addPane('pane-near', 'near-conversation', NOW); const far = addPane('pane-far', 'far-conversation', NOW - 4_000); diff --git a/test/session-claude-conversation-chain.test.ts b/test/session-claude-conversation-chain.test.ts new file mode 100644 index 000000000..49d000609 --- /dev/null +++ b/test/session-claude-conversation-chain.test.ts @@ -0,0 +1,118 @@ +/** + * @fileoverview Session.claudeSessionChain — the record of which Claude + * conversations a pane has actually been on. + * + * Which conversation the response viewer reads is `Session.claudeSessionId`, + * and `start()` reassigns it to the launch id at THREE separate points. That is + * correct for a fresh pane and a lie for a re-attached one: a mux session that + * survived a Codeman restart never stopped, so the CLI may have `/clear`ed hours + * ago and moved to a conversation the launch id knows nothing about. The chain + * is what carries that across the restart, and its tail must therefore outrank + * the launch id on the restored path only. + * + * Two properties are pinned here because both were broken in ways nothing else + * caught: + * + * 1. **Only a first-hand adoption extends the chain.** The id has to come from + * the CLI's own hook payload, delivered under the pane's `$CODEMAN_SESSION_ID`. + * A history-correlated guess writing into this record would make the + * "showed a stranger's conversation" bug permanent instead of transient. + * 2. **A restored conversation survives every reset point.** The mux branch and + * the unconditional "third reset point" after it both reassign the field, so + * patching only the first leaves the restore silently undone. + * + * Port: N/A + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { Session } from '../src/session.js'; + +describe('Session claude conversation chain', () => { + it('extends the chain only for a first-hand adoption', () => { + const session = new Session({ workingDir: '/tmp', mode: 'claude' }); + + // A correlated guess: adopted for display, but never recorded. + session.adoptClaudeSessionId('guessed-conversation'); + expect(session.claudeSessionId).toBe('guessed-conversation'); + expect(session.claudeSessionChain).toEqual([]); + expect(session.claudeSessionIdIsFirstHand).toBe(false); + + // The CLI's own hook: recorded. + session.adoptClaudeSessionId('hook-conversation', { firstHand: true }); + expect(session.claudeSessionChain).toEqual(['hook-conversation']); + expect(session.claudeSessionIdIsFirstHand).toBe(true); + }); + + it('records a /clear successor once, however many prompts report it', () => { + const session = new Session({ workingDir: '/tmp', mode: 'claude' }); + + session.adoptClaudeSessionId('conv-1', { firstHand: true }); + session.adoptClaudeSessionId('conv-1', { firstHand: true }); // every prompt reports the same id + session.adoptClaudeSessionId('conv-2', { firstHand: true }); // the user ran /clear + + expect(session.claudeSessionChain).toEqual(['conv-1', 'conv-2']); + expect(session.claudeSessionId).toBe('conv-2'); + }); + + it('moves a resumed conversation to the tail instead of duplicating it', () => { + const session = new Session({ workingDir: '/tmp', mode: 'claude' }); + + session.adoptClaudeSessionId('conv-1', { firstHand: true }); + session.adoptClaudeSessionId('conv-2', { firstHand: true }); + session.adoptClaudeSessionId('conv-1', { firstHand: true }); // /resume back + + expect(session.claudeSessionChain).toEqual(['conv-2', 'conv-1']); + }); + + it('round-trips the chain through toState and re-pins the conversation on restore', () => { + const original = new Session({ workingDir: '/tmp', mode: 'claude' }); + original.adoptClaudeSessionId('conv-1', { firstHand: true }); + original.adoptClaudeSessionId('conv-2', { firstHand: true }); + + const state = original.toState() as { claudeSessionChain?: string[] }; + expect(state.claudeSessionChain).toEqual(['conv-1', 'conv-2']); + + // Boot recovery rebuilds the pane from that state. The launch id would point + // the viewer at the pre-/clear conversation; the chain's tail corrects it. + const restored = new Session({ + workingDir: '/tmp', + mode: 'claude', + id: original.id, + claudeSessionChain: state.claudeSessionChain, + }); + expect(restored.claudeSessionId).toBe('conv-2'); + // ⚠️ NOT restored: a persisted claim is not a fact. The pane re-earns the + // guess-free path from its next hook. + expect(restored.claudeSessionIdIsFirstHand).toBe(false); + }); + + it('omits the chain from toState when the pane never moved conversation', () => { + const session = new Session({ workingDir: '/tmp', mode: 'claude' }); + expect((session.toState() as { claudeSessionChain?: string[] }).claudeSessionChain).toBeUndefined(); + }); + + it('applies the restored conversation at EVERY reset point in start()', () => { + // ⚠️ Structural pin, not a behavioural one: exercising start() needs a real + // PTY and mux. start() reassigns _claudeSessionId at three points, and the + // last one runs unconditionally AFTER the mux branch — so patching only the + // mux branch leaves the restore silently undone, which is what shipped + // before this existed. Every assignment built from the launch-id fallback + // must therefore carry `restoredConversation` first. + const source = readFileSync(resolve(import.meta.dirname, '../src/session.ts'), 'utf8'); + const fallbackAssignments = source.match( + /_claudeSessionId =\s*\n?\s*[^;]*?_resumeSessionId \|\| this\._ompConfig\?\.resumeSessionId \|\| this\.id;/g + ); + expect(fallbackAssignments).not.toBeNull(); + expect(fallbackAssignments!.length).toBeGreaterThanOrEqual(2); + for (const assignment of fallbackAssignments!) { + expect(assignment).toContain('restoredConversation ||'); + } + }); + + it('leaves a fresh pane on its launch id', () => { + const session = new Session({ workingDir: '/tmp', mode: 'claude' }); + expect(session.claudeSessionId).toBe(session.id); + expect(session.claudeSessionChain).toEqual([]); + }); +});