fix(server): a provider session that never finishes opening fails the run instead of hanging

[shivamhwp]

If a provider never answers while its session is opening, the run stays on "Working" forever, with no error. Stop works here since #14201, but nothing settles the run unless someone notices and presses it.

This adds a 2-minute deadline (PROVIDER_SESSION_OPEN_TIMEOUT) around providerSessions.open in ProviderTurnStartService, and nothing else. A timed-out open fails the run through the same path #14183 already uses for an open that errors, with the message "The provider didn't finish starting within 2 minutes." Turn start, compaction, other outbox effects, the lease and retry counts are unchanged. A timed-out open is retried like any other failed open, so with the worker's 5 attempts a provider that never answers fails the run after about 10 minutes, not 2.

This is a behavior change, so it needs maintainer agreement. That's proposed in #15932. It's the narrow version of #14208, which Julius closed because its deadlines, fail-on-first-timeout and backstop on every outbox effect hadn't been agreed. This PR adds one deadline on one call and keeps the existing retry behavior. The value, retry behavior and error shape are open questions in the discussion.

Known limits, also raised in the discussion:

• The deadline covers everything inside openSession, not only a spawn and handshake. For ACP providers that includes authentication, MCP setup and session load or resume. For ACP registry providers and Antigravity it also includes installing the provider the first time it's used, which currently gets up to 20 minutes. A test with a valid download that takes 130 seconds shows 2 minutes cancels that install. Whether installation should count toward the deadline needs a maintainer decision.
• Each timed-out attempt's provider process now exits, because fix(server): provider sessions clean up when their start is interrupted #15571 closes the session when its open is interrupted. On the rebased branch I tracked the server's own child processes across two timed-out attempts, and none were left behind. This used to depend on fix(server): Stop no longer leaves the provider process running when a session is still opening #15923, which fix(server): provider sessions clean up when their start is interrupted #15571 replaced.

Testing

Real server on an isolated home, real web app, and a fake Grok CLI that never answers session/new.

Before (main at 14fe0158ed): the run stays on "Working" and never settles. This recording watches it for about 105 seconds, sped up about 3x. On October 3 the same setup was still starting after 7 minutes 46 seconds.

Video: https://files.catbox.moe/4lshda.mp4

After: the run fails with "Provider session failed to open" and the timeout message, and the thread is usable again. To keep the clip short, this recording used a throwaway build with the deadline set to 12 seconds, which is why it says "0.2 minutes". The committed value is 2 minutes. A separate run with that value logged "within 2 minutes" on each timed-out attempt. The clip is sped up about 2x.

Video: https://files.catbox.moe/8wmjev.mp4

• vp test run apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts: 20 passed, including 4 new tests that use TestClock and no real sleeps:
  • a hung open fails the run with the message once the deadline passes
  • a hung open on an attempt that will be retried stays starting and hands the error to the worker to retry
  • an open that finishes at 110 seconds still starts the run
  • Stop before the deadline still interrupts the run instead of reporting a timeout
• Without the fix, the hung-open test hangs instead of passing.
• vp run -F t3 typecheck and lint on both files: clean.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a global hard-coded two-minute deadline for provider session startup, changing the default user-visible outcome from indefinite startup to failure and affecting whether the run proceeds. The implementation is small and tested, but the new runtime policy warrants human review.

No code changes detected at b49956b. Prior analysis still applies.

^{You can add or adjust custom eligibility rules. Learn more.}

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	20.9 KiB	20.9 KiB	0 B (0.0%)	29.3 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	−24 B (−0.5%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	−24 B (−2.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	−41 B (−0.2%)	29.3 KiB	✅
Claude	Live turn messages	2	1	−1 (−50.0%)	8	✅

Baseline: d720210 · PR result: b49956b · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 108.5 KiB
• Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 06b6028e-bcbe-4d51-945b-639962ea5b8a

📥 Commits

Reviewing files that changed from the base of the PR and between 250e052 and fa9be04.

📒 Files selected for processing (2)

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

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

---

📝 Walkthrough

Walkthrough

ProviderTurnStartService now times out provider session opening after two minutes and reports ProviderSessionOpenError. Tests cover timeout outcomes, delayed success, and interruption.

Changes

Provider session open timeout

Layer / File(s)	Summary
Apply and verify the session-open timeout apps/server/src/orchestration-v2/ProviderTurnStartService.ts, apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts	ProviderTurnStartService fails an open that exceeds two minutes. Tests cover final and retryable timeout outcomes, completion after 110 seconds, and interruption before the deadline.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to fa9be

The change adds a two-minute deadline to provider startup so a hung open fails the run instead of hanging. It reuses the existing retry and failure handling and has tests for the main outcomes. No concrete merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to fa9be

The deadline improves recovery from stalled starts. However, automatic retries can leave background processes and access tokens alive after a failed start, weakening cleanup and failure containment.

Retained concerns

• Medium · security · inferred: Timeout-driven retries can abandon a partially opened provider process before ownership is registered. The manager's interruption path drops the MCP credential reservation without closing the session scope or revoking a freshly issued credential. ACP initial startup relies on that scope being closed to dispose its runtime. A later retry can therefore acquire the same session key and create another process while the earlier process remains outside normal live-session cleanup. Manual cancellation already exposed this cleanup gap at the base; this PR makes it an automatic, repeatable transition. The security-relevant consequence is weaker containment of provider resources and potentially delegated tool access after visible startup failure, not a demonstrated increase in credential privileges.

Security review details

Security Blast Radius

• inferred — The supported exposure is provider processes on the affected server and credentials issued for the selected thread and environment. A configured provider that stalls its handshake can exercise the automatic timeout-and-retry path. The inspected change does not establish unauthenticated reachability, cross-environment authority or additional credential privileges; repeated permitted starts could nevertheless accumulate unowned resources beyond the per-effect retry bound.

Security Findings and Attack Paths

• inferred — The relevant path is a stalled provider handshake, followed by deadline interruption, reservation release without scope cleanup, and another open attempt. ACP initial startup creates a runtime scope whose finalizers depend on explicit closure; its startup interruption handling resets startup state without closing that scope. This supports an abandoned-process and delegated-access lifecycle concern, not a verified unauthorized tool invocation or data-exfiltration finding.

Trust Boundaries and Controls

• observed — The existing runtime policy and session identity continue to cross into adapter startup unchanged. Credential reuse validates thread, provider instance and optional browser/device capabilities. Ordinary startup errors close the scope and revoke only freshly issued credentials, preserving credentials owned by other live sessions; the interruption path does not apply that cleanup.

Resilience and Maintainability Implications

• observed — Registered-session release closes scopes and performs identity-sensitive credential revocation. ACP replacement-runtime startup also has explicit interruption cleanup. These are meaningful countercontrols, but neither covers the inspected initial acquisition interrupted before a live entry exists.

Hardening Proposals

• proposed — Make initial acquisition interruption-safe: retain cleanup ownership until the live entry is published, close partially created process scopes before retry, and revoke freshly issued credentials without revoking credentials still owned elsewhere. Validate the transition with process-backed timeout and repeated-retry checks, including late responses and concurrent Stop.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing indefinitely hung provider-session opens from leaving runs unresolved.
Description check	✅ Passed	The description covers the problem, implementation, scope, approval discussion, known limits, and focused verification results. It uses a Testing heading instead of Verification, but it provides the r…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}