fix(cli): make failed meta deps sync atomic and fail check on unloadable snapshot

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**

docs/features/metadata-dependencies.md

@@ -81,8 +81,11 @@ here). It then writes the committed snapshot
 (`.metaobjects/deps/<name>/<name>.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.

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,34 +64,54 @@ 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<number> => {
+    message();
+    if (backup) {
+      await backup.restore();
+      log.error("meta deps sync: rolled back — the previous lock and snapshot are restored.");
+    }
+    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") {
-    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
@@ -101,16 +122,18 @@ 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();
+  printReport();
   return 0;
 }
 
@@ -176,6 +199,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 +219,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<number> {

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<void>;
+  /** Drop the saved copy; call once the sync is known good. */
+  discard(): Promise<void>;
+}
+
+export async function backupSyncState(configDir: string): Promise<SyncBackup> {
+  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

server/typescript/packages/cli/test/deps-sync.test.ts

@@ -563,3 +563,103 @@ describe("meta deps sync <name> — 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",
+    );
+
+    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");
+  });
+});
+
+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");
+  });
+});