Skip to content

Commit 6100ac0

Browse files
committed
fix(gate-manifest): declare the forwarded selector rule instead of proving it
The forwarding exemption fails open in two more ways, and closing them soundly leaves it exempting nothing. An upstream callback was never part of the provenance chain, so `BUILD_OWNERSHIP.filter((entry) => { entry.rule = computed; return true; }) .map(...)` was accepted: `yieldsOwnershipEntries` walked the chain by method name only. And `onlyReadsAsProperty` counted `entry.mutate()` as a property read, because `entry.mutate` is a member access whose object is `entry`. Closing the second requires treating any member call on the binding as unsafe. The live call site is `BUILD_OWNERSHIP.filter((entry) => entry.owns(file)).map(...)`, and `entry.owns(file)` is exactly that — a member call no reader can prove does not mutate the entry. So the sound version of the exemption rejects the one call it existed to admit. There is no proof left to keep, and six rounds of narrowing it (pattern → name → lexical scope → binder kind → total read-only proof) were rounds spent on a construct that could not reach soundness. The reader now collects string literals and nothing else. The single forward is declared in FORWARDED_SELECTOR_RULES, keyed on the exact source text of the call, and policed like every other waiver: check.ts fails if it matches no call (inert) or more than one (spread beyond what was reviewed), and the message quotes the exact text to paste. The category universe is unchanged — `entry.rule` can only be a `rule:` literal the table already contributes, which is what makes the declaration true. selector-rules.ts drops from 344 to 222 lines. The shape-by-shape tests become one table so all seventeen negatives, including the two above, stay as the regression record rather than as arguments to relitigate.
1 parent 4e8a2ec commit 6100ac0

6 files changed

Lines changed: 404 additions & 441 deletions

File tree

docs/agents/testing.md

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,21 @@ action → package script → aggregate chain → terminal`, and asserts:
226226
Two properties are load-bearing. **Ownership is proven, never inferred from a name appearing in
227227
workflow text** — commands resolve to *terminals* (`vitest:<project>`, `node-test:<file>`,
228228
`exec:<argv>`), so renaming an intermediate script breaks the chain instead of leaving the claim
229-
standing. And it **fails closed**: an unreadable edge (a `${{ … }}` in command position, a
230-
missing local action, an opaque runner that spawns its own child) is a failure until classified
231-
in `scripts/gate-manifest/waivers.ts` with a reason and a tracking issue. Waivers are themselves
232-
checked, so one that stops applying fails rather than rotting.
229+
standing. And it **fails closed**: anything unreadable — a `${{ … }}` in command position, a
230+
missing local action, an opaque runner that spawns its own child, a selector rule that is not a
231+
string literal — is a failure until classified in `scripts/gate-manifest/waivers.ts` with a
232+
reason and a tracking issue. Waivers are themselves checked, so one that stops applying fails
233+
rather than rotting: each is re-resolved with itself removed and must change the outcome.
234+
235+
Where the two meet is worth knowing before you edit the selector. The path-category universe is
236+
*derived* from `scripts/check-affected/model.ts` — the third argument of each `reason()` call and
237+
each `BUILD_OWNERSHIP` entry's `rule:` — so a new ownership rule widens the universe and fails
238+
the gate until a representative path exercises it. Only string literals count. A rule the reader
239+
cannot see would slip past the reachability check while the gate stayed green, so a computed one
240+
is an error naming its line. The single call that forwards `entry.rule` out of the ownership loop
241+
is declared in `FORWARDED_SELECTOR_RULES`, keyed on the exact text of the call, rather than
242+
recognized by shape; `selector-rules.ts` records why the shape-recognizing version could not be
243+
made sound.
233244

234245
The gate is deterministic, offline, and needs no GitHub token — it runs from a clean checkout in
235246
the `Affected-check Selector` job. Branch-protection required-contexts drift is the one part

scripts/gate-manifest/check.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@ import {
2727
unrepresentedRules,
2828
type PathCategory,
2929
} from './path-categories.ts';
30-
import { selectorRuleIds } from './selector-rules.ts';
30+
import { readSelectorRules } from './selector-rules.ts';
3131
import { suiteUniverse, unownedTerminals } from './suite-ownership.ts';
3232
import { buildLanes, parseWorkflow, type WorkflowFile } from './workflow-lanes.ts';
3333
import {
3434
CATALOG_CLAIM_WAIVERS,
3535
DECLARED_EDGES,
3636
DOCS_LANE_OWNERS,
37+
FORWARDED_SELECTOR_RULES,
3738
LOCAL_ONLY,
3839
TRANSPARENT_WRAPPERS,
3940
type DocsLaneOwner,
@@ -70,7 +71,12 @@ const actions = new Map(
7071
const packageJson = JSON.parse(read('package.json')) as { scripts?: Record<string, string> };
7172
const packageScripts = new Map(Object.entries(packageJson.scripts ?? {}));
7273
const vitestProjects = vitestProjectNames('vitest.config.ts', read('vitest.config.ts'));
73-
const selectorRules = selectorRuleIds(SELECTOR_SOURCE, read(SELECTOR_SOURCE));
74+
const selector = readSelectorRules(
75+
SELECTOR_SOURCE,
76+
read(SELECTOR_SOURCE),
77+
FORWARDED_SELECTOR_RULES.map((entry) => entry.call),
78+
);
79+
const selectorRules = selector.rules;
7480

7581
const trackedFiles = listFiles();
7682
const trackedSet = new Set(trackedFiles);
@@ -210,6 +216,22 @@ const staleWaivers = [
210216
`TRANSPARENT_WRAPPERS "${entry.file}" changes nothing — no resolved command forwards ` +
211217
`through it, so the waiver is inert`,
212218
),
219+
// A forwarded-rule waiver is applied-reachable when the call it names is really there — and
220+
// only that call. Matching none means the forward is gone and the waiver now excuses nothing;
221+
// matching several means one reviewed claim has silently spread to a call nobody looked at.
222+
...FORWARDED_SELECTOR_RULES.map((entry) => ({
223+
entry,
224+
matches: selector.waiverMatches.get(entry.call) ?? 0,
225+
}))
226+
.filter(({ matches }) => matches !== 1)
227+
.map(({ entry, matches }) =>
228+
matches === 0
229+
? `FORWARDED_SELECTOR_RULES "${entry.call}" matches no reason() call in ` +
230+
`${SELECTOR_SOURCE} — the forward is gone, so the waiver is inert`
231+
: `FORWARDED_SELECTOR_RULES "${entry.call}" matches ${matches} calls in ` +
232+
`${SELECTOR_SOURCE} — one waiver cannot stand for several forwards; make each call ` +
233+
`distinguishable, or write the rules as literals`,
234+
),
213235
...DECLARED_EDGES.filter(
214236
(entry) =>
215237
reportedProblems(
@@ -414,5 +436,10 @@ console.log(
414436
`across ${lanes.length} jobs in ${workflows.length} workflows (${prLanes} PR-triggered); ` +
415437
`${CHECK_CATALOG.length} catalog entries wired to live jobs; ` +
416438
`${selectorRules.length} selector categories represented and reachable; ` +
417-
`${LOCAL_ONLY.length + DECLARED_EDGES.length + TRANSPARENT_WRAPPERS.length} owned waivers.`,
439+
`${
440+
LOCAL_ONLY.length +
441+
DECLARED_EDGES.length +
442+
TRANSPARENT_WRAPPERS.length +
443+
FORWARDED_SELECTOR_RULES.length
444+
} owned waivers.`,
418445
);

scripts/gate-manifest/path-categories.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import { selectChecks } from '../check-affected/model.ts';
88
import type { CatalogEntry } from './catalog-wiring.ts';
99
import { pathFilterMatches, triggersOnPath } from './path-filters.ts';
1010
import { unreachablePathCategories, unrepresentedRules } from './path-categories.ts';
11-
import { selectorRuleIds } from './selector-rules.ts';
11+
import { readSelectorRules } from './selector-rules.ts';
1212
import { buildLanes, parseWorkflow } from './workflow-lanes.ts';
1313
import { context } from './test-context.ts';
14+
import { FORWARDED_SELECTOR_RULES } from './waivers.ts';
1415

1516
const repoRoot = path.resolve(import.meta.dirname, '../..');
1617
const SELECTOR_SOURCE = 'scripts/check-affected/model.ts';
@@ -109,9 +110,10 @@ test('a selector rule no sample path exercises is reported as unrepresented', ()
109110
// --- The real tree ----------------------------------------------------------
110111

111112
test("the selector's real rule universe is derived, and excludes its fail-open classes", () => {
112-
const rules = selectorRuleIds(
113+
const { rules } = readSelectorRules(
113114
SELECTOR_SOURCE,
114115
fs.readFileSync(path.join(repoRoot, SELECTOR_SOURCE), 'utf8'),
116+
FORWARDED_SELECTOR_RULES.map((entry) => entry.call),
115117
);
116118
// Live selection rules, read from `reason(...)` calls and the BUILD_OWNERSHIP table.
117119
for (const expected of ['gate:lint', 'platform-src', 'own:swift', 'own:mcp', 'src-prod']) {

0 commit comments

Comments
 (0)