From a3a5cb6d7b1f1ecd619d4f4475489d5165067f46 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 18:55:55 +0000 Subject: [PATCH 1/4] docs(plugin-audit): document the `os serve` opt-in and rule out a config-derived audit helper (#9863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docs page for record-view auditing told readers the capability had "no knob to turn it on" under `os serve`. That stopped being true when #9864 declared and pinned the duplicate-registration contract: a configured `AuditPlugin` in the stack's `plugins` array supersedes the CLI's option-less instance by name. Both the page and the published README now spell that path. #9863's open question — whether `os serve` should grow an `appAuditPluginOptions(config)` helper mirroring its `SecurityPlugin` sibling — is ruled NO, with the reasoning recorded at the registration site and pinned by `serve-audit-registration.contract.test.ts`. No runtime behaviour changed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --- .../audit-plugin-boot-path-reachability.md | 11 + .../docs/permissions/record-view-auditing.mdx | 57 ++++- .../serve-audit-registration.contract.test.ts | 233 ++++++++++++++++++ packages/cli/src/commands/serve.ts | 56 ++++- packages/plugins/plugin-audit/README.md | 27 ++ 5 files changed, 377 insertions(+), 7 deletions(-) create mode 100644 .changeset/audit-plugin-boot-path-reachability.md create mode 100644 packages/cli/src/commands/serve-audit-registration.contract.test.ts diff --git a/.changeset/audit-plugin-boot-path-reachability.md b/.changeset/audit-plugin-boot-path-reachability.md new file mode 100644 index 0000000000..bb2813c6e4 --- /dev/null +++ b/.changeset/audit-plugin-boot-path-reachability.md @@ -0,0 +1,11 @@ +--- +"@objectstack/plugin-audit": patch +--- + +**Docs (published README) + ruling:** record-view auditing now documents how to turn it on under `objectstack serve`, and the answer to "should `os serve` grow an `appAuditPluginOptions(config)` helper?" is **no** (#9863). + +The README and `content/docs/permissions/record-view-auditing.mdx` both said the audited set is configured "where you compose the kernel", and the docs page went further: *"The CLI's `os serve` registers `AuditPlugin` with no options, so a stack served that way has record-view auditing off and no knob to turn it on."* That last clause stopped being true when #9864 declared and pinned the duplicate-registration contract. The knob is the stack's `plugins` array — a configured `new AuditPlugin({ readAudit: { objects: [...] } })` there supersedes the CLI's option-less instance by name, last-one-wins, on both kernels, with the displaced instance never reaching `init()`. Both pages now spell that path, and name the `Plugin superseded: 'com.objectstack.audit'` boot line as the opt-in working rather than a misconfiguration. + +**No new configuration surface was added, deliberately.** A `config.audit` key read by an `appAuditPluginOptions(config)` helper would reproduce, in `objectstack.config.ts`, exactly the failure #8992's ruling refused for the object-metadata spelling: a declaration that survives in a deployment which never installs this package, reading as coverage while recording nothing. It would also be a *second* configuration surface that silently loses to the first, since an app's own `plugins` entry supersedes whatever the CLI constructed. The `#7001` symmetry argument does not carry it either — `@objectstack/verify`'s `bootStack` constructs no `AuditPlugin` and does not depend on this package, so there is no second boot path to disagree with. + +No runtime behaviour changed. `packages/cli` gains only the reasoning at its registration site and `serve-audit-registration.contract.test.ts`, which pins the three facts the ruling rests on — including the load-bearing ordering (`AuditPlugin` registered above the stack `plugins` loop) that until now was asserted by a comment and nothing else. diff --git a/content/docs/permissions/record-view-auditing.mdx b/content/docs/permissions/record-view-auditing.mdx index fd4f4c6e99..3fd36bb1f1 100644 --- a/content/docs/permissions/record-view-auditing.mdx +++ b/content/docs/permissions/record-view-auditing.mdx @@ -68,9 +68,60 @@ that never installs this plugin, producing metadata that *reads* as audited and records nothing — and on a compliance surface, a declaration a reviewer mistakes for coverage is worse than an absent feature. -The practical consequence is that this is configured **where you compose the -kernel**. The CLI's `os serve` registers `AuditPlugin` with no options, so a -stack served that way has record-view auditing off and no knob to turn it on. +The practical consequence is that this is configured **where the plugin is +installed**. On the `os serve` boot path that place is your stack's `plugins` +array — see [Under `os serve`](#under-os-serve) below. + + +### Under `os serve` + +`os serve` may auto-register `AuditPlugin` for you, and when it does it passes +**no options** — that instance audits no views. To turn record-view auditing on, +put your own configured instance in the stack's `plugins` array: + +```typescript +// objectstack.config.ts +import { defineStack } from '@objectstack/spec'; +import { AuditPlugin } from '@objectstack/plugin-audit'; + +export default defineStack({ + manifest: { name: 'my-app', version: '1.0.0' }, + plugins: [ + new AuditPlugin({ readAudit: { objects: ['contact', 'account'] } }), + ], + // objects, apps, views, … +}); +``` + +That is a declared contract, not a lucky ordering. The CLI registers its +option-less instance **before** it walks `plugins`, and registering a plugin +whose `name` is already taken **overwrites** the earlier registration — +last-one-wins, identically on both kernels, with a `warn` naming both versions. +Your configured instance is the one that boots; the CLI's is discarded before it +ever reaches `init()`, so nothing is started twice and nothing leaks. A line like + +```text +Plugin superseded: 'com.objectstack.audit' — the later registration (v1.0.0) +REPLACED the earlier one (v1.0.0). Only the later instance is initialized and +started; the earlier one is discarded without ever running init(). +``` + +in your boot log **is the opt-in working**, not a misconfiguration. (You will +not always see it: the CLI's auto-registration is paired with its `AuthPlugin` +bootstrap, so a stack that supplies its own `AuthPlugin` never gets a second +audit instance to supersede in the first place.) + + +**There is no stack-config key either.** No `audit:` block in +`objectstack.config.ts` names the audited objects, deliberately, and for exactly +the reason there is no object-metadata key: such a key would survive in a +deployment that never installs `@objectstack/plugin-audit` at all, reading as +coverage while recording nothing. + +`requires: ['audit']` is not a substitute — it makes the plugin's presence a +hard boot requirement, but it constructs the plugin with no options and offers +nowhere to name objects. Naming audited objects means constructing the plugin +yourself, as above. ## What counts as a record view diff --git a/packages/cli/src/commands/serve-audit-registration.contract.test.ts b/packages/cli/src/commands/serve-audit-registration.contract.test.ts new file mode 100644 index 0000000000..7e778b3f05 --- /dev/null +++ b/packages/cli/src/commands/serve-audit-registration.contract.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `objectstack serve`'s AuditPlugin registration — the facts the #9863 ruling +// rests on, mechanised. +// +// ## The question, and the answer +// +// #9863 asked whether `os serve` should grow an `appAuditPluginOptions(config)` +// helper mirroring the `appSecurityPluginOptions(config)` sibling six lines +// above it, so that record-view auditing (`AuditPluginOptions.readAudit`) could +// be turned on from `objectstack.config.ts` — rather than only by an app +// putting its OWN configured `new AuditPlugin({ readAudit: … })` in the stack's +// `plugins` array, where it supersedes the CLI's option-less instance under the +// declared last-one-wins registration contract (#9864, maintainer ruling +// 2026-08-19, option B). +// +// Ruled NO, on four measurements — the reasoning lives at the registration site +// in `serve.ts` and in #9863's ruling comment; what lives HERE is the part that +// has to keep being true: +// +// 1. The CLI constructs `AuditPlugin` exactly once, with NO options. That is +// the ruling itself. Re-opening it means editing this file deliberately, +// not discovering later that the shape drifted. +// 2. That construction sits inside the auth-gated pair block and ABOVE the +// stack `plugins` loop. The ORDER is load-bearing and was, until this file, +// asserted by nothing: invert it and the CLI's option-less instance +// supersedes the app's configured one, silently turning record-view +// auditing back OFF for every deployment that had opted in. Nothing else +// in the repo goes red on that edit. +// 3. `@objectstack/verify`'s `bootStack` constructs no `AuditPlugin` at all. +// This is why the #7001 argument for the security helper does not transfer: +// that helper exists because TWO boot paths both built a `SecurityPlugin` +// and silently disagreed about its options. Audit has exactly one boot path +// with an opinion, so there is no disagreement for a shared helper to close +// — and `@objectstack/verify` does not even depend on +// `@objectstack/plugin-audit` (see its package.json), so it cannot grow one +// by accident. If that changes, the ruling's basis changes with it, and +// this assertion is what says so. +// +// ## Why a source scan rather than a boot +// +// Same reason as this directory's `serve-verify-security-parity.contract.test.ts` +// and `serve-email-config-parity.contract.test.ts`: the failure mode is an EDIT +// to these files, and every one of the three facts above is invisible to a +// behavioural test. `serve.ts`'s audit block is reachable only from a live +// `objectstack serve` boot with `@objectstack/plugin-auth` and +// `@objectstack/plugin-audit` installed, an auth secret set and no app-supplied +// AuthPlugin; a unit test that got there would be testing the fixture. The grep +// that WOULD have caught each edit, mechanised, is the honest instrument. + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); + +/** + * `packages/cli/src/commands/` → `packages/`. Reading verify's harness from + * here is what makes fact 3 an assertion instead of a comment; + * `@objectstack/verify` is a real dependency of this package and the read is + * test-only (tests never ship — `files: ["dist"]`). The glob is already + * declared for `@objectstack/cli` in `scripts/check-cross-package-test-inputs.mjs` + * and hashed by `@objectstack/cli#test` in turbo.json, for the sibling parity + * scan; this file adds a second reader of the same path, not a new radius. + */ +const PACKAGES_DIR = path.resolve(HERE, '../../..'); + +/** + * Absence must be loud (AGENTS.md, Route & surface ownership §3). A scan that + * reports success because it could not find its subject is worse than no scan: + * every assertion below is of the form "this shape is present / is not present", + * and an empty string satisfies half of them for free. + */ +function readBootPath(relative: string): string { + const full = path.join(PACKAGES_DIR, relative); + try { + return readFileSync(full, 'utf8'); + } catch (e) { + throw new Error( + `#9863 audit-registration scan cannot read its subject '${relative}' (looked at ${full}). ` + + 'The file moved or was renamed — repoint this scan; do NOT delete it. The ruling it pins ' + + `is still in force. (${(e as Error).message})`, + ); + } +} + +/** + * Comments stripped, because this scan is about what the two files DO. + * + * Not optional here, and not a copy of the sibling's caution: `serve.ts`'s audit + * block DESCRIBES the very construction being counted — it spells + * `new AuditPlugin({ readAudit: … })` in prose to explain the app-side opt-in it + * documents, and it names `appAuditPluginOptions` to record that the helper was + * ruled against. Over raw text this file would count two constructions where + * the code has one, and its own ruling assertion would fail on the sentence + * stating the ruling. + * + * ## Line comments FIRST, and that ordering is measured, not stylistic + * + * This directory's two older parity scans run the block pass first. On + * `serve.ts` that pass is not conservative — it is destructive. The `5d.` + * header comment contains the URL glob `/api/v1/auth/*`, whose `/*` opens a + * block comment as far as a regex is concerned; the next block-comment close + * in the file is + * the closing of `import(/* webpackIgnore: true *\/)` ten lines below, so the + * pass silently deletes the whole intervening region — the `hasAuthPlugin` + * computation and the `if (!hasAuthPlugin && tierEnabled('auth'))` gate this + * file measures against. Measured over the two files this scan reads: + * block-first keeps 1895 code-bearing lines of `serve.ts`, line-first keeps + * 2098. The 203-line difference is code, not prose. + * + * Stripping `//` runs first, so `/api/v1/auth/*` is gone before anything looks + * for a block opener. The verdicts do not change for the anchors either order + * preserves (`new AuditPlugin(` 2 → 1, `new SecurityPlugin(` 2 → 1 in + * `serve.ts`; 4 → 1 in `harness.ts` — the prose mentions dropped, the + * constructions kept), so this is a strictly wider view of the same subject. + * + * Approximate by design, and safe here: the result feeds only the literal + * searches below, so a `//` mangled out of a string literal + * (`'http://localhost:*'` in `serve.ts`) cannot affect a verdict. Do not reuse + * this for anything that reads string contents. + */ +function stripComments(source: string): string { + return source.replace(/(^|[^:])\/\/[^\n]*/g, '$1').replace(/\/\*[\s\S]*?\*\//g, ' '); +} + +const SERVE = stripComments(readBootPath('cli/src/commands/serve.ts')); +const HARNESS = stripComments(readBootPath('verify/src/harness.ts')); + +/** + * Every `new AuditPlugin(...)` construction in a file, with its argument text. + * + * Walks parentheses rather than matching `\(([^)]*)\)`, for the reason the + * sibling parity scan measured: an options argument is itself brace- and + * paren-bearing (`appAuditPluginOptions(config)`, `{ readAudit: { objects: [] } }`), + * and a non-nesting match stops at the first inner `)` and silently reports a + * truncation. The empty-argument case this file asserts today would be reported + * identically by both forms, which is exactly how a scan that cannot read the + * shape it guards passes until the day it matters. + */ +function auditPluginConstructions(source: string): string[] { + const NEW = 'new AuditPlugin('; + const found: string[] = []; + for (let i = source.indexOf(NEW); i !== -1; i = source.indexOf(NEW, i + 1)) { + let depth = 1; + let j = i + NEW.length; + for (; j < source.length && depth > 0; j++) { + if (source[j] === '(') depth++; + else if (source[j] === ')') depth--; + } + if (depth !== 0) throw new Error(`unbalanced \`${NEW}…\` at offset ${i} — the scan cannot read this file`); + found.push(source.slice(i + NEW.length, j - 1).trim()); + } + return found; +} + +/** + * The offset of an anchor that must appear exactly once. Both "missing" and + * "appeared twice" are reported as failures rather than folded into an offset + * comparison, because an ordering assertion between two anchors is meaningless + * if either is ambiguous — and a duplicated anchor is how a refactor most + * plausibly arrives. + */ +function soleOffset(source: string, anchor: string, role: string): number { + const first = source.indexOf(anchor); + if (first === -1) { + throw new Error( + `#9863 audit-registration scan: the ${role} anchor \`${anchor}\` is gone from serve.ts. ` + + 'It was the landmark this scan measured the AuditPlugin registration against. ' + + 'Repoint the anchor at whatever replaced it — the invariant (the CLI registration ' + + 'stays inside the auth-gated pair block and ABOVE the stack `plugins` loop) is unchanged.', + ); + } + if (source.indexOf(anchor, first + 1) !== -1) { + throw new Error( + `#9863 audit-registration scan: the ${role} anchor \`${anchor}\` now appears more than once ` + + 'in serve.ts, so "before" and "after" no longer name one place. Give this scan an ' + + 'unambiguous landmark before trusting its verdict.', + ); + } + return first; +} + +/** The `if (!hasAuthPlugin && tierEnabled('auth'))` block the audit pair lives in. */ +const AUTH_GATE = "if (!hasAuthPlugin && tierEnabled('auth'))"; +/** The stack `plugins` loop, i.e. where an app's own configured instance is registered. */ +const PLUGINS_LOOP = 'for (const plugin of plugins)'; + +describe('os serve registers AuditPlugin bare, above the stack `plugins` loop (#9863)', () => { + it('constructs AuditPlugin exactly once, with NO options — the ruling', () => { + // The empty string is the whole point: `[]` would mean "never constructed" + // and `['appAuditPluginOptions(config)']` would mean the ruling was reversed. + expect(auditPluginConstructions(SERVE)).toEqual(['']); + }); + + it('does not reach for a config-derived audit options helper', () => { + // Re-opening #9863 is allowed; doing it by accident is not. A helper wired + // in HERE would take effect only when the app supplies no AuthPlugin of its + // own and an auth secret is set (see the gate below) — a declared config key + // whose effect depends on unrelated auth conditions, on a compliance + // surface. If the ruling is revisited, the capability resolver's + // `CAPABILITY_PROVIDERS.audit` entry — which is NOT auth-gated and already + // carries the `configKey` mechanism `analytics` uses — is the site to argue + // about, and this assertion moves in the same edit as the ruling. + expect(SERVE).not.toContain('appAuditPluginOptions'); + }); + + it('registers inside the auth-gated pair block and ABOVE the stack `plugins` loop', () => { + const gate = soleOffset(SERVE, AUTH_GATE, 'auth-gate'); + const loop = soleOffset(SERVE, PLUGINS_LOOP, 'stack-plugins-loop'); + const audit = soleOffset(SERVE, 'new AuditPlugin(', 'audit-construction'); + + // ABOVE the loop: the half `serve.ts` calls load-bearing. Below it, the + // CLI's option-less instance would supersede the app's configured one and + // record-view auditing would be off wherever it had been opted in. + expect(audit).toBeLessThan(loop); + + // INSIDE the auth block: the half nothing had written down. The pair is + // registered by the `5d. Auto-register AuthPlugin (and paired + // Security/Audit)` branch, so an app that supplies its own AuthPlugin — or + // a production boot with no auth secret — gets NO CLI AuditPlugin, and the + // supersede this card is named after never happens there at all. Hoisting + // the registration out of the block is a real change of meaning, not a + // tidy-up, and it is exactly the edit an offset comparison against the gate + // catches. + expect(audit).toBeGreaterThan(gate); + }); + + it("verify's bootStack has no AuditPlugin opinion — no #7001-shaped disagreement to close", () => { + expect(auditPluginConstructions(HARNESS)).toEqual([]); + }); +}); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index cca735b3c2..9b8c0863c1 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -2525,10 +2525,58 @@ export default class Serve extends Command { // ⚠️ The dependency is on the ORDER as much as on the overwrite: // this registration must stay ABOVE the stack's `plugins` loop, or // the CLI's option-less instance would supersede the app's - // configured one instead. #9863 remains open on its own question — - // whether `os serve` should grow an `appAuditPluginOptions(config)` - // helper like its `SecurityPlugin` sibling above, rather than - // reaching the capability only through a supersede. + // configured one instead — silently turning record-view auditing + // back OFF for every deployment that had opted in. That order is no + // longer held by this comment alone: + // `serve-audit-registration.contract.test.ts` fails on the + // inversion, and on the two facts below. + // + // ⚠️ It is also AUTH-GATED, which nothing had written down. This + // pair is registered by the `5d. Auto-register AuthPlugin (and + // paired Security/Audit)` branch, so an app that supplies its own + // AuthPlugin — or a production boot with no auth secret, or a host + // kernel — never reaches this line at all. There the app's + // `plugins` entry is the ONLY AuditPlugin and no supersede happens. + // The supersede is how the opt-in survives one particular boot + // shape; it is not the mechanism the opt-in is built on. + // + // [#9863, ruled 2026-08-20] NO `appAuditPluginOptions(config)`. + // The open question was whether to mirror the `SecurityPlugin` + // helper six lines up. Four measurements say no: + // + // • #7001's REASON DOES NOT TRANSFER. That helper exists so two + // boot paths could not disagree about one plugin's options. + // `@objectstack/verify`'s `bootStack` constructs no AuditPlugin + // at all and does not depend on `@objectstack/plugin-audit`, so + // audit has exactly one boot path with an opinion and there is + // no disagreement for a shared helper to close. + // • IT WOULD HAVE NOTHING TO READ. `appSecurityPluginOptions` + // derives from `config.permissions`, an already-declared spec + // surface. There is no `audit` key in the stack schema and no + // object-metadata audit field, so an audit helper means minting + // an authorable surface, not reading one. + // • THAT SURFACE IS THE SHAPE #8992 ALREADY REFUSED for the + // object-metadata spelling (maintainer ruling 2026-08-16): a + // declaration that survives in a deployment which never + // installs the plugin — the import below is best-effort — i.e. + // config that READS as audited and records nothing. On a + // compliance surface that is worse than an absent feature. + // • IT WOULD BE A SECOND SURFACE THAT SILENTLY LOSES TO THE + // FIRST. An app setting both would have its config-derived + // options superseded by its own `plugins` entry, by the very + // contract above — a new footgun on the same capability. + // + // Measured pull for the surface: zero `readAudit` call sites in the + // repo outside `plugin-audit` itself. The reachability story is + // instead DOCUMENTED, now that #9864 made it a contract rather than + // the accident #9863 found: `content/docs/permissions/ + // record-view-auditing.mdx` and the plugin README both spell the + // `plugins`-array opt-in and the `Plugin superseded:` line it logs. + // + // If the ruling is ever revisited, the site to argue about is + // `Serve.CAPABILITY_PROVIDERS.audit` — the `requires:` resolver is + // NOT auth-gated and already carries the `configKey` mechanism + // `analytics` uses — not this line. try { const auditPkg = '@objectstack/plugin-audit'; const { AuditPlugin } = await import(/* webpackIgnore: true */ auditPkg); diff --git a/packages/plugins/plugin-audit/README.md b/packages/plugins/plugin-audit/README.md index a8fe565207..2e5aa3cc36 100644 --- a/packages/plugins/plugin-audit/README.md +++ b/packages/plugins/plugin-audit/README.md @@ -179,6 +179,33 @@ claim. - an empty (or fully excluded) set registers **no hook at all**, so a deployment that opts nothing in pays nothing on its read path. +### Turning it on under `os serve` + +"The place the plugin is installed" is a kernel you compose yourself in most of this +README. On the `objectstack serve` boot path it is your stack's `plugins` array: + +```typescript +import { defineStack } from '@objectstack/spec'; +import { AuditPlugin } from '@objectstack/plugin-audit'; + +const stack = defineStack({ + plugins: [new AuditPlugin({ readAudit: { objects: ['contact', 'account'] } })], +}); +``` + +The CLI may auto-register an option-less `AuditPlugin` of its own earlier in the same boot. +When it has, yours **supersedes** it: registering a plugin whose `name` is already taken +overwrites the earlier registration — last-one-wins, on both `ObjectKernel` and +`LiteKernel`, with a `warn` naming both versions, and the displaced instance never reaches +`init()`. That is a declared contract (`packages/core/src/plugin-registration.ts`), not an +ordering accident, so a `Plugin superseded: 'com.objectstack.audit'` line in the boot log +is this opt-in working. + +⛔ There is no stack-config key for the audited set, for the same reason there is no +object-metadata key: a key in `objectstack.config.ts` survives in a deployment that never +installs this package, and would read as coverage while recording nothing. `requires: +['audit']` loads the plugin but constructs it with no options — it cannot name objects. + ### Only record-detail views produce a row A read is recorded when **both** hold: From ae061f486b9585eca7e63e218fbef04aeb6388b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 19:14:43 +0000 Subject: [PATCH 2/4] test(cli): name the cross-package gate by its script, not by a path the scan never reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:cross-package-test-inputs` takes any quoted path literal without parsing, so a decorative mention of its own filename in a JSDoc block demanded a declaration for a file this test never opens — which would have put cli's whole suite on every edit of that gate. The comment now names the runnable script and records why. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --- .../serve-audit-registration.contract.test.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/serve-audit-registration.contract.test.ts b/packages/cli/src/commands/serve-audit-registration.contract.test.ts index 7e778b3f05..769de7461f 100644 --- a/packages/cli/src/commands/serve-audit-registration.contract.test.ts +++ b/packages/cli/src/commands/serve-audit-registration.contract.test.ts @@ -59,10 +59,16 @@ const HERE = path.dirname(fileURLToPath(import.meta.url)); * `packages/cli/src/commands/` → `packages/`. Reading verify's harness from * here is what makes fact 3 an assertion instead of a comment; * `@objectstack/verify` is a real dependency of this package and the read is - * test-only (tests never ship — `files: ["dist"]`). The glob is already - * declared for `@objectstack/cli` in `scripts/check-cross-package-test-inputs.mjs` - * and hashed by `@objectstack/cli#test` in turbo.json, for the sibling parity - * scan; this file adds a second reader of the same path, not a new radius. + * test-only (tests never ship — `files: ["dist"]`). The glob is already declared + * for `@objectstack/cli` by `pnpm check:cross-package-test-inputs`, and hashed + * by `@objectstack/cli#test` in turbo.json, for the sibling parity scan; this + * file adds a second reader of the same path, not a new radius. + * + * That gate is named by its runnable script rather than by its file path on + * purpose. Its literal collector takes any quoted path without parsing, so + * spelling the path here — in prose, about a file this test never opens — would + * demand a declaration for it and put cli's whole suite on every edit of the + * gate. The reads this scan really makes are the two below. */ const PACKAGES_DIR = path.resolve(HERE, '../../..'); From 57e4e5e5f5e13b29759690bde0704dbb58038bbd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 19:44:31 +0000 Subject: [PATCH 3/4] test(cli): separate code from prose with the shared masker, and declare the read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first draft of `serve-audit-registration.contract.test.ts` carried a private two-regex `stripComments` copied from this directory's older parity scans, block pass first. That is the defect class #9367 named: `serve.ts` has the route wildcard `/api/v1/auth/*` in a line comment, whose `/*` opens a phantom block comment running to the next real terminator ten lines below — deleting the `hasAuthPlugin` computation and the auth gate this scan measures against (1895 code-bearing lines survive the naive strip, 2098 survive the masker). `scripts/js-comment-mask.mjs` is the repo's one answer to that question, and it blanks rather than deletes, so the ordering assertion compares offsets into the real file. Masker and naive strip were cross-checked to agree on all four anchor counts across both subjects, which is also what rules out #10427's open desync for this pair. The import escapes the package, so it is declared for `@objectstack/cli` in the cross-package roster and hashed by `@objectstack/cli#test`. The gate did not demand it — its literal collector does not recognise an escaping relative import specifier — and that blind spot is filed separately rather than relied on. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --- .../serve-audit-registration.contract.test.ts | 67 ++++++++++--------- scripts/check-cross-package-test-inputs.mjs | 12 ++++ turbo.json | 3 +- 3 files changed, 48 insertions(+), 34 deletions(-) diff --git a/packages/cli/src/commands/serve-audit-registration.contract.test.ts b/packages/cli/src/commands/serve-audit-registration.contract.test.ts index 769de7461f..09b869ca68 100644 --- a/packages/cli/src/commands/serve-audit-registration.contract.test.ts +++ b/packages/cli/src/commands/serve-audit-registration.contract.test.ts @@ -52,6 +52,11 @@ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +// @ts-expect-error -- the repo's one comment/code separator (#9367) is a plain +// `.mjs` script with no type declarations. This file IS in cli's tsc program, +// so the suppression is a real one, not a phantom: delete the import and tsc +// reports the unused directive. +import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url)); @@ -92,47 +97,43 @@ function readBootPath(relative: string): string { } /** - * Comments stripped, because this scan is about what the two files DO. + * Comments MASKED, because this scan is about what the two files DO. * - * Not optional here, and not a copy of the sibling's caution: `serve.ts`'s audit - * block DESCRIBES the very construction being counted — it spells + * Not optional here, and not caution copied from the sibling scans: `serve.ts`'s + * audit block DESCRIBES the very construction being counted — it spells * `new AuditPlugin({ readAudit: … })` in prose to explain the app-side opt-in it - * documents, and it names `appAuditPluginOptions` to record that the helper was - * ruled against. Over raw text this file would count two constructions where - * the code has one, and its own ruling assertion would fail on the sentence - * stating the ruling. + * documents, and it names the helper by name to record that the helper was ruled + * against. Over raw text this file would count two constructions where the code + * has one, and its own ruling assertion would fail on the sentence stating the + * ruling. * - * ## Line comments FIRST, and that ordering is measured, not stylistic + * ## Why the SHARED masker rather than a private `stripComments` * - * This directory's two older parity scans run the block pass first. On - * `serve.ts` that pass is not conservative — it is destructive. The `5d.` - * header comment contains the URL glob `/api/v1/auth/*`, whose `/*` opens a - * block comment as far as a regex is concerned; the next block-comment close - * in the file is - * the closing of `import(/* webpackIgnore: true *\/)` ten lines below, so the - * pass silently deletes the whole intervening region — the `hasAuthPlugin` - * computation and the `if (!hasAuthPlugin && tierEnabled('auth'))` gate this - * file measures against. Measured over the two files this scan reads: - * block-first keeps 1895 code-bearing lines of `serve.ts`, line-first keeps - * 2098. The 203-line difference is code, not prose. + * Because the private ones are a measured defect class here (#9367), and this + * file walked straight into it. Its first draft used the two-regex strip the two + * older scans in this directory still carry, block pass first — and `serve.ts` + * has a route wildcard in a line comment (`/api/v1/auth/*` in the `5d.` header). + * A regex cannot tell that `/*` from a real opener, so it ran a phantom block + * comment to the next real terminator ten lines below, inside + * `import(/* webpackIgnore: true *\/ …)`, deleting the `hasAuthPlugin` + * computation and the auth gate this file measures against. Measured on this + * exact pair: the naive strip keeps 1895 code-bearing lines of `serve.ts`, the + * masker keeps 2098. * - * Stripping `//` runs first, so `/api/v1/auth/*` is gone before anything looks - * for a block opener. The verdicts do not change for the anchors either order - * preserves (`new AuditPlugin(` 2 → 1, `new SecurityPlugin(` 2 → 1 in - * `serve.ts`; 4 → 1 in `harness.ts` — the prose mentions dropped, the - * constructions kept), so this is a strictly wider view of the same subject. + * `maskComments` also BLANKS rather than deletes — spans become spaces, newlines + * kept — so the offsets the ordering assertion below compares are offsets into + * the real file rather than into a shrunken copy of it. Cross-checked on both + * subjects: masker and naive-strip agree on all four anchor counts, and the + * masker leaves the line count identical (4638 → 4638), which is the property + * being bought. * - * Approximate by design, and safe here: the result feeds only the literal - * searches below, so a `//` mangled out of a string literal - * (`'http://localhost:*'` in `serve.ts`) cannot affect a verdict. Do not reuse - * this for anything that reads string contents. + * #10427 (open) has the masker desyncing on nested template literals in 16 + * files. Neither file read here is among them, and the cross-check above is what + * says so rather than assuming it. */ -function stripComments(source: string): string { - return source.replace(/(^|[^:])\/\/[^\n]*/g, '$1').replace(/\/\*[\s\S]*?\*\//g, ' '); -} -const SERVE = stripComments(readBootPath('cli/src/commands/serve.ts')); -const HARNESS = stripComments(readBootPath('verify/src/harness.ts')); +const SERVE = maskComments(readBootPath('cli/src/commands/serve.ts')); +const HARNESS = maskComments(readBootPath('verify/src/harness.ts')); /** * Every `new AuditPlugin(...)` construction in a file, with its argument text. diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index ff30622781..76e7ae228e 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -242,6 +242,17 @@ const CROSS_PACKAGE_TEST_INPUTS = { // designed trade (over-collection can only widen a radius, never narrow one), // and declaring one rarely-touched file is cheaper than teaching the scanner to // tell prose from code, or than rewording a comment to dodge a scanner. + // + // `js-comment-mask.mjs` is the first entry declared for an IMPORT rather than + // a file read: src/commands/serve-audit-registration.contract.test.ts imports + // `maskComments` from it to separate code from prose in the two boot paths it + // scans (#9863). This gate did NOT demand the declaration -- its literal + // collector recognises path-shaped reads, and a relative import specifier + // that escapes the package is not one of the spellings it knows. Declared by + // hand because the coupling is real whatever the collector saw: the scan's + // verdict is a function of that module's masking behaviour, so a change to it + // has to re-run cli's suite. The undetected-import spelling is filed + // separately; widening a radius by hand is never the reason not to. globs: [ 'packages/verify/src/**', 'packages/plugins/plugin-security/src/**', @@ -254,6 +265,7 @@ const CROSS_PACKAGE_TEST_INPUTS = { 'content/docs/deployment/index.mdx', 'content/docs/permissions/authentication.mdx', 'scripts/check-nul-bytes.mjs', + 'scripts/js-comment-mask.mjs', ], }, '@objectstack/lint': { diff --git a/turbo.json b/turbo.json index 568f6e1690..a01449251d 100644 --- a/turbo.json +++ b/turbo.json @@ -72,7 +72,8 @@ "$TURBO_ROOT$/content/docs/deployment/cli.mdx", "$TURBO_ROOT$/content/docs/deployment/index.mdx", "$TURBO_ROOT$/content/docs/permissions/authentication.mdx", - "$TURBO_ROOT$/scripts/check-nul-bytes.mjs" + "$TURBO_ROOT$/scripts/check-nul-bytes.mjs", + "$TURBO_ROOT$/scripts/js-comment-mask.mjs" ] }, "@objectstack/lint#test": { From 983a292fc26da1f691a893077bd04a2ece97dc16 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 03:13:58 +0000 Subject: [PATCH 4/4] fix(cli): drop the now-unused @ts-expect-error on the comment-mask import The suppression was real when this branch was cut: `scripts/js-comment-mask.mjs` had no type declarations, so importing it from cli's tsc program was TS7016. #10398 then landed `scripts/js-comment-mask.d.mts` on main -- adding types for the same import from `packages/spec/scripts/`, which #5475 had put inside a tsc program. Merged with main the import type-checks, the directive becomes unused, and `tsc --noEmit` fails with TS2578. That is why this PR's own CI was green while its merge-queue build was not: the two trees genuinely differed. Verified both ways: cli typecheck passes on the branch as-is, fails with `serve-audit-registration.contract.test.ts(55,1): error TS2578` once main is merged in, and passes again with the directive removed. The comment is rewritten rather than deleted so the next reader does not restore a directive that now breaks the build. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --- .../commands/serve-audit-registration.contract.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/serve-audit-registration.contract.test.ts b/packages/cli/src/commands/serve-audit-registration.contract.test.ts index 09b869ca68..3d8e4f8e87 100644 --- a/packages/cli/src/commands/serve-audit-registration.contract.test.ts +++ b/packages/cli/src/commands/serve-audit-registration.contract.test.ts @@ -52,10 +52,10 @@ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -// @ts-expect-error -- the repo's one comment/code separator (#9367) is a plain -// `.mjs` script with no type declarations. This file IS in cli's tsc program, -// so the suppression is a real one, not a phantom: delete the import and tsc -// reports the unused directive. +// The separator (#9367) is a plain `.mjs`, but it ships a hand-written `.d.mts` +// declaration alongside it (#10398), so this import is typed and needs no +// suppression. A `@ts-expect-error` here is an UNUSED directive, and cli's tsc +// program does include this file, so tsc fails the build on one. import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url));