Skip to content

revert: drop the stale-base un-park recovery (#7602) #7640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 1 commit into from
Jul 26, 2026
Merged

revert: drop the stale-base un-park recovery (#7602) #7640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 0 additions & 49 deletions .github/workflows/qwen-autofix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2044,16 +2044,6 @@ jobs:
| [ scan("<!-- autofix-redcheck head=([0-9a-f]+) -->") ] | .[]
| {sha: .[0], at: ($c.created_at // "")} ]
| sort_by(.at) | last | .sha // ""' "${WORKDIR}/ic.json")"
# The head a gate-rejection handoff last parked this PR on (mirror of
# RED_HEAD). While it equals LIVE_HEAD the PR is parked on the CURRENT
# head: a later push writes a newer eval comment WITHOUT this marker,
# so the latest handoff head stops matching and the parking clears.
# Used only to auto-recover a stale-base parking (see the idle path).
HANDOFF_HEAD="$(jq -r --arg ab "${AUTOFIX_BOT}" '
[ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "")
| [ scan("<!-- autofix-handoff head=([0-9a-f]+) -->") ] | .[]
| {sha: .[0], at: ($c.created_at // "")} ]
| sort_by(.at) | last | .sha // ""' "${WORKDIR}/ic.json")"
# A maintainer '@qwen-code /retry' posts that marker to say
# "evaluate this feedback again": markers written BEFORE it stop
# holding the watermark down. This is the sanctioned exception to
Expand Down Expand Up @@ -2310,27 +2300,6 @@ jobs:
if [[ "${MERGEABLE}" == "CONFLICTING" ]]; then HAS_CONFLICT='true'; fi

if [[ "${N_REVIEWS}" -eq 0 && "${N_COMMENTS}" -eq 0 && "${N_ISSUE_COMMENTS}" -eq 0 && "${N_FAILED_CHECKS}" -eq 0 && "${N_RED_NOW}" -eq 0 && "${HAS_CONFLICT}" != "true" ]]; then
# Nothing new — but is this a stale-base PARKING? A gate-rejection
# handoff left an autofix-handoff marker on the head it parked on.
# While that still matches LIVE_HEAD (no push since) and the head is
# now BEHIND main, the build that failed may have failed on a
# since-changed dependency (#7471), not the fix. Merge main in and
# re-arm so the loop re-reads the feedback against fresh main. The
# forward case is handled in-round by the report step; this recovers
# a PR that was ALREADY parked when the base moved under it. Guarded
# by head-match (a push clears it) and behind-main (self-limiting:
# the update makes it current, so it cannot re-fire), and every API
# call is fail-safe — any failure just falls through to idle.
if [[ -n "${LIVE_HEAD}" && "${HANDOFF_HEAD}" == "${LIVE_HEAD}" ]]; then
UNPARK_CMP="$(gh api "repos/${REPO}/compare/${DEFAULT_BRANCH:-main}...${LIVE_HEAD}" --jq '.status' 2> /dev/null || echo '')"
if [[ "${UNPARK_CMP}" == 'behind' || "${UNPARK_CMP}" == 'diverged' ]] \
&& gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${LIVE_HEAD}" > /dev/null 2>&1; then
gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '♻️ AutoFix auto-recovered a stale base: this PR was parked after a fix failed to build, but its head was behind `%s`, so current main was merged in via update-branch and the next scan will re-read the feedback against it. A stale base can fail the build without being the fix at fault; if it still fails once current, it hands off to a human again.\n\n<details>\n<summary>中文说明</summary>\n\n♻️ AutoFix 自动恢复了陈旧 base:本 PR 因修复构建失败而被搁置,但其 head 落后于 `%s`,故已将当前 main 合入,下次扫描会对它重读反馈。陈旧 base 可能使构建失败而并非修复之过;若更新后仍失败,将再次交人。\n\n</details>\n\n<!-- autofix-rearm -->' "${DEFAULT_BRANCH:-main}" "${DEFAULT_BRANCH:-main}")" || echo "::warning::Failed to post stale-base recovery comment on PR #${PR}"
echo "♻️ #${PR}: parked on a stale base (behind ${DEFAULT_BRANCH:-main}) — merged main + re-armed"
fleet_row "${PR}" 'unparked' "stale-base parking recovered — merged ${DEFAULT_BRANCH:-main} + re-armed"
continue
fi
fi
echo "✅ #${PR}: nothing new since ${EFF_WM} (conflict=${HAS_CONFLICT})"
fleet_row "${PR}" 'idle' "nothing new since ${EFF_WM} (round ${ROUND}/${EFF_MAX_ROUNDS}, conflict=${HAS_CONFLICT})"
continue
Expand Down Expand Up @@ -3633,12 +3602,6 @@ jobs:
HEADLINE="🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind \`${DEFAULT_BRANCH:-main}\`, so it merged current main in via update-branch and will retry on the next scan. A stale base (a dependency or symbol main already changed) can fail the build without being the fix's fault; if it still fails once current, it hands off to a human."
else
HEADLINE="🤖 Could not address the latest feedback automatically (round ${MARK_ROUND}/${MAX_ROUNDS}). A human should take over this PR."
# The gate rejected a real fix and the PR is up to date now, so
# this is a genuine parking. Flag it head-scoped so a LATER
# scan can auto-recover it if the base goes stale under it
# (behind main → the build may have failed on a since-changed
# dependency, not the fix). See the autofix-handoff marker.
PARKED_STALE_CANDIDATE=true
fi
fi
elif [[ "${PREPARE_OUTCOME}" != 'success' && "${PREPARE_OUTCOME}" != 'failure' ]]; then
Expand Down Expand Up @@ -3799,18 +3762,6 @@ jobs:
if [[ "${MARK_TS}" != '9999-12-31T23:59:59Z' ]]; then
echo "<!-- autofix-redcheck head=${REPORT_HEAD} -->"
fi
# A gate-REJECTION handoff (the agent produced a fix that failed
# verification, so the PR is now parked with the watermark
# advanced) is a stale-base recovery candidate: if this head later
# falls behind main, the build may have failed on something main
# has since changed (#7471's update-notifier), not the fix. The
# scan reads this head-scoped marker and, only while it still
# matches LIVE_HEAD (a later push clears it), auto-updates the base
# and re-arms. Head-scoped like autofix-redcheck, and only on this
# branch — a crash/timeout handoff produced no fix to re-verify.
if [[ "${PARKED_STALE_CANDIDATE:-false}" == 'true' ]]; then
echo "<!-- autofix-handoff head=${REPORT_HEAD} -->"
fi
} > "${WORKDIR}/report.md"
gh pr comment "${PR}" --repo "${REPO}" --body-file "${WORKDIR}/report.md" || echo "::warning::Failed to post handoff comment on PR #${PR}"
fi
196 changes: 0 additions & 196 deletions scripts/tests/qwen-autofix-workflow.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,202 +1032,6 @@ describe('qwen-autofix workflow', () => {
// budget is tight for this many cases, so give it a comfortable margin.
}, 20000);

it('auto-recovers a PR parked on a stale base: handoff head matches + behind main → update-branch + re-arm', () => {
const block = reviewScanJob.match(
/( {12}if \[\[ "\$\{N_REVIEWS\}" -eq 0[\s\S]*?\n {12}fi)\n\n {12}echo "🔎/,
)?.[1];
expect(block).toBeTruthy();
const script = block.replace(/^ {12}/gm, '');

const run = ({
handoffHead = 'live1',
liveHead = 'live1',
cmp = 'behind',
updateOk = true,
}) => {
const dir = mkdtempSync(join(tmpdir(), 'unpark-'));
const bin = join(dir, 'bin');
mkdirSync(bin);
writeFileSync(
join(bin, 'gh'),
[
'#!/usr/bin/env bash',
`echo "$*" >> ${JSON.stringify(join(dir, 'calls.log'))}`,
'for a in "$@"; do case "$a" in',
` */compare/*) printf '%s' '${cmp}'; exit 0;;`,
` */update-branch) exit ${updateOk ? 0 : 1};;`,
'esac; done',
'exit 0',
].join('\n'),
);
chmodSync(join(bin, 'gh'), 0o755);
const out = execFileSync(
'bash',
[
'-c',
`set -uo pipefail\nfleet_row(){ :; }\nfor _ in x; do\n${script}\nprintf 'FELL_THROUGH'\ndone`,
],
{
env: {
...process.env,
PATH: `${bin}:${process.env.PATH}`,
REPO: 'o/r',
PR: '1',
N_REVIEWS: '0',
N_COMMENTS: '0',
N_ISSUE_COMMENTS: '0',
N_FAILED_CHECKS: '0',
N_RED_NOW: '0',
HAS_CONFLICT: 'false',
LIVE_HEAD: liveHead,
HANDOFF_HEAD: handoffHead,
EFF_WM: 'wm',
ROUND: '1',
EFF_MAX_ROUNDS: '100',
},
encoding: 'utf8',
},
);
const calls = existsSync(join(dir, 'calls.log'))
? readFileSync(join(dir, 'calls.log'), 'utf8')
: '';
rmSync(dir, { recursive: true, force: true });
return {
unparked: /update-branch/.test(calls) && /autofix-rearm/.test(calls),
idle: out.includes('nothing new'),
};
};

// Parked on the CURRENT head (handoff head == live) and behind main → merge
// current main in and re-arm; the loop re-reads the feedback next scan.
expect(run({ cmp: 'behind' })).toEqual({ unparked: true, idle: false });
// 'diverged' (ahead AND behind) also merges main in.
expect(run({ cmp: 'diverged' })).toEqual({ unparked: true, idle: false });
// Up to date ('ahead') → not a stale base; a genuine parking stays for a
// human, no update, no re-arm.
expect(run({ cmp: 'ahead' })).toEqual({ unparked: false, idle: true });
// Handoff head != live head (a push landed after the parking) → the parking
// is cleared; do nothing (no compare-driven update).
expect(run({ handoffHead: 'old0', liveHead: 'live1' })).toEqual({
unparked: false,
idle: true,
});
// No handoff marker at all (a healthy "no changes needed" idle) → idle.
expect(run({ handoffHead: '', liveHead: 'live1' })).toEqual({
unparked: false,
idle: true,
});
// Behind but update-branch FAILS (PAT scope, or a race) → fail-safe: fall
// through to idle, never re-arm on an un-updated base.
expect(run({ cmp: 'behind', updateOk: false })).toEqual({
unparked: false,
idle: true,
});
// Both heads empty (API failures on both sides) → the -n guard must
// short-circuit; without this, "" == "" would enter the unpark block.
expect(run({ handoffHead: '', liveHead: '' })).toEqual({
unparked: false,
idle: true,
});
}, 20000);

it('marks a gate-rejection parking with a head-scoped handoff marker', () => {
// The could-not-address branch (a real fix rejected, PR up to date now) sets
// the flag; a crash/timeout handoff (no fix produced) must not.
expect(reviewAddressReportStep).toMatch(
/A human should take over this PR\."\n[\s\S]{0,500}?PARKED_STALE_CANDIDATE=true/,
);
// The report emits the head-scoped marker ONLY under that flag, so a
// crash/timeout parking is never treated as a stale-base candidate.
expect(reviewAddressReportStep).toMatch(
/if \[\[ "\$\{PARKED_STALE_CANDIDATE:-false\}" == 'true' \]\]; then\n\s*echo "<!-- autofix-handoff head=\$\{REPORT_HEAD\} -->"/,
);
// The scan parses HANDOFF_HEAD head-scoped, exactly like RED_HEAD.
expect(reviewScanJob).toContain(
'scan("<!-- autofix-handoff head=([0-9a-f]+) -->")',
);
});

it('parses HANDOFF_HEAD from issue comments: latest bot marker wins, non-bot and markerless ignored', () => {
const assignment = reviewScanJob.match(
/ {12}HANDOFF_HEAD="\$\(jq -r[\s\S]*?ic\.json"\)"\n/,
)?.[0];
expect(assignment).toBeTruthy();
const script = assignment.replace(/^ {12}/gm, '');

const run = (comments) => {
const dir = mkdtempSync(join(tmpdir(), 'handoff-jq-'));
writeFileSync(join(dir, 'ic.json'), JSON.stringify(comments));
const out = execFileSync(
'bash',
[
'-c',
`set -uo pipefail\nAUTOFIX_BOT='bot'\nWORKDIR=${JSON.stringify(dir)}\n${script}\nprintf '%s' "$HANDOFF_HEAD"`,
],
{ encoding: 'utf8' },
);
rmSync(dir, { recursive: true, force: true });
return out;
};

// No comments → empty.
expect(run([])).toBe('');
// Bot comment with a handoff marker → the sha.
expect(
run([
{
user: { login: 'bot' },
body: 'handoff <!-- autofix-handoff head=abc123 -->',
created_at: '2026-01-01T00:00:00Z',
},
]),
).toBe('abc123');
// Non-bot comment with the same marker → ignored.
expect(
run([
{
user: { login: 'human' },
body: '<!-- autofix-handoff head=abc123 -->',
created_at: '2026-01-01T00:00:00Z',
},
]),
).toBe('');
// Multiple bot markers → latest by created_at wins.
expect(
run([
{
user: { login: 'bot' },
body: '<!-- autofix-handoff head=aaa111 -->',
created_at: '2026-01-01T00:00:00Z',
},
{
user: { login: 'bot' },
body: '<!-- autofix-handoff head=bbb222 -->',
created_at: '2026-01-02T00:00:00Z',
},
]),
).toBe('bbb222');
// Bot comment without the marker → empty.
expect(
run([
{
user: { login: 'bot' },
body: 'just a regular comment',
created_at: '2026-01-01T00:00:00Z',
},
]),
).toBe('');
// Missing user.login field → no crash, empty.
expect(
run([
{
body: '<!-- autofix-handoff head=abc123 -->',
created_at: '2026-01-01T00:00:00Z',
},
]),
).toBe('');
});

it('keeps a still-red check visible, but only once per head', () => {
// A red check is a STATE, not the instant it turned red. Counting only
// "failed since the watermark" made a still-failing PR invisible the
Expand Down
Loading