Skip to content

Commit 6bae069

Browse files
bloveclaude
andcommitted
fix(ci): stop fanning out the whole cockpit e2e matrix for unrelated changes
PR #932 changed three files under apps/website/src and ran 41 `Cockpit — e2e` lanes. Two independent scoping defects stacked up. 1. apps/cockpit carried `scope:cockpit-e2e`. The cockpit-e2e matrix dispatches `nx e2e` for the standalone Angular cap apps under cockpit/**; none of the 40 caps depends on the apps/cockpit Next.js shell, and no workflow runs the shell's own `e2e` target. The tag could never select real work — only over-select. And it did: apps/cockpit statically depends on apps/website, so a website-only PR made the shell nx-affected and flipped cockpit_e2e true. 2. cockpit-matrix matched affected projects on `cap.angular` only. A cap is two unlinked nx projects (Angular app + python backend), so a python-only cap change attributed nothing, which main() reads as "nx found no cap" and answers with the full fleet — 40 lanes to cover one cap. Measured on this checkout, before → after: website-only (PR #932 base..head) 41 lanes → 0 (job gate is now false) python-only cap change 40 lanes → 1 (the cap that changed) libs/chat change 40 lanes → 40 (unchanged; no coverage lost) The python sibling's nx name is read from its own project.json rather than derived from the path, because a wrong guess fails open to the full fleet without ever reporting that it guessed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 44388f2 commit 6bae069

4 files changed

Lines changed: 181 additions & 19 deletions

File tree

apps/cockpit/project.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
"tags": [
77
"scope:cockpit",
88
"scope:cockpit-deploy-smoke",
9-
"scope:cockpit-e2e",
109
"scope:cockpit-examples",
1110
"type:app"
1211
],

scripts/ci-scope.spec.mjs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,3 +372,52 @@ describe('SCOPE_KEYS export', () => {
372372
]);
373373
});
374374
});
375+
376+
describe('classifyFromAffected — cockpit shell does not own the e2e matrix', () => {
377+
// The cockpit-e2e matrix dispatches `nx e2e` for the standalone Angular cap
378+
// apps under cockpit/**; none of them depends on the apps/cockpit Next.js
379+
// shell, and no workflow runs the shell's own `e2e` target. A
380+
// `scope:cockpit-e2e` tag on the shell therefore cannot select any real
381+
// work — it can only over-select. It used to: apps/cockpit imports from
382+
// apps/website, so a website-only PR made the shell nx-affected, flipped
383+
// cockpit_e2e true, and (with no cap affected) hit the dispatcher's
384+
// full-fleet fallback. PR #932 changed three apps/website/src files and ran
385+
// the whole cap matrix.
386+
it('apps/cockpit is not tagged scope:cockpit-e2e', async () => {
387+
const project = JSON.parse(
388+
await readFile('apps/cockpit/project.json', 'utf8')
389+
);
390+
391+
assert.ok(
392+
!project.tags.includes('scope:cockpit-e2e'),
393+
'the cockpit shell must not select the cockpit-e2e cap matrix'
394+
);
395+
});
396+
397+
it('a website-only change leaves cockpit_e2e false', async () => {
398+
const cockpit = JSON.parse(
399+
await readFile('apps/cockpit/project.json', 'utf8')
400+
);
401+
const website = JSON.parse(
402+
await readFile('apps/website/project.json', 'utf8')
403+
);
404+
405+
// The real nx-affected set for PR #932 was [website, cockpit, scripts]:
406+
// apps/cockpit statically depends on apps/website.
407+
const scope = classifyFromAffected(
408+
[
409+
'apps/website/src/app/layout.tsx',
410+
'apps/website/src/components/shared/SiteFooter.tsx',
411+
],
412+
[
413+
{ name: 'website', tags: website.tags },
414+
{ name: 'cockpit', tags: cockpit.tags },
415+
]
416+
);
417+
418+
assert.equal(scope.cockpit_e2e, false);
419+
// The shell still builds and tests — it consumes the changed website code.
420+
assert.equal(scope.cockpit, true);
421+
assert.equal(scope.website, true);
422+
});
423+
});

scripts/cockpit-matrix.mjs

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,32 @@
11
#!/usr/bin/env node
22
// SPDX-License-Identifier: MIT
33

4+
/**
5+
* Does this cap own one of the nx-affected projects?
6+
*
7+
* A cap is two independent nx projects — the Angular app that owns the `e2e`
8+
* target and the python backend it talks to — with no edge between them in the
9+
* project graph. Matching only on the Angular name made a python-only change
10+
* look unattributed, which main() answers with the full fleet.
11+
*
12+
* @param {{angular: string, pythonName?: string}} cap
13+
* @param {Set<string>} affectedNames
14+
* @returns {boolean}
15+
*/
16+
export function isCapAffected(cap, affectedNames) {
17+
if (affectedNames.has(cap.angular)) return true;
18+
// Guard the falsy case: caps with a Node-hosted backend carry '' here, and
19+
// `affectedNames` must never be probed with an empty key.
20+
return Boolean(cap.pythonName) && affectedNames.has(cap.pythonName);
21+
}
22+
423
/**
524
* Pure-function classifier for the cockpit-e2e matrix.
625
*
7-
* @param {Array<{angular: string, python: string}>} allCockpitCaps
26+
* @param {Array<{angular: string, python: string, pythonName: string}>} allCockpitCaps
827
* All cockpit angular projects with an e2e target, paired with
9-
* their python sibling path. Derived from the project graph by
10-
* the CLI wrapper (or hard-coded in tests).
28+
* their python sibling path and project name. Derived from the
29+
* project graph by the CLI wrapper (or hard-coded in tests).
1130
* @param {Set<string>} affectedNames
1231
* Set of project names nx-affected returned for this diff.
1332
* @param {{fullFleet: boolean}} opts
@@ -19,7 +38,7 @@
1938
*/
2039
export function selectCockpitCaps(allCockpitCaps, affectedNames, { fullFleet }) {
2140
if (fullFleet) return allCockpitCaps;
22-
return allCockpitCaps.filter((cap) => affectedNames.has(cap.angular));
41+
return allCockpitCaps.filter((cap) => isCapAffected(cap, affectedNames));
2342
}
2443

2544
// ── CLI wrapper ────────────────────────────────────────────────────────────
@@ -100,7 +119,29 @@ function deriveCockpitCaps() {
100119
return false;
101120
}
102121
})();
103-
caps.push({ angular: angularName, python: hasPython ? relPython : '' });
122+
// Read the python sibling's own project.json for its nx name rather
123+
// than deriving one from the path — the name is what nx-affected
124+
// reports, and a convention-derived guess would fail open (no match
125+
// → full fleet) without ever saying so.
126+
const pythonName = (() => {
127+
if (!hasPython) return '';
128+
try {
129+
const sibling = JSON.parse(
130+
readFileSync(
131+
path.join(repoRoot, relPython, 'project.json'),
132+
'utf8',
133+
),
134+
);
135+
return typeof sibling.name === 'string' ? sibling.name : '';
136+
} catch {
137+
return '';
138+
}
139+
})();
140+
caps.push({
141+
angular: angularName,
142+
python: hasPython ? relPython : '',
143+
pythonName,
144+
});
104145
} catch {
105146
// No project.json or invalid JSON — skip silently.
106147
}
@@ -135,14 +176,18 @@ function main() {
135176

136177
// Empty-affected fallback: when scope says e2e is required but nx
137178
// didn't attribute any cap (lib fanout), run all caps.
138-
const haveAnyCockpitAffected = allCaps.some((c) => affected.has(c.angular));
179+
const haveAnyCockpitAffected = allCaps.some((c) => isCapAffected(c, affected));
139180
const effectiveFullFleet = args.fullFleet || !haveAnyCockpitAffected;
140181

141182
const selected = selectCockpitCaps(allCaps, affected, {
142183
fullFleet: effectiveFullFleet,
143184
});
144185

145-
const json = JSON.stringify(selected);
186+
// ci.yml reads matrix.cap.angular / matrix.cap.python; pythonName is an
187+
// internal attribution detail, so keep it out of the emitted matrix.
188+
const json = JSON.stringify(
189+
selected.map(({ angular, python }) => ({ angular, python })),
190+
);
146191

147192
const ghOutput = process.env.GITHUB_OUTPUT;
148193
if (ghOutput) {

scripts/cockpit-matrix.spec.mjs

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,25 @@
11
import { test, describe } from 'node:test';
22
import assert from 'node:assert/strict';
3-
import { selectCockpitCaps } from './cockpit-matrix.mjs';
3+
import { isCapAffected, selectCockpitCaps } from './cockpit-matrix.mjs';
44

55
const ALL_CAPS = [
6-
{ angular: 'cockpit-chat-messages-angular', python: 'cockpit/chat/messages/python' },
7-
{ angular: 'cockpit-chat-input-angular', python: 'cockpit/chat/input/python' },
8-
{ angular: 'cockpit-langgraph-streaming-angular', python: 'cockpit/langgraph/streaming/python' },
6+
{
7+
angular: 'cockpit-chat-messages-angular',
8+
python: 'cockpit/chat/messages/python',
9+
pythonName: 'cockpit-chat-messages-python',
10+
},
11+
{
12+
angular: 'cockpit-chat-input-angular',
13+
python: 'cockpit/chat/input/python',
14+
pythonName: 'cockpit-chat-input-python',
15+
},
16+
{
17+
angular: 'cockpit-langgraph-streaming-angular',
18+
python: 'cockpit/langgraph/streaming/python',
19+
pythonName: 'cockpit-langgraph-streaming-python',
20+
},
21+
// Node-hosted backend: no python sibling on disk.
22+
{ angular: 'cockpit-runtimes-mastra-angular', python: '', pythonName: '' },
923
];
1024

1125
describe('selectCockpitCaps', () => {
@@ -15,9 +29,7 @@ describe('selectCockpitCaps', () => {
1529
new Set(['cockpit-chat-messages-angular']),
1630
{ fullFleet: false },
1731
);
18-
assert.deepEqual(result, [
19-
{ angular: 'cockpit-chat-messages-angular', python: 'cockpit/chat/messages/python' },
20-
]);
32+
assert.deepEqual(result, [ALL_CAPS[0]]);
2133
});
2234

2335
test('returns multiple affected caps preserving input order', () => {
@@ -26,10 +38,7 @@ describe('selectCockpitCaps', () => {
2638
new Set(['cockpit-langgraph-streaming-angular', 'cockpit-chat-messages-angular']),
2739
{ fullFleet: false },
2840
);
29-
assert.deepEqual(result, [
30-
{ angular: 'cockpit-chat-messages-angular', python: 'cockpit/chat/messages/python' },
31-
{ angular: 'cockpit-langgraph-streaming-angular', python: 'cockpit/langgraph/streaming/python' },
32-
]);
41+
assert.deepEqual(result, [ALL_CAPS[0], ALL_CAPS[2]]);
3342
});
3443

3544
test('returns all caps when fullFleet=true regardless of affected', () => {
@@ -69,3 +78,63 @@ describe('selectCockpitCaps', () => {
6978
assert.deepEqual(JSON.parse(JSON.stringify(result)), result);
7079
});
7180
});
81+
82+
describe('selectCockpitCaps — python sibling attribution', () => {
83+
// A cap's python project is a separate nx project from its Angular app, and
84+
// the two are not linked in the project graph. Matching only on `cap.angular`
85+
// meant a python-only cap change produced an empty selection, which
86+
// cockpit-matrix's main() reads as "nx attributed nothing" and answers with
87+
// the full fleet — ~40 lanes to cover a one-cap change.
88+
test('selects the cap when only its python project is affected', () => {
89+
const result = selectCockpitCaps(
90+
ALL_CAPS,
91+
new Set(['cockpit-chat-messages-python']),
92+
{ fullFleet: false },
93+
);
94+
assert.deepEqual(result, [ALL_CAPS[0]]);
95+
});
96+
97+
test('does not double-select when both siblings are affected', () => {
98+
const result = selectCockpitCaps(
99+
ALL_CAPS,
100+
new Set(['cockpit-chat-messages-python', 'cockpit-chat-messages-angular']),
101+
{ fullFleet: false },
102+
);
103+
assert.deepEqual(result, [ALL_CAPS[0]]);
104+
});
105+
106+
test('mixes angular- and python-attributed caps', () => {
107+
const result = selectCockpitCaps(
108+
ALL_CAPS,
109+
new Set(['cockpit-langgraph-streaming-python', 'cockpit-chat-input-angular']),
110+
{ fullFleet: false },
111+
);
112+
assert.deepEqual(result, [ALL_CAPS[1], ALL_CAPS[2]]);
113+
});
114+
});
115+
116+
describe('isCapAffected', () => {
117+
test('matches on the angular project name', () => {
118+
assert.equal(
119+
isCapAffected(ALL_CAPS[0], new Set(['cockpit-chat-messages-angular'])),
120+
true,
121+
);
122+
});
123+
124+
test('matches on the python project name', () => {
125+
assert.equal(
126+
isCapAffected(ALL_CAPS[0], new Set(['cockpit-chat-messages-python'])),
127+
true,
128+
);
129+
});
130+
131+
test('an empty pythonName never matches an empty-string entry', () => {
132+
// cockpit-runtimes-mastra has no python sibling; a falsy pythonName must
133+
// not turn `affectedNames.has('')` into a match.
134+
assert.equal(isCapAffected(ALL_CAPS[3], new Set([''])), false);
135+
});
136+
137+
test('unrelated affected names do not match', () => {
138+
assert.equal(isCapAffected(ALL_CAPS[0], new Set(['chat', 'website'])), false);
139+
});
140+
});

0 commit comments

Comments
 (0)