208 commits

Author	SHA1	Message	Date
Shaojin Wen	cc508dd7ba	fix(autofix): salvage race-lost pushes by merging the moved head and retrying (#8042) ... * fix(autofix): salvage race-lost pushes by merging the moved head and retrying The review-address push is one-shot: when anything pushes to the PR head during the agent's ~50-minute window, the final push dies 'fetch first' and the entire verified agent run is discarded. The per-PR head-write concurrency group cannot prevent this — it serialises this repo's workflows, not the PR author or the fork side. Observed twice in one day (#7983 after a 56-minute run, #7985 after 43 minutes). On rejection, fetch the moved head, merge it into the local line, and retry (bounded at 3 attempts). Merge rather than rebase: the agent's own conflict-resolution rounds create merge commits, and a rebase would flatten them and can silently re-introduce the conflicts they resolved. The merge result descends from the remote head, so the retried push is a fast-forward and rewrites nothing. A genuine content conflict aborts the merge and falls through to the existing failure path unchanged. When a salvage merge happened, the round report discloses that the round's verification predates the merge so mid-run commits get re-checked by a human. * fix(autofix): address salvage-loop review findings - Gate the PUSH_RACE_MERGED disclosure on HEAD actually advancing: a transient push failure (upload timeout, 503) on an unmoved branch no-ops the merge ('Already up to date') and must not tell the reviewer to re-check mid-run commits that never existed. - Annotate the salvage fetch failure with ::error:: like the two adjacent failure paths, so a deleted fork branch or network error does not kill the step with an unannotated exit 128 under bash -e. - Re-pin the same-repo push URL construction in tests: it lost its old 'origin "${BRANCH}"' pin in this rework, leaving a ${REPO}→${HEAD_REPO} mutation (malformed remote in the same-repo case) unkillable. * test(autofix): restore dropped mutation-killing pins and add structural assertions (#8042) * test(autofix): pin exit 1 in the give-up guard regex to kill the deletion mutation (#8042) * test(autofix): pin exit 1 in the fetch-failure and merge-conflict salvage paths (#8042) * test(autofix): strengthen salvage-test pins to kill init-value and capture-order mutations (#8042) --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-07-30 11:38:47 +00:00
Shaojin Wen	f6ae9202b1	feat(autofix): per-source feedback budget in Critical-only mode (#8071) ... * fix(autofix): never defer maintainer feedback in Critical-only mode Critical-only mode (after 5 change-producing rounds) classifies feedback lexically: only a literal **[Critical]** tag or a CHANGES_REQUESTED review survives; everything else is deferred before the agent reads a word of it. That rule was built to stop the review bot's suggestion ping-pong, but it catches maintainers too. Observed four times in two days (#8037, #7944, #7885, #7799): a maintainer's review with explicit merge-blocking findings — #8037's said 'I'd fix before merge' on a correctness bug and a security-adjacent one — was wholesale-deferred as one 'non-Critical item', and the bot then reported 'No Critical feedback. The Issue-level comments sections are empty', which was lexically true and substantively false. The bot's own advertised definition ('correctness bugs, security issues, or formally requested changes') is exactly what the deferred comments contained; the agent that could have applied that definition never saw them. The lexical test now applies exclusively to the review bot's output: - All three actionable filters (reviews, inline, issue-level) pass anything not authored by the review bot straight through in Critical-only mode — the agent judges maintainer feedback on content, as everywhere else. - All three deferred-list builders keep only review-bot items, so a maintainer comment can never appear as an 'audit record'. - The deferral note says what is actually deferred (the automated reviewer's non-Critical suggestions), states that maintainer feedback is never deferred, and names the exit (@qwen-code /retry opens a fresh counting window). - SKILL.md's Critical-only policy now marks everything rendered in the actionable sections as in scope, so the agent does not re-refuse what the filter passed through. Behavioral test updated both ways: maintainer comments/reviews stay actionable in Critical-only mode across all three sources, bot suggestions still defer, and structural pins hold the bypass and the bot-only select in all six filters. * feat(autofix): per-author feedback budget in Critical-only mode Follow-up to the author-based split, prompted by the obvious counterexample: a human account can host an automated reviewer loop with the exact regeneration property the review bot has — feedback re-generated after every push at zero marginal cost — so 'not the bot' cannot mean 'never throttled'. An account is an accountability unit, not a throttle; the brake has to key on measured regeneration. Unified model: once Critical-only engages, every source has a bounded budget of untagged feedback batches per counting window. The review bot's budget is zero (all deferred, as before). A human's is CRITICAL_ONLY_HUMAN_BATCHES (2) CONSUMED batches: feedback items are bucketed into the (prev marker ts, marker ts] span that evaluated them, only spans from Critical-only rounds count, and an author needs K distinct consumed spans before their new untagged feedback defers. Fresh unevaluated feedback never counts against its own author, and the census is window-scoped, so /retry resets the budget with the window. The observed cases (#8037/#7944/#7885/#7799 — one or two late verification reports each) stay fully served under K=2; a looped reviewer is throttled after 5+K driven rounds instead of grinding to the 100-round cap. Past the budget, continuing requires one conscious act — **[Critical]**, a Request changes review, or /retry — which is precisely what separates intent from automation. Over-budget authors are named in the deferral note with those exact escapes. Tests: the six filter replays gain over-budget cases both ways (the tagged/CR escapes survive even over budget), and the budget census itself is replayed over fixture files — two consumed critical-tail batches list the author; one batch, pre-Critical batches, unconsumed feedback, untrusted authors, and command comments never count. * fix(autofix): fix deferred-feedback bash quoting and drop a dead jq binding (#8071) * test(autofix): exercise census window-isolation guard with a stale-window fixture (#8071) * test(autofix): make census command-exclusion observable; surface census stderr (#8071) * fix(autofix): exclude never-deferrable feedback from the budget census (#8071) The Critical-only per-author budget census counted every trusted review, inline comment, and issue comment, including feedback the deferred renderer would never defer: **[Critical]**-tagged comments, Request changes / APPROVED reviews, inline replies rooted at a Critical comment, and inline comments attached to a Request changes review. A maintainer who followed the documented escape hatches (tag Critical, request changes) thereby spent their own budget and had later untagged feedback silently deferred — the exact bug this PR fixes, re-created one level down. Mirror the three deferred-builder predicates in the census item filter so a batch is counted only when it is actually deferrable. Extend the census replay test with protected authors (Critical-only, Request changes, APPROVED, Critical-rooted replies, Request-changes-review inlines, the review bot as a trusted MEMBER, and a sentinel-ts marker probe) that each carry two consumed-span batches yet must stay absent, so dropping any one exclusion now fails the suite. Also fold bash's stderr into the bash -n guard assertion so a future quoting regression reports the syntax error, not just a non-zero exit. --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-30 11:38:40 +00:00
Shaojin Wen	4dab39c8d7	fix(autofix): answer round-cap refusals on the PR instead of only in logs (#8067) ... * fix(autofix): answer cap-gate refusals on the PR instead of only in logs Observed on #7836: the fleet shepherd detected a merge conflict, posted 'dispatched the autofix loop to resolve it', and the dispatch died at the scan's round-cap gate with only a log line — the PR page showed a promise, the run showed green, and the conflict sat unhandled for hours. Three silences stacked: the standard-management cap itself is silent (the pause notice was takeover-only, so #7836 hit 10/10 with zero PR-visible notice), the forced-dispatch refusal is silent, and the shepherd dedups per head SHA — a capped PR gets no pushes, so its head never changes and conflict handling froze permanently. Two scan-side changes (the shepherd stays untouched — the windowed round computation lives in the scan and duplicating it would drift): - A FORCED dispatch (shepherd conflict lever or a human) refused at the cap gate now answers on the PR: cap value, what stays unhandled, and the two recovery commands (/retry for a fresh window, /takeover for the raised cap). No dedup — the shepherd sends at most one dispatch per head, and a human asking twice deserves two answers. - The cap pause notice covers ALL managed PRs: the takeover variant keeps its wording, standard bot PRs get their own (/retry or /takeover). Same marker, same once-per-window dedup, same consent and PAT-identity checks — skip wins everywhere, and only the takeover variant requires the label to still be present. After a re-arm the next scheduled scan picks the PR up normally (conflict targets are label-independent), so the frozen-head loop resolves without any shepherd change. * test(autofix): replay the cap-notice consent gate across label/takeover permutations (#8067) * fix(autofix): gate the loud cap-refusal on workflow_dispatch (#8067) FORCED_PR is populated for every trusted pull_request_review (route emits pr_number for those), not just workflow_dispatch, so on a capped PR each review submission landed in the un-deduped refusal branch — 7 "Dispatch refused" comments on #7836 where 2 carried the information. Answer only workflow_dispatch (the shepherd lever or a human); review submissions stay covered by the once-per-window pause notice. Adds a verbatim behavioral replay of the guard so a dropped EVENT_NAME condition fails the test. --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-07-30 09:46:45 +00:00
Shaojin Wen	b64a6c4f13	fix(verify-pr): budget evidence capture in scope selection (#8104) ... Post-merge measurement of #8016, on the first two live runs after it landed (#7975 and #8066): the browser installed successfully — "Install evidence browser: success" in both — and both reports carried **zero images**. #7975 has 31 tables, #8066 has 407 assertions. Neither has a witness. The cause is the third instance of one structural bug, and it is the same one twice already fixed: #7917 the /verify recommendation sat inside a section headed "local invocation ONLY" -> 1/16 reach #8016 captures were "Optionally ... when text cannot carry the oracle" -> 0/14 now captures are a required deliverable in the ARTIFACT CONTRACT, but the plan the agent executes is the Scope-selection budget list, and that list had zero mentions of evidence, png, capture or screenshot An agent that budgets by that list never allocates time for a capture, however firmly a later section says to produce one. So captures are now budget item 4 alongside the A/B, the harnesses and the gates, with the time reserved (~5 min), the gate named (QWEN_VERIFY_CHROMIUM=1), the count bounded (normally two, at most a handful), and the measured failure stated so the rule carries its own reason. The report structure gets the matching half: the A/B table item now says to reference the capture of those cells by filename. A produced image with nowhere to go is as dead as an unproduced one. Mutation-verified 4/4: dropping the budget item, the time allowance, the count bound, or the report-side reference each turns the test red. Not changed, and worth stating rather than implying: the seven techniques from #8010 do not appear in either report. I do not read that as failure — every one of them is conditional ("when one fix bundles two changes", "when the oracle is an instrument", "when a PR adds a writer to a shared store") and neither PR met those conditions. #8010 is not yet falsifiable from these runs. What both reports DO show is older skill rules working: a naive-matcher differential in #8066, and #7975's realpath check finding node_modules/@qwen-code/qwen-code-core resolving into the HEAD tree and then reasoning about whether that contaminates the control. 107/107 tests; prettier and eslint clean. Co-authored-by: wenshao <wenshao@example.com>	2026-07-30 07:50:14 +00:00
Shaojin Wen	f4cd6e1d8b	fix(ci): gate the attachment guard before it allocates a runner (#8095) ... Measured on a congested pool: 88 active jobs, 72 hosted and 15 self-hosted. The self-hosted 15 were all running with zero queued; the hosted 72 were contending, and 20 of them were the SAME job — `remove-suspicious-attachments`, all queued, none running. Its real cost is not the work. Recent completed runs: queue=629s run=5s queue=568s run=2s queue=518s run=2s queue=340s run=3s Two to five seconds of API calls behind up to ten minutes of queueing. And almost none of it needed to happen. The trust check lived INSIDE the github-script, so a runner was queued, allocated and started before the job could decide it had nothing to do. Over the 200 most recent comments on this repo: 184 from trusted associations, 9 from bots, 7 actually needing a scan. 96.5% of these runs existed to print "Trusted author; skipping". Two changes: - Hoist the association and bot checks into the job `if:`. GitHub evaluates `if:` BEFORE allocating a runner, so a trusted comment now costs nothing. The script keeps its own copies: the gate is an optimisation, not the control, and the two must be able to disagree without becoming unsafe. Every ambiguity therefore resolves toward RUNNING the scan — an unrecognised payload yields an empty association, which is not in the trusted list, so the job runs. - Add a per-comment concurrency group with cancel-in-progress. The workflow listens on `edited` as well as `created`, and the bot PATCHes its own comments constantly, so repeated edits of one comment stacked. The scan reads the comment's CURRENT body, so a queued earlier scan is already stale and cancelling it loses nothing. (Contrast the verify lane, where cancel-in-progress is deliberately false because a cancelled run destroys evidence.) The key falls back to run_id so an unexpected payload gets its own group instead of serialising every scan into one. Deliberately NOT moved to the self-hosted pool, though it would fit technically (no checkout, no PR code, API calls only): the 20 stacked jobs were duplicates, so relocating them just fills the ECS pool instead — and that pool is what /verify and /triage depend on. It also holds issues:write while processing untrusted comment bodies, which belongs on ephemeral hardware rather than reused machines. The `if:` semantics are verified against all payload shapes — 12 cases covering both `comment.*` and `review.*` associations, bots, and missing/empty payloads, each asserting which direction it resolves. CONTRIBUTOR is deliberately NOT trusted: a merged PR does not make someone's links safe. Mutation-verified 6/6: dropping the review payload path, dropping the bot check, adding CONTRIBUTOR to the trusted list, turning off cancel-in-progress, collapsing the group to a global key, and inverting the gate so untrusted comments are the ones skipped — each turns a test red. The last is the one that matters; it is the only mutation here that would be a security regression rather than a cost regression. 148/148 tests across both suites; actionlint exit 0; prettier and eslint clean. Co-authored-by: wenshao <wenshao@example.com>	2026-07-30 06:32:55 +00:00
Shaojin Wen	6b0d2bbca0	feat(triage): make /verify evidence screenshots actually possible (#8016) ... * feat(triage): make /verify evidence screenshots actually possible Measured across the 14 real verify reports on this repo: tables appear in 14/14 (4 to 31 rows), images in 0/14. The hosting machinery has been complete the whole time — pr-assets branch, PNG magic check, 8 images, 2 MB cap, sanitised names — and it has never once been fed. Two independent causes, both fixed: 1. The agent physically could not produce a PNG. It runs as `node` under `env -i` with HOME pointed at a per-run scratch dir, so `playwright install --with-deps chromium` needs an apt it has no rights to and would re-download ~170 MB into a directory deleted at the end of every run. Chromium is now installed by ROOT in the tools step into a shared, world-readable PLAYWRIGHT_BROWSERS_PATH, and the agent env carries that path plus QWEN_VERIFY_CHROMIUM=1 — but ONLY when the install actually succeeded, so the variable's absence is a real signal rather than a stale promise. The install is best-effort: a failure warns and the run continues to a text-only report, which is what every run has produced anyway. 2. The skill discouraged it. Captures were "Optionally … use them when text cannot carry the oracle", with TUI rendering as the only example — so an agent verifying a daemon or a CLI correctly concluded text carried the oracle and skipped. It now says produce them whenever a harness ran, and says WHY: a table is the agent's claim about what happened; a capture of the run is a witness that the numbers came from a real execution, which is the part a reviewer cannot get any other way. It names the three highest-value shots (A/B cells side by side, the mutation matrix as printed, raw harness output behind a headline number) and forbids `playwright install` explicitly, so the failure mode of cause 1 cannot be rediscovered by an agent burning budget on it. Affordable now because #8014 raised the agent budget to 120m; at 20m a browser download alone was most of the run. Mutation-verified 4/4: handing the agent the variable unconditionally, failing the job on a chromium install failure, reverting the skill to "Optionally", and dropping the do-not-install warning each turn the test red. 90/90 tests; prettier, eslint, actionlint and shellcheck clean. * fix(triage): match browser to lockfile and fix pr-assets branch conflict (#8016) Split the chromium install into system deps (tools step, as root) and browser binary (new post-checkout step, as node using the checkout's own Playwright). This eliminates the version pin that produced an unusable browser tree. Change evidence hosting from a bare pr-assets leaf branch — which cannot coexist with the existing pr-assets/* namespace — to per-PR branches (pr-assets/pr<N>-verify) with orphan-init fallback. Update terminal-capture skill to warn CI agents away from the forbidden playwright install command, and close test blind spots for conditionality (M1b), version matching (M5), and marker ordering (M6). * fix(triage): unpin Playwright install-deps so apt list tracks lockfile browser (#8016) * fix(triage): assert the browser path reaches the agent, not just the flag Review finding on #8016, verified by mutation before accepting it: deleting `"PLAYWRIGHT_BROWSERS_PATH=$CHROMIUM_PATH"` from the agent env left the test green. That is the nastiest arm of this feature — the agent is TOLD chromium is available via QWEN_VERIFY_CHROMIUM=1, then Playwright looks in the default ~/.cache/ms-playwright instead of the shared install and every capture fails, so a successful install still degrades to a text-only report. The test now asserts both variables and that BOTH sit behind the success guard. Mutation-verified 2/2: deleting the path line fails on the containment assertion, and hoisting it outside the guard fails on the ordering one (`expected 21928 to be less than 21834`). Not changed, because the other two findings do not describe this codebase — replies on the threads carry the evidence. 90/90 tests; prettier and eslint clean. * fix(triage): cover orphan-init delivery and delete verify asset branches Two review findings on #8016, both about the per-PR branch scheme the autofix bot introduced on this branch (029beba3ef), and both correct. **Orphan-init's success path had no test.** The existing scenarios both take the clone-failed branch, but one seeded the branch (so clone succeeds) and the other points at a non-repo (so the push fails too) — neither proves orphan-init can DELIVER. A third scenario now points at a valid bare remote with the branch deleted first: clone fails, orphan-init runs, and the images land. It also asserts the branch has exactly one commit, so a graft onto unrelated history would fail rather than pass as "orphan". Without it, a dropped `git init` or `remote add origin` would silently discard every image on every PR's first verification run. Mutation-verified: dropping `remote add origin` and dropping `git init` each kill it. A third mutation — pointing `checkout --orphan` at the wrong branch name — SURVIVES, and correctly: the push is `HEAD:$assets_branch`, which names its destination explicitly, so the local branch name cannot affect delivery. Recorded as a non-defect rather than papered over with a test that pins an irrelevant detail. **Per-PR asset branches were never deleted.** `pr-assets/*` had exactly one producer and one deleter; the verify lane became a second producer without being added, so every verified PR would leave a single-commit branch in the base repo permanently. The cleanup workflow now deletes both refs, is renamed to match what it does, and carries a note that every new `pr-assets/*` producer must be added — a branch nothing deletes is permanent. The loop deliberately drops `set -e`: most PRs produce neither branch, so a 404 on the first must not skip the second. A real delete failure still warns and sets a non-zero exit. The new test executes the script against a stubbed `gh` where the visuals branch 404s, and asserts the verify branch is still deleted — which a `set -e` version would fail. 92/92 tests; prettier and eslint clean. * fix(triage): match browser install to the harness Playwright and re-arm the strip guard (#8016) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(triage): gate chromium marker on deps and resolve Playwright from the harness (#8016) Address review feedback on the verify evidence-browser path: - Resolve the Playwright CLI from the capture harness's own directory (require.resolve of the exported package.json, then join cli.js) instead of assuming npm hoists playwright to the root, so the installed binary tracks the harness's import even if the hoist layout changes. cli.js is not in the package's exports map, which is why a direct path was used before; resolving the exported package.json keeps that bypass while removing the hoist assumption. - Gate the verify-chromium-path marker on a new verify-chromium-deps-ok marker written by the tools step. apt and the Playwright CDN are independent servers with no shared success signal, so a binary download alone must not promise chromium to the agent when system deps failed. - Add a delete-failure case to the PR asset cleanup test so the status=1 and exit path is executed, not just statically asserted. * fix(triage): strip cache creds from browser install and guard cli.js resolution (#8016) The evidence-browser install runs the PR-resolved Playwright CLI but did not drop ACTIONS_RUNTIME_TOKEN/URL/CACHE_URL the way the adjacent prepare and agent steps do, leaving runner-injected cache credentials readable by PR-controlled code. Strip them in the same env -u chain. Also replace the string-only guard on the require.resolve + cli.js join with an execution guard that resolves the expression against the installed tree, so a Playwright bump that relocates cli.js fails the test instead of silently degrading evidence capture at runtime. --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-30 05:37:42 +00:00
Shaojin Wen	cfcc711a70	feat(autofix): back off scan inspection of idle candidates (#8049) ... * feat(autofix): back off scan inspection of idle candidates The scheduled scan inspects every candidate every tick. The takeover pool doubled in two days (28 open takeover PRs, 8 of them idle in 'nothing new' state for 10+ hours), and idle candidates crowd the two SHARED budgets: MAX_CANDIDATE_INSPECTIONS (60) and the 10-target cap. Observed on #8002: freshly engaged, admitted by the 09:03 scan, then deferred by the target budget while long-idle PRs re-confirmed their idleness yet again. Candidates whose list-provided updatedAt (no extra API call) is older than 24h are now inspected on roughly every 4th scan, on a deterministic slot keyed by PR number and UTC hour so no PR waits forever. The skip is free — it sits with the busy skip before the inspection-budget increment. Safe by construction: every real wake-up bumps updatedAt (reviews, comments, labels, pushes) or routes in real time anyway, so the only thing deferred is the scheduled re-confirmation of idleness plus worst-case a few hours of base-conflict-detection latency for a PR nobody touched in a day. The forced-dispatch path never builds the list files, so a forced PR is always inspected. * feat(autofix): idle-backoff review follow-ups - Corrected the comment's cost model: idle candidates hit 'continue' before the TARGETS append, so they never contend for the 10-target cap — the real win is the shared inspection budget plus the serial scan-walk latency (the walk is what delayed #8002's pickup by ~6 minutes), and the comment now says exactly that. - Slot quantum changed from the hour to the scan tick (600s, the same quantum as ROT_OFF): an hourly slot against the */10 cron meant 6 back-to-back inspections then a ~3h blind window per PR — same 25% average, terrible shape. The gap is now bounded at ~30 minutes, which is what the operator-facing strings promise ('gap ≤30m'). - The two scan-only signals updatedAt cannot see (a base conflict appearing when main moves; still-red checks awaiting the redcheck marker) are named in the comment instead of papered over. - The per-candidate jq fork became a single precomputed set + a bash substring test, matching the busy skip's idiom and the 'free' claim. - Tests: the skip predicate and set builder got a behavioral replay (idle+out-of-slot defers, idle+in-slot inspects, fresh inspects, missing-from-lookup inspects); the two byte-distance assertions became a loop-head slice (comment growth cannot red-light CI, and budget-consuming code between the skips and the increment fails); the --json field pin is order-independent; the 3600 quantum is pinned OUT. * test(autofix): pin the null-updatedAt defensive guard in idle-backoff replay (#8049) * fix(autofix): extract idle-backoff predicate from workflow, fix gap bound 30→40m (#8049) * test(autofix): pin takeover-prs.json in idle-backoff replay (#8049) * fix(autofix): state idle-backoff gap probabilistically, unpin quantum (#8049) Round-2 verification showed the scheduled scan lands every ~40-70 min on this repo, not every 10 min, so the (epoch/600)%4 slot is an independent ~25% draw per scan, not a deterministic 1-in-4 rotation. The gap is geometric (measured median ~2h, p90 ~6h), not bounded at ~40m. Reword the operator-facing strings (skip echo, fleet row) and the workflow comment to state the behaviour probabilistically and drop the false "bounded" / "no PR is unlucky forever" claims; correct the misattributed #8002 latency (queue/startup, not the serial walk). Relax the tests so a truthful number is not a CI failure: pin the mod-4 time-quantum shape instead of the exact 600s constant, and drop the /3600 exclusion that forbade the better-tailed quantum. Mechanism logic is unchanged. --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-07-30 04:26:43 +00:00
Shaojin Wen	d5ac7a9642	fix(autofix): post the takeover engage ack from the command itself (#8043) ... * fix(autofix): post the takeover engage ack from the command itself The engage ack rode a pull_request:labeled round-trip: takeover-command applies the label, the labeled event routes, and the takeover-ack job posts the confirmation. That event has now been observed to simply not fire twice in one day (#7999 — the author read the silence as failure and removed the label; #8002 — an engaged fork PR with no ack for hours), and fork label events can never ack at all since they carry no secrets: a fork /takeover stayed silent until the next scan picked the PR up (2h41m on #7993). takeover-command now posts the engage ack directly after applying the label — every admission gate has already passed at that point, so 'engaged' is truthful for in-repo and fork PRs alike; the fork variant adds the expectation that the first round comes from the next scheduled scan. The route side suppresses the label-path ack when the label sender is the bot (only the ack: the immediate scan still routes), and the review-scan's existing first-pickup ack dedups against the command's comment and heals it if the post failed. Two more silent paths become audible while here: a /takeover on a stacked (non-main-base) PR now refuses out loud instead of dropping with only a log line, and a /takeover stop on a non-main PR now proceeds to remove the label instead of leaving it stuck. * fix(autofix): ack command-driven releases directly and key the scan grace on the label actor Review follow-up: the engage-side fix left the release side on the fragile round-trip — a loud add next to a mute stop re-creates the exact 'did it work or did the event get lost?' ambiguity this PR set out to remove, now on release. And a fork or non-main release could never ack at all (fork unlabeled events carry no secrets; the route ignores non-main releases). - takeover-command now posts the release ack directly after removing the label, mirroring the engage side, with the same three variants as the ack job (plain release / bot-authored / bot-authored+skip) chosen from the same PR_INFO the gates used. The route suppresses the unlabeled-path ack when the label sender is the bot. - The scan's first-pickup grace is keyed on the label event's actor: a bot-applied label defers only 45s (the command's own write is seconds behind the label — fork or in-repo alike), so the immediate scan still heals a failed command ack instead of slipping to the next scheduled tick, and an ic.json snapshot taken between the label write and the command ack cannot double-post. A human in-repo label keeps the 3-minute ack-job grace; a human fork still posts right away. - The fork note now says 'usually within minutes', matching the */10 cron instead of contradicting it by 6x. - Tests kill the two surviving mutations from review: the EN/ZH fork-note swap (per-language-half assertions) and the warning fallback downgraded to '|| true' (both fallback strings pinned). * test(autofix): behaviorally pin the LAST_LABELED_BY jq extraction (#8043) * fix(autofix): correct stale comments about scan heal timing and command blast radius (#8043) * test(autofix): pin release-ack body identity across the command and ack jobs (#8043) --------- Co-authored-by: verify <verify@local> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-07-30 02:41:08 +00:00
Shaojin Wen	c631c0448e	fix(autofix): cumulative timeout breaker, narrowed retry prompt, truthful handoff wording (#8044) ... * fix(autofix): stop repeat-timeout PRs with a cumulative breaker and narrow the retry prompt The consecutive-failure cap resets on every pushed round, so it never fires on the observed failure shape: timeouts interleaved with successes. #7929 burned three 50-minute agent timeouts (with pushed rounds in between) and #7846 two — each a full agent budget with nothing to show, invisible to the breaker by construction. Three changes: - Cumulative timeout breaker (TIMEOUT_WINDOW_CAP=3): time-budget exhaustions are counted over the whole counting window, successes in between included. At the cap the PR goes terminal with a skip-until-/retry headline, exactly like the consecutive breaker. The census reuses PRIOR_HEADS, so it is window-scoped and a re-arm clears it. - Prompt narrowing from the second attempt: when the current window already contains a timeout marker, the prepare step appends a budget warning to feedback.md — smallest blocking subset first, commit early, defer the rest explicitly — instead of re-running the identical address-everything prompt into the same wall. - Truthful gate-rejection handoff: 'A human should take over this PR' read as a full release, but the loop keeps managing the PR (new feedback, base conflicts) — #7929 posted it and then kept pushing rounds. The headline now says exactly which half is over: this item will not be retried automatically, management continues. * fix(autofix): timeout-breaker review follow-ups All six review findings addressed: - The narrowing prompt now restates the SKILL deferral contract instead of pointing at 'your summary': deferred findings stay out of resolved-comments.txt and get a comment-replies.json entry so every open thread carries its reason — the summary-only shortcut is exactly what a budget-pressured agent would otherwise take. 'skip refactors entirely' became 'decline with a one-line reason' for the same never-drop-silently rule. - The narrowing census counts timeouts SINCE THE LAST SUCCESSFUL ROUND, not cumulatively: a push falsifies 'not converging', and one old timeout must not degrade every later round of a 100-round window. The breaker stays cumulative (a push does not make the next timeout cheaper in budget terms). - The breaker headline states what the census measured ('this counting window now contains N time-budget exhaustions') instead of 'stopped after N' — the tripping round can itself be a gate rejection, which is the exact rollout state of #7929/#7846. - Both census needles anchor on the verbatim emitted headline ('AutoFix ran out of time before finishing') — first lines can embed provider error text, so the loose phrase could count a model error as a timeout. - The precedence assertion got teeth: 'consecutive' alone matched both branches; it now asserts the consecutive breaker's own phrase and the absence of the timeout one (the if-true guard mutation now fails). - The narrowing census got a behavioral replay over fixture ic.json: push-resets, trailing counts, legacy no-win markers under 'none', old-window exclusion, author filtering — plus a pin on the -ge 1 trigger so a threshold mutation cannot leave the feature inert. * fix(autofix): state what the timeout breaker measured, pin the inherited guarantees Second-round review follow-ups: - The breaker headline no longer infers 'is not converging' — a window can carry 40 pushes and still trip the (deliberately cumulative) cap, so the sentence now states only what the census measured: N full agent runs that pushed nothing. - Two guardrail tests pin behavior that held only by construction: a non-timeout failure landing on an already-capped window still trips the breaker (the documented rollout state — an 'only count when this round timed out' cleanup would silently delete it), and the transient-API-error exemption inherited from the outer guard (a refactor hoisting the block out of it must not mass-terminate every in-flight PR during a provider outage). * fix(autofix): name a gate rejection only when the gate ran; pin the timeout breaker's stale-base exemption Third-round review follow-ups: - The handoff headline no longer claims 'the verification gate rejected the attempt' for every outcome=failed verdict: that branch is reached for five distinct failures (failure.md abort, dirty tree, unchanged branch, missing summary, and a real gate rejection), only one of which is a gate decision. The clause is now gated on gate-rejection.md, which reject_fix is the sole writer of — so the other four paths keep the neutral framing instead of repeating the very wording-doesn't-match-behaviour bug this PR fixes. A replay test pins both halves: the clause appears when gate-rejection.md exists and is absent when it does not. - The timeout breaker's stale-base exemption is now pinned by a replay (five in-window timeouts plus a stale-base retry current round stays retryable), matching the existing transient-API-error pin: a refactor hoisting the timeout block out of the outer guard would otherwise delete this exemption silently. - The prepare-step narrowing census jq now fails open (2> /dev/null || true) like its report-step sibling, so a malformed ic.json drops the budget warning instead of aborting prepare under errexit and turning the round into a terminal 'could not start evaluation' handoff. - The decay comment now matches the code: the warning resets on a push/no-op round but fires on every failing round until then (gate rejections included), which is correct since nothing has converged. * fix(autofix): clear stale gate-rejection.md in repair cleanup and pin headline template (#8044) * test(autofix): cover no-op reset branch in timeout census (#8044) --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-30 02:41:00 +00:00
Shaojin Wen	25f5147677	feat(autofix): post a takeover milestone digest every tenth pushed round (#8046) ... * feat(autofix): post a takeover milestone digest every tenth pushed round The takeover round cap (100) bounds runaway but carries no signal about when a human should step in: #7469 ground to round 12 over seven days of takeover with fifteen pushes, and the only place that trajectory was visible was the Actions logs. Every 10th pushed round under takeover, the report step now posts a window-scoped census on the PR itself — pushed fixes, no-change reviews, timeouts, rejected attempts, base updates — plus the three options (keep going / split or reduce / release takeover). The digest is a separate comment with its own autofix-milestone marker and no autofix-eval marker, so every census (round, consecutive-failure, watermark) ignores it and the feedback filters keep it out of the agent's prompt. Posting is best-effort: a digest failure never fails a good push. The rejected-attempt count matches both the current and the reworded gate-rejection headline so the census cannot silently zero; base updates carry no win= field and are windowed by timestamp instead (the window key is the engage ack's created_at). * feat(autofix): milestone digest review follow-ups - Residual bucket: every outcome the four buckets missed (crash, model error, gate error, infra) now lands in 'other round(s)' — a window that burned 80% of its budget on crashes was rendering as four zeros, QUIETER than a healthy window, the inverse of the digest's premise. - Crossing trigger: fire on the first pushed round once 10+ rounds have accumulated since the last digest in this window, instead of an exact %10 hit — failure rounds advance the counter too, so push@9/crash@10/ push@11 skipped the digest forever on exactly the failure-heavy PRs it exists for. - The success log is chained to the post; a failed comment no longer logs 'posted' after its own warning. - WINDOW=none says 'since the PR opened (no counting window yet)' instead of claiming a current window while counting all time. - A census that parses zero window markers at round 10+ skips the digest instead of posting a fabricated all-zero one. - autofix-milestone joins BOT_COMMENT_FILTER (marker inventory). - The timeout needle anchors on the verbatim emitted headline. - The pin-only test became a behavioral replay: the digest block runs under bash with a stubbed gh over fixture ic.json histories — bucket counts, residual loudness, crossing suppression/resumption, old-window isolation, none-window phrasing, non-pushed/non-takeover gating, and the empty-census skip. The OUTCOME == fixed conjunct is pinned. * test(autofix): cross-pin digest census needles to headline emission sites (#8046) * test(autofix): widen rejected-headline cross-pin to match #8044 reword (#8046) * test(autofix): behaviorally cover the digest comment-failure branch (#8046) --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-07-30 02:40:55 +00:00
Shaojin Wen	36fe53d421	feat(triage): raise the /verify agent budget from 25m to 120m (#8014) ... * feat(triage): raise the /verify agent budget from 25m to 120m Maintainer decision: give /verify enough time to do what a maintainer's local round does, and add machines if the pool gets tight. Three values encode this one budget and each has its own silent failure when edited apart, so all three move together: - agent kill 25m -> 120m (the graceful budget; ships a partial report on expiry) - watchdog secs 1500 -> 7200 (distinguishes that kill from an OOM; left low, a late 137 is mislabelled `timeout` and publishes "partial evidence" for a crash) - job timeout 60m -> 150m (guards infra hangs only; it must clear agent + install/build + overhead or it kills the container mid-run and the ship-what-ran path never executes) 150 rather than 130: measured install+build is ~6m (run 30284341325, npm ci 3m00 + build 2m40), budgeted at 15m for a cold cache, plus ~5m of tools/checkout/pin/upload/cleanup — worst case ~140m, leaving 10m of headroom. The skill's advertised budget moves too, and this is the part that actually changes behaviour: it read "≈ 20 minutes (hard 25-minute kill)", and an agent obeying that would have self-limited no matter what the workflow allowed. It now reads 110/120 and says what the extra time is FOR — bisecting a threshold through the real code path, compiling an intermediate build to separate the halves of a bundled fix, adjudicating mutation survivors, driving a real daemon end to end. It also says plainly that spending it on breadth is the one way to waste it; the rule that one proven load-bearing claim beats ten unverified observations does not relax because the clock did. Pinned by a new test that asserts the RELATIONSHIPS rather than the numbers: watchdog == agent budget in seconds, job >= agent + 20m, and the skill's advertised hard kill == the workflow's, with the soft budget strictly below it. Mutation-verified 4/4 — leaving the watchdog at 1500, the job at 60, the skill at 25m, or raising the soft budget above the hard kill each turn it red. Cost, stated so it is a decision and not a surprise: a verify run now holds one ECS slot for up to 2.5h instead of 1h. Concurrency is unchanged (per-PR group, cancel-in-progress false), so this reduces how many distinct PRs can verify at once, not how many runs a PR can queue. 90/90 tests; prettier, eslint and actionlint clean. * fix(ci): pin soft-budget lower bound in verify consistency test (#8014) * fix(ci): harden verify budget guard and clarify budget is a ceiling (#8014) * fix(ci): measure elapsed before artifact copy and tighten budget guard (#8014) * test(ci): pin elapsed-time chain in verify budget drift guard (#8014) * refactor(ci): derive verify watchdog threshold from the agent budget (#8014) The watchdog threshold that tells a graceful timeout (137 at the budget) from an OOM kill (137 before it) was a bare `7200` coupled to the `120m` agent timeout only by a comment. Define the budget once as AGENT_BUDGET_M and derive both the timeout (`${AGENT_BUDGET_M}m`) and the threshold (`$((AGENT_BUDGET_M * 60))`) from it, so editing one cannot silently desync the other. The consistency test now asserts that derivation rather than re-extracting two independent literals. Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-07-30 02:40:34 +00:00
Shaojin Wen	0232e73811	feat(verify-pr): add seven techniques from maintainer verification rounds (#8010) ... * feat(verify-pr): add four techniques from maintainer verification rounds Two hand-written maintainer rounds contained methods the skill could not have produced. Checked each against the current text before adding it; these four had no coverage at all. From #7914 (live daemon A/B on the artifact-recording change): - Run every control on BOTH arms. That round's sharpest finding came from a control whose only job was to validate the BASE probe — "the empty list is a real absence, so have the model call record_artifact and watch an entry appear". Run on head as well, it showed the curated title being silently discarded. The control was not hunting for a bug; running it symmetrically is what found one. - A new writer into a shared store is an ordering change. The PR added write_file as a second writer into the artifact list; the bug was not in the new writer but in the collision, where a pre-existing first-writer-wins merge began discarding record_artifact's curated title and description while still reporting success. Enumerate the other writers, exercise the collision in both orders, and check what the loser is told — and separate the pre-existing cause from the PR's contribution so the author is not blamed for the policy. From #7998 (ink cursor fix, real-terminal A/B): - When the oracle is an instrument, corroborate it with a mechanism that does not use that instrument. The hardware cursor row came from `tmux display-message -p '#{cursor_y}'`, then from a marker printed after the TUI exits — which lands wherever the cursor actually was. Two agreeing instruments turn a measurement into evidence; one tool's report about the system is not the system. - Re-run the generator on committed generated artifacts and diff. That round re-ran `npx patch-package ink` and found byte-different hunk headers, proving the .d.ts hunks were hand-written rather than regenerated as the description claimed. Also strengthens Not covered: proving a limitation is environmental requires an A/A control (boot base and head identically, show both fail the same way), because "seems environmental" and a real regression look identical in a report. Mutation-verified 4/4, each with landing proof. Two initially reported `landed: False` — the assertions match the whitespace-normalised text while the rules wrap across lines in the source, so the replace never fired and the green result proved nothing. Re-run against line-accurate anchors, both kill. 89/89 tests; prettier and eslint clean. * feat(verify-pr): teach the timing-race and scenario-arrival checks Third maintainer round mined for method (#7934 R4). The blocker it found had zero coverage in the skill — `timer`, `wall-clock`, `flake`, `retry`, `duration`, `deterministic` all returned 0, and the one `race` hit was a substring of "trace". - **Timing-triggered assertions have a threshold — measure it, do not sample it.** A new guard (`expect(false).toBe(true)` after an abort loop) turned a vacuous pass into a deterministic failure, because the case triggers its abort from `setTimeout(..., 1000)` while the query's duration is set by CLI startup rather than the server. Natural completion measured 730-2151 ms, so every box on the fast side of 1000 ms fails. The rule says to measure the operation's natural duration with the trigger disabled and compare it to the timer, because a green run only proves this box was slow enough. - **A speed-correlated failure is not flake, and a retry budget does not absorb it.** Random flake becomes a pass under `retry: 2`; this failed 5/5 runs on all three attempts. The two get opposite verdicts, so the kind has to be established before the verdict is written. Stated plainly in the skill: the verify job runs on a shared, loaded runner — the regime where such a test PASSES. Repetition cannot reproduce a fast-machine failure there; only computing the margin can. A rule that said "run it more times" would be useless in this lane. - **The failure one level before vacuity: the scenario never reached the code under test.** The vacuity check asks whether an assertion can fail; this asks whether the code ever ran. Four abort cases fired during CLI process startup, so the fake server saw zero requests and a suite named for mid-stream aborts never streamed — with every assertion green. Instrument the seam and assert the count is non-zero. Mutation-verified 5/5, each with landing proof against line-accurate anchors. 89/89 tests; prettier and eslint clean. Skill is 472 lines, up from 392. * feat(verify-pr): six more techniques, from three maintainer rounds Mined #7836 R2, #7885 and #7899 for method. Checked each candidate against the current text first; six had zero coverage, the rest were already there (harness teeth-checks, pin dereferencing, boundary probing, and the follow-up round's "re-measure, never diff"). The one that corrects the skill's own core method, from #7836: - **Before calling a survivor vacuous, escalate to a finer mutation.** A whole-file revert is blunt enough to remove the PRECONDITION a test depends on, so a good test goes green because its scenario no longer occurs — from the outside, identical to a test that asserts nothing. A `finally`-cleanup test survived reverting four production files and died to deleting one line. Coarse survived + fine killed ⇒ the test is fine and the mutation was wrong. A false "your test is vacuous" costs the author more than a missed survivor does. From #7836, the root cause shared by both of its blockers: - **When the same predicate is checked in two places, verify they see the same state.** A guard duplicated across a process boundary is two implementations of one question that diverge when their INPUTS differ. One settings key made a route ask sessionExistsInAnyState() with an unpinned runtime dir while the child asked with a pinned one, turning a clean 409 into a 500 plus a process.exit(1) that killed every session on the channel. Includes the temporal half: lazily-created backing files leave a window where a just-created entity is invisible to any on-disk existence check. - **Measure the blast radius on bystanders.** The caller's own error code understates a shared-state failure; the number that matters is an unrelated session going 200 -> 404 and a workspace list going 2 -> 0. From #7885, which took a performance claim apart: - **Isolate the slice the mechanism can actually affect.** A speedup claim is two claims: the mechanism works, and the thing it speeds up matters. `--ignore-scripts` isolated what an npm download cache can touch — 36s of a 226s install — so the ceiling was 20s and the real saving 15%, not the claimed 75%. Then check it against the whole job: 33s off 14m37s. - **A mechanism that persists something has a cost — price it.** 219 MB per lockfile hash into a pool at 9.98 GB of a 10 GB cap, with 39 distinct lockfile states in 30 days: at the cap every entry evicts by LRU, including entries other jobs need and its own. - **Test the scarier consequences and report which do NOT hold.** The write-path finding was real; code injection was disproved (npm integrity-checks a tampered cache and refetches) and privilege escalation was disproved (chown -R does not follow symlinks). A finding that names what it is not is harder to wave away. - **Verify third-party actions from their own manifest.** The PR said the cache dir was discarded after the job; `action.yml` declares `post: dist/save/index.js` with `post-if: success()`, which uploads it as root with credentials intact — the opposite of the claim, and the whole finding. From #7899: - **To exercise real production data safely, interpose a refusing proxy on the write path.** Wrap the client so every mutating call hard- fails, then run the shipped script verbatim: real counts, mechanical guarantee of no side effects. Mutation-verified 9/9, each with landing proof against line-accurate anchors. 89/89 tests; prettier and eslint clean. The skill is now 546 lines, up from 392 — the growth is deliberate and called out in the PR body's risk section. * feat(verify-pr): decomposed fixes, contextual limits, destination counts From #7862 R4. Three additions, and a deliberate stop. - **When one fix bundles two changes, build the intermediate variants.** An A/B against base proves the pair works and says nothing about what each half does. That round compiled a third build with only the ordering change reverted, and the three-row table showed the halves do different jobs: moving `initialized = true` after the fallible work converts a 2,999-and-climbing backlog flood into a fail-safe retry, while `reduce()` restores liveness. Either alone leaves a channel that floods or wedges — a conclusion the two-cell A/B cannot reach. - **A limit measured in isolation does not transfer to the real call site.** The same `Math.max` spread threw between 110k and 130k elements inside a deep async stack, well below a standalone micro-benchmark. Bisect thresholds through the real code path and quote the harness; a limit taken from documentation or a toy loop is a guess about the system under test. - **Count at the destination, not at the component boundary.** The mirror of the scenario-arrival rule added earlier: envelopes the adapter emitted and prompts that reached the agent are different numbers, and every gate lives between them. A count taken at the seam can be right while the feature is silently dropped downstream. Declined from the same report, to protect prompt budget rather than because they are wrong: siblings-as-convention-oracle (the lockfile version table across five channels), degenerate fixtures that cannot distinguish two sort keys, and naming the condition under which a cosmetic finding becomes real. Each is a good technique; none is worth another rule competing for attention with the ones already here. The skill is now 578 lines, up from 392 on main (+47%) across this branch. That growth is the main risk on this PR and further additions should wait until a live round shows the current set changes behaviour. Mutation-verified 3/3 with landing proof. One mutation initially SURVIVED — it deleted text sitting AFTER the asserted phrase, so the assertion still matched and the green proved nothing; re-run against the phrase itself, it kills. 89/89 tests; prettier and eslint clean. * test(scripts): drop stale technique count from verify test name (#8010) * fix(triage): correct verify-skill worked examples and verdict path (#8010) Address review feedback on the verification-techniques skill: - Make the npm-cache worked example's numbers close: separate the 20 s download-slice ceiling (36 s to 16 s) from the 15% end-to-end saving (226 s to 193 s) rather than conflating them. - Stop overstating the tarball experiment: one tarball was poisoned, and the 2262-entry integrity coverage is a separate static fact. - Give the speed-correlated-failure rule a contract-legal verdict path by encoding the margin as a scripted assertion, and mark the load/idle sweep as the local-mode variant. - Fix the one bullet that broke its 2-space list continuation. - Pin the new contract-encoding clause in the workflow test. --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-07-30 02:40:26 +00:00
易良	c55e63c42a	fix(release): bump preview base past published stable (#7978) ... * fix(release): bump preview base past published stable When the nightly tag base (e.g. 0.21.0) is already published as stable, getPreviewVersion() now bumps the patch (→ 0.21.1-preview.0) instead of deriving a preview for the released version. This prevents the scheduled Tuesday preview release from hitting npm E403 on channel packages that were already published at that version. Fixes #7969 * fix(release): address preview version review feedback * fix(release): reject divergent preview baseline * fix(release): skip already published packages * test(release): cover publish skip guards * fix(release): bump preview past newer stable * fix(release): address review feedback on preview version guard - Expand doesVersionExist to check all 10 published packages instead of only @qwen-code/qwen-code, so the auto-increment loop detects versions taken on sibling channel packages. - Use the rollback-aware getAndVerifyTags lookup for the latest stable instead of the raw dist-tag, preventing a retrograde preview base when the dist-tag has been rolled back. - Emit :⚠️: in the channel publish loop when every package was already published, making a fully-skipped release visible. - Move preview stable-guard tests to Advanced Scenarios, add npmTag and previousReleaseTag assertions, add a non-bump boundary case, and assert the subshell wrapper and all-skipped warning in the workflow test. * test(release): cover channel package version conflicts --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-07-29 23:21:00 +00:00
易良	ff5c639e26	fix(release): raise model timeouts and shrink batch size for slow networks (#8007) ... * fix(release): raise model timeouts and shrink batch size for slow networks The AI release-notes generator timed out on every batch in the v0.21.1 finalize run on a GitHub-hosted runner (US, Azure westus3) hitting a remote LLM endpoint. The 60s per-request timeout was too close to the edge: two successful requests took 54.2s and 56.7s, and every other batch hit the 60s abort. The 12-minute total budget was consumed by retries on those timeouts, and the circuit breaker opened after 3 consecutive failures, skipping highlights entirely. - timeoutMs: 60s -> 180s — give the model enough headroom to generate a full JSON response over a high-RTT cross-region connection. - totalTimeoutMs: 12min -> 30min — ~140 PRs at 8 per batch is ~18 batches; at ~90s each that's ~27 minutes of model time. - batchSize: 12 -> 8 — fewer entries per prompt means the model generates less per request, returning faster and reducing the chance of a single slow request dragging the whole batch. - workflow timeout-minutes: 15 -> 35 — match the new 30min budget plus a margin for git/npm setup. No changes to retry logic, circuit breaker, or prompt shape. * fix(release): sync timeout workflow test * test(release): assert step timeout exceeds script budget The workflow `timeout-minutes` and the script's `totalTimeoutMs` are defined in separate files with no shared constant. If they drift apart silently and the budget exceeds the step timeout, the runner SIGKILLs the step and even the fallback release notes are lost. Add a cross-file assertion so any future change to either value that breaks the invariant fails the test immediately. Suggested-by: wenshao in PR review. * test(release): parse release notes timeout expression	2026-07-29 22:38:35 +00:00
Shaojin Wen	ea4c25e25f	feat(triage): run external /verify on ECS behind a workspace wipe (#7985) ... * feat(triage): sponsored /verify for external PRs on ephemeral runners /verify used to require the PR AUTHOR to hold write access, because the lane executes the author's code on the persistent ECS pool — which excluded external contributors' PRs entirely (16 of 38 open PRs eligible when measured on 2026-07-28), and those are the PRs where sandboxed evidence is worth the most. The practical alternative was a maintainer running the verification on their own machine, which is strictly worse: personal credentials, SSH keys, and no isolation at all. The risk was not avoided, only relocated. The author's permission now ROUTES instead of gating, using the same conditional runs-on pattern the authorize/triage jobs already use for fork PRs: - write-access author -> the persistent ECS pool, unchanged; - external author -> a SPONSORED run on an ephemeral GitHub-hosted VM (destroyed after the job), free on a public repository. The commenter gate is unchanged: only a write-access maintainer can trigger either lane, so runs stay maintainer-metered. Their comment is the sponsorship, and authorize snapshots the head OID at that moment; the resolve step refuses to run any other commit, so code pushed while the job queues is never executed on the sponsor's approval. On the ECS lane the execution-time re-check keeps its old meaning (the author's write permission is the control there, and it can be revoked while queued); its refusal now names the sponsored alternative. Before anything executes on the hosted lane, a pre-execution risk screen reads the PR diff as data: mechanical checks (added npm lifecycle scripts, dependencies resolving outside registry.npmjs.org, package-manager config changes, long opaque single-line content) and a model screen, every arm failing CLOSED — an unfetchable diff, missing screen config, a model error, and an unparseable reply each refuse the run. Reasons posted to the PR are fixed strings chosen by the workflow, never diff text, so a crafted diff cannot steer the bot's comment. The screen is defense in depth, not the control: what bounds a malicious PR is the ephemeral VM. The model can be fooled; the machine being destroyed cannot. The ECS kill switch no longer strands the hosted lane (an ephemeral runner does not depend on the pool), and the authorize-side denial notice for external authors is gone — the case it explained no longer denies. The 2b-bis skill guidance now offers the sponsored run instead of declaring the lanes unavailable, with an explicit warning that an external author's report is adversarial input to be read like the fork's own CI logs. Mutation-verified 8/8, each with landing proof: dropping the commenter gate, inverting the routing, dropping the OID snapshot, letting a permission-API failure route instead of deny, dropping the head-moved check, dropping the lifecycle-script check, tolerating an unparseable model reply, and pinning the hosted lane back to ECS each turn a test red. The screen tests run the real resolve-step text against a stubbed gh and a live local HTTP server standing in for the model endpoint — in a separate process, because spawnSync blocks the test runner's event loop and an in-process server would deadlock into the fetch's 120 s abort instead of testing anything. actionlint category diff vs main: -3 SC2016:info (the removed denial step), +3 SC2129:style (new GITHUB_OUTPUT groups); both non-gating. * docs(triage): persistence, not lateral movement, is the ECS threat The maintainer confirmed the ECS runners are network-isolated, so the comments justifying the hosted routing now name the decisive risk correctly: machines are reused and the next run arrives carrying credentials that planted artifacts could harvest. Lateral movement was never the load-bearing argument; persistence is, and it is the one threat a destroyed VM removes without depending on sweep completeness. * feat(triage): run external /verify on ECS behind a workspace wipe Maintainer decision (2026-07-29): external contributors' PRs run on the persistent ECS pool like everyone else, not on ephemeral GitHub-hosted VMs. The pool is network-isolated, and the realistic alternative — a maintainer verifying on a personal machine with real credentials and no isolation — is worse than a hardened shared runner. That removes machine destruction from the design, so the author's permission now selects CONTROLS rather than a machine: trusted -> unchanged: write permission is the control, re-checked at execution time, head may move freely. external -> pinned head OID (the sponsoring comment approves the code that existed then), pre-execution risk screen, and a full workspace wipe. The wipe is what replaces destruction. The residual ECS risk for untrusted code is PERSISTENCE: machines are reused, and the next run starts by handling credentials (CI_BOT_PAT in the resolve step, the model key in the proxy). The existing sweep is an allowlist of vectors we have thought of, and an adversarial author gets unlimited attempts to find one we have not — so external runs delete the workspace outright before checkout instead of sweeping it. Deny-by-default costs one cold clone and removes hooks, worktree config, planted dotfiles, staged artifacts and out-of-tree symlinks in one move, known or not. Scope is stated in the step: RUNNER_TEMP and /tmp also persist, and the steps reading them already rm -rf their own subtrees; the container is recreated per job, so /home/node and its npm cache are not cross-run state. The screen's status changes with the machine: on ephemeral hardware it was a cost-saver, here it is load-bearing — but still not sufficient alone, and the comment says so, naming the wipe and the sweep as what actually bounds a screened-but-hostile diff. Mutation-verified 6/6: removing the wipe, moving it after checkout, applying it to every run, tolerating a non-empty workspace, dropping the suspicious-path guard, and classifying an external author as trusted each turn a test red. Also fixes a live hazard in the guard's own test. It passed real paths (/, /usr, /root) to a live `rm -rf`, which is safe only while the guard exists — so it detonates precisely when the guard is removed, which is when it is meant to protect you. It did: a mutation run that deleted the guard spent six minutes attempting to delete / before being killed. macOS permissions absorbed it and nothing was lost, but that outcome was luck, not design. `rm` is now stubbed to a recorder on PATH, so the destructive primitive cannot fire from the test suite under any edit, and the assertion is on the decision — with the guard gone the recorder shows the attempted delete and the test fails, having deleted nothing. * fix(triage): close two screen bypasses and a vacuous routing assertion Review round on #7985 found four issues; all four were real. 1. npm lifecycle alternation stopped at the install lifecycle, but npm runs pre/post hooks for ANY `npm run <script>`. This job runs `npm run build` and the agent runs the PR's suites, so a `prebuild` or `pretest` executes exactly like a `postinstall` and walked past the mechanical screen. Added build and test to the alternation. 2. The off-registry check excluded any line CONTAINING `registry.npmjs.org`, so a lookalike host — registry.npmjs.org.evil.com, or an @-userinfo form — read as npmjs and passed. The tarball's own postinstall lives inside the tarball, never in the diff, so this arm was the only mechanical defense against it. The exclusion is now anchored to scheme + host + '/'. 3. A routing assertion read `.runner`, a key from the earlier ephemeral-VM design that `gate()` never returns, so it was `expect(undefined).toBeUndefined()` — green no matter how badly the trust level leaked into /triage or /tmux. Asserts the real keys now, on both non-verify paths. 4. Three screen arms had no scenario at all: package-manager config, the long-opaque-line awk, and the unconfigured-model fail-closed path. All three are covered, each with a negative control so the arm is not merely refusing everything — the genuine registry URL and a 700-char lockfile integrity line must still pass. Mutation-verified 6/6, each failure naming the payload it let through: reverting the alternation (`prebuild was not flagged`), un-anchoring the registry match (`registry.npmjs.org.evil.com was not flagged`), breaking the package-manager grep (`.yarnrc was not flagged`), raising the awk threshold, letting the unconfigured path fall through to clear, and leaking verify_trust into /triage — which is exactly what the vacuous assertion could never see. 93/93 tests; prettier, eslint and shellcheck clean. * fix(triage): close post-run persistence gap and screen bypasses (#7985) Maintainer review found the wipe only ran BEFORE external code, leaving the next pool job to meet the allowlist sweep alone. Add a post-run wipe with `if: always()` so a cancelled or timed-out external job still cleans up. The model screen silently truncated at 200 KB — a fail-open in a fail-closed step. Refuse oversized diffs mechanically before the model call. The screen prompt still described an "ephemeral, credential-free sandbox" from the deleted ephemeral-runner design, calibrating the model for leniency the persistent pool does not warrant. Describe the actual environment. Also fix the stale "hosted lane" comment. Three cheap mechanical-screen hardenings from the review: drop the root anchor on the package-manager config grep (a nested .npmrc is just as dangerous), match `rename to` headers (a rename-only hunk emits no `+++ b/` line), and relax the opaque-line awk from "no space in the line" to "no space-free field >= 600" so a single space anywhere is no longer a bypass. 94/94 tests; build, typecheck, lint clean. * test(triage): cover the unfetchable-diff fail-closed screen guard (#7985) --------- 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@service.alibaba.com>	2026-07-29 12:03:37 +00:00
Shaojin Wen	d0481ad884	feat(triage): lead the verify comment with a qualitative verdict, fold the Chinese (#7974) ... * feat(triage): lead the verify comment with a qualitative verdict, fold the Chinese Maintainer feedback on the first 14-run day of the lane, pointing at the report on PR #7836: give a plain pass/no-pass, and make the layout English-by-default with the Chinese folded (the repo's PR-body convention), instead of doubling every head line. Three changes: - every headline leads with a qualitative call — ✅ passed / ❌ not passed / ⚠️ inconclusive / ⚠️ incomplete — ahead of the lane taxonomy. The mapping is honest about what each outcome can claim: only agent verdicts judge the code; a crashed, timed-out, or verdict-less run renders as incomplete/inconclusive, never as a pass or a fail, and a test pins that no process-outcome arm carries ✅ or ❌. - the Chinese version folds into one <details> whose SUMMARY carries the qualitative verdict — collapsed, a Chinese reader still sees 通过/不通过 in one line. English scope and assertion count stay unfolded; both language assertion lines are built from the same validated triple, so inconsistent JSON still suppresses both. - the #7836 root cause gets a Hard rule in the verify-pr skill. That report said "merge-ready — the 7 failures are all expected A/B base-cell failures proving the tests load-bearing" while assertions.json said fail:7, so the publisher's trust rule (merge-ready requires fail==0) correctly refused it and the headline degraded to "no usable structured verdict". Both sides told the truth about different questions. The rule fixes the semantics: an A/B control cell is an assertion that the base arm FAILS — when it does, that assertion PASSED. fail counts only unexpected outcomes, which is what makes a qualitative headline trustworthy at a glance. Mutation-verified 6/6: dropping the qualitative prefix, collapsing all Chinese quals onto one string, unfolding the Chinese scope, giving timeout a ✅, dropping the folded Chinese assertion line, and deleting the new Hard rule each turn at least one test red. The fifth mutation initially reported "landed: False" — the regex never matched and the file was untouched, so the green result proved nothing; re-applied as a line filter with a removed-block count, it kills two tests. One new shellcheck SC2016 info finding (backticks inside a single-quoted Chinese printf), same shape as the 11 pre-existing ones; info-level, does not gate. * test(triage): pin the dropped verdict arms and prepare-failure glyphs (#7974) Restore rendering coverage for the reachable infra-error and unknown catch-all case arms in the qualitative-headline bijection, and assert the prepare-failure path's qualitative glyphs, Chinese fold summaries, and the Chinese retry clause so a swapped emoji or dropped PREPARE_ATTEMPTS_ZH interpolation can no longer ship undetected. * fix(triage): migrate all weak headlines, explain merge-ready mismatch (#7974) * fix(triage): fold Chinese mismatch explanation, differentiate distrust warnings (#7974) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code CI Bot <qwen-code-ci-bot@users.noreply.github.com>	2026-07-29 10:14:40 +00:00
qwen-code-dev-bot	324b5fba38	fix(release): skip notes-start-tag when previous release diverges from target (#7969) (#7970) ... * fix(release): skip notes-start-tag when previous release diverges from target (#7969) * style(release): fix prettier formatting in release workflow (#7969) * fix(release): warn when notes-start-tag is skipped for a divergent tag (#7969) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>	2026-07-29 08:23:34 +00:00
易良	6b9b861398	fix(ci): update the Qwen binary used by ECS runners (#8000) ... * fix(ci): install Qwen into system npm prefix * test(ci): update ECS runner workflow guard	2026-07-29 05:24:56 +00:00
Shaojin Wen	f56303e3e1	feat(triage): make the not-verified sentence a mechanical 2b-bis trigger (#7965) ... Post-merge measurement of #7917, one day in: 9 eligible PRs, two considered-and-declined mentions (both correct calls), zero positive recommendations. The one clear behavioural candidate — #7947, bounded reads of large text files — wrote "Not verified: Windows and Linux manual runs (author tested on macOS only)" in its own Stage 2 comment and never named a lane. That is the failure shape worth fixing: the judgement-based rule ("when neither static review nor 2b substantiates it") failed exactly where the comment had already written the gap down in so many words. The model judged that pending CI would cover it; a green suite proves the tests pass, not that the untested behaviour holds. So the trigger is now textual, not judgemental: before posting, grep your own draft. A sentence of the shape "not verified", "author tested on one platform only", or "author's claim, not independently re-run" IS the trigger — the 2b-bis line is that same sentence with the remedy attached, and omitting it means telling the maintainer what is missing while withholding the one command that would supply it. Pending CI does not lift the trigger. The two legitimate skip cases (nothing behavioural to settle; author lacks write) are unchanged, and the rule is ordered before them so they read as outs from the requirement, not the requirement as an out from them. The trigger phrases are verbatim from real comments: "not verified" and "author tested on macOS only" from #7947, "author's claim, not independently re-run" from #7951. Mutation-verified 3/3: dropping the trigger paragraph, moving it after the skip cases, and dropping the pending-CI sentence each turn the test red. n=1 is thin evidence for a behavioural rule change — but this rule is text-matching, not probability-weighing, so it cannot overfit to the sample that motivated it. Co-authored-by: wenshao <wenshao@example.com>	2026-07-29 03:41:18 +00:00
qqqys	0c0ca5fed0	feat(autofix): defer suggestions after five change rounds (#7913) ... * feat(autofix): defer suggestions after five change rounds * test(autofix): execute deferred jq queries against fixture data (#7913) * fix(autofix): scrub control markers from deferred feedback reports (#7913) * test(autofix): execute actionable reviews and issue-level filters against fixtures (#7913) --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.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@service.alibaba.com>	2026-07-28 17:41:49 +00:00
Shaojin Wen	ddf4b8875d	fix(triage): make the build-process guard diagnosable and zombie-aware (#7858) ... * fix(triage): make the build-process guard diagnosable and zombie-aware The first real /verify run to reach the agent step (job 30267953352) was stopped by the guard I added to stop detached lifecycle children from outliving their step: ::error::Processes owned by the build user survived; refusing to start the agent. That is all it printed. No pid, no state, no command line — so a genuine threat and a harmless leftover were indistinguishable, including to the person who wrote it. The control had no observability of its own. Two changes, both in the verify and tmux lanes: - name the survivors. The error now lists each one as pid, state and command line, so the next occurrence can actually be diagnosed. - disregard zombies. A zombie has already exited and released everything except its exit status: it cannot re-plant an artifact or touch the agent's inputs, and it cannot be killed either — so counting one means the check can never clear, no matter how many times SIGKILL is sent. The container runs no init to reap orphans, so they are expected here. The platform difference is the reason the old form could hang: Linux pgrep reports defunct processes ("Defunct processes are reported." — pgrep(1), procps-ng), while macOS pgrep does not list them at all (verified directly: ps shows our zombie, pgrep does not). So this failure mode was unreachable in local replay and only appears on the runner. That same difference shapes the tests. The behavioural arm constructs a real zombie, confirms it survives SIGKILL, and shows the state filter drops it — but it cannot discriminate the old implementation from the new one on macOS, because pgrep never saw the zombie there in the first place. So the portable guarantee is asserted structurally: the filter must read process state and exclude Z, and must not be a bare pgrep. Mutation- verified 3/3 — removing the state filter, dropping the survivor names, or reverting the message each turn one test red. Also gives both proxy-watchdog tests an explicit timeout. They stream 20 chunks at 200 ms — 4 s before the stall arm even starts — so they cannot fit vitest's 5 s default and were timing out on main. * fix(triage): tolerate zero-process exit in build guard (#7858) --------- 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@service.alibaba.com>	2026-07-28 17:13:40 +00:00
Shaojin Wen	25e357a279	feat(triage): make the verify report readable in Chinese (#7918) ... * feat(triage): make the verify report readable in Chinese The scope disclaimer under the verify headline has been bilingual since the lane shipped. The verdict above it was not, and neither was the assertion count — so a Chinese reader of the unfolded comment got the caveat ("advisory evidence, not a review") and not the conclusion. Three changes, all on the parts a reader reaches first: - every headline carries a Chinese twin, across all eight arms — merge-ready / findings / blocked / inconclusive from the agent, and completed / fail / timeout / infra-error from the process outcome; - the assertion count renders in both languages, off the same validated object, so an inconsistent assertions.json still suppresses both and the comment cannot grow a number no gate checked; - in the report itself, 中文摘要 moves from last item to second, right after the verdict. The whole report is already inside a <details> on the PR, so leaving the Chinese summary at the bottom meant expanding that fold and scrolling the entire English report — about 90 lines on a real one — to reach the one section written for that reader. It stays collapsed, so it costs everyone else exactly one line. The headline test pins the pairing as a bijection, not as containment: asserting only that each arm renders some Chinese leaves a single hardcoded string — or one that echoes the English — passing every arm. Mutation-verified 4/4: dropping the Chinese headline, collapsing all arms onto one Chinese string, dropping the Chinese assertion sentence, and moving 中文摘要 back to the end each turn one test red. * fix(ci): correct cross-reference direction and cover all verdict branches in bilingual headline test (#7918) * fix(ci): narrow bilingual headline comment to match actual scope (#7918) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-28 17:13:23 +00:00
易良	bfd4c8e519	fix(scripts): slim release-note model prompts and log request timing (#7941)	2026-07-28 15:32:56 +00:00
Shaojin Wen	b6b55c598c	feat(triage): surface the sandboxed lanes on the CI path (#7917) ... Measured on 2026-07-28 across 22 open PRs: of the 16 whose AUTHOR had write access and that already carried a triage comment, exactly 1 mentioned `/verify`. The lane it recommends has produced real evidence on four PRs (#7829 2565 assertions, #7821 1347, #7830 27, #7881 17), so the gap is not that the lane is useless — it is that almost nobody is told it exists. The instruction was there the whole time. It was a conditional clause inside a section headed "2c. Real-Scenario Testing — local invocation ONLY" whose first words are "Never in unattended CI." An agent running in CI reasonably skips that section, and the assembly order at the end of Stage 2 only allotted 2c a slot "when one was driven locally" — so on the CI path there was no place for the recommendation to go even if it had been read. Split the CI-path half out into its own section, 2b-bis, sited immediately after the CI-evidence step it follows from: 2b tells you the suite is green, and cannot tell you the suite pins the change. It is a required element of the Stage 2 comment when the central claim is behavioural, with two explicit skip conditions — nothing behavioural to settle, or the author lacks write (both lanes execute the author's code, so recommending them on an external contributor's PR is a guaranteed denial). It must name the specific unsubstantiated claim, because a bare "you could run /verify" is noise and noise is why the line got skipped. Stage 1e carried the same dead pointer, and worse: on the high-risk paths — the strongest triage-time signal in the skill, 10 of 31 reverted PRs against 5 of 60 controls, p = 0.006 — it recommended tmux alone and never named /verify. The PRs most likely to be reverted were the ones never offered the lane that proves a change is load-bearing. 1e now points at 2b-bis and names both lanes. The fix is positional, so the tests are positional: asserting that the file mentions `/verify` would have passed throughout the entire period the recommendation was dead. Mutation-verified 4/4 — moving 2b-bis back below the local-only heading, dropping it from the assembly order, dropping the author-write carve-out, and reverting 1e to the tmux-only pointer each turn a test red. Prose assertions are whitespace-normalised and shown to survive a maximal re-wrap, since prettier reflows this file. Co-authored-by: wenshao <wenshao@example.com>	2026-07-28 14:28:39 +00:00
Shaojin Wen	43c35e533b	fix(ci): keep the post-merge E2E signal on main alive (#7795) ... * fix(ci): stop cancelling in-progress E2E runs on main Merges land on main roughly every 18 minutes (median) while a full E2E run takes about 40, so cancelling the in-flight run on every push starved the suite: across the last 100 push runs, 67 were cancelled and only 25 ever reported a result. The nightly regression was the only reliable signal, and each cancelled run still burned roughly 10 minutes on three runners before dying. Turning cancellation off for main hands the coalescing to GitHub's concurrency queue, which keeps at most one pending run per group and cancels the previously pending one. The queue therefore collapses to the newest tree on its own while the in-flight run always finishes, so every result covers the batch of commits merged since the last one — bisect that range when it goes red. Dev branches keep cancelling superseded runs, where only the latest push matters. * fix(ci): dedupe main CI failure issues by failing test The autofix issue for a red main was deduped on the commit SHA, so a standing failure opened a brand new issue on every merge: one broken E2E test on 2026-07-26 produced six duplicate issues in twelve hours, each pointing the autofix agent at an unrelated commit. The failing tests are now identified from the logs of the failed jobs and used as the dedupe key, with one marker per test so a failure set that grows still matches the issue that already tracks part of it. Later commits hitting the same failure are appended to that issue as recurrences (bounded, newest first) instead of opening another one, and notes written by a human or the agent are preserved when the machine-owned trailer is refreshed. Runs with no identifiable test — an install or build break — keep the previous per-commit behaviour. * fix(ci): keep the failure analysis out of the bot-PAT job Identifying the failing tests means running a helper from the repository, and the workflow deliberately checked out nothing so that the job holding the bot PAT could never execute repository code — an invariant its own test enforces. Rather than weaken it, the work is split: a read-only job checks out the tree, reads the failed run's logs, finds any issue that already tracks the failure and renders the title and body, handing both over as outputs. The job with the PAT keeps checking out nothing and only writes what it was given. The invariant test now asserts that separation — the PAT job runs no repository code and holds only issue write — instead of banning checkout everywhere in the file. * fix(ci): harden main CI failure dedupe per review (#7795) - Match the `[run <id>]` link text instead of the run URL when deduping recurrences: `/301` is a substring of `/3010`, so the URL match silently deleted an unrelated run's line. - Rebuild the machine-owned "## Also failing" section from the live failure set on every merge so a test that has since been fixed drops out instead of being listed forever. - Assert the analyze checkout is SHA-pinned with persist-credentials disabled and pin the failed-job log-download paths in the workflow test. - Add an e2e workflow test guarding the cancel-in-progress expression that keeps in-progress runs on main from being cancelled. * fix(ci): bound the issue body and randomize the heredoc delimiter (#7795) * fix(ci): test the runCli --existing merge path (#7795) * fix(ci): address review feedback on e2e signal PR (#7795) - Assert the full cancel-in-progress expression including && so a mutation to || is caught by the e2e-workflow test - Filter the capped-summary line from missingTests so it is not rendered as a fake bullet under Also failing --------- 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> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>	2026-07-28 11:54:52 +00:00
易良	45a6a69cf0	feat(triage): add revert-pattern high-risk path detection (#7414) ... * feat(triage): add revert-pattern high-risk path detection Replace the behavior-neutral PR filter (PR #7414 v1, ~2% hit rate) with a data-backed triage gate based on revert-history analysis of 111 revert commits and 46 unique reverted PRs in this repo. Stage 1e checks three signals identified by the analysis: - touches_high_risk (66.7% precision, 32.3% recall) - contested-merge pattern (50.0% precision, 19.4% recall) - non-maintainer + high-risk (58.3% precision, 22.6% recall) The gate escalates review depth and recommends maintainer sign-off; it never blocks or closes PRs. Design doc and analysis scripts included. * fix(triage): avoid stale-exempt hold label * fix(triage): address review risk detection feedback * fix(triage): tighten high-risk path patterns * fix(triage): address review feedback on Stage 1e revert-pattern gate (#7414) * fix(triage): address round-2 review feedback on Stage 1e gate (#7414) - Fix APPROVE → APPROVED state name (GitHub API enum) - Use gh api --paginate for file listing (fixes 100-file truncation) - Anchor shell/relaunch/sandbox patterns with (^|/) to avoid false positives - Append || true to grep (exit 1 on no match is the 92% case) - Scope E2E recommendation to write-access authors per Stage 2c - Add bot author filter to contested-merge query - Define core paths explicitly in contested-merge condition - Wire Stage 1e do-not-auto-approve into Stage 3 guardrail - Replace precision percentages with p-values/raw counts in skill text - Add sampling caveat and statistical significance notes to design doc - Fix design doc errors: 71%→61.5%, 10→8 PRs, Rule 3 attribution, ~20% baseline→10% prevalence, Area field, no_e2e inconsistency - Make test assertions specific to Stage 1e (not vacuous) - Add Risk: template field assertion - Revert drive-by prettier reflow - Note need-discussion label removal by maintainer * fix(triage): address round-3 review feedback on Stage 1e gate (#7414) - Separate gh api call from grep so API failures are visible instead of being masked by || true (rc:3660753982) - Include author identity in contested-merge jq output and require different reviewers for the disagreement check, avoiding false positives from same-reviewer iteration (rc:3660753990) - Add Stage 1e to the approval summary checklist so it is not omitted from the pre-approval conditions (rc:3660753994) * fix(triage): address round-4 review feedback on Stage 1e gate (#7414) * fix(triage): use portable ERE grep for test-file exclusion (#7414) * fix(triage): guard deferred approval on discussion label * fix(triage): keep only supported revert signal --------- Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-07-28 05:53:57 +00:00
易良	47118be482	feat(ci): Deduplicate E2E failure issues by commenting on existing issue (#7792) ... * feat(ci): deduplicate E2E failure issues by commenting on existing issue * fix(ci): address failure issue review feedback * test(ci): cover CI issue dedup invariants * fix(ci): include comments in failure marker dedup * fix(ci): include issue comments in failure marker search * fix(ci): share failure issue title prefix * fix(ci): filter dedup issue titles * test(ci): assert SHA dedup runs before workflow-name dedup (#7792) * fix(ci): limit SHA dedup search to issue bodies * fix(ci): verify bot ownership before reusing title-matched issue (#7792) --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-28 05:40:07 +00:00
Shaojin Wen	ec62797c1c	fix(review): give the review retry the remaining time budget (#7852) ... * fix(review): give the review retry the remaining time budget A transient API failure on the first review attempt was retried with a hardcoded 5-minute timeout while the first attempt got the whole 180-minute budget. The retry does not resume the failed review, it re-runs it from scratch, so on a large PR it spends those five minutes re-fetching the PR worktree, re-chunking the diff and re-launching review agents and is then killed on the clock before producing a single finding. Every retry after a transient failure therefore reported a timeout, and the resulting comment advised raising --timeout, which does not affect the retry cap at all. All attempts already share the single-review budget, so that budget is the only bound the retry needs. Give every attempt the remaining budget and gate the retry on having enough of it left to plausibly finish; below that, report the transient failure so the next run starts over with a full budget. * test(review): pin the per-attempt review timeout budget The retry-loop harness stubbed timeout with `shift; shift; exec "$@"`, which discarded the duration argument, so no test could observe how much budget each attempt was given. That is the invariant the retry fix turns on: a reintroduced per-attempt cap, or a miscalculated remaining budget, would have left the suite green while making every retry unusable. Record the duration the stub receives and assert on it. Cover the retry gate from both sides as well, since every existing scenario runs with a 180-minute budget and none of them exercise the threshold: a budget just under it must not start a retry that cannot finish, and one just over it must still retry.	2026-07-28 05:29:20 +00:00
Shaojin Wen	203093c53f	fix(triage): retry a transient npm ci before blaming the PR for it (#7884) ... Run 30319209722 posted `Sandboxed verification: fail` on PR #7856 with "The PR could not be built ... treated as a PR failure verdict rather than an infrastructure failure." The PR changed six source files. The install died here: Error: spawnSync .../web-templates/node_modules/esbuild/bin/esbuild ETXTBSY at validateBinaryVersion (esbuild/install.js:99:28) ETXTBSY is npm writing a dependency's binary and that package's own install script exec'ing it before the write is closed — a race, and one the PR had no part in. The comment accused its author anyway. Both sandbox lanes now retry `npm ci` once. `npm ci` removes node_modules before installing, so the second attempt cannot inherit the half-written file; the transient class is simply absorbed instead of being classified. Retrying rather than classifying is the point. The obvious fix — match ETXTBSY in the log and downgrade to infra-error — would read text the PR's own lifecycle scripts can print, which is exactly the forgeable signal the verdict logic was rewritten to stop consulting: it let a PR launder its own deterministic breakage into "infrastructure, please re-run". A retry consults nothing. A genuinely broken tree fails twice and still earns `fail`, and the second attempt is only ever paid on a path that is already failing. The build is deliberately not retried: a compile error is deterministic, so a second run would only double the cost of an honest failure. Because the install now gets two chances, the sentence that blames the PR says so — "failed twice in a row". PREPARE_ATTEMPTS is initialised rather than defaulted at the point of use, or an inherited value would claim a single-shot `npm run build` had failed twice, which is the same false accusation pointed the other way. Mutation-verified 5/5: an unbounded retry, no retry at all, a retry on the build, dropping the initialisation, and reverting the wording each turn at least one test red. The retry itself is covered behaviourally — the real step text runs against a stubbed `runuser`/`npm`, so the tests count actual install attempts instead of asserting that a loop exists, which would pass on both a loop that never retries and one that never stops. Co-authored-by: wenshao <wenshao@example.com>	2026-07-28 04:17:50 +00:00
易良	923e5ab425	fix(scripts): harden retry classification and preserve deadline error context (#7854) ... * fix(scripts): harden retry classification and preserve deadline error context Follow-up to #7535 addressing remaining review suggestions: - isRetryableModelError: content-validation errors ('Model response did not contain message content.') are now non-retryable. These are deterministic failures from our own code — retrying the same prompt reproduces the same failure. Network-level errors (no HTTP status) remain retryable. - Deadline-expired errors now include the original error message instead of the generic 'budget exhausted', so oncall can see the root cause. - Added 4 tests: HTTP 429 retry, network error retry, content-validation non-retry, deadline error preservation. * fix(release): preserve last model error across retry budget * test(release): cover retry budget expiry after backoff * refactor(release): extract deadline error helper and preserve cause Consolidate the three identical budget-exhausted throw sites into a single deadlineError() helper so they share one message format and source variable, and attach the last model error as the Error cause so its stack trace survives into CI logs. Also conform the release-notes tests added by this series to Prettier.	2026-07-27 23:48:43 +00:00
qqqys	c994524a2d	feat(autofix): retry deterministic rejection once (#7796) ... * feat(autofix): retry deterministic rejection once * fix(autofix): use repaired verification result * fix(autofix): preserve repair safety context * test(autofix): cover empty verification outcomes	2026-07-27 17:46:21 +00:00
Yufeng He	e5df5b22e3	fix(scripts): retry model calls and surface degraded release notes (#7535) ... * fix(scripts): retry model calls and surface degraded release notes The stable-release notes generator fell back to pull-request titles on any model error and the finalize workflow stayed green with no visible degradation. Four recent stable releases lost every summary batch and the highlights call to 60s timeouts without anyone noticing. The completer now retries each request (default 2 retries, exponential backoff with jitter) on timeouts, network errors, 429, and 5xx, while 4xx fails immediately. After three consecutive failed summary batches a circuit breaker stops paying the timeout for the rest and skips the highlights call, since the model side is down rather than slow. Fallback warnings are emitted as :⚠️: workflow commands so they render as run annotations, and a degraded-run block is appended to GITHUB_STEP_SUMMARY when any fallback occurred, distinguishing fully degraded (titles only) from partially degraded runs. Refs #7523 Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * ci: retrigger after flaky fork-dispatch test failure * fix(scripts): create the step-summary parent dir before appending The ubuntu CI shard of this PR failed the summary test with ENOENT on readFileSync after appendFileSync: the append reached the runner fine, but the parent of the summary path can be missing there. Create it recursively first, which is what the function's contract (the note must land in the summary file) already implies. * fix(scripts): assert the degraded summary through the fs mock test-setup.ts no-op mocks appendFileSync for the whole scripts project, so the new test's real-file read could never see the write under test:scripts. Assert on the mock call instead. * fix(scripts): keep summary-write failures from costing the notes and make the reset test real appendDegradedStepSummary now runs through tryAppendDegradedStepSummary so a filesystem error on the auxiliary summary cannot kill the release before the notes are written, and the circuit-breaker recovery test's mock returns valid summary payloads so the reset path is actually exercised. * fix(scripts): escape model-controlled text in workflow commands Warnings can carry model output (parse errors, PR-derived fields) into :⚠️: commands. Percent-encode %/CR/LF so a forged ::error:: line cannot emit a second runner command, and collapse newlines in the step-summary markdown list so a multi-line warning does not break the list. Closes the P1 from PR review. * fix(scripts): bound release-note AI generation time * fix(scripts): log model request retries to stderr Retry attempts in createOpenAiCompleter were silent, so an oncall engineer could not distinguish slow model responses from exponential- backoff retries in the CI log. Emit a one-line stderr log on each retry with the attempt count, the triggering error, and the backoff delay. Plain stderr (not :⚠️:) so transient retries that recover do not become run annotations. * fix(scripts): escape retry log workflow commands --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: yiliang114 <effortyiliang@gmail.com>	2026-07-27 14:55:11 +00:00
易良	ad2252401b	ci(release): comment released-in version on merged PRs (#7814) ... * ci(release): comment released-in version on merged PRs After a stable release is published, comment on each merged PR in the release range with a link to the release tag, so contributors can find which version shipped their change directly from the PR timeline. Collects PR numbers from squash-merge commit subjects between the previous and current stable tags, posts a marker-tagged comment, and skips PRs that already carry the marker (re-run safety). Nightly and preview releases are out of scope, since finalize only handles stable tags. * fix(ci): narrow release PR extraction * test(ci): cover release comment safety checks	2026-07-27 14:51:53 +00:00
Shaojin Wen	2e08486b52	fix(triage): carry the /verify lane's hardening across to /tmux (#7753) ... * feat(triage): add sandboxed /verify deep-verification lane @qwen-code /verify on a PR now runs a local-verification-style evidence round in the isolated /tmux sandbox contract (container, token-free agent env, loopback model proxy, author-write gate) and publishes the report via a separate PR-code-free job: - new verify job: merge-ref checkout at depth 2 (base tip + PR head for A/B), skills pinned from base so the tree under test can never rewrite its own verifier, PR-planted tmp/*-verify-* artifacts dropped, git exec-vector sweep for the persistent workspace, agent verdict allowlisted before it reaches workflow outputs - new publish-verify job: upserts one marker comment (running status -> final report), HTML-escapes the untrusted report, reports skip/na/ prepare-fail/infra outcomes explicitly since /verify is always an explicit request - new verify-pr skill: A/B load-bearing proof, vacuity check on new tests, mock-free wire-oracle harnesses, targeted gates, fixed report/ verdict/assertions artifact contract, counts-are-sacred rules - triage skill Stage 2c now names /verify (not just /tmux) as the trigger to recommend when a PR's central claim needs behavioral evidence The verify check-runs ride the issue_comment event, which the finalize workflow's event == "pull_request" universe structurally excludes, so they cannot pollute the CI table or the deferred-approval gate. * feat(triage): teach /verify round continuity and artifact-matched methods Fold two more hand-verification patterns into the verify lane: - round continuity: the resolve step snapshots the previous verify report (if any) into the agent context before the status upsert overwrites it, and the skill re-checks each prior finding at the new head (fixed/stands/superseded), scoping new probes to the delta - harness quality: prefer configuration seams over module interception, encode the upstream's real semantics in the fake peer, add decoy targets - artifact-matched methods: per-commit load-bearing tables for multi-commit PRs; workflow/CI PRs get embedded-script replay against real data, repo lint gates, and day-one trigger cost math from real event history; every new config knob must trace to an observable effect, and default-path dispatch combinations get probed - findings quality: blockers enumerate blast radius, demonstrate the sharpest consequence end-to-end when budget allows, and carry a collapsed minimal suggested fix preserving the original commit's intent * feat(triage): host /verify evidence images and encode quantified-A/B rules Borrow the image-evidence and quantified-verification patterns from hand-run rounds (#7265, #7471, #7686 r2 and the pr-assets convention): - publish-verify now hosts agent-produced evidence/*.png on the pr-assets branch (verify/pr<N>-<run>-<attempt>/) and appends them below the escaped report. Untrusted-payload discipline: strict filename allowlist, 8-image / 2 MB caps enforced in the find predicates, racing-push retry, and every failure degrades to a text-only comment. VERIFY_ASSETS_REMOTE is a test seam; the block was dry-run against a local bare remote covering hosting, hostile filenames, oversize files, dotfiles, missing branch, and no-image runs - skill: evidence images are named as kebab-case captions binding image to claim, before/after pairs over lone after-shots; follow-up rounds lead with a previous-finding status table (fixed/stands/superseded/declined, with adjudication) and re-measure instead of diffing the old report; size/perf claims get measured-metric Δ tables with residual deltas accounted for; unreachable branches get the configuration that reaches them constructed; defensive guards get their accept path checked against real production artifacts, not just mocked rejects * fix(triage): address /review suggestions on the verify lane - skill: local invocation resolves --repo and passes it to every gh call - skill: call out the dependency confound when the base A/B side reuses the PR-installed node_modules and the PR touches package.json/lockfile - workflow: document the pin step's bootstrap logic — issue_comment jobs run the default branch's YAML, so base always carries the verify-pr skill by the time this job exists * fix(triage): harden /verify gate, comment budget, and evidence hosting per review Address review round 5078770575 items 1-3 plus the cheap follow-ups: - authorize: /verify now requires write from BOTH the PR author (whose code runs) and the commenter (who spends a scarce runner slot + model budget) — a drive-by account can no longer burn 45 minutes of ecs-qwen on someone else's PR; duplicates check once; /tmux and /triage gates unchanged. Replayed 8 principal scenarios against a stubbed gh - authorize acks /verify with the eyes reaction from the always-hosted job, so a queued/saturated sandbox pool no longer means total silence - publish: emit_block escapes FIRST and caps the escaped size (45 KB for the report) — a raw-side cap let dense <>& content inflate past GitHub's 65,536-char comment limit, 422 the post, and strand the running status with no report at all; iconv -c keeps a UTF-8 sequence split by the byte cut (likely, given the mandated 中文 summary) from shipping broken; replayed: 50 KB dense report -> 45,873-byte body - publish: image cap is byte-exact (-size -2097153c; find's -2M rounds sizes UP to MiB, silently making the documented 2 MB cap 1 MiB), bytes must carry the PNG magic (extension is attacker-choosable), duplicate sanitized names dedupe instead of overwriting + double-rendering, and dropped images are reported in the comment instead of vanishing - publish: weak terminal notices (cancelled/infra/skipped/n-a) only replace this run's own running status; a previous round's real report survives as the marker comment and the notice posts fresh - publish: report.md/assertions.json lookups pin the artifact-dir shape and sort (bare find -name order is filesystem-dependent); the verify job's verdict.txt lookup sorts likewise - verify: global npm install runs from RUNNER_TEMP (the persistent workspace still holds the PREVIOUS run's tree, whose .npmrc would apply to a root install); both cleanup passes remove leftover tmp/ worktrees (git worktree prune alone only drops metadata); the run step no longer re-chowns 50k node_modules files; pr-assets clone sets its committer identity once so the racing-push rebase retry can commit - skill: worktree guidance now tells the agent to remove its base tree itself, with the workflow sweep as backstop only * fix(triage): close runtime-plant and stale-RUNNER_TEMP channels in /verify Address review round 2 (comment 5079157987) and the CHANGES_REQUESTED round on the verify lane: - run step re-sweeps tmp/*-verify-* AFTER npm ci/build and before the agent starts: the pin step's sweep runs before PR lifecycle scripts (postinstall etc.), which could re-plant a fake artifact dir whose zeroed timestamp deterministically wins the sorted collector. From the sweep on, only the agent writes those dirs; a steered agent forging its own artifacts remains the documented advisory-report residual - RUNNER_TEMP verify-results/verify-context are rm'd before mkdir: the pool is persistent and runner temp hygiene is runner-managed — a stale report or previous-report.md from ANOTHER PR must never ride along - symlinks are stripped from verify-results before upload: actions/upload-artifact dereferences them, so a node-planted link would exfiltrate whatever it points at into the artifact - a trusted commenter invoking /verify on a PR whose author lacks write now gets an explanation comment from the hosted authorize job instead of total silence (the commenter is checked first; drive-by accounts and API errors still get nothing); job timeout 45->60 so a slow install can never let the JOB limit kill the agent past its own graceful 25m budget - stale tmp/base-tree (skill's canonical scratch worktree) is removed by name at job start — a plain dir isn't git-registered, so the worktree sweep alone misses it and the next worktree add would fail - scripts/tests/qwen-triage-workflow.test.js gains a verify-lane describe block: an 8-arm stub-gh replay of the dual principal gate (drive-by deny, author-without-write deny + explain flag, self-comment dedupe, 404 fail-closed, /tmux and /triage unchanged) plus guards for the post-prepare sweep placement, the symlink strip, and the RUNNER_TEMP resets — the replay found this commit's sweep edit had silently not applied, which is exactly the regression class it exists to catch * fix(triage): close proxy-hijack, gate-bypass, and false-verdict paths in /verify Address the Codex /review round (19 findings) and the bot's follow-up. Each fix was replayed locally; the proxy fix has a decisive A/B. Gate and routing: - the shell command match is case-insensitive: GitHub Actions expression comparisons ignore case, so `@QWEN-CODE /VERIFY` reached the step and fell through to the commenter-only branch — running the PR author's code with the author never checked - the verify ack and denial notice require github.event.issue.pull_request: /verify on a plain issue was acknowledged but could never report - publish-verify joins the verify job's per-PR concurrency group, and a failed PATCH falls back to posting fresh instead of going silent Untrusted-input paths: - the model proxy binds an EPHEMERAL port, reports it through a root-owned file, and its health check must echo a per-run nonce with the recorded PID alive. A/B with a squatter on 8787: the old code's proxy dies EADDRINUSE yet still reports enabled and points qwen at the squatter; the new code comes up unaffected on an ephemeral port - worktree-scoped git config is deleted before hooksPath is resolved: `extensions.worktreeConfig` is allowlisted and .git/config.worktree is invisible to `git config --local`, so a prior run could set core.hooksPath=/ and make the hook sweep's recursive delete walk / as root (verified locally). The sweep now also refuses any hooks path outside the repository's git dir - marker-comment lookups accept only bot-owned comments that START with the marker: any user can paste the marker and divert the bot into PATCHing a stranger's comment - the upload staging dir is re-flushed after npm lifecycle scripts Honest verdicts: - the docs-only classifier no longer uses a pipeline (grep -q made the writer take SIGPIPE, so under pipefail a long file list with an early code file classified a code PR as docs-only and skipped verification), and executable markdown/YAML (.qwen, .github/workflows, scripts) is classified as behavioral before the extension rule - tee's status is checked alongside qwen's: a full results volume made a truncated evidence stream publish as pass - 137 is split by elapsed budget into watchdog timeout vs crash/OOM - the agent's verdict is honored only for VERDICT=pass with a report and zero failed assertions; otherwise the process outcome headlines and the scope paragraph says the run did not complete - verdict.txt is read through a bounded prefix (SIGPIPE under pipefail) Skill contract corrections: per-commit tables only when the commits are reachable at depth 2 (else aggregate + Not covered); internal workspace symlinks must have their realpaths asserted before a base control is trusted; repo lint gates and event-history cost math are qualified to what the token-free container can actually run; --repo is never inferred from `origin` (a fork holds a different PR under the same number). Tests: 9 new guards, all mutation-verified (reverting each fix turns one red), including an executable escaping/size-cap/UTF-8 test for the publisher's own emit_block and a fix to the earlier command-file test, which matched the tmux job's identically named step. * fix(triage): re-establish the /verify trust boundary after PR code runs Third review round (31 findings). The unifying defect: everything the lane pinned or swept happened BEFORE npm ci/build executed PR-authored lifecycle scripts as node, so each control could be undone afterwards. Trust boundary, rebuilt in order before the agent starts: - kill every process owned by the build user and fail the step if any survives — a detached postinstall child could otherwise wait out each one-shot sweep and re-plant afterwards - re-pin .qwen from the base commit again, now root-owned and read-only: the prepare step chowns the workspace to node, so a lifecycle script could rewrite the very skill that defines /verify-pr - give the agent a fresh HOME/QWEN_HOME: qwen loads user-scope file commands from $HOME/.qwen, and /home/node belongs to the build user, so a planted commands/verify-pr.toml could shadow the pinned skill - the model proxy now requires a per-run bearer token, closing the blind-localhost-scan path to an unauthenticated signer for the real model credential (a command the agent itself launches still inherits it — documented residual, not closed) Authorization and lifecycle: - re-verify the PR author's write permission at execution time and pin the authorized head OID; refuse if the checked-out HEAD^2 differs, so a push during the runner wait cannot smuggle in unreviewed code - validate each principal separately: an empty author vanished in word splitting and left only the commenter checked - honor MAINTAINER_ECS_RUNNER_DISABLED with an explicit notice instead of queueing forever against a disabled pool - status comments carry a machine state marker; inferring 'running' from prose let a report quoting that sentence be overwritten - previous-report.md snapshots the newest substantive report, never a weak/cancelled notice, so prior findings survive into the next round - bot-identity lookup failures fail closed instead of widening the ownership filter to every user's comments - publish-verify uses a per-run concurrency group: a per-PR group holds only one pending job, so a second /verify could cancel a completed run's pending publisher Correctness: - install/build failures are classified: signals, ENOSPC, registry and network errors are infra-error, not a PR verdict - watchdog classification measures the child's own elapsed time, not shell-global $SECONDS which includes proxy setup - assertions.json must be three non-negative integers with a positive total and total == pass + fail before it counts as evidence - the proxy keeps its upstream deadline armed until the body ends and aborts upstream when the client disconnects - cleanups remove .qwen/tmp itself: PR code can make it a symlink, and globbing below it deleted the target's contents as root (verified) - emit_block materializes the escaped text and truncates on a character boundary via node — iconv -c passes an incomplete trailing sequence through on BSD (measured), which the new test caught Skill: local mode requires the same isolation CI provides and must not assume HEAD^1/HEAD^2 on a plain head checkout; shallow boundaries make rev-list counts unreliable for per-commit claims; never run scripts/lint.js with no arguments (it runs prettier --write and rewrites the tree under the harnesses); a vacuity check must fail the intended assertion, not the import. pr-workflow.md now says both sandboxed lanes need the author to have write, so triage stops recommending a guaranteed denial on external PRs. Tests: 9 more guards, all mutation-verified, including executable replays of the docs-only classifier (SIGPIPE + executable-markdown cases), the uppercase-command gate, the empty-principal deny, and the untrusted-image hosting path against a bare pr-assets remote. * test(triage): pass the classifier fixture through a file, not argv The new docs-only classifier replay passed on macOS and failed on CI with `Cannot read properties of undefined (reading 'trim')`: its 60,001-entry fixture is ~889 KB and was passed as a single argv element. Linux caps one argument at MAX_ARG_STRLEN (128 KB), so the spawn failed with E2BIG and stdout was undefined; macOS has no per-argument limit and only a ~1 MB total, so the same call succeeded locally (verified both). Write the list to a temp file and pass the path. The harness now also asserts the spawn succeeded, so a future spawn failure reports itself instead of surfacing as a TypeError on undefined output. * fix(triage): make the /verify report match what the run actually produced Three publisher findings, all introduced by my own previous round: - an artifact download failure (the step is continue-on-error) let the full-report path run with no results: the headline read 'completed' and the scope paragraph claimed the A/B, the harnesses and the gates had run when nothing had been delivered. The download outcome is now an input, and its failure gets its own body saying the results could not be retrieved - the prepare-failure branch ignored the verdict the prepare step had just computed, so an install killed by a registry outage or OOM (classified infra-error) still told the author 'this is treated as a PR failure verdict rather than an infrastructure failure' — the exact opposite. It now branches on the verdict, and an infra-classified prepare failure is a weak body that cannot overwrite a real report - weak notices were being snapshotted as the follow-up round's previous-report.md: they lack the running marker, so 'newest non-running comment' selected them. Bodies that carry findings now mark themselves (qwen-triage:verify-substantive) and the snapshot selects on that marker. A/B on the real jq: report A then cancelled B now snapshots A (101), the old filter picked B (102) Tests: 4 more guards, all mutation-verified — the publisher is rendered for each outcome with a stubbed gh and the assertions read the body it would post, and the snapshot test runs the workflow's own jq program verbatim against a paginate-shaped fixture. * fix(triage): stop PR build output from masquerading as an infra failure Two review findings plus a test-helper hazard: - classify_failure grepped the prepare log for bare words like ENOSPC and ETIMEDOUT, but that log is written by PR-controlled code: a genuine build failure that merely prints 'expected ETIMEDOUT to equal ok' would be published as an infrastructure incident, telling the author to re-run something that fails identically. The patterns are now anchored to lines only npm's reporter or the kernel emits ('npm ERR! code E…', 'npm ERR! network …', kernel OOM, bare 'Killed'); a signal exit still needs no log evidence. Replayed 10 cells: four PR-authored logs quoting infra words stay 'fail', five real diagnostics and one signal exit are 'infra-error' - the two execution-time controls added last round — re-verifying the author's permission after the runner wait, and refusing a head that moved since authorization — had no tests. Both are now executed: the re-auth snippet against a stubbed permission API (write proceeds and pins head_oid; read skips with a publishable reason), and the pin step against a real git repo with a real merge commit (matching head proceeds, moved head exits non-zero) - add a stepIn(job, step) test helper. Several step names exist in both the tmux and verify jobs, and the unscoped step() returns the first match, so a verify-lane assertion silently tests the tmux copy — that has now bitten this suite three times, including in this commit. * docs(triage): teach verify-pr test-only PRs, differential oracles, gate liveness Fold techniques from the round-2 verification on #7620 (an ANSI parser PR) that the skill had no equivalent for: - test-only PRs get their own method: a mutation A/B across TEST FILES (same mutants of the unmodified production file, only the test file swapped), reporting killed/total on both sides, requiring that no mutant regressed from killed to survived, checking that the killing assertion is the one the commit claims to have strengthened, and adjudicating every survivor as coverage gap or defect with independent evidence rather than by inspection - when the code emulates a known implementation, that implementation is the oracle: feed identical input to both and report disagreement counts per side, lift reference tables verbatim out of the shipped dependency, and build the corpus from bytes captured off a real producer alongside synthesized sweeps - prove a gate is live before citing it: plant a violation the linter must catch, confirm it is reported, remove it — a linter that matched no files exits 0 exactly like one that passed - attribute pre-existing failures by byte-identical failing file AND test names on both sides, with deltas, not just totals - when the base is far behind, verify the merge: trial-merge into current main, confirm it is conflict-free, and re-run the affected suite on the merged tree - round continuity gains its one legitimate shortcut: a production file proven byte-identical (sha256 quoted at both heads) carries prior evidence forward by construction * style(triage): reflow verify-pr skill to prettier's markdown wrapping The previous commit's added paragraphs were hand-wrapped and prettier --check flagged the file; the repo runs prettier over all of it. * test(triage): cover the disabled-runner-pool notice The kill-switch path had no test: a refactor could drop the notice and leave a /verify request acknowledged with 👀 but permanently unanswered, since the verify job refuses to start and publish-verify skips with it. Fold the step into the existing PR-guard loop (now scoped through stepIn, so it cannot match a same-named step in another job) and assert the parts that make the answer useful — the kill-switch and permission conditions, both languages, the alternative it points at, and the verify job's own exclusion of the disabled pool. All three mutations turn it red: removing the step, dropping its PR guard, or letting the verify job queue against the disabled pool. * fix(triage): repair a step-killing PIPESTATUS read and six forgeable controls Sixth review round, 12 findings. Several are regressions from my own two previous rounds; the first would have broken every single run. - `AGENT_STATUS=${PIPESTATUS[0]}` is itself a command and resets PIPESTATUS, so the next line's ${PIPESTATUS[1]} was unset and `set -u` aborted the step immediately after the agent finished — before artifact collection, the verdict, or anything else. Verified by replaying the exact structure: 'PIPESTATUS[1]: unbound variable'. Both elements are now snapshotted in one command - concurrency predicates were broader than the job conditions they guard, and GitHub evaluates concurrency BEFORE the job `if`: a /verify comment entered the triage job's shared per-PR group (where it could displace a pending /triage and then skip), and a /verify queued while the runner kill switch was on did the same to a real verification. Both predicates now match their job's runnable set exactly - an outward-resolving .git/hooks entry was only warned about and left in place, so the next root-owned git command would run it. It is now unlinked without traversing its target, a root-owned hooks directory is restored, and core.hooksPath is unset - the second .qwen pin re-derived HEAD^1 from git metadata after the workspace, including .git, had been handed to the build user. The base OID is now recorded while .git is still root-owned and the re-pin archives that content-addressed OID - classify_failure took both of its inputs from PR-controlled sources: a lifecycle script can exit with a signal status and can print any line the log patterns matched, turning its own deterministic breakage into 'infrastructure, please re-run' — which hid the failure and preserved a stale report. No infra verdict is derivable there, so the prepare step reports `fail` and lets the embedded log speak for itself - cleanups descended through PR-writable parents: `.qwen` itself can be a symlink, and the worktree sweep trusted git metadata with only a lexical prefix check. Symlinks are unlinked without traversal and worktree paths must canonicalize inside the workspace. Replayed all three escapes - skipped and docs-only outcomes upload no artifact, so the new download-failure branch pre-empted them and made their real reason unreachable; they are answered first now - a run that crashed before writing report.md still claimed the substantive marker, letting a headline overwrite the previous round's evidence. The marker now requires a report Skill: the byte-identical shortcut needs the whole input closure, not one file hash; the credential-free local path cannot call `gh` at all (fetch the metadata outside and mount it read-only); and the A/B base is `baseRefOid` in local mode, not `HEAD^1`. Tests: 7 new guards plus 4 updated to the new shapes, all mutation-verified (50/50). * fix(triage): answer dropped /verify requests and prove the proxy rejects Maintainer review (yiliang114), 7 items: - a third /verify while two runs are in flight is dropped by the concurrency group with no job and therefore no comment. The hosted authorize job now counts this workflow's other in-flight runs and says so; an API hiccup leaves the request alone rather than denying it - the proxy's bearer check had no executable test. It now starts the real proxy against a real upstream and issues real requests: no header and a wrong token are 401, this run's token is 200, and a route other than /chat/completions is 403 — with the health endpoint echoing the nonce - the 502 path forwarded the raw upstream error, which can name resolved hosts and TLS detail to PR code. It logs server-side and returns a generic failure - publish-verify inherited the 360-minute default; it downloads one artifact and posts one comment, so it is bounded at 10 - removing the log classifier last round left the comment block it replaced, which still said failures are classified from the exit status and the log. Deleted - that removal also left every install failure reported as the PR's fault, including a registry outage. There is exactly one signal here PR code cannot write — asking the registry ourselves, as root, with the container's resolver — so an install failure is downgraded to infra-error only when that probe fails. It proves reachability now rather than at failure time, so it can only ever downgrade, never confirm; a build failure has no equivalent and stays the tree's problem - the skill's local-invocation warning ran into the preceding sentence, which GFM renders as one paragraph Tests: 5 new guards, all mutation-verified (55/55). * fix(triage): resolve hooks hermetically and mirror symlink guards at job end Maintainer review round (doudouOUC), 6 findings. Two were Critical and both reproduced: - the hooks sweep resolved its path with the ambient git config in play. With a global core.hooksPath set — which the reviewer has and I do not, which is why my earlier replay showed a false pass — `git rev-parse --git-path hooks` returns that global path, the in-git-dir guard reads 'outside', and a planted `.git/hooks` symlink survives untouched. A/B: old code leaves the symlink under a global hooksPath, new code removes it in both environments and never touches the link target. Resolution now runs with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM pointed at /dev/null - the END-of-job cleanup still used the bare `rm -rf .qwen/tmp` that the start-of-job cleaner was hardened against two rounds ago. The agent executes PR code between the two, so the end is no safer than the start: it now unlinks symlinks without descending and canonicalizes worktree paths inside the workspace before deleting Plus four suggestions, all valid: - the saturation notice counted this workflow's in-flight runs across every PR while the concurrency group is per-PR, so a run on another PR would trigger a warning about a queue that does not exist. It now matches on the PR title (the only per-PR handle an issue_comment run record carries) and stays silent when that cannot be resolved - the skill recommended `require.resolve` for the workspace-realpath check; these packages are ESM-only with import-only exports, so it throws ERR_PACKAGE_PATH_NOT_EXPORTED and reads like a missing module. Verified, and replaced with `readlink -f node_modules/@qwen-code/...` - the symlink-escape test inherited the developer's git config, which is what hid the first finding. It now runs with global/system config neutralized AND repeats the case with a global core.hooksPath planted - the publisher's build-phase arm was never rendered by any test (every case used 'install'), so a typo in that command name would have shipped. Now covered, along with an unrecognized phase Mutation-verified 4/4. The hooks guard needed a discriminating assertion: git's own `*.sample` files must survive the sweep, because the outward-path fallback removes the whole directory and would otherwise satisfy a bare 'planted hook is gone' check. * fix(triage): count only /verify runs for saturation, and test the PATCH arm Bot review round, 2 suggestions, both valid: - the saturation notice matched runs by PR title, which narrowed to this PR but not to /verify. /triage and /tmux live in their own concurrency groups, so two of those in flight would warn about a verify queue that is actually empty. It now also requires the run to have a job named 'verify' — the run record carries no command, but its job list does. Replayed: two non-verify runs stay silent, two verify runs warn - every publish fixture returned an empty comments listing, so the PATCH arm was never executed: a broken PATCH would have stranded the running status comment and posted a duplicate below it, with the suite green. The publisher now runs against a stubbed listing and the test asserts which verb went to which comment id — bot-owned live status is PATCHed in place, an absent comment posts fresh, and a marker comment owned by someone else is left alone and posted around Mutation-verified 3/3: counting every command, never PATCHing, and accepting foreign-owned markers each turn one test red. Two stub bugs found while writing these, both mine and both silent: ${*#pattern} applies per positional parameter rather than to the joined string (yielding a wrong run id), and the paginate fixture needs one array per page, not an array of pages. * fix(triage): fix the real silent drop and drop the step built on a wrong premise Review round 4. The blocker was mine twice over: the saturation notice I added last round had GitHub's concurrency semantics backwards, and the silent drop it claimed to cover was somewhere else entirely. - GitHub cancels the OLDER pending run in a group and admits the new one (confirmed against the workflow-syntax reference). My step told the person who had just typed /verify that their request might be dropped, when theirs is the one that runs — and said nothing to the person whose queued run actually died. This PR already had it right in publish-verify's own comment, so the file contradicted itself and the user-facing copy followed the wrong half. The step is removed rather than reworded: with the fix below there is nothing left for it to warn about, and it cost 2+N API calls on every /verify. - the actual drop: a verify job cancelled while still PENDING never reaches a runner, so its outputs block — where the "|| github.event.issue.number" fallback lived — is never evaluated. publish-verify then read an empty PR_NUMBER, hit its own guard and exited 0, making the cancelled branch unreachable in exactly the scenario that produces cancellations. The fallback now lives where the value is read. Reproduced both arms by executing the real step: with a number the cancelled notice posts, with an empty one it only warns. - same one-line class in publish-tmux, fixed alongside. Two copy defects from the classifier removal, both mis-attribution pointed the other way: - the infra-error body still named a signal/OOM kill and a full disk, none of which the current prepare step can produce — infra-error now requires npm ci to fail AND the registry probe to fail. It names that condition only, and offers a re-run instead of asserting it is the fix. - the code comment above it still described the deleted classifier. Also fixes the indentation break an earlier scripted edit left in the publish body builder, and replaces the saturation test with one that executes the cancelled path. Mutation-verified 2/2; the copy needed its own guard, since reverting the wording alone left every test green. * docs(triage): teach verify-pr survivor accounting and observability regressions Fold techniques from the re-verification on #7709 that the skill had no equivalent for: - the mutation matrix must report the mutations that changed NOTHING, not only the ones that failed. Each survivor gets classified as an ordinary coverage gap or as dead code — a guard whose deletion leaves every test green is one of those two, and the difference is what the author needs. Survivors mirroring a pre-existing gap are labelled as such, and the set is framed as completeness reporting rather than merge conditions - the sharper case that report demonstrates: a test that passes for the WRONG REASON. If deleting the new guard leaves its own new test green, that test is pinned by an earlier early-return, not by the change, and asserts nothing about it. Name what actually pins it - and do not generalize from one dead guard to its siblings: the same report shows a clause that is unreachable on one path while being the only protection on another. Check each, report the contrast - observability regressions: when a change suppresses output, follow the value before calling the suppression correct. A bare catch on the path plus a field with no readers anywhere in the repo means the cause is now unobservable even in devtools — a real loss that no behavioural assertion can see - report structure gains a Corrections section: when an earlier round or bot comment described the code inaccurately, state the correct fact with evidence and label it as a correction to the description, not a request to change code. A wrong description left standing costs the next reader more than the original finding did * fix(triage): carry the /verify lane's hardening across to /tmux The /tmux job executes the same untrusted PR code, as the same user, on the same persistent self-hosted pool as the /verify lane that #7710 hardened. Five of those controls had no equivalent here. Each was found on the verify side by reproducing an attack or a failure, not by reading the code, so the same evidence applies unchanged. - the model proxy bound a FIXED port (8787). PR lifecycle scripts run before it, so a detached child can squat that port: the real proxy then dies with EADDRINUSE while the health probe succeeds against the squatter, and the agent takes its chat completions. Now an ephemeral port published through a root-owned file, a per-run nonce the health endpoint must echo, and a liveness check on the PID we started. Replayed with 8787 occupied: the proxy comes up on an OS-chosen port and answers with the nonce. - nothing swept planted artifact directories. npm ci/build run the PR's lifecycle scripts, which can create tmp/<name>-tmux-<ts>/ holding a report.md and a transcript; the collector globs *-tmux-* and the publisher takes the first match, so a planted directory could supply the comment's contents. Swept after the last PR-controlled process and before the agent. - the global npm install ran with the workspace as cwd, where the PREVIOUS run's checked-out tree still sits. npm reads a cwd .npmrc, and a --registry flag does not override script-shell or hooks, so that config reached a root-privileged install. It now runs from RUNNER_TEMP. - the end-of-job cleanup globbed below .qwen/tmp. PR code ran in this workspace, so either .qwen or .qwen/tmp can be a symlink out of the tree — verified on the verify lane, where the glob deleted the link target's contents as root. Symlinks are unlinked without descending. - emit_block capped the raw log then escaped it. Escaping inflates every & < > by 4-5 bytes, so dense content can push the assembled body past GitHub's 65,536-character comment limit, 422 the post, and leave no comment at all. It now escapes first, caps the escaped bytes, and truncates on a character boundary via node — BSD iconv -c passes an incomplete trailing UTF-8 sequence through unchanged. Tests: a tmux-lane-parity suite, all six mutations verified (restoring the fixed port, dropping the sweep, moving the install back, dropping the symlink guard, reverting to a raw-side cap, and dropping the character-boundary truncation each turn one test red; a no-op control correctly changes nothing). One pre-existing assertion updated: it pinned emit_block's old inline-capture shape, and the guarantee it protects — a render failure is caught — is asserted in the new form. Also adds the regression guard for the publish-tmux PR_NUMBER fallback that landed in #7710 without one: a job cancelled while pending never evaluates its outputs, so without the fallback the result comment silently does not post. * fix(triage): address review — symlink guard, artifact strip, bearer auth (#7753) * fix(triage): address R2 review — proxy parity, bearer wire tests, process kill (#7753) * fix(triage): address R3 review — publisher parity, dedup ownership, cap budget tests (#7753) * fix(triage): address R4 review — drop redundant tmux-lane .mjs guards (#7753) * fix(triage): address R5 review — tmp symlink sweep guard, proxy timer clear (#7753) * fix(triage): address R6 review — hoist proxy timer out of try, dead-upstream 502 tests (#7753) * fix(triage): address R7 review — make proxy watchdog idle, end stalled response (#7753) --------- 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>	2026-07-27 08:28:38 +00:00
Shaojin Wen	c003e17181	fix(autofix): answer every review thread, resolve the ones actually fixed (#7758) ... * fix(autofix): answer every review thread, resolve the ones actually fixed Two gaps made a handled finding look unhandled on #7731. A finding the bot declined, deferred, or escalated keeps its thread open, but its reason was recorded only in the round summary — a separate comment. The reviewer who opens that thread sees their finding answered by silence. The agent now writes comment-replies.json and the push step posts each reason as a reply on that finding's own thread, leaving the thread open. Replies are neutralised like the summary body, since a reply is model output posted verbatim under the bot identity. Resolution was also keyed on "did I edit a file this round", so a Critical an earlier commit had already fixed stayed open and read as unaddressed. Key it on the finding being resolved in the code, which covers a prior commit's fix the agent re-verified still holds. * style: apply prettier to the new review-reply test * test(autofix): update stale escape-site comment from five to six sites * fix(autofix): reply at thread roots and guard the reply id, per review (#7758) Address the maintainer review on the in-thread reply mechanism: - Map each reply to its thread's top-level comment before posting. The feedback step lists every review comment, replies included, so an rc:<id> can be a reply id; GitHub rejects a reply aimed at another reply, which would have left escalated findings answered by silence. Hoist the threads GraphQL fetch above both the resolve and reply blocks so a reply-only round still has it, and fall back to the id as given past the page cap. - Skip any id already present in resolved-comments.txt so a finding is never both resolved and replied to; the match tolerates the rc: prefix and a trailing CR like the resolve block's own parsing. - Mutation-verify the previously untested ^[0-9]+$ id guard (the boundary between a model-authored id and an arbitrary API path), the -f body= field name, the type=="array" skip, and the new cross-check. --------- Co-authored-by: wenshao <wenshao@example.com>	2026-07-26 23:39:13 +00:00
易良	e8e15e3fb0	fix(ci): rename triage status marker to avoid duplicate-guard collision (#7723) ... * fix(ci): rename triage status marker to avoid duplicate-guard collision The lifecycle status comment used <!-- qwen-triage stage=status --> which the triage agent's duplicate guard sometimes matches as a prior stage marker (stage=N), causing it to exit without posting the actual triage analysis. Rename to <!-- qwen-triage-lifecycle --> so the guard never matches infrastructure comments. Fixes the probabilistic silent-triage gap seen in #7713. * fix(ci): update test assertions for renamed triage lifecycle marker * fix(ci): sync triage finalize status marker * test(ci): cover triage status marker parity * fix(ci): preserve triage marker consumers * test(ci): pin triage lifecycle marker checks * fix(ci): add bot-author filter and startswith to triage status lookups qwen-triage.yml's two status-comment lookups matched any comment containing the marker — a human reviewer quoting the marker would have their comment overwritten by the bot PATCH. Add select(.user.login == $bot) (already present in qwen-triage-finalize.yml) and resolve BOT_LOGIN via gh api user. Both workflows used contains() for marker matching; startswith() is a strict improvement since every status comment body begins with the marker. This prevents the demonstrated defect where the finalize step resolved EXISTING_ID to a bot-authored Stage comment that merely quoted the marker. * fix(ci): guard triage status bot lookup --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-07-26 15:51:22 +00:00
jinye	9bdc62c74b	perf(cli): replace comment-json settings parser (#7747) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-26 14:42:51 +00:00
Shaojin Wen	e3cf1c3b45	revert: drop the stale-base un-park recovery (#7602) (#7640) ... Reverts #7602. The Fleet Shepherd (qwen-fleet-shepherd.yml) now keeps the bot fleet within 25 commits of main by proactively update-branching, and #7595 already retries a stale-base gate rejection at ANY behind distance instead of parking — so a PR no longer parks because its base went stale. That leaves #7602 firing only on a PR parked by a GENUINE failure that later drifted behind main, where re-arming it just re-runs a real failure on a fresh base — speculative, near-zero value, and it carried its own autofix-handoff marker plus scan/report logic and tests. The retroactive cases it was built for (PRs parked before #7595) were already recovered by hand. Keeps #7595 (reactive stale-base gate recovery below the shepherd's threshold) and #7554 (check-driven stale-base sync) — both cover the sub-25-behind window and triggers the shepherd does not. Co-authored-by: wenshao <wenshao@example.com>	2026-07-26 14:26:46 +00:00
Shaojin Wen	883094da36	feat(triage): add sandboxed /verify deep-verification lane (#7710) ... * feat(triage): add sandboxed /verify deep-verification lane @qwen-code /verify on a PR now runs a local-verification-style evidence round in the isolated /tmux sandbox contract (container, token-free agent env, loopback model proxy, author-write gate) and publishes the report via a separate PR-code-free job: - new verify job: merge-ref checkout at depth 2 (base tip + PR head for A/B), skills pinned from base so the tree under test can never rewrite its own verifier, PR-planted tmp/*-verify-* artifacts dropped, git exec-vector sweep for the persistent workspace, agent verdict allowlisted before it reaches workflow outputs - new publish-verify job: upserts one marker comment (running status -> final report), HTML-escapes the untrusted report, reports skip/na/ prepare-fail/infra outcomes explicitly since /verify is always an explicit request - new verify-pr skill: A/B load-bearing proof, vacuity check on new tests, mock-free wire-oracle harnesses, targeted gates, fixed report/ verdict/assertions artifact contract, counts-are-sacred rules - triage skill Stage 2c now names /verify (not just /tmux) as the trigger to recommend when a PR's central claim needs behavioral evidence The verify check-runs ride the issue_comment event, which the finalize workflow's event == "pull_request" universe structurally excludes, so they cannot pollute the CI table or the deferred-approval gate. * feat(triage): teach /verify round continuity and artifact-matched methods Fold two more hand-verification patterns into the verify lane: - round continuity: the resolve step snapshots the previous verify report (if any) into the agent context before the status upsert overwrites it, and the skill re-checks each prior finding at the new head (fixed/stands/superseded), scoping new probes to the delta - harness quality: prefer configuration seams over module interception, encode the upstream's real semantics in the fake peer, add decoy targets - artifact-matched methods: per-commit load-bearing tables for multi-commit PRs; workflow/CI PRs get embedded-script replay against real data, repo lint gates, and day-one trigger cost math from real event history; every new config knob must trace to an observable effect, and default-path dispatch combinations get probed - findings quality: blockers enumerate blast radius, demonstrate the sharpest consequence end-to-end when budget allows, and carry a collapsed minimal suggested fix preserving the original commit's intent * feat(triage): host /verify evidence images and encode quantified-A/B rules Borrow the image-evidence and quantified-verification patterns from hand-run rounds (#7265, #7471, #7686 r2 and the pr-assets convention): - publish-verify now hosts agent-produced evidence/*.png on the pr-assets branch (verify/pr<N>-<run>-<attempt>/) and appends them below the escaped report. Untrusted-payload discipline: strict filename allowlist, 8-image / 2 MB caps enforced in the find predicates, racing-push retry, and every failure degrades to a text-only comment. VERIFY_ASSETS_REMOTE is a test seam; the block was dry-run against a local bare remote covering hosting, hostile filenames, oversize files, dotfiles, missing branch, and no-image runs - skill: evidence images are named as kebab-case captions binding image to claim, before/after pairs over lone after-shots; follow-up rounds lead with a previous-finding status table (fixed/stands/superseded/declined, with adjudication) and re-measure instead of diffing the old report; size/perf claims get measured-metric Δ tables with residual deltas accounted for; unreachable branches get the configuration that reaches them constructed; defensive guards get their accept path checked against real production artifacts, not just mocked rejects * fix(triage): address /review suggestions on the verify lane - skill: local invocation resolves --repo and passes it to every gh call - skill: call out the dependency confound when the base A/B side reuses the PR-installed node_modules and the PR touches package.json/lockfile - workflow: document the pin step's bootstrap logic — issue_comment jobs run the default branch's YAML, so base always carries the verify-pr skill by the time this job exists * fix(triage): harden /verify gate, comment budget, and evidence hosting per review Address review round 5078770575 items 1-3 plus the cheap follow-ups: - authorize: /verify now requires write from BOTH the PR author (whose code runs) and the commenter (who spends a scarce runner slot + model budget) — a drive-by account can no longer burn 45 minutes of ecs-qwen on someone else's PR; duplicates check once; /tmux and /triage gates unchanged. Replayed 8 principal scenarios against a stubbed gh - authorize acks /verify with the eyes reaction from the always-hosted job, so a queued/saturated sandbox pool no longer means total silence - publish: emit_block escapes FIRST and caps the escaped size (45 KB for the report) — a raw-side cap let dense <>& content inflate past GitHub's 65,536-char comment limit, 422 the post, and strand the running status with no report at all; iconv -c keeps a UTF-8 sequence split by the byte cut (likely, given the mandated 中文 summary) from shipping broken; replayed: 50 KB dense report -> 45,873-byte body - publish: image cap is byte-exact (-size -2097153c; find's -2M rounds sizes UP to MiB, silently making the documented 2 MB cap 1 MiB), bytes must carry the PNG magic (extension is attacker-choosable), duplicate sanitized names dedupe instead of overwriting + double-rendering, and dropped images are reported in the comment instead of vanishing - publish: weak terminal notices (cancelled/infra/skipped/n-a) only replace this run's own running status; a previous round's real report survives as the marker comment and the notice posts fresh - publish: report.md/assertions.json lookups pin the artifact-dir shape and sort (bare find -name order is filesystem-dependent); the verify job's verdict.txt lookup sorts likewise - verify: global npm install runs from RUNNER_TEMP (the persistent workspace still holds the PREVIOUS run's tree, whose .npmrc would apply to a root install); both cleanup passes remove leftover tmp/ worktrees (git worktree prune alone only drops metadata); the run step no longer re-chowns 50k node_modules files; pr-assets clone sets its committer identity once so the racing-push rebase retry can commit - skill: worktree guidance now tells the agent to remove its base tree itself, with the workflow sweep as backstop only * fix(triage): close runtime-plant and stale-RUNNER_TEMP channels in /verify Address review round 2 (comment 5079157987) and the CHANGES_REQUESTED round on the verify lane: - run step re-sweeps tmp/*-verify-* AFTER npm ci/build and before the agent starts: the pin step's sweep runs before PR lifecycle scripts (postinstall etc.), which could re-plant a fake artifact dir whose zeroed timestamp deterministically wins the sorted collector. From the sweep on, only the agent writes those dirs; a steered agent forging its own artifacts remains the documented advisory-report residual - RUNNER_TEMP verify-results/verify-context are rm'd before mkdir: the pool is persistent and runner temp hygiene is runner-managed — a stale report or previous-report.md from ANOTHER PR must never ride along - symlinks are stripped from verify-results before upload: actions/upload-artifact dereferences them, so a node-planted link would exfiltrate whatever it points at into the artifact - a trusted commenter invoking /verify on a PR whose author lacks write now gets an explanation comment from the hosted authorize job instead of total silence (the commenter is checked first; drive-by accounts and API errors still get nothing); job timeout 45->60 so a slow install can never let the JOB limit kill the agent past its own graceful 25m budget - stale tmp/base-tree (skill's canonical scratch worktree) is removed by name at job start — a plain dir isn't git-registered, so the worktree sweep alone misses it and the next worktree add would fail - scripts/tests/qwen-triage-workflow.test.js gains a verify-lane describe block: an 8-arm stub-gh replay of the dual principal gate (drive-by deny, author-without-write deny + explain flag, self-comment dedupe, 404 fail-closed, /tmux and /triage unchanged) plus guards for the post-prepare sweep placement, the symlink strip, and the RUNNER_TEMP resets — the replay found this commit's sweep edit had silently not applied, which is exactly the regression class it exists to catch * fix(triage): close proxy-hijack, gate-bypass, and false-verdict paths in /verify Address the Codex /review round (19 findings) and the bot's follow-up. Each fix was replayed locally; the proxy fix has a decisive A/B. Gate and routing: - the shell command match is case-insensitive: GitHub Actions expression comparisons ignore case, so `@QWEN-CODE /VERIFY` reached the step and fell through to the commenter-only branch — running the PR author's code with the author never checked - the verify ack and denial notice require github.event.issue.pull_request: /verify on a plain issue was acknowledged but could never report - publish-verify joins the verify job's per-PR concurrency group, and a failed PATCH falls back to posting fresh instead of going silent Untrusted-input paths: - the model proxy binds an EPHEMERAL port, reports it through a root-owned file, and its health check must echo a per-run nonce with the recorded PID alive. A/B with a squatter on 8787: the old code's proxy dies EADDRINUSE yet still reports enabled and points qwen at the squatter; the new code comes up unaffected on an ephemeral port - worktree-scoped git config is deleted before hooksPath is resolved: `extensions.worktreeConfig` is allowlisted and .git/config.worktree is invisible to `git config --local`, so a prior run could set core.hooksPath=/ and make the hook sweep's recursive delete walk / as root (verified locally). The sweep now also refuses any hooks path outside the repository's git dir - marker-comment lookups accept only bot-owned comments that START with the marker: any user can paste the marker and divert the bot into PATCHing a stranger's comment - the upload staging dir is re-flushed after npm lifecycle scripts Honest verdicts: - the docs-only classifier no longer uses a pipeline (grep -q made the writer take SIGPIPE, so under pipefail a long file list with an early code file classified a code PR as docs-only and skipped verification), and executable markdown/YAML (.qwen, .github/workflows, scripts) is classified as behavioral before the extension rule - tee's status is checked alongside qwen's: a full results volume made a truncated evidence stream publish as pass - 137 is split by elapsed budget into watchdog timeout vs crash/OOM - the agent's verdict is honored only for VERDICT=pass with a report and zero failed assertions; otherwise the process outcome headlines and the scope paragraph says the run did not complete - verdict.txt is read through a bounded prefix (SIGPIPE under pipefail) Skill contract corrections: per-commit tables only when the commits are reachable at depth 2 (else aggregate + Not covered); internal workspace symlinks must have their realpaths asserted before a base control is trusted; repo lint gates and event-history cost math are qualified to what the token-free container can actually run; --repo is never inferred from `origin` (a fork holds a different PR under the same number). Tests: 9 new guards, all mutation-verified (reverting each fix turns one red), including an executable escaping/size-cap/UTF-8 test for the publisher's own emit_block and a fix to the earlier command-file test, which matched the tmux job's identically named step. * fix(triage): re-establish the /verify trust boundary after PR code runs Third review round (31 findings). The unifying defect: everything the lane pinned or swept happened BEFORE npm ci/build executed PR-authored lifecycle scripts as node, so each control could be undone afterwards. Trust boundary, rebuilt in order before the agent starts: - kill every process owned by the build user and fail the step if any survives — a detached postinstall child could otherwise wait out each one-shot sweep and re-plant afterwards - re-pin .qwen from the base commit again, now root-owned and read-only: the prepare step chowns the workspace to node, so a lifecycle script could rewrite the very skill that defines /verify-pr - give the agent a fresh HOME/QWEN_HOME: qwen loads user-scope file commands from $HOME/.qwen, and /home/node belongs to the build user, so a planted commands/verify-pr.toml could shadow the pinned skill - the model proxy now requires a per-run bearer token, closing the blind-localhost-scan path to an unauthenticated signer for the real model credential (a command the agent itself launches still inherits it — documented residual, not closed) Authorization and lifecycle: - re-verify the PR author's write permission at execution time and pin the authorized head OID; refuse if the checked-out HEAD^2 differs, so a push during the runner wait cannot smuggle in unreviewed code - validate each principal separately: an empty author vanished in word splitting and left only the commenter checked - honor MAINTAINER_ECS_RUNNER_DISABLED with an explicit notice instead of queueing forever against a disabled pool - status comments carry a machine state marker; inferring 'running' from prose let a report quoting that sentence be overwritten - previous-report.md snapshots the newest substantive report, never a weak/cancelled notice, so prior findings survive into the next round - bot-identity lookup failures fail closed instead of widening the ownership filter to every user's comments - publish-verify uses a per-run concurrency group: a per-PR group holds only one pending job, so a second /verify could cancel a completed run's pending publisher Correctness: - install/build failures are classified: signals, ENOSPC, registry and network errors are infra-error, not a PR verdict - watchdog classification measures the child's own elapsed time, not shell-global $SECONDS which includes proxy setup - assertions.json must be three non-negative integers with a positive total and total == pass + fail before it counts as evidence - the proxy keeps its upstream deadline armed until the body ends and aborts upstream when the client disconnects - cleanups remove .qwen/tmp itself: PR code can make it a symlink, and globbing below it deleted the target's contents as root (verified) - emit_block materializes the escaped text and truncates on a character boundary via node — iconv -c passes an incomplete trailing sequence through on BSD (measured), which the new test caught Skill: local mode requires the same isolation CI provides and must not assume HEAD^1/HEAD^2 on a plain head checkout; shallow boundaries make rev-list counts unreliable for per-commit claims; never run scripts/lint.js with no arguments (it runs prettier --write and rewrites the tree under the harnesses); a vacuity check must fail the intended assertion, not the import. pr-workflow.md now says both sandboxed lanes need the author to have write, so triage stops recommending a guaranteed denial on external PRs. Tests: 9 more guards, all mutation-verified, including executable replays of the docs-only classifier (SIGPIPE + executable-markdown cases), the uppercase-command gate, the empty-principal deny, and the untrusted-image hosting path against a bare pr-assets remote. * test(triage): pass the classifier fixture through a file, not argv The new docs-only classifier replay passed on macOS and failed on CI with `Cannot read properties of undefined (reading 'trim')`: its 60,001-entry fixture is ~889 KB and was passed as a single argv element. Linux caps one argument at MAX_ARG_STRLEN (128 KB), so the spawn failed with E2BIG and stdout was undefined; macOS has no per-argument limit and only a ~1 MB total, so the same call succeeded locally (verified both). Write the list to a temp file and pass the path. The harness now also asserts the spawn succeeded, so a future spawn failure reports itself instead of surfacing as a TypeError on undefined output. * fix(triage): make the /verify report match what the run actually produced Three publisher findings, all introduced by my own previous round: - an artifact download failure (the step is continue-on-error) let the full-report path run with no results: the headline read 'completed' and the scope paragraph claimed the A/B, the harnesses and the gates had run when nothing had been delivered. The download outcome is now an input, and its failure gets its own body saying the results could not be retrieved - the prepare-failure branch ignored the verdict the prepare step had just computed, so an install killed by a registry outage or OOM (classified infra-error) still told the author 'this is treated as a PR failure verdict rather than an infrastructure failure' — the exact opposite. It now branches on the verdict, and an infra-classified prepare failure is a weak body that cannot overwrite a real report - weak notices were being snapshotted as the follow-up round's previous-report.md: they lack the running marker, so 'newest non-running comment' selected them. Bodies that carry findings now mark themselves (qwen-triage:verify-substantive) and the snapshot selects on that marker. A/B on the real jq: report A then cancelled B now snapshots A (101), the old filter picked B (102) Tests: 4 more guards, all mutation-verified — the publisher is rendered for each outcome with a stubbed gh and the assertions read the body it would post, and the snapshot test runs the workflow's own jq program verbatim against a paginate-shaped fixture. * fix(triage): stop PR build output from masquerading as an infra failure Two review findings plus a test-helper hazard: - classify_failure grepped the prepare log for bare words like ENOSPC and ETIMEDOUT, but that log is written by PR-controlled code: a genuine build failure that merely prints 'expected ETIMEDOUT to equal ok' would be published as an infrastructure incident, telling the author to re-run something that fails identically. The patterns are now anchored to lines only npm's reporter or the kernel emits ('npm ERR! code E…', 'npm ERR! network …', kernel OOM, bare 'Killed'); a signal exit still needs no log evidence. Replayed 10 cells: four PR-authored logs quoting infra words stay 'fail', five real diagnostics and one signal exit are 'infra-error' - the two execution-time controls added last round — re-verifying the author's permission after the runner wait, and refusing a head that moved since authorization — had no tests. Both are now executed: the re-auth snippet against a stubbed permission API (write proceeds and pins head_oid; read skips with a publishable reason), and the pin step against a real git repo with a real merge commit (matching head proceeds, moved head exits non-zero) - add a stepIn(job, step) test helper. Several step names exist in both the tmux and verify jobs, and the unscoped step() returns the first match, so a verify-lane assertion silently tests the tmux copy — that has now bitten this suite three times, including in this commit. * docs(triage): teach verify-pr test-only PRs, differential oracles, gate liveness Fold techniques from the round-2 verification on #7620 (an ANSI parser PR) that the skill had no equivalent for: - test-only PRs get their own method: a mutation A/B across TEST FILES (same mutants of the unmodified production file, only the test file swapped), reporting killed/total on both sides, requiring that no mutant regressed from killed to survived, checking that the killing assertion is the one the commit claims to have strengthened, and adjudicating every survivor as coverage gap or defect with independent evidence rather than by inspection - when the code emulates a known implementation, that implementation is the oracle: feed identical input to both and report disagreement counts per side, lift reference tables verbatim out of the shipped dependency, and build the corpus from bytes captured off a real producer alongside synthesized sweeps - prove a gate is live before citing it: plant a violation the linter must catch, confirm it is reported, remove it — a linter that matched no files exits 0 exactly like one that passed - attribute pre-existing failures by byte-identical failing file AND test names on both sides, with deltas, not just totals - when the base is far behind, verify the merge: trial-merge into current main, confirm it is conflict-free, and re-run the affected suite on the merged tree - round continuity gains its one legitimate shortcut: a production file proven byte-identical (sha256 quoted at both heads) carries prior evidence forward by construction * style(triage): reflow verify-pr skill to prettier's markdown wrapping The previous commit's added paragraphs were hand-wrapped and prettier --check flagged the file; the repo runs prettier over all of it. * test(triage): cover the disabled-runner-pool notice The kill-switch path had no test: a refactor could drop the notice and leave a /verify request acknowledged with 👀 but permanently unanswered, since the verify job refuses to start and publish-verify skips with it. Fold the step into the existing PR-guard loop (now scoped through stepIn, so it cannot match a same-named step in another job) and assert the parts that make the answer useful — the kill-switch and permission conditions, both languages, the alternative it points at, and the verify job's own exclusion of the disabled pool. All three mutations turn it red: removing the step, dropping its PR guard, or letting the verify job queue against the disabled pool. * fix(triage): repair a step-killing PIPESTATUS read and six forgeable controls Sixth review round, 12 findings. Several are regressions from my own two previous rounds; the first would have broken every single run. - `AGENT_STATUS=${PIPESTATUS[0]}` is itself a command and resets PIPESTATUS, so the next line's ${PIPESTATUS[1]} was unset and `set -u` aborted the step immediately after the agent finished — before artifact collection, the verdict, or anything else. Verified by replaying the exact structure: 'PIPESTATUS[1]: unbound variable'. Both elements are now snapshotted in one command - concurrency predicates were broader than the job conditions they guard, and GitHub evaluates concurrency BEFORE the job `if`: a /verify comment entered the triage job's shared per-PR group (where it could displace a pending /triage and then skip), and a /verify queued while the runner kill switch was on did the same to a real verification. Both predicates now match their job's runnable set exactly - an outward-resolving .git/hooks entry was only warned about and left in place, so the next root-owned git command would run it. It is now unlinked without traversing its target, a root-owned hooks directory is restored, and core.hooksPath is unset - the second .qwen pin re-derived HEAD^1 from git metadata after the workspace, including .git, had been handed to the build user. The base OID is now recorded while .git is still root-owned and the re-pin archives that content-addressed OID - classify_failure took both of its inputs from PR-controlled sources: a lifecycle script can exit with a signal status and can print any line the log patterns matched, turning its own deterministic breakage into 'infrastructure, please re-run' — which hid the failure and preserved a stale report. No infra verdict is derivable there, so the prepare step reports `fail` and lets the embedded log speak for itself - cleanups descended through PR-writable parents: `.qwen` itself can be a symlink, and the worktree sweep trusted git metadata with only a lexical prefix check. Symlinks are unlinked without traversal and worktree paths must canonicalize inside the workspace. Replayed all three escapes - skipped and docs-only outcomes upload no artifact, so the new download-failure branch pre-empted them and made their real reason unreachable; they are answered first now - a run that crashed before writing report.md still claimed the substantive marker, letting a headline overwrite the previous round's evidence. The marker now requires a report Skill: the byte-identical shortcut needs the whole input closure, not one file hash; the credential-free local path cannot call `gh` at all (fetch the metadata outside and mount it read-only); and the A/B base is `baseRefOid` in local mode, not `HEAD^1`. Tests: 7 new guards plus 4 updated to the new shapes, all mutation-verified (50/50). * fix(triage): answer dropped /verify requests and prove the proxy rejects Maintainer review (yiliang114), 7 items: - a third /verify while two runs are in flight is dropped by the concurrency group with no job and therefore no comment. The hosted authorize job now counts this workflow's other in-flight runs and says so; an API hiccup leaves the request alone rather than denying it - the proxy's bearer check had no executable test. It now starts the real proxy against a real upstream and issues real requests: no header and a wrong token are 401, this run's token is 200, and a route other than /chat/completions is 403 — with the health endpoint echoing the nonce - the 502 path forwarded the raw upstream error, which can name resolved hosts and TLS detail to PR code. It logs server-side and returns a generic failure - publish-verify inherited the 360-minute default; it downloads one artifact and posts one comment, so it is bounded at 10 - removing the log classifier last round left the comment block it replaced, which still said failures are classified from the exit status and the log. Deleted - that removal also left every install failure reported as the PR's fault, including a registry outage. There is exactly one signal here PR code cannot write — asking the registry ourselves, as root, with the container's resolver — so an install failure is downgraded to infra-error only when that probe fails. It proves reachability now rather than at failure time, so it can only ever downgrade, never confirm; a build failure has no equivalent and stays the tree's problem - the skill's local-invocation warning ran into the preceding sentence, which GFM renders as one paragraph Tests: 5 new guards, all mutation-verified (55/55). * fix(triage): resolve hooks hermetically and mirror symlink guards at job end Maintainer review round (doudouOUC), 6 findings. Two were Critical and both reproduced: - the hooks sweep resolved its path with the ambient git config in play. With a global core.hooksPath set — which the reviewer has and I do not, which is why my earlier replay showed a false pass — `git rev-parse --git-path hooks` returns that global path, the in-git-dir guard reads 'outside', and a planted `.git/hooks` symlink survives untouched. A/B: old code leaves the symlink under a global hooksPath, new code removes it in both environments and never touches the link target. Resolution now runs with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM pointed at /dev/null - the END-of-job cleanup still used the bare `rm -rf .qwen/tmp` that the start-of-job cleaner was hardened against two rounds ago. The agent executes PR code between the two, so the end is no safer than the start: it now unlinks symlinks without descending and canonicalizes worktree paths inside the workspace before deleting Plus four suggestions, all valid: - the saturation notice counted this workflow's in-flight runs across every PR while the concurrency group is per-PR, so a run on another PR would trigger a warning about a queue that does not exist. It now matches on the PR title (the only per-PR handle an issue_comment run record carries) and stays silent when that cannot be resolved - the skill recommended `require.resolve` for the workspace-realpath check; these packages are ESM-only with import-only exports, so it throws ERR_PACKAGE_PATH_NOT_EXPORTED and reads like a missing module. Verified, and replaced with `readlink -f node_modules/@qwen-code/...` - the symlink-escape test inherited the developer's git config, which is what hid the first finding. It now runs with global/system config neutralized AND repeats the case with a global core.hooksPath planted - the publisher's build-phase arm was never rendered by any test (every case used 'install'), so a typo in that command name would have shipped. Now covered, along with an unrecognized phase Mutation-verified 4/4. The hooks guard needed a discriminating assertion: git's own `*.sample` files must survive the sweep, because the outward-path fallback removes the whole directory and would otherwise satisfy a bare 'planted hook is gone' check. * fix(triage): count only /verify runs for saturation, and test the PATCH arm Bot review round, 2 suggestions, both valid: - the saturation notice matched runs by PR title, which narrowed to this PR but not to /verify. /triage and /tmux live in their own concurrency groups, so two of those in flight would warn about a verify queue that is actually empty. It now also requires the run to have a job named 'verify' — the run record carries no command, but its job list does. Replayed: two non-verify runs stay silent, two verify runs warn - every publish fixture returned an empty comments listing, so the PATCH arm was never executed: a broken PATCH would have stranded the running status comment and posted a duplicate below it, with the suite green. The publisher now runs against a stubbed listing and the test asserts which verb went to which comment id — bot-owned live status is PATCHed in place, an absent comment posts fresh, and a marker comment owned by someone else is left alone and posted around Mutation-verified 3/3: counting every command, never PATCHing, and accepting foreign-owned markers each turn one test red. Two stub bugs found while writing these, both mine and both silent: ${*#pattern} applies per positional parameter rather than to the joined string (yielding a wrong run id), and the paginate fixture needs one array per page, not an array of pages. * fix(triage): fix the real silent drop and drop the step built on a wrong premise Review round 4. The blocker was mine twice over: the saturation notice I added last round had GitHub's concurrency semantics backwards, and the silent drop it claimed to cover was somewhere else entirely. - GitHub cancels the OLDER pending run in a group and admits the new one (confirmed against the workflow-syntax reference). My step told the person who had just typed /verify that their request might be dropped, when theirs is the one that runs — and said nothing to the person whose queued run actually died. This PR already had it right in publish-verify's own comment, so the file contradicted itself and the user-facing copy followed the wrong half. The step is removed rather than reworded: with the fix below there is nothing left for it to warn about, and it cost 2+N API calls on every /verify. - the actual drop: a verify job cancelled while still PENDING never reaches a runner, so its outputs block — where the "|| github.event.issue.number" fallback lived — is never evaluated. publish-verify then read an empty PR_NUMBER, hit its own guard and exited 0, making the cancelled branch unreachable in exactly the scenario that produces cancellations. The fallback now lives where the value is read. Reproduced both arms by executing the real step: with a number the cancelled notice posts, with an empty one it only warns. - same one-line class in publish-tmux, fixed alongside. Two copy defects from the classifier removal, both mis-attribution pointed the other way: - the infra-error body still named a signal/OOM kill and a full disk, none of which the current prepare step can produce — infra-error now requires npm ci to fail AND the registry probe to fail. It names that condition only, and offers a re-run instead of asserting it is the fix. - the code comment above it still described the deleted classifier. Also fixes the indentation break an earlier scripted edit left in the publish body builder, and replaces the saturation test with one that executes the cancelled path. Mutation-verified 2/2; the copy needed its own guard, since reverting the wording alone left every test green. * docs(triage): teach verify-pr survivor accounting and observability regressions Fold techniques from the re-verification on #7709 that the skill had no equivalent for: - the mutation matrix must report the mutations that changed NOTHING, not only the ones that failed. Each survivor gets classified as an ordinary coverage gap or as dead code — a guard whose deletion leaves every test green is one of those two, and the difference is what the author needs. Survivors mirroring a pre-existing gap are labelled as such, and the set is framed as completeness reporting rather than merge conditions - the sharper case that report demonstrates: a test that passes for the WRONG REASON. If deleting the new guard leaves its own new test green, that test is pinned by an earlier early-return, not by the change, and asserts nothing about it. Name what actually pins it - and do not generalize from one dead guard to its siblings: the same report shows a clause that is unreachable on one path while being the only protection on another. Check each, report the contrast - observability regressions: when a change suppresses output, follow the value before calling the suppression correct. A bare catch on the path plus a field with no readers anywhere in the repo means the cause is now unobservable even in devtools — a real loss that no behavioural assertion can see - report structure gains a Corrections section: when an earlier round or bot comment described the code inaccurately, state the correct fact with evidence and label it as a correction to the description, not a request to change code. A wrong description left standing costs the next reader more than the original finding did --------- Co-authored-by: wenshao <wenshao@example.com>	2026-07-26 13:44:28 +00:00
Shaojin Wen	172a567a30	ci(autofix): number the status comment like every other round message (#7748) ... `effective_round` counts rounds already finished, and "Push and report" posts ROUND + 1 as the round it just performed. The status comment used the raw value, so the same round showed two numbers in one thread — observed on #7724 at 10:10 UTC: the report said "round 6/100" while the status said "AutoFix round 5 finished". Display ROUND + 1 in both status messages, guarded so a missing or non-numeric round degrades to the raw value instead of failing the step. Co-authored-by: wenshao <wenshao@example.com>	2026-07-26 12:38:19 +00:00
Shaojin Wen	cfd7c104bf	ci(autofix): show a live-progress status comment while a round runs (#7738) ... * ci(autofix): show a live-progress status comment while a round runs Takeover engages, and then the PR thread goes quiet: review-address runs the agent for up to 80 minutes plus a verification gate, but nothing reaches the thread until "Push and report" at the very end. Observed on #7731 — 43 minutes of silence with no way to tell a working round from a stuck one. The agent's output already streams live to the Actions log; only the link was missing. Announce the round up front with that link, and flip the same comment to a terminal state when the round ends. Upserted by marker so one comment per PR is edited each round (edits notify nobody) instead of stacking against a 100-round cap. Both steps are gated on the stale-duplicate flag: the per-PR concurrency group runs a discarded duplicate AFTER the real round finalised, so an ungated finalize would overwrite that round's "finished" with its own "ended without publishing". Best-effort throughout — a failed status post warns and never costs a round. * ci(autofix): hand the status comment id to the finalize step Addresses review feedback: the announcement and the finalize each ran their own paginated comment scan, twice per round on a PR that can accumulate hundreds of comments over 100 rounds. The announcement already knows the id — it either found one or just created one — so it now writes it to $GITHUB_OUTPUT (capturing the id of a freshly posted comment via --jq) and the finalize consumes that. The finalize's scan is removed outright rather than kept as a fallback: an empty id means this round never announced, so no comment claims the round is working, there is nothing to flip, and a previous round's comment is already terminal (the next round's announcement re-PATCHes it either way). One scan per round instead of two, and less code. * test(autofix): assert finalize step error guards and dry_run gate (#7738) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix[bot]@users.noreply.github.com>	2026-07-26 08:13:34 +00:00
Shaojin Wen	0f56e35c0a	ci: keep the critical-audit gate honest when npm cannot answer (#7743) ... npm retired the `security/audits/quick` endpoint, which now 400s for this package tree. `npm audit` exits 1 for that exactly as it does for a real critical finding, so every PR in the repo went red on 2026-07-26 with "Invalid package tree" — a failure no branch can fix. The exit code alone cannot gate a merge: treating every non-zero as vulnerable blocks the repo on an npm outage, and ignoring it retires the gate. The payload separates them — a real audit always carries metadata.vulnerabilities, a transport failure carries the request error. Classify on that: a finding still fails, an unreachable endpoint warns and passes, and anything unrecognised fails closed so a payload-shape change gets a human rather than a silently disabled gate. Co-authored-by: wenshao <wenshao@example.com>	2026-07-26 06:59:13 +00:00
Shaojin Wen	3d395606ae	fix(triage): only the bot's own approval counts as already approved (#7737) ... A maintainer approved PR #7620 three minutes before re-triggering `@qwen-code /triage`. The run reviewed the PR, scored it 5/5, and then reported "✅ Approved (5/5) — existing approval from prior run still valid, head SHA unchanged" without ever calling the approve API. The approval it read was the maintainer's, on the same head commit; the bot's own latest review there was a `/review` downgrade to COMMENTED, and its earlier approval had been dismissed by a push. The PR sat at 1 of the 2 required approvals with nothing in the run log marked wrong. The skill documents an "already exists, skip re-submitting" rule for CHANGES_REQUESTED only, and that snippet filters on the bot's login. Nothing covered approvals, so the rule was generalized to them with the author filter dropped. Since the maintainer's habit is to approve and then ask triage for the second vote, this reproduces on every re-run. Spell the approval check out instead of leaving it to inference: the skip applies only to the bot's own APPROVED review pinned to the exact reviewed commit — another account's approval is a different vote, a DISMISSED review is not an approval, and an approval on an earlier commit was already voided by the push. Keep the skip itself, so three re-runs still don't stack three approvals. Back it with a workflow check, since the failure is silent by nature. "Notify silent triage re-run" already detected that no review was added; it just said so in wording that read as a normal ending. It now reports whether the bot has a review of its own on the head commit, and warns when it does not. Also paginate the CHANGES_REQUESTED probe. An unpaginated read sees only the first page, and re-runs happen on exactly the heavily-reviewed PRs where the gating review has scrolled past it. Co-authored-by: verify <verify@local>	2026-07-26 03:43:34 +00:00
jinye	8fa8085036	perf(core): Lazy-load first-use dependencies (#7686) ... * perf(core): Lazy-load first-use dependencies Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): Fix simple-git loader mock Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): Cover abort during xterm load Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Address lazy-loader review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Validate lazy dependency module shapes Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-26 03:04:33 +00:00
Shaojin Wen	638dc9a1c3	fix(triage): resolve finalize PRs from the open-PR list, not the commit association (#7706) ... Live verification of the finalize loop (#7693) on its first real fork PR caught the deferred approval being silently dropped: CI landed green, the approve-on-green marker was in place, but commits/:sha/pulls returned an empty list for the PR's current head — the association endpoint is not reliable for fork-branch commits (workflow_run.pull_requests is likewise empty for forks). The run logged 'No open PR; nothing to finalize' and exited, so the approval never posted. Resolve PRs by filtering the open-PR list on head.sha as the primary source — it cannot miss the PR a current-head firing belongs to — and keep the association endpoint as the second source (when it works it also surfaces PRs whose head moved past the SHA, which powers the stale note). Union both, deduped.	2026-07-25 07:20:45 +00:00
Shaojin Wen	52f6eaf8f0	fix(triage): resolve stage comment ids by marker at patch time, harden model injection (#7703) ... * fix(triage): resolve stage comment ids by marker at patch time, harden model injection Two hardenings from shepherding #7693, plus a wording fix: - Re-run comment updates now resolve the target comment id by its stage marker (bot-author-filtered, startswith match) immediately before each PATCH, instead of trusting remembered ids or list positions. Observed on a real re-run: the agent PATCHed the stage=3 comment with stage=1 content mid-run before self-correcting — with four bot comments in the thread, remembered-id bookkeeping is fragile. - The model-name injection step previously no-opped silently if the 'qwen3.7-max' literal ever left the skill (shipping the wrong signature in every comment), and corrupted the skill text on model names carrying sed metacharacters (/ & \). It now fails the job loudly when the target literal is missing and escapes the replacement. Covered by a behavioral test that runs the extracted step script against fixture files. - The finalize status text said 'stage comments above', but the status comment is created first, so the stage comments are below it — now 'in this thread'. * chore(triage): drop unrelated formatting churn from qwen-triage.yml The previous commit let prettier rewrite untouched lines (runs-on quoting, comment spacing) while formatting the edited step. Restore those lines to main's form; the diff now carries only the injection hardening and the status wording fix. * test(triage): pin the stage_comment_id recipe's load-bearing constraints Guards the startswith match and the bot-author filter in the skill's re-run comment-id recipe against silent regression — a contains match or a dropped author filter re-introduces the wrong-comment-overwrite bug this PR fixes. * test(triage): shim BSD sed only on darwin in the injection test The extracted step script uses GNU 'sed -i' (the step only runs on ubuntu runners), but this suite also runs in the macOS merge-queue job where BSD sed needs an extension argument after -i. Rewrite to sed -i '' on darwin only — on GNU sed a separated '' parses as the sed script, so the unconditional rewrite would break the Linux runs that mirror production.	2026-07-25 07:20:01 +00:00
Shaojin Wen	1f9318f974	feat(triage): stop in-agent CI polling, finalize evidence and approval after CI completes (#7693) ... * feat(triage): stop in-agent CI polling, finalize evidence and approval after CI completes The triage agent's Stage 2b polled pending checks for up to 10 minutes, but this repo's unit suite runs ~30 minutes, so the poll always burned its full budget, gave up with 'CI still running', and Stage 3 could then approve before the suite finished (observed on a PR approved 12 minutes before its Test job completed). Split the wait out of the agent entirely: - pr-workflow.md Stage 2b now forbids polling: fetch check-runs once, report pending checks honestly, and wrap the CI table in qwen-triage-ci region markers keyed to the reviewed SHA. - Stage 3 defers a clean-verdict approval when checks are still pending: the comment carries an approve-on-green marker instead of an immediate APPROVE. - New qwen-triage-finalize.yml fires on workflow_run completion of 'Qwen Code CI' / 'E2E Tests' and, with plain bash over the API (no model, no checkout), rewrites the marked table region with the settled results and posts the commit-pinned approval only when every check landed green — failing closed on red checks, a moved head, or a closed/draft PR, and flipping the triage status comment to say which way it resolved. Markers are honored only in comments authored by the bot identity itself, and check names (attacker-influenced on fork PRs) go through the same HTML-escape chain the skill mandates for file paths. Stage comments now land ~10 minutes sooner and the approval, when deferred, lands at CI completion with full evidence instead of before it. * fix(triage): address finalize review — broken red gate, table truncation, dead trigger Review findings on the finalize workflow, all reproduced before fixing: - Blocker 1: the RED jq used the array-first membership form, where | rebinds . and .conclusion indexes an array — jq exits 5 every run, RED comes back empty, [ "" -gt 0 ] errors, and control falls through to the approve path: a red CI auto-approved. The gate now binds the conclusion before the membership test (IN(...)), and the counters are numeric-validated so any future jq failure reads as 'cannot attest', never 'approve'. - Blocker 2: the table rendered raw check-runs — on a real PR (96 runs, 35 names, 68 skipped) alphabetical sort + head -60 truncated away every actual test job. table_rows now dedups per name (latest run), drops skipped rows, and sorts running/non-green first so the cap can only cut green rows. Replayed against the same PR: 96 rows -> 16, unit suite present. - The approval gate now reads workflow runs filtered to event=pull_request (deduped per workflow) instead of head-SHA check-runs, which also carry long-running bot orchestration jobs that would wedge PENDING above zero at the exact moment the last CI workflow fires — silently dropping the deferred approval forever. The skill's Stage 3 PENDING count matches. - E2E Tests had no pull_request trigger (dead entry); the workflows list is now exactly the six pull_request-triggered workflows, so the last finisher always re-fires the job. - Head/state re-check moved before the red/deferred verdicts so a cancel-in-progress firing on a stale SHA cannot stamp a red status over the new head's comment; the still-deferred branch now updates the status comment instead of staying invisible. - replace_region fails closed when the end-marker text only precedes the begin marker (awk END guard) — previously that shape truncated the comment body, eating the signature and reviewed-commit footer. - Region content is deterministic (no run URL) so the no-op cmp works; empty run list or unavailable gate skips approval; comment wording fixed (workflow_run jobs are attributed to the default branch, so the self-check exclusion is belt-and-braces, not load-bearing). Tests now execute the decision logic, not just grep for it: gate_counts and table_rows run against fixtures covering every conclusion class, non-PR events, re-run dedup, skipped filtering, ordering, and both marker-order failure shapes. 30/30 passing. * fix(triage): keep a stale finalize firing from clobbering the newer review's status comment The status comment is deliberately not SHA-scoped (the triage workflow creates it unscoped; scoping only the finalize side would orphan the pairing), so a finalize firing for an old SHA that loses the race against a newer head's green approval would overwrite the ✅ status with a stale warning. Guard the stale path: when the current head already carries bot sha= markers (a re-review owns the status comment), stay silent; when the head moved with no re-review yet — triage does not auto-rerun on synchronize — the stale note is accurate and still posts. Closed/draft PRs now just log instead of flipping the status. * fix(triage): close the guardrail bypass and align the finalize table with the gate Second review round, all four findings reproduced or confirmed before fixing: - The approve-on-green marker was emitted in Step 1 while the fork-refactor GUARD only ran in Step 2 — a marker that slipped out on a fork refactor would have been honored by the finalize job on green CI, bypassing the guardrail entirely. GUARD now computes in Step 1 and gates the marker's emission, and the finalize job re-asserts it structurally from the PR state it already fetched (null head.repo = deleted fork = blocked), with a 'guarded' status message instead of an approval. - table_rows now restricts check-runs to the suites of the same deduped event=pull_request workflow runs the gate trusts. Without it, 5 of 8 rendered rows on this PR's own head were bot plumbing presented as CI evidence; with it, 115 raw check-runs reduce to exactly the 3 CI rows. - A firing that saw PENDING>0 after the approval landed flipped the status comment back to 'deferred' with nothing to ever right it; the already-approved branch now repairs the status. - Zero surviving table rows (failed runs fetch, missing suite ids) skips the region rewrite instead of blanking the agent's table, and replace_region refuses an empty region file (an unchecked getline would have deleted the region and its markers unrecoverably). Nits: the house github.repository guard on the job, the table header matches the skill template, and the run-URL stays out of the region so the no-op cmp keeps working.	2026-07-25 00:58:16 +00:00
易良	65b4a5a383	fix(ci): update qwen in the runner's active npm prefix (#7689) ... * fix(ci): update qwen in runner npm prefix * test(ci): cover writable runner prefix install	2026-07-24 19:11:41 +00:00
qqqys	b1ce0c2087	refactor(autofix): extract review verification runner (#7644) ... * refactor(autofix): extract review verification runner * test(ci): follow extracted autofix verifier * docs(autofix): document the review verification runner env contract (#7644)	2026-07-24 16:46:24 +00:00
Shaojin Wen	66da87d7ad	ci(triage): surface live progress via an early status comment (#7654) ... `@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>	2026-07-24 06:27:53 +00:00