fix(core): reject nested background requests - #7593
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with solid evidence. Issue #7571 documents a real session where an explicit Direction: aligned. Failing fast with a clear error is the right call — it follows the existing precedent for nested forks (same Size: 3 files changed, ~17 production logic lines (agent.ts guard clause + description updates), ~20 test lines, 1 docs line. Well under any threshold. Approach: the scope feels right — one guard clause following an established pattern, with matching updates to the tool description, system prompt, docs, and tests. No unrelated changes, no scope creep. The implicit-background downgrade path is correctly preserved for cases where the model didn't explicitly request background. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 bug,证据充分。Issue #7571 记录了一个真实会话:嵌套上下文中显式 方向:对齐。快速失败并给出明确错误是正确的做法——遵循嵌套 fork 的现有先例(相同的 规模:3 个文件变更,约 17 行生产逻辑(agent.ts 守卫子句 + 描述更新),约 20 行测试,1 行文档。远低于任何阈值。 方案:范围合理——一个遵循已有模式的守卫子句,配套更新工具描述、系统提示、文档和测试。无无关改动,无范围蔓延。隐式后台降级路径被正确保留。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I'd add a guard clause right after the existing nested-fork rejection, checking Comparison with the diff: the PR does exactly this. The guard is at the right location (after fork rejection, before No blockers found. The change is minimal and follows the established pattern precisely. Real-Scenario TestingDrove the real product in tmux with the same prompt on both builds. The prompt forces a nested subagent to explicitly request Before (installed build, v0.20.1)The explicit After (this PR via
|
|
Confidence: 5/5 — clean across every stage; the problem is real, the fix is minimal and follows the established pattern, and the before/after proves it works. This is a textbook small fix for a well-documented bug. The silent foreground downgrade violated the execution-mode contract — the caller explicitly asked for background and got foreground with no feedback, turning a recoverable context mistake into main-session blocking and 991K tokens of unintended consumption. The fix does exactly what the issue triage recommended: apply the same The implementation is tight — one guard clause, strict The before/after tmux test is the clearest evidence: on the installed build, the nested agent ran silently in the foreground and the model reported "background subagent has officially completed" with no idea the mode changed. On this PR, the request is rejected at the tool-call level with an actionable error, and the model correctly understands and reports the limitation. 中文说明置信度:5/5 — 每个阶段都很干净;问题真实存在,修复最小化且遵循已有模式,before/after 证明修复有效。 这是一个针对充分记录 bug 的教科书式小修复。静默前台降级违反了执行模式契约——调用者显式请求后台却得到前台且无任何反馈,将可恢复的上下文错误变成主会话阻塞和 991K tokens 的意外消耗。修复完全按照 issue 分诊建议执行:应用嵌套 fork 已使用的 实现紧凑——一个守卫子句,严格 before/after tmux 测试是最清晰的证据:已安装构建上,嵌套代理静默在前台运行,模型报告"后台子代理已完成"完全不知道模式被更改。本 PR 上,请求在工具调用层被拒绝并返回可操作的错误,模型正确理解并报告了限制。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
Rejects an explicit
run_in_background: truerequest from a nested subagent instead of silently running the subagent in the foreground. Implicit nested launches and nested launches withrun_in_background: falsecontinue to run inline.Why it's needed
A nested launcher cannot receive background completion notifications. Silent foreground fallback therefore blocks its parent even though the caller requested detached execution. Returning a failed tool result makes the limitation actionable and preserves the existing foreground behavior when background mode was not explicitly requested.
Reviewer Test Plan
How to verify
From a nested subagent, request a regular subagent with
run_in_background: true; confirm the call returns an error explaining that background execution is only available to top-level sessions and that no child is launched. Omit the flag in the same context; confirm the result still arrives inline. Request a nested fork with the flag set; confirm the existing nested-fork error remains the result.Evidence (Before & After)
N/A - this has no TUI surface. The focused unit test covers the error result, scheduler failure display, no agent construction or registry registration, the omitted-flag foreground path, and the existing nested-fork priority.
Tested on
Environment (optional)
Windows 11 with Node.js v24.11.1.
packages/core/src/tools/agent/agent.test.tspassed 197/197.Risk & Scope
trueretain their foreground behavior in nested contexts.false.Linked Issues
Fixes #7571
中文说明
此 PR 的作用
当嵌套子代理显式请求
run_in_background: true时,拒绝该请求,而不是静默地在前台运行子代理。隐式嵌套启动以及设置run_in_background: false的嵌套启动仍会以内联方式运行。为什么需要此改动
嵌套启动器无法接收后台完成通知。因此,即使调用方请求了分离执行,静默回退到前台仍会阻塞其父级。返回失败的工具结果能让这一限制可操作,并保留未显式请求后台模式时原有的前台行为。
审阅者测试计划
如何验证
从嵌套子代理中请求一个普通子代理,并设置
run_in_background: true;确认调用返回说明后台执行只适用于顶层会话的错误,且没有启动子代理。在同一上下文中省略该参数;确认结果仍以内联方式返回。请求一个设置该参数的嵌套 fork;确认结果仍为已有的嵌套 fork 错误。证据(前后对比)
不适用 - 此改动没有 TUI 界面。聚焦单元测试覆盖错误结果、调度器失败显示、不构造子代理或注册表登记、省略参数时的前台路径,以及现有嵌套 fork 的优先级。
已测试环境
环境(可选)
Windows 11,Node.js v24.11.1。
packages/core/src/tools/agent/agent.test.ts197/197 通过。风险与范围
true的调用在嵌套上下文中仍保留前台行为。false。关联 Issue
修复 #7571