fix(orchestrator): Claude plan-mode plan proposals + real provider error causes#3696
Conversation
ClaudeAgentSdkQueryRunnerError.message now appends the underlying defect message (and the failing method) instead of a fixed string, so provider error cards show the actual reason (missing cwd, spawn failure, resume failure) via makeProviderFailure's Error-message extraction. The three run-path warn/error logs that printed 'cause: [Object]' now render Cause.pretty output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Plan-mode Claude sessions were launched without the SDK permission callback, so ExitPlanMode was never available and the plan interaction mode could only produce prose — no plan artifact, no Plan Ready state, no approve flow. Install the callback in plan mode and intercept ExitPlanMode in canUseTool: emit the proposed plan as v2 plan/node/turn item artifacts (status active, so hasActionableProposedPlan flips) and deny the tool call with guidance to end the turn, keeping the session in plan mode until the user approves from the app. Flip the adapter's emitsProposedPlan capability accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:trusted
… only Effect service conventions require wrapper messages to come from stable structural attributes, never from cause.message. Keep the runner error message structural (method only) and instead unwrap the underlying SDK Error at the provider-failure boundary, where makeProviderFailure's existing redaction/bounding pipeline renders it — the error card keeps showing the actionable cause without the error class stringifying its defect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…estion A single event that fails ingestNormalized (e.g. a late async-subagent notification referencing an already-finalized run) used to tear down the whole Stream.tap pipeline, leaving the session's event routing dead and the next run wedged at 'starting' with no recovery except a server restart. Failed non-terminal events are now logged and dropped while the pipeline keeps draining; root turn.terminal failures still escalate so the outer fallback finalization is preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lization The SDK reports async Task completion via a system/task_notification message that usually arrives after the parent turn finalized — when activeTurn is null — or while an unrelated turn is active, so it was silently dropped: the subagent projection stayed 'running' forever and RunExecutionService kept the run's ingestion pipeline waiting on a child that could never terminalize. Retain the turn contexts of tasks that are still running when a turn finalizes (keyed by native task id) and route task_notification to the owning context regardless of the current active turn, emitting the subagent/node/turn-item updates under the original run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… failure Resuming a Claude session pins resumeSessionAt to the recorded native conversation head. After a dirty shutdown the killed process may never have flushed that head to its transcript, so the first query on the resumed session fails (and only an unexplained manual retry recovered). When a pinned query dies before producing any SDK message, mark the native thread so the next open resumes without the pin — one-shot, so healthy pins keep working and a failing unpinned open re-suppresses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Medium
The Effect.catchCause added around the per-event handler swallows non-turn.terminal failures, but trackChildLifecycle(event) runs after the catch block. When a routed provider_turn.updated or subagent.updated event fails ingestion, it is logged and dropped before trackChildLifecycle executes, so the child is never added to activeChildProviderTurns or activeChildSubagents. After the root terminal arrives, shouldStopProviderEventIngestion then returns true and Stream.takeUntilEffect stops the subscription early, skipping later child events that should still be ingested. Consider moving trackChildLifecycle(event) before the Effect.catchCause wrapper so child lifecycle tracking happens regardless of ingestion failures.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/RunExecutionService.ts around line 678:
The `Effect.catchCause` added around the per-event handler swallows non-`turn.terminal` failures, but `trackChildLifecycle(event)` runs *after* the catch block. When a routed `provider_turn.updated` or `subagent.updated` event fails ingestion, it is logged and dropped before `trackChildLifecycle` executes, so the child is never added to `activeChildProviderTurns` or `activeChildSubagents`. After the root terminal arrives, `shouldStopProviderEventIngestion` then returns `true` and `Stream.takeUntilEffect` stops the subscription early, skipping later child events that should still be ingested. Consider moving `trackChildLifecycle(event)` before the `Effect.catchCause` wrapper so child lifecycle tracking happens regardless of ingestion failures.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eeb5f95. Configure here.
| if ( | ||
| failedQuery !== null && | ||
| failedQuery.openedWithResumeSessionAt && | ||
| !failedQuery.receivedSdkMessage |
There was a problem hiding this comment.
Resume pin suppression never runs
High Severity
The new resumeSessionAt recovery logic isn't working as intended. When a pinned resume query fails, queryContext is cleared prematurely, preventing finalizeActiveTurnAfterQueryExit from registering the failure and updating suppressResumeSessionAt. This causes subsequent resume attempts to fail repeatedly, requiring manual intervention.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit eeb5f95. Configure here.
| cause: Cause.pretty(eventCause), | ||
| }, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Dropped ingest skips child lifecycle
Medium Severity
When ingestNormalized fails, the new Effect.catchCause swallows the error for non-terminal events but the whole tap body aborts before trackChildLifecycle, so a dropped terminal subagent.updated never clears activeChildSubagents and the root ingest stream can stay blocked after turn.terminal.
Reviewed by Cursor Bugbot for commit eeb5f95. Configure here.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces significant new runtime behavior including Claude plan mode proposals and error recovery mechanisms. Multiple unresolved review comments identify potential bugs in the new logic, including a high-severity issue with the resume pin suppression mechanism. You can customize Macroscope's approvability policy. Learn more. |
1 participant
What Changed
Five focused bug fixes for the Claude adapter / run execution in the v2 orchestrator (this PR's branch):
Surface real causes in Claude provider failures —
ClaudeAgentSdkQueryRunnerError.messagewas a fixed string ("Claude Agent SDK query failed."), so every provider-error card showed that and nothing else, and the run-path logs printedcause: [Object]. The error message now appends the failing SDK method and the underlying defect message, and the three warn/error logs on the run path renderCause.prettyoutput.Let Claude plan mode propose approvable plans — plan-mode sessions were launched with
installPermissionCallback: false, so the SDK never exposedExitPlanModeand plan mode could only ever end in prose: no plan artifact, no "Plan Ready" state, no approve/implement flow (orchestration_v2_projection_plansstayed empty). The callback is now installed in plan mode, andcanUseToolinterceptsExitPlanMode: it emits the proposed plan as v2 node/plan/turn-item artifacts (status: "active", which flipshasActionableProposedPlan) and denies the tool call with guidance to end the turn, so the session stays in plan mode until the user approves from the app.emitsProposedPlancapability flipped totrue, regression test added for the plan-mode query policy.Drop poison provider events instead of killing ingestion — one event failing
ingestNormalized(e.g. a late async-subagent notification referencing a finalized run) tore down the wholeStream.tappipeline, leaving the session's event routing dead and the next run wedged atstartinguntil a server restart. Failed non-terminal events are now logged and dropped; rootturn.terminalfailures still escalate to the existing fallback finalization. Regression test included.Resolve async subagent completions after turn finalization — the SDK's
system/task_notificationfor an async Task usually arrives after the parent turn finalized (activeTurn === null), so it was silently dropped: the subagent projection stayedrunningforever andRunExecutionServicekept waiting on a child that could never terminalize. Turn contexts with still-running tasks are now retained (keyed by native task id) and late notifications resolve against the owning context, emitting the subagent/node/turn-item updates under the original run.Skip a stale resume pin after a dirty-shutdown query failure — resuming pins
resumeSessionAtto the recorded conversation head; after a dirty shutdown that head may never have been flushed to the native transcript, so the first query failed and only an unexplained manual retry recovered. A pinned query that dies before producing any SDK message now marks the native thread so the next open resumes unpinned (one-shot).The
ClaudeAgentSdkQueryRunnerErrormessage is derived from structural fields only (per the Effect service conventions); the underlying SDK error reaches the provider-failure card viamakeProviderFailure's existing redaction/bounding pipeline instead.Known follow-up (not in this PR): a stop/cancel affordance for runs stuck in
queued/preparing/starting.Why
Found while testing this PR end-to-end with the Claude provider:
transport_error · "Claude Agent SDK query failed." · code: null), and the logs gave nothing more.makeProviderFailurealready extractsError#message; the information was being discarded at the error-class layer.claudeAgentwas a dead-end: the model itself reports it has noExitPlanModetool, writes its plan as chat prose, and the plan interaction mode can never hand off to the build flow. With this fix the full loop works: plan request → plan card + "Plan Ready" pill → Implement → build run executes the plan.Both verified live against a scratch project (screenshots below); full server suite passes (1199 tests).
UI Changes
No component changes — both fixes are server-side, but they change what the existing UI displays.
Provider error card (same failure class — a project whose workspace folder was deleted). Before: the bare one-liner with
code: null. After: the card names the failing SDK method and the real cause.Plan mode. Before: the plan exists only as chat prose — no plan card, no Plan Ready, nothing to approve — and the model states it has no
ExitPlanModetool. After: a proposed-plan card is emitted, the thread flips to "Plan Ready", and Implement kicks off the build run that executes the plan (card + pill mid-flow state in the collapsed section below).Extra: the proposed-plan card + "Plan Ready" pill + Implement button state (mid-flow)
Checklist
🤖 Generated with Claude Code
Note
Medium Risk
Changes core orchestration behavior (event ingestion, Claude session resume, plan/subagent lifecycle) in the v2 run path; mistakes could wedge runs or mis-route subagent/plan state, though failures are mostly isolated with logging and tests.
Overview
Hardens the Claude v2 adapter and run execution path with five targeted fixes.
Plan mode now installs the SDK permission callback in plan mode and handles
ExitPlanModeby emitting proposed-plan node/plan/turn-item artifacts (then denying the tool so review stays in-app). CapabilityemitsProposedPlanis set to true.Provider failures unwrap
ClaudeAgentSdkQueryRunnerErrorto the underlyingErrorformakeProviderFailure, and run-path logs useCause.prettyinstead of opaque cause objects.Async subagents retain turn context for running tasks after parent finalization so late
task_notificationmessages still terminalize subagents on the original run.Resume skips a stale
resumeSessionAtpin when a pinned query fails before any SDK message (one-shot per native thread).Run ingestion logs and drops non-terminal events that fail normalization instead of killing the stream; root
turn.terminalfailures still escalate. Regression tests cover plan-mode query policy and poison-event handling.Reviewed by Cursor Bugbot for commit eeb5f95. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix Claude plan-mode plan proposals and provider error cause unwrapping
emitsProposedPlantotrueinClaudeProviderCapabilitiesV2and installs the permission callback in plan mode so the adapter can interceptExitPlanModetool calls, emit aproposed_planartifact, and keep the session in plan mode.claudeProviderFailureCauseto unwrap the underlyingErrorfromClaudeAgentSdkQueryRunnerError, so provider failure logs now report the real SDK error cause rather than the wrapper.ActiveClaudeTurnContextfor async tasks after a turn finalizes sosystem.task_notificationmessages received late are still resolved correctly.RunExecutionService(logging a warning) while still aborting on rootturn.terminalfailures.resumeSessionAtfails before any SDK message arrives, the next open for that native thread suppresses the resume pin and logs a warning.Macroscope summarized eeb5f95.