lance-graph-hydrate: merge hydrate_dir/hydrate_file staging bodies

.claude/board/ISSUES.md

@@ -26,7 +26,7 @@ file must be mirrored by hand. Both module docs say so; neither can enforce
 it, and nothing fails when they diverge. That is the live risk this entry
 exists to keep visible.
 
-## ISS-HYDRATE-DIR-AND-FILE-DUPLICATE-THEIR-STAGING-BODIES (2026-08-17) — OPEN, partially-mitigated
+## ISS-HYDRATE-DIR-AND-FILE-DUPLICATE-THEIR-STAGING-BODIES (2026-08-17) — RESOLVED (follow-up PR)
 
 `copy::hydrate_dir` and `file::hydrate_file` carry near-identical
 staging → fetch → verify → publish-by-rename bodies. PR #958 unified only the
@@ -50,6 +50,44 @@ the merge worth doing deliberately, not as diff-noise inside a hardening PR.
 Cheap to do now precisely BECAUSE consumers are still zero — that window closes
 the moment q2 or OGAR wires this in.
 
+> **⊘ RESOLVED (follow-up PR, `crates/lance-graph-hydrate/src/publish.rs`).**
+> Landed while the zero-consumer window was still open, per this entry's own
+> reasoning. Extracted `publish::publish_by_rename` — the PUBLISH half only
+> (pre-rename re-check, rename, cleanup-on-error, rename-race remap); the
+> FETCH half (list+get many objects vs stream+hash one object) stayed
+> per-caller, genuinely different in shape. `copy::hydrate_dir` and
+> `file::hydrate_file` now route ALL staging removal (not only the publish
+> tail — the fetch-error and empty/checksum-reject paths too) through the
+> same `publish::remove_staging`, so the "cleanup ladder" half of this
+> entry's finding is closed as completely as the "rename-race remap" half.
+>
+> **The asymmetry this entry named (dir fails loudly ENOTEMPTY; file
+> silently clobbers) is preserved, not flattened** — `publish_by_rename`
+> now runs BOTH defenses (a pre-rename re-check AND a post-rename remap) for
+> BOTH callers, which is strictly a narrowing of each window, never a
+> weakening: the pre-check was file's real defense (its danger case never
+> manifests as a rename error to remap) and is now also narrowing dir's
+> window further; the post-rename remap was dir's real defense and is inert
+> defense-in-depth for file. See `publish.rs`'s own module doc for the
+> full reasoning, so it isn't restated in a second place — which was the
+> point.
+>
+> Four new falsifiers in `publish.rs` prove the merge, including two that
+> could not be written before this seam existed (the pre-check narrowing
+> `hydrate_dir`'s window is a NEW guarantee, not merely a refactor of an old
+> one): `publish_by_rename_directory_race_is_reported_as_already_published_
+> and_cleans_up` and its file-kind twin construct the exact race (a
+> competing publisher already occupies the destination when this caller
+> reaches its own publish attempt) and assert BOTH that the loser is
+> reported `AlreadyPublished` (not a raw I/O error) AND that the winner's
+> content survives untouched — a two-sided proof, not just "it doesn't
+> crash". All 4 pre-existing `hydrate_dir`/`hydrate_file` tests pass
+> unchanged (the merge is behavior-preserving at every previously-tested
+> path); `lib.rs`'s own "does NOT merge" deferral paragraph is replaced with
+> a "follow-up landed" pointer to this entry, so a future reader of the
+> crate doc sees the resolution without needing to cross-reference here
+> first.
+
 ## ISS-HYDRATE-NAME-COLLIDES-WITH-TWO-EXISTING-WORKSPACE-MEANINGS (2026-08-17) — OPEN, cosmetic, no-correctness-impact
 
 Two names in `lance-graph-hydrate` already mean something else in this

.claude/board/LATEST_STATE.md

@@ -1,3 +1,13 @@
+## 2026-08-17 — branch `claude/hydrate-staging-merge` — `lance-graph-hydrate`: the deferred staging/publish merge lands (closes `ISS-HYDRATE-DIR-AND-FILE-DUPLICATE-THEIR-STAGING-BODIES`)
+
+### Current Contract Inventory — new private module (`crates/lance-graph-hydrate/src/publish.rs`)
+
+- **`publish::publish_by_rename(staging, publish_path, StagingKind) -> Result<(), PublishError>`** — the PUBLISH half of `copy::hydrate_dir` and `file::hydrate_file`'s near-identical bodies, extracted per the `crate/lance-graph-hydrate`-hardening council's own named follow-up (`.claude/board/ISSUES.md` `ISS-HYDRATE-DIR-AND-FILE-DUPLICATE-THEIR-STAGING-BODIES`, filed 2026-08-17, resolved same day once the zero-consumer window it was priced against was confirmed still open). The FETCH half (list+get many objects vs stream+hash one object) stayed per-caller — genuinely different in shape, merging it would obscure more than clarify.
+- **A real improvement, not just deduplication**: `publish_by_rename` runs BOTH a pre-rename re-check AND a post-rename ENOTEMPTY remap for BOTH callers. Before this: `hydrate_dir` had only the post-rename remap (no pre-check — its window was wider than it needed to be); `hydrate_file` had only the pre-check (its real defense, since POSIX file-onto-file rename SILENTLY CLOBBERS rather than erroring, so a post-rename remap can't see the danger case). Unifying to BOTH strictly narrows each window; it weakens neither.
+- **`publish::remove_staging(staging, StagingKind) -> io::Result<()>`** — also absorbed the fetch-error and empty/checksum-reject cleanup call sites in both callers (not only the publish tail), so the ISS's "own cleanup ladder" half is closed as completely as the "own rename-race remap" half. Callers choose swallow (`let _ = ...`) vs propagate (`...?`) per call site — the Ok-path-must-not-lie behavior from the prior council pass (C7) is preserved exactly, not weakened by the extraction.
+- **Gates:** 4 new falsifiers in `publish.rs` (publish-a-dir, publish-a-file, and — the two that could not exist before this seam — a directory-race and a file-race test, each constructing a competing publisher already at the destination and asserting BOTH `AlreadyPublished` (not a raw I/O error) AND that the winner's content survives untouched). All 4 pre-existing `hydrate_dir`/`hydrate_file` tests pass unchanged — the merge is behavior-preserving at every previously-tested path.
+- **Verification status — same honest caveat as the prior two PRs on this crate**: not locally compiled in this session's container (disk-constrained; see the 2026-08-17 entries below for the full history). Real verification is this repo's CI (`rust-test.yml` et al.), watched per the standing PR-ownership protocol.
+
 ## 2026-08-17 — branch `claude/lance-graph-hydrate-hardening-council` — 5+3 council hardens `lance-graph-hydrate` (merged PR #957); corrects 3 claims made in the entry below
 
 **Council record:** `.claude/plans/hydrate-crate-hardening-council-v1.md` —

crates/lance-graph-hydrate/src/copy.rs

@@ -16,10 +16,13 @@
 //! `E-A-REPEATABLE-TRANSFER-IS-NOT-IDEMPOTENCE-OVER-A-MULTI-FILE-DIRECTORY-1`,
 //! cited rather than restated as new): objects land in a private sibling
 //! staging directory first, then ONE atomic directory rename publishes the
-//! complete artifact. A caller observing the publish path therefore either
-//! sees nothing (not yet hydrated) or the complete artifact (hydrated) —
-//! never a partial one. This is a filesystem-atomicity boundary, deliberately
-//! NOT a lock/lease protocol.
+//! complete artifact — via `crate::publish::publish_by_rename`, shared
+//! with [`crate::file::hydrate_file`] (see that module's doc for the merge
+//! and why the TOCTOU-narrowing details live there now, not restated here).
+//! A caller observing the publish path therefore either sees nothing (not
+//! yet hydrated) or the complete artifact (hydrated) — never a partial one.
+//! This is a filesystem-atomicity boundary, deliberately NOT a lock/lease
+//! protocol.
 //!
 //! **The idempotency boundary has TWO conditions, not one** (doctrine §4a):
 //! (a) a pinned source version and (b) an empty/uncontested destination.
@@ -30,6 +33,7 @@
 //! snapshot or an in-flux one. (For the single-object case,
 //! [`crate::file::hydrate_file`]'s checksum pin IS condition (a).)
 
+use crate::publish::{publish_by_rename, remove_staging, PublishError, StagingKind};
 use crate::staging::staging_suffix;
 use futures::TryStreamExt;
 use object_store::{path::Path as ObjPath, ObjectStore};
@@ -90,7 +94,7 @@ pub async fn hydrate_dir(
     let report = match fetch_all(store, remote_root, &staging).await {
         Ok(r) => r,
         Err(e) => {
-            let _ = tokio::fs::remove_dir_all(&staging).await;
+            let _ = remove_staging(&staging, StagingKind::Dir).await;
             return Err(e);
         }
     };
@@ -100,28 +104,17 @@ pub async fn hydrate_dir(
         // `Ok` return promises "leaves NOTHING at publish_dir", and silently
         // discarding this error would let that promise be false (a council
         // finding, 2026-08-17).
-        tokio::fs::remove_dir_all(&staging).await?;
+        remove_staging(&staging, StagingKind::Dir).await?;
         return Ok(report);
     }
 
-    if let Err(e) = tokio::fs::rename(&staging, publish_dir).await {
-        // A concurrent hydrate_dir call may have published between our entry
-        // check and this rename. `rename` onto an existing non-empty
-        // directory fails (ENOTEMPTY) — remap that specific race to the
-        // documented `AlreadyPublished` contract instead of leaking it as an
-        // opaque `Io` error. This NARROWS the TOCTOU window (checked at
-        // entry AND at rename) — it does not CLOSE it: a publish landing in
-        // the instant between this re-check and returning is still
-        // (vanishingly) possible. No lock is taken (doctrine: filesystem-
-        // atomicity boundary, not a coordination protocol).
-        let _ = tokio::fs::remove_dir_all(&staging).await;
-        return if publish_dir.exists() {
+    match publish_by_rename(&staging, publish_dir, StagingKind::Dir).await {
+        Ok(()) => Ok(report),
+        Err(PublishError::AlreadyPublished) => {
             Err(HydrateError::AlreadyPublished(publish_dir.to_path_buf()))
-        } else {
-            Err(e.into())
-        };
+        }
+        Err(PublishError::Io(e)) => Err(e.into()),
     }
-    Ok(report)
 }
 
 async fn fetch_all(

crates/lance-graph-hydrate/src/file.rs

@@ -1,8 +1,11 @@
 //! Single-file hydration: checksum-pinned download with the same
 //! hydrate-aside/publish-by-rename discipline as [`crate::copy::hydrate_dir`]
-//! (see that module's doc for the mechanism's doctrine citation and
-//! provenance), generalized from q2's `osm_slab_hydrate.rs::download_verified`
-//! (the `.part` + atomic-rename sidecar pattern for one artifact rather than a
+//! — shared via `crate::publish::publish_by_rename`, see that module's
+//! doc for the mechanism's doctrine citation, provenance, and why this
+//! function's pre-rename re-check (its real defense against POSIX's
+//! file-onto-file silent-clobber rename semantics) now lives there —
+//! generalized from q2's `osm_slab_hydrate.rs::download_verified` (the
+//! `.part` + atomic-rename sidecar pattern for one artifact rather than a
 //! whole Lance directory — e.g. a `SHA256SUMS` manifest, a single baked
 //! `.soa`/`.chains`/`.books` sidecar).
 //!
@@ -11,6 +14,7 @@
 //! [`crate::copy::hydrate_dir`] has no equivalent and names condition (a) as
 //! the caller's responsibility instead.
 
+use crate::publish::{publish_by_rename, remove_staging, PublishError, StagingKind};
 use crate::staging::staging_suffix;
 use futures::TryStreamExt;
 use object_store::{path::Path as ObjPath, ObjectStore};
@@ -82,41 +86,27 @@ pub async fn hydrate_file(
     let hasher = match fetch_result {
         Ok(h) => h,
         Err(e) => {
-            let _ = tokio::fs::remove_file(&part_path).await;
+            let _ = remove_staging(&part_path, StagingKind::File).await;
             return Err(e);
         }
     };
 
     let actual = hex_lower(&hasher.finalize());
     if !actual.eq_ignore_ascii_case(expected_sha256_hex) {
-        let _ = tokio::fs::remove_file(&part_path).await;
+        let _ = remove_staging(&part_path, StagingKind::File).await;
         return Err(HydrateFileError::ChecksumMismatch {
             expected: expected_sha256_hex.to_string(),
             actual,
         });
     }
 
-    // Re-assert immediately before the rename: unlike `hydrate_dir`'s
-    // dir-onto-dir rename (which fails loudly, ENOTEMPTY, on a race), a
-    // file-onto-file `rename` on POSIX SILENTLY CLOBBERS an existing
-    // destination rather than failing — so the danger case is exactly the
-    // one where the rename would otherwise SUCCEED, not where it errors.
-    // This narrows (does not eliminate) the TOCTOU window between the entry
-    // check and here; see `copy::hydrate_dir`'s doc for the same caveat
-    // stated once (a council found the entry-only check here left this race
-    // completely unguarded, 2026-08-17).
-    if publish_path.exists() {
-        let _ = tokio::fs::remove_file(&part_path).await;
-        return Err(HydrateFileError::AlreadyPublished(
+    match publish_by_rename(&part_path, publish_path, StagingKind::File).await {
+        Ok(()) => Ok(()),
+        Err(PublishError::AlreadyPublished) => Err(HydrateFileError::AlreadyPublished(
             publish_path.to_path_buf(),
-        ));
-    }
-
-    if let Err(e) = tokio::fs::rename(&part_path, publish_path).await {
-        let _ = tokio::fs::remove_file(&part_path).await;
-        return Err(e.into());
+        )),
+        Err(PublishError::Io(e)) => Err(e.into()),
     }
-    Ok(())
 }
 
 fn hex_lower(bytes: &[u8]) -> String {

crates/lance-graph-hydrate/src/lib.rs

@@ -64,20 +64,24 @@
 //! policy from `.claude/plans/idle-flush-dataset-eviction-v1.md` — that plan
 //! is still a PROPOSAL. This crate ships the mechanisms the policy would
 //! call (hydrate, dirty-check, flush-gate, release); the scheduling policy
-//! is deliberately out of scope for v1. It also does not merge
-//! [`copy::hydrate_dir`] and [`file::hydrate_file`]'s near-identical
-//! staging/publish bodies into one shared primitive beyond the shared nonce
-//! helper (`staging::staging_suffix`) — a 5+3 hardening council on this
-//! crate (2026-08-17) named that merge as worth considering but explicitly
-//! deferred it to keep this PR's diff to the uniqueness fix it actually
-//! needed; tracked as a named follow-up, not silently dropped.
+//! is deliberately out of scope for v1.
+//!
+//! **Follow-up landed:** [`copy::hydrate_dir`] and [`file::hydrate_file`]'s
+//! staging/publish bodies were merged into `publish::publish_by_rename` —
+//! a 5+3 hardening council on this crate (2026-08-17) named the duplication
+//! as `ISS-HYDRATE-DIR-AND-FILE-DUPLICATE-THEIR-STAGING-BODIES`
+//! (`.claude/board/ISSUES.md`) and deferred it deliberately, since it was
+//! cheap only while this crate had zero consumers; that window was still
+//! open, so this follow-up closes it. See the `publish` module's doc for
+//! what merged and what stayed per-caller.
 
 pub mod copy;
 pub mod dirty;
 pub mod env;
 pub mod file;
 pub mod lifecycle;
 pub mod marker;
+mod publish;
 pub mod release;
 mod staging;