Skip to content

fix(mcp): bound agent_run provisioning stalls (#419)#443

Open
moonray wants to merge 1 commit into
repoprompt:mainfrom
moonray:fix/agent-run-provisioning-deadline-419
Open

fix(mcp): bound agent_run provisioning stalls (#419)#443
moonray wants to merge 1 commit into
repoprompt:mainfrom
moonray:fix/agent-run-provisioning-deadline-419

Conversation

@moonray

@moonray moonray commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Bounds two unbounded awaits in agent_run / agent_explore provisioning that turned dispatch stalls into ~30-minute MCP client idle timeouts under multi-window load (#419).

HeadlessAgentConnectionGate — the bootstrap amplifier

The gate is a process-wide singleton that serializes every agent bootstrap, and its acquire wait had no deadline. When one start's bootstrap stalled, every subsequent start across all windows queued indefinitely behind it — the "concurrent multi-window load" trigger. (Operationally corroborated: restarting only the MCP server didn't clear it; a full app restart was required — pinning the stuck state to the app process, exactly where this singleton lives.)

  • acquire / acquireWithDiagnostics / waitForClearConnection now take a bounded deadline (default 120s, MCPTimeoutPolicy.headlessAgentConnectionAcquireDeadline). On expiry the queued waiter resumes with .timedOut → returns acquired: false → the runner tears the start down cleanly instead of parking forever.
  • WindowStateComposition.make gains a workspaceSearchService injection seam (mirrors the existing workspaceFileContextStore seam) for deterministic tests.

WorkspaceManagerViewModel.loadWorkspaceFolders — the convergence loop

The search-readiness convergence loop had no iteration cap and was uncancellable by construction (its guards checked hydration but never Task.isCancelled — which is why the MCP execution watchdog's cancellation couldn't rescue the stall). Under multi-window catalog churn the live catalogGeneration keeps advancing, so rebuildIndex chased a moving target indefinitely.

  • Added maxSearchIndexConvergenceRebuilds = 3 cap + a !Task.isCancelled guard. On cap, break to the existing .ready / .degraded decision — safe because startKeepingFresh (subscribed before the loop) self-heals in the background and isStale reporting stays honest.
  • WorkspaceSearchService gains a DEBUG-only setRebuildIndexReportedGenerationTransformForTesting hook (idiomatic; mirrors setSearchDidCaptureGenerationHandler) for deterministic non-convergence tests.

loadRoot's directory walk is intentionally not bounded here — it is a finite computation already covered externally by the watchdog; only a dead mount would hang it, which a Swift cap does not fix.

Design note

agent_run stays .lifecycleManagedCancellable (no dispatch watchdog) by deliberate contract (testAgentRunAndExploreUseLifecycleManagedExemption). This PR bounds the two specific unbounded awaits rather than reversing that contract; an inline watchdog wrap of the @MainActor provisioning path was rejected as fighting the isolation.

Tests

  • HeadlessAgentConnectionGateDeadlineTests — a queued acquire with a short deadline returns false and the waiter is removed from the queue; uncapped it parks forever.
  • WorkspaceSearchIndexConvergenceCapTests — a deterministic non-converging rebuild drives the loop; asserts .switched, elapsed < 10s, and exact rebuildIndex count == 1 + maxSearchIndexConvergenceRebuilds.

Both green on main (post #437 target split — tests import RepoPromptApp). Red state re-confirmed on main: the gate has no deadline and the convergence loop has no cap / no Task.isCancelled guard, so the fix is still needed. Regression suites green (ContextBuilderRunLifecycleTests 10/10, WorkspaceSwitchRecoveryTests 22/22).

Follow-ups

  • Contract ledger: verify-ledger reports missing=2 for these new methods (main itself is currently missing=22, so it's a curation tool, not a CI gate). Happy to add the two rows if you want them curated now.
  • The convergence loop is a candidate for future extraction into a bounded helper; an Oracle review judged that over-engineering for a single call site, so this ships the minimal inline cap.

Refs #419.

🤖 Generated with Claude Code

Two unbounded awaits turned agent_run/agent_explore dispatch stalls into
~30-min MCP client idle timeouts under multi-window load.

- HeadlessAgentConnectionGate: bound acquire/waitForClearConnection with a
  deadline (default 120s, MCPTimeoutPolicy.headlessAgentConnectionAcquireDeadline)
  so a queued bootstrap fails fast instead of parking forever. Kills the
  process-wide amplifier where one stalled start blocked every subsequent start
  across windows. WindowStateComposition.make gains a workspaceSearchService
  injection seam (mirrors workspaceFileContextStore).
- WorkspaceManagerViewModel.loadWorkspaceFolders: cap the search-readiness
  convergence loop (maxSearchIndexConvergenceRebuilds) and add a Task.isCancelled
  guard (the loop was uncancellable by construction — why the watchdog couldn't
  rescue it). On cap, break to the existing readiness decision; the keep-fresh
  listener continues catch-up. WorkspaceSearchService gains a DEBUG-only
  rebuildIndex transform hook for deterministic non-convergence tests.

Tests: HeadlessAgentConnectionGateDeadlineTests,
WorkspaceSearchIndexConvergenceCapTests.

Refs repoprompt#419.
@moonray

moonray commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Re: scope — yes, "deadline + convergence caps" is the intended scope here; the git-pool foreground lane is deliberately deferred, not omitted. Three reasons:

1. The git-admission slice is no longer the hang vector. GitProcessAdmissionController already ships foreground deadline/EDF admission with .deadlineExceeded fail-fast (v1.0.24; GitService auto-attaches a deadline for rootBootstrap/userInitiatedAuthority at GitService.swift:4006/4789/6864). So git admission no longer parks a foreground waiter indefinitely — it errors within the deadline. (Confirmed on main: there's still no reserved foreground lane — the controller's own doc says "Active subprocesses remain non-preemptive" — but the deadline is what stops the hang.)

2. This PR closes the residual non-git hang surface that the git fix exposed. #419's sample was captured on v1.0.24 — with the git deadline — and still stalled. The stall had moved to the non-git awaits in provisioning: the HeadlessAgentConnectionGate bootstrap wait (process-wide singleton, no deadline → the multi-window amplifier) and the loadWorkspaceFolders convergence loop (no cap, and uncancellable by construction — its guards never checked Task.isCancelled, which is why the dispatch watchdog's cancellation couldn't rescue it). agent_run stays .lifecycleManagedCancellable (no dispatch watchdog) per the existing contract (testAgentRunAndExploreUseLifecycleManagedExemption), so any unbounded await there degenerates to the ~30-min client-idle hang. Bounding those two is exactly what this PR does.

3. The foreground lane is a complementary, distinct change. It targets foreground git throughput (work completing promptly under saturation) — #419's named root cause — whereas the admission deadline converts "hang" → "fail-fast" and this PR converts "non-git hang" → "bounded." A reserved/preemptive foreground lane is a deeper admission-fairness change that alters cross-window scheduling for all windows, not just the agent_run path.

Suggested resolution: keep this PR scoped as the hang-bounding fix, and track the foreground lane as the core #419 work (separate follow-up PR, or directly under #419). If you'd rather close #419 in one move, I'm happy to expand this PR to add a reserved foreground lane to GitProcessAdmissionController — but I'd treat that as a separate decision since it changes admission semantics broadly.

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.

1 participant