mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-26 00:53:48 +00:00
580 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
464e8910e8
|
fix(desktop): harden release pipeline (#9009)
* fix(desktop): harden release pipeline * fix(desktop): resolve release hardening review |
||
|
|
a32ec1ee4a
|
feat(desktop): add Aliyun OSS release mirror (#8976)
* feat(desktop): mirror releases to Aliyun OSS * fix(desktop): harden OSS mirror workflow and tests - Add ref guard to sync-desktop-to-oss.yml (dispatch only from main) - Add diagnostic error messages for missing Windows/Linux installers - Harden test: pin verify-index > 0 before ordering comparison - Harden test: pin confirm-before-publish ordering and source comparison - Add test: stable-only release validation in reusable sync job * fix(desktop): harden OSS mirror permissions, stable-version guard, and non-latest repair - Remove workflow-level actions:read; grant it only to the sync-oss caller job - Reject suffixed versions for published stable releases in prepare - Turn latest-feed comparison into a non-fatal check; condition publish/verify on match - Assert both check_for_update call sites in release test - Add jq stable-only guard assertion and endpoint default alignment test Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
3a59332361
|
fix(ci): seed the dist-rebuild warning on every retryable A/B exit (#8958)
* fix(ci): seed the dist-rebuild warning on every retryable A/B exit The baseline leg rebuilds dist/ from baseline sources, and every retryable exit of the verify gate hands that tree to the repair agent — but the "run npm run build first" steering note only reached the green-baseline rejection. The comm -23 comparison failure and different-signature exits sent the repair agent in blind, free to trust or test against stale baseline artifacts (the different-signature exit carried the note on #8765's branch; the #8878 port kept it on the green exit only). #8765's post-close round-3 review flagged the comm path as Critical. Append the note on both missing exits and pin all three paths: the DIFFERENT-reason test now asserts the note, a new test stubs comm to fail and asserts the same, and the pre-existing test pins the note OUT of its document — no repair runs for that verdict. Mutation-tested, 3 of 3 caught: comm-path note dropped, different-reason note dropped, note leaked into the pre-existing document. * fix(ci): single emit point for the dist note, name the comm-failure exit Address the two review suggestions on #8958: - The steering note existed as three byte-identical copies, and the "every retryable exit seeds the note" invariant depended on copy-paste — the exact drift this PR was patching (one exit seeded on #8765's branch, one lost in the #8878 port). Both reviewers flagged it. The string now lives in seed_dist_note(), called from all three exits. - The comm-failure exit seeded the note but, unlike its sibling retryable exits, emitted no verdict-rationale line — an oncall could not distinguish "the comparison itself failed" from "baseline is green" without re-running the A/B. It now says so. Mutation-tested: mutating the string inside the helper fails all three path assertions at once; mutating the rationale line fails the comm-exit test. * test(ci): pin the no-identity baseline arm of the A/B gate (#8958) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
2cff1e7ad3
|
fix(ci): restore Live Host release mirroring (#8917) | ||
|
|
9946d13a35
|
ci: install tmux and zip tooling on the Linux test lane, and pin it (#8792)
* ci: install tmux and zip tooling on the Linux test lane, and pin it Two suites go quiet on a runner without their tooling. capture-tui's real-tmux block is `describe.skipIf(!hasTmux)`-gated, so on the hosted ubuntu image — which ships no tmux — every real-tmux behavior skips inside a green required check. The install-script packaging suite needs zip and unzip the same way. This installs both where apt exists, bounded (timeout-minutes) and advisory (continue-on-error absorbs the bound), so a stalled mirror or dpkg lock neither hangs the job toward its 60-minute cap nor reds the check. A step that silently stops installing is the same outage as no step at all, so it comes with a pin that reads the workflow the way bash does rather than the way it looks. The parser is quote-aware (a `#` inside a message is literal; a `;` inside one does not split statements), continuation-aware (only an odd run of trailing backslashes continues a line, and a comment never continues), and wrapper-aware (an apt-get behind sudo/env/nice/ timeout/time/nohup, with or without their options, or inside a subshell, is still an apt-get). On top of that it pins what actually keeps the step working: the if-condition whole (a one-character `&&`→`||` took the already-installed branch on a lane with zip but no tmux), the elif whole (a near-miss falsifier like `false2 &&` killed the install branch), the else fallback (deleting it removed the only signal on a lane with neither tmux nor sudo), exact package tokens (`powerline-tmux` is not tmux), the flags that make it work unattended (-y, sudo) and the ones that make it a no-op (-s, --download-only and friends exit 0 having installed nothing), that nothing may hard-fail the step in any errexit spelling or through a subshell exit, that every apt-get REACHES a `|| echo` guard (an AND-list is transparent, a pipeline or a `;` is not), and that every annotation reaches stdout unredirected with `:⚠️:` at line start — a workflow command the runner cannot see is just log noise. Split out of #8388, whose capture-tui command is what made the tmux half load-bearing. Each pin was mutation-verified against this workflow file. * test(ci): close the pin's own escapes, and give its bash model tests Thirteen findings on this PR, all against the pin rather than the step, and two of them were the pin being WRONG rather than weak — those matter most, because a pin that reds a legitimate edit is worse than one that misses. Wrong: the redirect check matched `>` anywhere in an echo statement, including inside the quoted message, so a semantics-preserving reword was rejected — exactly what this file says it must not pin. And the sudo requirement demanded literal `sudo apt-get` adjacency, which rejects `sudo -n apt-get`, the sibling workflow's own convention. Both now ask the question at the right level: quoted spans are blanked before any question about shell syntax, and "runs through sudo" is asked of the raw statement. Weak, each mutation-verified against ci.yml: `&>` was split as a separator, so `echo ':⚠️:…' &> /dev/null` tore into a clean echo and an unchecked redirect; apt's `-o APT::Get::Simulate=true` reached simulate mode through a door the token blacklist never saw; the package pin matched the whole logical line, so a package dropped from the install survived if its token appeared in the guard's message; `apt-get update` was required by nothing; `-y` and sudo were checked only on the FIRST install statement, and nothing kept a foreign command off the install's chain to short-circuit it; the step's `shell:` key — the one key that decides which interpreter parses the entire block — was unpinned; the closing `fi` was unpinned, so deleting it passed 5/5 while bash rejects the block and the step runs nothing; and the else branch's annotation was a substring check unanchored to any emitting command, so `: ':⚠️:…'` muted it. The ~180 lines of bash-semantics helpers are the ORACLE every pin reasons through, and they had no tests of their own. They do now — a table per rule, each rule traceable to a mutation that escaped an earlier version. Writing them corrected one of my own assumptions: `echo a \` + newline + `b` joins to `echo a b`, not `echo ab`, because the space before the backslash survives. Also corrected the rationale in both files: they asserted in the present tense that a real-tmux suite exists in this tree. It does not — it arrives with #8388, and the tmux half of this step is deliberately pre-landed for it. Saying so is the honest version. * test(ci): close the pin's remaining escapes, and finish its bash-model fixtures Four gaps survived the first pass, each probe-verified against the suite before the fix: - an always-failing `apt-get install` prefixed in the same && chain short-circuited the real install with every pin green, because every per-statement pin binds to a statement on the line and none sees the skip — now the step must carry exactly one install statement; - `apt-get update` could be deleted or moved behind the install — now it must exist AND precede the install in statement order; - `return` and `exec` were missing from the hard-fail blacklist, the same family as `exit`/`false` under the runner's `bash -e`: either leaves the required check green with nothing installed and not even the else-branch warning emitted (sandboxed verification's F1) — now blacklisted, with an end-to-end splice fixture pinning the axis; - the guard walker's operator alphabet still split on the `&` of `&>` although statementsOf no longer does, reddening a guarded `apt-get … &> /dev/null || echo` — both now share the exemption. Also finished the bash-model fixtures round one asked for: a quote spanning a newline, and `timeout -k 1 5 sudo …` wrapper stripping. Writing them exposed one oracle bug the round's own failure scenario had measured: `set -- -e` assigns positional parameters, it does not enable errexit — `--` now ends option parsing for the check. Mutation battery against ci.yml: the five escape mutants (prefixed install, return 0, exec true, update deleted, update reordered) all flip red; the five tolerated forms (sudo -n sibling, dropped --no-install-recommends, wrapper-wrapped update, &> redirect with the guard intact, reworded warning) all stay green. * test(ci): close the escapes a real edit would take, and stop there Seven of sixteen findings, chosen by one question: would an honest edit hit this? Each is mutation-verified against ci.yml. - apt's long option spellings reached simulate mode past a `-o`-anchored regex (`--option APT::Get::Simulate=true`, `--simulate=yes`), as did `--version`/`--help`, which print and exit having installed nothing. - The chain allowlist checked only a statement's leading word, so `&& sudo apt-get remove -y tmux` appended after the install passed while undoing it. It checks the subcommand now. - `apt-get update` was pinned as load-bearing but never required to run through sudo, though that is the whole reason the install is. - A quoted redirect target (`>'/dev/null'`) muted an annotation: blanking quoted spans leaves the operator, so the pin matches the operator itself. - The already-installed branch's BODY was unpinned — emptying it to `:` left a broken-but-installed tmux undetected on the lane that takes it. - The step was pinned before the test step but not AFTER the `ci_profile` step its `if:` reads; above it the condition is always false and the step silently never runs. - The separator alphabet was encoded three times and the copies had already drifted. One definition now, used by all three consumers: a model of bash that depends on which function you ask is not a model. The nine I did not take are recorded on their threads with reasons. Most are adversarial rewrites — swapping branch bodies, wrapping the block in a never-executing loop, nesting a conditional inside a pinned branch, hiding the chain in a heredoc — and this file cannot win that game: anyone who can restructure the step can also delete it. It guards accidental drift, which is the failure that actually happens. One asks for a composite action shared with qwen-autofix.yml; that is a repo-wide refactor and does not belong in this PR. * ci: make the install fail fast, bound its apt calls, and verify all three tools Four review findings, all about the step rather than its pin, and all four taken — they are the shapes a real runner produces. `sudo` becomes `sudo -n`, the convention nine other steps in this file and the sibling workflow already use: without it, a runner without passwordless sudo gets a password PROMPT on stdin and the step hangs to its timeout instead of falling through to the warning. The `elif` tests sudo's USABILITY (`sudo -n true`) rather than its presence: `command -v sudo` passes on a runner where sudo exists and cannot be used, which takes the install branch straight into that hang. The apt calls carry their own `timeout 280` inside the step's 5-minute bound. When the STEP timeout fires there is no `|| echo` — the lane loses its tooling and says nothing. A shorter command-level bound lets the guard run and the annotation reach the check UI. The already-installed branch verifies zip and unzip too, not just tmux: it is taken when all three are present, so a broken-but-present zip failed the packaging suite with no warning explaining why. The pins moved with the step: "runs through sudo" now walks the wrapper prefix (a bounded install is `timeout 280 sudo -n apt-get …`, and requiring sudo FIRST would have redded it), the elif equality carries the new condition, and all three tool probes are pinned to exist AND to be guarded. That last pin needed a second pass: matching by substring reported the zip probe present after deletion, because `unzip -v` contains `zip -v` — it is word-anchored now, and each of the three deletions turns it red. * fix(ci): install the tooling on root lanes too, where sudo does not exist The Test check reds on a lane that ships no zip: install-script.test.js throws at module load on a CI host missing zip/unzip — loudly, by design — and this step could not prevent it on a root-container lane, because its only install branch required `sudo -n true`, and root lanes have no sudo (and need none). The step fell through to the advisory warning, nothing installed, and the packaging suite threw inside the required check. Add a root branch ahead of the sudo branch: uid 0 runs the same bounded, guarded chain as-is. Lane probes under the runner's exact bash flags: a root lane installs without sudo, a sudo lane installs through sudo -n, a bare lane warns and exits 0, and an apt failure still reaches the guard. The pins moved with the step: each branch must carry its own update-then-install chain with exactly one install, and the branch decides the sudo shape — the root chain must NOT reach for sudo (on a lane with no sudo binary that exits 127, the guard fires, nothing installs), the sudo chain must. Six mutants red, pristine green. * fix(ci): fit the apt bounds inside the step cap, and pin what still escaped * test(ci): close the pin's round-4 escapes, and stop reddening its rewordings Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ci): close the pin's round-5 escapes with a branch-body allowlist --------- 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> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
a86f1e5c0a
|
fix(ci): stream autofix agent progress (#8895)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
* fix(ci): stream autofix agent progress * fix(ci): preserve autofix API error retries * fix(ci): trust structured autofix results * fix(ci): bound autofix stream processing * fix(ci): ignore oversized autofix stream lines --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> |
||
|
|
5dc98240c7
|
fix(ci): port the verify gate's remaining hardening from #8765 (#8878)
#8816's branch accidentally carried #8765's early commits, and the takeover loop evolved the gate further there (subset identity via comm -23, the retryable third arg, subset fixtures) — so #8765 closes as subsumed, and this PR ports what main still lacks: the two improvements its reviewers named for porting, plus the open round-6/7 findings that survive on main's gate. - Pre-detach short-circuit: an empty head signature (vite/esbuild/ crash — the KNOWN LIMIT class) fails closed regardless of the baseline, so decide it BEFORE paying the detach + full baseline re-run + restore. - Build-dirt guard: the A/B'd build REWRITES a tracked file (the vscode companion settings schema), and the undiscarded rewrite makes either checkout refuse — degrading a real verdict into the crash path. `git restore -- .` before both checkouts; tracked-only, and the tree was asserted clean before the deterministic checks. - Restore-failure semantics: a plain outcome=failed is an EVALUATED rejection — the watermark advances and a transient git failure strands the item as a permanent human handoff. The gate now leaves outcome unset (the gate-crashed path retries next scan) and still writes the detail document so the crash comment explains itself. - The dist-rebuilt steering note seeds the repair feedback on both retryable A/B exits — the repair agent's only warning that dist/ holds baseline-built artifacts. - The stale-base retry handoff prefixes its embedded rejection with a the-base-has-moved note, so the retry agent is not steered toward no-action by framing written before the auto-update. - The two A/B side logs joined the repair step's cleanup list. - Tests: identity-less short-circuit, tracked-dirt survival, verdict-less restore crash, long-preamble render cap, PREEXISTING clause selection through the executable report harness, and the stale-framing note pin. Mutation-tested, 5 of 5 caught: short-circuit dropped, restore guards dropped, restore-failure reverted to the evaluated rejection, dist note dropped, stale-framing note dropped. Co-authored-by: verify <verify@local> |
||
|
|
7f61e03b82
|
perf(ci): make the triage budget operator-tunable and raise it (#8810)
* perf(ci): make the triage budget operator-tunable and raise it The triage job's fixed 30-minute cap was killing every substantial PR's triage at exactly the budget — five observed (8723, 8726, 8731, 8764, 8801), each a full agent run discarded — and the successful-run distribution shows why: 22 jobs sampled ran median 5.8m / p90 22.3m / max 29.5m. A max just under the cap is a distribution being clipped, not one that fits. Back the budget with QWEN_TRIAGE_TIMEOUT_MINUTES (fallback 60), the same pattern as the autofix fleet caps in #8731 and probe-verified there for expression-valued job fields. Triage is advisory: the worst case a longer budget risks is one held runner, while a killed run wastes the same runner and the work it was about to publish. Pinned by a test asserting the variable-backed form with its fallback; reverting to the fixed literal fails it. * Address review: sanitize the knob, pin the active line Both findings verified before fixing. The knob was unvalidated, and the failure mode is nasty precisely because the knob exists to be changed WITHOUT a PR: a malformed repository variable — '60 minutes', '1h', a 6O typo, quotes, a decimal — would make fromJSON throw at expression time, killing the triage job before any step runs: no always() finalize, no status comment, a silent stop whose run log blames fromJSON rather than the variable. And '0' parses fine into an instantly-cancelled job. timeout-minutes cannot be validated by the job that consumes it, so the authorize job (which triage already needs, and which can run bash) sanitizes it: bare-integer check, floor 10, ceiling 600, every rejection warning with the VARIABLE's name. The triage job consumes the sanitized output; || 60 covers the events where authorize is skipped. The pin was an unanchored substring over raw YAML, satisfied by a commented-out line while the job silently inherits GitHub's 360-minute default — the review probe-verified that mutant surviving. The pin now matches active lines only, and the sanitize step is replayed verbatim against the review's whole enumeration: unset, sane, zero, runaway, and five malformed shapes, each asserted to fall back and name the knob. Mutation-tested, 3 of 3 caught: the timeout line commented out, the floor clamp dropped, the integer check bypassed. * fix(ci): clamp over-long triage budgets before bash arithmetic (#8810) A repository variable with more than 18 digits overflows bash's 64-bit arithmetic, and the wrapped residue can land inside [10,600], silently skipping both clamps with no warning — the one malformed shape where the sanitizer's diagnostics go dark. Check the digit count before the arithmetic and clamp such values to the ceiling with a warning. Also pin the seams the replay test cannot see: which repository variable feeds RAW, that the sanitize step is unconditional, the exact clamp boundaries, the 10# decimal guard, and the overflow clamp. * fix(ci): strip leading zeros from triage budget before the width guard (#8810) * fix(ci): pin triage timeout seams on parsed YAML and runner shell flags (#8810) --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
c0c0352e25
|
feat(cli): mirror Live Host releases through OSS (#8674)
* feat(cli): mirror Live Host releases through OSS * fix(cli): harden Live Host OSS mirror fallback |
||
|
|
358091833b
|
fix(ci): watchdog silent sandbox hangs and reap the containers they leak (#8816)
* feat(ci): A/B deterministic gate rejections against the pre-round ref A deterministic rejection in the autofix verification gate is only chargeable to the round if the same check passes without the round's commit. The gate charged every red to the fix unconditionally, and run 31276008548 measured what that costs when the premise is false: PR 8614's branch predated #8693's tsconfig guard while node_modules came from the post-#8693 trusted base, so `npm run build` was equally red at origin/<branch> — 63 minutes of accepted agent work discarded, an 18-minute repair burned on a failure the repair agent is forbidden to touch (it may only amend the round's own fix), thirteen rounds in a row, and the same again on the #8616 leg. On rejection the gate now re-runs the failing check at origin/<branch> (the branch as pushed, before the round) in the same environment: - baseline green: today's path exactly — outcome=failed, retryable=true, the repair pass gets its chance. - baseline red too: outcome=failed with preexisting=true and NO retryable. The repair step keys on retryable and is skipped — it cannot reach a failure outside the round's diff by construction — and gate-rejection.md says outright that the branch needs a base update (merge main), which flows into the failure comment as-is. Fail-closed toward today's semantics: any A/B infrastructure problem (missing ref, checkout failure) charges the fix as before, and a restore failure after the baseline run rejects outright since the tree can no longer be trusted. The round's work is still not pushed — this changes the verdict's honesty and cost, not the push policy. Tested by executing the real script in a real two-remote git repo with an npm stub whose failures are keyed by commit SHA: round-caused red (baseline green), pre-existing red (both red), and the untouched green path. Mutation-tested, 3 of 3 caught: skipping the A/B, claiming pre-existing without measuring, and dropping the tree restore. * Address review: bound the A/B to checks it can honestly compare All seven findings verified before fixing; the three Criticals were each a way the A/B compared something other than the check that failed. R1-1 — the contracts check feeds on stdin, which its first run drains; the baseline leg re-ran against EOF and checked an empty file list. R1-3 — the schema check's verdict rides on packages/core/dist, which the core-rebuild guard built from ROUND sources and which, being gitignored, survives the detach. Both checks are now A/B-exempt (run_check_no_ab): their baseline verdicts prove nothing, and their rejections stay where the repair agent can actually act on them. R1-2 — a workspace the round ADDS does not exist at the baseline, and npm exits 1 there with "No workspaces found" (measured; --if-present forgives a missing script, not a missing workspace) — a round-caused failure misread as pre-existing, skipping the one repair that can fix the round's own package. The per-package loop now A/Bs only when the workspace exists at origin/<branch>. R1-4 — a chatty PASSING baseline used to flood the tail -c 3000 evidence window and push the actual failure text out of gate-rejection.md, the sole carrier into the repair feedback, the PR comment, and the next round's LAST_REJECTION. The baseline transcript now goes to a side log and only a FAILING tail is merged back, where it is the evidence. R1-5 — the pre-existing paragraph pushed gate-rejection.md past the report's head -c 3500 cap, truncating the closing fence for branch names past 44 characters. Cap raised to 3900, invariant comment updated with the new arithmetic. R1-6 — preexisting=true had no read site. It now flows verify → Finalize verification → the failure report, whose headline swaps the generic gate clause for "PRE-EXISTING failure … needs a base update (merge main)". R1-7 — the no-round-commit guard was unpinned (deleting it kept all tests green). Now exercised through the core-rebuild path, the one A/B-eligible check that runs before the commit gate. Four new behavioral scenarios (chatty baseline, no-commit round, A/B-exempt checks, round-added workspace) plus workflow pins for the forwarding, the clause, and the cap. Mutation-tested, 4 of 4 caught: schema back to A/B (3 tests), guard dropped, side log reverted, no-commit guard dropped. * Address review round 2: A/B only what it can prove, prove what it claims Ten findings across two rounds, each verified before fixing. The three deepest share one lesson: the A/B is only sound for a check whose inputs travel entirely with the git ref, and whose failure it can IDENTIFY, not merely observe. R2-1 — rc=1 at both legs does not make them the same failure: the branch can fail for reason A while the round fails for reason B, and a baseline infrastructure hiccup is a nonzero exit too. Pre-existing now requires a MATCHING failure identity — tsc diagnostics normalized to file + error code (positions shift with the round's edits), compared via comm(1) on a per-check transcript. No diagnostics on either side means identity cannot be established and the round stays charged. R2-2 / R2-7 — gitignored dist survives the detach carrying the ROUND's build, so any dist-consuming check A/Bs reverted sources against round-built artifacts: package tests (channel-base resolved through dist exports) and typecheck (sdk-typescript resolves core's d.ts — probe-verified three-arm flip). Both are now A/B-exempt, as is lint, leaving `npm run build` — the incident class, and the one check that rebuilds its own inputs from the checked-out sources — as the sole A/B candidate. The workspace-existence guard dissolves with it. R2-3 — the fixture inherited the caller's global git config; a failing global pre-commit hook broke all seven cases. The harness now isolates GIT_CONFIG_GLOBAL/SYSTEM for every git child, and the suite is proven green under a deliberately hostile hooksPath. R2-4 — Finalize verification now selects preexisting from the same attempt whose outcome it selects (repair verification included). R2-5 / R2-8 — the "merge main" advice is now conditional at both layers: the script paragraph states the measured fact and hedges the remedy; the report headline uses the compare the step already ran — behind/diverged gets the base-update clause, an up-to-date branch is told its own pre-round code needs attention. R2-6 — the rejection document now sizes its evidence tail against its preamble (floor 500 bytes, total under the 3900-byte render cap), so the closing fence can no longer be truncated off by a long branch name. R2-9 — dissolved by R2-2: package tests no longer A/B, the guard and its uncovered positive branch are gone. R2-10 — the baseline-evidence merge is now pinned: the pre-existing scenario asserts the baseline leg's own failure line (keyed by its SHA) reaches gate-rejection.md. Eight behavioral scenarios; mutation-tested 5 of 5: identity dropped, typecheck re-enrolled, package tests re-enrolled, evidence merge dropped, fixed tail restored. * Address review round 4: sharpen identity, stage the git failures, sync prose Nine findings, all refinements — the design held, the edges did not. Identity now keeps the diagnostic MESSAGE (file + code collide: two unrelated TS2339s in one file compared equal, skipping a repair that could have shipped — probe-reproduced by the review), and the fixture emits a SHIFTED position on the baseline leg so the position strip is load-bearing instead of decorative (deleting the sed survived every test before; it fails one now). vite/esbuild failures still yield an empty signature by design — documented as the fail-closed limit rather than half-widened. The fail_signature assignments take `|| true`: grep exits 1 on the normal no-match case and survives errexit today only because the caller sits in an if-condition — a future unconditional call site would crash the gate verdict-less. The restore-failure branch is now stageable and staged: the baseline leg recreates (untracked) a file the branch tracks, the checkout back refuses, and the test pins retryable-not-preexisting with the 'could not restore' label. Relaxing the branch to `|| true` fails it. Prose synced to the mechanisms that replaced it: the render-cap invariant restates against the dynamic tail budget (the old 3000-based arithmetic would misguide the next retune), the no-round-commit guard comment names the core rebuild (schema/contracts left the A/B last round), the describe wording counts both A/B-eligible builds, and the pre-existing clauses no longer claim "the repair pass was skipped" — with REPAIR_PREEXISTING forwarded, repair may have RUN; they now state the invariant that is true either way: repair may only amend the round's own fix, so it cannot reach this failure. Mutation-tested, 3 of 3 caught: position strip dropped, message dropped from the identity, restore rejection relaxed. * fix(ci): watchdog silent sandbox hangs and reap the containers they leak Four autofix rounds have died the same way (#8663 twice, #8761 r3, #8763 r4): the agent's last output is the sandbox wrapper's "ContainerName (regular): …" line at docker container entry, then nothing — not one event — until the 2-hour absolute budget kills the round. Four different runners, two image versions: systemic, not a bad machine. Where exactly the container wedges is still unknown (that needs docker state on the runner); what is certain from the logs is the shape — a wedged sandbox produces NOTHING, and a legitimate run is never silent for long (the fleet's longest tolerated quiet is the review pipeline's 10-minute stream-idle window for thinking phases). Two mitigations, each aimed at a measured half of the damage: - run-agent.mjs gains an idle watchdog (QWEN_IDLE_TIMEOUT_MS, default 20 minutes = 2x that longest legitimate silence): zero output for the window kills the agent with a distinct "idle-timeout … the sandbox likely hung at startup" detail, so the failure comment names the right knob and a hung round costs 20 minutes instead of 120. Polled, not reset-per-chunk — a busy stream should not spend its time re-arming timers. - Both sandboxed jobs reap stale qwen-code-* containers at job start: a budget kill reaps the HOST-side docker client, not the container, so every killed sandbox keeps running on the persistent runner — observed directly when a later leg's container-name counter found qwen-code-0.21.8-0 already occupied and picked -1. One job per runner at a time makes any container alive at job start stale by definition. Tested by executing the real run-agent.mjs end to end with stub agents: the hang shape (one line, then silence) dies at the idle window naming the idle limit, and a slow-but-talking agent that outputs every 400ms across a 1500ms window survives to a clean exit — the test that distinguishes a watchdog from a disguised absolute timer. Mutation- tested, 3 of 3 caught: watchdog disabled, last-output tracking dropped (the disguised-timer regression), cleanup dropped from a job. * Address review round 5: the gate's verdict defects and the reaper's live kill Budget-warning round — the five Criticals from both reviewers, no suggestions (each deferred with a recorded reply). fail_signature: `[^\n]*` in an ERE bracket expression does not mean "rest of line" — in POSIX bracket expressions `\` is literal, so it matched "neither backslash nor the letter n" and truncated every tsc message at its first n. Nearly every real message has an early n ("Cannot find name", "is not assignable"), so distinct same-file failures collapsed into identical signatures and a round-caused failure could be labeled pre-existing, skipping the repair. grep is line-oriented: `.*` is exactly the rest of the line. New fixture: two messages differing only after their first n. Pre-existing verdict: the intersection test mislabeled in both directions. A round that ADDS a diagnostic sharing one normalized line with the baseline was called pre-existing (repair skipped for a round-caused, repairable failure); and `comm -12 | grep -q` under `set -eo pipefail` SIGPIPEs comm (exit 141) once the shared output outruns the pipe buffer, charging true pre-existing failures to the round — the exact 18-minute repair waste the gate exists to kill. Pre-existing now means the round's failing set is a SUBSET of the baseline's, and the difference is captured before testing. New fixture: a round adding a second diagnostic to a failing baseline. Restore failure after the baseline leg: was retryable=true with HEAD still detached at the baseline commit — the repair agent works in that very checkout and does no git recovery, so its commit would land on the baseline and be orphaned. Now rejected non-retryable (reject_fix grows a third arg); the next round starts clean from the trusted checkout. The restoreClash test pins the new semantics. Stale-container reap: the premise "a runner runs one job at a time, so any live qwen-code-* container is stale" holds per runner registration, but the filter queries the docker daemon, which is per host — and this pool runs several registrations on one OS. With per-issue/PR serialization only, a concurrent job's sandbox is a substring match away from `docker rm -f`. The reap now takes only provably-dead containers (--filter status=exited/dead, both jobs) and the comment says why a running one is left alone. Preamble printf: the `\`` escapes sat inside a single-quoted format where backslash is literal, so every pre-existing rejection rendered raw backticks instead of code spans (shellcheck SC2016). Backticks need no escaping there. Also syncs the side-log comment to the dynamic tail_budget it actually renders. Verified: scripts suite 140/140 (was 138; the two new fixtures and the rewritten restoreClash test all fail against the pre-fix script), npm run build / typecheck / lint pass, bash -n clean. * Address review round 6: reap the kill's own orphan, tolerate the reaper * Address review: hang-bound the reaper, unblock the kill path, pin the unpinned arms - Wrap every docker call in the stale-container reap with timeout 30: an alive-but-wedged daemon blocks docker ps indefinitely, and the existing || guards only catch nonzero exits, not hangs (R3-1). - Make the kill-path container removal async in run-agent.mjs: the spawnSync blocked the event loop between SIGTERM and the 10s SIGKILL backstop for up to its 30s timeout — in exactly the wedged-daemon scenario the watchdog exists for. The main flow awaits the removal so the leak warning stays deterministic (R3-6). - Split the pre-existing gate clause for an empty CMP_R: a transient compare-API failure is "never measured", not "measured not-behind", and must not assert the branch's own code is at fault (R3-7). - Swap the timeout breaker's closing remedy to the sandbox investigation when every counted timeout was idle, mirroring the round-level split (R3-11). - Tests: pin the budget kill path separately from the idle kill path (R3-3), parameterize the idle-window parse guard over -1/0/NaN (R3-5), add a stderr-only liveness case (R3-12), pin the strict-subset A/B arm via a baseline-superset fixture knob (R3-15), and pin the breaker's current-round idle increment (R3-18). --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
e60d182241
|
perf(ci): tighten the automatic review kill switch for micro diffs (#8774)
* perf(ci): tighten the automatic review kill switch for micro diffs Below the review skill's sweep floor (25 changed lines) an automatic review keeps --effort high and its inline comments — a medium downgrade would drop exactly the inline findings a source fix deserves, and with the file-scoped suites and the one-round reverse audit the pipeline itself is what shrinks on a micro diff. What a micro run must not keep is a multi-hour kill switch: the small-PR budget halves with the same 90-minute floor the docs-only downgrade uses, so a hung run dies at the scale of its work. Reuses the PR_SIZE_LINES the size-aware budget already fetched — no new API call; an unknown size never tightens, an explicit --timeout wins (the size block is skipped), and a docs-only run is already halved, never twice. Gate tests drive the extracted step source with seeded sizes: the 24/25 boundary, the floor, the docs-only interaction, and the unknown-size fallback. * fix(ci): share one halve-with-floor implementation and pin the guard states Review rework. The docs-only branch and the micro tightening now call one halve_budget_floor() — a / 2 → / 3 mutant survived every test because both micro inputs land on the floor under any divisor ≥ 2, and two verbatim copies let a one-sided edit diverge the branches while the comments claimed they matched; the floor cases now execute the shared function and a structural pin asserts one definition, two calls, one occurrence of the arithmetic. The threshold comment states the unit honestly (total churn rides the skill's source-weighted sweep floor in the direction that cannot over-tighten). Two surviving guard mutants get pins: a manually requested review with a populated size is never tightened (the caller owns its timeout), and a failed docs classification still tightens a micro automatic run (the guard keys on != "true", not = "false"). * fix(ci): state the micro-tightening's true justification, not a false invariant The comment claimed "total churn < 25 implies the skill's source-weighted measure < 25", which is backwards: srcDiffLines counts raw unified-diff lines (file/hunk headers, context), so it is LARGER than churn — a scattered micro diff (churn 10 across 5 files) computes srcDiffLines ~65 and keeps sweep on, un-shrunk, while the gate still halves its budget. The threshold is now stated as what it is: an independent "small PR" churn bound, deliberately not SWEEP_FLOOR (the two measures differ, so a micro diff may still run the sweep and the full reverse audit). The tightening is justified by "churn < 25 bounds the reviewed territory and 90 minutes is ample for it even on the full pipeline" — measured, a 23-line PR runs high end to end in ~30 min — not by the pipeline shrinking. With the SWEEP_FLOOR coupling claim removed, there is no cross-file coupling left to drift, so no equality pin is owed. * fix(ci): reword the micro test comment to the independent churn bound Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): reword the micro gate comment to the 180-minute budget it actually halves Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: verify <verify@local> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
3c3084e78a
|
fix(ci): route workflow label mutations through REST (#8761)
* fix(ci): route workflow label mutations through REST `gh pr edit` cannot mutate anything on this repository: its GraphQL lookup requests repository.pullRequest.projectCards, and with Projects (classic) attached GitHub returns the deprecation as an error, so the command exits 1 before applying the change. Reproduced from a live clone against PR #8755 — the error names the field outright. Three workflows carried label mutations through it: - pr-self-report-label.yml: every add/remove arm failed — 43 straight run failures from 2026-08-04 on; the green runs were all the nothing-to-do arm. Self-reported PRs (like #8755, whose author also opened #8750) never got the label. - qwen-autofix.yml: the `@qwen-code /takeover` and `/takeover stop` COMMAND paths never toggled the label — only the UI label events worked, so the command was dead weight wearing an ack. - repo-hygiene.yml: the add was `|| echo`-guarded, so it never failed the run — it just never labeled anything, while the fallback message blamed a label that exists. All five sites now use the REST issues/labels endpoints, which never touch that query. Two traps handled on the way: - Every label involved contains a slash, and in the DELETE the label is a PATH SEGMENT — unencoded it 404s. Encoded via jq @uri, and the tests assert the literal %2F because a real jq runs in the replay. - The REST add auto-creates a missing label, which repo-hygiene explicitly promises never to do — that site gets an existence probe first, and its misdiagnosing fallback message is corrected. Verified live on #8755 before editing anything: the exact gh pr edit call fails with the projectCards error; REST POST applies the label (backfilling the one it was owed), DELETE with %2F removes it. Tests: the stub-driven replays for both the self-report step and the takeover toggle now pin the full REST method + path (encoding included), and a repo-wide guard bans `gh pr edit --add-label/ --remove-label` in every workflow so the class cannot return. Mutation-tested, 6 of 6 caught: each of the five sites reverted to gh pr edit, and the DELETE stripped of its encoding. * fix(ci): harden REST label mutation steps per review (#8761) * fix(ci): pin REST label failure policies per review (#8761) Review round for the REST migration: - The DELETE arms tolerated EVERY failure (`|| true`), masking 403/5xx/network errors behind a green run and a false "removed" log. They now tolerate only the documented 404 race — any other failure emits a :⚠️: while keeping the step green (pr-self-report-label) and the release ack alive (qwen-autofix). - Neither replay harness could make a `gh api` call fail, so both failure policies were unpinned. They gain failure knobs (knob value on stderr like a real gh HTTP error) and now pin: 404 race silent, other DELETE failures warned, POST loud. The toggle replay also moves to -eo pipefail like the runner's bash default, reproducing the step's real failure semantics. - The jq stub enforced only the --arg shape; it now also enforces the `$l|@uri` program, so a filter mutation fails the suite instead of riding the stub's unconditional percent-encoding. - The gh-pr-edit guard misfired on comments and miscounted lines after joining continuations: comments are stripped before matching, and offenders are reported at the physical line where the (possibly wrapped) command starts. Mutation-tested with 8 probes, all caught: blanket || true on either DELETE, || true on either POST, dropped |@uri, a comment quoting the ban (stays green), an executable and a wrapped violation (both red, correct line). * Address review round 3: close the guard evasions, convert the release path Four round-3 findings, each reproduced before fixing, plus the release path the round-1 scope note deferred. - The ban guard now scans what bash executes, not the YAML surface: the decoded run: values of every parsed workflow, whole-line comments stripped, continuations joined the way bash joins them (backslash- newline removed, nothing inserted), matched whitespace-tolerantly. All three reproduced evasions — a # inside a quoted string eating the trailing backslash, wraps inside the command prefix or a flag token, and folded scalars — are fixture-pinned. Offenders report as file » job » step; line numbers stopped meaning anything after joins. - classify-release-notes.mjs mutates labels through REST now, and the guard grew an argv-form scan over .github/scripts/*.mjs that flags the old file (negative-controlled) — the release path was the last gh pr edit label site, failing silently behind continue-on-error. - JQ_STUB enforces the full invocation: -rn (with -r alone real jq evaluates zero inputs and prints nothing), the binding name l (real jq exits 3 on $l undefined), and the program. Either reproduced mutation previously expanded the substitution empty, sent the DELETE to …/labels/ with no name segment, and the 404 tolerance swallowed it. - The takeover engage POST gets the idempotent create its siblings carry, pinned to the label's real color (1D76DB): the REST add would re-create a deleted label silently with a random color. - runToggle captures writes on throw, and the engage-failure assertion now pins the ORDER its comment claims: a failing apply must leave no "takeover-ack engaged" in the captured writes — the bare toThrow passed even with the ack moved above the POST (reproduced). - The two REMOVE_ERR DELETE idioms are drift-pinned byte-identical modulo the label variable, the honest substitute for sharing shell across workflow files. Mutation-tested, 6 of 6 caught: the evadable regex restored, -rn and the binding name mutated in the workflow, the create dropped, the ack posted before the POST, and the old .mjs flagged by the new scan. --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
afcc937ec5
|
fix(ci): render the queued-acknowledgement comment (#8726)
* fix(ci): render the queued-acknowledgement comment The ack comment posted on every PR that requests a review was built as <!-- qwen-review-ack -->_Qwen Code review request accepted. …[workflow run](URL)._ with the prose glued straight onto the marker. A line opening with `<!--` starts an HTML block, and that block runs to the line containing the closing delimiter INCLUSIVE — the rest of that line is still inside it and never parsed as Markdown. So the comment shipped as raw source: no emphasis, and the link to the workflow run was dead text. That link is the only pointer a PR author gets to their review run — `issue_comment` runs are not attached to a commit, so they never appear in the PR's checks list. Losing it leaves no way to reach the run from the PR. Measured through GitHub's own renderer (POST /markdown, mode=gfm) on the exact bodies: marker + text -> 0 <a>, 0 <em> marker + \n + text -> 1 <a>, 1 <em> marker + \n\n + text -> 1 <a>, 1 <em> Use the blank-line form, matching how autofix-status already builds its body. The marker text is unchanged, so the `contains(...)` upsert lookup still finds prior acks and updates them in place. Pinned by a test that scans every marker in the workflow and rejects one with prose glued to it, skipping comment lines. It fails against main, naming the offending line. * fix(ci): harden the marker guard per review round 2 Pin the workflow-run URL weaving into the ack printf, bound the marker scan to the marker's physical line, anchor the newline exemption to the literal the marker opens, widen it to double-quoted printf formats, and flag unquoted command-substitution concatenation. Declare the remaining coverage gaps in the test instead of papering over them. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): widen marker-guard regex and declare known gaps per review * fix(ci): pin ack link shape and dedupe workflow scan per review --------- Co-authored-by: verify <verify@local> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
9381827c88
|
fix(ci): enter Critical-only mode after five change rounds (#8751)
The suggestion cutoff sat at ten change-producing rounds, but the strict round cap discards a plain PR at round ten before that threshold can engage — so in practice it only ever bound takeover PRs, which spent ten rounds growing their diff on lower-severity feedback before the brake applied. Lower it to five so the loop stops implementing suggestions while the diff is still reviewable. The autofix skill already documented the five-round boundary; the workflow now matches it. |
||
|
|
d91c66119b
|
fix(ci): match /review commands followed by a newline (#8723)
A comment of `@qwen-code /review` plus a newline and a body has never
triggered anything. The shape match tried to accept it with
startsWith(body, format('@qwen-code /review{0}', '\n'))
but GitHub expression string literals are NOT escape-processed: that
'\n' is a literal backslash + n, so the branch matched nothing. The
command was silently ignored — no run, no feedback, in a path whose
whole job is to be the manual escape hatch.
Measured on a live runner rather than assumed:
startsWith(<LF body>, format(…, '\n')) => false
startsWith(<LF body>, format(…, fromJSON('"\n"'))) => true
startsWith(<CRLF body>, format(…, fromJSON('"\n"'))) => false
startsWith(<CRLF body>, format(…, fromJSON('"\r"'))) => true
fromJSON parses JSON, which IS escape-processed, so it yields a real
newline. Both endings are needed: the REST API sends LF, the web UI
sends CRLF, and an LF pattern does not match a CRLF body. Applied to all
7 shape matches (6 /review, 1 /resolve).
The shell half had the matching gap: the command line is taken as
everything before the first LF, which on CRLF keeps a trailing CR. IFS
has no CR, so word splitting produced tokens like `--timeout=300<CR>`
that failed the numeric check with no visible cause. Strip it.
Three tests pin this: no shape match may use a non-escape-processed
literal, every shape match must carry both endings, and the CR strip
must follow the first-line split. All three fail against the workflow on
main. Mutation-tested: dropping the CR branches or the CR strip each
fails exactly its own test.
Found while re-triggering the PRs stranded by the #8648 outage — 19
multi-line trigger comments were accepted by `authorize`, then silently
dropped by this branch.
Co-authored-by: verify <verify@local>
|
||
|
|
ee98f7420b
|
perf(ci): make the autofix fleet caps operator-tunable and raise them (#8731)
The three caps that bound the autofix review loop were compiled-in literals sized for a much smaller takeover pool, so growing the pool meant editing the workflow, opening a PR and waiting for review every time. The takeover pool is not static — it grew to 37 PRs by 2026-08-08, and it keeps growing. Back all three with repository variables, keeping the literals as fallbacks, so the loop is resized in Settings → Variables with no code change: QWEN_AUTOFIX_MAX_PARALLEL (fallback 20, was 5) QWEN_AUTOFIX_MAX_TARGETS_PER_SCAN (fallback 30, was 10) QWEN_AUTOFIX_MAX_CANDIDATE_INSPECTIONS (fallback 60, unchanged) Verified on a live runner that `max-parallel` accepts the expression and schedules by it — a 6-leg matrix resolving to 3 started exactly 3 legs and began the 4th only after a slot freed. Not assumed: an invalid expression here makes the whole file invalid, which this repository just paid 12.9 hours of dead review automation for. The raised fallbacks are sized against measurements, not guesses. At 5 slots the fleet served ~14% of the takeover pool at once, reproducing at a larger scale the 81-minute tail measured back at 3. The ecs-qwen fleet is 84 runners, so 20 concurrent legs take under a quarter of it, and the legs sampled that day finished in 3-28 minutes. Worst case rises to 100 runner-hours across the fleet (20 slots x the 300-minute job cap), and per-PR head-write concurrency groups are per-PR, so this adds no push contention. MAX_TARGETS_PER_SCAN has to stay above max-parallel or the scan cannot emit enough legs to fill the matrix. That relation is pinned for the fallbacks by an existing test and stated at both definitions for the variables, where it becomes an operator invariant. Mutation-tested, 4 of 4 caught: fallback equal to the budget, fallback above it, and dropping either variable back to a literal. Co-authored-by: verify <verify@local> |
||
|
|
6cae50c7ea
|
fix(ci): keep the review workflow under the expression-length limit (#8720)
The review workflow has been invalid since #8648 merged, so every event it declares has been dead for ~12 hours: Invalid workflow file: .github/workflows/qwen-code-pr-review.yml#L1 (Line: 751, Col: 14): Exceeded max expression length 21000 A `run:` body containing `${{ }}` is evaluated as ONE expression template, and GitHub caps a single expression at 21000 characters. "Run review" went 17705 -> 22282 chars in #8648 (17:00:32 on 2026-08-07); the first startup failure is stamped 17:00:50. #8683 took it to 24042. An over-limit expression does not fail a job — it invalidates the whole file, so no run is created at all. Across the 400 runs since that merge there is not one success, not one `pull_request_target` and not one `issue_comment`: both automatic review and `@qwen-code /review` were unreachable, while CI stayed green throughout because nothing covered it. Pass the three context values the script reads through the step's env, leaving the body free of `${{ }}`. The runner then never templates it and its length stops mattering. No behaviour changes: each substitution is a rename of the same value. Pin it with a test that walks every workflow and fails any templated run block over the limit, plus one that keeps this body untemplated — it is past 21000 on its own, so a single `${{ }}` added back takes the whole workflow down again. Both fail against the file currently on main, naming it: `qwen-code-pr-review.yml > review-pr > Run review: 24042 chars`. Mutation-tested, 4 of 4 caught: restoring a `${{ }}` in the body (3 tests), dropping either env binding, and hardcoding the value the env used to carry. Co-authored-by: verify <verify@local> |
||
|
|
ee2e5be666
|
fix(review): stop the agent transcript from executing workflow commands (#8683)
* fix(review): stop the agent transcript from executing workflow commands The review agent streams its entire transcript to stdout, and the runner scans every line for workflow commands. A tool result that quotes a file containing one therefore gets EXECUTED. Observed on run 31167034020 (PR #8681). That PR changes an `actions/setup-node` input, so the agent read the action's own main.ts, which legitimately contains: core.info(`##[add-matcher]${path.join(matchersPath, 'tsc.json')}`); The runner took the rest of the JSON line as a matcher path: ##[error]Unable to process command '{"type":"user","uuid":...' successfully. ##[error]The path '...' is too long, or a component of the specified path is too long. Three of those, and the step failed after 1h37m — a full review discarded for quoting a file. Nothing about that PR is unusual: any review whose transcript quotes `##[...]` or `::...::` breaks the same way, including every review of this repository's own workflows. Wrap the agent invocation in `::stop-commands::`, with a token randomised per attempt so no output the agent produces can guess it and re-enable parsing early. Parsing resumes immediately after PIPESTATUS is captured: `echo` clobbers PIPESTATUS, so resuming any earlier would read the echo's status instead of the agent's and report every timeout or crash as a clean run. Resuming is on the errexit-disabled straight line, so it is reached on success, crash and timeout alike — leaving it off would silence the job's own ::error:: and the fallback comment's diagnostics for the rest of the run. Tested by driving the real extracted retry loop with a stub agent that emits `##[add-matcher]`, asserting the bracket contains it, that the token is random rather than fixed, and that parsing resumes on success, hard exit and timeout. Mutation-checked: removing the guard, never resuming, resuming before the status capture, and using a fixed token each fail. * fix(review): resume workflow commands on a line the runner can see Round-2 review follow-ups on the stop-commands guard. The resume was `echo`d, so a `--kill-after` SIGKILL that cut the agent off mid-line appended it to that fragment. The runner matches `::cmd::` at a line start only, so parsing stayed off for the rest of the job — losing the retry `:⚠️:` and every later diagnostic on the one path the guard exists to survive. Emit it with a leading newline. The ordering assertions had no teeth: `indexOf` returns -1 when a line is deleted or reworded, and -1 satisfies `toBeLessThan`. Deleting the stop line left the suite green. Every anchor is now asserted present. Cover the outcomes no scenario reached: an agent that streams and then dies (the stub `timeout` exited before ever running it), a failing log write (the only early return left unpinned), and a retry, which pins the bracket as per-attempt with a token the previous attempt cannot reuse. Mutation-tested, 7 of 7 caught: reverting the printf, moving the resume past the tee check or before the PIPESTATUS capture, hoisting the bracket out of the function, fixing the token, and deleting either end. --------- Co-authored-by: verify <verify@local> |
||
|
|
7b7ff19bc3
|
fix(tests): avoid blocking integration test cleanup (#8688)
* fix(tests): avoid blocking integration test cleanup Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tests): avoid failing fast on telemetry waits for live CLIs (#8688) * fix(tests): drop dead telemetry-ready return and gate rig tests (#8688) * fix(tests): pin the gated rig test in the no-AK guard (#8688) --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> 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> |
||
|
|
7942197666
|
fix(tests): apply integration worker limits to forks (#8689)
* fix(tests): apply integration worker limits to forks Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tests): keep no-AK integration gate at two fork workers (#8689) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-ci-bot <25325202+qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
f4802031d0
|
perf(ci): run docs-only automatic reviews at medium effort (#8648)
* perf(ci): run docs-only automatic reviews at medium effort
A 1-line docs PR costs the same 57-180 minute high-effort review as a code
change, and on a diff with zero source lines the passes medium drops - the
adversarial personas and the reverse audit - have no failure mode to hunt.
Counterfactual analysis over six dissected CI runs showed the one case
where those passes caught a real Critical was a source PR, which this gate
never touches: classification reuses the Test workflow's conservative
classify-profile.mjs (docs/**.md(x) + root prose only; markdown under any
src/ tree stays full, matching the review skill's own source rule), and
any fetch or classifier failure falls back to the full review.
Only the automatic pull_request_target review downgrades; every explicit
request (workflow_dispatch, @qwen-code /review) keeps full high effort.
Because an effective --comment forces high and medium never posts, the
downgrade drops --comment and a new step relays the review CLI's verbatim
"Review complete:" line - its machine-readable completion contract - as a
single PR comment, with a pointer for requesting the full review. The
docs-only budget is the size-aware timeout halved with a 90-minute floor.
* perf(ci): address review feedback on the docs-only medium gate
All nine review suggestions, each verified before fixing:
- review_requested is an explicit ask: the AUTO_REVIEW flag now excludes
that action (authorize write-permission-checks its requester), so a
maintainer requesting the bot's review gets the full high-effort run.
- The fetch-and-classify wrapper is extracted to
.github/scripts/ci/classify-pr-profile.sh and consumed by both ci.yml
and the review gate, so the classifier's input contract lives in one
place; distinct exit codes preserve each caller's fallback messages.
- Neither completion-line fallback mints the reserved "Review complete: "
prefix anymore, and the relayed line passes a strict not-posted
disposition allowlist - on this never-posts path any posted-form
disposition is false by definition (the measured phantom APPROVE
posted), so it falls back to a neutral non-scrapable form.
- The relay upserts by its marker (mirroring the queued-acknowledgement
step) instead of stacking a comment per push, retries the POST/PATCH
three times, and never fails the job - a failed relay after a
successful review must not trip the failure fallback into announcing
a review failure that never happened.
- The Chinese relay copy no longer parses as "发行" and renders
high-effort as 高强度 rather than 高档.
- The qwen-review docs-only-medium marker is registered in all six
BOT_COMMENT_FILTER sites in qwen-autofix.yml, so clean docs-only
relays cannot select PRs into autofix rounds as actionable feedback.
- The gate's behavioral invariants are pinned in
scripts/tests/qwen-pr-review-workflow.test.js by executing the
extracted bash: prompt-branch order (--effort medium instead of
--comment), the halve-with-90-minute-floor arithmetic, the
completion-line allowlist including the phantom shapes, AUTO_REVIEW
exclusivity, the six-site marker registration, and the shared-wrapper
routing in both workflows.
* perf(ci): harden the docs-only gate against round-2 review findings
Thirteen findings across two review passes; every fix is executed by a
test rather than asserted as text where the finding was behavioral.
- The relay marker exclusion in qwen-autofix.yml is author-scoped at all
six filter sites: only the relay bot's own marker comment is filtered,
so a human quoting the marker stays actionable feedback.
- classify-pr-profile.sh guards the 3,000-file listing cap (any mismatch
against the PR's declared changed_files classifies full), uses
mktemp+trap instead of a fixed path on the shared persistent pool, and
ships its own node:test suite (renamed source→docs pins the projection
contract; exit codes 2/3 pinned) registered in HELPER_TESTS.
- classify-profile.mjs restricts reserved root prose basenames to inert
extensions - README.js / SECURITY.ts / LICENSE.sh classify full.
- The completion-line allowlist binds to pr-<number> and to the only
verdict a medium run can produce (Comment, not posted) - a stale line
for another PR or an Approve-shaped injection falls back to neutral.
- A dedicated review_completed output gates the relay: the state/head
guards exit 0 without running the review, and outcome==success alone
would have announced a review that never ran.
- The relay upsert filters by the authenticated bot login, re-resolves
the comment id on every attempt, and falls back to POST when the PATCH
target is gone - a participant posting the marker can no longer capture
the upsert, a transient listing failure no longer mints duplicates.
- The gate and relay are now executed under stubbed executables in
qwen-pr-review-workflow.test.js (docs_only/full/failure/explicit
scenarios; POST/PATCH/never-fail branches), the AUTO_REVIEW pin covers
both guard halves, and the marker contract is pinned producer-side and
filter-side.
* perf(ci): fix the medium Request-changes swallow and the stale docs badge
Round-3 review findings (2 Critical, 8 test-gap Suggestions), each fix
executed by a test where the finding was behavioral:
- The completion-line allowlist accepts `Request changes, not posted` -
compose-review caps only Approve at medium, so a docs-only run that
verifies a Critical legitimately emits Request changes, and the old
Comment-only allowlist swallowed exactly the blocker-finding outcome
into the neutral fallback. Target binding to pr-<number> is unchanged
and now pinned by a test, as is the last-line selection over a stale or
injected earlier completion line.
- A stale docs-only badge can no longer outlive its revision: the full
automatic review path now supersedes the bot-authored marker comment
(strikethrough + superseded note) via a new --update-only mode that
never mints a badge where none existed.
- The marker+author upsert protocol is extracted to
.github/scripts/upsert-bot-comment.sh - one implementation shared by
the relay and the supersede step (the per-step copies had already
drifted), with its own node:test suite covering the author scope, the
per-attempt re-resolution (deleted-mid-retry falls back to POST), and
the --update-only no-op; registered in HELPER_TESTS.
- The classify-pr-profile gh stub now applies the wrapper's own --jq
argument with real jq over API-shaped fixtures, so the projection
contract is genuinely under test (negative control: dropping `status`
turns the renamed-source scenario red).
- New pins: review_completed wiring end to end (run-step emit + both
consumers' if clauses), the auto_review output->env wiring at both
links, and both AUTO_REVIEW guard halves.
* perf(ci): never let a failed lookup mint or keep a stale docs badge
Round-4 review findings (1 Critical, 7 Suggestions):
- The upsert script no longer conflates failed lookups with empty
results: the authenticated login, the listing, and the jq extraction
are all resolved inside the retry loop as one prerequisite chain, an
attempt whose prerequisites failed retries instead of falling through
to POST (the shape that minted a permanent duplicate badge off one
transient 5xx), and --update-only exits 1 on a failed lookup so the
supersede warning fires instead of a false no-op success. New tests
pin the failed-listing-then-PATCH path, the persistent identity
failure, the update-only failure exit, and the update-only PATCH.
- Supersede now covers every path that owes the correction: a FAILED
full review and an EXPLICIT requested review (the badge's own CTA)
both retire the badge, gated only on docs_only_medium == 'false' -
empty on runs that failed before classifying, so a badge is never
superseded on ignorance. The body is cause-neutral: it asserts only
that the badge described an earlier revision.
- The marker literal is defined once per step (MARKER variable, the
qwen-triage convention) and shared between body and lookup argument;
a pin requires the definition and --update-only on the supersede
invocation.
- New behavioral pins: the Approve verdict stays rejected by the
allowlist, github_ci_only never downgrades (CI helpers are
executable), and review_completed's emit position is asserted AFTER
the closed-PR and stale-head guards (the hoist mutant survived
position-independent contains checks).
* perf(ci): make docs_only_medium three-valued and pin the untested guards
Round-5 review findings (1 Critical, 6 Suggestions):
- docs_only_medium no longer conflates "determined not docs-only" with
"never determined": the output is three-valued ('' when the
classification failed or never ran), so a transient classifier failure
or a dispatch dry-run can no longer retire a still-accurate badge. The
supersede condition names its two licensed paths explicitly - a
POSITIVE not-docs-only determination (without requiring review
success), or an explicit comment-mode review that completed (the
badge's CTA; report-mode dry runs retire nothing).
- The count-mismatch fallback in classify-pr-profile.sh logs to stderr,
so a systematic divergence is distinguishable from every PR genuinely
classifying full.
- Six probed surviving mutants now each turn a test red: the supersede
body is executed (existing-badge PATCH and the never-fail guard),
ci.yml's rc-handling fragment is executed (exit 0/2/3 with the
full fallback), the changed_files fetch failure exits 2, duplicate
badges PATCH the last (newest) comment, and the relay's POSTed body
must carry the marker that keys both the upsert and the supersede.
* perf(ci): bind the docs badge to the reviewed head and retire it on failure
MDX pages are executable (imported components, expressions), so the
classifier no longer treats them as inert docs-only changes. The relay
and supersede writes re-read the live PR state/head immediately before
the mutation and skip unless the PR is still open at the reviewed SHA,
the badge body names that SHA, a failed docs-only review now retires the
singleton badge instead of leaving the previous revision's outcome
visible, and the retired wording is cause-neutral (an explicit review
can complete on the very head the badge describes).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: verify <verify@local>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
|
||
|
|
20b9504276
|
feat(autofix): bridge fork-PR reviews into the credentialed review lane (#8676)
* feat(autofix): bridge fork-PR reviews into the credentialed review lane
A workflow run tied to a pull request whose head lives in a fork is
created with `Secret source: None`, so `secrets.CI_DEV_BOT_PAT` is empty
and the autofix review lane cannot authenticate. Those reviews therefore
wait for the scheduled scan, which GitHub throttles hard — the `*/10`
cron lands every 40-70 minutes on this repo.
Reading the PR with `github.token` instead would not help: review-address
still needs the PAT to push and comment, and it is empty for the whole
run. The credential has to come from a different run.
This is the standard `workflow_run` bridge for that.
- `qwen-autofix-fork-signal.yml` runs on `pull_request_review`, where the
fork PR's event can reach it, and records only which PR was reviewed.
It is deliberately powerless: `permissions: {}`, no checkout, no
repository code, and the hosted pool rather than the persistent
self-hosted one.
- `qwen-autofix-fork-bridge.yml` runs on `workflow_run`, which GitHub
creates on the default branch with the repository's own token, and
dispatches `qwen-autofix.yml` for that PR using `actions: write` — the
same lever Fleet Shepherd already pulls. It holds no PAT.
The dispatched run re-derives admission from live API state, so the PR
number says when to look, never who may be touched. It is bound to
`workflow_run.head_sha` regardless: the signal run carries the reviewed
PR's head SHA, and only that PR has that head.
Measured, not assumed:
- `workflow_run.pull_requests` is empty for fork PRs, and
`/commits/{sha}/pulls` does not resolve a fork head either (it does
resolve an in-repo one), so the artifact is the only way to learn the
number.
- `pull_request_review` runs the workflow file from the BASE branch: run
31152873061's PR branch predates a main-only change to
`qwen-autofix.yml` and it still executed the new file. A fork cannot
edit what the signal writes.
- That run's `head_sha` equals PR #8436's `headRefOid` exactly, which is
what makes the binding possible.
The bridge's validate-and-dispatch block is replayed under bash against a
stub API: forgery, an unreadable head, both heads empty, non-numeric and
path-shaped artifacts, a closed or retargeted PR, download and read
failures, and dispatch retry then exhaustion.
* fix(autofix): align fork bridge with route admission and coalesce bridged dispatches
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(autofix): harden the fork-review bridge per review feedback (#8676)
---------
Co-authored-by: verify <verify@local>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
|
||
|
|
188802719b
|
fix(ci): give review runs upstream-timeout headroom (#8673)
* fix(ci): give review runs upstream-timeout headroom PR 8507's automatic review aborted twice on 2026-08-07 with "Request timeout after 483s" - a classification message printing elapsed time, not a configured threshold. Two distinct guards fired under a degraded upstream: the 17-agent fan-out generation on a ~1.27M-token context stalled chunk delivery past the 240s stream-idle default (the pipeline-level retry doubled the loss to ~16 minutes), and on the fresh retry an early small turn burned three ~120s connect/TTFB timeouts through the SDK's internal retries. Three knobs, applied to CI review runs only: - model.generationConfig.timeout 600000 via the per-run QWEN_HOME settings.json (no env knob exists for the SDK request timeout); - QWEN_STREAM_IDLE_TIMEOUT_MS 600000, tolerating a long thinking phase between chunks; - QWEN_STREAM_MAX_LIFETIME_MS 1800000, keeping the drip-feed hard bound strictly above the idle window. The outer GNU timeout and the review deadline still bound the job; the knobs trade hang-detection latency for survival on slow-but-alive upstreams. A test pins all three and the lifetime>idle ordering. * fix(ci): deliver review timeout headroom via step env (#8673) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): pin stream-guard env values and bound stream-retry trade-off (#8673) --------- Co-authored-by: verify <verify@local> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
6668c876a6
|
perf(autofix): stop restoring a 2.65 GB npm cache to protect a 29 s install (#8681)
* perf(autofix): stop restoring a 2.65 GB npm cache to protect a 29 s install `actions/setup-node` is configured with `cache: 'npm'` in all three heavy autofix jobs. Measured on one review-address leg, that step took 339s: Node itself was free (`Found in cache @ .../_tool/node/22.23.2/x64`) and 2,654,052,865 bytes arrived at ~10 MB/s. The `npm ci` it protects ran in 29s in the very next step. The bill is per job, not per run: build-cli pays it once (280s measured), issue-autofix once, and EVERY review-address leg once — up to ten legs a scan, five at a time. Observed leg costs ranged from 446s to 1207s. The persistent pool keeps ~/.npm across jobs, so the download buys nothing there. The hosted fallback is ephemeral and still wants it, so the cache is chosen from `runner.environment` rather than from a copy of the runs-on expression, which differs per job and would drift. Also narrows an existing negative pin. `expect(workflow).not.toContain( "runner.environment == 'self-hosted'")` was added to keep the reverted dedicated-runner design out, whose artefact was a `command -v node` step gated on exactly that expression (removed in #6261). That step is pinned out by name on the following line, so the substring form only forbade the `runner` context by accident — the same test requires `RUNNER_ENVIRONMENT: '${{ runner.environment }}'` a few lines below. It now matches the shape that was actually reverted: a step whose entire `if:` is that expression. Mutation-checked: re-adding that step still fails the test. * fix(autofix): actually disable the npm cache restore on the persistent pool --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
18b925103a
|
fix(triage): finalize the status comment on cancellation too (#8436)
* fix(triage): finalize the status comment on cancellation too The 'Finalize triage status comment' step ran under success() || failure(), so a job-timeout (timeout-minutes: 30, reachable on large PRs) or a manual cancel skipped it and the early '🔄 Qwen Triage is running' lifecycle comment kept claiming the run was in progress forever — the verify lane has publish-verify to catch its cancelled case, but the triage lane has no other publisher. Run the step under always() (still gated on a resolved number), branch on job.status to say the run was cancelled rather than 'ended early', and point the reader at re-running. The step body is already best-effort throughout, so widening the condition cannot fail the job. * test(triage): pin the cancelled finalize condition in the shape test * fix(triage): finalize status comment only on this run's own comment The cancelled finalize PATCHes whatever lifecycle comment the marker lookup finds. When a run is cancelled before its own status claim posts, the lookup lands on a previous run's comment — including the terminal verdict wording written by qwen-triage-finalize.yml, which shares the marker — and rewrites it to "was cancelled". Only PATCH a comment this run owns: the claim embeds the run link, so ownership is observable as the found body containing $RUN_URL. A found but foreign comment is left untouched; a missing one is still POSTed. Also pin what the shape test could not: the cancelled branch's Chinese wording, and an executed-composer test that runs the step's script against a stubbed gh to assert the posted body for each (TRIAGE_OUTCOME, JOB_STATUS) combination plus the ownership cases — swapping the success and cancelled bodies no longer survives the suite. * test(triage): pin the last-comment selector in the finalize ownership lookup Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(triage): harden finalize ownership pins and document lane race Round-4 review feedback: - Negatively assert the ZH terminal wordings too, so a composer body mixing two terminal states cannot pass the combos loop. - Cross-pin the claim and finalize steps' RUN_URL env definitions, which the ownership contract couples; one-sided drift would otherwise make finalize classify its own claim as foreign. - Document the accepted lane race with qwen-triage-finalize.yml's update_status writer at the ownership rule. * fix(triage): bound finalize ownership match to the exact run link * fix(triage): never strand a running claim in the finalize step A foreign lifecycle comment made finalize skip every write, so a claim whose POST had failed — or a legacy marker body that carries no run URL — stayed at "running" forever. Post a fresh terminal comment for unowned markers instead; the rare lane race costs one extra comment but the thread always ends in a terminal state. The marker lookup also picked the newest comment before asking who owns it, letting a newer foreign comment mask this run's own claim. Select the last comment this run owns so the classifier is order-independent, and treat an unreadable comment list as its own state (warn and skip) instead of conflating it with "no comment" and posting a duplicate. The success wording is now gated on the job not failing: 'Check triage response' exits 1 on an empty summary while steps.triage.outcome stays 'success'. The behavioral test ran the step body without errexit even though GitHub adds -eo pipefail, so such regressions stayed green; align the harness with the real shell flags and add the missing {success, failure} combo. Also reword the cancelled message, which prescribed a re-run the run cannot know is necessary — cancellation is usually cancel-in-progress supersession — and switch the two Chinese strings to full-width punctuation per the file's convention. * fix(triage): reclaim stranded running claims in the claim step (#8436) The finalize foreign arm posts a second lifecycle marker, and once two markers exist the claim step's newest-wins pick never touches an older stranded "running" claim again — the base tree healed those by overwriting its single slot. Select this run's own marker first (a re-run keeps its run_id), then the oldest still-running marker, then the newest. Also reword the stale "no second post" comments to the ownership semantics, soften the finalize invariant to its best-effort boundary, and pin the previously untested behavior: one write-failure scenario per finalize write arm, the empty-vs-foreign diagnostic, the legacy marker path, and the last-own-marker rerun selection. * fix(triage): keep the finalize status flip off live running claims (#8436) qwen-triage-finalize.yml's update_status is the third writer of the lifecycle marker and still picked the newest one unconditionally. When it fires while a triage run is in flight, that newest marker is the run's live "running" claim: PATCHing it erases the run URL the run's finalize keys on, so the run classifies foreign and posts a second comment, while the overwritten one — never the newest again and no longer "running" for the claim step's reclaim — strands forever. Prefer the newest marker that is not a live running claim, falling back to the base newest-wins overwrite only when running markers are all the thread has. Pin the selector with an executed test. Also give the claim step's marker selection the one-line stderr diagnostic the finalize step's arms already have, so a maintainer investigating "the bot overwrote the wrong comment" can see which arm fired and which id was chosen without refetching the comment list. The two lifecycle harnesses each pasted their own gh-stub + runner and had already diverged (only the finalize stub had the failure arms), so a contract fix would have to land twice or silently degrade one test. Extract one shared makeGhHarness, and use the arms it brings to pin the claim step's previously untestable guards: one write-failure scenario per `|| echo` arm, the `|| EXISTING_ID=''` list-failure fallback, the legacy marker slot, the last-own-marker rerun selection, and the claim body starting with the lifecycle marker every selector is gated on. Also pin the finalize composer's `[view run]($RUN_URL)` embedding, the finalize-side twin of the claim-side coupling already pinned above it. * fix(triage): key the status finalize on the claim's exported comment id (#8436) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(triage): pin claim --jq '.id' extraction and document finalize empty-id loss (#8436) --------- 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> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
63a99c2c11
|
fix(autofix): decline fork PR reviews instead of routing them to a scan that cannot authenticate (#8671)
GitHub hands no repository secrets to a workflow run tied to a pull request whose head lives in a fork — the run header reads `Secret source: None` — so `secrets.CI_DEV_BOT_PAT` arrives empty and every `gh` call made with it is unauthenticated. Route admitted those PRs anyway. The real-time fork branch spent two API reads deciding it, then review-scan spent three more failing and exited 1 on `metadata_fetch_failed` — a reason whose blocked comment promises "a later scheduled scan will retry", true for a 5xx and false for a credential the run was never handed. Every review of a fork PR reddened the workflow while changing nothing, and that noise buried the failures that do need a human. The branch's comment explained the admission by saying the event "runs in BASE-repo context". That holds for the workflow FILE, which is read from base, but not for the credentials. So decline in route, exactly as the `pull_request` label branch already does for the same reason. Nothing that functioned is lost: the admitted path could never authenticate, and the scheduled scan admits fork takeover PRs in repo context on its own. Real-time pickup for fork PRs needs a credentialed lane, which a `workflow_run` bridge provides separately. Chosen over gating the PAT-backed jobs on a route output: that reached the same outcome through a new output, two job conditions and a concurrency expression that hand-mirrored them, while route already declines this class of event one branch away and the two API reads still got paid for. Also add a hard guard on an empty PAT at the top of review-scan. No job `if:` can read the `secrets` context, so a deleted or renamed secret — or a lane nobody has modelled — is invisible until the step looks, and it must stop there: unauthenticated reads answer as if the repository held no PRs, which the scan would report as a healthy fleet of zero and stay green while the loop is dead. Tests: the fork-admission replay becomes a fork-decline replay over the same shapes (including the two the removed branch existed to admit), and pins that the decline costs no API call; the new guard is replayed under bash with a present-PAT negative control. Co-authored-by: verify <verify@local> |
||
|
|
5fdcdb28e2
|
fix(ci): avoid root-owned npm cache workspace files (#8669) | ||
|
|
7edc16ba11
|
feat(review): say so when the bundle is older than the review it runs (#8390)
* feat(review): say so when the bundle is older than the review it runs Every `qwen review …` step runs the BUILT bundle, not the working tree. So editing a review command, or switching to a branch that contains one, changes nothing about the run until someone rebuilds -- and the failure is silent and total: the run behaves like the last build, and every conclusion drawn from it is a conclusion about that build. Measured on 2026-08-02, dogfooding /review against #8368 from a checkout whose bundle was fourteen hours old. Three things were invalidated at once and none announced itself: `drive` and `mock-provider` had merged that morning and were absent from the binary, so "the agent never reached for them" measured nothing; and #8345's guard against scoring a mutant `survived` when its own collocated test was red had merged too, so the run reproduced the bug it fixed and filed three findings the current code holds as `inconclusive`. The round was discarded and re-run after a rebuild. `parse-args` is the first command of every review, which makes it the only place a notice reaches a reader before they act on a result. It names the file that is ahead, by how much, what actually runs from the bundle, and the command to rebuild -- "rebuild" without evidence is advice nobody can check. mtime, not git: the question is whether this bundle was built from this source, and a git comparison answers a different one. A margin absorbs a checkout, which writes everything at once in no guaranteed order. An installed package has no sources beside it, finds nothing to compare, and stays silent -- a check that cannot see the files must not accuse the build. Also documents `findings --test-delta` for users: it can lower a severity, and therefore change what the verdict is computed from, so it belongs beside `--outcomes` rather than only in the skill. * fix(review): watch the file every subcommand is registered in `packages/cli/src/commands/review.ts` is where all 30-odd subcommands are imported and registered, and it sits beside the directory rather than in it -- so a new command, or a changed dispatch, was exactly the change this check could not see. A root may now be a single file, which is what that one is. Confirmed end to end: with `review.ts` three hours ahead of a fresh bundle, the warning names it. Also two comments that did not match the code: symlinks of every kind are skipped, not only directories (`isFile()` is false for a symlinked file too), and the module now says what `QWEN_CODE_CLI` already covers -- talking to a different program -- so it is clear this guards the other half, the right program built before the change. * fix(review): compare content, because a timestamp check cried wolf The first version compared the bundle's mtime against the newest review source, and it was wrong in the direction that matters most. `git checkout` rewrites every file that differs between two commits, so returning to the branch a bundle was built from re-stamps exactly those files and the check calls a byte-for-byte correct bundle stale. Measured: with the sources untouched and the bundle two minutes older, it warned. A line that fires when nothing is wrong teaches its reader to skip the line, which would have made this worse than absent. The build now stamps a digest of the review sources it bundled into `dist/review-sources.sha256`, and the check re-derives that digest from the tree and compares. No margin to tune, no clock to trust, and no answer but the true one. Verified end to end across all five cases: a clean tree is silent, a source touched but unchanged is silent, and a real change under any of the three roots -- the command directory, the `review.ts` that registers them, the bundled skill -- warns. The digest is now one rule stated twice, since the build script cannot import the package it runs before building. `scripts/tests/review-source-digest.test.ts` holds the two equal, on this repo and on a synthetic tree that exercises the file-shaped root; a package test may not reach into `scripts/`, so it lives on the side of the boundary that may. Paths are folded relative to the repo root with separators normalised, and the file list is sorted -- `readdir` order is a property of the filesystem, so without it a bundle built in CI and a tree cloned locally would hash the same source differently and every run would warn. * fix(review): a diagnostic must not kill the run, and tests are not the bundle Two Criticals and five suggestions from review, all verified before changing anything. `writeStderrLine` throws on EPIPE, so stderr piped to `head` would have killed the review before it parsed a single argument -- a warning that destroys the run it was warning about, and the opposite of this change's own invariant. `writeStderrLineSafe` is the convention for diagnostics in this subsystem and is what it calls now. `reviewSourceRoots` builds paths with the platform `join`, and the test asserted forward-slash literals, so all three elements would have failed on the merge queue's Windows leg -- which the pull_request event never runs, so the green CI here proved nothing about it. Test files left the digest. esbuild follows imports from the CLI entry and no test is reachable that way, so folding them in fired the warning for an edit that cannot change a byte of the bundle -- the false positive this module already rejected once. 112 files became 61, and a test-only edit is now silent while a production one still warns. The handler wiring is tested at last, against a real temp tree rather than a mock of the reads under test: the derivation from `process.argv[1]`, the stamp read, and the warning. All three mutations the review named -- dropping the call, reading the stamp from the wrong directory, collapsing repoRoot to distDir -- now redden it. Also: the stamp's filename is pinned across the boundary it crosses (the build wrote a literal while the check read `DIGEST_FILE`, so a one-sided rename would have silenced the feature with every test green); the digest is computed only when there is a stamp to compare it against, instead of hashing a hundred files for a value the first guard discards; the `rebuildCommand` parameter no caller ever set is gone; and the build script's comment no longer claims a code-sharing relationship that does not exist. * fix(review): fixtures are not in the bundle either The same false positive, a third time and one directory over. Excluding tests from the digest was right and incomplete: `review/__fixtures__` holds four files — three responder modules and a captured comment — that a test loads at runtime, from no import the bundler follows. Measured against `dist`: none of the four appears in it, so editing one changed the digest while the bundle stayed byte-identical and the warning claimed a review command had changed. Both walks skip the directory now, and the parity test's synthetic tree grows a fixture and a `.spec.tsx` so the two implementations are held equal on the whole exclusion, not just the part the first case exercised. Reverting one side reddens the local case AND both parity cases, which is what that guard is for. Verified the other direction too, since an exclusion can overshoot: every review source that reaches `dist` is still covered. `DESIGN.md` and `SKILL.md` both ship and both remain in the digest — checked, not assumed, after two rounds of this exact mistake. Six cases end to end after a rebuild: a clean tree, a test edit and a fixture edit are silent; a production edit, a `review.ts` edit and a `DESIGN.md` edit each warn. * fix(review): allowlist the stamp, and stop guessing what the bundle holds The Critical first: `create-standalone-package.js` fails on any top-level dist entry outside its allowlist, and `review-sources.sha256` was on neither list. The next release would have aborted the standalone archive on all five targets, and no PR-time job runs the packager, which is why this suite is green. Allowlisted -- shipping it is harmless, since a standalone install has no `packages/` to compare against and the check stays silent there. `lib/test-utils.ts` was in the digest: test support with a production-looking name, imported by two test files and nothing else. That is the fourth patch to one rule -- `.test.ts`, then `__fixtures__/`, then this, plus `.DS_Store` -- and each was found by a reviewer after it shipped. So the rule stops being a list somebody remembers to extend: a new test asserts the property the list approximates, that every file the digest folds in is reachable from production code and nothing reachable is left out. Dropping `test-utils.ts` from the exclusion reddens it, which is the fifth instance failing in CI instead of in a review. Three branches that no test reached, each with a mutant the review measured surviving the whole suite: the walk's symlink skip (a directory cycle would send the first command of every review into unbounded recursion), the read-failure path (hashing the survivors of a concurrent checkout would accuse a tree that is merely mid-change), and the build's stamp call site (removing it left the scripts suite green while `npm run bundle` silently stopped writing the stamp). All three now redden. And `unmeasured` had no reader, so the one edge this check cannot measure but can see -- sources present, stamp absent -- passed in silence. That is the state of every existing checkout the moment this ships, and it is exactly the silent failure the change was written to end. It now says so, while an installed package, which has no sources either, still says nothing. * fix(review): the guard was shallower than the property it claimed The guard added last round asserts that every file in the digest is reachable from production code. It did not: a file imported by nothing passed, because the filter also required some test to import it; only `.ts` was inspected, so a test-only `.tsx` or `.mts` helper walked through; and it read static imports only, while this directory has nine `await import('./…')` edges. It asserts the property now — every extension, orphans included, dynamic edges seen — and the tree has no violators, so the strictness cost nothing today and is there for the next file. `__snapshots__` joins the exclusions. `vitest --update` regenerating a snapshot would have moved the digest with the bundle byte-identical; none exists under the review roots today only by chance, and 120 `toMatchSnapshot()` calls live elsewhere in this package. Three couplings that no test held: - the allowlist entry that fixed the release-breaking R2-1 -- reverting those five lines left the whole scripts suite green, and the next failure would have been a release aborting on all five targets. `isAllowedDistEntry` is exported and the stamp's own name is asserted against it, so a one-sided rename fails here instead; - the `.DS_Store` member of `NOT_BUNDLED_FILE`, absent from the repo and so from the parity tree -- one-sided removal stayed green while a macOS checkout would digest differently on the two sides forever; - each `unmeasured` reason. Swapping the two arguments at the single call site kept all 76 tests green while telling a pre-stamp checkout its sources were missing. And two comments that said the opposite of the code beneath them: the digest is computed unconditionally on purpose (the pre-stamp notice needs it), and `NOT_BUNDLED_FILE` helpers are deliberately not importers, since nothing reaches the bundle through a file the bundle does not contain. The two stderr diagnostics are documented for users, beside the sibling paragraph this PR already added. * fix(review): measure only the layout that can carry a stamp `npm start` launches `node <root>/packages/cli`, and node sets `argv[1]` to that directory -- so the derivation found sources under `<root>` with no stamp beside them and printed "could not check" on every review, forever, with advice that could never make it stop. That is the fires-when-nothing-is-wrong failure this change argues against, on the path `start.js` sets `QWEN_CODE_CLI` to precisely so reviews reach that build. Only a `<root>/dist/cli.js` layout is measured now; anything else has no stamp to find and no way to grow one. The build-side digest could kill `npm run bundle` where the check side degrades gracefully: a file vanishing mid-walk threw out of the hash loop, and the stamp is the copier's last step, so the build would fail with every asset already in place. Caught and skipped -- a missing stamp is `unmeasured`, which the runtime already treats as an acceptable answer. The skill now says what to do with the warning, which is the half that makes it reach a human: `parse-args` runs inside an agent's shell tool, the user reads the agent's summary rather than raw stderr, and a line nobody repeats is a line nobody sees -- which is how the 2026-08-02 round went wrong in the first place. It also records that the instruction cannot help the run that needs it, since the skill comes from the same bundle. And the scope is stated where silence could be over-read: the digest covers the review commands, the file that registers them, and the bundled skill -- not the shared helpers those import. A quiet run means the review code matches the bundle, not that the tree does. * fix(review): refuse to certify a bundle the copier may not describe The stamp described the tree as the COPIER saw it, and the copier runs after esbuild -- so a source edited in between, or `copy_bundle_assets.js` run on its own (it self-executes), wrote a digest certifying a `cli.js` built from something else. Silence then means "verified fresh" when it is not, and that is the only direction here where a quiet run is affirmatively wrong rather than merely uninformative: every other gap degrades to `unmeasured`. Timestamps are the wrong tool for judging staleness and the right one for judging whether this stamp can be honest at all, so the build refuses when any source is newer than the bundle it would attest to, and says why. Driven for real: touching a review source and running the copier alone now prints "skipped the source digest rather than certify a bundle it may not describe". `it('counts the same files')` compared nothing -- it asserted `> 50` on the build side while the check side exposes no count, so the title claimed a parity the body never checked, and the margin over the real 56 made it a future false alarm in `scripts/` for an unrelated change. Removed; the digest parity already holds the file set. "Root is a file" was inferred from `readdirSync` raising ENOTDIR, an assumption about every platform's libuv on the one root that is a file -- `review.ts`, where "a new subcommand was registered" lives. `statSync(root).isFile()` says it instead. And the check itself moves out of the handler into `bundleStalenessNotices`, which is where the rest of it already lived. `parse-args` is about parsing arguments again, the wording is testable without the yargs harness, and a second caller -- an agent resuming a review never runs step 1 -- is one line. * fix(review): align the twin walk, and stop a test from passing on nothing The build side still inferred "this root is a file" from `readdirSync` raising ENOTDIR, one commit after the check side stopped doing exactly that and said why. A platform that maps the case differently would drop `commands/review.ts` from one digest and not the other, and a byte-for-byte correct bundle would warn on every review forever, on that platform alone, with rebuilding reproducing the same one-sided walk. Both sides ask `statSync(...).isFile()` now. Fixing one half of a pair and not the other is the mistake this file keeps making. The filename parity test had been passing on nothing since the previous commit: it matched `writeFileSync(join(distDir, '…'))` against the script's source, the literal moved into a `stampPath` variable, and the regex returned `undefined` so the assertion compared against nothing. It runs the build against a fixture now and reads the name off `dist/`, so it measures what the build does instead of what its source looks like. Renaming the stamp on one side reddens it. Also from review: the duplicated comment block in `parse-args`; an unreadable source now says the check could not run rather than passing in the same silence as an installed package, which is what the docstring already promised; the "could not check" line no longer asserts that the checkout predates the feature, since the build has three refusal paths and one of them means the opposite; every refusal removes an existing stamp, because leaving an older attestation beside a newer bundle is a weaker form of the certifying it refuses; and `drive` calls the check, which the module comment argued for and the diff had not done -- a resumed review never runs step 1, and that is where the long work starts. * fix(review): pin the regex group the parity tree missed, and say source, not command * fix(review): allowlist what the bundle holds, and cover the drive notice (#8390) * fix(review): treat unreadable review sources as unmeasured (#8390) * test(review): pin the stamp guard mutations that survived the suite (#8390) * fix(review): close staleness-check gaps and pin the round-4 survivors (#8390) * fix(review): close round-5 staleness gaps for parity, refusals, and partial checkouts (#8390) * fix(review): close round-6 gaps in the clause classifier, symlink layout, and pin honesty (#8390) * fix(review): close round-7 gaps in the closure oracle, parity pin, and refusal pins (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-8 gaps from the maintainer review (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-9 nits from the maintainer review (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): pin the lease root in the synthetic digest parity case (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Autofix <autofix@qwen-code.dev> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
d5e47709a3
|
fix(ci): surface blocked autofix takeover admission (#8410)
Some checks are pending
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* fix(ci): surface blocked autofix takeover admission
* fix(ci): clarify blocked takeover retries
* fix(ci): retry blocked takeover reporting
* fix(ci): serialize forced takeover status writes
* fix(ci): report blocked managed PRs
* fix(autofix): treat terminal permission answers as a routine rejection
Two review blockers on the forced takeover admission gate.
R7-1: `read_live_permission` whitelisted only admin/maintain/write/triage/
read, so GitHub's definitive answers for "holds nothing here" never matched.
Bot-type logins (dependabot[bot], github-actions[bot], renovate[bot]) and org
logins return HTTP 200 with permission 'none'; nonexistent or empty logins
return 404. Both burned three API calls plus back-off and then returned
`permission_lookup_failed`, so the forced path exited 1 (a red run) instead of
the routine `author_permission_none` rejection, the blocked comment promised
"a later scheduled scan will retry" — a retry that can never succeed — and the
actionable "grant the fork author write access" guidance behind
author_permission_* was unreachable. `author_permission_${FPERM:-none}` could
never render `none`. The scheduled loop re-paid the same cost per candidate
per tick, permanently.
Accept 'none', answer HTTP 404 terminally, skip the call for an empty login,
and keep the retry budget for genuinely transient answers (5xx, network, auth)
so a legitimate write-holder is never silently rejected. gh's own stderr now
rides along in the warning instead of going to /dev/null — a rate limit, an
expired PAT and a 5xx were indistinguishable before.
R6-1: the new blocked-status comment lookup is the 14th `--paginate` code
site, but the deliberate site-count pin still asserted 13, failing
`Test (ubuntu-latest, Node 22.x)` deterministically. Bump it to 14 and record
why this site stays out of the `jq -s 'add // []'` normalizer: it consumes the
page stream inline via `--jq ... | .id` into `tail -1` and never lands in a
WORKDIR json file, so wrapping it in an array would break the tail-1 consumer.
R6-2: pin the forced-admission wiring (reader -> classifier -> live-permission
gate -> reporter) end to end, plus fixtures for none, 404, empty login, every
grant level, and a transient 5xx.
* fix(autofix): harden the blocked-takeover status write (#8410)
Addresses review round 2 on e49aadef.
- Status-comment lookup now uses the same jq filter as the sibling upsert in
'Post autofix status comment': `// ""` guards so one comment with a null
body cannot abort the program (verified: the old filter exits 5, so all
three attempts fail and the run reds out without posting the very status
it exists to post), and --arg for AUTOFIX_BOT so a repo-configured login
containing " or \ is a mismatch rather than a jq parse error.
- The lookup no longer sends gh's stderr to /dev/null, matching the rule
read_live_permission states 60 lines above.
- The blocked body resolves its run link from GITHUB_SERVER_URL like every
other status writer, instead of hardcoding github.com.
- All four reporter warnings go to stderr like the two reader helpers, so
the reporter stays safe to wrap in $( ).
Tests: the reporter and reader harnesses now run under production shell
options (`set -eo pipefail`) and call through `|| exit $?` so errexit is
suspended inside the helper exactly as the `if !` call sites do. New cases
cover the null-bodied comment page, the PAT-identity mismatch branch, the
in-repo pass-through (no collaborator call), and the permission_lookup_failed
red exit. Each kills a probe-verified mutant that previously survived:
dropping the `// ""` guard, dropping the `isCrossRepository == true`
conjunct, and flipping that `exit 1` to `exit 0`.
Verified: npm run test:scripts 946 passed / 9 skipped, 46 files;
prettier --check clean; eslint clean; git diff --check clean.
* fix(autofix): narrow the forced-scan lock to the review condition (#8410)
Review round 9 raised two Suggestion-level findings on the forced
admission path; both are addressed here.
R9-1 (.github/workflows/qwen-autofix.yml:1698) — the review-scan
concurrency group was broader than the job's own `if:`. GitHub
evaluates concurrency before the job condition, and `route` emits
`pr_number` unconditionally from the dispatch input, so a
`workflow_dispatch` with `phase: issue` + `pr_number: N` resolved the
group to `qwen-pr-head-write-N` even though `do_review` is false and
the job only skips. Because `issue-autofix` declares
`needs: ['route', 'review-scan']`, the skipped leg could queue behind
an in-flight address round for PR N (`timeout-minutes: 300`,
`cancel-in-progress: false`) and stall the issue phase the operator
actually dispatched, with only a "queued" badge to explain it. The
group now carries the same `do_review == 'true'` conjunct, so a run
that will skip falls to the per-run `github.run_id` group. The forced
path's serialization against review-address is unchanged.
R9-2 (scripts/tests/qwen-autofix-workflow.test.js:4034) — the pin
asserted both jobs' groups as independent literals and never compared
the two prefixes, so renaming one side while updating its literal in
the same block would ship green with the lost-update race on the
status comment silently reopened. The test now extracts both prefixes
and asserts equality, mirroring `groupOf` in
qwen-resolve-workflow.test.js, and a new case pins the group predicate
against the job's own `if:` block.
Verified: qwen-autofix / qwen-resolve / qwen-triage workflow suites
266/266 pass. Both new assertions were mutation-checked — reverting
the group to the broad predicate fails 2 tests, and renaming only
review-scan's prefix fails the equality assertion. Prettier and ESLint
clean (the group is double-quoted because the expression embeds
'true', which Prettier will not leave escaped).
* fix(autofix): carry pipefail on the blocked-status comment read (#8410)
Maintainer verification of the forced-admission path found the one `if`
in `report_forced_takeover_blocked` that tests a PIPELINE rather than a
single command:
if status_ids="$(gh api .../comments --paginate 2> "${err}" | jq -rs ...)"
A gh failure carrying an HTTP status prints the error body to stdout, so
jq chokes on it and the bounded retry fires. A CONNECTION-level failure
(TCP reset, TLS abort, DNS blip) leaves stdout empty — `jq -rs` then
prints nothing and exits 0. Absent pipefail that reads as success on
nothing read: the loop breaks on attempt 1, `status_lookup_ok` goes true,
and the empty id routes the writer to its "no status comment yet" branch,
posting a DUPLICATE blocked comment beside the stale one, run green.
Set the option locally on that command substitution. `defaults.run.shell:
bash` expands to `bash --noprofile --norc -eo pipefail`, so the step is
already pipefail on a real runner and this is redundant today; it is the
only guard that survives that default changing or the helper moving to a
step that sets its own options.
Tests: replay the same connection-level failure with the ambient pipefail
dropped and pin 3 bounded reads, exit 1, and no comment written; add the
HTTP-status half so the two failure shapes stay distinguishable and the
local option cannot be "simplified" away as carried by jq; pin both the
`set -o pipefail` and `defaults.run.shell: 'bash'` textually, since the
ambient half is what every other gh|jq writer in this file relies on
without saying so.
Both mutations verified to fail the suite.
---------
Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
|
||
|
|
732f4d8a29
|
feat(voice): support trusted private ASR base URLs (#8350)
* feat(voice): support trusted private ASR base URLs * fix(voice): address private endpoint review findings * test(voice): cover private endpoint edge cases * test(voice): pin remaining endpoint edge cases * fix(voice): address private endpoint review feedback * fix(voice): clarify allowlist URL and normalize IPv6 * fix(voice): harden NAT64 address validation * fix(voice): address managed endpoint review findings * refactor(voice): extract shared IPv6 transition unwrap ladder (#8350) Deduplicate the IPv6-transition unwrapping sequence (mapped, compatible, NAT64, dotted-quad) that was repeated verbatim between isPrivateNetworkIp and isAlwaysBlockedVoiceAddress on both CLI and Desktop surfaces. A single unwrapIpv6TransitionStep helper now yields the next canonical address (or 'blocked' for unrecognized ::ffff: forms), and each predicate recurses through it, preserving the exact re-check semantics at every unwrap level. * test(voice): cover allowInsecureBaseUrl wiring through desktop default transports (#8350) * fix(voice): add allowlist hint to private-network rejection error (#8350) * fix(voice): reject always-blocked base URLs before offering the allowlist hint (#8350) * fix(voice): resolve exact desktop voice provider before OAuth (#8350) * fix(voice): address review feedback for trusted private base URLs (#8350) * fix(voice): align desktop voice resolution with CLI semantics (#8350) * fix(voice): scope desktop fail-closed resolution to policy-bearing entries (#8350) * fix(voice): address round-8 review findings for trusted private base URLs (#8350) Run the invasive process-global `mock.module('ws')` suite as voice-ws-handler.isolated.ts so the desktop package's single-process `bun test` run no longer leaks the fake socket into unrelated ws consumers; the existing isolated loop runs it in its own process. Shape-guard the desktop provider scan: non-object modelProviders elements are skipped (falling through to OAuth instead of throwing a raw TypeError), and non-string baseUrl/envKey/settings.env values on a voice-model entry now surface the PROVIDER_ENTRY_REMEDY remediation error instead of crashing. Compute the DashScope-compatible /v1 rewrite before any allowlist match in fromExactModelProvider so the stage-1 check, the remediation messages, and the top-level recheck all compare the same final URL and a single allowlist entry converges for split-horizon deployments. Extend the CLI allowlist remediation messages to state which settings scopes honor the entry, since serve mode never shows the interactive workspace-strip warning. Thread providerProtocol through the CLI voice model seams (createVoiceModelSource and the daemon buildModelsConfig) so protocol-mapped custom provider groups resolve like the rest of the CLI model surface, and document the remaining protocol-agnostic desktop scan in the design doc. Correct the getHomeEnvFallback comment: it adopts the narrower getHomeEnvFallbackVars candidate set on purpose. Add multi-record DNS answer tests on both CLI and desktop net guards so the records.some classification is pinned against the array shape defaultLookupHost always produces in production. * fix(voice): address round-9 review findings for trusted private base URLs (#8350) * fix(voice): address round-10 review findings for trusted private base URLs (#8350) * fix(voice): classify desktop voice duplicates before ambiguity check (#8350) * fix(scripts): compare voice guard mirrors as parse trees (#8350) --------- Co-authored-by: rockybot2026 <265985139+rockybot2026@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> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
|
35108c0cf0
|
ci(autofix): run heavy autofix jobs on the ECS pool with hosted fallback (#8603)
* ci(autofix): run heavy autofix jobs on the ECS pool with hosted fallback
The heavy autofix jobs (issue-autofix, build-cli, review-address) were
pinned to GitHub-hosted runners, where they hold up to five 300-minute
slots per scan. Combined with CI, that saturates the org's ~20 hosted
concurrency slots and starves the PR-review workflow's hosted-only jobs
(delay timer, fork authorize, /resolve).
Route the three heavy jobs to the persistent ecs-qwen pool using the same
repository-guarded expression as the rest of the fleet, with an
ubuntu-latest fallback for forks of this repo and for
MAINTAINER_ECS_RUNNER_DISABLED.
This is safe for the fleet's two secret-bearing agent jobs:
- Every review-address target is live-gated to write+ (internal) authors
at scan AND address time; in-repo PR heads already require push access.
- CI_DEV_BOT_PAT is a write-level branch-push token with no privilege
escalation (cannot bypass branch protection).
- The ECS pool ships docker, and repo-hygiene already runs the sandbox
image there in production; sandbox containers use --rm with no fixed
name, so concurrent legs on one machine do not collide.
Relax the 'Check runner environment' gate to accept self-hosted, and drop
the github-hosted condition on 'Set up Node.js' so Node 22 is provisioned
on ECS. The contract test is updated to pin the new ECS routing expression
(including the repository guard and hosted fallback) instead of the old
hosted-only one.
* ci(autofix): harden ECS-routed heavy jobs for the shared persistent pool
- Restore workspace ownership before checkout in issue-autofix,
build-cli, and review-address, matching the sibling pool workflows
(root-owned leftovers from containerised jobs break checkout/npm ci).
- Sanitize the reused workspace's local git config (allowlisted
unset-all + symlink-aware hook sweep, qwen-triage's pattern) before
the PAT-bearing git steps in issue-autofix and review-address.
- Give issue-autofix a per-run WORKDIR (/tmp/autofix-<run_id>): pool
registrations share one /tmp and issue-phase runs never serialize.
- Drop stale local autofix/issue-* branches after checkout so a retry
on the same registration cannot hit "branch already exists" or push
a failed attempt's stale commits.
- Extend the ECS routing guard for PR-family events (same-repo head or
write+ author), mirroring ci.yml's classify_pr convention, and fix
the runs-on comments that claimed fork PRs already fell back.
- Stop persisting CI_DEV_BOT_PAT into the origin remote URL: publish
pushes authenticate transiently, and the redundant set-url in
'Push and report' is removed (push/fetch already use a URL inline).
- Pin all of the above in the workflow contract tests, reusing hoisted
job-block constants.
* ci(autofix): harden ECS pool hygiene with shared hardened sanitize action
Review round for the ECS routing of the heavy autofix jobs. The
sanitize step shipped in the previous commit was the trusted-lane
variant of the qwen-triage sweep, which two verified bypasses defeat
(worktree-scoped config carrying core.hooksPath, and a global
core.hooksPath steering the hook deletion). Port the hardened variant
into a composite action so all three heavy jobs share one copy, and
add it to build-cli, whose checkout runs in the same reused workspace
and feeds the bundle every PAT-bearing leg executes.
Also completes the persistent-pool hygiene the routing depends on:
per-run/per-target WORKDIRs now chmod 700 at creation and are removed
by an always() teardown step after the artifact upload (nothing else
ever deleted them); the stale-branch sweep detaches HEAD first and
globs via BRANCH_PREFIX; the artifact upload reads env.WORKDIR
instead of a duplicated literal.
The tautological runner-environment gate is repurposed as a
fail-fast docker preflight (a missing daemon otherwise surfaces only
at 'Resolve sandbox image', after npm ci/build), and Install tmux
switches to sudo -n so hosts without passwordless sudo fail with a
clear message instead of dying on a prompt.
The runs-on comments are rewritten to say exactly what they guard:
which events carry the same-repo/write+ clause and which rely on the
live write+ or label gates, why review-address is an author-permission
gate rather than a head-repository gate, and where the pool's docker
availability is proven in-repo (qwen-triage's container jobs on the
same labels).
Contract tests pin all of it: hygiene steps present and ordered
before checkout in every heavy job, the hardened sweep's ordering and
containment in the shared action, the docker preflight and tmux
hardening, the WORKDIR lifecycle, hosted-only short jobs, and generic
next-job slice boundaries so a job inserted after review-address can
no longer hide inside its test slice.
* ci(autofix): inline git-config sanitize step before checkout
The composite sanitize action was invoked as a local action
(`uses: './.github/actions/sanitize-workspace-git-config'`) BEFORE
`actions/checkout` in all three ECS-routed jobs. A local action resolves
from $GITHUB_WORKSPACE, so:
- on any clean workspace it hard-fails with "Can't find 'action.yml'" —
every hosted-fallback run (forks, MAINTAINER_ECS_RUNNER_DISABLED=true,
i.e. the documented rollback path) and any freshly registered ECS
runner — and it cannot self-heal, dying before checkout.
- on a reused workspace it executes the leftover copy, which for
review-address is the previous run's PR branch — pre-checkout execution
of PR-controlled content, the exact vector the step exists to prevent.
Inline the hardened sanitize script as a plain `run:` step in each job
(the qwen-triage 'Clean stale agent state' pattern) and delete the
now-consumer-less composite action. Update the contract test, which had
pinned the broken ordering, to pin the inline run step instead, and add a
generic guard that no local action may precede a checkout.
* ci(autofix): harden PAT handling and pool isolation on the shared pool
Address the Medium findings from the ECS-routing review:
- PAT pushes (Publish PR, Push and report, and the fork dry-run
preflight) now authenticate via a transient credential helper instead of
a token-bearing remote URL. The token enters neither the reused
workspace's .git/config nor the process argv, which is world-readable
via /proc on this shared host; the helper config holds only a
${GITHUB_TOKEN} reference and is swept by the next sanitize pass.
- 'Check runner environment' now asserts the self-hosted runner is an
ecs-qwen pool member by name, so a mis-labelled registration cannot
silently claim a PAT-bearing 300-minute job.
- 'Reset autofix workspace' age-sweeps abandoned /tmp/autofix-* dirs (a
hard runner kill skips the always() teardown and run_id never repeats)
and prunes the reused workspace's .git.
The contract test is updated to pin the tokenless push/fetch URLs, the
credential-helper authentication, and the absence of a token-bearing URL.
* ci(autofix): create autofix workdirs with umask 077 and correct cleanup comments
* ci(autofix): scope PAT credential helper to a one-shot git -c, plus review fixes
Address the latest review round on the ECS-routed heavy jobs.
High — the transient credential helper was not transient: `git config
--local credential.helper` wrote into the reused workspace's .git/config
and the matching unset was skipped on every error path (bash -e abort,
the fork-preflight exit 0, the salvage-loop exit 1s), stranding a helper
that answers any host with the PAT where ci.yml / pr-review jobs could
read it. All three push paths now use a one-shot, host-scoped
`git -c credential."https://github.com".helper=...` (a git_auth wrapper
for the salvage loop), so nothing is ever written to .git/config.
Medium — narrow the sanitize allowlist's `remote.*` to
`remote.<n>.(url|fetch|pushurl)` (uploadpack/receivepack are command
strings the sweep previously kept), and cover all three inlined sanitize
copies in the hardening test with a byte-identical pin (previously one
of three), rewording the stale "composite action" references.
Low — drop the recursive `rm -rf` hooks fallback that could wipe the git
dir itself (warn instead), extend the /tmp sweep glob to the legacy fixed
dir (`autofix*`), and make `git gc` threshold-driven (`--auto --prune=now`).
The contract test pins the git -c / git_auth form, the absence of any
config-written credential helper, and the tokenless URLs throughout.
* ci(autofix): pin gc to the foreground and anchor push auth assertions
Round-2 review follow-ups:
- `git gc --auto --prune=now` detaches by default (gc.autoDetach=true),
and --prune=now is unsafe when other processes write concurrently —
exactly what follows in these 300-minute jobs. Force it synchronous
with `-c gc.autoDetach=false` so the repack still only fires when the
--auto thresholds say so, but never races the checkout, agent commits,
or merge/retry loop that come after it.
- Tighten the two push assertions (dry-run preflight, Publish PR) to
require the host-scoped `git -c credential."https://github.com".helper`
prefix immediately before the push, instead of a bare
`push --no-verify …` match that a `git push` regression would still
satisfy.
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
|
||
|
|
2c514b50b9
|
fix(autofix): serialize scan-and-pick issue runs in one concurrency group (#8435)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
* fix(autofix): serialize scan-and-pick issue runs in one concurrency group The issue-phase concurrency group fell back to github.run_id for every run that route did not force a target — which is every scheduled run and every label-triggered run. Run-unique groups exclude nothing, so two overlapping scans (cron effectively fires every 40-70 minutes while the job may run for up to 180) could both pass the live label recheck during the minutes-long assess call, double-claim the same issue, and burn two multi-hour agent runs on it; the loser then fails its push and posts a withdraw comment. Key forced dispatches per issue, label events on the payload issue, and all scan-and-pick runs (cron or unforced dispatch) on one shared 'scheduled' group. cancel-in-progress stays false so a superseded tick still runs when targets remain. * fix(autofix): make concurrency group pin prettier-stable (#8435) * fix(tests): sync qwen-resolve-workflow timeout pins with repository variables (#8435) * fix(ci): pin issue-phase concurrency group equal to FORCED_ISSUE (#8435) * test(ci): anchor right edge of issue-phase concurrency group pin (#8435) * fix(autofix): keep never-runnable runs out of the issue-phase concurrency group (#8435) * test(autofix): pin issue-autofix concurrency gate to the job if predicate (#8435) * fix(autofix): exclude dry runs from the issue-phase concurrency groups (#8435) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(autofix): pin the Claim/Publish dry-run step gates (#8435) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: verify <verify@local> Co-authored-by: 易良 <1204183885@qq.com> 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> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
8fd0162c68
|
fix(release): normalize Live Host signing identity (#8579) | ||
|
|
89b3d5ea8e
|
fix(autofix): ship core dist in the review CLI bundle (#8612)
* fix(autofix): ship core dist in the review CLI bundle The review fan-out (#8548) dropped the per-leg build: legs restore the shared bundle's repo-root dist/ and rely on the verify gate's full build for branch verification. But the gate's settings-schema and i18n checks run BEFORE any build, on every path including no-action, and their tsx-transpiled cli sources import '@qwen-code/qwen-code-core', which resolves through the workspace symlink to packages/core/dist/index.js. With no build on the leg, the generator crashes with ERR_MODULE_NOT_FOUND and the gate misreports a deterministic "settings schema is stale" rejection (run 31031063525 on PR 8600), then burns an 18-minute repair agent round on an environment problem no agent can fix. Ship packages/core/dist (+~8.5MB gzipped) alongside the root dist/ in the fan-out artifact and assert its entry point on restore. This restores exactly the pre-fan-out state: legs used to build the trusted base themselves before the branch checkout, so the gate always ran against base-built core dist. The workflow contract tests pin the new tar command and the restore-side assertion. * fix(autofix): rebuild branch-touched core dist before the schema gate Review feedback on the core-dist bundle fix: - Rebuild packages/core from branch sources in the review verify gate when the branch diff touches core's sources, so the pre-build settings-schema check never compares the branch's committed schema against a base-built dist (changed runtime constants) or crashes the generator (changed exports). Lives in the shared gate script so both the initial and the repair gate are covered. - Narrow the bundle/restore comments and their test mirror to the settings-schema generator: the i18n check resolves core to sources via the packages/cli tsconfig paths map and needs no dist (verified empirically). - Anchor the tar contract pin at end-of-line so additive path drift fails the suite instead of passing on a substring match. |
||
|
|
a123d0030a
|
ci(review): prepare evidence-image tooling for GitHub-triggered reviews (#8454)
* ci(review): prepare evidence-image tooling for GitHub-triggered reviews Reviews triggered on GitHub cannot attach images today for three reasons; this wires the two that live in the workflow: - Install tmux and freeze (pinned, checksum-verified) before the review runs. Both are optional by contract — the evidence ladder degrades honestly without them (png -> ans-only -> refused, recorded in the capture manifest) — so the step never fails the review; it only decides which rung the runner can reach. tmux mirrors the tolerant install qwen-autofix.yml already uses; freeze falls back to ~/.local/bin when passwordless sudo is absent. - Pass QWEN_REVIEW_ASSETS_REPO from a repository variable to the review step. Publishing stays OPT-IN by design: with the variable unset the env is empty and publish-assets refuses (parseAssetsRepo trims and rejects empty), so nothing changes until a maintainer sets the variable. When set, evidence images land on commit-pinned pr-assets/<pr>-review branches — already covered by the visuals cleanup workflow — pushed with the same CI_BOT_PAT the step uses. The third reason is release lag: the capture producer (capture-tui, #8388) has to merge and ship in a release before rendering claims can generate images on CI at all. This change is inert until then. * fix(ci): capture-tools step review fixes — enforced tolerance, version pin, cached fallback R1-1: the never-fails contract is now enforced twice — continue-on-error at the YAML level (the belt) and set +e with a trailing exit 0 inside (the suspenders); under the runner's default bash -e several statements (mktemp, install, sudo install with an empty path) could previously abort the step and fail the review the comment promised never to fail. R1-6: probe the VERSION, not just the binary — on a persistent self-hosted runner an installed freeze made any FREEZE_VERSION/SHA bump a silent no-op; the pin now forces a refresh when the cached binary does not match. Cached-fallback fix: put ~/.local/bin on PATH (and GITHUB_PATH) before the probe — a sudo-less runner otherwise re-downloads the tarball on every review run forever. R1-3: the step comment says capture-tui is UPCOMING (#8388, not in the released CLI) and names qwen review drive as today's tmux consumer, so the step cannot be mistaken for stale dead weight and deleted from under the follow-up. R1-4: the retention comment scopes the cleanup-workflow claim to the same-repository designation; a fork or scratch destination manages its own retention (docs updated to match, plus a note documenting the repository VARIABLE a maintainer sets to enable publishing). R1-5: the step's real bash now runs in the workflow behavioural harness under bash -e with stubbed sudo/apt/curl/sha256sum/tar/uname: worst-runner and checksum-reject scenarios exit 0 installing nothing, the no-sudo happy path pins the ~/.local/bin + GITHUB_PATH pairing, and the version-pin probe is pinned from both sides (wrong version re-downloads, matching version skips). Real freeze/sudo on a developer machine are shadowed so the tests are deterministic and can never install to /usr/local/bin. Nit: both sudo guards now check sudo -n true. * fix(ci): capture-tools step review fixes — step-owned tool dir, anchored probe, honest failures Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): capture-tools test harness — shadow tmux, don't blank its PATH dir The harness dropped every host PATH directory that ships a tmux so the step's apt branch would depend on the scenario, not on the machine hosting the suite. On GitHub-hosted ubuntu runners tmux lives in /usr/bin, so the filter blanked /usr/bin wholesale — bash, grep, mkdir, and tar included — and execFileSync('bash') died of ENOENT: all seven capture-tools tests failed in the Test (ubuntu-latest Node 22.x) job while passing on tmux-less dev machines. Replace the directory-level drop with an entry-level shadow: each tmux-bearing directory is mirrored (symlinks) into a scratch dir minus the tmux entry, in place, preserving PATH order and the empty-entry stripping the old filter did. Hosts without tmux take the map through unchanged, and Windows (no tmux in its PATH, no symlink branch) keeps its current behavior exactly. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): capture-tools test review fixes — faithful stubs, env shape pins, missing-branch scenarios Round-3 review findings: the harness executed several of the step's branches but asserted nothing about them, so probe-verified mutants (dropped tmux guard, deleted warning/degradation messages, malformed or missing FREEZE_VERSION/FREEZE_SHA256, wrong hash variable, dropped URL `v` prefix or curl `-L`, severed tarball paths, broken regex boundary, leaked mktemp dir) all shipped green. - Make the curl/sha256sum/tar stubs model their real contracts: exact pinned URL, pinned checksum over a file curl actually wrote, existing -xzf operand - Pin FREEZE_VERSION/FREEZE_SHA256 shape in captureToolsSource - Pin the full curl flag set and the three-site tarball path agreement - Assert the stale-renderer warning (fires on degraded re-download, silent on the happy path) and the tmux-unavailable message - Pin TMPDIR and assert the mktemp cleanup leaves it empty - Add the two missing scenarios: tmux-present skips apt, cached version extending the pin with a leading digit re-downloads Verified by 13 mutation probes: every named mutant now turns the suite red (13/13 killed), baseline 34/34 green. * fix(ci): capture-tools step review fixes — hash-verified cache, per-run PATH promotion * fix(ci): capture-tools step review fixes — verified-bytes-only installs, step timeout Review findings on the capture-tools step: - Drop the PATH-trust branch: a freeze already on PATH was accepted on its own --version and executed to probe it — exactly the self-report the FREEZE_BIN_SHA256 comment declares attacker-controllable, from dirs writable between jobs on both runner classes. The checksummed download always runs now; the cache makes it free after the first run. - Guard $tools_bin in the download branch: with mktemp failing, the unguarded install resolved to /freeze — harmless unprivileged, but a root-in-container self-hosted runner writes it and reports success with nothing on PATH. - Copy-then-verify the cache: install into the fresh per-run dir FIRST, verify THOSE bytes, delete both copies on mismatch — the verified bytes are the bytes later steps execute, closing the check-then-copy race for free. This makes the separate pre-verify block redundant; it is deleted. - Add timeout-minutes: 5 — continue-on-error bounds failure, not duration, and a stalled `sudo apt-get update` mirror had no other bound under the 300-minute job cap. - Report block: say the resolved freeze is likely broken when its --version produces nothing, instead of echoing a blank line and calling it stale; the mismatch wording is direction-neutral now. Tests: replace the PATH-trust scenario with a planted-PATH one (marker outside the scenario dir proves the plant never executes), add the mktemp-failure scenario (the install stub succeeds like root would, so the unguarded mutant is caught) and the promoted-dir 0700 assertion; re-anchor the two digit-boundary tests on the report's warning. 41/41 green; both fix mutants verified killed. * fix(ci): capture-tools review fixes — stale-dir cleanup, pinned guards Address round-5 review: - R5-1 (Critical): the per-run qwen-review-tools.* dir under RUNNER_TEMP was never removed; RUNNER_TEMP survives across jobs on the shared pool, so every review run accumulated one dir + one Go binary, unbounded. 'Clean stale agent state' now removes stale dirs before the install step creates the current run's dir, matching the qwen-triage.yml convention. The harness comment claiming the dirs were runner-cleaned is corrected. - R5-6: the cache re-verification rejection branch now logs why it deletes the cached binary instead of degrading silently. - R5-7: bump-checklist note beside the freeze pins — the harness stubs key on the same env values, so a transposed hash pair must be caught against the real release artifacts at bump time. - R5-2/R5-3/R5-4/R5-5: four unpinned step properties now pinned (the if: guard, the sudo -n probe flag, install-after-context ordering, and the cache branch's tools_bin guard via a new mktemp-fails scenario); six mutation probes confirm each pin kills its mutant. * fix(ci): capture-tools review fixes — curl budget, swept scratch dir, wiring pins * fix(ci): capture-tools review fixes — harness mutation pins, pin-pair self-check Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): capture-tools review fixes — shadow-farm cleanup, backoff budget term * fix(ci): capture-tools review fixes — report probes only installed freeze, age-gated sweep --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
|
c73b5ed887
|
ci: run Windows merge queue tests on ECS (#8386)
* ci: run Windows merge queue tests on ECS
* test(channels): skip POSIX mode assertion on Windows
* ci: expose Git Bash on Windows ECS runner
* ci: scope Windows ECS tuning to self-hosted and restore full test:ci
Review feedback on the Windows ECS routing: dropping test:scripts removed the only Windows execution of 9 Windows-only install-script tests, and the job-wide PowerShell default plus narrowed test command changed the kill-switch fallback away from the known-good hosted configuration.
Restore the full npm run test:ci on both paths (bash is available: pre-installed on hosted runners, exposed via the Git Bash PATH entry on ECS) and gate every ECS-specific adjustment on runner.environment: the PowerShell setup step (now also skip_ci-guarded), TEMP/TMP/LC_ALL env writes, and the Linux-style Node setup split that fails with an actionable error naming MAINTAINER_ECS_RUNNER_DISABLED. The windows-2022 fallback is byte-for-byte the pre-ECS job again.
* test: make Windows CI suites platform-aware
* ci: add stale-checkout guard to Windows ECS test job
* test(core): compare canonical directory identity
* ci: add fork guard and review follow-ups to Windows ECS job
* test(core): exercise real directory identity change
* test(core): wait for killed lease process exit
* test(scripts): avoid cmd echo trailing spaces
* test(scripts): use unambiguous cmd echo syntax
* test(cli): avoid sidecar I/O in truncation test
* test: fix Windows script-suite gaps and unify platform gating
- Fix missed trailing-space cmd stub in package-scripts.test.js so the
'runs prepare steps in order' assertion passes on Windows.
- Add qwen-pr-review-workflow.test.js and pr-self-report-label.test.js to
the win32 exclude list (both test Linux-only workflows and are not
portable to Windows).
- Replace local itPosix/describeOnNonWindows consts with vitest's built-in
it.skipIf/it.runIf/describe.skipIf, matching the codebase idiom.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(scripts): restore Windows workflow coverage
* test(scripts): re-exclude Windows-incompatible workflow tests on win32
Re-add pr-self-report-label.test.js and qwen-pr-review-workflow.test.js to
the win32 exclude list. Both fail on a Windows runner for reasons the code
still carries: qwen-pr-review-workflow.test.js calls execFileSync('mkdir'),
which has no executable to resolve there, and pr-self-report-label.test.js
joins PATH with ':', corrupting the ';'-separated Windows PATH so its gh
stub never resolves. Excluding them restores a green Windows gate; Linux CI
remains their authoritative coverage. Document the criterion inline.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* ci: extract checkout-head guard into composite action, pin Windows gate (#8386)
Address review round 2: move the stale-checkout guard shared by the four CI gates into .github/actions/verify-checkout-head so the copies cannot drift, pin the Windows gate kill-switch routing and guard wiring in the script tests, re-enable lint.test.js on Windows via separator normalization and a lazy linter setup in scripts/lint.js, unify the platform skips on it.skipIf(process.platform === 'win32'), and document the queued-run behavior of the ECS kill switch.
* ci: fail fast in Windows gate environment setup (#8386)
* ci: dedupe self-hosted runner steps into actions, pin gate mutations (#8386)
* fix(ci): checkout before repository-local actions in Windows gates (#8386)
* fix(ci): configure Windows runner before bash guard
* test(ci): pin remaining shared-action wiring in script tests (#8386)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(ci): skip zip-dependent packaging tests when zip is missing (#8386)
* fix(ci): validate full Windows smoke path
* fix(ci): match Windows smoke shell to gate and drop dead runs-on guard (#8386)
* fix(ci): make SIGTERM escalation test Windows-aware and tighten pins (#8386)
The CDP acceptance test asserted a POSIX-only SIGKILL escalation, which
fails deterministically on Windows where kill('SIGTERM') terminates the
child directly — blocking the Windows merge-queue gate. Assert the
platform-appropriate signal instead.
Also address review suggestions: probe `unzip` alongside `zip`, pin the
integration_cli guard's missing step-level `if:`, stop getWorkflowStep
at unnamed steps, pin install-script.test.js out of the win32 excludes,
add the stale-checkout guard to windows-runner-smoke.yml, pin the
Node preflight warning branch and the guard reject path contiguously,
and extend the smoke shell-parity loop to the npm cache step.
* docs(ci): clarify Windows runner trust boundary
---------
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-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com>
|
||
|
|
93cd019088
|
fix(release): reuse configured Apple signing secrets (#8574) | ||
|
|
a5c637b749
|
feat(web-shell): add native Live Voice (#7859)
* feat(web-shell): add native Live Voice * fix(web-shell): address review feedback for Live Voice PR (#7859) - Quote all strings in electron-builder.yml to fix yamllint CI failure - Gate discovery publish on liveVoiceEnabledAtBoot to avoid writing bearer token to disk when Live Voice is disabled (M1) - Add child identity guard to CommandMonitor stdout/stderr handlers to prevent stale helper output from corrupting the new buffer (M4) - Add exponential backoff to sent-completion delivery retry (M3) - Skip broadcastState when setCallState/setTranscript value is unchanged to reduce per-audio-delta overhead (H1) - Document sent-mode completion notification in module docstring (H2) - Remove dead protocol/nonce aliases from readDiscoveryFile - Fix single instance lock fall-through with process.exit(0) * fix(cli): register realtime_voice in docs contract and env guard (#7859) * fix(web-shell): address review feedback for Live Voice PR (#7859) * fix(cli): discard orphaned isolated dir when parent restore fails (#7859) * fix(web-shell): address review feedback for Live Voice PR (#7859) * fix(serve): harden live turn recovery * fix(desktop): restore Live Host native build * fix(live): align native host and session isolation * fix(acp): preserve live worker continuation lineage * fix(live): classify provider close reasons * fix(serve): discard unused recovered conversation dirs * fix(live): isolate authorized realtime responses * fix(live): preserve realtime response authority * feat(web-shell): complete Live Voice onboarding * fix(live): persist realtime-owned dialogue * fix(live): preserve final speech while stopping * Revert "fix(web-shell): address review feedback for Live Voice PR (#7859)" This reverts commit 7110bec6b034c702bca6e28e35b93c7f70e729cd. * Revert "fix(cli): discard orphaned isolated dir when parent restore fails (#7859)" This reverts commit 85165f1b2ddfaa311b8be91acdd76a6f388f6204. * Revert "fix(web-shell): address review feedback for Live Voice PR (#7859)" This reverts commit 9199fa633e102bb8f24e4b216d322be4323eb3fc. * Revert "fix(cli): register realtime_voice in docs contract and env guard (#7859)" This reverts commit 6b6b1718352ef01a98a73976b5c7c4433fd14c35. * Revert "fix(web-shell): address review feedback for Live Voice PR (#7859)" This reverts commit e083779105199d26de3afd8ad00719a08efe3099. * revert(live): remove remaining takeover behavior * revert(live): restore pre-rollback implementation * test(cli): align Live diagnostics env guard * test(release): cover Live Host publication * fix(ci): re-sign Live Host package before verification * fix(serve): scope sent completion notifications to Live * fix(web-shell): preserve live setup errors * fix(live): align realtime backend speech lifecycle * ci(live): publish Live Host independently * test(cli): mock Live speech bridge handler * test(release): align Live Host workflow contract * fix(live): address release and lifecycle review findings * fix(live): release completed call tracking --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
|
da37110e60
|
perf(autofix): build the review CLI bundle once per scan and fan it out to legs (#8548)
* perf(autofix): build the review CLI bundle once per scan and fan it out to legs Each review-address leg repeated the same trusted-base build: measured 3.5-5 minutes of npm ci + build + bundle per leg (~25 runner-minutes on one 6-leg scan) before the agent could start. A build-cli job now compiles the bundle once per scan, uploads the repo-root dist/ as an artifact, and the legs download it; their checkout is pinned to the compiled SHA so a mid-run base push can never pair a leg's bundle with different sources. The legs keep npm ci (the agent and the verify gate still need node_modules against the PR branch), and the issue phase is untouched — it runs only when no review targets exist, so gating the build on do_issue too would rebuild on every quiet scheduled tick. * fix(autofix): validate fan-out bundle SHA and pin shared CLI recipe contracts (#8548) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.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> |
||
|
|
e34780e24d
|
fix(ci): clean review worktrees after cancellation (#8474)
* fix(ci): clean review worktrees after cancellation * fix(ci): remove orphaned review worktree directories * fix(tests): sync qwen-resolve-workflow expectations with externalized review timeouts (#8474) * fix(ci): pin review worktree cleanup patterns to paths.ts (#8474) * fix(ci): harden review cleanup sweeps and cover integration_cli (#8474) * fix(ci): extend review cleanup sweep to web_shell_e2e_smoke (#8474) * fix(ci): harden review cleanup git calls * fix(ci): tighten review cleanup comments and test guards (#8474) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ci): pin review cleanup recipe copies byte-identical (#8474) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): guard review worktree removal and pin cleanup invariants (#8474) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- 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> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
2601d815dd
|
test(ci): align resolve-workflow timeout expectations with externalized variables (#8460 follow-up) (#8485)
* test(ci): align resolve-workflow timeout expectations with externalized variables #8460 moved the review timeout numbers into repository variables (QWEN_REVIEW_JOB_TIMEOUT_MINUTES / QWEN_REVIEW_MAX_TIMEOUT_MINUTES) but left qwen-resolve-workflow.test.js expecting the old hardcoded 300/240 values — main's Test lane has been red since. The five expectations now pin the variable-driven forms the workflow actually carries. * test(ci): pin resolve-workflow timeout enforcement, tiering, and fallback machinery * test(ci): pin the timeout-advice branch CONDITION, not just both bodies With both branch bodies asserted as substrings of the same step, any comparison flip (-ge/-gt/-le) keeps both strings present and ships the wrong recovery advice on every timeout — a below-max run told it already used the maximum, an at-cap run told to retry with a timeout that cannot work. Raised in #8388's review round (R4-21). * test(ci): anchor resolve-workflow arm slices at their if starts Address review feedback on #8485: search for each sliced arm's else from the arm's own if start so an unrelated earlier if/else in the step cannot invert the slice (or vacuously satisfy .not.toContain on an empty slice). Also slice the small-PR tier arm instead of comparing first-occurrence indexes, unifying the technique and proving the assignments sit in opposite arms. * test(ci): keep the max-timeout text out of the below-max arm slice * test(ci): pin fallback arm bodies, comment wiring, and tiering order * review: line-anchor the at-max arm's closing fi A bare indexOf('fi') stops at the first word CONTAINING the letters — 'specified', 'notification' — so a harmless wording change inside the at-max arm would silently truncate the slice and hand the arm's not-toContain a vacuous pass. The end anchor is now the line-anchored /\n\s*fi\b/, with its own found-assertion. --------- Co-authored-by: Qwen Autofix <autofix@qwen-code.dev> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: verify <verify@local> |
||
|
|
3ad4fbdb7d
|
fix(cli): preserve Qwen Review startup version in footers (#8431)
* fix(cli): preserve review startup version in footers * fix(cli): keep review startup version dynamic in bundle * fix(cli): reset review version after managed update * test(cli): use indexed env access * fix(cli): harden review footer strip and version stamping (#8431) * fix(tests): sync qwen-resolve-workflow expectations with externalized review timeouts (#8431) The timeout externalization in #8460 replaced the hardcoded 300/240 values in qwen-code-pr-review.yml with the QWEN_REVIEW_JOB_TIMEOUT_MINUTES and QWEN_REVIEW_MAX_TIMEOUT_MINUTES repository variables but left scripts/tests/qwen-resolve-workflow.test.js asserting the old literals, so the full-profile Test job fails on any branch carrying that change. Update the three affected assertions to pin the externalized shape. * fix(cli): harden the review footer strip per review feedback (#8431) The strip regex kept a 2^(N-1) partition ambiguity for same-line footer runs (measured 5.3 s at n=20) and missed footers truncated before their closing `_`; forged footers also survived on the body channel through `bodyCriticals`, and the values interpolated into the footer were not shape-validated. Guard the repeated group so an iteration cannot span another footer's start, make the final `_` optional, strip body Criticals per entry, refuse footer-forging model ids and non-version stamps, refuse non-object comment entries, pin the CLI-glue test suite against an ambient startup stamp, and cross-assert the LGTM filter regex against the footer builder. * fix(cli): strip review footers before ledger carryover * fix(cli): align ledger footer regression expectation --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
d2c7cb1f15
|
fix(autofix): normalize paginated fetches to one flat array per file (#8438)
* fix(autofix): normalize paginated fetches to one flat array per file gh api --paginate emits one JSON array PER PAGE, so any PR past 100 comments/reviews/events produces a multi-document stream. The workflow already slurps correctly in a few readers (jq -rs add, --slurpfile + add), but more than a dozen plain-jq consumers of the WORKDIR files mis-aggregate on a multi-doc input: - MARKERS/REARM_AT/RED_HEAD/REARM_KEY in the scan and their LIVE_* mirrors in prepare emit one result per page; ROUND then becomes a multi-line string, [[ -ge ]] arithmetic fails, and the round cap silently stops holding — on exactly the PRs (takeover, 100-round cap, one report comment per round) that reach page two first. - CAP_NOTICED / BASE_UPDATE_RECENT / LAST_REJECTION / PRIOR_TIMEOUTS / the milestone census and the report-step consecutive-failure census all degrade the same way. - NEWEST and LIVE_NEW bind rv/rc/ic/checks POSITIONALLY (.[0]..[3]); a two-page rv.json shifts rc/ic into the wrong slots and later feedback is silently lost. Fix at the fetch sites: every --paginate that lands in a WORKDIR json file (and the report step's COMMENTS_JSON fallback) now pipes through jq -s 'add // []', so each file holds ONE flat array. Existing slurp-style readers are unaffected — add is idempotent over a single array — and every plain consumer becomes correct past 100 items with no program changes. Failure semantics are preserved: the workflow-level bash default gives -eo pipefail, so a failed gh still fails the pipeline exactly where it failed the bare redirect before, and the pr-events/COMMENTS_JSON fallbacks keep their '[]' paths. The check-runs/annotations/status-comment reads stay raw on purpose: they aggregate per-page via --jq + slurp, line-streams, or .[][] and were already pagination-safe. Tests: a behavioral case runs the real MARKERS→ROUND pipeline and the positional NEWEST program against two-page fixtures through the normalizer, with negative controls demonstrating the pre-fix corruption (two MARKERS lines; the page-2 review timestamp lost to slot shift). Shape assertions pin all nine normalized fetch sites and ban raw --paginate file redirects. * fix(autofix): pin total --paginate occurrence count in tripwire test (#8438) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(autofix): correct gh --paginate merge model in pagination comments (#8438) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(autofix): make engage-ack ic re-fetch atomic on failure (#8438) * test(autofix): pin atomic engage-ack re-fetch and empty-input normalization (#8438) --------- Co-authored-by: verify <verify@local> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
10291e16f2
|
refactor(core): move review skill incident narratives to DESIGN.md (#8499)
* perf(review): issue independent setup calls in one response Measured on a real small-PR run: the stretch from parse-args to the first agent launch took 7 minutes of wall clock, one round-trip at a time, on calls that never needed an order — pr-context, comment-status and the Step 2 rules load are mutually independent reads. Step 1 now tells the orchestrator to issue all three in a single response (the same rule Step 3 already enforces for the agent fan-out) and to page their outputs in shared responses too. comment-status loses its wait-for-the-context-file guard in worktree mode: learning whether inline comments exist cost a serial round-trip, while running it on a commentless PR just writes an empty index. Step 6's two deterministic gates (script-lint, test-plan) get the same one-response note. The orderings that matter are kept explicit: fetch-pr before everything (it creates the worktree and the plan), the roster after the rules load (it bakes the rules into every brief). * refactor(core): move review skill incident narratives to DESIGN.md SKILL.md is injected wholesale into the review orchestrator's context on every /review run and re-billed on each of its turns, and ~16KB of it was incident narrative — accounts of past dogfood failures and measurements that justify rules but are not themselves instructions. Move 50 such narrative blocks into a new 'Measured incidents (moved from SKILL.md)' section of DESIGN.md (47 anchors, not loaded at runtime), leaving every rule in place with a short '(measured; DESIGN.md — <anchor>)' pointer. Force-bearing figures stay inline where the number is the argument (e.g. the ~161s cold npm ci, the 41% test-code median, the PR #6457 one-of-five checklist measurement). No instruction, gate, format, flag, threshold, or ordering changed; the YAML frontmatter and all 35 fenced code blocks are byte-identical, and the MUST / Do not / never imperative counts are unchanged outside the moved narrative text (verified by script). SKILL.md: 237,847 -> 228,266 bytes; DESIGN.md: 106,708 -> 125,184 bytes. * fix(review): keep DESIGN.md out of the runtime bundle and pin pointers The slim refactor left DESIGN.md shipped beside SKILL.md in dist/bundled/, so one curious read_file of the 125 KB maintainer document would cost more context than the refactor saves. The bundle copy now skips DESIGN.md, and SKILL.md gains a one-line guard telling the orchestrator the pointers are for humans auditing a rule. Also addresses review feedback: a test pins both directions of the SKILL.md incident-pointer mapping, the transcribed-argument narrative keeps its referent after the move, the incidents section title loses its changelog suffix, and Step 2 no longer asks for a base fetch that fetch-pr already performed. * fix(review): gate setup batching by effort and consolidate incident blocks Address round-1 review feedback on the skill-slim PR: - Gate the ONE-response setup batch and the comment-status call to high and medium effort, matching Step 2's low-effort skip. - Scope the Step 6 lint/test-plan batching to same-repo PR reviews. - Merge same-run incident blocks (self-composed Approve into the paraphrased roster prompt; archive verdict into the narrated-away cap), cross-reference the roster-size and relocated-Critical tellings, and state the #8368 path in its block plus the pointer it was missing. - Pointer-ize the last inline QQChannel narrative and fix the scripts-nobody-ran summary to match its block. - Extend the DESIGN.md exclusion to copy_files.js so the transpiled dist/src build and the published core tarball stop shipping it. - Pin the no-read_file guard and the batch ordering constraints in SKILL.test.ts, and fail loudly on pointers the regex cannot parse. --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
|
06cc41ee3f
|
ci: route trusted-author fork PRs and no-checkout jobs to the ECS pool (#8502)
Some checks failed
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
npm cache producer / Save npm cache (push) Has been cancelled
* ci: route trusted-author fork PRs and no-checkout jobs to the ECS pool Fork PRs whose author has write access (OWNER/MEMBER/COLLABORATOR association) now run Linux CI on the self-hosted ECS pool instead of the saturated GitHub-hosted quota, and bot workflows that check out no code move to ECS unconditionally. Everything stays gated on the MAINTAINER_ECS_RUNNER_DISABLED kill-switch. * ci: address review — real write-permission routing, watchdog independence, timeouts Route the triage agent on the collaborator-permission API result computed by authorize instead of the coarse author_association, which admits org members and read-only collaborators; the two permission-gate jobs revert to the same-repo guard. Keep the fleet watchdog and the CI-failure reporter hosted so they stay independent of the pool they watch. Add missing timeouts, wipe serve-ab's reused workspace, and pin the routing logic with drift and negative-case tests. --------- Co-authored-by: 易良 <1204183885@qq.com> |
||
|
|
d6f55a1c95
|
fix(ci): align review workflow tests with externalized timeout variables (#8486)
#8460 moved the review timeouts into the QWEN_REVIEW_JOB_TIMEOUT_MINUTES and QWEN_REVIEW_MAX_TIMEOUT_MINUTES repository variables but left the workflow-text assertions in scripts/tests/qwen-resolve-workflow.test.js pinned to the old hardcoded 240/300 values, so the workspace test suite fails (Release Quality Checks and the PR Test job). |
||
|
|
fddcda86ed
|
ci: reduce SDK Java runner queueing (#8441)
* ci: cancel stale SDK Java pull request runs * fix(ci): preserve SDK Java push scheduling * ci: route trusted SDK Java jobs to ECS * ci: simplify SDK Java runner routing * fix(ci): provision Maven on ECS Java jobs * fix(ci): clean SDK daemon ECS test state * fix(ci): satisfy SDK Java lint |
||
|
|
72bd3dccc2
|
ci: remove broken legacy scheduled PR triage workflow (#8434)
The Gemini-era scheduled PR triage workflow has been dead weight for a long time: - Its only business value — syncing labels from the linked issue to the PR — never fires: gh exports closingIssuesReferences as a flat array, so the script's '.closingIssuesReferences.nodes[0].number' jq path always errors, the error is swallowed by 2>/dev/null, and every PR falls into the "No linked issue found" branch. The latest production run logged 157 "No linked issue" hits and zero label syncs, despite many of those PRs having linked issues. - LABELS_TO_REMOVE is computed but never applied, PRS_NEEDING_COMMENT is never appended to, and the prs_needing_comment job output has no consumer — the rest of the script is dead code. - It burns 1+N API calls against every open PR every 15 minutes. - The id-token: write permission is a leftover from the Gemini/GCP OIDC era; nothing in the bash script uses it. Real PR triage lives in qwen-triage.yml. Remove the workflow and its script, drop the stale docs section describing behavior it never had, and pin the file into the legacy-workflow regression list. Co-authored-by: verify <verify@local> |
||
|
|
563f744329
|
feat(ci): size-aware default timeout for the PR review job (#8377)
The review-pr job ran every PR with a fixed 180-minute default budget. A medium PR (#8241, +1577/-57) exhausted it and was killed at exactly 180 minutes without ever posting its review, while the same review can finish in ~90 minutes on a less loaded runner. When the caller does not pass an explicit --timeout, size the default budget by the diff (additions + deletions): small PRs (<= 300 lines) keep the proven 180 minutes, and anything larger gets the full 240 cap. An explicit --timeout=N still wins, and a failed size lookup falls back to 180 rather than failing the review. Raise the job-level timeout from 260 to 300 so the 240 budget plus the shared retry and comment posting fit with headroom. |
||
|
|
07e35bf02f
|
feat(ci): add repo-hygiene skill and weekly patrol workflow (#7908)
* feat(ci): add repo-hygiene skill and weekly patrol workflow Add .qwen/skills/repo-hygiene (SKILL.md + scripts/run-agent.mjs) and the corresponding GitHub Actions workflow that runs a weekly scan-and-fix patrol every Monday 03:00 UTC. The skill defines a single scan-and-fix mode that scans the repo for small, certain docs/test/code hygiene issues (broken Markdown emphasis, test coverage truthfulness, contract mismatches, real boundary conditions), batches up to 8 fixes as individual Conventional Commits on ONE branch, and writes findings.json + bilingual report-only.md + PR body. High-risk paths (packages/core/src/**, auth/providers/models/config/tools/services) are report-only — they go to the PR comment, never the diff. The workflow implements the trust split: agent never holds GitHub credentials; only the workflow publishes. Includes dedup (skip when a hygiene PR is already open), PAT identity verification, an independent verification gate (build/typecheck/lint + per-package vitest --changed), and a structural gate (commit cap, total diff cap, report-only path assertion). Closes #7383. * docs(repo-hygiene): expand judgment scans to six parallel search angles Port the richer search-angle taxonomy from the operator's interactive loop-hygiene prompt into the SKILL.md. Section B (judgment scans) now enumerates six explicit angles — test-coverage truthfulness, implementation/contract mismatch, resource lifecycle, real boundary conditions, user-visible configuration/API, and docs-as-secondary-scan — each with a grep/code-reference evidence requirement. The previous three-bullet version collapsed resource lifecycle and user-visible config/API into adjacent items, which meant the agent under-scanned AbortController/finally/iterator-return/stream-cleanup gaps and config-vs-schema disagreements in practice. Also makes the subagent workflow explicit: subagents report candidates only and never touch the working tree; the main agent deduplicates and decides. This matches the operator prompt's 'retrieval subagent only reports, never modifies' constraint. * refactor(repo-hygiene): drop deterministic-docs section, fold its principles into the scan loop Section A (Deterministic docs patterns) was too narrow: it gave two specific rg commands that dated quickly and duplicated the docs-as-secondary-scan rule already present in the judgment-scan taxonomy. Remove the section and promote Section B to the only scan-targets list. Two of Section A's cross-cutting principles are preserved by folding them into the scan loop's opening paragraph and the Docs bullet: - 'a pattern hit is a lead, not a finding' — now applies to rg, grep, and any other scanner - 'broken-but-rendering-fine emphasis stays untouched' — now part of the Docs bullet's exclusion list * refactor(repo-hygiene): partition the scan into nine parallel subagents The previous Scan Targets section dispatched subagents by search angle (six parallel scans of the whole monorepo). Each subagent ended up with a shallow read of every file it touched — it couldn't hold a module's contract graph in context, so findings tended to be one-line-deep (docs-vs-code, rendering bugs, dead-code hints). Switch to partition-first dispatch: one subagent per of nine module boundaries (cli/config, cli/runtime, cli/ui, core, extensions, sdk-typescript, sdk-python-java, ui-apps, docs). Each subagent owns its partition, finds the package's own entry points/schemas/registries, then applies the six search angles inside that scope. The main agent deduplicates across partitions afterward. A subagent must stay inside its partition; cross-partition findings are marked crossPartition:true for the main agent to merge or drop. Reading lists are intentionally not prescribed — the subagent picks the partition's own sources of truth. * refactor(repo-hygiene): allow cross-partition tracing, drop anti-padding bullets Two simplifications: 1. Replace 'subagent must stay inside its partition; crossPartition:true' with 'partition is a starting boundary, not a fence'. A subagent may now follow a call chain, import graph, or contract reference into another partition to build evidence. When a finding's evidence lands in a report-only path (core/*, auth/*, providers, models, config, tools, services, cross-package contracts), it goes to reportOnly — never into fixes. This replaces the previous crossPartition handoff machinery with one uniform rule that already exists in Scope Limits. 2. Drop two anti-padding bullets from Scope Limits ('do not batch trivial fixes to reach quota', 'do not manufacture findings to fill the run'). The Shared Rules' 'no speculative edits / keep changes minimal and scoped' already covers the same behavior; restating it here was noise. * docs(repo-hygiene): fix stale 'scans A and B' reference in scan-and-fix mode The A/B section split was collapsed into a single Scan Targets section two commits ago, but step 1 of Mode: scan-and-fix still said 'Run scans A and B'. Reword to describe the actual dispatch: nine partition subagents, six angles inside each, collect and deduplicate across partitions. * refactor(repo-hygiene): switch report-only rule from path to impact scope Drop the path-based report-only whitelist (packages/core/src/**, */src/auth/**, providers/models/config/tools/services/**, cross-package contracts). The report-only trigger is now impact scope: a finding whose minimal fix touches more than three files or more than one hundred lines of production code is report-only. Rationale: the path list was a proxy for 'high-impact area' that over-blocked trivial, certain fixes inside those paths and under-blocked sprawling fixes outside them. The file/line threshold measures impact directly. Scope Limits already caps each fix at 20 lines of production diff, so anything past 100 lines is an order of magnitude beyond that — past the four-file mark is past the single-root-cause test. * docs(repo-hygiene): expand each partition with package layout and correctness criteria The previous Nine-partitions section was a one-line scope pointer per partition. Each partition is now a mini architecture block naming the package path, what the package does, its key subdirectories, and what 'correct' looks like inside the partition. Directory names verified against the current tree (cli/src/{commands,serve,acp-integration,services,remoteInput,dualOutput,startup,i18n,utils,core,export}, cli/src/ui/{agent-view,arena,extensions,mcp,hooks,subagents,background-view,shared,messages,contexts,themes,state,layouts,voice,selection,editors,daemon,models,noninteractive}, core/src/{agents,models,providers,tools,services,prompts,utils,hooks,memory,skills,subagents,permissions,confirmation-bus,mcp,lsp,ide,goals,resources,followup,extension,config,telemetry,output,qwen}, desktop/apps/{electron,viewer}, web-shell/client). * refactor(ci): replace path-based gate with per-commit impact-scope check The workflow gate previously rejected any branch touching core/src or auth/providers/models/config/tools/services paths. This conflicted with the SKILL.md's impact-scope rule (>3 files or >100 lines = report-only). Replace with a per-commit check: each commit must touch ≤3 files and have ≤100 diff lines. This aligns the deterministic gate with the agent's report-only threshold. * chore(ci): allow repo-hygiene workflow on fork for testing * refactor(repo-hygiene): remove fix cap, file report-only as consolidated issue - Remove MAX_FIXES (8) and MAX_TOTAL_DIFF_LINES (150) caps from workflow gate - SKILL.md: no limit on fixes per run, only per-commit scope threshold - Add workflow step to create single GitHub issue for all report-only findings - Issue is deduplicated against existing open hygiene issues - Per-commit impact-scope gate (≤3 files, ≤100 lines) remains unchanged * fix(repo-hygiene): per-fix typecheck prompt + auto-revert on verification failure - SKILL.md: require typecheck after each individual fix, not batched - Workflow: typecheck runs first with auto-revert loop; bad commits are dropped while good ones survive, then full build+lint+test runs - Prevents one bad fix from killing the entire patrol run * fix(repo-hygiene): forbid retrying failed fixes to avoid loop detection Agent got stuck retrying typecheck fixes repeatedly, hitting the per-turn tool-call cap. Explicitly instruct to accept failures as report-only and move on without retrying. * fix(repo-hygiene): use only existing labels, structured failure handling - Remove HYGIENE_LABEL env var; PR dedup by title+author search instead - Issue uses 'bug' label (always exists); dedup by title search - Remove label creation step - SKILL.md: give agent concrete 3-step failure protocol (checkout, move to reportOnly, continue) instead of vague 'don't retry' * fix(repo-hygiene): remove failure protocol, add anti-loop instruction - Remove 3-step failure handling protocol (was over-engineered) - Restore original 'write failure.md' behavior for failed verification - Add: never re-issue identical tool calls (prevents loop detection) * feat(repo-hygiene): add scan/fix split mode for two-phase execution - SKILL.md: add Mode: scan-only and Mode: fix-only sections - Workflow: add mode input (scan-and-fix/scan/fix) + scan_run_id input - run-agent.mjs: add scan and fix specs with proper inputs/outputs - Scan mode: only scans, uploads findings as artifact (~15min) - Fix mode: downloads previous scan findings, only fixes (~25min) - Gate/verify/push skipped for scan-only mode - Branch name resolution skipped for scan-only mode * refactor(repo-hygiene): split into scan + fix jobs in one workflow Two-phase design within single workflow run: - scan job: read-only, dispatches subagents, writes findings.json (~15min) - fix job: depends on scan, downloads findings, fixes + verifies + pushes (~25min) Benefits: - Each phase stays within model's tool-call budget - Scan failures don't waste fix-phase budget - Fix failures don't require re-scanning - phase input: both (default), scan, fix (with scan_run_id) * simplify(repo-hygiene): remove phase/mode inputs, always scan then fix Users just trigger the workflow. Scan and fix jobs run in sequence automatically. No need for phase selection or scan_run_id. * simplify(repo-hygiene): remove Mode sections from SKILL.md - Replace three Mode sections with single 'Execution Steps' heading - Step 1: scan phase (stop after findings.json) - Step 2: fix phase (read findings, fix, verify, write PR) - run-agent.mjs: invocation now tells agent which phase it's in using plain English instead of slash commands * fix(repo-hygiene): use plain expression in job-level if condition Remove ${{ }} wrapper from job-level if conditions to avoid potential YAML parsing issues. * fix(repo-hygiene): configure git remote with PAT before push Checkout uses persist-credentials: false, so git push needs explicit authentication via remote URL with x-access-token. * fix(repo-hygiene): use single quotes for all if conditions yamllint requires single-quoted strings. Changed all double-quoted ${{ }} expressions to single-quoted with escaped inner quotes. * feat(repo-hygiene): gate drops oversized commits instead of failing - Collect oversized commits during scan loop - Capture commit messages before rebase - Use git rebase -i to drop only the bad commits - Move dropped findings from fixes to reportOnly in findings.json - Continue with remaining commits instead of aborting the run * refactor(repo-hygiene): split SKILL.md into per-phase documents - SKILL.md keeps only shared content: rules, scope limits, findings.json format, output contract - scan.md: scan targets (9 partitions, 6 angles) + scan steps - fix.md: fix steps - run-agent.mjs concatenates SKILL.md + phase doc by mode, so each phase's prompt contains only what that phase needs - Drop the unused scan-and-fix mode * refactor(repo-hygiene): move phase docs into references/ per skill convention Match the triage/docs-audit skill layout: SKILL.md is the entry with shared rules and a Workflow routing section; phase details live in references/scan.md and references/fix.md. The CI runner still injects the right phase doc into the prompt; interactive invocations follow the routing section instead. * refactor(repo-hygiene): adopt references/ progressive-disclosure layout Match the triage skill convention: SKILL.md is the single entry with shared rules and a Workflow routing section; phase details live in references/scan.md and references/fix.md which the model reads itself. The runner now sends only SKILL.md plus a one-line phase invocation. * docs(repo-hygiene): drop redundant duplicate-tool-call rule Loop guard already covers real loop scenarios; forbidding same-arg calls at the SKILL layer added no value for a 9-subagent scan with independent budgets and a state-heavy fix phase. * fix(repo-hygiene): enable agent tool for scan and preserve partial findings Scan settings never whitelisted the agent tool, so the nine-subagent dispatch was impossible and the model scanned serially, blowing the 50-minute budget with zero output. Add agent to scan coreTools, teach the serial fallback to write findings.json incrementally after each partition, and upload agent.log/failure.md so failed scans can be diagnosed. * chore(repo-hygiene): drop fork-only repository guard * chore: drop unrelated advisor design draft from branch * fix(repo-hygiene): harden gate, dedup, and verification semantics - Zero-fix runs are now green: fix phase stops silently instead of writing failure.md when the fixes array is empty - PR dedup matches by hygiene/ branch prefix instead of trusting the agent-written title to contain 'repo-hygiene' - Gate drops oversized commits in one rebase, removing the implicit newest-first ordering dependency of sequential rebases - Dropped and reverted commits are appended as notes to pr-body.md so the PR description matches what is actually in the branch - Scan job timeout raised to 75min (agent 50min cap + ~10min setup left a negative margin); typecheck output no longer suppressed * fix(repo-hygiene): raise agent timeout for nine-partition scans The v4 local run timed out at the 50-minute default while the v5 run completed successfully but close to the limit. Nine parallel subagents need more headroom, especially on slower CI runners. - run-agent.mjs default: 50min → 70min - Scan phase explicit QWEN_TIMEOUT_MS: 65min (job limit 75min) - Fix phase explicit QWEN_TIMEOUT_MS: 75min (job limit 90min) * fix(ci): use full SHAs in gate rebase and document typecheck-only revert * fix(ci): skip push when verification reverts every hygiene commit * fix(ci): route dropped and reverted findings into reportOnly * fix(ci): correlate dropped commits to findings by bracketed id * fix(ci): label hygiene PRs with autofix/repo-hygiene * docs(skill): correct scan partition facts and skip empty report-only sentinel * fix(skill): stop requiring report-only.md as a run-agent output * fix(skill): settle the run-agent promise when the log stream errors * fix(ci): gate the fix job on a scan-phase failure.md and validate findings JSON * fix(ci): make the push gate's success dependency explicit * fix(ci): keep dry runs from filing real report-only issues * fix(ci): drop unregistered read_many_files and grant git clean for reverts * fix(ci): count only production files and lines in the size gate * perf(ci): skip the scan phase while a hygiene PR is still open * docs(skill): resolve failure-handling and schema-regen contradictions * fix(ci): apply review fixes to hygiene gates, runner signals, and skill docs * docs(skill): align scan paths, reportOnly schema, and drop dispositions - Document intentionally excluded packages in scan.md - Fix cli/ui subpackage paths to include components/ prefix - Add optional status field to reportOnly schema in SKILL.md - Carry status in workflow gate/verify node scripts - Give fix.md step 4b the same reportOnly drop path as 4a/4c * fix(ci): add fork guard to repo-hygiene dedup job (#7908) * fix(ci): surface surviving hygiene findings when verification fails (#7908) A post-typecheck failure (build/lint/settings-schema/test) exited the verify step non-zero without moving the surviving committed findings to reportOnly, so the consolidated report-only issue never listed them and a persistent failure silently discarded the same findings every week. Mirror the revert path: an ERR trap now moves every remaining fix to reportOnly (status failed-verify) before the step exits non-zero. Also resolve a Scope Limits contradiction in the skill docs: the <=20-line per-fix rule read as a hard cap while scan.md and the workflow gate enforce only the 100-line/3-file report-only threshold. Make <=20 an explicit target and document the new failed-verify status. * fix(ci): salvage committed findings on failure, append to existing report-only issue, and incremental parallel scan writes (#7908) * fix(ci): address review — contracts gate, sandbox image, gate salvage, revert labeling (#7908) * fix(ci): enforce shell allowlist via auto-edit and salvage rebase conflicts (#7908) * fix(ci): address review — scan mkdir allowlist, rg sandbox note, write deny rules (#7908) * fix(ci): deny write_file to .git and persistence vectors in fix agent (#7908) * fix(ci): deny write_file to executable configs and node_modules in fix agent (#7908) * fix(ci): sandbox verification execution and isolate the PAT-bearing push (#7908) * fix(ci): clarify verification sandbox flags harden, not mirror, the CLI defaults (#7908) * fix(ci): sandbox resolve-owning-packages, re-stage gates, deny lockfile writes (#7908) * fix(ci): make hygiene graceful-failure reachable, gate findings schema, scope artifacts (#7908) * fix(ci): disable core.fsmonitor in gate, deny vite.config writes, align scanner wording (#7908) * test(ci): add a repo-hygiene workflow test suite (#7908) * fix(ci): match test assertions to workflow YAML quoting style (#7908) * fix(ci): use double-quoted YAML if-conditions to match test assertions (#7908) * fix(ci): mount hygiene WORKDIR read-only during sandboxed verification (#7908) --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.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> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |