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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 42 additions & 8 deletions .github/scripts/run-autofix-review-verification.sh
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,25 @@ baseline_also_fails() {
# is A/B-eligible) — the baseline IS the tree under test; nothing to
# compare.
[[ "${baseline}" != "${current}" ]] || return 1
# The head transcript is already complete, and an empty head signature
# fails closed regardless of what the baseline would say — so decide it
# BEFORE paying the detach + full re-run + restore for a verdict that was
# never in question (esbuild/vite/crash failures, the KNOWN LIMIT class).
local sig_head
sig_head="$(fail_signature "${GATE_LOG}.check")" || true
if [[ -z "${sig_head}" ]]; then
echo "🔁 no failure identity in the head transcript — charged to the round" \
| tee -a "${GATE_LOG}"
return 1
fi
echo "🔁 Baseline A/B: re-running the failed check at origin/${BRANCH}" \
"(${baseline})" | tee -a "${GATE_LOG}"
# The build under test may have REWRITTEN tracked artifacts (the vscode
# companion settings schema is regenerated by scripts/build.js): discard
# build dirt or the checkout refuses and a real verdict degrades into the
# restore-failure crash below. Tracked-only, and the tree was asserted
# clean before the deterministic checks — anything here is build output.
git restore -- . 2>> "${GATE_LOG}" || true
git checkout --quiet --detach "${baseline}" 2>> "${GATE_LOG}" || return 1
# The baseline transcript goes to a SIDE log: gate-rejection.md renders
# the dynamic `tail_budget` tail of GATE_LOG as the evidence window, and
Expand All @@ -116,16 +133,33 @@ baseline_also_fails() {
if ! "$@" >> "${ab_log}" 2>&1; then
rc=1
fi
git restore -- . 2>> "${GATE_LOG}" || true
if ! git checkout --quiet "${BRANCH}" 2>> "${GATE_LOG}"; then
# The tree is no longer the one under verification and nothing after
# this point may trust it. Not retryable either: the repair agent works
# in this very checkout and performs no git recovery, so on a detached
# tree its commit would land on the baseline and be orphaned. The round
# ends here; the next one starts clean from the trusted checkout.
reject_fix 'could not restore the verification tree after the baseline check' \
false false
# this point may trust it — including the repair agent (its commit would
# orphan on the detached baseline). But a transient git-state failure is
# NOT a verdict about the failure's origin, and a plain outcome=failed
# is an EVALUATED rejection: the watermark advances and the item is
# handed off for good. Leave outcome UNSET so the report's gate-crashed
# path retries on the next scan's fresh checkout — and write the detail
# document so the crash comment still explains itself.
echo "❌ could not restore the verification tree after the baseline check"
{
echo '**could not restore the verification tree after the baseline check**'
echo
echo '````'
tail -c 3000 "${GATE_LOG}" 2> /dev/null
echo '````'
} > "${WORKDIR}/gate-rejection.md" || true
exit 1
fi
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}"
echo "🔁 baseline is green — the failure belongs to this round" \
| tee -a "${GATE_LOG}"
return 1
Expand All @@ -143,13 +177,13 @@ baseline_also_fails() {
# failure sets to NO-MATCH. No diagnostics on either side means identity
# cannot be established, and the rejection stays charged to the round
# (fail closed).
local sig_head sig_base new_in_round
local sig_base new_in_round
# `|| true`: grep exits 1 on the NORMAL no-match case, and these
# assignments only survive `set -e` today because this function is called
# from an `if` condition (which suspends errexit). A future unconditional
# call site would otherwise turn the documented fail-closed path into a
# verdict-less gate crash.
sig_head="$(fail_signature "${GATE_LOG}.check")" || true
# (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}"))" ||
return 1
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4595,6 +4595,8 @@ jobs:
"${WORKDIR}/failure.md" \
"${WORKDIR}/handoff.md" \
"${WORKDIR}/gate-output.log" \
"${WORKDIR}/gate-output.log.check" \
"${WORKDIR}/gate-output.log.baseline" \
"${WORKDIR}/gate-rejection.md" \
"${WORKDIR}/agent-api-error" \
"${WORKDIR}/agent-api-error-kind" \
Expand Down Expand Up @@ -5603,6 +5605,16 @@ jobs:
echo '<!-- autofix-gate-rejection-start -->'
echo "**Why it was not pushed:**"
echo
# The rejection text below was written BEFORE the stale-base
# auto-update: on that path its framing (pre-existing,
# unreachable, cured by a base update) is already stale — the
# base HAS been updated, and the next round re-measures and
# may charge the round. Say so, or the retry agent is steered
# toward no-action on the one round designed to re-measure.
if [[ "${STALE_BASE_RETRY:-false}" == 'true' ]]; then
echo "_Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round._"
echo
fi
# reject_fix sizes its own document: the evidence tail is
# dynamic (budget 3300 minus the preamble, floored at 500),
# so the finished file tops out ≈3.3 KB. 3900 is headroom,
Expand Down
107 changes: 98 additions & 9 deletions scripts/tests/qwen-autofix-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7721,6 +7721,11 @@ exit 1
/PUSH_RACE_MERGED='false'\n\s+for push_attempt in 1 2 3; do/,
);
expect(pushAndReportStep).toContain('verification predates that merge');
// The STALE_BASE_RETRY handoff embeds a rejection written BEFORE the
// auto-update; the note un-poisons its framing for the retry agent.
expect(reviewAddressReportStep).toContain(
'the base has since been auto-updated',
);
// Bounded: the loop gives up after the last attempt instead of spinning.
// The structural pin connects the guard value to the error exit — a
// mutation of == 3 to == 4 survives presence-only checks: the loop
Expand Down Expand Up @@ -8738,6 +8743,28 @@ exit 1
expect(staleConflict.split('|')[0]).toBe(NEWEST);
expect(staleConflict).toContain('Could not produce a passing fix');

// Pre-existing verdicts pick their remedy from the compare the step
// already ran — swapping the two clause bodies must fail here, not ship
// a headline prescribing a merge that changes nothing.
const preAhead = run(
{ OUTCOME: 'failed', PREEXISTING: 'true' },
{ gateRejection: true },
);
expect(preAhead).toContain('PRE-EXISTING failure');
expect(preAhead).toContain('own pre-round code needs attention');
expect(preAhead).not.toContain('base update (merge main)');
const preBehindConflict = run(
{
OUTCOME: 'failed',
PREEXISTING: 'true',
CMP_STATUS_STUB: 'behind',
UPDATE_OK_STUB: '0',
},
{ gateRejection: true },
);
expect(preBehindConflict).toContain('PRE-EXISTING failure');
expect(preBehindConflict).toContain('base update (merge main)');

// Gate crash (no verdict): keep the feedback live and retry.
const crashed = run({ OUTCOME: '' });
expect(crashed.split('|')[0]).toBe(SENTINEL);
Expand Down Expand Up @@ -11323,6 +11350,8 @@ describe('review verification gate: baseline A/B on deterministic rejection', ()
extraBaselineDiag = false,
restoreClash = false,
hugeFail = false,
noIdentity = false,
trackedDirt = false,
}) => {
const dir = mkdtempSync(join(tmpdir(), 'gate-ab-'));
try {
Expand Down Expand Up @@ -11400,6 +11429,16 @@ describe('review verification gate: baseline A/B on deterministic rejection', ()
' if [[ -n "${HEAD_MSG:-}" ]]; then msg="${HEAD_MSG}"; fi',
' fi',
' echo "stub build FAILED at $head"',
' if [[ "${NO_IDENTITY:-}" == "1" ]]; then',
// 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:-}" && "${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
// restore checkout refuse.
' echo dirt > f.txt',
' fi',
' echo "src/f.ts${pos}: error TS${code}: ${msg}"',
' if [[ "${HUGE_FAIL:-}" == "1" ]]; then',
' for i in $(seq 1 200); do echo "verbose failure context line $i ****************************************"; done',
Expand Down Expand Up @@ -11471,6 +11510,8 @@ describe('review verification gate: baseline A/B on deterministic rejection', ()
EXTRA_ROUND_DIAG: extraRoundDiag ? '1' : '',
EXTRA_BASELINE_DIAG: extraBaselineDiag ? '1' : '',
RESTORE_CLASH: restoreClash ? '1' : '',
NO_IDENTITY: noIdentity ? '1' : '',
TRACKED_DIRT: trackedDirt ? '1' : '',
SCHEMA_FAIL: schemaFail ? '1' : '',
TYPECHECK_FAIL: typecheckFail ? '1' : '',
NOISY_SUCCESS: noisySuccess ? '1' : '',
Expand Down Expand Up @@ -11500,6 +11541,10 @@ describe('review verification gate: baseline A/B on deterministic rejection', ()
expect(r.status).toBe(1);
expect(r.outputs).toContain('outcome=failed');
expect(r.outputs).toContain('retryable=true');
// The repair agent's only warning that dist/ now holds baseline-built
// artifacts — dropped, it burns its budget on phantom dist-consuming
// failures.
expect(r.rejection).toContain('run npm run build before typecheck/tests');
expect(r.outputs).not.toContain('preexisting=true');
// The A/B genuinely ran — the verdict is measured, not assumed.
expect(r.stdout).toContain('Baseline A/B');
Expand Down Expand Up @@ -11537,23 +11582,67 @@ describe('review verification gate: baseline A/B on deterministic rejection', ()
expect(r.stdout).toContain('DIFFERENT reason');
});

it('rejects WITHOUT retry when the baseline leg breaks the restore', () => {
// The baseline run recreates (untracked) a file the branch tracks, so
// `git checkout` back refuses — the tree can no longer be trusted. No
// pre-existing label (a transient git failure is not a verdict about
// the failure's origin) and no retry either: the repair agent works in
// this very checkout and performs no git recovery, so on the detached
// tree its commit would land on the baseline and be orphaned. The next
// round starts clean from the trusted checkout instead.
it('crashes verdict-less when the baseline leg breaks the restore (retry, not handoff)', () => {
// The baseline run recreates (untracked) a file the branch tracks
// (`git restore -- .` touches tracked files only), so the checkout back
// refuses — the tree can no longer be trusted, and the repair must not
// run in it (its commit would orphan on the detached baseline). But a
// transient git-state failure is NOT a verdict either: a plain
// outcome=failed is an EVALUATED rejection — the watermark advances and
// the item is handed off for good. The gate therefore leaves outcome
// UNSET (the report's gate-crashed path retries next scan) while still
// writing the detail document so the crash comment explains itself.
const r = runGate({
failAt: ['feature', 'origin/feature'],
restoreClash: true,
});
expect(r.status).toBe(1);
expect(r.outputs).toContain('outcome=failed');
expect(r.outputs).not.toContain('outcome=');
expect(r.outputs).not.toContain('retryable=true');
expect(r.outputs).not.toContain('preexisting=true');
expect(r.stdout).toContain('could not restore the verification tree');
expect(r.rejection).toContain('could not restore the verification tree');
});

it('short-circuits before the detach when the head has no failure identity', () => {
// vite/esbuild/crash failures carry no tsc diagnostic: an empty head
// signature fails closed REGARDLESS of the baseline, so the gate must
// decide before paying the detach + full baseline re-run + restore.
const r = runGate({ failAt: ['feature'], noIdentity: true });
expect(r.status).toBe(1);
expect(r.outputs).toContain('retryable=true');
expect(r.outputs).not.toContain('preexisting=true');
expect(r.stdout).toContain('no failure identity in the head transcript');
expect(r.stdout).not.toContain('Baseline A/B');
});

it('discards tracked build dirt so a real verdict survives the restore', () => {
// The baseline build REWRITES a tracked file (the settings-schema
// shape): without the pre-checkout `git restore -- .` the restore
// refuses and a clean pre-existing verdict degrades into the
// verdict-less crash.
const r = runGate({
failAt: ['feature', 'origin/feature'],
trackedDirt: true,
});
expect(r.status).toBe(1);
expect(r.outputs).toContain('preexisting=true');
expect(r.stdout).not.toContain('could not restore the verification tree');
});

it('caps the LONG-preamble (pre-existing) rejection under the render window', () => {
// The short-preamble flood is pinned above; the pre-existing path adds
// ~490 bytes of preamble, and the ${#preamble} subtraction is what
// keeps THIS document under the cap — a constant would pass the short
// case and truncate this one's closing fence.
const r = runGate({
failAt: ['feature', 'origin/feature'],
hugeFail: true,
});
expect(r.status).toBe(1);
expect(r.outputs).toContain('preexisting=true');
expect(r.rejection.length).toBeLessThanOrEqual(3900);
expect(r.rejection.endsWith('````\n')).toBe(true);
});

it('keeps the full message past the first n (the bracket class ate it)', () => {
Expand Down
Loading