fix(core): preserve background agent launch flags - #5061
Conversation
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. — qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, test plan included. On direction: this is a clear correctness fix. Background agents silently inheriting the parent session's flags on resume is a real bug — an agent launched with sandbox or a specific model should resume with those same settings, not whatever the parent happens to have now. Claude Code shipped the exact same fix ("Background sessions now preserve On approach: the scope is tight and appropriate. Extending Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必要章节齐全,中英双语,包含测试计划。 方向:这是一个明确的正确性修复。background agent 在 resume 时悄悄继承父会话的 flag 是真实 bug——用 sandbox 或特定 model 启动的 agent,恢复时应该保持原设置,而不是变成父会话当前的配置。Claude Code 已经发布了完全相同的功能("Background sessions now preserve 方案:范围紧凑合理。在 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewI read the PR description and independently proposed how I'd solve this before looking at the diff. My approach: extend What I looked at: The core logic lives in three functions: No blockers found. A few observations:
Test ResultsAll focused tests pass: Typecheck and lint clean: Real-Scenario TestingAttempted E2E via tmux with However, the unit test artifacts confirm the sidecar is being written correctly at launch: The Tmux Capture中文说明代码审查在阅读 PR 描述后,我先独立构思了解决方案再对照 diff。我的方案是:扩展 未发现阻塞性问题。 测试结果所有 132 个测试通过,typecheck 和 lint 均无问题。 真实场景测试通过 tmux 尝试了 E2E 测试,但 headless 父会话直接处理了 prompt 而未派发 background agent,因此无法触发完整的 kill/resume 流程。不过单元测试产物确认 sidecar 在启动时被正确写入(见上方 JSON),resume 路径的 flag 恢复由专门的测试用例覆盖。 — Qwen Code · qwen3.7-max |
ReflectionStepping back: this PR fixes a genuine correctness bug where background agents silently lose their launch-time configuration on process restart. The impact is real — an agent launched with sandbox protection could resume without it, or silently switch to a different model. These are the kinds of bugs that are hard to notice and harder to debug. My independent proposal matched the PR's approach exactly: typed flag snapshot in What I'm confident about:
What I'd note:
Verdict: approve. This is a clean, well-tested correctness fix that ships what it promises. 中文说明反思这个 PR 修复了一个真实的正确性 bug:background agent 在进程重启后会悄悄丢失启动时的配置。影响是实际的——带 sandbox 启动的 agent 可能恢复后失去 sandbox 保护,或者悄悄切换到不同的 model。这类 bug 难以发现,更难调试。 我独立构思的方案与 PR 实现完全一致:在 确认:bug 真实且修复直接命中;范围最小化;向后兼容处理得当;132 个测试全部通过,typecheck 和 lint 干净;sidecar 在启动时正确写入 flag(从测试产物确认);代码遵循项目现有模式。 注意:headless 模式下无法完成完整的 kill/resume E2E 测试(模型直接处理了 prompt 而未派发 background agent)。单元测试充分覆盖了 resume 路径,但维护者做一次手动 TUI 测试会增加信心。 结论:通过。这是一个干净、测试充分的正确性修复。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Local end-to-end verification report (maintainer test pass)Built this PR locally and exercised the real TUI kill/resume flow under tmux with live model sessions — the exact end-to-end path the PR lists as "not validated / out of scope". The fix works as described, including the legacy-sidecar fallback. LGTM from a runtime-behavior standpoint. This also adds the macOS datapoint missing from the PR's test matrix. Environment: PR head Test designThe key trick: session B (the resuming parent) was deliberately started with a model that is unauthorized in my environment (
Request-log timeline for the resumed agent (T2), Also checked in passing: both Scope notes (what this pass did not observe directly)
中文版(Chinese version)本地端到端验证报告(维护者测试)本地构建本 PR,在 tmux 中以真实模型会话走通了完整的 TUI kill/resume 流程——正是 PR 自述「未验证 / 不在范围内」的端到端路径。修复行为与描述一致,旧 sidecar 兼容回退也成立。从运行时行为角度 LGTM。 同时补上了 PR 测试矩阵中缺失的 macOS 数据点。 环境: PR head 测试设计关键技巧:会话 B(执行 resume 的新父进程)故意使用一个在我环境中未授权的模型(
T2 resumed agent 的请求日志时间线( 顺带检查:两处 范围说明(本轮未直接观察到的部分)
|
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
What this PR does
Preserves the launch-time background-agent runtime flags in the agent meta sidecar and reapplies them when the interrupted agent is resumed after a process restart.
The persisted allow-list covers the flags from #4884 that affect resumed execution behavior: approval mode, bare mode, sandbox, screen reader mode, model, max session turns, and max tool calls. The resume path applies these overrides before rebuilding the child tool registry, so a restored bare-mode agent gets the matching tool surface instead of only restored getter values.
Why it's needed
Today the resume path creates the resumed agent config through prototype delegation from the current parent session. That means an agent launched with one runtime shape can resume with the flags from a later parent process. For example, a background agent originally launched with a sandbox or a specific model can silently resume without that sandbox or under the parent's new model.
This keeps old sidecars backward compatible: sidecars without the new persisted flag snapshot continue to fall back to the current behavior. Existing
resolvedApprovalModereconciliation, including folder-trust downgrades, is preserved.Reviewer Test Plan
How to verify
Run the focused core tests around background-agent resume and agent config overrides. The new tests assert that persisted flags are written at launch, restored during resume, and applied before tool-registry rebuild.
Commands used locally:
npm ci npm run test --workspace=packages/core -- agents/background-agent-resume.test.ts tools/agent/agent-override.test.ts tools/agent/agent.test.ts npm run typecheck --workspace=packages/core npx eslint packages/core/src/agents/agent-transcript.ts packages/core/src/agents/background-agent-resume.ts packages/core/src/agents/background-agent-resume.test.ts packages/core/src/tools/agent/agent.ts packages/core/src/tools/agent/agent-override.test.ts packages/core/src/tools/agent/agent.test.ts --max-warnings 0 git diff --check upstream/main..HEADEvidence (Before & After)
N/A — this is a non-UI runtime correctness fix. The added tests cover the before/after behavior directly.
Tested on
Environment (optional)
Windows 11, Node/npm from the local development environment.
Risk & Scope
Linked Issues
Fixes #4884
中文说明
这个 PR 做了什么
这个 PR 会把 background agent 启动时的运行 flag 保存到 agent meta sidecar,并在进程重启后 resume 该 agent 时重新应用这些 flag。
持久化的 allow-list 覆盖 #4884 中会影响恢复执行行为的 flag:approval mode、bare mode、sandbox、screen reader mode、model、max session turns 和 max tool calls。resume 路径会在重建子 agent 的 tool registry 之前应用这些 override,所以恢复 bare-mode agent 时拿到的是匹配的工具集,而不只是 getter 值被恢复。
为什么需要
当前 resume 路径通过原型链从当前父会话派生 resumed agent config。这会导致一个用旧 flag 启动的 background agent,在之后的父进程里被恢复时悄悄继承新的父会话 flag。例如原本带 sandbox 或指定 model 启动的 agent,可能恢复成没有 sandbox 或使用父会话的新 model。
这个改动保持旧 sidecar 兼容:没有新 flag 快照的旧 sidecar 仍然走当前 fallback 行为。现有
resolvedApprovalMode调和逻辑,包括 folder trust 被撤销时的降级,也保持不变。Reviewer Test Plan
如何验证
运行 background-agent resume 和 agent config override 的核心测试。新增测试会验证启动时写入持久化 flag、resume 时恢复 flag,以及在 tool registry 重建前应用 flag。
本地已运行:
npm ci npm run test --workspace=packages/core -- agents/background-agent-resume.test.ts tools/agent/agent-override.test.ts tools/agent/agent.test.ts npm run typecheck --workspace=packages/core npx eslint packages/core/src/agents/agent-transcript.ts packages/core/src/agents/background-agent-resume.ts packages/core/src/agents/background-agent-resume.test.ts packages/core/src/tools/agent/agent.ts packages/core/src/tools/agent/agent-override.test.ts packages/core/src/tools/agent/agent.test.ts --max-warnings 0 git diff --check upstream/main..HEADEvidence(Before & After)
N/A。这是非 UI 的运行时正确性修复,新增测试直接覆盖 before/after 行为。
Tested on
Environment(可选)
Windows 11,本地开发环境中的 Node/npm。
风险与范围
关联 Issue
Fixes #4884