feat(autofix): require isolated targeted E2E proof - #8318
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the PR! Template looks good ✓ Problem: this is an observed architectural gap, not a theoretical concern. The existing Autofix publication gate runs deterministic checks (build, typecheck, lint, changed-package tests) but never independently reruns the specific E2E case that created the issue on the exact candidate commit. Editable issue prose, candidate-controlled verification inputs, and process/filesystem escape paths are real attack surfaces in a system that runs with elevated credentials. The motivation is well-grounded. Direction: clearly aligned — this hardens the Autofix pipeline's trust boundary, which is core CI/CD infrastructure for this repo. The fail-closed posture (unsupported or unverifiable failures stop safely) is the right default for a system that publishes branches and PRs with a bot PAT. Size: ~3000 production lines (workflow YAML + shell + Node scripts) and ~2700 test lines across 28 files. No core module paths ( Approach: the scope feels right for what it's doing. The verification chain (immutable metadata → approval digest → credential-free isolated rerun → publication revalidation) is a complete unit, and each stage addresses a specific trust boundary. The extensive revalidation at publication time (claim ownership, prose digest, branch OID, PR head OID, closing-reference bindings) is defensive but justified given the elevated credentials involved. The test coverage is thorough (~2700 lines). One question worth considering: the Risk: no elevated risk signals — none of the changed files match the high-revert-correlation paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个已观察到的架构缺陷，而非理论性问题。现有的 Autofix 发布门禁运行确定性检查（build、typecheck、lint、变更包测试），但从未在精确候选提交上独立重跑创建 issue 的特定 E2E 用例。可编辑的 issue 文本、候选代码可控的验证输入、进程/文件系统逃逸路径都是使用提升凭据运行的系统中的真实攻击面。动机充分。 方向：明确对齐——这加固了 Autofix 流水线的信任边界，属于本仓库的核心 CI/CD 基础设施。fail-closed 姿态（不受支持或无法验证的失败安全停止）对于使用 bot PAT 发布分支和 PR 的系统是正确的默认选择。 规模：约 3000 行生产代码（workflow YAML + shell + Node 脚本）和约 2700 行测试代码，共 28 个文件。未触及核心模块路径（ 方案：范围与目标匹配。验证链（不可变元数据 → 批准摘要 → 无凭据隔离重跑 → 发布重验证）是完整单元，每个阶段处理特定信任边界。发布阶段的广泛重验证（claim 所有权、prose 摘要、分支 OID、PR head OID、closing-reference 绑定）虽然防御性强，但鉴于涉及的提升凭据，是合理的。测试覆盖充分（约 2700 行）。一个值得考虑的问题：发布步骤中的 风险：无升级风险信号——变更文件均未匹配高回滚关联路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: for "Autofix can publish without proving the specific E2E case passed on the published commit," I would (1) attach immutable failure metadata as a CI artifact rather than in editable issue prose, (2) gate eligibility to Linux sandbox:none failures only, (3) rerun the exact failing test in a credential-free, privilege-dropped environment, and (4) revalidate issue state, claim ownership, and metadata integrity at publication time to close the TOCTOU gap. Comparison: the PR matches and exceeds this in every dimension. The metadata is transported via CI artifacts with producer/source run validation. Eligibility is gated to Linux sandbox:none with an explicit trusted-test allowlist. The isolated verifier uses No critical blockers found. Two observations:
Security model is sound. The privilege-drop ordering ( Supporting changes are clean. Files changed (28 of 28 shown)
sequenceDiagram
participant P1 as main-ci-failure-issue
participant P2 as issue-autofix
participant P3 as issue-autofix-verify
participant P4 as targeted-e2e
participant P5 as issue-autofix-publish
P1->>P1: Analyze failure and build metadata
P1->>P1: Upload artifact and record approval digest
P2->>P2: Scan candidates and match approval digest
P2->>P2: Claim issue via git ref with force-with-lease
P2->>P2: Run agent and package candidate bundle
P3->>P3: Restore candidate from bundle
P3->>P3: Run build, typecheck, lint as unprivileged user
P3->>P3: Validate outputs and protected paths
P4->>P4: Load and validate E2E metadata from artifact
P4->>P4: Rerun exact failing test in isolated environment
P4->>P4: Validate Vitest report matches expected case
P5->>P5: Revalidate claim, prose, metadata, OIDs
P5->>P5: Push branch and create PR with proof
P5->>P5: Verify PR bindings and release claim
Testing
The PR's own CI has not been triggered yet for The author reports local verification with 124 workflow contract tests and 61 verifier/failure-signature Node tests, plus build, typecheck, lint, Prettier, and actionlint. This is the author's claim, not independently verified — the CI suite will confirm. This PR is CI/CD infrastructure — its central claim is about workflow behavior, not CLI runtime behavior. The appropriate evidence is the CI test suite (workflow contract tests + Node unit tests), not not verified: Linux DAC enforcement ( 中文说明代码审查独立方案： 针对"Autofix 可以在不证明特定 E2E 用例在发布提交上通过的情况下发布"的问题，我会 (1) 将不可变失败元数据作为 CI artifact 而非可编辑 issue 文本传递，(2) 将资格限制为仅 Linux sandbox:none 失败，(3) 在无凭据、降权环境中重跑精确失败测试，(4) 在发布时重验证 issue 状态、claim 所有权和元数据完整性以关闭 TOCTOU 间隙。 对比： PR 在每个维度都匹配并超越了此方案。元数据通过 CI artifact 传递并带有生产者/源运行验证。资格限制为 Linux sandbox:none 并带有明确的受信任测试允许列表。隔离验证器使用 未发现关键阻塞项。两个观察：
安全模型健全。 降权顺序（ 测试PR 自身的 CI 尚未针对 作者报告本地验证了 124 项 workflow 合同测试和 61 项 verifier/failure-signature Node 测试，以及 build、typecheck、lint、Prettier 和 actionlint。这是作者声明，非独立验证——CI 套件将确认。 此 PR 是 CI/CD 基础设施——其核心声明关于 workflow 行为，而非 CLI 运行时行为。适当证据是 CI 测试套件，不需要 未验证：Linux DAC 强制执行（ — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, well-designed security hardening with thorough test coverage; CI hasn't run yet but branch protection gates merge on it. This is the right PR at the right time. The Autofix pipeline needed a way to prove that the specific E2E case which created an issue actually passes on the published commit — deterministic checks alone don't close that gap. The four-job pipeline (agent → deterministic verify → targeted E2E → publish with revalidation) is a clean separation of concerns, and each job runs with minimal permissions. The security model is the standout: privilege-dropped execution via The CI hasn't been triggered yet for this head SHA (no 中文说明置信度：4/5 — 干净、设计良好的安全加固，测试覆盖充分；CI 尚未运行，但分支保护会在合并前要求 CI 通过。 这是正确的 PR，出现在正确的时间。Autofix 流水线需要一种方法来证明创建 issue 的特定 E2E 用例确实在发布提交上通过——仅靠确定性检查无法关闭这个间隙。四作业流水线（agent → 确定性验证 → targeted E2E → 带重验证的发布）是清晰的关注点分离，每个作业以最小权限运行。 安全模型是亮点：通过 两个脚本之间的 CI 尚未针对此 head SHA 触发（不存在 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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. |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
[Critical] Current head f06ecadb69f5c406bceed3e14b33547b1ecdfb50 moves Verification gate out of issue-autofix into the new issue-autofix-verify job (.github/workflows/qwen-autofix.yml:1459-1546), but scripts/tests/package-scripts.test.js:547-550 still calls getWorkflowJob(workflow, "issue-autofix") and requires that step. The current Ubuntu CI therefore fails exactly at this assertion (expected -1 to be >= 0), so this PR cannot merge. Update the test to assert the gate on issue-autofix-verify (or otherwise preserve the contract) and rerun the focused test.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Addressed the requested contract fix in |
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
|
@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 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action requiredThe single Critical finding from @qqqys — Verified locally on current HEAD (
No other actionable feedback (no inline comments, no failed checks). 中文说明无需修改@qqqys 提出的唯一 Critical 问题—— 在当前 HEAD（
无其他可操作的反馈（无行内评论、无失败检查）。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action requiredThis round triaged the feedback newer than the last evaluation. There is no
No code changes were made and nothing was committed this round. The review 中文说明Autofix 审查轮次 —— 无需处理本轮对上次评估之后新增的反馈进行了分类。没有可处理的发现：
本轮没有进行任何代码更改，也没有提交任何内容。审查超时由工作流 / CI 基础设施 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| if ! gh api "repos/${REPO}/actions/jobs/${job_id}/logs" > "${log_path}"; then | ||
| echo "::warning::Could not download the log of job ${job_id}" | ||
| rm -f "${log_dir}/${job_id}.log" | ||
| rm -f "${log_path}" |
There was a problem hiding this comment.
[Critical] This echo "::warning::…" now sits inside the jq -c … | while … done | jq -s '.' > "${manifest}" pipeline this PR added, so the warning text (not JSON) is fed into jq -s. When any single failed job's log cannot be downloaded — the exact case the code anticipates ("A missing log only costs precision"): expired/unavailable logs or a transient gh api error — jq -s exits with a parse error and, because it is the terminal pipeline stage, bash -e aborts the whole "Download failed job logs" step. The manifest is never written and the issue-creation job fails hard, inverting the documented graceful degradation into a hard failure and swallowing the annotation. — Failure scenario: one transient log-download failure → jq -s parse error → step aborts → no CI-failure issue is filed at all.
| if ! gh api "repos/${REPO}/actions/jobs/${job_id}/logs" > "${log_path}"; then | |
| echo "::warning::Could not download the log of job ${job_id}" | |
| rm -f "${log_dir}/${job_id}.log" | |
| rm -f "${log_path}" | |
| echo "::warning::Could not download the log of job ${job_id}" >&2 |
Sending the annotation to stderr keeps it out of the JSON pipe while still surfacing it.
中文说明
此 echo "::warning::…" 现在位于本 PR 新增的 jq -c … | while … done | jq -s '.' > "${manifest}" 管道内部，因此告警文本（非 JSON）会被送入 jq -s。当任何一个失败 job 的日志无法下载时（正是代码所预期的情况——“缺少日志只会损失精度”：日志过期/不可用或 gh api 瞬时错误），jq -s 会以解析错误退出，并且由于它是管道末端阶段，bash -e 会中止整个 “Download failed job logs” 步骤。manifest 永远不会被写入，issue 创建 job 会硬失败，从而把文档中所述的优雅降级反转为硬失败，同时吞掉该告警。— 失败场景：一次瞬时日志下载失败 → jq -s 解析错误 → 步骤中止 → 完全不会创建任何 CI 失败 issue。修复：将告警发送到 stderr（>&2），使其不进入 JSON 管道但仍可显示。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| existing_issue="$( | ||
| gh issue list \ | ||
| --repo "${REPO}" \ | ||
| --state open \ | ||
| --state all \ |
There was a problem hiding this comment.
[Critical] The search was changed from --state open to --state all with a jq fallback (map(select(.state == "OPEN"))[0] // map(select(.state == "CLOSED"))[0]).number, so a CLOSED issue is returned as issue_number. Downstream, the file_issue job then exits without updating the body, reopening, or creating a replacement when it receives a closed EXISTING_ISSUE: the non-eligible path exits immediately, and the eligible path fails the .state == "OPEN" check → route_allowed=false → exits. There is no gh issue reopen anywhere, and the create branch is never reached because EXISTING_ISSUE is non-empty. — Failure scenario: a failure is fixed and its issue #100 closed; the same failure recurs (regression); the search finds closed #100 → file_issue exits silently → the recurring failure is completely swallowed (no issue created, no body updated, no comment). Before this diff, --state open would not have found the closed issue and a new one would have been created.
| existing_issue="$( | |
| gh issue list \ | |
| --repo "${REPO}" \ | |
| --state open \ | |
| --state all \ | |
| --state open \ |
Either keep --state open (so a recurrence creates a fresh issue), or add a gh issue reopen + state re-fetch when the matched issue is CLOSED.
中文说明
搜索从 --state open 改为 --state all，并带有 jq 回退 (map(select(.state == "OPEN"))[0] // map(select(.state == "CLOSED"))[0]).number，因此 CLOSED（已关闭）的 issue 会作为 issue_number 返回。下游的 file_issue job 在收到已关闭的 EXISTING_ISSUE 时，会直接退出而不更新正文、不重新打开、也不创建替代 issue：非 eligible 路径立即退出；eligible 路径在 .state == "OPEN" 检查处失败 → route_allowed=false → 退出。整个 workflow 中没有任何 gh issue reopen，并且由于 EXISTING_ISSUE 非空，create 分支永远不会到达。— 失败场景：某失败被修复、其 issue #100 被关闭；同一失败再次发生（回归）；搜索找到已关闭的 #100 → file_issue 静默退出 → 该复发失败被完全吞掉（不创建 issue、不更新正文、不评论）。在本 diff 之前，--state open 不会找到已关闭的 issue，因而会创建一个新 issue。修复：保留 --state open（使复发创建新 issue），或在匹配到的 issue 为 CLOSED 时增加 gh issue reopen + 重新获取状态。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| gh api "repos/${REPO}/actions/runs/${WORKFLOW_RUN_ID}/attempts/${WORKFLOW_RUN_ATTEMPT}/jobs?per_page=100" \ | ||
| --paginate \ | ||
| --slurp \ | ||
| | jq '[.[].jobs[] | select(.conclusion == "failure") | {id, name}]' \ | ||
| > "${jobs_json}" |
There was a problem hiding this comment.
[Suggestion] Rewriting this failed-job fetch from a process substitution (mapfile … < <(gh api …)) into a pipe makes a transient gh api failure abort the entire analyze job: with defaults.run.shell: bash (i.e. bash -eo pipefail), a non-zero gh api exit propagates through pipefail+errexit. The old form ignored the process-substitution exit status, degraded to an empty list, and still produced a per-commit fallback issue. — Concrete cost: a transient 502 / secondary rate limit / timeout on this upstream fetch now yields no tracking issue at all instead of a degraded per-commit one. (Distinct from the Critical above, which is the per-job log download; this is the upstream job-list fetch.)
| gh api "repos/${REPO}/actions/runs/${WORKFLOW_RUN_ID}/attempts/${WORKFLOW_RUN_ATTEMPT}/jobs?per_page=100" \ | |
| --paginate \ | |
| --slurp \ | |
| | jq '[.[].jobs[] | select(.conclusion == "failure") | {id, name}]' \ | |
| > "${jobs_json}" | |
| if ! gh api "repos/${REPO}/actions/runs/${WORKFLOW_RUN_ID}/attempts/${WORKFLOW_RUN_ATTEMPT}/jobs?per_page=100" \ | |
| --paginate \ | |
| --slurp \ | |
| | jq '[.[].jobs[] | select(.conclusion == "failure") | {id, name}]' \ | |
| > "${jobs_json}"; then | |
| echo "::warning::Could not list failed jobs; falling back to per-commit issue" >&2 | |
| echo '[]' > "${jobs_json}" | |
| fi |
Wrapping in if ! suppresses the errexit/pipefail abort and degrades to an empty list.
中文说明
把这个失败 job 列表的获取从进程替换（mapfile … < <(gh api …)）改写为管道，会使一次瞬时的 gh api 失败中止整个 analyze job：在 defaults.run.shell: bash（即 bash -eo pipefail）下，gh api 的非零退出会通过 pipefail+errexit 传播。旧形式会忽略进程替换的退出状态，降级为空列表，并仍然生成按 commit 的回退 issue。— 具体代价：此上游获取的一次瞬时 502 / 二级限流 / 超时，现在会导致完全不创建任何跟踪 issue，而不是降级的按 commit issue。（与上面的 Critical 不同：那是每个 job 的日志下载；这是上游的 job 列表获取。）修复：用 if ! 包裹以抑制 errexit/pipefail 中止，并降级为空列表。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| concurrent_issue="$( | ||
| gh issue list \ | ||
| --repo "${REPO}" \ | ||
| --state all \ |
There was a problem hiding this comment.
[Suggestion] This best-effort concurrent-reuse search is a bare command substitution assigned to a variable under bash -e. If gh issue list exits non-zero (transient 502 / secondary rate limit / search-API timeout), the assignment's exit status is non-zero and set -e aborts the whole file_issue step immediately — the primary operation (creating/updating the tracking issue) never executes. This is the only non-critical gh call in the step whose failure should degrade gracefully rather than abort. — Concrete cost: a transient API blip here prevents the CI-failure issue from being created/updated until the next occurrence.
| concurrent_issue="$( | |
| gh issue list \ | |
| --repo "${REPO}" \ | |
| --state all \ | |
| concurrent_issue="$( | |
| gh issue list \ | |
| --repo "${REPO}" \ | |
| --state all \ | |
| --author "${AUTOFIX_BOT}" \ | |
| --search "${marker} in:body" \ | |
| --json number,state \ | |
| --jq '(map(select(.state == "OPEN"))[0] // map(select(.state == "CLOSED"))[0]).number // ""' | |
| )" || concurrent_issue='' |
Appending || concurrent_issue='' makes the search best-effort.
中文说明
这个“尽力而为”的并发复用搜索是一个在 bash -e 下赋值给变量的裸命令替换。如果 gh issue list 非零退出（瞬时 502 / 二级限流 / 搜索 API 超时），该赋值的退出状态即为非零，set -e 会立即中止整个 file_issue 步骤——主操作（创建/更新跟踪 issue）永远不会执行。这是该步骤中唯一一个非关键、其失败应当优雅降级而非中止的 gh 调用。— 具体代价：此处一次瞬时 API 抖动会阻止 CI 失败 issue 被创建/更新，直到下一次失败发生。修复：追加 || concurrent_issue='' 使该搜索变为尽力而为。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| const uid = Number(process.env['AUTOFIX_VERIFY_UID']); | ||
| const gid = Number(process.env['AUTOFIX_VERIFY_GID']); | ||
|
|
||
| if (!candidateCli || !Number.isInteger(uid) || !Number.isInteger(gid)) { |
There was a problem hiding this comment.
[Suggestion] The guard accepts uid/gid of 0: Number(''), Number(' ') and Number('0') all coerce to 0, and Number.isInteger(0) is true, so an empty/whitespace/"0" AUTOFIX_VERIFY_UID/GID passes the guard and process.setuid(0)/setgid(0) keep the process running as root — defeating the privilege-drop isolation that is this launcher's whole purpose. (An unset var → NaN is correctly rejected; an empty/whitespace/"0" var is not.) Not reachable from today's sole caller (run-autofix-vitest.sh uses id -u under set -e), but the launcher's validation exists to be a self-contained safety net, and the net has a hole. — Concrete cost: a future caller mis-interpolation, or a workflow refactor introducing a ${{ }} expression that evaluates to empty, would run the untrusted candidate CLI as root.
| const uid = Number(process.env['AUTOFIX_VERIFY_UID']); | |
| const gid = Number(process.env['AUTOFIX_VERIFY_GID']); | |
| if (!candidateCli || !Number.isInteger(uid) || !Number.isInteger(gid)) { | |
| const uid = Number(process.env['AUTOFIX_VERIFY_UID']); | |
| const gid = Number(process.env['AUTOFIX_VERIFY_GID']); | |
| if (!candidateCli || !Number.isInteger(uid) || uid <= 0 || !Number.isInteger(gid) || gid <= 0) { |
Adding uid <= 0 || gid <= 0 closes the hole (probe-confirmed).
中文说明
该守卫会接受 uid/gid 为 0：Number('')、Number(' ') 和 Number('0') 都会被强制转换为 0，而 Number.isInteger(0) 为 true，因此空/空白/"0" 的 AUTOFIX_VERIFY_UID/GID 会通过守卫，并使 process.setuid(0)/setgid(0) 让进程继续以 root 运行——破坏了本启动器全部目的所在的降权隔离。（未设置的变量 → NaN 会被正确拒绝；空/空白/"0" 的变量则不会。）当前的唯一调用方无法触发（run-autofix-vitest.sh 在 set -e 下使用 id -u），但本启动器的校验存在的意义就是成为一个自包含的安全网，而该网存在漏洞。— 具体代价：未来调用方的错误插值，或引入求值为空的 ${{ }} 表达式的 workflow 重构，将以 root 运行不可信的候选 CLI。修复：增加 uid <= 0 || gid <= 0（已用 probe 验证）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| const publicExistingBody = isAutofixEligible(analysis) | ||
| ? publicMachineMarkers(existingBody) | ||
| : existingBody; |
There was a problem hiding this comment.
[Suggestion] For autofix-eligible failures, publicMachineMarkers(existingBody) reduces the prior issue body to only the <!-- qwen-main-ci-failure-test:<12hex> --> marker lines, discarding the occurrence block (<!-- qwen-main-ci-failure-occurrences --> and its - \sha` · …lines) and the signature marker.renderIssueBody's merge path then calls splitOccurrenceBlock, finds no occurrence marker → lines: []→ thekeptfilter keeps nothing, so only the current run's occurrence survives. — Concrete cost: every prior recurrence (up toMAX_OCCURRENCES = 10) is silently dropped from autofix-eligible issues; humans lose the recurrence count and run links the occurrence machinery exists to preserve. The existing test feeds the prior body in but never asserts [run 301]` survives, so this ships green.
Have publicMachineMarkers also preserve the occurrence block and signature marker (extract them before the strip and re-prepend, or keep lines matching OCCURRENCE_MARKER, the - \`` occurrence pattern, SIGNATURE_MARKER_PREFIX, and TRIMMED_NOTE`), and add a test assertion that the prior occurrence survives.
中文说明
对于 autofix-eligible 的失败，publicMachineMarkers(existingBody) 会把先前的 issue 正文精简为仅保留 <!-- qwen-main-ci-failure-test:<12hex> --> 标记行，丢弃出现次数块（<!-- qwen-main-ci-failure-occurrences --> 及其 - \sha` · …行）和签名标记。随后renderIssueBody的合并路径调用splitOccurrenceBlock，找不到出现次数标记 → lines: []→kept过滤器什么都不保留，因此只有当前运行的出现记录存活。— 具体代价：autofix-eligible issue 中所有先前的出现记录（最多MAX_OCCURRENCES = 10条）都会被静默丢弃；人类会丢失出现次数计数和该出现次数机制本应保留的运行链接。现有测试会传入先前正文，但从未断言[run 301]存活，因此该问题会在测试全绿的情况下上线。修复：让publicMachineMarkers` 同时保留出现次数块和签名标记，并增加断言确认先前出现记录存活。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| return { | ||
| ...analysis, | ||
| tests, | ||
| title: `Main CI failed: ${analysis.workflow} — ${tests[0].id}${extra}`, |
There was a problem hiding this comment.
[Suggestion] publicIssueAnalysis reads tests[0].id with no empty-tests guard. isAutofixEligible gates on analysis.targetedE2e.eligible (built from --jobs), while analysis.tests is built independently from the positional log files, so an eligible analysis can have tests: []. — Failure scenario: analyze invoked with --jobs containing parseable E2E failures but no positional logs (or logs with no FAIL lines) → targetedE2e.eligible: true, tests: [] → plan calls publicIssueAnalysis → TypeError: Cannot read properties of undefined (reading 'id'), aborting issue creation (probe-confirmed via the real CLI). Today's workflow caller derives both from the same downloaded logs and cannot produce the shape, but the CLI's own contract accepts --jobs with zero positional logs.
| title: `Main CI failed: ${analysis.workflow} — ${tests[0].id}${extra}`, | |
| title: `Main CI failed: ${analysis.workflow} — ${tests[0]?.id ?? '<no test>'}${extra}`, |
Or guard the eligible branch: if (!isAutofixEligible(analysis) || !analysis.tests.length) return analysis;.
中文说明
publicIssueAnalysis 读取 tests[0].id 时没有对空 tests 的守卫。isAutofixEligible 以 analysis.targetedE2e.eligible（由 --jobs 构建）为门控，而 analysis.tests 由位置参数日志文件独立构建，因此一个 eligible 的分析可能具有 tests: []。— 失败场景：以 --jobs（含可解析的 E2E 失败）但没有位置参数日志（或日志中无 FAIL 行）调用 analyze → targetedE2e.eligible: true、tests: [] → plan 调用 publicIssueAnalysis → TypeError: Cannot read properties of undefined (reading 'id')，中止 issue 创建（已通过真实 CLI 用 probe 确认）。当前 workflow 调用方从同一份下载日志派生两者，无法产生该形状，但 CLI 自身的契约接受零位置参数日志的 --jobs。修复：对该 eligible 分支加守卫，或使用可选链。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| function autofixDisposition(analysis) { | ||
| return isAutofixEligible(analysis) |
There was a problem hiding this comment.
[Suggestion] The JSDoc block immediately above (A run that failed before any test result was reported … keeps the original per-commit marker and title.) documents renderPerCommitBody, but the newly inserted autofixDisposition now sits between that comment and its subject, so the comment is misattached to autofixDisposition. — Concrete cost: a reader (or IDE hover-doc) sees a per-commit marker/title description attached to a function that returns an eligibility string — misleading anyone tracing the per-commit path.
Move autofixDisposition above the JSDoc block (or re-attach the comment to renderPerCommitBody with a separating blank line).
中文说明
紧贴上方的 JSDoc 块（A run that failed before any test result was reported … keeps the original per-commit marker and title.）描述的是 renderPerCommitBody，但新插入的 autofixDisposition 现在位于该注释与其描述对象之间，因此该注释被错误地挂到了 autofixDisposition 上。— 具体代价：读者（或 IDE 悬停文档）会看到一段关于 per-commit 标记/标题的描述被挂到一个返回 eligibility 字符串的函数上——会误导任何追踪 per-commit 路径的人。修复：把 autofixDisposition 移到该 JSDoc 块之上（或用空行把注释重新挂回 renderPerCommitBody）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| COMMENT_ID: '${{ needs.issue-autofix.outputs.comment_id }}' | ||
| CLAIMED: '${{ needs.issue-autofix.outputs.claimed }}' |
There was a problem hiding this comment.
[Suggestion] COMMENT_ID and CLAIMED (and the issue-autofix job outputs comment_id/claimed that feed them) are dead state left over from the pre-marker-based withdraw design. The rewritten Withdraw claim on failure step recovers the claim comment via the <!-- autofix-claim … --> marker search, and a whole-file grep shows ${COMMENT_ID}/${CLAIMED} are referenced nowhere in the publish job. — Concrete cost: maintenance noise — two job outputs exist solely to feed two unconsumed env vars, and claimed can be mistaken for a load-bearing gate when the real gate is the step-level steps.claim.outputs.claimed.
| COMMENT_ID: '${{ needs.issue-autofix.outputs.comment_id }}' | |
| CLAIMED: '${{ needs.issue-autofix.outputs.claimed }}' |
Delete these two env lines and drop the now-unconsumed comment_id/claimed entries from the issue-autofix job outputs: block (keep the step-level claimed output, which still gates Post claim comment).
中文说明
COMMENT_ID 和 CLAIMED（以及为它们供值的 issue-autofix job 输出 comment_id/claimed）是从基于 marker 之前的 withdraw 设计遗留下来的死状态。重写后的 Withdraw claim on failure 步骤通过 <!-- autofix-claim … --> marker 搜索来恢复 claim 评论，且全文件 grep 显示 ${COMMENT_ID}/${CLAIMED} 在 publish job 中没有任何引用。— 具体代价：维护噪声——两个 job 输出仅为两个未被消费的 env 变量供值而存在，并且 claimed 可能被误认为是承重门控，而真正的门控是步骤级的 steps.claim.outputs.claimed。修复：删除这两行 env，并从 issue-autofix job 的 outputs: 块中移除现已未被消费的 comment_id/claimed（保留步骤级 claimed 输出，它仍然门控 Post claim comment）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| expect( | ||
| claimIssueStep.indexOf('autofix-approved-prose-sha256'), | ||
| ).toBeLessThan( | ||
| claimIssueStep.indexOf('--remove-label "${AUTOFIX_APPROVED_LABEL}"'), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This ordering assertion's left operand claimIssueStep.indexOf('autofix-approved-prose-sha256') has no toContain guard in claimIssueStep (the only toContain for that literal is on recordApprovedIssueProseStep, a different step). If a refactor drops the claim step's prose-approval-marker scan (qwen-autofix.yml:1235), indexOf returns -1 and -1 < positive is vacuously true — the test stays green while the property "the claim verifies the recorded prose-approval marker BEFORE the approval label is removed" is unenforced (probe-confirmed: mutating the marker to a different string leaves the test passing). — This is one instance of a recurring pattern in this PR's new tests; the same vacuous-indexOf / non-discriminating shape also appears at lines 5537 (claimOidOutputIndex), 2269 (git rev-parse HEAD), 5443 (the split label-conjunction assertion — an and→or regression ships green), 1916 (the [!=]= character class matches both != and ==, so a base-OID guard inversion is not detected), 6911 (a not.toContain needle that can never occur given the workflow's line-wrapping), 7055 (the unsafe\nname.ts resolver fixture does not discriminate NUL- vs newline-splitting), plus load-autofix-e2e-metadata.test.mjs lines 127/229/271/307 (guards/call sites the tests never drive to reject).
| expect( | |
| claimIssueStep.indexOf('autofix-approved-prose-sha256'), | |
| ).toBeLessThan( | |
| claimIssueStep.indexOf('--remove-label "${AUTOFIX_APPROVED_LABEL}"'), | |
| ); | |
| expect(claimIssueStep.indexOf('autofix-approved-prose-sha256')).toBeGreaterThan(-1); | |
| expect( | |
| claimIssueStep.indexOf('autofix-approved-prose-sha256'), | |
| ).toBeLessThan( | |
| claimIssueStep.indexOf('--remove-label "${AUTOFIX_APPROVED_LABEL}"'), | |
| ); |
Add a toBeGreaterThan(-1) presence guard before the ordering check (mirroring addInProgressIndex/actorCheckIndex elsewhere in this file), and apply the same hardening to the sibling locations above.
中文说明
此排序断言的左操作数 claimIssueStep.indexOf('autofix-approved-prose-sha256') 在 claimIssueStep 中没有 toContain 守卫（该字面量唯一的 toContain 位于 recordApprovedIssueProseStep，是另一个步骤）。如果某次重构删除了 claim 步骤的 prose-approval-marker 扫描（qwen-autofix.yml:1235），indexOf 返回 -1，而 -1 < positive 会空泛地为真——测试保持全绿，但“claim 在移除 approval 标签之前先验证已记录的 prose-approval marker”这一属性已不再被强制（已用 probe 确认：把 marker 改成不同字符串后测试仍通过）。— 这是本 PR 新测试中一个反复出现模式的实例；同样的空泛 indexOf / 无判别力形状还出现在：5537 行（claimOidOutputIndex）、2269 行（git rev-parse HEAD）、5443 行（被拆分的标签合取断言——and→or 回归会全绿上线）、1916 行（[!=]= 字符类同时匹配 != 和 ==，因此检测不到 base-OID 门控反转）、6911 行（鉴于 workflow 的换行方式，not.toContain 的 needle 永远不可能出现）、7055 行（unsafe\nname.ts resolver fixture 无法区分 NUL 切分与换行切分），以及 load-autofix-e2e-metadata.test.mjs 的 127/229/271/307 行（测试从未驱动到拒绝分支的守卫/调用点）。修复：在排序检查前增加 toBeGreaterThan(-1) 存在性守卫（参照本文件其他处的 addInProgressIndex/actorCheckIndex），并对上述同类位置作同样加固。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
…flows (#8318) - Redirect warning echo to stderr inside jq pipeline to prevent parse errors - Revert issue search to --state open so recurring failures create fresh issues - Wrap job-list fetch in if ! with graceful fallback to empty list - Make concurrent-reuse search best-effort with || fallback - Reject uid/gid of 0 in autofix-cli-launcher privilege-drop guard - Preserve occurrence block and signature marker in publicMachineMarkers - Guard publicIssueAnalysis against empty tests array - Re-attach JSDoc to renderPerCommitBody (was misattached to autofixDisposition) - Remove dead COMMENT_ID/CLAIMED env vars and unused job outputs - Add indexOf presence guards before ordering assertions in workflow tests
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedCritical findings
Suggestions implemented
Verification
中文说明已处理的评审反馈Critical 发现
已实施的 Suggestion
验证
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/模型 |
Code reviewOverviewThis PR closes the strongest remaining gap in the Autofix publication chain: a candidate could previously pass deterministic checks without ever re-proving the exact E2E case that created the issue. The new design is a fail-closed pipeline: authenticated failure metadata travels as an issue-bound Actions artifact (never as editable prose), approval is bound to a SHA-256 digest of the exact title/body, the claim is an atomic Strengths worth calling out:
No blocking correctness bug found. Findings below are one functional regression, several drift/maintenance risks, and validation gaps the PR itself acknowledges. Findings1. Non-eligible recurrences no longer append to existing issue bodies, but the body still promises they will. In 2. Eligible recurrences overwrite maintainer body edits. For eligible issues, a trusted recurrence rewrites the body from 3. Two 4. 5. 6. 7. Any unparseable artifact name from the trusted producer hard-fails the whole load. In 8. Residual: 9. Ubuntu validation is the real remaining risk (acknowledged in the PR). Beyond the listed DAC/ Test coverageExtensive and well-aimed: 124 workflow-contract tests plus 61 Node tests covering ordering invariants (scope-check-before-install, seal-before-build, report-before-vitest), credential scrubbing, symlink/UTF-8/newline path attacks, artifact provenance ordering and tie-breaks, and prompt-injection stripping from public issue prose. The VerdictSolid, carefully reasoned hardening; the fail-closed posture is applied consistently, including in the uncomfortable recovery paths. Item 1 is the only functional regression I would fix before merge; items 3–7 are follow-up-sized; items 8–9 are known/acknowledged risks that need an Ubuntu validation run rather than code changes. |
|
🔀 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 review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Autofix review-round summary (PR #8318)Feedback triage
Root cause of the failing Test checkReproduced the CI Test-job steps locally. The deterministic failure was in the The test pins the total number of I audited all five new sites before bumping the pin: each is an inline Local failures that are environment-specific, not code defects
Verification (commands actually run, in order)
Changes this round
No conflicts ( 中文说明Autofix 评审轮次总结（PR #8318）反馈分类
Test 检查失败的根因在本地复现了 CI Test 作业的各个步骤。确定性的失败出现在 该测试钉住了 在提升钉住值之前，我逐一审查了这五个新增位：它们都是内联的 本地失败但属于环境问题、并非代码缺陷的部分
验证（实际执行的命令，按顺序）
本轮改动
无冲突（ 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/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review-round summary (PR #8318) — no action this roundFeedback triage
No reviews, inline comments, or issue-level comments newer than the last evaluation were present, and no other check is failing. Why no code change is warrantedThe failed check is the LLM code-review job. Evidence gathered from the PR's comment/review dumps supplied to this round and from the workflow definition:
Recovery pathRe-run the review once the runner pool is healthy: comment VerificationNo change was made, so no fix verification applies. The working tree is clean and unchanged ( 中文说明Autofix 评审轮次总结（PR #8318）—— 本轮不做任何改动反馈分诊
不存在比上次评估更新的评审（reviews）、行内评论或议题级评论，也没有其他检查处于失败状态。 为什么不需要做代码改动失败的检查是 LLM 代码评审任务。证据来自本轮提供的 PR 评论/评审数据导出，以及工作流定义本身：
恢复路径等运行器池恢复健康后重试评审即可：评论 验证本轮没有做任何改动，因此不涉及修复验证。工作区干净且无变化（ Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite (the targeted-E2E path this PR is built around) did not run locally; macOS/Windows Test legs were also skipped (this PR changes no npm-workspace code; the sealed execution path was exercised only through mocked wrapper tests).
Not reviewed: reverse-audit — stopped after round 1 without two consecutive dry rounds: round 1 covered all 27 chunks and surfaced 24 confirmed findings (all verified in Step 4); the diff's 4,700-line test suite invites an unbounded tail of test-pinning findings.
Not reviewed: You are review agent verify — Verification agent (round... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite (the targeted-E2E path this PR is built around) did not run locally; macOS/Windows Test legs were also skipped (this PR changes no npm-workspace code; the sealed execution path was exercised only through mocked wrapper tests)。
未审查：reverse-audit — stopped after round 1 without two consecutive dry rounds: round 1 covered all 27 chunks and surfaced 24 confirmed findings (all verified in Step 4); the diff's 4,700-line test suite invites an unbounded tail of test-pinning findings。
未审查：You are review agent verify — Verification agent (round...——它被指向 diff 的行却从未打开：有工具调用，但没有一次读取 diff。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| ! gh issue edit "${ISSUE}" --repo "${REPO}" \ | ||
| --remove-label "${AUTOFIX_APPROVED_LABEL}"; then |
There was a problem hiding this comment.
[Critical] In Neutralize expired targeted E2E requirement, the requirement label is removed BEFORE the approval is consumed. A transient gh failure on this second edit leaves the issue approved-but-no-longer-required — exactly the degraded state this step's own comment says it must prevent — and no later run repairs it: with the requirement label gone, Load targeted E2E requirement writes required=false forever, neutralize never re-fires, and the next cycle publishes on deterministic gates alone, skipping the targeted-E2E proof the issue was routed as requiring. — Failure scenario: expired metadata → neutralize runs → --remove-label autofix/e2e-verification-required succeeds → this edit (consuming autofix/approved) hits a transient API failure → step exits 1 → issue permanently carries approval without requirement → next cycle publishes without the targeted-E2E proof.
Swap the two removals so every partial state fails closed:
# 1) consume the approval FIRST
if jq -e --arg label "${AUTOFIX_APPROVED_LABEL}" \
'(.labels // []) | map(.name) | index($label) != null' \
<<< "${live_issue_json}" > /dev/null &&
! gh issue edit "${ISSUE}" --repo "${REPO}" \
--remove-label "${AUTOFIX_APPROVED_LABEL}"; then
echo '::error::Failed to consume the approval for the expired targeted E2E requirement.'
exit 1
fi
# 2) THEN remove the requirement label
if ! gh issue edit "${ISSUE}" --repo "${REPO}" \
--remove-label "${E2E_REQUIRED_LABEL}"; then
echo '::error::Failed to remove the expired targeted E2E requirement label.'
exit 1
fi(Approval-consumption failure leaves the requirement label in place → next run re-enters neutralize and retries; requirement-removal failure after consumption leaves an unapproved, still-required issue → fails closed until re-approval.)
中文说明
在 Neutralize expired targeted E2E requirement 中，requirement 标签在消费 approval 之前被移除。第二次 edit 若遇到瞬时 gh 失败，issue 会停留在"已批准但不再要求 E2E"的降级状态——正是本步骤注释声明必须防止的状态——且后续任何运行都无法修复：requirement 标签消失后，Load targeted E2E requirement 永远写入 required=false，neutralize 不会再触发，下一周期将仅凭确定性门禁发布，跳过该 issue 被路由时所要求的 targeted-E2E 证明。— 失败场景：元数据过期 → neutralize 运行 → --remove-label autofix/e2e-verification-required 成功 → 此次 edit（消费 autofix/approved）遇到瞬时 API 失败 → 步骤退出 1 → issue 永久带着 approval 却没有 requirement → 下一周期发布时跳过 targeted-E2E 证明。修复：先消费 approval，再移除 requirement 标签——这样所有部分失败状态都 fail closed，下次运行会重新进入 neutralize。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const baseProse = | ||
| autofixEligible && !strippedProse.includes(FAILING_TESTS_HEADING) | ||
| ? `${strippedProse}\n\n${FAILING_TESTS_HEADING}\n\n${testLines.join('\n')}` | ||
| : strippedProse; |
There was a problem hiding this comment.
[Critical] The recurrence merge keys redaction off the CURRENT RUN's eligibility, not the issue's — probe-reproduced via runCli plan. An ineligible recurrence merged into a machine-redacted eligible issue re-injects raw log-sourced test identifiers under ## Also failing while the redaction note stays, and silently voids the recorded approval digest. — Failure scenario: issue created eligible (body redacted to case <key> lines, approval digest marker recorded). A later run fails the same trusted test plus any non-allowlisted test (routine — the allowlist holds exactly one file), or the jobs-API call transiently fails ([] fallback → ineligible). Dedupe matches the eligible issue; plan runs with autofixEligible=false → publicMachineMarkers skipped → merged body carries RAW identifiers (probe observed both) → file_issue's ineligible branch overwrites the approved issue's body unconditionally → body no longer hashes to the recorded marker → the scan drops the issue until manual re-approval. — Suggested fix: key the merge on the existing body's redacted state (when the body carries AUTOFIX_REDACTION_NOTE, render the recurrence redacted and suppress raw ## Also failing additions), and make file_issue's ineligible branch comment-only for routed/approved issues instead of overwriting the body.
中文说明
复发合并按"当前运行"的 eligibility 而非 issue 的状态决定是否脱敏——已通过 runCli plan 探针复现。一次 ineligible 的复发合并进已机器脱敏的 eligible issue 时，会在 ## Also failing 下重新注入原始日志来源的测试标识符（脱敏说明仍在），并静默作废已记录的 approval 摘要。— 失败场景：issue 以 eligible 创建（正文脱敏为 case <key> 行，approval 摘要 marker 已记录）。后续某次运行除同一 trusted 测试外还有任何非 allowlist 测试失败（常态——allowlist 只有一个文件），或 jobs API 瞬时失败（[] 兜底 → ineligible）。去重命中该 eligible issue；plan 以 autofixEligible=false 运行 → 跳过 publicMachineMarkers → 合并后的正文携带原始标识符（探针已观察到）→ file_issue 的 ineligible 分支无条件覆盖已批准 issue 的正文 → 正文不再匹配已记录 marker 的哈希 → 扫描丢弃该 issue，直到人工重新批准。— 修复建议：按已有正文的脱敏状态决定合并方式（正文携带 AUTOFIX_REDACTION_NOTE 时以脱敏形式渲染复发内容并抑制原始 ## Also failing 追加），并让 file_issue 的 ineligible 分支对已路由/已批准 issue 只发评论、不覆盖正文。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| if: |- | ||
| ${{ github.event_name == 'issues' && needs.route.outputs.do_issue == 'true' }} |
There was a problem hiding this comment.
[Critical] Record approved issue prose re-records an approval marker for the CURRENT prose on ANY routed issues event — prose edited after approval is silently re-approved by the next routine trusted label/assignment event, without the approval label ever being re-applied. This defeats the PR's central invariant ("editing issue prose consumes the effective approval until a maintainer re-applies the approval label"). — Failure scenario: maintainer approves prose v1 → marker(v1) recorded. The issue author (untrusted — authors can always edit their own issues) edits the body to v2; edited is not in on: issues types and nothing removes autofix/approved on edit. Any later trusted issues event routes to the issue phase (the route job dispatches late non-trigger labels on approved+ready issues; bot-assignment events also trigger): this step finds no marker for digest(v2), its live re-validation passes (live == payload == v2, labels present), and posts marker(v2). The next scan treats v2 as approved; claim binds live prose v2 and never sees v1 — the agent executes attacker-edited prose with CI_DEV_BOT_PAT. — Suggested fix: when bot approval-marker comments exist but none matches the current digest, treat that as proof of a post-approval edit and fail closed; or restrict fresh recording to the autofix/approved labeled action / authenticated CI recurrences.
中文说明
Record approved issue prose 会在任何被路由到 issue 阶段的 issues 事件上，为"当前"正文重新记录 approval marker——批准之后被编辑的正文，会被下一个常规可信 label/assignment 事件静默"重新批准"，而无需任何人重新应用 approval 标签。这破坏了本 PR 的核心不变量（"编辑 issue 正文即消费有效批准，直到维护者重新应用 approval 标签"）。— 失败场景：维护者批准正文 v1 → 记录 marker(v1)。issue 作者（不可信——作者永远可以编辑自己的 issue）把正文改为 v2；edited 不在 on: issues types 中，且编辑时没有任何逻辑移除 autofix/approved。之后任何可信 issues 事件都会路由到 issue 阶段（route job 会把 approved+ready issue 上的迟到非触发标签事件、以及 bot assignment 事件转发过来）：本步骤发现没有 digest(v2) 对应的 marker，live 复核通过（live == payload == v2、标签齐全），于是发布 marker(v2)。下次扫描把 v2 视为已批准；claim 绑定 live 正文 v2、永远见不到 v1——agent 将带着 CI_DEV_BOT_PAT 执行攻击者编辑过的正文。— 修复建议：当存在 bot approval-marker 评论但没有一个匹配当前摘要时，视为"批准后被编辑"的证据并 fail closed；或把新 marker 的记录限制为 autofix/approved 标签事件 / 可信 CI 复发。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const ignored = execFileSync('git', args, { | ||
| cwd: workspace, | ||
| encoding: 'utf8', | ||
| maxBuffer: MAX_GIT_OUTPUT_BYTES, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] The output audit decodes raw -z git output as UTF-8 with silent replacement (here and at the second execFileSync below); a symlink whose NAME contains invalid UTF-8 bytes is mangled to U+FFFD, lstat misses it (ENOENT → not a symlink), and the mangled name still satisfies the dist/ prefix allowlist — defeating "reject symbolic links even below allowed output paths". Probe-reproduced: dist/link-<0xff> → /etc/passwd returns [] (silently allowed), while the sibling listProtectedCandidateChanges throws on the same input class — the asymmetry is internal to this file. (Downgraded from Critical: the 0555 finalize seal bounds the planting window and no sealed-phase consumer follows the hidden link — but this is the layer whose contract is fail-closed.) — Suggested fix:
const ignored = execFileSync('git', args, { cwd: workspace, maxBuffer: MAX_GIT_OUTPUT_BYTES });
const ignoredText = ignored.toString('utf8');
if (Buffer.from(ignoredText).equals(ignored) === false) {
throw new Error('Verification output path is not valid UTF-8');
}(same round-trip validation listProtectedCandidateChanges uses; apply to both reads)
中文说明
输出审计把原始 -z git 输出按 UTF-8 解码并静默替换非法字节（此处与下方第二个 execFileSync 均如此）；文件名包含非法 UTF-8 字节的符号链接会被替换成 U+FFFD，lstat 找不到它（ENOENT → 视为非符号链接），而替换后的名字仍满足 dist/ 前缀 allowlist——"即使在允许的输出路径下也拒绝符号链接"被绕过。探针复现：dist/link-<0xff> → /etc/passwd 返回 []（被静默放行），而同类输入在 listProtectedCandidateChanges 中会抛错——不对称就在本文件内部。（从 Critical 降级：0555 finalize 封印限制了植入窗口，且 sealed 阶段没有消费者会跟随该隐藏链接——但本层的契约是 fail-closed。）— 修复建议：把两次读取都改为 Buffer，并执行与 listProtectedCandidateChanges 相同的 UTF-8 往返校验，遇到非法字节即 fail closed。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| ); | ||
| // The resolver reads NUL-delimited input; dropping -z makes its | ||
| // while-read loop exit immediately and CHANGED_PKGS silently empty. | ||
| expect(verify).toMatch(/git diff --name-only -z origin\/main\.\.\.HEAD/); |
There was a problem hiding this comment.
[Suggestion] This pin only requires that SOME git diff --name-only -z origin/main...HEAD exists in the verify step — not that -z is on the pipe consumed by resolve-owning-packages.sh; the step contains a second plain diff pipe (to check-autofix-contracts.sh, repo-hygiene.yml:785) one line above. Probe: mutation "move -z to the contracts pipe and drop it from the resolver pipe" keeps the test green — the resolver (NUL reader) then exits 0 with empty output → CHANGED_PKGS empty → repo-hygiene silently skips all package tests; and the contracts script strips NUL bytes in command substitution → grep -Fxq never matches. The sibling pin in qwen-autofix-workflow.test.js (~6828) binds the full pipe — use that shape. — Suggested fix:
expect(verify).toMatch(/git diff --name-only -z origin\/main\.\.\.HEAD \\\n\s*\| docker run .*resolve-owning-packages\.sh/);中文说明
该断言只要求 verify 步骤中存在某个 git diff --name-only -z origin/main...HEAD——并未把 -z 绑定到 resolve-owning-packages.sh 消费的那条管道；同步骤上一行还有另一条不带 -z 的 diff 管道（通往 check-autofix-contracts.sh，repo-hygiene.yml:785）。探针：变异"把 -z 挪到 contracts 管道并从 resolver 管道移除"后测试仍为绿——resolver（NUL 读取器）会输出为空并以 0 退出 → CHANGED_PKGS 为空 → repo-hygiene 静默跳过所有包测试；contracts 脚本在命令替换中剥掉 NUL 字节 → grep -Fxq 永不匹配。qwen-autofix-workflow.test.js（~6828）中的姊妹断言绑定了完整管道——请采用该形状。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| expect(issueAutofixJob).toContain('! -f "${WORKDIR}/agent-timeout"'); | ||
| expect(issueAutofixJob).toContain('! -f "${WORKDIR}/agent-api-error"'); |
There was a problem hiding this comment.
[Suggestion] Pattern — workflow/test assertions don't pin the wiring they claim. This instance: the decline test pins only the two sentinel exclusions; the -s "${WORKDIR}/failure.md" precondition (yml:1447) is asserted nowhere — probe: dropping it keeps 118/118 green. — Failure scenario: a sentinel-less failure (e.g. missing AUTOFIX_OPENAI_API_KEY exits before run-agent.mjs writes failure.md) then records a decline → withdraw adds autofix/skip → a transient/config failure permanently skips the issue (which also already had its approval consumed). — Suggested fix: expect(issueAutofixJob).toContain('-s "${WORKDIR}/failure.md"') (or pin the full conjoined condition line).
中文说明
模式——断言没有固定它们所声称的接线。本例：decline 测试只固定了两个 sentinel 排除项；-s "${WORKDIR}/failure.md" 前置条件（yml:1447）没有任何断言——探针：删除后 118/118 仍全绿。— 失败场景：无 sentinel 的失败（例如缺少 AUTOFIX_OPENAI_API_KEY 时会在 run-agent.mjs 写出 failure.md 之前退出）也会被记录为 decline → withdraw 添加 autofix/skip → 瞬时/配置故障导致 issue 被永久跳过（且其 approval 已被消费）。— 修复建议：expect(issueAutofixJob).toContain('-s "${WORKDIR}/failure.md"')（或固定完整的联合条件行）。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| expect(agentResultIndex).toBeGreaterThan(-1); | ||
| expect(verifyResultIndex).toBeGreaterThan(agentResultIndex); |
There was a problem hiding this comment.
[Suggestion] Pattern — workflow/test assertions don't pin the wiring they claim. This instance: the elif-ladder test pins the three conditions' relative order and the four DETAIL messages' presence, but nothing pairs each condition with its message — probe: transposing the verify/targeted DETAIL strings ships green. — Failure scenario: on a deterministic verification failure (AGENT=success, VERIFY=failure, TARGETED=skipped) the withdraw comment blames the skipped targeted job, sending the responder to empty logs — exactly the misattribution the test's adjacent comment claims to prevent. — Suggested fix: bound each message between its condition and the next condition with index-range assertions (e.g. withdrawClaimStep.indexOf('"${VERIFY_RESULT}" != \'success\'') < withdrawClaimStep.indexOf('Deterministic verification of the candidate failed…') < indexOf(next condition)).
中文说明
模式——断言没有固定它们所声称的接线。本例：elif 阶梯测试固定了三个条件的相对顺序与四条 DETAIL 消息的存在，但没有把每个条件与其消息配对——探针：交换 verify/targeted 的 DETAIL 字符串后仍可绿着上线。— 失败场景：确定性验证失败（AGENT=success、VERIFY=failure、TARGETED=skipped）时，withdraw 评论会归咎于被跳过的 targeted job，把响应者引向空日志——正是测试旁注声称要防止的错误归因。— 修复建议：用索引区间断言把每条消息限定在其条件与下一条件之间（如 withdrawClaimStep.indexOf('"${VERIFY_RESULT}" != \'success\'') < withdrawClaimStep.indexOf('Deterministic verification of the candidate failed…') < indexOf(下一条件)）。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
|
||
| A second fresh job downloads the original candidate artifact, requires its OID to equal the deterministic job output, loads current issue-bound metadata before starting any candidate lifecycle script, and runs the trusted targeted verifier. It uploads a verified artifact containing only the original bundle, fixed OID, human-authored PR files, and the verifier report. | ||
|
|
||
| A third fresh publication job contains the bot PAT. It executes no candidate package script or test code. Before changing issue ownership, the claim operation creates a unique commit whose tree and parent are the independently captured trusted base and whose trusted message binds the workflow run ID and attempt. It atomically creates `refs/heads/autofix/claim-issue-<issue>` at that unique OID with an expected-absent lease. Overlapping scheduled runs cannot both create the ref, and an old run cannot mistake a later delete-and-recreate cycle for its own claim, so only the exact owner may assign the Autofix bot, remove approval, execute candidate code, withdraw ownership, or release the ref. A failure before any issue ownership write removes the ref with its exact-OID lease. Once an ownership write may have partially succeeded, API uncertainty preserves the unique ref for recovery. Every later job receives the claim OID as immutable job output and requires the live ref to remain at that exact value. A missing or mismatched ref fails closed and leaves ownership untouched rather than allowing an older run to withdraw a newer claim. |
There was a problem hiding this comment.
[Suggestion] The doc narrates the claim operation inside the paragraph whose subject is "A third fresh publication job" and states "Every later job receives the claim OID", but in qwen-autofix.yml the claim is created by the FIRST issue-autofix job (before Develop fix), and only issue-autofix-publish receives/validates CLAIM_OID — verify and targeted-e2e contain zero references. Under the paragraph's own placement the text is vacuous (if the third job created the claim, no "later job" exists). — Failure scenario: a maintainer extending the chain uses this doc as the map — adds a claim step to the publication job (whose expected-absent lease then fails permanently against the ref the first job already created), or skips the claim check in a new post-verification job. — Suggested fix: rewrite to match the implementation — the claim runs in the issue job before any ownership write; deterministic/targeted/publication jobs validate the live claim ref via the claim OID output, and only publication writes issue state.
中文说明
文档把 claim 操作放在主语为"A third fresh publication job"的段落中，并称"Each later job receives the claim OID"，但在 qwen-autofix.yml 中 claim 由第一个 issue-autofix job 创建（在 Develop fix 之前），且只有 issue-autofix-publish 接收/校验 CLAIM_OID——verify 与 targeted-e2e 零引用。按该段落自身的布局，这段文字是空转的（若 claim 由第三个 job 创建，则不存在"后续 job"）。— 失败场景：扩展该链路的维护者以此文档为地图——在 publish job 里新增 claim 步骤（其 expected-absent lease 会永久失败于第一个 job 已创建的 ref），或在新建的验证后 job 中跳过 claim 检查。— 修复建议：按实现改写——claim 在 issue job 中、任何所有权写入之前执行；确定性/targeted/publish job 通过 claim OID 输出校验存活 claim ref，只有 publish 写 issue 状态。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
|
||
| For a targeted issue, the writer binds the issue number into the metadata and uploads an immutable artifact named `autofix-e2e-failure-<issue>-<source-run>-<source-attempt>-<producer-run>-<producer-attempt>`. The loader enumerates all live artifacts for the issue, validates every name against authenticated producer and source runs, and selects the newest trusted source recurrence, using producer run, producer attempt, and artifact ID as immutable tie-breakers. A closed bot-authored issue remains the authoritative match for its public failure marker even if another open duplicate exists, so recurrence cannot recreate an automatically approved replacement after a maintainer closes the original issue. | ||
|
|
||
| An eligible issue starts or resumes a routing transaction only while it remains open, ready, approved, bot-owned, unclaimed, and unlinked. The writer applies `autofix/routing` before changing an existing issue body, or creates a new issue with the routing lock already present. Autofix excludes that label from forced, scheduled, selected, and claim paths. After the immutable artifact upload, the writer revalidates the live issue state, adds `autofix/e2e-verification-required`, records a bot-authored SHA-256 marker over the exact current title and body, and only then removes `autofix/routing`. An issues event routed to the issue phase records the event payload's exact title and body as the approval marker, only when the live issue still matches that payload, and skips events whose marker is already present: the two-label gate is often completed by the second label event, the bot assignment, or a late non-trigger label rather than the `autofix/approved` labeled event itself, and keying on that event alone left such approvals unrecorded. Scheduled and event-driven Autofix candidates must match a bot-authored approval marker during scanning and again immediately before claim, so editing issue prose consumes the effective approval until a maintainer re-applies the approval label or a later authenticated CI recurrence records its trusted update. At rollout, approvals labeled before the marker existed are grandfathered: the scan backfills the missing marker from the approval label event timestamp when it predates a fixed cutover and the issue itself has not been updated since the cutover, after which the ordinary marker checks apply unchanged. An issue edited after the cutover therefore cannot ride the grandfather path indefinitely; its current prose was never approved, so it fails closed until a maintainer re-applies the approval label. Claim records the live prose digest as an immutable job output for every route, including manual dispatch; proof revalidation and every publication-time issue check require the current title and body to retain that digest. Manual dispatch therefore bypasses the approval marker but not the end-to-end prose binding. Any interruption leaves a visible fail-closed lock; a later authenticated recurrence may resume the transaction only while the human-controlled ready, approval, ownership, and cancellation signals still permit it. Cancellation uses structured state—ownership, labels, issue state, and linked PRs—rather than attempting to interpret untrusted natural-language comments. The writer never removes and later restores approval as part of publication. |
There was a problem hiding this comment.
[Suggestion] The doc states the grandfather gate as two conditions (approval label event predates cutover; issue not updated since cutover), omitting the implementation's third conjunct — no activity beyond a 60-second window after the approval label event (qwen-autofix.yml:969-972) — and says the marker is backfilled "from the approval label event timestamp" when it is the digest of the CURRENT prose (the timestamp only gates eligibility). The code is stricter than the doc (bounded harm), but rollout-safety reasoning works from a wrong gate. — Suggested fix: reword to the actual three-conjunct gate, and state that the backfilled marker is the SHA-256 digest of the current title/body.
中文说明
文档把 grandfather 门禁表述为两个条件（approval 标签事件早于 cutover；issue 在 cutover 之后未更新），遗漏了实现的第三个合取项——approval 标签事件之后 60 秒窗口内无活动（qwen-autofix.yml:969-972）——并称 marker 是"从 approval 标签事件时间戳"回填的，而实际上是当前正文的摘要（时间戳只用于资格判定）。代码比文档更严格（危害有限），但关于灰度安全性的推理会基于错误的门禁。— 修复建议：按实际的三合取门禁改写，并说明回填的 marker 是当前 title/body 的 SHA-256 摘要。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
|
||
| Every completed `workflow_run` remains independently processable; neither the workflow nor its jobs use Actions concurrency groups that could replace a pending failure. First-occurrence deduplication remains marker-based, but only issues authored by the configured Autofix bot may be reused; a user-created issue containing a publicly computable marker cannot be promoted into trusted agent input. A recurrence may update the issue body. For eligible issues the body is rebuilt from machine markers and occurrence lines only, so maintainer prose added to the body is discarded; maintainers should use comments for investigative notes. For non-eligible issues the existing body is preserved and only the recurrence trailer is refreshed. In both cases the writer re-reads live ownership and cancellation state before routing and never restores ready/approved labels or bot assignment after a maintainer has opted out, requested information/retesting, linked another PR, or changed ownership. Publication remains fail-closed. GitHub does not provide an atomic lock for a previously unseen failure signature, so two simultaneous first occurrences can still create duplicate issues and independent Autofix attempts. A fixed global or issue-scoped concurrency group is not an acceptable workaround because GitHub may replace a pending `workflow_run` and lose one failure event. Preventing both event loss and duplicate publication requires a future external atomic store or a canonical cross-issue claim key; the current design prioritizes retaining every authenticated failure and leaves duplicate reconciliation to maintainers. | ||
|
|
||
| For a targeted issue, the writer binds the issue number into the metadata and uploads an immutable artifact named `autofix-e2e-failure-<issue>-<source-run>-<source-attempt>-<producer-run>-<producer-attempt>`. The loader enumerates all live artifacts for the issue, validates every name against authenticated producer and source runs, and selects the newest trusted source recurrence, using producer run, producer attempt, and artifact ID as immutable tie-breakers. A closed bot-authored issue remains the authoritative match for its public failure marker even if another open duplicate exists, so recurrence cannot recreate an automatically approved replacement after a maintainer closes the original issue. |
There was a problem hiding this comment.
[Suggestion] The doc claims a closed bot-authored issue "remains the authoritative match" for its marker even when an open duplicate exists, but the matcher takes .[0] of a --state all search with no closed-issue preference — when the open duplicate ranks first, the recurrence re-routes the duplicate and the closed original receives nothing. The guaranteed outcome (no automatically approved replacement after a maintainer closes the original) still holds narrowly, but the stated mechanism — closed-issue preference — does not exist anywhere in the workflow. — Suggested fix: implement the preference in the matcher, or rewrite the sentence to what the code does: any existing bot-authored match prevents creating a replacement, and an open duplicate is re-routed per its live state.
中文说明
文档声称即使存在 open 的重复 issue，closed 的 bot 创建 issue 仍是其 marker 的"权威匹配"，但匹配器只是对 --state all 搜索取 .[0]，没有任何 closed 优先——当 open 重复 issue 排在前面时，复发会重新路由到该重复 issue，closed 原件什么都不会收到。"维护者关闭原件后不会再自动创建已批准替代品"这一保证在狭义上仍成立，但所述机制——closed 优先——在 workflow 中任何地方都不存在。— 修复建议：在匹配器中实现 closed 优先；或把该句改写为代码实际行为：任何已存在的 bot 创建匹配都会阻止创建替代品，若匹配到 open 重复 issue 则按其存活状态重新路由。
— qwen3.8-max via Qwen Code /review (v0.21.5)
…8318) Address the review's three Critical findings plus twenty Suggestions. Security/correctness fixes: - Consume the approval before removing the targeted-E2E requirement label so no partial neutralize state leaves an issue approved-but-not-required. - Key recurrence-merge redaction on the existing body's redaction state, and record ineligible recurrences on approved issues as a comment instead of overwriting the machine-redacted, digest-bound body. - Treat a stale approval marker as a post-approval edit: only re-applying the approval label may record a fresh marker; every other event fails closed, and the marker record read is now fail-closed too. - Round-trip validate UTF-8 for both verification-output listings. - Retry the final claim-ref release with backoff to avoid orphaning it. Test pins (each load-bearing; the --no-renames pin is mutation-verified): rename case, retention bracket, source-run identity checks, setpriv privilege drop, VERIFIED_CANDIDATE_OID cross-checks, scan data contract, env bindings, guard/index-order ranges, and elif message pairing. Also corrects three design-doc inaccuracies.
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 7/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8318 (
|
|
🔀 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-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox), macOS and Windows test legs were skipped in CI; this PR changes no npm-workspace code and the suites collecting its changed files (HELPER_TESTS, test:scripts) were run locally green.
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox), macOS and Windows test legs were skipped in CI; this PR changes no npm-workspace code and the suites collecting its changed files (HELPER_TESTS, test:scripts) were run locally green。
未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| test_script="$(node -e 'const fs = require("node:fs"); const pkg = JSON.parse(fs.readFileSync(process.argv[1], "utf8")); process.stdout.write(pkg.scripts?.test || "");' "${p}/package.json")" | ||
| if [[ "${test_script}" == *vitest* ]]; then | ||
| "${verify_cmd}" "${GITHUB_WORKSPACE}" \ | ||
| npm run test --workspace "${p}" --if-present -- --changed "${base_oid}" --passWithNoTests |
There was a problem hiding this comment.
[Critical] R8-1: The sealed deterministic Verification gate runs every Vitest invocation AFTER finalize makes the worktree read-only (root-owned 0555 dirs, a-w files), but Vite loads the packages' TypeScript vitest configs by writing a <config>.timestamp-*.mjs temp file next to the config — EACCES under the seal, as the unprivileged verify user. Probe-confirmed: sealed TS config → vitest exit 1 EACCES; an external .mjs config → exit 0. The structural tests pin exactly this ordering (finalize < contracts < package tests), so the suite cannot catch it. — Failure scenario: any real candidate touches ≥1 vitest workspace → every package-test run fails EACCES regardless of correctness → verify fails → targeted-e2e skipped → withdraw fires and the consumed approval is lost → the publication chain this PR builds can never complete.
Suggested fix: Stage each package's vitest.config.ts (a protected input a candidate cannot change) into RUNNER_TEMP and invoke vitest with --config <staged copy> --root "${GITHUB_WORKSPACE}", or run the vitest gates before finalize.
中文说明
密封后的确定性 Verification gate 在 finalize 将 worktree 变为只读（root 属主、目录 0555、文件 a-w）之后才运行所有 Vitest 调用；但 Vite 加载各包的 TypeScript vitest 配置时，会在配置文件旁写入 <config>.timestamp-*.mjs 临时文件——非特权 verify 用户在密封环境下写入会 EACCES。探针已确认：密封下的 TS 配置 → vitest 以 EACCES 退出 1；外部 .mjs 配置 → 退出 0。结构化测试恰好固定了该顺序（finalize < contracts < package tests），因此套件无法捕获。— 失败场景：任何真实候选都会改动至少一个 vitest 工作区 → 每个包测试无论正确与否都因 EACCES 失败 → verify 失败 → targeted-e2e 被跳过 → withdraw 触发且已消费的 approval 丢失 → 本 PR 构建的发布链永远无法完成。修复：把每个包的 vitest.config.ts（候选无法改动的受保护输入）暂存到 RUNNER_TEMP，以 --config <暂存副本> --root "${GITHUB_WORKSPACE}" 调用 vitest；或在 finalize 之前运行 vitest gate。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| { | ||
| echo 'agent_declined=true' | ||
| echo "agent_detail<<${delim}" | ||
| head -c 1500 "${WORKDIR}/failure.md" | iconv -f utf-8 -t utf-8 -c |
There was a problem hiding this comment.
[Critical] R8-2: Record agent decline pipes head -c 1500 into iconv -c without the || true guard all four sibling iconv sites carry. Probe-confirmed: when byte 1500 lands inside a multi-byte character, iconv exits 1 ('incomplete character or shift sequence at end of buffer'), and under bash -eo pipefail the step dies mid-heredoc with an unterminated GITHUB_OUTPUT. The agent_declined output survives, but the job fails at this step with an opaque runner file-command error and agent_detail is lost. — Failure scenario: any decline whose failure.md exceeds 1500 bytes and is cut inside a multi-byte char (~2 of 3 cut points for this repo's bilingual agent output) → the run reads as an infrastructure failure instead of completing the decline path, and the withdraw comment relays an empty 'What the agent found' detail.
Suggested fix: Append || true to match the sibling sites.
| head -c 1500 "${WORKDIR}/failure.md" | iconv -f utf-8 -t utf-8 -c | |
| head -c 1500 "${WORKDIR}/failure.md" | iconv -f utf-8 -t utf-8 -c || true |
中文说明
Record agent decline 把 head -c 1500 管道给 iconv -c，但没有像其余四处同类 iconv 调用那样加 || true 保护。探针确认：当第 1500 字节落在多字节字符中间时，iconv 以 1 退出（'incomplete character or shift sequence at end of buffer'），在 bash -eo pipefail 下该步骤会在 heredoc 中途终止，GITHUB_OUTPUT 未闭合。agent_declined 输出会保留，但 job 在本步骤以不透明的 runner 文件命令错误失败，且 agent_detail 丢失。— 失败场景：任何 failure.md 超过 1500 字节且在多字节字符处被截断的 decline（本仓库双语 agent 输出约 2/3 的截断点如此）→ 本次运行看起来像基础设施故障而非完成 decline 路径，withdraw 评论中的 'What the agent found' 详情为空。修复：与同类调用一致，追加 || true。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| file, | ||
| ) || | ||
| /(^|\/)node_modules(?:\/|$)/.test(file) || | ||
| /(^|\/)[^/]+\.(?:test|spec)\.[cm]?[jt]sx?$/.test(file) || |
There was a problem hiding this comment.
[Critical] R8-3: The protected-path denylist misses test-only infrastructure files whose names fall outside the shape regexes, so a candidate can modify test scaffolding without tripping the trusted-inputs gate. Probe-verified in this repo: isProtectedVerificationPath returns false for packages/core/src/services/session-writer-lease.test-helper.ts (.test-helper. ≠ .test.), packages/cli/src/commands/review/lib/test-utils.ts (a FILE named test-utils.ts; only the test-utils/ directory form is protected), and packages/cli/src/serve/cdp-tunnel/acceptance/acceptance-helpers.mjs + fixture-server.mjs (acceptance/ is not listed) — controls return true. Each is imported only by tests; test-utils.ts's seedParseArgs seeds the exact state capture-local/plan-diff tests assert on. This contradicts the design doc, which lists test utilities among the inputs a candidate may not change. — Failure scenario: a candidate changes one of these helpers to force a failing test green; both consumers (node validate --base and validateCandidateScope) pass the change silently, vitest --changed selects the importing tests, they pass against tampered scaffolding, and the tampered candidate proceeds to publication.
Suggested fix: Protect *.test-helper.* files, any path whose basename is test-utils.*, and the cdp-tunnel acceptance/ helpers dir; longer-term derive protection from the test dependency graph rather than filename shapes.
中文说明
受保护路径拒绝列表遗漏了名称不匹配各形状正则的纯测试基础设施文件，候选可以在不触发可信输入 gate 的情况下修改测试脚手架。已在本仓库探针验证：isProtectedVerificationPath 对 packages/core/src/services/session-writer-lease.test-helper.ts（.test-helper. ≠ .test.）、packages/cli/src/commands/review/lib/test-utils.ts（名为 test-utils.ts 的文件；只保护了 test-utils/ 目录形式）、packages/cli/src/serve/cdp-tunnel/acceptance/acceptance-helpers.mjs + fixture-server.mjs（acceptance/ 未列入）均返回 false——对照组返回 true。这些文件只被测试导入；test-utils.ts 的 seedParseArgs 正是 capture-local/plan-diff 测试断言的状态来源。这与设计文档矛盾——文档把测试工具列为候选不得改动的输入。— 失败场景：候选修改其中一个 helper 使失败的测试变绿；两个消费方（node validate --base 与 validateCandidateScope）都会静默放行，vitest --changed 选中导入方测试，测试在被篡改的脚手架上通过，被篡改的候选继续进入发布。修复：保护 *.test-helper.* 文件、basename 为 test-utils.* 的任意路径、cdp-tunnel 的 acceptance/ helper 目录；长期应从测试依赖图推导保护，而不是文件名形状。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| assert.ok(!forged.body.includes('[run 998]')); | ||
| assert.ok(!forged.body.includes('[run 997]')); | ||
| assert.ok(!forged.body.includes('evil.example')); | ||
| assert.ok(!forged.body.includes('attacker-org/phish')); |
There was a problem hiding this comment.
[Suggestion] R8-4: The forged-occurrence test pins the domain, repo-path, and timestamp-slot checks of occurrencePattern but not its trailing $ end anchor — deleting the anchor keeps all 41 tests green (mutation-probed). — Failure scenario: a future edit drops the $; an occurrence line valid up to the run link but carrying appended markdown (e.g. · [bonus](https://evil.example/ride)) planted by anyone with body-edit rights survives the filter and is re-emitted verbatim on every machine rebuild — the forge-rides-rebuild outcome this test's comment says it prevents (both arms probe-confirmed).
Suggested fix: Add a forged case with an appended [bonus](https://evil.example/ride) segment and assert it is stripped.
中文说明
伪造 occurrence 测试固定了 occurrencePattern 的域名、仓库路径与时间戳槽检查，但没有固定结尾的 $ 锚点——删除该锚点后 41 个测试仍全绿（已做变异探针）。— 失败场景：未来某次编辑删掉 $；任何有正文编辑权限的人植入一条在 run 链接之前都合法、但追加了 markdown 的 occurrence 行（例如 · [bonus](https://evil.example/ride)），该行会存活过过滤器，并在每次机器重建时原样重新出现——正是本测试注释声称要防止的'伪造内容搭载重建'结果（两个分支均已探针确认）。修复：新增一个追加了 [bonus](https://evil.example/ride) 段的伪造用例，并断言其被剥离。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Deferred to a follow-up: this is review round 8, and repository policy (AGENTS.md) lands only Critical fixes once a PR has been through roughly five review rounds, deferring remaining Suggestions with a recorded reason so nothing is silently dropped. The missing $ end-anchor pin on occurrencePattern is a real mutation gap worth a forged-appended-segment test. It stays tracked for a follow-up issue/PR batched with the other deferred round-8 pins.
中文说明
延迟至后续处理：这是第 8 轮评审，仓库政策（AGENTS.md）规定 PR 经过约 5 轮评审后只合入 Critical 修复，其余 Suggestion 延迟处理并记录原因，确保不被静默丢弃。R8-4：occurrencePattern 结尾 $ 锚点缺失固定确实是真实的变异测试缺口，值得补充追加段伪造用例。该项将与其他延迟的第 8 轮固定项一起归入后续 issue/PR 批量处理。
| const recurrence = JSON.parse(output); | ||
| assert.ok(!recurrence.body.includes('Ignore previous instructions')); | ||
| assert.ok(!recurrence.body.includes(TRUSTED_VITEST_TEST_ID)); | ||
| assert.ok(recurrence.body.includes(`<!-- ${historicalMarker} -->`)); |
There was a problem hiding this comment.
[Suggestion] R8-5: The rebuild-retention assertions pin survival of markers/occurrences/note but not of the qwen-main-ci-failure-sig: signature line that publicMachineMarkers preserves — deleting that push keeps all 41 tests green (mutation-probed, both arms). — Failure scenario: if the signature-line push is removed by a later refactor, the signature silently disappears from every redacted issue after its first recurrence and never returns (only the create path emits it); no machine consumer exists — it is for humans comparing issues — so the loss is invisible in automation.
Suggested fix: Assert recurrence.body.match(/<!-- qwen-main-ci-failure-sig:[0-9a-f]{12} -->/) in the recurrence phase.
中文说明
重建保留断言固定了 marker/occurrence/note 的存活，但没有固定 publicMachineMarkers 保留的 qwen-main-ci-failure-sig: 签名行——删除该 push 后 41 个测试仍全绿（已做变异探针，两个分支均确认）。— 失败场景：若后续重构移除了签名行 push，签名会在每个已脱敏 issue 首次复发后静默消失且永不恢复（只有创建路径会输出签名）；签名没有机器消费方——它供人工对比 issue 使用——因此这种丢失在自动化中不可见。修复：在复发阶段断言 recurrence.body.match(/<!-- qwen-main-ci-failure-sig:[0-9a-f]{12} -->/)。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Deferred to a follow-up: this is review round 8, and repository policy (AGENTS.md) lands only Critical fixes once a PR has been through roughly five review rounds, deferring remaining Suggestions with a recorded reason so nothing is silently dropped. The missing signature-line retention assertion in the rebuild phase is a real mutation gap. It stays tracked for a follow-up issue/PR batched with the other deferred round-8 pins.
中文说明
延迟至后续处理：这是第 8 轮评审，仓库政策（AGENTS.md）规定 PR 经过约 5 轮评审后只合入 Critical 修复，其余 Suggestion 延迟处理并记录原因，确保不被静默丢弃。R8-5：重建阶段缺少签名行保留断言确实是真实的变异测试缺口。该项将与其他延迟的第 8 轮固定项一起归入后续 issue/PR 批量处理。
| 'repos/${REPO}/issues/${candidate_issue}/comments?per_page=100', | ||
| ); | ||
| expect(findCandidateIssuesStep).toContain( | ||
| 'any(.[].[]; .user.login == $bot and (.body // "") == $marker)', |
There was a problem hiding this comment.
[Suggestion] R8-40: None of the five new gh api --paginate … --slurp consumers (record:834, scan-comments:970, scan-timeline:989, claim:1369, withdraw:2482) pins --slurp; dropping it leaves 120/120 green for all five (mutation-probed individually). — Failure scenario: without --slurp the payload is a single page array and .[].[] iterates each comment's field values — jq exits 5 or finds nothing. In the record step the marker dedupe and stale-marker checks fail OPEN, so any trusted issues event re-records a fresh marker for the current prose — re-approving post-approval edits no maintainer approved, the exact bypass the step's comment says it prevents. In the claim step the marker match always fails, loudly failing every non-dispatch claim.
Suggested fix: Bind --slurp to each of the five consumers.
中文说明
五个新增的 gh api --paginate … --slurp 消费方（record:834、scan-comments:970、scan-timeline:989、claim:1369、withdraw:2482）都没有固定 --slurp；删除它后五处均 120/120 全绿（逐一变异探针确认）。— 失败场景：没有 --slurp 时负载是单页数组，.[].[] 会遍历每条评论的字段值——jq 退出 5 或找不到任何结果。record 步骤中 marker 去重与过期 marker 检查会 fail OPEN，于是任何可信 issues 事件都会为当前 prose 重新记录新 marker——重新批准了没有维护者批准过的批准后编辑，正是该步骤注释声称要防止的绕过。claim 步骤中 marker 匹配永远失败，使每个非 dispatch claim 响亮失败。修复：把 --slurp 固定到五个消费方上。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Deferred to a follow-up: this is review round 8, and repository policy (AGENTS.md) lands only Critical fixes once a PR has been through roughly five review rounds, deferring remaining Suggestions with a recorded reason so nothing is silently dropped. Pinning --slurp at all five paginate consumers is a real mutation gap. It stays tracked for a follow-up issue/PR batched with the other deferred round-8 pins.
中文说明
延迟至后续处理：这是第 8 轮评审，仓库政策（AGENTS.md）规定 PR 经过约 5 轮评审后只合入 Critical 修复，其余 Suggestion 延迟处理并记录原因，确保不被静默丢弃。R8-40：在五个 paginate 消费方上固定 --slurp 确实是真实的变异测试缺口。该项将与其他延迟的第 8 轮固定项一起归入后续 issue/PR 批量处理。
| // form also matters: piping into `as` re-roots the input and breaks | ||
| // `.assignees`/`.closedByPullRequestsReferences` at runtime. | ||
| expect(readDecisionStep.replace(/\s+/g, ' ')).toContain( | ||
| '(.labels // [] | map(.name)) as $labels | (($labels | index($ready)) and ($labels | index($approved)) and (($labels | index($routing)) == null)) and ((.assignees // []) | length > 0 and all(.login == $bot)) and ((.closedByPullRequestsReferences // []) | length == 0)', |
There was a problem hiding this comment.
[Suggestion] R8-41: This diff adds a routing-stage gate consuming .assignees and .closedByPullRequestsReferences, but the supplying gh issue view fetch is pinned only by the pre-existing prefix ending --json labels,state — dropping the two new fields from the fetch (yml:1169) keeps 120/120 green (mutation-probed). The claim-step fetch IS fully pinned; only this pin is truncated. — Failure scenario: without assignees, the gate's length > 0 and all(.login == $bot) conjunct is false for every issue → go_issue empty → the run logs 'no longer has both required labels or is still routing' and exits 0; nothing downstream re-reads assignees, so every scheduled tick re-scans and re-rejects forever — permanent silent shutdown of the issue phase with green runs.
Suggested fix: Pin the full field list scoped to the step: --json labels,state,assignees,closedByPullRequestsReferences.
中文说明
本 diff 新增了消费 .assignees 与 .closedByPullRequestsReferences 的路由阶段 gate，但提供数据的 gh issue view 请求只被以 --json labels,state 结尾的既有前缀固定——从请求中删除这两个新字段（yml:1169）后 120/120 仍全绿（已做变异探针）。claim 步骤的请求有完整固定；只有这里的固定被截断。— 失败场景：没有 assignees 时，gate 的 length > 0 and all(.login == $bot) 合取项对每个 issue 都为假 → go_issue 为空 → 运行打印 'no longer has both required labels or is still routing' 后退出 0；下游没有任何逻辑重读 assignees，于是每次定时 tick 都重新扫描并永远拒绝——issue 阶段在运行全绿中永久静默停摆。修复：按步骤范围固定完整字段列表：--json labels,state,assignees,closedByPullRequestsReferences。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Deferred to a follow-up: this is review round 8, and repository policy (AGENTS.md) lands only Critical fixes once a PR has been through roughly five review rounds, deferring remaining Suggestions with a recorded reason so nothing is silently dropped. Pinning the full --json labels,state,assignees,closedByPullRequestsReferences field list is a real mutation gap. It stays tracked for a follow-up issue/PR batched with the other deferred round-8 pins.
中文说明
延迟至后续处理：这是第 8 轮评审，仓库政策（AGENTS.md）规定 PR 经过约 5 轮评审后只合入 Critical 修复，其余 Suggestion 延迟处理并记录原因，确保不被静默丢弃。R8-41：固定完整的 --json labels,state,assignees,closedByPullRequestsReferences 字段列表确实是真实的变异测试缺口。该项将与其他延迟的第 8 轮固定项一起归入后续 issue/PR 批量处理。
| expect(withdrawClaimStep).toContain( | ||
| 'What the agent found, in case it helps a human contributor:', | ||
| ); | ||
| expect(withdrawClaimStep).toContain('"${AGENT_DECLINED}" == \'true\''); |
There was a problem hiding this comment.
[Suggestion] R8-42: The decline test pins AGENT_DETAIL plumbing only as far as the publish-job env var; the final hop DETAIL="${AGENT_DETAIL}" in withdraw (yml:2435) and the else shielding the stage-detail ladder are pinned by no assertion — although the test comment claims to pin exactly this. Mutation-probed: deleting the assignment OR the else keeps 120/120 green each. — Failure scenario: without the assignment, the decline comment posts the 'What the agent found' header over an empty body — the agent's failure.md verdict this PR adds the relay for is silently lost; every pin stays green ("${AGENT_DECLINED}" == 'true' occurs three times in withdraw, so presence proves nothing). Without the else, the stage ladder overwrites REASON/DETAIL with boilerplate false for a declined issue.
Suggested fix: Add toContain('DETAIL="${AGENT_DETAIL}"') and an ordering pin binding it to the decline selector.
中文说明
decline 测试对 AGENT_DETAIL 链路的固定只到 publish job 的 env 变量为止；withdraw 中的最后一跳 DETAIL="${AGENT_DETAIL}"（yml:2435）以及屏蔽阶段详情阶梯的 else 都没有任何断言固定——尽管测试注释声称固定的正是这个。变异探针确认：删除该赋值或删除 else 后各自 120/120 仍全绿。— 失败场景：没有该赋值时，decline 评论会在空正文上打印 'What the agent found' 标题——本 PR 专门为其添加中继的 agent failure.md 结论被静默丢失；所有固定仍全绿（"${AGENT_DECLINED}" == 'true' 在 withdraw 中出现三次，存在性什么都证明不了）。没有 else 时，阶段阶梯会用对 declined issue 而言错误的样板文本覆盖 REASON/DETAIL。修复：新增 toContain('DETAIL="${AGENT_DETAIL}"') 及按顺序绑定到 decline 选择器的固定。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Deferred to a follow-up: this is review round 8, and repository policy (AGENTS.md) lands only Critical fixes once a PR has been through roughly five review rounds, deferring remaining Suggestions with a recorded reason so nothing is silently dropped. Pinning the final DETAIL="${AGENT_DETAIL}" hop and its shielding else is a real mutation gap. It stays tracked for a follow-up issue/PR batched with the other deferred round-8 pins.
中文说明
延迟至后续处理：这是第 8 轮评审，仓库政策（AGENTS.md）规定 PR 经过约 5 轮评审后只合入 Critical 修复，其余 Suggestion 延迟处理并记录原因，确保不被静默丢弃。R8-42：固定最后一跳 DETAIL="${AGENT_DETAIL}" 及其屏蔽 else 确实是真实的变异测试缺口。该项将与其他延迟的第 8 轮固定项一起归入后续 issue/PR 批量处理。
| expect(issueAutofixPublishJob).toContain( | ||
| "steps.proof.outputs.preserve_claim != 'true'", | ||
| ); | ||
| expect(withdrawClaimStep).toContain('always()'); |
There was a problem hiding this comment.
[Suggestion] R8-43: The withdraw step's if: guard is pinned only as disconnected substrings; its && connectives are unpinned, so a single &&→|| flip keeps every pin green (no full-expression pin exists anywhere — verified by grep). Mutation-probed: 120/120 green. — Failure scenario: publish calls preserve_claim unconditionally before the push and never clears it. Flip the last connective to ||: on a clean successful publication withdraw fires, the ref is already released → the absent-ref branch sees the still-present in-progress label → exit 1 'preserving the claim' → the publish job goes red and 'Autofix publication failed' posts for a run that published. The symmetric flip releases the claim ref proof asked to preserve.
Suggested fix: Pin the whole if: as one whitespace-normalized string, like the routing-gate pin in this same diff.
中文说明
withdraw 步骤的 if: 守卫只以彼此断开的子串固定；其 && 连接词未被固定，因此单个 &&→|| 翻转就能让所有固定保持绿色（任何地方都不存在完整表达式固定——已 grep 验证）。变异探针确认：120/120 全绿。— 失败场景：publish 在 push 前无条件调用 preserve_claim 且从不清除。把最后一个连接词翻转为 ||：一次干净的成功发布也会触发 withdraw，ref 已释放 → 缺 ref 分支看到仍在的 in-progress 标签 → 退出 1 'preserving the claim' → publish job 变红，并对一个已发布的运行打印 'Autofix publication failed'。对称的翻转会释放 proof 要求保留的 claim ref。修复：像本 diff 中的路由 gate 固定一样，把整个 if: 作为一个空白归一化字符串固定。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Deferred to a follow-up: this is review round 8, and repository policy (AGENTS.md) lands only Critical fixes once a PR has been through roughly five review rounds, deferring remaining Suggestions with a recorded reason so nothing is silently dropped. Pinning the withdraw if: guard as one whitespace-normalized expression is a real mutation gap. It stays tracked for a follow-up issue/PR batched with the other deferred round-8 pins.
中文说明
延迟至后续处理：这是第 8 轮评审，仓库政策（AGENTS.md）规定 PR 经过约 5 轮评审后只合入 Critical 修复，其余 Suggestion 延迟处理并记录原因，确保不被静默丢弃。R8-43：把 withdraw 的 if: 守卫作为一个空白归一化表达式固定确实是真实的变异测试缺口。该项将与其他延迟的第 8 轮固定项一起归入后续 issue/PR 批量处理。
| // the settings-schema invocation's prefix. | ||
| const issueVerifyGate = verificationGateBodies[0]; | ||
| expect(issueVerifyGate.replace(/\s+/g, ' ')).toContain( | ||
| '| AUTOFIX_VERIFY_COMMAND="${verify_cmd}" \\ bash "${RUNNER_TEMP}/check-autofix-contracts.sh"', |
There was a problem hiding this comment.
[Suggestion] R8-44: The new pin binds the sealed-job wrapper env (AUTOFIX_VERIFY_COMMAND) only to the contracts-script invocation; the sibling settings-schema invocation's identical prefix — what makes the gate skip under the seal — is pinned nowhere. Mutation-probed: dropping the prefix from the schema invocation keeps 120/120 green. — Failure scenario: at runtime the gate runs after finalize sealed the worktree root-owned read-only; without the env, check-settings-schema.sh enters the generate branch and unconditionally writeFileSyncs the committed schema → EACCES for the non-root runner user → every candidate fails deterministic verification and every claim withdraws. Distinct from R8-22 (env inside the command wrapper); this is the gate-level skip wiring.
Suggested fix: Add the sibling pin for the settings-schema invocation.
中文说明
新固定把密封 job 的 wrapper 环境变量（AUTOFIX_VERIFY_COMMAND）只绑定到 contracts 脚本调用；settings-schema 调用的相同前缀——让该 gate 在密封下跳过的那部分——没有任何固定。变异探针确认：从 schema 调用中删除该前缀后 120/120 仍全绿。— 失败场景：运行时该 gate 在 finalize 把 worktree 密封为 root 属主只读之后执行；没有该环境变量，check-settings-schema.sh 进入 generate 分支并无条件 writeFileSync 已提交的 schema → 非 root runner 用户 EACCES → 每个候选的确定性验证都失败、每个 claim 都撤回。与 R8-22（命令 wrapper 内部的环境变量）不同；这里是 gate 层的跳过接线。修复：为 settings-schema 调用新增同级的固定。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Deferred to a follow-up: this is review round 8, and repository policy (AGENTS.md) lands only Critical fixes once a PR has been through roughly five review rounds, deferring remaining Suggestions with a recorded reason so nothing is silently dropped. Pinning the settings-schema invocation's AUTOFIX_VERIFY_COMMAND prefix is a real load-bearing gap. It stays tracked for a follow-up issue/PR batched with the other deferred round-8 pins.
中文说明
延迟至后续处理：这是第 8 轮评审，仓库政策（AGENTS.md）规定 PR 经过约 5 轮评审后只合入 Critical 修复，其余 Suggestion 延迟处理并记录原因，确保不被静默丢弃。R8-44：固定 settings-schema 调用的 AUTOFIX_VERIFY_COMMAND 前缀确实是承重缺口。该项将与其他延迟的第 8 轮固定项一起归入后续 issue/PR 批量处理。
Address round-8 review's three Critical findings. - Run the contracts gate and changed-package Vitest runs before the finalize seal: Vite bundles each package's TypeScript vitest config into a `<config>.timestamp-*.mjs` temp file next to the config, and the coverage/JUnit reporters write into the tree, all of which fail EACCES once every directory is root-owned read-only for the verifier UID. Disable coverage and JUnit reporting for those runs so their ignored artifacts cannot fail the post-test output audit; the audit repeated after the seal still fails closed on anything left behind. - Add the missing `|| true` to the decline-path iconv guard so a multi-byte failure.md cut at byte 1500 cannot kill the step. - Protect `*.test-helper.*` files, file-form `test-utils.*` names, and `acceptance/` directories in the trusted-inputs denylist.
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment 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/31034811152 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: script-lint — shellcheck unavailable on this host; 7 changed shell scripts not linted (actionlint checks disclosed as deferred).
Not reviewed: the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/check-settings-schema.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/prepare-autofix-verification-worktree.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/resolve-owning-packages.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-verification-command.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-vitest.sh: shellcheck is not installed.
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Not reviewed: You are review agent verify — Verification agent. — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查：script-lint — shellcheck unavailable on this host; 7 changed shell scripts not linted (actionlint checks disclosed as deferred)。
未审查：the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/check-settings-schema.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/prepare-autofix-verification-worktree.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/resolve-owning-packages.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-verification-command.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-vitest.sh: shellcheck is not installed。
未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
未审查：You are review agent verify — Verification agent.——它被指向 diff 的行却从未打开：有工具调用，但没有一次读取 diff。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| "${WORKDIR}/scan.json" > "${WORKDIR}/candidates.json"; then | ||
| if ! jq -c --arg bot "${AUTOFIX_BOT}" ' | ||
| [ .[] | | ||
| select(((.assignees // []) | length > 0 and all(.login == $bot)) and |
There was a problem hiding this comment.
[Critical] The new "bot must already be the sole assignee" candidate gate orphans every pre-rollout approved issue the old flow kept unassigned. The scan select, the forced-issue elif, and the claim pre-check all require the bot assigned BEFORE selection, while the only code that assigns the bot runs AFTER selection (claim step) or at creation in the routing workflow. The rollout grandfather backfill only iterates issues that already survived the select, so it is unreachable for exactly that population, and the notice remediation ("re-apply autofix/approved") does not remediate: re-labeling only posts a marker — the issue stays unassigned and is still filtered out. — Failure scenario: at rollout, an issue already status/ready-for-agent + autofix/approved but unassigned (the base scan's no:assignee admitted exactly these) is dropped before the marker loop is consulted; the workflow_dispatch escape hatch also skips unassigned forced issues → silently stuck issues plus a remediation message that does not remediate. Probe-reproduced: the select/forced-gate jq rejects assignees=[] and admits assignees=[bot]; a live release-created, approved, unassigned issue exists in this repo today. Suggested fix: give the flow a path to adopt existing unassigned approved issues — e.g. when an issue carries both required labels and no non-bot assignee, have the forced-issue path (or a migration step) run gh issue edit --add-assignee "${AUTOFIX_BOT}" instead of skipping, or relax the select to admit unassigned issues with a matching approval marker during the cutover window.
中文说明
新的"bot 必须已是唯一 assignee"候选门禁会让所有上线前已被批准、但旧流程刻意保持未指派状态的 issue 成为孤儿。scan select、forced-issue elif、claim 前置检查都要求 bot 在选择之前已被指派，而唯一指派 bot 的代码都在选择之后（claim 步骤）或只在 routing workflow 创建 issue 时执行。上线补录（grandfather backfill）只遍历已通过 select 的 issue，对这批 issue 永远不可达；公告给出的补救（重新打 autofix/approved 标签）也无效：重新打标只会补记 marker，issue 仍然没有 assignee，依旧被过滤。— 失败场景：上线时一个已带 status/ready-for-agent + autofix/approved 但未指派的 issue（旧 scan 的 no:assignee 恰好接收这类 issue）在 marker 循环之前就被丢弃；workflow_dispatch 逃生路径同样跳过未指派的 forced issue → issue 被静默卡死，且补救说明本身不起作用。已用探针复现：select/forced-gate jq 拒绝 assignees=[]、接受 assignees=[bot]；本仓库当前就存在一个 release 创建、已批准、未指派的存活 issue。修复建议：为流程增加接收存量未指派已批准 issue 的路径——例如当 issue 带齐两个标签且无非 bot assignee 时，让 forced-issue 路径（或迁移步骤）执行 gh issue edit --add-assignee "${AUTOFIX_BOT}" 而不是跳过；或在切换窗口内放宽 select，允许带匹配 approval marker 的未指派 issue 进入。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| echo "⏭️ Forced issue #${FORCED_ISSUE} is not open; skipping." | ||
| jq -n -c '[]' > "${WORKDIR}/candidates.json" | ||
| elif jq -e --arg bot "${AUTOFIX_BOT}" ' | ||
| ((.assignees // []) | length == 0 or any(.login != $bot)) or |
There was a problem hiding this comment.
[Critical] The assignee contract flip breaks release.yml, an untouched consumer: it creates issues with type/bug, status/ready-for-agent, autofix/approved labels but NO assignee, then dispatches this workflow with phase=issue issue_number=<N>. The forced-issue elif's assignee check is unconditional (it is NOT gated on EVENT_NAME != 'workflow_dispatch' — only the two label gates below it are), so the dispatch writes an empty candidate list and exits green having done nothing. The scheduled-scan fallback release.yml explicitly relies on ("so that, if the dispatch below fails, the scheduled ready-for-agent scan can still find it") is also dead, because the select now filters unassigned issues out. Even the claim step's --add-assignee cannot heal this: selection gates run before claim. — Failure scenario: any release-workflow failure after this merge → release.yml creates an unassigned approved issue and dispatches; the forced path skips it, the run exits green doing nothing, the scan also filters it out → release failures are never auto-fixed; the dispatch is a silent no-op. Base verified: at merge-base the forced path had no assignee requirement and the scan's no:assignee admitted exactly these issues. Suggested fix: add --assignee "${AUTOFIX_BOT}" to release.yml's gh issue create (and the existing-issue gh issue edit path), and decide explicitly whether the scan fallback should also support CI-created issues.
中文说明
assignee 契约的反转破坏了未被本 PR 修改的消费方 release.yml：它创建的 issue 带 type/bug、status/ready-for-agent、autofix/approved 标签但没有 assignee，然后以 phase=issue issue_number=<N> dispatch 本 workflow。forced-issue elif 的 assignee 检查是无条件的（它并不受 EVENT_NAME != 'workflow_dispatch' 门禁约束——只有其下方的两个标签门禁受约束），因此 dispatch 会写入空候选列表并绿通过、什么都没做。release.yml 明确依赖的定时 scan 兜底（"这样即使下面的 dispatch 失败，定时 ready-for-agent scan 仍能找到它"）也已失效，因为 select 现在会过滤掉未指派的 issue。claim 步骤的 --add-assignee 也无法补救：选择门禁先于 claim 执行。— 失败场景：合并后任何 release 工作流失败 → release.yml 创建未指派的已批准 issue 并 dispatch；forced 路径跳过它，运行绿通过且无事发生，scan 也将其过滤 → release 失败永远不会被自动修复，dispatch 成为静默空操作。已核对 merge-base：旧 forced 路径没有 assignee 要求，旧 scan 的 no:assignee 恰好接收这些 issue。修复建议：在 release.yml 的 gh issue create（及存量 issue 的 gh issue edit 路径）加上 --assignee "${AUTOFIX_BOT}"，并明确决定 scan 兜底是否也要支持 CI 创建的 issue。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| '(.assignees // []) | map(.login) | index($bot) != null' \ | ||
| <<< "${live_issue_json}" > /dev/null && | ||
| ! gh issue edit "${ISSUE}" --repo "${REPO}" \ | ||
| --remove-assignee "${AUTOFIX_BOT}"; then |
There was a problem hiding this comment.
[Critical] R8-33: Still standing from round 8 (then Suggestion; escalated to Critical this round after tracing the full recovery chain). Withdraw strips the bot assignee that every pickup path this PR adds now requires, while the posted recovery instruction names only re-adding autofix/approved — so the published recovery procedure silently no-ops. This PR flips the assignee from an exclusion (no:assignee in the old search) to a hard routing precondition (scan select, forced elif, decision revalidation, claim precondition, publish proof, check_live_issue), and nothing ever assigns the bot except the watcher at issue creation and the claim step's post-selection --add-assignee. — Failure scenario: a maintainer follows the posted instruction and re-adds autofix/approved → the issues route fires, Record approved issue prose succeeds, then the scan's jq filter drops the assignee-less issue and the run goes green with "0 candidate(s) found" — no comment, no error. A recurrence of the same CI failure does not heal it (the watcher's reuse gate also requires the bot assignee and never restores it). The issue is stranded until a human happens to assign the bot — the one action no message mentions. Pre-PR, re-adding the approval label alone resumed autofix; this diff breaks that contract. Suggested fix: treat the bot assignee as routing state that predates the claim, not claim state to revert — drop the --remove-assignee block (mirroring Neutralize expired targeted E2E requirement, which consumes the approval but never touches the assignee). If the removal is kept deliberately, update the withdraw comment to require re-adding autofix/approved AND assigning the bot, and make the scan post a notice when it drops an otherwise-eligible issue for a missing bot assignee.
中文说明
R8-33：第 8 轮遗留未修（当时为 Suggestion；本轮追踪完整恢复链路后升级为 Critical）。Withdraw 会移除 bot assignee，而本 PR 新增的所有拾取路径都要求该 assignee 存在；但 withdraw 发布的恢复指引只提到重新添加 autofix/approved —— 因此公布的恢复流程实际上是静默空操作。本 PR 把 assignee 从排除条件（旧搜索中的 no:assignee）反转为硬性路由前置条件（scan select、forced elif、decision 复核、claim 前置、publish proof、check_live_issue），而除了创建时的 watcher 和选择之后的 claim 步骤 --add-assignee，没有任何代码会指派 bot。— 失败场景：维护者按指引重新添加 autofix/approved → issues 路由触发，Record approved issue prose 成功，随后 scan 的 jq 过滤丢弃这个没有 assignee 的 issue，运行以 "0 candidate(s) found" 绿通过——没有评论、没有报错。同一 CI 失败再次复发也无法自愈（watcher 的复用门同样要求 bot assignee 且从不恢复它）。issue 会一直卡死，直到某个人类碰巧指派了 bot——而这恰恰是没有任何提示提到的操作。本 PR 之前，仅重新添加批准标签即可恢复 autofix；此 diff 破坏了该契约。修复建议：把 bot assignee 视为先于 claim 存在的路由状态、而非需要回滚的 claim 状态——去掉 --remove-assignee 块（与 Neutralize expired targeted E2E requirement 一致，后者消费批准但从不改动 assignee）。若刻意保留移除逻辑，则更新 withdraw 评论，要求同时重新添加 autofix/approved 并 指派 bot，并让 scan 在因缺少 bot assignee 而丢弃本应合格的 issue 时发布公告。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| ((.assignees // []) | length > 0 and all(.login == $bot)) and | ||
| ((.closedByPullRequestsReferences // []) | length == 0) | ||
| ' <<< "${live_state}" > /dev/null; then | ||
| echo "Issue #${ISSUE} changed during publication; leaving its current routing unchanged." |
There was a problem hiding this comment.
[Critical] The routing transaction adds autofix/routing in the first step (file_issue: --add-label for existing issues, --label in the create args) and removes it only as the very last command of the second step (Route issue to Autofix); a repo-wide grep confirms this is the ONLY removal site. Every intermediate failure aborts with the label applied, and one fully green exit path leaks it too. — Failure scenario (red path): the Route step makes six API calls (E2E label create/add, issue view, digest comment, label remove) after the artifact-upload step; a rate limit or 5xx on any of them — routine for a busy bot PAT — aborts the job with the label leaked. (Green path): if the issue changes between the two steps (e.g. a human briefly self-assigns), the live re-check fails and the step exits 0 — "changed during publication" — without removing the label, and no job turns red. The leaked label is a dead end: AUTOFIX_ISSUE_EXCLUDES carries -label:autofix/routing, the forced-issue path skips it, the read-decision revalidation, claim gate, proof revalidation and publish's check_live_issue all require its absence. Self-heal requires a later ELIGIBLE recurrence of the same failure to re-run the transaction — for one-off/flaky failures none comes, and the issue is silently excluded from autofix forever; the design doc's "visible fail-closed lock" claim does not cover the exit-0 variant. Suggested fix: add a final if: always() step to file_issue that idempotently removes autofix/routing from steps.issue.outputs.number whenever that output is set, or add a staleness reaper for the label in the scheduled autofix scan.
中文说明
路由事务在第一步添加 autofix/routing（file_issue：存量 issue 用 --add-label，创建路径用 create args 中的 --label），只在第二步（Route issue to Autofix）的最后一条命令才移除；全仓库 grep 确认这是唯一的移除点。任何中间失败都会带着已添加的标签中止，而且有一条完全绿色的退出路径也会泄漏标签。— 失败场景（红色路径）：Route 步骤在 artifact 上传之后要执行六个 API 调用（E2E 标签创建/添加、issue 查看、摘要评论、标签移除）；对繁忙的 bot PAT 来说常见的限流或 5xx 发生在任何一个调用上，都会使 job 带着泄漏的标签中止。（绿色路径）：如果 issue 在两步之间发生变化（例如人类短暂自我指派），live 复核失败，步骤以 0 退出——"changed during publication"——不移除标签，且没有任何 job 变红。泄漏的标签是死路：AUTOFIX_ISSUE_EXCLUDES 带 -label:autofix/routing，forced-issue 路径跳过它，read-decision 复核、claim 门禁、proof 复核、publish 的 check_live_issue 都要求它不存在。自愈需要同一失败后来再次发生合格复发以重跑事务——对一次性/偶发失败来说复发不会来，issue 会被静默地永远排除在 autofix 之外；设计文档"可见的失败闭环锁"的说法不覆盖 exit-0 变体。修复建议：给 file_issue 增加一个 if: always() 的最终步骤，只要 steps.issue.outputs.number 有值就幂等移除 autofix/routing；或在定时 autofix scan 中增加该标签的过期清理。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if jq -e --arg bot "${AUTOFIX_BOT}" \ | ||
| 'any(.[].[]; .user.login == $bot and ((.body // "") | test("^<!-- autofix-approved-prose-sha256:[0-9a-f]{64} -->$")))' \ | ||
| <<< "${approval_comments}" > /dev/null; then |
There was a problem hiding this comment.
[Suggestion] The re-approval protection in Record approved issue prose only triggers when a bot marker ALREADY exists; when no marker exists, the step falls through and records a marker for whatever the current prose is on any trusted issues event, bypassing the freshness invariant the scan's grandfather check enforces. — Failure scenario: a pre-rollout auto-routed issue carries autofix/approved and the bot assignee but no marker (the marker concept is new in this PR), and its body changed after approval. The first trusted issues event after merge (e.g. a maintainer adds an unrelated label — do_issue requires both labels + trusted sender, both true) enters this step: the gate finds no bot marker so it does not fire, the live check passes, and a marker for the never-approved current body is posted. This step runs earlier in the same job as the scan, so the marker loop's exact-match short-circuit admits the issue before the grandfather's updatedAt-vs-label-event freshness check is ever consulted → autofix develops against prose no maintainer approved. A live marker-less approved issue exists in the repo today. Suggested fix: treat "no marker present" like the stale-marker case — only record when EVENT_ACTION == 'labeled' && ISSUE_LABEL == AUTOFIX_APPROVED_LABEL, or apply the same cutover/updatedAt freshness check before posting.
中文说明
Record approved issue prose 的重复批准保护只在 bot marker 已存在时触发；当不存在任何 marker 时，该步骤会直接穿透，在任何受信 issues 事件下为当前正文记录 marker，绕过了 scan 的 grandfather 检查所强制的新鲜度不变量。— 失败场景：一个上线前已被自动路由的 issue 带有 autofix/approved 和 bot assignee 但没有 marker（marker 概念是本 PR 新增），且其正文在批准后被修改过。合并后第一个受信 issues 事件（例如维护者添加一个无关标签——do_issue 只要求两个标签 + 受信发送者，两者都满足）进入此步骤：门禁找不到 bot marker 因此不触发，live 检查通过，于是为从未被批准的当前正文发布了 marker。此步骤在同一 job 中先于 scan 运行，marker 循环的精确匹配短路会在 grandfather 的 updatedAt-标签事件新鲜度检查被查询之前就放行该 issue → autofix 会基于没有任何维护者批准过的正文开展工作。仓库当前就存在一个无 marker 的已批准存活 issue。修复建议：把"无 marker"视同"marker 过期"处理——仅在 EVENT_ACTION == 'labeled' && ISSUE_LABEL == AUTOFIX_APPROVED_LABEL 时记录，或在发布前应用同样的 cutover/updatedAt 新鲜度检查。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| --jq '.object.sha' 2>"${claim_ref_error_file}")"; then | ||
| if grep -q 'HTTP 404' "${claim_ref_error_file}"; then | ||
| rm -f "${claim_ref_error_file}" | ||
| echo '::error::The claim ref no longer exists: an earlier attempt already withdrew it (or this run released it before taking ownership). Re-running this job cannot publish; re-apply the autofix/approved label to start a fresh cycle.' |
There was a problem hiding this comment.
[Suggestion] This 404 recovery annotation names only re-applying autofix/approved, but the bot assignee — now a routing precondition everywhere — has already been removed by the withdrawal that deleted the claim ref, so following the instruction is the same silent no-op R8-33 identified on the withdraw comment, at a second site. — Failure scenario: attempt N fails after claim; its withdraw removes autofix/in-progress, removes the bot assignee, deletes the claim ref. A re-run reaches revalidate-proof, gets HTTP 404 on the ref, and shows this annotation. The maintainer re-applies autofix/approved; the record step happily records a fresh marker — but the issue has no bot assignee, so the forced-path elif and the scan select filter it forever. The remediation the annotation prescribes cannot restore routing. Fixing R8-33's withdraw comment does not fix this annotation. Suggested fix: extend the instruction to also re-assign the bot (or have withdraw/recovery restore the assignee), matching whatever fix R8-33 receives.
中文说明
这个 404 恢复提示只提到重新添加 autofix/approved，但 bot assignee——如今在所有地方都是路由前置条件——已经被删除 claim ref 的那次 withdraw 移除了，因此照此指引操作会与 R8-33 在 withdraw 评论中指出的问题一样成为静默空操作，这是第二个发生点。— 失败场景：尝试 N 在 claim 之后失败；其 withdraw 移除 autofix/in-progress、移除 bot assignee、删除 claim ref。重跑到达 revalidate-proof，对该 ref 得到 HTTP 404，显示此提示。维护者重新添加 autofix/approved；record 步骤会正常记录新 marker——但 issue 没有 bot assignee，因此 forced-path elif 和 scan select 会永远过滤它。该提示开出的补救方案无法恢复路由。修复 R8-33 的 withdraw 评论并不能修复这个提示。修复建议：把指引扩展为同时重新指派 bot（或让 withdraw/恢复流程恢复 assignee），与 R8-33 获得的修复保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if ! approval_comments="$(gh api --paginate \ | ||
| "repos/${REPO}/issues/${ISSUE}/comments?per_page=100" --slurp)" || | ||
| ! jq -e --arg bot "${AUTOFIX_BOT}" --arg marker "${approval_marker}" \ |
There was a problem hiding this comment.
[Suggestion] The claim step conflates a transient comments-API failure with a genuine approval mismatch, reporting "prose no longer matches a bot-recorded approval" for a transport error — unlike its two siblings, which distinguish the cases. — Failure scenario: a rate limit, 5xx, or network blip on the comments fetch during claim makes the if true; the step prints the prose-tampering error and exits 1, sending maintainers to investigate tampering that never happened. The EXIT trap releases the claim ref; withdraw then finds the ref absent while the bot assignee remains, and exits red with "preserving the claim for recovery" — a misleading red, since nothing is preserved. The issue self-heals on the next scan (no approval was consumed yet), but the diagnosis is actively misleading. The sibling record step separates fetch failure ("refusing to record an approval") from stale-marker rejection, and revalidate-proof preserves the claim on transport errors — the claim step wraps neither. Suggested fix: split the two conditions — on fetch failure, emit a distinct transport-error message and exit before evaluating the marker; then run the jq -e marker match as a separate check with the existing message.
中文说明
claim 步骤把瞬时的 comments API 失败与真正的批准不匹配混为一谈，对传输错误报告"正文不再匹配 bot 记录的批准"——而它的两个兄弟步骤都区分了这两种情况。— 失败场景：claim 期间 comments 拉取遇到限流、5xx 或网络抖动会使 if 为真；步骤打印正文被篡改的错误并退出 1，让维护者去调查从未发生的篡改。EXIT trap 释放 claim ref；随后 withdraw 发现 ref 不存在但 bot assignee 仍在，以"为恢复保留 claim"红退出——这是一个误导性的红，因为什么都没有保留。issue 会在下次 scan 时自愈（批准尚未被消费），但诊断信息具有主动误导性。兄弟 record 步骤把拉取失败（"refusing to record an approval"）与 marker 过期拒绝分开，revalidate-proof 在传输错误时保留 claim——claim 步骤两者都没做。修复建议：拆分两个条件——拉取失败时输出明确的传输错误消息并在评估 marker 之前退出；然后把 jq -e marker 匹配作为独立检查使用现有消息。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(claimCommentStep).toContain( | ||
| "${{ steps.claim.outputs.claimed == 'true' }}", | ||
| ); | ||
| expect(claimCommentStep).toContain('gh issue comment "${ISSUE}"'); |
There was a problem hiding this comment.
[Suggestion] The claim-comment marker contract is pinned only on the reader side — withdraw's claim_comment_marker prefix is pinned — but the writer side, the CLAIM_COMMENT_MARKER format posted by Post claim comment, is pinned nowhere, so the two sides can silently disagree. — Failure scenario (probe-verified): changing the posted marker to <!-- autofix-claimed issue=... --> keeps 120/120 tests green; adding a writer-side pin catches the mutation. Any edit to the posted marker's format makes withdraw's contains($marker) comment search return zero matches; claim_comment_id is then empty and the [[ -n ]] guard skips the DELETE, so withdraw completes — label removed, assignee removed, ref released, withdrawal comment posted — while the stale "The scheduled autofix agent is picking this issue up" claim comment stays on the issue forever, directly contradicting the withdrawal comment beneath it. Suggested fix: pin the posted marker alongside the reader-side pin, e.g. expect(claimCommentStep).toContain('CLAIM_COMMENT_MARKER="<!-- autofix-claim issue=${ISSUE} run=${GITHUB_RUN_ID} attempt='), enforcing the writer↔reader pairing like the existing agent-api-error contract test.
中文说明
claim 评论 marker 契约只在读取侧被钉住——withdraw 的 claim_comment_marker 前缀有钉住——但写入侧，即 Post claim comment 发布的 CLAIM_COMMENT_MARKER 格式，没有任何地方钉住，因此两侧可能静默不一致。— 失败场景（已探针验证）：把发布的 marker 改为 <!-- autofix-claimed issue=... --> 后 120/120 测试仍为绿；补上写入侧钉住即可捕获该变异。任何对发布 marker 格式的编辑都会使 withdraw 的 contains($marker) 评论搜索返回零匹配；claim_comment_id 为空时 [[ -n ]] 守卫跳过 DELETE，于是 withdraw 完成——标签移除、assignee 移除、ref 释放、withdrawal 评论发布——而过期的"The scheduled autofix agent is picking this issue up" claim 评论永远留在 issue 上，与下方的 withdrawal 评论直接矛盾。修复建议：在读取侧钉住项旁同时钉住发布的 marker，例如 expect(claimCommentStep).toContain('CLAIM_COMMENT_MARKER="<!-- autofix-claim issue=${ISSUE} run=${GITHUB_RUN_ID} attempt=')，像现有 agent-api-error 契约测试一样强制写入↔读取配对。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| // The footer and both verification reports are assembled into the final | ||
| // PR body before creation, not injected into the model-authored files. | ||
| const publicationBodyIndex = publishPrStep.indexOf( | ||
| 'publication_body="${workdir}/publication-pr-body.md"', |
There was a problem hiding this comment.
[Suggestion] The new publication-body ordering chain pins the section headers, the e2e-report append, the footer and gh pr create, but never positions the two other content appends (append_agent_content "${workdir}/pr-body.md" and cat "${workdir}/targeted-e2e-report.md"), which are only presence-pinned elsewhere. — Failure scenario (probe-verified for both lines): transposing each after the gh pr create if/fi block keeps 120/120 green; adding a position pin catches each mutation. Every published autofix PR would then ship with an empty "Agent-authored change summary" section — or, worse, an empty "Trusted targeted E2E proof" section that advertises evidence the body does not contain. Suggested fix: add the two appends to the chain and assert trustedProofContentIndex falls between trustedProofIndex and agentSummaryIndex, and prBodyIndex between agentSummaryIndex and agentReportIndex.
中文说明
新的发布正文顺序链钉住了各节标题、e2e-report 追加、页脚和 gh pr create，但从未定位另外两个内容追加（append_agent_content "${workdir}/pr-body.md" 和 cat "${workdir}/targeted-e2e-report.md"），它们只在别处被存在性钉住。— 失败场景（两行均已探针验证）：把任一追加移到 gh pr create 的 if/fi 块之后，120/120 仍为绿；补上位置钉住即可捕获各自的变异。这样每个发布的 autofix PR 都会带着空的"Agent-authored change summary"节——或者更糟，带着一个空的"Trusted targeted E2E proof"节，宣称存在正文中并不包含的证据。修复建议：把两个追加加入顺序链，并断言 trustedProofContentIndex 位于 trustedProofIndex 与 agentSummaryIndex 之间、prBodyIndex 位于 agentSummaryIndex 与 agentReportIndex 之间。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(workflow).toContain( | ||
| 'cp .github/scripts/load-autofix-e2e-metadata.mjs "${RUNNER_TEMP}/load-autofix-e2e-metadata.mjs"', |
There was a problem hiding this comment.
[Suggestion] The metadata-loader staging cp is pinned by workflow-wide toContain while three near-identical cp sites exist (issue-autofix, issue-autofix-targeted-e2e's Stage trusted targeted verifier, publish); the pin loses locality, so the cp inside any one consuming job is unpinned — unlike its sibling run-autofix-targeted-e2e.mjs, whose single cp site makes the same assertion effectively job-bound. — Failure scenario (probe-verified): removing the cp from the targeted-e2e job's staging step keeps 120/120 green; a job-scoped pin catches it. Each job gets a fresh runner and RUNNER_TEMP, so Load current targeted requirement then runs node "${RUNNER_TEMP}/load-autofix-e2e-metadata.mjs" on a file that was never staged — ERR_MODULE_NOT_FOUND fails issue-autofix-targeted-e2e after deterministic verification already succeeded, withdrawing a healthy claim and consuming the approval cycle per attempt, for every issue carrying autofix/e2e-verification-required. Deleting the producer-job cp has the same shape. Suggested fix: scope the assertion to each consuming job (or assert the cp's total occurrence count is 3).
中文说明
metadata loader 的暂存 cp 以全 workflow 范围的 toContain 钉住，而实际存在三个几乎相同的 cp 点（issue-autofix、issue-autofix-targeted-e2e 的 Stage trusted targeted verifier、publish）；该钉住失去了位置性，因此任何一个消费 job 内部的 cp 都没有被钉住——不像其兄弟 run-autofix-targeted-e2e.mjs，后者唯一的 cp 点使同样的断言实际上绑定到具体 job。— 失败场景（已探针验证）：从 targeted-e2e job 的暂存步骤删除该 cp 后 120/120 仍为绿；job 范围的钉住可以捕获。每个 job 都使用全新的 runner 和 RUNNER_TEMP，因此 Load current targeted requirement 会对一个从未被暂存的文件运行 node "${RUNNER_TEMP}/load-autofix-e2e-metadata.mjs——ERR_MODULE_NOT_FOUND 使 issue-autofix-targeted-e2e 在确定性验证已经成功之后失败，撤回健康的 claim 并逐次消费批准周期，影响每个带 autofix/e2e-verification-required 的 issue。删除 producer job 的 cp 同理。修复建议：把断言限定到每个消费 job（或断言该 cp 的总出现次数为 3）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: script-lint — shellcheck unavailable on this host; 7 changed shell scripts not linted (actionlint checks disclosed as deferred).
Not reviewed: reverse audit — stopped after round 1 without two consecutive dry rounds: round 1 covered all 28 chunks and surfaced 23 new findings (all verified in Step 4); round 2 not run — the diff's 5,000-line test suite invites an unbounded tail of test-pinning findings.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI; the targeted-E2E execution path this PR adds is exercised only by production runs, not by any test leg of this PR (changed files covered locally: HELPER_TESTS node --test 83/83, test:scripts 962 passed, full build clean).
Not reviewed: the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/check-settings-schema.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/prepare-autofix-verification-worktree.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/resolve-owning-packages.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-verification-command.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-vitest.sh: shellcheck is not installed.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] R8-33: Withdraw claim on failure removes the bot assignee, but every re-entry path this PR adds requires assignees == [bot] (scan select ~951, decision live check ~1184, claim precondition ~1341, forced dispatch ~903), and nothing ever re-assigns the bot after withdrawal; the posted recovery message names only re-adding autofix/approved. Withdrawn issues enter a permanent silent dead state. Still standing from round 8/9; re-verified at the reviewed commit. (Inline copy not re-posted: duplicate of the round-9 comment at qwen-autofix.yml:2484) · 中文：withdraw 移除 bot assignee，但本 PR 新增的所有重入路径都要求 assignees == [bot]，且撤回后没有任何路径重新指派 bot；恢复指引只提到重新加 autofix/approved 标签。被撤回的 issue 进入永久静默死状态。R8/9 遗留，本轮在受审提交上重新验证。
[Critical] R9-1: The new bot-assignee candidate gate silently orphans every approved issue that is not already bot-assigned: issues that pass the label search but fail the assignee/linked-PR filter are dropped with no log line, and nothing assigns the bot on labeling (the only --add-assignee is inside Claim issue, which runs after selection). Human-filed and pre-rollout approved issues are invisibly and permanently ineligible. Still standing from round 9. (Inline copy not re-posted: duplicate of the round-9 comment at qwen-autofix.yml:951) · 中文：新的 bot-assignee 候选门禁静默孤立所有非 bot 分配的已批准 issue：通过标签搜索但未通过 assignee/linked-PR 过滤的 issue 被无日志丢弃，且打标签时没有任何路径指派 bot。人工创建与上线前已批准的 issue 不可见地永久失效。R9 遗留。
[Critical] R9-2: The assignee contract flip breaks release.yml, an untouched consumer: it creates release-failure issues with type/bug + status/ready-for-agent + autofix/approved labels but NO assignee (release.yml:68-73), then dispatches this workflow with phase=issue; the forced-issue elif's assignee check is unconditional, so every release-failure dispatch is skipped ('has no bot assignee … skipping'), and the scheduled scan also requires the bot assignee — release autofix is silently disabled. Still standing from round 9; verified directly against release.yml at the reviewed commit. (Inline copy not re-posted: duplicate of the round-9 comment at qwen-autofix.yml:903) · 中文：assignee 契约翻转破坏了未被本 PR 触及的 release.yml：它创建带 ready+approved 标签但无 assignee 的发布失败 issue 并 dispatch 本 workflow；强制路径的 assignee 检查无条件生效，每次发布失败的 dispatch 都被跳过，定时扫描同样要求 bot assignee——release autofix 被静默禁用。R9 遗留，已在受审提交上直接对照 release.yml 验证。
中文说明
未审查：script-lint — shellcheck unavailable on this host; 7 changed shell scripts not linted (actionlint checks disclosed as deferred)。
未审查：reverse audit — stopped after round 1 without two consecutive dry rounds: round 1 covered all 28 chunks and surfaced 23 new findings (all verified in Step 4); round 2 not run — the diff's 5,000-line test suite invites an unbounded tail of test-pinning findings。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI; the targeted-E2E execution path this PR adds is exercised only by production runs, not by any test leg of this PR (changed files covered locally: HELPER_TESTS node --test 83/83, test:scripts 962 passed, full build clean)。
未审查：the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/check-settings-schema.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/prepare-autofix-verification-worktree.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/resolve-owning-packages.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-verification-command.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-vitest.sh: shellcheck is not installed。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R8-33: Withdraw claim on failure removes the bot assignee, but every re-entry path this PR adds requires assignees == [bot] (scan select ~951, decision live check ~1184, claim precondition ~1341, forced dispatch ~903), and nothing ever re-assigns the bot after withdrawal; the posted recovery message names only re-adding autofix/approved. Withdrawn issues enter a permanent silent dead state. Still standing from round 8/9; re-verified at the reviewed commit. (Inline copy not re-posted: duplicate of the round-9 comment at qwen-autofix.yml:2484) · 中文：withdraw 移除 bot assignee，但本 PR 新增的所有重入路径都要求 assignees == [bot]，且撤回后没有任何路径重新指派 bot；恢复指引只提到重新加 autofix/approved 标签。被撤回的 issue 进入永久静默死状态。R8/9 遗留，本轮在受审提交上重新验证。
[Critical] R9-1: The new bot-assignee candidate gate silently orphans every approved issue that is not already bot-assigned: issues that pass the label search but fail the assignee/linked-PR filter are dropped with no log line, and nothing assigns the bot on labeling (the only --add-assignee is inside Claim issue, which runs after selection). Human-filed and pre-rollout approved issues are invisibly and permanently ineligible. Still standing from round 9. (Inline copy not re-posted: duplicate of the round-9 comment at qwen-autofix.yml:951) · 中文：新的 bot-assignee 候选门禁静默孤立所有非 bot 分配的已批准 issue：通过标签搜索但未通过 assignee/linked-PR 过滤的 issue 被无日志丢弃，且打标签时没有任何路径指派 bot。人工创建与上线前已批准的 issue 不可见地永久失效。R9 遗留。
[Critical] R9-2: The assignee contract flip breaks release.yml, an untouched consumer: it creates release-failure issues with type/bug + status/ready-for-agent + autofix/approved labels but NO assignee (release.yml:68-73), then dispatches this workflow with phase=issue; the forced-issue elif's assignee check is unconditional, so every release-failure dispatch is skipped ('has no bot assignee … skipping'), and the scheduled scan also requires the bot assignee — release autofix is silently disabled. Still standing from round 9; verified directly against release.yml at the reviewed commit. (Inline copy not re-posted: duplicate of the round-9 comment at qwen-autofix.yml:903) · 中文：assignee 契约翻转破坏了未被本 PR 触及的 release.yml：它创建带 ready+approved 标签但无 assignee 的发布失败 issue 并 dispatch 本 workflow；强制路径的 assignee 检查无条件生效，每次发布失败的 dispatch 都被跳过，定时扫描同样要求 bot assignee——release autofix 被静默禁用。R9 遗留，已在受审提交上直接对照 release.yml 验证。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| sudo chown -R root:root "${workspace}" | ||
| sudo find "${workspace}" -xdev -type d -exec chmod 0555 -- {} + | ||
| sudo find "${workspace}" -xdev -type f -exec chmod a-w -- {} + | ||
| sudo install -d -o root -g root -m 0711 "${workspace}/.integration-tests" |
There was a problem hiding this comment.
[Critical] R10-1: finalize creates .integration-tests with install -d, which follows a pre-planted symlink. In issue-autofix-verify the candidate's own tests run via the credential-free wrapper BEFORE finalize while every non-.git directory is mode 1777 (world-creatable), .integration-tests is gitignored and untracked, so the verify user can plant it as a symlink to any directory; the root install -d -o root -g root -m 0711 then applies ownership/mode to the target. cleanup's rm -rf removes the symlink before the post-seal output audit, erasing the trace. Probe-confirmed both directions (install -d follows; rm -rf erases before audit). — Failure scenario: Malicious candidate test code creates ${workspace}/.integration-tests as a symlink to any directory (e.g. /home/runner); finalize follows it as root, chown/chmod-ing the target. Targeting a directory later steps need makes verify fail on confusing infra errors instead of a gate verdict; targeting an unused directory leaves no observable trace. Blast radius is bounded by the ephemeral VM and the job's lack of credentials, but this is a root symlink-follow primitive in a chain whose design doc explicitly claims to prevent candidate-planted symlink substitution of privileged paths.
中文说明
finalize 用 install -d 创建 .integration-tests，而 install -d 会跟随预先植入的符号链接（探针确认：对指向目录的符号链接执行 install -d，属主/权限应用到目标）。在 issue-autofix-verify 中，候选自己的测试在 finalize 之前经无凭据包装器运行，且所有非 .git 目录都是 1777（全局可创建）、.integration-tests 被 gitignore 且未被跟踪——verify 用户可将其植入为指向任意目录的符号链接，root 的 install -d 会把属主/权限应用到目标。cleanup 的 rm -rf 只删符号链接（不跟随），在封禁后审计之前抹掉痕迹。爆炸半径受临时 VM 与 job 无凭据限制，但这是密封链设计文档明确声明要防止的 root 符号链接跟随原语。修复：在 install -d 之前先 sudo rm -rf -- "${workspace}/.integration-tests"。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| --repo "${REPO}" \ | ||
| --body "Main CI failure recurred after this issue was closed (source run: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }}). Not recreating an automatically approved issue; reopen this issue or file a new one if this is a regression." | ||
| fi | ||
| echo "Issue #${EXISTING_ISSUE} has a live claim, cancellation, ownership change, or linked PR; leaving it and its trusted metadata unchanged." |
There was a problem hiding this comment.
[Critical] R10-2: An eligible recurrence whose marker search lands on an open bot-created issue WITHOUT approval labels (created via the ineligible path) is silently dropped: the eligible branch's live check requires ready-for-agent + autofix/approved, so route_allowed=false; state is OPEN so no recurrence comment is posted and the body edit below never runs; exit 0 with no artifact upload. Every later recurrence resolves the same issue the same way. The deleted apply_autofix_route used to re-apply labels on existing issues; its replacement has no comment/re-file fallback for the OPEN-never-approved case. — Failure scenario: Run 1 fails on tests [A, B] with B outside the trusted harness → ineligible create files issue #100 with only type/bug but body markers for A and B. Run 2 fails on [A] only → eligible; the analyze search (--state all, marker A in:body) resolves #100; route_allowed=false; the step exits 0. The eligible failure is never routed, no artifact is uploaded, and #100 carries no trace — a red-main failure stays un-autofixed and invisible until human intervention.
中文说明
一次 eligible 复发若标记搜索命中一个“开放但无批准标签”的 bot issue（由 ineligible 路径创建），会被永久静默丢弃：eligible 分支的 live 检查要求 ready+approved → route_allowed=false；状态为 OPEN 故不发复发评论，其后的正文编辑也不会执行，exit 0 且不上传 artifact。每次后续复发都以同样方式命中同一 issue。被删除的 apply_autofix_route 过去会对已有 issue 重新应用标签；新代码对“OPEN 且从未批准”的情形没有任何评论/重立案回退。结果：main 持续失败的 eligible 用例永远不会被路由，也不可见，直到人工介入。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| gh issue edit "${EXISTING_ISSUE}" \ | ||
| --repo "${REPO}" \ | ||
| --add-label "${AUTOFIX_ROUTING_LABEL}" |
There was a problem hiding this comment.
[Critical] R9-3: The routing transaction adds autofix/routing in file_issue (--add-label for existing issues, --label in the create args) and removes it only as the very last command of 'Route issue to Autofix' (line 437) — the ONLY removal site. Every intermediate failure aborts with the label applied, and the live-state-recheck 'leaving its current routing unchanged' exit-0 path also leaks it. Still standing from round 9. — Failure scenario: Any gh failure between add and remove (metadata upload, digest comment, e2e-required label), or the fully-green 'issue changed during publication' exit, leaves autofix/routing on the issue. The scan excludes routing-labeled issues, so the issue disappears from scheduled selection until a marker-matching recurrence happens to re-enter routing; if the failure signature never recurs, it stays excluded forever.
中文说明
路由事务在 file_issue 中添加 autofix/routing 标签（既有 issue 用 --add-label，新建用 --label），但只在 "Route issue to Autofix" 步骤的最后一条命令移除（全仓库唯一移除点）。任何中间失败（metadata 上传、digest 评论、e2e-required 标签）都会带着已应用的标签中止；live-state 复检的 "leaving its current routing unchanged" exit-0 绿色路径同样泄漏标签。带路由标签的 issue 会被定时扫描排除；若该失败签名不再复发，issue 将永久无法被选择。R9 遗留，仍未修复。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| issue_json="$(gh issue view "${ISSUE}" --repo "${REPO}" \ | ||
| --json state,title,body,labels)" |
There was a problem hiding this comment.
[Critical] R9-4: Escalated from Suggestion (round 9) after tracing the full chain: 'Record approved issue prose' fires on every trusted issues event with do_issue == 'true'; its re-recording refusal is conditioned on an EXISTING bot marker. With no marker (every pre-rollout approved issue at rollout, plus any issue whose original recording run failed transiently), it falls through and posts a fresh marker for the current prose, bypassing the cutover/updatedAt/60s protections the grandfather backfill enforces for the identical marker-less class. — Failure scenario: Issue approved pre-rollout; the author edits the body days later. After this PR deploys, a maintainer assigns the bot (an assigned event with assignee == AUTOFIX_BOT is a route trigger from a trusted sender) or adds any non-trigger label → do_issue=true → the record step posts a marker for the edited prose. The scan's exact-marker match then short-circuits before the backfill guards; forced path, decision live check, and claim marker check all pass; the autonomous agent develops a fix for prose no maintainer ever approved.
中文说明
由 R9 的 Suggestion 升级为 Critical。"Record approved issue prose" 在任何 do_issue=true 的可信 issues 事件上都会触发；其拒绝重录的逻辑以“已存在 bot 标记”为前提。对没有任何标记的 issue（上线前已批准的所有 issue，以及首次记录运行瞬时失败的 issue），该步骤会直接为当前 prose 发布新标记，绕过 grandfather backfill 对同一类别强制执行的 cutover/updatedAt/60 秒保护。失败场景：上线前批准的 issue 在批准后被作者编辑；部署本 PR 后，维护者指派 bot（assignee == AUTOFIX_BOT 的 assigned 事件是可信触发）或添加任何非触发标签 → 为已编辑的 prose 发布标记 → 扫描的精确标记匹配在 backfill 守卫之前短路 → 强制路径、decision 复检、claim 标记检查全部通过 → agent 在从未被维护者批准的文本上开发修复。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| const failedTests = extractFailingTests(job.log); | ||
| if (!failedTests.length) { | ||
| reasons.push(`no exact Vitest failure found in job: ${job.name}`); |
There was a problem hiding this comment.
[Suggestion] R10-10: Three fail-closed reason branches in buildTargetedE2eAnalysis have no test exercising them: 'unsupported failed job:' (~143), 'no exact Vitest failure found in job:' (~153), 'unsupported E2E test identifier:' (~159). Mutation probe: deleting all three reasons.push lines leaves the suite 41/41 green. Behavioral probe flips: with the recordings gone, a mixed failure set (one install-break leg + one trusted allowlisted failure) becomes eligible/complete. — Failure scenario: A future refactor dropping one of these recordings makes a mixed failure set eligible: one matrix leg fails at npm ci before any test output while another produces one trusted allowlisted failure; reasons stays empty, eligible/complete flip to true, the issue is auto-approved, and the isolated verifier reruns only the one parsed case before publication even though a second failed leg was never attributed to an exact case — violating the design doc's fail-closed contract.
中文说明
buildTargetedE2eAnalysis 中三个 fail-closed reason 分支（"unsupported failed job:"、"no exact Vitest failure found in job:"、"unsupported E2E test identifier:"）没有任何测试覆盖。突变探针：删除三处 reasons.push 后 41/41 仍全绿；行为探针双向翻转：记录消失后，混合失败集（一个 npm 安装失败腿 + 一个可信 allowlist 失败）变为 eligible/complete。未来重构删除这些记录会让混合失败集被自动批准，隔离验证器发布前只重跑一个用例——违反设计文档“失败必须被完整归因”的 fail-closed 契约。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(withdrawClaimStep).toContain( | ||
| "LABEL_ARGS=(--remove-label 'autofix/in-progress')", | ||
| 'claim_comment_marker="<!-- autofix-claim issue=${ISSUE} run=${GITHUB_RUN_ID}"', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R10-26: The claim-comment marker contract is pinned only on the reader (withdraw) side; the writer side in Post claim comment (CLAIM_COMMENT_MARKER="") has no pin tying it to the prefix withdraw matches with contains($marker). Probe-confirmed: dropping run=${GITHUB_RUN_ID} from the writer marker keeps 120/120 green. Same mechanism as round-9's R9-18, re-verified this round. — Failure scenario: A writer-side regression (renaming autofix-claim, dropping run=, loosening to a bare comment) keeps every pin green: the stale claim comment is never deleted and nothing adopts it again; an over-broad marker matches multiple historical comments, tripping 'Found multiple matching claim comments' → exit 1 → the preserved-ref wedge of R10-25.
中文说明
claim 评论标记契约只在读取方（withdraw）被 pin；写入方（Post claim comment 的 CLAIM_COMMENT_MARKER=""）没有任何 pin 绑定到 withdraw 用 contains($marker) 匹配的前缀。探针：从写入方标记去掉 run=${GITHUB_RUN_ID} 后 120/120 仍绿。写入方回归会使陈旧 claim 评论永不被删除且无人认领；过宽标记会匹配多条评论，触发 "Found multiple matching claim comments" → exit 1 → R10-25 的保留 ref 楔死。与 R9-18 同一机制，本轮重新验证。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| elif jq -e \ | ||
| --arg ready "${READY_FOR_AGENT_LABEL}" \ | ||
| --arg approved "${AUTOFIX_APPROVED_LABEL}" ' | ||
| (.labels // []) | map(.name) | | ||
| index($ready) != null and index($approved) != null | ||
| ' <<< "${existing_labels_json}" > /dev/null; then | ||
| preserve_body='true' |
There was a problem hiding this comment.
[Critical] R10-3: The non-eligible recurrence branch's preserve_body guard keys on index($ready) and index($approved) only — the exact labels Claim issue consumes — and has NO check for autofix/in-progress or claim state, while the eligible branch explicitly checks claim/routing state. During an active claim, an ineligible recurrence sharing one marker overwrites the claimed issue's body. The design doc's 'the writer re-reads live ownership and cancellation state before routing' is not honored by this branch. — Failure scenario: Eligible issue claimed (in-progress added, approved removed, approved_prose_sha256 recorded). During the agent run (up to 180 minutes), main fails again with an ineligible set sharing one marker. file_issue finds the issue; preserve_body=false (approved gone); gh issue edit --body-file rewrites the body. Publication's check_live_issue digest comparison fails; the verified branch is deleted; the claim is withdrawn; approval was already consumed; the agent run is wasted and the issue needs manual re-approval.
中文说明
非 eligible 复发分支的 preserve_body 守卫只检查 ready+approved 标签——恰是 Claim issue 会消费掉的标签——没有检查 autofix/in-progress 或 claim 状态；而 eligible 分支明确检查 claim/路由状态。失败场景：eligible issue 被 claim（in-progress 添加、approved 移除、记录 approved_prose_sha256）；agent 运行期间（可达 180 分钟）main 再次失败且共享一个标记但整体 ineligible → file_issue 找到该 issue，preserve_body=false（approved 已被消费）→ gh issue edit --body-file 重写正文 → 发布时 check_live_issue 摘要比对失败 → 已验证分支被删、claim 撤回、批准已被消费、agent 运行被浪费，issue 需人工重新批准。设计文档声称“写入方在路由前重读 live 所有权与取消状态”，该分支并未遵守。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| approval_marker="<!-- autofix-approved-prose-sha256:${approval_digest} -->" | ||
| if ! approval_comments="$(gh api --paginate \ | ||
| "repos/${REPO}/issues/${ISSUE}/comments?per_page=100" --slurp)" || |
There was a problem hiding this comment.
[Suggestion] R10-24: The claim step's approval-marker comment fetch conjoins ! approval_comments="$(gh api --paginate ...)" || ! jq -e ... into one '::error::Issue #N prose no longer matches a bot-recorded approval.' — a transient 502/rate-limit produces the same message as a genuine marker mismatch. The three sibling fetches added in this PR (record step, scan loop, withdraw recovery) each distinguish transport from semantic failure; the claim step alone breaks the pattern at the highest-stakes read site. Probe-confirmed on the extracted step. Abort is clean (trap releases the ref); diagnosability, not state corruption. Same mechanism as round-9's R9-17, re-verified this round. — Failure scenario: Transient comments-API failure after the claim ref push aborts with a false prose/marker diagnosis; a maintainer triaging the red run audits prose edit history and marker comments, finds nothing, and has no trace of the real cause.
中文说明
claim 步骤的批准标记评论获取把传输失败与语义拒绝合并为一个条件——瞬时 502/限流/超时与真正的标记失配输出相同的 "::error::Issue #N prose no longer matches a bot-recorded approval."。本 PR 新增/重写的三处姊妹获取都区分两类失败：record 步骤（"Failed to read the approval marker record…refusing to record"）、scan 循环（"Failed to load approval records…skipping"）、withdraw 恢复（"Failed to recover the claim comment; preserving…"）；claim 独自在最高风险的读取点（claim ref push 之后）破坏该模式。探针在提取的步骤上双向确认。中止是干净的（EXIT trap 释放 ref）；属可诊断性缺陷。与 R9-17 同一机制，本轮重新验证。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| <<< "${live_issue_json}" > /dev/null && | ||
| ! gh issue edit "${ISSUE}" --repo "${REPO}" \ | ||
| --remove-label 'autofix/in-progress'; then | ||
| echo '::warning::Failed to remove the visible claim label; preserving the claim ref for recovery.' |
There was a problem hiding this comment.
[Suggestion] R10-25: Every withdraw 'preserving … for recovery' branch exits 1 BEFORE any issue comment is posted (the explanatory comment is the step's last action), and no automated re-entry path can clear a preserved claim ref: the scan excludes autofix/in-progress, the forced path checks the same, the create-only lease rejects, and a re-run's claim step exits before writing claim_owned which the publish withdraw's if: requires. The workflow concedes 'no sweep exists yet'. (A human 'Re-run failed jobs' on the publish job can re-fire withdraw — recovery needs a human action nothing on the issue prompts.) — Failure scenario: A single transient API failure on any of withdraw's single-attempt calls leaves the ref + in-progress label + bot assignee + claim comment all in place with no notification; the issue looks actively claimed while nothing runs; it is wedged until a human deletes the claim ref and cleans the labels, and nothing tells the human that.
中文说明
withdraw 的每个 "preserving … for recovery" 分支都在发布 issue 评论之前 exit 1（说明评论是步骤的最后一句，只在完全成功时到达），且没有自动重入路径能清除被保留的 claim ref：扫描排除 autofix/in-progress、强制路径同样检查、create-only 租约拒绝、重跑的 claim 步骤在写 claim_owned 之前退出（publish 的 withdraw if: 需要它）。workflow 自己承认 "no sweep exists yet"。失败场景：withdraw 任一单次尝试调用的瞬时 API 失败 → ref + in-progress 标签 + bot assignee + claim 评论全部留在原地且无任何通知；issue 看起来仍在被 claim 而实际没有运行，直到人工删 ref 并清理标签——issue 上没有任何提示。（人工对 publish job "Re-run failed jobs" 可重新触发 withdraw——但那是无人提示的人工动作。）
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
|
||
| Each fresh deterministic, targeted, and publication job independently requires both the artifact base OID and its own trusted checkout `HEAD^{commit}` to equal the captured base output. The deterministic-verification job then requires the trusted base OID to remain an ancestor of the candidate, checks out the candidate detached, and runs build, typecheck, lint, structural gates, and changed-package tests against that exact base. Its immutable job output records the verified candidate OID. | ||
|
|
||
| A second fresh job downloads the original candidate artifact, requires its OID to equal the deterministic job output, loads current issue-bound metadata before starting any candidate lifecycle script, and runs the trusted targeted verifier. It uploads a verified artifact containing only the original bundle, fixed OID, human-authored PR files, and the verifier report. |
There was a problem hiding this comment.
[Suggestion] R10-19: The doc calls the workdir PR files (pr-title.txt/pr-body.md/e2e-report.md) 'human-authored'; they are coding-agent-authored and quarantined as untrusted everywhere they surface. The same doc's publication paragraph relies on the opposite fact ('agent-controlled headings cannot impersonate a sibling trusted-proof section'), and the workflow labels the identical files 'generated by the coding agent'. — Failure scenario: A maintainer extending the pipeline reads the design doc and trusts that provenance — e.g. lifting pr-body.md out of the blockquote quarantine or feeding a workdir file into a workflow-generated proof section — reintroducing exactly the heading-impersonation the blockquote rule exists to prevent.
中文说明
设计文档把 workdir PR 文件（pr-title.txt/pr-body.md/e2e-report.md）称为 "human-authored"；它们是编码 agent 生成的，且在所有出现处都被隔离为不可信（PR 正文中以 blockquote 呈现、步骤摘要中标注 "context only"）。同一文档的发布段落依赖相反事实（“agent 控制的标题不能冒充相邻可信证明区块”），workflow 也把这些文件标注为 "generated by the coding agent"。修复：改为 "agent-authored PR files"。失败场景：扩展该流水线 的维护者信任此来源说明，把 pr-body.md 提出 blockquote 隔离区或把 workdir 文件喂进 workflow 生成的证明区块——重新引入 blockquote 规则要防止的标题冒充。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution: PR #8318 vs mainRoot cause. This PR split the single Semantic, not textual. Both sides changed the publish push's authentication. The PR pushed via git -c credential."https://github.com".helper='!f(){ echo username=x-access-token; echo "password=${GITHUB_TOKEN}"; };f' \
push --no-verify \
--force-with-lease="refs/heads/${BRANCH}:" \
"https://github.com/${REPO}.git" "HEAD:refs/heads/${BRANCH}"The PR's create-only lease sits between Load-bearing.
Not verified. No build/tests were run; I validated YAML/JS syntax and ~45 key contract-test pins mechanically. Remaining risk: the PR's claim-ref pushes (Claim issue / Withdraw steps) still embed the PAT in argv URLs — #8603 eliminated that pattern for the shared ecs-qwen pool, but no test pins these new sites. Worth a follow-up port to the credential-helper form. 中文说明根因：本 PR 把单一 语义冲突：双方都修改了发布推送的鉴权。PR 用 关键点：按 #8435 测试钉住在 Publish PR 步骤 未验证：未运行构建/测试，仅机械校验了约 45 个关键合约钉住。遗留风险：PR 新增的 claim ref 推送仍在 argv URL 中嵌入 PAT，建议后续改为 credential helper 形式。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] R8-33 (still standing from rounds 8-10, re-verified at the reviewed commit): Withdraw claim on failure removes the bot assignee, but every re-entry path this PR adds requires assignees == [bot] (scan select ~1125, forced path ~1078, decision live recheck ~1354, claim precondition ~1517), and nothing ever re-assigns the bot after withdrawal; the posted recovery message names only re-adding autofix/approved. Withdrawn issues enter a permanent silent dead state until someone manually re-assigns the bot.
[Critical] R9-1 (still standing from rounds 9-10, re-verified at the reviewed commit): the bot-assignee candidate gate silently orphans every approved issue that is not already bot-assigned (human-filed and pre-rollout approvals): the scan select drops them with no log line, and the only bot-assignment sites are gh issue create in main-ci-failure-issue.yml (new issues) and the claim step itself (which runs AFTER selection and pre-requires the assignee). The rollout grandfather backfill services exactly this cohort with markers, but the backfilled issues can never pass the select that requires the bot assignee.
中文说明
未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R8-33 (still standing from rounds 8-10, re-verified at the reviewed commit): Withdraw claim on failure removes the bot assignee, but every re-entry path this PR adds requires assignees == [bot] (scan select ~1125, forced path ~1078, decision live recheck ~1354, claim precondition ~1517), and nothing ever re-assigns the bot after withdrawal; the posted recovery message names only re-adding autofix/approved. Withdrawn issues enter a permanent silent dead state until someone manually re-assigns the bot.
[Critical] R9-1 (still standing from rounds 9-10, re-verified at the reviewed commit): the bot-assignee candidate gate silently orphans every approved issue that is not already bot-assigned (human-filed and pre-rollout approvals): the scan select drops them with no log line, and the only bot-assignment sites are gh issue create in main-ci-failure-issue.yml (new issues) and the claim step itself (which runs AFTER selection and pre-requires the assignee). The rollout grandfather backfill services exactly this cohort with markers, but the backfilled issues can never pass the select that requires the bot assignee.
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(issueAutofixPublishJob).toContain( | ||
| 'origin "HEAD:refs/heads/${BRANCH}"', |
There was a problem hiding this comment.
[Critical] R11-1: This PR-added test pins the publication push to the origin remote, but the merged workflow pushes to an explicit URL — npm run test:scripts fails deterministically at the reviewed commit (reproduced twice). The merge with main adopted main's tokenless URL push (qwen-autofix.yml:2505) but kept this branch's origin pin. — Failure scenario: any test:scripts/test:ci run (including this PR's own CI) → the suite is red and the publication-gate test gates nothing while red.
| expect(issueAutofixPublishJob).toContain( | |
| 'origin "HEAD:refs/heads/${BRANCH}"', | |
| expect(issueAutofixPublishJob).toContain( | |
| '"https://github.com/${REPO}.git" "HEAD:refs/heads/${BRANCH}"', |
中文说明
本 PR 新增的测试把发布推送固定到 origin 远端，但合并后的 workflow 使用显式 URL 推送——在受审提交上 npm run test:scripts 确定性失败（已复现两次）。与 main 的合并采用了 main 的无令牌 URL 推送（qwen-autofix.yml:2505），却保留了本分支的 origin 断言。— 失败场景：任何 test:scripts/test:ci 运行（包括本 PR 自己的 CI）都会红，且发布门禁测试在红色状态下形同虚设。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| - name: 'Set up Node.js' | ||
| uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 | ||
| with: | ||
| node-version: '22.x' | ||
|
|
||
| - name: 'Protect candidate worktree' | ||
| run: |- |
There was a problem hiding this comment.
[Critical] R11-2: The two new Set up Node.js steps (issue-autofix-verify, issue-autofix-targeted-e2e) break the lockstep recipe test keeps the Node setup recipe identical across the autofix jobs — it expects exactly 3 steps with cache: 'npm' and now counts 5 (the two new ones deliberately omit the cache). Deterministic failure at the reviewed commit (test.js:6801); the merge adopted main's plain step naming, so the branch's (hosted) filter no longer excludes the new steps. — Failure scenario: every test:scripts run is red until the recipe test and the new steps are reconciled.
中文说明
两个新增的 Set up Node.js 步骤（issue-autofix-verify、issue-autofix-targeted-e2e）破坏了锁步配方测试 keeps the Node setup recipe identical across the autofix jobs——该测试期望恰好 3 个带 cache: 'npm' 的步骤，现在数到 5 个（两个新步骤有意省略缓存）。在受审提交上确定性失败（test.js:6801）；合并采用了 main 的普通步骤命名，分支原来的 (hosted) 过滤器不再排除新步骤。— 失败场景：在配方测试与新步骤对齐之前，每次 test:scripts 都红。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if: |- | ||
| ${{ (failure() || cancelled()) && steps.claim.outcome == 'success' }} | ||
| ${{ always() && needs.issue-autofix.outputs.claim_owned == 'true' && steps.publish.outcome != 'success' && steps.publish.outputs.preserve_claim != 'true' && steps.proof.outputs.preserve_claim != 'true' }} |
There was a problem hiding this comment.
[Critical] R11-3: A claim ref preserved by a failed Withdraw claim on failure is unrecoverable by every workflow path. Withdraw is gated on the CURRENT attempt's claim_owned; if withdraw itself hits an exit-1 preserve path (transient gh error on --remove-label 'autofix/in-progress' at ~2663, or on the final retry-less git push --delete at ~2707), any re-run that re-executes issue-autofix recomputes outputs: the scan excludes -label:autofix/in-progress and the forced path skips on it, so go_issue='', the job exits green WITHOUT claim_owned, this if: is false forever, and every fresh claim dies at the create-only lease ('already has an active or unrecoverable Autofix claim'). — Failure scenario: one transient API error during withdraw → claim ref + labels + assignee persist indefinitely; recovery is manual ref deletion, which no message instructs. Suggested fix: make withdraw re-entrant independent of the current attempt's claim outputs (run it whenever a claim ref naming this run exists for the job's issue), give the final ref delete the same bounded retry as publish's remove_claim_ref, and name the manual recovery in the preserve warnings.
中文说明
一次失败的 Withdraw claim on failure 所保留的 claim 引用（claim ref）无法被任何 workflow 路径恢复。withdraw 以当前 attempt 的 claim_owned 为门槛；若 withdraw 自身走到任一 exit-1 保留路径（~2663 处 --remove-label 'autofix/in-progress' 的瞬时 gh 错误，或 ~2707 处无重试的最终 git push --delete），任何重新执行 issue-autofix 的重跑都会重算输出：扫描排除 -label:autofix/in-progress、强制路径也跳过它，于是 go_issue=''，job 绿色结束且不写 claim_owned，此 if: 永远为假，之后每次新 claim 都死于 create-only lease（“已存在活动或不可恢复的 Autofix claim”）。— 失败场景：withdraw 期间一次瞬时 API 错误 → claim ref + 标签 + 受让人无限期残留；唯一恢复手段是手工删除引用，而没有任何提示信息指明。建议：让 withdraw 的重入不依赖当前 attempt 的 claim 输出（只要该 issue 存在以本 run 命名的 claim ref 就运行），给最终的引用删除加上与发布侧 remove_claim_ref 相同的有界重试，并在保留告警中写明手工恢复步骤。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if [[ "${concurrent_reuse}" != 'true' ]]; then | ||
| gh issue edit "${EXISTING_ISSUE}" \ | ||
| --repo "${REPO}" \ | ||
| --body-file "${body_file}" |
There was a problem hiding this comment.
[Critical] R11-18: The routing transaction adds autofix/routing BEFORE overwriting the issue body, so the issues:labeled event GitHub fires carries the pre-edit body B0; Route issue to Autofix then posts the approval marker over digest(B1). The event-triggered qwen-autofix run computes approved_prose from the event payload (B0): with the marker present (digest B1) the stale-marker branch fires, and since the event label is autofix/routing (not autofix/approved), Record approved issue prose exits 1 with the misleading 'prose changed after its recorded approval' error — a deterministically red qwen-autofix run on EVERY eligible recurrence into an existing issue (the steady state for main failures). The flow self-heals via the second labeled event (autofix/e2e-verification-required, body B1), so the harm is the red run + misleading security-flavored error, not a wedge. — Suggested fix: edit the body BEFORE adding autofix/routing (the event payload then carries the final body), and/or make the record step exit 0 while autofix/routing is present, mirroring every other gate this PR adds.
中文说明
路由事务先添加 autofix/routing 标签、再覆写 issue 正文，因此 GitHub 触发的 issues:labeled 事件携带的是编辑前正文 B0；随后 Route issue to Autofix 基于 digest(B1) 发布批准标记。事件触发的 qwen-autofix 运行从事件 payload（B0）计算 approved_prose：标记已存在（digest 为 B1）时走 stale-marker 分支，又因事件标签是 autofix/routing（不是 autofix/approved），Record approved issue prose 以误导性错误“prose changed after its recorded approval”exit 1——任何一次命中既有 issue 的 eligible 复发（main 失败的常态）都会确定性产生一个红色 qwen-autofix 运行。流程会经第二个 labeled 事件（autofix/e2e-verification-required，正文 B1）自愈，因此危害是红色运行 + 误导性的安全措辞错误，而非永久卡死。— 建议：先编辑正文、再添加 autofix/routing（事件 payload 即携带最终正文），并/或让 record 步骤在存在 autofix/routing 时 exit 0，与本 PR 其他所有门禁保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| node "${helper}" analyze \ | ||
| --workflow "${WORKFLOW_NAME}" \ | ||
| --jobs "${RUNNER_TEMP}/failed-job-manifest.json" \ |
There was a problem hiding this comment.
[Suggestion] Part of a 12-location pattern: behaviors this PR introduces are pinned by no test (or only weakly) — each mutation below survives the full suite, verified by probe/mutation at the reviewed commit. The producer half of the --jobs manifest contract (manifest path, {name, logPath} shape, logPath: null on failed download) has zero test pins; the consumer is unit-tested only with hand-written manifests. — Failure scenario: a future edit or merge resolution applying this mutation ships green. — Suggested fix: undefined
中文说明
pin the handoff in main-ci-failure-issue-workflow.test.js: the analyze invocation carries --jobs "${RUNNER_TEMP}/failed-job-manifest.json", the collect step writes to the same path, and jq emits logPath (null on failure).
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(findCandidateIssuesStep).toContain( | ||
| 'select(.event == "labeled" and (.label.name // "") == $approved)', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Part of a 12-location pattern: behaviors this PR introduces are pinned by no test (or only weakly) — each mutation below survives the full suite, verified by probe/mutation at the reviewed commit. The grandfather timeline pins cover the select(...) but not the load-bearing | max // empty aggregation (workflow:1166): mutation-verified — max → min survives the suite, and a semantic probe shows min drops a valid re-labeled pre-cutover approval (earliest label event makes the 60s window fail). — Failure scenario: a future edit or merge resolution applying this mutation ships green. — Suggested fix: undefined
中文说明
pin the max selection alongside the existing select(...) pin.
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect( | ||
| workflow.indexOf("- name: 'Stage trusted schema gate'"), | ||
| ).toBeLessThan(workflow.indexOf('git checkout "${BRANCH}"')); | ||
| workflow.match(/- name: 'Stage trusted metadata loader'/g) ?? [], | ||
| ).toHaveLength(2); |
There was a problem hiding this comment.
[Suggestion] Part of a 12-location pattern: behaviors this PR introduces are pinned by no test (or only weakly) — each mutation below survives the full suite, verified by probe/mutation at the reviewed commit. The diff deleted the producer job's staging-before-contamination ordering assertion (replaced by count/existence pins only), leaving the trusted-staging invariant of Stage trusted metadata loader unpinned: mutation-verified — relocating the staging below Develop fix stages the agent's copy of the loader with the suite green. The three fresh jobs' staging IS pinned (test.js:2071-2098); the producer's no longer is. — Failure scenario: a future edit or merge resolution applying this mutation ships green. — Suggested fix: undefined
中文说明
pin issueAutofixJob.indexOf("- name: 'Stage trusted metadata loader'") < indexOf("- name: 'Develop fix'").
— qwen3.8-max via Qwen Code /review (v0.21.6)
| rejects candidate commits that touch tests, fixtures, mocks, snapshots, | ||
| scripts, CI files, settings sources (`packages/cli/src/config/settings.ts`, | ||
| `settingsSchema.ts`), the generated `settings.schema.json`, or any other |
There was a problem hiding this comment.
[Suggestion] R11-7: Develop-issue step 4 contradicts the same file's unchanged mode-agnostic "GitHub Actions Rules" (~lines 131-148), which still list npm run generate:settings-schema as trusted "when a settings source changed" and instruct committing the regenerated settings.schema.json in the same commit. — Failure scenario: an issue rooted in a settings source gives the develop-issue agent two opposite instructions; following the general rule gets the whole autofix round rejected by listProtectedCandidateChanges instead of failing fast into failure.md, and the instructions stay permanently contradictory. — Suggested fix: scope the generated-artifact rule to modes where committing verification inputs is still allowed (address-review keeps them), or add a develop-issue caveat pointing at step 4.
中文说明
develop-issue 第 4 步与同文件中未改动的、模式无关的 “GitHub Actions Rules”（约 131-148 行）矛盾：后者仍把 npm run generate:settings-schema 列为“settings 源变更时”的可信命令，并要求在同一提交中提交重新生成的 settings.schema.json。— 失败场景：根因在 settings 源的 issue 会给 develop-issue agent 两条相反指令；遵循通用规则会让整轮 autofix 被 listProtectedCandidateChanges 拒绝，而不是快速失败写 failure.md，且指令永久矛盾。— 建议：把生成物规则限定到仍允许提交验证输入的模式（address-review 保留），或为 develop-issue 加指向第 4 步的说明。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| claim_ref="refs/heads/autofix/claim-issue-${ISSUE}" | ||
| claim_push_url="https://x-access-token:${GITHUB_TOKEN}@github.com/${REPO}.git" |
There was a problem hiding this comment.
[Suggestion] R11-12: The claim create/release and withdraw ref pushes embed CI_DEV_BOT_PAT in the git remote URL, so during each push the PAT appears in the git process argv (/proc/<pid>/cmdline, world-readable on this shared pool) and can surface in git error text. The SAME PR's publish push documents and uses the tokenless one-shot credential-helper form ('The URL is tokenless — the PAT is never persisted in the remote'), and tests pin the tokenless form only for the publish/pushAndReport steps. — Failure scenario: a co-hosted process sampling /proc/*/cmdline during the push window captures a PAT with issues/labels write + ref push scope (mitigated by step-env secrets already being same-UID readable, hence Suggestion). — Suggested fix: use the same git -c credential."https://github.com".helper=... pattern with a tokenless URL here.
中文说明
claim 创建/释放与 withdraw 的引用推送把 CI_DEV_BOT_PAT 嵌入 git 远端 URL，因此每次推送期间 PAT 都会出现在 git 进程 argv（/proc/<pid>/cmdline，在此共享池上全局可读）中，并可能出现在 git 错误文本里。同一 PR 的发布推送记录并使用了无令牌的一次性 credential-helper 形式（“URL 无令牌——PAT 永不持久化到远端”），而测试只对 publish/pushAndReport 步骤固定了无令牌形式。— 失败场景：共享宿主机上的进程在推送窗口内采样 /proc/*/cmdline 即可捕获具备 issues/labels 写 + 引用推送权限的 PAT（因步骤环境密钥本就同 UID 可读而降级为 Suggestion）。— 建议：此处改用同样的 git -c credential."https://github.com".helper=... 无令牌 URL 模式。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| issue-autofix-verify: | ||
| needs: ['route', 'issue-autofix'] | ||
| if: |- | ||
| ${{ needs.issue-autofix.result == 'success' && needs.issue-autofix.outputs.issue != '' && needs.route.outputs.dry_run != 'true' }} |
There was a problem hiding this comment.
[Suggestion] R11-14: dry_run dispatch no longer runs issue-phase deterministic verification: issue-autofix-verify (and transitively issue-autofix-targeted-e2e) is gated off by dry_run != 'true', while the dry_run input description promises 'Assess/develop/address and verify, but do not claim, push, or comment'. Pre-PR the in-job Verification gate had no dry-run exclusion and the removed report step consumed its outcome; the review phase still verifies under dry_run. — Failure scenario: a maintainer dispatch with dry_run=true runs the develop agent on full budget, packages the candidate, and ends green with zero verify verdicts to judge it on. — Suggested fix: drop && needs.route.outputs.dry_run != 'true' from this if: (the verify job is strictly read-only: actions: read/contents: read, no gh writes), or update the input description to say the issue phase does not verify.
中文说明
dry_run 调度不再运行 issue 阶段的确定性验证：issue-autofix-verify（以及传递依赖它的 issue-autofix-targeted-e2e）被 dry_run != 'true' 门禁排除，而 dry_run 输入描述承诺 “Assess/develop/address and verify, but do not claim, push, or comment”。本 PR 之前，job 内验证门禁没有 dry-run 排除，被删除的报告步骤还会消费其 outcome；review 阶段在 dry_run 下仍会验证。— 失败场景：维护者以 dry_run=true 调度时，develop agent 全额运行并打包候选，最后绿色结束却没有任何验证结论可供判断。— 建议：从此 if: 中移除 && needs.route.outputs.dry_run != 'true'（verify job 严格只读：actions: read/contents: read，无 gh 写操作），或更新输入描述说明 issue 阶段不做验证。
— qwen3.8-max via Qwen Code /review (v0.21.6)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: script-lint — shellcheck is not installed on this host; the 7 changed shell scripts were not linted (reported as unreviewed, not clean); 4 workflow files deferred (actionlint embedded-shell limitation, disclosed, not capping).
Not reviewed: the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/check-settings-schema.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/prepare-autofix-verification-worktree.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/resolve-owning-packages.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-verification-command.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/run-autofix-vitest.sh: shellcheck is not installed.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] R12-4 (new; escalates the round-8 Suggestion at comment 3719332353; not posted inline because its anchor line is occupied by the R11-3 comment): cancellation regression — the PR deletes the in-job withdraw (base ran under failure() || cancelled()) and the only replacement lives in the queued issue-autofix-publish job gated on always(), but a cancelled run never starts queued jobs even with always() (proven empirically from this repo's own cancelled schedule runs 31127141927/31126398020/31125681657/31123966842, where the downstream always()-gated job ended conclusion=skipped). Cancellation between the claim's ownership writes (qwen-autofix.yml:1555) and publish startup permanently orphans the claim ref, autofix/in-progress label, bot assignee, and consumed approval; every future claim fails the create-only lease; the file admits 'no sweep exists yet' (:2588). Pre-PR this self-healed. Fix: restore a cancelled()-gated cleanup step in issue-autofix that releases the claim ref (exact-OID lease) and removes the label/assignee/claim comment.
[Critical] R9-4 (still standing, rounds 9-11; re-traced at the reviewed commit): Record approved issue prose no-marker path — both re-approval guards condition on an EXISTING bot marker; with no marker (approval applied while route conditions unmet, or original recording failed), a later trusted event falls through and posts a marker for the current prose, so prose edited after approval is published without approval of the edited text (qwen-autofix.yml:1029/1049). The grandfather path implements exactly the missing 'no update since the approval label event' check for its own no-marker case; the record step does not.
[Critical] R11-1 (still standing; re-measured at the reviewed commit): the PR-added assertion at scripts/tests/qwen-autofix-workflow.test.js:2295 pins the publication push to the origin remote, but this PR's publish step pushes to the tokenless URL "https://github.com/${REPO}.git" "HEAD:refs/heads/${BRANCH}" — npm run test:scripts is red at PR head (2 failed | 121 passed).
[Critical] R11-2 (still standing; re-measured): the two new Set up Node.js steps (issue-autofix-verify, issue-autofix-targeted-e2e) break the lockstep test at scripts/tests/qwen-autofix-workflow.test.js:6801 — expect(nodeSetupSteps).toHaveLength(3) sees 5, and the new steps intentionally omit cache: 'npm' which the same loop asserts present.
[Critical] R11-3 (still standing): a claim ref preserved by a failed 'Withdraw claim on failure' is unrecoverable — the final claim-ref delete is single-attempt while the publish-side equivalent has a 3-attempt backoff loop whose comment names exactly this harm; a transient failure orphans refs/heads/autofix/claim-issue- while all visible state is already removed, wedging every future claim until manual ref deletion (qwen-autofix.yml:2706).
[Critical] R11-18 (still standing; re-verified at the reviewed commit): the routing transaction adds autofix/routing BEFORE overwriting the issue body (main-ci-failure-issue.yml:321-331), so the issues:labeled event carries the pre-edit body B0; the event-triggered record step computes the digest from payload B0 and — with a bot marker present (digest B1) and no routing bypass in the record step (verified absent at HEAD) — exits 1 red with 'prose changed after its recorded approval' on every eligible recurrence into an existing issue (the steady state), self-healing only via the second labeled event.
[Critical] R8-33 (still standing, rounds 8-11; mechanism re-verified this round by enumerating every assignment site): Withdraw claim on failure removes the bot assignee, but every re-entry path this PR adds requires assignees == [bot] (scan select :1125, forced path :1078, decision recheck :1354, claim precondition :1517), and the only bot-assignment sites are issue creation and the claim step itself (which runs AFTER selection) — withdrawn issues enter a permanent silent dead state, and the posted recovery message (re-add autofix/approved) silently no-ops.
[Critical] R9-1 (still standing, rounds 9-11): the bot-assignee candidate gate silently orphans every approved issue that is not already bot-assigned (human-filed and pre-rollout approvals): the scan select drops them with no log line, and nothing assigns the bot on labeling; the rollout grandfather backfill gives this cohort markers, but the backfilled issues can never pass the select that requires the bot assignee (qwen-autofix.yml:1125).
中文说明
未审查：script-lint — shellcheck is not installed on this host; the 7 changed shell scripts were not linted (reported as unreviewed, not clean); 4 workflow files deferred (actionlint embedded-shell limitation, disclosed, not capping)。
未审查：the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/check-settings-schema.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/prepare-autofix-verification-worktree.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/resolve-owning-packages.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-verification-command.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/run-autofix-vitest.sh: shellcheck is not installed。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/repo-hygiene.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R12-4 (new; escalates the round-8 Suggestion at comment 3719332353; not posted inline because its anchor line is occupied by the R11-3 comment): cancellation regression — the PR deletes the in-job withdraw (base ran under failure() || cancelled()) and the only replacement lives in the queued issue-autofix-publish job gated on always(), but a cancelled run never starts queued jobs even with always() (proven empirically from this repo's own cancelled schedule runs 31127141927/31126398020/31125681657/31123966842, where the downstream always()-gated job ended conclusion=skipped). Cancellation between the claim's ownership writes (qwen-autofix.yml:1555) and publish startup permanently orphans the claim ref, autofix/in-progress label, bot assignee, and consumed approval; every future claim fails the create-only lease; the file admits 'no sweep exists yet' (:2588). Pre-PR this self-healed. Fix: restore a cancelled()-gated cleanup step in issue-autofix that releases the claim ref (exact-OID lease) and removes the label/assignee/claim comment.
[Critical] R9-4 (still standing, rounds 9-11; re-traced at the reviewed commit): Record approved issue prose no-marker path — both re-approval guards condition on an EXISTING bot marker; with no marker (approval applied while route conditions unmet, or original recording failed), a later trusted event falls through and posts a marker for the current prose, so prose edited after approval is published without approval of the edited text (qwen-autofix.yml:1029/1049). The grandfather path implements exactly the missing 'no update since the approval label event' check for its own no-marker case; the record step does not.
[Critical] R11-1 (still standing; re-measured at the reviewed commit): the PR-added assertion at scripts/tests/qwen-autofix-workflow.test.js:2295 pins the publication push to the origin remote, but this PR's publish step pushes to the tokenless URL "https://github.com/${REPO}.git" "HEAD:refs/heads/${BRANCH}" — npm run test:scripts is red at PR head (2 failed | 121 passed).
[Critical] R11-2 (still standing; re-measured): the two new Set up Node.js steps (issue-autofix-verify, issue-autofix-targeted-e2e) break the lockstep test at scripts/tests/qwen-autofix-workflow.test.js:6801 — expect(nodeSetupSteps).toHaveLength(3) sees 5, and the new steps intentionally omit cache: 'npm' which the same loop asserts present.
[Critical] R11-3 (still standing): a claim ref preserved by a failed 'Withdraw claim on failure' is unrecoverable — the final claim-ref delete is single-attempt while the publish-side equivalent has a 3-attempt backoff loop whose comment names exactly this harm; a transient failure orphans refs/heads/autofix/claim-issue- while all visible state is already removed, wedging every future claim until manual ref deletion (qwen-autofix.yml:2706).
[Critical] R11-18 (still standing; re-verified at the reviewed commit): the routing transaction adds autofix/routing BEFORE overwriting the issue body (main-ci-failure-issue.yml:321-331), so the issues:labeled event carries the pre-edit body B0; the event-triggered record step computes the digest from payload B0 and — with a bot marker present (digest B1) and no routing bypass in the record step (verified absent at HEAD) — exits 1 red with 'prose changed after its recorded approval' on every eligible recurrence into an existing issue (the steady state), self-healing only via the second labeled event.
[Critical] R8-33 (still standing, rounds 8-11; mechanism re-verified this round by enumerating every assignment site): Withdraw claim on failure removes the bot assignee, but every re-entry path this PR adds requires assignees == [bot] (scan select :1125, forced path :1078, decision recheck :1354, claim precondition :1517), and the only bot-assignment sites are issue creation and the claim step itself (which runs AFTER selection) — withdrawn issues enter a permanent silent dead state, and the posted recovery message (re-add autofix/approved) silently no-ops.
[Critical] R9-1 (still standing, rounds 9-11): the bot-assignee candidate gate silently orphans every approved issue that is not already bot-assigned (human-filed and pre-rollout approvals): the scan select drops them with no log line, and nothing assigns the bot on labeling; the rollout grandfather backfill gives this cohort markers, but the backfilled issues can never pass the select that requires the bot assignee (qwen-autofix.yml:1125).
— qwen3.8-max via Qwen Code /review (v0.21.6)
| /(^|\/)__snapshots__\//.test(file) || | ||
| /(^|\/)(?:test-setup|setup-tests?)\.[cm]?[jt]sx?$/.test(file) || | ||
| /(^|\/)(?:build|esbuild)\.(?:[cm]?[jt]s|sh)$/.test(file) || | ||
| /(^|\/)(?:babel|esbuild|eslint|jest|playwright|postcss|rollup|tailwind|vite|vitest|webpack)(?:\.[^/]*)?\.config\.[cm]?[jt]s$/.test( |
There was a problem hiding this comment.
[Critical] R12-1 (1/2): isProtectedVerificationPath omits vitest's auto-detected vitest.workspace.* / vitest.projects.* files — package-level placement. The tool-config regex requires .config. and no other clause matches, so a candidate commit adding packages/<pkg>/vitest.workspace.ts (or a .projects.* variant) passes the --base scope gate. The per-package gate then runs npm run test --workspace "<pkg>" -- --changed "<base>" --passWithNoTests; vitest 3.2.4 auto-loads a workspace file found next to the config, and a project whose include matches nothing collects zero tests and exits 0 — the candidate ships with none of that package's tests executed. Probe-verified end-to-end with the locked vitest 3.2.4 (baseline failing test exits 1; with the planted file: "No test files found, exiting with code 0", exit 0). The repo-root placement of the same hole achieves code execution and verdict forgery — see the companion comment on run-autofix-vitest.sh. — Failure scenario: candidate commits packages/core/vitest.workspace.ts with export default [{ test: { include: ['no-such-dir/**/*.test.ts'] } }] → the verify gate exits 0 with zero tests executed, defeating the tampering this script exists to prevent.
| /(^|\/)(?:babel|esbuild|eslint|jest|playwright|postcss|rollup|tailwind|vite|vitest|webpack)(?:\.[^/]*)?\.config\.[cm]?[jt]s$/.test( | |
| /(^|\/)vitest\.(?:workspace|projects)\.(?:[cm]?[jt]s|json)$/.test(file) || | |
| /(^|\/)(?:babel|esbuild|eslint|jest|playwright|postcss|rollup|tailwind|vite|vitest|webpack)(?:\.[^/]*)?\.config\.[cm]?[jt]s$/.test( |
中文说明
R12-1（1/2）：isProtectedVerificationPath 遗漏了 vitest 自动探测的 vitest.workspace.* / vitest.projects.* 文件——包级放置。工具配置正则要求 .config.，且没有其他规则匹配，因此候选提交添加 packages/<pkg>/vitest.workspace.ts（或 .projects.* 变体）能通过 --base 范围门禁。随后按包门禁运行 npm run test --workspace "<pkg>" -- --changed "<base>" --passWithNoTests；vitest 3.2.4 会自动加载配置文件旁的 workspace 文件，若其项目 include 匹配不到任何文件，则收集 0 个测试并以退出码 0 结束——候选在完全未运行该包测试的情况下被放行。已在锁定的 vitest 3.2.4 上端到端探针验证（基线下失败测试以 1 退出；放置该文件后输出 "No test files found, exiting with code 0"，退出 0）。同一缺口的仓库根级放置可实现代码执行与结论伪造——见 run-autofix-vitest.sh 上的配套评论。— 失败场景：候选提交 packages/core/vitest.workspace.ts，内容为 export default [{ test: { include: ['no-such-dir/**/*.test.ts'] } }] → 验证门禁以 0 退出且没有执行任何测试，使该脚本本要防止的篡改得逞。修复：在保护列表中加入 workspace/projects 文件规则（见 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| npx --no-install vitest run \ | ||
| --config "${config}" \ |
There was a problem hiding this comment.
[Critical] R12-1 (2/2): Same missing protection, repo-root placement — candidate-controlled code executes inside the targeted-E2E sandbox and can forge the verdict. A root-level vitest.workspace.ts / vitest.projects.* is matched by no clause of isProtectedVerificationPath (no prefix applies at repo root, the config regex requires .config.), passes validateCandidateScope, and is auto-imported by this vitest invocation EVEN WITH explicit --config — probe-verified on the locked vitest 3.2.4: the file's top-level code executed before project resolution (vitest even logs "The workspace file is deprecated… use test.projects", proving it loaded the file). The coordinator is deliberately uid 0 here (report sealing needs root), and the per-case report dir is root-owned 0700 — writable by that uid-0 process: the probe wrote a forged report.json to the exact sealed path and the real validateVitestReport ACCEPTED the fabricated verdict. — Failure scenario: candidate adds a root vitest.workspace.ts whose top-level code overwrites ${report} with a passing payload and exits → ANY candidate passes the targeted-E2E gate. Fix: extend isProtectedVerificationPath with the workspace/projects globs (see the companion comment on validate-autofix-verification-outputs.mjs — one change closes both placements).
中文说明
R12-1（2/2）：同一保护缺口，仓库根级放置——候选可控代码会在定向 E2E 沙箱内执行，并能伪造验证结论。根级 vitest.workspace.ts / vitest.projects.* 不匹配 isProtectedVerificationPath 的任何规则（根级无前缀可匹配，配置正则要求 .config.），能通过 validateCandidateScope，并且即使此处显式传了 --config，vitest 仍会自动导入该文件——已在锁定的 vitest 3.2.4 上探针验证：该文件的顶层代码在项目解析前执行（vitest 甚至输出 "The workspace file is deprecated… use test.projects"，证明确实加载了它）。此处的协调进程有意保持 uid 0（报告封存需要 root），而每个用例的报告目录为 root 所有的 0700——该 uid 0 进程可写：探针向封存路径写入了伪造的 report.json，真实的 validateVitestReport 接受了该伪造结论。— 失败场景：候选添加根级 vitest.workspace.ts，其顶层代码用通过的载荷覆写 ${report} 并退出 → 任意候选都能通过定向 E2E 门禁。修复：在 isProtectedVerificationPath 中加入 workspace/projects 通配规则（见 validate-autofix-verification-outputs.mjs 上的配套评论——一处修改同时关闭两种放置）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| const args = [ | ||
| 'ls-files', | ||
| '--others', | ||
| '--ignored', |
There was a problem hiding this comment.
[Critical] R12-2: Gitlink evasion of the output audit. git ls-files --others [--ignored] never recurses into embedded repositories, and git status --porcelain reports a reconstructed gitlink as clean — so a candidate commit adding a gitlink (mode 160000) can plant arbitrary files inside it, invisible to both audit invocations, the final status check, and the OID check. Reproduced end-to-end against this PR's own functions: isProtectedVerificationPath('vendor/tool') = false and it is not a symlink, so validateCandidateScope accepts it; reconstructing the embedded repo byte-for-byte at the recorded SHA leaves listUnexpectedVerificationOutputs() = [] and git status --porcelain clean while vendor/tool/payload.txt exists in the workspace. The same file planted outside a gitlink IS caught (the probe discriminates). Blast radius today is bounded by the ephemeral credential-free VMs, but this is a demonstrated full bypass of the fail-closed output audit the design doc explicitly claims, in both verification jobs. — Failure scenario: candidate commits gitlink vendor/tool at a precomputed SHA whose tree contains attacker files; candidate test code reconstructs the embedded repo at exactly that SHA during verification → attacker-chosen files present in the workspace, unseen by every audit. Suggested fix: reject new mode-160000 (gitlink) entries in listProtectedCandidateChanges (e.g. via git diff --raw base...HEAD), and treat on-disk contents beneath committed gitlink paths as unexpected outputs in this audit.
中文说明
R12-2：gitlink（嵌入式仓库）逃逸输出审计。git ls-files --others [--ignored] 从不递归进入嵌入式仓库，且 git status --porcelain 对按记录 SHA 重建的 gitlink 报告为干净——因此候选提交添加 gitlink（mode 160000）即可在其中植入任意文件，对两次审计调用、最终 status 检查与 OID 检查全部不可见。已用本 PR 自己的函数端到端复现：isProtectedVerificationPath('vendor/tool') = false 且非符号链接，故 validateCandidateScope 放行；按记录 SHA 逐字节重建嵌入式仓库后，listUnexpectedVerificationOutputs() = []、git status --porcelain 干净，而 vendor/tool/payload.txt 确实存在于工作区。同样的文件若种植在 gitlink 之外则会被捕获（探针可区分）。当前影响范围受限于一次性无凭据虚拟机，但这是对本 PR 设计文档明确声称的失败关闭输出审计的已证实完全绕过，且同时影响两个验证 job。— 失败场景：候选以预计算 SHA 提交 gitlink vendor/tool（其树包含攻击者文件）；候选测试代码在验证期间按该 SHA 精确重建嵌入式仓库 → 攻击者所选文件存在于工作区且所有审计均不可见。修复建议：在 listProtectedCandidateChanges 中拒绝新增的 mode-160000（gitlink）条目（如通过 git diff --raw base...HEAD），并在本审计中将已提交 gitlink 路径下的磁盘内容视为意外输出。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| echo "Issue #${EXISTING_ISSUE} has a live claim, cancellation, ownership change, or linked PR; leaving it and its trusted metadata unchanged." | ||
| echo "number=${EXISTING_ISSUE}" >> "${GITHUB_OUTPUT}" | ||
| echo 'route_allowed=false' >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
[Critical] R12-3: Never-approved carrier wedge — an eligible recurrence that matches an OPEN bot-filed carrier created ineligible is silently never routed. A first occurrence can be created ineligible (a macOS/docker leg failing, or one failed job-log download — both tolerated by this PR's own manifest builder); the carrier issue carries test markers in its body but only type/bug. When an eligible recurrence later matches it via the marker search, this gate fails on the missing status/ready-for-agent + autofix/approved + bot assignee — state that NO code path ever adds to an existing issue (only the create path applies those, and it never runs while the carrier matches the marker). The issue is OPEN, so no comment is posted, and the step exits 0 logging a factually wrong reason (no claim/cancellation/ownership-change/linked-PR is true). Every future eligible recurrence of this failure signature exits the same way — trusted routing is permanently dead until a human manually adds both labels AND the bot assignee, with nothing on GitHub indicating why. The diff's own comment on the closed-ineligible reset states a never-approved carrier must not be reused because it carries no approval state — an OPEN never-approved carrier gets the opposite treatment. — Failure scenario: run A (macOS leg fails) → carrier #N with markers but no approval labels; run B (linux-only, eligible) → marker search finds #N, this gate fails, exit 0 with the wrong log line; the signature is wedged forever. Suggested fix: distinguish never-approved from cancelled — if the carrier is OPEN, bot-authored, and lacks both labels, either promote it (apply ready/approved/routing + bot assignee, consistent with the create path's auto-approval) or clear EXISTING_ISSUE and file a new issue as the closed reset does; at minimum post a comment and stop logging the false reason.
中文说明
R12-3：从未被批准的载体 issue 造成的死锁——命中一个以「不合格」状态创建的 OPEN bot 载体的合格复发，会被静默地永久拒绝路由。首次出现可能以不合格状态创建 issue（macOS/docker 分支失败，或某个 job 日志下载失败——两者都是本 PR 自己的 manifest 构建逻辑所容忍的）；载体 issue 正文中带有测试 marker，但只有 type/bug 标签。之后当合格复发通过 marker 搜索命中它时，此门禁因缺少 status/ready-for-agent + autofix/approved + bot 指派人而失败——但这些状态没有任何代码路径会为既有 issue 补上（只有创建路径会加，而载体命中 marker 时创建路径永远不会执行）。该 issue 是 OPEN，所以不会发评论，步骤以 0 退出并输出一条与事实不符的日志（并不存在 claim/取消/所有权变更/关联 PR）。该失败签名的每一次未来合格复发都会同样退出——可信路由永久失效，直到人工同时加上两个标签并指派 bot，而 GitHub 上没有任何线索说明原因。diff 中关于「已关闭的不合格载体」重置的注释明确写道：不带批准状态的载体不应被复用——而一个 OPEN 且从未被批准的载体却得到了相反的待遇。— 失败场景：运行 A（macOS 分支失败）→ 载体 #N 带 marker 但无批准标签；运行 B（仅 linux、合格）→ marker 搜索命中 #N，此门禁失败，以错误日志退出 0；该签名永久卡死。修复建议：区分「从未批准」与「已被取消」——若载体是 OPEN、bot 创建且缺少两个标签，要么提升它（加上 ready/approved/routing + bot 指派，与创建路径的自动批准保持一致），要么像已关闭重置那样清空 EXISTING_ISSUE 并新建 issue；至少应发一条评论并停止输出错误原因的日志。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if [[ -n "$(git status --porcelain)" ]]; then | ||
| echo '❌ Candidate branch has uncommitted changes.' | ||
| git status --short | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
[Critical] R12-5: The develop-issue contract this PR writes collides with this new gate. SKILL.md step 3 instructs "construct the closest focused regression test or surrogate", and the new step 4 blesses the residue: "scratch tests for local verification are fine but must stay uncommitted". But git status --porcelain includes untracked files, so the instructed scratch-test residue hard-fails Package candidate — and the agent cannot clean up: the Develop fix coreTools allowlist has no rm / git clean / delete capability of any kind, write_file cannot delete, and run-agent.mjs performs no workspace cleanup (the later git clean -ffdx calls run in the verify/targeted jobs' fresh checkouts, unreachable once this gate exits 1). A contract-following run therefore discards a valid fix and withdraws the claim; and when the agent writes a legitimate failure.md decline after constructing scratch tests, the :1678 check fires as "wrote failure.md after leaving a dirty workspace" instead of the clean intentional-decline branch. Probe: adding an untracked my-scratch.test.ts makes git status --porcelain print ?? my-scratch.test.ts and this check exits 1. — Failure scenario: agent follows steps 3+4 → valid fix discarded, claim withdrawn, approval consumed; the failure.md abort path misfires the same way. Suggested fix: restrict the check to tracked changes (below), or clean untracked files before the check, or give the develop agent a deletion capability and instruct steps 4/8 to remove scratch tests before finishing.
| if [[ -n "$(git status --porcelain)" ]]; then | |
| echo '❌ Candidate branch has uncommitted changes.' | |
| git status --short | |
| exit 1 | |
| fi | |
| if [[ -n "$(git status --porcelain --untracked-files=no)" ]]; then |
中文说明
R12-5：本 PR 编写的 develop-issue 约定与此新门禁冲突。SKILL.md 第 3 步要求「构造最接近的聚焦回归测试或替代测试」，新的第 4 步允许残留物存在：「用于本地验证的临时测试没问题，但保持未提交状态」。然而 git status --porcelain 包含未跟踪文件，因此按约定留下的临时测试残留会使 Package candidate 硬性失败——且 agent 无法清理：Develop fix 的 coreTools 允许列表中没有任何 rm / git clean / 删除能力，write_file 不能删除文件，run-agent.mjs 也不做任何工作区清理（后面的 git clean -ffdx 调用运行在 verify/targeted job 的全新检出里，一旦此门禁以 1 退出就不可达）。因此一次完全遵守约定的运行会丢弃有效修复并撤回 claim；而当 agent 在构造临时测试后写出合理的 failure.md 拒绝时，:1678 的检查会以「留下脏工作区后写了 failure.md」触发，而不是走干净的主动拒绝分支。探针：添加未跟踪的 my-scratch.test.ts 会使 git status --porcelain 输出 ?? my-scratch.test.ts，此检查以 1 退出。— 失败场景：agent 遵循第 3、4 步 → 有效修复被丢弃、claim 被撤回、批准被消耗；failure.md 中断路径也同样误触发。修复建议：将检查限制为已跟踪文件的变更（见 suggestion），或在检查前清理未跟踪文件，或给 develop agent 增加删除能力并在第 4/8 步要求结束前删除临时测试。
— qwen3.8-max via Qwen Code /review (v0.21.6)
Autofix could publish a fix after deterministic checks alone, without proving that the exact E2E case which created the issue passes on the exact commit being published. Carry immutable failure metadata outside editable issue prose, bind maintainer approval to the exact issue title and body, verify the exact candidate commit in an isolated credential-free environment, rerun the exact supported E2E case there, and allow publication only for the commit that passed both deterministic and targeted verification. Publication revalidates issue ownership, cancellation state, approved prose, metadata, claim ownership, branch OID, PR head OID, and closing-reference bindings. Unsupported, incomplete, Docker-dependent, provider-dependent, stale, ambiguous, or malformed metadata stays ineligible and fails closed. The two sealed verification jobs run setup-node with no cache input: the entry behind `cache: npm` is repo-scoped and written by ordinary jobs, so restoring it would feed unverified bytes into the very environment this change isolates. The Node-setup contract pins now count the cached and sealed steps separately, and a hardcoded `cache: 'npm'` anywhere in the workflow is pinned against directly, so neither bucket can absorb a new job silently. Supersedes #8318 (squashed into a single commit).
6 participants
What this PR does
This PR adds a fail-closed verification chain for Autofix issues created from supported post-merge E2E failures. It transports immutable failure metadata outside editable issue prose, binds maintainer approval to the exact issue title and body, verifies the exact candidate commit in an isolated credential-free environment, reruns the exact supported E2E case, and only allows publication of the commit that passed both deterministic and targeted verification. Publication also revalidates issue ownership, cancellation state, approved prose, metadata, claim ownership, branch OID, PR head OID, and closing-reference bindings.
Why it's needed
The existing Autofix publication gate could run deterministic checks without proving that the specific E2E case which created an issue passed on the published commit. Editable issue text, candidate-controlled verification inputs, process and filesystem escape paths, and API uncertainty also required explicit trust boundaries so unsupported or unverifiable failures stop safely instead of producing an inadequately verified PR.
Reviewer Test Plan
How to verify
Reviewers should confirm that a supported Linux, sandbox-none post-merge E2E failure produces immutable issue-bound metadata; scheduled Autofix only selects prose with a matching bot-authored approval digest; candidate changes to tests, verification scripts, configuration, dependencies, or other protected inputs are rejected; deterministic verification and the exact targeted test run against the same candidate OID; issue prose or ownership changes stop publication; and the final PR body presents workflow-generated proof before blockquoted agent-authored notes. Unsupported, incomplete, Docker-dependent, provider-dependent, stale, ambiguous, or malformed metadata should remain ineligible or fail closed.
Local verification completed with the Autofix workflow contract suites (124 tests), verifier and failure-signature Node tests (61 tests), repository build, typecheck, lint, changed-file Prettier checks, changed-workflow actionlint, and
git diff --check.Evidence (Before & After)
Before: Autofix could publish after deterministic checks without independently rerunning and binding the originating E2E case to the exact published candidate commit.
After: eligible failures carry authenticated metadata through an isolated exact-test verifier, and branch/PR publication is allowed only for the exact verified OID while issue, approval, metadata, claim, and PR bindings remain valid.
Tested on
Environment (optional)
macOS local development environment with Node.js 22. No Docker, Podman, Colima, Lima, Multipass, Vagrant, or OrbStack runtime was available, so Linux DAC behavior and a real targeted E2E execution were not run locally.
Risk & Scope
setpriv,/procand process-group cleanup behavior, Docker-socket denial, and a real targeted E2E run require Ubuntu validation. Duplicate first-occurrence issues remain possible because GitHub issue creation has no atomic marker CAS. Review-address and takeover flows are not redesigned by this change. The planned two consecutive clean forward/reverse audit rounds were stopped at user request; the latest exact tree completed one clean reverse audit while its matching forward audit was cancelled.Linked Issues
N/A
中文说明
What this PR does
此 PR 为由受支持的合并后 E2E 失败生成的 Autofix issue 增加 fail-closed 强验证闭环。它在可编辑 issue 文本之外传递不可变失败元数据，将维护者批准绑定到精确的 issue 标题和正文，在无凭据隔离环境中验证精确候选提交，重新运行精确的受支持 E2E 用例，并且只允许发布同时通过确定性验证和 targeted 验证的提交。发布阶段还会重新验证 issue 所有权、取消状态、已批准 prose、元数据、claim 所有权、分支 OID、PR head OID 与 closing reference 绑定。
Why it's needed
现有 Autofix 发布门禁可以运行确定性检查，但不能证明创建 issue 的特定 E2E 用例已在最终发布提交上通过。可编辑 issue 文本、候选代码可控的验证输入、进程与文件系统逃逸路径，以及 API 不确定性也需要明确的信任边界，使不受支持或无法验证的失败安全停止，而不是生成验证不足的 PR。
Reviewer Test Plan
How to verify
审阅者应确认：受支持的 Linux、sandbox-none 合并后 E2E 失败会生成与 issue 绑定的不可变元数据；定时 Autofix 只选择与 bot approval digest 匹配的 prose；候选对测试、验证脚本、配置、依赖或其他受保护输入的修改会被拒绝；确定性验证与精确 targeted 测试针对同一 candidate OID；issue prose 或所有权变化会停止发布；最终 PR 正文先展示 workflow 生成的可信证明，再以引用形式展示 agent 生成的说明。不受支持、不完整、依赖 Docker、依赖 provider、过期、歧义或格式错误的元数据应保持不可选或 fail closed。
本地验证包括 Autofix workflow 合同测试（124 项）、verifier 与 failure-signature Node 测试（61 项）、仓库 build、typecheck、lint、改动文件 Prettier 检查、改动 workflow actionlint，以及
git diff --check。Evidence (Before & After)
之前：Autofix 可以在确定性检查后发布，但不会独立重跑并绑定创建 issue 的 E2E 用例与精确发布候选提交。
之后：符合条件的失败通过隔离的精确测试 verifier 传递认证元数据，只有精确 verified OID 才能发布分支和 PR，并且 issue、批准、元数据、claim 与 PR 绑定必须持续有效。
Tested on
Environment (optional)
macOS 本地开发环境，Node.js 22。没有可用的 Docker、Podman、Colima、Lima、Multipass、Vagrant 或 OrbStack runtime，因此未在本地运行 Linux DAC 行为与真实 targeted E2E。
Risk & Scope
setpriv、/proc与进程组清理行为、Docker socket 拒绝，以及真实 targeted E2E 运行需要 Ubuntu 验证。由于 GitHub issue 创建缺少原子 marker CAS，首次并发失败仍可能产生重复 issue。此变更不重新设计 review-address 与 takeover 流程。原计划的连续两轮正向/反向 clean 审计已按用户要求停止；最新精确 tree 完成了一次 clean 反向审计，对应正向审计被取消。Linked Issues
N/A