镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(server): Claude sessions survive slow SessionStart hooks - #23202

Open
juliusmarminge wants to merge 8 commits into
mainfrom
fix/issue-23151
Open

juliusmarminge wants to merge 8 commits into
mainfrom
fix/issue-23151

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Problem

A Claude turn in a new worktree fails while a legitimate SessionStart setup hook is still running: T3 gives MCP registration 90 seconds, but Claude does not answer that request until the hook completes. In the Linux web dev app with Claude Code 2.1.303, a cold-worktree-only 105-second hook reproduced the failure at 90.2 seconds; the same fixture worked directly in Claude and from its main checkout.

Change

Increase the Claude MCP registration deadline to 690 seconds, allowing the default 600-second hook timeout plus server connection time and slack. Add one regression test that completes registration after 105 seconds. The deadline remains bounded; an unresponsive CLI can now occupy an effect-worker slot for up to 11.5 minutes instead of 1.5 minutes. Hooks explicitly configured beyond this allowance can still time out.

Scope and approval

Fixes https://git.cafe/pingdotgg/t3code/pulls/23151

Triaged bug via the automated issue flow; GitCafe mirror #23155. This fixes the reproduced startup failure in the existing Claude adapter. Both initial isolated reviewers returned MERGE. The subsequent GPT review and Macroscope require human approval for the timing policy: an unresponsive CLI may occupy one of four effect-worker slots for longer. That approval remains pending. No client rendering or tool capabilities change.

Verification

  • Merged main into 52c7dcd. Resolved the timeout comment conflict while preserving the 690-second deadline and single regression test. Focused query-runner tests pass 4/4; scoped lint has no errors and one pre-existing unused layer warning. Formatting and PR-scoped diff checks pass. CI on this head has failed Lint and the aggregate Check. Both PR files pass targeted lint (also with CI=true), formatting and whitespace checks. The Lint job also runs Knip, generated-code checks and vp check; the failing substep is unknown without failed-step logs. No speculative code change was made; the preceding ff91e40 passed CI. Human timing-policy approval remains pending.

  • Red proof: unchanged regression test fails on main plus the test, with “Claude Code did not answer the MCP server registration.” Failing output.

  • Green: vp test run apps/server/src/orchestration-v2/Adapters/ClaudeAgentSdkQueryRunner.test.ts: 3/3 passed, including after merging current main into 13b0993.

  • Follow-up 4ef439b nests the regression test’s layer provisioning to avoid Effect diagnostic TS377033 (multipleEffectProvide). A minimal compiler fixture reproduces exit 2 before and exit 0 after; the focused test file still passes 3/3. CI failed on that earlier head without accessible failed-step diagnostics; the preceding merged head passed CI; CI is pending after the latest main merge. Production code is unchanged from the runtime-tested fix.

  • Targeted lint passed with one pre-existing unused layer warning; git diff --check passed. Scoped server typecheck was stopped after overloading the VM and has no result.

  • Server-only proof matrix is empty: no render paths changed. Runtime captures below were made on b5a226e and 7b67539. The production timeout change remains identical on the latest merged head; the regression test has only provisioning and import adjustments. Focused tests passed again on the merged head.

Attempt Before After
Cold linked worktree, 105-second SessionStart hook Registration failed after 90,208.79 ms Registration succeeded after 106,767.69 ms; reply ok
Controls on main Main checkout, warmed worktree and a resumed follow-up registered in about 0.8–1.0 seconds No control behavior was changed

Before: Before: cold worktree turn fails at the registration deadline

After: After: cold worktree turn completes following its startup hook

Before recording · After recording. These are supplemental illustrations; the runtime transcript and regression test are the server proof. Chromium headless, Linux x64, 1280×800, light theme. The before recording begins partway through the wait; the after recording shows the whole attempt. No rendering changes are claimed.

Proof index and runtime transcript · Empty render matrix.

Unverified: reporter’s macOS desktop surface, native Windows and mobile; head controls, live-query reuse, the exact 690-second timeout/cleanup boundary, and remote/relay/tunnel execution. Reproduction used isolated empty development state because the real-data snapshot had incompatible migration names. Executing revisions are corroborated by reflog chronology and runtime traces, without a contemporaneous process-to-full-SHA attestation. The proof auditor used local artifact counterparts after public downloads returned HTTP 403. A first after attempt was interrupted by a dev-server restart and is excluded from the success claim.

Implemented with Claude Opus 5.5 through Claude Agent SDK; tests with Claude Opus 5.5; reviewed with GPT-6.1-Sol through Codex and Claude Opus 5.5 through Claude Agent SDK. Triage and evidence coordinated with GPT-6.1-Sol through Codex in T3 Code.

juliusmarminge and others added 3 commits October 21, 2026 23:42
…ude open

Tests reproduce issue 23151: a Claude Code SessionStart hook that runs past
90 s makes ClaudeAgentSdkQueryRunner.open fail with "Claude Code did not
answer the MCP server registration."

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude Code reads mcp_set_servers only after its SessionStart hooks finish.
A hook can run up to the CLI's default 10 minute hook timeout, for example
a cold worktree setup script, so the 90 s registration deadline failed the
open while the CLI was healthy. The deadline now covers that hook budget
plus the per-server connect time. The SDK still rejects the request if the
CLI exits, and interrupting the open still closes the query.

Fixes #23151

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 22, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — Claude sessions with MCP servers now allow 11.5 minutes rather than 90 seconds for registration, accommodating slow SessionStart hooks. This changes the default startup deadline and can extend the wait before a stalled registration fails.

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

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

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

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 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 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 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: 034f3ba · PR result: da798c0 · 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

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The Claude MCP registration timeout increases from 90 to 690 seconds. Tests cover successful opening after registration resolves at 105 seconds and existing timeout recovery, retry, and failure cases with the longer deadline.

Changes: Claude MCP registration timeout

Layer / File(s) Summary
Configure and test the registration timeout
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/ClaudeAgentSdkQueryRunner.test.ts
The timeout increases to 690 seconds. The comments describe SessionStart-hook delays and per-server connection timeouts. Tests cover registration resolving after 105 seconds, stalled-registration recovery, process-exit retry behavior, and failure after repeated timeouts.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

  • pingdotgg/t3code#23151 — The issue reports startup failure when a SessionStart hook exceeds the former 90-second deadline. This change raises that deadline.

Possibly related PRs

  • pingdotgg/t3code#22365 — Both changes address Claude MCP registration-timeout handling and its effects on session startup.
  • pingdotgg/t3code#25372 — Both changes cover timeout recovery and process cleanup in the Claude adapter and its tests.
  • pingdotgg/t3code#30166 — Both changes modify Claude SDK startup deadlines, including MCP registration timing.

Suggested reviewers: bil0000

Merge Risk: Minimal · previous review 96d68

Claude sessions can wait longer for slow startup hooks. No concrete issue currently prevents merging, though the longer wait is an operational tradeoff.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: allowing Claude sessions to survive slow SessionStart hooks.
Description check Passed The description includes all required sections: Problem, Change, Scope and approval, and Verification. It provides reproduction details, implementation scope, test results, runtime evidence, known lim…
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.
Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It provides reproduction details, implementation scope, test results, runtime evidence, known limitations, and unresolved CI and approval items.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 52c7dcd and 96d6890.

Files selected for processing: 2

  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts
  • apps/server/src/orchestration-v2/Adapters/ClaudeAgentSdkQueryRunner.test.ts

Limit details: You’ve used all 10 included reviews currently available.

…ted pipes

tsc-rs fails the server typecheck on effect(multipleEffectProvide) when one
pipe chains two Effect.provide calls. Match the neighbouring fork test's shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Member Author

The new regression test triggered Effect diagnostic TS377033 (multipleEffectProvide) in a minimal compiler reproduction: chained provisioning exits 2; nested provisioning exits 0. 4ef439b applies the existing nested test pattern. Focused tests pass 3/3; CI confirmation is pending. The production timeout change is unchanged.

The longer deadline's tradeoff remains for maintainer approval: an unresponsive Claude process can hold an effect-worker slot for up to 690 seconds. No automatic landing is enabled.

Copy link
Copy Markdown
Member Author

CI on 4ef439b has 12 failed checks: Build and Typecheck, Lint, Test, all eight server shards, and Check. Cafe provides conclusions and timestamps, but no failure output or annotations; its job-log endpoint rejects the GitHub check IDs. No current failure cause is established. Several other checks were skipped, including Test Web and Rust.

Diagnosis needs the failed step, first error, and exit code from the CI run, starting with Lint, Build, and Server 1. No further code change was made.

Resolve the query runner test conflict. Main's resumed-session retry tests
now advance the clock by the exported registration deadline instead of a
literal 90 seconds, and the slow SessionStart regression uses the shared
real query runner with a fresh session ID, the case the retry never covers.

AI-assisted; verified with the tests and tools listed above.
@juliusmarminge

Copy link
Copy Markdown
Member Author

Evidence

Configure a startup SessionStart command hook with timeout 600 and a first-run setup script that sleeps 105 seconds. Create a fresh linked-worktree Claude thread and send reply ok. Repeat with another fresh worktree on the other commit using the same driver.

Environment on both sides: real web app and Claude provider, separate fresh databases, Chromium 1280×800, light theme, synthetic data. Chat captures exclude private metadata. Videos begin approximately 28 seconds into startup.

Before After
a0efdaeb89 96d6890a9b
Before: Claude MCP registration timed out After: Claude answered ok after slow setup
Startup recording Startup recording
Failed after 90.675 seconds, before setup completed. Hook completed after 105 seconds; ok arrived after 110.029 seconds.

Reproduction driver · Persisted timing transcript

AI-assisted; verified with the tests and tools listed above.

Not shown:

  • The full 690-second registration allowance was not exercised; this comparison verifies the reported 105-second startup hook.

@juliusmarminge

Copy link
Copy Markdown
Member Author

The automated pipeline stopped on this pull request and will not merge it:

someone else pushed to fix/issue-23151 (head is now da798c0); the pipeline does not build on commits it did not review

It stays open for a person to finish or close.

@t3dotgg

t3dotgg commented Nov 11, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Fable 5.2 responding on behalf of Theo

#30166 merged as 86e2cb2 and covers most of this PR. This branch now conflicts with main in ClaudeAdapterV2.ts.

What changed on main: the 90 second registration limit starts after Claude Code finishes startup, and startup has its own 5 minute limit (CLAUDE_BOOT_TIMEOUT).

Measured on the merge commit with the real Claude CLI (2.1.322) and Agent SDK 0.3.321: a SessionStart / startup hook that sleeps 100s no longer fails. ClaudeAgentSdkQueryRunner.open succeeds after 100.3s. The 105 second fixture in this PR's evidence is in that range.

What is left: a hook that runs for more than 5 minutes. This PR allows 690 seconds, and main allows 300 seconds for startup. If that case matters, the smaller change is now to raise CLAUDE_BOOT_TIMEOUT, not CLAUDE_MCP_REGISTRATION_TIMEOUT.

My judgment: this PR is superseded unless hooks longer than 5 minutes need support. I did not close it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:gate-failed A land gate failed size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants