diff --git a/AGENTS.md b/AGENTS.md index dcde670d..24dbd445 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,6 +28,60 @@ the file directly when the trigger applies: | [`capnweb`](.agents/skills/capnweb/SKILL.md) | Touching anything that crosses the RPC boundary: `packages/rpc`, `packages/workspace`, the `wsd` client, or the Durable Object server. | | [`cloudflare`](.agents/skills/cloudflare/SKILL.md) | Index of host-side Cloudflare skills — Workers, Durable Objects, wrangler, sandbox SDK, agents SDK. | +## Environment setup + +A fresh container does not have everything the tests need. The traps +below cost real time if you discover them one failure at a time. + +**Native build tools.** `packages/wsd` depends on `fuse-native`, a +native addon. Building it needs a C toolchain and the libfuse2 headers. +On Debian or Ubuntu: + +```bash +apt-get install build-essential libfuse-dev +``` + +If the `fuse-native` build fails, `npm install` aborts the whole +install, not just that one package. When you only need the rest of the +workspace, install with `npm install --ignore-scripts` to skip the +native build. + +**arm64 hosts.** `fuse-native` ships a prebuilt libfuse for x64 only. +On a Linux arm64 host or container (including a Linux container on +Apple Silicon, or arm64 CI) the link fails with `file in wrong +format`. The path below is Debian or Ubuntu arm64; a native macOS host +uses macFUSE instead and does not hit this. Replace the bundled library +with the system one and rebuild: + +```bash +cp /usr/lib/aarch64-linux-gnu/libfuse.so.2 \ + node_modules/fuse-shared-library-linux/libfuse/lib/libfuse.so +cd node_modules/fuse-native && npx node-gyp rebuild +``` + +**Build before you test.** The test scripts don't build the sibling +packages first. Several suites need build output that is absent in +a clean checkout: `packages/wsd` imports the sibling `@cloudflare/dofs` +and `@cloudflare/workspace-rpc` packages from their `dist/` +directories, `packages/wsd`'s `src/cli/wsd.test.ts` spawns the bundled +CLI at `dist/cli/wsd.cjs`, and `examples/think-compare-runtimes` +imports `@cloudflare/workspace/backends/container`, which exists only +after the `workspace` package is built. Run `npm run build` across the +workspace before `npm test` on a clean checkout. + +**Real FUSE needs privilege.** `packages/wsd`'s `src/cli/wsd.test.ts` +runs its real-FUSE case only when `/dev/fuse` is reachable; otherwise +it resolves to the shim and skips. The guard is a bare existence check, +so a `mknod`'d `/dev/fuse` in an unprivileged container defeats the +skip and the mount then fails with `EPERM`, turning a clean skip into a +hard failure. Leave the device absent unless the container is +privileged (`--privileged`, or `CAP_SYS_ADMIN` with device access). The +`src/exec/runner.fuse.test.ts` suite is separate: it skips unless both +Docker and the prebuilt `wsd` binary are available, and runs `wsd` +inside a privileged container. See the +[`debugging-wsd-fuse`](.agents/skills/debugging-wsd-fuse/SKILL.md) skill +for the privileged Docker setup. + ## Checks before you finish Run from the repo root: diff --git a/PR.md b/PR.md new file mode 100644 index 00000000..da776f62 --- /dev/null +++ b/PR.md @@ -0,0 +1,27 @@ +Every file and directory on the `wsd` FUSE mount reports zero disk usage. Writing twelve bytes and asking `du` for the size returns `0`: + +```sh +printf 'hello world\n' > /workspace/du-repro.txt +stat -c 'size=%s blocks=%b' /workspace/du-repro.txt +# size=12 blocks=0 +du -B1 /workspace/du-repro.txt +# 0 /workspace/du-repro.txt +``` + +This is not a `du` bug. `du` reads `st_blocks` from `stat(2)`, not `st_size`, and the FUSE driver was leaving `st_blocks` empty. The `getattr` path built its stat result without the `blocks` and `blksize` fields, so the kernel saw zero allocated blocks for every inode on the mount. + +The fix populates both fields wherever the driver builds a stat. `st_blocks` counts allocation in fixed 512-byte units, the unit POSIX defines for that field regardless of the filesystem's logical block size, so a 513-byte file occupies two blocks and an empty file occupies none. `st_blksize` is the preferred input/output size, a separate value that stays at `4096` to match what `statfs` already advertises and what the backing virtual filesystem reports. The backing filesystem already supplies both fields for files written to disk, so the driver passes those through and only derives the values for the in-memory cases: a freshly created file before its first flush, and a file whose buffered size has outrun the size on disk. + +Reviewers with a privileged FUSE-capable container can verify the behavior against a real mount: + +```sh +printf 'hello world\n' > /workspace/du-repro.txt +stat -c 'size=%s blocks=%b' /workspace/du-repro.txt +# size=12 blocks=1 +du -B1 /workspace/du-repro.txt +# 512 /workspace/du-repro.txt +``` + +The regression tests cover the same block accounting without requiring a mount. They assert block counts for a 513-byte file, an empty file, a freshly created file that has not flushed, and a file whose buffered size has grown past a block boundary before flush. These tests fail against the old stat shape because `blocks` and `blksize` are missing, and pass with this change. + +This also updates the setup documentation around running the tests from a clean container. `AGENTS.md` now calls out the native build tools `fuse-native` needs, the Linux arm64 libfuse swap needed when the package's bundled x64 library cannot link, the need to build sibling package output before running tests, and the different gates used by the two real-FUSE test suites. The `packages/wsd` README no longer claims that its test script builds first or uses Node's type stripping; it describes the Vitest command, the required build output, and the difference between the `/dev/fuse`-guarded CLI test and the Docker-backed real-FUSE runner test. diff --git a/packages/wsd/README.md b/packages/wsd/README.md index e4104c66..8301e4e8 100644 --- a/packages/wsd/README.md +++ b/packages/wsd/README.md @@ -135,13 +135,17 @@ Caveats. The shim is dev-only: ## Tests -Tests live next to the source files and are written in TypeScript. The package test script builds first, then runs Node's experimental TypeScript stripping: +Tests live next to the source files and are written in TypeScript. Vitest runs them directly: ```sh npm test --workspace=@cloudflare/workspace-wsd ``` -This package requires Node.js 22+ because `@platformatic/vfs` does, and because the test script uses `--experimental-strip-types`, which is only available on Node 22+ (unflagged on 23.6+). +The test command does not build first. Some suites need build output that is not there in a clean checkout: the tests import the sibling `@cloudflare/dofs` and `@cloudflare/workspace-rpc` packages from their `dist/` directories, and `src/cli/wsd.test.ts` spawns the bundled CLI at `dist/cli/wsd.cjs`. Run `npm run build` across the workspace before `npm test`, or those tests fail to resolve the imports or exit early with no bundle to spawn. + +This package requires Node.js 22+ because `@platformatic/vfs` does. + +The two real-FUSE suites gate themselves differently. `src/cli/wsd.test.ts` runs its real-FUSE case only when `/dev/fuse` is reachable; otherwise auto-detection resolves to the shim and the case skips. The guard is a bare existence check, so a `mknod`'d `/dev/fuse` in an unprivileged container defeats the skip and the mount then fails with `EPERM` — leave the device absent unless the container is privileged (`--privileged`, or `CAP_SYS_ADMIN` with device access). `src/exec/runner.fuse.test.ts` is separate: it skips unless both Docker and the prebuilt `wsd` binary are available, and runs `wsd` inside a privileged container, so the host's `/dev/fuse` does not matter. See the [`debugging-wsd-fuse`](../../.agents/skills/debugging-wsd-fuse/SKILL.md) skill for the privileged Docker setup. ## Standalone release artifacts diff --git a/packages/wsd/src/fuse/driver.test.ts b/packages/wsd/src/fuse/driver.test.ts index c924fb95..274e4698 100644 --- a/packages/wsd/src/fuse/driver.test.ts +++ b/packages/wsd/src/fuse/driver.test.ts @@ -906,3 +906,96 @@ test("FUSE getattr on a pending-create file returns a stable mtime", async () => (first.result as { mtime: Date }).mtime.getTime(), ); }); + +test("FUSE getattr reports st_blocks so du sees non-zero usage", async () => { + // Regression: getattr omitted `blocks`/`blksize`, so the kernel + // reported st_blocks=0 for every inode and `du`, which reads + // st_blocks rather than st_size, reported zero usage across the + // whole mount. POSIX st_blocks counts 512-byte units; a 513-byte + // file occupies two of them. + const { vfs } = await createNodeVirtualFileSystem(); + const ops = makeFUSEOps(vfs); + + const create = await callback((cb) => ops.create("/du.txt", 0o644, cb)); + expect(create.errno).toBe(0); + + const payload = Buffer.alloc(513, 0x61); + expect( + await status((cb) => + ops.write("/du.txt", create.result as number, payload, payload.length, 0, cb), + ), + ).toBe(payload.length); + expect(await status((cb) => ops.flush("/du.txt", create.result as number, cb))).toBe(0); + expect(await status((cb) => ops.release("/du.txt", create.result as number, cb))).toBe(0); + + const stat = await callback((cb) => ops.getattr("/du.txt", cb)); + expect(stat.errno).toBe(0); + expect(stat.result).toMatchObject({ size: 513, blksize: 4096, blocks: 2 }); +}); + +test("FUSE getattr reports zero blocks for an empty file", async () => { + const { vfs } = await createNodeVirtualFileSystem(); + const ops = makeFUSEOps(vfs); + + const create = await callback((cb) => ops.create("/empty.txt", 0o644, cb)); + expect(create.errno).toBe(0); + expect(await status((cb) => ops.flush("/empty.txt", create.result as number, cb))).toBe(0); + expect(await status((cb) => ops.release("/empty.txt", create.result as number, cb))).toBe(0); + + const stat = await callback((cb) => ops.getattr("/empty.txt", cb)); + expect(stat.errno).toBe(0); + expect(stat.result).toMatchObject({ size: 0, blksize: 4096, blocks: 0 }); +}); + +test("FUSE getattr on a pending-create file reports block metadata", async () => { + // The pending-create window stats out of the in-memory buffer, not + // the VFS. Its block accounting must match the buffered size so a + // `du` before the first flush still sees the bytes. + const { vfs } = await createNodeVirtualFileSystem(); + const ops = makeFUSEOps(vfs); + + const create = await callback((cb) => ops.create("/pending.txt", 0o644, cb)); + expect(create.errno).toBe(0); + + const payload = Buffer.alloc(1025, 0x62); + expect( + await status((cb) => + ops.write("/pending.txt", create.result as number, payload, payload.length, 0, cb), + ), + ).toBe(payload.length); + + const stat = await callback((cb) => ops.getattr("/pending.txt", cb)); + expect(stat.errno).toBe(0); + expect(stat.result).toMatchObject({ size: 1025, blksize: 4096, blocks: 3 }); +}); + +test("FUSE getattr block count tracks buffered size before flush", async () => { + // With buffered writes the VFS inode still holds the old size while + // the FileEntry carries the fresh bytes. getattr overrides size + // with the buffered value; blocks must follow so a `du` before the + // spill matches the size the kernel sees. + const { vfs } = await createNodeVirtualFileSystem(); + const ops = makeFUSEOps(vfs); + + const create = await callback((cb) => ops.create("/buf-blocks.txt", 0o644, cb)); + expect(create.errno).toBe(0); + const seed = Buffer.from("seed"); + expect( + await status((cb) => + ops.write("/buf-blocks.txt", create.result as number, seed, seed.length, 0, cb), + ), + ).toBe(seed.length); + expect(await status((cb) => ops.flush("/buf-blocks.txt", create.result as number, cb))).toBe(0); + + // Grow the buffer past a block boundary without flushing. + const grow = Buffer.alloc(2000, 0x63); + expect( + await status((cb) => + ops.write("/buf-blocks.txt", create.result as number, grow, grow.length, 0, cb), + ), + ).toBe(grow.length); + + const stat = await callback((cb) => ops.getattr("/buf-blocks.txt", cb)); + expect(stat.errno).toBe(0); + expect(stat.result).toMatchObject({ size: 2000, blksize: 4096, blocks: 4 }); +}); diff --git a/packages/wsd/src/fuse/driver.ts b/packages/wsd/src/fuse/driver.ts index 6bca34d1..441ae30b 100644 --- a/packages/wsd/src/fuse/driver.ts +++ b/packages/wsd/src/fuse/driver.ts @@ -24,6 +24,22 @@ const ERRNO = { // typical container memory limits. const MAX_FILE_BYTES = 256 * 1024 * 1024; +// POSIX st_blocks counts allocation in fixed 512-byte units regardless +// of the filesystem's logical block size, so consumers like GNU `du` +// (which reads st_blocks, not st_size) compute usage against this +// constant. A getattr that omits blocks makes the kernel surface +// st_blocks=0 and `du` reports zero usage for the whole mount. +const STAT_BLOCK_SIZE = 512; + +// st_blksize is the preferred I/O block size, not the st_blocks unit. +// Match what statfs advertises (bsize: 4096) and what the backing VFS +// reports so a fabricated stat stays consistent with a persisted one. +const PREFERRED_IO_BLOCK_SIZE = 4096; + +function blocksForSize(size: number): number { + return size <= 0 ? 0 : Math.ceil(size / STAT_BLOCK_SIZE); +} + type StatusCallback = (errnoOrBytes: number) => void; type ResultCallback = (errno: number, result: T) => void; type NotImplementedOperation = (...args: unknown[]) => void; @@ -97,6 +113,8 @@ export interface FuseStat { gid: number; nlink: number; ino: number; + blksize: number; + blocks: number; } export interface FuseBufferStats { @@ -308,6 +326,8 @@ export function makeFUSEOps(vfs: NodeVirtualFileSystem, mountPoint = "/"): FuseO gid: typeof process.getgid === "function" ? process.getgid() : 0, nlink: 1, ino: 0, + blksize: PREFERRED_IO_BLOCK_SIZE, + blocks: blocksForSize(entry.size), }; }; // Returns true on success, false if `needed` exceeds MAX_FILE_BYTES. @@ -404,8 +424,14 @@ export function makeFUSEOps(vfs: NodeVirtualFileSystem, mountPoint = "/"): FuseO const entry = files.get(path); const stat = entry?.pendingCreate === true ? pendingStat(entry) : statNode(vfs.lstatSync(toVfs(path))); - // File content lives outside the VFS, so prefer our size. - if (entry !== undefined) stat.size = entry.size; + // File content lives outside the VFS, so prefer our size. The + // buffered size can outrun the persisted inode before a spill, + // so recompute block accounting from it to keep st_blocks + // coherent with the size the kernel sees. + if (entry !== undefined) { + stat.size = entry.size; + stat.blocks = blocksForSize(entry.size); + } const override = meta.get(path); if (override) { if (override.mode !== undefined) { @@ -1021,6 +1047,8 @@ function statNode(stat: { mode: number; nlink?: number; ino?: number; + blksize?: number; + blocks?: number; isDirectory(): boolean; }): FuseStat { return { @@ -1033,6 +1061,11 @@ function statNode(stat: { gid: typeof process.getgid === "function" ? process.getgid() : 0, nlink: stat.nlink ?? (stat.isDirectory() ? 2 : 1), ino: stat.ino ?? 0, + // Prefer provider-supplied block accounting when the VFS exposes + // it; otherwise derive from size in 512-byte units so `du` and + // other st_blocks consumers see real usage. + blksize: stat.blksize ?? PREFERRED_IO_BLOCK_SIZE, + blocks: stat.blocks ?? blocksForSize(stat.size), }; }