fix(verify-pr): run the artifact check in the CI publisher (#15938)

* fix(verify-pr): run the artifact check in the CI publisher

The CI publisher validated only the assertion count triple. The failure
ledger and the per-commit mutation claim were checked by
verify-pr-check.mjs, which local rounds run before posting and CI never
did, so the sandboxed lane could publish shapes every local round refuses.
The publisher now runs that checker from its trusted checkout; a refusal,
or a checker that cannot run, leaves the agent verdict untrusted and the
comment says why.

A trusted findings verdict with no failed assertion is now published as
"passed with findings" instead of "not passed". Over the last 30 days, 94
of 150 published findings reports had zero failed assertions, and the
reports among the latest 18 run artifacts that hit this case all
described their findings as non-blocking. The skill now states the
counting rule that headline relies on: a defect reproduced at head is a
failed assertion attributed to the PR, not an expected-red pass.

* fix(verify-pr): bind the headline to the checked report directory

The report and the assertion counts were picked by two independent
finds, so a run that left an earlier round directory holding only
assertions.json could headline a later report with that directory's
zero: "passed with findings" over a report whose own counts carry a
PR-attributed failure. The counts now come from the report's own
directory when there is a report, and the checker must read the same
verdict the run reported beside that report; a mismatch is a refusal.

A test now runs the checker through its relative production path and
pins the checkout layout that path relies on, since every other case
injects an absolute path.

The comments added to the triage workflow are trimmed, and its size
baseline moves to 370027 bytes: the remaining growth is the checker
wiring and its bilingual explanation in the comment body, which the
job has no sibling script or document to hold.

* fix(verify-pr): read only the checker summary and say which check failed

The publisher matched the reported verdict anywhere in the checker log,
and the warnings above the summary can spell out an agent-chosen
evidence filename, so it now reads only the last stdout line. Node's own
errors go to a separate stream that the publisher prints when the
checker cannot run. Warnings are still never printed, and a test now
pins that.

The untrusted-verdict paragraph pointed every state at "errors in the
run log", which two of the three states never print. A refusal still
points at its errors; a verdict that differs from the one beside the
report, and a checker that could not run, each get a sentence of their
own.

The publisher checks out scripts/lib/ as a directory instead of naming
one helper. The production-path test runs the checker from exactly the
paths that checkout fetches, so an import outside them fails the suite
whatever form it takes. That replaces the import-list check, which
could not see side-effect imports.

The local-round design record no longer says the checker merely mirrors
the publisher, in either language. The size baseline moves to the
file's new size, 370588 bytes.

* fix(verify-pr): read the verdict from the report's own directory

The verify job picks the agent verdict by its own search, which can land
on a leftover round directory holding only verdict.txt while the report
comes from a later, complete one. The previous fix flagged that as a
mismatch, blaming a verdict file that was consistent with its own
report. The publisher now reads the verdict from the report's directory
itself, with the same four-word allowlist, so the report, the counts
and the verdict always come from one round, and the mismatch state is
gone.

The vocabulary pin now covers both places the step spells out the four
words: the allowlist and the headline labels. The Chinese note for a
checker that cannot run is asserted too.

The design record no longer promises that every checker rule change
reaches CI in the same commit, since the publish step still validates
the count triple and refuses merge-ready over failures on its own.
Decision 4 now rests on the reason that still holds: legitimate rounds
can lack a screenshot or a cited head. Both languages changed together.

* fix(verify-pr): read verdict.txt once, beside the published report

The verify job read its own copy of verdict.txt from whichever round
directory its search found first, emitted it as agent_verdict, and wrote
it to its step summary, while the publisher read the word again from the
report's directory. With a leftover round directory the two could name
different verdicts in the same run. Drop the verify job's reader and its
output so the publisher is the only reader, and:

- warn when the report's directory has no usable verdict.txt instead of
  blanking the word silently;
- bound the report search to depth 2 so a report.md nested in a round
  cannot stand in for the round's own;
- drop the unreachable "report.md missing" warning.

Tests pin the single reader, the report-only round (no borrowed counts
or word), the nested report, and blocked staying not-passed over an
all-green ledger. The skill's harness-fault rule now states what CI
actually publishes for findings over non-PR failures.
This commit is contained in:
Shaojin Wen authored and GitHub committed 2026-10-31 10:14:55 +08:00
1 parent bd0950774e
commit 10b48ab972
7 files changed
+416 -72

No files matched your search

+1 -1
View File
@@ -47,7 +47,7 @@
5760 qwen-pr-safety-precheck.yml
3243 qwen-review-runner-schedule.yml
27648 qwen-triage-finalize.yml
362377 qwen-triage.yml
369831 qwen-triage.yml
15726 release-chrome-extension.yml
26801 release-managed-agent-reference.yml
13464 release-managed-runtime.yml
+96 -33
View File
@@ -2263,7 +2263,6 @@ jobs:
# empty -> the job broke before deciding (publish reports infra).
verdict: '${{ steps.run.outputs.verdict || steps.prepare.outputs.verdict || steps.pr.outputs.verdict }}'
failure_phase: '${{ steps.prepare.outputs.failure_phase }}'
agent_verdict: '${{ steps.run.outputs.agent_verdict }}'
# Report-footer attribution: the configured model, the runner's CLI
# version, and the content hash of the base-pinned skill that ran.
# Empty when the run step never reached its output writes; the
@@ -4695,23 +4694,9 @@ jobs:
echo "::error::Failed to write the agent output stream (tee exit ${TEE_STATUS}); reporting an infrastructure failure rather than a pass."
fi
echo "verdict=$VERDICT" >> "$GITHUB_OUTPUT"
# The skill writes its verdict as one word in verdict.txt. It is
# output of a run that executed untrusted PR code, so allowlist it
# instead of echoing it into workflow outputs.
AGENT_VERDICT=''
VERDICT_FILE="$(find tmp -maxdepth 2 -type f -path '*-verify-*/verdict.txt' 2>/dev/null | sort | head -1 || true)"
if [ -n "$VERDICT_FILE" ]; then
# Bound the read before the pipeline: with `head` closing early on
# an oversized file, the producer takes SIGPIPE and `pipefail`
# would abort the step instead of ignoring an unusable verdict.
CANDIDATE="$(head -c 4096 "$VERDICT_FILE" | tr -d '[:space:]' | cut -c1-32)"
case "$CANDIDATE" in
merge-ready|findings|blocked|inconclusive) AGENT_VERDICT="$CANDIDATE" ;;
*) echo "::warning::Unrecognized agent verdict in ${VERDICT_FILE}; ignoring." ;;
esac
fi
echo "agent_verdict=$AGENT_VERDICT" >> "$GITHUB_OUTPUT"
# verdict.txt is read only by the publisher, from the uploaded
# report's own directory: a second reader here picked its own
# round directory and could name a different word.
# Emit the attribution collected before the agent started (see the
# re-pin above) — the saved values, never re-derived here: the
@@ -4722,7 +4707,7 @@ jobs:
echo "cli_version=$CLI_VERSION"
echo "skill_hash=$SKILL_HASH"
} >> "$GITHUB_OUTPUT"
echo "verify verdict: $VERDICT agent: ${AGENT_VERDICT:-none} (exit $EXIT_CODE)" >> "$GITHUB_STEP_SUMMARY"
echo "verify verdict: $VERDICT (exit $EXIT_CODE)" >> "$GITHUB_STEP_SUMMARY"
# The gate log's authoritative copy lives root-owned inside the 0700
# root-only home (/flake-gate/log), which no PR-controlled
@@ -5218,10 +5203,10 @@ jobs:
contents: 'read'
pull-requests: 'write'
steps:
# Sparse — the uploader, its helper dependency, and the root
# package.json that marks the uploader's `.js` as ESM (without it the
# uploader only parses on the Node versions that infer module syntax,
# and this job pins no Node).
# Sparse — the uploader, the artifact checker, their helpers, and the
# root package.json that marks the uploader's `.js` as ESM (without it
# the uploader only parses on the Node versions that infer module
# syntax, and this job pins no Node).
- name: 'Checkout the OSS publisher scripts'
uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3
with:
@@ -5235,6 +5220,8 @@ jobs:
/package.json
scripts/upload-aliyun-oss-assets.js
scripts/release-script-utils.js
scripts/verify-pr-check.mjs
scripts/lib/
sparse-checkout-cone-mode: false
persist-credentials: false
@@ -5309,7 +5296,6 @@ jobs:
# cancellations. Verified by executing this step with both values.
PR_NUMBER: '${{ needs.verify.outputs.pr_number || github.event.issue.number }}'
VERDICT: '${{ needs.verify.outputs.verdict }}'
AGENT_VERDICT: '${{ needs.verify.outputs.agent_verdict }}'
VERIFY_MODEL: '${{ needs.verify.outputs.model }}'
VERIFY_CLI_VERSION: '${{ needs.verify.outputs.cli_version }}'
VERIFY_SKILL_HASH: '${{ needs.verify.outputs.skill_hash }}'
@@ -5347,6 +5333,8 @@ jobs:
exit 0
fi
BODY_FILE="${RUNNER_TEMP:-/tmp}/verify-comment.md"
# Set only from the published report's own verdict.txt below.
AGENT_VERDICT=''
# Same embedding discipline as publish-tmux: the report is untrusted
# PR-run output, so it goes inside <details><pre><code> with &, <, >
@@ -5935,9 +5923,27 @@ jobs:
else
collect_and_host_evidence
# Pin the artifact-dir shape and sort: a bare -name search takes
# whatever directory find visits first, which is unordered.
REPORT="$(find verify-results -mindepth 2 -type f -path '*-verify-*/report.md' 2>/dev/null | sort | head -1 || true)"
ASSERTIONS_FILE="$(find verify-results -mindepth 2 -type f -path '*-verify-*/assertions.json' 2>/dev/null | sort | head -1 || true)"
# whatever directory find visits first, which is unordered. Depth
# 2 exactly: a report.md nested in a round (evidence/) sorts
# ahead of the round's own and would stand in for it.
REPORT="$(find verify-results -mindepth 2 -maxdepth 2 -type f -path '*-verify-*/report.md' 2>/dev/null | sort | head -1 || true)"
# The report's own directory is the round: its counts and its
# verdict word headline it, never another directory's. This is
# the only reader of verdict.txt, which untrusted PR code wrote.
if [ -n "$REPORT" ]; then
ASSERTIONS_FILE="$(dirname "$REPORT")/assertions.json"
[ -f "$ASSERTIONS_FILE" ] || ASSERTIONS_FILE=''
AGENT_VERDICT="$(head -c 4096 "$(dirname "$REPORT")/verdict.txt" 2>/dev/null | tr -d '[:space:]' | cut -c1-32 || true)"
case "$AGENT_VERDICT" in
merge-ready|findings|blocked|inconclusive) ;;
*)
echo "::warning::No usable verdict.txt beside the published report.md; the round has no agent verdict."
AGENT_VERDICT=''
;;
esac
else
ASSERTIONS_FILE="$(find verify-results -mindepth 2 -type f -path '*-verify-*/assertions.json' 2>/dev/null | sort | head -1 || true)"
fi
# Exact root-level path, never a find: the staging step writes
# this name last as root, and a same-named file nested inside a
# collected agent artifact dir must not shadow it.
@@ -5974,13 +5980,37 @@ jobs:
echo "::warning::assertions.json missing or inconsistent; not treating it as evidence."
fi
fi
# The checker local rounds run before posting, from this trusted
# checkout, so both paths share one set of rules. It must pass on
# the report's directory and read the verdict taken from it above.
# Only its last stdout line is the summary: warnings above it can
# carry agent-chosen evidence filenames, so they are never echoed.
# node also exits 1 when the script cannot load; stderr says why.
ARTIFACT_CHECK=''
if [ "${VERDICT:-}" = 'pass' ] && [ -n "$REPORT" ] && [ -n "$ASSERT_LINE" ] && [ -n "${AGENT_VERDICT:-}" ]; then
check_log="${RUNNER_TEMP:-/tmp}/verify-pr-check.log"
check_status=0
node "${VERIFY_PR_CHECK:-scripts/verify-pr-check.mjs}" "$(dirname "$REPORT")" > "$check_log" 2> "$check_log.err" || check_status=$?
case "${check_status}:$(tail -n 1 "$check_log")" in
"0: ok: verdict=${AGENT_VERDICT} ("*) ARTIFACT_CHECK=ok ;;
'1: failed: verdict='*)
ARTIFACT_CHECK=refused
grep '^ error ' "$check_log" | head -20 | cut -c1-300 || true
;;
*)
ARTIFACT_CHECK=error
echo "::warning::The artifact check could not run (exit ${check_status}); not trusting the agent verdict."
head -n 8 "$check_log.err" | cut -c1-300 || true
;;
esac
fi
# The agent's own verdict is only honoured for a run that
# actually completed AND left consistent evidence: an early
# `merge-ready` file must not headline a run that then timed out,
# crashed, or produced no report/assertions. Otherwise the
# headline comes from the process outcome.
TRUST_AGENT_VERDICT=false
if [ "${VERDICT:-}" = 'pass' ] && [ -n "$REPORT" ] && [ -n "$ASSERT_LINE" ]; then
if [ "${VERDICT:-}" = 'pass' ] && [ -n "$REPORT" ] && [ -n "$ASSERT_LINE" ] && [ "$ARTIFACT_CHECK" = ok ]; then
case "${AGENT_VERDICT:-}" in
merge-ready) [ "$A_FAIL" = '0' ] && TRUST_AGENT_VERDICT=true ;;
findings|blocked|inconclusive) TRUST_AGENT_VERDICT=true ;;
@@ -5995,9 +6025,18 @@ jobs:
# or produced no usable verdict says nothing about the PR and
# gets incomplete/inconclusive, never a pass or a fail.
if [ "$TRUST_AGENT_VERDICT" = true ]; then
# Verdicts come from harness exits: `findings` with no failed
# assertion is advisory, and the checker has already refused
# failures none of which the ledger attributes to the PR.
case "$AGENT_VERDICT" in
merge-ready) QUAL='✅ passed'; QUAL_ZH='✅ 通过'; HEADLINE='merge-ready (agent verdict)'; HEADLINE_ZH='可合入(agent 判定)' ;;
findings) QUAL='❌ not passed'; QUAL_ZH='❌ 不通过'; HEADLINE='findings reported (agent verdict)'; HEADLINE_ZH='报告了发现(agent 判定)' ;;
findings)
if [ "$A_FAIL" = '0' ]; then
QUAL='✅ passed with findings'; QUAL_ZH='✅ 通过(有发现)'; HEADLINE='non-blocking findings, every scripted assertion passed (agent verdict)'; HEADLINE_ZH='非阻塞发现,脚本断言全部通过(agent 判定)'
else
QUAL='❌ not passed'; QUAL_ZH='❌ 不通过'; HEADLINE='findings reported (agent verdict)'; HEADLINE_ZH='报告了发现(agent 判定)'
fi
;;
blocked) QUAL='❌ not passed'; QUAL_ZH='❌ 不通过'; HEADLINE='blocked (agent verdict)'; HEADLINE_ZH='阻塞(agent 判定)' ;;
inconclusive) QUAL='⚠️ inconclusive'; QUAL_ZH='⚠️ 无法判定'; HEADLINE='the agent could not reach a conclusion'; HEADLINE_ZH='agent 未能得出结论' ;;
esac
@@ -6011,12 +6050,14 @@ jobs:
esac
if [ -n "${AGENT_VERDICT:-}" ]; then
if [ "${VERDICT:-}" = 'pass' ]; then
if [ -z "$REPORT" ]; then
echo "::warning::Agent wrote verdict '${AGENT_VERDICT}' but it was not trusted (report.md missing); reporting the process outcome instead."
elif [ -z "$ASSERT_LINE" ]; then
# A word is only ever read beside a report, so the report
# is present here.
if [ -z "$ASSERT_LINE" ]; then
echo "::warning::Agent wrote verdict '${AGENT_VERDICT}' but it was not trusted (assertions.json missing or inconsistent); reporting the process outcome instead."
else
elif [ "$AGENT_VERDICT" = 'merge-ready' ] && [ "${A_FAIL:-0}" != '0' ]; then
echo "::warning::Agent wrote verdict '${AGENT_VERDICT}' but it was not trusted (assertions recorded ${A_FAIL:-?} failures); reporting the process outcome instead."
else
echo "::warning::Agent wrote verdict '${AGENT_VERDICT}' but it was not trusted (artifact check: ${ARTIFACT_CHECK}); reporting the process outcome instead."
fi
else
echo "::warning::Agent wrote verdict '${AGENT_VERDICT}' but the run did not complete cleanly (process verdict '${VERDICT:-}'); reporting the process outcome instead."
@@ -6067,6 +6108,22 @@ jobs:
FLAKE_LINE_ZH='抖动门:⚠️ error — 发布时门判定缺失或无法识别,按 error 处理'
;;
esac
# The merge-ready-with-failures case has its own paragraph below.
CHECK_NOTE=''
CHECK_NOTE_ZH=''
if [ "$TRUST_AGENT_VERDICT" != true ] &&
! { [ "${AGENT_VERDICT:-}" = 'merge-ready' ] && [ "${A_FAIL:-0}" != '0' ]; }; then
case "$ARTIFACT_CHECK" in
refused)
CHECK_NOTE='its artifacts fail the consistency check (`verify-pr-check.mjs`), so that verdict was not trusted — the workflow run log lists the errors'
CHECK_NOTE_ZH='其产物未通过一致性检查(`verify-pr-check.mjs`),因此该判定未被采信——检查的错误见工作流运行日志'
;;
error)
CHECK_NOTE='the consistency check (`verify-pr-check.mjs`) could not run on its artifacts, so that verdict was not trusted — the workflow run log shows why'
CHECK_NOTE_ZH='一致性检查(`verify-pr-check.mjs`)无法在其产物上运行,因此该判定未被采信——原因见工作流运行日志'
;;
esac
fi
if [ -z "$REPORT" ]; then
MISSING_REPORT_NOTE='No report.md was found in the run artifacts, so the report section is omitted — see the workflow run output.'
echo "::warning::${MISSING_REPORT_NOTE}"
@@ -6107,6 +6164,9 @@ jobs:
if [ "${AGENT_VERDICT:-}" = 'merge-ready' ] && [ "$TRUST_AGENT_VERDICT" != true ] && [ "${A_FAIL:-0}" != '0' ]; then
printf 'The agent reported `merge-ready`, but `assertions.json` recorded %s failures, so that claim was not trusted — see the report for whether these are expected A/B control cells.\n\n' "$A_FAIL"
fi
if [ -n "$CHECK_NOTE" ]; then
printf 'The agent reported `%s`, but %s.\n\n' "$AGENT_VERDICT" "$CHECK_NOTE"
fi
if [ "${FLAKE_VERDICT:-}" = 'flaky' ]; then
printf 'The deterministic flakiness gate re-ran the test files this PR changes and got different outcomes from identical runs%s. A test that can fail with no code changing lands as intermittent red on unrelated PRs, so this run is reported as not passed regardless of the agent verdict — the per-round matrix is in the flakiness gate log below.\n\n' "${AGENT_VERDICT:+ (agent verdict: \`${AGENT_VERDICT}\`)}"
if [ ! -f "$FLAKE_LOG" ]; then
@@ -6132,6 +6192,9 @@ jobs:
if [ "${AGENT_VERDICT:-}" = 'merge-ready' ] && [ "$TRUST_AGENT_VERDICT" != true ] && [ "${A_FAIL:-0}" != '0' ]; then
printf 'agent 报告了 `merge-ready`,但 `assertions.json` 记录了 %s 个失败,因此该判定未被采信——请参阅报告确认这些是否为预期的 A/B 对照单元。\n\n' "$A_FAIL"
fi
if [ -n "$CHECK_NOTE_ZH" ]; then
printf 'agent 报告了 `%s`,但%s。\n\n' "$AGENT_VERDICT" "$CHECK_NOTE_ZH"
fi
if [ "${FLAKE_VERDICT:-}" = 'flaky' ]; then
printf '确定性抖动门将本 PR 改动的测试文件原样重跑了多轮,得到了不一致的结果%s。一个在代码不变时也会失败的测试会以间歇性红灯落在无关的 PR 上,因此无论 agent 判定如何,本次运行按不通过报告——各轮结果矩阵见下方抖动门日志。\n\n' "${AGENT_VERDICT:+(agent 判定:\`${AGENT_VERDICT}\`)}"
if [ ! -f "$FLAKE_LOG" ]; then
+14 -2
View File
@@ -1591,6 +1591,16 @@ reviewer's attention; `blocked` = the central claim failed its A/B or a
regression reproduced; `inconclusive` = budget or environment prevented the
central claim from being tested — say why.
The published headline follows the counts, not the word alone: `findings`
with `fail: 0` is published as passed with findings. A defect you reproduced
at head is therefore a failed assertion attributed to `pr`, never an
expected-red pass — that rule covers control cells, which exist to fail, not
head behaviour the PR got wrong. Measured example: a round reproduced a
regression, asserted that the head showed it, and published `blocked` over
4382 passes and 0 failures. The CI publisher runs `verify-pr-check.mjs` before
it trusts any verdict, so run it on the artifact dir before you finish, in
either path.
### report.md structure
1. **Verdict line first**, with assertion totals and the verified head OID
@@ -1687,8 +1697,10 @@ defect.
the verdict cannot be `merge-ready` — and the publisher enforces exactly
that. When the unexpected failure is in the harness itself (a flaky probe,
a broken A/B control cell) rather than in the PR's code, the verdict is
`inconclusive`, not `findings` — `findings` stamps ❌ on the PR for a
problem it did not cause.
`inconclusive`, not `findings` — the failure is not the PR's, and CI
refuses a `findings` round whose failures name no `pr` cause: it publishes
the round as inconclusive under an untrusted-verdict note (see the
attribution rule below).
**Attribute every failure to a cause, and name the host as its own.**
`assertions.json` carries `failures: [{ id, attribution }]`, one
entry per failed assertion, with `attribution` one of `pr`, `harness` or
+26 -10
View File
@@ -163,13 +163,26 @@ A published verification comment ends with the same attribution shape
excludes by design, so it always runs on the host. A gate there would fire on
every round and turn the override into the normal path, so the check is a
separate mode intended to run inside the boundary.
3. **The local checker mirrors the publisher instead of replacing it.**
Extracting the publish job's assertions into shared code would touch the
workflow and its meta-tests; the rule set is four checks, stable, and stated
in one place. Divergence is the cost, recorded below.
4. **Warnings, not errors, for report shape.** The publisher does not enforce
report structure either. A checker stricter than the publisher would reject
artifact the lane accepts, which teaches maintainers to ignore it.
3. **The publisher runs the local checker.** The checker started as a mirror
of the publish job's inline rules, because the rule set was four stable
checks and sharing it would touch the workflow and its meta-tests. The
mirror did not hold: the failure ledger and the coverage claim were added
here only, so the lane published shapes every local round refuses. The
publish job now runs this script from its trusted checkout (the
default-branch commit, never PR code) and trusts an agent verdict only when
the script passes on the report's own directory. That directory is also
the only place the lane reads `verdict.txt`: the verify job used to read
its own copy from whichever round directory its search found first.
Tightening a rule here therefore tightens CI too.
Loosening one does not always loosen it: the publish step still validates
the count triple itself (the checker runs only on a valid one) and still
refuses `merge-ready` over failures on its own, so those two need the
workflow changed as well.
4. **Warnings, not errors, for report shape.** Report structure, evidence
naming and head-OID citation stay warnings because a legitimate round can
lack them — a documentation-only round has no screenshot to offer — and the
checker cannot tell whether a harness ran. Since the publish job runs this
script, making one of them fatal would refuse those rounds in CI as well.
5. **Attribution is a ledger, not a fifth verdict word.** The tempting fix for
an environment red is a verdict like `merge-ready-with-caveats`, which would
have to be added to the publisher, its case labels and their pin. The
@@ -208,9 +221,12 @@ A published verification comment ends with the same attribution shape
for this head (otherwise it rebuilds per the local install strategy), and
the prepare success line says "verify or refresh its install/build first"
instead of advertising unconditional build reuse.
- Two implementations of the publish rules can drift. A workflow meta-test pins
the checker verdict vocabulary to the publisher case labels, so a fifth word
on one side fails the suite. The CI publisher remains authoritative.
- The checker is on the CI publish path. Tightening a rule refuses verdicts the
lane used to accept, and those rounds publish as inconclusive. The
publish-fidelity meta-tests run the real script, including from exactly the
paths the publish job checks out, so run them with any checker change. A
workflow meta-test still pins the checker verdict vocabulary to the
publisher case labels, so a fifth word on one side fails the suite.
- The PR title is untrusted input reaching a maintainer terminal, so control
characters are dropped before it is printed.
- These are local helpers: the lane pins `.qwen` from the base branch, so the
+3 -3
View File
@@ -51,8 +51,8 @@
1. **`--repo` 必填。** 脚本无法区分维护者克隆与贡献者 fork,而在 fork 上 `origin` 会把同一个 PR 号解析成另一个无关 PR。fail closed 的代价是一个参数,消掉的正是 skill 已经警告的那种失败。
2. **脚本不碰轮次本身,坐标路径也不对隔离设闸。** prepare 只抓元数据、建目录;它不建 worktree、不构建、不执行 PR 代码。但它对树设闸,因为轮次在当前树里跑 A/B 的 head 侧:HEAD 不是 PR head、或有任何未提交改动,测到的就是报告没有点名的代码,所以两者都在写出任何东西之前被拒绝。它也无法判断本进程是否有资格执行那些代码:解析元数据必须访问 `gh`,而边界按设计排除 `gh`,所以它总是在宿主上运行。在那里设闸会让每一轮都触发,并让覆盖开关变成常规路径,因此检查被拆成独立模式,用于在边界内运行。
3. **本地检查器镜像发布侧,而不是替换它。** 把发布 job 里的断言抽成共享代码会动到 workflow 及其 meta 测试;规则只有四条、稳定,且集中在一处。代价是可能漂移,记录在下方。
4. **报告形态只报警告。** 发布侧同样不强制报告结构。比发布侧更严格的检查器会拒绝 lane 本来接受的产物,结果只会教会维护者忽略它。
3. **发布侧运行本地检查器。** 检查器最初只是发布 job 内联规则的镜像,理由是规则只有四条且稳定,共享它会动到 workflow 及其 meta 测试。镜像没能保持:失败归因账本和覆盖声明的校验只加在了这里,于是 lane 发布了每个本地轮次都会拒绝的形态。现在发布 job 从其可信检出(默认分支的提交,绝不是 PR 代码)运行这个脚本,只有当脚本在报告所在目录上通过时才采信 agent 的判定。整个 lane 也只在这个目录读取 `verdict.txt`:verify job 过去会自己读一份,读的是它的搜索先找到的那个轮次目录。因此在这里收紧一条规则,CI 也随之收紧。放宽则不一定:发布步骤仍自己校验计数三元组(只有三元组有效时才会运行检查器),也仍自行拒绝带失败的 `merge-ready`,放宽这两条需要同时改 workflow。
4. **报告形态只报警告。** 报告结构、证据命名和 head OID 引用都只报警告,因为合法的轮次也可能缺少它们——纯文档轮次没有截图可给——而检查器无法判断 harness 是否跑过。既然发布 job 运行的就是这个脚本,把其中任何一条改成致命,会让这些轮次在 CI 中同样被拒绝。
5. **归因是一份账本,不是第五个判定词。** 面对环境红项,最诱人的修法是加一个 `merge-ready-with-caveats` 之类的判定,但那要同时改发布侧、它的 case 标签和对应 pin。判定词仍是四个:红项被改写成它本来就是的那条断言——两臂都因原因 R 失败——账本则记录谁造成了什么。检查器于是读归因,而不是从 prose 里猜。
6. **守卫接管抓取,而不只是比对。** 只比对两个文件的脚本会把手写抓取留在原地,而调用失败后写出的空文件比对起来是干净的。因此调用失败即致命且不写任何文件,比对侧又独立地把「列表变短」判为 dirty。两个半程缺任何一个,洞都还开着。不设重试参数:让维护者重跑一次拒绝是诚实的,而静默的重试预算会掩盖一台连不上 API 的机器。
7. **检查器校验的是覆盖声明与轮次自己给出的枚举是否一致。** 它没有仓库,无法推导提交集合;`productionCommits` 列得不全是 skill 规则的责任,文档里也这么写明。可机器校验的是「声明 vs 所给列表」,而这正是实测那一轮的缺口所在——外加唯一一种原本能通过全部检查的不完整列表:空列表。拒绝它对纯文档轮次没有代价:它该做的是不提出这个声明,而不是对空集合提出声明。
@@ -61,7 +61,7 @@
- 隔离检测只把容器类信号计为边界。内置 macOS 沙箱设置 `SANDBOX=sandbox-exec`,但其默认 profile 仍能读取宿主文件,因此只作为注意事项报告,不算边界。仅有标记也不等于无凭据就绪:CLI 自带的 Docker/Podman 启动器会挂载 gcloud 配置与 ADC 文件并复制模型 key,因此闸门还要求显式声明,并在看到已知信号时拒绝。
- 树闸门只证明源码身份。`dist/` 与 `node_modules/` 都被 gitignore,head 的干净检出仍可能带着更早提交的构建——评审中已复现:prepare 退出 0,而被忽略的构建以新 head 的名义跑旧代码。因此 install/build 的复用是有条件的:轮次必须先确立产物确为本 head 所建(否则按本地安装策略重建),prepare 的成功行也相应写成「先校验或刷新其 install/build」,而不是宣传无条件复用构建。
- 发布规则存在两份实现,可能漂移。一条 workflow 元测试把检查器的判定词表钉到发布侧的 case 标签上,任一侧新增第五个词都会让套件失败。CI 发布侧仍是权威。
- 检查器位于 CI 发布路径上。收紧一条规则,会拒绝 lane 以前接受的判定,这些轮次随之发布为「无法判定」。发布保真元测试运行的是真实脚本,包括从发布 job 检出的那组路径原样运行,所以改动检查器时要一并跑这些测试。另有一条 workflow 元测试仍把检查器的判定词表钉到发布侧的 case 标签上,任一侧新增第五个词都会让套件失败。
- PR 标题是不可信输入且会到达维护者终端,因此在打印前剥离控制字符。
- 这些是本地辅助脚本:lane 从基线分支钉住 `.qwen`,因此引用它们的 skill 文本要在本改动合入后才生效。
- 守卫把每个条目归一为 `id`、一个取值 `updated_at ?? submitted_at` 的时间戳、`body`,以及仅 review 才有的 `state`。只看时间戳回答不了「有没有东西变过」:对本仓库 API 实测,review 对象只有 `submitted_at`、没有 `updated_at`,而编辑正文或 dismiss 这条 review 都不会改动这个时间戳。PR 15388 上的 review 5673309455 报告的 `submitted_at` 是 2026-10-27T19:47:16Z,而它在五小时三十六分钟后的 2026-10-28T01:22:57Z 被 dismiss。时间戳仍是更早的那个值,说明 dismiss 没有动它,并且这条 review 仍报告 4838 字的正文:除了 `state`,没有任何字段记录它被 dismiss 过。正文是否也被改过则无法观测——这正是要点——而正是这次 dismiss 解开了合并闸门。三个列表都返回字符串 `body`、review 还返回字符串 `state`,因此两者都与时间戳一起参与比对;当某个条目没有 body、或某条 review 没有 state 时,整次抓取直接拒绝,而不是当作「什么都没变」去比。代价是快照文件更大——正文按原样保存,这也让轮次无需再抓一次就能读到究竟来了什么。
+270 -17
View File
@@ -7,6 +7,7 @@
import { execFileSync, spawn, spawnSync } from 'node:child_process';
import {
chmodSync,
cpSync,
existsSync,
lstatSync,
mkdirSync,
@@ -76,6 +77,14 @@ function stepIn(jobName, stepName) {
return match?.[0] ?? '';
}
// The paths publish-verify checks out at the trusted ref.
function publisherSparseCheckout() {
const checkout = parse(workflow).jobs['publish-verify'].steps.find(
(s) => s.name === 'Checkout the OSS publisher scripts',
);
return checkout.with['sparse-checkout'].split('\n');
}
function job(name) {
const start = workflow.indexOf(`\n ${name}:`);
if (start === -1) {
@@ -4392,13 +4401,16 @@ describe('qwen-triage verify hardening round 2', () => {
expect(checkout).toContain("ref: '${{ github.sha }}'");
expect(checkout).toContain('persist-credentials: false');
expect(checkout).toContain('sparse-checkout-cone-mode: false');
for (const entry of [
// Whether this set is a complete closure for the checker is proven by
// running the checker from exactly these paths (see "resolves the
// checker at its production path"), not by reading its imports.
expect(publisherSparseCheckout()).toEqual([
'/package.json',
'scripts/upload-aliyun-oss-assets.js',
'scripts/release-script-utils.js',
]) {
expect(checkout).toContain(`\n ${entry}\n`);
}
'scripts/verify-pr-check.mjs',
'scripts/lib/',
]);
});
// A CDN retry loop whose worst case exceeds the 10-minute job cap turns
@@ -5489,18 +5501,24 @@ describe('qwen-triage verify hardening round 2', () => {
expect(publishStep).toContain('all(type == "number" and . >= 0');
});
// The local artifact checker mirrors this step instead of sharing code with
// it (design decision 3), so the mirror needs a pin. The numeric rule is
// already asserted on both sides; the verdict vocabulary was the axis with
// nothing holding the two implementations together, so a fifth word added to
// the publisher would leave the local path silently accepting four.
// This step runs the checker itself (design decision 3), but still spells
// out the verdict vocabulary twice: the allowlist that reads verdict.txt and
// the headline labels. Both must stay the checker's four words, so a fifth
// word on either side fails here.
it('keeps the local checker aligned with the publisher verdicts', () => {
const publishStep = step('Post verification report comment');
const agentCase = publishStep
.slice(publishStep.indexOf('case "$AGENT_VERDICT"'))
const marker = 'case "$AGENT_VERDICT" in';
const allowlist = publishStep
.slice(publishStep.indexOf(marker) + marker.length)
.trim()
.split(')')[0]
.split('|');
expect(new Set(allowlist)).toEqual(new Set(VERDICTS));
const headlineCase = publishStep
.slice(publishStep.lastIndexOf(marker))
.split('esac')[0];
const labels = new Set(
[...agentCase.matchAll(/^\s+([a-z][a-z-]*)\)/gm)].map(
[...headlineCase.matchAll(/^\s+([a-z][a-z-]*)\)/gm)].map(
(match) => match[1],
),
);
@@ -5514,11 +5532,23 @@ describe('qwen-triage verify publish fidelity', () => {
.match(/run: \|-\n([\s\S]*)$/)?.[1]
.replace(/^ {10}/gm, '');
// Render the publisher for a given outcome with a stubbed gh, and return
// the comment body it would post.
const render = (dir, env) => {
// Render the publisher for a given outcome with a stubbed gh and the real
// artifact checker, and return the comment body it would post plus the
// step log. The publisher reads the agent's word only from verdict.txt
// beside the report, so AGENT_VERDICT names what the fixture writes there
// unless VERDICT_TXT says otherwise.
const publish = (dir, env) => {
const out = join(dir, `body-${env.NAME}.md`);
const work = join(dir, 'work');
const verdictFile = join(
work,
'verify-results',
'prA-verify-1',
'verdict.txt',
);
rmSync(verdictFile, { force: true });
const word = env.VERDICT_TXT ?? env.AGENT_VERDICT;
if (word) writeFileSync(verdictFile, `${word}\n`);
const res = spawnSync('bash', ['-c', publishScript()], {
cwd: work,
encoding: 'utf8',
@@ -5540,12 +5570,14 @@ describe('qwen-triage verify publish fidelity', () => {
AGENT_VERDICT: '',
SKIP_REASON: '',
PREPARE_FAILURE_PHASE: '',
VERIFY_PR_CHECK: join(process.cwd(), 'scripts', 'verify-pr-check.mjs'),
...env,
},
});
expect(res.status).toBe(0);
return readFileSync(out, 'utf8');
return { body: readFileSync(out, 'utf8'), log: res.stdout };
};
const render = (dir, env) => publish(dir, env).body;
const fixture = () => {
const dir = mkdtempSync(join(tmpdir(), 'verify-publish2-'));
@@ -5758,6 +5790,10 @@ describe('qwen-triage verify publish fidelity', () => {
// verdict-less run says nothing about the PR and must render as
// incomplete/inconclusive, never as a pass or a fail.
const dir = fixture();
// A failed assertion the PR caused is what may fail the PR; findings
// over an all-green ledger are advisory.
const PR_RED =
'{"pass":9,"fail":1,"total":10,"failures":[{"id":"head-regression","attribution":"pr"}]}';
try {
const ARMS = [
[
@@ -5765,11 +5801,20 @@ describe('qwen-triage verify publish fidelity', () => {
'\u2705 passed \u2014 merge-ready (agent verdict)',
'\u2705 \u901a\u8fc7 \u00b7 \u53ef\u5408\u5165\uff08agent \u5224\u5b9a\uff09',
],
[
{ VERDICT: 'pass', AGENT_VERDICT: 'findings' },
'\u2705 passed with findings \u2014 non-blocking findings, every scripted assertion passed (agent verdict)',
'\u2705 \u901a\u8fc7\uff08\u6709\u53d1\u73b0\uff09 \u00b7 \u975e\u963b\u585e\u53d1\u73b0\uff0c\u811a\u672c\u65ad\u8a00\u5168\u90e8\u901a\u8fc7\uff08agent \u5224\u5b9a\uff09',
],
[
{ VERDICT: 'pass', AGENT_VERDICT: 'findings' },
'\u274c not passed \u2014 findings reported (agent verdict)',
'\u274c \u4e0d\u901a\u8fc7 \u00b7 \u62a5\u544a\u4e86\u53d1\u73b0\uff08agent \u5224\u5b9a\uff09',
PR_RED,
],
// Unlike findings, blocked stays not-passed over an all-green
// ledger on purpose: the measured blocked-at-zero round (SKILL.md)
// was a real regression counted as a pass.
[
{ VERDICT: 'pass', AGENT_VERDICT: 'blocked' },
'\u274c not passed \u2014 blocked (agent verdict)',
@@ -5808,7 +5853,17 @@ describe('qwen-triage verify publish fidelity', () => {
];
const seenEn = new Set();
const seenZh = new Set();
ARMS.forEach(([env, en, zh], i) => {
ARMS.forEach(([env, en, zh, assertions], i) => {
writeFileSync(
join(
dir,
'work',
'verify-results',
'prA-verify-1',
'assertions.json',
),
assertions ?? '{"pass":10,"fail":0,"total":10}',
);
const body = render(dir, { NAME: `hl${i}`, AGENT_VERDICT: '', ...env });
expect(body).toContain(`**Sandboxed verification: ${en}**`);
expect(body).toContain(
@@ -5835,6 +5890,204 @@ describe('qwen-triage verify publish fidelity', () => {
}
});
// The publisher used to check only the count triple, so CI published the
// shapes every local round refuses: findings over environment-only reds,
// and blocked over a harness fault.
it('does not trust a verdict the artifact checker refuses', () => {
const dir = fixture();
const assertionsFile = join(
dir,
'work',
'verify-results',
'prA-verify-1',
'assertions.json',
);
const UNTRUSTED =
'⚠️ inconclusive — completed without a usable structured verdict';
try {
writeFileSync(
assertionsFile,
'{"pass":8,"fail":2,"total":10,"failures":[{"id":"head","attribution":"environment"},{"id":"base-aa","attribution":"environment"}]}',
);
const envOnly = publish(dir, { NAME: 'env', AGENT_VERDICT: 'findings' });
expect(envOnly.body).toContain(UNTRUSTED);
expect(envOnly.body).not.toContain('❌');
expect(envOnly.body).toContain(
'The agent reported `findings`, but its artifacts fail the consistency check (`verify-pr-check.mjs`), so that verdict was not trusted — the workflow run log lists the errors.',
);
expect(envOnly.body).toContain(
'agent 报告了 `findings`,但其产物未通过一致性检查(`verify-pr-check.mjs`),因此该判定未被采信——检查的错误见工作流运行日志。',
);
expect(envOnly.log).toContain(
' error findings with 2 failed assertions and none attributed to the PR',
);
expect(envOnly.log).toContain('(artifact check: refused)');
// This round's checker also warned (no PNG, verdict word not in the
// report); warnings can name agent-chosen files and stay unprinted.
expect(envOnly.log).not.toContain(' warn ');
writeFileSync(
assertionsFile,
'{"pass":9,"fail":1,"total":10,"failures":[{"id":"probe","attribution":"harness"}]}',
);
const blocked = render(dir, { NAME: 'blk', AGENT_VERDICT: 'blocked' });
expect(blocked).toContain(UNTRUSTED);
expect(blocked).toContain('The agent reported `blocked`');
// A checker that cannot load is not a clean check.
const missing = publish(dir, {
NAME: 'nochk',
AGENT_VERDICT: 'findings',
VERIFY_PR_CHECK: join(dir, 'absent.mjs'),
});
expect(missing.body).toContain(UNTRUSTED);
expect(missing.body).toContain(
'but the consistency check (`verify-pr-check.mjs`) could not run on its artifacts, so that verdict was not trusted — the workflow run log shows why.',
);
expect(missing.body).toContain(
'agent 报告了 `findings`,但一致性检查(`verify-pr-check.mjs`)无法在其产物上运行,因此该判定未被采信——原因见工作流运行日志。',
);
expect(missing.body).not.toContain('lists the errors');
expect(missing.log).toContain('The artifact check could not run');
expect(missing.log).toContain('Cannot find module');
// merge-ready over failures keeps its own explanation, not both.
writeFileSync(
assertionsFile,
'{"pass":9,"fail":1,"total":10,"failures":[{"id":"x","attribution":"pr"}]}',
);
const mr = render(dir, { NAME: 'mr', AGENT_VERDICT: 'merge-ready' });
expect(mr).toContain(
'recorded 1 failures, so that claim was not trusted',
);
expect(mr).not.toContain('verify-pr-check.mjs');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
// REPORT, the counts and the verdict were picked by independent searches
// (the verdict in another job), so a run that left an earlier round
// directory behind could headline the later report with that directory's
// zero or verdict word.
it("binds the headline to the report's own directory", () => {
const dir = fixture();
const results = join(dir, 'work', 'verify-results');
const round = join(results, 'prA-verify-1');
try {
// The leftover sorts first and holds a complete green round's files.
mkdirSync(join(results, 'prA-verify-0'));
writeFileSync(
join(results, 'prA-verify-0', 'assertions.json'),
'{"pass":10,"fail":0,"total":10}',
);
writeFileSync(
join(results, 'prA-verify-0', 'verdict.txt'),
'merge-ready\n',
);
writeFileSync(
join(round, 'assertions.json'),
'{"pass":9,"fail":1,"total":10,"failures":[{"id":"x","attribution":"pr"}]}',
);
const mixed = render(dir, { NAME: 'mix', AGENT_VERDICT: 'findings' });
expect(mixed).toContain(
'❌ not passed — findings reported (agent verdict)',
);
expect(mixed).toContain(
'Scripted assertions: 9 passed · 1 failed · 10 total',
);
expect(mixed).not.toContain('merge-ready (agent verdict)');
expect(mixed).not.toContain('verify-pr-check.mjs');
// A round holding only its report has no counts and no word, and the
// leftover must lend it neither.
rmSync(join(round, 'assertions.json'));
const bare = publish(dir, { NAME: 'bare', AGENT_VERDICT: '' });
expect(bare.body).toContain(
'⚠️ inconclusive — completed without a usable structured verdict',
);
expect(bare.body).not.toContain('Scripted assertions:');
expect(bare.log).toContain(
'::warning::No usable verdict.txt beside the published report.md',
);
// A report.md nested inside the round sorts ahead of the round's own.
writeFileSync(
join(round, 'assertions.json'),
'{"pass":10,"fail":0,"total":10}',
);
mkdirSync(join(round, 'evidence'));
writeFileSync(join(round, 'evidence', 'report.md'), '## nested\n');
const nested = render(dir, {
NAME: 'nested',
AGENT_VERDICT: 'merge-ready',
});
expect(nested).toContain('✅ passed — merge-ready (agent verdict)');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
// verdict.txt has one reader. The verify job used to read its own copy
// from whichever round directory its search found first and hand the
// publisher that word.
it('reads verdict.txt only beside the published report', () => {
const doc = parse(workflow);
const code = (s) =>
(s.run ?? '')
.split('\n')
.filter((line) => !/^\s*#/.test(line))
.join('\n');
const readers = Object.entries(doc.jobs).flatMap(([name, job]) =>
(job.steps ?? [])
.filter((s) => code(s).includes('verdict.txt'))
.map((s) => `${name}/${s.name}`),
);
expect(readers).toEqual([
'publish-verify/Post verification report comment',
]);
expect(doc.jobs.verify.outputs).not.toHaveProperty('agent_verdict');
const post = doc.jobs['publish-verify'].steps.find(
(s) => s.name === 'Post verification report comment',
);
expect(post.env).not.toHaveProperty('AGENT_VERDICT');
});
// Every other case injects an absolute checker path. The production value
// is relative to the job's checkout, so pin the layout it relies on and run
// it from exactly the paths that checkout fetches: an import of anything
// outside them fails here, whatever form the import takes.
it('resolves the checker at its production path', () => {
const wf = parse(workflow);
const steps = wf.jobs['publish-verify'].steps;
const checkout = steps.find(
(s) => s.name === 'Checkout the OSS publisher scripts',
);
const post = steps.find(
(s) => s.name === 'Post verification report comment',
);
expect(checkout.with.path).toBeUndefined();
expect(post['working-directory']).toBeUndefined();
expect(wf.jobs['publish-verify'].defaults).toBeUndefined();
const dir = fixture();
const work = join(dir, 'work');
try {
for (const entry of publisherSparseCheckout()) {
const rel = entry.replace(/^\//, '');
cpSync(rel, join(work, rel), { recursive: true });
}
const body = render(dir, {
NAME: 'rel',
AGENT_VERDICT: 'findings',
VERIFY_PR_CHECK: '',
});
expect(body).toContain('✅ passed with findings');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it.each([
['pass', { VERDICT: 'pass', AGENT_VERDICT: 'merge-ready' }],
['timeout', { VERDICT: 'timeout', AGENT_VERDICT: '' }],
+6 -6
View File
@@ -7,13 +7,13 @@
*/
/**
* Applies the publisher's consistency rules to a local `/verify-pr` round.
* Applies the publisher's consistency rules to a `/verify-pr` round.
*
* In CI the publish job refuses an artifact whose verdict contradicts its own
* assertion counts, so a `merge-ready` report carrying `fail: 7` degrades to
* "no usable structured verdict". A local round has no publisher: the skill
* asks the maintainer to "keep the counts honest yourself", which is the one
* step of that path a machine should own.
* The CI publish job runs this file from its trusted checkout and does not
* trust a verdict it refuses, so a `merge-ready` report carrying `fail: 7`
* degrades to "no usable structured verdict". A local round has no publisher,
* so the skill runs it before posting: the one step of that path a machine
* should own.
*
* Usage:
* node scripts/verify-pr-check.mjs <artifact-dir>