Skip to content

Commit ed6f910

Browse files
committed
fix(scripts): check-org-identifier takes the shared string-aware comment mask
The gate decided "comment or code?" per line, with a `trimmed.startsWith` triple and `line.replace(/\/\/.*$/, '')`. That truncates at the FIRST doubled slash on the line whatever it is, so a URL or any slash-bearing string literal deleted the rest of its own line -- including the very `session.tenantId` read the gate exists to catch. Silent under-reporting: it printed OK over a line it had truncated. It was wrong in the mirror direction too, which the card did not name: only a line STARTING with `*`, `//` or `/*` counted as a comment, so an interior line of a block comment and a trailing `/* … */` on a code line both read as live code and would have been reported as findings. Both directions go away with `maskComments` from scripts/js-comment-mask.mjs (#9367), which is string-, template- and regex-aware and blanks in place, so the reported line number stays true. The waiver marker keeps being read from the RAW line: it lives in a comment, and testing the masked line for it would silently revoke every waiver in the tree. Measured on 2051 author-facing files: the two projections disagree on the text of 271 files, the gate's verdict on 0 -- all 10 corpus lines naming the alias are comments, so the defect is LATENT. Structurally so: this is a zero-occurrence hard-fail guard, so a corpus holding the hazard is a corpus where the gate is already red. The near-miss half is everywhere -- 665 lines in 181 files carry a doubled slash inside a literal. The 14 shapes are pinned in a new `--self-test`, wired into `check:org-identifier` the way every other gate in this tree wires its own. Fixes #9444 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
1 parent af2a989 commit ed6f910

2 files changed

Lines changed: 173 additions & 49 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
"check:adr-links": "node scripts/check-adr-links.mjs --self-test && node scripts/check-adr-links.mjs",
5858
"check:adr-merge-approval": "node scripts/check-adr-merge-approval.mjs --self-test && node scripts/check-adr-merge-approval.mjs",
5959
"check:platform-checklist": "node scripts/checklist-select.mjs --self-test && node scripts/check-platform-checklist.mjs",
60-
"check:org-identifier": "node scripts/check-org-identifier.mjs",
60+
"check:org-identifier": "node scripts/check-org-identifier.mjs --self-test && node scripts/check-org-identifier.mjs",
6161
"check:authz-resolver": "node scripts/check-single-authz-resolver.mjs --self-test && node scripts/check-single-authz-resolver.mjs",
6262
"check:slot-lookup": "node scripts/check-slot-lookup-ratchet.mjs",
6363
"check:query-options-erasure": "node scripts/check-query-options-erasure-ratchet.mjs --self-test && node scripts/check-query-options-erasure-ratchet.mjs",

‎scripts/check-org-identifier.mjs‎

Lines changed: 172 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
// (#3290). The framework's own hook/action surface no longer emits or reads
2020
// `session.tenantId` (engine `buildSession`, the record-change trigger, and
2121
// the ObjectQL audit-stamp plugin were migrated to `organizationId`), so
22-
// packages/ is now held to the same bar as reference apps — an author or AI
22+
// packages/ is now held to the same bar as reference apps -- an author or AI
2323
// copying a package example body will not find the removed name.
2424
// • The generic DRIVER-LAYER tenancy knob is untouched and never matched: the
2525
// pattern anchors on the `session` receiver, so `execCtx.tenantId` /
@@ -30,84 +30,205 @@
3030
// • Test/spec files are EXCLUDED: they legitimately reference the removed
3131
// token to assert its ABSENCE (`expect(session.tenantId).toBeUndefined()`),
3232
// and are not reference bodies an author copies a hook from.
33-
// • Comment lines are SKIPPED (JSDoc `*`, `//`, `/*`, and trailing `// …`): a
34-
// migration note that NAMES the removed alias to explain its removal is
35-
// documentation, not an executable read. Only code is checked.
33+
// • Comments are SKIPPED -- a migration note that NAMES the removed alias to
34+
// explain its removal is documentation, not an executable read. Which spans
35+
// ARE comments is decided by the ONE shared string-, template- and regex-
36+
// aware scanner (`scripts/js-comment-mask.mjs`, #9367), not by this gate.
3637
// • skills/ and content/docs/ are EXCLUDED: prose there may still name the
3738
// removed alias when documenting the migration.
3839
//
3940
// node scripts/check-org-identifier.mjs
41+
// node scripts/check-org-identifier.mjs --self-test
4042
//
4143
// Scope: tracked sources under examples/, apps/, and packages/ (git ls-files).
44+
//
45+
// ## Why the comment split is not this gate's own business (#9444)
46+
//
47+
// This gate used to answer "comment or code?" per line, with a `trimmed`
48+
// `startsWith` triple and then
49+
//
50+
// const code = line.replace(/\/\/.*$/, '');
51+
//
52+
// which truncates at the FIRST doubled slash on the line, whatever that slash
53+
// is. A URL or any slash-bearing string literal therefore deleted the rest of
54+
// its own line -- including the very `session.tenantId` read this gate exists
55+
// to catch. Silent under-reporting: the gate printed OK over a line it had
56+
// truncated, the failure direction AGENTS.md calls worse than no verifier.
57+
//
58+
// It was wrong in the MIRROR direction too, which the card did not name. Only a
59+
// line whose first non-space characters were `*`, `//` or `/*` counted as a
60+
// comment, so an interior line of a block comment (`/*` on one line, prose with
61+
// no leading `*` on the next) and a trailing `/* … */` on a code line both read
62+
// as live code: the gate would have MANUFACTURED a finding out of prose. Both
63+
// directions are gone with one shared mask, and both are pinned in `--self-test`.
64+
//
65+
// ## LIVE or LATENT, measured on 51a46a440's successor (af2a989be)
66+
//
67+
// Corpus: 2051 author-facing source files. Projections (this gate's old strip
68+
// vs `maskComments`) disagree on the text of **271** files, but the gate's
69+
// VERDICT changes on **0** -- every one of the 10 corpus lines that names
70+
// `session.tenantId` today is inside a comment, and old and new agree on all
71+
// ten. So the defect is LATENT here, and structurally so: this is a hard-fail
72+
// ZERO-occurrence guard, so a corpus that contains the hazard is a corpus in
73+
// which the gate is already red. The intersection the card asks for can only
74+
// ever be empty while the gate is green; the measurement that means something
75+
// is the NEAR MISS, and that half is everywhere -- **665 lines across 181
76+
// files** carry a doubled slash inside a string, template or regex literal.
77+
// The day one of them also carries the removed read, the old gate goes quiet.
78+
//
79+
// ## Why `maskComments` and not `stripComments`
80+
//
81+
// This gate reports a FILE and a LINE, so it takes the blanking projection: the
82+
// masked text stays byte-for-byte aligned with the source and line `i` is still
83+
// line `i`. Cost measured over the same 2051 files (best of 3, scan + match
84+
// only): old per-line strip 195ms, `maskComments` 2088ms, `stripComments`
85+
// 1275ms. #9367 measured a 51x cliff (6.4s -> 5m27s) when a LAZY `[\s\S]*?`
86+
// matcher was dragged across the whitespace blanking leaves behind; this gate's
87+
// matcher is a short anchored pattern run per line, so it is not exposed to
88+
// that, and ~2s is the scanner's own linear cost over 29MB.
4289

4390
import { execFileSync } from 'node:child_process';
4491
import { readFileSync } from 'node:fs';
4592
import { join } from 'node:path';
93+
import { maskComments } from './js-comment-mask.mjs';
4694

4795
const ROOTS = ['examples', 'apps', 'packages'];
4896
const EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.cts', '.mts'];
4997
const EXCLUDED = /(^|\/)(node_modules|dist|build|\.next|\.turbo)\//;
5098
// Tests assert the alias is GONE, so they reference the token on purpose.
5199
const TEST_FILE = /(\.(test|spec)\.[cm]?[jt]sx?$)|((^|\/)__tests__\/)/;
52100

53-
// `ctx.session.tenantId`, `session?.tenantId`, `this.session . tenantId`, … —
101+
// `ctx.session.tenantId`, `session?.tenantId`, `this.session . tenantId`, … --
54102
// the `session` receiver immediately before `.tenantId`. Anchored on the
55103
// `session` word so `execCtx.tenantId` / `opts.tenantId` never match.
56104
const PATTERN = /\bsession\s*\??\.\s*tenantId\b/;
57105
const ALLOW_MARKER = 'os-allow-tenant-id';
58106

59-
const root = execFileSync('git', ['rev-parse', '--show-toplevel'], {
60-
encoding: 'utf8',
61-
}).trim();
62-
63-
// Newline-delimited on purpose (not `-z`): tracked paths under these roots
64-
// never contain a newline, and avoiding the NUL delimiter keeps this very
65-
// script free of any raw NUL byte (which would make it invisible to grep — the
66-
// exact #3127 failure mode this repo already guards with check:nul-bytes).
67-
const files = execFileSync('git', ['ls-files', '--', ...ROOTS], {
68-
cwd: root,
69-
encoding: 'utf8',
70-
maxBuffer: 64 * 1024 * 1024,
71-
})
72-
.split('\n')
73-
.filter(Boolean)
74-
.filter((f) => EXTENSIONS.some((ext) => f.endsWith(ext)))
75-
.filter((f) => !EXCLUDED.test(f))
76-
.filter((f) => !TEST_FILE.test(f));
77-
78-
const offenders = [];
79-
for (const file of files) {
80-
const text = readFileSync(join(root, file), 'utf8');
107+
/**
108+
* Every removed-alias read in one source, as `{ file, line, text }`.
109+
*
110+
* The comment/code split comes from `maskComments`, once per file, and the
111+
* masked text is index-aligned with the raw text, so a finding still quotes the
112+
* RAW line the author will open.
113+
*
114+
* The waiver is read from the RAW line ON PURPOSE. It is documented as a
115+
* comment on the offending line, and the mask blanks comments -- testing the
116+
* masked line for it would silently revoke every waiver in the tree, which is
117+
* the sort of change a green run cannot show you.
118+
*/
119+
export function findOffenders(text, file) {
81120
const lines = text.split('\n');
121+
const code = maskComments(text).split('\n');
122+
const offenders = [];
82123
for (let i = 0; i < lines.length; i++) {
83124
const line = lines[i];
84125
if (line.includes(ALLOW_MARKER)) continue;
85-
// Skip comment lines — a migration note that names the removed alias is
86-
// documentation, not an executable read (JSDoc `*`, line/block `//`,`/*`).
87-
const trimmed = line.trimStart();
88-
if (trimmed.startsWith('*') || trimmed.startsWith('//') || trimmed.startsWith('/*')) continue;
89-
// Drop any trailing line-comment so `foo(); // …session.tenantId…` is clean.
90-
const code = line.replace(/\/\/.*$/, '');
91-
if (!PATTERN.test(code)) continue;
126+
if (!PATTERN.test(code[i] ?? '')) continue;
92127
offenders.push({ file, line: i + 1, text: line.trim() });
93128
}
129+
return offenders;
94130
}
95131

96-
if (offenders.length === 0) {
97-
console.log(
98-
`check-org-identifier: OK (${files.length} author-facing source file(s), no removed session.tenantId alias).`,
99-
);
100-
process.exit(0);
101-
}
132+
/**
133+
* The shapes, not the corpus.
134+
*
135+
* A green run over today's tree proves only that today's tree lacks the shape,
136+
* and for a zero-occurrence hard-fail guard it can prove nothing else -- so
137+
* these cases ARE this gate's contract. `BLIND` marks a case the pre-#9444 strip
138+
* got wrong by MISSING a real read; `FABRICATE` marks one it got wrong by
139+
* inventing a finding out of prose.
140+
*/
141+
function selfTest() {
142+
const BT = String.fromCharCode(96); // backtick, kept out of the literals below
143+
const cases = [
144+
// [source, expected offender count, label]
145+
[" const docs = 'https://objectstack.ai/x'; return session.tenantId;", 1,
146+
'BLIND: a URL in a string hid the read behind it (#9444)'],
147+
[' const glob = ' + "'packages//src'" + '; const t = ctx.session.tenantId;', 1,
148+
'BLIND: a bare doubled slash inside a string'],
149+
[" const p = /https:\\/\\//; return session.tenantId;", 1,
150+
'BLIND: a regex literal whose escaped slashes read as a comment opener'],
151+
[' const hint = ' + BT + 'see https://x/y' + BT + '; return session?.tenantId;', 1,
152+
'BLIND: the same shape inside a template literal'],
153+
['/*\n a block comment interior line naming session.tenantId\n*/\nconst ok = 1;', 0,
154+
'FABRICATE: an interior comment line with no leading star'],
155+
['doThing(); /* session.tenantId was removed in v11 */', 0,
156+
'FABRICATE: a trailing block comment on a code line'],
157+
['const org = ctx.user.organizationId; // session.tenantId is gone (#3290)', 0,
158+
'a trailing line comment naming the alias is documentation'],
159+
['// the deprecated session.tenantId alias was removed in v11', 0,
160+
'a whole-line comment is documentation'],
161+
['/**\n * the deprecated session.tenantId alias was removed in v11\n */\nconst ok = 1;', 0,
162+
'a JSDoc continuation line inside a real docblock is documentation'],
163+
['const t = ctx.session.tenantId; // os-allow-tenant-id: driver isolation column', 0,
164+
'the waiver marker still exempts the line'],
165+
['const t = execCtx.tenantId ?? opts.tenantId;', 0,
166+
'the driver-layer tenancy knob is never matched'],
167+
['const t = this.session . tenantId;', 1,
168+
'whitespace between the receiver and the property'],
169+
['/** doc naming session.tenantId */\nreturn session.tenantId;', 1,
170+
'a docblock does not swallow the code line under it'],
171+
['const sample = ' + BT + 'ctx.session.tenantId' + BT + ';', 1,
172+
'a string is not a comment: an authoring sample teaching the removed alias is a finding'],
173+
];
102174

103-
const plural = offenders.length === 1 ? 'occurrence' : 'occurrences';
104-
console.error(
105-
`check-org-identifier: ${offenders.length} removed \`session.tenantId\` ${plural} in author-facing code\n`,
106-
);
107-
for (const o of offenders) {
108-
console.error(` • ${o.file}:${o.line} ${o.text}`);
175+
let failed = 0;
176+
for (const [src, want, label] of cases) {
177+
const got = findOffenders(src, 'self-test.ts').length;
178+
if (got !== want) {
179+
console.error(` ✗ self-test "${label}": expected ${want} offender(s), got ${got}`);
180+
failed++;
181+
}
182+
}
183+
if (failed) {
184+
console.error(`\n✗ check-org-identifier self-test failed (${failed} case(s)).`);
185+
process.exit(1);
186+
}
187+
console.log(`✓ check-org-identifier self-test: ${cases.length} cases pass.`);
109188
}
110-
console.error(`
189+
190+
function main() {
191+
if (process.argv.includes('--self-test')) return selfTest();
192+
193+
const root = execFileSync('git', ['rev-parse', '--show-toplevel'], {
194+
encoding: 'utf8',
195+
}).trim();
196+
197+
// Newline-delimited on purpose (not `-z`): tracked paths under these roots
198+
// never contain a newline, and avoiding the NUL delimiter keeps this very
199+
// script free of any raw NUL byte (which would make it invisible to grep -- the
200+
// exact #3127 failure mode this repo already guards with check:nul-bytes).
201+
const files = execFileSync('git', ['ls-files', '--', ...ROOTS], {
202+
cwd: root,
203+
encoding: 'utf8',
204+
maxBuffer: 64 * 1024 * 1024,
205+
})
206+
.split('\n')
207+
.filter(Boolean)
208+
.filter((f) => EXTENSIONS.some((ext) => f.endsWith(ext)))
209+
.filter((f) => !EXCLUDED.test(f))
210+
.filter((f) => !TEST_FILE.test(f));
211+
212+
const offenders = [];
213+
for (const file of files) {
214+
offenders.push(...findOffenders(readFileSync(join(root, file), 'utf8'), file));
215+
}
216+
217+
if (offenders.length === 0) {
218+
console.log(
219+
`check-org-identifier: OK (${files.length} author-facing source file(s), no removed session.tenantId alias).`,
220+
);
221+
process.exit(0);
222+
}
223+
224+
const plural = offenders.length === 1 ? 'occurrence' : 'occurrences';
225+
console.error(
226+
`check-org-identifier: ${offenders.length} removed \`session.tenantId\` ${plural} in author-facing code\n`,
227+
);
228+
for (const o of offenders) {
229+
console.error(` • ${o.file}:${o.line} ${o.text}`);
230+
}
231+
console.error(`
111232
\`session.tenantId\` was REMOVED from the hook/action ctx.session surface (#3290);
112233
it no longer carries a value. In a hook or action body read the caller's active
113234
org under the blessed name instead:
@@ -117,4 +238,7 @@ org under the blessed name instead:
117238
It matches the \`organization_id\` column and \`current_user.organizationId\` in
118239
RLS. For a genuine driver-layer use (a configurable isolation column, not the
119240
caller's org), add an \`${ALLOW_MARKER}\` comment on the line.`);
120-
process.exit(1);
241+
process.exit(1);
242+
}
243+
244+
main();

0 commit comments

Comments
 (0)