From 7440e0d1a136d301cbd95df67d6bac0c087e2193 Mon Sep 17 00:00:00 2001 From: Test User Date: Sat, 27 Jun 2026 15:21:01 +0800 Subject: [PATCH] fix(auto-recall): thread parsed STDIN through the hook-dispatch path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit autoRecall() read process.stdin itself (via readStdin). The hook dispatcher (hook-dispatch-cli.ts) already reads STDIN once, parses it, and fans the parsed object out to every handler — so by the time the auto-recall handler ran, process.stdin was drained and autoRecall() saw "no STDIN data" and returned immediately. The PostToolUse hooks that ship today all go through `teamai hook-dispatch` (legacy `teamai auto-recall --stdin` entries are removed by cleanupLegacyHooks), so auto-recall never fired in production. The handler even carried a TODO noting it "can't easily pipe stdin to the function" and worked around it by capturing stdout — but never fed it data. Fix (resolves that TODO): - Extract the HookInput construction from readStdin into a pure parseHookInput(data) so callers that already hold the parsed JSON can build the input without touching the stream. - autoRecall(input?) accepts an optional pre-parsed input; the legacy `teamai auto-recall --stdin` command path still falls back to readStdin. - The handler now parseHookInput(stdin)s the dispatcher's object and passes it to autoRecall(input) instead of calling it with no args. Added a regression test that executes the auto-recall handler and asserts autoRecall receives a correctly-parsed input (toolName/toolInput/toolOutput/ sessionId). It fails on the old handler (`expected undefined to be defined` — autoRecall was called with no args). The legacy direct command and all existing auto-recall tests are unchanged. `npm test` 1441 passed; `tsc --noEmit` clean. Co-Authored-By: Claude --- src/__tests__/hook-handlers.test.ts | 38 ++++++++++++- src/auto-recall.ts | 88 +++++++++++++++++------------ src/hook-handlers.ts | 20 +++---- 3 files changed, 97 insertions(+), 49 deletions(-) diff --git a/src/__tests__/hook-handlers.test.ts b/src/__tests__/hook-handlers.test.ts index 01527a6..e7c54c9 100644 --- a/src/__tests__/hook-handlers.test.ts +++ b/src/__tests__/hook-handlers.test.ts @@ -33,9 +33,12 @@ vi.mock('../usage-tracker.js', () => ({ updateKnownSkills: vi.fn().mockResolvedValue(undefined), })); -vi.mock('../auto-recall.js', () => ({ - autoRecall: mockAutoRecallFromParsed, -})); +vi.mock('../auto-recall.js', async (importOriginal) => { + // Keep the real parseHookInput (and other helpers); only stub autoRecall so + // we can assert what the handler passes to it. + const actual = await importOriginal(); + return { ...actual, autoRecall: mockAutoRecallFromParsed }; +}); vi.mock('../contribute-check.js', () => ({ contributeCheck: vi.fn().mockResolvedValue(undefined), @@ -143,4 +146,33 @@ describe('hook-handlers registry', () => { const dashboard = registry.find((r) => r.handler.name === 'dashboard-report'); expect(pull!.timeoutMs).toBeGreaterThan(dashboard!.timeoutMs!); }); + + it('auto-recall handler parses STDIN and passes it to autoRecall (does not re-read process.stdin)', async () => { + // Regression: the handler used to call autoRecall() with NO arguments, + // expecting it to re-read process.stdin. But the dispatcher reads STDIN + // once and drains the stream, so autoRecall() saw no data and auto-recall + // never fired through the hook-dispatch path. The handler must now build + // the input from the dispatcher's already-parsed object and pass it in. + const registry = buildHandlerRegistry(); + const reg = registry.find((r) => r.event === 'post-tool-use' && r.matcher === 'Bash'); + expect(reg).toBeDefined(); + + const stdin = { + tool_name: 'Bash', + tool_input: { command: 'npm test' }, + tool_response: { stdout: 'Error: tests failed', stderr: '' }, + session_id: 'sess-abc', + }; + + await reg!.handler.execute(stdin, 'claude'); + + expect(mockAutoRecallFromParsed).toHaveBeenCalledTimes(1); + const passed = mockAutoRecallFromParsed.mock.calls[0][0]; + // Before the fix this was `undefined` (autoRecall called with no args). + expect(passed).toBeDefined(); + expect(passed.toolName).toBe('Bash'); + expect(passed.toolInput).toEqual({ command: 'npm test' }); + expect(passed.toolOutput).toContain('Error: tests failed'); + expect(passed.sessionId).toBe('sess-abc'); + }); }); diff --git a/src/auto-recall.ts b/src/auto-recall.ts index 6378cad..b2715b1 100644 --- a/src/auto-recall.ts +++ b/src/auto-recall.ts @@ -420,6 +420,48 @@ interface HookInput { sessionId: string; } +/** + * Build a {@link HookInput} from an already-parsed STDIN object. + * + * Factored out of {@link readStdin} so callers that already hold the parsed + * JSON (notably the hook dispatcher, which reads STDIN once and fans out to + * every handler) can construct the input directly instead of re-reading the + * now-drained `process.stdin` stream. + */ +export function parseHookInput(data: Record): HookInput { + const toolName = typeof data.tool_name === 'string' ? data.tool_name : ''; + + // Parse tool_input (the parameters passed to the tool) + const rawInput = data.tool_input; + const toolInput: Record = + rawInput !== null && typeof rawInput === 'object' && !Array.isArray(rawInput) + ? rawInput as Record + : {}; + + // Claude Code PostToolUse STDIN format: + // { tool_name, tool_input, tool_response: { stdout, stderr } } + // Other formats may use tool_output or tool_result directly. + const toolResponse = data.tool_response as Record | undefined; + const toolOutput = typeof data.tool_output === 'string' + ? data.tool_output + : typeof data.tool_result === 'string' + ? data.tool_result + : toolResponse + ? [ + typeof toolResponse.stdout === 'string' ? toolResponse.stdout : '', + typeof toolResponse.stderr === 'string' ? toolResponse.stderr : '', + ].filter(Boolean).join('\n') + : ''; + + // Derive session ID (same logic as contribute-check) + const sessionId = + (typeof data.session_id === 'string' && data.session_id) || + process.env.CLAUDE_SESSION_ID || + `pid-${process.ppid ?? process.pid}`; + + return { toolName, toolInput, toolOutput, sessionId }; +} + /** * Read and parse STDIN hook JSON. * Returns null if STDIN is a TTY or JSON is invalid. @@ -436,38 +478,7 @@ export async function readStdin(): Promise { try { const data = JSON.parse(raw) as Record; - - const toolName = typeof data.tool_name === 'string' ? data.tool_name : ''; - - // Parse tool_input (the parameters passed to the tool) - const rawInput = data.tool_input; - const toolInput: Record = - rawInput !== null && typeof rawInput === 'object' && !Array.isArray(rawInput) - ? rawInput as Record - : {}; - - // Claude Code PostToolUse STDIN format: - // { tool_name, tool_input, tool_response: { stdout, stderr } } - // Other formats may use tool_output or tool_result directly. - const toolResponse = data.tool_response as Record | undefined; - const toolOutput = typeof data.tool_output === 'string' - ? data.tool_output - : typeof data.tool_result === 'string' - ? data.tool_result - : toolResponse - ? [ - typeof toolResponse.stdout === 'string' ? toolResponse.stdout : '', - typeof toolResponse.stderr === 'string' ? toolResponse.stderr : '', - ].filter(Boolean).join('\n') - : ''; - - // Derive session ID (same logic as contribute-check) - const sessionId = - (typeof data.session_id === 'string' && data.session_id) || - process.env.CLAUDE_SESSION_ID || - `pid-${process.ppid ?? process.pid}`; - - return { toolName, toolInput, toolOutput, sessionId }; + return parseHookInput(data); } catch { return null; } @@ -492,19 +503,24 @@ export async function readStdin(): Promise { * Output: STDOUT JSON with hookSpecificOutput.additionalContext when matching results found. * Claude Code reads additionalContext and passes it to AI as context. */ -export async function autoRecall(): Promise { +export async function autoRecall(input?: HookInput | null): Promise { // ─── Eval harness: disable flag ──────────────────── if (process.env.TEAMAI_RECALL_DISABLED === '1') { return; } - const input = await readStdin(); - if (!input) { + // Accept a pre-parsed input when the caller already has it (the hook + // dispatcher reads STDIN once and passes the parsed object through). + // Fall back to reading STDIN for the legacy `teamai auto-recall --stdin` + // command path. Without this, the dispatcher's drained process.stdin would + // yield no data and auto-recall would never fire. + const resolved = input ?? (await readStdin()); + if (!resolved) { log.debug('auto-recall: no STDIN data'); return; } - const { toolName, toolInput, toolOutput, sessionId } = input; + const { toolName, toolInput, toolOutput, sessionId } = resolved; // Fast path: unknown tools → exit immediately if (!RECALL_TOOLS.has(toolName)) { diff --git a/src/hook-handlers.ts b/src/hook-handlers.ts index 5f72ba9..9a90267 100644 --- a/src/hook-handlers.ts +++ b/src/hook-handlers.ts @@ -166,13 +166,16 @@ const contributeCheckHandler: HookHandler = { const autoRecallHandler: HookHandler = { name: 'auto-recall', async execute(stdin, _tool) { - // Auto-recall has complex internal logic (tool dispatch, error detection, rate limiting) - // For now, delegate to the existing function by temporarily mocking STDIN. - // TODO: Refactor autoRecall to accept parsed data directly. - const { autoRecall } = await import('./auto-recall.js'); + const { autoRecall, parseHookInput } = await import('./auto-recall.js'); - // The auto-recall function reads STDIN internally. To avoid changing its signature - // in this phase, we capture its STDOUT output via a process.stdout.write intercept. + // The dispatcher reads STDIN once and hands us the already-parsed object. + // autoRecall() used to re-read process.stdin internally, but that stream is + // already drained here — so build the hook input from the parsed object and + // pass it through directly (otherwise auto-recall never fires). + const input = parseHookInput(stdin); + + // autoRecall writes its result (PostToolUse additionalContext JSON) to + // STDOUT. Capture that output so the dispatcher can relay it. let capturedOutput: string | null = null; const originalWrite = process.stdout.write.bind(process.stdout); process.stdout.write = ((chunk: unknown) => { @@ -185,10 +188,7 @@ const autoRecallHandler: HookHandler = { }) as typeof process.stdout.write; try { - // We can't easily pipe stdin to the function, so for this handler - // we'll rely on the environment (process.stdin being piped from Claude Code). - // In the dispatcher, auto-recall will be invoked with the raw data. - await autoRecall(); + await autoRecall(input); } finally { process.stdout.write = originalWrite; }