diff --git a/server/typescript/packages/cli/test/library-manifest-resolved.test.ts b/server/typescript/packages/cli/test/library-manifest-resolved.test.ts index 81b58aca7..31ad7804a 100644 --- a/server/typescript/packages/cli/test/library-manifest-resolved.test.ts +++ b/server/typescript/packages/cli/test/library-manifest-resolved.test.ts @@ -41,10 +41,13 @@ function findRepoRoot(start: string): string { * The pattern matches a key POSITION, not the key text anywhere in the file, and that * distinction is the whole point. Every embedded value is the library's own YAML, and * that YAML talks about its own ref: `library/ai/db.yaml` opens with "Opted into as - * `\"ai/db\"`". So a substring search for `"ai/db"` hits the payload and passes even if - * the key is deleted — which is what this file used to do. A key is line-initial (or - * follows `.put(`) and contains no backslash; a payload mention is mid-string and - * escaped. Excluding `\\` from the captured class is what separates them. + * `\"ai/db\"`". + * + * What separates a key from that mention is the ANCHOR, not the escaping. For TypeScript, + * C# and Python the pattern is line-initial and every entry is one line, so a mention + * inside a value can never be at column zero. Java's `.put("` is unanchored, and there the + * `[^"\\]+` class is what does the work, since a payload occurrence carries `.put(\"`. + * Both halves are pinned by the fixtures below. */ const EMBEDS: Record = { typescript: { @@ -168,16 +171,17 @@ describe("every library manifest fact is resolved against the thing it claims", }); test(`${name}: every layer's refs are embedded in EVERY port`, () => { - // The row says `ports: [typescript, java, kotlin, csharp, python]`, and the basis - // for that claim is that one generator script writes the embed for all of them. - // Checked rather than asserted, because "the library is reachable from your port" - // is the single fact a polyglot adopter acts on. + // The row says `ports: [typescript, java, kotlin, csharp, python]`. Checked rather + // than asserted, because "the library is reachable from your port" is the single + // fact a polyglot adopter acts on — and because TWO scripts write these five files + // (`scripts/generate-embedded-library.ts` and the Python one), so agreement between + // them is a result, not a premise. const refs = declaredRefs(name); expect(refs.length, `${name} declares no refs at all`).toBeGreaterThan(0); for (const port of SERVER_LANGS) { const embedded = embeddedKeys(port).refs; for (const ref of refs) { - expect([...embedded].includes(ref), `${port} does not embed ${ref}`).toBe(true); + expect(embedded.has(ref), `${port} does not embed ${ref}`).toBe(true); } } }); @@ -189,7 +193,10 @@ describe("every library manifest fact is resolved against the thing it claims", // pattern that silently stops matching would make every containment check above // vacuously true, which is the way a gate of this shape stops gating. expect(Object.keys(EMBEDS).sort(), "a port with no embed mapping").toEqual([...SERVER_LANGS].sort()); - const allRefs = NAMES.flatMap(declaredRefs).sort(); + // De-duplicated: this is the side built from manifests, and it is compared against a + // set extracted from each embed — so a ref legitimately declared by two layers of one + // library would otherwise fail every port against embeds that are correct. + const allRefs = [...new Set(NAMES.flatMap(declaredRefs))].sort(); expect(allRefs.length, "no library declares any ref").toBeGreaterThan(0); for (const port of SERVER_LANGS) { @@ -200,17 +207,21 @@ describe("every library manifest fact is resolved against the thing it claims", }); describe("the key extractor reads a POSITION, not a spelling", () => { - // What this replaced was `text.includes(`"${ref}"`)` over the same files. That check - // is not currently wrong — every embedded payload escapes its own quotes, so a ref - // named in a comment reads as `\\"ai/db\\"` and the bare `"ai/db"` occurs exactly once, - // as the key. It is fragile rather than broken: it cannot see an EXTRA key, it cannot - // tell the ref record from the manifest record, and it holds only for as long as the - // generator keeps escaping payloads the way it does today. + // What this replaced was `text.includes(`"${ref}"`)` over the same files, and on ONE + // port that check was not weak but VACUOUS. There are two embed generators, not one: + // `scripts/generate-embedded-library.ts` writes TS/Java/C# and escapes inner quotes, + // so a ref named in a payload reads as `\\"ai/db\\"` and the bare `"ai/db"` occurs once, + // as the key. `server/python/scripts/generate_embedded_library.py` emits through + // `repr()`, which single-quotes the key and leaves inner `"` RAW — so in + // `embedded_library.py` the key is `\'ai/db\'` while the payload carries a bare + // `"ai/db"`, and the old assertion was matching the comment and never the key. + // Measured, not inferred: with each key line deleted from each real embed, the old + // predicate went false on TS/Java/C# and stayed TRUE on Python, for all six refs. // - // The extractor does not depend on that. A key is a POSITION — line-initial, or after - // `.put(` — so a mention anywhere inside a value is not a key however it is spelled. - // These fixtures pin that, including the unescaped case a change to the generator - // would produce, where a substring check WOULD produce a false positive. + // The extractor does not depend on either convention. A key is a POSITION — line- + // initial, or after `.put(` — so a mention inside a value is not a key however it is + // spelled. The fixtures below pin both the escaped and the unescaped form, the latter + // being today's Python output rather than a hypothetical. const ESCAPED = JSON.stringify('"ai/db"').slice(1, -1); // \"ai/db\" /** One line of each record, in that port's syntax, for an arbitrary ref key. */ diff --git a/server/typescript/packages/cli/test/subpath-resolves-under-scripts-paths.test.ts b/server/typescript/packages/cli/test/subpath-resolves-under-scripts-paths.test.ts new file mode 100644 index 000000000..69182d624 --- /dev/null +++ b/server/typescript/packages/cli/test/subpath-resolves-under-scripts-paths.test.ts @@ -0,0 +1,201 @@ +// Gate — every exports-map subpath must resolve, under `tsconfig.scripts.json`'s `paths`, +// to THE SAME MODULE the package declares. +// +// That file maps `@metaobjectsdev//*` to `/src/*` so the repo-root `scripts/` +// typecheck reads workspace SOURCE rather than build output. The `*` matches across `/`, +// so nesting is fine — `./templates/entity-file` substitutes to +// `src/templates/entity-file.ts` and is correct. The rule is narrower than "no nesting": +// the subpath NAME must mirror the layout under `src/`. `./vocabulary-rewrite-yaml` whose +// source sits at `src/core/vocabulary-rewrite-yaml.ts` breaks it, because the substitution +// yields `src/vocabulary-rewrite-yaml` and that is not where the module is. +// +// Missing is not the same as failing. tsc falls through to `node_modules`, finds the +// package's own `dist/**/*.d.ts`, and compiles clean — on a machine that has built. The +// `gates` lane runs `bun install` and never builds, so `dist/` is absent there and the same +// specifier is `TS2307`. Green locally, red on CI, and only for whoever imports it first. +// `./library` shipped in exactly that state and took the lane down after the FR-043 merge. +// +// Resolving to SOMETHING is not enough, which is the second half. If `paths` lands on one +// module and the `bun` condition names another, tsc and the runtime disagree silently — +// the type-level form of the cross-package identity defect `tsconfig.scripts.json`'s own +// rationale is written against. Following `src/library/index.ts`'s instruction to +// re-export new modules there, while `bun` still pointed at `library-sources.ts`, would +// typecheck clean and be `undefined` at runtime. So both are asserted: the substitution +// must land, and it must land on the declared file. +import { describe, test, expect } from "bun:test"; +import { existsSync, readFileSync, realpathSync } from "node:fs"; +import { dirname, join } from "node:path"; + +function findRepoRoot(start: string): string { + let dir = start; + for (;;) { + if (existsSync(join(dir, "fixtures")) && existsSync(join(dir, "server"))) return dir; + const parent = dirname(dir); + if (parent === dir) throw new Error("no repo root (a dir holding fixtures/ and server/)"); + dir = parent; + } +} + +const ROOT = findRepoRoot(import.meta.dir); +const MODULE_EXTS = [".ts", ".tsx", ".d.ts", ".js", ".jsx"] as const; + +/** Where TypeScript lands for a `paths` substitution of `candidate`, or undefined. */ +function resolveModule(candidate: string): string | undefined { + for (const ext of MODULE_EXTS) if (existsSync(candidate + ext)) return candidate + ext; + for (const ext of MODULE_EXTS) { + const idx = join(candidate, `index${ext}`); + if (existsSync(idx)) return idx; + } + return undefined; +} + +type ExportTarget = string | Record; + +/** + * The TypeScript SOURCE a subpath declares, by this repo's convention that the `bun` + * condition names it. Undefined for an export that is not TypeScript source at all — an + * asset like `./form.css`, whose target is the stylesheet and which a bundler resolves and + * tsc never does. + * + * A conditions object that declares no `bun` is NOT silently skipped; it comes back as + * `null` so the caller can fail it. Swallowing that is how a broken TS subpath would + * escape a gate written to catch broken TS subpaths. + */ +function declaredSource(target: ExportTarget): string | undefined | null { + if (typeof target === "string") return /\.tsx?$/.test(target) ? target : undefined; + const bun = target["bun"]; + if (typeof bun === "string") return /\.tsx?$/.test(bun) ? bun : undefined; + // No `bun` condition. If nothing here looks like a module at all, treat it as an asset; + // otherwise it is a TS export the scripts typecheck cannot reach through source. + const anyModule = JSON.stringify(target).includes(".js") || JSON.stringify(target).includes(".ts"); + return anyModule ? null : undefined; +} + +interface Wildcard { + /** The package specifier prefix, e.g. `@metaobjectsdev/metadata`. */ + pkg: string; + /** Repo-relative directory the `*` expands under, e.g. `server/.../metadata/src`. */ + srcDir: string; + /** Repo-relative package root, where its `package.json` lives. */ + pkgDir: string; +} + +/** Every `@metaobjectsdev//*` entry in the scripts typecheck's `paths` map. */ +function wildcards(): Wildcard[] { + // `tsconfig.scripts.json` carries its rationale under a `"//"` KEY, which is ordinary + // JSON — no comment stripping needed. + const cfg = JSON.parse(readFileSync(join(ROOT, "tsconfig.scripts.json"), "utf8")) as { + compilerOptions: { paths: Record }; + }; + const out: Wildcard[] = []; + for (const [spec, targets] of Object.entries(cfg.compilerOptions.paths)) { + if (!spec.endsWith("/*")) continue; + const target = targets[0]; + if (target === undefined || !target.endsWith("/*")) continue; + const srcDir = target.replace(/^\.\//, "").slice(0, -2); // "./a/b/src/*" -> "a/b/src" + out.push({ pkg: spec.slice(0, -2), srcDir, pkgDir: dirname(srcDir) }); + } + return out; +} + +describe("every exports-map subpath resolves under the scripts typecheck's paths map", () => { + const entries = wildcards(); + + test("the paths map was actually read", () => { + // Guards every assertion below from passing over an empty list — the way a gate of + // this shape silently stops gating. + expect(entries.length, "no @metaobjectsdev/*/* wildcard entries found").toBeGreaterThan(10); + }); + + test("every mapped package's manifest is findable", () => { + // `pkgDir` assumes the target is `/src/*`. A target a segment deeper would + // point this at a directory with no `package.json`, and skipping it quietly would drop + // that package from the gate with no signal — so it fails here instead. + const unfindable = entries + .filter(({ pkgDir }) => !existsSync(join(ROOT, pkgDir, "package.json"))) + .map(({ pkg, pkgDir }) => `${pkg} — no package.json at ${pkgDir}`); + expect(unfindable, "a paths entry whose package root could not be derived").toEqual([]); + }); + + test("each subpath lands, and lands on the module the package declares", () => { + const misses: string[] = []; + const diverged: string[] = []; + const unreachable: string[] = []; + let checked = 0; + + for (const { pkg, srcDir, pkgDir } of entries) { + const manifestPath = join(ROOT, pkgDir, "package.json"); + if (!existsSync(manifestPath)) continue; // reported by the test above + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as { + exports?: Record; + }; + + for (const [sub, target] of Object.entries(manifest.exports ?? {})) { + // `.` is the root entry, which `paths` maps separately (without the `/*`). + // `./package.json` is resolved by `require.resolve`, never as a type-level import. + if (sub === "." || sub === "./package.json") continue; + // A subpath PATTERN stands for many modules, so "does it resolve" has no single + // answer; whatever it expands to is covered by the files themselves. + if (sub.includes("*")) continue; + + const source = declaredSource(target); + if (source === undefined) continue; // asset export — not tsc's to resolve + if (source === null) { + unreachable.push(`${pkg}${sub.slice(1)} — declares no \`bun\` condition naming source`); + continue; + } + + checked++; + const viaPaths = resolveModule(join(ROOT, srcDir, sub.slice(2))); + if (viaPaths === undefined) { + misses.push( + `${pkg}${sub.slice(1)} — paths yields "${join(srcDir, sub.slice(2))}", ` + + `and neither that file nor an index under it exists`, + ); + continue; + } + const declaredAbs = join(ROOT, pkgDir, source.replace(/^\.\//, "")); + if (!existsSync(declaredAbs)) { + misses.push(`${pkg}${sub.slice(1)} — declared source "${source}" does not exist`); + continue; + } + if (realpathSync(viaPaths) !== realpathSync(declaredAbs)) { + diverged.push( + `${pkg}${sub.slice(1)} — paths reads "${viaPaths.slice(ROOT.length + 1)}" ` + + `but \`bun\` declares "${source}"`, + ); + } + } + } + + expect(checked, "no subpaths were checked at all").toBeGreaterThan(0); + + expect( + misses, + "A declared subpath that `tsconfig.scripts.json` cannot resolve to source.\n" + + "tsc falls through to node_modules and reads the package's built `dist/`, so this\n" + + "compiles where someone has built and is TS2307 on a fresh CI checkout, where the\n" + + "`gates` lane runs `bun install` and never builds.\n" + + "Fix the package: the subpath NAME must mirror the layout under `src/` (nesting is\n" + + "fine — the `*` matches across `/`). Do NOT add a per-subpath entry to\n" + + "`tsconfig.scripts.json`; the mapping is the contract.\n " + + misses.join("\n "), + ).toEqual([]); + + expect( + diverged, + "A subpath where tsc and the runtime read DIFFERENT modules. Types would be taken\n" + + "from one file and values from another — an export added to only one of them\n" + + "typechecks clean and is `undefined` at runtime.\n " + + diverged.join("\n "), + ).toEqual([]); + + expect( + unreachable, + "A TypeScript subpath with no `bun` condition. This repo's convention is that `bun`\n" + + "names the source file; without it the scripts typecheck has only `dist/` to read,\n" + + "which is the failure this gate exists to prevent.\n " + + unreachable.join("\n "), + ).toEqual([]); + }); +}); diff --git a/server/typescript/packages/metadata/package.json b/server/typescript/packages/metadata/package.json index f461c7d1e..d9db44086 100644 --- a/server/typescript/packages/metadata/package.json +++ b/server/typescript/packages/metadata/package.json @@ -27,9 +27,9 @@ "default": "./dist/constants.js" }, "./vocabulary-rewrite-yaml": { - "bun": "./src/core/vocabulary-rewrite-yaml.ts", - "types": "./dist/core/vocabulary-rewrite-yaml.d.ts", - "default": "./dist/core/vocabulary-rewrite-yaml.js" + "bun": "./src/vocabulary-rewrite-yaml.ts", + "types": "./dist/vocabulary-rewrite-yaml.d.ts", + "default": "./dist/vocabulary-rewrite-yaml.js" } }, "files": [ diff --git a/server/typescript/packages/metadata/src/library/index.ts b/server/typescript/packages/metadata/src/library/index.ts index 4f028085f..67652c09c 100644 --- a/server/typescript/packages/metadata/src/library/index.ts +++ b/server/typescript/packages/metadata/src/library/index.ts @@ -1,8 +1,11 @@ // @metaobjectsdev/metadata/library — the shipped-library surface (FR-043). // -// Every other subpath of this package resolves through a directory `index.ts` -// (`/core`, and the root entry itself). This one pointed straight at -// `library-sources.ts`, and that inconsistency was not cosmetic: the repo-root +// The invariant is not "every subpath has an index" — `./constants` is a plain +// `src/constants.ts` and is fine — nor is it "no nesting", since the `paths` `*` matches +// across `/`. It is that the subpath NAME must mirror the layout under `src/`. A subpath +// named `library` substitutes to `src/library`, so the module has to be `src/library.ts` +// or `src/library/index.ts`; it was `src/library/library-sources.ts`, which the +// substitution never reaches. That was not cosmetic: the repo-root // `tsconfig.scripts.json` maps `@metaobjectsdev/metadata/*` to // `packages/metadata/src/*` so that `scripts/` typechecks against workspace // SOURCE rather than a build output. With no `index.ts` here that mapping had @@ -14,4 +17,7 @@ // // So this file is the subpath's entry, and `package.json` names it. Adding a // module under `library/` means re-exporting it here. +// +// `cli/test/subpath-resolves-under-scripts-paths.test.ts` now gates the whole class, +// across every package the scripts typecheck maps. export * from "./library-sources.js"; diff --git a/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts b/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts new file mode 100644 index 000000000..090704518 --- /dev/null +++ b/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts @@ -0,0 +1,21 @@ +// @metaobjectsdev/metadata/vocabulary-rewrite-yaml — the YAML arm of `meta upgrade`. +// +// The implementation stays under `core/`, beside the canonical-JSON rewriter it mirrors. +// This file exists so the SUBPATH NAME mirrors the layout under `src/`, which is what +// `tsconfig.scripts.json`'s `@metaobjectsdev/metadata/*` → `src/*` mapping requires. +// +// Nesting is NOT the problem — the `*` matches across `/`, which is why +// `@metaobjectsdev/codegen-ts/templates/entity-file` resolves to +// `src/templates/entity-file.ts` perfectly well. The problem was a subpath NAMED +// `vocabulary-rewrite-yaml` whose source sat at `src/core/vocabulary-rewrite-yaml.ts`: +// the substitution yields `src/vocabulary-rewrite-yaml`, and that is not where the module +// is. Renaming the export to `./core/vocabulary-rewrite-yaml` would also have satisfied +// the rule and was the cheaper edit, but this package is published and that subpath is +// public API, so the name stays and the layout moves to meet it. +// +// When the mapping misses, tsc falls through to `node_modules` and reads `dist/` — present +// for anyone who has built, absent on a fresh CI checkout where the `gates` lane runs +// `bun install` and never builds. `./library` shipped that way and took the lane down; +// this one had not been imported from anywhere `scripts/` typechecks, so it was latent. +// `cli/test/subpath-resolves-under-scripts-paths.test.ts` now gates the class. +export { rewriteYamlDocument, type YamlRewriteResult } from "./core/vocabulary-rewrite-yaml.js";