From a747f6433ddd5cb5051c893e78ddcf75a0081dd5 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sun, 16 Aug 2026 18:28:22 +0800 Subject: [PATCH 1/6] feat(autofix): audit the approach instead of stopping on growth-budget breach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../run-autofix-review-verification.sh | 34 + .github/workflows/qwen-autofix.yml | 272 +++-- .qwen/skills/autofix/SKILL.md | 41 +- docs/design/autofix-growth-audit.md | 320 ++++++ scripts/tests/qwen-autofix-workflow.test.js | 1003 +++++++++++++---- 5 files changed, 1335 insertions(+), 335 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 369d688fa8c..11a29e681a1 100755 --- a/.github/scripts/run-autofix-review-verification.sh +++ b/.github/scripts/run-autofix-review-verification.sh @@ -129,6 +129,40 @@ reject_fix() { echo "::warning::could not write the gate rejection detail; the verdict stands." exit 1 } +# 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 at the head of the check section (after reject_fix so the rejection +# shape is shared), before the build/schema/footprint checks AND before the +# no-commit/no-op exits below: 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 + AUDIT_VERDICT="$(jq -r ' + select((.verdict // "") | IN("sound", "drift", "conflict")) + | select((.kiss.result // "") | IN("pass", "fail")) + | select((.minimal_change.result // "") | IN("pass", "fail")) + | .verdict' "${WORKDIR}/growth-audit.json" 2> /dev/null || true)" + fi + 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 carry verdict sound|drift|conflict plus kiss.result and minimal_change.result each pass|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}" + # 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}" +fi 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 diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 8ac057f0192..d9b8dc32817 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -165,15 +165,18 @@ env: # value falls back to its default at the read site. 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 @@ -4865,26 +4868,17 @@ jobs: CRITICAL_ONLY='true' CRITICAL_ONLY_GROWTH='true' fi - # 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. - if [[ ! "${GROWTH_DIVERGENCE_ROUNDS}" =~ ^([1-9][0-9]{0,3})$ ]]; then - echo "::warning::GROWTH_DIVERGENCE_ROUNDS='${GROWTH_DIVERGENCE_ROUNDS}' is not a positive count; using 2" - GROWTH_DIVERGENCE_ROUNDS=2 - fi - # 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: + # Growth audit: a budget breach engages Critical-only AND makes the + # round a growth-audit round — a size signal triggers a JUDGMENT, + # never a stop. The agent audits the approach (KISS + minimal + # change, burden of proof inverted) and records a machine-readable + # verdict the verification gate requires: sound re-arms the window + # at the current size and the loop continues, drift simplifies + # first, conflict is the only growth path to a human. Count this + # window's prior per-round over-budget rounds for the audit's + # context (the trajectory clause in feedback.md uses the same + # number). Read this window's prior per-round growth markers + # (written by the report step): # # 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 @@ -4910,56 +4904,95 @@ jobs: # 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. - GROWTH_DIVERGED='false' + # 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. + 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 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" @@ -5524,6 +5575,9 @@ jobs: # 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 @@ -5647,6 +5701,7 @@ jobs: # 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' }}" + 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 @@ -5714,7 +5769,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 handoff.md gate-rejection.md gate-advisories.md agent-api-error agent-api-error-kind agent-timeout resolved-comments.txt comment-replies.json pr.diff; do + for f in feedback.md address-summary.md no-action.md failure.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 pr.diff; do if [[ -f "${WORKDIR}/${f}" ]]; then echo "=============== ${f} ===============" cat "${WORKDIR}/${f}" @@ -5764,10 +5819,20 @@ 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). + KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}' + # The verdict the verification GATE validated (the repair pass's if + # it ran, else the first pass's) — never a re-read of the + # branch-writable growth-audit.json. Empty when the gate rejected + # the verdict itself or never ran; the marker then stays absent. + AUDIT_VERDICT: '${{ steps.verify_repair.outputs.audit_verdict || steps.verify.outputs.audit_verdict }}' MEASURED_AT: '${{ steps.prepare.outputs.measured_at }}' run: |- # gh has its own $GITHUB_ENV-injectable channels: pin the host and @@ -5792,6 +5857,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 @@ -6161,6 +6250,7 @@ jobs: # 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 @@ -6194,6 +6284,7 @@ jobs: # 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 @@ -6343,7 +6434,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 @@ -6354,6 +6445,10 @@ 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 }}' + KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}' + # Gate-validated verdict (see 'Push and report'): never a re-read + # of the branch-writable file. + AUDIT_VERDICT: '${{ steps.verify_repair.outputs.audit_verdict || steps.verify.outputs.audit_verdict }}' run: |- # The head the agent actually evaluated — captured in prepare before # any mutation, not the report-time remote head (which can move @@ -6362,6 +6457,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)' { @@ -6864,6 +6976,10 @@ jobs: # 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 e5f1b897a79..9bf34c1396e 100644 --- a/.qwen/skills/autofix/SKILL.md +++ b/.qwen/skills/autofix/SKILL.md @@ -349,18 +349,35 @@ 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. - 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 — 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, + 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. This is the ONLY + growth-related path to a human, and the question you leave must be the + narrowed contested choice with evidence, not "the diff is too big". - 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 00000000000..bccfd937459 --- /dev/null +++ b/docs/design/autofix-growth-audit.md @@ -0,0 +1,320 @@ +# 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. 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. + +### D. Verdict routing and the audit trail (qwen-autofix.yml report step) + +The report never re-reads `growth-audit.json`: the gate records the +verdict it VALIDATED as a step output (`audit_verdict`), and both +report steps consume that (repair pass first if it ran). The branch's +own build/tests run as the runner user on a predictable WORKDIR after +the gate looks, so a re-read could be overwritten in between — a +forged re-arm, or a conflict verdict flipped back to sound, defeating +the park. The gate-validated verdict is the only verdict that may reach +the trail marker and the re-arm. + +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` sees that its predecessor already blessed the approach and must +bring new evidence to repeat the verdict. 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`, 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. 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 the whole thing. + +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 Qwen Autofix workflow. The Qwen Autofix workflow's OWN + check runs (address lanes included) are excluded wholesale: 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 + (its check concludes after the handoff posts) — counting it would let + the loop's own output unpark the round it came from, and the wasted + failure rounds would feed the consecutive-failure cap toward a + terminal lockout on the exact PR a human is settling. `/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 + re-audits with the prior verdict marker visible, 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`; 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/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index b3b5c1669b5..57a17279f4e 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -2896,11 +2896,12 @@ 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. + // Six sites now: the four feedback/deferral exclusions, the over-budget + // census (command comments are not feedback batches), and the conflict + // handoff wake filter (a /command comment is not a trusted-human + // response and must not unpark a conflict verdict). expect(workflow.split('test("^\\\\s*@qwen-code /") | not').length - 1).toBe( - 5, + 6, ); }); @@ -6739,21 +6740,55 @@ 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-')); + expect( + workflow.match( + /KISS_AUDIT: '\$\{\{ steps\.prepare\.outputs\.kiss_audit \}\}'/g, + ) ?? [], + ).toHaveLength(4); + expect(verificationGateSteps[1]).toContain( + "KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}'", + ); + expect(repairVerificationGateStep).toContain( + "KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}'", + ); + expect(pushAndReportStep).toContain( + "KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}'", + ); + expect(reviewAddressReportStep).toContain( + "KISS_AUDIT: '${{ steps.prepare.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 @@ -6787,12 +6822,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 = '', @@ -6802,195 +6835,171 @@ 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. - expect( - diverge({ - src: 500, - test: 300, - 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. + // 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: 999, test: 999, history: [marker(500, 300, 'true', 1)] }), - ).toBe('false 1'); + census({ + history: [marker(300, 200, 'true', 1), marker(400, 250, 'true', 2)], + }).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. - expect( - diverge({ - src: 250, - test: 150, + }).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( + census({ history: [ marker(300, 200, 'true', 1, 'W1', { run: 1001 }), - marker(600, 300, 'true', 2, 'W1', { + marker(80, 40, 'false', 2, 'W1', { run: 1002, - measured: '2026-01-01T00:01:00Z', + measured: '2026-01-01T00:02:00Z', }), - marker(150, 150, 'true', 2, 'W1', { + marker(500, 400, 'true', 2, 'W1', { run: 1002, - measured: '2026-01-01T00:02:00Z', + measured: '2026-01-01T00:09:00Z', + }), + marker(90, 45, 'false', 2, 'W1', { + run: 1002, + measured: '2026-01-01T00:05: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, + }).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(60, 40, 'true', 1, 'W1', { - run: 1001, - measured: '2026-01-01T00:09:00Z', + marker(300, 200, 'true', 1, 'W1', { run: 1001 }), + marker(600, 300, 'true', 2, 'W1', { + run: 1002, + measured: '2026-01-01T00:01:00Z', }), - marker(500, 400, 'true', 2, 'W1', { + marker(150, 150, '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', { @@ -7003,31 +7012,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, @@ -7038,8 +7043,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 — @@ -7048,9 +7053,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' }, @@ -7063,16 +7066,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. - expect( - diverge({ - src: 500, - test: 300, + }).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( + census({ history: [ { user: { login: 'qwen-code-dev-bot' }, @@ -7090,15 +7090,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' }, @@ -7111,15 +7109,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' }, @@ -7127,13 +7123,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: [ { @@ -7142,16 +7136,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: [ { @@ -7160,33 +7152,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. - 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, + }).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( + 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 @@ -7214,9 +7197,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' }, @@ -7224,19 +7205,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 @@ -7330,28 +7310,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}`); @@ -7360,20 +7338,419 @@ 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 = [], + }) => { + 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` + + `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( + 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( - 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({ + 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 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 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 bind the gate-validated verdict (the repair pass's + // if it ran, else the first pass's). + const auditVerdictBind = + "AUDIT_VERDICT: '${{ steps.verify_repair.outputs.audit_verdict || steps.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}"', + ); + // 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', + ), + ); + + // 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", + ); + rmSync(dir, { recursive: true, force: true }); }); it('anchors a per-window growth baseline and splits src/test nets against a real repo', () => { @@ -7912,7 +8289,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) ?? [], @@ -15174,6 +15556,10 @@ describe('review verification gate: baseline A/B on deterministic rejection', () baselineNoIdentity = false, trackedDirt = false, commFail = false, + // Growth-audit rounds: tag the gate with KISS_AUDIT and optionally seed + // the audit's verdict file in the workdir. + kissAudit = false, + auditJson = null, }) => { const dir = mkdtempSync(join(tmpdir(), 'gate-ab-')); try { @@ -15319,6 +15705,9 @@ describe('review verification gate: baseline A/B on deterministic rejection', () const workdir = join(dir, 'wd'); mkdirSync(workdir); writeFileSync(join(workdir, 'address-summary.md'), 'summary\n'); + if (auditJson !== null) { + writeFileSync(join(workdir, 'growth-audit.json'), auditJson); + } const outFile = join(dir, 'gh-output'); writeFileSync(outFile, ''); @@ -15353,6 +15742,7 @@ 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', }, }, ); @@ -15645,6 +16035,129 @@ 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', + }); + + 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('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', () => { From 364d28df1f73b3e33850278b6abc66bb4853df61 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sun, 16 Aug 2026 20:20:22 +0800 Subject: [PATCH 2/6] fix(autofix): update the artifact-list pin for the growth-audit.json upload entry --- scripts/tests/qwen-autofix-workflow.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 57a17279f4e..b5ef9ec139c 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -11912,7 +11912,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'); From ba4eb0916b758e56c123fe914aaadcc2e6dceb92 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sun, 16 Aug 2026 17:22:31 +0000 Subject: [PATCH 3/6] fix(autofix): surface conflict verdicts past the failure.md exits and strip verdict forgery channels (#9262) --- .../run-autofix-review-verification.sh | 95 +++++--- .github/workflows/qwen-autofix.yml | 35 +-- .qwen/skills/autofix/SKILL.md | 15 +- docs/design/autofix-growth-audit.md | 48 ++-- scripts/tests/qwen-autofix-workflow.test.js | 210 +++++++++++++++++- 5 files changed, 326 insertions(+), 77 deletions(-) diff --git a/.github/scripts/run-autofix-review-verification.sh b/.github/scripts/run-autofix-review-verification.sh index 11a29e681a1..cc2f9bb8f0a 100755 --- a/.github/scripts/run-autofix-review-verification.sh +++ b/.github/scripts/run-autofix-review-verification.sh @@ -60,27 +60,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 - -# 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" @@ -133,12 +112,15 @@ reject_fix() { # 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 at the head of the check section (after reject_fix so the rejection -# shape is shared), before the build/schema/footprint checks AND before the -# no-commit/no-op exits below: 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. +# 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 @@ -146,16 +128,33 @@ if [[ "${KISS_AUDIT:-false}" == 'true' ]]; then 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' "${WORKDIR}/growth-audit.json" 2> /dev/null || true)" fi + # Anchor the parsed value: jq applies the shape check per input document + # and happily parses a *stream* of concatenated verdict objects, which + # would otherwise surface as a multi-line AUDIT_VERDICT. + [[ "${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 carry verdict sound|drift|conflict plus kiss.result and minimal_change.result each pass|fail. Re-run the audit and produce the file; do not push without it." + 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" && ! -f "${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 # 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 @@ -163,6 +162,27 @@ if [[ "${KISS_AUDIT:-false}" == 'true' ]]; then # the only verdict that may reach the trail marker and the re-arm. echo "audit_verdict=${AUDIT_VERDICT}" >> "${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 + +# 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 @@ -210,7 +230,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 @@ -305,6 +325,17 @@ 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), and GITHUB_ENV/GITHUB_PATH +# plant environment for the PAT-bearing steps that follow. 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 "$@" +} 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 @@ -312,7 +343,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 @@ -332,7 +363,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 } @@ -873,7 +904,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__/**' \ diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index e320127f0a5..220d0787727 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -4800,7 +4800,7 @@ jobs: | select((.body // "") | contains("" 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 "" @@ -6430,8 +6434,8 @@ 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 "" @@ -6598,8 +6602,9 @@ jobs: GROWTH_BASE_WIN: '${{ steps.prepare.outputs.growth_base_win }}' KISS_AUDIT: '${{ steps.prepare.outputs.kiss_audit }}' # Gate-validated verdict (see 'Push and report'): never a re-read - # of the branch-writable file. - AUDIT_VERDICT: '${{ steps.verify_repair.outputs.audit_verdict || steps.verify.outputs.audit_verdict }}' + # of the branch-writable file; first pass preferred over the + # repair pass's re-read for the same forgery reason. + AUDIT_VERDICT: '${{ steps.verify.outputs.audit_verdict || steps.verify_repair.outputs.audit_verdict }}' UPSERT_SRC: '${{ steps.stage.outputs.upsert_src }}' TRUSTED_PATH: '${{ steps.stage.outputs.trusted_path }}' run: |- @@ -7134,8 +7139,8 @@ 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 "" diff --git a/.qwen/skills/autofix/SKILL.md b/.qwen/skills/autofix/SKILL.md index 46057bce3f2..298b02cc107 100644 --- a/.qwen/skills/autofix/SKILL.md +++ b/.qwen/skills/autofix/SKILL.md @@ -356,12 +356,15 @@ silently overriding or silently complying. 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 — 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, - and a repeated verdict after a prior audit this window must bring new - evidence (the feedback section lists the prior audits). + `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 diff --git a/docs/design/autofix-growth-audit.md b/docs/design/autofix-growth-audit.md index bccfd937459..0cc92bab186 100644 --- a/docs/design/autofix-growth-audit.md +++ b/docs/design/autofix-growth-audit.md @@ -138,21 +138,35 @@ 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. 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. +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`: the gate records the verdict it VALIDATED as a step output (`audit_verdict`), and both -report steps consume that (repair pass first if it ran). The branch's -own build/tests run as the runner user on a predictable WORKDIR after -the gate looks, so a re-read could be overwritten in between — a -forged re-arm, or a conflict verdict flipped back to sound, defeating -the park. The gate-validated verdict is the only verdict that may reach -the trail marker and the re-arm. +report steps consume the FIRST pass's if it ran (the repair pass +re-reads the branch-writable file after the first pass's build had a +write window, so its re-read can only ever lose). The branch's own +build/tests run as the runner user on a predictable WORKDIR after the +gate looks, so a re-read could be overwritten in between — a forged +re-arm, or a conflict verdict flipped back to sound, defeating the +park. The gate-validated verdict is the only verdict that may reach +the trail marker and the re-arm. The check subprocesses that run the +branch's build/tests are stripped of the runner injection channels +(`GITHUB_OUTPUT`/`GITHUB_ENV`/`GITHUB_PATH`), so branch code cannot +append its own `audit_verdict` after the gate's write (step outputs +are last-write-wins). Every audit round posts its verdict in the round report comment with a machine-readable marker @@ -166,14 +180,15 @@ 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`, the report step additionally posts the re-arm -marker comment (``). This reuses the existing +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. Effectively an automatic, -audit-gated `/retry`. +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 @@ -296,8 +311,9 @@ behavior and must be rewritten with the change: 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`; conflict - handoff idempotence. + `` 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 diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index f1c9137c771..c074cbff328 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -7647,10 +7647,12 @@ exit 1 )?.[0]; expect(emitMarkerFnFailure).toBeTruthy(); expect(emitMarkerFnFailure).toBe(emitMarkerFn); - // Both report steps bind the gate-validated verdict (the repair pass's - // if it ran, else the first pass's). + // Both report steps bind the gate-validated verdict, FIRST PASS + // preferred: the repair pass re-reads the branch-writable file after + // the first pass's build had a write window, so a forged rewrite must + // lose to the first pass's validated verdict. const auditVerdictBind = - "AUDIT_VERDICT: '${{ steps.verify_repair.outputs.audit_verdict || steps.verify.outputs.audit_verdict }}'"; + "AUDIT_VERDICT: '${{ steps.verify.outputs.audit_verdict || steps.verify_repair.outputs.audit_verdict }}'"; expect(pushAndReportStep).toContain(auditVerdictBind); expect(reviewAddressReportStep).toContain(auditVerdictBind); // Push + no-op report paths allow the re-arm; the failure/handoff path @@ -7732,6 +7734,26 @@ exit 1 expect(reviewVerificationRunner).toContain( 'echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}"', ); + // jq parses a *stream* of concatenated documents happily; the anchored + // regex keeps a multi-document verdict file out. + expect(reviewVerificationRunner).toContain( + '[[ "${AUDIT_VERDICT}" =~ ^(sound|drift|conflict)$ ]] || AUDIT_VERDICT=\'\'', + ); + // 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 "$@"', + ); + 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( @@ -7743,6 +7765,13 @@ exit 1 '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). @@ -14965,11 +14994,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, ''); @@ -16811,6 +16852,13 @@ describe('review verification gate: baseline A/B on deterministic rejection', () // the audit's verdict file in the workdir. kissAudit = false, auditJson = null, + // 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, }) => { const dir = mkdtempSync(join(tmpdir(), 'gate-ab-')); try { @@ -16870,6 +16918,14 @@ 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', + 'fi', 'if [[ "$1" == "run" && "$2" == "build" ]]; then', ' head="$(git rev-parse HEAD)"', ' for s in ${FAIL_BUILD_SHAS}; do', @@ -16959,6 +17015,12 @@ describe('review verification gate: baseline A/B on deterministic rejection', () 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); + } const outFile = join(dir, 'gh-output'); writeFileSync(outFile, ''); @@ -16994,6 +17056,7 @@ describe('review verification gate: baseline A/B on deterministic rejection', () WORKSPACE_TEST_FAIL: addWorkspace ? '1' : '', RESOLVED_PKGS: addWorkspace ? 'packages/newpkg' : '', KISS_AUDIT: kissAudit ? 'true' : 'false', + FORGE_OUTPUT: forgeOutput ? '1' : '', }, }, ); @@ -17299,6 +17362,18 @@ describe('review verification gate: baseline A/B on deterministic rejection', () 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 }); @@ -17393,6 +17468,125 @@ describe('review verification gate: baseline A/B on deterministic rejection', () 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', + }); + 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'); + expect(r.outputs.match(/audit_verdict=/g)).toHaveLength(1); + // Proof the forge branch actually executed inside the check. + expect(r.stdout).toContain('forge blocked: GITHUB_OUTPUT not inherited'); + }); + + 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('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 From b5e8ca6badfa1f0fa98dbc55719550f542661deb Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sun, 16 Aug 2026 21:58:56 +0000 Subject: [PATCH 4/6] fix(autofix): harden the growth-audit verdict pipeline and park wake set (#9262) --- .../run-autofix-review-verification.sh | 76 ++++- .github/workflows/qwen-autofix.yml | 57 +++- docs/design/autofix-growth-audit.md | 62 ++-- scripts/tests/qwen-autofix-workflow.test.js | 287 +++++++++++++++++- 4 files changed, 423 insertions(+), 59 deletions(-) diff --git a/.github/scripts/run-autofix-review-verification.sh b/.github/scripts/run-autofix-review-verification.sh index cc2f9bb8f0a..d2546def2c6 100755 --- a/.github/scripts/run-autofix-review-verification.sh +++ b/.github/scripts/run-autofix-review-verification.sh @@ -45,6 +45,15 @@ 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 +# child sources at startup — a $GITHUB_ENV plant from an earlier step +# would run inside the trusted helpers this gate spawns, past the channel +# strip below (it covers their environment, not bash's startup file) — +# and 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 # 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 @@ -76,6 +85,9 @@ 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}" + 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 @@ -108,6 +120,16 @@ 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 past the record therefore +# re-appends the validated verdict INLINE (no function call: gate snippets +# extracted by the contract suite must stay executable standalone), 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. # 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 @@ -124,19 +146,24 @@ reject_fix() { if [[ "${KISS_AUDIT:-false}" == 'true' ]]; then AUDIT_VERDICT='' if [[ -f "${WORKDIR}/growth-audit.json" ]]; then - AUDIT_VERDICT="$(jq -r ' - select((.verdict // "") | IN("sound", "drift", "conflict")) + # 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' "${WORKDIR}/growth-audit.json" 2> /dev/null || true)" + | .verdict end' "${WORKDIR}/growth-audit.json" 2> /dev/null || true)" fi - # Anchor the parsed value: jq applies the shape check per input document - # and happily parses a *stream* of concatenated verdict objects, which - # would otherwise surface as a multi-line AUDIT_VERDICT. + # 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 { @@ -152,15 +179,21 @@ if [[ "${KISS_AUDIT:-false}" == 'true' ]]; then # 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" && ! -f "${WORKDIR}/handoff.md" ]]; then + 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 @@ -251,6 +284,9 @@ baseline_also_fails() { tail -c 3000 "${GATE_LOG}" 2> /dev/null echo '````' } > "${WORKDIR}/gate-rejection.md" || true + 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 @@ -329,12 +365,15 @@ seed_dist_note() { # 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), and GITHUB_ENV/GITHUB_PATH -# plant environment for the PAT-bearing steps that follow. Same class the +# 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 "$@" + 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 @@ -425,6 +464,9 @@ 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}" + 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" @@ -1073,6 +1115,9 @@ if [[ "${#BITE_FILES[@]}" -gt 0 && -n "${BITE_SRC}" ]]; then tail -c 3000 "${GATE_LOG}" 2> /dev/null echo '````' } > "${WORKDIR}/gate-rejection.md" || true + if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" + fi exit 1 } git reset --quiet 2>> "${GATE_LOG}" || true @@ -1123,5 +1168,16 @@ 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}" +if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then + echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}" +fi diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 220d0787727..cfab654d071 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -4977,6 +4977,15 @@ jobs: # /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. CONFLICT_SINCE="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" ' [ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "") | [ scan("") ] | .[] @@ -4984,7 +4993,8 @@ jobs: | max // ""' "${WORKDIR}/ic.json" 2> /dev/null || echo "")" if [[ -n "${CONFLICT_SINCE}" && "${STALE}" != 'true' ]]; then CONFLICT_WAKE="$(jq -rs \ - --arg since "${CONFLICT_SINCE}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ + --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) @@ -4998,9 +5008,10 @@ jobs: | select(((.author_association // "") | IN($trust[]))) | select((.body // "") | test("`), so later rounds' audits can read the trail — a second audit after a prior -`sound` sees that its predecessor already blessed the approach and must -bring new evidence to repeat the verdict. The marker's `win` must be -`steps.prepare.outputs.growth_base_win` (the key the baseline was READ +`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 @@ -195,7 +213,9 @@ 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 the whole thing. +`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. @@ -273,8 +293,10 @@ converges and the label releases with zero human rounds. ## Failure modes and bounds -- **Audit wrongly blesses real drift.** Bounded: the next breach - re-audits with the prior verdict marker visible, and repeated +- **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 diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index c074cbff328..d8f1d72ca00 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -7404,6 +7404,8 @@ exit 1 // (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'), @@ -7418,6 +7420,7 @@ exit 1 '-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"`, ], @@ -7617,6 +7620,53 @@ exit 1 ], }), ).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 }); @@ -7647,12 +7697,15 @@ exit 1 )?.[0]; expect(emitMarkerFnFailure).toBeTruthy(); expect(emitMarkerFnFailure).toBe(emitMarkerFn); - // Both report steps bind the gate-validated verdict, FIRST PASS - // preferred: the repair pass re-reads the branch-writable file after - // the first pass's build had a write window, so a forged rewrite must - // lose to the first pass's validated verdict. + // 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.verify.outputs.audit_verdict || steps.verify_repair.outputs.audit_verdict }}'"; + "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 @@ -7734,11 +7787,31 @@ exit 1 expect(reviewVerificationRunner).toContain( 'echo "audit_verdict=${AUDIT_VERDICT}" >> "${GITHUB_OUTPUT}"', ); - // jq parses a *stream* of concatenated documents happily; the anchored - // regex keeps a multi-document verdict file out. + // 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'"); + // Five re-record sites: reject_fix, the two crash exits, and the + // noop/fixed outcome writes — dropping any one re-opens the forge on + // that exit path. + expect( + reviewVerificationRunner.match( + /if \[\[ "\$\{AUDIT_VERDICT_RECORDED:-false\}" == 'true' \]\]; then/g, + ) ?? [], + ).toHaveLength(5); + expect(reviewVerificationRunner).toContain('unset BASH_ENV BITE_RUNNER'); // 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( @@ -7749,7 +7822,7 @@ exit 1 // 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 "$@"', + 'env -u GITHUB_OUTPUT -u GITHUB_ENV -u GITHUB_PATH -u GITHUB_STEP_SUMMARY "$@"', ); expect(reviewVerificationRunner).toContain( 'strip_runner_channels npm run test', @@ -12773,8 +12846,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 = ( @@ -13424,6 +13499,49 @@ 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' }), + ).toMatchObject({ + status: 0, + written: expect.stringContaining('audit_verdict=sound'), + }); }); it('posts a human-handoff marker when review addressing reaches a terminal handoff', () => { @@ -16859,6 +16977,10 @@ describe('review verification gate: baseline A/B on deterministic rejection', () // 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, }) => { const dir = mkdtempSync(join(tmpdir(), 'gate-ab-')); try { @@ -16925,6 +17047,12 @@ describe('review verification gate: baseline A/B on deterministic rejection', () ' 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)"', @@ -16999,7 +17127,16 @@ 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_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'), @@ -17021,8 +17158,14 @@ describe('review verification gate: baseline A/B on deterministic rejection', () if (handoffMd !== null) { writeFileSync(join(workdir, 'handoff.md'), handoffMd); } - 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, ''); + const summaryFile = join(dir, 'step-summary'); + writeFileSync(summaryFile, ''); const res = spawnSync( 'bash', @@ -17038,6 +17181,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, @@ -17057,6 +17201,7 @@ describe('review verification gate: baseline A/B on deterministic rejection', () RESOLVED_PKGS: addWorkspace ? 'packages/newpkg' : '', KISS_AUDIT: kissAudit ? 'true' : 'false', FORGE_OUTPUT: forgeOutput ? '1' : '', + DISCOVER_OUTPUT: discoverOutput ? '1' : '', }, }, ); @@ -17064,6 +17209,7 @@ describe('review verification gate: baseline A/B on deterministic rejection', () 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') : '', @@ -17478,6 +17624,9 @@ describe('review verification gate: baseline A/B on deterministic rejection', () 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'); @@ -17530,9 +17679,15 @@ describe('review verification gate: baseline A/B on deterministic rejection', () expect(r.status).toBe(1); expect(r.outputs).toContain('audit_verdict=drift'); expect(r.outputs).not.toContain('audit_verdict=sound'); - expect(r.outputs.match(/audit_verdict=/g)).toHaveLength(1); - // Proof the forge branch actually executed inside the check. + // 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', () => { @@ -17587,6 +17742,112 @@ describe('review verification gate: baseline A/B on deterministic rejection', () ); }); + 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('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 From 7bb4d2fde1679ff9192a9e7aa5ae4684435f5db1 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Mon, 17 Aug 2026 03:33:54 +0000 Subject: [PATCH 5/6] fix(autofix): close the verdict-pipeline forgeries and loop-generated wake entrances (#9262) --- .../run-autofix-review-verification.sh | 81 ++- .github/workflows/qwen-autofix.yml | 196 ++++++- docs/design/autofix-growth-audit.md | 82 ++- scripts/tests/package-scripts.test.js | 2 +- scripts/tests/qwen-autofix-workflow.test.js | 495 ++++++++++++++++-- 5 files changed, 773 insertions(+), 83 deletions(-) diff --git a/.github/scripts/run-autofix-review-verification.sh b/.github/scripts/run-autofix-review-verification.sh index d2546def2c6..5a1918aa0dd 100755 --- a/.github/scripts/run-autofix-review-verification.sh +++ b/.github/scripts/run-autofix-review-verification.sh @@ -47,13 +47,40 @@ if [ -s /etc/gitconfig ]; then fi # Two more inherited knobs steer EXECUTION itself, and neither has a # legitimate setter: BASH_ENV names a file every non-interactive bash -# child sources at startup — a $GITHUB_ENV plant from an earlier step -# would run inside the trusted helpers this gate spawns, past the channel -# strip below (it covers their environment, not bash's startup file) — -# and BITE_RUNNER selects the bite check's runner command, which executes -# unwrapped with the gate's full environment. Strip them with the GIT_* -# class. +# 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 @@ -85,6 +112,7 @@ 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 @@ -124,12 +152,21 @@ reject_fix() { # 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 past the record therefore -# re-appends the validated verdict INLINE (no function call: gate snippets -# extracted by the contract suite must stay executable standalone), 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. +# 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 @@ -201,6 +238,10 @@ if [[ -f "${WORKDIR}/failure.md" && -n "$(git status --porcelain)" ]]; then 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 @@ -208,6 +249,10 @@ 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 @@ -284,6 +329,7 @@ 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 @@ -464,6 +510,7 @@ 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 @@ -471,12 +518,20 @@ if git diff --quiet "origin/${BRANCH}...${BRANCH}"; then 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 @@ -1115,6 +1170,7 @@ 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 @@ -1178,6 +1234,7 @@ if [[ "${AUDIT_VERDICT:-}" == 'conflict' ]]; then 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.yml b/.github/workflows/qwen-autofix.yml index cfab654d071..cc6d8a71ad5 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -3437,6 +3437,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("") ] | .[] @@ -5009,7 +5078,7 @@ jobs: | select((.body // "") | test("`, + }; + 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 @@ -16970,6 +17304,14 @@ describe('review verification gate: baseline A/B on deterministic rejection', () // 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, @@ -17127,7 +17469,15 @@ describe('review verification gate: baseline A/B on deterministic rejection', () mkdirSync(rt); writeFileSync( join(rt, 'check-settings-schema.sh'), - 'if [[ "${DISCOVER_OUTPUT:-}" == "1" ]]; then\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' + @@ -17148,7 +17498,12 @@ describe('review verification gate: baseline A/B on deterministic rejection', () ); const workdir = join(dir, 'wd'); mkdirSync(workdir); - writeFileSync(join(workdir, 'address-summary.md'), 'summary\n'); + 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); } @@ -17164,6 +17519,10 @@ describe('review verification gate: baseline A/B on deterministic rejection', () // 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, ''); @@ -17202,9 +17561,15 @@ describe('review verification gate: baseline A/B on deterministic rejection', () 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}`, @@ -17848,6 +18213,66 @@ describe('review verification gate: baseline A/B on deterministic rejection', () 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 From 9ae9e7cdb17d73f4202ca8c29163052576b75225 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Thu, 20 Aug 2026 13:51:52 +0800 Subject: [PATCH 6/6] fix(ci): drop the retired divergence rationale records (af-046/af-047) from qwen-autofix.md --- .github/workflows/qwen-autofix.md | 83 ------------------------------- 1 file changed, 83 deletions(-) diff --git a/.github/workflows/qwen-autofix.md b/.github/workflows/qwen-autofix.md index 3db3bafeff4..54294dcc52c 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…