From 25ef2712c2ae7a8535eca3a09612e211ad096b62 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 17:20:13 +0000 Subject: [PATCH] Convert both canonical-envelope page gates onto the shared comment mask 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 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 --- .../canonical-expression-envelopes.test.ts | 143 +++++++++++++++++- .../canonical-expression-envelopes.test.ts | 140 ++++++++++++++++- scripts/cross-package-test-inputs.mjs | 40 ++++- turbo.json | 20 ++- 4 files changed, 329 insertions(+), 14 deletions(-) diff --git a/packages/cloud-connection/src/canonical-expression-envelopes.test.ts b/packages/cloud-connection/src/canonical-expression-envelopes.test.ts index 0809657703..f3acfa20af 100644 --- a/packages/cloud-connection/src/canonical-expression-envelopes.test.ts +++ b/packages/cloud-connection/src/canonical-expression-envelopes.test.ts @@ -44,6 +44,14 @@ import { auditPageExpressionEnvelopes, renderBareExpressionFindings, } from '@objectstack/lint'; +// The one answer this tree has to "comment, literal, or code". It is a plain +// `.mjs`, but `scripts/js-comment-mask.d.mts` beside it is a hand-written +// declaration mirror (governed by `check:declaration-mirrors`), so this import +// is typed and needs no suppression -- a `@ts-expect-error` here would be an +// UNUSED directive. That `.d.mts` is what gives `maskComments` its type, so it +// is an input to this package's typecheck verdict as well as to this scan. +// Same spelling `packages/cli`'s contract tests use. +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { CloudConnectionSettingsPage } from './cloud-connection-ui.js'; import { MarketplaceInstalledPage } from './marketplace-ui.js'; @@ -92,9 +100,47 @@ function tsFilesUnder(dir: string, out: string[] = []): string[] { return out; } -/** Strip comments so a `: Page =` inside prose is not read as a declaration. */ -function stripComments(source: string): string { - return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^[ \t]*\/\/.*$/gm, ''); +/** + * Every `export const X: Page = …` this source text declares, read from CODE. + * + * ## Why the shared mask and not a private comment-stripper + * + * Masking is not a detail of this scan, it is the scan's population rule: text + * this function mistakes for a comment is a page this gate never audits, and + * the gate still reports GREEN — a green line over a page nobody read, which is + * the exact defect class this file exists to prevent, re-entering through the + * detector instead of through the authoring. + * + * What used to stand here was two regexes, block pass first + * (`/\*[\s\S]*?\*\/` lazily, then `^[ \t]*\/\/.*$`) — the same pair #9367 + * retired from six gates and #10453 found surviving in two `packages/cli` + * tests. Its failure is the silent one: a block-comment OPENER that is not a + * comment at all — inside a string literal, or inside a line comment — opens a + * phantom comment that runs to the next real `\*\/` and deletes every line + * between, declarations included. + * + * This package is where that stopped being hypothetical. Measured on this tree + * at the time of the conversion, `src/cloud-connection-ui.ts` — the file + * declaring `CloudConnectionSettingsPage` — carries one: the line comment + * `// … /api/v1/cloud-connection/* routes this plugin mounts.` opens a phantom + * that the NEXT docblock's terminator closes, and the retired regex deletes 122 + * bytes of live page literal in between, `type: 'cloud-connection:panel'` + * included. `src/marketplace-proxy-plugin.ts` carries two more spans, 277 bytes. + * This gate stayed green only because that opener sits BELOW the + * `export const … : Page =` the scan anchors on — a page declared thirty lines + * further down that file would simply have vanished from the population, and + * every audit below would have reported green over it. + * + * `maskComments` blanks comment spans and leaves string, template and regex + * literals intact, so offsets and line numbers both survive and a `: Page =` + * inside prose still cannot be read as a declaration. + * + * Split out from the walk so the pin below drives the REAL scan over a fixture + * rather than over whatever this package happens to contain today. + */ +function pageDeclarationsIn(source: string): string[] { + return [...maskComments(source).matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)] + .map(match => match[1]!); } /** @@ -108,9 +154,8 @@ function stripComments(source: string): string { function declaredPageExports(): { name: string; file: string }[] { const out: { name: string; file: string }[] = []; for (const file of tsFilesUnder(HERE)) { - const source = stripComments(readFileSync(file, 'utf8')); - for (const match of source.matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)) { - out.push({ name: match[1]!, file: file.slice(HERE.length + 1) }); + for (const name of pageDeclarationsIn(readFileSync(file, 'utf8'))) { + out.push({ name, file: file.slice(HERE.length + 1) }); } } return out.sort((a, b) => a.name.localeCompare(b.name)); @@ -246,3 +291,89 @@ describe('downgrade control — a shipped page, bare predicate injected', () => expect(renderBareExpressionFindings(pristine.findings)).toBe(''); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// Pin — a phantom comment cannot delete a page from the population +// ─────────────────────────────────────────────────────────────────────────── + +/** + * The conversion above, pinned by the shape it was made for. + * + * Both fixtures carry a block-comment OPENER that is not a comment — one in a + * line comment (the shape `src/cloud-connection-ui.ts` ships today), one in a + * string literal — followed by a real terminator further down, with a `Page` + * declaration in between. The retired two-regex stripper honours the opener, + * runs lazily to that terminator, and the declaration between them is gone; the + * scan then reports a population one page short and every audit below is green + * over a page it never read. + * + * Measured at the conversion, on this exact text: the retired regex found only + * the trailing page in each fixture, `maskComments` finds both. Reverting + * `pageDeclarationsIn` to that stripper reds these two cases and nothing else in + * this file — the scan is the only thing they exercise. + * + * The `openerIsNotAComment` precondition is here so the pin cannot go quietly + * vacuous: strip the opener out of a fixture while editing and both strippers + * agree again, leaving two tests that pass without asserting anything. + */ +const PHANTOM_IN_LINE_COMMENT = [ + "import type { Page } from '@objectstack/spec/ui';", + '', + '// The console panel talks to the same-origin /api/v1/cloud-connection/*', + '// routes this plugin mounts.', + '', + 'export const PhantomPage: Page = {', + " name: 'phantom_page',", + " regions: [{ name: 'main', width: 'full', components: [] }],", + '};', + '', + '/** Setup-nav contribution — the terminator that closes the phantom. */', + "export const LaterPage: Page = { name: 'later_page', regions: [] };", +].join('\n'); + +const PHANTOM_IN_STRING_LITERAL = [ + "import type { Page } from '@objectstack/spec/ui';", + '', + "const PROXY_GLOB = '/api/v1/marketplace/*';", + '', + 'export const LiteralPhantomPage: Page = {', + " name: 'literal_phantom_page',", + ' regions: [],', + '};', + '', + '/** A docblock whose terminator closes the phantom opened in the string. */', + "export const LiteralLaterPage: Page = { name: 'literal_later', regions: [] };", +].join('\n'); + +/** The fixture still carries the shape: an opener above, a terminator below. */ +function openerIsNotAComment(fixture: string, declaration: string): void { + const opener = fixture.indexOf('/' + '*'); + const declaredAt = fixture.indexOf(declaration); + const terminator = fixture.indexOf('*' + '/', opener); + expect(opener, 'fixture lost its block-comment opener').toBeGreaterThan(-1); + expect(declaredAt, 'fixture lost its page declaration').toBeGreaterThan(opener); + expect(terminator, 'fixture lost the terminator that closes the phantom') + .toBeGreaterThan(declaredAt); +} + +describe('population scan reads comments, not comment-shaped text', () => { + it('keeps a page straddled by an opener inside a LINE COMMENT', () => { + openerIsNotAComment(PHANTOM_IN_LINE_COMMENT, 'export const PhantomPage'); + expect(pageDeclarationsIn(PHANTOM_IN_LINE_COMMENT)).toEqual(['PhantomPage', 'LaterPage']); + }); + + it('keeps a page straddled by an opener inside a STRING LITERAL', () => { + openerIsNotAComment(PHANTOM_IN_STRING_LITERAL, 'export const LiteralPhantomPage'); + expect(pageDeclarationsIn(PHANTOM_IN_STRING_LITERAL)) + .toEqual(['LiteralPhantomPage', 'LiteralLaterPage']); + }); + + it('still refuses a `: Page =` written inside genuine prose', () => { + const prose = [ + '/** Authors write `export const X: Page = {}` in docblocks like this. */', + '// and in line comments: export const YPage: Page = {}', + "export const RealPage: Page = { name: 'real', regions: [] };", + ].join('\n'); + expect(pageDeclarationsIn(prose)).toEqual(['RealPage']); + }); +}); diff --git a/packages/platform-objects/src/pages/canonical-expression-envelopes.test.ts b/packages/platform-objects/src/pages/canonical-expression-envelopes.test.ts index e10c968e05..6791bc618b 100644 --- a/packages/platform-objects/src/pages/canonical-expression-envelopes.test.ts +++ b/packages/platform-objects/src/pages/canonical-expression-envelopes.test.ts @@ -40,6 +40,14 @@ import { join, resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; import type { Page } from '@objectstack/spec/ui'; import { auditPageExpressionEnvelopes, renderBareExpressionFindings } from '@objectstack/lint'; +// The one answer this tree has to "comment, literal, or code". It is a plain +// `.mjs`, but `scripts/js-comment-mask.d.mts` beside it is a hand-written +// declaration mirror (governed by `check:declaration-mirrors`), so this import +// is typed and needs no suppression -- a `@ts-expect-error` here would be an +// UNUSED directive. That `.d.mts` is what gives `maskComments` its type, so it +// is an input to this package's typecheck verdict as well as to this scan. +// Same spelling `packages/cli`'s contract tests use. +import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; import * as pageExports from './index.js'; type AnyRec = Record; @@ -90,9 +98,45 @@ function tsFilesUnder(dir: string, out: string[] = []): string[] { return out; } -/** Strip comments so a `: Page =` inside prose is not read as a declaration. */ -function stripComments(source: string): string { - return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^[ \t]*\/\/.*$/gm, ''); +/** + * Every `export const X: Page = …` this source text declares, read from CODE. + * + * ## Why the shared mask and not a private comment-stripper + * + * Masking is not a detail of this scan, it is the scan's population rule: text + * this function mistakes for a comment is a page this gate never audits, and + * the gate still reports GREEN — a green line over a page nobody read, which is + * the exact defect class this file exists to prevent, re-entering through the + * detector instead of through the authoring. + * + * What used to stand here was two regexes, block pass first + * (`/\*[\s\S]*?\*\/` lazily, then `^[ \t]*\/\/.*$`) — the same pair #9367 + * retired from six gates and #10453 found surviving in two `packages/cli` + * tests. Its failure is + * the silent one: a block-comment OPENER that is not a comment at all — inside + * a string literal, or inside a line comment — opens a phantom comment that + * runs to the next real `\*\/` and deletes every line between, declarations + * included. + * + * That is not a shape only a fixture writes. Measured on this tree at the time + * of the conversion, `packages/cloud-connection/src/cloud-connection-ui.ts` + * carries one: the line comment `// … /api/v1/cloud-connection/* routes this + * plugin mounts.` opens a phantom that the next docblock's terminator closes, + * and the retired regex deletes 122 bytes of live page literal in between. Both + * gates stayed green only because the opener happens to sit BELOW the + * `export const … : Page =` the scan anchors on — a page declared thirty lines + * further down that file would simply have vanished from the population. + * + * `maskComments` blanks comment spans and leaves string, template and regex + * literals intact, so offsets and line numbers both survive and a `: Page =` + * inside prose still cannot be read as a declaration. + * + * Split out from the walk so the pin below drives the REAL scan over a fixture + * rather than over whatever this package happens to contain today. + */ +function pageDeclarationsIn(source: string): string[] { + return [...maskComments(source).matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)] + .map(match => match[1]!); } /** @@ -108,9 +152,8 @@ function stripComments(source: string): string { function declaredPageExports(): { name: string; file: string }[] { const out: { name: string; file: string }[] = []; for (const file of tsFilesUnder(PACKAGE_SRC)) { - const source = stripComments(readFileSync(file, 'utf8')); - for (const match of source.matchAll(/export\s+const\s+(\w+)\s*:\s*Page\s*=/g)) { - out.push({ name: match[1]!, file: file.slice(PACKAGE_SRC.length + 1) }); + for (const name of pageDeclarationsIn(readFileSync(file, 'utf8'))) { + out.push({ name, file: file.slice(PACKAGE_SRC.length + 1) }); } } return out.sort((a, b) => a.name.localeCompare(b.name)); @@ -225,3 +268,88 @@ describe('downgrade control — a shipped page, predicate downgraded to bare', ( expect(renderBareExpressionFindings(pristine.findings)).toBe(''); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// Pin — a phantom comment cannot delete a page from the population +// ─────────────────────────────────────────────────────────────────────────── + +/** + * The conversion above, pinned by the shape it was made for. + * + * Both fixtures carry a block-comment OPENER that is not a comment — one in a + * line comment, one in a string literal — followed by a real terminator further + * down, with a `Page` declaration in between. The retired two-regex stripper + * honours the opener, runs lazily to that terminator, and the declaration + * between them is gone; the scan then reports a population one page short and + * every audit downstream is green over a page it never read. + * + * Measured at the conversion, on this exact text: the retired regex found only + * the trailing page in each fixture, `maskComments` finds both. Reverting + * `pageDeclarationsIn` to that stripper reds these two cases and nothing else + * in this file — the scan is the only thing they exercise. + * + * The `openerIsNotAComment` precondition is here so the pin cannot go quietly + * vacuous: strip the opener out of a fixture while editing and both strippers + * agree again, leaving two tests that pass without asserting anything. + */ +const PHANTOM_IN_LINE_COMMENT = [ + "import type { Page } from '@objectstack/spec/ui';", + '', + '// The console panel talks to the same-origin /api/v1/cloud-connection/*', + '// routes this plugin mounts.', + '', + 'export const PhantomPage: Page = {', + " name: 'phantom_page',", + " regions: [{ name: 'main', width: 'full', components: [] }],", + '};', + '', + '/** Setup-nav contribution — the terminator that closes the phantom. */', + "export const LaterPage: Page = { name: 'later_page', regions: [] };", +].join('\n'); + +const PHANTOM_IN_STRING_LITERAL = [ + "import type { Page } from '@objectstack/spec/ui';", + '', + "const PROXY_GLOB = '/api/v1/marketplace/*';", + '', + 'export const LiteralPhantomPage: Page = {', + " name: 'literal_phantom_page',", + ' regions: [],', + '};', + '', + '/** A docblock whose terminator closes the phantom opened in the string. */', + "export const LiteralLaterPage: Page = { name: 'literal_later', regions: [] };", +].join('\n'); + +/** The fixture still carries the shape: an opener above, a terminator below. */ +function openerIsNotAComment(fixture: string, declaration: string): void { + const opener = fixture.indexOf('/' + '*'); + const declaredAt = fixture.indexOf(declaration); + const terminator = fixture.indexOf('*' + '/', opener); + expect(opener, 'fixture lost its block-comment opener').toBeGreaterThan(-1); + expect(declaredAt, 'fixture lost its page declaration').toBeGreaterThan(opener); + expect(terminator, 'fixture lost the terminator that closes the phantom') + .toBeGreaterThan(declaredAt); +} + +describe('population scan reads comments, not comment-shaped text', () => { + it('keeps a page straddled by an opener inside a LINE COMMENT', () => { + openerIsNotAComment(PHANTOM_IN_LINE_COMMENT, 'export const PhantomPage'); + expect(pageDeclarationsIn(PHANTOM_IN_LINE_COMMENT)).toEqual(['PhantomPage', 'LaterPage']); + }); + + it('keeps a page straddled by an opener inside a STRING LITERAL', () => { + openerIsNotAComment(PHANTOM_IN_STRING_LITERAL, 'export const LiteralPhantomPage'); + expect(pageDeclarationsIn(PHANTOM_IN_STRING_LITERAL)) + .toEqual(['LiteralPhantomPage', 'LiteralLaterPage']); + }); + + it('still refuses a `: Page =` written inside genuine prose', () => { + const prose = [ + '/** Authors write `export const X: Page = {}` in docblocks like this. */', + "// and in line comments: export const YPage: Page = {}", + "export const RealPage: Page = { name: 'real', regions: [] };", + ].join('\n'); + expect(pageDeclarationsIn(prose)).toEqual(['RealPage']); + }); +}); diff --git a/scripts/cross-package-test-inputs.mjs b/scripts/cross-package-test-inputs.mjs index 47b2433147..88df65bcdf 100644 --- a/scripts/cross-package-test-inputs.mjs +++ b/scripts/cross-package-test-inputs.mjs @@ -399,12 +399,50 @@ export const CROSS_PACKAGE_TEST_INPUTS = { 'packages/cli/src/commands/**': ['packages/lint/src/authoring-rule-wiring.test.ts'], }, }, + '@objectstack/cloud-connection': { + // src/canonical-expression-envelopes.test.ts (#12267) imports `maskComments` + // from `js-comment-mask.mjs` to decide which text in this package's `src/` is + // a comment and which is a `Page` declaration — the same conversion #9367 + // made for six gates. The coupling is real: that gate's POPULATION is a + // function of the module's masking behaviour, so a change to it has to re-run + // this package's suite. The `.d.mts` sibling is declared alongside it because + // it is what gives `maskComments` its type, so this package's typecheck + // verdict is a function of it too — the reason the `@objectstack/cli` entry + // above declares the pair rather than the module alone. + globs: [ + 'scripts/js-comment-mask.mjs', + 'scripts/js-comment-mask.d.mts', + ], + }, '@objectstack/platform-objects': { // src/managed-api-method-affordance-sweep.test.ts (#7934) imports every // `*.object.ts` in the monorepo and runs `validateManagedApiMethods` over // it — the population `os lint` never walks, because these objects ship as // code rather than in an authored stack. - globs: ['packages/**/*.object.ts'], + // + // `js-comment-mask.mjs` and its `.d.mts` sibling are read by + // src/pages/canonical-expression-envelopes.test.ts (#12267), which imports + // `maskComments` to decide which text in this package's `src/` is a comment + // and which is a `Page` declaration. Declared for both reasons the + // `@objectstack/cli` entry above records: the import is a real coupling — + // that gate's POPULATION is a function of the module's masking behaviour, so + // a change to it has to re-run this package's suite — and the `.d.mts` is + // what gives `maskComments` its type, so this package's `tsc --noEmit` + // verdict is a function of it too. + // + // `page-envelope-audit.test.ts` and `cloud-connection-ui.ts` are named in + // that same file's prose and read by nothing. The literal collector takes + // quoted paths without parsing, so a mention forces a declaration; the + // `check-nul-bytes.mjs` entry above settles that trade — declaring the file + // beats rewording a comment to dodge a scanner, and over-collection can only + // widen a radius, never narrow one. + globs: [ + 'packages/**/*.object.ts', + 'scripts/js-comment-mask.mjs', + 'scripts/js-comment-mask.d.mts', + 'packages/lint/src/page-envelope-audit.test.ts', + 'packages/cloud-connection/src/cloud-connection-ui.ts', + ], }, '@objectstack/plugin-auth': { // src/managed-extension-fields.test.ts walks every `*.object.ts`, and pins diff --git a/turbo.json b/turbo.json index 3d4dc9f53d..e0d9e78000 100644 --- a/turbo.json +++ b/turbo.json @@ -127,6 +127,20 @@ "$TURBO_ROOT$/examples/app-showcase/src/ui/views/contact.view.ts" ] }, + "@objectstack/cloud-connection#test": { + "dependsOn": [ + "^build" + ], + "outputs": [], + "inputs": [ + "$TURBO_DEFAULT$", + "!dist/**", + "!coverage/**", + "!.turbo/**", + "$TURBO_ROOT$/scripts/js-comment-mask.mjs", + "$TURBO_ROOT$/scripts/js-comment-mask.d.mts" + ] + }, "@objectstack/platform-objects#test": { "dependsOn": ["^build"], "outputs": [], @@ -135,7 +149,11 @@ "!dist/**", "!coverage/**", "!.turbo/**", - "$TURBO_ROOT$/packages/**/*.object.ts" + "$TURBO_ROOT$/packages/**/*.object.ts", + "$TURBO_ROOT$/scripts/js-comment-mask.mjs", + "$TURBO_ROOT$/scripts/js-comment-mask.d.mts", + "$TURBO_ROOT$/packages/lint/src/page-envelope-audit.test.ts", + "$TURBO_ROOT$/packages/cloud-connection/src/cloud-connection-ui.ts" ] }, "@objectstack/plugin-auth#test": {