Skip to content

fix(hook): remove payload temp files on every dispatch exit - #1814

Closed
hobostay wants to merge 1 commit into
Tencent:mainfrom
hobostay:fix/hook-payload-cleanup
Closed

hobostay wants to merge 1 commit into
Tencent:mainfrom
hobostay:fix/hook-payload-cleanup

Conversation

@hobostay

@hobostay hobostay commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

hook-dispatch's Windows/WMI spawn path writes each hook payload to %TEMP%\teamai-hook-<pid>-<ms>.json, because WMI has no stdin pipe. Only readStdinFile() removed that file, so every early return before the read (the launcher gate, the another-host gate, or any error) left the full payload — prompts, tool input and output — on disk indefinitely. A parent killed before its detached child started leaked it the same way.

This makes cleanup independent of the read: hookDispatchCli now removes --stdin-file in a finally, and on Windows every --bg-only child sweeps teamai-hook-*.json files in the temp directory that are older than ten minutes, collecting files orphaned by a parent that died first. Fresh files are untouched, so a live sibling's payload is never swept. The sweep is Windows-only because the plain spawn paths pipe the payload and create no temp file. A CHANGELOG entry is included.

Fixes #1812.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

  • npx tsc --noEmit passes (npm run typecheck).
  • npm run lint passes, including documentation checks.
  • npx vitest run src/__tests__/hook-dispatch-cli.test.ts — 41 passed. New tests cover the early-return cleanup, that a cleanup failure cannot fail the dispatch, that sweepStaleHookPayloads removes only old teamai-hook-*.json files while keeping fresh and unrelated files, and that the sweep is skipped off Windows.
  • Related hook suites pass: hook-dispatch-cli, hook-dispatch-scope, recall-attribution, dashboard-report-scope, score-drain-stop, votes-scope — 6 files, 311 tests.
  • Real-CLI verification after npm run build (macOS, node dist/index.js):
    1. Created a payload file and ran TEAMAI_LAUNCHED_BY=judge node dist/index.js hook-dispatch stop --tool claude --stdin-file <file> → exit 0, file removed. This is the early-return path that previously left the file behind.
    2. Created a stale teamai-hook-*.json (mtime 11 minutes old) in os.tmpdir() and ran the same background pass with --bg-only --stdin-file <fresh> → exit 0, the fresh stdin file was removed, and the stale file remained, confirming the Windows-only gate on macOS.

I could not exercise the Windows WMI spawn itself on this macOS host. That spawn path is unchanged. The sweep's removal behavior is covered by the unit test that supplies a temp directory directly; the Windows-only integration is left to Windows CI. Windows CI does not currently run hook-dispatch-cli.test.ts.

Related Issues

Fixes #1812.

Notes for Reviewers

  • The sweep only considers regular files whose name starts with teamai-hook- and ends with .json, and ignores every cleanup error, so it can never affect dispatch.
  • A live child reads and removes its own payload immediately on entry; ten minutes is far beyond that window, so only abandoned files qualify.
  • The finally also covers the normal path, where readStdinFile() already removed the file. fs.rmSync(..., { force: true }) makes the second removal a no-op.

🤖 Generated with Claude Code

@SaulMoro

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Thanks for this. #1813 fixes the same issue (#1812) in the same file. It was opened earlier and is already in review, so only one of the two should land. I compared them so the choice is quick:

#1813 #1814
Removes the file on early returns yes, finally in hookDispatchCli yes, same finally
Removes it right after the read no: the finally replaced the read-time removal, so the file stays for the whole background pass yes: readStdinFile keeps its removal, and the finally is a no-op on that path
Sweep of stale teamai-hook-*.json Windows only every --bg-only pass on every platform, so a macOS or Linux hook lists the temp directory each time, though only the Windows path writes these files
CHANGELOG entry for the fix and the sweep none

This PR's read-time removal is the part I asked #1813 to add. Its sweep should be limited to Windows, as #1813's is, and it needs a CHANGELOG entry.

I'm leaving this PR open while the author of #1813 answers. @jeff-r2026 your call on this one: which of the two to keep, since they are the same fix for #1812.

@github-actions

Copy link
Copy Markdown

No findings in the specified PR diff.

The PR description includes representative real-CLI verification of both cleanup paths, satisfying the review rules’ testing-record requirement. Windows WMI execution is explicitly untested; that spawn path is unchanged.

I did not run, build, or install anything from the PR.

@SaulMoro

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

#1813 is merged (531eb15f) and closes #1812, including the read-time removal this PR also had. @jeff-r2026 this one can be closed as a duplicate, unless you see something here #1813 does not cover.

The Windows/WMI spawn path hands each hook payload to the detached child
as teamai-hook-<pid>-<ms>.json because WMI has no stdin pipe. Only
readStdinFile() removed it, so early returns before that read (launcher
and another-host gates, errors) left the full payload in %TEMP%
indefinitely; a parent killed before any child started did the same.

Wrap the dispatch in a finally that always removes --stdin-file, and on
Windows have every --bg-only child sweep teamai-hook-*.json files older
than ten minutes so files orphaned by a dead parent are collected too.
The sweep is Windows-only because the plain spawn paths pipe the payload
and create no temp file. Fresh files are untouched, so a live sibling's
payload survives. Cleanup errors are swallowed: a completed dispatch
must not fail on cleanup.

Fixes Tencent#1812

Co-Authored-By: Claude Code <noreply@anthropic.com>
@hobostay
hobostay force-pushed the fix/hook-payload-cleanup branch from 1a97966 to aec745b Compare November 10, 2026 08:47
@hobostay

Copy link
Copy Markdown
Contributor Author

Applied both points at aec745b:

  • The stale sweep now runs only when process.platform === 'win32'; macOS and Linux background passes do not list the temp directory.
  • Added a CHANGELOG entry under Unreleased → Bug Fixes.

On payload lifetime: the read-time removal in readStdinFile() is untouched on this branch, so a payload the child actually reads is deleted immediately; the outer finally only covers exits that never read it (and swallows cleanup errors so a completed dispatch cannot fail on cleanup).

Tests at this head: hook-dispatch-cli.test.ts 41 passed, the six related hook suites 311 passed, npm run typecheck and npm run lint pass. Real CLI on macOS: the early-return path removes the file, and an old teamai-hook-*.json is left alone by a background pass, which verifies the Windows-only gate.

#1813 is the earlier PR for the same issue and now has the read-time removal too. I am happy to close this one in favour of #1813 if you prefer the first-opened branch; there is no need to land both.

@hobostay

Copy link
Copy Markdown
Contributor Author

Superseded by #1813, which merged to main as 531eb15 while this was under review. The merged fix has the same read-time removal, outer finally for exits that never read, Windows-only ten-minute sweep and CHANGELOG entry, so there is nothing left to land here. Closing to avoid a duplicate; thanks to @Walternater and @SaulMoro for the triage and comparison.

@hobostay hobostay closed this Nov 10, 2026
@github-actions

Copy link
Copy Markdown

No findings in the specified PR diff. The earlier review also reported none, so there are no prior findings to mark resolved.

The PR description documents representative real-CLI verification, satisfying the testing-record requirement. Windows integration is explicitly deferred; the WMI spawn path is unchanged.

I did not run, build, or install anything from the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Windows: hook-dispatch leaves %TEMP%\teamai-hook-*.json payload files behind

3 participants