Repository navigation
fix(core): hide subagent tools at the configured depth limit - #56521
rudycelekli wants to merge 2 commits into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Looks good. The context, compaction and generate hooks now drop the subagent tool once the session's depth reaches experimental.subagent_depth, and the check that rejects the call at run time stays.
How I checked: I ran the new test cases against the base code, and 9 of them fail there (the tool is still offered to children at the limit and at depth 0). With this PR, all 26 tests in tool-subagent.test.ts pass, and packages/core type-checks.
Public API: unchanged.
One thing worth fixing: the depth calculation is now in two places (see the inline comment).
| Effect.gen(function* () { | ||
| const tool = event.tools[name] | ||
| if (!tool) return | ||
| let current = yield* sessions.get(event.sessionID).pipe(Effect.orDie) |
There was a problem hiding this comment.
This repeats the ancestry walk and limit lookup from execute above. #53356 happened because the prompt and the run-time check disagreed, so could both use one small helper (e.g. const atDepthLimit = (sessionID) => ...)? Then the tool is hidden exactly when a call would be rejected, and the two can't drift apart. The helper could also handle a missing parent session in one place. Right now the hook uses Effect.orDie, so a missing parent would break prompt building instead of returning the tool error.
Signed-off-by: Rudy Celekli <[email protected]>
Issue for this PR
Closes #53356
Type of change
What does this PR do?
Remove the subagent tool from context, compaction, and generation hooks when session ancestry reaches the configured depth. Retain the registered execution guard.
How did you verify your code works?
The original source-bound qualification exercised PluginSupervisor, Session ancestry, Tool registry and PluginHooks: nine regression failures and six controls before; all 15 matrix cases passed after.
The requested review update shares one depth-limit predicate across execution and all three hooks. Current native Linux qualification passed all 26 existing subagent tests both before and after the consolidation; full Core passed 6,351 tests with 345 skips, package typecheck and root check (34/34 tasks) passed, and the exact patch identity was unchanged through validation. No tests were added by the consolidation.
Missing-parent handling remains unchanged. This update does not establish or claim an ordinary missing-parent defect or change that policy. Hosted validation is separate from upstream CI authorization; this Mac did not run the full candidate.
Screenshots / recordings
No visual UI code changed; behavior is asserted directly.
Checklist