Skip to content

Commit 2909919

Browse files
dmealingclaude
andcommitted
fix(docs): prevent silent doc-page overwrite on cross-package short-name collisions (hard-error + package layout)
Doc pages were placed by short name (<Name>.md). Two metadata nodes sharing a short name across packages (e.g. an object acme::sales::Order and a template.output acme::comms::Order) both wrote Order.md and silently overwrote each other. - New docs-paths.ts: single source of truth for page placement AND cross-link hrefs. docPageOutputPath()/docPageHref() derive a file's location and every inbound link from the SAME effective package, so they can never diverge. effectivePackage() reads the package off the node's resolutionKey() (object fqn() stays bare per FR5d). - assertNoDuplicateDocPaths(): hard backstop (ALL layouts) — two nodes resolving to the same path throw, naming both FQNs + the path. - docsFile() respects ctx.config.outputLayout; package layout folds pages under package-path subdirs (acme/sales/Order.md) with correct relative cross-links (../comms/OrderEmail.md). - meta docs gains --layout flat|package (default flat = unchanged). Duplicate-path error surfaces as a clean non-zero exit, not a trace. Flat default + existing goldens/byte-identity/neutrality unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 08e90a4 commit 2909919

8 files changed

Lines changed: 504 additions & 11 deletions

File tree

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

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,35 @@ import {
2626
import type { GenContext } from "@metaobjectsdev/codegen-ts";
2727
import { docsFile } from "@metaobjectsdev/codegen-ts/generators";
2828

29+
type DocsLayout = "flat" | "package";
30+
2931
interface DocsFlags {
3032
/** Project root holding `metaobjects/` (the metadata to document). */
3133
metadata: string;
3234
/** Output directory for the rendered pages. */
3335
out: string;
36+
/** Page-placement layout. `flat` (default) writes `<Name>.md` at the out
37+
* root; `package` folds pages under package-path subdirs (collision-proof
38+
* for multi-package models with repeated short names). */
39+
layout: DocsLayout;
3440
/** Optional override for the project root used to resolve adopter
3541
* `templates/` overrides. Defaults to the metadata root. */
3642
templates?: string;
3743
}
3844

45+
function parseLayout(v: string | undefined, flag: string): DocsLayout {
46+
if (v === undefined) throw new Error(`${flag} requires flat|package`);
47+
if (v !== "flat" && v !== "package") {
48+
throw new Error(`${flag} must be "flat" or "package" (got "${v}")`);
49+
}
50+
return v;
51+
}
52+
3953
function parseDocsArgs(argv: string[], cwd: string): DocsFlags {
4054
let metadata: string | undefined;
4155
let out: string | undefined;
4256
let templates: string | undefined;
57+
let layout: DocsLayout | undefined;
4358
for (let i = 0; i < argv.length; i++) {
4459
const a = argv[i]!;
4560
if (a === "--out" || a === "-o") {
@@ -48,6 +63,10 @@ function parseDocsArgs(argv: string[], cwd: string): DocsFlags {
4863
out = v;
4964
} else if (a.startsWith("--out=")) {
5065
out = a.slice("--out=".length);
66+
} else if (a === "--layout") {
67+
layout = parseLayout(argv[++i], a);
68+
} else if (a.startsWith("--layout=")) {
69+
layout = parseLayout(a.slice("--layout=".length), "--layout");
5170
} else if (a === "--templates") {
5271
const v = argv[++i];
5372
if (v === undefined) throw new Error(`${a} requires a directory argument`);
@@ -68,6 +87,8 @@ function parseDocsArgs(argv: string[], cwd: string): DocsFlags {
6887
metadata: metadata ?? cwd,
6988
// Default out dir, resolved against the metadata root below.
7089
out: out ?? "./docs",
90+
// Default flat preserves today's single-package output (+ existing goldens).
91+
layout: layout ?? "flat",
7192
...(templates !== undefined ? { templates } : {}),
7293
};
7394
}
@@ -148,7 +169,13 @@ export async function docsCommand(args: string[], cwd: string): Promise<number>
148169
entities: root.objects(),
149170
loadedRoot: root,
150171
matches: () => true,
151-
config: { outDir, extStyle: "none", dbImport: "", dialect: "sqlite" } as never,
172+
config: {
173+
outDir,
174+
extStyle: "none",
175+
dbImport: "",
176+
dialect: "sqlite",
177+
outputLayout: flags.layout,
178+
} as never,
152179
renderContext,
153180
projectRoot,
154181
warn: (msg) => log.warn(`docs: ${msg}`),
@@ -159,6 +186,13 @@ export async function docsCommand(args: string[], cwd: string): Promise<number>
159186
files = await docsFile().generate(ctx);
160187
} catch (err) {
161188
const msg = (err as Error).message;
189+
// Duplicate output path (silent-overwrite backstop): the generator already
190+
// names both colliding FQNs + the path and starts with "docs:". Surface it
191+
// verbatim as a clean non-zero exit (no double prefix, no stack trace).
192+
if (msg.startsWith("docs: duplicate output path")) {
193+
log.error(msg);
194+
return 1;
195+
}
162196
// The framework templates resolve from disk; inside the compiled `meta`
163197
// binary they live on a virtual fs the provider cannot read. Surface that
164198
// as an actionable message rather than a cryptic render failure.

server/typescript/packages/cli/test/docs-command.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,47 @@ async function customTypeProject(): Promise<string> {
113113
return root;
114114
}
115115

116+
// TWO-package metadata with a SHORT-NAME collision across packages: an
117+
// `object.value acme::sales::Order` and a `template.output acme::comms::Order`.
118+
// In flat layout both want `Order.md` (collision → hard error); in package
119+
// layout they fold under distinct subdirs.
120+
const COLLIDING_META = {
121+
"metadata.root": {
122+
children: [
123+
{
124+
"object.value": {
125+
name: "Order",
126+
package: "acme::sales",
127+
children: [{ "field.string": { name: "sku" } }],
128+
},
129+
},
130+
{
131+
"template.output": {
132+
name: "Order",
133+
package: "acme::comms",
134+
"@kind": "document",
135+
"@payloadRef": "Order",
136+
"@textRef": "comms/order",
137+
"@format": "html",
138+
},
139+
},
140+
],
141+
},
142+
};
143+
144+
/** Project root holding the cross-package short-name collision metadata. */
145+
async function collidingProject(): Promise<string> {
146+
const root = await mkdtemp(join(tmpdir(), "meta-docs-collide-"));
147+
dirs.push(root);
148+
await mkdir(join(root, "metaobjects"), { recursive: true });
149+
await writeFile(
150+
join(root, "metaobjects", "meta.json"),
151+
JSON.stringify(COLLIDING_META),
152+
"utf8",
153+
);
154+
return root;
155+
}
156+
116157
afterAll(async () => {
117158
for (const d of dirs) await rm(d, { recursive: true, force: true });
118159
});
@@ -253,4 +294,54 @@ describe("meta docs — standalone neutral metadata docs", () => {
253294
const help = logged.join("\n");
254295
expect(help).toContain("docs");
255296
});
297+
298+
test("default (flat) on a cross-package short-name collision → non-zero, no overwrite", async () => {
299+
const root = await collidingProject();
300+
const out = join(root, "out-collide-flat");
301+
302+
const errLogged: string[] = [];
303+
const origErr = console.error;
304+
console.error = (...args: unknown[]) => errLogged.push(args.map(String).join(" "));
305+
let code: number;
306+
try {
307+
code = await docsCommand([root, "--out", out], root);
308+
} finally {
309+
console.error = origErr;
310+
}
311+
312+
expect(code).not.toBe(0); // hard error, not a silent overwrite
313+
const stderr = errLogged.join("\n");
314+
expect(stderr).toContain("duplicate output path");
315+
expect(stderr).toContain("Order.md");
316+
expect(stderr).toContain("acme::sales::Order");
317+
expect(stderr).toContain("acme::comms::Order");
318+
});
319+
320+
test("--layout package: distinct nested files, exit 0", async () => {
321+
const root = await collidingProject();
322+
const out = join(root, "out-collide-pkg");
323+
324+
const code = await docsCommand([root, "--out", out, "--layout", "package"], root);
325+
expect(code).toBe(0);
326+
327+
// Both pages survive under package-folded subdirs.
328+
expect(existsSync(join(out, "acme", "sales", "Order.md"))).toBe(true);
329+
expect(existsSync(join(out, "acme", "comms", "Order.md"))).toBe(true);
330+
});
331+
332+
test("--layout package preserves single-package output as nested (no flat regression)", async () => {
333+
// The default-flat single-package fixture still emits flat <name>.md.
334+
const root = await project();
335+
const out = join(root, "out-flat-default");
336+
expect(await docsCommand([root, "--out", out], root)).toBe(0);
337+
expect(existsSync(join(out, "Welcome.md"))).toBe(true);
338+
expect(existsSync(join(out, "WelcomePage.md"))).toBe(true);
339+
});
340+
341+
test("--layout rejects an invalid value", async () => {
342+
const root = await project();
343+
const out = join(root, "out-badlayout");
344+
const code = await docsCommand([root, "--out", out, "--layout", "nested"], root);
345+
expect(code).toBe(2); // arg parse error
346+
});
256347
});
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// docs-paths.ts — the SINGLE source of truth for where a docs page is written
2+
// AND how one page links to another. The file location and every inbound link
3+
// href are derived from the SAME functions here, so they can never diverge.
4+
//
5+
// Why this matters: pages are placed by short name. In "flat" layout two nodes
6+
// that share a short name across different packages (e.g. `acme::sales::Order`
7+
// and `acme::billing::Order`) would both want `Order.md` and one would silently
8+
// overwrite the other. `assertNoDuplicateDocPaths()` is the hard backstop that
9+
// turns that data-loss into a clear error; "package" layout folds pages under
10+
// package-path subdirs (`acme/sales/Order.md`) so multi-package models work.
11+
12+
import { PACKAGE_SEPARATOR } from "@metaobjectsdev/metadata";
13+
import { relative as posixRelative } from "node:path/posix";
14+
import { packageToPath, type OutputLayout } from "./import-path.js";
15+
16+
/** The minimal shape needed to place a docs page / compute a link to it: a
17+
* short name and its EFFECTIVE package. Build one from a metadata node via
18+
* `docPageNode()`. */
19+
export interface DocPageNode {
20+
readonly name: string;
21+
readonly package?: string | undefined;
22+
}
23+
24+
/** A metadata node enough to derive page placement. `resolutionKey()` carries
25+
* the EFFECTIVE package (own package OR the file-default captured at parse
26+
* time) folded as `<pkg>::<name>` — `.package` alone is often undefined for
27+
* objects (FR5d keeps object fqn() bare), so we read placement off the
28+
* resolution key instead. */
29+
interface PlaceableNode {
30+
readonly name: string;
31+
resolutionKey(): string;
32+
}
33+
34+
/** Effective package of a placeable node: the prefix of `resolutionKey()`
35+
* before the trailing `::<name>`, or undefined when the node is package-less. */
36+
export function effectivePackage(node: PlaceableNode): string | undefined {
37+
const key = node.resolutionKey();
38+
const suffix = `${PACKAGE_SEPARATOR}${node.name}`;
39+
if (key === node.name) return undefined;
40+
if (key.endsWith(suffix)) {
41+
const pkg = key.slice(0, key.length - suffix.length);
42+
return pkg === "" ? undefined : pkg;
43+
}
44+
return undefined;
45+
}
46+
47+
/** Build a placement node ({name, effective package}) from a metadata node. The
48+
* single bridge from a loaded node to the path/href helpers — so file location
49+
* and link href derive from the SAME effective package. */
50+
export function docPageNode(node: PlaceableNode): DocPageNode {
51+
return { name: node.name, package: effectivePackage(node) };
52+
}
53+
54+
/** Output path (relative to the docs out dir) for a node's `.md` page.
55+
* Flat → `<name>.md` (today's value, byte-identical). Package → folded under
56+
* the package path (`acme/sales/Order.md`); a package-less node stays at root. */
57+
export function docPageOutputPath(layout: OutputLayout, node: DocPageNode): string {
58+
const filename = `${node.name}.md`;
59+
if (layout === "flat") return filename;
60+
const dir = packageToPath(node.package);
61+
return dir === "" ? filename : `${dir}/${filename}`;
62+
}
63+
64+
/** Relative href FROM `fromNode`'s page TO `toNode`'s page. Derived from the
65+
* same `docPageOutputPath()` placement, so a link always points at the file's
66+
* real location in BOTH layouts. Flat → `./<to>.md`; package → a correct
67+
* relative path (e.g. `../comms/OrderEmail.md`). */
68+
export function docPageHref(
69+
layout: OutputLayout,
70+
fromNode: DocPageNode,
71+
toNode: DocPageNode,
72+
): string {
73+
const toPath = docPageOutputPath(layout, toNode);
74+
if (layout === "flat") return `./${toPath}`;
75+
const fromPath = docPageOutputPath(layout, fromNode);
76+
// Relative path from the FROM page's directory to the TO page.
77+
const fromDir = fromPath.includes("/") ? fromPath.slice(0, fromPath.lastIndexOf("/")) : "";
78+
let rel = posixRelative(fromDir, toPath);
79+
if (!rel.startsWith(".")) rel = `./${rel}`;
80+
return rel;
81+
}
82+
83+
/** A page about to be emitted, paired with the FQN of the node that produced it
84+
* (for a precise collision diagnostic). */
85+
export interface DocPagePlacement {
86+
path: string;
87+
fqn: string;
88+
}
89+
90+
/** Hard backstop against silent overwrite (ALL layouts): if two placements
91+
* resolve to the SAME output path, THROW naming both colliding node FQNs and
92+
* the path. Guarantees a docs run never silently drops a page. */
93+
export function assertNoDuplicateDocPaths(placements: DocPagePlacement[]): void {
94+
const seen = new Map<string, string>();
95+
for (const { path, fqn } of placements) {
96+
const prior = seen.get(path);
97+
if (prior !== undefined) {
98+
throw new Error(
99+
`docs: duplicate output path "${path}" from nodes ${prior} and ${fqn} — ` +
100+
`use package layout (outputLayout: "package" / meta docs --layout package) ` +
101+
`to disambiguate.`,
102+
);
103+
}
104+
seen.set(path, fqn);
105+
}
106+
}

server/typescript/packages/codegen-ts/src/generators/docs-data-builder.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ import {
4141
} from "@metaobjectsdev/metadata";
4242
import type { Dialect } from "../column-mapper.js";
4343
import type { ColumnNamingStrategy } from "../metaobjects-config.js";
44+
import type { OutputLayout } from "../import-path.js";
45+
import { docPageHref, docPageNode } from "../docs-paths.js";
4446
import { enumValues } from "../enum-meta.js";
4547
import { hasWritableRdbSource } from "../source-detect.js";
4648
import { GENERATED_HEADER } from "../constants.js";
@@ -57,6 +59,8 @@ export interface BuildDocDataOpts {
5759
dialect: Dialect;
5860
columnNamingStrategy?: ColumnNamingStrategy;
5961
loadedRoot: MetaRoot;
62+
/** Page-placement layout. Defaults to "flat" (back-compat: same-dir links). */
63+
layout?: OutputLayout;
6064
}
6165

6266
function isFieldRequired(field: MetaField): boolean {
@@ -288,6 +292,7 @@ export function buildEntityDocData(
288292
opts: BuildDocDataOpts,
289293
): EntityDocData {
290294
const root = opts.loadedRoot;
295+
const layout = opts.layout ?? "flat";
291296
const primary = entity.primaryIdentity();
292297
const pkFields = primary?.fields ?? [];
293298
const pkFieldNames = new Set<string>(pkFields);
@@ -362,11 +367,13 @@ export function buildEntityDocData(
362367
const ref = child.ownAttr(TEMPLATE_ATTR_PAYLOAD_REF);
363368
if (typeof ref !== "string") continue;
364369
if (stripPackage(ref) !== entity.name) continue;
365-
// Link to the template's own doc page. Task 3 emits the template page as
366-
// `<TemplateName>.md` using the RAW node name (same convention as entity
367-
// pages), so the href MUST use the raw name to resolve.
370+
// Link to the template's own doc page. The href is derived from the SAME
371+
// page-placement function used to write the template page, so it resolves
372+
// in BOTH layouts (flat → `./<Tmpl>.md`; package → a correct relative path
373+
// like `../comms/OrderEmail.md`).
374+
const href = docPageHref(layout, docPageNode(entity), docPageNode(child));
368375
usedByMatches.push({
369-
bullet: `[\`template.${child.subType} ${child.name}\`](./${child.name}.md) — uses \`${entity.name}\` as \`@payloadRef\``,
376+
bullet: `[\`template.${child.subType} ${child.name}\`](${href}) — uses \`${entity.name}\` as \`@payloadRef\``,
370377
});
371378
}
372379
const usedBy = usedByMatches.length > 0 ? usedByMatches : undefined;

server/typescript/packages/codegen-ts/src/generators/docs-file.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,12 @@ import type { MetaObject } from "@metaobjectsdev/metadata";
2222
import { TYPE_TEMPLATE, TEMPLATE_SUBTYPE_OUTPUT } from "@metaobjectsdev/metadata";
2323
import { render } from "@metaobjectsdev/render";
2424
import type { Generator, GeneratorFactory, EmittedFile } from "../generator.js";
25-
import { entityOutputPath } from "../import-path.js";
25+
import {
26+
docPageOutputPath,
27+
docPageNode,
28+
assertNoDuplicateDocPaths,
29+
type DocPagePlacement,
30+
} from "../docs-paths.js";
2631
import { projectProvider } from "../render-engine/framework-provider.js";
2732
import { buildEntityDocData } from "./docs-data-builder.js";
2833
import { buildTemplateDocData } from "./template-doc-builder.js";
@@ -47,13 +52,18 @@ export const docsFile = function docsFile(opts?: DocsFileOpts): Generator {
4752
const rc = ctx.renderContext;
4853
const provider = projectProvider(ctx.projectRoot ?? process.cwd());
4954
const layout = ctx.config.outputLayout ?? "flat";
55+
// Track every (path, fqn) so we can hard-error on a collision (defense
56+
// against silent doc-page overwrite) AFTER all pages are placed.
57+
const placements: DocPagePlacement[] = [];
5058
const files: EmittedFile[] = ctx.loadedRoot
5159
.objects()
5260
.filter(ctx.matches)
5361
.map((entity: MetaObject) => {
54-
const path = entityOutputPath(layout, entity.package, `${entity.name}.md`);
62+
const path = docPageOutputPath(layout, docPageNode(entity));
63+
placements.push({ path, fqn: entity.resolutionKey() });
5564
const payload = buildEntityDocData(entity, {
5665
dialect: rc.dialect,
66+
layout,
5767
...(rc.columnNamingStrategy !== undefined && {
5868
columnNamingStrategy: rc.columnNamingStrategy,
5969
}),
@@ -82,8 +92,9 @@ export const docsFile = function docsFile(opts?: DocsFileOpts): Generator {
8292
// (`<name>.md`), agreeing with the entity Used-by back-link target.
8393
for (const child of ctx.loadedRoot.ownChildren()) {
8494
if (child.type !== TYPE_TEMPLATE || child.subType !== TEMPLATE_SUBTYPE_OUTPUT) continue;
85-
const path = entityOutputPath(layout, child.package, `${child.name}.md`);
86-
const payload = buildTemplateDocData(child);
95+
const path = docPageOutputPath(layout, docPageNode(child));
96+
placements.push({ path, fqn: child.resolutionKey() });
97+
const payload = buildTemplateDocData(child, { layout, loadedRoot: ctx.loadedRoot });
8798
let content: string;
8899
try {
89100
content = render({
@@ -102,6 +113,10 @@ export const docsFile = function docsFile(opts?: DocsFileOpts): Generator {
102113
files.push({ path, content });
103114
}
104115

116+
// Hard backstop against silent overwrite (ALL layouts): two nodes that
117+
// resolve to the same output path → throw naming both FQNs + the path.
118+
assertNoDuplicateDocPaths(placements);
119+
105120
return files;
106121
},
107122
};

0 commit comments

Comments
 (0)