Skip to content

Commit 29db3cd

Browse files
claude[bot]os-try2026claude
authored
test(objectql,runtime): widen the deleted-member absence pin from one file to the tree (#15085)
* test(objectql,runtime): widen the deleted-member absence pin from one file to the tree PR #14667 deleted the private `ObjectQLPlugin.actionObjectKey` and wrote a guard for it: `expect(plugin!.text).not.toContain('actionObjectKey')`. The kind of guard was right; its SCOPE was the defect. A pin written by the deleting PR can only look where its author thought to look, and the whole failure mode is references the author did not know about — five files in three other packages went on naming the dead member as a live reader, and one deletion produced two separate follow-up cards. Both `action-owner-key-single-source.test.ts` pins now assert the absence tree-wide instead: one `git grep` over `.ts` under `packages/` and `examples/`, tracked plus untracked, with three exclusion rules carried in the pin beside their reasons (published CHANGELOGs, `.changeset/`, and the two pins themselves, which name the member because naming it is how they hunt for it). Widening also covers the half no removal-time check can see. Three of the five references existed when the member died; the other two were written 1 h 41 min AFTER it, by a later PR, into a file that was clean at deletion time. A pin that runs on every PR reddens on that second kind at the moment it is written. Also in this change: - `packages/spec/src/stack.zod.ts` — two comments naming the dead member as the registration-key reader TODAY now name `standaloneActionOwnerKey`, the live helper. Only that one word rots; `collectBundleActions` beside it is alive. - `packages/objectql/src/action-governance.ts` — accurate history, reworded so it no longer carries the dead name. - `scripts/cross-package-test-inputs.mjs` and `turbo.json` — the declaration a tree-scoped test owes. Without it neither the affected-subset filter nor the turbo cache re-runs these suites for the files they now judge, which is the blind spot `check:cross-package-test-inputs` exists to close. ⛔ An assertion of absence is not a stale mention. The pins name the member on purpose; "repairing" those lines deletes the guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk * fix(objectql,runtime): scope the absence pin to packages/, which the gate farm owns `pnpm check:pm-dispatch-gates` went red on the previous commit — reproduced locally, 1 of 1291 self-test cases failed, exit 1: ✗ but no hint of this gate reaches a test file outside packages/** Root cause, measured rather than guessed. `scripts/cross-package-test-inputs.mjs` is a declaration table that `check-cross-package-test-inputs.mjs` imports, and `dispatch-gates.mjs` appends a followed module's globs to every importer as watch hints. So the `examples/**/*.ts` glob the previous commit added became an inherited hint on that gate — and the self-test pins that no hint of it reaches a test file outside `packages/**`, which is the whole reason the gate is listed as a change-KIND rather than a path derivation. That case is not a count to bump. All 41 tracked test files outside `packages/` are under `examples/`, so one examples-wide glob does not shrink the residue class it guards, it EMPTIES it — and the case's own instruction ("re-point at another member of its class") cannot be followed because no other member exists. Editing it would be weakening a gate to fit a declaration, in a `scripts/pm/` file this change does not own. So the pin narrows instead: `SCANNED_ROOTS` is `packages/` alone, and the declared glob drops to `packages/**/*.ts`, which the table already carried for `@objectstack/core` and `@objectstack/types` — the hint population is now byte-identical to `origin/main`'s and the census does not move at all. ⚠️ What that costs is written into both pins rather than left to be rediscovered: of this symbol's five surviving references, four were under `packages/**` and one was a test under the showcase example, which this pin no longer sees. Widening needs the residue measurement behind that self-test case redone first; the headers say so, and say that editing the case is not the repair. The reach control moves with the scope — from the showcase test to `packages/cli/src/commands/lint.ts`, which is also one of the files that carried the dead name until it was repaired, so it still proves the scan leaves its home package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk --------- Co-authored-by: Claude <trymqms@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 06017ed commit 29db3cd

6 files changed

Lines changed: 625 additions & 15 deletions

File tree

packages/objectql/src/action-governance.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ export function isObjectLessActionKey(objectName: string | undefined | null): bo
7474
* Standalone `action` metadata declares `objectName` (spec `ActionSchema`);
7575
* bundle collectors attach `object`; an object-less action owns the canonical
7676
* `'global'` key. Three other writers spelled this same three-line ladder —
77-
* the runtime's `standaloneActionObjectName`, the ObjectQL plugin's private
78-
* `actionObjectKey`, and an inline copy inside
77+
* the runtime's `standaloneActionObjectName`, a private owner-key method on the
78+
* ObjectQL plugin, and an inline copy inside
7979
* {@link collectEngineActionDeclarations}. All of them resolve HERE now: the
8080
* plugin calls this function directly (same package) and
8181
* `@objectstack/runtime` re-exports it, keeping `standaloneActionObjectName`

packages/objectql/src/action-owner-key-single-source.test.ts

Lines changed: 291 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@
1212
* silently different the first time that constant moves.
1313
*
1414
* `@objectstack/runtime` carries the matching weld for its own copy
15-
* (`action-owner-key-single-source.test.ts` there). This one is scoped to this
16-
* package's source so it stays a package-local test input.
15+
* (`action-owner-key-single-source.test.ts` there). The LADDER halves below are
16+
* scoped to this package's source so they stay package-local test inputs; the
17+
* absence half is not, and the section on it explains why.
1718
*/
1819

19-
import { readFileSync, readdirSync } from 'node:fs';
20-
import { dirname, join } from 'node:path';
20+
import { execFileSync } from 'node:child_process';
21+
import { existsSync, readFileSync, readdirSync } from 'node:fs';
22+
import { dirname, join, resolve } from 'node:path';
2123
import { describe, it, expect } from 'vitest';
2224
import { GLOBAL_ACTION_OBJECT_KEY, standaloneActionOwnerKey } from './action-governance.js';
2325

@@ -66,12 +68,14 @@ describe('standalone-action owner key — one spelling in @objectstack/objectql
6668
}
6769
});
6870

69-
it('leaves no private `actionObjectKey` behind on the plugin', () => {
71+
it('derives the plugin owner key through the canonical helper', () => {
7072
const plugin = nonTestSources().find((s) => s.file === 'plugin.ts');
7173
expect(plugin, 'plugin.ts is missing from the scan').toBeDefined();
72-
expect(plugin!.text).not.toContain('actionObjectKey');
73-
// Positive control for the negative above: the plugin does still derive
74-
// owner keys — it just does it through the canonical helper now.
74+
// The negative that used to live here — "plugin.ts does not name the
75+
// deleted member" — moved to the TREE-scoped section at the bottom of
76+
// this file (#14878). Its scope was the defect, not its subject. What
77+
// stays here is the positive half: the plugin still derives owner keys,
78+
// it just does it through the canonical helper now.
7579
expect(plugin!.text).toContain('standaloneActionOwnerKey(');
7680
});
7781

@@ -89,3 +93,282 @@ describe('standalone-action owner key — one spelling in @objectstack/objectql
8993
expect(body[1]).not.toContain("'global'");
9094
});
9195
});
96+
97+
/**
98+
* ── [#14878] The absence assertion is TREE-scoped, not FILE-scoped ──────────
99+
*
100+
* The negative that used to sit in the plugin test above read `plugin.ts` and
101+
* nothing else, and THAT SCOPE was the defect. A pin written by the deleting PR
102+
* can only look where its author thought to look, and the whole failure mode is
103+
* references the author did not know about: the file-scoped pin stayed green
104+
* while five other files in three other packages went on naming the deleted
105+
* member as something that reads a key TODAY, and the one deletion produced two
106+
* separate follow-up cards.
107+
*
108+
* Widening to the tree also covers the half that nothing keyed on the deleting
109+
* diff can ever see. Three of those five references already existed when the
110+
* member died. The other two were written 1 h 41 min AFTER it, by a later PR,
111+
* into a file that was clean at deletion time — so a check that greps the
112+
* deleting PR's own post-image is structurally blind to them. A pin that runs on
113+
* every PR is not: it reddens on the second kind at the moment it is written,
114+
* which is the only moment the person who can classify the mention is present.
115+
*
116+
* ⛔ AN ASSERTION OF ABSENCE IS NOT A STALE MENTION. This file and its twin in
117+
* `@objectstack/runtime` name the dead member because naming it is how they hunt
118+
* for it. "Repairing" those lines deletes the guard — a naive fixer turning a
119+
* pin into its own removal. That is why the two pins exclude themselves below,
120+
* with the reason written beside the rule; it is the first thing to get right
121+
* about this shape, not a refinement of it.
122+
*
123+
* ── Scope, and where it stops ───────────────────────────────────────────────
124+
*
125+
* `.ts` under `packages/`, and that boundary is a MEASURED TRADE rather than a
126+
* default — read this before widening it.
127+
*
128+
* `examples/` was in the scan for one commit. It is the right radius on the
129+
* evidence (one of the five surviving references lived there), and the repo's
130+
* gate farm refused it: declaring an examples-wide `.ts` glob in
131+
* `scripts/cross-package-test-inputs.mjs` makes that glob an inherited watch
132+
* hint on every importer of that table, `check:cross-package-test-inputs`
133+
* included — and `dispatch-gates.mjs`'s self-test pins that no hint of that gate
134+
* reaches a test file outside `packages/**`, because the whole reason it is
135+
* listed as a change-KIND rather than a path derivation is that the hint route
136+
* cannot reach the population it judges. Measured on this tree: all 41 tracked
137+
* test files outside `packages/` are under `examples/`, so that one glob does
138+
* not shrink the residue class, it EMPTIES it, and the case cannot be
139+
* re-pointed at another member because there is none.
140+
*
141+
* ⇒ Widening this pin to `examples/` is not a two-line change and ⛔ must not be
142+
* done by editing that self-test case. It needs the residue measurement behind
143+
* that case redone, which is a `scripts/pm/` decision owned by another lane.
144+
* What it costs today, stated rather than discovered later: of this symbol's
145+
* five surviving references, four were `packages/**` and one was a test under
146+
* the showcase example — which this pin would not have caught.
147+
*
148+
* ⚠️ Any widening — `examples/`, `docs/`, `content/`, `skills/`, `apps/` — is
149+
* TWO edits, never one: `SCANNED_ROOTS` here AND this package's globs in
150+
* `scripts/cross-package-test-inputs.mjs` (a NEW top-level root needs a matching
151+
* ci.yml `crosspkg:` entry too, which `check-ci-filter-parity.mjs` gates).
152+
* Widening the scanner alone reads as coverage while turbo never re-runs this
153+
* suite for the files it now claims to judge.
154+
*/
155+
156+
/**
157+
* The member PR #14667 deleted from `ObjectQLPlugin`. Held as DATA: naming a
158+
* symbol in a string cannot resurrect it, and this file is excluded from its own
159+
* scan precisely so it may carry the name.
160+
*/
161+
const DELETED_PLUGIN_MEMBER = 'actionObjectKey';
162+
163+
/**
164+
* The live spelling that replaced it. Used as the scan's reach control below —
165+
* it is the one symbol guaranteed to sit in both scanned roots for as long as
166+
* the convergence holds, and if it ever stops doing so this pin should say so
167+
* loudly rather than quietly stop reaching.
168+
*/
169+
const CANONICAL_HELPER = 'standaloneActionOwnerKey';
170+
171+
/** The two pins that hunt the dead member, and therefore have to name it. */
172+
const PIN_FILES: readonly string[] = [
173+
'packages/objectql/src/action-owner-key-single-source.test.ts',
174+
'packages/runtime/src/action-owner-key-single-source.test.ts',
175+
];
176+
177+
/**
178+
* Where a mention of the dead member is NOT a defect, each with its reason
179+
* beside it.
180+
*
181+
* ⛔ This is a scan-SCOPE decision written where the scan lives, and it must
182+
* stay that: an allowlist FILE — one more path pasted in whenever a report is
183+
* inconvenient — is the permission slip this whole shape exists to avoid. A rule
184+
* here has to be a statement about a CLASS of file that is true by construction,
185+
* never "this one site is fine".
186+
*
187+
* The first two rules cannot fire while `SCANNED_EXTENSION` is `.ts`, and they
188+
* are kept anyway: they are the ruled exclusions, and the day someone widens the
189+
* extension set they are what stops the release record from being re-admitted as
190+
* a pile of false reds.
191+
*/
192+
const NOT_A_STALE_MENTION: ReadonlyArray<{ readonly covers: (file: string) => boolean; readonly why: string }> = [
193+
{
194+
// A published CHANGELOG entry is the record OF the removal. It is true in
195+
// the past tense, it is what a consumer reads to find out the member is
196+
// gone, and rewriting it would falsify shipped release history.
197+
covers: (file) => file === 'CHANGELOG.md' || file.endsWith('/CHANGELOG.md'),
198+
why: 'a published CHANGELOG is the record of the removal itself',
199+
},
200+
{
201+
// The same record before the release process compiles it into the above.
202+
covers: (file) => file.startsWith('.changeset/'),
203+
why: 'a changeset is that record before it is compiled into a CHANGELOG',
204+
},
205+
{
206+
// The pins carry the name as their own search string and as accurate
207+
// history of what they pin. Excluding them is what lets the pin exist:
208+
// a scan that flagged its own needle would have no green state at all.
209+
covers: (file) => PIN_FILES.includes(file),
210+
why: 'the pin carries the name as its own search string — repairing it deletes the guard',
211+
},
212+
];
213+
214+
/**
215+
* This package is CJS-typed (no `"type": "module"`), so `module: NodeNext`
216+
* forbids `import.meta` here — the same constraint `srcDir()` above records.
217+
* Walk up from the CWD to this package's own manifest instead, which works
218+
* wherever vitest is invoked from.
219+
*/
220+
function findUp(marker: (dir: string) => boolean, what: string): string {
221+
let dir = process.cwd();
222+
for (;;) {
223+
if (marker(dir)) return dir;
224+
const parent = dirname(dir);
225+
if (parent === dir) throw new Error(`could not locate ${what} walking up from ${process.cwd()}`);
226+
dir = parent;
227+
}
228+
}
229+
230+
const PACKAGE_ROOT = findUp((dir) => {
231+
const manifest = join(dir, 'package.json');
232+
if (!existsSync(manifest)) return false;
233+
const { name } = JSON.parse(readFileSync(manifest, 'utf8')) as { name?: string };
234+
return name === '@objectstack/objectql';
235+
}, 'the @objectstack/objectql package root');
236+
237+
/**
238+
* The repo root by ARITHMETIC from this package rather than by a second
239+
* marker-file walk, deliberately: a walk keyed on a workspace-root marker would
240+
* NAME that root file, and a declared root-level path is a new top-level root
241+
* that ci.yml's `crosspkg:` filter would then have to carry. Anchoring off the
242+
* manifest keeps this pin's declared radius inside roots that already exist.
243+
*
244+
* The arithmetic is not trusted on faith — the reach test below fails on any
245+
* wrong root, because no wrong root can see both scanned trees.
246+
*/
247+
const REPO_ROOT = resolve(PACKAGE_ROOT, '../..');
248+
249+
/** The tree this pin binds. See the scope note above before changing it. */
250+
const SCANNED_ROOTS: readonly string[] = ['packages'];
251+
252+
/** Spelled once so the declared glob and the scan stay in correspondence. */
253+
const SCANNED_EXTENSION = '.ts';
254+
255+
/**
256+
* Generous on purpose. The scan is one `git grep` and a handful of file reads —
257+
* tens of milliseconds — so this is not a budget, it is headroom against a
258+
* merge-queue runner doing a full monorepo build at the same time. A pin that
259+
* times out before its assertion runs reports nothing, and reporting nothing is
260+
* indistinguishable from finding nothing.
261+
*/
262+
const SCAN_TIMEOUT_MS = 60_000;
263+
264+
function git(args: string[]): string[] {
265+
let stdout: string;
266+
try {
267+
stdout = execFileSync('git', args, { cwd: REPO_ROOT, encoding: 'utf8', maxBuffer: 1 << 28 });
268+
} catch (error) {
269+
const failure = error as { status?: number; stderr?: string };
270+
// `git grep` exits 1 for "found nothing", which is data. Anything else is
271+
// a BROKEN scan and must never read as "no stale mentions" — throwing
272+
// here, plus the reach test below, is what keeps a green result meaning
273+
// "looked and found nothing" rather than "never looked".
274+
if (failure.status === 1) return [];
275+
throw new Error(
276+
`git ${args.join(' ')} failed with status ${String(failure.status)}: ${failure.stderr ?? ''}`,
277+
);
278+
}
279+
return stdout.split('\0').filter((entry) => entry.length > 0);
280+
}
281+
282+
/**
283+
* Every scanned file that so much as mentions `symbol`.
284+
*
285+
* Tracked files PLUS untracked ones with ignored paths excluded (`--untracked`)
286+
* — i.e. exactly the files a human authored, never build output. A file written
287+
* but not yet `git add`ed still reddens, which is what makes this a local-loop
288+
* guard rather than something you find out about in the merge queue.
289+
*/
290+
function filesMentioning(symbol: string): string[] {
291+
return git([
292+
'grep',
293+
'--files-with-matches',
294+
'-z',
295+
'--untracked',
296+
'--text',
297+
'--fixed-strings',
298+
'-e',
299+
symbol,
300+
'--',
301+
...SCANNED_ROOTS,
302+
]).filter((file) => file.endsWith(SCANNED_EXTENSION));
303+
}
304+
305+
/** `<file>:<line>` for every mention that no rule above excuses. */
306+
function staleMentionSites(symbol: string): string[] {
307+
const sites: string[] = [];
308+
for (const file of filesMentioning(symbol)) {
309+
if (NOT_A_STALE_MENTION.some((rule) => rule.covers(file))) continue;
310+
const lines = readFileSync(join(REPO_ROOT, file), 'utf8').split('\n');
311+
lines.forEach((text, index) => {
312+
if (text.includes(symbol)) sites.push(`${file}:${index + 1}`);
313+
});
314+
}
315+
return sites;
316+
}
317+
318+
describe('standalone-action owner key — the deleted member is dead TREE-WIDE (#14878)', () => {
319+
it(
320+
'is named nowhere outside the release record and the two pins',
321+
() => {
322+
const sites = staleMentionSites(DELETED_PLUGIN_MEMBER);
323+
expect(
324+
sites,
325+
sites.length === 0
326+
? ''
327+
: [
328+
`These files name \`${DELETED_PLUGIN_MEMBER}\`, a private \`ObjectQLPlugin\``,
329+
'member that was DELETED when the standalone-action owner-key ladder was',
330+
'converged onto one spelling:',
331+
'',
332+
...sites.map((site) => ` - ${site}`),
333+
'',
334+
`The live spelling is \`${CANONICAL_HELPER}\`, exported from`,
335+
'`@objectstack/objectql` (packages/objectql/src/action-governance.ts). If the',
336+
'sentence is otherwise accurate, rename the one word rather than rewriting',
337+
'the clause — the neighbouring names in these sentences are usually alive.',
338+
'',
339+
'⛔ Before you touch a site, decide which of three it is:',
340+
' (a) a LIVE CLAIM that the member exists -> fix it',
341+
' (b) accurate HISTORY naming it in the past -> reword so it no longer',
342+
' carries the dead name, or add a rule to NOT_A_STALE_MENTION above',
343+
' with the reason beside it — never an allowlist file',
344+
' (c) an ASSERTION THAT IT IS GONE -> ⛔ leave it alone. It is the guard.',
345+
].join('\n'),
346+
).toEqual([]);
347+
},
348+
SCAN_TIMEOUT_MS,
349+
);
350+
351+
it(
352+
'the scan reaches both roots and can see the name it hunts',
353+
() => {
354+
// Anti-vacuity, at both stages a tree scan can go silently blind.
355+
//
356+
// A grep that matched nothing — wrong repo root, git missing, a
357+
// pathspec that names no tree — yields the same empty violation set
358+
// as a clean repo, and the assertion above cannot tell them apart.
359+
// That is the exact property the file-scoped pin lost.
360+
expect(filesMentioning(DELETED_PLUGIN_MEMBER)).toContain(PIN_FILES[0]);
361+
362+
// ...and it must LEAVE this package, which is the half a file-scoped
363+
// pin never had. The live helper is the reach control: it is the one
364+
// symbol the convergence guarantees outside this package, and the CLI
365+
// site below is one of the files that carried the DEAD name until it
366+
// was repaired — so a scan that cannot see it is a scan that would
367+
// not have caught the defect this pin exists for.
368+
const reached = filesMentioning(CANONICAL_HELPER);
369+
expect(reached).toContain('packages/cli/src/commands/lint.ts');
370+
expect(reached).toContain('packages/runtime/src/action-execution.ts');
371+
},
372+
SCAN_TIMEOUT_MS,
373+
);
374+
});

0 commit comments

Comments
 (0)