Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 89 additions & 10 deletions .github/workflows/qwen-triage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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')")"
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
# 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
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -3320,19 +3399,19 @@ 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
echo "::warning::Failed to push evidence images; posting a text-only report." >&2
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
Expand Down
44 changes: 30 additions & 14 deletions .github/workflows/web-shell-visuals-cleanup.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
fi
done
exit "$status"
10 changes: 8 additions & 2 deletions .qwen/skills/terminal-capture/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

```
Expand Down Expand Up @@ -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.
Expand Down
38 changes: 27 additions & 11 deletions .qwen/skills/verify-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading