mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-23 23:55:50 +00:00
65 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Shaojin Wen
|
313f191150
|
fix(autofix): make the brake's BLOCKED handoff a first-class round outcome (#9297)
* fix(autofix): make the brake's BLOCKED handoff a first-class round outcome When the growth brake fires, feedback.md tells the address agent to stop BLOCKED with a handoff — but the output contract only accepted address-summary.md or no-action.md, so a round that followed the instruction died as 'finished without required output file(s)', the brake's decision text was buried under a generic failure.md, the report said 'could not produce a passing fix', and the job left a red review-address check that the next scan counts as new feedback. Observed on #9222 rounds 6/7. The handoff becomes a first-class verdict end to end: run-agent.mjs honors an agent-written handoff.md (with no fix verdict) as a graceful exit the way it already honors failure.md, and shields it from the API-error retry reclassification; the verification gate reports outcome=handoff for a no-commit round with a handoff and no failure.md; finalize lets handoff pass without failing the job; the report step runs for this outcome, posts the handoff note with the eval marker (watermark advances — the feedback is consumed as evaluated), and names the stop honestly instead of reporting it as a failed fix. The skill now tells the agent exactly which file to write when the brake fires. A coexisting spec output still outranks the handoff, and failure.md coexistence keeps the failed classification, so crash paths are unchanged. * fix(autofix): align the handoff outcome's consumers and pins with its contract (#9297) Review found the new handoff outcome breaking two pinned helper tests (stale breaker-headline wording, unclassified headline in the fleet-shepherd contract test), misreporting handoff rounds in the status-comment finalize step, and leaving the whole handoff chain unpinned against mutation. - Update the breaker headline pin to the PR's reworded headline. - Classify the handoff headline as transient in the shepherd contract test and drop its "AutoFix stopped" prefix so the shepherd's terminal-only REASON regex cannot capture a transient stop (the shepherd workflow itself stays outside this round's footprint). - Include handoff in the Finalize-status published-report branch. - Give deliberate stops their own takeover-digest census bucket instead of the residual crash/infra bucket (EN + ZH). - Neutralize :: workflow commands at the two new handoff echo sites. - Use the runner's non-empty missing() convention for handoff.md so an empty file cannot read as a verdict in one layer and not the other. - Correct the run-agent.mjs precedence comment: when a handoff coexists with a spec output, the gate (handoff branch first) decides the round, matching the documented "handoff + no-action -> handoff" contract. - Pin the handoff chain where its siblings are pinned: finalize replay, POST_HANDOFF replay, mark/headline replays, the gate's no-commit decision table, the stub-runner handoff/empty/API-error cases, the report-step if-clause, and the census needle-to-emit cross-pins. * fix(autofix): classify a no-commit handoff before the gate's structural checks (#9297) Review proved the new handoff classification unreachable exactly where the brake fires: the structural pre-checks (core rebuild, settings schema, contracts) judge the PR's own diff and reject before the no-commit fork, and the growth brake fires on precisely the red PRs whose diff trips them. A compliant handoff (no commit, only handoff.md) then classified as a retryable failure, so the repair pass deleted handoff.md and could commit against the brake's explicit stop — the self-feeding loop the handoff exists to prevent. Reproduced with the real gate script: schema-check-fail + no-commit handoff exited 1 with no outcome=handoff. Move the no-commit handoff classification above the structural checks (right after the failure.md exits, which keep their precedence). A handoff claims nothing — acted=false, deferred to a human — so the checks' false-no-action rationale does not apply, and the retryable/ repair machinery must never engage on a round the brake told to stop. The no-op fork reverts to no-action-only classification. - Add a gate test: stale schema + no commit + handoff.md classifies outcome=handoff, exit 0, no retryable (fails on the pre-fix gate). - Pin the handoff-note :: workflow-command neutralization in both layers (the gate's sed and the runner's replaceAll), which review showed were surviving mutations. * fix(autofix): reject a no-commit handoff written over a dirty workspace (#9297) * fix(autofix): report a dirty-handoff rejection honestly, not as a failed fix (#9297) * fix(autofix): reject a handoff written beside a round commit, non-retryably (#9297) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): classify the committed handoff shape as its own non-retryable outcome R7-1 on this PR: a round that HAS a commit beside handoff.md skipped both brake-violation guards (clean tree misses the dirty guard; committed ref misses the no-commit branch) and fell through to the structural checks, where reject_fix defaults to retryable and the repair pass deletes handoff.md and may commit again against the brake's stop. Classify it before the structural checks under its own outcome committed_handoff, sibling of dirty_handoff: non-retryable, its own honest report headline (reusing dirty_handoff's wording would claim nothing was committed when a commit exists), listed among the report-publishing outcomes in the status classifier, and never routed through finalize's pass list. Pins updated in the same pass: the committedWithHandoff gate case now expects committed_handoff with no retryable, the shepherd contract test classifies the new headline as transient (loop stays engaged), the status-classifier pin names all five outcomes, and the handoff-contract gate test gets an explicit subprocess budget (eight fixture arms outgrew the 5s default). * fix(ci): count committed-handoff rounds in the milestone census rejected bucket (#9297) * fix(autofix): publish brake violations green and preserve handoffs across crashes (#9297) Two Critical review findings on the handoff output contract. Brake-violation rounds (dirty_handoff / committed_handoff) ended with a red review-address check: the eval marker stamps ts=NEWEST, strictly before the check completes, and the scan counts failed checks completed after the watermark — including this workflow's own review-address checks — as new feedback. The next scan re-selected the PR and burned a full agent round on the item the posted headline promised not to retry, once per violation. Admit both outcomes to the green finalize arm the way the clean handoff already is (the diff's own comment names this self-feeding loop as the reason handoff went green), and key the report step's routing and POST_HANDOFF trigger on the outcomes themselves so the green rounds still publish their honest headline, handoff note, and eval marker instead of going silent. A crash, budget kill, or loop guard after the agent wrote handoff.md synthesized a failure.md that shadowed the note: the gate reads failure.md first (outcome=failed), the report preferred it, and the timeout sentinel re-handed the item the brake stopped. Preserve the agent-written handoff in the crash branch (exit 0, mirroring the agent-written-failure.md arm), and never let writeHandoff overwrite a non-empty agent verdict. Both findings reproduced against this commit's verbatim code before fixing: the case/jq replay showed the violation check red and counted as new feedback, and a stub run showed the synthesized failure.md shadowing the handoff. New behavioral tests fail pre-fix and pass post-fix. * test(ci): give four subprocess-heavy replays explicit budgets The milestone digest, stale-duplicate revalidation, deny-by-default footprint, and recoverable-API-render tests spawn multiple bash replays of the real workflow/gate scripts each; the files those replays parse grew with this PR's handoff chain, and all four outgrew the 5s default (each verified to pass with an explicit 30s budget, matching the suite's convention for subprocess-heavy tests). * fix(ci): mirror the handoff outcome consumers into the recovery clone (#9297) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
Shaojin Wen
|
3b3818db87
|
fix(ci): keep qwen-autofix.yml under GitHub's 500 KB start-runs limit (#9517)
GitHub does not start runs for a workflow file larger than 500 KB (512,000 bytes) and reports nothing when it stops. qwen-autofix.yml crossed that line on 2026-08-19 at 512,782 bytes: schedule ticks stopped firing, every workflow_dispatch sat "queued" forever with zero jobs and could not be cancelled, and issues/issue_comment went quiet — while pull_request_review runs kept succeeding, because a PR event resolves the workflow from the PR's own branch and those carry older, smaller copies of this file. The loop therefore looked half-alive and stayed dark for a day. Move 75 long comment blocks (1,326 lines) verbatim into a sibling design record, .github/workflows/qwen-autofix.md, leaving each block's opening lines plus a `qwen-autofix.md#af-NNN` pointer where it sat: 518,055 -> 426,437 bytes. No executable line changes — the YAML parses to an identical document outside `run:`, every `run:` script still passes `bash -n`, and the only lines removed anywhere are comments. Steps that are duplicated verbatim across jobs share one pointer so they stay byte-identical. Add .github/scripts/check-workflow-size.sh (gate at 470,000 bytes), wired into CI on every profile: a .github-only PR classifies as `github_ci_only` and skips the `full`-only checks, which is exactly the PR that can trip this. Tests pin the gate, every workflow's size, and pointer/section symmetry. Delete qwen-autofix-recovery.yml. It was cloned during the incident on the theory that the workflow ENTITY was wedged, but it carried the same oversized file, so its dispatches queued identically and its schedule never fired. |
||
易良
|
517a2bfc28
|
fix(triage): compute the flake-gate diff before the env -i re-exec (#9468)
* test(triage): pin the parent-computed diff and the child copy Update the record-step pins to the RUNNER_TEMP-staged file and add a pin asserting the scrubbed child copies it rather than re-running git. * fix(triage): compute the flake-gate diff before the env -i re-exec The scrubbed (env -i) child cannot read the shallow merge-ref objects: git global safe.directory lives under HOME, which env -i strips, so git refuses to read the base commit and the diff fails with "Could not access <base-oid>". Compute the NUL-delimited diff in the parent (normal environment, no PR code executes) and stage it under RUNNER_TEMP; the clean child copies it into the root-only gate home. * fix(triage): rm -f before the parent diff redirect, slash-path the parent commands * test(triage): pin diff-before-re-exec ordering and slash-pathed commands * fix(triage): harden the flake-record staging path against directory and symlink plants |
||
|
易良
|
0530cb04df
|
fix(triage): diff the flake-gate file list against the pinned base OID (#9464)
The "Record changed test files for the flakiness gate" step runs its `git diff 'HEAD^1' HEAD` inside the env -i scrubbed child. On the persistent pool, resolving the `^1` parent there intermittently fails with "Could not access 'HEAD^1'" — the shallow merge-ref object store is left unreadable by prior `--depth=2` fetches — which takes down the whole verify lane. The "Pin agent inputs" step already captures the base OID to $RUNNER_TEMP/verify-base-oid while .git is still root-owned. Diff against that content-addressed OID instead of re-resolving the parent: it needs no parent walk and is the same value the workflow already trusts for its post-build re-pin. Add a pin asserting the record step reads the recorded OID rather than re-deriving HEAD^1. |
||
|
Shaojin Wen
|
f0dcdfc157
|
feat(triage): add a deterministic flakiness gate to sandboxed verification (#9130)
* feat(triage): add a deterministic flakiness gate to sandboxed verification Closes #9125. PR #9086's ~50% mtime-assertion flake passed every automated layer because each executed the changed tests exactly once — a coin flip a single green run cannot distinguish from health. The gate re-runs the PR's added/modified unit-test files N times (default 5, vars.QWEN_VERIFY_FLAKE_ROUNDS to override, clamped to 2..10) through the same entry points CI uses and compares outcomes per group across rounds. Design constraints, each pinned by a workflow test: - One-way authority: 'flaky' demotes the published headline (even a trusted agent merge-ready); no gate value can raise or soften one. The gate runs the PR's own test code, so it can always be neutered — but a gate that can only demote is not worth forging. - Divergence-only signal: a group failing identically every round is deterministic (CI owns it) and an environment-sensitive suite must not false-positive here; both report informationally, never demote. - Fail open: the gate is not under -e and every terminal path exits 0 — a gate bug reports verdict 'error' instead of taking down the verify lane. - Honest file list: recorded from HEAD^1..HEAD before install/build hands the workspace (and .git) to PR lifecycle code; the gate consumes the root-owned recorded list and never re-derives the diff. - Untrusted text stays out of outputs: summaries are fixed text plus counters; PR-controlled paths live in flake-gate.log, embedded through the publisher's escaping emit_block. Job timeout raised 150 -> 175 for the gate's ~25m worst case (15m round budget checked before each invocation + one 10m-capped in-flight run). * fix(triage): survive the runner wrapper's -e, per-file gate granularity, hardened log staging Round-1 review + sandboxed-verify feedback, all seven findings: - set +e after set -uo pipefail: the runner wraps every run: block in 'bash -e -o pipefail' and set -uo does NOT clear that inherited -e, so the first failing test invocation killed the step — fail-open inverted to fail-closed for exactly the flaky/consistent-fail populations the gate classifies (verify cells C/D). An EXIT trap additionally converts any abnormal ending (set -u death) into the fixed 'error' verdict. - Per-FILE groups: one runner invocation per changed test file, so a consistently failing file can no longer mask another file's run-to-run divergence behind a shared exit bit. - Owning-package resolution: nearest ancestor package.json (nested workspaces like packages/channels/base are entered themselves) plus a vitest-config probe; unsupported runner families (packages/desktop's bun test) and */e2e/* specs are logged out-of-scope instead of being mis-run as permanent consistent-fail noise. - Operands are ./-prefixed before %q, so a checked-in filename beginning with '-' (e.g. --config=x) can never be parsed as a runner option. - Log staging moved to a dedicated always() root step after the agent exits — the last write to verify-results/flake-gate.log — and the publisher pins that exact path instead of find|sort|head, so an early agent abort cannot lose the matrix and agent-era PR code (which owns a chowned verify-results) cannot control or shadow what is embedded. - Detection math corrected: N=5 catches a 50/50 flake with ~94% (1 - 2*(1/2)^5), not ~97% — all-pass and all-fail rounds both miss. - New behavioral suite executes the extracted gate and publisher fragments under the production wrapper itself (bash --noprofile --norc -e -o pipefail) with scripted per-file P/F sequences: pass, flaky-next-to-consistent-fail, consistent-fail, missing-list error, out-of-scope n/a, nested-package + leading-dash operand, and the seven-value one-way demotion — closing the structural blindness where YAML-string tests stayed green while the shipped behavior regressed. * fix(triage): isolate flake-gate rounds, classify infra exits, widen runner resolution (#9130) Review-round fixes for the deterministic flakiness gate: - Reset shared state between rounds (restore tracked files, tear down test-user processes, fresh per-invocation TMPDIR) so a deterministic test cannot fail on its own residue and fake a divergence (R1-8). - Classify timeout/signal exits (124, 128+N) as infrastructure, not F marks, and report the informational timeout verdict instead of a fake flaky (R2-3/R3-2). - Resolve the vitest runner by owning package + vitest's real config list (vite.config.* included), keyed on the package lookup instead of a packages/* prefix, so webui and integrations workspaces are re-run instead of skipped (R2-2/R3-3). - Narrow the scripts/tests arm to the pinned config's *.test.{js,ts} include set so admitted-but-rejected files are skipped, not mis-run into a bogus consistent-fail (R3-11). - Harden the gate-log staging: kill leftover build-user processes, and remove a planted destination entry before copying so a FIFO/symlink can neither hang the copy nor redirect it (R1-5). - Cap the embedded gate log at 10000 chars to keep the assembled comment under GitHub's 65,536-char limit (R3-4). - Record changed files with core.quotePath=false so non-ASCII test filenames are not silently dropped (R3-5). - Behavioral tests: hermetic timeout/pkill stubs (the suite no longer depends on GNU coreutils, fixing the macOS red), infra-exit and round-reset scenarios, trap-abort fail-open, node --test arm, FLAKE_ROUNDS clamping, fixed-shape summary, record-step shape pins. * test(triage): follow the widened special-file strip into the vitest twin pins Commit |
||
|
易良
|
19a4b973eb
|
fix(ci): back-port the checkout-heal wipe guard to the triage and serve-ab wipes (#9277)
* fix(ci): back-port the checkout-heal wipe guard to the triage and serve-ab wipes The "empty the workspace, keep the directory" idiom exists in three copies; only the review workflow's copy received the #9220 hardening (canonicalization, trailing-slash strip, RUNNER_WORKSPACE allowlist). Measured on main for #9265, the two triage guards let non-canonical spellings of the guarded roots through (/home/, /home/., //usr, /root/, /var/ all reached the rm), and serve-ab's wipe had no guard at all — even `/home` or an empty string arrived at `find … -exec rm -rf`. Port the reference guard to all three sites, keeping each site's exit contract: triage fails loud both before and after external code, serve-ab stays bare under the job's `-eo pipefail` so an unclearable workspace fails before either checkout builds on top of the leftovers. Pin each ported copy with its own tests: bad-path batteries under an rm recorder (the destructive primitive cannot fire under any edit), an allowlist-escaping `..` case gated on a GNU-realpath host probe (the lesson from 90fa6bb4), a realpath-absent trailing-slash RUNNER_WORKSPACE case, and text pins on the ported layers. Every pin was mutation-verified red against a deletion of the layer it guards. * test(ci): pin guarded serve wipe * fix(ci): close wipe guard fallback gaps * fix(ci): fail closed without realpath * fix(ci): keep wipe guards portable * test(ci): pin wipe-guard RWS layers and unmask the pre-run battery - run the rewritten pre-run sweep battery under -e -o pipefail so a failing sweep can no longer report success (bare bash -c masked it) - pin the RWS '..' refusal and degenerate-root refusal text in all copies, and add RUNNER_WORKSPACE='/' exec cases to both copy suites - exercise both pre-run and post-run copies in the realpath-absent refusal test - replace the '..' escape vector with a symlink escape that only the realpath line can refuse, and correct the mutant-outcome comments - add the serve-ab wipe-before-checkouts ordering pin from the sister suite and a happy-path RWS canonicalization pin * test(ci): correct wipe-guard mutant-outcome comments for find -P The symlink-escape comments claimed that with the WS realpath line deleted, find reaches rm through the link target. GNU find's default -P mode does not descend symlink operands: the mutant passes every guard, wipes nothing, and exits 0, so only the non-zero-status assertion catches it — the rm-log assertion passes vacuously. Reword both twin comments (R5-1). --------- Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com> |
||
DennisYu07
|
ba2d512497
|
feat: chain Terminal-Bench release evaluation (#9120)
* feat: chain terminal bench release evaluation * fix: gate terminal bench on release publication * test: add one-task terminal bench release smoke * test: run one SWE and TB task for release smoke tags * fix: publish EAS cache under runtime lookup tag * fix: pin smoke releases to published Qwen version * fix: require explicit prerelease Qwen reference * ci: cover terminal bench manifest and harden release inputs |
||
|
易良
|
d8b5d532c6
|
fix(ci): minimize new spam comments on creation (#9266)
* fix(ci): run spam cleanup every five minutes * fix(ci): minimize spam comments on creation * fix(ci): tolerate deleted spam comments * fix(ci): preserve live spam comments * test(ci): evaluate spam minimizer jq filter * fix(ci): handle deleted spam comments * test(ci): pin spam minimizer guards * test(ci): pin spam guard parentheses |
||
|
Shaojin Wen
|
a9bff6c9b8
|
feat(autofix): defer verified out-of-footprint findings to a surviving follow-up queue (#9189)
* feat(autofix): route verified out-of-footprint findings to a surviving follow-up queue
Anti-drift closure for the review loop: a finding that is REAL but whose
fix lies outside the PR's footprint previously had only wrong outcomes —
implement it (scope drift), decline it (the finding is lost when the PR
merges and nobody re-reads its threads), or push it onto a maintainer.
- SKILL gains the fourth disposition, Defer to follow-up: verified +
out-of-footprint → record {id, path, reason} in
deferred-findings.json, reply on the thread that it is deferred, leave
the thread open. Decline stays for what is not worth doing anywhere;
defer is for what is worth doing elsewhere.
- The report step upserts these into one per-PR 'Deferred review
findings' issue (marker-keyed, append-only by rc id, agent text
token-neutralized and length-capped), for BOTH pushed and no-op
outcomes. Best-effort: an upsert failure never fails a round.
Deliberately no ready-for-agent label — feeding the bot's own
deferrals back into its issue queue is a human authorization.
- deferred-findings.json rides the artifact dump and the repair
cleanup; the neutralization ledger grows to ten sites.
- Tests: shape validation (non-empty array of numeric-id items), line
building (dedupe by rc id against the existing issue body, newline
flattening, truncation), and wiring pins for both call sites.
* fix(autofix): rebuild the deferred-findings upsert per review round 1
- Extracted to a trusted staged script callable from ALL outcome paths —
the failure/handoff path persists verified findings too (a failed
round's commit dying says nothing about the findings' validity).
- Append-only durability: the tracking issue's body is written once;
every later round POSTS a comment — no read-modify-write can race a
maintainer's edits, and a failed body/comments read SKIPS the round
(never mistaken for empty history). Success is logged only when the
write call succeeded; failures say NOT persisted.
- Structured lookup: jq filtering over the real bodies (no line-joined
awk under pipefail), pull requests excluded, lookup failure skips
rather than creating duplicates.
- Dedupe is line-anchored ('- rc:<id> ' at line start, body+comments
corpus) with intra-batch unique_by; ids the round resolved in code are
excluded (a finding cannot be implemented and outstanding at once).
- Shape gate covers path (string when present); path bytes are
charset-sanitized so a crafted path cannot forge queue bullets.
- Publication-trust posture recorded: the deferred lines are the same
agent-authored trust class as every other published output — marker
neutralization, mention-free sanitized charset, length caps, and a
20-item batch cap bound the surface.
- Tests: the real script runs against a recording gh stub — create,
append+dedupe (body and comments), PR-carrying-marker exclusion,
anchored dedupe vs free-text mentions, read-fail skip,
resolved-exclusion, shape-gate loudness, write-fail honesty, and
forged-path sanitization; the neutralization ledger returns to nine
workflow sites with the script-side tenth pinned in place.
* fix(autofix): harden deferred-findings upsert per review round 2
- Pass the known-id corpus to jq via --rawfile: a large corpus in one
--arg argv element hits Linux MAX_ARG_STRLEN and the swallowed exec
failure would silently drop the round's deferrals.
- Digest-gate the staged upsert script: record upsert_sha256 at stage
time (expression context) and verify before each of the three
invocations; RUNNER_TEMP is agent-writable in between. A mismatch
skips persistence, never the round.
- Add the gh hygiene preamble (GH_HOST pin, GH_TOKEN unset, fresh
GH_CONFIG_DIR) to the review-address failure/handoff report step —
the one PAT-bearing gh step that lacked it.
- Query the tracking-issue lookup with state=all so a maintainer-closed
issue is appended to instead of forking a duplicate.
- Enforce integer positive finding ids in the shape gate (a float id's
dot is a regex wildcard in the anchored dedupe and never
index()-matches resolved ids).
- Clip the 20-item batch loudly and qualify success messages with
kept/total counts instead of claiming full persistence.
- Tests: digest + hygiene wiring pins; stub knobs for list/comments
fetch failures; append-write failure, multiline-reason flattening,
bad-id, loud-cap, and state=all cases.
* fix(autofix): close bash/transport channel gaps and failure-path gate holes (review round 3)
- Sweep BASH_ENV/ENV and imported BASH_FUNC_*%% functions plus proxy
(HTTPS_PROXY/HTTP_PROXY/ALL_PROXY + lowercase) and SSL_CERT_FILE/DIR
at all four gh-hygiene sites: both families are GITHUB_ENV-plantable
and bypass the TRUSTED_PATH pin (child-bash startup) or reroute/
decrypt PAT-bearing HTTPS. Also de-shadow gate-critical names and
hash -r, since a planted BASH_ENV runs before the step body.
- Pin PATH to the staged trusted value (guarded for pre-stage crashes)
and drop the loader trio in the failure/handoff report step — its
digest gate previously ran under ambient PATH/LD_PRELOAD.
- Failure-path upsert: skip with a plain notice when stage never ran
(empty digest is not a tamper alarm), and verify the PAT's bot
identity before writing (POST_HANDOFF's check is skipped on the
fixed/noop-outcome path); correct the guard comment that claimed
parity with the handoff guards.
- Fold the twice-pasted digest-gate + upsert block in 'Push and report'
into a step-local run_deferred_upsert(), matching the
resolve_and_reply_threads convention.
- Dedupe corpus reads bot-authored comments only, so a third party
commenting on the public tracking issue cannot suppress a finding.
- Tests: hygiene sweep pins ordered before the first gh call; placement
assertions (function defined once, called after both resolve arms;
failure invocation inside the DRY_RUN/STALE/token guard slice);
digest/identity/notice pins; behavioral cases for foreign-author
suppression, intra-batch duplicate ids, markerless-issue create path,
creator/marker anchors, and marker neutralization through the append
path.
* fix(autofix): isolate deferred-upsert in a clean env -i child, drop the unsound in-shell sweep (review round 4)
Round 4 (R4-1, five Criticals of one class) showed the in-shell
BASH_FUNC/proxy denylist sweep the prior round added is unsound: it
bootstraps trust from the very shell namespace it sanitizes, and a
planted BASH_FUNC_env%%/unset%%/command%%, an expand_aliases alias, a
readonly -f shadow or a DEBUG trap each defeat it — ending in the
staged upsert script executing with CI_DEV_BOT_PAT in env.
- Replace it with sound isolation: both upsert sites (run_deferred_upsert
in 'Push and report', and the failure/handoff path) run the digest
gate, the PAT identity check and the staged script in a fresh
'/usr/bin/env -i … bash --norc -c' child. /usr/bin/env is invoked by
absolute path — bash never does function/alias lookup on a
slash-bearing word, so a planted BASH_FUNC_env%% cannot intercept it —
and env -i drops every BASH_FUNC_*, BASH_ENV, SHELLOPTS, alias and
trap before any gated work. GH_CONFIG_DIR is minted inside the clean
child (its mktemp cannot be shadowed there), closing the mktemp-shadow
hole in the failure step's preamble.
- Remove the sweep and the in-step gh/PATH preamble the prior round
added to all four PAT gh steps; the three pre-existing steps revert to
their prior posture. Hardening the pre-existing PAT gh calls
(handoff/report comment, push, publish) against BASH_FUNC/transport
plants is noted as separate, out of this feature's scope.
- Script: accept a contract-valid empty array as a clean no-op instead
of a false 'malformed' alarm; 'set +C' so a planted read-only
SHELLOPTS=noclobber cannot silently empty the dedupe corpus (belt to
the env -i child that already drops SHELLOPTS).
- Tests: replace the sweep pins with clean-child pins (absolute-path
env -i at both sites, GH_CONFIG_DIR/PATH inside the child, failure
launch inside the guard slice); add no-findings exit-0, empty-array
no-op and set +C cases. Behavioral probe: a fully tainted parent
(BASH_FUNC/alias/BASH_ENV plants) cannot reach into the env -i child.
* fix(autofix): strip LD_* before the env -i upsert child; tighten id gate, mktemp/cap guards (review round 5)
- R5-1 (Critical): LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH is the one channel
env -i cannot block — ld.so maps a planted library into /usr/bin/env
itself at execve, before -i wipes anything. Neutralize it with a
command-prefix assignment (LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH=)
before /usr/bin/env at both upsert launch sites: a pure shell
parameter assignment no BASH_FUNC can shadow, applied to env's own
environment. Probe: a parent LD_PRELOAD=/evil.so no longer reaches the
env binary or the child.
- R5-3: shape gate rejects integer-valued floats jq renders in
scientific notation past 2^53 (1e21 -> "1E+21") — the '+' is a
regex-active byte in the anchored dedupe. Add a <2^53 bound and a
tostring plain-digits belt.
- R5-4: guard mktemp failure (a known /tmp-exhaustion CI state) so it
warns and skips instead of a silent exit 0 that violates the header
contract.
- R5-2: the cap warning no longer promises 're-defer in a later round'
(impossible — the eval-watermark filters evaluated feedback out
permanently); it names the dropped bullets for a maintainer.
- Tests: LD prefix, GH_HOST-in-child and gate/exec ordering pins; tie
the two near-verbatim clean-child bodies together (R5-6); source pins
for both --paginate sites (R5-5); if-!-echo gate-condition pin (R5-9);
failure step added to the sweep-removal regression loop (R5-11);
behavioral cases for sci-notation id, mktemp failure, prefix-colliding
dedupe boundary (R5-10) and the reworded cap.
* fix(autofix): defuse mentions in deferred bullets, verify child liveness (review round 6)
- R6-9 (Critical): the reason is agent-influenced prose published under
the bot identity, so a raw @ fired real mentions from the tracking
issue. Defuse before rendering: @ gets a trailing ZWSP, and the entity
spellings GitHub decodes BEFORE its mention filter (@ @
@ @) get their & escaped. Both measured inert against the
real renderer; \@ and leaving & alone are not. Paths were already
charset-reduced. Byte-exact probe: 2/2 @ defused, all four entity
spellings escaped, no raw spelling survives.
- R6-8: LD_* cannot be enumerated — LD_TRACE_LOADED_OBJECTS is
presence-tested, so even the empty prefix assignment leaves trace mode
on and /usr/bin/env prints its libs and exits 0 without ever running
the child (probed). Verify the RESULT instead: the child prints a
liveness sentinel first and its absence is reported. The inspection
uses bash builtins only — an external grep would itself
print-and-exit-0 under trace mode, neutering the check (measured: the
first grep-based attempt failed exactly this way).
- R6-4: guard the child's own GH_CONFIG_DIR mktemp; an empty value falls
back to the shared ~/.config/gh.
- R6-6: a line-builder (jq/sed) failure warns instead of exiting
silently as 'nothing new' — the last path skipping the header
contract.
- R6-3: write-failure warnings say the findings are LOST (watermark-
gated, never retried) and name the bullets, matching the round-5 cap
wording fix.
- Tests: allow-list entry pins (R6-7), sentinel/builtin-inspection and
child-mktemp pins, identity-check ordering (R6-5), plus behavioral
cases for mention defusing, reason-type and id-0 gate clauses (R6-2)
and line-builder failure.
* fix(autofix): defer findings from all three feedback sources; carry them across repair (review round 7)
- R7-1 (Critical): only inline comments carried an id in feedback.md, so
a verified out-of-footprint finding raised in a review body or an
issue-level PR comment could not be deferred and was lost at merge.
Feedback now renders [rv:<id>] and [ic:<id>] alongside the existing
[rc:<id>], the record takes an optional "source", and bullets anchor
under a per-source prefix so id spaces cannot collide. The resolved-id
exclusion stays inline-only (that is what resolved-comments.txt holds).
SKILL documents all three sources and that only inline findings have a
thread to reply on.
- R7-2 (Critical): every abort path said "skipping ... this round",
implying a retry that cannot happen (the eval watermark filters this
round's feedback out of every later round and the next reset wipes the
file). All six now report the findings as LOST and dump the raw
deferrals for manual recovery, with :: neutralized — the dump is
agent-influenced and a raw :: at line start is a workflow command.
- R7-3 (Critical): 'Repair deterministic rejection' deleted
deferred-findings.json before any upsert site ran, so run 1's
deferrals died in a repaired round. It now carries them into a sidecar
the upsert unions in (merging if an earlier repair left one), and the
sidecar rides the artifact dump. A/B probe: base arm loses run 1's
deferral, fix arm persists both.
- R7-4: a present-but-non-string path (false) passed the gate because //
treats false as absent; the gate now tests .path|type directly.
- R7-5: LD_PROFILE/LD_PROFILE_OUTPUT/LD_DEBUG/LD_DEBUG_OUTPUT are
non-blocking loader file-write channels the liveness sentinel cannot
catch, so they join the command-prefix neutralization (probed inert
when empty).
- Tests: per-source anchoring and dedupe, unknown-source rejection,
path:false, carry-only and carry-merge cases, LOST dump with ::
neutralization, plus wiring pins for the carry, the feedback ids and
the extended LD prefix.
* perf(autofix): bound the deferred-issue lookup and name gh failure causes
Clears the two backlog items the review has re-raised every round since
round 2 (R2-6, R2-10); both live in the file this PR adds.
- R2-6: the tracking-issue lookup ran a full --paginate over every issue
the bot has ever opened, on every round that defers anything, keeping
only the first match. It now walks newest-first pages and stops at the
first marker match: one request in the common case, a short page ends
the scan (corpus exhausted -> create), and a 10-page cap bounds the
worst case. Reaching the cap without a match SKIPS rather than opening
a second tracking issue for the same PR.
- R2-10: every gh call discarded stderr, so a rate limit, an expired PAT,
a transport error and a 404 rendered identically in the feature's only
signal. All five calls now capture stderr to one sink and the warnings
name the cause, :: neutralized like every other echoed API/agent
content.
Measured on the shipped script with a recording gh stub: first-page hit
1 request, empty corpus 1 request + create, page-2 hit 2 requests, cap
10 requests + skip, and the 403/401 bodies reaching the warning text.
* test(autofix): close the pin gaps the round-8 mutation sweep found
Round 8 raised 15 findings, none Critical: two behavioural, the rest
test pins the reviewer proved vacuous by mutation.
- R8-3: the carry-merge failure branch discarded THIS run's deferrals
with no raw dump — the one loss path in the feature without recovery
output. It now prints the set (:: neutralized) before deleting it.
- R8-1/2/5/6/7/8/9/10/11/12/13/14/15: pins that survived their own
mutations. Notably: the negative sweep pins now assert the PROPERTY
(no BASH_FUNC / unset -f / hash -r in non-comment lines) instead of
round 3's exact spelling; allow-list entries must sit inside the
env -i argument list, not merely somewhere in the step; the identity
check is pinned whole so a fail-OPEN mutation cannot pass; the repair
cleanup's deletion pin is spelling-independent and allows exactly the
one delete that follows a merge; the staging pins are scoped to the
stage step with cp ordered before the digest record; and the script's
<!-- escape site gets the count+canonical treatment its workflow
siblings already had.
Each new pin was mutation-verified: 8/8 injected regressions turn the
suite red (differently-spelled sweep, relocated allow-list entry,
deleted GH_CONFIG_DIR export, fail-open identity check, re-added
cleanup deletion, deleted re-print loops, ascending lookup order,
no-op sed spelling).
* fix(autofix): close the upsert TOCTOU and the per-id deferral collapse (review round 9)
- R9-1 (Critical): the digest gate was check-then-use — sha256sum read
the staged path and bash re-opened it, two opens of a path this PR
itself calls agent-writable. The child now reads the script ONCE and
runs those exact bytes (bash -c "$UPSERT_SRC"), so the bytes hashed
are the bytes executed. A/B against an inotify-driven same-user
rename(2) watcher: old shape 20/20 payload executions with the gate
never firing, new shape 0/20 (legit 20/20, control 20/20).
- R9-2 (Critical): unique_by([source, id]) collapsed DISTINCT findings
sharing one review-body or issue-comment id — the two sources this PR
adds — and reported success while losing them. Dedupe identity and the
corpus check are now per rendered line for those sources (inline
comments keep id identity and the cross-round anchor). Probe: two
findings under one review id now both persist ("3 of 3 new"),
byte-identical records still collapse, inline behaviour unchanged.
- R8-4 (re-raised): fixed structurally instead of by the suggested
prefix entry, which is a no-op — LD_SHOW_AUXV is presence-tested, so
an empty assignment still dumps 22 auxv lines (measured; env -u does
not help either). Loader side channels write to the LAUNCH process's
stdout, so that stdout is discarded and the child logs to a private
file; path and read-back are fork-free ($$ expansion, $(<file)) so a
polluted parent cannot leak noise into the value. Measured: with
LD_SHOW_AUXV planted the log holds 0 auxv lines and the child still
runs; with LD_TRACE planted the sentinel is absent and the warning
fires.
- R9-3/4/5: the manual-recovery dumps now say when they truncate, and
name the full byte count.
- R9-11: the artifact dump neutralizes :: in the agent-written files it
prints, like every other echo of them.
- Tests: pins for the single-read exec, the private log, the fork-free
parent handling, the identity check's ENFORCEMENT (R9-9), an
allow-list that must hold ONLY the sanctioned entries (R9-10), the
sentinel comparison inside the re-print loop (R9-13), and R9-2's
multi-finding cases. Also fixes an argList slice that anchored on a
comment mention and silently widened to the whole step.
* fix(autofix): keep a poisoned carry from sinking the round; clear the pin backlog
Clears the eleven items carried from round 9.
- R9-18: a carried sidecar that PARSES but fails the shape gate used to
abort this round's valid deferrals too — asymmetric with the
unparseable-carry branch, which persists this round only. The gate is
now a function applied to the merged set, with a retry on this round's
own file; the carry is dumped and named LOST. Measured on the real
script: valid own + gate-invalid carry -> own persisted, carry dumped;
valid own + unparseable carry -> own persisted; invalid own -> loud
total abort, nothing written.
- R9-20 / R9-7: the union argument order IS the freshness guarantee
(jq unique_by keeps first-of-group in original order), pinned at both
sites; measured: a duplicate id keeps this round's text, not the
carried one.
- R9-3/4/5 follow-up: the three truncation dumps became one dump_file
helper instead of a fourth copy.
- Pins: --paginate anchored to the comments call (R9-8), the stale
two-sites comment corrected (R9-6), the no-in-shell-sweep property
widened past function-unset spellings to alias/trap/proxy forms
(R9-12), the repair cleanup's deletion pin extended to rm -rf and to
any second multi-line list (R9-16), runUpsert's spawnSync bounded like
its sibling harness (R9-17), the explicit review_comment spelling
covered (R9-19), and the 20-item cap's survivor set pinned from a
MEASURED run whose sort order and input order disagree (R9-14) — the
four records written first are the ones dropped.
Mutation-verified 6/6: relocating --paginate, an alias-form sweep, an
rm -rf deletion, either union order swapped, and dropping the
poisoned-carry fallback all turn the suite red.
* test(autofix): widen the denylist-sweep guards past their word-boundary hole
`\btrap -\b` cannot match `trap - ERR EXIT`: the boundary sits between
`-` and a space, both non-word characters. The same hole was in two
sibling guards — `\bunset -f\b` misses `unset -fv name`, and
`\bhash -r\b` misses any suffixed spelling. Drop the trailing
boundary on all three.
Mutation-verified: injecting `trap - ERR EXIT INT TERM`, `trap -- EXIT`
or `unset -fv sha256sum` into a PAT-bearing step now turns the suite
red; each passed before.
* fix(autofix): two -e-fatal paths, an escape-order dedupe hole, and a rewording duplicate (review round 10)
Six Criticals; four were defects this PR introduced.
- R10-19 + R10-22 (Critical): the PAT steps run under 'bash -eo
pipefail' (defaults.run.shell: bash). Measured: 'rm -f' on a planted
DIRECTORY at the predictable log path exits 1 and kills the step, ': >'
onto one likewise, and $(<missing) is fatal in a way NEITHER '|| true'
NOR 'if !' rescues. Creation is now 'rm -rf' + '(set -C; : >)' with a
warn-and-skip, and the read-back tests -f/-r first while staying
fork-free. Probed all four planted shapes (fresh/file/symlink/dir):
the step survives each.
- R10-5 (Critical): the <!-- neutralization ran in a sed AFTER the jq
corpus comparison, so an rv/ic line carrying <!-- compared its RAW
rendering against the ESCAPED stored form — never matched, republished
every round. Escaping moved inside jq, before the compare.
- R10-17 (Critical): rv/ic identity was the exact rendered line, so any
reworded re-emission (routine: the repair flow re-runs the agent)
published a permanent duplicate. Identity is now a normalized digest —
case-folded, punctuation-collapsed, trimmed, capped — which absorbs
phrasing churn while keeping distinct findings apart. The tension with
R9-2 is real and resolved deliberately toward a visible duplicate over
a silent loss.
- R10-1 (Critical): the sweep tripwire is a spelling denylist over an
unbounded space. Reframed as what it is — a drift alarm, not the
boundary (the boundary is the env -i child, pinned separately) — and
aimed at the ENUMERATION PRIMITIVES a sweep needs (compgen -e,
declare -x, env pipes, export -n) instead of more name vocabulary.
- R10-6 (Critical): the jq stub hardcoded /usr/bin/jq, which does not
exist on macOS; it now resolves through the original PATH.
- R10-13: the child's log comes from agent-writable RUNNER_TEMP, so ::
is neutralized on re-emission via parameter expansion (no fork).
- R9-5 leftover: the third truncation dump now names the clipped size.
Mutation-verified 5/5 and behaviour-probed 5/5 (escape-before-compare,
reworded re-emission, distinct sibling, R9-2 no-regression, planted-path
shapes).
* fix(autofix): make the deferral identity lossless; keep the unmerged set on disk (review round 11)
Two Criticals, both defects this PR introduced, plus the eleven items
carried from round 10.
- Identity key (Critical): the normalized key stripped every non-[a-z0-9]
byte and capped at 160 chars, so CJK siblings collapsed to one key (this
repo is bilingual) and a long path pushed the reason out of the identity
entirely — silent loss, the exact outcome the feature exists to prevent
and the opposite of what its own comment claimed. The key now normalizes
case and PUNCTUATION only, keeping every letter of every script and no
cap, at both the build and corpus sites. Rewording tolerance is
unchanged. Probed: 2 CJK siblings -> 2 of 2; 2 siblings on a 200-char
path -> 2 of 2; reworded duplicate -> 1 of 1.
- Repair merge failure (Critical): the branch deleted
deferred-findings.json before 'Show run artifacts' and the artifact
upload ran, so its own pointer at the artifact dump was false past the
4000-byte clip. It now renames the set to deferred-findings.unmerged.json
(kept in WORKDIR, added to the dump list) on the failure path and deletes
only on the merge-success path. Probed: 6245 bytes preserved where the
dump clipped at 4000.
- R10-12: neither side is known to be the corrupt one (jq -s fails if
either input is unparseable), so both merge-failure warnings say that
instead of blaming this round.
- R10-18: a second identity anchor — the derived title — so an edited body
that loses the marker no longer orphans the issue into a duplicate; the
marker still wins when both are present, and a same-titled PR is still
never adopted.
- R10-3: the carry branch is unreachable in today's topology (WORKDIR is
wiped at run start, one repair step); kept as defensive with that stated.
- R10-10: the builtins-only discipline is scoped to the child-output
INSPECTION, which is what it always meant.
- Pins: delimited-token allow-list incl. UPSERT_LOG's value (R10-7), every
respelling of executing the staged path (R10-8), the tripwire extended to
the issue-autofix failure steps (R10-11), launch-line anchoring in both
steps (R10-15), flagless rm counted (R10-16), and behavioural cases for
the 200/500 caps (R10-4), CJK and long-path siblings, and the title
anchor.
Mutation-verified 7/7.
* refactor(autofix): remove the agent-writable paths the upsert depended on (review round 12)
Rounds 9-12 each closed one hole in a design that read the staged script
from an agent-writable path and buffered the child's output through
another. Round 12 found four more of the same class (TOCTOU on the log
reopen, a plantable FIFO and an unbounded read on each path). Rather than
patch a fifth time, remove both paths.
- The script travels as CONTENT: the stage step captures it from the
trusted checkout into a step output (random heredoc delimiter), and the
clean child runs `bash -c "$UPSERT_SRC"`. With no agent-writable copy
there is nothing to verify — the digest gate, its check-then-use
window, the staged cp and the FIFO/huge-file read all disappear.
- The child's messages travel on fd 3, which the parent captures, while
fd 1/2 are discarded. Every loader side channel writes there, so the
noise still cannot reach the parsed output — and there is no log file
to plant, race, bound, or clean up. Probed with LD_SHOW_AUXV and
LD_TRACE planted: clean output, sentinel behaviour unchanged.
- RC-1: resolved-comments.txt went to jq as one argv element, the exact
MAX_ARG_STRLEN failure the neighbouring comment describes and that
`known` already avoided. Both corpora use --rawfile now. Measured on a
348 KB corpus: the old form dies with "Argument list too long", the new
one publishes normally.
Net -113 lines, and the pins follow: no-path invariants replace the
digest/log battery. Mutation-verified 6/6.
* fix(autofix): reject multi-document deferral files; survive a base without the script (review round 13)
- Multi-document JSON (Critical): `jq -e` without -s evaluates each
document in turn and its exit status reflects only the LAST, so
`[valid]\n[]` exited 0 silently (findings lost, no warning) and
`[bad-id]\n[valid]` passed the shape gate outright. A single_doc gate
now runs first, with the asymmetry the earlier rounds settled on: a bad
OWN file is a total abort, a bad CARRY costs only the carry.
- R13-7: the stage step reads the script from the TRUSTED BASE, where it
does not exist until this PR merges — under -e that killed every
pre-merge pull_request-triggered round (true of the old cp too, so this
has been latent since the script was added). It now tolerates the
absence and lets the consumers' own empty-content guard skip the round.
- R13-2: the carry union requires both inputs to BE arrays; `add` on two
non-arrays yields whatever they add to.
- RA1R4-B: the resolved-corpus test is -f/-r, so a directory or FIFO at
that path is treated as unusable rather than present.
- R13-1: nine rationale comments still described the staged copy and the
digest gate that round 12 removed.
Probed: both multi-document shapes are rejected loudly with zero writes;
a bad carry still leaves this round publishing 1 of 1.
* docs(autofix): retire the last stale rationale comments (R13-1)
Six of the nine locations were in the test file: comments still describing
the digest gate, the staged copy and the read-once invariant that round 12
removed. Same honesty issue as their three workflow siblings.
* fix(autofix): strip BSD wc padding; let wrapper warnings stay annotations (review round 14)
- BSD wc (Critical): `wc -l` pads its count with leading spaces on
macOS, and TOTAL_NEW is interpolated into the cap warning and the
success line — the sibling `wc -c` already stripped it, this one did
not. Tested with a padding `wc` stub, since GNU wc never pads and the
regression is invisible on Linux CI otherwise.
- The re-emit loop demoted the feature's own failure signal: every `::`
became `;;`, including the wrapper's trusted messages. Wrapper-authored
lines now carry a marker and are emitted VERBATIM (so they render as
annotations again); the script's output, which interpolates agent
content, stays neutralized.
- \b on `declare -x`/`export -n` had the same word-boundary hole already
fixed for `trap -`/`unset -f`.
- Pins: the heredoc CLOSING delimiter, the merge-failure quarantine
rename, and an allow-list comparison that is a sorted multiset rather
than a Set — a symmetric same-name addition is exactly what that check
exists to catch and a Set hid it.
Mutation-verified 5/5.
* fix(autofix): un-truncate the sibling identity; align the carry precedence (review round 15)
Zero Criticals this round; five behavioural findings among the pins.
- The rv/ic intra-batch identity was derived from the RENDERED line, i.e.
after the 500-char reason cap, so two siblings differing only past the
cap collided and one vanished silently — the same silent-loss class as
the CJK and long-path entrances. It now comes from the uncapped
path+reason; the corpus check still compares rendered forms (that is
all the issue stores), so cross-round the cap can cost a duplicate,
never a loss.
- The repair carry union put the OLDER set first, inverting the
newer-wins precedence the script documents for its own union.
- The resolved-id parser dropped any line with stray surrounding
whitespace, so a padded `rc:<id>` no longer suppressed its finding.
- The clean child's catch-all warning lacked the trusted marker added
last round, so the feature's most common failure message was still
demoted out of annotation form.
- The truncation notice pointed at the artifact dump even when the
dumped file is a merge temp outside WORKDIR, which is never uploaded.
Pins: the stage step's `id: 'stage'` (the link whose break empties every
UPSERT_SRC), the empty-content skip branch, the capture's `|| true`, and
the trusted marker on every warning inside the child.
Mutation-verified 6/6.
|
||
|
Shaojin Wen
|
337da2143c
|
fix(ci): stop dropping agent settings in resolve and follow-up workflows (#9252)
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
Security Checks / Dependency CVE audit (push) Waiting to run
Security Checks / Secret scan (TruffleHog) (push) Waiting to run
* fix(ci): stop dropping agent settings in resolve and follow-up workflows Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ci): pin remaining agent-settings guard gaps from review Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
易良
|
5b125f0b89
|
fix(ci): minimize spam inline review comments (#9229) | ||
|
Shaojin Wen
|
e7a7ac1bfb
|
feat(autofix): deny-by-default footprint gate and positional window censuses (#9156)
* feat(autofix): deny-by-default footprint gate, positional window censuses, review-loop backlog Follow-up to #8981/#8996, closing the structural causes behind their review-round non-convergence: - Deny-by-default footprint: every file a round touches maps to an AREA (declared workspace, else top-level directory, else the root file itself); areas outside the PR's own footprint are surfaced in a gate-authored advisory, or rejected retryably once the repo variable QWEN_AUTOFIX_FOOTPRINT_ENFORCE is staged to 'reject'. The enumerated class gate keeps rejecting regardless — this inverts the default for everything it cannot enumerate (a denylist is not a boundary). - The three window censuses (PRIOR_TIMEOUTS, WIN_HEADS, PRIOR_HEADS) attribute comments positionally over their own scan-parsed eval markers instead of whole-body win= substrings: a neutralized marker quoted in a handoff excerpt, or any future marker embedding win=, can no longer double-attribute a comment (decoy fixture included; the census fixture's non-numeric round= placeholder is corrected). - BITE_ENFORCE's reply arm inherits the thread root's CHANGES_REQUESTED membership, not just its body tag. - Backlog tests: the bite restore-failure crash contract (verdict-less exit with the rejection document, driven by a ref-deleting runner), merge-base-anchored footprint compares under an advanced main (afterPr fixture hook), and the shrink+bite advisory append order. - SKILL: cap each round's implemented batch (~8 findings, Critical first, defer the rest via comment-replies) — nine review rounds of evidence that oversized fix batches breed fix-of-fix defects — and document the footprint gate. * fix(autofix): close the R1 footprint-gate findings - Advisory lifecycle: one reset at gate start, every writer appends — the footprint advisory no longer dies to the shrink section's rm or its truncating write. - Footprint membership is REF-ANCHORED: areas derive from the pre-round root manifest's workspaces globs (longest ancestor wins, nested workspaces correct), so a round cannot redefine its own boundary and the on-disk resolver is out of this path entirely; non-workspace paths under packages/ keep two segments so sibling projects stay distinct areas; emitted areas are newline-sanitized against phantom footprint grants. - The enforcement knob rides step-level env at both verify gates — $GITHUB_ENV writes from earlier steps cannot downgrade 'reject'. - TESTSIDE's critical() mirrors cr_attached (root and self), keeping enforcement and demotion on one comment set. - Census ownership is LAST-WINS over scan-parsed markers (a stray quoted-or-appended marker cannot double-attribute), the replay decoy is now genuinely discriminating (old whole-body → 0, new → 1), and the growth-gate comment stops citing retired whole-body matchers. Queued per the batch cap: per-line advisory bullets and the third sink charset, discriminating fixtures at the two remaining census sites, the reply-arm bite fixture, freight and merge-base footprint fixtures, and digest-pinning the staged resolver for its remaining consumers. * fix(autofix): close the R2 footprint-gate findings - list_areas reads and translates the ref's workspaces globs ONCE per invocation and matches ancestors in-bash (was ~21ms git+jq+sed per file×ancestor call), emits printf %q keys — line-safe AND injective, so distinct areas can never collapse into one comparison key — and both render arms print one bullet per area from those keys. - Producer failures are a STATE: a failed round- or PR-side diff (orphan history, transient git error) skips the footprint check loudly instead of shrinking one side into a verdict. - The workflow-level FOOTPRINT_ENFORCE env is gone (the step-level pins are the only consumers and outrank it — dead config removed); the two step wirings are count-pinned. - Fixtures: nested-workspace membership discriminates against the packages/ two-segment fallback (sibling nested workspaces stay distinct areas), and the advisory-lifecycle discriminator proves an earlier section's advisory survives the shrink section. Queued: consolidating the six eval-marker regex variants behind one grammar constant (touches six jq programs; its own change). |
||
|
Shaojin Wen
|
4ee6a087e5
|
feat(autofix): judge review-feedback validity by content, not author (#8996)
* feat(autofix): judge review-feedback validity by content, not author
Wrong feedback drives wrong rounds regardless of who wrote it: maintainers
increasingly draft comments with models, so author identity carries no
correctness signal. The trust gate stays as the injection/authorization
boundary it always was, but the validity layer becomes source-blind and
execution-based, enforced by the verification gate rather than prose.
Three mechanisms:
- Bite check: a round changing both source and tests has its changed tests
re-run against the pre-round tree (origin/<branch> sources with the
round's test files overlaid). All green there means the claimed defect
never reproduced — the shape of a plausible-but-false finding implemented
as a fix — and the round is rejected, non-retryable, with the measurement
in LAST_REJECTION so the next round can decline or escalate the finding.
Fails open on every scope limit: single-workspace rounds only (gitignored
dist carries the round's build across the detach, the same confound that
A/B-exempts typecheck), runnable unit tests only, and any pre-round
failure counts as biting.
- Sensitive-area footprint: a round may not expand into CI/verification
machinery the PR itself never touched — .github/, .husky/, eslint/vitest/
tsconfig configs, and the scripts section of existing root or first-level
workspace manifests (the gate's own command surface). Judged by area
class so takeover on an infra PR keeps full freedom; round-added
workspace manifests are exempt. Rejected retryably (the repair pass can
revert).
- Test-deletion advisory: shrinking coverage is surfaced by a gate-authored
section in the round report (deleted files, net test lines), never by the
agent's own prose, so a maintainer reads the agent's justification next
to the machine measurement.
SKILL.md rewrites the address-review protocol to match: identical
verification for every author, probe evidence outranks any assertion,
refuted maintainer claims are escalated with the measurement instead of
silently obeyed or overridden, and severity tags alone no longer make an
item Required — the claim must be checkable and reproduced.
* fix(autofix): harden the validity gates per review round
- Scan round/PR diffs NUL-delimited with --no-renames: a rename out of a
sensitive area now classifies the vacated source path (moving a
workflow out of .github/ is a removal of verification machinery), and
specially named files are no longer core.quotePath-mangled past the
case patterns.
- Narrow the capability classes: .github/workflows|actions, .github/
scripts, and passive .github metadata are separate areas (an
issue-template PR no longer licenses workflow rewrites), and the
transitive executable surface — repo scripts/ (minus scripts/tests/)
and .npmrc/.nvmrc — joins the protected set.
- Gate the bite consequence on machine-read intent: rejection now
requires the round to RESOLVE a Critical-tagged or CHANGES_REQUESTED
finding (resolved-comments.txt matched against rc.json/rv.json);
every other src+test round gets a gate-authored advisory on all-green
instead — a behavior-preserving refactor pinning existing behavior is
no longer rejected.
- Drop the blanket *.md exclusion from bite source detection: skill
markdown is executable agent behavior, and the intent gating now keeps
doc-only rounds safe from rejection.
- Sanitize deleted-test filenames in the gate advisory through a safe
character set: a backtick in a legal git filename could close the code
span and forge gate-authored markdown.
- Replace per-path basename spawns with parameter expansion.
- Tests: rename-evasion, metadata-vs-workflow class split, repo-scripts
class with the scripts/tests carve-out, filename-forgery rendering,
enforce-vs-advisory bite consequences (Critical tag and CR review),
and tree-state-proving runners that flip on pre-round source with the
round's test overlaid (plus the round-leak negative control).
One reviewed finding is declined with evidence in the thread: existential
batch semantics for mixed Critical rounds (per-behavior probe binding
needs test-result parsing; documented as a known limit at the check).
* fix(autofix): close the round-2 validity-gate findings
Sensitive-area scan: read NUL records directly (no tr re-mangling — a
newline filename cannot mint phantom footprint grants); resolve declared
workspace manifests and workspace-root configs through the trusted
resolver (nested workspaces protected, src-tree scaffolds exempt); split
root vs workspace manifest classes; guard the root workspaces array; give
the loop's own workflow and gate script their own class; classify .qwen/
(skills are executable agent behavior); anchor footprint content compares
at the merge base; sanitize violation paths in the rejection document.
Bite check: tolerate rc:-prefixed and CRLF resolved-comment ids (the
handle format SKILL prescribes — enforcement never fired without this);
count replies resolved in Critical-rooted threads as defect claims; skip
non-vitest workspaces (a vacuous --if-present pass must never reject),
self-package-name imports (dist confound), and rounds with paths outside
the resolved workspace; include renamed tests and changed snapshots in
the overlay; drop nested fences from the rejection document; surface
test-only defect claims as an advisory; document the already-fixed
re-raise limit and steer it to a no-code round.
Tests: classifier probe over every arm, footprint cases for the new
classes, enforce-vs-advisory negatives, reply-root enforcement, and the
rc:/CRLF handle round-trip.
* fix(autofix): close the round-3 Critical findings on the validity gates
- Gate-consumed helper scripts (resolve-owning-packages, settings-schema
and contracts checks) join the autofix-loop class: an unrelated
.github/scripts footprint no longer licenses rewriting machinery the
gate executes.
- Skip round-scan files whose content equals current origin/main: a
round that merges main (the flow SKILL prescribes on conflicts) made
ROUND_RANGE degenerate and attributed all incoming main churn to the
round, false-rejecting ordinary base updates.
- Round-added workspace-root configs are the round's own surface (same
cat-file exemption manifests have); deleted workspace manifests are
classified from pre-round existence instead of the on-disk resolver
that can no longer see them.
- The bite vitest guard reads the PRE-ROUND manifest — the tree whose
test script the detached runner actually executes.
* fix(autofix): close R4 validity-gate findings — gate-consumed surfaces join the taxonomy
- Supply-chain surfaces classify: lockfiles/shrinkwraps (root and nested)
and patches/ (patch-package runs on every install) as supply-chain;
.gitattributes (root and nested) as measurement-config — a -diff rule
could blind numstat-based advisories.
- manifest_scripts_changed inspects resolution fields too: workspace
manifests compare {scripts, exports, main, types}; the root manifest
adds exports alongside workspaces.
- resolve-sandbox-image.mjs joins the autofix-loop class (it establishes
the loop's isolation boundary).
- The noop path emits verified_head, making the prescribed no-code
re-verification round mechanically able to resolve threads.
- The bite transcript is cleaned at gate start like its sibling logs;
the advisory's test definition aligns with the growth brake's six
globs (__tests__/, test-utils/ included).
R4-3 (post-round on-disk workspace resolution racing a same-round
workspaces negation) is declined in-thread: it requires the PR footprint
to already license manifest-scripts-root, which is the accountability
boundary working as designed; pre-round-tree resolution is queued with
the census follow-up. R4-5 (advisory in failure paths) queued likewise.
* fix(autofix): deflake the bite harness and align the test taxonomy
- Isolate fixture git from ambient global/system config (the sibling A/B
fixture's GIT_CONFIG_GLOBAL=/dev/null pattern) and fail loudly on spawn
errors with the exit status in the assertion message — the advisory
sub-case intermittently died spawn-level under load with empty streams
and no diagnostic (reproduced 1/6 locally, once on CI).
- BITE_SRC excludes __tests__/ like the gate's own TEST_PATHSPEC.
- SKILL's boundary enumeration names the supply-chain and
measurement-config classes and the full protected manifest fields.
* fix(autofix): close R6 validity-gate findings
- Test-side defect claims take the advisory arm: when every resolved
Critical thread sits on a test file (rc.json .path), the fixed test
legitimately passes pre-round — enforcement grade 'advisory', never a
rejection; the test-only advisory also no longer requires a matching
*.test.* glob (snapshot-/helper-only resolutions surface too).
- Classifier arms: newline-bearing paths fail CLOSED as their own class;
qwen-pr-safety-precheck.yml + pr-safety-precheck.mjs join autofix-loop;
nested .npmrc/.nvmrc; eslint.legacy-filenames.mjs (imported by the lint
leg's config); root manifest filter carries main/types.
- The self-import dist-confound guard matches the package name delimited
(quote or subpath), so @qwen-code/qwen-code no longer swallows its
-core sibling's imports.
- Test isolation extends to the footprint and advisory spawns (R5's
rationale applied everywhere), spawn errors fail loudly there too, the
classifier probe pins the supply-chain/measurement-config arms, the
coverageOnly fixture asserts the advisory text, and the neutralization
ledger header matches its count.
- The resolve-threads design doc records the widened no-op
verified_head rule and its safety argument.
Deferred to the backlog per the convergence note: the bite-side restore
crash-contract test (shared-fixture work), origin/main-advanced footprint
fixtures, and advisory append-order pins.
* fix(autofix): close R7 validity-gate findings
- The resolve/reply pass is a shared function serving BOTH the pushed and
no-op outcomes: the no-code re-verification escape can now actually
resolve threads, and no-op declines finally post their in-thread
replies (a pre-existing silence gap). The design doc states the shared
path, its guards, and the named first-round residual.
- TESTSIDE demotion votes only over resolved CRITICAL threads (a source
Suggestion resolved alongside no longer breaks it; a source Critical
alongside keeps full enforcement) — three fixtures pin the matrix.
- The shrinkage advisory measures with --no-renames (a rename out of
runner discovery is a shrink) and NUL-safe deleted names.
- Bite inputs pass through the merge-freight filter the class scan
already applies, and BITE_SRC collects NUL-safe.
- Demoted rounds get their own advisory text (all-green is their
expected shape, not a failed reproduction).
- The manifest block comment matches the resolver-backed code; the
footprint and advisory test spawns get the isolation and loud
spawn-error handling previously claimed — the R6 reply overstated
that fix and this commit is the correction.
* fix(autofix): close the review-body re-checks on the validity gates
- Deleted-manifest classification honors the fixture exemption from the
PRE-ROUND root manifest's workspaces globs (was_workspace_dir) — a
deleted src-tree fixture manifest is no longer false-rejected, while a
deleted declared workspace still classifies; the PR-footprint scan
gets the same treatment anchored at the merge base, so a PR-deleted
workspace keeps licensing later rounds.
- A config added into a PRE-EXISTING workspace is machinery (the gate's
legs execute it); only a config born with its round-added workspace
keeps the exemption.
- The shrinkage advisory applies the merge-freight skip per file (NUL
numstat records), so a base-merging round is not charged main-side
test churn in trusted-voice text.
- The bite rejection document renders filenames through the safe
charset and collapses backtick runs in the runner tail below the
outer fence length.
- AGENTS.md/CLAUDE.md classify as agent-policy; the root-manifest
comparator covers lint-staged and config (sandboxImageUri) too.
Still standing by recorded design, acknowledged in the review body:
R1-9 (already-fixed re-raise), R1-27 (existential batch semantics),
R4-5 (post-round resolver vs same-round workspaces negation).
* fix(autofix): close the round-9 validity-gate re-checks
- TESTSIDE's critical() carries the CHANGES_REQUESTED review-state arm
and receives rv.json, mirroring BITE_ENFORCE — a CR-enforced test-side
claim demotes to the advisory arm, and a CR-enforced source claim can
no longer collapse into it (R8-1, both directions).
- was_workspace_dir matches workspaces globs PATH-AWARE ('*' stops at
'/', '**' spans, '?' single, '!' entries skipped conservatively): a
nested src-tree fixture manifest deletion no longer false-rejects
while a declared workspace deletion still classifies (R9-1); both
pinned by fixtures.
- The PR-footprint manifest arm answers aliveness and membership from
refs (origin/<branch> / merge base), never the round's on-disk tree —
a PR-added workspace a round later deletes keeps its footprint class
instead of walling the deletion (R9-3).
---------
Co-authored-by: verify <verify@local>
|
||
|
易良
|
8189c284ae
|
fix(ci): use repository token for spam minimization (#9140) | ||
|
易良
|
85a06bbec8
|
fix(desktop): bridge Electron users on Windows and Linux (#9079)
* fix(desktop): bridge Electron users on Windows and Linux * test(desktop): satisfy bridge contract lint * fix(desktop): harden Electron bridge migration * chore(desktop): note sibling-script regex sync for bridge artifacts * fix(desktop): satisfy release manifest lint * fix(desktop): validate Electron uninstaller path before migration The migration ExecWait target is assembled from the HKCU InstallLocation registry value; require the uninstaller executable to exist before running it, and pin the conjunct in the release contract suite. |
||
|
Shaojin Wen
|
5a6452a2a5
|
fix(ci): make autofix verification gates hermetic to runner git config (#8961)
* fix(ci): make autofix verification gates hermetic to runner git config A leaked global exec knob on the persistent pool (run 31516789251: diff.external=global-driver in the runner user's ~/.gitconfig) failed four per-hunk probe tests in packages/cli on #8613. The rejection was charged to the round (package tests are A/B-exempt), which burned the 18-minute repair on a failure no repair can reach and ended the round as a timeout — attempt 4 of the failure window, with nothing pushed. Three layers against that class: - Both verification gates (the review script and the issue-fix inline twin) now export a per-run throwaway GIT_CONFIG_GLOBAL (seeded with the workspace safe.directory) and GIT_CONFIG_SYSTEM=/dev/null before their first git command, so every check they spawn — vitest fixture repos included — is hermetic to the host, and a branch-authored `git config --global` dies with the run instead of poisoning the next one. - The sanitize step (all three byte-identical copies) now also scrubs the runner USER's global config — denylist of the command-execution families only, so infra-owned routing/credential keys survive. This self-heals the live pollution on the affected runner on its next job and removes (not merely bypasses) a planted global core.hooksPath. - test-efficacy.integration.test.ts gets the same GIT_CONFIG_GLOBAL / HOME isolation as git.integration.test.ts: the code under test pins --no-ext-diff, but the test scaffolding's plain `git diff` did not, so a hostile user git config could fail the suite anywhere. Contract tests pin the gate redirects (before the first git command, truncated per run) and functionally run the extracted scrub pipeline against a poisoned fixture config, asserting the kept/removed sets. * fix(ci): widen the config scrub and re-sanitize before PAT-bearing steps Address the #8961 review findings (2 Critical, 8 Suggestions across two reviews), all probe-verified by the reviewers: - Denylist regex: subsection slots are .+ (git subsection names may contain dots — diff."a.b".command flattened past [^.]+), and the missing exec families are added: gpg.(*.)program, pager.*, interactive.diffFilter, difftool./mergetool., remote.*.uploadpack/ receivepack. The functional fixture now covers every alternation plus dotted subsections, non-exec/corrupt/missing-config arms pin the two load-bearing '|| true' guards. - The global scrub moved above the .git early-exit: host hygiene owes nothing to the workspace existing. - New resanitize-git-config.sh (staged from the trusted base) re-runs the local allowlist sweep and the global denylist scrub at the top of both PAT-bearing git steps — the gates run branch test code on the host after the job-start sanitize, and the env redirect is not a filesystem boundary. Contract tests pin script/step lists equal, the staging in both jobs, the call-before-credential ordering, and run the script functionally against planted local+global keys. - All three one-shot credential helpers lead with -c credential.helper= (empty resets the helper list; the first helper to answer wins, so a planted one must never run first). Count-pinned in the contract test. - comment-status.integration.test.ts gets the same git-config isolation as its siblings; test-efficacy gains an isolation tripwire test that goes red if the redirect is removed, instead of only on hostile hosts. - Comment fixes: the copies are cross-referenced as contract-test-pinned, and the system-config bypass is documented in both gates. * fix(ci): close the XDG/env/transport bypasses around the config scrub Address the #8961 round-2 review findings (5 Critical + 8 Suggestions, probe-verified by the reviewers; the XDG listing gap independently reproduced on git 2.55): - The global scrub sweeps BOTH files of the global scope: with ~/.gitconfig and $XDG_CONFIG_HOME/git/config both present, `git config --global` lists/unsets only the former, so keys planted in the XDG file survived every copy. The scrub is now a loop that redirects GIT_CONFIG_GLOBAL at each file in turn. - Denylist adds url.*.insteadOf/pushInsteadOf (transport rewrite of the PAT push/fetch; rest of url.* stays) and http.*.sslVerify/sslCAInfo (turns a kept http.proxy into a TLS-terminating interceptor); the three PAT helper chains lead with -c http.sslVerify=true. - The staged resanitize script's provenance holds at cp time only — RUNNER_TEMP is writable by the branch code that runs in between — so the staging steps record its sha256 in GITHUB_OUTPUT and the PAT steps verify before executing. - Both gates and both PAT steps export GIT_CONFIG_COUNT=0: GITHUB_ENV-injected GIT_CONFIG_KEY/VALUE entries apply at command-line precedence and outrank every file-level guard. - Gates emit a ::notice when /etc/gitconfig exists (bypassed by the redirect — replicate needed settings via per-job env). - Tests: the scrub's functional harness drives HOME/XDG fixtures and covers the new families; the resanitize run plants worktree-scoped config (deleting the rm -f line previously stayed green); the gate redirect block is executed against a hostile HOME and an env-planted GIT_CONFIG_* key; the isolation tripwire pins the NOSYSTEM leg and probes system-scope leakage. - The process-env git isolation pattern is extracted into isolateHostGitConfig() in review/lib/test-utils.ts and adopted by all five suites that duplicated it; comment-status gains the same tripwire. * fix(ci): take PAT git steps off host scopes and close the env channels Address the #8961 round-3 review (5 Critical + 6 Suggestions, probe-verified by the reviewer): - Both PAT-bearing steps now run fully hermetic, same shape as the gates: a per-run throwaway GIT_CONFIG_GLOBAL + GIT_CONFIG_SYSTEM= /dev/null, so a concurrent job rewriting the shared ~/.gitconfig in the sweep->push window (max-parallel, one HOME across ~27 runner registrations) can no longer steer the push, and a URL-scoped sslVerify=false there can no longer override the -c pin. Both steps and both gates also strip the git ENV channels that outrank file config: GIT_CONFIG_PARAMETERS, GIT_SSL_NO_VERIFY/CAINFO, GIT_PROXY_COMMAND, GIT_EXEC_PATH, GIT_DIR/WORK_TREE, GIT_ASKPASS, GIT_SSH/_COMMAND, plus GIT_CONFIG_COUNT=0. - The push-race salvage merge runs -c commit.gpgsign=false: a global commit.gpgsign=true with no key would exit 128 and be misread as a content conflict, discarding a verified round (R2-10). - The maintainer-fork fetch, the one PAT-bearing network site the round-2 rollout skipped, leads with -c http.sslVerify=true -c credential.helper= (anonymous; public fork heads need no auth, so it fails closed on a 401 instead of feeding a planted helper the PAT). - Denylist widens protocol.ext.allow to protocol.(ext.)?allow (the top-level fallback policy arms ext:: too) in all four copies. - Tests: the two PAT hermetic blocks and the two gate blocks are pinned equal; the sha256 verify line is pinned verbatim and asserted to carry no bypass; the resanitize fixture plants a live XDG exec key (drops of the loop's XDG leg now fail); the gate redirect functional exec adds the env-channel unsets; diff-plan adopts isolateHostGitConfig (sixth suite) keeping its GIT_TERMINAL_PROMPT delta; comment-status tripwire gains the GIT_CONFIG_GLOBAL assertion. * fix(ci): pin PATH, seal repo-redirect and env channels, harden all PAT sites Address the #8961 round-4 review (6 Critical + suggestions, probe-verified by the reviewer): - PATH is pinned to a value the stage step records before any branch code runs, and LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH are dropped, at the top of every PAT step and both gate steps — a $GITHUB_ENV-planted PATH or preload would otherwise swap the git/sha256sum/bash the digest gate itself runs on. - The Prepare step (PAT-bearing, previously unhardened) now takes the same hermetic preamble as the push steps; all three PAT preambles are pinned identical by the contract test. - The throwaway global config is created with mktemp, not a fixed literal path a same-user watcher could re-plant into after the seed. - The env-strip list gains GIT_ALLOW_PROTOCOL (env twin of protocol.allow), GIT_COMMON_DIR / GIT_OBJECT_DIRECTORY / GIT_ALTERNATE_OBJECT_DIRECTORIES / GIT_SHALLOW_FILE (repo-redirect twins), across all PAT and gate copies; the salvage/fork fetches carry -c fetch.recurseSubmodules=false -c protocol.ext.allow=never so a planted submodule cannot execute an ext:: URL with the PAT. - resanitize removes .git/commondir and .git/shallow (the file twins of GIT_COMMON_DIR/GIT_SHALLOW_FILE), and Push-and-report refuses to push a HEAD that is not the gate's recorded verified_head — closing the repo-redirect path that pushed attacker content. - The gate runner (run-autofix-review-verification.sh) is now digest- verified before both gate passes, like resanitize already was: the branch runs its own build/test between the passes, so an unverified copy would let it define its own verdict. - Contract tests pin every new surface: the three identical PAT preambles, the full unset var set, the mktemp path, the trusted-PATH wiring, the two gate-runner digest checks, the recurse/protocol pins, the HEAD==verified_head guard, and the commondir/shallow removal. * fix(ci): pin gh env channels, harmonize allowlist subsection slots Round-4 follow-ups: - Pin GH_HOST=github.com and unset GH_TOKEN/GH_ENTERPRISE_TOKEN/ GH_CONFIG_DIR before the first gh call in all three PAT steps, so a $GITHUB_ENV-planted GH_HOST cannot spoof the identity check and a planted GH_TOKEN cannot outrank the inline one. - Harmonize the local allowlist's remote/submodule subsection slots to .+ (matching the denylist comment and preventing a dotted-name remote from silently losing its url/fetch on every resanitize). * fix(ci): pin gh config dir and push the exact verified object Round-5 closable findings before landing: - R5-7: pin GH_CONFIG_DIR to a fresh mktemp -d instead of unsetting it, so PAT-bearing gh calls no longer fall back to the attacker-writable ~/.config/gh (whose config.yml can carry http_unix_socket and other transport reroutes) on the shared HOME. - R5-8: push the exact verified commit object (PUSH_SHA:refs/heads/...), not symbolic HEAD which the push would re-resolve — closing the check-then-use race the verified-HEAD guard was added to close. PUSH_SHA is pinned to VERIFIED_HEAD under the guard and re-pinned to the merge result after each salvage merge. The remaining round-5 Criticals (BASH_ENV/BASH_FUNC_* and LD_PRELOAD executing at step-shell startup before any unset runs; GITHUB_OUTPUT writable by gate-run branch code) are not closable from inside a Actions step — they require runner-level isolation and are tracked as a follow-up. |
||
|
易良
|
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> |
||
|
Shaojin Wen
|
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> |
||
|
Shaojin Wen
|
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> |
||
|
Shaojin Wen
|
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> |
||
|
Shaojin Wen
|
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> |
||
|
易良
|
1cbf2e8fc7
|
feat(ci): auto-assign issues to area owners from labels (#8668)
* feat(ci): auto-assign issues to area owners from labels Route labelled issues to a maintainer with push access, without putting issue text in the path of a write token. Assignment is a pure function of the issue's labels and a checked-in label -> owner map, evaluated by a standalone workflow on issues:labeled. No model runs in the assignment path and the script never reads issue title, body, or comments, so untrusted issue text cannot select an assignee. Owners come from CODEOWNERS (asserted by test) and every candidate is re-checked against the collaborator permission API before the write, so editing the map cannot grant access. Among eligible owners the least loaded wins, rotating by issue number to break ties. * fix(ci): decouple issue owner map from CODEOWNERS CODEOWNERS answers who owns a code path, which is narrower than who may be assigned an issue in an area: the repository has ~44 collaborators with push access against 7 CODEOWNERS entries, so the membership test would have rejected legitimate additions such as admins and maintainers who own no path. Drop that assertion and document the actual process for adding owners. The live collaborator permission check remains the boundary. Add the validation the map does need: duplicate owners would skew load balancing, and duplicate area names would silently shadow each other under first-match-wins. * fix(ci): keep issue ownership triggers disjoint * fix(ci): recheck issue owner assignment before write * test(ci): cover issue owner label recheck * docs(ci): Correct CODEOWNERS count in issue assignment rationale * fix(automation): preserve autofix issue ownership * feat(ci): widen core issue owner pool to active repository maintainers * feat(ci): add four more collaborators to core issue owner pool * fix(ci): tighten issue-owner map validation and sync trigger docs * fix(ci): tighten issue-owner assignment tests and login validation (#8668) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
Shaojin Wen
|
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>
|
||
|
易良
|
5fdcdb28e2
|
fix(ci): avoid root-owned npm cache workspace files (#8669) | ||
|
Shaojin Wen
|
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. |
||
|
Shaojin Wen
|
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> |
||
|
Shaojin Wen
|
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> |
||
|
易良
|
89b5aa7a03
|
feat(desktop): bridge Electron users to Tauri updates (#8392)
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
npm cache producer / Save npm cache (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
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
* feat(desktop): bridge Electron updates to Tauri * test(desktop): cover parseArguments validation in electron bridge manifest (#8392) * chore(desktop): address bridge review follow-ups --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
易良
|
cb2555c7c5
|
feat(desktop): package Web Shell as a release-ready desktop app (#8132)
* feat(desktop): add Web Shell Tauri proof of concept * feat(desktop): prepare Web Shell shell for release * fix(desktop): make release dry runs portable * fix(desktop): harden cross-platform release smoke * fix(desktop): stabilize Windows and Linux CI * fix(desktop): scope bootstrap env to daemon * fix(desktop): stabilize packaged app smoke * fix(desktop): diagnose Linux packaged startup * fix(desktop): address release readiness review * fix(desktop): address follow-up review findings * fix(desktop): address runtime review blockers * fix(desktop): gate cookie auth acceptance behind desktop bootstrap flag - Cookie→Bearer translation middleware now only active when desktopShellBootstrap is enabled - Use timing-safe comparison for bootstrap token validation * fix(desktop): replace cookie handshake with URL fragment auth - Navigate the desktop WebView to /#token=<token>; the fragment never reaches the server, so drop the desktop cookie bootstrap middleware, its cookie->bearer translation, and the related serve tests - Skip the deferred-runtime auth gate for pre-auth Web Shell routes (GET|HEAD / and /assets/*): a document navigation cannot carry an Authorization header, so the fast-path window used to answer the first desktop navigation with 401 Unauthorized until a manual reload - Poll /health?deep=true before navigating: deep health stays 503 (reason: bootstrap) until the runtime app that mounts the Web Shell is ready, so readiness can no longer race the deferred window - Run the folder picker off the main thread and only store the runtime after the WebView navigation succeeds - Enable withGlobalTauri plus a bootstrap capability so the bootstrap page can subscribe to desktop lifecycle events - Update smoke-packaged to assert the fragment contract (unauthenticated root navigation 200, no cookies minted, API routes still 401) and sync the release design doc * fix(desktop): fix Linux smoke log path, add runtime .gitkeep, correct README (#8132) * fix(desktop): close release readiness gaps * fix(cli): keep deferred serve auth gate closed when web shell unmounted (#8132) * fix(desktop): address review feedback on auth gates and runtime bundle (#8132) - Cover the method guard in isPreAuthWebShellRequest: assert unauthenticated POST to / and /assets/* is still 401 during the deferred runtime window. - Add unit tests for is_allowed_navigation covering the unset origin, set origin, and bootstrap-after-origin cases. - Drop DEV:'true' from the release bundle step so the esbuild metafile is no longer shipped as dead weight in the desktop runtime. * fix(desktop): address review feedback on runtime extraction and release workflow (#8132) - Extract .zip Node archives with unzip so Linux cross-builds for win32-x64 no longer crash on GNU tar. - Build the Windows signing config with ConvertTo-Json instead of backslash escapes, which PowerShell treats as a parse error. - Fetch the runtime Web Shell without a bearer token so the smoke test exercises the pre-auth navigation path the shell relies on. - Make GitHub release creation idempotent so a re-run after a partial publish uploads assets instead of failing on the existing tag. * fix(desktop): normalize artifact filenames to prevent updater 404s (#8132) GitHub rewrites spaces to dots when release assets are uploaded, but the updater manifest encoded spaces as %20 via encodeURIComponent. This caused every platform's auto-update URL to 404 on published releases. Replace spaces with hyphens in the Collect artifacts step for all platforms so the local filename, the manifest URL, and the published asset name agree by construction. Update test-release.js fixtures to match and assert no artifact name contains a space. * fix(desktop): address review feedback on security, lint, and code quality (#8132) * fix(desktop): address review feedback on smoke test, error UX, and window state (#8132) * fix(desktop): address review feedback on crate build, recovery UX, auth gate, and CI (#8132) * fix(desktop): address review feedback on settings race, version script, and log growth (#8132) * fix(desktop): address review feedback on retry, auth gate, and release clobber (#8132) * fix(desktop): gate commands to bootstrap origin and show native update dialog (#8132) * fix(desktop): use matches! instead of PartialEq on JoinError result (#8132) * fix(desktop): wait for deferred runtime in smoke tests and sync release flags on clobber (#8132) --------- Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> |
||
qwen-code-dev-bot
|
253f8b8daf
|
fix(ci): harden self-hosted runner workspace ownership recovery (#8115)
* fix(ci): harden self-hosted runner workspace ownership recovery Containerised jobs (qwen-triage verify/tmux) leave root- or node-owned files in the runner workspace. When the next job's actions/checkout tries to remove them it fails with EACCES, permanently poisoning the runner for all subsequent jobs. Three-layer fix: 1. qwen-triage.yml: split the ownership-restore (chmod + chown back to the runner user) out of the conditional 'Clean up runner workspace' step into its own 'if: always()' step, so it runs even when the job is cancelled or skipped — while the container still has root. 2. qwen-code-pr-review.yml / ci.yml: after the existing chown + sudo attempts, probe each known problem dir (.qwen, .git) with touch; if unwriteable, rename it aside (mv only needs write on the parent directory, which the runner user owns). ci.yml gains the full 'Restore workspace ownership' step it previously lacked. 3. ci.yml 'Clean stale .qwen before checkout' now also removes any .qwen.stale.* directories renamed aside by the step above. Refs: runs 30339720611 (actions-runner-8), 30480422410 (actions-runner-test-14) — both EACCES on .qwen/agents. * fix(ci): address review feedback on workspace ownership recovery Repoint the verify cleanup regression test at the new 'Restore workspace ownership' step so the chmod-before-chown guard is active again, and remove renamed-aside .git.stale.* / .qwen.stale.* dirs in ci.yml and qwen-code-pr-review.yml so they no longer accumulate on self-hosted runners. * test(ci): guard unconditional ownership restore for verify and tmux (#8115) * test(ci): guard rename-aside ownership recovery in ci.yml and pr-review (#8115) * fix(ci): address review feedback on workspace ownership recovery - Make .stale.* cleanup failure visible with :⚠️: instead of silent || true; add sudo -n rm -rf fallback to ci.yml cleanup step - Add .stale.* sweep to containerised verify/tmux-testing ownership- restore steps (run as root, the only actor that can delete them) - Extend ownership recovery to web_shell_e2e_smoke and integration_cli jobs which share the same ecs-qwen self-hosted runner pool - Probe-first optimization: skip expensive recursive chown/chmod on healthy runs; only pay for the full-tree walk when a probe fails - Use mkdir/rmdir instead of touch/rm for writability probe (mkdir never follows symlinks, avoiding a planted-symlink vector) - Use GITHUB_RUN_ID.GITHUB_RUN_ATTEMPT instead of $$ for unique suffix (PID recycles on long-lived runners) - Add set -uo pipefail and $GITHUB_WORKSPACE/ absolute paths to ci.yml cleanup step - Keep rename-aside blocks byte-identical across ci.yml and qwen-code-pr-review.yml with a NOTE comment explaining why extraction into .github/scripts/ is impossible - Add :⚠️: on chmod failure in verify ownership-restore step - Add tests: new job coverage, byte-identical block assertion, .stale.* sweep assertions, cleanup hardening assertions (#8115) * test(ci): guard all four rename-aside copies in byte-identical assertion (#8115) * fix(ci): drop inert rename-aside, restore unconditional recovery (#8115) Review verification showed the pre-checkout rename-aside fallback never unblocks actions/checkout: checkout deletes every workspace entry (or runs git clean -ffdx), walking straight into the renamed dir, and the rename only fires when the runner does not own the dir — exactly when rm -rf cannot empty it either. Drop it from all four checkout jobs and the qwen-triage root sweeps, restoring the simpler unconditional chown/chmod recovery. The probe-first gating is removed for the same reason: poisoning is workspace-wide (root-owned node_modules/dist with no .qwen/.git), so a probe that only checks .qwen/.git reports "healthy" and skips the chown that main did unconditionally — a regression on runners with passwordless sudo. The layer-1 fix (qwen-triage ownership restore running as root under if: always()) is the actual root-cause fix and is kept unchanged. * fix(ci): address review feedback on ownership tests (#8115) Hoist assertUnconditional helper to module scope and reuse it in the pr-review describe block, add sudo-fallback and stat-based UID discovery assertions to guard the recovery branches that are load-bearing on non-root and containerised runners. * fix(ci): cover triage job, add chown warnings, tmux chmod (#8115) * fix(ci): assert restore-before-checkout ordering, surface sudo chown stderr (#8115) * fix(ci): widen verify restore chmod, guard symlinked .qwen, surface sudo chmod stderr (#8115) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-autofix[bot] <qwen-code-autofix[bot]@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
8efdf749ad
|
fix(autofix): guard review thread resolution (#8231)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
qwen-code-dev-bot
|
0d3c8641f1
|
ci: cache npm downloads for verify and tmux build steps (#7885)
* ci: cache npm downloads for verify and tmux build steps The "Install and build PR app" step in both the verify and tmux jobs runs `npm ci` from scratch every time, taking ~5m40s out of a 15-minute verify run. Add an `actions/cache@v4` step before each build that restores the npm download cache keyed by `package-lock.json` hash. Security model: the cache restore runs as root (with full Actions credentials) in a separate step. The build step itself still strips ACTIONS_RUNTIME_TOKEN/URL/CACHE_URL before running PR lifecycle scripts as the `node` user, so untrusted code cannot read or write the Actions cache. The restored cache directory is chowned to `node:node` and passed via `npm ci --cache` so the build user reads packages from the local cache without touching the cache API. Expected improvement: npm ci drops from ~4min to ~1min on cache hit, cutting total verify time from ~15min to ~10min. * ci: pin actions/cache to SHA for supply-chain security (#7885) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * ci: add npm cache comment to verify job matching tmux job (#7885) * fix(ci): use actions/cache/restore to prevent cache writes from PR code (#7885) * test(ci): pin actions/cache/restore as restore-only invariant in both lanes (#7885) * test(ci): assert npm ci consumes the restored cache directory (#7885) * fix(ci): align prepare log with npm ci cache flag and harden cache tests (#7885) * fix(ci): harden npm cache tests and document missing save step (#7885) * fix(ci): add npm cache producer and clear stale cache before restore (#7885) * test(ci): harden npm cache guards per review (#7885) * fix(ci): make npm cache test robust to prettier YAML quoting (#7885) Prettier reformats the hashFiles() key value from single-quoted YAML (with '' escaping) to double-quoted, breaking the raw-string comparison in the cache producer test. Compare parsed scalar values instead. * fix(ci): run npm cache producer on the consumer runner so restores hit (#7885) actions/cache scopes an entry by a hash of the literal cache path plus the compression method. The producer ran on ubuntu-latest (host path, zstd) while the verify/tmux consumers run in a node:22-bookworm container (container path, gzip), so the versions never matched and every restore was a guaranteed permanent miss. Move the producer onto the same runs-on + container so path and compression match by construction, give the restore step an id and report cache-hit to the job summary so any future miss is visible, and point the stale-cache clear step at $RUNNER_TEMP so it removes the container path rather than the inert host path. --------- 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-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code CI <qwen-code-ci@users.noreply.github.com> |
||
|
Shaojin Wen
|
f66bfaad57
|
fix(release): keep notes anchored and cap the release body (#8199)
* fix(release): keep notes anchored and cap the release body
The v0.21.2 publish failed at "Create GitHub Release and Tag" with
HTTP 422 "body is too long (maximum is 125000 characters)", after every
npm package had already been published.
Stable releases are tagged on their own release/* branch and merged back
to main only afterwards, so the previous stable tag is never an ancestor
of the branch being released. The ancestor guard therefore dropped
--notes-start-tag on every stable release, and without an anchor GitHub
generates notes across the entire branch history (8000+ commits), which
overruns the body limit.
Always pass the previous tag instead: GitHub diffs it through the merge
base, which is how v0.21.1 produced a 27KB body from a tag that was
equally divergent. Generate the body through the generate-notes API
first so an oversized changelog is truncated on a UTF-8 boundary, and
degrade to an unanchored body and then a minimal one, rather than
aborting a release whose packages are already on npm.
* test(release): pin the anchored release-notes contract
The workflow test asserted the ancestor guard that dropped
--notes-start-tag on every stable release. Assert the replacement
instead: the previous tag is always passed to generate-notes, the body
is capped, and ancestry no longer decides whether notes are anchored.
* refactor(release): extract release-notes capping into a tested helper
The degradation chain lived inline in the workflow bash, so nothing
pinned that a capped body plus its footer stays under GitHub's 125000
character limit, that truncation never splits a multi-byte character, or
that the chain always yields a non-empty body. Move it to
.github/scripts/cap-release-notes.mjs with a collocated node:test suite,
matching the other workflow helpers.
Capping on code points rather than bytes drops the head/iconv dance and
makes the surrogate-pair case testable. The helper also absorbs the
empty-body fallback, which caught a real defect: gh writes the API error
payload to stdout when generate-notes fails, so a doubly failed call
would have published `{"message":"Not Found",...}` as the release body.
Discard a failed attempt's output instead.
* test(release): exercise the surrogate-pair cut and footer-overflow branch (#8199)
---------
Co-authored-by: Qwen Code Bot <qwen-code-bot@alibabacloud.com>
|
||
|
qwen-code-dev-bot
|
6097d7ab63
|
ci: auto-minimize comments from org-blocked users (#7899)
* ci: auto-minimize comments from org-blocked users Adds a scheduled workflow that runs every hour to scan recent issue/PR comments and minimize any from users blocked at the org level. This cleans up spam comments that were posted before a block was applied. The workflow: 1. Fetches the org's blocked-user list via REST API 2. Queries recent comments (last 2h) via GraphQL 3. Matches comment authors against the blocked list 4. Minimizes unmatched comments as OFF_TOPIC via GraphQL Also triggerable manually via workflow_dispatch with a configurable lookback window. * ci: use repo blocklist file instead of org blocked-users API The org blocked-users API requires admin:org scope which the CI bot PAT doesn't have. Switch to a plain-text blocklist file at .github/spam-blocklist.txt — one username per line, case-insensitive, # for comments. No special scopes needed. Also adds danialzivehdadr as the first entry. * ci: make auto-minimize-spam failures visible, handle empty blocklist * fix(ci): address review feedback on auto-minimize-spam workflow (#7899) - Change hours input type from string to number (project convention) - Extract duplicated step-summary writes into write_summary() function - Capture gh stderr (2>&1) and include it in failure warnings - Document coverage limits in header comment * fix(ci): quote auto-minimize-spam expressions to pass yamllint (#7899) * test(ci): add regression guards for auto-minimize-spam workflow invariants (#7899) --------- 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> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
DennisYu07
|
1b5c36ce15
|
ci: add isolated DSW SWE-bench release pipeline (#7656)
* ci: add isolated DSW SWE release pipeline
* ci: bootstrap branch-only DSW full-suite test
* Revert "ci: bootstrap branch-only DSW full-suite test"
This reverts commit
|
||
|
易良
|
15ca9818e0
|
fix(ci): restore verify workspace permissions (#7992) | ||
|
Shaojin Wen
|
43c35e533b
|
fix(ci): keep the post-merge E2E signal on main alive (#7795)
* fix(ci): stop cancelling in-progress E2E runs on main Merges land on main roughly every 18 minutes (median) while a full E2E run takes about 40, so cancelling the in-flight run on every push starved the suite: across the last 100 push runs, 67 were cancelled and only 25 ever reported a result. The nightly regression was the only reliable signal, and each cancelled run still burned roughly 10 minutes on three runners before dying. Turning cancellation off for main hands the coalescing to GitHub's concurrency queue, which keeps at most one pending run per group and cancels the previously pending one. The queue therefore collapses to the newest tree on its own while the in-flight run always finishes, so every result covers the batch of commits merged since the last one — bisect that range when it goes red. Dev branches keep cancelling superseded runs, where only the latest push matters. * fix(ci): dedupe main CI failure issues by failing test The autofix issue for a red main was deduped on the commit SHA, so a standing failure opened a brand new issue on every merge: one broken E2E test on 2026-07-26 produced six duplicate issues in twelve hours, each pointing the autofix agent at an unrelated commit. The failing tests are now identified from the logs of the failed jobs and used as the dedupe key, with one marker per test so a failure set that grows still matches the issue that already tracks part of it. Later commits hitting the same failure are appended to that issue as recurrences (bounded, newest first) instead of opening another one, and notes written by a human or the agent are preserved when the machine-owned trailer is refreshed. Runs with no identifiable test — an install or build break — keep the previous per-commit behaviour. * fix(ci): keep the failure analysis out of the bot-PAT job Identifying the failing tests means running a helper from the repository, and the workflow deliberately checked out nothing so that the job holding the bot PAT could never execute repository code — an invariant its own test enforces. Rather than weaken it, the work is split: a read-only job checks out the tree, reads the failed run's logs, finds any issue that already tracks the failure and renders the title and body, handing both over as outputs. The job with the PAT keeps checking out nothing and only writes what it was given. The invariant test now asserts that separation — the PAT job runs no repository code and holds only issue write — instead of banning checkout everywhere in the file. * fix(ci): harden main CI failure dedupe per review (#7795) - Match the `[run <id>]` link text instead of the run URL when deduping recurrences: `/301` is a substring of `/3010`, so the URL match silently deleted an unrelated run's line. - Rebuild the machine-owned "## Also failing" section from the live failure set on every merge so a test that has since been fixed drops out instead of being listed forever. - Assert the analyze checkout is SHA-pinned with persist-credentials disabled and pin the failed-job log-download paths in the workflow test. - Add an e2e workflow test guarding the cancel-in-progress expression that keeps in-progress runs on main from being cancelled. * fix(ci): bound the issue body and randomize the heredoc delimiter (#7795) * fix(ci): test the runCli --existing merge path (#7795) * fix(ci): address review feedback on e2e signal PR (#7795) - Assert the full cancel-in-progress expression including && so a mutation to || is caught by the e2e-workflow test - Filter the capped-summary line from missingTests so it is not rendered as a fake bullet under Also failing --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> |
||
|
易良
|
45a6a69cf0
|
feat(triage): add revert-pattern high-risk path detection (#7414)
* feat(triage): add revert-pattern high-risk path detection Replace the behavior-neutral PR filter (PR #7414 v1, ~2% hit rate) with a data-backed triage gate based on revert-history analysis of 111 revert commits and 46 unique reverted PRs in this repo. Stage 1e checks three signals identified by the analysis: - touches_high_risk (66.7% precision, 32.3% recall) - contested-merge pattern (50.0% precision, 19.4% recall) - non-maintainer + high-risk (58.3% precision, 22.6% recall) The gate escalates review depth and recommends maintainer sign-off; it never blocks or closes PRs. Design doc and analysis scripts included. * fix(triage): avoid stale-exempt hold label * fix(triage): address review risk detection feedback * fix(triage): tighten high-risk path patterns * fix(triage): address review feedback on Stage 1e revert-pattern gate (#7414) * fix(triage): address round-2 review feedback on Stage 1e gate (#7414) - Fix APPROVE → APPROVED state name (GitHub API enum) - Use gh api --paginate for file listing (fixes 100-file truncation) - Anchor shell/relaunch/sandbox patterns with (^|/) to avoid false positives - Append || true to grep (exit 1 on no match is the 92% case) - Scope E2E recommendation to write-access authors per Stage 2c - Add bot author filter to contested-merge query - Define core paths explicitly in contested-merge condition - Wire Stage 1e do-not-auto-approve into Stage 3 guardrail - Replace precision percentages with p-values/raw counts in skill text - Add sampling caveat and statistical significance notes to design doc - Fix design doc errors: 71%→61.5%, 10→8 PRs, Rule 3 attribution, ~20% baseline→10% prevalence, Area field, no_e2e inconsistency - Make test assertions specific to Stage 1e (not vacuous) - Add Risk: template field assertion - Revert drive-by prettier reflow - Note need-discussion label removal by maintainer * fix(triage): address round-3 review feedback on Stage 1e gate (#7414) - Separate gh api call from grep so API failures are visible instead of being masked by || true (rc:3660753982) - Include author identity in contested-merge jq output and require different reviewers for the disagreement check, avoiding false positives from same-reviewer iteration (rc:3660753990) - Add Stage 1e to the approval summary checklist so it is not omitted from the pre-approval conditions (rc:3660753994) * fix(triage): address round-4 review feedback on Stage 1e gate (#7414) * fix(triage): use portable ERE grep for test-file exclusion (#7414) * fix(triage): guard deferred approval on discussion label * fix(triage): keep only supported revert signal --------- Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
qqqys
|
c994524a2d
|
feat(autofix): retry deterministic rejection once (#7796)
* feat(autofix): retry deterministic rejection once * fix(autofix): use repaired verification result * fix(autofix): preserve repair safety context * test(autofix): cover empty verification outcomes |
||
|
Shaojin Wen
|
2da42099d4
|
fix(ci): don't fail triage cleanup when there is nothing to clean (#7688)
The 'Clean stale agent state' step strips non-allowlisted keys from the persistent workspace's local git config through a `git config --list | grep -ivE <allowlist> | while ...` pipeline. When the config holds only allowlisted keys — the steady state on a reused runner this step already sanitized, since actions/checkout's post step removes its auth extraheader at job end — grep matches nothing and exits 1. Under the default `bash -e` shell combined with the script's `set -o pipefail`, that kills the step exactly when there is nothing to clean, before any output, and every downstream triage step is skipped (seen on run 30095456731, runner ecs-qwen-runner-sg-4). Guard the grep with `|| true` so an empty match feeds an empty loop instead of failing the job. Sanitization behavior is unchanged: non-allowlisted keys (core.pager, include.path, `!`-aliases) are still stripped. The workflow test harness missed this because its allowlist test re-assembles the pipeline without the step's shell flags and always plants non-allowlisted keys first. Add a steady-state regression test that runs the step's actual script under `bash -e` against a config holding only allowlisted keys, plus a negative control that strips the guard and demands the step die — red on the old workflow, green now. Co-authored-by: verify <verify@local> |
||
|
qqqys
|
b1ce0c2087
|
refactor(autofix): extract review verification runner (#7644)
* refactor(autofix): extract review verification runner * test(ci): follow extracted autofix verifier * docs(autofix): document the review verification runner env contract (#7644) |
||
|
Shaojin Wen
|
f8014652a5
|
test(triage): regression-guard the triage workflow, and make the git cleanup an allowlist (#7660)
Guards the security-critical invariants of qwen-triage.yml that broke silently once already — the `settings_json:` input name was wrong, so the action dropped it and the review agent ran with the full default toolset and no deny list. A new `node:test` suite (wired into the shared HELPER_TESTS list both CI paths run) asserts: the `settings:` input name (never `settings_json:`), the tools.core registration whitelist and the deny list, the fork-PR runner routing invariants, and the git exec-vector cleanup. It also flips that cleanup from a best-effort denylist — which kept missing new families (pager, filter.*, includeIf subsections, url.*, credential…) — to a keep-known-safe allowlist: unset every local config key that isn't plumbing actions/checkout needs (repo format, remote, branch, fetch/gc/pack/index, safe.directory, extensions, submodule url/active/branch — not submodule.*.update, which can be `!cmd`). This closes the whole exec-vector class, including knobs not yet enumerated. The harness runs the workflow's actual allowlist pattern against a scratch repo to prove it unsets every exec family and preserves the checkout plumbing. Co-authored-by: verify <verify@local> |
||
|
qqqys
|
cb98102149
|
ci(autofix): add cross-package contract verification (#7642) | ||
|
Shaojin Wen
|
14f1f2bb36
|
fix(ci): don't let one failing scenario sink the whole visual preview (#7511)
The web-shell visuals render runs every screenshot and flow in a single `test:e2e:visuals`, and that step had no `continue-on-error`, while the compose and upload steps had no `if: always()`. So one failing or timing-out scenario failed the job, the artifact was never uploaded, and the publish workflow had nothing to post — the entire preview vanished even when every other scenario passed and its PNG was already on disk. A flow (a long multi-click sequence) is the most fragile scenario kind, so the fragile one silently takes down the deterministic screenshots. PR #7498 hit exactly this: 29 scenarios passed, one new channel-management flow timed out, and the PR got no preview and no comment at all. Make the after-capture step `continue-on-error` so the passing captures survive and the later steps still compose and upload them. The publish job only runs on a `success` conclusion, so the job must stay green — but a masked failure must not read as a clean preview. Ship the step's real `.outcome` (which continue-on-error does NOT mask, unlike `.conclusion`) to the publisher as `render-status.txt`, and have the comment builder use it: an empty preview whose render failed says "one or more scenarios failed to render" and is explicitly NOT the reassuring green check or the coverage-gap prompt (both imply the render ran); a partial preview is labelled partial above the shots that did render. A missing status file (older run) defaults to complete, so this only ever adds a warning, never suppresses a real preview. The failing scenario still needs fixing — it's now surfaced in the comment rather than by silently deleting everyone else's preview. Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
82157d1c03
|
fix(ci): tell a visuals coverage gap apart from "no visual change" (#7375)
An empty visual preview means one of two opposite things: the change genuinely moves no pixel, or no scenario renders the UI it touches. The bot printed the same green check for both, so the second — a coverage gap, where the preview literally cannot see the feature — read as a clean bill of health. That has now happened three times (#7035 primary label, #7221 worktree badge, #7365 empty-state toggle), each caught only because a maintainer noticed the missing image and asked. The signal to tell them apart was already there and unused: the render workflow only runs when the web-shell client or webui source changed, so an empty preview is by construction "UI code changed, nothing rendered differently". When no view changed, look at which files the PR touched. If any are render-shaping (.tsx / .css / .svg under the rendered surface, excluding test and scenario code), list them and say the result is ambiguous, with a pointer to where a scenario goes. Otherwise keep the green check — a logic-only PR with no visual delta is expected, and prompting there would train everyone to ignore the prompt when it matters. The path list comes from the PR files API in the privileged publish job, which never checks out PR code; if that call fails the comment falls back to the current wording. Co-authored-by: wenshao <wenshao@example.com> |
||
|
易良
|
cda0e0348d
|
ci: move release-note classifier from per-PR workflow to release-time batch (#7339)
* ci: move release-note classifier from per-PR workflow to release-time batch * fix(ci): enumerate release PRs from generated notes * fix(ci): reconcile release note labels * fix(ci): address release classifier review * fix(ci): surface release classifier failures |
||
|
Shaojin Wen
|
eca654f365
|
fix(autofix): resolve owning package for nested paths; report verify-failed handoffs as not pushed (#7330)
* fix(autofix): resolve owning package for nested paths; report verify-failed handoffs as not pushed The verify gate mapped each changed file to a flat `packages/<dir>` and read `<dir>/package.json`, which ENOENT-crashed on nested packages such as packages/channels/base — the container packages/channels has no package.json. Walk each changed file up to its nearest package.json in both the issue-fix and review-address verify steps, and skip any candidate that still has none. When such a verify failure follows an agent commit, the review-address handoff rendered the agent's optimistic address-summary.md (which can cite a commit SHA) under a neutral "what I found" heading, so a maintainer chased a commit that was discarded with the runner workspace. An EXIT trap now records any post-commit non-zero exit as outcome=failed, and the handoff states plainly that the change did NOT pass the gate and was NOT pushed. Tests: walk-up detection over a nested package tree, the outcome=failed trap, and the not-pushed handoff wording — each mutation-verified. * refactor(autofix): extract owning-package resolver to a shared staged script Addresses review on #7330. Extract the changed-file → owning-package walk into .github/scripts/resolve-owning-packages.sh, staged to RUNNER_TEMP from the trusted base alongside check-settings-schema.sh and invoked from both verify gates, so the two gates cannot drift into resolving packages differently (the 8-line walk was otherwise duplicated verbatim in each). Updates the package-scripts test that pinned the old inline grep. Narrow the verify-failed handoff lead-in to "This change was NOT pushed": four paths set outcome=failed BEFORE the deterministic gate runs (agent abort via failure.md, dirty tree, unchanged branch, missing address-summary.md), so the previous "did NOT pass the verification gate" claim was factually wrong for them. The specific reason stays in the headline and the quoted summary. * style(autofix): brace variable references in resolve-owning-packages.sh The repo's shellcheck gate runs --enable=all --severity=style, under which bare $f/$d references trip SC2250 (prefer ${var}). Brace them to match the convention already used in check-settings-schema.sh, and update the script content assertions accordingly. Verified with shellcheck 0.11.0 using the exact CI flags: clean. * fix(autofix): resolve owning workspace via npm query; key unpushed-handoff on commit existence Addresses the deeper review on #7330. Blocking issue: the "nearest package.json" resolver mapped a change under a workspace's fixture/example package (e.g. packages/cli/src/commands/extensions/examples/starter) to that fixture, whose test script is not Vitest — silently SKIPPING packages/cli's own tests, a coverage regression invisible in the log. Resolve against the authoritative `npm query .workspace` set instead and take each file's longest-prefix workspace: nested workspaces (packages/channels/base) match exactly, fixtures and non-workspace paths (packages/sdk-python, packages/README.md, the excluded packages/desktop) drop. Also harden the resolver against a final line with no trailing newline and against an unmatched last line, which under `set -o pipefail` would otherwise abort the script. Handoff wording: keying "was NOT pushed / commit discarded" on outcome=failed was wrong for the abort paths (failure.md, dirty tree, unchanged branch, missing address-summary.md), which set outcome=failed before ever making a commit. Record committed=true right after checkout — before any gate can fail — and key the wording on that; the abort/no-op paths keep the neutral framing. This removes the EXIT trap entirely (its only observable effect was that wording), so it no longer mislabels pre-commit failures either. * fix(autofix): expand workspaces on-disk so branch-added packages are tested; harden resolver Addresses the re-review on #7330. The resolver sourced its workspace set from `npm query .workspace`, which reads node_modules — installed from the BASE checkout. A workspace the PR branch ADDS (a new channel adapter, a new sdk — the issue-fix job's whole purpose) was invisible, so its tests were silently skipped, and for a nested new package the ENOENT crash this PR fixes turned into a silent skip. Expand the set from the on-disk root package.json `workspaces` globs instead (shallow `dir/*` + literals, honouring `!` negations, keeping dirs with a package.json): it reflects the branch, matches what `npm run --workspace` accepts downstream, and needs no install. Verified to reproduce `npm query`'s set exactly on the current tree. Also from the review: - Fail the gate loudly on an empty/unreadable workspace set instead of the silent "no package changes" skip, and drop the now-unneeded `|| true` at both resolver call sites (the resolver already exits 0 on legitimate no-match). - Record committed=true at the TOP of the step (ref-only diff), covering an agent that commits then aborts, and count only `git diff --quiet` exit 1 as a commit (128 is a git error, not a discarded commit). - Correct the two call-site comments that still described the superseded nearest-package.json approach. Also hardens the resolver against a final changed-path with no trailing newline and an unmatched last line under `set -o pipefail`. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
易良
|
8fe1f926e9
|
ci: auto-skip internal CI changes in release notes (#7251)
* ci: auto-skip internal CI changes in release notes * ci: harden release note classifier * fix(ci): close release note classifier review gaps * fix(ci): close release classifier review gaps * fix(ci): avoid release classifier label loop * test(ci): cover release classifier test path variants |
||
|
Shaojin Wen
|
076427650d
|
feat(ci): auto-open a deflake fix issue for confirmed flaky tests (#7231)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
* feat(ci): auto-open a deflake fix issue for confirmed flaky tests
The CI Failure Patrol reruns flaky failures but never fixes them, so
the same tests flake forever on a rerun treadmill. This closes the
loop: when the patrol classifies a rerun as a nondeterministic TEST
(not infra), it now also opens ONE deflake issue that the existing
autofix issue pipeline develops into a reviewable stabilization PR.
- ci-flaky-patrol SKILL: a rerun decision whose cause is a specific
named flaky test carries an optional flakyTest {file, name}; infra
reruns (ENOSPC, network, runner death) never do.
- ci-flaky-rerun.mjs: validates flakyTest (malformed → the whole
decision is rejected, so a bad classification can't open a bogus
issue); after a rerun, ensureDeflakeIssue upserts a deflake issue
deduped by a stable (file, name) marker — one open issue per flaky
test across all PRs — labeled status/ready-for-agent + autofix/
approved so the scheduled autofix scan picks it up.
- .qwen/skills/deflake/SKILL.md: constrains the fix to four
assertion-preserving patterns (raise timeout/poll budget, stabilize
timing/waiting, make randomness/time deterministic, isolate
interference) and forbids skipping/deleting/loosening the check;
write failure.md if none applies or the failure looks like a real
bug. The produced PR is reviewable, never auto-merged.
Tests: deflakeKey stability/collision-freedom, the bilingual issue
body, one-issue-per-test dedup, no issue for infra reruns, and
malformed-flakyTest rejection. 34/34 across both patrol suites.
* fix(ci): deflake review hardening — rerun survives bad metadata, no markup injection
Addresses the two Criticals + suggestions on #7231:
- **Critical: a malformed/over-length flakyTest no longer kills the
rerun.** flakyTest validation is removed from validDecision (which
gated the PRIMARY action on secondary metadata — a >200-char nested
test name or a null silently dropped a valid rerun). Well-formedness
is now checked in ensureDeflakeIssue, which simply skips the deflake
issue when the metadata is bad; the rerun always stands.
- **Critical: markup/mention injection via the test path/name.** file
and name are code-span-stripped of backticks (which cannot be escaped
inside a span and would break out into live Markdown, turning
into a mention in a bot-created issue) and both now sit in
code spans. safeReason alone did not close this (it does not touch
backticks).
- Best-effort deflake: ensureDeflakeIssue is wrapped in try/catch so a
transient createIssue failure — after the marker is already posted —
no longer surfaces as a misleading "skipping PR" and permanently
suppresses the deflake; it retries on the next flaky occurrence.
- Run link uses the patrol's own repo (client.repo) instead of the dead
target.repo, so deflake issues on a fork don't 404.
- Body reworded: it no longer claims the rerun already passed (it runs
right after the rerun is triggered) — it says a real deterministic
failure is NOT flakiness and must not be stabilized.
- SKILL: bound file/name to 200 chars, and note a malformed one is
ignored (never drops the rerun).
Tests: malformed flakyTest keeps rerun (no createIssue); long title
truncates ≤240; backtick path/name cannot inject; run link honors the
repo; a throwing createIssue leaves the rerun intact. 38/38.
---------
Co-authored-by: wenshao <wenshao@example.com>
|
||
|
Shaojin Wen
|
a319dd0b86
|
ci(web-shell): denoise cross-job font-AA so visual previews stop false-flagging (#7210)
The before/after preview flagged text-heavy views (e.g. workspace-sidebar) as ~0.1-0.3% "changed" on PRs that do not touch them — #7204 is a live example (the two panels are pixel-for-pixel indistinguishable). Root cause: base and head render in SEPARATE CI jobs, so Linux font anti-aliasing is not bit-identical between them, and the naive per-pixel diff counts the scatter of isolated / 1px-wide glyph-edge pixels that leaves. At the tight 0.02% threshold that scatter crosses the line. Measure the changed fraction AFTER a cluster denoise: a differing pixel counts only when at least 4 of its 8 neighbours also differ. AA scatter (isolated = 0 neighbours, a 1px line = 2) erodes to ~zero, while a real change — a badge, chip, icon, panel — is a solid block whose interior keeps 5-8 and easily clears the threshold, so the threshold stays tight without raising it (which would miss small real changes like a workspace badge). The browser now returns a compact bit-mask; the denoise + count run in node against the unit-tested countDenoisedChanges, so there is one tested implementation of the metric. Co-authored-by: wenshao <wenshao@example.com> |