Sign up
View stargazersSign up

fix(claude): reject waits on native subagent commands#25015

closed4
main
GitHub fork: fix/claude-subagent-background-wait
opened15d agobyZZheruel
3 files changed
View on GitHubTwo-way sync enabledPR last imported 1h ago

Foreground native Claude subagents lose their background commands and Monitors when they return. t3_background_wait accepted those tasks anyway, so a subagent could promise a continuation that would never arrive.

The tool now returns those task IDs in unmatched and tells subagents to finish their commands before returning. Since Claude can omit parent IDs or later ownership flags, the adapter retains ownership across Bash timeouts and task-list refreshes. It also clears awaited marks when a subagent owner appears.

Regression tests fail on the original implementation. Local SDK and MCP replays exercised wait results, permissions, and validation. No live Claude session was run; the provider boundary was replayed locally.

Local MCP replay
text
1{"taskIds":["root","owned","missing"]}
2=> {"matched":["root"],"unmatched":["owned","missing"]}

Important

The guard rejects waits on all native-subagent-owned tasks. T3 already requires native Claude subagents to run in the foreground. T3-delegated children have separate sessions and keep their existing waits.

Closes #24956

Model: gpt-6.1-sol with high reasoning effort. Harness: Codex in T3 Code.

Checks

7 successful · 7 skipped
All checks passed
1667893

Added 4 commits

  • fix(claude): reject waits on native subagent commands2570cef
  • fix(claude): retain native task ownership through backgrounding8d353e2
  • test(claude): trim redundant background wait checksc1090a3
  • Merge branch 'main' into fix/claude-subagent-background-wait1667893
CcoderabbitaiBot15d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 2ed92618e60f5ee6ecc694eb834c28105a502d09 and 166789315a3d1dd2630e8830af757bd3e5355e91.

External image Files selected for processing: 3
  • apps/server/src/mcp/toolkits/orchestrator/tools.ts
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts

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

Walkthrough

The Claude adapter now tracks subagent task ownership across task events and roster updates. It excludes subagent-owned tasks from awaited-task marking. The tool description and adapter instructions clarify how native subagent background work behaves.

Changes: Subagent task ownership

Layer / File(s)Summary
Task ownership metadata
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts		Internal task records capture subagent ownership from SDK metadata and parent task IDs. Provider-facing task projections omit the internal ownership marker.
Ownership across task events and rosters
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts		Task events and roster updates preserve ownership, including when snapshots omit it. Task completion and thread task-state clearing remove ownership tracking.
Wait eligibility and guidance
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts, apps/server/src/mcp/toolkits/orchestrator/tools.ts		Awaited-task marking excludes subagent-owned tasks. The adapter instructions and tool description explain that native subagent background work does not persist after return. Tests cover ownership updates and awaited-task matching.

Priority: External image Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: bugfix

Suggested reviewers: juliusmarminge, t3dotgg, macroscopeapp

Merge Risk: External image Minimal · up to 16678

The change is mergeable after normal checks. The supplied evidence does not identify an actionable failure in native-subagent background waits.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main change: rejecting waits for native Claude subagent commands.
Description checkPassedThe description covers the problem, change, scope, issue reference, verification results, limitations, and agent details. It clearly states that no live Claude session was run.
Linked Issues checkPassedIssue #24956 requires t3_background_wait to avoid accepting tasks owned by a foreground native subagent and to warn the subagent before it returns. ClaudeAdapterV2.ts tracks subagent ownership fro…
Out of Scope Changes checkPassedThe changes stay within issue #24956. The adapter changes preserve ownership needed to enforce the foreground-subagent wait guard. The tool-description change documents the guard. The added tests veri…
Full details: Linked Issues check

Explanation

Issue #24956 requires t3_background_wait to avoid accepting tasks owned by a foreground native subagent and to warn the subagent before it returns. ClaudeAdapterV2.ts tracks subagent ownership from owned_by_subagent and parent_task_id, retains that ownership across task-list refreshes and Bash timeout transitions, removes owned tasks from the awaited set, and returns them as unmatched. The injected Claude instructions and the t3_background_wait description direct native subagents to finish commands before returning and to use delegate_task for work that must outlive the turn. Regression tests cover the unmatched result, ownership retention, and awaited-mark clearing.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #24956. The adapter changes preserve ownership needed to enforce the foreground-subagent wait guard. The tool-description change documents the guard. The added tests verify the guard and its ownership edge cases. No unrelated product behavior or unrelated files appear in the reviewed changes.

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.

JJulius Marminge7h ago

Thanks @Zheruel for the earlier work on this foreground-subagent stall. This PR has been stale since October 25, and main has changed the affected adapter since then. Closing as a duplicate of #25036 by @juliusmarminge, which addresses the same false wait acceptance with regression coverage and real runtime evidence.