From d3d8f7b117fd1421905b1d9d9172d99beec18135 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 08:35:09 +0000 Subject: [PATCH 1/6] test: pin declared mirror parity (#4175) --- .changelog/next/fixed-issue-4175.md | 1 + server/lib/appIdentity.mirror.test.js | 9 +++ server/lib/issueLength.mirror.test.js | 55 ++++++++++++++ server/lib/mirrorCoverage.test.js | 89 +++++++++++++++++++++++ server/lib/scenePrompt.test.js | 36 +++++++++ server/lib/seasonStructure.mirror.test.js | 23 ++++++ server/lib/shotGrammar.mirror.test.js | 22 ++++++ 7 files changed, 235 insertions(+) create mode 100644 .changelog/next/fixed-issue-4175.md create mode 100644 server/lib/appIdentity.mirror.test.js create mode 100644 server/lib/issueLength.mirror.test.js create mode 100644 server/lib/mirrorCoverage.test.js create mode 100644 server/lib/seasonStructure.mirror.test.js create mode 100644 server/lib/shotGrammar.mirror.test.js diff --git a/.changelog/next/fixed-issue-4175.md b/.changelog/next/fixed-issue-4175.md new file mode 100644 index 0000000000..db0da3b767 --- /dev/null +++ b/.changelog/next/fixed-issue-4175.md @@ -0,0 +1 @@ +- Server and client mirror contracts now detect missing parity coverage before drift reaches users. diff --git a/server/lib/appIdentity.mirror.test.js b/server/lib/appIdentity.mirror.test.js new file mode 100644 index 0000000000..53af99262a --- /dev/null +++ b/server/lib/appIdentity.mirror.test.js @@ -0,0 +1,9 @@ +import { describe, expect, it } from 'vitest'; +import { PORTOS_APP_ID as serverAppId } from './appIdentity.js'; +import { PORTOS_APP_ID as clientAppId } from '../../client/src/lib/appIdentity.js'; + +describe('appIdentity — server/client mirror parity', () => { + it('keeps the baseline app id identical', () => { + expect(clientAppId).toBe(serverAppId); + }); +}); diff --git a/server/lib/issueLength.mirror.test.js b/server/lib/issueLength.mirror.test.js new file mode 100644 index 0000000000..b49ae390b9 --- /dev/null +++ b/server/lib/issueLength.mirror.test.js @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest'; +import { + CUSTOM_MINUTE_MAX as serverMinuteMax, + CUSTOM_MINUTE_MIN as serverMinuteMin, + CUSTOM_PAGE_MAX as serverPageMax, + CUSTOM_PAGE_MIN as serverPageMin, + DEFAULT_LENGTH_PROFILE as serverDefaultProfile, + LENGTH_PROFILES as serverProfiles, +} from './issueLength.js'; +import { + CUSTOM_MINUTE_MAX as clientMinuteMax, + CUSTOM_MINUTE_MIN as clientMinuteMin, + CUSTOM_PAGE_MAX as clientPageMax, + CUSTOM_PAGE_MIN as clientPageMin, + DEFAULT_LENGTH_PROFILE as clientDefaultProfile, + LENGTH_PROFILES as clientProfiles, +} from '../../client/src/lib/issueLength.js'; + +describe('issueLength — server/client picker parity', () => { + it('keeps the profiles the client displays aligned with server targets', () => { + const serverPickerProfiles = Object.fromEntries(Object.entries(serverProfiles).map(([id, profile]) => [ + id, + { + label: profile.label, + pageTarget: profile.pageTarget, + minutesTarget: profile.minutesTarget, + }, + ])); + const clientPickerProfiles = Object.fromEntries(Object.entries(clientProfiles).map(([id, profile]) => [ + id, + { + label: profile.label, + pageTarget: profile.pageTarget, + minutesTarget: profile.minutesTarget, + }, + ])); + + expect(clientPickerProfiles).toEqual(serverPickerProfiles); + expect(clientDefaultProfile).toBe(serverDefaultProfile); + }); + + it('keeps every custom-override bound identical', () => { + expect({ + pageMin: clientPageMin, + pageMax: clientPageMax, + minuteMin: clientMinuteMin, + minuteMax: clientMinuteMax, + }).toEqual({ + pageMin: serverPageMin, + pageMax: serverPageMax, + minuteMin: serverMinuteMin, + minuteMax: serverMinuteMax, + }); + }); +}); diff --git a/server/lib/mirrorCoverage.test.js b/server/lib/mirrorCoverage.test.js new file mode 100644 index 0000000000..3a9b829455 --- /dev/null +++ b/server/lib/mirrorCoverage.test.js @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync, readdirSync } from 'fs'; +import { basename, dirname, join } from 'path'; +import { fileURLToPath } from 'url'; + +const here = dirname(fileURLToPath(import.meta.url)); +const CLIENT_LIB = join(here, '../../client/src/lib'); +const CLIENT_README = join(CLIENT_LIB, 'README.md'); +const SERVER_README = join(here, 'README.md'); + +function listTestFiles(dir) { + return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const path = join(dir, entry.name); + if (entry.isDirectory()) return listTestFiles(path); + return entry.name.endsWith('.test.js') ? [path] : []; + }); +} + +function listedMirrorPairs(readme) { + const rows = [...readme.matchAll(/^\|\s+`([^`]+\.js)`\s+\|\s+(.+)\|$/gm)]; + const pairs = rows.flatMap(([, clientFile, description]) => { + if (!/\bmirror/i.test(description)) return []; + const serverMatch = description.match(/server\/lib\/([\w/-]+\.js)/); + if (!serverMatch || serverMatch[1].includes('/') || clientFile !== basename(serverMatch[1])) return []; + return [{ clientFile, serverFile: serverMatch[1] }]; + }); + return pairs; +} + +function listedServerMirrorPairs(readme) { + const rows = [...readme.matchAll(/^\|\s+`([^`]+\.js)`\s+\|\s+(.+)\|$/gm)]; + const pairs = rows.flatMap(([, serverFile, description]) => { + if (!/\bmirror/i.test(description)) return []; + const clientMatch = description.match(/client\/src\/lib\/([\w/-]+\.js)/); + if (!clientMatch || clientMatch[1].includes('/') || serverFile !== basename(clientMatch[1])) return []; + return [{ clientFile: clientMatch[1], serverFile }]; + }); + return pairs; +} + +function uniquePairs(pairs) { + return [...new Map(pairs.map((pair) => [`${pair.serverFile}:${pair.clientFile}`, pair])).values()]; +} + +function missingParityPins(pairs, testSources) { + return pairs.filter(({ clientFile, serverFile }) => { + const serverName = basename(serverFile); + return !testSources.some(({ path, source }) => ( + path !== fileURLToPath(import.meta.url) + && (source.includes(`client/src/lib/${clientFile}`) || source.includes(`'./${clientFile}'`)) + && source.includes(serverName) + )); + }); +} + +describe('declared server/client mirror coverage', () => { + const readme = readFileSync(CLIENT_README, 'utf8'); + const pairs = uniquePairs([ + ...listedMirrorPairs(readme), + ...listedServerMirrorPairs(readFileSync(SERVER_README, 'utf8')), + ]); + const testSources = [...listTestFiles(here), ...listTestFiles(CLIENT_LIB)].map((path) => ({ + path, + source: readFileSync(path, 'utf8'), + })); + + it('finds direct same-name mirror declarations in the client catalog', () => { + expect(pairs).toContainEqual({ clientFile: 'seasonStructure.js', serverFile: 'seasonStructure.js' }); + expect(pairs).toContainEqual({ clientFile: 'shotGrammar.js', serverFile: 'shotGrammar.js' }); + expect(pairs).toContainEqual({ clientFile: 'appIdentity.js', serverFile: 'appIdentity.js' }); + expect(pairs).toContainEqual({ clientFile: 'issueLength.js', serverFile: 'issueLength.js' }); + }); + + it('also includes direct same-name declarations from the server catalog', () => { + expect(pairs).toContainEqual({ clientFile: 'catalogTypes.js', serverFile: 'catalogTypes.js' }); + }); + + it('requires every declared direct mirror to have a test that reads both copies', () => { + const missing = missingParityPins(pairs, testSources); + expect(missing, `missing parity pins: ${missing.map(({ clientFile }) => clientFile).join(', ')}`).toEqual([]); + }); + + it('reports a synthetic declared mirror when no test reads both copies', () => { + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + expect(missingParityPins(synthetic, [])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); +}); diff --git a/server/lib/scenePrompt.test.js b/server/lib/scenePrompt.test.js index f05b4d1076..37c3f4c59e 100644 --- a/server/lib/scenePrompt.test.js +++ b/server/lib/scenePrompt.test.js @@ -1,4 +1,7 @@ import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; import { normalizeSlugline, normCharKey, @@ -9,6 +12,11 @@ import { buildScenePrompt, __testing, } from './scenePrompt.js'; +import { compareDeclaration } from './mirrorParity.js'; + +const here = dirname(fileURLToPath(import.meta.url)); +const SERVER_COPY = join(here, 'scenePrompt.js'); +const CLIENT_COPY = join(here, '../../client/src/lib/scenePrompt.js'); describe('scenePrompt — normalizeSlugline', () => { it('collapses em/en/hyphen + punctuation + spaces so equivalent sluglines match', () => { @@ -307,3 +315,31 @@ describe('scenePrompt — buildScenePrompt wardrobe appearances', () => { expect(out).not.toContain('Wearing:'); }); }); + +describe('scenePrompt — server/client mirror parity', () => { + const server = readFileSync(SERVER_COPY, 'utf8'); + const client = readFileSync(CLIENT_COPY, 'utf8'); + const mirroredDeclarations = [ + 'PROMPT_MAX', + 'normalizeSlugline', + 'normCharKey', + 'buildCharByKey', + 'matchSceneCharacters', + 'matchCharactersInText', + 'buildPlaceByKey', + 'matchScenePlace', + 'matchEntriesByCandidates', + 'matchPlacesInText', + 'matchObjectsInText', + 'appendWardrobe', + 'buildScenePrompt', + ]; + + for (const name of mirroredDeclarations) { + it(`keeps ${name} identical`, () => { + const { clientDecl, serverNorm, clientNorm } = compareDeclaration(server, client, name); + expect(clientDecl, `client/src/lib/scenePrompt.js is missing ${name}`).not.toBeNull(); + expect(clientNorm).toBe(serverNorm); + }); + } +}); diff --git a/server/lib/seasonStructure.mirror.test.js b/server/lib/seasonStructure.mirror.test.js new file mode 100644 index 0000000000..17f34d4612 --- /dev/null +++ b/server/lib/seasonStructure.mirror.test.js @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'fs'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; +import { compareDeclaration } from './mirrorParity.js'; + +const here = dirname(fileURLToPath(import.meta.url)); +const SERVER_COPY = join(here, 'seasonStructure.js'); +const CLIENT_COPY = join(here, '../../client/src/lib/seasonStructure.js'); +const MIRRORED_DECLARATIONS = ['pickSeasonCount', 'recommendStructure', 'describeStructure']; + +describe('seasonStructure — server/client mirror parity', () => { + const server = readFileSync(SERVER_COPY, 'utf8'); + const client = readFileSync(CLIENT_COPY, 'utf8'); + + for (const name of MIRRORED_DECLARATIONS) { + it(`keeps ${name} identical`, () => { + const { clientDecl, serverNorm, clientNorm } = compareDeclaration(server, client, name); + expect(clientDecl, `client/src/lib/seasonStructure.js is missing ${name}`).not.toBeNull(); + expect(clientNorm).toBe(serverNorm); + }); + } +}); diff --git a/server/lib/shotGrammar.mirror.test.js b/server/lib/shotGrammar.mirror.test.js new file mode 100644 index 0000000000..11bf69b819 --- /dev/null +++ b/server/lib/shotGrammar.mirror.test.js @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'fs'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; +import { compareDeclaration } from './mirrorParity.js'; + +const here = dirname(fileURLToPath(import.meta.url)); +const SERVER_COPY = join(here, 'shotGrammar.js'); +const CLIENT_COPY = join(here, '../../client/src/lib/shotGrammar.js'); + +describe('shotGrammar — server/client vocabulary parity', () => { + const server = readFileSync(SERVER_COPY, 'utf8'); + const client = readFileSync(CLIENT_COPY, 'utf8'); + + for (const name of ['SHOT_TYPES', 'SCREEN_DIRECTIONS']) { + it(`keeps ${name} identical`, () => { + const { clientDecl, serverNorm, clientNorm } = compareDeclaration(server, client, name); + expect(clientDecl, `client/src/lib/shotGrammar.js is missing ${name}`).not.toBeNull(); + expect(clientNorm).toBe(serverNorm); + }); + } +}); From a70086d8febda3bdb9c6bfdcee1af59c3af2ae61 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 07:29:29 -0700 Subject: [PATCH 2/6] simplify: dedup listedMirrorPairs/listedServerMirrorPairs into one helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both functions parsed README mirror-declaration table rows with identical row-matching and mirror-description-filter logic, differing only in which side (client vs server) was "this file" vs. "the other file it mirrors" — exactly the duplication mirrorParity.js exists to prevent one file over. --- server/lib/mirrorCoverage.test.js | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/server/lib/mirrorCoverage.test.js b/server/lib/mirrorCoverage.test.js index 3a9b829455..bca3641e16 100644 --- a/server/lib/mirrorCoverage.test.js +++ b/server/lib/mirrorCoverage.test.js @@ -16,26 +16,29 @@ function listTestFiles(dir) { }); } -function listedMirrorPairs(readme) { +// Both catalogs declare mirrors the same way — a backtick-fenced filename in +// column 1, a description in column 2 naming the counterpart path — differing +// only in which side is "this file" vs. "the other file it mirrors". One +// parameterized walker keeps that row-parsing logic from drifting between the +// two catalogs the way the mirrored declarations it guards must not drift. +function listedPairsFor(readme, otherPathRe) { const rows = [...readme.matchAll(/^\|\s+`([^`]+\.js)`\s+\|\s+(.+)\|$/gm)]; - const pairs = rows.flatMap(([, clientFile, description]) => { + return rows.flatMap(([, thisFile, description]) => { if (!/\bmirror/i.test(description)) return []; - const serverMatch = description.match(/server\/lib\/([\w/-]+\.js)/); - if (!serverMatch || serverMatch[1].includes('/') || clientFile !== basename(serverMatch[1])) return []; - return [{ clientFile, serverFile: serverMatch[1] }]; + const otherMatch = description.match(otherPathRe); + if (!otherMatch || otherMatch[1].includes('/') || thisFile !== basename(otherMatch[1])) return []; + return [{ thisFile, otherFile: otherMatch[1] }]; }); - return pairs; +} + +function listedMirrorPairs(readme) { + return listedPairsFor(readme, /server\/lib\/([\w/-]+\.js)/) + .map(({ thisFile, otherFile }) => ({ clientFile: thisFile, serverFile: otherFile })); } function listedServerMirrorPairs(readme) { - const rows = [...readme.matchAll(/^\|\s+`([^`]+\.js)`\s+\|\s+(.+)\|$/gm)]; - const pairs = rows.flatMap(([, serverFile, description]) => { - if (!/\bmirror/i.test(description)) return []; - const clientMatch = description.match(/client\/src\/lib\/([\w/-]+\.js)/); - if (!clientMatch || clientMatch[1].includes('/') || serverFile !== basename(clientMatch[1])) return []; - return [{ clientFile: clientMatch[1], serverFile }]; - }); - return pairs; + return listedPairsFor(readme, /client\/src\/lib\/([\w/-]+\.js)/) + .map(({ thisFile, otherFile }) => ({ clientFile: otherFile, serverFile: thisFile })); } function uniquePairs(pairs) { From 6c7f49ef017d1241a1c6b07819fdb654647a4676 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 07:39:06 -0700 Subject: [PATCH 3/6] address review (claude): fix vacuous guard in missingParityPins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare './' fallback matched any test file's source containing that literal substring, without checking WHERE the test lived. For a direct mirror, clientFile and serverFile are the same string, so a plain server-only unit test (e.g. bareUrl.test.js importing './bareUrl.js') trivially satisfied both substring checks and was wrongly counted as a parity pin — verified by simulating removal of bareUrl.mirror.test.js, which left missingParityPins reporting no gap. Restrict the './' form to test files that actually live under client/src/lib, and add a bypass-probe test pinning the fix. --- server/lib/mirrorCoverage.test.js | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/server/lib/mirrorCoverage.test.js b/server/lib/mirrorCoverage.test.js index bca3641e16..42944d8ac9 100644 --- a/server/lib/mirrorCoverage.test.js +++ b/server/lib/mirrorCoverage.test.js @@ -50,7 +50,14 @@ function missingParityPins(pairs, testSources) { const serverName = basename(serverFile); return !testSources.some(({ path, source }) => ( path !== fileURLToPath(import.meta.url) - && (source.includes(`client/src/lib/${clientFile}`) || source.includes(`'./${clientFile}'`)) + // The bare `'./'` form only proves "reads the client copy" when the + // test itself lives in client/src/lib — a same-name server-only unit test + // (e.g. bareUrl.test.js importing './bareUrl.js') would otherwise satisfy + // this on its own, since serverName and clientFile are identical strings + // for a direct mirror. Without the directory check, deleting the actual + // parity-pinning *.mirror.test.js leaves this guard silently reporting + // no missing pins. + && (source.includes(`client/src/lib/${clientFile}`) || (path.startsWith(CLIENT_LIB) && source.includes(`'./${clientFile}'`))) && source.includes(serverName) )); }); @@ -89,4 +96,20 @@ describe('declared server/client mirror coverage', () => { { clientFile: 'example.js', serverFile: 'example.js' }, ]); }); + + it('does not accept a same-name server-only unit test as a parity pin (bypass probe)', () => { + // A direct mirror's clientFile and serverFile are the same string, so a + // plain server-side unit test importing its own module via a bare + // relative path (e.g. `bareUrl.test.js` doing `from './bareUrl.js'`) + // trivially contains both `'./example.js'` and the server filename + // without ever touching the client copy. Pin that this does NOT count. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const serverOnlyUnitTest = { + path: join(here, 'example.test.js'), + source: "import { thing } from './example.js';\n", + }; + expect(missingParityPins(synthetic, [serverOnlyUnitTest])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); }); From 9c12b58e2dbb9716dfdf26ace7202b527810b50d Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 07:48:09 -0700 Subject: [PATCH 4/6] address review (claude): fix second vacuous-guard path in missingParityPins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clientFile and serverName are the identical string for a direct mirror, so the unconditional `source.includes(serverName)` check was implied by the SAME occurrence that already proved "reads the client copy" via the bare `'./'` branch. A plain client-only unit test (e.g. catalogTypes.test.js importing only './catalogTypes.js') was therefore wrongly counted as a parity pin — verified in isolation. Fix strips whichever string proved client-copy evidence before checking for independent server-copy evidence, taking care not to strip the bare import when it's server-side (that's the only evidence catalogTypes.parity.test.js has for the server copy). Verified against every declared mirror pair in the real repo and all 41 mirror/parity test files still pass; added a matching bypass-probe test. --- server/lib/mirrorCoverage.test.js | 53 ++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/server/lib/mirrorCoverage.test.js b/server/lib/mirrorCoverage.test.js index 42944d8ac9..ce8b719c51 100644 --- a/server/lib/mirrorCoverage.test.js +++ b/server/lib/mirrorCoverage.test.js @@ -48,18 +48,32 @@ function uniquePairs(pairs) { function missingParityPins(pairs, testSources) { return pairs.filter(({ clientFile, serverFile }) => { const serverName = basename(serverFile); - return !testSources.some(({ path, source }) => ( - path !== fileURLToPath(import.meta.url) - // The bare `'./'` form only proves "reads the client copy" when the - // test itself lives in client/src/lib — a same-name server-only unit test - // (e.g. bareUrl.test.js importing './bareUrl.js') would otherwise satisfy - // this on its own, since serverName and clientFile are identical strings - // for a direct mirror. Without the directory check, deleting the actual - // parity-pinning *.mirror.test.js leaves this guard silently reporting - // no missing pins. - && (source.includes(`client/src/lib/${clientFile}`) || (path.startsWith(CLIENT_LIB) && source.includes(`'./${clientFile}'`))) - && source.includes(serverName) - )); + // For a direct mirror, clientFile and serverName are the identical + // string — so "reads the client copy" and "reads the server copy" can't + // be proven by a plain substring check on each independently, or the + // SAME occurrence satisfies both (a client-only test that only imports + // its own module via `'./example.js'` would otherwise pass as a valid + // parity pin, and symmetrically for a server-only test — see the two + // bypass-probe tests below). Strip whichever string just proved + // "reads the client copy" before checking for server-copy evidence, so + // the two proofs must come from genuinely different occurrences. + const clientPathRef = `client/src/lib/${clientFile}`; + const bareRef = `'./${clientFile}'`; + return !testSources.some(({ path, source }) => { + if (path === fileURLToPath(import.meta.url)) return false; + const inClientLib = path.startsWith(CLIENT_LIB); + const readsClient = source.includes(clientPathRef) || (inClientLib && source.includes(bareRef)); + if (!readsClient) return false; + let remainder = source.split(clientPathRef).join(''); + // Only strip the bare-import string when it was actually used as + // client-copy evidence above (inClientLib) — outside client/src/lib + // that same bare string is exactly what proves the SERVER copy was + // read (see catalogTypes.parity.test.js: `from './catalogTypes.js'` + // alongside the full client path), so stripping it unconditionally + // would erase legitimate server-copy evidence. + if (inClientLib) remainder = remainder.split(bareRef).join(''); + return remainder.includes(serverName) || (!inClientLib && remainder.includes(`'./${serverName}'`)); + }); }); } @@ -112,4 +126,19 @@ describe('declared server/client mirror coverage', () => { { clientFile: 'example.js', serverFile: 'example.js' }, ]); }); + + it('does not accept a same-name client-only unit test as a parity pin (bypass probe)', () => { + // The mirror image of the probe above: a plain client-side unit test + // importing its own module via a bare relative path (e.g. + // client/src/lib/catalogTypes.test.js doing `from './catalogTypes.js'`) + // never touches the server copy. Pin that this does NOT count either. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const clientOnlyUnitTest = { + path: join(CLIENT_LIB, 'example.test.js'), + source: "import { thing } from './example.js';\n", + }; + expect(missingParityPins(synthetic, [clientOnlyUnitTest])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); }); From d9642caad08c70bfbaaf922ad33bc0f95b1ca1cd Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 07:55:01 -0700 Subject: [PATCH 5/6] address review (claude): require quoted import specifiers in missingParityPins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every prior check was a bare substring test, so a stray comment mentioning a mirrored filename (or an unrelated same-prefix fixture) could satisfy "reads the client/server copy" without any real import existing — verified in isolation: a client-only test whose only server reference was a trailing comment ("// keep this in sync with server/lib/example.js") was wrongly accepted as a parity pin. Replace every substring check with importsRef(), which requires the filename to appear inside an actual quoted specifier. This also let the server-side bare-import branch collapse into the same check, since a quoted './file.js' specifier now matches directly. Verified against every declared pair in the real repo and all 41 mirror/parity test files (600 tests) still pass; added a third bypass-probe test for the comment-only case. --- server/lib/mirrorCoverage.test.js | 51 ++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/server/lib/mirrorCoverage.test.js b/server/lib/mirrorCoverage.test.js index ce8b719c51..eeaad085d8 100644 --- a/server/lib/mirrorCoverage.test.js +++ b/server/lib/mirrorCoverage.test.js @@ -45,34 +45,42 @@ function uniquePairs(pairs) { return [...new Map(pairs.map((pair) => [`${pair.serverFile}:${pair.clientFile}`, pair])).values()]; } +// Matches `ref` only when it appears as (part of) a quoted string — i.e. an +// actual import/require specifier — not a bare substring. A prose comment +// mentioning a filename, or an unrelated same-prefix fixture, must not count +// as "this test imports that file". +function importsRef(source, ref) { + const escaped = ref.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp(`['"\`][^'"\`]*${escaped}['"\`]`).test(source); +} + function missingParityPins(pairs, testSources) { return pairs.filter(({ clientFile, serverFile }) => { const serverName = basename(serverFile); // For a direct mirror, clientFile and serverName are the identical // string — so "reads the client copy" and "reads the server copy" can't - // be proven by a plain substring check on each independently, or the - // SAME occurrence satisfies both (a client-only test that only imports - // its own module via `'./example.js'` would otherwise pass as a valid - // parity pin, and symmetrically for a server-only test — see the two - // bypass-probe tests below). Strip whichever string just proved - // "reads the client copy" before checking for server-copy evidence, so - // the two proofs must come from genuinely different occurrences. + // be proven by a plain check on each independently, or the SAME + // occurrence satisfies both (a client-only test that only imports its + // own module via `'./example.js'` would otherwise pass as a valid + // parity pin, and symmetrically for a server-only test — see the + // bypass-probe tests below). Strip whichever string just proved "reads + // the client copy" before checking for server-copy evidence, so the two + // proofs must come from genuinely different occurrences. const clientPathRef = `client/src/lib/${clientFile}`; - const bareRef = `'./${clientFile}'`; return !testSources.some(({ path, source }) => { if (path === fileURLToPath(import.meta.url)) return false; const inClientLib = path.startsWith(CLIENT_LIB); - const readsClient = source.includes(clientPathRef) || (inClientLib && source.includes(bareRef)); + const readsClient = importsRef(source, clientPathRef) || (inClientLib && importsRef(source, clientFile)); if (!readsClient) return false; let remainder = source.split(clientPathRef).join(''); - // Only strip the bare-import string when it was actually used as + // Only strip the bare-import specifier when it was actually used as // client-copy evidence above (inClientLib) — outside client/src/lib - // that same bare string is exactly what proves the SERVER copy was - // read (see catalogTypes.parity.test.js: `from './catalogTypes.js'` + // that same specifier is exactly what proves the SERVER copy was read + // (see catalogTypes.parity.test.js: `from './catalogTypes.js'` // alongside the full client path), so stripping it unconditionally // would erase legitimate server-copy evidence. - if (inClientLib) remainder = remainder.split(bareRef).join(''); - return remainder.includes(serverName) || (!inClientLib && remainder.includes(`'./${serverName}'`)); + if (inClientLib) remainder = remainder.split(`'./${clientFile}'`).join('').split(`"./${clientFile}"`).join(''); + return importsRef(remainder, serverName); }); }); } @@ -141,4 +149,19 @@ describe('declared server/client mirror coverage', () => { { clientFile: 'example.js', serverFile: 'example.js' }, ]); }); + + it('does not accept a bare prose mention of the filename as a parity pin (bypass probe)', () => { + // A comment mentioning the server path in passing — without an actual + // import of it — must not satisfy the guard either, or a stray comment + // surviving the deletion of the real parity-pinning test would keep this + // suite silently green. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const commentOnlyMention = { + path: join(CLIENT_LIB, 'example.test.js'), + source: "import { thing } from './example.js';\n// keep this in sync with server/lib/example.js\n", + }; + expect(missingParityPins(synthetic, [commentOnlyMention])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); }); From 36bd97dd84a20149f98d7f618b6abf794bd8d36d Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 07:59:56 -0700 Subject: [PATCH 6/6] address review (claude): drop backtick from importsRef's quote class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backtick-fenced file paths are this codebase's dominant docstring style for referencing a mirror's counterpart (every existing parity-test header does this, e.g. personaTraitBlend.parity.test.js), so accepting backtick as an import-specifier delimiter reopened the exact prose-mention bypass the prior fix closed, just via backticks instead of bare text — verified: a JSDoc comment merely mentioning both paths in backticks, with no real import, was wrongly accepted. Restrict the quote class to real string-literal delimiters ('/") only, which is what every actual import/require/readFileSync path in this codebase uses. Verified against all declared pairs in the real repo; added a matching bypass-probe test. --- server/lib/mirrorCoverage.test.js | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/server/lib/mirrorCoverage.test.js b/server/lib/mirrorCoverage.test.js index eeaad085d8..5f2891bbd8 100644 --- a/server/lib/mirrorCoverage.test.js +++ b/server/lib/mirrorCoverage.test.js @@ -51,7 +51,12 @@ function uniquePairs(pairs) { // as "this test imports that file". function importsRef(source, ref) { const escaped = ref.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - return new RegExp(`['"\`][^'"\`]*${escaped}['"\`]`).test(source); + // Quote characters only — no backtick. Backtick-fenced prose is this + // codebase's dominant style for referencing a file path in a comment or + // JSDoc header (see every existing parity-test docstring), so treating it + // as an import specifier would reopen the exact prose-mention bypass this + // helper exists to close. + return new RegExp(`['"][^'"]*${escaped}['"]`).test(source); } function missingParityPins(pairs, testSources) { @@ -164,4 +169,19 @@ describe('declared server/client mirror coverage', () => { { clientFile: 'example.js', serverFile: 'example.js' }, ]); }); + + it('does not accept a backtick-fenced prose mention as a parity pin (bypass probe)', () => { + // Backtick-fenced file references are this codebase's dominant docstring + // style (see every existing parity-test header) — a JSDoc comment that + // mentions both paths in backticks, with no real import backing it, must + // not count either. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const backtickOnlyMention = { + path: join(CLIENT_LIB, 'example.test.js'), + source: 'import { thing } from \'./example.js\';\n// mirrors `server/lib/example.js` in spirit only, no import here.\n', + }; + expect(missingParityPins(synthetic, [backtickOnlyMention])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); });