Skip to content

fix(reports): await the late usage report in session-start pull - #1323

Merged
SaulMoro merged 1 commit into
Tencent:mainfrom
scpia:fix/1322-await-session-start-report
Oct 23, 2026
Merged

SaulMoro merged 1 commit into
Tencent:mainfrom
scpia:fix/1322-await-session-start-report

Conversation

@scpia

@scpia scpia commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #1322.

What changes

The session-start pull handler runs in the detached hook-dispatch --bg-only pass. pull() waits at most 5 s for its usage report and then returns while the report is still pushing; the dispatcher's process.exit(0) then cut that push. The commit stayed in the local reports checkout, .reports-lock kept a dead PID, and the next session start ran into the same race.

  • pull.ts keeps the late report together with the release of its sync locks (the chain it already built) and exports settleLateUsageReport(), which resolves once both are done.
  • The pull handler awaits it before it returns. Background handlers only run in the detached pass, which the host does not wait for, so the session is not delayed. The handler stays bounded by PULL_TIMEOUT_MS, and a stalled reports git is still killed by the block timeout from fix(reports): kill stalled side-branch git with createGitWithBlockTimeout #1268.

The foreground hook path, teamai pull from the CLI and the detached pull of the git hooks are unchanged; the last two already exit only when the event loop is empty.

Testing

After npm run build:

  • New real-CLI case in src/__tests__/e2e/report-timeout.test.ts: hook-dispatch session-start --tool claude --bg-only against a remote whose update hook sleeps 7 s. Without the fix it fails with fatal: invalid object name 'teamai-reports' (the push never reached the remote); with it the stats land, the usage file is consumed and .sync-lock is gone. npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/report-timeout.test.ts: 20 passed.
  • New unit case in hook-handlers.test.ts: the pull handler returns only after settleLateUsageReport() resolves. The four other suites that mock ../pull.js now mock the new export too, so their pull handler no longer throws at its end. The seven suites that mock pull.js or exercise hook dispatch: 432 passed.
  • npx tsc --noEmit and npm run lint are clean.
  • Full npx vitest run on WSL2 (Ubuntu, Node 22.23): 12021 passed, 12 failed. Eleven fail the same way on main 2401d04 in this environment (cursor-bubble-range, cursor-user-images, init-hang-regression, self-mode-agents, submodule-checkout, and git-kind-reports "a stalled origin"). workflow-reconciliation failed under the full run's load and passes when run alone.

Only Claude and the git provider were run; the change does not depend on the agent or the provider, the rest is left to CI.

The session-start pull runs in the detached hook-dispatch --bg-only
pass. pull() stops waiting for its usage report after 5 s, and the
dispatcher's process.exit(0) then cut the push: the commit stayed in
the local reports checkout and .reports-lock kept a dead PID.

pull() now keeps the late report together with the release of its
sync locks and exports settleLateUsageReport(); the pull handler
awaits it before returning. The host does not wait for the detached
pass, so the session is not delayed.

Fixes Tencent#1322
@github-actions

Copy link
Copy Markdown

No findings in the specified PR diff.

The PR description documents sufficient testing: a post-build real-CLI session-start run verifies the slow report push completes, usage data is consumed, and .sync-lock is released. One representative Claude/git run satisfies the review criteria; the remaining matrix is explicitly deferred to CI.

Reviewed only the diff; 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.

@scpia Thanks, approving. This fixes #1322.

We built this head and its base 2401d047 and ran the issue's repro with the real CLI: a 7 s reports update hook, then hook-dispatch session-start --tool claude --bg-only.

  • base: the pass exits after 5.3 s, stats/alice.yaml never reaches the remote, and usage is not truncated.
  • this PR: the pass waits about 8 s, the stats reach the remote (review.count: 1), usage is truncated, and no commit stays unpushed. Both locks are released.

report-timeout.test.ts passes 20/20 here, and the new case fails on base with fatal: invalid object name 'teamai-reports'. tsc, build and lint pass. The foreground path is unchanged, since the pull handler is background: true.

Suggestions:

  • [P2 non-blocking] The failure half of #1322 ("fails with a logged reason and a released lock") has no test here. #1324's keeps a rejected session-start report retryable and releases its locks passes on this head. Pulling it in would cover it.
  • [P3 nit] settleLateUsageReport() runs only if pull() resolves. If pull() throws after the late report starts, the pass can exit with the push in flight. We found no trigger. A finally would cover it:
-    await pull({ silent: true });
+    try {
+      await pull({ silent: true });
+    } finally {
+      await settleLateUsageReport();
+    }

Not run: the full unit and e2e suites, Windows.

@SaulMoro
SaulMoro merged commit 83f6d1d into Tencent:main Oct 23, 2026
18 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] A session-start usage report is killed by the hook-dispatch exit before its push lands

3 participants