feat(triage): make /verify evidence screenshots actually possible - #8016
Conversation
Measured across the 14 real verify reports on this repo: tables appear in 14/14 (4 to 31 rows), images in 0/14. The hosting machinery has been complete the whole time — pr-assets branch, PNG magic check, 8 images, 2 MB cap, sanitised names — and it has never once been fed. Two independent causes, both fixed: 1. The agent physically could not produce a PNG. It runs as `node` under `env -i` with HOME pointed at a per-run scratch dir, so `playwright install --with-deps chromium` needs an apt it has no rights to and would re-download ~170 MB into a directory deleted at the end of every run. Chromium is now installed by ROOT in the tools step into a shared, world-readable PLAYWRIGHT_BROWSERS_PATH, and the agent env carries that path plus QWEN_VERIFY_CHROMIUM=1 — but ONLY when the install actually succeeded, so the variable's absence is a real signal rather than a stale promise. The install is best-effort: a failure warns and the run continues to a text-only report, which is what every run has produced anyway. 2. The skill discouraged it. Captures were "Optionally … use them when text cannot carry the oracle", with TUI rendering as the only example — so an agent verifying a daemon or a CLI correctly concluded text carried the oracle and skipped. It now says produce them whenever a harness ran, and says WHY: a table is the agent's claim about what happened; a capture of the run is a witness that the numbers came from a real execution, which is the part a reviewer cannot get any other way. It names the three highest-value shots (A/B cells side by side, the mutation matrix as printed, raw harness output behind a headline number) and forbids `playwright install` explicitly, so the failure mode of cause 1 cannot be rediscovered by an agent burning budget on it. Affordable now because #8014 raised the agent budget to 120m; at 20m a browser download alone was most of the run. Mutation-verified 4/4: handing the agent the variable unconditionally, failing the job on a chromium install failure, reverting the skill to "Optionally", and dropping the do-not-install warning each turn the test red. 90/90 tests; prettier, eslint, actionlint and shellcheck clean.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
@qwen-code /takeover |
|
Thanks for the PR! Template looks good ✓ Problem: Observed gap with hard data — 14/14 verify reports carry tables, 0/14 carry images. The hosting machinery (pr-assets branch, PNG magic check, size caps) has been complete since the lane shipped but never fed. Two independent causes are identified: the agent physically cannot install chromium (runs as Direction: Aligned. This closes the gap between the verify lane's output and the maintainer-written rounds it models. Internal tooling, no public contract impact. Size: No core module paths touched. 486 additions / 45 deletions across 5 files: ~162 production lines (workflow YAML + skill docs), ~332 test lines. Well within bounds. Approach: Scope feels right — each change serves the stated goal. The two-phase install (system deps as root, binary as node via the correct Playwright CLI), marker-gated env vars, per-PR asset branches, and cleanup workflow update are all necessary pieces. No drive-by changes or scope creep. Risk: No elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 有硬数据支撑的已观测缺口——14/14 份验证报告有表格,0/14 有图片。托管机制自车道上线起就是完整的,但一次都没被喂过数据。两个独立原因:agent 物理上无法安装 chromium(以 方向: 对齐。弥合验证车道产出与维护者手写轮次之间的差距。内部工具,无公共契约影响。 规模: 未触及核心模块路径。5 个文件,486 行新增 / 45 行删除:约 162 行生产代码(workflow YAML + skill 文档),约 332 行测试代码。 方案: 范围合理——每项改动都服务于既定目标。两阶段安装(root 装系统依赖、node 用正确的 Playwright CLI 装二进制)、标记文件门控环境变量、按 PR 分支托管资产、清理工作流更新,都是必要的组成部分。无顺手改动或范围蔓延。 风险: 无升级风险信号——未匹配高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
Code ReviewIndependent proposal: I would pre-install chromium as root in the runner setup, pass the browser path to the agent via env vars gated on install success, update the skill to require captures, and make the whole thing best-effort so failures degrade to today's text-only behavior. The PR matches and exceeds this. The two-phase install is smarter than a single step: system deps (apt, root-only) and the browser binary (Playwright CDN, node-readable) are independent failure domains, so separate markers for each prevent a binary-only success from promising a browser that dies on a missing Findings: No critical blockers. No convention violations. The security posture is sound: the browser step strips The cleanup workflow change ( Test coverage is thorough — 332 new lines pinning the wiring (marker gating, positional assertions, credential stripping, skill wording), the per-PR branch lifecycle (orphan init, push, cleanup), and the cleanup script's failure modes (executed against a stubbed Not verified: whether chromium actually installs and renders on the ECS runners — the author says so honestly, and the best-effort design makes this safe (failure degrades to today's text-only report). The first live verify run is the acceptance check. No sandboxed lane ( CI Test EvidenceFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The ubuntu unit suite is still running. No failures so far. Approval is deferred until CI lands green (marker below). 中文说明代码审查独立方案: 我会以 root 身份在 runner 初始化时预装 chromium,通过环境变量把浏览器路径传给 agent(以安装成功为门控),更新 skill 要求截图,并让整个流程尽力而为——失败时退化为当前的纯文本行为。 本 PR 与我的方案一致且更优。两阶段安装比单步更聪明:系统依赖(apt,仅 root)和浏览器二进制(Playwright CDN,node 可读)是独立的故障域,分别用标记文件防止"仅二进制成功"就承诺一个缺 发现: 无关键阻塞项。无规范违反。 安全姿态可靠:浏览器步骤在运行 PR 解析的代码前剥离了 清理工作流的改动正确:一个分支缺失是正常情况,不能阻止另一个被删除,但真正的删除失败仍会以 warning 和非零退出码呈现。 测试覆盖全面——332 行新测试钉住接线、按 PR 分支生命周期和清理脚本的故障模式。 未验证: chromium 在 ECS runner 上是否真能安装并渲染——作者诚实地说明了这一点,尽力而为的设计使其安全。首次真实验证运行即验收标准。无沙箱车道适用:改动针对的是验证基础设施本身,而非产品行为。 CI 测试证据ubuntu 单元测试仍在运行中。目前无失败。批准推迟至 CI 全绿。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean review, sound design, thorough tests; the only reservation is the inherent gap that the first live run is the real acceptance test for chromium on ECS, but the best-effort design makes that safe. This PR does one thing and does it well: it closes the gap between the verify lane's output and the maintainer-written rounds it models, where captures of the live run are the norm. The 0-for-14 image record is the clearest possible evidence that the problem is real, and the fix addresses both independent causes without overreaching. The two-phase install with separate markers is the right level of care for a persistent-runner environment, and the per-PR branch naming avoids a git ref conflict that would have bitten on the first run. If I had to maintain this in six months, the comments and tests would tell me exactly what each piece does and why — I'd thank the author. The skill wording change from "Optionally" to "Produce these whenever you ran a harness" is the higher-leverage half of the fix: even with chromium available, the old wording would have kept the agent from using it. Naming the three highest-value shots and forbidding Approval deferred until CI lands green on 中文说明置信度:4/5 —— 审查干净、设计合理、测试全面;唯一的保留是首次真实运行才是 chromium 在 ECS 上的真正验收测试,但尽力而为的设计使其安全。 本 PR 只做一件事且做得好:弥合验证车道产出与维护者手写轮次之间的差距。0/14 的图片记录是问题真实存在的最清晰证据,修复方案解决了两个独立原因且没有过度扩展。两阶段安装配合独立标记,对常驻 runner 环境是正确的细致程度;按 PR 分支命名避免了首次运行就会遇到的 git ref 冲突。 skill 措辞从 "Optionally" 改为 "Produce these whenever you ran a harness" 是修复中杠杆更高的半部分:即使 chromium 可用,旧措辞也会阻止 agent 使用它。点名三种最高价值截图并禁止 批准推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
🔀 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 将重新运行。 |
Local real-execution verification roundBuilt and ran this PR locally on Linux (Debian, Headline: the wiring is correct and the degradation is safe, but the capability still cannot produce a single image. Two independent blockers remain, neither addressed here, and the second one means the PR's own acceptance criterion — images-per-report going from 0 to something small — cannot be met even after the first is fixed. Both look like small changes.
🔴 1. The pinned browser build does not match the Playwright the agent runsThe tools step pins
Run for real, both cells identical except the pin — same unprivileged user, same Three things make this worse than a plain miss:
I checked the other route too, in case the agent ignores the checkout and reaches for For the record: Suggested fix. The tools step deliberately runs before checkout, so it cannot read the lockfile. Rather than hardcoding runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
npx playwright install chromium # resolves node_modules/.bin/playwright → exact lockfile version
🔴 2. The branch the publish job hosts on cannot exist
Server-side rejection, reproduced locally: And the publish job's exact clone, run read-only against the real remote: That lands on the guard three lines later and returns The comment above the function says this is "the same convention hand-run verification rounds use" — but hand-run rounds use Net effect: with finding 1 fixed, an agent that produces PNGs still yields a text-only report — just with a different warning. This is the last link in the chain, and it is the one that decides the acceptance number. Suggested fix. Either host per-PR under the existing convention ( 🟠 3. The route the skill names re-teaches the forbidden command
Line 230's trigger is exactly the failure finding 1 produces. So an agent that hits the version mismatch is routed by the named skill straight into the command the calling skill forbids — reintroducing cause 1 by reference. Worth a matching edit to 🟡 4. Test blind spotsI re-ran the PR's four mutations independently — all four KILLED, claim confirmed. I then added three the test does not catch (matrix in the screenshot above):
M1b and M6 are the same stale-promise bug the test is written to prevent: the positional check compares To be clear: the shipped guard is correct (verified by execution below). The gap is that the test would not notice if it stopped being. What checks out ✅
Not covered
Verdict
中文说明本地真实执行验证轮次在本机 Linux(Debian, 结论:接线是对的、降级是安全的,但这个能力目前一张图也产不出来。 存在两个本 PR 未处理的独立阻塞点,其中第二个意味着即使修好第一个,PR 自己设定的验收标准(每份报告的图片数从 0 变成一个少而有目的的值)依然无法达成。两处看起来都是小改动。
🔴 1. pin 的浏览器版本与 agent 实际运行的 Playwright 不匹配tools 步骤 pin 的是
真实执行的 A/B,两格除 pin 外完全相同——同一个非特权用户、同样的 有三点让情况比"单纯没弄成"更糟:
我也验证了另一条路线,以防 agent 不走 checkout 而直接在全新 附带澄清:本仓库的 修复建议。 tools 步骤是有意跑在 checkout 之前的,读不到 lockfile。与其硬编码 runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
npx playwright install chromium # 解析到 node_modules/.bin/playwright → 精确的 lockfile 版本
🔴 2. 发布任务要托管的分支根本无法存在
本地复现的服务端拒绝: 以及对真实远端只读执行的、发布任务的原样 clone: 于是三行之后就撞上守卫,返回 函数上方的注释说这是*"维护者手工验证轮次使用的同一套约定"*——但手工轮次用的是 净效果:即使修好发现 1,产出了 PNG 的 agent 拿到的仍是纯文本报告——只是换了一条 warning。这是链条上的最后一环,也是决定验收数字的那一环。 修复建议。 要么按既有约定按 PR 托管( 🟠 3. skill 指定的路线又把被禁的命令教了回去
第 230 行的触发条件,正是发现 1 产生的那个报错。也就是说,撞上版本不匹配的 agent,会被它被指定使用的 skill 直接引向调用方 skill 所禁止的命令——以引用的方式把原因 1 重新引入。建议同步修改 🟡 4. 测试盲区我独立重跑了 PR 的四个变异——四个全部 KILLED,声明属实。随后我补了三个测试抓不到的:
M1b 与 M6 正是该测试意图防范的那个"过期承诺"缺陷:位置断言比较的是 需要说明:已交付的守卫代码本身是正确的(下文有执行验证)。问题在于,如果它哪天不正确了,测试不会察觉。 通过验证的部分 ✅
未覆盖
结论
🤖 Verified locally with Claude Code — model: Claude Opus 5 (1M context). Evidence images hosted on |
…ict (#8016) Split the chromium install into system deps (tools step, as root) and browser binary (new post-checkout step, as node using the checkout's own Playwright). This eliminates the version pin that produced an unusable browser tree. Change evidence hosting from a bare pr-assets leaf branch — which cannot coexist with the existing pr-assets/* namespace — to per-PR branches (pr-assets/pr<N>-verify) with orphan-init fallback. Update terminal-capture skill to warn CI agents away from the forbidden playwright install command, and close test blind spots for conditionality (M1b), version matching (M5), and marker ordering (M6).
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed🔴 1. Pinned browser build does not match the checkout's Playwright — FixedSplit the single
The marker file ( 🔴 2. The
|
doudouOUC
left a comment
There was a problem hiding this comment.
[Critical] pr-assets branch cannot exist: git clone --branch pr-assets at workflow line 3081 fails because 365 pr-assets/* branches prevent creating a leaf pr-assets. Production always takes GITHUB_REPOSITORY path (VERIFY_ASSETS_REMOTE not wired). Images cannot be hosted even if produced. (Pre-existing blocker by @wenshao, still stands at this commit.)
[Critical] terminal-capture/SKILL.md contradicts the new forbid: verify-pr/SKILL.md says 'Do not run playwright install', but .qwen/skills/terminal-capture/SKILL.md lines 20 and 230 still instruct npx playwright install chromium. When the version mismatch causes launch failure, Playwright's own error suggests this exact command, and the named skill route leads there too. (Pre-existing blocker by @wenshao, still stands at this commit.)
— qwen3.7-max via Qwen Code /review
|
🔀 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
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed (PR #8016)1. [Critical, review-level]
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
Re-verification round — the round-2 fix for blocker 1 did not landRe-checked
🔴 1.
|
| package (lockfile) | version | chromium tree |
|---|---|---|
@playwright/test — owns .bin/playwright |
1.61.1 | chromium-1228 / chromium_headless_shell-1228 |
playwright — what terminal-capture.ts:18 imports |
1.58.2 | chromium-1208 / chromium_headless_shell-1208 |
The two CLIs, dry-run, no download:
$ npx playwright install --dry-run chromium # what the PR runs
Chrome for Testing 149.0.7827.55 (playwright chromium v1228)
Install location: $PLAYWRIGHT_BROWSERS_PATH/chromium-1228
$ node node_modules/playwright/cli.js install --dry-run chromium # what the harness needs
Chrome for Testing 145.0.7632.6 (playwright chromium v1208)
Install location: $PLAYWRIGHT_BROWSERS_PATH/chromium-1208
And the launch, against a browsers path holding only what the PR installs:
$ PLAYWRIGHT_BROWSERS_PATH=<dir with chromium-1228 only> node -e "require('playwright').chromium.launch()"
browserType.launch: Executable doesn't exist at .../chromium_headless_shell-1208/chrome-headless-shell-linux64/chrome-headless-shell
╔═════════════════════════════════════════════════════════════════╗
║ Please run the following command to download new browsers: ║
║ npx playwright install ║
╚═════════════════════════════════════════════════════════════════╝
So the previous round's cell A reproduces unchanged — same LAUNCH-FAILED, same "stale promise" (QWEN_VERIFY_CHROMIUM=1 is set), same Playwright error pointing at the one command the skill forbids. Only the revision moved, 1194 → 1228. The mechanism was never the pin; it is which package's CLI runs.
Fix — call the CLI of the package the harness imports:
- if runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
- npx playwright install chromium; then
+ if runuser -u node -- env -u GITHUB_OUTPUT -u GITHUB_STATE -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY \
+ PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
+ node node_modules/playwright/cli.js install chromium; then(The -u flags are finding 2; the CLI path is this one.) The step comment needs the same correction — node_modules/.bin/playwright resolves a lockfile version, not the harness's.
Note the new test currently pins the failing command literally. Mutated the workflow to the line above and re-ran: pre-installs chromium and hands it to the agent fails (expect(browser).toContain('npx playwright install chromium')), i.e. the test as written locks in the defect. With that literal updated, plus the two edits under finding 2, the suite is 91/91.
🟠 2. The new step drops the command-file stripping — and the test boundary was moved so the guard cannot see it
Every runuser -u node in the verify lane either strips the runner's command files or runs under env -i:
- prepare:
runuser -u node -- env -u GITHUB_OUTPUT -u GITHUB_STATE -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY npm ci … - agent:
runuser -u node -- env -i "${QWEN_ENV[@]}" … - new step: neither.
strips GitHub command files from both verify lifecycle commands exists to catch exactly this — its expect(commands.length).toBe(2) is a completeness guard ("a bare step() lookup … would pass silently"). The PR moves the slice end from 'Run verification agent' to "name: 'Install evidence browser'", which exempts the new command instead of complying. Restoring the original boundary at head:
× strips GitHub command files from both verify lifecycle commands
→ expected 3 to be 2
Why it is not only hygiene: by this point npm ci has run, so npx playwright executes whatever the PR's own package.json/package-lock.json made node_modules/.bin/playwright — PR-controlled code, as node, with $GITHUB_ENV, $GITHUB_PATH, $GITHUB_OUTPUT and $GITHUB_STEP_SUMMARY all pointing at live runner command files that steer the remainder of this job. No token is exposed (the job has no job-level env:), but the prepare step still nulls GITHUB_TOKEN/GH_TOKEN explicitly — worth mirroring on a step that runs PR code.
Fix: the -u flags in the diff above, then re-extend the guard rather than narrowing it — slice end back to 'Run verification agent' and toBe(3), so all three lifecycle commands are checked. Verified: 91/91.
🟡 3. The success marker is never cleared, and RUNNER_TEMP outlives the run
verify-chromium-path is written on success and left alone on failure. On this pool RUNNER_TEMP persists between runs — the prepare step says so in its own comment (rm -rf "$RUNNER_TEMP/verify-results" … "a stale verify-results from an EARLIER PR's run would otherwise ride along"). So after one successful run, a later run whose download fails still finds the marker and still hands the agent QWEN_VERIFY_CHROMIUM=1 plus a path that may hold another PR's browser revision. That is the stale promise the PR set out to eliminate, one run removed.
One line next to the existing cleanup keeps the browser step's positional assertions intact:
rm -rf "$RUNNER_TEMP/verify-results"
+ rm -f "$RUNNER_TEMP/verify-chromium-path"
mkdir -p "$RUNNER_TEMP/verify-results"🟡 4. expect(ifBlock).toContain('if') is vacuous
The M1b conditionality check slices runStep.slice(guard - 20, marker + 30), which begins inside the marker path — the shell if keyword is not in the window, and the assertion passes on the if in verify-chromium-path:
"t \"${RUNNER_TEMP:?}/verify-chromium-path\" 2>/dev/null)\" &&\n [ -n \"$CHROMIUM_PATH\" ]; then\n QWEN_ENV+=(\n …"
contains 'if': true /(^|\s)if\s/ : false
then is carrying the whole assertion. Widening the start to ifIdx (as the browser-step test already does) makes it mean what it says.
⚪ 5. Branch name diverges from the 369 existing ones
origin has 369 pr-assets/* branches and they are pr-assets/<N>-verify — including this PR's own evidence at pr-assets/8016-verify. The publisher writes pr-assets/pr${PR_NUMBER}-verify under a comment claiming it matches hand-run convention. Cosmetic, but it splits the namespace against the one thing the comment cites. (Confirmed the URL shape is fine either way: raw.githubusercontent.com/QwenLM/qwen-code/pr-assets/6433-verify/a-plain-80col.png → 200 image/png.)
Notes, not findings
- feat(triage): raise the /verify agent budget from 25m to 120m #8014 is still open. Base
mainistimeout-minutes: 60withtimeout --kill-after=10s 25mfor the agent. If this merges first, the browser download plus the apt layer sit on the critical path ahead of a 25-minute agent budget, not the 120/150 the description assumes. - The description is a round behind the code — it still describes root-installing
--with-depsinto a shared path withchmod -R a+rX, which the test now asserts against (expect(tools).not.toContain('install --with-deps')). Worth refreshing before merge, since the body is the merge record.
What checks out ✅
- 91/91 at
314a408; the four claimed mutations plus the M1b/M6 positional guards are present. - Blocker 2 is genuinely fixed. There is no
refs/heads/pr-assetson origin and it cannot be created while 369 children exist, so the oldclone --branch pr-assetswas structurally dead — hosting had never been reachable. The orphan fallback is correct (git init+checkout --orphanon an unborn HEAD works; verified on git 2.47.3), and the identity/retry/push path is set outside the branch so both routes get it. - Splitting apt deps (root, pre-checkout) from the binary (as
node, post-npm ci) is the right shape — the node-owned tree removes thechmod -R a+rXstep entirely. Only the CLI it invokes is wrong.
Verdict
findings — fix 1 and 2 before merge. Without 1 the acceptance number stays 0 while every run reports success; 2 re-opens a guard the lane added deliberately.
中文说明
复验轮次 —— 第 2 轮针对阻塞点 1 的修复并未真正生效
针对 314a408,按真实依赖树而非 workflow 文本复验,重点是上一轮提出、autofix 标记为"已解决"的两个阻塞点。阻塞点 1 依然存在,同时新步骤引入了一个本仓库原本由测试守住、现在守不住的问题。测试套件 91/91;阻塞点 2(分支)确实修好了。
🔴 1. npx playwright install chromium 解析到的并不是 checkout 的 Playwright
第 2 轮报告称该命令"解析到 checkout 的 lockfile(node_modules/.bin/playwright)"。但本仓库中这个 bin 名并不属于 harness 实际 import 的那个包:
$ readlink -f node_modules/.bin/playwright
node_modules/@playwright/test/cli.js
| 包(lockfile) | 版本 | chromium 目录 |
|---|---|---|
@playwright/test(占用 .bin/playwright) |
1.61.1 | chromium-1228 |
playwright(terminal-capture.ts:18 实际 import) |
1.58.2 | chromium-1208 |
dry-run 对照:npx playwright install --dry-run chromium → v1228;node node_modules/playwright/cli.js install --dry-run chromium → v1208。仅放入 1228 后真实启动:
browserType.launch: Executable doesn't exist at .../chromium_headless_shell-1208/...
Please run: npx playwright install
即上一轮的 A 格原样复现——同样 LAUNCH-FAILED、同样设着 QWEN_VERIFY_CHROMIUM=1 的"过期承诺"、同样把 agent 指向 skill 明令禁止的那条命令,只是 revision 从 1194 变成 1228。问题从来不在 pin,而在执行的是哪个包的 CLI。
修复: 改为 node node_modules/playwright/cli.js install chromium(同时补上发现 2 的 -u 参数);步骤注释也需更正——.bin/playwright 解析到的是"某个" lockfile 版本,不是 harness 用的那个。另外,新测试把出问题的命令逐字钉死了:把命令换成上述修复后,pre-installs chromium and hands it to the agent 失败。连同发现 2 的两处改动一起,套件 91/91 通过。
🟠 2. 新步骤丢掉了 command-file 剥离,而测试边界被挪开导致守卫失效
verify 车道里每一处 runuser -u node 要么剥离 GITHUB_OUTPUT/STATE/ENV/PATH/STEP_SUMMARY,要么走 env -i;新步骤两者皆无。strips GitHub command files… 里的 toBe(2) 正是防止新增命令漏检的完整性守卫,而 PR 把切片终点从 'Run verification agent' 改成了 "name: 'Install evidence browser'",等于豁免而非合规。把边界还原后:× expected 3 to be 2。
这不只是洁癖:此时 npm ci 已执行,npx playwright 运行的是 PR 自己的 package.json/lock 所决定的 node_modules/.bin/playwright——即 PR 可控代码,以 node 身份运行,且环境中带着能左右本 job 后续步骤的 $GITHUB_ENV/$GITHUB_PATH/$GITHUB_OUTPUT/$GITHUB_STEP_SUMMARY。(该 job 无 job 级 env:,因此没有 token 暴露;但 prepare 步骤仍显式置空 GITHUB_TOKEN/GH_TOKEN,运行 PR 代码的步骤值得照做。)修复: 补上 5 个 -u,并把守卫范围恢复(终点改回 'Run verification agent',toBe(3)),已验证 91/91。
🟡 3. 成功标记从不清除,而 RUNNER_TEMP 跨运行存活
verify-chromium-path 只在成功时写、失败时不删。本池的 RUNNER_TEMP 会跨运行留存——prepare 步骤自己的注释就是证据("上一个 PR 运行遗留的 verify-results 会混进本次运行")。因此一次成功之后,后续下载失败的运行仍会读到该标记,仍然把 QWEN_VERIFY_CHROMIUM=1 和一个可能属于别的 PR 版本的路径交给 agent——正是本 PR 想消灭的"过期承诺",只是延后了一轮。建议在现有清理旁加一行 rm -f "$RUNNER_TEMP/verify-chromium-path"(这样也不会破坏浏览器步骤的位置断言)。
🟡 4. expect(ifBlock).toContain('if') 是空断言
切片 runStep.slice(guard - 20, marker + 30) 起点落在标记路径内部,窗口里根本没有 shell 的 if 关键字,断言是靠 verify-chromium-path 里的 "if" 通过的(/(^|\s)if\s/ 为 false)。实际起作用的只有 then。把起点放宽到 ifIdx(浏览器步骤的测试已经这么做)即可名副其实。
⚪ 5. 分支命名与现有 369 个不一致
origin 上 369 个 pr-assets/* 分支都是 pr-assets/<N>-verify——包括本 PR 自己的证据图 pr-assets/8016-verify;而发布器写的是 pr-assets/pr${PR_NUMBER}-verify,注释却说"与手工轮次约定一致"。纯外观问题,但确实把命名空间劈成两半。(URL 形态两者都可用:raw.githubusercontent.com/.../pr-assets/6433-verify/... → 200 image/png。)
说明(非发现)
- feat(triage): raise the /verify agent budget from 25m to 120m #8014 仍处于 open:base
main是timeout-minutes: 60+ agent25m。若本 PR 先合入,浏览器下载与 apt 层将横在 25 分钟 agent 预算之前,而非描述所假设的 120/150。 - PR 描述落后代码一轮:仍在讲 root 端
install --with-deps+chmod -R a+rX,而测试现在断言的恰恰相反(not.toContain('install --with-deps'))。描述是合入记录,建议合并前刷新。
已核实通过 ✅
314a408上 91/91;4 个声称的变异与 M1b/M6 位置守卫均在。- 阻塞点 2 确实修好了:origin 上不存在
refs/heads/pr-assets,且在 369 个子分支存在时无法创建,因此旧的clone --branch pr-assets是结构性死路——托管从来就没通过。orphan 回退正确(空仓库上git init+checkout --orphan可用,git 2.47.3 验证),身份/重试/推送逻辑也在分支外设置,两条路径都覆盖到。 - 把 apt 依赖(root、checkout 前)与浏览器二进制(
node、npm ci后)拆开是对的形状——node 属主的目录树直接省掉了chmod -R a+rX。错的只是它调用的那个 CLI。
结论
findings —— 建议合并前修掉 1 和 2。不修 1,验收数字会永远停在 0 而每次运行都报成功;2 则重新打开了本车道特意加上的守卫。
Review —
|
| module | version | resolves for |
|---|---|---|
node_modules/.bin/playwright → @playwright/test/cli.js |
1.61.1 | what npx playwright install chromium runs |
node_modules/playwright (hoisted from packages/webui, integration-tests/terminal-capture) |
1.58.2 | what terminal-capture.ts:18 imports |
And the pinned chromium revisions differ:
playwright-core 1.58.2 → chromium=1208 chromium-headless-shell=1208
playwright-core 1.61.1 → chromium=1228 chromium-headless-shell=1228
So Install evidence browser downloads 1228 into PLAYWRIGHT_BROWSERS_PATH, writes the success marker, and reports "Chromium available for evidence screenshots" — then chromium.launch() in integration-tests/terminal-capture/terminal-capture.ts:282 looks for 1208 and fails with Executable doesn't exist at …/chromium_headless_shell-1208/….
This is worse than a plain miss because of the new guidance interlock: the agent hits browser not found, reads the new troubleshooting line — "in CI verify runs, this means the pre-install step failed; report it, do not install" — and correctly declines to fix it. The marker's stated invariant ("absence is a real signal rather than a stale promise") is inverted: presence becomes the stale promise. That is mutation #1's failure mode arriving through a route the mutation test doesn't cover, and it defeats the PR's whole purpose on the first live run.
The step comment — "node_modules/.bin/playwright resolves the exact lockfile version … so it always matches the lockfile" — is true of the .bin shim and false of the consumer.
- if runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
- npx playwright install chromium; then
+ # Use the module terminal-capture.ts actually imports (`playwright`,
+ # hoisted from packages/webui) — NOT node_modules/.bin/playwright,
+ # which is @playwright/test's CLI and pins a different chromium
+ # revision. Two Playwright trees exist in this lockfile.
+ if runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
+ node ./node_modules/playwright/cli.js install chromium; thenPLAYWRIGHT_BROWSERS_PATH holds revision-suffixed directories, so installing both CLIs is also safe if you'd rather cover either consumer. The new test currently pins the wrong command (expect(browser).toContain('npx playwright install chromium')) — it should assert the resolved module, and ideally assert that the CLI used and the module terminal-capture.ts imports are the same package.
2. The new runuser -u node drops the GitHub command-file strip — and the guard test was narrowed to exempt it
Every other node-run command in this job carries the strip, or env -i:
2270: runuser -u node -- env -u GITHUB_OUTPUT -u GITHUB_STATE -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY npm ci
2283: runuser -u node -- env -u GITHUB_OUTPUT -u GITHUB_STATE -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY npm run build
2810: timeout … runuser -u node -- env -i "${QWEN_ENV[@]}" …
2346: runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" npx playwright install chromium ← new, none of them
npx playwright resolves node_modules/.bin/playwright from the tree npm ci just built out of the PR's own package-lock.json — PR-controlled code, executed before the surviving-lifecycle-process sweep in Run verification agent. With $GITHUB_ENV/$GITHUB_PATH still in the environment, that is the escalation the strip exists to close: injected entries land in subsequent root-run steps, and PATH is forwarded verbatim into the agent's otherwise-curated allowlist ("PATH=$PATH" at line 2768).
What makes this reviewable-but-easy-to-miss is that the test change hides it. The slice end moved from Run verification agent to Install evidence browser, so the new command falls outside the audited region and expect(commands.length).toBe(2) still holds:
const prepare = verifyJob.slice(
verifyJob.indexOf('Install and build PR app'),
- verifyJob.indexOf('Run verification agent'),
+ verifyJob.indexOf("name: 'Install evidence browser'"),
);Fix is one line in the workflow plus reverting the slice:
- if runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
+ if runuser -u node -- env -u GITHUB_OUTPUT -u GITHUB_STATE -u GITHUB_ENV \
+ -u GITHUB_PATH -u GITHUB_STEP_SUMMARY PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \…and expect(commands.length).toBe(3) with the original Run verification agent boundary, so the invariant keeps covering every node-run command in the lane by construction rather than by enumeration.
(No secret exposure here — the verify job has no job-level env: and the model key is scoped to the agent step. The concern is purely the command files.)
3. Untested happy path: orphan-branch creation
The replaced case is the one that now matters most:
- // No reachable pr-assets branch -> text-only, never an aborted report.
- sh(`git init -q --bare "${dir}/empty.git"`);
+ // Unreachable remote -> text-only, never an aborted report.
+ sh(`rm -rf "${dir}/empty.git" && mkdir -p "${dir}/empty.git"`);A reachable bare remote with the branch absent used to assert graceful degradation; it is now the primary path for every PR's first verify run — init → checkout --orphan → push creates pr-assets/pr<N>-verify. Nothing exercises it. Please keep both: the new non-repo case for the unreachable remote, and the old bare-remote case re-purposed to assert the branch gets created and the rendered URL is …/pr-assets/pr7999-verify/verify/…. That is the single most valuable assertion in the file, given the old path was dead for 14 runs without anyone noticing.
Smaller loss in the same hunk: the explicit ::warning::pr-assets branch unavailable is gone, so an auth/network failure is now indistinguishable from a first run until the push-failure warning fires. A one-line echo "::notice::…creating $assets_branch" in the orphan branch would keep that diagnostic.
4. Chromium will likely need --no-sandbox in this container
terminal-capture.ts:282 is chromium.launch({ headless: true }) with no args. Under node:22-bookworm as the node user, the sandbox needs unprivileged user namespaces to be permitted by the runner's seccomp/AppArmor profile. Given Linux is the acknowledged untested platform, this is the second-most-likely reason the first live run still ships text-only, and it costs one line to de-risk:
this.browser = await chromium.launch({
headless: true,
args: process.env.QWEN_VERIFY_CHROMIUM ? ['--no-sandbox'] : [],
});Gated on the CI marker so local behaviour is unchanged.
5. Minor
- Unpinned root-executed fetch.
npx --yes playwright install-deps chromiumpullsplaywright@latestfrom the registry and runs it as root on a persistent self-hosted runner — in a step whose own comment explains its pre-checkout ordering is specifically to keep npm out of PR reach. The "apt list must track current Playwright" rationale is sound, but a floating minor pin (playwright@1.61) is still a superset of 1.58's apt needs and bounds the supply-chain surface. - Stale marker.
verify-chromium-pathis written on success and never cleared on failure. Within one job the step runs once, so this is theory-only today — butrm -f "${RUNNER_TEMP:?}/verify-chromium-path"before theifmakes "absence is a real signal" hold unconditionally instead of depending onRUNNER_TEMPlifetime. - Vacuous assertion.
expect(ifBlock).toContain('if')— the slice starts atguard - 20, i.e. inside…/verify-chromium-path, andver**if**yalready containsif. It passes with the guard deleted. The positionalguard < markercheck is the real teeth; consider replacing the pair with one regex over the actual construct (/if\s+CHROMIUM_PATH=[\s\S]*?then[\s\S]*?QWEN_VERIFY_CHROMIUM=1[\s\S]*?\nfi/). - PR body drifted from the diff. The description and Reviewer Test Plan describe installing "as root in the tools step (
--with-deps chromium) into a shared path,chmod -R a+rX"; the diff doesinstall-depsas root plus a separate node-run binary download, with nochmod. Also, feat(triage): raise the /verify agent budget from 25m to 120m #8014 is still open — head istimeout --kill-after=10s 25mandtimeout-minutes: 60, not 120/150. The download sits outside the agent's clock so the merge order isn't fatal, but the capture work itself competes for 25 minutes today, which is worth stating accurately.
Verdict: the diagnosis and the branch fix are right and worth landing, but as written the browser install cannot serve the module that launches it (#1), and the node-run hardening invariant is weakened by exactly the test edit that would have caught it (#2). Both are small, mechanical fixes. #3 is what turns "wired but unproven" into "proven on the next run".
中文摘要
两个诊断都成立,第二个可从外部证实:本仓库不存在 refs/heads/pr-assets,而 refs/heads/pr-assets/* 子引用有 369 个 —— 由于 D/F 冲突,裸叶子分支根本无法存在,因此旧的 clone --branch pr-assets 从来不可能成功。即使 agent 真的产出过 PNG,也会被这一步静默吞掉。
两个阻塞项:
-
装的浏览器不是真正被启动的那一个。 本仓库有两棵 Playwright 树:
.bin/playwright指向@playwright/test@1.61.1(chromium 1228),而terminal-capture.ts:18导入的playwright解析到 1.58.2(chromium 1208)。npx playwright install chromium只装 1228,随后chromium.launch()找 1208 并报Executable doesn't exist。更糟的是新加的排查指引告诉 agent「这说明预装步骤失败了,只需报告、不要安装」——于是标记位的语义被反转:存在才成了过期承诺。应改用node ./node_modules/playwright/cli.js install chromium,并让测试钉住解析到的模块而非npx。 -
新的
runuser -u node丢掉了 GitHub command-file 剥离。 本 job 中其他每一条 node 命令都带-u GITHUB_OUTPUT/STATE/ENV/PATH/STEP_SUMMARY或env -i,唯独新步骤没有;而npx playwright解析的是 PR 自己npm ci产生的树(PR 可控代码),且运行在残留进程清扫之前。注入$GITHUB_PATH/$GITHUB_ENV会进入后续以 root 运行的步骤,PATH还会被原样带进 agent 的env -i白名单。同时,守卫测试的切片终点被从Run verification agent改到Install evidence browser,恰好把新命令排除在断言之外(length仍为 2)。请补上五个-u并把切片改回去、期望值改为 3。
其余: 孤儿分支创建这条「首次运行必经路径」没有测试覆盖(原来的 bare remote 用例被替换掉了,建议两个都保留);容器内 chromium.launch 很可能需要 --no-sandbox;npx --yes playwright install-deps 以 root 拉取未固定版本;标记文件失败时未清除;expect(ifBlock).toContain('if') 因 ver**if**y 而恒真;PR 描述与实际 diff 不符,且 #8014 尚未合并(当前仍是 25m / 60m)。
…flag Review finding on #8016, verified by mutation before accepting it: deleting `"PLAYWRIGHT_BROWSERS_PATH=$CHROMIUM_PATH"` from the agent env left the test green. That is the nastiest arm of this feature — the agent is TOLD chromium is available via QWEN_VERIFY_CHROMIUM=1, then Playwright looks in the default ~/.cache/ms-playwright instead of the shared install and every capture fails, so a successful install still degrades to a text-only report. The test now asserts both variables and that BOTH sit behind the success guard. Mutation-verified 2/2: deleting the path line fails on the containment assertion, and hoisting it outside the guard fails on the ordering one (`expected 21928 to be less than 21834`). Not changed, because the other two findings do not describe this codebase — replies on the threads carry the evidence. 90/90 tests; prettier and eslint clean.
Two review findings on #8016, both about the per-PR branch scheme the autofix bot introduced on this branch (029beba), and both correct. **Orphan-init's success path had no test.** The existing scenarios both take the clone-failed branch, but one seeded the branch (so clone succeeds) and the other points at a non-repo (so the push fails too) — neither proves orphan-init can DELIVER. A third scenario now points at a valid bare remote with the branch deleted first: clone fails, orphan-init runs, and the images land. It also asserts the branch has exactly one commit, so a graft onto unrelated history would fail rather than pass as "orphan". Without it, a dropped `git init` or `remote add origin` would silently discard every image on every PR's first verification run. Mutation-verified: dropping `remote add origin` and dropping `git init` each kill it. A third mutation — pointing `checkout --orphan` at the wrong branch name — SURVIVES, and correctly: the push is `HEAD:$assets_branch`, which names its destination explicitly, so the local branch name cannot affect delivery. Recorded as a non-defect rather than papered over with a test that pins an irrelevant detail. **Per-PR asset branches were never deleted.** `pr-assets/*` had exactly one producer and one deleter; the verify lane became a second producer without being added, so every verified PR would leave a single-commit branch in the base repo permanently. The cleanup workflow now deletes both refs, is renamed to match what it does, and carries a note that every new `pr-assets/*` producer must be added — a branch nothing deletes is permanent. The loop deliberately drops `set -e`: most PRs produce neither branch, so a 404 on the first must not skip the second. A real delete failure still warns and sets a non-zero exit. The new test executes the script against a stubbed `gh` where the visuals branch 404s, and asserts the verify branch is still deleted — which a `set -e` version would fail. 92/92 tests; prettier and eslint clean.
…rm the strip guard (#8016) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
| // Deps success is recorded in a marker the browser step gates on: apt | ||
| // and the Playwright CDN are independent servers, so a binary download | ||
| // alone must not promise chromium to the agent. | ||
| expect(tools).toContain('verify-chromium-deps-ok'); |
There was a problem hiding this comment.
[Suggestion] This assertion matches both the success-path printf 'ok' > …/verify-chromium-deps-ok and the failure-path rm -f …/verify-chromium-deps-ok in the tools step, so deleting the rm -f from the else branch survives the test (confirmed by mutation probe). On the persistent runner pool a previous run's success marker would then survive a failed deps install: the browser step sees the marker, downloads the binary, writes verify-chromium-path, and the agent is told QWEN_VERIFY_CHROMIUM=1 while system .so files are missing — every Playwright launch dies and the agent burns budget before falling back to text-only, the exact "binary-only success" the two-marker design exists to prevent. Pin the removal specifically:
| expect(tools).toContain('verify-chromium-deps-ok'); | |
| expect(tools).toContain('verify-chromium-deps-ok'); | |
| expect(tools).toContain('rm -f "${RUNNER_TEMP:?}/verify-chromium-deps-ok"'); |
中文说明
该断言同时匹配 tools 步骤中成功路径的 printf 'ok' 与失败路径的 rm -f(两者都含 verify-chromium-deps-ok),因此删掉 else 分支里的 rm -f 测试仍会通过(已用变异探针证实)。在常驻 runner 池上,上一次运行的成功标记会在一次失败的依赖安装后残留:浏览器步骤读到标记、下载二进制、写入 verify-chromium-path,agent 被告知 QWEN_VERIFY_CHROMIUM=1,但系统 .so 缺失——每次 Playwright 启动都失败,agent 在退化为纯文本前白白消耗预算,正是双标记设计要防止的"仅二进制成功"。建议单独钉住删除动作(见上方 suggestion)。
— qwen3.8-max-preview via Qwen Code /review
| const ifIdx = browser.indexOf( | ||
| 'if [ -f "${RUNNER_TEMP:?}/verify-chromium-deps-ok" ]', | ||
| ); | ||
| const markerIdx = browser.indexOf('verify-chromium-path'); | ||
| const elseIdx = browser.indexOf('::warning::Chromium unavailable'); |
There was a problem hiding this comment.
[Suggestion] The three-index ordering check (ifIdx < markerIdx < elseIdx) verifies sequence but not branch membership. verify-chromium-path occurs exactly once inside this step (the printf at qwen-triage.yml:2379), so moving that printf into the else branch ahead of the ::warning:: preserves all three assertions while inverting the gate — the marker is then written on download FAILURE, the agent step reads it and sets QWEN_VERIFY_CHROMIUM=1/PLAYWRIGHT_BROWSERS_PATH, and the agent burns budget on a never-installed browser. The comment above ("Marker is written ONLY inside the success branch (M6)") documents an intent these assertions do not enforce. Anchor the marker between the ; then boundary and the else — e.g. add expect(markerIdx).toBeGreaterThan(browser.indexOf('; then', ifIdx)) and assert the warning sits after the else.
中文说明
三索引顺序检查(ifIdx < markerIdx < elseIdx)只验证了先后顺序,未验证分支归属。verify-chromium-path 在本步骤内恰好出现一次(即 qwen-triage.yml:2379 的 printf),因此把该 printf 移到 else 分支、::warning:: 之前,三个断言依旧全部通过,但门控被反转——标记会在下载失败时被写入,agent 步骤读到它并设置 QWEN_VERIFY_CHROMIUM=1/PLAYWRIGHT_BROWSERS_PATH,agent 会为一个从未安装的浏览器白白消耗预算。上方注释("Marker is written ONLY inside the success branch (M6)")声明的意图并未被这些断言真正守住。建议把标记锚定在 ; then 边界与 else 之间——例如新增 expect(markerIdx).toBeGreaterThan(browser.indexOf('; then', ifIdx)),并断言 warning 位于 else 之后。
— qwen3.8-max-preview via Qwen Code /review
| expect(browser).toContain('-u ACTIONS_RUNTIME_TOKEN'); | ||
| expect(browser).toContain('-u ACTIONS_RUNTIME_URL'); | ||
| expect(browser).toContain('-u ACTIONS_CACHE_URL'); |
There was a problem hiding this comment.
[Suggestion] These assertions cover the env -u stripping (the ACTIONS_* cache credentials here, the GITHUB_* command-file vars in the loop test above) but nothing asserts the step-level env: GITHUB_TOKEN: '' / GH_TOKEN: '' block. The env -u list does not strip GITHUB_TOKEN/GH_TOKEN, so that block is the only mechanism keeping the job token away from the PR-controlled $PW_CLI; grep confirms zero GITHUB_TOKEN assertions in this file. A refactor dropping the two-line block would pass every test. (Blast radius: the verify job sets permissions: contents: 'read', so a leaked token is read-only on a public repo — this is defense-in-depth, not a write-access escalation.) Guard the block explicitly:
| expect(browser).toContain('-u ACTIONS_RUNTIME_TOKEN'); | |
| expect(browser).toContain('-u ACTIONS_RUNTIME_URL'); | |
| expect(browser).toContain('-u ACTIONS_CACHE_URL'); | |
| expect(browser).toContain('-u ACTIONS_RUNTIME_TOKEN'); | |
| expect(browser).toContain('-u ACTIONS_RUNTIME_URL'); | |
| expect(browser).toContain('-u ACTIONS_CACHE_URL'); | |
| expect(browser).toContain("GITHUB_TOKEN: ''"); | |
| expect(browser).toContain("GH_TOKEN: ''"); |
中文说明
这些断言覆盖了 env -u 剥离(此处是 ACTIONS_* 缓存凭据,上方循环测试覆盖 GITHUB_* 命令文件变量),但没有任何断言守住步骤级的 env: GITHUB_TOKEN: '' / GH_TOKEN: '' 块。env -u 列表并不剥离 GITHUB_TOKEN/GH_TOKEN,因此该块是让 job token 远离 PR 可控 $PW_CLI 的唯一机制;grep 确认本文件中没有任何 GITHUB_TOKEN 断言。删掉这两行的重构能让所有测试通过。(影响范围:verify job 设置了 permissions: contents: 'read',泄露的 token 对公共仓库只读——这是纵深防御,而非写权限升级。)建议显式守住该块(见上方 suggestion)。
— qwen3.8-max-preview via Qwen Code /review
| expect(stepIn('verify', 'Install and build PR app')).toContain( | ||
| 'rm -f "$RUNNER_TEMP/verify-chromium-path"', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This asserts the prepare step clears verify-chromium-path, but nothing asserts it does NOT also clear verify-chromium-deps-ok. The two-marker handoff requires the deps marker (written by the tools step) to survive the prepare step so the browser step's gate can read it; a future editor adding rm -f "$RUNNER_TEMP/verify-chromium-deps-ok" here as a "consistency" measure would make that gate always fail and silently degrade every run to text-only, while the suite stays green (the tools-step check at line 2217 and the browser-step check neither assert what prepare does NOT contain). Grep confirms zero not.toContain assertions for any verify-chromium string. Add the negative assertion:
| expect(stepIn('verify', 'Install and build PR app')).toContain( | |
| 'rm -f "$RUNNER_TEMP/verify-chromium-path"', | |
| ); | |
| expect(stepIn('verify', 'Install and build PR app')).toContain( | |
| 'rm -f "$RUNNER_TEMP/verify-chromium-path"', | |
| ); | |
| expect(stepIn('verify', 'Install and build PR app')).not.toContain( | |
| 'verify-chromium-deps-ok', | |
| ); |
中文说明
此断言验证了 prepare 步骤会清除 verify-chromium-path,但没有任何断言保证它不会同时清除 verify-chromium-deps-ok。双标记交接要求依赖标记(由 tools 步骤写入)在 prepare 步骤后依然存活,浏览器步骤的门控才能读到它;若未来有人在此处出于"一致性"补上一句 rm -f "$RUNNER_TEMP/verify-chromium-deps-ok",该门控将永远失败,每次运行都会被静默退化为纯文本,而测试套件依旧全绿(tools 步骤检查与浏览器步骤检查都不验证 prepare 不包含什么)。grep 确认本文件中没有任何针对 verify-chromium 字符串的 not.toContain 断言。建议补上反向断言(见上方 suggestion)。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no action takenThis round had no actionable feedback:
Critical-only mode is active (the PR has completed five change-producing rounds). The only feedback present is in the Deferred non-Critical feedback section, which is an audit record for human follow-up — not work for this bot. Per the review policy, those items are left open and untouched: no code changes, no thread resolutions, and no replies on their behalf. No commits were made this round. 中文说明Autofix 审查轮次 —— 未采取任何操作本轮没有可处理的反馈:
当前处于仅处理 Critical 的模式(该 PR 已完成五个产生改动的轮次)。唯一存在的反馈位于 延迟处理的非 Critical 反馈 部分,它是供人工跟进的审计记录,而非本机器人的工作。根据审查策略,这些条目保持开放且不做改动:不修改代码、不解决线程、也不代为回复。 本轮未产生任何提交。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. Any items listed below stay open for human follow-up; do not modify code, resolve threads, or reply on their behalf.
中文说明完成 5 个产生改动的轮次后,进入仅处理 Critical 的模式。以上内容保持开放,留待人工跟进;不要为其修改代码、解决线程或代为回复。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Maintainer local verification — real build, real chromium, real capturesVerified Verdict: merge-ready. The core design is correct and I proved it is load-bearing. Three items are worth settling before merge, and none of them is in the code: a merge-order dependency, a stale PR body, and a third root cause the body does not mention — which turns out to be the decisive one. 1. The central design claim is true, and measuredThe PR resolves Playwright's CLI from the capture harness's own directory instead of using
I then installed chromium through both CLIs into two separate
Arm B is what the old skill told the agent to run. Arm C is the mutation the PR calls "the nastiest arm" — confirmed: keeping Arm D is the end-to-end result the PR body lists as unproven. Under Worth recording why the 2. Finding — there is a third cause of 0/14, and it alone would have kept it at 0The body frames this as "two independent causes". There is a third, in the publish job, and it is unconditional. On
So on I also confirmed the assumption the rename depends on — that Suggestion: promote this to the body. It changes the PR from "wiring a new capability" to "the publish path was unreachable", which is a materially stronger reason to merge. 3. Finding — #8014 is still open; merge order mattersThe body says the feature is "affordable now because #8014 raised the agent budget to 120 minutes" and "within the 150-minute job limit from #8014". #8014 is The browser download itself lands in its own step, so it spends job time, not agent time — that part is better than the body claims. But the skill change is unconditional: the agent is now told to produce captures whenever a harness ran. Landing that against a 25-minute budget spends verification time on captures that #8014 was supposed to pay for. Merge #8014 first, or land this with the skill wording softened until it does. 4. Tests: 19/19 mutants killed, and the new coverage is real
Two mutants first read SURVIVED against a I also executed the cleanup run block independently and confirmed both producers are covered exhaustively: the repo has exactly two Gates: 5. Suggestion (verified) — gate the marker on a launch, not a downloadThe marker is written when the download exits 0. Arms B and C both downloaded successfully and still could not launch — that entire failure class is invisible to the gate, and in it the agent is told chromium is ready and every capture silently fails. That is also where the one unverified assumption lands: One line closes it, and I confirmed it discriminates exactly the right cases: runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
node -e "require('playwright').chromium.launch().then(b=>b.close())"
# pw-harness (1208) -> exit 0 pw-naive (1228) -> exit 1Cost is one browser start (~0.3 s) and it converts "we downloaded something" into "the harness can launch it" — the property the agent actually needs. Not a blocker; the current design degrades safely in every other arm. 6. Finding — the PR body is stale, and 2 of 5 changed files are undisclosedThe body describes the design from an earlier commit. At
7. Finding — the report will outlive its own imagesThe cleanup now deletes
Blast radius today is zero — I cross-checked all 289 Not covered
中文说明维护者本地验证 —— 真实构建、真实 chromium、真实截图在隔离 worktree 中用本 PR 自己的 lockfile 做了真实 结论:可以合并。 核心设计是正确的,而且我证明了它是承重的。合并前有三件事值得先处理,且都不在代码里:一个合并顺序依赖、一份过期的 PR 描述,以及描述中未提到的第三个根因——而这个根因恰恰是决定性的。 1. 核心设计主张成立,并已量化
我用两个 CLI 分别把 chromium 装进两个独立的
arm B 正是旧 skill 让 agent 去跑的那条路。arm C 即 PR 所称"最难缠的一支"——已确认:保留 补充一点 2. 发现 —— 0/14 存在第三个根因,且它单独就足以让结果保持为 0描述把问题归为"两个相互独立的原因"。在 publish job 里还有第三个,而且是无条件的。
也就是说在 我还验证了改名所依赖的前提: 建议:把这一点写进描述。 它把本 PR 从"接通一个新能力"提升为"发布路径原本根本走不通",这是强得多的合并理由。 3. 发现 —— #8014 尚未合并,合并顺序有影响描述称"因为 #8014 把预算提到 120 分钟所以现在负担得起"、"在 #8014 的 150 分钟 job 上限之内"。#8014 状态为 浏览器下载本身在独立步骤里,花的是 job 时间而不是 agent 时间——这一点比描述说的更好。但 skill 的改动是无条件的:现在只要跑过 harness 就要求 agent 产出截图。在 25 分钟预算下落地,等于用 #8014 本该买下的验证时间去做截图。建议先合 #8014,或先把 skill 措辞放软,等 #8014 落地再收紧。 4. 测试:19/19 变异全部被杀,且新增覆盖是真的
其中两个变异最初显示 SURVIVED,原因是我的 我另外独立执行了 cleanup 的 run 块,并确认两个生产者被穷尽覆盖:仓库里恰好只有两个 门禁:两个 workflow 上 5. 建议(已验证)—— marker 应以"能否启动"为门槛,而不是"是否下载成功"marker 是在下载 exit 0 时写入的。而 arm B 与 arm C 都下载成功了却都启动不了——这一整类失败对该门禁是不可见的;一旦落入其中,agent 会被告知 chromium 就绪,然后每次截图静默失败。唯一未验证的假设也正好落在这里:最新 Playwright 的 apt 列表被假定是 1.58.2 的超集,而这里一旦回退,失败发生在启动而非下载。 一行即可堵上,我已确认它能精确区分这两种情况: runuser -u node -- env PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
node -e "require('playwright').chromium.launch().then(b=>b.close())"
# pw-harness (1208) -> exit 0 pw-naive (1228) -> exit 1代价是一次浏览器启动(约 0.3 秒),把"我们下载到了某个东西"变成"harness 能启动它"——后者才是 agent 真正需要的性质。不是阻塞项,其余各支现有设计都能安全降级。 6. 发现 —— PR 描述已过期,且 5 个改动文件中有 2 个未披露描述写的是更早某个 commit 的设计。在
7. 发现 —— 报告会比它自己的图片活得更久cleanup 现在会在 PR 关闭时删除
今天的影响面为零——我把 289 个 未覆盖
|
|
@qwen-code /triage |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no actionNo actionable feedback this round.
The only item present is a deferred non-Critical PR comment No commits were made this round. 中文说明Autofix 审查轮次 — 无操作本轮没有需要处理的反馈。
当前唯一存在的条目是一条被延迟的非 Critical PR 评论 本轮未产生任何提交。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. Any items listed below stay open for human follow-up; do not modify code, resolve threads, or reply on their behalf.
中文说明完成 5 个产生改动的轮次后,进入仅处理 Critical 的模式。以上内容保持开放,留待人工跟进;不要为其修改代码、解决线程或代为回复。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at e85b1f0. The workflow changes themselves check out: I traced both markers' full lifecycles (verify-chromium-deps-ok written/removed only by the tools step, read at the browser-step gate; verify-chromium-path cleared in prepare, written only in the success branch, read by the agent step), confirmed the browser-install step strips GITHUB_TOKEN/GH_TOKEN at step level plus env -us the Actions runtime credentials, and confirmed the hosting path keeps the PNG magic check, name sanitisation, and count/size caps. Deps-install failure degrades to text-only rather than half-configuring the agent. No new correctness or security findings in the workflow.
Two things keep this from an approve right now:
- One measured test-runtime regression, inline below.
- The 4 unresolved ci-bot suggestions on the test file all still stand at this HEAD — I checked each against the code: the
verify-chromium-deps-okcontainment assertion still matches both the successprintfand the failurerm -f; the three-index ordering check still doesn't pin branch membership; the step-levelenv: GITHUB_TOKEN: ''block is still unasserted; and nothing asserts prepare preservesverify-chromium-deps-ok. They're all Suggestions, so tighten or decline explicitly — either is fine, but the threads should be closed out.
| // remote), so neither proves orphan-init can actually DELIVER. Without | ||
| // this, a bug in `checkout --orphan` or a dropped `remote add origin` | ||
| // would silently discard every image on every PR's first run. | ||
| sh(`git -C "${dir}/assets.git" branch -D pr-assets/7999-verify`); |
There was a problem hiding this comment.
[Suggestion] The orphan-init leg added to this test pushes its runtime past the suite's 30s testTimeout. Measured on the same machine, single-test run (-t 'hosts only valid…', 120s timeout): 23.3s at the merge-base cc617e6, 27.6s at this HEAD. Under a full-file run (npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-triage-workflow.test.js) the test hit the 30s cap and failed — reproducibly, while every other test passed (92/93). CI absorbs this today, but <3s of headroom means any slower runner fails the suite on a test that is actually passing. Suggest passing a per-test timeout as the third it() argument (e.g. 60_000), matching how the config comment already frames the 30s value as a floor for spawn-heavy tests.
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM!✅
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Root causeWhile PR #8016 was open, Textual or semanticTextual in the only conflicted file: the sides add independent, } finally {
rmSync(dir, { recursive: true, force: true });
}
}); // <-- added: closes the PR's last test
// One budget drives the agent's graceful kill ...
it('keeps the verify budget, watchdog and job limit consistent', () => {
...
}); // <-- pre-existing shared line, now closes main's testThe files these tests assert on ( What is load-bearing
What I could not verifyNo build/lint/tests run. Beyond 中文说明根本原因:PR 开放期间 main 重构了 /verify 执行步骤(引入 文本性冲突:唯一冲突文件中两侧是互不重叠的新增测试,无共享逻辑。解决方式为 关键依赖:自动合并后的执行步骤中,chromium 环境守卫必须位于 未验证项:未运行构建/lint/测试;仅通过语法检查和 grep 确认两侧断言目标均 |
76dfab1
|
@qwen-code /triage |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM!✅
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: No action taken this round. There is no new actionable feedback: no reviews, no inline comments, no issue-level comments, and no failed or still-red checks. Critical-only mode is active after five change-producing rounds, so the two deferred non-Critical items (review by @doudouOUC and inline comment rc:3679446055) remain open for human follow-up and are intentionally left untouched. 中文说明本轮未采取任何操作。 没有新的可处理反馈:无评审、无行内评论、无 Issue 级评论,也没有失败或持续红色的检查项。经过五个产生改动的轮次后,已进入仅处理 Critical 的模式,因此两个被延迟的非 Critical 项目(@doudouOUC 的评审和行内评论 rc:3679446055)保持开放,留待人工跟进,本轮有意不做处理。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. Any items listed below stay open for human follow-up; do not modify code, resolve threads, or reply on their behalf.
中文说明完成 5 个产生改动的轮次后,进入仅处理 Critical 的模式。以上内容保持开放,留待人工跟进;不要为其修改代码、解决线程或代为回复。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Post-merge measurement of QwenLM#8016, on the first two live runs after it landed (QwenLM#7975 and QwenLM#8066): the browser installed successfully — "Install evidence browser: success" in both — and both reports carried **zero images**. QwenLM#7975 has 31 tables, QwenLM#8066 has 407 assertions. Neither has a witness. The cause is the third instance of one structural bug, and it is the same one twice already fixed: QwenLM#7917 the /verify recommendation sat inside a section headed "local invocation ONLY" -> 1/16 reach QwenLM#8016 captures were "Optionally ... when text cannot carry the oracle" -> 0/14 now captures are a required deliverable in the ARTIFACT CONTRACT, but the plan the agent executes is the Scope-selection budget list, and that list had zero mentions of evidence, png, capture or screenshot An agent that budgets by that list never allocates time for a capture, however firmly a later section says to produce one. So captures are now budget item 4 alongside the A/B, the harnesses and the gates, with the time reserved (~5 min), the gate named (QWEN_VERIFY_CHROMIUM=1), the count bounded (normally two, at most a handful), and the measured failure stated so the rule carries its own reason. The report structure gets the matching half: the A/B table item now says to reference the capture of those cells by filename. A produced image with nowhere to go is as dead as an unproduced one. Mutation-verified 4/4: dropping the budget item, the time allowance, the count bound, or the report-side reference each turns the test red. Not changed, and worth stating rather than implying: the seven techniques from QwenLM#8010 do not appear in either report. I do not read that as failure — every one of them is conditional ("when one fix bundles two changes", "when the oracle is an instrument", "when a PR adds a writer to a shared store") and neither PR met those conditions. QwenLM#8010 is not yet falsifiable from these runs. What both reports DO show is older skill rules working: a naive-matcher differential in QwenLM#8066, and QwenLM#7975's realpath check finding node_modules/@qwen-code/qwen-code-core resolving into the HEAD tree and then reasoning about whether that contaminates the control. 107/107 tests; prettier and eslint clean. Co-authored-by: wenshao <wenshao@example.com>
|
Released in v0.21.2. |









What this PR does
Makes
/verifyable to produce the screenshots its reports have always had a slot for.Measured across the 14 real verify reports on this repo:
The hosting machinery has been complete since the lane shipped —
pr-assetsbranch, PNG magic check, sanitised names, 8 images, 2 MB each — and it has never once been fed. Two independent causes, both fixed here.1. The agent physically could not produce a PNG
It runs as
nodeunderenv -iwithHOMEpointed at a per-run scratch dir. Sonpx playwright install --with-deps chromium:aptit has no rights to (system deps), andChromium is now installed by root in the tools step into a shared, world-readable
PLAYWRIGHT_BROWSERS_PATH, and the agent env carries that path plusQWEN_VERIFY_CHROMIUM=1— only when the install actually succeeded, so the variable's absence is a real signal rather than a stale promise. The install is best-effort: a failure emits a warning and the run continues to a text-only report, which is what every run has produced anyway.2. The skill discouraged it
The old wording:
An agent verifying a daemon or a CLI reads that, correctly concludes text carries the oracle fine, and skips. That is exactly what happened 14 times.
It now says produce them whenever a harness ran, and says why:
with the three highest-value shots named (A/B cells side by side, the mutation matrix as it printed, raw harness output behind a headline number), and
playwright installexplicitly forbidden so cause 1 cannot be rediscovered by an agent burning budget on it.Affordable now because #8014 raised the agent budget to 120 minutes; at 20 minutes a browser download alone was most of the run.
Why it's needed
This is precisely the gap between the lane's output and the maintainer-written rounds it is modelled on — those carry a capture of the live daemon A/B, the mutation matrix, the raw driver output. The screenshot is not decoration: it is the provenance artifact. A markdown table is trivially fabricable by a model; a terminal capture of the harness that produced it is not.
Reviewer Test Plan
How to verify
One test pins both halves — the wiring and its failure modes:
--with-deps chromium), into a shared path,chmod -R a+rX;exit 1;playwright install.Mutation-verified 4/4, each with landing proof:
exit 1on chromium install failure90/90 tests; the tools step passes
bash -nandshellcheck --enable=all; prettier, eslint and actionlint clean.Tested on
Linux is the honest gap and the reason the install is best-effort: I could not execute
playwright install --with-deps chromiuminsidenode:22-bookwormfrom here (no container runtime on this machine), so whether chromium actually installs and renders on the ECS runners is unverified until the first live run. The design assumes it may fail and degrades to today's behaviour when it does — the test asserts that degradation rather than the install.Risk & Scope
::warning::and produces exactly the report it produces today — no verification is lost. If it installs but the capture path is broken, an agent could spend budget on failed screenshots; the skill's instruction to ship text-only and note it under Not covered in one line is the mitigation, and it is guidance rather than a mechanism.Linked Issues
Follow-up to #7710. Depends on the budget raise in #8014 to be affordable. No issues closed.
中文说明
本 PR 做了什么
让
/verify真正能产出它报告中一直预留了位置的截图。对本仓库 14 份真实 verify 报告的实测:
托管机制自这条车道上线起就是完整的——
pr-assets分支、PNG magic 校验、文件名净化、8 张上限、每张 2 MB——而它一次都没有被喂过数据。两个相互独立的原因,本 PR 都修了。1. agent 在物理上就无法产出 PNG
它以
node身份、在env -i下运行,且HOME指向每次运行都会被删除的临时目录。因此npx playwright install --with-deps chromium:需要它无权使用的apt(系统依赖),并且会把约 170 MB 下载进一个运行结束即删除的目录。现在 chromium 由 root 在 tools 步骤中安装到共享、全局可读的
PLAYWRIGHT_BROWSERS_PATH,agent 环境携带该路径以及QWEN_VERIFY_CHROMIUM=1——且仅在安装确实成功时才携带,因此该变量的缺失是一个真实信号,而不是过期的承诺。安装是尽力而为的:失败则发出 warning,运行继续产出纯文本报告,也就是目前每次运行本就产出的东西。2. skill 在劝阻它
旧措辞是:"Optionally
evidence/*.png…… 当文本无法承载 oracle 时使用:TUI 渲染……"一个在验证 daemon 或 CLI 的 agent 读到这句,会正确地判断"文本足以承载 oracle",然后跳过。这正是发生了 14 次的事情。
现在它写的是:只要跑过 harness 就产出截图,并说明为什么:
同时点名了三种最高价值的截图(并排的 A/B 单元格、打印出来的变异矩阵、支撑标题数字的原始 harness 输出),并明确禁止
playwright install,使原因 1 不会被某个 agent 重新发现并为此烧掉预算。之所以现在负担得起,是因为 #8014 把 agent 预算提高到了 120 分钟;在 20 分钟的预算下,光是下载浏览器就占去大半。
为什么需要
这正是本车道的产出与它所对标的维护者手写轮次之间的差距——那些报告带着真实 daemon A/B 的截图、变异矩阵、原始 driver 输出。截图不是装饰,而是溯源凭证:一张 markdown 表格对模型而言是轻易可编造的,而产生这些数字的 harness 的终端截图不是。
评审验证方案
一条测试同时钉住接线与其失败模式:由 root 在 tools 步骤安装(
--with-deps chromium)到共享路径并chmod -R a+rX;尽力而为——失败仅 warning,步骤中不含exit 1;agent 仅在成功标记之后才拿到该变量(以位置断言:守卫的索引先于赋值);tmux 车道未被触及;skill 不再写 "Optionally"、确实写了 "Produce these whenever you ran a harness"、且确实禁止了playwright install。4/4 变异验证,每项均带落地证明:无条件把变量交给 agent(失败的安装变成过期承诺);chromium 安装失败即
exit 1(一次浏览器下载故障会让所有验证失败);skill 回退为 "Optionally"(精确复现 0/14 的现状);删除禁止安装的警告(agent 会重新推导出原因 1,并为一个它无法完成的 170 MB 下载烧掉预算)。90/90 测试;tools 步骤通过
bash -n与shellcheck --enable=all;prettier、eslint、actionlint 干净。测试平台
macOS ✅;Windows N/A;Linux⚠️
Linux 是诚实的缺口,也正是安装采取尽力而为策略的原因:我无法从本机在
node:22-bookworm内执行playwright install --with-deps chromium(本机没有容器运行时),因此chromium 在 ECS runner 上是否真能安装并渲染,在首次真实运行前是未经验证的。该设计假定它可能失败,并在失败时退化为今天的行为——测试断言的是这种退化,而不是安装本身。风险与范围
::warning::,并产出与今天完全相同的报告——不损失任何验证。若安装成功但截图路径有问题,agent 可能把预算花在失败的截图上;skill 中"产出纯文本报告并在 Not covered 中用一行说明"的指令是缓解措施,但它是引导而非机制。关联 Issue
#7710 的后续。依赖 #8014 的预算提升才负担得起。不关闭任何 issue。