diff --git a/apps/server/src/core/ai-chat/tools/docmost-client.loader.spec.ts b/apps/server/src/core/ai-chat/tools/docmost-client.loader.spec.ts index 5ca7bbfa..973dec3d 100644 --- a/apps/server/src/core/ai-chat/tools/docmost-client.loader.spec.ts +++ b/apps/server/src/core/ai-chat/tools/docmost-client.loader.spec.ts @@ -1,7 +1,15 @@ import { createHash } from 'node:crypto'; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { + mkdtempSync, + mkdirSync, + writeFileSync, + rmSync, + readdirSync, + statSync, + readFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join, relative, sep } from 'node:path'; import { computeSrcRegistryStamp } from './docmost-client.loader'; @@ -30,10 +38,14 @@ function assertStaleGuard( } } -// Build a throwaway `/build/index.js` + optional `/src/tool-specs.ts` -// layout so `computeSrcRegistryStamp(/build/index.js)` resolves src the same -// way the loader does (dirname(dirname(entry))/src/tool-specs.ts). -function makeFakePackage(toolSpecsSource: string | null): { +// Build a throwaway `/build/index.js` + optional `/src/` tree so +// `computeSrcRegistryStamp(/build/index.js)` resolves src the same way the +// loader does (dirname(dirname(entry))/src). Since #486 the stamp hashes the WHOLE +// src tree, so a fixture is a { relPath: content } map. A bare string is sugar for +// a single `tool-specs.ts`; `null` means "no src tree" (the prod no-op path). +function makeFakePackage( + src: string | Record | null, +): { entry: string; cleanup: () => void; } { @@ -42,10 +54,15 @@ function makeFakePackage(toolSpecsSource: string | null): { mkdirSync(buildDir, { recursive: true }); const entry = join(buildDir, 'index.js'); writeFileSync(entry, '// fake @docmost/mcp build entry\n', 'utf8'); - if (toolSpecsSource !== null) { + if (src !== null) { + const files = + typeof src === 'string' ? { 'tool-specs.ts': src } : src; const srcDir = join(root, 'src'); - mkdirSync(srcDir, { recursive: true }); - writeFileSync(join(srcDir, 'tool-specs.ts'), toolSpecsSource, 'utf8'); + for (const [rel, content] of Object.entries(files)) { + const full = join(srcDir, rel); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, content, 'utf8'); + } } return { entry, cleanup: () => rmSync(root, { recursive: true, force: true }) }; } @@ -93,34 +110,109 @@ describe('computeSrcRegistryStamp (#447 stale-build guard)', () => { } }); - // CROSS-IMPL EQUALITY (covers reviewer suggestion 2). The SAME fixed input and + // #486 CORE (negative): an edit to a NON-tool-specs src file (client.ts) with a + // rebuild NOT run must move the src stamp away from the built REGISTRY_STAMP, so + // the loader's stale-check refuses. Under the old tool-specs.ts-only hash this + // edit was invisible and a stale build/ served the old client.ts silently. + it('a client.ts edit (no rebuild) moves the src stamp -> loader refuses (#486)', () => { + // "Built" state: the package as it was compiled. + const built = makeFakePackage({ + 'tool-specs.ts': 'export const SPECS = 1;\n', + 'client.ts': "export const impl = 'v1';\n", + }); + // "Dev edited src, forgot to rebuild": client.ts changed, tool-specs.ts not. + const edited = makeFakePackage({ + 'tool-specs.ts': 'export const SPECS = 1;\n', + 'client.ts': "export const impl = 'v2';\n", + }); + try { + const builtStamp = computeSrcRegistryStamp(built.entry); + const editedStamp = computeSrcRegistryStamp(edited.entry); + expect(builtStamp).not.toBeNull(); + expect(editedStamp).not.toBe(builtStamp); + // build/ still carries builtStamp; src now hashes to editedStamp -> refuse. + expect(() => assertStaleGuard(editedStamp, builtStamp as string)).toThrow( + STALE_BUILD_MESSAGE, + ); + } finally { + built.cleanup(); + edited.cleanup(); + } + }); + + // *.generated.ts is excluded (the codegen's own output — a fixed-point cycle + // otherwise): its presence/content must not move the stamp. + it('excludes *.generated.ts from the stamp', () => { + const without = makeFakePackage({ 'tool-specs.ts': 'x\n' }); + const withGen = makeFakePackage({ + 'tool-specs.ts': 'x\n', + 'registry-stamp.generated.ts': 'export const REGISTRY_STAMP = "abc";\n', + }); + try { + expect(computeSrcRegistryStamp(withGen.entry)).toBe( + computeSrcRegistryStamp(without.entry), + ); + } finally { + without.cleanup(); + withGen.cleanup(); + } + }); + + // CROSS-IMPL EQUALITY (covers reviewer suggestion 2). The SAME fixed tree and // EXPECTED hash are asserted in the mcp-side node test // (packages/mcp/test/unit/registry-stamp.test.mjs) against the codegen's // `computeRegistryStamp`. Asserting the SAME pair here against the loader's - // `computeSrcRegistryStamp` proves both implementations normalize+hash + // `computeSrcRegistryStamp` proves both implementations enumerate+normalize+hash // identically; a divergence in EITHER side reddens one of the two tests. - it('matches the documented cross-impl hash for a fixed input', () => { - const FIXED_INPUT = 'line1\r\nline2\n'; - const EXPECTED = - '683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83'; - const { entry, cleanup } = makeFakePackage(FIXED_INPUT); + const CROSS_IMPL_TREE = { + 'tool-specs.ts': 'line1\r\nline2\n', + 'client/read.ts': 'export const R = 1;\n', + 'registry-stamp.generated.ts': 'export const REGISTRY_STAMP="ignored";\n', + }; + const CROSS_IMPL_EXPECTED = + '131c1b9e4e2f5a7d6cef91ca8df619822b442f52bc45ebd09474a4c1d6728616'; + + it('matches the documented cross-impl hash for a fixed tree', () => { + const { entry, cleanup } = makeFakePackage(CROSS_IMPL_TREE); try { - expect(computeSrcRegistryStamp(entry)).toBe(EXPECTED); + expect(computeSrcRegistryStamp(entry)).toBe(CROSS_IMPL_EXPECTED); } finally { cleanup(); } }); - it('the documented EXPECTED is the normalize+sha256 of the fixed input', () => { - // Proves EXPECTED is not a magic constant but the documented computation. - const FIXED_INPUT = 'line1\r\nline2\n'; - const normalized = FIXED_INPUT.replace(/\r\n/g, '\n').replace(/\n$/, ''); - const expected = createHash('sha256') - .update(normalized, 'utf8') - .digest('hex'); - const { entry, cleanup } = makeFakePackage(FIXED_INPUT); + it('the documented EXPECTED is the enumerate+normalize+sha256 of the tree', () => { + // Proves EXPECTED is not a magic constant but the documented computation — a + // local re-implementation of the loader's tree walk. + const { entry, cleanup } = makeFakePackage(CROSS_IMPL_TREE); try { - expect(computeSrcRegistryStamp(entry)).toBe(expected); + const srcDir = join(dirname(dirname(entry)), 'src'); + const collect = (dir: string): string[] => { + const out: string[] = []; + for (const e of readdirSync(dir)) { + const f = join(dir, e); + if (statSync(f).isDirectory()) out.push(...collect(f)); + else if (e.endsWith('.ts') && !e.endsWith('.generated.ts')) + out.push(f); + } + return out; + }; + const files = collect(srcDir) + .map((abs) => ({ rel: relative(srcDir, abs).split(sep).join('/'), abs })) + .sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); + const h = createHash('sha256'); + for (const { rel, abs } of files) { + const n = readFileSync(abs, 'utf8') + .replace(/\r\n/g, '\n') + .replace(/\n$/, ''); + h.update(rel, 'utf8'); + h.update('\0', 'utf8'); + h.update(n, 'utf8'); + h.update('\0', 'utf8'); + } + const localHash = h.digest('hex'); + expect(computeSrcRegistryStamp(entry)).toBe(localHash); + expect(localHash).toBe(CROSS_IMPL_EXPECTED); } finally { cleanup(); } diff --git a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts index 24f16f6a..8029a239 100644 --- a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts +++ b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts @@ -1,6 +1,6 @@ import { createHash } from 'node:crypto'; -import { existsSync, readFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'; +import { dirname, join, relative, sep } from 'node:path'; import { pathToFileURL } from 'node:url'; import type { DocmostClient, SharedToolSpec } from '@docmost/mcp'; @@ -191,33 +191,52 @@ interface DocmostMcpModule { * present. Returns the stamp string, or `null` when the source is absent (a prod * image ships only build/, no src/). MUST stay byte-for-byte identical to * packages/mcp/scripts/gen-registry-stamp.mjs's `computeRegistryStamp` so the - * build-time and src-time hashes agree: same input file (src/tool-specs.ts), same - * normalization (CRLF -> LF, strip a single trailing newline), same sha256. + * build-time and src-time hashes agree: same file set (every src/**\/*.ts except + * *.generated.ts), same POSIX-relative sort, same per-file normalization (CRLF -> + * LF, strip a single trailing newline) with the same path+content framing, same + * sha256. Hashing the WHOLE src tree (not just tool-specs.ts) is #486: an edit to + * client.ts / a client/* module / comment-signal / drawio-* without a rebuild + * must also be caught, otherwise build/ silently serves the old code. * * DEV vs PROD detection is by FILE EXISTENCE, not NODE_ENV: we resolve the * package's own directory from `require.resolve('@docmost/mcp')` (which points at - * build/index.js) and look for ../src/tool-specs.ts next to it. In a dev/test - * worktree that file exists; in a prod image (build/ only, src/ stripped) it does - * not, so this returns null and the caller skips the check. Any error (ENOENT, a - * bad resolve) is swallowed to null — the stale-check must NEVER break startup. + * build/index.js) and look for ../src next to it. In a dev/test worktree that + * directory exists; in a prod image (build/ only, src/ stripped) it does not, so + * this returns null and the caller skips the check. Any error (ENOENT, a bad + * resolve) is swallowed to null — the stale-check must NEVER break startup. * * Exported for unit testing (docmost-client.loader.spec.ts): the export keyword * is behaviourally a no-op — the module-internal caller `loadDocmostMcp` is * unaffected. The test drives the null (no-src) path and asserts this - * normalize+sha256 stays identical to the codegen's `computeRegistryStamp`. + * enumerate+normalize+sha256 stays identical to the codegen's + * `computeRegistryStamp`. */ export function computeSrcRegistryStamp(packageEntry: string): string | null { try { // packageEntry is /build/index.js; the source lives at /src/. - const toolSpecsPath = join( - dirname(dirname(packageEntry)), - 'src', - 'tool-specs.ts', - ); - if (!existsSync(toolSpecsPath)) return null; // prod: no src tree -> skip. - const source = readFileSync(toolSpecsPath, 'utf8'); - const normalized = source.replace(/\r\n/g, '\n').replace(/\n$/, ''); - return createHash('sha256').update(normalized, 'utf8').digest('hex'); + const srcDir = join(dirname(dirname(packageEntry)), 'src'); + if (!existsSync(srcDir)) return null; // prod: no src tree -> skip. + // Enumerate every src/**\/*.ts except the codegen's own *.generated.ts + // output (including it would be a fixed-point cycle). Sort by POSIX-relative + // path so ordering is platform-independent, then fold each file's relative + // path + normalized content into one hash — identical to the codegen. + const files = collectStampFiles(srcDir) + .map((abs) => ({ + rel: relative(srcDir, abs).split(sep).join('/'), + abs, + })) + .sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); + const hash = createHash('sha256'); + for (const { rel, abs } of files) { + const normalized = readFileSync(abs, 'utf8') + .replace(/\r\n/g, '\n') + .replace(/\n$/, ''); + hash.update(rel, 'utf8'); + hash.update('\0', 'utf8'); + hash.update(normalized, 'utf8'); + hash.update('\0', 'utf8'); + } + return hash.digest('hex'); } catch { // Never let a resolution/read hiccup break server startup — treat as "no // src available" and skip the check (identical to the prod no-op path). @@ -225,6 +244,24 @@ export function computeSrcRegistryStamp(packageEntry: string): string | null { } } +/** + * Recursively enumerate every `*.ts` under `dir`, EXCLUDING `*.generated.ts`. + * Mirror of the codegen's `collectStampFiles` (packages/mcp/scripts/ + * gen-registry-stamp.mjs) — keep the two walk/filter rules identical. + */ +function collectStampFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + out.push(...collectStampFiles(full)); + } else if (entry.endsWith('.ts') && !entry.endsWith('.generated.ts')) { + out.push(full); + } + } + return out; +} + // TS with module:commonjs downlevels a literal `import()` to `require()`, which // cannot load the ESM-only `@docmost/mcp` package. Indirect through Function so // the real dynamic `import()` survives compilation and can load ESM from diff --git a/packages/mcp/scripts/gen-registry-stamp.mjs b/packages/mcp/scripts/gen-registry-stamp.mjs index ecf865a5..82a559ab 100644 --- a/packages/mcp/scripts/gen-registry-stamp.mjs +++ b/packages/mcp/scripts/gen-registry-stamp.mjs @@ -1,60 +1,100 @@ // Codegen: emit src/registry-stamp.generated.ts with a REGISTRY_STAMP hash of -// the tool-specs REGISTRY CONTENT, so a build/ vs src/ skew (issue #447) is -// detectable at runtime. +// the ENTIRE src/ tree, so a build/ vs src/ skew (issue #447) is detectable at +// runtime for ANY source file — not just tool-specs.ts. // -// WHY hash the raw source text (not extracted structured data): -// SHARED_TOOL_SPECS carries `buildShape` functions (the input SCHEMAS) which are -// NOT serializable. The input schema is exactly one of the things that MUST stay -// in sync between build/ and src/, so we cannot drop it from the hash. Rather -// than probe zod with a fragile shim to reconstruct the schema shape, we hash the -// STABLE, deterministic source TEXT of tool-specs.ts. That text fully captures -// every field that must stay in sync — mcpName, inAppKey, description, tier, -// catalogLine AND the buildShape bodies (input schemas) — with zero probing -// fragility. Any edit to a spec (a renamed tool, a reworded description, a -// changed schema field) changes the text and therefore the stamp. +// WHY hash the whole src tree (not just tool-specs.ts): the runtime tools are +// assembled from far more than the spec registry — client.ts, the client/* +// domain modules, comment-signal.ts and the drawio-* helpers all ship in build/ +// and are loaded by the in-app server. Hashing ONLY tool-specs.ts meant an edit +// to any of those (e.g. a behavioural fix in client.ts) left the stamp unchanged, +// so a stale build/ served the OLD code silently (issue #486). Hashing every +// src/**/*.ts closes that gap: any source edit changes the stamp. // -// DETERMINISM: the hash is computed over the file bytes with line endings -// normalized to LF and a single trailing newline stripped, so a CRLF checkout or -// an editor's trailing-newline habit cannot make build/ and src/ disagree. No -// Date.now / randomness. The loader's dev-only stale-check (docmost-client.loader.ts) -// re-runs THIS SAME normalization + sha256 over src/tool-specs.ts and compares to -// the built REGISTRY_STAMP; the two must compute identically. +// WHY hash the raw source text (not extracted structured data): the tool input +// SCHEMAS live as `buildShape` functions which are NOT serializable, so we cannot +// reduce them to structured data without a fragile zod shim. Hashing the STABLE, +// deterministic source TEXT captures every field that must stay in sync with zero +// probing fragility. Any edit to any source file changes the text → the stamp. +// +// DETERMINISM: files are enumerated recursively, filtered to *.ts EXCLUDING +// *.generated.ts (the codegen's OWN output — including it would create a +// fixed-point cycle), and sorted by their POSIX-normalized path relative to src/ +// so the order is platform-independent. Each file contributes its relative path +// AND its content with line endings normalized to LF and a single trailing +// newline stripped, so a CRLF checkout or an editor's trailing-newline habit +// cannot make build/ and src/ disagree. No Date.now / randomness. The loader's +// dev-only stale-check (docmost-client.loader.ts) re-runs THIS SAME enumeration + +// normalization + sha256 and compares to the built REGISTRY_STAMP; the two must +// compute identically. // // This script runs from the `build` and `pretest` npm scripts BEFORE tsc, so -// build/ always carries a stamp derived from the tool-specs.ts that was compiled. +// build/ always carries a stamp derived from the src/ tree that was compiled. import { createHash } from 'node:crypto'; -import { readFileSync, writeFileSync } from 'node:fs'; +import { readdirSync, readFileSync, statSync, writeFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { dirname, join } from 'node:path'; +import { dirname, join, relative, sep } from 'node:path'; const __dirname = dirname(fileURLToPath(import.meta.url)); const SRC_DIR = join(__dirname, '..', 'src'); -const TOOL_SPECS_PATH = join(SRC_DIR, 'tool-specs.ts'); const OUT_PATH = join(SRC_DIR, 'registry-stamp.generated.ts'); /** - * Deterministic stamp of the tool-specs registry content. Kept as a plain - * function (exported) so the algorithm has a single home; the loader duplicates - * only the tiny normalize+sha256 steps because it lives in the CJS server build - * and cannot import this ESM script. If you change the normalization here, mirror - * it in apps/server/src/core/ai-chat/tools/docmost-client.loader.ts. + * Recursively enumerate every `*.ts` file under `dir`, EXCLUDING the codegen's + * own `*.generated.ts` output (a self-referential cycle otherwise). Returns + * absolute paths, unsorted (the caller sorts by relative path for determinism). + * Kept as a plain exported function so the algorithm has a single home; the + * loader duplicates it because it lives in the CJS server build and cannot import + * this ESM script. If you change the walk/filter here, mirror it in + * apps/server/src/core/ai-chat/tools/docmost-client.loader.ts. */ -export function computeRegistryStamp(toolSpecsSource) { - const normalized = toolSpecsSource.replace(/\r\n/g, '\n').replace(/\n$/, ''); - return createHash('sha256').update(normalized, 'utf8').digest('hex'); +export function collectStampFiles(dir) { + const out = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + out.push(...collectStampFiles(full)); + } else if (entry.endsWith('.ts') && !entry.endsWith('.generated.ts')) { + out.push(full); + } + } + return out; +} + +/** + * Deterministic stamp of the whole src/ tree. Enumerate + sort by POSIX-relative + * path, then fold each file's relative path AND normalized content into one + * sha256. MUST stay byte-for-byte identical to the loader's recompute. + */ +export function computeRegistryStamp(srcDir) { + const files = collectStampFiles(srcDir) + .map((abs) => ({ + rel: relative(srcDir, abs).split(sep).join('/'), + abs, + })) + .sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); + const hash = createHash('sha256'); + for (const { rel, abs } of files) { + const normalized = readFileSync(abs, 'utf8') + .replace(/\r\n/g, '\n') + .replace(/\n$/, ''); + hash.update(rel, 'utf8'); + hash.update('\0', 'utf8'); + hash.update(normalized, 'utf8'); + hash.update('\0', 'utf8'); + } + return hash.digest('hex'); } function main() { - const source = readFileSync(TOOL_SPECS_PATH, 'utf8'); - const stamp = computeRegistryStamp(source); + const stamp = computeRegistryStamp(SRC_DIR); const out = '// AUTO-GENERATED by scripts/gen-registry-stamp.mjs — DO NOT EDIT BY HAND.\n' + - '// A deterministic hash of src/tool-specs.ts content (tool names, descriptions,\n' + - '// tiers, catalog lines and input schemas). Regenerated on every build/pretest\n' + - '// so build/ always matches the compiled src. The in-app loader recomputes this\n' + - '// from src and refuses to run on a mismatch (issue #447). This file is\n' + - '// gitignored and produced by the build — see .gitignore.\n' + + '// A deterministic hash of the whole src/ tree (every src/**/*.ts except\n' + + '// *.generated.ts). Regenerated on every build/pretest so build/ always\n' + + '// matches the compiled src. The in-app loader recomputes this from src and\n' + + '// refuses to run on a mismatch (issue #447/#486). This file is gitignored\n' + + '// and produced by the build — see .gitignore.\n' + `export const REGISTRY_STAMP = ${JSON.stringify(stamp)};\n`; writeFileSync(OUT_PATH, out, 'utf8'); // eslint-disable-next-line no-console diff --git a/packages/mcp/test/unit/registry-stamp.test.mjs b/packages/mcp/test/unit/registry-stamp.test.mjs index b9d992c4..32006688 100644 --- a/packages/mcp/test/unit/registry-stamp.test.mjs +++ b/packages/mcp/test/unit/registry-stamp.test.mjs @@ -1,101 +1,220 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { createHash } from "node:crypto"; -import { readFileSync } from "node:fs"; +import { + mkdtempSync, + mkdirSync, + writeFileSync, + rmSync, + readdirSync, + statSync, + readFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; import { fileURLToPath } from "node:url"; -import { dirname, join } from "node:path"; +import { dirname, join, relative, sep } from "node:path"; +import { createHash } from "node:crypto"; import { computeRegistryStamp } from "../../scripts/gen-registry-stamp.mjs"; import { REGISTRY_STAMP } from "../../build/index.js"; -// Guard tests for the build/src-skew stamp (issue #447). The codegen script -// exports `computeRegistryStamp(sourceText)` — a sha256 over normalized source -// text (CRLF->LF, single trailing newline stripped). The in-app loader -// (apps/server/.../docmost-client.loader.ts) DUPLICATES that normalize+sha256 to -// recompute the stamp from src and refuse a stale build. These tests pin the -// algorithm's behaviour AND assert the built stamp matches the current src, so a -// stale generated file OR a normalize divergence reddens. +// Guard tests for the build/src-skew stamp (issues #447/#486). The codegen script +// exports `computeRegistryStamp(srcDir)` — a sha256 over the WHOLE src/ tree +// (every src/**/*.ts EXCEPT *.generated.ts), each file folded in as its +// POSIX-relative path + its normalized content (CRLF->LF, single trailing newline +// stripped). Hashing the whole tree (not just tool-specs.ts) is #486: an edit to +// client.ts / a client/* module without a rebuild must ALSO redden. The in-app +// loader (apps/server/.../docmost-client.loader.ts) DUPLICATES this enumerate+ +// normalize+sha256 to refuse a stale build. These tests pin the algorithm and +// assert the built stamp matches the current src. const __dirname = dirname(fileURLToPath(import.meta.url)); -const TOOL_SPECS_PATH = join(__dirname, "..", "..", "src", "tool-specs.ts"); +const SRC_DIR = join(__dirname, "..", "..", "src"); -test("computeRegistryStamp is deterministic: same input -> same hash", () => { - const input = "export const X = 1;\nexport const Y = 2;\n"; - assert.equal(computeRegistryStamp(input), computeRegistryStamp(input)); +// Build a throwaway src/ tree from a { relPath: content } map and return its dir. +function makeSrcTree(files) { + const root = mkdtempSync(join(tmpdir(), "mcp-stamp-tree-")); + const src = join(root, "src"); + for (const [rel, content] of Object.entries(files)) { + const full = join(src, rel); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, content, "utf8"); + } + return { src, cleanup: () => rmSync(root, { recursive: true, force: true }) }; +} + +test("computeRegistryStamp is deterministic: same tree -> same hash", () => { + const a = makeSrcTree({ "tool-specs.ts": "export const X = 1;\n" }); + const b = makeSrcTree({ "tool-specs.ts": "export const X = 1;\n" }); + try { + assert.equal(computeRegistryStamp(a.src), computeRegistryStamp(b.src)); + } finally { + a.cleanup(); + b.cleanup(); + } }); test("computeRegistryStamp returns a 64-char lowercase hex sha256", () => { - const stamp = computeRegistryStamp("anything"); - assert.match(stamp, /^[0-9a-f]{64}$/); + const t = makeSrcTree({ "tool-specs.ts": "anything\n" }); + try { + assert.match(computeRegistryStamp(t.src), /^[0-9a-f]{64}$/); + } finally { + t.cleanup(); + } }); -test("normalizes CRLF vs LF: the same content hashes equal", () => { - const lf = "line1\nline2\nline3"; - const crlf = "line1\r\nline2\r\nline3"; - assert.equal(computeRegistryStamp(crlf), computeRegistryStamp(lf)); +// #486 CORE: an edit to a NON-tool-specs source file (client.ts) must change the +// stamp. Under the old single-file (tool-specs.ts only) hash this edit was +// invisible and a stale build/ served the old client.ts silently. +test("editing client.ts (not tool-specs.ts) changes the stamp (#486)", () => { + const before = makeSrcTree({ + "tool-specs.ts": "export const SPECS = 1;\n", + "client.ts": "export const impl = 'v1';\n", + }); + const after = makeSrcTree({ + "tool-specs.ts": "export const SPECS = 1;\n", + "client.ts": "export const impl = 'v2';\n", + }); + try { + assert.notEqual( + computeRegistryStamp(before.src), + computeRegistryStamp(after.src), + "a client.ts edit with an unchanged tool-specs.ts must move the stamp", + ); + } finally { + before.cleanup(); + after.cleanup(); + } }); -test("normalizes a trailing newline: with/without a final \\n hashes equal", () => { - const noTrailing = "alpha\nbeta"; - const trailing = "alpha\nbeta\n"; - assert.equal(computeRegistryStamp(trailing), computeRegistryStamp(noTrailing)); +test("editing a nested client/* module changes the stamp", () => { + const before = makeSrcTree({ + "tool-specs.ts": "x\n", + "client/read.ts": "export const READ = 1;\n", + }); + const after = makeSrcTree({ + "tool-specs.ts": "x\n", + "client/read.ts": "export const READ = 2;\n", + }); + try { + assert.notEqual( + computeRegistryStamp(before.src), + computeRegistryStamp(after.src), + ); + } finally { + before.cleanup(); + after.cleanup(); + } }); -test("a CRLF checkout WITH a trailing CRLF still hashes equal to bare LF", () => { - // A worst-case Windows checkout: CRLF line endings + a trailing CRLF. Both the - // \r\n->\n replace and the trailing-newline strip must apply for parity. - const bare = "alpha\nbeta"; - const crlfTrailing = "alpha\r\nbeta\r\n"; - assert.equal( - computeRegistryStamp(crlfTrailing), - computeRegistryStamp(bare), - ); +// *.generated.ts is EXCLUDED (else the codegen's own output is a fixed-point +// cycle): adding/removing/changing it must not move the stamp. +test("*.generated.ts is excluded from the stamp", () => { + const without = makeSrcTree({ "tool-specs.ts": "x\n" }); + const withGen = makeSrcTree({ + "tool-specs.ts": "x\n", + "registry-stamp.generated.ts": 'export const REGISTRY_STAMP = "abc";\n', + }); + try { + assert.equal( + computeRegistryStamp(without.src), + computeRegistryStamp(withGen.src), + "a *.generated.ts file must not affect the stamp", + ); + } finally { + without.cleanup(); + withGen.cleanup(); + } }); -test("a real content change hashes differently", () => { - const before = "export const description = 'search a page';\n"; - const after = "export const description = 'search a PAGE';\n"; - assert.notEqual(computeRegistryStamp(before), computeRegistryStamp(after)); +test("a CRLF checkout WITH trailing CRLF hashes equal to bare LF", () => { + const bare = makeSrcTree({ "tool-specs.ts": "alpha\nbeta" }); + const crlfTrailing = makeSrcTree({ "tool-specs.ts": "alpha\r\nbeta\r\n" }); + try { + assert.equal( + computeRegistryStamp(crlfTrailing.src), + computeRegistryStamp(bare.src), + ); + } finally { + bare.cleanup(); + crlfTrailing.cleanup(); + } }); -// Only a SINGLE trailing newline is stripped — a second blank line is content and -// must change the hash. This pins the exact `/\n$/` semantics the loader mirrors. +// Only a SINGLE trailing newline is stripped — a second blank line is content. test("only ONE trailing newline is stripped (two differ from one)", () => { - assert.notEqual( - computeRegistryStamp("x\n"), - computeRegistryStamp("x\n\n"), - ); + const one = makeSrcTree({ "tool-specs.ts": "x\n" }); + const two = makeSrcTree({ "tool-specs.ts": "x\n\n" }); + try { + assert.notEqual( + computeRegistryStamp(one.src), + computeRegistryStamp(two.src), + ); + } finally { + one.cleanup(); + two.cleanup(); + } }); -// Cross-impl equality against a fixed, documented input. The SAME literal input -// and expected hash are asserted in the server-side jest test -// (docmost-client.loader.spec.ts). If either side's normalize+sha256 ever -// diverges, one of the two tests reddens. Input exercises BOTH normalize steps. -test("fixed-input hash matches the documented cross-impl value", () => { - const FIXED_INPUT = "line1\r\nline2\n"; - const EXPECTED = - "683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83"; - assert.equal(computeRegistryStamp(FIXED_INPUT), EXPECTED); +// Cross-impl equality against a fixed, documented tree. The SAME literal tree and +// expected hash are asserted in the server-side jest test +// (docmost-client.loader.spec.ts). If either side's enumerate+normalize+sha256 +// ever diverges, one of the two tests reddens. The tree exercises: a nested file, +// BOTH normalize steps (tool-specs.ts uses CRLF + trailing \n) and the +// *.generated.ts exclusion. +const CROSS_IMPL_TREE = { + "tool-specs.ts": "line1\r\nline2\n", + "client/read.ts": "export const R = 1;\n", + "registry-stamp.generated.ts": 'export const REGISTRY_STAMP="ignored";\n', +}; +const CROSS_IMPL_EXPECTED = + "131c1b9e4e2f5a7d6cef91ca8df619822b442f52bc45ebd09474a4c1d6728616"; + +test("fixed-tree hash matches the documented cross-impl value", () => { + const t = makeSrcTree(CROSS_IMPL_TREE); + try { + assert.equal(computeRegistryStamp(t.src), CROSS_IMPL_EXPECTED); + } finally { + t.cleanup(); + } }); -// DESYNC GUARD (covers reviewer suggestion 2). Recompute the stamp from the -// actual src/tool-specs.ts and assert it equals the REGISTRY_STAMP baked into the -// freshly-built build/index.js. This reddens if the generated file is stale OR if -// the codegen normalize ever diverges from what produced the built stamp. -test("built REGISTRY_STAMP equals the stamp recomputed from src/tool-specs.ts", () => { - const source = readFileSync(TOOL_SPECS_PATH, "utf8"); - assert.equal(computeRegistryStamp(source), REGISTRY_STAMP); +// Sanity: the EXPECTED constant is not a magic value but the documented +// enumerate+normalize+sha256 of CROSS_IMPL_TREE (a local re-implementation). +test("the documented EXPECTED is the enumerate+normalize+sha256 of the tree", () => { + const t = makeSrcTree(CROSS_IMPL_TREE); + try { + const collect = (dir) => { + const out = []; + for (const e of readdirSync(dir)) { + const f = join(dir, e); + if (statSync(f).isDirectory()) out.push(...collect(f)); + else if (e.endsWith(".ts") && !e.endsWith(".generated.ts")) out.push(f); + } + return out; + }; + const files = collect(t.src) + .map((abs) => ({ rel: relative(t.src, abs).split(sep).join("/"), abs })) + .sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); + const h = createHash("sha256"); + for (const { rel, abs } of files) { + const n = readFileSync(abs, "utf8") + .replace(/\r\n/g, "\n") + .replace(/\n$/, ""); + h.update(rel, "utf8"); + h.update("\0", "utf8"); + h.update(n, "utf8"); + h.update("\0", "utf8"); + } + assert.equal(h.digest("hex"), CROSS_IMPL_EXPECTED); + } finally { + t.cleanup(); + } }); -// Sanity: the fixed-input helper computes the SAME way the codegen does, proving -// the EXPECTED constant above is not an arbitrary magic value but the documented -// normalize+sha256 of FIXED_INPUT. Belt-and-braces so a bad EXPECTED can't hide a -// real regression. -test("the documented EXPECTED constant is the normalize+sha256 of FIXED_INPUT", () => { - const FIXED_INPUT = "line1\r\nline2\n"; - const normalized = FIXED_INPUT.replace(/\r\n/g, "\n").replace(/\n$/, ""); - const expected = createHash("sha256") - .update(normalized, "utf8") - .digest("hex"); - assert.equal(computeRegistryStamp(FIXED_INPUT), expected); +// DESYNC GUARD. Recompute the stamp from the REAL src/ tree and assert it equals +// the REGISTRY_STAMP baked into the freshly-built build/index.js. This reddens if +// the generated file is stale OR if the codegen ever diverges from what produced +// the built stamp. +test("built REGISTRY_STAMP equals the stamp recomputed from src/", () => { + assert.equal(computeRegistryStamp(SRC_DIR), REGISTRY_STAMP); });