mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-11 01:36:35 +00:00
#8816's branch accidentally carried #8765's early commits, and the takeover loop evolved the gate further there (subset identity via comm -23, the retryable third arg, subset fixtures) — so #8765 closes as subsumed, and this PR ports what main still lacks: the two improvements its reviewers named for porting, plus the open round-6/7 findings that survive on main's gate. - Pre-detach short-circuit: an empty head signature (vite/esbuild/ crash — the KNOWN LIMIT class) fails closed regardless of the baseline, so decide it BEFORE paying the detach + full baseline re-run + restore. - Build-dirt guard: the A/B'd build REWRITES a tracked file (the vscode companion settings schema), and the undiscarded rewrite makes either checkout refuse — degrading a real verdict into the crash path. `git restore -- .` before both checkouts; tracked-only, and the tree was asserted clean before the deterministic checks. - Restore-failure semantics: a plain outcome=failed is an EVALUATED rejection — the watermark advances and a transient git failure strands the item as a permanent human handoff. The gate now leaves outcome unset (the gate-crashed path retries next scan) and still writes the detail document so the crash comment explains itself. - The dist-rebuilt steering note seeds the repair feedback on both retryable A/B exits — the repair agent's only warning that dist/ holds baseline-built artifacts. - The stale-base retry handoff prefixes its embedded rejection with a the-base-has-moved note, so the retry agent is not steered toward no-action by framing written before the auto-update. - The two A/B side logs joined the repair step's cleanup list. - Tests: identity-less short-circuit, tracked-dirt survival, verdict-less restore crash, long-preamble render cap, PREEXISTING clause selection through the executable report harness, and the stale-framing note pin. Mutation-tested, 5 of 5 caught: short-circuit dropped, restore guards dropped, restore-failure reverted to the evaluated rejection, dist note dropped, stale-framing note dropped. Co-authored-by: verify <verify@local>
This commit is contained in:
Shaojin Wen
GitHub
parent
95e17691a9
commit
5dc98240c7
3 changed files with 152 additions and 17 deletions
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
12
.github/workflows/qwen-autofix.yml
vendored
12
.github/workflows/qwen-autofix.yml
vendored
|
|
@ -4600,6 +4600,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" \
|
||||
|
|
@ -5608,6 +5610,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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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);
|
||||
|
|
@ -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 {
|
||||
|
|
@ -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',
|
||||
|
|
@ -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' : '',
|
||||
|
|
@ -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');
|
||||
|
|
@ -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)', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue