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
82 changes: 76 additions & 6 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2555,6 +2555,27 @@ jobs:
| select((.completedAt // .updatedAt // "") > $wm)
| "- \(((.name // .workflowName) // "external check") | gsub("[^A-Za-z0-9 _./()-]"; "") | .[0:80]): \(.conclusion // .state // "?")"' \
"${WORKDIR}/checks.json"
# If the LAST round ended in a gate rejection, show the agent WHY.
# It is otherwise invisible on the retry: the reason lives in the
# bot's own handoff comment, which the feedback filter above
# (correctly) excludes, so the agent would re-read only the original
# review points and re-make the same mistake - #7208 was handed to a
# human over a two-character TS4111 fix its own compiler output had
# already spelled out.
LAST_REJECTION="$(jq -r --arg ab "${AUTOFIX_BOT}" '
[ .[] | select((.user.login // "") == $ab)
| select((.body // "") | contains("<!-- autofix-eval ts=")) ]
| sort_by(.created_at) | last | .body // ""' "${WORKDIR}/ic.json" \
| sed -n '/<!-- autofix-gate-rejection-start -->/,/<!-- autofix-gate-rejection-end -->/p' \
| sed '1d;$d')"
if [[ -n "${LAST_REJECTION}" ]]; then
echo
echo '## Your previous attempt was REJECTED by the verification gate'
echo
echo 'Fix this first — the same change will be rejected again otherwise.'
echo
printf '%s\n' "${LAST_REJECTION}"
fi
} > "${WORKDIR}/feedback.md"
echo '--- feedback.md ---'
cat "${WORKDIR}/feedback.md"
Expand Down Expand Up @@ -2721,16 +2742,49 @@ jobs:
# on a failed job means the gate never reached a verdict (its own bug,
# an infra blip), and the agent's work must then be retried rather
# than buried by a watermark advance.
# Capture each check's output. A rejection has to tell the agent WHY
# its change was refused: without that, the next round re-reads only
# the original review feedback and re-makes the same mistake - #7208
# was handed to a human over a two-character TS4111 fix its own
# compiler output already spelled out.
GATE_LOG="${WORKDIR}/gate-output.log"
: > "${GATE_LOG}"
reject_fix() {
echo "❌ ${1}"
# Declare the verdict FIRST. The handoff routes on outcome=, and an
# empty outcome on a failed job means "the gate never reached a
# verdict" — i.e. a crash, which is RETRIED. So a rejection that
# dies while writing its detail file would be re-attempted forever
# instead of reported once. A detail we cannot write is a degraded
# message; it must never cost the verdict, hence this order and the
# non-fatal write below.
echo "outcome=failed" >> "${GITHUB_OUTPUT}"
{
echo "**${1}**"
echo
# A four-backtick fence cannot be closed by a ``` line, so
# captured output containing its own fences stays inside the
# block when this is posted verbatim as a PR comment.
echo '````'
tail -c 3000 "${GATE_LOG}" 2> /dev/null
echo '````'
} > "${WORKDIR}/gate-rejection.md" ||
echo "::warning::could not write the gate rejection detail; the verdict stands."
exit 1
}
run_check() {
# pipefail makes the pipeline carry the command's status, not tee's.
local label="${1}"
shift
if ! "$@" 2>&1 | tee -a "${GATE_LOG}"; then
reject_fix "${label}"
fi
}

echo '🔬 Re-running deterministic checks (independent of the agent)...'
npm run build || reject_fix 'build failed on the agent-committed fix'
npm run typecheck || reject_fix 'typecheck failed on the agent-committed fix'
npm run lint || reject_fix 'lint failed on the agent-committed fix'
run_check 'build failed on the agent-committed fix' npm run build
run_check 'typecheck failed on the agent-committed fix' npm run typecheck
run_check 'lint failed on the agent-committed fix' npm run lint

# Test changed/related files for the packages this PR touches.
# --changed follows the import graph so transitive breakage is caught.
Expand Down Expand Up @@ -2761,8 +2815,8 @@ jobs:
continue
fi
echo "🧪 Testing ${p} (changed files only)..."
npm run test --workspace "${p}" --if-present -- --changed origin/main --passWithNoTests \
|| reject_fix "tests failed in ${p}"
run_check "tests failed in ${p}" \
npm run test --workspace "${p}" --if-present -- --changed origin/main --passWithNoTests
done
fi
echo "outcome=fixed" >> "${GITHUB_OUTPUT}"
Expand All @@ -2774,7 +2828,7 @@ jobs:
if git rev-parse --verify "${BRANCH}" > /dev/null 2>&1; then
git diff "origin/main...${BRANCH}" > "${WORKDIR}/pr.diff" || true
fi
for f in feedback.md address-summary.md no-action.md failure.md handoff.md agent-api-error agent-api-error-kind resolved-comments.txt pr.diff; do
for f in feedback.md address-summary.md no-action.md failure.md handoff.md gate-rejection.md agent-api-error agent-api-error-kind resolved-comments.txt pr.diff; do
if [[ -f "${WORKDIR}/${f}" ]]; then
echo "=============== ${f} ==============="
cat "${WORKDIR}/${f}"
Expand Down Expand Up @@ -3191,6 +3245,22 @@ jobs:
else
echo "AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why)."
fi
# Carry the gate's own rejection reason. Without it the comment
# shows only the agent's optimistic summary, so neither a human
# nor the NEXT round can see why the change was refused - the
# agent then re-reads the original review feedback and repeats
# the same mistake. Delimited so `Prepare branch and feedback`
# can lift it back out on the retry.
if [[ -s "${WORKDIR}/gate-rejection.md" ]]; then
echo
echo '<!-- autofix-gate-rejection-start -->'
echo "**Why it was not pushed:**"
echo
# Must stay >= reject_fix's tail -c 3000 + label + two four-backtick
# fences (~3.1 KB), or the closing fence is silently truncated.
head -c 3500 "${WORKDIR}/gate-rejection.md" | iconv -f utf-8 -t utf-8 -c | sed 's/<!--/<!\\-\\-/g' || true
echo '<!-- autofix-gate-rejection-end -->'
fi
echo
echo
echo "Run log: ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}"
Expand Down
179 changes: 172 additions & 7 deletions scripts/tests/qwen-autofix-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3413,13 +3413,22 @@ describe('qwen-autofix workflow', () => {
'::warning::Failed to post handoff comment on PR #${PR}',
);
expect(reviewAddressReportStep).toContain('human should take over');
// Token-breaking neutralization at ALL FOUR agent-derived publish sites
// (the three model-output bodies + the API_ERROR_DETAIL headline, whose
// Token-breaking neutralization at ALL FIVE agent-derived publish sites
// (address-summary, no-action, DETAIL_FILE, API_ERROR_DETAIL, and the
// gate-rejection body, whose
// content is agent stdout that can echo external comment text), and it
// must be LINE-INDEPENDENT: a whole-comment strip misses a marker whose
// --> sits on another line, while jq scan() matches across newlines.
// Proven end-to-end on a split forged marker.
expect(workflow.split("sed 's/<!--/<!\\\\-\\\\-/g'").length - 1).toBe(4);
// Count the correct spelling AND prove no site uses a different one.
// Counting alone is not enough: a fifth site added with `\-\-` (single
// backslashes — a NO-OP on both GNU and BSD sed, verified) left the count
// at four and this test green, shipping an unescaped publish site.
const escapeSites = workflow.match(/sed 's\/<!--\/[^']*\/g'/g) ?? [];
expect(escapeSites).toHaveLength(5);
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
for (const site of escapeSites) {
expect(site).toBe("sed 's/<!--/<!\\\\-\\\\-/g'");
}
const forged =
'<!-- autofix-eval ts=2099-01-01T00:00:00Z\nx acted=true round=99 -->';
const sedCmd = workflow.match(/sed 's\/<!--\/[^']*\/g'/)?.[0];
Expand Down Expand Up @@ -3680,11 +3689,14 @@ describe('qwen-autofix workflow', () => {
// writes outcome=failed; an unwired check would read as a gate crash and be
// retried instead of reported. Drive the extracted helper for real.
const gate = verificationGateSteps[1];
// Each check runs through run_check, which tees its output to GATE_LOG and
// calls reject_fix on failure - so the verdict is declared AND the reason
// is captured for the retry.
for (const check of [
"npm run build || reject_fix 'build failed on the agent-committed fix'",
"npm run typecheck || reject_fix 'typecheck failed on the agent-committed fix'",
"npm run lint || reject_fix 'lint failed on the agent-committed fix'",
'reject_fix "tests failed in ${p}"',
"run_check 'build failed on the agent-committed fix' npm run build",
"run_check 'typecheck failed on the agent-committed fix' npm run typecheck",
"run_check 'lint failed on the agent-committed fix' npm run lint",
'run_check "tests failed in ${p}"',
]) {
expect(gate).toContain(check);
}
Expand All @@ -3705,6 +3717,46 @@ describe('qwen-autofix workflow', () => {
}
expect(status).not.toBe(0);
expect(readFileSync(out, 'utf8')).toContain('outcome=failed');
// The verdict must be declared BEFORE the detail file is written, and the
// write must be non-fatal. An empty outcome on a failed job reads as "the
// gate never reached a verdict" — a CRASH, which is retried — so a
// rejection that died writing its detail would be re-attempted forever
// instead of reported once. Drive it with an unwritable WORKDIR: the
// detail is lost, the verdict is not.
//
// The ordering is asserted STATICALLY as the primary guard, because the
// behavioural half is not portable: bash 3.2 suspends set -e through a
// `||`-invoked function and bash 5 does not, so the wrong order runs
// clean on macOS and aborts on a Linux runner. That is precisely how this
// shipped green locally and red in CI, so the guard must not depend on
// which bash the reviewer happens to have.
expect(helper.indexOf('outcome=failed')).toBeLessThan(
helper.indexOf('gate-rejection.md'),
);
// ...and the detail write is non-fatal, so it cannot abort before exit 1.
expect(helper).toMatch(/gate-rejection\.md" \|\|\n/);
const outNoDir = join(dir, 'gh_output_nodir');
writeFileSync(outNoDir, '');
let degraded = 0;
try {
execFileSync(
'bash',
['-c', `set -eo pipefail\n${helper}\nfalse || reject_fix 'boom'`],
{
env: {
...process.env,
GITHUB_OUTPUT: outNoDir,
WORKDIR: join(dir, 'does', 'not', 'exist'),
},
encoding: 'utf8',
stdio: 'pipe',
},
);
} catch (e) {
degraded = e.status;
}
expect(degraded).not.toBe(0);
expect(readFileSync(outNoDir, 'utf8')).toContain('outcome=failed');
rmSync(dir, { recursive: true, force: true });
});

Expand Down Expand Up @@ -4000,6 +4052,119 @@ describe('qwen-autofix workflow', () => {
expect(failed.stdout).toContain('Bad credentials');
});

it('feeds the gate rejection back so the retry can fix what it broke', () => {
// #7208 was handed to a human over a two-character TS4111 error its own
// compiler output already spelled out: the gate rejected the commit, the
// handoff showed only the agent's optimistic summary, and the next round
// re-read the original review points with no idea why it had been refused.
const gate = verificationGateSteps[1];
const prep =
workflow.match(
/- name: 'Prepare branch and feedback'[\s\S]*?(?=\n {6}- name: )/,
)?.[0] ?? '';

// 1. A failing check records WHY, not just THAT, it failed.
const capture = gate.match(
/GATE_LOG="\$\{WORKDIR\}\/gate-output\.log"[\s\S]*?\n {10}\}\n {10}run_check\(\) \{[\s\S]*?\n {10}\}/,
)?.[0];
expect(capture).toBeTruthy();
const dir = mkdtempSync(join(tmpdir(), 'gate-'));
const out = join(dir, 'gh_output');
writeFileSync(out, '');
let status = 0;
try {
execFileSync(
'bash',
[
'-c',
[
'set -eo pipefail',
`WORKDIR=${JSON.stringify(dir)}`,
capture,
"run_check 'build failed on the agent-committed fix' bash -c \"echo 'src/goals/goalJudge.ts(364,13): error TS4111'; exit 1\"",
].join('\n'),
],
{ env: { ...process.env, GITHUB_OUTPUT: out }, encoding: 'utf8' },
);
} catch (e) {
status = e.status;
}
expect(status).not.toBe(0);
expect(readFileSync(out, 'utf8')).toContain('outcome=failed');
const rejection = readFileSync(join(dir, 'gate-rejection.md'), 'utf8');
expect(rejection).toContain('build failed on the agent-committed fix');
// The compiler's own words must survive - that is the whole point.
expect(rejection).toContain('error TS4111');
// A four-backtick fence cannot be closed by captured ``` output.
expect(rejection).toContain('````');

// 2. The handoff delimits it so the retry can lift it back out.
expect(reviewAddressReportStep).toContain(
'<!-- autofix-gate-rejection-start -->',
);
expect(reviewAddressReportStep).toContain(
'<!-- autofix-gate-rejection-end -->',
);

// 3. Round-trip: the prepare step recovers it from the bot's newest comment.
const extract = prep.match(
/LAST_REJECTION="\$\(jq[\s\S]*?\n {14}\| sed '1d;\$d'\)"/,
)?.[0];
expect(extract).toBeTruthy();
const runExtract = (comments) => {
const d = mkdtempSync(join(tmpdir(), 'fb-'));
writeFileSync(join(d, 'ic.json'), JSON.stringify(comments));
const res = execFileSync(
'bash',
[
'-c',
[
'set -uo pipefail',
`WORKDIR=${JSON.stringify(d)}`,
'AUTOFIX_BOT=qwen-code-dev-bot',
extract,
'printf "%s" "${LAST_REJECTION}"',
].join('\n'),
],
{ encoding: 'utf8' },
);
rmSync(d, { recursive: true, force: true });
return res;
};
const withRejection = [
{
user: { login: 'qwen-code-dev-bot' },
created_at: '2026-07-20T10:00:00Z',
body: 'old <!-- autofix-eval ts=1 acted=true round=1 -->',
},
{
user: { login: 'qwen-code-dev-bot' },
created_at: '2026-07-20T19:32:00Z',
body: [
'handoff',
'<!-- autofix-gate-rejection-start -->',
'**build failed on the agent-committed fix**',
"error TS4111: Property must be accessed with ['truncated']",
'<!-- autofix-gate-rejection-end -->',
'<!-- autofix-eval ts=2 acted=false round=5 -->',
].join('\n'),
},
];
const recovered = runExtract(withRejection);
expect(recovered).toContain('error TS4111');
expect(recovered).not.toContain('autofix-gate-rejection'); // markers stripped
// A round that pushed carries no rejection - nothing to replay.
expect(
runExtract([
{
user: { login: 'qwen-code-dev-bot' },
created_at: '2026-07-20T19:32:00Z',
body: 'pushed <!-- autofix-eval ts=2 acted=true round=5 -->',
},
]).trim(),
).toBe('');
});

it('resolves only the review threads whose findings it implemented', () => {
// A human re-reviewing should see what is still OPEN, not re-read every
// thread to work out what the bot handled. The agent cannot resolve threads
Expand Down
Loading