Refuse an engine-only restore over a database that still has blobs

[cb1kenobi]

restoreBlobSnapshot leaves the live blob roots untouched and logs a warning when a backup has no blob snapshot. That is right in isolation — purging them would strip blobs the restored records still reference — but it means restoring an exclude_blobs backup silently produces a mixed generation: rolled-back records addressing whichever blobs are on disk now. The operator got a log line and a successful restore.

That restore is now refused unless the caller passes allow_engine_only, and the restore itself runs behind a fence that stops every blob-root mutation while it purges and rewrites.

For the human reviewer

1. This breaks existing scripted engine-only restores, including into a new database. Anything running restore_backup against an exclude_blobs backup now fails until allow_engine_only is added. Naming a fresh target_database is not an escape, which is the part that changed late: blob ids are a per-database counter that getNextFileId re-seeds by scanning the roots, so empty roots re-seed at 1 — the id space the restored records already reference — and the next blob written lands on a path one of them points at. An empty root is not evidence that a backup is blob-free.
2. The guard reads the manifest and the opt-in, and nothing else. No filesystem access, so it cannot be slow or fail on an unreadable root, and each path checks once because no concurrent writer can change a manifest-only answer.
3. The durable fix for hazard 1 is not here. Persisting the blob-id high-water mark with the engine, so the counter cannot fall behind the references it has handed out, would let the opt-in be dropped for the empty-root case — and would also fix the same latent hazard outside restore, where a cold start after deletions can reissue a live id. That is a change to the blob write path rather than a guard, so it wants its own PR. The reasoning is recorded in the JSDoc.
4. Closing a database is not a write barrier, so restore has explicit close and reload phases. A save is an async file pipeline that outlives the handle it started from; a reclamation is a timer that never consults the database; an orphan sweep is dispatched fire-and-forget. All three are fenced.
5. The fence is a current owner, not a flag or a token set. Restores of one database can overlap, because a restore releases its lock before awaiting its reload broadcast. A flag lets the first one's late reload unfence the second. A set is worse in the case that matters: a restore that dies after destruction never reloads, so its token would keep the database fenced for the life of the process — defeating the rerun the error tells the operator to perform. Ownership transfers instead.
6. Long-running work compares generations, not "is it fenced now." An orphan sweep can read the fence as clear, have an entire restore happen while it awaits an unlink, and resume deleting on a verdict reached against the replaced generation. It captures a restore generation instead.

Changes

• dataLayer/blobBackup.ts — assertEngineOnlyRestoreAllowed refuses any blob-excluding backup without the opt-in, decided from the manifest alone, naming both hazards in one message.
• dataLayer/rocksdbBackup.ts — both restore paths check once before anything destructive; the restore mints an owner token carried on both schema phases, and the reload phase reports whether the generation was actually replaced; validateRestoreBackup and the offline path share one boolean validation; the generated backup README explains why a new target is not an escape.
• resources/blob.ts — the owner fence, the restore generation, and fencing for saves, deferred reclamation, save-failure cleanup and orphan cleanup. Unlink tracking is a counter with waiters, so the no-restore path allocates nothing beyond a map lookup.
• server/itc/serverHandlers.js, server/threads/itc.js, utility/signalling.ts — the close broadcast is strict, bounded at 30s, and includes job workers, because they write blobs too; a close-barrier failure fails the restore with a retryable 409 rather than proceeding or hanging.
• dataLayer/DESIGN.md — the barrier and owner-token protocol.

Verification

unitTests/resources/blob.test.js (129 passing) covers the save drain, owner replacement, rerun recovery after a restore that died mid-flight, the reclamation discard, the aborted-restore case that must not discard, the generation change, and the fenced orphan sweep. unitTests/dataLayer/ (74 passing) covers the guard directly and through real offline restores. unitTests/server/itc/, unitTests/utility/signalling.test.js and unitTests/server/threads/stuckWorkerDiagnostics.test.js (67 passing) cover the strict broadcast and handler ordering.

Fails-on-base performed for the reclamation fence: removing the fence skip and the generation discard makes a reclamation queued before a restore does not unlink what the restore wrote fail, and restoring them makes it pass.

Known gaps, stated rather than implied:

• No test drives a real online restore end to end through worker broadcasts and snapshot replacement. Coverage is helper-level and offline.
• signalSchemaChange uses Promise.all over the local close and the bounded peer broadcast, so a peer timeout can reject while a local close handler is still draining. Not addressed.
• Two findings about the thread fabric are open and not addressed here: job cleanup can satisfy the strict barrier without draining blob mutations, and a worker registered after the broadcast's recipient snapshot can miss the fence entirely.
• A failed save can settle before its asynchronous ERROR-stub write, so that write can land after the restore. Open.
• cleanup_orphan_blobs is still dispatched without a rejection observer (pre-existing).

Complexity: complicated

---

Framing-Verdict: chosen-approach-sound

The planning review first returned better-alternative-exists and was right: it identified that a token set makes a failed restore unrecoverable without a process restart, and that the fence did not cover orphan cleanup or save-failure deletion. Both were adopted — the fence became a current owner and the deletion paths were brought in — and the re-run cleared.

Fence release, not a fence lease. A review suggestion asked for an expiry on the restore fence so a failed restore could not leave a database fenced for the life of the process. Declined: a deadline is the wrong shape for this barrier, because one elapsing during a slow restore would silently unfence mid-purge and re-admit the saves the fence exists to exclude. Most of the harm was already covered — an abandoned restore marker refuses to load the database at all (409, naming the rerun). The uncovered case is an operator who drops/recreates the name instead of rerunning, so the destructive-failure path now broadcasts its reload on the way out, releasing the fence deterministically when the restore ends. This reverses what I told @kriszyp earlier on that thread, and is called out there.

Cross-model review. codex gpt-6-astra, Gemini 3.1 Pro and cursor-grok, planning and implementation rounds against main. The receipt is re-pinned to the current head after a main merge; this PR's own files are byte-identical to the reviewed sha (8539c00), so the reviewed artifact and the head artifact are the same change. The Harper-domain adjudication leg was SIGKILLed in every round, so findings were triaged by the author rather than adjudicated — weaker, and the reason Human-Review-Need is not lower. In the final round the codex leg also hung and was killed after 720s idle; it delivered on a fresh retry, so that round's coverage is three outside legs rather than a clean single pass.

Adopted from the code review: the orphan sweep's stale-generation blocker, the repair locks its abort leaked, the unconditional reclamation discard that stranded work from an intact generation, strict manifest checking, and offline/online opt-in validation parity.

Round 4 confirmed the release routing correct by code trace and raised no blocker. Its one adopted item was documentation: the JSDoc and design note still described the token set the fence no longer uses — "the last token is released", "while any token is outstanding" — which is the shape whose flaw the code was changed to avoid. Corrected. Human-Review-Need therefore pins one commit behind head: the only delta since is that comment text, with no executable change (verified by diff).

Adopted from the final-artifact round: the restore release now reaches job workers. Opting them into the close broadcast fenced them, but the reload fell through to the ordinary path that excludes job workers, so every job thread took the fence and never got it back — the same stuck-fence class fixed a commit earlier, introduced by the close-phase inclusion itself.

Adopted from the delta round before it: the generation is captured before the sweep's scan rather than before its deletions (a restore landing during the scan already invalidates the verdict); abandoning the sweep no longer duplicates the candidate set to find its held locks, which is a heap risk on exactly the hours-long sweeps this path supports; and lifting a fence now wakes the reclamation queue, since a drain that ran while fenced skipped entries without recording a next deadline and nothing came back for them.

Not adopted, each disproved by a line rather than by judgement:

• Gemini reported storageInfo as undefined at resources/blob.ts:1654. It is declared at resources/blob.ts:1588 as pending.fileInfo — the exact value the finding proposed switching to.
• Gemini reported requireBooleanOption breaking clients that omit the flag. It returns false for undefined and only throws on a non-boolean (dataLayer/rocksdbBackup.ts:169-174).
• Both lenses questioned CLI boolean coercion. buildRequest runs JSON.parse on every key=value whose field is not in RAW_STRING_FIELDS, and that set is {'ref'} (bin/cliOperations.ts:630-646), so a real boolean arrives. A malformed value did not, which is what the shared validation now fixes.

_{Review-Coverage: authored=claude; ran=gemini,codex,cursor-grok; blocked=domain(exit--1); declined=cursor-composer,cursor-kimi,cursor-muse; rounds=4; full=3 @ cb30d47}

_{Human-Review-Need: 4 @ 35b6777}

[gemini-code-assist]

Code Review

This pull request introduces a safety check to prevent accidental in-place restores of engine-only backups (backups created with exclude_blobs) over databases that still contain blob files. It adds the blobRootsHaveFiles and assertEngineOnlyRestoreAllowed helper functions in dataLayer/blobBackup.ts to detect existing blobs and enforce this restriction unless the operator explicitly opts in by passing allow_engine_only=true. The online and offline restore paths in dataLayer/rocksdbBackup.ts and bin/backup.ts have been updated to validate and handle this new option, and comprehensive unit tests have been added to verify the behavior. There are no review comments to address, and the changes conform to the repository's style guidelines.

[kriszyp]

A good safety measure.

_{🤖 Reviewed with Codex}