diff --git a/.changeset/calm-ravens-sync.md b/.changeset/calm-ravens-sync.md new file mode 100644 index 00000000..b79ac8b0 --- /dev/null +++ b/.changeset/calm-ravens-sync.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": patch +--- + +Create missing parent directories while applying sync entries. diff --git a/packages/dofs/src/fs/mkdir.ts b/packages/dofs/src/fs/mkdir.ts index 4d664c61..ddc3b72a 100644 --- a/packages/dofs/src/fs/mkdir.ts +++ b/packages/dofs/src/fs/mkdir.ts @@ -3,7 +3,7 @@ import { canonicalizePath } from "../path.js"; import { incrementRev } from "../rev.js"; import { ROOT_INODE } from "../schema/index.js"; import type { Database } from "../storage.js"; -import { assertNotReadOnly } from "./mount-guard.js"; +import { assertNotInReadOnlyMount, assertNotReadOnly } from "./mount-guard.js"; import { invalidateResolveExact } from "./resolveCache.js"; export interface MkdirOptions { @@ -68,6 +68,25 @@ function createDir( } export function mkdir(db: Database, path: string, options: MkdirOptions, now: () => number): void { + mkdirWithGuard(db, path, options, now, assertNotReadOnly); +} + +export function mkdirForSyncParents( + db: Database, + path: string, + options: MkdirOptions, + now: () => number, +): void { + mkdirWithGuard(db, path, options, now, assertNotInReadOnlyMount); +} + +function mkdirWithGuard( + db: Database, + path: string, + options: MkdirOptions, + now: () => number, + guard: (db: Database, path: string) => void, +): void { const { parts, path: canonical } = canonicalizePath(path); const recursive = options.recursive === true; const mode = (options.mode ?? 0o755) & 0o7777; @@ -79,7 +98,7 @@ export function mkdir(db: Database, path: string, options: MkdirOptions, now: () // undefined, but our docs treat mkdir("/") as nonsensical). throw createWorkspaceError("EEXIST", `path exists: ${canonical}`, canonical); } - assertNotReadOnly(db, canonical); + guard(db, canonical); db.transactionSync(() => { const rev = incrementRev(db); diff --git a/packages/dofs/src/fs/resolve.ts b/packages/dofs/src/fs/resolve.ts index 5248757c..abc100fc 100644 --- a/packages/dofs/src/fs/resolve.ts +++ b/packages/dofs/src/fs/resolve.ts @@ -106,6 +106,16 @@ export function resolveInode( return cte.node; } +// Resolve a path in one statement without following symlinks in any +// component. A path containing a symlink returns null. Sync parent +// creation uses this for the common case, then inspects each ancestor +// only when the path is missing or blocked. +export function resolveInodeWithoutSymlinks(db: Database, path: string): ResolvedInode | null { + const { parts } = canonicalizePath(path); + const resolution = resolveViaCte(db, parts); + return resolution.kind === "resolved" ? resolution.node : null; +} + interface CteRow { level: number; inode: number; diff --git a/packages/dofs/src/sync/apply.test.ts b/packages/dofs/src/sync/apply.test.ts index 8c3245c2..2076d9b9 100644 --- a/packages/dofs/src/sync/apply.test.ts +++ b/packages/dofs/src/sync/apply.test.ts @@ -76,6 +76,315 @@ describe("applyChanges", () => { ); }); + describe.each(["async", "sync"] as const)("%s apply", (mode) => { + const apply = async (db: import("../storage.js").Database, entries: ChangeEntry[]) => { + if (mode === "sync") { + return applyChangesSync(db, entries, new Map(), { source: "upstream" }); + } + return applyChanges(db, entries, new Map(), { source: "upstream" }); + }; + + it("creates missing parents before applying a file", async () => { + await withDB(async (db) => { + await apply(db, [ + { + kind: "file", + rev: 4, + path: "/workspace/newdir/file.txt", + mode: 0o640, + mtime: 4, + size: 0, + chunks: [], + }, + { kind: "dir", rev: 8, path: "/workspace/newdir", mode: 0o700, mtime: 8 }, + { kind: "dir", rev: 9, path: "/workspace", mode: 0o750, mtime: 9 }, + ]); + + expect(await readFile(db, "/workspace/newdir/file.txt", "utf8")).toBe(""); + expect(resolveInode(db, "/workspace", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o750, + }); + expect(resolveInode(db, "/workspace/newdir", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + }); + }); + }); + + it("creates missing parents before applying a symlink", async () => { + await withDB(async (db) => { + await apply(db, [ + { + kind: "symlink", + rev: 4, + path: "/workspace/newdir/link", + target: "../target", + mode: 0o777, + mtime: 4, + }, + { kind: "dir", rev: 8, path: "/workspace/newdir", mode: 0o700, mtime: 8 }, + { kind: "dir", rev: 9, path: "/workspace", mode: 0o750, mtime: 9 }, + ]); + + expect(readlink(db, "/workspace/newdir/link")).toBe("../target"); + expect(resolveInode(db, "/workspace", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o750, + }); + expect(resolveInode(db, "/workspace/newdir", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + }); + }); + }); + + it.each(["file", "symlink"] as const)( + "replaces a blocking %s ancestor before applying a file", + async (kind) => { + await withDB(async (db) => { + if (kind === "file") { + await writeFile(db, "/workspace", "old", {}, () => 1); + } else { + mkdir(db, "/target", {}, () => 1); + await writeFile(db, "/target/keep.txt", "keep", {}, () => 2); + symlink(db, "/target", "/workspace", () => 3); + } + + await apply(db, [ + { + kind: "file", + rev: 4, + path: "/workspace/newdir/file.txt", + mode: 0o640, + mtime: 4, + size: 0, + chunks: [], + }, + { kind: "dir", rev: 8, path: "/workspace/newdir", mode: 0o700, mtime: 8 }, + { kind: "dir", rev: 9, path: "/workspace", mode: 0o750, mtime: 9 }, + ]); + + expect(await readFile(db, "/workspace/newdir/file.txt", "utf8")).toBe(""); + expect(resolveInode(db, "/workspace", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o750, + }); + expect(resolveInode(db, "/workspace/newdir", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + }); + if (kind === "symlink") { + expect(await readFile(db, "/target/keep.txt", "utf8")).toBe("keep"); + } + }); + }, + ); + + it("writes a file through a reachable symlink parent", async () => { + await withDB(async (db) => { + mkdir(db, "/target", {}, () => 1); + await writeFile(db, "/target/keep.txt", "keep", {}, () => 2); + symlink(db, "/target", "/workspace", () => 3); + + await apply(db, [ + { + kind: "file", + rev: 4, + path: "/workspace/new.txt", + mode: 0o640, + mtime: 4, + size: 0, + chunks: [], + }, + ]); + + expect(resolveInode(db, "/workspace", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(db, "/workspace")).toBe("/target"); + expect(await readFile(db, "/workspace/keep.txt", "utf8")).toBe("keep"); + expect(await readFile(db, "/target/new.txt", "utf8")).toBe(""); + }); + }); + + it("creates a directory through a reachable symlink parent", async () => { + await withDB(async (db) => { + mkdir(db, "/target", {}, () => 1); + await writeFile(db, "/target/keep.txt", "keep", {}, () => 2); + symlink(db, "/target", "/workspace", () => 3); + + await apply(db, [ + { kind: "dir", rev: 4, path: "/workspace/newdir", mode: 0o700, mtime: 4 }, + ]); + + expect(resolveInode(db, "/workspace", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(db, "/workspace")).toBe("/target"); + expect(await readFile(db, "/workspace/keep.txt", "utf8")).toBe("keep"); + expect(resolveInode(db, "/target/newdir", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + }); + }); + }); + + it("creates a symlink through a reachable symlink parent", async () => { + await withDB(async (db) => { + mkdir(db, "/target", {}, () => 1); + await writeFile(db, "/target/keep.txt", "keep", {}, () => 2); + symlink(db, "/target", "/workspace", () => 3); + + await apply(db, [ + { + kind: "symlink", + rev: 4, + path: "/workspace/newlink", + target: "keep.txt", + mode: 0o777, + mtime: 4, + }, + ]); + + expect(resolveInode(db, "/workspace", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(db, "/workspace")).toBe("/target"); + expect(await readFile(db, "/workspace/keep.txt", "utf8")).toBe("keep"); + expect(readlink(db, "/target/newlink")).toBe("keep.txt"); + }); + }); + + it.each(["file", "symlink"] as const)( + "replaces a blocking %s ancestor when directories arrive child-first", + async (kind) => { + await withDB(async (db) => { + if (kind === "file") { + await writeFile(db, "/workspace", "old", {}, () => 1); + } else { + symlink(db, "/missing", "/workspace", () => 1); + } + + await apply(db, [ + { kind: "dir", rev: 8, path: "/workspace/newdir", mode: 0o700, mtime: 8 }, + { kind: "dir", rev: 9, path: "/workspace", mode: 0o750, mtime: 9 }, + ]); + + expect(resolveInode(db, "/workspace", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o750, + }); + expect(resolveInode(db, "/workspace/newdir", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + }); + }); + }, + ); + + it("keeps a blocking ancestor that protects a read-only mount", async () => { + await withDB(async (db) => { + mkdir(db, "/target/r2", { recursive: true }, () => 1); + symlink(db, "/target", "/workspace", () => 2); + db.run( + "INSERT INTO _vfs_mounts (root, kind, indexed, mode) VALUES (?, ?, 1, 'read-only')", + "/workspace/r2", + "test", + ); + invalidateReadOnlyMountCache(db); + + const result = await apply(db, [ + { + kind: "file", + rev: 4, + path: "/workspace/newdir/file.txt", + mode: 0o640, + mtime: 4, + size: 0, + chunks: [], + }, + ]); + + expect(result.applied).toBe(0); + expect(result.skipped).toEqual([ + { + path: "/workspace/newdir/file.txt", + mountRoot: "/workspace/r2", + op: "write", + reason: "read-only", + }, + ]); + expect(resolveInode(db, "/workspace", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(db, "/workspace")).toBe("/target"); + expect(resolveInode(db, "/workspace/r2")?.type).toBe("dir"); + expect(resolveInode(db, "/workspace/newdir/file.txt")).toBeNull(); + }); + }); + + it("creates a missing ancestor of a read-only mount", async () => { + await withDB(async (db) => { + db.run( + "INSERT INTO _vfs_mounts (root, kind, indexed, mode) VALUES (?, ?, 1, 'read-only')", + "/workspace/r2", + "test", + ); + invalidateReadOnlyMountCache(db); + + const result = await apply(db, [ + { + kind: "file", + rev: 4, + path: "/workspace/file.txt", + mode: 0o640, + mtime: 4, + size: 0, + chunks: [], + }, + { kind: "dir", rev: 9, path: "/workspace", mode: 0o750, mtime: 9 }, + ]); + + expect(await readFile(db, "/workspace/file.txt", "utf8")).toBe(""); + expect(resolveInode(db, "/workspace", { followSymlinks: false })?.type).toBe("dir"); + expect(result.skipped).toContainEqual({ + path: "/workspace", + mountRoot: "/workspace/r2", + op: "write", + reason: "read-only", + }); + }); + }); + }); + + it("checks an existing parent chain in linear database reads", async () => { + await withDB(async (db) => { + const parentParts = Array.from({ length: 8 }, (_, index) => `d${index}`); + const parentPath = `/${parentParts.join("/")}`; + mkdir(db, parentPath, { recursive: true }, () => 1); + + const all = vi.spyOn(db, "all"); + try { + await applyChanges( + db, + [ + { + kind: "file", + rev: 10, + path: `${parentPath}/file.txt`, + mode: 0o640, + mtime: 10, + size: 0, + chunks: [], + }, + ], + new Map(), + ); + + const childLookups = all.mock.calls.filter( + ([query]) => + query === "SELECT child_inode FROM vfs_dirents WHERE parent_inode = ? AND name = ?", + ); + expect(childLookups.length).toBeLessThanOrEqual(2 * (parentParts.length + 1)); + } finally { + all.mockRestore(); + } + }); + }); + it("links staged chunks without reading payload bytes", async () => { const content = `${"a".repeat(CHUNK_SIZE)}b`; await withTwoDBs( diff --git a/packages/dofs/src/sync/apply.ts b/packages/dofs/src/sync/apply.ts index b9df856c..a4dfdbaf 100644 --- a/packages/dofs/src/sync/apply.ts +++ b/packages/dofs/src/sync/apply.ts @@ -1,6 +1,6 @@ -import { mkdir } from "../fs/mkdir.js"; +import { mkdir, mkdirForSyncParents } from "../fs/mkdir.js"; import { readOnlyRootFor } from "../fs/mount-guard.js"; -import { resolveInode } from "../fs/resolve.js"; +import { resolveInode, resolveInodeWithoutSymlinks } from "../fs/resolve.js"; import { invalidateResolveSubtree } from "../fs/resolveCache.js"; import { rm } from "../fs/rm.js"; import { symlink } from "../fs/symlink.js"; @@ -12,6 +12,7 @@ import type { Database } from "../storage.js"; import { stageBlob } from "./blobs.js"; import type { ChangeEntry } from "./changes.js"; import { computeManifestHash } from "./manifests.js"; +import { pathOf } from "./paths.js"; // One container-side change that landed under a read-only mount and // was therefore skipped rather than applied. Callers (the workspace @@ -210,6 +211,47 @@ function applyDirectoryEntry(db: Database, entry: Extract mtime); + return { path: canonical }; + } + if (ancestor.type === "dir") continue; + const blockingRoot = readOnlyRootFor(db, ancestorPath); + if (blockingRoot !== undefined) return { path: canonical, blockingRoot }; + removeInodeTreeAtPath(db, ancestorPath, ancestor.inode, ancestor.type); + mkdirForSyncParents(db, parentPath, { recursive: true }, () => mtime); + return { path: canonical }; + } + return { path: canonical }; +} + // Drive a ChangeEntry stream against `db`, batching writes so peak // memory stays bounded and a crash mid-apply leaves the DB in a // consistent state. Each batch runs inside a single transactionSync @@ -276,23 +318,52 @@ export async function applyChanges( continue; } if (entry.kind === "dir") { - applyDirectoryEntry(db, entry); + const parentResult = ensureParentDirectories(db, entry.path, entry.mtime); + if (parentResult.blockingRoot !== undefined) { + skipped.push({ + path: entry.path, + mountRoot: parentResult.blockingRoot, + op: "write", + reason: "read-only", + }); + continue; + } + applyDirectoryEntry(db, { ...entry, path: parentResult.path }); applied++; pathsInBatch++; if (pathsInBatch >= maxPaths) flush(); continue; } if (entry.kind === "symlink") { - removeReplaceableFinalEntry(db, entry.path, "symlink"); - symlink(db, entry.target, entry.path, () => entry.mtime); + const parentResult = ensureParentDirectories(db, entry.path, entry.mtime); + if (parentResult.blockingRoot !== undefined) { + skipped.push({ + path: entry.path, + mountRoot: parentResult.blockingRoot, + op: "write", + reason: "read-only", + }); + continue; + } + removeReplaceableFinalEntry(db, parentResult.path, "symlink"); + symlink(db, entry.target, parentResult.path, () => entry.mtime); applied++; pathsInBatch++; if (pathsInBatch >= maxPaths) flush(); continue; } - const total = applyFileEntry(db, entry, objects); + const fileResult = applyFileEntry(db, entry, objects); + if (fileResult.blockingRoot !== undefined) { + skipped.push({ + path: entry.path, + mountRoot: fileResult.blockingRoot, + op: "write", + reason: "read-only", + }); + continue; + } applied++; - bytesInBatch += total; + bytesInBatch += fileResult.total; pathsInBatch++; if (bytesInBatch >= maxBytes || pathsInBatch >= maxPaths) flush(); } @@ -367,23 +438,52 @@ export function applyChangesSync( continue; } if (entry.kind === "dir") { - applyDirectoryEntry(db, entry); + const parentResult = ensureParentDirectories(db, entry.path, entry.mtime); + if (parentResult.blockingRoot !== undefined) { + skipped.push({ + path: entry.path, + mountRoot: parentResult.blockingRoot, + op: "write", + reason: "read-only", + }); + continue; + } + applyDirectoryEntry(db, { ...entry, path: parentResult.path }); applied++; pathsInBatch++; if (pathsInBatch >= maxPaths) flush(); continue; } if (entry.kind === "symlink") { - removeReplaceableFinalEntry(db, entry.path, "symlink"); - symlink(db, entry.target, entry.path, () => entry.mtime); + const parentResult = ensureParentDirectories(db, entry.path, entry.mtime); + if (parentResult.blockingRoot !== undefined) { + skipped.push({ + path: entry.path, + mountRoot: parentResult.blockingRoot, + op: "write", + reason: "read-only", + }); + continue; + } + removeReplaceableFinalEntry(db, parentResult.path, "symlink"); + symlink(db, entry.target, parentResult.path, () => entry.mtime); applied++; pathsInBatch++; if (pathsInBatch >= maxPaths) flush(); continue; } - const total = applyFileEntry(db, entry, objects); + const fileResult = applyFileEntry(db, entry, objects); + if (fileResult.blockingRoot !== undefined) { + skipped.push({ + path: entry.path, + mountRoot: fileResult.blockingRoot, + op: "write", + reason: "read-only", + }); + continue; + } applied++; - bytesInBatch += total; + bytesInBatch += fileResult.total; pathsInBatch++; if (bytesInBatch >= maxBytes || pathsInBatch >= maxPaths) flush(); } @@ -409,7 +509,7 @@ function applyFileEntry( db: Database, entry: Extract, objects: Map, -): number { +): { total: number; blockingRoot?: string } { assertChunkWindows(entry.chunks, entry.path); let total = 0; for (const c of entry.chunks) { @@ -427,10 +527,14 @@ function applyFileEntry( } assertChunkSize(staged, c.size, c.hash, entry.path); } - removeReplaceableFinalEntry(db, entry.path, "file"); - const { parts, path: canonical } = canonicalizePath(entry.path); + const parentResult = ensureParentDirectories(db, entry.path, entry.mtime); + if (parentResult.blockingRoot !== undefined) { + return { total, blockingRoot: parentResult.blockingRoot }; + } + removeReplaceableFinalEntry(db, parentResult.path, "file"); + const { parts, path: canonical } = canonicalizePath(parentResult.path); linkStagedChunksSync(db, canonical, parts, entry.chunks, { mode: entry.mode }, entry.mtime); - return total; + return { total }; } // Return a staged chunk's size without loading its payload bytes.