[bug] Spawn-backed subagents drop reasoningEffort; endpoints that require thinking reject every delegation #4666
Replies: 12 comments 1 reply
|
Branch: One-click PR: master...yangwuan55:deepseek-harness:fix/spawn-effort-inheritance |
复现步骤(最小)
对照实验:同一父会话改用 绕过方式(用户侧)换用不强制 thinking 的默认模型(如 deepseek-v4-flash)即可立即解开;但这是绕路由模型差异掩盖缺陷,并非修复。 |
|
Reviewed the branch (
Test quality is good: a direct Three notes, none blocking:
Thanks for the minimal reproduction (the |
|
Operator field note: the smallest rc.2 proof is a three-header comparison, not the parent-facing error string: capture the parent request/header, the fresh spawn child first request/header, and a fork child header for the same bounded task. Matching provider/model with effort present only on parent and fork isolates the route-fidelity gap before credentials, maxTokens, or retries are changed. For containment, fork is reasonable only when inherited completed context is acceptable; otherwise route the spawn-backed tool explicitly to a model whose contract permits omitted effort. A successful fork is a control result, not proof that spawn is repaired. I documented the evidence template, safe containment boundaries, and fourteen regression gates here: https://sandbaseai.github.io/deepseek-harness-handbook/spawn-subagent-reasoning-effort.html (independent community handbook). |
|
Good template — and it aligns with the branch's own coverage: the author's integration test asserts the spawn child's On containment: agreed, and worth scoping the branch precisely — it fixes inherited effort (parent → child). Per-delegation effort overrides are still dropped at the tool-subagent schema boundary (no |
|
The effort-inheritance half is well covered — branch reviewed line by line, and @denial123789's three-header template is the right containment. I want to argue for your fix #2, which has had four comments of attention on #1 and none on it.
And this is a repeating class in this repo, not a one-off. Four instances I have read this week, all the same shape — the system held the diagnostic and dropped it before the person who needed it:
The practical argument for landing #2 with #1 rather than after: effort inheritance is one endpoint-contract mismatch. There will be others — a required parameter some gateway wants, a tool-choice mode some model rejects — and each one, with #1 landed and #2 not, produces the identical bare One implementation note, since forwarding raw provider text to a parent agent is the kind of thing that gets pushback in review: the child's terminal Interest disclosure: I maintain a third-party DSH plugin, and I ship subagent functionality through it, so I am not a neutral party on this topic. That is exactly why I am not proposing anything of mine here — the branch on this thread is the right fix and DSH's native subagent path is not something we touch or could repair. The only thing I am asking for is that the second half of your own report not fall off the end. |
|
This is the strongest argument I've read for #2 being part of the same change rather than a follow-up. Four instances this week across different subsystems, and the shape is identical: the system held the diagnostic and dropped it before the person who could act. The one I can independently confirm from rc.2 source is #902's Your "repeating class, not a one-off" framing also sharpens why #2 is the cheap half. The child's terminal Two notes to make #2 reviewable, since "forward raw provider text to a parent agent" is exactly the kind of thing that will get pushback:
Your disclosure is exactly right, and the argument stands independent of it — the branch on this thread is the fix, and #2 is as much the report as #1. |
|
Thanks for the thorough review — really appreciate the line-by-line check. One blocker on my side: my token only has pull access to this repo, so I can't open the PR myself. If you or anyone with write access wants to take it the rest of the way, the branch is Thanks again! |
|
Thanks for the branch and the offer. On the PR itself, I checked the upstream state: this repo has both issues and PRs disabled ( What I can do, and would offer: I've reviewed the branch and the diagnostic half especially. Two things that would make it land cleanly if/when a channel opens, or if a maintainer picks it up:
If you want, I can build the branch locally and run the existing subagent test suites against it as an extra signal (I can't merge or open a PR, but I can at least confirm it holds up on the test side). Let me know if that's useful. |
|
Following up on the per-call effort gap in this thread: I built and published a community plugin that closes it end to end. @argszero/cordis-plugin-subagent-effort — https://www.npmjs.com/package/@argszero/cordis-plugin-subagent-effort (source: https://github.com/argszero/cordis-plugin-subagent-effort) It works on two layers:
Usage (mount in your profile's - set:
- id: subagent-effort
config:
provider: spawnVerified against the published rc.2 packages with a 13-case smoke test on a real Cordis context (tool lifecycle, request injection, option forwarding). It's deliberately an independent community plugin rather than an upstream change, since PRs are disabled in this repo — and I'd be glad to fold any learnings back into the upstream fix if that path opens. |
EN / EnglishYour root-cause trace matches what we found line-by-line when we were digging into We'd been poking at the same class of bug — option propagation, module-level singletons, lifecycle handoff — and #4793 is where we left our notes on it: Agent disposal does not propagate to continuable subagents Your #4666 is a clean, independently-reproduced instance of the option-propagation half, and the Verified on
中文版 / ZH你这条根因追踪和我们抠 我们之前也一直在抠这一类问题(选项传播、模块级单例、生命周期交接),#4793 就是我们留下的笔记: Agent disposal does not propagate to continuable subagents(Agent dispose 不传播到可续传子代理) 你的 #4666 是"选项传播"这一半一个干净、独立复现的实例;你提的 验证环境:
|
|
Thanks for the maintainer-side confirmation — and good timing: this gap is now fixed natively in
So the option-propagation half of the #4666/#4793 pattern is closed as of the alpha.1 line. My community plugin One loose end the thread raised that I'd still like to see covered: the (Related: the browser-UUID half of this same "weak cross-module contract" theme — #4756 — also landed in alpha.1 via the new |
Uh oh!
There was an error while loading. Please reload this page.
Summary
Fresh spawn-backed subagents silently drop
reasoningEffortwhile inheriting the parent'sprovider/modelroute, producing requests without a thinking parameter. Endpoints whose models always think then hard-reject every delegation.Real-world failure (z.ai GLM
glm-5.3-flash, reasoningEfforts low/high/max):The parent tool only sees
Error: subagent run failed— no diagnostic — because the in-process driver never populatesSubagentResult.diagnostic.Evidence
Two child sessions, same parent, same provider/model (
glm/glm-5.3-flash):subagent_fork{provider, model, reasoningEffort: "max"}subagent(spawn){provider, model}— no effortRoot cause chain (v0.1.1-rc.2 sources):
AgentOptions(packages/core/agent/src/runtime-types.ts) has noreasoningEffortfield at all;resolveChildAgentOptions(packages/subagent/subagent/src/child-agent.ts) documents inheriting "the parent's provider/model/maxTokens route" — effort cannot even ride along;installModelSelection, and a fresh loop has no persistedrequest/headerto restore effort from (agent-loop/src/agent.ts buildRequestseeds{provider, model}only);dsh-llm-pi-aitranslates absence as provider-default) → z.ai rejects.Fork is unaffected because it clones the full session header including the persisted effort.
Proposed fixes (PR incoming)
reasoningEffort?: ReasoningEffortIdtoAgentOptions; inherit it inresolveChildAgentOptions; fall back to it when building a fresh loop's seed config (persisted header keeps precedence).SubagentResult.diagnosticin the in-process driver from the child terminalturn/enderror (LlmFailure.message) so parents see endpoint detail instead of a bare headline.Environment
DSH 0.1.1-rc.2 · agent preset gamedev · tool-subagent provider
spawn(backgroundMode continuable)Filed here because the repository issue tracker is disabled.
All reactions