feat(review): check cache identity when reviewing workflow PRs

[wenshao]

What this PR does

Adds one review class and one verification method for cache and reuse mechanisms whose two sides never agree on identity. The /review workflow path-rule gains a blocker entry, so every dimension agent whose territory holds a .github/workflows/*.yml file is asked whether the side that writes a cache and the side that reads it can actually match. The verify-pr skill gains a matching artifact-type entry describing how to settle that question, plus an environment-contract note that the verify container is itself a live sample of the lane's runtime.

Why it's needed

actions/cache matches an entry on (key | restore-key) and a version that hashes the literal path strings plus the compression method — not the key alone. Two jobs can therefore share a key, share the path: line exactly as written, and never hit once: ${{ runner.temp }} expands to a different string on ubuntu-latest than in a container job or on a self-hosted runner, and an image without the zstd binary picks gzip where a hosted runner picks zstd.

Nothing in the review asks that question today. The workflow checklist covered a cache a fork can poison, but not one that can never fire, and no assertion about the YAML's shape can answer it — the two sides' key: and path: strings match in exactly the case that fails, so shape-parity tests stay green forever. A mechanism that can never fire is not a slow optimisation; it is a no-op carrying maintenance cost.

The verify lane also has an advantage that was never written down: its agent runs inside the same node:22-bookworm container the lane itself uses. When the diff changes what those lanes execute, command -v zstd or echo "$RUNNER_TEMP" settles in one shell command what no amount of YAML reading settles — and it needs no GitHub token, which that environment does not have. Two facts that recur are recorded with it: $RUNNER_TEMP is /__w/_temp inside the container while the ${{ runner.temp }} expression evaluates to the runner's host path, and that image ships no zstd binary.

Reviewer Test Plan

How to verify

1. From packages/cli: npx vitest run src/commands/review/lib/path-rules.test.ts — 14 passed, including the new asks whether a cache mechanism can fire at all case.
2. Vacuity check, so the new case is known to hold down the new text rather than passing on something else: sed -i '/never agree on identity/d' packages/cli/src/commands/review/lib/path-rules.ts, re-run step 1, and confirm exactly one test fails (Tests 1 failed | 13 passed (14)). Restore the file with git checkout afterwards.
3. From the repo root: npx vitest run scripts/tests/qwen-triage-workflow.test.js — 107 passed. This suite reads .qwen/skills/verify-pr/SKILL.md, so it is the gate on the skill edit.
4. From the repo root: node --test .github/scripts/qwen-triage-workflow.test.mjs — 31 passed.
5. npx prettier --check and npx eslint on the three touched files — clean.

Observed: all of the above pass, and step 2 fails exactly one test as described.

Evidence (Before & After)

N/A — no user-visible or TUI change. The change alters the text handed to review agents and the verify skill's method, not any rendered output.

Tested on

OS	Status
🍏 macOS	N/A
🪟 Windows	N/A
🐧 Linux	✅

Environment (optional)

Node 22, unit tests only.

Risk & Scope

• Main risk or tradeoff: the workflow checklist gets longer, and a rule that fires on every review is a rule that gets skimmed. This is bounded by the existing scoping — pathRulesFor attaches the checklist only to agents whose territory actually contains a workflow file — and the new entry names a concrete comparison to run rather than a thing to be alert to.
• Not validated / out of scope: Agent 4's performance dimension still stops at "unverified" for a claim it cannot recompute locally. Teaching it that a key match across two environments is decidable without recomputation would be a separate change to agent-briefs.ts. Also out of scope: the built dist/ copy of the review skill, which picks this up on the next normal build.
• Breaking changes / migration notes: none.

Linked Issues

N/A

中文说明

这个 PR 做了什么

为"写入端与读取端永远对不上身份"的缓存/复用机制补上一条审查类别和一套验证方法。/review 的 workflow path-rule 新增一条 blocker，凡是 territory 中含 .github/workflows/*.yml 的 dimension agent 都会被要求追问：写缓存的一端和读缓存的一端到底能不能匹配上。verify-pr skill 新增一条对应的 artifact-type 条目说明如何判定这个问题，并在 environment contract 中记录：verify 容器本身就是该 lane 运行时的活样本。

为什么需要

actions/cache 的匹配条件是 (key | restore-key) 加上 一个 version，而这个 version 是对 path 字面量加压缩方式求的哈希，不只是 key。因此两个 job 可以共用同一个 key、共用逐字相同的 path: 行，却一次都命中不了：${{ runner.temp }} 在 ubuntu-latest 上、在容器 job 中、在 self-hosted runner 上会展开成不同的字符串；没有 zstd 二进制的镜像会选 gzip，而 hosted runner 选 zstd。

目前的审查完全不会问这个问题。workflow checklist 覆盖了 fork 可以毒化的缓存，但没覆盖永远无法生效的缓存；而任何关于 YAML 形状的断言都答不了它——恰恰在会失败的那种情形下，两端的 key: 和 path: 字符串完全一致，于是形状断言永远是绿的。一个永远不会触发的机制不是"慢一点的优化"，而是带着维护成本的 no-op。

verify lane 还有一个从未被写下来的优势：它的 agent 就跑在该 lane 自己使用的同一个 node:22-bookworm 容器里。当 diff 改的正是这些 lane 所执行的东西时，command -v zstd 或 echo "$RUNNER_TEMP" 一条 shell 命令就能定案，而这是再多的 YAML 阅读都定不了的；并且它不需要 GitHub token——而那个环境恰恰没有 token。随之记录了两条反复用到的事实：容器内 $RUNNER_TEMP 是 /__w/_temp，而 ${{ runner.temp }} 表达式求值得到的是 runner 的宿主机路径；以及该镜像不带 zstd 二进制。

审查者测试计划

如何验证

1. 在 packages/cli 目录下：npx vitest run src/commands/review/lib/path-rules.test.ts —— 14 条通过，含新增的 asks whether a cache mechanism can fire at all。
2. Vacuity check，用于确认新用例钉住的是新文本而不是搭了别处的便车：执行 sed -i '/never agree on identity/d' packages/cli/src/commands/review/lib/path-rules.ts，重跑第 1 步，确认恰好有一条失败（Tests 1 failed | 13 passed (14)）。之后用 git checkout 还原文件。
3. 在仓库根目录：npx vitest run scripts/tests/qwen-triage-workflow.test.js —— 107 条通过。该套件会读取 .qwen/skills/verify-pr/SKILL.md，是 skill 改动的把关测试。
4. 在仓库根目录：node --test .github/scripts/qwen-triage-workflow.test.mjs —— 31 条通过。
5. 对三个改动文件执行 npx prettier --check 与 npx eslint —— 均干净。

实测：以上全部通过，且第 2 步如描述那样恰好失败一条。

证据（Before & After）

N/A —— 无用户可见或 TUI 变化。本次改动改变的是交给 review agent 的文本与 verify skill 的方法，不涉及任何渲染输出。

测试平台

OS	状态
🍏 macOS	N/A
🪟 Windows	N/A
🐧 Linux	✅

环境（可选）

Node 22，仅单元测试。

风险与范围

• 主要风险或取舍：workflow checklist 变长了，而一条在每次审查中都触发的规则就是一条会被略读的规则。这一点受现有的作用域机制约束——pathRulesFor 只会把 checklist 附加给 territory 中确实含有 workflow 文件的 agent——并且新条目给出的是一个可执行的具体比对动作，而不是一句"要留意"。
• 未验证 / 范围外：Agent 4 的性能维度对于本地无法复算的主张仍然停在 "unverified"。要让它明白"跨两个环境的键匹配"无需复算即可判定，属于 agent-briefs.ts 的另一处改动。同样范围外的还有 review skill 的 dist/ 构建产物，它会在下一次正常构建时取得本次改动。
• 破坏性变更 / 迁移说明：无。

关联 Issue

N/A

[qwen-code-ci-bot]

✅ Qwen Triage finished — CI landed green on a4951f5 and the deferred approval was posted. finalize run

✅ Qwen Triage 已完成 —— a4951f5 的 CI 全绿，延迟审批已提交。查看 finalize 运行

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓ — all required sections present, bilingual body included.

Problem: this is a coverage gap, not a runtime bug, and it's a real one. The workflow checklist already covers a cache a fork can poison, but nothing asks whether a cache can ever fire. The mechanism cited is correct: actions/cache identifies an entry by (key | restore-key) plus a version derived from the cache paths and the compression method, so two jobs can share a key and a path: line and still never match — ${{ runner.temp }} expands to a different string on a hosted runner than in a container, and an image without zstd picks gzip where the hosted runner picks zstd. The worked example in the description (matching keys, matching path: lines, 152 green shape assertions, zero possible hits) is exactly the case that slips through.

Direction: aligned. This is squarely the review tooling's job — the GITHUB_ACTIONS path rule exists precisely to name defect classes the dimension agents can't see from YAML, and a cache that can never hit is one of them. Scoped correctly too: pathRulesFor only hands the checklist to agents whose territory actually contains a workflow file, so this doesn't fire on every review.

Size: not applicable — packages/cli/src/commands/review/lib/ is not a protected core path. 1 production line (the checklist bullet), 17 test lines, 33 lines of verify-pr skill docs.

Approach: the scope feels right and minimal — one checklist bullet, one test pinning it, and a matching method note in the verify-pr skill. I considered whether the skill edit could be dropped, but it earns its place: the review rule names the question, and the skill records how to settle it (plus the environment-contract note that the verify container is itself a live sample of the lane runtime). No simpler path that keeps both halves working.

Risk: no elevated risk signals — none of the changed files match the high-revert paths.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓ —— 必填章节齐全，含中文正文。

问题：这是一个覆盖盲区，而非运行时 bug，而且是真实存在的。workflow checklist 已经覆盖了 fork 可以毒化的缓存，但没有任何一条问"缓存到底能不能生效"。所引用的机制是正确的：actions/cache 通过 (key | restore-key) 加上一个由缓存路径与压缩方式派生的 version 来标识条目，因此两个 job 可以共用 key、共用 path: 行，却永远匹配不上——${{ runner.temp }} 在 hosted runner 与容器里展开成不同的字符串，没有 zstd 的镜像会选 gzip 而 hosted runner 选 zstd。描述中的实例（key 相同、path: 行相同、152 条形状断言全绿、却一次都不可能命中）恰恰是会漏掉的那种情形。

方向：对齐。这正是 review 工具的职责——GITHUB_ACTIONS path rule 的存在就是为了点名那些 dimension agent 从 YAML 看不到的缺陷类别，而"永远不会命中的缓存"就是其中之一。作用域也正确：pathRulesFor 只会把 checklist 交给 territory 中确实含有 workflow 文件的 agent，所以它不会在每次审查中都触发。

规模：不适用 —— packages/cli/src/commands/review/lib/ 不是受保护的核心路径。1 行生产代码（checklist 条目）、17 行测试、33 行 verify-pr skill 文档。

方案：范围合理且最小化——一条 checklist 条目、一条钉住它的测试、以及 verify-pr skill 中对应的方法说明。我考虑过能否砍掉 skill 改动，但它有其价值：review rule 提出问题，skill 记录如何定案（外加 environment-contract 说明：verify 容器本身就是该 lane 运行时的活样本）。没有更简的路径能同时保住这两半。

风险：无升级风险信号 —— 改动文件均未命中高 revert 路径。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-preview

_{Reviewed at a4951f5446c4bfaa1185481297f0b76a00bda0ee · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

Read the title and "Why it's needed" first and proposed my own version before looking at the diff: I'd add a single blocker bullet to the GITHUB_ACTIONS rule in path-rules.ts telling reviewers to compare the environments of a cache's write and read sides (runs-on, container, path expansion, compression method) rather than the YAML key:/path: strings, plus a test pinning the text. The PR matches that almost exactly, and goes one step further by recording the settlement method in the verify-pr skill — a reasonable addition, not scope creep.

The production change is one markdown bullet appended to the GITHUB_ACTIONS checklist (path-rules.ts), and it fits the file's deliberate style — every existing bullet names a concrete defect class plus the concrete trigger/outcome, and this one does the same. I checked the four test assertions against the actual bullet text: never agree on identity (the bullet header), Compare the **environments**, runs-on, and $GITHUB_STEP_SUMMARY are all present, and pathRulesFor(['.github/workflows/x.yml']) does route through this rule, so the test pins the new text rather than passing on something adjacent. The PR's described vacuity check (delete the line → exactly one test fails) is the right way to prove that, and CI's own run of path-rules.test.ts is the gate.

The verify-pr/SKILL.md additions are documentation: a "can the mechanism fire at all" method for cache/reuse PRs, and an environment-contract note that the verify container is a live sample of the lane runtime. Both are accurate and consistent with the review rule. No correctness, security, or convention concerns anywhere in the diff; nothing to extract or deduplicate.

Testing evidence (this PR's own CI — I do not run PR code on the triage path)

The Linux unit suite is still running on the reviewed commit; nothing has failed. precheck-pr passed. macOS/Windows tests and the integration suite are skipped for this commit (docs + review-prompt change, no platform-specific surface). The Qwen Triage Finalize job will rewrite the table below in place once CI settles.

Final CI results for a4951f5 (auto-updated by the triage finalize job after CI completed):

Check	Conclusion
Classify PR	✅ success
Test (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success

_{One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。}

Not verified: the runtime behaviour of any product surface — there is none here. The change is review-agent prompt text plus skill documentation, and the unit test above pins the text; there is no behavioural claim for a sandboxed /verify or /tmux lane to settle, so none is named.

中文说明

代码审查

先只看标题与"Why it's needed"，在看 diff 之前提出我自己的方案：我会在 path-rules.ts 的 GITHUB_ACTIONS rule 中加一条 blocker，让 reviewer 去比对缓存写入端与读取端的环境（runs-on、container、路径展开、压缩方式），而不是 YAML 的 key:/path: 字符串，并加一条测试钉住这段文本。PR 与这个方案几乎完全一致，并更进一步把判定方法记录进 verify-pr skill——这是合理的补充，不是范围蔓延。

生产改动是追加到 GITHUB_ACTIONS checklist 的一条 markdown 条目（path-rules.ts），契合该文件刻意的风格——现有每一条都点名一个具体缺陷类别加具体触发条件/结果，这一条也是如此。我把四条测试断言逐一对照了实际条目文本：never agree on identity（条目标题）、Compare the **environments**、runs-on、$GITHUB_STEP_SUMMARY 均存在，且 pathRulesFor(['.github/workflows/x.yml']) 确实会走到这条 rule，因此测试钉住的是新文本，而非搭了别处的便车。PR 所描述的 vacuity check（删掉该行 → 恰好一条失败）正是证明这一点的正确方式，而 CI 对 path-rules.test.ts 的运行就是把关。

verify-pr/SKILL.md 的改动是文档：一条针对缓存/复用 PR 的"机制到底能不能生效"判定方法，以及一条 environment-contract 说明（verify 容器是该 lane 运行时的活样本）。两者都准确，且与 review rule 一致。整个 diff 无正确性、安全性或规范问题；无可抽取或重复之处。

测试证据（本 PR 自己的 CI —— triage 路径不运行 PR 代码）

Linux 单元测试套件在被审 commit 上仍在运行；目前无任何失败。precheck-pr 通过。macOS/Windows 测试与集成套件对本 commit 为 skipped（文档 + review 提示词改动，无平台相关面）。Qwen Triage Finalize 任务会在 CI 结束后就地改写下方表格。

未验证：任何产品面的运行时行为——本次没有。改动是 review-agent 提示词加 skill 文档，上面的单元测试已钉住文本；没有需要 /verify 或 /tmux 沙箱 lane 来定案的行为性主张，因此不点名任何 lane。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at a4951f5446c4bfaa1185481297f0b76a00bda0ee · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 5/5 — a real, demonstrated gap closed by one idiomatic bullet and a test that pins it; nothing to second-guess.

Stepping back: the problem is genuine and the PR proves it rather than asserting it — a cache whose two sides share a key and a path: line yet can never match is a real actions/cache failure mode (the version folds in the paths and the compression method), and the review genuinely didn't ask about it. My independent proposal was essentially what shipped, minus the skill note; the skill note earns its keep, so the PR exceeds my baseline rather than missing a simpler path. The diff is the minimal change for the goal — no drive-by edits, no churn — and it reads like the rest of the deliberately thorough GITHUB_ACTIONS rule. In six months this is a thank-you, not a curse: it names a concrete comparison to run, not a thing to be vaguely alert to.

I'm approving on the merits. CI is still running on the reviewed commit (the Linux unit suite is in flight, nothing red), so the approval is deferred rather than posted now — approval lands green on a4951f5446c4bfaa1185481297f0b76a00bda0ee once the suite passes, pinned to that commit by the finalize job.

中文说明

置信度：5/5 —— 一个真实且有实证支撑的盲区，由一条符合惯例的条目加一条钉住它的测试关闭；无可质疑之处。

退一步看：问题是真实的，且 PR 证明了它而非仅仅断言——一个两端共用 key 与 path: 行却永远无法匹配的缓存，是 actions/cache 的真实失效模式（version 会把路径与压缩方式折进去），而审查此前确实不会问这个。我的独立方案与最终落地的基本一致，只是少了 skill 说明；那条 skill 说明有其价值，因此 PR 超出了我的基线，而非漏掉了更简路径。diff 是达成目标的最小改动——无顺手改动、无 churn——读起来与该 GITHUB_ACTIONS rule 一贯的"刻意详尽"风格一致。六个月后这是一声感谢，而非一句咒骂：它点名了一个可执行的具体比对，而非一句含糊的"要留意"。

我基于实质内容认可本 PR。CI 仍在被审 commit 上运行（Linux 单元测试套件进行中，无红），因此认可推迟发出而非现在提交——待套件通过后，针对 a4951f5446c4bfaa1185481297f0b76a00bda0ee 转绿落地，由 finalize 任务钉住该 commit。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at a4951f5446c4bfaa1185481297f0b76a00bda0ee · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship — CI landed green after the review. ✅

[qwen-code-ci-bot]

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

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

[yiliang114]

LGTM.

[yiliang114]

LGTM — well-motivated addition based on a real dogfood finding. The cache identity check fills a real gap: YAML-shape parity tests pass when neither side can reach the other's cache. The verify-pr environment contract note is practical (one shell command settles what YAML reading can't). Test follows existing content-presence pattern with vacuity check documented.

[qwen-code-ci-bot]

Released in v0.21.3.