vrr/qwen-code
2
0
Fork 0
mirror of https://github.com/QwenLM/qwen-code.git synced 2026-08-04 05:40:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 18

feat(triage): make /verify evidence screenshots actually possible (#8016)

Browse source 
* feat(triage): make /verify evidence screenshots actually possible

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.

* fix(triage): match browser to lockfile and fix pr-assets branch conflict (#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).

* fix(triage): unpin Playwright install-deps so apt list tracks lockfile browser (#8016)

* fix(triage): assert the browser path reaches the agent, not just the 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.

* fix(triage): cover orphan-init delivery and delete verify asset branches

Two review findings on #8016, both about the per-PR branch scheme the
autofix bot introduced on this branch (029beba3ef), 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.

* fix(triage): match browser install to the harness Playwright and re-arm the strip guard (#8016)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(triage): gate chromium marker on deps and resolve Playwright from the harness (#8016)

Address review feedback on the verify evidence-browser path:

- Resolve the Playwright CLI from the capture harness's own directory
  (require.resolve of the exported package.json, then join cli.js) instead
  of assuming npm hoists playwright to the root, so the installed binary
  tracks the harness's import even if the hoist layout changes. cli.js is
  not in the package's exports map, which is why a direct path was used
  before; resolving the exported package.json keeps that bypass while
  removing the hoist assumption.
- Gate the verify-chromium-path marker on a new verify-chromium-deps-ok
  marker written by the tools step. apt and the Playwright CDN are
  independent servers with no shared success signal, so a binary download
  alone must not promise chromium to the agent when system deps failed.
- Add a delete-failure case to the PR asset cleanup test so the status=1
  and exit path is executed, not just statically asserted.

* fix(triage): strip cache creds from browser install and guard cli.js resolution (#8016)

The evidence-browser install runs the PR-resolved Playwright CLI but did
not drop ACTIONS_RUNTIME_TOKEN/URL/CACHE_URL the way the adjacent prepare
and agent steps do, leaving runner-injected cache credentials readable by
PR-controlled code. Strip them in the same env -u chain. Also replace the
string-only guard on the require.resolve + cli.js join with an execution
guard that resolves the expression against the installed tree, so a
Playwright bump that relocates cli.js fails the test instead of silently
degrading evidence capture at runtime.

---------

Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
This commit is contained in:
Shaojin Wen 2026-07-30 13:37:42 +08:00 • committed by GitHub
parent 0ae7757b0b
commit 6b0d2bbca0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 486 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

99
.github/workflows/qwen-triage.yml vendored
View file

@ -2247,6 +2247,25 @@ jobs:
(cd "${RUNNER_TEMP:?}" && npm install -g --registry=https://registry.npmjs.org '@qwen-code/qwen-code@latest')
qwen --version
# Chromium system dependencies (apt packages) for evidence
# screenshots. install-deps is unpinned so the apt list tracks
# current Playwright (a superset of the lockfile binary's needs);
# the version-sensitive BINARY is downloaded after checkout by the
# "Install evidence browser" step, using the checkout's own
# Playwright so it always matches the lockfile. Best-effort: a
# failure here must not fail the job.
# Record success in a marker the "Install evidence browser" step
# gates on: apt and the Playwright CDN are independent servers with
# no shared success signal, so a binary download alone must not
# promise chromium to the agent. Rewritten or removed every run so
# a stale success on the persistent pool cannot leak through.
if (cd "${RUNNER_TEMP:?}" && npx --yes playwright install-deps chromium); then
printf 'ok' > "${RUNNER_TEMP:?}/verify-chromium-deps-ok"
else
rm -f "${RUNNER_TEMP:?}/verify-chromium-deps-ok"
echo "::warning::Chromium system deps install failed; the verification agent will produce a text-only report."
fi
# This container mounts the persistent runner workspace, and a previous
# run EXECUTED PR code as the node user with write access to .git —
# hooks or config planted then would fire during the checkout below (as
@ -2444,8 +2463,11 @@ jobs:
unset ACTIONS_RUNTIME_TOKEN ACTIONS_RUNTIME_URL ACTIONS_CACHE_URL
# rm first: on the persistent pool a stale verify-results from an
# EARLIER PR's run would otherwise ride along into this run's
# artifact upload and report selection.
# artifact upload and report selection. The chromium marker is
# cleared too, so a previous run's success cannot promise this run
# a browser it never installed.
rm -rf "$RUNNER_TEMP/verify-results"
rm -f "$RUNNER_TEMP/verify-chromium-path"
mkdir -p "$RUNNER_TEMP/verify-results"
chown -R node:node "$GITHUB_WORKSPACE"
prepare_log="$RUNNER_TEMP/verify-results/prepare.log"
@ -2536,6 +2558,46 @@ jobs:
fi
echo "Install/build completed before verification." >> "$GITHUB_STEP_SUMMARY"
- name: 'Install evidence browser'
if: "steps.pr.outputs.decision == 'run' && steps.prepare.outputs.verdict == ''"
env:
GITHUB_TOKEN: ''
GH_TOKEN: ''
run: |-
set -uo pipefail
# Download the browser binary for the Playwright package the
# capture harness actually imports. This lockfile has TWO
# Playwright trees: terminal-capture.ts imports `playwright`, but
# node_modules/.bin/playwright is @playwright/test's CLI, which
# pins a different chromium revision — so `npx playwright install`
# would download a browser the harness cannot launch. Resolve the
# CLI from the harness's own directory: require.resolve runs the
# same algorithm as its `from 'playwright'` import, so the binary
# matches the lockfile even if npm's hoist layout changes (nothing
# pins playwright to the root node_modules). cli.js is not in the
# package's exports map, so resolve the exported package.json and
# join. Runs as node so the tree is agent-readable without chmod.
# Best-effort: failure degrades to a text-only report.
PW_PATH="${RUNNER_TEMP:?}/pw-browsers"
mkdir -p "$PW_PATH"
chown node:node "$PW_PATH"
PW_CLI="$(runuser -u node -- node -p "require('path').join(require('path').dirname(require.resolve('playwright/package.json', { paths: ['./integration-tests/terminal-capture'] })), 'cli.js')")"
# The marker gates QWEN_VERIFY_CHROMIUM in the agent step, so it
# requires BOTH halves: the system deps installed as root in the
# tools step (which writes verify-chromium-deps-ok) AND the binary
# download below. A binary-only success would tell the agent
# chromium is ready when every launch still dies on a missing .so.
if [ -f "${RUNNER_TEMP:?}/verify-chromium-deps-ok" ] &&
runuser -u node -- env -u GITHUB_OUTPUT -u GITHUB_STATE -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY \
-u ACTIONS_RUNTIME_TOKEN -u ACTIONS_RUNTIME_URL -u ACTIONS_CACHE_URL \
PLAYWRIGHT_BROWSERS_PATH="$PW_PATH" \
node "$PW_CLI" install chromium; then
printf '%s' "$PW_PATH" > "${RUNNER_TEMP:?}/verify-chromium-path"
echo "Chromium available for evidence screenshots." >> "$GITHUB_STEP_SUMMARY"
else
echo "::warning::Chromium unavailable (system deps or browser download failed); the verification agent will produce a text-only report."
fi
- name: 'Run verification agent'
if: "steps.pr.outputs.decision == 'run' && steps.prepare.outputs.verdict == ''"
id: 'run'
@ -2970,6 +3032,19 @@ jobs:
"QWEN_CI_REAL_GH=${QWEN_CI_REAL_GH:-}"
"QWEN_CI_REAL_GIT=${QWEN_CI_REAL_GIT:-}"
)
# Evidence screenshots: the "Install evidence browser" step
# downloaded chromium into a shared path using the checkout's
# Playwright. Both variables are set only when that install
# actually succeeded, so their ABSENCE is the agent's signal to
# skip captures rather than burn budget on a download it cannot
# complete.
if CHROMIUM_PATH="$(cat "${RUNNER_TEMP:?}/verify-chromium-path" 2>/dev/null)" &&
[ -n "$CHROMIUM_PATH" ]; then
QWEN_ENV+=(
"PLAYWRIGHT_BROWSERS_PATH=$CHROMIUM_PATH"
"QWEN_VERIFY_CHROMIUM=1"
)
fi
if [ -n "${OPENAI_MODEL:-}" ]; then
QWEN_ENV+=("OPENAI_MODEL=$OPENAI_MODEL")
fi
@ -3251,8 +3326,8 @@ jobs:
printf '</code></pre>\n\n</details>\n\n'
}
# Host the agent's evidence images (if any) on the pr-assets branch
# — the same convention hand-run verification rounds use — and build
# Host the agent's evidence images (if any) on a per-PR branch
# (pr-assets/<N>-verify, matching hand-run convention) and build
# a markdown section referencing them. Image bytes come from a run
# that executed PR code: inert but untrusted, so filenames pass a
# strict allowlist, count/size are capped (8 files, <2 MB each; the
@ -3263,6 +3338,7 @@ jobs:
collect_and_host_evidence() {
local imgs=() f base safe seen=' ' hosted=0 total=0 skipped=0
local dest_dir="verify/pr${PR_NUMBER}-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT:-1}"
local assets_branch="pr-assets/${PR_NUMBER}-verify"
local clone_dir="${RUNNER_TEMP:-/tmp}/pr-assets"
local remote="${VERIFY_ASSETS_REMOTE:-https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git}"
# `|| true` on every find: the artifact download is
@ -3282,9 +3358,12 @@ jobs:
return 0
fi
rm -rf "$clone_dir"
if ! git clone -q --depth 1 --branch pr-assets "$remote" "$clone_dir" 2>/dev/null; then
echo "::warning::pr-assets branch unavailable; posting a text-only report." >&2
return 0
if ! git clone -q --depth 1 --branch "$assets_branch" "$remote" "$clone_dir" 2>/dev/null; then
rm -rf "$clone_dir"
mkdir -p "$clone_dir"
git -C "$clone_dir" init -q
git -C "$clone_dir" checkout -q --orphan "$assets_branch"
git -C "$clone_dir" remote add origin "$remote"
fi
# Rebase in the racing-push retry needs a committer identity, so
# set it once on the clone instead of per-command -c flags.
@ -3320,11 +3399,11 @@ jobs:
git add "$dest_dir" &&
git commit -q -m "verify evidence for PR #${PR_NUMBER} (run ${GITHUB_RUN_ID})" &&
{
git push -q origin HEAD:pr-assets 2>/dev/null ||
git push -q origin "HEAD:$assets_branch" 2>/dev/null ||
{
# One retry after a racing push from another assets job.
git pull -q --rebase origin pr-assets 2>/dev/null &&
git push -q origin HEAD:pr-assets 2>/dev/null
git pull -q --rebase origin "$assets_branch" 2>/dev/null &&
git push -q origin "HEAD:$assets_branch" 2>/dev/null
}
}
); then
@ -3332,7 +3411,7 @@ jobs:
EVIDENCE_SECTION=''
return 0
fi
local raw_base="https://raw.githubusercontent.com/${GITHUB_REPOSITORY}/pr-assets/${dest_dir}"
local raw_base="https://raw.githubusercontent.com/${GITHUB_REPOSITORY}/${assets_branch}/${dest_dir}"
EVIDENCE_SECTION=$'### Evidence images\n\n'
for f in "$clone_dir/$dest_dir"/*.png; do
[ -f "$f" ] || continue

44
.github/workflows/web-shell-visuals-cleanup.yml vendored
View file

@ -1,9 +1,13 @@
name: 'Web-shell Visuals Cleanup'
name: 'PR Asset Branch Cleanup'
# When a PR closes, delete its per-PR visuals asset branch so the `pr-assets/*`
# refs (one per PR that ever produced a preview) don't accumulate without bound
# in the base repository. Runs in the base context (pull_request_target) but
# never checks out or runs PR code — it only deletes one ref by name.
# When a PR closes, delete its per-PR asset branches so the `pr-assets/*`
# refs (one per PR that ever produced a preview or a verification report)
# don't accumulate without bound in the base repository. Runs in the base
# context (pull_request_target) but never checks out or runs PR code — it
# only deletes refs by name.
#
# Both producers are covered, and every new `pr-assets/*` producer must be
# added here: a branch nothing deletes is permanent.
on:
pull_request_target:
types:
@ -18,17 +22,29 @@ jobs:
runs-on: 'ubuntu-latest'
timeout-minutes: 5
steps:
- name: 'Delete the PR asset branch'
- name: 'Delete the PR asset branches'
env:
# Deleting a ref needs contents:write, which the CI_BOT_PAT carries.
GH_TOKEN: '${{ secrets.CI_BOT_PAT }}'
PR_NUMBER: '${{ github.event.pull_request.number }}'
run: |-
set -euo pipefail
branch="pr-assets/web-shell-visuals-${PR_NUMBER}"
if gh api "repos/${GITHUB_REPOSITORY}/git/refs/heads/${branch}" >/dev/null 2>&1; then
gh api -X DELETE "repos/${GITHUB_REPOSITORY}/git/refs/heads/${branch}"
echo "Deleted ${branch}."
else
echo "No asset branch ${branch}; nothing to delete."
fi
set -uo pipefail
# Not `set -e`: one branch missing, or one delete failing, must not
# stop the others. Each is independent and absence is normal — most
# PRs produce neither.
status=0
for branch in \
"pr-assets/web-shell-visuals-${PR_NUMBER}" \
"pr-assets/${PR_NUMBER}-verify"; do
if ! gh api "repos/${GITHUB_REPOSITORY}/git/refs/heads/${branch}" >/dev/null 2>&1; then
echo "No asset branch ${branch}; nothing to delete."
continue
fi
if gh api -X DELETE "repos/${GITHUB_REPOSITORY}/git/refs/heads/${branch}"; then
echo "Deleted ${branch}."
else
echo "::warning::Failed to delete ${branch}; it will need removing by hand."
status=1
fi
done
exit "$status"

10
.qwen/skills/terminal-capture/SKILL.md
View file

@ -17,9 +17,14 @@ Ensure the following dependencies are installed before running:
```bash
npm install # Install project dependencies.
npx playwright install chromium # Install Playwright browser
npx playwright install chromium # Install Playwright browser (skip in CI: see note below)
```
> **CI / verify context:** when `QWEN_VERIFY_CHROMIUM=1` is set, the browser
> is already installed and `PLAYWRIGHT_BROWSERS_PATH` points at it. Do **not**
> run `playwright install` — it downloads ~170 MB and fails on system deps
> the agent user cannot install.
## Architecture
```
@ -227,7 +232,8 @@ This tool is commonly used for visual verification during PR reviews.
- Playwright error `browser not found`
Cause: browser not installed.
Solution: `npx playwright install chromium`.
Solution: `npx playwright install chromium` (local dev only — in CI verify
runs, this means the pre-install step failed; report it, do not install).
- Blank screenshot
Cause: process starts slowly or build failed.
Solution: check build success and the spawn command.

38
.qwen/skills/verify-pr/SKILL.md
View file

@ -525,17 +525,33 @@ workflow globs). It must contain:
- `assertions.json` — `{"pass": <int>, "fail": <int>, "total": <int>}`,
counting **only scripted assertions that actually executed**.
- Harness scripts and raw logs (per-cell stdout/stderr, build logs).
- Optionally `evidence/*.png` — rendered image evidence. The publish job
hosts these on the `pr-assets` branch and appends them below the report,
capped at **8 images, 2 MB each**; anything beyond stays in the run
artifacts only. Use them when text cannot carry the oracle: TUI rendering
(`terminal-capture` skill: node-pty → xterm → Playwright PNG;
`npx playwright install chromium` on demand) or a one-image harness
summary. Name each file as a kebab-case caption that binds image to claim
(`01-bundle-ab-base-vs-head.png`, `02-repaint-after-sigcont.png`) — the
filename becomes the published caption — and reference it from report.md
prose by that name. Before/after pairs beat single "after" shots; a
screenshot that does not name what to look at proves nothing.
- `evidence/*.png` — image evidence. **Produce these whenever you ran a
harness**, not only for TUI work. A table in the report is your _claim_
about what happened; a capture of the run is a _witness_ that the numbers
came from a real execution, and it is the part a reviewer cannot get any
other way. The highest-value shots, in order: the A/B cells side by side,
the mutation matrix as it printed, and the raw harness output behind a
headline number. One capture of the terminal showing `2999 → 0` is worth
more than the sentence asserting it.
**Chromium is pre-installed for you** when `QWEN_VERIFY_CHROMIUM=1` is set;
`PLAYWRIGHT_BROWSERS_PATH` already points at it. Do **not** run
`playwright install` — you run as `node` with a fresh `HOME` and no apt
rights, so it downloads ~170 MB and then fails on system deps. If
`QWEN_VERIFY_CHROMIUM` is unset the capability is unavailable in this run:
ship the text-only report and note it under _Not covered_ in one line, do
not spend budget working around it.
Route: `terminal-capture` skill (node-pty → xterm.js → Playwright PNG).
The publish job hosts what you produce on a per-PR branch
(`pr-assets/<N>-verify`) and appends it below the report, capped at
**8 images, 2 MB each**; anything
beyond stays in the run artifacts. Name each file as a kebab-case caption
that binds image to claim (`01-bundle-ab-base-vs-head.png`,
`02-repaint-after-sigcont.png`) — the filename becomes the published
caption — and reference it from report.md prose by that name. Before/after
pairs beat single "after" shots; a screenshot that does not name what to
look at proves nothing.
`verdict.txt` meanings: `merge-ready` = every executed assertion passed and no
new blocking finding; `findings` = evidence produced concrete problems worth a

340
scripts/tests/qwen-triage-workflow.test.js
View file

@ -6,6 +6,7 @@
import { spawn, spawnSync } from 'node:child_process';
import {
existsSync,
mkdirSync,
mkdtempSync,
readdirSync,
@ -1515,15 +1516,19 @@ describe('qwen-triage verify hardening', () => {
// The lifecycle-script command-file guards must be asserted on the verify
// job's own commands: a bare step() lookup returns the tmux job's
// identically named step, so verify-side regressions would pass silently.
it('strips GitHub command files from both verify lifecycle commands', () => {
// Bound to the prepare step: the agent step's own `runuser` launches
// qwen under `env -i`, which needs no per-variable stripping.
it('strips GitHub command files from every node-run verify command', () => {
// Bound to the lifecycle commands that run as node before the agent:
// npm ci and npm run build in the prepare step, plus the evidence
// browser download. The slice stops at the agent step, whose own
// `runuser` launches qwen under `env -i` and needs no per-variable
// stripping. Covering all three by construction (not enumeration) is
// what catches a future node-run command added without the strip.
const prepare = verifyJob.slice(
verifyJob.indexOf('Install and build PR app'),
verifyJob.indexOf('Run verification agent'),
);
const commands = prepare.match(/runuser -u node -- env[\s\S]*?\n/g) ?? [];
expect(commands.length).toBe(2);
expect(commands.length).toBe(3);
expect(step('Run verification agent')).toContain(
'runuser -u node -- env -i',
);
@ -1869,7 +1874,7 @@ describe('qwen-triage verify hardening round 2', () => {
// A bare remote with a pr-assets branch, plus a gh stub.
sh(`git init -q --bare "${dir}/assets.git"`);
sh(
`mkdir -p "${dir}/seed" && cd "${dir}/seed" && git init -q && git checkout -q -b pr-assets && echo s > s.txt && git add . && git -c user.name=t -c user.email=t@t commit -qm s && git push -q "${dir}/assets.git" pr-assets`,
`mkdir -p "${dir}/seed" && cd "${dir}/seed" && git init -q && git checkout -q -b pr-assets/7999-verify && echo s > s.txt && git add . && git -c user.name=t -c user.email=t@t commit -qm s && git push -q "${dir}/assets.git" pr-assets/7999-verify`,
);
writeFileSync(
join(dir, 'gh'),
@ -1931,7 +1936,7 @@ describe('qwen-triage verify hardening round 2', () => {
});
expect(res.status).toBe(0);
const hosted = sh(
`git -C "${dir}/assets.git" ls-tree -r --name-only pr-assets | grep verify/ || true`,
`git -C "${dir}/assets.git" ls-tree -r --name-only pr-assets/7999-verify | grep verify/ || true`,
)
.stdout.trim()
.split('\n')
@ -1947,8 +1952,8 @@ describe('qwen-triage verify hardening round 2', () => {
expect(comment).not.toContain('02-fake');
expect(comment).toContain('did not pass the hosting checks');
// 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"`);
const out2 = join(dir, 'comment2.md');
const res2 = sh(script, {
cwd: work,
@ -1976,6 +1981,62 @@ describe('qwen-triage verify hardening round 2', () => {
const comment2 = readFileSync(out2, 'utf8');
expect(comment2).toContain('Sandboxed verification');
expect(comment2).not.toContain('Evidence images');
// FIRST RUN on a PR: the remote is valid but the per-PR branch does
// not exist yet, so the clone fails and the orphan-init path runs for
// real. Both scenarios above take the clone-failed branch too, but
// both then fail to push (one seeded the branch, the other has no
// 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`);
expect(
sh(
`git -C "${dir}/assets.git" branch --list pr-assets/7999-verify`,
).stdout.trim(),
).toBe('');
const out3 = join(dir, 'comment3.md');
const res3 = sh(script, {
cwd: work,
env: {
...process.env,
PATH: `${dir}:${process.env.PATH}`,
GH_STUB_OUT: out3,
GH_TOKEN: 'x',
GITHUB_REPOSITORY: 'QwenLM/qwen-code',
RUNNER_TEMP: dir,
GITHUB_STEP_SUMMARY: '/dev/null',
GITHUB_RUN_ID: '79',
GITHUB_RUN_ATTEMPT: '1',
PR_NUMBER: '7999',
RUN_URL: 'u',
VERIFY_RESULT: 'success',
VERDICT: 'pass',
AGENT_VERDICT: 'findings',
SKIP_REASON: '',
PREPARE_FAILURE_PHASE: '',
VERIFY_ASSETS_REMOTE: `${dir}/assets.git`,
},
});
expect(res3.status).toBe(0);
// The branch was created by orphan-init and carries this run's images.
const hosted3 = sh(
`git -C "${dir}/assets.git" ls-tree -r --name-only pr-assets/7999-verify | grep verify/ || true`,
)
.stdout.trim()
.split('\n')
.filter(Boolean);
expect(hosted3.map((p) => p.split('/').pop()).sort()).toEqual([
'01-ab.png',
'04-edge.png',
]);
// Orphan, not a graft onto unrelated history: exactly one commit.
expect(
sh(
`git -C "${dir}/assets.git" rev-list --count pr-assets/7999-verify`,
).stdout.trim(),
).toBe('1');
expect(readFileSync(out3, 'utf8')).toContain('![01-ab](');
} finally {
rmSync(dir, { recursive: true, force: true });
}
@ -2996,6 +3057,269 @@ describe('qwen-triage verify round-3 hardening', () => {
expect(group).toContain("vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true'");
});
// Evidence images: the agent cannot install chromium (runs as `node`,
// `env -i`, fresh HOME, no apt). The tools step installs system deps
// as root; a post-checkout step downloads the browser binary using the
// checkout's own Playwright so the version always matches the lockfile.
it('pre-installs chromium and hands it to the agent', () => {
// System deps only — no browser binary, no version-sensitive download.
const tools = stepIn('verify', 'Install verify runner tools');
expect(tools).toContain('install-deps chromium');
expect(tools).not.toContain('install --with-deps');
expect(tools).toContain('::warning::Chromium system deps install failed');
// 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');
// No hardcoded Playwright pin here either: the apt list must track
// current Playwright so it covers the lockfile-matched binary below.
expect(tools).not.toMatch(/playwright@[\d.]/);
// Browser binary: downloaded after npm ci, and by the CLI of the
// package the capture harness actually imports — never a hardcoded pin
// (M5). This lockfile has TWO Playwright trees: terminal-capture.ts
// imports `playwright`, but node_modules/.bin/playwright (what `npx
// playwright` resolves) is @playwright/test's CLI, which pins a
// different chromium revision. The install must therefore resolve the
// imported package's cli.js from the harness's own directory (the same
// algorithm as its import), not assume npm hoists `playwright` to the
// root — a hoist nothing pins. cli.js is absent from the package's
// exports map, so the workflow resolves the exported package.json and
// joins; binding this assertion to the harness's import keeps the two
// from drifting apart.
const capture = readFileSync(
'integration-tests/terminal-capture/terminal-capture.ts',
'utf8',
);
expect(capture).toMatch(/from 'playwright'/);
const browser = stepIn('verify', 'Install evidence browser');
expect(browser).toContain(
"require.resolve('playwright/package.json', { paths: ['./integration-tests/terminal-capture'] })",
);
expect(browser).toContain('node "$PW_CLI" install chromium');
expect(browser).not.toContain('./node_modules/playwright/cli.js');
expect(browser).not.toContain('npx playwright install chromium');
expect(browser).not.toMatch(/playwright@[\d.]/);
expect(browser).toContain('PLAYWRIGHT_BROWSERS_PATH');
// The CLI runs PR-controlled code ($PW_CLI resolves from the PR's
// node_modules), so it must drop the same runner-injected cache
// credentials the prepare and agent steps unset — a doctored
// playwright package could otherwise write to the Actions cache.
expect(browser).toContain('-u ACTIONS_RUNTIME_TOKEN');
expect(browser).toContain('-u ACTIONS_RUNTIME_URL');
expect(browser).toContain('-u ACTIONS_CACHE_URL');
// Marker is written ONLY inside the success branch (M6): the if
// condition must precede the marker write, and the else branch must
// carry the warning instead. The condition must ALSO require the deps
// marker (M6b): a binary download alone is not enough.
expect(browser).toContain('verify-chromium-deps-ok');
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');
expect(ifIdx).toBeGreaterThan(-1);
expect(markerIdx).toBeGreaterThan(ifIdx);
expect(elseIdx).toBeGreaterThan(markerIdx);
// Best-effort: a failed download must not fail a verification.
expect(browser).not.toContain('exit 1');
// A stale marker from an earlier run on the persistent pool must not
// ride along: prepare clears it before this step writes a fresh one,
// so absence stays a real signal even though RUNNER_TEMP outlives the
// run.
expect(stepIn('verify', 'Install and build PR app')).toContain(
'rm -f "$RUNNER_TEMP/verify-chromium-path"',
);
// The agent is told ONLY when the install actually succeeded, so the
// variable's absence is a real signal rather than a stale promise.
// The guard must be CONDITIONAL (M1b): QWEN_VERIFY_CHROMIUM=1 must
// appear inside the if-block that reads the marker, not unconditionally.
const runStep = stepIn('verify', 'Run verification agent');
expect(runStep).toContain('verify-chromium-path');
expect(runStep).toContain('QWEN_VERIFY_CHROMIUM=1');
// Both variables, not just the flag. Losing the path alone is the
// nastiest arm: the agent is TOLD chromium is available, Playwright
// then looks in the default ~/.cache/ms-playwright instead of the
// shared install, and captures degrade to text-only after a successful
// install — verified by mutation, this test passed without this line.
expect(runStep).toContain('PLAYWRIGHT_BROWSERS_PATH=$CHROMIUM_PATH');
const guard = runStep.indexOf('verify-chromium-path');
const marker = runStep.indexOf('QWEN_VERIFY_CHROMIUM=1');
const path = runStep.indexOf('PLAYWRIGHT_BROWSERS_PATH=$CHROMIUM_PATH');
expect(guard).toBeLessThan(marker);
expect(guard).toBeLessThan(path);
// The if-fi block must wrap the QWEN_ENV+= assignment. Anchor the
// slice at the real `if` keyword: `guard - 20` lands inside the marker
// path, where "ver-if-y-chromium-path" satisfies toContain('if') even
// with the guard deleted.
const runIfIdx = runStep.lastIndexOf('if CHROMIUM_PATH', guard);
expect(runIfIdx).toBeGreaterThan(-1);
const ifBlock = runStep.slice(runIfIdx, marker + 30);
expect(ifBlock).toMatch(/(^|\s)if\s/);
expect(ifBlock).toContain('then');
// The tmux lane is untouched: it has no evidence-image path.
expect(stepIn('tmux-testing', 'Run tmux real-user testing')).not.toContain(
'QWEN_VERIFY_CHROMIUM',
);
// And the skill must stop calling captures optional, must name the
// gate, and must forbid the install the agent cannot complete.
const flat = verifySkill.replace(/\s+/g, ' ');
expect(flat).toContain('Produce these whenever you ran a harness');
expect(flat).toContain('QWEN_VERIFY_CHROMIUM=1');
expect(flat).toContain('Do **not** run `playwright install`');
expect(flat).not.toContain('Optionally `evidence/*.png`');
});
// The browser step's require.resolve + cli.js join is otherwise guarded
// only by a literal string match, so a Playwright bump that relocates
// cli.js would break the download at runtime while that assertion still
// passes. Execute the workflow's exact resolution against the installed
// tree and require it to land on a real cli.js. Skipped only when
// playwright is absent entirely (the require.resolve itself fails),
// mirroring the jq tool-availability guard above.
const resolveHarnessCli =
"process.stdout.write(require('path').join(require('path').dirname(" +
"require.resolve('playwright/package.json', { paths: ['./integration-tests/terminal-capture'] })" +
"), 'cli.js'))";
it.skipIf(spawnSync('node', ['-e', resolveHarnessCli]).status !== 0)(
'resolves the harness Playwright cli.js to a real file',
() => {
const cli = spawnSync('node', ['-e', resolveHarnessCli], {
encoding: 'utf8',
});
expect(cli.status).toBe(0);
expect(cli.stdout).toMatch(/cli\.js$/);
expect(existsSync(cli.stdout)).toBe(true);
},
);
// The publish job must host images on a per-PR branch that can coexist
// with the existing pr-assets/* namespace — a bare `pr-assets` leaf
// cannot be created while `pr-assets/…` children exist.
it('hosts evidence on a per-PR branch, not a bare pr-assets leaf', () => {
const publish = stepIn(
'publish-verify',
'Post verification report comment',
);
expect(publish).toContain('pr-assets/${PR_NUMBER}-verify');
expect(publish).toContain('checkout -q --orphan');
expect(publish).not.toMatch(/--branch pr-assets["\s]/);
});
// Every `pr-assets/*` producer needs a deleter, or its branches are
// permanent: one single-commit branch per verified PR, forever, slowing
// `git ls-remote` and cluttering the branch list for every contributor.
// The verify lane became a second producer and was not added.
it('deletes both pr-assets producers when a PR closes', () => {
const cleanup = readFileSync(
'.github/workflows/web-shell-visuals-cleanup.yml',
'utf8',
);
// Both branch names, built from the same PR number.
expect(cleanup).toContain('pr-assets/web-shell-visuals-${PR_NUMBER}');
expect(cleanup).toContain('pr-assets/${PR_NUMBER}-verify');
// Runs in the base context and never touches PR code.
expect(cleanup).toContain('pull_request_target');
expect(cleanup).not.toContain('actions/checkout');
// One branch missing or one delete failing must not stop the other:
// most PRs produce neither, so absence is the normal case.
expect(cleanup).not.toContain('set -euo pipefail');
expect(cleanup).toContain('continue');
// ...but a real delete failure still has to be visible.
expect(cleanup).toContain('::warning::Failed to delete');
// Execute it against a stubbed gh: the loop must attempt both refs, and
// a missing first branch must not skip the second.
const script = cleanup.match(/run: \|-\n([\s\S]*?)$/)?.[1];
expect(script).toBeTruthy();
const dir = mkdtempSync(join(tmpdir(), 'assets-cleanup-'));
try {
const calls = join(dir, 'gh-calls');
writeFileSync(calls, '');
writeFileSync(
join(dir, 'gh'),
[
'#!/usr/bin/env bash',
'printf "%s\\n" "$*" >> "$GH_CALLS"',
// Only the verify branch exists; the visuals one 404s.
'case "$*" in',
' *web-shell-visuals*) exit 1 ;;',
'esac',
'exit 0',
].join('\n'),
{ mode: 0o755 },
);
const res = spawnSync('bash', ['-c', script.replace(/^ {10}/gm, '')], {
encoding: 'utf8',
env: {
...process.env,
PATH: `${dir}:${process.env.PATH}`,
GH_CALLS: calls,
GH_TOKEN: 'x',
GITHUB_REPOSITORY: 'QwenLM/qwen-code',
PR_NUMBER: '7999',
},
});
expect(res.status).toBe(0);
const lines = readFileSync(calls, 'utf8').trim().split('\n');
const deletes = lines.filter((l) => l.includes('DELETE'));
// The missing visuals branch was probed but never deleted...
expect(
lines.some((l) => l.includes('pr-assets/web-shell-visuals-7999')),
).toBe(true);
expect(deletes.some((l) => l.includes('web-shell-visuals'))).toBe(false);
// ...and the loop carried on to the verify branch and deleted it.
// This is the assertion the whole test exists for: a `set -e` script
// would have exited on the first 404 and never reached here.
expect(deletes.some((l) => l.includes('pr-assets/7999-verify'))).toBe(
true,
);
expect(res.stdout).toContain('Deleted pr-assets/7999-verify');
// Delete-FAILURE path: the verify branch probes OK but its DELETE
// fails (transient 500, or the PAT lost contents:write). The loop
// must set a non-zero exit and surface the warning — without this, a
// future edit dropping `exit "$status"` leaves the job green while
// the orphaned branch this workflow exists to prevent accumulates.
writeFileSync(calls, '');
writeFileSync(
join(dir, 'gh'),
[
'#!/usr/bin/env bash',
'printf "%s\\n" "$*" >> "$GH_CALLS"',
// The verify branch probes OK but its DELETE fails; the visuals
// branch still 404s on the probe.
'case "$*" in',
' *web-shell-visuals*) exit 1 ;;',
' *"-X DELETE"*) exit 1 ;;',
'esac',
'exit 0',
].join('\n'),
{ mode: 0o755 },
);
const res2 = spawnSync('bash', ['-c', script.replace(/^ {10}/gm, '')], {
encoding: 'utf8',
env: {
...process.env,
PATH: `${dir}:${process.env.PATH}`,
GH_CALLS: calls,
GH_TOKEN: 'x',
GITHUB_REPOSITORY: 'QwenLM/qwen-code',
PR_NUMBER: '7999',
},
});
expect(res2.status).toBe(1);
expect(res2.stdout).toContain(
'::warning::Failed to delete pr-assets/7999-verify',
);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
// One budget drives the agent's graceful kill and the watchdog threshold
// that distinguishes that kill from an OOM — both derive from a single
// AGENT_BUDGET_M, so they cannot drift apart. The job limit and the