Skip to content

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

Merged
nissa-seru merged 5 commits into
anima-research:mainfrom
nissa-seru:fix/workspace-deleted-on-disk-advice
Oct 18, 2026
Merged

nissa-seru merged 5 commits into
anima-research:mainfrom
nissa-seru:fix/workspace-deleted-on-disk-advice

Conversation

@nissa-seru

Copy link
Copy Markdown
Contributor

TL;DR: where disk has deleted a file the workspace holds, the workspace's refusals and the edit's conflict note now say that a sync of the path takes disk's deletion, which removes the workspace's copy, and, where they name force, that a forced materialize writes the workspace's copy back. They offered to take or adopt disk's copy, or to overwrite it, so an agent that synced to take disk's copy lost the workspace's copy and got nothing in its place. Where disk does hold a copy, the edit's note now says that a sync keeps it and gives up the workspace's, which is true for a copy the workspace can't store too.

Problem

On main b96e918b, where the workspace module is as #456 left it at e57e48a9:

  • The edit's conflict note said "Sync the path to take disk's copy instead, or materialize it with force to write the workspace's over disk's." It comes on an edit's refusal for what it found, and on a success where the mount doesn't push.
    • Where disk has deleted the file, as in a deleted-on-disk conflict, disk has no copy. A sync of the path takes the deletion (adoptDisk's branch for an absent disk copy, which this change doesn't touch), so it removes the agent's draft, and force has no disk copy to write over.
    • Where disk holds a copy the workspace can't store, binary or past maxFileSize, a sync gives up the draft and takes nothing in (adoptDisk's branch for a copy it can't ingest), so the next read answers that the file is on disk but not stored.
  • A recorded conflict keeps its kind after disk deletes the file, since decide() returns a recorded conflict as it stands. So a both-changed conflict whose disk copy is deleted later still names both-changed, and a note chosen by the kind would give it the advice for disk's copy.
  • A push's refusal for a file disk has deleted, which materialize lists and a pushing mount's write or edit quotes, offered to adopt or overwrite a disk copy that isn't there. Where disk changed and the workspace didn't, it said "sync this path to adopt the disk version, or materialize it with force to overwrite it". A conflict's reason offered the same two ways out in its kind's words, which also described a disk copy, as in "disk holds a different copy" or "disk holds bytes that match neither…".
  • A path whose branch intent can't be read (readIntent in branch-intent.ts says so for a record whose blob is missing or that isn't a branch intent) was told to "sync this path to adopt the disk version, which replaces the intent" where disk had deleted the file. That sync takes the deletion too.

Changes

All in src/modules/workspace/.

  • The edit's conflict note (handleEdit) goes by what the edit's pass found on disk, report.diskNow, not by the conflict's kind.
    • Where disk has no copy: "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."
    • Where disk holds one: "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."
  • pushPaths (reconcile.ts): a path the workspace holds and disk lacks (gone) gets the same two ways out, in the stale copy's reason and in every conflict's. A conflict's reason there says "disk has no copy of this file", with the conflict's kind beside it. Where nothing records which side is newer, it still names the path for force, since a forced push writes such a file only for a path named to it. An unreadable intent's reason still names the sync alone, and now says that it takes the deletion.

What callers will notice

  • Agents: for a file disk has deleted, these refusals and notes say what a sync and a forced materialize do there, and the edit's note says a sync gives up the workspace's copy wherever disk holds one.
  • Scripts: these texts changed. Nothing in connectome-host a8f672b3, chapterx c38b2cd1, discord-mcpl 411837a1, context-manager ae7222b6 or membrane a76f9b3e matches on the workspace texts this change replaces.

Tests

On Linux, Node 24.22.0, installed with npm install as CI does:

  • The head: 82735148 is ca4dc393 with main b96e918b merged in (141e95c6, whose diff against main is ca4dc393's against e57e48a9) and one commit of tests after it.
  • The tests: test/workspace-file-tools.test.ts gains five cases and changes three, and test/workspace-three-way.test.ts gains one. At 82735148 the first passes 40 of 40, the second 214 of 214, and the module's nine other test files pass beside them, none skipped.
    • An edit of a draft whose disk copy was deleted: its miss, its success note, and each way out doing what the note says. After the sync a read finds no file, and after the forced materialize disk holds the draft.
    • A recorded both-changed conflict whose disk copy is deleted since, for the edit's note and the push's reason, and a conflict with nothing to say which side is newer, for the push's reason and a forced materialize of the named path, which writes the copy back.
    • A push where disk has deleted the file, from a materialize and from a write's own push, and a forced materialize writing the copy back.
    • An edit of a draft whose disk copy the workspace can't store, binary and past maxFileSize: its note, a sync that leaves the file on disk and not stored, and a listing that shows it disk-only.
    • An interrupted conflict whose disk copy is deleted since: its reason, and a forced materialize writing the copy back.
    • The changed cases: the unreadable intent's test, which now expects the new wording once disk's copy is gone and checks that the sync removed the workspace's copy, and two edits in conflict with a disk copy, which expect the note's new wording.
  • Red-first: against main's index.ts and reconcile.ts, which are e57e48a9's, the eight cases this change adds or changes in workspace-file-tools fail and the other 32 pass, and the new workspace-three-way case fails and the other 213 pass.
  • Removals: at 82735148, each of ten removals, made alone in the built code, fails at least one case. They remove the edit note's condition or make it the conflict's kind, and restore the note's old wording where disk holds a copy. They drop gone, the sync's and force's wording where disk's copy is gone, a conflict's reason there, its naming the path where nothing records which side is newer, and its place before the interrupted conflict's own words. Another drops the unreadable intent's wording.

Not verified

  • Windows, and macOS until CI runs.
  • watch: 'on-agent-action' and 'always'. The tests run on 'never'.
  • A store-origin-collision conflict whose disk copy is deleted since. Its reason takes the same branch as the unknown-provenance one, which a test drives.

Out of scope

  • ls's description keeps its general "sync the path to take disk, or materialize that path with force to take the workspace". That's true for a deletion and for a copy the workspace can't store, and the refusals and the edit's note say what each way out does there.
  • A push's refusal where disk holds a copy keeps "sync this path to adopt the disk version", which says which side the sync takes. Where the workspace can't store that copy, the path is then listed disk-only.
  • A recorded conflict's kind isn't updated when disk changes again, so a refusal can name both-changed for a file disk has since deleted. Its ways out now follow what disk holds.

What each part is for

  • The note follows disk's state, not the kind. The kind is what was recorded, possibly before disk deleted the file, and the way out acts on what disk holds now. Conditioning on deleted-on-disk alone would give a recorded both-changed conflict over a deleted file the advice for disk's copy, and the removal that makes it the kind fails a test.
  • "Keep disk's copy and give up the workspace's" where disk holds a copy, since it's true whether or not the workspace can store that copy, and it says what the agent gives up.
  • One condition in pushPaths, gone, decides every reason that names a sync or force, so the wording follows disk's state in one place. Rewording each reason on its own would repeat the test in three places.
  • A conflict's reason says "disk has no copy" rather than its kind's own words, since each of those describes a disk copy. The kind stays in parentheses, as the conflict's record has it.
  • The path is still named for force where nothing records which side is newer, since a forced push writes such a file only for a path named to it.
  • The unreadable intent still names the sync alone, as fix(workspace): the file tools store exactly what they're given or refuse, and say when a stored change didn't reach disk #456 has it. This change says what that sync does where disk has no copy.

Review

Reviewed in the house: an internal review of e57e48a9..af5f2189, whose findings ca4dc393 answers. Ogden-2688 read and approved af5f2189, then ca4dc393, which this head merges with main before its commit of tests. Abigail-2676 found the edit note's wording, for a deleted file and for a copy the workspace can't store, while reviewing a related change, and Ogden-2688 found the same in an unreadable intent's reason while reading this one.

— Phoebe-2419 (Opus), contributing through nissa-seru

Phoebe-2419 added 5 commits October 17, 2026 17:37
…s ways out say a sync takes the deletion and a forced push writes the copy back

Abigail-2676 found that the edit's conflict note told the agent to "Sync the path to take disk's copy instead" for a deleted-on-disk conflict. Disk has no copy there. A path sync takes the deletion (adoptDisk's absent arm) and removes the workspace's copy, and "write the workspace's over disk's" names a disk copy that isn't there. A push's refusals said the same in their own words: "sync this path to adopt the disk version, or materialize it with force to overwrite it".

- The edit's conflict note now goes by what the pass found on disk (report.diskNow), not by the conflict's kind, since a recorded conflict keeps its kind after disk deletes the file. Where disk has no copy, it says 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 to disk.
- In pushPaths, a path the workspace holds and disk lacks gets the same two ways out, both where disk changed and the workspace didn't and where they conflict. A conflict's reason there says disk has no copy of the file, whatever kind was recorded, and still names the path for force where nothing records which side is newer.
- Tests cover an edit of a draft whose disk copy was deleted, with its miss, its success note and each way out doing what the note says. They also cover a recorded both-changed conflict and a no-evidence conflict whose disk copies were deleted since, and a push's refusal for a file disk deleted, from a materialize and from a write.

Runs on this tree before the commit, Linux, Node 24.22.0: the eleven workspace test files pass, workspace-file-tools 38/38. Removing each of the six pieces fails at least one case. Against the index.ts and reconcile.ts of anima-research#456's head e57e48a, the three new tests fail and the other 35 pass.
… sync says it takes the deletion

Ogden-2688 found that the refusal for a path whose branch intent can't be read still told the agent to "sync this path to adopt the disk version, which replaces the intent" where disk has deleted a file the workspace holds. That sync takes disk's deletion (adoptDisk's absent arm), which removes the workspace's copy. The reason now says so, and still names the sync alone, as before.

- The test of an unreadable intent expects that wording once disk's copy is gone, and checks that the sync removed the workspace's copy.
- The changelog fragment says it too.

Runs on this tree before the commit, Linux, Node 24.22.0: the eleven workspace test files pass, workspace-file-tools 38/38. Removing the new condition fails a case, and so does conditioning the edit's note on the conflict's kind rather than on disk's state. Against the index.ts and reconcile.ts of anima-research#456's head e57e48a, the four tests this branch adds or changes fail and the other 34 pass.
…y and gives up the workspace's; tests pin an interrupted conflict and a named forced push over a deleted file

Answers af-ws-deleted-on-disk (Ogden-2688's internal review of e57e48a..af5f218, 9 issues judged in the review store) and Abigail-2676's second wording case (room-203 #135053).

- Where disk holds a copy, the edit's conflict note says "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." It said "take disk's copy instead", but where the workspace can't store disk's copy, binary or past maxFileSize, a sync gives up the draft and takes nothing in, so the next read answers that the file is on disk but not stored. A test drives that case, and the note's other assertions follow the wording.
- A three-way test pins that an interrupted conflict whose disk copy is deleted since says disk has no copy, before the interrupted conflict's own words, and that force writes the workspace's copy back (I2).
- The recorded-conflict test follows the no-evidence reason's advice: a forced materialize of the named path writes the workspace's copy back (I9).
- The changelog fragment says the note's new wording.

Not taken, as the verdicts say: I1 and I4 (not-worth), and I3, I5, I6, I7 and I8 (not-real). A probe at this tree shows that a path whose branch intent can't be read, and that neither the workspace nor disk holds, never reaches a push: a named materialize, a whole-mount one and a forced one plan nothing for it.

Runs on this tree before the commit, Linux, Node 24.22.0: the eleven workspace test files pass, workspace-file-tools 39/39 and workspace-three-way 214/214. Restoring the note's old wording fails four cases, and answering an interrupted conflict before a gone copy fails the new three-way case. Against the index.ts and reconcile.ts of anima-research#456's head e57e48a, workspace-file-tools fails the seven cases this branch adds or changes and passes the other 32, and workspace-three-way fails the new case and passes the other 213.
…e same note and sync as a binary one, and the listing shows it disk-only

From Ogden-2688's read of the pull request's text (room-203 #135771): the changelog fragment says a copy the workspace can't store, "binary or past the size limit", stays on disk alone, and only the binary case had a test.

- The binary case's test runs for a copy past the mount's maxFileSize too (setup takes maxFileSize). In each, the edit's note says a sync keeps disk's copy and gives up the workspace's, the sync leaves the file on disk and not stored, and disk keeps its bytes.
- Each also lists the mount after the sync and checks that the path is disk-only, which the description says of such a path.

Tests only.
@greptile-apps

greptile-apps Bot commented Oct 17, 2026

Copy link
Copy Markdown

Required label not found on this PR.

@nissa-seru
nissa-seru merged commit 7706636 into anima-research:main Oct 18, 2026
10 of 11 checks passed
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 18, 2026
, anima-research#462 and anima-research#466

Conflicts in src/modules/workspace/index.ts (11 hunks) and observe.ts
(one), each resolved as the change on both sides:
- The imports are unions, less readFile, which the merged code no
  longer calls (F145's readBinaryFromDisk reads through
  openContainedFile), and coveredByIgnore, whose use anima-research#460 replaced.
- commitToolChange: anima-research#462 (F48) answers { refused, modeKept } and F145
  { refused, note }. The merged answer carries all three, every return
  gives each, and the doc comment names both.
- writeBinary, write and edit take anima-research#462's side, which reads modeKept;
  delete takes F145's, which reads note.

anima-research#460 makes MountView's others required, so F145's test view gives
others: [], as the anima-research#460 trial (1e3fbb6e9) did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 18, 2026
main at d9bd6dc brings anima-research#460, anima-research#462, anima-research#466, anima-research#424, anima-research#445 and anima-research#425 into code
this change rewrites. The resolutions:

- src/modules/workspace/index.ts:
  - anima-research#460: editablePath parses a path with parseFilePath, so edit and
    edit_many refuse a path in a nested mount as every file tool does,
    and applyEdits' pass names the paths it settles ({ named: true }), so
    a file the mount passes over by design is taken in as a named sync
    takes it.
  - anima-research#462: commitToolChanges answers each path's kept mode beside its
    refusal, as commitToolChange does for one path. edit's answer carries
    it as modeKept, and edit_many's in each file's entry. applyEdits
    leaves a mode taken in alone unsaid, as handleEdit did, and the edits
    keep the entry's mode, as before.
  - anima-research#466: applyEdits already words the conflict note's advice by what
    disk holds now, so this change's side stands.
  - anima-research#424: the imports keep both sides.
- src/framework.ts, src/module-registry.ts and src/types/events.ts: anima-research#445's
  callerTurnId beside this change's resultMaxChars. The utils call
  carries both, the registry passes the call on whole, callerTurnId
  included, and events.ts documents both. anima-research#425's comment on a script's
  result keeps this change's SCRIPT_TOOL_RESULT_MAX_CHARS.
- test/workspace-file-tools.test.ts: this change's expectations, which
  already carry anima-research#466's advice in the refusal's new wording.

Also in this commit, as Anarchid's review asked (R1-4): edit_many cases
in the two nested-mount tests, which assert the nested-mount refusal's
exact text for edit and edit_many, since the named pass's note names the
inner mount too, a test of edit_many naming files the mount passes over
by design, and edit_many in the test of a push that keeps
disk's mode, all in workspace-three-way. anima-research#460's ignore-list adoption test
and anima-research#466's three deletion tests now expect the edit's refusal wording.
The read-back's mode test calls the commit directly, since an overwrite
now keeps the entry's mode and no tool sets the same bytes under another
mode. planFile's doc gives the text's length as how long it is, not its
characters, and the changelog says edit_many carries modeKept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant