Skip to content

Fixes for latest issues - #124

Open
aron-cf wants to merge 4 commits into
mainfrom
fixes
Open

Fixes for latest issues#124
aron-cf wants to merge 4 commits into
mainfrom
fixes

Conversation

@aron-cf

@aron-cf aron-cf commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

This closes four reports: #118, #119, #120 and #121. Three of them change what Workspace.fs offers callers and a bug.

For #121, mkdir walked the path with its own helper, which read each directory entry directly and treated anything that wasn't a directory as an error. So if you pointed a symbolic link at a directory, mkdir refused to create anything underneath it.

Every other write path follows links, writeFile included. mkdir now does the same. It also tracks the resolved path as it walks, so the new directory lands under the directory the link points at instead of next to the link.

  • The read-only mount guard checks that resolved location too.
  • Recursive creation puts its missing ancestors there too.

A parent that turns out to be a file still reports ENOTDIR, a dangling parent link reports ENOENT instead of creating whatever the link pointed at, and a loop reports ELOOP.

await fs.mkdir("/real");
await fs.symlink("/real", "/alias");
await fs.mkdir("/alias/new-directory"); // creates /real/new-directory

find checked its glob before returning an entry, but it walked into every directory either way. Search a workspace with node_modules or .git in it.

You can now pass globs to exclude matched against the same path relative to where the search started. Excluding beats including, so an excluded entry never comes back.

The check runs before the walker asks a directory for its children, so an excluded directory takes its whole subtree with it and nothing under it is ever read. The order is unchanged, and limit and offset count what's left.

const sources = await workspace.fs.find("/workspace", "**/*.ts", {
  exclude: ["node_modules", "node_modules/**", ".git", ".git/**"],
});

The store has been able to move files, directories and symbolic links in one transaction for a while now, and it handles:

  • replacing what's already there
  • refusing a directory that still has children
  • respecting read-only mounts
  • stamping revisions so sync notices

None of that reached Workspace.fs. Callers had to copy the source and then delete it, and the Worker shell did exactly that for mv. If it failed in between, you were left with the bytes at both paths, or half a directory.

rename is on the public surface now, and on the stub as well, so a caller over remote procedure calls sees the same surface as one running in the same process. The shell uses it, and only falls back to copy-then-delete when rename won't replace the destination, which happens when the shell is merging one tree into another.

Nothing new crosses the connection to the container, because sync already carries the entries and tombstones a move produces.

// Publish a build atomically.
await fs.writeFile("/workspace/site/index.html.tmp", html);
await fs.rename("/workspace/site/index.html.tmp", "/workspace/site/index.html");

It replaces the destination when both ends are the same sort of thing, so a file or symbolic link can replace a file or symbolic link, and a directory can replace an empty one. Anything else gets the error you'd expect: ENOENT, ENOTEMPTY, EISDIR, ENOTDIR, EINVAL or EROFS.

The documentation now covers the full implementation. Each method gets its return value and its errors, and the comparison against node:fs/promises. The note on symbolic links now gives the two rules that cover everything. A link in the middle of a path is always followed. A link at the end is followed by everything except lstat and readlink, which exist to describe the link itself.

await fs.symlink("../shared/config.json", "/workspace/app/config.json");
await fs.readlink("/workspace/app/config.json"); // "../shared/config.json"
(await fs.lstat("/workspace/app/config.json")).isSymbolicLink; // true
await fs.chmod("/workspace/bin/run.sh", 0o755);

To check the work, run the suites in both environments. The Node run uses an in-process SQLite database, the Workers run drives a real durable object, and that difference is why both are worth running.

npm run build
npm test --workspace @cloudflare/dofs                # 563 tests
npm run test:workers --workspace @cloudflare/dofs    # 552 tests
npm test --workspace @cloudflare/computer            # 1142 tests

There are four changesets, one per report. One thing I left alone: grep shares the walk with find and would take the same option almost for free, but I kept its signature as it is so this change closes the four reports and nothing else.


Devin Review

agent added 4 commits August 25, 2026 08:12
mkdir walked the target path with a helper that read each dirent
directly and treated every node other than a directory as ENOTDIR, so
a symbolic link to a directory blocked directory creation rather than
resolving through it. Every other write path, writeFile included,
follows links and shares one forty-hop budget.

The parent walk now expands intermediate links the way writeFile's
does, tracking the resolved path so the new directory lands under the
directory the link points at and the read-only mount guard sees the
location actually written. Recursive creation places its missing
ancestors under that resolved parent. A resolved parent that is a file
still reports ENOTDIR, a dangling parent link reports ENOENT in both
modes rather than being materialised, and a chain beyond forty hops
reports ELOOP.

Closes #119.
The walker tested the inclusion glob before yielding an entry but
descended into every directory regardless, so a search in a workspace
holding node_modules, .git, or generated build output paid for those
trees even when the caller wanted nothing from them.

FindOptions gains exclude, a list of globs of the same shape as the
inclusion pattern and matched against the same directory-relative
path. An exclusion is decided before inclusion, so it always wins, and
before any child query, so an excluded directory takes its whole
subtree with it rather than being filtered out afterwards. Traversal
stays deterministic and limit and offset apply to what survives. The
option reaches the public find tool, whose schema now advertises it.

Closes #121.
The store has implemented transactional file, directory, and symbolic
link moves for some time, covering destination replacement, non-empty
directories, read-only mounts, tombstones, revision stamping, and
subtree tracking. None of that reached Workspace.fs, so a caller had
to copy the source and then delete it, and the Worker shell used that
fallback for mv. A failure between the two steps left the entry at
both paths or a directory half copied.

WorkspaceFilesystem now forwards rename, and WorkspaceFilesystemStub
mirrors it with the usual filesystem observation span, so the Workers
RPC surface matches the in-process one. The shell adapter calls it and
keeps copy-then-delete only for a destination rename refuses to
replace, which is what the shell expects when it merges a tree. No new
method crosses the Cap'n Web boundary: the existing synchronisation
protocol already carries the resulting live entries and tombstones.

Closes #120.
The filesystem specification said symbolic links were an internal
primitive, that Workspace.fs exposed neither symlink nor readlink,
that there was no lstat, and that an existing file's mode could not be
changed. The shipped API contradicts all four: WorkspaceFilesystem
exposes symlink, readlink, lstat, and chmod, the stub mirrors them
across the Workers RPC boundary, and the Dynamic Worker filesystem
adapters rely on them for the node:fs behaviour a shell expects.
Removing the methods would be a breaking change and would leave those
adapters without a way to serve ln -s, readlink, or test -L, so the
document follows the code.

Each of the four methods gains a section with its return value and its
errors, the comparison with node:fs/promises maps them rather than
striking them out, and the note on symbolic links now states the two
rules that cover the surface: intermediate segments are always
followed, and a trailing link is followed by everything except lstat
and readlink. The rename and find entries added alongside are
documented in the same pass.

Closes #118.
@changeset-bot

changeset-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2201c59

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@cloudflare/computer Minor
@cloudflare/dofs Minor
@cloudflare/computer-rpc Minor
@cloudflare/computerd Minor

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

@pkg-pr-new

pkg-pr-new Bot commented Aug 28, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@cloudflare/computer@124

commit: 2201c59

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

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