Skip to content

[finding] check-org-identifier excludes test files wholesale, so a fixture that CONSTRUCTS the removed session dialect reads the same as one asserting its absence #9809

Description

@os-steve

Found while implementing #9691 (the receiver-name anchor). Reported, not fixed — #9691's scope is the receiver anchor, and this is a separate axis of the same gate. Unassigned. Not a claim.

Measured on 11b779e0f plus the #9691 branch.

The mechanism

scripts/check-org-identifier.mjs drops every test file before scanning:

constTEST_FILE=/(\.(test|spec)\.[cm]?[jt]sx?$)|((^|\/)__tests__\/)/;

with the header's reason:

Test/spec files are EXCLUDED: they legitimately reference the removed token to assert its ABSENCE (expect(session.tenantId).toBeUndefined()), and are not reference bodies an author copies a hook from.

That reason is correct about one population and blind to another. A test can name the removed alias in two structurally different ways:

  • asserting its absence — the case the exclusion is written for, and legitimate;
  • CONSTRUCTING it as an input — handing a hook a session object literal that spells tenantId, a dialect HookContextSchema strips and ObjectQLEngine.buildSession never emits.

The second is not a reference body an author copies, so the header's stated reason does not cover it — but it is the shape that can hold a production defect green, because the fixture supplies the very key production cannot.

The instance that made this concrete

packages/services/service-storage/src/attachment-access-hooks.test.ts covered the session-fallback path of callerContext() like this (pre-#9691):

session: {userId: 'u1',tenantId: 'org_1',positions: ['p1']},
...
expect(canEdit.mock.calls[0][2]).toEqual({userId: 'u1',tenantId: 'org_1',positions: ['p1']});

The production code under it read s.tenantId off the session — always undefined in reality, so the attachment kit forwarded an envelope with no organization to ISharingService.canEdit. The fixture passed for exactly as long as the code was wrong, and could only have failed if the code became right. It went red the moment #9691 fixed the source.

So the fixture was not merely uncovered by the gate; it was the mechanism keeping a live defect green, in a file the gate is written to skip.

Census

Across the 2435 tracked test files under examples/apps/packages/, fixtures that build a session: object literal carrying a tenantId key:

packages/spec/src/data/hook.test.ts:619 deliberate #3290 absence pin
packages/plugins/plugin-audit/src/audit-writers.test.ts:1666 deliberate absence pin (PR #9692)
packages/plugins/plugin-audit/src/comment-access-hooks.test.ts deliberate absence pin (#9691)
packages/services/service-storage/src/attachment-access-hooks.test.ts deliberate absence pin (#9691)

4 sites, 0 of them wrong today — the two #9691 ones were rewritten in that PR, and the other two were always deliberate. So this is LATENT, not live. It is filed because the population it is blind to is the population that produced the only measured instance, and because the exclusion's stated reason is narrower than the exclusion itself.

What a fix would have to decide (not decided here)

The distinguishing signal is available structurally: an absence pin puts the removed key on the ASSERTION side (toBeUndefined, not.toHaveProperty), a phantom-green fixture puts it on the INPUT side (a session: property of a context literal handed to the code under test). Whether that distinction is worth a rule, a lint, or nothing at all is a judgement, and the honest options differ in cost:

  1. leave it, and narrow the header's stated reason to match what the exclusion actually does;
  2. scan test files for the INPUT shape only (a session object literal carrying tenantId), which is a new recognizer with its own blind spots;
  3. treat it as an instance of Seven gates report "clean" when they mean "I saw nothing I understood" — make unrecognised a verdict distinct from pass #9747 and let the family-level answer cover it.

My weak preference is (1) plus recording the measurement, on the grounds that 4 sites with 0 wrong does not obviously earn a new recognizer, and #9747 is already the place where "my recognizer is narrower than the code" is being decided as a family.

Related

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions