Skip to content

Commit c71978f

Browse files
yinlianghuiclaude
yinlianghui
and
claude
authored
Convert both canonical-envelope page gates onto the shared comment mask (#12317)
Both gates carried a private two-regex comment stripper, byte-identical to each other and to the family #9367 retired from six gates and #10453 found surviving in two packages/cli tests: source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^[ \t]*\/\/.*$/gm, '') Each fed the `export const X: Page =` scan that decides which pages its package's gate audits, so a block-comment opener that is not a comment -- inside a string literal, or inside a line comment -- opens a phantom comment running to the next real terminator, deletes the declarations in between, and the gate reports GREEN over a page it never read. That shape is already shipped. In packages/cloud-connection/src, the retired regex deletes 122 bytes of live page literal from cloud-connection-ui.ts (the file declaring CloudConnectionSettingsPage) and 277 more from marketplace-proxy-plugin.ts. Both gates were green only because the opener sits BELOW the declaration the scan anchors on. Both now import `maskComments` from scripts/js-comment-mask.mjs, and the scan is split into `pageDeclarationsIn(source)` so the new pins drive the real code path over fixtures rather than over today's tree. Cross-package input radius declared for both packages (the import escapes the package, and the .d.mts mirror types it), with the matching turbo.json inputs. Part of #12267 Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8e31083 commit c71978f

4 files changed

Lines changed: 329 additions & 14 deletions

File tree

‎packages/cloud-connection/src/canonical-expression-envelopes.test.ts‎

Lines changed: 137 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ import {
4444
auditPageExpressionEnvelopes,
4545
renderBareExpressionFindings,
4646
} from '@objectstack/lint';
47+
// The one answer this tree has to "comment, literal, or code". It is a plain
48+
// `.mjs`, but `scripts/js-comment-mask.d.mts` beside it is a hand-written
49+
// declaration mirror (governed by `check:declaration-mirrors`), so this import
50+
// is typed and needs no suppression -- a `@ts-expect-error` here would be an
51+
// UNUSED directive. That `.d.mts` is what gives `maskComments` its type, so it
52+
// is an input to this package's typecheck verdict as well as to this scan.
53+
// Same spelling `packages/cli`'s contract tests use.
54+
import { maskComments } from '../../../scripts/js-comment-mask.mjs';
4755
import { CloudConnectionSettingsPage } from './cloud-connection-ui.js';
4856
import { MarketplaceInstalledPage } from './marketplace-ui.js';
4957

@@ -92,9 +100,47 @@ function tsFilesUnder(dir: string, out: string[] = []): string[] {
92100
return out;
93101
}
94102

95-
/** Strip comments so a `: Page =` inside prose is not read as a declaration. */
96-
function stripComments(source: string): string {
97-
return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^[ \t]*\/\/.*$/gm, '');
103+
/**
104+
* Every `export const X: Page = …` this source text declares, read from CODE.
105+
*
106+
* ## Why the shared mask and not a private comment-stripper
107+
*
108+
* Masking is not a detail of this scan, it is the scan's population rule: text
109+
* this function mistakes for a comment is a page this gate never audits, and
110+
* the gate still reports GREEN — a green line over a page nobody read, which is
111+
* the exact defect class this file exists to prevent, re-entering through the
112+
* detector instead of through the authoring.
113+
*
114+
* What used to stand here was two regexes, block pass first
115+
* (`/\*[\s\S]*?\*\/` lazily, then `^[ \t]*\/\/.*$`) — the same pair #9367
116+
* retired from six gates and #10453 found surviving in two `packages/cli`
117+
* tests. Its failure is the silent one: a block-comment OPENER that is not a
118+
* comment at all — inside a string literal, or inside a line comment — opens a
119+
* phantom comment that runs to the next real `\*\/` and deletes every line
120+
* between, declarations included.
121+
*
122+
* This package is where that stopped being hypothetical. Measured on this tree
123+
* at the time of the conversion, `src/cloud-connection-ui.ts` — the file
124+
* declaring `CloudConnectionSettingsPage` — carries one: the line comment
125+
* `// … /api/v1/cloud-connection/* routes this plugin mounts.` opens a phantom
126+
* that the NEXT docblock's terminator closes, and the retired regex deletes 122
127+
* bytes of live page literal in between, `type: 'cloud-connection:panel'`
128+
* included. `src/marketplace-proxy-plugin.ts` carries two more spans, 277 bytes.
129+
* This gate stayed green only because that opener sits BELOW the
130+
* `export const … : Page =` the scan anchors on — a page declared thirty lines
131+
* further down that file would simply have vanished from the population, and
132+
* every audit below would have reported green over it.
133+
*
134+
* `maskComments` blanks comment spans and leaves string, template and regex
135+
* literals intact, so offsets and line numbers both survive and a `: Page =`
136+
* inside prose still cannot be read as a declaration.
137+
*
138+
* Split out from the walk so the pin below drives the REAL scan over a fixture
139+
* rather than over whatever this package happens to contain today.
140+
*/
141+
function pageDeclarationsIn(source: string): string[] {
142+
return [...maskComments(source).matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)]
143+
.map(match => match[1]!);
98144
}
99145

100146
/**
@@ -108,9 +154,8 @@ function stripComments(source: string): string {
108154
function declaredPageExports(): { name: string; file: string }[] {
109155
const out: { name: string; file: string }[] = [];
110156
for (const file of tsFilesUnder(HERE)) {
111-
const source = stripComments(readFileSync(file, 'utf8'));
112-
for (const match of source.matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)) {
113-
out.push({ name: match[1]!, file: file.slice(HERE.length + 1) });
157+
for (const name of pageDeclarationsIn(readFileSync(file, 'utf8'))) {
158+
out.push({ name, file: file.slice(HERE.length + 1) });
114159
}
115160
}
116161
return out.sort((a, b) => a.name.localeCompare(b.name));
@@ -246,3 +291,89 @@ describe('downgrade control — a shipped page, bare predicate injected', () =>
246291
expect(renderBareExpressionFindings(pristine.findings)).toBe('');
247292
});
248293
});
294+
295+
// ───────────────────────────────────────────────────────────────────────────
296+
// Pin — a phantom comment cannot delete a page from the population
297+
// ───────────────────────────────────────────────────────────────────────────
298+
299+
/**
300+
* The conversion above, pinned by the shape it was made for.
301+
*
302+
* Both fixtures carry a block-comment OPENER that is not a comment — one in a
303+
* line comment (the shape `src/cloud-connection-ui.ts` ships today), one in a
304+
* string literal — followed by a real terminator further down, with a `Page`
305+
* declaration in between. The retired two-regex stripper honours the opener,
306+
* runs lazily to that terminator, and the declaration between them is gone; the
307+
* scan then reports a population one page short and every audit below is green
308+
* over a page it never read.
309+
*
310+
* Measured at the conversion, on this exact text: the retired regex found only
311+
* the trailing page in each fixture, `maskComments` finds both. Reverting
312+
* `pageDeclarationsIn` to that stripper reds these two cases and nothing else in
313+
* this file — the scan is the only thing they exercise.
314+
*
315+
* The `openerIsNotAComment` precondition is here so the pin cannot go quietly
316+
* vacuous: strip the opener out of a fixture while editing and both strippers
317+
* agree again, leaving two tests that pass without asserting anything.
318+
*/
319+
const PHANTOM_IN_LINE_COMMENT = [
320+
"import type { Page } from '@objectstack/spec/ui';",
321+
'',
322+
'// The console panel talks to the same-origin /api/v1/cloud-connection/*',
323+
'// routes this plugin mounts.',
324+
'',
325+
'export const PhantomPage: Page = {',
326+
" name: 'phantom_page',",
327+
" regions: [{ name: 'main', width: 'full', components: [] }],",
328+
'};',
329+
'',
330+
'/** Setup-nav contribution — the terminator that closes the phantom. */',
331+
"export const LaterPage: Page = { name: 'later_page', regions: [] };",
332+
].join('\n');
333+
334+
const PHANTOM_IN_STRING_LITERAL = [
335+
"import type { Page } from '@objectstack/spec/ui';",
336+
'',
337+
"const PROXY_GLOB = '/api/v1/marketplace/*';",
338+
'',
339+
'export const LiteralPhantomPage: Page = {',
340+
" name: 'literal_phantom_page',",
341+
' regions: [],',
342+
'};',
343+
'',
344+
'/** A docblock whose terminator closes the phantom opened in the string. */',
345+
"export const LiteralLaterPage: Page = { name: 'literal_later', regions: [] };",
346+
].join('\n');
347+
348+
/** The fixture still carries the shape: an opener above, a terminator below. */
349+
function openerIsNotAComment(fixture: string, declaration: string): void {
350+
const opener = fixture.indexOf('/' + '*');
351+
const declaredAt = fixture.indexOf(declaration);
352+
const terminator = fixture.indexOf('*' + '/', opener);
353+
expect(opener, 'fixture lost its block-comment opener').toBeGreaterThan(-1);
354+
expect(declaredAt, 'fixture lost its page declaration').toBeGreaterThan(opener);
355+
expect(terminator, 'fixture lost the terminator that closes the phantom')
356+
.toBeGreaterThan(declaredAt);
357+
}
358+
359+
describe('population scan reads comments, not comment-shaped text', () => {
360+
it('keeps a page straddled by an opener inside a LINE COMMENT', () => {
361+
openerIsNotAComment(PHANTOM_IN_LINE_COMMENT, 'export const PhantomPage');
362+
expect(pageDeclarationsIn(PHANTOM_IN_LINE_COMMENT)).toEqual(['PhantomPage', 'LaterPage']);
363+
});
364+
365+
it('keeps a page straddled by an opener inside a STRING LITERAL', () => {
366+
openerIsNotAComment(PHANTOM_IN_STRING_LITERAL, 'export const LiteralPhantomPage');
367+
expect(pageDeclarationsIn(PHANTOM_IN_STRING_LITERAL))
368+
.toEqual(['LiteralPhantomPage', 'LiteralLaterPage']);
369+
});
370+
371+
it('still refuses a `: Page =` written inside genuine prose', () => {
372+
const prose = [
373+
'/** Authors write `export const X: Page = {}` in docblocks like this. */',
374+
'// and in line comments: export const YPage: Page = {}',
375+
"export const RealPage: Page = { name: 'real', regions: [] };",
376+
].join('\n');
377+
expect(pageDeclarationsIn(prose)).toEqual(['RealPage']);
378+
});
379+
});

‎packages/platform-objects/src/pages/canonical-expression-envelopes.test.ts‎

Lines changed: 134 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ import { join, resolve } from 'node:path';
4040
import { describe, expect, it } from 'vitest';
4141
import type { Page } from '@objectstack/spec/ui';
4242
import { auditPageExpressionEnvelopes, renderBareExpressionFindings } from '@objectstack/lint';
43+
// The one answer this tree has to "comment, literal, or code". It is a plain
44+
// `.mjs`, but `scripts/js-comment-mask.d.mts` beside it is a hand-written
45+
// declaration mirror (governed by `check:declaration-mirrors`), so this import
46+
// is typed and needs no suppression -- a `@ts-expect-error` here would be an
47+
// UNUSED directive. That `.d.mts` is what gives `maskComments` its type, so it
48+
// is an input to this package's typecheck verdict as well as to this scan.
49+
// Same spelling `packages/cli`'s contract tests use.
50+
import { maskComments } from '../../../../scripts/js-comment-mask.mjs';
4351
import * as pageExports from './index.js';
4452

4553
type AnyRec = Record<string, unknown>;
@@ -90,9 +98,45 @@ function tsFilesUnder(dir: string, out: string[] = []): string[] {
9098
return out;
9199
}
92100

93-
/** Strip comments so a `: Page =` inside prose is not read as a declaration. */
94-
function stripComments(source: string): string {
95-
return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^[ \t]*\/\/.*$/gm, '');
101+
/**
102+
* Every `export const X: Page = …` this source text declares, read from CODE.
103+
*
104+
* ## Why the shared mask and not a private comment-stripper
105+
*
106+
* Masking is not a detail of this scan, it is the scan's population rule: text
107+
* this function mistakes for a comment is a page this gate never audits, and
108+
* the gate still reports GREEN — a green line over a page nobody read, which is
109+
* the exact defect class this file exists to prevent, re-entering through the
110+
* detector instead of through the authoring.
111+
*
112+
* What used to stand here was two regexes, block pass first
113+
* (`/\*[\s\S]*?\*\/` lazily, then `^[ \t]*\/\/.*$`) — the same pair #9367
114+
* retired from six gates and #10453 found surviving in two `packages/cli`
115+
* tests. Its failure is
116+
* the silent one: a block-comment OPENER that is not a comment at all — inside
117+
* a string literal, or inside a line comment — opens a phantom comment that
118+
* runs to the next real `\*\/` and deletes every line between, declarations
119+
* included.
120+
*
121+
* That is not a shape only a fixture writes. Measured on this tree at the time
122+
* of the conversion, `packages/cloud-connection/src/cloud-connection-ui.ts`
123+
* carries one: the line comment `// … /api/v1/cloud-connection/* routes this
124+
* plugin mounts.` opens a phantom that the next docblock's terminator closes,
125+
* and the retired regex deletes 122 bytes of live page literal in between. Both
126+
* gates stayed green only because the opener happens to sit BELOW the
127+
* `export const … : Page =` the scan anchors on — a page declared thirty lines
128+
* further down that file would simply have vanished from the population.
129+
*
130+
* `maskComments` blanks comment spans and leaves string, template and regex
131+
* literals intact, so offsets and line numbers both survive and a `: Page =`
132+
* inside prose still cannot be read as a declaration.
133+
*
134+
* Split out from the walk so the pin below drives the REAL scan over a fixture
135+
* rather than over whatever this package happens to contain today.
136+
*/
137+
function pageDeclarationsIn(source: string): string[] {
138+
return [...maskComments(source).matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)]
139+
.map(match => match[1]!);
96140
}
97141

98142
/**
@@ -108,9 +152,8 @@ function stripComments(source: string): string {
108152
function declaredPageExports(): { name: string; file: string }[] {
109153
const out: { name: string; file: string }[] = [];
110154
for (const file of tsFilesUnder(PACKAGE_SRC)) {
111-
const source = stripComments(readFileSync(file, 'utf8'));
112-
for (const match of source.matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)) {
113-
out.push({ name: match[1]!, file: file.slice(PACKAGE_SRC.length + 1) });
155+
for (const name of pageDeclarationsIn(readFileSync(file, 'utf8'))) {
156+
out.push({ name, file: file.slice(PACKAGE_SRC.length + 1) });
114157
}
115158
}
116159
return out.sort((a, b) => a.name.localeCompare(b.name));
@@ -225,3 +268,88 @@ describe('downgrade control — a shipped page, predicate downgraded to bare', (
225268
expect(renderBareExpressionFindings(pristine.findings)).toBe('');
226269
});
227270
});
271+
272+
// ───────────────────────────────────────────────────────────────────────────
273+
// Pin — a phantom comment cannot delete a page from the population
274+
// ───────────────────────────────────────────────────────────────────────────
275+
276+
/**
277+
* The conversion above, pinned by the shape it was made for.
278+
*
279+
* Both fixtures carry a block-comment OPENER that is not a comment — one in a
280+
* line comment, one in a string literal — followed by a real terminator further
281+
* down, with a `Page` declaration in between. The retired two-regex stripper
282+
* honours the opener, runs lazily to that terminator, and the declaration
283+
* between them is gone; the scan then reports a population one page short and
284+
* every audit downstream is green over a page it never read.
285+
*
286+
* Measured at the conversion, on this exact text: the retired regex found only
287+
* the trailing page in each fixture, `maskComments` finds both. Reverting
288+
* `pageDeclarationsIn` to that stripper reds these two cases and nothing else
289+
* in this file — the scan is the only thing they exercise.
290+
*
291+
* The `openerIsNotAComment` precondition is here so the pin cannot go quietly
292+
* vacuous: strip the opener out of a fixture while editing and both strippers
293+
* agree again, leaving two tests that pass without asserting anything.
294+
*/
295+
const PHANTOM_IN_LINE_COMMENT = [
296+
"import type { Page } from '@objectstack/spec/ui';",
297+
'',
298+
'// The console panel talks to the same-origin /api/v1/cloud-connection/*',
299+
'// routes this plugin mounts.',
300+
'',
301+
'export const PhantomPage: Page = {',
302+
" name: 'phantom_page',",
303+
" regions: [{ name: 'main', width: 'full', components: [] }],",
304+
'};',
305+
'',
306+
'/** Setup-nav contribution — the terminator that closes the phantom. */',
307+
"export const LaterPage: Page = { name: 'later_page', regions: [] };",
308+
].join('\n');
309+
310+
const PHANTOM_IN_STRING_LITERAL = [
311+
"import type { Page } from '@objectstack/spec/ui';",
312+
'',
313+
"const PROXY_GLOB = '/api/v1/marketplace/*';",
314+
'',
315+
'export const LiteralPhantomPage: Page = {',
316+
" name: 'literal_phantom_page',",
317+
' regions: [],',
318+
'};',
319+
'',
320+
'/** A docblock whose terminator closes the phantom opened in the string. */',
321+
"export const LiteralLaterPage: Page = { name: 'literal_later', regions: [] };",
322+
].join('\n');
323+
324+
/** The fixture still carries the shape: an opener above, a terminator below. */
325+
function openerIsNotAComment(fixture: string, declaration: string): void {
326+
const opener = fixture.indexOf('/' + '*');
327+
const declaredAt = fixture.indexOf(declaration);
328+
const terminator = fixture.indexOf('*' + '/', opener);
329+
expect(opener, 'fixture lost its block-comment opener').toBeGreaterThan(-1);
330+
expect(declaredAt, 'fixture lost its page declaration').toBeGreaterThan(opener);
331+
expect(terminator, 'fixture lost the terminator that closes the phantom')
332+
.toBeGreaterThan(declaredAt);
333+
}
334+
335+
describe('population scan reads comments, not comment-shaped text', () => {
336+
it('keeps a page straddled by an opener inside a LINE COMMENT', () => {
337+
openerIsNotAComment(PHANTOM_IN_LINE_COMMENT, 'export const PhantomPage');
338+
expect(pageDeclarationsIn(PHANTOM_IN_LINE_COMMENT)).toEqual(['PhantomPage', 'LaterPage']);
339+
});
340+
341+
it('keeps a page straddled by an opener inside a STRING LITERAL', () => {
342+
openerIsNotAComment(PHANTOM_IN_STRING_LITERAL, 'export const LiteralPhantomPage');
343+
expect(pageDeclarationsIn(PHANTOM_IN_STRING_LITERAL))
344+
.toEqual(['LiteralPhantomPage', 'LiteralLaterPage']);
345+
});
346+
347+
it('still refuses a `: Page =` written inside genuine prose', () => {
348+
const prose = [
349+
'/** Authors write `export const X: Page = {}` in docblocks like this. */',
350+
"// and in line comments: export const YPage: Page = {}",
351+
"export const RealPage: Page = { name: 'real', regions: [] };",
352+
].join('\n');
353+
expect(pageDeclarationsIn(prose)).toEqual(['RealPage']);
354+
});
355+
});

‎scripts/cross-package-test-inputs.mjs‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,12 +399,50 @@ export const CROSS_PACKAGE_TEST_INPUTS = {
399399
'packages/cli/src/commands/**': ['packages/lint/src/authoring-rule-wiring.test.ts'],
400400
},
401401
},
402+
'@objectstack/cloud-connection': {
403+
// src/canonical-expression-envelopes.test.ts (#12267) imports `maskComments`
404+
// from `js-comment-mask.mjs` to decide which text in this package's `src/` is
405+
// a comment and which is a `Page` declaration — the same conversion #9367
406+
// made for six gates. The coupling is real: that gate's POPULATION is a
407+
// function of the module's masking behaviour, so a change to it has to re-run
408+
// this package's suite. The `.d.mts` sibling is declared alongside it because
409+
// it is what gives `maskComments` its type, so this package's typecheck
410+
// verdict is a function of it too — the reason the `@objectstack/cli` entry
411+
// above declares the pair rather than the module alone.
412+
globs: [
413+
'scripts/js-comment-mask.mjs',
414+
'scripts/js-comment-mask.d.mts',
415+
],
416+
},
402417
'@objectstack/platform-objects': {
403418
// src/managed-api-method-affordance-sweep.test.ts (#7934) imports every
404419
// `*.object.ts` in the monorepo and runs `validateManagedApiMethods` over
405420
// it — the population `os lint` never walks, because these objects ship as
406421
// code rather than in an authored stack.
407-
globs: ['packages/**/*.object.ts'],
422+
//
423+
// `js-comment-mask.mjs` and its `.d.mts` sibling are read by
424+
// src/pages/canonical-expression-envelopes.test.ts (#12267), which imports
425+
// `maskComments` to decide which text in this package's `src/` is a comment
426+
// and which is a `Page` declaration. Declared for both reasons the
427+
// `@objectstack/cli` entry above records: the import is a real coupling —
428+
// that gate's POPULATION is a function of the module's masking behaviour, so
429+
// a change to it has to re-run this package's suite — and the `.d.mts` is
430+
// what gives `maskComments` its type, so this package's `tsc --noEmit`
431+
// verdict is a function of it too.
432+
//
433+
// `page-envelope-audit.test.ts` and `cloud-connection-ui.ts` are named in
434+
// that same file's prose and read by nothing. The literal collector takes
435+
// quoted paths without parsing, so a mention forces a declaration; the
436+
// `check-nul-bytes.mjs` entry above settles that trade — declaring the file
437+
// beats rewording a comment to dodge a scanner, and over-collection can only
438+
// widen a radius, never narrow one.
439+
globs: [
440+
'packages/**/*.object.ts',
441+
'scripts/js-comment-mask.mjs',
442+
'scripts/js-comment-mask.d.mts',
443+
'packages/lint/src/page-envelope-audit.test.ts',
444+
'packages/cloud-connection/src/cloud-connection-ui.ts',
445+
],
408446
},
409447
'@objectstack/plugin-auth': {
410448
// src/managed-extension-fields.test.ts walks every `*.object.ts`, and pins

0 commit comments

Comments
 (0)