`@qwen-code /triage` runs the agent as one long workflow step, so the PR
thread stays silent until the first stage comment lands — the maintainer
can't tell it started or how far along it is. The agent's output already
streams live to the Actions log; the run link was only surfaced at the end.
Post a `stage=status` comment up front carrying that live run link, and
finalize the same comment (by marker, so a re-run reuses one comment) to a
terminal state at the end. Covers manual, auto (pull_request_target), and
dispatch triage. Best-effort — a failed status post never fails triage.
Co-authored-by: wenshao <wenshao@example.com>
The address-review classification had only two dispositions — fix, or
decline-with-reason — so when a finding turned on a judgment that is the
maintainer's to make (a v1 tradeoff, two reviewers wanting opposite
things, whether the problem is worth solving at all), the agent was
forced to either quietly implement one contested direction or decline it
as "out of scope" — both of which ARE deciding.
Add a third disposition: escalate. The agent names the decision, gives
the options and its recommendation, and leaves the thread unresolved so
the maintainer reads a question, not a verdict already reached. It is not
a failure and not "could not address": everything else is addressed this
round and the answer arrives as ordinary new feedback the next round — no
new marker or state, it just rides along in the summary. Distinguishes
decline (the change is not worth doing) from escalate (the call is not
the agent's to make).
Pins the new disposition in the existing SKILL policy test.
Co-authored-by: wenshao <wenshao@example.com>
* ci: label a PR that closes an issue its own author opened
Some PRs fix an issue the PR author themselves reported — self-reported
and self-fixed. That is not wrong, but the problem was never
independently validated, so a reviewer wants to check the issue is real,
not only that the fix is correct. This applies a `review/self-reported`
label so that shows at a glance and can be filtered.
A small pull_request_target workflow, metadata only (it never checks out
the PR's code): it reads the PR's closingIssuesReferences ("Fixes/Closes
#N" plus the Development-sidebar links) and, if any of those issues was
opened by the PR author, adds the label; it removes the label if the
link is later re-pointed or dropped. PR-controlled values reach the
script only through env, never interpolated into the run body.
* ci: single-quote workflow string values for yamllint
The repo's .yamllint.yml enforces quoted-strings (quote-type single,
required). The initial workflow left name, on/types, permissions,
concurrency group, runs-on, and the env values unquoted, failing the
Test job's yaml lint. Single-quote them (double where a value contains
single quotes, block scalar for the if), matching the qwen-fleet-shepherd
style. No behaviour change.
* fix(ci): never strip self-report label on a failed GraphQL query
Track whether the closingIssuesReferences query succeeded (API_OK) and gate label removal on it, so an API blip can no longer masquerade as "no self-reported link" and strip a correct label. Also re-run on synchronize so a commit-message "Fixes #N" link updates the label, add a fail-open regression test, and use the root yaml dependency in the test.
* fix(ci): add timeout-minutes and labelCreated test assertion (#7630)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Review rounds ratchet code upward — each round tends to ADD (a guard, a
comment, a test) to satisfy a finding, with no counter-pressure to
simplify, so PRs accrete over-defensive, over-commented bloat. AGENTS.md
already forbids this ("Simplicity First ... No error handling for
impossible scenarios", "Comments: Default to none"), but the
address-review flow's "implement each valuable finding" never invokes it.
Wire it in: when addressing findings, apply Simplicity First and the
Comments rule (smallest change, no impossible-case guards, no
restate-the-code comments) and, since rounds only add, ask each round
what the change lets you REMOVE. A suggestion whose only effect is more
defense, config, or narration is a Decline, not an auto-implement. The
pre-commit self-audit now also rejects bloat, not just defects. Points at
AGENTS.md rather than duplicating it; pins the wording in the SKILL test.
Co-authored-by: wenshao <wenshao@example.com>
* feat(autofix): auto-recover a PR parked on a stale base
#7595 catches a stale-base build failure DURING an address-review round,
but a PR already parked when the base moved under it never gets a round
for it to fire in: green PR checks, no new feedback (the handoff advanced
the watermark), no conflict — so the scan skips it forever. Five managed
PRs were stuck this way, 29-86 commits behind main, each "build failed on
the agent-committed fix"; recovering them took a manual update-branch +
/retry per PR.
The gate-rejection handoff now drops a head-scoped autofix-handoff marker
(only when a real fix was rejected — not a crash/timeout, which produced
no fix to re-verify). The scan reads it and, while it still matches the
live head and that head is behind main, merges main in and re-arms so the
loop re-reads the feedback on a fresh base. Self-limiting: a push clears
the head match, and the update makes it current so behind-main cannot
re-fire. Every API call is fail-safe.
The retroactive counterpart to #7595, closing the "parked when the base
went stale" blind spot.
* test(autofix): cover both-empty heads guard in stale-base unpark (#7602)
* fix(autofix): add fail-safe handler to stale-base recovery comment (#7602)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
* feat(autofix): auto-update a PR red only from a stale, since-fixed base
A PR can be red purely because it merged a main that was broken then and
is fixed now — observed twice today: a web-shell TS break and an
agent-registry test, each stranding healthy PRs on a failure with nothing
to do with them. The recovery was manual: merge current main and let CI
re-run. The scan now does that automatically via GitHub's update-branch
(a merge, never a rebase, so no force-push and no dismissed history).
The single safety gate is that the SAME failing check is passing on
current main. That one condition proves both halves at once: the red is
base-inherited (green on main = not the PR's own bug) AND main is healthy
on that check right now (so the merge cannot import a fresh breakage). It
acts only when the PR is also BEHIND main (compare status behind/diverged)
— otherwise the update is a no-op and the red is not stale-base after all.
Self-limiting: after the update the PR contains main's head, so it is no
longer behind and the next scan will not re-update. A failed update (merge
conflict) is logged and the PR is left for a human. Runs before the
feedback logic because a stuck-on-stale-base PR often has no new feedback
at all — it just sits red — which is exactly what stranded #7490.
* fix(ci): move pipefail fallback outside command substitution (#7554)
* fix(ci): guard stale-base update-branch with expected_head_sha (#7554)
* test(ci): pin fail-closed behavior for empty MAIN_HEAD and CMP_STATUS (#7554)
* fix(ci): guard stale-base update-branch with DRY_RUN (#7554)
* test(autofix): repair the merge-resolution test breakage
Resolving the base conflict kept this branch's older CONSECUTIVE_FAILURE
and handoff-decision tests (which predate main's PREPARE_OUTCOME env
plumbing), so both broke, while it correctly re-anchored the stale-base
and infra block extractions.
Take main's test file wholesale — its consec-fail, handoff, infra and
bilingual tests are all current — then re-add this PR's one intentional
test (the stale-base auto-update), and re-anchor the infra test's block
extraction onto the "# Auto-rerun a check that died on INFRASTRUCTURE"
comment so it stops at that block instead of over-extracting past the
now-adjacent stale-base block. Full suite green bar the pre-existing
load flakes (eligibility recheck, permanent API failures terminal).
* fix(autofix): fall through to feedback on failed update-branch; assert CAS param (#7554)
* fix(autofix): address review — fix stale-base gate source, add base dimension, bound repetition (#7554)
* fix(autofix): drop self-contradictory predicate 2, add state/PR_HEAD_OID tests (#7554)
* fix(autofix): address review — identity-gate the stale-base write, correct the green-checks safety claim, per-selector guard test, gate the compare call (#7554)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
A fix that fails to build is not always the fix's fault. #7471 stalled
when the agent's verification gate failed with "Cannot find module
'update-notifier'" — a dependency main removed in #7515, still imported
on a branch 32 commits behind. The loop could not tell a stale-base build
failure from a genuine one, so it advanced past the feedback and asked a
human to take over.
In the gate-rejection branch, before the handoff, compare the checked-out
head with main; if it is behind or diverged, update-branch (a CAS on
REPORT_HEAD) merges main in and the round retries (sentinel ts keeps the
feedback live). It self-limits: after the update the PR is current, so a
next-round rejection is no longer "behind" and falls through to the human
handoff — a genuine fix failure costs at most one base-update. The round
is exempt from the consecutive-failure breaker (not the PR's fault), and
every API call is fail-safe.
This is the agent-gate sibling of #7554, which only sees PR status checks,
never the gate's own build.
Co-authored-by: wenshao <wenshao@example.com>
* feat(autofix): auto-rerun a check that died on infrastructure, once
A failed check can be red because the machine died, not the code — a
self-hosted runner losing the server, the disk filling. #7490's E2E
failed with "runner lost communication with the server" and went green
on a rerun. The scan now reruns such a check's failed jobs automatically.
Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES)
— only unambiguous machine failures, never a test-level timeout, which
could be a real regression. The one-shot guard is run_attempt, not a
marker: a run already retried to attempt 2 and still infra-failing is
persistent, so it is left for a human; after a rerun the attempt
increments, so the next scan will not rerun it. Every step is fail-safe
(any API error → no rerun), it runs only when the PR actually has a
failed check, and the gate carries the same review-address carve-out as
the other check selectors so the loop never reruns its own runs.
This is the transient-infra sibling of #7554 (stale-base): that merges
current main when a check is base-inherited; this reruns when a check
died on the runner. Neither touches a check that is a genuine failure.
Note: rerun-failed-jobs needs the PAT to hold `actions: write`.
* fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562)
* fix(autofix): also treat a git fetch/clone transport death as infra
#6506's checkout died mid-transfer — "fetch-pack: invalid index-pack
output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job
into the 20m limit. That is infra, not the PR (it only touches a doc),
and a re-run made it green. But the infra-signature whitelist did not
cover it, so the auto-rerun did not fire and it waited on a human.
Add `invalid index-pack output` and `RPC failed` — the two canonical
git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present
job-timeout line does not block the match (one matching line classifies
the run), and a BARE timeout with no transport signature is still left
alone, since it can be a real regression. Both new signatures are pinned
in the test's per-signature loop, plus a case on #6506's real composite
annotation and a bare-timeout-is-not-rerun guard.
* test(autofix): single-source the infra-signature list from the workflow
The infra-rerun test re-typed INFRA_FAILURE_SIGNATURES as an inline
mirror of the workflow's env value. Two copies that must be hand-synced
can drift — the test could keep passing against a stale list while
production changed, or vice versa. That is exactly the copy the
git-transport follow-up had to remember to update in two places.
Extract the list from the workflow source instead, the same
extract-from-source idiom the file already uses for NON_BLOCKING_CHECKS,
so there is only one copy and drift is impossible. A toContain guard
fails loudly if the env is renamed or the regex breaks, rather than
letting an empty pattern match every line and silently pass.
* fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
* feat(autofix): auto-rerun a check that died on infrastructure, once
A failed check can be red because the machine died, not the code — a
self-hosted runner losing the server, the disk filling. #7490's E2E
failed with "runner lost communication with the server" and went green
on a rerun. The scan now reruns such a check's failed jobs automatically.
Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES)
— only unambiguous machine failures, never a test-level timeout, which
could be a real regression. The one-shot guard is run_attempt, not a
marker: a run already retried to attempt 2 and still infra-failing is
persistent, so it is left for a human; after a rerun the attempt
increments, so the next scan will not rerun it. Every step is fail-safe
(any API error → no rerun), it runs only when the PR actually has a
failed check, and the gate carries the same review-address carve-out as
the other check selectors so the loop never reruns its own runs.
This is the transient-infra sibling of #7554 (stale-base): that merges
current main when a check is base-inherited; this reruns when a check
died on the runner. Neither touches a check that is a genuine failure.
Note: rerun-failed-jobs needs the PAT to hold `actions: write`.
* fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562)
* fix(autofix): also treat a git fetch/clone transport death as infra
#6506's checkout died mid-transfer — "fetch-pack: invalid index-pack
output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job
into the 20m limit. That is infra, not the PR (it only touches a doc),
and a re-run made it green. But the infra-signature whitelist did not
cover it, so the auto-rerun did not fire and it waited on a human.
Add `invalid index-pack output` and `RPC failed` — the two canonical
git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present
job-timeout line does not block the match (one matching line classifies
the run), and a BARE timeout with no transport signature is still left
alone, since it can be a real regression. Both new signatures are pinned
in the test's per-signature loop, plus a case on #6506's real composite
annotation and a bare-timeout-is-not-rerun guard.
* fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
The agent's address-summary.md / no-action.md already ends with a
collapsed Chinese translation, but the workflow-appended wrapper lines
around it — the "Addressed/Reviewed the latest feedback" lead-in, the
"Base-conflict check" line, and the "Re-review when you have a moment"
footer — were English-only and sat outside that block. So the posted
comment was only half translated, unlike the takeover-ack comments
(full collapsed Chinese block) and the "model/模型" sign-off in this
same report (already inline-bilingual).
Give each wrapper line an inline Chinese translation, matching the
model/模型 idiom. The English halves are preserved verbatim — the
streak-reset detector globs on "Addressed the latest review feedback"
and "no changes needed", and a test extracts these lines — so behaviour
is unchanged and old English-only comments still match. A new test pins
each English-Chinese pair so a future reword that drops the Chinese
fails. The terminal handoff/failure comment is left English-only for
now (SKILL.md keeps it so by design); that is a separate change.
Co-authored-by: wenshao <wenshao@example.com>
A timeout evaluated NOTHING — the agent ran out of budget before finishing,
so nothing was committed and the feedback is unaddressed. It was treated as
an evaluated verdict (real ts, watermark advances), which strands that
feedback: the next scan sees "nothing new" and never retries. Observed on
#7471 (round 13/100), a heavily-reviewed 1871-line PR: rounds 11 and 13
timed out, but round 12 pushed — so a timeout is transient far more often
than not, and advancing past it left the round-13 feedback unhandled.
run-agent.mjs now drops an `agent-timeout` signal on result.timedOut, and
the handoff routes it like a pre-verdict crash: sentinel ts (feedback stays
live) and a retry, with a headline that names the real fix at the cap
(split the PR or raise the budget). A PR that PERSISTENTLY times out is
bounded by the round cap and the consecutive-failure cap, so this cannot
loop forever — it just stops treating a one-off budget blip as a verdict.
The loop guard stays terminal (a tool-call loop is a real defect, not a
budget blip). An API error still routes to its own model-key handoff; the
timeout signal is written only when NOT an API error.
Co-authored-by: wenshao <wenshao@example.com>
* fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal
A base/infra failure BEFORE the agent runs was misread as an agent crash
and terminated the PR forever. When an early step fails — installing or
building the trusted base, checkout, node setup — the `Prepare branch and
feedback` step is skipped, so NEWEST is empty, and the report step's
"crashed before reading feedback" branch fired: MARK_ROUND=MAX_ROUNDS,
terminal, scan skips it on every future tick.
Observed: a web-shell TypeScript break on `main` failed `Install
dependencies and build` (which builds the trusted base) across a whole
scan batch, and SIX healthy PRs were stranded terminal at round=100 in
one run — including ones at round 9 and 11 that had nothing to do with
the break. `round=100` there is a terminal sentinel, not 100 attempts.
NEWEST-empty now splits on steps.prepare.outcome:
- 'skipped' (an earlier step failed, the agent never ran) is infra/base
and transient: retry with a sentinel ts so the feedback stays live,
incrementing the round so a PERSISTENTLY broken base is still bounded
and stops at the cap (recoverable with /retry).
- 'success'/'failure' (Prepare ran, no feedback produced) is a genuine
pre-read agent crash: unchanged terminal behaviour.
This is the reverse of the asymmetry #7482 addresses: that bounds a
crash AFTER reading that retried forever; this stops a transient failure
BEFORE reading from going terminal after one.
* docs(autofix): note a pre-Prepare cancel also retries intentionally (#7490)
* fix(autofix): also retry a cancelled/empty prepare outcome, not just skipped
A previous review comment on this PR noted that a job cancelled before
Prepare should retry too. It was right about the intent but the code did
not do it: `steps.prepare.outcome` is 'cancelled' for a cancel and '' for
a job that stopped before Prepare entered the step context — both DISTINCT
from 'skipped', so `== 'skipped'` sent them to the terminal branch, the
same over-termination this PR exists to fix.
Match on "not a real Prepare run" (`!= 'success' && != 'failure'`)
instead, so skipped, cancelled, and empty all retry; only a Prepare that
actually ran to a verdict (success/failure) with no feedback stays
terminal — the genuine pre-read agent crash. Test extended to drive the
cancelled and empty cases (retry) and both real-run outcomes (terminal);
mutation-verified that reverting to `== 'skipped'` reddens the cancelled
case.
* test(autofix): update the pre-read-crash case for the broadened retry
The prior commit broadened NEWEST-empty retry to skipped/cancelled/empty
but left the older 'replays the handoff decision' test asserting the old
terminal behaviour for an unset PREPARE_OUTCOME (which now retries). That
test's terminal cases now set PREPARE_OUTCOME=success/failure explicitly —
the only outcomes that still terminate — so it exercises the genuine
pre-read agent crash rather than the infra/cancel path.
* test(autofix): anchor the skipped-Prepare extraction past the CONSEC block
CI reddened `retries a skipped-Prepare` after main's consecutive-failure
cap (#7482) merged into this branch: that block was inserted between this
decision block and the report `{`, and it calls `gh api`. The test's
`{`-anchored regex over-captured through it, so the extracted script ran
the unstubbed `gh api` and failed. Anchor the end on the same
`# Consecutive-failure` comment the sibling gate-crash test already uses,
so the extraction stops at this decision block's own closing `fi`.
* fix(autofix): exempt skipped-Prepare from the consecutive-failure breaker
A broken base build skips Prepare, producing no API error file — so the
consecutive-failure breaker ran on the new retry path and, after 5
scans, re-introduced the exact mass-stranding this PR exists to prevent.
Exempt pre-agent infra failures (skipped/cancelled/empty outcome) from
the breaker, mirroring the transient 429/5xx exemption: same failure
class (not the PR's fault, self-heals, hits the whole batch). The round
cap + sentinel-ts /retry recovery already bounds a persistently broken
base.
Also trim "checkout" from the retry headlines (checkout failures do not
land in this branch) and hoist the duplicated MARK_TS assignment.
* fix(autofix): reset the consecutive-failure streak on prior infra-failure markers
The streak walker counted prior infra-failure headlines ("AutoFix could
not start —…") as failures, inflating the consecutive-failure count on
subsequent rounds. A PR with 3 real agent failures, then 3 rounds of
base-build infra failures, then 1 more real failure would trip the
cap-5 breaker even though only 4 rounds were the PR's fault.
Add the two infra-failure headline patterns as reset strings in the
streak walker, alongside the existing push and no-op resets. The
genuine agent-crash headline ("AutoFix could not start evaluation —…")
is deliberately excluded — it is a real failure and must still count.
* fix(autofix): clarify infra-failure headlines and else-branch comment (#7490)
Address review nits: the retry headline now mentions cancelled runs,
the cap headline says 'reached the round cap' instead of overstating
'could not start for N rounds', the else-branch comment says 'prepare
itself crashed' instead of 'agent crash', and the streak-reset pattern
is simplified now that both infra headlines share the same prefix.
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
* fix(ci): autofix route checks existing labels on non-trigger label events
When triage adds multiple labels in sequence, per-issue concurrency
cancels earlier runs. If the last label is not a trigger label
(e.g. scope/build-system), the surviving run skips the issue phase
even though the issue already has autofix/approved +
status/ready-for-agent.
Before ignoring a non-trigger label event, check ISSUE_LABELS_JSON
for both required labels. If present and the issue is open, proceed
with the issue phase. Trust was already established when the trigger
labels were applied (both require triage+ permission).
* fix(ci): require trusted sender for label fallback
* feat(autofix): stop a PR that fails to push for N rounds in a row
Under takeover the round cap is 100, which is right for a PR that needs
many PRODUCTIVE rounds. It is wrong for one that fails every round: #6723
ran 7 consecutive failed rounds (3 agent timeouts at 50 min, 4 gate
rejections whose fix broke tests) over 8 hours, heading for round 100,
because it is a 5700-line, 47-file, 5-day-old PR racing a fast-moving
main — every round re-resolves a conflict it cannot finish or that fails
the gate. Retrying at the same per-round budget will not converge; a
human has to rebase or split it.
Adds CONSECUTIVE_FAILURE_CAP (5), distinct from the total round cap. The
handoff step already runs only when a round did NOT push, so it counts
the unbroken run of prior failure markers — stopping at the first push
("Addressed the latest review feedback") or legitimate no-op ("no
changes needed"), either of which proves progress and resets the streak.
At the cap it forces the terminal round even under takeover, with a
handoff that names the real fix (rebase/split, then /retry). Cause-
agnostic: a timeout and a gate rejection both count.
* fix(autofix): address review feedback on consecutive-failure circuit breaker (#7482)
- Fix misleading comment: the walk is oldest-first (API order) with
reset-on-success, not newest-first with early stop
- Prefer the already-fetched ic.json over a redundant gh api call,
falling back to the API only when the file is missing
- Filter eval markers by re-arm window (win=) so pre-re-arm failures
do not immediately re-terminate a re-armed PR
- Add test coverage for the MARK_ROUND == MAX_ROUNDS guard and for
window-scoped streak counting
* fix(autofix): exempt transient model errors from consecutive-failure breaker (#7482)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
* docs(autofix): require evidenced pre-commit verification, not a bare "verified"
The skill already said to run build/typecheck/lint/Vitest before
committing, but softly — and #7408 committed a fix with a TS error the
gate then rejected while its summary claimed "verified all 3 commits".
A self-assessment the gate contradicts wastes a whole round.
Strengthens the address-review contract from "run the checks" to:
- actually run them, do not assert them from reading the diff;
- if typecheck or a touched-package test fails, do NOT commit — treat
the feedback as unresolved (failure.md);
- end address-summary.md with a `## Verification` section listing each
command run and its result; a bare "verified" is not acceptable.
The framing is structural, not etiquette: the deterministic gate re-runs
the same commands and discards the round on any failure, so skipping them
only moves the rejection later. Pinned by a test so it cannot soften back.
This is the checkable half of "audit before committing" — the
undirected/reverse-audit-until-clean practice does not transfer to an
unsupervised agent (no verifiable stopping condition, and it worsens the
timeouts seen on large PRs), but "run the gate's own checks first and
show the evidence" does.
* fix(autofix): clarify Verification section precedes collapsed Chinese translation (#7486)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
mobile-mcp has its own release cadence and CODEOWNER (@LaZzyMan).
Bumping it in every core release PR forces an extra approval from
LaZzyMan even though the change is a routine version bump.
Exclude @qwen-code/mobile-mcp from the workspacesToExclude list in
scripts/version.js, following the same pattern as @qwen-code/sdk.
Closes#7462
* fix(autofix): keep a still-red check visible until its head is judged
A red check is a persistent STATE, but the scan only counted checks that
failed AFTER the watermark. The moment the watermark passed the failure
the PR went quiet while still red. Measured on the live fleet:
#6451 watermark 10:55 3 reds completed 09:30, 09:30, 09:51
#7357 watermark 09:18 1 red completed 07:59
#7390 watermark 11:27:37 red completed 11:27:37 — a strict `>` hid it
the instant it appeared
All three sat red for hours while every scan logged "nothing new", and
#6451 wrote two consecutive no-ops whose reasoning never mentions the
three failures, because they were not in its feedback at all.
A currently-red check now counts as feedback until the head it ran
against has been evaluated. The address job records that head in its own
`autofix-redcheck` marker — carried inside the eval comment, so no
ts/acted/round parser changes and the agent still never sees it as
feedback — and the scan skips a PR whose recorded head still matches.
That bounds this to ONE look per head rather than every scan, which is
what keeps a permanently-red PR from being re-selected forever.
The head comes from the REMOTE, not local HEAD: after a rejected push
the two differ, and recording a sha that never landed would suppress the
reds on the head that actually exists. Empty on failure — matches no
marker, so the reds stay visible.
Two existing count assertions are replaced by the property they stood
for: every check selector in the scan carries the address carve-out.
* fix(autofix): pair REPORT_HEAD with the steps that emit its marker
Review found the assignment had landed in issue-autofix's "Report dry-run
/ failure" step, which emits no redcheck marker and has no ${PR} in scope
— dead code plus a malformed, swallowed API call. Verifying it surfaced a
second half the review did not state: review-address's OWN handoff step
emits the marker at line 3362 with REPORT_HEAD never assigned in that
step, since shell variables do not cross step boundaries. Neither step
sets `set -u`, so it expanded empty and the marker recorded no head —
fail-open, but the handoff path never recorded one.
Deletes the dead assignment, adds the missing one, and rewords the scan
log so the two overlapping counts no longer read as a sum.
The test now asserts the PAIRING per step block — emits iff defines —
rather than counting each kind. Counting was what let this through: both
counts were "right". The first fix for it keyed the sets by step NAME,
which merged the two identically-named "Report dry-run / failure" steps
and still passed with the bug reintroduced; keying by step block catches
it.
* fix(autofix): note fail-closed asymmetry on empty LIVE_HEAD (#7438)
* fix(autofix): close three state-transition gaps in persistent red-check tracking (#7438)
- Forward persistent red checks into agent feedback: the scan selects
via N_RED_NOW but the prepare renderer only showed checks that failed
AFTER the watermark, leaving the agent with an empty Failed checks
section. Add a Still-red checks section with the complement filter.
- Omit the redcheck marker on sentinel/retry handoffs: a sentinel ts
means the agent evaluated nothing, so recording a judged head would
suppress the retry the handoff promises.
- Record the checked-out head, not the report-time remote head: capture
the SHA in prepare before agent mutations and forward it as a step
output, so a mid-run branch move cannot stamp an unevaluated head as
judged.
* fix(autofix): test empty-LIVE_HEAD fail-closed path (#7438)
* fix(autofix): discard no-op same-head duplicates in the queued-job stale gate (#7438)
Two near-simultaneous scans can both enqueue the same PR with the same
watermark. When the first serialized job ends in a no-op, it records a
redcheck marker for the head it judged but leaves both the eval timestamp
and the round UNCHANGED — so the live-watermark/round revalidation never
fires, and the second job re-runs the agent and posts a duplicate report
for the same head.
Parse the latest live redcheck marker during prepare (mirroring the scan's
RED_HEAD parse) and add its head match against CHECKED_OUT_HEAD as a third
stale-duplicate signature, reusing the existing "nothing newer" revalidation
so newer feedback or a live conflict still keeps the target actionable.
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
* perf(startup): load undici lazily behind package-local dynamic imports
* fix(web-search): preload undici before building runtime fetch options
Address review: web search builds fetch options outside the content
generator preload path, so 33 web-search tests (and any standalone
search invocation) hit the requireUndici fail-loud guard. Also redact
and rethrow proxy dispatcher install failures, guard early promise
rejections against unhandledRejection, and pin the guard message with
a test.
* test(cli): cover loadUndici interop and gitUtils proxy path
Address review suggestions: add parameterized tests for the CJS
unwrap normalization used by both core and cli loadUndici helpers,
and verify getLatestGitHubRelease instantiates ProxyAgent when a
proxy argument is passed.
* fixup! test(cli): fix loadUndici test type errors
Export UndiciModule type and loosen test helper typing so the cli
package builds under tsc --build.
* fix(autofix): retry a verification-gate crash instead of burying the agent's fix
A gate failure had two very different meanings collapsed into one outcome. When
the gate DECLARES a verdict (outcome=failed) it evaluated the agent's attempt
and rejected it, so advancing the watermark is right — the same feedback would
reproduce the same rejection, and MAX_ROUNDS bounds it. But when the gate dies
WITHOUT a verdict it never judged the work at all, and advancing buries a fix
the agent had already written: the next scan sees "nothing new" and the PR sits
until a human deletes the marker by hand.
That is exactly how the nested-package ENOENT stranded #7329 and #7336. Both
agents had implemented the review feedback — the handoff even quoted the
implemented changes — but the gate crashed on its own bug while resolving
packages/channels/*, the commit was discarded, and the PRs read as "Could not
address the latest feedback automatically".
Two halves:
- The review-address gate now declares every rejection it can legitimately
reach: build, typecheck, lint and the per-package tests each call a
`reject_fix` helper that writes outcome=failed before exiting. (The resolver
call is deliberately left undeclared — a resolver error IS a gate bug.)
- The handoff treats an EMPTY outcome on a non-success job as the gate's own
crash and routes it to the existing sentinel/retry path, so the feedback
stays live and the next scan retries. The round still increments, so a
persistently crashing gate is bounded exactly as before, and the headline
names the real cause ("hit a verification-gate error before reaching a
verdict") and, on the final attempt, points at the gate logs.
Unchanged: a declared rejection still advances and reads as before, a
no-output crash keeps its own wording and retry, and a crash before the
feedback was read stays terminal.
Tests: the real extracted decision block is replayed under bash across declared
rejection (advances to NEWEST), gate crash (sentinel + retry + round+1), no
output (sentinel, original wording), the round cap (operator fix), and a
successful job (never a crash); plus the reject_fix helper is driven for real
to prove a rejection writes outcome=failed. Both mutation-verified — dropping
the crash arm, or unwiring one known rejection, turns them red.
* feat(autofix): feed the gate's rejection back so the retry can fix what it broke
#7208 was handed to a human over a two-character fix. The agent implemented two
review findings, the gate refused the commit because it did not compile
(TS4111: `truncated` comes from an index signature, use `['truncated']`), and
the loop stopped there — round 5/100, "A human should take over this PR".
Nothing in the loop could have recovered on its own, because the reason was
never carried anywhere the loop could read it:
- the handoff comment showed only the agent's optimistic summary, so neither a
human nor the next round could see WHY it was refused;
- the feedback filter (correctly) excludes the bot's own comments, so a retry
re-read only the original review points;
- so `@qwen-code /retry` would have re-run the same agent against the same
input and produced the same non-compiling change.
The compiler had already said exactly what was wrong. The loop just threw it
away. Three pieces carry it instead:
- Each deterministic check now runs through `run_check`, which tees its output
to a gate log; `reject_fix` writes the label plus the tail of that output to
gate-rejection.md. (A four-backtick fence keeps captured ``` output from
breaking out when this is posted as a comment.)
- The handoff comment carries that block between
`<!-- autofix-gate-rejection-start/end -->` markers, so a human sees the real
reason next to the summary instead of a report that reads like success.
- `Prepare branch and feedback` lifts it back out of the bot's newest comment
and puts it at the top of the next round's feedback: "Your previous attempt
was REJECTED by the verification gate — fix this first."
So a mechanical rejection now closes inside the loop, which is the point of
takeover. A rejection the agent cannot fix still burns rounds and ends at the
same handoff, bounded exactly as before.
Tests: the round trip is exercised end to end — a failing check's compiler
output lands in gate-rejection.md with its label, the handoff delimits it, and
the prepare step recovers the text (markers stripped) from the newest bot
comment while a round that pushed yields nothing to replay. Both halves
mutation-verified. #7351's verdict test is retargeted to run_check.
* fix(autofix): declare the gate verdict before writing its detail file
CI caught this and macOS could not: reject_fix wrote gate-rejection.md
first and outcome=failed second, so a failure to write the detail took
the verdict with it. An empty outcome on a failed job is the signal for
"the gate never reached a verdict" — a crash, which is RETRIED — so a
clean rejection whose detail write failed would be re-attempted every
round instead of being reported once.
The verdict is now written first and the detail write is non-fatal.
The ordering is pinned by a STATIC assertion, not only the behavioural
one: 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 exactly how it shipped green locally and red in CI, and
a guard that depends on the reviewer's bash would let it happen again.
* fix(autofix): escape the gate-rejection detail for real
The gate-rejection publish site used `sed 's/<!--/<!\-\-/g'` — single
backslashes, which sed reads as escaped literal `-`, so the replacement
is byte-identical to the match and the whole command is a no-op on both
GNU and BSD sed. The other four publish sites use `\\-\\-` correctly.
That mattered: the detail is `tail -c 3000` of build/typecheck/lint/test
output, published verbatim in a bot-authored comment. The scan parses
markers by matching the literal `<!-- autofix-eval ts=`, and it only
counts markers in bot-authored comments — so any check output containing
that string would have been parsed as a real eval marker.
The existing test counted the CORRECT spelling and asserted there were
four of them. A fifth site with the wrong spelling did not match the
counted string, so the count stayed at four and the test stayed green.
It now asserts every `s/<!--/…/g` site is byte-identical to the correct
form, which fails on exactly this bug.
Reported by qwen-code-ci-bot on PR #7368.
* chore(autofix): correct stale "ALL FOUR" escape-site comment to five (#7368)
* chore(autofix): document the head/tail byte-limit invariant (#7368)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* feat(autofix): raise the strict round cap from 5 to 10
Measured across the last 40 bot-authored PRs: 17 finished at round 0, 12
at 1, 4 at 2, 2 at 3, 1 at 4, and 3 reached the cap of 5. All three that
reached it merged AT it rather than stalling — and one of those spent two
of its five rounds on the verify-gate ENOENT that #7330 has since fixed.
So the ceiling was never the thing that stopped a PR, but it sat close
enough to bind on a bad day with no headroom. 10 gives that headroom.
The cap exists to stop an unproductive LOOP, not to ration ordinary
iteration; a genuinely stuck PR still stops, just later.
Deliberately not larger: retries for a transient model or gate failure
increment the same counter, so the cap also bounds how much one bad
provider window can spend. API_AUTH_MAX_ROUNDS stays at 3 and still
short-circuits the errors only a maintainer can fix.
Replaces the literal `MAX_ROUNDS: '5'` assertion with the ordering the
numbers must satisfy — auth cap < strict cap < takeover cap — so the
values stay tunable and a cap that stops binding fails instead.
* fix(scripts): anchor round-cap regex with word boundary (#7412)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Both no-response cases produced the same error, and its advice — "check
the 'Run Qwen Triage' step stderr above for diagnostics" — pointed at
diagnostics that do not exist for one of them: the action installs the
CLI with `npm --silent`, so an install failure prints an exit code and
nothing else. Observed on a PR whose triage died at exit 243 during that
install, leaving a maintainer told to read an empty log.
The two causes need opposite responses, and steps.triage.outcome already
distinguishes them:
- action failed -> no model call happened, nothing about the PR can
explain it, and the fix is to re-run the job
- action succeeded, empty summary -> the agent ran and returned nothing,
which IS worth reading the step output for
Measured baseline for the retry advice: of 19 executed triage runs, 4
failed, 3 of them on unrelated PRs, and one PR both succeeded and failed
within an hour on the same head.
The outcome is passed through env like RESPONSE already is, and the
script is replayed under bash for both branches.
Co-authored-by: wenshao <wenshao@example.com>
The scan skips any PR with checks in flight so a FAILED check can be read
as feedback. `review-pr` is not that kind of check: its output is a
REVIEW, delivered by its own real-time pull_request_review trigger and
counted by the review path, so its conclusion carries nothing the loop
acts on — but the PR stayed invisible to the scan for its whole duration.
Measured over 32 completed review-pr runs: median 49 minutes, p75 78,
p90 123, max 158. That is what a PR waited before autofix could touch it,
even when it already had unaddressed feedback.
Only `review-pr` is excluded, by name, via NON_BLOCKING_CHECKS.
Build/test/lint still block, because a failed one IS feedback. So does
`resolve-pr`, which mutates the branch. A test pins each excluded name to
a real job id in qwen-code-pr-review.yml — a rename there would silently
restore the wait with nothing failing.
The behavioral replay caught a real bug before it shipped: the first
version wrote `$nonblocking | index(.name)`, where `.` is already the
array, so `.name` indexed the wrong object and jq errored out — which
would have made HAS_PENDING_CHECKS empty and skipped nothing at all.
Co-authored-by: wenshao <wenshao@example.com>
`@qwen-code /resolve` and the autofix loop's own conflict path both merge
the base branch and push to the PR's head, but they live in different
workflows, so their per-PR concurrency groups only guarded each against
itself. On #7355 they ran together: /resolve pushed at 03:51, the autofix
leg pushed at 04:05 and was rejected `fetch first`, discarding a full
agent run and leaving no marker behind.
GitHub concurrency groups are repository-scoped, so both jobs now use the
same `qwen-pr-head-write-<pr>` group and queue behind each other instead
of racing. The prefix has to be a literal in both files — job-level
`concurrency` cannot read the `env` context — so a test pins the two
equal; renaming one side alone would silently re-open the race.
Co-authored-by: wenshao <wenshao@example.com>
* perf(telemetry): lazy-load the SDK and split OTLP exporter chains by protocol
* fix(telemetry): close lazy SDK init/shutdown races and make load failure non-fatal
Addresses PR #7276 review feedback: shutdown now awaits an in-flight init before tearing down (was racing past the sync flag and leaking a started SDK whose buffered spans/logs never flushed); the dynamic imports now sit inside init's try so a chunk-load failure degrades telemetry instead of aborting daemon runtime startup. Also breaks the sdk<->sdk-impl import cycle via a leaf otlp-urls module, hardens the sdk-node exporter stub for thenable/interop probes with a unit-tested separator-independent resolve, lists the HTTP exporter packages explicitly in the bundle guard, and adds lazy-init lifecycle tests.
max-parallel is the only place different PRs wait on each other — the
per-scan target budget (10) and the candidate-inspection budget (60) are
both far from binding at the current pool of 15. On the scan that
selected 7 PRs the legs ran exactly 3 at a time, each new one starting
3-4 seconds after a slot freed, so the 7th waited 81 minutes for a slot.
Replaces the literal `max-parallel: 3` assertion with the invariant it
was standing in for: a bound must exist and must still bind below
MAX_TARGETS_PER_SCAN. Pinning the number only detected edits — it would
not have caught the key being deleted outright, which is the actual
regression, and it forced this tuning change to touch a test.
Co-authored-by: wenshao <wenshao@example.com>
* fix(ci): stop /resolve reports from being guillotined mid-sentence
Every substantive /resolve summary was hitting the 2000-byte cap exactly
and stopping mid-word: #2993, #4256 and #6206 all ended at 2100 bytes
total, cut inside a sentence, with nothing saying the report had been
clipped rather than abandoned.
Two causes, both fixed:
- The contract asked for a file-by-file inventory, which duplicates the
diff and grows without bound. It now asks for what only the resolver
knows — the root cause on the base branch, whether the merge was
semantic or merely textual, what the resolution's correctness rests
on, and what it could not verify (this command runs no tests and may
not touch non-conflicted files, so a merge that breaks an untouched
test can only be reported).
- The cap was silent and too low. It is now 6000, above the 4000 the
prompt asks for, and a report that still exceeds it says so.
Also adds the project's collapsed Chinese section to the contract; no
/resolve report had one.
* fix(ci): make the truncation test fatal-decode real and link the run in the notice (#7389)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* fix(autofix): retry a model API error instead of stranding the PR
When the agent's qwen subprocess dies on a model-side [API Error]
(403 access denied, a 429 quota, a 5xx), run-agent.mjs wrote a
handoff/failure.md, so the handoff step treated it as an EVALUATED
handoff — it advanced the watermark and the next scan saw 'nothing
new', stranding the PR until a manual re-arm. But the agent never
actually evaluated the feedback; the model was unreachable.
#7220 hit exactly this: fork-takeover engaged and ran the agent, the
model returned '[API Error: 403 Model access denied]' (the autofix key
lacks access to qwen3.8-max-preview), and the PR was left with an
advanced watermark that will not retry.
Fix, mirroring #7229's no-output-crash handling:
- run-agent.mjs extracts a [API Error: 4xx/5xx] from the captured
output tail, includes it in failure.md, and drops an
marker file.
- The handoff step reads that marker and routes the failure to the
sentinel-ts (retry) path — the watermark does NOT advance, so the
next scan retries; the round still increments so a PERSISTENT model
failure is bounded by MAX_ROUNDS. The headline names the model error
and, on the final attempt, tells the maintainer to check the autofix
model key/access and re-arm — instead of a generic crash message.
Tests: run-agent.mjs flags a model [API Error] (marker + failure.md)
and does NOT flag a generic failure; the handoff replay treats an
API-error handoff as sentinel|retry (not a watermark advance) with a
model-aware, cause-specific headline. 62/62 + 12/12.
* fix(autofix): scope + broaden the retryable model-API detection (review)
Addresses wenshao's review on #7247:
- Behavioral (1): the agent-api-error marker was written on ANY non-zero
exit whose output tail contained an API-error string — so a loop
guard, a timeout, or an agent-written failure.md (a real verdict)
would wrongly retry and, worst case, silently discard a verdict. The
write is now scoped to the bare-failure branch and guarded by
!timedOut, so only an un-evaluated model failure retries.
- Coverage (2): the old regex only matched a LEADING status digit, so
it missed the canonical rate-limit render, the (Status: …) form, the
bad-key 401, the Chinese quota text, and the unwrapped Qwen OAuth
quota — i.e. most real errors this targets. Detection is now a
whitelist of RECOVERABLE errors (401/402/403/429/5xx + rate-limit /
quota / api-key / RESOURCE_EXHAUSTED / overloaded phrasings, plus the
standalone OAuth-quota form); a 400/404 stays terminal.
- Test gap (3): a writer↔reader contract test now runs the REAL
run-agent.mjs to write the marker, then the extracted workflow reader
block against that same workdir — a rename on either side (proven
with the YAML-only mutation) now fails the suite.
- Smaller: API_ERROR_DETAIL is comment-escaped (sed) and capped
(cut -c1-200) since it derives from agent stdout; the marker match is
single-line ([^]\n]) so a multi-line render can't smuggle a newline;
agent-api-error is added to the run-artifacts list.
Non-recoverable 4xx (400/404) deliberately stay terminal; the live
401/403 config cases retry and self-heal once the key/access is fixed.
79/79 across both suites.
* test(autofix): cover the timeout guard and the OAuth-quota fallback (review)
Two coverage gaps from the ci-bot review on #7247:
- The !result.timedOut guard was only asserted indirectly — no test
emitted an [API Error] AND timed out. Added a case (spawnSync +
QWEN_TIMEOUT_MS=100): qwen streams [API Error: 503] then hangs past
the budget → killed → no marker. A refactor to !loopDetected now
fails here.
- The standalone Qwen-OAuth-quota fallback (unwrapped, no [API Error:])
had no test. Added a case emitting bare 'Qwen OAuth quota exceeded
(limit: 100/min)' → marker written, wrapped as
'[API Error: Qwen OAuth quota exceeded …]'.
* fix(autofix): anchor the API-error code, split retry budget by cause, keep the headline UTF-8
Addresses the review on #7247.
Classifier (points 2 and 4): the status code is now read from its POSITION in
the render (`[API Error: <code>`) instead of matched anywhere in the message.
Matching anywhere retried permanent failures forever — `400 Invalid value for
max_tokens: must be <= 512` matched a bare \b5\d\d\b and `400 context length
exceeded` matched a bare `exceeded`. `exceeded` now only counts as part of
`quota`. A 404 whose message says the model "does not exist or you do not have
access to it" — the OpenAI-compatible render of what a 403 reports — is no
longer terminal.
Retry budget (point 3): the marker now carries the cause class. A transient
429/5xx self-heals and keeps the full round budget; an auth/access error that
only a maintainer can fix is capped at API_AUTH_MAX_ROUNDS (3) and then goes
terminal with the "check the autofix model key/access, then re-arm" headline —
instead of ~100 agent runs and ~100 PR comments over ~17h on a takeover PR.
The terminal round is stamped so the scan's round gate skips the PR while the
sentinel ts keeps the feedback live for a re-arm.
Headline (point 1): `cut -c` counts bytes under GNU coreutils and the
classifier deliberately matches CJK renders, so the 200-byte cap could split a
multi-byte character and emit invalid UTF-8. Guarded with
`iconv -f utf-8 -t utf-8 -c || true`, matching the sibling publish site (the
`|| true` is required — iconv -c exits 1 when it discards).
Minor (point 5): documented that detection is best-effort because apiError is
derived from the last 20 KB of output; `head -1` -> `head -n 1`; tests added
for a permanent 400 carrying a 3-digit number >= 500 and for a >200-byte CJK
render staying valid UTF-8.
* test(autofix): cover the auth-capped retry budget and Chinese API-error patterns (#7247)
* fix(autofix): short-circuit 400 as terminal and classify only the last API error (#7247)
* fix(autofix): treat transport-level API failures as retryable
#7365 stranded at round 2/100 on this render:
[API Error: terminated (cause: read ECONNRESET)]
The connection to the model dropped mid-run. That is as transient as a 429, but
the classifier never saw it that way: a transport failure never got far enough
to have an HTTP status, so it fell through to the keyword arm, and the keyword
arm only knew about rate limits and quotas. It was classified terminal, the
watermark advanced, and a PR that needed nothing but a re-run was handed to a
human.
Verified against the shipped classifier before the fix — every transport render
came back terminal:
terminated (cause: read ECONNRESET) -> terminal
fetch failed -> terminal
socket hang up -> terminal
connect ETIMEDOUT -> terminal
Adds a transport arm to the code-less branch: ECONNRESET, ECONNREFUSED,
ETIMEDOUT, EPIPE, EAI_AGAIN, socket hang up, fetch failed, terminated.
ENOTFOUND is deliberately excluded. A hostname that does not resolve is a
misconfigured endpoint, which repeats forever — the same reasoning that keeps a
bad model name terminal.
Coded errors are unaffected: the arm sits after the status-code branch, so the
400 short-circuit added in 719991a3b still runs first.
* fix(autofix): address review — OAuth fallback override, comment accuracy, display clamp (#7247)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: 易良 <1204183885@qq.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Applying autofix/takeover to a PR that targets another branch left no
visible trace: the label stuck, the pull_request:labeled route run went
green, and the only record was one line in a job log. A stacked PR then
looked exactly like a managed one while nothing was managing it.
The route now emits a 'base-refused' ack, and the ack job posts a
bilingual explanation naming the base it was refused against. The
refusal deliberately reads no live PR state, so the one ack whose whole
purpose is to explain silence cannot itself be silenced by an unrelated
API failure.
Co-authored-by: wenshao <wenshao@example.com>
* feat(autofix): render the managed fleet into the scan's run summary
Seeing whether the loop was healthy meant reconstructing it by hand: list the
bot's PRs, fetch each one's comments, regex the autofix-eval markers for round
and watermark, then cross-check gh pr checks and the fork/takeover state. That
is how today's triage of #7246, #7259, #7329, #7333 and #7336 was done, and it
is why a stalled PR stayed invisible until somebody went looking for it.
The scan already computes every one of those facts while deciding what to
process — it just wrote them to a job log nobody reads. Each per-PR terminal
decision now also records a row, and the step renders one markdown table into
the run summary:
| PR | State | Detail |
| #7329 | SELECTED | 1 review + 5 inline new (round 0/5) |
| #7333 | idle | nothing new since 2026-07-20T13:54:18Z |
| #7262 | waiting | active checks in flight |
| #7208 | round-capped | round 100/100 - needs a human or @qwen-code /retry |
States cover every branch that ends a PR's inspection: busy, skipped, unknown,
waiting, round-capped, idle and SELECTED — so a PR cannot drop out of the table
by returning early, which is exactly the invisibility this fixes.
No new API calls (the data is already in hand), no writes outside the run
summary, and the helper is defined at the top of the step so it stays clear of
the BUSY_PRS/INSPECTED proximity guard that keeps the free busy-skip from
consuming the inspection budget.
Tests: the real helper and render block are replayed over fixtures (table
structure, one row per state, and an empty fleet still rendering a table), plus
each decision branch is pinned to its fleet_row. Mutation-verified: dropping
one branch's row turns it red.
* fix(autofix): use temp file for fleet test replay; cover fork-head skip (#7355)
* test(autofix): assert each skipped fleet_row call site individually (#7355)
* fix(autofix): record fleet rows for both budget-break paths (#7355)
The candidate-inspection budget break incremented INSPECTED but never
called fleet_row, so the PR that tripped the budget was silently absent
from the fleet table. The target-budget break left all remaining
candidates invisible with no truncation signal.
Add a per-PR deferred row before the inspection-budget break and a
summary deferred row before the target-budget break so the fleet table
stays complete in both cases.
* fix(autofix): harden fleet summary render and clean up temp file (#7355)
Address review feedback:
- Escape '|' in detail values to prevent broken table columns
- Render budget summary row (PR '-') as em dash instead of '#-'
- Add trap for FLEET_FILE cleanup on early exit paths
- Document deferred summary row semantics in test comment
* fix(autofix): use summary row for candidate-inspection budget break (#7355)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(autofix): retry a verification-gate crash instead of burying the agent's fix
A gate failure had two very different meanings collapsed into one outcome. When
the gate DECLARES a verdict (outcome=failed) it evaluated the agent's attempt
and rejected it, so advancing the watermark is right — the same feedback would
reproduce the same rejection, and MAX_ROUNDS bounds it. But when the gate dies
WITHOUT a verdict it never judged the work at all, and advancing buries a fix
the agent had already written: the next scan sees "nothing new" and the PR sits
until a human deletes the marker by hand.
That is exactly how the nested-package ENOENT stranded #7329 and #7336. Both
agents had implemented the review feedback — the handoff even quoted the
implemented changes — but the gate crashed on its own bug while resolving
packages/channels/*, the commit was discarded, and the PRs read as "Could not
address the latest feedback automatically".
Two halves:
- The review-address gate now declares every rejection it can legitimately
reach: build, typecheck, lint and the per-package tests each call a
`reject_fix` helper that writes outcome=failed before exiting. (The resolver
call is deliberately left undeclared — a resolver error IS a gate bug.)
- The handoff treats an EMPTY outcome on a non-success job as the gate's own
crash and routes it to the existing sentinel/retry path, so the feedback
stays live and the next scan retries. The round still increments, so a
persistently crashing gate is bounded exactly as before, and the headline
names the real cause ("hit a verification-gate error before reaching a
verdict") and, on the final attempt, points at the gate logs.
Unchanged: a declared rejection still advances and reads as before, a
no-output crash keeps its own wording and retry, and a crash before the
feedback was read stays terminal.
Tests: the real extracted decision block is replayed under bash across declared
rejection (advances to NEWEST), gate crash (sentinel + retry + round+1), no
output (sentinel, original wording), the round cap (operator fix), and a
successful job (never a crash); plus the reject_fix helper is driven for real
to prove a rejection writes outcome=failed. Both mutation-verified — dropping
the crash arm, or unwiring one known rejection, turns them red.
* fix(autofix): clarify retry-branch comments per review nits (#7351)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
* feat(autofix): resolve the review threads whose findings it implemented
A human re-reviewing a managed PR currently has to re-read every thread to work
out what the bot already handled. #7308 shows the cost: 17 review threads, 13
still open, with no way to tell which of those were fixed and which were
declined.
The agent already decides per finding — it records each one in
address-summary.md as implemented or declined with a reason. What was missing
was a way to act on that: the agent's sandbox carries no GitHub token, so it
cannot resolve anything itself, and feedback.md gave it no stable handle to
point at even if it could.
Three small pieces close that:
- feedback.md now renders each inline comment with its id (`- [rc:<id>] …`),
giving the agent a handle it can echo back.
- The SKILL asks the agent to write resolved-comments.txt: one id per line, for
findings it IMPLEMENTED only. A declined or deferred finding must stay
unresolved so its recorded reason actually gets read.
- After a successful push, the step that already holds the PAT maps each id to
its review thread and resolves it.
Deliberately narrow: only threads the agent claims it implemented, only ones
not already resolved, and entirely best-effort — a resolve failure warns and
never fails a good push.
Tests: the real extracted block is driven with a stubbed gh over fixture
threads — an implemented finding's open thread is resolved, a DECLINED
finding's thread is left open, an already-resolved thread is skipped, and an
unknown id matches nothing. Mutation-verified: dropping the isResolved guard
turns it red.
* fix(autofix): harden review-thread resolution per review feedback
- Guard --jq against null pullRequest (// {nodes:[]}) so a transient
API inconsistency cannot crash the step after a successful push
- Add pageInfo{hasNextPage} and emit :⚠️: when threads exceed
the first-100 page cap
- Tolerate rc: prefix and trailing CR in resolved-comments.txt
- Emit :⚠️: when a valid numeric id matches no open thread
- Match production set -euo pipefail flags in the extracted-block test
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>
* fix(ci): stop a slow patrol classifier from killing every flaky rerun
The CI Failure Patrol has been effectively offline. Across the last 30
scheduled runs, 28 were cancelled and only 2 succeeded — and both survivors ran
at 00:0x, in ~2 minutes, when the model was idle.
Step timings show why. Setup, checkout, node and the scan finish in 28
seconds; the model step then runs 9m39s and is killed by the job's 10-minute
timeout. Because the job dies there, Validate and Upload never run, the `act`
job is skipped on `classify.result != 'success'`, and nothing is ever re-run.
One slow step was taking down the whole patrol, every cycle, for hours.
That is why #7333 still carries a red `web-shell E2E Smoke` whose failure is
`No space left on device` — a textbook infra flake, inside the patrol's own
TARGET_WORKFLOW, that the patrol never got far enough to re-run.
- The classifier step is now bounded at 5 minutes, below the job's 10, and
marked continue-on-error: a slow model costs one patrol cycle instead of the
patrol, and the next tick simply tries again.
- An empty classifier result is reported (has_decisions=false) rather than
failing the job, so the run finishes cleanly instead of looking like a broken
patrol.
- Upload and the `act` job are gated on decisions actually EXISTING, not merely
on the classify job having survived — otherwise a no-decision cycle would
look actionable.
Tests: the step's timeout is asserted to be strictly below the job's, plus the
continue-on-error and the has_decisions gating on Upload and `act`; and the
Validate step's bash is replayed for real with and without a decisions file,
asserting exit 0 and the right flag in both. Mutation-verified — removing the
step bound, or the decisions step id, turns it red.
* fix(ci): validate JSON syntax in patrol decisions before acting (#7358)
---------
Co-authored-by: wenshao <wenshao@example.com>
* feat(autofix): pick up managed fork PRs in real time instead of waiting for the throttled schedule
The `pull_request_review` trigger already routes feedback straight to the PR it
arrived on, but it admitted ONLY in-repo bot PRs — every fork under takeover
had to wait for the scheduled scan. That schedule is far slower than it looks:
the cron says `*/10`, but GitHub throttles scheduled events on this repo hard
enough that the observed interval is 40-70 minutes (and in the same window
`pull_request_review` fired 17 times while `schedule` fired once). So the
takeover PRs a maintainer is actively iterating on were the ones waiting
longest for their feedback to be picked up.
Real-time pickup now applies the SAME admission the scheduled scan uses for a
fork: allow-edits on, and either the bot's own fork or an explicit
autofix/takeover label. Nothing about *what* may run changes — this event runs
in base-repo context, and review-address independently re-verifies allow-edits,
a live write+ author and a matching live head repo before it touches the
branch. Only *when* the same gated work happens changes.
Unchanged: non-main targets, untrusted senders, human in-repo PRs and forks
that are neither the bot's own nor takeover-labelled are all still ignored, and
only `pull_request_review:submitted` triggers (not per-comment events).
Tests: a behavioural replay drives the extracted route block with a stubbed gh
across eight cases — in-repo bot admitted, in-repo human rejected, bot fork and
takeover-labelled fork admitted and routed to that PR, and no-allow-edits,
unlabelled fork, non-main base and untrusted sender all rejected. Mutation-
verified: restoring the blanket fork rejection turns it red.
* fix(autofix): admit real-time fork PRs in review-scan's forced predicate (#7350)
The route step now admits managed fork PRs for real-time review pickup, but
review-scan's forced-PR predicate still required `.isCrossRepository == false`,
so every fork was rejected there: targets=[] / has_targets=false and
review-address never started — the feature was silently discarded for the very
PRs it added. Admit forced fork PRs under the scheduled scan's OWN fork rules
(allow-edits on, plus a live write+ author check mirroring the scan's
per-candidate gate); in-repo PRs keep the fail-closed `.isCrossRepository ==
false` test. review-address still re-verifies allow-edits, a live write+ author
and a matching head repo before pushing. Also exercise the route step's
metadata-read-failure branch (fails closed) in the workflow tests.
---------
Co-authored-by: wenshao <wenshao@example.com>
* feat(autofix): re-arm a stranded PR with @qwen-code /retry instead of deleting a marker
Recovering a stranded managed PR meant running `gh api -X DELETE` against the
bot's own autofix-eval marker comment. That needed raw API access and the
comment id, erased the audit trail, and was undiscoverable unless you had read
the workflow — it came up twice while triaging #7246, #7329 and #7336.
`@qwen-code /retry` now posts a single `<!-- autofix-rearm -->` marker, which
does both halves of what the deletion did:
- The scan's watermark ignores eval markers written BEFORE the newest re-arm,
so the feedback those markers buried is read again. The watermark stays
global otherwise — this is an explicit, maintainer-issued exception, which is
exactly what the deletion was, only recorded instead of destructive.
- The marker also opens a fresh counting window (it joins the engage ack in
REARM_KEY), so the round counter resets and a terminal round stops skipping
the PR. That also means the existing "a re-arm supersedes queued old-window
jobs" guard covers /retry for free.
The address job's live recheck mirrors both, so a run selected before a re-arm
still discards itself instead of stamping an old-sequence marker.
Authorization is the takeover command's, unchanged and reused rather than
reinvented: exact body match, live permission lookup, in-repo-only author
privilege. The route prefilter now admits the second command. The job verifies
CI_DEV_BOT_PAT authenticates as the bot before commenting, because both
scanners only count markers authored by it. The marker is registered as a
control comment so the agent never sees the re-arm as feedback to address.
Tests: the real extracted scan block is replayed over synthetic comment
fixtures — stranded (watermark held, round 2), after /retry (watermark
released, window reset, round 0), a marker written after the re-arm counting
again, and a re-arm from a non-bot author correctly ignored. Both halves
mutation-verified.
* test(autofix): add behavioral test for address-side re-arm stale check (#7354)
* fix(autofix): generalize remaining command-ignored messages and assert all filter sites (#7354)
* test(autofix): add behavioral test for the retry-command re-arm marker job (#7354)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
* fix(autofix): resolve owning package for nested paths; report verify-failed handoffs as not pushed
The verify gate mapped each changed file to a flat `packages/<dir>` and
read `<dir>/package.json`, which ENOENT-crashed on nested packages such
as packages/channels/base — the container packages/channels has no
package.json. Walk each changed file up to its nearest package.json in
both the issue-fix and review-address verify steps, and skip any
candidate that still has none.
When such a verify failure follows an agent commit, the review-address
handoff rendered the agent's optimistic address-summary.md (which can
cite a commit SHA) under a neutral "what I found" heading, so a
maintainer chased a commit that was discarded with the runner workspace.
An EXIT trap now records any post-commit non-zero exit as outcome=failed,
and the handoff states plainly that the change did NOT pass the gate and
was NOT pushed.
Tests: walk-up detection over a nested package tree, the outcome=failed
trap, and the not-pushed handoff wording — each mutation-verified.
* refactor(autofix): extract owning-package resolver to a shared staged script
Addresses review on #7330.
Extract the changed-file → owning-package walk into
.github/scripts/resolve-owning-packages.sh, staged to RUNNER_TEMP from the
trusted base alongside check-settings-schema.sh and invoked from both verify
gates, so the two gates cannot drift into resolving packages differently (the
8-line walk was otherwise duplicated verbatim in each). Updates the
package-scripts test that pinned the old inline grep.
Narrow the verify-failed handoff lead-in to "This change was NOT pushed": four
paths set outcome=failed BEFORE the deterministic gate runs (agent abort via
failure.md, dirty tree, unchanged branch, missing address-summary.md), so the
previous "did NOT pass the verification gate" claim was factually wrong for
them. The specific reason stays in the headline and the quoted summary.
* style(autofix): brace variable references in resolve-owning-packages.sh
The repo's shellcheck gate runs --enable=all --severity=style, under which
bare $f/$d references trip SC2250 (prefer ${var}). Brace them to match the
convention already used in check-settings-schema.sh, and update the script
content assertions accordingly. Verified with shellcheck 0.11.0 using the
exact CI flags: clean.
* fix(autofix): resolve owning workspace via npm query; key unpushed-handoff on commit existence
Addresses the deeper review on #7330.
Blocking issue: the "nearest package.json" resolver mapped a change under a
workspace's fixture/example package (e.g.
packages/cli/src/commands/extensions/examples/starter) to that fixture, whose
test script is not Vitest — silently SKIPPING packages/cli's own tests, a
coverage regression invisible in the log. Resolve against the authoritative
`npm query .workspace` set instead and take each file's longest-prefix
workspace: nested workspaces (packages/channels/base) match exactly, fixtures
and non-workspace paths (packages/sdk-python, packages/README.md, the excluded
packages/desktop) drop. Also harden the resolver against a final line with no
trailing newline and against an unmatched last line, which under
`set -o pipefail` would otherwise abort the script.
Handoff wording: keying "was NOT pushed / commit discarded" on outcome=failed
was wrong for the abort paths (failure.md, dirty tree, unchanged branch,
missing address-summary.md), which set outcome=failed before ever making a
commit. Record committed=true right after checkout — before any gate can fail
— and key the wording on that; the abort/no-op paths keep the neutral framing.
This removes the EXIT trap entirely (its only observable effect was that
wording), so it no longer mislabels pre-commit failures either.
* fix(autofix): expand workspaces on-disk so branch-added packages are tested; harden resolver
Addresses the re-review on #7330.
The resolver sourced its workspace set from `npm query .workspace`, which reads
node_modules — installed from the BASE checkout. A workspace the PR branch ADDS
(a new channel adapter, a new sdk — the issue-fix job's whole purpose) was
invisible, so its tests were silently skipped, and for a nested new package the
ENOENT crash this PR fixes turned into a silent skip. Expand the set from the
on-disk root package.json `workspaces` globs instead (shallow `dir/*` + literals,
honouring `!` negations, keeping dirs with a package.json): it reflects the
branch, matches what `npm run --workspace` accepts downstream, and needs no
install. Verified to reproduce `npm query`'s set exactly on the current tree.
Also from the review:
- Fail the gate loudly on an empty/unreadable workspace set instead of the
silent "no package changes" skip, and drop the now-unneeded `|| true` at both
resolver call sites (the resolver already exits 0 on legitimate no-match).
- Record committed=true at the TOP of the step (ref-only diff), covering an
agent that commits then aborts, and count only `git diff --quiet` exit 1 as a
commit (128 is a git error, not a discarded commit).
- Correct the two call-site comments that still described the superseded
nearest-package.json approach.
Also hardens the resolver against a final changed-path with no trailing newline
and an unmatched last line under `set -o pipefail`.
---------
Co-authored-by: wenshao <wenshao@example.com>
* fix(ci): tighten API error detection to avoid false positive on review prose
The result-text classifier matched *"[API Error"* which hits review
summaries that quote the pattern in prose (e.g. reviewing PR #7247 whose
summary mentions "[API Error: ...]" and "quota … limit"). The quota
grep then fired on the coincidental "quota … limit" substring, falsely
reporting quota exhaustion on a successful review.
Require a digit after "[API Error: " so only real API error messages
(e.g. "[API Error: 429 …]") trigger the failure path.
* fix(ci): anchor API error detection on tail position, not status-code shape
The status-code glob from the previous commit would silently miss real
aborts whose message has no leading digit (Connection error, Status:
suffix, Chinese rate-limit text) — trading a loud false positive for a
silent false negative.
Anchor on position instead: an aborted run renders the API error as (or
at the very end of) the result text, while a successful review that
discusses API errors quotes them mid-prose. Checking only the last 600
bytes separates the two without constraining the error message format.
Also adds fixtures for the non-digit-leading shapes and for prose
quoting a real status code mid-body, and removes a stale comment.
* fix(ci): catch aborts longer than the tail window; grep full text for quota
Add a whole-result check (case "$RESULT_TEXT" in "[API Error: "*)
alongside the tail check so errors whose body exceeds 600 bytes are
still detected — the prefix falls outside the tail window but the
result starts with it.
Move the quota grep back onto the full RESULT_TEXT so a long error
with quota wording early in the message is still classified as quota
(not downgraded to retryable).
* fix(ci): anchor abort detection on trailing shape, not byte window
Replace the prefix+tail-window split with an ends-with check: the
stream-json adapter appends the formatted API error last, so an
aborted run's result ENDS with "[API Error: …]" optionally followed
by a rate-limit guidance suffix. Strip the three known suffixes,
rtrim, then match *"[API Error: "*"]".
This catches the production abort shape (partial review + appended
error) at any error length, without a byte-window constant to tune
or leave untested. Prose that quotes the pattern mid-body keeps
writing afterwards and does not end with "]".
Adds fixtures for the production shape (appended error, long appended
error, rate-limit suffix after the bracket).
* test(ci): pin suffix sync with errorParsing.ts; document ]-ending trade-off
Add a sync test that reads RATE_LIMIT_MESSAGE_BY_AUTH from
errorParsing.ts and asserts all three suffixes appear in the
workflow — prevents silent drift if someone rewords one.
Add a KNOWN-limitation fixture documenting that prose ending with ]
after quoting the pattern is a false positive (accepted trade-off;
the durable fix is checking that the bot comment landed).
The autofix workflow now plumbs QWEN_AUTOFIX_MODEL (with a
QWEN_PR_REVIEW_MODEL fallback) into the report steps, and the prepare
step documents the verification gate's git diff --quiet check in a
comment. Update the two stale assertions so they match the workflow
again without dropping their original intent.
A fork PR the autofix bot itself opened (its codex flow pushes to
qwen-code-dev-bot/qwen-code) is the bot's own generated work — same
author, same code provenance, and the bot holds write+ — so it is
trust-equal to an in-repo bot PR. Requiring a manual autofix/takeover
label on it was redundant: in-repo bot PRs are auto-managed with no
label, and the takeover label exists to authorize EXTERNAL (human)
fork authors, not the bot's own.
Now a fork authored by AUTOFIX_BOT with 'Allow edits from maintainers'
is admitted and managed WITHOUT a label:
- Scan: fork candidates are unioned from bot-prs.json (the bot's own
forks — --author AUTOFIX_BOT, so no label needed) AND the
takeover-labeled list (non-bot forks, explicit opt-in). Both still
require allow-edits and pass the per-candidate live write+ gate.
- Eligibility: the fork chain no longer demands the takeover label when
the author is the bot (the author check already exempts it); it still
demands allow-edits + a live write+ author + a matching live head
repo.
autofix/skip still opts any such PR out. Non-bot forks are unchanged —
they still need the explicit label.
Tests: the fork-candidate union admits a bot fork (no label) + a
labeled human fork, dropping no-allow-edits/in-repo/skip; the
eligibility replay makes a bot fork with allow-edits eligible without a
label and discards it without allow-edits. 62/62 + 12/12.
Co-authored-by: wenshao <wenshao@example.com>
* feat(ci): auto-open a deflake fix issue for confirmed flaky tests
The CI Failure Patrol reruns flaky failures but never fixes them, so
the same tests flake forever on a rerun treadmill. This closes the
loop: when the patrol classifies a rerun as a nondeterministic TEST
(not infra), it now also opens ONE deflake issue that the existing
autofix issue pipeline develops into a reviewable stabilization PR.
- ci-flaky-patrol SKILL: a rerun decision whose cause is a specific
named flaky test carries an optional flakyTest {file, name}; infra
reruns (ENOSPC, network, runner death) never do.
- ci-flaky-rerun.mjs: validates flakyTest (malformed → the whole
decision is rejected, so a bad classification can't open a bogus
issue); after a rerun, ensureDeflakeIssue upserts a deflake issue
deduped by a stable (file, name) marker — one open issue per flaky
test across all PRs — labeled status/ready-for-agent + autofix/
approved so the scheduled autofix scan picks it up.
- .qwen/skills/deflake/SKILL.md: constrains the fix to four
assertion-preserving patterns (raise timeout/poll budget, stabilize
timing/waiting, make randomness/time deterministic, isolate
interference) and forbids skipping/deleting/loosening the check;
write failure.md if none applies or the failure looks like a real
bug. The produced PR is reviewable, never auto-merged.
Tests: deflakeKey stability/collision-freedom, the bilingual issue
body, one-issue-per-test dedup, no issue for infra reruns, and
malformed-flakyTest rejection. 34/34 across both patrol suites.
* fix(ci): deflake review hardening — rerun survives bad metadata, no markup injection
Addresses the two Criticals + suggestions on #7231:
- **Critical: a malformed/over-length flakyTest no longer kills the
rerun.** flakyTest validation is removed from validDecision (which
gated the PRIMARY action on secondary metadata — a >200-char nested
test name or a null silently dropped a valid rerun). Well-formedness
is now checked in ensureDeflakeIssue, which simply skips the deflake
issue when the metadata is bad; the rerun always stands.
- **Critical: markup/mention injection via the test path/name.** file
and name are code-span-stripped of backticks (which cannot be escaped
inside a span and would break out into live Markdown, turning
into a mention in a bot-created issue) and both now sit in
code spans. safeReason alone did not close this (it does not touch
backticks).
- Best-effort deflake: ensureDeflakeIssue is wrapped in try/catch so a
transient createIssue failure — after the marker is already posted —
no longer surfaces as a misleading "skipping PR" and permanently
suppresses the deflake; it retries on the next flaky occurrence.
- Run link uses the patrol's own repo (client.repo) instead of the dead
target.repo, so deflake issues on a fork don't 404.
- Body reworded: it no longer claims the rerun already passed (it runs
right after the rerun is triggered) — it says a real deterministic
failure is NOT flakiness and must not be stabilized.
- SKILL: bound file/name to 200 chars, and note a malformed one is
ignored (never drops the rerun).
Tests: malformed flakyTest keeps rerun (no createIssue); long title
truncates ≤240; backtick path/name cannot inject; run link honors the
repo; a throwing createIssue leaves the rerun intact. 38/38.
---------
Co-authored-by: wenshao <wenshao@example.com>
* test(autofix): exercise the SKILL stage↔resolve contract end-to-end
Follow-up to #7225, implementing the reviewer's non-blocking
suggestions.
The staging guard #7225 added pins the mirrored LAYOUT but
re-implements run-agent.mjs's `<dir>/../SKILL.md` convention in the
test. If that coupling ever moves in the RUNNER (e.g. ../../SKILL.md),
the string test stays green while prod breaks again — the same class
of blind spot that let #7165 ship. This adds the one check that
exercises the contract for real: stage the actual runner into a
mirrored tmp layout, run it with --print-prompt, and assert it reads
the staged SKILL (sentinel body + resolved skill dir). The negative
case — the flat layout #7165 shipped — is asserted to crash with
ENOENT, proving the test catches that regression.
Also replaces the brittle fixed-width `[\s\S]{0,200}` bound between
`core.hooksPath .husky` and the runner invocation with a direct
ordering assertion (indexOf), so adding a comment between the two
lines can no longer fail the test spuriously.
61/61 + 12/12.
* test(autofix): harden the stage↔resolve integration test per review
Applies all four inline suggestions on #7227:
- spawn process.execPath, not the bare 'node' string, so a
version-manager shim or a PATH without node can't turn the test into
an opaque 'null !== 0'.
- nest the flat-layout runner under dir/flat/ so its ../SKILL.md
resolves to dir/SKILL.md (never created) instead of a shared
tmpdir()/SKILL.md that a concurrent job could leave behind and make
the negative case pass spuriously — a real flake in the deflake-test
itself.
- reuse the existing withRunnerDir helper instead of duplicating its
mkdtemp/try/finally/rmSync.
- bound each spawnSync with timeout: 10_000 so a hung runner fails the
test instead of the whole CI job (spawnSync blocks the event loop, so
vitest's async timeout can't fire).
61/61 + 12/12.
---------
Co-authored-by: wenshao <wenshao@example.com>
* feat(review): retry transient API failures once; surface quota clearly
The PR-review workflow failed permanently on any single API hiccup — a
502/503, a dropped connection, a rate limit — with only a fallback
comment, so an idle PR sat without a review until someone re-ran
`@qwen-code /review` by hand.
Now the qwen invocation runs in a budget-guarded retry loop:
- A transient outcome (non-quota API error, dropped/aborted run, empty
output, error result) retries ONCE after a 60s backoff.
- All attempts SHARE the review timeout budget, and a retry is capped
at 5 minutes — a cleared transient succeeds fast, and a still-failing
retry can't burn another hour (the observed quota run took 61 min, so
an unbounded second attempt could blow the job timeout).
- A quota-exhausted 429 is NOT retried in-run: its reset is typically
hours out. It fails with kind=quota and the fallback comment now
states the reset time and how to re-run once it resets — a clear
recovery instead of a dead end.
- A real timeout or a hard/config failure never retries (unchanged).
Detection is exactly as before; only the disposition (retry / quota
kind) is new. Behavioral test spawns the extracted loop under bash with
a scripted stub qwen: success→1 try, transient→2 then success,
persistent-transient→2 then fail, quota→1 try + quota kind + reset
time, error-result→retry, hard-exit→no retry. 7/7.
* fix(ci): guard the quota-detail grep, sync the timeout assertion, harden the retry tests
Review follow-through on #7233's two Criticals and every suggestion:
- The quota-detail grep ran unguarded in an assignment under
`set -euo pipefail`: a 429 whose message lacks "reset at" exited the
step before fail() wrote failure_kind, so the quota-aware fallback
never fired. Guarded with `|| true`, and a quota_noreset scenario
reproduces the exact message shape that died.
- qwen-resolve-workflow.test.js still asserted the pre-refactor literal
`fail "… ${QWEN_TIMEOUT} minutes." 1 "timeout"`; it now pins the
OUTCOME/REASON pattern the loop actually uses.
- The timeout REASON reported the total budget even when a 5-minute
retry cap fired; it now names the attempt's own timeout beside the
budget, so the fallback's --timeout advice matches what actually
expired.
- Quota matching tightened to quota-plus-context (exhaust/exceed/limit/
reset) so a transient "quota configuration" style error keeps its
retry; the new test file uses the root `yaml` dependency instead of a
hoisted js-yaml, carries the license header, anchors the loop
extraction on the retry-budget comment instead of lastIndexOf, and
gains the two uncovered scenarios: a real timeout is not retried, and
an attempt with under 30s of budget never starts.
---------
Co-authored-by: wenshao <wenshao@example.com>
