From f1585c645e003d848ca2378e1f54219a3537da2f Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Wed, 21 Oct 2026 14:43:04 -0400 Subject: [PATCH 1/4] fix(workspace): a file the workspace doesn't hold, under a pass that decided nothing, is unchecked, not "File not found" A disk pass decides nothing, and reports no path, when the selected branch kept changing while it gathered, or when a sync's branch was no longer selected. Its result then carries no branchId, and the region it leaves incomplete says why. delete, readBinary, edit and edit_many each looked at the pass's report for a path the workspace doesn't hold, found none, and answered "File not found", though disk may hold the file: the same call made again found it (#552). Each now says that disk couldn't be checked, with the pass's reason, and to try again: - delete and readBinary: " is not in the workspace, and disk could not be checked (). Try again to check it." - edit and edit_many: " couldn't be checked against disk (), so the edit changed nothing. Try again to check it.", per file for edit_many, which stores none of its edits. undecidedOf reads the pass's missing branchId, which only those two early returns leave, so a region left incomplete beside a pass's reports doesn't count. ensureSynced now answers it beside the report. read's own answer is unchanged here. Co-Authored-By: Claude Opus 5.5 --- changelog.d/552-pass-decided-nothing.fixed.md | 7 +++ src/modules/workspace/index.ts | 46 +++++++++++++---- test/workspace-three-way.test.ts | 49 +++++++++++++++++++ 3 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 changelog.d/552-pass-decided-nothing.fixed.md diff --git a/changelog.d/552-pass-decided-nothing.fixed.md b/changelog.d/552-pass-decided-nothing.fixed.md new file mode 100644 index 000000000..444983688 --- /dev/null +++ b/changelog.d/552-pass-decided-nothing.fixed.md @@ -0,0 +1,7 @@ +- The workspace's `delete`, `edit` and `edit_many`, and its `readBinary`, + which the framework's `read_image` reads through, no longer answer "File + not found" for a path the workspace doesn't hold when the check of disk + before them decided nothing, because the selected branch kept changing + while it ran. Each now says that disk couldn't be checked, with that + reason, and to try again. Before, disk might hold the file, and the same + call made again found it (#552). diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index ea740350e..20c75fbb3 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -268,6 +268,16 @@ export interface ReadFileFromDiskOptions { maxBytes?: number; } +/** + * Why a pass decided nothing, or null where it decided its paths. A pass gives + * up deciding, and reports no path, when the selected branch kept changing + * while it gathered, or when a sync's branch was no longer selected; it then + * carries no branchId, and the region it leaves incomplete says why. + */ +function undecidedOf(pass: PassResult): string | null { + return pass.branchId === undefined ? (pass.incomplete[0]?.reason ?? 'the pass decided nothing') : null; +} + function errnoOf(err: unknown): string | undefined { return typeof err === 'object' && err !== null && 'code' in err && typeof (err as { code?: unknown }).code === 'string' ? (err as { code: string }).code @@ -2284,9 +2294,9 @@ export class WorkspaceModule implements Module { if (mount.direct) return this.bytesFromDisk(mount, relativePath, mountPrefixedPath, mount.config.maxFileSize ?? DEFAULT_MAX_FILE_SIZE); const store = this.getStore(); return this.withMount(mount, async () => { - await this.ensureSynced(mount, relativePath); + const { undecided } = await this.ensureSynced(mount, relativePath); const entry = store.treeGet(mount.treeStateId, relativePath); - if (!entry) return { error: `File not found: ${mountPrefixedPath}` }; + if (!entry) return { error: undecided !== null ? this.notCheckedYet(mountPrefixedPath, undecided) : `File not found: ${mountPrefixedPath}` }; const blob = store.getBlob(entry.blobHash); if (!blob) return { error: `Blob not found for: ${mountPrefixedPath}` }; return { data: blob }; @@ -3676,13 +3686,24 @@ export class WorkspaceModule implements Module { * path, as a file tool names the one it acts on: a path the mount passes * over by design is observed, as a sync naming it would observe it (#276), * while a path in another mount's directory is refused before this, and - * passed over by any pass. The caller holds the mount's turn. + * passed over by any pass. Answers the pass's report on the path, and why + * the pass decided nothing where it did (undecidedOf), which leaves disk + * unchecked. The caller holds the mount's turn. */ - private async ensureSynced(mount: MountState, relativePath: string): Promise { + private async ensureSynced(mount: MountState, relativePath: string): Promise<{ report: PathReport | undefined; undecided: string | null }> { const store = this.getStore(); - if (store.treeGet(mount.treeStateId, relativePath)) return undefined; + if (store.treeGet(mount.treeStateId, relativePath)) return { report: undefined, undecided: null }; const pass = await this.passUnlocked(mount, { kind: 'paths', paths: [relativePath] }, { named: true }); - return pass.reports.get(relativePath); + return { report: pass.reports.get(relativePath), undecided: undecidedOf(pass) }; + } + + /** + * Why the workspace holds no entry at a path, after a pass over it that + * decided nothing: disk wasn't checked, so it may have a copy, and the same + * call made again checks it. + */ + private notCheckedYet(path: string, reason: string): string { + return `${this.notInWorkspace(path, { path, state: 'unverified', note: reason })} Try again to check it.`; } /** Why the workspace holds no entry at a path, when disk explains it. */ @@ -3715,7 +3736,7 @@ export class WorkspaceModule implements Module { if (mount.direct) return this.bytesOnDisk(mount, relativePath, path); const store = this.getStore(); return this.withMount(mount, async () => { - const report = await this.ensureSynced(mount, relativePath); + const { report } = await this.ensureSynced(mount, relativePath); const entry = store.treeGet(mount.treeStateId, relativePath); if (!entry) return { error: this.notInWorkspace(path, report) }; const blob = store.getBlob(entry.blobHash); @@ -4086,6 +4107,8 @@ export class WorkspaceModule implements Module { // As every file tool's pass does, it names the paths it acts on, so a // file the mount passes over by design is observed (see PassOptions.named). const pass = await this.passUnlocked(mount, { kind: 'paths', paths: [...targets.values()].map((target) => target.relativePath) }, { named: true }); + // A pass that decided nothing checked no file's disk copy. + const undecided = undecidedOf(pass); // Nothing below awaits until the commit's push. const maxSize = mount.config.maxFileSize ?? DEFAULT_MAX_FILE_SIZE; const hints = hintBudget(); @@ -4154,7 +4177,9 @@ export class WorkspaceModule implements Module { // the disk-only answer describes; any other drop is disk's deletion. const text = took('deleted') && report?.state !== 'disk-only' ? `File not found: ${target.path}. Disk's copy was deleted since the workspace last agreed with it, and that was taken in first.` - : this.notInWorkspace(target.path, report); + : undecided !== null + ? `${target.path} couldn't be checked against disk (${undecided}), so ${subject} changed nothing. Try again to check it.` + : this.notInWorkspace(target.path, report); problems.push({ kind: 'file', edits: target.edits, path: target.path, text, whole: true }); continue; } @@ -4378,11 +4403,12 @@ export class WorkspaceModule implements Module { let committed = false; try { return await this.withMount(mount, async (): Promise => { - const report = await this.ensureSynced(mount, relativePath); + const { report, undecided } = await this.ensureSynced(mount, relativePath); await this.settleBeforeMutation(mount, relativePath); const entry = store.treeGet(mount.treeStateId, relativePath); if (!entry) { - return { success: false, error: this.notInWorkspace(input.path, report), isError: true }; + const error = undecided !== null ? this.notCheckedYet(input.path, undecided) : this.notInWorkspace(input.path, report); + return { success: false, error, isError: true }; } const { refused, note } = await this.commitToolChange(mount, relativePath, { kind: 'remove', previousHash: entry.blobHash }, diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 0d8de884d..575c8b6f1 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -7028,6 +7028,55 @@ describe('on-agent-action', () => { assert.deepEqual(data.incomplete, [{ path: '', reason: 'the selected branch kept changing while the scan ran' }]); }); + /** + * Run `fn` with the selected branch changing during each of a pass's + * gathers, so the pass decides nothing, and then no more: each file a gather + * reads switches the branch, three times in all, as many as a pass gathers. + */ + const whileBranchKeepsChanging = (env: Env, fn: () => Promise): Promise => { + const main = env.store.currentBranch().name; + env.store.createBranch('other', main); + return withReadHook((n) => { if (n <= 3) env.store.switchBranch(n % 2 === 1 ? 'other' : main); }, fn); + }; + const KEPT_CHANGING = 'the selected branch kept changing while the scan ran'; + + test('a file the workspace doesn\'t hold, under a pass that decided nothing, is answered as unchecked by delete, readBinary, edit and edit_many, not as not found', async (t) => { + const answers: Record = {}; + for (const tool of ['delete', 'readBinary', 'edit', 'edit_many'] as const) { + const env = new Env(t); + const m = await env.open(); + env.writeDisk('new.txt', 'hello on disk\n'); + answers[tool] = await whileBranchKeepsChanging(env, async () => { + if (tool === 'readBinary') { + const read = await m.readBinary('work/new.txt'); + return 'error' in read ? read.error : `read ${read.data.length} bytes`; + } + const input = tool === 'delete' ? { path: 'work/new.txt' } + : tool === 'edit' ? { path: 'work/new.txt', oldString: 'hello', newString: 'bye' } + : { edits: [{ path: 'work/new.txt', oldString: 'hello', newString: 'bye' }] }; + const res = await m.handleToolCall({ id: 't', name: tool, input }); + assert.equal(res.success, false, JSON.stringify(res)); + return String(res.error); + }); + // Disk's file is untouched, and the same call, made again, finds it. + assert.equal(env.readDisk('new.txt'), 'hello on disk\n'); + if (tool === 'readBinary') { + const again = await m.readBinary('work/new.txt'); + assert.ok('data' in again && again.data.toString() === 'hello on disk\n', JSON.stringify(again)); + } else { + assert.equal(await contentOf(m, 'new.txt'), 'hello on disk\n'); + } + } + const unchecked = `work/new.txt is not in the workspace, and disk could not be checked (${KEPT_CHANGING}). Try again to check it.`; + assert.deepEqual(answers, { + delete: unchecked, + readBinary: unchecked, + edit: `work/new.txt couldn't be checked against disk (${KEPT_CHANGING}), so the edit changed nothing. Try again to check it.`, + edit_many: 'The edit_many call stored none of its edits: 1 problem.\n\n' + + `edit 1: work/new.txt couldn't be checked against disk (${KEPT_CHANGING}), so edit 1 changed nothing. Try again to check it.`, + }); + }); + test('a branch switched away during the root check and back during a read is decided on the branch enumerated', async (t) => { const env = new Env(t); const m = await env.open(); From fe9687b6e9f6add854ef9c55004eeb17c9161a55 Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Wed, 21 Oct 2026 14:59:05 -0400 Subject: [PATCH 2/4] fix(workspace): edit and edit_many refuse a file the workspace holds when their disk pass decided nothing An edit takes in a change on disk to a file the workspace holds, and applies to that text. When its pass decides nothing (the selected branch kept changing while it gathered), it took nothing in and checked no copy on disk, and edit and edit_many applied to the workspace's copy without a word: - where disk had changed, the edits applied to text disk had moved past. A mount that pushes then refused the push as a stale copy and left the path in a both-changed conflict; one that doesn't push answered with a plain success, and the conflict showed only in a later listing; - and they committed on whichever branch was selected once the pass gave up, which needn't be the one selected when the call began. Now they refuse every file the call names, held or not, as they already do one the workspace doesn't hold: " couldn't be checked against disk (), so the edit changed nothing. Try again to check it." A call made again checks disk first, as a pass that gave up because the branch kept changing usually passes. A copy disk couldn't be read, which the pass does decide, still gets the edits applied to the workspace's copy, with an answer that says so. Co-Authored-By: Claude Opus 5.5 --- .../552-pass-decided-nothing.changed.md | 8 ++++ src/modules/workspace/index.ts | 18 +++++++-- test/workspace-three-way.test.ts | 39 +++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 changelog.d/552-pass-decided-nothing.changed.md diff --git a/changelog.d/552-pass-decided-nothing.changed.md b/changelog.d/552-pass-decided-nothing.changed.md new file mode 100644 index 000000000..d805322cd --- /dev/null +++ b/changelog.d/552-pass-decided-nothing.changed.md @@ -0,0 +1,8 @@ +- The workspace's `edit` and `edit_many` refuse a file the workspace holds + when the check of disk before them decided nothing, because the selected + branch kept changing while it ran. They store nothing, and say so, with + that reason and to try again. Before, they applied to the workspace's copy + without a word, so where disk had changed they edited text disk had moved + past and left the path in conflict, and they committed on whichever + branch was selected when the check gave up. Where disk's copy couldn't be + read, an `edit` still applies to the workspace's copy and says so. diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 20c75fbb3..750ac7d15 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -4132,6 +4132,20 @@ export class WorkspaceModule implements Module { // workspace didn't hold has no older copy, so taking it in needs no // word. const subject = tool === 'edit' ? 'the edit' : editList(target.edits); + // A pass that decided nothing checked no copy on disk, held or not, + // and took nothing in, so the edits would apply to text disk may + // have moved past, on whichever branch is selected once it gave up. + // They're refused, as a call made again can check. + if (undecided !== null) { + problems.push({ + kind: 'file', + edits: target.edits, + path: target.path, + text: `${target.path} couldn't be checked against disk (${undecided}), so ${subject} changed nothing. Try again to check it.`, + whole: true, + }); + continue; + } const whichCopy = (verb: string): string | null => took('modified') ? `Disk's copy${tool === 'edit' ? '' : ` of ${target.path}`} had changed since the workspace last agreed with it, so it was taken in first, and ${subject} ${verb} that text.` : report?.state === 'unverified' @@ -4177,9 +4191,7 @@ export class WorkspaceModule implements Module { // the disk-only answer describes; any other drop is disk's deletion. const text = took('deleted') && report?.state !== 'disk-only' ? `File not found: ${target.path}. Disk's copy was deleted since the workspace last agreed with it, and that was taken in first.` - : undecided !== null - ? `${target.path} couldn't be checked against disk (${undecided}), so ${subject} changed nothing. Try again to check it.` - : this.notInWorkspace(target.path, report); + : this.notInWorkspace(target.path, report); problems.push({ kind: 'file', edits: target.edits, path: target.path, text, whole: true }); continue; } diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 575c8b6f1..7b41c7e4b 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -7077,6 +7077,45 @@ describe('on-agent-action', () => { }); }); + for (const autoMaterialize of [true, false]) { + test(`an edit of a file the workspace holds, under a pass that decided nothing, changes nothing on either branch or disk, and says to try again (${autoMaterialize ? 'a mount that pushes' : "one that doesn't push"})`, async (t) => { + for (const disk of ['unchanged', 'changed'] as const) { + const env = new Env(t); + const m = await env.open({ autoMaterialize }); + await seedSynced(env, m, 'a.txt', 'A line\n'); + const main = env.store.currentBranch().name; + if (disk === 'changed') env.writeDisk('a.txt', 'disk says A2\n'); + const edit = await whileBranchKeepsChanging(env, () => m.handleToolCall({ id: 't', name: 'edit', input: { path: 'work/a.txt', oldString: 'A line', newString: 'A edited' } })); + assert.equal(edit.success, false, JSON.stringify(edit)); + assert.equal(edit.error, `work/a.txt couldn't be checked against disk (${KEPT_CHANGING}), so the edit changed nothing. Try again to check it.`, disk); + // So does edit_many's, the branch left on `other` by the edit's pass + // changing under its three gathers in turn. + assert.equal(env.store.currentBranch().name, 'other'); + const many = await withReadHook((n) => { if (n <= 3) env.store.switchBranch(n % 2 === 1 ? main : 'other'); }, + () => m.handleToolCall({ id: 't', name: 'edit_many', input: { edits: [{ path: 'work/a.txt', oldString: 'A line', newString: 'A edited' }] } })); + assert.equal(many.success, false, JSON.stringify(many)); + assert.equal(many.error, 'The edit_many call stored none of its edits: 1 problem.\n\n' + + `edit 1: work/a.txt couldn't be checked against disk (${KEPT_CHANGING}), so edit 1 changed nothing. Try again to check it.`, disk); + // Disk is as it was, and each branch's workspace copy is the one the + // edits found, read from the store, which no pass changes. + const onDisk = disk === 'changed' ? 'disk says A2\n' : 'A line\n'; + assert.equal(env.readDisk('a.txt'), onDisk); + const treeStateId = (m as any).mounts.get('work').treeStateId as string; + for (const branch of [main, 'other']) { + env.store.switchBranch(branch); + const entry = env.store.treeGet(treeStateId, 'a.txt'); + assert.equal(entry && env.store.getBlob(entry.blobHash)?.toString(), 'A line\n', `${disk} on ${branch}`); + } + // Made again, the edit checks disk: it applies to the text disk holds, + // and where disk changed, that text no longer has the old line. + env.store.switchBranch(main); + const again = await m.handleToolCall({ id: 't', name: 'edit', input: { path: 'work/a.txt', oldString: 'A line', newString: 'A edited' } }); + if (disk === 'unchanged') assert.equal(again.success, true, JSON.stringify(again)); + else assert.match(String(again.error), /^The edit did nothing: oldString isn't in work\/a\.txt\./); + } + }); + } + test('a branch switched away during the root check and back during a read is decided on the branch enumerated', async (t) => { const env = new Env(t); const m = await env.open(); From d75531cb099056ebfb6b5e0b232a96bedd2fbb7b Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Wed, 21 Oct 2026 15:48:03 -0400 Subject: [PATCH 3/4] fix(workspace): readBinary answers a path it doesn't hold as delete does; tests and text from the review readBinary said "File not found" for every path the workspace doesn't hold, whatever the pass's report said, so a disk copy it couldn't read, a file the workspace deleted or one this branch lacks all read as missing. It now gives notInWorkspace's answer for the report, as delete does. The tests: - the test of a file the workspace doesn't hold now makes each refused call again and checks it succeeds, where it read the file instead; - the held-file tests date the file's mtime ahead (unvouched), so no fingerprint vouches and each of the pass's gathers reads it; - a test of edit_many naming two held files and one not held, each gather's reads selecting a branch made for it: all three are refused, per file, and no branch holds an edit; - readBinary and delete give the same answer for a file whose disk copy couldn't be read; - an edit under a pass that decided, leaving a region incomplete (a replaced mount root), applies to the workspace's copy and says so, which pins undecidedOf's reading of branchId over a region left incomplete. The changelog fragments no longer say the framework's read_image reads through readBinary: it calls the workspace's own read_image where the module offers one, as this one does. The .changed fragment names edit_many beside edit where disk's copy couldn't be read. Co-Authored-By: Claude Opus 5.5 --- .../552-pass-decided-nothing.changed.md | 3 +- changelog.d/552-pass-decided-nothing.fixed.md | 16 ++-- src/modules/workspace/index.ts | 4 +- test/workspace-three-way.test.ts | 77 ++++++++++++++++++- 4 files changed, 86 insertions(+), 14 deletions(-) diff --git a/changelog.d/552-pass-decided-nothing.changed.md b/changelog.d/552-pass-decided-nothing.changed.md index d805322cd..21f93ebda 100644 --- a/changelog.d/552-pass-decided-nothing.changed.md +++ b/changelog.d/552-pass-decided-nothing.changed.md @@ -5,4 +5,5 @@ without a word, so where disk had changed they edited text disk had moved past and left the path in conflict, and they committed on whichever branch was selected when the check gave up. Where disk's copy couldn't be - read, an `edit` still applies to the workspace's copy and says so. + read, an `edit` or `edit_many` still applies to the workspace's copy and + says so. diff --git a/changelog.d/552-pass-decided-nothing.fixed.md b/changelog.d/552-pass-decided-nothing.fixed.md index 444983688..a87c4c549 100644 --- a/changelog.d/552-pass-decided-nothing.fixed.md +++ b/changelog.d/552-pass-decided-nothing.fixed.md @@ -1,7 +1,9 @@ -- The workspace's `delete`, `edit` and `edit_many`, and its `readBinary`, - which the framework's `read_image` reads through, no longer answer "File - not found" for a path the workspace doesn't hold when the check of disk - before them decided nothing, because the selected branch kept changing - while it ran. Each now says that disk couldn't be checked, with that - reason, and to try again. Before, disk might hold the file, and the same - call made again found it (#552). +- The workspace's `delete`, `edit` and `edit_many`, and its `readBinary` + method, no longer answer "File not found" for a path the workspace doesn't + hold when the check of disk before them decided nothing, because the + selected branch kept changing while it ran. Each now says that disk + couldn't be checked, with that reason, and to try again. Before, disk might + hold the file, and the same call made again found it (#552). `readBinary` + also gives `delete`'s answer for every other path the workspace doesn't + hold, such as one whose disk copy couldn't be read, where it said "File not + found" whatever the reason. diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 750ac7d15..1de91d250 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -2294,9 +2294,9 @@ export class WorkspaceModule implements Module { if (mount.direct) return this.bytesFromDisk(mount, relativePath, mountPrefixedPath, mount.config.maxFileSize ?? DEFAULT_MAX_FILE_SIZE); const store = this.getStore(); return this.withMount(mount, async () => { - const { undecided } = await this.ensureSynced(mount, relativePath); + const { report, undecided } = await this.ensureSynced(mount, relativePath); const entry = store.treeGet(mount.treeStateId, relativePath); - if (!entry) return { error: undecided !== null ? this.notCheckedYet(mountPrefixedPath, undecided) : `File not found: ${mountPrefixedPath}` }; + if (!entry) return { error: undecided !== null ? this.notCheckedYet(mountPrefixedPath, undecided) : this.notInWorkspace(mountPrefixedPath, report) }; const blob = store.getBlob(entry.blobHash); if (!blob) return { error: `Blob not found for: ${mountPrefixedPath}` }; return { data: blob }; diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 7b41c7e4b..64cd43c99 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -7039,6 +7039,15 @@ describe('on-agent-action', () => { return withReadHook((n) => { if (n <= 3) env.store.switchBranch(n % 2 === 1 ? 'other' : main); }, fn); }; const KEPT_CHANGING = 'the selected branch kept changing while the scan ran'; + /** + * Date a file's mtime past now, so no fingerprint recorded for it vouches + * (fingerprintVouches needs the hash taken past the racy window after the + * change), and every gather of a pass reads it. + */ + const unvouched = (env: Env, path: string) => { + const later = new Date(Date.now() + 60_000); + utimesSync(env.disk(path), later, later); + }; test('a file the workspace doesn\'t hold, under a pass that decided nothing, is answered as unchecked by delete, readBinary, edit and edit_many, not as not found', async (t) => { const answers: Record = {}; @@ -7046,14 +7055,14 @@ describe('on-agent-action', () => { const env = new Env(t); const m = await env.open(); env.writeDisk('new.txt', 'hello on disk\n'); + const input = tool === 'delete' ? { path: 'work/new.txt' } + : tool === 'edit' ? { path: 'work/new.txt', oldString: 'hello', newString: 'bye' } + : { edits: [{ path: 'work/new.txt', oldString: 'hello', newString: 'bye' }] }; answers[tool] = await whileBranchKeepsChanging(env, async () => { if (tool === 'readBinary') { const read = await m.readBinary('work/new.txt'); return 'error' in read ? read.error : `read ${read.data.length} bytes`; } - const input = tool === 'delete' ? { path: 'work/new.txt' } - : tool === 'edit' ? { path: 'work/new.txt', oldString: 'hello', newString: 'bye' } - : { edits: [{ path: 'work/new.txt', oldString: 'hello', newString: 'bye' }] }; const res = await m.handleToolCall({ id: 't', name: tool, input }); assert.equal(res.success, false, JSON.stringify(res)); return String(res.error); @@ -7064,7 +7073,8 @@ describe('on-agent-action', () => { const again = await m.readBinary('work/new.txt'); assert.ok('data' in again && again.data.toString() === 'hello on disk\n', JSON.stringify(again)); } else { - assert.equal(await contentOf(m, 'new.txt'), 'hello on disk\n'); + const again = await m.handleToolCall({ id: 't', name: tool, input }); + assert.equal(again.success, true, `${tool} again: ${JSON.stringify(again)}`); } } const unchecked = `work/new.txt is not in the workspace, and disk could not be checked (${KEPT_CHANGING}). Try again to check it.`; @@ -7085,6 +7095,7 @@ describe('on-agent-action', () => { await seedSynced(env, m, 'a.txt', 'A line\n'); const main = env.store.currentBranch().name; if (disk === 'changed') env.writeDisk('a.txt', 'disk says A2\n'); + unvouched(env, 'a.txt'); const edit = await whileBranchKeepsChanging(env, () => m.handleToolCall({ id: 't', name: 'edit', input: { path: 'work/a.txt', oldString: 'A line', newString: 'A edited' } })); assert.equal(edit.success, false, JSON.stringify(edit)); assert.equal(edit.error, `work/a.txt couldn't be checked against disk (${KEPT_CHANGING}), so the edit changed nothing. Try again to check it.`, disk); @@ -7116,6 +7127,64 @@ describe('on-agent-action', () => { }); } + test('edit_many under a pass that decided nothing refuses every file it names, held or not, and stores nothing on any branch', async (t) => { + const env = new Env(t); + const m = await env.open({ autoMaterialize: true }); + await seedSynced(env, m, 'a.txt', 'A line\n'); + await seedSynced(env, m, 'b.txt', 'B line\n'); + env.writeDisk('new.txt', 'N line\n'); + for (const path of ['a.txt', 'b.txt', 'new.txt']) unvouched(env, path); + // Each file a gather reads selects a branch made for it, so every gather + // ends on another branch than it began on, however many files it reads. + const branches = [env.store.currentBranch().name]; + const many = await withReadHook((n) => { + const name = `churn-${n}`; + env.store.createBranch(name, env.store.currentBranch().name); + env.store.switchBranch(name); + branches.push(name); + }, () => m.handleToolCall({ id: 't', name: 'edit_many', input: { edits: [ + { path: 'work/a.txt', oldString: 'A line', newString: 'A edited' }, + { path: 'work/b.txt', oldString: 'B line', newString: 'B edited' }, + { path: 'work/new.txt', oldString: 'N line', newString: 'N edited' }, + ] } })); + assert.equal(many.success, false, JSON.stringify(many)); + const refused = (i: number, path: string) => `edit ${i}: work/${path} couldn't be checked against disk (${KEPT_CHANGING}), so edit ${i} changed nothing. Try again to check it.`; + assert.equal(many.error, ['The edit_many call stored none of its edits: 3 problems.', refused(1, 'a.txt'), refused(2, 'b.txt'), refused(3, 'new.txt')].join('\n\n')); + assert.deepEqual(['a.txt', 'b.txt', 'new.txt'].map((p) => env.readDisk(p)), ['A line\n', 'B line\n', 'N line\n']); + const treeStateId = (m as any).mounts.get('work').treeStateId as string; + for (const branch of branches) { + env.store.switchBranch(branch); + const text = (path: string) => { const entry = env.store.treeGet(treeStateId, path); return entry ? env.store.getBlob(entry.blobHash)?.toString() : null; }; + assert.deepEqual([text('a.txt'), text('b.txt'), text('new.txt')], ['A line\n', 'B line\n', null], branch); + } + }); + + test('readBinary and delete answer alike for a file the workspace doesn\'t hold whose disk copy couldn\'t be read', { skip: IS_ROOT ? 'root reads every file' : false }, async (t) => { + const env = new Env(t); + const m = await env.open(); + env.writeDisk('new.txt', 'hello on disk\n'); + chmodSync(env.disk('new.txt'), 0o000); + const read = await m.readBinary('work/new.txt'); + const deleted = await m.handleToolCall({ id: 't', name: 'delete', input: { path: 'work/new.txt' } }); + assert.ok('error' in read, JSON.stringify(read)); + assert.match(read.error, /^work\/new\.txt is not in the workspace, and disk could not be checked \(.+\)\.$/); + assert.equal(read.error, deleted.error); + }); + + test("an edit under a pass that decided, with a region left incomplete, applies to the workspace's copy and says so, where one that decided nothing refuses", async (t) => { + const env = new Env(t); + const m = await env.open(); + await seedSynced(env, m, 'a.txt', 'A line\n'); + // Another directory where the mount's root was: a pass decides every + // path as disk couldn't check it, and leaves the root's region incomplete. + renameSync(env.dir, join(env.root, 'unmounted')); + mkdirSync(env.dir); + writeFileSync(join(env.dir, 'a.txt'), 'A line\n'); + const edit = await m.handleToolCall({ id: 't', name: 'edit', input: { path: 'work/a.txt', oldString: 'A line', newString: 'A edited' } }); + assert.equal(edit.success, true, JSON.stringify(edit)); + assert.match(JSON.stringify(edit.data), /couldn't be checked against disk \(the mount root is not the directory disk last agreed with/); + }); + test('a branch switched away during the root check and back during a read is decided on the branch enumerated', async (t) => { const env = new Env(t); const m = await env.open(); From f323cf20a19aa116c13b7ae30866678ab1832bd3 Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Wed, 21 Oct 2026 16:55:59 -0400 Subject: [PATCH 4/4] fix(workspace): an unchecked answer names the call to make again "Try again to check it." left its "it" open: an agent could read it as "check the file", look for a check or sync tool, and call that instead of repeating the call. The call made again is what checks disk first, so the answers from delete, readBinary, edit and edit_many now end "Make the same call again, which checks disk first." The .fixed fragment no longer says the same call made again found the file before this change, which no run showed for delete, edit or edit_many; it says they answered "File not found" even when disk held it. Co-Authored-By: Claude Opus 5.5 --- changelog.d/552-pass-decided-nothing.changed.md | 12 ++++++------ changelog.d/552-pass-decided-nothing.fixed.md | 10 +++++----- src/modules/workspace/index.ts | 4 ++-- test/workspace-three-way.test.ts | 12 ++++++------ 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/changelog.d/552-pass-decided-nothing.changed.md b/changelog.d/552-pass-decided-nothing.changed.md index 21f93ebda..c0ee3fb42 100644 --- a/changelog.d/552-pass-decided-nothing.changed.md +++ b/changelog.d/552-pass-decided-nothing.changed.md @@ -1,9 +1,9 @@ - The workspace's `edit` and `edit_many` refuse a file the workspace holds when the check of disk before them decided nothing, because the selected branch kept changing while it ran. They store nothing, and say so, with - that reason and to try again. Before, they applied to the workspace's copy - without a word, so where disk had changed they edited text disk had moved - past and left the path in conflict, and they committed on whichever - branch was selected when the check gave up. Where disk's copy couldn't be - read, an `edit` or `edit_many` still applies to the workspace's copy and - says so. + that reason and to make the same call again. Before, they applied to the + workspace's copy without a word, so where disk had changed they edited + text disk had moved past and left the path in conflict, and they committed + on whichever branch was selected when the check gave up. Where disk's copy + couldn't be read, an `edit` or `edit_many` still applies to the + workspace's copy and says so. diff --git a/changelog.d/552-pass-decided-nothing.fixed.md b/changelog.d/552-pass-decided-nothing.fixed.md index a87c4c549..e72c6a37e 100644 --- a/changelog.d/552-pass-decided-nothing.fixed.md +++ b/changelog.d/552-pass-decided-nothing.fixed.md @@ -2,8 +2,8 @@ method, no longer answer "File not found" for a path the workspace doesn't hold when the check of disk before them decided nothing, because the selected branch kept changing while it ran. Each now says that disk - couldn't be checked, with that reason, and to try again. Before, disk might - hold the file, and the same call made again found it (#552). `readBinary` - also gives `delete`'s answer for every other path the workspace doesn't - hold, such as one whose disk copy couldn't be read, where it said "File not - found" whatever the reason. + couldn't be checked, with that reason, and to make the same call again. + Before, they answered "File not found" even when disk held the file + (#552). `readBinary` also gives `delete`'s answer for every other path the + workspace doesn't hold, such as one whose disk copy couldn't be read, where + it said "File not found" whatever the reason. diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index 1de91d250..8be71e792 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -3703,7 +3703,7 @@ export class WorkspaceModule implements Module { * call made again checks it. */ private notCheckedYet(path: string, reason: string): string { - return `${this.notInWorkspace(path, { path, state: 'unverified', note: reason })} Try again to check it.`; + return `${this.notInWorkspace(path, { path, state: 'unverified', note: reason })} Make the same call again, which checks disk first.`; } /** Why the workspace holds no entry at a path, when disk explains it. */ @@ -4141,7 +4141,7 @@ export class WorkspaceModule implements Module { kind: 'file', edits: target.edits, path: target.path, - text: `${target.path} couldn't be checked against disk (${undecided}), so ${subject} changed nothing. Try again to check it.`, + text: `${target.path} couldn't be checked against disk (${undecided}), so ${subject} changed nothing. Make the same call again, which checks disk first.`, whole: true, }); continue; diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index 64cd43c99..d20a497c0 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -7077,13 +7077,13 @@ describe('on-agent-action', () => { assert.equal(again.success, true, `${tool} again: ${JSON.stringify(again)}`); } } - const unchecked = `work/new.txt is not in the workspace, and disk could not be checked (${KEPT_CHANGING}). Try again to check it.`; + const unchecked = `work/new.txt is not in the workspace, and disk could not be checked (${KEPT_CHANGING}). Make the same call again, which checks disk first.`; assert.deepEqual(answers, { delete: unchecked, readBinary: unchecked, - edit: `work/new.txt couldn't be checked against disk (${KEPT_CHANGING}), so the edit changed nothing. Try again to check it.`, + edit: `work/new.txt couldn't be checked against disk (${KEPT_CHANGING}), so the edit changed nothing. Make the same call again, which checks disk first.`, edit_many: 'The edit_many call stored none of its edits: 1 problem.\n\n' - + `edit 1: work/new.txt couldn't be checked against disk (${KEPT_CHANGING}), so edit 1 changed nothing. Try again to check it.`, + + `edit 1: work/new.txt couldn't be checked against disk (${KEPT_CHANGING}), so edit 1 changed nothing. Make the same call again, which checks disk first.`, }); }); @@ -7098,7 +7098,7 @@ describe('on-agent-action', () => { unvouched(env, 'a.txt'); const edit = await whileBranchKeepsChanging(env, () => m.handleToolCall({ id: 't', name: 'edit', input: { path: 'work/a.txt', oldString: 'A line', newString: 'A edited' } })); assert.equal(edit.success, false, JSON.stringify(edit)); - assert.equal(edit.error, `work/a.txt couldn't be checked against disk (${KEPT_CHANGING}), so the edit changed nothing. Try again to check it.`, disk); + assert.equal(edit.error, `work/a.txt couldn't be checked against disk (${KEPT_CHANGING}), so the edit changed nothing. Make the same call again, which checks disk first.`, disk); // So does edit_many's, the branch left on `other` by the edit's pass // changing under its three gathers in turn. assert.equal(env.store.currentBranch().name, 'other'); @@ -7106,7 +7106,7 @@ describe('on-agent-action', () => { () => m.handleToolCall({ id: 't', name: 'edit_many', input: { edits: [{ path: 'work/a.txt', oldString: 'A line', newString: 'A edited' }] } })); assert.equal(many.success, false, JSON.stringify(many)); assert.equal(many.error, 'The edit_many call stored none of its edits: 1 problem.\n\n' - + `edit 1: work/a.txt couldn't be checked against disk (${KEPT_CHANGING}), so edit 1 changed nothing. Try again to check it.`, disk); + + `edit 1: work/a.txt couldn't be checked against disk (${KEPT_CHANGING}), so edit 1 changed nothing. Make the same call again, which checks disk first.`, disk); // Disk is as it was, and each branch's workspace copy is the one the // edits found, read from the store, which no pass changes. const onDisk = disk === 'changed' ? 'disk says A2\n' : 'A line\n'; @@ -7148,7 +7148,7 @@ describe('on-agent-action', () => { { path: 'work/new.txt', oldString: 'N line', newString: 'N edited' }, ] } })); assert.equal(many.success, false, JSON.stringify(many)); - const refused = (i: number, path: string) => `edit ${i}: work/${path} couldn't be checked against disk (${KEPT_CHANGING}), so edit ${i} changed nothing. Try again to check it.`; + const refused = (i: number, path: string) => `edit ${i}: work/${path} couldn't be checked against disk (${KEPT_CHANGING}), so edit ${i} changed nothing. Make the same call again, which checks disk first.`; assert.equal(many.error, ['The edit_many call stored none of its edits: 3 problems.', refused(1, 'a.txt'), refused(2, 'b.txt'), refused(3, 'new.txt')].join('\n\n')); assert.deepEqual(['a.txt', 'b.txt', 'new.txt'].map((p) => env.readDisk(p)), ['A line\n', 'B line\n', 'N line\n']); const treeStateId = (m as any).mounts.get('work').treeStateId as string;