mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
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 (Session.write / writeViaMux). A user who attaches to the pane's tmux session directly never set it, so resolveActiveClaudeSessionIdFromHistory() 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 UserPromptSubmit hook reports the live conversation id from inside the CLI process, delivered under the pane's own $CODEMAN_SESSION_ID. That binding is a fact rather than a correlation: it never consults workingDir, so it cannot be claimed by a sibling pane on the same folder, a closed tab, or a bare `claude` in the user's terminal. A pane holding such an id skips the correlation entirely, so the number of prompts eligible for cwd-based guessing goes DOWN, never up — the naive alternative (relax the guard, or synthesize an anchor from PTY activity) is the reverted bug the resolver's own comment describes. The hook also stamps lastSubmitAt, so it finally means "a prompt was submitted" rather than "typed into Codeman's web terminal". Conversations vouched for first-hand — and only those — extend a persisted claudeSessionChain, whose tail re-pins the conversation when a surviving tmux session is re-attached after a restart. ⚠️ start() resets the id at THREE points and the last one runs unconditionally after the mux branch, so the tail is applied there too; patching only the mux branch looks right and silently does nothing. ⚠️ The hook's stdout is discarded with curl's own -o /dev/null. Claude Code injects a UserPromptSubmit hook's stdout into the model's context ("Exit code 0 - stdout shown to Claude"), and a trailing >/dev/null does NOT work: 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. The discard is opt-in so the five SSE-fed events keep byte-identical command text and no workspace's settings file is rewritten for them. The staleness marker is quote-free for the matching reason: hooksJson is JSON.stringify'd, so a quoted needle never matches and the gate would rewrite every workspace on every spawn. Existing workspaces heal on their next Claude spawn through the staleness sweep.
This commit is contained in:
shenlvkang-collab
@@ -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');
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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<typeof vi.fn>;
|
||||
};
|
||||
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);
|
||||
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user