Skip to content

refactor(autofix): extract review verification runner - #7644

Merged
wenshao merged 4 commits into
QwenLM:mainfrom
qqqys:agent/autofix-review-verification-runner
Jul 24, 2026
Merged

refactor(autofix): extract review verification runner#7644
wenshao merged 4 commits into
QwenLM:mainfrom
qqqys:agent/autofix-review-verification-runner

Conversation

@qqqys

@qqqys qqqys commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Extracts the review-address deterministic verification body into a reusable trusted runner. The workflow stages that runner from the trusted base checkout and invokes it from the existing verification step.

This is a behavior-preserving refactor: the existing condition, step id, environment inputs, output keys, no-op handling, failure classification, deterministic checks, changed-workspace tests, hook isolation, and push/report consumers remain unchanged.

Why it's needed

Same-run repair needs to execute the same deterministic verification more than once without duplicating a large inline shell block. Establishing one trusted runner first makes the later control-flow change small and reviewable while preserving the current push-after-independent-verification boundary.

Reviewer Test Plan

How to verify

  1. Confirm the extracted runner is staged once from the trusted base before the workflow switches to the candidate branch.
  2. Confirm the review verification step retains its existing condition and id and only invokes the staged runner.
  3. Compare the extracted shell body with the previous inline body; all 154 behavior lines should match exactly.
  4. Run the workflow regression suite and confirm all 101 tests pass, including committed-state classification, no-op ordering, gate-rejection capture, hook isolation, and report behavior.

Evidence (Before & After)

N/A — behavior-preserving workflow refactor.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Local Node.js 22 workspace. Verified with exact old/new shell-body comparison, focused workflow tests (101/101), full build, typecheck, ESLint, Bash syntax validation, and targeted Prettier check.

Risk & Scope

  • Main risk or tradeoff: Verification now depends on the staged runner being present; structural tests pin its trusted staging location and invocation.
  • Not validated / out of scope: Retry behavior, new verification checks, trigger changes, and hosted-runner end-to-end execution.
  • Breaking changes / migration notes: None.

Linked Issues

Refs #7638

中文说明

本 PR 做了什么

将 review-address 的 deterministic verification 主体抽取为可复用的 trusted runner。workflow 会从 trusted base checkout 暂存该 runner,并在现有 verification step 中调用。

这是行为等价重构:现有 condition、step id、环境输入、输出 key、no-op 处理、失败分类、deterministic checks、changed-workspace tests、hook 隔离以及 push/report 消费逻辑均保持不变。

为什么需要

同轮修复需要多次执行同一套 deterministic verification,同时不能复制一大段 inline shell。先建立单一 trusted runner,可以让后续控制流变更更小、更容易评审,并保留“独立验证通过后才 push”的现有安全边界。

Reviewer Test Plan

如何验证

  1. 确认抽取后的 runner 在 workflow 切换到候选分支前,仅从 trusted base 暂存一次。
  2. 确认 review verification step 保留现有 condition 和 id,并且只调用暂存后的 runner。
  3. 对比抽取后的 shell body 与此前 inline body,154 行行为代码应逐行完全一致。
  4. 运行 workflow 回归测试,确认 101 个测试全部通过,包括 committed-state 分类、no-op 顺序、gate-rejection 捕获、hook 隔离和 report 行为。

证据(Before & After)

不适用——这是行为等价的 workflow 重构。

测试平台

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

本地 Node.js 22 workspace。已验证新旧 shell body 精确对比、focused workflow tests(101/101)、完整 build、typecheck、ESLint、Bash 语法和定向 Prettier 检查。

风险与范围

  • 主要风险或取舍:验证现在依赖暂存后的 runner 存在;结构测试会固定其 trusted staging 位置和调用方式。
  • 未验证 / 不在范围内:retry 行为、新验证检查、触发变更和 hosted-runner 端到端执行。
  • Breaking changes / 迁移说明:无。

关联 Issue

Refs #7638

@qqqys
qqqys marked this pull request as ready for review July 24, 2026 03:55
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Re-run on the current head (the autofix loop pushed changes since the last pass).

Template looks good ✓

Problem: This is phase 2 of the maintainer-driven plan in #7638 ("extract the review verification gate into a reusable trusted runner without changing behavior"). It's structural prep, not a bug fix — the motivation is concrete and maintainer-defined: same-run repair (phase 3) needs to execute the same deterministic verification more than once without duplicating a large inline shell block. So the usual before/after reproduction doesn't apply here; the "problem" is the duplication that phase 3 would otherwise have to carry.

Direction: Aligned. It implements exactly the second bullet of #7638, and the issue's constraints (preserve the push-after-independent-verification boundary, behavior-preserving, covered by workflow tests) are precisely what this PR targets.

Size: Not applicable — no core-module paths. The change is confined to .github/ (workflow + one new script) and scripts/tests/. Of the 240/178 lines, the runner script (162 added) and the workflow body (155 removed) are a relocation of the same block; the rest is test updates.

Approach: Scope feels right and genuinely minimal — it's a pure extraction with no drive-by changes. I verified separately (Stage 2) that the extracted body is byte-for-byte identical to the inline block it replaces, so there's nothing to cut.

Moving on to code review. 🔍

中文说明

基于最新 head 重跑(autofix loop 自上次审查后又推送了改动)。

模板完整 ✓

问题: 这是 maintainer 主导的 #7638 计划的第二阶段("将 review verification gate 抽取为可复用的 trusted runner,且不改变行为")。它是结构性铺垫,而非 bug 修复——动机具体且由 maintainer 定义:同轮修复(第三阶段)需要多次执行同一套 deterministic verification,而又不能复制一大段 inline shell。因此通常的 before/after 复现在此不适用;这里的"问题"是第三阶段 otherwise 必须承担的重复代码。

方向: 对齐。它精确实现了 #7638 的第二条,且 issue 的约束(保留"独立验证通过后才 push"的边界、行为等价、由 workflow 测试覆盖)正是本 PR 的目标。

规模: 不适用——未触及核心模块路径。改动仅限于 .github/(workflow + 一个新脚本)和 scripts/tests/。在 240/178 行中,runner 脚本(162 行新增)与 workflow 主体(155 行删除)是同一代码块的搬迁;其余为测试更新。

方案: 范围合理且确实最小化——纯粹抽取,没有顺手改动。我已在 Stage 2 单独验证抽取后的主体与被替换的 inline 块逐字节一致,因此没有可砍的部分。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 4ec8d2ea08e77b0cd3fd5e2b81c03a54ace33b07 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Code review

Independent proposal first. Given "run the same deterministic verification more than once without duplicating the inline shell," I'd move the gate body into a standalone script that is staged from the trusted base checkout into RUNNER_TEMP and invoked from the existing step — reusing the exact trusted-runner pattern already used for check-settings-schema.sh, check-autofix-contracts.sh, and resolve-owning-packages.sh. The critical constraint is that the verification logic must keep coming from the trusted base, never from the branch under verification (an agent commit can touch .github/scripts). That is exactly what this PR does.

Findings — no blockers.

  • Extraction is byte-for-byte identical. I diffed the new run-autofix-review-verification.sh body against the current inline block on main (de-indented): all 155 lines match exactly. The only additions are #!/usr/bin/env bash and set -eo pipefail, which match GitHub Actions' default bash shell (bash -eo pipefail) — so runtime semantics, including the pipefail that run_check relies on, are preserved.
  • Security model intact and test-pinned. The runner is staged via cp .github/scripts/run-autofix-review-verification.sh "${RUNNER_TEMP}/…" in the "Stage trusted schema gate and agent runner" step (which runs before "Prepare branch and feedback" checks out the candidate branch), and the gate step invokes bash "${RUNNER_TEMP}/run-autofix-review-verification.sh" — never the working-tree copy. The structural tests pin all three invariants: the copy happens exactly once, staging precedes the branch switch, and the gate step does not invoke .github/scripts/… directly and no longer contains npm run build (the body moved out).
  • Test updates are consistent with the move. verificationGateBodies = [issue-autofix gate (still inline), extracted runner] correctly substitutes the relocated body wherever the old verificationGateSteps[1] was asserted; the core.hooksPath count is preserved by concatenating workflow + runner (one occurrence relocated, total still 5); the function-definition regexes were re-anchored for the de-indented \n}. No assertions were weakened — they were relocated alongside the code.

No correctness, security, or AGENTS.md concerns.

Test evidence (this PR's own CI — I did not run any PR code)

Fetched check-runs for the reviewed commit 4ec8d2ea08:

  • 46 checks, all completed — every conclusion is success or skipped; zero failures.
  • Test (ubuntu-latest, Node 22.x)success. This suite includes the workflow structural tests changed here (scripts/tests/qwen-autofix-workflow.test.js, scripts/tests/package-scripts.test.js), so the extraction's invariants pass on real CI.
  • Test (macos-latest / windows-latest) → skipped; Integration Tests (CLI, No Sandbox) → skipped (expected for a workflow-only change).
  • review-scan, precheck-pr / precheck, Classify PR, web-shell E2E Smoke (ubuntu) → success.

Not a TUI surface, so no real-scenario tmux run applies (and this is an unattended CI re-run, where live runs are out of scope by rule). Not verified: hosted-runner end-to-end execution of the refactored gate — the PR itself scopes that out, and the structural tests + green CI are the appropriate signal here.

中文说明

代码审查

先独立提出方案。 给定"多次执行同一套 deterministic verification 而不复制 inline shell"这个目标,我会把 gate 主体移入一个独立脚本,从 trusted base checkout 暂存到 RUNNER_TEMP,再由现有 step 调用——复用 check-settings-schema.shcheck-autofix-contracts.shresolve-owning-packages.sh 已在用的 trusted-runner 模式。关键约束是验证逻辑必须始终来自 trusted base,绝不能来自被验证的分支(agent 的 commit 可能改动 .github/scripts)。本 PR 正是这么做的。

结论——无阻塞项。

  • 抽取逐字节一致。 我将新的 run-autofix-review-verification.sh 主体与 main 上当前的 inline 块(去缩进后)做了 diff:155 行全部精确匹配。唯一的新增是 #!/usr/bin/env bashset -eo pipefail,与 GitHub Actions 默认的 bash shell(bash -eo pipefail)一致——因此运行时语义(包括 run_check 依赖的 pipefail)得以保留。
  • 安全模型完整且由测试固定。 runner 通过 "Stage trusted schema gate and agent runner" step 中的 cp … "${RUNNER_TEMP}/…" 暂存(该 step 在 "Prepare branch and feedback" 检出候选分支之前运行),gate step 调用 bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"——绝不调用工作区副本。结构测试固定了全部三条不变量:copy 恰好发生一次、暂存先于分支切换、gate step 直接调用 .github/scripts/… 且不再包含 npm run build(主体已移出)。
  • 测试更新与搬迁一致。 verificationGateBodies = [issue-autofix gate(仍为 inline), 抽取后的 runner] 在所有原先断言 verificationGateSteps[1] 处正确替换为搬迁后的主体;core.hooksPath 计数通过拼接 workflow + runner 保持不变(一处搬迁,总数仍为 5);函数定义的正则按去缩进后的 \n} 重新锚定。没有断言被削弱——它们随代码一同搬迁。

无正确性、安全性或 AGENTS.md 问题。

测试证据(来自本 PR 自身的 CI——我未运行任何 PR 代码)

针对被审查 commit 4ec8d2ea08 拉取 check-runs:

  • 46 个检查全部 completed——结论均为 successskipped,零失败。
  • Test (ubuntu-latest, Node 22.x)success。该套件包含此处改动涉及的 workflow 结构测试,因此抽取的不变量在真实 CI 上通过。
  • Test (macos / windows) → skipped;Integration Tests → skipped(纯 workflow 改动,符合预期)。

非 TUI 界面,因此不适用真实场景 tmux 测试(且这是无人值守的 CI 重跑,按规则不做实时运行)。未验证:重构后 gate 的 hosted-runner 端到端执行——PR 本身已将其排除在范围外,此处结构测试 + 绿色 CI 即为恰当的信号。

Qwen Code · qwen3.8-max-preview

Reviewed at 4ec8d2ea08e77b0cd3fd5e2b81c03a54ace33b07 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 3/5 — clean, verifiably behavior-preserving review; capped at 3 purely by the fork-refactor guardrail, which needs a maintainer's sign-off. @wenshao has already approved this exact commit (4ec8d2ea08) ✅, so the human-eye requirement the guardrail exists to enforce is satisfied.

Going back to my independent proposal: the PR matches it precisely. I asked "extract the gate body into a trusted-staged runner, invoked from RUNNER_TEMP, reusing the existing pattern" — that's what landed, nothing more. The extraction is byte-for-byte identical to the inline block it replaces (verified by direct diff, not by trusting the PR's claim), the only additions reproduce GitHub's default bash flags, and the trusted-base staging invariant is preserved and pinned by tests. The scope is genuinely minimal — no drive-by edits, no scope creep, and the test changes relocate assertions rather than weaken them.

If I had to maintain this in six months I'd thank the author: the verification logic now lives in one reviewable file instead of an inline block, which is exactly what phase 3 (same-run repair) needs, and the security boundary that makes this gate trustworthy is intact. CI is fully green (46 checks, no failures; the workflow structural tests pass on Ubuntu).

I'm not approving only because the deterministic guardrail bars the bot from auto-approving any cross-repo refactor — that's policy, not doubt about the code. With @wenshao's approval already on this commit, this is ready for a maintainer to merge.

⏸️ Deferring to @wenshao — fork-refactor guardrail (policy): the bot never auto-approves a cross-repository refactor. You've already approved 4ec8d2ea08; this just needs your merge.

中文说明

置信度:3/5 —— 审查干净、可验证地行为等价;3 分纯粹由 fork-refactor 护栏所限,该护栏需要 maintainer 签字。@wenshao 已经批准了这个确切的 commit(4ec8d2ea08)✅,因此护栏所要强制的"人眼审查"要求已满足。

回到我的独立方案:PR 与之精确吻合。我提出"将 gate 主体抽取为 trusted 暂存的 runner,从 RUNNER_TEMP 调用,复用现有模式"——落地的正是如此,不多不少。抽取与被替换的 inline 块逐字节一致(通过直接 diff 验证,而非轻信 PR 的声明),唯一的新增复现了 GitHub 默认的 bash 标志,trusted-base 暂存不变量得以保留并由测试固定。范围确实最小化——没有顺手改动、没有范围蔓延,测试改动是搬迁断言而非削弱断言。

如果六个月后由我维护,我会感谢作者:验证逻辑现在位于一个可审查的文件中,而非 inline 块,这正是第三阶段(同轮修复)所需的,而使该 gate 值得信赖的安全边界完好无损。CI 全绿(46 个检查,零失败;workflow 结构测试在 Ubuntu 上通过)。

我之所以不批准,仅因为确定性护栏禁止 bot 自动批准任何跨仓库 refactor——这是策略,而非对代码的疑虑。鉴于 @wenshao 已在此 commit 上批准,本 PR 已可由 maintainer 合并。

⏸️ 转交 @wenshao —— fork-refactor 护栏(策略):bot 永不自动批准跨仓库 refactor。你已批准 4ec8d2ea08;此处只需你的合并。

Qwen Code · qwen3.8-max-preview

Reviewed at 4ec8d2ea08e77b0cd3fd5e2b81c03a54ace33b07 · re-run with @qwen-code /triage

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Code Review — refactor(autofix): extract review verification runner

Verdict: ✅ Merge-ready. This is a genuinely behavior-preserving extraction, verified below against the actual base/head files rather than taken on faith.

What it does

Lifts the review-address Verification gate inline shell body (154 lines) out of .github/workflows/qwen-autofix.yml into a new trusted script .github/scripts/run-autofix-review-verification.sh, staged from the trusted base checkout and invoked as bash "${RUNNER_TEMP}/run-autofix-review-verification.sh". Groundwork for same-run repair, which needs to run the gate more than once without duplicating the block (Refs #7638).

Verification I ran

  • Body is byte-identical. Diffing the base inline body (de-indented) against the new script minus its shebang/set header shows zero content difference — the only delta is one trailing YAML block-separator blank line. The PR's "154 lines match exactly" claim holds.
  • Shell flags preserved. The added #!/usr/bin/env bash + set -eo pipefail header exactly replicates GitHub's default step shell (bash --noprofile --norc -eo pipefail {0} — no -u/-x), so -e/pipefail semantics carry over into the child shell. bash -n is clean.
  • Env inheritance is sound. WORKDIR and BRANCH are job-level env: on the review-address job (not step-level), and GITHUB_OUTPUT/RUNNER_TEMP are runner-provided — all are real environment variables, so the child bash inherits them. Step keeps id: 'verify', so committed=/outcome= writes to $GITHUB_OUTPUT stay attributed to the step. git checkout/git config core.hooksPath are on-disk operations, so they persist past the child shell's exit for the downstream push/report steps.
  • Trusted-staging order is correct. cp run-autofix-review-verification.sh lands in Stage trusted schema gate and agent runner (alongside check-settings-schema.sh + resolve-owning-packages.sh), which runs before Prepare branch and feedback switches the working tree to the PR branch, which runs before the gate. So the gate logic still comes from the trusted base and can't be tampered with by the branch under verification.

Strengths

  • The security invariant is now explicitly test-pinned. New assertions forbid the working-tree invocation (not.toContain('bash .github/scripts/run-autofix-review-verification.sh')), require the RUNNER_TEMP form, pin the staging step before Prepare branch and feedback, keep the hook-sever count at 5 across workflow + runner, and drive the committed_rc snippet against the extracted script. The re-indentation of the body regexes (\n {10}\}\n\}) is correct.
  • Comment rationale (verdict-before-detail ordering, four-backtick fence, git diff --quiet exit-1-vs-128) travels with the code into the script, so the reasoning isn't lost in the move.

Notes (non-blocking)

  • The issue-fix job's Verification gate stays inline and is similar but not identical to this review runner (different git refs), so this isn't a candidate for sharing one runner — no new drift risk introduced. Just worth keeping in mind that review-verification now has a single home while issue-verification does not.
  • Staging surface grows to three trusted scripts. Correctness now depends on all three being staged-from-base and invoked-from-RUNNER_TEMP consistently; that's exactly what the new tests guard, so the risk is contained.

Nice, reviewable first step — the follow-up control-flow change should indeed be small on top of this.

中文说明

结论:✅ 可以合并。 这是一次真正行为等价的抽取,以下结论均基于对 base/head 实际文件的核对,而非仅凭 PR 描述。

做了什么: 将 review-address 的 Verification gate 内联 shell 主体(154 行)从 .github/workflows/qwen-autofix.yml 抽取为新的 trusted 脚本 .github/scripts/run-autofix-review-verification.sh,从 trusted base 暂存并以 bash "${RUNNER_TEMP}/run-autofix-review-verification.sh" 调用。为 same-run repair 做铺垫(Refs #7638)。

我做的验证:

  • 主体逐字节一致。 将 base 内联主体去缩进后与新脚本(去掉 shebang/set 头)对比,内容零差异,唯一区别是一行 YAML 块分隔空行。
  • shell 选项保持一致。 新增的 set -eo pipefail 精确复现 GHA 默认 step shell(无 -u/-x);bash -n 通过。
  • 环境变量继承无误。 WORKDIR/BRANCH作业级 env:,GITHUB_OUTPUT/RUNNER_TEMP 由 runner 提供,子 bash 均可继承;step 保留 id: 'verify',输出归属不变。
  • trusted 暂存顺序正确。 cp 位于切换到 PR 分支的 Prepare branch and feedback 之前,gate 逻辑仍来自 trusted base,不受被验证分支篡改。

优点: 安全不变量现由新测试固定(禁止工作树路径调用、强制 RUNNER_TEMP 形式、暂存顺序、hook 切断计数=5、committed_rc 片段实测);正文正则的 10 空格→列 0 重缩进正确。

说明(非阻断): issue-fix 作业的 gate 仍为内联且与本 runner 相似但不完全相同(git ref 不同),未引入新漂移风险;trusted 暂存脚本增至三个,一致性由新测试守护。

wenshao
wenshao previously approved these changes Jul 24, 2026

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

— qwen3.7-max via Qwen Code /review

wenshao
wenshao previously approved these changes Jul 24, 2026
@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code attempted to resolve merge conflicts but the run did not complete successfully.

address-summary.md

Merge resolution summary

Root cause

PR #7642 (ci(autofix): add cross-package contract verification) landed on main after this PR branched. It added check-autofix-contracts.sh — a new script and an inline call in the review-address verification gate. This PR (#7644) had already extracted that same inline gate into .github/scripts/run-autofix-review-verification.sh. The two changes collided: main modified the inline code that the PR removed in favor of a script call.

Textual or semantic

Semantic. Both sides modified the same verification logic — main added a new check to the inline gate, and the PR extracted the entire gate into a script. The resolution is not just textual; the contracts check must appear in the extracted script at the correct position for the verification pipeline to work.

Resolved extracted script (.github/scripts/run-autofix-review-verification.sh):

bash "${RUNNER_TEMP}/check-settings-schema.sh"
git diff --name-only "origin/main...${BRANCH}" \
  | bash "${RUNNER_TEMP}/check-autofix-contracts.sh"

if git diff --quiet "origin/${BRANCH}...${BRANCH}"; then

What is load-bearing

  1. Positioning of the contracts check. It MUST appear after check-settings-schema.sh and BEFORE the outcome=noop exit path. The contracts check runs on every path (including no-op PRs), because a stale-schema PR can wrongly write no-action.md while contracts are broken. Moving it after the no-op check would skip it on legitimate no-op PRs.

  2. Staging to RUNNER_TEMP. The workflow must cp the contracts script to RUNNER_TEMP in both the issue-autofix and review-address staging steps (2 occurrences total), matching the test expectation toHaveLength(2).

  3. Test verificationGateBodies composition. The array is [verificationGateSteps[0], reviewVerificationRunner]. The reviewVerifyGate is found by searching for the body containing outcome=noop — this resolves to the extracted script content. The contracts check test asserts position within this body.

What I could not verify

  • No build or tests were run (per instructions). The test file's verificationGateSteps[1] must match the review-address verification gate step — if a main change added or removed a Verification gate step name before it, the index could shift. The auto-merged workflow does not appear to have changed the step count, but this was not verified by running the test.
  • The .qwen/skills/autofix/SKILL.md was auto-merged from main (non-conflicted). Tests asserting new SKILL.md content (Needs a maintainer's decision, Simplicity First) were auto-merged into the test file alongside and should be consistent, but this was not runtime-verified.
中文说明

根本原因

PR #7642ci(autofix): 添加跨包合约验证)在本 PR 分支后合入了 main。它在 review-address 验证门控中新增了 check-autofix-contracts.sh 脚本及其内联调用。而本 PR (#7644) 已将该内联门控提取到独立脚本 .github/scripts/run-autofix-review-verification.sh 中。两者冲突:main 修改了本 PR 已移除并替换为脚本调用的内联代码。

解决方式

保留提取方案(工作流 YAML 调用外部脚本),并将合约检查以相同位置(在 check-settings-schema.sh 之后、no-op 门控之前)纳入提取脚本。测试文件同时保留两侧的断言和变量声明。

关键约束

  1. 合约检查必须在 no-op 退出路径之前执行,否则 no-op PR 会跳过合约检查。
  2. 工作流必须在两处暂存步骤中将合约脚本复制到 RUNNER_TEMP(测试断言 toHaveLength(2))。
  3. 未运行构建或测试,verificationGateSteps[1] 的索引正确性未做运行时验证。

Check the workflow run for full logs.

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Jul 24, 2026
…w-verification-runner

# Conflicts:
#	.github/workflows/qwen-autofix.yml
#	scripts/tests/qwen-autofix-workflow.test.js

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

— qwen3.7-max via Qwen Code /review

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 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/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Code Review — PR #7644 refactor(autofix): extract review verification runner

Verdict: ✅ Merge-ready. Re-verified at head eb7df28d (after the upstream/main merge).

What it does

Extracts the ~155-line review-address deterministic verification body out of .github/workflows/qwen-autofix.yml into a reusable trusted runner .github/scripts/run-autofix-review-verification.sh. The runner is staged from the trusted base checkout and the verify step now just invokes bash "${RUNNER_TEMP}/run-autofix-review-verification.sh". Groundwork for same-run repair (Refs #7638), which needs to run the gate more than once without duplicating the block.

Correctness — verified, not assumed

Because the head includes a merge commit that resolved conflicts in both the workflow and the test file, the real risk was the extracted script silently drifting from current logic. I checked it directly:

  • Byte-identity vs latest main. Extracted the review-address inline gate body from the current main (713a083a), de-indented, and diffed against run-autofix-review-verification.sh minus its shebang/set/blank. Result: identical, 155/155 lines. The conflict resolution introduced zero drift — the extraction faithfully represents current logic.
  • Trusted staging preserved. The new cp .github/scripts/run-autofix-review-verification.sh … is co-located in the Stage trusted schema gate and agent runner step alongside the three already-proven staged scripts, and runs before Prepare branch and feedback switches the tree to the candidate branch. Gate logic stays immune to PR-branch tampering. ✔
  • Step contract intact. The verify step keeps id: 'verify' and its always() && steps.prepare.outputs.stale != 'true' condition; body is invocation-only, so GITHUB_OUTPUT writes stay attributed. ✔
  • Env inheritance. WORKDIR/BRANCH remain job-level env; GITHUB_OUTPUT/RUNNER_TEMP are runner-provided — all inherited by the child bash. set -eo pipefail correctly replicates GHA's default shell flags. ✔
  • Syntax. bash -n clean.

Conventions & tests

  • Invoked as bash "${RUNNER_TEMP}/…" (not the working-tree path), consistent with the sibling staged gates — the branch-tampering avoidance pattern is respected.
  • The test updates are strong and self-consistent: they now read the extracted script file, pin staging-before-Prepare ordering, forbid the working-tree invocation (bash .github/scripts/run-autofix-review-verification.sh), keep the hooksPath-sever count at 5 across workflow + runner, and drive the extracted committed_rc/reject_fix snippets for real. CI Test (ubuntu-latest, Node 22.x) is green, so the merged test changes pass.

Risk / performance

Behavior-preserving; no runtime path or trigger change. The verification gate isn't exercised by ordinary PR CI (those jobs skip), and the staged file lives on main post-merge, so the cp resolves exactly as the other three staged scripts do.

Minor (non-blocking) suggestion

The standalone script opens with a comment about committed_rc; a one-line header documenting the env contract it inherits from the caller (WORKDIR, BRANCH, GITHUB_OUTPUT, RUNNER_TEMP) would help anyone reading the file outside the workflow. Optional.

Nice clean extraction — LGTM.

中文说明

结论:✅ 可合并。 已在 head eb7df28d(upstream/main 合并之后)重新验证。

做了什么:将 review-address 约 155 行的确定性验证主体从 .github/workflows/qwen-autofix.yml 抽取为可复用的 trusted runner .github/scripts/run-autofix-review-verification.sh。runner 从 trusted base 暂存,验证步骤改为仅调用 bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"。为同轮修复(Refs #7638)打基础。

正确性(已实测,非假设):由于 head 含一个在 workflow 与测试文件两处都解决了冲突的 merge commit,主要风险是抽取脚本与当前逻辑悄悄出现分歧。直接验证:

  • 与最新 main 逐字节比对:从当前 main(713a083a)抽取 review-address inline gate 主体、去缩进,与 runner(去掉 shebang/set/空行)做 diff → 完全一致,155/155 行,冲突解决零漂移。
  • trusted 暂存顺序保留:新增的 cp … run-autofix-review-verification.sh 与另外三个已验证脚本同处 Stage trusted schema gate and agent runner 步骤,且在 Prepare branch and feedback 切换到候选分支之前执行。✔
  • 步骤契约不变:保留 id: 'verify'always() && steps.prepare.outputs.stale != 'true' 条件;主体仅为调用。✔
  • 环境继承WORKDIR/BRANCH 仍为 job 级 env;GITHUB_OUTPUT/RUNNER_TEMP 由 runner 提供,子 bash 继承;set -eo pipefail 正确复刻 GHA 默认 shell 标志。✔
  • bash -n 通过。

约定与测试:以 bash "${RUNNER_TEMP}/…" 调用(非工作树路径),符合防分支篡改约定。测试更新健壮且自洽(读取抽取脚本文件、固定暂存顺序、禁止工作树调用、hooksPath 剪断计数在 workflow + runner 上仍为 5、真实驱动 committed_rc/reject_fix 片段),CI Test (ubuntu-latest, Node 22.x) 绿

风险:行为等价,无运行路径/触发变更。合并后脚本位于 main,cp 与其余三个暂存脚本一致解析。

小建议(非阻塞):脚本开头可加一行注释,说明其从调用方继承的 env 契约(WORKDIRBRANCHGITHUB_OUTPUTRUNNER_TEMP),方便脱离 workflow 阅读该文件。可选。

干净利落的抽取,LGTM。

wenshao
wenshao previously approved these changes Jul 24, 2026
@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments above for the result.

Qwen Triage 已完成 —— 查看运行。结果见上方各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

Thanks to @wenshao for the two thorough merge-ready reviews, and to the automated reviewer.

Feedback points and decisions

  1. Automated reviewer (qwen-code-ci-bot), two reviews — "Downgraded from Approve to Comment: CI still running. Reviewed."

    • Decision: No action. These are approvals downgraded only because CI was still in flight at review time; they carry no actionable finding.
  2. @wenshao review pre-release: fix ci #1 — ✅ Merge-ready, with non-blocking observations.

    • Decision: No action. The notes (the issue-fix job's gate staying inline; the trusted-staging surface growing to three scripts) are observations explicitly flagged as non-blocking, not change requests. Both are already guarded by the existing tests.
  3. @wenshao review Where is the config saved? #2 — ✅ Merge-ready, with one minor (non-blocking) suggestion:

    The standalone script opens with a comment about committed_rc; a one-line header documenting the env contract it inherits from the caller (WORKDIR, BRANCH, GITHUB_OUTPUT, RUNNER_TEMP) would help anyone reading the file outside the workflow.

    • Decision: Implemented. Added a brief header to run-autofix-review-verification.sh, right after set -eo pipefail, documenting that the script is invoked as a child bash from the review-address verify step and inherits everything from the caller's environment — WORKDIR and BRANCH are job-level env, GITHUB_OUTPUT and RUNNER_TEMP are runner-provided, and none is defined in the script. This is a genuine hidden-constraint comment (the variables are used but never declared locally), so it fits the project's "comment only the non-obvious why" rule.

Changes

  • .github/scripts/run-autofix-review-verification.sh: +4 lines (a 3-line env-contract header comment plus a separating blank line). Comment-only; no behavior change. bash -n stays clean.

Conflict notes

  • --conflict false: no merge performed; none needed.

Notes

  • The header sits before the existing committed_rc comment block and does not affect any test: every assertion on this script is a substring/toContain check or a block-anchored regex (committed_rc=0…, git config core.hooksPath…, reject_fix() {…, GATE_LOG=…); none anchors to the file start or asserts byte-identity. The hooksPath-sever count across workflow + runner stays at 5.

Verification

  • bash -n .github/scripts/run-autofix-review-verification.sh — passed (syntax clean)
  • npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow package-scripts — 116 passed (2 files)
  • npm run lint — passed
  • npm run typecheck — passed
  • npm run build — passed

No settings source changed, so npm run generate:settings-schema was not needed. The change is a comment in a workflow helper script, not exercised through the bundled CLI, so no post-bundle integration run was needed.

中文说明

已处理的评审反馈

感谢 @wenshao 两次详尽的「可合并」评审,以及自动评审机器人。

反馈点与处理决定

  1. 自动评审机器人(qwen-code-ci-bot)两条评审 ——「⚠️ 因 CI 仍在运行,由 Approve 降级为 Comment。已评审。」

    • 决定:无需处理。 这两条仅是因为评审时 CI 仍在运行而被降级的批准,不含任何可执行的发现。
  2. @wenshao 评审 pre-release: fix ci #1 —— ✅ 可合并,附非阻断性观察。

    • 决定:无需处理。 其中的说明(issue-fix 作业的 gate 仍为内联;trusted 暂存脚本增至三个)均被明确标注为非阻断性的观察,而非修改请求,且二者都已由现有测试守护。
  3. @wenshao 评审 Where is the config saved? #2 —— ✅ 可合并,附一条 minor(非阻断)建议:

    独立脚本以一段关于 committed_rc 的注释开头;可加一行说明其从调用方继承的 env 契约(WORKDIRBRANCHGITHUB_OUTPUTRUNNER_TEMP)的头部注释,方便脱离 workflow 阅读该文件。

    • 决定:已实现。run-autofix-review-verification.sh 中、紧跟 set -eo pipefail 之后加了一段简短头部注释,说明该脚本以子 bash 形式从 review-address 的 verify 步骤调用,并继承调用方环境中的全部变量 —— WORKDIRBRANCH 为作业级 env,GITHUB_OUTPUTRUNNER_TEMP 由 runner 提供,脚本内部均未定义。这是一条真正的「隐藏约束」注释(这些变量被使用却从未在本地声明),符合项目「仅对不显然的 why 加注释」的规则。

改动

  • .github/scripts/run-autofix-review-verification.sh:+4 行(一段 3 行的 env 契约头部注释,外加一行分隔空行)。纯注释,无行为变化。bash -n 仍通过。

冲突说明

  • --conflict false:未执行合并,也无需合并。

备注

  • 该头部注释位于既有 committed_rc 注释块之前,不影响任何测试:对该脚本的所有断言都是子串/toContain 检查或块锚定正则(committed_rc=0…git config core.hooksPath…reject_fix() {…GATE_LOG=…),没有任何断言锚定到文件开头或断言逐字节一致。workflow + runnerhooksPath 的剪断计数仍为 5。

验证

  • bash -n .github/scripts/run-autofix-review-verification.sh — 通过(语法干净)
  • npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow package-scripts — 116 通过(2 个文件)
  • npm run lint — 通过
  • npm run typecheck — 通过
  • npm run build — 通过

未改动任何 settings 源,因此无需运行 npm run generate:settings-schema。该改动是 workflow 辅助脚本中的一条注释,不通过打包后的 CLI 行使,因此无需在 bundle 之后运行集成测试。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

No action required

The only review on this PR is a COMMENTED status from the automated reviewer (qwen-code-ci-bot), downgraded from Approve to Comment solely because CI was still running at the time of review. The review contains no defects, suggestions, or change requests.

There are no inline comments, no issue-level comments, no failed checks, and no still-red checks to address.

No code changes are needed this round.

中文说明

无需操作

本 PR 上唯一的评审是来自自动评审器(qwen-code-ci-bot)的 COMMENTED 状态,仅因评审时 CI 仍在运行而从 Approve 降级为 Comment。该评审不包含任何缺陷、建议或变更请求。

没有行内评论、没有 issue 级别的评论、没有失败的检查、也没有持续未通过的检查需要处理。

本轮无需进行代码变更。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean behavior-preserving refactor — verification gate inline script extracted to standalone runner, faithful line-by-line extraction, correct security isolation (script staged from trusted base checkout, not from PR branch). Tests adapted correctly (regex de-indent, verificationGateBodies array covers both paths).

One thing to confirm: the staging step rename from 'Stage trusted schema gate' to 'Stage trusted schema gate and agent runner' — is this already in the base branch? If not, the test indexOf will return -1 and fail.

— qwen3.8-max-preview via Qwen Code /review

@wenshao
wenshao added this pull request to the merge queue Jul 24, 2026
Merged via the queue into QwenLM:main with commit b1ce0c2 Jul 24, 2026
61 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.1.

@yiliang114

yiliang114 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

PR #7644 已合并(2026-07-24)并已随 v0.21.1 发布。新评论只是 CI bot 的发版通知,无需代码改动或回复。


✅ completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants