fix(workspace): materialize refuses files whose disk copy changed since last write

changelog.d/materialize-freshness-guard.fixed.md

@@ -0,0 +1 @@
+- **`materialize` no longer silently overwrites disk files changed by another writer** (#109). Before writing, each file's disk hash is compared against what the workspace layer last materialized; a mismatch means a shell/tool edit happened since, and the path is refused and listed in the result (`skipped`, with the remedy) instead of silently reverted. A disk copy that exists but cannot be read (e.g. a write-only file) is refused the same way, with the filesystem error in the reason, rather than treated as absent. `force: true` remains the deliberate override. Refused paths stay owed to every later materialize (and count in `status.pendingChanges`) without holding back the sequence the branch guard reads. `sync` re-pins the baseline when it adopts a disk copy, so the agent's next edit to that file is not refused. Baselines and refused paths persist across restarts, and a first materialize that finds every file already in place still pins the branch. Files whose disk copy already matches the target bytes are left untouched (mtime preserved). The post-undo/redo `_config` restore keeps its overwrite semantics.

src/modules/workspace/index.ts

@@ -673,6 +673,7 @@ export class WorkspaceModule implements Module {
         initialSyncDone: false,
         lastMaterializedBranchId: null,
         materializedHashes: new Map(),
+        refusedPaths: new Set(),
         watcherReadyAt: null,
         watcherError: null,
       };
@@ -712,6 +713,12 @@ export class WorkspaceModule implements Module {
       if (mount) {
         mount.lastMaterializedSeq = meta.lastMaterializedSeq;
         mount.lastMaterializedBranchId = meta.lastMaterializedBranchId ?? null;
+        // Freshness-guard baselines: without them the first materialize after
+        // a restart can't tell a shell edit from our own last write (#109).
+        for (const [path, hash] of Object.entries(meta.materializedHashes ?? {})) {
+          if (!mount.materializedHashes.has(path)) mount.materializedHashes.set(path, hash);
+        }
+        for (const path of meta.refusedPaths ?? []) mount.refusedPaths.add(path);
         // watcherReadyAt intentionally not restored — each session must
         // observe its own watcher attach, otherwise a stale timestamp
         // would hide a new-session attach failure.
@@ -900,6 +907,8 @@ export class WorkspaceModule implements Module {
         state.mounts[name] = {
           lastMaterializedSeq: mount.lastMaterializedSeq,
           lastMaterializedBranchId: mount.lastMaterializedBranchId ?? undefined,
+          materializedHashes: Object.fromEntries(mount.materializedHashes),
+          refusedPaths: [...mount.refusedPaths],
           watcherReadyAt: mount.watcherReadyAt,
           watcherError: mount.watcherError,
         };
@@ -1053,7 +1062,7 @@ export class WorkspaceModule implements Module {
           properties: {
             path: { type: 'string', description: 'Specific path to materialize (optional — defaults to all changed)' },
             mount: { type: 'string', description: 'Specific mount (optional)' },
-            force: { type: 'boolean', description: 'Materialize even if the current branch has diverged from the branch last written to disk (default false). Only needed for genuine divergence — descendant branches pass automatically.' },
+            force: { type: 'boolean', description: 'Overwrite files whose disk copy changed since the last materialize (another writer), and materialize even if the current branch has diverged from the branch last written to disk (default false). Without it, divergent files are skipped and listed.' },
           },
         },
       },
@@ -2229,6 +2238,8 @@ export class WorkspaceModule implements Module {
       const changes = mount.lastMaterializedSeq > 0
         ? store.treeDiff(mount.treeStateId, mount.lastMaterializedSeq, currentSeq)
         : [];
+      // Refused paths (#109) are pending too, past the watermark or not.
+      const pending = new Set([...changes.map((c) => c.path), ...mount.refusedPaths]);
 
       const currentBranch = store.currentBranch();
       status[name] = {
@@ -2238,7 +2249,7 @@ export class WorkspaceModule implements Module {
         fileCount: entries.length,
         lastMaterializedSeq: mount.lastMaterializedSeq,
         currentSeq,
-        pendingChanges: changes.length,
+        pendingChanges: pending.size,
         initialSyncDone: mount.initialSyncDone,
         currentBranch: currentBranch.name,
         lastMaterializedBranch: mount.lastMaterializedBranchId,
@@ -2307,19 +2318,25 @@ export class WorkspaceModule implements Module {
 
       // Suppress watcher for paths we're about to write
       const watcher = this.watchers.get(name);
-      const written = await materializeToFs(store, mount, paths);
+      const { written, unchanged, skipped } = await materializeToFs(store, mount, paths, { force: input.force });
 
       for (const p of written) {
         watcher?.suppress(p);
         allWritten.push({ mount: name, path: p });
       }
+      // Freshness-guard refusals (#109) ride the same skipped list as branch
+      // blocks: divergence must be visible, not resolved silently either way.
+      for (const s of skipped) {
+        blocked.push({ mount: name, reason: `${s.path}: ${s.reason}` });
+      }
 
       // Track which branch we materialized on. Re-pin on a clean empty
       // materialize too (previously-pinned mount, nothing pending): disk
       // already reflects the current branch's tree, and leaving the old pin
       // would keep force required forever after a cross-branch materialize
-      // that happened to write nothing.
-      if (written.length > 0 || mount.lastMaterializedBranchId !== null) {
+      // that happened to write nothing. A first materialize that found every
+      // file already in place pins too: disk holds this branch's tree.
+      if (written.length > 0 || unchanged.length > 0 || mount.lastMaterializedBranchId !== null) {
         mount.lastMaterializedBranchId = store.currentBranch().id;
       }
     }
@@ -2349,11 +2366,14 @@ export class WorkspaceModule implements Module {
     mount.lastMaterializedSeq = 0;
 
     const watcher = this.watchers.get(mountName);
-    const written = await materializeToFs(store, mount);
+    // force: this path only runs after a deliberate undo/redo/branch switch
+    // on the framework's own _config mount — restoring disk to the branch
+    // state IS the operator intent, so the freshness guard yields.
+    const { written, unchanged } = await materializeToFs(store, mount, undefined, { force: true });
     for (const p of written) {
       watcher?.suppress(p);
     }
-    if (written.length > 0) {
+    if (written.length > 0 || unchanged.length > 0) {
       mount.lastMaterializedBranchId = store.currentBranch().id;
     }
     return written;

src/modules/workspace/sync.ts

@@ -119,6 +119,7 @@ export async function syncFromFs(
         store.treeRemove(mount.treeStateId, relativePath);
         result.synced.push({ path: relativePath, op: 'deleted' });
       }
+      mount.materializedHashes.delete(relativePath);
       continue;
     }
 
@@ -153,7 +154,8 @@ export async function syncFromFs(
       const existing = store.treeGet(mount.treeStateId, relativePath);
 
       if (existing && existing.blobHash === hash) {
-        // No change
+        // No change. Disk and tree agree, so this is the baseline now.
+        mount.materializedHashes.set(relativePath, hashContent(buffer));
         continue;
       }
 
@@ -181,6 +183,12 @@ export async function syncFromFs(
       }
 
       store.treeSet(mount.treeStateId, relativePath, entry);
+      // The tree adopted the disk copy, so disk and tree agree: re-pin the
+      // baseline. Left at the old hash, materialize would read the adopted
+      // edit as a fresh external change and refuse every later agent edit
+      // to this file, with no sync able to clear it. Raw disk bytes, which is
+      // what materializeToFs compares against.
+      mount.materializedHashes.set(relativePath, hashContent(buffer));
       result.synced.push({ path: relativePath, op: existing ? 'modified' : 'created' });
     } catch (error) {
       result.skipped.push({
@@ -193,24 +201,55 @@ export async function syncFromFs(
   return result;
 }
 
+export interface MaterializeResult {
+  /** Paths actually written to disk */
+  written: string[];
+  /** Paths whose disk copy already held exactly the tree's bytes: nothing
+   *  written, but disk reflects the current tree for them all the same. */
+  unchanged: string[];
+  /** Paths refused by the freshness guard, each with the reason — a silently
+   *  overwritten shell edit is unrecoverable, so divergence must surface
+   *  instead of being resolved toward the workspace copy (issue #109: a bulk
+   *  materialize reverted 217 shell-edited files to a stale snapshot). */
+  skipped: SkippedFile[];
+}
+
 /**
  * Materialize Chronicle tree state to filesystem.
  *
+ * Freshness guard (#109): files get modified by two hands — this layer AND
+ * direct shell/tool edits. Before overwriting, the disk content's hash is
+ * compared against `materializedHashes` (the hash this layer last wrote): a
+ * mismatch means another writer changed the file since, and the path is
+ * refused loudly instead of silently reverted, unless `force`. Symmetric
+ * with syncFromFs's conflict detection, and with the same blind spot: no
+ * baseline (never materialized, or synced) means no guard. Baselines and
+ * refused paths persist across restarts with the module state.
+ *
+ * Refused paths are kept in `mount.refusedPaths` and retried by every later
+ * materialize, so the diff watermark can still advance: holding it back
+ * would understate what disk holds to the branch guard, which reads the
+ * same sequence.
+ *
  * @param store Chronicle store
  * @param mount Mount state
  * @param paths Specific paths to materialize. If undefined, materializes all changed since last.
- * @returns List of paths that were written
+ * @param opts force: overwrite even where the disk copy changed under us.
+ * @returns Paths written, and paths the freshness guard refused
  */
 export async function materializeToFs(
   store: JsStore,
   mount: MountState,
   paths?: string[],
-): Promise<string[]> {
+  opts?: { force?: boolean },
+): Promise<MaterializeResult> {
   if (mount.config.mode === 'read-only') {
-    return [];
+    return { written: [], unchanged: [], skipped: [] };
   }
 
   const written: string[] = [];
+  const unchanged: string[] = [];
+  const skipped: SkippedFile[] = [];
 
   // Get changed files since last materialization
   const currentSeq = store.currentSequence();
@@ -255,13 +294,68 @@ export async function materializeToFs(
     }));
   }
 
+  // Paths an earlier materialize refused are still owed. A path the tree no
+  // longer has is owed nothing.
+  if (!paths) {
+    const queued = new Set(filesToMaterialize.map((f) => f.path));
+    for (const p of mount.refusedPaths) {
+      if (queued.has(p)) continue;
+      const entry = store.treeGet(mount.treeStateId, p);
+      if (entry) filesToMaterialize.push({ path: p, blobHash: entry.blobHash });
+      else mount.refusedPaths.delete(p);
+    }
+  }
+
   for (const { path: relativePath, blobHash } of filesToMaterialize) {
     const absolutePath = safePath(mount.config.path, relativePath);
     if (!absolutePath) continue; // Path traversal — skip silently
 
     const blob = store.getBlob(blobHash);
     if (!blob) continue;
 
+    let diskBuffer: Buffer | null = null;
+    try {
+      diskBuffer = await readFile(absolutePath);
+    } catch (err) {
+      // Only absence proves there is nothing to overwrite. Any other read
+      // failure (EACCES on a write-only file, EISDIR, EIO...) means freshness
+      // cannot be verified, and writeFile may still succeed — so refuse
+      // rather than treat "unreadable" as "not there".
+      const code = (err as NodeJS.ErrnoException).code;
+      if (code !== 'ENOENT' && !opts?.force) {
+        skipped.push({
+          path: relativePath,
+          reason:
+            `cannot verify disk copy (${code ?? (err as Error).message}) — ` +
+            'fix its permissions and materialize again, or pass force to overwrite it',
+        });
+        mount.refusedPaths.add(relativePath);
+        continue;
+      }
+    }
+    if (diskBuffer) {
+      const diskHash = hashContent(diskBuffer);
+      if (diskHash === blobHash) {
+        // Disk already holds exactly these bytes: re-pin the baseline and
+        // leave the file (and its mtime) alone.
+        mount.materializedHashes.set(relativePath, blobHash);
+        mount.refusedPaths.delete(relativePath);
+        unchanged.push(relativePath);
+        continue;
+      }
+      const baselineHash = mount.materializedHashes.get(relativePath);
+      if (!opts?.force && baselineHash !== undefined && diskHash !== baselineHash) {
+        skipped.push({
+          path: relativePath,
+          reason:
+            'stale copy: disk changed since last materialize (another writer) — ' +
+            'sync first to adopt the disk version, or pass force to overwrite it',
+        });
+        mount.refusedPaths.add(relativePath);
+        continue;
+      }
+    }
+
     // Create parent directories
     await mkdir(dirname(absolutePath), { recursive: true });
 
@@ -271,11 +365,12 @@ export async function materializeToFs(
 
     // Record blob hash at materialization time for conflict detection
     mount.materializedHashes.set(relativePath, blobHash);
+    mount.refusedPaths.delete(relativePath);
   }
 
   mount.lastMaterializedSeq = currentSeq;
 
-  return written;
+  return { written, unchanged, skipped };
 }
 
 /**

src/modules/workspace/types.ts

@@ -96,6 +96,9 @@ export interface MountState {
   lastMaterializedBranchId: string | null;
   /** Per-file hash at time of last materialization — baseline for conflict detection */
   materializedHashes: Map<string, string>;
+  /** Paths the materialize freshness guard refused (#109), owed to every
+   *  later materialize until one writes them or the tree drops them. */
+  refusedPaths: Set<string>;
   /**
    * Wall-clock time chokidar emitted `ready` for this mount, or null if the
    * watcher hasn't finished its initial scan. null after session start =
@@ -115,6 +118,9 @@ export interface WorkspaceModuleState {
   mounts: Record<string, {
     lastMaterializedSeq: number;
     lastMaterializedBranchId?: string;
+    /** Freshness-guard baselines (#109), so a restart doesn't drop the guard */
+    materializedHashes?: Record<string, string>;
+    refusedPaths?: string[];
     watcherReadyAt?: number | null;
     watcherError?: string | null;
   }>;