feat(agent): cap runtime subagent spawn depth at MAX_SPAWN_DEPTH=3#2234
Conversation
Add a task-local depth guard around run_subagent so runtime delegation cannot recurse beyond the documented harness limit. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a task-local spawn-depth tracker (MAX_SPAWN_DEPTH = 3), enforces it in run_subagent (returns SpawnDepthExceeded when exceeded), wraps execution in the scoped context, updates traces, and adds tests and docs reflecting runtime enforcement. ChangesSpawn-Depth Enforcement for Sub-Agent Delegation
Sequence Diagram(s)sequenceDiagram
participant RunSubagent
participant DepthAPI as current_spawn_depth / MAX_SPAWN_DEPTH
participant Checker as Depth Check
participant Context as with_spawn_depth
participant TypedMode as run_typed_mode
participant Provider as ToolProvider
participant Error as SpawnDepthExceeded
RunSubagent->>DepthAPI: read current depth
RunSubagent->>Checker: compute attempted_depth = current + 1
Checker->>Checker: compare attempted_depth to MAX_SPAWN_DEPTH
alt depth exceeded
Checker->>Error: return SpawnDepthExceeded
Error-->>RunSubagent: error
else within limit
RunSubagent->>Context: with_spawn_depth(attempted_depth, ...)
Context->>TypedMode: run_typed_mode (sandboxed)
TypedMode->>Provider: dispatch provider call
Provider-->>TypedMode: result
TypedMode-->>Context: complete
Context-->>RunSubagent: depth restored
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/agent/harness/subagent_runner/ops.rs`:
- Around line 232-234: The code computes attempted_depth using plain addition
(current_spawn_depth() + 1) which can wrap on usize overflow and bypass the
MAX_SPAWN_DEPTH guard; change construction of attempted_depth to use saturating
arithmetic (e.g., current_spawn_depth().saturating_add(1)) so it never wraps and
the MAX_SPAWN_DEPTH check in the spawn path remains effective (alternatively use
checked_add and handle None), updating the binding named attempted_depth and any
uses of current_spawn_depth() accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55b6fb7f-9815-448b-9249-6ae95f95316e
📒 Files selected for processing (7)
gitbooks/developing/architecture/agent-harness.mdsrc/openhuman/agent/harness/harness_gap_tests.rssrc/openhuman/agent/harness/mod.rssrc/openhuman/agent/harness/spawn_depth_context.rssrc/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/harness/subagent_runner/ops_tests.rssrc/openhuman/agent/harness/subagent_runner/types.rs
Prevents usize wrap-around that could bypass MAX_SPAWN_DEPTH guard in release builds. Addresses CodeRabbit review on PR tinyhumansai#2234.
The new with_spawn_depth task-local scope adds a second stacked task_local wrapper around run_typed_mode. Under cargo-llvm-cov instrumentation, the combined future state machine overflows tokio's default 2 MiB per-thread test stack in turn_dispatches_spawn_subagent_through_full_path. Box::pin the inner future so the large state machine lives on the heap.
graycyrus
left a comment
There was a problem hiding this comment.
Solid work — the spawn-depth gate is well-designed and follows the existing sandbox_context pattern cleanly.
| Area | Files | Verdict |
|---|---|---|
| New module | spawn_depth_context.rs |
Clean task-local implementation, good unwrap_or(0) default, scoped nesting works correctly |
| Runtime guard | ops.rs |
Depth check before provider dispatch is correct; saturating_add overflow fix already landed |
| Error variant | types.rs |
SpawnDepthExceeded fits naturally in the existing enum |
| Tests | ops_tests.rs, spawn_depth_context.rs |
Boundary tests (allow at MAX-1, reject at MAX) plus task-local isolation — good coverage |
| Docs | agent-harness.md |
Status flipped from planned to live, accurate |
| Gap tests | harness_gap_tests.rs |
Gap entry correctly removed and pointer added |
The Box::pin wrapper to prevent stack overflow under cargo-llvm-cov instrumentation is a pragmatic fix and benefits production too (deeply nested async state machines).
No findings beyond what CodeRabbit already caught (and which was addressed). Clean PR.
…inyhumansai#2234) Co-authored-by: Cursor <cursoragent@cursor.com>
…inyhumansai#2234) Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
MAX_SPAWN_DEPTH = 3runtime guard aroundrun_subagentso nested sub-agent delegation cannot recurse past the harness contract.SubagentRunError::SpawnDepthExceeded { attempted_depth, max_depth }and a newspawn_depth_contextmodule holding the task-local counter.gitbooks/developing/architecture/agent-harness.mdandharness_gap_tests.rsto flip the depth gate from "planned" to "live".tokio::testcases that pin the boundary: allow atMAX_SPAWN_DEPTH - 1, reject atMAX_SPAWN_DEPTH, and assert the task-local does not leak out of the scope.Problem
The agent harness already validated the spawn hierarchy at loader time via
validate_tier_hierarchy, but the matching runtime depth gate documented ingitbooks/developing/architecture/agent-harness.mdwas not implemented.harness_gap_tests.rsexplicitly listedSpawnDepthExceededas an "untestable because no underlying code exists" gap. A workspace TOML that drops theagent_tierannotation, or any tool / MCP surface that callsrun_subagentrecursively, could therefore recurse past the intendedchat → reasoning → workerchain. The static tier check is insufficient on its own as a defence-in-depth layer.Solution
src/openhuman/agent/harness/spawn_depth_context.rsexposesMAX_SPAWN_DEPTH = 3, aCURRENT_SPAWN_DEPTHtokio::task_local!,current_spawn_depth(), andwith_spawn_depth(depth, future). The module follows the existingsandbox_contextpattern so the additive surface is consistent with the rest of the harness.run_subagentreadscurrent_spawn_depth()before any provider work, computesattempted_depth = current + 1, and returns the newSpawnDepthExceededvariant early if it exceedsMAX_SPAWN_DEPTH. On the allowed path it wraps the innerwith_current_sandbox_mode(...)future insidewith_spawn_depth(attempted_depth, ...)so the counter naturally increments for every nestedrun_subagentcall inside the same tokio task and unwinds when the scope ends.spawn_depthso production traces can confirm the gate is engaged.agent-harness.mdflips the status from "planned" to "live";harness_gap_tests.rsremoves the "no underlying code" entry and points to the new coverage inops_tests.rs.Submission Checklist
runner_allows_spawn_at_max_depth(happy boundary) andrunner_rejects_spawn_beyond_max_depth(failure boundary) insrc/openhuman/agent/harness/subagent_runner/ops_tests.rs, plus three task-local unit tests inspawn_depth_context.rs.cargo testcould not run in this environment becausewhisper-rs-sysrequirescmake, which is not installed on the local machine. The added lines are exercised by the new tests; please rely on CIcargo-llvm-covfor the official diff-cover gate.## Related— N/A: no matrix IDs touched.ScriptedProvidertest double.Closes #NNNin the## Relatedsection — N/A: no tracked issue, opening as a follow-up to the documented gap inharness_gap_tests.rs.Impact
run_subagentcall plus a scopedtask_local!setter; negligible compared with provider/tool work.agent_tier, tools / MCP that callrun_subagentdirectly).#[derive(Error)]. Existing matchers default-branch on it like any otherSubagentRunError. No public function signatures changed.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— N/A: Rust-only change.pnpm typecheck— N/A: Rust-only change.cargo test --manifest-path Cargo.toml runner_— blocked, see "Validation Blocked".cargo fmt --manifest-path Cargo.toml -- --checkpassed;git diff --checkclean.Validation Blocked
command:cargo test --manifest-path Cargo.toml runner_error:whisper-rs-sysbuild script panicked withfailed to execute command: No such file or directory (os error 2) — is cmake not installed?.cmakeis not installed in this local environment.impact:Did not run the focused Rust tests locally. CI will exercise them on itscargo-llvm-covjob; both new tests are deterministic and use the existingScriptedProviderharness.Behavior Changes
run_subagentnow refuses to dispatch a sub-agent beyondMAX_SPAWN_DEPTH = 3nested invocations within the same tokio task, returningSubagentRunError::SpawnDepthExceededearly.Parity Contract
with_current_sandbox_modescope, sandbox mode propagation, max-iteration handling, andmax_result_charstruncation are all preserved verbatim — the change only wraps that future inwith_spawn_depth(...).Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Documentation
Tests