Skip to content

computerd: Add support for an ignore field to the ContainerBackend - #187

Open
aron-cf wants to merge 7 commits into
mainfrom
computerd-ignore
Open

aron-cf wants to merge 7 commits into
mainfrom
computerd-ignore

Conversation

@aron-cf

@aron-cf aron-cf commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

This adds a new ignore flag to the CloudflareContainerBackend class, all directories will write directly through to disk rather than be included in the fuse mount.

new CloudflareContainerBackend({
  container: env.CONTAINER,
  workspace: { binding: "SESSIONS", id: sessionId },
  ignore: ["/node_modules", "/.venv", "/dist"],
});

It does this by setting MOUNT_IGNORE in the container environment which is picked up by computerd.

The MOUNT_IGNORE_PATH can also be set which will be the path to the directory that holds the actual files. It defaults to /tmp/$MOUNT_POINT. This allows you to backup/restore this directory if needed.


Devin Review

Everything a container command writes under MOUNT_POINT is recorded in
the VFS and pulled into the Durable Object after the command. That is
right for source and wrong for node_modules, .venv, target/ and dist/:
tens of thousands of rebuildable files that never need to be durable,
and no way to exclude them.

This is the path set for #179, and only that. Nothing calls it yet.
The decision cache and the passthrough I/O that will consume it belong
in driver.ts and land separately; keeping the pure part apart is what
lets its semantics be pinned with no mount, no syscalls and no
container.

Entries are plain paths relative to the mount root. No glob syntax and
no negation: an entry names one location, and a path is local-only if
it equals that entry or sits underneath it.

The simplicity is the design. An entry resolves to a known location,
so the mapping onto MOUNT_IGNORE_PATH is a prefix substitution fixed
at startup rather than a question that can only be answered once a
path arrives. Matching reduces to a segment-aware prefix test, which
the driver's per-inode cache can collapse to one lookup per directory.
And the set of paths that silently lose durability stays reviewable by
reading it, which matters more than expressiveness when the cost of a
wrong entry is data that exists only inside one container.

The limitation is that an entry does not match at every depth: a
monorepo lists app/node_modules and web/node_modules rather than
writing node_modules once. That is more lines in a Dockerfile. If
depth matching is ever needed, a single leading `**/` form is the
smallest addition that stays resolvable.

Entries are normalised once: slashes stripped, absolute paths under
the mount accepted and made relative, "." and ".." rejected rather
than resolved so an escaping entry cannot hide behind a path that
looks intentional, and duplicates or entries nested inside another
dropped as redundant. The normalised list is what the mount applies
and what diagnostics should report.

Verified by mutation. Four mutants, all killed: the segment boundary
replaced by a plain startsWith, descendant matching reduced to
equality, ".." rejection skipped, and nested-entry collapsing
disabled. The first is the one that matters -- a naive startsWith
passes every other test in the file and reports node_modules_extra as
living under node_modules.
Wires the path set added in the previous commit into the FUSE op
layer, so configuring MOUNT_IGNORE now does something. Matching paths
are served from MOUNT_IGNORE_PATH (default /tmp/$MOUNT_POINT) instead
of the VFS: never recorded, never pushed, never pulled.

Implemented as a decorator over FuseOps rather than branches inside
makeFUSEOps. The VFS driver stays unaware of the feature, so a bug
here is bounded by the ignore set; disabling it is provably free,
because an empty set returns the source object unchanged; and it
composes the way the tracer already does.

Ignored-ness is decided per directory and inherited. A node_modules
tree is tens of thousands of entries under a handful of directories,
so without inheritance every lookup would re-test the entry list. The
cache is invalidated when a directory is removed, or a recreated path
would keep a stale decision and silently land in the wrong layer.

Local file handles are allocated from a high range so they cannot
collide with the VFS driver's, which counts from 1. A handle that
crossed layers would read one file and write another.

A rename across the boundary returns EXDEV. The two sides are
different filesystems, so the operation cannot be atomic, and copying
underneath would turn a crash mid-copy into a half-written file where
the caller was promised all-or-nothing. Tools already handle EXDEV by
falling back to copy-then-unlink. Within one layer it is a real
rename.

Configuration fails closed at startup: a root that is relative, equal
to or inside MOUNT_POINT, or the filesystem root is refused, as is a
malformed entry. A silently dropped entry would send a full
node_modules into the Durable Object, which is the failure this
prevents. The root defaults under /tmp so a container snapshot
captures it, since that is the only durability local-only content has.

/__computerd/info reports the resolved set, the entries dropped as
redundant, and fastPaths. Passthrough is reported false with its
reason rather than omitted: the host kernel supports FOPEN_PASSTHROUGH
but fuse-native binds libfuse 2.9, which cannot negotiate it, so this
flips on a binding change rather than an infrastructure one. Writeback
caching is unavailable for the same reason.

Verified by mutation. Six mutants, all killed: EXDEV turned into a
silent copy-through, the local handle range overlapped with the VFS's,
parent creation removed, the decision cache left un-invalidated on
rmdir, inheritance disabled, and the readdir merge dropped.

The package suite is now fully green (255 passing). The 32 previously
failing cli tests were spawning an unbuilt binary; building it as part
of this work let them run, and the one real break -- an exact-equality
assertion on /__computerd/info -- is updated to cover the new block.
Adds `ignore` to CloudflareContainerBackend and exposes the resolved
set as `handle.ignore`. The backend reads /__computerd/info during
connect() and refuses the connection when the container disagrees with
what the caller declared.

The option is a declaration, not a setting. The set belongs to the
image, which reads MOUNT_IGNORE at startup; a client cannot change it.
Making it configurable here would promise a per-session knob the
architecture cannot honour, because the mount is per-container and
compiled once -- two sessions sharing an image cannot hold different
views of which paths are durable.

Failing the connection rather than warning is deliberate. The failure
being guarded is silent and slow: an image built without MOUNT_IGNORE,
or carrying a stale set, is indistinguishable from a correct one until
a command writes a large dependency tree and the whole thing is pulled
into the Durable Object. That is the symptom #179 reports -- a command
timeout, then a storage-timeout cascade the workspace does not recover
from. A deployment error is cheaper discovered loudly.

The check runs before the handle is published, and tears the transport
down on rejection. Running it afterwards would let the first exec write
into a path the caller believes is local-only, which is the state that
is expensive to find later.

Omitting `ignore` skips the check and accepts whatever the container
provides, so this cannot break an existing deployment. A failure to
reach /__computerd/info is likewise reported as unsupported rather than
propagated: the endpoint is diagnostic, and a caller that declared
nothing should not lose a working connection to a failed diagnostic
request. A caller that did declare still fails.

Comparison is order-insensitive and tolerates slash decoration on
either side, because computerd normalises its own set and a host
listing the same paths differently means the same thing. Both
directions are reported: a path the image does not apply will be
synced unexpectedly, and one it applies but the caller did not declare
will not be synced when the caller thinks it is.

Verified by mutation. Five mutants, all killed: accepting an
unsupported container, never throwing on mismatch, checking even when
the declaration is omitted, comparing only one direction, and dropping
slash normalisation.
A rename between a MOUNT_IGNORE path and a synced one returns EXDEV.
The kernel surfaces that to the caller as "cross-device link", which
on a path that is plainly not a device is the kind of message an
operator loses an afternoon to.

The errno is all the FUSE callback can carry, so the guidance goes to
the log instead: both sides named, why the rename cannot be atomic,
and the entry to add to MOUNT_IGNORE to make it atomic again. Build
tools that stage into a sibling and rename into place are the common
cause, and ignoring the staging path alongside its destination is
almost always the fix.

Logged once per mount, not once per rename. A build that does this
does it in a loop, and a line per occurrence would bury everything
else in the log. The full count stays available on the passthrough
stats for anyone who wants it.

Verified by mutation: removing the once-guard, removing the call, and
swapping which side is reported as local-only each fail a test.
Adds docs/20_local_only_paths.md covering MOUNT_IGNORE: what it does,
the per-image rule, validation, diagnostics, and the EXDEV rename
contract.

The durability trade-off leads rather than trails. These paths are
invisible to workspace.fs and the worker shell, and survive container
replacement only through a snapshot -- which is the right trade for a
dependency tree a package manager can rebuild and the wrong one for
anything a user typed. A reader deciding whether to use this needs
that before the syntax, not after it.

EXDEV gets its own section because it is the one runtime failure a
correctly configured deployment can still hit. It explains why the
copy is refused rather than performed -- faking atomicity turns a
crash mid-copy into a silent half-written file -- and gives the fix,
with the staging-directory case called out since that is how build
tools produce it.

Also records what these paths do not make faster. The saving is the
transfer, not the I/O: the bytes still cross the FUSE boundary,
because passthrough needs libfuse 3.17 and fuse-native binds 2.9.
Without that, the first benchmark against raw disk reads as a
regression.

19_performance.md gains a matching note. Its npm install table stops
when the install returns and does not count the pull into the Durable
Object, which for a dependency tree is the larger cost and is the part
#179 reports timing out. The ignored-vs-not table is added with its
cells deliberately empty: bytes-pulled is the load-bearing number for
this feature and has not been measured yet, and an invented figure in
a performance document is worse than a visible gap.
Drop the U2/U3/U5 milestone markers. They refer to phases in a planning
doc that is not published with this repo, so a reader has no way to
resolve them. Issue references are kept.

Cut comments that restate the code or duplicate docs/20_local_only_paths.md,
and repoint a dangling "See DESIGN" at that doc. What stays is the
reasoning a reader cannot recover from the source: why MOUNT_IGNORE is
newline-delimited, why EXDEV is refused rather than copied, why the
assertion fails the connection instead of warning, and why the store
defaults under /tmp.

Comments only; no behaviour change.
…aths

Three changes to the local-only path interface.

MOUNT_IGNORE is now a comma-separated list rather than newline-delimited,
so it reads naturally as a single environment variable:

  MOUNT_IGNORE=/node_modules,/.venv,/dist

A leading slash now anchors at the mount root rather than the filesystem
root, so "/node_modules" means "$MOUNT_POINT/node_modules". The
fully-qualified form ("/workspace/dist") is still accepted. A path
containing a comma can no longer be expressed, and an entry that looks
like it names somewhere else on disk is reinterpreted as mount-relative
rather than rejected.

The backend's `ignore` option now configures rather than asserts: it is
passed to the container in its start environment, so the set belongs to
the deployment instead of the image and changing it needs no rebuild. An
explicit MOUNT_IGNORE in containerEnv still wins. connect() continues to
read the resolved set back and reject a disagreement.

handle.ignore.paths are now absolute container paths
("/workspace/node_modules") rather than mount-relative, and the handle
carries mountPoint so the comparison can still be made on equal footing.
@aron-cf aron-cf added the allow-pr Allow a PR to remain open. label Oct 1, 2026
@changeset-bot

changeset-bot Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 66a95e3

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/computerd Minor
@cloudflare/dofs Minor
@cloudflare/computer-rpc 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

@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.

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 13 potential issues.

Devin Review

Comment on lines +586 to +590
const res = await host.fetchPort(
this.#options.containerPort,
"http://container/__computerd/info",
{ signal: AbortSignal.timeout(this.#options.healthProbeTimeoutMs) },
);

@devin-ai-integration devin-ai-integration Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Configured local-only paths block every connection

With ignore configured, #resolveIgnore receives 401 because isAuthorized protects the info endpoint. assertIgnoreMatches then rejects every connection as unsupported.

Learn more

The daemon applies the bearer check to all routes except /health, including /__computerd/info (isAuthorized). The backend has clientSecret after host.start(), but its diagnostic request omits the bearer header. A 401 is converted to supported: false, and assertIgnoreMatches refuses the connection whenever ignore is set.

Example: With ignore: ['/node_modules'], the daemon starts with that path, then rejects the diagnostic GET with 401. The backend throws a mismatch despite the path being active.

Recommended fix: Pass the clientSecret into #resolveIgnore and include Authorization: Bearer ${clientSecret} in its fetch. Test the full authenticated connection path, not just the fake's unauthenticated endpoint.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 693 to 695
shim = await mountShim({ vfs, mountPoint });
fuse = shim;
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Shim fallback silently syncs ignored paths

When FUSE_MOUNT=auto selects the shim, ignoreConfig still reports ignored paths as active. The shim receives no local routing, so those paths enter the VFS and sync despite a matching backend assertion.

(Refers to this code)

Learn more

A configured MOUNT_IGNORE is resolved and reported through describeMountIgnore regardless of which mount mode starts. The real FUSE branch supplies localPaths; the shim branch does not. In an environment without /dev/fuse, auto chooses shim and its disk-to-VFS reconciliation pulls ignored content into the store. The backend compares only the reported paths, so the mismatch is invisible.

Example: Set FUSE_MOUNT=auto and MOUNT_IGNORE=node_modules without /dev/fuse. Writing /workspace/node_modules/a through the shim synchronizes a despite an affirmative info report.

Recommended fix: Implement the ignore boundary in the shim as well, or refuse to start when ignore entries are configured but the selected backend cannot enforce them; report only applied configuration.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +447 to +458
ftruncate(path, fh, size, cb) {
if (!isLocalHandle(fh)) {
ops.ftruncate(path, fh, size, cb);
return;
}
localOps += 1;
try {
fs.truncateSync(localPath(path), size);
cb(0);
} catch (error) {
cb(toErrno(error));
}

@devin-ai-integration devin-ai-integration Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Truncation targets a replacement file

After an ignored file is renamed or replaced, ftruncate truncates its old pathname instead of the open descriptor. It can truncate another file or fail despite a valid open handle.

Learn more

An ignored file's fh maps to a real disk descriptor in handles. ftruncate ignores that descriptor and uses localPath(path), which is no longer the same inode after a rename or replacement. This breaks the descriptor-based truncate contract and can destroy bytes in a different file.

Example: Open node_modules/a, rename it to node_modules/b, create a new node_modules/a, then truncate the old handle. The new a is truncated instead of the open file now named b.

Recommended fix: Look up fh in handles, reject unknown or directory descriptors, and use ftruncateSync(handle.fd, size); add an open-then-rename/replace regression test.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +425 to +431
fsync(path, fh, datasync, cb) {
if (!isLocalHandle(fh)) {
ops.fsync(path, fh, datasync, cb);
return;
}
cb(0);
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Local fsync falsely confirms durable writes

For ignored files, fsync returns success without flushing the descriptor. An abrupt crash can lose writes that an application already committed with fsync.

Learn more

The ignored side writes through Node's writeSync, which writes into the kernel's filesystem cache rather than ensuring stable storage. fsync currently returns success without performing any disk flush. Applications using fsync(2) as a commit boundary cannot rely on that boundary for local-only content.

Example: A program writes a dependency lock artifact under an ignored directory, calls fsync, and the container crashes before the kernel writes dirty pages. The acknowledged data can disappear.

Recommended fix: Use fsyncSync or fdatasyncSync on the open descriptor according to datasync, mapping errors to FUSE errno.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +612 to +615
const destination = localPath(path);
ensureParent(destination);
fs.symlinkSync(target, destination);
cb(0);

@devin-ai-integration devin-ai-integration Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Local symlinks cannot reach synced files

A relative link from ignored node_modules into synced src resolves against MOUNT_IGNORE_PATH, not the mount. symlink preserves the text, so tools following that link cannot find the synced file.

Learn more

A local-only directory has two path spaces: its mount path and its backing disk path. symlinkSync creates the link on the disk, so relative targets are interpreted relative to the backing disk rather than the visible mount directory. Cross-boundary symlinks used by package managers therefore resolve to the wrong target. The localPath mapping does not rewrite those targets.

Example: /workspace/node_modules/.bin/tool links to ../../src/tool.js. The corresponding backing link at /tmp/workspace/node_modules/.bin/tool resolves to /tmp/workspace/src/tool.js, not /workspace/src/tool.js.

Recommended fix: Define and test cross-boundary symlink semantics; preserve mount-namespace resolution for links instead of blindly following links in the backing filesystem.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +318 to +327
opendir(path, flags, cb) {
if (!isLocal(path)) {
ops.opendir(path, flags, cb);
return;
}
localOps += 1;
// Directory handles carry no fd: readdir re-resolves by path, and
// holding an O_PATH fd per open directory would leak under a
// recursive walk of a large dependency tree.
cb(0, allocateHandle(-1, path));

@devin-ai-integration devin-ai-integration Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Directory opens defer path errors

opendir accepts an ignored path without checking whether it exists or is a directory. A later readdir reports the error instead.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +621 to +632
access(path, mode, cb) {
if (!isLocal(path)) {
ops.access(path, mode, cb);
return;
}
localOps += 1;
try {
fs.lstatSync(localPath(path));
cb(0);
} catch (error) {
cb(toErrno(error));
}

@devin-ai-integration devin-ai-integration Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Local access ignores permission requests

access reports success when an ignored path exists, regardless of the requested mode. Check X_OK and W_OK behavior for consumers.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +113 to +121
// Paths the container keeps on its local disk instead of the
// workspace (#179). Written as mount-relative absolute paths
// ("/node_modules"), and passed to the container at start time as
// MOUNT_IGNORE.
//
// connect() reads the resolved set back off /__computerd/info and
// refuses the connection if it disagrees, which catches an image
// whose computerd is too old to honour the variable.
ignore?: readonly string[];

@devin-ai-integration devin-ai-integration Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Container example omits the new option

The public ignore option appears in documentation but not examples/container. Repository guidelines ask for examples to track public API changes.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +988 to +991
const routedOps =
options.localPaths === undefined
? baseOps
: withLocalPassthrough(baseOps, options.localPaths).ops;

@devin-ai-integration devin-ai-integration Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Description overstates disk routing

The PR description says all directories write directly to disk. The implementation routes only configured ignored paths to disk under real FUSE.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +228 to +235
const ensureParent = (target: string): void => {
const parent = dirname(target);
try {
fs.mkdirSync(parent, { recursive: true, mode: DEFAULT_DIR_MODE });
options.onMaterialise?.(parent);
} catch (error) {
if (errnoOf(error) !== "EEXIST") throw error;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟥 Ignored-directory symlinks escape local storage root

When an ignored directory contains a symlink, localPath-based operations follow it outside MOUNT_IGNORE_PATH. A container command can then access files elsewhere on the disk through the mount.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@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 found 3 new potential issues.

Devin Review

Comment on lines +175 to +178
`EXDEV` is also not an exotic error. It is what any Unix returns for a
cross-device rename, so `mv`, Node's `fs.rename`, Python's
`shutil.move`, and Go's `os.Rename` callers already fall back to
copy-then-unlink. Most tools recover without noticing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Rename fallback guidance overstates recovery

Node's fs.rename and Go's os.Rename return EXDEV; they do not perform copy-then-unlink. Callers need their own fallback, so the recovery guidance can mislead users diagnosing failed builds.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +208 to +209
It logs once because a build that does this does it in a loop. The
per-mount count is available on the passthrough stats.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Rename count is not exposed

The count exists in withLocalPassthrough, but mountFuse discards its stats accessor. Operators cannot read this per-mount count from the daemon's stats endpoint.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +5 to +6
> Shipped behaviour as of `@cloudflare/computerd` with `MOUNT_IGNORE`
> support; the client-side assertion ships in `@cloudflare/computer`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Documentation spelling conflicts with prose rules

The prose rules require American English. This guide uses behaviour, honour, and normalised; the new changeset and revised code comment also use honour.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-pr Allow a PR to remain open.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant