diff --git a/docs/02_sync_protocol.md b/docs/02_sync_protocol.md index 1dd6d99a..4cc1fe19 100644 --- a/docs/02_sync_protocol.md +++ b/docs/02_sync_protocol.md @@ -38,7 +38,12 @@ A typical `exec()` round-trip: 1. **Push.** The DO streams every `ChangeEntry` with a higher revision than the container has seen, **coalesced to one entry per path** (the latest state wins — five rewrites of the same - path between execs cost one entry on the wire, not five). Bytes + path between execs cost one entry on the wire, not five). A + hardlinked inode carries several names: `coalesceChanges` emits + one entry **per name**, so every path materialises on the + receiver. Hardlink identity is not preserved across the wire — + each name becomes an independent file with the same content, not + a shared inode. Bytes are not inline; entries carry chunk hashes only. The **sender** (the DO) calls `remote.hasObjects(...)` on the referenced hashes and follows up with `remote.pushObjects(missing)` for the subset @@ -54,10 +59,10 @@ A typical `exec()` round-trip: [06. Mount Interface](./06_mount_interface.md). 3. **Exec.** The command runs. FUSE writes are captured by the in-container VFS as they happen, each stamped with a fresh revision. -4. **Fetch.** The DO calls `fetchChanges({ sinceRev: fetchRev })`. The - container streams `ChangeEntry` records — one per touched path, - per-file entries carrying `chunks: (hash, size)[]`. No bytes - inline. +4. **Fetch.** The DO calls `fetchChanges({ after: fetchCursor })`. + The container streams `ChangeEntry` records after that `(rev, path)` + cursor — one per touched path, per-file entries carrying + `chunks: (hash, size)[]`. No bytes inline. 5. **Diff.** The DO reads up to `PULL_BATCH_SIZE` (256) entries from the stream, unions the chunk hashes referenced by that batch, probes its own `vfs_blobs` for which it already has, and calls @@ -69,13 +74,13 @@ A typical `exec()` round-trip: `transactionSync` inside `writeFile`/`mkdir`/`rm`/`symlink` is the real durability boundary. The driver then loops back to step 5 for the next batch. - `fetchRev` is advanced **per committed batch** to the max `rev` - any entry in that batch carried. `coalesceChanges` emits entries - in ascending rev order so this checkpoint is safe — everything - below `batchMaxRev` has been applied. A crash mid-pull resumes - from the last per-batch advance, so re-fetched work is bounded - by `PULL_BATCH_SIZE` (256) entries, not the whole stream. The - receiver's `alreadyApplied` check inside `applyChanges` still + The fetch cursor is advanced **per committed batch** to the last + streamed entry's `(rev, path)`. `coalesceChanges` emits entries in + ascending `rev`, then ascending `path`, so this checkpoint is safe + even when one rev contains more than one batch. A crash mid-pull + resumes from the last per-batch advance, so re-fetched work is + bounded by `PULL_BATCH_SIZE` (256) entries, not the whole stream. + The receiver's `alreadyApplied` check inside `applyChanges` still drops already-applied entries on the floor so re-apply is idempotent and cheap. @@ -83,6 +88,92 @@ A typical `exec()` round-trip: step 1 is "this single change", steps 3–6 are skipped. `workspace.push()` runs step 1 on demand; `workspace.pull()` runs steps 4–6. +Renames are local inode moves, but the sync wire has no rename opcode. +The wire stays final-state based — live entries plus tombstones — so +apply remains idempotent and does not need operation-order replay. The +cost is that a directory rename stamps every moved inode with one new +revision and records tombstones for the old paths in one synchronous +transaction. Large directory renames are therefore O(subtree) in local +writes and wire entries, with no separate cap beyond the caller's own +workload. Parent directory mtimes are not changed by rename, which +differs from POSIX `rename(2)` but keeps parent directory metadata out +of content sync. + +Directory entries carry mode and mtime. New directories are created +with the incoming mtime, but idempotence for an existing directory is +mode-only. That keeps mtime drift on matching directories from +becoming sync traffic. + +When an upstream file, directory, or symlink lands where the receiver +has a different node type, the receiver removes the local node tree and +applies the upstream entry. This is last-writer-wins conflict handling: +it converges the tree, but local-only children under the conflicting +path are discarded without separate tombstones. + +## Alternatives considered + +Representing a rename as a full-subtree restamp produces one wire entry +per subtree item at a single revision. Two cheaper encodings were +considered and rejected. + +### A rename opcode + +A dedicated `rename` entry carrying `{ fromPath, toPath, inode }` would +collapse a directory move to one wire row. It was rejected because it is +an operation, while the rest of the protocol is state-based: +`materialiseChange` resolves each entry to the path's current state at +fetch time, the receiver reconciles against its own live state, and +`alreadyApplied` makes re-apply idempotent without ordered replay. + +An opcode breaks that model in three ways. It is relative to the +receiver's prior state: `relink(from -> to)` is meaningless to a peer +that never held `from`, so a cold-start peer pulling from rev 0 has +nothing to relink. The pull path is deliberately receiver-history +agnostic: the producer answers "changes after cursor X" by +materialising current state and knows nothing about what a given +receiver has seen, so it cannot decide when an opcode is safe to emit. +And making the opcode idempotent against final state requires +re-deriving the same state reconciliation the opcode was meant to avoid, +while still not solving cold start. The per-subtree cost is the price of +keeping one state-based representation that bootstraps, converges, and +replays under a single rule. + +### Chunking a rename across revisions + +A scalar fetch watermark can only resume at revision boundaries, so a +large rename at one rev forces a crash to replay the whole rev. One way +to bound that without a path cursor is to split a single rename across +many revisions, so a scalar watermark resumes at a chunk boundary. + +This was rejected because it weakens an invariant the protocol relies +on: `rev` is bumped atomically once per mutation (see +[03. Filesystem Schema](./03_filesystem_schema.md)), so every `rev` +value names one committed point in the mutation log. Chunking would +mint intermediate revisions that never committed as a whole, leaving +most `rev` values describing tree states that never existed. + +The `(rev, path)` fetch cursor avoids that. `path` is an orthogonal +second coordinate that records resume progress within a rev. A rename +still stamps exactly one revision across its subtree, while a crash can +resume mid-rev. Resumability is bought without minting phantom +revisions. + +**What a cursor guarantees.** A `(rev, path)` cursor is a *resume +point*, not a snapshot handle. `{rev, path: null}` means "every change +committed at or before `rev` has been offered to the receiver"; a +non-null `path` means "offered up to `path` within `rev`." It is +deliberately not a point-in-time snapshot read: `coalesceChanges` +materialises each path's *current* state at stream time, because the +store keeps no content history. A path that is rewritten or deleted +again after a snapshot opens has its live rev pushed past the +advertised `currentCursor`, so that entry is dropped from the current +stream and redelivered under a later cursor. The receiver's tree at +cursor `{5, null}` therefore need not byte-match the rev-5 snapshot for +a path that raced ahead — but convergence holds, because the rev that +caused the drop is greater than `currentCursor.rev`, so the next pull +re-scans and delivers the path's then-current state. The cursor never +advances past the rev that would redeliver an omitted path. + ### Chunking Files are split at a fixed `CHUNK_SIZE` (512 KiB). Chunk boundaries are @@ -105,10 +196,10 @@ one name. | Watermark | Owner | Meaning | | --- | --- | --- | | `pushRev` | DO | Last DO-side `rev` successfully pushed to the container. | -| `fetchRev` | DO | Last container-side `rev` the DO has fetched. | +| `fetchCursor` | DO | Last container-side cursor the DO has fetched; `path = null` means every change committed at or before that rev has been offered (a resume point, not a point-in-time snapshot — see above). | | `currentRev` | DO | Latest `rev` stamped on a DO-side mutation. | | `currentRev` | Container | Latest `rev` stamped on a container-side mutation. | -| `appliedPushRev` | Container | Largest DO `rev` the container has fully applied. Echoed on every **push** response. | +| `appliedPushCursor` | Container | DO-side cursor the container has applied. Echoed on every **push** and **fetchChanges** response. | The DO watermarks live in the `_vfs_watermark` table so they survive DO restarts. The container's watermarks live in the same `Database` @@ -122,15 +213,14 @@ fresh receiver). ### Cross-side invariant After every successful `push` **and** every `fetchChanges`, the -response carries the receiver's current `appliedPushRev` (the -largest `senderRev` it has fully applied). The DO asserts -`appliedPushRev >= pushRev` before continuing. The two sides never -share a single clock, but echoing the largest applied rev makes the -"receiver is caught up with our pushes" invariant inspectable on -the wire instead of load-bearing in-process state. A regression in -the post-apply `pushRev` advancement path (see step 1 above) trips -the assertion on the next push or pull rather than corrupting data -silently. +response carries the receiver's current `appliedPushCursor`. The DO +asserts that cursor covers its local `{ rev: pushRev, path: null }` +before continuing. The two sides never share a single clock, but +echoing the applied cursor makes the "receiver is caught up with our +pushes" invariant inspectable on the wire instead of load-bearing +in-process state. A regression in the post-apply cursor advancement +path trips the assertion on the next push or pull rather than +corrupting data silently. ## Wire shape @@ -139,10 +229,14 @@ records, both probe with `hasObjects`, both transfer bytes by hash. Naming follows git's vocabulary — the DO *pushes* entries and objects to the container, and *fetches* entries and objects back. +The DO and `wsd` are deployed as a matched pair. The protocol has no +version negotiation, so changes to request or response shapes are hard +wire breaks and require lockstep rollout. + | RPC | Direction | Returns | Notes | | --- | --- | --- | --- | -| `push({ senderRev, changes })` | DO → container | `{ rev, appliedPushRev }` | Streams a coalesced batch of `ChangeEntry` via the `changes` `ReadableStream`. The sender then calls `hasObjects` on the referenced hashes and follows up with `pushObjects` for the missing subset. See the `senderRev` branches below. | -| `fetchChanges({ sinceRev?, ignore? })` | container → DO | `Promise<{ currentRev, appliedPushRev, stream: ReadableStream }>` | Streams one entry per touched path. For files, `chunks: (hash, size)[]` (no bytes inline); for dirs, metadata; for deletes, a tombstone. `currentRev` is the receiver's rev at stream open; the puller advances `fetchRev` no further than this. `appliedPushRev` carries the cross-side invariant check on the pull path. | +| `push({ senderRev, changes })` | DO → container | `{ rev, appliedPushCursor }` | Streams a coalesced batch of `ChangeEntry` via the `changes` `ReadableStream`. The sender then calls `hasObjects` on the referenced hashes and follows up with `pushObjects` for the missing subset. See the `senderRev` branches below. | +| `fetchChanges({ after?, ignore? })` | container → DO | `Promise<{ currentCursor, appliedPushCursor, stream: ReadableStream }>` | Streams one entry per touched path after `after`, ordered by `rev` then `path`. For files, `chunks: (hash, size)[]` (no bytes inline); for dirs, metadata; for deletes, a tombstone. `currentCursor` is `{ rev: currentRev, path: null }` at stream open; the puller writes it after a clean drain. `appliedPushCursor` carries the cross-side invariant check on the pull path. | | `hasObjects(hashes[])` | sender probes receiver | `Uint8Array[]` | Returns the subset of the input the receiver already holds. The git `have` line, batched. | | `fetchObjects(hashes[])` | container → DO | `ReadableStream<{ hash, bytes }>` | Streams chunk bytes by hash. The git `want`/pack response on the fetch path. | | `pushObjects(objects)` | DO → container | `void` | Streams chunk bytes by hash. The push-direction mirror of `fetchObjects`. | @@ -155,7 +249,8 @@ load-test rationale): - **`senderRev > 0` — sync peer.** A DO calling its container counterpart (or vice versa). The receiver applies the batch as `upstream`, - advances its own `fetchRev` to `senderRev`, and on the *sender's* + advances its own fetch cursor to `{ rev: senderRev, path: null }`, + and on the *sender's* side `pushRev` is advanced past the rev just shipped (gated on no interleaved local writes — see step 1 above). - **`senderRev === 0` — external writer / fresh receiver.** Used by @@ -175,7 +270,7 @@ edited file) shows up exactly once on the wire. See - **Container restart mid-exec.** The DO's connection detects the closed WebSocket and self-destructs. The next call transparently rebuilds against the still-running `wsd` (or restarts it if needed). - `pushRev` and `fetchRev` mean the catch-up is incremental, modulo + `pushRev` and the fetch cursor mean the catch-up is incremental, modulo whatever the container's deployment chose for its DB lifetime. - **Container crash mid-apply.** `push` is atomic from the DO's perspective on the receiver: the server wraps the whole batch in a @@ -187,11 +282,12 @@ edited file) shows up exactly once on the wire. See applied so far; the receiver never sees a partial push. The pull path keeps the per-mutation model because the streaming batches can't hold a synchronous transaction across network I/O. -- **DO restart mid-pull.** `fetchRev` advances per committed batch - to the max `rev` the batch carried, so a restart mid-pull resumes - from the last per-batch checkpoint. Wasted work is bounded by - `PULL_BATCH_SIZE` entries (256), not the whole stream. End state is - correct either way — apply is idempotent. +- **DO restart mid-pull.** The fetch cursor advances per committed + batch to the last entry's `(rev, path)`, so a restart mid-pull + resumes from the last per-batch checkpoint, including within a + single large rev. Wasted work is bounded by `PULL_BATCH_SIZE` + entries (256), not the whole stream. End state is correct either + way — apply is idempotent. - **DO restart.** Watermarks are persisted, so the new DO instance picks up where the old one left off. The container keeps `wsd` alive across the gap. diff --git a/docs/03_filesystem_schema.md b/docs/03_filesystem_schema.md index f61a458d..9df4dfaa 100644 --- a/docs/03_filesystem_schema.md +++ b/docs/03_filesystem_schema.md @@ -14,8 +14,10 @@ ultimately hits one of these tables. All tables are prefixed with collide with application-owned tables in the same DO storage. Paths are resolved through an inode-style indirection (`vfs_dirents` -→ `vfs_nodes`), so renames are O(1) regardless of subtree size and -hardlinks fall out for free. +→ `vfs_nodes`), so the local namespace move in a rename is O(1) and +hardlinks fall out for free. Directory rename sync still walks the +moved subtree because the wire represents the move as live entries at +the new paths plus tombstones at the old paths. ## Tables @@ -68,10 +70,9 @@ positional read primitive can read it directly instead of running `SUM(size) FROM vfs_chunks` on every call. Every write path stamps it alongside `mode`/`mtime`/`rev`. -The `vfs_nodes_by_rev` index supports `coalesceChanges`'s -`WHERE rev > sinceRev` scan over live inodes, which the sync protocol -calls once per pull to enumerate everything modified since the last -fetch watermark. +The `vfs_nodes_by_rev` index supports `coalesceChanges`'s cursor scan +over live inodes, which the sync protocol calls once per pull to +enumerate everything modified after the last fetch cursor. There is no `ignored` column: ignored paths are entirely invisible to the DO-side filesystem API (see @@ -216,6 +217,23 @@ Stores `pushRev` and `fetchRev` (see [02. Sync Protocol](./02_sync_protocol.md#watermarks)). Survives DO restarts so reconnects resume cleanly. +### `_vfs_fetch_cursor` — fetch path tie-breaker + +```sql +CREATE TABLE _vfs_fetch_cursor ( + k TEXT PRIMARY KEY CHECK(k = 'fetch'), + path TEXT +); +``` + +Stores the path component for the fetch cursor. The numeric rev remains +in `_vfs_watermark.fetchRev`; `path = NULL` means every change committed +at or before that rev has been offered to the receiver, and a non-null +`path` resumes within `fetchRev`. The cursor is a resume point, not a +point-in-time snapshot: a path rewritten after a fetch opens is deferred +to a later cursor rather than frozen at `fetchRev`. See +[02. Sync Protocol](./02_sync_protocol.md) for the full contract. + ### `_vfs_mounts` — mount index state ```sql diff --git a/docs/06_mount_interface.md b/docs/06_mount_interface.md index 3f361276..3c911fb9 100644 --- a/docs/06_mount_interface.md +++ b/docs/06_mount_interface.md @@ -284,7 +284,7 @@ This is deliberate, not an oversight: - R2 and GitHub have no monotonic rev clock. Treating them as peers would force per-tick polling and a diff against a remembered snapshot. -- The protocol's invariants — `appliedPushRev`, watermark +- The protocol's invariants — `appliedPushCursor`, watermark reconciliation, tombstones — assume one peer. They don't generalize to N peers without a real CRDT / LWW story. - The "container always wins" conflict policy is a deliberate diff --git a/docs/08_capnweb_interface.md b/docs/08_capnweb_interface.md index 00771a79..b28aa5e8 100644 --- a/docs/08_capnweb_interface.md +++ b/docs/08_capnweb_interface.md @@ -58,34 +58,42 @@ interface SyncRPC { // not inline: the DO sends ChangeEntry records with chunk hashes, // the receiver calls back via hasObjects / pushObjects for the // missing subset. Returns the receiver's new rev plus the - // appliedPushRev it stamped for this batch. + // appliedPushCursor it stamped for this batch. push(input: { senderRev: number; changes: ReadableStream; - }): Promise<{ rev: number; appliedPushRev: number }>; + }): Promise<{ + rev: number; + appliedPushCursor: { rev: number; path: string | null }; + }>; - // Container ← DO. Stream every ChangeEntry with rev > sinceRev, - // alongside the receiver's currentRev (cursor the puller advances - // fetchRev to) and appliedPushRev (cross-side invariant check on - // the pull path, mirroring the push response). Per-file entries - // carry (hash, size) chunk lists; no bytes inline. + // Container ← DO. Stream every ChangeEntry after `after`, ordered + // by rev then path. The cursor is a resume point, not a snapshot + // handle: path=null means every change committed at or before that + // rev has been offered; a cursor with path set resumes after that + // path inside the same rev. A path rewritten after the stream opens + // is deferred to a later cursor rather than frozen at this rev (see + // docs/02). `currentCursor` is the receiver's currentRev at stream + // open with path=null, and `appliedPushCursor` is the receiver's + // cursor for sender changes it has applied. Per-file entries carry + // (hash, size) chunk lists; no bytes inline. fetchChanges(input: { - sinceRev?: number; - ignore?: string[]; + after?: { rev: number; path: string | null }; + ignore?: string[]; }): Promise<{ - currentRev: number; - appliedPushRev: number; + currentCursor: { rev: number; path: string | null }; + appliedPushCursor: { rev: number; path: string | null }; stream: ReadableStream; }>; // Diagnostic surface for soak tests, dashboards, and the agent // when it wants to wait for the wire to drain. pushRev / - // fetchRev only move when the receiver is acting as a sync - // peer; otherwise they sit at 0. + // fetchCursor only move when the receiver is acting as a sync + // peer; otherwise they sit at 0 / { rev: 0, path: null }. watermarks(): Promise<{ currentRev: number; pushRev: number; - fetchRev: number; + fetchCursor: { rev: number; path: string | null }; }>; // Materialise a single path as a ChangeEntry without driving @@ -116,26 +124,32 @@ interface SyncRPC { } ``` +The durable object and `wsd` are deployed as a matched pair. This +interface has no version negotiation, so request and response shape +changes are hard wire breaks and require lockstep rollout. + `ChangeEntry` is defined in `packages/dofs/src/sync/changes.ts`. Schema column references match [03. Filesystem Schema](./03_filesystem_schema.md). #### Rev-0 baseline (no separate snapshot) There is no dedicated `snapshot()` RPC. A fresh DO with no watermark -calls `fetchChanges({ sinceRev: 0 })`, which streams every live entry -plus any tombstones the receiver has retained. Treating the baseline as -a degenerate fetch keeps the wire shape minimal: the same pull path -covers both cold-start replication and incremental catch-up. +calls `fetchChanges({ after: { rev: 0, path: null } })`, which streams +every live entry plus any tombstones the receiver has retained. +Treating the baseline as a degenerate fetch keeps the wire shape +minimal: the same pull path covers both cold-start replication and +incremental catch-up. #### Push semantics: peer vs external `push` distinguishes two callers via `senderRev`: - **`senderRev > 0` — sync peer.** The sender is replicating its own - log forward. The receiver advances `fetchRev` to `senderRev` once - the batch settles, echoes it back as `appliedPushRev`, and uses - that value to silence the loopback (the next `fetchChanges` from - the peer won't replay these entries back at it). + log forward. The receiver advances its fetch cursor to + `{ rev: senderRev, path: null }` once the batch settles, echoes it + back as `appliedPushCursor`, and uses that value to silence the + loopback (the next `fetchChanges` from the peer won't replay these + entries back at it). - **`senderRev === 0` — external orchestrator.** The sender doesn't have a rev space of its own (an agent, a CI script, a one-shot writer). The receiver applies the batch as ordinary local writes, @@ -199,19 +213,19 @@ ends: streaming `ChangeEntry` records, the container calls `hasObjects` on the chunk hashes referenced, the DO follows up with `pushObjects` (itself a stream) for the missing subset, the container applies the - batch and returns `{ rev, appliedPushRev }`. -- **Fetch (container → DO).** The DO calls `fetchChanges({ sinceRev })`, - which returns `{ currentRev, appliedPushRev, stream }` in one round- - trip. `currentRev` is the target watermark; `appliedPushRev` + batch and returns `{ rev, appliedPushCursor }`. +- **Fetch (container → DO).** The DO calls `fetchChanges({ after })`, + which returns `{ currentCursor, appliedPushCursor, stream }` in one round- + trip. `currentCursor` is the target cursor; `appliedPushCursor` carries the cross-side invariant (the puller asserts it covers - the local `pushRev` before draining). The DO then streams + `{ rev: pushRev, path: null }` before draining). The DO then streams `ChangeEntry` records, accumulates chunk hashes, calls `hasObjects` on itself (cheap, local) to find what it already has, then calls `fetchObjects` for the rest. | Aspect | Value | | --- | --- | -| Round-trips per fetch | 1 streaming `fetchChanges` (carries `currentRev` + `appliedPushRev` + entry stream) + 1 `hasObjects` per batch + 1 streaming `fetchObjects` per batch (only if any hashes are missing) | +| Round-trips per fetch | 1 streaming `fetchChanges` (carries `currentCursor` + `appliedPushCursor` + entry stream) + 1 `hasObjects` per batch + 1 streaming `fetchObjects` per batch (only if any hashes are missing) | | Round-trips per push | 1 streaming `push` (carries `senderRev`) + 1 `hasObjects` (server-driven) + 1 streaming `pushObjects` (only if any hashes are missing) | | Bytes inline in `ChangeEntry` | None — entries carry chunk hashes only | | Object transfer shape | `ReadableStream<{ hash, bytes }>` in both directions | diff --git a/docs/11_lifecycle.md b/docs/11_lifecycle.md index ecb990cf..55e0157c 100644 --- a/docs/11_lifecycle.md +++ b/docs/11_lifecycle.md @@ -44,7 +44,7 @@ The 1:1 mapping is load-bearing for several reasons: - The container's WebSocket peer is unambiguous — there is at most one capnweb session per DO at any time. -- The DO's persisted watermarks (`pushRev`, `fetchRev`) speak to a +- The DO's persisted watermarks (`pushRev`, fetch cursor) speak to a single counterparty. - Hibernation enablement (see below) becomes tractable because the DO doesn't have to multiplex multiple WS peers. @@ -79,9 +79,9 @@ an incarnation boundary. What survives is: - The SQLite store backing `Workspace.fs` — every committed `writeFile`, `mkdir`, `rm`, `symlink` is durable. -- The `_vfs_watermark` row, which holds `pushRev` (last DO-side rev - successfully pushed to the container) and `fetchRev` (last - container-side rev the DO has fetched). These are written via the +- The sync watermark rows, which hold `pushRev` (last DO-side rev + successfully pushed to the container) and the fetch cursor (last + container-side cursor the DO has fetched). These are written via the same SQLite transaction as the data they describe, so they cannot drift out of sync with the store. @@ -186,14 +186,15 @@ Because the rev counters drive every operation, a torn RPC is safe to retry against a fresh session. Specifically: - **`pushOnce`.** `pushRev` is written only after - `assertAppliedPushRev` succeeds. A torn push leaves `pushRev` at + `assertAppliedPushCursor` succeeds. A torn push leaves `pushRev` at the previous value; the next push replays the same batch. `applyChanges` on the receiver is idempotent. -- **`pullOnce`.** `fetchRev` advances per committed batch to the max - `rev` the batch carried. A torn pull leaves `fetchRev` at the last - per-batch checkpoint; the next pull re-fetches only the entries - past that point. `applyChanges`'s `alreadyApplied` check drops any - duplicates the resume happens to overlap with. +- **`pullOnce`.** The fetch cursor advances per committed batch to the + last streamed entry's `(rev, path)`. A torn pull leaves the cursor at + the last per-batch checkpoint; the next pull re-fetches only entries + past that point, including within the same rev. `applyChanges`'s + `alreadyApplied` check drops any duplicates the resume happens to + overlap with. - **`exec.events`.** Each event carries a monotonic `seq` per exec id. The client reattaches via `getExec({ id, after: seq })`. @@ -334,11 +335,13 @@ Two things have to change for capnweb + hibernation to work: wake as a fresh session. The peer must retry any in-flight RPC. This is the same semantics as a transport reset, which the protocol already handles via the rev cursors. - - **Sync streams: nothing to store.** `pushRev` and `fetchRev` - are already written to `_vfs_watermark` inside the same SQLite - transaction as the data they describe. On wake, the next - `pushOnce` / `pullOnce` reads them from durable storage and - resumes. No attachment write is required. + - **Sync streams: nothing to store.** `pushRev` is written to + SQLite with the pushed data it describes. The durable fetch + cursor is written after each committed pull batch, not in the + same transaction as the data apply. On wake, the next `pushOnce` + / `pullOnce` reads the durable counters and resumes; any overlap + is dropped by the idempotent apply path. No attachment write is + required. - **Exec streams: store `{ [id]: seq }` per in-flight exec.** The `WorkspaceShell` driver inside the DO is the only place that knows where the consumer got to in the event stream. diff --git a/packages/dofs/README.md b/packages/dofs/README.md index c9fb8470..737c9e0b 100644 --- a/packages/dofs/README.md +++ b/packages/dofs/README.md @@ -17,7 +17,7 @@ This package exposes a JavaScript module, not a CLI. It bundles three layers tha - A `Database` wrapper around Durable Object SQL storage plus `initializeSchema` for the `vfs_*` tables. - Filesystem primitives under `src/fs/*` (`mkdir`, `writeFile`, `readFile`, `rm`, `readdir`, `stat`, `lstat`, `chmod`, `find`, `ls`, `grep`, `symlink`, `readlink`, `gc`, `watch`) operating on a `Database`. - `SQLiteWorkspaceProvider`, a `@platformatic/vfs` adapter that composes those primitives into a node-shaped filesystem (fd table, positional `readSync`/`writeSync`, `watchSync`, symlinks). This is what `wsd` mounts via FUSE. -- Sync protocol building blocks operating on the same `Database`: `applyChanges`, `stageBlob`, `materialiseChange`, `coalesceChanges`, `fetchChanges`, `fetchObjects`, `hasObjects`, `pushObjects`, `buildManifest`, `currentRev`, `readWatermark`/`writeWatermark`, `assertAppliedPushRev`, and `DEFAULT_IGNORE`/`isIgnored`. The wire wiring lives in `@cloudflare/workspace-rpc`. +- Sync protocol building blocks operating on the same `Database`: `applyChanges`, `stageBlob`, `materialiseChange`, `coalesceChanges`, `fetchChanges`, `fetchObjects`, `hasObjects`, `pushObjects`, `buildManifest`, `currentRev`, `compareChangeCursors`, `readWatermark`/`writeWatermark`, `assertAppliedPushCursor`, and `DEFAULT_IGNORE`/`isIgnored`. The wire wiring lives in `@cloudflare/workspace-rpc`. Minimal DO-side usage — initialize the schema; the `Database` becomes the handle every other helper takes: diff --git a/packages/dofs/src/fs/mount-guard.test.ts b/packages/dofs/src/fs/mount-guard.test.ts index 03bfa575..cc729a48 100644 --- a/packages/dofs/src/fs/mount-guard.test.ts +++ b/packages/dofs/src/fs/mount-guard.test.ts @@ -7,7 +7,10 @@ import { getReadOnlyMountRoots, invalidateReadOnlyMountCache, } from "./mount-guard.js"; +import { rename } from "./rename.js"; +import { resolveInode } from "./resolve.js"; import { rm } from "./rm.js"; +import { symlink } from "./symlink.js"; import { withDB } from "./with-db.js"; import { writeFile, writeFileSync } from "./writeFile.js"; @@ -240,4 +243,76 @@ describe("rm under a read-only mount", () => { expect(() => rm(db, "/workspace/rw/hi.txt", {})).not.toThrow(); }); }); + + it("rejects rm through a symlinked parent that resolves into a read-only mount", async () => { + await withDB((db) => { + mkdir(db, "/mnt", { recursive: true }, () => 0); + writeFileSync(db, "/mnt/file.txt", new Uint8Array([1]), {}, () => 0); + symlink(db, "/mnt", "/link", () => 0); + stageMount(db, "/mnt", "read-only"); + const before = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + expect(() => rm(db, "/link/file.txt", {})).toThrowError( + expect.objectContaining({ code: "EROFS" }), + ); + + expect(resolveInode(db, "/mnt/file.txt")).not.toBeNull(); + expect(db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'")).toBe(before); + }); + }); + + it("rejects recursive rm of a directory inside a read-only mount via a symlinked parent", async () => { + await withDB((db) => { + mkdir(db, "/mnt/dir", { recursive: true }, () => 0); + writeFileSync(db, "/mnt/dir/file.txt", new Uint8Array([1]), {}, () => 0); + symlink(db, "/mnt", "/link", () => 0); + stageMount(db, "/mnt", "read-only"); + const before = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + expect(() => rm(db, "/link/dir", { recursive: true })).toThrowError( + expect.objectContaining({ code: "EROFS" }), + ); + + expect(resolveInode(db, "/mnt/dir/file.txt")).not.toBeNull(); + expect(db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'")).toBe(before); + }); + }); +}); + +describe("rename under a read-only mount", () => { + it("rejects rename from a symlinked parent that resolves into a read-only mount", async () => { + await withDB((db) => { + mkdir(db, "/mnt", { recursive: true }, () => 0); + writeFileSync(db, "/mnt/file.txt", new Uint8Array([1]), {}, () => 0); + symlink(db, "/mnt", "/link", () => 0); + stageMount(db, "/mnt", "read-only"); + const before = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + expect(() => rename(db, "/link/file.txt", "/moved.txt")).toThrowError( + expect.objectContaining({ code: "EROFS" }), + ); + + expect(resolveInode(db, "/mnt/file.txt")).not.toBeNull(); + expect(resolveInode(db, "/moved.txt")).toBeNull(); + expect(db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'")).toBe(before); + }); + }); + + it("rejects rename to a symlinked parent that resolves into a read-only mount", async () => { + await withDB((db) => { + mkdir(db, "/mnt", { recursive: true }, () => 0); + writeFileSync(db, "/src.txt", new Uint8Array([1]), {}, () => 0); + symlink(db, "/mnt", "/link", () => 0); + stageMount(db, "/mnt", "read-only"); + const before = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + expect(() => rename(db, "/src.txt", "/link/file.txt")).toThrowError( + expect.objectContaining({ code: "EROFS" }), + ); + + expect(resolveInode(db, "/src.txt")).not.toBeNull(); + expect(resolveInode(db, "/mnt/file.txt")).toBeNull(); + expect(db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'")).toBe(before); + }); + }); }); diff --git a/packages/dofs/src/fs/rename.ts b/packages/dofs/src/fs/rename.ts new file mode 100644 index 00000000..d9398a01 --- /dev/null +++ b/packages/dofs/src/fs/rename.ts @@ -0,0 +1,211 @@ +import { createWorkspaceError } from "../errors.js"; +import { canonicalizePath } from "../path.js"; +import { incrementRev } from "../rev.js"; +import type { Database } from "../storage.js"; +import { recordDelete } from "../sync/changes.js"; +import { pathOf } from "../sync/paths.js"; +import { assertNotReadOnly } from "./mount-guard.js"; +import { resolveInode } from "./resolve.js"; +import { unlinkDirent } from "./unlink.js"; + +interface DirChild { + name: string; + child_inode: number; + type: NodeType; +} + +interface SubtreeEntry { + path: string; + inode: number; + type: NodeType; +} + +type NodeType = "file" | "dir" | "symlink"; + +export function rename(db: Database, oldPath: string, newPath: string): void { + const { path: oldCanonical } = canonicalizePath(oldPath); + const { parts: newParts, path: newCanonical } = canonicalizePath(newPath); + + if (oldCanonical === "/") { + throw createWorkspaceError("EINVAL", "cannot rename root", oldCanonical); + } + if (newParts.length === 0) { + throw createWorkspaceError("EINVAL", "cannot rename onto root", newCanonical); + } + + assertNotReadOnly(db, oldCanonical); + assertNotReadOnly(db, newCanonical); + + db.transactionSync(() => { + const source = resolveInode(db, oldCanonical, { followSymlinks: false }); + if (source === null) { + throw createWorkspaceError("ENOENT", `no such path: ${oldCanonical}`, oldCanonical); + } + + // Resolve the source's real parent dirent. The parent path is + // resolved with symlinks followed so a request through a symlinked + // directory lands on the real container; the inode is then + // identified by (parent_inode, name) rather than by child_inode so + // a hardlinked source touches only the requested name. + const { parts: oldParts } = canonicalizePath(oldCanonical); + const oldName = oldParts[oldParts.length - 1]; + const oldParentPath = oldParts.length === 1 ? "/" : `/${oldParts.slice(0, -1).join("/")}`; + const oldParent = resolveInode(db, oldParentPath); + if (oldParent === null || oldParent.type !== "dir") { + throw createWorkspaceError("ENOENT", `no such path: ${oldCanonical}`, oldCanonical); + } + const oldParentReal = pathOf(db, oldParent.inode); + if (oldParentReal === null) { + throw createWorkspaceError("ENOENT", `no such path: ${oldCanonical}`, oldCanonical); + } + const oldRealPath = oldParentReal === "/" ? `/${oldName}` : `${oldParentReal}/${oldName}`; + assertNotReadOnly(db, oldRealPath); + + if (oldCanonical === newCanonical) return; + + const newName = newParts[newParts.length - 1]; + const newParentPath = newParts.length === 1 ? "/" : `/${newParts.slice(0, -1).join("/")}`; + const newParent = resolveInode(db, newParentPath); + if (newParent === null || newParent.type !== "dir") { + throw createWorkspaceError( + "ENOENT", + `parent directory missing: ${newCanonical}`, + newCanonical, + ); + } + const newParentReal = pathOf(db, newParent.inode); + if (newParentReal === null) { + throw createWorkspaceError( + "ENOENT", + `parent directory missing: ${newCanonical}`, + newCanonical, + ); + } + const newRealPath = newParentReal === "/" ? `/${newName}` : `${newParentReal}/${newName}`; + assertNotReadOnly(db, newRealPath); + + // A rename whose source and destination resolve to the very same + // dirent (same real parent and name, e.g. through a symlinked path) + // is a true no-op: leave the tree and the change stream untouched. + // This is distinct from renaming one hardlink onto another, where + // the names differ and the source link must still be removed. + if (oldParent.inode === newParent.inode && oldName === newName) return; + + const existing = db.one<{ child_inode: number; type: "file" | "dir" | "symlink" }>( + `SELECT d.child_inode AS child_inode, n.type AS type + FROM vfs_dirents d + JOIN vfs_nodes n ON n.inode = d.child_inode + WHERE d.parent_inode = ? AND d.name = ?`, + newParent.inode, + newName, + ); + + const oldEntries = + source.type === "dir" + ? collectSubtree(db, source.inode, oldRealPath) + : [{ path: oldRealPath, inode: source.inode, type: source.type }]; + if (source.type === "dir") { + // Authoritative directory self-move guard. It tests the *resolved* + // destination parent inode against the source subtree, so it + // catches a symlinked destination that lands inside the source and + // allows one that resolves outside it. A textual prefix test on the + // unresolved path cannot do either and is intentionally absent. + assertDestinationParentOutsideSource(oldEntries, newParent.inode, oldRealPath, newCanonical); + } + + if (existing !== undefined) { + assertCompatibleOverwrite(source.type, existing.type, newCanonical); + if (existing.type === "dir") { + const childCount = db.scalar( + "SELECT COUNT(*) FROM vfs_dirents WHERE parent_inode = ?", + existing.child_inode, + ); + if ((childCount ?? 0) > 0) { + throw createWorkspaceError("ENOTEMPTY", `not empty: ${newCanonical}`, newCanonical); + } + } + // Displace only the destination name. The displaced inode may + // carry other hardlinks (or be the source inode itself), so reap + // its chunks and node row only once the final link disappears. + // Order matters: displace before unlinking the source so a + // hardlink-onto-hardlink rename never momentarily drops to zero + // links and reaps the inode it is about to re-point. + unlinkDirent(db, newParent.inode, newName, existing.child_inode, existing.type); + } + + // Unlink only the source name; a hardlinked source keeps its other + // names alive. + db.run("DELETE FROM vfs_dirents WHERE parent_inode = ? AND name = ?", oldParent.inode, oldName); + db.run( + "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", + newParent.inode, + newName, + source.inode, + ); + + const rev = incrementRev(db); + // Rename is represented on the wire as old-path tombstones plus + // live entries for the moved inode subtree, so stamp only that + // subtree with the shared rev. Parent directory mtimes are left + // unchanged on purpose; this diverges from POSIX rename(2), but + // avoids treating the old and new parents as content changes. + for (const entry of oldEntries) { + db.run("UPDATE vfs_nodes SET rev = ? WHERE inode = ?", rev, entry.inode); + recordDelete(db, rev, entry.path); + } + }); +} + +function assertCompatibleOverwrite( + sourceType: NodeType, + existingType: NodeType, + path: string, +): void { + if (sourceType === "dir" && existingType === "dir") return; + if (existingType === "dir") { + throw createWorkspaceError("EISDIR", `cannot overwrite directory: ${path}`, path); + } + if (sourceType === "dir") { + throw createWorkspaceError("ENOTDIR", `cannot overwrite non-directory: ${path}`, path); + } +} + +function assertDestinationParentOutsideSource( + oldEntries: SubtreeEntry[], + newParentInode: number, + oldCanonical: string, + newCanonical: string, +): void { + if (!oldEntries.some((entry) => entry.inode === newParentInode)) return; + + throw createWorkspaceError( + "EINVAL", + `cannot rename a directory into itself: ${oldCanonical}`, + newCanonical, + ); +} + +function collectSubtree(db: Database, rootInode: number, rootPath: string): SubtreeEntry[] { + const entries: SubtreeEntry[] = [{ path: rootPath, inode: rootInode, type: "dir" }]; + for (let idx = 0; idx < entries.length; idx++) { + const entry = entries[idx]; + if (entry.type !== "dir") continue; + const children = db.all( + `SELECT d.name AS name, d.child_inode AS child_inode, n.type AS type + FROM vfs_dirents d + JOIN vfs_nodes n ON n.inode = d.child_inode + WHERE d.parent_inode = ? + ORDER BY d.name`, + entry.inode, + ); + for (const child of children) { + const childPath = entry.path === "/" ? `/${child.name}` : `${entry.path}/${child.name}`; + entries.push({ + path: childPath, + inode: child.child_inode, + type: child.type, + }); + } + } + return entries; +} diff --git a/packages/dofs/src/fs/rm.test.ts b/packages/dofs/src/fs/rm.test.ts index 5ba45528..5acd002d 100644 --- a/packages/dofs/src/fs/rm.test.ts +++ b/packages/dofs/src/fs/rm.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import type { Database } from "../storage.js"; import { mkdir } from "./mkdir.js"; import { readdir } from "./readdir.js"; +import { readFile } from "./readFile.js"; import { resolveInode } from "./resolve.js"; import { rm } from "./rm.js"; import { symlink } from "./symlink.js"; @@ -41,6 +42,23 @@ describe("rm", () => { }); }); + it("records tombstones at the resolved path through intermediate symlinks", async () => { + await withDB(async (db) => { + mkdir(db, "/real", {}, () => 0); + await writeFile(db, "/real/file.txt", "content", {}, () => 0); + symlink(db, "/real", "/link", () => 0); + + rm(db, "/link/file.txt", {}); + + expect(listChanges(db)).toContainEqual( + expect.objectContaining({ path: "/real/file.txt", op: "delete" }), + ); + expect(listChanges(db)).not.toContainEqual( + expect.objectContaining({ path: "/link/file.txt", op: "delete" }), + ); + }); + }); + it("bumps rev once per call", async () => { await withDB(async (db) => { await writeFile(db, "/a.txt", "hi", {}, () => 0); @@ -86,6 +104,16 @@ describe("rm", () => { }); }); + it("removes a dangling symlink", async () => { + await withDB((db) => { + symlink(db, "/missing", "/dangling", () => 0); + + rm(db, "/dangling", {}); + + expect(resolveInode(db, "/dangling", { followSymlinks: false })).toBeNull(); + }); + }); + it("rejects ENOENT for a missing path", async () => { await withDB((db) => { expect(() => rm(db, "/missing", {})).toThrowError( @@ -143,6 +171,19 @@ describe("rm", () => { }); }); + it("recursive removes a symlink to a directory without deleting its target", async () => { + await withDB(async (db) => { + mkdir(db, "/target/sub", { recursive: true }, () => 0); + await writeFile(db, "/target/sub/file.txt", "content", {}, () => 0); + symlink(db, "/target", "/link", () => 0); + + rm(db, "/link", { recursive: true }); + + expect(resolveInode(db, "/link", { followSymlinks: false })).toBeNull(); + expect(await readFile(db, "/target/sub/file.txt", "utf8")).toBe("content"); + }); + }); + it("recursive records one tombstone per removed path", async () => { await withDB(async (db) => { mkdir(db, "/d", {}, () => 0); @@ -156,6 +197,25 @@ describe("rm", () => { }); }); + it("recursive records resolved subtree tombstones through intermediate symlinks", async () => { + await withDB(async (db) => { + mkdir(db, "/real/dir", { recursive: true }, () => 0); + await writeFile(db, "/real/dir/a", "x", {}, () => 0); + await writeFile(db, "/real/dir/b", "y", {}, () => 0); + symlink(db, "/real", "/link", () => 0); + + rm(db, "/link/dir", { recursive: true }); + + const paths = listChanges(db) + .map((r) => r.path) + .sort(); + expect(paths).toEqual(expect.arrayContaining(["/real/dir", "/real/dir/a", "/real/dir/b"])); + expect(paths).not.toEqual( + expect.arrayContaining(["/link/dir", "/link/dir/a", "/link/dir/b"]), + ); + }); + }); + it("recursive still bumps rev only once for the whole tree", async () => { await withDB(async (db) => { mkdir(db, "/d", {}, () => 0); diff --git a/packages/dofs/src/fs/rm.ts b/packages/dofs/src/fs/rm.ts index 1dc2bcdb..9ce423fa 100644 --- a/packages/dofs/src/fs/rm.ts +++ b/packages/dofs/src/fs/rm.ts @@ -3,8 +3,10 @@ import { canonicalizePath } from "../path.js"; import { incrementRev } from "../rev.js"; import type { Database } from "../storage.js"; import { recordDelete } from "../sync/changes.js"; +import { pathOf } from "../sync/paths.js"; import { assertNotReadOnly } from "./mount-guard.js"; import { resolveInode } from "./resolve.js"; +import { unlinkDirent } from "./unlink.js"; export interface RmOptions { recursive?: boolean; @@ -92,15 +94,34 @@ export function rm(db: Database, path: string, options: RmOptions): void { } } + // Resolve the entry's real path from its parent rather than from + // the inode: a hardlinked file has several names, and pathOf would + // pick an arbitrary one. Following symlinks on the parent lets a + // request through a symlinked directory land on the real container + // while still removing exactly the requested name. + const name = parts[parts.length - 1]; + const parentPath = parts.length === 1 ? "/" : `/${parts.slice(0, -1).join("/")}`; + const parent = resolveInode(db, parentPath); + if (parent === null || parent.type !== "dir") { + throw createWorkspaceError("ENOENT", `no such path: ${canonical}`, canonical); + } + const parentReal = pathOf(db, parent.inode); + if (parentReal === null) { + throw createWorkspaceError("ENOENT", `no such path: ${canonical}`, canonical); + } + const realPath = parentReal === "/" ? `/${name}` : `${parentReal}/${name}`; + assertNotReadOnly(db, realPath); + const rev = incrementRev(db); - if (node.type === "file" || !recursive) { + if (node.type !== "dir" || !recursive) { // Single entry removal — file, symlink, or empty directory. A // file inode may have multiple dirents (hardlinks), so remove // only the requested name and reap chunks/node after the final - // link disappears. - removeEntry(db, canonical, node.inode, node.type); - recordDelete(db, rev, canonical); + // link disappears. The tombstone is recorded at the resolved + // real path so sync sees the move-aware location. + removeEntry(db, realPath, node.inode, node.type); + recordDelete(db, rev, realPath); return; } @@ -108,7 +129,7 @@ export function rm(db: Database, path: string, options: RmOptions): void { // sees an empty parent by the time we get to it. File entries may // be hardlinked outside this subtree, so delete by path rather // than by child inode. - for (const entry of walkPostOrder(db, node.inode, canonical)) { + for (const entry of walkPostOrder(db, node.inode, realPath)) { removeEntry(db, entry.path, entry.inode, entry.type); recordDelete(db, rev, entry.path); } @@ -129,15 +150,5 @@ function removeEntry( throw createWorkspaceError("ENOENT", `parent directory missing: ${canonical}`, canonical); } - db.run("DELETE FROM vfs_dirents WHERE parent_inode = ? AND name = ?", parent.inode, name); - const remaining = db.scalar( - "SELECT COUNT(*) FROM vfs_dirents WHERE child_inode = ?", - inode, - ); - if ((remaining ?? 0) > 0) return; - - if (type === "file") { - db.run("DELETE FROM vfs_chunks WHERE inode = ?", inode); - } - db.run("DELETE FROM vfs_nodes WHERE inode = ?", inode); + unlinkDirent(db, parent.inode, name, inode, type); } diff --git a/packages/dofs/src/fs/unlink.ts b/packages/dofs/src/fs/unlink.ts new file mode 100644 index 00000000..7a7d33c2 --- /dev/null +++ b/packages/dofs/src/fs/unlink.ts @@ -0,0 +1,34 @@ +import type { Database } from "../storage.js"; + +type NodeType = "file" | "dir" | "symlink"; + +// Remove a single (parent, name) dirent and reap the child inode's +// node and chunk rows only once its last link disappears. A file inode +// can carry several hardlink names, so the node and its chunks survive +// until the final dirent is gone. Returns true when the inode was +// reaped, false when other links keep it alive. +// +// Callers own rev bumps and tombstones; this helper touches only +// vfs_dirents, vfs_chunks, and vfs_nodes. It is the single place the +// refcount-gated reap is implemented — rm, rename, and the sync apply +// path all funnel through here so the invariant lives once. +export function unlinkDirent( + db: Database, + parentInode: number, + name: string, + childInode: number, + type: NodeType, +): boolean { + db.run("DELETE FROM vfs_dirents WHERE parent_inode = ? AND name = ?", parentInode, name); + const remaining = db.scalar( + "SELECT COUNT(*) FROM vfs_dirents WHERE child_inode = ?", + childInode, + ); + if ((remaining ?? 0) > 0) return false; + + if (type === "file") { + db.run("DELETE FROM vfs_chunks WHERE inode = ?", childInode); + } + db.run("DELETE FROM vfs_nodes WHERE inode = ?", childInode); + return true; +} diff --git a/packages/dofs/src/index.ts b/packages/dofs/src/index.ts index abca31e7..6277a640 100644 --- a/packages/dofs/src/index.ts +++ b/packages/dofs/src/index.ts @@ -43,12 +43,19 @@ export type { CoalesceOptions } from "./sync/coalesce.js"; export { coalesceChanges } from "./sync/coalesce.js"; export { fetchChanges, fetchObjects, hasObjects } from "./sync/fetch.js"; export { DEFAULT_IGNORE, isIgnored } from "./sync/ignore.js"; -export { assertAppliedPushRev } from "./sync/invariant.js"; +export { assertAppliedPushCursor } from "./sync/invariant.js"; export type { ManifestChunk } from "./sync/manifests.js"; export { buildManifest, MANIFEST_VERSION } from "./sync/manifests.js"; export { pushObjects } from "./sync/push.js"; -export type { WatermarkKey } from "./sync/watermarks.js"; -export { currentRev, readWatermark, writeWatermark } from "./sync/watermarks.js"; +export type { ChangeCursor, WatermarkKey } from "./sync/watermarks.js"; +export { + compareChangeCursors, + currentRev, + readFetchCursor, + readWatermark, + writeFetchCursor, + writeWatermark, +} from "./sync/watermarks.js"; export type { ExecutedStatement } from "./testing-recording.js"; // RecordingStorage is workerd-safe (pure JS). SQLiteTestStorage // wraps node:sqlite and must be imported from diff --git a/packages/dofs/src/provider.test.ts b/packages/dofs/src/provider.test.ts index 299f777e..bb25cae9 100644 --- a/packages/dofs/src/provider.test.ts +++ b/packages/dofs/src/provider.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "vitest"; import { withDB } from "./fs/with-db.js"; import { SQLiteWorkspaceProvider } from "./provider.js"; +import type { Database } from "./storage.js"; +import type { ChangeEntry } from "./sync/changes.js"; +import { coalesceChanges } from "./sync/coalesce.js"; // Each provider test gets a fresh DB via withDB, which the workers // runner aliases to a DO-backed implementation. The provider holds @@ -10,6 +13,22 @@ async function withProvider(fn: (p: SQLiteWorkspaceProvider) => T | Promise fn(new SQLiteWorkspaceProvider(db, { now: () => 1000 }))); } +async function withProviderAndDB( + fn: (p: SQLiteWorkspaceProvider, db: Database) => T | Promise, +): Promise { + return withDB((db) => fn(new SQLiteWorkspaceProvider(db, { now: () => 1000 }), db)); +} + +async function drainChanges(db: Database, afterRev: number): Promise { + const out: ChangeEntry[] = []; + for await (const entry of coalesceChanges(db, afterRev)) out.push(entry); + return out; +} + +function kindPath(entries: ChangeEntry[]): Array<[ChangeEntry["kind"], string]> { + return entries.map((entry) => [entry.kind, entry.path]).sort((a, b) => a[1].localeCompare(b[1])); +} + describe("SQLiteWorkspaceProvider — capability flags", () => { it("reports the supported feature set", async () => { await withProvider((p) => { @@ -142,6 +161,18 @@ describe("SQLiteWorkspaceProvider — implemented methods", () => { }); }); + it("unlinkSync removes a symlink without deleting its target", async () => { + await withProvider((p) => { + p.writeFileSync("/target", "content"); + p.symlinkSync("/target", "/link"); + + p.unlinkSync("/link"); + + expect(p.existsSync("/link")).toBe(false); + expect(p.readFileSync("/target", "utf8")).toBe("content"); + }); + }); + it("rmdirSync removes an empty directory", async () => { await withProvider((p) => { p.mkdirSync("/a", {}); @@ -202,6 +233,119 @@ describe("SQLiteWorkspaceProvider — implemented methods", () => { }); describe("SQLiteWorkspaceProvider — renameSync overwrite matrix", () => { + it("records rename as an old-path delete and new-path live entry", async () => { + await withProviderAndDB(async (p, db) => { + p.writeFileSync("/src", "new"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/src", "/dst"); + + const entries = await drainChanges(db, cursor); + expect(kindPath(entries)).toEqual([ + ["file", "/dst"], + ["delete", "/src"], + ]); + }); + }); + + it("records directory rename for the whole moved subtree", async () => { + await withProviderAndDB(async (p, db) => { + p.mkdirSync("/src", {}); + p.writeFileSync("/src/a.txt", "a"); + p.mkdirSync("/src/sub", {}); + p.writeFileSync("/src/sub/b.txt", "b"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/src", "/dst"); + + const entries = await drainChanges(db, cursor); + expect(kindPath(entries)).toEqual([ + ["dir", "/dst"], + ["file", "/dst/a.txt"], + ["dir", "/dst/sub"], + ["file", "/dst/sub/b.txt"], + ["delete", "/src"], + ["delete", "/src/a.txt"], + ["delete", "/src/sub"], + ["delete", "/src/sub/b.txt"], + ]); + }); + }); + + it("records rename tombstones at the resolved old file path", async () => { + await withProviderAndDB(async (p, db) => { + p.mkdirSync("/real", {}); + p.writeFileSync("/real/file.txt", "x"); + p.symlinkSync("/real", "/link"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/link/file.txt", "/dst.txt"); + + const entries = await drainChanges(db, cursor); + expect(kindPath(entries)).toEqual([ + ["file", "/dst.txt"], + ["delete", "/real/file.txt"], + ]); + }); + }); + + it("records directory rename tombstones at the resolved old subtree paths", async () => { + await withProviderAndDB(async (p, db) => { + p.mkdirSync("/real", {}); + p.mkdirSync("/real/dir", {}); + p.writeFileSync("/real/dir/a.txt", "a"); + p.mkdirSync("/real/dir/sub", {}); + p.writeFileSync("/real/dir/sub/b.txt", "b"); + p.symlinkSync("/real", "/link"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/link/dir", "/dst"); + + const entries = await drainChanges(db, cursor); + expect(kindPath(entries)).toEqual([ + ["dir", "/dst"], + ["file", "/dst/a.txt"], + ["dir", "/dst/sub"], + ["file", "/dst/sub/b.txt"], + ["delete", "/real/dir"], + ["delete", "/real/dir/a.txt"], + ["delete", "/real/dir/sub"], + ["delete", "/real/dir/sub/b.txt"], + ]); + }); + }); + + it("same-inode rename through a symlinked file path is a no-op", async () => { + await withProviderAndDB(async (p, db) => { + p.mkdirSync("/real", {}); + p.writeFileSync("/real/file.txt", "x"); + p.symlinkSync("/real", "/link"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/real/file.txt", "/link/file.txt"); + + expect(p.readFileSync("/real/file.txt", "utf8")).toBe("x"); + expect(p.readFileSync("/link/file.txt", "utf8")).toBe("x"); + expect(await drainChanges(db, cursor)).toEqual([]); + }); + }); + + it("same-inode rename through a symlinked directory path is a no-op", async () => { + await withProviderAndDB(async (p, db) => { + p.mkdirSync("/real", {}); + p.mkdirSync("/real/dir", {}); + p.writeFileSync("/real/dir/a.txt", "a"); + p.symlinkSync("/real", "/link"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/real/dir", "/link/dir"); + + expect(p.readFileSync("/real/dir/a.txt", "utf8")).toBe("a"); + expect(p.readFileSync("/link/dir/a.txt", "utf8")).toBe("a"); + expect(await drainChanges(db, cursor)).toEqual([]); + }); + }); + it("file → existing file overwrites atomically", async () => { await withProvider((p) => { p.writeFileSync("/src", "new"); @@ -237,6 +381,119 @@ describe("SQLiteWorkspaceProvider — renameSync overwrite matrix", () => { }); }); + it.each([ + { + name: "file → existing symlink", + setup(p: SQLiteWorkspaceProvider) { + p.writeFileSync("/target", "x"); + p.writeFileSync("/src", "new"); + p.symlinkSync("/target", "/dst"); + }, + assertRenamed(p: SQLiteWorkspaceProvider) { + expect(p.existsSync("/src")).toBe(false); + expect(p.lstatSync("/dst").isFile()).toBe(true); + expect(p.readFileSync("/dst", "utf8")).toBe("new"); + expect(p.readFileSync("/target", "utf8")).toBe("x"); + }, + }, + { + name: "symlink → existing file", + setup(p: SQLiteWorkspaceProvider) { + p.writeFileSync("/target", "x"); + p.symlinkSync("/target", "/src"); + p.writeFileSync("/dst", "old"); + }, + assertRenamed(p: SQLiteWorkspaceProvider) { + expect(p.existsSync("/src")).toBe(false); + expect(p.lstatSync("/dst").isSymbolicLink()).toBe(true); + expect(p.readlinkSync("/dst")).toBe("/target"); + }, + }, + { + name: "symlink → existing symlink", + setup(p: SQLiteWorkspaceProvider) { + p.writeFileSync("/target", "x"); + p.writeFileSync("/other", "y"); + p.symlinkSync("/target", "/src"); + p.symlinkSync("/other", "/dst"); + }, + assertRenamed(p: SQLiteWorkspaceProvider) { + expect(p.existsSync("/src")).toBe(false); + expect(p.lstatSync("/dst").isSymbolicLink()).toBe(true); + expect(p.readlinkSync("/dst")).toBe("/target"); + }, + }, + ])("$name overwrites atomically", async ({ setup, assertRenamed }) => { + await withProvider((p) => { + setup(p); + p.renameSync("/src", "/dst"); + assertRenamed(p); + }); + }); + + it.each([ + { + name: "file → existing empty dir", + code: "EISDIR", + setup(p: SQLiteWorkspaceProvider) { + p.writeFileSync("/src", "new"); + p.mkdirSync("/dst", {}); + }, + assertUnchanged(p: SQLiteWorkspaceProvider) { + expect(p.readFileSync("/src", "utf8")).toBe("new"); + expect(p.statSync("/dst").isDirectory()).toBe(true); + }, + }, + { + name: "symlink → existing empty dir", + code: "EISDIR", + setup(p: SQLiteWorkspaceProvider) { + p.writeFileSync("/target", "x"); + p.symlinkSync("/target", "/src"); + p.mkdirSync("/dst", {}); + }, + assertUnchanged(p: SQLiteWorkspaceProvider) { + expect(p.readlinkSync("/src")).toBe("/target"); + expect(p.statSync("/dst").isDirectory()).toBe(true); + }, + }, + { + name: "dir → existing file", + code: "ENOTDIR", + setup(p: SQLiteWorkspaceProvider) { + p.mkdirSync("/src", {}); + p.writeFileSync("/dst", "old"); + }, + assertUnchanged(p: SQLiteWorkspaceProvider) { + expect(p.statSync("/src").isDirectory()).toBe(true); + expect(p.readFileSync("/dst", "utf8")).toBe("old"); + }, + }, + { + name: "dir → existing symlink", + code: "ENOTDIR", + setup(p: SQLiteWorkspaceProvider) { + p.writeFileSync("/target", "x"); + p.mkdirSync("/src", {}); + p.symlinkSync("/target", "/dst"); + }, + assertUnchanged(p: SQLiteWorkspaceProvider) { + expect(p.statSync("/src").isDirectory()).toBe(true); + expect(p.readlinkSync("/dst")).toBe("/target"); + }, + }, + ])("$name rejects without recording sync changes", async ({ code, setup, assertUnchanged }) => { + await withProviderAndDB(async (p, db) => { + setup(p); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + expect(() => p.renameSync("/src", "/dst")).toThrowError(expect.objectContaining({ code })); + + assertUnchanged(p); + expect(await drainChanges(db, cursor)).toEqual([]); + }); + }); + it("source missing throws ENOENT", async () => { await withProvider((p) => { expect(() => p.renameSync("/missing", "/dst")).toThrowError( @@ -245,6 +502,35 @@ describe("SQLiteWorkspaceProvider — renameSync overwrite matrix", () => { }); }); + it("same-path rename validates the source before no-op", async () => { + await withProvider((p) => { + expect(() => p.renameSync("/missing", "/missing")).toThrowError( + expect.objectContaining({ code: "ENOENT" }), + ); + }); + }); + + it("same-path rename of an existing file is a no-op", async () => { + await withProviderAndDB(async (p, db) => { + p.writeFileSync("/src", "x"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/src", "/src"); + + expect(p.readFileSync("/src", "utf8")).toBe("x"); + expect(await drainChanges(db, cursor)).toEqual([]); + }); + }); + + it("rename of a file into itself as a parent does not report directory self-move", async () => { + await withProvider((p) => { + p.writeFileSync("/file", "x"); + expect(() => p.renameSync("/file", "/file/child")).toThrowError( + expect.objectContaining({ code: "ENOENT" }), + ); + }); + }); + it("rename onto root throws EINVAL", async () => { await withProvider((p) => { p.writeFileSync("/src", "x"); @@ -263,11 +549,62 @@ describe("SQLiteWorkspaceProvider — renameSync overwrite matrix", () => { }); }); - // TODO: renaming a symlink should move the link itself, not the - // target. Today renameSync uses resolveInode() with the default - // followSymlinks: true, so a rename on a symlink actually moves the - // pointed-at file. Pin this once renameSync grows an - // lresolveInode-style call (see provider.ts:248). + it("rename of a symlink moves the link itself", async () => { + await withProvider((p) => { + p.writeFileSync("/target", "x"); + p.symlinkSync("/target", "/link"); + p.renameSync("/link", "/moved"); + expect(p.existsSync("/target")).toBe(true); + expect(p.readlinkSync("/moved")).toBe("/target"); + expect(p.existsSync("/link")).toBe(false); + }); + }); + + it("rename of a directory into its own subtree throws EINVAL", async () => { + await withProvider((p) => { + p.mkdirSync("/src", {}); + p.mkdirSync("/src/sub", {}); + expect(() => p.renameSync("/src", "/src/sub/dst")).toThrowError( + expect.objectContaining({ code: "EINVAL" }), + ); + }); + }); + + it("rename of a directory through a symlink into its own subtree throws EINVAL", async () => { + await withProviderAndDB(async (p, db) => { + p.mkdirSync("/src", {}); + p.mkdirSync("/src/sub", {}); + p.symlinkSync("/src/sub", "/link"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + expect(() => p.renameSync("/src", "/link/dst")).toThrowError( + expect.objectContaining({ code: "EINVAL" }), + ); + + expect(p.statSync("/src/sub").isDirectory()).toBe(true); + expect(p.readlinkSync("/link")).toBe("/src/sub"); + expect(await drainChanges(db, cursor)).toEqual([]); + }); + }); + + it("rename of a directory through a symlink out of its subtree succeeds", async () => { + await withProviderAndDB(async (p, db) => { + p.mkdirSync("/src", {}); + p.mkdirSync("/other", {}); + // /src/link points outside the source subtree, so the destination + // /src/link/dst resolves to /other/dst even though the literal path + // is lexically under /src. The self-move guard is inode-based and + // must allow this move rather than reject it on a textual prefix. + p.symlinkSync("/other", "/src/link"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/src", "/src/link/dst"); + + expect(p.statSync("/other/dst").isDirectory()).toBe(true); + expect(p.existsSync("/src")).toBe(false); + expect(await drainChanges(db, cursor)).not.toEqual([]); + }); + }); }); describe("SQLiteWorkspaceProvider — unimplemented surface (stubs)", () => { diff --git a/packages/dofs/src/provider.ts b/packages/dofs/src/provider.ts index 19354c15..0746828f 100644 --- a/packages/dofs/src/provider.ts +++ b/packages/dofs/src/provider.ts @@ -15,6 +15,7 @@ import { mkdir as mkdirImpl } from "./fs/mkdir.js"; import { readdir as readdirImpl } from "./fs/readdir.js"; import { readRangeSync as readRangeSyncImpl } from "./fs/readFile.js"; import { readlink as readlinkImpl } from "./fs/readlink.js"; +import { rename as renameImpl } from "./fs/rename.js"; import { resolveInode } from "./fs/resolve.js"; import { rm as rmImpl } from "./fs/rm.js"; import { stat as statImpl } from "./fs/stat.js"; @@ -317,105 +318,27 @@ export class SQLiteWorkspaceProvider { } renameSync(oldPath: string, newPath: string): void { - // The FS module doesn't expose rename as a standalone operation - // yet; we lean on the existing schema-level pieces here. When - // rename grows up (cross-directory, overwriting an existing file, - // ...) it should move into fs/rename.ts with its own tests. + // Commit any still-pending creates at either end before the rename + // touches dirents: the source needs a real inode to move, and a + // pending buffer at the destination would otherwise slip past + // rename's dirent-based existence check and lose bytes on release. flushPendingByPath(this.db, oldPath, this.now); flushPendingByPath(this.db, newPath, this.now); - const node = resolveInode(this.db, oldPath); - if (node === null) { - throw createWorkspaceError("ENOENT", `no such path: ${oldPath}`, oldPath); - } - const { parts: oldParts, path: oldCanonical } = canonicalizePath(oldPath); - const oldName = oldParts[oldParts.length - 1]; - const oldParentPath = oldParts.length === 1 ? "/" : `/${oldParts.slice(0, -1).join("/")}`; - const oldParent = resolveInode(this.db, oldParentPath, { followSymlinks: false }); - if (oldParent === null || oldParent.type !== "dir") { - throw createWorkspaceError( - "ENOENT", - `parent directory missing: ${oldCanonical}`, - oldCanonical, - ); - } - const { parts, path: newCanonical } = canonicalizePath(newPath); - if (oldCanonical === newCanonical) return; - if (parts.length === 0) { - throw createWorkspaceError("EINVAL", "cannot rename onto root", newCanonical); - } - const newName = parts[parts.length - 1]; - const newParentPath = parts.length === 1 ? "/" : `/${parts.slice(0, -1).join("/")}`; - const newParent = resolveInode(this.db, newParentPath); - if (newParent === null || newParent.type !== "dir") { - throw createWorkspaceError( - "ENOENT", - `parent directory missing: ${newCanonical}`, - newCanonical, - ); - } - this.db.transactionSync(() => { - // If the destination already exists, displace it before linking - // the source dirent. POSIX rename(2) is atomic and overwrites a - // regular file or empty directory at the target. We follow the - // same semantics: an existing file at newPath is unlinked, and - // its inode (and chunks / blobs) are reaped via the existing - // gc() safety window. - const existing = this.db.one<{ child_inode: number; type: string }>( - `SELECT d.child_inode AS child_inode, n.type AS type - FROM vfs_dirents d JOIN vfs_nodes n ON n.inode = d.child_inode - WHERE d.parent_inode = ? AND d.name = ?`, - newParent.inode, - newName, - ); - const destinationAlreadyNamesSource = existing?.child_inode === node.inode; - if (existing !== undefined && !destinationAlreadyNamesSource) { - // Refuse to overwrite a non-empty directory or replace a - // directory with a file (Linux rename semantics). - if (existing.type === "dir") { - const childCount = this.db.scalar( - "SELECT COUNT(*) FROM vfs_dirents WHERE parent_inode = ?", - existing.child_inode, - ); - if ((childCount ?? 0) > 0) { - throw createWorkspaceError("ENOTEMPTY", `not empty: ${newCanonical}`, newCanonical); - } - } - // Unlink only the displaced destination name. If other - // hardlinks still reference the displaced file inode, keep its - // chunks and node alive. - this.db.run( - "DELETE FROM vfs_dirents WHERE parent_inode = ? AND name = ?", - newParent.inode, - newName, - ); - const remaining = this.db.scalar( - "SELECT COUNT(*) FROM vfs_dirents WHERE child_inode = ?", - existing.child_inode, - ); - if ((remaining ?? 0) === 0) { - this.db.run("DELETE FROM vfs_chunks WHERE inode = ?", existing.child_inode); - this.db.run("DELETE FROM vfs_nodes WHERE inode = ?", existing.child_inode); - // The displaced inode is gone from SQL. Any open write - // buffer keyed by it would otherwise hold bytes pointed at - // a dead inode; release would then commit chunks against a - // missing row (0-row UPDATE, silent data loss). - deleteWriteBuffer(this.db, existing.child_inode); - } - } - this.db.run( - "DELETE FROM vfs_dirents WHERE parent_inode = ? AND name = ?", - oldParent.inode, - oldName, + // Capture the destination inode before the rename so we can evict + // its write-buffer cache entry if the rename displaced and reaped + // it. Without this, a release on an open destination would commit + // chunks against a dead inode (0-row UPDATE, silent data loss). + const displaced = resolveInode(this.db, newPath, { followSymlinks: false }); + renameImpl(this.db, oldPath, newPath); + if (displaced !== null) { + const stillAlive = this.db.scalar( + "SELECT inode FROM vfs_nodes WHERE inode = ?", + displaced.inode, ); - if (!destinationAlreadyNamesSource) { - this.db.run( - "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", - newParent.inode, - newName, - node.inode, - ); + if (stillAlive === undefined) { + deleteWriteBuffer(this.db, displaced.inode); } - }); + } } // -- Default implementations --------------------------------------- diff --git a/packages/dofs/src/schema/index.ts b/packages/dofs/src/schema/index.ts index 3e479940..e0147e96 100644 --- a/packages/dofs/src/schema/index.ts +++ b/packages/dofs/src/schema/index.ts @@ -64,6 +64,11 @@ export function initializeSchema(db: Database, now: () => number): void { "fetchRev", 0, ); + db.run( + "INSERT OR IGNORE INTO _vfs_fetch_cursor (k, backend, path) VALUES (?, 'default', ?)", + "fetch", + null, + ); db.run( `INSERT OR IGNORE INTO vfs_nodes diff --git a/packages/dofs/src/schema/sync.ts b/packages/dofs/src/schema/sync.ts index 3680b455..fdbac101 100644 --- a/packages/dofs/src/schema/sync.ts +++ b/packages/dofs/src/schema/sync.ts @@ -34,6 +34,17 @@ export const SYNC_STATEMENTS = [ v INTEGER NOT NULL, PRIMARY KEY (k, backend) )`, + // The fetch cursor's same-rev `path` component, keyed by + // (k, backend) so each backend resumes a partially-drained rev + // independently. The rev component lives in _vfs_watermark under + // 'fetchRev'; this table only holds the in-rev path. `backend` + // mirrors _vfs_watermark and defaults to 'default'. + `CREATE TABLE IF NOT EXISTS _vfs_fetch_cursor ( + k TEXT NOT NULL CHECK(k = 'fetch'), + backend TEXT NOT NULL DEFAULT 'default', + path TEXT, + PRIMARY KEY (k, backend) + )`, // The `mode` column was added at schema v2; `schema/migrations.ts` // owns the ALTER for existing databases. Keep the CHECK // constraint here aligned with the migration's CHECK so fresh diff --git a/packages/dofs/src/sync/apply.test.ts b/packages/dofs/src/sync/apply.test.ts index b326e0be..314e2a9e 100644 --- a/packages/dofs/src/sync/apply.test.ts +++ b/packages/dofs/src/sync/apply.test.ts @@ -1,16 +1,21 @@ import { describe, expect, it } from "vitest"; +import { link } from "../fs/link.js"; import { mkdir } from "../fs/mkdir.js"; import { invalidateReadOnlyMountCache } from "../fs/mount-guard.js"; import { readFile } from "../fs/readFile.js"; +import { readlink } from "../fs/readlink.js"; +import { rename } from "../fs/rename.js"; import { resolveInode } from "../fs/resolve.js"; +import { rm } from "../fs/rm.js"; +import { symlink } from "../fs/symlink.js"; import { withDB, withTwoDBs } from "../fs/with-db.js"; -import { writeFile } from "../fs/writeFile.js"; +import { writeFile, writeFileSync } from "../fs/writeFile.js"; import { applyChanges, applyChangesSync } from "./apply.js"; import type { ChangeEntry } from "./changes.js"; import { coalesceChanges } from "./coalesce.js"; import { fetchObjects } from "./fetch.js"; -import { writeWatermark } from "./watermarks.js"; +import { currentRev } from "./watermarks.js"; async function drain(it: AsyncIterable): Promise { const out: T[] = []; @@ -24,6 +29,10 @@ function hex(bytes: Uint8Array): string { return s; } +function deepChildPath(depth: number): string { + return `/dst/${Array.from({ length: depth }, () => "d").join("/")}/old.txt`; +} + async function collectObjects( db: import("../storage.js").Database, entries: ChangeEntry[], @@ -64,51 +73,466 @@ describe("applyChanges", () => { ); }); - it("advances fetchRev to the largest applied rev", async () => { + it("commits in batches capped by byte budget", async () => { + // Force many small files; with a tiny byte budget the apply + // path should still converge, just across more batches. We + // verify convergence rather than batch count (batch count is + // an implementation detail). await withTwoDBs( async (a) => { - await writeFile(a, "/x.txt", "x", {}, () => 1); + for (let i = 0; i < 10; i++) { + await writeFile(a, `/f${i}.txt`, `payload ${i}`, {}, () => 100 + i); + } const entries = await drain(coalesceChanges(a, 0)); return { entries, objects: await collectObjects(a, entries) }; }, async (b, { entries, objects }) => { - await applyChanges(b, entries, objects, { advanceFetchRev: 5 }); - const got = await import("./watermarks.js").then((m) => m.readWatermark(b, "fetchRev")); - expect(got).toBe(5); + await applyChanges(b, entries, objects, { maxBytesPerBatch: 16 }); + for (let i = 0; i < 10; i++) { + expect(await readFile(b, `/f${i}.txt`, "utf8")).toBe(`payload ${i}`); + } + }, + ); + }); + + it("applies a file rename over an existing file", async () => { + await withTwoDBs( + async (a) => { + await writeFile(a, "/src", "new", {}, () => 1); + await writeFile(a, "/dst", "old", {}, () => 2); + const cursor = currentRev(a); + + rename(a, "/src", "/dst"); + + const entries = await drain(coalesceChanges(a, cursor)); + return { entries, objects: await collectObjects(a, entries) }; + }, + async (b, { entries, objects }) => { + await writeFile(b, "/src", "new", {}, () => 1); + await writeFile(b, "/dst", "old", {}, () => 2); + + await applyChanges(b, entries, objects); + + expect(resolveInode(b, "/src")).toBeNull(); + expect(await readFile(b, "/dst", "utf8")).toBe("new"); }, ); }); - it("does not regress fetchRev on partial replay", async () => { + it("applies a directory rename over an existing empty directory", async () => { + await withTwoDBs( + async (a) => { + mkdir(a, "/src", { mode: 0o700, recursive: true }, () => 1); + await writeFile(a, "/src/inside", "x", {}, () => 2); + mkdir(a, "/dst", { mode: 0o755 }, () => 3); + const cursor = currentRev(a); + + rename(a, "/src", "/dst"); + + const entries = await drain(coalesceChanges(a, cursor)); + return { entries, objects: await collectObjects(a, entries) }; + }, + async (b, { entries, objects }) => { + mkdir(b, "/src", { mode: 0o700, recursive: true }, () => 1); + await writeFile(b, "/src/inside", "x", {}, () => 2); + mkdir(b, "/dst", { mode: 0o755 }, () => 3); + + await applyChanges(b, entries, objects); + + expect(resolveInode(b, "/src")).toBeNull(); + expect(resolveInode(b, "/dst", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + mtime: 1, + }); + expect(await readFile(b, "/dst/inside", "utf8")).toBe("x"); + }, + ); + }); + + it("applyChangesSync updates existing directory metadata", async () => { + await withDB((db) => { + mkdir(db, "/dst", { mode: 0o755 }, () => 3); + + applyChangesSync( + db, + [{ kind: "dir", rev: 99, path: "/dst", mode: 0o700, mtime: 1 }], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + mtime: 1, + }); + }); + }); + + it("does not update existing directory mtime for upstream entries with matching mode", async () => { await withDB(async (db) => { - // Pretend a previous apply pass advanced fetchRev to 10. - writeWatermark(db, "fetchRev", 10); - await applyChanges(db, [], new Map(), { advanceFetchRev: 3 }); - const got = await import("./watermarks.js").then((m) => m.readWatermark(db, "fetchRev")); - // The helper takes the max of current and requested, never - // moves backwards. - expect(got).toBe(10); + mkdir(db, "/dst", { mode: 0o700 }, () => 3); + + await applyChanges( + db, + [{ kind: "dir", rev: 99, path: "/dst", mode: 0o700, mtime: 1 }], + new Map(), + { source: "upstream" }, + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o700, + mtime: 3, + }); }); }); - it("commits in batches capped by byte budget", async () => { - // Force many small files; with a tiny byte budget the apply - // path should still converge, just across more batches. We - // verify convergence rather than batch count (batch count is - // an implementation detail). + it("applies a file rename over an existing symlink", async () => { await withTwoDBs( async (a) => { - for (let i = 0; i < 10; i++) { - await writeFile(a, `/f${i}.txt`, `payload ${i}`, {}, () => 100 + i); - } - const entries = await drain(coalesceChanges(a, 0)); + await writeFile(a, "/target", "target", {}, () => 1); + await writeFile(a, "/src", "new", {}, () => 2); + symlink(a, "/target", "/dst", () => 3); + const cursor = currentRev(a); + + rename(a, "/src", "/dst"); + + const entries = await drain(coalesceChanges(a, cursor)); return { entries, objects: await collectObjects(a, entries) }; }, async (b, { entries, objects }) => { - await applyChanges(b, entries, objects, { maxBytesPerBatch: 16 }); - for (let i = 0; i < 10; i++) { - expect(await readFile(b, `/f${i}.txt`, "utf8")).toBe(`payload ${i}`); - } + await writeFile(b, "/target", "target", {}, () => 1); + await writeFile(b, "/src", "new", {}, () => 2); + symlink(b, "/target", "/dst", () => 3); + + await applyChanges(b, entries, objects); + + expect(resolveInode(b, "/src")).toBeNull(); + expect(resolveInode(b, "/dst", { followSymlinks: false })?.type).toBe("file"); + expect(await readFile(b, "/dst", "utf8")).toBe("new"); + expect(await readFile(b, "/target", "utf8")).toBe("target"); + }, + ); + }); + + it("applies a symlink rename over an existing file", async () => { + await withTwoDBs( + async (a) => { + await writeFile(a, "/target", "target", {}, () => 1); + symlink(a, "/target", "/src", () => 2); + await writeFile(a, "/dst", "old", {}, () => 3); + const cursor = currentRev(a); + + rename(a, "/src", "/dst"); + + const entries = await drain(coalesceChanges(a, cursor)); + return { entries, objects: await collectObjects(a, entries) }; + }, + async (b, { entries, objects }) => { + await writeFile(b, "/target", "target", {}, () => 1); + symlink(b, "/target", "/src", () => 2); + await writeFile(b, "/dst", "old", {}, () => 3); + + await applyChanges(b, entries, objects); + + expect(resolveInode(b, "/src", { followSymlinks: false })).toBeNull(); + expect(resolveInode(b, "/dst", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(b, "/dst")).toBe("/target"); + expect(await readFile(b, "/target", "utf8")).toBe("target"); + }, + ); + }); + + it("applies a symlink rename over an existing symlink", async () => { + await withTwoDBs( + async (a) => { + await writeFile(a, "/target", "target", {}, () => 1); + await writeFile(a, "/other", "other", {}, () => 2); + symlink(a, "/target", "/src", () => 3); + symlink(a, "/other", "/dst", () => 4); + const cursor = currentRev(a); + + rename(a, "/src", "/dst"); + + const entries = await drain(coalesceChanges(a, cursor)); + return { entries, objects: await collectObjects(a, entries) }; + }, + async (b, { entries, objects }) => { + await writeFile(b, "/target", "target", {}, () => 1); + await writeFile(b, "/other", "other", {}, () => 2); + symlink(b, "/target", "/src", () => 3); + symlink(b, "/other", "/dst", () => 4); + + await applyChanges(b, entries, objects); + + expect(resolveInode(b, "/src", { followSymlinks: false })).toBeNull(); + expect(resolveInode(b, "/dst", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(b, "/dst")).toBe("/target"); + expect(await readFile(b, "/target", "utf8")).toBe("target"); + expect(await readFile(b, "/other", "utf8")).toBe("other"); + }, + ); + }); + + it("applies a directory rename containing a symlink without deleting its target", async () => { + await withTwoDBs( + async (a) => { + await writeFile(a, "/target", "target", {}, () => 1); + mkdir(a, "/src", {}, () => 2); + symlink(a, "/target", "/src/link", () => 3); + const cursor = currentRev(a); + + rename(a, "/src", "/dst"); + + const entries = await drain(coalesceChanges(a, cursor)); + return { entries, objects: await collectObjects(a, entries) }; + }, + async (b, { entries, objects }) => { + await writeFile(b, "/target", "target", {}, () => 1); + mkdir(b, "/src", {}, () => 2); + symlink(b, "/target", "/src/link", () => 3); + + await applyChanges(b, entries, objects); + + expect(resolveInode(b, "/src/link", { followSymlinks: false })).toBeNull(); + expect(resolveInode(b, "/dst/link", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(b, "/dst/link")).toBe("/target"); + expect(await readFile(b, "/target", "utf8")).toBe("target"); + }, + ); + }); + + it("applies a file over an existing directory subtree", async () => { + await withDB(async (db) => { + mkdir(db, "/dst/sub", { recursive: true }, () => 1); + await writeFile(db, "/dst/sub/old.txt", "old", {}, () => 2); + + await applyChanges( + db, + [ + { + kind: "file", + rev: 99, + path: "/dst", + mode: 0o644, + mtime: 3, + size: 0, + chunks: [], + }, + ], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })?.type).toBe("file"); + expect(resolveInode(db, "/dst/sub/old.txt")).toBeNull(); + }); + }); + + it("applies a file over a deeply nested existing directory subtree", async () => { + await withDB(async (db) => { + const oldPath = deepChildPath(12_000); + mkdir(db, oldPath.slice(0, -"/old.txt".length), { recursive: true }, () => 1); + await writeFile(db, oldPath, "old", {}, () => 2); + + await applyChanges( + db, + [ + { + kind: "file", + rev: 99, + path: "/dst", + mode: 0o644, + mtime: 3, + size: 0, + chunks: [], + }, + ], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })?.type).toBe("file"); + expect(resolveInode(db, oldPath, { followSymlinks: false })).toBeNull(); + }); + }); + + it("applies a symlink over an existing directory subtree", async () => { + await withDB((db) => { + mkdir(db, "/dst/sub", { recursive: true }, () => 1); + writeFileSync(db, "/dst/sub/old.txt", new Uint8Array(), {}, () => 2); + + applyChangesSync( + db, + [ + { + kind: "symlink", + rev: 99, + path: "/dst", + mode: 0o777, + mtime: 3, + target: "/target", + }, + ], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(db, "/dst")).toBe("/target"); + expect(resolveInode(db, "/dst/sub/old.txt")).toBeNull(); + }); + }); + + it("applies a symlink over one hardlink without removing sibling hardlinks", async () => { + await withDB(async (db) => { + await writeFile(db, "/dst", "shared", {}, () => 1); + link(db, "/dst", "/other"); + + await applyChanges( + db, + [ + { + kind: "symlink", + rev: 99, + path: "/dst", + mode: 0o777, + mtime: 2, + target: "/target", + }, + ], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(db, "/dst")).toBe("/target"); + expect(await readFile(db, "/other", "utf8")).toBe("shared"); + }); + }); + + it("applies a directory over one hardlink without removing sibling hardlinks", async () => { + await withDB(async (db) => { + await writeFile(db, "/dst", "shared", {}, () => 1); + link(db, "/dst", "/other"); + + await applyChanges( + db, + [{ kind: "dir", rev: 99, path: "/dst", mode: 0o755, mtime: 2 }], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })?.type).toBe("dir"); + expect(await readFile(db, "/other", "utf8")).toBe("shared"); + }); + }); + + it("applies a file over a directory whose subtree hardlinks a file outside it", async () => { + await withDB(async (db) => { + await mkdir(db, "/dir", { mode: 0o755 }, () => 1); + await writeFile(db, "/dir/inner", "shared", {}, () => 1); + // /outside is a second name for /dir/inner; replacing /dir must + // walk the subtree by name and keep /outside alive. + link(db, "/dir/inner", "/outside"); + + await applyChanges( + db, + [ + { + kind: "file", + rev: 99, + path: "/dir", + mode: 0o644, + mtime: 2, + size: 0, + chunks: [], + }, + ], + new Map(), + ); + + expect(resolveInode(db, "/dir", { followSymlinks: false })?.type).toBe("file"); + expect(resolveInode(db, "/dir/inner", { followSymlinks: false })).toBeNull(); + expect(await readFile(db, "/outside", "utf8")).toBe("shared"); + }); + }); + + it("applyChangesSync applies a symlink over a deeply nested existing directory subtree", async () => { + await withDB((db) => { + const oldPath = deepChildPath(12_000); + mkdir(db, oldPath.slice(0, -"/old.txt".length), { recursive: true }, () => 1); + writeFileSync(db, oldPath, new Uint8Array(), {}, () => 2); + + applyChangesSync( + db, + [ + { + kind: "symlink", + rev: 99, + path: "/dst", + mode: 0o777, + mtime: 3, + target: "/target", + }, + ], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })?.type).toBe("symlink"); + expect(readlink(db, "/dst")).toBe("/target"); + expect(resolveInode(db, oldPath, { followSymlinks: false })).toBeNull(); + }); + }); + + it("applies a directory over an existing file", async () => { + await withDB(async (db) => { + await writeFile(db, "/dst", "old", {}, () => 1); + + await applyChanges( + db, + [{ kind: "dir", rev: 99, path: "/dst", mode: 0o755, mtime: 2 }], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o755, + mtime: 2, + }); + }); + }); + + it("applies a directory over an existing symlink", async () => { + await withDB((db) => { + symlink(db, "/target", "/dst", () => 1); + + applyChangesSync( + db, + [{ kind: "dir", rev: 99, path: "/dst", mode: 0o755, mtime: 2 }], + new Map(), + ); + + expect(resolveInode(db, "/dst", { followSymlinks: false })).toMatchObject({ + type: "dir", + mode: 0o755, + mtime: 2, + }); + }); + }); + + it("applies a coalesced file-to-directory replacement", async () => { + await withTwoDBs( + async (a) => { + await writeFile(a, "/dst", "old", {}, () => 1); + const cursor = currentRev(a); + rm(a, "/dst", { force: true }); + mkdir(a, "/dst", {}, () => 2); + const entries = await drain(coalesceChanges(a, cursor)); + expect(entries).toEqual([expect.objectContaining({ kind: "dir", path: "/dst" })]); + return { entries, objects: await collectObjects(a, entries) }; + }, + async (b, { entries, objects }) => { + await writeFile(b, "/dst", "old", {}, () => 1); + + await applyChanges(b, entries, objects); + + expect(resolveInode(b, "/dst", { followSymlinks: false })?.type).toBe("dir"); }, ); }); diff --git a/packages/dofs/src/sync/apply.ts b/packages/dofs/src/sync/apply.ts index 3b02b202..62a6362f 100644 --- a/packages/dofs/src/sync/apply.ts +++ b/packages/dofs/src/sync/apply.ts @@ -3,7 +3,10 @@ import { readOnlyRootFor } from "../fs/mount-guard.js"; import { resolveInode } from "../fs/resolve.js"; import { rm } from "../fs/rm.js"; import { symlink } from "../fs/symlink.js"; +import { unlinkDirent } from "../fs/unlink.js"; import { writeFile, writeFileSync } from "../fs/writeFile.js"; +import { canonicalizePath } from "../path.js"; +import { incrementRev } from "../rev.js"; import type { Database } from "../storage.js"; import type { ChangeEntry } from "./changes.js"; import { computeManifestHash } from "./manifests.js"; @@ -47,11 +50,6 @@ export interface ApplyOptions { maxBytesPerBatch?: number; // Soft cap on entries per batch. Default 1024 paths. maxPathsPerBatch?: number; - // After the stream drains, advance fetchRev to this value if it's - // higher than the current persisted value. Callers pass the - // sender's currentRev so the next pull resumes from the right - // cursor. Never regresses the watermark. - advanceFetchRev?: number; // Where the entries came from. 'local' (default) treats the apply // path like any other mutation: writeFile/mkdir/etc bump vfs_meta.rev // and the push loop later ships those new revs upstream. 'upstream' @@ -74,12 +72,136 @@ export interface ApplyOptions { const DEFAULT_MAX_BYTES = 64 * 1024 * 1024; const DEFAULT_MAX_PATHS = 1024; +type NodeType = "file" | "dir" | "symlink"; + function hex(bytes: Uint8Array): string { let s = ""; for (let i = 0; i < bytes.byteLength; i++) s += bytes[i].toString(16).padStart(2, "0"); return s; } +function removeReplaceableFinalEntry( + db: Database, + path: string, + incomingKind: "file" | "symlink", +): void { + const existing = resolveInode(db, path, { followSymlinks: false }); + if (existing === null) return; + if (incomingKind === "file" && existing.type === "file") return; + + removeInodeTreeAtPath(db, path, existing.inode, existing.type); +} + +// Structural conflict cleanup for upstream applies. This removes +// the local shape without recording tombstones because the incoming +// entry is the authoritative state for this path. +function removeInodeTreeAtPath(db: Database, path: string, inode: number, type: NodeType): void { + const root = direntForPath(db, path, inode); + const stack: Array<{ + path: string; + parentInode: number; + name: string; + inode: number; + type: NodeType; + expanded: boolean; + }> = [ + { + path, + parentInode: root.parentInode, + name: root.name, + inode, + type, + expanded: false, + }, + ]; + + while (stack.length > 0) { + const current = stack.pop(); + if (current === undefined) break; + + if (current.type === "dir" && !current.expanded) { + const children = db.all<{ name: string; child_inode: number; type: NodeType }>( + `SELECT d.name AS name, d.child_inode AS child_inode, n.type AS type + FROM vfs_dirents d + JOIN vfs_nodes n ON n.inode = d.child_inode + WHERE d.parent_inode = ?`, + current.inode, + ); + stack.push({ ...current, expanded: true }); + for (let i = children.length - 1; i >= 0; i--) { + const child = children[i]; + const childPath = current.path === "/" ? `/${child.name}` : `${current.path}/${child.name}`; + stack.push({ + path: childPath, + parentInode: current.inode, + name: child.name, + inode: child.child_inode, + type: child.type, + expanded: false, + }); + } + continue; + } + + // Unlink this one name and reap the inode only when its last link + // disappears, so a sibling hardlink (inside or outside the subtree) + // keeps the file alive. (parent, name) is unique, so this removes + // exactly the dirent the walk is visiting. + unlinkDirent(db, current.parentInode, current.name, current.inode, current.type); + } +} + +function direntForPath( + db: Database, + path: string, + inode: number, +): { parentInode: number; name: string } { + const { parts, path: canonical } = canonicalizePath(path); + if (parts.length === 0) { + throw new Error(`applyChanges: cannot structurally replace root ${canonical}`); + } + const name = parts[parts.length - 1]; + const parentPath = parts.length === 1 ? "/" : `/${parts.slice(0, -1).join("/")}`; + const parent = resolveInode(db, parentPath, { followSymlinks: false }); + if (parent === null || parent.type !== "dir") { + throw new Error(`applyChanges: parent missing for structural replacement ${canonical}`); + } + const child = db.scalar( + "SELECT child_inode FROM vfs_dirents WHERE parent_inode = ? AND name = ?", + parent.inode, + name, + ); + if (child !== inode) { + throw new Error(`applyChanges: dirent mismatch for structural replacement ${canonical}`); + } + return { parentInode: parent.inode, name }; +} + +function applyDirectoryEntry(db: Database, entry: Extract): void { + const mode = entry.mode & 0o7777; + const existing = resolveInode(db, entry.path, { followSymlinks: false }); + if (existing === null) { + mkdir(db, entry.path, { mode, recursive: true }, () => entry.mtime); + return; + } + if (existing.type !== "dir") { + removeInodeTreeAtPath(db, entry.path, existing.inode, existing.type); + mkdir(db, entry.path, { mode, recursive: true }, () => entry.mtime); + return; + } + + db.transactionSync(() => { + const rev = incrementRev(db); + db.run( + "UPDATE vfs_nodes SET mode = ?, mtime = ?, rev = ? WHERE inode = ?", + mode, + entry.mtime, + rev, + existing.inode, + ); + }); +} + // 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 @@ -150,13 +272,14 @@ export async function applyChanges( continue; } if (entry.kind === "dir") { - mkdir(db, entry.path, { mode: entry.mode, recursive: true }, () => entry.mtime); + applyDirectoryEntry(db, entry); applied++; pathsInBatch++; if (pathsInBatch >= maxPaths) flush(); continue; } if (entry.kind === "symlink") { + removeReplaceableFinalEntry(db, entry.path, "symlink"); symlink(db, entry.target, entry.path, () => entry.mtime); applied++; pathsInBatch++; @@ -190,6 +313,7 @@ export async function applyChanges( buf.set(p, off); off += p.byteLength; } + removeReplaceableFinalEntry(db, entry.path, "file"); await writeFile(db, entry.path, buf, { mode: entry.mode }, () => entry.mtime); applied++; bytesInBatch += total; @@ -197,16 +321,6 @@ export async function applyChanges( if (bytesInBatch >= maxBytes || pathsInBatch >= maxPaths) flush(); } - // Advance fetchRev only after the stream drains so a crash - // mid-apply leaves the watermark behind and the next pull - // re-fetches anything not yet committed. - if (options.advanceFetchRev !== undefined) { - const current = readWatermark(db, "fetchRev", options.backend); - if (options.advanceFetchRev > current) { - writeWatermark(db, "fetchRev", options.advanceFetchRev, options.backend); - } - } - // Loopback suppression: when this apply pass reflects entries // from upstream, the writeFile/mkdir/symlink/rm calls inside // bumped vfs_meta.rev. Without this advance, the next push tick @@ -226,7 +340,12 @@ export async function applyChanges( // In the unsafe case we leave pushRev alone. The next pushOnce // drains both the unpushed locals and the apply's own bumps; // the receiver's alreadyApplied() check suppresses the latter. - // One redundant round-trip per apply, bounded. + // Normal loopback suppression still catches the common case by + // advancing pushRev after the upstream apply. When pending local + // writes make that unsafe, the apply's rev bumps may be sent back + // once, but the origin sees matching live state in alreadyApplied() + // and drops them without another bump. That makes directory-entry + // re-application a bounded echo rather than an unbounded ping-pong. if (options.source === "upstream") { const revAfter = currentRev(db); const existing = readWatermark(db, "pushRev", options.backend); @@ -291,13 +410,14 @@ export function applyChangesSync( continue; } if (entry.kind === "dir") { - mkdir(db, entry.path, { mode: entry.mode, recursive: true }, () => entry.mtime); + applyDirectoryEntry(db, entry); applied++; pathsInBatch++; if (pathsInBatch >= maxPaths) flush(); continue; } if (entry.kind === "symlink") { + removeReplaceableFinalEntry(db, entry.path, "symlink"); symlink(db, entry.target, entry.path, () => entry.mtime); applied++; pathsInBatch++; @@ -328,6 +448,7 @@ export function applyChangesSync( buf.set(p, off); off += p.byteLength; } + removeReplaceableFinalEntry(db, entry.path, "file"); writeFileSync(db, entry.path, buf, { mode: entry.mode }, () => entry.mtime); applied++; bytesInBatch += total; @@ -335,13 +456,6 @@ export function applyChangesSync( if (bytesInBatch >= maxBytes || pathsInBatch >= maxPaths) flush(); } - if (options.advanceFetchRev !== undefined) { - const current = readWatermark(db, "fetchRev", options.backend); - if (options.advanceFetchRev > current) { - writeWatermark(db, "fetchRev", options.advanceFetchRev, options.backend); - } - } - if (options.source === "upstream") { const revAfter = currentRev(db); const existing = readWatermark(db, "pushRev", options.backend); @@ -354,12 +468,8 @@ export function applyChangesSync( } // Compare an entry against the local node graph. Returns true when -// the entry would be a no-op apply: the manifest hash (files), -// mode + symlink target (symlinks), or mode (dirs) already matches. -// We deliberately skip mtime comparison — mtime is metadata -// the source decides on, and re-applying it would still bump the -// local rev counter for nothing. Receivers see eventual mtime -// drift between peers; the wire stays quiet. +// the entry would be a no-op apply: the manifest hash (files), mode +// (dirs), or mode + symlink target (symlinks) already matches. function alreadyApplied(db: Database, entry: Exclude): boolean { const live = resolveInode(db, entry.path, { followSymlinks: false }); if (live === null) return false; diff --git a/packages/dofs/src/sync/coalesce.test.ts b/packages/dofs/src/sync/coalesce.test.ts index 38429820..3bbc9749 100644 --- a/packages/dofs/src/sync/coalesce.test.ts +++ b/packages/dofs/src/sync/coalesce.test.ts @@ -1,10 +1,14 @@ import { describe, expect, it } from "vitest"; +import { link } from "../fs/link.js"; import { mkdir } from "../fs/mkdir.js"; +import { rename } from "../fs/rename.js"; import { rm } from "../fs/rm.js"; +import { symlink } from "../fs/symlink.js"; import { withDB } from "../fs/with-db.js"; -import { writeFile } from "../fs/writeFile.js"; +import { writeFile, writeFileSync } from "../fs/writeFile.js"; import { coalesceChanges } from "./coalesce.js"; +import { currentRev } from "./watermarks.js"; // Drain an async iterable into an array. Tests stay synchronous-looking // while the production code can stream. @@ -22,7 +26,7 @@ describe("coalesceChanges", () => { }); }); - it("yields one entry per touched path since sinceRev", async () => { + it("yields one entry per touched path after the cursor", async () => { await withDB(async (db) => { mkdir(db, "/d", { mode: 0o755 }, () => 1); await writeFile(db, "/d/a.txt", "alpha", {}, () => 2); @@ -57,6 +61,28 @@ describe("coalesceChanges", () => { }); }); + it("emits resolved delete paths for removes through intermediate symlinks", async () => { + await withDB(async (db) => { + mkdir(db, "/real", {}, () => 1); + await writeFile(db, "/real/file.txt", "x", {}, () => 2); + symlink(db, "/real", "/link", () => 3); + + rm(db, "/link/file.txt", {}); + + const entries = await drain(coalesceChanges(db, 0)); + expect(entries).toContainEqual({ + kind: "delete", + rev: expect.any(Number), + path: "/real/file.txt", + }); + expect(entries).not.toContainEqual({ + kind: "delete", + rev: expect.any(Number), + path: "/link/file.txt", + }); + }); + }); + it("delete-then-recreate yields a single live entry, not a delete", async () => { await withDB(async (db) => { await writeFile(db, "/x.txt", "first", {}, () => 1); @@ -69,7 +95,7 @@ describe("coalesceChanges", () => { }); }); - it("sinceRev filters out changes the receiver has already seen", async () => { + it("cursor rev filters out changes the receiver has already seen", async () => { await withDB(async (db) => { await writeFile(db, "/old.txt", "old", {}, () => 1); // Read current rev counter to use as the cursor. @@ -102,6 +128,123 @@ describe("coalesceChanges", () => { } }); }); + + it("resumes after a path within the same rev", async () => { + await withDB(async (db) => { + mkdir(db, "/src", {}, () => 1); + await writeFile(db, "/src/a.txt", "a", {}, () => 2); + await writeFile(db, "/src/b.txt", "b", {}, () => 3); + const beforeRename = currentRev(db); + + rename(db, "/src", "/dst"); + const renameRev = currentRev(db); + expect(renameRev).toBeGreaterThan(beforeRename); + + const allSameRev = await drain(coalesceChanges(db, { rev: beforeRename, path: null })); + const paths = allSameRev.map((entry) => entry.path); + expect(paths).toEqual([...paths].sort()); + expect(new Set(allSameRev.map((entry) => entry.rev))).toEqual(new Set([renameRev])); + + const resumed = await drain(coalesceChanges(db, { rev: renameRev, path: "/dst/a.txt" })); + expect(resumed.map((entry) => entry.path)).toEqual( + paths.filter((path) => path > "/dst/a.txt"), + ); + }); + }); + + it("skips a whole rev when the cursor path is null", async () => { + await withDB(async (db) => { + mkdir(db, "/src", {}, () => 1); + await writeFile(db, "/src/a.txt", "a", {}, () => 2); + const beforeRename = currentRev(db); + + rename(db, "/src", "/dst"); + const renameRev = currentRev(db); + expect(await drain(coalesceChanges(db, { rev: beforeRename, path: null }))).not.toEqual([]); + expect(await drain(coalesceChanges(db, { rev: renameRev, path: null }))).toEqual([]); + }); + }); + + it("orders entries deterministically by rev then path", async () => { + await withDB(async (db) => { + mkdir(db, "/src", {}, () => 1); + await writeFile(db, "/src/b.txt", "b", {}, () => 2); + await writeFile(db, "/src/a.txt", "a", {}, () => 3); + rename(db, "/src", "/dst"); + + const entries = await drain(coalesceChanges(db, { rev: 0, path: null })); + const pairs = entries.map((entry) => [entry.rev, entry.path] as const); + expect(pairs).toEqual( + [...pairs].sort((a, b) => { + if (a[0] !== b[0]) return a[0] - b[0]; + return a[1].localeCompare(b[1]); + }), + ); + }); + }); + + it("excludes entries newer than the through rev", async () => { + await withDB(async (db) => { + await writeFile(db, "/included.txt", "included", {}, () => 1); + const throughRev = currentRev(db); + await writeFile(db, "/excluded.txt", "excluded", {}, () => 2); + + const entries = await drain( + coalesceChanges(db, { rev: 0, path: null }, { through: { rev: throughRev, path: null } }), + ); + + expect(entries.map((entry) => entry.path)).toContain("/included.txt"); + expect(entries.map((entry) => entry.path)).not.toContain("/excluded.txt"); + }); + }); + + it("excludes same-rev entries after the through path", async () => { + await withDB(async (db) => { + mkdir(db, "/src", {}, () => 1); + await writeFile(db, "/src/a.txt", "a", {}, () => 2); + await writeFile(db, "/src/b.txt", "b", {}, () => 3); + const beforeRename = currentRev(db); + + rename(db, "/src", "/dst"); + const renameRev = currentRev(db); + + const entries = await drain( + coalesceChanges( + db, + { rev: beforeRename, path: null }, + { + through: { rev: renameRev, path: "/dst/a.txt" }, + }, + ), + ); + + expect(entries.map((entry) => entry.path)).toEqual(["/dst", "/dst/a.txt"]); + }); + }); + + it("skips an entry that materializes beyond the through cursor", async () => { + await withDB(async (db) => { + await writeFile(db, "/file.txt", "first", {}, () => 1); + const throughRev = currentRev(db); + + const originalOne = db.one.bind(db); + let rewrote = false; + db.one = ((query: string, ...bindings: unknown[]) => { + if (!rewrote && query === "SELECT rev FROM vfs_nodes WHERE inode = ?") { + rewrote = true; + writeFileSync(db, "/file.txt", new TextEncoder().encode("second"), {}, () => 2); + } + return originalOne(query, ...bindings); + }) as typeof db.one; + + const entries = await drain( + coalesceChanges(db, { rev: 0, path: null }, { through: { rev: throughRev, path: null } }), + ); + + expect(rewrote).toBe(true); + expect(entries).toEqual([]); + }); + }); }); describe("coalesceChanges (ignore)", () => { @@ -133,4 +276,67 @@ describe("coalesceChanges (ignore)", () => { expect(entries.some((e) => e.path === "/node_modules/x.js")).toBe(true); }); }); + + it("emits an entry for every hardlink name of a touched inode", async () => { + await withDB(async (db) => { + await writeFile(db, "/a", "shared", {}, () => 1); + link(db, "/a", "/b"); + + const entries = await drain(coalesceChanges(db, 0)); + const files = entries.filter((e) => e.kind === "file").map((e) => e.path); + // Both names share one inode; pathOf would pick only one of them. + // The wire has to carry both so the receiver materialises each. + expect(files).toContain("/a"); + expect(files).toContain("/b"); + }); + }); + + it("defers a path whose live state raced past the snapshot, then redelivers it", async () => { + // Pins the documented cursor contract (docs/02_sync_protocol.md): + // `through` is a resume bound, not a point-in-time snapshot. A path + // deleted at the snapshot rev but recreated above it is dropped from + // the bounded scan, because materialiseChange reads the live state + // and inCursorWindow filters it out. The omission is not loss — the + // recreate's rev is above the snapshot, so the next scan redelivers + // the path. Convergence holds without the store keeping history. + await withDB(async (db) => { + await writeFile(db, "/x", "v1", {}, () => 1); + rm(db, "/x", {}); + const snapshot = currentRev(db); + await writeFile(db, "/x", "v2", {}, () => 2); + + // Bounded by the snapshot: the rev-`snapshot` delete is a + // candidate, but the live entry now sits above the window, so /x + // is omitted from this snapshot rather than frozen at the delete. + const bounded = await drain( + coalesceChanges(db, 0, { through: { rev: snapshot, path: null } }), + ); + expect(bounded.some((e) => e.path === "/x")).toBe(false); + + // The next scan resumes after the snapshot rev and redelivers /x + // at its current state. The rev that caused the drop is above the + // snapshot, so a later window always covers it. + const next = await drain(coalesceChanges(db, snapshot)); + expect(next.find((e) => e.path === "/x")).toMatchObject({ kind: "file", path: "/x" }); + }); + }); + + it("emits the new name when a hardlinked file is renamed", async () => { + await withDB(async (db) => { + await writeFile(db, "/a", "shared", {}, () => 1); + link(db, "/a", "/b"); + // /a and /b now share an inode. Renaming /a to /c leaves the + // inode named /b and /c; pathOf might resolve it to /b and never + // emit /c, dropping the renamed name on the wire. + const baseline = currentRev(db); + rename(db, "/a", "/c"); + + const entries = await drain(coalesceChanges(db, baseline)); + const live = entries.filter((e) => e.kind !== "delete").map((e) => e.path); + const deletes = entries.filter((e) => e.kind === "delete").map((e) => e.path); + expect(live).toContain("/c"); + expect(live).toContain("/b"); + expect(deletes).toContain("/a"); + }); + }); }); diff --git a/packages/dofs/src/sync/coalesce.ts b/packages/dofs/src/sync/coalesce.ts index 6dabb005..bfd4b304 100644 --- a/packages/dofs/src/sync/coalesce.ts +++ b/packages/dofs/src/sync/coalesce.ts @@ -1,35 +1,10 @@ -import { ROOT_INODE } from "../schema/index.js"; import type { Database } from "../storage.js"; import { type ChangeEntry, materialiseChange } from "./changes.js"; import { isIgnored } from "./ignore.js"; +import { pathsOf } from "./paths.js"; +import { type ChangeCursor, compareChangeCursors } from "./watermarks.js"; -// Walk vfs_dirents from `inode` up to ROOT_INODE, gathering the path -// segments along the way. Returns null when the inode is unreachable -// (orphan after a partially-applied rm; should not happen inside a -// healthy DB but the caller treats null as "skip this entry"). -function pathOf(db: Database, inode: number): string | null { - if (inode === ROOT_INODE) return "/"; - const segments: string[] = []; - let current = inode; - // Bound the walk: a million levels deep is well past any real FS; - // anything beyond that is corruption and should not loop forever. - for (let i = 0; i < 1_000_000; i++) { - const row = db.one<{ parent_inode: number; name: string }>( - "SELECT parent_inode, name FROM vfs_dirents WHERE child_inode = ?", - current, - ); - if (row === undefined) return null; - segments.push(row.name); - if (row.parent_inode === ROOT_INODE) { - segments.reverse(); - return `/${segments.join("/")}`; - } - current = row.parent_inode; - } - return null; -} - -// Yield one ChangeEntry per path touched since `sinceRev`. Per-path +// Yield one ChangeEntry per path touched after `after`. Per-path // coalescing: five rewrites of the same path between watermarks // produce one entry (the latest state wins). Tombstoned paths get a // delete entry unless they have been recreated, in which case the @@ -47,14 +22,24 @@ export interface CoalesceOptions { // Path-segment patterns to drop before yielding. The wire never // carries entries under an ignored segment. ignore?: string[]; + // Internal snapshot bound. When set, entries are limited to the + // cursor window `after < entry <= through`. The bound is applied to + // each path's *live* rev at materialise time, not to a frozen + // snapshot: a path whose state moves past `through` after the scan + // (a concurrent rewrite or delete) is dropped here and redelivered + // under a later cursor, since the cursor never advances past the rev + // that caused the drop. See the redelivery note on the yield loop. + through?: ChangeCursor; } export async function* coalesceChanges( db: Database, - sinceRev: number, + after: ChangeCursor | number, options: CoalesceOptions = {}, ): AsyncIterable { const ignore = options.ignore ?? []; + const cursor = typeof after === "number" ? { rev: after, path: null } : after; + const through = options.through; // Build the per-path candidate set in two passes, keeping the // highest rev seen for each path. A live mutation that landed @@ -66,28 +51,48 @@ export async function* coalesceChanges( // Live mutations: every mkdir / writeFile / symlink bumps // vfs_nodes.rev. The by_rev index makes this a range scan. - const touched = db.all<{ inode: number; rev: number }>( - "SELECT inode, rev FROM vfs_nodes WHERE rev > ? ORDER BY rev", - sinceRev, - ); + const lowerRev = cursor.path === null ? cursor.rev : cursor.rev - 1; + const touched = + through === undefined + ? db.all<{ inode: number; rev: number }>( + "SELECT inode, rev FROM vfs_nodes WHERE rev > ? ORDER BY rev", + lowerRev, + ) + : db.all<{ inode: number; rev: number }>( + "SELECT inode, rev FROM vfs_nodes WHERE rev > ? AND rev <= ? ORDER BY rev", + lowerRev, + through.rev, + ); for (const { inode, rev } of touched) { - const path = pathOf(db, inode); - if (path === null) continue; - if (isIgnored(path, ignore)) continue; - const prior = candidates.get(path); - if (prior === undefined || rev > prior.rev) { - candidates.set(path, { path, rev }); + // One inode can carry several hardlink names; every name has to + // become a candidate so the wire materialises each, not just the + // arbitrary one pathOf would return. + for (const path of pathsOf(db, inode)) { + if (!inCursorWindow({ rev, path }, cursor, through)) continue; + if (isIgnored(path, ignore)) continue; + const prior = candidates.get(path); + if (prior === undefined || rev > prior.rev) { + candidates.set(path, { path, rev }); + } } } // Tombstones: each rm appends a row to vfs_changes with the // post-bump rev. The highest rev per path wins (a path can be // deleted-recreated-deleted; we want the last rm's rev). - const tombs = db.all<{ path: string; rev: number }>( - "SELECT path, MAX(rev) AS rev FROM vfs_changes WHERE rev > ? AND op = 'delete' GROUP BY path", - sinceRev, - ); + const tombs = + through === undefined + ? db.all<{ path: string; rev: number }>( + "SELECT path, MAX(rev) AS rev FROM vfs_changes WHERE rev > ? AND op = 'delete' GROUP BY path", + lowerRev, + ) + : db.all<{ path: string; rev: number }>( + "SELECT path, MAX(rev) AS rev FROM vfs_changes WHERE rev > ? AND rev <= ? AND op = 'delete' GROUP BY path", + lowerRev, + through.rev, + ); for (const { path, rev } of tombs) { + if (!inCursorWindow({ rev, path }, cursor, through)) continue; if (isIgnored(path, ignore)) continue; const prior = candidates.get(path); if (prior === undefined || rev > prior.rev) { @@ -103,8 +108,34 @@ export async function* coalesceChanges( return a.path < b.path ? -1 : a.path > b.path ? 1 : 0; }); + // materialiseChange reads each path's *current* state, not the state + // it held at the candidate's rev — the VFS keeps no content history. + // If a path was rewritten or deleted again after the scan, its live + // rev can now sit above `through`; inCursorWindow drops it. The + // dropped change is not lost: the rev that pushed it past `through` + // is, by definition, greater than `through.rev` (the cursor the + // puller persists), so the next pull's `after < entry` scan finds the + // path again and delivers its then-current state. The consequence is + // that a `{rev, null}` cursor means "every change committed at or + // before rev has been *offered*"; it does not guarantee the receiver + // tree byte-matches the rev snapshot for a path that raced ahead. + // Convergence is preserved because the cursor never advances past the + // racing rev. See docs/02_sync_protocol.md. for (const { path } of ordered) { const entry = materialiseChange(db, path); - if (entry !== null) yield entry; + if (entry !== null && inCursorWindow(entry, cursor, through)) { + yield entry; + } } } + +function inCursorWindow( + entry: ChangeCursor & { path: string }, + after: ChangeCursor, + through?: ChangeCursor, +): boolean { + return ( + compareChangeCursors(entry, after) > 0 && + (through === undefined || compareChangeCursors(entry, through) <= 0) + ); +} diff --git a/packages/dofs/src/sync/fetch.test.ts b/packages/dofs/src/sync/fetch.test.ts index 9ef3eaec..8d36c08d 100644 --- a/packages/dofs/src/sync/fetch.test.ts +++ b/packages/dofs/src/sync/fetch.test.ts @@ -38,6 +38,17 @@ describe("fetch wire", () => { }); }); + it("fetchChanges resumes from a rev/path cursor", async () => { + await withDB(async (db) => { + await writeFile(db, "/a.txt", "alpha", {}, () => 1); + await writeFile(db, "/b.txt", "beta", {}, () => 2); + const entries = await drain(fetchChanges(db, { rev: 0, path: null })); + const first = entries[0]; + const resumed = await drain(fetchChanges(db, { rev: first.rev, path: first.path })); + expect(resumed).toEqual(entries.slice(1)); + }); + }); + it("fetchObjects yields each hash exactly once", async () => { await withDB(async (db) => { await writeFile(db, "/a.txt", "shared", {}, () => 1); diff --git a/packages/dofs/src/sync/fetch.ts b/packages/dofs/src/sync/fetch.ts index f722a06d..9d4e3c07 100644 --- a/packages/dofs/src/sync/fetch.ts +++ b/packages/dofs/src/sync/fetch.ts @@ -2,6 +2,7 @@ import type { Database } from "../storage.js"; import type { ChangeEntry } from "./changes.js"; import { coalesceChanges } from "./coalesce.js"; import { pushObjects } from "./push.js"; +import type { ChangeCursor } from "./watermarks.js"; // The fetch wire is the mirror of the push wire: same SQL, // opposite direction. The DO calls fetchChanges / fetchObjects on @@ -10,10 +11,10 @@ import { pushObjects } from "./push.js"; export function fetchChanges( db: Database, - sinceRev: number, + after: ChangeCursor | number, options: { ignore?: string[] } = {}, ): AsyncIterable { - return coalesceChanges(db, sinceRev, options); + return coalesceChanges(db, after, options); } export function fetchObjects( diff --git a/packages/dofs/src/sync/invariant.test.ts b/packages/dofs/src/sync/invariant.test.ts index 65a80bf4..7e40d9f3 100644 --- a/packages/dofs/src/sync/invariant.test.ts +++ b/packages/dofs/src/sync/invariant.test.ts @@ -1,18 +1,32 @@ import { describe, expect, it } from "vitest"; -import { assertAppliedPushRev } from "./invariant.js"; +import { assertAppliedPushCursor } from "./invariant.js"; -describe("assertAppliedPushRev", () => { - it("passes when applied >= pushed", () => { - expect(() => assertAppliedPushRev(10, 10)).not.toThrow(); - expect(() => assertAppliedPushRev(11, 10)).not.toThrow(); +describe("assertAppliedPushCursor", () => { + it("passes when applied covers pushed", () => { + expect(() => + assertAppliedPushCursor({ rev: 10, path: null }, { rev: 10, path: null }), + ).not.toThrow(); + expect(() => + assertAppliedPushCursor({ rev: 11, path: "/partial.txt" }, { rev: 10, path: null }), + ).not.toThrow(); }); it("passes at zero", () => { - expect(() => assertAppliedPushRev(0, 0)).not.toThrow(); + expect(() => + assertAppliedPushCursor({ rev: 0, path: null }, { rev: 0, path: null }), + ).not.toThrow(); }); - it("throws when the container is behind the DO's push watermark", () => { - expect(() => assertAppliedPushRev(5, 10)).toThrowError(/appliedPushRev.*5.*pushRev.*10/i); + it("throws when the receiver only partially applied the pushed rev", () => { + expect(() => + assertAppliedPushCursor({ rev: 10, path: "/partial.txt" }, { rev: 10, path: null }), + ).toThrowError(/appliedPushCursor.*pushCursor/i); + }); + + it("throws when the receiver is behind the sender's push cursor", () => { + expect(() => + assertAppliedPushCursor({ rev: 5, path: null }, { rev: 10, path: null }), + ).toThrowError(/appliedPushCursor.*pushCursor/i); }); }); diff --git a/packages/dofs/src/sync/invariant.ts b/packages/dofs/src/sync/invariant.ts index dfdf04f6..036fd0e7 100644 --- a/packages/dofs/src/sync/invariant.ts +++ b/packages/dofs/src/sync/invariant.ts @@ -1,22 +1,29 @@ +import { type ChangeCursor, compareChangeCursors } from "./watermarks.js"; + // Cross-side invariant: every fetchChanges and push response carries -// the container's current appliedPushRev. The DO asserts -// appliedPushRev >= pushRev on every response. +// the receiver's current applied push cursor. The sender asserts that +// cursor covers its local push cursor on every response. // // The two sides never share a single clock, but echoing the largest -// applied DO rev makes the "container is caught up with the DO's +// applied sender cursor makes the "receiver is caught up with our // pushes" invariant inspectable on the wire instead of load-bearing -// in-process state. A regression in the suppress-dirty-tracking -// apply path trips the assertion immediately rather than corrupting -// data silently. +// in-process state. A regression in the suppress-dirty-tracking apply +// path trips the assertion immediately rather than corrupting data +// silently. // // Throwing an Error is the right escalation: a violation means the // protocol is broken; the connection should tear down and rebuild // rather than soldiering on with stale state. -export function assertAppliedPushRev(appliedPushRev: number, pushRev: number): void { - if (appliedPushRev < pushRev) { +export function assertAppliedPushCursor( + appliedPushCursor: ChangeCursor, + pushCursor: ChangeCursor, +): void { + if (compareChangeCursors(appliedPushCursor, pushCursor) < 0) { throw new Error( - `cross-side invariant violated: appliedPushRev (${appliedPushRev}) < pushRev (${pushRev})`, + `cross-side invariant violated: appliedPushCursor (${JSON.stringify( + appliedPushCursor, + )}) < pushCursor (${JSON.stringify(pushCursor)})`, ); } } diff --git a/packages/dofs/src/sync/paths.ts b/packages/dofs/src/sync/paths.ts new file mode 100644 index 00000000..d68a1e14 --- /dev/null +++ b/packages/dofs/src/sync/paths.ts @@ -0,0 +1,46 @@ +import { ROOT_INODE } from "../schema/index.js"; +import type { Database } from "../storage.js"; + +// Walk vfs_dirents from `inode` up to ROOT_INODE, gathering the path +// segments along the way. Returns null when the inode is unreachable. +export function pathOf(db: Database, inode: number): string | null { + if (inode === ROOT_INODE) return "/"; + const segments: string[] = []; + let current = inode; + // Bound the walk: a million levels deep is well past any real FS; + // anything beyond that is corruption and should not loop forever. + for (let i = 0; i < 1_000_000; i++) { + const row = db.one<{ parent_inode: number; name: string }>( + "SELECT parent_inode, name FROM vfs_dirents WHERE child_inode = ?", + current, + ); + if (row === undefined) return null; + segments.push(row.name); + if (row.parent_inode === ROOT_INODE) { + segments.reverse(); + return `/${segments.join("/")}`; + } + current = row.parent_inode; + } + return null; +} + +// Every path that currently names `inode`. A file may carry several +// hardlink names; pathOf collapses them to one arbitrary name, which +// is wrong for the change stream — every name has to reach the wire so +// the receiver materialises each. Directories cannot be hardlinked, so +// each parent walk is unambiguous. +export function pathsOf(db: Database, inode: number): string[] { + if (inode === ROOT_INODE) return ["/"]; + const dirents = db.all<{ parent_inode: number; name: string }>( + "SELECT parent_inode, name FROM vfs_dirents WHERE child_inode = ?", + inode, + ); + const paths: string[] = []; + for (const { parent_inode, name } of dirents) { + const parent = pathOf(db, parent_inode); + if (parent === null) continue; + paths.push(parent === "/" ? `/${name}` : `${parent}/${name}`); + } + return paths; +} diff --git a/packages/dofs/src/sync/watermarks.test.ts b/packages/dofs/src/sync/watermarks.test.ts index e6c380c2..7b9213b0 100644 --- a/packages/dofs/src/sync/watermarks.test.ts +++ b/packages/dofs/src/sync/watermarks.test.ts @@ -2,13 +2,19 @@ import { describe, expect, it } from "vitest"; import { withDB } from "../fs/with-db.js"; import { writeFile } from "../fs/writeFile.js"; -import { currentRev, readWatermark, writeWatermark } from "./watermarks.js"; +import { + compareChangeCursors, + currentRev, + readFetchCursor, + readWatermark, + writeFetchCursor, + writeWatermark, +} from "./watermarks.js"; describe("watermarks", () => { it("readWatermark returns 0 for a fresh DB", async () => { await withDB(async (db) => { expect(readWatermark(db, "pushRev")).toBe(0); - expect(readWatermark(db, "fetchRev")).toBe(0); }); }); @@ -16,12 +22,61 @@ describe("watermarks", () => { await withDB(async (db) => { writeWatermark(db, "pushRev", 42); expect(readWatermark(db, "pushRev")).toBe(42); - expect(readWatermark(db, "fetchRev")).toBe(0); - writeWatermark(db, "fetchRev", 7); - expect(readWatermark(db, "fetchRev")).toBe(7); }); }); + it("persists the fetch cursor rev and path separately", async () => { + await withDB(async (db) => { + expect(readFetchCursor(db)).toEqual({ rev: 0, path: null }); + writeFetchCursor(db, { rev: 12, path: "/dir/file.txt" }); + expect(readFetchCursor(db)).toEqual({ rev: 12, path: "/dir/file.txt" }); + writeFetchCursor(db, { rev: 13, path: null }); + expect(readFetchCursor(db)).toEqual({ rev: 13, path: null }); + }); + }); + + it("does not persist an intermediate full-rev cursor when a partial cursor write fails", async () => { + await withDB(async (db) => { + writeFetchCursor(db, { rev: 12, path: null }); + + const originalRun = db.run.bind(db); + db.run = ((query: string, ...bindings: unknown[]) => { + if (query.includes("_vfs_fetch_cursor")) { + throw new Error("forced cursor path failure"); + } + return originalRun(query, ...bindings); + }) as typeof db.run; + + expect(() => writeFetchCursor(db, { rev: 13, path: "/partial.txt" })).toThrow( + "forced cursor path failure", + ); + expect(readFetchCursor(db)).toEqual({ rev: 12, path: null }); + }); + }); + + it("returns a fresh start cursor for a zero fetchRev", async () => { + await withDB(async (db) => { + const cursor = readFetchCursor(db); + cursor.rev = 99; + cursor.path = "/mutated.txt"; + + expect(readFetchCursor(db)).toEqual({ rev: 0, path: null }); + }); + }); + + it("orders partial cursors before full same-rev cursors", () => { + expect(compareChangeCursors({ rev: 0, path: null }, { rev: 0, path: null })).toBe(0); + expect(compareChangeCursors({ rev: 12, path: null }, { rev: 12, path: "/partial.txt" })).toBe( + 1, + ); + expect(compareChangeCursors({ rev: 12, path: "/partial.txt" }, { rev: 12, path: null })).toBe( + -1, + ); + expect(compareChangeCursors({ rev: 13, path: "/partial.txt" }, { rev: 12, path: null })).toBe( + 1, + ); + }); + it("watermarks advance monotonically (the caller enforces this)", async () => { await withDB(async (db) => { writeWatermark(db, "pushRev", 5); @@ -48,9 +103,8 @@ describe("watermarks", () => { }); it("rejects unknown watermark keys at the type level via the helper signature", () => { - // Compile-time only: writeWatermark only accepts the union - // "pushRev" | "fetchRev". This test is a placeholder that - // documents the contract; the type system catches misuse. + // Compile-time only: writeWatermark only accepts "pushRev". + // Fetch progress must go through readFetchCursor/writeFetchCursor. expect(true).toBe(true); }); diff --git a/packages/dofs/src/sync/watermarks.ts b/packages/dofs/src/sync/watermarks.ts index 96858775..084c3c68 100644 --- a/packages/dofs/src/sync/watermarks.ts +++ b/packages/dofs/src/sync/watermarks.ts @@ -1,27 +1,38 @@ import type { Database } from "../storage.js"; -// Watermarks owned by the DO. Keyed by (k, backend) so a single -// workspace can host more than one backend and each keeps its own -// sync cursors. The container's appliedPushRev lives in-memory on +// Watermarks owned by the local database. Keyed by (k, backend) so a +// single workspace can host more than one backend and each keeps its +// own sync cursors. The container's appliedPushRev lives in-memory on // the container side; we don't store it here. // -// pushRev — last DO-side rev successfully pushed to the backend. -// fetchRev — last backend-side rev the DO has fetched and applied. +// pushRev — last DO-side rev successfully pushed to the backend. // -// initializeSchema() seeds both at 0 in _vfs_watermark for the +// initializeSchema() seeds pushRev at 0 in _vfs_watermark for the // default backend. The schema table is the durability surface; // readers and writers always go through this module so the SQL // stays in one place. // // `backend` defaults to DEFAULT_BACKEND_ID so older callers that // only ran one backend (or ran the package against a schema before -// per-backend keying landed) keep working unchanged. The v2 → v3 +// per-backend keying landed) keep working unchanged. The v3 → v4 // schema migration backfills the column on existing rows with the // same default. -export type WatermarkKey = "pushRev" | "fetchRev"; +// +// Fetch progress is a `{ rev, path }` cursor. Its rev component is +// still stored in _vfs_watermark for schema compatibility, but callers +// must use readFetchCursor() / writeFetchCursor() so rev and path stay +// consistent. +export type WatermarkKey = "pushRev"; export const DEFAULT_BACKEND_ID = "default"; +// Cursor into the remote change stream. `path: null` means `rev` +// is fully drained and the next fetch resumes strictly after that +// rev. A string path means resume inside the same rev after that +// path. The empty string is a real path value, not a sentinel, and +// must not be used to mean "start of rev". +export type ChangeCursor = { rev: number; path: string | null }; + export function readWatermark( db: Database, key: WatermarkKey, @@ -32,9 +43,19 @@ export function readWatermark( ); } -export function writeWatermark( +function readFetchRev(db: Database, backend: string = DEFAULT_BACKEND_ID): number { + return ( + db.scalar( + "SELECT v FROM _vfs_watermark WHERE k = ? AND backend = ?", + "fetchRev", + backend, + ) ?? 0 + ); +} + +function writeWatermarkValue( db: Database, - key: WatermarkKey, + key: WatermarkKey | "fetchRev", value: number, backend: string = DEFAULT_BACKEND_ID, ): void { @@ -47,10 +68,63 @@ export function writeWatermark( ); } +function writeFetchCursorPath( + db: Database, + path: string | null, + backend: string = DEFAULT_BACKEND_ID, +): void { + db.run( + "INSERT INTO _vfs_fetch_cursor (k, backend, path) VALUES (?, ?, ?) " + + "ON CONFLICT(k, backend) DO UPDATE SET path = excluded.path", + "fetch", + backend, + path, + ); +} + +export function writeWatermark( + db: Database, + key: WatermarkKey, + value: number, + backend: string = DEFAULT_BACKEND_ID, +): void { + writeWatermarkValue(db, key, value, backend); +} + +export function readFetchCursor(db: Database, backend: string = DEFAULT_BACKEND_ID): ChangeCursor { + const rev = readFetchRev(db, backend); + if (rev === 0) return { rev: 0, path: null }; + const path = db.scalar( + "SELECT path FROM _vfs_fetch_cursor WHERE k = ? AND backend = ?", + "fetch", + backend, + ); + return { rev, path: path ?? null }; +} + +export function writeFetchCursor( + db: Database, + cursor: ChangeCursor, + backend: string = DEFAULT_BACKEND_ID, +): void { + db.transactionSync(() => { + writeWatermarkValue(db, "fetchRev", cursor.rev, backend); + writeFetchCursorPath(db, cursor.path, backend); + }); +} + +export function compareChangeCursors(a: ChangeCursor, b: ChangeCursor): number { + if (a.rev !== b.rev) return a.rev - b.rev; + if (a.path === b.path) return 0; + if (a.path === null) return 1; + if (b.path === null) return -1; + return a.path < b.path ? -1 : 1; +} + // The latest rev stamped on any DO-side mutation. coalesceChanges // reads this implicitly via vfs_nodes.rev; the sync layer exposes it -// to callers that want to record "what cursor should I pass back as -// sinceRev next time". +// to callers that want to record the rev component of their next +// cursor. export function currentRev(db: Database): number { return db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; } diff --git a/packages/rpc/src/interface.ts b/packages/rpc/src/interface.ts index 9d9b4dee..a618b509 100644 --- a/packages/rpc/src/interface.ts +++ b/packages/rpc/src/interface.ts @@ -17,39 +17,37 @@ // WorkspaceRPC, so the wire stub exposes one stable surface while // the two halves stay internally separable. -import type { ChangeEntry } from "@cloudflare/dofs"; +import type { ChangeCursor, ChangeEntry } from "@cloudflare/dofs"; export interface SyncRPC { // DO → container. Stream a coalesced batch of changes. Bytes are // not inline: the DO sends ChangeEntry records with chunk hashes, // the container calls back via hasObjects / asks for the missing // bytes through pushObjects. `senderRev` is the sender's - // currentRev at the moment it captured the batch — the - // receiver advances its fetchRev to this value after the apply - // settles, and echoes it back as `appliedPushRev` so the sender - // can assert applied ≥ pushed on every response. + // currentRev at the moment it captured the batch. The receiver + // advances its fetch cursor to this completed rev after the apply + // settles, and echoes that cursor back as `appliedPushCursor` so + // the sender can assert applied covers pushed on every response. push(input: { senderRev: number; changes: ReadableStream }): Promise<{ rev: number; - appliedPushRev: number; + appliedPushCursor: ChangeCursor; }>; - // Container ← DO. Stream every ChangeEntry with rev > sinceRev, - // alongside two scalars used to drive the pull: + // Container ← DO. Stream every ChangeEntry after the supplied cursor, + // alongside cursors used to drive the pull: // - // currentRev — the receiver's currentRev captured at the - // start of the stream. The puller advances - // fetchRev no further than this on any pull - // so the watermark stays consistent with what - // the stream actually carried. - // appliedPushRev — the largest senderRev the receiver has - // fully applied. The puller asserts - // appliedPushRev >= local.pushRev before - // draining, mirroring the same check on push. + // currentCursor — the receiver's currentRev captured at the + // start of the stream, with path=null to mark + // the whole rev complete after a clean drain. + // appliedPushCursor — the receiver's cursor for sender changes + // it has applied. The puller asserts this + // covers local pushRev before draining, + // mirroring the same check on push. // // Per-file entries carry (hash, size) chunk lists; no bytes inline. - fetchChanges(input: { sinceRev?: number; ignore?: string[] }): Promise<{ - currentRev: number; - appliedPushRev: number; + fetchChanges(input: { after?: ChangeCursor; ignore?: string[] }): Promise<{ + currentCursor: ChangeCursor; + appliedPushCursor: ChangeCursor; stream: ReadableStream; }>; @@ -60,11 +58,12 @@ export interface SyncRPC { // // currentRev — latest rev stamped on any local mutation. // pushRev — highest rev already shipped to the upstream. - // fetchRev — highest upstream rev applied locally. + // fetchCursor — upstream fetch cursor, including same-rev path + // progress. // - // pushRev / fetchRev only move when the receiver is acting as + // pushRev / fetchCursor only move when the receiver is acting as // a sync peer. Otherwise they sit at 0. - watermarks(): Promise<{ currentRev: number; pushRev: number; fetchRev: number }>; + watermarks(): Promise<{ currentRev: number; pushRev: number; fetchCursor: ChangeCursor }>; // Materialise the receiver's view of a single path as a // ChangeEntry. Returns null when the path doesn't exist and diff --git a/packages/rpc/src/server.ts b/packages/rpc/src/server.ts index d7f12ac8..9deaa222 100644 --- a/packages/rpc/src/server.ts +++ b/packages/rpc/src/server.ts @@ -6,16 +6,20 @@ import { applyChangesSync, + type ChangeCursor, type ChangeEntry, coalesceChanges, + compareChangeCursors, currentRev, type Database, DEFAULT_IGNORE, fetchObjects, hasObjects, materialiseChange, + readFetchCursor, readWatermark, stageBlob, + writeFetchCursor, } from "@cloudflare/dofs"; import { newWebSocketRpcSession, nodeHttpBatchRpcResponse, RpcTarget } from "capnweb"; @@ -93,7 +97,7 @@ class SyncRPCServer extends RpcTarget implements SyncRPC { async push(input: { senderRev: number; changes: ReadableStream; - }): Promise<{ rev: number; appliedPushRev: number }> { + }): Promise<{ rev: number; appliedPushCursor: ChangeCursor }> { const entries: ChangeEntry[] = []; const reader = input.changes.getReader(); try { @@ -106,7 +110,7 @@ class SyncRPCServer extends RpcTarget implements SyncRPC { reader.releaseLock(); } // senderRev > 0 — the caller is a sync peer with its - // own rev space; advance fetchRev to that point and let + // own rev space; advance the fetch cursor to that point and let // loopback suppression silence the outbound push so we // don't ping-pong the same entries back. // @@ -125,8 +129,13 @@ class SyncRPCServer extends RpcTarget implements SyncRPC { this.db.transactionSync(() => { applyChangesSync(this.db, entries, new Map(), { source: isPeer ? "upstream" : "local", - ...(isPeer ? { advanceFetchRev: input.senderRev } : {}), }); + if (isPeer) { + const nextCursor = { rev: input.senderRev, path: null }; + if (compareChangeCursors(nextCursor, readFetchCursor(this.db)) > 0) { + writeFetchCursor(this.db, nextCursor); + } + } }); if (this.options.afterApply !== undefined && entries.length > 0) { try { @@ -140,13 +149,13 @@ class SyncRPCServer extends RpcTarget implements SyncRPC { } return { rev: currentRev(this.db), - appliedPushRev: input.senderRev, + appliedPushCursor: { rev: input.senderRev, path: null }, }; } - async fetchChanges(input: { sinceRev?: number; ignore?: string[] }): Promise<{ - currentRev: number; - appliedPushRev: number; + async fetchChanges(input: { after?: ChangeCursor; ignore?: string[] }): Promise<{ + currentCursor: ChangeCursor; + appliedPushCursor: ChangeCursor; stream: ReadableStream; }> { if (this.options.beforeFetch !== undefined) { @@ -159,16 +168,17 @@ class SyncRPCServer extends RpcTarget implements SyncRPC { console.warn("[SyncRPCServer] beforeFetch hook failed:", err); } } - const sinceRev = input.sinceRev ?? 0; + const after = input.after ?? { rev: 0, path: null }; const ignore = input.ignore ?? (this.options.ignore.length > 0 ? this.options.ignore : DEFAULT_IGNORE); - // appliedPushRev == fetchRev on the receiver: every senderRev > 0 - // push advances fetchRev to senderRev on apply, so fetchRev is - // the largest senderRev the receiver has fully applied. + const snapshotRev = currentRev(this.db); + const currentCursor = { rev: snapshotRev, path: null }; return { - currentRev: currentRev(this.db), - appliedPushRev: readWatermark(this.db, "fetchRev"), - stream: iterableToReadableStream(coalesceChanges(this.db, sinceRev, { ignore })), + currentCursor, + appliedPushCursor: readFetchCursor(this.db), + stream: iterableToReadableStream( + coalesceChanges(this.db, after, { ignore, through: currentCursor }), + ), }; } @@ -176,11 +186,11 @@ class SyncRPCServer extends RpcTarget implements SyncRPC { return materialiseChange(this.db, path); } - async watermarks(): Promise<{ currentRev: number; pushRev: number; fetchRev: number }> { + async watermarks(): Promise<{ currentRev: number; pushRev: number; fetchCursor: ChangeCursor }> { return { currentRev: currentRev(this.db), pushRev: readWatermark(this.db, "pushRev"), - fetchRev: readWatermark(this.db, "fetchRev"), + fetchCursor: readFetchCursor(this.db), }; } diff --git a/packages/rpc/src/sync-driver.bench.ts b/packages/rpc/src/sync-driver.bench.ts index 00f9ea7e..f44bd205 100644 --- a/packages/rpc/src/sync-driver.bench.ts +++ b/packages/rpc/src/sync-driver.bench.ts @@ -16,6 +16,7 @@ import { currentRev, Database, initializeSchema, + readFetchCursor, readWatermark, SQLiteWorkspaceProvider, } from "@cloudflare/dofs"; @@ -84,7 +85,7 @@ describe("sync driver — push throughput", () => { for (let i = 0; i < bytes.byteLength; i += 4096) { bytes[i] = (i * 31) & 0xff; } - provider.writeFileSync("/big.bin", bytes); + provider.writeFileSync("/big.bin", Buffer.from(bytes)); await pushOnce(a.db, b.rpc); } finally { a.close(); @@ -105,7 +106,7 @@ describe("sync driver — push throughput", () => { for (let i = 0; i < bytes.byteLength; i += 4096) { bytes[i] = (i * 31) & 0xff; } - provider.writeFileSync("/big.bin", bytes); + provider.writeFileSync("/big.bin", Buffer.from(bytes)); await pushOnce(a.db, b.rpc); } finally { a.close(); @@ -199,7 +200,7 @@ describe("sync driver — bidirectional convergence", () => { // time. Reading watermarks here adds noise we can // tolerate vs. running a no-op closure for the // baseline. Sanity assert outside the iteration body: - if (readWatermark(b.db, "fetchRev") <= 0) throw new Error("pull didn't advance"); + if (readFetchCursor(b.db).rev <= 0) throw new Error("pull didn't advance"); if (currentRev(b.db) <= 0) throw new Error("apply didn't bump rev"); } finally { a.close(); diff --git a/packages/rpc/src/sync-driver.test.ts b/packages/rpc/src/sync-driver.test.ts index 472bc55f..8b8e8e29 100644 --- a/packages/rpc/src/sync-driver.test.ts +++ b/packages/rpc/src/sync-driver.test.ts @@ -3,9 +3,11 @@ import { currentRev, Database, initializeSchema, + readFetchCursor, readWatermark, SQLiteWorkspaceProvider, stageBlob, + writeFetchCursor, writeWatermark, } from "@cloudflare/dofs"; import { SQLiteTestStorage } from "@cloudflare/dofs/testing"; @@ -33,6 +35,55 @@ function fileEntries(db: Database): string[] { .map((r) => r.name); } +async function drainStream(stream: ReadableStream): Promise { + const out: T[] = []; + const reader = stream.getReader(); + try { + while (true) { + const { value, done } = await reader.read(); + if (done) break; + out.push(value); + } + } finally { + reader.releaseLock(); + } + return out; +} + +// Wrap a SyncRPC so the fetchChanges result carries a tracked +// [Symbol.dispose]. pullOnce owns that envelope and must dispose it on +// every exit, including the throwing paths. Options inject the two +// failure modes: a lying appliedPushCursor (trips the cross-side +// invariant before the stream is read) and an empty hasObjects (forces +// applyChanges to throw on a missing object mid-stream). +function trackFetchDisposal( + rpc: SyncRPC, + opts: { lowerCursor?: boolean; failHasObjects?: boolean } = {}, +): { rpc: SyncRPC; disposeCount: () => number } { + let disposeCount = 0; + const wrapped = new Proxy(rpc as object, { + get(target, prop, receiver) { + if (prop === "fetchChanges") { + return async (...args: Parameters) => { + const real = await Reflect.get(target, prop, receiver).call(target, ...args); + return { + ...real, + ...(opts.lowerCursor ? { appliedPushCursor: { rev: 0, path: null } } : {}), + [Symbol.dispose]() { + disposeCount += 1; + }, + }; + }; + } + if (prop === "hasObjects" && opts.failHasObjects) { + return async () => []; + } + return Reflect.get(target, prop, receiver); + }, + }) as SyncRPC; + return { rpc: wrapped, disposeCount: () => disposeCount }; +} + describe("sync driver — pullOnce", () => { it("pulls a single entry from upstream", async () => { const a = makePeer(); @@ -335,6 +386,130 @@ describe("SyncRPC server — beforeFetch hook", () => { }); }); +describe("SyncRPC server — fetchChanges snapshots", () => { + it("bounds the returned stream to the advertised currentCursor", async () => { + const upstream = makePeer(); + try { + const provider = new SQLiteWorkspaceProvider(upstream.db, { now: () => 1 }); + provider.writeFileSync("/before.txt", "before"); + + const originalAll = upstream.db.all.bind(upstream.db); + let rewrote = false; + upstream.db.all = ((query: string, ...bindings: unknown[]) => { + if (!rewrote && query.startsWith("SELECT inode, rev FROM vfs_nodes WHERE rev > ?")) { + rewrote = true; + provider.writeFileSync("/after.txt", "after"); + } + return originalAll(query, ...bindings); + }) as typeof upstream.db.all; + + const result = await upstream.rpc.fetchChanges({ after: { rev: 0, path: null } }); + const advertisedCursor = result.currentCursor; + + const entries = await drainStream(result.stream); + expect(rewrote).toBe(true); + expect(result.currentCursor).toEqual(advertisedCursor); + expect(entries.map((entry) => entry.path)).toContain("/before.txt"); + expect(entries.map((entry) => entry.path)).not.toContain("/after.txt"); + expect(advertisedCursor.rev).toBeLessThan(currentRev(upstream.db)); + } finally { + upstream.close(); + } + }); + + it("pullOnce persists only the advertised snapshot cursor", async () => { + const upstream = makePeer(); + const downstream = makePeer(); + try { + const providerA = new SQLiteWorkspaceProvider(upstream.db, { now: () => 1 }); + providerA.writeFileSync("/before.txt", "before"); + + let advertisedCursor: { rev: number; path: string | null } | undefined; + const wrapped = new Proxy(upstream.rpc as object, { + get(target, prop, receiver) { + if (prop === "fetchChanges") { + return async (...args: Parameters) => { + const result = await Reflect.get(target, prop, receiver).call(target, ...args); + advertisedCursor = result.currentCursor; + providerA.writeFileSync("/after.txt", "after"); + return result; + }; + } + return Reflect.get(target, prop, receiver); + }, + }) as typeof upstream.rpc; + + await pullOnce(downstream.db, wrapped); + + expect(advertisedCursor).toBeDefined(); + expect(fileEntries(downstream.db)).toContain("before.txt"); + expect(fileEntries(downstream.db)).not.toContain("after.txt"); + expect(readFetchCursor(downstream.db)).toEqual(advertisedCursor); + expect(readFetchCursor(downstream.db).rev).toBeLessThan(currentRev(upstream.db)); + } finally { + upstream.close(); + downstream.close(); + } + }); +}); + +describe("sync driver — pullOnce envelope disposal", () => { + it("disposes the fetchChanges envelope when the cross-side invariant trips", async () => { + const remote = makePeer(); + try { + const local = new Database(new SQLiteTestStorage()); + initializeSchema(local, () => 1000); + // Local claims to have pushed rev 42; the wrapper echoes back a + // rev-0 cursor, so assertAppliedPushCursor throws before the + // stream is ever read. + writeWatermark(local, "pushRev", 42); + const providerR = new SQLiteWorkspaceProvider(remote.db, { now: () => 1 }); + providerR.writeFileSync("/seed.txt", "x"); + + const tracked = trackFetchDisposal(remote.rpc, { lowerCursor: true }); + await expect(pullOnce(local, tracked.rpc)).rejects.toThrow(/cross-side invariant violated/i); + expect(tracked.disposeCount()).toBe(1); + } finally { + remote.close(); + } + }); + + it("disposes the fetchChanges envelope when applyChanges throws mid-stream", async () => { + const remote = makePeer(); + const local = makePeer(); + try { + const providerR = new SQLiteWorkspaceProvider(remote.db, { now: () => 1 }); + providerR.writeFileSync("/seed.txt", "needs-bytes"); + + // hasObjects lies (returns nothing), so pullOnce stages no blob + // bytes and applyChanges throws on the missing object. + const tracked = trackFetchDisposal(remote.rpc, { failHasObjects: true }); + await expect(pullOnce(local.db, tracked.rpc)).rejects.toThrow(/missing object/i); + expect(tracked.disposeCount()).toBe(1); + } finally { + remote.close(); + local.close(); + } + }); + + it("disposes the fetchChanges envelope on the happy path", async () => { + const remote = makePeer(); + const local = makePeer(); + try { + const providerR = new SQLiteWorkspaceProvider(remote.db, { now: () => 1 }); + providerR.writeFileSync("/seed.txt", "x"); + + const tracked = trackFetchDisposal(remote.rpc); + await pullOnce(local.db, tracked.rpc); + expect(tracked.disposeCount()).toBe(1); + expect(fileEntries(local.db)).toContain("seed.txt"); + } finally { + remote.close(); + local.close(); + } + }); +}); + describe("sync driver — bidirectional convergence", () => { it("two peers writing in parallel converge after a few ticks", async () => { const a = makePeer(); @@ -406,21 +581,21 @@ describe("sync driver — bidirectional convergence", () => { }); describe("sync driver — cross-side invariant", () => { - it("pushOnce throws when the remote echoes back a lower appliedPushRev", async () => { + it("pushOnce throws when the remote echoes back a lower appliedPushCursor", async () => { const a = makePeer(); const b = makePeer(); try { const providerA = new SQLiteWorkspaceProvider(a.db, { now: () => 1 }); providerA.writeFileSync("/x.txt", "x"); - // Wrap B's rpc to lie about appliedPushRev. Simulates a + // Wrap B's rpc to lie about appliedPushCursor. Simulates a // regression in the suppress-dirty-tracking apply path. const lyingRpc = new Proxy(b.rpc as object, { get(target, prop, receiver) { if (prop === "push") { return async (input: { senderRev: number; changes: ReadableStream }) => { const real = await Reflect.get(target, prop, receiver).call(target, input); - return { ...real, appliedPushRev: 0 }; + return { ...real, appliedPushCursor: { rev: 0, path: null } }; }; } return Reflect.get(target, prop, receiver); @@ -434,10 +609,10 @@ describe("sync driver — cross-side invariant", () => { } }); - it("pullOnce throws when fetchChanges echoes back a lower appliedPushRev", async () => { + it("pullOnce throws when fetchChanges echoes back a lower appliedPushCursor", async () => { // Symmetric to the push case. fetchChanges returns the remote's - // appliedPushRev alongside the entry stream; the DO asserts - // appliedPushRev >= pushRev before draining, so a regression in + // appliedPushCursor alongside the entry stream; the DO asserts + // appliedPushCursor covers pushRev before draining, so a regression in // the remote's apply path that loses applied state trips the // invariant on the next pull instead of corrupting fetchRev. const remote = makePeer(); @@ -455,9 +630,9 @@ describe("sync driver — cross-side invariant", () => { const lyingRpc = new Proxy(remote.rpc as object, { get(target, prop, receiver) { if (prop === "fetchChanges") { - return (input: { sinceRev?: number; ignore?: string[] }) => { + return (input: Parameters[0]) => { const real = Reflect.get(target, prop, receiver).call(target, input); - return { ...real, appliedPushRev: 0 }; + return { ...real, appliedPushCursor: { rev: 0, path: null } }; }; } return Reflect.get(target, prop, receiver); @@ -469,6 +644,50 @@ describe("sync driver — cross-side invariant", () => { remote.close(); } }); + + it("fetchChanges reports applied push progress as a cursor", async () => { + const remote = makePeer(); + try { + writeFetchCursor(remote.db, { rev: 42, path: "/partial.txt" }); + + const result = await remote.rpc.fetchChanges({ after: { rev: 0, path: null } }); + + expect(result.appliedPushCursor).toEqual({ rev: 42, path: "/partial.txt" }); + await result.stream.cancel(); + } finally { + remote.close(); + } + }); + + it("pullOnce rejects when the remote only partially applied local pushRev", async () => { + const remote = makePeer(); + try { + const local = new Database(new SQLiteTestStorage()); + initializeSchema(local, () => 1000); + writeWatermark(local, "pushRev", 42); + writeFetchCursor(remote.db, { rev: 42, path: "/partial.txt" }); + + await expect(pullOnce(local, remote.rpc)).rejects.toThrow(/cross-side invariant violated/i); + } finally { + remote.close(); + } + }); + + it("pullOnce accepts a partial remote cursor after local pushRev", async () => { + const remote = makePeer(); + try { + const local = new Database(new SQLiteTestStorage()); + initializeSchema(local, () => 1000); + writeWatermark(local, "pushRev", 42); + writeFetchCursor(remote.db, { rev: 43, path: "/partial.txt" }); + + const result = await pullOnce(local, remote.rpc); + + expect(result).toEqual({ applied: 0, skipped: [] }); + } finally { + remote.close(); + } + }); }); describe("sync driver — streaming pullOnce", () => { @@ -537,7 +756,7 @@ describe("sync driver — streaming pullOnce", () => { get(target, prop, receiver) { if (prop === "hasObjects") { return async (hashes: Uint8Array[]) => { - sampledRevs.push(readWatermark(b.db, "fetchRev")); + sampledRevs.push(readFetchCursor(b.db).rev); return Reflect.get(target, prop, receiver).call(target, hashes); }; } @@ -556,7 +775,142 @@ describe("sync driver — streaming pullOnce", () => { expect(sampledRevs[i]).toBeGreaterThan(sampledRevs[i - 1]); } // End state still matches the remote. - expect(readWatermark(b.db, "fetchRev")).toBe(remoteFinalRev); + expect(readFetchCursor(b.db).rev).toBe(remoteFinalRev); + } finally { + a.close(); + b.close(); + } + }); + + it("does not let an older overlapping pull move the fetch cursor backward", async () => { + const upstream = makePeer(); + const downstream = makePeer(); + try { + const providerA = new SQLiteWorkspaceProvider(upstream.db, { now: () => 1 }); + const providerB = new SQLiteWorkspaceProvider(downstream.db, { now: () => 1 }); + for (let i = 0; i < 300; i++) { + providerA.writeFileSync(`/f${i.toString().padStart(3, "0")}.txt`, `old ${i}`); + } + + let releaseOldHasObjects: (() => void) | undefined; + let oldHasObjectsEntered: (() => void) | undefined; + const oldHasObjectsStarted = new Promise((resolve) => { + oldHasObjectsEntered = resolve; + }); + const oldHasObjectsGate = new Promise((resolve) => { + releaseOldHasObjects = resolve; + }); + const sampledCursorBeforeSecondOldBatch: Array<{ rev: number; path: string | null }> = []; + let oldHasObjectsCalls = 0; + const olderRpc = new Proxy(upstream.rpc as object, { + get(target, prop, receiver) { + if (prop === "hasObjects") { + return async (hashes: Uint8Array[]) => { + oldHasObjectsCalls++; + if (oldHasObjectsCalls === 1) { + oldHasObjectsEntered?.(); + await oldHasObjectsGate; + } else { + sampledCursorBeforeSecondOldBatch.push(readFetchCursor(downstream.db)); + } + return Reflect.get(target, prop, receiver).call(target, hashes); + }; + } + return Reflect.get(target, prop, receiver); + }, + }) as typeof upstream.rpc; + + const olderPull = pullOnce(downstream.db, olderRpc); + await oldHasObjectsStarted; + + providerA.writeFileSync("/newer.txt", "newer"); + const newestCursor = { rev: currentRev(upstream.db), path: null }; + const newerPull = await pullOnce(downstream.db, upstream.rpc); + expect(newerPull.applied).toBe(301); + expect(readFetchCursor(downstream.db)).toEqual(newestCursor); + + releaseOldHasObjects?.(); + const olderPullResult = await olderPull; + + expect(olderPullResult.applied).toBe(0); + expect(providerB.readFileSync("/newer.txt", "utf8")).toBe("newer"); + expect(sampledCursorBeforeSecondOldBatch).toEqual([newestCursor]); + expect(readFetchCursor(downstream.db)).toEqual(newestCursor); + } finally { + upstream.close(); + downstream.close(); + } + }); + + it("resumes inside one large same-rev rename after a failed batch", async () => { + const a = makePeer(); + const b = makePeer(); + try { + const providerA = new SQLiteWorkspaceProvider(a.db, { now: () => 1 }); + const providerB = new SQLiteWorkspaceProvider(b.db, { now: () => 1 }); + providerA.mkdirSync("/src", {}); + for (let i = 0; i < 300; i++) { + providerA.writeFileSync(`/src/f${i.toString().padStart(3, "0")}.txt`, `content ${i}`); + } + + await pullOnce(b.db, a.rpc); + expect(providerB.readFileSync("/src/f299.txt", "utf8")).toBe("content 299"); + + providerA.renameSync("/src", "/dst"); + const renameRev = currentRev(a.db); + let hasObjectsCalls = 0; + const flaky = new Proxy(a.rpc as object, { + get(target, prop, receiver) { + if (prop === "hasObjects") { + return async (hashes: Uint8Array[]) => { + hasObjectsCalls++; + if (hasObjectsCalls === 2) { + throw new Error("injected pull failure after first batch"); + } + return Reflect.get(target, prop, receiver).call(target, hashes); + }; + } + return Reflect.get(target, prop, receiver); + }, + }) as typeof a.rpc; + + await expect(pullOnce(b.db, flaky)).rejects.toThrow(/injected pull failure/); + expect(readFetchCursor(b.db)).toMatchObject({ rev: renameRev }); + expect(readFetchCursor(b.db).path).not.toBeNull(); + + const retried = await pullOnce(b.db, a.rpc); + expect(retried.applied).toBeGreaterThan(0); + expect(providerB.existsSync("/src")).toBe(false); + expect(providerB.readFileSync("/dst/f000.txt", "utf8")).toBe("content 0"); + expect(providerB.readFileSync("/dst/f299.txt", "utf8")).toBe("content 299"); + expect(readFetchCursor(b.db)).toEqual({ rev: renameRev, path: null }); + } finally { + a.close(); + b.close(); + } + }); + + it("lets a peer push supersede a partial pull cursor", async () => { + const a = makePeer(); + const b = makePeer(); + try { + const providerA = new SQLiteWorkspaceProvider(a.db, { now: () => 1 }); + const providerB = new SQLiteWorkspaceProvider(b.db, { now: () => 1 }); + providerA.writeFileSync("/before-stale-path.txt", "pushed"); + const pushedRev = currentRev(a.db); + + writeFetchCursor(b.db, { rev: pushedRev, path: "/zzzz" }); + + expect(await pushOnce(a.db, b.rpc)).toBeGreaterThan(0); + expect(readFetchCursor(b.db)).toEqual({ rev: pushedRev, path: null }); + expect(providerB.readFileSync("/before-stale-path.txt", "utf8")).toBe("pushed"); + + providerA.writeFileSync("/after-push.txt", "pulled later"); + const pulled = await pullOnce(b.db, a.rpc); + + expect(pulled.applied).toBe(1); + expect(providerB.readFileSync("/after-push.txt", "utf8")).toBe("pulled later"); + expect(readFetchCursor(b.db)).toEqual({ rev: currentRev(a.db), path: null }); } finally { a.close(); b.close(); @@ -626,7 +980,7 @@ describe("sync driver — reconcileWatermarks", () => { // container lifetimes; the container's watermarks are // process-lifetime in today's wsd. After a container restart with // no new DO-side writes, pushOnce's localRev <= sincePush - // early-return means the assertAppliedPushRev check never runs and + // early-return means the assertAppliedPushCursor check never runs and // the container's empty FUSE mount is invisible to the DO. The // reconcile catches the mismatch by comparing the local cursors // against the remote's watermarks(), resetting fetchRev to 0 when @@ -639,11 +993,11 @@ describe("sync driver — reconcileWatermarks", () => { // (currentRev = 1 from initializeSchema seeding the root). const local = new Database(new SQLiteTestStorage()); initializeSchema(local, () => 1000); - writeWatermark(local, "fetchRev", 42); + writeFetchCursor(local, { rev: 42, path: null }); writeWatermark(local, "pushRev", 0); await reconcileWatermarks(local, remote.rpc); - expect(readWatermark(local, "fetchRev")).toBe(0); + expect(readFetchCursor(local)).toEqual({ rev: 0, path: null }); expect(readWatermark(local, "pushRev")).toBe(0); } finally { remote.close(); @@ -654,10 +1008,10 @@ describe("sync driver — reconcileWatermarks", () => { const remote = makePeer(); try { // Local pushRev = 17, but the remote is fresh: its pushRev, - // which doubles as appliedPushRev on the wire, is 0. + // which is echoed as appliedPushCursor on the wire, is 0/null. const local = new Database(new SQLiteTestStorage()); initializeSchema(local, () => 1000); - writeWatermark(local, "fetchRev", 0); + writeFetchCursor(local, { rev: 0, path: null }); writeWatermark(local, "pushRev", 17); await reconcileWatermarks(local, remote.rpc); @@ -677,11 +1031,11 @@ describe("sync driver — reconcileWatermarks", () => { const local = new Database(new SQLiteTestStorage()); initializeSchema(local, () => 1000); const remoteCurrent = currentRev(remote.db); - writeWatermark(local, "fetchRev", remoteCurrent); + writeFetchCursor(local, { rev: remoteCurrent, path: null }); writeWatermark(local, "pushRev", 0); await reconcileWatermarks(local, remote.rpc); - expect(readWatermark(local, "fetchRev")).toBe(remoteCurrent); + expect(readFetchCursor(local)).toEqual({ rev: remoteCurrent, path: null }); expect(readWatermark(local, "pushRev")).toBe(0); } finally { remote.close(); diff --git a/packages/rpc/src/sync-driver.ts b/packages/rpc/src/sync-driver.ts index 355d5f29..148c0d66 100644 --- a/packages/rpc/src/sync-driver.ts +++ b/packages/rpc/src/sync-driver.ts @@ -12,14 +12,18 @@ import { type ApplyResult, applyChanges, - assertAppliedPushRev, + assertAppliedPushCursor, + type ChangeCursor, type ChangeEntry, coalesceChanges, + compareChangeCursors, currentRev, type Database, + readFetchCursor, readWatermark, type SkippedEntry, stageBlob, + writeFetchCursor, writeWatermark, } from "@cloudflare/dofs"; @@ -31,38 +35,6 @@ function hex(bytes: Uint8Array): string { return s; } -// Pipe `stream` through an identity TransformStream that fires `onDone` -// exactly once when the stream finishes — clean end, cancel, or error. -// Used to release a capnweb result envelope as soon as the stream it -// carried is drained, without having to keep the envelope reference -// alive across the consume site. `onDone` errors are swallowed: the -// stream's content is the load-bearing thing, not the cleanup. -function disposeOnDone(stream: ReadableStream, onDone: () => void): ReadableStream { - let fired = false; - const fire = () => { - if (fired) return; - fired = true; - try { - onDone(); - } catch { - // ignore — disposer errors are not actionable here - } - }; - return stream.pipeThrough( - new TransformStream({ - transform(chunk, controller) { - controller.enqueue(chunk); - }, - flush() { - fire(); - }, - cancel() { - fire(); - }, - }), - ); -} - // Best-effort dispose of a capnweb result envelope. Real envelopes // expose [Symbol.dispose]; the test fakes return plain objects, so // the symbol may be absent. @@ -78,7 +50,7 @@ function maybeDispose(value: unknown): void { const PULL_BATCH_SIZE = 256; // Pull every entry the remote has produced since the last successful -// pull, apply locally, advance fetchRev. Returns an `ApplyResult` +// pull, apply locally, advance the fetch cursor. Returns an `ApplyResult` // folded across every batch so callers see both the applied count // (decide whether to tick again) and any entries skipped because // they targeted a read-only mount root (surface to the user). @@ -88,140 +60,159 @@ const PULL_BATCH_SIZE = 256; // dedup work without per-chunk round-trips. // // The entry stream is drained in batches of PULL_BATCH_SIZE so peak -// memory stays bounded on a large tree. fetchRev still advances -// once at the end of the whole stream — per-batch advance would -// require the wire to carry a rev cursor per entry. Crash safety is -// idempotent re-apply: the receiver's alreadyApplied() check inside -// applyChanges drops a re-fetched batch on the floor. +// memory stays bounded on a large tree. The durable fetch cursor +// advances after each committed batch to the last streamed entry's +// (rev, path), so a retry can resume inside a single large rev. The +// cursor is read and written per backend so concurrent backends keep +// independent resume points. export async function pullOnce( db: Database, remote: SyncRPC, backend?: string, ): Promise { - const sinceRev = readWatermark(db, "fetchRev", backend); + const after = readFetchCursor(db, backend); const localPushRev = readWatermark(db, "pushRev", backend); - // fetchChanges hands back the remote's currentRev (cursor we - // advance fetchRev to), its appliedPushRev (cross-side invariant - // check on the pull path), and the entry stream itself. One - // round-trip instead of the previous currentRev() + fetchChanges() - // pair. + // fetchChanges hands back the remote's currentCursor (cursor we + // advance to after a clean drain), its appliedPushCursor (cross-side + // invariant check on the pull path), and the entry stream itself. + // One round-trip instead of the previous currentRev() + + // fetchChanges() pair. // The fetchChanges return is a capnweb result envelope wrapping a // stream stub; without explicit disposal it sits in the exports // table until the session ends. We can't bind it to `using` // because the stream inside outlives this scope (we hand it off // to the reader loop below), so wrap the stream in a transform // that disposes the envelope when the stream finishes draining. - const fetchResult = await remote.fetchChanges({ sinceRev }); - const { currentRev: remoteRev, appliedPushRev } = fetchResult; - // Run the cross-side invariant check before touching the stream. - // Symmetric to the push response check: the remote must have - // applied at least everything we claimed to push. A drop here - // means apply lost state on the receiver; tear down and rebuild - // rather than corrupt watermarks. - assertAppliedPushRev(appliedPushRev, localPushRev); - const stream = disposeOnDone(fetchResult.stream, () => maybeDispose(fetchResult)); - if (remoteRev <= sinceRev) { - // Drain the (empty) stream so the remote's iterator is - // released; cancel is the right surface for that. - await stream.cancel().catch(() => {}); - return { applied: 0, skipped: [] }; - } - - const reader = stream.getReader(); - let totalApplied = 0; - const totalSkipped: SkippedEntry[] = []; - let streamDone = false; + // pullOnce owns the fetchChanges result envelope: it wraps a stream + // stub that sits in the exports table until disposed. The stream is + // fully consumed within this call (drained, cancelled, or abandoned + // on a throw), so a try/finally disposing the envelope covers every + // exit — the clean drain, the early-complete return, the cross-side + // invariant trip, and any throw inside the batch loop. Disposing the + // envelope tears down the contained stream stub, releasing the + // remote iterator. + const fetchResult = await remote.fetchChanges({ after }); try { - while (!streamDone) { - // Read up to PULL_BATCH_SIZE entries before processing the batch. - const batch: ChangeEntry[] = []; - const wantedHashes: Uint8Array[] = []; - const seenHash = new Set(); - let batchMaxRev = 0; - while (batch.length < PULL_BATCH_SIZE) { - const { value, done } = await reader.read(); - if (done) { - streamDone = true; - break; - } - batch.push(value); - if (value.rev > batchMaxRev) batchMaxRev = value.rev; - if (value.kind === "file") { - for (const c of value.chunks) { - const k = hex(c.hash); - if (!seenHash.has(k)) { - seenHash.add(k); - wantedHashes.push(c.hash); + const { currentCursor, appliedPushCursor } = fetchResult; + // Cross-side invariant, symmetric to the push response check: the + // remote must have applied at least everything we claimed to push. + // A gap means apply lost state on the receiver; tear down and + // rebuild rather than corrupt watermarks. Runs inside the try so a + // trip still disposes the envelope. + assertAppliedPushCursor(appliedPushCursor, { rev: localPushRev, path: null }); + if (cursorComplete(after, currentCursor)) { + return { applied: 0, skipped: [] }; + } + + const reader = fetchResult.stream.getReader(); + let totalApplied = 0; + const totalSkipped: SkippedEntry[] = []; + let streamDone = false; + try { + while (!streamDone) { + // Read up to PULL_BATCH_SIZE entries before processing the batch. + const batch: ChangeEntry[] = []; + const wantedHashes: Uint8Array[] = []; + const seenHash = new Set(); + while (batch.length < PULL_BATCH_SIZE) { + const { value, done } = await reader.read(); + if (done) { + streamDone = true; + break; + } + batch.push(value); + if (value.kind === "file") { + for (const c of value.chunks) { + const k = hex(c.hash); + if (!seenHash.has(k)) { + seenHash.add(k); + wantedHashes.push(c.hash); + } } } } - } - if (batch.length === 0) break; + if (batch.length === 0) break; - // Probe + fetch missing chunk bytes for just this batch. Bytes - // the receiver already holds (or the remote doesn't have) are - // skipped, so the per-batch network cost is bounded. - if (wantedHashes.length > 0) { - const haveSubset = await remote.hasObjects(wantedHashes); - const remoteHasLocally = new Set(); - for (const h of haveSubset) remoteHasLocally.add(hex(h)); - const missing = wantedHashes.filter((h) => { - const k = hex(h); - if (!remoteHasLocally.has(k)) return false; - const row = db.one<{ hash: Uint8Array }>("SELECT hash FROM vfs_blobs WHERE hash = ?", h); - return row === undefined; - }); - if (missing.length > 0) { - // Bare ReadableStream return — no envelope to dispose, - // capnweb releases the stream stub when the stream itself - // closes. The reader-loop below drains to completion. - const bytesStream = await remote.fetchObjects(missing); - const bytesReader = bytesStream.getReader(); - try { - while (true) { - const { value, done } = await bytesReader.read(); - if (done) break; - stageBlob(db, value.hash, value.bytes, Date.now()); + // Probe + fetch missing chunk bytes for just this batch. Bytes + // the receiver already holds (or the remote doesn't have) are + // skipped, so the per-batch network cost is bounded. + if (wantedHashes.length > 0) { + const haveSubset = await remote.hasObjects(wantedHashes); + const remoteHasLocally = new Set(); + for (const h of haveSubset) remoteHasLocally.add(hex(h)); + const missing = wantedHashes.filter((h) => { + const k = hex(h); + if (!remoteHasLocally.has(k)) return false; + const row = db.one<{ hash: Uint8Array }>( + "SELECT hash FROM vfs_blobs WHERE hash = ?", + h, + ); + return row === undefined; + }); + if (missing.length > 0) { + // Bare ReadableStream return — no envelope to dispose, + // capnweb releases the stream stub when the stream itself + // closes. The reader-loop below drains to completion. + const bytesStream = await remote.fetchObjects(missing); + const bytesReader = bytesStream.getReader(); + try { + while (true) { + const { value, done } = await bytesReader.read(); + if (done) break; + stageBlob(db, value.hash, value.bytes, Date.now()); + } + } finally { + bytesReader.releaseLock(); } - } finally { - bytesReader.releaseLock(); } } - } - // Advance fetchRev per committed batch. Because coalesceChanges - // emits in ascending rev order, every entry already applied - // (this batch + all previous ones) has rev <= batchMaxRev, so - // it's safe to checkpoint here. A crash after this commit - // means the next pull resumes from batchMaxRev and re-fetches - // only entries from later batches — bounded by PULL_BATCH_SIZE - // instead of the whole stream. - const batchResult = await applyChanges(db, batch, new Map(), { - source: "upstream", - advanceFetchRev: batchMaxRev, - backend, - }); - totalApplied += batchResult.applied; - if (batchResult.skipped.length > 0) { - for (const s of batchResult.skipped) totalSkipped.push(s); + const batchResult = await applyChanges(db, batch, new Map(), { + source: "upstream", + backend, + }); + const last = batch[batch.length - 1]; + // Cursor advancement intentionally happens after applyChanges() + // and is not atomic with it. A crash between apply and this + // checkpoint re-fetches the batch; upstream apply is idempotent + // because alreadyApplied() drops entries whose live state + // already matches. + writeFetchCursorIfAhead(db, { rev: last.rev, path: last.path }, backend); + totalApplied += batchResult.applied; + if (batchResult.skipped.length > 0) { + for (const s of batchResult.skipped) totalSkipped.push(s); + } } + } finally { + reader.releaseLock(); } + + // Mark the receiver's captured current rev as fully drained. This + // preserves the ignored/no-op window behavior: if the stream had no + // entries because every path was filtered out, the next pull still + // starts after that rev. + // fetchChanges() is snapshot-bounded by currentCursor, so this + // final drain marker cannot skip beyond any per-batch cursor written + // above. + writeFetchCursorIfAhead(db, currentCursor, backend); + return { applied: totalApplied, skipped: totalSkipped }; } finally { - reader.releaseLock(); + maybeDispose(fetchResult); } +} + +function cursorComplete(after: ChangeCursor, current: ChangeCursor): boolean { + if (current.rev < after.rev) return true; + return current.rev === after.rev && after.path === null; +} - // Nudge fetchRev to the remote's currentRev captured at the start. - // The per-batch advances above bring fetchRev up to the max rev - // any entry carried, but if the rev window contained entries that - // were all filtered out (e.g. ignored paths), the stream is empty - // and the per-batch path never fires. Advancing to remoteRev here - // is still safe because we captured it before draining the stream - // and never regress. - const current = readWatermark(db, "fetchRev", backend); - if (remoteRev > current) { - writeWatermark(db, "fetchRev", remoteRev, backend); +function writeFetchCursorIfAhead(db: Database, cursor: ChangeCursor, backend?: string): void { + // Overlapping pulls can complete out of order, so checkpoint writes + // compare against the latest persisted cursor instead of the value + // observed when this pull started. + if (compareChangeCursors(cursor, readFetchCursor(db, backend)) > 0) { + writeFetchCursor(db, cursor, backend); } - return { applied: totalApplied, skipped: totalSkipped }; } // Push every entry the local store has produced since the last @@ -235,7 +226,7 @@ export async function pushOnce(db: Database, remote: SyncRPC, backend?: string): const entries: ChangeEntry[] = []; const wantedHashes: Uint8Array[] = []; const seenHash = new Set(); - for await (const e of coalesceChanges(db, sincePush)) { + for await (const e of coalesceChanges(db, { rev: sincePush, path: null })) { entries.push(e); if (e.kind === "file") { for (const c of e.chunks) { @@ -289,11 +280,11 @@ export async function pushOnce(db: Database, remote: SyncRPC, backend?: string): }); const response = await remote.push({ senderRev: localRev, changes: entryStream }); - // Cross-side invariant: the receiver must echo back at least - // the rev we just claimed to push. A drift means the apply - // path lost data, or a stale receiver is serving an old - // snapshot. Tear down loudly rather than corrupt watermarks. - assertAppliedPushRev(response.appliedPushRev, localRev); + // Cross-side invariant: the receiver must echo back a cursor that + // covers the rev we just claimed to push. A drift means the apply + // path lost data, or a stale receiver is serving an old snapshot. + // Tear down loudly rather than corrupt watermarks. + assertAppliedPushCursor(response.appliedPushCursor, { rev: localRev, path: null }); // Local pushRev advances to the rev we observed at the start of // this round. Anything written after that gets caught next tick. @@ -337,17 +328,17 @@ export async function reconcileWatermarks( backend?: string, ): Promise<{ fetchRevReset: boolean; pushRevReset: boolean }> { const remoteWatermarks = await remote.watermarks(); - const localFetchRev = readWatermark(db, "fetchRev", backend); + const localFetchCursor = readFetchCursor(db, backend); const localPushRev = readWatermark(db, "pushRev", backend); let fetchRevReset = false; let pushRevReset = false; - // If the remote's currentRev is below our fetchRev, the remote's + // If the remote's currentRev is below our fetch cursor rev, the remote's // log is shorter than we remember — it lost state since we last // pulled. Re-baseline from 0. - if (remoteWatermarks.currentRev < localFetchRev) { - writeWatermark(db, "fetchRev", 0, backend); + if (remoteWatermarks.currentRev < localFetchCursor.rev) { + writeFetchCursor(db, { rev: 0, path: null }, backend); fetchRevReset = true; } diff --git a/packages/rpc/tests/wire.test.ts b/packages/rpc/tests/wire.test.ts index 6db78517..0ebd5837 100644 --- a/packages/rpc/tests/wire.test.ts +++ b/packages/rpc/tests/wire.test.ts @@ -2,13 +2,16 @@ import { createServer, type Server } from "node:http"; import type { AddressInfo } from "node:net"; import { + assertAppliedPushCursor, type ChangeEntry, coalesceChanges, Database, fetchObjects, initializeSchema, ROOT_INODE, + readFetchCursor, SQLiteWorkspaceProvider, + writeFetchCursor, } from "@cloudflare/dofs"; import { SQLiteTestStorage } from "@cloudflare/dofs/testing"; import { afterEach, describe, expect, it } from "vitest"; @@ -64,7 +67,7 @@ describe("SyncRPC over a real WebSocket", () => { const client = createSyncClient({ url: harness.url }); try { - const { stream } = await client.fetchChanges({ sinceRev: 0, ignore: [] }); + const { stream } = await client.fetchChanges({ after: { rev: 0, path: null }, ignore: [] }); const entries: ChangeEntry[] = []; const reader = stream.getReader(); try { @@ -157,7 +160,7 @@ describe("SyncRPC push convergence", () => { const entries: ChangeEntry[] = []; const hashes: Uint8Array[] = []; const seenHash = new Set(); - for await (const e of coalesceChanges(senderDb, 0)) { + for await (const e of coalesceChanges(senderDb, { rev: 0, path: null })) { entries.push(e); if (e.kind === "file") { for (const c of e.chunks) { @@ -361,8 +364,6 @@ describe("WireError propagation", () => { }); }); -import { assertAppliedPushRev } from "@cloudflare/dofs"; - describe("cross-side invariant", () => { let harness: Harness | undefined; afterEach(async () => { @@ -370,7 +371,7 @@ describe("cross-side invariant", () => { harness = undefined; }); - it("push returns a {rev, appliedPushRev} that satisfies appliedPushRev >= 0", async () => { + it("push returns an appliedPushCursor that covers senderRev", async () => { harness = await startHarness(); const client = createSyncClient({ url: harness.url }); try { @@ -380,12 +381,13 @@ describe("cross-side invariant", () => { }, }); const result = await client.push({ senderRev: 0, changes: empty }); - expect(result.appliedPushRev).toBeGreaterThanOrEqual(0); - // The DO would pass result.appliedPushRev as `applied` - // and its own pushRev counter as `pushed`. With the - // container reporting >= 0 and the DO holding 0 at this - // point, the invariant holds. - expect(() => assertAppliedPushRev(result.appliedPushRev, 0)).not.toThrow(); + expect(result.appliedPushCursor).toEqual({ rev: 0, path: null }); + // The durable object would pass result.appliedPushCursor as + // `applied` and its own push cursor as `pushed`. With both at + // zero, the invariant holds. + expect(() => + assertAppliedPushCursor(result.appliedPushCursor, { rev: 0, path: null }), + ).not.toThrow(); } finally { await client.close(); } @@ -496,12 +498,12 @@ describe("push semantics — external vs sync peer", () => { // outbound sync loop (a wsd with UPSTREAM_URL set) would // see the new entry on the next tick. expect(readWatermark(harness.db, "pushRev")).toBe(0); - // fetchRev was NOT advanced either — the sender has + // The fetch cursor was NOT advanced either — the sender has // no rev space. - expect(readWatermark(harness.db, "fetchRev")).toBe(0); + expect(readFetchCursor(harness.db)).toEqual({ rev: 0, path: null }); // The entry is in the coalesce stream. const drained: { path: string }[] = []; - for await (const e of coalesceChanges(harness.db, 0)) drained.push(e); + for await (const e of coalesceChanges(harness.db, { rev: 0, path: null })) drained.push(e); expect(drained.some((e) => e.path === "/external.txt")).toBe(true); } finally { await client.close(); @@ -510,7 +512,9 @@ describe("push semantics — external vs sync peer", () => { it("push with senderRev>0 (sync peer) advances pushRev to silence loopback", async () => { harness = await startHarness(); - const { currentRev, readWatermark, writeWatermark } = await import("@cloudflare/dofs"); + const { currentRev, readFetchCursor, readWatermark, writeWatermark } = await import( + "@cloudflare/dofs" + ); const client = createSyncClient({ url: harness.url }); try { // Seed pushRev at the current point so the F1 guard @@ -551,8 +555,37 @@ describe("push semantics — external vs sync peer", () => { // entries back to the peer. const cur = currentRev(harness.db); expect(readWatermark(harness.db, "pushRev")).toBe(cur); - // fetchRev was advanced to senderRev. - expect(readWatermark(harness.db, "fetchRev")).toBe(42); + // The fetch cursor was advanced to senderRev. + expect(readFetchCursor(harness.db)).toEqual({ rev: 42, path: null }); + } finally { + await client.close(); + } + }); + + it("push with senderRev>0 advances the fetch cursor monotonically", async () => { + harness = await startHarness(); + const client = createSyncClient({ url: harness.url }); + try { + writeFetchCursor(harness.db, { rev: 10, path: "/partial.txt" }); + await client.push({ + senderRev: 10, + changes: new ReadableStream({ + start(c) { + c.close(); + }, + }), + }); + expect(readFetchCursor(harness.db)).toEqual({ rev: 10, path: null }); + + await client.push({ + senderRev: 3, + changes: new ReadableStream({ + start(c) { + c.close(); + }, + }), + }); + expect(readFetchCursor(harness.db)).toEqual({ rev: 10, path: null }); } finally { await client.close(); } diff --git a/packages/workspace/src/mounts/index.test.ts b/packages/workspace/src/mounts/index.test.ts index 738b255b..2a637930 100644 --- a/packages/workspace/src/mounts/index.test.ts +++ b/packages/workspace/src/mounts/index.test.ts @@ -485,12 +485,12 @@ describe("mount indexer", () => { } finally { reader.releaseLock(); } - return { rev: 0, appliedPushRev: input.senderRev }; + return { rev: 0, appliedPushCursor: { rev: input.senderRev, path: null } }; }, async fetchChanges() { return { - currentRev: 0, - appliedPushRev: 0, + currentCursor: { rev: 0, path: null }, + appliedPushCursor: { rev: 0, path: null }, stream: new ReadableStream({ start(c) { c.close(); @@ -512,7 +512,7 @@ describe("mount indexer", () => { }); }, async watermarks() { - return { currentRev: 0, pushRev: 0, fetchRev: 0 }; + return { currentRev: 0, pushRev: 0, fetchCursor: { rev: 0, path: null } }; }, async pushObjects(objects) { const reader = objects.getReader(); diff --git a/packages/workspace/src/observe-integration.test.ts b/packages/workspace/src/observe-integration.test.ts index ebc6b0ed..62295c38 100644 --- a/packages/workspace/src/observe-integration.test.ts +++ b/packages/workspace/src/observe-integration.test.ts @@ -33,12 +33,12 @@ function fakeSync(): SyncRPC { } finally { reader.releaseLock(); } - return { rev: 0, appliedPushRev: input.senderRev }; + return { rev: 0, appliedPushCursor: { rev: input.senderRev, path: null } }; }, async fetchChanges() { return { - currentRev: 0, - appliedPushRev: 0, + currentCursor: { rev: 0, path: null }, + appliedPushCursor: { rev: 0, path: null }, stream: new ReadableStream({ start(c) { c.close(); @@ -60,7 +60,7 @@ function fakeSync(): SyncRPC { }); }, async watermarks() { - return { currentRev: 0, pushRev: 0, fetchRev: 0 }; + return { currentRev: 0, pushRev: 0, fetchCursor: { rev: 0, path: null } }; }, async pushObjects(objects) { const reader = objects.getReader(); diff --git a/packages/workspace/src/shell.test.ts b/packages/workspace/src/shell.test.ts index 5b6d522d..96a9de88 100644 --- a/packages/workspace/src/shell.test.ts +++ b/packages/workspace/src/shell.test.ts @@ -113,7 +113,7 @@ function fakeRpc(options: FakeRpcOptions = {}): FakeRpc { return null; }, async watermarks() { - return { currentRev: 0, pushRev: 0, fetchRev: 0 }; + return { currentRev: 0, pushRev: 0, fetchCursor: { rev: 0, path: null } }; }, async hasObjects() { return []; diff --git a/packages/workspace/src/stub.test.ts b/packages/workspace/src/stub.test.ts index 423182f7..09272690 100644 --- a/packages/workspace/src/stub.test.ts +++ b/packages/workspace/src/stub.test.ts @@ -44,12 +44,12 @@ function composite( function fakeSync(): import("@cloudflare/workspace-rpc").SyncRPC { return { async push() { - return { rev: 0, appliedPushRev: 0 }; + return { rev: 0, appliedPushCursor: { rev: 0, path: null } }; }, async fetchChanges() { return { - currentRev: 0, - appliedPushRev: 0, + currentCursor: { rev: 0, path: null }, + appliedPushCursor: { rev: 0, path: null }, stream: new ReadableStream({ start(c) { c.close(); @@ -61,7 +61,7 @@ function fakeSync(): import("@cloudflare/workspace-rpc").SyncRPC { return null; }, async watermarks() { - return { currentRev: 0, pushRev: 0, fetchRev: 0 }; + return { currentRev: 0, pushRev: 0, fetchCursor: { rev: 0, path: null } }; }, async hasObjects() { return []; diff --git a/packages/workspace/src/workspace.test.ts b/packages/workspace/src/workspace.test.ts index 0e849088..e21dc7dd 100644 --- a/packages/workspace/src/workspace.test.ts +++ b/packages/workspace/src/workspace.test.ts @@ -58,12 +58,12 @@ function fakeRpc(): import("@cloudflare/workspace-rpc").SyncRPC { } finally { reader.releaseLock(); } - return { rev: 0, appliedPushRev: input.senderRev }; + return { rev: 0, appliedPushCursor: { rev: input.senderRev, path: null } }; }, async fetchChanges() { return { - currentRev: 0, - appliedPushRev: 0, + currentCursor: { rev: 0, path: null }, + appliedPushCursor: { rev: 0, path: null }, stream: new ReadableStream({ start(c) { c.close(); @@ -99,7 +99,7 @@ function fakeRpc(): import("@cloudflare/workspace-rpc").SyncRPC { }); }, async watermarks() { - return { currentRev: 0, pushRev: 0, fetchRev: 0 }; + return { currentRev: 0, pushRev: 0, fetchCursor: { rev: 0, path: null } }; }, async pushObjects(objects) { const reader = objects.getReader(); @@ -512,19 +512,22 @@ describe("Workspace backend selection", () => { ...fakeRpc(), async watermarks() { watermarksCalls++; - return { currentRev: 0, pushRev: 0, fetchRev: 0 }; + return { currentRev: 0, pushRev: 0, fetchCursor: { rev: 0, path: null } }; }, }; const storage = makeStorage(); const ws = new Workspace({ storage, backends: [makeBackend("only", sync)] }); - const { writeWatermark, readWatermark } = await import("@cloudflare/dofs"); + // Pre-seed local watermarks for the "only" backend. + const { readFetchCursor, readWatermark, writeFetchCursor, writeWatermark } = await import( + "@cloudflare/dofs" + ); writeWatermark(ws.db, "pushRev", 17, "only"); - writeWatermark(ws.db, "fetchRev", 42, "only"); + writeFetchCursor(ws.db, { rev: 42, path: null }, "only"); // ready() alone no longer dials; ready(id) forces the connect. await ws.ready("only"); expect(watermarksCalls).toBe(1); expect(readWatermark(ws.db, "pushRev", "only")).toBe(0); - expect(readWatermark(ws.db, "fetchRev", "only")).toBe(0); + expect(readFetchCursor(ws.db, "only")).toEqual({ rev: 0, path: null }); }); it("skips push/pull when the backend declares sync: 'none'", async () => { @@ -690,7 +693,7 @@ describe("Workspace mutation serialization", () => { reader.releaseLock(); } inFlight.push--; - return { rev: 0, appliedPushRev: input.senderRev }; + return { rev: 0, appliedPushCursor: { rev: input.senderRev, path: null } }; }, }; @@ -733,7 +736,7 @@ describe("Workspace mutation serialization", () => { } finally { reader.releaseLock(); } - return { rev: 0, appliedPushRev: input.senderRev }; + return { rev: 0, appliedPushCursor: { rev: input.senderRev, path: null } }; }, }; const ws = new Workspace({ storage: makeStorage(), backends: [makeBackend("fake", rpc)] }); diff --git a/packages/workspace/tests/stub-soak-worker.ts b/packages/workspace/tests/stub-soak-worker.ts index f3a43cc5..8c7f7a87 100644 --- a/packages/workspace/tests/stub-soak-worker.ts +++ b/packages/workspace/tests/stub-soak-worker.ts @@ -38,12 +38,12 @@ export interface Env { function fakeBackend(): WorkspaceBackend { const sync: SyncRPC = { async push() { - return { rev: 0, appliedPushRev: 0 }; + return { rev: 0, appliedPushCursor: { rev: 0, path: null } }; }, async fetchChanges() { return { - currentRev: 0, - appliedPushRev: 0, + currentCursor: { rev: 0, path: null }, + appliedPushCursor: { rev: 0, path: null }, stream: new ReadableStream({ start(c) { c.close(); @@ -55,7 +55,7 @@ function fakeBackend(): WorkspaceBackend { return null; }, async watermarks() { - return { currentRev: 0, pushRev: 0, fetchRev: 0 }; + return { currentRev: 0, pushRev: 0, fetchCursor: { rev: 0, path: null } }; }, async hasObjects() { return []; diff --git a/packages/wsd/src/cli/wsd.test.ts b/packages/wsd/src/cli/wsd.test.ts index 64933b6d..9cf132b5 100644 --- a/packages/wsd/src/cli/wsd.test.ts +++ b/packages/wsd/src/cli/wsd.test.ts @@ -116,7 +116,10 @@ test("/ws serves a capnweb WorkspaceRPC session", async (_ctx) => { // hasObjects against a fresh DB returns the empty subset. expect(await client.sync.hasObjects([])).toEqual([]); // fetchChanges streams zero entries against a fresh DB. - const { stream } = await client.sync.fetchChanges({ sinceRev: 0, ignore: [] }); + const { stream } = await client.sync.fetchChanges({ + after: { rev: 0, path: null }, + ignore: [], + }); const reader = stream.getReader(); const entries = []; while (true) { diff --git a/packages/wsd/src/fuse/vfs.test.ts b/packages/wsd/src/fuse/vfs.test.ts index 7fd35061..c6111f96 100644 --- a/packages/wsd/src/fuse/vfs.test.ts +++ b/packages/wsd/src/fuse/vfs.test.ts @@ -30,8 +30,8 @@ test("createNodeVirtualFileSystem pulls initial state from an upstream SyncRPC", async fetchChanges() { fetchChangesCalls++; return { - currentRev: 1, - appliedPushRev: 0, + currentCursor: { rev: 1, path: null }, + appliedPushCursor: { rev: 0, path: null }, stream: new ReadableStream({ start(c) { c.enqueue({ @@ -62,7 +62,7 @@ test("createNodeVirtualFileSystem pulls initial state from an upstream SyncRPC", }); }, async push() { - return { rev: 0, appliedPushRev: 0 }; + return { rev: 0, appliedPushCursor: { rev: 0, path: null } }; }, async pushObjects() {}, };