Capture only whole blobs in a backup, substituting a marker for one still being written

[kriszyp]

A blob is written in place at its final path and only carries its real size once the write ends, so the file exists and grows for the whole write. The backup walk hard-linked whatever it found, with no check, so a snapshot could share an inode whose bytes then kept changing — or ended truncated if the write aborted. The module's own doc comment asserted the opposite.

Every entry is now classified by one shared rule (classifyBlobFileForCapture, in resources/blob.ts where the on-disk format lives). A complete blob and a stable abort marker are linked as before. Anything else gets a marker written into the snapshot rather than being dropped — retryable (PENDING) if it was not complete yet, terminal (ERROR) if its bytes are gone for good. Dropping is what must not happen: getNextFileId recovers the id counter by scanning the directory, so an absent file lets a restored record's blob id be reissued to a different record. The get_backup tar walk goes through the same rule; it was archiving truncated in-flight blobs too, and had drifted to turning an ERROR stub into PENDING.

Fixes #2262.

For the human reviewer

1. A marker instead of the writer's final bytes (in-flight-blob-stub-vs-live-inode). On the same filesystem the old hard link self-healed: the snapshot shared the inode, so a write that went on to succeed left the snapshot holding the complete blob. This rule gives that up — a 4 GB blob streaming during a nightly backup that finishes 200 ms later used to restore intact, and now restores as a stub. It's chosen because the old behavior was correct only when the write succeeded, and silently truncated when it aborted, with no way to tell the two apart at restore. If you'd rather keep the win, the reviewer-suggested middle path is real: refuse to overwrite a live complete blob during restore. Every backup taken meanwhile bakes the choice in — reversing later doesn't repair snapshots already written.

2. A mid-walk delete becomes terminal, not retryable (gone-as-terminal-error-type). Those bytes are unrecoverable, and cleanupOrphans will never reclaim the marker because the checkpointed record still references the id — so PENDING would promise a retry that can never succeed and hold each read for blobReadTimeout first. ERROR says so honestly. Cost: a peer that does hold the bytes can no longer heal it, because a 500 is replicated as-is where a 503 is held as a gap.

3. A systemic errno fails the whole backup (systemic-errno-fails-whole-backup). EIO/EMFILE/ENOSPC/EROFS propagate rather than degrading that file to a marker. This is a policy call about whether a partial backup beats no backup, and note the blast radius: finalizeBackup responds to a throw by deleting the engine backup too. Non-systemic failures deliberately fall back to capturing the file unverified, because link() needs no read permission on the source and a read failure is not evidence the bytes are bad.

4. Classification at capture time, not quiescing writes (classify-in-both-walks-vs-quiesce-blob-writes). The root-cause fix is for backup to coordinate with the blob write lock, or to capture only ids present in the checkpoint, excluding in-flight blobs by construction — that would delete this whole classification path. Not done here because it needs the store handle in the walk and a lossless path→fileId mapping, neither of which exists today. Expensive to retrofit once the marker format is in shipped backups.

5. Substitution is invisible in the manifest (substitution-invisible-in-manifest). It's a log line at backup time. An operator restoring months later gets success and then 503s, with no record of which ids need repair. A count in the manifest is the fix; it's a format change restore would have to read, so it's not bundled here — and adding it later doesn't annotate existing backups.

6. Blob internals exported to dataLayer (capture-surface-in-resources-blob). classifyBlobFileForCapture, createCaptureMarker, and isSystemicIoError are exported rather than hidden behind one façade, because the two consumers need different capture mechanics (hard link vs pack.entry) and share only the classification. harper-pro sees this surface too.

7. Every compressed blob is fully inflated during capture (surfaced by the graded leg). A deflate header records the uncompressed length, so a growing compressed body is invisible to the size check and has to be read — which makes backup cost scale with logical rather than stored size. I tried gating this on an mtime window and removed it: a stalled deflate write carries a final-looking header from its first byte, so the byte-copying capture paths (get_backup's stream, the cross-filesystem copyFile fallback) would have truncated it. Accepted because compress is opt-in per createBlob and nothing in core passes it, so an ordinary corpus reads nothing — but it is a latent cost for anyone who does opt in, and a cheap deflate-terminator check is the obvious future out.

One claim a core-only reviewer cannot check. The capture arm for an existing abort marker is safe only if nothing rewrites a published blob path in place. Two comments in resources/blob.ts previously asserted the opposite ("the re-stream overwrites this stub, createWriteStream flags 'w'"); both are corrected here. I verified it against harper-pro — replication/replicationConnection.ts:6524-6531 builds a fresh blob via createBlob(stream, remoteBlob) and takes a new file id, so the stub is orphaned rather than truncated, and repairBlobFile is the only same-id writer (via .repair + rename). That stale sentence caused three separate wrong readings of this bug, two of them mine. A pro-side inode assertion is the right follow-up to make it durable; nothing tests it today.

Verification

Fails-on-base, against git merge-base HEAD main sources with a forced clean rebuild (rm -rf dist) and the built artifact grepped to confirm base code was actually under test:

• unitTests/dataLayer/blobBackup.test.js — 5 failing on base
• unitTests/dataLayer/rocksdbBackup.test.js "packs a PENDING marker…" — failing on base
• unitTests/resources/blob.test.js — 3 failing on base, including the end-to-end case: a real createBlob write streaming while snapshotBlobs walks the root, asserting a different inode, a marker, and that snapshot bytes do not change after the write completes

Reader-level classification is asserted through the real read path, not by inspecting a header byte: a restored retryable marker reads 503, a terminal one reads 500.

Two tests pass with and without the fix and are forward regression guards, not bug proofs: "preserves an existing PENDING marker" and "captures a complete compressed blob".

Suites: test:unit:backup 82 passing; unitTests/resources/blob.test.js 106 passing; tsc --noEmit, lint:required, prettier clean. Not run locally: the test:unit:main / test:unit:resources full gates (shared-lock contention on this machine) — relying on CI.

Complexity: medium

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=12 @ 4a9931b}

_{Human-Review-Need: 4 (decisions: stub-in-flight-vs-wait-for-completion, gone-marker-terminal-vs-retryable, classifier-lives-in-blob-ts, systemic-error-list-aborts-the-backup, repair-temps-now-omitted-from-the-tar) @ 4a9931b}

[gemini-code-assist]

Code Review

This pull request introduces a mechanism to safely capture blob files during live backups without freezing writes. Instead of capturing incomplete or growing blobs, the backup process now classifies each blob file and substitutes incomplete or deleted blobs with a PENDING or GONE marker. This reserves the file ID and allows for subsequent repair after a restore. The changes span blobBackup.ts, rocksdbBackup.ts, and blob.ts, along with corresponding unit tests. Feedback on the changes suggests explicitly destroying the inflate stream in inflatesToExactly to prevent potential zlib resource leaks during stream failures.

[claude]

Reviewed; no blockers found.