Repository navigation
fix(cli): extend the #8663 loader denylist and harden its scrub lifecycle - #8763
Conversation
…ycle Follow-up to #8663. Its inherited-env denylist closed the NODE_OPTIONS/ NODE_PATH class but left sibling code-execution and TLS-trust-anchor vars that reach the same #8653 cross-workspace outcome — an untrusted workspace `.env` is frozen into daemonRuntimeBaseEnv and distributed to every workspace's session subprocesses. Denylist additions, split by the PR's own tiering: - Scrubbed loader tier (INHERITED_LOADER_ENV_KEYS — scrubbed from the inherited launch env and rejected from every `.env`/settings.env scope), for pure-injection vars with no legitimate operator-shell use: OPENSSL_CONF (startup dlopen of an attacker OpenSSL engine), NODE_REPL_EXTERNAL_MODULE, npm_config_node_gyp, npm_config_init_module. - Reject-from-project-`.env` tier (PROJECT_ENV_HARDCODED_EXCLUSIONS — rejected from project files, preserved from the shell / home `.env`), for vars with a legitimate operator-shell use whose only exposed vector is an untrusted project file: * TLS trust anchors SSL_CERT_FILE, SSL_CERT_DIR, CURL_CA_BUNDLE, REQUESTS_CA_BUNDLE, GIT_SSL_CAINFO (siblings of NODE_EXTRA_CA_CERTS; an attacker CA MITMs a session's git/npm/pip/curl traffic). * git command-execution family GIT_SSH_COMMAND, GIT_EXTERNAL_DIFF, GIT_CONFIG_GLOBAL/SYSTEM/COUNT and the numbered GIT_CONFIG_KEY_<n>/ GIT_CONFIG_VALUE_<n> pairs (matched by prefix). core/utils/git-branches.ts already scrubs these from the repo's own git invocations. * node-gyp interpreter selection NODE_GYP_FORCE_PYTHON, npm_config_python, PYTHON (run as the build Python during native-addon installs). Concurrency: the daemon's process.env scrub/restore and the loader-key rejection reporter were process-global with no guard for concurrent embedded daemons in one process (a documented supported config). The first daemon's close() restored loader vars into the shared env, re-poisoning a still-live sibling's sessions, and dropped its reporter. The scrub is now reference counted (acquireInheritedLoaderEnvScrub — snapshot on first acquire, restore only on last release) and the reporter is cleared only when still active. Test hardening from the same review: pin the daemon-worker scrub breadcrumb (not just key removal); pin the fast-path settings.env case-folded hardcoded-exclusion gate; drain the module-global fast-path stash so the accumulate assertion is order-independent. Docs updated for the new keys.
e925043 to
92c525f
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
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. |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31283575452 🧠 Handled by Qwen Code · model/模型 |
… surface The refcounted acquireInheritedLoaderEnvScrub read/wrote process.env from config/shared-env-keys.ts, which the serve process.env guard does not scan — moving the access out of run-qwen-serve.ts dropped its allowlisted count and failed process-env-guard.test.ts. Pass the env into the coordinator instead so run-qwen-serve.ts still owns the process.env reference (matching the existing scrub helpers), and update the allowlist to the new count.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action requiredNothing to address this round on PR #8763 (head Feedback triage
No code changes were made and no commit was created; no verification commands were required because the working tree is untouched. 中文说明Autofix 评审轮次：无需处理本轮在 PR #8763（head 反馈分类
未做任何代码改动，也未创建提交；由于工作区未被修改，无需运行任何验证命令。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31290546316 🧠 Handled by Qwen Code · model/模型 |
…denylist Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #8763 (round after budget timeout)Scope decisionThis round carried a budget warning: the previous round exhausted its time budget without finishing anything. Per the workflow rules, this round addressed only the smallest blocking subset — the two Critical findings — committed as soon as complete, and deferred the twelve Suggestions with per-thread replies ( Feedback dispositionsAddressed (2 Criticals, one commit:
|
| Finding | Disposition | Change |
|---|---|---|
[Critical] GIT_CONFIG_PARAMETERS left the git config-injection family reachable (inline 3742328269) |
Fixed — verified: core/utils/git-branches.ts GIT_ENV_VARS_TO_CLEAR (the list this PR's comment claims to mirror) already strips it, and the test at shared-env-keys.test.ts:166 pinned the omission |
Added 'GIT_CONFIG_PARAMETERS' to PROJECT_ENV_HARDCODED_EXCLUSIONS; flipped the pinned assertion (the prefix-overmatch negative case now uses GIT_CONFIG_NOSYSTEM, which only skips the system gitconfig read and injects nothing); extended the case-insensitive coverage; extended the end-to-end loadEnvironment rejection test (environment.test.ts); updated the docs enumeration |
[Critical] GIT_SSH (legacy counterpart of the blocked GIT_SSH_COMMAND) not excluded (inline 3742328276) |
Fixed — verified: git still execs GIT_SSH for SSH transports, and the denylist is centralized in isHardcodedProjectEnvExclusion, which all six application gates (4 in environment.ts, 2 in fast-path-settings.ts) route through |
Added 'GIT_SSH' next to 'GIT_SSH_COMMAND'; same test/doc updates as above |
Both keys stay in the reject-from-project-files tier (not the scrubbed loader set), consistent with their siblings: shell-exported and user-level .env values are preserved.
Deferred with replies (12 Suggestions — see comment-replies.json)
- 3742328281
GIT_SSL_NO_VERIFYTLS-tier addition — valid; first candidate for the follow-up round. - 3742328283 docs overstate the settings.env tier scope — needs a maintainer decision between rewording the docs and making the settings.env gates scope-aware (option (b) changes the PR's security semantics); recommendation recorded in the reply.
- 3742328285
npm_config_gitmissing from the docs enumeration — valid; lands with the follow-up. - 3742328278 reporter slot LIFO-order diagnostic loss — valid; stack/registry redesign deferred to a dedicated change.
- 3742328286 no diagnostics for hardcoded-tier rejections — valid; warn-once machinery mirror deferred to a dedicated change.
- 3742328289, 3742328293, 3742328295, 3742328297, 3742328299, 3742328302, 3742328305 test-efficacy probes (surviving mutants / hunk-survived reverts) — all valid; the proposed tests are grouped into the follow-up (one two-cycle scrub test covers three of the mutants).
Review-level notes
- The
CHANGES_REQUESTEDstate was driven by issue-level comment 5228685910, which reports the autofix agent's own failed run on a stale base (status 125) and names no code defect — there is nothing to trace in this diff; the substantive unresolved findings were the two Criticals above, now fixed. - The reviewer's "not reviewed" note (Integration Tests (CLI, No Sandbox) skipped in CI) is acknowledged: the changed behavior is exercised at the
loadEnvironment/fast-path unit surface, which the focused suites cover end-to-end, including the exact attack scenario (project.env→ rejected). - The reviewer's budget-cut exploration (close()-teardown ordering before
restoreScrubbedLoaderEnv()) was left open by the reviewer itself and is not a finding; the scrub-lifecycle questions it raises are covered by the deferred test-efficacy replies above.
Conflict notes
--conflict false: no merge of origin/main performed; none needed.
Verification
Commands actually run this round (repo root unless noted):
npm run build— passed (also produced the missingpackages/channels/base/distentry that madefast-path.test.tsuncollectable before the build — a stale-checkout artifact, not related to this change)npm run typecheck— passednpm run lint— passednpx vitest run src/config/shared-env-keys.test.ts src/config/environment.test.ts src/serve/fast-path.test.ts(inpackages/cli) — 141 passed (3 files)npx vitest run src/commands/channel/daemon-worker.test.ts src/serve/process-env-guard.test.ts(inpackages/cli) — 81 passed (2 files)npx prettier --checkon the four touched files — passed
Integration tests after npm run bundle were not run: the touched behavior (project .env/settings.env rejection) is fully exercised by the unit surfaces above, not only through the bundled CLI. npm run generate:settings-schema was not needed: no settings source changed.
中文说明
处理评审总结 — PR #8763（预算超时后的本轮）
范围决定
本轮带有预算警告：上一轮耗尽了时间预算且没有完成任何内容。按照工作流规则，本轮只处理最小阻断子集——两条 Critical 发现——完成后立即提交，并将十二条 Suggestion 以逐线程回复（comment-replies.json）的方式延迟处理，确保没有任何发现被悄悄丢弃。
反馈处置
已处理（2 条 Critical，单次提交：46e2690465）
| 发现 | 处置 | 变更 |
|---|---|---|
[Critical] GIT_CONFIG_PARAMETERS 使 git 配置注入家族仍然可达（inline 3742328269） |
已修复——已核实：core/utils/git-branches.ts 的 GIT_ENV_VARS_TO_CLEAR（本 PR 注释声称要对齐的列表）本来就清除了它，而 shared-env-keys.test.ts:166 的测试把这一遗漏钉死了 |
在 PROJECT_ENV_HARDCODED_EXCLUSIONS 中新增 'GIT_CONFIG_PARAMETERS'；翻转被钉死的断言（前缀过度匹配的负例改用 GIT_CONFIG_NOSYSTEM——它只跳过系统 gitconfig 读取，不能注入任何配置）；补充大小写不敏感覆盖；扩展端到端 loadEnvironment 拒绝测试（environment.test.ts）；更新文档枚举 |
[Critical] GIT_SSH（已被阻断的 GIT_SSH_COMMAND 的旧版对应变量）未被排除（inline 3742328276） |
已修复——已核实：git 在 SSH 传输时仍会执行 GIT_SSH，且拒绝名单集中在 isHardcodedProjectEnvExclusion，全部六个应用门控（environment.ts 中 4 处、fast-path-settings.ts 中 2 处）都经由它 |
在 'GIT_SSH_COMMAND' 旁新增 'GIT_SSH'；测试/文档更新同上 |
两个键都保留在"仅从项目文件拒绝"层（不进入被剥离的 loader 集合），与其同族键一致：shell 导出值与用户级 .env 值仍然保留。
以回复延迟处理（12 条 Suggestion——见 comment-replies.json）
- 3742328281 TLS 层补充
GIT_SSL_NO_VERIFY——有效；下一轮的第一候选。 - 3742328283 文档夸大了 settings.env 层的作用域——需要维护者在"改写文档"与"让 settings.env 门控区分作用域"之间做决定（选项 (b) 会改变本 PR 的安全语义）；回复中已记录建议。
- 3742328285 文档枚举缺少
npm_config_git——有效；随下一轮一起合入。 - 3742328278 reporter 槽在 LIFO 顺序下的诊断丢失——有效；栈/注册表改造延迟到专门的变更。
- 3742328286 硬编码层拒绝没有任何诊断——有效；复刻 warn-once 机制延迟到专门的变更。
- 3742328289、3742328293、3742328295、3742328297、3742328299、3742328302、3742328305 测试有效性探针（存活变异体 / hunk 存活回退）——全部有效；所提议的测试归入下一轮（其中一个双周期剥离测试可覆盖其中三个变异体）。
评审层面的说明
CHANGES_REQUESTED状态源自 issue 级评论 5228685910，该评论报告的是 autofix 代理自身在陈旧 base 上的失败运行（status 125），没有指出任何代码缺陷——本 diff 中无可追溯的内容；实质性的未决发现就是上面两条 Critical，现已修复。- 评审者的"未审查"说明（CI 中跳过了 Integration Tests (CLI, No Sandbox)）已知悉：本次变更的行为在
loadEnvironment/fast-path 单元层面被完整覆盖，聚焦套件已端到端覆盖，包括确切的攻击场景（项目.env→ 被拒绝）。 - 评审者因预算用尽而未完成的探索（
restoreScrubbedLoaderEnv()之前的 close() 拆除顺序）由评审者自己留作未决，不构成发现；其引发的剥离生命周期问题已由上面延迟的测试有效性回复覆盖。
冲突说明
--conflict false：未合并 origin/main，也无需合并。
验证
本轮实际执行的命令（未注明者均在仓库根目录）：
npm run build—— 通过（同时生成了此前缺失的packages/channels/base/dist入口文件——该缺失曾导致fast-path.test.ts无法收集，属于检出环境的陈旧产物，与本变更无关）npm run typecheck—— 通过npm run lint—— 通过npx vitest run src/config/shared-env-keys.test.ts src/config/environment.test.ts src/serve/fast-path.test.ts（在packages/cli内）—— 141 通过（3 个文件）npx vitest run src/commands/channel/daemon-worker.test.ts src/serve/process-env-guard.test.ts（在packages/cli内）—— 81 通过（2 个文件）- 对四个被改动文件执行
npx prettier --check—— 通过
未运行 npm run bundle 之后的集成测试：本次变更的行为（项目 .env/settings.env 拒绝）已由上述单元层面完整覆盖，并非只能经由打包后的 CLI 验证。未运行 npm run generate:settings-schema：未改动任何 settings 源。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 4/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31307298125 🧠 Handled by Qwen Code · model/模型 |
…c-file tiers Close the round-2 review findings: block the remaining git command-execution siblings (GIT_EXEC_PATH, GIT_TEMPLATE_DIR, GIT_ASKPASS, GIT_PROXY_COMMAND, GIT_EDITOR), the npm/pip TLS trust knobs (npm_config_cafile, npm_config_ca, npm_config_strict_ssl, PIP_CERT, GIT_SSL_CAPATH), and the curl/wget rc-file redirects (CURL_HOME, WGETRC) from project .env files. Freeze the numbered GIT_CONFIG_KEY_/VALUE_ pairs on reload together with GIT_CONFIG_COUNT, and sync the qwen-serve.md loader-key enumeration with settings.md.
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Round response to round-2 review feedback (PR #8763)Commit: Note: this round ran under a budget warning from the previous round, so the work focused on the blocking subset plus the cheap in-scope fixes; the one decline below is recorded with its reason and its thread stays open. Feedback dispositions
What changedrc:3743394920 (Critical). Added the remaining git command-execution siblings to the reject-from-project- rc:3743394923. Added the npm/pip/git trust knobs to the TLS tier: rc:3743394925. The rc:3743394926. rc:3743394927. Took the finding's documentation option rather than the provenance-based deletion change: settings.md now states the reject-only keys are frozen at boot from a user-level rc:3743394928. Declined this round, with a reply on the thread: the finding is diagnostics-only (reject/restore behavior is correct and test-pinned, as the finding itself notes), and the durable-log coordination plus pre-close callback rework is disproportionate diff growth for a budget-constrained round. The thread stays open so the suggestion is not silently dropped. Review-level CHANGES_REQUESTED (integration suite). The "Integration Tests (CLI, No Sandbox)" job is merge-queue-gated by design ( Verification
中文说明对第 2 轮审查反馈的回应（PR #8763）提交： 说明：由于上一轮耗尽时间预算，本轮带有预算警告，因此工作聚焦于阻塞子集加上低成本的范围内修复；下方唯一一条拒绝已记录理由，其讨论串保持打开。 反馈处置
变更内容rc:3743394920（Critical）。 将剩余的 git 命令执行家族变量加入"拒绝来自项目 rc:3743394923。 将 npm/pip/git 的信任旋钮加入 TLS 层： rc:3743394925。 rc:3743394926。 rc:3743394927。 采用该发现提供的文档选项，而非基于来源跟踪的删除传播：settings.md 现已说明这些"仅拒绝项目文件"的键在来自用户级 rc:3743394928。 本轮拒绝，并已在讨论串回复：该发现仅影响诊断（拒绝/恢复行为正确且有测试钉住，发现本身也如此说明），而持久日志协调加上关闭前回调通道的改造，对于预算受限轮次而言属于不成比例的 diff 增长。讨论串保持打开，该建议不会被静默丢弃。 审查级 CHANGES_REQUESTED（集成套件）。 "Integration Tests (CLI, No Sandbox)" 任务在设计上仅由合并队列触发（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Could not produce a passing fix for this feedback (round 6/100). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31322652532 🧠 Handled by Qwen Code · model/模型 |
…ist-followup # Conflicts: # packages/cli/src/config/shared-env-keys.test.ts
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
中文说明
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| sharedProcessEnvScrubDepth++; | ||
| const removedKeys = scrubAndReportInheritedLoaderEnv( | ||
| env, |
There was a problem hiding this comment.
[Critical] R1-2: A nested scrub acquisition deletes a later host assignment without snapshotting it. — Failure scenario: daemon A acquires, the embedding host assigns NODE_OPTIONS, then daemon B acquires and scrubs it; the final release leaves the value absent or restores an older snapshot, corrupting the embedding process environment. Preserve newly present loader-key assignments before nested scrubs and add the exact A → assign → B → release regression test.
中文说明
R1-2： 嵌套 scrub acquire 会删除后续 host 赋值，却没有把它记入快照。失败场景：daemon A acquire 后，嵌入宿主设置 NODE_OPTIONS，随后 daemon B acquire 并将其剥离；最终 release 后该值保持缺失或恢复成更旧的快照，破坏嵌入进程环境。请在嵌套剥离前保存新出现的 loader 键赋值，并增加精确的 A → 赋值 → B → release 回归测试。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| 'npm_config_strict_ssl', | ||
| 'npm_config_strict-ssl', | ||
| 'PIP_CERT', |
There was a problem hiding this comment.
[Critical] R1-3: PIP_CONFIG_FILE remains accepted and can redirect all pip configuration. — Failure scenario: an untrusted project points it at a repository file that sets index-url, trusted-host, proxy, cert, or client-cert; a session pip invocation sends traffic or credentials to attacker-controlled infrastructure. Add it to the project-only exclusions and cover every application/reload boundary.
中文说明
R1-3： PIP_CONFIG_FILE 仍可被接受，并能重定向 pip 的全部配置。失败场景：不受信项目将其指向仓库内配置文件，设置 index-url、trusted-host、代理、证书或客户端证书；会话中的 pip 调用会把流量或凭据发送到攻击者控制的基础设施。请将其加入仅项目级排除名单，并覆盖所有应用/重载边界。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| 'GIT_TEMPLATE_DIR', | ||
| 'GIT_ASKPASS', | ||
| 'GIT_PROXY_COMMAND', |
There was a problem hiding this comment.
[Critical] R1-4: SSH_ASKPASS remains accepted although Git uses it as a fallback executable. — Failure scenario: a project sets SSH_ASKPASS to an attacker script and forces askpass; a Git authentication challenge executes the script as the daemon user. Add SSH_ASKPASS (and evaluate SSH_ASKPASS_REQUIRE) to the project exclusion tier with application and reload tests.
中文说明
R1-4： SSH_ASKPASS 仍可被接受，而 Git 会把它作为后备可执行程序。失败场景：项目将 SSH_ASKPASS 设置为攻击者脚本并强制 askpass；Git 认证挑战会以 daemon 用户身份执行该脚本。请将 SSH_ASKPASS（并评估 SSH_ASKPASS_REQUIRE）加入项目排除层，补齐应用和重载测试。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| 'npm_config_script_shell', | ||
| 'npm_config_prefix', | ||
| 'NODE_PATH', |
There was a problem hiding this comment.
[Critical] R1-6: LESSOPEN remains accepted although less executes it as an input preprocessor. — Failure scenario: a project sets LESSOPEN to an attacker command; a session running less on a file executes that command as the daemon user. Add it to the appropriate exclusion tier and cover every environment-loading boundary.
中文说明
R1-6： LESSOPEN 仍可被接受，而 less 会将其作为输入预处理器执行。失败场景：项目将 LESSOPEN 设置为攻击者命令；会话对文件运行 less 时会以 daemon 用户身份执行该命令。请将其加入合适的排除层，并覆盖所有环境加载边界。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| return HARDCODED_PROJECT_ENV_EXCLUSION_PREFIXES.some((prefix) => | ||
| lowerKey.startsWith(prefix), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R1-7: The prefix matcher rejects nonnumeric GIT_CONFIG_KEY_/GIT_CONFIG_VALUE_ suffixes that Git does not consume. — Concrete cost: a project-defined variable such as GIT_CONFIG_KEY_CACHE is silently rejected and frozen although Git only interprets decimal-numbered pairs. Match a numeric suffix and add negative tests for empty/nonnumeric suffixes.
中文说明
R1-7： 前缀匹配会拒绝 Git 并不消费的非数字 GIT_CONFIG_KEY_/GIT_CONFIG_VALUE_ 后缀。具体代价：如 GIT_CONFIG_KEY_CACHE 这样的项目自定义变量会被静默拒绝并冻结，尽管 Git 只解释数字编号对。请只匹配数字后缀，并增加空后缀/非数字后缀负例。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| // An ordinary key next to them still rotates — the freeze is key-scoped. | ||
| expect(process.env['RUNTIME_DOTENV']).toBe('allowed'); |
There was a problem hiding this comment.
[Suggestion] R1-8: The ordinary-key reload assertion is vacuous because the fixture value never changes. — Concrete cost: if reload accidentally freezes every environment key, RUNTIME_DOTENV remains allowed and this test still passes. Rewrite the second fixture with a distinct value such as rotated and assert it after reload.
中文说明
R1-8： 普通键重载断言是空洞的，因为 fixture 的值从未变化。具体代价：若重载错误地冻结所有环境键，RUNTIME_DOTENV 仍为 allowed，测试依然通过。请将第二个 fixture 改为不同值（如 rotated），并在重载后断言该值。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 7/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review round — PR #8763Feedback triageCritical-only mode is active after five change-producing rounds. This round's actionable sections contained no feedback: no reviews, no inline comments, no issue-level comments, no failed checks, and no still-red checks. The Base-conflict resolution (requested via
|
|
🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #8763All six findings (4 Critical, 2 Suggestion) were verified against the code and addressed in one commit: Findings and dispositions
Docs: the settings.md denylist enumeration now lists the new reject-only keys. No conflict handling was needed ( Verification
中文说明审查轮次总结 — PR #8763全部 6 条发现（4 条 Critical、2 条 Suggestion）均已对照代码核实，并在一次提交中处理完毕： 发现与处理
文档：settings.md 中的 denylist 清单已列出新增的仅拒绝键。 无需处理冲突（ 验证
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
@qwen-code /triage |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — PR #8763 (no action)Feedback triageCritical-only mode is active (after 5 change-producing rounds). Everything listed under No failed checks and no still-red checks were reported for this round. The single actionable item and its dispositionThe Disposition: no code action required (declined with evidence).
Verification performed this roundThe review recorded a coverage gap from its own run: only filtered runs of the new fast-path tests had executed, and the The review also noted "Integration Tests (CLI, No Sandbox) was skipped in CI". That is a workflow skip, not a failing check, and no named defect is attached to it — no code action to take. VerificationCommands actually run this round and their results:
OutcomeNo code changes this round; no commit created. Working tree clean; HEAD unchanged at 中文说明Autofix 本轮总结 — PR #8763（无操作）反馈分类当前处于仅处理 Critical 的模式（在 5 个产生改动的轮次之后）。 本轮没有报告失败的检查，也没有报告持续红色的检查。 唯一的可处理条目及其处置该 处置：无需代码操作（附证据予以驳回）。
本轮执行的验证审查记录了其自身运行时留下的覆盖缺口：当时只对新增的 fast-path 测试做了过滤运行，且 审查还提到 "Integration Tests (CLI, No Sandbox) was skipped in CI"（集成测试在 CI 中被跳过）。这是工作流层面的跳过，并非检查失败，且没有与之关联的已指名缺陷——无需采取代码操作。 验证本轮实际执行的命令及结果：
结果本轮无代码改动；未创建提交。工作树干净；HEAD 保持在 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action required on PR #8763This review round has no actionable items:
Critical-only mode is active after 5 change-producing rounds. The only item in No code was changed and no commit was made in this round. The branch stays at 中文说明PR #8763 无需处理本轮评审没有任何可处理的项目：
在 5 个产生改动的轮次之后，已进入仅处理 Critical 的模式。"Deferred non-Critical feedback"（延后的非 Critical 反馈）部分中唯一的条目是 @qwen-code-ci-bot 的一条自动化 PR 评论。按照本轮规则，该部分属于审计记录而非待办工作：未修改任何代码、未解决任何讨论串，也未就该条目撰写任何回复。该条目保持开放，留待人工跟进。 本轮未改动任何代码，也未创建任何提交。分支保持在 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Unresolved, please confirm: [Critical] issue-level comment 5228685910: autofix status — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell. Re-checked against the code at 3b70728: the comment is an autofix infrastructure status report, not a code defect.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
[Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status) — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell
中文说明
未决，请确认：[Critical] issue-level comment 5228685910: autofix status — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell. Re-checked against the code at 3b70728: the comment is an autofix infrastructure status report, not a code defect.
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
[Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status) — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| for (const key of Object.keys(env)) { | ||
| if (!isLoaderEnvKey(key)) continue; | ||
| const value = env[key]; | ||
| if (value !== undefined) sharedProcessEnvScrubOriginals.set(key, value); | ||
| } | ||
| sharedProcessEnvScrubDepth++; | ||
| const removedKeys = scrubAndReportInheritedLoaderEnv( |
There was a problem hiding this comment.
[Suggestion] acquireInheritedLoaderEnvScrub double-iterates process.env — the snapshot loop (lines 371-375) and the subsequent scrubAndReportInheritedLoaderEnv call both iterate Object.keys(env) and both call isLoaderEnvKey on every key. — Failure scenario: On every daemon startup (including overlapping embedded daemon acquires), process.env is iterated twice instead of once. For a typical ~150-entry env, this is ~300 isLoaderEnvKey calls per acquire, each doing a canonicalLoaderKey transform (2 regex substitutions + .replace()). Suggested fix: Merge the snapshot and the scrub into a single Object.keys(env) pass that both records the value and deletes the key.
中文说明
acquireInheritedLoaderEnvScrub 对 process.env 做了两次迭代——快照循环（第 371-375 行）和后续的 scrubAndReportInheritedLoaderEnv 调用都遍历了 Object.keys(env) 并对每个键调用了 isLoaderEnvKey。失败场景：每次 daemon 启动时（包括重迭内嵌 daemon 的 acquire），process.env 被迭代两次而非一次。建议：将快照和剥离合并为一次 Object.keys(env) 遍历，同时记录值并删除键。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| if (sharedProcessEnvScrubDepth === 0) { | ||
| sharedProcessEnvScrubOriginals.clear(); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: deleting the sharedProcessEnvScrubOriginals.clear() at acquire time (line 364) leaves every affected test green. — Failure scenario: A future change that removes or skips this clear() call would not be caught by any test in this diff, allowing stale originals from a prior scrub cycle to leak into the next scrub's snapshot. Suggested fix: Add a test that verifies sharedProcessEnvScrubOriginals is empty when the scrub depth drops to zero after a full acquire/release cycle.
中文说明
变体存活：删除 acquire 时的 sharedProcessEnvScrubOriginals.clear()（第 364 行）所有测试仍然通过。失败场景：未来删除了该 clear() 调用不会被任何测试捕获。建议：增加测试验证 sharedProcessEnvScrubOriginals 在完整 acquire/release 周期后为空。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| if (sharedProcessEnvScrubDepth > 0) return; | ||
| for (const [key, value] of sharedProcessEnvScrubOriginals) { |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: deleting the sharedProcessEnvScrubOriginals.clear() at release time (line 397) leaves every affected test green. — Failure scenario: Same as the acquire-time clear — a future removal would not be caught by any test. Suggested fix: Confirm or add coverage for this reset path.
中文说明
变体存活：删除 release 时的 sharedProcessEnvScrubOriginals.clear()（第 397 行）所有测试仍然通过。建议：确认或补充此重置路径的测试覆盖。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| export function resetInheritedLoaderEnvScrubForTesting(): void { | ||
| sharedProcessEnvScrubDepth = 0; | ||
| sharedProcessEnvScrubOriginals.clear(); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: deleting the sharedProcessEnvScrubOriginals.clear() in the test-only reset helper (line 403) leaves every affected test green. — Failure scenario: A future removal of this clear() call would compromise test isolation between dependent test suites. Suggested fix: Confirm or add coverage for this reset path.
中文说明
变体存活：删除仅测试重置辅助函数中的 sharedProcessEnvScrubOriginals.clear()（第 403 行）所有测试仍然通过。建议：确认或补充此重置路径的测试覆盖。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const value = env[key]; | ||
| if (value !== undefined) sharedProcessEnvScrubOriginals.set(key, value); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: the if (value !== undefined) guard (line 374) is never exercised — no test pins when the guard must NOT fire. — Failure scenario: A change that removes the value !== undefined guard would not be caught by any test, allowing undefined values to be stored in sharedProcessEnvScrubOriginals. Suggested fix: Add a test case that exercises the value === undefined branch and verifies the key is not stored.
中文说明
变体存活：if (value !== undefined) 守卫（第 374 行）从未被触发——没有测试验证守卫在什么情况下必须不触发。建议：增加测试用例，验证当 value === undefined 时该键不会被存储。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
acquireInheritedLoaderEnvScrub iterated process.env twice (a snapshot pass, then the scrub); record the originals inside the scrub's single pass instead. Drop the acquire-time snapshot clear, which the release-time clear made unreachable defense, and add tests that kill the previously surviving mutants on the release-time clear, the test-only reset, and the undefined-value guard.
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 9/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8763Feedback points and dispositionsReview-level Critical: issue-level comment 5228685910 (autofix status) — Declined (no code defect)Confirmed, as the reviewer's own re-check concluded: comment 5228685910 is the [rc:3746607294] Double iteration of process.env in acquire — ResolvedMerged the snapshot and the scrub into one pass: [rc:3746607299] Mutant survived: acquire-time
|
|
🤖 Addressed the latest review feedback (round 10/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 10/100 轮）。改动内容与我反驳保留之处如下： Round summary (PR #8763, review round 4)All seven actionable findings were verified against the exact code at this Findings and dispositions
No finding was declined or escalated. Also in this commit
Verification
中文说明轮次总结（PR #8763，审查第 4 轮）全部 7 条可执行发现均已在本提交的代码上逐一核实，并在一次提交（ 发现与处置
无发现被拒绝或升级待决。 本提交的其他内容
验证
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
📊 Takeover milestone — round 10/100, in the current window. Census: 6 pushed fix(es), 3 no-change review(s), 2 timeout(s), 1 rejected attempt(s), 1 other round(s) (crash / model error / gate error / infra), 2 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 10/100 轮（当前窗口）。统计：推送修复 6 次、审阅无需改动 3 次、超时 2 次、验证拒绝 1 次、其他轮次（崩溃/模型错误/门错误/infra）1 次、base 更新 2 次。 轮次到这个量值得人工看一眼。可选：继续（无需操作）；若轮次持续累积，考虑拆分或缩减 PR；或释放接管（移除 |
|
@qwen-code /triage |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31381560750 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed the exact current head (02fcc3b). I revalidated the unresolved Critical threads against the tree rather than relying on GitHub thread state: the previously reported git config/SSH, config-discovery, browser-launch, and CDP command surfaces are now present in the hardcoded project-env gate and covered across the initial load, reload, runtime-env build, and serve fast path. The ref-counted inherited-env scrub also has balanced startup/close cleanup.
The remaining open items are non-blocking Suggestions: reporter diagnostics for overlapping embedded daemons, hardcoded-tier warning/docs accuracy, a few adjacent denylist candidates, and mutation-strength test gaps. This PR has already exceeded the repository review-round budget, so those should stay in the stated follow-up rather than widening this round. Approving with suggestions; CI should still finish green.
…eak (QwenLM#8816) * feat(ci): A/B deterministic gate rejections against the pre-round ref A deterministic rejection in the autofix verification gate is only chargeable to the round if the same check passes without the round's commit. The gate charged every red to the fix unconditionally, and run 31276008548 measured what that costs when the premise is false: PR 8614's branch predated QwenLM#8693's tsconfig guard while node_modules came from the post-QwenLM#8693 trusted base, so `npm run build` was equally red at origin/<branch> — 63 minutes of accepted agent work discarded, an 18-minute repair burned on a failure the repair agent is forbidden to touch (it may only amend the round's own fix), thirteen rounds in a row, and the same again on the QwenLM#8616 leg. On rejection the gate now re-runs the failing check at origin/<branch> (the branch as pushed, before the round) in the same environment: - baseline green: today's path exactly — outcome=failed, retryable=true, the repair pass gets its chance. - baseline red too: outcome=failed with preexisting=true and NO retryable. The repair step keys on retryable and is skipped — it cannot reach a failure outside the round's diff by construction — and gate-rejection.md says outright that the branch needs a base update (merge main), which flows into the failure comment as-is. Fail-closed toward today's semantics: any A/B infrastructure problem (missing ref, checkout failure) charges the fix as before, and a restore failure after the baseline run rejects outright since the tree can no longer be trusted. The round's work is still not pushed — this changes the verdict's honesty and cost, not the push policy. Tested by executing the real script in a real two-remote git repo with an npm stub whose failures are keyed by commit SHA: round-caused red (baseline green), pre-existing red (both red), and the untouched green path. Mutation-tested, 3 of 3 caught: skipping the A/B, claiming pre-existing without measuring, and dropping the tree restore. * Address review: bound the A/B to checks it can honestly compare All seven findings verified before fixing; the three Criticals were each a way the A/B compared something other than the check that failed. R1-1 — the contracts check feeds on stdin, which its first run drains; the baseline leg re-ran against EOF and checked an empty file list. R1-3 — the schema check's verdict rides on packages/core/dist, which the core-rebuild guard built from ROUND sources and which, being gitignored, survives the detach. Both checks are now A/B-exempt (run_check_no_ab): their baseline verdicts prove nothing, and their rejections stay where the repair agent can actually act on them. R1-2 — a workspace the round ADDS does not exist at the baseline, and npm exits 1 there with "No workspaces found" (measured; --if-present forgives a missing script, not a missing workspace) — a round-caused failure misread as pre-existing, skipping the one repair that can fix the round's own package. The per-package loop now A/Bs only when the workspace exists at origin/<branch>. R1-4 — a chatty PASSING baseline used to flood the tail -c 3000 evidence window and push the actual failure text out of gate-rejection.md, the sole carrier into the repair feedback, the PR comment, and the next round's LAST_REJECTION. The baseline transcript now goes to a side log and only a FAILING tail is merged back, where it is the evidence. R1-5 — the pre-existing paragraph pushed gate-rejection.md past the report's head -c 3500 cap, truncating the closing fence for branch names past 44 characters. Cap raised to 3900, invariant comment updated with the new arithmetic. R1-6 — preexisting=true had no read site. It now flows verify → Finalize verification → the failure report, whose headline swaps the generic gate clause for "PRE-EXISTING failure … needs a base update (merge main)". R1-7 — the no-round-commit guard was unpinned (deleting it kept all tests green). Now exercised through the core-rebuild path, the one A/B-eligible check that runs before the commit gate. Four new behavioral scenarios (chatty baseline, no-commit round, A/B-exempt checks, round-added workspace) plus workflow pins for the forwarding, the clause, and the cap. Mutation-tested, 4 of 4 caught: schema back to A/B (3 tests), guard dropped, side log reverted, no-commit guard dropped. * Address review round 2: A/B only what it can prove, prove what it claims Ten findings across two rounds, each verified before fixing. The three deepest share one lesson: the A/B is only sound for a check whose inputs travel entirely with the git ref, and whose failure it can IDENTIFY, not merely observe. R2-1 — rc=1 at both legs does not make them the same failure: the branch can fail for reason A while the round fails for reason B, and a baseline infrastructure hiccup is a nonzero exit too. Pre-existing now requires a MATCHING failure identity — tsc diagnostics normalized to file + error code (positions shift with the round's edits), compared via comm(1) on a per-check transcript. No diagnostics on either side means identity cannot be established and the round stays charged. R2-2 / R2-7 — gitignored dist survives the detach carrying the ROUND's build, so any dist-consuming check A/Bs reverted sources against round-built artifacts: package tests (channel-base resolved through dist exports) and typecheck (sdk-typescript resolves core's d.ts — probe-verified three-arm flip). Both are now A/B-exempt, as is lint, leaving `npm run build` — the incident class, and the one check that rebuilds its own inputs from the checked-out sources — as the sole A/B candidate. The workspace-existence guard dissolves with it. R2-3 — the fixture inherited the caller's global git config; a failing global pre-commit hook broke all seven cases. The harness now isolates GIT_CONFIG_GLOBAL/SYSTEM for every git child, and the suite is proven green under a deliberately hostile hooksPath. R2-4 — Finalize verification now selects preexisting from the same attempt whose outcome it selects (repair verification included). R2-5 / R2-8 — the "merge main" advice is now conditional at both layers: the script paragraph states the measured fact and hedges the remedy; the report headline uses the compare the step already ran — behind/diverged gets the base-update clause, an up-to-date branch is told its own pre-round code needs attention. R2-6 — the rejection document now sizes its evidence tail against its preamble (floor 500 bytes, total under the 3900-byte render cap), so the closing fence can no longer be truncated off by a long branch name. R2-9 — dissolved by R2-2: package tests no longer A/B, the guard and its uncovered positive branch are gone. R2-10 — the baseline-evidence merge is now pinned: the pre-existing scenario asserts the baseline leg's own failure line (keyed by its SHA) reaches gate-rejection.md. Eight behavioral scenarios; mutation-tested 5 of 5: identity dropped, typecheck re-enrolled, package tests re-enrolled, evidence merge dropped, fixed tail restored. * Address review round 4: sharpen identity, stage the git failures, sync prose Nine findings, all refinements — the design held, the edges did not. Identity now keeps the diagnostic MESSAGE (file + code collide: two unrelated TS2339s in one file compared equal, skipping a repair that could have shipped — probe-reproduced by the review), and the fixture emits a SHIFTED position on the baseline leg so the position strip is load-bearing instead of decorative (deleting the sed survived every test before; it fails one now). vite/esbuild failures still yield an empty signature by design — documented as the fail-closed limit rather than half-widened. The fail_signature assignments take `|| true`: grep exits 1 on the normal no-match case and survives errexit today only because the caller sits in an if-condition — a future unconditional call site would crash the gate verdict-less. The restore-failure branch is now stageable and staged: the baseline leg recreates (untracked) a file the branch tracks, the checkout back refuses, and the test pins retryable-not-preexisting with the 'could not restore' label. Relaxing the branch to `|| true` fails it. Prose synced to the mechanisms that replaced it: the render-cap invariant restates against the dynamic tail budget (the old 3000-based arithmetic would misguide the next retune), the no-round-commit guard comment names the core rebuild (schema/contracts left the A/B last round), the describe wording counts both A/B-eligible builds, and the pre-existing clauses no longer claim "the repair pass was skipped" — with REPAIR_PREEXISTING forwarded, repair may have RUN; they now state the invariant that is true either way: repair may only amend the round's own fix, so it cannot reach this failure. Mutation-tested, 3 of 3 caught: position strip dropped, message dropped from the identity, restore rejection relaxed. * fix(ci): watchdog silent sandbox hangs and reap the containers they leak Four autofix rounds have died the same way (QwenLM#8663 twice, QwenLM#8761 r3, QwenLM#8763 r4): the agent's last output is the sandbox wrapper's "ContainerName (regular): …" line at docker container entry, then nothing — not one event — until the 2-hour absolute budget kills the round. Four different runners, two image versions: systemic, not a bad machine. Where exactly the container wedges is still unknown (that needs docker state on the runner); what is certain from the logs is the shape — a wedged sandbox produces NOTHING, and a legitimate run is never silent for long (the fleet's longest tolerated quiet is the review pipeline's 10-minute stream-idle window for thinking phases). Two mitigations, each aimed at a measured half of the damage: - run-agent.mjs gains an idle watchdog (QWEN_IDLE_TIMEOUT_MS, default 20 minutes = 2x that longest legitimate silence): zero output for the window kills the agent with a distinct "idle-timeout … the sandbox likely hung at startup" detail, so the failure comment names the right knob and a hung round costs 20 minutes instead of 120. Polled, not reset-per-chunk — a busy stream should not spend its time re-arming timers. - Both sandboxed jobs reap stale qwen-code-* containers at job start: a budget kill reaps the HOST-side docker client, not the container, so every killed sandbox keeps running on the persistent runner — observed directly when a later leg's container-name counter found qwen-code-0.21.8-0 already occupied and picked -1. One job per runner at a time makes any container alive at job start stale by definition. Tested by executing the real run-agent.mjs end to end with stub agents: the hang shape (one line, then silence) dies at the idle window naming the idle limit, and a slow-but-talking agent that outputs every 400ms across a 1500ms window survives to a clean exit — the test that distinguishes a watchdog from a disguised absolute timer. Mutation- tested, 3 of 3 caught: watchdog disabled, last-output tracking dropped (the disguised-timer regression), cleanup dropped from a job. * Address review round 5: the gate's verdict defects and the reaper's live kill Budget-warning round — the five Criticals from both reviewers, no suggestions (each deferred with a recorded reply). fail_signature: `[^\n]*` in an ERE bracket expression does not mean "rest of line" — in POSIX bracket expressions `\` is literal, so it matched "neither backslash nor the letter n" and truncated every tsc message at its first n. Nearly every real message has an early n ("Cannot find name", "is not assignable"), so distinct same-file failures collapsed into identical signatures and a round-caused failure could be labeled pre-existing, skipping the repair. grep is line-oriented: `.*` is exactly the rest of the line. New fixture: two messages differing only after their first n. Pre-existing verdict: the intersection test mislabeled in both directions. A round that ADDS a diagnostic sharing one normalized line with the baseline was called pre-existing (repair skipped for a round-caused, repairable failure); and `comm -12 | grep -q` under `set -eo pipefail` SIGPIPEs comm (exit 141) once the shared output outruns the pipe buffer, charging true pre-existing failures to the round — the exact 18-minute repair waste the gate exists to kill. Pre-existing now means the round's failing set is a SUBSET of the baseline's, and the difference is captured before testing. New fixture: a round adding a second diagnostic to a failing baseline. Restore failure after the baseline leg: was retryable=true with HEAD still detached at the baseline commit — the repair agent works in that very checkout and does no git recovery, so its commit would land on the baseline and be orphaned. Now rejected non-retryable (reject_fix grows a third arg); the next round starts clean from the trusted checkout. The restoreClash test pins the new semantics. Stale-container reap: the premise "a runner runs one job at a time, so any live qwen-code-* container is stale" holds per runner registration, but the filter queries the docker daemon, which is per host — and this pool runs several registrations on one OS. With per-issue/PR serialization only, a concurrent job's sandbox is a substring match away from `docker rm -f`. The reap now takes only provably-dead containers (--filter status=exited/dead, both jobs) and the comment says why a running one is left alone. Preamble printf: the `\`` escapes sat inside a single-quoted format where backslash is literal, so every pre-existing rejection rendered raw backticks instead of code spans (shellcheck SC2016). Backticks need no escaping there. Also syncs the side-log comment to the dynamic tail_budget it actually renders. Verified: scripts suite 140/140 (was 138; the two new fixtures and the rewritten restoreClash test all fail against the pre-fix script), npm run build / typecheck / lint pass, bash -n clean. * Address review round 6: reap the kill's own orphan, tolerate the reaper * Address review: hang-bound the reaper, unblock the kill path, pin the unpinned arms - Wrap every docker call in the stale-container reap with timeout 30: an alive-but-wedged daemon blocks docker ps indefinitely, and the existing || guards only catch nonzero exits, not hangs (R3-1). - Make the kill-path container removal async in run-agent.mjs: the spawnSync blocked the event loop between SIGTERM and the 10s SIGKILL backstop for up to its 30s timeout — in exactly the wedged-daemon scenario the watchdog exists for. The main flow awaits the removal so the leak warning stays deterministic (R3-6). - Split the pre-existing gate clause for an empty CMP_R: a transient compare-API failure is "never measured", not "measured not-behind", and must not assert the branch's own code is at fault (R3-7). - Swap the timeout breaker's closing remedy to the sandbox investigation when every counted timeout was idle, mirroring the round-level split (R3-11). - Tests: pin the budget kill path separately from the idle kill path (R3-3), parameterize the idle-window parse guard over -1/0/NaN (R3-5), add a stderr-only liveness case (R3-12), pin the strict-subset A/B arm via a baseline-superset fixture knob (R3-15), and pin the breaker's current-round idle increment (R3-18). --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <[email protected]> Co-authored-by: qwen-code-dev-bot <[email protected]>
3 participants
What this PR does
Follow-up to #8663. The
/reviewpass that ran right after #8663 merged surfaced 14 findings that were never addressed (all still unresolved on that PR). This PR handles the substantive ones.#8663's inherited-env denylist closed the
NODE_OPTIONS/NODE_PATHclass but left sibling variables that reach the same #8653 cross-workspace outcome: an untrusted workspace.envis frozen intodaemonRuntimeBaseEnvand distributed to every workspace's session subprocesses. This PR extends the denylist along #8663's own two tiers, hardens the scrub lifecycle against concurrent embedded daemons, and closes the test/diagnostic gaps the review named.Denylist additions
Scrubbed loader tier (
INHERITED_LOADER_ENV_KEYS— scrubbed from the inherited launch env and rejected from every.env/settings.envscope). Pure-injection vars with no legitimate operator-shell use:OPENSSL_CONF— Node's startup crypto init dlopens an attacker-configured OpenSSL engine/provider.sobefore any user code runs.NODE_REPL_EXTERNAL_MODULE— a spawnednodeREPLrequire()s an attacker file at startup.npm_config_node_gyp— npm's shim runs"$npm_config_node_gyp" "$@"verbatim.npm_config_init_module—require()d bynpm init(evennpm init -y).Reject-from-project-
.envtier (PROJECT_ENV_HARDCODED_EXCLUSIONS— rejected from project files, but a value the operator sets in their own shell or home.envis preserved). Vars with legitimate operator-shell use whose only exposed vector is an untrusted project file:SSL_CERT_FILE,SSL_CERT_DIR,CURL_CA_BUNDLE,REQUESTS_CA_BUNDLE,GIT_SSL_CAINFO— siblings of the already-blockedNODE_EXTRA_CA_CERTS; an attacker CA MITMs the token-bearing traffic a session'sgit/npm/pip/curlcalls make.GIT_SSH_COMMAND,GIT_EXTERNAL_DIFF,GIT_CONFIG_GLOBAL/SYSTEM/COUNTand the numberedGIT_CONFIG_KEY_<n>/GIT_CONFIG_VALUE_<n>pairs (matched by prefix).core/utils/git-branches.tsalready scrubs exactly these from the repo's own git invocations, so a project.envsetting them contradicts our own model.NODE_GYP_FORCE_PYTHON,npm_config_python,PYTHON— run as the build Python during native-addonnpm install.Concurrency hardening
The daemon's
process.envscrub/restore and the loader-key rejection reporter were process-global with no guard for concurrent embedded daemons in one process — a documented supported config (acp-bridge/src/bridgeOptions.tschildEnvOverrides). The first daemon'sclose()restored loader vars into the sharedprocess.env, re-poisoning a still-live sibling's sessions, and dropped its reporter. The scrub is now reference-counted (acquireInheritedLoaderEnvScrub— snapshot on first acquire, restore only on last release) and the reporter is cleared only when still the active one.Test / diagnostic hardening
settings.envcase-folded hardcoded-exclusion gate (previously only the.envloop's case-fold was pinned).accumulatesassertion is order-independent (no longer depends on a sibling test consuming first).settings.md) updated for all new keys.Deliberately not changed
LD_LIBRARY_PATH,PYTHONPATH, …) and the residualPATH-prefix leak remain the deferred follow-up fix(cli): scrub inherited loader env vars from daemon session subprocesses #8663 already tracks — rejecting them breaks mainstream toolchains.Verification
npm run buildclean;tsc --noEmit,eslint, andprettier --checkclean on all changed files. Affected suites:shared-env-keys.test.ts,environment.test.ts,fast-path.test.ts,daemon-worker.test.ts,run-qwen-serve.test.ts— 458 tests pass. New tests cover every added key at the predicate and.env/settings.envapplication layers, the refcounted scrub (restore does not re-poison while a second holder is live), and the reporter clear-if-current guard.这个 PR 做了什么
#8663 的后续。#8663 合入后紧接着跑的
/review给出了 14 条一直未处理的评审意见（在该 PR 上至今全部 unresolved）。本 PR 处理其中实质性的部分。#8663 的继承环境拒绝列表关闭了
NODE_OPTIONS/NODE_PATH这一类，但遗漏了通向同一 #8653 跨 workspace 结果的同族变量：不受信 workspace 的.env会被冻结进daemonRuntimeBaseEnv并分发到每个 workspace 的会话子进程。本 PR 沿 #8663 自身的两层结构扩展拒绝列表，加固剥离生命周期以应对同进程并发内嵌 daemon，并补齐评审指出的测试/诊断缺口。拒绝列表新增
剥离 loader 层（从继承的启动环境剥离，且在所有
.env/settings.env作用域被拒绝）——无正当登录 shell 用途的纯注入变量：OPENSSL_CONF（启动时 dlopen 攻击者 OpenSSL engine）、NODE_REPL_EXTERNAL_MODULE、npm_config_node_gyp、npm_config_init_module。仅拒绝项目
.env层（从项目文件拒绝，但运维在自己 shell 或 home.env设置的值保留）——有正当 shell 用途、仅经不受信项目文件暴露的变量：TLS 信任锚SSL_CERT_FILE/SSL_CERT_DIR/CURL_CA_BUNDLE/REQUESTS_CA_BUNDLE/GIT_SSL_CAINFO（NODE_EXTRA_CA_CERTS的同族，MITM 会话的 git/npm/pip/curl 携带 token 的流量）；git 命令执行家族GIT_SSH_COMMAND/GIT_EXTERNAL_DIFF/GIT_CONFIG_GLOBAL/SYSTEM/COUNT及编号GIT_CONFIG_KEY_<n>/GIT_CONFIG_VALUE_<n>对（按前缀匹配；core/utils/git-branches.ts已剥离这些）；node-gyp 解释器选择NODE_GYP_FORCE_PYTHON/npm_config_python/PYTHON。并发加固
daemon 对
process.env的剥离/恢复与 loader 键拒绝 reporter 是进程全局的，对同进程并发内嵌 daemon（文档化的受支持配置）无保护。第一个 daemon 的close()会把 loader 变量恢复进共享process.env，重新污染仍存活的同伴会话，并丢掉其 reporter。现改为引用计数（acquireInheritedLoaderEnvScrub——首次 acquire 快照、仅最后一次 release 恢复），reporter 仅在仍是当前活跃者时才清除。测试/诊断加固
钉住 daemon-worker channel 边界剥离的 breadcrumb（不仅是键删除）；钉住快速路径
settings.env的大小写折叠硬编码排除门控；在accumulates断言前排空模块全局 stash 使其与测试顺序无关；settings.md更新所有新键。有意未改动
warn-once 去重按进程生效（移除后再加不再复警）是 R3-4 既定设计；库搜索路径（
LD_LIBRARY_PATH、PYTHONPATH等）与残留的PATH前缀泄漏仍为 #8663 已跟踪的 deferred 项——拒绝它们会破坏主流工具链。