Skip to content

Commit 7ef0511

Browse files
os-steveclaude
andauthored
docs(scripts): narrow check-org-identifier's stated reason for excluding tests, and pin what it cannot see (#9874)
The header justified the wholesale test/spec exclusion as "tests legitimately reference the removed token to assert its ABSENCE". That describes one population and is silent about a second: a fixture can also CONSTRUCT the removed session dialect as INPUT, which is the shape that can hold a production defect green. Measured on 83f8267: the exclusion is not what hides the construction shape. Both rules grade a `.tenantId` READ off a session-valued receiver, and a `session: { tenantId: … }` object literal is a construction, not a read, so it scores zero with the exclusion bypassed. Deleting the filter would not surface a single construction site; reaching them is a new recognizer on a new axis. Census over the 2449 excluded test files: 4 such literals, 0 wrong today, and 0 in the 2058-file scanned population. Not built, per that census. Both shapes are now pinned in --self-test (28 -> 31 cases) so the blind spot is executable rather than remembered. Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja Co-authored-by: Claude <noreply@anthropic.com>
1 parent f9d7acf commit 7ef0511

1 file changed

Lines changed: 91 additions & 4 deletions

File tree

scripts/check-org-identifier.mjs

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,15 @@
2929
// database-per-tenant kernels) do not trip it. For the rare genuine
3030
// driver-layer `session.tenantId`, add an `os-allow-tenant-id` comment on
3131
// the same line.
32-
// • Test/spec files are EXCLUDED: they legitimately reference the removed
33-
// token to assert its ABSENCE (`expect(session.tenantId).toBeUndefined()`),
34-
// and are not reference bodies an author copies a hook from.
32+
// • Test/spec files are EXCLUDED, and the exclusion is WHOLESALE: it drops
33+
// the FILE, not a shape inside it. The population it is written for is
34+
// real -- a body naming the removed token to assert its ABSENCE
35+
// (`expect(session.tenantId).toBeUndefined()`) is not a reference body an
36+
// author copies a hook from. But "asserts its absence" is narrower than
37+
// the exclusion, which also drops fixtures that CONSTRUCT the removed
38+
// dialect as INPUT. That second population is not a reference body either,
39+
// and it is invisible to the DETECTOR as well -- so this filter is not what
40+
// hides it. Measured under "What the test exclusion covers" below (#9809).
3541
// • Comments are SKIPPED -- a migration note that NAMES the removed alias to
3642
// explain its removal is documentation, not an executable read. Which spans
3743
// ARE comments is decided by the ONE shared string-, template- and regex-
@@ -152,9 +158,75 @@
152158
// • The BINDING rule reads code, not strings: an aliased read taught inside a
153159
// template literal is seen by neither rule (the TEXT rule catches only the
154160
// literal spelling there).
161+
// • A `session` OBJECT LITERAL is not a read, so no rule scores it. Both
162+
// rules resolve a session-valued RECEIVER and grade a `.tenantId` READ off
163+
// it; `session: { tenantId: … }` CONSTRUCTS the removed dialect instead and
164+
// scores zero in test and non-test files alike (#9809, below).
155165
// These are the shapes to widen to if one ever goes live. They are named here
156166
// so a future green is read as "clean where this gate can see", never as proof.
157167
//
168+
// ## What the test exclusion covers, and what it does not (#9809)
169+
//
170+
// The bullet above used to justify the exclusion as "tests assert the alias is
171+
// GONE". That is true of one population and silent about a second: a fixture
172+
// can also CONSTRUCT the removed dialect, handing the code under test a
173+
// `session: { …, tenantId: … }` literal that `HookContextSchema` strips and
174+
// `ObjectQLEngine.buildSession` never emits. The second shape is the one that
175+
// can hold a production defect green, because the fixture supplies the very key
176+
// production cannot -- which is how the pre-#9691 attachment fixture kept
177+
// `callerContext()` reading a dead name for two majors.
178+
//
179+
// ⚠️ But the exclusion is NOT what hides that shape, and this is the correction
180+
// worth carrying. Both rules grade a `.tenantId` READ off a receiver shown to
181+
// be a session. An object literal whose KEY is `tenantId` under a `session:`
182+
// property is a CONSTRUCTION, not a read, so no rule scores it -- and it scores
183+
// zero in the scanned population too. Measured by running `findOffenders` over
184+
// each shape under a NON-test filename, i.e. with the exclusion bypassed:
185+
//
186+
// `expect(session.tenantId).toBeUndefined()` (assertion side) ........ 1
187+
// `session: { userId, tenantId, positions }` (input side) ............ 0
188+
// the verbatim pre-#9691 phantom-green, input + echoed expectation .... 0
189+
//
190+
// So deleting the test exclusion would not surface a single construction site.
191+
// Reaching them is a NEW recognizer on a new axis (literal construction), not a
192+
// loosening of this filter -- which is what makes it a judgement call rather
193+
// than a repair, and the census says it is not earned yet.
194+
//
195+
// CENSUS on 83f8267f5 (2026-08-19), over the 2449 test/spec files the exclusion
196+
// drops -- 2058 files remain scanned. Reproduce with:
197+
//
198+
// rg -l --multiline --multiline-dotall 'session\s*:\s*\{[^{}]*\btenantId\b' \
199+
// -g '*.test.ts' -g '*.spec.ts' examples apps packages
200+
//
201+
// `session: { … tenantId … }` literals in EXCLUDED test files ......... 4
202+
// ... of them wrong today ............................................ 0
203+
// the same literal in the SCANNED (non-test) population ............... 0
204+
//
205+
// packages/spec/src/data/hook.test.ts:619 #3290 absence pin
206+
// packages/plugins/plugin-audit/src/audit-writers.test.ts:1666 absence pin
207+
// packages/plugins/plugin-audit/src/comment-access-hooks.test.ts:529 #9691
208+
// packages/services/service-storage/src/attachment-access-hooks.test.ts:750
209+
//
210+
// (The recipe reports 5 matches across those 4 files: `hook.test.ts` carries a
211+
// second one whose `tenantId` sits inside a COMMENT in the literal's body.)
212+
//
213+
// ⛔ The distinguishing signal is NOT "input side vs assertion side". All four
214+
// deliberate pins put the removed key on the INPUT side -- constructing the
215+
// dialect on purpose is HOW you pin that it gets stripped. What separates them
216+
// from a phantom-green is whether the fixture asserts the key's FATE (absent,
217+
// or inert downstream) or echoes its VALUE back as expected output. Even that
218+
// does not reduce to a text rule: `attachment-access-hooks.test.ts` legitimately
219+
// asserts `toEqual({ …, tenantId: 'org_1', … })` one test earlier, because
220+
// `tenantId` on the way OUT is `ExecutionContext`'s driver-layer name for the
221+
// same value -- byte-identical to what a phantom-green would write. A recognizer
222+
// on this axis has to tell those two apart, and today it would ship catching
223+
// nothing: 4 sites, 0 wrong.
224+
//
225+
// Both shapes are pinned in `--self-test`, so this stays executable rather than
226+
// remembered. An author who later builds the construction-axis recognizer will
227+
// see the input-shape cases flip from 0 to 1: that is the contract moving on
228+
// purpose, not a regression.
229+
//
158230
// ## The population invariant -- zero is a broken scan, not a clean repo
159231
//
160232
// The BINDING rule is only as good as the population it discovers, and this
@@ -201,7 +273,10 @@ import { maskComments } from './js-comment-mask.mjs';
201273
const ROOTS = ['examples', 'apps', 'packages'];
202274
const EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.cts', '.mts'];
203275
const EXCLUDED = /(^|\/)(node_modules|dist|build|\.next|\.turbo)\//;
204-
// Tests assert the alias is GONE, so they reference the token on purpose.
276+
// Dropped WHOLESALE: absence pins and fixtures that CONSTRUCT the removed
277+
// dialect alike. Neither is a reference body an author copies -- and the
278+
// construction shape is invisible to both rules anyway, so this filter is not
279+
// what hides it (#9809, header).
205280
const TEST_FILE = /(\.(test|spec)\.[cm]?[jt]sx?$)|((^|\/)__tests__\/)/;
206281

207282
// `ctx.session.tenantId`, `session?.tenantId`, `this.session . tenantId`, … --
@@ -566,6 +641,18 @@ function selfTest() {
566641
'a comment naming an ALIASED read is documentation -- the tree never sees it'],
567642
['function h(ctx) {\n const execCtx = ctx.input.options.context;\n return execCtx.tenantId;\n}', 0,
568643
'NO FALSE RED: the driver-layer envelope is not reached through `.session`'],
644+
645+
// ── #9809: what the test exclusion covers, and what it does not ─────
646+
// These run through the DETECTOR under a non-test filename, so they state
647+
// what the RULES do independently of the TEST_FILE filter. An author who
648+
// builds the construction-axis recognizer flips the two 0s to 1s: that is
649+
// the contract moving on purpose, not a regression.
650+
['expect(session.tenantId).toBeUndefined();', 1,
651+
'ABSENCE PIN: the shape the test exclusion is written for -- a finding but for the filter'],
652+
["await hook({\n object: 'sys_attachment',\n session: { userId: 'u1', tenantId: 'stale_org', positions: ['p1'] },\n});", 0,
653+
'CONSTRUCTION: a `session:` literal spelling the removed key is not a READ, so no rule scores it -- deleting the exclusion would not reach it'],
654+
["await hook({\n session: { userId: 'u1', tenantId: 'org_1' },\n});\nexpect(canEdit.mock.calls[0][2]).toEqual({ userId: 'u1', tenantId: 'org_1' });", 0,
655+
'CONSTRUCTION: the verbatim pre-#9691 phantom-green -- input literal plus echoed expectation, invisible to both rules'],
569656
];
570657

571658
let failed = 0;

0 commit comments

Comments
 (0)