From e41f44ecb9709b1167fa93b3b77be1ac8fddc889 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Tue, 27 Oct 2026 23:21:56 -0400 Subject: [PATCH 1/3] fix(cli): make meta deps sync atomic and meta deps check fail on an unloadable snapshot --- CHANGELOG.md | 10 ++ .../packages/cli/src/commands/deps.ts | 42 +++++++-- .../packages/cli/src/lib/dependency-sync.ts | 39 +++++++- .../packages/cli/test/deps-sync.test.ts | 92 +++++++++++++++++++ 4 files changed, 175 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e714671c9..71e4e9900a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,16 @@ here.** [docs/features/api-contract.md](docs/features/api-contract.md#report-export) and [ADR-0031 Amendment 1](spec/decisions/ADR-0031-serialization-vs-data-download-boundary.md). +### Fixed + +- **A failed `meta deps sync` no longer leaves a lock and snapshot that cannot load.** The + command wrote `deps.lock.json` and the `.metaobjects/deps/` snapshot before loading the + combined model, and on a load failure it exited 1 but kept the new files. Because the lock + and snapshot agreed with each other, `meta deps check` then reported green while every other + command failed to load. The sync now saves the previous lock and snapshot and restores both + on any failure, so the previous state is intact. `meta deps check` also loads the model once + its hashes match and exits 1 with the load error when it does not load. + ### Documentation - **The fit-assessment prompt's capability sheet is re-checked against 1.1.4 / Maven 8.1.4** diff --git a/server/typescript/packages/cli/src/commands/deps.ts b/server/typescript/packages/cli/src/commands/deps.ts index 666f5828a0..75f97e4651 100644 --- a/server/typescript/packages/cli/src/commands/deps.ts +++ b/server/typescript/packages/cli/src/commands/deps.ts @@ -19,6 +19,7 @@ import { reportLoadError } from "../lib/load-error.js"; import { collectionLoadOptions } from "../lib/collection-load-options.js"; import { applySync, + backupSyncState, checkDependencies, ERR_DEPENDENCY_UPSTREAM_DRIFT, formatCheckLine, @@ -63,12 +64,22 @@ async function runSync(configDir: string, specs: readonly DependencySpec[], flag return 1; } + // A real sync is atomic: keep a copy of the lock and snapshot, and put them + // back if the sync or the load that proves it fails, so a failed sync leaves + // the previous (loadable) state in place rather than one that checks green + // and loads nowhere. + const backup = flags.dryRun ? undefined : await backupSyncState(configDir); + const fail = async (message: () => void): Promise => { + message(); + await backup?.restore(); + return 1; + }; + let applied; try { applied = await applySync(configDir, plan, { dryRun: flags.dryRun }); } catch (err) { - log.error(describeError(err)); - return 1; + return fail(() => log.error(describeError(err))); } if (fmt === "text") { @@ -101,16 +112,17 @@ async function runSync(configDir: string, specs: readonly DependencySpec[], flag try { collection = await resolveCollection(configDir); } catch (err) { - log.error(`meta deps sync: the updated dependency set does not resolve: ${describeError(err)}`); - return 1; + return fail(() => + log.error(`meta deps sync: the updated dependency set does not resolve: ${describeError(err)}`), + ); } try { await loadMemory(collection.configDir, collectionLoadOptions(collection)); } catch (err) { - reportLoadError(log, "meta deps sync: the updated dependency set does not load", err); - return 1; + return fail(() => reportLoadError(log, "meta deps sync: the updated dependency set does not load", err)); } + await backup?.discard(); return 0; } @@ -176,6 +188,18 @@ async function runCheck( const results = await checkDependencies(configDir, specs, lock); const failing = results.filter((r) => r.status !== "current"); + // Matching hashes only prove the snapshot is the locked one, not that the + // model still loads with it. A check that cannot load must not pass. + let loadFailure: string | undefined; + if (failing.length === 0) { + try { + const collection = await resolveCollection(configDir); + await loadMemory(collection.configDir, collectionLoadOptions(collection)); + } catch (err) { + loadFailure = describeError(err); + } + } + if (fmt === "text") { for (const r of results) log.info(formatCheckLine(r)); if (failing.length > 0) { @@ -184,17 +208,21 @@ async function runCheck( `${results.length === 1 ? "y" : "ies"} drifted or unresolved (${ERR_DEPENDENCY_UPSTREAM_DRIFT}).`, ); } + if (loadFailure !== undefined) { + log.error(`meta deps check: the locked dependency set does not load: ${loadFailure}`); + } } else { emitStructured( { dependencies: results.map((r) => ({ ...r, line: formatCheckLine(r) })), ...(failing.length > 0 ? { code: ERR_DEPENDENCY_UPSTREAM_DRIFT } : {}), + ...(loadFailure !== undefined ? { loadError: loadFailure } : {}), }, fmt, ); } - return failing.length > 0 ? 1 : 0; + return failing.length > 0 || loadFailure !== undefined ? 1 : 0; } export async function depsCommand(args: string[], cwd: string, fmt: OutputFormat): Promise { diff --git a/server/typescript/packages/cli/src/lib/dependency-sync.ts b/server/typescript/packages/cli/src/lib/dependency-sync.ts index f642172400..b9b9e00d9d 100644 --- a/server/typescript/packages/cli/src/lib/dependency-sync.ts +++ b/server/typescript/packages/cli/src/lib/dependency-sync.ts @@ -10,7 +10,8 @@ // resolution, per-dependency validation, the cross-dependency node-collision // check); `applySync` is the only function that touches the filesystem. import { existsSync, statSync } from "node:fs"; -import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { cp, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { codeSource, @@ -467,6 +468,42 @@ export async function applySync( return { lock, report }; } +/** + * A copy of everything `applySync` writes — `deps.lock.json` and the + * `.metaobjects/deps/` snapshot tree — so a sync that fails AFTER writing + * (the full-collection load in `deps.ts`) can put the previous state back. + * Without it a failed sync leaves a lock and snapshot that agree with each + * other (so `meta deps check` is green) but do not load. + */ +export interface SyncBackup { + /** Put the lock and snapshot tree back exactly as `backupSyncState` saw them. */ + restore(): Promise; + /** Drop the saved copy; call once the sync is known good. */ + discard(): Promise; +} + +export async function backupSyncState(configDir: string): Promise { + const lockPath = join(configDir, DEFAULT_METAOBJECTS_DIR, LOCK_FILE); + const depsPath = join(configDir, DEFAULT_METAOBJECTS_DIR, DEPS_DIR); + const hadLock = existsSync(lockPath); + const hadDeps = existsSync(depsPath); + const saved = await mkdtemp(join(tmpdir(), "meta-deps-backup-")); + if (hadLock) await cp(lockPath, join(saved, "lock")); + if (hadDeps) await cp(depsPath, join(saved, "deps"), { recursive: true }); + return { + async restore() { + await rm(lockPath, { force: true }); + await rm(depsPath, { recursive: true, force: true }); + if (hadDeps) await cp(join(saved, "deps"), depsPath, { recursive: true }); + if (hadLock) await cp(join(saved, "lock"), lockPath); + await rm(saved, { recursive: true, force: true }); + }, + async discard() { + await rm(saved, { recursive: true, force: true }); + }, + }; +} + // --------------------------------------------------------------------------- // Task 15 — `meta deps check` / `verify --deps`: does the INSTALLED artifact // (re-resolved live, exactly as `sync` steps 1-2 do) still hash to what the diff --git a/server/typescript/packages/cli/test/deps-sync.test.ts b/server/typescript/packages/cli/test/deps-sync.test.ts index a211daf53c..462f6928c6 100644 --- a/server/typescript/packages/cli/test/deps-sync.test.ts +++ b/server/typescript/packages/cli/test/deps-sync.test.ts @@ -563,3 +563,95 @@ describe("meta deps sync — the name filter (FR-023 Phase 1a Task 14, fi expect(after.dependencies[SECOND_NAME]).toEqual(baseline.dependencies[SECOND_NAME]); }); }); + +describe("meta deps sync — a failed post-sync load leaves the previous state intact", () => { + /** A consumer file redeclaring a node the dependency exports: the plan + * (which compares declared node LISTS) passes, the full-collection load + * does not. */ + const COLLIDING = JSON.stringify({ + "metadata.root": { + package: "acme::common", + children: [ + { + "object.entity": { + name: "Customer", + children: [ + { "source.rdb": { "@table": "customers" } }, + { "field.long": { name: "id" } }, + { "identity.primary": { name: "pk", "@fields": ["id"] } }, + ], + }, + }, + ], + }, + }); + + test("a first sync whose load fails exits 1 and leaves no lock and no snapshot", async () => { + const { consumerRoot } = setupProject(); + writeFileSync(join(consumerRoot, "metaobjects", "meta.clash.json"), COLLIDING, "utf8"); + + const exit = await run(["deps", "sync", "--format", "text", "--cwd", consumerRoot]); + expect(exit).toBe(1); + expect(existsSync(join(consumerRoot, ".metaobjects", "deps.lock.json"))).toBe(false); + expect(existsSync(join(consumerRoot, ".metaobjects", "deps", DEP_NAME))).toBe(false); + }); + + test("a re-sync whose load fails restores the previous lock and snapshot byte for byte", async () => { + const { consumerRoot, publisherDir } = setupProject(); + expect(await run(["deps", "sync", "--format", "text", "--cwd", consumerRoot])).toBe(0); + const lockPath = join(consumerRoot, ".metaobjects", "deps.lock.json"); + const snapPath = join(consumerRoot, ".metaobjects", "deps", DEP_NAME, ARTIFACT_BASENAME); + const lockBefore = readFileSync(lockPath); + const snapBefore = readFileSync(snapPath); + + // The consumer's model now clashes with the dependency, and the publisher + // ships a new version: the sync must fail without touching the old state. + writeFileSync(join(consumerRoot, "metaobjects", "meta.clash.json"), COLLIDING, "utf8"); + writeFileSync(join(publisherDir, ARTIFACT_BASENAME), WIDENED_BYTES); + writeFileSync( + join(publisherDir, MANIFEST_BASENAME), + manifestJson({ version: "1.1.0", integrity: WIDENED_HASH, nodes: NODES }), + "utf8", + ); + + const exit = await run(["deps", "sync", "--format", "text", "--cwd", consumerRoot]); + expect(exit).toBe(1); + expect(readFileSync(lockPath)).toEqual(lockBefore); + expect(readFileSync(snapPath)).toEqual(snapBefore); + }); +}); + +describe("meta deps check — a snapshot that cannot load is not green", () => { + test("hashes match the lock but the model no longer loads: exit 1 with the load error", async () => { + const { consumerRoot } = setupProject(); + expect(await run(["deps", "sync", "--format", "text", "--cwd", consumerRoot])).toBe(0); + expect(await run(["deps", "check", "--format", "text", "--cwd", consumerRoot])).toBe(0); + + writeFileSync( + join(consumerRoot, "metaobjects", "meta.clash.json"), + JSON.stringify({ + "metadata.root": { + package: "acme::common", + children: [ + { + "object.entity": { + name: "Customer", + children: [ + { "source.rdb": { "@table": "customers" } }, + { "field.long": { name: "id" } }, + { "identity.primary": { name: "pk", "@fields": ["id"] } }, + ], + }, + }, + ], + }, + }), + "utf8", + ); + + out = []; + err = []; + expect(await run(["deps", "check", "--format", "text", "--cwd", consumerRoot])).toBe(1); + expect([...out, ...err].join("\n")).toContain("does not load"); + }); +}); From 502d9b32d26a8689f64ca0381b3903a2dc3c67bb Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Tue, 27 Oct 2026 23:50:14 -0400 Subject: [PATCH 2/3] no-mistakes(test): Print deps sync report only after load succeeds --- .../packages/cli/src/commands/deps.ts | 43 ++++++++++++------- .../packages/cli/test/deps-sync.test.ts | 8 ++++ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/server/typescript/packages/cli/src/commands/deps.ts b/server/typescript/packages/cli/src/commands/deps.ts index 75f97e4651..4c42487500 100644 --- a/server/typescript/packages/cli/src/commands/deps.ts +++ b/server/typescript/packages/cli/src/commands/deps.ts @@ -71,7 +71,10 @@ async function runSync(configDir: string, specs: readonly DependencySpec[], flag const backup = flags.dryRun ? undefined : await backupSyncState(configDir); const fail = async (message: () => void): Promise => { message(); - await backup?.restore(); + if (backup) { + await backup.restore(); + log.error("meta deps sync: rolled back — the previous lock and snapshot are restored."); + } return 1; }; @@ -82,26 +85,33 @@ async function runSync(configDir: string, specs: readonly DependencySpec[], flag return fail(() => log.error(describeError(err))); } - if (fmt === "text") { - for (const w of plan.warnings) log.info(w); - if (applied.report.length === 0) { - log.info("meta deps sync: nothing to do — no dependencies declared."); + // The per-dependency report is printed only once the sync is known good: a + // report of "synced" for a sync that is then rolled back would lie. + const printReport = (): void => { + if (fmt === "text") { + for (const w of plan.warnings) log.info(w); + if (applied.report.length === 0) { + log.info("meta deps sync: nothing to do — no dependencies declared."); + } else { + for (const r of applied.report) log.info(r.line); + } } else { - for (const r of applied.report) log.info(r.line); + emitStructured( + { + warnings: plan.warnings, + dependencies: applied.report.map((r) => ({ name: r.name, action: r.action, message: r.line })), + }, + fmt, + ); } - } else { - emitStructured( - { - warnings: plan.warnings, - dependencies: applied.report.map((r) => ({ name: r.name, action: r.action, message: r.line })), - }, - fmt, - ); - } + }; // A dry run wrote nothing to validate — the plan already stands for what // WOULD load. - if (flags.dryRun) return 0; + if (flags.dryRun) { + printReport(); + return 0; + } // DESIGN §4.1's closing sentence: load the full collection once, so a sync // that produces an unloadable model (a collision the plan missed because it @@ -123,6 +133,7 @@ async function runSync(configDir: string, specs: readonly DependencySpec[], flag } await backup?.discard(); + printReport(); return 0; } diff --git a/server/typescript/packages/cli/test/deps-sync.test.ts b/server/typescript/packages/cli/test/deps-sync.test.ts index 462f6928c6..8075230b26 100644 --- a/server/typescript/packages/cli/test/deps-sync.test.ts +++ b/server/typescript/packages/cli/test/deps-sync.test.ts @@ -614,10 +614,18 @@ describe("meta deps sync — a failed post-sync load leaves the previous state i "utf8", ); + out = []; + err = []; const exit = await run(["deps", "sync", "--format", "text", "--cwd", consumerRoot]); expect(exit).toBe(1); expect(readFileSync(lockPath)).toEqual(lockBefore); expect(readFileSync(snapPath)).toEqual(snapBefore); + // The failure output reports the rollback and the load error, and never a + // "synced" line for a sync that was reverted. + const output = [...out, ...err].join("\n"); + expect(output).toContain("rolled back"); + expect(output).toContain("does not load"); + expect(output).not.toContain("synced"); }); }); From dce91b0783ddc457fe81add1114f32f537a92362 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Wed, 28 Oct 2026 01:05:05 -0400 Subject: [PATCH 3/3] no-mistakes(document): Docs updated for atomic deps sync and check load step --- docs/features/metadata-dependencies.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/features/metadata-dependencies.md b/docs/features/metadata-dependencies.md index 67f838da9c..da1f457ff4 100644 --- a/docs/features/metadata-dependencies.md +++ b/docs/features/metadata-dependencies.md @@ -81,8 +81,11 @@ here). It then writes the committed snapshot (`.metaobjects/deps//.metaobjects.json`) and the lock (`.metaobjects/deps.lock.json`), and finally loads the whole updated collection once — so a sync that produces an unloadable model fails in `sync` itself, with the -loader's own error, rather than surfacing on the next unrelated `meta gen`. Narrow to -one or more names as positionals; `--dry-run` reports the plan and writes nothing. +loader's own error, rather than surfacing on the next unrelated `meta gen`. A failed +sync is atomic: it restores the previous lock and snapshot, prints the rollback and +the load error, and exits 1. The per-dependency `synced` / `unchanged` / `pruned` +lines print only after the load succeeds. Narrow to one or more names as positionals; +`--dry-run` reports the plan and writes nothing. ``` $ meta deps sync @@ -91,7 +94,9 @@ synced acme-common 1.0.0→1.1.0 (10fbf886→a1b2c3d4) **`meta deps check`** re-resolves each declared dependency exactly as `sync`'s first two steps do, and compares the artifact's *currently installed* hash against what the -lock recorded — read-only, it never touches the lock or the snapshot. It needs the +lock recorded. Once the hashes match, it also loads the committed snapshot and exits 1 +with the load error if the model does not load: matching hashes do not prove the model +loads. It never writes the lock or the snapshot. It needs the publisher's `path` reachable right now, which is why it is a separate command from loading (below): CI running against a committed snapshot does not need the publisher checked out, but a `check` run does.