Skip to content

fix(server): background subagents keep T3 tools after the parent turn ends - #19751

Open
maria-rcks wants to merge 6 commits into
mainfrom
fix/background-subagent-mcp-caller
Open

maria-rcks wants to merge 6 commits into
mainfrom
fix/background-subagent-mcp-caller

Conversation

@maria-rcks

@maria-rcks maria-rcks commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Replaces #19237, moved from a fork head to this repo.

Problem

A Claude background subagent (Agent tool with run_in_background: true) keeps running after the parent turn ends, and it calls T3 tools with the parent thread's MCP credential. Once the parent run settles, assertLiveCaller sees activeRunId === null and every tool that acts as the caller (html_render, preview_open, preview_click, t3_thread_send, ...) fails with parent_not_active: The calling provider no longer owns an active thread run. Agents then fall back to other browsers or hold the parent turn open with blocking waits.

Seen on 0.0.46-nightly.20261013.3042: 10 preview tool failures from 7 Claude background subagents, and one t3_thread_send from a Codex native subagent 6 s after its parent run completed. Each came after the parent run had completed and before the next run started.

Related but separate: #17096 (credential slot left on another provider instance) and #17885 (wake turns stuck behind a held queue).

Change

A thread caller is also live while its shell lists a running background subagent owned by the caller's provider instance (pendingBackgroundTasks with kind: "subagent" and a matching providerInstanceId). Ownership comes from the item's provider thread, including child provider threads, so background work from an earlier provider cannot keep a new provider's credential live. This list is the one that drives the sidebar's Waiting state, and it is already gated on the latest run having settled. The same check backs the cross-thread/cross-project check in OrchestratorMcpService. When the projection has no active run, it decides from one fresh shell snapshot, accepting a matching run that started in between.

Everything else still applies: archived threads, a different provider instance, and credentials revoked when the provider process goes away are all refused. After Stop ends the background work, the thread has no running subagent, so calls are refused again. delegate_task, create_threads and request_secret still need an active run, because they attach to its node.

Verification

Same prompt on a dev server before and after: a Claude parent starts a background subagent, then ends its turn. The subagent runs ping -c 30 and then calls html_render.

  • Before: html_render fails with parent_not_active 33 s after the parent run completed.
  • After: html_render returns htmlRender 33 s after the parent run completed, with no run active.

before: background subagent html_render fails with parent_not_active after the parent turn ended

after: background subagent html_render succeeds after the parent turn ended

before recording

after recording

McpToolAccess.test.ts adds rows to the access table. A caller whose run ended but whose subagent still runs can act and write (ran). It is refused (parent_not_active) when the subagent belongs to an earlier provider, when the caller's provider instance differs, when the thread is archived, and when the subagent has no known owner. The existing "ended" rows still return parent_not_active.

Not covered: a subagent that finished and is then resumed by its own background task. T3 marks it completed and does not see it running again until the next wake run starts, so its calls in that gap still fail. That fix belongs in the Claude adapter's wake handling.

Re-verified on 30869a2

A server built from 30869a2, with Claude Sonnet 5.5 and Codex GPT-6-Astra. Times are UTC.

  • Claude: the parent run completed at 04:05:17.788. The background subagent's html_render ran at 04:05:24.219 and returned htmlRender. The wake run was requested at 04:05:29.008, so no run was active during the call. ping -c 30 failed at once in this sandbox (no cap_net_raw), so the gap is 6 s instead of 30 s.
  • Codex: the parent run completed at 04:11:02.169. Its native subagent called t3_thread_list at 04:11:58.461, after a sleep 40, and got the thread list. No run was active.
  • Refused once the subagent is not running: a Claude subagent ended its own turn (its item completed at 04:07:29.94) while its background sleep ran. When the sleep finished it resumed and called html_render between 04:12:25 and 04:12:32, and got parent_not_active. This is the "Not covered" case below. Pressing Stop on a running subagent ends it, so no call follows the Stop.

30869a2: claude background subagent renders after the parent run completed

30869a2: parent thread with the wake reply quoting the htmlRender result

30869a2: codex native subagent's t3_thread_list result after its parent run completed

30869a2: a subagent resumed by its own background task is refused with parent_not_active

Written by Claude Opus 5.5 (claude-opus-5-5) in Claude Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 15, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 15, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR not approvable — This PR changes the MCP credential-liveness and authorization boundary, allowing background subagents to continue performing actions after the parent run ends, with supporting changes to SQL/in-memory projections and contracts. The access-control impact and broader runtime authority require human review.

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

@github-actions

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 4.9 KiB 5.0 KiB +23 B (+0.5%) 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 +23 B (+1.9%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.8 KiB 20.9 KiB +41 B (+0.2%) 29.3 KiB ✅
Codex Live turn messages 1 2 +1 (+100.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.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: e536bfd · PR result: 30869a2 · 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 15, 2026 •

Copy link
Copy Markdown

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: f89e15eb-9dcf-4fec-9707-17c85babedfa
📥 Commits

Reviewing files that changed from the base of the PR and between e536bfd and 30869a2.

📒 Files selected for processing: 10
  • apps/server/src/mcp/McpToolAccess.test.ts
  • apps/server/src/mcp/OrchestratorMcpService.activity.test.ts
  • apps/server/src/mcp/OrchestratorMcpService.ts
  • apps/server/src/mcp/threadAccess.ts
  • apps/server/src/orchestration-v2/ProjectionStore.test.ts
  • apps/server/src/orchestration-v2/ProjectionStore.ts
  • packages/contracts/src/orchestrationV2.test.ts
  • packages/contracts/src/orchestrationV2.ts
  • packages/shared/src/orchestrationV2PendingBackgroundWork.test.ts
  • packages/shared/src/orchestrationV2PendingBackgroundWork.ts

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

📝 Walkthrough

The change records provider-instance ownership on pending subagent tasks. Projection readers attach that ownership to pending work. MCP caller-liveness checks use the ownership to accept matching background tasks and reject mismatched, archived, or otherwise inactive callers.

Changes: Provider-scoped background caller access

Layer / File(s) Summary
Provider ownership in pending work
packages/contracts/src/orchestrationV2.ts, packages/contracts/src/orchestrationV2.test.ts, packages/shared/src/orchestrationV2PendingBackgroundWork.ts, packages/shared/src/orchestrationV2PendingBackgroundWork.test.ts
The pending subagent task contract accepts an optional provider instance ID. Derived tasks use the owning turn item or provider thread instance, and tests cover ownership and omission cases.
Provider ownership in projections
apps/server/src/orchestration-v2/ProjectionStore.ts, apps/server/src/orchestration-v2/ProjectionStore.test.ts
Projection replay tracks provider-thread ownership. SQL and memory shell readers attach that ownership to pending items. Tests cover ownership changes, cancellation, and rollback.
MCP caller liveness
apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/threadAccess.ts, apps/server/src/mcp/OrchestratorMcpService.activity.test.ts, apps/server/src/mcp/McpToolAccess.test.ts
Caller checks use shell liveness and provider-instance ownership. Tests cover matching and mismatched providers, pending tasks, active runs, archived shells, and unavailable operations.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant MCPCaller
  participant OrchestratorMcpService
  participant ProjectionStore
  MCPCaller->>OrchestratorMcpService: Request caller-scoped operation
  OrchestratorMcpService->>ProjectionStore: Read caller thread shell
  ProjectionStore-->>OrchestratorMcpService: Shell with pending task provider ownership
  OrchestratorMcpService->>OrchestratorMcpService: Check provider instance and shell liveness
  OrchestratorMcpService-->>MCPCaller: Allow or return parent_not_active
Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 30869

The changed caller-access path appears mergeable after normal checks; no actionable risk remains from the reviewed change.

Security architecture risk: 🟡 Moderate · up to 30869

Background work can retain existing tool permissions after the foreground task ends. Ownership and access limits remain enforced, but timely cleanup of background-work state now matters to authorization, and behavior during concurrent provider changes is not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A holder of a valid thread credential gains a longer interval in which to exercise its existing capabilities. This includes caller-scoped actions and existing cross-thread or cross-project orchestration paths, not just preview access. Target runtime and interaction-mode limits remain applicable; the inspected change does not establish newly granted capability categories.

Trust Boundaries and Controls

  • observed — The post-run exception requires a non-archived shell matching the credential's provider instance and a subagent task owned by that same instance. The changed tests specify rejection for old-provider work, provider mismatch, archived callers, and unknown task ownership. Credential replacement additionally revokes prior credentials for the thread before issuing another.

Resilience and Maintainability Implications

  • observed — The inspected Claude paths clear task rosters on task notification, failed or interrupted turn finalization, and native-process reset, then publish the revised provider-thread roster. These are concrete cleanup controls for state that can now sustain tool authority; they do not establish equivalent coverage for every provider adapter.
Pre-merge checks: Passed 4
Passed 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 identifies the server fix that keeps background subagents able to use T3 tools after the parent turn ends.
Description check Passed The description explains the problem, implementation, verification results, limitations, and agent usage. It does not include a dedicated Scope and approval section or explicit maintainer approval, bu…
✨ 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

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

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

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:L 100-499 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