Skip to content

fix(server): stop native subagents after runtime loss - #19544

Open
omar7550 wants to merge 4 commits into
pingdotgg:mainfrom
omar7550:fix/native-subagent-stop-orphans
Open

omar7550 wants to merge 4 commits into
pingdotgg:mainfrom
omar7550:fix/native-subagent-stop-orphans

Conversation

@omar7550

@omar7550 omar7550 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

A native Claude subagent can remain Running after its owning runtime disappears. Targeted Stop now acknowledges proven ownership loss as Interrupted and uses the existing durable settlement path. Partial output stays available, siblings and the completed owner remain unchanged, and delayed Stop receipts cannot settle a newer task instance. A matching live query with an unknown task, missing Stop support, provider errors, or timeouts still fails normally.

Rebuilt from @omar7550’s #19544, preserving the author’s commits and merging current main. The original adapter’s three added tests are reduced to two; the existing missing-session lifecycle case now checks durable Interrupted repair and retained output. No app-owned delegation behavior changes.

Scope and approval: this is the explicit native Stop gap triaged in #17847. Maintainer takeover is authorized by this claim. The change repairs targeted Stop only; it does not broaden task stopping support or change runtime cleanup.

Verification:

  • Three changed behaviors fail against exact main. Temporary proof: six selected Stop cases pass.
  • Expanded case counts across ClaudeAdapterV2.test.ts, ThreadStop.test.ts, and unchanged ProviderTurnControlService.test.ts: main 319 → candidate 321. The test-stage proof passed 320; one existing steer/never-answers test times out on unchanged main and was excluded from that run by CLI filter, without changing or skipping its source.
  • Final implementation: 54 distinct focused Stop/service/adapter cases pass; server, contracts, and provider-core typechecks, changed-file lint/format and diff checks pass (three existing lint warnings). Independent full-diff Opus and Sol reviews found no code blockers (one round). Opus also passed 51 unfiltered service/effect-worker cases and 40 selected adapter Stop/resume cases.
  • Primary rerun of the final candidate’s four native-child Stop outcomes passes, including the real effect-worker missing-session repair.
  • Primary client attempt against exact main: normal startup recovery immediately cancels preloaded historical orphans. This confirms startup cleanup, but cannot demonstrate targeted Stop after a runtime disappears during operation. The repair is proven through the real orchestrator, event store and effect worker in focused tests; no live-provider or client Stop demonstration is claimed. No UI code changed, and the original screenshot was removed because it does not verify this rebuilt head.
  • Not verified with a real Claude runtime, remote/relay/tunnel connection, Electron desktop, macOS, Windows, iOS, or Android.

Models and harness: tests by Codex gpt-6-astra; implementation by Codex gpt-6.1-sol; independent reviews by Claude Opus 5.5 and Codex gpt-6-sol. T3 Code delegation through the Codex and Claude Agent harnesses.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 14, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d1154e7

Macroscope's review found this PR approvable — This is a focused server bug fix that settles orphaned native subagents as interrupted while preserving existing live-runtime safeguards and partial results. The production behavior change is narrowly scoped and backed by targeted regression tests.

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

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 30aad318-7bb3-4245-8cb5-a0c6df0c2406
📥 Commits

Reviewing files that changed from the base of the PR and between d1154e7 and dc2eb76.

📒 Files selected for processing: 7
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts
  • apps/server/src/orchestration-v2/Orchestrator.ts
  • apps/server/src/orchestration-v2/ProviderTurnControlService.ts
  • apps/server/src/orchestration-v2/ThreadStop.test.ts
  • packages/contracts/src/orchestrationV2.ts
  • packages/provider-core/src/server/ProviderAdapter.ts

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

📝 Walkthrough

Subagent Stop handling now returns an interrupted outcome when the provider session or native thread is unavailable. The orchestration settlement path accepts this status and omits the result for interrupted outcomes. Tests cover adapter behavior and settlement state.

Changes: Native subagent Stop handling

Layer / File(s) Summary
Interrupted outcome contract and no-session fallback
packages/provider-core/src/server/ProviderAdapter.ts, packages/contracts/src/orchestrationV2.ts, apps/server/src/orchestration-v2/ProviderTurnControlService.ts
The subagent outcome contracts now accept interrupted. When no live provider session exists, provider turn control returns an interrupted outcome with the current time and a null result.
Claude adapter ownership checks
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
Claude stop handling returns an interrupted outcome when the native thread has no matching live query or the subagent belongs to an earlier process. Tests cover historical orphan and unknown-task cases.
Interrupted outcome settlement
apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/orchestration-v2/ThreadStop.test.ts
Orchestration omits the result when settling an interrupted outcome. Stop settlement tests check the status, timestamp, retained partial result, messages, owner run, sibling, and child state.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to dc2eb

Native subagents whose runtime has disappeared now settle as Interrupted instead of staying Running, and partial output is kept. No actionable merge-blocking risk was found. Live-provider validation was not performed.

Security architecture risk: 🔵 Low · up to dc2eb

The change is narrowly scoped to acknowledging interrupted work after runtime ownership is lost. Existing ownership and stale-request checks remain, and partial results are preserved. Complete termination of native work during failed or delayed cleanup remains unconfirmed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed settlement scope is the selected durable native subagent and its associated child projection. The inspected path does not grant authority over additional provider instances, siblings or the completed owning run.

Trust Boundaries and Controls

  • observed — Clients cannot directly submit the widened internal settlement command through the client command contract. For matching live Claude queries, unknown task identity, missing Stop support, provider errors and unconfirmed outcomes still fail rather than being converted into interrupted success.

Resilience and Maintainability Implications

  • inferred — A missing session establishes loss of registered runtime ownership, not independently verified termination of all native execution. The pre-existing release path removes the session before closing its runtime scope, and closure can time out. The PR changes terminal bookkeeping, not that cleanup ordering; no resulting increase in native execution authority was established.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the server fix for stopping native subagents after runtime loss. It is concise, specific, and aligned with the main change.
Description check Passed The description explains the problem and expected behavior, describes the implementation scope, provides issue and approval references, and includes detailed verification results and limitations. It d…
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 explains the problem and expected behavior, describes the implementation scope, provides issue and approval references, and includes detailed verification results and limitations. It does not use explicit Problem and Change headings, but the required information is present and the description is otherwise complete.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one. I’ll rebuild the targeted orphan Stop repair on current main, preserve the ownership safeguards and partial results, and run focused tests plus independent reviews before pushing here. Thanks @omar7550 for the implementation and reproduction.

@juliusmarminge juliusmarminge changed the title fix(server): Stop clears native subagents whose runtime is gone fix(server): stop native subagents after runtime loss Oct 19, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@juliusmarminge

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants