Repository navigation
fix(workspace): reasons say what stands in the way and a way on, and an empty mount or path no longer means every mount - #849
Merged
Conversation
… carry their way on Each reason the workspace gives for what a push, a sync, a listing or a read couldn't do now says what gets past it, or that calling again won't change it and who can. - A failed system call is said in plain words (errnoWords), with its errno carried beside the reason: callFailed in effects.ts and failedOn in observe.ts word the failed calls a push and an observation meet, so those reasons quote neither Node's message nor the host's path, an unlink's included (anima-research#831). The push's catch-all words what it catches the same way, and hashAt's verifying read is worded as an untouched failure. - One chooser picks the way on, explained and wayOn in reconcile.ts, by the reason's cause (errno, again, ownStep) and what met it (a write, an unlink, a sync, a listing, a read; a push's chmod step takes the write's way on without inTheWay, whose texts are a write's). A state that stays until someone else acts (a link, a place outside the mount, what isn't a regular file, a gone root, an errno whose fix is the host's, a name too long) leads with "<again> won't change that". inTheWay grows to name a symlink at the path that the mount doesn't follow or a path leading out of the mount through a symlink, a parent leading out, and a pipe or a link to a directory under a write. A disk copy a push couldn't verify gets its note's way on, where "fix it and materialize again" followed every note; a note that names its own step (a nested mount's place, a link's refusal) gets nothing added. A way on after a fix someone else makes says materializing again then gets past it: what disk holds then may be a conflict of its own. - A push that failed after its effect began, or whose fsync failed, says that whether disk holds the change isn't known until it's checked, and that a materialize of the path checks it. - sync's skips, and the incomplete regions of ls, sync, grep and an agent-action scan, carry a way on where the walk failed, met a change, found the root unavailable, met a link or a place leading out of the mount or a link to a directory, or reached its cap. A sync skip of a path the walk passed over (ignored, an editor's swap or backup file, beneath a link it doesn't enter) names the sync of that path, which checks it. A listing's per-entry notes and grep's per-file skips stay descriptions. The walk's errno reaches a tracked file beneath a directory it couldn't list. - linkWords: a link to a directory inside the mount names that directory to use in its place, and the mount whose followSymlinks whoever runs the host can set (on a mount with an include, either reaches only what it covers there); one to something that isn't a directory says nothing lies beneath it; one leading outside, nowhere (ENOENT, ENOTDIR or ELOOP from realpath), or somewhere its refusal doesn't name says what gets past it; a directory the walk can't ask gives its errno's way on. LinkOnTheWay carries the reason, so the tools answer `"<path>" can't be reached: <reason>` while readFileFromDisk's message stays `<path>: <reason>`. No tool shows an agent where a link leads, so a symlink the mount doesn't follow whose target the answer doesn't name says whoever runs the host can set followSymlinks if it leads inside the mount, mount what it leads to, or a directory that holds it, if it leads outside, or remove it if it leads nowhere (such a link is refused before it's resolved). One leading outside, or a path leaving the mount through a symlink or a directory resolving outside, says nothing done through the mount reaches it and whoever runs the host can mount it, or a directory that holds it, read's escape answer and a direct mount's write included: a mount's root is a directory, and a link may lead to a file. - Beneath a followed symlink, confirmAbsent passes on locateDir's own reason, errno and ownStep (a nested mount's place), and keeps BENEATH_SYMLINK for nothing there and a directory outside the mount, whose words now say it won't change. - read of a file the workspace doesn't hold takes its note's way on from wayOn, as a push or a sync does: none beside a note that names its own step, and calling again for any note of a change while it was read. - checkRoot gives an unavailable root with no errno the errno of what is at its path (rootAbsence), so a file where the mount's directory was reads as ENOTDIR, and its answer says whoever runs the host can put the directory back. - A forced materialize without applyDeletions lists under skipped a workspace deletion no push makes, where pendingDeletions promised what applyDeletions couldn't do: one inTheWay names (over a symlink the mount doesn't follow or one leading out, beneath a directory leading out or a followed symlink to a file), and one whose note names its own step (a link among its directories the mount doesn't follow, a nested mount's place, or a directory whose readlink failed, whose note says calling again gets past it). - A forced write meeting EISDIR through a symlink to a directory names the link, as the unforced push does. A push's create meeting a link that leads nowhere says so, and that removing the link gets past it. - A push of a file whose stored copy is lost names the read, which takes disk's copy where it can and otherwise says what can be done. - materialize and sync refuse "mount": "" given without a path, and "path": "" whatever "mount" holds, which they took as not given, acting on every mount; each refusal says what leaving the field out covers (anima-research#832). - A direct mount's delete gives its unlink's errno a way on, says calling again checks afresh where the file changed while it was checked, and quotes the path where the unlink may have landed. - save_recent_image words a writeBinary throw: a workspace refusal as it is, anything else by its errno in plain words with a way on; it logs the throw whole. No tool description or schema changes. Tests: test/workspace-reasons.test.ts pins each requirement, save_recent_image tests pin its throw's words and its log, and the tests that pinned the old words are turned round. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review
Recommendation: merge and fast-follow. 3 medium severity findings can be fixed after the merge.
Reviewed commit 11057d6 with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.
Does this change belong in the project?
It fits the project. This fixes two filed issues in the existing WorkspaceModule: clearer refusal reasons, and treating an empty mount or path as unset. The work stays inside that module and follows the project's PR and changelog rules.
5 reasons with sources
The one structural point is small: framework.ts now imports workspace helpers at runtime.
- It fixes behaviour of a built-in module the project already ships. The README lists WorkspaceModule (mounts, materialization) as a built-in module. The PR closes issues #831 and #832, which an internal review of #822 found. It only rewords the workspace's refusal reasons and stops
"mount": ""and"path": ""from acting on every mount. No tool schema or description changes. (README.md (Built-in modules)) - The code changes stay inside src/modules/workspace, next to the reasons they reword. Reasons and refusals already live in answers.ts, observe.ts, reconcile.ts and effects.ts. The PR edits them in place and doesn't add a second way of reporting reasons. (src/modules/workspace/answers.ts, observe.ts, reconcile.ts)
- framework.ts now imports workspace helpers at runtime, where before it imported only the WorkspaceModule type. At base, framework.ts's only import from src/modules/ was
import type { WorkspaceModule }. The PR adds a runtime import of Refusal, errnoCode, errnoWayOn and systemWords from answers.ts so that save_recent_image can word store errors. save_recent_image already exists only to write into the workspace, so the coupling is small. A maintainer may still prefer that the module expose the wording itself. (src/framework.ts (imports, line 115 at base)) - It follows the contribution rules: changelog fragments, tests, and before/after evidence. It adds two
changelog.d/*.changed.mdfragments. It adds a new test/workspace-reasons.test.ts and updates the existing tests. The description gives real before/after outputs. The changelog warns host code that matches on the old link-refusal wording. Whether that warning should be a breaking fragment is left to the code review. (CONTRIBUTING.md (What a PR should contain; Changelog)) - It is one coherent change, even though it is large. Both linked issues come from the same review of #822, and both concern how materialize and sync answer. Bundling them doesn't bring in unrelated work. (CONTRIBUTING.md (How changes land))
Findings
2 findings of medium or high severity, see the inline comments:
- Medium severity. Pass incomplete regions always tell the agent to list again, even in sync, grep and scan answers.
- Medium severity. Root that vanishes mid-walk carries no errno, so its region says "checks it afresh" instead of "put the directory back".
1 finding about lines the diff does not show:
- Medium severity. Refusal for a link that can't be checked hard-codes "calling again", even inside a materialize's skip.
src/modules/workspace/observe.ts:841-844
Details of this finding
- Medium severity,
src/modules/workspace/observe.ts:841-844. Refusal for a link that can't be checked hard-codes "calling again", even inside a materialize's skip. linkWords appendserrnoClause(code, 'calling again', 'calling again then gets past it')when it can't tell whether something is a symlink. linkReason marks this reasonownStep, so a push adds no way on of its own. A materialize'sskippedtherefore says "cannot verify disk copy (cannot tell whether "work/a" is a symlink (not permitted); calling again won't change that; whoever runs this host can change the permissions, and calling again then gets past it)". workspace-parent-link.test.ts expects this text. That breaks the PR's stated rule that a push's reason names the call ("materializing again", or with applyDeletions for a deletion), and the same reason in a sync names no sync.
2 more findings, less severe
- Low severity, documentation,
changelog.d/workspace-reasons.changed.md:73. Host-facing word change is filed as a 'changed' entry rather than a breaking fragment. This bullet says hosts that call readFileFromDisk, readBinaryFromDisk or writeBinary, or that readincompleteonlastAgentActionScanand theworkspace:agent-action-scan-incompleteevent, get new words, and that "code that matches on those words needs updating". It opens with the bolded "Hosts …:" lead that changelog.d/README.md reserves for breaking fragments ("Breaking fragments open by naming who needs to act"). Yet it sits in a.changed.mdfile, so at release it lands under Changed and not Breaking. Hosts that scan the Breaking section for actions they must take would miss it. It belongs in its ownworkspace-reasons.breaking.md. - Low severity, maintainability,
src/framework.ts:116. Framework core now imports workspace-module internals at runtime. Until now framework.ts took onlyimport type { WorkspaceModule }from the workspace module, so the core had no runtime dependency on it. This change adds a value import ofRefusal,errnoCode,errnoWayOnandsystemWordsfrom./modules/workspace/answers.jsto word save_recent_image's thrown errors. That ties the core to a module's internal wording helpers. A shared errno-wording helper (next to error-message.ts) or a method on WorkspaceModule would keep the layering.
For a deeper review by several readers and a judge, which takes longer, a maintainer can comment /review deep.
Comment on lines
+1290
to
+1292
| if (region.kind === 'error' || region.kind === 'cap' || unreached || wholeScope) { | ||
| result.incomplete.push({ path: region.path, reason: explained(region.reason, region.errno !== undefined ? { errno: region.errno } : {}, 'list') }); | ||
| } |
There was a problem hiding this comment.
Medium severity. Pass incomplete regions always tell the agent to list again, even in sync, grep and scan answers.
reconcilePass words every incomplete region with explained(..., 'list') (here, at the kept changing region at line 1130, and in gather's root-unavailable regions). The pass does not know who called it, so a sync, a grep or an agent-action scan also returns text like "listing it won't change that; … and the workspace then reaches it" or "listing it checks it afresh". The PR says each reason names the call that helps ("syncing this path again" and so on), but a sync's incomplete names a different tool. workspace-reasons.test.ts line 600 asserts this wording for sync { mount: 'gone' }, so the mismatch is locked in. The meets value should come from the caller, or the wording should be applied in the tool layer where the tool is known.
Comment on lines
2263
to
+2265
| const proof = rel === '' ? ROOT_UNAVAILABLE : await confirmAbsent(view, rootReal, rel, 'beneath'); | ||
| if (typeof proof === 'string') fail(rel, proof); | ||
| else if ('reason' in proof) fail(rel, proof.reason, proof.errno); |
There was a problem hiding this comment.
Medium severity. Root that vanishes mid-walk carries no errno, so its region says "checks it afresh" instead of "put the directory back".
When listing fails at rel === '', walkScope records the bare string ROOT_UNAVAILABLE through fail(rel, proof) with no errno. checkRoot now calls rootAbsence to attach ENOENT/ENOTDIR for exactly this reason. Without that errno, wayOn takes the cause.errno === undefined branch and says "listing it checks it afresh; if it's still unavailable, whoever runs this host can check the mount's directory and its path". If the directory is really gone, the answer should be "whoever runs this host can put the mount's directory back at its path". The same reason gets different ways on depending on which code path met it.
nissa-seru
pushed a commit
to nissa-seru/agent-framework
that referenced
this pull request
Oct 27, 2026
…earch#845) into the re-arm branch
tel-0s
added a commit
that referenced
this pull request
Oct 28, 2026
110 commits from main, 20 pull requests: #852 (supplied text can't read as the framework's own), #853 (every check of whether an agent sees a row asks its view), #827 (the heartbeat moves into the framework), #846 (exhausted turns re-armed), #830 (chat:system notices), #801 (the reply judge), #840, #843, #839, #791, #845, #847, #849, #822, #854, #836, #855, #857, #858, #859 and the 0.28.0 release. package.json takes main's version and ranges (context-manager ^0.23.1, membrane ^0.8.0). Conflicts: - src/framework.ts, TurnRouting: both fields kept, the branch's addressedElsewhere and main's heard (and both in freshTurnRouting). - src/framework.ts, createAgent: main's `sees` predicate, recorded in storeSees, is the branch's composition: heldFromResidents (the RESIDENT_VIEW_FILTER rule) and !isAnotherAgentsInjection; the view filter reads it. - src/framework.ts, tool-boundary drain: the branch's releaseLandedMail and main's noteSpokenTo both run; the injection skip asks agentSees (main) where the branch asked heldFromResidents, which it subsumes. - src/framework.ts, the fork wake and the routed fork wake: the branch's tag-only isAddressedMessage, with main's systemNoticeMark added. - src/framework.ts, a coalesced batch's wake: tag-only addressed, with main's note on why it carries no systemNotice mark. - src/framework.ts, isUnreadStoredMessage: main's form (the slot's readers each asked their own view) over the registry's sharedSlotReaders. - src/framework.ts, noteArrival: main's agentSees and heard, the branch's arrival with `addressed`. - src/framework.ts, noteForOtherResidents: the registry's readers, with main's noteSpokenTo. - src/framework.ts, the channel tool dispatch: the branch's publishInputWithTurnLocus, with main's earlierWake on the origin. - src/framework.ts, tune_out repeated on a tuned-out channel: the branch's awaited form, the name quoted by channelRegistry.quotedShown (#852). - src/mcpl/channel-registry.ts, markAccepted: the branch's (admission's noteDelivered sets the locus); main's chat:system is a marker (isMarkerTagged), so noteDelivered's rule covers it, and its comment says so. - src/mcpl/channel-registry.ts, tuneOutEndText: the branch's serverId form for a channel no name picks alone, a name written by quotedValue. - src/tune-out/coordinator.ts: the branch's readOptions, all-except refusal, renderHeldLine/historyTool, reader-fork invocation and cancel_tuneout scope logic kept; main's quotedShown, NOTICE_TAG texts and suppliedValue author adopted in each. - test/batched-wake-locus: main's new two-closed-channels test is placed in the branch's rig (framework read from rig.framework()); the branch's ambient-only tests kept. - test/channel-registry-routing: the branch's noteDelivered comment, with main's chat:system added to the marker loop. Main's changes integrated into the branch's structures: - heartbeatTargets reads registry.untargetedRecipients() (an untargeted push's recipients) and heartbeatRefusal registry.homeChannel, in place of conversationAgentHomes. - releaseAgentRuntime also drops #846's re-arm count and exhausting re-arm and #840's exhausted-turn record, and ends the run of stated waits (endStatedWaitRun). - storeSees is recorded for a derived manager (its source's predicate, which context-manager's derive keeps as its view filter) and for a context reopened on the shared slot (the residents' view). - The closed-channel invitation for a deliverTo correction reads the recipient admission chose, where main rewrites event.targetAgents. - #852 on the branch's own texts: authorLine/presentAuthor write the author through suppliedValue; the addressed-elsewhere notice's author too; `Focus` and `Agent lifecycle` are NOTICE_TAG entries and their notices use them; tune-out's branch-only notices use NOTICE_TAG and name channels by shown/quoted names. Tests read onto this branch, each named in a comment: - heartbeat (two cases): the fake fork is a registry record (conversationForkSpec), not a conversationAgentHomes entry. - prose-drafts (stated provider wait, two cases): scout is made a conversation agent through its registry record (homeChannel, no provider scheduling), as main does through conversationAgentHomes. - channel-registry-routing (chat:system locus): each message is delivered with noteDelivered, where admission records the locus here. - supplied-text-in-context (three cases): a channel message is stored and sent under its author's name (presentAuthor); the delivered body's digest is compared with the marked body before that decoration. - tune-out-reader-queued: its fake registry gains quotedShown. Tests: built with zero errors; 29 stale compiled tests removed. The targeted run (dendrite-*, tune-out-*, batched-wake-locus, channel-registry-routing, compression-after-release, stored-injections and main's added or changed framework-area tests, 63 files) gave 2507 tests, 2491 pass, 14 fail, 2 skipped before the readings above; after them, 73 files (those 63 and all-except-unit, all-except-e2e, author-presentation, module-rows-held-from-residents, refusals-are-error-results, state-read-and-write-errors, inbound-source, reinit-after-branch-move, channel-agent-words, channel-lifecycle-order) gave 2860 tests, 2858 pass, 0 fail, 2 skipped (main's runtime-conditional skips). The full suite was not run. 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.
The workspace's reasons for what a push, a sync, a listing or a read couldn't do now say what stands in the way and a way on, or that nothing the agent does helps and who can. A failed system call is said in plain words, where an unlink's failure quoted Node's message with the host's absolute path.
materializeandsyncnow refuse"path": "", and"mount": ""given without a path, where they acted on every mount. No tool's description or schema changes.Problem
Our internal review of #822, whose merge
913afe34is this change's base, found two things, filed as issues:mountand act on more than one mount #832:materializeandsynctook"mount": ""as not given, and acted on every mount.Closes #831. Closes #832.
Changes
For an agent
The examples are from real calls on Linux with Node 24.22.1, at
913afe34, this change's base, and at this change's tree. The temporary directory in the first is shortened to….materializewithapplyDeletionsof a file in a read-only directory:locked/x.txt: not unlinked: could not unlink the file: EACCES: permission denied, unlink '/home/work/t/…/one/locked/x.txt'locked/x.txt: not unlinked: could not unlink the file: permission denied — materializing again with applyDeletions won't change that; whoever runs this host can change the permissions, and materializing again with applyDeletions then gets past itmaterialize'sskippedand in the push sentence ofwrite,edit,edit_many,deleteandwriteBinary. Where the state stays until someone else acts, the reason says that calling again won't change that, naming the call ("materializing again", "syncing this path again"), then who can fix it. Where the way on depends on that fix, it says that materializing again then gets past it.sync's skips, and each incomplete region ofls,sync,grepand an agent-action scan that comes from an error, give a way on. Asyncskip of a path its walk passed over names the sync of that path. A listing's per-entry notes andgrep's per-file skips stay descriptions.followSymlinkson that mount. Before:"nf/indir" is a symlink to "nf/real", which this mount does not follow; the mount's host can set followSymlinks: true. After:…; use "nf/real" in place of "nf/indir", or whoever runs this host can set followSymlinks: true on mount "nf";followed or not, nothing done through this mount reaches what it leads to, and whoever runs this host can mount it, or a directory that holds it;"nf/nodir" is a symlink that leads nowhere, which this mount does not follow; whoever runs this host can remove it or make it lead to what it stood for;whoever runs this host can set followSymlinks: true on this mount if it leads inside the mount, mount what it leads to, or a directory that holds it, if it leads outside, or remove it if it leads nowhere. At913afe34it saidif you know the file it leads to, name that file by its own path. For a link leading outside its mount, no workspace tool shows an agent where it leads, so an agent couldn't know that file: at this change,ls(plain and recursive),glob,grep,status,readandsyncname no target for such a link, including one into another mount. That wording came with fix(workspace): answer a refused or failed call with what didn't happen and the way on #822, and 0.28.0 has it.materializeandsyncrefuse"mount": ""given without a path, and"path": ""whatever"mount"holds. Each refusal acts on no mount, quotes the field, names the mounts, and says what leaving it out covers. Before, both took them as not given:materializewith"mount": ""wrote the unwritten files of mountsoneandtwo, and now answersThe materialize did nothing: "mount" is "", which is no mount's name; the mounts are "one", "two", "auto" and "direct". Leaving "mount" out covers every mount that takes writes and doesn't already write them straight to disk.A path still names its own mount, and"mount"beside a path is ignored whatever its value, as before.materializewithoutapplyDeletionslists a workspace deletion that no push makes, such as one beneath a symlink the mount doesn't follow, underskippedwith why, not among the deletionsapplyDeletionswould make.readof a file the workspace doesn't hold, where disk couldn't be checked, gives the way on its reason calls for;walkCap;deletewhose unlink fails says the cause in plain words, with a way on;save_recent_image, where the store throws on a write, says the store's errno in plain words with a way on, still names the images already saved, and logs the error whole;For hosts
readFileFromDisk, the reason after<path>:now ends with its way on, as in the examples above. AwriteBinaryfailure quotes the push's reasons in the new words. Theincompletereasons onstatus'slastAgentActionScanand on theworkspace:agent-action-scan-incompleteevent carry their way on, an errno in plain words. Code that matches on those words needs updating.maine7f1e5ff(connectome-host 0.15.0),src/modules/identity-module.ts:49–51tells the agent that the reason shows a link leads to a directory "by saying the mount's host can set followSymlinks". With this change, the reason for such a link sayswhoever runs this host can set followSymlinks: true on mount "<mount>"instead.changelog.d/workspace-reasons.changed.mdandchangelog.d/workspace-empty-mount.changed.md.What this leaves
Tests
test/workspace-reasons.test.ts. Thirteen other test files change, most of them to expect the new wording.test/save-recent-image.test.tsadds two tests and extends one, and one of them makes Chronicle 0.4.0's store refuse a write. Intest/workspace-three-way.test.ts, the mode-only race test, which already swapped a link in, now checks the link case's reason and adds a case with a directory swapped in.test/workspace-direct-write.test.tsadds a write beneath a directory resolving outside.8ff287dd, made locally.mainhas since taken only 0.28.0's release commit, which changes no source or test file. The merge gave 8,561 tests, 8,499 passing, none failing, 62 skipped.913afe34's source fail 128 of their 1,087 tests, each at an assertion.getTools(), dumped with four mount shapes (read-write, read-only, direct, and one that follows links), is byte-identical at913afe34and at this head, 12 tools.913afe34and at this change's tree. 34 of the 42 answers differ.Not verified
readFileFromDisk's check after its open. Only a link swapped in between itslstatand itsrealpathreaches it.test/workspace-path-spelling.test.ts:1223, and its new words were written without a run. macOS CI runs most of the 62.changelog.d/workspace-reasons.changed.mdandchangelog.d/workspace-empty-mount.changed.md.Pippa-2571 (Claude Opus 5.5) wrote the change from Ada-1017's design calls, and our internal review checked it. Naomi-2422 (Claude Opus 5.5) read it as someone who built none of it, and Alfred-2570 (Claude Opus 5.5) read its answers as an agent that meets them would. Lottie-2661 (Claude Opus 5.5) wrote this description, from Pippa-2571's facts.
🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared
nissa-seruaccount.