* feat(triage): add a deterministic flakiness gate to sandboxed verification
Closes#9125. PR #9086's ~50% mtime-assertion flake passed every automated
layer because each executed the changed tests exactly once — a coin flip a
single green run cannot distinguish from health. The gate re-runs the PR's
added/modified unit-test files N times (default 5, vars.QWEN_VERIFY_FLAKE_ROUNDS
to override, clamped to 2..10) through the same entry points CI uses and
compares outcomes per group across rounds.
Design constraints, each pinned by a workflow test:
- One-way authority: 'flaky' demotes the published headline (even a trusted
agent merge-ready); no gate value can raise or soften one. The gate runs
the PR's own test code, so it can always be neutered — but a gate that can
only demote is not worth forging.
- Divergence-only signal: a group failing identically every round is
deterministic (CI owns it) and an environment-sensitive suite must not
false-positive here; both report informationally, never demote.
- Fail open: the gate is not under -e and every terminal path exits 0 — a
gate bug reports verdict 'error' instead of taking down the verify lane.
- Honest file list: recorded from HEAD^1..HEAD before install/build hands
the workspace (and .git) to PR lifecycle code; the gate consumes the
root-owned recorded list and never re-derives the diff.
- Untrusted text stays out of outputs: summaries are fixed text plus
counters; PR-controlled paths live in flake-gate.log, embedded through
the publisher's escaping emit_block.
Job timeout raised 150 -> 175 for the gate's ~25m worst case (15m round
budget checked before each invocation + one 10m-capped in-flight run).
* fix(triage): survive the runner wrapper's -e, per-file gate granularity, hardened log staging
Round-1 review + sandboxed-verify feedback, all seven findings:
- set +e after set -uo pipefail: the runner wraps every run: block in
'bash -e -o pipefail' and set -uo does NOT clear that inherited -e, so
the first failing test invocation killed the step — fail-open inverted
to fail-closed for exactly the flaky/consistent-fail populations the
gate classifies (verify cells C/D). An EXIT trap additionally converts
any abnormal ending (set -u death) into the fixed 'error' verdict.
- Per-FILE groups: one runner invocation per changed test file, so a
consistently failing file can no longer mask another file's run-to-run
divergence behind a shared exit bit.
- Owning-package resolution: nearest ancestor package.json (nested
workspaces like packages/channels/base are entered themselves) plus a
vitest-config probe; unsupported runner families (packages/desktop's
bun test) and */e2e/* specs are logged out-of-scope instead of being
mis-run as permanent consistent-fail noise.
- Operands are ./-prefixed before %q, so a checked-in filename beginning
with '-' (e.g. --config=x) can never be parsed as a runner option.
- Log staging moved to a dedicated always() root step after the agent
exits — the last write to verify-results/flake-gate.log — and the
publisher pins that exact path instead of find|sort|head, so an early
agent abort cannot lose the matrix and agent-era PR code (which owns a
chowned verify-results) cannot control or shadow what is embedded.
- Detection math corrected: N=5 catches a 50/50 flake with ~94%
(1 - 2*(1/2)^5), not ~97% — all-pass and all-fail rounds both miss.
- New behavioral suite executes the extracted gate and publisher
fragments under the production wrapper itself (bash --noprofile
--norc -e -o pipefail) with scripted per-file P/F sequences: pass,
flaky-next-to-consistent-fail, consistent-fail, missing-list error,
out-of-scope n/a, nested-package + leading-dash operand, and the
seven-value one-way demotion — closing the structural blindness where
YAML-string tests stayed green while the shipped behavior regressed.
* fix(triage): isolate flake-gate rounds, classify infra exits, widen runner resolution (#9130)
Review-round fixes for the deterministic flakiness gate:
- Reset shared state between rounds (restore tracked files, tear down
test-user processes, fresh per-invocation TMPDIR) so a deterministic
test cannot fail on its own residue and fake a divergence (R1-8).
- Classify timeout/signal exits (124, 128+N) as infrastructure, not F
marks, and report the informational timeout verdict instead of a fake
flaky (R2-3/R3-2).
- Resolve the vitest runner by owning package + vitest's real config
list (vite.config.* included), keyed on the package lookup instead of
a packages/* prefix, so webui and integrations workspaces are re-run
instead of skipped (R2-2/R3-3).
- Narrow the scripts/tests arm to the pinned config's *.test.{js,ts}
include set so admitted-but-rejected files are skipped, not mis-run
into a bogus consistent-fail (R3-11).
- Harden the gate-log staging: kill leftover build-user processes, and
remove a planted destination entry before copying so a FIFO/symlink
can neither hang the copy nor redirect it (R1-5).
- Cap the embedded gate log at 10000 chars to keep the assembled
comment under GitHub's 65,536-char limit (R3-4).
- Record changed files with core.quotePath=false so non-ASCII test
filenames are not silently dropped (R3-5).
- Behavioral tests: hermetic timeout/pkill stubs (the suite no longer
depends on GNU coreutils, fixing the macOS red), infra-exit and
round-reset scenarios, trap-abort fail-open, node --test arm,
FLAKE_ROUNDS clamping, fixed-shape summary, record-step shape pins.
* test(triage): follow the widened special-file strip into the vitest twin pins
Commit 13708068bd widened the verify-lane artifact strip from symlinks
only to \( -type l -o -type p -o -type s -o -type b -o -type c \) so a
planted FIFO/socket/device cannot hang or redirect the collection — but
two pins in scripts/tests/qwen-triage-workflow.test.js still asserted
the old '-type l -delete' literal and went red (the Test job's only
failures). Update both pins to the full new expression; the intent they
guard (strip present, and AFTER the artifact copy) is unchanged, and the
tmux-side pin keeps the old literal because the tmux lane still uses it.
* fix(triage): build-user round resets incl. pre-round-1, artifact-loss-proof flaky demotion, mechanism-anchored pins
Round-4 review, all 22 findings (2 Critical dual-anchored + 20):
- Round-state reset (Critical): run the reset AS THE BUILD USER — a root
checkout restores node-mutated tracked files as root-owned inodes that
later node rounds cannot write (EACCES divergence) — add 'git clean
-fd' (no -x) so untracked round residue is dropped, and run one reset
BEFORE round 1 so lifecycle-script mutations cannot make the first
sample differ from the rest. Behavioral scenarios: untracked residue,
pre-gate tree mutation (both PPPPP/pass), each red without its fix.
- Staging step: continue-on-error (evidence-copying must not let the
publisher discard a recorded verdict as 'infrastructure failure'), and
the order chain (pkill -> dir guard -> mkdir -> unlink dst -> cp) is
now pinned by index comparison, not presence-only regexes.
- Publisher: the flaky demotion now fires in the artifact-download-
failure branch too — FLAKE_VERDICT travels via job outputs and does
not need the artifact — instead of a neutral 'results unavailable'.
- Pins anchored to mechanisms, not adjacency (R2-P1): exact-line record
assignment (kills ;/& status swallowing and covers the -c form), word-
based no-re-derivation, line-anchored 'timeout -k 30 600 runuser'
invocation (comment-proof, also pins the per-invocation cap), build-
user reset lines, agent/gate if-equivalence.
- Unpinned guards now pinned (R2-P2/P3): ACTIONS_* credential strip, the
env -u runner-file isolation, whole-env key set (a future secret in
the gate env must be an explicit test decision), intake extension set,
record->gate handoff filename, child-env line (CI/heap/TMPDIR).
- New behavioral scenarios: wall-budget expiry via a scripted date stub
(both timeout branches, pinning rounds_done placement), space-bearing
filename through %q as one operand, bilingual one-way demotion (the
collapsed Chinese summary is the one verdict a zh reader sees).
* fix(triage): NUL-delimited gate intake, zero-collection class, front-loaded matrix, hardened resets
Round-5 review, all 28 findings:
- NUL-delimited record end to end (git diff -z, grep -z, gate read -d ''):
quotePath=false only stops quoting of bytes >= 0x80 — ASCII specials
(backslash, tab, quote, control chars) stayed C-quoted and silently
failed the $-anchored line grep with no skip-log entry. The raw diff
never passes through $( ) (command substitution strips NUL). Intake
extension set gains .mts/.cts (vitest's default include collects them).
- Zero-collection class ('N' mark): a file the runner's include set
rejects exits 1 every round with 'No test files found' — publishing
that as consistent-fail claimed 'deterministic, CI owns it' with both
clauses false. All-uncollected lands n/a; mixed runs pass with the
not-collected count in the summary. Faking the marker can only
SUPPRESS a demotion the PR could already dodge — one-way authority.
- Per-invocation detail moved behind the matrix/verdict: the publisher
embeds the FIRST 10,000 chars, and failure tails (8 KB each) pushed
the promised per-round matrix past the cap in exactly the flaky runs
the demotion points at. Plus a bilingual fallback note when the gate
log could not be staged into the artifact.
- reset_round_state kills FIRST (a live daemon re-dirties the tree after
checkout), with SIGKILL + a bounded wait replacing the one-shot TERM.
- Behavioral hardening: runner-injection-env and operand-resolution
guards baked into the default stub (pins the cd and %q for every arm),
hostile filenames through the generic and node --test arms, mid-round
budget expiry, flaky-outranks-timeout, infra-exit amid divergence,
vitest.workspace.ts entry, exact summary counters, step-summary
read-back, gate status line in the demotion drive.
- Structural pins anchored to mechanisms: exact NUL-record statements,
continuation-proof no-re-derivation (plus log/show/whatchanged),
unset-before-invocation ordering, FLAKE_ROUNDS wired to the repo var,
inv_tmp lifecycle, record/gate if-pins, flake-before-agent order,
staging line-anchored order chain incl. both guard halves and the
guard's rm reaction, DOWNLOAD_OUTCOME wiring.
* fix(triage): close the desktop-app runnability hole and the staging replant race; demote in every terminal branch
Round-6 review (2 Critical + 19 Suggestion; 17 applied, 2 declined with
rationale on-thread):
- Desktop/docs-site exclusion (Critical): packages/desktop/apps/* each
carry a package.json plus a BUILD vite.config.ts, so the generic
resolver treated bun-family tests as runnable — 102/319 real desktop
test files misclassified, published under a false include-set-mismatch
diagnosis while draining the shared wall budget. Explicit skip arm
ahead of the generic arm; behavioral scenario pins both trees.
- Staging replant race (Critical): the one-shot pkill lost to setsid
daemons/continuous forkers, and verify-results stayed node-owned — a
survivor could swap the staged log for a symlink between cp and
upload-artifact's link-following enumeration: root-readable-file
exfiltration into the public comment. The kill now uses the bounded
survivor wait, and the directory is chown -R root:root before the
copy, revoking the replant capability regardless of the race.
- A recorded flaky now demotes in EVERY terminal publisher branch:
cancelled and job-failure used to post the neutral notice while
needs.verify.outputs still carried the verdict (the download-failure
branch already honored it). Bilingual, with run-log pointers.
- Dead 'uncollected' counter removed (ncoll already counts N marks).
- Tests: ghost-file and desktop-skip scenarios; behavioral child-env
guards in the default stub (CI/heap/TMPDIR-under-RUNNER_TEMP);
invocation-count ground truth for the mid-round budget stop; sixth
drive field pins the Chinese gate status line; adjacency-pinned record
statements; anchored credential-strip/wait-loop/publisher pins;
cancelled/failure/download branch pins; upload transport pins;
workflow/job-level env emptiness pins; guard then-body pin.
Declined (reasoning on the threads): whitespace-spelling re-derivation
evasions (bounded by one-way authority — re-derivation can only narrow
the gate), and an inv_tmp position pin (superseded by the stub's
behavioral TMPDIR guard, which every scenario now enforces).
* fix(triage): per-sample reset equivalence, 125-127/N-transition classification, intake+staging hardening
Round-7 Critical cluster on the flakiness gate:
- Reset runs before EVERY invocation (not once per round) so file i never
samples what files 1..i-1 left this round; HOME joins TMPDIR in the
per-invocation isolation; `git checkout HEAD -- .` restores from the
root-pinned commit instead of the index; `git clean -ffd` also drops
nested-.git dirs plain -fd refuses; both git calls gain the lane's
runner-injection strip and a `timeout -k 30 120` wrapper, and a failed
reset fails open to the fixed error verdict instead of sampling dirty.
- Exits 125-127 (timeout's own failure modes) classify as infrastructure
like 124/128+N; a per-file collection-state transition (N next to P/F)
is divergence, no longer collapsed to pass/consistent-fail.
- Record step: diff-filter gains T (typechange); a grep error (status 2)
fails the step loudly instead of starving the gate to n/a; the
owning-package walk hands its result through a variable, never a `$( )`
capture that strips trailing newlines.
- Staging: chown preserves modes, so the root re-own is completed by
`chmod -R go-rwx` and a post-revoke sweep before the copy.
Behavioral scenarios pin each defect (all fail pre-round, measured):
per-sample equivalence across residue/nested-repo/staged/HOME classes,
exit-127 infra classification, and N-transition divergence.
* fix(triage): give the gate a root-only home and rebuild the upload tree
Round-7 R7-8 (Critical) and the R1-5 line it supersedes, one root cause:
$RUNNER_TEMP's top level is uid-1000 mode-755 on this pool and the job
container's `node` is uid 1000, so PR-controlled code held OWNER WRITE
on the directory holding every gate working file — and each root-side
consumer followed symlinks. Four probe-confirmed exfiltration routes:
the staging source, the recorded list (swapped for /proc/<pid>/environ,
whose NUL-framed KEY=VALUE is exactly the intake framing, landing
ACTIONS_RUNTIME_TOKEN in the published log), the fixed-path round
output, and the destination entry itself (renameable even after the
in-place hardening, so a kill-race survivor could swap the whole tree
for a symlink farm that upload-artifact follows).
Removing the capability rather than patching each consumer:
- All gate working files (list, log, detail, round output) now live in
$RUNNER_TEMP/flake-gate, created 0700 root:root by the record step
(rm -rf first: the entry may be a plant from an earlier run on the
persistent pool). A directory node cannot enter is one whose entries
it can neither create, unlink, nor rename — routes (a)(b)(c) close by
construction.
- The gate verifies that home fail-closed before reading anything:
not a symlink, a directory, owned by the EFFECTIVE user (root in
production; keeps the extracted script runnable under a harness), and
mode 700 — otherwise the fixed `error` verdict, exit 0.
- Staging BUILDS a trusted upload tree in that home instead of hardening
the agent-era one: kill+wait unconditionally (node can unlink the log,
which must not skip the rebuild), then copy only regular files with
--no-dereference out of verify-results, then the authoritative log
last. The artifact now uploads from the rebuilt tree, so the entry the
enumeration walks was never in a PR-writable directory — route (d).
The publisher's paths are unchanged (same inner layout). Behavioral
scenario pins the fail-closed refusal; the staging pin chain follows the
rebuild order.
* fix(ci): close flake-gate TOCTOU, reset-integrity, and verdict-discard routes (#9130)
* fix(ci): keep sub-2-round gate stops informational and scrub startup env channels (#9130)
The reset-failure early stop and a mid-run home swap both reached
classification with fewer than two completed rounds — one published
`pass` off a single agreeing round, the other discarded samples that
already encoded a computed flaky, letting a PR dodge its demotion by
renaming the gate home. Both stops now degrade to the informational
timeout verdict under two rounds while an observed divergence still
demotes, and a swap after samples keeps the collected results.
The record/gate/staging blocks also scrubbed only PATH, while the
channels consumed at shell/loader startup stayed live: BASH_ENV and
the LD_* loader channels are now blanked at step env (with a
fail-closed check if the blank loses), BASH_FUNC_* imports are
dropped by a one-shot env -i re-exec whose child marker is positional
(an env sentinel would be forgeable through the same file-command
channel), and the GIT_* family is stripped on the reset and sampling
children. Staging additionally validates the run identity stamped by
the record step, and a verify-results stat race degrades to a skipped
copy instead of aborting before the authoritative log copy.
The count pin covered five of the seven stripped node-run commands,
which is what failed the scripts lane; the job timeout budget now
includes the resets the deadline check runs ahead of (175 -> 190).
* fix(ci): close gate re-exec startup races and stale-evidence paths (#9130)
* fix(ci): harden flake-gate startup decisions and close staging swap windows (#9130)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(ci): move the flake-gate home out of PR-writable space; pin git metadata
Rounds 8-16 kept re-reporting one class (R8-1, R8-36, R12-2 and its
re-reports, R13-21, R15-1, R16-1) because every fix was a re-validation
of a path that could still be swapped underneath it. The premise, not
the consumers, was wrong: rename(2) and unlink(2) need write permission
on the PARENT directory, never on the entry, and $RUNNER_TEMP's top
level is uid-1000 mode-755 on this pool while the container's node is
uid 1000. A 0700 root:root home there could always be renamed away
wholesale and replaced — so each added check (ownership, mode, inode
anchor, run-id marker, opened-directory re-stat) only narrowed the
window, and the inter-step window before a `uses:` upload could not be
closed at all, because that step cannot run shell.
- The home moves to /flake-gate. `/` is root:root 755 in this container:
node can neither create, unlink nor rename entries in it, so the
entry — and everything reached through it — is outside PR-controlled
reach by construction, with no window left to re-check. The upload
ships that tree directly. No env override: $GITHUB_ENV is uid-1000
writable, so a configurable home would be a PR-reachable channel, and
the record step rm -rf's whatever the home names. The extracted-script
harness relocates that one constant into its scratch tree instead, and
the production value is pinned structurally.
- inv_tmp stays under RUNNER_TEMP by necessity (the build user cannot
enter the root-only home) and gains the :? guard.
- .git is re-owned to root after prepare's workspace chown (R4-1/R4-2):
with it node-owned, a lifecycle script could plant a smudge filter and
have root's per-invocation reset execute it every round, or rewrite
HEAD so the restore installs a tree of its choosing. Non-fatal, since
the reset already runs git with GIT_* stripped, under a timeout,
against an OID pinned before any PR code ran.
- The scripts/tests arm gets the same substring-collision labelling the
generic arm already had, so a same-stem sibling collected by vitest's
positional filter is named in the attribution (R8-31/R13-28 lineage).
Suites: helper 112/112, twin 134/134 — both run as a NON-ROOT user,
which is what CI does; as root the gate pins its own PATH and the
harness stubs are bypassed by design, so a root run reports false
failures.
* fix(ci): clear the publisher's downloaded results before the download
R16-3: publish-verify runs on the persistent ECS pool and downloads the
artifact into a workspace-relative `verify-results`, which the runner
does not clean between jobs. The publisher treats the presence of
`verify-results/flake-gate.log` as proof that THIS run staged it, so a
previous run's log — possibly from another PR — could be embedded as
this run's evidence. The verify side already applies the same rm-first
rule to its own $RUNNER_TEMP tree; this brings the publisher in line.
Pinned by a structural test asserting the clear step exists and precedes
the download.
* fix(ci): drop the .git re-own — it breaks the build user's own reset
Reverting the .git hardening from 387a843434 after measuring it: the
per-invocation reset runs as `node` (root's git trips the
dubious-ownership guard), and a root-owned .git makes it fail at
`Unable to create '.git/index.lock'` — every round's reset would abort,
which the gate correctly reports as `error`, i.e. the gate would stop
working entirely. Probe: root:root + go-w on .git, reset as the build
user → 'Permission denied' on index.lock.
The R4-1/R4-2/R16-1 surface it aimed at (metadata-steered resets) keeps
its existing defences — OID pinned before sampling, GIT_* stripped from
the reset's environment, timeouts, and the per-round strip of .git
execution vectors — and the residual (that strip is a denylist, so
include/includeIf indirection can still reach it) is tracked as
follow-up rather than closed by a change that disables the gate.
* fix(ci): close flake-gate startup-channel shadowing; blank the upload loader env (#9130)
* fix(ci): close flake-gate startup-window races in the re-exec and verdict path (#9130)
R18-1: anchor the record/gate/staging re-exec snapshots to the inode
bash is executing (fd 255) and re-verify the path against it before the
exec — a swap that lands between bash's open of the runner-written step
script and the snapshot is filesystem state the kill cannot un-land.
R18-2: POSIXLY_CORRECT in all four gate-family step envs — bash then
refuses BASH_FUNC imports named after special builtins at startup, so a
poisoned `set` cannot run attacker code on the body's first command
(the in-script POSIX switch arrives one command late); a poisoned
startup fails the step red, and the abort is the refusal.
R18-3: every pre-re-exec refusal writes through /usr/bin/printf — echo
is a regular builtin, shadowable by a BASH_FUNC_echo%% import even in
POSIX mode (probe-verified).
R18-4: parent-side identity gates query the kernel via /usr/bin/id -u
instead of reading $EUID, which bash imports from the process
environment — one planted EUID line could skip the kill sweeps and the
poisoned-env refusals in every later step.
R16-4 (interim): the publisher's full-report branch maps a missing or
unrecognized FLAKE_VERDICT to a visible fixed-text error line instead
of silently dropping it — on that branch the gate ran and owes a
verdict, so absence means the uid-1000-writable $GITHUB_OUTPUT backing
channel corrupted it in transit. Fixed text only: the raw value is
attacker-influenced on this path and is never embedded.
The behavioral harness now applies the step env block (production
parity), and the two round-15 poison scenarios are re-pinned to the
stronger startup refusal.
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
f0dcdfc157