Repository navigation
fix(ci): give the macOS and Windows lanes a trigger again #9370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+1,514
−471
Merged
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
7c9c073
fix(ci): give the macOS and Windows lanes a trigger again
wenshao 49ae2b2
fix(ci): gate the Windows lane's checkout verification per trigger
wenshao a119a73
Merge branch 'main' into fix/revive-platform-lanes
qwen-code-dev-bot 8147795
Merge branch 'main' into fix/revive-platform-lanes
qwen-code-dev-bot ffcb290
Merge branch 'main' into fix/revive-platform-lanes
qwen-code-dev-bot c1a7a68
fix(ci): close review round on the revived platform lanes (#9370)
qwen-code-dev-bot 66ec023
Merge branch 'main' into fix/revive-platform-lanes
wenshao 951521c
fix(ci): pin the Windows lane routing to the canonical trust policy (…
qwen-code-dev-bot 6103257
Merge branch 'fix/revive-platform-lanes' of https://github.com/QwenLM…
qwen-code-dev-bot 92eec8f
Merge branch 'main' into fix/revive-platform-lanes
qwen-code-dev-bot 97544b3
fix(ci): host-probe the macOS lane's bash and GNU dependencies (#9370)
qwen-code-dev-bot 462962e
fix(ci): stop the subsystem rule matching compounds that name somethi…
wenshao 7f7223a
test(ci): pin the gate as a disjunction and the watcher's name binding
wenshao 14d138d
fix(ci): close the classifier's CRLF gap and widen the lane step scan
wenshao b9de49d
Merge branch 'main' into fix/revive-platform-lanes
wenshao ba1e80c
style(ci): format the runner-routing suite
wenshao 4fefc81
Merge branch 'main' into fix/revive-platform-lanes
wenshao fcdf317
Merge branch 'main' into fix/revive-platform-lanes
wenshao 2f871fb
Merge branch 'main' into fix/revive-platform-lanes
wenshao bb599f6
Merge branch 'main' into fix/revive-platform-lanes
wenshao 6073afb
Merge remote-tracking branch 'origin/main' into fix/revive-platform-l…
qwen-code-dev-bot bcaa9a3
Merge remote-tracking branch 'origin/main' into fix/revive-platform-l…
qwen-code-dev-bot bab6596
fix(ci): gate the two mapfile-crossing gate tests on the host probe (…
qwen-code-dev-bot 07040ac
Merge branch 'main' into fix/revive-platform-lanes
wenshao 6f73fbf
fix(ci): keep pull requests off the persistent Windows pool and cover…
qwen-code-dev-bot b66a4fa
Merge branch 'main' into fix/revive-platform-lanes
qwen-code-dev-bot bb5c28b
Merge branch 'main' into fix/revive-platform-lanes
wenshao File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| #!/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. Matched on whole path segments and | ||
| // on the dot/dash/underscore-separated parts of a basename, so `pty-host.ts` | ||
| // and `src/sandbox/index.ts` match while `computed.ts` and `spatial/` do not. | ||
| const PLATFORM_SUBSYSTEM = | ||
| /(?:^|[/\-._])(?:pty|tty|sandbox|seatbelt|shell|terminal|clipboard|platform|posix|darwin|macos|windows|win32|linux|keychain|codesign|installer|filesystem)(?:[/\-._]|$)/i; | ||
|
wenshao marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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) || | ||
| PLATFORM_SUBSYSTEM.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) | ||
| .split('\n') | ||
|
wenshao marked this conversation as resolved.
Outdated
wenshao marked this conversation as resolved.
Outdated
|
||
| .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(); | ||
| } | ||
179 changes: 179 additions & 0 deletions
179
.github/scripts/ci/classify-platform-sensitivity.test.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| 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', | ||
| ]) { | ||
| 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); | ||
| } | ||
| // 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('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('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, | ||
| ); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.