Repository navigation
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 intoOct 18, 2026
Conversation
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.
|
Required label not found on this PR. |
This was referenced Oct 18, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
mainb96e918b, where the workspace module is as #456 left it ate57e48a9:deleted-on-diskconflict, 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.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.decide()returns a recorded conflict as it stands. So aboth-changedconflict whose disk copy is deleted later still namesboth-changed, and a note chosen by the kind would give it the advice for disk's copy.writeoreditquotes, 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…".readIntentinbranch-intent.tssays 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/.handleEdit) goes by what the edit's pass found on disk,report.diskNow, not by the conflict's kind.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
a8f672b3, chapterxc38b2cd1, discord-mcpl411837a1, context-managerae7222b6or membranea76f9b3ematches on the workspace texts this change replaces.Tests
On Linux, Node 24.22.0, installed with
npm installas CI does:82735148isca4dc393withmainb96e918bmerged in (141e95c6, whose diff againstmainisca4dc393's againste57e48a9) and one commit of tests after it.test/workspace-file-tools.test.tsgains five cases and changes three, andtest/workspace-three-way.test.tsgains one. At82735148the first passes 40 of 40, the second 214 of 214, and the module's nine other test files pass beside them, none skipped.both-changedconflict 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.maxFileSize: its note, a sync that leaves the file on disk and not stored, and a listing that shows it disk-only.main'sindex.tsandreconcile.ts, which aree57e48a9's, the eight cases this change adds or changes inworkspace-file-toolsfail and the other 32 pass, and the newworkspace-three-waycase fails and the other 213 pass.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 dropgone, 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
watch: 'on-agent-action'and'always'. The tests run on'never'.store-origin-collisionconflict whose disk copy is deleted since. Its reason takes the same branch as theunknown-provenanceone, 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.both-changedfor a file disk has since deleted. Its ways out now follow what disk holds.What each part is for
deleted-on-diskalone would give a recordedboth-changedconflict over a deleted file the advice for disk's copy, and the removal that makes it the kind fails a test.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.Review
Reviewed in the house: an internal review of
e57e48a9..af5f2189, whose findingsca4dc393answers. Ogden-2688 read and approvedaf5f2189, thenca4dc393, which this head merges withmainbefore 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