test(cli): mask comments before scanning serve.ts in two contract/pin scans

packages/cli/src/commands/serve-email-config-parity.contract.test.ts

@@ -29,8 +29,12 @@
 // This package's `tsconfig.json` includes `src` (tests and all), so the
 // compile-time witness below is real: `pnpm --filter @objectstack/cli
 // typecheck` fails on a config the schema cannot express, before any test runs.
-// The spec-side companion (`packages/spec/src/system/email-config.test.ts`) has
-// to be runtime-only — that package excludes `**/*.test.ts` from its tsconfig
+// The spec-side companion — `@objectstack/spec`'s own `system/email-config.test.ts`,
+// named without its repo-relative path here for the reason this directory's
+// `serve-multi-node-cap-advisory.pin.test.ts` gives for the same gate: a quoted
+// literal starting at `packages/` would force `check:cross-package-test-inputs`
+// to demand a glob for a file this test never actually reads — has to be
+// runtime-only, since that package excludes `**/*.test.ts` from its tsconfig
 // (#5286).
 
 import { describe, it, expect } from 'vitest';
@@ -40,12 +44,26 @@ import { fileURLToPath } from 'node:url';
 import { EmailServiceConfigSchema } from '@objectstack/spec/system';
 import type { EmailServiceConfig } from '@objectstack/spec/system';
 import { resolveEmailCapabilityArg } from './serve.js';
+// The repo's one comment/code separator (#9367). This file used to scan RAW
+// source with no separator at all (#10514): a docblock or a `// TODO: also
+// read cfgEmail.foo` line was indistinguishable from a real dot access. Typed
+// by the hand-written `scripts/js-comment-mask.d.mts` next to it (this
+// package's `tsconfig.json` includes `src`), so this import needs no
+// suppression.
+import { maskComments } from '../../../../scripts/js-comment-mask.mjs';
 
 const SERVE_SOURCE = readFileSync(
  path.resolve(path.dirname(fileURLToPath(import.meta.url)), 'serve.ts'),
  'utf8',
 );
 
+/**
+ * `serve.ts` with every comment span blanked (offsets preserved, bytes
+ * replaced with spaces) — see `keysReadFromConfigEmail` below for why the
+ * scan reads this instead of `SERVE_SOURCE` directly (#10514).
+ */
+const MASKED_SERVE_SOURCE = maskComments(SERVE_SOURCE);
+
 /**
  * Every `config.email` key the resolver reads, straight from its source — the
  * issue's own repro command:
@@ -57,9 +75,14 @@ const SERVE_SOURCE = readFileSync(
  * keys), which is what makes a source scan an exact measure rather than an
  * approximation. Should that ever stop being true, this comment is the place
  * the next reader learns the scan has to change with it.
+ *
+ * Takes `source` explicitly (default the real file's masked text, #10514)
+ * rather than closing over `SERVE_SOURCE`/`MASKED_SERVE_SOURCE` directly, so
+ * the vacuity-proof tests below can drive the exact same regex over a raw vs.
+ * a masked variant of a shape and show the two disagree.
  */
-function keysReadFromConfigEmail(): string[] {
- const reads = SERVE_SOURCE.match(/cfgEmail\.[A-Za-z_$][\w$]*/g) ?? [];
+function keysReadFromConfigEmail(source: string = MASKED_SERVE_SOURCE): string[] {
+ const reads = source.match(/cfgEmail\.[A-Za-z_$][\w$]*/g) ?? [];
  return [...new Set(reads.map((r) => r.slice('cfgEmail.'.length)))].sort();
 }
 
@@ -127,6 +150,52 @@ describe('EmailServiceConfigSchema ↔ resolveEmailCapabilityArg', () => {
  });
 });
 
+/**
+ * Vacuity proof (#10514): both directions the raw scan was one ordinary
+ * comment away from getting wrong, reproduced on synthetic sources shaped
+ * like the real resolver so the two legs (raw vs. masked) can be compared
+ * without waiting for `serve.ts` to actually regress. Each `it` shows the RAW
+ * leg producing the wrong verdict — the verdict this file's scan would have
+ * produced before #10514 — and the MASKED leg producing the right one.
+ */
+describe('the key scan ignores prose that looks like a cfgEmail read (#10514)', () => {
+ it('does not let a comment fabricate an undeclared key ("declares every key" direction)', () => {
+ const synthetic = [
+ 'function resolveEmailCapabilityArg(cfgEmail = {}) {',
+ ' // TODO: someday also read cfgEmail.bogusKey from the config',
+ " const provider = cfgEmail.provider;",
+ ' return { provider };',
+ '}',
+ ].join('\n');
+
+ // Pre-#10514 (raw): the comment's `cfgEmail.bogusKey` is indistinguishable
+ // from a real dot access — this is what would have made "declares every
+ // config.email key the resolver reads" go RED over a comment alone.
+ expect(keysReadFromConfigEmail(synthetic)).toEqual(['bogusKey', 'provider']);
+ // Post-#10514 (masked): the comment is blanked, so only the real read
+ // survives.
+ expect(keysReadFromConfigEmail(maskComments(synthetic))).toEqual(['provider']);
+ });
+
+ it('does not let a comment fabricate a read of a declared-but-unread key — the #5447 shape, "the worse one" ("reads every key" direction)', () => {
+ const synthetic = [
+ 'function resolveEmailCapabilityArg(cfgEmail = {}) {',
+ " const provider = cfgEmail.provider;",
+ ' // historically we also honoured cfgEmail.persist here',
+ ' return { provider };',
+ '}',
+ ].join('\n');
+
+ // Pre-#10514 (raw): the comment alone counts as a "read" of `persist` —
+ // silently restoring the exact `DECLARED_BUT_UNREAD` exemption this
+ // file's docblock (above) says was deleted for good after #5447/#5470.
+ expect(keysReadFromConfigEmail(synthetic)).toContain('persist');
+ // Post-#10514 (masked): the comment does not count, so a schema key with
+ // no real reader still reads as unread here.
+ expect(keysReadFromConfigEmail(maskComments(synthetic))).not.toContain('persist');
+ });
+});
+
 /**
  * Compile-time half — the author-facing symptom #5307 is written about. Every
  * value here is one the runtime has honoured since #5160; before the

packages/cli/src/commands/serve-multi-node-cap-advisory.pin.test.ts

@@ -34,6 +34,15 @@ import { describe, it, expect } from 'vitest';
 import { readFileSync } from 'node:fs';
 import { dirname, join, resolve } from 'node:path';
 import { fileURLToPath } from 'node:url';
+// The repo's one comment/code separator (#9367). The four shape assertions
+// below used to match against RAW `SERVE_SOURCE` (#10514): a trailing comment
+// describing the old call shape (e.g. quoting a reverted
+// `checkMultiNodeAllowed(replicas)`) was indistinguishable from the real call.
+// `interfaceFields()` further down does its own narrower, brace-matched strip
+// over an `export interface` body and is deliberately left alone — out of
+// scope for #10514, noted there so a future re-derivation doesn't read it as
+// the same defect.
+import { maskComments } from '../../../../scripts/js-comment-mask.mjs';
 
 const HERE = dirname(fileURLToPath(import.meta.url));
 
@@ -43,6 +52,15 @@ const REPO_ROOT = resolve(HERE, '../../../..');
 /** `packages/cli/src/commands/serve.ts` — the consumer. */
 const SERVE_SOURCE = readFileSync(resolve(HERE, 'serve.ts'), 'utf8');
 
+/**
+ * `SERVE_SOURCE` with every comment span blanked (offsets preserved) — what
+ * the four shape assertions below actually match against (#10514), so a
+ * comment naming `checkMultiNodeAllowed(…)` cannot satisfy — or hide behind —
+ * any of them. `interfaceFields()` still reads raw `SERVE_SOURCE`; see the
+ * import comment above for why that is out of scope here.
+ */
+const MASKED_SERVE_SOURCE = maskComments(SERVE_SOURCE);
+
 /**
  * The producer, read from source rather than imported: the CLI has no
  * dependency on this package (that is the whole reason the cast exists), and
@@ -115,20 +133,20 @@ describe('os serve ↔ multi-node gate', () => {
  it('calls the gate WITH a requested node count', () => {
  // The exact regression: `checkMultiNodeAllowed()`. Passing nothing makes the
  // licensed-overflow verdict unreachable rather than merely unread.
- expect(SERVE_SOURCE).not.toMatch(/checkMultiNodeAllowed\(\s*\)/);
- expect(SERVE_SOURCE).toMatch(/checkMultiNodeAllowed\(\s*[^)\s]/);
+ expect(MASKED_SERVE_SOURCE).not.toMatch(/checkMultiNodeAllowed\(\s*\)/);
+ expect(MASKED_SERVE_SOURCE).toMatch(/checkMultiNodeAllowed\(\s*[^)\s]/);
  });
 
  it('passes the operator-declared replica count', () => {
  // A stated decision, not an accident: `OS_CLUSTER_REPLICAS` is a *declared*
  // desired count, identical in every replica, not a live membership count —
  // which is right for an advisory message about the operator's own
  // configuration, and is NOT sufficient for enforcement.
- expect(SERVE_SOURCE).toMatch(/checkMultiNodeAllowed\(\s*Number\(process\.env\.OS_CLUSTER_REPLICAS\)\s*\)/);
+ expect(MASKED_SERVE_SOURCE).toMatch(/checkMultiNodeAllowed\(\s*Number\(process\.env\.OS_CLUSTER_REPLICAS\)\s*\)/);
  });
 
  it('types the dynamic import with the mirrored verdict, not an inline literal', () => {
- expect(SERVE_SOURCE).toMatch(
+ expect(MASKED_SERVE_SOURCE).toMatch(
  /checkMultiNodeAllowed:\s*\(requested\?:\s*number\)\s*=>\s*MultiNodeGateVerdict/,
  );
  });
@@ -152,3 +170,34 @@ describe('os serve ↔ multi-node gate', () => {
  ).toEqual(producer);
  });
 });
+
+/**
+ * Vacuity proof (#10514): a synthetic regression shaped exactly like the
+ * issue's own repro — the zero-arg call reintroduced, with a trailing comment
+ * quoting the OLD argued call, the way a careless revert reads. Both legs are
+ * shown so the RAW leg's wrong verdict — what this pin's assertions would
+ * have produced before #10514 — is visible next to the MASKED leg's correct
+ * one, not just asserted.
+ */
+describe('the shape assertions ignore a comment that quotes the old call (#10514)', () => {
+ it('a reverted zero-arg call cannot hide behind a comment describing the argued call it replaced', () => {
+ const regressed = [
+ 'const __gate = checkMultiNodeAllowed();',
+ '// checkMultiNodeAllowed(Number(process.env.OS_CLUSTER_REPLICAS)) used to be called here',
+ ].join('\n');
+
+ // Pre-#10514 (raw): the negative assertion correctly catches the bad
+ // shape…
+ expect(regressed).toMatch(/checkMultiNodeAllowed\(\s*\)/);
+ // …but the positive assertion is ALSO satisfied — by the comment alone —
+ // which is exactly how this pin's "calls the gate WITH a requested node
+ // count" test would have stayed green over the regression it exists to
+ // catch.
+ expect(regressed).toMatch(/checkMultiNodeAllowed\(\s*[^)\s]/);
+
+ // Post-#10514 (masked): the comment is blanked, so the positive assertion
+ // correctly fails to find an argued call — the regression is no longer
+ // hidden.
+ expect(maskComments(regressed)).not.toMatch(/checkMultiNodeAllowed\(\s*[^)\s]/);
+ });
+});