Upstream deletes from the container are dropped as stale: tombstoneIsStale compares revs from different peers' rev spaces

[mjaskolski]

Summary

Deletes made inside the container (rm, and therefore mv) are never applied to the Durable Object's VFS once the DO's local revision counter is ahead of computerd's. tombstoneIsStale compares revisions from two unrelated counters.

Environment

• @cloudflare/computer 0.3.1 with computer-computerd-linux-x64:0.3.1.
• withWorkspaceContainer + CloudflareContainerBackend, standard-1.
• Same code on main (e5e28a7).

What happens

1. A workspace has some history. Local writes via ws.fs.writeFile bump vfs_meta.rev, and so does every file pulled back from the container, because linkStagedChunksSync stamps it with a fresh local rev.
2. ws.runtime.exec('rm x', { backend: 'linux' }).
3. The container's ls no longer shows x, and result() reports sync.status: 'complete', pulled: 0.
4. x is still in the DO's VFS. It is never re-pushed, because it did not change locally, so the two sides stay diverged. A mv a b arrives as "b created, a still present".

Files created in the container do propagate, because alreadyApplied compares manifest hashes, not revs.

Cause

packages/dofs/src/sync/apply.ts:

function tombstoneIsStale(db, entry) {
  const live = resolveInode(db, entry.path, { followSymlinks: false });
  if (live === null) return false;
  const row = db.one("SELECT rev FROM vfs_nodes WHERE inode = ?", live.inode);
  return row !== undefined && row.rev > entry.rev;
}

• row.rev is the local node's rev (local vfs_meta.rev space).
• For an upstream entry, entry.rev comes from the peer's change log (computerd's own counter).
• packages/rpc/src/server.ts itself describes the push caller as "a sync peer with its own rev space".

computerd starts fresh after every sleep, so its counter is small, while the DO's grows with every write and pull. row.rev > entry.rev therefore holds for practically every path, and every container delete is dropped as "stale".

On main the predicate is also used by applyChangesSync (apply.ts:426), so the RPC server path has the same comparison. The same predicate runs on the computerd side for deletes pushed from the DO. It works today only because the DO's revs are larger, and would drop DO deletes if the container's counter were ever ahead.

Repro without a container

Drive a real Workspace over node:sqlite against a fake SyncRPC/ShellRPC peer whose counter starts at 1:

1. Write /workspace/x a few times so the local rev is well above the peer's.
2. Make the fake command append { kind: 'delete', path: '/workspace/x', rev: <peer rev> } to the peer's change log.
3. Run ws.runtime.exec(..., { backend }).

Result: pulled === 0 and /workspace/x still exists.

Workaround we are running

One possible fix, which we applied as a local patch to dist/index.js: decide staleness by whether the peer has seen the local version. A node pushed to this backend (rev <= pushRev) yields to the peer's delete; a local change newer than the last completed push still wins.

function tombstoneIsStale(db, entry, backend) {
  const live = resolveInode(db, entry.path, { followSymlinks: false });
  if (live === null) return false;
  const row = db.one("SELECT rev FROM vfs_nodes WHERE inode = ?", live.inode);
  if (row === void 0) return false;
  return row.rev > readWatermark(db, "pushRev", backend ?? DEFAULT_BACKEND_ID);
}
// call site in applyChanges: tombstoneIsStale(db, entry, options.backend)

After a watermark divergence (pullOnceImpl resets pushRev to 0), deletes stay conservatively dropped until the next push re-establishes the watermark. We are not sure this is the right fix for the computerd side, or for pack pulls, but the cross-space comparison looks unintended.

[scuffi]

@gardener-cf can you triage and find a fix for this issue?

[github-actions]

@scuffi Triaged: the report is valid. tombstoneIsStale in packages/dofs/src/sync/apply.ts compares the live node's rev (local counter) against entry.rev (peer's counter), and packages/rpc/src/server.ts applies peer pushes as source: 'upstream' — so container deletes are checked across unrelated rev spaces and dropped once local revs run ahead. Likely fix: decide staleness in local rev space, e.g. compare against the backend's pushRev watermark like your patch, minding watermark resets and pack pulls. Needs a code change; none made here.

[scuffi]

@gardener-cf validate the issue and push a PR with a fix