Skip to content

fix(hooks): remove the payload file a background hook hands over - #1813

Merged
SaulMoro merged 1 commit into
Tencent:mainfrom
Walternater:fix/hook-payload-cleanup
Nov 10, 2026
Merged

SaulMoro merged 1 commit into
Tencent:mainfrom
Walternater:fix/hook-payload-cleanup

Conversation

@Walternater

@Walternater Walternater commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Every hook-dispatch that runs with --stdin-file removes that file as soon as it reads it, and on the exits that never read it: a session teamai launched for itself (judge, scorer, recall-promote, codebase), and a host where Cursor or Copilot CLI has its own hooks. A background pass therefore holds the payload no longer than main did.
  • A --bg-only pass sweeps teamai-hook-*.json files older than ten minutes in its temp directory. That covers the other leak: the parent writes the payload, is killed by the host's hard-exit timer or job object, and never starts the child that would have removed it.
  • The sweep is Windows-only, since the plain spawn paths pipe the payload and create no file. Nothing else changes on macOS or Linux.
  • Both leaks left a full payload on disk, tool input and output included, so this was session content surviving after the hook, not just clutter.

Refs #1812. Fixes #1812.

Validation

  • npm run build — passes
  • npm run typecheck — passes
  • npm run lint — passes, including the documentation checks
  • npx vitest run src/__tests__/hook-dispatch-cli.test.ts src/__tests__/hook-dispatch.test.ts src/__tests__/hook-fast-path.test.ts src/__tests__/dashboard-report-scope.test.ts src/__tests__/git-hook.test.ts — 246/246 pass

Proof first: the early-return case fails on main for the reason the issue gives.

$ git show origin/main:src/hook-dispatch-cli.ts > src/hook-dispatch-cli.ts && npx vitest run src/__tests__/hook-dispatch-cli.test.ts
 FAIL  src/__tests__/hook-dispatch-cli.test.ts > hookDispatchCli > removes the payload file on a path that returns before reading it
AssertionError: expected true to be false // Object.is equality
      Tests  3 failed | 37 passed (40)

Of those three, two are the sweep cases, which fail on main only because the function does not exist there. The read-time case went the other way: main removed the file at the read, the first head of this branch moved that removal to the outermost finally, and the new case removes the payload file as soon as it has read it failed on that head (expected true to be false) until the removal went back beside the read, with the outer finally kept for the exits that never read.

Real-CLI run

A pair of worlds, same command in each, on this macOS host. The issue's failing path is reproduced against main and gone against the branch.

World from main (b8c811db0cfe+dirty, teamai 0.22.0, sha256 3ee52079161f) — the +dirty is this worktree's untracked node_modules symlink; the tree is otherwise origin/main:

$ printf '{"session_id":"abc"}' > $WORLD/tmp/payload.json
$ npm run -s verify -- sh 20261110T064812-7e3f03 env TEAMAI_LAUNCHED_BY=judge teamai hook-dispatch post-tool-use --tool claude --bg-only --stdin-file $WORLD/tmp/payload.json
$ ls $WORLD/tmp
node-compile-cache
payload.json

FAILED: the payload file is still there, as the issue describes.

World from the branch (95152bb8a0c4+dirty, teamai 0.22.0, sha256 307d54b65078) — same node_modules caveat:

$ printf '{"session_id":"abc"}' > $WORLD/tmp/payload.json
$ npm run -s verify -- sh 20261110T081458-c2ede5 env TEAMAI_LAUNCHED_BY=judge teamai hook-dispatch post-tool-use --tool claude --bg-only --stdin-file $WORLD/tmp/payload.json
$ ls $WORLD/tmp
node-compile-cache

VERIFIED: the file is gone, exit 0, and the dispatch still returns early for a launcher session. npm run -s verify -- ledger 20261110T081458-c2ede5 ends with Ledger: CLEAN.

Limitations

  • The sweep itself is UNVERIFIABLE in a world on this machine: it refuses to run off Windows, which is the platform whose spawn path writes the file. It is covered by a unit test that injects platform: 'win32' and by the macOS check that it leaves the directory alone; Windows CI runs a subset of the tests, and src/hook-dispatch-cli.ts is not in it.
  • The age cutoff is ten minutes, far beyond any hook run. A payload left by a parent killed seconds ago waits for the next background pass on that machine.
  • Nothing changes for tools, scopes, team repo modes or layers: --stdin-file is internal to the Windows spawn path, and every host reaches the same dispatch.

@github-actions

Copy link
Copy Markdown

No findings in the specified PR diff.

The description satisfies the testing requirement: it records a representative real-CLI reproduction on main, successful verification on the PR head, and a clean world ledger. Windows sweep verification is explicitly disclosed as unavailable locally.

Review was read-only; I did not run, build, or install PR code.

@SaulMoro

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Reviewed at 20ccd00b, with the built CLI on main (b8c811db) and on this head:

  • Early return. With TEAMAI_LAUNCHED_BY=judge, main leaves the payload file and this head removes it. Both exit 0.
  • Sweep. On macOS a stale teamai-hook-*.json is kept on both builds, as the Windows-only guard intends. With the platform and os.tmpdir patched to win32 in one process, the stale file goes, and a fresh file and settings.json stay. That is not a real Windows run.
  • Scope. The sweep is what [bug] Windows: hook-dispatch leaves %TEMP%\teamai-hook-*.json payload files behind #1812 asks for, and the CHANGELOG lists it. Nothing breaks.

One point before I approve:

  • [P2 non-blocking] The payload now stays on disk for the whole background pass. On main, readStdinFile removed the file as soon as it was read. Here that finally moved to hookDispatchCli (src/hook-dispatch-cli.ts:455), which wraps all of runHookDispatch. A --bg-only pass that runs its handlers for several seconds now keeps the full payload in the temp directory for that long, when main kept it only for the read. Keeping the removal in readStdinFile, and adding an early rmSync for the paths that return before the read, would give both: the file goes on every exit, and as soon as it has been read. I found this by reading the code; I did not time a pass.

  • [P3 nit] The new existsSync assertion in the no-scope case passes on main too. The two sweep cases fail on main only because the function does not exist yet. The red-on-main proof in the body holds for the early-return case.

Also, hook-dispatch-cli.test.ts is not in the Windows CI subset, so these cases do not run on Windows. #1775 edits the same file in other hunks, and they merge cleanly.

CI is still running at this head.

A Windows hook writes its payload to a temp file for the child it starts
through WMI, and only the read path removed it. A child that returned early,
and a parent killed before it started one, left the full payload — tool input
and output — in %TEMP% for good. The dispatch now removes it on every exit,
and a background pass sweeps payload files an earlier one left behind.

Refs Tencent#1812
@Walternater
Walternater force-pushed the fix/hook-payload-cleanup branch from 20ccd00 to 95152bb Compare November 10, 2026 08:15
@Walternater

Copy link
Copy Markdown
Collaborator Author

Both points are addressed in 95152bb8.

  • P2, payload lifetime. You're right, and the fix is the one you described: removal is back in readStdinFile's finally, so it goes the moment it is read, and hookDispatchCli keeps an outer finally for the exits that never read. There is a new case, removes the payload file as soon as it has read it, which asserts the file is gone from inside the dispatcher call, not only after the pass. It passes on main and fails on 20ccd00b (expected true to be false), so the regression I introduced is now guarded rather than just removed.
  • P3, the red-on-main proof. Correct, and the body now says so: one case fails on main for the behavior, the two sweep cases fail only because the function is new there, and the read-time case is the one that failed on the previous head. The existsSync assertion in the no-scope case is a guard, not proof.

Windows CI still does not run hook-dispatch-cli.test.ts; the sweep's win32 branch is covered by the injected-platform unit test only, and the body says UNVERIFIABLE for a real Windows run. Rebased on b8c811db as before; the pair of worlds was rebuilt at this head — 95152bb8a0c4+dirty, sha256 307d54b65078, ledger CLEAN, same FAILED-on-main / VERIFIED-on-head result.

@github-actions

Copy link
Copy Markdown

No findings in the specified PR diff.

The description satisfies the testing requirement: it records a representative real-CLI reproduction on main, successful verification on the specified PR head, and a clean world ledger. Windows sweep verification is explicitly disclosed as unavailable locally.

The earlier review raised no findings, so none require resolution.

Review was read-only; I did not run, build, or install PR code.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Re-checked at 95152bb8. The payload point is fixed: readStdinFile removes the file again as soon as it reads it, and the finally in hookDispatchCli now only covers the exits that never reach the read, through the same removeStdinFile. The new case checks that the file is already gone while the handlers run, which is the behaviour main had and the earlier head lost. The sweep, its Windows-only guard and the CHANGELOG entry are unchanged from my previous run.

Not run here: the Windows spawn path and the sweep on real Windows. This test file is also outside the Windows CI subset.

Approving. I'll merge once CI is green at this head.

@SaulMoro
SaulMoro merged commit 531eb15 into Tencent:main Nov 10, 2026
19 checks passed
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