diff --git a/CHANGELOG.md b/CHANGELOG.md index a4a5b9a064..c0dcdbd87e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes +- `hook-dispatch` removes the temp file that carries a payload to a background child on every exit, including the paths that return before reading it, and a background pass clears payload files an earlier one left in the temp directory. A Windows hook whose child exited early, or one killed before its child started, kept a full hook payload — tool input and output — in `%TEMP%` for good (for [#1812](https://github.com/Tencent/teamai-cli/issues/1812)). - Pull says when it could not apply the team MCP servers, and why, such as `Team MCP servers were not applied: EACCES: permission denied, open '.../.mcp.json...'`. It wrote that only to `~/.teamai/debug.log`, so a failed write looked like a pull that changed nothing - A tool teamai starts in a directory gets `PWD` set to that directory. OpenCode works in `PWD`, not in its process's directory, so a workflow step or benchmark trial on OpenCode started from a member's checkout edited that checkout instead of its own worktree or clone (for [#1793](https://github.com/Tencent/teamai-cli/issues/1793)). - A label a local confined judge gave names the model alias its scorer asked for, beside the model the tool reported. It named only the model, so a benchmark read every judged verdict as a deviation from its pinned judge (for [#1793](https://github.com/Tencent/teamai-cli/issues/1793)). diff --git a/src/__tests__/hook-dispatch-cli.test.ts b/src/__tests__/hook-dispatch-cli.test.ts index db1eb2eb81..2a4de9bd6e 100644 --- a/src/__tests__/hook-dispatch-cli.test.ts +++ b/src/__tests__/hook-dispatch-cli.test.ts @@ -24,7 +24,7 @@ vi.mock('node:child_process', async (importOriginal) => ({ const { runHookFastPath } = await import('../hook-fast-path.js'); const { readStdin } = await import('../utils/hook-stdin.js'); -const { parseStdin, trySpawnDetachedViaWmi, deriveDispatchSessionId, hookDispatchCli, claudeHookRunsInAnotherHost } = +const { parseStdin, trySpawnDetachedViaWmi, deriveDispatchSessionId, hookDispatchCli, claudeHookRunsInAnotherHost, sweepStalePayloads } = await import('../hook-dispatch-cli.js'); const { claudeHookOtherHostSkip } = await import('../claude-hook-host.js'); const { reconcileHooks } = await import('../hooks.js'); @@ -74,6 +74,43 @@ describe('hookDispatchCli', () => { } finally { resolveScope.mockRestore(); readFile.mockRestore(); vi.unstubAllEnvs(); } }); + // The payload file is a child's to remove, and an early return used to skip + // that: every dispatch became another full hook payload left in %TEMP% (#1812). + it('removes the payload file on a path that returns before reading it', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'payload-exit-')); + const file = path.join(dir, 'payload.json'); + fs.writeFileSync(file, '{"session_id":"abc"}'); + vi.stubEnv('TEAMAI_LAUNCHED_BY', 'judge'); + try { + await hookDispatchCli('post-tool-use', 'claude', '*', { bgOnly: true, stdinFile: file }); + expect(fs.existsSync(file)).toBe(false); + } finally { + vi.unstubAllEnvs(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + // A pass runs its handlers for seconds; the payload holds tool input and + // output, so it goes the moment it is read, not when the pass ends (#1813). + it('removes the payload file as soon as it has read it', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'payload-read-')); + const file = path.join(dir, 'payload.json'); + fs.writeFileSync(file, JSON.stringify({ session_id: 's', cwd: dir, tool_name: 'Read' })); + let presentWhileDispatching: boolean | null = null; + mockDispatcher.dispatch.mockImplementationOnce(async () => { + presentWhileDispatching = fs.existsSync(file); + + return { errors: [], output: null }; + }); + try { + await hookDispatchCli('post-tool-use', 'claude', '*', { stdinFile: file }); + expect(presentWhileDispatching).toBe(false); + expect(fs.existsSync(file)).toBe(false); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + // The fast path read stdin before it found the payload had no session id, so the pipe is drained (#1788). it('dispatches the payload the fast path read before it fell through to the full dispatch', async () => { const stdin = new PassThrough(); @@ -140,6 +177,8 @@ describe('hookDispatchCli', () => { expect(mockDispatcher.dispatch).not.toHaveBeenCalled(); expect(log.debug).toHaveBeenCalledWith(`[hook-dispatch] ${event}/claude no scope`); + // The payload file is this child's to remove, read or not (#1812). + expect(fs.existsSync(stdinFile)).toBe(false); }); }); @@ -663,6 +702,39 @@ afterEach(() => { for (const f of createdPayloads.splice(0)) fs.rmSync(f, { force: true }); }); +describe('sweepStalePayloads', () => { + let dir: string; + let stale: string; + let fresh: string; + let other: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'hook-sweep-')); + stale = path.join(dir, 'teamai-hook-123-1700000000000.json'); + fresh = path.join(dir, 'teamai-hook-456-1700000000001.json'); + other = path.join(dir, 'settings.json'); + for (const file of [stale, fresh, other]) fs.writeFileSync(file, '{}'); + const anHourAgo = new Date(Date.now() - 60 * 60 * 1000); + fs.utimesSync(stale, anHourAgo, anHourAgo); + fs.utimesSync(other, anHourAgo, anHourAgo); + }); + + afterEach(() => fs.rmSync(dir, { recursive: true, force: true })); + + it('removes payload files no live child can still be reading, and nothing else', () => { + expect(sweepStalePayloads(dir, Date.now(), 'win32')).toBe(1); + + expect(fs.existsSync(stale)).toBe(false); + expect(fs.existsSync(fresh)).toBe(true); + expect(fs.existsSync(other)).toBe(true); + }); + + it('leaves the temp directory alone on a platform whose spawn paths pipe the payload', () => { + expect(sweepStalePayloads(dir, Date.now(), 'darwin')).toBe(0); + expect(fs.existsSync(stale)).toBe(true); + }); +}); + describe('trySpawnDetachedViaWmi', () => { it('creates the child through Win32_Process, hidden, with the payload as a file', async () => { await expect(trySpawnDetachedViaWmi( diff --git a/src/hook-dispatch-cli.ts b/src/hook-dispatch-cli.ts index 11d09efad7..4e152dfe0c 100644 --- a/src/hook-dispatch-cli.ts +++ b/src/hook-dispatch-cli.ts @@ -154,6 +154,12 @@ export function salvageStdinFields(raw: string): Partial { + try { + await runHookDispatch(event, tool, matcher, options); + } finally { + // The read path removed it already; this covers every exit that never got + // that far (#1812) — the file is this child's only, and it is the only + // process that knows its path. + if (options.stdinFile) removeStdinFile(options.stdinFile); + } +} + +async function runHookDispatch( + event: string, + tool: string, + matcher: string, + options: HookDispatchOptions, ): Promise { // A session teamai launched for itself, a judge or an AI call, is not the member's: only a workflow step's runs hooks. const launcher = launchedBy(process.env); @@ -416,6 +485,10 @@ export async function hookDispatchCli( const { bgOnly = false, stdinFile } = options; setStderrOnly(true); + // A parent killed before its child starts leaves its payload behind, and no + // process is left to remove it: this pass is that removal (#1812). + if (bgOnly) sweepStalePayloads(); + if (claudeHookRunsInAnotherHost(tool, process.env, false)) { log.debug('hook-dispatch: skipping claude hooks because Cursor or Copilot CLI has its own teamai hooks');