Sign up
View stargazersSign up

fix(server): explain background wait registration timeouts#33658

open1
main
GitHub fork: t3/background-wait-timeout
opened15h agobyVvoltcrash
2 files changed
View on GitHubTwo-way sync enabledPR last imported 3h ago

Problem

t3_background_wait reports its 15-second registration timeout as “Failed to update the awaited background work.” Agents cannot distinguish that timeout from an ordinary provider failure or see that they can retry.

Change

Report registration timeouts separately, with guidance to retry using the same task IDs. Keep the existing deadline, underlying cause and caller interruption behavior.

Scope and approval

Fixes #33646. This is a small, focused correction to error classification and guidance for an existing operation. Provider locking and wait registration behavior stay the same.

Verification

  • The regression failed on unchanged upstream: expected timed-out, received failed. The fixed service suite passes all 29 tests.
  • env -u DISPLAY -u WAYLAND_DISPLAY vp run check:changed upstream/main passes, including three affected package typechecks and 31 tests.
  • A native-clock probe using the real service, in-memory SQLite and synthetic session boundaries reached the actual 15-second deadline. Before: generic failed with TimeoutError. After: timed-out, retry guidance, one adapter invocation and cancellation. Success, ordinary provider failure and caller interruption retained their behavior. The MCP handler passes the service message through unchanged.

Actual loaded-Claude lock contention remains unproven. A real-model retry eval was not run because the eval runner's credentials/proxy were unavailable; no model-level retry success is claimed.

Reviewed with review-loop: 1 round with Claude Opus 5.5 on xhigh. Reviewer verdict: approved.

Created with GPT-6.1 Sol in Codex through T3 Code.

Checks

23 successful · 8 skipped
All checks passed
d5be223

Added 1 commit

  • fix(server): distinguish background wait registration timeoutsd5be223
MmacroscopeappBot15h ago
Synced from GitHub

Approvability

Verdict: Approved at d5be223

Macroscope's review found this PR approvable — Background-wait registration timeouts now receive a specific message with retry guidance. The existing deadline and underlying registration behavior remain unchanged, limiting exposure to error reporting.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot15h ago
Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Background wait timeouts now return a distinct timed-out error with retry guidance. Tests cover timeout, provider failure, interruption, and existing unavailable and unsupported cases.

Changes: Background wait error handling

Layer / File(s)Summary
Timeout error contract and classification
apps/server/src/orchestration-v2/ThreadManagementService.ts		The error schema now accepts timed-out, with a message that advises retrying with the same task IDs. awaitBackgroundTasks maps timeout causes to this reason and other failures to failed.
Background wait failure tests
apps/server/src/orchestration-v2/ThreadManagementService.test.ts		Tests cover provider errors, timeout at 15 seconds, interruption, and existing unavailable and unsupported cases. They verify provider calls use the active thread with awaited: true.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • pingdotgg/t3code#24610: Introduced t3_background_wait and its 15-second timeout path, which this change updates.

Suggested labels: bugfix, size:S

Suggested reviewers: t3dotgg, macroscopeapp

Merge Risk: External image Minimal · up to d5be2

The background wait timeout now gives retry guidance, with no identified issue that needs resolution before merge.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly identifies the server fix for background-wait registration timeouts.
Description checkPassedThe description covers the problem, change, scope and linked issue, and detailed verification results. It also states unverified limitations.
Linked Issues checkPassed#33646 asks for registration timeouts to be identified as timeouts and to tell the agent to retry. ThreadManagementService.ts adds the timed-out reason, classifies timeout causes separately, and g…
Out of Scope Changes checkPassedThe changes are limited to background-wait error classification and its tests. Both changes directly support #33646. No unrelated changes are present in the reviewed diff.
Full details: Linked Issues check

Explanation

#33646 asks for registration timeouts to be identified as timeouts and to tell the agent to retry. ThreadManagementService.ts adds the timed-out reason, classifies timeout causes separately, and gives retry guidance using the same task IDs. Tests cover the 15-second timeout, its cause, ordinary provider failures, and caller interruption. The existing timeout and interruption behavior remain intact.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between 9a3f8db9b39704cd1adb522babda23c21de8e1e4 and d5be223a79b77e09bed01927c0878b0ed344c08c.

External image Files selected for processing: 2

  • apps/server/src/orchestration-v2/ThreadManagementService.test.ts
  • apps/server/src/orchestration-v2/ThreadManagementService.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.