diff --git a/.github/scripts/run-autofix-review-verification.sh b/.github/scripts/run-autofix-review-verification.sh index 4a46a182fa3..1e334ea923a 100755 --- a/.github/scripts/run-autofix-review-verification.sh +++ b/.github/scripts/run-autofix-review-verification.sh @@ -153,13 +153,15 @@ baseline_also_fails() { } > "${WORKDIR}/gate-rejection.md" || true exit 1 fi + # Every retryable exit below hands the tree to the repair agent with + # dist/ REBUILT FROM BASELINE SOURCES (the restore checkout brings back + # tracked files only) — the mirror of the dist confound that exempted + # typecheck from the A/B. seed_dist_note seeds the repair feedback so + # the agent rebuilds before it trusts any dist-consuming check. The + # pre-existing exit is the exception: no repair runs for it, so the + # note stays out of its document. if [[ "${rc}" -ne 1 ]]; then - # Both retryable exits below hand the tree to the repair agent with - # dist/ REBUILT FROM BASELINE SOURCES (the restore checkout brings back - # tracked files only) — the mirror of the dist confound that exempted - # typecheck from the A/B. The note seeds the repair feedback so the - # agent rebuilds before it trusts any dist-consuming check. - echo "⚠️ the baseline leg rebuilt dist/ from baseline sources — run npm run build before typecheck/tests" >> "${GATE_LOG}" + seed_dist_note echo "🔁 baseline is green — the failure belongs to this round" \ | tee -a "${GATE_LOG}" return 1 @@ -185,9 +187,14 @@ baseline_also_fails() { # verdict-less gate crash. # (sig_head was extracted before the detach.) sig_base="$(fail_signature "${ab_log}")" || true - new_in_round="$(comm -23 <(printf '%s\n' "${sig_head}") <(printf '%s\n' "${sig_base}"))" || + new_in_round="$(comm -23 <(printf '%s\n' "${sig_head}") <(printf '%s\n' "${sig_base}"))" || { + seed_dist_note + echo "🔁 signature comparison failed — fail-closed, charged to the round" \ + | tee -a "${GATE_LOG}" return 1 + } if [[ -z "${sig_head}" || -z "${sig_base}" ]] || [[ -n "${new_in_round}" ]]; then + seed_dist_note echo "🔁 baseline fails for a DIFFERENT reason — charged to the round" \ | tee -a "${GATE_LOG}" return 1 @@ -212,6 +219,12 @@ fail_signature() { grep -oE "[^ '\"]+\([0-9]+,[0-9]+\): error TS[0-9]+.*" "${1}" 2> /dev/null \ | sed -E 's/\([0-9]+,[0-9]+\)//' | sort -u } +# The one emit point for the dist-rebuild steering note — every retryable +# exit of baseline_also_fails after the baseline leg calls this, so the +# guidance cannot drift across exits. +seed_dist_note() { + echo "⚠️ the baseline leg rebuilt dist/ from baseline sources — run npm run build before typecheck/tests" >> "${GATE_LOG}" +} 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 diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 1ea08ed3189..02b428e3d45 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -11581,7 +11581,9 @@ describe('review verification gate: baseline A/B on deterministic rejection', () restoreClash = false, hugeFail = false, noIdentity = false, + baselineNoIdentity = false, trackedDirt = false, + commFail = false, }) => { const dir = mkdtempSync(join(tmpdir(), 'gate-ab-')); try { @@ -11663,6 +11665,11 @@ describe('review verification gate: baseline A/B on deterministic rejection', () // vite/esbuild shape: a red build with no tsc diagnostic at all. ' echo "error during build: something exploded"; exit 1', ' fi', + ' if [[ "$head" == "${BASELINE_SHA:-}" && "${BASELINE_NO_IDENTITY:-}" == "1" ]]; then', + // Same shape restricted to the baseline leg — the head keeps its + // tsc identity while the baseline loses its. + ' echo "error during build: something exploded"; exit 1', + ' fi', ' if [[ "$head" == "${BASELINE_SHA:-}" && "${TRACKED_DIRT:-}" == "1" ]]; then', // The build rewrites a TRACKED file (the settings-schema shape): // f.txt differs across refs, so an undiscarded rewrite makes the @@ -11698,6 +11705,13 @@ describe('review verification gate: baseline A/B on deterministic rejection', () ].join('\n'), ); chmodSync(join(bin, 'npm'), 0o755); + if (commFail) { + // Shadows the system comm via PATH precedence: the signature + // comparison itself fails (the SIGPIPE-under-pipefail class), so + // the gate takes its fail-closed retryable exit. + writeFileSync(join(bin, 'comm'), '#!/bin/bash\nexit 1\n'); + chmodSync(join(bin, 'comm'), 0o755); + } const rt = join(dir, 'rt'); mkdirSync(rt); writeFileSync( @@ -11741,6 +11755,7 @@ describe('review verification gate: baseline A/B on deterministic rejection', () EXTRA_BASELINE_DIAG: extraBaselineDiag ? '1' : '', RESTORE_CLASH: restoreClash ? '1' : '', NO_IDENTITY: noIdentity ? '1' : '', + BASELINE_NO_IDENTITY: baselineNoIdentity ? '1' : '', TRACKED_DIRT: trackedDirt ? '1' : '', SCHEMA_FAIL: schemaFail ? '1' : '', TYPECHECK_FAIL: typecheckFail ? '1' : '', @@ -11795,6 +11810,11 @@ describe('review verification gate: baseline A/B on deterministic rejection', () // The baseline leg's own transcript is the ONLY proof behind the // verdict — it must reach the rejection document. expect(r.rejection).toContain(`stub build FAILED at ${r.baselineSha}`); + // No repair runs for a pre-existing failure — the dist/ steering note + // is for the repair agent and stays out of this document. + expect(r.rejection).not.toContain( + 'run npm run build before typecheck/tests', + ); expect(r.headAfter).toBe('feature'); }); @@ -11934,6 +11954,46 @@ describe('review verification gate: baseline A/B on deterministic rejection', () expect(r.outputs).toContain('retryable=true'); expect(r.outputs).not.toContain('preexisting=true'); expect(r.stdout).toContain('DIFFERENT reason'); + // The same repair handoff as the green path — the dist/ warning must + // seed this rejection too. + expect(r.rejection).toContain('run npm run build before typecheck/tests'); + }); + + it('charges the round when the baseline fails without a failure identity', () => { + // Mirror of the head-side noIdentity shape on the other leg: the + // baseline crashes vite/esbuild-style with no tsc diagnostic, so its + // signature is empty and identity cannot be established — fail + // closed and charge the round, with the same repair handoff (and + // dist/ note) as the sibling retryable exits. + const r = runGate({ + failAt: ['feature', 'origin/feature'], + baselineNoIdentity: true, + }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('retryable=true'); + expect(r.outputs).not.toContain('preexisting=true'); + expect(r.stdout).toContain('DIFFERENT reason'); + expect(r.rejection).toContain('run npm run build before typecheck/tests'); + }); + + it('seeds the dist-rebuild warning when the signature comparison itself fails', () => { + // comm failing (SIGPIPE under pipefail, an infrastructure hiccup) + // takes the same retryable handoff as the green/different-signature + // exits — without the note the repair agent trusts baseline-built + // dist/ and chases phantom dist-consuming failures. + const r = runGate({ + failAt: ['feature', 'origin/feature'], + commFail: true, + }); + expect(r.status).toBe(1); + expect(r.outputs).toContain('retryable=true'); + expect(r.outputs).not.toContain('preexisting=true'); + expect(r.rejection).toContain('run npm run build before typecheck/tests'); + // Like its sibling exits, this one names its verdict — an oncall must + // distinguish "the comparison itself failed" from "baseline is green" + // without re-running the A/B. + expect(r.rejection).toContain('signature comparison failed'); + expect(r.headAfter).toBe('feature'); }); it('keeps the green path intact', () => {