computer: Add workspace file tools - #81
Conversation
馃 Changeset detectedLatest commit: 260632a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for your interest in Cloudflare Computer. This repository does not accept unsolicited pull requests. Please use one of the accepted contribution paths instead:
If a maintainer asked you to open this pull request, they can add the |
commit: |
88b9b07 to
8166cb0
Compare
8166cb0 to
6f86cf0
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| path, | ||
| async () => { | ||
| try { | ||
| await store.remove(path, { recursive, force: true }); |
There was a problem hiding this comment.
馃煛 Deleting a path that does not exist reports success
Deletion always suppresses the missing-path error (force: true at packages/computer/src/tools/fs/delete.ts:30), so removing a path that is not there returns a success result instead of telling the caller nothing was found.
Impact: A model that mistypes a path is told the file was deleted and can proceed on the false belief that the file is gone.
How `force` swallows ENOENT
store.remove forwards to ws.fs.rm (packages/computer/src/tools/fs/store.ts:112-114), and rm returns silently when the node cannot be resolved and force is set (packages/dofs/src/fs/rm.ts:113-117). The tool then returns { deleted: path }. Since the tool input has no force flag, the caller has no way to get the ENOENT signal. Passing force: recursive === true (or not forcing at all, and mapping ENOENT to a structured error) would keep the useful ENOTEMPTY/ENOENT feedback.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
6dbf9dd to
c872ff0
Compare
c872ff0 to
ca97036
Compare
|
|
||
| const matches: WorkspaceGrepMatch[] = []; | ||
| const state: ScanState = { seen: 0, accepted: 0 }; | ||
| const filePaths = node.type === "file" ? [canonical] : filesUnder(db, canonical, options.include); |
There was a problem hiding this comment.
馃煛 Text search now returns results in a different file order than before
Files to search are collected in walk order instead of sorted path order (filesUnder(...) at packages/dofs/src/fs/grep.ts:71), so results for a directory come back in a different sequence than they used to.
Impact: Callers that relied on results arriving in path order, including anyone re-running a paged search built against the old order, see results grouped differently.
Depth-first walk order versus lexicographic path order
The previous implementation collected find(db, canonical) results, filtered to files, and called .sort(), giving lexicographic path order. The new filesUnder generator (packages/dofs/src/fs/grep.ts:116-124) yields entries in the depth-first order produced by walk in packages/dofs/src/fs/find.ts:93-118, which descends into a directory as soon as it is encountered.
These differ whenever a directory name is a prefix of a sibling file name. With /d/b/c.txt and /d/b.txt, dirents sort as b, b.txt, so the walk yields /d/b/c.txt before /d/b.txt, while lexicographic order puts /d/b.txt first (. is 0x2E, / is 0x2F). The existing test "applies offset and limit across files in path and line order" only uses root-level files, so it does not catch the difference. The new order is still deterministic, so pagination within one order remains consistent; the change is in the contract, not in stability. If path order is still intended, the walk results need sorting (which costs the streaming benefit) or the documented ordering needs updating.
Prompt for agents
packages/dofs/src/fs/grep.ts previously sorted the candidate file paths lexicographically before scanning; it now consumes iterateFoundEntries lazily, which yields depth-first walk order. The two orders differ when a directory name is a prefix of a sibling file name (for example /d/b/c.txt is yielded before /d/b.txt). Decide whether grep's documented result ordering is path order or traversal order, then either restore sorting (accepting that it materializes the file list again) or update the documentation and the test named "applies offset and limit across files in path and line order" so the intended contract is explicit.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
ca97036 to
5e62be9
Compare
5e62be9 to
27b4253
Compare
Share a store-scoped, normalized-path lock between edit and write so read-modify-write cycles cannot clobber concurrent writes or block unrelated workspaces.
Expose the missing workspace tools, keep find and grep available in read-only mode, bound their result pages, and serialize delete with other mutations.
Allow any non-negative grep offset so continuation values emitted after large result sets remain valid inputs to the next tool call.
Record the new search and deletion tools and shared mutation locking with the Computer package that exposes them.
27b4253 to
5391ec5
Compare
1 participant
Stack 3 of 7. Base:
stack/improve-tools-02-computer-fs-plumbing. Head:stack/improve-tools-03-file-tools.The Computer tool set lacked recursive path search, text search, and deletion. Writes could also race an edit between its read and write phases, losing one caller's update.
This change adds
find,grep, anddeletetocreateAITools. Search remains available in read-only mode, while deletion follows the existing read-only policy for mutations.edit,write, anddeletenow share a store-scoped lock keyed by normalized path. Search responses are bounded and return offsets that remain valid for result sets of any size.Verification
npm run typecheck --workspace @cloudflare/computer npm test --workspace @cloudflare/computer -- src/tools/ai.test.tsThe next stack part improves
readoutput and adds model-facing image and PDF content.