diff --git a/.github/scripts/ci-runner-routing.test.mjs b/.github/scripts/ci-runner-routing.test.mjs index d10a45ecc7f..04f030056a8 100644 --- a/.github/scripts/ci-runner-routing.test.mjs +++ b/.github/scripts/ci-runner-routing.test.mjs @@ -6,6 +6,13 @@ // they drift, classify and the Test job land on different pools. These tests // evaluate BOTH against the same event matrix — including the negative // associations that must stay hosted — and assert they agree. +// +// test_windows carries a deliberately different policy. A pull_request run +// executes the workflow YAML from the PR's own merge commit, so any trust +// clause a PR can read it can also rewrite. The matrix evaluates the real +// expression text and asserts the only enforceable shape: every pull request +// stays hosted, and only the merge queue, schedule and dispatch reach the +// persistent pool. import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; @@ -28,8 +35,11 @@ const serveAbDoc = parse( const TRUSTED = ['OWNER', 'MEMBER', 'COLLABORATOR']; const ECS = '["self-hosted", "linux", "x64", "ecs-qwen"]'; const HOSTED = '["ubuntu-latest"]'; +const WIN_ECS = ['self-hosted', 'Windows', 'X64', 'ecs-win']; +const WIN_HOSTED = ['windows-2022']; const classifyRunsOn = String(ciDoc.jobs.classify_pr['runs-on']); +const windowsRunsOn = String(ciDoc.jobs.test_windows['runs-on']); const pickRunner = ciDoc.jobs.classify_pr.steps.find( (s) => s.id === 'pick_runner', ); @@ -44,6 +54,48 @@ function simulateRunsOn({ ecsDisabled, sameRepo, assoc, mergeGroup }) { return ecs ? ECS : HOSTED; } +// Evaluates a real `runs-on` expression text with the routing inputs +// substituted, leaving only the &&/||/parenthesis skeleton — which matches +// GitHub's operator semantics closely enough for this fixed shape: both +// return the winning operand, and the winning operand is a fromJSON runner +// label, unwrapped here to the array it names. Any term the substitutions +// do not recognise fails loud, so an edited expression is re-read here +// instead of silently outgrowing the matrix. +function evalRunsOn(expression, { ecsDisabled, eventName, sameRepo, assoc }) { + const substitutions = [ + [/vars\.MAINTAINER_ECS_RUNNER_DISABLED != 'true'/, String(!ecsDisabled)], + [ + /github\.event_name == 'merge_group'/, + String(eventName === 'merge_group'), + ], + [ + /github\.event_name != 'pull_request'/, + String(eventName !== 'pull_request'), + ], + [ + /github\.event\.pull_request\.head\.repo\.full_name == github\.repository/, + String(sameRepo), + ], + [ + /contains\(fromJSON\('\["OWNER","MEMBER","COLLABORATOR"\]'\), github\.event\.pull_request\.author_association\)/, + String(TRUSTED.includes(assoc)), + ], + ]; + let expr = expression.replace(/^\$\{\{\s*/, '').replace(/\s*\}\}$/, ''); + for (const [term, value] of substitutions) { + expr = expr.replace(term, value); + } + expr = expr.replace(/fromJSON\('(\[[^\]]*\])'\)/g, '$1'); + assert.doesNotMatch( + expr, + /github\.|vars\.|contains\(|fromJSON\(/, + `routing expression carries a term the matrix does not model: ${expr}`, + ); + const selected = new Function(`return (${expr});`)(); + assert.ok(Array.isArray(selected), `no runner label selected: ${expr}`); + return selected; +} + // Executes the real pick_runner shell with the same inputs and returns the // selected runner exactly as CI would publish it. function runPickRunner({ ecsDisabled, sameRepo, assoc, eventName, dispatch }) { @@ -69,18 +121,19 @@ function runPickRunner({ ecsDisabled, sameRepo, assoc, eventName, dispatch }) { return line.slice('Selected Linux runner: '.length); } +const ASSOCIATIONS = [ + ...TRUSTED, + 'CONTRIBUTOR', + 'FIRST_TIME_CONTRIBUTOR', + 'FIRST_TIMER', + 'NONE', + '', +]; + describe('ci.yml classify_pr runner routing', () => { it('the expression and the shell step agree on every association', () => { - const associations = [ - ...TRUSTED, - 'CONTRIBUTOR', - 'FIRST_TIME_CONTRIBUTOR', - 'FIRST_TIMER', - 'NONE', - '', - ]; for (const sameRepo of [true, false]) { - for (const assoc of associations) { + for (const assoc of ASSOCIATIONS) { const expected = simulateRunsOn({ ecsDisabled: false, sameRepo, @@ -176,6 +229,68 @@ describe('ci.yml classify_pr runner routing', () => { }); }); +describe('ci.yml test_windows runner routing', () => { + it('keeps every pull request hosted, whoever opens it', () => { + // A pull_request run executes the workflow YAML from the PR's own merge + // commit: any PR this lane admits could rewrite `runs-on` in the same + // diff (editing this file is what classifies it platform-sensitive), so + // no trust clause evaluated on that event is enforceable. The enforceable + // shape is unconditional — pull requests never reach the persistent pool. + for (const sameRepo of [true, false]) { + for (const assoc of ASSOCIATIONS) { + assert.deepEqual( + evalRunsOn(windowsRunsOn, { + ecsDisabled: false, + eventName: 'pull_request', + sameRepo, + assoc, + }), + WIN_HOSTED, + `pull_request sameRepo=${sameRepo} assoc='${assoc}' must stay hosted`, + ); + } + } + }); + + it('keeps the pool for every non-pull-request trigger', () => { + // The denial form exists so the queue, the nightly and dispatch runs stay + // on the pool without a pull_request context to read; an && / || flip in + // the gate must not exile them to hosted runners. + for (const eventName of ['merge_group', 'schedule', 'workflow_dispatch']) { + assert.deepEqual( + evalRunsOn(windowsRunsOn, { + ecsDisabled: false, + eventName, + sameRepo: false, + assoc: '', + }), + WIN_ECS, + `${eventName} must keep the pool`, + ); + } + }); + + it('the kill-switch wins on every event', () => { + for (const eventName of [ + 'pull_request', + 'merge_group', + 'schedule', + 'workflow_dispatch', + ]) { + assert.deepEqual( + evalRunsOn(windowsRunsOn, { + ecsDisabled: true, + eventName, + sameRepo: true, + assoc: 'OWNER', + }), + WIN_HOSTED, + `kill-switch must win on ${eventName}`, + ); + } + }); +}); + describe('serve-ab.yml runner routing', () => { const runsOn = String(serveAbDoc.jobs.ab['runs-on']); diff --git a/.github/scripts/ci/classify-platform-sensitivity.mjs b/.github/scripts/ci/classify-platform-sensitivity.mjs new file mode 100644 index 00000000000..9da696c89de --- /dev/null +++ b/.github/scripts/ci/classify-platform-sensitivity.mjs @@ -0,0 +1,157 @@ +#!/usr/bin/env node +import { readFileSync } from 'node:fs'; + +// Does this change need the macOS and Windows lanes? +// +// Those lanes are the only signal this repository has about a host that is not +// Linux with a GNU userland, and they are expensive, so they run on the diffs +// whose behaviour the HOST decides rather than the code: shell scripts and the +// CI definitions that embed shell (a GNU-only flag, a BSD `sed`, a path +// separator), the script layer and its tests, the test-runner configuration +// that says which suites run where, and the handful of source subtrees named +// after a platform-coupled subsystem. +// +// This list is deliberately a NET, not a proof. It cannot see a platform +// assumption inside an ordinary source file — one that resolves a path, spawns +// a process, or compares two spellings of the same directory — and no +// path-based rule ever will. That gap is what the scheduled run on `main` +// exists for: this classifier buys an early signal on the diffs that carry the +// known failure class, and the nightly catches the rest a day later. Widening +// the list until it matches everything would just restore the cost the lanes +// were moved off pull requests to avoid. +// +// Fail-safe in every direction: an unreadable list, an unparsable entry, or no +// entries at all classifies as sensitive. A missed lane is a defect that ships; +// a needless lane is twenty minutes. + +export const PLATFORM_SENSITIVE = 'true'; +export const PLATFORM_INSENSITIVE = 'false'; + +// Anything a shell interprets. `.ps1`/`.bat`/`.cmd` are here for the same +// reason as `.sh`: they are the Windows lane's subject, not an exception to it. +const SHELL_SCRIPT = /\.(?:sh|bash|zsh|ps1|bat|cmd)$/i; + +// Workflow and composite-action YAML embeds shell in `run:` blocks, and the +// scripts those blocks call are part of the same program. +const CI_DEFINITION = /^\.github\/(?:workflows|actions|scripts)\//; + +// The script layer and its tests: this repository's build, release and CI +// helpers, the suites that drive them, and the fixtures those suites build out +// of real filesystem paths. +const SCRIPT_LAYER = /^scripts\//; + +// Which suites run on which lane is itself platform-deciding: an exclusion list +// keyed on `process.platform` is exactly how a suite ends up unrun on one host +// and red on another. +const RUNNER_CONFIG = /(?:^|\/)vitest(?:\.[^/]*)?\.config\.[cm]?[jt]s$/i; + +// The dependency and script manifests: a changed `test:ci`, a native module, or +// an optional per-platform dependency changes what each lane executes. +const MANIFEST = new Set(['package.json', 'package-lock.json']); + +// Source subtrees whose subject IS the host. +// +// A keyword counts when it NAMES the thing: a whole path segment +// (`src/sandbox/index.ts`, `src/platform/paths.ts`) or the head of a file's +// stem (`pty-host.ts`, `win32.ts`, `shell.ts`). It does not count inside a +// compound that names something else — `packages/web-shell/**` is a browser +// UI, not a shell, and matching it there summoned both expensive lanes on +// every change to one of this repository's largest packages. Nor inside a +// longer word: `Shellfish.tsx`, `plateauDetector.ts`, `cryptic.ts`. +const SUBSYSTEMS = + 'pty|tty|sandbox|seatbelt|shell|terminal|clipboard|platform|posix|darwin|macos|windows|win32|linux|keychain|codesign|installer|filesystem|audio'; +// A directory or file segment that IS the keyword (optionally with an +// extension): `sandbox/`, `shell.ts`, `win32.test.ts`. +const SUBSYSTEM_SEGMENT = new RegExp( + `(?:^|/)(?:${SUBSYSTEMS})(?:\\.[^/]*)?(?:/|$)`, + 'i', +); +// Or the keyword as the head of a hyphen/underscore-separated stem: +// `pty-host.ts`, `shell_exec.ts`. The head only — a trailing part belongs to +// whatever the leading word names. +const SUBSYSTEM_STEM_HEAD = new RegExp( + `(?:^|/)(?:${SUBSYSTEMS})[-_][^/]*(?:/|$)`, + 'i', +); + +function isSensitivePath(file) { + const p = String(file).replace(/\\/g, '/').replace(/^\.\//, ''); + if (!p) return true; + return ( + SHELL_SCRIPT.test(p) || + CI_DEFINITION.test(p) || + SCRIPT_LAYER.test(p) || + RUNNER_CONFIG.test(p) || + MANIFEST.has(p) || + SUBSYSTEM_SEGMENT.test(p) || + SUBSYSTEM_STEM_HEAD.test(p) + ); +} + +/** + * Every name an entry touches. A rename moves a file between two paths, and + * either side can be the sensitive one — a script moved out of `scripts/` is + * still a script change on the lane that ran it. + */ +function namesOf(entry) { + if (typeof entry === 'string') return [entry]; + if (!entry || typeof entry !== 'object') return []; + return [entry.filename, entry.previous_filename].filter( + (n) => typeof n === 'string' && n.length > 0, + ); +} + +export function classifyChangedFiles(entries) { + if (!Array.isArray(entries) || entries.length === 0) + return PLATFORM_SENSITIVE; + for (const entry of entries) { + const names = namesOf(entry); + // An entry that carries no usable name is an unknown change, and an + // unknown change is sensitive. + if (names.length === 0) return PLATFORM_SENSITIVE; + if (names.some(isSensitivePath)) return PLATFORM_SENSITIVE; + } + return PLATFORM_INSENSITIVE; +} + +/** The JSONL contract of classify-pr-profile.sh: one projected entry per line. */ +export function parseChangedFiles(text) { + return ( + String(text) + // `\r?\n`, matching the sibling classifier's reader: a CRLF listing would + // otherwise leave a trailing `\r` on every filename and defeat the + // end-anchored suffix rules above. + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean) + .map((line) => { + try { + return JSON.parse(line); + } catch { + // Not JSON: treat the raw line as a filename rather than dropping it. + return line; + } + }) + ); +} + +function main() { + const filePath = process.argv[2]; + if (!filePath) { + console.log(PLATFORM_SENSITIVE); + return; + } + try { + console.log( + classifyChangedFiles(parseChangedFiles(readFileSync(filePath, 'utf8'))), + ); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + console.error(`::warning::Failed to read changed files: ${message}`); + console.log(PLATFORM_SENSITIVE); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(); +} diff --git a/.github/scripts/ci/classify-platform-sensitivity.test.mjs b/.github/scripts/ci/classify-platform-sensitivity.test.mjs new file mode 100644 index 00000000000..9bd2f761368 --- /dev/null +++ b/.github/scripts/ci/classify-platform-sensitivity.test.mjs @@ -0,0 +1,235 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + PLATFORM_INSENSITIVE, + PLATFORM_SENSITIVE, + classifyChangedFiles, + parseChangedFiles, +} from './classify-platform-sensitivity.mjs'; + +test('an ordinary source change does not summon the expensive lanes', () => { + // The whole point of a gate: the common pull request pays nothing. If this + // ever flips, the lanes are back on every PR and the cost that moved them + // off is back with them. + assert.equal( + classifyChangedFiles([ + 'packages/cli/src/ui/components/Header.tsx', + 'packages/core/src/prompts/system.ts', + 'docs/users/configuration.md', + ]), + PLATFORM_INSENSITIVE, + ); +}); + +test('shell scripts pull in the lanes, on every dialect', () => { + for (const file of [ + 'scripts/build.sh', + 'tools/release.bash', + 'installer/setup.ps1', + 'ci/run.bat', + 'ci/run.cmd', + 'deep/nested/dir/helper.SH', + 'tools/setup.zsh', + ]) { + assert.equal( + classifyChangedFiles([`packages/core/src/x.ts`, file]), + PLATFORM_SENSITIVE, + file, + ); + } +}); + +test('CI definitions and the scripts they call are shell too', () => { + // The failure this gate exists for lived in a workflow's `run:` block and + // in the suite that drove it: `realpath -m` is GNU-only, so the guard it + // canonicalized with silently did nothing on macOS. + for (const file of [ + '.github/workflows/ci.yml', + '.github/actions/configure-windows-runner/action.yml', + '.github/scripts/ci/classify-profile.mjs', + 'scripts/tests/qwen-pr-review-workflow.test.js', + 'scripts/version.js', + ]) { + assert.equal(classifyChangedFiles([file]), PLATFORM_SENSITIVE, file); + } +}); + +test('the runner configuration decides which lane runs what', () => { + // An exclusion keyed on process.platform is how a suite ends up unrun on + // one host and red on another; a change to it must be seen by both lanes. + for (const file of [ + 'vitest.config.ts', + 'scripts/tests/vitest.config.ts', + 'vitest.terminal-bench.config.ts', + 'packages/cli/vitest.config.mts', + ]) { + assert.equal(classifyChangedFiles([file]), PLATFORM_SENSITIVE, file); + } + // Not every config is the runner's. + assert.equal( + classifyChangedFiles(['packages/cli/eslint.config.js']), + PLATFORM_INSENSITIVE, + ); +}); + +test('the manifests change what each lane executes', () => { + assert.equal(classifyChangedFiles(['package.json']), PLATFORM_SENSITIVE); + assert.equal(classifyChangedFiles(['package-lock.json']), PLATFORM_SENSITIVE); + // A workspace manifest is not the root one; it reaches the lanes through + // the subsystem rules or not at all. + assert.equal( + classifyChangedFiles(['packages/webui/package.json']), + PLATFORM_INSENSITIVE, + ); +}); + +test('platform-coupled subsystems match on segments, not substrings', () => { + for (const file of [ + 'packages/core/src/sandbox/index.ts', + 'packages/cli/src/ui/pty-host.ts', + 'packages/cli/src/utils/clipboard.ts', + 'packages/core/src/tools/shell.ts', + 'packages/cli/src/platform/paths.ts', + 'packages/cli/src/utils/win32.ts', + ]) { + assert.equal(classifyChangedFiles([file]), PLATFORM_SENSITIVE, file); + } + // A compound that names something else. `packages/web-shell/**` is one of + // this repository's largest packages and a browser UI, not a shell: the + // first spelling of this rule split on dashes anywhere in a segment and + // summoned both expensive lanes on every change to it. A directory that IS + // named for the subsystem still counts, wherever it sits. + for (const [file, expected] of [ + ['packages/web-shell/client/App.tsx', PLATFORM_INSENSITIVE], + ['packages/web-shell/client/index.html', PLATFORM_INSENSITIVE], + ['packages/web-shell/client/components/shell/Term.tsx', PLATFORM_SENSITIVE], + ['packages/cli/src/pty-host/index.ts', PLATFORM_SENSITIVE], + ]) { + assert.equal(classifyChangedFiles([file]), expected, file); + } + + // The substring trap: these contain "shell", "pty", "os" or "platform" + // inside a longer word and must NOT drag both lanes in. + for (const file of [ + 'packages/webui/src/components/Shellfish.tsx', + 'packages/core/src/utils/cryptic.ts', + 'packages/cli/src/ui/emptyState.ts', + 'packages/core/src/telemetry/uploader.ts', + 'packages/cli/src/services/plateauDetector.ts', + ]) { + assert.equal(classifyChangedFiles([file]), PLATFORM_INSENSITIVE, file); + } +}); + +test('native build workspaces are caught by the subsystem they name', () => { + // packages/audio-capture is a node-gyp module compiled per-host on exactly + // the two lanes this gate feeds: its native sources carry no shell + // extension, `mac_permission` names `mac` and not the keyword `macos`, and + // the workspace manifest is not the root one — so the subsystem rule is the + // only net under it. The keyword names the workspace directory, so every + // file below it counts. + for (const file of [ + 'packages/audio-capture/native/mac_permission.mm', + 'packages/audio-capture/native/audio_capture.cc', + 'packages/audio-capture/native/miniaudio.h', + 'packages/audio-capture/install.js', + 'packages/audio-capture/binding.gyp', + ]) { + assert.equal(classifyChangedFiles([file]), PLATFORM_SENSITIVE, file); + } + // The keyword names a subsystem, not a file extension: an ordinary `.cc` + // elsewhere is still ordinary source. + assert.equal( + classifyChangedFiles(['packages/core/src/utils/parser.cc']), + PLATFORM_INSENSITIVE, + ); +}); + +test('a rename is judged on both of its names', () => { + // A script moved out of the script layer is still a script change on the + // lane that used to run it — and one moved in is a new one to run. + assert.equal( + classifyChangedFiles([ + { + filename: 'tools/build.mjs', + status: 'renamed', + previous_filename: 'scripts/build.mjs', + }, + ]), + PLATFORM_SENSITIVE, + ); + assert.equal( + classifyChangedFiles([ + { + filename: 'scripts/build.mjs', + status: 'renamed', + previous_filename: 'tools/build.mjs', + }, + ]), + PLATFORM_SENSITIVE, + ); + assert.equal( + classifyChangedFiles([ + { + filename: 'src/b.ts', + status: 'renamed', + previous_filename: 'src/a.ts', + }, + ]), + PLATFORM_INSENSITIVE, + ); +}); + +test('every unknown answers sensitive, never insensitive', () => { + // A gate that fails open silently stops testing. Each of these is a way the + // input can arrive broken, and each one must still run the lanes. + assert.equal(classifyChangedFiles([]), PLATFORM_SENSITIVE); + assert.equal(classifyChangedFiles(null), PLATFORM_SENSITIVE); + assert.equal(classifyChangedFiles(undefined), PLATFORM_SENSITIVE); + assert.equal(classifyChangedFiles('scripts/x.sh'), PLATFORM_SENSITIVE); + assert.equal(classifyChangedFiles([{ status: 'added' }]), PLATFORM_SENSITIVE); + assert.equal(classifyChangedFiles([null]), PLATFORM_SENSITIVE); + assert.equal(classifyChangedFiles([{ filename: '' }]), PLATFORM_SENSITIVE); +}); + +test('parses the wrapper JSONL contract, and survives a non-JSON line', () => { + const parsed = parseChangedFiles( + [ + '{"filename":"src/a.ts","status":"modified","previous_filename":null}', + '', + 'scripts/raw-line.sh', + ].join('\n'), + ); + assert.equal(parsed.length, 2); + assert.equal(parsed[0].filename, 'src/a.ts'); + assert.equal(parsed[1], 'scripts/raw-line.sh'); + assert.equal(classifyChangedFiles(parsed), PLATFORM_SENSITIVE); +}); + +test('a CRLF listing does not smuggle a carriage return into a filename', () => { + // The suffix rules are end-anchored, so a trailing `\r` defeats every one + // of them and a script-layer change would classify as ordinary source. The + // sibling classifier splits on /\r?\n/ for the same reason. + const parsed = parseChangedFiles( + '{"filename":"scripts/build.sh","status":"modified"}\r\n{"filename":"src/a.ts","status":"modified"}\r\n', + ); + assert.equal(parsed.length, 2); + assert.equal(parsed[0].filename, 'scripts/build.sh'); + assert.equal(classifyChangedFiles(parsed), PLATFORM_SENSITIVE); + // And the raw-line path, where the `\r` would land on the name itself. + assert.equal( + classifyChangedFiles(parseChangedFiles('scripts/build.sh\r\nsrc/a.ts\r\n')), + PLATFORM_SENSITIVE, + ); +}); + +test('windows path separators classify the same as posix ones', () => { + // The listing is API-shaped and uses forward slashes, but a caller feeding + // this from a local `git diff` on Windows must not silently classify a + // script layer change as ordinary source. + assert.equal( + classifyChangedFiles(['scripts\\tests\\install-script.test.js']), + PLATFORM_SENSITIVE, + ); +}); diff --git a/.github/scripts/ci/classify-pr-profile.sh b/.github/scripts/ci/classify-pr-profile.sh index f9659915c5f..7119fe66d74 100755 --- a/.github/scripts/ci/classify-pr-profile.sh +++ b/.github/scripts/ci/classify-pr-profile.sh @@ -9,13 +9,25 @@ # meant the same PR could classify differently in each workflow, silently, # because both fall back to `full` on their own errors. # -# Usage: classify-pr-profile.sh -# Prints the profile (docs_only | github_ci_only | full) on stdout. +# Usage: classify-pr-profile.sh [mode] +# mode `profile` (default) prints docs_only | github_ci_only | full. +# mode `platform` prints true | false — whether the macOS and Windows lanes +# need to run for this PR. Both modes take the SAME listing through this +# script, for the reason above: a second copy of the listing contract is a +# second way for two call sites to see different files for the same PR. # Exit codes: 0 classified; 2 file listing failed; 3 classifier failed. set -euo pipefail -repo="${1:?usage: classify-pr-profile.sh }" -pr="${2:?usage: classify-pr-profile.sh }" +repo="${1:?usage: classify-pr-profile.sh [mode]}" +pr="${2:?usage: classify-pr-profile.sh [mode]}" +mode="${3:-profile}" +# Each mode also names what a TRUNCATED listing must fall back to — the +# conservative answer differs per mode (run everything vs. run the lanes). +case "$mode" in + profile) classifier='classify-profile.mjs'; truncated='full' ;; + platform) classifier='classify-platform-sensitivity.mjs'; truncated='true' ;; + *) echo "classify-pr-profile: unknown mode '${mode}'" >&2; exit 3 ;; +esac # mktemp + trap, not a fixed name: the self-hosted pool is persistent and # shared, so a predictable path is a leftover-file landmine, and ci.yml's @@ -33,13 +45,14 @@ fi # The list-files endpoint caps at 3,000 entries. A truncated listing can be # all docs while an omitted later entry is source, so any mismatch against -# the PR's own changed-file count conservatively classifies as `full`. +# the PR's own changed-file count conservatively classifies as the mode's +# run-everything answer. declared="$(gh api "repos/${repo}/pulls/${pr}" --jq '.changed_files')" || exit 2 retrieved="$(wc -l < "${files}")" if [ "${retrieved}" -ne "${declared}" ]; then - echo "classify-pr-profile: retrieved ${retrieved} file entries but PR declares ${declared}; classifying full." >&2 - echo "full" + echo "classify-pr-profile: retrieved ${retrieved} file entries but PR declares ${declared}; classifying ${truncated}." >&2 + echo "${truncated}" exit 0 fi -node "$(dirname "$0")/classify-profile.mjs" "${files}" || exit 3 +node "$(dirname "$0")/${classifier}" "${files}" || exit 3 diff --git a/.github/scripts/ci/classify-pr-profile.test.mjs b/.github/scripts/ci/classify-pr-profile.test.mjs index a154972bcff..e68c8effb1d 100644 --- a/.github/scripts/ci/classify-pr-profile.test.mjs +++ b/.github/scripts/ci/classify-pr-profile.test.mjs @@ -31,7 +31,7 @@ import { fileURLToPath } from 'node:url'; const here = dirname(fileURLToPath(import.meta.url)); const wrapper = join(here, 'classify-pr-profile.sh'); -function run(scenario, { stubNodeFailure = false } = {}) { +function run(scenario, { stubNodeFailure = false, mode } = {}) { const dir = mkdtempSync(join(tmpdir(), 'classify-pr-profile-')); const bin = join(dir, 'bin'); mkdirSync(bin); @@ -60,6 +60,7 @@ function run(scenario, { stubNodeFailure = false } = {}) { ' list-fail) exit 1 ;;', ' docs-only) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1},{"filename":"README.md","status":"modified","previous_filename":null,"sha":"y","additions":1}]\' ;;', ' renamed-source) FIXTURE=\'[{"filename":"docs/new.md","status":"renamed","previous_filename":"packages/core/src/runtime.ts","sha":"z","additions":0}]\' ;;', + ' script-change) FIXTURE=\'[{"filename":"scripts/tests/install-script.test.js","status":"modified","previous_filename":null,"sha":"s","additions":3}]\' ;;', ' truncated) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1}]\' ;;', ' declared-fails) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1}]\' ;;', ' *) exit 9 ;;', @@ -70,6 +71,7 @@ function run(scenario, { stubNodeFailure = false } = {}) { ' truncated) echo 5 ;;', ' declared-fails) exit 1 ;;', ' docs-only) echo 2 ;;', + ' script-change) echo 1 ;;', ' renamed-source) echo 1 ;;', ' *) exit 9 ;;', ' esac ;;', @@ -82,7 +84,9 @@ function run(scenario, { stubNodeFailure = false } = {}) { write('node', '#!/bin/bash\nexit 1\n'); } try { - const stdout = execFileSync('bash', [wrapper, 'o/r', '42'], { + const argv = [wrapper, 'o/r', '42']; + if (mode) argv.push(mode); + const stdout = execFileSync('bash', argv, { encoding: 'utf8', env: { ...process.env, @@ -126,3 +130,36 @@ test('exit 2 when the changed_files fetch fails after a successful listing', () const r = run('declared-fails'); assert.equal(r.code, 2); }); + +// The platform mode added for the macOS/Windows lane gate. It shares this +// wrapper's listing so both gates always judge the same files — the reason the +// listing lives here — but it answers a different question, and each mode has +// its own conservative answer when the listing cannot be trusted. +test('platform mode: an ordinary docs change does not need the expensive lanes', () => { + assert.deepEqual(run('docs-only', { mode: 'platform' }), { + code: 0, + stdout: 'false', + }); +}); + +test('platform mode: a script-layer change does', () => { + assert.deepEqual(run('script-change', { mode: 'platform' }), { + code: 0, + stdout: 'true', + }); +}); + +test('platform mode: a truncated listing runs the lanes, not skips them', () => { + // The mode-specific half of the 3,000-file cap guard: `full` is the + // conservative answer for the profile, `true` for the lanes. Hardcoding + // either one for both modes silently skips the lanes on a large PR. + assert.deepEqual(run('truncated', { mode: 'platform' }), { + code: 0, + stdout: 'true', + }); +}); + +test('an unknown mode fails loudly instead of falling back to a classifier', () => { + // A typo'd mode must not silently classify with the wrong question. + assert.equal(run('docs-only', { mode: 'platfrom' }).code, 3); +}); diff --git a/.github/workflows/.size-baseline b/.github/workflows/.size-baseline index cb712ab9b15..266ddbf8962 100644 --- a/.github/workflows/.size-baseline +++ b/.github/workflows/.size-baseline @@ -18,7 +18,7 @@ 4638 build-and-publish-image.yml 49610 cd-cua-driver.yml 2076 cd-mobile-mcp.yml -74315 ci.yml +81137 ci.yml 1482 codeql.yml 9389 comment-attachment-guard.yml 31677 desktop-release.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1d4bdee765..f2497c96994 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,6 +12,15 @@ on: - 'main' - 'release/**' merge_group: + # A daily heartbeat for the macOS and Windows lanes ONLY. Those two are + # gated on `merge_group`, and the merge queue is not enabled on this + # repository — no queue run since 2026-07-02 — so they had stopped running + # entirely: skipped on every pull request, and never reached afterwards. The + # pull-request gate below catches the diffs a path list can recognise; this + # catches everything it cannot, one day later, on `main`. Every other job + # here excludes `schedule` explicitly, so a nightly run is exactly two jobs. + schedule: + - cron: '17 19 * * *' workflow_dispatch: inputs: branch_ref: @@ -50,7 +59,7 @@ env: # BOTH the github_ci_only helper step and the full-profile Test step, so a # new helper test can't be added to one path and silently dropped from the # other. - HELPER_TESTS: '.github/scripts/pr-safety-precheck.test.mjs .github/scripts/cap-release-notes.test.mjs .github/scripts/ci/classify-profile.test.mjs .github/scripts/ci/classify-pr-profile.test.mjs .github/scripts/upsert-bot-comment.test.mjs .github/scripts/ci/main-failure-signature.test.mjs .github/scripts/classify-release-notes.test.mjs .github/scripts/dsw-swe-verified/make-manifest.test.mjs .github/scripts/dsw-swe-verified/make-terminal-bench-manifest.test.mjs .github/scripts/resolve-sandbox-image.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/serve-ab-diff.test.mjs .github/scripts/serve-ab-drive.test.mjs .github/scripts/qwen-triage-workflow.test.mjs .github/scripts/assign-issue-owner.test.mjs .github/scripts/auto-minimize-spam.test.mjs .github/scripts/ci-runner-routing.test.mjs' + HELPER_TESTS: '.github/scripts/pr-safety-precheck.test.mjs .github/scripts/cap-release-notes.test.mjs .github/scripts/ci/classify-profile.test.mjs .github/scripts/ci/classify-platform-sensitivity.test.mjs .github/scripts/ci/classify-pr-profile.test.mjs .github/scripts/upsert-bot-comment.test.mjs .github/scripts/ci/main-failure-signature.test.mjs .github/scripts/classify-release-notes.test.mjs .github/scripts/dsw-swe-verified/make-manifest.test.mjs .github/scripts/dsw-swe-verified/make-terminal-bench-manifest.test.mjs .github/scripts/resolve-sandbox-image.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/serve-ab-diff.test.mjs .github/scripts/serve-ab-drive.test.mjs .github/scripts/qwen-triage-workflow.test.mjs .github/scripts/assign-issue-owner.test.mjs .github/scripts/auto-minimize-spam.test.mjs .github/scripts/ci-runner-routing.test.mjs' # The growth ratchet and its vitest mirror compare each workflow against # the PR's base commit to tell "this PR grew the file" apart from "the # baseline went stale on main" (#9904). Wired once here so every lane @@ -68,7 +77,11 @@ jobs: # else a busy hosted pool delays it and blocks the ECS-bound jobs. The # kill-switch is read here, so flipping it reverts everything to hosted. # This runs-on and the pick_runner step below are the canonical home of - # the association routing; sdk-java.yml and serve-ab.yml mirror it. + # the association routing; sdk-java.yml and serve-ab.yml mirror it (the + # routing tests hold the mirrors to it). test_windows deliberately does + # not: a pull_request run executes the PR's own YAML, so no runs-on trust + # clause is enforceable there and its lane never admits pull requests to + # the pool. runs-on: '${{ (vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && (github.event.pull_request.head.repo.full_name == github.repository || contains(fromJSON(''["OWNER","MEMBER","COLLABORATOR"]''), github.event.pull_request.author_association) || github.event_name == ''merge_group'')) && fromJSON(''["self-hosted", "linux", "x64", "ecs-qwen"]'') || fromJSON(''["ubuntu-latest"]'') }}' continue-on-error: true outputs: @@ -139,6 +152,71 @@ jobs: echo "ubuntu_runner=${ubuntu_runner}" >> "${GITHUB_OUTPUT}" echo "Selected Linux runner: ${ubuntu_runner}" + # Does this pull request need the macOS and Windows lanes? They are the only + # signal this repository has about a host that is not Linux with a GNU + # userland, and they are expensive, so they run on the diffs whose behaviour + # the HOST decides — shell, CI definitions, the script layer, the runner + # config, the platform-coupled subtrees. The classifier is a net, not a + # proof; the scheduled run on `main` is what covers everything a path list + # cannot see. + # + # Its own job, not a step in classify_pr: that job's outputs pick the Linux + # runner for everything else, and a new failure mode there (this one needs a + # checkout, on a pool whose workspace other jobs can poison) would take the + # whole run's routing with it. Here a failure costs one skipped + # classification, which the gate reads as "run the lanes". + # + # Hosted, and the checkout is of the pull request's BASE commit: this runs + # before any review, so checking out the contributor's head would run their + # classifier with this job's token, and staying off the ECS pool keeps it + # away from the poisoned-workspace class entirely. + classify_platform: + name: 'Classify platform sensitivity' + if: "${{ github.event_name == 'pull_request' }}" + runs-on: 'ubuntu-latest' + timeout-minutes: 5 + continue-on-error: true + permissions: + contents: 'read' + pull-requests: 'read' + outputs: + platform_sensitive: '${{ steps.platform.outputs.platform_sensitive }}' + steps: + - name: 'Check out the classifier from the base branch' + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 + with: + ref: '${{ github.event.pull_request.base.sha }}' + persist-credentials: false + sparse-checkout: '.github/scripts/ci' + + - name: 'Classify platform sensitivity' + id: 'platform' + env: + GH_TOKEN: '${{ github.token }}' + PR_NUMBER: '${{ github.event.pull_request.number }}' + IS_SAME_REPO_PR: '${{ github.event.pull_request.head.repo.full_name == github.repository }}' + run: |- + set -uo pipefail + # Fail-safe in every direction: only a confident `false` from the + # classifier skips the lanes. A classifier error, an unexpected + # word, a fork PR, or this whole job failing all end as "run" — a + # gate that fails open stops testing without ever saying so. + sensitive=true + if [ "${IS_SAME_REPO_PR}" = 'true' ]; then + set +e + classified="$(.github/scripts/ci/classify-pr-profile.sh "${GITHUB_REPOSITORY}" "${PR_NUMBER}" platform)" + rc=$? + set -e + case "${rc}:${classified}" in + 0:true|0:false) sensitive="${classified}" ;; + *) echo "::warning::Platform-sensitivity classifier returned rc=${rc} '${classified}'; running the macOS and Windows lanes." ;; + esac + else + echo "Fork PR detected; running the macOS and Windows lanes." + fi + echo "platform_sensitive=${sensitive}" >> "${GITHUB_OUTPUT}" + echo "Platform-sensitive: ${sensitive}" + # # Test: Node # @@ -149,7 +227,7 @@ jobs: # report; the per-step skip_ci guards below make them no-op (pass) there. # Not on push: the merge queue already tested the merged tree, so a # post-merge re-run on `main` would be redundant. - if: "${{ !cancelled() && github.event_name != 'push' }}" + if: "${{ !cancelled() && github.event_name != 'push' && github.event_name != 'schedule' }}" runs-on: '${{ fromJSON(needs.classify_pr.outputs.ubuntu_runner || ''["ubuntu-latest"]'') }}' timeout-minutes: 60 outputs: @@ -827,19 +905,48 @@ jobs: packages/web-shell/client/e2e/playwright-report if-no-files-found: 'ignore' - # macOS/Windows: slowest/costliest runners, rare platform regressions — run - # only in the merge queue. Skipped on PR (ubuntu is the fast PR signal) and on - # push (the queue already tested the merged tree, so a post-merge re-run is - # redundant). Two named jobs, not a matrix: a skipped matrix job reports one - # collapsed check name, never the per-OS required contexts, so PRs would sit - # "Expected" forever and never enter the queue. A skipped named job reports - # under its exact name and satisfies the required check (same as the - # Integration Tests job). + # macOS/Windows: slowest/costliest runners, and the only signal this + # repository has about a host that is not Linux with a GNU userland. + # + # They used to run in the merge queue alone. That queue is not enabled here — + # no `merge_group` run since 2026-07-02, and merges land as squashes — so the + # gate meant they never ran at all: reported as "skipped" on every pull + # request, and never reached afterwards. A macOS-only failure could ship and + # sit in `main` indefinitely, which is what happened in #9220 (a GNU-only + # `realpath -m` in a workflow guard, with the suite that pinned it red on + # every Mac). + # + # Three triggers now, in cost order: a pull request whose diff the + # platform-sensitivity classifier recognises (shell, CI definitions, the + # script layer, the runner config, the platform-coupled subtrees), the merge + # queue if it is ever enabled again, and a nightly run on `main` for + # everything a path list cannot see. The pull-request gate is fail-safe — + # only a confident `false` skips, so a broken classifier costs runner minutes + # rather than coverage. + # + # Two named jobs, not a matrix: a skipped matrix job reports one collapsed + # check name, never the per-OS contexts, so a required-check configuration + # would sit "Expected" forever. A skipped named job reports under its exact + # name. (No status check is required on `main` today — the ruleset carries + # only deletion, non-fast-forward and pull_request rules — so this shape is + # currently insurance, not a live constraint.) test_macos: name: 'Test (macos-latest, Node 22.x)' - needs: 'classify_pr' - if: "${{ !cancelled() && github.event_name == 'merge_group' }}" + needs: + - 'classify_pr' + - 'classify_platform' + if: |- + ${{ + !cancelled() && ( + github.event_name == 'merge_group' || + github.event_name == 'schedule' || + github.event_name == 'workflow_dispatch' || + (github.event_name == 'pull_request' && + needs.classify_platform.outputs.platform_sensitive != 'false') + ) + }} runs-on: 'macos-latest' + timeout-minutes: 60 permissions: contents: 'read' steps: @@ -915,16 +1022,31 @@ jobs: # Windows counterpart of test_macos (see that job's note). ECS is the default # with a windows-2022 kill-switch fallback; the check name stays unchanged so - # it matches the required-status-check context. The job is merge_group-only, - # so code reaching it is post-approval; maintainers can still queue fork PRs. - # The runs-on expression therefore needs only the kill switch. ECS-only - # tuning is gated on runner.environment; the hosted fallback is the pre-ECS - # job plus the checkout guard and a job-level timeout-minutes. + # it matches the required-status-check context. The lane now runs on pull + # requests too, but every pull request runs on hosted windows-2022: a + # pull_request run executes the workflow YAML from the PR's own merge commit, + # so any PR this lane admits could rewrite `runs-on` in the same diff that + # reaches it. A gate the gated tree controls is no gate; the pool is reached + # only by triggers an unreviewed PR cannot open — the post-approval merge + # queue, schedule and dispatch. ECS-only tuning is gated on runner.environment; + # the hosted fallback is the pre-ECS job plus the checkout guard and a + # job-level timeout-minutes. test_windows: name: 'Test (windows-latest, Node 22.x)' - needs: 'classify_pr' - if: "${{ !cancelled() && github.event_name == 'merge_group' }}" - runs-on: '${{ vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && fromJSON(''["self-hosted", "Windows", "X64", "ecs-win"]'') || fromJSON(''["windows-2022"]'') }}' + needs: + - 'classify_pr' + - 'classify_platform' + if: |- + ${{ + !cancelled() && ( + github.event_name == 'merge_group' || + github.event_name == 'schedule' || + github.event_name == 'workflow_dispatch' || + (github.event_name == 'pull_request' && + needs.classify_platform.outputs.platform_sensitive != 'false') + ) + }} + runs-on: '${{ vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && github.event_name != ''pull_request'' && fromJSON(''["self-hosted", "Windows", "X64", "ecs-win"]'') || fromJSON(''["windows-2022"]'') }}' timeout-minutes: 60 permissions: contents: 'read' @@ -959,11 +1081,17 @@ jobs: # Same stale-checkout guard as the Ubuntu gate: this job now runs on ECS, # so fail loud if the checkout lacks the merge-queue head rather than # silently testing the wrong tree into a merge. + # Written when this lane ran in the merge queue alone, so its expected + # SHA named only the queue's event: on any other trigger the input is + # empty and the step fails the whole lane before a single test runs. + # That is what the revived triggers hit first. Same event-aware shape as + # the Ubuntu gate now, and skipped where there is no head to verify — + # the scheduled and dispatch runs check out a branch by name. - name: 'Verify checkout includes expected head commit' - if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" + if: "${{ needs.classify_pr.outputs.skip_ci != 'true' && (github.event_name == 'pull_request' || github.event_name == 'merge_group') }}" uses: './.github/actions/verify-checkout-head' with: - expected_sha: '${{ github.event.merge_group.head_sha }}' + expected_sha: "${{ github.event_name == 'merge_group' && github.event.merge_group.head_sha || github.event.pull_request.head.sha }}" # Avoid setup-node downloads on ECS, where nodejs.org may be unreachable # through the egress proxy; reuse the machine's Node instead. @@ -1277,7 +1405,7 @@ jobs: desktop_shell: name: 'Desktop Shell (${{ matrix.os }})' needs: 'classify_pr' - if: "${{ !cancelled() && github.event_name != 'push' && needs.classify_pr.outputs.skip_ci != 'true' }}" + if: "${{ !cancelled() && github.event_name != 'push' && github.event_name != 'schedule' && needs.classify_pr.outputs.skip_ci != 'true' }}" strategy: fail-fast: false matrix: diff --git a/.github/workflows/main-ci-failure-issue.yml b/.github/workflows/main-ci-failure-issue.yml index b6c8617b673..8615ce03e9b 100644 --- a/.github/workflows/main-ci-failure-issue.yml +++ b/.github/workflows/main-ci-failure-issue.yml @@ -4,8 +4,17 @@ name: 'Main CI Failure Issue' on: workflow_run: - workflows: ['E2E Tests', 'SDK Python'] + # 'Qwen Code CI' is watched for its SCHEDULED run only (see the event + # filter below): that nightly is the macOS and Windows lanes' only + # trigger outside a pull request, and a lane nobody is told about is a + # lane nobody looks at — which is how those two came to stop running. + workflows: ['E2E Tests', 'SDK Python', 'Qwen Code CI'] types: ['completed'] + # Trigger-level branch filter, not only the `if` below: 'Qwen Code CI' + # completes on every pull request, and without this each of those would + # raise a workflow_run event here just to skip. The job filter still + # enforces main — this only keeps the event log honest. + branches: ['main'] defaults: run: @@ -18,7 +27,7 @@ jobs: # and hands the finished title and body to the privileged job as outputs. analyze: name: 'Identify the failing tests' - if: "${{ github.repository == 'QwenLM/qwen-code' && github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.head_branch == 'main' && github.event.workflow_run.event == 'push' }}" + if: "${{ github.repository == 'QwenLM/qwen-code' && github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.head_branch == 'main' && (github.event.workflow_run.event == 'push' || (github.event.workflow_run.event == 'schedule' && github.event.workflow_run.name == 'Qwen Code CI')) }}" runs-on: 'ubuntu-latest' timeout-minutes: 10 permissions: @@ -33,7 +42,7 @@ jobs: body: '${{ steps.plan.outputs.body }}' steps: - name: 'Checkout' - uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: persist-credentials: false diff --git a/scripts/tests/ci-platform-lanes.test.js b/scripts/tests/ci-platform-lanes.test.js new file mode 100644 index 00000000000..056f43f09c8 --- /dev/null +++ b/scripts/tests/ci-platform-lanes.test.js @@ -0,0 +1,261 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +// The macOS and Windows lanes, and the gate that decides when they run. +// +// They were gated on `merge_group` alone while no merge queue was enabled, so +// they had not run since 2026-07-02: reported as "skipped" on every pull +// request — which reads as agreement — and never reached afterwards. The +// repository's only non-Linux, non-GNU signal was silently off, and a macOS +// failure shipped and sat in `main` (#9220). Nothing here can prove a lane +// ran; what these tests hold is the wiring that lets it: the triggers, the +// fail-safe direction of the gate, the nightly's blast radius, and the +// alerting that makes a nightly failure visible. + +import { readFileSync } from 'node:fs'; +import { describe, expect, it } from 'vitest'; +import { parse } from 'yaml'; + +const ci = parse(readFileSync('.github/workflows/ci.yml', 'utf8')); +const failureIssue = parse( + readFileSync('.github/workflows/main-ci-failure-issue.yml', 'utf8'), +); +// `on:` parses as the boolean true in YAML 1.1. +const triggers = ci[true] ?? ci['on']; +const LANES = ['test_macos', 'test_windows']; +const condOf = (job) => String(ci.jobs[job].if ?? ''); + +describe('platform lanes — triggers', () => { + it('gives the workflow a scheduled trigger', () => { + // Without it the lanes have no path to `main` at all: `ci.yml` has no + // push trigger by design, so a merge-queue-only gate on a repository with + // no merge queue is an off switch. + expect(triggers.schedule).toBeDefined(); + expect(Array.isArray(triggers.schedule)).toBe(true); + expect(triggers.schedule[0].cron).toMatch(/^\S+ \S+ \S+ \S+ \S+$/); + }); + + for (const lane of LANES) { + it(`${lane} runs on the schedule, the queue, a dispatch, and a sensitive PR`, () => { + const cond = condOf(lane); + // Presence AND the disjunction between clauses: an `&&` where a `||` + // belongs leaves the gate unsatisfiable for a trigger (event_name is + // single-valued) while a presence-only check stays green. + expect(cond).toMatch(/event_name == 'schedule'\s*\|\|/); + expect(cond).toMatch(/event_name == 'merge_group'\s*\|\|/); + expect(cond).toMatch(/event_name == 'workflow_dispatch'\s*\|\|/); + expect(cond).toContain("github.event_name == 'pull_request'"); + expect(cond).toContain( + 'needs.classify_platform.outputs.platform_sensitive', + ); + }); + + it(`${lane}'s triggers are alternatives, not requirements`, () => { + // The clause-presence assertions above survive a connective mutation: + // `||` → `&&` between two event clauses leaves every string in place + // and makes the gate unsatisfiable for every trigger, silently turning + // both lanes off again — the exact state this PR exists to end. Read + // the event group and require it to be a disjunction. + const cond = condOf(lane).replace(/\s+/g, ' '); + // From the first event clause to the close of the group — not from the + // first `(`, which belongs to `!cancelled()`. + const group = cond.slice( + cond.indexOf('github.event_name'), + cond.lastIndexOf(')'), + ); + expect(group).toContain("github.event_name == 'schedule'"); + expect(group.split('||').length).toBeGreaterThanOrEqual(4); + // The only `&&` allowed inside the group is the one binding the + // pull-request clause to its classifier output. + for (const clause of group.split('||')) { + if (clause.includes('platform_sensitive')) continue; + expect( + clause, + `event clause is conjoined: ${clause.trim()}`, + ).not.toContain('&&'); + } + }); + + it(`${lane} skips only on a confident 'false'`, () => { + // The fail-safe direction is the whole design: `== 'true'` would turn + // every classifier error, every skipped classify job and every empty + // output into a silently skipped lane. `!= 'false'` spends runner + // minutes instead of coverage. + const cond = condOf(lane); + expect(cond).toContain("platform_sensitive != 'false'"); + expect(cond).not.toContain("platform_sensitive == 'true'"); + // And the gate must survive a skipped or failed classifier job. + expect(cond).toContain('!cancelled()'); + expect(ci.jobs[lane].needs).toContain('classify_platform'); + }); + } + + for (const lane of LANES) { + it(`${lane} is bounded so a hang cannot burn the 360-minute default`, () => { + // The nightly's alert fires only when the run completes; a lane hung + // on a host-specific prompt otherwise sits out GitHub's default + // timeout before it fails and anyone is told. + expect(ci.jobs[lane]['timeout-minutes'], lane).toBe(60); + }); + } + + for (const lane of LANES) { + it(`${lane}'s steps are gated for every trigger it now has`, () => { + // The first thing the revived triggers hit was not a test failure but + // the lane's own plumbing: a `verify-checkout-head` step written when + // this lane ran in the merge queue alone, with `expected_sha` naming + // only `github.event.merge_group.head_sha`. On a pull request that + // input is empty and the step fails the lane before a single test + // runs. A step whose inputs name one event must be gated to that + // event — for every step in a job that now runs on four. + for (const step of ci.jobs[lane].steps ?? []) { + // Every place a step can read an event context, not just `with:` — + // an interpolation in `run:` or `env:` is the same defect wearing a + // different key. + const inputs = JSON.stringify({ + with: step.with ?? {}, + env: step.env ?? {}, + run: step.run ?? '', + }); + const gate = String(step.if ?? ''); + for (const [context, event] of [ + ['github.event.merge_group', "'merge_group'"], + ['github.event.pull_request', "'pull_request'"], + ]) { + if (!inputs.includes(context)) continue; + const guarded = + inputs.includes(`github.event_name == ${event}`) || + gate.includes(`github.event_name == ${event}`); + expect( + guarded, + `${lane} step "${step.name}" reads ${context} on every trigger`, + ).toBe(true); + } + } + }); + } + + it('keeps a nightly run to exactly the two lanes', () => { + // A `schedule:` trigger fires the whole workflow. Every other job must + // therefore either exclude `schedule` outright or gate on an event + // allowlist that cannot contain it — otherwise the nightly quietly + // becomes a full CI run every day. + for (const [name, job] of Object.entries(ci.jobs)) { + if (LANES.includes(name)) continue; + const cond = String(job.if ?? ''); + const excluded = + cond.includes("github.event_name != 'schedule'") || + /github\.event_name == '(pull_request|merge_group|workflow_dispatch)'/.test( + cond, + ); + expect(excluded, `${name} would also run on the nightly schedule`).toBe( + true, + ); + // Mentioning an allowlisted event is not the same as excluding this + // one: `event == 'pull_request' || event == 'schedule'` satisfies the + // check above while running nightly. Require the impossibility. + expect( + cond, + `${name} admits the schedule event explicitly`, + ).not.toContain("github.event_name == 'schedule'"); + expect(cond, `${name} has no event gate at all`).not.toBe(''); + } + }); +}); + +describe('platform lanes — the sensitivity classifier job', () => { + const job = ci.jobs.classify_platform; + + it('exists, is cheap, and cannot take the run down with it', () => { + expect(job).toBeDefined(); + expect(job['continue-on-error']).toBe(true); + expect(job['timeout-minutes']).toBeLessThanOrEqual(10); + // Hosted on purpose: it needs a checkout, and the persistent pool's + // workspace is exactly what other jobs have poisoned before. + expect(job['runs-on']).toBe('ubuntu-latest'); + expect(job.outputs.platform_sensitive).toContain( + 'steps.platform.outputs.platform_sensitive', + ); + }); + + it('checks out the base commit, never the pull request head', () => { + // This job runs before any review and executes a script from the tree it + // checks out. The contributor's head would be the contributor's + // classifier, running with this job's token. + const checkout = job.steps.find((s) => + String(s.uses ?? '').includes('actions/checkout'), + ); + expect(checkout).toBeDefined(); + expect(checkout.with.ref).toBe('${{ github.event.pull_request.base.sha }}'); + expect(checkout.with.ref).not.toContain('head'); + expect(checkout.with['persist-credentials']).toBe(false); + }); + + it('answers "run the lanes" for anything it is not sure about', () => { + const run = job.steps.find((s) => s.id === 'platform').run; + // A fork PR is not classified at all — the listing call is the same one + // the profile gate restricts to same-repo PRs. + expect(run).toContain('IS_SAME_REPO_PR'); + expect(run).toContain('sensitive=true'); + // Only the two words the classifier is allowed to say are accepted; a + // non-zero exit or anything else warns and runs the lanes. + expect(run).toContain('0:true|0:false'); + expect(run).toMatch(/::warning::.*running the macOS and Windows lanes/); + // The wrapper call is wrapped in `set +e`/`set -e`: the runner invokes + // `shell: bash` steps with `-e`, so without the guard a non-zero exit + // aborts the step at the assignment and the warn-and-run case above is + // dead code. Same shape as the sibling Classify CI profile step. + expect(run).toContain( + [ + ' set +e', + ' classified="$(.github/scripts/ci/classify-pr-profile.sh "${GITHUB_REPOSITORY}" "${PR_NUMBER}" platform)"', + ' rc=$?', + ' set -e', + ].join('\n'), + ); + }); + + it('drives the classifier through the shared listing wrapper', () => { + // Not a second listing: the wrapper's comment declares itself the single + // home of that contract, and two call sites listing separately is how the + // same PR ends up classified differently in two places. + const run = job.steps.find((s) => s.id === 'platform').run; + expect(run).toContain( + '.github/scripts/ci/classify-pr-profile.sh "${GITHUB_REPOSITORY}" "${PR_NUMBER}" platform', + ); + }); + + it('runs the classifier unit tests in CI', () => { + // The helper-test list is the single place both the github_ci_only step + // and the full Test step read; a classifier not named there is untested + // on every profile. + expect(ci.env.HELPER_TESTS).toContain( + '.github/scripts/ci/classify-platform-sensitivity.test.mjs', + ); + }); +}); + +describe('platform lanes — a failing nightly is visible', () => { + it('files an issue when the scheduled CI run fails on main', () => { + // A nightly nobody is told about is the same silence the merge-queue gate + // produced: the run goes red on a branch nobody watches and the lane is + // effectively off again. + const wr = (failureIssue[true] ?? failureIssue['on']).workflow_run; + expect(wr.workflows).toContain('Qwen Code CI'); + // Both sides of the binding: `workflow_run.workflows` matches the watched + // workflow's `name:`, so renaming ci.yml silently unhooks the watcher and + // the nightly goes back to failing where nobody is told. + expect(ci.name).toBe('Qwen Code CI'); + // `workflow_run.workflows` matches the watched workflow's `name:` key: + // pin the coupling itself, so renaming ci.yml's name fails here instead + // of silently stopping the nightly's workflow_run events. + expect(wr.workflows).toContain(ci.name); + const cond = String(failureIssue.jobs.analyze.if); + expect(cond).toContain("workflow_run.event == 'schedule'"); + expect(cond).toContain("workflow_run.head_branch == 'main'"); + expect(cond).toContain("workflow_run.conclusion == 'failure'"); + }); +}); diff --git a/scripts/tests/main-ci-failure-issue-workflow.test.js b/scripts/tests/main-ci-failure-issue-workflow.test.js index 99abbced7e7..d7a75f5c8ca 100644 --- a/scripts/tests/main-ci-failure-issue-workflow.test.js +++ b/scripts/tests/main-ci-failure-issue-workflow.test.js @@ -18,9 +18,17 @@ describe('main CI failure issue workflow', () => { it('opens an autofix-ready issue only for failed main CI runs', () => { expect(workflow).toContain('workflow_run:'); - expect(workflow).toContain("workflows: ['E2E Tests', 'SDK Python']"); - expect(workflow).not.toContain("'Qwen Code CI'"); + expect(workflow).toContain( + "workflows: ['E2E Tests', 'SDK Python', 'Qwen Code CI']", + ); expect(workflow).toContain("types: ['completed']"); + // 'Qwen Code CI' joined the list when the macOS and Windows lanes got a + // nightly run on main: that run is their only trigger outside a + // pull request, and a red lane nobody is told about is the same silence + // the merge-queue-only gate produced. It completes on every pull request + // too, so the branch filter keeps those events out entirely rather than + // raising one per run just to skip it. + expect(workflow).toContain("branches: ['main']"); expect(workflow).toContain("github.repository == 'QwenLM/qwen-code'"); expect(workflow).toContain( "github.event.workflow_run.conclusion == 'failure'", @@ -28,7 +36,20 @@ describe('main CI failure issue workflow', () => { expect(workflow).toContain( "github.event.workflow_run.head_branch == 'main'", ); - expect(workflow).toContain("github.event.workflow_run.event == 'push'"); + // Push covers the other two watched workflows; schedule is scoped to + // 'Qwen Code CI' — that nightly is the platform lanes' only trigger + // outside a pull request, and the other watched workflows' own + // nightlies must not dispatch the autofix agent through this watcher. + // A pull-request run of any of them must never open an issue — that is + // contributor-triggered, and the branch filter plus this clause are + // what keep it out. Pin the whole event clause so a connective or + // scope mutation fails here. + expect(workflow).toContain( + "(github.event.workflow_run.event == 'push' || (github.event.workflow_run.event == 'schedule' && github.event.workflow_run.name == 'Qwen Code CI'))", + ); + expect(workflow).not.toContain( + "github.event.workflow_run.event == 'pull_request'", + ); }); it('creates an issue that the existing autofix worker can pick up', () => { diff --git a/scripts/tests/no-ak-integration-ci.test.js b/scripts/tests/no-ak-integration-ci.test.js index b226f7637de..50d260a8f62 100644 --- a/scripts/tests/no-ak-integration-ci.test.js +++ b/scripts/tests/no-ak-integration-ci.test.js @@ -317,7 +317,7 @@ describe('no-AK integration CI wiring', () => { "expected_sha: '${{ github.event.pull_request.head.sha }}'", ); expect(guardCalls.test_windows).toContain( - "expected_sha: '${{ github.event.merge_group.head_sha }}'", + 'expected_sha: "${{ github.event_name == \'merge_group\' && github.event.merge_group.head_sha || github.event.pull_request.head.sha }}"', ); expect(guardCalls.integration_cli).toContain( "expected_sha: '${{ github.event.merge_group.head_sha }}'", @@ -334,7 +334,7 @@ describe('no-AK integration CI wiring', () => { 'if: "${{ github.event_name == \'pull_request\' }}"', ); expect(guardCalls.test_windows).toContain( - 'if: "${{ needs.classify_pr.outputs.skip_ci != \'true\' }}"', + "if: \"${{ needs.classify_pr.outputs.skip_ci != 'true' && (github.event_name == 'pull_request' || github.event_name == 'merge_group') }}\"", ); expect(guardCalls.integration_cli).not.toContain('if:'); }); @@ -346,23 +346,27 @@ describe('no-AK integration CI wiring', () => { ); const windowsJob = getWorkflowJob(workflow, 'test_windows'); - // The runs-on expression is the Windows gate's escape hatch. Pin the - // whole line so a variable typo, a quoting regression in the nested - // ''true'' escapes, or an && / || regrouping fails here instead of - // surfacing only when the switch is flipped. + // The runs-on expression is the Windows gate's escape hatch and its fork + // trust policy: pull requests never reach the pool, because a + // pull_request run executes the PR's own YAML and could rewrite runs-on + // itself. Pin the whole line so a variable typo, a quoting regression in + // the nested ''true'' escapes, or an && / || regrouping fails here + // instead of surfacing only when the switch is flipped — or when a fork + // PR finds the persistent pool. const windowsRunsOn = windowsJob .split('\n') .find((line) => line.startsWith(' runs-on:')); expect(windowsRunsOn).toBe( - ` runs-on: '\${{ vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && fromJSON(''["self-hosted", "Windows", "X64", "ecs-win"]'') || fromJSON(''["windows-2022"]'') }}'`, + ` runs-on: '\${{ vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && github.event_name != ''pull_request'' && fromJSON(''["self-hosted", "Windows", "X64", "ecs-win"]'') || fromJSON(''["windows-2022"]'') }}'`, ); expect(windowsJob.split('\n')).toContain(' timeout-minutes: 60'); - // The guard must stay wired to the merge-queue head for this job. + // The guard must stay wired to the expected head for this job: the + // event-aware shape, since the revived triggers have no merge-queue head. const guard = getWorkflowStep(windowsJob, GUARD_STEP); expect(guard).toContain("uses: './.github/actions/verify-checkout-head'"); expect(guard).toContain( - "expected_sha: '${{ github.event.merge_group.head_sha }}'", + 'expected_sha: "${{ github.event_name == \'merge_group\' && github.event.merge_group.head_sha || github.event.pull_request.head.sha }}"', ); // The self-hosted-only tuning comes from the composite action shared with diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 324eaa4a04c..4edbd3fb952 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -17,6 +17,7 @@ import { } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { getWorkflowJob } from './workflow-helpers.js'; @@ -210,6 +211,18 @@ const nodeSetupSteps = workflow.match(/- name: 'Set up Node.js'[\s\S]*?(?=\n[ ]{6}- name: ')/g) ?? []; +// The gate script's bite check reads NUL-separated diff lists through +// `mapfile -d ''`, a bash >= 4.4 builtin: on a host whose `bash` is older +// (macOS ships 3.2) every spawn of the real script or the extracted block +// dies with `mapfile: command not found` before the semantics under test +// can run. The gate only executes on Linux runners, so the defect cannot +// exist in production; probe the host, not the platform — a Mac with a +// newer bash fronting PATH keeps the coverage (same shape as +// hasGnuRealpath in qwen-pr-review-workflow.test.js). +const hasBashMapfile = + spawnSync('bash', ['-c', 'mapfile -d "" -t x <<< y'], { stdio: 'ignore' }) + .status === 0; + // GitHub Actions expressions return operand VALUES from &&/||, not // booleans: && yields the first falsy operand (else the last operand), || // the first truthy (else the last), '' is falsy, and && binds tighter @@ -14300,418 +14313,425 @@ exit 1 expect(forgedErr.out).not.toContain('::error::forged'); }); - it('bite check: rejects a round whose changed tests pass on the pre-round tree', () => { - // Ends at the FINAL assert: the conflict push-boundary refusal sits - // between it and the verified_head write and is not bite machinery. - const block = reviewVerificationRunner.match( - /(# Bite check:[\s\S]*?)\nassert_verification_tree\n/, - )?.[1]; - expect(block).toBeTruthy(); - const run = ( - build, - { runnerExit, runnerScript, resolverLines, workdir, prelude = '' }, - ) => { - const { dir, git } = validityFixture(build); - const tools = mkdtempSync(join(tmpdir(), 'autofix-validity-tools-')); - // Stub owning-package resolver and bite runner — the block treats the - // runner as an opaque command. A fixed exit code drives the semantic - // cases; a runnerScript drives the tree-state-proving cases. - writeFileSync( - join(tools, 'resolve-owning-packages.sh'), - `cat > /dev/null\nprintf '%s\\n' ${resolverLines.map((l) => `'${l}'`).join(' ')}\n`, - ); - writeFileSync( - join(tools, 'bite-runner'), - runnerScript ?? - `#!/usr/bin/env bash\necho "bite-runner: $*" >&2\nexit ${runnerExit}\n`, - ); - chmodSync(join(tools, 'bite-runner'), 0o755); - // WORKDIR fixtures: resolved-comments.txt + rc.json/rv.json make the - // round a DEFECT-CLAIM round (it resolves a Critical / CR finding). - for (const [name, content] of Object.entries(workdir ?? {})) { - writeFileSync(join(tools, name), content); - } - const res = spawnSync( - 'bash', - [ - '-c', - [ - 'set -eo pipefail', - 'cd "$1"', - 'BRANCH=feat', - 'WORKDIR="$2"', - 'RUNNER_TEMP="$2"', - 'GATE_LOG="$2/gate.log"', - ': > "$GATE_LOG"', - 'ROUND_RANGE="origin/feat...feat"', - freightHelper(), - prelude, - 'BITE_RUNNER="$2/bite-runner"', - 'reject_fix() { echo "REJECT:${1}"; exit 1; }', - block, - 'echo SURVIVED', - ].join('\n'), + it.skipIf(!hasBashMapfile)( + 'bite check: rejects a round whose changed tests pass on the pre-round tree', + () => { + // Ends at the FINAL assert: the conflict push-boundary refusal sits + // between it and the verified_head write and is not bite machinery. + const block = reviewVerificationRunner.match( + /(# Bite check:[\s\S]*?)\nassert_verification_tree\n/, + )?.[1]; + expect(block).toBeTruthy(); + const run = ( + build, + { runnerExit, runnerScript, resolverLines, workdir, prelude = '' }, + ) => { + const { dir, git } = validityFixture(build); + const tools = mkdtempSync(join(tmpdir(), 'autofix-validity-tools-')); + // Stub owning-package resolver and bite runner — the block treats the + // runner as an opaque command. A fixed exit code drives the semantic + // cases; a runnerScript drives the tree-state-proving cases. + writeFileSync( + join(tools, 'resolve-owning-packages.sh'), + `cat > /dev/null\nprintf '%s\\n' ${resolverLines.map((l) => `'${l}'`).join(' ')}\n`, + ); + writeFileSync( + join(tools, 'bite-runner'), + runnerScript ?? + `#!/usr/bin/env bash\necho "bite-runner: $*" >&2\nexit ${runnerExit}\n`, + ); + chmodSync(join(tools, 'bite-runner'), 0o755); + // WORKDIR fixtures: resolved-comments.txt + rc.json/rv.json make the + // round a DEFECT-CLAIM round (it resolves a Critical / CR finding). + for (const [name, content] of Object.entries(workdir ?? {})) { + writeFileSync(join(tools, name), content); + } + const res = spawnSync( 'bash', - dir, - tools, - ], - { encoding: 'utf8', env: isolatedGitEnv }, - ); - expect(res.error).toBeUndefined(); - const status = git('status', '--porcelain'); - const head = git('rev-parse', '--abbrev-ref', 'HEAD').trim(); - const advisoryPath = join(tools, 'gate-advisories.md'); - const advisory = existsSync(advisoryPath) - ? readFileSync(advisoryPath, 'utf8') - : ''; - const rejectionPath = join(tools, 'gate-rejection.md'); - const rejection = existsSync(rejectionPath) - ? readFileSync(rejectionPath, 'utf8') - : ''; - rmSync(dir, { recursive: true, force: true }); - rmSync(tools, { recursive: true, force: true }); - return { - out: `${res.stdout}\n${res.stderr}\n[spawn status=${res.status}]`, - status, - head, - advisory, - rejection, - }; - }; - const srcAndTest = { - base: ({ write }) => { - // The bite runner guard reads the workspace's test script and the - // self-import guard reads its name (absent from the test files). - write( - 'packages/cli/package.json', - '{"name":"@fixture/cli","scripts":{"test":"vitest run"}}\n', + [ + '-c', + [ + 'set -eo pipefail', + 'cd "$1"', + 'BRANCH=feat', + 'WORKDIR="$2"', + 'RUNNER_TEMP="$2"', + 'GATE_LOG="$2/gate.log"', + ': > "$GATE_LOG"', + 'ROUND_RANGE="origin/feat...feat"', + freightHelper(), + prelude, + 'BITE_RUNNER="$2/bite-runner"', + 'reject_fix() { echo "REJECT:${1}"; exit 1; }', + block, + 'echo SURVIVED', + ].join('\n'), + 'bash', + dir, + tools, + ], + { encoding: 'utf8', env: isolatedGitEnv }, ); - write('packages/cli/src/a.ts', 'a\n'); - write('packages/cli/src/a.test.ts', 't\n'); - }, - pr: ({ write }) => write('packages/cli/src/a.ts', 'b\n'), - round: ({ write }) => { - write('packages/cli/src/a.ts', 'c\n'); - write('packages/cli/src/a.test.ts', 't2\n'); - }, - }; - // Artifacts that mark the round as resolving a Critical finding. - const criticalClaim = { - // rc: prefix + CRLF, exactly as SKILL tells the agent to write the - // handle and as the other consumers tolerate. - 'resolved-comments.txt': 'rc:101\r\n', - 'rc.json': JSON.stringify([ - { id: 101, body: '**[Critical]** stale owner routes writes' }, - ]), - 'rv.json': JSON.stringify([]), - }; - // Defect-claim round + all changed tests green on the pre-round tree => - // the claimed defect does not reproduce => non-retryable rejection. - const rejected = run(srcAndTest, { - runnerExit: 0, - resolverLines: ['packages/cli'], - workdir: criticalClaim, - }); - expect(rejected.out).toContain('REJECT:bite check'); - // The SAME all-green result WITHOUT a defect claim (a refactor pinning - // existing behavior, an optional cleanup) is an advisory, not a - // rejection — and the tree still comes back clean on the branch. - const advisory = run(srcAndTest, { - runnerExit: 0, - resolverLines: ['packages/cli'], - }); - expect(advisory.out).toContain('SURVIVED'); - expect(advisory.out).not.toContain('REJECT:'); - expect(advisory.advisory).toContain('pass on the pre-round tree'); - expect(advisory.status).toBe(''); - expect(advisory.head).toBe('feat'); - // Enforcement needs a RESOLVED Critical: resolving only a Suggestion, - // or merely having an unresolved Critical present in rc.json, stays - // advisory-grade. - for (const workdir of [ - { - 'resolved-comments.txt': '303\n', - 'rc.json': JSON.stringify([ - { id: 303, body: '**[Suggestion]** rename this helper' }, - { id: 101, body: '**[Critical]** stale owner routes writes' }, - ]), - 'rv.json': JSON.stringify([]), - }, - { - 'resolved-comments.txt': '999\n', + expect(res.error).toBeUndefined(); + const status = git('status', '--porcelain'); + const head = git('rev-parse', '--abbrev-ref', 'HEAD').trim(); + const advisoryPath = join(tools, 'gate-advisories.md'); + const advisory = existsSync(advisoryPath) + ? readFileSync(advisoryPath, 'utf8') + : ''; + const rejectionPath = join(tools, 'gate-rejection.md'); + const rejection = existsSync(rejectionPath) + ? readFileSync(rejectionPath, 'utf8') + : ''; + rmSync(dir, { recursive: true, force: true }); + rmSync(tools, { recursive: true, force: true }); + return { + out: `${res.stdout}\n${res.stderr}\n[spawn status=${res.status}]`, + status, + head, + advisory, + rejection, + }; + }; + const srcAndTest = { + base: ({ write }) => { + // The bite runner guard reads the workspace's test script and the + // self-import guard reads its name (absent from the test files). + write( + 'packages/cli/package.json', + '{"name":"@fixture/cli","scripts":{"test":"vitest run"}}\n', + ); + write('packages/cli/src/a.ts', 'a\n'); + write('packages/cli/src/a.test.ts', 't\n'); + }, + pr: ({ write }) => write('packages/cli/src/a.ts', 'b\n'), + round: ({ write }) => { + write('packages/cli/src/a.ts', 'c\n'); + write('packages/cli/src/a.test.ts', 't2\n'); + }, + }; + // Artifacts that mark the round as resolving a Critical finding. + const criticalClaim = { + // rc: prefix + CRLF, exactly as SKILL tells the agent to write the + // handle and as the other consumers tolerate. + 'resolved-comments.txt': 'rc:101\r\n', 'rc.json': JSON.stringify([ { id: 101, body: '**[Critical]** stale owner routes writes' }, ]), 'rv.json': JSON.stringify([]), - }, - ]) { - const soft = run(srcAndTest, { + }; + // Defect-claim round + all changed tests green on the pre-round tree => + // the claimed defect does not reproduce => non-retryable rejection. + const rejected = run(srcAndTest, { runnerExit: 0, resolverLines: ['packages/cli'], - workdir, + workdir: criticalClaim, }); - expect(soft.out).toContain('SURVIVED'); - expect(soft.out).not.toContain('REJECT:'); - expect(soft.advisory).toContain('pass on the pre-round tree'); - } - // A reply resolved inside a Critical-rooted thread is a defect claim, - // matching how the feedback renderers classify replies. - expect( - run(srcAndTest, { + expect(rejected.out).toContain('REJECT:bite check'); + // The SAME all-green result WITHOUT a defect claim (a refactor pinning + // existing behavior, an optional cleanup) is an advisory, not a + // rejection — and the tree still comes back clean on the branch. + const advisory = run(srcAndTest, { runnerExit: 0, resolverLines: ['packages/cli'], - workdir: { - 'resolved-comments.txt': '404\n', + }); + expect(advisory.out).toContain('SURVIVED'); + expect(advisory.out).not.toContain('REJECT:'); + expect(advisory.advisory).toContain('pass on the pre-round tree'); + expect(advisory.status).toBe(''); + expect(advisory.head).toBe('feat'); + // Enforcement needs a RESOLVED Critical: resolving only a Suggestion, + // or merely having an unresolved Critical present in rc.json, stays + // advisory-grade. + for (const workdir of [ + { + 'resolved-comments.txt': '303\n', 'rc.json': JSON.stringify([ + { id: 303, body: '**[Suggestion]** rename this helper' }, { id: 101, body: '**[Critical]** stale owner routes writes' }, - { id: 404, body: 'fixed here', in_reply_to_id: 101 }, ]), 'rv.json': JSON.stringify([]), }, - }).out, - ).toContain('REJECT:bite check'); - // A Critical anchored ON a test file is a test-side claim: all-green is - // its expected shape, so it demotes to the advisory arm... - const testSide = run(srcAndTest, { - runnerExit: 0, - resolverLines: ['packages/cli'], - workdir: { - 'resolved-comments.txt': 'rc:101\n', - 'rc.json': JSON.stringify([ - { - id: 101, - path: 'packages/cli/src/a.test.ts', - body: '**[Critical]** this test asserts the wrong behavior', + { + 'resolved-comments.txt': '999\n', + 'rc.json': JSON.stringify([ + { id: 101, body: '**[Critical]** stale owner routes writes' }, + ]), + 'rv.json': JSON.stringify([]), + }, + ]) { + const soft = run(srcAndTest, { + runnerExit: 0, + resolverLines: ['packages/cli'], + workdir, + }); + expect(soft.out).toContain('SURVIVED'); + expect(soft.out).not.toContain('REJECT:'); + expect(soft.advisory).toContain('pass on the pre-round tree'); + } + // A reply resolved inside a Critical-rooted thread is a defect claim, + // matching how the feedback renderers classify replies. + expect( + run(srcAndTest, { + runnerExit: 0, + resolverLines: ['packages/cli'], + workdir: { + 'resolved-comments.txt': '404\n', + 'rc.json': JSON.stringify([ + { id: 101, body: '**[Critical]** stale owner routes writes' }, + { id: 404, body: 'fixed here', in_reply_to_id: 101 }, + ]), + 'rv.json': JSON.stringify([]), }, - ]), - 'rv.json': JSON.stringify([]), - }, - }); - expect(testSide.out).toContain('SURVIVED'); - expect(testSide.out).not.toContain('REJECT:'); - expect(testSide.advisory).toContain('test-side defect claim'); - // ...but only RESOLVED CRITICAL threads vote: a source-file Suggestion - // resolved alongside must not break the demotion. - expect( - run(srcAndTest, { + }).out, + ).toContain('REJECT:bite check'); + // A Critical anchored ON a test file is a test-side claim: all-green is + // its expected shape, so it demotes to the advisory arm... + const testSide = run(srcAndTest, { runnerExit: 0, resolverLines: ['packages/cli'], workdir: { - 'resolved-comments.txt': '101\n303\n', + 'resolved-comments.txt': 'rc:101\n', 'rc.json': JSON.stringify([ { id: 101, path: 'packages/cli/src/a.test.ts', body: '**[Critical]** this test asserts the wrong behavior', }, - { - id: 303, - path: 'packages/cli/src/a.ts', - body: '**[Suggestion]** rename this helper', - }, ]), 'rv.json': JSON.stringify([]), }, - }).out, - ).not.toContain('REJECT:'); - // A Critical anchored on SOURCE keeps full enforcement even when a - // test-side Critical is resolved in the same round. - expect( - run(srcAndTest, { + }); + expect(testSide.out).toContain('SURVIVED'); + expect(testSide.out).not.toContain('REJECT:'); + expect(testSide.advisory).toContain('test-side defect claim'); + // ...but only RESOLVED CRITICAL threads vote: a source-file Suggestion + // resolved alongside must not break the demotion. + expect( + run(srcAndTest, { + runnerExit: 0, + resolverLines: ['packages/cli'], + workdir: { + 'resolved-comments.txt': '101\n303\n', + 'rc.json': JSON.stringify([ + { + id: 101, + path: 'packages/cli/src/a.test.ts', + body: '**[Critical]** this test asserts the wrong behavior', + }, + { + id: 303, + path: 'packages/cli/src/a.ts', + body: '**[Suggestion]** rename this helper', + }, + ]), + 'rv.json': JSON.stringify([]), + }, + }).out, + ).not.toContain('REJECT:'); + // A Critical anchored on SOURCE keeps full enforcement even when a + // test-side Critical is resolved in the same round. + expect( + run(srcAndTest, { + runnerExit: 0, + resolverLines: ['packages/cli'], + workdir: { + 'resolved-comments.txt': '101\n102\n', + 'rc.json': JSON.stringify([ + { + id: 101, + path: 'packages/cli/src/a.test.ts', + body: '**[Critical]** this test asserts the wrong behavior', + }, + { + id: 102, + path: 'packages/cli/src/a.ts', + body: '**[Critical]** stale owner routes writes', + }, + ]), + 'rv.json': JSON.stringify([]), + }, + }).out, + ).toContain('REJECT:bite check'); + // TESTSIDE demotion honors the review-STATE arm too: a CR-attached + // comment on a test path demotes like a body-tagged Critical does. + const crTestSide = run(srcAndTest, { runnerExit: 0, resolverLines: ['packages/cli'], workdir: { - 'resolved-comments.txt': '101\n102\n', + 'resolved-comments.txt': '505\n', 'rc.json': JSON.stringify([ { - id: 101, + id: 505, path: 'packages/cli/src/a.test.ts', - body: '**[Critical]** this test asserts the wrong behavior', - }, - { - id: 102, - path: 'packages/cli/src/a.ts', - body: '**[Critical]** stale owner routes writes', + body: 'this test asserts the wrong behavior', + pull_request_review_id: 9, }, ]), - 'rv.json': JSON.stringify([]), + 'rv.json': JSON.stringify([{ id: 9, state: 'CHANGES_REQUESTED' }]), }, - }).out, - ).toContain('REJECT:bite check'); - // TESTSIDE demotion honors the review-STATE arm too: a CR-attached - // comment on a test path demotes like a body-tagged Critical does. - const crTestSide = run(srcAndTest, { - runnerExit: 0, - resolverLines: ['packages/cli'], - workdir: { - 'resolved-comments.txt': '505\n', - 'rc.json': JSON.stringify([ - { - id: 505, - path: 'packages/cli/src/a.test.ts', - body: 'this test asserts the wrong behavior', - pull_request_review_id: 9, + }); + expect(crTestSide.out).not.toContain('REJECT:'); + expect(crTestSide.advisory).toContain('test-side defect claim'); + // Resolving a comment attached to a CHANGES_REQUESTED review enforces + // the same way a Critical tag does. + expect( + run(srcAndTest, { + runnerExit: 0, + resolverLines: ['packages/cli'], + workdir: { + 'resolved-comments.txt': '202\n', + 'rc.json': JSON.stringify([ + { + id: 202, + body: 'null branch crashes', + pull_request_review_id: 9, + }, + ]), + 'rv.json': JSON.stringify([{ id: 9, state: 'CHANGES_REQUESTED' }]), }, - ]), - 'rv.json': JSON.stringify([{ id: 9, state: 'CHANGES_REQUESTED' }]), - }, - }); - expect(crTestSide.out).not.toContain('REJECT:'); - expect(crTestSide.advisory).toContain('test-side defect claim'); - // Resolving a comment attached to a CHANGES_REQUESTED review enforces - // the same way a Critical tag does. - expect( - run(srcAndTest, { - runnerExit: 0, + }).out, + ).toContain('REJECT:bite check'); + // Any failure on the pre-round tree = the tests bite => round proceeds, + // and the verification tree is restored to the branch, clean. + const bit = run(srcAndTest, { + runnerExit: 1, resolverLines: ['packages/cli'], - workdir: { - 'resolved-comments.txt': '202\n', - 'rc.json': JSON.stringify([ - { id: 202, body: 'null branch crashes', pull_request_review_id: 9 }, - ]), - 'rv.json': JSON.stringify([{ id: 9, state: 'CHANGES_REQUESTED' }]), + workdir: criticalClaim, + }); + expect(bit.out).toContain('bite confirmed'); + expect(bit.out).toContain('SURVIVED'); + expect(bit.status).toBe(''); + expect(bit.head).toBe('feat'); + // Tree-state proof: the runner inspects the ACTUAL checkout instead of + // returning a fixed code. It fails (bites) only when it sees PRE-ROUND + // source ('b') alongside the ROUND's test ('t2') — passing proves the + // detach reverted the source AND the overlay delivered the round's test. + const treeProof = run(srcAndTest, { + runnerScript: [ + '#!/usr/bin/env bash', + 'grep -qx b packages/cli/src/a.ts || exit 0', + 'grep -qx t2 packages/cli/src/a.test.ts || exit 0', + 'exit 1', + ].join('\n'), + resolverLines: ['packages/cli'], + workdir: criticalClaim, + }); + expect(treeProof.out).toContain('bite confirmed'); + // Negative control: a runner that bites only on ROUND source ('c') + // never sees it on the detached tree — all-green, so the defect-claim + // round is rejected, proving the detach actually reverted the source. + const roundLeak = run(srcAndTest, { + runnerScript: [ + '#!/usr/bin/env bash', + 'grep -qx c packages/cli/src/a.ts && exit 1', + 'exit 0', + ].join('\n'), + resolverLines: ['packages/cli'], + workdir: criticalClaim, + }); + expect(roundLeak.out).toContain('REJECT:bite check'); + // A cross-package round skips (dist confound), it never rejects. + const skipped = run(srcAndTest, { + runnerExit: 0, + resolverLines: ['packages/cli', 'packages/core'], + workdir: criticalClaim, + }); + expect(skipped.out).toContain('bite check skipped'); + expect(skipped.out).toContain('SURVIVED'); + // A test-only round (no source change) is coverage addition, not a + // defect claim — no bite requirement. + const coverageOnly = run( + { + base: ({ write }) => { + write('packages/cli/src/a.ts', 'a\n'); + write('packages/cli/src/a.test.ts', 't\n'); + }, + pr: () => {}, + round: ({ write }) => write('packages/cli/src/a.test.ts', 't-more\n'), }, - }).out, - ).toContain('REJECT:bite check'); - // Any failure on the pre-round tree = the tests bite => round proceeds, - // and the verification tree is restored to the branch, clean. - const bit = run(srcAndTest, { - runnerExit: 1, - resolverLines: ['packages/cli'], - workdir: criticalClaim, - }); - expect(bit.out).toContain('bite confirmed'); - expect(bit.out).toContain('SURVIVED'); - expect(bit.status).toBe(''); - expect(bit.head).toBe('feat'); - // Tree-state proof: the runner inspects the ACTUAL checkout instead of - // returning a fixed code. It fails (bites) only when it sees PRE-ROUND - // source ('b') alongside the ROUND's test ('t2') — passing proves the - // detach reverted the source AND the overlay delivered the round's test. - const treeProof = run(srcAndTest, { - runnerScript: [ - '#!/usr/bin/env bash', - 'grep -qx b packages/cli/src/a.ts || exit 0', - 'grep -qx t2 packages/cli/src/a.test.ts || exit 0', - 'exit 1', - ].join('\n'), - resolverLines: ['packages/cli'], - workdir: criticalClaim, - }); - expect(treeProof.out).toContain('bite confirmed'); - // Negative control: a runner that bites only on ROUND source ('c') - // never sees it on the detached tree — all-green, so the defect-claim - // round is rejected, proving the detach actually reverted the source. - const roundLeak = run(srcAndTest, { - runnerScript: [ - '#!/usr/bin/env bash', - 'grep -qx c packages/cli/src/a.ts && exit 1', - 'exit 0', - ].join('\n'), - resolverLines: ['packages/cli'], - workdir: criticalClaim, - }); - expect(roundLeak.out).toContain('REJECT:bite check'); - // A cross-package round skips (dist confound), it never rejects. - const skipped = run(srcAndTest, { - runnerExit: 0, - resolverLines: ['packages/cli', 'packages/core'], - workdir: criticalClaim, - }); - expect(skipped.out).toContain('bite check skipped'); - expect(skipped.out).toContain('SURVIVED'); - // A test-only round (no source change) is coverage addition, not a - // defect claim — no bite requirement. - const coverageOnly = run( - { - base: ({ write }) => { - write('packages/cli/src/a.ts', 'a\n'); - write('packages/cli/src/a.test.ts', 't\n'); + { + runnerExit: 0, + resolverLines: ['packages/cli'], + workdir: criticalClaim, }, - pr: () => {}, - round: ({ write }) => write('packages/cli/src/a.test.ts', 't-more\n'), - }, - { - runnerExit: 0, + ); + expect(coverageOnly.out).toContain('SURVIVED'); + expect(coverageOnly.out).not.toContain('REJECT:'); + expect(coverageOnly.advisory).toContain('test-only changes'); + + // Restore-failure crash contract: when the tree cannot come back to + // the branch, the gate crashes VERDICT-LESS — rejection document + // written, exit 1, and reject_fix (which would advance the watermark) + // never runs. + const crash = run(srcAndTest, { + runnerScript: [ + '#!/usr/bin/env bash', + 'git update-ref -d refs/heads/feat', + 'exit 0', + ].join('\n'), resolverLines: ['packages/cli'], workdir: criticalClaim, - }, - ); - expect(coverageOnly.out).toContain('SURVIVED'); - expect(coverageOnly.out).not.toContain('REJECT:'); - expect(coverageOnly.advisory).toContain('test-only changes'); - - // Restore-failure crash contract: when the tree cannot come back to - // the branch, the gate crashes VERDICT-LESS — rejection document - // written, exit 1, and reject_fix (which would advance the watermark) - // never runs. - const crash = run(srcAndTest, { - runnerScript: [ - '#!/usr/bin/env bash', - 'git update-ref -d refs/heads/feat', - 'exit 0', - ].join('\n'), - resolverLines: ['packages/cli'], - workdir: criticalClaim, - }); - expect(crash.out).toContain( - 'could not restore the verification tree after the bite check', - ); - expect(crash.out).toContain('[spawn status=1]'); - expect(crash.out).not.toContain('REJECT:'); - expect(crash.rejection).toContain('could not restore'); - - // Append order: a shrink advisory (truncating write) followed by the - // bite advisory (append) must leave BOTH in the report file. - const advisoryBlock2 = reviewVerificationRunner.match( - /(TEST_PATHSPEC=\(':\(glob\)[\s\S]*?advisory written for the report' \| tee -a "\$\{GATE_LOG\}"\nfi)/, - )?.[1]; - expect(advisoryBlock2).toBeTruthy(); - const combined = run( - { - base: ({ write }) => { - write( - 'packages/cli/package.json', - '{"name":"@fixture/cli","scripts":{"test":"vitest run"}}\n', - ); - write('packages/cli/src/a.ts', 'a\n'); - write('packages/cli/src/a.test.ts', 't\n'); - write( - 'packages/cli/src/big.test.ts', - `${Array.from({ length: 40 }, (_, i) => `b${i}`).join('\n')}\n`, - ); + }); + expect(crash.out).toContain( + 'could not restore the verification tree after the bite check', + ); + expect(crash.out).toContain('[spawn status=1]'); + expect(crash.out).not.toContain('REJECT:'); + expect(crash.rejection).toContain('could not restore'); + + // Append order: a shrink advisory (truncating write) followed by the + // bite advisory (append) must leave BOTH in the report file. + const advisoryBlock2 = reviewVerificationRunner.match( + /(TEST_PATHSPEC=\(':\(glob\)[\s\S]*?advisory written for the report' \| tee -a "\$\{GATE_LOG\}"\nfi)/, + )?.[1]; + expect(advisoryBlock2).toBeTruthy(); + const combined = run( + { + base: ({ write }) => { + write( + 'packages/cli/package.json', + '{"name":"@fixture/cli","scripts":{"test":"vitest run"}}\n', + ); + write('packages/cli/src/a.ts', 'a\n'); + write('packages/cli/src/a.test.ts', 't\n'); + write( + 'packages/cli/src/big.test.ts', + `${Array.from({ length: 40 }, (_, i) => `b${i}`).join('\n')}\n`, + ); + }, + pr: ({ write }) => write('packages/cli/src/a.ts', 'b\n'), + round: ({ write, dir }) => { + write('packages/cli/src/a.ts', 'c\n'); + write('packages/cli/src/a.test.ts', 't2\n'); + rmSync(join(dir, 'packages/cli/src/big.test.ts')); + }, }, - pr: ({ write }) => write('packages/cli/src/a.ts', 'b\n'), - round: ({ write, dir }) => { - write('packages/cli/src/a.ts', 'c\n'); - write('packages/cli/src/a.test.ts', 't2\n'); - rmSync(join(dir, 'packages/cli/src/big.test.ts')); + { + runnerExit: 0, + resolverLines: ['packages/cli'], + prelude: advisoryBlock2, }, - }, - { - runnerExit: 0, - resolverLines: ['packages/cli'], - prelude: advisoryBlock2, - }, - ); - expect(combined.advisory).toContain('test coverage shrank'); - expect(combined.advisory).toContain('pass on the pre-round tree'); - - // Contract pins: the rejection is non-retryable (a repair pass cannot - // make a nonexistent defect reproduce), and the report step embeds the - // gate-authored advisory file, which is also uploaded as an artifact. - expect(reviewVerificationRunner).toContain( - "reject_fix 'bite check: changed tests pass on the pre-round tree (claimed defect does not reproduce)' 'false' 'false'", - ); - expect(pushAndReportStep).toContain('gate-advisories.md'); - expect(reviewAddressJob).toContain( - 'gate-advisories.md growth-audit.json agent-api-error', - ); - const skill = readFileSync('.qwen/skills/autofix/SKILL.md', 'utf8'); - expect(skill).toContain('Verification is SOURCE-BLIND'); - expect(skill).toContain('changed tests against the pre-round branch'); - expect(skill).toContain("outside the PR's own"); - }); + ); + expect(combined.advisory).toContain('test coverage shrank'); + expect(combined.advisory).toContain('pass on the pre-round tree'); + + // Contract pins: the rejection is non-retryable (a repair pass cannot + // make a nonexistent defect reproduce), and the report step embeds the + // gate-authored advisory file, which is also uploaded as an artifact. + expect(reviewVerificationRunner).toContain( + "reject_fix 'bite check: changed tests pass on the pre-round tree (claimed defect does not reproduce)' 'false' 'false'", + ); + expect(pushAndReportStep).toContain('gate-advisories.md'); + expect(reviewAddressJob).toContain( + 'gate-advisories.md growth-audit.json agent-api-error', + ); + const skill = readFileSync('.qwen/skills/autofix/SKILL.md', 'utf8'); + expect(skill).toContain('Verification is SOURCE-BLIND'); + expect(skill).toContain('changed tests against the pre-round branch'); + expect(skill).toContain("outside the PR's own"); + }, + ); it('still runs review verification reporting when the agent step fails', () => { expect(verificationGateSteps).toHaveLength(2); @@ -20480,7 +20500,26 @@ describe('review verification gate: baseline A/B on deterministic rejection', () expect(r.headAfter).toBe('feature'); }); - it('keeps the green path intact', () => { + it('gates every mapfile-crossing runGate flow on the host probe', () => { + // These flows run the REAL script past the bite section's + // unconditional top-level `mapfile -d ''`: on a bash without mapfile + // (macOS ships 3.2) the spawn dies there with exit 127 before the + // semantics under test can execute, so each carries the host gate. + // Two siblings a main merge added went ungated and turned the revived + // macOS lane red; the pin keeps a dropped gate from doing it again. + const self = readFileSync(fileURLToPath(import.meta.url), 'utf8'); + for (const title of [ + 'keeps the green path intact', + 'rejects a conflict verdict whose round completed as fixed', + 'locks the runner file-command backing files against env plants', + ]) { + expect(self).toMatch( + new RegExp(`it\\.skipIf\\(!hasBashMapfile\\)\\(\\s*'${title}'`), + ); + } + }); + + it.skipIf(!hasBashMapfile)('keeps the green path intact', () => { const r = runGate({ failAt: [] }); expect(r.status).toBe(0); expect(r.outputs).toContain('outcome=fixed'); @@ -20927,30 +20966,33 @@ describe('review verification gate: baseline A/B on deterministic rejection', () ); }); - it('rejects a conflict verdict whose round completed as fixed', () => { - // The routing check cannot see the planted-handoff shape: conflict + - // handoff.md + commit + address-summary + green checks clears every - // earlier gate and would push the contested code under outcome=fixed - // while the report posts the park marker. The refusal sits at the push - // boundary — NOT at the verdict gate, where it would also refuse a - // legitimate repair-pass re-audit to conflict (which runs behind the - // first pass's commit and stops with failure.md). - const r = runGate({ - kissAudit: true, - auditJson: conflictAuditJson, - handoffMd: 'conflict handoff\n', - }); - expect(r.status).toBe(1); - expect(r.outputs).not.toContain('outcome=fixed'); - expect(r.outputs).not.toContain('retryable=true'); - // The validated verdict still surfaces: the trail marker posts and the - // park engages on the handoff's question. - expect(r.outputs).toContain('audit_verdict=conflict'); - expect(r.outputs).toContain('outcome=failed'); - expect(r.rejection).toContain( - 'growth-audit verdict is conflict but the round completed as fixed', - ); - }); + it.skipIf(!hasBashMapfile)( + 'rejects a conflict verdict whose round completed as fixed', + () => { + // The routing check cannot see the planted-handoff shape: conflict + + // handoff.md + commit + address-summary + green checks clears every + // earlier gate and would push the contested code under outcome=fixed + // while the report posts the park marker. The refusal sits at the push + // boundary — NOT at the verdict gate, where it would also refuse a + // legitimate repair-pass re-audit to conflict (which runs behind the + // first pass's commit and stops with failure.md). + const r = runGate({ + kissAudit: true, + auditJson: conflictAuditJson, + handoffMd: 'conflict handoff\n', + }); + expect(r.status).toBe(1); + expect(r.outputs).not.toContain('outcome=fixed'); + expect(r.outputs).not.toContain('retryable=true'); + // The validated verdict still surfaces: the trail marker posts and the + // park engages on the handoff's question. + expect(r.outputs).toContain('audit_verdict=conflict'); + expect(r.outputs).toContain('outcome=failed'); + expect(r.rejection).toContain( + 'growth-audit verdict is conflict but the round completed as fixed', + ); + }, + ); it('passes a conflict round that stopped with a non-empty handoff', () => { // The handoff.md stop shape the routing check exists for: conflict + @@ -21079,19 +21121,22 @@ describe('review verification gate: baseline A/B on deterministic rejection', () expect(r.outputs).toContain('kiss_audit=true'); }); - it('locks the runner file-command backing files against env plants', () => { - // The strip removes the GITHUB_ENV VARIABLE from the checks, but the - // backing files under $RUNNER_TEMP/_runner_file_commands/ stay - // discoverable (a predictable path) and writable — an append there - // plants environment into every later step of the job, the PAT- - // bearing one included. The gate locks them for the step's lifetime. - const r = runGate({ forgeEnvFile: true }); - expect(r.status).toBe(0); - expect(r.stdout).toContain('env forge blocked: backing file locked'); - // The gate's OWN channel keeps working through the lock. - expect(r.outputs).toContain('outcome=fixed'); - expect(r.outputs).toContain('kiss_audit=false'); - }); + it.skipIf(!hasBashMapfile)( + 'locks the runner file-command backing files against env plants', + () => { + // The strip removes the GITHUB_ENV VARIABLE from the checks, but the + // backing files under $RUNNER_TEMP/_runner_file_commands/ stay + // discoverable (a predictable path) and writable — an append there + // plants environment into every later step of the job, the PAT- + // bearing one included. The gate locks them for the step's lifetime. + const r = runGate({ forgeEnvFile: true }); + expect(r.status).toBe(0); + expect(r.stdout).toContain('env forge blocked: backing file locked'); + // The gate's OWN channel keeps working through the lock. + expect(r.outputs).toContain('outcome=fixed'); + expect(r.outputs).toContain('kiss_audit=false'); + }, + ); it('leaves the growth-audit verdict check inert on non-audit rounds', () => { // Without the KISS_AUDIT tag a malformed verdict file must not engage diff --git a/scripts/tests/qwen-pr-review-workflow.test.js b/scripts/tests/qwen-pr-review-workflow.test.js index 5102240d07a..6fcd4c67c4e 100644 --- a/scripts/tests/qwen-pr-review-workflow.test.js +++ b/scripts/tests/qwen-pr-review-workflow.test.js @@ -5,7 +5,7 @@ */ import { afterAll, describe, expect, it } from 'vitest'; -import { execFileSync } from 'node:child_process'; +import { execFileSync, spawnSync } from 'node:child_process'; import { chmodSync, existsSync, @@ -3138,6 +3138,21 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { expect(run.trim()).toMatch(/exit "\$status"$/); }); + // The repair path names its canary with GNU `mktemp -u` (print-only): + // BSD mktemp's `-u` still tries to create the file, so in the unwritable + // directory the repair case breaks it exits nonzero with an empty name + // and the post-repair touch can never succeed. The health probe only + // runs on Linux runners (the review pool is Linux-only, same as the + // realpath case above), so the defect cannot exist in production; probe + // the host, not the platform — a BSD host with GNU coreutils fronting + // PATH keeps the coverage. + const hasGnuMktemp = + spawnSync( + 'mktemp', + ['-u', join(tmpdir(), 'qwen-no-such-dir', '.probe-XXXXXX')], + { stdio: 'ignore' }, + ).status === 0; + // Executed shape for the health probe: run the step's REAL bash against // a fake runner tree with a stub sudo, so the repair-vs-fail-fast // decision is exercised, not just textually pinned. @@ -3210,13 +3225,16 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { expect(r.stdout).not.toContain('repaired'); }); - it('repairs a single unwritable directory instead of failing fast', () => { - // Mutant control: a status=1 right after the first failed touch would - // abort this repairable runner (exit 1) — a false fail-fast. - const r = runHealthProbe({ breakDir: 'home' }); - expect(r.status).toBe(0); - expect(r.stdout).toContain('repaired write access'); - }); + it.skipIf(!hasGnuMktemp)( + 'repairs a single unwritable directory instead of failing fast', + () => { + // Mutant control: a status=1 right after the first failed touch would + // abort this repairable runner (exit 1) — a false fail-fast. + const r = runHealthProbe({ breakDir: 'home' }); + expect(r.status).toBe(0); + expect(r.stdout).toContain('repaired write access'); + }, + ); it('fails fast when repair is impossible', () => { // Mutant control: dropping the post-repair re-probe would report this