diff --git a/.claude/board/EPIPHANIES.md b/.claude/board/EPIPHANIES.md index 1cd552fd9..dfa89e915 100644 --- a/.claude/board/EPIPHANIES.md +++ b/.claude/board/EPIPHANIES.md @@ -1,3 +1,124 @@ +## 2026-08-22 — E-THE-GATE-IS-A-HAND-MAINTAINED-ALLOWLIST-NOT-THE-WORKSPACE-1 — nine workspace members are in no CI job at all, and adding one to `members` adds it to nothing + +**Status:** FINDING (measured; every claim is a grep over `.github/workflows/` +and root `Cargo.toml`). **Confidence:** High. **Supersedes the cause** given in +`E-A-CRATE-WITH-ZERO-CONSUMERS-IS-BUILT-BY-NOTHING-AND-CAN-BE-MERGED-BROKEN-1` +(same day, same session, mine). + +That entry explained a crate that did not compile at `main` by saying a crate +with zero consumers is in nobody's build graph. **That is wrong.** +`crates/lance-graph-hydrate` is listed in `[workspace] members` (root +`Cargo.toml:25`); `cargo build --workspace` would compile it. The explanation +was plausible, fitted the symptom, and was never checked against the manifest. + +**The actual mechanism.** No workflow in this repo runs `--workspace` or +`--all`. Every gate names one crate by path: + +``` +build.yml:78 cargo build --manifest-path crates/lance-graph/Cargo.toml +rust-test.yml cargo test --manifest-path crates//Cargo.toml (×14) +style.yml:75+ cargo clippy --manifest-path crates//Cargo.toml (×6) +style.yml:150+ cargo fmt --manifest-path crates//Cargo.toml (×3) +``` + +`style.yml:152` even says so in a comment: *"`cargo fmt --all` never reaches +it."* The gate is a **hand-maintained allowlist**. Membership in +`[workspace] members` is therefore not a build guarantee — it is a lockfile and +`-p` convenience, nothing more. + +**Measured scope, not just my crate.** Of 25 members, ELEVEN appear in no +workflow. Two of those (`lance-graph-catalog`, `lance-graph-planner`) are +dependencies of `lance-graph`, so their LIBS compile inside a gated build +(their tests still never run). The remaining NINE are gated by nothing at all: + +``` +lance-graph-benches lance-graph-rbac +neural-debug lance-graph-ontology +lance-graph-archetype lance-graph-consumer-conformance +sigma-tier-router cognitive-shader-driver +lance-graph-hydrate +``` + +`lance-graph-hydrate` was simply the one that a semver-compatible upstream +change (`object_store 0.13.2` moving `get`/`put` onto `ObjectStoreExt`) happened +to break. Nine crates carry the same exposure; nothing about this was specific +to having no consumer. + +**Why the wrong cause was attractive** — worth naming, because it is the +Kahneman System-1 shape the workspace already warns about: the crate's own +`lib.rs` prominently says it has zero consumers, so a consumer-shaped +explanation was *pre-loaded* by the file being read. It explained the symptom +without contradicting anything visible, which is exactly when a claim needs its +own check and does not get one. The check was one grep of `Cargo.toml`. + +**The durable fix is a gate, not a consumer.** Landing +`VersionedGraph::hydrate_from` (this branch) does make `lance-graph-hydrate` a +dependency of a gated crate, so its lib will now compile in CI — but that is a +side effect, and it leaves its TESTS ungated and the other eight untouched. +The fix is a workflow line per crate, or one `--workspace` job. Filed as +`ISSUES.md` `ISS-CI-GATE-IS-AN-ALLOWLIST-NINE-MEMBERS-UNGATED`. + +**Outcome, same day (PR #984):** both, and the count above was wrong. A +workflow line per crate for every member — each measured locally before its +gate was armed — PLUS `cargo build --workspace` as the net that covers future +members without a line. Wrong count because the member check extracted with +`"crates/[a-z0-9-]+"`: no underscore, so `crates/surreal_container` was +invisible to it, and `tools/dto-class-check` is not under `crates/` at all. +Eleven, not nine. A membership check blind to two of its inputs is this +entry's own defect class, one level up, in the instrument rather than the +workflow — which is the part worth carrying forward. + +## 2026-08-22 — E-A-CRATE-WITH-ZERO-CONSUMERS-IS-BUILT-BY-NOTHING-AND-CAN-BE-MERGED-BROKEN-1 — the hydration crate did not compile at `main`, and its own doc says why nobody found out + +**Status:** ⊘ SUPERSEDED SAME-DAY by +`E-THE-GATE-IS-A-HAND-MAINTAINED-ALLOWLIST-NOT-THE-WORKSPACE-1` — the measured +breakage is real and unchanged; the MECHANISM named below ("zero consumers ⇒ +not in the build graph") is WRONG. `lance-graph-hydrate` is a workspace +MEMBER. Kept in place per append-only; read the successor for the cause. +**Confidence:** High on the failure, RETRACTED on the cause. + +`crates/lance-graph-hydrate` was minted 2026-08-17 (PR #957 + follow-ups) as +the generic object-store → local-volume hydration lifecycle, with a 5+3 +hardening council on it. Its `lib.rs` states plainly that it has no consumer +yet — the deferral of `ISS-HYDRATE-DIR-AND-FILE-DUPLICATE-THEIR-STAGING-BODIES` +is even justified by that fact (*"cheap only while this crate had zero +consumers"*). + +**Measured 2026-08-22, on a clean checkout of `origin/main` (#981):** + +``` +cargo build -p lance-graph-hydrate +error[E0599]: no method named `get` found for reference `&dyn ObjectStore` +... 5 errors, in the LIB, not the tests +``` + +`object_store 0.13.2` — the version the workspace lockfile already pins, on +`main`, unchanged by this branch — moved `get`/`put` off the base +`ObjectStore` trait onto an extension trait `ObjectStoreExt`. The fix is two +`use` lines. `cargo fmt -p lance-graph-hydrate` also rewrites four of its +files, so it was merged unformatted as well. + +**The finding is not the breakage; it is the mechanism that hid it.** A crate +with zero consumers is not in any consumer's build graph. Nothing that CI +actually runs reaches it, so a semver-COMPATIBLE upstream change (0.13.x, no +major bump, no lockfile movement) silently invalidated it and no gate fired. +The absence of a consumer was recorded in the crate's own doc as a *cost +tradeoff* — it is also, and more importantly, a **verification hole**: the +crate's tests pass only in the one command nobody runs. + +Two consequences, both narrow: + +1. **A mint without a consumer needs an explicit build gate**, or it is + documentation with a `Cargo.toml`. Either land the first consumer in the + same arc, or add the crate to whatever CI job actually compiles. +2. **"Hardened by a 5+3 council" is orthogonal to "builds."** The council + reviewed intent, duplication, and doctrine conformance — all real, all + preserved by this fix. None of that is a compiler. + +Corrected in the same commit that gives the crate its first mechanism with a +consumer path (`archive::hydrate_archive`), so the hole closes rather than +being recorded and left open. + ## 2026-08-21 — E-ATTENTION-MASK-IS-A-RENAME-REGISTER-FILE-NOT-A-RESIDUE-CARRIER-1 — the fourth homonym collision of this arc, and the only one where the shipped type is COMPLETE for a different contract **Status:** FINDING (D-ACR-0, report-only deliverable; every claim a read of diff --git a/.claude/board/ISSUES.md b/.claude/board/ISSUES.md index 113bd9052..1efb351ae 100644 --- a/.claude/board/ISSUES.md +++ b/.claude/board/ISSUES.md @@ -1,5 +1,79 @@ # Issues Log — Open + Resolved (double-entry, append-only) +## ISS-CI-GATE-IS-AN-ALLOWLIST-NINE-MEMBERS-UNGATED (2026-08-22) — RESOLVED same-day by PR #984 + +No workflow runs `--workspace` or `--all`. Every gate names one crate by path +(`--manifest-path crates//Cargo.toml`), so the CI surface is a +hand-maintained allowlist and **adding a crate to `[workspace] members` adds it +to no gate**. `style.yml:152` states this in a comment about `cargo fmt --all`. + +Measured 2026-08-22: 11 of 25 members appear in no workflow. Two +(`lance-graph-catalog`, `lance-graph-planner`) are deps of `lance-graph`, so +their libs compile inside a gated build but their tests never run. **Nine are +gated by nothing:** `lance-graph-benches`, `neural-debug`, +`lance-graph-archetype`, `lance-graph-rbac`, `lance-graph-ontology`, +`lance-graph-consumer-conformance`, `sigma-tier-router`, +`cognitive-shader-driver`, `lance-graph-hydrate`. + +This is not hypothetical: `lance-graph-hydrate` did not compile at `main` +(#981) — `object_store 0.13.2`, a semver-COMPATIBLE upstream release already +in the lockfile, moved `get`/`put` onto an extension trait. It was also +fmt-dirty and carried a live clippy warning. Full account: +`EPIPHANIES.md` `E-THE-GATE-IS-A-HAND-MAINTAINED-ALLOWLIST-NOT-THE-WORKSPACE-1`. + +**Fixed here:** `lance-graph-hydrate` gets its `rust-test.yml` + `style.yml` +lines in this branch, and becomes a dependency of `lance-graph` (so its lib +also compiles inside an existing gate). + +**RESOLVED 2026-08-22 by PR #984**, and the resolution went further than what +this entry proposed. The text below is kept as written — it was the state when +filed — with the outcome recorded here rather than edited into it: + +- All members are gated now, not just `lance-graph-hydrate`. #984 measured each + crate locally first and added a test step plus a rustfmt step per crate. +- `cargo build --workspace` landed as the structural net, so a FUTURE member is + covered the moment it is listed in `[workspace]`. That was the standing + choice this entry left to the operator; it was taken. +- The count in this entry's own title is WRONG and stays wrong on purpose. It + says nine; there were eleven, because the member check behind it extracted + with `"crates/[a-z0-9-]+"` — no underscore — and could not see + `crates/surreal_container` or `tools/dto-class-check`. A membership check + blind to two of its inputs is the same defect class this entry describes, + one level up. Both are gated in #984. +- Not taken, still open: a `cargo test --workspace` job. Measured at 14 GB + across 86 binaries against 3.5 GB for the compile — the same order as a + runner's free disk. The per-crate test steps remain, so a new member's TESTS + still need a line. + +Original text, as filed: + +**Still open — deliberately not fixed blind:** the other eight. Some exclusions +may be intentional (a benches crate, a research crate), and adding eight jobs +without knowing which are meant to be gated would trade a silent hole for +silent CI cost. What is needed is one decision — either a single `--workspace` +job (cheapest, catches every future member automatically) or an explicit, +per-crate rationale for each omission recorded next to the allowlist. That +decision is the operator's; this entry is the measurement it needs. + + +## ISS-HYDRATE-CRATE-HAS-NO-BUILD-GATE (2026-08-22) — SUPERSEDED same-day by ISS-CI-GATE-IS-AN-ALLOWLIST-NINE-MEMBERS-UNGATED (the cause was mis-stated; the crate is a workspace MEMBER) + +`crates/lance-graph-hydrate` did not compile at `origin/main` (#981) and was +also fmt-dirty; both are fixed in the commit that files this. See +`EPIPHANIES.md` +`E-A-CRATE-WITH-ZERO-CONSUMERS-IS-BUILT-BY-NOTHING-AND-CAN-BE-MERGED-BROKEN-1` +for the measurement and the mechanism. + +**What is fixed:** the two `ObjectStoreExt` imports and the formatting. + +**What is NOT fixed, and is the actual issue:** nothing in CI compiles this +crate, because no consumer depends on it. The same class of upstream change +can invalidate it again tomorrow with no gate firing. The durable fix is one +of: (a) land the first real consumer — `VersionedGraph::hydrate_from`, the +piece `ISS-REMOTE-URI-CONSTRUCTORS-PREDATE-THE-HYDRATION-DOCTRINE` names — so +the crate enters `lance-graph`'s build graph; or (b) add it explicitly to the +workspace job that runs on every PR. (a) is preferable: it closes two issues +with one edge instead of adding a gate around an unused artifact. ## ISS-CAUSAL-EDGE-CARRIES-SEVEN-PRE-EXISTING-CLIPPY-FINDINGS (2026-08-22) — OPEN `crates/causal-edge` is workspace-EXCLUDED but a path-dep of `lance-graph`, diff --git a/.claude/board/LATEST_STATE.md b/.claude/board/LATEST_STATE.md index 2ebf79b85..ade1da08c 100644 --- a/.claude/board/LATEST_STATE.md +++ b/.claude/board/LATEST_STATE.md @@ -1,5 +1,70 @@ ## 2026-08-21 — D-ACR-7 IMPLEMENTED — `contract::band_reading` (the 59..63 reading contract) +### Current Contract Inventory — `VersionedGraph::hydrate_from`, and the CI gate that should have caught all of this + +- **`lance_graph::graph::versioned::VersionedGraph::hydrate_from`** (new) — + the shape `ISS-REMOTE-URI-CONSTRUCTORS-PREDATE-THE-HYDRATION-DOCTRINE` names. + `local`/`s3`/`azure`/`gcs` address a store WHERE IT SITS; for the three + remote ones that makes the object store the store, which the doctrine + explicitly does not. They are KEPT (addressing a remote store is legitimate + when a caller means it, and better at >1 replica); this adds the doctrine's + own shape beside them. + - **Ensure-hydrated, not hydrate-or-fail**: an existing destination is the + warm path, returned as `Hydration::AlreadyLocal` rather than an error. + - **`Hydration { Fresh(ArchiveReport), AlreadyLocal }`** (new) — the + distinction is returned rather than folded away: a boot that fetched and a + boot that found it are different events. + - **`GraphError::Hydration { source, location }`** (new variant) — its own + variant, not a flattened message, so a caller can tell a checksum mismatch + (never retry) from a transport error (retry). +- **CI is no longer this branch's concern — PR #984 carries it.** An earlier + draft of this entry said `lance-graph-hydrate` is "gated for the first time" + here and that "eight more members are still ungated". Both were true when + written and are false now: #984 gated EVERY member (each measured locally + first), added `cargo build --workspace` as the structural net so future + members need no line, and pinned the toolchain in all seven workflows. The + count was also wrong — eleven, not nine, because the member check behind it + could not see names with an underscore. See `ISSUES.md` + `ISS-CI-GATE-IS-AN-ALLOWLIST-NINE-MEMBERS-UNGATED` (now RESOLVED, with the + outcome recorded above its original text) and `EPIPHANIES.md` + `E-THE-GATE-IS-A-HAND-MAINTAINED-ALLOWLIST-NOT-THE-WORKSPACE-1`. +- Tests: `lance-graph-hydrate` 39 green; `lance-graph` `--lib` green incl. two + new `hydrate_from` falsifiers scoped to what that function adds (the warm/ + error mapping — the archive mechanics are falsified one crate down). + `cargo clippy -p lance-graph --lib --tests -- -D warnings` clean. + +### Current Contract Inventory — 1 new module in `lance-graph-hydrate`, and that crate now COMPILES + +- **`lance_graph_hydrate::archive`** (new) — the `absent -> hydrated` edge for a + dataset shipped as ONE checksum-pinned **zip** object, alongside the existing + `copy::hydrate_dir` (a tree of objects) and `file::hydrate_file` (one plain + object). + - `hydrate_archive(store, remote_object, publish_dir, expected_sha256_hex, + root) -> ArchiveReport` — composes the two mechanisms the crate already + owns (`hydrate_file` for the pinned fetch, `publish::publish_by_rename` + + `StagingKind::Dir` for the atomic publish) and adds only the middle: + expanding one verified container into staging under a containment rule. + - `ArchiveReport { files, bytes }`; `HydrateArchiveError` adds + `EscapingEntry { entry, root }` (Zip-Slip refusal, whole archive rejected + off the CENTRAL DIRECTORY before any byte is written) and + `NoFiles { root }` (an all-directory tree would publish an unopenable + dataset). + - **Zip and not tar, by operator ruling 2026-08-22** (*"bitte als zip, nicht + dass wir ein Verzeichnis mit einzelnen Dateien shippen"*): a zip carries a + central directory, so entries can be enumerated and sought without a + sequential scan — which is also what makes the whole-index validation above + possible before extraction starts. +- **`lance-graph-hydrate` builds again.** It did not compile at `origin/main` + (#981): `object_store 0.13.2` moved `get`/`put` onto `ObjectStoreExt`. Two + `use` lines; four files also picked up `cargo fmt`. Why a merged crate could + be broken at HEAD — and what that says about minting without a consumer — is + `EPIPHANIES.md` + `E-A-CRATE-WITH-ZERO-CONSUMERS-IS-BUILT-BY-NOTHING-AND-CAN-BE-MERGED-BROKEN-1`; + the durable gap is `ISSUES.md` `ISS-HYDRATE-CRATE-HAS-NO-BUILD-GATE`. +- Tests: 39 green in the crate (33 pre-existing, 6 new). Both new guards are + mutation-checked — disabling the containment check and the file counter turns + exactly their two tests red and leaves the other four green. + ### Current Contract Inventory — 1 new zero-dep module + 1 ClassView provided method + 1 gate test in `causal-edge` - **`lance_graph_contract::band_reading`** (new, zero new bytes) — implements diff --git a/Cargo.lock b/Cargo.lock index e58038889..7d154ef17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -152,6 +152,15 @@ dependencies = [ "object", ] +[[package]] +name = "arbitrary" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3d036a3c4ab069c7b410a2ce876bd74808d2d0888a82667669f8e783a898bf1" +dependencies = [ + "derive_arbitrary", +] + [[package]] name = "arc-swap" version = "1.9.1" @@ -2704,6 +2713,17 @@ dependencies = [ "serde_core", ] +[[package]] +name = "derive_arbitrary" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e567bd82dcff979e4b03460c307b3cdc9e96fde3d73bed1496d2bc75d9dd62a" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "derive_builder" version = "0.20.2" @@ -4626,6 +4646,7 @@ dependencies = [ "lance-graph-catalog", "lance-graph-cognitive", "lance-graph-contract 0.1.0", + "lance-graph-hydrate", "lance-graph-planner", "lance-index", "lance-linalg", @@ -4763,6 +4784,7 @@ dependencies = [ "tempfile", "thiserror 1.0.69", "tokio", + "zip", ] [[package]] @@ -4771,7 +4793,6 @@ version = "0.1.0" dependencies = [ "lance-graph-contract 0.1.0", "lance-graph-ontology", - "ogar-adapter-surrealql", "ogar-class-view", "ogar-ontology", "ogar-vocab", @@ -5929,13 +5950,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "ogar-adapter-surrealql" -version = "0.1.0" -dependencies = [ - "ogar-vocab", -] - [[package]] name = "ogar-class-view" version = "0.1.0" @@ -10206,6 +10220,23 @@ dependencies = [ "syn", ] +[[package]] +name = "zip" +version = "2.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fabe6324e908f85a1c52063ce7aa26b68dcb7eb6dbc83a2d148403c9bc3eba50" +dependencies = [ + "arbitrary", + "crc32fast", + "crossbeam-utils", + "displaydoc", + "flate2", + "indexmap 2.14.0", + "memchr", + "thiserror 2.0.18", + "zopfli", +] + [[package]] name = "zlib-rs" version = "0.6.4" @@ -10218,6 +10249,18 @@ version = "1.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa" +[[package]] +name = "zopfli" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f05cd8797d63865425ff89b5c4a48804f35ba0ce8d125800027ad6017d2b5249" +dependencies = [ + "bumpalo", + "crc32fast", + "log", + "simd-adler32", +] + [[package]] name = "zstd" version = "0.13.3" diff --git a/crates/lance-graph-hydrate/Cargo.toml b/crates/lance-graph-hydrate/Cargo.toml index ea8789bab..e55dbc434 100644 --- a/crates/lance-graph-hydrate/Cargo.toml +++ b/crates/lance-graph-hydrate/Cargo.toml @@ -30,6 +30,15 @@ object_store = { workspace = true } futures = "0.3" tokio = { version = "1.37", features = ["fs", "rt", "macros"] } sha2 = "0.10" +# `zip` — the SHIPPING container for a dataset directory (operator ruling, +# 2026-08-22: *"bitte als zip, nicht dass wir ein Verzeichnis mit einzelnen +# Dateien shippen"*). Deliberately zip and not tar: a zip carries a central +# directory, so a reader can enumerate and seek to one entry; a tar must be +# scanned sequentially, which for a multi-hundred-MB dataset is the difference +# between reading one file and reading all of them. Sync API, driven from +# `spawn_blocking` — the work is CPU + local file I/O after the download, not +# network. +zip = { version = "2", default-features = false, features = ["deflate"] } thiserror = "1" [target.'cfg(unix)'.dependencies] diff --git a/crates/lance-graph-hydrate/src/archive.rs b/crates/lance-graph-hydrate/src/archive.rs new file mode 100644 index 000000000..15f561d86 --- /dev/null +++ b/crates/lance-graph-hydrate/src/archive.rs @@ -0,0 +1,545 @@ +//! The `absent -> hydrated` edge for a dataset shipped as ONE zip object. +//! +//! # Why a single archive, and why zip +//! +//! [`crate::copy::hydrate_dir`] hydrates a dataset that lives in the object +//! store as a TREE of objects: list, then get each one. That is the right +//! shape when the store IS the dataset's home. It is the wrong shape for +//! *distribution*, where the artifact is a versioned, checksum-pinned release: +//! a tree has no single identity to pin, no atomic publish upstream, and its +//! listing can interleave with a producer's write. +//! +//! Operator ruling, 2026-08-22: *"bitte als zip, nicht dass wir ein Verzeichnis +//! mit einzelnen Dateien shippen"* — a shipped dataset travels as one +//! container, not as loose files. +//! +//! Zip rather than tar, concretely: a zip ends with a **central directory**, so +//! a reader can enumerate entries and seek to any one of them without touching +//! the rest. A tar has no index; every lookup is a sequential scan. For a +//! multi-hundred-megabyte dataset that is the difference between reading one +//! file and reading all of them — and it is what makes a *partial* or +//! *verifying* read possible at all. +//! +//! # What this composes, and what it adds +//! +//! Nothing here re-implements a mechanism this crate already has: +//! +//! - the checksum-pinned single-object fetch is [`crate::file::hydrate_file`] +//! (stream + hash + `.part` + rename), so the archive's bytes are verified +//! before a single entry is read; +//! - the publish is [`crate::publish::publish_by_rename`] with +//! [`StagingKind::Dir`], so the directory lands with one atomic rename and a +//! concurrent reader sees either nothing or the whole dataset. +//! +//! What is new is the middle: expanding one verified container into a +//! directory tree inside private staging, under a **containment rule** — +//! every entry must live under the declared `root`, with no `..` and no +//! absolute path. A zip is an untrusted index of paths; an extractor that +//! trusts it writes wherever the archive says (Zip Slip). This one refuses the +//! whole archive on the first entry that leaves `root`, before anything is +//! published, because a dataset with one file missing is not a partial +//! success — it is an unopenable dataset that `count_rows` would report as a +//! wrong-sized table. +//! +//! # The idempotency boundary, unchanged +//! +//! Same two conditions as the rest of the crate (doctrine +//! `.claude/knowledge/s3-hydration-lifecycle.md` §4a): (a) a pinned source — +//! here the `expected_sha256_hex` argument IS condition (a), since one object +//! with one digest is exactly a pinned version; (b) an empty/uncontested +//! destination, enforced by the entry check and again at rename time. + +use crate::file::{hydrate_file, HydrateFileError}; +use crate::publish::{publish_by_rename, remove_staging, PublishError, StagingKind}; +use crate::staging::staging_suffix; +use object_store::{path::Path as ObjPath, ObjectStore}; +use std::path::{Component, Path as FsPath, PathBuf}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum HydrateArchiveError { + #[error("io error: {0}")] + Io(#[from] std::io::Error), + #[error("object store error: {0}")] + Store(#[from] object_store::Error), + #[error("destination already exists, refusing to overwrite: {0}")] + AlreadyPublished(PathBuf), + #[error("checksum mismatch: expected {expected}, got {actual}")] + ChecksumMismatch { expected: String, actual: String }, + #[error("archive error: {0}")] + Archive(String), + /// A Zip-Slip refusal: the named entry does not live under the declared + /// root. Nothing was published. + #[error("archive entry {entry} is outside {root}/ — refusing to unpack")] + EscapingEntry { entry: String, root: String }, + /// The archive unpacked without error but carried no FILES under `root` — + /// a tree of empty directories would publish an unopenable dataset. + #[error("archive carries no files under {root}/")] + NoFiles { root: String }, +} + +/// What a hydration moved. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct ArchiveReport { + /// Files extracted (directory entries are not counted). + pub files: usize, + /// Uncompressed bytes across those files. + pub bytes: u64, +} + +/// Hydrate a dataset directory from ONE checksum-pinned zip object. +/// +/// `root` is the single top-level directory the archive is expected to +/// contain (e.g. `"all_lanes.lance"`); it is what gets published AS +/// `publish_dir`, so the caller controls the local name independently of the +/// archive's. +/// +/// Returns `Err(AlreadyPublished)` **without touching the network** when +/// `publish_dir` already exists — this function performs the +/// `Absent -> Hydrated` transition only; it never merges into or overwrites an +/// existing local dataset. +pub async fn hydrate_archive( + store: &dyn ObjectStore, + remote_object: &ObjPath, + publish_dir: &FsPath, + expected_sha256_hex: &str, + root: &str, +) -> Result { + if publish_dir.exists() { + return Err(HydrateArchiveError::AlreadyPublished( + publish_dir.to_path_buf(), + )); + } + let parent = publish_dir.parent().unwrap_or_else(|| FsPath::new(".")); + tokio::fs::create_dir_all(parent).await?; + + // Staging sits beside the destination, so the publish rename never + // crosses a filesystem boundary. One suffix for both, so a crashed run + // leaves at most one identifiable pair behind. + let suffix = staging_suffix(); + let archive_path = parent.join(format!(".hydrate-archive-{suffix}.zip")); + let staging_dir = parent.join(format!(".hydrate-staging-{suffix}")); + + // 1. Verified bytes on disk. `hydrate_file` owns the pin. + if let Err(e) = hydrate_file(store, remote_object, &archive_path, expected_sha256_hex).await { + return Err(match e { + HydrateFileError::Io(e) => HydrateArchiveError::Io(e), + HydrateFileError::Store(e) => HydrateArchiveError::Store(e), + HydrateFileError::AlreadyPublished(p) => HydrateArchiveError::AlreadyPublished(p), + HydrateFileError::ChecksumMismatch { expected, actual } => { + HydrateArchiveError::ChecksumMismatch { expected, actual } + } + }); + } + + // 2. Expand into private staging. Every failure from here on removes both + // the archive and the staging tree before returning. + let unpack = { + let archive_path = archive_path.clone(); + let staging_dir = staging_dir.clone(); + let root = root.to_string(); + tokio::task::spawn_blocking(move || unpack_zip(&archive_path, &staging_dir, &root)).await + }; + let cleanup = |e: HydrateArchiveError| async { + let _ = remove_staging(&staging_dir, StagingKind::Dir).await; + let _ = remove_staging(&archive_path, StagingKind::File).await; + e + }; + let report = match unpack { + Ok(Ok(r)) => r, + Ok(Err(e)) => return Err(cleanup(e).await), + Err(join) => { + return Err(cleanup(HydrateArchiveError::Archive(format!( + "unpack task failed: {join}" + ))) + .await) + } + }; + + // 3. One rename publishes. `publish_by_rename` removes `staging` itself on + // failure; the archive is ours to clean either way. + let staged_root = staging_dir.join(root); + let published = publish_by_rename(&staged_root, publish_dir, StagingKind::Dir).await; + let _ = remove_staging(&archive_path, StagingKind::File).await; + match published { + Ok(()) => { + // The now-empty staging parent is not the published artifact. + let _ = remove_staging(&staging_dir, StagingKind::Dir).await; + Ok(report) + } + Err(PublishError::AlreadyPublished) => { + let _ = remove_staging(&staging_dir, StagingKind::Dir).await; + Err(HydrateArchiveError::AlreadyPublished( + publish_dir.to_path_buf(), + )) + } + Err(PublishError::Io(e)) => { + let _ = remove_staging(&staging_dir, StagingKind::Dir).await; + Err(HydrateArchiveError::Io(e)) + } + } +} + +/// Expand every entry into `staging`, refusing the archive on the first entry +/// that does not live under `root`. +/// +/// Deliberately NOT `ZipArchive::extract`: that method writes what the archive +/// names, and this function's contract is that it writes only what lives under +/// one declared root. The check runs on EVERY entry before any of it is +/// written, so a refusal leaves nothing half-placed. +fn unpack_zip( + archive: &FsPath, + staging: &FsPath, + root: &str, +) -> Result { + let f = std::fs::File::open(archive)?; + let mut zip = zip::ZipArchive::new(f) + .map_err(|e| HydrateArchiveError::Archive(format!("open zip: {e}")))?; + + // Validate the whole index FIRST, off the central directory — the reason + // a zip is worth shipping. A tar would have to be scanned to learn this, + // by which point entries are already streaming past. + for i in 0..zip.len() { + let entry = zip + .by_index_raw(i) + .map_err(|e| HydrateArchiveError::Archive(format!("read entry {i}: {e}")))?; + let name = entry.name().to_string(); + // `enclosed_name` is zip's own traversal guard; the root check is the + // narrower contract on top of it. Both must hold. + let ok = entry + .enclosed_name() + .map(|p| under(&p, root)) + .unwrap_or(false); + if !ok { + return Err(HydrateArchiveError::EscapingEntry { + entry: name, + root: root.to_string(), + }); + } + } + + std::fs::create_dir_all(staging)?; + let mut files = 0usize; + let mut bytes = 0u64; + for i in 0..zip.len() { + let mut entry = zip + .by_index(i) + .map_err(|e| HydrateArchiveError::Archive(format!("read entry {i}: {e}")))?; + let rel = entry + .enclosed_name() + .ok_or_else(|| HydrateArchiveError::EscapingEntry { + entry: entry.name().to_string(), + root: root.to_string(), + })?; + let dest = staging.join(&rel); + if entry.is_dir() { + std::fs::create_dir_all(&dest)?; + continue; + } + if let Some(p) = dest.parent() { + std::fs::create_dir_all(p)?; + } + let mut out = std::fs::File::create(&dest)?; + bytes += std::io::copy(&mut entry, &mut out)?; + files += 1; + } + if files == 0 { + return Err(HydrateArchiveError::NoFiles { + root: root.to_string(), + }); + } + Ok(ArchiveReport { files, bytes }) +} + +/// True iff `path` is a relative path whose first component is exactly `root` +/// and which contains no `..`. +fn under(path: &FsPath, root: &str) -> bool { + let mut comps = path.components(); + if comps.next() != Some(Component::Normal(root.as_ref())) { + return false; + } + comps.all(|c| matches!(c, Component::Normal(_))) +} + +#[cfg(test)] +mod tests { + use super::*; + use object_store::local::LocalFileSystem; + use sha2::{Digest, Sha256}; + use std::io::Write as _; + use std::sync::Arc; + + fn store_at(root: &FsPath) -> Arc { + Arc::new(LocalFileSystem::new_with_prefix(root).expect("local object store")) + } + + /// Build a zip from `(name, contents)` pairs; `None` means a directory. + fn zip_with(entries: &[(&str, Option<&[u8]>)]) -> Vec { + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts: zip::write::FileOptions<'_, ()> = + zip::write::FileOptions::default().compression_method(zip::CompressionMethod::Stored); + for (name, body) in entries { + match body { + Some(data) => { + w.start_file(*name, opts).unwrap(); + w.write_all(data).unwrap(); + } + None => w.add_directory(*name, opts).unwrap(), + } + } + w.finish().unwrap().into_inner() + } + + /// The shape a real Lance dataset has — dataset dir, transaction log, + /// versions, one data file — so the happy path is not proven on a toy. + fn realistic_zip() -> Vec { + zip_with(&[ + ("all_lanes.lance/", None), + ("all_lanes.lance/_transactions/", None), + ("all_lanes.lance/_transactions/0-abc.txn", Some(b"txn0")), + ("all_lanes.lance/_versions/", None), + ("all_lanes.lance/_versions/1.manifest", Some(b"manifest")), + ("all_lanes.lance/data/", None), + ("all_lanes.lance/data/0001.lance", Some(&[7u8; 4096])), + ]) + } + + fn sha_hex(bytes: &[u8]) -> String { + Sha256::digest(bytes) + .iter() + .map(|b| format!("{b:02x}")) + .collect() + } + + /// Put `bytes` in the remote root as `artifact.zip`; return (store, path). + fn remote_with(remote_root: &FsPath, bytes: &[u8]) -> (Arc, ObjPath) { + std::fs::write(remote_root.join("artifact.zip"), bytes).unwrap(); + (store_at(remote_root), ObjPath::from("artifact.zip")) + } + + fn residue(dir: &FsPath) -> Vec { + std::fs::read_dir(dir) + .map(|rd| { + rd.map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .filter(|n| n.starts_with(".hydrate-")) + .collect() + }) + .unwrap_or_default() + } + + // ── CAN FIRE ──────────────────────────────────────────────────────────── + #[tokio::test] + async fn hydrates_the_whole_dataset_from_one_zip() { + let remote = tempfile::tempdir().unwrap(); + let local = tempfile::tempdir().unwrap(); + let bytes = realistic_zip(); + let (store, obj) = remote_with(remote.path(), &bytes); + let dest = local.path().join("hydrated.lance"); + + let report = hydrate_archive( + store.as_ref(), + &obj, + &dest, + &sha_hex(&bytes), + "all_lanes.lance", + ) + .await + .expect("hydrate"); + + assert_eq!( + report, + ArchiveReport { + files: 3, + bytes: 4 + 8 + 4096 + }, + "files counted, directories not" + ); + assert_eq!( + std::fs::read(dest.join("data/0001.lance")).unwrap(), + vec![7u8; 4096], + "the data file arrives byte-for-byte" + ); + assert_eq!( + std::fs::read_to_string(dest.join("_transactions/0-abc.txn")).unwrap(), + "txn0", + "the transaction log arrives — a dataset without it is not openable" + ); + assert!(dest.join("_versions/1.manifest").is_file()); + // The caller's local name wins over the archive's root name. + assert!(!local.path().join("all_lanes.lance").exists()); + assert!( + residue(local.path()).is_empty(), + "no staging or archive left behind: {:?}", + residue(local.path()) + ); + } + + // ── CAN STAY SILENT — and prove it never reached the network ──────────── + #[tokio::test] + async fn an_existing_destination_is_refused_without_fetching() { + let remote = tempfile::tempdir().unwrap(); + let local = tempfile::tempdir().unwrap(); + // The remote object DOES NOT EXIST. If this function fetched before + // checking, it would fail with a store error, not AlreadyPublished. + let store = store_at(remote.path()); + let dest = local.path().join("hydrated.lance"); + std::fs::create_dir_all(dest.join("data")).unwrap(); + std::fs::write(dest.join("data/live.lance"), b"grown-since-hydrate").unwrap(); + + let err = hydrate_archive( + store.as_ref(), + &ObjPath::from("artifact.zip"), + &dest, + "00", + "all_lanes.lance", + ) + .await + .expect_err("must refuse"); + assert!( + matches!(err, HydrateArchiveError::AlreadyPublished(_)), + "got {err:?}" + ); + assert_eq!( + std::fs::read_to_string(dest.join("data/live.lance")).unwrap(), + "grown-since-hydrate", + "a live dataset is never overwritten" + ); + } + + // ── REFUSALS ──────────────────────────────────────────────────────────── + #[tokio::test] + async fn an_entry_outside_the_root_is_refused_and_publishes_nothing() { + for escape in [ + "../escape.txt", + "/etc/passwd", + "other_table.lance/data/0001.lance", + "all_lanes.lance/../../escape.txt", + ] { + let remote = tempfile::tempdir().unwrap(); + let local = tempfile::tempdir().unwrap(); + let bytes = zip_with(&[ + ("all_lanes.lance/", None), + ("all_lanes.lance/data/0001.lance", Some(b"real")), + (escape, Some(b"pwned")), + ]); + let (store, obj) = remote_with(remote.path(), &bytes); + let dest = local.path().join("hydrated.lance"); + + let err = hydrate_archive( + store.as_ref(), + &obj, + &dest, + &sha_hex(&bytes), + "all_lanes.lance", + ) + .await + .expect_err("must refuse"); + assert!( + matches!(err, HydrateArchiveError::EscapingEntry { .. }), + "{escape}: got {err:?}" + ); + assert!(!dest.exists(), "{escape}: nothing published"); + assert!( + !local.path().join("escape.txt").exists() + && !local.path().parent().unwrap().join("escape.txt").exists(), + "{escape}: the escaping entry never lands" + ); + assert!( + residue(local.path()).is_empty(), + "{escape}: residue {:?}", + residue(local.path()) + ); + } + } + + #[tokio::test] + async fn a_wrong_checksum_publishes_nothing() { + let remote = tempfile::tempdir().unwrap(); + let local = tempfile::tempdir().unwrap(); + let bytes = realistic_zip(); + let (store, obj) = remote_with(remote.path(), &bytes); + let dest = local.path().join("hydrated.lance"); + + // A VALID archive with the WRONG pin — so what refuses is the pin, + // not the content. + let err = hydrate_archive( + store.as_ref(), + &obj, + &dest, + &sha_hex(b"other"), + "all_lanes.lance", + ) + .await + .expect_err("must refuse"); + assert!( + matches!(err, HydrateArchiveError::ChecksumMismatch { .. }), + "got {err:?}" + ); + assert!(!dest.exists()); + assert!( + residue(local.path()).is_empty(), + "{:?}", + residue(local.path()) + ); + + // …and with the RIGHT pin the same archive DOES hydrate — so the + // refusal above is the pin check, not an inert path. + hydrate_archive( + store.as_ref(), + &obj, + &dest, + &sha_hex(&bytes), + "all_lanes.lance", + ) + .await + .expect("hydrate with the correct pin"); + assert!(dest.join("data/0001.lance").is_file()); + } + + #[tokio::test] + async fn an_archive_of_only_directories_is_refused() { + // Anti-vacuity for the file counter: an all-directory tree extracts + // without error and would publish an unopenable dataset. + let remote = tempfile::tempdir().unwrap(); + let local = tempfile::tempdir().unwrap(); + let bytes = zip_with(&[("all_lanes.lance/", None), ("all_lanes.lance/data/", None)]); + let (store, obj) = remote_with(remote.path(), &bytes); + let dest = local.path().join("hydrated.lance"); + + let err = hydrate_archive( + store.as_ref(), + &obj, + &dest, + &sha_hex(&bytes), + "all_lanes.lance", + ) + .await + .expect_err("must refuse"); + assert!( + matches!(err, HydrateArchiveError::NoFiles { .. }), + "got {err:?}" + ); + assert!(!dest.exists()); + assert!( + residue(local.path()).is_empty(), + "{:?}", + residue(local.path()) + ); + } + + #[test] + fn under_accepts_the_root_and_rejects_traversal() { + assert!(under( + FsPath::new("all_lanes.lance/data/x"), + "all_lanes.lance" + )); + assert!(under(FsPath::new("all_lanes.lance"), "all_lanes.lance")); + assert!(!under( + FsPath::new("all_lanes.lance/../x"), + "all_lanes.lance" + )); + assert!(!under(FsPath::new("/all_lanes.lance/x"), "all_lanes.lance")); + assert!(!under(FsPath::new("crystal.lance/x"), "all_lanes.lance")); + assert!(!under(FsPath::new(""), "all_lanes.lance")); + } +} diff --git a/crates/lance-graph-hydrate/src/lib.rs b/crates/lance-graph-hydrate/src/lib.rs index dd272bcd5..b57769b8a 100644 --- a/crates/lance-graph-hydrate/src/lib.rs +++ b/crates/lance-graph-hydrate/src/lib.rs @@ -75,6 +75,7 @@ //! open, so this follow-up closes it. See the `publish` module's doc for //! what merged and what stayed per-caller. +pub mod archive; pub mod copy; pub mod dirty; pub mod env; @@ -85,6 +86,7 @@ mod publish; pub mod release; mod staging; +pub use archive::{hydrate_archive, ArchiveReport, HydrateArchiveError}; pub use copy::{hydrate_dir, HydrateError, HydrationReport}; pub use dirty::{is_dirty, lifecycle_of, DirtyCheckError}; pub use env::{env as env_var, HydrationSource}; diff --git a/crates/lance-graph/Cargo.toml b/crates/lance-graph/Cargo.toml index 23deb99a9..fd995017f 100644 --- a/crates/lance-graph/Cargo.toml +++ b/crates/lance-graph/Cargo.toml @@ -11,6 +11,19 @@ keywords = ["lance", "graph", "cypher", "query", "datafusion"] categories = ["database", "data-structures", "science"] [dependencies] +# The hydration lifecycle, minted in this workspace, and `object_store` — the +# client type `hydrate_from` takes. `object_store` was previously a DEV +# dependency here: the lib named no object store, only examples and tests did. +# `hydrate_from` is the first library surface that does. +# +# This dep closes two issues at once. It is the shape +# ISS-REMOTE-URI-CONSTRUCTORS-PREDATE-THE-HYDRATION-DOCTRINE names, and it puts +# `lance-graph-hydrate` into a build graph CI actually walks — the thing it +# lacked when it was merged un-compiling (EPIPHANIES +# E-A-CRATE-WITH-ZERO-CONSUMERS-IS-BUILT-BY-NOTHING-AND-CAN-BE-MERGED-BROKEN-1, +# ISSUES ISS-HYDRATE-CRATE-HAS-NO-BUILD-GATE). +lance-graph-hydrate = { path = "../lance-graph-hydrate" } +object_store = { version = "0.13", features = ["aws"] } # arrow + datafusion bumped to 58 + 53 to align with lance 6.0's transitive versions. # TODO(lance-bump): downstream crates may need follow-up to align. arrow = { workspace = true, features = ["prettyprint"] } diff --git a/crates/lance-graph/src/error.rs b/crates/lance-graph/src/error.rs index 84cad27cb..5997d8cf2 100644 --- a/crates/lance-graph/src/error.rs +++ b/crates/lance-graph/src/error.rs @@ -59,6 +59,19 @@ pub enum GraphError { source: arrow::error::ArrowError, location: Location, }, + + /// Hydration error — object store to local volume. + /// + /// `VersionedGraph::hydrate_from` is the only producer. Kept as its own + /// variant rather than folded into `ExecutionError { message }`: a caller + /// deciding whether to retry needs to tell a checksum mismatch (never + /// retry — the pin or the artifact is wrong) from a transport error + /// (retry), and a flattened string cannot be matched on. + #[snafu(display("Hydration error: {source}"))] + Hydration { + source: lance_graph_hydrate::HydrateArchiveError, + location: Location, + }, } impl From for GraphError { @@ -87,3 +100,12 @@ impl From for GraphError { } } } + +impl From for GraphError { + fn from(source: lance_graph_hydrate::HydrateArchiveError) -> Self { + Self::Hydration { + source, + location: Location::new(file!(), line!(), column!()), + } + } +} diff --git a/crates/lance-graph/src/graph/versioned.rs b/crates/lance-graph/src/graph/versioned.rs index df9150c6b..817d32fea 100644 --- a/crates/lance-graph/src/graph/versioned.rs +++ b/crates/lance-graph/src/graph/versioned.rs @@ -85,6 +85,19 @@ pub struct GraphDiff { pub seal_status: GraphSealStatus, } +/// Which path [`VersionedGraph::hydrate_from`] took. +/// +/// Returned rather than folded away: a boot that fetched the artifact and one +/// that found it already on the volume are different events, and the whole +/// point of hydrating once is being able to see which happened. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Hydration { + /// The archive was fetched, verified, expanded and published by this call. + Fresh(lance_graph_hydrate::ArchiveReport), + /// The store was already on disk; nothing was fetched. + AlreadyLocal, +} + // --------------------------------------------------------------------------- // VersionedGraph // --------------------------------------------------------------------------- @@ -137,6 +150,64 @@ impl VersionedGraph { } } + /// Hydrate this graph's whole store from ONE checksum-pinned zip object, + /// then open it locally. + /// + /// This is the shape `ISS-REMOTE-URI-CONSTRUCTORS-PREDATE-THE-HYDRATION- + /// DOCTRINE` names. The three constructors above take a URI and address the + /// store WHERE IT SITS; for `s3`/`azure`/`gcs` that makes the object store + /// the store itself, which the hydration doctrine + /// (`.claude/knowledge/s3-hydration-lifecycle.md`) explicitly does not: + /// there the object store is the hydration SOURCE and a local mmap-capable + /// directory is THE STORE. They are kept — addressing a remote store + /// directly is a legitimate choice when a caller means it, and at >1 + /// replica it is the better one — but until now there was no way to express + /// the doctrine's own shape. This is that way. + /// + /// # Ensure-hydrated, not hydrate-or-fail + /// + /// A destination that already exists is the WARM PATH, not an error: a + /// caller asking for a hydrated graph has one. The distinction is returned + /// rather than discarded, because "downloaded 380 MB" and "found it + /// already there" are different facts and a boot log that prints the same + /// line for both hides the one that matters. + /// + /// # Arguments + /// + /// - `local_base` — where the store lives afterwards; this becomes + /// [`Self::base_path`]. Never a URI. + /// - `root` — the single top-level directory inside the archive. It is the + /// PRODUCER's name for the store; `local_base` is the caller's. Keeping + /// both explicit is what lets a deploy name its volume independently of + /// whoever baked the artifact. + /// - `expected_sha256_hex` — the pin. This IS the doctrine's + /// "pinned source version" condition; without it the idempotency + /// boundary has only half its premises. + pub async fn hydrate_from( + store: &dyn object_store::ObjectStore, + remote_archive: &object_store::path::Path, + local_base: &str, + expected_sha256_hex: &str, + root: &str, + ) -> crate::error::Result<(Self, Hydration)> { + let dest = std::path::Path::new(local_base); + match lance_graph_hydrate::hydrate_archive( + store, + remote_archive, + dest, + expected_sha256_hex, + root, + ) + .await + { + Ok(report) => Ok((Self::local(local_base), Hydration::Fresh(report))), + Err(lance_graph_hydrate::HydrateArchiveError::AlreadyPublished(_)) => { + Ok((Self::local(local_base), Hydration::AlreadyLocal)) + } + Err(e) => Err(e.into()), + } + } + // -- dataset paths ------------------------------------------------------ /// The base directory or URI this graph is rooted at (local path, `s3://`, @@ -669,6 +740,72 @@ impl VersionedGraph { #[cfg(test)] mod tests { + //! Note on scope: `hydrate_from`'s ARCHIVE mechanics — fetch, checksum, + //! Zip-Slip containment, atomic publish — are falsified in + //! `lance_graph_hydrate::archive` (6 tests, both guards mutation-checked). + //! What is tested HERE is only what this function adds on top: the mapping + //! from the hydration result onto an opened graph, and specifically that a + //! destination which already exists is a WARM SUCCESS rather than the + //! error the underlying call returns. + + /// CAN STAY SILENT: an already-hydrated store is not re-fetched. + /// + /// The remote object deliberately does NOT exist. If `hydrate_from` + /// reached the store at all, this would fail with a transport error rather + /// than returning `AlreadyLocal` — so the assertion is evidence about + /// control flow, not just about the returned value. + #[tokio::test] + async fn an_already_hydrated_store_is_a_warm_success_and_is_not_refetched() { + let tmp = tempfile::tempdir().expect("tempdir"); + let base = tmp.path().join("graph-store"); + std::fs::create_dir_all(base.join("nodes.lance")).expect("pre-existing store"); + let store = object_store::local::LocalFileSystem::new_with_prefix(tmp.path()) + .expect("local object store"); + + let (graph, how) = VersionedGraph::hydrate_from( + &store, + &object_store::path::Path::from("absent.zip"), + base.to_str().unwrap(), + "00", + "graph-store", + ) + .await + .expect("a hydrated store is not an error"); + + assert_eq!(how, Hydration::AlreadyLocal); + assert_eq!(graph.base_path(), base.to_str().unwrap()); + assert!(base.join("nodes.lance").is_dir(), "left untouched"); + } + + /// CAN FIRE: a genuine failure is NOT laundered into a warm success. + /// + /// Same absent object, but with no local store — so the only honest answer + /// is an error. Without this, the test above would also pass on an + /// implementation that swallowed every error as `AlreadyLocal`. + #[tokio::test] + async fn a_missing_archive_with_no_local_store_is_an_error() { + let tmp = tempfile::tempdir().expect("tempdir"); + let base = tmp.path().join("graph-store"); + let store = object_store::local::LocalFileSystem::new_with_prefix(tmp.path()) + .expect("local object store"); + + let err = VersionedGraph::hydrate_from( + &store, + &object_store::path::Path::from("absent.zip"), + base.to_str().unwrap(), + "00", + "graph-store", + ) + .await + .expect_err("an absent archive with nothing local must fail"); + assert!( + matches!(err, crate::error::GraphError::Hydration { .. }), + "the hydration error keeps its own variant rather than being \ + flattened into a message: {err:?}" + ); + assert!(!base.exists(), "nothing was published"); + } + use super::*; use arrow_array::builder::FixedSizeBinaryBuilder; use arrow_schema::DataType;