diff --git a/packages/core/src/loop/boringstack/generate.ts b/packages/core/src/loop/boringstack/generate.ts index 0fa8582a..5f20e8ad 100644 --- a/packages/core/src/loop/boringstack/generate.ts +++ b/packages/core/src/loop/boringstack/generate.ts @@ -31,14 +31,20 @@ export async function generateResource( // IDEMPOTENT on retry: `new:resource` refuses to overwrite an existing resource // dir, so on a second attempt (the model is fixing gate failures) we must NOT // regenerate — that would crash AND clobber the model's in-progress fixes. Only - // scaffold + wire when the resource doesn't exist yet; always re-run the - // downstream sync (format + db:push) so a schema/format tweak is reflected. + // SCAFFOLD when the resource doesn't exist yet. if (!existsSync(`${apiCwd}/src/api/${camel}`)) { await execOrThrow(exec, ["bun", "run", "new:resource", "--", name], apiCwd); - - await wireResource(cwd, name); } + // …but WIRE on EVERY attempt — this is the API analog of generateFeature's + // unconditional `wireUiFeature`. Mounting the route (routes map + app `.group` + + // swagger) was previously gated on the scaffold branch, so once the dir existed — + // a pre-existing dir, or a near-green rollback that reverted the mount — the route + // was NEVER mounted again: knip `unused file` + an unreachable route → park + // (observed live, build13). wireResource is idempotent (each wire* checks it isn't + // already present), so re-running it every attempt is safe and self-heals a lost mount. + await wireResource(cwd, name); + // Format with BoringStack's OWN pinned prettier (its `format` script), NEVER // `bunx prettier` — bunx pulls the latest prettier, which formats differently // (e.g. union types) than the pinned version the gate checks against, so the diff --git a/packages/core/src/loop/boringstack/wire-resource.ts b/packages/core/src/loop/boringstack/wire-resource.ts index 350aec95..2d593f6e 100644 --- a/packages/core/src/loop/boringstack/wire-resource.ts +++ b/packages/core/src/loop/boringstack/wire-resource.ts @@ -29,36 +29,65 @@ export function wireRoutesFile(src: string, name: string): string { const importLine = `import ${camel}Routes from "../../api/${camel}/${camel}.routes";\n`; const objectEntry = ` ${camel}: ${camel}Routes,\n`; - const lastImportIndex = src.lastIndexOf("import "); - - if (lastImportIndex === -1) { - throw new Error("No import statement found"); + // wireResource runs on EVERY attempt (like the UI wirer), so this must be idempotent. + // Guard the import and the map entry INDEPENDENTLY so every partial state self-heals to + // the complete pair without duplicating: both present → no-op; neither → both added; + // import-only (entry reverted) → add entry; entry-only (import reverted) → add import. + const hasImport = src.includes(importLine.trimEnd()); + const hasEntry = src.includes(`${camel}: ${camel}Routes`); + + if (hasImport && hasEntry) { + return src; } - const importEndIndex = src.indexOf("\n", lastImportIndex); + let out = src; + + if (!hasImport) { + const lastImportIndex = out.lastIndexOf("import "); + + if (lastImportIndex === -1) { + throw new Error("No import statement found"); + } + + const importEndIndex = out.indexOf("\n", lastImportIndex); + + if (importEndIndex === -1) { + throw new Error("No newline after import found"); + } - if (importEndIndex === -1) { - throw new Error("No newline after import found"); + out = + out.slice(0, importEndIndex + 1) + + importLine + + out.slice(importEndIndex + 1); } - const afterImports = - src.slice(0, importEndIndex + 1) + - importLine + - src.slice(importEndIndex + 1); + if (!hasEntry) { + out = insertBeforeLast(out, "};", objectEntry); + } - return insertBeforeLast(afterImports, "};", objectEntry); + return out; } export function wireAppFile(src: string, name: string): string { const camel = toCamelCase(name); const groupCall = `.group("/api/v1/${camel}", (group) => group.use(routes.${camel}))\n `; + // Idempotent (retry-safe): the sub-router is already mounted at this prefix. + if (src.includes(`.group("/api/v1/${camel}",`)) { + return src; + } + return insertBeforeLast(src, ");", groupCall); } export function wireSwaggerFile(src: string, name: string): string { const tagEntry = `{ name: "${name}", description: "${name} resource" },\n `; + // Idempotent (retry-safe): the swagger tag is already registered. + if (src.includes(`{ name: "${name}",`)) { + return src; + } + return insertBeforeLast(src, "],", tagEntry); } diff --git a/packages/core/tests/boringstack-generate.test.ts b/packages/core/tests/boringstack-generate.test.ts index fafba554..9d1e8be3 100644 --- a/packages/core/tests/boringstack-generate.test.ts +++ b/packages/core/tests/boringstack-generate.test.ts @@ -99,11 +99,13 @@ describe("generateResource", () => { }); describe("generateResource idempotency (retry-safe)", () => { - test("skips new:resource + wiring when the resource already exists, still re-syncs", async () => { + test("skips new:resource on an existing dir, but STILL wires the route + re-syncs (build13 remount fix)", async () => { const tmpDir = await createTestEnv(); try { - // Simulate a prior attempt: the resource dir already exists on disk. + // Simulate a prior attempt: the resource dir already exists on disk, but the route + // is NOT mounted (a rollback reverted the mount, or the dir pre-existed). Before the + // fix, wiring was gated on the scaffold branch and skipped here → knip unused + park. await mkdir(join(tmpDir, "apps/api/src/api/project"), { recursive: true, }); @@ -115,13 +117,43 @@ describe("generateResource idempotency (retry-safe)", () => { // new:resource would CRASH on an existing dir — it must be skipped on retry… expect(joined.some((c) => c.includes("new:resource"))).toBe(false); - // …but the downstream sync still runs so a fix is reflected. + // …but the route IS now mounted (wireResource runs every attempt, self-healing a + // lost/absent mount) — the exact build13 park fix. + const routesSrc = await Bun.file( + join(tmpDir, "apps/api/src/config/routes/routes.ts") + ).text(); + + expect(routesSrc).toContain("project: projectRoutes,"); + // …and the downstream sync still runs so a fix is reflected. expect(joined.some((c) => c.includes("run format"))).toBe(true); expect(joined.some((c) => c.includes("db:push"))).toBe(true); } finally { await rm(tmpDir, { recursive: true, force: true }); } }); + + test("re-running generateResource does NOT double-mount (idempotent wire on every attempt)", async () => { + const tmpDir = await createTestEnv(); + + try { + await mkdir(join(tmpDir, "apps/api/src/api/project"), { + recursive: true, + }); + const { exec } = recorder(); + + await generateResource(tmpDir, "Project", exec); + await generateResource(tmpDir, "Project", exec); + + const routesSrc = await Bun.file( + join(tmpDir, "apps/api/src/config/routes/routes.ts") + ).text(); + const mounts = routesSrc.split("project: projectRoutes,").length - 1; + + expect(mounts).toBe(1); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } + }); }); describe("generateFeature", () => { diff --git a/packages/core/tests/boringstack-wire-resource.test.ts b/packages/core/tests/boringstack-wire-resource.test.ts index f8833f3a..ca3a2f51 100644 --- a/packages/core/tests/boringstack-wire-resource.test.ts +++ b/packages/core/tests/boringstack-wire-resource.test.ts @@ -129,6 +129,36 @@ describe("wireRoutesFile", () => { expect(out).toContain("invoice: invoiceRoutes,"); expect(out).toContain("health: healthRoutes,"); }); + + test("is idempotent — re-wiring an already-mounted route is a no-op (build13 remount safety)", () => { + const src = `import healthRoutes from "../../api/health/health.routes";\n\nexport const routes = {\n health: healthRoutes,\n};\n`; + const once = wireRoutesFile(src, "Invoice"); + + // wireResource now runs on EVERY attempt, so a second wire must NOT double-insert. + expect(wireRoutesFile(once, "Invoice")).toBe(once); + }); + + test("self-heals a PARTIAL state: entry present but import reverted → adds the import (no dup entry)", () => { + // import line removed, map entry left behind — the guard must restore the import only. + const src = `import healthRoutes from "../../api/health/health.routes";\n\nexport const routes = {\n health: healthRoutes,\n invoice: invoiceRoutes,\n};\n`; + const out = wireRoutesFile(src, "Invoice"); + + expect(out).toContain( + 'import invoiceRoutes from "../../api/invoice/invoice.routes";' + ); + expect(out.split("invoice: invoiceRoutes,").length - 1).toBe(1); // entry not duplicated + }); + + test("self-heals a PARTIAL state: import present but entry reverted → adds the entry (no dup import)", () => { + const src = `import healthRoutes from "../../api/health/health.routes";\nimport invoiceRoutes from "../../api/invoice/invoice.routes";\n\nexport const routes = {\n health: healthRoutes,\n};\n`; + const out = wireRoutesFile(src, "Invoice"); + + expect(out).toContain("invoice: invoiceRoutes,"); + expect( + out.split('import invoiceRoutes from "../../api/invoice/invoice.routes";') + .length - 1 + ).toBe(1); // import not duplicated + }); }); describe("wireAppFile", () => { @@ -142,6 +172,13 @@ describe("wireAppFile", () => { ); expect(out).toContain(".use(routes.health)"); }); + + test("is idempotent — the group mount is inserted at most once", () => { + const src = ` return (\n app\n .use(routes.health)\n );\n`; + const once = wireAppFile(src, "Invoice"); + + expect(wireAppFile(once, "Invoice")).toBe(once); + }); }); describe("wireSwaggerFile", () => { @@ -155,6 +192,13 @@ describe("wireSwaggerFile", () => { ); expect(out).toContain('{ name: "Health", description: "probes" }'); }); + + test("is idempotent — the swagger tag is added at most once", () => { + const src = ` tags: [\n { name: "Health", description: "probes" },\n ],\n`; + const once = wireSwaggerFile(src, "Invoice"); + + expect(wireSwaggerFile(once, "Invoice")).toBe(once); + }); }); describe("wireTestHelperFile", () => {