Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
55 changes: 51 additions & 4 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,12 @@ jobs:
DRY_RUN: '${{ needs.route.outputs.dry_run }}'
OUTCOME: '${{ steps.verify.outputs.outcome }}'
run: |-
# The head the reds were evaluated against. Taken from the REMOTE,
# not local HEAD: after a rejected push the two differ, and
# recording a sha that never landed would suppress the still-red
# checks on the head that actually exists. Empty on failure, which
# matches no marker and therefore keeps them visible — fail-open.
REPORT_HEAD="$(gh api "repos/${REPO}/pulls/${PR}" --jq '.head.sha // ""' 2> /dev/null || echo '')"
SUFFIX=''
[[ "${DRY_RUN}" == "true" ]] && SUFFIX=' (dry-run, nothing pushed)'
{
Expand Down Expand Up @@ -1744,7 +1750,7 @@ jobs:
# (the watermark floor below), and labels (the effective round
# cap) — avoids extra round-trips per candidate PR.
PR_META="$(gh pr view "${PR}" --repo "${REPO}" \
--json headRefName,statusCheckRollup,createdAt,labels,isCrossRepository,headRepositoryOwner,headRepository 2> /dev/null || echo '{}')"
--json headRefName,headRefOid,statusCheckRollup,createdAt,labels,isCrossRepository,headRepositoryOwner,headRepository 2> /dev/null || echo '{}')"
HEAD_REPO_FULL="${REPO}"
if [[ "$(jq -r '.isCrossRepository // false' <<< "${PR_META}")" == "true" ]]; then
HR_OWNER="$(jq -r '.headRepositoryOwner.login // ""' <<< "${PR_META}")"
Expand Down Expand Up @@ -1902,6 +1908,16 @@ jobs:
[ .[] | select((.user.login // "") == $ab)
| select((.body // "") | contains("<!-- autofix-rearm -->"))
| .created_at ] | max // ""' "${WORKDIR}/ic.json")"
# The head this PR's red checks were last reported against. Carried
# as its OWN marker inside the eval comment rather than a new field
# on autofix-eval, so none of the ts/acted/round parsers change —
# and the comment still matches the eval filter, so the agent never
# sees it as feedback.
RED_HEAD="$(jq -r --arg ab "${AUTOFIX_BOT}" '
[ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "")
| [ scan("<!-- autofix-redcheck 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 @@ -1995,6 +2011,28 @@ jobs:
| select((.completedAt // .updatedAt // "") > $wm) ]
| length
' <<< "${CHECKS_JSON}")"
# A red check is a persistent STATE, not the instant it turned red.
# Counting only "failed since the watermark" made a still-failing PR
# invisible the moment the watermark passed the failure: measured on
# #6451 (3 reds, all completed 09:30-09:51, watermark 10:55),
# #7357 (red 07:59, watermark 09:18) and #7390 (red and watermark
# both 11:27:37, so a strict `>` hid it the instant it appeared) —
# all three sat red for hours while every scan logged "nothing new".
#
# So: a currently-red check counts as feedback until the head it ran
# against has been evaluated. The address job records the head it
# reported on; a PR whose recorded head still matches is left alone,
# which bounds this to ONE look per head instead of every scan.
LIVE_HEAD="$(jq -r '.headRefOid // ""' <<< "${PR_META}")"
N_RED_NOW=0
if [[ -n "${LIVE_HEAD}" && "${RED_HEAD}" != "${LIVE_HEAD}" ]]; then
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
Resolved
N_RED_NOW="$(jq '
[ .[]
| select((.conclusion // .state // "") | IN("FAILURE", "FAILED", "ERROR", "TIMED_OUT", "ACTION_REQUIRED"))
| select(((.workflowName // "") != "Qwen Autofix") or (((.name // "") | startswith("review-address")))) ]
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
Resolved
| length
' <<< "${CHECKS_JSON}")"
fi

gh api "repos/${REPO}/pulls/${PR}/reviews" --paginate > "${WORKDIR}/rv.json"
gh api "repos/${REPO}/pulls/${PR}/comments" --paginate > "${WORKDIR}/rc.json"
Expand Down Expand Up @@ -2049,14 +2087,14 @@ jobs:
HAS_CONFLICT='false'
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 && "${HAS_CONFLICT}" != "true" ]]; then
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
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
fi

echo "🔎 #${PR}: ${N_REVIEWS} review(s) + ${N_COMMENTS} inline + ${N_ISSUE_COMMENTS} issue comment(s) + ${N_FAILED_CHECKS} failed check(s) new, conflict=${HAS_CONFLICT}, round=${ROUND}"
fleet_row "${PR}" 'SELECTED' "${N_REVIEWS} review + ${N_COMMENTS} inline + ${N_ISSUE_COMMENTS} comment + ${N_FAILED_CHECKS} failed-check new (round ${ROUND}/${EFF_MAX_ROUNDS}, conflict=${HAS_CONFLICT})"
echo "🔎 #${PR}: ${N_REVIEWS} review(s) + ${N_COMMENTS} inline + ${N_ISSUE_COMMENTS} issue comment(s) + ${N_FAILED_CHECKS} failed check(s) new + ${N_RED_NOW} still-red on ${LIVE_HEAD:0:9}, conflict=${HAS_CONFLICT}, round=${ROUND}"
fleet_row "${PR}" 'SELECTED' "${N_REVIEWS} review + ${N_COMMENTS} inline + ${N_ISSUE_COMMENTS} comment + ${N_FAILED_CHECKS} failed-check new + ${N_RED_NOW} still-red (round ${ROUND}/${EFF_MAX_ROUNDS}, conflict=${HAS_CONFLICT})"
TARGETS="$(jq -c \
--arg pr "${PR}" --arg branch "${BRANCH}" --arg issue "${ISSUE}" \
--arg round "${ROUND}" --arg wm "${EFF_WM}" --arg mr "${EFF_MAX_ROUNDS}" \
Expand Down Expand Up @@ -2906,6 +2944,12 @@ jobs:
# variable (not a secret), already the agent's OPENAI_MODEL.
MODEL: '${{ vars.QWEN_AUTOFIX_MODEL || vars.QWEN_PR_REVIEW_MODEL }}'
run: |-
# The head the reds were evaluated against. Taken from the REMOTE,
# not local HEAD: after a rejected push the two differ, and
# recording a sha that never landed would suppress the still-red
# checks on the head that actually exists. Empty on failure, which
# matches no marker and therefore keeps them visible — fail-open.
REPORT_HEAD="$(gh api "repos/${REPO}/pulls/${PR}" --jq '.head.sha // ""' 2> /dev/null || echo '')"
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
Resolved
# Prepare may have adopted a sibling's live round; the matrix value
# would double-write that round's marker.
ROUND="${EFFECTIVE_ROUND:-${ROUND}}"
Expand Down Expand Up @@ -3011,6 +3055,7 @@ jobs:
echo "🧠 Handled by **Qwen Code** · model/模型 \`${MODEL_DISPLAY}\`"
echo
echo "<!-- autofix-eval ts=${NEWEST} acted=true round=${NEXT_ROUND} win=${WINDOW:-none} -->"
echo "<!-- autofix-redcheck head=${REPORT_HEAD} -->"
} > "${WORKDIR}/report.md"
STATUS="pushed (round ${NEXT_ROUND}/${MAX_ROUNDS})"
else
Expand All @@ -3027,6 +3072,7 @@ jobs:
echo "🧠 Handled by **Qwen Code** · model/模型 \`${MODEL_DISPLAY}\`"
echo
echo "<!-- autofix-eval ts=${NEWEST} acted=false round=${ROUND} win=${WINDOW:-none} -->"
echo "<!-- autofix-redcheck head=${REPORT_HEAD} -->"
} > "${WORKDIR}/report.md"
STATUS="no action needed"
fi
Expand Down Expand Up @@ -3313,6 +3359,7 @@ jobs:
echo "🧠 Handled by **Qwen Code** · model/模型 \`${MODEL_DISPLAY}\`"
echo
echo "<!-- autofix-eval ts=${MARK_TS} acted=false round=${MARK_ROUND} win=${WINDOW:-none} -->"
echo "<!-- autofix-redcheck head=${REPORT_HEAD} -->"
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
Resolved
} > "${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
83 changes: 81 additions & 2 deletions scripts/tests/qwen-autofix-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,16 @@ describe('qwen-autofix workflow', () => {
expect(reviewScanJob).toContain('.conclusion // .state // ""');
expect(reviewScanJob).toContain('.workflowName // ""');
expect(reviewScanJob).toContain('startswith("review-address")');
// Every failed-check selector must carry the address-check carve-out, or
// the loop reads its OWN runs as feedback about the PR. Asserting the
// property rather than the count lets a new selector be added, but not
// one that forgets the carve-out.
const scanCheckSelectors =
reviewScanJob.match(/IN\("(?:FAILURE|QUEUED)"/g) ?? [];
expect(scanCheckSelectors.length).toBeGreaterThanOrEqual(3);
expect(
reviewScanJob.match(/startswith\("review-address"\)/g) ?? [],
).toHaveLength(2);
).toHaveLength(scanCheckSelectors.length);
expect(reviewScanJob).toContain('"${N_FAILED_CHECKS}" -eq 0');
expect(reviewScanJob).toContain('${N_FAILED_CHECKS} failed check(s) new');
expect(reviewScanJob).toContain('.completedAt // .updatedAt // ""');
Expand Down Expand Up @@ -394,6 +401,78 @@ describe('qwen-autofix workflow', () => {
expect(run([{ ...llm, name: 'resolve-pr' }])).toBe('true');
});

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
// moment the watermark passed the failure. Measured: #6451 (3 reds
// completed 09:30-09:51, watermark 10:55), #7357 (red 07:59, watermark
// 09:18), #7390 (red and watermark BOTH 11:27:37, so a strict `>` hid it
// the instant it appeared) — all three sat red for hours while every scan
// logged "nothing new".
const block = reviewScanJob.match(
/LIVE_HEAD="\$\(jq -r[\s\S]*?\n {12}fi\n/,
)?.[0];
expect(block).toBeTruthy();
const script = block.replace(/^ {12}/gm, '');
const run = (reds, redHead, liveHead) =>
execFileSync(
'bash',
['-c', `set -uo pipefail\n${script}\nprintf '%s' "$N_RED_NOW"`],
{
env: {
...process.env,
PR_META: JSON.stringify({ headRefOid: liveHead }),
CHECKS_JSON: JSON.stringify(
reds.map((name) => ({
name,
conclusion: 'FAILURE',
workflowName: 'CI',
})),
),
RED_HEAD: redHead,
},
encoding: 'utf8',
},
);

// Never evaluated: the red is visible however old it is.
expect(run(['Test'], '', 'abc123')).toBe('1');
// Already evaluated on THIS head: left alone. This is what bounds it to
// one look per head instead of re-selecting the PR every single scan.
expect(run(['Test'], 'abc123', 'abc123')).toBe('0');
// A new commit re-opens it — the reds now belong to a head nobody judged.
expect(run(['Test'], 'old999', 'abc123')).toBe('1');
// Green stays green.
expect(run([], '', 'abc123')).toBe('0');
expect(run(['a', 'b', 'c'], '', 'abc123')).toBe('3');

// The count must actually GATE selection — computing it and then not
// consulting it is the whole bug, and every other assertion here still
// passes without this line.
const idleGate = reviewScanJob.match(
/if \[\[ "\$\{N_REVIEWS\}" -eq 0[^\n]*\]\]; then/,
)?.[0];
expect(idleGate).toBeTruthy();
expect(idleGate).toContain('"${N_RED_NOW}" -eq 0');

// The head is recorded by its OWN marker inside the eval comment, so no
// ts/acted/round parser changes — and the comment still matches the eval
// filter, so the agent never sees it as feedback.
expect(reviewScanJob).toContain('autofix-redcheck head=([0-9a-f]+)');
expect(
workflow.match(/<!-- autofix-redcheck head=\$\{REPORT_HEAD\} -->/g) ?? [],
).toHaveLength(3);
// Taken from the REMOTE head: after a rejected push local HEAD is ahead of
// what landed, and recording a sha that never existed would suppress the
// reds on the head that does. Empty on failure — matches no marker, so the
// reds stay visible.
expect(
workflow.match(/REPORT_HEAD="\$\(gh api "repos\/\$\{REPO\}\/pulls/g) ??
[],
).toHaveLength(2);
expect(workflow).not.toContain('REPORT_HEAD="$(git rev-parse HEAD)"');
});

it('bounds fleet-wide simultaneity below the per-scan target budget', () => {
// max-parallel is the ONE place different PRs wait on each other: the scan
// emits every eligible PR (up to MAX_TARGETS_PER_SCAN) and the matrix
Expand Down Expand Up @@ -1379,7 +1458,7 @@ describe('qwen-autofix workflow', () => {
).length - 1,
).toBe(2);
expect(reviewScanJob).toContain(
'--json headRefName,statusCheckRollup,createdAt,labels',
'--json headRefName,headRefOid,statusCheckRollup,createdAt,labels',
);
// Command-style comments are instructions, not feedback — excluded at
// ALL FOUR feedback sites (scan count via $cf; NEWEST, LIVE_NEW, and
Expand Down
Loading