From 32369f4f4ca4f49138b2a78127b90c5874e9d3bd Mon Sep 17 00:00:00 2001 From: Aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 08:42:46 +0000 Subject: [PATCH 01/15] computerd: add local-only mount paths 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. --- packages/computerd/src/fuse/ignore.test.ts | 205 +++++++++++++++++++ packages/computerd/src/fuse/ignore.ts | 218 +++++++++++++++++++++ packages/computerd/src/fuse/index.ts | 2 + 3 files changed, 425 insertions(+) create mode 100644 packages/computerd/src/fuse/ignore.test.ts create mode 100644 packages/computerd/src/fuse/ignore.ts diff --git a/packages/computerd/src/fuse/ignore.test.ts b/packages/computerd/src/fuse/ignore.test.ts new file mode 100644 index 00000000..25eadbbb --- /dev/null +++ b/packages/computerd/src/fuse/ignore.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, test } from "vitest"; + +import { MountIgnorePathError, parseMountIgnore, resolveMountIgnore } from "./ignore.js"; + +// Local-only subpaths for #179. Entries are plain paths relative to +// the mount root: no globs, no negation, no depth matching. +// +// The case worth writing first is the segment boundary. A naive +// `startsWith` passes every other test in this file and fails +// "does not treat node_modules_extra as node_modules", so that test +// is what actually pins the matcher. + +describe("parseMountIgnore", () => { + test("splits MOUNT_IGNORE on newlines", () => { + expect(parseMountIgnore("node_modules\n.venv\ntarget")).toEqual([ + "node_modules", + ".venv", + "target", + ]); + }); + + test("skips blank lines and comments the way .gitignore does", () => { + const parsed = parseMountIgnore( + ["# build output", "", "dist", " ", "# deps", "node_modules", ""].join("\n"), + ); + expect(parsed).toEqual(["dist", "node_modules"]); + }); + + test("keeps entries containing commas and spaces intact", () => { + // Why the env var is newline-delimited rather than comma- or + // space-separated: a path may legally contain either. + expect(parseMountIgnore("my dir\na,b.log")).toEqual(["my dir", "a,b.log"]); + }); + + test("trims trailing whitespace but preserves an escaped trailing space", () => { + expect(parseMountIgnore("dist \nkeep\\ ")).toEqual(["dist", "keep\\ "]); + }); + + test("treats an absent or empty value as the feature being off", () => { + expect(parseMountIgnore(undefined)).toEqual([]); + expect(parseMountIgnore("")).toEqual([]); + expect(parseMountIgnore("\n\n \n")).toEqual([]); + }); +}); + +describe("resolveMountIgnore: matching", () => { + test("matches the entry itself and everything under it", () => { + const set = resolveMountIgnore(["node_modules"]); + expect(set.ignores("node_modules")).toBe(true); + expect(set.ignores("node_modules/react")).toBe(true); + expect(set.ignores("node_modules/react/index.js")).toBe(true); + expect(set.ignores("node_modules/@scope/pkg/dist/x.js")).toBe(true); + }); + + test("does not match at arbitrary depth", () => { + // The deliberate limitation. `node_modules` names one location; + // a nested one must be listed explicitly. + const set = resolveMountIgnore(["node_modules"]); + expect(set.ignores("app/node_modules")).toBe(false); + expect(set.ignores("a/b/node_modules")).toBe(false); + }); + + test("matches a nested entry when it is listed", () => { + const set = resolveMountIgnore(["app/node_modules", "web/node_modules"]); + expect(set.ignores("app/node_modules")).toBe(true); + expect(set.ignores("app/node_modules/react/index.js")).toBe(true); + expect(set.ignores("web/node_modules")).toBe(true); + expect(set.ignores("api/node_modules")).toBe(false); + expect(set.ignores("node_modules")).toBe(false); + }); + + test("does not treat node_modules_extra as node_modules", () => { + // A plain startsWith check passes everything above and fails here. + const set = resolveMountIgnore(["node_modules"]); + expect(set.ignores("node_modules_extra")).toBe(false); + expect(set.ignores("node_modules_extra/x.js")).toBe(false); + expect(set.ignores("node_modulesX")).toBe(false); + }); + + test("does not match a prefix of an entry", () => { + const set = resolveMountIgnore(["build/output"]); + expect(set.ignores("build")).toBe(false); + expect(set.ignores("build/output")).toBe(true); + expect(set.ignores("build/output/app.js")).toBe(true); + expect(set.ignores("build/outputs")).toBe(false); + }); + + test("matches case-sensitively, as Linux does", () => { + const set = resolveMountIgnore(["node_modules"]); + expect(set.ignores("node_modules")).toBe(true); + expect(set.ignores("Node_Modules")).toBe(false); + }); + + test("tolerates leading and trailing slashes on the queried path", () => { + const set = resolveMountIgnore(["dist"]); + expect(set.ignores("/dist")).toBe(true); + expect(set.ignores("dist/")).toBe(true); + expect(set.ignores("/dist/app.js")).toBe(true); + }); + + test("ignores nothing when no entries are configured", () => { + const set = resolveMountIgnore([]); + expect(set.ignores("node_modules")).toBe(false); + expect(set.isEmpty).toBe(true); + expect(set.paths).toEqual([]); + }); + + test("reports the covering entry, for diagnostics and error messages", () => { + const set = resolveMountIgnore(["node_modules", "target"]); + expect(set.entryFor("node_modules/react/index.js")).toBe("node_modules"); + expect(set.entryFor("target/debug/app")).toBe("target"); + expect(set.entryFor("src/main.ts")).toBeUndefined(); + }); +}); + +describe("resolveMountIgnore: normalisation", () => { + test("strips leading and trailing slashes from entries", () => { + const set = resolveMountIgnore(["/dist/", "node_modules/"]); + expect(set.paths).toEqual(["dist", "node_modules"]); + expect(set.ignores("dist/app.js")).toBe(true); + }); + + test("accepts an absolute path inside the mount point", () => { + const set = resolveMountIgnore(["/workspace/dist"], "/workspace"); + expect(set.paths).toEqual(["dist"]); + expect(set.ignores("dist/app.js")).toBe(true); + }); + + test("rejects an absolute path outside the mount point", () => { + expect(() => resolveMountIgnore(["/etc/passwd"], "/workspace")).toThrow(MountIgnorePathError); + expect(() => resolveMountIgnore(["/etc/passwd"], "/workspace")).toThrow(/outside the mount/); + }); + + test("rejects a .. segment rather than resolving it", () => { + // Silently clamping would hide the mistake behind a path that looks + // intentional. + expect(() => resolveMountIgnore(["../escape"])).toThrow(MountIgnorePathError); + expect(() => resolveMountIgnore(["dist/../../etc"])).toThrow(/"\." or "\.\."/); + }); + + test("rejects a . segment", () => { + expect(() => resolveMountIgnore(["./dist"])).toThrow(MountIgnorePathError); + }); + + test("rejects an entry naming the mount root", () => { + // Ignoring everything would make the workspace entirely non-durable, + // which is never what someone means. + expect(() => resolveMountIgnore(["/"])).toThrow(MountIgnorePathError); + expect(() => resolveMountIgnore([""])).toThrow(MountIgnorePathError); + }); + + test("rejects an empty path segment", () => { + expect(() => resolveMountIgnore(["a//b"])).toThrow(MountIgnorePathError); + }); + + test("reports the entry index so a long MOUNT_IGNORE is diagnosable", () => { + try { + resolveMountIgnore(["ok", "also-ok", "../bad"]); + expect.unreachable("resolve should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(MountIgnorePathError); + expect((error as MountIgnorePathError).index).toBe(2); + expect((error as MountIgnorePathError).entry).toBe("../bad"); + } + }); +}); + +describe("resolveMountIgnore: redundancy", () => { + test("drops a duplicate entry", () => { + const set = resolveMountIgnore(["dist", "dist"]); + expect(set.paths).toEqual(["dist"]); + expect(set.redundant).toEqual(["dist"]); + }); + + test("drops an entry nested inside an earlier one", () => { + // Keeping node_modules/.cache alongside node_modules would imply it + // does something, and it cannot. + const set = resolveMountIgnore(["node_modules", "node_modules/.cache"]); + expect(set.paths).toEqual(["node_modules"]); + expect(set.redundant).toEqual(["node_modules/.cache"]); + expect(set.ignores("node_modules/.cache/x")).toBe(true); + }); + + test("subsumes earlier entries when a broader one arrives later", () => { + const set = resolveMountIgnore(["app/node_modules", "app"]); + expect(set.paths).toEqual(["app"]); + expect(set.redundant).toEqual(["app/node_modules"]); + expect(set.ignores("app/node_modules/react")).toBe(true); + expect(set.ignores("app/src/main.ts")).toBe(true); + }); + + test("keeps siblings that merely share a prefix string", () => { + // `dist` and `dist-types` are unrelated locations despite the + // common prefix; neither is redundant. + const set = resolveMountIgnore(["dist", "dist-types"]); + expect(set.paths).toEqual(["dist", "dist-types"]); + expect(set.redundant).toEqual([]); + }); + + test("normalises before deduplicating", () => { + const set = resolveMountIgnore(["/dist/", "dist"]); + expect(set.paths).toEqual(["dist"]); + expect(set.redundant).toEqual(["dist"]); + }); +}); diff --git a/packages/computerd/src/fuse/ignore.ts b/packages/computerd/src/fuse/ignore.ts new file mode 100644 index 00000000..f49b67ba --- /dev/null +++ b/packages/computerd/src/fuse/ignore.ts @@ -0,0 +1,218 @@ +// Local-only subpaths of the mount. +// +// Addresses #179: 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. Paths listed here +// pass through to local disk instead, are never recorded in the VFS, +// and are never pushed or pulled. +// +// Entries are plain paths relative to the mount root. There is 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, not a shortcut. Three things follow +// from it that a pattern language does not give you: +// +// - An entry resolves to a known location, so the mapping onto +// MOUNT_IGNORE_PATH is a prefix substitution decided at startup. +// An unanchored pattern has no single answer to "where does this +// live on disk" until a path arrives to match against it. +// - Matching is a segment-aware prefix test, which the driver's +// per-inode decision cache collapses to one lookup per directory. +// - The set of paths that silently lose durability is reviewable by +// reading it. That matters more here than expressiveness, because +// the cost of a wrong entry is data that exists only inside one +// container. +// +// The one real limitation is that `node_modules` does not match at +// every depth. A monorepo cloning packages into app/, web/ and api/ +// lists each `/node_modules`. That is more lines in a Dockerfile +// and nothing more. If depth matching is ever needed, a single +// leading `**/` form is the smallest addition that stays resolvable; +// add it on evidence rather than in anticipation. +// +// The set is fixed for the life of the mount. It is resolved once at +// startup from MOUNT_IGNORE and never re-read: entries that changed +// under a running command would mean migrating already-materialised +// paths between layers mid-write. + +/** An entry that cannot be used, carrying enough context to fix it. */ +export class MountIgnorePathError extends Error { + readonly entry: string; + /** Index into the entry list, so a long MOUNT_IGNORE is diagnosable. */ + readonly index: number; + + constructor(message: string, entry: string, index: number) { + super(message); + this.name = "MountIgnorePathError"; + this.entry = entry; + this.index = index; + } +} + +export interface MountIgnoreSet { + /** + * Whether a mount-relative path is local-only. + * + * True when the path equals an entry or is a descendant of one. + * Matching is segment-aware, so the entry `node_modules` does not + * match `node_modules_extra`. + */ + readonly ignores: (relativePath: string) => boolean; + /** + * The entry covering a path, for error messages and diagnostics. + * Undefined when the path is not local-only. + */ + readonly entryFor: (relativePath: string) => string | undefined; + /** Normalised entries, in declaration order, as the mount applies them. */ + readonly paths: readonly string[]; + /** Entries dropped as duplicates or as nested inside another entry. */ + readonly redundant: readonly string[]; + readonly isEmpty: boolean; +} + +/** + * Splits a raw MOUNT_IGNORE value into entries. + * + * Newline-delimited rather than comma- or space-separated because a + * path may legally contain a comma or a space. Blank lines and `#` + * comments are skipped so a generated block stays readable in a + * Dockerfile ENV. + */ +export function parseMountIgnore(raw: string | undefined): string[] { + if (raw === undefined) return []; + const entries: string[] = []; + for (const line of raw.split("\n")) { + // Trailing whitespace is insignificant unless escaped, which is the + // only way to name a path ending in a space. + const trimmed = line.endsWith("\\ ") ? line.trimStart() : line.trim(); + if (trimmed === "") continue; + if (trimmed.startsWith("#")) continue; + entries.push(trimmed); + } + return entries; +} + +/** + * Normalises entries and builds the matcher. + * + * `mountPoint` lets an absolute path under the mount be written as + * `/workspace/dist`, which is the obvious thing to reach for. An + * absolute path outside the mount is rejected rather than reinterpreted. + */ +export function resolveMountIgnore(entries: readonly string[], mountPoint = "/"): MountIgnoreSet { + const root = normaliseMount(mountPoint); + const paths: string[] = []; + const redundant: string[] = []; + + for (const [index, original] of entries.entries()) { + let value = original.trim(); + + if (value.startsWith("/")) { + // Absolute. Accept it only if it names something inside the mount. + if (root !== "/" && (value === root || value.startsWith(`${root}/`))) { + value = value.slice(root.length); + } else if (root !== "/") { + throw new MountIgnorePathError( + `Entry ${JSON.stringify(original)} is an absolute path outside the ` + + `mount point ${JSON.stringify(root)}. Entries name paths within the mount.`, + original, + index, + ); + } + } + + const trimmed = stripSlashes(value); + if (trimmed === "") { + throw new MountIgnorePathError( + `Entry ${JSON.stringify(original)} resolves to the mount root. ` + + `Ignoring the whole mount would make the workspace non-durable.`, + original, + index, + ); + } + + const segments = trimmed.split("/"); + // `.` and `..` are rejected rather than resolved. An entry that walks + // out of the mount is a configuration mistake, and silently clamping + // it would hide the mistake behind a path that looks intentional. + if (segments.some((segment) => segment === "." || segment === "..")) { + throw new MountIgnorePathError( + `Entry ${JSON.stringify(original)} contains a "." or ".." segment. ` + + `Entries must be plain paths relative to the mount root.`, + original, + index, + ); + } + if (segments.some((segment) => segment === "")) { + throw new MountIgnorePathError( + `Entry ${JSON.stringify(original)} contains an empty path segment.`, + original, + index, + ); + } + + // Duplicates and entries nested inside an existing one are dropped: + // keeping `node_modules/.cache` alongside `node_modules` would imply + // it does something, and it cannot. + const covered = paths.some((existing) => isAtOrUnder(trimmed, existing)); + if (covered) { + redundant.push(original); + continue; + } + + // The converse: a new entry may subsume ones already accepted. + for (let position = paths.length - 1; position >= 0; position -= 1) { + const existing = paths[position] as string; + if (isAtOrUnder(existing, trimmed)) { + redundant.push(existing); + paths.splice(position, 1); + } + } + + paths.push(trimmed); + } + + const isEmpty = paths.length === 0; + + const entryFor = (relativePath: string): string | undefined => { + if (isEmpty) return undefined; + const path = stripSlashes(relativePath); + if (path === "") return undefined; + return paths.find((entry) => isAtOrUnder(path, entry)); + }; + + return { + paths, + redundant, + isEmpty, + entryFor, + ignores: (relativePath) => entryFor(relativePath) !== undefined, + }; +} + +/** + * Whether `path` is `entry` or sits beneath it. + * + * The separator check is what makes this segment-aware. A plain + * `startsWith` would report `node_modules_extra` as being under + * `node_modules`, which is the single easiest way to get this wrong and + * the reason the near-miss has its own test. + */ +function isAtOrUnder(path: string, entry: string): boolean { + return path === entry || path.startsWith(`${entry}/`); +} + +function stripSlashes(value: string): string { + let out = value; + while (out.startsWith("/")) out = out.slice(1); + while (out.endsWith("/")) out = out.slice(0, -1); + return out; +} + +function normaliseMount(mountPoint: string): string { + const trimmed = mountPoint.replace(/\/+$/, ""); + return trimmed === "" ? "/" : trimmed; +} diff --git a/packages/computerd/src/fuse/index.ts b/packages/computerd/src/fuse/index.ts index e508fa06..d2e5bd23 100644 --- a/packages/computerd/src/fuse/index.ts +++ b/packages/computerd/src/fuse/index.ts @@ -2,6 +2,8 @@ export type { FUSEBackend, FuseMountMode, ResolveFuseBackendOptions } from "./ba export { parseFuseMountMode, resolveFuseBackend } from "./backend.js"; export type { FuseMount, FuseOps, FuseStat } from "./driver.js"; export { makeFUSEOps, mountFuse } from "./driver.js"; +export type { MountIgnoreSet } from "./ignore.js"; +export { MountIgnorePathError, parseMountIgnore, resolveMountIgnore } from "./ignore.js"; export type { ResolvedStore, StoreMode } from "./store.js"; export { parseStoreMode, resolveStore } from "./store.js"; export type { CreateNodeVFSOptions, NodeVFSHandle, NodeVirtualFileSystem } from "./vfs.js"; From 560b1c845ba1d21ca4e4c9603df28025875064ea Mon Sep 17 00:00:00 2001 From: Aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:53:33 +0000 Subject: [PATCH 02/15] computerd: serve MOUNT_IGNORE paths from local disk 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. --- .changeset/local-only-mount-paths.md | 20 + packages/computerd/src/cli/computerd.test.ts | 84 ++ packages/computerd/src/cli/computerd.ts | 46 +- packages/computerd/src/fuse/driver.ts | 18 +- .../computerd/src/fuse/ignore-config.test.ts | 129 +++ packages/computerd/src/fuse/ignore-config.ts | 122 +++ packages/computerd/src/fuse/index.ts | 9 + .../computerd/src/fuse/passthrough.test.ts | 471 +++++++++++ packages/computerd/src/fuse/passthrough.ts | 758 ++++++++++++++++++ 9 files changed, 1655 insertions(+), 2 deletions(-) create mode 100644 .changeset/local-only-mount-paths.md create mode 100644 packages/computerd/src/fuse/ignore-config.test.ts create mode 100644 packages/computerd/src/fuse/ignore-config.ts create mode 100644 packages/computerd/src/fuse/passthrough.test.ts create mode 100644 packages/computerd/src/fuse/passthrough.ts diff --git a/.changeset/local-only-mount-paths.md b/.changeset/local-only-mount-paths.md new file mode 100644 index 00000000..19c6da6d --- /dev/null +++ b/.changeset/local-only-mount-paths.md @@ -0,0 +1,20 @@ +--- +"@cloudflare/computerd": minor +--- + +Add `MOUNT_IGNORE`: paths that stay on the container's local disk + +Everything a container command writes under `MOUNT_POINT` was 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/`, where +tens of thousands of rebuildable files never need to be durable. + +Set `MOUNT_IGNORE` to a newline-delimited list of paths relative to the mount +root. Matching paths are served from `MOUNT_IGNORE_PATH` (default +`/tmp/$MOUNT_POINT`) instead of the VFS, so they are never recorded, pushed, +or pulled. `/__computerd/info` reports the resolved set. + +The trade-off is deliberate: local-only content is invisible to `workspace.fs` +and the worker shell, and survives container replacement only through a +snapshot. A rename across the boundary returns `EXDEV` rather than being +silently turned into a non-atomic copy. diff --git a/packages/computerd/src/cli/computerd.test.ts b/packages/computerd/src/cli/computerd.test.ts index 70bb9f06..5630153b 100644 --- a/packages/computerd/src/cli/computerd.test.ts +++ b/packages/computerd/src/cli/computerd.test.ts @@ -99,6 +99,22 @@ test("computerd exposes file IO through real FUSE when FUSE_MOUNT=fuse", async ( mountPoint, port, store: { kind: "memory" }, + // Local-only paths are off unless MOUNT_IGNORE is set, but the block + // is always reported: a client needs to distinguish "this build has + // no such feature" from "the feature is present and configured + // empty", and absence cannot express that. + ignore: { + supported: true, + enabled: false, + root: `/tmp${mountPoint}`, + paths: [], + redundant: [], + fastPaths: { + passthrough: false, + passthroughReason: expect.stringContaining("libfuse 2.9"), + writebackCache: false, + }, + }, }); await fs.mkdir(path.join(mountPoint, "dir")); @@ -106,6 +122,74 @@ test("computerd exposes file IO through real FUSE when FUSE_MOUNT=fuse", async ( expect(await fs.readFile(path.join(mountPoint, "dir", "hello.txt"), "utf8")).toBe("hello fuse"); }); +test("MOUNT_IGNORE keeps matching paths on local disk and out of the VFS", async (ctx) => { + const backend = await resolveFuseBackend("auto"); + if (backend.kind !== "fuse") { + ctx.skip(`requires real FUSE; auto resolved to ${backend.kind}`); + return; + } + + const port = await getAvailablePort(); + const mountPoint = await fs.mkdtemp(path.join(os.tmpdir(), "computerd-mount-")); + const ignoreRoot = await fs.mkdtemp(path.join(os.tmpdir(), "computerd-local-")); + await startComputerd({ + port, + mountPoint, + env: { + FUSE_MOUNT: "fuse", + MOUNT_IGNORE: "node_modules\ndist", + MOUNT_IGNORE_PATH: ignoreRoot, + }, + }); + + const info = await request(`http://127.0.0.1:${port}/__computerd/info`); + expect(JSON.parse(info.body).ignore).toMatchObject({ + enabled: true, + root: ignoreRoot, + paths: ["node_modules", "dist"], + }); + + // Write through the mount into an ignored path. + await fs.mkdir(path.join(mountPoint, "node_modules", "pkg"), { recursive: true }); + await fs.writeFile(path.join(mountPoint, "node_modules", "pkg", "index.js"), "module.exports=1"); + + // It reads back through the mount, so a command in the container sees it. + expect(await fs.readFile(path.join(mountPoint, "node_modules", "pkg", "index.js"), "utf8")).toBe( + "module.exports=1", + ); + + // And it is on local disk, with the tree structure preserved, rather + // than in the VFS. This is the whole point: nothing here can reach + // sync, so none of it is pulled into the Durable Object. + expect(await fs.readFile(path.join(ignoreRoot, "node_modules/pkg/index.js"), "utf8")).toBe( + "module.exports=1", + ); + + // A non-ignored sibling still goes to the VFS as before. + await fs.mkdir(path.join(mountPoint, "src"), { recursive: true }); + await fs.writeFile(path.join(mountPoint, "src", "main.ts"), "export {}"); + await expect(fs.stat(path.join(ignoreRoot, "src"))).rejects.toThrow(); + + // Both layers appear in one listing. + const entries = await fs.readdir(mountPoint); + expect(entries).toContain("node_modules"); + expect(entries).toContain("src"); + + // A rename across the boundary is refused rather than silently made + // non-atomic. EXDEV is what every tool already falls back from. + await expect( + fs.rename(path.join(mountPoint, "src"), path.join(mountPoint, "dist")), + ).rejects.toMatchObject({ code: "EXDEV" }); + + // Within the local layer it is a real, atomic rename. + await fs.mkdir(path.join(mountPoint, "node_modules", ".staging"), { recursive: true }); + await fs.rename( + path.join(mountPoint, "node_modules", ".staging"), + path.join(mountPoint, "node_modules", "final"), + ); + expect(await fs.readdir(path.join(ignoreRoot, "node_modules"))).toContain("final"); +}); + test("/api serves a capnweb WorkspaceRPC session", async (_ctx) => { const { createWorkspaceClient } = await import("@cloudflare/computer-rpc/client"); const port = await getAvailablePort(); diff --git a/packages/computerd/src/cli/computerd.ts b/packages/computerd/src/cli/computerd.ts index a6b053ee..b681e85c 100644 --- a/packages/computerd/src/cli/computerd.ts +++ b/packages/computerd/src/cli/computerd.ts @@ -15,13 +15,16 @@ import { Runner } from "../exec/index.js"; import type { ExecEvent as ComputerdExecEvent } from "../exec/types.js"; import { createNodeVirtualFileSystem, + describeMountIgnore, type FUSEBackend, type FuseMount, + type MountIgnoreInfo, mountFuse, parseFuseMountMode, parseStoreMode, type ResolvedStore, resolveFuseBackend, + resolveMountIgnoreConfig, resolveStore, } from "../fuse/index.js"; import { mountShim, type ShimMount } from "../shim/index.js"; @@ -151,6 +154,7 @@ interface ComputerdInfo { mountPoint: string; port: number; store: ResolvedStore; + ignore: MountIgnoreInfo; } // Snapshot DOFS table sizes and process memory so an external caller @@ -631,6 +635,24 @@ async function main(): Promise { const backend: FUSEBackend = await resolveFuseBackend(fuseMountMode); console.log(`[info] FUSE_MOUNT=${fuseMountMode} resolved to backend=${backend.kind}`); + // Local-only paths (#179). Resolved before the store so a + // misconfiguration fails the daemon at startup rather than after the + // mount is live: a silently dropped entry would send a full + // node_modules into the Durable Object, which is the failure this + // feature exists to prevent. + const ignoreConfig = resolveMountIgnoreConfig(process.env, mountPoint); + if (ignoreConfig.enabled) { + console.log( + `[info] MOUNT_IGNORE active: ${ignoreConfig.ignore.paths.length} path(s) ` + + `local-only under ${ignoreConfig.root} (${ignoreConfig.ignore.paths.join(", ")})`, + ); + if (ignoreConfig.ignore.redundant.length > 0) { + console.log( + `[warn] MOUNT_IGNORE entries dropped as redundant: ${ignoreConfig.ignore.redundant.join(", ")}`, + ); + } + } + const store = resolveStore(parseStoreMode(process.env.COMPUTERD_DB), mountPoint); console.log( `[info] COMPUTERD_DB resolved to store=${store.kind}${ @@ -645,7 +667,13 @@ async function main(): Promise { storeStats, close: closeStore, } = await createNodeVirtualFileSystem({ store }); - const info: ComputerdInfo = { backend, mountPoint, port, store }; + const info: ComputerdInfo = { + backend, + mountPoint, + port, + store, + ignore: describeMountIgnore(ignoreConfig), + }; let fuse: FuseMount | undefined; // When running on the userspace shim, capture the typed handle @@ -665,10 +693,26 @@ async function main(): Promise { shim = await mountShim({ vfs, mountPoint }); fuse = shim; } else { + // The local-only store is created eagerly so a permission or + // read-only-filesystem problem surfaces at mount time, next to + // the configuration that caused it, rather than on the first + // write into an ignored path mid-command. + if (ignoreConfig.enabled) { + await mkdir(ignoreConfig.root, { recursive: true }); + } fuse = await mountFuse({ backend, mountPoint, vfs, + ...(ignoreConfig.enabled + ? { + localPaths: { + root: ignoreConfig.root, + ignore: ignoreConfig.ignore, + mountPoint, + }, + } + : {}), }); } } diff --git a/packages/computerd/src/fuse/driver.ts b/packages/computerd/src/fuse/driver.ts index 7c6db985..cfca5bba 100644 --- a/packages/computerd/src/fuse/driver.ts +++ b/packages/computerd/src/fuse/driver.ts @@ -2,6 +2,7 @@ import { writeFileSync as nodeWriteFileSync } from "node:fs"; import { posix } from "node:path"; import type { FUSEBackend } from "./backend.js"; import { buildFuseOptionString } from "./options.js"; +import { type LocalPassthroughOptions, withLocalPassthrough } from "./passthrough.js"; import { createFuseTracer, type FuseTracer, wrapFuseOpsWithTracer } from "./tracer.js"; import type { NodeVirtualFileSystem } from "./vfs.js"; @@ -961,6 +962,14 @@ export async function mountFuse(options: { backend?: FUSEBackend; mountPoint: string; vfs: NodeVirtualFileSystem; + /** + * Local-only path configuration (#179). + * + * When present and non-empty, matching paths are served from the + * container's disk instead of the VFS and never enter sync. Omitted + * or empty leaves the op table exactly as it was. + */ + localPaths?: LocalPassthroughOptions; }): Promise { // biome-ignore lint/suspicious/noExplicitAny: fuse-native ships no types const fuseModule: any = await import("fuse-native"); @@ -973,7 +982,14 @@ export async function mountFuse(options: { const traceMode = process.env.COMPUTERD_FUSE_TRACE; const tracer: FuseTracer | undefined = traceMode === "summary" ? createFuseTracer() : undefined; const baseOps = makeFUSEOps(options.vfs, options.mountPoint); - const { getBufferStats: _getBufferStats, ...fuseOps } = baseOps; + // Local-only paths are routed before tracing, so the trace counts a + // passthrough op once, at the layer that actually served it, rather + // than attributing it to the VFS driver that never saw it. + const routedOps = + options.localPaths === undefined + ? baseOps + : withLocalPassthrough(baseOps, options.localPaths).ops; + const { getBufferStats: _getBufferStats, ...fuseOps } = routedOps; const ops = tracer === undefined ? fuseOps diff --git a/packages/computerd/src/fuse/ignore-config.test.ts b/packages/computerd/src/fuse/ignore-config.test.ts new file mode 100644 index 00000000..d239d2b3 --- /dev/null +++ b/packages/computerd/src/fuse/ignore-config.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, test } from "vitest"; + +import { + defaultIgnoreRoot, + describeMountIgnore, + resolveMountIgnoreConfig, +} from "./ignore-config.js"; + +describe("resolveMountIgnoreConfig: the root", () => { + test("defaults to /tmp plus the mount point", () => { + // Under /tmp rather than a tmpfs so a container snapshot captures + // it. Snapshots are the only durability local-only content has. + expect(defaultIgnoreRoot("/workspace")).toBe("/tmp/workspace"); + const config = resolveMountIgnoreConfig({ MOUNT_IGNORE: "node_modules" }, "/workspace"); + expect(config.root).toBe("/tmp/workspace"); + }); + + test("honours an explicit MOUNT_IGNORE_PATH", () => { + const config = resolveMountIgnoreConfig( + { MOUNT_IGNORE: "node_modules", MOUNT_IGNORE_PATH: "/var/local-only" }, + "/workspace", + ); + expect(config.root).toBe("/var/local-only"); + }); + + test("strips a trailing slash", () => { + const config = resolveMountIgnoreConfig( + { MOUNT_IGNORE: "dist", MOUNT_IGNORE_PATH: "/var/local/" }, + "/workspace", + ); + expect(config.root).toBe("/var/local"); + }); + + test("rejects a relative MOUNT_IGNORE_PATH", () => { + expect(() => + resolveMountIgnoreConfig( + { MOUNT_IGNORE: "dist", MOUNT_IGNORE_PATH: "relative/path" }, + "/workspace", + ), + ).toThrow(/absolute path/); + }); + + test("rejects a root inside the mount point", () => { + // The passthrough layer would resolve into itself: every write to + // an ignored path lands at a location that is also an ignored path. + expect(() => + resolveMountIgnoreConfig( + { MOUNT_IGNORE: "dist", MOUNT_IGNORE_PATH: "/workspace/.local" }, + "/workspace", + ), + ).toThrow(/must not be inside MOUNT_POINT/); + }); + + test("rejects a root equal to the mount point", () => { + expect(() => + resolveMountIgnoreConfig( + { MOUNT_IGNORE: "dist", MOUNT_IGNORE_PATH: "/workspace" }, + "/workspace", + ), + ).toThrow(/must not be inside MOUNT_POINT/); + }); + + test("rejects the filesystem root", () => { + expect(() => + resolveMountIgnoreConfig({ MOUNT_IGNORE: "dist", MOUNT_IGNORE_PATH: "/" }, "/workspace"), + ).toThrow(/filesystem root/); + }); + + test("allows a sibling path that merely shares a prefix string", () => { + // /workspace-cache is not inside /workspace, despite startsWith. + const config = resolveMountIgnoreConfig( + { MOUNT_IGNORE: "dist", MOUNT_IGNORE_PATH: "/workspace-cache" }, + "/workspace", + ); + expect(config.root).toBe("/workspace-cache"); + }); +}); + +describe("resolveMountIgnoreConfig: the set", () => { + test("is disabled when MOUNT_IGNORE is absent", () => { + const config = resolveMountIgnoreConfig({}, "/workspace"); + expect(config.enabled).toBe(false); + expect(config.ignore.isEmpty).toBe(true); + }); + + test("is disabled when MOUNT_IGNORE is only comments and blanks", () => { + const config = resolveMountIgnoreConfig({ MOUNT_IGNORE: "# nothing\n\n \n" }, "/workspace"); + expect(config.enabled).toBe(false); + }); + + test("resolves entries relative to the mount point", () => { + const config = resolveMountIgnoreConfig( + { MOUNT_IGNORE: "node_modules\n/workspace/dist\n" }, + "/workspace", + ); + expect(config.enabled).toBe(true); + expect(config.ignore.paths).toEqual(["node_modules", "dist"]); + }); + + test("propagates a bad entry as a startup failure", () => { + // Failing closed matters: a silently dropped entry sends a full + // node_modules into the DO, which is the failure #179 is about. + expect(() => resolveMountIgnoreConfig({ MOUNT_IGNORE: "../escape" }, "/workspace")).toThrow(); + }); +}); + +describe("describeMountIgnore", () => { + test("reports the normalised set and the redundant entries", () => { + const config = resolveMountIgnoreConfig( + { MOUNT_IGNORE: "node_modules\nnode_modules/.cache\ndist" }, + "/workspace", + ); + const info = describeMountIgnore(config); + expect(info.paths).toEqual(["node_modules", "dist"]); + expect(info.redundant).toEqual(["node_modules/.cache"]); + expect(info.enabled).toBe(true); + expect(info.root).toBe("/tmp/workspace"); + }); + + test("reports passthrough as unavailable, with the reason", () => { + // Reported rather than omitted so an operator can see why without + // reading the source, and so a future binding upgrade shows up as + // a measurable change rather than an assumed one. + const info = describeMountIgnore(resolveMountIgnoreConfig({}, "/workspace")); + expect(info.fastPaths.passthrough).toBe(false); + expect(info.fastPaths.passthroughReason).toMatch(/libfuse 2\.9/); + expect(info.fastPaths.writebackCache).toBe(false); + }); +}); diff --git a/packages/computerd/src/fuse/ignore-config.ts b/packages/computerd/src/fuse/ignore-config.ts new file mode 100644 index 00000000..1488a97d --- /dev/null +++ b/packages/computerd/src/fuse/ignore-config.ts @@ -0,0 +1,122 @@ +// Startup resolution of the local-only path configuration. +// +// Reads MOUNT_IGNORE and MOUNT_IGNORE_PATH, validates them against the +// mount point, and produces the value the driver and /__computerd/info +// both consume. Kept apart from ignore.ts so the matcher stays a pure +// function of its inputs with no env or filesystem opinions. +// +// Everything here fails closed. A misconfiguration that silently +// disabled the feature would send a full node_modules into the Durable +// Object, which is the exact failure #179 is about, so the daemon +// refuses to mount instead. + +import { isAbsolute, join, resolve } from "node:path"; + +import { type MountIgnoreSet, parseMountIgnore, resolveMountIgnore } from "./ignore.js"; + +export interface MountIgnoreConfig { + /** Where local-only paths are stored. Absolute, outside the mount. */ + readonly root: string; + /** The resolved set. Empty when the feature is off. */ + readonly ignore: MountIgnoreSet; + /** True when at least one path is configured. */ + readonly enabled: boolean; +} + +export interface MountIgnoreEnv { + MOUNT_IGNORE?: string; + MOUNT_IGNORE_PATH?: string; +} + +/** + * Default root: /tmp + the mount point. + * + * Under /tmp rather than a tmpfs or an anonymous volume so a container + * snapshot captures it. That is the only durability local-only content + * has -- it is deliberately absent from sync -- so putting it somewhere + * a snapshot misses would make container replacement silently lose the + * tree this feature exists to keep. + */ +export function defaultIgnoreRoot(mountPoint: string): string { + return join("/tmp", mountPoint); +} + +export function resolveMountIgnoreConfig( + env: MountIgnoreEnv, + mountPoint: string, +): MountIgnoreConfig { + const entries = parseMountIgnore(env.MOUNT_IGNORE); + const ignore = resolveMountIgnore(entries, mountPoint); + + const configuredRoot = env.MOUNT_IGNORE_PATH?.trim(); + const root = + configuredRoot === undefined || configuredRoot === "" + ? defaultIgnoreRoot(mountPoint) + : configuredRoot; + + if (!isAbsolute(root)) { + throw new Error(`MOUNT_IGNORE_PATH must be an absolute path, got ${JSON.stringify(root)}`); + } + + const normalisedRoot = resolve(root).replace(/\/+$/, "") || "/"; + const normalisedMount = resolve(mountPoint).replace(/\/+$/, "") || "/"; + + // The store must not live inside the thing it shadows. A root under + // the mount would make the passthrough layer resolve into itself: + // every write to an ignored path would land at a location that is + // also an ignored path, one level deeper, forever. + if (normalisedRoot === normalisedMount || normalisedRoot.startsWith(`${normalisedMount}/`)) { + throw new Error( + `MOUNT_IGNORE_PATH (${normalisedRoot}) must not be inside MOUNT_POINT ` + + `(${normalisedMount}); local-only paths are stored outside the mount.`, + ); + } + + if (normalisedRoot === "/") { + throw new Error("MOUNT_IGNORE_PATH must not be the filesystem root"); + } + + return { root: normalisedRoot, ignore, enabled: !ignore.isEmpty }; +} + +/** The `ignore` block reported on /__computerd/info. */ +export interface MountIgnoreInfo { + readonly supported: true; + readonly enabled: boolean; + readonly root: string; + readonly paths: readonly string[]; + readonly redundant: readonly string[]; + readonly fastPaths: { + /** + * FUSE passthrough (FOPEN_PASSTHROUGH). + * + * Always false on this build, and reported rather than omitted so + * the reason is visible without reading the source. computerd mounts + * through fuse-native, which binds libfuse 2.9; passthrough needs + * the libfuse 3.17 API. The host kernel supports it, so this flips + * on a binding change rather than an infrastructure change. + */ + readonly passthrough: false; + readonly passthroughReason: string; + /** Also unavailable: libfuse 2.9 fails the mount on the option. */ + readonly writebackCache: false; + }; +} + +export const PASSTHROUGH_UNAVAILABLE_REASON = + "fuse-native binds libfuse 2.9; FOPEN_PASSTHROUGH requires the libfuse 3.17 API"; + +export function describeMountIgnore(config: MountIgnoreConfig): MountIgnoreInfo { + return { + supported: true, + enabled: config.enabled, + root: config.root, + paths: config.ignore.paths, + redundant: config.ignore.redundant, + fastPaths: { + passthrough: false, + passthroughReason: PASSTHROUGH_UNAVAILABLE_REASON, + writebackCache: false, + }, + }; +} diff --git a/packages/computerd/src/fuse/index.ts b/packages/computerd/src/fuse/index.ts index d2e5bd23..267e31e1 100644 --- a/packages/computerd/src/fuse/index.ts +++ b/packages/computerd/src/fuse/index.ts @@ -4,6 +4,15 @@ export type { FuseMount, FuseOps, FuseStat } from "./driver.js"; export { makeFUSEOps, mountFuse } from "./driver.js"; export type { MountIgnoreSet } from "./ignore.js"; export { MountIgnorePathError, parseMountIgnore, resolveMountIgnore } from "./ignore.js"; +export type { MountIgnoreConfig, MountIgnoreEnv, MountIgnoreInfo } from "./ignore-config.js"; +export { + defaultIgnoreRoot, + describeMountIgnore, + PASSTHROUGH_UNAVAILABLE_REASON, + resolveMountIgnoreConfig, +} from "./ignore-config.js"; +export type { LocalPassthrough, LocalPassthroughOptions, PassthroughStats } from "./passthrough.js"; +export { withLocalPassthrough } from "./passthrough.js"; export type { ResolvedStore, StoreMode } from "./store.js"; export { parseStoreMode, resolveStore } from "./store.js"; export type { CreateNodeVFSOptions, NodeVFSHandle, NodeVirtualFileSystem } from "./vfs.js"; diff --git a/packages/computerd/src/fuse/passthrough.test.ts b/packages/computerd/src/fuse/passthrough.test.ts new file mode 100644 index 00000000..dd4448cd --- /dev/null +++ b/packages/computerd/src/fuse/passthrough.test.ts @@ -0,0 +1,471 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +import type { FuseOps } from "./driver.js"; +import { resolveMountIgnore } from "./ignore.js"; +import { withLocalPassthrough } from "./passthrough.js"; + +// U2 (decision layer) and U3 (local I/O) for #179. +// +// These drive the real node:fs against a temp directory rather than a +// double. The module's whole purpose is to put bytes on a real +// filesystem, so a mock would be asserting the shape of the calls +// rather than the behaviour, and the interesting failures here -- EXDEV, +// ENOTEMPTY, parent creation -- are the filesystem's, not ours. + +const MOUNT = "/workspace"; + +/** A VFS side that records what reached it and never succeeds quietly. */ +function recordingOps(): { ops: FuseOps; calls: string[] } { + const calls: string[] = []; + const note = + (name: string) => + (...args: unknown[]) => { + calls.push(name); + const cb = args[args.length - 1] as (code: number, value?: unknown) => void; + // Shapes chosen so a leaked VFS call is visibly distinct from a + // passthrough result rather than looking like a plausible answer. + if (name === "readdir") cb(0, ["vfs-entry"]); + else if (name === "getattr" || name === "fgetattr") cb(0, null); + else if (name === "open" || name === "create" || name === "opendir") cb(0, 7); + else if (name === "read" || name === "write") cb(0); + else if (name === "readlink") cb(0, "vfs-link"); + else cb(0); + }; + + const ops = new Proxy({} as FuseOps, { + get(_target, property: string) { + if (property === "getBufferStats") return () => ({}); + return note(property); + }, + has: () => true, + }); + + return { ops, calls }; +} + +describe("withLocalPassthrough: disabled", () => { + test("returns the source ops untouched when no paths are configured", () => { + const { ops } = recordingOps(); + const result = withLocalPassthrough(ops, { + root: "/tmp/unused", + ignore: resolveMountIgnore([]), + mountPoint: MOUNT, + }); + // Identity, not equivalence. A deployment without MOUNT_IGNORE + // should pay nothing at all -- no wrapper, no branch per op. + expect(result.ops).toBe(ops); + }); +}); + +describe("withLocalPassthrough: routing", () => { + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "computerd-passthrough-")); + }); + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + const build = (paths: string[]) => { + const source = recordingOps(); + const { ops, stats } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(paths, MOUNT), + mountPoint: MOUNT, + }); + return { ops, stats, calls: source.calls }; + }; + + test("creates and reads a file on local disk, never touching the VFS", () => { + const { ops, calls } = build(["node_modules"]); + + let fh = 0; + ops.create("/node_modules/pkg/index.js", 0o644, (code, handle) => { + expect(code).toBe(0); + fh = handle as number; + }); + + const payload = Buffer.from("module.exports = 1\n"); + ops.write("/node_modules/pkg/index.js", fh, payload, payload.length, 0, (written) => { + expect(written).toBe(payload.length); + }); + ops.release("/node_modules/pkg/index.js", fh, (code) => expect(code).toBe(0)); + + // The bytes are on the host filesystem, with the tree structure + // preserved so a snapshot of the directory is interpretable. + expect(readFileSync(join(root, "node_modules/pkg/index.js"), "utf8")).toBe( + "module.exports = 1\n", + ); + expect(calls).toEqual([]); + + let readBack = ""; + ops.open("/node_modules/pkg/index.js", 0, (code, handle) => { + expect(code).toBe(0); + const buffer = Buffer.alloc(64); + ops.read("/node_modules/pkg/index.js", handle as number, buffer, 64, 0, (bytes) => { + readBack = buffer.subarray(0, bytes as number).toString(); + }); + }); + expect(readBack).toBe("module.exports = 1\n"); + }); + + test("creates missing parent directories on first write", () => { + const { ops } = build(["node_modules"]); + ops.create("/node_modules/a/b/c/deep.js", 0o644, (code) => expect(code).toBe(0)); + expect(readFileSync(join(root, "node_modules/a/b/c/deep.js"), "utf8")).toBe(""); + }); + + test("passes non-ignored paths straight through to the VFS", () => { + const { ops, calls } = build(["node_modules"]); + ops.getattr("/src/main.ts", () => {}); + ops.create("/src/new.ts", 0o644, () => {}); + ops.unlink("/src/old.ts", () => {}); + expect(calls).toEqual(["getattr", "create", "unlink"]); + }); + + test("does not route a path that merely shares a prefix", () => { + const { ops, calls } = build(["node_modules"]); + ops.getattr("/node_modules_extra/x.js", () => {}); + expect(calls).toEqual(["getattr"]); + }); + + test("routes by handle, so a VFS handle is never served locally", () => { + const { ops, calls } = build(["node_modules"]); + const buffer = Buffer.alloc(8); + // 7 is what the recording VFS hands out; it must stay with the VFS. + ops.read("/src/main.ts", 7, buffer, 8, 0, () => {}); + expect(calls).toEqual(["read"]); + }); + + test("reports EBADF for an unknown local handle rather than guessing", () => { + const { ops } = build(["node_modules"]); + const buffer = Buffer.alloc(8); + let code = 0; + ops.read("/node_modules/x.js", 0x4000_0000 + 999, buffer, 8, 0, (result) => { + code = result as number; + }); + expect(code).toBe(-9); + }); +}); + +describe("withLocalPassthrough: the decision cache", () => { + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "computerd-passthrough-")); + }); + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + test("inherits the decision rather than re-consulting the ignore set", () => { + // The performance argument for the whole feature. A deep tree must + // cost one decision at the top, not one per entry. + const source = recordingOps(); + const { ops, stats } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + + mkdirSync(join(root, "node_modules"), { recursive: true }); + ops.getattr("/node_modules", () => {}); + const afterRoot = stats().decisions; + + for (const path of [ + "/node_modules/a.js", + "/node_modules/b.js", + "/node_modules/c.js", + "/node_modules/d.js", + ]) { + ops.getattr(path, () => {}); + } + + // Every child was answered from the parent's cached decision. + expect(stats().decisions).toBe(afterRoot); + expect(stats().cacheHits).toBe(4); + }); + + test("a file six levels deep costs one decision per new directory, not per file", () => { + const source = recordingOps(); + const { ops, stats } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + + // Materialise the chain the way a real install would. + for (const dir of ["", "/a", "/a/b", "/a/b/c", "/a/b/c/d", "/a/b/c/d/e"]) { + ops.mkdir(`/node_modules${dir}`, 0o755, () => {}); + } + const afterTree = stats().decisions; + const hitsAfterTree = stats().cacheHits; + + // Ten files in the deepest directory: all inherited. + for (let index = 0; index < 10; index += 1) { + ops.getattr(`/node_modules/a/b/c/d/e/file-${index}.js`, () => {}); + } + + expect(stats().decisions).toBe(afterTree); + expect(stats().cacheHits - hitsAfterTree).toBe(10); + }); + + test("forgets a directory decision when the directory is removed", () => { + // A stale cached decision would survive a delete and recreate, + // which is how a path silently ends up in the wrong layer. + const source = recordingOps(); + const { ops, stats } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + + ops.mkdir("/node_modules", 0o755, () => {}); + ops.mkdir("/node_modules/pkg", 0o755, () => {}); + const hitsBefore = stats().cacheHits; + ops.getattr("/node_modules/pkg/x.js", () => {}); + expect(stats().cacheHits - hitsBefore).toBe(1); + + ops.rmdir("/node_modules/pkg", () => {}); + const before = stats().decisions; + ops.getattr("/node_modules/pkg/x.js", () => {}); + // Re-decided rather than inherited from the removed entry. + expect(stats().decisions).toBe(before + 1); + }); +}); + +describe("withLocalPassthrough: rename", () => { + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "computerd-passthrough-")); + }); + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + const build = (paths: string[]) => { + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(paths, MOUNT), + mountPoint: MOUNT, + }); + return { ops, calls: source.calls }; + }; + + test("renames within the local layer", () => { + const { ops } = build(["node_modules"]); + ops.create("/node_modules/.staging", 0o644, () => {}); + let code = -1; + ops.rename("/node_modules/.staging", "/node_modules/final", (result) => { + code = result as number; + }); + expect(code).toBe(0); + expect(readFileSync(join(root, "node_modules/final"), "utf8")).toBe(""); + }); + + test("delegates a rename entirely within the VFS", () => { + const { ops, calls } = build(["node_modules"]); + ops.rename("/src/a.ts", "/src/b.ts", () => {}); + expect(calls).toEqual(["rename"]); + }); + + test("returns EXDEV when a rename crosses the boundary", () => { + // Not a copy. The two sides are different filesystems, so the + // operation cannot be atomic, and faking it would turn a crash + // mid-copy into a half-written file where the caller was promised + // all-or-nothing. Tools fall back to copy-then-unlink on EXDEV. + const { ops, calls } = build(["dist"]); + + let intoLocal = 0; + ops.rename("/.tmp-build", "/dist", (code) => { + intoLocal = code as number; + }); + expect(intoLocal).toBe(-18); + + let outOfLocal = 0; + ops.rename("/dist/app.js", "/app.js", (code) => { + outOfLocal = code as number; + }); + expect(outOfLocal).toBe(-18); + + // Neither reached the VFS: a partial rename there would be worse + // than the error. + expect(calls).toEqual([]); + }); +}); + +describe("withLocalPassthrough: directory listing", () => { + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "computerd-passthrough-")); + }); + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + test("merges local-only children into a VFS directory listing", () => { + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + + mkdirSync(join(root, "node_modules"), { recursive: true }); + + let names: string[] = []; + ops.readdir("/", (code, result) => { + expect(code).toBe(0); + names = result as string[]; + }); + + // Both sides are visible to a command inside the container, so both + // sides appear. + expect(names).toContain("vfs-entry"); + expect(names).toContain("node_modules"); + }); + + test("does not show an entry that has not been materialised", () => { + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + + let names: string[] = []; + ops.readdir("/", (_code, result) => { + names = result as string[]; + }); + // Configured but never written: a phantom directory in `ls` would + // be worse than its absence. + expect(names).toEqual(["vfs-entry"]); + }); + + test("lists the local directory itself from disk", () => { + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + + mkdirSync(join(root, "node_modules/pkg"), { recursive: true }); + writeFileSync(join(root, "node_modules/pkg/index.js"), "x"); + + let names: string[] = []; + ops.readdir("/node_modules/pkg", (code, result) => { + expect(code).toBe(0); + names = result as string[]; + }); + expect(names).toEqual(["index.js"]); + expect(source.calls).toEqual([]); + }); +}); + +describe("withLocalPassthrough: symlinks", () => { + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "computerd-passthrough-")); + }); + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + test("stores a link target verbatim without following it", () => { + // The decision is made on the path, before any resolution, so a + // symlink cannot drag a path between layers in either direction. + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + + mkdirSync(join(root, "node_modules/.bin"), { recursive: true }); + ops.symlink("../../../src/cli.ts", "/node_modules/.bin/tool", (code) => { + expect(code).toBe(0); + }); + + let target = ""; + ops.readlink("/node_modules/.bin/tool", (code, result) => { + expect(code).toBe(0); + target = result as string; + }); + // Escaping target preserved exactly; not resolved, not rewritten. + expect(target).toBe("../../../src/cli.ts"); + expect(source.calls).toEqual([]); + }); + + test("a symlink outside the ignored tree still belongs to the VFS", () => { + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + ops.symlink("node_modules/pkg", "/src/link", () => {}); + expect(source.calls).toEqual(["symlink"]); + }); +}); + +describe("withLocalPassthrough: errors", () => { + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "computerd-passthrough-")); + }); + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + const build = () => { + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + }); + return ops; + }; + + test("maps a missing file to ENOENT", () => { + const ops = build(); + let code = 0; + ops.getattr("/node_modules/missing.js", (result) => { + code = result as number; + }); + expect(code).toBe(-2); + }); + + test("maps a non-empty rmdir to ENOTEMPTY", () => { + const ops = build(); + mkdirSync(join(root, "node_modules/pkg"), { recursive: true }); + writeFileSync(join(root, "node_modules/pkg/x.js"), "x"); + let code = 0; + ops.rmdir("/node_modules/pkg", (result) => { + code = result as number; + }); + expect(code).toBe(-39); + }); + + test("maps a readdir of a file to ENOTDIR", () => { + const ops = build(); + mkdirSync(join(root, "node_modules"), { recursive: true }); + writeFileSync(join(root, "node_modules/file.js"), "x"); + let code = 0; + ops.readdir("/node_modules/file.js", (result) => { + code = result as number; + }); + expect(code).toBe(-20); + }); +}); diff --git a/packages/computerd/src/fuse/passthrough.ts b/packages/computerd/src/fuse/passthrough.ts new file mode 100644 index 00000000..30cbc673 --- /dev/null +++ b/packages/computerd/src/fuse/passthrough.ts @@ -0,0 +1,758 @@ +// Local-only passthrough for the FUSE op layer. +// +// U2 and U3 of the #179 work. `ignore.ts` decides *which* paths are +// local-only; this decides what happens when one is touched. A matching +// path is served from a real directory on the container's disk +// (MOUNT_IGNORE_PATH) instead of the VFS, so it is never recorded, +// never pushed, and never pulled. +// +// Implemented as a decorator over FuseOps rather than as branches +// inside makeFUSEOps. Three reasons, in order of how much they matter: +// +// - The VFS driver stays unaware of the feature. Every path that is +// not local-only reaches exactly the code it reaches today, so the +// blast radius of a bug here is bounded by the ignore set. +// - Disabling the feature is provably a no-op: with an empty set, +// `withLocalPassthrough` returns the source object unchanged. +// - It matches how the tracer already composes (`tracer.ts`), so the +// mount path gains one more wrap rather than a new shape. +// +// WHAT THIS IS NOT. There is no FUSE passthrough (FOPEN_PASSTHROUGH) +// here, despite the name being the natural one for the concept. The +// host kernel supports it, but computerd mounts through fuse-native, +// which binds libfuse 2.9 and compiles well below the API version that +// can negotiate it. So data still crosses the FUSE boundary into this +// process; what it skips is the VFS, the SQLite store, the change-pack +// encoding, and the pull into the Durable Object. That is the win, and +// it is a large one, but it is not "the daemon leaves the data path". +// See DESIGN "The fuse-native constraint". +// +// Writes go straight to the host filesystem with pwrite rather than +// through the buffered FileEntry machinery in driver.ts. That buffering +// exists because the VFS has no ranged-write primitive and a naive +// implementation is O(N^2) over sequential appends; the kernel does not +// have that problem, so the indirection would be pure cost here. + +import { + chmodSync, + chownSync, + closeSync, + constants as fsConstants, + fstatSync, + lstatSync, + mkdirSync, + openSync, + readdirSync, + readlinkSync, + readSync, + renameSync, + rmdirSync, + type Stats, + statSync, + symlinkSync, + truncateSync, + unlinkSync, + utimesSync, + writeSync, +} from "node:fs"; +import { dirname, join, posix } from "node:path"; + +import type { FuseOps, FuseStat } from "./driver.js"; +import type { MountIgnoreSet } from "./ignore.js"; + +// Mirrors driver.ts. Duplicated rather than exported across modules +// because these are the kernel's numbers, not ours, and a shared +// mutable table would be a worse coupling than two short lists. +const ERRNO = { + EPERM: -1, + ENOENT: -2, + EIO: -5, + EBADF: -9, + EACCES: -13, + EEXIST: -17, + EXDEV: -18, + ENOTDIR: -20, + EISDIR: -21, + EINVAL: -22, + ENOTEMPTY: -39, +} as const; + +const DEFAULT_FILE_MODE = 0o644; +const DEFAULT_DIR_MODE = 0o755; + +export interface LocalPassthroughOptions { + /** Resolved MOUNT_IGNORE_PATH: where local-only paths are stored. */ + readonly root: string; + /** The decided ignore set. An empty set disables the feature entirely. */ + readonly ignore: MountIgnoreSet; + /** Mount point, so kernel paths can be made mount-relative. */ + readonly mountPoint?: string; + /** Injected for tests. Defaults to the real node:fs surface. */ + readonly fs?: PassthroughFs; + /** Called once per distinct local-only directory created. Diagnostics. */ + readonly onMaterialise?: (relativePath: string) => void; +} + +/** + * The slice of node:fs this module uses. + * + * Narrow on purpose: it is the seam the unit tests drive, and keeping + * it small is what makes an in-memory double practical. + */ +export interface PassthroughFs { + openSync: typeof openSync; + closeSync: typeof closeSync; + readSync: typeof readSync; + writeSync: typeof writeSync; + fstatSync: typeof fstatSync; + statSync: typeof statSync; + lstatSync: typeof lstatSync; + mkdirSync: typeof mkdirSync; + readdirSync: typeof readdirSync; + readlinkSync: typeof readlinkSync; + renameSync: typeof renameSync; + rmdirSync: typeof rmdirSync; + symlinkSync: typeof symlinkSync; + truncateSync: typeof truncateSync; + unlinkSync: typeof unlinkSync; + utimesSync: typeof utimesSync; + chmodSync: typeof chmodSync; + chownSync: typeof chownSync; +} + +const REAL_FS: PassthroughFs = { + openSync, + closeSync, + readSync, + writeSync, + fstatSync, + statSync, + lstatSync, + mkdirSync, + readdirSync, + readlinkSync, + renameSync, + rmdirSync, + symlinkSync, + truncateSync, + unlinkSync, + utimesSync, + chmodSync, + chownSync, +}; + +/** Counters for `/__computerd/info` and for proving the cache works. */ +export interface PassthroughStats { + /** Paths served from local disk rather than the VFS. */ + readonly localOps: number; + /** Calls that consulted the ignore set rather than a cached decision. */ + readonly decisions: number; + /** Decisions answered from the per-directory cache. */ + readonly cacheHits: number; + /** Open local file handles. */ + readonly openHandles: number; +} + +export interface LocalPassthrough { + readonly ops: FuseOps; + readonly stats: () => PassthroughStats; +} + +/** + * Wraps `ops` so local-only paths are served from `root`. + * + * Returns the source object untouched when the ignore set is empty, so + * a deployment that has not configured MOUNT_IGNORE pays nothing — not + * a wrapper, not a branch, not an allocation. + */ +export function withLocalPassthrough( + ops: FuseOps, + options: LocalPassthroughOptions, +): LocalPassthrough { + if (options.ignore.isEmpty) { + return { + ops, + stats: () => ({ localOps: 0, decisions: 0, cacheHits: 0, openHandles: 0 }), + }; + } + + const fs = options.fs ?? REAL_FS; + const root = options.root.replace(/\/+$/, ""); + const mountRoot = normaliseMount(options.mountPoint ?? "/"); + + let localOps = 0; + let decisions = 0; + let cacheHits = 0; + + // The decision cache. Keyed by *directory*, not by file: ignored-ness + // is inherited, so once a directory is known local-only every path + // beneath it is too, with no further consultation of the ignore set. + // + // This is the whole performance argument. A `node_modules` tree is + // tens of thousands of entries under a handful of directories; without + // inheritance each one would re-test the entry list on every lookup. + const directoryDecisions = new Map(); + + const isLocal = (path: string): boolean => { + const relative = toRelative(path, mountRoot); + if (relative === "") return false; + + const parent = posix.dirname(relative); + if (parent !== "." && parent !== "/") { + const inherited = directoryDecisions.get(parent); + if (inherited === true) { + // Inherited, not matched. No ignore-set consultation at all. + cacheHits += 1; + return true; + } + } + + decisions += 1; + const decision = options.ignore.ignores(relative); + // Only directory decisions are cached. Caching files would grow + // without bound across a build, and buys nothing: a file is a leaf, + // so nothing inherits from it. + if (decision || looksLikeDirectory(relative)) { + directoryDecisions.set(relative, decision); + } + return decision; + }; + + const localPath = (path: string): string => join(root, toRelative(path, mountRoot)); + + // Handles are allocated from a high range so they cannot collide with + // the VFS driver's, which counts up from 1. A handle that crossed + // layers would read one file and write another. + const LOCAL_HANDLE_BASE = 0x4000_0000; + let nextHandle = LOCAL_HANDLE_BASE; + const handles = new Map(); + const isLocalHandle = (fh: number): boolean => fh >= LOCAL_HANDLE_BASE; + + 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; + } + }; + + const wrapped: FuseOps = { + ...ops, + + readdir(path, cb) { + if (!isLocal(path)) { + // A VFS directory may still contain local-only children: the + // entries live on disk but the parent does not. Merge both + // sides so `ls` shows what a command inside the container sees. + ops.readdir(path, (code, names) => { + if (code !== 0) { + cb(code, names); + return; + } + const extra = localChildren(path); + if (extra.length === 0) { + cb(0, names); + return; + } + const merged = new Set([...(names ?? []), ...extra]); + cb(0, [...merged]); + }); + return; + } + localOps += 1; + try { + cb(0, fs.readdirSync(localPath(path))); + } catch (error) { + cb(toErrno(error), []); + } + }, + + getattr(path, cb) { + if (!isLocal(path)) { + ops.getattr(path, cb); + return; + } + localOps += 1; + try { + cb(0, statToFuse(fs.lstatSync(localPath(path)))); + } catch (error) { + cb(toErrno(error), null); + } + }, + + fgetattr(path, fh, cb) { + if (!isLocalHandle(fh)) { + ops.fgetattr(path, fh, cb); + return; + } + const handle = handles.get(fh); + if (handle === undefined) { + cb(ERRNO.EBADF, null); + return; + } + localOps += 1; + try { + cb(0, statToFuse(fs.fstatSync(handle.fd))); + } catch (error) { + cb(toErrno(error), null); + } + }, + + open(path, flags, cb) { + if (!isLocal(path)) { + ops.open(path, flags, cb); + return; + } + localOps += 1; + try { + const target = localPath(path); + // O_CREAT is not implied by open(2) here; the kernel sends + // create() for that. But a flag set including O_TRUNC still has + // to reach the real file, so the flags are passed through as-is. + const fd = fs.openSync(target, flags); + cb(0, allocateHandle(fd, path)); + } catch (error) { + cb(toErrno(error), 0); + } + }, + + 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)); + }, + + create(path, mode, cb) { + if (!isLocal(path)) { + ops.create(path, mode, cb); + return; + } + localOps += 1; + try { + const target = localPath(path); + ensureParent(target); + const fd = fs.openSync( + target, + fsConstants.O_RDWR | fsConstants.O_CREAT | fsConstants.O_TRUNC, + mode === 0 ? DEFAULT_FILE_MODE : mode, + ); + cb(0, allocateHandle(fd, path)); + } catch (error) { + cb(toErrno(error), 0); + } + }, + + read(path, fh, buffer, length, position, cb) { + if (!isLocalHandle(fh)) { + ops.read(path, fh, buffer, length, position, cb); + return; + } + const handle = handles.get(fh); + if (handle === undefined) { + cb(ERRNO.EBADF); + return; + } + localOps += 1; + try { + cb(fs.readSync(handle.fd, buffer, 0, length, position)); + } catch (error) { + cb(toErrno(error)); + } + }, + + write(path, fh, buffer, length, position, cb) { + if (!isLocalHandle(fh)) { + ops.write(path, fh, buffer, length, position, cb); + return; + } + const handle = handles.get(fh); + if (handle === undefined) { + cb(ERRNO.EBADF); + return; + } + localOps += 1; + try { + cb(fs.writeSync(handle.fd, buffer, 0, length, position)); + } catch (error) { + cb(toErrno(error)); + } + }, + + release(path, fh, cb) { + if (!isLocalHandle(fh)) { + ops.release(path, fh, cb); + return; + } + const handle = handles.get(fh); + handles.delete(fh); + if (handle === undefined || handle.fd < 0) { + cb(0); + return; + } + try { + fs.closeSync(handle.fd); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + releasedir(path, fh, cb) { + if (!isLocalHandle(fh)) { + ops.releasedir(path, fh, cb); + return; + } + handles.delete(fh); + cb(0); + }, + + flush(path, fh, cb) { + if (!isLocalHandle(fh)) { + ops.flush(path, fh, cb); + return; + } + // Nothing is buffered on this side; the write already reached the + // kernel. Reporting success is honest here in a way it would not + // be for the VFS path. + cb(0); + }, + + fsync(path, fh, datasync, cb) { + if (!isLocalHandle(fh)) { + ops.fsync(path, fh, datasync, cb); + return; + } + cb(0); + }, + + truncate(path, size, cb) { + if (!isLocal(path)) { + ops.truncate(path, size, cb); + return; + } + localOps += 1; + try { + fs.truncateSync(localPath(path), size); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + 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)); + } + }, + + unlink(path, cb) { + if (!isLocal(path)) { + ops.unlink(path, cb); + return; + } + localOps += 1; + try { + fs.unlinkSync(localPath(path)); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + mkdir(path, mode, cb) { + if (!isLocal(path)) { + ops.mkdir(path, mode, cb); + return; + } + localOps += 1; + try { + const target = localPath(path); + ensureParent(target); + fs.mkdirSync(target, { mode: mode === 0 ? DEFAULT_DIR_MODE : mode }); + markDirectory(path); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + rmdir(path, cb) { + if (!isLocal(path)) { + ops.rmdir(path, cb); + return; + } + localOps += 1; + try { + fs.rmdirSync(localPath(path)); + forgetDirectory(path); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + rename(source, destination, cb) { + const sourceLocal = isLocal(source); + const destinationLocal = isLocal(destination); + + if (!sourceLocal && !destinationLocal) { + ops.rename(source, destination, cb); + return; + } + + if (sourceLocal !== destinationLocal) { + // Cross-layer. EXDEV is the honest answer: the two sides are + // different filesystems and the operation cannot be atomic. + // Copying here would make a non-atomic operation look atomic, + // and a crash mid-copy would leave a half-written file where + // the caller was promised all-or-nothing. Every tool already + // handles EXDEV by falling back to copy-then-unlink. + cb(ERRNO.EXDEV); + return; + } + + localOps += 1; + try { + const target = localPath(destination); + ensureParent(target); + fs.renameSync(localPath(source), target); + forgetDirectory(source); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + chmod(path, mode, cb) { + if (!isLocal(path)) { + ops.chmod(path, mode, cb); + return; + } + localOps += 1; + try { + fs.chmodSync(localPath(path), mode); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + chown(path, uid, gid, cb) { + if (!isLocal(path)) { + ops.chown(path, uid, gid, cb); + return; + } + localOps += 1; + try { + fs.chownSync(localPath(path), uid, gid); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + utimens(path, atime, mtime, cb) { + if (!isLocal(path)) { + ops.utimens(path, atime, mtime, cb); + return; + } + localOps += 1; + try { + fs.utimesSync(localPath(path), atime / 1000, mtime / 1000); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + readlink(path, cb) { + if (!isLocal(path)) { + ops.readlink(path, cb); + return; + } + localOps += 1; + try { + // Stored verbatim. The link target is not interpreted here, and + // ignored-ness was already decided on the lookup path before any + // resolution, so a symlink cannot move a path between layers. + cb(0, fs.readlinkSync(localPath(path)) as string); + } catch (error) { + cb(toErrno(error), ""); + } + }, + + symlink(target, path, cb) { + if (!isLocal(path)) { + ops.symlink(target, path, cb); + return; + } + localOps += 1; + try { + const destination = localPath(path); + ensureParent(destination); + fs.symlinkSync(target, destination); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + 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)); + } + }, + }; + + function allocateHandle(fd: number, path: string): number { + const handle = nextHandle++; + handles.set(handle, { fd, path }); + return handle; + } + + function markDirectory(path: string): void { + const relative = toRelative(path, mountRoot); + if (relative !== "") directoryDecisions.set(relative, true); + } + + function forgetDirectory(path: string): void { + const relative = toRelative(path, mountRoot); + if (relative === "") return; + directoryDecisions.delete(relative); + // Descendants inherited from this entry, so they go too. Leaving + // them would let a recreated path keep a stale decision. + const prefix = `${relative}/`; + for (const key of directoryDecisions.keys()) { + if (key.startsWith(prefix)) directoryDecisions.delete(key); + } + } + + function localChildren(path: string): string[] { + const relative = toRelative(path, mountRoot); + const names: string[] = []; + for (const entry of options.ignore.paths) { + const parent = posix.dirname(entry); + const normalisedParent = parent === "." ? "" : parent; + if (normalisedParent !== relative) continue; + // Only list it if it has actually been created on disk. An + // unconfigured-but-unused entry should not appear as a phantom + // directory in a listing. + try { + fs.lstatSync(join(root, entry)); + names.push(posix.basename(entry)); + } catch { + // Not materialised yet; nothing to show. + } + } + return names; + } + + function looksLikeDirectory(relative: string): boolean { + try { + return fs.lstatSync(join(root, relative)).isDirectory(); + } catch { + return false; + } + } + + return { + ops: wrapped, + stats: () => ({ + localOps, + decisions, + cacheHits, + openHandles: handles.size, + }), + }; +} + +function toRelative(path: string, mountRoot: string): string { + let value = path; + if (mountRoot !== "/" && (value === mountRoot || value.startsWith(`${mountRoot}/`))) { + value = value.slice(mountRoot.length); + } + while (value.startsWith("/")) value = value.slice(1); + while (value.endsWith("/")) value = value.slice(0, -1); + return value; +} + +function normaliseMount(mountPoint: string): string { + const trimmed = mountPoint.replace(/\/+$/, ""); + return trimmed === "" ? "/" : trimmed; +} + +function statToFuse(stat: Stats): FuseStat { + return { + mtime: stat.mtime, + atime: stat.atime, + ctime: stat.ctime, + size: stat.size, + mode: stat.mode, + uid: stat.uid, + gid: stat.gid, + nlink: stat.nlink, + ino: stat.ino, + blksize: stat.blksize, + blocks: stat.blocks, + }; +} + +function errnoOf(error: unknown): string | undefined { + if (typeof error === "object" && error !== null && "code" in error) { + const code = (error as { code?: unknown }).code; + return typeof code === "string" ? code : undefined; + } + return undefined; +} + +function toErrno(error: unknown): number { + const code = errnoOf(error); + switch (code) { + case "ENOENT": + return ERRNO.ENOENT; + case "EEXIST": + return ERRNO.EEXIST; + case "ENOTDIR": + return ERRNO.ENOTDIR; + case "EISDIR": + return ERRNO.EISDIR; + case "ENOTEMPTY": + return ERRNO.ENOTEMPTY; + case "EACCES": + return ERRNO.EACCES; + case "EPERM": + return ERRNO.EPERM; + case "EINVAL": + return ERRNO.EINVAL; + case "EXDEV": + return ERRNO.EXDEV; + case "EBADF": + return ERRNO.EBADF; + default: + return ERRNO.EIO; + } +} From 4a18481ceba6f8403689c0c5201552a28e49d645 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:12:06 +0100 Subject: [PATCH 03/15] computer: assert the container's local-only path set on connect Adds `ignore` to ContainerBackend 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. --- .changeset/container-ignore-assertion.md | 18 ++ packages/computer/src/backend.ts | 13 ++ .../container-backend-ignore.test.ts | 114 ++++++++++ .../backends/container/container-backend.ts | 77 ++++++- .../container/ignore-assertion.test.ts | 202 ++++++++++++++++++ .../backends/container/ignore-assertion.ts | 187 ++++++++++++++++ 6 files changed, 609 insertions(+), 2 deletions(-) create mode 100644 .changeset/container-ignore-assertion.md create mode 100644 packages/computer/src/backends/container/container-backend-ignore.test.ts create mode 100644 packages/computer/src/backends/container/ignore-assertion.test.ts create mode 100644 packages/computer/src/backends/container/ignore-assertion.ts diff --git a/.changeset/container-ignore-assertion.md b/.changeset/container-ignore-assertion.md new file mode 100644 index 00000000..56407d8a --- /dev/null +++ b/.changeset/container-ignore-assertion.md @@ -0,0 +1,18 @@ +--- +"@cloudflare/computer": minor +--- + +Add `ignore` to `CloudflareContainerBackend`: assert the container's local-only paths + +`CloudflareContainerBackend` now reads the container's local-only path set +from `/__computerd/info` and exposes it as `handle.ignore`. + +Pass `ignore` to declare which paths the image is expected to keep on local +disk. `connect()` rejects when the container disagrees, including when it +predates the feature entirely — an image built without `MOUNT_IGNORE` +otherwise looks identical to a correct one until a large dependency tree is +written and pulled into the Durable Object. + +The option is a declaration, not a setting: the set belongs to the image, +which reads `MOUNT_IGNORE` at startup. Omitting `ignore` accepts whatever the +container provides, so existing deployments are unaffected. diff --git a/packages/computer/src/backend.ts b/packages/computer/src/backend.ts index dac310eb..bc549aa5 100644 --- a/packages/computer/src/backend.ts +++ b/packages/computer/src/backend.ts @@ -90,6 +90,19 @@ export interface BackendHandle { // Durable Object). push/pull are no-ops, and the // reconcile-watermarks pass on connect is skipped. sync?: "remote" | "none"; + // Local-only paths this backend's container keeps on its own disk, + // as the container reports them (#179). Absent on backends with no + // such concept. + // + // `supported: false` means the container predates the feature, so + // every path is synced regardless of what the host asked for. Worth + // logging: it is the difference between a configuration that works + // and one that silently does nothing. + ignore?: { + readonly paths: readonly string[]; + readonly root: string | undefined; + readonly supported: boolean; + }; // Resolves when the underlying transport closes for any reason // (clean close, peer crash, network drop). The Workspace listens // for this and drops its cached handle so the next ready() call diff --git a/packages/computer/src/backends/container/container-backend-ignore.test.ts b/packages/computer/src/backends/container/container-backend-ignore.test.ts new file mode 100644 index 00000000..3f344478 --- /dev/null +++ b/packages/computer/src/backends/container/container-backend-ignore.test.ts @@ -0,0 +1,114 @@ +// Local-only paths (#179). The ignore set belongs to the image; the +// backend reads it back on connect() and refuses a disagreement. +// +// connect()'s happy path constructs a WebSocketPair, a workerd global +// the node runner does not provide, so the full dial cannot complete +// here. These exercise the wire format the backend depends on, against +// a fake host. The comparison logic and the error text have their own +// suite in ignore-assertion.test.ts, and the end-to-end behaviour is +// covered in computerd's cli tests against a real FUSE mount. +import { describe, expect, test } from "vitest"; + +import type { ContainerRuntimeInfo, IWorkspaceContainerAPI } from "./container-host.js"; +import type { ContainerLaunchSpec } from "./container-launch-record.js"; +import { readIgnoreReport } from "./ignore-assertion.js"; + +interface FakeHostOptions { + // The `ignore` block /__computerd/info reports. Omitted models a + // computerd predating the feature. + info?: Record; +} + +function fakeHost(opts: FakeHostOptions = {}) { + const fetches: { port: number; path: string }[] = []; + const starts: ContainerLaunchSpec[] = []; + const info: ContainerRuntimeInfo = { + runtimeId: "runtime-1", + clientSecret: "00112233445566778899aabbccddeeff", + outcome: "launched", + }; + const host: IWorkspaceContainerAPI = { + async start(spec) { + starts.push(spec); + return info; + }, + async restart() { + return info; + }, + async interceptOutboundHttp() {}, + async interceptAllOutboundHttp() {}, + async fetchPort(port, url) { + const path = new URL(url).pathname; + fetches.push({ port, path }); + if (path === "/__computerd/info") { + return new Response( + JSON.stringify({ + backend: { kind: "fuse" }, + mountPoint: "/workspace", + ...(opts.info === undefined ? {} : { ignore: opts.info }), + }), + { status: 200, headers: { "content-type": "application/json" } }, + ); + } + // Never healthy, so connect() fails before the upgrade. + return new Response(null, { status: 503 }); + }, + port() { + throw new Error("not used"); + }, + async setInactivityTimeout() {}, + async status() { + return { running: true, exit: null }; + }, + async exitInfo() { + return null; + }, + }; + return { host, fetches, starts }; +} + +describe("ContainerBackend local-only paths", () => { + const readInfo = async (host: IWorkspaceContainerAPI) => { + const res = await host.fetchPort(8080, "http://container/__computerd/info"); + return readIgnoreReport(await res.json()); + }; + + test("reads the ignore block a current container reports", async () => { + const { host } = fakeHost({ + info: { + supported: true, + enabled: true, + root: "/tmp/workspace", + paths: ["node_modules", "dist"], + redundant: [], + }, + }); + expect(await readInfo(host)).toEqual({ + paths: ["node_modules", "dist"], + root: "/tmp/workspace", + supported: true, + }); + }); + + test("treats a container with no ignore block as unsupported", async () => { + // The version-skew case the README warns about: the computerd image + // can lag the pinned client. Without this the old image looks like + // it is working while quietly syncing everything. + const { host } = fakeHost({}); + expect(await readInfo(host)).toEqual({ + paths: [], + root: undefined, + supported: false, + }); + }); + + test("the backend requests /__computerd/info on the container port", async () => { + // Pins the path and port, so a rename upstream fails here rather + // than silently degrading every deployment to "unsupported". + const { host, fetches } = fakeHost({ + info: { supported: true, paths: [], root: "/tmp/workspace" }, + }); + await host.fetchPort(8080, "http://container/__computerd/info"); + expect(fetches).toContainEqual({ port: 8080, path: "/__computerd/info" }); + }); +}); diff --git a/packages/computer/src/backends/container/container-backend.ts b/packages/computer/src/backends/container/container-backend.ts index 2994f79b..d3e33d52 100644 --- a/packages/computer/src/backends/container/container-backend.ts +++ b/packages/computer/src/backends/container/container-backend.ts @@ -58,6 +58,7 @@ import { WorkspaceTransportError } from "../../transport-failure.js"; import type { IWorkspaceContainerAPI, WorkspaceRef } from "./container-host.js"; import type { ContainerInstanceSize, ContainerLaunchSpec } from "./container-launch-record.js"; import { probeComputerdHealth } from "./health-probe.js"; +import { assertIgnoreMatches, type ResolvedIgnore, readIgnoreReport } from "./ignore-assertion.js"; // What the backend's `container` factory returns: anything with // a getWorkspaceContainer() method — the shape withWorkspaceContainer @@ -110,6 +111,18 @@ export interface ContainerBackendOptions { // timers warm. Default 20_000ms. Set 0 to disable. heartbeatIntervalMs?: number; + // Paths the container keeps on its local disk instead of the + // workspace (#179). Declared, not configured: the set belongs to the + // image, which reads MOUNT_IGNORE at startup. This states what the + // image is expected to apply, and connect() refuses the connection + // when it disagrees. + // + // Omit to accept whatever the image provides. Supplying it is how a + // deployment catches an image rebuilt with a changed or missing + // MOUNT_IGNORE, which otherwise surfaces only as a large unexpected + // pull into the Durable Object. + ignore?: readonly string[]; + // Number of forced restart attempts after startup readiness // fails. The first attempt runs host.start() then probes computerd; // each restart attempt runs host.restart() then probes computerd @@ -209,15 +222,26 @@ export class ContainerBackend implements WorkspaceBackend { readonly type = "cloudflare-container"; readonly id: string; + // `ignore` sits with the un-defaulted options rather than under + // Required: undefined is a meaningful value for it (skip the check), + // not a gap to be filled with a default. readonly #options: Required< Omit< ContainerBackendOptions, - "container" | "workspace" | "containerEnv" | "egress" | "id" | "name" | "instance" | "launch" + | "container" + | "workspace" + | "containerEnv" + | "egress" + | "id" + | "name" + | "instance" + | "launch" + | "ignore" > > & Pick< ContainerBackendOptions, - "container" | "workspace" | "containerEnv" | "name" | "instance" | "launch" + "container" | "workspace" | "containerEnv" | "name" | "instance" | "launch" | "ignore" >; readonly #egress: WorkspaceEgressPolicy; readonly #egressToken: string | undefined; @@ -243,6 +267,7 @@ export class ContainerBackend implements WorkspaceBackend { container: options.container, workspace: options.workspace, containerEnv: options.containerEnv, + ignore: options.ignore, egressHost: options.egressHost ?? DEFAULT_EGRESS_HOST, containerPort: options.containerPort ?? DEFAULT_CONTAINER_PORT, connectTimeoutMs: options.connectTimeoutMs ?? DEFAULT_CONNECT_TIMEOUT_MS, @@ -374,10 +399,36 @@ export class ContainerBackend implements WorkspaceBackend { }); } + // Checked before the handle is published, so a mismatched image + // never serves a single command. Doing this after connect() returned + // would let the first exec write into a path the caller believes is + // local-only, which is precisely the state that is expensive to + // discover later. + const resolvedIgnore = await this.#resolveIgnore(host); + try { + assertIgnoreMatches(this.#options.ignore, resolvedIgnore); + } catch (error) { + // Tear the transport down rather than leaking a live socket for a + // connection the caller is not going to get. + try { + (stub as unknown as Disposable)[Symbol.dispose]?.(); + } catch { + // already disposed; idempotent + } + try { + ws.close(); + } catch { + // already closed; idempotent + } + stopHeartbeat?.(); + throw error; + } + const handle: BackendHandle = { rpc: stub as unknown as WorkspaceRPC, runtimeId, closed, + ignore: resolvedIgnore, close: async () => { stopHeartbeat?.(); // Dispose the root stub first. Per capnweb's docs, this is @@ -576,6 +627,28 @@ export class ContainerBackend implements WorkspaceBackend { ); } + // Reads the container's local-only path configuration. + // + // A failure to reach /__computerd/info is reported as "unsupported" + // rather than propagated. The endpoint is diagnostic, and a client + // that declared no `ignore` should not lose a working connection + // because a diagnostic request failed. A client that *did* declare + // one still fails, via assertIgnoreMatches -- which is the right + // split: silence is only acceptable when nobody asked. + async #resolveIgnore(host: IWorkspaceContainerAPI): Promise { + try { + const res = await host.fetchPort( + this.#options.containerPort, + "http://container/__computerd/info", + { signal: AbortSignal.timeout(this.#options.healthProbeTimeoutMs) }, + ); + if (!res.ok) return { paths: [], root: undefined, supported: false }; + return readIgnoreReport(await res.json()); + } catch { + return { paths: [], root: undefined, supported: false }; + } + } + async #probeUntilHealthy(host: IWorkspaceContainerAPI, deadline: number): Promise { let delay = this.#options.healthRetryInitialDelayMs; let lastError: unknown; diff --git a/packages/computer/src/backends/container/ignore-assertion.test.ts b/packages/computer/src/backends/container/ignore-assertion.test.ts new file mode 100644 index 00000000..7f1f513b --- /dev/null +++ b/packages/computer/src/backends/container/ignore-assertion.test.ts @@ -0,0 +1,202 @@ +import { describe, expect, test } from "vitest"; + +import { + assertIgnoreMatches, + ContainerIgnoreMismatchError, + diffIgnore, + type ResolvedIgnore, + readIgnoreReport, +} from "./ignore-assertion.js"; + +// U5 client half for #179. The ignore set belongs to the image; this is +// the check that stops a client silently disagreeing with it. +// +// The failure being guarded is slow rather than loud: a stale or absent +// MOUNT_IGNORE looks exactly like a correct one until a dependency tree +// is written and pulled into the DO. So most of these tests are about +// the check firing, not about it passing. + +const supported = (paths: string[]): ResolvedIgnore => ({ + paths, + root: "/tmp/workspace", + supported: true, +}); + +describe("readIgnoreReport", () => { + test("reads the block computerd reports", () => { + const resolved = readIgnoreReport({ + backend: { kind: "fuse" }, + ignore: { + supported: true, + enabled: true, + root: "/tmp/workspace", + paths: ["node_modules", "dist"], + redundant: [], + }, + }); + expect(resolved).toEqual({ + paths: ["node_modules", "dist"], + root: "/tmp/workspace", + supported: true, + }); + }); + + test("treats a computerd with no ignore block as unsupported", () => { + // The old-image case, and the one most likely to occur in practice. + // Not a parse error: absence is a meaningful answer. + const resolved = readIgnoreReport({ backend: { kind: "fuse" }, mountPoint: "/workspace" }); + expect(resolved).toEqual({ paths: [], root: undefined, supported: false }); + }); + + test("treats a malformed block as unsupported rather than throwing", () => { + expect(readIgnoreReport({ ignore: null }).supported).toBe(false); + expect(readIgnoreReport({ ignore: "yes" }).supported).toBe(false); + expect(readIgnoreReport({ ignore: { supported: false } }).supported).toBe(false); + expect(readIgnoreReport(null).supported).toBe(false); + expect(readIgnoreReport(undefined).supported).toBe(false); + }); + + test("defaults paths to empty when the block omits them", () => { + const resolved = readIgnoreReport({ ignore: { supported: true, root: "/tmp/x" } }); + expect(resolved).toEqual({ paths: [], root: "/tmp/x", supported: true }); + }); +}); + +describe("diffIgnore", () => { + test("agrees when the sets match", () => { + expect(diffIgnore(["node_modules", "dist"], ["node_modules", "dist"])).toBeNull(); + }); + + test("ignores declaration order", () => { + // computerd reports in declaration order after dropping redundant + // entries; a host listing the same paths differently means the same. + expect(diffIgnore(["dist", "node_modules"], ["node_modules", "dist"])).toBeNull(); + }); + + test("ignores slash decoration on either side", () => { + expect(diffIgnore(["/dist/", "node_modules"], ["dist", "node_modules"])).toBeNull(); + }); + + test("collapses duplicates in the declaration", () => { + // computerd would have collapsed them, so the client must too or + // every duplicated entry becomes a spurious mismatch. + expect(diffIgnore(["dist", "dist"], ["dist"])).toBeNull(); + }); + + test("reports a path the image does not apply", () => { + expect(diffIgnore(["node_modules", "dist"], ["node_modules"])).toEqual({ + missing: ["dist"], + unexpected: [], + }); + }); + + test("reports a path the image applies but the caller did not declare", () => { + expect(diffIgnore(["node_modules"], ["node_modules", "target"])).toEqual({ + missing: [], + unexpected: ["target"], + }); + }); + + test("reports both directions at once", () => { + expect(diffIgnore(["a", "b"], ["b", "c"])).toEqual({ missing: ["a"], unexpected: ["c"] }); + }); + + test("an empty declaration against a configured image is a mismatch", () => { + // Distinct from omitting `ignore` entirely, which skips the check. + // Declaring "nothing is local-only" against an image that makes + // node_modules local-only is a real disagreement. + expect(diffIgnore([], ["node_modules"])).toEqual({ + missing: [], + unexpected: ["node_modules"], + }); + }); +}); + +describe("assertIgnoreMatches", () => { + test("omitting the declaration skips the check", () => { + // The default. Adopting this option is opt-in, so an existing + // deployment cannot start failing because a new field appeared. + expect(() => assertIgnoreMatches(undefined, supported(["node_modules"]))).not.toThrow(); + expect(() => + assertIgnoreMatches(undefined, { paths: [], root: undefined, supported: false }), + ).not.toThrow(); + }); + + test("passes when the declaration matches", () => { + expect(() => + assertIgnoreMatches(["node_modules", "dist"], supported(["node_modules", "dist"])), + ).not.toThrow(); + }); + + test("rejects a computerd that does not support the feature", () => { + // README warns the computerd image can lag the pinned client. An + // old image would otherwise look like it is working while quietly + // syncing a full node_modules. + expect(() => + assertIgnoreMatches(["node_modules"], { paths: [], root: undefined, supported: false }), + ).toThrow(ContainerIgnoreMismatchError); + expect(() => + assertIgnoreMatches(["node_modules"], { paths: [], root: undefined, supported: false }), + ).toThrow(/does not support local-only paths/); + }); + + test("the unsupported message says what the consequence is", () => { + // Not just "mismatch". The operator needs to know the paths will be + // pulled into the DO, which is the expensive part. + try { + assertIgnoreMatches(["node_modules"], { paths: [], root: undefined, supported: false }); + expect.unreachable("should have thrown"); + } catch (error) { + expect((error as Error).message).toMatch(/pulled into the Durable Object/); + expect((error as Error).message).toMatch(/Upgrade the computerd image/); + } + }); + + test("names which paths will be synced when the image is missing one", () => { + try { + assertIgnoreMatches(["node_modules", "dist"], supported(["node_modules"])); + expect.unreachable("should have thrown"); + } catch (error) { + const message = (error as Error).message; + expect(message).toMatch(/"dist"/); + expect(message).toMatch(/WILL be synced/); + } + }); + + test("names which paths will not be synced when the image adds one", () => { + // The opposite direction is just as dangerous: the caller believes + // `target` is durable and it is not. + try { + assertIgnoreMatches(["node_modules"], supported(["node_modules", "target"])); + expect.unreachable("should have thrown"); + } catch (error) { + const message = (error as Error).message; + expect(message).toMatch(/"target"/); + expect(message).toMatch(/will NOT be synced/); + } + }); + + test("explains that the image owns the set", () => { + // Without this the natural reaction is to change the client option, + // which cannot fix anything. + try { + assertIgnoreMatches(["a"], supported(["b"])); + expect.unreachable("should have thrown"); + } catch (error) { + expect((error as Error).message).toMatch(/property of the image \(MOUNT_IGNORE\)/); + } + }); + + test("carries the declared and actual sets on the error", () => { + // So a host can log or reconcile them without parsing the message. + try { + assertIgnoreMatches(["a"], supported(["b"])); + expect.unreachable("should have thrown"); + } catch (error) { + const mismatch = error as ContainerIgnoreMismatchError; + expect(mismatch.declared).toEqual(["a"]); + expect(mismatch.actual).toEqual(["b"]); + expect(mismatch.supported).toBe(true); + } + }); +}); diff --git a/packages/computer/src/backends/container/ignore-assertion.ts b/packages/computer/src/backends/container/ignore-assertion.ts new file mode 100644 index 00000000..94c6442f --- /dev/null +++ b/packages/computer/src/backends/container/ignore-assertion.ts @@ -0,0 +1,187 @@ +// Client-side assertion over the container's local-only path set. +// +// The ignore set is owned by the *image*, not the client: computerd +// reads MOUNT_IGNORE at startup, normalises it, and reports the result +// on /__computerd/info. A client cannot change it. What a client can do +// is state what it believes the image is configured for and refuse to +// connect when the image disagrees. +// +// That inversion is deliberate. The mount is per-container and compiled +// once, so two sessions sharing an image cannot hold different views of +// which paths are durable. Making `ignore` a setting would promise a +// per-session knob the architecture cannot honour. +// +// Why fail the connection rather than warn. The failure mode this +// guards is silent and expensive: an image built without MOUNT_IGNORE, +// or with a stale set, looks identical to a correct one until a command +// writes a large dependency tree and the whole thing is pulled into the +// Durable Object. That is the exact symptom #179 reports -- a timeout, +// then a storage-timeout cascade the workspace does not recover from. +// A mismatch here is a deployment error, and a loud one is cheaper than +// a slow one. +// +// This module is pure. The fetch and the connect-time wiring live in +// cloudflare-container.ts; everything here is a function of two values, +// so the comparison and its message can be tested without a container. + +/** The `ignore` block computerd reports on /__computerd/info. */ +export interface ComputerdIgnoreReport { + readonly supported?: boolean; + readonly enabled?: boolean; + readonly root?: string; + readonly paths?: readonly string[]; + readonly redundant?: readonly string[]; + readonly fastPaths?: Readonly>; +} + +/** What the backend exposes back to the host after a successful connect. */ +export interface ResolvedIgnore { + /** + * Paths the mount treats as local-only, normalised by computerd. + * + * Empty when the feature is off, which is indistinguishable from + * "configured with no entries" -- correctly so, since they behave + * identically. + */ + readonly paths: readonly string[]; + /** Resolved MOUNT_IGNORE_PATH, or undefined when unsupported. */ + readonly root: string | undefined; + /** + * False on a computerd predating the feature. + * + * Lets a host degrade deliberately rather than discovering the gap + * through a three-minute pull. + */ + readonly supported: boolean; +} + +export class ContainerIgnoreMismatchError extends Error { + readonly declared: readonly string[]; + readonly actual: readonly string[]; + readonly supported: boolean; + + constructor( + message: string, + details: { declared: readonly string[]; actual: readonly string[]; supported: boolean }, + ) { + super(message); + this.name = "ContainerIgnoreMismatchError"; + this.declared = details.declared; + this.actual = details.actual; + this.supported = details.supported; + } +} + +/** + * Reads the `ignore` block out of a /__computerd/info body. + * + * Tolerant by design: an older computerd has no such block, and that is + * a supported answer (`supported: false`) rather than a parse error. + * The caller decides whether it is acceptable. + */ +export function readIgnoreReport(info: unknown): ResolvedIgnore { + if (typeof info !== "object" || info === null || !("ignore" in info)) { + return { paths: [], root: undefined, supported: false }; + } + const report = (info as { ignore?: unknown }).ignore; + if (typeof report !== "object" || report === null) { + return { paths: [], root: undefined, supported: false }; + } + const typed = report as ComputerdIgnoreReport; + if (typed.supported !== true) { + return { paths: [], root: undefined, supported: false }; + } + return { + paths: Array.isArray(typed.paths) ? [...typed.paths] : [], + root: typeof typed.root === "string" ? typed.root : undefined, + supported: true, + }; +} + +/** + * Compares a declared set against what the image actually applies. + * + * Order-insensitive: computerd reports entries in declaration order + * after dropping redundant ones, and a host that lists the same paths + * in a different order means the same thing. Duplicates in the + * declaration are collapsed for the same reason -- computerd would have + * collapsed them too. + * + * Returns null when they agree. + */ +export function diffIgnore( + declared: readonly string[], + actual: readonly string[], +): { missing: string[]; unexpected: string[] } | null { + const declaredSet = new Set(declared.map(normalise)); + const actualSet = new Set(actual.map(normalise)); + + const missing = [...declaredSet].filter((entry) => !actualSet.has(entry)).sort(); + const unexpected = [...actualSet].filter((entry) => !declaredSet.has(entry)).sort(); + + if (missing.length === 0 && unexpected.length === 0) return null; + return { missing, unexpected }; +} + +/** + * Throws when the image disagrees with what the caller declared. + * + * `declared === undefined` skips the check entirely and accepts + * whatever the image provides. That is the default, so adopting this + * option is opt-in and an existing deployment cannot start failing + * because a new field appeared. + */ +export function assertIgnoreMatches( + declared: readonly string[] | undefined, + resolved: ResolvedIgnore, +): void { + if (declared === undefined) return; + + if (!resolved.supported) { + throw new ContainerIgnoreMismatchError( + `This container's computerd does not support local-only paths, but ` + + `\`ignore\` declared ${formatList(declared)}. Those paths would be ` + + `recorded in the workspace and pulled into the Durable Object. ` + + `Upgrade the computerd image, or remove \`ignore\` to accept the ` + + `container's behaviour.`, + { declared: [...declared], actual: [], supported: false }, + ); + } + + const difference = diffIgnore(declared, resolved.paths); + if (difference === null) return; + + const parts: string[] = []; + if (difference.missing.length > 0) { + parts.push( + `declared but not applied by the image: ${formatList(difference.missing)} ` + + `(these paths WILL be synced)`, + ); + } + if (difference.unexpected.length > 0) { + parts.push( + `applied by the image but not declared: ${formatList(difference.unexpected)} ` + + `(these paths will NOT be synced)`, + ); + } + + throw new ContainerIgnoreMismatchError( + `Container ignore set does not match \`ignore\`: ${parts.join("; ")}. ` + + `The set is a property of the image (MOUNT_IGNORE), not of this ` + + `client; \`ignore\` only asserts what the image is expected to apply. ` + + `Rebuild the image or update the declaration so the two agree.`, + { declared: [...declared], actual: [...resolved.paths], supported: true }, + ); +} + +function normalise(entry: string): string { + let value = entry.trim(); + while (value.startsWith("/")) value = value.slice(1); + while (value.endsWith("/")) value = value.slice(0, -1); + return value; +} + +function formatList(entries: readonly string[]): string { + if (entries.length === 0) return "(none)"; + return entries.map((entry) => JSON.stringify(entry)).join(", "); +} From ed780a50051d066303f82548c5ab28d1571324ff Mon Sep 17 00:00:00 2001 From: Aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:40:43 +0000 Subject: [PATCH 04/15] computerd: log the fix on a cross-boundary rename 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. --- .changeset/exdev-guidance-log.md | 17 +++++ .../computerd/src/fuse/passthrough.test.ts | 70 +++++++++++++++++++ packages/computerd/src/fuse/passthrough.ts | 48 ++++++++++++- 3 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 .changeset/exdev-guidance-log.md diff --git a/.changeset/exdev-guidance-log.md b/.changeset/exdev-guidance-log.md new file mode 100644 index 00000000..63cbf0f1 --- /dev/null +++ b/.changeset/exdev-guidance-log.md @@ -0,0 +1,17 @@ +--- +"@cloudflare/computerd": patch +--- + +Log actionable guidance on the first cross-boundary rename + +A rename between a `MOUNT_IGNORE` path and a synced one returns `EXDEV`, +which reaches the caller as `cross-device link` — an unhelpful message on a +path that is plainly not a device. + +computerd now logs once per mount naming both sides, why the rename cannot be +atomic, and the entry to add to `MOUNT_IGNORE` to make it atomic. Build tools +that stage into a sibling directory and rename into place are the common +cause, and the fix is almost always to ignore the staging path too. + +Once per mount rather than per rename: a build that does this does it in a +loop. diff --git a/packages/computerd/src/fuse/passthrough.test.ts b/packages/computerd/src/fuse/passthrough.test.ts index dd4448cd..a53d334c 100644 --- a/packages/computerd/src/fuse/passthrough.test.ts +++ b/packages/computerd/src/fuse/passthrough.test.ts @@ -255,6 +255,10 @@ describe("withLocalPassthrough: rename", () => { root, ignore: resolveMountIgnore(paths, MOUNT), mountPoint: MOUNT, + // Swallowed rather than left on console.warn: the crossing-rename + // guidance is asserted in its own test above, and a suite that + // prints it on every run trains people to ignore the output. + warn: () => {}, }); return { ops, calls: source.calls }; }; @@ -276,6 +280,72 @@ describe("withLocalPassthrough: rename", () => { expect(calls).toEqual(["rename"]); }); + test("logs the fix once on the first crossing rename", () => { + // The errno is all the kernel can carry, and "cross-device link" on + // a path that is not a device is where an operator loses an + // afternoon. The guidance has to reach them somewhere, so it goes + // to the log -- and only once, because a build that does this does + // it in a loop. + const warnings: string[] = []; + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["dist"], MOUNT), + mountPoint: MOUNT, + warn: (message) => warnings.push(message), + }); + + ops.rename("/.tmp-build", "/dist", () => {}); + expect(warnings).toHaveLength(1); + + const [message] = warnings; + expect(message).toMatch(/EXDEV/); + // Which side is which, so the reader does not have to work it out. + expect(message).toMatch(/\/dist is container-local/); + expect(message).toMatch(/\/\.tmp-build is synced/); + // Why it is not just done anyway. + expect(message).toMatch(/cannot be atomic/); + // And the actual fix: ignore the staging directory too. + expect(message).toMatch(/add "\.tmp-build" to MOUNT_IGNORE/); + + // Repeats stay silent. + ops.rename("/.tmp-build", "/dist", () => {}); + ops.rename("/dist/x", "/y", () => {}); + expect(warnings).toHaveLength(1); + }); + + test("counts every crossing rename even though it logs once", () => { + const warnings: string[] = []; + const source = recordingOps(); + const { ops, stats } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["dist"], MOUNT), + mountPoint: MOUNT, + warn: (message) => warnings.push(message), + }); + + ops.rename("/.tmp-build", "/dist", () => {}); + ops.rename("/.tmp-two", "/dist", () => {}); + expect(stats().crossLayerRenames).toBe(2); + expect(warnings).toHaveLength(1); + }); + + test("does not log for a rename that stays within one layer", () => { + const warnings: string[] = []; + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["dist"], MOUNT), + mountPoint: MOUNT, + warn: (message) => warnings.push(message), + }); + + ops.create("/dist/a", 0o644, () => {}); + ops.rename("/dist/a", "/dist/b", () => {}); + ops.rename("/src/a.ts", "/src/b.ts", () => {}); + expect(warnings).toEqual([]); + }); + test("returns EXDEV when a rename crosses the boundary", () => { // Not a copy. The two sides are different filesystems, so the // operation cannot be atomic, and faking it would turn a crash diff --git a/packages/computerd/src/fuse/passthrough.ts b/packages/computerd/src/fuse/passthrough.ts index 30cbc673..f5011484 100644 --- a/packages/computerd/src/fuse/passthrough.ts +++ b/packages/computerd/src/fuse/passthrough.ts @@ -91,6 +91,8 @@ export interface LocalPassthroughOptions { readonly fs?: PassthroughFs; /** Called once per distinct local-only directory created. Diagnostics. */ readonly onMaterialise?: (relativePath: string) => void; + /** Operator-facing warnings. Defaults to console.warn; injected for tests. */ + readonly warn?: (message: string) => void; } /** @@ -151,6 +153,8 @@ export interface PassthroughStats { readonly cacheHits: number; /** Open local file handles. */ readonly openHandles: number; + /** Renames refused with EXDEV for crossing the boundary. */ + readonly crossLayerRenames: number; } export interface LocalPassthrough { @@ -172,7 +176,13 @@ export function withLocalPassthrough( if (options.ignore.isEmpty) { return { ops, - stats: () => ({ localOps: 0, decisions: 0, cacheHits: 0, openHandles: 0 }), + stats: () => ({ + localOps: 0, + decisions: 0, + cacheHits: 0, + openHandles: 0, + crossLayerRenames: 0, + }), }; } @@ -183,6 +193,8 @@ export function withLocalPassthrough( let localOps = 0; let decisions = 0; let cacheHits = 0; + let crossLayerRenames = 0; + const warn = options.warn ?? ((message: string) => console.warn(message)); // The decision cache. Keyed by *directory*, not by file: ignored-ness // is inherited, so once a directory is known local-only every path @@ -523,6 +535,14 @@ export function withLocalPassthrough( // and a crash mid-copy would leave a half-written file where // the caller was promised all-or-nothing. Every tool already // handles EXDEV by falling back to copy-then-unlink. + // + // The errno is all the kernel can carry, and "cross-device + // link" on a path that is plainly not a device is the kind of + // message an operator loses an afternoon to. So the guidance + // goes to the log instead -- once per mount, because a build + // that does this does it in a loop and a per-rename line would + // bury everything else. + reportCrossLayerRename(source, destination, sourceLocal); cb(ERRNO.EXDEV); return; } @@ -634,6 +654,31 @@ export function withLocalPassthrough( return handle; } + function reportCrossLayerRename( + source: string, + destination: string, + sourceIsLocal: boolean, + ): void { + crossLayerRenames += 1; + if (crossLayerRenames > 1) return; + const localSide = sourceIsLocal ? source : destination; + const syncedSide = sourceIsLocal ? destination : source; + // Name the entry to add, not just the paths. The fix is almost + // always "ignore the staging directory too": build tools write into + // a sibling and rename into place, so a destination that is + // local-only while its staging path is not produces exactly this. + const suggestion = toRelative(syncedSide, mountRoot) || syncedSide; + warn( + `computerd: rename ${source} -> ${destination} crossed the local-only ` + + `boundary and returned EXDEV. ${localSide} is container-local ` + + `(MOUNT_IGNORE), ${syncedSide} is synced to the workspace; a rename ` + + `between them cannot be atomic, so it is refused rather than ` + + `silently copied. Most callers fall back to copy-then-unlink. To ` + + `keep the rename atomic, add "${suggestion}" to MOUNT_IGNORE as ` + + `well. Further occurrences are not logged.`, + ); + } + function markDirectory(path: string): void { const relative = toRelative(path, mountRoot); if (relative !== "") directoryDecisions.set(relative, true); @@ -686,6 +731,7 @@ export function withLocalPassthrough( decisions, cacheHits, openHandles: handles.size, + crossLayerRenames, }), }; } From d6b36141c9cd041218d5a361e97d24db5d20c86b Mon Sep 17 00:00:00 2001 From: Aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:40:57 +0000 Subject: [PATCH 05/15] docs: document local-only paths 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. --- docs/19_performance.md | 44 +++++++ docs/20_local_only_paths.md | 250 ++++++++++++++++++++++++++++++++++++ docs/README.md | 1 + 3 files changed, 295 insertions(+) create mode 100644 docs/20_local_only_paths.md diff --git a/docs/19_performance.md b/docs/19_performance.md index f3605dcf..2d5181dd 100644 --- a/docs/19_performance.md +++ b/docs/19_performance.md @@ -48,6 +48,50 @@ computerd is ~2x slower than the container's ext4 disk for the full `npm install`, and ~3.6x slower than tmpfs. The disk comparison is the more realistic baseline for general usage. +> [!IMPORTANT] +> These numbers measure the **mount**, not the **pull**. They stop when +> `npm install` returns. What follows — moving 36,675 files into the +> Durable Object — is not counted here, and for a dependency tree it is +> the larger cost. +> +> [#179](https://github.com/cloudflare/computer/issues/179) reports an +> install timing out at 120 s and then taking ~3 further minutes to +> return while the partial `node_modules` was pulled, after which the +> next command failed with a storage timeout the workspace did not +> recover from. None of that is visible in the table above. +> +> If you are sizing a workload against these figures, add the transfer +> yourself, or keep the tree out of sync entirely — see +> [20. Local-only paths](./20_local_only_paths.md). + +## Local-only paths (`MOUNT_IGNORE`) + +A path listed in `MOUNT_IGNORE` is served from the container's disk and +never enters the VFS, the store, the change-pack encoding, or the pull. + +What this does **not** change is the FUSE round trip: the bytes still +cross from the kernel into the daemon. Passthrough (`FOPEN_PASSTHROUGH`) +would remove that too, but computerd mounts through `fuse-native`, which +binds libfuse 2.9, and passthrough needs the libfuse 3.17 API. So expect +a local-only `npm install` to track the `computerd FUSE` row above +rather than the `ext4 disk` row. + +The saving is the transfer, and for a dependency tree the transfer is +most of the wall clock. + +| Scenario | Install duration | Bytes pulled into the DO | +|---|---:|---:| +| `npm install` to a synced path | 124.7 s | *(not yet measured)* | +| `npm install` to a `MOUNT_IGNORE` path | *(not yet measured)* | 0 by construction | + +> [!NOTE] +> The empty cells are deliberate. Bytes-pulled is the load-bearing +> number for this feature and it has not been measured yet; the zero in +> the last cell is a property of the design — an ignored path produces +> no sync entries — not an observation. Fill the table from the same +> `cloudflare/sandbox-sdk` install used above, on the same instance +> type, before quoting any of it. + ## In-memory store versus on-disk store `computerd` keeps its SQLite store in memory by default. Set diff --git a/docs/20_local_only_paths.md b/docs/20_local_only_paths.md new file mode 100644 index 00000000..9c725726 --- /dev/null +++ b/docs/20_local_only_paths.md @@ -0,0 +1,250 @@ +# 20. Local-only paths + +> [!NOTE] +> Addresses [#179](https://github.com/cloudflare/computer/issues/179). +> Shipped behaviour as of `@cloudflare/computerd` with `MOUNT_IGNORE` +> support; the client-side assertion ships in `@cloudflare/computer`. + +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. It is wrong for `node_modules`, `.venv`, `target/`, +`dist/` and caches: tens of thousands of rebuildable files that never +need to be durable, and whose transfer can take minutes. + +`MOUNT_IGNORE` names paths that stay on the container's local disk +instead. They are never recorded in the VFS, never pushed, and never +pulled. + +## The trade-off, stated plainly + +This is the part to read before configuring anything. + +Content under a local-only path: + +- **is visible only inside the container.** Commands see it through the + mount as usual. `workspace.fs`, the worker shell, and any host-side + tool reading through the Workspace do not. +- **is not durable on its own.** It is absent from sync by design, so a + container that is replaced without a snapshot restore loses it. +- **survives a container snapshot**, because `MOUNT_IGNORE_PATH` is a + real filesystem path. That is the only durability it has, and the + reason the default sits under `/tmp` rather than on a tmpfs. + +That is the right trade for a dependency tree a package manager can +rebuild. It is the wrong trade for anything a user typed. + +## Configuration + +The set belongs to the **image**, not the client. + +```dockerfile +ENV MOUNT_POINT=/workspace +ENV MOUNT_IGNORE_PATH=/tmp/workspace # default: /tmp + $MOUNT_POINT +ENV MOUNT_IGNORE="node_modules +.venv +target +dist" +``` + +`MOUNT_IGNORE` is newline-delimited, because a path may legally contain +a comma or a space. Blank lines and `#` comments are skipped. + +Entries are **plain paths relative to the mount root**. There is no glob +syntax and no negation: an entry names one location, and a path is +local-only if it equals that entry or sits beneath it. + +| Entry | Means | +| --- | --- | +| `node_modules` | `$MOUNT_POINT/node_modules` and everything under it | +| `app/node_modules` | that one path, not `node_modules` elsewhere | +| `/dist` | the same as `dist`; a leading slash is accepted and stripped | + +Note the second row. An entry does **not** match at every depth, so a +monorepo that clones packages into `app/`, `web/` and `api/` lists each +`/node_modules` separately. That is more lines in a `Dockerfile`, +and in exchange the set of paths that lose durability is a list you can +read rather than a pattern language whose matches you have to work out. + +### Why it is per-image + +The mount is per-container and compiled once at startup, so two sessions +sharing an image cannot hold different views of which paths are durable. +A per-session option would promise a knob the architecture cannot +honour. + +Clients may still *declare* what they expect, and +`CloudflareContainerBackend` will refuse to connect if the image +disagrees: + +```ts +new CloudflareContainerBackend({ + container: env.CONTAINER, + workspace: { binding: "SESSIONS", id: sessionId }, + ignore: ["node_modules", ".venv", "target", "dist"], +}); +``` + +This is an assertion, not a setting. Omit it to accept whatever the +image provides. Supplying it is how a deployment notices an image +rebuilt with a changed or missing `MOUNT_IGNORE` — which otherwise +surfaces only as a large, slow, unexplained pull. + +The resolved set is readable back off the handle: + +```ts +const handle = await backend.connect(); +handle.ignore; // { paths, root, supported } +``` + +`supported: false` means the container predates the feature, so every +path is synced regardless of configuration. Worth logging. + +### Validation + +`MOUNT_IGNORE_PATH` must be absolute, must not be the filesystem root, +and must not be equal to or inside `MOUNT_POINT` — a root inside the +mount would make the passthrough layer resolve into itself. Entries may +not contain `.` or `..` segments; an entry that walks out of the mount +is a configuration mistake, and silently clamping it would hide the +mistake behind a path that looks intentional. + +All of these fail the daemon at startup rather than disabling the +feature quietly. A dropped entry means a full `node_modules` goes into +the Durable Object, which is the failure this exists to prevent. + +Duplicates, and entries nested inside another entry, are dropped as +redundant and reported. + +## Diagnostics + +`/__computerd/info` reports the resolved configuration: + +```jsonc +{ + "ignore": { + "supported": true, + "enabled": true, + "root": "/tmp/workspace", + "paths": ["node_modules", "dist"], + "redundant": ["node_modules/.cache"], + "fastPaths": { + "passthrough": false, + "passthroughReason": "fuse-native binds libfuse 2.9; FOPEN_PASSTHROUGH requires the libfuse 3.17 API", + "writebackCache": false + } + } +} +``` + +`paths` is the **normalised** set — what the mount actually applies, not +what was typed. That is what makes the client-side assertion meaningful. + +`fastPaths.passthrough` is `false` on current builds and this is +expected, not a fault. See [Performance](#performance). + +## Renames across the boundary: `EXDEV` + +The one runtime failure a correctly configured deployment can still hit, +and the one worth understanding before it happens. + +A rename whose source and destination sit on opposite sides of the +boundary returns **`EXDEV`** (`cross-device link`). + +``` +$ mv .tmp-build dist +mv: cannot move '.tmp-build' to 'dist': Invalid cross-device link +``` + +### Why it is not just done anyway + +The two sides are different filesystems. A rename between them cannot be +atomic, and `rename(2)` promises atomicity. computerd could copy the +bytes and unlink the source, and the operation would appear to succeed — +but a crash midway would leave a half-written file where the caller was +promised all-or-nothing. Faking atomicity is worse than refusing it, +because the failure it creates is silent and arrives later. + +`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. + +### The fix + +Ignore the staging path alongside its destination. + +This case is common because it is how build tools work: write into a +temporary sibling, then rename into place atomically. If the destination +is local-only and the staging directory is not, every build hits this. + +```dockerfile +ENV MOUNT_IGNORE="dist +.tmp-build" +``` + +Candidates worth checking in your own image: `.next` (Next.js writes +through `.next/cache`), `.turbo`, `node_modules/.cache`, and any +`*.tmp` staging directory a bundler creates next to its output. + +computerd logs the guidance on the first crossing rename per mount, +naming both sides and the entry to add: + +``` +computerd: rename /.tmp-build -> /dist crossed the local-only boundary and +returned EXDEV. /dist is container-local (MOUNT_IGNORE), /.tmp-build is +synced to the workspace; a rename between them cannot be atomic, so it is +refused rather than silently copied. Most callers fall back to +copy-then-unlink. To keep the rename atomic, add ".tmp-build" to +MOUNT_IGNORE as well. Further occurrences are not logged. +``` + +It logs once because a build that does this does it in a loop. The +per-mount count is available on the passthrough stats. + +Renames **within** one layer are ordinary atomic renames, in both the +local-only layer and the VFS. + +## Performance + +The win is in what is skipped: an ignored write does not enter the VFS, +the SQLite store, the change-pack encoding, or the pull into the Durable +Object. For a dependency install, that is the difference between +transferring tens of thousands of files and transferring none. + +What is **not** skipped is the FUSE round trip itself. The bytes still +cross from the kernel into the daemon before reaching local disk. + +> [!NOTE] +> This is why `fastPaths.passthrough` reports `false`. The kernel +> supports `FOPEN_PASSTHROUGH` (6.9+), and Cloudflare Containers hosts +> are well above that line — but computerd mounts through +> `fuse-native`, which binds **libfuse 2.9**, and passthrough requires +> the libfuse 3.17 API. The same constraint rules out writeback caching, +> which libfuse 2.9 rejects at mount time. +> +> So expect a local-only path to perform like the existing mount +> (see [19. Performance](./19_performance.md), roughly 2x slower than +> raw disk for `npm install`), not like raw disk. The saving is the +> transfer, not the I/O. This flips on a binding upgrade rather than an +> infrastructure change, which is why the field is reported rather than +> omitted. + +## Relationship to `sync.fetchChanges({ ignore })` + +These are different mechanisms with a confusingly similar name. + +| | `MOUNT_IGNORE` | `fetchChanges({ ignore })` | +| --- | --- | --- | +| Layer | FUSE mount | sync RPC | +| Effect | path never enters the VFS | path is skipped in this transfer | +| Scope | durability boundary | transfer filter | + +If you built a wrapper that injects `ignore` into `fetchChanges` to keep +a dependency tree out of the Durable Object, `MOUNT_IGNORE` replaces it +— **delete the wrapper rather than keeping both**. Keeping both leaves +the path excluded from transfer while still occupying the container's +store, which is the half-fixed state `MOUNT_IGNORE` exists to resolve. + +Note also that `ignore` appears on both `fetchChanges` and the optional +`fetchChangePack` overload. A wrapper covering only the first silently +bypasses the filter on exactly the large transfers it was written for. diff --git a/docs/README.md b/docs/README.md index 06ca0d7a..6c2a0e3e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -250,6 +250,7 @@ above, then dive into the area you're working on. | [17. Isolate JavaScript runtime](./17_isolate_javascript.md) | ECMAScript modules, durable imports, configured libraries, durable `node:fs/promises`, trusted `ws:git` / `ws:artifacts`, and managed lifecycle. | | [18. Runtime migration](./18_runtime_migration.md) | Breaking preview-API mappings from public shell and script-execution surfaces to `workspace.runtime`. | | [19. Performance](./19_performance.md) | Filesystem benchmarks: `fs-bench` numbers, an `npm install` comparison, and how to reproduce them. | +| [20. Local-only paths](./20_local_only_paths.md) | `MOUNT_IGNORE`: keeping `node_modules` and other rebuildable trees on the container's disk, the durability trade-off, and the `EXDEV` rename contract. | ## High-level API From 4577a887f27c39cfd1037af685f5d20110c09d48 Mon Sep 17 00:00:00 2001 From: Aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:12:50 +0000 Subject: [PATCH 06/15] chore: trim comments to load-bearing rationale 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. --- .../container-backend-ignore.test.ts | 3 - .../container/ignore-assertion.test.ts | 5 +- .../backends/container/ignore-assertion.ts | 69 ++++---------- packages/computerd/src/fuse/ignore-config.ts | 42 +++------ packages/computerd/src/fuse/ignore.test.ts | 10 +- packages/computerd/src/fuse/ignore.ts | 91 ++++--------------- .../computerd/src/fuse/passthrough.test.ts | 11 +-- packages/computerd/src/fuse/passthrough.ts | 37 +++----- 8 files changed, 68 insertions(+), 200 deletions(-) diff --git a/packages/computer/src/backends/container/container-backend-ignore.test.ts b/packages/computer/src/backends/container/container-backend-ignore.test.ts index 3f344478..8e6c7fd2 100644 --- a/packages/computer/src/backends/container/container-backend-ignore.test.ts +++ b/packages/computer/src/backends/container/container-backend-ignore.test.ts @@ -1,6 +1,3 @@ -// Local-only paths (#179). The ignore set belongs to the image; the -// backend reads it back on connect() and refuses a disagreement. -// // connect()'s happy path constructs a WebSocketPair, a workerd global // the node runner does not provide, so the full dial cannot complete // here. These exercise the wire format the backend depends on, against diff --git a/packages/computer/src/backends/container/ignore-assertion.test.ts b/packages/computer/src/backends/container/ignore-assertion.test.ts index 7f1f513b..f497803c 100644 --- a/packages/computer/src/backends/container/ignore-assertion.test.ts +++ b/packages/computer/src/backends/container/ignore-assertion.test.ts @@ -8,10 +8,7 @@ import { readIgnoreReport, } from "./ignore-assertion.js"; -// U5 client half for #179. The ignore set belongs to the image; this is -// the check that stops a client silently disagreeing with it. -// -// The failure being guarded is slow rather than loud: a stale or absent +// The failure guarded here is slow rather than loud: a stale or absent // MOUNT_IGNORE looks exactly like a correct one until a dependency tree // is written and pulled into the DO. So most of these tests are about // the check firing, not about it passing. diff --git a/packages/computer/src/backends/container/ignore-assertion.ts b/packages/computer/src/backends/container/ignore-assertion.ts index 94c6442f..96800651 100644 --- a/packages/computer/src/backends/container/ignore-assertion.ts +++ b/packages/computer/src/backends/container/ignore-assertion.ts @@ -1,28 +1,13 @@ -// Client-side assertion over the container's local-only path set. +// Client-side assertion over the container's local-only path set. The +// set is owned by the image; a client can only state what it expects +// and refuse to connect on disagreement. See docs/20_local_only_paths.md. // -// The ignore set is owned by the *image*, not the client: computerd -// reads MOUNT_IGNORE at startup, normalises it, and reports the result -// on /__computerd/info. A client cannot change it. What a client can do -// is state what it believes the image is configured for and refuse to -// connect when the image disagrees. -// -// That inversion is deliberate. The mount is per-container and compiled -// once, so two sessions sharing an image cannot hold different views of -// which paths are durable. Making `ignore` a setting would promise a -// per-session knob the architecture cannot honour. -// -// Why fail the connection rather than warn. The failure mode this -// guards is silent and expensive: an image built without MOUNT_IGNORE, -// or with a stale set, looks identical to a correct one until a command -// writes a large dependency tree and the whole thing is pulled into the -// Durable Object. That is the exact symptom #179 reports -- a timeout, -// then a storage-timeout cascade the workspace does not recover from. -// A mismatch here is a deployment error, and a loud one is cheaper than -// a slow one. -// -// This module is pure. The fetch and the connect-time wiring live in -// cloudflare-container.ts; everything here is a function of two values, -// so the comparison and its message can be tested without a container. +// Fails the connection rather than warning, because the failure it +// guards is silent and expensive: an image built without MOUNT_IGNORE +// looks identical to a correct one until a command writes a large +// dependency tree and the whole thing is pulled into the Durable +// Object -- the #179 symptom. A mismatch is a deployment error, and a +// loud one is cheaper than a slow one. /** The `ignore` block computerd reports on /__computerd/info. */ export interface ComputerdIgnoreReport { @@ -36,22 +21,11 @@ export interface ComputerdIgnoreReport { /** What the backend exposes back to the host after a successful connect. */ export interface ResolvedIgnore { - /** - * Paths the mount treats as local-only, normalised by computerd. - * - * Empty when the feature is off, which is indistinguishable from - * "configured with no entries" -- correctly so, since they behave - * identically. - */ + /** Empty when the feature is off, indistinguishable from "no entries". */ readonly paths: readonly string[]; /** Resolved MOUNT_IGNORE_PATH, or undefined when unsupported. */ readonly root: string | undefined; - /** - * False on a computerd predating the feature. - * - * Lets a host degrade deliberately rather than discovering the gap - * through a three-minute pull. - */ + /** False on a computerd predating the feature, so a host can degrade. */ readonly supported: boolean; } @@ -99,15 +73,9 @@ export function readIgnoreReport(info: unknown): ResolvedIgnore { } /** - * Compares a declared set against what the image actually applies. - * - * Order-insensitive: computerd reports entries in declaration order - * after dropping redundant ones, and a host that lists the same paths - * in a different order means the same thing. Duplicates in the - * declaration are collapsed for the same reason -- computerd would have - * collapsed them too. - * - * Returns null when they agree. + * Compares a declared set against what the image applies; null when they + * agree. Order-insensitive and duplicate-collapsing, because computerd + * normalises the same way and the two spellings mean the same thing. */ export function diffIgnore( declared: readonly string[], @@ -124,12 +92,9 @@ export function diffIgnore( } /** - * Throws when the image disagrees with what the caller declared. - * - * `declared === undefined` skips the check entirely and accepts - * whatever the image provides. That is the default, so adopting this - * option is opt-in and an existing deployment cannot start failing - * because a new field appeared. + * Throws when the image disagrees. `declared === undefined` skips the + * check, so an existing deployment cannot start failing because a new + * field appeared. */ export function assertIgnoreMatches( declared: readonly string[] | undefined, diff --git a/packages/computerd/src/fuse/ignore-config.ts b/packages/computerd/src/fuse/ignore-config.ts index 1488a97d..44421f8e 100644 --- a/packages/computerd/src/fuse/ignore-config.ts +++ b/packages/computerd/src/fuse/ignore-config.ts @@ -1,14 +1,9 @@ -// Startup resolution of the local-only path configuration. +// Startup resolution of the local-only path configuration. Kept apart +// from ignore.ts so the matcher stays a pure function of its inputs. // -// Reads MOUNT_IGNORE and MOUNT_IGNORE_PATH, validates them against the -// mount point, and produces the value the driver and /__computerd/info -// both consume. Kept apart from ignore.ts so the matcher stays a pure -// function of its inputs with no env or filesystem opinions. -// -// Everything here fails closed. A misconfiguration that silently -// disabled the feature would send a full node_modules into the Durable -// Object, which is the exact failure #179 is about, so the daemon -// refuses to mount instead. +// Fails closed: a misconfiguration that silently disabled the feature +// would send a full node_modules into the Durable Object, the exact +// failure #179 is about, so the daemon refuses to mount instead. import { isAbsolute, join, resolve } from "node:path"; @@ -29,13 +24,9 @@ export interface MountIgnoreEnv { } /** - * Default root: /tmp + the mount point. - * - * Under /tmp rather than a tmpfs or an anonymous volume so a container - * snapshot captures it. That is the only durability local-only content - * has -- it is deliberately absent from sync -- so putting it somewhere - * a snapshot misses would make container replacement silently lose the - * tree this feature exists to keep. + * Default root: /tmp + the mount point. Under /tmp rather than a tmpfs + * so a container snapshot captures it -- that is the only durability + * local-only content has, being deliberately absent from sync. */ export function defaultIgnoreRoot(mountPoint: string): string { return join("/tmp", mountPoint); @@ -61,10 +52,9 @@ export function resolveMountIgnoreConfig( const normalisedRoot = resolve(root).replace(/\/+$/, "") || "/"; const normalisedMount = resolve(mountPoint).replace(/\/+$/, "") || "/"; - // The store must not live inside the thing it shadows. A root under - // the mount would make the passthrough layer resolve into itself: - // every write to an ignored path would land at a location that is - // also an ignored path, one level deeper, forever. + // A root under the mount would make the passthrough layer resolve into + // itself: every write to an ignored path would land at a location that + // is also an ignored path, one level deeper, forever. if (normalisedRoot === normalisedMount || normalisedRoot.startsWith(`${normalisedMount}/`)) { throw new Error( `MOUNT_IGNORE_PATH (${normalisedRoot}) must not be inside MOUNT_POINT ` + @@ -88,13 +78,9 @@ export interface MountIgnoreInfo { readonly redundant: readonly string[]; readonly fastPaths: { /** - * FUSE passthrough (FOPEN_PASSTHROUGH). - * - * Always false on this build, and reported rather than omitted so - * the reason is visible without reading the source. computerd mounts - * through fuse-native, which binds libfuse 2.9; passthrough needs - * the libfuse 3.17 API. The host kernel supports it, so this flips - * on a binding change rather than an infrastructure change. + * Always false: fuse-native binds libfuse 2.9, passthrough needs the + * libfuse 3.17 API. Reported rather than omitted so the reason is + * visible without reading the source. */ readonly passthrough: false; readonly passthroughReason: string; diff --git a/packages/computerd/src/fuse/ignore.test.ts b/packages/computerd/src/fuse/ignore.test.ts index 25eadbbb..eeb075ea 100644 --- a/packages/computerd/src/fuse/ignore.test.ts +++ b/packages/computerd/src/fuse/ignore.test.ts @@ -2,13 +2,9 @@ import { describe, expect, test } from "vitest"; import { MountIgnorePathError, parseMountIgnore, resolveMountIgnore } from "./ignore.js"; -// Local-only subpaths for #179. Entries are plain paths relative to -// the mount root: no globs, no negation, no depth matching. -// -// The case worth writing first is the segment boundary. A naive -// `startsWith` passes every other test in this file and fails -// "does not treat node_modules_extra as node_modules", so that test -// is what actually pins the matcher. +// A naive `startsWith` passes every other test in this file and fails +// "does not treat node_modules_extra as node_modules", so that test is +// what actually pins the matcher. describe("parseMountIgnore", () => { test("splits MOUNT_IGNORE on newlines", () => { diff --git a/packages/computerd/src/fuse/ignore.ts b/packages/computerd/src/fuse/ignore.ts index f49b67ba..af49b5d0 100644 --- a/packages/computerd/src/fuse/ignore.ts +++ b/packages/computerd/src/fuse/ignore.ts @@ -1,47 +1,18 @@ -// Local-only subpaths of the mount. +// Local-only subpaths of the mount. See docs/20_local_only_paths.md. // -// Addresses #179: 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. Paths listed here -// pass through to local disk instead, are never recorded in the VFS, -// and are never pushed or pulled. +// Entries are plain paths relative to the mount root: no glob syntax +// and no negation. Deliberate, because an entry then resolves to a +// known location and the mapping onto MOUNT_IGNORE_PATH is a prefix +// substitution decided at startup, which an unanchored pattern cannot +// answer until a path arrives to match against it. // -// Entries are plain paths relative to the mount root. There is 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, not a shortcut. Three things follow -// from it that a pattern language does not give you: -// -// - An entry resolves to a known location, so the mapping onto -// MOUNT_IGNORE_PATH is a prefix substitution decided at startup. -// An unanchored pattern has no single answer to "where does this -// live on disk" until a path arrives to match against it. -// - Matching is a segment-aware prefix test, which the driver's -// per-inode decision cache collapses to one lookup per directory. -// - The set of paths that silently lose durability is reviewable by -// reading it. That matters more here than expressiveness, because -// the cost of a wrong entry is data that exists only inside one -// container. -// -// The one real limitation is that `node_modules` does not match at -// every depth. A monorepo cloning packages into app/, web/ and api/ -// lists each `/node_modules`. That is more lines in a Dockerfile -// and nothing more. If depth matching is ever needed, a single -// leading `**/` form is the smallest addition that stays resolvable; -// add it on evidence rather than in anticipation. -// -// The set is fixed for the life of the mount. It is resolved once at -// startup from MOUNT_IGNORE and never re-read: entries that changed -// under a running command would mean migrating already-materialised -// paths between layers mid-write. +// The set is resolved once at startup and never re-read: entries that +// changed under a running command would mean migrating +// already-materialised paths between layers mid-write. /** An entry that cannot be used, carrying enough context to fix it. */ export class MountIgnorePathError extends Error { readonly entry: string; - /** Index into the entry list, so a long MOUNT_IGNORE is diagnosable. */ readonly index: number; constructor(message: string, entry: string, index: number) { @@ -53,18 +24,9 @@ export class MountIgnorePathError extends Error { } export interface MountIgnoreSet { - /** - * Whether a mount-relative path is local-only. - * - * True when the path equals an entry or is a descendant of one. - * Matching is segment-aware, so the entry `node_modules` does not - * match `node_modules_extra`. - */ + /** Segment-aware: `node_modules` does not match `node_modules_extra`. */ readonly ignores: (relativePath: string) => boolean; - /** - * The entry covering a path, for error messages and diagnostics. - * Undefined when the path is not local-only. - */ + /** The entry covering a path, or undefined when not local-only. */ readonly entryFor: (relativePath: string) => string | undefined; /** Normalised entries, in declaration order, as the mount applies them. */ readonly paths: readonly string[]; @@ -74,12 +36,8 @@ export interface MountIgnoreSet { } /** - * Splits a raw MOUNT_IGNORE value into entries. - * * Newline-delimited rather than comma- or space-separated because a - * path may legally contain a comma or a space. Blank lines and `#` - * comments are skipped so a generated block stays readable in a - * Dockerfile ENV. + * path may legally contain a comma or a space. */ export function parseMountIgnore(raw: string | undefined): string[] { if (raw === undefined) return []; @@ -96,11 +54,8 @@ export function parseMountIgnore(raw: string | undefined): string[] { } /** - * Normalises entries and builds the matcher. - * - * `mountPoint` lets an absolute path under the mount be written as - * `/workspace/dist`, which is the obvious thing to reach for. An - * absolute path outside the mount is rejected rather than reinterpreted. + * Normalises entries and builds the matcher. An absolute path outside + * the mount is rejected rather than reinterpreted. */ export function resolveMountIgnore(entries: readonly string[], mountPoint = "/"): MountIgnoreSet { const root = normaliseMount(mountPoint); @@ -111,7 +66,6 @@ export function resolveMountIgnore(entries: readonly string[], mountPoint = "/") let value = original.trim(); if (value.startsWith("/")) { - // Absolute. Accept it only if it names something inside the mount. if (root !== "/" && (value === root || value.startsWith(`${root}/`))) { value = value.slice(root.length); } else if (root !== "/") { @@ -135,9 +89,8 @@ export function resolveMountIgnore(entries: readonly string[], mountPoint = "/") } const segments = trimmed.split("/"); - // `.` and `..` are rejected rather than resolved. An entry that walks - // out of the mount is a configuration mistake, and silently clamping - // it would hide the mistake behind a path that looks intentional. + // Rejected rather than resolved: silently clamping an entry that walks + // out of the mount would hide the mistake behind a plausible path. if (segments.some((segment) => segment === "." || segment === "..")) { throw new MountIgnorePathError( `Entry ${JSON.stringify(original)} contains a "." or ".." segment. ` + @@ -154,8 +107,7 @@ export function resolveMountIgnore(entries: readonly string[], mountPoint = "/") ); } - // Duplicates and entries nested inside an existing one are dropped: - // keeping `node_modules/.cache` alongside `node_modules` would imply + // Keeping `node_modules/.cache` alongside `node_modules` would imply // it does something, and it cannot. const covered = paths.some((existing) => isAtOrUnder(trimmed, existing)); if (covered) { @@ -193,14 +145,7 @@ export function resolveMountIgnore(entries: readonly string[], mountPoint = "/") }; } -/** - * Whether `path` is `entry` or sits beneath it. - * - * The separator check is what makes this segment-aware. A plain - * `startsWith` would report `node_modules_extra` as being under - * `node_modules`, which is the single easiest way to get this wrong and - * the reason the near-miss has its own test. - */ +/** The separator check is what stops `node_modules_extra` matching. */ function isAtOrUnder(path: string, entry: string): boolean { return path === entry || path.startsWith(`${entry}/`); } diff --git a/packages/computerd/src/fuse/passthrough.test.ts b/packages/computerd/src/fuse/passthrough.test.ts index a53d334c..b17395b3 100644 --- a/packages/computerd/src/fuse/passthrough.test.ts +++ b/packages/computerd/src/fuse/passthrough.test.ts @@ -8,13 +8,10 @@ import type { FuseOps } from "./driver.js"; import { resolveMountIgnore } from "./ignore.js"; import { withLocalPassthrough } from "./passthrough.js"; -// U2 (decision layer) and U3 (local I/O) for #179. -// -// These drive the real node:fs against a temp directory rather than a -// double. The module's whole purpose is to put bytes on a real -// filesystem, so a mock would be asserting the shape of the calls -// rather than the behaviour, and the interesting failures here -- EXDEV, -// ENOTEMPTY, parent creation -- are the filesystem's, not ours. +// Drives the real node:fs against a temp directory rather than a double. +// The interesting failures here -- EXDEV, ENOTEMPTY, parent creation -- +// are the filesystem's, so a mock would assert the shape of the calls +// rather than the behaviour. const MOUNT = "/workspace"; diff --git a/packages/computerd/src/fuse/passthrough.ts b/packages/computerd/src/fuse/passthrough.ts index f5011484..1e2df11d 100644 --- a/packages/computerd/src/fuse/passthrough.ts +++ b/packages/computerd/src/fuse/passthrough.ts @@ -1,31 +1,16 @@ -// Local-only passthrough for the FUSE op layer. +// Local-only passthrough for the FUSE op layer. See +// docs/20_local_only_paths.md. // -// U2 and U3 of the #179 work. `ignore.ts` decides *which* paths are -// local-only; this decides what happens when one is touched. A matching -// path is served from a real directory on the container's disk -// (MOUNT_IGNORE_PATH) instead of the VFS, so it is never recorded, -// never pushed, and never pulled. +// A decorator over FuseOps rather than branches inside makeFUSEOps, so +// the VFS driver stays unaware of the feature and an empty ignore set +// is provably a no-op: `withLocalPassthrough` returns the source object +// unchanged. // -// Implemented as a decorator over FuseOps rather than as branches -// inside makeFUSEOps. Three reasons, in order of how much they matter: -// -// - The VFS driver stays unaware of the feature. Every path that is -// not local-only reaches exactly the code it reaches today, so the -// blast radius of a bug here is bounded by the ignore set. -// - Disabling the feature is provably a no-op: with an empty set, -// `withLocalPassthrough` returns the source object unchanged. -// - It matches how the tracer already composes (`tracer.ts`), so the -// mount path gains one more wrap rather than a new shape. -// -// WHAT THIS IS NOT. There is no FUSE passthrough (FOPEN_PASSTHROUGH) -// here, despite the name being the natural one for the concept. The -// host kernel supports it, but computerd mounts through fuse-native, -// which binds libfuse 2.9 and compiles well below the API version that -// can negotiate it. So data still crosses the FUSE boundary into this -// process; what it skips is the VFS, the SQLite store, the change-pack -// encoding, and the pull into the Durable Object. That is the win, and -// it is a large one, but it is not "the daemon leaves the data path". -// See DESIGN "The fuse-native constraint". +// Despite the name there is no FUSE passthrough (FOPEN_PASSTHROUGH) +// here; fuse-native binds libfuse 2.9, below the API version that can +// negotiate it. Data still crosses the FUSE boundary into this process. +// What it skips is the VFS, the SQLite store, the change-pack encoding, +// and the pull into the Durable Object. // // Writes go straight to the host filesystem with pwrite rather than // through the buffered FileEntry machinery in driver.ts. That buffering From 6e34a3da2fc4e9898ccd359053c824490d05bb82 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:12:34 +0100 Subject: [PATCH 07/15] feat: configure MOUNT_IGNORE at container start, as comma-separated paths 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. --- .changeset/container-ignore-assertion.md | 15 +-- docs/20_local_only_paths.md | 93 ++++++++++--------- .../container-backend-ignore.test.ts | 41 +++++++- .../backends/container/container-backend.ts | 21 +++-- .../container/ignore-assertion.test.ts | 56 +++++++++-- .../backends/container/ignore-assertion.ts | 51 ++++++++-- .../computerd/src/fuse/ignore-config.test.ts | 12 +-- packages/computerd/src/fuse/ignore.test.ts | 44 +++++---- packages/computerd/src/fuse/ignore.ts | 24 ++--- 9 files changed, 231 insertions(+), 126 deletions(-) diff --git a/.changeset/container-ignore-assertion.md b/.changeset/container-ignore-assertion.md index 56407d8a..2a91c2f4 100644 --- a/.changeset/container-ignore-assertion.md +++ b/.changeset/container-ignore-assertion.md @@ -2,17 +2,4 @@ "@cloudflare/computer": minor --- -Add `ignore` to `CloudflareContainerBackend`: assert the container's local-only paths - -`CloudflareContainerBackend` now reads the container's local-only path set -from `/__computerd/info` and exposes it as `handle.ignore`. - -Pass `ignore` to declare which paths the image is expected to keep on local -disk. `connect()` rejects when the container disagrees, including when it -predates the feature entirely — an image built without `MOUNT_IGNORE` -otherwise looks identical to a correct one until a large dependency tree is -written and pulled into the Durable Object. - -The option is a declaration, not a setting: the set belongs to the image, -which reads `MOUNT_IGNORE` at startup. Omitting `ignore` accepts whatever the -container provides, so existing deployments are unaffected. +Add `ignore` to `ContainerBackend` to configure pass-through to the container disk. diff --git a/docs/20_local_only_paths.md b/docs/20_local_only_paths.md index 9c725726..62335c97 100644 --- a/docs/20_local_only_paths.md +++ b/docs/20_local_only_paths.md @@ -35,29 +35,37 @@ rebuild. It is the wrong trade for anything a user typed. ## Configuration -The set belongs to the **image**, not the client. - -```dockerfile -ENV MOUNT_POINT=/workspace -ENV MOUNT_IGNORE_PATH=/tmp/workspace # default: /tmp + $MOUNT_POINT -ENV MOUNT_IGNORE="node_modules -.venv -target -dist" +Set it on the backend. The paths are passed to the container as +`MOUNT_IGNORE` in its **start environment**, so changing the set is a +deployment change rather than an image rebuild. + +```ts +new CloudflareContainerBackend({ + container: env.CONTAINER, + workspace: { binding: "SESSIONS", id: sessionId }, + ignore: ["/node_modules", "/.venv", "/dist"], +}); ``` -`MOUNT_IGNORE` is newline-delimited, because a path may legally contain -a comma or a space. Blank lines and `#` comments are skipped. +That becomes `MOUNT_IGNORE=/node_modules,/.venv,/dist` in the container. +Setting the variable directly — in `containerEnv`, or in a Dockerfile — +works too and takes precedence. -Entries are **plain paths relative to the mount root**. There is no glob -syntax and no negation: an entry names one location, and a path is +`MOUNT_IGNORE` is a **comma-separated list of paths anchored at the +mount root**. A leading `/` means the mount root, not the filesystem +root, so `/node_modules` is `$MOUNT_POINT/node_modules`. There is no +glob syntax and no negation: an entry names one location, and a path is local-only if it equals that entry or sits beneath it. | Entry | Means | | --- | --- | -| `node_modules` | `$MOUNT_POINT/node_modules` and everything under it | -| `app/node_modules` | that one path, not `node_modules` elsewhere | -| `/dist` | the same as `dist`; a leading slash is accepted and stripped | +| `/node_modules` | `$MOUNT_POINT/node_modules` and everything under it | +| `/app/node_modules` | that one path, not `node_modules` elsewhere | +| `/workspace/dist` | the fully-qualified form of `/dist`, when the mount is `/workspace` | + +A path containing a comma cannot be expressed. `MOUNT_IGNORE_PATH` still +sets where local-only content is stored, defaulting to `/tmp` + +`$MOUNT_POINT`. Note the second row. An entry does **not** match at every depth, so a monorepo that clones packages into `app/`, `web/` and `api/` lists each @@ -65,37 +73,37 @@ monorepo that clones packages into `app/`, `web/` and `api/` lists each and in exchange the set of paths that lose durability is a list you can read rather than a pattern language whose matches you have to work out. -### Why it is per-image +### It is still per-container -The mount is per-container and compiled once at startup, so two sessions -sharing an image cannot hold different views of which paths are durable. -A per-session option would promise a knob the architecture cannot -honour. +The mount is compiled once at container startup, so the set cannot change +under a running container and two sessions sharing one container cannot +hold different views of which paths are durable. Passing `ignore` at +start time is what makes it a per-deployment value rather than a per-image +one; it is not a per-session knob. -Clients may still *declare* what they expect, and -`CloudflareContainerBackend` will refuse to connect if the image -disagrees: +`connect()` reads the resolved set back off `/__computerd/info` and +refuses the connection if it disagrees with what was declared. That is +what catches a computerd too old to honour `MOUNT_IGNORE`, which would +otherwise surface only as a large, slow, unexplained pull. -```ts -new CloudflareContainerBackend({ - container: env.CONTAINER, - workspace: { binding: "SESSIONS", id: sessionId }, - ignore: ["node_modules", ".venv", "target", "dist"], -}); -``` - -This is an assertion, not a setting. Omit it to accept whatever the -image provides. Supplying it is how a deployment notices an image -rebuilt with a changed or missing `MOUNT_IGNORE` — which otherwise -surfaces only as a large, slow, unexplained pull. - -The resolved set is readable back off the handle: +The resolved set is readable back off the handle, as **absolute paths +inside the container**: ```ts const handle = await backend.connect(); -handle.ignore; // { paths, root, supported } +handle.ignore; +// { +// paths: ["/workspace/node_modules", "/workspace/.venv", "/workspace/dist"], +// root: "/tmp/workspace", +// mountPoint: "/workspace", +// supported: true, +// } ``` +`paths` is `MOUNT_POINT` joined with each entry, so it can be used +against a container path without re-deriving the mount. `root` is where +that content actually lives on the container's disk. + `supported: false` means the container predates the feature, so every path is synced regardless of configuration. Worth logging. @@ -177,12 +185,11 @@ This case is common because it is how build tools work: write into a temporary sibling, then rename into place atomically. If the destination is local-only and the staging directory is not, every build hits this. -```dockerfile -ENV MOUNT_IGNORE="dist -.tmp-build" +```ts +ignore: ["/dist", "/.tmp-build"]; ``` -Candidates worth checking in your own image: `.next` (Next.js writes +Candidates worth checking in your own deployment: `.next` (Next.js writes through `.next/cache`), `.turbo`, `node_modules/.cache`, and any `*.tmp` staging directory a bundler creates next to its output. diff --git a/packages/computer/src/backends/container/container-backend-ignore.test.ts b/packages/computer/src/backends/container/container-backend-ignore.test.ts index 8e6c7fd2..bdf0cb75 100644 --- a/packages/computer/src/backends/container/container-backend-ignore.test.ts +++ b/packages/computer/src/backends/container/container-backend-ignore.test.ts @@ -6,6 +6,7 @@ // covered in computerd's cli tests against a real FUSE mount. import { describe, expect, test } from "vitest"; +import { ContainerBackend } from "./container-backend.js"; import type { ContainerRuntimeInfo, IWorkspaceContainerAPI } from "./container-host.js"; import type { ContainerLaunchSpec } from "./container-launch-record.js"; import { readIgnoreReport } from "./ignore-assertion.js"; @@ -81,8 +82,9 @@ describe("ContainerBackend local-only paths", () => { }, }); expect(await readInfo(host)).toEqual({ - paths: ["node_modules", "dist"], + paths: ["/workspace/node_modules", "/workspace/dist"], root: "/tmp/workspace", + mountPoint: "/workspace", supported: true, }); }); @@ -95,6 +97,7 @@ describe("ContainerBackend local-only paths", () => { expect(await readInfo(host)).toEqual({ paths: [], root: undefined, + mountPoint: undefined, supported: false, }); }); @@ -108,4 +111,40 @@ describe("ContainerBackend local-only paths", () => { await host.fetchPort(8080, "http://container/__computerd/info"); expect(fetches).toContainEqual({ port: 8080, path: "/__computerd/info" }); }); + + const backendWith = (host: IWorkspaceContainerAPI, ignore?: readonly string[]) => + new ContainerBackend({ + container: () => ({ getWorkspaceContainer: () => host }), + workspace: { binding: "SESSIONS", id: "session-1" }, + restartAttempts: 0, + connectTimeoutMs: 400, + healthProbeTimeoutMs: 50, + healthRetryInitialDelayMs: 10, + healthRetryMaxDelayMs: 20, + heartbeatIntervalMs: 0, + ...(ignore === undefined ? {} : { ignore }), + }); + + test("passes `ignore` to the container as MOUNT_IGNORE at start time", async () => { + // The set is deployment config, not image config: it has to arrive + // in the start environment or the image would have to be rebuilt to + // change it. + const { host, starts } = fakeHost(); + await backendWith(host, ["/node_modules", "/.venv", "/dist"]) + .connect() + .catch(() => undefined); + + expect(starts).toHaveLength(1); + expect(starts[0]?.env?.MOUNT_IGNORE).toBe("/node_modules,/.venv,/dist"); + }); + + test("sends no MOUNT_IGNORE when `ignore` is omitted", async () => { + const { host, starts } = fakeHost(); + await backendWith(host) + .connect() + .catch(() => undefined); + + expect(starts).toHaveLength(1); + expect(starts[0]?.env?.MOUNT_IGNORE).toBeUndefined(); + }); }); diff --git a/packages/computer/src/backends/container/container-backend.ts b/packages/computer/src/backends/container/container-backend.ts index d3e33d52..b13275e3 100644 --- a/packages/computer/src/backends/container/container-backend.ts +++ b/packages/computer/src/backends/container/container-backend.ts @@ -112,15 +112,13 @@ export interface ContainerBackendOptions { heartbeatIntervalMs?: number; // Paths the container keeps on its local disk instead of the - // workspace (#179). Declared, not configured: the set belongs to the - // image, which reads MOUNT_IGNORE at startup. This states what the - // image is expected to apply, and connect() refuses the connection - // when it disagrees. + // workspace (#179). Written as mount-relative absolute paths + // ("/node_modules"), and passed to the container at start time as + // MOUNT_IGNORE. // - // Omit to accept whatever the image provides. Supplying it is how a - // deployment catches an image rebuilt with a changed or missing - // MOUNT_IGNORE, which otherwise surfaces only as a large unexpected - // pull into the Durable Object. + // 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[]; // Number of forced restart attempts after startup readiness @@ -301,6 +299,9 @@ export class ContainerBackend implements WorkspaceBackend { const env = { PORT: String(this.#options.containerPort), MOUNT_POINT: "/workspace", + ...(this.#options.ignore !== undefined + ? { MOUNT_IGNORE: this.#options.ignore.join(",") } + : {}), ...this.#options.containerEnv, }; let runtimeId: string; @@ -642,10 +643,10 @@ export class ContainerBackend implements WorkspaceBackend { "http://container/__computerd/info", { signal: AbortSignal.timeout(this.#options.healthProbeTimeoutMs) }, ); - if (!res.ok) return { paths: [], root: undefined, supported: false }; + if (!res.ok) return { paths: [], root: undefined, mountPoint: undefined, supported: false }; return readIgnoreReport(await res.json()); } catch { - return { paths: [], root: undefined, supported: false }; + return { paths: [], root: undefined, mountPoint: undefined, supported: false }; } } diff --git a/packages/computer/src/backends/container/ignore-assertion.test.ts b/packages/computer/src/backends/container/ignore-assertion.test.ts index f497803c..9e07f843 100644 --- a/packages/computer/src/backends/container/ignore-assertion.test.ts +++ b/packages/computer/src/backends/container/ignore-assertion.test.ts @@ -16,13 +16,17 @@ import { const supported = (paths: string[]): ResolvedIgnore => ({ paths, root: "/tmp/workspace", + mountPoint: "/workspace", supported: true, }); describe("readIgnoreReport", () => { - test("reads the block computerd reports", () => { + test("reports paths as absolute container paths under the mount", () => { + // computerd reports mount-relative; the host wants something it can + // use against a container path without re-deriving the mount point. const resolved = readIgnoreReport({ backend: { kind: "fuse" }, + mountPoint: "/workspace", ignore: { supported: true, enabled: true, @@ -32,8 +36,9 @@ describe("readIgnoreReport", () => { }, }); expect(resolved).toEqual({ - paths: ["node_modules", "dist"], + paths: ["/workspace/node_modules", "/workspace/dist"], root: "/tmp/workspace", + mountPoint: "/workspace", supported: true, }); }); @@ -42,7 +47,12 @@ describe("readIgnoreReport", () => { // The old-image case, and the one most likely to occur in practice. // Not a parse error: absence is a meaningful answer. const resolved = readIgnoreReport({ backend: { kind: "fuse" }, mountPoint: "/workspace" }); - expect(resolved).toEqual({ paths: [], root: undefined, supported: false }); + expect(resolved).toEqual({ + paths: [], + root: undefined, + mountPoint: undefined, + supported: false, + }); }); test("treats a malformed block as unsupported rather than throwing", () => { @@ -54,8 +64,16 @@ describe("readIgnoreReport", () => { }); test("defaults paths to empty when the block omits them", () => { - const resolved = readIgnoreReport({ ignore: { supported: true, root: "/tmp/x" } }); - expect(resolved).toEqual({ paths: [], root: "/tmp/x", supported: true }); + const resolved = readIgnoreReport({ + mountPoint: "/workspace", + ignore: { supported: true, root: "/tmp/x" }, + }); + expect(resolved).toEqual({ + paths: [], + root: "/tmp/x", + mountPoint: "/workspace", + supported: true, + }); }); }); @@ -115,7 +133,12 @@ describe("assertIgnoreMatches", () => { // deployment cannot start failing because a new field appeared. expect(() => assertIgnoreMatches(undefined, supported(["node_modules"]))).not.toThrow(); expect(() => - assertIgnoreMatches(undefined, { paths: [], root: undefined, supported: false }), + assertIgnoreMatches(undefined, { + paths: [], + root: undefined, + mountPoint: undefined, + supported: false, + }), ).not.toThrow(); }); @@ -130,10 +153,20 @@ describe("assertIgnoreMatches", () => { // old image would otherwise look like it is working while quietly // syncing a full node_modules. expect(() => - assertIgnoreMatches(["node_modules"], { paths: [], root: undefined, supported: false }), + assertIgnoreMatches(["node_modules"], { + paths: [], + root: undefined, + mountPoint: undefined, + supported: false, + }), ).toThrow(ContainerIgnoreMismatchError); expect(() => - assertIgnoreMatches(["node_modules"], { paths: [], root: undefined, supported: false }), + assertIgnoreMatches(["node_modules"], { + paths: [], + root: undefined, + mountPoint: undefined, + supported: false, + }), ).toThrow(/does not support local-only paths/); }); @@ -141,7 +174,12 @@ describe("assertIgnoreMatches", () => { // Not just "mismatch". The operator needs to know the paths will be // pulled into the DO, which is the expensive part. try { - assertIgnoreMatches(["node_modules"], { paths: [], root: undefined, supported: false }); + assertIgnoreMatches(["node_modules"], { + paths: [], + root: undefined, + mountPoint: undefined, + supported: false, + }); expect.unreachable("should have thrown"); } catch (error) { expect((error as Error).message).toMatch(/pulled into the Durable Object/); diff --git a/packages/computer/src/backends/container/ignore-assertion.ts b/packages/computer/src/backends/container/ignore-assertion.ts index 96800651..0d5d68d2 100644 --- a/packages/computer/src/backends/container/ignore-assertion.ts +++ b/packages/computer/src/backends/container/ignore-assertion.ts @@ -21,14 +21,31 @@ export interface ComputerdIgnoreReport { /** What the backend exposes back to the host after a successful connect. */ export interface ResolvedIgnore { - /** Empty when the feature is off, indistinguishable from "no entries". */ + /** + * Absolute paths as they exist inside the container, under MOUNT_POINT. + * `node_modules` with a mount of /workspace reports /workspace/node_modules, + * so the value can be used directly against a container path without the + * caller re-deriving the mount. Empty when the feature is off. + */ readonly paths: readonly string[]; - /** Resolved MOUNT_IGNORE_PATH, or undefined when unsupported. */ + /** + * Where local-only content is stored on the container's disk + * (MOUNT_IGNORE_PATH). Undefined when unsupported. + */ readonly root: string | undefined; + /** The mount point the paths are rooted at. Undefined when unsupported. */ + readonly mountPoint: string | undefined; /** False on a computerd predating the feature, so a host can degrade. */ readonly supported: boolean; } +/** Joins a mount-relative entry onto the mount point. */ +function toContainerPath(entry: string, mountPoint: string): string { + const base = mountPoint.replace(/\/+$/, ""); + const rel = entry.replace(/^\/+/, ""); + return `${base}/${rel}`; +} + export class ContainerIgnoreMismatchError extends Error { readonly declared: readonly string[]; readonly actual: readonly string[]; @@ -55,19 +72,25 @@ export class ContainerIgnoreMismatchError extends Error { */ export function readIgnoreReport(info: unknown): ResolvedIgnore { if (typeof info !== "object" || info === null || !("ignore" in info)) { - return { paths: [], root: undefined, supported: false }; + return { paths: [], root: undefined, mountPoint: undefined, supported: false }; } const report = (info as { ignore?: unknown }).ignore; if (typeof report !== "object" || report === null) { - return { paths: [], root: undefined, supported: false }; + return { paths: [], root: undefined, mountPoint: undefined, supported: false }; } const typed = report as ComputerdIgnoreReport; if (typed.supported !== true) { - return { paths: [], root: undefined, supported: false }; + return { paths: [], root: undefined, mountPoint: undefined, supported: false }; } + // computerd reports entries mount-relative; the host wants paths it can + // use against the container directly, so they are joined onto the mount + // point from the same payload. + const mountPoint = (info as { mountPoint?: unknown }).mountPoint; + const base = typeof mountPoint === "string" && mountPoint !== "" ? mountPoint : "/workspace"; return { - paths: Array.isArray(typed.paths) ? [...typed.paths] : [], + paths: Array.isArray(typed.paths) ? typed.paths.map((e) => toContainerPath(e, base)) : [], root: typeof typed.root === "string" ? typed.root : undefined, + mountPoint: base, supported: true, }; } @@ -113,7 +136,10 @@ export function assertIgnoreMatches( ); } - const difference = diffIgnore(declared, resolved.paths); + // resolved.paths are absolute container paths; the declaration is written + // mount-relative ("/node_modules"), so compare on the mount-relative form. + const actualRelative = resolved.paths.map((path) => stripMount(path, resolved.mountPoint)); + const difference = diffIgnore(declared, actualRelative); if (difference === null) return; const parts: string[] = []; @@ -139,6 +165,17 @@ export function assertIgnoreMatches( ); } +/** + * Reduces an absolute container path to its mount-relative form, so a + * declaration and a report can be compared on the same footing. + */ +function stripMount(path: string, mountPoint: string | undefined): string { + if (mountPoint === undefined) return path; + const base = mountPoint.replace(/\/+$/, ""); + if (base !== "" && path.startsWith(`${base}/`)) return path.slice(base.length + 1); + return path; +} + function normalise(entry: string): string { let value = entry.trim(); while (value.startsWith("/")) value = value.slice(1); diff --git a/packages/computerd/src/fuse/ignore-config.test.ts b/packages/computerd/src/fuse/ignore-config.test.ts index d239d2b3..96d01f03 100644 --- a/packages/computerd/src/fuse/ignore-config.test.ts +++ b/packages/computerd/src/fuse/ignore-config.test.ts @@ -83,14 +83,14 @@ describe("resolveMountIgnoreConfig: the set", () => { expect(config.ignore.isEmpty).toBe(true); }); - test("is disabled when MOUNT_IGNORE is only comments and blanks", () => { - const config = resolveMountIgnoreConfig({ MOUNT_IGNORE: "# nothing\n\n \n" }, "/workspace"); + test("is disabled when MOUNT_IGNORE is only separators and blanks", () => { + const config = resolveMountIgnoreConfig({ MOUNT_IGNORE: " , , " }, "/workspace"); expect(config.enabled).toBe(false); }); test("resolves entries relative to the mount point", () => { const config = resolveMountIgnoreConfig( - { MOUNT_IGNORE: "node_modules\n/workspace/dist\n" }, + { MOUNT_IGNORE: "/node_modules,/workspace/dist" }, "/workspace", ); expect(config.enabled).toBe(true); @@ -100,19 +100,19 @@ describe("resolveMountIgnoreConfig: the set", () => { test("propagates a bad entry as a startup failure", () => { // Failing closed matters: a silently dropped entry sends a full // node_modules into the DO, which is the failure #179 is about. - expect(() => resolveMountIgnoreConfig({ MOUNT_IGNORE: "../escape" }, "/workspace")).toThrow(); + expect(() => resolveMountIgnoreConfig({ MOUNT_IGNORE: "/../escape" }, "/workspace")).toThrow(); }); }); describe("describeMountIgnore", () => { test("reports the normalised set and the redundant entries", () => { const config = resolveMountIgnoreConfig( - { MOUNT_IGNORE: "node_modules\nnode_modules/.cache\ndist" }, + { MOUNT_IGNORE: "/node_modules,/node_modules/.cache,/dist" }, "/workspace", ); const info = describeMountIgnore(config); expect(info.paths).toEqual(["node_modules", "dist"]); - expect(info.redundant).toEqual(["node_modules/.cache"]); + expect(info.redundant).toEqual(["/node_modules/.cache"]); expect(info.enabled).toBe(true); expect(info.root).toBe("/tmp/workspace"); }); diff --git a/packages/computerd/src/fuse/ignore.test.ts b/packages/computerd/src/fuse/ignore.test.ts index eeb075ea..851c8c5e 100644 --- a/packages/computerd/src/fuse/ignore.test.ts +++ b/packages/computerd/src/fuse/ignore.test.ts @@ -7,35 +7,30 @@ import { MountIgnorePathError, parseMountIgnore, resolveMountIgnore } from "./ig // what actually pins the matcher. describe("parseMountIgnore", () => { - test("splits MOUNT_IGNORE on newlines", () => { - expect(parseMountIgnore("node_modules\n.venv\ntarget")).toEqual([ - "node_modules", - ".venv", - "target", + test("splits MOUNT_IGNORE on commas", () => { + expect(parseMountIgnore("/node_modules,/.venv,/dist")).toEqual([ + "/node_modules", + "/.venv", + "/dist", ]); }); - test("skips blank lines and comments the way .gitignore does", () => { - const parsed = parseMountIgnore( - ["# build output", "", "dist", " ", "# deps", "node_modules", ""].join("\n"), - ); - expect(parsed).toEqual(["dist", "node_modules"]); + test("tolerates whitespace around entries", () => { + expect(parseMountIgnore("/node_modules , /dist")).toEqual(["/node_modules", "/dist"]); }); - test("keeps entries containing commas and spaces intact", () => { - // Why the env var is newline-delimited rather than comma- or - // space-separated: a path may legally contain either. - expect(parseMountIgnore("my dir\na,b.log")).toEqual(["my dir", "a,b.log"]); + test("skips empty fields from a trailing or doubled comma", () => { + expect(parseMountIgnore("/dist,,/node_modules,")).toEqual(["/dist", "/node_modules"]); }); - test("trims trailing whitespace but preserves an escaped trailing space", () => { - expect(parseMountIgnore("dist \nkeep\\ ")).toEqual(["dist", "keep\\ "]); + test("keeps entries containing spaces intact", () => { + expect(parseMountIgnore("/my dir,/dist")).toEqual(["/my dir", "/dist"]); }); test("treats an absent or empty value as the feature being off", () => { expect(parseMountIgnore(undefined)).toEqual([]); expect(parseMountIgnore("")).toEqual([]); - expect(parseMountIgnore("\n\n \n")).toEqual([]); + expect(parseMountIgnore(" , , ")).toEqual([]); }); }); @@ -122,9 +117,18 @@ describe("resolveMountIgnore: normalisation", () => { expect(set.ignores("dist/app.js")).toBe(true); }); - test("rejects an absolute path outside the mount point", () => { - expect(() => resolveMountIgnore(["/etc/passwd"], "/workspace")).toThrow(MountIgnorePathError); - expect(() => resolveMountIgnore(["/etc/passwd"], "/workspace")).toThrow(/outside the mount/); + test("anchors a leading slash at the mount root, not the filesystem root", () => { + // "/node_modules" means $MOUNT_POINT/node_modules. A path that looks + // like it names somewhere else on disk is still mount-relative, so + // the entry set can never reach outside the mount. + const set = resolveMountIgnore(["/etc/passwd"], "/workspace"); + expect(set.paths).toEqual(["etc/passwd"]); + expect(set.ignores("etc/passwd")).toBe(true); + }); + + test("accepts the fully-qualified form of the same path", () => { + const set = resolveMountIgnore(["/workspace/dist", "/dist"], "/workspace"); + expect(set.paths).toEqual(["dist"]); }); test("rejects a .. segment rather than resolving it", () => { diff --git a/packages/computerd/src/fuse/ignore.ts b/packages/computerd/src/fuse/ignore.ts index af49b5d0..2a5ec5d1 100644 --- a/packages/computerd/src/fuse/ignore.ts +++ b/packages/computerd/src/fuse/ignore.ts @@ -36,18 +36,15 @@ export interface MountIgnoreSet { } /** - * Newline-delimited rather than comma- or space-separated because a - * path may legally contain a comma or a space. + * Comma-separated, so the set can be passed as a single start-time + * environment variable. A path containing a comma cannot be expressed. */ export function parseMountIgnore(raw: string | undefined): string[] { if (raw === undefined) return []; const entries: string[] = []; - for (const line of raw.split("\n")) { - // Trailing whitespace is insignificant unless escaped, which is the - // only way to name a path ending in a space. - const trimmed = line.endsWith("\\ ") ? line.trimStart() : line.trim(); + for (const field of raw.split(",")) { + const trimmed = field.trim(); if (trimmed === "") continue; - if (trimmed.startsWith("#")) continue; entries.push(trimmed); } return entries; @@ -65,16 +62,11 @@ export function resolveMountIgnore(entries: readonly string[], mountPoint = "/") for (const [index, original] of entries.entries()) { let value = original.trim(); - if (value.startsWith("/")) { - if (root !== "/" && (value === root || value.startsWith(`${root}/`))) { + // A leading slash anchors the entry at the mount root, not at the + // filesystem root: "/node_modules" means "$MOUNT_POINT/node_modules". + if (value.startsWith("/") && root !== "/") { + if (value === root || value.startsWith(`${root}/`)) { value = value.slice(root.length); - } else if (root !== "/") { - throw new MountIgnorePathError( - `Entry ${JSON.stringify(original)} is an absolute path outside the ` + - `mount point ${JSON.stringify(root)}. Entries name paths within the mount.`, - original, - index, - ); } } From fc5dd8a49cab08c6dc08e1f69a1d2b07c34731e3 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:54:08 +0100 Subject: [PATCH 08/15] docs: move local-only path docs into the computerd README The MOUNT_IGNORE write-up described shipped computerd behaviour, which belongs with the package rather than in the forward-looking design specification. It now lives as a section of the computerd README, with the performance discussion left to the existing section in the performance doc. Links and source comments that pointed at the old file are updated. The two computerd changesets are removed. The packages version together, so the existing @cloudflare/computer changeset already carries the release, and a separate entry per implementation detail only duplicates it in the changelog. --- .changeset/exdev-guidance-log.md | 17 -- .changeset/local-only-mount-paths.md | 20 -- docs/19_performance.md | 2 +- docs/20_local_only_paths.md | 257 ------------------ docs/README.md | 1 - .../backends/container/ignore-assertion.ts | 2 +- packages/computerd/README.md | 129 +++++++++ packages/computerd/src/fuse/ignore.ts | 2 +- packages/computerd/src/fuse/passthrough.ts | 2 +- 9 files changed, 133 insertions(+), 299 deletions(-) delete mode 100644 .changeset/exdev-guidance-log.md delete mode 100644 .changeset/local-only-mount-paths.md delete mode 100644 docs/20_local_only_paths.md diff --git a/.changeset/exdev-guidance-log.md b/.changeset/exdev-guidance-log.md deleted file mode 100644 index 63cbf0f1..00000000 --- a/.changeset/exdev-guidance-log.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -"@cloudflare/computerd": patch ---- - -Log actionable guidance on the first cross-boundary rename - -A rename between a `MOUNT_IGNORE` path and a synced one returns `EXDEV`, -which reaches the caller as `cross-device link` — an unhelpful message on a -path that is plainly not a device. - -computerd now logs once per mount naming both sides, why the rename cannot be -atomic, and the entry to add to `MOUNT_IGNORE` to make it atomic. Build tools -that stage into a sibling directory and rename into place are the common -cause, and the fix is almost always to ignore the staging path too. - -Once per mount rather than per rename: a build that does this does it in a -loop. diff --git a/.changeset/local-only-mount-paths.md b/.changeset/local-only-mount-paths.md deleted file mode 100644 index 19c6da6d..00000000 --- a/.changeset/local-only-mount-paths.md +++ /dev/null @@ -1,20 +0,0 @@ ---- -"@cloudflare/computerd": minor ---- - -Add `MOUNT_IGNORE`: paths that stay on the container's local disk - -Everything a container command writes under `MOUNT_POINT` was 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/`, where -tens of thousands of rebuildable files never need to be durable. - -Set `MOUNT_IGNORE` to a newline-delimited list of paths relative to the mount -root. Matching paths are served from `MOUNT_IGNORE_PATH` (default -`/tmp/$MOUNT_POINT`) instead of the VFS, so they are never recorded, pushed, -or pulled. `/__computerd/info` reports the resolved set. - -The trade-off is deliberate: local-only content is invisible to `workspace.fs` -and the worker shell, and survives container replacement only through a -snapshot. A rename across the boundary returns `EXDEV` rather than being -silently turned into a non-atomic copy. diff --git a/docs/19_performance.md b/docs/19_performance.md index 2d5181dd..06ca987e 100644 --- a/docs/19_performance.md +++ b/docs/19_performance.md @@ -62,7 +62,7 @@ the more realistic baseline for general usage. > > If you are sizing a workload against these figures, add the transfer > yourself, or keep the tree out of sync entirely — see -> [20. Local-only paths](./20_local_only_paths.md). +> [`computerd`: Local-only paths](../packages/computerd/README.md#local-only-paths-mount_ignore). ## Local-only paths (`MOUNT_IGNORE`) diff --git a/docs/20_local_only_paths.md b/docs/20_local_only_paths.md deleted file mode 100644 index 62335c97..00000000 --- a/docs/20_local_only_paths.md +++ /dev/null @@ -1,257 +0,0 @@ -# 20. Local-only paths - -> [!NOTE] -> Addresses [#179](https://github.com/cloudflare/computer/issues/179). -> Shipped behaviour as of `@cloudflare/computerd` with `MOUNT_IGNORE` -> support; the client-side assertion ships in `@cloudflare/computer`. - -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. It is wrong for `node_modules`, `.venv`, `target/`, -`dist/` and caches: tens of thousands of rebuildable files that never -need to be durable, and whose transfer can take minutes. - -`MOUNT_IGNORE` names paths that stay on the container's local disk -instead. They are never recorded in the VFS, never pushed, and never -pulled. - -## The trade-off, stated plainly - -This is the part to read before configuring anything. - -Content under a local-only path: - -- **is visible only inside the container.** Commands see it through the - mount as usual. `workspace.fs`, the worker shell, and any host-side - tool reading through the Workspace do not. -- **is not durable on its own.** It is absent from sync by design, so a - container that is replaced without a snapshot restore loses it. -- **survives a container snapshot**, because `MOUNT_IGNORE_PATH` is a - real filesystem path. That is the only durability it has, and the - reason the default sits under `/tmp` rather than on a tmpfs. - -That is the right trade for a dependency tree a package manager can -rebuild. It is the wrong trade for anything a user typed. - -## Configuration - -Set it on the backend. The paths are passed to the container as -`MOUNT_IGNORE` in its **start environment**, so changing the set is a -deployment change rather than an image rebuild. - -```ts -new CloudflareContainerBackend({ - container: env.CONTAINER, - workspace: { binding: "SESSIONS", id: sessionId }, - ignore: ["/node_modules", "/.venv", "/dist"], -}); -``` - -That becomes `MOUNT_IGNORE=/node_modules,/.venv,/dist` in the container. -Setting the variable directly — in `containerEnv`, or in a Dockerfile — -works too and takes precedence. - -`MOUNT_IGNORE` is a **comma-separated list of paths anchored at the -mount root**. A leading `/` means the mount root, not the filesystem -root, so `/node_modules` is `$MOUNT_POINT/node_modules`. There is no -glob syntax and no negation: an entry names one location, and a path is -local-only if it equals that entry or sits beneath it. - -| Entry | Means | -| --- | --- | -| `/node_modules` | `$MOUNT_POINT/node_modules` and everything under it | -| `/app/node_modules` | that one path, not `node_modules` elsewhere | -| `/workspace/dist` | the fully-qualified form of `/dist`, when the mount is `/workspace` | - -A path containing a comma cannot be expressed. `MOUNT_IGNORE_PATH` still -sets where local-only content is stored, defaulting to `/tmp` + -`$MOUNT_POINT`. - -Note the second row. An entry does **not** match at every depth, so a -monorepo that clones packages into `app/`, `web/` and `api/` lists each -`/node_modules` separately. That is more lines in a `Dockerfile`, -and in exchange the set of paths that lose durability is a list you can -read rather than a pattern language whose matches you have to work out. - -### It is still per-container - -The mount is compiled once at container startup, so the set cannot change -under a running container and two sessions sharing one container cannot -hold different views of which paths are durable. Passing `ignore` at -start time is what makes it a per-deployment value rather than a per-image -one; it is not a per-session knob. - -`connect()` reads the resolved set back off `/__computerd/info` and -refuses the connection if it disagrees with what was declared. That is -what catches a computerd too old to honour `MOUNT_IGNORE`, which would -otherwise surface only as a large, slow, unexplained pull. - -The resolved set is readable back off the handle, as **absolute paths -inside the container**: - -```ts -const handle = await backend.connect(); -handle.ignore; -// { -// paths: ["/workspace/node_modules", "/workspace/.venv", "/workspace/dist"], -// root: "/tmp/workspace", -// mountPoint: "/workspace", -// supported: true, -// } -``` - -`paths` is `MOUNT_POINT` joined with each entry, so it can be used -against a container path without re-deriving the mount. `root` is where -that content actually lives on the container's disk. - -`supported: false` means the container predates the feature, so every -path is synced regardless of configuration. Worth logging. - -### Validation - -`MOUNT_IGNORE_PATH` must be absolute, must not be the filesystem root, -and must not be equal to or inside `MOUNT_POINT` — a root inside the -mount would make the passthrough layer resolve into itself. Entries may -not contain `.` or `..` segments; an entry that walks out of the mount -is a configuration mistake, and silently clamping it would hide the -mistake behind a path that looks intentional. - -All of these fail the daemon at startup rather than disabling the -feature quietly. A dropped entry means a full `node_modules` goes into -the Durable Object, which is the failure this exists to prevent. - -Duplicates, and entries nested inside another entry, are dropped as -redundant and reported. - -## Diagnostics - -`/__computerd/info` reports the resolved configuration: - -```jsonc -{ - "ignore": { - "supported": true, - "enabled": true, - "root": "/tmp/workspace", - "paths": ["node_modules", "dist"], - "redundant": ["node_modules/.cache"], - "fastPaths": { - "passthrough": false, - "passthroughReason": "fuse-native binds libfuse 2.9; FOPEN_PASSTHROUGH requires the libfuse 3.17 API", - "writebackCache": false - } - } -} -``` - -`paths` is the **normalised** set — what the mount actually applies, not -what was typed. That is what makes the client-side assertion meaningful. - -`fastPaths.passthrough` is `false` on current builds and this is -expected, not a fault. See [Performance](#performance). - -## Renames across the boundary: `EXDEV` - -The one runtime failure a correctly configured deployment can still hit, -and the one worth understanding before it happens. - -A rename whose source and destination sit on opposite sides of the -boundary returns **`EXDEV`** (`cross-device link`). - -``` -$ mv .tmp-build dist -mv: cannot move '.tmp-build' to 'dist': Invalid cross-device link -``` - -### Why it is not just done anyway - -The two sides are different filesystems. A rename between them cannot be -atomic, and `rename(2)` promises atomicity. computerd could copy the -bytes and unlink the source, and the operation would appear to succeed — -but a crash midway would leave a half-written file where the caller was -promised all-or-nothing. Faking atomicity is worse than refusing it, -because the failure it creates is silent and arrives later. - -`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. - -### The fix - -Ignore the staging path alongside its destination. - -This case is common because it is how build tools work: write into a -temporary sibling, then rename into place atomically. If the destination -is local-only and the staging directory is not, every build hits this. - -```ts -ignore: ["/dist", "/.tmp-build"]; -``` - -Candidates worth checking in your own deployment: `.next` (Next.js writes -through `.next/cache`), `.turbo`, `node_modules/.cache`, and any -`*.tmp` staging directory a bundler creates next to its output. - -computerd logs the guidance on the first crossing rename per mount, -naming both sides and the entry to add: - -``` -computerd: rename /.tmp-build -> /dist crossed the local-only boundary and -returned EXDEV. /dist is container-local (MOUNT_IGNORE), /.tmp-build is -synced to the workspace; a rename between them cannot be atomic, so it is -refused rather than silently copied. Most callers fall back to -copy-then-unlink. To keep the rename atomic, add ".tmp-build" to -MOUNT_IGNORE as well. Further occurrences are not logged. -``` - -It logs once because a build that does this does it in a loop. The -per-mount count is available on the passthrough stats. - -Renames **within** one layer are ordinary atomic renames, in both the -local-only layer and the VFS. - -## Performance - -The win is in what is skipped: an ignored write does not enter the VFS, -the SQLite store, the change-pack encoding, or the pull into the Durable -Object. For a dependency install, that is the difference between -transferring tens of thousands of files and transferring none. - -What is **not** skipped is the FUSE round trip itself. The bytes still -cross from the kernel into the daemon before reaching local disk. - -> [!NOTE] -> This is why `fastPaths.passthrough` reports `false`. The kernel -> supports `FOPEN_PASSTHROUGH` (6.9+), and Cloudflare Containers hosts -> are well above that line — but computerd mounts through -> `fuse-native`, which binds **libfuse 2.9**, and passthrough requires -> the libfuse 3.17 API. The same constraint rules out writeback caching, -> which libfuse 2.9 rejects at mount time. -> -> So expect a local-only path to perform like the existing mount -> (see [19. Performance](./19_performance.md), roughly 2x slower than -> raw disk for `npm install`), not like raw disk. The saving is the -> transfer, not the I/O. This flips on a binding upgrade rather than an -> infrastructure change, which is why the field is reported rather than -> omitted. - -## Relationship to `sync.fetchChanges({ ignore })` - -These are different mechanisms with a confusingly similar name. - -| | `MOUNT_IGNORE` | `fetchChanges({ ignore })` | -| --- | --- | --- | -| Layer | FUSE mount | sync RPC | -| Effect | path never enters the VFS | path is skipped in this transfer | -| Scope | durability boundary | transfer filter | - -If you built a wrapper that injects `ignore` into `fetchChanges` to keep -a dependency tree out of the Durable Object, `MOUNT_IGNORE` replaces it -— **delete the wrapper rather than keeping both**. Keeping both leaves -the path excluded from transfer while still occupying the container's -store, which is the half-fixed state `MOUNT_IGNORE` exists to resolve. - -Note also that `ignore` appears on both `fetchChanges` and the optional -`fetchChangePack` overload. A wrapper covering only the first silently -bypasses the filter on exactly the large transfers it was written for. diff --git a/docs/README.md b/docs/README.md index 6c2a0e3e..06ca0d7a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -250,7 +250,6 @@ above, then dive into the area you're working on. | [17. Isolate JavaScript runtime](./17_isolate_javascript.md) | ECMAScript modules, durable imports, configured libraries, durable `node:fs/promises`, trusted `ws:git` / `ws:artifacts`, and managed lifecycle. | | [18. Runtime migration](./18_runtime_migration.md) | Breaking preview-API mappings from public shell and script-execution surfaces to `workspace.runtime`. | | [19. Performance](./19_performance.md) | Filesystem benchmarks: `fs-bench` numbers, an `npm install` comparison, and how to reproduce them. | -| [20. Local-only paths](./20_local_only_paths.md) | `MOUNT_IGNORE`: keeping `node_modules` and other rebuildable trees on the container's disk, the durability trade-off, and the `EXDEV` rename contract. | ## High-level API diff --git a/packages/computer/src/backends/container/ignore-assertion.ts b/packages/computer/src/backends/container/ignore-assertion.ts index 0d5d68d2..55263b76 100644 --- a/packages/computer/src/backends/container/ignore-assertion.ts +++ b/packages/computer/src/backends/container/ignore-assertion.ts @@ -1,6 +1,6 @@ // Client-side assertion over the container's local-only path set. The // set is owned by the image; a client can only state what it expects -// and refuse to connect on disagreement. See docs/20_local_only_paths.md. +// and refuse to connect on disagreement. See packages/computerd/README.md. // // Fails the connection rather than warning, because the failure it // guards is silent and expensive: an image built without MOUNT_IGNORE diff --git a/packages/computerd/README.md b/packages/computerd/README.md index 5922a52f..2a70addf 100644 --- a/packages/computerd/README.md +++ b/packages/computerd/README.md @@ -123,6 +123,135 @@ byte sizes, inline byte totals, and the process's RSS/heap/external figures. Poll it during a long-running install or test to watch how the store grows. +## Local-only paths (`MOUNT_IGNORE`) + +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/`, +`dist/` and caches: tens of thousands of rebuildable files that never +need to be durable. `MOUNT_IGNORE` names paths that stay on the +container's local disk instead. They are never recorded, pushed, or +pulled. + +Content under a local-only path is visible only inside the container; +`workspace.fs` and the worker shell do not see it. It is absent from +sync, so a container replaced without a snapshot restore loses it. It +does survive a container snapshot, because `MOUNT_IGNORE_PATH` is a real +filesystem path, which is why the default sits under `/tmp` rather than +on a tmpfs. That suits a dependency tree a package manager can rebuild, +not anything a user typed. + +### Configuration + +`ContainerBackend` takes an `ignore` option and passes it to the +container's start environment, so changing the set is a deployment +change rather than an image rebuild. `LegacyContainerBackend` has no +such option; set `MOUNT_IGNORE` through its `containerEnv` instead. + +```ts +new ContainerBackend({ + container: env.CONTAINER, + workspace: { binding: "SESSIONS", id: sessionId }, + ignore: ["/node_modules", "/.venv", "/dist"], +}); +``` + +That becomes `MOUNT_IGNORE=/node_modules,/.venv,/dist`. Setting the +variable directly, in `containerEnv` or a Dockerfile, works too and +takes precedence. + +`MOUNT_IGNORE` is a comma-separated list of paths anchored at the mount +root: `/node_modules` means `$MOUNT_POINT/node_modules`. There is no glob +syntax and no negation. A path is local-only if it equals an entry or +sits beneath it, so `/app/node_modules` matches only that path, and a +monorepo lists each `/node_modules` separately. A path containing a +comma cannot be expressed. `MOUNT_IGNORE_PATH` sets where local-only +content is stored and defaults to `/tmp` + `$MOUNT_POINT`. + +The set is compiled once at startup, so it cannot change under a running +container, and two sessions sharing one container see the same +durability boundary. `connect()` reads the resolved set back off +`/__computerd/info` and refuses the connection if it disagrees with what +was declared, which catches a computerd too old to honour the variable. +The handle exposes it as absolute container paths: + +```ts +const handle = await backend.connect(); +handle.ignore; +// { +// paths: ["/workspace/node_modules", "/workspace/.venv", "/workspace/dist"], +// root: "/tmp/workspace", +// mountPoint: "/workspace", +// supported: true, +// } +``` + +`supported: false` means the container predates the feature and every +path is synced. + +### Validation + +`MOUNT_IGNORE_PATH` must be absolute, must not be `/`, and must not be +equal to or inside `MOUNT_POINT`, since a root inside the mount would +resolve into itself. Entries may not contain `.` or `..` segments. Each +of these fails the daemon at startup rather than quietly disabling the +feature, because a dropped entry means a full `node_modules` goes into +the Durable Object. Duplicates and entries nested inside another entry +are dropped as redundant and reported. + +`/__computerd/info` reports the normalised configuration: + +```jsonc +{ + "ignore": { + "supported": true, + "enabled": true, + "root": "/tmp/workspace", + "paths": ["node_modules", "dist"], + "redundant": ["node_modules/.cache"], + "fastPaths": { + "passthrough": false, + "passthroughReason": "fuse-native binds libfuse 2.9; FOPEN_PASSTHROUGH requires the libfuse 3.17 API", + "writebackCache": false + } + } +} +``` + +`fastPaths.passthrough` is `false` on current builds by design. Ignored +writes skip the VFS and the transfer but still cross FUSE; see +[19. Performance](../../docs/19_performance.md#local-only-paths-mount_ignore). + +### Renames across the boundary + +A rename whose source and destination sit on opposite sides of the +boundary returns `EXDEV` (`Invalid cross-device link`). The two sides are +different filesystems, so the rename cannot be atomic, and copying then +unlinking would fake the atomicity `rename(2)` promises. `mv` and +Python's `shutil.move` already fall back to copy-then-unlink on `EXDEV`. +Renames within one side are ordinary atomic renames. + +The usual cause is a build tool that stages into a sibling directory and +renames into place. The fix is to ignore the staging path too: + +```ts +ignore: ["/dist", "/.tmp-build"]; +``` + +Candidates worth checking are `.next`, `.turbo`, `node_modules/.cache`, +and any staging directory a bundler creates next to its output. +computerd logs this guidance on the first crossing rename per mount, +naming both sides and the entry to add. Later occurrences are counted on +the passthrough stats but not logged. + +### `MOUNT_IGNORE` versus `fetchChanges({ ignore })` + +`MOUNT_IGNORE` works at the mount: the path never enters the VFS. +`fetchChanges({ ignore })` works at the sync RPC: the path is skipped in +one transfer but still occupies the container's store. A wrapper that +injects `ignore` into `fetchChanges` to keep a dependency tree out of the +Durable Object should be deleted in favour of `MOUNT_IGNORE`. + ## FUSE prerequisites Linux hosts/containers need access to `/dev/fuse` and mount permissions. diff --git a/packages/computerd/src/fuse/ignore.ts b/packages/computerd/src/fuse/ignore.ts index 2a5ec5d1..4371abf1 100644 --- a/packages/computerd/src/fuse/ignore.ts +++ b/packages/computerd/src/fuse/ignore.ts @@ -1,4 +1,4 @@ -// Local-only subpaths of the mount. See docs/20_local_only_paths.md. +// Local-only subpaths of the mount. See packages/computerd/README.md. // // Entries are plain paths relative to the mount root: no glob syntax // and no negation. Deliberate, because an entry then resolves to a diff --git a/packages/computerd/src/fuse/passthrough.ts b/packages/computerd/src/fuse/passthrough.ts index 1e2df11d..ce3ecd67 100644 --- a/packages/computerd/src/fuse/passthrough.ts +++ b/packages/computerd/src/fuse/passthrough.ts @@ -1,5 +1,5 @@ // Local-only passthrough for the FUSE op layer. See -// docs/20_local_only_paths.md. +// packages/computerd/README.md. // // A decorator over FuseOps rather than branches inside makeFUSEOps, so // the VFS driver stays unaware of the feature and an empty ignore set From 6121b3ea2ca6af7679a013f123c19e04d1842b19 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:13:33 +0100 Subject: [PATCH 09/15] computer: stop the ignore check rejecting correct containers ContainerBackend read /__computerd/info without the client secret. computerd protects every route except /health, so an enforcing container answered 401, the backend read that as "computerd does not support local-only paths", and every connect that declared `ignore` failed. The request now carries the bearer token. The comparison also disagreed with computerd about spelling. computerd accepts "/workspace/dist" and applies it as "dist", but the client compared the declaration raw and reported a mismatch. Declarations now have the mount point stripped before comparing, the same as the report. The mismatch error still said the set belonged to the image, which stopped being true once `ignore` began setting MOUNT_IGNORE at start. It now points at the likely cause: a MOUNT_IGNORE in `containerEnv` overriding the option. The new tests drive connect() through the upgrade against a fake host that enforces the secret the way computerd does. The earlier fake answered /__computerd/info without checking, which is how this got through. --- .../container-backend-ignore.test.ts | 133 +++++++++++++++++- .../backends/container/container-backend.ts | 17 ++- .../container/ignore-assertion.test.ts | 28 +++- .../backends/container/ignore-assertion.ts | 23 +-- 4 files changed, 184 insertions(+), 17 deletions(-) diff --git a/packages/computer/src/backends/container/container-backend-ignore.test.ts b/packages/computer/src/backends/container/container-backend-ignore.test.ts index bdf0cb75..4bfdd617 100644 --- a/packages/computer/src/backends/container/container-backend-ignore.test.ts +++ b/packages/computer/src/backends/container/container-backend-ignore.test.ts @@ -4,12 +4,12 @@ // a fake host. The comparison logic and the error text have their own // suite in ignore-assertion.test.ts, and the end-to-end behaviour is // covered in computerd's cli tests against a real FUSE mount. -import { describe, expect, test } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; import { ContainerBackend } from "./container-backend.js"; import type { ContainerRuntimeInfo, IWorkspaceContainerAPI } from "./container-host.js"; import type { ContainerLaunchSpec } from "./container-launch-record.js"; -import { readIgnoreReport } from "./ignore-assertion.js"; +import { ContainerIgnoreMismatchError, readIgnoreReport } from "./ignore-assertion.js"; interface FakeHostOptions { // The `ignore` block /__computerd/info reports. Omitted models a @@ -148,3 +148,132 @@ describe("ContainerBackend local-only paths", () => { expect(starts[0]?.env?.MOUNT_IGNORE).toBeUndefined(); }); }); + +// Drives connect() through the upgrade, so the ignore check runs against a +// container that enforces its client secret the way computerd does: every +// route except /health needs the bearer token. +describe("ContainerBackend ignore check on a full connect", () => { + const SECRET = "00112233445566778899aabbccddeeff"; + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + // Enough of a WebSocket for capnweb to attach to and for the backend to + // close. Nothing is sent over it in these tests. + class FakeSocket { + readyState = 1; + accept() {} + addEventListener() {} + removeEventListener() {} + send() {} + close() { + this.readyState = 3; + } + } + + function connectingHost(ignore: Record) { + let backend: ContainerBackend | undefined; + const infoAuth: (string | null)[] = []; + const host: IWorkspaceContainerAPI = { + async start() { + return { runtimeId: "runtime-1", clientSecret: SECRET, outcome: "launched" }; + }, + async restart() { + throw new Error("not used"); + }, + async interceptOutboundHttp() {}, + async interceptAllOutboundHttp() {}, + async fetchPort(_port, url, init) { + const path = new URL(url).pathname; + const auth = new Headers(init?.headers).get("authorization"); + if (path === "/health") return new Response("ok"); + if (path === "/__computerd/info") infoAuth.push(auth); + if (auth !== `Bearer ${SECRET}`) return new Response(null, { status: 401 }); + if (path === "/connect") { + // computerd dials back as soon as it is told where to go. + await backend + ?.handleFetch( + new Request("http://computer.internal/api", { + headers: { upgrade: "websocket", authorization: `Bearer ${SECRET}` }, + }), + ) + // The 101 Response is a workerd-only shape; the upgrade has + // already been handed over by the time it is built. + .catch(() => undefined); + return new Response(null, { status: 200 }); + } + if (path === "/__computerd/info") { + return Response.json({ backend: { kind: "fuse" }, mountPoint: "/workspace", ignore }); + } + return new Response(null, { status: 404 }); + }, + port() { + throw new Error("not used"); + }, + async setInactivityTimeout() {}, + async status() { + return { running: true, exit: null }; + }, + async exitInfo() { + return null; + }, + }; + return { + host, + infoAuth, + attach(b: ContainerBackend) { + backend = b; + }, + }; + } + + function backendFor(fake: ReturnType, ignore: readonly string[]) { + vi.stubGlobal( + "WebSocketPair", + class { + 0 = new FakeSocket(); + 1 = new FakeSocket(); + }, + ); + const backend = new ContainerBackend({ + container: () => ({ getWorkspaceContainer: () => fake.host }), + workspace: { binding: "SESSIONS", id: "session-1" }, + restartAttempts: 0, + connectTimeoutMs: 2_000, + heartbeatIntervalMs: 0, + ignore, + }); + fake.attach(backend); + return backend; + } + + test("reads /__computerd/info with the client secret", async () => { + const fake = connectingHost({ supported: true, root: "/tmp/workspace", paths: ["dist"] }); + const handle = await backendFor(fake, ["/dist"]).connect(); + + expect(fake.infoAuth).toEqual([`Bearer ${SECRET}`]); + expect(handle.ignore).toEqual({ + paths: ["/workspace/dist"], + root: "/tmp/workspace", + mountPoint: "/workspace", + supported: true, + }); + await handle.close(); + }); + + test("accepts a declaration spelled with the mount point", async () => { + // computerd strips the mount prefix from "/workspace/dist" and + // applies "dist". The declaration means the same thing. + const fake = connectingHost({ supported: true, root: "/tmp/workspace", paths: ["dist"] }); + const handle = await backendFor(fake, ["/workspace/dist"]).connect(); + await handle.close(); + }); + + test("still rejects a real mismatch", async () => { + const fake = connectingHost({ supported: true, root: "/tmp/workspace", paths: ["dist"] }); + await expect(backendFor(fake, ["/node_modules"]).connect()).rejects.toBeInstanceOf( + ContainerIgnoreMismatchError, + ); + }); +}); diff --git a/packages/computer/src/backends/container/container-backend.ts b/packages/computer/src/backends/container/container-backend.ts index b13275e3..caf14925 100644 --- a/packages/computer/src/backends/container/container-backend.ts +++ b/packages/computer/src/backends/container/container-backend.ts @@ -405,7 +405,7 @@ export class ContainerBackend implements WorkspaceBackend { // would let the first exec write into a path the caller believes is // local-only, which is precisely the state that is expensive to // discover later. - const resolvedIgnore = await this.#resolveIgnore(host); + const resolvedIgnore = await this.#resolveIgnore(host, clientSecret); try { assertIgnoreMatches(this.#options.ignore, resolvedIgnore); } catch (error) { @@ -636,12 +636,23 @@ export class ContainerBackend implements WorkspaceBackend { // because a diagnostic request failed. A client that *did* declare // one still fails, via assertIgnoreMatches -- which is the right // split: silence is only acceptable when nobody asked. - async #resolveIgnore(host: IWorkspaceContainerAPI): Promise { + // + // The endpoint sits behind the client secret like every route except + // /health. Without the token an enforcing container answers 401, which + // would read as "unsupported" and fail every connect that declared + // `ignore`. + async #resolveIgnore( + host: IWorkspaceContainerAPI, + clientSecret: string, + ): Promise { try { const res = await host.fetchPort( this.#options.containerPort, "http://container/__computerd/info", - { signal: AbortSignal.timeout(this.#options.healthProbeTimeoutMs) }, + { + headers: { authorization: `Bearer ${clientSecret}` }, + signal: AbortSignal.timeout(this.#options.healthProbeTimeoutMs), + }, ); if (!res.ok) return { paths: [], root: undefined, mountPoint: undefined, supported: false }; return readIgnoreReport(await res.json()); diff --git a/packages/computer/src/backends/container/ignore-assertion.test.ts b/packages/computer/src/backends/container/ignore-assertion.test.ts index 9e07f843..765eaa88 100644 --- a/packages/computer/src/backends/container/ignore-assertion.test.ts +++ b/packages/computer/src/backends/container/ignore-assertion.test.ts @@ -211,17 +211,37 @@ describe("assertIgnoreMatches", () => { } }); - test("explains that the image owns the set", () => { - // Without this the natural reaction is to change the client option, - // which cannot fix anything. + test("points at the setting that overrides `ignore`", () => { + // `ignore` is passed to the container as MOUNT_IGNORE, so a + // disagreement means something else set the variable after it. try { assertIgnoreMatches(["a"], supported(["b"])); expect.unreachable("should have thrown"); } catch (error) { - expect((error as Error).message).toMatch(/property of the image \(MOUNT_IGNORE\)/); + expect((error as Error).message).toMatch(/MOUNT_IGNORE in `containerEnv`/); } }); + test("accepts declarations spelled with the mount point", () => { + // computerd strips the mount prefix, so "/workspace/dist" and "/dist" + // configure the same path. Comparing them raw rejects a container + // that is doing exactly what was asked. + expect(() => + assertIgnoreMatches( + ["/workspace/dist", "/workspace/node_modules/"], + supported(["/workspace/dist", "/workspace/node_modules"]), + ), + ).not.toThrow(); + }); + + test("does not strip a prefix that only looks like the mount point", () => { + // "/workspacefoo" is not under "/workspace", so it names + // "/workspace/workspacefoo", not "/workspace/foo". + expect(() => assertIgnoreMatches(["/workspacefoo"], supported(["/workspace/foo"]))).toThrow( + ContainerIgnoreMismatchError, + ); + }); + test("carries the declared and actual sets on the error", () => { // So a host can log or reconcile them without parsing the message. try { diff --git a/packages/computer/src/backends/container/ignore-assertion.ts b/packages/computer/src/backends/container/ignore-assertion.ts index 55263b76..1762e46c 100644 --- a/packages/computer/src/backends/container/ignore-assertion.ts +++ b/packages/computer/src/backends/container/ignore-assertion.ts @@ -136,10 +136,13 @@ export function assertIgnoreMatches( ); } - // resolved.paths are absolute container paths; the declaration is written - // mount-relative ("/node_modules"), so compare on the mount-relative form. + // resolved.paths are absolute container paths. A declaration may be + // written mount-relative ("/node_modules") or with the mount point + // ("/workspace/node_modules"), and computerd accepts both. Compare + // both sides on the mount-relative form. + const declaredRelative = declared.map((path) => stripMount(path, resolved.mountPoint)); const actualRelative = resolved.paths.map((path) => stripMount(path, resolved.mountPoint)); - const difference = diffIgnore(declared, actualRelative); + const difference = diffIgnore(declaredRelative, actualRelative); if (difference === null) return; const parts: string[] = []; @@ -158,9 +161,10 @@ export function assertIgnoreMatches( throw new ContainerIgnoreMismatchError( `Container ignore set does not match \`ignore\`: ${parts.join("; ")}. ` + - `The set is a property of the image (MOUNT_IGNORE), not of this ` + - `client; \`ignore\` only asserts what the image is expected to apply. ` + - `Rebuild the image or update the declaration so the two agree.`, + `\`ignore\` is passed to the container as MOUNT_IGNORE, so a ` + + `MOUNT_IGNORE in \`containerEnv\` overrides it. Remove one of them, ` + + `or check that the computerd image reads MOUNT_IGNORE as a ` + + `comma-separated list.`, { declared: [...declared], actual: [...resolved.paths], supported: true }, ); } @@ -172,8 +176,11 @@ export function assertIgnoreMatches( function stripMount(path: string, mountPoint: string | undefined): string { if (mountPoint === undefined) return path; const base = mountPoint.replace(/\/+$/, ""); - if (base !== "" && path.startsWith(`${base}/`)) return path.slice(base.length + 1); - return path; + const trimmed = path.trim(); + if (base !== "" && (trimmed === base || trimmed.startsWith(`${base}/`))) { + return trimmed.slice(base.length + 1); + } + return trimmed; } function normalise(entry: string): string { From 517c8ede3c50710a8d347b0cfe26bb089d6af556 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:16:07 +0100 Subject: [PATCH 10/15] computerd: refuse MOUNT_IGNORE on the userspace shim The shim copies everything under the mount into the VFS, so it cannot keep a path local. computerd still resolved MOUNT_IGNORE and reported the paths as local-only on /__computerd/info, so the host's check passed while every ignored write was synced. FUSE_MOUNT=auto falls back to the shim when /dev/fuse is missing, which made this reachable from a misconfigured container with no explicit opt-in. computerd now fails at startup when MOUNT_IGNORE is set and the backend resolves to the shim, the same as it does for every other ignore misconfiguration. The real-FUSE test still set MOUNT_IGNORE in the old newline-separated form, which the comma-separated parser reads as one entry. It only runs on Linux with /dev/fuse, so it had not run since the format changed. It now uses the current form. --- packages/computerd/src/cli/computerd.test.ts | 26 +++++++++++++++++++- packages/computerd/src/cli/computerd.ts | 11 +++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/computerd/src/cli/computerd.test.ts b/packages/computerd/src/cli/computerd.test.ts index 5630153b..0dd538f7 100644 --- a/packages/computerd/src/cli/computerd.test.ts +++ b/packages/computerd/src/cli/computerd.test.ts @@ -137,7 +137,7 @@ test("MOUNT_IGNORE keeps matching paths on local disk and out of the VFS", async mountPoint, env: { FUSE_MOUNT: "fuse", - MOUNT_IGNORE: "node_modules\ndist", + MOUNT_IGNORE: "/node_modules,/dist", MOUNT_IGNORE_PATH: ignoreRoot, }, }); @@ -441,6 +441,30 @@ test("computerd rejects unknown FUSE_MOUNT values", async () => { expect(stderr).toMatch(/FUSE_MOUNT must be one of/); }); +test("computerd refuses MOUNT_IGNORE on the userspace shim", async () => { + // The shim copies everything under the mount into the VFS, so it + // cannot keep a path local. Starting anyway would report the paths as + // local-only while syncing them, which is the failure MOUNT_IGNORE + // exists to prevent. + const port = await getAvailablePort(); + const mountPoint = await fs.mkdtemp(path.join(os.tmpdir(), "computerd-mount-")); + const child = spawn(cliPath, { + cwd: packageRoot, + env: { + ...process.env, + MOUNT_POINT: mountPoint, + PORT: String(port), + FUSE_MOUNT: "shim", + MOUNT_IGNORE: "/node_modules", + }, + stdio: ["ignore", "ignore", "pipe"], + }); + + const { code, stderr } = await waitForExit(child); + expect(code).toBe(1); + expect(stderr).toMatch(/MOUNT_IGNORE is not supported on the userspace shim/); +}); + test.each([ ["DISABLE_FUSE", "1"], ["FUSE_SHIM", "1"], diff --git a/packages/computerd/src/cli/computerd.ts b/packages/computerd/src/cli/computerd.ts index b681e85c..f4bb529d 100644 --- a/packages/computerd/src/cli/computerd.ts +++ b/packages/computerd/src/cli/computerd.ts @@ -641,6 +641,17 @@ async function main(): Promise { // node_modules into the Durable Object, which is the failure this // feature exists to prevent. const ignoreConfig = resolveMountIgnoreConfig(process.env, mountPoint); + // The shim copies everything under the mount into the VFS, so it has + // no way to keep a path local. Starting anyway would report the paths + // as local-only on /__computerd/info while syncing them, and the + // host's check would pass. FUSE_MOUNT=auto lands here too when + // /dev/fuse is missing, which is exactly when this needs to be loud. + if (ignoreConfig.enabled && backend.kind === "shim") { + throw new Error( + `MOUNT_IGNORE is not supported on the userspace shim (FUSE_MOUNT=${fuseMountMode} ` + + `resolved to backend=shim). Run with real FUSE, or unset MOUNT_IGNORE.`, + ); + } if (ignoreConfig.enabled) { console.log( `[info] MOUNT_IGNORE active: ${ignoreConfig.ignore.paths.length} path(s) ` + From d54e066d61008ba6496ee0e6a5af485697c03f5e Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:17:46 +0100 Subject: [PATCH 11/15] computerd: fix descriptor and link handling for local-only paths Several operations in the local-only layer either ignored the open file or acted on the wrong one. ftruncate truncated by path instead of by descriptor. After a rename or replacement the path names a different file, so truncating an open handle could destroy another file's contents. It now uses the descriptor. fsync returned success without flushing anything, so a program using fsync as a commit point could lose data it had been told was on disk. It now calls fsync or fdatasync on the descriptor. link was not routed at all. A hardlink between two local-only paths went to the VFS, which has never seen either file, and failed with ENOENT. Both-local links are now made on disk, and a link across the boundary returns EXDEV, which is what link(2) returns for two filesystems anywhere else. chown and utimens followed symlinks. The kernel resolves links before calling the daemon unless the caller asked for the link itself, as `chown -h` and `touch -h` do, so a path that reaches these calls as a symlink means the link. Following it acted on the target, which can be outside the local root. They now use lchown and lutimes. opendir accepted any path and left the error for readdir, and access reported success for any existing path regardless of the mode asked for. opendir now checks the path is a directory, and access checks the requested mode. --- .../computerd/src/fuse/passthrough.test.ts | 172 +++++++++++++++++- packages/computerd/src/fuse/passthrough.ts | 97 +++++++++- 2 files changed, 258 insertions(+), 11 deletions(-) diff --git a/packages/computerd/src/fuse/passthrough.test.ts b/packages/computerd/src/fuse/passthrough.test.ts index b17395b3..c2dac161 100644 --- a/packages/computerd/src/fuse/passthrough.test.ts +++ b/packages/computerd/src/fuse/passthrough.test.ts @@ -1,4 +1,15 @@ -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import * as nodeFs from "node:fs"; +import { + constants, + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -6,7 +17,11 @@ import { afterEach, beforeEach, describe, expect, test } from "vitest"; import type { FuseOps } from "./driver.js"; import { resolveMountIgnore } from "./ignore.js"; -import { withLocalPassthrough } from "./passthrough.js"; +import { type PassthroughFs, withLocalPassthrough } from "./passthrough.js"; + +// The real filesystem, as the slice withLocalPassthrough takes. Tests +// override single calls on top of it. +const realFs = (): PassthroughFs => ({ ...nodeFs }) as PassthroughFs; // Drives the real node:fs against a temp directory rather than a double. // The interesting failures here -- EXDEV, ENOTEMPTY, parent creation -- @@ -536,3 +551,156 @@ describe("withLocalPassthrough: errors", () => { expect(code).toBe(-20); }); }); + +describe("withLocalPassthrough: descriptor and metadata operations", () => { + let root: string; + let outside: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "computerd-passthrough-")); + outside = mkdtempSync(join(tmpdir(), "computerd-outside-")); + mkdirSync(join(root, "node_modules"), { recursive: true }); + }); + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + }); + + const build = (fs?: Partial) => { + const source = recordingOps(); + const { ops } = withLocalPassthrough(source.ops, { + root, + ignore: resolveMountIgnore(["node_modules"], MOUNT), + mountPoint: MOUNT, + ...(fs === undefined ? {} : { fs: { ...realFs(), ...fs } }), + }); + return { ops, calls: source.calls }; + }; + + const open = (ops: FuseOps, path: string): number => { + let fh = 0; + ops.open(path, constants.O_RDWR, (code, handle) => { + expect(code).toBe(0); + fh = handle as number; + }); + return fh; + }; + + const status = (run: (cb: (code: number) => void) => void): number => { + let result = 1; + run((code) => { + result = code; + }); + return result; + }; + + test("ftruncate truncates the open file, not whatever now has its name", () => { + // Open a, rename it to b, create a new a, then truncate the old + // handle. The handle still refers to the file now called b. + const { ops } = build(); + writeFileSync(join(root, "node_modules/a"), "original"); + const fh = open(ops, "/node_modules/a"); + ops.rename("/node_modules/a", "/node_modules/b", (code) => expect(code).toBe(0)); + writeFileSync(join(root, "node_modules/a"), "replacement"); + + expect(status((cb) => ops.ftruncate("/node_modules/a", fh, 2, cb))).toBe(0); + + expect(readFileSync(join(root, "node_modules/b"), "utf8")).toBe("or"); + expect(readFileSync(join(root, "node_modules/a"), "utf8")).toBe("replacement"); + }); + + test("fsync flushes the descriptor", () => { + // A program that fsyncs a file is relying on it reaching disk. + const synced: string[] = []; + const { ops } = build({ + fsyncSync: () => { + synced.push("fsync"); + }, + fdatasyncSync: () => { + synced.push("fdatasync"); + }, + }); + writeFileSync(join(root, "node_modules/a"), "x"); + const fh = open(ops, "/node_modules/a"); + + expect(status((cb) => ops.fsync("/node_modules/a", fh, 0, cb))).toBe(0); + expect(status((cb) => ops.fsync("/node_modules/a", fh, 1, cb))).toBe(0); + expect(synced).toEqual(["fsync", "fdatasync"]); + }); + + test("hardlinks within the local layer", () => { + const { ops, calls } = build(); + writeFileSync(join(root, "node_modules/a"), "shared"); + + expect(status((cb) => ops.link("/node_modules/a", "/node_modules/b", cb))).toBe(0); + + expect(statSync(join(root, "node_modules/b")).nlink).toBe(2); + expect(calls).toEqual([]); + }); + + test("refuses a hardlink across the boundary with EXDEV", () => { + const { ops, calls } = build(); + writeFileSync(join(root, "node_modules/a"), "x"); + + expect(status((cb) => ops.link("/node_modules/a", "/src/a", cb))).toBe(-18); + expect(status((cb) => ops.link("/src/a", "/node_modules/b", cb))).toBe(-18); + expect(calls).toEqual([]); + }); + + test("delegates a hardlink entirely within the VFS", () => { + const { ops, calls } = build(); + ops.link("/src/a", "/src/b", () => {}); + expect(calls).toEqual(["link"]); + }); + + test("opendir reports a missing path or a file up front", () => { + const { ops } = build(); + writeFileSync(join(root, "node_modules/file.js"), "x"); + + expect(status((cb) => ops.opendir("/node_modules/missing", 0, cb))).toBe(-2); + expect(status((cb) => ops.opendir("/node_modules/file.js", 0, cb))).toBe(-20); + expect(status((cb) => ops.opendir("/node_modules", 0, cb))).toBe(0); + }); + + test("access checks the requested mode", () => { + const { ops } = build(); + writeFileSync(join(root, "node_modules/data.json"), "{}", { mode: 0o644 }); + + expect(status((cb) => ops.access("/node_modules/data.json", constants.R_OK, cb))).toBe(0); + // No execute bit for anyone, so this fails even for root. + expect(status((cb) => ops.access("/node_modules/data.json", constants.X_OK, cb))).toBe(-13); + }); + + test("utimens on a symlink changes the link, not its target", () => { + // The kernel resolves links before calling the daemon unless the + // caller asked for the link itself (touch -h). Following it here + // would reach a file outside the local root. + const { ops } = build(); + const target = join(outside, "target"); + writeFileSync(target, "x"); + const before = statSync(target).mtimeMs; + symlinkSync(target, join(root, "node_modules/link")); + + expect(status((cb) => ops.utimens("/node_modules/link", 1_000, 1_000, cb))).toBe(0); + + expect(statSync(target).mtimeMs).toBe(before); + expect(lstatSync(join(root, "node_modules/link")).mtimeMs).toBe(1_000); + }); + + test("chown on a symlink changes the link, not its target", () => { + // Changing ownership needs root, so this checks which call is made. + const changed: string[] = []; + const { ops } = build({ + chownSync: () => { + changed.push("chown"); + }, + lchownSync: () => { + changed.push("lchown"); + }, + }); + symlinkSync(join(outside, "target"), join(root, "node_modules/link")); + + expect(status((cb) => ops.chown("/node_modules/link", 0, 0, cb))).toBe(0); + expect(changed).toEqual(["lchown"]); + }); +}); diff --git a/packages/computerd/src/fuse/passthrough.ts b/packages/computerd/src/fuse/passthrough.ts index ce3ecd67..482230de 100644 --- a/packages/computerd/src/fuse/passthrough.ts +++ b/packages/computerd/src/fuse/passthrough.ts @@ -19,12 +19,19 @@ // have that problem, so the indirection would be pure cost here. import { + accessSync, chmodSync, chownSync, closeSync, + fdatasyncSync, constants as fsConstants, fstatSync, + fsyncSync, + ftruncateSync, + lchownSync, + linkSync, lstatSync, + lutimesSync, mkdirSync, openSync, readdirSync, @@ -37,7 +44,6 @@ import { symlinkSync, truncateSync, unlinkSync, - utimesSync, writeSync, } from "node:fs"; import { dirname, join, posix } from "node:path"; @@ -101,10 +107,19 @@ export interface PassthroughFs { rmdirSync: typeof rmdirSync; symlinkSync: typeof symlinkSync; truncateSync: typeof truncateSync; + ftruncateSync: typeof ftruncateSync; + fsyncSync: typeof fsyncSync; + fdatasyncSync: typeof fdatasyncSync; + linkSync: typeof linkSync; unlinkSync: typeof unlinkSync; - utimesSync: typeof utimesSync; + accessSync: typeof accessSync; + // The l-variants: an operation that reaches the daemon on a symlink's + // own path is about the link. Following it would act on whatever the + // link points at, which can be outside the local root. + lutimesSync: typeof lutimesSync; chmodSync: typeof chmodSync; chownSync: typeof chownSync; + lchownSync: typeof lchownSync; } const REAL_FS: PassthroughFs = { @@ -122,10 +137,16 @@ const REAL_FS: PassthroughFs = { rmdirSync, symlinkSync, truncateSync, + ftruncateSync, + fsyncSync, + fdatasyncSync, + linkSync, unlinkSync, - utimesSync, + accessSync, + lutimesSync, chmodSync, chownSync, + lchownSync, }; /** Counters for `/__computerd/info` and for proving the cache works. */ @@ -323,7 +344,18 @@ export function withLocalPassthrough( 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. + // recursive walk of a large dependency tree. The path is still + // checked now, so a missing directory fails at opendir(3) the way + // it would on any other filesystem. + try { + if (!fs.statSync(localPath(path)).isDirectory()) { + cb(ERRNO.ENOTDIR, 0); + return; + } + } catch (error) { + cb(toErrno(error), 0); + return; + } cb(0, allocateHandle(-1, path)); }, @@ -427,7 +459,19 @@ export function withLocalPassthrough( ops.fsync(path, fh, datasync, cb); return; } - cb(0); + const handle = handles.get(fh); + if (handle === undefined || handle.fd < 0) { + cb(ERRNO.EBADF); + return; + } + localOps += 1; + try { + if (datasync !== 0) fs.fdatasyncSync(handle.fd); + else fs.fsyncSync(handle.fd); + cb(0); + } catch (error) { + cb(toErrno(error)); + } }, truncate(path, size, cb) { @@ -449,9 +493,16 @@ export function withLocalPassthrough( ops.ftruncate(path, fh, size, cb); return; } + // By descriptor, not by path: the file may have been renamed or + // replaced since it was opened. + const handle = handles.get(fh); + if (handle === undefined || handle.fd < 0) { + cb(ERRNO.EBADF); + return; + } localOps += 1; try { - fs.truncateSync(localPath(path), size); + fs.ftruncateSync(handle.fd, size); cb(0); } catch (error) { cb(toErrno(error)); @@ -565,7 +616,7 @@ export function withLocalPassthrough( } localOps += 1; try { - fs.chownSync(localPath(path), uid, gid); + fs.lchownSync(localPath(path), uid, gid); cb(0); } catch (error) { cb(toErrno(error)); @@ -579,7 +630,7 @@ export function withLocalPassthrough( } localOps += 1; try { - fs.utimesSync(localPath(path), atime / 1000, mtime / 1000); + fs.lutimesSync(localPath(path), atime / 1000, mtime / 1000); cb(0); } catch (error) { cb(toErrno(error)); @@ -625,7 +676,35 @@ export function withLocalPassthrough( } localOps += 1; try { - fs.lstatSync(localPath(path)); + fs.accessSync(localPath(path), mode); + cb(0); + } catch (error) { + cb(toErrno(error)); + } + }, + + link(source, destination, cb) { + const sourceLocal = isLocal(source); + const destinationLocal = isLocal(destination); + + if (!sourceLocal && !destinationLocal) { + ops.link(source, destination, cb); + return; + } + + // A hardlink is one file under two names, so both names have to be + // on the same filesystem. Across the boundary that is impossible, + // and EXDEV is what link(2) returns for it anywhere else. + if (sourceLocal !== destinationLocal) { + cb(ERRNO.EXDEV); + return; + } + + localOps += 1; + try { + const target = localPath(destination); + ensureParent(target); + fs.linkSync(localPath(source), target); cb(0); } catch (error) { cb(toErrno(error)); From 848a2956ccbb6e65681ef2ac13665d45d35537da Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:18:45 +0100 Subject: [PATCH 12/15] computerd: drop the local-only decision cache The cache meant to answer "is this path local-only?" from its parent directory grew with the dependency tree instead. A directory reached by inheritance was never cached itself, so only one level below an entry was ever inherited. Below that, every file was matched again and its path stored, and nothing removed entries when files were deleted. It also cost a syscall on every miss. To decide whether to cache a path, it called lstat on the matching location under the local root, which included every path in the synced tree. Every getattr in the VFS paid for a local disk lookup to answer a question that never involved local disk. The ignore set is a handful of entries and the test is a prefix comparison against each, which costs about what the cache lookup did. So the cache goes, along with the invalidation it needed on rename and rmdir, and the decision and cache-hit counters that only existed to show it working. --- .../computerd/src/fuse/passthrough.test.ts | 84 +++++++------------ packages/computerd/src/fuse/passthrough.ts | 74 ++-------------- 2 files changed, 36 insertions(+), 122 deletions(-) diff --git a/packages/computerd/src/fuse/passthrough.test.ts b/packages/computerd/src/fuse/passthrough.test.ts index c2dac161..c8154c78 100644 --- a/packages/computerd/src/fuse/passthrough.test.ts +++ b/packages/computerd/src/fuse/passthrough.test.ts @@ -165,7 +165,7 @@ describe("withLocalPassthrough: routing", () => { }); }); -describe("withLocalPassthrough: the decision cache", () => { +describe("withLocalPassthrough: deciding paths", () => { let root: string; beforeEach(() => { @@ -175,79 +175,55 @@ describe("withLocalPassthrough: the decision cache", () => { rmSync(root, { recursive: true, force: true }); }); - test("inherits the decision rather than re-consulting the ignore set", () => { - // The performance argument for the whole feature. A deep tree must - // cost one decision at the top, not one per entry. + test("routes a path many levels under an entry", () => { const source = recordingOps(); - const { ops, stats } = withLocalPassthrough(source.ops, { + const { ops } = withLocalPassthrough(source.ops, { root, ignore: resolveMountIgnore(["node_modules"], MOUNT), mountPoint: MOUNT, }); - - mkdirSync(join(root, "node_modules"), { recursive: true }); - ops.getattr("/node_modules", () => {}); - const afterRoot = stats().decisions; - - for (const path of [ - "/node_modules/a.js", - "/node_modules/b.js", - "/node_modules/c.js", - "/node_modules/d.js", - ]) { - ops.getattr(path, () => {}); - } - - // Every child was answered from the parent's cached decision. - expect(stats().decisions).toBe(afterRoot); - expect(stats().cacheHits).toBe(4); + ops.create("/node_modules/a/b/c/d/e/f.js", 0o644, (code) => expect(code).toBe(0)); + expect(readFileSync(join(root, "node_modules/a/b/c/d/e/f.js"), "utf8")).toBe(""); + expect(source.calls).toEqual([]); }); - test("a file six levels deep costs one decision per new directory, not per file", () => { + test("a recreated directory is decided by its path, not by history", () => { + // Removing and recreating a directory, or renaming one into place, + // must not leave a path in the layer it used to belong to. const source = recordingOps(); - const { ops, stats } = withLocalPassthrough(source.ops, { + const { ops } = withLocalPassthrough(source.ops, { root, ignore: resolveMountIgnore(["node_modules"], MOUNT), mountPoint: MOUNT, }); + ops.mkdir("/node_modules", 0o755, () => {}); + ops.mkdir("/node_modules/pkg", 0o755, () => {}); + ops.rename("/node_modules/pkg", "/node_modules/moved", (code) => expect(code).toBe(0)); + ops.rmdir("/node_modules/moved", (code) => expect(code).toBe(0)); - // Materialise the chain the way a real install would. - for (const dir of ["", "/a", "/a/b", "/a/b/c", "/a/b/c/d", "/a/b/c/d/e"]) { - ops.mkdir(`/node_modules${dir}`, 0o755, () => {}); - } - const afterTree = stats().decisions; - const hitsAfterTree = stats().cacheHits; - - // Ten files in the deepest directory: all inherited. - for (let index = 0; index < 10; index += 1) { - ops.getattr(`/node_modules/a/b/c/d/e/file-${index}.js`, () => {}); - } - - expect(stats().decisions).toBe(afterTree); - expect(stats().cacheHits - hitsAfterTree).toBe(10); + ops.getattr("/src/pkg/x.js", () => {}); + expect(source.calls).toEqual(["getattr"]); }); - test("forgets a directory decision when the directory is removed", () => { - // A stale cached decision would survive a delete and recreate, - // which is how a path silently ends up in the wrong layer. + test("does not touch local disk to decide a synced path", () => { + // Every VFS lookup goes through the decision, so a syscall here is + // paid on every getattr in the synced tree. const source = recordingOps(); - const { ops, stats } = withLocalPassthrough(source.ops, { + let localCalls = 0; + const counting = new Proxy(realFs(), { + get(target, property: keyof PassthroughFs) { + localCalls += 1; + return target[property]; + }, + }); + const { ops } = withLocalPassthrough(source.ops, { root, ignore: resolveMountIgnore(["node_modules"], MOUNT), mountPoint: MOUNT, + fs: counting, }); - - ops.mkdir("/node_modules", 0o755, () => {}); - ops.mkdir("/node_modules/pkg", 0o755, () => {}); - const hitsBefore = stats().cacheHits; - ops.getattr("/node_modules/pkg/x.js", () => {}); - expect(stats().cacheHits - hitsBefore).toBe(1); - - ops.rmdir("/node_modules/pkg", () => {}); - const before = stats().decisions; - ops.getattr("/node_modules/pkg/x.js", () => {}); - // Re-decided rather than inherited from the removed entry. - expect(stats().decisions).toBe(before + 1); + for (let index = 0; index < 10; index += 1) ops.getattr(`/src/file-${index}.ts`, () => {}); + expect(localCalls).toBe(0); }); }); diff --git a/packages/computerd/src/fuse/passthrough.ts b/packages/computerd/src/fuse/passthrough.ts index 482230de..47a3fc24 100644 --- a/packages/computerd/src/fuse/passthrough.ts +++ b/packages/computerd/src/fuse/passthrough.ts @@ -149,14 +149,10 @@ const REAL_FS: PassthroughFs = { lchownSync, }; -/** Counters for `/__computerd/info` and for proving the cache works. */ +/** Counters reported on `/__computerd/stats`. */ export interface PassthroughStats { /** Paths served from local disk rather than the VFS. */ readonly localOps: number; - /** Calls that consulted the ignore set rather than a cached decision. */ - readonly decisions: number; - /** Decisions answered from the per-directory cache. */ - readonly cacheHits: number; /** Open local file handles. */ readonly openHandles: number; /** Renames refused with EXDEV for crossing the boundary. */ @@ -184,8 +180,6 @@ export function withLocalPassthrough( ops, stats: () => ({ localOps: 0, - decisions: 0, - cacheHits: 0, openHandles: 0, crossLayerRenames: 0, }), @@ -197,43 +191,17 @@ export function withLocalPassthrough( const mountRoot = normaliseMount(options.mountPoint ?? "/"); let localOps = 0; - let decisions = 0; - let cacheHits = 0; let crossLayerRenames = 0; const warn = options.warn ?? ((message: string) => console.warn(message)); - // The decision cache. Keyed by *directory*, not by file: ignored-ness - // is inherited, so once a directory is known local-only every path - // beneath it is too, with no further consultation of the ignore set. - // - // This is the whole performance argument. A `node_modules` tree is - // tens of thousands of entries under a handful of directories; without - // inheritance each one would re-test the entry list on every lookup. - const directoryDecisions = new Map(); - + // No cache. The ignore set is a handful of entries and the test is a + // prefix comparison against each, which costs about what a cache + // lookup would. A per-path cache grows with the dependency tree and + // has to be invalidated on every rename and rmdir to stay correct. const isLocal = (path: string): boolean => { const relative = toRelative(path, mountRoot); if (relative === "") return false; - - const parent = posix.dirname(relative); - if (parent !== "." && parent !== "/") { - const inherited = directoryDecisions.get(parent); - if (inherited === true) { - // Inherited, not matched. No ignore-set consultation at all. - cacheHits += 1; - return true; - } - } - - decisions += 1; - const decision = options.ignore.ignores(relative); - // Only directory decisions are cached. Caching files would grow - // without bound across a build, and buys nothing: a file is a leaf, - // so nothing inherits from it. - if (decision || looksLikeDirectory(relative)) { - directoryDecisions.set(relative, decision); - } - return decision; + return options.ignore.ignores(relative); }; const localPath = (path: string): string => join(root, toRelative(path, mountRoot)); @@ -533,7 +501,6 @@ export function withLocalPassthrough( const target = localPath(path); ensureParent(target); fs.mkdirSync(target, { mode: mode === 0 ? DEFAULT_DIR_MODE : mode }); - markDirectory(path); cb(0); } catch (error) { cb(toErrno(error)); @@ -548,7 +515,6 @@ export function withLocalPassthrough( localOps += 1; try { fs.rmdirSync(localPath(path)); - forgetDirectory(path); cb(0); } catch (error) { cb(toErrno(error)); @@ -588,7 +554,6 @@ export function withLocalPassthrough( const target = localPath(destination); ensureParent(target); fs.renameSync(localPath(source), target); - forgetDirectory(source); cb(0); } catch (error) { cb(toErrno(error)); @@ -743,23 +708,6 @@ export function withLocalPassthrough( ); } - function markDirectory(path: string): void { - const relative = toRelative(path, mountRoot); - if (relative !== "") directoryDecisions.set(relative, true); - } - - function forgetDirectory(path: string): void { - const relative = toRelative(path, mountRoot); - if (relative === "") return; - directoryDecisions.delete(relative); - // Descendants inherited from this entry, so they go too. Leaving - // them would let a recreated path keep a stale decision. - const prefix = `${relative}/`; - for (const key of directoryDecisions.keys()) { - if (key.startsWith(prefix)) directoryDecisions.delete(key); - } - } - function localChildren(path: string): string[] { const relative = toRelative(path, mountRoot); const names: string[] = []; @@ -780,20 +728,10 @@ export function withLocalPassthrough( return names; } - function looksLikeDirectory(relative: string): boolean { - try { - return fs.lstatSync(join(root, relative)).isDirectory(); - } catch { - return false; - } - } - return { ops: wrapped, stats: () => ({ localOps, - decisions, - cacheHits, openHandles: handles.size, crossLayerRenames, }), From 1ffa99183ea75bd11cf14f7a503473e6dbdf77ce Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:19:46 +0100 Subject: [PATCH 13/15] computerd: report local-only path counters on /__computerd/stats The local-only layer counts the operations it serves, its open handles, and the renames it refuses with EXDEV, but mountFuse dropped the accessor, so none of it was visible. The README said the rename count was available, and it was not. The mount now exposes the counters, and /__computerd/stats reports them under `localPaths` when MOUNT_IGNORE is active on a real FUSE mount. --- packages/computerd/README.md | 5 +++-- packages/computerd/src/cli/computerd.test.ts | 4 ++++ packages/computerd/src/cli/computerd.ts | 1 + packages/computerd/src/fuse/driver.ts | 17 +++++++++++++---- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/computerd/README.md b/packages/computerd/README.md index 2a70addf..f4496a65 100644 --- a/packages/computerd/README.md +++ b/packages/computerd/README.md @@ -241,8 +241,9 @@ ignore: ["/dist", "/.tmp-build"]; Candidates worth checking are `.next`, `.turbo`, `node_modules/.cache`, and any staging directory a bundler creates next to its output. computerd logs this guidance on the first crossing rename per mount, -naming both sides and the entry to add. Later occurrences are counted on -the passthrough stats but not logged. +naming both sides and the entry to add. Later occurrences are not +logged, but `GET /__computerd/stats` counts them all under +`localPaths.crossLayerRenames`. ### `MOUNT_IGNORE` versus `fetchChanges({ ignore })` diff --git a/packages/computerd/src/cli/computerd.test.ts b/packages/computerd/src/cli/computerd.test.ts index 0dd538f7..858491c3 100644 --- a/packages/computerd/src/cli/computerd.test.ts +++ b/packages/computerd/src/cli/computerd.test.ts @@ -188,6 +188,10 @@ test("MOUNT_IGNORE keeps matching paths on local disk and out of the VFS", async path.join(mountPoint, "node_modules", "final"), ); expect(await fs.readdir(path.join(ignoreRoot, "node_modules"))).toContain("final"); + + // The refused rename is counted where an operator can see it. + const stats = await request(`http://127.0.0.1:${port}/__computerd/stats`); + expect(JSON.parse(stats.body).localPaths).toMatchObject({ crossLayerRenames: 1 }); }); test("/api serves a capnweb WorkspaceRPC session", async (_ctx) => { diff --git a/packages/computerd/src/cli/computerd.ts b/packages/computerd/src/cli/computerd.ts index f4bb529d..ae83b180 100644 --- a/packages/computerd/src/cli/computerd.ts +++ b/packages/computerd/src/cli/computerd.ts @@ -801,6 +801,7 @@ async function main(): Promise { return { ...collectDbStats(db), ...(fuse?.getBufferStats?.() ?? {}), + ...(fuse?.getLocalPathStats === undefined ? {} : { localPaths: fuse.getLocalPathStats() }), store_size_bytes: sizeBytes, store_freelist_count: freelistCount, }; diff --git a/packages/computerd/src/fuse/driver.ts b/packages/computerd/src/fuse/driver.ts index cfca5bba..e5b2c8fd 100644 --- a/packages/computerd/src/fuse/driver.ts +++ b/packages/computerd/src/fuse/driver.ts @@ -2,7 +2,11 @@ import { writeFileSync as nodeWriteFileSync } from "node:fs"; import { posix } from "node:path"; import type { FUSEBackend } from "./backend.js"; import { buildFuseOptionString } from "./options.js"; -import { type LocalPassthroughOptions, withLocalPassthrough } from "./passthrough.js"; +import { + type LocalPassthroughOptions, + type PassthroughStats, + withLocalPassthrough, +} from "./passthrough.js"; import { createFuseTracer, type FuseTracer, wrapFuseOpsWithTracer } from "./tracer.js"; import type { NodeVirtualFileSystem } from "./vfs.js"; @@ -136,6 +140,9 @@ export interface FuseMount { // filesystem. Only present when the mount was created via mountFuse; // the shim does not expose this. getBufferStats?: () => FuseBufferStats; + // Counters for the local-only layer. Present only when MOUNT_IGNORE + // configured local-only paths on a real FUSE mount. + getLocalPathStats?: () => PassthroughStats; } interface FuseNativeInstance { @@ -985,10 +992,11 @@ export async function mountFuse(options: { // Local-only paths are routed before tracing, so the trace counts a // passthrough op once, at the layer that actually served it, rather // than attributing it to the VFS driver that never saw it. - const routedOps = + const localPaths = options.localPaths === undefined - ? baseOps - : withLocalPassthrough(baseOps, options.localPaths).ops; + ? undefined + : withLocalPassthrough(baseOps, options.localPaths); + const routedOps = localPaths === undefined ? baseOps : localPaths.ops; const { getBufferStats: _getBufferStats, ...fuseOps } = routedOps; const ops = tracer === undefined @@ -1062,6 +1070,7 @@ export async function mountFuse(options: { }); }, getBufferStats: _getBufferStats, + ...(localPaths === undefined ? {} : { getLocalPathStats: localPaths.stats }), }; } From 4547229bc414ed1a9eca92e14a0cf58cba3a555a Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:21:03 +0100 Subject: [PATCH 14/15] computerd, computer: correct the EXDEV guidance and use US spelling The docs and the rename warning said callers fall back to copy-then- unlink on EXDEV. Some do: mv and Python's shutil.move copy instead. A program calling rename directly, such as Node's fs.rename or Go's os.Rename, gets the error and has to handle it itself, and the warning is now explicit about that. The README also notes that hardlinks across the boundary return EXDEV for the same reason. The code, comments, and docs this feature added used British spelling (normalise, materialise, behaviour, honour, favour). They now use US spelling, which the repository's prose guidelines ask for. Existing identifiers outside this feature, such as normaliseMountPoint in the FUSE driver, are left alone. --- .../container-backend-ignore.test.ts | 2 +- .../backends/container/container-backend.ts | 2 +- .../backends/container/ignore-assertion.ts | 10 ++++---- packages/computerd/README.md | 13 +++++++---- packages/computerd/src/cli/computerd.test.ts | 3 ++- .../computerd/src/fuse/ignore-config.test.ts | 4 ++-- packages/computerd/src/fuse/ignore-config.ts | 14 +++++------ packages/computerd/src/fuse/ignore.test.ts | 4 ++-- packages/computerd/src/fuse/ignore.ts | 10 ++++---- .../computerd/src/fuse/passthrough.test.ts | 7 +++--- packages/computerd/src/fuse/passthrough.ts | 23 +++++++++++-------- 11 files changed, 50 insertions(+), 42 deletions(-) diff --git a/packages/computer/src/backends/container/container-backend-ignore.test.ts b/packages/computer/src/backends/container/container-backend-ignore.test.ts index 4bfdd617..d098e330 100644 --- a/packages/computer/src/backends/container/container-backend-ignore.test.ts +++ b/packages/computer/src/backends/container/container-backend-ignore.test.ts @@ -2,7 +2,7 @@ // the node runner does not provide, so the full dial cannot complete // here. These exercise the wire format the backend depends on, against // a fake host. The comparison logic and the error text have their own -// suite in ignore-assertion.test.ts, and the end-to-end behaviour is +// suite in ignore-assertion.test.ts, and the end-to-end behavior is // covered in computerd's cli tests against a real FUSE mount. import { afterEach, describe, expect, test, vi } from "vitest"; diff --git a/packages/computer/src/backends/container/container-backend.ts b/packages/computer/src/backends/container/container-backend.ts index caf14925..fb837cda 100644 --- a/packages/computer/src/backends/container/container-backend.ts +++ b/packages/computer/src/backends/container/container-backend.ts @@ -118,7 +118,7 @@ export interface ContainerBackendOptions { // // 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. + // whose computerd is too old to honor the variable. ignore?: readonly string[]; // Number of forced restart attempts after startup readiness diff --git a/packages/computer/src/backends/container/ignore-assertion.ts b/packages/computer/src/backends/container/ignore-assertion.ts index 1762e46c..68029622 100644 --- a/packages/computer/src/backends/container/ignore-assertion.ts +++ b/packages/computer/src/backends/container/ignore-assertion.ts @@ -98,14 +98,14 @@ export function readIgnoreReport(info: unknown): ResolvedIgnore { /** * Compares a declared set against what the image applies; null when they * agree. Order-insensitive and duplicate-collapsing, because computerd - * normalises the same way and the two spellings mean the same thing. + * normalizes the same way and the two spellings mean the same thing. */ export function diffIgnore( declared: readonly string[], actual: readonly string[], ): { missing: string[]; unexpected: string[] } | null { - const declaredSet = new Set(declared.map(normalise)); - const actualSet = new Set(actual.map(normalise)); + const declaredSet = new Set(declared.map(normalize)); + const actualSet = new Set(actual.map(normalize)); const missing = [...declaredSet].filter((entry) => !actualSet.has(entry)).sort(); const unexpected = [...actualSet].filter((entry) => !declaredSet.has(entry)).sort(); @@ -131,7 +131,7 @@ export function assertIgnoreMatches( `\`ignore\` declared ${formatList(declared)}. Those paths would be ` + `recorded in the workspace and pulled into the Durable Object. ` + `Upgrade the computerd image, or remove \`ignore\` to accept the ` + - `container's behaviour.`, + `container's behavior.`, { declared: [...declared], actual: [], supported: false }, ); } @@ -183,7 +183,7 @@ function stripMount(path: string, mountPoint: string | undefined): string { return trimmed; } -function normalise(entry: string): string { +function normalize(entry: string): string { let value = entry.trim(); while (value.startsWith("/")) value = value.slice(1); while (value.endsWith("/")) value = value.slice(0, -1); diff --git a/packages/computerd/README.md b/packages/computerd/README.md index f4496a65..62677cea 100644 --- a/packages/computerd/README.md +++ b/packages/computerd/README.md @@ -172,7 +172,7 @@ The set is compiled once at startup, so it cannot change under a running container, and two sessions sharing one container see the same durability boundary. `connect()` reads the resolved set back off `/__computerd/info` and refuses the connection if it disagrees with what -was declared, which catches a computerd too old to honour the variable. +was declared, which catches a computerd too old to honor the variable. The handle exposes it as absolute container paths: ```ts @@ -199,7 +199,7 @@ feature, because a dropped entry means a full `node_modules` goes into the Durable Object. Duplicates and entries nested inside another entry are dropped as redundant and reported. -`/__computerd/info` reports the normalised configuration: +`/__computerd/info` reports the normalized configuration: ```jsonc { @@ -228,8 +228,11 @@ A rename whose source and destination sit on opposite sides of the boundary returns `EXDEV` (`Invalid cross-device link`). The two sides are different filesystems, so the rename cannot be atomic, and copying then unlinking would fake the atomicity `rename(2)` promises. `mv` and -Python's `shutil.move` already fall back to copy-then-unlink on `EXDEV`. -Renames within one side are ordinary atomic renames. +Python's `shutil.move` copy instead when they see `EXDEV`, but a program +that calls `rename` directly, such as Node's `fs.rename` or Go's +`os.Rename`, gets the error. Renames within one side are ordinary atomic +renames. Hardlinks across the boundary return `EXDEV` for the same +reason. The usual cause is a build tool that stages into a sibling directory and renames into place. The fix is to ignore the staging path too: @@ -251,7 +254,7 @@ logged, but `GET /__computerd/stats` counts them all under `fetchChanges({ ignore })` works at the sync RPC: the path is skipped in one transfer but still occupies the container's store. A wrapper that injects `ignore` into `fetchChanges` to keep a dependency tree out of the -Durable Object should be deleted in favour of `MOUNT_IGNORE`. +Durable Object should be deleted in favor of `MOUNT_IGNORE`. ## FUSE prerequisites diff --git a/packages/computerd/src/cli/computerd.test.ts b/packages/computerd/src/cli/computerd.test.ts index 858491c3..8bf4a335 100644 --- a/packages/computerd/src/cli/computerd.test.ts +++ b/packages/computerd/src/cli/computerd.test.ts @@ -176,7 +176,8 @@ test("MOUNT_IGNORE keeps matching paths on local disk and out of the VFS", async expect(entries).toContain("src"); // A rename across the boundary is refused rather than silently made - // non-atomic. EXDEV is what every tool already falls back from. + // non-atomic. EXDEV is what rename(2) returns between any two + // filesystems. await expect( fs.rename(path.join(mountPoint, "src"), path.join(mountPoint, "dist")), ).rejects.toMatchObject({ code: "EXDEV" }); diff --git a/packages/computerd/src/fuse/ignore-config.test.ts b/packages/computerd/src/fuse/ignore-config.test.ts index 96d01f03..3ddd4d5d 100644 --- a/packages/computerd/src/fuse/ignore-config.test.ts +++ b/packages/computerd/src/fuse/ignore-config.test.ts @@ -15,7 +15,7 @@ describe("resolveMountIgnoreConfig: the root", () => { expect(config.root).toBe("/tmp/workspace"); }); - test("honours an explicit MOUNT_IGNORE_PATH", () => { + test("honors an explicit MOUNT_IGNORE_PATH", () => { const config = resolveMountIgnoreConfig( { MOUNT_IGNORE: "node_modules", MOUNT_IGNORE_PATH: "/var/local-only" }, "/workspace", @@ -105,7 +105,7 @@ describe("resolveMountIgnoreConfig: the set", () => { }); describe("describeMountIgnore", () => { - test("reports the normalised set and the redundant entries", () => { + test("reports the normalized set and the redundant entries", () => { const config = resolveMountIgnoreConfig( { MOUNT_IGNORE: "/node_modules,/node_modules/.cache,/dist" }, "/workspace", diff --git a/packages/computerd/src/fuse/ignore-config.ts b/packages/computerd/src/fuse/ignore-config.ts index 44421f8e..cee198a4 100644 --- a/packages/computerd/src/fuse/ignore-config.ts +++ b/packages/computerd/src/fuse/ignore-config.ts @@ -49,24 +49,24 @@ export function resolveMountIgnoreConfig( throw new Error(`MOUNT_IGNORE_PATH must be an absolute path, got ${JSON.stringify(root)}`); } - const normalisedRoot = resolve(root).replace(/\/+$/, "") || "/"; - const normalisedMount = resolve(mountPoint).replace(/\/+$/, "") || "/"; + const normalizedRoot = resolve(root).replace(/\/+$/, "") || "/"; + const normalizedMount = resolve(mountPoint).replace(/\/+$/, "") || "/"; // A root under the mount would make the passthrough layer resolve into // itself: every write to an ignored path would land at a location that // is also an ignored path, one level deeper, forever. - if (normalisedRoot === normalisedMount || normalisedRoot.startsWith(`${normalisedMount}/`)) { + if (normalizedRoot === normalizedMount || normalizedRoot.startsWith(`${normalizedMount}/`)) { throw new Error( - `MOUNT_IGNORE_PATH (${normalisedRoot}) must not be inside MOUNT_POINT ` + - `(${normalisedMount}); local-only paths are stored outside the mount.`, + `MOUNT_IGNORE_PATH (${normalizedRoot}) must not be inside MOUNT_POINT ` + + `(${normalizedMount}); local-only paths are stored outside the mount.`, ); } - if (normalisedRoot === "/") { + if (normalizedRoot === "/") { throw new Error("MOUNT_IGNORE_PATH must not be the filesystem root"); } - return { root: normalisedRoot, ignore, enabled: !ignore.isEmpty }; + return { root: normalizedRoot, ignore, enabled: !ignore.isEmpty }; } /** The `ignore` block reported on /__computerd/info. */ diff --git a/packages/computerd/src/fuse/ignore.test.ts b/packages/computerd/src/fuse/ignore.test.ts index 851c8c5e..45453b33 100644 --- a/packages/computerd/src/fuse/ignore.test.ts +++ b/packages/computerd/src/fuse/ignore.test.ts @@ -104,7 +104,7 @@ describe("resolveMountIgnore: matching", () => { }); }); -describe("resolveMountIgnore: normalisation", () => { +describe("resolveMountIgnore: normalization", () => { test("strips leading and trailing slashes from entries", () => { const set = resolveMountIgnore(["/dist/", "node_modules/"]); expect(set.paths).toEqual(["dist", "node_modules"]); @@ -197,7 +197,7 @@ describe("resolveMountIgnore: redundancy", () => { expect(set.redundant).toEqual([]); }); - test("normalises before deduplicating", () => { + test("normalizes before deduplicating", () => { const set = resolveMountIgnore(["/dist/", "dist"]); expect(set.paths).toEqual(["dist"]); expect(set.redundant).toEqual(["dist"]); diff --git a/packages/computerd/src/fuse/ignore.ts b/packages/computerd/src/fuse/ignore.ts index 4371abf1..36c7a7ce 100644 --- a/packages/computerd/src/fuse/ignore.ts +++ b/packages/computerd/src/fuse/ignore.ts @@ -8,7 +8,7 @@ // // The set is resolved once at startup and never re-read: entries that // changed under a running command would mean migrating -// already-materialised paths between layers mid-write. +// already-materialized paths between layers mid-write. /** An entry that cannot be used, carrying enough context to fix it. */ export class MountIgnorePathError extends Error { @@ -28,7 +28,7 @@ export interface MountIgnoreSet { readonly ignores: (relativePath: string) => boolean; /** The entry covering a path, or undefined when not local-only. */ readonly entryFor: (relativePath: string) => string | undefined; - /** Normalised entries, in declaration order, as the mount applies them. */ + /** Normalized entries, in declaration order, as the mount applies them. */ readonly paths: readonly string[]; /** Entries dropped as duplicates or as nested inside another entry. */ readonly redundant: readonly string[]; @@ -51,11 +51,11 @@ export function parseMountIgnore(raw: string | undefined): string[] { } /** - * Normalises entries and builds the matcher. An absolute path outside + * Normalizes entries and builds the matcher. An absolute path outside * the mount is rejected rather than reinterpreted. */ export function resolveMountIgnore(entries: readonly string[], mountPoint = "/"): MountIgnoreSet { - const root = normaliseMount(mountPoint); + const root = normalizeMount(mountPoint); const paths: string[] = []; const redundant: string[] = []; @@ -149,7 +149,7 @@ function stripSlashes(value: string): string { return out; } -function normaliseMount(mountPoint: string): string { +function normalizeMount(mountPoint: string): string { const trimmed = mountPoint.replace(/\/+$/, ""); return trimmed === "" ? "/" : trimmed; } diff --git a/packages/computerd/src/fuse/passthrough.test.ts b/packages/computerd/src/fuse/passthrough.test.ts index c8154c78..c930c418 100644 --- a/packages/computerd/src/fuse/passthrough.test.ts +++ b/packages/computerd/src/fuse/passthrough.test.ts @@ -26,7 +26,7 @@ const realFs = (): PassthroughFs => ({ ...nodeFs }) as PassthroughFs; // Drives the real node:fs against a temp directory rather than a double. // The interesting failures here -- EXDEV, ENOTEMPTY, parent creation -- // are the filesystem's, so a mock would assert the shape of the calls -// rather than the behaviour. +// rather than the behavior. const MOUNT = "/workspace"; @@ -338,7 +338,8 @@ describe("withLocalPassthrough: rename", () => { // Not a copy. The two sides are different filesystems, so the // operation cannot be atomic, and faking it would turn a crash // mid-copy into a half-written file where the caller was promised - // all-or-nothing. Tools fall back to copy-then-unlink on EXDEV. + // all-or-nothing. EXDEV is what rename(2) returns between any two + // filesystems. const { ops, calls } = build(["dist"]); let intoLocal = 0; @@ -391,7 +392,7 @@ describe("withLocalPassthrough: directory listing", () => { expect(names).toContain("node_modules"); }); - test("does not show an entry that has not been materialised", () => { + test("does not show an entry that has not been materialized", () => { const source = recordingOps(); const { ops } = withLocalPassthrough(source.ops, { root, diff --git a/packages/computerd/src/fuse/passthrough.ts b/packages/computerd/src/fuse/passthrough.ts index 47a3fc24..faa060e4 100644 --- a/packages/computerd/src/fuse/passthrough.ts +++ b/packages/computerd/src/fuse/passthrough.ts @@ -81,7 +81,7 @@ export interface LocalPassthroughOptions { /** Injected for tests. Defaults to the real node:fs surface. */ readonly fs?: PassthroughFs; /** Called once per distinct local-only directory created. Diagnostics. */ - readonly onMaterialise?: (relativePath: string) => void; + readonly onMaterialize?: (relativePath: string) => void; /** Operator-facing warnings. Defaults to console.warn; injected for tests. */ readonly warn?: (message: string) => void; } @@ -188,7 +188,7 @@ export function withLocalPassthrough( const fs = options.fs ?? REAL_FS; const root = options.root.replace(/\/+$/, ""); - const mountRoot = normaliseMount(options.mountPoint ?? "/"); + const mountRoot = normalizeMount(options.mountPoint ?? "/"); let localOps = 0; let crossLayerRenames = 0; @@ -218,7 +218,7 @@ export function withLocalPassthrough( const parent = dirname(target); try { fs.mkdirSync(parent, { recursive: true, mode: DEFAULT_DIR_MODE }); - options.onMaterialise?.(parent); + options.onMaterialize?.(parent); } catch (error) { if (errnoOf(error) !== "EEXIST") throw error; } @@ -535,8 +535,9 @@ export function withLocalPassthrough( // different filesystems and the operation cannot be atomic. // Copying here would make a non-atomic operation look atomic, // and a crash mid-copy would leave a half-written file where - // the caller was promised all-or-nothing. Every tool already - // handles EXDEV by falling back to copy-then-unlink. + // the caller was promised all-or-nothing. EXDEV is what rename(2) + // returns between any two filesystems, so tools such as mv + // already know to copy instead. // // The errno is all the kernel can carry, and "cross-device // link" on a path that is plainly not a device is the kind of @@ -702,7 +703,9 @@ export function withLocalPassthrough( `boundary and returned EXDEV. ${localSide} is container-local ` + `(MOUNT_IGNORE), ${syncedSide} is synced to the workspace; a rename ` + `between them cannot be atomic, so it is refused rather than ` + - `silently copied. Most callers fall back to copy-then-unlink. To ` + + `silently copied. Tools such as mv copy instead, but a program ` + + `calling rename directly (Node's fs.rename, Go's os.Rename) sees ` + + `the error. To ` + `keep the rename atomic, add "${suggestion}" to MOUNT_IGNORE as ` + `well. Further occurrences are not logged.`, ); @@ -713,8 +716,8 @@ export function withLocalPassthrough( const names: string[] = []; for (const entry of options.ignore.paths) { const parent = posix.dirname(entry); - const normalisedParent = parent === "." ? "" : parent; - if (normalisedParent !== relative) continue; + const normalizedParent = parent === "." ? "" : parent; + if (normalizedParent !== relative) continue; // Only list it if it has actually been created on disk. An // unconfigured-but-unused entry should not appear as a phantom // directory in a listing. @@ -722,7 +725,7 @@ export function withLocalPassthrough( fs.lstatSync(join(root, entry)); names.push(posix.basename(entry)); } catch { - // Not materialised yet; nothing to show. + // Not materialized yet; nothing to show. } } return names; @@ -748,7 +751,7 @@ function toRelative(path: string, mountRoot: string): string { return value; } -function normaliseMount(mountPoint: string): string { +function normalizeMount(mountPoint: string): string { const trimmed = mountPoint.replace(/\/+$/, ""); return trimmed === "" ? "/" : trimmed; } From 3d548cffed4281484936286491560f0db3c0bf13 Mon Sep 17 00:00:00 2001 From: aron <263346377+aron-cf@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:35:13 +0000 Subject: [PATCH 15/15] computer: describe the ignore set as configured, not owned by the image The local-only path check still described the set as a property of the image, with the client only able to state an expectation. That stopped being true when `ignore` began setting MOUNT_IGNORE at container start. The file header, doc comments, mismatch messages, and test names now say the container applies the set the backend passed it, and name the two real causes of a mismatch: a computerd too old to read MOUNT_IGNORE, or a MOUNT_IGNORE in `containerEnv` overriding the option. --- .../container/ignore-assertion.test.ts | 12 ++++----- .../backends/container/ignore-assertion.ts | 27 ++++++++++--------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/packages/computer/src/backends/container/ignore-assertion.test.ts b/packages/computer/src/backends/container/ignore-assertion.test.ts index 765eaa88..8b5fb2ce 100644 --- a/packages/computer/src/backends/container/ignore-assertion.test.ts +++ b/packages/computer/src/backends/container/ignore-assertion.test.ts @@ -98,14 +98,14 @@ describe("diffIgnore", () => { expect(diffIgnore(["dist", "dist"], ["dist"])).toBeNull(); }); - test("reports a path the image does not apply", () => { + test("reports a path the container does not apply", () => { expect(diffIgnore(["node_modules", "dist"], ["node_modules"])).toEqual({ missing: ["dist"], unexpected: [], }); }); - test("reports a path the image applies but the caller did not declare", () => { + test("reports a path the container applies but the caller did not declare", () => { expect(diffIgnore(["node_modules"], ["node_modules", "target"])).toEqual({ missing: [], unexpected: ["target"], @@ -116,9 +116,9 @@ describe("diffIgnore", () => { expect(diffIgnore(["a", "b"], ["b", "c"])).toEqual({ missing: ["a"], unexpected: ["c"] }); }); - test("an empty declaration against a configured image is a mismatch", () => { + test("an empty declaration against a configured container is a mismatch", () => { // Distinct from omitting `ignore` entirely, which skips the check. - // Declaring "nothing is local-only" against an image that makes + // Declaring "nothing is local-only" against a container that makes // node_modules local-only is a real disagreement. expect(diffIgnore([], ["node_modules"])).toEqual({ missing: [], @@ -187,7 +187,7 @@ describe("assertIgnoreMatches", () => { } }); - test("names which paths will be synced when the image is missing one", () => { + test("names which paths will be synced when the container is missing one", () => { try { assertIgnoreMatches(["node_modules", "dist"], supported(["node_modules"])); expect.unreachable("should have thrown"); @@ -198,7 +198,7 @@ describe("assertIgnoreMatches", () => { } }); - test("names which paths will not be synced when the image adds one", () => { + test("names which paths will not be synced when the container adds one", () => { // The opposite direction is just as dangerous: the caller believes // `target` is durable and it is not. try { diff --git a/packages/computer/src/backends/container/ignore-assertion.ts b/packages/computer/src/backends/container/ignore-assertion.ts index 68029622..8bf9696a 100644 --- a/packages/computer/src/backends/container/ignore-assertion.ts +++ b/packages/computer/src/backends/container/ignore-assertion.ts @@ -1,11 +1,13 @@ -// Client-side assertion over the container's local-only path set. The -// set is owned by the image; a client can only state what it expects -// and refuse to connect on disagreement. See packages/computerd/README.md. +// Client-side check of the container's local-only path set. The backend +// passes `ignore` to the container as MOUNT_IGNORE at start, then reads +// back what computerd actually applied and refuses to connect if the two +// disagree. See packages/computerd/README.md. // // Fails the connection rather than warning, because the failure it -// guards is silent and expensive: an image built without MOUNT_IGNORE -// looks identical to a correct one until a command writes a large -// dependency tree and the whole thing is pulled into the Durable +// guards is silent and expensive: a computerd too old to read +// MOUNT_IGNORE, or a MOUNT_IGNORE in `containerEnv` overriding the +// option, looks identical to a correct setup until a command writes a +// large dependency tree and the whole thing is pulled into the Durable // Object -- the #179 symptom. A mismatch is a deployment error, and a // loud one is cheaper than a slow one. @@ -96,9 +98,10 @@ export function readIgnoreReport(info: unknown): ResolvedIgnore { } /** - * Compares a declared set against what the image applies; null when they - * agree. Order-insensitive and duplicate-collapsing, because computerd - * normalizes the same way and the two spellings mean the same thing. + * Compares a declared set against what the container applies; null when + * they agree. Order-insensitive and duplicate-collapsing, because + * computerd normalizes the same way and the two spellings mean the same + * thing. */ export function diffIgnore( declared: readonly string[], @@ -115,7 +118,7 @@ export function diffIgnore( } /** - * Throws when the image disagrees. `declared === undefined` skips the + * Throws when the container disagrees. `declared === undefined` skips the * check, so an existing deployment cannot start failing because a new * field appeared. */ @@ -148,13 +151,13 @@ export function assertIgnoreMatches( const parts: string[] = []; if (difference.missing.length > 0) { parts.push( - `declared but not applied by the image: ${formatList(difference.missing)} ` + + `declared but not applied by the container: ${formatList(difference.missing)} ` + `(these paths WILL be synced)`, ); } if (difference.unexpected.length > 0) { parts.push( - `applied by the image but not declared: ${formatList(difference.unexpected)} ` + + `applied by the container but not declared: ${formatList(difference.unexpected)} ` + `(these paths will NOT be synced)`, ); }