Skip to content

fix(push): clear timeout timer after push settles to avoid event-loop hang#211

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
hobostay:fix/push-timeout-leak
Jul 20, 2026
Merged

fix(push): clear timeout timer after push settles to avoid event-loop hang#211
jeff-r2026 merged 1 commit into
Tencent:mainfrom
hobostay:fix/push-timeout-leak

Conversation

@hobostay

Copy link
Copy Markdown
Contributor

Problem

reportUsageToTeam (run from pull() on every teamai pull that has usage events to report — i.e. on most session-start hook fires) and contribute() both guarded their git push with a plain Promise.race:

const pushPromise = pushRepoDirectly(repoPath, commitMsg, filesToPush);
const timeoutPromise = new Promise<never>((__, reject) =>
  setTimeout(() => reject(new Error('Auto-report timeout (5s)')), 5000),
);
await Promise.race([pushPromise, timeoutPromise]);

When the push resolves quickly — the normal case — the losing setTimeout is never cleared. It stays pending on the Node event loop, so the process hangs at the terminal for the full 5000 ms (pull) / 10_000 ms (contribute) after all real work has finished.

Because teamai pull runs from session-start hooks, this hang recurred on nearly every session and was very visible. clearTimeout never appears in either file; by contrast clone.ts already implements the correct pattern (clearTimeout(timer) in both the close and error handlers).

Fix

Replace both sites with a shared withTimeout() helper (src/utils/async.ts) that clears its timer in a finally once the guarded promise settles:

export async function withTimeout<T>(promise: Promise<T>, timeoutMs: number, message: string): Promise<T> {
  let timer: ReturnType<typeof setTimeout> | undefined;
  const timeoutPromise = new Promise<never>((_, reject) => {
    timer = setTimeout(() => reject(new Error(message)), timeoutMs);
  });
  try {
    return await Promise.race([promise, timeoutPromise]);
  } finally {
    if (timer) clearTimeout(timer);
  }
}

Behavior is otherwise unchanged: it still rejects with the timeout message if the push exceeds the budget, and propagates the push's own rejection if it fails first.

Test plan

  • npx tsc --noEmit — clean
  • npm run build — succeeds
  • npm test — 1766 passed (1760 baseline + 6 new)
  • New unit tests (src/__tests__/async-timeout.test.ts) assert the timer is cleared on both the success and failure paths, plus a handle-count check that no timer survives a fast success.
  • End-to-end timing demo (inlined old vs new pattern, 3000 ms budget, ~60 ms push):
    • OLD: race returns in 62 ms, but real 3.04s — process hangs on the leaked timer.
    • NEW: race returns in 62 ms, real 0.11s — process exits promptly.

🤖 Generated with Claude Code

… hang

`reportUsageToTeam` (run on every `teamai pull` that has usage events to
report) and `contribute` both guarded their git push with a plain
`Promise.race([pushPromise, timeoutPromise])`. When the push resolved
quickly — the normal case — the losing `setTimeout` was never cleared, so a
5s (pull) / 10s (contribute) timer stayed pending on the Node event loop and
the process hung at the terminal for the full duration after all real work
had finished.

Because `teamai pull` runs from session-start hooks, this hang recurred on
nearly every session and was very visible to users.

Replace both sites with a shared `withTimeout()` helper that clears its
timer in a `finally` once the guarded promise settles — the same correct
pattern already used in `clone.ts` (`runCommand`). Adds unit tests asserting
the timer is cleared on both the success and failure paths (the regression
guard), plus a handle-count check that no timer survives a fast success.

Verified end-to-end: the OLD pattern returns in ~60ms but keeps the process
alive for the full 3000ms timeout (3.04s wall clock); with the fix the
process exits in ~0.11s.

Co-Authored-By: Claude <noreply@anthropic.com>
@jeff-r2026
jeff-r2026 merged commit 8a66020 into Tencent:main Jul 20, 2026
6 checks passed
Eyre921 added a commit to Eyre921/teamai-cli that referenced this pull request Jul 21, 2026
…est feed

Implements Features 1 & 5 from docs/designs/team-intelligence-platform.md,
which were accepted ("ENG LOCKED") but never built — digest.ts's
getRecentSessions() has been reading an always-empty sessions/ directory.

`teamai session save` folds the dashboard's existing per-session event stream
(tool sequence, prompt turns, interventions) into a compact markdown summary and
appends it to a local monthly log (~/.teamai/session-logs/<year-month>.md). With
`--push`, a "valuable" session (interventions or substantial tool use, per
Decision 8) is committed to the team repo at sessions/<user>/<month>.md — the
path digest already reads, so "Session Highlights" lights up with no change to
digest.ts.

Reuses existing infrastructure rather than a new collection path:
- dashboard-collector readEvents() + aggregateSessionMetrics() for the data
- utils/git pushRepoDirectly() for the direct team commit (no MR), like contribute
- redact() (from Tencent#191) on the only free text included (the first-prompt line)

Review feedback addressed (round 1):
- Placed under a `session` subcommand group (`teamai session save`) instead of a
  top-level verb, matching how `roles` / `source` are organized, leaving room
  for `session list` / `session show` later.
- Team upload defaults to counts + tools only; the redacted first-prompt line is
  opt-in via `--include-prompt`, since redact() is best-effort. Local logs keep it.

Review feedback addressed (round 2):
- Fixed user-facing command strings that still said `teamai save-session` (an
  unknown command): the read-only error hint and the retry hint now say
  `teamai session save --push`; aligned docstrings in save-session.ts /
  session-collector.ts / types.ts and the design-doc references.
- `assertNotReadOnly` on the --push path is now wrapped in try/catch: an
  HTTP-mode (read-only) team prints a friendly message and keeps the local log
  instead of throwing an uncaught stack trace.
- Idempotency key is now the full session id in a non-rendering HTML comment
  (`<!-- teamai:session <id> -->`) rather than the 8-char short id, removing the
  ~1-in-4B same-month prefix collision that could silently drop a session.
- Replaced the ad-hoc `Promise.race`/`setTimeout` push guard with the shared
  `withTimeout()` helper (src/utils/async.ts, from Tencent#211), which clears its timer
  in a finally so the process doesn't linger after a fast push.

Team upload is opt-in; local logs are pruned after 90 days (Feature 5 retention).

Test Plan:
- npx tsc --noEmit
- npx vitest run (145 files, 1801 tests; 15 in session-collector.test.ts,
  incl. a new "distinct sessions sharing an 8-char id prefix" case)
- npm run build
- real-CLI E2E: seeded ~/.teamai/dashboard/events.jsonl, ran
  `node dist/index.js session save --session-id <sid>`; wrote
  ~/.teamai/session-logs/2026-07.md with folded stats + full-id marker, a `ghp_`
  token in the prompt came out `<REDACTED:gh_tok>`, a second run was idempotent;
  `--push` against a read-only HTTP team printed the graceful message (exit 0, no
  stack trace); old `save-session` errors as unknown; `--include-prompt` appears
  in `session save --help`.
jeff-r2026 pushed a commit that referenced this pull request Jul 21, 2026
…est feed (#192)

Implements Features 1 & 5 from docs/designs/team-intelligence-platform.md,
which were accepted ("ENG LOCKED") but never built — digest.ts's
getRecentSessions() has been reading an always-empty sessions/ directory.

`teamai session save` folds the dashboard's existing per-session event stream
(tool sequence, prompt turns, interventions) into a compact markdown summary and
appends it to a local monthly log (~/.teamai/session-logs/<year-month>.md). With
`--push`, a "valuable" session (interventions or substantial tool use, per
Decision 8) is committed to the team repo at sessions/<user>/<month>.md — the
path digest already reads, so "Session Highlights" lights up with no change to
digest.ts.

Reuses existing infrastructure rather than a new collection path:
- dashboard-collector readEvents() + aggregateSessionMetrics() for the data
- utils/git pushRepoDirectly() for the direct team commit (no MR), like contribute
- redact() (from #191) on the only free text included (the first-prompt line)

Review feedback addressed (round 1):
- Placed under a `session` subcommand group (`teamai session save`) instead of a
  top-level verb, matching how `roles` / `source` are organized, leaving room
  for `session list` / `session show` later.
- Team upload defaults to counts + tools only; the redacted first-prompt line is
  opt-in via `--include-prompt`, since redact() is best-effort. Local logs keep it.

Review feedback addressed (round 2):
- Fixed user-facing command strings that still said `teamai save-session` (an
  unknown command): the read-only error hint and the retry hint now say
  `teamai session save --push`; aligned docstrings in save-session.ts /
  session-collector.ts / types.ts and the design-doc references.
- `assertNotReadOnly` on the --push path is now wrapped in try/catch: an
  HTTP-mode (read-only) team prints a friendly message and keeps the local log
  instead of throwing an uncaught stack trace.
- Idempotency key is now the full session id in a non-rendering HTML comment
  (`<!-- teamai:session <id> -->`) rather than the 8-char short id, removing the
  ~1-in-4B same-month prefix collision that could silently drop a session.
- Replaced the ad-hoc `Promise.race`/`setTimeout` push guard with the shared
  `withTimeout()` helper (src/utils/async.ts, from #211), which clears its timer
  in a finally so the process doesn't linger after a fast push.

Team upload is opt-in; local logs are pruned after 90 days (Feature 5 retention).

Test Plan:
- npx tsc --noEmit
- npx vitest run (145 files, 1801 tests; 15 in session-collector.test.ts,
  incl. a new "distinct sessions sharing an 8-char id prefix" case)
- npm run build
- real-CLI E2E: seeded ~/.teamai/dashboard/events.jsonl, ran
  `node dist/index.js session save --session-id <sid>`; wrote
  ~/.teamai/session-logs/2026-07.md with folded stats + full-id marker, a `ghp_`
  token in the prompt came out `<REDACTED:gh_tok>`, a second run was idempotent;
  `--push` against a read-only HTTP team printed the graceful message (exit 0, no
  stack trace); old `save-session` errors as unknown; `--include-prompt` appears
  in `session save --help`.

Co-authored-by: Eyre921 <Eyre921@users.noreply.github.com>
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.

2 participants