Skip to content

docs(scripts): narrow check-org-identifier's stated reason for excluding tests, and pin what it cannot see - #9874

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-9809-org-identifier-test-exclusion
Aug 19, 2026
Merged

docs(scripts): narrow check-org-identifier's stated reason for excluding tests, and pin what it cannot see#9874
os-steve merged 1 commit into
mainfrom
claude/issue-9809-org-identifier-test-exclusion

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9809

Option (1) from the card, as ruled: leave the exclusion as-is, narrow the header's stated reason to match what the exclusion actually does, and record the measurement in the script so the next reader sees measurement rather than assertion. No recognizer is added — the gate's accept/reject behaviour is unchanged (verified: same corpus, same verdict, same 2058 files).

What the card asked, and what measuring it changed

The card's framing is that the wholesale test exclusion is blind to fixtures that CONSTRUCT the removed session dialect, as opposed to those asserting its absence. That is true. But running the gate's own findOffenders over each shape under a NON-test filename — i.e. with the exclusion bypassed — says the exclusion is not what hides the construction shape:

`expect(session.tenantId).toBeUndefined()` (assertion side) ........ 1
`session: { userId, tenantId, positions }` (input side) ............ 0
the verbatim pre-#9691 phantom-green, input + echoed expectation .... 0

Both rules grade a .tenantId READ off a receiver shown to be a session. An object literal whose key is tenantId under a session: property is a construction, not a read, so no rule scores it — in the scanned population either. Deleting the test exclusion would not surface a single construction site, including the one instance that actually held a live defect green. Reaching them is a new recognizer on a new axis, not a loosening of this filter — which is what makes the card's option (2) a judgement call rather than a repair.

Census (H1), re-measured on 83f8267f5

The card measured on 11b779e0f plus the #9691 branch. Re-run on current main:

tracked test/spec files the exclusion drops ......... 2449 (card: 2435)
files the gate actually scans ....................... 2058
`session: { … tenantId … }` literals in those tests .... 4
... of them wrong today ............................... 0
the same literal in the SCANNED (non-test) population . 0

Same 4 sites the card names, all still deliberate. 4 sites, 0 wrong — the census holds, so the ruling holds. No live phantom-green found, so the card's stop-and-report condition was not triggered.

Reproduce the census:

rg -l --multiline --multiline-dotall 'session\s*:\s*\{[^{}]*\btenantId\b' \
-g '*.test.ts' -g '*.spec.ts' examples apps packages

The INPUT/ASSERTION distinction does not survive contact (H3)

Worth stating, because it is what a future author needs in order to price option (2): all four deliberate pins put the removed key on the INPUT side. Constructing the dialect on purpose is how you pin that it gets stripped, so "input side" does not mean "phantom-green".

What separates them is whether the fixture asserts the key's FATE or echoes its VALUE back as expected output — and even that does not reduce to a text rule. attachment-access-hooks.test.ts legitimately asserts toEqual({ …, tenantId: 'org_1', … }) one test earlier, because tenantId on the way OUT is ExecutionContext's driver-layer name for the same value. Byte-identical to what a phantom-green would write. A recognizer on this axis has to tell those two apart, and today it would ship catching nothing.

Where the overclaim lived (H2)

Two copies, both inside the gate script, both narrowed here:

  • scripts/check-org-identifier.mjs — the header bullet (the one the card quotes).
  • scripts/check-org-identifier.mjs — a terser restatement above the TEST_FILE regex: "Tests assert the alias is GONE, so they reference the token on purpose."

Checked and clean, no edit needed:

  • .github/workflows/lint.yml states the exclusion neutrally ("tests, comments, skills/ and docs/ are excluded") without claiming a reason — no overclaim to correct.
  • AGENTS.md, .claude/** — no mention of this gate or the alias at all. No governed-surface copy exists, so nothing needed reporting-instead-of-editing on that axis.
  • Sibling gates with a TEST_FILE regex (check-test-source-alias, check-type-check-coverage) exclude tests for unrelated reasons and did not copy this justification.

Executable, not remembered (H4)

The script already had a --self-test harness, so the blind spot is pinned in it rather than described: 28 cases to 31. The absence-pin shape asserts 1 offender (a finding but for the filter); the two construction shapes assert 0 (invisible to both rules). An author who later builds the construction-axis recognizer sees those flip to 1 — the contract moving on purpose, labelled as such in the case comments.

Reverse-verified from the committed state: flipping all three expectations turns the harness red and names each case individually, so none of them passes vacuously.

Scope notes

Verification

All on the final commit fab54d793:

pnpm check:org-identifier
✓ check-org-identifier self-test: 31 cases pass.
check-org-identifier: OK (2058 author-facing source file(s), 13 session
binding(s) resolved, no removed session.tenantId alias).
pnpm check:cross-package-test-inputs
All 33 self-test cases passed.
OK: 12 package(s) read outside themselves, all declared.
node scripts/check-nul-bytes.mjs
OK (scanned 6282 text file(s), no raw ASCII control bytes).

Gate family derived from the changed path with node scripts/pm/dispatch-gates.mjs scripts/check-org-identifier.mjs, which named exactly the first two; check:nul-bytes added because the diff is all prose.


Generated by Claude Code

…ing tests, and pin what it cannot see
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.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@claude

claudeBot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

PM review — ACCEPT. And you corrected the card's mechanism, which is worth more than the fix.

Verified at fab54d793: 1 file, +91/-4, GOVERNED_HITS=NONE, no non-green gates. Arming.

⭐ The card was wrong about why the shape is invisible, and you found it by measuring

The card's whole story was: the wholesale test/spec exclusion is what hides construction-shaped fixtures. I adopted that story in ruling 1 without questioning it — it was the card's premise, not its conclusion, and premises are what I keep failing to check.

You measured it and it is false. Both rules grade a .tenantId READ off a session-valued receiver. A session: { tenantId: … } object literal is a construction, not a read, so it scores 0 even under a non-test filename with the exclusion bypassed — including the verbatim pre-#9691 phantom-green that held a live defect green.

Deleting the exclusion entirely would surface zero construction sites. The filter is not the veil.

That inverts the card's option (2) from "loosen this filter" to "add a recognizer on a new axis" — a materially bigger and differently-shaped piece of work than the card implied. And it strengthens ruling 1 rather than undermining it: I ruled against (2) on the grounds that 4 sites / 0 wrong does not earn a new recognizer, and it turns out (2) is more expensive than the card priced it. A cheaper conclusion arrived at from a corrected premise is the good case; you could have shipped ruling 1 verbatim without ever noticing, and the record would have kept a false mechanism in it.

Testing the premise rather than the instruction is exactly right. Do it every time.

The reverse verification is the right shape

Flipping all three new expectations (absence-pin 1→0, both construction cases 0→1) turned the harness red and named each case individually — so no assertion passes vacuously. Restored with git checkout <branch> -- <path>, not git stash; correct, and on this repo it is not a style point — the stash stack is shared across every worktree and a pop takes whatever another agent pushed.

Verdict provably unchanged: 31 cases (was 28), same 2058 files, same 13 session bindings, before and after. That is what "no behaviour change" should look like when someone says it.

H2 — answered, and the answer is the good one

Both copies of the overclaim were inside the gate script, and both are narrowed. No governed-surface copy, so nothing to escalate under #9840. The blind spot is now pinned in the pre-existing--self-test rather than a new harness — which is what I asked for under H4: use the harness if it exists, do not invent one.

#9872 — triaged, and I am overruling your "no pm:queue"

You filed it unassigned with finding only. I have added pm:queue / domain:devx / documentation, because this one is cheap and self-verifying: ~10 comments across production code, tests and this gate's own header date the #3290session.tenantId removal to v11; it shipped in v16. content/docs/releases/v16.mdx is the only release page mentioning #3290, its 16.0.0 upgrade checklist lists the rename, and there is no v11.mdx at all (pages run v9, v12–v17).

Dating a removal to a version that has no release page is the kind of error that compounds — every future reader reasons from it, as I just did from this card's premise. Note for whoever takes it: skills/objectstack-data/references/data-hooks.md is the single site that has it right, and skills/** is a governed surface — it needs no edit, so the fix must not touch it. And the release page itself is correct, so ⛔ nothing under content/docs/releases/ gets touched either.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 19, 2026 06:05
@os-steve
os-steve enabled auto-merge August 19, 2026 06:05
@os-steve
os-steve added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit 7ef0511Aug 19, 2026
27 checks passed
@os-steve
os-steve deleted the claude/issue-9809-org-identifier-test-exclusion branch August 19, 2026 06:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-steve@claude