Skip to content

Commit 604eb34

Browse files
dmealingclaude
andcommitted
fix(cli): meta init declares what meta gen's output imports, not just what its generators do
A brand-new project could not run the next step the tool itself prints. `npm init -y`, install the CLI, `meta init`, `meta gen`, then `npx tsc` — the line `meta gen` ends on — and pnpm reported NINE TS2307s across all three generated files, on five specifiers the generated code imports and no manifest declared: drizzle-orm, zod, fastify, `@metaobjectsdev/runtime-ts` and its `/drizzle-fastify` subpath. Under npm, hoisting out of the CLI's own tree hides four of the five and only fastify fails; the release smoke test runs both arms for exactly this reason. `addScaffoldDevDependencies` already fixed this defect one layer in, and its docstring records why: the generator sources under `codegen/generators/` import `@metaobjectsdev/codegen-ts` and `@metaobjectsdev/metadata`, nothing declared them, and "the scaffold arrived un-typecheckable: ten TS2307s on files `meta init` had just written." The identical argument reaches one layer further out — to the code `meta gen` writes — and nobody followed it there. They go in `dependencies`, not `devDependencies`: generated routes and queries are application source that runs in production. The two build-time packages stay where they are. The RANGES are read from `@metaobjectsdev/runtime-ts`'s own `peerDependencies` rather than written into the scaffolder. That package already declares the versions its helpers are built against, bounded above (the peer-range gate enforces the bound), so a second copy here would be a second thing to keep in step, and the failure mode of drift is an adopter installing a major nothing was tested against. A range that cannot be read means the package is SKIPPED, never guessed at. Proven against the published artifact, not just locally: the pnpm arm of the rc.3 smoke test went from 9 errors to `tsc --noEmit` exit 0 with exactly these four declarations added and nothing else changed. The gate asks the question the manifest can answer. Every test that runs `gen` scaffolds into `test/fixtures/__tmp__/` DELIBERATELY, so node resolution walks up into cli's own node_modules where fastify, drizzle-orm and zod all sit — `mkGenProjectDir`'s comment says so outright. That is why the generated imports resolved in every existing test for a reason having nothing to do with the adopter's manifest, and why none of them could see this. The new test runs in the same place and asks whether the scaffolder DECLARED each bare specifier, which is answerable from the manifest alone and is what a strict installer enforces. It also asserts the four specifiers are really present in the output, so a regression that stops emitting them cannot make it pass vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3v6M6uqECZ2Sb5eUv6YY
1 parent 4cc54a4 commit 604eb34

2 files changed

Lines changed: 195 additions & 0 deletions

File tree

server/typescript/packages/cli/src/commands/init.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { mkdir, writeFile, readFile, readdir, stat, rm } from "node:fs/promises"
22
import { join } from "node:path";
33
import { basename, dirname } from "node:path";
44
import { existsSync as existsSyncWrap, readFileSync as readFileSyncWrap } from "node:fs";
5+
import { createRequire } from "node:module";
56
import { DEFAULT_CONFIG, ConfigSchema, saveConfig, PACKAGE_MANIFEST_FILE, DEFAULT_METADATA_DIR, DEFAULT_METAOBJECTS_DIR } from "@metaobjectsdev/sdk";
67
import {
78
assemble, resolveAgentContextRoot, planScaffold,
@@ -774,6 +775,7 @@ async function ensureEsmPackageType(cwd: string, result: InitResult): Promise<vo
774775
const declaredType = pkg.type; // read BEFORE the mutation below overwrites it
775776
pkg.type = "module";
776777
const added = addScaffoldDevDependencies(pkg);
778+
const addedRuntime = addScaffoldRuntimeDependencies(pkg);
777779
// Preserve the file's existing indentation rather than reformatting someone's manifest.
778780
const indent = /\n(\s+)"/.exec(raw)?.[1] ?? " ";
779781
await writeFile(pkgPath, `${JSON.stringify(pkg, null, indent)}\n`, "utf8");
@@ -798,6 +800,13 @@ async function ensureEsmPackageType(cwd: string, result: InitResult): Promise<vo
798800
"Run your package manager's install before `meta gen`.",
799801
);
800802
}
803+
if (addedRuntime.length > 0) {
804+
result.warnings.push(
805+
`added ${addedRuntime.join(" + ")} to dependencies — the code \`meta gen\` writes ` +
806+
"imports them, so `npx tsc` reports TS2307 on the generated files until they are " +
807+
"installed. Run your package manager's install before `meta gen`.",
808+
);
809+
}
801810
}
802811

803812
/**
@@ -844,6 +853,86 @@ function addScaffoldDevDependencies(pkg: Record<string, unknown>): string[] {
844853
return added;
845854
}
846855

856+
/**
857+
* The third-party packages the scaffolded suite's GENERATED OUTPUT imports, each named
858+
* with the artifact that imports it. This is the set, not the ranges — see
859+
* `scaffoldRuntimeDependencies` for where those come from.
860+
*
861+
* `@metaobjectsdev/runtime-ts` is deliberately not here: its range is the CLI's own
862+
* version, like the two build-time packages above, not a peer range read off itself.
863+
*/
864+
const SCAFFOLD_OUTPUT_PEERS = [
865+
"drizzle-orm", // <Entity>.ts (the table + column builders) and <Entity>.queries.ts
866+
"zod", // <Entity>.ts — the Insert/Update schemas
867+
"fastify", // <Entity>.routes.ts — `import type { FastifyInstance }`
868+
] as const;
869+
870+
/**
871+
* The runtime dependencies of the code `meta gen` will WRITE, for `dependencies`.
872+
*
873+
* `addScaffoldDevDependencies` above fixed this defect one layer in: the generator
874+
* SOURCES under `codegen/generators/` import `@metaobjectsdev/codegen-ts` and
875+
* `@metaobjectsdev/metadata`, nothing declared them, and the scaffold arrived
876+
* un-typecheckable. The same argument reaches one layer further out and was not
877+
* followed there. Generated `<Entity>.ts` / `.queries.ts` / `.routes.ts` import
878+
* drizzle-orm, zod, fastify and `@metaobjectsdev/runtime-ts/drizzle-fastify` — five
879+
* specifiers, none declared — so `npx tsc`, which is the next step `meta gen` itself
880+
* prints, reported NINE TS2307s on a brand-new project that had done nothing wrong.
881+
* npm hides four of the five by hoisting them out of the CLI's own tree; pnpm's strict
882+
* layout, which is the point of testing both, shows all five.
883+
*
884+
* `dependencies`, not `devDependencies`: generated routes and queries are application
885+
* source that runs in production. The two build-time packages stay where they are.
886+
*
887+
* The RANGES are read from `@metaobjectsdev/runtime-ts`'s own `peerDependencies` rather
888+
* than written here. That package already declares the versions its helpers are built
889+
* against, bounded above (the peer-range gate enforces the bound), so a second copy of
890+
* those ranges in the scaffolder is a second thing to keep in step — and the failure
891+
* mode of drift is an adopter installing a major nothing was tested against. If a range
892+
* cannot be read, the package is SKIPPED rather than guessed at, and `meta gen` still
893+
* type-checks for anyone whose manifest already declares it.
894+
*/
895+
function scaffoldRuntimeDependencies(): Record<string, string> {
896+
const wanted: Record<string, string> = {
897+
"@metaobjectsdev/runtime-ts": `^${cliVersion()}`,
898+
};
899+
const peers = runtimeTsPeerRanges();
900+
for (const name of SCAFFOLD_OUTPUT_PEERS) {
901+
const range = peers[name];
902+
if (range !== undefined) wanted[name] = range;
903+
}
904+
return wanted;
905+
}
906+
907+
/** `@metaobjectsdev/runtime-ts`'s declared peer ranges, or `{}` if it cannot be read. */
908+
function runtimeTsPeerRanges(): Record<string, string> {
909+
try {
910+
const req = createRequire(import.meta.url);
911+
const manifestPath = req.resolve("@metaobjectsdev/runtime-ts/package.json");
912+
const manifest = JSON.parse(readFileSyncWrap(manifestPath, "utf8")) as PackageManifest;
913+
return (manifest.peerDependencies ?? {}) as Record<string, string>;
914+
} catch {
915+
return {};
916+
}
917+
}
918+
919+
/** Adds any missing `scaffoldRuntimeDependencies()` to `dependencies`. Only ever ADDS a
920+
* missing key — an existing pin, in any of the four dependency fields, is the user's. */
921+
function addScaffoldRuntimeDependencies(pkg: Record<string, unknown>): string[] {
922+
const deps = (pkg.dependencies ?? {}) as Record<string, string>;
923+
const declared = declaredDependencyNames(pkg as PackageManifest);
924+
const added: string[] = [];
925+
for (const [name, range] of Object.entries(scaffoldRuntimeDependencies())) {
926+
if (declared.has(name)) continue;
927+
deps[name] = range;
928+
added.push(name);
929+
}
930+
if (added.length > 0) {
931+
pkg.dependencies = Object.fromEntries(Object.entries(deps).sort(([a], [b]) => a.localeCompare(b)));
932+
}
933+
return added;
934+
}
935+
847936
/** True when the project has hand-written CommonJS at the root (excluding tooling dirs). */
848937
async function hasCommonJsSources(cwd: string): Promise<boolean> {
849938
const SKIP = new Set(["node_modules", ".git", "dist", "build", ".metaobjects", "codegen"]);
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Everything `meta gen` WRITES must be resolvable from what `meta init` DECLARED.
2+
//
3+
// It was not. A brand-new project — `npm init -y`, install the CLI, `meta init`,
4+
// `meta gen`, then `npx tsc`, which is the next step `meta gen` itself prints —
5+
// reported NINE TS2307s under pnpm, on five specifiers the generated files import and
6+
// no manifest declared: drizzle-orm, zod, fastify, `@metaobjectsdev/runtime-ts` and its
7+
// `/drizzle-fastify` subpath. npm hides four of the five by hoisting them out of the
8+
// CLI's own dependency tree; pnpm's strict layout shows all five, which is why the
9+
// release smoke test runs both.
10+
//
11+
// `addScaffoldDevDependencies` had already fixed this defect ONE LAYER IN — the
12+
// generator sources under `codegen/generators/` import `@metaobjectsdev/codegen-ts` and
13+
// `@metaobjectsdev/metadata`, and its own docstring records the scaffold arriving
14+
// un-typecheckable with ten TS2307s. The identical argument reaches the generated
15+
// output and nobody followed it there.
16+
//
17+
// Why no existing test could see it: every test that runs `gen` scaffolds into
18+
// `test/fixtures/__tmp__/`, deliberately, so that node resolution walks up into cli's
19+
// OWN node_modules — where fastify, drizzle-orm and zod all sit as devDependencies.
20+
// `mkGenProjectDir`'s comment says so outright. So the generated imports resolved in
21+
// every test for a reason that has nothing to do with the adopter's manifest. This test
22+
// runs in the same place and asks a different question: not "does it resolve here" but
23+
// "did the scaffolder DECLARE it", which is answerable from the manifest alone and is
24+
// exactly what a strict installer will enforce.
25+
import { describe, test, expect } from "bun:test";
26+
import { mkdtempSync, mkdirSync, rmSync, readFileSync, writeFileSync, readdirSync } from "node:fs";
27+
import { join } from "node:path";
28+
import { initCommand } from "../src/commands/init.js";
29+
import { declaredDependencyNames, type PackageManifest } from "../src/lib/package-manifest.js";
30+
31+
const PROBE_ENTITY = JSON.stringify({
32+
metadata: {
33+
package: "probe",
34+
children: [{
35+
"object.entity": {
36+
name: "Author",
37+
children: [
38+
{ "source.rdb": { "@table": "authors" } },
39+
{ "field.string": { name: "id" } },
40+
{ "field.string": { name: "penName", "@column": "pen_name" } },
41+
{ "identity.primary": { "@fields": ["id"] } },
42+
],
43+
},
44+
}],
45+
},
46+
}, null, 2);
47+
48+
/** Bare package specifiers imported by a generated file — not relative, not `node:`.
49+
* A subpath resolves to its package (`@scope/pkg/sub` -> `@scope/pkg`), which is what
50+
* a manifest declares. */
51+
function bareImports(source: string): Set<string> {
52+
const found = new Set<string>();
53+
for (const m of source.matchAll(/(?:^|\n)\s*(?:import|export)[\s\S]*?from\s+"([^"]+)"/g)) {
54+
const spec = m[1];
55+
if (spec === undefined || spec.startsWith(".") || spec.startsWith("node:")) continue;
56+
const parts = spec.split("/");
57+
found.add(spec.startsWith("@") ? parts.slice(0, 2).join("/") : (parts[0] ?? spec));
58+
}
59+
return found;
60+
}
61+
62+
describe("meta init declares what meta gen's output imports", () => {
63+
test("every bare specifier in the generated files is a declared dependency", async () => {
64+
// In-package, so `gen` can resolve the scaffolded generators' own imports.
65+
const root = join(import.meta.dirname, "fixtures", "__tmp__");
66+
mkdirSync(root, { recursive: true });
67+
const dir = mkdtempSync(join(root, "scaffold-deps-"));
68+
try {
69+
// The `npm init -y` shape, which is the documented first step.
70+
writeFileSync(
71+
join(dir, "package.json"),
72+
`${JSON.stringify({ name: "probe", version: "1.0.0", type: "commonjs" }, null, 2)}\n`,
73+
);
74+
expect(await initCommand([], dir)).toBe(0);
75+
writeFileSync(join(dir, "metaobjects", "meta.common.json"), PROBE_ENTITY);
76+
const { genCommand } = await import("../src/commands/gen.js");
77+
expect(await genCommand([], dir)).toBe(0);
78+
79+
const outDir = join(dir, "src", "generated");
80+
const emitted = readdirSync(outDir).filter((f) => f.endsWith(".ts"));
81+
expect(emitted.length).toBeGreaterThan(0);
82+
83+
const declared = declaredDependencyNames(
84+
JSON.parse(readFileSync(join(dir, "package.json"), "utf8")) as PackageManifest,
85+
);
86+
const undeclared: string[] = [];
87+
for (const file of emitted) {
88+
for (const spec of bareImports(readFileSync(join(outDir, file), "utf8"))) {
89+
if (!declared.has(spec)) undeclared.push(`${file} imports ${spec}`);
90+
}
91+
}
92+
// Named individually: the failure an adopter sees is a list of TS2307s, and the
93+
// useful message here is the same list, before they ever run tsc.
94+
expect(undeclared).toEqual([]);
95+
96+
// ...and the specifiers that caused this test to exist are really present in the
97+
// output, so a regression that stops EMITTING them cannot make it pass vacuously.
98+
const all = new Set(emitted.flatMap((f) => [...bareImports(readFileSync(join(outDir, f), "utf8"))]));
99+
for (const required of ["drizzle-orm", "zod", "fastify", "@metaobjectsdev/runtime-ts"]) {
100+
expect([...all]).toContain(required);
101+
}
102+
} finally {
103+
rmSync(dir, { recursive: true, force: true });
104+
}
105+
});
106+
});

0 commit comments

Comments
 (0)