refactor(core): centralize worktree config derivation - #9915
Merged
Merged
Conversation
yiliang114
requested review from
LaZzyMan,
doudouOUC,
tanzhenxin and
wenshao
as code owners
August 24, 2026 10:25
Collaborator
Collaborator
|
Re-run on
Moving on to code review. 🔍 中文说明在
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewThe PR content at this head is byte-identical to what I fully reviewed and approved at
Files changed (19 of 19 shown)
TestingEvidence below is the PR's own CI fetched via the API for the reviewed commit; per triage policy no PR code is built or executed here, and checks are fetched once — no polling.
The unit suite pins each behavior round 1 broke. What it cannot compose is a live spawn end-to-end: sandboxed verification would settle that — Real-scenario (tmux) testing: N/A — internal refactor with no user-visible behavior change; there is nothing a terminal session could show before vs after. 中文说明代码审查本 head 上的 PR 内容与我在
（改动文件总表见英文版。） 测试以下证据来自按受审提交抓取的 PR 自身 CI（API 获取）；按 triage 策略，此处不构建、不执行任何 PR 代码，检查只抓取一次、不轮询。
（CI 结果表见英文版区域标记内，由 finalize 工作流在 CI 完成后原地更新。） 单测套件钉住了 round-1 破坏的每一项行为；套件无法组合出真实启动的端到端链路：沙箱验证可以补上—— 真实场景（tmux）测试：N/A——内部重构、无用户可见行为变化；终端会话里不存在可对比的前后差异。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — the change content is byte-identical to what was fully reviewed and approved at Stepping back: nothing in this re-run changed my read of the PR. My independent proposal for "the worktree rebind overlays and the per-agent overlay drift apart" was exactly this shape — one derivation boundary per concern, paired getter/private-field rebinding as a single operation, approval lifecycle owned by a dedicated profile, and a lint tripwire so the next hand-rolled overlay can't sneak in. The delta since the approved head is only the merge with current main: I diffed the PR-against-main diff at both heads and every added/removed line matches, and the two conflict resolutions (the test mock shape in Verdict: approve — deferred until CI lands. The 中文说明Confidence: 4/5 —— 改动内容与在 退一步看：这次 re-run 没有改变我对 PR 的判断。我对"两份 worktree 重绑定覆盖层与 per-agent 覆盖层互相漂移"的独立方案正是这个形状——每个关注点一条派生边界、成对 getter/私有字段重绑定作为一次操作、approval 生命周期由专用 profile 拥有、外加 lint 门禁让下一个手写覆盖层无法溜进来。距已批准 head 的变化只有与当前 main 的合并：我把两个 head 相对 main 的 PR diff 逐行比对，新增/删除行完全一致，两处冲突解决（ **结论：approve——等 CI 落定后生效。**受审提交上的 （延迟批准标记见英文版，供 finalize 工作流在 CI 变绿后执行。） — Qwen Code · qwen3.8-max Reviewed at |
This was referenced Aug 24, 2026
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
previously approved these changes
Aug 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
* refactor(core): centralize agent config derivation * refactor(core): centralize scoped config profiles (#9920)
qwen-code-ci-bot
requested changes
Aug 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Comment on lines
+14
to
+17
| function isObjectCreate(node) { | ||
| return ( | ||
| node.callee.type === 'MemberExpression' && | ||
| !node.callee.computed && |
Collaborator
There was a problem hiding this comment.
[Suggestion] The rule only catches literal Object.create(...) in modules that import the Config name from a path ending in /config/config.js. Verified bypasses: (1) aliased/computed calls — const create = Object.create; create(base), const O = Object; O.create(base), Object['create'](base) (the !node.callee.computed + Identifier checks skip all three); (2) non-Object.create derivation — { __proto__: base }, Object.setPrototypeOf({}, base); (3) modules that overlay a Config without importing the Config name itself (importsConfig keys on the import specifier, not the argument type). Cost: the next hand-rolled overlay — the exact regression this rule exists to prevent — lands undetected when written in one of these shapes. Suggested fix: document the known bypasses beside the rule (it reads as a deliberate tripwire), or broaden detection: also flag computed Object['create'] and module-scope aliased create bindings, and/or key on any import from a path ending /config/config.js rather than the Config specifier specifically.
中文说明
该规则只能捕获在从 /config/config.js 结尾路径导入了 Config 名称的模块中的字面 Object.create(...)。已验证的绕过方式：(1) 别名/计算调用——const create = Object.create; create(base)、const O = Object; O.create(base)、Object['create'](base)（!node.callee.computed + Identifier 检查会跳过这三种）；(2) 非 Object.create 的派生——{ __proto__: base }、Object.setPrototypeOf({}, base)；(3) 没有导入 Config 名称本身、却对 Config 做覆盖层的模块（importsConfig 以导入说明符为判据，而不是参数类型）。代价：下一个手写覆盖层——正是这条规则要防止的回归——以这些形态写出时会不被发现地落地。建议修复：在规则旁记录已知的绕过方式（它读起来像一个有意为之的绊线），或者扩大检测：同时标记计算形式的 Object['create'] 和模块作用域内别名的 create 绑定，以及/或者以任意来自 /config/config.js 结尾路径的导入为判据，而不限于 Config 说明符。
— qwen3.8-max via Qwen Code /review (v0.22.0)
… and dispatch configs Give every per-agent config child-local approval state, not just the ones spawned with an explicit mode: tools bind to the config and teammate mode switches (Shift+Tab) plus 'Proceed always' confirmations call setApprovalMode on it, which the derived-Config guard rejects without an approval profile. Snapshot the base mode when none is requested so no AUTO strip is acquired and cleanup stays a no-op. Layer an approval profile over the workflow-orchestrator's derived dispatch contexts (worktree isolation / workingDir) as agent.ts does, with cleanup in the outer finally.
… Config Derived agent profiles rebind targetDir to their own workspace, but the plan file stays in the owning base's configured plans directory. The containment assertions in savePlan/loadPlan compared against the derived workspace and threw FatalConfigError for teammates whose cwd differs from the parent project root, which savePlanBestEffort swallowed into a debug warning — silently dropping the plan. Resolve the anchor to the Config that owns the plans-directory state instead.
Mirror the deriveWorktreeConfig pairing test: assert the public getter overrides (getCwd/getProjectRoot/getWorkingDir/getTargetDir, plan-file path) and the paired private targetDir/cwd field writes plus own workspaceContext/fileDiscoveryService, so a refactor that drops any rebind fails loudly instead of leaking parent-tree reads into spawned agents.
…create
Mirror no-core-root-barrel-config.test.js: load the real eslint.config.js via
new ESLint({overrideConfigFile}) and lintText virtual packages/core/src/**
paths, so a future edit to the flat-config wiring (plugin key, rule name,
ignores) that silently stops the gate applying to packages/core/src/** turns
this test red instead of degrading the invariant with no signal.
Collaborator
Author
|
Closeout — round 1 (cap-4 + scope correction):
Also merged current origin/main (ded65e6). One non-force push 9fd6266..ded65e6. No /triage — the push auto-triggers re-review. |
Collaborator
|
Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. A transient error is retried automatically; if you are seeing this, retry with |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 95 passed · 0 failed · 95 total Flakiness gate: ✅ 8 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：95 通过 · 0 失败 · 95 总计 抖动门：✅ 8 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9915 Deep Verification — refactor(core): centralize worktree config derivationVerdict: 中文摘要结论：
Central claim and A/B proofCentral claim. Both worktree launch paths (agent.ts provisioned isolation; workflow-orchestrator One harness (
Key reading: the C3 flip is not just preservation — on the base arm the dispatch path really does drop the plan (the containment assertion anchored at the rebound CorrectionsNone needed (first round; no prior bot claims to correct). Findings (informational; none blocking)F1 — behavior change (intended, measured): per-agent F2 — behavior change (intended, pinned): teammates spawned without an explicit approval mode snapshot the parent mode instead of prototype-following it. Base's raw F3 — lint gate coarseness (as designed; one unpinned axis). Mutation matrix (vacuity of the new/changed tests)Every mutant reverted a real hunk in a scratch copy, ran, then restored (final
M2 + M7 are the layered pair: the guard exception and the own-method install are each independently load-bearing (reverting either alone breaks the transition), so no single-hunk survivor exists to reclassify. Positive controls: every kill quotes expected-vs-actual values; the two M6-negative tests ("allows deriveConfig…", "ignores unrelated…") correctly stayed green under M6, proving the harness discriminates. Reverse mutation: see F3. Targeted gates
Not covered
MethodologyEnvironment: the CI verify container (node:22-bookworm), working tree at Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
previously approved these changes
Aug 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ Round-1 Criticals verified fixed; CI green on the reviewed commit.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
eslint-rules/no-config-object-create.js:4 — [probe] no-config-object-create misses Config imported via the same-directory './config.js' specifier
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Comment on lines
+999
to
+1002
| const approvalHandle = deriveApprovalModeConfig( | ||
| effectiveContext, | ||
| config.getApprovalMode(), | ||
| ); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The approval profile layered over derived dispatch contexts here — and the approvalCleanup?.() call in the outer finally — has no test pairing. I mutation-tested it at this commit: deleting this block and the cleanup call leaves the entire workflow-orchestrator suite green (BASE(mutated): Tests 169 passed (169)), while a control mutation corrupting deriveWorktreeConfig's getTargetDir rebind turns exactly two of this PR's new tests red (CONTROL: Tests 2 failed | 167 passed (169)) — so the suite can see the worktree rebinds but not this layering. If a future refactor drops this block (or the cleanup call), any setApprovalMode during a workflow dispatch throws Derived Configs cannot change approval mode from the bare-derived guard, and a dispatch that does transition into AUTO leaks the strip on the parent's PermissionManager; no test turns red under either mutation. The sibling InProcessBackend change got a dedicated regression test ("lets a teammate without an explicit approval mode switch modes child-locally"); this orchestrator-side equivalent got none. Suggested fix — add an orchestrator test that exposes the recorded dispatch runtimeContext and asserts setApprovalMode(...) on it does not throw and leaves the parent's getApprovalMode() unchanged, mirroring the InProcessBackend test:
it('lets a derived dispatch context switch modes child-locally', async () => {
// ... drive a dispatch with isolation: 'worktree' and capture runtimeContext
expect(() => runtimeContext.setApprovalMode(ApprovalMode.AUTO_EDIT)).not.toThrow();
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT); // parent untouched
});中文说明
这里叠加在派生派发上下文之上的 approval profile——以及外层 finally 中的 approvalCleanup?.() 调用——没有任何测试配对。我在本提交上做了变异测试：删除这个代码块和 cleanup 调用后，整个 workflow-orchestrator 测试套件仍然全绿（BASE(mutated): Tests 169 passed (169)）；而对照变异——破坏 deriveWorktreeConfig 的 getTargetDir 重绑定——恰好让本 PR 新增的两个测试变红（CONTROL: Tests 2 failed | 167 passed (169)）——说明套件能看见 worktree 重绑定，却看不见这层 approval 叠加。如果未来某次重构删掉这个块（或 cleanup 调用），workflow 派发期间的任何 setApprovalMode 都会撞上裸派生 Config 守卫抛出 Derived Configs cannot change approval mode；真正切换到 AUTO 的派发还会在父级 PermissionManager 上泄漏 AUTO strip——两种变异下都没有测试变红。同批的 InProcessBackend 改动有专门的回归测试（"lets a teammate without an explicit approval mode switch modes child-locally"），orchestrator 这一侧的等价物却没有。建议修复——新增一个 orchestrator 测试：暴露记录的派发 runtimeContext，断言对它调用 setApprovalMode(...) 不抛异常、且父级 getApprovalMode() 保持不变，与 InProcessBackend 的测试对称。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Comment on lines
+16
to
+17
| node.callee.type === 'MemberExpression' && | ||
| !node.callee.computed && |
Collaborator
There was a problem hiding this comment.
[Suggestion] R1-6: Still stands at this commit — the rule only catches literal Object.create(...) in modules that import the Config name from a path ending in /config/config.js. Verified bypasses, re-checked against the unchanged rule: (1) aliased/computed calls — const create = Object.create; create(base), const O = Object; O.create(base), Object['create'](base) (the !node.callee.computed + Identifier checks skip all three); (2) non-Object.create derivation — { __proto__: base }, Object.setPrototypeOf({}, base); (3) modules that overlay a Config without importing the Config name itself (importsConfig keys on the import specifier, not the argument type). Cost: the next hand-rolled overlay — the exact regression this rule exists to prevent — lands undetected when written in one of these shapes. Suggested fix: document the known bypasses beside the rule (it reads as a deliberate tripwire), or broaden detection: also flag computed Object['create'] and module-scope aliased create bindings, and/or key on any import from a path ending /config/config.js rather than the Config specifier specifically.
中文说明
R1-6：在本提交上仍然存在——该规则只能捕获在从 /config/config.js 结尾路径导入了 Config 名称的模块中的字面 Object.create(...)。已对未改动的规则重新核实绕过方式：(1) 别名/计算调用——const create = Object.create; create(base)、const O = Object; O.create(base)、Object['create'](base)（!node.callee.computed + Identifier 检查会跳过这三种）；(2) 非 Object.create 的派生——{ __proto__: base }、Object.setPrototypeOf({}, base)；(3) 没有导入 Config 名称本身、却对 Config 做覆盖层的模块（importsConfig 以导入说明符为判据，而不是参数类型）。代价：下一个手写覆盖层——正是这条规则要防止的回归——以这些形态写出时会不被发现地落地。建议修复：在规则旁记录已知的绕过方式（它读起来像一个有意为之的绊线），或者扩大检测：同时标记计算形式的 Object['create'] 和模块作用域内别名的 create 绑定，以及/或者以任意来自 /config/config.js 结尾路径的导入为判据，而不限于 Config 说明符。
— qwen3.8-max via Qwen Code /review (v0.22.0)
# Conflicts: # packages/core/src/agents/runtime/workflow-orchestrator.test.ts # packages/core/src/memory/skillReviewAgentPlanner.ts
wenshao
approved these changes
Aug 27, 2026
yiliang114
enabled auto-merge
August 27, 2026 11:58
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 146 passed · 0 failed · 146 total Flakiness gate: ✅ 8 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：146 通过 · 0 失败 · 146 总计 抖动门：✅ 8 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9915 Deep Verification (round 2) — refactor(core): centralize worktree config derivationVerdict: 中文摘要结论：
Previous findings (round 1 → this round)
Delta since round 1 (what this round probed additionally)The new head is the conflict-resolving merge of
Central claim and A/B proofCentral claim. Both worktree launch paths rebind workspace state through the shared derived-Config boundary behavior-preserving: every public cwd/project getter and the private field reads behind them resolve to the child, the parent Config stays unchanged, and approval-mode transitions keep working through the layered approval profile. Secondaries: plans-anchor fix (S1), child-local approval state on every per-agent config (S2), One harness (
Key readings:
CorrectionsNone. (Round-1 claims re-checked and hold; no bot claims about this PR were found inaccurate.) Findings (informational; none blocking)F1/F2/F3 — carried over, statuses in the table above. No new blocking findings this round. The only behavioral deltas at head remain the three intended ones (drift fix, snapshot semantics, guard coverage expansion) plus the C3 latent-bug fix, all pinned by tests whose load-bearingness the mutation matrix re-proved at this head. Mutation matrix (vacuity of the new/changed tests, re-run at new head)Every mutant applied an exact hunk revert to the head source, ran its killer suite, and restored the file (runner verified byte-identical restoration for each; final
No survivors. Positive controls: M1/M3/M4 mutants live in Targeted gates
Not covered
MethodologyEnvironment: the CI verify container (node:22-bookworm), working tree at Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
Collaborator
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
chiga0
approved these changes
Aug 27, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Scope: diff + cross-file context for the factory functions (config.ts, agent.ts, workflow-orchestrator.ts, InProcessBackend.ts). NOT reviewed: runtime execution (no working tree available), Windows/Linux runtime behaviour (author's own disclosure).
CI disclosure — mandatory: Every check is SKIPPED except routing/label automation. No build, typecheck, lint, or test job ran against this branch in CI. The author reports 753 focused Config/Agent/Workflow tests and a full build pass locally on macOS (Node 22); Windows and Linux behaviour is explicitly unvalidated. Rungs 1–2 of the execution tier are therefore unverified.
No blocking findings.
Checked:
deriveWorktreeConfig/deriveAgentConfig/deriveApprovalModeConfigfactories — getter and private-field rebound together; no split-path hazard.setApprovalModeguard (isDerivedConfig && !hasOwnProperty('setApprovalMode')) — correctly allows only the approval-profile factory's own method through; bare derived Configs still throw.getPlansAnchorTargetDir()prototype-chain walk — correctly anchors the containment assertion at the plans-owning base Config whencwddiffers; test case "saves a plan on a derived agent config whose cwd differs from the project root" covers this path.createApprovalModeOverrideinagent.ts— cleanup now invoked in thecatchblock on registry-build failure; the AUTO-strip leak from the old implementation is closed.approvalCleanup?.()inworkflow-orchestrator.ts— correctly placed in thefinallyblock; a no-op unless the dispatch context transitions into AUTO.deriveConfig(runtimeContext)inSubagentManager.buildSubagentContextOverride— distinct instance triggers lazyfileReadCacheinstall; marker Symbol still inherited via prototype for the "skip rebuild" path.no-config-object-createESLint rule — all migrated call sites confirmed; integration test covers production vs test vsconfig.tsexemption paths.
Suggestion (confirmed; already filed as R1-6 by qwen-code-ci-bot, still present): The importsConfig predicate in eslint-rules/no-config-object-create.js requires the import specifier to endsWith('/config/config.js'). A same-directory import using './config.js' evades the rule. The risk is narrow today (only non-config.ts siblings in packages/core/src/config/ would use the short form, and they are not excluded), but a future utility in that directory that derives Config via the short specifier would be silently unguarded. Extending the predicate to also match === './config.js' or anchoring on /config.js more broadly would close the gap.
Cross-check against prior rounds:
- Round 1 Critical (
setApprovalModeguard on bare derived Config, plans containment assertion, lint gate with no test,deriveAgentConfigtest coverage): all fixed per author commits 23349f1, 94d0174, b553b21, feca442 — confirmed at head. - R1-6 (lint rule path gap): still present → confirmed above.
- R2-1 (approvalCleanup in
finally— suggestion): pattern is correct; no concern.
Reviewed with AI assistance.
| function importsConfig(node) { | ||
| return ( | ||
| typeof node.source.value === 'string' && | ||
| node.source.value.endsWith('/config/config.js') && |
Collaborator
There was a problem hiding this comment.
The importsConfig predicate requires node.source.value.endsWith('/config/config.js'). A same-directory specifier './config.js' (used by siblings of config.ts within packages/core/src/config/) would evade the rule. The risk is narrow today — the exemption list already excludes config.ts itself, and no current sibling uses the short form — but a future utility added to that directory that derives Config would be silently unguarded. Extending the predicate to also match === './config.js' would close the gap. (Confirms qwen-code-ci-bot R1-6, still present.)
Collaborator
Local real-stack verification of #9915 (post-merge confirmation)I rebuilt this change from scratch on a local machine and drove it through the real bundled CLI ( Tested tree == merged tree. The change set I exercised ( Verdict: the four Reviewer-Test-Plan claims all hold on the real stack, and there is no user-visible behaviour change on either worktree launch path. Two follow-ups worth filing — a coverage gap (§4) and a latent, currently-unreachable invariant (§5). 1 · Environment and repository gatesmacOS 15 (darwin 25.6.0) · Node v24.18.1 · npm 11.16.0 · clean
Every failure above was A/B'd: the two 2 · Claims 1 + 2 — real CLI end-to-end, both worktree launch pathsFixture: a throwaway git repo with a committed
The mutation row is the load-bearing part: reverting just the 3 · Claims 3 + 4Claim 3 (approval overlay above a derived worktree) was probed against the compiled Claim 4 — the same hand-rolled
Scope note: the rule keys on an 4 · Mutation matrix — is the test suite load-bearing?One reverted hunk at a time against the 961 focused Config/Agent/Subagent/Workflow tests:
Seven of nine are killed, several precisely (S3 kills 11; S5 pins the new
5 · Latent finding — two dispatch chains end up with a bare derived Config that binds toolsBecause
It is not reachable today, and I drove the real TUI to establish that rather than assuming it. A workflow-dispatched agent's edit parks in the background panel, which offers only allow once / No:
The identical edit issued by the main agent does offer Yes, allow always, so the option is not globally absent — it is specifically suppressed for parked approvals:
The suppression is Cheap follow-up — drop the condition in - if (effectiveContext !== config) {
+ {
const approvalHandle = deriveApprovalModeConfig(
effectiveContext,
config.getApprovalMode(),
);
approvalCleanup = approvalHandle.cleanup;
effectiveContext = approvalHandle.config;
}The initial mode equals the base mode, so no AUTO strip is acquired and 6 · One measured behaviour delta worth namingUnifying both approval-override builders onto
In an untrusted folder, 7 · What I did not coverWindows and Linux runtime behaviour (macOS only, matching the PR's own "Tested on"); background-agent derivation, which the PR explicitly scopes out; and ACP / web-shell as approval surfaces for parked approvals — I verified the TUI only, though 中文说明#9915 本地真实环境验证（合入后确认）我在本地从零重建了这个改动，并用真实打包的 CLI（ 实测树 == 合入树。 我实测的改动集（ 结论：Reviewer Test Plan 的四条断言在真实链路上全部成立，两条 worktree 启动路径都没有用户可见的行为变化。 有两点值得开后续单：一个覆盖盲区（§4）和一个潜伏但当前不可达的不变量（§5）。 1 · 环境与仓库门禁macOS 15（darwin 25.6.0）· Node v24.18.1 · npm 11.16.0 · 干净 见上方门禁表。所有失败项都做了 A/B：两个 2 · 断言 1 + 2 —— 真实 CLI 端到端，两条 worktree 启动路径夹具：临时 git 仓库，已提交
表中的变异行是判别力所在：只回退 3 · 断言 3 + 4断言 3（派生 worktree 之上的 approval 覆盖）是对两条腿的编译产物 断言 4 —— 把同一个手写 范围说明：该规则以 4 · 变异矩阵 —— 测试有没有判别力？每次只回退一个 hunk，跑 961 个聚焦的 Config/Agent/Subagent/Workflow 测试：9 个变异 7 个被杀，且相当精准（S3 杀 11 个；S5 用一个专门的 plan 持久化测试钉住了新的
5 · 潜伏问题 —— 两条分发链最终拿到的是绑定了工具的裸派生 Config因为
但当前不可达，而且我是真的跑了 TUI 去确认，而不是靠推断。workflow 分发的 agent 的编辑会停在后台面板，只提供 allow once / No；同一个编辑由主 agent 发起时 Yes, allow always 是有的——说明这个选项不是全局缺失，而是专门对停泊审批做了抑制。 抑制来自 低成本的后续修法——去掉 6 · 一个实测到、值得点名的行为变化把两个 approval override 构造器统一到 在不受信任的目录下， 7 · 未覆盖的部分Windows / Linux 运行时行为（只测了 macOS，与 PR 自述的 Tested on 一致）；PR 明确划到范围外的 background-agent 派生；以及 ACP / web-shell 作为停泊审批的呈现面——我只验了 TUI，不过 |
Collaborator
|
Released in v0.22.3. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR moves both worktree launch paths behind the shared derived-Config boundary. Newly provisioned isolation worktrees and caller-owned worktrees now rebind workspace paths, workspace context, and file discovery state through the same ownership operation, including both public getters and the internal state those getters depend on.
It also preserves approval-mode overlays layered above a derived worktree Config: a bare derived Config remains unable to mutate approval state, while the specialized child overlay can change modes without changing its canonical parent.
Stacked-scope note: #9918 (centralize agent config derivation) was merged INTO this branch rather than main, so this PR's diff against main additionally migrates the approval, in-process-agent, subagent, memory, remember, and skill-review derivations behind the same boundary and adds the
no-config-object-createlint rule that keeps hand-rolled overlays from bypassing it. Merging this PR lands both slices together.Why it's needed
The two launch paths previously rebuilt the same prototype overlay independently. That duplication allowed target directory, cwd, workspace context, file service, custom ignore files, and approval ownership to drift apart. Centralizing this slice completes the first production caller migration after the ownership contract landed in #8100; #9918 continued the same direction for the agent-execution paths and was stacked onto this branch, so both slices land together.
Reviewer Test Plan
How to verify
no-config-object-createlint rule flags a hand-rolledObject.createConfig overlay underpackages/core/src(flat-config integration test included).Evidence (Before & After)
N/A — internal refactor with no user-visible behavior change.
Tested on
Environment (optional)
Node.js 22; clean dependency install and full build, repository typecheck, focused ESLint and Prettier checks, and 753 focused Config/Agent/Workflow tests.
Risk & Scope
no-config-object-createlint rule.Linked Issues
Refs #8083
中文说明
这个 PR 做了什么
这个 PR 将两条 worktree 启动路径迁移到共享的派生 Config 边界。新创建的隔离 worktree 和调用方已有的 worktree 现在通过同一个所有权操作重绑定工作区路径、工作区上下文和文件发现状态，同时覆盖公共 getter 及其依赖的内部状态。
它也保留了叠加在派生 worktree Config 之上的 approval mode 覆盖：普通派生 Config 仍不能修改 approval 状态，而专用子级覆盖可以切换模式且不修改规范父 Config。
堆叠范围说明：#9918（集中 agent config 派生）是合入到本分支而不是 main 的，因此本 PR 相对 main 的 diff 还会把 approval、进程内 agent、subagent、memory、remember 和 skill-review 派生一并迁移到同一边界之后，并新增
no-config-object-createlint 规则，防止手写 overlay 绕过该边界。合入本 PR 会同时落地这两个切片。为什么需要
此前两条启动路径各自重建同一套 prototype overlay，容易让目标目录、cwd、工作区上下文、文件服务、自定义 ignore 文件和 approval 所有权发生漂移。集中这一切片后，完成了 #8100 所有权契约落地后的第一组生产调用方迁移；#9918 沿同一方向继续迁移了 agent 执行路径并堆叠到本分支，因此两个切片会一起合入。
Reviewer Test Plan
如何验证
no-config-object-createlint 规则会在packages/core/src下拦截手写Object.createConfig overlay（附 flat-config 集成测试）。Evidence (Before & After)
N/A —— 内部重构，没有用户可见行为变化。
Tested on
Environment (optional)
Node.js 22；独立依赖安装与全仓构建、仓库 typecheck、聚焦 ESLint/Prettier 检查，以及 753 个 Config/Agent/Workflow 聚焦测试。
Risk & Scope
no-config-object-createlint 规则。Linked Issues
Refs #8083