fix(workspace): refusals and the edit's conflict note say what a sync takes from disk, a deletion included

changelog.d/workspace-deleted-on-disk-advice.fixed.md

@@ -0,0 +1 @@
+- Where disk has deleted a file the workspace still holds, the workspace's ways out say what they do there. A push's refusal for that path, and an edit's note on it, now say that a sync of the path takes disk's deletion, which removes the workspace's copy, and that a forced materialize writes the workspace's copy back. Where the path's branch intent can't be read, the refusal still names the sync alone, and now says it takes the deletion. Before, they offered to "take disk's copy" or "adopt the disk version" and to overwrite a disk copy that wasn't there, so an agent that synced to take disk's copy lost the workspace's copy instead. A conflict recorded before disk deleted the file keeps its kind, and its refusal now says disk has no copy of the file. Where disk does hold a copy, an edit's conflict note now says that a sync keeps disk's copy and gives up the workspace's, which holds whether or not the workspace can store disk's copy: one it can't store, binary or past the size limit, stays on disk alone.

src/modules/workspace/index.ts

@@ -2348,8 +2348,15 @@ export class WorkspaceModule implements Module {
         // (below). A stored edit that couldn't finish leaves it to the push's
         // reason, which names the conflict, or to ls, which its answer points to.
         const conflict = report?.state === 'conflict' ? report.conflict?.kind ?? 'both-changed' : null;
+        // Where disk has no copy now, whatever the conflict's kind, a sync of
+        // the path takes disk's deletion and a forced materialize writes the
+        // workspace's copy back. Where it has one, a sync keeps it and gives
+        // up the workspace's, whether or not the workspace can hold disk's
+        // copy: one it can't hold stays on disk alone.
         const conflictNote = (where: string): string | null => conflict === null ? null
-          : `The path is in conflict with disk (${conflict}): the edit ${where} the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's.`;
+          : `The path is in conflict with disk (${conflict}): the edit ${where} the workspace's copy. ` + (report?.diskNow === 'absent'
+            ? "Sync the path to take disk's deletion, which removes the workspace's copy, or materialize it with force to write the workspace's copy back to disk."
+            : "Sync the path to keep disk's copy and give up the workspace's, or materialize it with force to write the workspace's over disk's.");
         const suffix = [whichCopy('looked in'), conflictNote('looked in')].filter((n) => n !== null).map((n) => ` ${n}`).join('');
         appliedNote = whichCopy('applied to');
 

src/modules/workspace/reconcile.ts

@@ -1064,7 +1064,13 @@ export async function pushPaths(
     // The ways out a refusal names, as materialize takes them: a deletion
     // reaches disk only with applyDeletions.
     const again = s ? 'materialize again' : 'materialize again with applyDeletions';
-    const forced = s ? 'materialize it with force to overwrite it' : 'materialize it with force and applyDeletions to delete it';
+    // Where disk has deleted a file the workspace holds, a sync takes the
+    // deletion, which removes the workspace's copy, and a forced push writes
+    // the workspace's copy back: there is no disk copy to adopt or overwrite.
+    const gone = s !== null && d.kind === 'absent';
+    const forced = !s ? 'materialize it with force and applyDeletions to delete it'
+      : gone ? "materialize it with force to write the workspace's copy back" : 'materialize it with force to overwrite it';
+    const takeDisk = gone ? "sync this path to take disk's deletion, which removes the workspace's copy" : 'sync this path to adopt the disk version';
     // What the effect may find: what this observation saw, unless forced.
     const expect: Expect = opts.force || d.kind === 'unobserved' || d.kind === 'unhashed'
       ? { kind: 'any' }
@@ -1078,7 +1084,7 @@ export async function pushPaths(
         result.skipped.push({
           path,
           reason: 'stale copy: disk changed since it last agreed with the workspace, which did not — ' +
-            `sync this path to adopt the disk version, or ${forced}`,
+            `${takeDisk}, or ${forced}`,
         });
         continue;
       }
@@ -1115,8 +1121,11 @@ export async function pushPaths(
         // replaces it (decide()), and a sync replaces it only where it sees
         // disk's copy, so a copy it can't see is named first. For the rest,
         // fixing what the note names holds, and forcedWay says where force
-        // does too.
-        const sync = 'sync this path to adopt the disk version, which replaces the intent';
+        // does too. Where disk has deleted the file, that sync takes the
+        // deletion.
+        const sync = gone
+          ? "sync this path to take disk's deletion, which removes the workspace's copy and replaces the intent"
+          : 'sync this path to adopt the disk version, which replaces the intent';
         result.skipped.push({
           path,
           reason: bi?.unreadable !== undefined
@@ -1144,7 +1153,12 @@ export async function pushPaths(
       }
       result.skipped.push({
         path,
-        reason: noEvidence
+        // Whatever was recorded, a disk with no copy now is answered for what
+        // it holds; a path with no evidence still needs its push named.
+        reason: gone
+          ? `disk has no copy of this file (conflict: ${kind}) — ${takeDisk}, or ` +
+            `materialize ${noEvidence ? 'this path' : 'it'} with force to write the workspace's copy back`
+          : noEvidence
           ? `disk holds a different copy, and nothing records which is newer (conflict: ${kind}) — ` +
             `sync this path to adopt the disk version, or materialize this path with force ${s ? 'to overwrite it' : 'and applyDeletions to delete it'}`
           : kind === 'interrupted'

test/workspace-file-tools.test.ts

@@ -11,13 +11,16 @@ import { JsStore } from '@animalabs/chronicle';
 import { WorkspaceModule } from '../src/modules/workspace/index.js';
 import type { ToolResult } from '../src/types/events.js';
 
-function setup(t: TestContext, opts: { autoMaterialize?: boolean } = {}) {
+function setup(t: TestContext, opts: { autoMaterialize?: boolean; maxFileSize?: number } = {}) {
   const root = mkdtempSync(join(tmpdir(), 'workspace-file-tools-'));
   const dir = join(root, 'mount');
   mkdirSync(dir);
   const store = JsStore.openOrCreate({ path: join(root, 'store') });
   const workspace = new WorkspaceModule({
-    mounts: [{ name: 'work', path: dir, mode: 'read-write', watch: 'never', autoMaterialize: opts.autoMaterialize ?? false }],
+    mounts: [{
+      name: 'work', path: dir, mode: 'read-write', watch: 'never', autoMaterialize: opts.autoMaterialize ?? false,
+      ...(opts.maxFileSize !== undefined ? { maxFileSize: opts.maxFileSize } : {}),
+    }],
   });
   workspace.initStore(store);
   t.after(() => { store.close(); rmSync(root, { recursive: true, force: true }); });
@@ -265,13 +268,13 @@ test('an edit of a draft whose disk copy also changed applies to the draft and n
 
   // A miss says so too: the text the agent sought may be disk's.
   assert.equal(errorOf(await call('edit', { path: 'work/k.txt', oldString: 'another', newString: 'x' })),
-    "String not found in work/k.txt. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's.");
+    "String not found in work/k.txt. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. Sync the path to keep disk's copy and give up the workspace's, or materialize it with force to write the workspace's over disk's.");
   assert.equal(errorOf(await call('edit', { path: 'work/k.txt', oldString: 'a', newString: 'x' })),
-    "String found 2 times in work/k.txt. Use replaceAll: true or provide more context. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's.");
+    "String found 2 times in work/k.txt. Use replaceAll: true or provide more context. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. Sync the path to keep disk's copy and give up the workspace's, or materialize it with force to write the workspace's over disk's.");
   const edit = await call('edit', { path: 'work/k.txt', oldString: 'draft', newString: 'draft, edited' });
   assert.equal(edit.success, true, edit.error);
   assert.equal((edit.data as { note: string }).note,
-    "The path is in conflict with disk (both-changed): the edit is in the workspace's copy. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's.");
+    "The path is in conflict with disk (both-changed): the edit is in the workspace's copy. Sync the path to keep disk's copy and give up the workspace's, or materialize it with force to write the workspace's over disk's.");
   assert.equal(await stored('k.txt'), 'agent draft, edited\n');
   assert.equal(disk('k.txt')!.toString(), 'another writer\n');
 });
@@ -284,7 +287,7 @@ test("an edit refused for a workspace copy that isn't valid UTF-8 names the path
   store.treeSet('workspace/work/tree', 'c.txt', { blobHash: store.storeBlob(latin1, 'text/plain'), size: latin1.byteLength, mode: 0o644 });
   writeFileSync(join(dir, 'c.txt'), 'disk copy\n');
   assert.match(errorOf(await call('edit', { path: 'work/c.txt', oldString: 'caf', newString: 'tea' })),
-    /^The edit did nothing: work\/c\.txt isn't valid UTF-8, .* The path is in conflict with disk \(unknown-provenance\): the edit looked in the workspace's copy\. Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's\.$/);
+    /^The edit did nothing: work\/c\.txt isn't valid UTF-8, .* The path is in conflict with disk \(unknown-provenance\): the edit looked in the workspace's copy\. Sync the path to keep disk's copy and give up the workspace's, or materialize it with force to write the workspace's over disk's\.$/);
 });
 
 // ---------------------------------------------------------------------------
@@ -586,11 +589,15 @@ test("a path whose branch intent can't be read is told to sync the path, which r
   assert.match(JSON.stringify(blind.data), /"path":"w\.txt","reason":"disk could not be checked: a symlink, which this mount does not follow"/);
   assert.match(JSON.stringify((await call('materialize', { path: 'work/w.txt' })).data), unseen, "a sync that can't see disk's copy leaves the intent");
   rmSync(join(dir, 'w.txt'));
-  // Absent is a copy a sync sees, so the reason names the sync alone.
+  // Absent is a copy a sync sees, so the reason names the sync alone, and
+  // says that it takes disk's deletion.
   assert.match(JSON.stringify((await call('materialize', { path: 'work/w.txt' })).data),
-    /w\.txt: its branch intent can't be read: its record is not a branch intent — sync this path to adopt the disk version, which replaces the intent"/);
+    /w\.txt: its branch intent can't be read: its record is not a branch intent — sync this path to take disk's deletion, which removes the workspace's copy and replaces the intent"/);
   const seen = await call('sync', { path: 'work/w.txt' });
   assert.equal(seen.success, true, seen.error);
+  const removed = await call('read', { path: 'work/w.txt' });
+  assert.equal(removed.success, false, "the sync took disk's deletion");
+  assert.equal(removed.error, 'File not found: work/w.txt');
   assert.doesNotMatch(JSON.stringify((await call('materialize', { path: 'work/w.txt' })).data), /branch intent/, 'once it sees disk, the sync replaces it');
 });
 
@@ -630,3 +637,106 @@ test("an edit of a file disk lacks whose push is refused doesn't say the file we
   assert.equal(await stored('f.txt/l.txt'), 'edited line\n');
   assert.equal(disk('f.txt')!.toString(), 'file\n', "the refused push left disk's file as it was");
 });
+
+// ---------------------------------------------------------------------------
+// Where disk has deleted the file, a sync takes the deletion
+// ---------------------------------------------------------------------------
+
+const DELETION_WAYS = "Sync the path to take disk's deletion, which removes the workspace's copy, or materialize it with force to write the workspace's copy back to disk.";
+
+test("an edit of a draft whose disk copy was deleted says what a sync and a forced materialize do", async (t) => {
+  const { dir, call, disk } = setup(t);
+  for (const name of ['s.txt', 'f.txt']) {
+    writeFileSync(join(dir, name), 'base\n');
+    assert.equal((await call('read', { path: `work/${name}` })).success, true);
+    assert.equal((await call('write', { path: `work/${name}`, content: 'agent draft\n' })).success, true);
+    rmSync(join(dir, name));
+  }
+  assert.equal(errorOf(await call('edit', { path: 'work/s.txt', oldString: 'missing', newString: 'x' })),
+    `String not found in work/s.txt. The path is in conflict with disk (deleted-on-disk): the edit looked in the workspace's copy. ${DELETION_WAYS}`);
+  const edit = await call('edit', { path: 'work/f.txt', oldString: 'draft', newString: 'draft, edited' });
+  assert.equal(edit.success, true, edit.error);
+  assert.equal((edit.data as { note?: string }).note,
+    `The path is in conflict with disk (deleted-on-disk): the edit is in the workspace's copy. ${DELETION_WAYS}`);
+  // Each way out does what the note says.
+  assert.equal((await call('sync', { path: 'work/s.txt' })).success, true);
+  assert.equal((await call('read', { path: 'work/s.txt' })).success, false, "the sync took disk's deletion");
+  const forced = await call('materialize', { path: 'work/f.txt', force: true });
+  assert.equal(forced.success, true, forced.error);
+  assert.equal(disk('f.txt')!.toString(), 'agent draft, edited\n', "the forced materialize wrote the workspace's copy back");
+});
+
+test('a recorded conflict whose disk copy has since been deleted is answered for what disk holds now', async (t) => {
+  const { store, dir, call, disk } = setup(t);
+  writeFileSync(join(dir, 'k.txt'), 'base\n');
+  assert.equal((await call('read', { path: 'work/k.txt' })).success, true);
+  assert.equal((await call('write', { path: 'work/k.txt', content: 'agent draft\n' })).success, true);
+  writeFileSync(join(dir, 'k.txt'), 'another writer\n');
+  // The edit's pass records the conflict as both-changed.
+  assert.match(errorOf(await call('edit', { path: 'work/k.txt', oldString: 'missing', newString: 'x' })),
+    / \(both-changed\): the edit looked in the workspace's copy\. Sync the path to keep disk's copy and give up the workspace's/);
+  rmSync(join(dir, 'k.txt'));
+  assert.equal(errorOf(await call('edit', { path: 'work/k.txt', oldString: 'missing', newString: 'x' })),
+    `String not found in work/k.txt. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. ${DELETION_WAYS}`);
+  assert.match(JSON.stringify((await call('materialize', { path: 'work/k.txt' })).data),
+    /k\.txt: disk has no copy of this file \(conflict: both-changed\) — sync this path to take disk's deletion, which removes the workspace's copy, or materialize it with force to write the workspace's copy back"/);
+
+  // A conflict with nothing to say which side is newer still needs its push
+  // named: an entry from before the evidence, beside a different disk copy.
+  const bytes = Buffer.from('legacy line\n');
+  store.treeSet('workspace/work/tree', 'n.txt', { blobHash: store.storeBlob(bytes, 'text/plain'), size: bytes.byteLength, mode: 0o644 });
+  writeFileSync(join(dir, 'n.txt'), 'disk copy\n');
+  assert.match(errorOf(await call('edit', { path: 'work/n.txt', oldString: 'missing', newString: 'x' })), / \(unknown-provenance\): /);
+  rmSync(join(dir, 'n.txt'));
+  assert.match(JSON.stringify((await call('materialize', { path: 'work/n.txt' })).data),
+    /n\.txt: disk has no copy of this file \(conflict: unknown-provenance\) — sync this path to take disk's deletion, which removes the workspace's copy, or materialize this path with force to write the workspace's copy back"/);
+  const forced = await call('materialize', { path: 'work/n.txt', force: true });
+  assert.equal(forced.success, true, forced.error);
+  assert.equal(disk('n.txt')!.toString(), 'legacy line\n', "the forced materialize of the named path wrote the workspace's copy back");
+});
+
+test("a push where disk has deleted the file says what a sync and a forced materialize do", async (t) => {
+  const { dir, call, disk, stored } = setup(t, { autoMaterialize: true });
+  // Disk deleted a file the workspace holds unchanged.
+  writeFileSync(join(dir, 'u.txt'), 'v1\n');
+  assert.equal((await call('read', { path: 'work/u.txt' })).success, true);
+  rmSync(join(dir, 'u.txt'));
+  assert.match(JSON.stringify((await call('materialize', { path: 'work/u.txt' })).data),
+    /u\.txt: stale copy: disk changed since it last agreed with the workspace, which did not — sync this path to take disk's deletion, which removes the workspace's copy, or materialize it with force to write the workspace's copy back"/);
+  // A write over a file disk deleted since the workspace read it.
+  writeFileSync(join(dir, 'w.txt'), 'v1\n');
+  assert.equal((await call('read', { path: 'work/w.txt' })).success, true);
+  rmSync(join(dir, 'w.txt'));
+  assert.equal(errorOf(await call('write', { path: 'work/w.txt', content: 'v2\n' })),
+    'The write of "work/w.txt" is stored in the workspace, so don\'t send it again. '
+    + "Its push to disk didn't complete: disk has no copy of this file (conflict: deleted-on-disk) — sync this path to take disk's deletion, which removes the workspace's copy, or materialize it with force to write the workspace's copy back.");
+  assert.equal(await stored('w.txt'), 'v2\n');
+  const forced = await call('materialize', { path: 'work/w.txt', force: true });
+  assert.equal(forced.success, true, forced.error);
+  assert.equal(disk('w.txt')!.toString(), 'v2\n', "the forced materialize wrote the workspace's copy back");
+});
+
+// The workspace doesn't store a disk copy with a null byte (binary), or one
+// past the mount's maxFileSize.
+for (const [kind, limit, copy] of [
+  ['binary', undefined, Buffer.from('bin\0ary\n')],
+  ['past the size limit', 16, Buffer.from(`${'x'.repeat(40)}\n`)],
+] as const) {
+  test(`an edit of a draft whose disk copy the workspace can't hold (${kind}) says a sync keeps disk's copy and gives up the workspace's`, async (t) => {
+    const { dir, call, disk } = setup(t, { maxFileSize: limit });
+    writeFileSync(join(dir, 'b.txt'), 'base\n');
+    assert.equal((await call('read', { path: 'work/b.txt' })).success, true);
+    assert.equal((await call('write', { path: 'work/b.txt', content: 'agent draft\n' })).success, true);
+    writeFileSync(join(dir, 'b.txt'), copy);
+    assert.equal(errorOf(await call('edit', { path: 'work/b.txt', oldString: 'missing', newString: 'x' })),
+      "String not found in work/b.txt. The path is in conflict with disk (both-changed): the edit looked in the workspace's copy. "
+      + "Sync the path to keep disk's copy and give up the workspace's, or materialize it with force to write the workspace's over disk's.");
+    assert.equal((await call('sync', { path: 'work/b.txt' })).success, true);
+    const after = await call('read', { path: 'work/b.txt' });
+    assert.equal(after.success, false, "the sync gave up the workspace's copy");
+    assert.equal(after.error, 'work/b.txt is on disk but not stored in the workspace (binary or over the size limit).');
+    const listed = (await call('ls', { path: 'work' })).data as { entries: Array<{ name: string; state: string }> };
+    assert.equal(listed.entries.find((e) => e.name === 'b.txt')?.state, 'disk-only', 'the listing shows it disk-only');
+    assert.deepEqual(disk('b.txt'), copy, 'disk keeps its copy');
+  });
+}