From 8502803f6c04c9f64ba237f2365a8e93dd692f7f Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sun, 13 Sep 2026 21:49:40 -0400 Subject: [PATCH 1/2] fix(metadata): gate the subpath-resolution class, and close the one still open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An independent review of the four ungated commits on main found the /library fix had a sibling still latent, and three claims in that work that the code does not support. This is all of it. **The class, gated.** `./vocabulary-rewrite-yaml` pointed at `src/core/vocabulary-rewrite-yaml.ts` — one directory deeper than `tsconfig.scripts.json`'s `@metaobjectsdev/metadata/*` → `src/*` substitution can reach. Exactly the shape that took the `gates` lane down after the FR-043 merge, and silent for the same reason: tsc misses, falls through to `node_modules`, and reads `dist/`, which a developer has and a fresh CI checkout does not. It had not gone red only because nothing in the scripts typecheck's include list imports it yet. A thin `src/vocabulary-rewrite-yaml.ts` re-export fixes it; the implementation stays under `core/` beside the canonical-JSON rewriter it mirrors. The fix that matters is the gate. `subpath-resolves-under-scripts-paths.test.ts` walks every `@metaobjectsdev//*` entry in the paths map, reads that package's own exports, and asserts each TypeScript subpath lands on something the substitution reaches. Asset exports (`./form.css`, whose target is the stylesheet) are excluded by target kind, since a bundler resolves those and tsc never does. It fails on `vocabulary-rewrite-yaml` before this commit's fix and passes after. **Three claims corrected.** All three were mine, in work already on main; the commits carrying them cannot be amended, so the corrections live here. The largest: `3adfa2d76` said the old `text.includes(`"${ref}"`)` embed check was "not currently wrong … fragile rather than broken". That is true of TypeScript, Java and C# and FALSE of Python, where the check was VACUOUS. There are two embed generators, not one — `generate_embedded_library.py` emits through `repr()`, which single-quotes the key and leaves inner `"` raw, so the bare `"ai/db"` in that file is the payload comment and never the key. Measured this time rather than inferred: delete each key line from each real embed and the old predicate goes false on TS/Java/C# and stays true on Python, six refs for six. So the rewrite closed a live hole on one port, not merely a fragility on four. The same commit's claim that the backslash exclusion is what separates key from mention holds only for Java, whose pattern is unanchored; elsewhere the line anchor does it. `6c39053cb` said pointing a subpath at an index is "what every other subpath does". `./constants` is a plain `src/constants.ts` and is fine. The invariant is that the substitution must land on something — which is what the new gate asserts, and which a file satisfies as well as a directory. `099d2d87c` cited "8 of 9 passed" as evidence the pre-change file was blind to the deleted guard. That file had 7 tests; 8 of 9 is the post-change run, which that message's own last line already reports. The pre-change figure is 7 of 7. Also: the cross-port set comparison took `NAMES.flatMap(declaredRefs)` as a list and compared it against a Set, so a ref legitimately declared by two layers of one library would have failed every port against embeds that were correct. Now de-duplicated. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011cQxyVuTkdPmAduLiedNEg --- .../test/library-manifest-resolved.test.ts | 51 +++++--- ...bpath-resolves-under-scripts-paths.test.ts | 121 ++++++++++++++++++ .../typescript/packages/metadata/package.json | 6 +- .../packages/metadata/src/library/index.ts | 10 +- .../metadata/src/vocabulary-rewrite-yaml.ts | 14 ++ 5 files changed, 176 insertions(+), 26 deletions(-) create mode 100644 server/typescript/packages/cli/test/subpath-resolves-under-scripts-paths.test.ts create mode 100644 server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts 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..c162b05f4 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: the right-hand side comes from a Set, so a ref legitimately declared + // by two layers of one library would otherwise fail every port on a set the embeds + // carry correctly. + 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..46a436be6 --- /dev/null +++ b/server/typescript/packages/cli/test/subpath-resolves-under-scripts-paths.test.ts @@ -0,0 +1,121 @@ +// Gate — every exports-map subpath must resolve under `tsconfig.scripts.json`'s `paths`. +// +// That file maps `@metaobjectsdev//*` to `/src/*` so the repo-root `scripts/` +// typecheck reads workspace SOURCE rather than build output. The substitution resolves a +// subpath the way TypeScript resolves any module: `src/.ts`, or `src//index.ts` +// if `` is a directory. A subpath whose `package.json` entry points somewhere else — +// `./src/core/vocabulary-rewrite-yaml.ts`, say, which is a directory deeper than the +// substitution can reach — has nothing to land on. +// +// 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. +// +// That is not hypothetical: `./library` shipped in exactly this state and took the lane +// down after the FR-043 merge. This gate is the general form of that fix, because the same +// mistake is available to every subpath added from here on. +import { describe, test, expect } from "bun:test"; +import { existsSync, readFileSync } 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); + +/** What TypeScript will try for a `paths` substitution that lands on `candidate`. */ +function resolvesAsModule(candidate: string): boolean { + const asFile = [".ts", ".tsx", ".d.ts", ".js", ".jsx"].some((ext) => existsSync(candidate + ext)); + const asDir = [".ts", ".tsx", ".d.ts", ".js", ".jsx"].some((ext) => + existsSync(join(candidate, `index${ext}`)), + ); + return asFile || asDir; +} + +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("no declared subpath falls through to node_modules", () => { + const misses: string[] = []; + let checked = 0; + + for (const { pkg, srcDir, pkgDir } of entries) { + const manifestPath = join(ROOT, pkgDir, "package.json"); + if (!existsSync(manifestPath)) continue; // a mapping with no package is its own problem + 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; + // Only TypeScript modules are subject to this. An ASSET export — `./form.css`, + // whose target is the stylesheet itself — is resolved by a bundler, never by tsc, + // so `paths` having nothing to offer it is correct rather than a defect. + const source = typeof target === "string" ? target : (target.bun ?? target.types ?? ""); + if (!/\.tsx?$/.test(source)) continue; + checked++; + if (!resolvesAsModule(join(ROOT, srcDir, sub.slice(2)))) { + misses.push( + `${pkg}${sub.slice(1)} — paths yields "${join(srcDir, sub.slice(2))}", ` + + `and neither that file nor an index.ts under it exists`, + ); + } + } + } + + 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 will fall through to node_modules and read the package's built `dist/`, so this\n" + + "compiles on a machine that 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: point the subpath at `src/.ts` or `src//index.ts`.\n" + + "Do NOT add a per-subpath entry to `tsconfig.scripts.json` — the mapping is the\n" + + "contract, and widening it per package is how it stops being one.\n " + + misses.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..91b65a215 100644 --- a/server/typescript/packages/metadata/src/library/index.ts +++ b/server/typescript/packages/metadata/src/library/index.ts @@ -1,8 +1,9 @@ // @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. It is that the subpath must land on something the +// `paths` substitution can reach. This one pointed at `library-sources.ts` under a +// directory, so it reached nothing, and 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 +15,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..563bf6aaa --- /dev/null +++ b/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts @@ -0,0 +1,14 @@ +// @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 has something to resolve to: `tsconfig.scripts.json` +// maps `@metaobjectsdev/metadata/*` to `src/*`, and a subpath declared as +// `src/core/vocabulary-rewrite-yaml.ts` is a directory deeper than that substitution can +// reach. Without an entry here, tsc misses, falls through to `node_modules`, and reads +// `dist/` — which exists on a machine that has built and not on a fresh CI checkout, where +// the `gates` lane runs `bun install` and never builds. +// +// `./library` shipped in exactly that state and took the lane down; this one had not been +// imported from anywhere `scripts/` typechecks yet, so it was latent rather than red. +// `cli/test/subpath-resolves-under-scripts-paths.test.ts` now gates the class. +export { rewriteYamlDocument, type YamlRewriteResult } from "./core/vocabulary-rewrite-yaml.js"; From b1d9ca9eab3bf8a38d8d42e9e8aec04e39234154 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sun, 13 Sep 2026 21:58:27 -0400 Subject: [PATCH 2/2] fix(gate): the subpath gate must compare against the DECLARED target, not just find a file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings from `/code-review high` on the previous commit. The one that mattered: the gate checked that `src/` resolves and never compared that result to what the package's `bun` condition declares — so tsc and the runtime could read DIFFERENT modules and the gate stayed green. Proven by reverting `./library`'s `bun` to `library-sources.ts` with `index.ts` still present: the old gate passed while `paths` read the index and bun read the other file. That is the type-level form of the identity defect `tsconfig.scripts.json`'s own rationale is written against, and following `library/index.ts`'s instruction to re-export new modules there would have typechecked clean and been `undefined` at runtime. The gate now resolves both sides and compares them by realpath; the same mutation now fails it, naming both files. Three more holes in the gate itself, all of which let something escape unchecked: a subpath declaring no `bun` condition was skipped rather than failed, though it is precisely a TS subpath the scripts typecheck cannot reach through source; a subpath PATTERN (`./templates/*`) was treated as a literal and would have reported a false miss against a correct package; and a `paths` target not shaped `/src/*` made the derived manifest path miss, which `continue`d silently and dropped that whole package from the gate. Patterns are now skipped explicitly, the other two fail loudly. And a premise correction, in three headers and the commit before this one. "A directory deeper than the substitution can reach" is WRONG: `paths` `*` matches across `/`, which is why `@metaobjectsdev/codegen-ts/templates/entity-file` resolves to `src/templates/entity-file.ts` without trouble. The actual rule is that the subpath NAME must mirror the layout under `src/` — `vocabulary-rewrite-yaml` named a module living at `src/core/vocabulary-rewrite-yaml.ts`. Stated the wrong way, the next maintainer concludes a nested module can never back a subpath and flattens files out of `core/` for no reason. Renaming the export to `./core/vocabulary-rewrite-yaml` would have satisfied the rule more cheaply; the shim stays because that subpath is public API of a published package. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011cQxyVuTkdPmAduLiedNEg --- .../test/library-manifest-resolved.test.ts | 6 +- ...bpath-resolves-under-scripts-paths.test.ts | 144 ++++++++++++++---- .../packages/metadata/src/library/index.ts | 8 +- .../metadata/src/vocabulary-rewrite-yaml.ts | 23 ++- 4 files changed, 135 insertions(+), 46 deletions(-) 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 c162b05f4..31ad7804a 100644 --- a/server/typescript/packages/cli/test/library-manifest-resolved.test.ts +++ b/server/typescript/packages/cli/test/library-manifest-resolved.test.ts @@ -193,9 +193,9 @@ 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()); - // De-duplicated: the right-hand side comes from a Set, so a ref legitimately declared - // by two layers of one library would otherwise fail every port on a set the embeds - // carry correctly. + // 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); 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 index 46a436be6..69182d624 100644 --- 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 @@ -1,22 +1,29 @@ -// Gate — every exports-map subpath must resolve under `tsconfig.scripts.json`'s `paths`. +// 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 substitution resolves a -// subpath the way TypeScript resolves any module: `src/.ts`, or `src//index.ts` -// if `` is a directory. A subpath whose `package.json` entry points somewhere else — -// `./src/core/vocabulary-rewrite-yaml.ts`, say, which is a directory deeper than the -// substitution can reach — has nothing to land on. +// 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. // -// That is not hypothetical: `./library` shipped in exactly this state and took the lane -// down after the FR-043 merge. This gate is the general form of that fix, because the same -// mistake is available to every subpath added from here on. +// 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 } from "node:fs"; +import { existsSync, readFileSync, realpathSync } from "node:fs"; import { dirname, join } from "node:path"; function findRepoRoot(start: string): string { @@ -30,14 +37,38 @@ function findRepoRoot(start: string): string { } const ROOT = findRepoRoot(import.meta.dir); +const MODULE_EXTS = [".ts", ".tsx", ".d.ts", ".js", ".jsx"] as const; -/** What TypeScript will try for a `paths` substitution that lands on `candidate`. */ -function resolvesAsModule(candidate: string): boolean { - const asFile = [".ts", ".tsx", ".d.ts", ".js", ".jsx"].some((ext) => existsSync(candidate + ext)); - const asDir = [".ts", ".tsx", ".d.ts", ".js", ".jsx"].some((ext) => - existsSync(join(candidate, `index${ext}`)), - ); - return asFile || asDir; +/** 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 { @@ -76,46 +107,95 @@ describe("every exports-map subpath resolves under the scripts typecheck's paths expect(entries.length, "no @metaobjectsdev/*/* wildcard entries found").toBeGreaterThan(10); }); - test("no declared subpath falls through to node_modules", () => { + 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; // a mapping with no package is its own problem + if (!existsSync(manifestPath)) continue; // reported by the test above const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as { - exports?: Record>; + 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; - // Only TypeScript modules are subject to this. An ASSET export — `./form.css`, - // whose target is the stylesheet itself — is resolved by a bundler, never by tsc, - // so `paths` having nothing to offer it is correct rather than a defect. - const source = typeof target === "string" ? target : (target.bun ?? target.types ?? ""); - if (!/\.tsx?$/.test(source)) 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++; - if (!resolvesAsModule(join(ROOT, srcDir, sub.slice(2)))) { + 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.ts under it exists`, + `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 will fall through to node_modules and read the package's built `dist/`, so this\n" + - "compiles on a machine that has built and is TS2307 on a fresh CI checkout, where the\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: point the subpath at `src/.ts` or `src//index.ts`.\n" + - "Do NOT add a per-subpath entry to `tsconfig.scripts.json` — the mapping is the\n" + - "contract, and widening it per package is how it stops being one.\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/src/library/index.ts b/server/typescript/packages/metadata/src/library/index.ts index 91b65a215..67652c09c 100644 --- a/server/typescript/packages/metadata/src/library/index.ts +++ b/server/typescript/packages/metadata/src/library/index.ts @@ -1,9 +1,11 @@ // @metaobjectsdev/metadata/library — the shipped-library surface (FR-043). // // The invariant is not "every subpath has an index" — `./constants` is a plain -// `src/constants.ts` and is fine. It is that the subpath must land on something the -// `paths` substitution can reach. This one pointed at `library-sources.ts` under a -// directory, so it reached nothing, and that was not cosmetic: the repo-root +// `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 diff --git a/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts b/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts index 563bf6aaa..090704518 100644 --- a/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts +++ b/server/typescript/packages/metadata/src/vocabulary-rewrite-yaml.ts @@ -1,14 +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 has something to resolve to: `tsconfig.scripts.json` -// maps `@metaobjectsdev/metadata/*` to `src/*`, and a subpath declared as -// `src/core/vocabulary-rewrite-yaml.ts` is a directory deeper than that substitution can -// reach. Without an entry here, tsc misses, falls through to `node_modules`, and reads -// `dist/` — which exists on a machine that has built and not on a fresh CI checkout, where -// the `gates` lane runs `bun install` and never builds. +// This file exists so the SUBPATH NAME mirrors the layout under `src/`, which is what +// `tsconfig.scripts.json`'s `@metaobjectsdev/metadata/*` → `src/*` mapping requires. // -// `./library` shipped in exactly that state and took the lane down; this one had not been -// imported from anywhere `scripts/` typechecks yet, so it was latent rather than red. +// 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";