mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-31 03:44:33 +00:00
450 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
1336923682 | Merge remote-tracking branch 'origin/main' into fix/release-notes-timeout | ||
|
|
ea4c25e25f
|
feat(triage): run external /verify on ECS behind a workspace wipe (#7985)
* feat(triage): sponsored /verify for external PRs on ephemeral runners
/verify used to require the PR AUTHOR to hold write access, because the
lane executes the author's code on the persistent ECS pool — which
excluded external contributors' PRs entirely (16 of 38 open PRs
eligible when measured on 2026-07-28), and those are the PRs where
sandboxed evidence is worth the most. The practical alternative was a
maintainer running the verification on their own machine, which is
strictly worse: personal credentials, SSH keys, and no isolation at
all. The risk was not avoided, only relocated.
The author's permission now ROUTES instead of gating, using the same
conditional runs-on pattern the authorize/triage jobs already use for
fork PRs:
- write-access author -> the persistent ECS pool, unchanged;
- external author -> a SPONSORED run on an ephemeral GitHub-hosted VM
(destroyed after the job), free on a public repository.
The commenter gate is unchanged: only a write-access maintainer can
trigger either lane, so runs stay maintainer-metered. Their comment is
the sponsorship, and authorize snapshots the head OID at that moment;
the resolve step refuses to run any other commit, so code pushed while
the job queues is never executed on the sponsor's approval. On the ECS
lane the execution-time re-check keeps its old meaning (the author's
write permission is the control there, and it can be revoked while
queued); its refusal now names the sponsored alternative.
Before anything executes on the hosted lane, a pre-execution risk
screen reads the PR diff as data: mechanical checks (added npm
lifecycle scripts, dependencies resolving outside registry.npmjs.org,
package-manager config changes, long opaque single-line content) and a
model screen, every arm failing CLOSED — an unfetchable diff, missing
screen config, a model error, and an unparseable reply each refuse the
run. Reasons posted to the PR are fixed strings chosen by the
workflow, never diff text, so a crafted diff cannot steer the bot's
comment. The screen is defense in depth, not the control: what bounds
a malicious PR is the ephemeral VM. The model can be fooled;
the machine being destroyed cannot.
The ECS kill switch no longer strands the hosted lane (an ephemeral
runner does not depend on the pool), and the authorize-side denial
notice for external authors is gone — the case it explained no longer
denies. The 2b-bis skill guidance now offers the sponsored run instead
of declaring the lanes unavailable, with an explicit warning that an
external author's report is adversarial input to be read like the
fork's own CI logs.
Mutation-verified 8/8, each with landing proof: dropping the commenter
gate, inverting the routing, dropping the OID snapshot, letting a
permission-API failure route instead of deny, dropping the head-moved
check, dropping the lifecycle-script check, tolerating an unparseable
model reply, and pinning the hosted lane back to ECS each turn a test
red. The screen tests run the real resolve-step text against a stubbed
gh and a live local HTTP server standing in for the model endpoint —
in a separate process, because spawnSync blocks the test runner's
event loop and an in-process server would deadlock into the fetch's
120 s abort instead of testing anything.
actionlint category diff vs main: -3 SC2016:info (the removed denial
step), +3 SC2129:style (new GITHUB_OUTPUT groups); both non-gating.
* docs(triage): persistence, not lateral movement, is the ECS threat
The maintainer confirmed the ECS runners are network-isolated, so the
comments justifying the hosted routing now name the decisive risk
correctly: machines are reused and the next run arrives carrying
credentials that planted artifacts could harvest. Lateral movement was
never the load-bearing argument; persistence is, and it is the one
threat a destroyed VM removes without depending on sweep completeness.
* feat(triage): run external /verify on ECS behind a workspace wipe
Maintainer decision (2026-07-29): external contributors' PRs run on the
persistent ECS pool like everyone else, not on ephemeral GitHub-hosted
VMs. The pool is network-isolated, and the realistic alternative — a
maintainer verifying on a personal machine with real credentials and no
isolation — is worse than a hardened shared runner.
That removes machine destruction from the design, so the author's
permission now selects CONTROLS rather than a machine:
trusted -> unchanged: write permission is the control, re-checked at
execution time, head may move freely.
external -> pinned head OID (the sponsoring comment approves the code
that existed then), pre-execution risk screen, and a full
workspace wipe.
The wipe is what replaces destruction. The residual ECS risk for
untrusted code is PERSISTENCE: machines are reused, and the next run
starts by handling credentials (CI_BOT_PAT in the resolve step, the
model key in the proxy). The existing sweep is an allowlist of vectors
we have thought of, and an adversarial author gets unlimited attempts to
find one we have not — so external runs delete the workspace outright
before checkout instead of sweeping it. Deny-by-default costs one cold
clone and removes hooks, worktree config, planted dotfiles, staged
artifacts and out-of-tree symlinks in one move, known or not. Scope is
stated in the step: RUNNER_TEMP and /tmp also persist, and the steps
reading them already rm -rf their own subtrees; the container is
recreated per job, so /home/node and its npm cache are not cross-run
state.
The screen's status changes with the machine: on ephemeral hardware it
was a cost-saver, here it is load-bearing — but still not sufficient
alone, and the comment says so, naming the wipe and the sweep as what
actually bounds a screened-but-hostile diff.
Mutation-verified 6/6: removing the wipe, moving it after checkout,
applying it to every run, tolerating a non-empty workspace, dropping the
suspicious-path guard, and classifying an external author as trusted
each turn a test red.
Also fixes a live hazard in the guard's own test. It passed real paths
(/, /usr, /root) to a live `rm -rf`, which is safe only while the guard
exists — so it detonates precisely when the guard is removed, which is
when it is meant to protect you. It did: a mutation run that deleted the
guard spent six minutes attempting to delete / before being killed.
macOS permissions absorbed it and nothing was lost, but that outcome was
luck, not design. `rm` is now stubbed to a recorder on PATH, so the
destructive primitive cannot fire from the test suite under any edit,
and the assertion is on the decision — with the guard gone the recorder
shows the attempted delete and the test fails, having deleted nothing.
* fix(triage): close two screen bypasses and a vacuous routing assertion
Review round on #7985 found four issues; all four were real.
1. npm lifecycle alternation stopped at the install lifecycle, but npm
runs pre/post hooks for ANY `npm run <script>`. This job runs
`npm run build` and the agent runs the PR's suites, so a `prebuild`
or `pretest` executes exactly like a `postinstall` and walked past
the mechanical screen. Added build and test to the alternation.
2. The off-registry check excluded any line CONTAINING
`registry.npmjs.org`, so a lookalike host — registry.npmjs.org.evil.com,
or an @-userinfo form — read as npmjs and passed. The tarball's own
postinstall lives inside the tarball, never in the diff, so this arm
was the only mechanical defense against it. The exclusion is now
anchored to scheme + host + '/'.
3. A routing assertion read `.runner`, a key from the earlier
ephemeral-VM design that `gate()` never returns, so it was
`expect(undefined).toBeUndefined()` — green no matter how badly the
trust level leaked into /triage or /tmux. Asserts the real keys now,
on both non-verify paths.
4. Three screen arms had no scenario at all: package-manager config,
the long-opaque-line awk, and the unconfigured-model fail-closed
path. All three are covered, each with a negative control so the arm
is not merely refusing everything — the genuine registry URL and a
700-char lockfile integrity line must still pass.
Mutation-verified 6/6, each failure naming the payload it let through:
reverting the alternation (`prebuild was not flagged`), un-anchoring the
registry match (`registry.npmjs.org.evil.com was not flagged`), breaking
the package-manager grep (`.yarnrc was not flagged`), raising the awk
threshold, letting the unconfigured path fall through to clear, and
leaking verify_trust into /triage — which is exactly what the vacuous
assertion could never see.
93/93 tests; prettier, eslint and shellcheck clean.
* fix(triage): close post-run persistence gap and screen bypasses (#7985)
Maintainer review found the wipe only ran BEFORE external code, leaving
the next pool job to meet the allowlist sweep alone. Add a post-run wipe
with `if: always()` so a cancelled or timed-out external job still
cleans up.
The model screen silently truncated at 200 KB — a fail-open in a
fail-closed step. Refuse oversized diffs mechanically before the model
call.
The screen prompt still described an "ephemeral, credential-free
sandbox" from the deleted ephemeral-runner design, calibrating the
model for leniency the persistent pool does not warrant. Describe the
actual environment. Also fix the stale "hosted lane" comment.
Three cheap mechanical-screen hardenings from the review: drop the
root anchor on the package-manager config grep (a nested .npmrc is
just as dangerous), match `rename to` headers (a rename-only hunk
emits no `+++ b/` line), and relax the opaque-line awk from
"no space in the line" to "no space-free field >= 600" so a single
space anywhere is no longer a bypass.
94/94 tests; build, typecheck, lint clean.
* test(triage): cover the unfetchable-diff fail-closed screen guard (#7985)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
|
||
|
|
0c5a4deabd
|
test(integration): migrate flaky E2E tests to fake-openai-server (#7934)
* test(integration): migrate flaky E2E tests to fake-openai-server Migrate 39 real-model test cases to use deterministic fake-openai-server scripting, eliminating model output variance as a failure source. - tool-control.test.ts: 23 cases migrated (tool filtering, permission denial, canUseTool routing, priority rules) - abort-and-lifecycle.test.ts: 15 cases migrated (abort mechanics, lifecycle, cleanup, debug output) - list_directory.test.ts: 1 case migrated (CLI TestRig with env injection) channel-plugin.test.ts (3 cases) is intentionally left on real model — it tests the full WebSocket→AcpBridge→model pipeline and belongs in the nightly smoke layer per #7616. Closes #7616 * test(ci): move channel-plugin to nightly + relax assertions channel-plugin.test.ts tests the full WebSocket→AcpBridge→model pipeline with real model inference. Its assertions on specific model output ('4', 'pineapple', '50') are inherently non-deterministic. - Exclude from post-merge E2E jobs (Linux + macOS) - Add dedicated nightly-only job with continue-on-error - Relax assertions: verify response is non-empty rather than matching specific model output content - Add retry: 2 to each test case * fix(test): use streaming chunks for abort test, restore comment - abort-and-lifecycle 'should handle abort during query execution': switch from non-streaming content to contentChunks so the abort signal reliably lands while the response is still streaming - channel-plugin: restore existing comment per AGENTS.md convention * test(integration): address fake server review feedback * test(integration): trim fake server review assertions * test(integration): tighten fake server cleanup * test(integration): fix permanently-failing stdin-close case and harden list_directory against user settings The stdin-close case's fake server needle contained a raw quote that JSON.stringify-escaped transcripts can never match, and the scripted write_file was rejected by prior-read enforcement; the script now keys off a quote-free marker and reads before writing. list_directory now pins auth via CLI flags so a developer's ~/.qwen/settings.json cannot silently route the run to a real model endpoint. * test(integration): tighten fake server review regressions * test(integration): guard abort assertions * test(integration): fix abort timer race and dead coreTools assertions * test(integration): address fake server review findings * test(integration): isolate fake fast model settings * test(e2e): simplify isolated test setup --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
|
d0481ad884
|
feat(triage): lead the verify comment with a qualitative verdict, fold the Chinese (#7974)
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 / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* feat(triage): lead the verify comment with a qualitative verdict, fold the Chinese Maintainer feedback on the first 14-run day of the lane, pointing at the report on PR #7836: give a plain pass/no-pass, and make the layout English-by-default with the Chinese folded (the repo's PR-body convention), instead of doubling every head line. Three changes: - every headline leads with a qualitative call — ✅ passed / ❌ not passed / ⚠️ inconclusive / ⚠️ incomplete — ahead of the lane taxonomy. The mapping is honest about what each outcome can claim: only agent verdicts judge the code; a crashed, timed-out, or verdict-less run renders as incomplete/inconclusive, never as a pass or a fail, and a test pins that no process-outcome arm carries ✅ or ❌. - the Chinese version folds into one <details> whose SUMMARY carries the qualitative verdict — collapsed, a Chinese reader still sees 通过/不通过 in one line. English scope and assertion count stay unfolded; both language assertion lines are built from the same validated triple, so inconsistent JSON still suppresses both. - the #7836 root cause gets a Hard rule in the verify-pr skill. That report said "merge-ready — the 7 failures are all expected A/B base-cell failures proving the tests load-bearing" while assertions.json said fail:7, so the publisher's trust rule (merge-ready requires fail==0) correctly refused it and the headline degraded to "no usable structured verdict". Both sides told the truth about different questions. The rule fixes the semantics: an A/B control cell is an assertion that the base arm FAILS — when it does, that assertion PASSED. fail counts only unexpected outcomes, which is what makes a qualitative headline trustworthy at a glance. Mutation-verified 6/6: dropping the qualitative prefix, collapsing all Chinese quals onto one string, unfolding the Chinese scope, giving timeout a ✅, dropping the folded Chinese assertion line, and deleting the new Hard rule each turn at least one test red. The fifth mutation initially reported "landed: False" — the regex never matched and the file was untouched, so the green result proved nothing; re-applied as a line filter with a removed-block count, it kills two tests. One new shellcheck SC2016 info finding (backticks inside a single-quoted Chinese printf), same shape as the 11 pre-existing ones; info-level, does not gate. * test(triage): pin the dropped verdict arms and prepare-failure glyphs (#7974) Restore rendering coverage for the reachable infra-error and unknown catch-all case arms in the qualitative-headline bijection, and assert the prepare-failure path's qualitative glyphs, Chinese fold summaries, and the Chinese retry clause so a swapped emoji or dropped PREPARE_ATTEMPTS_ZH interpolation can no longer ship undetected. * fix(triage): migrate all weak headlines, explain merge-ready mismatch (#7974) * fix(triage): fold Chinese mismatch explanation, differentiate distrust warnings (#7974) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code CI Bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
|
5daf75197e
|
ci: add Windows runner smoke test (#8008)
* ci: add Windows runner smoke test * ci: use built-in Windows PowerShell * ci: secure Windows runner smoke trigger * ci: run full test suite in Windows validation * ci: keep Windows validation manual |
||
|
|
324b5fba38
|
fix(release): skip notes-start-tag when previous release diverges from target (#7969) (#7970)
* fix(release): skip notes-start-tag when previous release diverges from target (#7969) * style(release): fix prettier formatting in release workflow (#7969) * fix(release): warn when notes-start-tag is skipped for a divergent tag (#7969) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> |
||
|
|
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
|
||
|
|
33414ab1b0 |
fix(release): raise model timeouts and shrink batch size for slow networks
The AI release-notes generator timed out on every batch in the v0.21.1 finalize run on a GitHub-hosted runner (US, Azure westus3) hitting a remote LLM endpoint. The 60s per-request timeout was too close to the edge: two successful requests took 54.2s and 56.7s, and every other batch hit the 60s abort. The 12-minute total budget was consumed by retries on those timeouts, and the circuit breaker opened after 3 consecutive failures, skipping highlights entirely. - timeoutMs: 60s -> 180s — give the model enough headroom to generate a full JSON response over a high-RTT cross-region connection. - totalTimeoutMs: 12min -> 30min — ~140 PRs at 8 per batch is ~18 batches; at ~90s each that's ~27 minutes of model time. - batchSize: 12 -> 8 — fewer entries per prompt means the model generates less per request, returning faster and reducing the chance of a single slow request dragging the whole batch. - workflow timeout-minutes: 15 -> 35 — match the new 30min budget plus a margin for git/npm setup. No changes to retry logic, circuit breaker, or prompt shape. |
||
|
|
6b9b861398
|
fix(ci): update the Qwen binary used by ECS runners (#8000)
* fix(ci): install Qwen into system npm prefix * test(ci): update ECS runner workflow guard |
||
|
|
15ca9818e0
|
fix(ci): restore verify workspace permissions (#7992) | ||
|
|
a7b1150816
|
fix(ci): clean stale .qwen/ before checkout to prevent Permission denied (#7977)
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 / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
On reused GitHub-hosted runners, a previous job (e.g. verify/tmux on the same runner pool) can leave .qwen/ files with restrictive permissions (chmod -R a-w). The next job's checkout step then fails with "error: unable to unlink old '.qwen/...': Permission denied" because git cannot delete the read-only files during workspace cleanup. Add a pre-checkout step that chmods .qwen/ writable and removes it before the checkout action runs. This is a no-op on fresh runners where .qwen/ doesn't exist yet. Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: 易良 <1204183885@qq.com> |
||
|
|
0c0ca5fed0
|
feat(autofix): defer suggestions after five change rounds (#7913)
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 / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* feat(autofix): defer suggestions after five change rounds * test(autofix): execute deferred jq queries against fixture data (#7913) * fix(autofix): scrub control markers from deferred feedback reports (#7913) * test(autofix): execute actionable reviews and issue-level filters against fixtures (#7913) --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
|
5b6714b9fb
|
fix(ci): give each job its own proxy wrapper directory (#7951)
The PR review job and the containerised triage jobs all built their gh/git proxy wrappers in one fixed directory under RUNNER_TEMP. On the shared self-hosted runner that path outlives a job, and the triage containers write it as root through the RUNNER_TEMP bind mount. Once that happened, the review job — running as the unprivileged runner user — could neither overwrite the wrapper nor remove the root-owned directory holding it, so every review scheduled on that runner died with EACCES while writing the wrapper, before the review itself had started. Each run now creates a private wrapper directory and removes it on exit, and the container jobs keep theirs on the container's own disk so they stop leaving root-owned state on the host. |
||
|
|
ddf4b8875d
|
fix(triage): make the build-process guard diagnosable and zombie-aware (#7858)
* fix(triage): make the build-process guard diagnosable and zombie-aware
The first real /verify run to reach the agent step (job 30267953352) was
stopped by the guard I added to stop detached lifecycle children from
outliving their step:
::error::Processes owned by the build user survived; refusing to start
the agent.
That is all it printed. No pid, no state, no command line — so a genuine
threat and a harmless leftover were indistinguishable, including to the
person who wrote it. The control had no observability of its own.
Two changes, both in the verify and tmux lanes:
- name the survivors. The error now lists each one as pid, state and
command line, so the next occurrence can actually be diagnosed.
- disregard zombies. A zombie has already exited and released everything
except its exit status: it cannot re-plant an artifact or touch the
agent's inputs, and it cannot be killed either — so counting one means
the check can never clear, no matter how many times SIGKILL is sent.
The container runs no init to reap orphans, so they are expected here.
The platform difference is the reason the old form could hang: Linux
pgrep reports defunct processes ("Defunct processes are reported." —
pgrep(1), procps-ng), while macOS pgrep does not list them at all
(verified directly: ps shows our zombie, pgrep does not). So this failure
mode was unreachable in local replay and only appears on the runner.
That same difference shapes the tests. The behavioural arm constructs a
real zombie, confirms it survives SIGKILL, and shows the state filter
drops it — but it cannot discriminate the old implementation from the new
one on macOS, because pgrep never saw the zombie there in the first place.
So the portable guarantee is asserted structurally: the filter must read
process state and exclude Z, and must not be a bare pgrep. Mutation-
verified 3/3 — removing the state filter, dropping the survivor names, or
reverting the message each turn one test red.
Also gives both proxy-watchdog tests an explicit timeout. They stream 20
chunks at 200 ms — 4 s before the stall arm even starts — so they cannot
fit vitest's 5 s default and were timing out on main.
* fix(triage): tolerate zero-process exit in build guard (#7858)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
|
||
|
|
25e357a279
|
feat(triage): make the verify report readable in Chinese (#7918)
* feat(triage): make the verify report readable in Chinese
The scope disclaimer under the verify headline has been bilingual since
the lane shipped. The verdict above it was not, and neither was the
assertion count — so a Chinese reader of the unfolded comment got the
caveat ("advisory evidence, not a review") and not the conclusion.
Three changes, all on the parts a reader reaches first:
- every headline carries a Chinese twin, across all eight arms —
merge-ready / findings / blocked / inconclusive from the agent, and
completed / fail / timeout / infra-error from the process outcome;
- the assertion count renders in both languages, off the same validated
object, so an inconsistent assertions.json still suppresses both and
the comment cannot grow a number no gate checked;
- in the report itself, 中文摘要 moves from last item to second, right
after the verdict. The whole report is already inside a <details> on
the PR, so leaving the Chinese summary at the bottom meant expanding
that fold and scrolling the entire English report — about 90 lines on
a real one — to reach the one section written for that reader. It
stays collapsed, so it costs everyone else exactly one line.
The headline test pins the pairing as a bijection, not as containment:
asserting only that each arm renders some Chinese leaves a single
hardcoded string — or one that echoes the English — passing every arm.
Mutation-verified 4/4: dropping the Chinese headline, collapsing all
arms onto one Chinese string, dropping the Chinese assertion sentence,
and moving 中文摘要 back to the end each turn one test red.
* fix(ci): correct cross-reference direction and cover all verdict branches in bilingual headline test (#7918)
* fix(ci): narrow bilingual headline comment to match actual scope (#7918)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
||
|
|
c7de1d7b7c
|
perf(ci): cut the E2E suite from ~40min to ~24min (#7798)
* perf(ci): shard the E2E suite and stop building it twice per job A full E2E run took about 40 minutes, twice the median gap between merges on main, so the post-merge suite could never keep up with the merge rate. Two thirds of that was avoidable work. Every job installed dependencies with the default `prepare` behaviour, which builds and bundles the whole workspace, and then ran explicit build and bundle steps that did it all a second time — about 3.5 minutes on Linux and 5 on macOS. The docker leg additionally let the sandbox script re-run install, build, bundle and pack on the host before building the image, none of which the image consumes: its Dockerfile rebuilds and packs inside its own builder stage. The remaining cost is the test run itself, which is now split across shards: three for each Linux sandbox mode and two for macOS. Vitest assigns files to shards by path hash, so the long tail of slow sdk-typescript files spreads out rather than clustering in one shard; replaying the last green run's per-file timings through that algorithm puts the slowest shard at about 6 minutes against the 16 to 25 minutes each leg used to spend. The matrix no longer fails fast either, because a cancelled sibling shard would hide most of the suite and report an incomplete failure set. Expected wall clock per run drops from about 40 minutes to about 24, with the sandbox image build — 13 minutes, now paid once per docker shard — as the largest remaining item and the obvious next target via Docker layer caching. * fix(ci): set QWEN_SANDBOX=docker on the sandbox image build step (#7798) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): quote the sandbox env value so yamllint passes The value added for the image build step was unquoted, which the repository's quoted-strings rule rejects, and note why the variable is needed at all. * fix(ci): stream docker build output in the sandbox image step (#7798) * fix(ci): skip duplicate build in sandbox image npm ci (#7798) The Dockerfile builder stage runs npm ci followed by explicit build and bundle steps. npm ci triggers the prepare script, which builds and bundles the whole workspace — duplicating the two steps that follow. Set QWEN_SKIP_PREPARE=1 on the npm ci so prepare only generates git-commit.ts, matching the same fix already applied to the host CI install steps. --------- Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> |
||
|
|
ce55360958
|
fix(ci): restore workspace ownership in cleanup to prevent EACCES (#7931)
The verify and tmux jobs chown the workspace to node:node for the build step, but the end-of-job cleanup never restores ownership. On self-hosted runners, the next job's checkout step (running as the runner user) fails with EACCES when trying to delete node-owned files in .qwen/ and node_modules/. Add a chown back to the runner user (detected via stat on $RUNNER_TEMP) at the end of both "Clean up runner workspace" steps. Only runs when the runner UID is non-root (i.e. on self-hosted runners where the runner user differs from root). 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> |
||
|
|
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> |
||
|
|
47118be482
|
feat(ci): Deduplicate E2E failure issues by commenting on existing issue (#7792)
* feat(ci): deduplicate E2E failure issues by commenting on existing issue * fix(ci): address failure issue review feedback * test(ci): cover CI issue dedup invariants * fix(ci): include comments in failure marker dedup * fix(ci): include issue comments in failure marker search * fix(ci): share failure issue title prefix * fix(ci): filter dedup issue titles * test(ci): assert SHA dedup runs before workflow-name dedup (#7792) * fix(ci): limit SHA dedup search to issue bodies * fix(ci): verify bot ownership before reusing title-matched issue (#7792) --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
|
ec62797c1c
|
fix(review): give the review retry the remaining time budget (#7852)
* fix(review): give the review retry the remaining time budget A transient API failure on the first review attempt was retried with a hardcoded 5-minute timeout while the first attempt got the whole 180-minute budget. The retry does not resume the failed review, it re-runs it from scratch, so on a large PR it spends those five minutes re-fetching the PR worktree, re-chunking the diff and re-launching review agents and is then killed on the clock before producing a single finding. Every retry after a transient failure therefore reported a timeout, and the resulting comment advised raising --timeout, which does not affect the retry cap at all. All attempts already share the single-review budget, so that budget is the only bound the retry needs. Give every attempt the remaining budget and gate the retry on having enough of it left to plausibly finish; below that, report the transient failure so the next run starts over with a full budget. * test(review): pin the per-attempt review timeout budget The retry-loop harness stubbed timeout with `shift; shift; exec "$@"`, which discarded the duration argument, so no test could observe how much budget each attempt was given. That is the invariant the retry fix turns on: a reintroduced per-attempt cap, or a miscalculated remaining budget, would have left the suite green while making every retry unusable. Record the duration the stub receives and assert on it. Cover the retry gate from both sides as well, since every existing scenario runs with a 180-minute budget and none of them exercise the threshold: a budget just under it must not start a retry that cannot finish, and one just over it must still retry. |
||
|
|
203093c53f
|
fix(triage): retry a transient npm ci before blaming the PR for it (#7884)
Run 30319209722 posted `Sandboxed verification: fail` on PR #7856 with "The PR could not be built ... treated as a PR failure verdict rather than an infrastructure failure." The PR changed six source files. The install died here: Error: spawnSync .../web-templates/node_modules/esbuild/bin/esbuild ETXTBSY at validateBinaryVersion (esbuild/install.js:99:28) ETXTBSY is npm writing a dependency's binary and that package's own install script exec'ing it before the write is closed — a race, and one the PR had no part in. The comment accused its author anyway. Both sandbox lanes now retry `npm ci` once. `npm ci` removes node_modules before installing, so the second attempt cannot inherit the half-written file; the transient class is simply absorbed instead of being classified. Retrying rather than classifying is the point. The obvious fix — match ETXTBSY in the log and downgrade to infra-error — would read text the PR's own lifecycle scripts can print, which is exactly the forgeable signal the verdict logic was rewritten to stop consulting: it let a PR launder its own deterministic breakage into "infrastructure, please re-run". A retry consults nothing. A genuinely broken tree fails twice and still earns `fail`, and the second attempt is only ever paid on a path that is already failing. The build is deliberately not retried: a compile error is deterministic, so a second run would only double the cost of an honest failure. Because the install now gets two chances, the sentence that blames the PR says so — "failed twice in a row". PREPARE_ATTEMPTS is initialised rather than defaulted at the point of use, or an inherited value would claim a single-shot `npm run build` had failed twice, which is the same false accusation pointed the other way. Mutation-verified 5/5: an unbounded retry, no retry at all, a retry on the build, dropping the initialisation, and reverting the wording each turn at least one test red. The retry itself is covered behaviourally — the real step text runs against a stubbed `runuser`/`npm`, so the tests count actual install attempts instead of asserting that a loop exists, which would pass on both a loop that never retries and one that never stops. Co-authored-by: wenshao <wenshao@example.com> |
||
|
|
083f5b97d0
|
fix(release): publish all channel packages, not just channel-base (#7845)
* fix(release): publish all channel packages, not just channel-base The release workflow only published @qwen-code/channel-base while the other 7 channel packages (dingtalk, feishu, github, qqbot, telegram, wecom, weixin) were never published to npm despite being non-private and version-synced. Add a loop step after channel-base that publishes all remaining channel packages with the same npm tag and dry-run support. * chore(release): record channel preview packages * chore(release): align channel preview versions * chore(release): restore channel source versions * chore(channels): emit declaration maps |
||
|
|
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 |
||
|
|
ef69eff20c
|
fix(ci): prevent worktree cleanup from failing the verify job (#7857)
The `*) [ -n "$wt_abs" ] && echo ...` pattern in the worktree cleanup loop exits 1 when a stale worktree entry has a missing directory (cd fails → wt_abs is empty → `[ -n "" ]` returns 1). Because the while loop is the last pipeline component under `set -e` + `pipefail`, this kills the entire step — contradicting the "never fail the job" intent and skipping the actual verification. Replace the `&&` compound with an `if` statement, which exits 0 when the condition is false. Fixes the "Clean stale agent state" and "Clean up runner workspace" failures seen in run 30273453000 (job 90001522381). Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
|
ad2252401b
|
ci(release): comment released-in version on merged PRs (#7814)
* ci(release): comment released-in version on merged PRs After a stable release is published, comment on each merged PR in the release range with a link to the release tag, so contributors can find which version shipped their change directly from the PR timeline. Collects PR numbers from squash-merge commit subjects between the previous and current stable tags, posts a marker-tagged comment, and skips PRs that already carry the marker (re-run safety). Nightly and preview releases are out of scope, since finalize only handles stable tags. * fix(ci): narrow release PR extraction * test(ci): cover release comment safety checks |
||
|
|
d70c5a28bb
|
fix(ci): add --init to container jobs to reap zombie processes (#7848)
The job container uses `tail -f /dev/null` as PID 1, which never calls wait() to reap orphaned children. After npm ci / npm run build, dead child processes become zombies. The pre-agent safety check (pgrep -u node) sees them, but pkill -KILL cannot reap zombies, so the agent refuses to start. Adding `options: --init` makes Docker inject tini as PID 1, which properly reaps orphaned/zombie processes. Applied to both tmux-testing and verify container jobs. |
||
|
|
7f8adc659e
|
fix(ci): add git safe.directory for container jobs (#7843)
The self-hosted runner runs as github-runner (uid 1000) but the job
container runs as root. Git detects the ownership mismatch on the
mounted workspace and refuses to operate ("dubious ownership",
exit 128). The runner does not auto-set safe.directory because it
sees no mismatch on the host side.
Add `git config --global --add safe.directory *` at the end of the
"Install PR resolver tools" step in both container jobs (tmux-testing
and verify) so all subsequent git commands trust the workspace.
|
||
|
|
3b8b8ba288
|
fix(ci): add default bash shell to container jobs in qwen-triage (#7838)
The `tmux-testing` and `verify` jobs run inside a `node:22-bookworm` container where `/bin/sh` is dash. Steps using `set -o pipefail` (a bashism) fail with "Illegal option -o pipefail" because the default shell for `run:` steps is `sh`. Add `defaults: run: shell: bash` to both container jobs so all steps consistently use bash, matching the non-container jobs that already specify `shell: 'bash'` on individual steps. Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
|
2e08486b52
|
fix(triage): carry the /verify lane's hardening across to /tmux (#7753)
* feat(triage): add sandboxed /verify deep-verification lane @qwen-code /verify on a PR now runs a local-verification-style evidence round in the isolated /tmux sandbox contract (container, token-free agent env, loopback model proxy, author-write gate) and publishes the report via a separate PR-code-free job: - new verify job: merge-ref checkout at depth 2 (base tip + PR head for A/B), skills pinned from base so the tree under test can never rewrite its own verifier, PR-planted tmp/*-verify-* artifacts dropped, git exec-vector sweep for the persistent workspace, agent verdict allowlisted before it reaches workflow outputs - new publish-verify job: upserts one marker comment (running status -> final report), HTML-escapes the untrusted report, reports skip/na/ prepare-fail/infra outcomes explicitly since /verify is always an explicit request - new verify-pr skill: A/B load-bearing proof, vacuity check on new tests, mock-free wire-oracle harnesses, targeted gates, fixed report/ verdict/assertions artifact contract, counts-are-sacred rules - triage skill Stage 2c now names /verify (not just /tmux) as the trigger to recommend when a PR's central claim needs behavioral evidence The verify check-runs ride the issue_comment event, which the finalize workflow's event == "pull_request" universe structurally excludes, so they cannot pollute the CI table or the deferred-approval gate. * feat(triage): teach /verify round continuity and artifact-matched methods Fold two more hand-verification patterns into the verify lane: - round continuity: the resolve step snapshots the previous verify report (if any) into the agent context before the status upsert overwrites it, and the skill re-checks each prior finding at the new head (fixed/stands/superseded), scoping new probes to the delta - harness quality: prefer configuration seams over module interception, encode the upstream's real semantics in the fake peer, add decoy targets - artifact-matched methods: per-commit load-bearing tables for multi-commit PRs; workflow/CI PRs get embedded-script replay against real data, repo lint gates, and day-one trigger cost math from real event history; every new config knob must trace to an observable effect, and default-path dispatch combinations get probed - findings quality: blockers enumerate blast radius, demonstrate the sharpest consequence end-to-end when budget allows, and carry a collapsed minimal suggested fix preserving the original commit's intent * feat(triage): host /verify evidence images and encode quantified-A/B rules Borrow the image-evidence and quantified-verification patterns from hand-run rounds (#7265, #7471, #7686 r2 and the pr-assets convention): - publish-verify now hosts agent-produced evidence/*.png on the pr-assets branch (verify/pr<N>-<run>-<attempt>/) and appends them below the escaped report. Untrusted-payload discipline: strict filename allowlist, 8-image / 2 MB caps enforced in the find predicates, racing-push retry, and every failure degrades to a text-only comment. VERIFY_ASSETS_REMOTE is a test seam; the block was dry-run against a local bare remote covering hosting, hostile filenames, oversize files, dotfiles, missing branch, and no-image runs - skill: evidence images are named as kebab-case captions binding image to claim, before/after pairs over lone after-shots; follow-up rounds lead with a previous-finding status table (fixed/stands/superseded/declined, with adjudication) and re-measure instead of diffing the old report; size/perf claims get measured-metric Δ tables with residual deltas accounted for; unreachable branches get the configuration that reaches them constructed; defensive guards get their accept path checked against real production artifacts, not just mocked rejects * fix(triage): address /review suggestions on the verify lane - skill: local invocation resolves --repo and passes it to every gh call - skill: call out the dependency confound when the base A/B side reuses the PR-installed node_modules and the PR touches package.json/lockfile - workflow: document the pin step's bootstrap logic — issue_comment jobs run the default branch's YAML, so base always carries the verify-pr skill by the time this job exists * fix(triage): harden /verify gate, comment budget, and evidence hosting per review Address review round 5078770575 items 1-3 plus the cheap follow-ups: - authorize: /verify now requires write from BOTH the PR author (whose code runs) and the commenter (who spends a scarce runner slot + model budget) — a drive-by account can no longer burn 45 minutes of ecs-qwen on someone else's PR; duplicates check once; /tmux and /triage gates unchanged. Replayed 8 principal scenarios against a stubbed gh - authorize acks /verify with the eyes reaction from the always-hosted job, so a queued/saturated sandbox pool no longer means total silence - publish: emit_block escapes FIRST and caps the escaped size (45 KB for the report) — a raw-side cap let dense <>& content inflate past GitHub's 65,536-char comment limit, 422 the post, and strand the running status with no report at all; iconv -c keeps a UTF-8 sequence split by the byte cut (likely, given the mandated 中文 summary) from shipping broken; replayed: 50 KB dense report -> 45,873-byte body - publish: image cap is byte-exact (-size -2097153c; find's -2M rounds sizes UP to MiB, silently making the documented 2 MB cap 1 MiB), bytes must carry the PNG magic (extension is attacker-choosable), duplicate sanitized names dedupe instead of overwriting + double-rendering, and dropped images are reported in the comment instead of vanishing - publish: weak terminal notices (cancelled/infra/skipped/n-a) only replace this run's own running status; a previous round's real report survives as the marker comment and the notice posts fresh - publish: report.md/assertions.json lookups pin the artifact-dir shape and sort (bare find -name order is filesystem-dependent); the verify job's verdict.txt lookup sorts likewise - verify: global npm install runs from RUNNER_TEMP (the persistent workspace still holds the PREVIOUS run's tree, whose .npmrc would apply to a root install); both cleanup passes remove leftover tmp/ worktrees (git worktree prune alone only drops metadata); the run step no longer re-chowns 50k node_modules files; pr-assets clone sets its committer identity once so the racing-push rebase retry can commit - skill: worktree guidance now tells the agent to remove its base tree itself, with the workflow sweep as backstop only * fix(triage): close runtime-plant and stale-RUNNER_TEMP channels in /verify Address review round 2 (comment 5079157987) and the CHANGES_REQUESTED round on the verify lane: - run step re-sweeps tmp/*-verify-* AFTER npm ci/build and before the agent starts: the pin step's sweep runs before PR lifecycle scripts (postinstall etc.), which could re-plant a fake artifact dir whose zeroed timestamp deterministically wins the sorted collector. From the sweep on, only the agent writes those dirs; a steered agent forging its own artifacts remains the documented advisory-report residual - RUNNER_TEMP verify-results/verify-context are rm'd before mkdir: the pool is persistent and runner temp hygiene is runner-managed — a stale report or previous-report.md from ANOTHER PR must never ride along - symlinks are stripped from verify-results before upload: actions/upload-artifact dereferences them, so a node-planted link would exfiltrate whatever it points at into the artifact - a trusted commenter invoking /verify on a PR whose author lacks write now gets an explanation comment from the hosted authorize job instead of total silence (the commenter is checked first; drive-by accounts and API errors still get nothing); job timeout 45->60 so a slow install can never let the JOB limit kill the agent past its own graceful 25m budget - stale tmp/base-tree (skill's canonical scratch worktree) is removed by name at job start — a plain dir isn't git-registered, so the worktree sweep alone misses it and the next worktree add would fail - scripts/tests/qwen-triage-workflow.test.js gains a verify-lane describe block: an 8-arm stub-gh replay of the dual principal gate (drive-by deny, author-without-write deny + explain flag, self-comment dedupe, 404 fail-closed, /tmux and /triage unchanged) plus guards for the post-prepare sweep placement, the symlink strip, and the RUNNER_TEMP resets — the replay found this commit's sweep edit had silently not applied, which is exactly the regression class it exists to catch * fix(triage): close proxy-hijack, gate-bypass, and false-verdict paths in /verify Address the Codex /review round (19 findings) and the bot's follow-up. Each fix was replayed locally; the proxy fix has a decisive A/B. Gate and routing: - the shell command match is case-insensitive: GitHub Actions expression comparisons ignore case, so `@QWEN-CODE /VERIFY` reached the step and fell through to the commenter-only branch — running the PR author's code with the author never checked - the verify ack and denial notice require github.event.issue.pull_request: /verify on a plain issue was acknowledged but could never report - publish-verify joins the verify job's per-PR concurrency group, and a failed PATCH falls back to posting fresh instead of going silent Untrusted-input paths: - the model proxy binds an EPHEMERAL port, reports it through a root-owned file, and its health check must echo a per-run nonce with the recorded PID alive. A/B with a squatter on 8787: the old code's proxy dies EADDRINUSE yet still reports enabled and points qwen at the squatter; the new code comes up unaffected on an ephemeral port - worktree-scoped git config is deleted before hooksPath is resolved: `extensions.worktreeConfig` is allowlisted and .git/config.worktree is invisible to `git config --local`, so a prior run could set core.hooksPath=/ and make the hook sweep's recursive delete walk / as root (verified locally). The sweep now also refuses any hooks path outside the repository's git dir - marker-comment lookups accept only bot-owned comments that START with the marker: any user can paste the marker and divert the bot into PATCHing a stranger's comment - the upload staging dir is re-flushed after npm lifecycle scripts Honest verdicts: - the docs-only classifier no longer uses a pipeline (grep -q made the writer take SIGPIPE, so under pipefail a long file list with an early code file classified a code PR as docs-only and skipped verification), and executable markdown/YAML (.qwen, .github/workflows, scripts) is classified as behavioral before the extension rule - tee's status is checked alongside qwen's: a full results volume made a truncated evidence stream publish as pass - 137 is split by elapsed budget into watchdog timeout vs crash/OOM - the agent's verdict is honored only for VERDICT=pass with a report and zero failed assertions; otherwise the process outcome headlines and the scope paragraph says the run did not complete - verdict.txt is read through a bounded prefix (SIGPIPE under pipefail) Skill contract corrections: per-commit tables only when the commits are reachable at depth 2 (else aggregate + Not covered); internal workspace symlinks must have their realpaths asserted before a base control is trusted; repo lint gates and event-history cost math are qualified to what the token-free container can actually run; --repo is never inferred from `origin` (a fork holds a different PR under the same number). Tests: 9 new guards, all mutation-verified (reverting each fix turns one red), including an executable escaping/size-cap/UTF-8 test for the publisher's own emit_block and a fix to the earlier command-file test, which matched the tmux job's identically named step. * fix(triage): re-establish the /verify trust boundary after PR code runs Third review round (31 findings). The unifying defect: everything the lane pinned or swept happened BEFORE npm ci/build executed PR-authored lifecycle scripts as node, so each control could be undone afterwards. Trust boundary, rebuilt in order before the agent starts: - kill every process owned by the build user and fail the step if any survives — a detached postinstall child could otherwise wait out each one-shot sweep and re-plant afterwards - re-pin .qwen from the base commit again, now root-owned and read-only: the prepare step chowns the workspace to node, so a lifecycle script could rewrite the very skill that defines /verify-pr - give the agent a fresh HOME/QWEN_HOME: qwen loads user-scope file commands from $HOME/.qwen, and /home/node belongs to the build user, so a planted commands/verify-pr.toml could shadow the pinned skill - the model proxy now requires a per-run bearer token, closing the blind-localhost-scan path to an unauthenticated signer for the real model credential (a command the agent itself launches still inherits it — documented residual, not closed) Authorization and lifecycle: - re-verify the PR author's write permission at execution time and pin the authorized head OID; refuse if the checked-out HEAD^2 differs, so a push during the runner wait cannot smuggle in unreviewed code - validate each principal separately: an empty author vanished in word splitting and left only the commenter checked - honor MAINTAINER_ECS_RUNNER_DISABLED with an explicit notice instead of queueing forever against a disabled pool - status comments carry a machine state marker; inferring 'running' from prose let a report quoting that sentence be overwritten - previous-report.md snapshots the newest substantive report, never a weak/cancelled notice, so prior findings survive into the next round - bot-identity lookup failures fail closed instead of widening the ownership filter to every user's comments - publish-verify uses a per-run concurrency group: a per-PR group holds only one pending job, so a second /verify could cancel a completed run's pending publisher Correctness: - install/build failures are classified: signals, ENOSPC, registry and network errors are infra-error, not a PR verdict - watchdog classification measures the child's own elapsed time, not shell-global $SECONDS which includes proxy setup - assertions.json must be three non-negative integers with a positive total and total == pass + fail before it counts as evidence - the proxy keeps its upstream deadline armed until the body ends and aborts upstream when the client disconnects - cleanups remove .qwen/tmp itself: PR code can make it a symlink, and globbing below it deleted the target's contents as root (verified) - emit_block materializes the escaped text and truncates on a character boundary via node — iconv -c passes an incomplete trailing sequence through on BSD (measured), which the new test caught Skill: local mode requires the same isolation CI provides and must not assume HEAD^1/HEAD^2 on a plain head checkout; shallow boundaries make rev-list counts unreliable for per-commit claims; never run scripts/lint.js with no arguments (it runs prettier --write and rewrites the tree under the harnesses); a vacuity check must fail the intended assertion, not the import. pr-workflow.md now says both sandboxed lanes need the author to have write, so triage stops recommending a guaranteed denial on external PRs. Tests: 9 more guards, all mutation-verified, including executable replays of the docs-only classifier (SIGPIPE + executable-markdown cases), the uppercase-command gate, the empty-principal deny, and the untrusted-image hosting path against a bare pr-assets remote. * test(triage): pass the classifier fixture through a file, not argv The new docs-only classifier replay passed on macOS and failed on CI with `Cannot read properties of undefined (reading 'trim')`: its 60,001-entry fixture is ~889 KB and was passed as a single argv element. Linux caps one argument at MAX_ARG_STRLEN (128 KB), so the spawn failed with E2BIG and stdout was undefined; macOS has no per-argument limit and only a ~1 MB total, so the same call succeeded locally (verified both). Write the list to a temp file and pass the path. The harness now also asserts the spawn succeeded, so a future spawn failure reports itself instead of surfacing as a TypeError on undefined output. * fix(triage): make the /verify report match what the run actually produced Three publisher findings, all introduced by my own previous round: - an artifact download failure (the step is continue-on-error) let the full-report path run with no results: the headline read 'completed' and the scope paragraph claimed the A/B, the harnesses and the gates had run when nothing had been delivered. The download outcome is now an input, and its failure gets its own body saying the results could not be retrieved - the prepare-failure branch ignored the verdict the prepare step had just computed, so an install killed by a registry outage or OOM (classified infra-error) still told the author 'this is treated as a PR failure verdict rather than an infrastructure failure' — the exact opposite. It now branches on the verdict, and an infra-classified prepare failure is a weak body that cannot overwrite a real report - weak notices were being snapshotted as the follow-up round's previous-report.md: they lack the running marker, so 'newest non-running comment' selected them. Bodies that carry findings now mark themselves (qwen-triage:verify-substantive) and the snapshot selects on that marker. A/B on the real jq: report A then cancelled B now snapshots A (101), the old filter picked B (102) Tests: 4 more guards, all mutation-verified — the publisher is rendered for each outcome with a stubbed gh and the assertions read the body it would post, and the snapshot test runs the workflow's own jq program verbatim against a paginate-shaped fixture. * fix(triage): stop PR build output from masquerading as an infra failure Two review findings plus a test-helper hazard: - classify_failure grepped the prepare log for bare words like ENOSPC and ETIMEDOUT, but that log is written by PR-controlled code: a genuine build failure that merely prints 'expected ETIMEDOUT to equal ok' would be published as an infrastructure incident, telling the author to re-run something that fails identically. The patterns are now anchored to lines only npm's reporter or the kernel emits ('npm ERR! code E…', 'npm ERR! network …', kernel OOM, bare 'Killed'); a signal exit still needs no log evidence. Replayed 10 cells: four PR-authored logs quoting infra words stay 'fail', five real diagnostics and one signal exit are 'infra-error' - the two execution-time controls added last round — re-verifying the author's permission after the runner wait, and refusing a head that moved since authorization — had no tests. Both are now executed: the re-auth snippet against a stubbed permission API (write proceeds and pins head_oid; read skips with a publishable reason), and the pin step against a real git repo with a real merge commit (matching head proceeds, moved head exits non-zero) - add a stepIn(job, step) test helper. Several step names exist in both the tmux and verify jobs, and the unscoped step() returns the first match, so a verify-lane assertion silently tests the tmux copy — that has now bitten this suite three times, including in this commit. * docs(triage): teach verify-pr test-only PRs, differential oracles, gate liveness Fold techniques from the round-2 verification on #7620 (an ANSI parser PR) that the skill had no equivalent for: - test-only PRs get their own method: a mutation A/B across TEST FILES (same mutants of the unmodified production file, only the test file swapped), reporting killed/total on both sides, requiring that no mutant regressed from killed to survived, checking that the killing assertion is the one the commit claims to have strengthened, and adjudicating every survivor as coverage gap or defect with independent evidence rather than by inspection - when the code emulates a known implementation, that implementation is the oracle: feed identical input to both and report disagreement counts per side, lift reference tables verbatim out of the shipped dependency, and build the corpus from bytes captured off a real producer alongside synthesized sweeps - prove a gate is live before citing it: plant a violation the linter must catch, confirm it is reported, remove it — a linter that matched no files exits 0 exactly like one that passed - attribute pre-existing failures by byte-identical failing file AND test names on both sides, with deltas, not just totals - when the base is far behind, verify the merge: trial-merge into current main, confirm it is conflict-free, and re-run the affected suite on the merged tree - round continuity gains its one legitimate shortcut: a production file proven byte-identical (sha256 quoted at both heads) carries prior evidence forward by construction * style(triage): reflow verify-pr skill to prettier's markdown wrapping The previous commit's added paragraphs were hand-wrapped and prettier --check flagged the file; the repo runs prettier over all of it. * test(triage): cover the disabled-runner-pool notice The kill-switch path had no test: a refactor could drop the notice and leave a /verify request acknowledged with 👀 but permanently unanswered, since the verify job refuses to start and publish-verify skips with it. Fold the step into the existing PR-guard loop (now scoped through stepIn, so it cannot match a same-named step in another job) and assert the parts that make the answer useful — the kill-switch and permission conditions, both languages, the alternative it points at, and the verify job's own exclusion of the disabled pool. All three mutations turn it red: removing the step, dropping its PR guard, or letting the verify job queue against the disabled pool. * fix(triage): repair a step-killing PIPESTATUS read and six forgeable controls Sixth review round, 12 findings. Several are regressions from my own two previous rounds; the first would have broken every single run. - `AGENT_STATUS=${PIPESTATUS[0]}` is itself a command and resets PIPESTATUS, so the next line's ${PIPESTATUS[1]} was unset and `set -u` aborted the step immediately after the agent finished — before artifact collection, the verdict, or anything else. Verified by replaying the exact structure: 'PIPESTATUS[1]: unbound variable'. Both elements are now snapshotted in one command - concurrency predicates were broader than the job conditions they guard, and GitHub evaluates concurrency BEFORE the job `if`: a /verify comment entered the triage job's shared per-PR group (where it could displace a pending /triage and then skip), and a /verify queued while the runner kill switch was on did the same to a real verification. Both predicates now match their job's runnable set exactly - an outward-resolving .git/hooks entry was only warned about and left in place, so the next root-owned git command would run it. It is now unlinked without traversing its target, a root-owned hooks directory is restored, and core.hooksPath is unset - the second .qwen pin re-derived HEAD^1 from git metadata after the workspace, including .git, had been handed to the build user. The base OID is now recorded while .git is still root-owned and the re-pin archives that content-addressed OID - classify_failure took both of its inputs from PR-controlled sources: a lifecycle script can exit with a signal status and can print any line the log patterns matched, turning its own deterministic breakage into 'infrastructure, please re-run' — which hid the failure and preserved a stale report. No infra verdict is derivable there, so the prepare step reports `fail` and lets the embedded log speak for itself - cleanups descended through PR-writable parents: `.qwen` itself can be a symlink, and the worktree sweep trusted git metadata with only a lexical prefix check. Symlinks are unlinked without traversal and worktree paths must canonicalize inside the workspace. Replayed all three escapes - skipped and docs-only outcomes upload no artifact, so the new download-failure branch pre-empted them and made their real reason unreachable; they are answered first now - a run that crashed before writing report.md still claimed the substantive marker, letting a headline overwrite the previous round's evidence. The marker now requires a report Skill: the byte-identical shortcut needs the whole input closure, not one file hash; the credential-free local path cannot call `gh` at all (fetch the metadata outside and mount it read-only); and the A/B base is `baseRefOid` in local mode, not `HEAD^1`. Tests: 7 new guards plus 4 updated to the new shapes, all mutation-verified (50/50). * fix(triage): answer dropped /verify requests and prove the proxy rejects Maintainer review (yiliang114), 7 items: - a third /verify while two runs are in flight is dropped by the concurrency group with no job and therefore no comment. The hosted authorize job now counts this workflow's other in-flight runs and says so; an API hiccup leaves the request alone rather than denying it - the proxy's bearer check had no executable test. It now starts the real proxy against a real upstream and issues real requests: no header and a wrong token are 401, this run's token is 200, and a route other than /chat/completions is 403 — with the health endpoint echoing the nonce - the 502 path forwarded the raw upstream error, which can name resolved hosts and TLS detail to PR code. It logs server-side and returns a generic failure - publish-verify inherited the 360-minute default; it downloads one artifact and posts one comment, so it is bounded at 10 - removing the log classifier last round left the comment block it replaced, which still said failures are classified from the exit status and the log. Deleted - that removal also left every install failure reported as the PR's fault, including a registry outage. There is exactly one signal here PR code cannot write — asking the registry ourselves, as root, with the container's resolver — so an install failure is downgraded to infra-error only when that probe fails. It proves reachability now rather than at failure time, so it can only ever downgrade, never confirm; a build failure has no equivalent and stays the tree's problem - the skill's local-invocation warning ran into the preceding sentence, which GFM renders as one paragraph Tests: 5 new guards, all mutation-verified (55/55). * fix(triage): resolve hooks hermetically and mirror symlink guards at job end Maintainer review round (doudouOUC), 6 findings. Two were Critical and both reproduced: - the hooks sweep resolved its path with the ambient git config in play. With a global core.hooksPath set — which the reviewer has and I do not, which is why my earlier replay showed a false pass — `git rev-parse --git-path hooks` returns that global path, the in-git-dir guard reads 'outside', and a planted `.git/hooks` symlink survives untouched. A/B: old code leaves the symlink under a global hooksPath, new code removes it in both environments and never touches the link target. Resolution now runs with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM pointed at /dev/null - the END-of-job cleanup still used the bare `rm -rf .qwen/tmp` that the start-of-job cleaner was hardened against two rounds ago. The agent executes PR code between the two, so the end is no safer than the start: it now unlinks symlinks without descending and canonicalizes worktree paths inside the workspace before deleting Plus four suggestions, all valid: - the saturation notice counted this workflow's in-flight runs across every PR while the concurrency group is per-PR, so a run on another PR would trigger a warning about a queue that does not exist. It now matches on the PR title (the only per-PR handle an issue_comment run record carries) and stays silent when that cannot be resolved - the skill recommended `require.resolve` for the workspace-realpath check; these packages are ESM-only with import-only exports, so it throws ERR_PACKAGE_PATH_NOT_EXPORTED and reads like a missing module. Verified, and replaced with `readlink -f node_modules/@qwen-code/...` - the symlink-escape test inherited the developer's git config, which is what hid the first finding. It now runs with global/system config neutralized AND repeats the case with a global core.hooksPath planted - the publisher's build-phase arm was never rendered by any test (every case used 'install'), so a typo in that command name would have shipped. Now covered, along with an unrecognized phase Mutation-verified 4/4. The hooks guard needed a discriminating assertion: git's own `*.sample` files must survive the sweep, because the outward-path fallback removes the whole directory and would otherwise satisfy a bare 'planted hook is gone' check. * fix(triage): count only /verify runs for saturation, and test the PATCH arm Bot review round, 2 suggestions, both valid: - the saturation notice matched runs by PR title, which narrowed to this PR but not to /verify. /triage and /tmux live in their own concurrency groups, so two of those in flight would warn about a verify queue that is actually empty. It now also requires the run to have a job named 'verify' — the run record carries no command, but its job list does. Replayed: two non-verify runs stay silent, two verify runs warn - every publish fixture returned an empty comments listing, so the PATCH arm was never executed: a broken PATCH would have stranded the running status comment and posted a duplicate below it, with the suite green. The publisher now runs against a stubbed listing and the test asserts which verb went to which comment id — bot-owned live status is PATCHed in place, an absent comment posts fresh, and a marker comment owned by someone else is left alone and posted around Mutation-verified 3/3: counting every command, never PATCHing, and accepting foreign-owned markers each turn one test red. Two stub bugs found while writing these, both mine and both silent: ${*#pattern} applies per positional parameter rather than to the joined string (yielding a wrong run id), and the paginate fixture needs one array per page, not an array of pages. * fix(triage): fix the real silent drop and drop the step built on a wrong premise Review round 4. The blocker was mine twice over: the saturation notice I added last round had GitHub's concurrency semantics backwards, and the silent drop it claimed to cover was somewhere else entirely. - GitHub cancels the OLDER pending run in a group and admits the new one (confirmed against the workflow-syntax reference). My step told the person who had just typed /verify that their request might be dropped, when theirs is the one that runs — and said nothing to the person whose queued run actually died. This PR already had it right in publish-verify's own comment, so the file contradicted itself and the user-facing copy followed the wrong half. The step is removed rather than reworded: with the fix below there is nothing left for it to warn about, and it cost 2+N API calls on every /verify. - the actual drop: a verify job cancelled while still PENDING never reaches a runner, so its outputs block — where the "|| github.event.issue.number" fallback lived — is never evaluated. publish-verify then read an empty PR_NUMBER, hit its own guard and exited 0, making the cancelled branch unreachable in exactly the scenario that produces cancellations. The fallback now lives where the value is read. Reproduced both arms by executing the real step: with a number the cancelled notice posts, with an empty one it only warns. - same one-line class in publish-tmux, fixed alongside. Two copy defects from the classifier removal, both mis-attribution pointed the other way: - the infra-error body still named a signal/OOM kill and a full disk, none of which the current prepare step can produce — infra-error now requires npm ci to fail AND the registry probe to fail. It names that condition only, and offers a re-run instead of asserting it is the fix. - the code comment above it still described the deleted classifier. Also fixes the indentation break an earlier scripted edit left in the publish body builder, and replaces the saturation test with one that executes the cancelled path. Mutation-verified 2/2; the copy needed its own guard, since reverting the wording alone left every test green. * docs(triage): teach verify-pr survivor accounting and observability regressions Fold techniques from the re-verification on #7709 that the skill had no equivalent for: - the mutation matrix must report the mutations that changed NOTHING, not only the ones that failed. Each survivor gets classified as an ordinary coverage gap or as dead code — a guard whose deletion leaves every test green is one of those two, and the difference is what the author needs. Survivors mirroring a pre-existing gap are labelled as such, and the set is framed as completeness reporting rather than merge conditions - the sharper case that report demonstrates: a test that passes for the WRONG REASON. If deleting the new guard leaves its own new test green, that test is pinned by an earlier early-return, not by the change, and asserts nothing about it. Name what actually pins it - and do not generalize from one dead guard to its siblings: the same report shows a clause that is unreachable on one path while being the only protection on another. Check each, report the contrast - observability regressions: when a change suppresses output, follow the value before calling the suppression correct. A bare catch on the path plus a field with no readers anywhere in the repo means the cause is now unobservable even in devtools — a real loss that no behavioural assertion can see - report structure gains a Corrections section: when an earlier round or bot comment described the code inaccurately, state the correct fact with evidence and label it as a correction to the description, not a request to change code. A wrong description left standing costs the next reader more than the original finding did * fix(triage): carry the /verify lane's hardening across to /tmux The /tmux job executes the same untrusted PR code, as the same user, on the same persistent self-hosted pool as the /verify lane that #7710 hardened. Five of those controls had no equivalent here. Each was found on the verify side by reproducing an attack or a failure, not by reading the code, so the same evidence applies unchanged. - the model proxy bound a FIXED port (8787). PR lifecycle scripts run before it, so a detached child can squat that port: the real proxy then dies with EADDRINUSE while the health probe succeeds against the squatter, and the agent takes its chat completions. Now an ephemeral port published through a root-owned file, a per-run nonce the health endpoint must echo, and a liveness check on the PID we started. Replayed with 8787 occupied: the proxy comes up on an OS-chosen port and answers with the nonce. - nothing swept planted artifact directories. npm ci/build run the PR's lifecycle scripts, which can create tmp/<name>-tmux-<ts>/ holding a report.md and a transcript; the collector globs *-tmux-* and the publisher takes the first match, so a planted directory could supply the comment's contents. Swept after the last PR-controlled process and before the agent. - the global npm install ran with the workspace as cwd, where the PREVIOUS run's checked-out tree still sits. npm reads a cwd .npmrc, and a --registry flag does not override script-shell or hooks, so that config reached a root-privileged install. It now runs from RUNNER_TEMP. - the end-of-job cleanup globbed below .qwen/tmp. PR code ran in this workspace, so either .qwen or .qwen/tmp can be a symlink out of the tree — verified on the verify lane, where the glob deleted the link target's contents as root. Symlinks are unlinked without descending. - emit_block capped the raw log then escaped it. Escaping inflates every & < > by 4-5 bytes, so dense content can push the assembled body past GitHub's 65,536-character comment limit, 422 the post, and leave no comment at all. It now escapes first, caps the escaped bytes, and truncates on a character boundary via node — BSD iconv -c passes an incomplete trailing UTF-8 sequence through unchanged. Tests: a tmux-lane-parity suite, all six mutations verified (restoring the fixed port, dropping the sweep, moving the install back, dropping the symlink guard, reverting to a raw-side cap, and dropping the character-boundary truncation each turn one test red; a no-op control correctly changes nothing). One pre-existing assertion updated: it pinned emit_block's old inline-capture shape, and the guarantee it protects — a render failure is caught — is asserted in the new form. Also adds the regression guard for the publish-tmux PR_NUMBER fallback that landed in #7710 without one: a job cancelled while pending never evaluates its outputs, so without the fallback the result comment silently does not post. * fix(triage): address review — symlink guard, artifact strip, bearer auth (#7753) * fix(triage): address R2 review — proxy parity, bearer wire tests, process kill (#7753) * fix(triage): address R3 review — publisher parity, dedup ownership, cap budget tests (#7753) * fix(triage): address R4 review — drop redundant tmux-lane .mjs guards (#7753) * fix(triage): address R5 review — tmp symlink sweep guard, proxy timer clear (#7753) * fix(triage): address R6 review — hoist proxy timer out of try, dead-upstream 502 tests (#7753) * fix(triage): address R7 review — make proxy watchdog idle, end stalled response (#7753) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
|
c003e17181
|
fix(autofix): answer every review thread, resolve the ones actually fixed (#7758)
* fix(autofix): answer every review thread, resolve the ones actually fixed Two gaps made a handled finding look unhandled on #7731. A finding the bot declined, deferred, or escalated keeps its thread open, but its reason was recorded only in the round summary — a separate comment. The reviewer who opens that thread sees their finding answered by silence. The agent now writes comment-replies.json and the push step posts each reason as a reply on that finding's own thread, leaving the thread open. Replies are neutralised like the summary body, since a reply is model output posted verbatim under the bot identity. Resolution was also keyed on "did I edit a file this round", so a Critical an earlier commit had already fixed stayed open and read as unaddressed. Key it on the finding being resolved in the code, which covers a prior commit's fix the agent re-verified still holds. * style: apply prettier to the new review-reply test * test(autofix): update stale escape-site comment from five to six sites * fix(autofix): reply at thread roots and guard the reply id, per review (#7758) Address the maintainer review on the in-thread reply mechanism: - Map each reply to its thread's top-level comment before posting. The feedback step lists every review comment, replies included, so an rc:<id> can be a reply id; GitHub rejects a reply aimed at another reply, which would have left escalated findings answered by silence. Hoist the threads GraphQL fetch above both the resolve and reply blocks so a reply-only round still has it, and fall back to the id as given past the page cap. - Skip any id already present in resolved-comments.txt so a finding is never both resolved and replied to; the match tolerates the rc: prefix and a trailing CR like the resolve block's own parsing. - Mutation-verify the previously untested ^[0-9]+$ id guard (the boundary between a model-authored id and an arbitrary API path), the -f body= field name, the type=="array" skip, and the new cross-check. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
|
e8e15e3fb0
|
fix(ci): rename triage status marker to avoid duplicate-guard collision (#7723)
* fix(ci): rename triage status marker to avoid duplicate-guard collision The lifecycle status comment used <!-- qwen-triage stage=status --> which the triage agent's duplicate guard sometimes matches as a prior stage marker (stage=N), causing it to exit without posting the actual triage analysis. Rename to <!-- qwen-triage-lifecycle --> so the guard never matches infrastructure comments. Fixes the probabilistic silent-triage gap seen in #7713. * fix(ci): update test assertions for renamed triage lifecycle marker * fix(ci): sync triage finalize status marker * test(ci): cover triage status marker parity * fix(ci): preserve triage marker consumers * test(ci): pin triage lifecycle marker checks * fix(ci): add bot-author filter and startswith to triage status lookups qwen-triage.yml's two status-comment lookups matched any comment containing the marker — a human reviewer quoting the marker would have their comment overwritten by the bot PATCH. Add select(.user.login == $bot) (already present in qwen-triage-finalize.yml) and resolve BOT_LOGIN via gh api user. Both workflows used contains() for marker matching; startswith() is a strict improvement since every status comment body begins with the marker. This prevents the demonstrated defect where the finalize step resolved EXISTING_ID to a bot-authored Stage comment that merely quoted the marker. * fix(ci): guard triage status bot lookup --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
|
e3cf1c3b45
|
revert: drop the stale-base un-park recovery (#7602) (#7640)
Reverts #7602. The Fleet Shepherd (qwen-fleet-shepherd.yml) now keeps the bot fleet within 25 commits of main by proactively update-branching, and #7595 already retries a stale-base gate rejection at ANY behind distance instead of parking — so a PR no longer parks because its base went stale. That leaves #7602 firing only on a PR parked by a GENUINE failure that later drifted behind main, where re-arming it just re-runs a real failure on a fresh base — speculative, near-zero value, and it carried its own autofix-handoff marker plus scan/report logic and tests. The retroactive cases it was built for (PRs parked before #7595) were already recovered by hand. Keeps #7595 (reactive stale-base gate recovery below the shepherd's threshold) and #7554 (check-driven stale-base sync) — both cover the sub-25-behind window and triggers the shepherd does not. Co-authored-by: wenshao <wenshao@example.com> |
||
|
|
883094da36
|
feat(triage): add sandboxed /verify deep-verification lane (#7710)
* feat(triage): add sandboxed /verify deep-verification lane @qwen-code /verify on a PR now runs a local-verification-style evidence round in the isolated /tmux sandbox contract (container, token-free agent env, loopback model proxy, author-write gate) and publishes the report via a separate PR-code-free job: - new verify job: merge-ref checkout at depth 2 (base tip + PR head for A/B), skills pinned from base so the tree under test can never rewrite its own verifier, PR-planted tmp/*-verify-* artifacts dropped, git exec-vector sweep for the persistent workspace, agent verdict allowlisted before it reaches workflow outputs - new publish-verify job: upserts one marker comment (running status -> final report), HTML-escapes the untrusted report, reports skip/na/ prepare-fail/infra outcomes explicitly since /verify is always an explicit request - new verify-pr skill: A/B load-bearing proof, vacuity check on new tests, mock-free wire-oracle harnesses, targeted gates, fixed report/ verdict/assertions artifact contract, counts-are-sacred rules - triage skill Stage 2c now names /verify (not just /tmux) as the trigger to recommend when a PR's central claim needs behavioral evidence The verify check-runs ride the issue_comment event, which the finalize workflow's event == "pull_request" universe structurally excludes, so they cannot pollute the CI table or the deferred-approval gate. * feat(triage): teach /verify round continuity and artifact-matched methods Fold two more hand-verification patterns into the verify lane: - round continuity: the resolve step snapshots the previous verify report (if any) into the agent context before the status upsert overwrites it, and the skill re-checks each prior finding at the new head (fixed/stands/superseded), scoping new probes to the delta - harness quality: prefer configuration seams over module interception, encode the upstream's real semantics in the fake peer, add decoy targets - artifact-matched methods: per-commit load-bearing tables for multi-commit PRs; workflow/CI PRs get embedded-script replay against real data, repo lint gates, and day-one trigger cost math from real event history; every new config knob must trace to an observable effect, and default-path dispatch combinations get probed - findings quality: blockers enumerate blast radius, demonstrate the sharpest consequence end-to-end when budget allows, and carry a collapsed minimal suggested fix preserving the original commit's intent * feat(triage): host /verify evidence images and encode quantified-A/B rules Borrow the image-evidence and quantified-verification patterns from hand-run rounds (#7265, #7471, #7686 r2 and the pr-assets convention): - publish-verify now hosts agent-produced evidence/*.png on the pr-assets branch (verify/pr<N>-<run>-<attempt>/) and appends them below the escaped report. Untrusted-payload discipline: strict filename allowlist, 8-image / 2 MB caps enforced in the find predicates, racing-push retry, and every failure degrades to a text-only comment. VERIFY_ASSETS_REMOTE is a test seam; the block was dry-run against a local bare remote covering hosting, hostile filenames, oversize files, dotfiles, missing branch, and no-image runs - skill: evidence images are named as kebab-case captions binding image to claim, before/after pairs over lone after-shots; follow-up rounds lead with a previous-finding status table (fixed/stands/superseded/declined, with adjudication) and re-measure instead of diffing the old report; size/perf claims get measured-metric Δ tables with residual deltas accounted for; unreachable branches get the configuration that reaches them constructed; defensive guards get their accept path checked against real production artifacts, not just mocked rejects * fix(triage): address /review suggestions on the verify lane - skill: local invocation resolves --repo and passes it to every gh call - skill: call out the dependency confound when the base A/B side reuses the PR-installed node_modules and the PR touches package.json/lockfile - workflow: document the pin step's bootstrap logic — issue_comment jobs run the default branch's YAML, so base always carries the verify-pr skill by the time this job exists * fix(triage): harden /verify gate, comment budget, and evidence hosting per review Address review round 5078770575 items 1-3 plus the cheap follow-ups: - authorize: /verify now requires write from BOTH the PR author (whose code runs) and the commenter (who spends a scarce runner slot + model budget) — a drive-by account can no longer burn 45 minutes of ecs-qwen on someone else's PR; duplicates check once; /tmux and /triage gates unchanged. Replayed 8 principal scenarios against a stubbed gh - authorize acks /verify with the eyes reaction from the always-hosted job, so a queued/saturated sandbox pool no longer means total silence - publish: emit_block escapes FIRST and caps the escaped size (45 KB for the report) — a raw-side cap let dense <>& content inflate past GitHub's 65,536-char comment limit, 422 the post, and strand the running status with no report at all; iconv -c keeps a UTF-8 sequence split by the byte cut (likely, given the mandated 中文 summary) from shipping broken; replayed: 50 KB dense report -> 45,873-byte body - publish: image cap is byte-exact (-size -2097153c; find's -2M rounds sizes UP to MiB, silently making the documented 2 MB cap 1 MiB), bytes must carry the PNG magic (extension is attacker-choosable), duplicate sanitized names dedupe instead of overwriting + double-rendering, and dropped images are reported in the comment instead of vanishing - publish: weak terminal notices (cancelled/infra/skipped/n-a) only replace this run's own running status; a previous round's real report survives as the marker comment and the notice posts fresh - publish: report.md/assertions.json lookups pin the artifact-dir shape and sort (bare find -name order is filesystem-dependent); the verify job's verdict.txt lookup sorts likewise - verify: global npm install runs from RUNNER_TEMP (the persistent workspace still holds the PREVIOUS run's tree, whose .npmrc would apply to a root install); both cleanup passes remove leftover tmp/ worktrees (git worktree prune alone only drops metadata); the run step no longer re-chowns 50k node_modules files; pr-assets clone sets its committer identity once so the racing-push rebase retry can commit - skill: worktree guidance now tells the agent to remove its base tree itself, with the workflow sweep as backstop only * fix(triage): close runtime-plant and stale-RUNNER_TEMP channels in /verify Address review round 2 (comment 5079157987) and the CHANGES_REQUESTED round on the verify lane: - run step re-sweeps tmp/*-verify-* AFTER npm ci/build and before the agent starts: the pin step's sweep runs before PR lifecycle scripts (postinstall etc.), which could re-plant a fake artifact dir whose zeroed timestamp deterministically wins the sorted collector. From the sweep on, only the agent writes those dirs; a steered agent forging its own artifacts remains the documented advisory-report residual - RUNNER_TEMP verify-results/verify-context are rm'd before mkdir: the pool is persistent and runner temp hygiene is runner-managed — a stale report or previous-report.md from ANOTHER PR must never ride along - symlinks are stripped from verify-results before upload: actions/upload-artifact dereferences them, so a node-planted link would exfiltrate whatever it points at into the artifact - a trusted commenter invoking /verify on a PR whose author lacks write now gets an explanation comment from the hosted authorize job instead of total silence (the commenter is checked first; drive-by accounts and API errors still get nothing); job timeout 45->60 so a slow install can never let the JOB limit kill the agent past its own graceful 25m budget - stale tmp/base-tree (skill's canonical scratch worktree) is removed by name at job start — a plain dir isn't git-registered, so the worktree sweep alone misses it and the next worktree add would fail - scripts/tests/qwen-triage-workflow.test.js gains a verify-lane describe block: an 8-arm stub-gh replay of the dual principal gate (drive-by deny, author-without-write deny + explain flag, self-comment dedupe, 404 fail-closed, /tmux and /triage unchanged) plus guards for the post-prepare sweep placement, the symlink strip, and the RUNNER_TEMP resets — the replay found this commit's sweep edit had silently not applied, which is exactly the regression class it exists to catch * fix(triage): close proxy-hijack, gate-bypass, and false-verdict paths in /verify Address the Codex /review round (19 findings) and the bot's follow-up. Each fix was replayed locally; the proxy fix has a decisive A/B. Gate and routing: - the shell command match is case-insensitive: GitHub Actions expression comparisons ignore case, so `@QWEN-CODE /VERIFY` reached the step and fell through to the commenter-only branch — running the PR author's code with the author never checked - the verify ack and denial notice require github.event.issue.pull_request: /verify on a plain issue was acknowledged but could never report - publish-verify joins the verify job's per-PR concurrency group, and a failed PATCH falls back to posting fresh instead of going silent Untrusted-input paths: - the model proxy binds an EPHEMERAL port, reports it through a root-owned file, and its health check must echo a per-run nonce with the recorded PID alive. A/B with a squatter on 8787: the old code's proxy dies EADDRINUSE yet still reports enabled and points qwen at the squatter; the new code comes up unaffected on an ephemeral port - worktree-scoped git config is deleted before hooksPath is resolved: `extensions.worktreeConfig` is allowlisted and .git/config.worktree is invisible to `git config --local`, so a prior run could set core.hooksPath=/ and make the hook sweep's recursive delete walk / as root (verified locally). The sweep now also refuses any hooks path outside the repository's git dir - marker-comment lookups accept only bot-owned comments that START with the marker: any user can paste the marker and divert the bot into PATCHing a stranger's comment - the upload staging dir is re-flushed after npm lifecycle scripts Honest verdicts: - the docs-only classifier no longer uses a pipeline (grep -q made the writer take SIGPIPE, so under pipefail a long file list with an early code file classified a code PR as docs-only and skipped verification), and executable markdown/YAML (.qwen, .github/workflows, scripts) is classified as behavioral before the extension rule - tee's status is checked alongside qwen's: a full results volume made a truncated evidence stream publish as pass - 137 is split by elapsed budget into watchdog timeout vs crash/OOM - the agent's verdict is honored only for VERDICT=pass with a report and zero failed assertions; otherwise the process outcome headlines and the scope paragraph says the run did not complete - verdict.txt is read through a bounded prefix (SIGPIPE under pipefail) Skill contract corrections: per-commit tables only when the commits are reachable at depth 2 (else aggregate + Not covered); internal workspace symlinks must have their realpaths asserted before a base control is trusted; repo lint gates and event-history cost math are qualified to what the token-free container can actually run; --repo is never inferred from `origin` (a fork holds a different PR under the same number). Tests: 9 new guards, all mutation-verified (reverting each fix turns one red), including an executable escaping/size-cap/UTF-8 test for the publisher's own emit_block and a fix to the earlier command-file test, which matched the tmux job's identically named step. * fix(triage): re-establish the /verify trust boundary after PR code runs Third review round (31 findings). The unifying defect: everything the lane pinned or swept happened BEFORE npm ci/build executed PR-authored lifecycle scripts as node, so each control could be undone afterwards. Trust boundary, rebuilt in order before the agent starts: - kill every process owned by the build user and fail the step if any survives — a detached postinstall child could otherwise wait out each one-shot sweep and re-plant afterwards - re-pin .qwen from the base commit again, now root-owned and read-only: the prepare step chowns the workspace to node, so a lifecycle script could rewrite the very skill that defines /verify-pr - give the agent a fresh HOME/QWEN_HOME: qwen loads user-scope file commands from $HOME/.qwen, and /home/node belongs to the build user, so a planted commands/verify-pr.toml could shadow the pinned skill - the model proxy now requires a per-run bearer token, closing the blind-localhost-scan path to an unauthenticated signer for the real model credential (a command the agent itself launches still inherits it — documented residual, not closed) Authorization and lifecycle: - re-verify the PR author's write permission at execution time and pin the authorized head OID; refuse if the checked-out HEAD^2 differs, so a push during the runner wait cannot smuggle in unreviewed code - validate each principal separately: an empty author vanished in word splitting and left only the commenter checked - honor MAINTAINER_ECS_RUNNER_DISABLED with an explicit notice instead of queueing forever against a disabled pool - status comments carry a machine state marker; inferring 'running' from prose let a report quoting that sentence be overwritten - previous-report.md snapshots the newest substantive report, never a weak/cancelled notice, so prior findings survive into the next round - bot-identity lookup failures fail closed instead of widening the ownership filter to every user's comments - publish-verify uses a per-run concurrency group: a per-PR group holds only one pending job, so a second /verify could cancel a completed run's pending publisher Correctness: - install/build failures are classified: signals, ENOSPC, registry and network errors are infra-error, not a PR verdict - watchdog classification measures the child's own elapsed time, not shell-global $SECONDS which includes proxy setup - assertions.json must be three non-negative integers with a positive total and total == pass + fail before it counts as evidence - the proxy keeps its upstream deadline armed until the body ends and aborts upstream when the client disconnects - cleanups remove .qwen/tmp itself: PR code can make it a symlink, and globbing below it deleted the target's contents as root (verified) - emit_block materializes the escaped text and truncates on a character boundary via node — iconv -c passes an incomplete trailing sequence through on BSD (measured), which the new test caught Skill: local mode requires the same isolation CI provides and must not assume HEAD^1/HEAD^2 on a plain head checkout; shallow boundaries make rev-list counts unreliable for per-commit claims; never run scripts/lint.js with no arguments (it runs prettier --write and rewrites the tree under the harnesses); a vacuity check must fail the intended assertion, not the import. pr-workflow.md now says both sandboxed lanes need the author to have write, so triage stops recommending a guaranteed denial on external PRs. Tests: 9 more guards, all mutation-verified, including executable replays of the docs-only classifier (SIGPIPE + executable-markdown cases), the uppercase-command gate, the empty-principal deny, and the untrusted-image hosting path against a bare pr-assets remote. * test(triage): pass the classifier fixture through a file, not argv The new docs-only classifier replay passed on macOS and failed on CI with `Cannot read properties of undefined (reading 'trim')`: its 60,001-entry fixture is ~889 KB and was passed as a single argv element. Linux caps one argument at MAX_ARG_STRLEN (128 KB), so the spawn failed with E2BIG and stdout was undefined; macOS has no per-argument limit and only a ~1 MB total, so the same call succeeded locally (verified both). Write the list to a temp file and pass the path. The harness now also asserts the spawn succeeded, so a future spawn failure reports itself instead of surfacing as a TypeError on undefined output. * fix(triage): make the /verify report match what the run actually produced Three publisher findings, all introduced by my own previous round: - an artifact download failure (the step is continue-on-error) let the full-report path run with no results: the headline read 'completed' and the scope paragraph claimed the A/B, the harnesses and the gates had run when nothing had been delivered. The download outcome is now an input, and its failure gets its own body saying the results could not be retrieved - the prepare-failure branch ignored the verdict the prepare step had just computed, so an install killed by a registry outage or OOM (classified infra-error) still told the author 'this is treated as a PR failure verdict rather than an infrastructure failure' — the exact opposite. It now branches on the verdict, and an infra-classified prepare failure is a weak body that cannot overwrite a real report - weak notices were being snapshotted as the follow-up round's previous-report.md: they lack the running marker, so 'newest non-running comment' selected them. Bodies that carry findings now mark themselves (qwen-triage:verify-substantive) and the snapshot selects on that marker. A/B on the real jq: report A then cancelled B now snapshots A (101), the old filter picked B (102) Tests: 4 more guards, all mutation-verified — the publisher is rendered for each outcome with a stubbed gh and the assertions read the body it would post, and the snapshot test runs the workflow's own jq program verbatim against a paginate-shaped fixture. * fix(triage): stop PR build output from masquerading as an infra failure Two review findings plus a test-helper hazard: - classify_failure grepped the prepare log for bare words like ENOSPC and ETIMEDOUT, but that log is written by PR-controlled code: a genuine build failure that merely prints 'expected ETIMEDOUT to equal ok' would be published as an infrastructure incident, telling the author to re-run something that fails identically. The patterns are now anchored to lines only npm's reporter or the kernel emits ('npm ERR! code E…', 'npm ERR! network …', kernel OOM, bare 'Killed'); a signal exit still needs no log evidence. Replayed 10 cells: four PR-authored logs quoting infra words stay 'fail', five real diagnostics and one signal exit are 'infra-error' - the two execution-time controls added last round — re-verifying the author's permission after the runner wait, and refusing a head that moved since authorization — had no tests. Both are now executed: the re-auth snippet against a stubbed permission API (write proceeds and pins head_oid; read skips with a publishable reason), and the pin step against a real git repo with a real merge commit (matching head proceeds, moved head exits non-zero) - add a stepIn(job, step) test helper. Several step names exist in both the tmux and verify jobs, and the unscoped step() returns the first match, so a verify-lane assertion silently tests the tmux copy — that has now bitten this suite three times, including in this commit. * docs(triage): teach verify-pr test-only PRs, differential oracles, gate liveness Fold techniques from the round-2 verification on #7620 (an ANSI parser PR) that the skill had no equivalent for: - test-only PRs get their own method: a mutation A/B across TEST FILES (same mutants of the unmodified production file, only the test file swapped), reporting killed/total on both sides, requiring that no mutant regressed from killed to survived, checking that the killing assertion is the one the commit claims to have strengthened, and adjudicating every survivor as coverage gap or defect with independent evidence rather than by inspection - when the code emulates a known implementation, that implementation is the oracle: feed identical input to both and report disagreement counts per side, lift reference tables verbatim out of the shipped dependency, and build the corpus from bytes captured off a real producer alongside synthesized sweeps - prove a gate is live before citing it: plant a violation the linter must catch, confirm it is reported, remove it — a linter that matched no files exits 0 exactly like one that passed - attribute pre-existing failures by byte-identical failing file AND test names on both sides, with deltas, not just totals - when the base is far behind, verify the merge: trial-merge into current main, confirm it is conflict-free, and re-run the affected suite on the merged tree - round continuity gains its one legitimate shortcut: a production file proven byte-identical (sha256 quoted at both heads) carries prior evidence forward by construction * style(triage): reflow verify-pr skill to prettier's markdown wrapping The previous commit's added paragraphs were hand-wrapped and prettier --check flagged the file; the repo runs prettier over all of it. * test(triage): cover the disabled-runner-pool notice The kill-switch path had no test: a refactor could drop the notice and leave a /verify request acknowledged with 👀 but permanently unanswered, since the verify job refuses to start and publish-verify skips with it. Fold the step into the existing PR-guard loop (now scoped through stepIn, so it cannot match a same-named step in another job) and assert the parts that make the answer useful — the kill-switch and permission conditions, both languages, the alternative it points at, and the verify job's own exclusion of the disabled pool. All three mutations turn it red: removing the step, dropping its PR guard, or letting the verify job queue against the disabled pool. * fix(triage): repair a step-killing PIPESTATUS read and six forgeable controls Sixth review round, 12 findings. Several are regressions from my own two previous rounds; the first would have broken every single run. - `AGENT_STATUS=${PIPESTATUS[0]}` is itself a command and resets PIPESTATUS, so the next line's ${PIPESTATUS[1]} was unset and `set -u` aborted the step immediately after the agent finished — before artifact collection, the verdict, or anything else. Verified by replaying the exact structure: 'PIPESTATUS[1]: unbound variable'. Both elements are now snapshotted in one command - concurrency predicates were broader than the job conditions they guard, and GitHub evaluates concurrency BEFORE the job `if`: a /verify comment entered the triage job's shared per-PR group (where it could displace a pending /triage and then skip), and a /verify queued while the runner kill switch was on did the same to a real verification. Both predicates now match their job's runnable set exactly - an outward-resolving .git/hooks entry was only warned about and left in place, so the next root-owned git command would run it. It is now unlinked without traversing its target, a root-owned hooks directory is restored, and core.hooksPath is unset - the second .qwen pin re-derived HEAD^1 from git metadata after the workspace, including .git, had been handed to the build user. The base OID is now recorded while .git is still root-owned and the re-pin archives that content-addressed OID - classify_failure took both of its inputs from PR-controlled sources: a lifecycle script can exit with a signal status and can print any line the log patterns matched, turning its own deterministic breakage into 'infrastructure, please re-run' — which hid the failure and preserved a stale report. No infra verdict is derivable there, so the prepare step reports `fail` and lets the embedded log speak for itself - cleanups descended through PR-writable parents: `.qwen` itself can be a symlink, and the worktree sweep trusted git metadata with only a lexical prefix check. Symlinks are unlinked without traversal and worktree paths must canonicalize inside the workspace. Replayed all three escapes - skipped and docs-only outcomes upload no artifact, so the new download-failure branch pre-empted them and made their real reason unreachable; they are answered first now - a run that crashed before writing report.md still claimed the substantive marker, letting a headline overwrite the previous round's evidence. The marker now requires a report Skill: the byte-identical shortcut needs the whole input closure, not one file hash; the credential-free local path cannot call `gh` at all (fetch the metadata outside and mount it read-only); and the A/B base is `baseRefOid` in local mode, not `HEAD^1`. Tests: 7 new guards plus 4 updated to the new shapes, all mutation-verified (50/50). * fix(triage): answer dropped /verify requests and prove the proxy rejects Maintainer review (yiliang114), 7 items: - a third /verify while two runs are in flight is dropped by the concurrency group with no job and therefore no comment. The hosted authorize job now counts this workflow's other in-flight runs and says so; an API hiccup leaves the request alone rather than denying it - the proxy's bearer check had no executable test. It now starts the real proxy against a real upstream and issues real requests: no header and a wrong token are 401, this run's token is 200, and a route other than /chat/completions is 403 — with the health endpoint echoing the nonce - the 502 path forwarded the raw upstream error, which can name resolved hosts and TLS detail to PR code. It logs server-side and returns a generic failure - publish-verify inherited the 360-minute default; it downloads one artifact and posts one comment, so it is bounded at 10 - removing the log classifier last round left the comment block it replaced, which still said failures are classified from the exit status and the log. Deleted - that removal also left every install failure reported as the PR's fault, including a registry outage. There is exactly one signal here PR code cannot write — asking the registry ourselves, as root, with the container's resolver — so an install failure is downgraded to infra-error only when that probe fails. It proves reachability now rather than at failure time, so it can only ever downgrade, never confirm; a build failure has no equivalent and stays the tree's problem - the skill's local-invocation warning ran into the preceding sentence, which GFM renders as one paragraph Tests: 5 new guards, all mutation-verified (55/55). * fix(triage): resolve hooks hermetically and mirror symlink guards at job end Maintainer review round (doudouOUC), 6 findings. Two were Critical and both reproduced: - the hooks sweep resolved its path with the ambient git config in play. With a global core.hooksPath set — which the reviewer has and I do not, which is why my earlier replay showed a false pass — `git rev-parse --git-path hooks` returns that global path, the in-git-dir guard reads 'outside', and a planted `.git/hooks` symlink survives untouched. A/B: old code leaves the symlink under a global hooksPath, new code removes it in both environments and never touches the link target. Resolution now runs with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM pointed at /dev/null - the END-of-job cleanup still used the bare `rm -rf .qwen/tmp` that the start-of-job cleaner was hardened against two rounds ago. The agent executes PR code between the two, so the end is no safer than the start: it now unlinks symlinks without descending and canonicalizes worktree paths inside the workspace before deleting Plus four suggestions, all valid: - the saturation notice counted this workflow's in-flight runs across every PR while the concurrency group is per-PR, so a run on another PR would trigger a warning about a queue that does not exist. It now matches on the PR title (the only per-PR handle an issue_comment run record carries) and stays silent when that cannot be resolved - the skill recommended `require.resolve` for the workspace-realpath check; these packages are ESM-only with import-only exports, so it throws ERR_PACKAGE_PATH_NOT_EXPORTED and reads like a missing module. Verified, and replaced with `readlink -f node_modules/@qwen-code/...` - the symlink-escape test inherited the developer's git config, which is what hid the first finding. It now runs with global/system config neutralized AND repeats the case with a global core.hooksPath planted - the publisher's build-phase arm was never rendered by any test (every case used 'install'), so a typo in that command name would have shipped. Now covered, along with an unrecognized phase Mutation-verified 4/4. The hooks guard needed a discriminating assertion: git's own `*.sample` files must survive the sweep, because the outward-path fallback removes the whole directory and would otherwise satisfy a bare 'planted hook is gone' check. * fix(triage): count only /verify runs for saturation, and test the PATCH arm Bot review round, 2 suggestions, both valid: - the saturation notice matched runs by PR title, which narrowed to this PR but not to /verify. /triage and /tmux live in their own concurrency groups, so two of those in flight would warn about a verify queue that is actually empty. It now also requires the run to have a job named 'verify' — the run record carries no command, but its job list does. Replayed: two non-verify runs stay silent, two verify runs warn - every publish fixture returned an empty comments listing, so the PATCH arm was never executed: a broken PATCH would have stranded the running status comment and posted a duplicate below it, with the suite green. The publisher now runs against a stubbed listing and the test asserts which verb went to which comment id — bot-owned live status is PATCHed in place, an absent comment posts fresh, and a marker comment owned by someone else is left alone and posted around Mutation-verified 3/3: counting every command, never PATCHing, and accepting foreign-owned markers each turn one test red. Two stub bugs found while writing these, both mine and both silent: ${*#pattern} applies per positional parameter rather than to the joined string (yielding a wrong run id), and the paginate fixture needs one array per page, not an array of pages. * fix(triage): fix the real silent drop and drop the step built on a wrong premise Review round 4. The blocker was mine twice over: the saturation notice I added last round had GitHub's concurrency semantics backwards, and the silent drop it claimed to cover was somewhere else entirely. - GitHub cancels the OLDER pending run in a group and admits the new one (confirmed against the workflow-syntax reference). My step told the person who had just typed /verify that their request might be dropped, when theirs is the one that runs — and said nothing to the person whose queued run actually died. This PR already had it right in publish-verify's own comment, so the file contradicted itself and the user-facing copy followed the wrong half. The step is removed rather than reworded: with the fix below there is nothing left for it to warn about, and it cost 2+N API calls on every /verify. - the actual drop: a verify job cancelled while still PENDING never reaches a runner, so its outputs block — where the "|| github.event.issue.number" fallback lived — is never evaluated. publish-verify then read an empty PR_NUMBER, hit its own guard and exited 0, making the cancelled branch unreachable in exactly the scenario that produces cancellations. The fallback now lives where the value is read. Reproduced both arms by executing the real step: with a number the cancelled notice posts, with an empty one it only warns. - same one-line class in publish-tmux, fixed alongside. Two copy defects from the classifier removal, both mis-attribution pointed the other way: - the infra-error body still named a signal/OOM kill and a full disk, none of which the current prepare step can produce — infra-error now requires npm ci to fail AND the registry probe to fail. It names that condition only, and offers a re-run instead of asserting it is the fix. - the code comment above it still described the deleted classifier. Also fixes the indentation break an earlier scripted edit left in the publish body builder, and replaces the saturation test with one that executes the cancelled path. Mutation-verified 2/2; the copy needed its own guard, since reverting the wording alone left every test green. * docs(triage): teach verify-pr survivor accounting and observability regressions Fold techniques from the re-verification on #7709 that the skill had no equivalent for: - the mutation matrix must report the mutations that changed NOTHING, not only the ones that failed. Each survivor gets classified as an ordinary coverage gap or as dead code — a guard whose deletion leaves every test green is one of those two, and the difference is what the author needs. Survivors mirroring a pre-existing gap are labelled as such, and the set is framed as completeness reporting rather than merge conditions - the sharper case that report demonstrates: a test that passes for the WRONG REASON. If deleting the new guard leaves its own new test green, that test is pinned by an earlier early-return, not by the change, and asserts nothing about it. Name what actually pins it - and do not generalize from one dead guard to its siblings: the same report shows a clause that is unreachable on one path while being the only protection on another. Check each, report the contrast - observability regressions: when a change suppresses output, follow the value before calling the suppression correct. A bare catch on the path plus a field with no readers anywhere in the repo means the cause is now unobservable even in devtools — a real loss that no behavioural assertion can see - report structure gains a Corrections section: when an earlier round or bot comment described the code inaccurately, state the correct fact with evidence and label it as a correction to the description, not a request to change code. A wrong description left standing costs the next reader more than the original finding did --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
|
172a567a30
|
ci(autofix): number the status comment like every other round message (#7748)
`effective_round` counts rounds already finished, and "Push and report" posts ROUND + 1 as the round it just performed. The status comment used the raw value, so the same round showed two numbers in one thread — observed on #7724 at 10:10 UTC: the report said "round 6/100" while the status said "AutoFix round 5 finished". Display ROUND + 1 in both status messages, guarded so a missing or non-numeric round degrades to the raw value instead of failing the step. Co-authored-by: wenshao <wenshao@example.com> |
||
|
|
cfd7c104bf
|
ci(autofix): show a live-progress status comment while a round runs (#7738)
* ci(autofix): show a live-progress status comment while a round runs Takeover engages, and then the PR thread goes quiet: review-address runs the agent for up to 80 minutes plus a verification gate, but nothing reaches the thread until "Push and report" at the very end. Observed on #7731 — 43 minutes of silence with no way to tell a working round from a stuck one. The agent's output already streams live to the Actions log; only the link was missing. Announce the round up front with that link, and flip the same comment to a terminal state when the round ends. Upserted by marker so one comment per PR is edited each round (edits notify nobody) instead of stacking against a 100-round cap. Both steps are gated on the stale-duplicate flag: the per-PR concurrency group runs a discarded duplicate AFTER the real round finalised, so an ungated finalize would overwrite that round's "finished" with its own "ended without publishing". Best-effort throughout — a failed status post warns and never costs a round. * ci(autofix): hand the status comment id to the finalize step Addresses review feedback: the announcement and the finalize each ran their own paginated comment scan, twice per round on a PR that can accumulate hundreds of comments over 100 rounds. The announcement already knows the id — it either found one or just created one — so it now writes it to $GITHUB_OUTPUT (capturing the id of a freshly posted comment via --jq) and the finalize consumes that. The finalize's scan is removed outright rather than kept as a fallback: an empty id means this round never announced, so no comment claims the round is working, there is nothing to flip, and a previous round's comment is already terminal (the next round's announcement re-PATCHes it either way). One scan per round instead of two, and less code. * test(autofix): assert finalize step error guards and dry_run gate (#7738) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix[bot]@users.noreply.github.com> |
||
|
|
3d395606ae
|
fix(triage): only the bot's own approval counts as already approved (#7737)
A maintainer approved PR #7620 three minutes before re-triggering `@qwen-code /triage`. The run reviewed the PR, scored it 5/5, and then reported "✅ Approved (5/5) — existing approval from prior run still valid, head SHA unchanged" without ever calling the approve API. The approval it read was the maintainer's, on the same head commit; the bot's own latest review there was a `/review` downgrade to COMMENTED, and its earlier approval had been dismissed by a push. The PR sat at 1 of the 2 required approvals with nothing in the run log marked wrong. The skill documents an "already exists, skip re-submitting" rule for CHANGES_REQUESTED only, and that snippet filters on the bot's login. Nothing covered approvals, so the rule was generalized to them with the author filter dropped. Since the maintainer's habit is to approve and then ask triage for the second vote, this reproduces on every re-run. Spell the approval check out instead of leaving it to inference: the skip applies only to the bot's own APPROVED review pinned to the exact reviewed commit — another account's approval is a different vote, a DISMISSED review is not an approval, and an approval on an earlier commit was already voided by the push. Keep the skip itself, so three re-runs still don't stack three approvals. Back it with a workflow check, since the failure is silent by nature. "Notify silent triage re-run" already detected that no review was added; it just said so in wording that read as a normal ending. It now reports whether the bot has a review of its own on the head commit, and warns when it does not. Also paginate the CHANGES_REQUESTED probe. An unpaginated read sees only the first page, and re-runs happen on exactly the heavily-reviewed PRs where the gating review has scrolled past it. Co-authored-by: verify <verify@local> |
||
|
|
638dc9a1c3
|
fix(triage): resolve finalize PRs from the open-PR list, not the commit association (#7706)
Live verification of the finalize loop (#7693) on its first real fork PR caught the deferred approval being silently dropped: CI landed green, the approve-on-green marker was in place, but commits/:sha/pulls returned an empty list for the PR's current head — the association endpoint is not reliable for fork-branch commits (workflow_run.pull_requests is likewise empty for forks). The run logged 'No open PR; nothing to finalize' and exited, so the approval never posted. Resolve PRs by filtering the open-PR list on head.sha as the primary source — it cannot miss the PR a current-head firing belongs to — and keep the association endpoint as the second source (when it works it also surfaces PRs whose head moved past the SHA, which powers the stale note). Union both, deduped. |
||
|
|
52f6eaf8f0
|
fix(triage): resolve stage comment ids by marker at patch time, harden model injection (#7703)
* fix(triage): resolve stage comment ids by marker at patch time, harden model injection Two hardenings from shepherding #7693, plus a wording fix: - Re-run comment updates now resolve the target comment id by its stage marker (bot-author-filtered, startswith match) immediately before each PATCH, instead of trusting remembered ids or list positions. Observed on a real re-run: the agent PATCHed the stage=3 comment with stage=1 content mid-run before self-correcting — with four bot comments in the thread, remembered-id bookkeeping is fragile. - The model-name injection step previously no-opped silently if the 'qwen3.7-max' literal ever left the skill (shipping the wrong signature in every comment), and corrupted the skill text on model names carrying sed metacharacters (/ & \). It now fails the job loudly when the target literal is missing and escapes the replacement. Covered by a behavioral test that runs the extracted step script against fixture files. - The finalize status text said 'stage comments above', but the status comment is created first, so the stage comments are below it — now 'in this thread'. * chore(triage): drop unrelated formatting churn from qwen-triage.yml The previous commit let prettier rewrite untouched lines (runs-on quoting, comment spacing) while formatting the edited step. Restore those lines to main's form; the diff now carries only the injection hardening and the status wording fix. * test(triage): pin the stage_comment_id recipe's load-bearing constraints Guards the startswith match and the bot-author filter in the skill's re-run comment-id recipe against silent regression — a contains match or a dropped author filter re-introduces the wrong-comment-overwrite bug this PR fixes. * test(triage): shim BSD sed only on darwin in the injection test The extracted step script uses GNU 'sed -i' (the step only runs on ubuntu runners), but this suite also runs in the macOS merge-queue job where BSD sed needs an extension argument after -i. Rewrite to sed -i '' on darwin only — on GNU sed a separated '' parses as the sed script, so the unconditional rewrite would break the Linux runs that mirror production. |
||
|
|
1f9318f974
|
feat(triage): stop in-agent CI polling, finalize evidence and approval after CI completes (#7693)
* feat(triage): stop in-agent CI polling, finalize evidence and approval after CI completes
The triage agent's Stage 2b polled pending checks for up to 10 minutes, but
this repo's unit suite runs ~30 minutes, so the poll always burned its full
budget, gave up with 'CI still running', and Stage 3 could then approve before
the suite finished (observed on a PR approved 12 minutes before its Test job
completed).
Split the wait out of the agent entirely:
- pr-workflow.md Stage 2b now forbids polling: fetch check-runs once, report
pending checks honestly, and wrap the CI table in qwen-triage-ci region
markers keyed to the reviewed SHA.
- Stage 3 defers a clean-verdict approval when checks are still pending: the
comment carries an approve-on-green marker instead of an immediate APPROVE.
- New qwen-triage-finalize.yml fires on workflow_run completion of 'Qwen Code
CI' / 'E2E Tests' and, with plain bash over the API (no model, no checkout),
rewrites the marked table region with the settled results and posts the
commit-pinned approval only when every check landed green — failing closed
on red checks, a moved head, or a closed/draft PR, and flipping the triage
status comment to say which way it resolved.
Markers are honored only in comments authored by the bot identity itself, and
check names (attacker-influenced on fork PRs) go through the same HTML-escape
chain the skill mandates for file paths.
Stage comments now land ~10 minutes sooner and the approval, when deferred,
lands at CI completion with full evidence instead of before it.
* fix(triage): address finalize review — broken red gate, table truncation, dead trigger
Review findings on the finalize workflow, all reproduced before fixing:
- Blocker 1: the RED jq used the array-first membership form, where | rebinds
. and .conclusion indexes an array — jq exits 5 every run, RED comes back
empty, [ "" -gt 0 ] errors, and control falls through to the approve path:
a red CI auto-approved. The gate now binds the conclusion before the
membership test (IN(...)), and the counters are numeric-validated so any
future jq failure reads as 'cannot attest', never 'approve'.
- Blocker 2: the table rendered raw check-runs — on a real PR (96 runs, 35
names, 68 skipped) alphabetical sort + head -60 truncated away every actual
test job. table_rows now dedups per name (latest run), drops skipped rows,
and sorts running/non-green first so the cap can only cut green rows.
Replayed against the same PR: 96 rows -> 16, unit suite present.
- The approval gate now reads workflow runs filtered to event=pull_request
(deduped per workflow) instead of head-SHA check-runs, which also carry
long-running bot orchestration jobs that would wedge PENDING above zero at
the exact moment the last CI workflow fires — silently dropping the
deferred approval forever. The skill's Stage 3 PENDING count matches.
- E2E Tests had no pull_request trigger (dead entry); the workflows list is
now exactly the six pull_request-triggered workflows, so the last finisher
always re-fires the job.
- Head/state re-check moved before the red/deferred verdicts so a
cancel-in-progress firing on a stale SHA cannot stamp a red status over
the new head's comment; the still-deferred branch now updates the status
comment instead of staying invisible.
- replace_region fails closed when the end-marker text only precedes the
begin marker (awk END guard) — previously that shape truncated the comment
body, eating the signature and reviewed-commit footer.
- Region content is deterministic (no run URL) so the no-op cmp works;
empty run list or unavailable gate skips approval; comment wording fixed
(workflow_run jobs are attributed to the default branch, so the self-check
exclusion is belt-and-braces, not load-bearing).
Tests now execute the decision logic, not just grep for it: gate_counts and
table_rows run against fixtures covering every conclusion class, non-PR
events, re-run dedup, skipped filtering, ordering, and both marker-order
failure shapes. 30/30 passing.
* fix(triage): keep a stale finalize firing from clobbering the newer review's status comment
The status comment is deliberately not SHA-scoped (the triage workflow
creates it unscoped; scoping only the finalize side would orphan the
pairing), so a finalize firing for an old SHA that loses the race against a
newer head's green approval would overwrite the ✅ status with a stale
warning. Guard the stale path: when the current head already carries bot
sha= markers (a re-review owns the status comment), stay silent; when the
head moved with no re-review yet — triage does not auto-rerun on
synchronize — the stale note is accurate and still posts. Closed/draft PRs
now just log instead of flipping the status.
* fix(triage): close the guardrail bypass and align the finalize table with the gate
Second review round, all four findings reproduced or confirmed before fixing:
- The approve-on-green marker was emitted in Step 1 while the fork-refactor
GUARD only ran in Step 2 — a marker that slipped out on a fork refactor
would have been honored by the finalize job on green CI, bypassing the
guardrail entirely. GUARD now computes in Step 1 and gates the marker's
emission, and the finalize job re-asserts it structurally from the PR
state it already fetched (null head.repo = deleted fork = blocked), with
a 'guarded' status message instead of an approval.
- table_rows now restricts check-runs to the suites of the same deduped
event=pull_request workflow runs the gate trusts. Without it, 5 of 8
rendered rows on this PR's own head were bot plumbing presented as CI
evidence; with it, 115 raw check-runs reduce to exactly the 3 CI rows.
- A firing that saw PENDING>0 after the approval landed flipped the status
comment back to 'deferred' with nothing to ever right it; the
already-approved branch now repairs the status.
- Zero surviving table rows (failed runs fetch, missing suite ids) skips
the region rewrite instead of blanking the agent's table, and
replace_region refuses an empty region file (an unchecked getline would
have deleted the region and its markers unrecoverably).
Nits: the house github.repository guard on the job, the table header
matches the skill template, and the run-URL stays out of the region so the
no-op cmp keeps working.
|
||
|
|
65b4a5a383
|
fix(ci): update qwen in the runner's active npm prefix (#7689)
* fix(ci): update qwen in runner npm prefix * test(ci): cover writable runner prefix install |
||
|
|
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> |
||
|
|
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) |
||
|
|
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> |
||
|
|
d550aeabee
|
fix(triage): actually restrict the CI review agent's tools (#7647)
The `settings_json:` input on the Qwen Code action does not exist — the action
reads `settings:`. The block was silently dropped ("Unexpected input(s)
'settings_json'" in the run logs), so the triage agent ran with the full
default toolset and no restrictions in a job that carries a write PAT.
- Rename to `settings:` and express it in the current schema (tools.core +
permissions.deny); verified to load through the real settings pipeline
(11 tools registered, 106 deny rules active).
- Deny interpreters, build tools, shells, network binaries, path execution, and
the git/gh write subcommands that execute configured commands or materialize
PR code. This is defense-in-depth, NOT a boundary: under the action's --yolo a
command denylist cannot be one (`$(...)` is not reliably deny-matchable, and
exec can hide in other commands) — see the in-file note. The real controls are
the skill's no-PR-code rule, ephemeral runners for fork PRs, and the git
exec-vector cleanup added here. Residual token-exfil-under-injection risk is
documented, with token isolation flagged as the structural follow-up.
- Route fork PRs (and comment/dispatch reruns, which may target fork PRs) to
ephemeral hosted runners so a steered agent cannot persist on the shared ECS
pool. Issue triage and same-repo PRs keep the ECS pool.
- Wipe stale git hooks/config/aliases each run before checkout.
Depends on the skill change that switches test evidence to the CI API and the
CHANGELOG fetch to `gh api`; merge that first so the npm/curl denials are
harmless.
Co-authored-by: verify <verify@local>
|
||
|
|
66da87d7ad
|
ci(triage): surface live progress via an early status comment (#7654)
`@qwen-code /triage` runs the agent as one long workflow step, so the PR thread stays silent until the first stage comment lands — the maintainer can't tell it started or how far along it is. The agent's output already streams live to the Actions log; the run link was only surfaced at the end. Post a `stage=status` comment up front carrying that live run link, and finalize the same comment (by marker, so a re-run reuses one comment) to a terminal state at the end. Covers manual, auto (pull_request_target), and dispatch triage. Best-effort — a failed status post never fails triage. Co-authored-by: wenshao <wenshao@example.com> |
||
|
|
09f01de881
|
ci: label a PR that closes an issue its own author opened (#7630)
* ci: label a PR that closes an issue its own author opened
Some PRs fix an issue the PR author themselves reported — self-reported
and self-fixed. That is not wrong, but the problem was never
independently validated, so a reviewer wants to check the issue is real,
not only that the fix is correct. This applies a `review/self-reported`
label so that shows at a glance and can be filtered.
A small pull_request_target workflow, metadata only (it never checks out
the PR's code): it reads the PR's closingIssuesReferences ("Fixes/Closes
#N" plus the Development-sidebar links) and, if any of those issues was
opened by the PR author, adds the label; it removes the label if the
link is later re-pointed or dropped. PR-controlled values reach the
script only through env, never interpolated into the run body.
* ci: single-quote workflow string values for yamllint
The repo's .yamllint.yml enforces quoted-strings (quote-type single,
required). The initial workflow left name, on/types, permissions,
concurrency group, runs-on, and the env values unquoted, failing the
Test job's yaml lint. Single-quote them (double where a value contains
single quotes, block scalar for the if), matching the qwen-fleet-shepherd
style. No behaviour change.
* fix(ci): never strip self-report label on a failed GraphQL query
Track whether the closingIssuesReferences query succeeded (API_OK) and gate label removal on it, so an API blip can no longer masquerade as "no self-reported link" and strip a correct label. Also re-run on synchronize so a commit-message "Fixes #N" link updates the label, add a fail-open regression test, and use the root yaml dependency in the test.
* fix(ci): add timeout-minutes and labelCreated test assertion (#7630)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
|
||
|
|
cb98102149
|
ci(autofix): add cross-package contract verification (#7642) | ||
|
|
edfb43e954
|
fix(sdk-java): Harden daemon transport reliability (#7603)
* fix(sdk-java): propagate daemon event epochs Pair SSE cursors with the daemon event epoch, learn validated response epochs, and fail closed when the epoch changes during prompt observation. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-java): harden daemon reliability follow-ups Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(sdk-java): cover duplicate SSE event epoch headers * test(sdk-java): restore retryable admission coverage Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
|
1697416a90
|
feat(autofix): auto-recover a PR parked on a stale base (#7602)
* feat(autofix): auto-recover a PR parked on a stale base #7595 catches a stale-base build failure DURING an address-review round, but a PR already parked when the base moved under it never gets a round for it to fire in: green PR checks, no new feedback (the handoff advanced the watermark), no conflict — so the scan skips it forever. Five managed PRs were stuck this way, 29-86 commits behind main, each "build failed on the agent-committed fix"; recovering them took a manual update-branch + /retry per PR. The gate-rejection handoff now drops a head-scoped autofix-handoff marker (only when a real fix was rejected — not a crash/timeout, which produced no fix to re-verify). The scan reads it and, while it still matches the live head and that head is behind main, merges main in and re-arms so the loop re-reads the feedback on a fresh base. Self-limiting: a push clears the head match, and the update makes it current so behind-main cannot re-fire. Every API call is fail-safe. The retroactive counterpart to #7595, closing the "parked when the base went stale" blind spot. * test(autofix): cover both-empty heads guard in stale-base unpark (#7602) * fix(autofix): add fail-safe handler to stale-base recovery comment (#7602) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
|
5003ee7a7c
|
feat(autofix): auto-update a PR red only from a stale, since-fixed base (#7554)
Some checks failed
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
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
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 Python / Classify PR (push) Has been cancelled
SDK Python / SDK Python (3.10) (push) Has been cancelled
SDK Python / SDK Python (3.11) (push) Has been cancelled
SDK Python / SDK Python (3.12) (push) Has been cancelled
* feat(autofix): auto-update a PR red only from a stale, since-fixed base A PR can be red purely because it merged a main that was broken then and is fixed now — observed twice today: a web-shell TS break and an agent-registry test, each stranding healthy PRs on a failure with nothing to do with them. The recovery was manual: merge current main and let CI re-run. The scan now does that automatically via GitHub's update-branch (a merge, never a rebase, so no force-push and no dismissed history). The single safety gate is that the SAME failing check is passing on current main. That one condition proves both halves at once: the red is base-inherited (green on main = not the PR's own bug) AND main is healthy on that check right now (so the merge cannot import a fresh breakage). It acts only when the PR is also BEHIND main (compare status behind/diverged) — otherwise the update is a no-op and the red is not stale-base after all. Self-limiting: after the update the PR contains main's head, so it is no longer behind and the next scan will not re-update. A failed update (merge conflict) is logged and the PR is left for a human. Runs before the feedback logic because a stuck-on-stale-base PR often has no new feedback at all — it just sits red — which is exactly what stranded #7490. * fix(ci): move pipefail fallback outside command substitution (#7554) * fix(ci): guard stale-base update-branch with expected_head_sha (#7554) * test(ci): pin fail-closed behavior for empty MAIN_HEAD and CMP_STATUS (#7554) * fix(ci): guard stale-base update-branch with DRY_RUN (#7554) * test(autofix): repair the merge-resolution test breakage Resolving the base conflict kept this branch's older CONSECUTIVE_FAILURE and handoff-decision tests (which predate main's PREPARE_OUTCOME env plumbing), so both broke, while it correctly re-anchored the stale-base and infra block extractions. Take main's test file wholesale — its consec-fail, handoff, infra and bilingual tests are all current — then re-add this PR's one intentional test (the stale-base auto-update), and re-anchor the infra test's block extraction onto the "# Auto-rerun a check that died on INFRASTRUCTURE" comment so it stops at that block instead of over-extracting past the now-adjacent stale-base block. Full suite green bar the pre-existing load flakes (eligibility recheck, permanent API failures terminal). * fix(autofix): fall through to feedback on failed update-branch; assert CAS param (#7554) * fix(autofix): address review — fix stale-base gate source, add base dimension, bound repetition (#7554) * fix(autofix): drop self-contradictory predicate 2, add state/PR_HEAD_OID tests (#7554) * fix(autofix): address review — identity-gate the stale-base write, correct the green-checks safety claim, per-selector guard test, gate the compare call (#7554) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> |