From 4c60e77cda2b73688c8b1af99a86bffd4087a417 Mon Sep 17 00:00:00 2001 From: Marve10s Date: Fri, 10 Jul 2026 12:59:05 +0300 Subject: [PATCH 1/2] feat(cli): structured merge for package.json/.env.example in bfs update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the deferred follow-up in scaffold-upgrade's isStructuredMergeFile: instead of always routing package.json and .env.example to manual review, `bfs update` now folds template-side changes into the user's file, reusing stack-update's mergePackageJson / mergeEnvExample (now exported). - bts.lock.json gains optional `baselines`: the pure-template render content of package.json / *.env.example, recorded at create (CLI + MCP), by `update --record-baseline`, and advanced on every reconciling apply (scaffold upgrade + stack update). - Merge semantics match stack-update: template additions/changes to deps, scripts, and env keys are applied when the user (or create-time post-processing) didn't touch the key; user-changed keys always win — if the template also changed one, the whole file becomes a conflict naming the blocked keys. Manifests without baselines fall back to manual review. - Lockfiles and .env (secrets) stay conservative: manual review, never patched. - New "merged" plan category is actionable, reported by `update`, and written on --apply with hashes + baselines refreshed. - fix(template-generator): later processCatalogs passes now resolve existing "catalog:" references when counting duplicates, so graph-mode db-package deps get catalogued like solo mode; previously create/update renders disagreed and a fresh project reported spurious package.json changes. --- apps/cli/src/commands/update.ts | 48 +++-- apps/cli/src/helpers/core/create-project.ts | 13 +- apps/cli/src/helpers/core/scaffold-upgrade.ts | 181 +++++++++++++++-- apps/cli/src/helpers/core/stack-update.ts | 13 +- apps/cli/src/mcp.ts | 8 +- apps/cli/src/utils/scaffold-manifest.ts | 63 +++++- apps/cli/test/scaffold-upgrade.test.ts | 190 +++++++++++++++++- .../src/post-process/catalogs.ts | 39 +++- 8 files changed, 496 insertions(+), 59 deletions(-) diff --git a/apps/cli/src/commands/update.ts b/apps/cli/src/commands/update.ts index aae472c3a..97fa336bc 100644 --- a/apps/cli/src/commands/update.ts +++ b/apps/cli/src/commands/update.ts @@ -5,12 +5,12 @@ import pc from "picocolors"; import { applyScaffoldUpgrade, planScaffoldUpgrade, + recordUpgradeBaseline, type UpgradePlan, } from "../helpers/core/scaffold-upgrade"; import { readBtsConfig } from "../utils/bts-config"; import { handleError } from "../utils/errors"; import { renderTitle } from "../utils/render-title"; -import { recordScaffoldManifest } from "../utils/scaffold-manifest"; export type UpdateCommandInput = { projectDir?: string; @@ -51,6 +51,15 @@ function reportManual(entries: UpgradePlan["manual"]): void { } } +function reportMerged(plan: UpgradePlan): void { + const merged = plan.files.filter((file) => file.category === "merged"); + if (merged.length === 0) return; + log.message(`Structured merges (template changes folded into your file) (${merged.length}):`); + for (const entry of merged) { + log.message(pc.dim(` ± ${entry.path}${entry.reason ? ` — ${entry.reason}` : ""}`)); + } +} + function reportRemoved(plan: UpgradePlan): void { const removed = plan.files.filter((file) => file.category === "removed"); if (removed.length === 0) return; @@ -74,6 +83,7 @@ function renderPlan(plan: UpgradePlan): void { log.message(""); reportGroup("Template drift (safe to patch)", "~", plan.drift); + reportMerged(plan); reportGroup("New files from templates", "+", plan.newFiles); reportGroup("Locally edited (kept as-is)", "*", plan.userEdited); reportGroup("Conflicts (template + local both changed)", "!", plan.conflicts); @@ -83,8 +93,9 @@ function renderPlan(plan: UpgradePlan): void { log.message(""); log.message( pc.dim( - `${plan.unchanged.length} up to date · ${plan.drift.length} drift · ${plan.newFiles.length} new · ` + - `${plan.userEdited.length} local · ${plan.conflicts.length} conflict · ${plan.manual.length} manual`, + `${plan.unchanged.length} up to date · ${plan.drift.length} drift · ${plan.merged.length} merge · ` + + `${plan.newFiles.length} new · ${plan.userEdited.length} local · ${plan.conflicts.length} conflict · ` + + `${plan.manual.length} manual`, ), ); } @@ -97,6 +108,7 @@ function toJsonPlan(plan: UpgradePlan) { summary: { unchanged: plan.unchanged.length, drift: plan.drift.length, + merged: plan.merged.length, newFiles: plan.newFiles.length, userEdited: plan.userEdited.length, conflicts: plan.conflicts.length, @@ -104,10 +116,13 @@ function toJsonPlan(plan: UpgradePlan) { removed: plan.removed.length, }, drift: plan.drift, + merged: plan.files + .filter((file) => file.category === "merged") + .map(({ path: filePath, reason }) => ({ path: filePath, reason })), newFiles: plan.newFiles, userEdited: plan.userEdited, conflicts: plan.conflicts, - manual: plan.manual, + manual: plan.manual.map(({ path: filePath, reason }) => ({ path: filePath, reason })), removed: plan.removed, actionable: plan.actionable, }; @@ -162,7 +177,7 @@ export async function updateCommand(input: UpdateCommandInput): Promise { } if (recordBaseline) { - const manifest = await recordScaffoldManifest(projectDir); + const manifest = await recordUpgradeBaseline(projectDir); if (json) { console.log( JSON.stringify( @@ -196,7 +211,7 @@ export async function updateCommand(input: UpdateCommandInput): Promise { } let plan: UpgradePlan; - let applied: { patched: string[]; added: string[] } | undefined; + let applied: { patched: string[]; added: string[]; merged: string[] } | undefined; if (apply) { const result = await applyScaffoldUpgrade(projectDir); if (!result.success) return failUpdate(projectDir, result.error, json); @@ -238,16 +253,16 @@ export async function updateCommand(input: UpdateCommandInput): Promise { log.message(""); if (applied) { - const total = applied.patched.length + applied.added.length; + const total = applied.patched.length + applied.added.length + applied.merged.length; if (total === 0) { log.success(pc.green("Already up to date. No template-drift patches to apply.")); } else { log.success( pc.green( - `Applied ${formatCount(applied.patched.length, "patch")} and added ${formatCount( - applied.added.length, - "file", - )}.`, + `Applied ${formatCount(applied.patched.length, "patch")}, ${formatCount( + applied.merged.length, + "structured merge", + )}, and added ${formatCount(applied.added.length, "file")}.`, ), ); } @@ -255,7 +270,7 @@ export async function updateCommand(input: UpdateCommandInput): Promise { if (leftover > 0) { log.warn( pc.yellow( - `${formatCount(leftover, "file")} still need manual review (conflicts + post-processed files).`, + `${formatCount(leftover, "file")} still need manual review (conflicts + lockfiles/manual files).`, ), ); } @@ -268,10 +283,11 @@ export async function updateCommand(input: UpdateCommandInput): Promise { } else { log.info( pc.cyan( - `Run \`bfs update --apply\` to patch ${formatCount( - plan.drift.length, - "drift file", - )} and add ${formatCount(plan.newFiles.length, "new file")}.`, + `Run \`bfs update --apply\` to patch ${formatCount(plan.drift.length, "drift file")}, ` + + `apply ${formatCount(plan.merged.length, "structured merge")}, and add ${formatCount( + plan.newFiles.length, + "new file", + )}.`, ), ); } diff --git a/apps/cli/src/helpers/core/create-project.ts b/apps/cli/src/helpers/core/create-project.ts index 8faf89f52..26448d0b3 100644 --- a/apps/cli/src/helpers/core/create-project.ts +++ b/apps/cli/src/helpers/core/create-project.ts @@ -11,7 +11,7 @@ import { isSilent } from "../../utils/context"; import { applyDependencyVersionChannel } from "../../utils/dependency-version-channel"; import { CLIError } from "../../utils/errors"; import { formatProject } from "../../utils/file-formatter"; -import { recordScaffoldManifest } from "../../utils/scaffold-manifest"; +import { collectStructuredBaselines, recordScaffoldManifest } from "../../utils/scaffold-manifest"; import { setupAddons } from "../addons/addons-setup"; import { setupDatabase } from "../core/db-setup"; import { initializeGit } from "./git"; @@ -83,9 +83,14 @@ export async function createProject(options: ProjectConfig, cliInput: CreateProj // Record the scaffold baseline (bts.lock.json) from the final formatted, // pre-install bytes so `bfs update` can later tell template drift apart from - // user edits. Best-effort: recordScaffoldManifest never throws, so a failure - // here disables update auto-patching without breaking scaffolding. - await recordScaffoldManifest(projectDir); + // user edits. The pure-render content of package.json / *.env.example (before + // the post-processing above mutated them on disk) is kept alongside the + // hashes so `bfs update` can structurally merge future template changes. + // Best-effort: recordScaffoldManifest never throws, so a failure here + // disables update auto-patching without breaking scaffolding. + await recordScaffoldManifest(projectDir, { + baselines: collectStructuredBaselines(result.tree), + }); if (!isSilent()) log.success("Project template successfully scaffolded!"); diff --git a/apps/cli/src/helpers/core/scaffold-upgrade.ts b/apps/cli/src/helpers/core/scaffold-upgrade.ts index 28b30fd79..06cefad53 100644 --- a/apps/cli/src/helpers/core/scaffold-upgrade.ts +++ b/apps/cli/src/helpers/core/scaffold-upgrade.ts @@ -7,8 +7,11 @@ import path from "node:path"; import { readBtsConfig } from "../../utils/bts-config"; import { + collectStructuredBaselines, hashContent, + isStructuredBaselinePath, readScaffoldManifest, + recordScaffoldManifest, type ScaffoldManifest, writeScaffoldManifest, } from "../../utils/scaffold-manifest"; @@ -16,25 +19,22 @@ import { configFromBtsConfig, formatGeneratedTree, generateTree, + mergeEnvExample, + mergePackageJson, treeToFileMap, } from "./stack-update"; const BINARY_FILE_MARKER = "[Binary file]"; /** - * Files whose on-disk bytes are mutated by create-time post-processing - * (package-manager version, dependency version channel, db-setup, addons) or by - * dependency install, so their scaffold baseline is not a pure-template render. - * Never auto-patched — always routed to manual review. A structured merge - * (reusing stack-update's mergePackageJson / mergeEnvExample) is a deferred - * follow-up; the MVP is conservative to avoid clobbering post-processed deps. + * Files that are never auto-patched: lockfiles are install artifacts and `.env` + * holds user secrets — both always go to manual review. package.json and + * *.env.example (see isStructuredBaselinePath) get a structured merge instead. */ -function isStructuredMergeFile(relPath: string): boolean { +function isConservativeFile(relPath: string): boolean { const name = path.basename(relPath); return ( - name === "package.json" || name === ".env" || - name.endsWith(".env.example") || name === "bun.lock" || name === "bun.lockb" || name === "package-lock.json" || @@ -43,6 +43,17 @@ function isStructuredMergeFile(relPath: string): boolean { ); } +/** + * Files whose on-disk bytes are mutated by create-time post-processing + * (package-manager version, dependency version channel, db-setup, addons) or by + * dependency install, so their scaffold baseline is not a pure-template render. + * They never take the plain hash-comparison path: they are either merged + * structurally or routed to manual review. + */ +function isStructuredMergeFile(relPath: string): boolean { + return isConservativeFile(relPath) || isStructuredBaselinePath(relPath); +} + /** * Generated docs (README) are re-derived from project mode / stack summary at * render time, so their bytes legitimately differ between the create-time @@ -59,6 +70,7 @@ export type UpgradeCategory = | "user-edited" | "conflict" | "manual" + | "merged" | "new-file" | "removed"; @@ -66,6 +78,8 @@ export type UpgradeFileEntry = { path: string; category: UpgradeCategory; reason?: string; + /** Merge result to write on `--apply` (category "merged" only). */ + mergedContent?: string; }; export type UpgradePlan = { @@ -79,16 +93,17 @@ export type UpgradePlan = { userEdited: string[]; conflicts: string[]; manual: UpgradeFileEntry[]; + merged: string[]; newFiles: string[]; removed: string[]; - /** Files `--apply` would write: drift patches + brand-new template files. */ + /** Files `--apply` would write: drift patches, structured merges, new files. */ actionable: string[]; }; export type UpgradeResult = UpgradePlan | { success: false; projectDir?: string; error: string }; export type UpgradeApplyResult = - | (UpgradePlan & { applied: { patched: string[]; added: string[] } }) + | (UpgradePlan & { applied: { patched: string[]; added: string[]; merged: string[] } }) | { success: false; projectDir?: string; error: string }; async function inferProjectName(projectDir: string): Promise { @@ -160,6 +175,72 @@ async function renderCurrentProject( } } +/** + * Structured 3-way merge for package.json / *.env.example, reusing stack-update's + * merge semantics: template-side changes (proposed vs the recorded render + * baseline) are folded into the user's file; keys the user (or create-time + * post-processing) changed are never overwritten — if the template also changed + * such a key, the whole file becomes a conflict naming the blocked keys. + */ +function classifyStructuredMerge( + filePath: string, + existingContent: string, + proposedContent: string | undefined, + baselineContent: string | undefined, +): UpgradeFileEntry { + if (proposedContent === undefined || proposedContent === BINARY_FILE_MARKER) { + return { path: filePath, category: "manual", reason: "no comparable template render" }; + } + + if (path.basename(filePath) === "package.json") { + if (baselineContent === undefined) { + return { + path: filePath, + category: "manual", + reason: + "no structured-merge baseline recorded — merge dependencies by hand or re-run `update --record-baseline`", + }; + } + const merged = mergePackageJson(existingContent, baselineContent, proposedContent); + if (merged.blockers.length > 0) { + return { + path: filePath, + category: "conflict", + reason: `template and local copy both changed: ${merged.blockers.join(", ")}`, + }; + } + if (merged.content) { + return { + path: filePath, + category: "merged", + reason: merged.summary.join("; "), + mergedContent: merged.content, + }; + } + return { + path: filePath, + category: "user-edited", + reason: "template dependencies/scripts unchanged — local changes kept", + }; + } + + // *.env.example: append template-added keys; existing keys are never touched. + const merged = mergeEnvExample(existingContent, baselineContent, proposedContent); + if (merged.content) { + return { + path: filePath, + category: "merged", + reason: `adds ${merged.keys.join(", ")}`, + mergedContent: merged.content, + }; + } + return { + path: filePath, + category: "user-edited", + reason: "no new template env keys — local changes kept", + }; +} + function summarize( projectDir: string, files: UpgradeFileEntry[], @@ -168,6 +249,7 @@ function summarize( const byCategory = (category: UpgradeCategory) => files.filter((file) => file.category === category).map((file) => file.path); const drift = byCategory("drift"); + const merged = byCategory("merged"); const newFiles = byCategory("new-file"); return { @@ -181,9 +263,10 @@ function summarize( userEdited: byCategory("user-edited"), conflicts: byCategory("conflict"), manual: files.filter((file) => file.category === "manual"), + merged, newFiles, removed: byCategory("removed"), - actionable: [...drift, ...newFiles].sort(), + actionable: [...drift, ...merged, ...newFiles].sort(), }; } @@ -198,7 +281,8 @@ export async function planScaffoldUpgrade(projectDirInput: string): Promise { const plan = await planScaffoldUpgrade(projectDirInput); @@ -322,6 +419,14 @@ export async function applyScaffoldUpgrade(projectDirInput: string): Promise toWrite.has(candidate)); } + const mergedEntries = plan.files.filter( + (file): file is UpgradeFileEntry & { mergedContent: string } => + file.category === "merged" && file.mergedContent !== undefined, + ); + for (const entry of mergedEntries) { + await fs.writeFile(path.join(projectDir, entry.path), entry.mergedContent, "utf-8"); + } + const manifest = await readScaffoldManifest(projectDir); if (manifest) { // Every file that now equals the current render becomes the new baseline; @@ -331,8 +436,44 @@ export async function applyScaffoldUpgrade(projectDirInput: string): Promise entry.path)]; + for (const filePath of reconciled) { + if (!isStructuredBaselinePath(filePath)) continue; + const content = renderFiles.get(filePath)?.content; + if (content !== undefined && content !== BINARY_FILE_MARKER) { + (manifest.baselines ??= {})[filePath] = content; + } + } await writeScaffoldManifest(projectDir, manifest); } - return { ...plan, applied: { patched: [...plan.drift], added: [...plan.newFiles] } }; + return { + ...plan, + applied: { + patched: [...plan.drift], + added: [...plan.newFiles], + merged: mergedEntries.map((entry) => entry.path), + }, + }; +} + +/** + * Record the scaffold baseline for an existing project (`update + * --record-baseline`): disk hashes plus, when the project still renders, the + * structured-merge content baselines for package.json / *.env.example. + */ +export async function recordUpgradeBaseline( + projectDirInput: string, +): Promise { + const projectDir = path.resolve(projectDirInput); + const rendered = await renderCurrentProject(projectDir); + const baselines = "error" in rendered ? undefined : collectStructuredBaselines(rendered.tree); + return recordScaffoldManifest(projectDir, { baselines }); } diff --git a/apps/cli/src/helpers/core/stack-update.ts b/apps/cli/src/helpers/core/stack-update.ts index 717d727a7..4a03e6518 100644 --- a/apps/cli/src/helpers/core/stack-update.ts +++ b/apps/cli/src/helpers/core/stack-update.ts @@ -32,7 +32,10 @@ import { import { validateConfigForProgrammaticUse } from "../../utils/config-validation"; import { formatCode } from "../../utils/file-formatter"; import { getEffectiveStack, getGraphSummary } from "../../utils/graph-summary"; -import { refreshScaffoldManifestFiles } from "../../utils/scaffold-manifest"; +import { + collectStructuredBaselines, + refreshScaffoldManifestFiles, +} from "../../utils/scaffold-manifest"; type JsonObject = Record; @@ -1268,7 +1271,7 @@ function diffJsonSection( return { values, blockers }; } -function mergePackageJson( +export function mergePackageJson( existingContent: string, previousContent: string | undefined, proposedContent: string, @@ -1339,7 +1342,7 @@ function parseEnvKeys(content: string): Set { return keys; } -function mergeEnvExample( +export function mergeEnvExample( existingContent: string, previousContent: string | undefined, proposedContent: string, @@ -1861,9 +1864,13 @@ export async function applyStackUpdate( createdAt: plan.proposedConfig.createdAt, }); + // The whole plan applied cleanly (manual blockers abort above), so every + // structured-merge file is now reconciled with the proposed render — advance + // its `bfs update` baseline alongside the hashes. await refreshScaffoldManifestFiles( plan.projectDir, plan.operations.map((operation) => operation.path), + collectStructuredBaselines(proposedTree), ); await writeMigrationChecklist(plan.projectDir, plan); diff --git a/apps/cli/src/mcp.ts b/apps/cli/src/mcp.ts index 2e674038f..86dc82a5f 100644 --- a/apps/cli/src/mcp.ts +++ b/apps/cli/src/mcp.ts @@ -1915,8 +1915,12 @@ export async function startMcpServer() { addonWarnings = await setupAddons(config); } - const { recordScaffoldManifest } = await import("./utils/scaffold-manifest.js"); - await recordScaffoldManifest(projectDir); + const { collectStructuredBaselines, recordScaffoldManifest } = await import( + "./utils/scaffold-manifest.js" + ); + await recordScaffoldManifest(projectDir, { + baselines: collectStructuredBaselines(result.tree), + }); const ecosystem = (input.ecosystem as string) ?? "typescript"; const installCmd = getInstallCommand( diff --git a/apps/cli/src/utils/scaffold-manifest.ts b/apps/cli/src/utils/scaffold-manifest.ts index 3a6f45f62..f43c6f481 100644 --- a/apps/cli/src/utils/scaffold-manifest.ts +++ b/apps/cli/src/utils/scaffold-manifest.ts @@ -1,3 +1,4 @@ +import type { VirtualFileTree, VirtualNode } from "@better-fullstack/template-generator"; import type { Dirent } from "node:fs"; import fs from "fs-extra"; @@ -47,8 +48,44 @@ export type ScaffoldManifest = { version: string; createdAt: string; hashes: Record; + /** + * Pure-template render content (pre post-processing) of structured-merge + * files — package.json and *.env.example. `bfs update` uses these as the + * "previous" side of a 3-way merge so template-side dependency/script/env-key + * changes can be folded into a post-processed or user-edited file without + * clobbering either. Optional: manifests recorded by older CLIs lack it. + */ + baselines?: Record; }; +/** Files whose render content is stored in the manifest for structured merges. */ +export function isStructuredBaselinePath(relPath: string): boolean { + const name = path.basename(relPath); + return name === "package.json" || name.endsWith(".env.example"); +} + +const BINARY_FILE_MARKER = "[Binary file]"; + +/** Extract structured-merge baseline contents from a generated virtual tree. */ +export function collectStructuredBaselines(tree: VirtualFileTree): Record { + const baselines: Record = {}; + + function walk(nodes: VirtualNode[]) { + for (const node of nodes) { + if (node.type === "file") { + if (isStructuredBaselinePath(node.path) && node.content !== BINARY_FILE_MARKER) { + baselines[node.path] = node.content; + } + } else { + walk(node.children); + } + } + } + + walk(tree.root.children); + return baselines; +} + export function hashContent(content: Buffer | string): string { return createHash("sha256").update(content).digest("hex"); } @@ -99,12 +136,15 @@ export async function writeScaffoldManifest( projectDir: string, manifest: ScaffoldManifest, ): Promise { + const sortEntries = (record: Record) => + Object.fromEntries(Object.entries(record).sort(([a], [b]) => a.localeCompare(b))); const sorted: ScaffoldManifest = { version: manifest.version, createdAt: manifest.createdAt, - hashes: Object.fromEntries( - Object.entries(manifest.hashes).sort(([a], [b]) => a.localeCompare(b)), - ), + hashes: sortEntries(manifest.hashes), + ...(manifest.baselines && Object.keys(manifest.baselines).length > 0 + ? { baselines: sortEntries(manifest.baselines) } + : {}), }; const manifestPath = path.join(projectDir, SCAFFOLD_MANIFEST_FILE); await fs.writeFile(manifestPath, `${JSON.stringify(sorted, null, 2)}\n`, "utf-8"); @@ -119,13 +159,14 @@ export async function writeScaffoldManifest( */ export async function recordScaffoldManifest( projectDir: string, - metadata: { createdAt?: string } = {}, + metadata: { createdAt?: string; baselines?: Record } = {}, ): Promise { try { const manifest: ScaffoldManifest = { version: MANIFEST_VERSION, createdAt: metadata.createdAt ?? new Date().toISOString(), hashes: await computeScaffoldHashes(projectDir), + baselines: metadata.baselines, }; await writeScaffoldManifest(projectDir, manifest); return manifest; @@ -143,16 +184,24 @@ export async function readScaffoldManifest(projectDir: string): Promise render content) advances the structured-merge baselines + * to the render the project was just reconciled against. + */ export async function refreshScaffoldManifestFiles( projectDir: string, relativePaths: Iterable, + baselines?: Record, ): Promise { const manifest = await readScaffoldManifest(projectDir); if (!manifest) return; @@ -167,5 +216,9 @@ export async function refreshScaffoldManifestFiles( ); } + if (baselines && Object.keys(baselines).length > 0) { + manifest.baselines = { ...manifest.baselines, ...baselines }; + } + await writeScaffoldManifest(projectDir, manifest); } diff --git a/apps/cli/test/scaffold-upgrade.test.ts b/apps/cli/test/scaffold-upgrade.test.ts index a748ea949..a41c463d3 100644 --- a/apps/cli/test/scaffold-upgrade.test.ts +++ b/apps/cli/test/scaffold-upgrade.test.ts @@ -10,6 +10,7 @@ import { applyScaffoldUpgrade, planScaffoldUpgrade } from "../src/helpers/core/s import { buildBtsConfigForPersistence, writeBtsConfig } from "../src/utils/bts-config"; import { formatProject } from "../src/utils/file-formatter"; import { + collectStructuredBaselines, hashContent, readScaffoldManifest, recordScaffoldManifest, @@ -79,7 +80,9 @@ async function scaffoldWithBaseline(projectDir: string, config: ProjectConfig): createdAt: persistedConfig.createdAt, }); await formatProject(projectDir); - await recordScaffoldManifest(projectDir); + await recordScaffoldManifest(projectDir, { + baselines: collectStructuredBaselines(result.tree), + }); } function assertSuccess( @@ -259,7 +262,7 @@ describe("scaffold-upgrade engine", () => { expect(await readFile(targetPath, "utf-8")).toBe(`// local change\n`); }); - it("always routes an edited package.json to manual review", async () => { + it("keeps a user-edited package.json as-is when the template side is unchanged", async () => { const dir = await makeTempDir(); await scaffoldWithBaseline(dir, makeConfig(dir)); @@ -270,18 +273,195 @@ describe("scaffold-upgrade engine", () => { const plan = await planScaffoldUpgrade(dir); assertSuccess(plan); - const manualPaths = plan.manual.map((entry) => entry.path); - expect(manualPaths).toContain("package.json"); + expect(plan.userEdited).toContain("package.json"); expect(plan.drift).not.toContain("package.json"); expect(plan.actionable).not.toContain("package.json"); - // Never auto-written. + // Never auto-written: the user's additions win when the template is idle. const applied = await applyScaffoldUpgrade(dir); assertSuccess(applied); expect(applied.applied.patched).not.toContain("package.json"); + expect(applied.applied.merged).not.toContain("package.json"); expect(await readFile(pkgPath, "utf-8")).toContain("left-pad"); }); + it("structurally merges template dependency/script additions into a user-edited package.json", async () => { + const dir = await makeTempDir(); + await scaffoldWithBaseline(dir, makeConfig(dir)); + + const target = "apps/server/package.json"; + const pkgPath = join(dir, target); + const render = JSON.parse(await readFile(pkgPath, "utf-8")); + const [templateDep] = Object.keys(render.dependencies); + const [templateScript, userScript] = Object.keys(render.scripts); + expect(templateDep).toBeDefined(); + expect(userScript).toBeDefined(); + + // Simulate an older template: drop one dependency and one script from both + // the on-disk file and the recorded render baseline, so the current render + // looks like a template that has since added them. + const manifest = await readScaffoldManifest(dir); + const baseline = JSON.parse(manifest!.baselines![target]!); + delete baseline.dependencies[templateDep]; + delete baseline.scripts[templateScript]; + manifest!.baselines![target] = `${JSON.stringify(baseline, null, 2)}\n`; + await writeScaffoldManifest(dir, manifest!); + + const edited = structuredClone(render); + delete edited.dependencies[templateDep]; + delete edited.scripts[templateScript]; + // User edits: a new dependency plus a customized script the template never touched. + edited.dependencies["left-pad"] = "^1.3.0"; + edited.scripts[userScript] = "echo custom"; + await writeFile(pkgPath, `${JSON.stringify(edited, null, 2)}\n`, "utf-8"); + + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + expect(plan.merged).toContain(target); + expect(plan.actionable).toContain(target); + expect(plan.manual.map((entry) => entry.path)).not.toContain(target); + + const applied = await applyScaffoldUpgrade(dir); + assertSuccess(applied); + expect(applied.applied.merged).toContain(target); + + // Union: template additions folded in, user edits preserved. + const result = JSON.parse(await readFile(pkgPath, "utf-8")); + expect(result.dependencies[templateDep]).toBe(render.dependencies[templateDep]); + expect(result.scripts[templateScript]).toBe(render.scripts[templateScript]); + expect(result.dependencies["left-pad"]).toBe("^1.3.0"); + expect(result.scripts[userScript]).toBe("echo custom"); + + // The content baseline advanced: a re-plan sees only the user's local edits. + const rePlan = await planScaffoldUpgrade(dir); + assertSuccess(rePlan); + expect(rePlan.merged).not.toContain(target); + expect(rePlan.userEdited).toContain(target); + expect(rePlan.actionable).not.toContain(target); + }); + + it("flags a conflict when the template and the user changed the same dependency", async () => { + const dir = await makeTempDir(); + await scaffoldWithBaseline(dir, makeConfig(dir)); + + const target = "apps/server/package.json"; + const pkgPath = join(dir, target); + const render = JSON.parse(await readFile(pkgPath, "utf-8")); + const [dep] = Object.keys(render.dependencies); + + // Template changed the version (baseline differs from the current render)... + const manifest = await readScaffoldManifest(dir); + const baseline = JSON.parse(manifest!.baselines![target]!); + baseline.dependencies[dep] = "0.0.1-old"; + manifest!.baselines![target] = `${JSON.stringify(baseline, null, 2)}\n`; + await writeScaffoldManifest(dir, manifest!); + + // ...and the user pinned their own version. + const edited = structuredClone(render); + edited.dependencies[dep] = "9.9.9"; + await writeFile(pkgPath, `${JSON.stringify(edited, null, 2)}\n`, "utf-8"); + + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + expect(plan.conflicts).toContain(target); + expect(plan.actionable).not.toContain(target); + const entry = plan.files.find((file) => file.path === target); + expect(entry?.reason).toContain(`dependencies.${dep}`); + + // Apply never touches a conflicted file: the user's pin wins. + const applied = await applyScaffoldUpgrade(dir); + assertSuccess(applied); + const result = JSON.parse(await readFile(pkgPath, "utf-8")); + expect(result.dependencies[dep]).toBe("9.9.9"); + }); + + it("falls back to manual review for package.json when the manifest has no content baseline", async () => { + const dir = await makeTempDir(); + await scaffoldWithBaseline(dir, makeConfig(dir)); + + // Simulate a manifest recorded by an older CLI (hashes only). + const manifest = await readScaffoldManifest(dir); + delete manifest!.baselines; + await writeScaffoldManifest(dir, manifest!); + + const pkgPath = join(dir, "package.json"); + const pkg = JSON.parse(await readFile(pkgPath, "utf-8")); + pkg.dependencies = { ...pkg.dependencies, "left-pad": "^1.3.0" }; + await writeFile(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`, "utf-8"); + + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + const entry = plan.manual.find((file) => file.path === "package.json"); + expect(entry).toBeDefined(); + expect(entry?.reason).toContain("baseline"); + expect(plan.actionable).not.toContain("package.json"); + }); + + it("appends template-added env keys to an edited .env.example, keeping user keys", async () => { + const dir = await makeTempDir(); + const config = makeConfig(dir, { + ecosystem: "go", + frontend: [], + backend: "none", + runtime: "none", + api: "none", + orm: "none", + database: "sqlite", + auth: "none", + goWebFramework: "gin", + goOrm: "gorm", + } as Partial); + await scaffoldWithBaseline(dir, config); + + const target = "apps/server/.env.example"; + const envPath = join(dir, target); + const render = await readFile(envPath, "utf-8"); + const lines = render.split("\n"); + const keyLine = lines.find((line) => /^[A-Z][A-Z0-9_]*=/.test(line)); + expect(keyLine).toBeDefined(); + const templateKey = (keyLine as string).split("=")[0] as string; + + // Simulate an older template without that key, plus a user-added key. + const withoutKey = lines.filter((line) => line !== keyLine).join("\n"); + const manifest = await readScaffoldManifest(dir); + manifest!.baselines![target] = withoutKey; + await writeScaffoldManifest(dir, manifest!); + await writeFile(envPath, `${withoutKey}\nCUSTOM_FLAG=1\n`, "utf-8"); + + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + expect(plan.merged).toContain(target); + const entry = plan.files.find((file) => file.path === target); + expect(entry?.reason).toContain(templateKey); + + const applied = await applyScaffoldUpgrade(dir); + assertSuccess(applied); + expect(applied.applied.merged).toContain(target); + const result = await readFile(envPath, "utf-8"); + expect(result).toContain(keyLine as string); + expect(result).toContain("CUSTOM_FLAG=1"); + }); + + it("routes .env (user secrets) to manual review, never auto-patching it", async () => { + const dir = await makeTempDir(); + await scaffoldWithBaseline(dir, makeConfig(dir)); + + // .env is generated for this stack; make it differ from the render. + const envPath = join(dir, "apps/server/.env"); + const original = await readFile(envPath, "utf-8"); + await writeFile(envPath, `${original}MY_SECRET=shh\n`, "utf-8"); + + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + const entry = plan.manual.find((file) => file.path === "apps/server/.env"); + expect(entry).toBeDefined(); + expect(plan.actionable).not.toContain("apps/server/.env"); + + const applied = await applyScaffoldUpgrade(dir); + assertSuccess(applied); + expect(await readFile(envPath, "utf-8")).toContain("MY_SECRET=shh"); + }); + it("never treats a generated README as drift, even when it differs from the render", async () => { const dir = await makeTempDir(); await scaffoldWithBaseline(dir, makeConfig(dir)); diff --git a/packages/template-generator/src/post-process/catalogs.ts b/packages/template-generator/src/post-process/catalogs.ts index 0ea17252b..84a1ffa06 100644 --- a/packages/template-generator/src/post-process/catalogs.ts +++ b/packages/template-generator/src/post-process/catalogs.ts @@ -67,7 +67,8 @@ export function processCatalogs(vfs: VirtualFileSystem, config: ProjectConfig): } } - const catalog = findDuplicateDependencies(packagesInfo, config.projectName); + const existingCatalog = readExistingCatalog(vfs, config); + const catalog = findDuplicateDependencies(packagesInfo, config.projectName, existingCatalog); if (Object.keys(catalog).length === 0) return; @@ -80,9 +81,35 @@ export function processCatalogs(vfs: VirtualFileSystem, config: ProjectConfig): updatePackageJsonsWithCatalogs(vfs, packagesInfo, catalog); } +/** + * Read the catalog a previous processCatalogs pass may already have written + * (graph mode runs the post-process pipeline more than once). Counting those + * entries again lets a later pass fold in dependencies added in between (e.g. + * the database package) instead of leaving them at literal versions. + */ +function readExistingCatalog(vfs: VirtualFileSystem, config: ProjectConfig): Record { + if (config.packageManager === "bun") { + const pkgJson = vfs.readJson("package.json"); + const workspaces = pkgJson?.workspaces; + if (workspaces && !Array.isArray(workspaces) && workspaces.catalog) { + return workspaces.catalog; + } + return {}; + } + + const content = vfs.readFile("pnpm-workspace.yaml"); + if (!content) return {}; + try { + return (yaml.parse(content)?.catalog as Record | undefined) ?? {}; + } catch { + return {}; + } +} + function findDuplicateDependencies( packagesInfo: PackageInfo[], projectName: string, + existingCatalog: Record = {}, ): Record { const depCount = new Map(); const projectScope = `@${projectName}/`; @@ -90,10 +117,14 @@ function findDuplicateDependencies( for (const pkg of packagesInfo) { const allDeps = { ...pkg.dependencies, ...pkg.devDependencies }; - for (const [depName, version] of Object.entries(allDeps)) { + for (const [depName, rawVersion] of Object.entries(allDeps)) { if (depName.startsWith(projectScope)) continue; - if (version.startsWith("workspace:")) continue; - if (version.startsWith("catalog:")) continue; + if (rawVersion.startsWith("workspace:")) continue; + // Resolve plain catalog references through the already-written catalog so + // they still count as participants; skip named catalogs and unresolvable + // references outright. + const version = rawVersion === "catalog:" ? existingCatalog[depName] : rawVersion; + if (version === undefined || version.startsWith("catalog:")) continue; const existing = depCount.get(depName); if (existing) { From 74cb699d48e393a6d8178e0ff7b26ef2cb537965 Mon Sep 17 00:00:00 2001 From: Marve10s Date: Fri, 10 Jul 2026 14:49:37 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix(cli):=20harden=20structured=20merge=20p?= =?UTF-8?q?er=20review=20=E2=80=94=20deletions,=20uncovered=20changes,=20e?= =?UTF-8?q?nv=20baseline?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three review findings on the bfs update structured merge: - diffJsonSection: a user-side deletion of a key the baseline had now blocks the key like any other user edit when the template also changed it, instead of silently re-adding it (applies to stack-update merges too). - package.json: template changes mergePackageJson cannot express — section key removals and changed top-level fields (exports, workspaces, ...) — route the file to manual review naming the changes, instead of being silently dropped under a user-edited label or partially merged. Comparison is key-order insensitive so legitimate render reordering does not false-positive. - *.env.example without a recorded content baseline now falls back to manual review like package.json; merging with an undefined previous side would mistake every proposed key for a template addition and re-append keys the user deliberately removed. --- apps/cli/src/helpers/core/scaffold-upgrade.ts | 91 ++++++++++++-- apps/cli/src/helpers/core/stack-update.ts | 13 +- apps/cli/test/scaffold-upgrade.test.ts | 116 ++++++++++++++++++ 3 files changed, 210 insertions(+), 10 deletions(-) diff --git a/apps/cli/src/helpers/core/scaffold-upgrade.ts b/apps/cli/src/helpers/core/scaffold-upgrade.ts index 06cefad53..f9892bfd3 100644 --- a/apps/cli/src/helpers/core/scaffold-upgrade.ts +++ b/apps/cli/src/helpers/core/scaffold-upgrade.ts @@ -21,6 +21,7 @@ import { generateTree, mergeEnvExample, mergePackageJson, + PACKAGE_JSON_SECTIONS, treeToFileMap, } from "./stack-update"; @@ -175,6 +176,67 @@ async function renderCurrentProject( } } +/** Deep equality ignoring object key order (renders may reorder catalog maps etc.). */ +function deepEqualUnordered(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (Array.isArray(a) || Array.isArray(b)) { + return ( + Array.isArray(a) && + Array.isArray(b) && + a.length === b.length && + a.every((item, index) => deepEqualUnordered(item, b[index])) + ); + } + if (a && b && typeof a === "object" && typeof b === "object") { + const aRecord = a as Record; + const bRecord = b as Record; + const aKeys = Object.keys(aRecord); + return ( + aKeys.length === Object.keys(bRecord).length && + aKeys.every((key) => key in bRecord && deepEqualUnordered(aRecord[key], bRecord[key])) + ); + } + return false; +} + +/** + * Template-side package.json changes mergePackageJson cannot express: key + * removals inside the merged sections and any change to other top-level fields + * (exports, workspaces, type, ...). Files with such changes go to manual review + * instead of being silently labeled user-edited or partially merged. + */ +function findUnmergeableTemplateChanges( + previousContent: string, + proposedContent: string, +): string[] { + let previous: unknown; + let proposed: unknown; + try { + previous = JSON.parse(previousContent); + proposed = JSON.parse(proposedContent); + } catch { + return []; // mergePackageJson already reports invalid JSON as a blocker + } + const isRecord = (value: unknown): value is Record => + Boolean(value && typeof value === "object" && !Array.isArray(value)); + if (!isRecord(previous) || !isRecord(proposed)) return []; + + const changes: string[] = []; + const mergedSections = new Set(PACKAGE_JSON_SECTIONS); + for (const section of PACKAGE_JSON_SECTIONS) { + const previousSection = isRecord(previous[section]) ? previous[section] : {}; + const proposedSection = isRecord(proposed[section]) ? proposed[section] : {}; + for (const key of Object.keys(previousSection)) { + if (!(key in proposedSection)) changes.push(`${section}.${key} removed`); + } + } + for (const key of new Set([...Object.keys(previous), ...Object.keys(proposed)])) { + if (mergedSections.has(key)) continue; + if (!deepEqualUnordered(previous[key], proposed[key])) changes.push(key); + } + return changes; +} + /** * Structured 3-way merge for package.json / *.env.example, reusing stack-update's * merge semantics: template-side changes (proposed vs the recorded render @@ -192,15 +254,20 @@ function classifyStructuredMerge( return { path: filePath, category: "manual", reason: "no comparable template render" }; } + // Without a recorded render baseline there is no "previous" side to diff + // against: package.json cannot 3-way merge at all, and an env merge would + // mistake every proposed key for a template addition and re-append keys the + // user deliberately removed. Both fall back to manual review. + if (baselineContent === undefined) { + return { + path: filePath, + category: "manual", + reason: + "no structured-merge baseline recorded — merge by hand or re-run `update --record-baseline`", + }; + } + if (path.basename(filePath) === "package.json") { - if (baselineContent === undefined) { - return { - path: filePath, - category: "manual", - reason: - "no structured-merge baseline recorded — merge dependencies by hand or re-run `update --record-baseline`", - }; - } const merged = mergePackageJson(existingContent, baselineContent, proposedContent); if (merged.blockers.length > 0) { return { @@ -209,6 +276,14 @@ function classifyStructuredMerge( reason: `template and local copy both changed: ${merged.blockers.join(", ")}`, }; } + const uncovered = findUnmergeableTemplateChanges(baselineContent, proposedContent); + if (uncovered.length > 0) { + return { + path: filePath, + category: "manual", + reason: `template changes the merge cannot apply (${uncovered.join(", ")}) — update by hand`, + }; + } if (merged.content) { return { path: filePath, diff --git a/apps/cli/src/helpers/core/stack-update.ts b/apps/cli/src/helpers/core/stack-update.ts index 4a03e6518..7ffec1bb3 100644 --- a/apps/cli/src/helpers/core/stack-update.ts +++ b/apps/cli/src/helpers/core/stack-update.ts @@ -155,7 +155,12 @@ const RISKY_ARCHITECTURE_KEYS: Array = [ "backend", "runtime", ]; -const PACKAGE_JSON_SECTIONS = ["dependencies", "devDependencies", "peerDependencies", "scripts"]; +export const PACKAGE_JSON_SECTIONS = [ + "dependencies", + "devDependencies", + "peerDependencies", + "scripts", +]; const BINARY_FILE_MARKER = "[Binary file]"; function isEnvFilePath(filePath: string): boolean { @@ -1261,7 +1266,11 @@ function diffJsonSection( for (const [name, proposedValue] of Object.entries(proposedSection)) { if (previousSection[name] === proposedValue) continue; const currentValue = currentSection[name]; - if (currentValue !== undefined && currentValue !== previousSection[name]) { + // Any user-side divergence from the baseline blocks the key — including a + // deletion (baseline had the key, the user removed it): re-adding it would + // silently undo the user's edit. A key absent from both baseline and + // current is a plain template addition and merges cleanly. + if (currentValue !== previousSection[name]) { blockers.push(`${section}.${name}`); continue; } diff --git a/apps/cli/test/scaffold-upgrade.test.ts b/apps/cli/test/scaffold-upgrade.test.ts index a41c463d3..943e18836 100644 --- a/apps/cli/test/scaffold-upgrade.test.ts +++ b/apps/cli/test/scaffold-upgrade.test.ts @@ -375,6 +375,78 @@ describe("scaffold-upgrade engine", () => { expect(result.dependencies[dep]).toBe("9.9.9"); }); + it("blocks re-adding a dependency the user deleted when the template changed it", async () => { + const dir = await makeTempDir(); + await scaffoldWithBaseline(dir, makeConfig(dir)); + + const target = "apps/server/package.json"; + const pkgPath = join(dir, target); + const render = JSON.parse(await readFile(pkgPath, "utf-8")); + const [dep] = Object.keys(render.dependencies); + + // Template changed the version since the baseline... + const manifest = await readScaffoldManifest(dir); + const baseline = JSON.parse(manifest!.baselines![target]!); + baseline.dependencies[dep] = "0.0.1-old"; + manifest!.baselines![target] = `${JSON.stringify(baseline, null, 2)}\n`; + await writeScaffoldManifest(dir, manifest!); + + // ...and the user deleted the dependency entirely. + const edited = structuredClone(render); + delete edited.dependencies[dep]; + await writeFile(pkgPath, `${JSON.stringify(edited, null, 2)}\n`, "utf-8"); + + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + expect(plan.conflicts).toContain(target); + expect(plan.actionable).not.toContain(target); + const entry = plan.files.find((file) => file.path === target); + expect(entry?.reason).toContain(`dependencies.${dep}`); + + // Apply must not resurrect the deleted dependency. + const applied = await applyScaffoldUpgrade(dir); + assertSuccess(applied); + const result = JSON.parse(await readFile(pkgPath, "utf-8")); + expect(result.dependencies[dep]).toBeUndefined(); + }); + + it("routes template changes outside the merged sections to manual review", async () => { + const dir = await makeTempDir(); + await scaffoldWithBaseline(dir, makeConfig(dir)); + + const target = "apps/server/package.json"; + const pkgPath = join(dir, target); + const render = JSON.parse(await readFile(pkgPath, "utf-8")); + + // Simulate an older template that shipped an extra dependency and a + // top-level field the current template no longer has: baseline and disk + // both carry them (user never touched the file), the proposed render lacks + // them. mergePackageJson cannot express removals or top-level changes. + const older = structuredClone(render); + older.dependencies["legacy-sdk"] = "1.0.0"; + older.sideEffects = false; + const olderContent = `${JSON.stringify(older, null, 2)}\n`; + const manifest = await readScaffoldManifest(dir); + manifest!.baselines![target] = olderContent; + await writeScaffoldManifest(dir, manifest!); + await writeFile(pkgPath, olderContent, "utf-8"); + + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + const entry = plan.manual.find((file) => file.path === target); + expect(entry).toBeDefined(); + expect(entry?.reason).toContain("dependencies.legacy-sdk removed"); + expect(entry?.reason).toContain("sideEffects"); + expect(plan.merged).not.toContain(target); + expect(plan.userEdited).not.toContain(target); + expect(plan.actionable).not.toContain(target); + + // Apply leaves the file for the user to reconcile. + const applied = await applyScaffoldUpgrade(dir); + assertSuccess(applied); + expect(await readFile(pkgPath, "utf-8")).toBe(olderContent); + }); + it("falls back to manual review for package.json when the manifest has no content baseline", async () => { const dir = await makeTempDir(); await scaffoldWithBaseline(dir, makeConfig(dir)); @@ -442,6 +514,50 @@ describe("scaffold-upgrade engine", () => { expect(result).toContain("CUSTOM_FLAG=1"); }); + it("routes an edited .env.example to manual review when the manifest has no content baseline", async () => { + const dir = await makeTempDir(); + const config = makeConfig(dir, { + ecosystem: "go", + frontend: [], + backend: "none", + runtime: "none", + api: "none", + orm: "none", + database: "sqlite", + auth: "none", + goWebFramework: "gin", + goOrm: "gorm", + } as Partial); + await scaffoldWithBaseline(dir, config); + + const target = "apps/server/.env.example"; + const envPath = join(dir, target); + const render = await readFile(envPath, "utf-8"); + const lines = render.split("\n"); + const keyLine = lines.find((line) => /^[A-Z][A-Z0-9_]*=/.test(line)); + expect(keyLine).toBeDefined(); + + // Older-CLI manifest (hashes only) + the user deliberately removed a key. + const manifest = await readScaffoldManifest(dir); + delete manifest!.baselines; + await writeScaffoldManifest(dir, manifest!); + await writeFile(envPath, lines.filter((line) => line !== keyLine).join("\n"), "utf-8"); + + // Without a baseline, a merge would mistake the removed key for a template + // addition and re-append it — the file must go to manual review instead. + const plan = await planScaffoldUpgrade(dir); + assertSuccess(plan); + const entry = plan.manual.find((file) => file.path === target); + expect(entry).toBeDefined(); + expect(entry?.reason).toContain("baseline"); + expect(plan.merged).not.toContain(target); + expect(plan.actionable).not.toContain(target); + + const applied = await applyScaffoldUpgrade(dir); + assertSuccess(applied); + expect(await readFile(envPath, "utf-8")).not.toContain(keyLine as string); + }); + it("routes .env (user secrets) to manual review, never auto-patching it", async () => { const dir = await makeTempDir(); await scaffoldWithBaseline(dir, makeConfig(dir));