From 2c64ebe980e7d0d9d7a97cf1de6dc47014bc3449 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 21 Aug 2026 04:54:07 +0000 Subject: [PATCH] feat(autofix): audit the approach instead of stopping on growth-budget breach (#9262) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(autofix): audit the approach instead of stopping on growth-budget breach A growth-budget breach no longer escalates to a maintainer handoff that stops the takeover. The breach now makes the round a growth-audit round: the agent audits the PR's approach on two axes — KISS (name a simpler alternative or prove each piece load-bearing) and minimal change (every hunk traces to the problem, an accepted finding, or a failing check) — and records a machine-readable verdict that the verification gate requires. sound re-arms the counting window at the current size and the loop keeps solving; drift simplifies first, then continues; conflict is the only growth path to a human, parked idempotently until a trusted human responds. The old divergence ladder (over budget for N rounds and not shrinking → stop) terminated takeovers whose remaining work could still fit: the growth it punished was protocol-mandated pinned tests (#9213 stalled at round 5 with two small Criticals left). A size signal now triggers a judgment, never a stop. Design: docs/design/autofix-growth-audit.md * fix(autofix): update the artifact-list pin for the growth-audit.json upload entry * fix(autofix): surface conflict verdicts past the failure.md exits and strip verdict forgery channels (#9262) * fix(autofix): harden the growth-audit verdict pipeline and park wake set (#9262) * fix(autofix): close the verdict-pipeline forgeries and loop-generated wake entrances (#9262) * fix(ci): drop the retired divergence rationale records (af-046/af-047) from qwen-autofix.md --------- Co-authored-by: qwen-code-dev-bot --- .../run-autofix-review-verification.sh | 361 ++- .github/workflows/qwen-autofix.md | 83 - .github/workflows/qwen-autofix.yml | 498 ++++- .qwen/skills/autofix/SKILL.md | 57 +- docs/design/autofix-growth-audit.md | 398 ++++ scripts/tests/package-scripts.test.js | 2 +- scripts/tests/qwen-autofix-workflow.test.js | 1944 ++++++++++++++--- 7 files changed, 2825 insertions(+), 518 deletions(-) create mode 100644 docs/design/autofix-growth-audit.md diff --git a/.github/scripts/run-autofix-review-verification.sh b/.github/scripts/run-autofix-review-verification.sh index 5dd184bbb1..8f3c3c5bef 100755 --- a/.github/scripts/run-autofix-review-verification.sh +++ b/.github/scripts/run-autofix-review-verification.sh @@ -45,6 +45,42 @@ git config --file "${GIT_CONFIG_GLOBAL}" safe.directory "$(pwd)" if [ -s /etc/gitconfig ]; then echo "::notice::/etc/gitconfig exists but is bypassed by the gate's GIT_CONFIG_SYSTEM redirect — replicate any setting the checks need via per-job env." fi +# Two more inherited knobs steer EXECUTION itself, and neither has a +# legitimate setter: BASH_ENV names a file every non-interactive bash +# sources at STARTUP — a body-side unset is one hop late (bash sources a +# plant before line 1), so the verify steps pin it empty at step level AND +# launch this gate through their env -i clean child; the unset here keeps +# the gate's own bash children clean too. BITE_RUNNER selects the bite +# check's runner command, which executes unwrapped with the gate's full +# environment. Strip them with the GIT_* class. +unset BASH_ENV BITE_RUNNER +# The verdict variables are GATE state, not inherited state: a plant of +# AUDIT_VERDICT_RECORDED=true plus a verdict from an earlier step would +# otherwise ride the every-exit re-append back into this step's outputs on +# paths where the gate validated nothing. +unset AUDIT_VERDICT AUDIT_VERDICT_RECORDED +# The runner backs $GITHUB_ENV/$GITHUB_PATH/$GITHUB_STEP_SUMMARY with files +# under $RUNNER_TEMP/_runner_file_commands/ that it reads back at step end. +# The channel strip below removes the VARIABLES from the checks, but the +# files stay discoverable under the inherited (predictable) $RUNNER_TEMP +# and stay WRITABLE — a check that appends there plants environment into +# every later step of this job, the PAT-bearing one included (discovery +# verified on a live runner). Lock the files for the lifetime of this +# step. The $GITHUB_OUTPUT backing file is the ONE exception: the gate +# must keep writing it, and forges against it lose to the every-exit +# re-append below plus the conclusion gate Finalize verification applies +# to outcome. The directory itself stays writable on purpose: the runner +# creates the NEXT step's backing files there at step start, and a locked +# directory would stall every later step of the job; the residual +# rename-over (create + rename onto a locked file) is documented in the +# design doc instead of bought at that price. +if [[ -n "${GITHUB_OUTPUT:-}" && -d "${RUNNER_TEMP}/_runner_file_commands" ]]; then + for _rfc in "${RUNNER_TEMP}/_runner_file_commands"/*; do + if [[ -f "${_rfc}" && "${_rfc}" != "${GITHUB_OUTPUT}" ]]; then + chmod a-w "${_rfc}" 2> /dev/null || true + fi + done +fi # Record whether the agent left a commit FIRST — this is a ref-only # diff, so it runs before the failure.md early-exits and covers an @@ -60,83 +96,6 @@ if [[ "${committed_rc}" -eq 1 ]]; then echo "committed=true" >> "${GITHUB_OUTPUT}" fi -if [[ -f "${WORKDIR}/failure.md" && -n "$(git status --porcelain)" ]]; then - echo "❌ Agent wrote failure.md after leaving a dirty workspace:" - git status --short - cat "${WORKDIR}/failure.md" - echo "outcome=failed" >> "${GITHUB_OUTPUT}" - exit 1 -fi - -if [[ -f "${WORKDIR}/failure.md" ]]; then - echo "🛑 Agent aborted intentionally:" - cat "${WORKDIR}/failure.md" - echo "outcome=failed" >> "${GITHUB_OUTPUT}" - exit 1 -fi - -# A handoff claims the round changed NOTHING — dirt beside it is a -# brake-violating partial patch (otherwise reported as a clean stop and -# discarded silently with the runner), and untracked leftovers would trip -# the NEXT round's dirty assert on the persistent pool. The ref-level -# commit diff below is blind to both. Non-retryable like failure.md+dirty -# above (a retryable rejection would engage the repair pass, which deletes -# handoff.md and may commit against the brake), but under its OWN outcome: -# outcome=failed would make the report step dress the rejection as a -# failed FIX ("could not produce a passing fix", or a stale-base retry -# promise) when no fix existed — the report step gives this shape its own -# honest headline. -if [[ -s "${WORKDIR}/handoff.md" && -n "$(git status --porcelain)" ]]; then - echo "❌ Agent wrote handoff.md after leaving a dirty workspace:" - git status --short - sed 's/::/;;/g' "${WORKDIR}/handoff.md" - echo "outcome=dirty_handoff" >> "${GITHUB_OUTPUT}" - exit 1 -fi - -# The committed sibling of the brake violation above: the round HAS a commit -# beside handoff.md. Judged by dirt alone it slips both guards — the dirty -# check sees a clean tree, and the no-commit handoff branch below requires -# an unchanged ref — so it would reach the structural checks, where -# reject_fix defaults to retryable and the repair pass deletes -# handoff.md and may commit AGAIN against the brake's stop. Non-retryable -# under its OWN outcome: a commit DID happen, so the dirty-handoff headline -# claiming nothing was committed would misreport it. Same reasoning as the -# dirty guard otherwise. -if [[ -s "${WORKDIR}/handoff.md" && "${committed_rc:-0}" -eq 1 ]]; then - echo "❌ Agent wrote handoff.md but the round HAS a commit — a brake violation:" - git log --oneline "origin/${BRANCH}..${BRANCH}" - sed 's/::/;;/g' "${WORKDIR}/handoff.md" - echo "outcome=committed_handoff" >> "${GITHUB_OUTPUT}" - exit 1 -fi - -# No-commit brake handoff, classified BEFORE the structural checks below: -# those judge the PR's OWN diff (core rebuild, schema freshness, contracts) -# and reject_fix on failure, and the growth brake fires on exactly the red -# PRs whose diff trips them. A compliant handoff commits nothing, so -# running the checks first would reclassify it as a retryable failure — -# the repair pass would delete handoff.md and commit against the brake's -# stop. A handoff claims nothing (acted=false, deferred to a human), so -# the checks' false-no-action rationale does not apply. failure.md -# coexistence keeps the failed classification via the exits above. -if git diff --quiet "origin/${BRANCH}...${BRANCH}" \ - && [[ -s "${WORKDIR}/handoff.md" ]]; then - echo "🤝 Branch unchanged with a handoff — the agent stopped under instruction and deferred this item to a human:" - # Agent-written content: a line-start `::` would be parsed as a workflow - # command (::error::, ::add-mask::), the same reason 'Show run artifacts' - # neutralizes these files. - sed 's/::/;;/g' "${WORKDIR}/handoff.md" - echo "outcome=handoff" >> "${GITHUB_OUTPUT}" - exit 0 -fi - -# Convention: hooks are severed at EVERY host checkout of the PR -# branch (no secret sits in this step's env, but a post-checkout -# hook still runs branch code on the host). -git config core.hooksPath /dev/null -git checkout "${BRANCH}" - GATE_LOG="${WORKDIR}/gate-output.log" : > "${GATE_LOG}" rm -f "${GATE_LOG}.bite" @@ -153,6 +112,10 @@ reject_fix() { # step means the gate itself crashed, so losing the detail file must not turn # a deterministic rejection into an infrastructure retry. echo "outcome=failed" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi if [[ "${preexisting}" == 'true' ]]; then # NOT retryable: the repair agent is only allowed to amend this round's # fix, and a failure that exists without the fix is outside that boundary @@ -185,6 +148,196 @@ reject_fix() { echo "::warning::could not write the gate rejection detail; the verdict stands." exit 1 } +# Last-writer binding for the audit verdict: the record below happens +# BEFORE the branch's build/tests run, and a check can still discover the +# step-output FILE through the inherited $RUNNER_TEMP (the strip removes +# the variable, not the backing file) and append its own audit_verdict — +# step outputs are last-write-wins. EVERY exit therefore re-appends the +# validated verdict INLINE (no function call: gate snippets extracted by +# the contract suite must stay executable standalone), including the exits +# that run after branch checks (a forge appended mid-check loses to the +# exit's rewrite) — so the gate's copy outwrites any forged append. The +# flag gates it: a verdict rejected BEFORE its record (missing, malformed, +# or a routing violation) never surfaces. kiss_audit rides the same +# discipline (recorded above, re-appended unconditionally at every exit). +# Defended control-bit surface: kiss_audit reaches every later step ONLY +# through this output — recorded HERE, before any branch code runs in this +# step, and re-appended at every exit below with the same last-writer +# discipline as the verdict. A consumer that read steps.prepare's copy +# directly would route the bit around the gate's defenses. +echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + +# Growth-audit verdict gate: a round tagged KISS_AUDIT (its counting window +# is over the growth budget) must carry the audit's machine-readable verdict +# — the audit IS the round's judgment of the over-budget approach, and a +# round that skipped it must not push (the rubber-stamp hole by absence). +# Sits BEFORE the failure.md early-exits below: a conflict round stops +# BLOCKED via failure.md, and its verdict must be validated and surfaced to +# GITHUB_OUTPUT before that exit writes outcome=failed — otherwise the +# conflict trail marker never posts and the idempotent park never engages. +# Also before the build/schema/footprint checks AND the no-commit/no-op +# exits further down: the verdict is required even for a no-op audit round +# whose verdict is sound with nothing left to fix. Malformed is agent +# misbehavior, not a build problem — NON-retryable, so the repair pass is +# never invoked and the next scan simply re-runs the audit. +if [[ "${KISS_AUDIT:-false}" == 'true' ]]; then + AUDIT_VERDICT='' + if [[ -f "${WORKDIR}/growth-audit.json" ]]; then + # Slurp so the document COUNT is part of validation: the per-document + # parse accepted a valid first document followed by one jq errors on + # (or shape-filters out) on the FIRST document's verdict — the gate's + # contract is a single JSON document, so reject every multi-document + # stream. + AUDIT_VERDICT="$(jq -rs ' + if length != 1 then empty else .[0] + | select((.verdict // "") | IN("sound", "drift", "conflict")) + | select((.kiss.result // "") | IN("pass", "fail")) + | select((.minimal_change.result // "") | IN("pass", "fail")) + | select((.verdict != "sound") + or ((.kiss.result == "pass") and (.minimal_change.result == "pass"))) + | select((.verdict != "drift") + or ((.kiss.result == "fail") or (.minimal_change.result == "fail"))) + | .verdict end' "${WORKDIR}/growth-audit.json" 2> /dev/null || true)" + fi + # Anchor the parsed value (defense in depth now that slurp rejects + # multi-document streams outright). + [[ "${AUDIT_VERDICT}" =~ ^(sound|drift|conflict)$ ]] || AUDIT_VERDICT='' + if [[ -z "${AUDIT_VERDICT}" ]]; then + { + echo "Growth-audit round (this counting window is over its growth budget) without a valid growth-audit.json verdict." + echo "The audit must run BEFORE any edit this round, and the verdict file must be a single JSON document carrying verdict sound|drift|conflict plus kiss.result and minimal_change.result each pass|fail, consistent with the taxonomy (sound requires both axes pass; drift requires at least one axis fail). Re-run the audit and produce the file; do not push without it." + } >> "${GATE_LOG}" + reject_fix 'growth-audit round missing a valid growth-audit.json verdict (audit skipped or malformed)' 'false' 'false' + fi + echo "🔎 growth-audit verdict: ${AUDIT_VERDICT}" + # Conflict routing is enforced HERE, not by convention: a conflict verdict + # must STOP BLOCKED with a handoff (the only growth path to a human). A + # round that kept fixing and committed would otherwise clear the gate like + # sound/drift, push the contested code, and park the next scan on a + # handoff question that was never asked. NON-retryable: re-audit, don't + # repair. + if [[ "${AUDIT_VERDICT}" == 'conflict' && ! -f "${WORKDIR}/failure.md" && ! -s "${WORKDIR}/handoff.md" ]]; then + reject_fix 'growth-audit verdict is conflict but the round did not stop with a handoff; conflict must STOP BLOCKED (no push)' 'false' 'false' + fi + # The no-push half of this routing is enforced at the success exit + # below: a repair pass re-auditing to conflict LEGITIMATELY runs behind + # the first pass's commit (committed_rc=1), so the push shape cannot be + # refused here without refusing it — the refusal sits at the push + # boundary itself. + # Record the verdict the GATE validated, for the report step to consume + # via the step output. The report must NOT re-read the file itself: the + # branch's own build/tests run as the runner user after this point and + # WORKDIR is a predictable path they can write — the validated verdict is + # the only verdict that may reach the trail marker and the re-arm. + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + AUDIT_VERDICT_RECORDED='true' +fi + +if [[ -f "${WORKDIR}/failure.md" && -n "$(git status --porcelain)" ]]; then + echo "❌ Agent wrote failure.md after leaving a dirty workspace:" + git status --short + cat "${WORKDIR}/failure.md" + echo "outcome=failed" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi + exit 1 +fi + +if [[ -f "${WORKDIR}/failure.md" ]]; then + echo "🛑 Agent aborted intentionally:" + cat "${WORKDIR}/failure.md" + echo "outcome=failed" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi + exit 1 +fi + +# These three handoff classifications skip a growth-audit CONFLICT verdict: +# that round has its own routing — the verdict gate (stop enforced here) and +# the push-boundary refusal at the success exit — and it must land +# outcome=failed so the conflict trail marker posts and the park engages, +# never the clean outcome=handoff. A plain (non-audit) handoff still takes +# these. +# A handoff claims the round changed NOTHING — dirt beside it is a +# brake-violating partial patch (otherwise reported as a clean stop and +# discarded silently with the runner), and untracked leftovers would trip +# the NEXT round's dirty assert on the persistent pool. The ref-level +# commit diff below is blind to both. Non-retryable like failure.md+dirty +# above (a retryable rejection would engage the repair pass, which deletes +# handoff.md and may commit against the brake), but under its OWN outcome: +# outcome=failed would make the report step dress the rejection as a +# failed FIX ("could not produce a passing fix", or a stale-base retry +# promise) when no fix existed — the report step gives this shape its own +# honest headline. +if [[ -s "${WORKDIR}/handoff.md" && -n "$(git status --porcelain)" \ + && "${AUDIT_VERDICT:-}" != 'conflict' ]]; then + echo "❌ Agent wrote handoff.md after leaving a dirty workspace:" + git status --short + sed 's/::/;;/g' "${WORKDIR}/handoff.md" + echo "outcome=dirty_handoff" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi + exit 1 +fi + +# The committed sibling of the brake violation above: the round HAS a commit +# beside handoff.md. Judged by dirt alone it slips both guards — the dirty +# check sees a clean tree, and the no-commit handoff branch below requires +# an unchanged ref — so it would reach the structural checks, where +# reject_fix defaults to retryable and the repair pass deletes +# handoff.md and may commit AGAIN against the brake's stop. Non-retryable +# under its OWN outcome: a commit DID happen, so the dirty-handoff headline +# claiming nothing was committed would misreport it. Same reasoning as the +# dirty guard otherwise. +if [[ -s "${WORKDIR}/handoff.md" && "${committed_rc:-0}" -eq 1 \ + && "${AUDIT_VERDICT:-}" != 'conflict' ]]; then + echo "❌ Agent wrote handoff.md but the round HAS a commit — a brake violation:" + git log --oneline "origin/${BRANCH}..${BRANCH}" + sed 's/::/;;/g' "${WORKDIR}/handoff.md" + echo "outcome=committed_handoff" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi + exit 1 +fi + +# No-commit brake handoff, classified BEFORE the structural checks below: +# those judge the PR's OWN diff (core rebuild, schema freshness, contracts) +# and reject_fix on failure, and the growth brake fires on exactly the red +# PRs whose diff trips them. A compliant handoff commits nothing, so +# running the checks first would reclassify it as a retryable failure — +# the repair pass would delete handoff.md and commit against the brake's +# stop. A handoff claims nothing (acted=false, deferred to a human), so +# the checks' false-no-action rationale does not apply. failure.md +# coexistence keeps the failed classification via the exits above. +if git diff --quiet "origin/${BRANCH}...${BRANCH}" \ + && [[ -s "${WORKDIR}/handoff.md" ]] \ + && [[ "${AUDIT_VERDICT:-}" != 'conflict' ]]; then + echo "🤝 Branch unchanged with a handoff — the agent stopped under instruction and deferred this item to a human:" + # Agent-written content: a line-start `::` would be parsed as a workflow + # command (::error::, ::add-mask::), the same reason 'Show run artifacts' + # neutralizes these files. + sed 's/::/;;/g' "${WORKDIR}/handoff.md" + echo "outcome=handoff" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi + exit 0 +fi + +# Convention: hooks are severed at EVERY host checkout of the PR +# branch (no secret sits in this step's env, but a post-checkout +# hook still runs branch code on the host). +git config core.hooksPath /dev/null +git checkout "${BRANCH}" baseline_also_fails() { # A deterministic rejection is only chargeable to this round if the same # check passes WITHOUT the round's commits. Measured counterexample, run @@ -232,7 +385,7 @@ baseline_also_fails() { local ab_log="${GATE_LOG}.baseline" : > "${ab_log}" rc=0 - if ! "$@" >> "${ab_log}" 2>&1; then + if ! strip_runner_channels "$@" >> "${ab_log}" 2>&1; then rc=1 fi git restore -- . 2>> "${GATE_LOG}" || true @@ -253,6 +406,10 @@ baseline_also_fails() { tail -c 3000 "${GATE_LOG}" 2> /dev/null echo '````' } > "${WORKDIR}/gate-rejection.md" || true + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi exit 1 fi # Every retryable exit below hands the tree to the repair agent with @@ -327,6 +484,20 @@ fail_signature() { seed_dist_note() { echo "⚠️ the baseline leg rebuilt dist/ from baseline sources — run npm run build before typecheck/tests" >> "${GATE_LOG}" } +# Every check below runs the BRANCH's own code (npm scripts, tests, and +# their lifecycle children) with this step's inherited environment. Strip +# the runner injection channels first: a check appending to GITHUB_OUTPUT +# would overwrite the gate's own outputs last-write-wins (a forged +# audit_verdict=sound after the gate's write), GITHUB_ENV/GITHUB_PATH +# plant environment for the PAT-bearing steps that follow, and +# GITHUB_STEP_SUMMARY lets branch code forge the job summary styled as +# gate output (the display-channel sibling; qwen-triage strips it when +# running external-author branch code for the same reason). Same class the +# deferred-upsert child closes with env -i; targeted -u here because the +# checks need the ordinary environment (PATH, HOME, …) to run at all. +strip_runner_channels() { + env -u GITHUB_OUTPUT -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY "$@" +} run_check() { # pipefail makes the pipeline carry the command's status, not tee's. The # side copy holds THIS check's transcript alone — the identity comparison @@ -334,7 +505,7 @@ run_check() { local label="${1}" shift : > "${GATE_LOG}.check" - if ! "$@" 2>&1 | tee -a "${GATE_LOG}" "${GATE_LOG}.check"; then + if ! strip_runner_channels "$@" 2>&1 | tee -a "${GATE_LOG}" "${GATE_LOG}.check"; then if baseline_also_fails "$@"; then reject_fix "${label} (pre-existing: also fails without this round's commit)" 'true' fi @@ -354,7 +525,7 @@ run_check_no_ab() { # allowlist). local label="${1}" shift - if ! "$@" 2>&1 | tee -a "${GATE_LOG}"; then + if ! strip_runner_channels "$@" 2>&1 | tee -a "${GATE_LOG}"; then reject_fix "${label}" fi } @@ -418,16 +589,28 @@ if git diff --quiet "origin/${BRANCH}...${BRANCH}"; then cat "${WORKDIR}/no-action.md" echo "verified_head=$(git rev-parse HEAD)" >> "${GITHUB_OUTPUT}" echo "outcome=noop" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi exit 0 fi echo "❌ Branch unchanged and no no-action.md — agent produced nothing" echo "outcome=failed" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi exit 1 fi if [[ ! -s "${WORKDIR}/address-summary.md" ]]; then echo "❌ Branch changed but address-summary.md is missing" echo "outcome=failed" >> "${GITHUB_OUTPUT}" + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi exit 1 fi @@ -897,7 +1080,7 @@ bite_runner_default() { # $1 = workspace dir, rest = test paths relative to the workspace. local ws="${1}" shift - npm run test --workspace "${ws}" --if-present -- "$@" + strip_runner_channels npm run test --workspace "${ws}" --if-present -- "$@" } mapfile -d '' -t BITE_FILES < <(git diff --name-only -z --no-renames --diff-filter=AM "${ROUND_RANGE}" \ -- ':(glob)**/*.test.*' ':(glob)**/*.spec.*' ':(exclude,glob)**/__snapshots__/**' \ @@ -1066,6 +1249,10 @@ if [[ "${#BITE_FILES[@]}" -gt 0 && -n "${BITE_SRC}" ]]; then tail -c 3000 "${GATE_LOG}" 2> /dev/null echo '````' } > "${WORKDIR}/gate-rejection.md" || true + echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi exit 1 } git reset --quiet 2>> "${GATE_LOG}" || true @@ -1116,5 +1303,17 @@ if [[ "${#BITE_FILES[@]}" -gt 0 && -n "${BITE_SRC}" ]]; then fi fi assert_verification_tree +# A conflict verdict must STOP BLOCKED: completing as fixed would push the +# contested code under the PAT while the report posts the park marker — +# the exact outcome the routing check above exists to prevent. The routing +# check cannot see this shape (a planted handoff.md satisfies it), so +# refuse at the push boundary. NON-retryable: re-audit, don't repair. +if [[ "${AUDIT_VERDICT:-}" == 'conflict' ]]; then + reject_fix 'growth-audit verdict is conflict but the round completed as fixed; conflict must STOP BLOCKED (no push)' 'false' 'false' +fi echo "verified_head=${VERIFICATION_HEAD}" >> "${GITHUB_OUTPUT}" echo "outcome=fixed" >> "${GITHUB_OUTPUT}" +echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}" +if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" +fi diff --git a/.github/workflows/qwen-autofix.md b/.github/workflows/qwen-autofix.md index 3db3bafeff..54294dcc52 100644 --- a/.github/workflows/qwen-autofix.md +++ b/.github/workflows/qwen-autofix.md @@ -93,8 +93,6 @@ YAML, and never delete a section without deleting its pointer. - [43. review-address · Prepare branch and feedback — Growth brake: measure the PR's net size (insertions minus deletions vs the merge base),…](#af-043) - [44. review-address · Prepare branch and feedback — An orphan-history branch (fork takeover / adoption admits one — nothing on this job's…](#af-044) - [45. review-address · Prepare branch and feedback — The marker's window field is spelled `key=`, NOT `win=`: this marker can legitimately…](#af-045) -- [46. review-address · Prepare branch and feedback — Divergence: Critical-only only trims non-Criticals, so when the GROWTH that trips the…](#af-046) -- [47. review-address · Prepare branch and feedback — Count runs whenever the net is measured (not only over budget), so the trajectory clause…](#af-047) - [48. review-address · Prepare branch and feedback — Which trusted humans have exhausted their per-window regular feedback budget (see…](#af-048) - [49. review-address · Prepare branch and feedback — Time-budget exhaustions SINCE THE LAST SUCCESSFUL ROUND mean the standard…](#af-049) - [50. review-address · Triage and address — Bound the agent below the job timeout so a runaway agent fails THIS step (not the whole…](#af-050) @@ -1397,87 +1395,6 @@ post-update size. (A conflict round's own merge of main is the narrower residual; its delta is bounded by the overlap.) ``` - - -### 46. review-address · Prepare branch and feedback — Divergence: Critical-only only trims non-Criticals, so when the GROWTH that trips the… - -In `review-address` · `Prepare branch and feedback`. - -```text -Divergence: Critical-only only trims non-Criticals, so when the -GROWTH that trips the brake is Critical-driven the diff keeps -climbing anyway. Read this window's prior per-round growth markers -(written by the report step): count the rounds that were over -budget, and take the MOST RECENT prior over-budget run's growth -SUM (latest measured= — see below; NOT the window-wide max, which a -one-off spike would raise forever). The round is DIVERGING when it -is over budget now, the brake has already fired for ->= GROWTH_DIVERGENCE_ROUNDS prior rounds, and the diff has NOT -shrunk from that most-recent sum — the fixes are not converging, so -the round must escalate to a human decision instead of patching -again. A diff that is over budget but SHRINKING (agent removing -code) or a one-off overshoot stays in ordinary Critical-only. -``` - - - -### 47. review-address · Prepare branch and feedback — Count runs whenever the net is measured (not only over budget), so the trajectory clause… - -In `review-address` · `Prepare branch and feedback`. - -```text -Count runs whenever the net is measured (not only over budget), so -the trajectory clause below is accurate even on a round that pulled -back under budget. markers: - -Deduped by run=GITHUB_RUN_ID (the per-workflow-run id) and ORDERED -by measured=: the report post's bounded retry re-posts one run's -marker, and a failed job's re-run keeps the same run_id, so a run -collapses to its LATEST measurement — and that collapse happens -BEFORE the over/window/cutoff filters, or a re-run that came back -under budget would still be represented by its stale over=true -attempt. Within the collapse an explicit measured= beats the -created_at fallback: a re-run attempt that crashed BEFORE prepare -— or whose measurement failed — posts an inert over=false marker -with no measured=, whose fallback (post-run) timestamp would -otherwise outdate and erase the same run's real prepare-time -measurement. Every distinct address run has a fresh run_id. -KNOWN RESIDUAL (#9114): during the one-time deploy transition a -run whose FIRST attempt posted a legacy (no measured=) over=true -marker and whose re-run crashes before prepare still collapses -fallback-vs-fallback on created_at — the later inert marker wins -and erases the count. Self-limiting: once deployed, every real -measurement carries measured= and beats any inert marker. -round=/eval-watermark are NOT a safe identity — a state-triggered -lane (a persistent merge conflict selects the PR every scan with no -new evaluable feedback) freezes both NEWEST and ROUND, so distinct -over-budget runs would share them and collapse, stalling the count. -Filtered on measured= (the prepare-time measurement instant, NOT -the comment's post-agent created_at) after GROWTH_NOW_CUTOFF, so a -prior sum measured against a pre-base-update tree is dropped rather -than compared to this round's. KNOWN RESIDUAL (#9114): the tree is -fixed at the branch fetch/checkout while the cutoff comes from -ic.json fetched afterwards, so a base update landing between the -fetch and the measured_at stamp admits a pre-update marker; -self-heals at the next re-arm/base update. measured= is OPTIONAL in -the scan: -markers posted before it existed fall back to their comment's -created_at, so deploying this does not blank the census of a window -that is already in flight. KNOWN RESIDUAL (#9114): during that -transition the sort mixes two clocks — a legacy marker's fallback -is its POST-RUN created_at while a new marker stamps prepare time — -so PREV_SUM can briefly come from an older measurement; the count -is unaffected and it self-heals at the next re-arm/base update. -The "not shrinking" test compares against the MOST RECENT prior -over-budget run's sum (latest measured=), not the window-wide max: a -single transient spike would otherwise raise the bar forever and a -genuine plateau-over-budget runaway (the exact case to escalate) -would never clear it. The CURRENT run's own markers are excluded -(run != GITHUB_RUN_ID): a re-run of a failed job keeps the same run -id and its failed attempt already posted a marker, so counting it -would over-report the round's own attempt as a PRIOR one. -``` - ### 48. review-address · Prepare branch and feedback — Which trusted humans have exhausted their per-window regular feedback budget (see… diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index b7a72b7218..bde6c5d4c5 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -117,15 +117,18 @@ env: # Full rationale → qwen-autofix.md#af-003 GROWTH_BUDGET_SRC_LINES: '${{ vars.QWEN_AUTOFIX_GROWTH_BUDGET_SRC_LINES || 400 }}' GROWTH_BUDGET_TEST_LINES: '${{ vars.QWEN_AUTOFIX_GROWTH_BUDGET_TEST_LINES || 400 }}' - # Non-convergence handoff: once the growth brake has been over budget for - # this many PRIOR rounds in the window AND the diff has not shrunk from the - # most recent over-budget round, the round is DIVERGING (the fixes keep - # growing the diff, so - # Critical-only — which only trims non-Criticals — cannot help). At that - # point the round escalates to a maintainer-decision handoff (split / accept - # core + track the tail / redesign) instead of patching again. Same tunable - # contract as the budgets above (malformed → default at the read site). - GROWTH_DIVERGENCE_ROUNDS: '${{ vars.QWEN_AUTOFIX_GROWTH_DIVERGENCE_ROUNDS || 2 }}' + # Growth audit: a budget breach engages Critical-only AND makes the round a + # growth-audit round. The agent audits the PR's approach on two axes — KISS + # (name a structurally simpler alternative or prove each piece load-bearing) + # and minimal change (every changed hunk traces to the PR's problem, an + # accepted finding, or a failing check) — and records a machine-readable + # verdict (sound/drift/conflict) in growth-audit.json, which the + # verification gate requires in audit rounds. sound re-arms the window at + # the current size (audit-gated /retry) and the loop continues; drift + # simplifies first, then continues; conflict is the ONLY growth path to a + # human, and it idles subsequent scans until a trusted human responds. A + # size signal triggers a JUDGMENT, never a stop: solving the problem is + # primary, growth control secondary. See docs/design/autofix-growth-audit.md. # An auth/access model error (401/402/403, "no access"/"does not exist") # never self-heals - only a maintainer can fix the key - and every retry # costs an agent run AND a PR comment. Cap those attempts far below @@ -3151,6 +3154,64 @@ jobs: if [[ "${REVIEW_PR_LIVE}" == "true" ]]; then continue fi + # Conflict-park gate for the loop's OWN head move: while a + # conflict handoff pends in the live window, an update-branch + # merge re-fires every synchronize-triggered workflow on the new + # head, and those loop-generated checks complete after both + # park clocks — lifting the park with zero human activity, and + # every woken round feeds CONSEC_FAIL toward a terminal lockout + # on the exact PR a human is settling. Mirrors prepare's + # conflict-handoff idempotence block (same marker scan, same + # wake legs, same fail-closed fallbacks); a base that goes + # stale during a park is re-handled by the address gate's own + # stale-base retry once a human wakes a round. + CONFLICT_PARKED='false' + CONFLICT_SINCE_SCAN="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${REARM_KEY}" ' + [ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "") + | [ scan("") ] | .[] + | select(.[0] == $key) | ($c.created_at // "") ] + | max // ""' "${WORKDIR}/ic.json" 2> /dev/null || echo "")" + if [[ -n "${CONFLICT_SINCE_SCAN}" ]]; then + gh api "repos/${REPO}/pulls/${PR}/reviews" --paginate 2> /dev/null \ + | jq -s 'add // []' > "${WORKDIR}/rv.scan.json" || echo '[]' > "${WORKDIR}/rv.scan.json" + gh api "repos/${REPO}/pulls/${PR}/comments" --paginate 2> /dev/null \ + | jq -s 'add // []' > "${WORKDIR}/rc.scan.json" || echo '[]' > "${WORKDIR}/rc.scan.json" + printf '%s' "${CHECKS_JSON}" > "${WORKDIR}/checks.scan.json" + BASE_UPD_AT_SCAN="$(jq -r --arg ab "${AUTOFIX_BOT}" ' + [ .[] | select((.user.login // "") == $ab) + | select((.body // "") | contains(" + # Deduped by run=GITHUB_RUN_ID (the per-workflow-run id) and ORDERED + # by measured=: the report post's bounded retry re-posts one run's + # marker, and a failed job's re-run keeps the same run_id, so a run + # collapses to its LATEST measurement — and that collapse happens + # BEFORE the over/window/cutoff filters, or a re-run that came back + # under budget would still be represented by its stale over=true + # attempt. Within the collapse an explicit measured= beats the + # created_at fallback: a re-run attempt that crashed BEFORE prepare + # — or whose measurement failed — posts an inert over=false marker + # with no measured=, whose fallback (post-run) timestamp would + # otherwise outdate and erase the same run's real prepare-time + # measurement. Every distinct address run has a fresh run_id. + # KNOWN RESIDUAL (#9114): during the one-time deploy transition a + # run whose FIRST attempt posted a legacy (no measured=) over=true + # marker and whose re-run crashes before prepare still collapses + # fallback-vs-fallback on created_at — the later inert marker wins + # and erases the count. Self-limiting: once deployed, every real + # measurement carries measured= and beats any inert marker. + # round=/eval-watermark are NOT a safe identity — a state-triggered + # lane (a persistent merge conflict selects the PR every scan with no + # new evaluable feedback) freezes both NEWEST and ROUND, so distinct + # over-budget runs would share them and collapse, stalling the count. + # Filtered on measured= (the prepare-time measurement instant, NOT + # the comment's post-agent created_at) after GROWTH_NOW_CUTOFF, so a + # round measured against a pre-base-update tree is dropped rather + # than counted in this window's census. KNOWN RESIDUAL (#9114): the + # tree is fixed at the branch fetch/checkout while the cutoff comes + # from ic.json fetched afterwards, so a base update landing between + # the fetch and the measured_at stamp admits a pre-update marker; + # self-heals at the next re-arm/base update. measured= is OPTIONAL + # in the scan: markers posted before it existed fall back to their + # comment's created_at, so deploying this does not blank the census + # of a window that is already in flight. + # The CURRENT run's own markers are excluded (run != GITHUB_RUN_ID): + # a re-run of a failed job keeps the same run id and its failed + # attempt already posted a marker, so counting it would over-report + # the round's own attempt as a PRIOR one. OVER_ROUNDS_PRIOR=0 - PREV_SUM=0 + KISS_AUDIT='false' if [[ "${NET_MEASURED}" == 'true' ]]; then - read -r OVER_ROUNDS_PRIOR PREV_SUM < <(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" --arg cutoff "${GROWTH_NOW_CUTOFF}" --arg curr "${GITHUB_RUN_ID}" ' + OVER_ROUNDS_PRIOR="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" --arg cutoff "${GROWTH_NOW_CUTOFF}" --arg curr "${GITHUB_RUN_ID}" ' [ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "") | [ scan("") ] | .[] - | {sum: ((.[0] | tonumber) + (.[1] | tonumber)), over: .[2], round: (.[3] | tonumber), run: (.[4] | tonumber), measured: (.[5] // ($c.created_at // "")), explicit: (.[5] != null), win: .[6]} ] + | {over: .[2], run: (.[4] | tonumber), measured: (.[5] // ($c.created_at // "")), explicit: (.[5] != null), win: .[6]} ] | group_by(.run) | map(max_by([.explicit, .measured])) | map(select(.win == $key and .over == "true")) | map(select(.run != ($curr | tonumber))) | map(select($cutoff == "" or (.measured > $cutoff))) - | sort_by(.measured) - | "\(length) \((last.sum) // 0)"' "${WORKDIR}/ic.json" 2> /dev/null || echo "0 0") + | length' "${WORKDIR}/ic.json" 2> /dev/null || echo 0)" [[ "${OVER_ROUNDS_PRIOR}" =~ ^[0-9]+$ ]] || OVER_ROUNDS_PRIOR=0 - [[ "${PREV_SUM}" =~ ^-?[0-9]+$ ]] || PREV_SUM=0 - if [[ "${CRITICAL_ONLY_GROWTH}" == 'true' \ - && "${OVER_ROUNDS_PRIOR}" -ge "${GROWTH_DIVERGENCE_ROUNDS}" \ - && $(( GROWTH_SRC + GROWTH_TEST )) -ge "${PREV_SUM}" ]]; then - GROWTH_DIVERGED='true' - fi + fi + # Audit on the FIRST breach, not after spending more rounds proving + # non-convergence: the judgment is what a budget breach means now. + if [[ "${CRITICAL_ONLY_GROWTH}" == 'true' ]]; then + KISS_AUDIT='true' fi # The over-budget flag feeds the report's per-round marker; the - # handoff itself is enforced by the feedback.md text below, so - # GROWTH_DIVERGED needs no step output. + # audit itself is enforced by the feedback.md section below plus + # the verification gate's verdict requirement. echo "critical_only_growth=${CRITICAL_ONLY_GROWTH}" >> "${GITHUB_OUTPUT}" - [[ "${GROWTH_DIVERGED}" == 'true' ]] && - echo "🛑 diff not converging: over budget now, ${OVER_ROUNDS_PRIOR} prior over-budget round(s) in this window, growth not shrinking — escalating to a maintainer decision instead of patching." + echo "kiss_audit=${KISS_AUDIT}" >> "${GITHUB_OUTPUT}" + [[ "${KISS_AUDIT}" == 'true' ]] && + echo "🔍 growth budget breached (source ${GROWTH_SRC} / test ${GROWTH_TEST} vs budgets ${GROWTH_BUDGET_SRC_LINES}/${GROWTH_BUDGET_TEST_LINES}; ${OVER_ROUNDS_PRIOR} prior over-budget round(s) this window) — this round is a growth-audit round." + # Conflict-handoff idempotence: a conflict verdict parks the PR at + # a genuinely human call. Until a trusted human responds, scans + # must not launch agents or post comments — review-bot regeneration + # alone (an update-branch merge re-reviews every new head) would + # otherwise churn one identical handoff after another. Wake only on + # feedback the loop cannot produce itself: trusted-human + # reviews/comments, or a failing check from OUTSIDE the Qwen Autofix + # workflow (a CI build/test the loop did not run). The Qwen Autofix + # workflow's OWN check runs are excluded wholesale: under a park no + # address round can legitimately run, so any review-address check + # newer than the marker is necessarily the conflict round's own + # failed check (posted after the handoff) — counting it would let + # the loop's own output unpark the very round it came from, and the + # resulting wasted failure rounds feed CONSEC_FAIL toward a terminal + # lockout on the exact PR a human is trying to settle. A manual + # job re-run reaches prepare and parks green (no failed check), and + # /retry remains the sanctioned lift. A /retry re-arm moves + # LIVE_REARM_KEY past the marker's win= and lifts the park on its + # own. + # Two more loop-generated events must not wake: a stale-base + # auto-update is the loop's OWN head move — the red checks it + # REACTS to completed before its marker (they are the condition it + # handles, not human feedback), so the checks leg counts only + # failures completing after BOTH the conflict marker and the + # latest base update; and CANCELLED never wakes — an + # update-branch push cancels in-flight runs on the old head (and a + # close/reopen does the same), which the loop produces without any + # human. + # The exclusion set is wider than the loop's own workflow: the + # loop's SIBLING machinery produces check events too — the review + # workflow re-fires on every head the loop's own base-update merge + # creates, the CI-failure patrol re-runs flaky failures on the + # UNCHANGED head by cron, and the fork lanes carry the loop's own + # checks for fork PRs. All of it completes after both clocks with + # no human anywhere in the input, so all of it is excluded by + # name; and while a handoff pends, the loop performs NO head + # moves at all (the scan's stale-base auto-update and the + # conflict round's own stale-base retry both skip parked PRs), + # so any check newer than both clocks is human-caused. + CONFLICT_SINCE="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" ' + [ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "") + | [ scan("") ] | .[] + | select(.[0] == $key) | ($c.created_at // "") ] + | max // ""' "${WORKDIR}/ic.json" 2> /dev/null || echo "")" + if [[ -n "${CONFLICT_SINCE}" && "${STALE}" != 'true' ]]; then + CONFLICT_WAKE="$(jq -rs \ + --arg since "${CONFLICT_SINCE}" --arg baseupd "${BASE_UPD_AT}" \ + --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ + --argjson trust "${TRUSTED_ASSOC}" ' + (.[0] | map(select((.submitted_at // "") > $since) + | select((.user.login // "") != $ab and (.user.login // "") != $rb) + | select(((.author_association // "") | IN($trust[]))) + | select((.state // "") | IN("CHANGES_REQUESTED", "COMMENTED"))) | length) + + (.[1] | map(select((.created_at // "") > $since) + | select((.user.login // "") != $ab and (.user.login // "") != $rb) + | select(((.author_association // "") | IN($trust[])))) | length) + + (.[2] | map(select((.created_at // "") > $since) + | select((.user.login // "") != $ab and (.user.login // "") != $rb) + | select(((.author_association // "") | IN($trust[]))) + | select((.body // "") | test("") ] | .[] + | select(.[1] == $key) | "- \($c.created_at // "?"): verdict=\(.[0])" ] + | .[]' "${WORKDIR}/ic.json" 2> /dev/null || true)" + if [[ -n "${PRIOR_AUDITS}" ]]; then + echo "Prior growth audits this window — a repeated verdict needs new evidence:" + echo + printf '%s\n' "${PRIOR_AUDITS}" + else + echo "No prior growth audit this window." + fi echo fi echo "## Reviews" @@ -4959,12 +5146,23 @@ jobs: # its case to exit 1, and the always() report step posts. timeout-minutes: 60 env: + # BASH_ENV is sourced by bash at process STARTUP, before line 1 of + # the body below — a body-side unset is one hop late. Pinning it + # empty at step level outranks any $GITHUB_ENV plant (same + # doctrine as FOOTPRINT_ENFORCE below); SHELLOPTS is the sibling + # option-import channel. The gate then launches through an env -i + # clean child, so its bash inherits nothing at all. + BASH_ENV: '' + SHELLOPTS: '' TRUSTED_PATH: '${{ steps.stage.outputs.trusted_path }}' VERIFY_RUNNER_SHA256: '${{ steps.stage.outputs.verify_runner_sha256 }}' # Step-level env outranks $GITHUB_ENV: an earlier shell-capable # step (the agent runs branch code on the host) must not be able # to downgrade a repo-variable 'reject' back to 'advisory'. FOOTPRINT_ENFORCE: "${{ vars.QWEN_AUTOFIX_FOOTPRINT_ENFORCE || 'advisory' }}" + # Growth-audit rounds must carry a valid growth-audit.json verdict; + # the gate enforces presence + shape before any push decision. + KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}' run: |- # The gate decides whether the PAT push runs, and the first pass # executes the branch's own build/test on the host before the @@ -4975,7 +5173,23 @@ jobs: export PATH="${TRUSTED_PATH}" unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH echo "${VERIFY_RUNNER_SHA256} ${RUNNER_TEMP}/run-autofix-review-verification.sh" | sha256sum -c - > /dev/null - bash "${RUNNER_TEMP}/run-autofix-review-verification.sh" + # Launch the gate through the workflow's env -i clean-child + # pattern: the step environment inherits every $GITHUB_ENV plant + # earlier steps left (verdict-variable plants, BITE_RUNNER + # overrides, whatever is next), and enumeration is the failure + # mode this design keeps hitting — an allowlisted child drops the + # whole class. The gate re-declares the variables it needs. + LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH= \ + /usr/bin/env -i \ + PATH="${TRUSTED_PATH}" \ + HOME="${HOME}" \ + RUNNER_TEMP="${RUNNER_TEMP}" \ + WORKDIR="${WORKDIR}" \ + BRANCH="${BRANCH}" \ + GITHUB_OUTPUT="${GITHUB_OUTPUT}" \ + KISS_AUDIT="${KISS_AUDIT:-false}" \ + FOOTPRINT_ENFORCE="${FOOTPRINT_ENFORCE:-advisory}" \ + bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh" - name: 'Repair deterministic rejection' id: 'repair' @@ -5133,12 +5347,25 @@ jobs: # Same bound as the first pass, for the same reason. timeout-minutes: 60 env: + # BASH_ENV is sourced by bash at process STARTUP, before line 1 of + # the body below — a body-side unset is one hop late. Pinning it + # empty at step level outranks any $GITHUB_ENV plant (same + # doctrine as FOOTPRINT_ENFORCE below); SHELLOPTS is the sibling + # option-import channel. The gate then launches through an env -i + # clean child, so its bash inherits nothing at all. + BASH_ENV: '' + SHELLOPTS: '' TRUSTED_PATH: '${{ steps.stage.outputs.trusted_path }}' VERIFY_RUNNER_SHA256: '${{ steps.stage.outputs.verify_runner_sha256 }}' # Step-level env outranks $GITHUB_ENV: an earlier shell-capable # step (the agent runs branch code on the host) must not be able # to downgrade a repo-variable 'reject' back to 'advisory'. FOOTPRINT_ENFORCE: "${{ vars.QWEN_AUTOFIX_FOOTPRINT_ENFORCE || 'advisory' }}" + # The control bit arrives through the FIRST gate's defended + # output (recorded before any check ran, re-appended at every + # exit); the prepare copy is only the fallback for a first pass + # that died before recording it. + KISS_AUDIT: '${{ steps.verify.outputs.kiss_audit || steps.prepare.outputs.kiss_audit }}' run: |- # The gate decides whether the PAT push runs, and the first pass # executes the branch's own build/test on the host before the @@ -5149,7 +5376,23 @@ jobs: export PATH="${TRUSTED_PATH}" unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH echo "${VERIFY_RUNNER_SHA256} ${RUNNER_TEMP}/run-autofix-review-verification.sh" | sha256sum -c - > /dev/null - bash "${RUNNER_TEMP}/run-autofix-review-verification.sh" + # Launch the gate through the workflow's env -i clean-child + # pattern: the step environment inherits every $GITHUB_ENV plant + # earlier steps left (verdict-variable plants, BITE_RUNNER + # overrides, whatever is next), and enumeration is the failure + # mode this design keeps hitting — an allowlisted child drops the + # whole class. The gate re-declares the variables it needs. + LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH= \ + /usr/bin/env -i \ + PATH="${TRUSTED_PATH}" \ + HOME="${HOME}" \ + RUNNER_TEMP="${RUNNER_TEMP}" \ + WORKDIR="${WORKDIR}" \ + BRANCH="${BRANCH}" \ + GITHUB_OUTPUT="${GITHUB_OUTPUT}" \ + KISS_AUDIT="${KISS_AUDIT:-false}" \ + FOOTPRINT_ENFORCE="${FOOTPRINT_ENFORCE:-advisory}" \ + bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh" - name: 'Finalize verification' id: 'final_verify' @@ -5165,10 +5408,23 @@ jobs: REPAIR_VERIFIED_HEAD: '${{ steps.verify_repair.outputs.verified_head }}' FIRST_PREEXISTING: '${{ steps.verify.outputs.preexisting }}' REPAIR_PREEXISTING: '${{ steps.verify_repair.outputs.preexisting }}' + FIRST_AUDIT_VERDICT: '${{ steps.verify.outputs.audit_verdict }}' + REPAIR_AUDIT_VERDICT: '${{ steps.verify_repair.outputs.audit_verdict }}' + # The pass conclusions are the tamper-evident seal on the output + # claims below: a gate that reached fixed/noop EXITED 0, and a + # step killed mid-check (whose output file stays discoverable and + # appendable under $RUNNER_TEMP) never concludes success. + FIRST_CONCLUSION: '${{ steps.verify.conclusion }}' + REPAIR_CONCLUSION: '${{ steps.verify_repair.conclusion }}' + FIRST_KISS_AUDIT: '${{ steps.verify.outputs.kiss_audit || steps.prepare.outputs.kiss_audit }}' + REPAIR_KISS_AUDIT: '${{ steps.verify_repair.outputs.kiss_audit }}' run: |- OUTCOME="${FIRST_OUTCOME}" COMMITTED="${FIRST_COMMITTED}" VERIFIED_HEAD="${FIRST_VERIFIED_HEAD}" + AUDIT_VERDICT="${FIRST_AUDIT_VERDICT}" + KISS_AUDIT="${FIRST_KISS_AUDIT}" + PASS_CONCLUSION="${FIRST_CONCLUSION}" # The flag travels WITH the attempt whose outcome is selected: the # first pass can fail a round-caused check, the repair fixes it, and # the repair verification can then hit a pre-existing failure — that @@ -5186,6 +5442,35 @@ jobs: OUTCOME="${REPAIR_OUTCOME}" COMMITTED="${REPAIR_COMMITTED:-${FIRST_COMMITTED}}" VERIFIED_HEAD="${REPAIR_VERIFIED_HEAD}" + # The verdict travels WITH the attempt whose outcome is + # selected: a repair pass legitimately re-audits (its feedback + # rebuild keeps the audit section; the SKILL mandates + # audit-first), and the verdict its gate validated is the one + # the round's code was judged by — binding the first pass + # unconditionally dropped it. The :- fallback mirrors COMMITTED: + # a repair that validated nothing leaves the first pass's + # validated verdict as the record. + AUDIT_VERDICT="${REPAIR_AUDIT_VERDICT:-${FIRST_AUDIT_VERDICT}}" + KISS_AUDIT="${REPAIR_KISS_AUDIT:-${FIRST_KISS_AUDIT}}" + PASS_CONCLUSION="${REPAIR_CONCLUSION}" + fi + # Conclusion gate: fixed/noop are the ONLY outcomes that release + # the PAT push. A silent gate death (the step killed mid-check) + # concludes failure, yet its step-output file stays discoverable + # under $RUNNER_TEMP and appendable — a forged outcome=fixed + + # verified_head must not flow to the push condition. Accept + # fixed/noop only from a pass whose step concluded success; + # anything else reads as a crashed gate (empty outcome → the + # report's retry path), never as a verdict, and the audit bit + # riding the tainted outputs is discarded with it. + if [[ "${OUTCOME}" == 'fixed' || "${OUTCOME}" == 'noop' ]] && + [[ "${PASS_CONCLUSION}" != 'success' ]]; then + echo "::error::verify pass claims outcome=${OUTCOME} but concluded '${PASS_CONCLUSION:-}' — discarding the claim (forged or crashed-gate outputs); NOT pushing" + OUTCOME='' + COMMITTED='' + VERIFIED_HEAD='' + AUDIT_VERDICT='' + KISS_AUDIT='' fi echo "outcome=${OUTCOME}" >> "${GITHUB_OUTPUT}" if [[ -n "${COMMITTED}" ]]; then @@ -5194,6 +5479,12 @@ jobs: if [[ -n "${VERIFIED_HEAD}" ]]; then echo "verified_head=${VERIFIED_HEAD}" >> "${GITHUB_OUTPUT}" fi + if [[ -n "${AUDIT_VERDICT}" ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi + if [[ -n "${KISS_AUDIT}" ]]; then + echo "kiss_audit=${KISS_AUDIT}" >> "${GITHUB_OUTPUT}" + fi case "${OUTCOME}" in # handoff and the two brake-violation rejections are # deliberate, PUBLISHED verdicts, not failures: the agent @@ -5218,7 +5509,7 @@ jobs: if git rev-parse --verify "${BRANCH}" > /dev/null 2>&1; then git diff "origin/main...${BRANCH}" > "${WORKDIR}/pr.diff" || true fi - for f in feedback.md address-summary.md no-action.md failure.md failure.zh.md handoff.md gate-rejection.md gate-advisories.md agent-api-error agent-api-error-kind agent-timeout resolved-comments.txt comment-replies.json deferred-findings.json deferred-findings.carry.json deferred-findings.unmerged.json pr.diff; do + for f in feedback.md address-summary.md no-action.md failure.md failure.zh.md handoff.md gate-rejection.md gate-advisories.md growth-audit.json agent-api-error agent-api-error-kind agent-timeout resolved-comments.txt comment-replies.json deferred-findings.json deferred-findings.carry.json deferred-findings.unmerged.json pr.diff; do if [[ -f "${WORKDIR}/${f}" ]]; then echo "=============== ${f} ===============" # Agent-written content: a line-start `::` would be parsed as a @@ -5276,10 +5567,25 @@ jobs: GROWTH_BASE_WIN: '${{ steps.prepare.outputs.growth_base_win }}' # This round's own growth + over-budget flag, written as the # per-round autofix-growth-now marker so later rounds can measure - # divergence (growth still climbing over budget = not converging). + # the growth trajectory (the audit's context reads the prior + # over-budget count). GROWTH_SRC: '${{ steps.prepare.outputs.growth_src }}' GROWTH_TEST: '${{ steps.prepare.outputs.growth_test }}' CRITICAL_ONLY_GROWTH: '${{ steps.prepare.outputs.critical_only_growth }}' + # Whether this round was a growth-audit round; when it was, the + # report carries the audit's verdict marker (and a sound verdict + # additionally re-arms the window at the current size). The bit + # rides Finalize verification's copy — the gates' defended output + # chain, never steps.prepare's raw copy. + KISS_AUDIT: '${{ steps.final_verify.outputs.kiss_audit }}' + # The verdict Finalize verification selected WITH the outcome — + # never a re-read of the branch-writable growth-audit.json. The + # pass whose outcome was selected wins (a repair pass legitimately + # re-audits, and its gate-validated verdict is the one the round's + # code was judged by); a repair that validated nothing falls back + # to the first pass's validated verdict. Empty when no gate + # validated a verdict; the marker then stays absent. + AUDIT_VERDICT: '${{ steps.final_verify.outputs.audit_verdict }}' MEASURED_AT: '${{ steps.prepare.outputs.measured_at }}' run: |- # gh has its own $GITHUB_ENV-injectable channels: pin the host and @@ -5304,6 +5610,30 @@ jobs: # would double-write that round's marker. ROUND="${EFFECTIVE_ROUND:-${ROUND}}" MODEL_DISPLAY="${MODEL:-default}" + # Growth-audit trail (+ re-arm on sound): audit rounds record the + # verdict under the key the baseline was READ under — same rule as + # the growth markers, same dead-key hazard (a supersede-exempt + # round can report under a stale WINDOW after a re-arm). The + # verdict comes from AUDIT_VERDICT — the verdict the verification + # GATE validated and surfaced as a step output — NOT a re-read of + # growth-audit.json: the branch's own build/tests run as the runner + # user and WORKDIR is a predictable path they can write, so the + # file could change after the gate looked. Re-arming is allowed + # for completed rounds only ($1 = allow): a sound verdict whose + # round then FAILED must not re-anchor the window — the failure + # path re-measures under the same window instead. + emit_growth_audit_marker() { + local allow_rearm="${1:-false}" + [[ "${KISS_AUDIT}" == 'true' ]] || return 0 + case "${AUDIT_VERDICT:-}" in + sound | drift | conflict) ;; + *) return 0 ;; + esac + echo "" + if [[ "${AUDIT_VERDICT}" == 'sound' && "${allow_rearm}" == 'true' ]]; then + echo "" + fi + } if [[ -z "${GITHUB_TOKEN}" ]]; then echo '::error::CI_DEV_BOT_PAT is required to push and report as qwen-code-dev-bot.' exit 1 @@ -5715,11 +6045,12 @@ jobs: if [[ "${GROWTH_BASE_NEW}" == 'true' ]]; then echo "" fi - # Per-round growth history the next round's divergence read - # counts; run=GITHUB_RUN_ID is the DEDUP identity (a retry or a + # Per-round growth history the next round's census counts; + # run=GITHUB_RUN_ID is the DEDUP identity (a retry or a # job re-run re-posts the same run; measured= orders and picks # that run's latest attempt). echo "" + emit_growth_audit_marker true } > "${WORKDIR}/report.md" STATUS="pushed (round ${NEXT_ROUND}/${MAX_ROUNDS})" else @@ -5752,11 +6083,12 @@ jobs: if [[ "${GROWTH_BASE_NEW}" == 'true' ]]; then echo "" fi - # Per-round growth history the next round's divergence read - # counts; run=GITHUB_RUN_ID is the DEDUP identity (a retry or a + # Per-round growth history the next round's census counts; + # run=GITHUB_RUN_ID is the DEDUP identity (a retry or a # job re-run re-posts the same run; measured= orders and picks # that run's latest attempt). echo "" + emit_growth_audit_marker true } > "${WORKDIR}/report.md" STATUS="no action needed" fi @@ -5907,7 +6239,7 @@ jobs: # This step also posts a round report (timeout / gate-rejection / # abort), so it writes the per-round growth-now marker too — else an # over-budget round that never reaches 'Push and report' leaves a - # history gap and the divergence count under-reports. Empty outputs + # history gap and the census under-reports. Empty outputs # (prepare never ran) fall through the :-0/:-false marker fallbacks # to an inert over=false entry — measured= then OMITS itself (an # EMPTY measured= value matches no scan and would silently drop the @@ -5918,6 +6250,13 @@ jobs: CRITICAL_ONLY_GROWTH: '${{ steps.prepare.outputs.critical_only_growth }}' MEASURED_AT: '${{ steps.prepare.outputs.measured_at }}' GROWTH_BASE_WIN: '${{ steps.prepare.outputs.growth_base_win }}' + # Same selection chain as AUDIT_VERDICT below — the gates' + # defended output, never steps.prepare's raw copy. + KISS_AUDIT: '${{ steps.final_verify.outputs.kiss_audit }}' + # The verdict Finalize verification selected WITH the outcome — + # see 'Push and report' for the selection rule. Never a re-read of + # the branch-writable file. + AUDIT_VERDICT: '${{ steps.final_verify.outputs.audit_verdict }}' UPSERT_SRC: '${{ steps.stage.outputs.upsert_src }}' TRUSTED_PATH: '${{ steps.stage.outputs.trusted_path }}' run: |- @@ -5927,6 +6266,23 @@ jobs: REPORT_HEAD="${CHECKED_OUT_HEAD}" ROUND="${EFFECTIVE_ROUND:-${ROUND}}" MODEL_DISPLAY="${MODEL:-default}" + # Same helper as 'Push and report' (each step is its own shell, so + # the definition does not carry over). The verdict is the one the + # verification GATE validated (AUDIT_VERDICT step output), never a + # re-read of the branch-writable file. Failure rounds record the + # verdict for a complete trail but never re-arm the window. + emit_growth_audit_marker() { + local allow_rearm="${1:-false}" + [[ "${KISS_AUDIT}" == 'true' ]] || return 0 + case "${AUDIT_VERDICT:-}" in + sound | drift | conflict) ;; + *) return 0 ;; + esac + echo "" + if [[ "${AUDIT_VERDICT}" == 'sound' && "${allow_rearm}" == 'true' ]]; then + echo "" + fi + } SUFFIX='' [[ "${DRY_RUN}" == "true" ]] && SUFFIX=' (dry-run, nothing pushed)' { @@ -6151,12 +6507,22 @@ jobs: # failure). Before handing to a human, check whether the PR is # Full rationale → qwen-autofix.md#af-065 STALE_BASE_RETRY=false - MAIN_HEAD_R="$(gh api "repos/${REPO}/commits/${DEFAULT_BRANCH:-main}" --jq '.sha' 2> /dev/null || echo '')" - if [[ -n "${MAIN_HEAD_R}" && -n "${REPORT_HEAD}" ]]; then - CMP_R="$(gh api "repos/${REPO}/compare/${MAIN_HEAD_R}...${REPORT_HEAD}" --jq '.status' 2> /dev/null || echo '')" - if [[ "${CMP_R}" == 'behind' || "${CMP_R}" == 'diverged' ]] \ - && gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${REPORT_HEAD}" > /dev/null 2>&1; then - STALE_BASE_RETRY=true + # A conflict round must PARK quietly at the human call: its + # own stale-base merge would re-fire every synchronize- + # triggered workflow on the new head, and those loop- + # generated checks complete after the conflict marker this + # same report posts — waking the very park it establishes. + # The scan's stale-base auto-update carries the matching + # gate; base staleness is re-handled by this retry once a + # human wakes. + if [[ "${AUDIT_VERDICT:-}" != 'conflict' ]]; then + MAIN_HEAD_R="$(gh api "repos/${REPO}/commits/${DEFAULT_BRANCH:-main}" --jq '.sha' 2> /dev/null || echo '')" + if [[ -n "${MAIN_HEAD_R}" && -n "${REPORT_HEAD}" ]]; then + CMP_R="$(gh api "repos/${REPO}/compare/${MAIN_HEAD_R}...${REPORT_HEAD}" --jq '.status' 2> /dev/null || echo '')" + if [[ "${CMP_R}" == 'behind' || "${CMP_R}" == 'diverged' ]] \ + && gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${REPORT_HEAD}" > /dev/null 2>&1; then + STALE_BASE_RETRY=true + fi fi fi if [[ "${STALE_BASE_RETRY}" == 'true' ]]; then @@ -6407,11 +6773,15 @@ jobs: echo "🧠 Handled by **Qwen Code** · model/模型 \`${MODEL_DISPLAY}\`" echo echo "" - # Per-round growth history the divergence read counts — same - # marker the push/no-op report paths write, so an over-budget + # Per-round growth history the census counts — same marker + # the push/no-op report paths write, so an over-budget # round that timed out or was gate-rejected is not a gap. run= # (per-workflow-run) is the DEDUP identity; measured= orders. echo "" + # The verdict still rides the failure report (the trail must be + # complete), but a round that FAILED does not get to re-arm the + # window — the failure path re-measures under the same window. + emit_growth_audit_marker false # A sentinel ts means the agent evaluated NOTHING (crash, API # error, gate crash) and the next scan must retry. Recording a # judged head here would make RED_HEAD == LIVE_HEAD, so the diff --git a/.qwen/skills/autofix/SKILL.md b/.qwen/skills/autofix/SKILL.md index 4bc5b4a537..135cefcc59 100644 --- a/.qwen/skills/autofix/SKILL.md +++ b/.qwen/skills/autofix/SKILL.md @@ -375,24 +375,45 @@ silently overriding or silently complying. root-cause, subtractive fixes over additive guards, and read a rising trajectory as a signal — if closing a finding would grow the diff materially AND the same class of gap keeps reappearing on code an earlier round added, - the right response is to escalate for a split, not to add another guard. -- Not converging (the diff keeps growing past budget): when `feedback.md` - contains a `Needs a maintainer's decision — this PR is not converging` - section, the growth brake has been over budget across rounds and the diff is - still not shrinking — the findings themselves are driving the growth, so - Critical-only cannot help (the Criticals ARE the growth). Do NOT apply more - code fixes this round. This is a `defer-to-human` item: STOP `BLOCKED` with a - handoff that names the decision and lays out the options — split the PR (land - the core, track the remaining findings as follow-up issues), redesign, or - accept the current state with the tail deferred — plus your recommendation. - Write that handoff to `/handoff.md` — English-only, no details - block — naming the decision, the options, your recommendation, and what was - tried; then stop without writing anything else: no commit, no - `address-summary.md`, no `no-action.md`, no `failure.md`. The harness - recognizes a handoff with no fix verdict as a deliberate deferral: the round - ends cleanly, the note is posted to the PR, and the item waits for the - maintainer instead of being re-run. Continuing to patch, or deciding the - split yourself, is exactly the wrong move; the call is the maintainer's. + consolidate or subtract instead of adding another guard. +- Growth audit required (the window is over its growth budget): when + `feedback.md` contains a `Growth audit required` section, this is a + growth-audit round. Solving the problem is primary, growth control + secondary — a size signal triggers a JUDGMENT, never a stop: the takeover + exists to land fixes, not to police line counts. BEFORE any other work or + edit this round, audit the approach on the two axes below, then record + `growth-audit.json` in the workdir — a single JSON document, verdict + `sound|drift|conflict` plus `kiss.result` and `minimal_change.result` + each `pass|fail`, the drift alternative or untraceable hunks, and a + rationale — and route on the verdict. The verification gate rejects the + round without a valid verdict (the taxonomy is enforced — `sound` + requires both axes `pass`, `drift` at least one `fail` — and a conflict + verdict must stop the round with the handoff), and a repeated verdict + after a prior audit this window must bring new evidence (the feedback + section lists the prior audits). + - KISS (structure): assume the PR IS over-engineered and try to prove it. + Either NAME a structurally simpler approach that achieves the same goal + (shape, not prose) or justify each accumulated piece as load-bearing for + a specific finding or failure mode. + - Minimal change (footprint): every changed file/hunk must trace to (a) the + PR's original problem, (b) an accepted review finding, or (c) fixing a + failing check. Hunks with no trace are deletion candidates. + - `sound` — the approach is justified; continue addressing feedback + normally. The workflow re-arms the counting window at the current size + and the loop continues. + - `drift` — implement the named simpler alternative and/or the deletion + list FIRST (typically net-negative), then continue addressing feedback. + - `conflict` — two defensible directions and the choice is not yours: STOP + `BLOCKED` with a handoff carrying the audit's reasoning — the narrowed + contested choice with evidence, not "the diff is too big". + Write that handoff to `/handoff.md` — English-only, no details + block — naming the decision, the options, your recommendation, and what + was tried; then stop without writing anything else: no commit, no + `address-summary.md`, no `no-action.md`, no `failure.md`. The harness + recognizes a handoff with no fix verdict as a deliberate deferral: the + round ends cleanly, the note is posted to the PR, and the item waits for + the maintainer instead of being re-run. This is the ONLY growth-related + path to a human. - Needs a maintainer's decision: a finding that turns on a judgment that is NOT yours to make — a product or scope tradeoff (is this acceptable for v1? should the PR be split?), two reviewers asking for opposite things, or whether diff --git a/docs/design/autofix-growth-audit.md b/docs/design/autofix-growth-audit.md new file mode 100644 index 0000000000..ebef35a79f --- /dev/null +++ b/docs/design/autofix-growth-audit.md @@ -0,0 +1,398 @@ +# Autofix growth brake: audit instead of stop + +## Problem statement + +PR #9213 (`fix(review): fix silent reverse-audit retirement failures`, +under `autofix/takeover`) stalled at round 5. The deterministic growth +brake measured window growth of source 286 / test 948 net lines against +budgets of 400/400, saw two prior over-budget rounds with no shrinkage, +set `GROWTH_DIVERGED`, and the round became a `defer-to-human` handoff: +no code changes, no commit, no resolved threads, and a maintainer +question ("how to land this PR") whose honest answers were only "merge +what exists" or "re-arm and let the loop continue" — both things the +loop could have decided itself. + +Three structural problems: + +1. **The size signal is wired to a stop effector.** Over budget → + Critical-only; still over budget across rounds → full stop. The loop + has no mode between "patch freely" and "halt", so a budget breach + that the remaining work could still satisfy terminates the takeover + anyway. + +2. **The growth the brake punishes is protocol-mandated.** The address + protocol requires a pinned regression test for every fix; #9213 fixes + behavior (receipt parsing, retirement semantics) that is ONLY + observable through tests. The loop was stopped for doing what the + loop's own rules require. 948 of the window's lines are the two test + blocks that pin the PR's stated problem. + +3. **The stopped state churns.** `GROWTH_DIVERGED` is enforced only by + feedback.md text (it is deliberately not a step output), so every + scan that sees new feedback past the watermark still launches an + agent run that re-derives "still blocked" and can re-post the + handoff — and new feedback keeps arriving: the review bot's + `CHANGES_REQUESTED` state always passes the Critical-only filter, + and update-branch merges regenerate reviews on every new head. The + takeover label stays on; runs keep burning; nothing progresses + until a human acts. + +Historical justification for the brake is real (#8853 grew 315 → 1393 +net lines in four bot rounds, +609 in a single round; #8276 grew ~2700 +net lines under management). The brake's MEASUREMENT is sound; its +EFFECTOR is wrong. + +## Design principles + +1. **Solving the problem is primary; growth control is secondary.** The + takeover exists to land fixes, not to police line counts. +2. **A size signal triggers a JUDGMENT, never a constraint or a stop.** + Over budget means "audit the approach", not "you may not add lines" + and never "halt". +3. **Terminal states are only "done" or "a genuinely human call".** + Done = everything affordable solved, the rest tracked in follow-up + issues. Human call = two defensible directions collide. Size is + neither. + +## Proposed changes + +### A. Trigger: budget breach starts an audit round (qwen-autofix.yml) + +The divergence ladder is replaced. Wherever the prepare step currently +sets `CRITICAL_ONLY_GROWTH=true` (window growth past either budget), +the round additionally becomes a growth-audit round (`KISS_AUDIT=true` +step output feeding feedback.md and the verdict gate). The +`GROWTH_DIVERGENCE_ROUNDS` escalation (over budget for N prior rounds +AND not shrinking → handoff) is retired with its repo variable; the +budgets themselves (`GROWTH_BUDGET_SRC_LINES`, +`GROWTH_BUDGET_TEST_LINES`) and the Critical-only engagement on breach +are unchanged — the audit rides on top of Critical-only, it does not +replace it. + +Auditing at FIRST breach (not after two more over-budget rounds) saves +the rounds the divergence ladder used to spend proving non-convergence; +#9213 would have audited at round 3 instead of stopping at round 5. + +The audit fires only when growth is measurable +(`NET_MEASURED=true`, i.e. a trusted merge base exists): the verdict +needs numbers to judge. The unmeasured advisory path (growth not +reported, no brake) is unchanged. + +### B. Audit mode in the autofix skill (.qwen/skills/autofix/SKILL.md) + +feedback.md gains a `Growth audit required` section (replacing the +`Needs a maintainer's decision — this PR is not converging` section) +carrying the growth numbers, the prior over-budget round count, and any +prior audit verdict markers (section D). The agent audits on two axes, +with the burden of proof inverted — the default assumption is that the +PR IS over-engineered, and the agent must disprove that: + +- **KISS (structure):** does a structurally simpler approach achieve + the same goal? The agent must either NAME the simpler alternative + (shape, not prose) or justify each accumulated piece as load-bearing + for a specific finding or failure mode. +- **Minimal change (footprint):** every changed file/hunk must trace to + one of (a) the PR's original problem, (b) an accepted review finding, + (c) fixing a failing check. The audit produces a traceability table; + hunks with no trace are deletion candidates. This axis is nearly + mechanical, which is what keeps the audit honest — a `sound` verdict + requires an accounted origin for every chunk of growth. + +The two axes are distinct: a fix can be structurally simple yet +footprint-wide, or footprint-tight yet guard-stacked. Either axis +failing is `drift`, and the verdict must name which. + +Before any edit in an audit round the agent writes +`${WORKDIR}/growth-audit.json` (verdict-before-edit is a protocol +requirement; the gate below enforces presence and shape): + +```json +{ + "verdict": "sound | drift | conflict", + "kiss": { "result": "pass | fail", "simpler_alternative": "… | null" }, + "minimal_change": { "result": "pass | fail", "untraceable_hunks": ["…"] }, + "rationale": "…" +} +``` + +Routing per verdict, same round: + +- `sound` — the approach is justified; continue addressing feedback + normally (the remaining Criticals etc.). +- `drift` — implement the named simpler alternative and/or the deletion + list first (typically net-negative), then continue addressing + feedback. +- `conflict` — two defensible directions and the choice is not the + agent's: STOP `BLOCKED` with a handoff that carries the audit's + reasoning. This is the ONLY growth-related path to a human, and the + human receives a narrowed question with evidence, not "the diff is + too big". + +### C. Verdict gate (.github/scripts/run-autofix-review-verification.sh) + +In a round tagged `KISS_AUDIT`, a missing or malformed +`growth-audit.json` fails verification NON-retryable: the round reports +failure and the next scan re-runs the audit. A malformed verdict is +agent misbehavior, not a build problem, so the repair pass cannot fix +it and must not be invoked. This closes the rubber-stamp hole by the +absence side: an audit round that skips the audit cannot push. The tag +reaches the gate as a verify-step env (same pattern as +`FOOTPRINT_ENFORCE`), and shape validation uses `jq`, already a +workflow dependency. Shape validation enforces the taxonomy where it +is unambiguous (`sound` requires both axes `pass`, `drift` at least +one `fail`, `conflict` unconstrained), rejects multi-document verdict +files, and enforces the conflict routing: a `conflict` verdict whose +round did not stop with a handoff fails NON-retryable — conflict must +STOP BLOCKED, never push. The verify step runs on `always()`, and the +check must sit before the gate script's no-commit/failure.md +early-exits so it also applies to no-op audit rounds (a verdict of +`sound` with nothing left to fix still requires the audit artifact) +AND to conflict rounds (whose BLOCKED stop exits via `failure.md`; +the verdict must be validated and surfaced before that exit, or the +trail marker never posts and the park never engages). + +### D. Verdict routing and the audit trail (qwen-autofix.yml report step) + +The report never re-reads `growth-audit.json`: each gate records the +verdict it VALIDATED as a step output (`audit_verdict`), 'Finalize +verification' surfaces the verdict of the pass whose OUTCOME was +selected (a repair pass legitimately re-audits — its feedback rebuild +keeps the audit section and the SKILL mandates audit-first — so its +gate-validated verdict is the one the round's code was judged by; a +repair that validated nothing falls back to the first pass's validated +verdict), and both report steps consume that single output. The +gate-validated verdict is the only verdict that may reach the trail +marker and the re-arm. + +Residual, shared trust domain: the verdict file is written during the +agent step, where branch code runs on a predictable WORKDIR, and the +gate's first read necessarily comes AFTER that — the gate validates +shape, taxonomy, and routing, but can never prove the file's +provenance against code that ran before its read (a planted +shape-valid `sound` replacing an honest `drift`/`conflict`). Blast +radius is control-plane forgery bounded by the window caps, held by +STRUCTURAL invariants rather than an enumerated channel list (the +entrance-by-entrance approach kept growing new entrances each review +round): + +- A push requires `outcome=fixed|noop`, and 'Finalize verification' + accepts those two outcomes ONLY from a pass whose step CONCLUSION is + success — a gate that reached them exited 0. A gate killed mid-check + concludes failure, so a forged `outcome=fixed` + `verified_head` + appended to its discovered output file is discarded there (read as a + crashed gate, retried) and never reaches the push condition. Every + live exit additionally writes its own outcome AFTER the checks, so a + mid-check append loses last-write-wins even without a kill. +- The gate launches through the workflow's `env -i` clean-child pattern + with a step-level `BASH_ENV`/`SHELLOPTS` pin: bash sources a planted + `BASH_ENV` at process STARTUP, before any body-side unset runs, so + the pin + allowlisted child close the class instead of enumerating + it. The runner's `$GITHUB_ENV`/`$GITHUB_PATH`/`$GITHUB_STEP_SUMMARY` + BACKING files under `$RUNNER_TEMP/_runner_file_commands/` (which stay + discoverable after the variable strip) are locked read-only for the + step's lifetime, so no check can append an environment plant into the + later PAT-bearing steps. The directory itself stays writable — the + runner creates the next step's backing files there at step start, and + locking it would stall every later step of the job — which leaves a + rename-over residual (create a new file and rename it onto a locked + one): narrower than the open append, and priced deliberately. +- The control bits ride the gate's own defended output: `kiss_audit` + is recorded before any branch code runs and re-appended at EVERY exit + with the same last-writer discipline as the verdict; later steps + consume it (like `audit_verdict`) through the finalize chain, never + steps.prepare's raw copy except as the crash fallback for a pass that + died before recording it. + +Known residuals, stated rather than claimed closed: the `$GITHUB_OUTPUT` +backing file itself stays writable (the gate must write it), so a +CONCURRENT detached writer spawned by branch code and outliving the gate +can still race the last append — outcome flips are blocked by the +conclusion gate above, and a forged trail marker on a FAILED round +cannot re-arm (the failure path never re-arms); and steps.prepare's +`kiss_audit` copy is consumed as the fallback only when no gate +recorded the bit (a crash path with no push). + +Every audit round posts its verdict in the round report comment with a +machine-readable marker +(``), so later +rounds' audits can read the trail — a second audit after a prior +`sound` IN THE SAME WINDOW sees that its predecessor already blessed +the approach and must bring new evidence to repeat the verdict. The +trail and its new-evidence obligation are per-window: the feedback +reader filters on the live window key, and a completed `sound` verdict +re-arms, which moves the key past the marker — so a `sound`→re-arm +chain is invisible from inside each round in it, and the +human-greppable comment stream is the only cross-window bound. The +marker's `win` must be `steps.prepare.outputs.growth_base_win` (the key the baseline was READ +under), for the same reason the growth-now marker uses it: a conflict +round is exempt from supersede discard and can run with a stale window +after a re-arm, so a marker written under the dead key would be +invisible to every later read. + +On `verdict=sound` on a COMPLETED round, the report step additionally +posts the re-arm marker comment (``). This reuses the existing +`LIVE_REARM_KEY` machinery exactly (window key = latest +`takeover-ack engaged` or `autofix-rearm` marker): the watermark +releases, queued old-window jobs supersede themselves, and the next +round re-anchors the growth baseline at the CURRENT size, so the +remaining work gets a fresh budget (completed-round report paths only +— a round that FAILED records the verdict but never re-arms). +Effectively an automatic, audit-gated `/retry`. + +Explicit decision: the re-arm has full `/retry` semantics — the +per-window round counter and the suggestion valve reset too. Continuing +to solve the problem includes suggestions; if the regenerated +suggestions reproduce the bloat, the brake re-trips after another full +budget of growth and re-audits with the trail visible. +`TAKEOVER_MAX_ROUNDS` bounds each window individually; a chain of +`sound` re-arms is bounded only by the public audit trail and +milestone prompts, not by any global cap. + +On `verdict=drift` there is no re-arm: the simplification is expected +to shrink the diff, and the brake re-measures naturally next round. + +### E. Budget deferral through the #9189 queue (depends on #9189) + +PR #9189 (unmerged as of this writing) adds the fourth address-review +disposition, Defer to follow-up: a VERIFIED finding whose fix lies +outside the PR's footprint/mainline is recorded in +`deferred-findings.json` and upserted into one per-PR tracking issue +that survives the merge. This design extends that reason taxonomy with +a budget class: an in-footprint, verified finding that does not fit the +window's remaining growth budget is deferred through the SAME pipeline +(single issue upsert, rc-id dedupe, token neutralization, thread reply, +left open). The existing "defer requires VERIFIED" constraint applies +unchanged, which is what prevents budget deferral from becoming a dump. + +Until #9189 lands, sections A–D + F stand alone; the unaffordable tail +then simply stays deferred by Critical-only (no loss, no structured +queue). + +### F. defer-to-human narrowed and idempotent + +- Growth reaches a human only via a `conflict` verdict (section B). The + skill's existing non-growth defer-to-human categories (product/scope + choices, contradictory reviewers) are unchanged. +- Conflict-handoff idempotence: once a conflict handoff has been posted + for this window, scans with no new wake since post nothing and do not + launch the agent. The wake set is feedback the loop cannot produce + itself: a trusted-human review or comment, or a failing check from + OUTSIDE the loop's fleet. Excluded wholesale from the checks leg: the + Qwen Autofix workflow's OWN check runs (address lanes included — under + a park no address round can legitimately run, so any review-address + check newer than the marker is the conflict round's OWN failed check, + and counting it would let the loop's own output unpark the round it + came from), AND the loop's sibling machinery — the review workflow + (re-fired by every head the loop's own base-update merge creates), + the CI-failure patrol (cron re-runs on the unchanged head), and the + fork bridge/signal lanes (the loop's own checks for fork PRs). All of + it completes after both park clocks with no human in the input, and + the wasted failure rounds would feed the consecutive-failure cap + toward a terminal lockout on the exact PR a human is settling. + Belt-and-braces with the name exclusion, the loop performs NO head + moves while a handoff pends: the scan's stale-base auto-update and + the conflict round's own stale-base retry both skip parked PRs, so + any check newer than both clocks ran on a head a human moved. + `/retry` (which moves the window key past the marker) is the + sanctioned lift. This fixes the handoff churn in problem 3 for the + one remaining stopping path; the non-stopping paths do not churn by + construction. + +## State machine + +Before: + +``` +normal → critical-only → (2+ over-budget rounds, not shrinking) → STOP, defer-to-human +``` + +After: + +``` +normal → critical-only (+ audit round at first budget breach) + ├─ verdict sound → continue; re-arm window at current size + ├─ verdict drift → simplify (net-negative), then continue + └─ verdict conflict → ONE idempotent handoff with audit evidence +affordable work exhausted → terminal success: core landed, + tail in the per-PR deferral issue, label released +``` + +## Walkthrough: PR #9213 under this design + +Round 3 (first breach): audit round. KISS axis — the accumulated +hardening (line-scoped polarity guard, single-receipt-form +certification, and the rest) each traces to a finding; no simpler +named alternative. Minimal axis — the 562-line repro block and +671-line retirement tests trace to the PR's original problem and +accepted findings. Verdict `sound` → re-arm → window baseline +re-anchored at current size. Rounds 4+: the two remaining Criticals +(small fixes) land well inside a fresh 400/400 budget; the reviewer's +marginal tail is deferred by Critical-only (and, post-#9189, its +verified off-mainline items queue into the tracking issue). PR +converges and the label releases with zero human rounds. + +## Failure modes and bounds + +- **Audit wrongly blesses real drift.** Bounded: the next breach in + the SAME window re-audits with the prior verdict marker visible + (across a `sound` re-arm the marker sits under the old window key — + the cross-window bound is the public comment stream), and repeated + `sound` verdicts against monotonically growing diffs are a public, + greppable pattern for maintainers. +- **Audit wrongly condemns a sound design.** Cost is one extra + simplification round; the deletion list is traceability-derived and + posted, so a bad list is visible before it is re-derived next round. + The failure mode is a wasted round, never a stop. +- **Rubber-stamping.** Burden inverted (assume over-engineered), + traceability table required, verdict gate rejects absent/malformed + verdicts, trail is public. +- **Cost.** One audit round per budget breach — one agent run, + replacing the handoff round that ran anyway. +- **Existing brakes untouched.** Round-based Critical-only, + per-window human feedback budgets, failed-check handling, and + `TAKEOVER_MAX_ROUNDS` all remain as they are. + +## Test impact + +`scripts/tests/qwen-autofix-workflow.test.js` pins the current +behavior and must be rewritten with the change: + +- The whole `it('escalates to a maintainer-decision handoff …')` case + (~L6742–7378): it pins the `GROWTH_DIVERGENCE_ROUNDS` variable, + extracts and executes the divergence block against a fixture history + of `autofix-growth-now` markers (deduped on `run=`, ordered on + `measured=`, filtered by the comparability cutoff), pins the + malformed-rounds sanitize fallback, then executes the feedback.md + handoff-guard block and asserts `## Needs a maintainer's decision`, + `defer-to-human`, and the SKILL text `this PR is not converging`. + All of it is replaced by the audit trigger, verdict routing, and the + new SKILL text. The fixture marker helper itself survives — the audit + reads the same `autofix-growth-now` history the divergence ladder + did. +- New pins: audit trigger at first breach (and NOT on round-based + Critical-only without a breach); verdict gate rejecting a KISS_AUDIT + round with missing/malformed `growth-audit.json`; + `` trail marker in the report; + `` posted iff verdict is `sound` on a completed + round (the failure path records the verdict but never re-arms); + conflict handoff idempotence. + +## Rollout and dependencies + +- Sections A–D and F are independent and can land first. +- Section E depends on #9189 merging; land #9189 first so there is + exactly one deferral pipeline. +- #9213 itself does not wait for this design: `@qwen-code /retry` is + today's manual equivalent of the `sound` exit, merging as-is plus + follow-up issues is today's manual equivalent of the deferral exit. + +## Non-goals + +- The bot never merges on its own; terminal success still ends in + human review/merge. +- Review-side finding generation is not made budget-aware here (the + reviewer keeps producing findings; the audit + deferral absorb them). + Making the review pipeline aware of budget state is a follow-up lever. +- No topology-scaled budgets. The audit makes the exact budget value + far less load-bearing; scaling it is deferred unless evidence says + otherwise. diff --git a/scripts/tests/package-scripts.test.js b/scripts/tests/package-scripts.test.js index 0b5027a4c4..0d30225400 100644 --- a/scripts/tests/package-scripts.test.js +++ b/scripts/tests/package-scripts.test.js @@ -628,7 +628,7 @@ describe('package scripts', () => { } expect(getWorkflowStep(reviewJob, 'Verification gate')).toContain( - 'bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"', + 'bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh"', ); }); }); diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 534c8ca298..87645fa5e5 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -518,11 +518,13 @@ describe('qwen-autofix workflow', () => { // Every failed-check selector must guard against the loop reading its OWN // runs as feedback about the PR. Most selectors carry the review-address // carve-out; the stale-base selector instead excludes ALL Qwen Autofix - // checks (no carve-out), which is strictly narrower. Assert PER SELECTOR - // that its own text carries one guard or the other — a global count is - // vacuous here because `!= "Qwen Autofix"` is a substring of the carve-out - // expression, so every carve-out selector increments BOTH counters and a - // guardless selector slips through (proven by A/B mutation). + // checks (no carve-out), which is strictly narrower, and the conflict- + // park wake filter excludes the loop's whole FLEET by name (wider + // still). Assert PER SELECTOR that its own text carries one guard or + // another — a global count is vacuous here because `!= "Qwen Autofix"` + // is a substring of the carve-out expression, so every carve-out + // selector increments BOTH counters and a guardless selector slips + // through (proven by A/B mutation). const scanCheckSelectors = reviewScanJob.match(/IN\("(?:FAILURE|QUEUED)"/g) ?? []; expect(scanCheckSelectors.length).toBeGreaterThanOrEqual(3); @@ -534,6 +536,7 @@ describe('qwen-autofix workflow', () => { return ( !/startswith\("review-address"\)/.test(sel) && !/!= "Qwen Autofix"/.test(sel) && + !/IN\("Qwen Autofix"/.test(sel) && // The review-in-flight gate (#8888) selects BY NAME for the LLM // review check — a liveness probe, not a feedback selector, so it // needs neither the review-address carve-out nor the workflow guard. @@ -3043,11 +3046,13 @@ describe('qwen-autofix workflow', () => { // invocations never burn an agent cycle on a no-action report. expect(reviewScanJob).toContain("COMMAND_FILTER='^\\s*@qwen-code /'"); expect(reviewScanJob).toContain('test($cf) | not'); - // Five sites now: the four feedback/deferral exclusions plus the - // over-budget census, which must not count command comments as - // feedback batches either. + // Seven sites now: the four feedback/deferral exclusions, the + // over-budget census (command comments are not feedback batches), the + // conflict handoff wake filter, and its scan-side mirror for the + // stale-base park gate (a /command comment is not a trusted-human + // response and must not unpark a conflict verdict in either). expect(workflow.split('test("^\\\\s*@qwen-code /") | not').length - 1).toBe( - 5, + 7, ); }); @@ -3071,11 +3076,13 @@ describe('qwen-autofix workflow', () => { // Pin the total --paginate code-site count so ANY new paginated site // forces a deliberate test update, however it is spaced or line-wrapped: // bump this count AND pipe the new site through the normalizer (bumping - // the count below too) — bumping this pin alone leaves toBe(10) green. - expect(workflow.split('--paginate').length - 1).toBe(19); + // the count below too) — bumping this pin alone leaves toBe(12) green. + expect(workflow.split('--paginate').length - 1).toBe(21); // scan ic + pr-events + ic re-fetch + scan rv/rc + prepare rv/rc/ic + // report COMMENTS_JSON fallback + the cap-branch release-evidence events - // fetch (R4-1) = ten normalized fetch sites. The + // fetch (R4-1) + the scan park gate's rv/rc fetches (the wake mirror + // needs the same human-feedback legs prepare reads) = twelve normalized + // fetch sites. The // blocked-takeover status lookup is deliberately NOT among them: like the // sibling STATUS_ID read, it consumes the page stream inline via // `--jq ... | .id` into `tail -1` and never lands in a WORKDIR json file, @@ -3091,7 +3098,7 @@ describe('qwen-autofix workflow', () => { // paginate whose `--jq '…nodes[]'` stream is slurped straight into // THREADS_JSON — so it bumps the total pin above without joining the // normalizer count below. - expect(workflow.split("jq -s 'add // []'").length - 1).toBe(10); + expect(workflow.split("jq -s 'add // []'").length - 1).toBe(12); // Empty-input semantics: a total gh failure feeds the fallback an EMPTY // stream, where the normalizer filter must yield '[]' and not 'null' — // the PRIOR_HEADS consumer below iterates the result with .[], which @@ -7573,21 +7580,56 @@ exit 1 expect(skill).toContain('Deferred non-Critical feedback'); }); - it('escalates to a maintainer-decision handoff when the diff keeps growing past budget (non-convergence)', () => { + it('turns a budget breach into a growth-audit round instead of a divergence stop', () => { // Critical-only only trims non-Criticals, so a Critical-driven diff keeps - // growing anyway. The divergence detector reads this window's prior - // per-round growth markers and, once the brake has been over budget for - // >= GROWTH_DIVERGENCE_ROUNDS rounds and the diff is still not shrinking, - // flags the round to STOP and hand off — not patch again. - expect(workflow).toContain( - "GROWTH_DIVERGENCE_ROUNDS: '${{ vars.QWEN_AUTOFIX_GROWTH_DIVERGENCE_ROUNDS || 2 }}'", + // growing anyway — but a budget breach no longer escalates to a + // maintainer handoff. The divergence ladder (GROWTH_DIVERGENCE_ROUNDS / + // GROWTH_DIVERGED / PREV_SUM) is retired wholesale: the breach engages + // Critical-only AND makes the round a growth-audit round — a size signal + // triggers a JUDGMENT, never a stop. Pin the old machinery gone from the + // workflow entirely, so a resurrection fails here instead of riding along + // silently under a renamed variable. + expect(workflow).not.toContain('GROWTH_DIVERGENCE_ROUNDS'); + expect(workflow).not.toContain('GROWTH_DIVERGED'); + expect(workflow).not.toContain('growth_diverged'); + expect(workflow).not.toContain('PREV_SUM'); + // The audit rides the prepare step's kiss_audit output into BOTH + // verification gates and BOTH report steps — deleted wiring would + // silently inert the verdict gate and the trail marker (the writers on + // either end fall back to :-false/:-none, so only an end-to-end count + // catches it). The failure/handoff report needs the tag too: it still + // emits the verdict trail marker (emit_growth_audit_marker false). + expect(prepareBranchAndFeedbackStep).toContain( + 'echo "kiss_audit=${KISS_AUDIT}"', ); - // Extract the divergence block and run it against fixture history. - const divBlock = prepareBranchAndFeedbackStep.match( - /(if \[\[ ! "\$\{GROWTH_DIVERGENCE_ROUNDS\}"[\s\S]*?GROWTH_DIVERGED='true'\n\s+fi\n\s+fi)/, - )?.[1]; - expect(divBlock).toBeTruthy(); - const dir = mkdtempSync(join(tmpdir(), 'autofix-diverge-')); + // The bit enters the FIRST gate from prepare, but every later consumer + // reads the gates' DEFENDED copy (recorded before any branch code ran, + // re-appended at every exit) through the finalize chain — a control + // bit routed around the gate's defenses re-opens the forgery class. + // The repair gate keeps prepare only as the crash fallback for a first + // pass that died before recording the bit. + expect(verificationGateSteps[1]).toContain( + "KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}'", + ); + expect(repairVerificationGateStep).toContain( + "KISS_AUDIT: '${{ steps.verify.outputs.kiss_audit || steps.prepare.outputs.kiss_audit }}'", + ); + expect(pushAndReportStep).toContain( + "KISS_AUDIT: '${{ steps.final_verify.outputs.kiss_audit }}'", + ); + expect(reviewAddressReportStep).toContain( + "KISS_AUDIT: '${{ steps.final_verify.outputs.kiss_audit }}'", + ); + + // Extract the census + audit-trigger block and run it against fixture + // marker history: it counts this window's prior over-budget rounds into + // OVER_ROUNDS_PRIOR (count only — no boolean, no PREV_SUM compare) and + // sets KISS_AUDIT on the FIRST breach. + const auditBlock = prepareBranchAndFeedbackStep.match( + /OVER_ROUNDS_PRIOR=0\n\s+KISS_AUDIT='false'[\s\S]*?echo "kiss_audit=\$\{KISS_AUDIT\}" >> "\$\{GITHUB_OUTPUT\}"/, + )?.[0]; + expect(auditBlock).toBeTruthy(); + const dir = mkdtempSync(join(tmpdir(), 'autofix-audit-')); // Markers carry round= (informational) and run= (GITHUB_RUN_ID) — deduped // on run= and ordered on measured= (the prepare-time instant; created_at // fallback for legacy markers), the per-workflow-run id: a retry or a @@ -7621,12 +7663,10 @@ exit 1 created_at: '2026-06-01T00:00:00Z', body: ``, }); - const diverge = ({ - src, - test, - criticalOnlyGrowth = 'true', + const census = ({ history, - div = 2, + criticalOnlyGrowth = 'true', + netMeasured = 'true', // The comparability cutoff (base update OR external head move); markers // measured at/before it are dropped as incomparable. cutoff = '', @@ -7636,96 +7676,93 @@ exit 1 currentRun = 9999, }) => { writeFileSync(join(dir, 'ic.json'), JSON.stringify(history)); - // The printf result is the FINAL line; a malformed-div round also emits - // a `::warning::` annotation to stdout first, so take the last line. - return execFileSync( + const outFile = join(dir, 'gh-output.txt'); + writeFileSync(outFile, ''); + const line = execFileSync( 'bash', [ '-c', `set -e\nAUTOFIX_BOT=qwen-code-dev-bot\nLIVE_REARM_KEY=W1\nWORKDIR=${dir}\n` + - `NET_MEASURED=true\nCRITICAL_ONLY_GROWTH=${criticalOnlyGrowth}\n` + + `NET_MEASURED=${netMeasured}\nCRITICAL_ONLY_GROWTH=${criticalOnlyGrowth}\n` + `GROWTH_NOW_CUTOFF='${cutoff}'\nGITHUB_RUN_ID=${currentRun}\n` + - `GROWTH_SRC=${src}\nGROWTH_TEST=${test}\nGROWTH_DIVERGENCE_ROUNDS=${div}\n` + - `${divBlock}\nprintf '\\n%s %s' "$GROWTH_DIVERGED" "$OVER_ROUNDS_PRIOR"`, + `GITHUB_OUTPUT=${outFile}\n` + + `${auditBlock}\nprintf '%s %s' "$OVER_ROUNDS_PRIOR" "$KISS_AUDIT"`, ], { encoding: 'utf8' }, - ) - .trim() - .split('\n') - .pop(); + ).trim(); + return { line, output: readFileSync(outFile, 'utf8') }; }; const climbing = [ marker(300, 200, 'true', 1), marker(400, 250, 'true', 2), marker(500, 300, 'true', 3), ]; - // 3 prior over-budget rounds, still climbing → diverged. - expect(diverge({ src: 550, test: 300, history: climbing })).toBe('true 3'); - // Same history but the diff SHRANK below the previous round's sum → not. - expect(diverge({ src: 100, test: 100, history: climbing })).toBe('false 3'); - // EXACTLY at the threshold (2 prior rounds, div=2), still climbing → diverged. + // 3 prior over-budget rounds counted, and the breach makes this round an + // audit round. The outputs feed the feedback renderer + report env. + const climbed = census({ history: climbing }); + expect(climbed.line).toBe('3 true'); + expect(climbed.output).toBe('critical_only_growth=true\nkiss_audit=true\n'); + // The audit fires at the FIRST breach — zero priors still audit. + expect(census({ history: [] }).line).toBe('0 true'); + // 2 and 1 prior over-budget rounds count exactly. expect( - diverge({ - src: 500, - test: 300, + census({ history: [marker(300, 200, 'true', 1), marker(400, 250, 'true', 2)], - }), - ).toBe('true 2'); - // Current sum EQUAL to the previous round's sum (not shrinking) → diverged. - expect(diverge({ src: 500, test: 300, history: climbing })).toBe('true 3'); - // A transient SPIKE does not raise the bar forever: after sums 350, 1150 - // (spike), 400, a plateau at 400 is still >= the PREVIOUS round (400), so a - // real runaway escalates — the window-wide max (1150) would have suppressed - // it for the rest of the window. - expect( - diverge({ - src: 250, - test: 150, - history: [ - marker(200, 150, 'true', 1), - marker(700, 450, 'true', 2), - marker(250, 150, 'true', 3), - ], - }), - ).toBe('true 3'); - // Only 1 prior over-budget round (< threshold) → not diverged yet. - expect( - diverge({ src: 999, test: 999, history: [marker(500, 300, 'true', 1)] }), - ).toBe('false 1'); + }).line, + ).toBe('2 true'); + expect(census({ history: [marker(500, 300, 'true', 1)] }).line).toBe( + '1 true', + ); // The CURRENT run's own markers (run == GITHUB_RUN_ID) are excluded: a // re-run of a failed job keeps the run id and its failed attempt already // posted a marker, which must not count as a PRIOR over-budget round. Here // run 9999 is the current run; only the genuine prior (run 1001) counts. expect( - diverge({ - src: 999, - test: 999, + census({ currentRun: 9999, history: [ marker(500, 300, 'true', 1, 'W1', { run: 1001 }), marker(600, 400, 'true', 1, 'W1', { run: 9999 }), ], - }), - ).toBe('false 1'); + }).line, + ).toBe('1 true'); // A retry-doubled marker (same run id) counts ONCE. expect( - diverge({ - src: 999, - test: 999, + census({ history: [ marker(500, 300, 'true', 1, 'W1', { run: 1001 }), marker(500, 300, 'true', 1, 'W1', { run: 1001 }), ], - }), - ).toBe('false 1'); - // When a run was re-run and posted two markers with DIFFERENT sums, the - // LATEST attempt (by measured=) wins, not jq's stale first: run 1002's - // fresh attempt (sum 300 @ T2) is PREV_SUM, so current 400 >= 300 → - // diverged. Keeping the stale first (sum 900) would read 400 >= 900 → not. + }).line, + ).toBe('1 true'); + // Within-run collapse keys on measured=, NOT array position: run 1002's + // LATEST measurement (by measured=) is over budget, but it sits BETWEEN + // two under-budget attempts in the comment list — an array-first OR an + // array-last collapse instead of max_by(measured=) would drop the run + // (#9192 R2-4 shape). expect( - diverge({ - src: 250, - test: 150, + census({ + history: [ + marker(300, 200, 'true', 1, 'W1', { run: 1001 }), + marker(80, 40, 'false', 2, 'W1', { + run: 1002, + measured: '2026-01-01T00:02:00Z', + }), + marker(500, 400, 'true', 2, 'W1', { + run: 1002, + measured: '2026-01-01T00:09:00Z', + }), + marker(90, 45, 'false', 2, 'W1', { + run: 1002, + measured: '2026-01-01T00:05:00Z', + }), + ], + }).line, + ).toBe('2 true'); + // …and a re-run posting two over-budget attempts still counts exactly + // ONCE — the collapse is a dedup, whatever attempt represents the run. + expect( + census({ history: [ marker(300, 200, 'true', 1, 'W1', { run: 1001 }), marker(600, 300, 'true', 2, 'W1', { @@ -7737,94 +7774,73 @@ exit 1 measured: '2026-01-01T00:02:00Z', }), ], - }), - ).toBe('true 2'); - // measured= order inverts run order across DISTINCT runs — a failed - // job's re-run keeps its OLD run id but stamps a NEWER measured=: - // PREV_SUM must follow measured=, so the current 150 >= 100 runaway - // escalates. Reverting to run-id ordering would read run 1002's stale - // 900 as PREV_SUM and suppress it (#9192 R2-4). - expect( - diverge({ - src: 100, - test: 50, - history: [ - marker(60, 40, 'true', 1, 'W1', { - run: 1001, - measured: '2026-01-01T00:09:00Z', - }), - marker(500, 400, 'true', 2, 'W1', { - run: 1002, - measured: '2026-01-01T00:02:00Z', - }), - ], - }), - ).toBe('true 2'); + }).line, + ).toBe('2 true'); // Two DISTINCT runs that share round= AND a frozen eval watermark (the // state-triggered conflict lane: a push stamps NEXT_ROUND, the following // no-op re-stamps the same ROUND, neither NEWEST nor ROUND advances) are // counted SEPARATELY by their distinct run ids — round=/wm alone (the - // pre-fix key) would have collapsed them and stalled the handoff forever. + // pre-fix key) would have collapsed them and stalled the count forever. expect( - diverge({ - src: 500, - test: 300, + census({ history: [ marker(300, 200, 'true', 2, 'W1', { run: 1001 }), marker(400, 250, 'true', 2, 'W1', { run: 1002 }), marker(500, 300, 'true', 2, 'W1', { run: 1003 }), ], - }), - ).toBe('true 3'); - // "Most recent" is the highest RUN id, not the max sum and not the first: - // prior over-budget sums 900 (run 1) then 500 (run 2, agent shrank), a - // partial regrow to 700 is >= the most-recent 500 → diverged. Comparing - // against the first/max (900) would wrongly suppress it (700 < 900). - expect( - diverge({ - src: 400, - test: 300, - history: [ - marker(600, 300, 'true', 1, 'W1', { run: 1001 }), - marker(300, 200, 'true', 2, 'W1', { run: 1002 }), - ], - }), - ).toBe('true 2'); - // Not over budget THIS round → still counts (accurate trajectory) but no handoff. - expect( - diverge({ - src: 999, - test: 999, - criticalOnlyGrowth: 'false', - history: climbing, - }), - ).toBe('false 3'); + }).line, + ).toBe('3 true'); + // Round-based Critical-only WITHOUT a breach is NOT an audit round: the + // audit rides on TOP of the growth breach, it does not replace the + // round-based ladder — and the count still reports the trajectory. + const roundsOnly = census({ + criticalOnlyGrowth: 'false', + history: climbing, + }); + expect(roundsOnly.line).toBe('3 false'); + expect(roundsOnly.output).toBe( + 'critical_only_growth=false\nkiss_audit=false\n', + ); + // Unmeasured growth (no trusted merge base) never audits: the census + // stays 0 (the jq read is gated on NET_MEASURED). criticalOnlyGrowth is + // the only state reachable unmeasured — see the zeroing pin below. + const unmeasured = census({ + netMeasured: 'false', + criticalOnlyGrowth: 'false', + history: climbing, + }); + expect(unmeasured.line).toBe('0 false'); + expect(unmeasured.output).toBe( + 'critical_only_growth=false\nkiss_audit=false\n', + ); + // …and a breach is UNREACHABLE unmeasured in the first place: the nets + // are zeroed when the measurement failed, so the budget compare cannot + // trip and KISS_AUDIT (which keys solely on CRITICAL_ONLY_GROWTH) stays + // false — the verdict needs numbers to judge, and it gets them or not + // at all. + expect(prepareBranchAndFeedbackStep).toContain( + '[[ "${NET_MEASURED}" != \'true\' ]] && { GROWTH_SRC=0; GROWTH_TEST=0; }', + ); // Prior markers under a DIFFERENT window key don't count. expect( - diverge({ - src: 999, - test: 999, + census({ history: [ marker(500, 300, 'true', 1, 'W2'), marker(600, 400, 'true', 2, 'W2'), ], - }), - ).toBe('false 0'); + }).line, + ).toBe('0 true'); // Markers from a non-bot author don't count. expect( - diverge({ - src: 999, - test: 999, + census({ history: climbing.map((m) => ({ ...m, user: { login: 'attacker' } })), - }), - ).toBe('false 0'); + }).line, + ).toBe('0 true'); // Markers measured at/before the comparability cutoff (a base update OR an // external head move) are excluded — re-anchoring makes pre-cutoff sums // incomparable; only the post-cutoff round remains. expect( - diverge({ - src: 999, - test: 999, + census({ cutoff: '2026-01-01T12:00:00Z', history: [ marker(500, 300, 'true', 1, 'W1', { @@ -7837,31 +7853,27 @@ exit 1 measured: '2026-01-01T18:00:00Z', }), ], - }), - ).toBe('false 1'); + }).line, + ).toBe('1 true'); // …and the boundary is STRICT: a marker measured exactly AT the cutoff // is dropped as incomparable (at second granularity a same-second stamp // and base-update comment can collide) — a `>` → `>=` flip ships green // without this pin (#9192 R4-7). expect( - diverge({ - src: 999, - test: 999, + census({ cutoff: '2026-01-01T12:00:00Z', history: [ marker(500, 300, 'true', 1, 'W1', { measured: '2026-01-01T12:00:00Z', }), ], - }), - ).toBe('false 0'); + }).line, + ).toBe('0 true'); // A re-run whose FRESH attempt came back under budget must not be // represented by its own stale over=true attempt: the per-run collapse // happens BEFORE the over-filter, so the run drops out entirely. expect( - diverge({ - src: 999, - test: 999, + census({ history: [ marker(300, 150, 'true', 1, 'W1', { run: 1001, @@ -7872,8 +7884,8 @@ exit 1 measured: '2026-01-01T00:09:00Z', }), ], - }), - ).toBe('false 0'); + }).line, + ).toBe('0 true'); // Mirror of that case for the FAILURE path's inert marker: a re-run // attempt that crashed BEFORE prepare posts over=false with NO measured= // (MEASURED_AT empty), so its fallback is the comment's created_at — @@ -7882,9 +7894,7 @@ exit 1 // over that fallback, or the inert marker erases the run's real // over-budget count (#9192 R3-1). expect( - diverge({ - src: 999, - test: 999, + census({ history: [ { user: { login: 'qwen-code-dev-bot' }, @@ -7897,16 +7907,13 @@ exit 1 body: '', }, ], - }), - ).toBe('false 1'); - // Same defect at the handoff threshold: two real over-budget priors, the - // second erased by the inert marker — without the explicit-measured - // preference the count drops to 1 and the divergence handoff (div=2) is - // suppressed while the diff keeps climbing. + }).line, + ).toBe('1 true'); + // Same defect shape at count 2: the second over-budget round erased by an + // inert marker — without the explicit-measured preference the census + // under-reports the trajectory the audit judges. expect( - diverge({ - src: 500, - test: 300, + census({ history: [ { user: { login: 'qwen-code-dev-bot' }, @@ -7924,15 +7931,13 @@ exit 1 body: '', }, ], - }), - ).toBe('true 2'); + }).line, + ).toBe('2 true'); // …and two LEGACY markers for the same run (neither carries measured=) // still collapse on the created_at fallback: the explicit-measured // preference must not disturb fallback-vs-fallback ordering. expect( - diverge({ - src: 999, - test: 999, + census({ history: [ { user: { login: 'qwen-code-dev-bot' }, @@ -7945,15 +7950,13 @@ exit 1 body: '', }, ], - }), - ).toBe('false 0'); + }).line, + ).toBe('0 true'); // Backward compatibility: a marker posted BEFORE measured= existed still // counts, falling back to its comment's created_at — deploying the // measured= switch must not blank the census of an in-flight window. expect( - diverge({ - src: 999, - test: 999, + census({ history: [ { user: { login: 'qwen-code-dev-bot' }, @@ -7961,13 +7964,11 @@ exit 1 body: '', }, ], - }), - ).toBe('false 1'); + }).line, + ).toBe('1 true'); // …and a legacy marker is still subject to the cutoff, via that fallback. expect( - diverge({ - src: 999, - test: 999, + census({ cutoff: '2026-01-01T12:00:00Z', history: [ { @@ -7976,16 +7977,14 @@ exit 1 body: '', }, ], - }), - ).toBe('false 0'); + }).line, + ).toBe('0 true'); // …and the created_at fallback ITSELF is pinned: a legacy marker whose // created_at sits AFTER the cutoff still counts. Dropping the fallback // (measured= absent → "") would exclude every post-update legacy marker // and under-count the census (#9192 R2-3). expect( - diverge({ - src: 999, - test: 999, + census({ cutoff: '2026-01-01T12:00:00Z', history: [ { @@ -7994,33 +7993,24 @@ exit 1 body: '', }, ], - }), - ).toBe('false 1'); - // A malformed GROWTH_DIVERGENCE_ROUNDS falls back to 2 (the sanitize guard - // at the top of the block) instead of crashing the `-ge` arithmetic: two - // prior over-budget rounds still climbing → diverged. + }).line, + ).toBe('1 true'); + // over=false markers (rounds that pulled back under budget) never count — + // a one-off overshoot that recovered must not inflate the trajectory. + // Pins the `.over == "true"` filter. expect( - diverge({ - src: 500, - test: 300, - div: 'abc', - history: [marker(300, 200, 'true', 1), marker(400, 250, 'true', 2)], - }), - ).toBe('true 2'); - // over=false markers (rounds that pulled back under budget) count neither - // toward OVER_ROUNDS_PRIOR nor as PREV_SUM — a one-off overshoot that - // recovered must NOT escalate. Pins the `.over == "true"` filter. - expect( - diverge({ - src: 999, - test: 999, + census({ history: [ marker(500, 300, 'true', 1), marker(100, 50, 'false', 2), marker(90, 40, 'false', 3), ], - }), - ).toBe('false 1'); + }).line, + ).toBe('1 true'); + // The census's jq-failure and non-numeric fallbacks are load-bearing (a + // crash here would kill prepare, not just the brake) — pin their shape. + expect(auditBlock).toContain('2> /dev/null || echo 0'); + expect(auditBlock).toContain('|| OVER_ROUNDS_PRIOR=0'); // Writer→reader round-trip: expand EACH real writer echo through bash and // feed the marker it produces back through the extracted reader, so a // format drift between writer and reader (the marker is encoded four @@ -8048,9 +8038,7 @@ exit 1 ).trim(); // Counted as 1 prior over-budget run — 0 is what a format mismatch or a // dead key= (e.g. key=${WINDOW} after a re-arm) would yield. - return diverge({ - src: 999, - test: 999, + return census({ history: [ { user: { login: 'qwen-code-dev-bot' }, @@ -8058,19 +8046,18 @@ exit 1 body: produced, }, ], - }); + }).line; }; - expect(roundTrip('NEXT_ROUND')).toBe('false 1'); // push path - expect(roundTrip('ROUND')).toBe('false 1'); // no-op path - expect(roundTrip('MARK_ROUND')).toBe('false 1'); // failure/handoff path + expect(roundTrip('NEXT_ROUND')).toBe('1 true'); // push path + expect(roundTrip('ROUND')).toBe('1 true'); // no-op path + expect(roundTrip('MARK_ROUND')).toBe('1 true'); // failure/handoff path // …and it still round-trips when MEASURED_AT is empty — prepare never // ran (#9192 R2-1) or the round could not measure (#9192 R4-3): measured= // omits itself rather than emit an empty value no scan can match, so the // marker survives on the created_at fallback instead of silently dropping. - expect(roundTrip('NEXT_ROUND', { omitMeasuredAt: true })).toBe('false 1'); - expect(roundTrip('ROUND', { omitMeasuredAt: true })).toBe('false 1'); - expect(roundTrip('MARK_ROUND', { omitMeasuredAt: true })).toBe('false 1'); - rmSync(dir, { recursive: true, force: true }); + expect(roundTrip('NEXT_ROUND', { omitMeasuredAt: true })).toBe('1 true'); + expect(roundTrip('ROUND', { omitMeasuredAt: true })).toBe('1 true'); + expect(roundTrip('MARK_ROUND', { omitMeasuredAt: true })).toBe('1 true'); // The report writes the per-round growth-now marker on ALL THREE report // paths so the history is complete (a no-op round records its size, and a @@ -8164,28 +8151,26 @@ exit 1 expect(prepareBranchAndFeedbackStep).toContain( 'GROWTH_NOW_CUTOFF="${BASE_UPD_AT}"', ); - // growth_diverged is NOT emitted as a step output — the handoff is - // enforced by the feedback.md text, so a dangling dead output would only - // mislead a future consumer. - expect(prepareBranchAndFeedbackStep).not.toContain('growth_diverged='); - // The trajectory + non-convergence blocks reach the agent via feedback.md, + + // The trajectory + growth-audit blocks reach the agent via feedback.md, // AND their render guards are executed both ways — a flipped guard (inject - // the handoff into converging rounds, or drop it from diverging ones) must + // the audit into converging rounds, or drop it from breaching ones) must // fail here, not ship green. const trajGuard = prepareBranchAndFeedbackStep.match( /if \[\[ "\$\{NET_MEASURED\}" == 'true' \]\]; then\n\s+echo "## Diff growth this window"[\s\S]*?\n\s+fi/, )?.[0]; - const handoffGuard = prepareBranchAndFeedbackStep.match( - /if \[\[ "\$\{GROWTH_DIVERGED\}" == 'true' \]\]; then\n\s+echo "## Needs a maintainer's decision[\s\S]*?\n\s+fi/, - )?.[0]; expect(trajGuard).toBeTruthy(); - expect(handoffGuard).toBeTruthy(); + const auditGuard = prepareBranchAndFeedbackStep.match( + /if \[\[ "\$\{KISS_AUDIT\}" == 'true' \]\]; then[\s\S]*?\n {12}fi(?=\n {12}echo "## Reviews")/, + )?.[0]; + expect(auditGuard).toBeTruthy(); // DISTINCT src/test values so a transposed ${GROWTH_SRC}/${GROWTH_TEST} in // either advisory body fails here (the numbers this feature feeds the // agent must be the right way round). const renderEnv = 'GROWTH_SRC=7\nGROWTH_TEST=9\nGROWTH_BUDGET_SRC_LINES=1\n' + - 'GROWTH_BUDGET_TEST_LINES=1\nOVER_ROUNDS_PRIOR=2\n'; + 'GROWTH_BUDGET_TEST_LINES=1\nOVER_ROUNDS_PRIOR=2\n' + + `AUTOFIX_BOT=qwen-code-dev-bot\nLIVE_REARM_KEY=W1\nWORKDIR=${dir}\n`; const runGuard = (block, vars) => execFileSync('bash', ['-c', `${vars}${block}`], { encoding: 'utf8' }); const trajOn = runGuard(trajGuard, `NET_MEASURED=true\n${renderEnv}`); @@ -8194,29 +8179,591 @@ exit 1 expect( runGuard(trajGuard, `NET_MEASURED=false\n${renderEnv}`), ).not.toContain('## Diff growth this window'); - const handoffOn = runGuard( - handoffGuard, - `GROWTH_DIVERGED=true\n${renderEnv}`, + // Audit round ON: the heading, the numbers, and the window's audit trail + // (a re-audit after a prior verdict must bring new evidence). + writeFileSync( + join(dir, 'ic.json'), + JSON.stringify([ + { + user: { login: 'qwen-code-dev-bot' }, + created_at: '2026-01-02T00:00:00Z', + body: '', + }, + // A trail marker under a DEAD window key is not this window's trail. + { + user: { login: 'qwen-code-dev-bot' }, + created_at: '2026-01-02T00:00:00Z', + body: '', + }, + ]), ); - expect(handoffOn).toContain("## Needs a maintainer's decision"); - expect(handoffOn).toContain('source 7 / test 9'); + const auditOn = runGuard(auditGuard, `KISS_AUDIT=true\n${renderEnv}`); + expect(auditOn).toContain( + '## Growth audit required — this window is over its growth budget', + ); + expect(auditOn).toContain('source 7 / test 9'); + expect(auditOn).toContain('2 prior round(s) already over budget'); + expect(auditOn).toContain('growth-audit.json'); + expect(auditOn).toContain( + 'Prior growth audits this window — a repeated verdict needs new evidence:', + ); + expect(auditOn).toContain('- 2026-01-02T00:00:00Z: verdict=sound'); + expect(auditOn).not.toContain('verdict=conflict'); + // No trail yet → the section says so (the audit is the first). + writeFileSync(join(dir, 'ic.json'), '[]'); + const auditFirst = runGuard(auditGuard, `KISS_AUDIT=true\n${renderEnv}`); + expect(auditFirst).toContain('No prior growth audit this window.'); + // Converging round → the audit section must not render. + expect(runGuard(auditGuard, `KISS_AUDIT=false\n${renderEnv}`)).toBe(''); + + // Conflict-handoff idempotence: a conflict verdict parks the PR at a + // genuinely human call. Until a trusted human responds (or a new failing + // check arrives), scans must not launch agents or post comments — + // review-bot regeneration alone would otherwise churn one identical + // handoff after another. Execute the real block against fixture state. + const conflictBlock = prepareBranchAndFeedbackStep.match( + /CONFLICT_SINCE="\$\(jq[\s\S]*?conflict handoff pending[\s\S]*?\n {10}fi\n/, + )?.[0]; + expect(conflictBlock).toBeTruthy(); + const conflictMarker = (createdAt, win = 'W1') => ({ + user: { login: 'qwen-code-dev-bot' }, + created_at: createdAt, + body: ``, + }); + const T0 = '2026-01-01T00:00:00Z'; + const park = ({ + stale = 'false', + markerCreatedAt = T0, + win = 'W1', + rv = [], + rc = [], + // Extra ISSUE comments appended after the conflict marker itself + // (control-marker/command comments live here in reality). + ic = [], + checks = [], + // Latest stale-base auto-update marker time (empty: none yet). + baseUpdAt = '', + }) => { + writeFileSync( + join(dir, 'ic.json'), + JSON.stringify([conflictMarker(markerCreatedAt, win), ...ic]), + ); + writeFileSync(join(dir, 'rv.json'), JSON.stringify(rv)); + writeFileSync(join(dir, 'rc.json'), JSON.stringify(rc)); + writeFileSync(join(dir, 'checks.json'), JSON.stringify(checks)); + const out = execFileSync( + 'bash', + [ + '-c', + `set -e\nAUTOFIX_BOT=qwen-code-dev-bot\nREVIEW_BOT=qwen-code-ci-bot\n` + + `LIVE_REARM_KEY=W1\nWORKDIR=${dir}\nSTALE=${stale}\n` + + `BASE_UPD_AT='${baseUpdAt}'\n` + + `TRUSTED_ASSOC='["OWNER", "MEMBER", "COLLABORATOR"]'\n` + + `${conflictBlock}\nprintf '%s' "$STALE"`, + ], + { encoding: 'utf8' }, + ); + return { + stale: out.trim().split('\n').pop(), + parked: out.includes('conflict handoff pending'), + }; + }; + // No response at all → the scan idles. + expect(park({})).toEqual({ stale: 'true', parked: true }); + // A NEWER trusted-human review wakes the PR. expect( - runGuard(handoffGuard, `GROWTH_DIVERGED=false\n${renderEnv}`), - ).not.toContain("## Needs a maintainer's decision"); - expect(handoffGuard).toContain('defer-to-human'); - // The agent-facing policy documents the handoff (a second guard). + park({ + rv: [ + { + user: { login: 'alice' }, + author_association: 'OWNER', + state: 'COMMENTED', + submitted_at: '2026-01-02T00:00:00Z', + body: 'take direction B', + }, + ], + }), + ).toEqual({ stale: 'false', parked: false }); + // …but a review OLDER than the conflict marker does not. + expect( + park({ + rv: [ + { + user: { login: 'alice' }, + author_association: 'OWNER', + state: 'CHANGES_REQUESTED', + submitted_at: '2025-12-31T00:00:00Z', + body: 'earlier feedback', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // The review bot's regeneration is exactly what must NOT wake: an + // update-branch merge re-reviews every new head. + expect( + park({ + rv: [ + { + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + state: 'CHANGES_REQUESTED', + submitted_at: '2026-01-02T00:00:00Z', + body: 'regenerated findings', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // Neither does an untrusted login nor an APPROVED review (only + // CHANGES_REQUESTED/COMMENTED carry actionable feedback). + expect( + park({ + rv: [ + { + user: { login: 'drive-by' }, + author_association: 'NONE', + state: 'COMMENTED', + submitted_at: '2026-01-02T00:00:00Z', + body: 'bump', + }, + { + user: { login: 'alice' }, + author_association: 'OWNER', + state: 'APPROVED', + submitted_at: '2026-01-02T00:00:00Z', + body: 'lgtm', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // A newer trusted-human ISSUE comment wakes… + expect( + park({ + ic: [ + { + user: { login: 'alice' }, + author_association: 'MEMBER', + created_at: '2026-01-02T00:00:00Z', + body: 'we discussed; going with option B', + }, + ], + }), + ).toEqual({ stale: 'false', parked: false }); + // …and so does a newer trusted-human reply in a review thread — a + // human answering inside the contested thread is exactly the response + // the handoff waits for (that leg carries no marker/command filter). + expect( + park({ + rc: [ + { + user: { login: 'alice' }, + author_association: 'MEMBER', + created_at: '2026-01-02T00:00:00Z', + body: 'direction B, see the design doc', + }, + ], + }), + ).toEqual({ stale: 'false', parked: false }); + // …but the loop's OWN control markers riding an issue comment are not + // human feedback (a re-arm posts its own marker), and neither are slash + // commands — the /retry lift happens through LIVE_REARM_KEY instead. + expect( + park({ + ic: [ + { + user: { login: 'alice' }, + author_association: 'MEMBER', + created_at: '2026-01-02T00:00:00Z', + body: '', + }, + { + user: { login: 'alice' }, + author_association: 'MEMBER', + created_at: '2026-01-02T00:00:00Z', + body: ' @qwen-code /retry', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // A NEW failing check wakes (the human answered through CI); a passing + // one does not. + expect( + park({ + checks: [ + { + name: 'build', + workflowName: 'CI', + conclusion: 'FAILURE', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'false', parked: false }); + expect( + park({ + checks: [ + { + name: 'build', + workflowName: 'CI', + conclusion: 'SUCCESS', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // …and the autofix workflow's OWN check runs are excluded wholesale, + // address lanes included: under a park no address round can legitimately + // run, so any review-address check newer than the marker is necessarily + // the conflict round's OWN failed check (posted after the handoff). + // Counting it would let the loop's own output unpark the very round it + // came from, and the wasted failure rounds feed CONSEC_FAIL toward a + // terminal lockout on the exact PR a human is settling. A manual re-run + // reaches prepare and parks green; /retry is the sanctioned lift. + expect( + park({ + checks: [ + { + name: 'develop-fix (1)', + workflowName: 'Qwen Autofix', + conclusion: 'FAILURE', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + expect( + park({ + checks: [ + { + name: 'review-address (1)', + workflowName: 'Qwen Autofix', + conclusion: 'TIMED_OUT', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // The loop's SIBLING machinery is excluded with it: the review + // workflow re-fires on every head the loop's own base-update merge + // creates (its failing checks complete AFTER both clocks), the + // CI-failure patrol re-runs flaky failures on the UNCHANGED head by + // cron, and the fork lanes carry the loop's own checks for fork PRs — + // all of it completes with no human anywhere in the input + // (probe-verified wake entrances). + for (const loopWorkflow of [ + '🧐 Qwen Pull Request Review', + 'Qwen CI Failure Patrol', + 'Qwen Autofix Fork Bridge', + 'Qwen Autofix Fork Signal', + ]) { + expect( + park({ + checks: [ + { + name: 'build', + workflowName: loopWorkflow, + conclusion: 'FAILURE', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + } + // The checks leg reads `.conclusion // .state` and `.completedAt // + // .updatedAt` — a check reported through the FALLBACK fields wakes too + // (dropping a fallback must not silently disable check-driven wakes). + expect( + park({ + checks: [ + { + name: 'build', + workflowName: 'CI', + state: 'FAILURE', + updatedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'false', parked: false }); + // A stale-base update is the loop's OWN head move: the red checks it + // REACTS to completed before its marker, so they are not human + // feedback — the checks leg counts only failures completing after + // BOTH the conflict marker and the latest base update. + expect( + park({ + baseUpdAt: '2026-01-01T12:00:00Z', + checks: [ + { + name: 'build', + workflowName: 'CI', + conclusion: 'FAILURE', + completedAt: '2026-01-01T06:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // A failure completing AFTER the latest base update still wakes — the + // base is current, so the red is new information. + expect( + park({ + baseUpdAt: '2026-01-01T12:00:00Z', + checks: [ + { + name: 'build', + workflowName: 'CI', + conclusion: 'FAILURE', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'false', parked: false }); + // CANCELLED never wakes: an update-branch push (the loop's own head + // move) cancels in-flight runs on the old head, and a close/reopen + // does the same — loop-generated events, not a human response. + expect( + park({ + checks: [ + { + name: 'build', + workflowName: 'CI', + conclusion: 'CANCELLED', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toEqual({ stale: 'true', parked: true }); + // A conflict marker under a DEAD window key (re-armed since) does not + // park — the new window has no pending handoff. + expect(park({ win: 'W0' })).toEqual({ stale: 'false', parked: false }); + // Already stale → the block is inert (no re-announcement, no recompute). + expect(park({ stale: 'true' })).toEqual({ stale: 'true', parked: false }); + + // Report marker emission: every audit round posts its verdict under the + // key the baseline was READ under; a sound verdict ADDITIONALLY re-arms — + // but only on the completed-round paths. Execute the real function. The + // verdict arrives via AUDIT_VERDICT — the verdict the verification GATE + // validated and surfaced as a step output — and the function must NOT + // re-read growth-audit.json: the branch's own build/tests run as the + // runner user and WORKDIR is a predictable path they can write, so a + // re-read could be overwritten after the gate looked (a forged re-arm, + // or a conflict verdict flipped back to sound, defeating the park). + const emitMarkerFn = pushAndReportStep.match( + /emit_growth_audit_marker\(\) \{[\s\S]*?\n {10}\}/, + )?.[0]; + expect(emitMarkerFn).toBeTruthy(); + expect(emitMarkerFn).not.toContain('growth-audit.json'); + expect(emitMarkerFn).toContain('AUDIT_VERDICT'); + // The failure/handoff report step has its OWN copy of the helper (each + // step is a fresh shell). A drift between the copies — marker format, + // re-arm suppression, the win= fallback — would ship green unless + // pinned: extract both and require them identical. + const emitMarkerFnFailure = reviewAddressReportStep.match( + /emit_growth_audit_marker\(\) \{[\s\S]*?\n {10}\}/, + )?.[0]; + expect(emitMarkerFnFailure).toBeTruthy(); + expect(emitMarkerFnFailure).toBe(emitMarkerFn); + // Both report steps consume the single verdict Finalize verification + // selects WITH the outcome: the pass whose outcome was selected wins — + // a repair pass legitimately re-audits (its feedback rebuild keeps the + // audit section and the SKILL mandates audit-first), and its + // gate-validated verdict is the one the round's code was judged by; a + // repair that validated nothing falls back to the first pass's + // validated verdict. Never a re-read of the branch-writable file. + const auditVerdictBind = + "AUDIT_VERDICT: '${{ steps.final_verify.outputs.audit_verdict }}'"; + expect(pushAndReportStep).toContain(auditVerdictBind); + expect(reviewAddressReportStep).toContain(auditVerdictBind); + // Push + no-op report paths allow the re-arm; the failure/handoff path + // records the verdict (the trail must stay complete) but must NOT re-arm + // — a FAILED round must not re-anchor the window. + expect( + pushAndReportStep.match(/emit_growth_audit_marker true/g) ?? [], + ).toHaveLength(2); + expect(reviewAddressReportStep).toContain('emit_growth_audit_marker false'); + const emitWith = ({ + allow, + kissAudit = 'true', + auditVerdict = 'sound', + growthBaseWin = 'W1', + window = 'none', + }) => + execFileSync( + 'bash', + [ + '-c', + `${emitMarkerFn}\nKISS_AUDIT=${kissAudit}\nAUDIT_VERDICT='${auditVerdict}'\n` + + `GROWTH_BASE_WIN=${growthBaseWin}\nWINDOW=${window}\n` + + `emit_growth_audit_marker ${allow}`, + ], + { encoding: 'utf8' }, + ).trim(); + // sound on a completed round → verdict marker AND re-arm. + expect(emitWith({ allow: 'true' })).toBe( + '\n', + ); + // sound on the FAILED report path → verdict marker, never the re-arm. + expect(emitWith({ allow: 'false' })).toBe( + '', + ); + // drift and conflict post the trail marker only — the simplification + // re-measures naturally; conflict is parked for the human. + expect(emitWith({ allow: 'true', auditVerdict: 'drift' })).toBe( + '', + ); + expect(emitWith({ allow: 'true', auditVerdict: 'conflict' })).toBe( + '', + ); + // Outside the verdict taxonomy or empty → NO marker (a garbage verdict + // must never reach the trail or trigger a re-arm; defense in depth — + // the gate only ever surfaces the three valid values). + expect(emitWith({ allow: 'true', auditVerdict: 'shrug' })).toBe(''); + expect(emitWith({ allow: 'true', auditVerdict: '' })).toBe(''); + expect( + emitWith({ + allow: 'true', + auditVerdict: 'sound win=W9 -->\n', + ); + + // The verification gate REQUIRES the verdict on audit rounds: presence + + // shape, NON-retryable (agent misbehavior, not a build problem — the + // repair pass must never be invoked). The behavioral half of this pin + // runs the real gate script end-to-end in the A/B describe below. + expect(reviewVerificationRunner).toContain( + 'if [[ "${KISS_AUDIT:-false}" == \'true\' ]]; then', + ); + expect(reviewVerificationRunner).toContain( + "reject_fix 'growth-audit round missing a valid growth-audit.json verdict (audit skipped or malformed)' 'false' 'false'", + ); + expect(reviewVerificationRunner).toContain( + 'IN("sound", "drift", "conflict")', + ); + expect(reviewVerificationRunner).toContain('.kiss.result'); + expect(reviewVerificationRunner).toContain('.minimal_change.result'); + // The validated verdict is surfaced as a step output for the report to + // consume — the TOCTOU guard: the report never re-reads the file. + expect(reviewVerificationRunner).toContain( + 'echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}"', + ); + // jq parses a *stream* of concatenated documents happily; slurp mode + // makes the document COUNT part of the validation, and the anchored + // regex stays as defense in depth. + expect(reviewVerificationRunner).toContain('if length != 1 then empty'); + expect(reviewVerificationRunner).toContain( + '[[ "${AUDIT_VERDICT}" =~ ^(sound|drift|conflict)$ ]] || AUDIT_VERDICT=\'\'', + ); + // Last-writer binding: the verdict record precedes the branch's checks, + // and the step-output file stays discoverable under $RUNNER_TEMP after + // the strip removes the variable — the gate re-records its validated + // verdict at every exit past the record (reject_fix and the outcome + // writes; never a trap — the no-trap pin below stands) so a forged + // append loses. A verdict rejected BEFORE its record never surfaces. + // BASH_ENV and BITE_RUNNER are execution-steering knobs with no + // legitimate setter. + expect(reviewVerificationRunner).toContain("AUDIT_VERDICT_RECORDED='true'"); + // Twelve re-record sites: reject_fix, the two failure.md exits, the two + // crash exits, the three handoff classification exits (dirty / committed + // / no-commit — inert on non-audit rounds because the record only happens + // for KISS_AUDIT), the noop exit, the two no-commit exits (which run AFTER + // the schema/contracts checks — probe-verified forge entrance), and the + // fixed outcome write. Dropping any one re-opens the forge on that + // exit path. + expect( + reviewVerificationRunner.match( + /if \[\[ "\$\{AUDIT_VERDICT_RECORDED:-false\}" == 'true' \]\]; then/g, + ) ?? [], + ).toHaveLength(12); + expect(reviewVerificationRunner).toContain('unset BASH_ENV BITE_RUNNER'); + // The verdict variables are GATE state: an inherited plant must not + // ride the every-exit re-append back into the outputs. + expect(reviewVerificationRunner).toContain( + 'unset AUDIT_VERDICT AUDIT_VERDICT_RECORDED', + ); + // kiss_audit rides the same last-writer discipline: one record before + // any branch code runs plus the re-append at all twelve exits (the + // three handoff classification exits included — inert on non-audit + // rounds, since they re-append the same `false` the record set). + expect( + reviewVerificationRunner.match( + /echo "kiss_audit=\$\{KISS_AUDIT:-false\}" >> "\$\{GITHUB_OUTPUT\}"/g, + ) ?? [], + ).toHaveLength(13); + // The runner backs GITHUB_ENV/GITHUB_PATH/GITHUB_STEP_SUMMARY with + // files under $RUNNER_TEMP/_runner_file_commands/ that stay + // discoverable after the variable strip — lock them so a check cannot + // plant environment into the later PAT-bearing steps. The + // GITHUB_OUTPUT backing file is the exception: the gate writes it, and + // forges against it lose to the every-exit re-append plus the finalize + // conclusion gate. The directory stays writable: the runner creates + // the next step's backing files there, and a lock would stall every + // later step (the rename-over residual is documented, not bought). + expect(reviewVerificationRunner).toContain( + 'if [[ -n "${GITHUB_OUTPUT:-}" && -d "${RUNNER_TEMP}/_runner_file_commands" ]]; then', + ); + expect(reviewVerificationRunner).toContain( + '[[ -f "${_rfc}" && "${_rfc}" != "${GITHUB_OUTPUT}" ]]', + ); + expect(reviewVerificationRunner).not.toContain( + 'chmod a-w "${RUNNER_TEMP}/_runner_file_commands"', + ); + // Conflict routing is gate-enforced: a conflict verdict that did not + // stop with a handoff must not clear the gate and push. + expect(reviewVerificationRunner).toContain( + "reject_fix 'growth-audit verdict is conflict but the round did not stop with a handoff; conflict must STOP BLOCKED (no push)' 'false' 'false'", + ); + // The checks run the branch's own code with the runner injection + // channels stripped — a check appending to GITHUB_OUTPUT would + // overwrite the gate's outputs last-write-wins (a forged + // audit_verdict=sound after the gate's write). + expect(reviewVerificationRunner).toContain( + 'env -u GITHUB_OUTPUT -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY "$@"', + ); + expect(reviewVerificationRunner).toContain( + 'strip_runner_channels npm run test', + ); + // The check sits BEFORE the no-commit/no-op exits: a no-op audit round + // whose verdict is sound with nothing left to fix still needs the artifact. + const verdictGateAt = reviewVerificationRunner.indexOf( + '# Growth-audit verdict gate:', + ); + expect(verdictGateAt).toBeGreaterThan(-1); + expect(verdictGateAt).toBeLessThan( + reviewVerificationRunner.indexOf( + 'if git diff --quiet "origin/${BRANCH}...${BRANCH}"; then', + ), + ); + // And BEFORE the failure.md early-exits: a BLOCKED conflict round + // exits via failure.md, and its verdict must be validated and surfaced + // before that exit writes outcome=failed — otherwise the trail marker + // never posts and the idempotent park never engages. + expect(verdictGateAt).toBeLessThan( + reviewVerificationRunner.indexOf('if [[ -f "${WORKDIR}/failure.md"'), + ); + + // The agent-facing policy documents the audit, and the old + // non-convergence handoff text is GONE (a second guard on both sides). const skill = readAutofixSkill(); - expect(skill).toContain('this PR is not converging'); + expect(skill).toContain('Growth audit required'); + expect(skill).toContain('growth-audit.json'); expect(skill).toContain('Diff-growth trajectory'); + expect(skill).not.toContain('this PR is not converging'); + expect(skill).not.toContain( + "Needs a maintainer's decision — this PR is not converging", + ); // The brake's handoff must land in handoff.md — the first-class stop // run-agent.mjs accepts (exit 0 with no spec output) and the verify gate // classifies as outcome=handoff. Routing it through failure.md instead - // would misreport a deliberate defer-to-human as a failed round — the - // run 32076785809 defect the gate-level acceptance supersedes. + // would misreport a deliberate defer-to-human as a failed round. expect(skill).toContain('Write that handoff to `/handoff.md`'); expect(skill).toContain( '`address-summary.md`, no `no-action.md`, no `failure.md`', ); + rmSync(dir, { recursive: true, force: true }); }); it('anchors a per-window growth baseline and splits src/test nets against a real repo', () => { @@ -8811,7 +9358,12 @@ exit 1 ); // Four sites: the NEWEST computation, the live-watermark revalidation, // the "Failed checks" rendering, and the "Still-red checks" rendering - // — all must share the same address-check carve-out. + // share the address-check carve-out (the autofix workflow's OTHER lanes + // failing is the loop's own business, not actionable feedback). The + // conflict-handoff wake filter deliberately does NOT share it: under a + // park no address round can legitimately run, so it excludes ALL Qwen + // Autofix checks — the conflict round's own failed check must not + // unpark its own park. expect( prepareBranchAndFeedbackStep.match(/startswith\("review-address"\)/g) ?? [], @@ -10625,7 +11177,7 @@ exit 1 ).toBeLessThan(reviewVerifyGate.indexOf('outcome=noop')); const reviewVerificationGateStep = verificationGateSteps[1]; expect(reviewVerificationGateStep).toContain( - 'bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"', + 'bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh"', ); expect(reviewVerificationGateStep).not.toContain('npm run build'); expect(reviewVerificationGateStep).not.toContain( @@ -12319,7 +12871,10 @@ exit 1 const launchIdx = argStart; expect(execIdx).toBeGreaterThan(launchIdx); } - expect(workflow.split('/usr/bin/env -i \\').length - 1).toBe(2); + // Four clean children: the two deferred-findings upserts plus the two + // verification-gate launches (the gate runs after the agent step's + // branch code, so its bash must inherit nothing at all). + expect(workflow.split('/usr/bin/env -i \\').length - 1).toBe(4); // R5-6: the failure-path child is near-verbatim of run_deferred_upsert's // child — tie their shared security scaffold together so drift in one is // caught. Compare the allow-list + prelude (everything up to where the @@ -13328,8 +13883,10 @@ exit 1 }); it('bite check: rejects a round whose changed tests pass on the pre-round tree', () => { + // Ends at the FINAL assert: the conflict push-boundary refusal sits + // between it and the verified_head write and is not bite machinery. const block = reviewVerificationRunner.match( - /(# Bite check:[\s\S]*?)\nassert_verification_tree\necho "verified_head/, + /(# Bite check:[\s\S]*?)\nassert_verification_tree\n/, )?.[1]; expect(block).toBeTruthy(); const run = ( @@ -13729,7 +14286,9 @@ exit 1 "reject_fix 'bite check: changed tests pass on the pre-round tree (claimed defect does not reproduce)' 'false' 'false'", ); expect(pushAndReportStep).toContain('gate-advisories.md'); - expect(reviewAddressJob).toContain('gate-advisories.md agent-api-error'); + expect(reviewAddressJob).toContain( + 'gate-advisories.md growth-audit.json agent-api-error', + ); const skill = readFileSync('.qwen/skills/autofix/SKILL.md', 'utf8'); expect(skill).toContain('Verification is SOURCE-BLIND'); expect(skill).toContain('changed tests against the pre-round branch'); @@ -13744,7 +14303,7 @@ exit 1 "if: |-\n ${{ always() && steps.prepare.outputs.stale != 'true' }}", ); expect(reviewVerificationGateStep).toContain( - 'bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"', + 'bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh"', ); expect(reviewVerificationRunner).toContain('failure.md'); expect(reviewVerificationRunner).toContain('outcome=failed'); @@ -13827,7 +14386,7 @@ exit 1 "steps.repair.outputs.attempted == 'true'", ); expect(repairVerificationGateStep).toContain( - 'bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"', + 'bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh"', ); expect( reviewVerificationRunner.match(/retryable=true/g) ?? [], @@ -13927,6 +14486,12 @@ exit 1 REPAIR_OUTCOME: '', REPAIR_COMMITTED: '', REPAIR_VERIFIED_HEAD: '', + FIRST_AUDIT_VERDICT: '', + REPAIR_AUDIT_VERDICT: '', + FIRST_CONCLUSION: '', + REPAIR_CONCLUSION: '', + FIRST_KISS_AUDIT: '', + REPAIR_KISS_AUDIT: '', ...env, }, }); @@ -13936,12 +14501,18 @@ exit 1 }; expect( - run({ FIRST_OUTCOME: 'fixed', FIRST_VERIFIED_HEAD: 'first-sha' }), + run({ + FIRST_OUTCOME: 'fixed', + FIRST_VERIFIED_HEAD: 'first-sha', + FIRST_CONCLUSION: 'success', + }), ).toMatchObject({ status: 0, written: expect.stringContaining('verified_head=first-sha'), }); - expect(run({ FIRST_OUTCOME: 'noop' })).toMatchObject({ + expect( + run({ FIRST_OUTCOME: 'noop', FIRST_CONCLUSION: 'success' }), + ).toMatchObject({ status: 0, written: expect.stringContaining('outcome=noop'), }); @@ -13976,6 +14547,8 @@ exit 1 REPAIR_OUTCOME: 'fixed', REPAIR_COMMITTED: 'true', REPAIR_VERIFIED_HEAD: 'repair-sha', + FIRST_CONCLUSION: 'failure', + REPAIR_CONCLUSION: 'success', }), ).toMatchObject({ status: 0, @@ -13987,6 +14560,7 @@ exit 1 FIRST_VERIFIED_HEAD: 'stale-first-sha', REPAIR_ATTEMPTED: 'true', REPAIR_OUTCOME: 'fixed', + REPAIR_CONCLUSION: 'success', }); expect(repairedWithoutVerifiedHead).toMatchObject({ status: 0, @@ -14011,6 +14585,125 @@ exit 1 REPAIR_OUTCOME: '', }), ).toMatchObject({ status: 1 }); + // The audit verdict travels WITH the attempt whose outcome is + // selected: a repair pass legitimately re-audits (its feedback rebuild + // keeps the audit section; the SKILL mandates audit-first), and its + // gate-validated verdict is the one the round's code was judged by — + // binding the first pass unconditionally dropped a repair-derived + // conflict and posted the handoff under a sound trail marker. + expect(workflow).toContain( + "FIRST_AUDIT_VERDICT: '${{ steps.verify.outputs.audit_verdict }}'", + ); + expect(workflow).toContain( + "REPAIR_AUDIT_VERDICT: '${{ steps.verify_repair.outputs.audit_verdict }}'", + ); + const repairConflict = run({ + FIRST_OUTCOME: 'failed', + REPAIR_ATTEMPTED: 'true', + REPAIR_OUTCOME: 'failed', + FIRST_AUDIT_VERDICT: 'sound', + REPAIR_AUDIT_VERDICT: 'conflict', + }); + expect(repairConflict.status).toBe(1); + expect(repairConflict.written).toContain('audit_verdict=conflict'); + expect(repairConflict.written).not.toContain('audit_verdict=sound'); + // Repair validated nothing (crash before its verdict gate): the first + // pass's validated verdict stays the record — the same :- shape + // COMMITTED uses. + expect( + run({ + FIRST_OUTCOME: 'failed', + REPAIR_ATTEMPTED: 'true', + REPAIR_OUTCOME: 'failed', + FIRST_AUDIT_VERDICT: 'drift', + }), + ).toMatchObject({ + status: 1, + written: expect.stringContaining('audit_verdict=drift'), + }); + // No repair: the first pass's verdict surfaces. + expect( + run({ + FIRST_OUTCOME: 'fixed', + FIRST_AUDIT_VERDICT: 'sound', + FIRST_CONCLUSION: 'success', + }), + ).toMatchObject({ + status: 0, + written: expect.stringContaining('audit_verdict=sound'), + }); + // Conclusion gate: fixed/noop are the ONLY outcomes that release the + // PAT push, and a gate that reached them exited 0 — step conclusion + // success. A silent gate death (killed mid-check) concludes failure, + // yet its step-output file stays discoverable under $RUNNER_TEMP and + // appendable; the forged claim must be discarded, never pushed + // (probe-verified entrance on the pre-gate finalize body). + expect(workflow).toContain( + "FIRST_CONCLUSION: '${{ steps.verify.conclusion }}'", + ); + expect(workflow).toContain( + "REPAIR_CONCLUSION: '${{ steps.verify_repair.conclusion }}'", + ); + const forgedFixed = run({ + FIRST_OUTCOME: 'fixed', + FIRST_COMMITTED: 'true', + FIRST_VERIFIED_HEAD: 'forged-sha', + FIRST_AUDIT_VERDICT: 'sound', + FIRST_CONCLUSION: 'failure', + }); + expect(forgedFixed.status).toBe(1); + expect(forgedFixed.written).not.toContain('outcome=fixed'); + expect(forgedFixed.written).not.toContain('verified_head='); + expect(forgedFixed.written).not.toContain('audit_verdict='); + // noop releases the push-bound report + thread resolution too. + expect( + run({ FIRST_OUTCOME: 'noop', FIRST_CONCLUSION: 'failure' }), + ).toMatchObject({ status: 1 }); + // A killed repair pass is gated identically. + expect( + run({ + FIRST_OUTCOME: 'failed', + REPAIR_ATTEMPTED: 'true', + REPAIR_OUTCOME: 'fixed', + REPAIR_CONCLUSION: 'failure', + }), + ).toMatchObject({ status: 1 }); + // A legitimate rejection (outcome=failed on a failing conclusion) + // still surfaces — the gate binds only the push-releasing outcomes. + expect( + run({ FIRST_OUTCOME: 'failed', FIRST_CONCLUSION: 'failure' }), + ).toMatchObject({ + status: 1, + written: expect.stringContaining('outcome=failed'), + }); + // kiss_audit rides the same selection chain — the gates' defended + // output, forwarded with the selected pass. + expect(workflow).toContain( + "FIRST_KISS_AUDIT: '${{ steps.verify.outputs.kiss_audit || steps.prepare.outputs.kiss_audit }}'", + ); + expect( + run({ + FIRST_OUTCOME: 'fixed', + FIRST_CONCLUSION: 'success', + FIRST_KISS_AUDIT: 'true', + }), + ).toMatchObject({ + status: 0, + written: expect.stringContaining('kiss_audit=true'), + }); + expect( + run({ + FIRST_OUTCOME: 'failed', + REPAIR_ATTEMPTED: 'true', + REPAIR_OUTCOME: 'fixed', + REPAIR_CONCLUSION: 'success', + FIRST_KISS_AUDIT: 'false', + REPAIR_KISS_AUDIT: 'true', + }), + ).toMatchObject({ + status: 0, + written: expect.stringContaining('kiss_audit=true'), + }); }); it("keeps a verdict round's own check out of the next scan's failed-check count", () => { @@ -14532,7 +15225,7 @@ exit 1 'for f in decision.json pr-title.txt pr-body.md e2e-report.md failure.md failure.zh.md fix.diff; do', ); expect(reviewAddressJob).toContain( - 'for f in feedback.md address-summary.md no-action.md failure.md failure.zh.md handoff.md gate-rejection.md gate-advisories.md agent-api-error agent-api-error-kind agent-timeout resolved-comments.txt comment-replies.json deferred-findings.json deferred-findings.carry.json deferred-findings.unmerged.json pr.diff; do', + 'for f in feedback.md address-summary.md no-action.md failure.md failure.zh.md handoff.md gate-rejection.md gate-advisories.md growth-audit.json agent-api-error agent-api-error-kind agent-timeout resolved-comments.txt comment-replies.json deferred-findings.json deferred-findings.carry.json deferred-findings.unmerged.json pr.diff; do', ); expect(reviewAddressReportStep).toContain( 'for f in address-summary.md no-action.md failure.md failure.zh.md handoff.md; do', @@ -15994,11 +16687,23 @@ exit 1 /- name: 'Prepare branch and feedback'[\s\S]*?(?=\n {6}- name: )/, )?.[0] ?? ''; - // 1. A failing check records WHY, not just THAT, it failed. - const capture = gate.match( - /GATE_LOG="\$\{WORKDIR\}\/gate-output\.log"[\s\S]*?\n\}\nrun_check\(\) \{[\s\S]*?\n\}/, - )?.[0]; - expect(capture).toBeTruthy(); + // 1. A failing check records WHY, not just THAT, it failed. Two + // extractions: the capture machinery (GATE_LOG init + reject_fix) and + // run_check with its channel-strip helper. The span BETWEEN them now + // holds the growth-audit verdict gate, the failure.md early-exits, and + // bare git lines (hooks sever + branch checkout) that cannot run in + // this standalone fixture — the verdict gate and the exits are inert + // here (no KISS_AUDIT tag, no failure.md), but the git lines are not, + // so they stay out of the extraction. + const capture = + (gate.match( + /GATE_LOG="\$\{WORKDIR\}\/gate-output\.log"[\s\S]*?\n\}\n/, + )?.[0] ?? '') + + (gate.match( + /strip_runner_channels\(\) \{[\s\S]*?\n\}\nrun_check\(\) \{[\s\S]*?\n\}/, + )?.[0] ?? ''); + expect(capture).toContain('reject_fix()'); + expect(capture).toContain('run_check()'); const dir = mkdtempSync(join(tmpdir(), 'gate-')); const out = join(dir, 'gh_output'); writeFileSync(out, ''); @@ -18436,6 +19141,181 @@ exit 0 }); }); +describe('growth-audit hardening: park wake set and verdict pipeline (round 3)', () => { + it('skips the scan stale-base update while a conflict handoff pends', () => { + // The loop's OWN head move must not fire wake checks while parked: an + // update-branch merge re-fires every synchronize-triggered workflow on + // the new head, and those loop-generated checks complete after both + // park clocks — lifting the park with zero human activity and feeding + // CONSEC_FAIL toward a terminal lockout on the exact PR a human is + // settling (probe-verified entrance). The scan block mirrors prepare's + // conflict-handoff idempotence wake set; execute the real block. + expect(reviewScanJob).toContain('&& "${CONFLICT_PARKED}" != \'true\' ]]'); + const scanParkGateBlock = reviewScanJob.match( + /CONFLICT_PARKED='false'[\s\S]*?rm -f "\$\{WORKDIR\}\/rv\.scan\.json" "\$\{WORKDIR\}\/rc\.scan\.json" "\$\{WORKDIR\}\/checks\.scan\.json"\n {12}fi\n/, + )?.[0]; + expect(scanParkGateBlock).toBeTruthy(); + const T0 = '2026-01-01T00:00:00Z'; + const runScanPark = ({ + markerWin = 'W1', + ic = [], + rv = [], + rc = [], + checks = [], + }) => { + const dir = mkdtempSync(join(tmpdir(), 'scan-park-')); + const bin = join(dir, 'bin'); + mkdirSync(bin); + try { + const marker = { + user: { login: 'qwen-code-dev-bot' }, + created_at: T0, + body: ``, + }; + writeFileSync(join(dir, 'ic.json'), JSON.stringify([marker, ...ic])); + writeFileSync(join(dir, 'rv.fixture.json'), JSON.stringify(rv)); + writeFileSync(join(dir, 'rc.fixture.json'), JSON.stringify(rc)); + writeFileSync( + join(bin, 'gh'), + [ + '#!/bin/bash', + 'case " $* " in', + ' *reviews*) cat "${RV_FIXTURE}" ;;', + ' *comments*) cat "${RC_FIXTURE}" ;;', + ' *) echo "[]" ;;', + 'esac', + ].join('\n'), + ); + chmodSync(join(bin, 'gh'), 0o755); + const out = execFileSync( + 'bash', + [ + '-c', + `set -e\n${scanParkGateBlock}\nprintf '%s' "$CONFLICT_PARKED"`, + ], + { + encoding: 'utf8', + env: { + ...process.env, + PATH: `${bin}:${process.env.PATH}`, + AUTOFIX_BOT: 'qwen-code-dev-bot', + REVIEW_BOT: 'qwen-code-ci-bot', + REARM_KEY: 'W1', + WORKDIR: dir, + REPO: 'o/r', + PR: '1', + CHECKS_JSON: JSON.stringify(checks), + TRUSTED_ASSOC: '["OWNER", "MEMBER", "COLLABORATOR"]', + RV_FIXTURE: join(dir, 'rv.fixture.json'), + RC_FIXTURE: join(dir, 'rc.fixture.json'), + }, + }, + ); + return out.trim().split('\n').pop(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + // No human response since the marker → parked, no head move. + expect(runScanPark({})).toBe('true'); + // A trusted-human comment or review after the marker lifts the gate — + // the base update may resume once a human is engaged. + expect( + runScanPark({ + ic: [ + { + user: { login: 'alice' }, + author_association: 'MEMBER', + created_at: '2026-01-02T00:00:00Z', + body: 'decision: option B', + }, + ], + }), + ).toBe('false'); + expect( + runScanPark({ + rv: [ + { + user: { login: 'alice' }, + author_association: 'OWNER', + state: 'COMMENTED', + submitted_at: '2026-01-02T00:00:00Z', + body: 'take direction B', + }, + ], + }), + ).toBe('false'); + // A marker under a DEAD window key (re-armed since) parks nothing. + expect(runScanPark({ markerWin: 'W0' })).toBe('false'); + // Loop-generated checks do not lift the gate either: the patrol's + // same-head re-run failure is excluded by the shared workflow filter. + expect( + runScanPark({ + checks: [ + { + name: 'build', + workflowName: 'Qwen CI Failure Patrol', + conclusion: 'FAILURE', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toBe('true'); + // …but a genuinely external failing check still lifts it. + expect( + runScanPark({ + checks: [ + { + name: 'build', + workflowName: 'Qwen Code CI', + conclusion: 'FAILURE', + completedAt: '2026-01-02T00:00:00Z', + }, + ], + }), + ).toBe('false'); + }); + + it('a conflict round parks quietly — no stale-base merge in its report', () => { + // The conflict round's own stale-base retry would re-fire every + // synchronize-triggered workflow on the new head; those loop-generated + // checks complete after the conflict marker the same report posts — + // waking the very park it establishes. The retry is gated on the + // verdict, BEFORE any compare/update-branch call. + const guard = reviewAddressReportStep.indexOf( + 'if [[ "${AUDIT_VERDICT:-}" != \'conflict\' ]]; then', + ); + expect(guard).toBeGreaterThan(-1); + expect(guard).toBeLessThan( + reviewAddressReportStep.indexOf( + 'gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch"', + ), + ); + }); + + it('launches both gates through pinned, allowlisted clean children', () => { + // BASH_ENV is sourced at bash STARTUP, before the body's line 1 — the + // steps pin it (and the SHELLOPTS option-import channel) empty at step + // level, which outranks any $GITHUB_ENV plant; the gate itself then + // runs through the workflow's env -i clean-child pattern, so its bash + // inherits nothing at all (enumerating plants is the failure mode the + // verdict pipeline kept hitting). + for (const step of [verificationGateSteps[1], repairVerificationGateStep]) { + expect(step).toContain("BASH_ENV: ''"); + expect(step).toContain("SHELLOPTS: ''"); + expect(step).toContain('/usr/bin/env -i'); + expect(step).toContain( + 'bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh"', + ); + // The gate re-declares the variables it needs inside the child. + expect(step).toContain('KISS_AUDIT="${KISS_AUDIT:-false}"'); + expect(step).toContain( + 'FOOTPRINT_ENFORCE="${FOOTPRINT_ENFORCE:-advisory}"', + ); + } + }); +}); + describe('review verification gate: baseline A/B on deterministic rejection', () => { // The A/B re-runs a failed check at the pre-round ref and reports // pre-existing ONLY when the baseline fails with a MATCHING failure @@ -18474,7 +19354,33 @@ describe('review verification gate: baseline A/B on deterministic rejection', () trackedDirt = false, dirtyTree = '', commFail = false, - workdirFiles = { 'address-summary.md': 'summary\n' }, + // Growth-audit rounds: tag the gate with KISS_AUDIT and optionally seed + // the audit's verdict file in the workdir. + kissAudit = false, + auditJson = null, + // The agent's stop-artifact shapes around the no-commit exits: a + // present no-action.md takes the unchanged-branch arm to noop; + // dropping address-summary.md reaches the missing-summary exit. + noAction = false, + summaryPresent = true, + // Forgery probe: the schema check attempts to plant an env line into + // the runner file-command backing files the gate must lock. + forgeEnvFile = false, + // Stop markers the agent leaves in the workdir: a BLOCKED conflict + // round exits via failure.md; handoff.md is the other stop shape. + failureMd = null, + handoffMd = null, + // Forgery probe: the stubbed build attempts to append a forged + // audit_verdict to the step output channel. + forgeOutput = false, + // Forgery probe: the schema check discovers the step-output file via + // the inherited $RUNNER_TEMP and appends a forged audit_verdict AFTER + // the gate's write (the strip removed the variable, not the file). + discoverOutput = false, + // Arbitrary extra workdir files — the handoff-classification fixtures + // drive stop-marker combinations through this. Defaults empty: the + // summary default is owned by summaryPresent above, not duplicated here. + workdirFiles = {}, }) => { const dir = mkdtempSync(join(tmpdir(), 'gate-ab-')); try { @@ -18541,6 +19447,20 @@ describe('review verification gate: baseline A/B on deterministic rejection', () join(bin, 'npm'), [ '#!/bin/bash', + 'if [[ "${FORGE_OUTPUT:-}" == "1" && "$1" == "run" && "$2" == "build" ]]; then', + ' if [[ -n "${GITHUB_OUTPUT:-}" ]]; then', + ' echo "audit_verdict=sound" >> "${GITHUB_OUTPUT}"', + ' echo "forge landed: GITHUB_OUTPUT inherited"', + ' else', + ' echo "forge blocked: GITHUB_OUTPUT not inherited"', + ' fi', + ' if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then', + ' echo "forged summary" >> "${GITHUB_STEP_SUMMARY}"', + ' echo "summary forge landed: GITHUB_STEP_SUMMARY inherited"', + ' else', + ' echo "summary forge blocked: GITHUB_STEP_SUMMARY not inherited"', + ' fi', + 'fi', 'if [[ "$1" == "run" && "$2" == "build" ]]; then', ' head="$(git rev-parse HEAD)"', ' for s in ${FAIL_BUILD_SHAS}; do', @@ -18614,7 +19534,24 @@ describe('review verification gate: baseline A/B on deterministic rejection', () mkdirSync(rt); writeFileSync( join(rt, 'check-settings-schema.sh'), - 'if [[ "${SCHEMA_FAIL:-}" == "1" ]]; then echo "schema stale"; exit 1; fi\nexit 0\n', + 'if [[ "${DISCOVER_ENV:-}" == "1" ]]; then\n' + + ' envfile="$(find "${RUNNER_TEMP:-}/_runner_file_commands" -name "set_env_*" 2>/dev/null | head -1)"\n' + + ' if [[ -n "${envfile}" ]] && echo "BASH_ENV=/evil" >> "${envfile}" 2>/dev/null; then\n' + + ' echo "env forge landed: backing file writable"\n' + + ' else\n' + + ' echo "env forge blocked: backing file locked"\n' + + ' fi\n' + + 'fi\n' + + 'if [[ "${DISCOVER_OUTPUT:-}" == "1" ]]; then\n' + + ' target="$(find "${RUNNER_TEMP:-}" -name "set_output_*" 2>/dev/null | head -1)"\n' + + ' if [[ -n "${target}" ]]; then\n' + + ' echo "audit_verdict=sound" >> "${target}"\n' + + ' echo "forge landed: output file discovered via RUNNER_TEMP"\n' + + ' else\n' + + ' echo "forge blocked: no output file discoverable"\n' + + ' fi\n' + + 'fi\n' + + 'if [[ "${SCHEMA_FAIL:-}" == "1" ]]; then echo "schema stale"; exit 1; fi\nexit 0\n', ); writeFileSync( join(rt, 'check-autofix-contracts.sh'), @@ -18626,11 +19563,36 @@ describe('review verification gate: baseline A/B on deterministic rejection', () ); const workdir = join(dir, 'wd'); mkdirSync(workdir); + if (summaryPresent) { + writeFileSync(join(workdir, 'address-summary.md'), 'summary\n'); + } + if (noAction) { + writeFileSync(join(workdir, 'no-action.md'), 'no action\n'); + } + if (auditJson !== null) { + writeFileSync(join(workdir, 'growth-audit.json'), auditJson); + } + if (failureMd !== null) { + writeFileSync(join(workdir, 'failure.md'), failureMd); + } + if (handoffMd !== null) { + writeFileSync(join(workdir, 'handoff.md'), handoffMd); + } for (const [name, content] of Object.entries(workdirFiles)) { writeFileSync(join(workdir, name), content); } - const outFile = join(dir, 'gh-output'); + // Under RUNNER_TEMP, mirroring the real runner layout: the strip + // removes the GITHUB_OUTPUT variable from the checks, but the + // backing file stays discoverable there — the entry the last-writer + // binding closes. + const outFile = join(rt, 'set_output_gate'); writeFileSync(outFile, ''); + if (forgeEnvFile) { + mkdirSync(join(rt, '_runner_file_commands'), { recursive: true }); + writeFileSync(join(rt, '_runner_file_commands', 'set_env_probe'), ''); + } + const summaryFile = join(dir, 'step-summary'); + writeFileSync(summaryFile, ''); const res = spawnSync( 'bash', @@ -18646,6 +19608,7 @@ describe('review verification gate: baseline A/B on deterministic rejection', () WORKDIR: workdir, RUNNER_TEMP: rt, GITHUB_OUTPUT: outFile, + GITHUB_STEP_SUMMARY: summaryFile, FAIL_BUILD_SHAS: failShas, BASELINE_SHA: baselineSha, BASELINE_CODE: baselineCode, @@ -18663,13 +19626,23 @@ describe('review verification gate: baseline A/B on deterministic rejection', () HUGE_FAIL: hugeFail ? '1' : '', WORKSPACE_TEST_FAIL: addWorkspace ? '1' : '', RESOLVED_PKGS: addWorkspace ? 'packages/newpkg' : '', + KISS_AUDIT: kissAudit ? 'true' : 'false', + FORGE_OUTPUT: forgeOutput ? '1' : '', + DISCOVER_OUTPUT: discoverOutput ? '1' : '', + DISCOVER_ENV: forgeEnvFile ? '1' : '', }, }, ); + // The gate locks the file-command directory for the step's + // lifetime; restore it so the fixture teardown can delete it. + if (forgeEnvFile) { + chmodSync(join(rt, '_runner_file_commands'), 0o755); + } return { status: res.status, stdout: `${res.stdout}\n${res.stderr}`, outputs: readFileSync(outFile, 'utf8'), + summary: readFileSync(summaryFile, 'utf8'), rejection: existsSync(join(workdir, 'gate-rejection.md')) ? readFileSync(join(workdir, 'gate-rejection.md'), 'utf8') : '', @@ -19096,6 +20069,435 @@ describe('review verification gate: baseline A/B on deterministic rejection', () expect(r.outputs).not.toContain('preexisting=true'); expect(r.stdout).not.toContain('Baseline A/B'); }); + + // Growth-audit rounds (the counting window is over its growth budget) must + // carry the audit's machine-readable verdict — the audit IS the round's + // judgment of the over-budget approach, and a round that skipped it must + // not push (the rubber-stamp hole by absence). Rejection is NON-retryable: + // a malformed verdict is agent misbehavior, not a build problem, so the + // repair pass must never be invoked. + const validAuditJson = JSON.stringify({ + verdict: 'sound', + kiss: { result: 'pass', simpler_alternative: null }, + minimal_change: { result: 'pass', untraceable_hunks: [] }, + rationale: 'every hunk traces to a finding', + }); + const driftAuditJson = JSON.stringify({ + verdict: 'drift', + kiss: { result: 'fail', simpler_alternative: 'drop the guard stack' }, + minimal_change: { result: 'pass', untraceable_hunks: [] }, + rationale: 'the kiss axis fails', + }); + const conflictAuditJson = JSON.stringify({ + verdict: 'conflict', + kiss: { result: 'pass', simpler_alternative: null }, + minimal_change: { result: 'pass', untraceable_hunks: [] }, + rationale: 'two defensible directions', + }); + + it('rejects a growth-audit round that skipped the audit, non-retryably', () => { + const r = runGate({ kissAudit: true }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('outcome=failed'); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.outputs).not.toContain('preexisting=true'); + // No verdict reached the gate, so none may reach the report either. + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.rejection).toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + // Rejected at the head of the check section, before any check ran. + expect(r.stdout).not.toContain('Baseline A/B'); + }); + + it('rejects a verdict-less audit round even on the no-commit path', () => { + // Behavioral proof of the ordering the indexOf pin asserts: the verdict + // gate sits BEFORE the no-commit/no-op exits, so an audit round whose + // agent produced no commit still needs the artifact — a new early exit + // added above the verdict gate would let this round escape it. + const r = runGate({ kissAudit: true, agentCommit: false }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('outcome=failed'); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.rejection).toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + }); + + it('rejects a malformed growth-audit verdict the same way (non-retryable)', () => { + // A verdict value outside the taxonomy… + let r = runGate({ + kissAudit: true, + auditJson: JSON.stringify({ + verdict: 'shrug', + kiss: { result: 'pass' }, + minimal_change: { result: 'pass' }, + }), + }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('outcome=failed'); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.rejection).toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + // …and a canonical verdict missing one axis result are both inert. + r = runGate({ + kissAudit: true, + auditJson: JSON.stringify({ + verdict: 'drift', + minimal_change: { result: 'pass' }, + }), + }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.outputs).not.toContain('audit_verdict='); + }); + + it('passes a growth-audit round carrying a valid verdict and proceeds to the checks', () => { + // A valid verdict lets the round PROCEED: the deterministic checks run + // and their own rejection (a stubbed red build) is what ends the round — + // retryable, unlike the verdict rejection. The all-green audit + // composition (valid verdict + green checks → outcome=fixed) is NOT + // executed anywhere: the green path runs into the bite section, whose + // mapfile needs bash >= 4 (the macOS system bash is 3.2). That gap is + // acceptable: the gate script references KISS_AUDIT/AUDIT_VERDICT only + // inside the verdict-gate block, so past it an audit round is + // structurally identical to a non-audit round ('keeps the green path + // intact' covers that shape with kissAudit unset). + // failAt terminates the run before the bite section for the same + // bash-version reason. + const r = runGate({ + kissAudit: true, + auditJson: validAuditJson, + failAt: ['feature'], + }); + expect(r.status).toBe(1); + expect(r.stdout).toContain('growth-audit verdict: sound'); + expect(r.stdout).not.toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + // The gate-validated verdict is surfaced for the report — the TOCTOU + // guard: the report consumes THIS, never a re-read of the file. + expect(r.outputs).toContain('audit_verdict=sound'); + // The round was charged for the BUILD, on the retryable path — proof it + // cleared the verdict gate and reached the deterministic checks. + expect(r.outputs).toContain('outcome=failed'); + expect(r.outputs).toContain('retryable=true'); + expect(r.rejection).toContain('stub build FAILED'); + }); + + it('surfaces the conflict verdict before the failure.md exit ends the round', () => { + // The composition the conflict park lives on: a BLOCKED conflict round + // stops via failure.md, and the verdict gate sits ABOVE that exit — so + // audit_verdict reaches GITHUB_OUTPUT before outcome=failed. A gate + // ordered after the exit surfaces nothing, the trail marker never posts, + // and the idempotent park never engages. + const r = runGate({ + kissAudit: true, + auditJson: conflictAuditJson, + failureMd: 'conflict handoff\n', + // No commit: a conflict round stops BLOCKED before editing; the + // committed shape is its own rejection test below. + agentCommit: false, + }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('audit_verdict=conflict'); + expect(r.stdout).toContain('growth-audit verdict: conflict'); + expect(r.outputs).toContain('outcome=failed'); + expect(r.outputs).not.toContain('retryable=true'); + }); + + it('rejects a verdict-less audit round even when failure.md is present', () => { + // Ordering proof for the failure.md side: the verdict gate runs BEFORE + // the failure.md early-exits, so an audit round that stopped without + // the artifact takes the verdict rejection (non-retryable), not the + // plain abort exit. + const r = runGate({ kissAudit: true, failureMd: 'blocked\n' }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.rejection).toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + }); + + it('passes a drift verdict end-to-end and proceeds to the checks', () => { + // Sound is not the only verdict that must clear the gate: drift has to + // surface as drift — a mutation reporting every verdict as sound would + // skip the simplify-first routing (the conflict end-to-end shape is + // the failure.md case above). + const r = runGate({ + kissAudit: true, + auditJson: driftAuditJson, + failAt: ['feature'], + }); + expect(r.status).toBe(1); + expect(r.stdout).toContain('growth-audit verdict: drift'); + expect(r.outputs).toContain('audit_verdict=drift'); + expect(r.outputs).toContain('retryable=true'); + }); + + it('strips the runner output channel from the branch checks', () => { + // The stubbed build tries to append a forged audit_verdict=sound + // (step outputs are last-write-wins): the gate strips GITHUB_OUTPUT + // from the check subprocesses, so the forge branch runs but lands + // nothing. + const r = runGate({ + kissAudit: true, + auditJson: driftAuditJson, + forgeOutput: true, + failAt: ['feature'], + }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('audit_verdict=drift'); + expect(r.outputs).not.toContain('audit_verdict=sound'); + // Two drift lines: the gate's write plus its every-exit re-record (the + // last-writer binding) — the forge reached neither channel. + expect(r.outputs.match(/audit_verdict=drift/g)).toHaveLength(2); + // Proof the forge branches actually executed inside the check. + expect(r.stdout).toContain('forge blocked: GITHUB_OUTPUT not inherited'); + expect(r.stdout).toContain( + 'summary forge blocked: GITHUB_STEP_SUMMARY not inherited', + ); + expect(r.summary).toBe(''); + }); + + it('rejects a verdict file holding a stream of concatenated documents', () => { + // jq applies the shape check per input document; without the anchored + // parse a two-document file would surface a multi-line verdict. + const r = runGate({ + kissAudit: true, + auditJson: `${validAuditJson}${driftAuditJson}`, + }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.rejection).toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + }); + + it('rejects verdicts contradicting the taxonomy', () => { + for (const auditJson of [ + // sound with a failing axis… + JSON.stringify({ + verdict: 'sound', + kiss: { result: 'fail' }, + minimal_change: { result: 'pass' }, + }), + // …and drift with both axes passing are both inert. + JSON.stringify({ + verdict: 'drift', + kiss: { result: 'pass' }, + minimal_change: { result: 'pass' }, + }), + ]) { + const r = runGate({ kissAudit: true, auditJson }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.rejection).toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + } + }); + + it('rejects a conflict verdict whose round did not stop with a handoff', () => { + // Conflict must STOP BLOCKED: a protocol-deviant round that kept + // fixing and committed is rejected non-retryably instead of clearing + // the gate and pushing the contested code. + const r = runGate({ kissAudit: true, auditJson: conflictAuditJson }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.rejection).toContain( + 'growth-audit verdict is conflict but the round did not stop with a handoff', + ); + }); + + it('rejects a conflict verdict whose round completed as fixed', () => { + // The routing check cannot see the planted-handoff shape: conflict + + // handoff.md + commit + address-summary + green checks clears every + // earlier gate and would push the contested code under outcome=fixed + // while the report posts the park marker. The refusal sits at the push + // boundary — NOT at the verdict gate, where it would also refuse a + // legitimate repair-pass re-audit to conflict (which runs behind the + // first pass's commit and stops with failure.md). + const r = runGate({ + kissAudit: true, + auditJson: conflictAuditJson, + handoffMd: 'conflict handoff\n', + }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('outcome=fixed'); + expect(r.outputs).not.toContain('retryable=true'); + // The validated verdict still surfaces: the trail marker posts and the + // park engages on the handoff's question. + expect(r.outputs).toContain('audit_verdict=conflict'); + expect(r.outputs).toContain('outcome=failed'); + expect(r.rejection).toContain( + 'growth-audit verdict is conflict but the round completed as fixed', + ); + }); + + it('passes a conflict round that stopped with a non-empty handoff', () => { + // The handoff.md stop shape the routing check exists for: conflict + + // non-empty handoff + no commit surfaces the verdict and ends failed — + // never fixed, never pushed. + const r = runGate({ + kissAudit: true, + auditJson: conflictAuditJson, + handoffMd: 'conflict handoff\n', + agentCommit: false, + }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('audit_verdict=conflict'); + expect(r.outputs).toContain('outcome=failed'); + expect(r.outputs).not.toContain('retryable=true'); + }); + + it('rejects a conflict verdict whose handoff.md is empty', () => { + // -f is mere existence: a zero-byte handoff satisfied "stopped with a + // handoff" while the failure report's -s DETAIL_FILE selection never + // embeds an empty file — the PR parked on a handoff that was never + // posted. The stop-artifact convention is -s (non-empty). + const r = runGate({ + kissAudit: true, + auditJson: conflictAuditJson, + handoffMd: '', + agentCommit: false, + }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.rejection).toContain( + 'growth-audit verdict is conflict but the round did not stop with a handoff', + ); + }); + + it('rejects a verdict stream whose later document is truncated or shape-filtered', () => { + // The per-document parse accepted a valid FIRST document followed by + // one jq errors on (or shape-filters out) on the first document's + // verdict — document count must be part of the validation. failAt ends + // the (pre-fix) accepted flow at the build rejection. + for (const auditJson of [ + `${validAuditJson}{"verdict":"conflict","kiss":`, + `${validAuditJson}{}`, + `${validAuditJson}null`, + ]) { + const r = runGate({ kissAudit: true, auditJson, failAt: ['feature'] }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.outputs).not.toContain('retryable=true'); + expect(r.rejection).toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + } + }); + + it('outwrites a RUNNER_TEMP-discovered output forge (last-writer binding)', () => { + // The strip removes the GITHUB_OUTPUT VARIABLE, but the backing file + // stays discoverable and writable under the inherited $RUNNER_TEMP: a + // check appending audit_verdict=sound after the gate's write wins + // last-write-wins unless the gate re-records its validated verdict on + // every exit. + const r = runGate({ + kissAudit: true, + auditJson: driftAuditJson, + discoverOutput: true, + failAt: ['feature'], + }); + expect(r.status).toBe(1); + // Proof the forge actually landed in the output file. + expect(r.stdout).toContain( + 'forge landed: output file discovered via RUNNER_TEMP', + ); + const verdicts = r.outputs + .split('\n') + .filter((l) => l.startsWith('audit_verdict=')); + expect(verdicts.length).toBeGreaterThan(1); + // Step outputs are last-write-wins: the gate's re-record must outwrite + // the forged append. + expect(verdicts.at(-1)).toBe('audit_verdict=drift'); + }); + + it('outwrites the forge at the post-check no-commit exits too', () => { + // The unchanged-branch and missing-summary exits run AFTER the + // schema/contracts checks: a RUNNER_TEMP-discovered forge appended + // mid-check won last-write-wins on both when those exits wrote only + // outcome (probe-verified entrance). The gate's validated verdict must + // be the last line at EVERY exit. + for (const shape of [ + { agentCommit: false }, + { agentCommit: true, summaryPresent: false }, + ]) { + const r = runGate({ + ...shape, + kissAudit: true, + auditJson: driftAuditJson, + discoverOutput: true, + }); + expect(r.status).toBe(1); + expect(r.stdout).toContain( + 'forge landed: output file discovered via RUNNER_TEMP', + ); + const verdicts = r.outputs + .split('\n') + .filter((l) => l.startsWith('audit_verdict=')); + expect(verdicts.at(-1)).toBe('audit_verdict=drift'); + // The control bit rides the same last-writer discipline. + expect(r.outputs).toContain('kiss_audit=true'); + } + }); + + it('outwrites the forge at the noop exit (regression pin)', () => { + const r = runGate({ + agentCommit: false, + noAction: true, + kissAudit: true, + auditJson: driftAuditJson, + discoverOutput: true, + }); + expect(r.status).toBe(0); + expect(r.outputs).toContain('outcome=noop'); + const verdicts = r.outputs + .split('\n') + .filter((l) => l.startsWith('audit_verdict=')); + expect(verdicts.at(-1)).toBe('audit_verdict=drift'); + expect(r.outputs).toContain('kiss_audit=true'); + }); + + it('locks the runner file-command backing files against env plants', () => { + // The strip removes the GITHUB_ENV VARIABLE from the checks, but the + // backing files under $RUNNER_TEMP/_runner_file_commands/ stay + // discoverable (a predictable path) and writable — an append there + // plants environment into every later step of the job, the PAT- + // bearing one included. The gate locks them for the step's lifetime. + const r = runGate({ forgeEnvFile: true }); + expect(r.status).toBe(0); + expect(r.stdout).toContain('env forge blocked: backing file locked'); + // The gate's OWN channel keeps working through the lock. + expect(r.outputs).toContain('outcome=fixed'); + expect(r.outputs).toContain('kiss_audit=false'); + }); + + it('leaves the growth-audit verdict check inert on non-audit rounds', () => { + // Without the KISS_AUDIT tag a malformed verdict file must not engage + // the check — the round proceeds to the checks and ends on their own + // (retryable) rejection, with no verdict line and no verdict rejection. + const r = runGate({ + auditJson: '{"verdict":"bogus"}', + failAt: ['feature'], + }); + expect(r.status).toBe(1); + expect(r.stdout).not.toContain('growth-audit verdict'); + expect(r.stdout).not.toContain( + 'growth-audit round missing a valid growth-audit.json verdict', + ); + expect(r.outputs).not.toContain('audit_verdict='); + expect(r.outputs).toContain('retryable=true'); + }); }); describe('review verification gate: preexisting output is consumed', () => {