fix(ci,mcp): REGISTRY_STAMP в билд-артефакте + кросс-пакетный CI — закрыть skew build/ vs src/ (#447)

Два структурных слепых пятна: (1) сервер грузит СКОМПИЛИРОВАННЫЙ
packages/mcp/build/index.js, а серверные guard-тесты читают src/tool-specs.ts —
правка src без пересборки оставляет тесты зелёными, но рантайм расходится со
спеками; (2) спеки добавляют в packages/mcp, а parity-тесты живут в jest-сьюте
apps/server — PR, трогающий только пакет, проходит зелёным, сломанная in-app-
проводка всплывает уже на develop (кейс f46d89ea).

- REGISTRY_STAMP: codegen (scripts/gen-registry-stamp.mjs) считает sha256 от
  нормализованного (CRLF→LF, один хвостовой \n снят) сырого текста
  src/tool-specs.ts, пишет src/registry-stamp.generated.ts (gitignored),
  index.ts реэкспортит → попадает в build/. Вшит в build/pretest/watch ДО tsc.
- Loader (dev/test): computeSrcRegistryStamp пересчитывает стамп из src рядом с
  build/index.js (dev-vs-prod по existsSync, любая ошибка → null), сверяет с
  build-стампом → при рассинхроне бросает «build is stale — rebuild». В prod
  (src нет) и на pre-#447 билдах (нет REGISTRY_STAMP) — чистый no-op.
- CI: job mcp-server-parity собирает shared-deps+mcp (регенерит стамп) и гоняет
  ОБА сьюта вместе (mcp node:test + server guard-спеки) — именованный гейт, его
  нельзя случайно расщепить.
- AGENTS.md: правка спеков требует ребилда @docmost/mcp.

Тесты (20): mcp-сайд (детерминизм, нормализация, desync-гард стамп-vs-билд) +
server-сайд (null при отсутствии src = prod no-op; mismatch → throw точного
сообщения; pre-#447 no-op). Кросс-импл equality-гард: один фиксированный вход →
один хэш на ОБЕИХ сторонах, ловит рассинхрон двух нормализаций. Внутреннее
ревью: APPROVE WITH SUGGESTIONS (обе — покрытие guard'а — закрыты этим тестом).
Мутационно: любой из двух normalize-имплов расходится → equality-тест краснеет.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-10 07:58:37 +03:00
parent f8d37d8956
commit 2e6f1c3de5
9 changed files with 496 additions and 3 deletions
@@ -0,0 +1,173 @@
import { createHash } from 'node:crypto';
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { computeSrcRegistryStamp } from './docmost-client.loader';
// The exact message the loader throws on a build/src skew (issue #447). Kept as a
// literal here so a reworded prod message reddens this test (the message is a
// developer-facing contract: it tells them how to fix it).
const STALE_BUILD_MESSAGE =
'@docmost/mcp build is stale (tool-specs changed since last build) — run: pnpm --filter @docmost/mcp build';
// Replica of the loader's inline stale-check predicate + throw from
// `loadDocmostMcp`. That guard is not independently exported (it lives inside the
// dynamic-import IIFE, wired to a fixed `require.resolve('@docmost/mcp')`), so we
// exercise the exact same three-condition logic against a stamp produced by the
// REAL `computeSrcRegistryStamp`. This documents and locks the throw/no-throw
// behaviour; if the prod predicate changes, this replica must change with it.
function assertStaleGuard(
srcStamp: string | null,
registryStamp: string | undefined,
): void {
if (
srcStamp !== null &&
typeof registryStamp === 'string' &&
srcStamp !== registryStamp
) {
throw new Error(STALE_BUILD_MESSAGE);
}
}
// Build a throwaway `<pkg>/build/index.js` + optional `<pkg>/src/tool-specs.ts`
// layout so `computeSrcRegistryStamp(<pkg>/build/index.js)` resolves src the same
// way the loader does (dirname(dirname(entry))/src/tool-specs.ts).
function makeFakePackage(toolSpecsSource: string | null): {
entry: string;
cleanup: () => void;
} {
const root = mkdtempSync(join(tmpdir(), 'mcp-stamp-'));
const buildDir = join(root, 'build');
mkdirSync(buildDir, { recursive: true });
const entry = join(buildDir, 'index.js');
writeFileSync(entry, '// fake @docmost/mcp build entry\n', 'utf8');
if (toolSpecsSource !== null) {
const srcDir = join(root, 'src');
mkdirSync(srcDir, { recursive: true });
writeFileSync(join(srcDir, 'tool-specs.ts'), toolSpecsSource, 'utf8');
}
return { entry, cleanup: () => rmSync(root, { recursive: true, force: true }) };
}
describe('computeSrcRegistryStamp (#447 stale-build guard)', () => {
it('returns null when src/tool-specs.ts is absent (prod no-op path)', () => {
// A prod image ships only build/, no src/ — the guard must be a silent no-op.
const { entry, cleanup } = makeFakePackage(null);
try {
expect(computeSrcRegistryStamp(entry)).toBeNull();
} finally {
cleanup();
}
});
it('returns null for a bogus package entry (swallowed error path)', () => {
// A resolution/read hiccup must NEVER break startup — it resolves to null.
expect(
computeSrcRegistryStamp('/no/such/pkg/build/index.js'),
).toBeNull();
});
it('computes a 64-char sha256 hex when src/tool-specs.ts exists', () => {
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
try {
const stamp = computeSrcRegistryStamp(entry);
expect(stamp).toMatch(/^[0-9a-f]{64}$/);
} finally {
cleanup();
}
});
it('normalizes CRLF->LF and strips a single trailing newline', () => {
// A CRLF+trailing-newline variant of the same content hashes identically to
// the bare-LF form — the guard must not fire on a checkout-style difference.
const bare = makeFakePackage('alpha\nbeta');
const crlfTrailing = makeFakePackage('alpha\r\nbeta\r\n');
try {
expect(computeSrcRegistryStamp(crlfTrailing.entry)).toBe(
computeSrcRegistryStamp(bare.entry),
);
} finally {
bare.cleanup();
crlfTrailing.cleanup();
}
});
// CROSS-IMPL EQUALITY (covers reviewer suggestion 2). The SAME fixed input 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
// 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);
try {
expect(computeSrcRegistryStamp(entry)).toBe(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);
try {
expect(computeSrcRegistryStamp(entry)).toBe(expected);
} finally {
cleanup();
}
});
});
describe('loadDocmostMcp stale-check predicate (#447)', () => {
it('THROWS the exact stale message when src stamp != built REGISTRY_STAMP', () => {
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
try {
const srcStamp = computeSrcRegistryStamp(entry);
expect(srcStamp).not.toBeNull();
// Simulate a stale build: build/ carries a DIFFERENT stamp than src.
expect(() => assertStaleGuard(srcStamp, 'a'.repeat(64))).toThrow(
STALE_BUILD_MESSAGE,
);
} finally {
cleanup();
}
});
it('does NOT throw when src stamp equals the built REGISTRY_STAMP', () => {
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
try {
const srcStamp = computeSrcRegistryStamp(entry);
// Fresh build: build/ stamp == src stamp -> guard is a no-op.
expect(() => assertStaleGuard(srcStamp, srcStamp as string)).not.toThrow();
} finally {
cleanup();
}
});
it('does NOT throw when src is absent (prod: srcStamp === null)', () => {
// Even against a present-but-mismatched REGISTRY_STAMP, a null src stamp
// (prod image with build/ only) must skip the check entirely.
expect(() => assertStaleGuard(null, 'a'.repeat(64))).not.toThrow();
});
it('does NOT throw when REGISTRY_STAMP is absent (pre-#447 build)', () => {
// An older @docmost/mcp build has no REGISTRY_STAMP export; the guard must be
// a no-op so an out-of-date build never wrongly blocks startup.
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
try {
const srcStamp = computeSrcRegistryStamp(entry);
expect(() => assertStaleGuard(srcStamp, undefined)).not.toThrow();
} finally {
cleanup();
}
});
});
@@ -1,3 +1,6 @@
import { createHash } from 'node:crypto';
import { existsSync, readFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { pathToFileURL } from 'node:url';
/**
@@ -344,6 +347,50 @@ interface DocmostMcpModule {
// loader in unit tests. The in-app layer treats an absent factory as "signal
// disabled" — a pure no-op that leaves tool results byte-identical.
createCommentSignalTracker?: CommentSignalTrackerFactory;
// Optional (#447): a deterministic hash of the tool-specs registry content,
// generated into build/ by the package's build. Absent on a pre-#447 build (or
// the mocked loader in unit tests) — the stale-check below is a NO-OP when it
// is missing, so an older build never wrongly fails startup.
REGISTRY_STAMP?: string;
}
/**
* Recompute the REGISTRY_STAMP (#447) from the @docmost/mcp source tree, if it is
* 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.
*
* 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.
*
* 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`.
*/
export function computeSrcRegistryStamp(packageEntry: string): string | null {
try {
// packageEntry is <pkg>/build/index.js; the source lives at <pkg>/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');
} 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).
return null;
}
}
// TS with module:commonjs downlevels a literal `import()` to `require()`, which
@@ -375,6 +422,23 @@ export async function loadDocmostMcp(): Promise<{
const mod = (await esmImport(
pathToFileURL(entry).href,
)) as DocmostMcpModule;
// #447 stale-build guard (dev/test only). The server loads the COMPILED
// build/ of @docmost/mcp, but the parity/tier guard tests read src/. If a
// tool spec is edited in src without rebuilding the package, build/ and src/
// silently diverge and the running server serves the OLD tools. Here we
// recompute the stamp from src/tool-specs.ts and compare it to the stamp
// baked into build/. In PROD the src tree is absent (image ships build/
// only), so computeSrcRegistryStamp returns null and this is a pure no-op.
const srcStamp = computeSrcRegistryStamp(entry);
if (
srcStamp !== null &&
typeof mod.REGISTRY_STAMP === 'string' &&
srcStamp !== mod.REGISTRY_STAMP
) {
throw new Error(
'@docmost/mcp build is stale (tool-specs changed since last build) — run: pnpm --filter @docmost/mcp build',
);
}
return mod;
})().catch((err) => {
// Do not cache a rejected import — allow the next call to retry.