Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
ci(windows): scope the recovery lane to its own inputs by Astro-Han · Pull Request #3914 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(windows): scope the recovery lane to its own inputs by Astro-Han · Pull Request #3914 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(windows): scope the recovery lane to its own inputs by Astro-Han · Pull Request #3914 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' ci(windows): scope the recovery lane to its own inputs by Astro-Han · Pull Request #3914 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(windows): scope the recovery lane to its own inputs by Astro-Han · Pull Request #3914 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(windows): scope the recovery lane to its own inputs by Astro-Han · Pull Request #3914 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); ci(windows): scope the recovery lane to its own inputs by Astro-Han · Pull Request #3914 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .asf.yaml
Original file line numberDiff line numberDiff line change
Expand Up@@ -67,12 +67,12 @@ github:
# Storybook. Renaming it there, or adding a paths filter that stops
# ci.yml from running, freezes every pull request: the check never
# reports and no committer can override it.
# A required context must report on every pull request, so a lane
# behind a paths filter cannot be listed here: the filter would keep
# the workflow from starting and the check would stay pending forever.
# windows_recovery is filtered and therefore deliberately absent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 从 required contexts 拿掉 windows_recovery 不是解堵 Windows 队列的最小改动。

后果有两层:

  1. 命中 filter 的 PR 上,recovery 变红也不再挡 merge。这是在撤销 fix(windows): make recovery evidence authoritative #3789 的 authoritative gate,不是「scope the lane to its own inputs」。
  2. squash 进 main 之后、ASF 应用这份 .asf.yaml 之前,workflow 上的 paths 已经生效,required context 还在。不匹配路径的 PR 不会启动检查、也无法变绿,比现在「排队但能跑完」更堵。PR 正文写了这个窗口,但窗口没有上界。

paths 和 required 不能共存,只对 workflow-level filter 成立。Job-level if: 跳过的 job 报告 skipped,不占 windows-latest,required 视为通过。要解堵:不要在 on.pull_request 上加 paths,保留这个 context;用单独的 select(ubuntu / 已有 planner)决定 windows_recovery 是否跑。相关 PR 失败仍挡;无关 PR 零 Windows runner;没有 ASF reconcile 窗口。

若产品决定 recovery 就该是 advisory(对齐 sandbox / owner-platform / #3261),应先只落地 .asf.yaml、等 required 真正消失,再加 paths。不要同一 squash 里两件事一起做。

contexts:
- test
# Windows recovery is a separate native crash/owner-death boundary.
# The workflow runs on every PR and main push so this context can be
# required without leaving unrelated pull requests pending forever.
- windows_recovery

rulesets:
- name: Immutable release tags
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-baseline.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ jobs:

# Full packages/storage test:dist is ~10 minutes on windows-latest and
# mostly duplicates the Linux unit lane. Baseline keeps process/path/
# lock-sensitive gates here. Release-blocking crash evidence belongs to
# lock-sensitive gates here. Crash and owner-death evidence belongs to
# windows-recovery.yml, so this diagnostic lane does not duplicate it.
# Bump concurrency carefully — several suites spawn child processes and
# fight for disk under high fan-out.
Expand Down
61 changes: 59 additions & 2 deletions .github/workflows/windows-recovery.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,73 @@

name: Windows recovery

# The paths below are a pre-filter, not this lane's real input. The real input
# is the import closure of the crash and owner-death recovery authorities in
# storage, runtime and Runtime Host, which reaches well past any list worth
# hand-maintaining. So they name those workspaces and every workspace they hold
# a TypeScript project reference to, plus the manifests, patches and scripts
# the unconditional install and clean steps consume. That keeps a change there
# reported before merge, and the nightly run covers the transitive edits the
# list cannot match.
#
# GitHub evaluates a path filter against the first 300 files of the diff only,
# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every
# recovery authority at once, which is one more reason the main push below
# carries no filter at all.
on:
pull_request:
branches: [main]
paths:
- 'package.json'
- 'package-lock.json'
- 'patches/**'
- 'scripts/apply-dependency-patches.mjs'
- 'scripts/install-electron-with-retry.mjs'
- 'scripts/run-electron-installer.cjs'
- 'scripts/clean-build.mjs'
- 'scripts/clean-paths.mjs'
- 'scripts/windows-runtime-host-local-ipc-trust.ps1'
- 'tsconfig.base.json'
- 'tsconfig.lib.json'
- 'packages/core/package.json'
- 'packages/core/tsconfig.json'
- 'packages/core/src/**'
- 'packages/storage/package.json'
- 'packages/storage/tsconfig.json'
- 'packages/storage/src/**'
- 'packages/runtime/package.json'
- 'packages/runtime/tsconfig.json'
- 'packages/runtime/src/**'
- 'packages/runtime/scripts/**'
- 'packages/runtime-host/package.json'
- 'packages/runtime-host/tsconfig.json'
- 'packages/runtime-host/src/**'
- '.github/workflows/windows-recovery.yml'
# Unfiltered on purpose: required_status_checks is `strict: false`, so a pull
# request goes green against a stale base and only the merged result proves
# two independently green halves still agree. It is also the backstop for the
# 300-file case above. The nightly alone would find either a day later,
# against a batch of commits instead of one.
push:
branches: [main]
schedule:
# Offset from windows-sandbox-w0 so the Windows lanes do not overlap.
- cron: '17 8 * * *'
workflow_dispatch:

# Pull request pushes supersede each other, keyed on the pull request number
# because github.head_ref is a bare branch name two forks can share: a second
# contributor pushing to their own `main` would otherwise cancel the first
# contributor's run, and a cancelled check is not a failed one.
# Scheduled, manual and main-push runs fall back to the run id, since
# github.ref is refs/heads/main for all three and one shared group would let a
# dispatch queue behind the nightly and then be discarded while still pending.
# That deliberately stops main pushes from superseding each other: this lane is
# the only place a merged Windows regression is observed, so every merge needs
# its own evidence rather than only the newest one surviving.
concurrency:
group: windows-recovery-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
group: windows-recovery-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions:
contents: read
Expand Down
172 changes: 162 additions & 10 deletions scripts/ci-test-plan.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,7 @@ import assert from 'node:assert/strict';
import { readdirSync, readFileSync } from 'node:fs';
import test from 'node:test';

import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs';
import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs';

const dirs = [
'packages/core',
Expand DownExpand Up@@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => {
]);
});

test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => {
test('the recovery lane pairs its path filter with a nightly run and a main push', () => {
// Read from the `on:` block with comments stripped, so documenting a trigger
// cannot break its contract.
const triggers = triggerBlock('windows-recovery.yml');

// Same contract as the sandbox lane: the filter is a pre-filter, not the
// lane's import closure, so dropping the schedule would silently lose every
// transitive edit it cannot match, and dropping the filter would put every
// Windows recovery run back on every pull request. The main push carries no
// filter because `strict: false` lets a stale-base pull request go green,
// and because a paths filter only sees the first 300 files of a diff.
// Stripped comment lines survive as blank ones, so the gap between the
// trigger and its list is any mix of blank and four-space lines.
assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u);
assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u);
assert.doesNotMatch(
triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '',
/\bpaths(-ignore)?:/u,
);
assert.match(triggers, /\n {2}schedule:\n/u);
assert.match(triggers, /\n {2}workflow_dispatch:/u);
assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u);
});

test('the recovery lane keeps every run kind out of one shared concurrency group', () => {
const workflow = readWorkflow('windows-recovery.yml');

// github.head_ref is a bare branch name, so two forks pushing their own
// `main` would share a group and cancel each other; github.ref is
// refs/heads/main for the nightly, a dispatch and a main push alike, so a
// ref-keyed group made a dispatch queue behind the nightly and let the next
// dispatch discard it while pending.
assert.match(
workflow,
/group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u,
);
assert.match(workflow, /\n {2}cancel-in-progress: true/u);
});

test('the recovery lane filters pull requests by the workspaces its steps execute', () => {
const workflow = readWorkflow('windows-recovery.yml');
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

// Derived from the dist paths the steps run, then widened along the workspace
// dependency graph the planner selects with. The separator class matches the
// backslash form too, because these steps run under pwsh where both are
// legal. A new workspace on this lane, or a new dependency under one of them,
// fails here until the filter admits its sources and project file.
const executed = [
...new Set(
[...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]),
),
].sort();
assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`));
assert.ok(closure.includes('packages/core'), 'dependency closure must reach core');
for (const dir of closure) {
assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`);
assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`);
assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`);
}
});

test('the recovery lane filter follows the postinstall launcher chain', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));
const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8'));
// Derived from postinstall itself, then one hop into whatever those entry
// points launch, because a launcher the filter cannot see still decides what
// `npm ci` produces on Windows. A restated list missed exactly that hop.
const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map(
(match) => match[1],
);
assert.ok(entrypoints.length > 0, 'postinstall runs no script');

for (const entrypoint of entrypoints) {
assert.ok(filtered.has(entrypoint), entrypoint);
const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8');
for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) {
assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`);
}
}
});

test('the recovery lane filters pull requests by what its install and clean steps consume', () => {
const filtered = new Set(pullRequestPathFilter('windows-recovery.yml'));

assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u);
assert.match(workflow, /\n {2}workflow_dispatch:/u);
assert.match(workflow, /\n {4}name: windows_recovery/u);
assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u);
// `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are
// first-class inputs of the lane rather than transitive edits the nightly can
// be left to cover. A grouped dependabot bump touches only the manifests, and
// the crash gates sit on a native file lock the Linux `test` lane never sees.
for (const path of [
'package.json',
'package-lock.json',
'patches/**',
'scripts/apply-dependency-patches.mjs',
'scripts/install-electron-with-retry.mjs',
'scripts/clean-build.mjs',
'scripts/clean-paths.mjs',
'scripts/windows-runtime-host-local-ipc-trust.ps1',
'tsconfig.base.json',
'tsconfig.lib.json',
'packages/runtime/scripts/**',
'.github/workflows/windows-recovery.yml',
]) {
assert.ok(filtered.has(path), path);
}
});

test('the sandbox lane pairs its path filter with a nightly run', () => {
Expand DownExpand Up@@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => {

const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url);

/**
* Reads the `paths` list belonging to a workflow's `pull_request` trigger.
* Anchoring to the trigger, instead of matching entry text anywhere in the
* file, is what makes the filter assertions fail when entries move under
* `paths-ignore`, under another trigger, or out of `on:` altogether.
*/
function pullRequestPathFilter(name) {
// Reads the `on:` block with comments already stripped, so a comment between
// the trigger and its list cannot end the scan, and accepts the quoting and
// spacing YAML allows, so a legal rewrite reports the entries it really has
// instead of an empty list that reads as a missing filter.
const lines = triggerBlock(name).split('\n');
const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line));
assert.ok(start >= 0, `${name}: no pull_request trigger`);

const paths = [];
let inPaths = false;
for (const line of lines.slice(start + 1)) {
if (line.trim() === '') continue;
if (/^ {0,2}\S/u.test(line)) break;
if (/^ {4}\S/u.test(line)) {
inPaths = /^ {4}paths:\s*$/u.test(line);
continue;
}
const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null;
if (entry) paths.push(entry[1]);
}
return paths;
}

/**
* Workspace dirs `seeds` depend on, transitively, read off the same graph the
* planner selects with rather than a second definition of the same edges. The
* graph stores dependents, so a dependency is any dir listing one of ours.
*/
function dependencyClosure(seeds) {
const graph = loadWorkspaceGraph();
const selected = new Set(seeds);
const pending = [...seeds];
while (pending.length > 0) {
const dir = pending.shift();
for (const [dependency, dependents] of graph.dependents) {
if (!dependents.has(dir) || selected.has(dependency)) continue;
selected.add(dependency);
pending.push(dependency);
}
}
return [...selected].sort();
}

function readWorkflow(name) {
return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8');
}
Expand All@@ -502,11 +651,14 @@ function readWorkflow(name) {
* Reads the `on:` block only, so a workflow cannot escape a trigger contract by
* writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one.
*/
function hasPullRequestTrigger(name) {
function triggerBlock(name) {
const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, '');
const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';

return /\bpull_request(_target)?\b/u.test(triggers);
return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? '';
}

function hasPullRequestTrigger(name) {
return /\bpull_request(_target)?\b/u.test(triggerBlock(name));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions scripts/product-release.test.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact',

test('repository control plane admits only reviewed immutable release tags', async () => {
const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8'));
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [
'test',
'windows_recovery',
]);
assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']);
const environments = config.github.environments;
for (const [name, tagPattern] of [
['release', 'v*-incubating-rc*'],
Expand Down