fix(storage): pending-upload marker + review round fixes

Address beardthelion + CodeRabbit round on d30af0d:

- [P1] Persisted pending-upload marker: written before every post-write
  upload, cleared only on success. sync_down_if_stale now distinguishes
  "local is ahead of storage" (failed/interrupted upload — serve local,
  never download) from "storage is ahead" (download), so a failed
  write-back upload can no longer roll a stale archive over an acked
  push. Stale markers without a local copy are dropped; the cache-miss
  download path clears leftover markers. Two regression tests.
- [P2] Detach and close the lock connection when pg_advisory_unlock
  fails, mirroring Drop — never repool a possibly-locked session.
- [P3] init() removes the local repo dir when its upload fails; fork
  deletes the already-uploaded archive when create_repo fails (new
  RepoStore::delete_archive).
- Error precedence in git_receive_pack: a failed push now reports the
  git failure even when the lock release also errors.
- Advisory-lock pool: acquire_timeout raised to match the 300s lock
  wait; pool size now env-tunable (GITLAWB_ADVISORY_LOCK_POOL_SIZE).
- Dropped list() from the BlobStore trait until a production consumer
  exists (kept as a test-only helper on the fs backend).
- Documented all storage knobs in .env.example and the README env table;
  fixed the two remaining "Tigris" strings; corrected the async_upload
  docs to describe the marker semantics.

.env.example

@@ -17,6 +17,30 @@ GITLAWB_PORT=7545
 # ── Storage ───────────────────────────────────────────────────────────────
 GITLAWB_REPOS_DIR=/data/repos
 
+# ── Object storage (durable repo archives) ────────────────────────────────
+# Backend for whole-repo archives: s3 | fs | ipfs. Empty = auto-detect:
+# s3 when a bucket is set, else fs when GITLAWB_STORAGE_FS_DIR is set, else
+# local-only (repos live only on this node's disk). `ipfs` is never
+# auto-selected — setting GITLAWB_IPFS_API alone keeps its pinning-only
+# meaning; opt in explicitly with GITLAWB_STORAGE_BACKEND=ipfs.
+GITLAWB_STORAGE_BACKEND=
+# Bucket for the s3 backend (Tigris, R2, AWS S3, MinIO, B2).
+# GITLAWB_TIGRIS_BUCKET is honored as a legacy alias.
+GITLAWB_S3_BUCKET=
+# Endpoint URL override for the s3 backend (R2/MinIO). On Tigris/Fly the
+# endpoint arrives via AWS_ENDPOINT_URL_S3 — leave empty.
+GITLAWB_S3_ENDPOINT=
+# Force path-style addressing (required by MinIO and some S3-compatibles).
+GITLAWB_S3_FORCE_PATH_STYLE=false
+# Directory for the fs (local filesystem) backend.
+GITLAWB_STORAGE_FS_DIR=
+# Ack pushes before the durable upload finishes (write-back). Lower latency;
+# opt-in durability tradeoff — see --help for the full semantics.
+GITLAWB_ASYNC_UPLOAD=false
+# Dedicated DB pool for per-repo write locks; a push pins one connection for
+# its lifetime, so this bounds per-node push concurrency.
+GITLAWB_ADVISORY_LOCK_POOL_SIZE=16
+
 # PostgreSQL connection URL. Required.
 # When using the bundled docker-compose, this is wired automatically.
 DATABASE_URL=postgresql://gitlawb:changeme@localhost:5432/gitlawb

README.md

@@ -344,7 +344,14 @@ Important node settings:
 | `GITLAWB_AUTO_SYNC` | Enable automatic sync from known peers. |
 | `GITLAWB_MAX_PACK_BYTES` | Max git pack body size for smart-HTTP routes. |
 | `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` | Max seconds a served git upload-pack/receive-pack may run before it is aborted (504). Default 600. Does not bound `info/refs` or the withheld-blob path. |
-| `GITLAWB_TIGRIS_BUCKET` | Optional S3/Tigris shared repo storage bucket. |
+| `GITLAWB_STORAGE_BACKEND` | Object-storage backend for repo archives: `s3`, `fs`, or `ipfs`. Empty = auto-detect (`s3` if a bucket is set, else `fs` if a dir is set, else local-only). `ipfs` is never auto-selected. |
+| `GITLAWB_S3_BUCKET` | Bucket for the `s3` backend (Tigris, R2, AWS S3, MinIO, B2). |
+| `GITLAWB_S3_ENDPOINT` | Endpoint URL override for the `s3` backend (R2/MinIO; empty on Tigris/Fly). |
+| `GITLAWB_S3_FORCE_PATH_STYLE` | Force path-style S3 addressing (MinIO and some S3-compatibles). |
+| `GITLAWB_STORAGE_FS_DIR` | Directory for the `fs` (local filesystem) backend. |
+| `GITLAWB_ASYNC_UPLOAD` | Ack pushes before the durable storage upload (write-back). Lower latency, opt-in durability tradeoff. Default `false`. |
+| `GITLAWB_ADVISORY_LOCK_POOL_SIZE` | Dedicated DB pool for per-repo write locks; bounds per-node push concurrency. Default 16. |
+| `GITLAWB_TIGRIS_BUCKET` | Legacy alias for `GITLAWB_S3_BUCKET` (selects the `s3` backend). |
 | `GITLAWB_PINATA_JWT` | Optional Pinata/IPFS warm-storage pinning. |
 | `GITLAWB_IRYS_URL` | Optional Irys/Arweave permanent anchoring. |
 

crates/gitlawb-node/src/api/repos.rs

@@ -953,10 +953,12 @@ pub async fn git_receive_pack(
         // can't observe a stale archive. If this detached task is cancelled by
         // runtime shutdown mid-upload, the guard's lock connection is closed
         // rather than repooled, so Postgres frees the advisory lock (see
-        // `LockedConn`). Durability tradeoff: if the node stops after this ack
-        // but before the upload, the local copy survives but STORAGE stays
-        // stale until this repo's next successful push re-uploads — it is not
-        // automatically reconciled. Hence async_upload is opt-in.
+        // `LockedConn`). Durability tradeoff: if the upload fails (or the node
+        // stops first), storage stays stale until this repo's next successful
+        // upload. The persisted pending-upload marker keeps the local copy
+        // authoritative on THIS node in that window — no access rolls it back
+        // to the stale archive — but other nodes still serve the stale archive
+        // until the re-upload lands. Hence async_upload is opt-in.
         let repo_label = name.to_string();
         tokio::spawn(async move {
             if let Err(e) = guard.release(true).await {
@@ -965,15 +967,19 @@ pub async fn git_receive_pack(
             }
         });
     } else {
-        // Strict path (or failed push): upload-before-ack. On a successful push
-        // whose durable upload then fails, surface an error so the client knows
-        // the push is not durably stored (a failed push returns Ok from release
-        // and the push error is reported below).
+        // Strict path (or failed push): upload-before-ack.
         if let Err(e) = guard.release(push_ok).await {
-            tracing::error!(repo = %name, err = %e, "durable upload failed after push");
-            return Err(AppError::Git(format!(
-                "push applied locally but durable upload to storage failed: {e}"
-            )));
+            if push_ok {
+                // A successful push whose durable upload then failed — the
+                // client must know the push is not durably stored.
+                tracing::error!(repo = %name, err = %e, "durable upload failed after push");
+                return Err(AppError::Git(format!(
+                    "push applied locally but durable upload to storage failed: {e}"
+                )));
+            }
+            // The push itself failed; log the release error but fall through
+            // so the real git failure (below) is what the client sees.
+            tracing::error!(repo = %name, err = %e, "lock release failed after failed push");
         }
     }
 
@@ -1660,6 +1666,16 @@ pub async fn fork_repo(
 
     if let Err(e) = state.db.create_repo(&record).await {
         cleanup_local_fork(disk_path).await;
+        // The archive was already uploaded; without a DB row it would be
+        // orphaned in storage, so remove it too.
+        if let Err(cleanup_err) = state
+            .repo_store
+            .delete_archive(&forker_did, &fork_name)
+            .await
+        {
+            tracing::warn!(fork = %fork_name, err = %cleanup_err,
+                "failed to remove orphaned storage archive after fork error");
+        }
         return Err(e.into());
     }
 
@@ -2606,7 +2622,7 @@ mod tests {
 
     /// The receive-pack *advertisement* (`GET info/refs?service=git-receive-pack`)
     /// must be throttled by the per-IP push limiter BEFORE it does the fresh
-    /// Tigris acquire — otherwise the flood brake on the POST is bypassable via
+    /// storage acquire — otherwise the flood brake on the POST is bypassable via
     /// the cheaper unauthenticated GET (PR #152 review P1). Pre-filling the
     /// bucket makes the assertion deterministic and keeps the test off the
     /// acquire path entirely.
@@ -2645,7 +2661,7 @@ mod tests {
         assert_eq!(
             status,
             StatusCode::TOO_MANY_REQUESTS,
-            "receive-pack advertisement must be throttled before the Tigris acquire"
+            "receive-pack advertisement must be throttled before the storage acquire"
         );
     }
 

crates/gitlawb-node/src/config.rs

@@ -163,13 +163,21 @@ pub struct Config {
 
     /// Acknowledge a push to the client before the durable upload to object
     /// storage finishes (write-back). Lowers push latency, but opens a
-    /// durability window: if the node stops between the ack and the upload, on
-    /// restart a stale remote archive can overwrite the newer local copy. Off
-    /// by default (strict upload-before-ack); opt in only where the latency win
-    /// is worth that risk.
+    /// durability window: if the upload fails or the node stops first, storage
+    /// stays stale until the next successful upload. A persisted pending-upload
+    /// marker keeps the local copy authoritative on this node in that window
+    /// (no rollback of the acked push), but other nodes serve the stale archive
+    /// until the re-upload lands. Off by default (strict upload-before-ack).
     #[arg(long, env = "GITLAWB_ASYNC_UPLOAD", default_value_t = false)]
     pub async_upload: bool,
 
+    /// Max connections in the dedicated advisory-lock pool. A push pins one
+    /// connection for its whole lifetime (lock held across receive-pack and
+    /// the storage upload), so this bounds per-node push concurrency. Counts
+    /// against Postgres `max_connections` on top of the handler pool.
+    #[arg(long, env = "GITLAWB_ADVISORY_LOCK_POOL_SIZE", default_value_t = 16)]
+    pub advisory_lock_pool_size: u32,
+
     /// Maximum pack body size for git-receive-pack and git-upload-pack, in bytes.
     /// Applies only to git smart-HTTP routes — all other API routes keep the 2 MB default.
     /// Default: 2 GB.  Set lower on resource-constrained nodes.

crates/gitlawb-node/src/git/repo_store.rs

@@ -92,6 +92,28 @@ impl RepoStore {
         let Some(ref archive) = self.archive else {
             return Ok(());
         };
+
+        let marker = pending_upload_marker(local_path);
+        if marker.exists() {
+            if local_path.exists() {
+                // The local copy has a write that storage never received (its
+                // upload failed, or the node stopped first). Storage is BEHIND
+                // local, so the etag-mismatch-means-remote-newer rule below is
+                // inverted here — downloading would roll the acked write back.
+                // Serve the local copy; the next successful post-write upload
+                // clears the marker and re-syncs storage. Cross-node caveat:
+                // until that upload lands, other nodes still see the stale
+                // archive — the marker protects this node's copy, it does not
+                // replicate it.
+                warn!(repo = %repo_name,
+                    "local copy ahead of storage (pending upload) — skipping download");
+                return Ok(());
+            }
+            // Marker without a local copy: the repo dir was removed out from
+            // under us, so the storage copy is the best remaining state. Drop
+            // the stale marker and fall through to the normal download.
+            let _ = std::fs::remove_file(&marker);
+        }
         let key = format!("{owner_slug}/{repo_name}");
 
         let remote_etag = match archive.head_etag(owner_slug, repo_name).await {
@@ -188,6 +210,10 @@ impl RepoStore {
                     .download(&owner_slug, repo_name, &local_path)
                     .await
                     .context("downloading repo from storage")?;
+                // The local copy didn't exist, so any pending-upload marker
+                // here is stale litter — clear it or it would wrongly pin the
+                // just-downloaded copy as "ahead of storage".
+                clear_pending_upload(&local_path);
                 let key = format!("{owner_slug}/{repo_name}");
                 self.migrated.lock().await.insert(key.clone());
                 self.versions.lock().await.insert(key, remote_etag);
@@ -251,10 +277,19 @@ impl RepoStore {
         // Upload the new repo synchronously under the advisory lock: a background
         // upload could land the empty repo *after* a racing first push and clobber
         // it, and a silent failure would leave the repo absent from storage. Fail
-        // closed instead — surface upload errors to the caller.
-        self.upload_under_lock(&owner_slug, repo_name, &local_path, false)
+        // closed instead — surface upload errors to the caller, and remove the
+        // just-created local dir so a retry of the same name doesn't hit an
+        // existing destination.
+        if let Err(e) = self
+            .upload_under_lock(&owner_slug, repo_name, &local_path, false)
             .await
-            .context("uploading new repo to storage")?;
+        {
+            if let Err(cleanup_err) = std::fs::remove_dir_all(&local_path) {
+                warn!(repo = %repo_name, err = %cleanup_err,
+                    "failed to remove local repo dir after init upload failure");
+            }
+            return Err(e).context("uploading new repo to storage");
+        }
 
         Ok(local_path)
     }
@@ -271,21 +306,45 @@ impl RepoStore {
             .local_path(owner_did, repo_name)
             .context("rejected unsafe path in release_after_write")?;
         let key = format!("{owner_slug}/{repo_name}");
+        mark_pending_upload(&local_path, repo_name);
         match archive.upload(&owner_slug, repo_name, &local_path).await {
             Ok(Some(etag)) => {
                 self.versions.lock().await.insert(key, etag);
+                clear_pending_upload(&local_path);
+                Ok(())
+            }
+            Ok(None) => {
+                clear_pending_upload(&local_path);
                 Ok(())
             }
-            Ok(None) => Ok(()),
             Err(e) => {
-                // Invalidate so the next access re-downloads rather than trusting
-                // a local copy we failed to persist.
+                // Storage is now behind local. Drop the cached etag, and leave
+                // the pending-upload marker in place so sync_down_if_stale
+                // serves the local copy instead of rolling it back to the
+                // stale archive.
                 self.versions.lock().await.remove(&key);
                 Err(e).context("uploading repo to storage after write")
             }
         }
     }
 
+    /// Remove a repo's archive object from storage. Cleanup for creation flows
+    /// that uploaded successfully but then failed before the DB row existed —
+    /// without this, the archive is orphaned with no owning record.
+    pub async fn delete_archive(&self, owner_did: &str, repo_name: &str) -> Result<()> {
+        let Some(ref archive) = self.archive else {
+            return Ok(());
+        };
+        let (owner_slug, _) = self
+            .local_path(owner_did, repo_name)
+            .context("rejected unsafe path in delete_archive")?;
+        self.versions
+            .lock()
+            .await
+            .remove(&format!("{owner_slug}/{repo_name}"));
+        archive.delete(&owner_slug, repo_name).await
+    }
+
     /// Upload `local_path` to storage while holding the per-repo advisory lock,
     /// so a background or init-time upload can't clobber a concurrent locked
     /// write by landing an older snapshot after it. With `skip_if_exists`, skips
@@ -433,7 +492,7 @@ fn validate_repo_name(repo_name: &str) -> Result<()> {
 /// `ipfs.rs`): the writer holding the lock may legitimately be mid-upload of a
 /// large archive, so a concurrent push must be willing to outwait the longest
 /// possible upload rather than failing while the lock holder is still healthy.
-const LOCK_ACQUIRE_TIMEOUT_SECS: u64 = 300;
+pub(crate) const LOCK_ACQUIRE_TIMEOUT_SECS: u64 = 300;
 
 /// A pool connection pinned for the lifetime of a session-scoped advisory lock.
 ///
@@ -521,7 +580,13 @@ impl LockedConn {
                 .execute(&mut *conn)
                 .await
             {
-                warn!(repo = %self.repo_label, err = %e, "failed to release advisory lock");
+                // The unlock failed, so the session may still hold the lock —
+                // close the connection (like `Drop`) instead of repooling it,
+                // or the lock would wedge this repo's writes until the pool
+                // happened to recycle that connection.
+                warn!(repo = %self.repo_label, err = %e,
+                    "failed to release advisory lock — closing its connection");
+                drop(conn.detach());
             }
         }
     }
@@ -585,19 +650,27 @@ impl RepoWriteGuard {
         // error; a write-back caller logs it).
         let upload_result: Result<()> = if success {
             if let Some(ref archive) = self.archive {
+                mark_pending_upload(&self.local_path, &self.repo_name);
                 match archive
                     .upload(&self.owner_slug, &self.repo_name, &self.local_path)
                     .await
                 {
                     Ok(Some(etag)) => {
                         self.versions.lock().await.insert(key.clone(), etag);
+                        clear_pending_upload(&self.local_path);
+                        Ok(())
+                    }
+                    Ok(None) => {
+                        clear_pending_upload(&self.local_path);
                         Ok(())
                     }
-                    Ok(None) => Ok(()),
                     Err(e) => {
-                        // The durable copy is now stale. Drop the cached etag so
-                        // the next write re-downloads and reconciles rather than
-                        // trusting a local copy we failed to persist.
+                        // Storage is now behind local (this holds even for an
+                        // already-acked write-back push). Drop the cached etag,
+                        // and leave the pending-upload marker so the next
+                        // access serves the local copy instead of rolling it
+                        // back to the stale archive; the next successful
+                        // upload re-syncs storage and clears the marker.
                         self.versions.lock().await.remove(&key);
                         Err(e).context("uploading repo to storage after write")
                     }
@@ -623,6 +696,34 @@ impl RepoWriteGuard {
     }
 }
 
+/// Sibling marker file recording that `local_path` holds writes storage has
+/// not received yet ("local is ahead"). Written before every post-write upload
+/// and removed only when the upload succeeds, so it survives process death and
+/// lets `sync_down_if_stale` distinguish "storage is ahead of local" (download)
+/// from "local is ahead of storage" (never download — that would roll back an
+/// acked write). Lives next to the repo dir, not inside it, so it is never
+/// packed into the archive.
+fn pending_upload_marker(local_path: &Path) -> PathBuf {
+    let name = local_path
+        .file_name()
+        .map(|n| n.to_string_lossy().into_owned())
+        .unwrap_or_default();
+    local_path.with_file_name(format!(".{name}.pending-upload"))
+}
+
+fn mark_pending_upload(local_path: &Path, repo_name: &str) {
+    if let Err(e) = std::fs::write(pending_upload_marker(local_path), b"") {
+        // Best-effort: without the marker we fall back to the pre-marker
+        // behavior (a failed upload risks rollback), but the upload itself
+        // must still proceed.
+        warn!(repo = %repo_name, err = %e, "failed to write pending-upload marker");
+    }
+}
+
+fn clear_pending_upload(local_path: &Path) {
+    let _ = std::fs::remove_file(pending_upload_marker(local_path));
+}
+
 /// Compute a stable i64 hash for a Postgres advisory lock key.
 fn advisory_lock_key(owner_slug: &str, repo_name: &str) -> i64 {
     use std::hash::{Hash, Hasher};
@@ -846,6 +947,98 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn pending_marker_prevents_rollback_and_clears_on_next_upload() {
+        let store_root = tempfile::tempdir().unwrap();
+        let repos_dir = tempfile::tempdir().unwrap();
+        let store = store_with_fs_archive(repos_dir.path().to_path_buf(), store_root.path());
+
+        // Storage holds v1; local downloads it.
+        let seed = tempfile::tempdir().unwrap();
+        std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap();
+        store
+            .archive
+            .as_ref()
+            .unwrap()
+            .upload("owner", "repo", seed.path())
+            .await
+            .unwrap();
+        let local = repos_dir.path().join("owner").join("repo.git");
+        store
+            .sync_down_if_stale("owner", "repo", &local, true)
+            .await
+            .unwrap();
+
+        // Simulate an acked write whose upload failed: local advances, the
+        // pending marker persists, and storage still holds the older v1 with a
+        // *different* etag than the local cache (cache invalidated on failure).
+        std::fs::write(local.join("HEAD"), b"ACKED-WRITE\n").unwrap();
+        mark_pending_upload(&local, "repo");
+        store.versions.lock().await.clear();
+
+        // Both the read and the write path must serve local, not roll it back.
+        for require_fresh in [false, true] {
+            store
+                .sync_down_if_stale("owner", "repo", &local, require_fresh)
+                .await
+                .unwrap();
+            assert_eq!(
+                std::fs::read(local.join("HEAD")).unwrap(),
+                b"ACKED-WRITE\n",
+                "pending marker must prevent rollback (require_fresh={require_fresh})"
+            );
+        }
+
+        // The next successful upload re-syncs storage and clears the marker.
+        store.release_after_write("owner", "repo").await.unwrap();
+        assert!(
+            !pending_upload_marker(&local).exists(),
+            "marker must be cleared by a successful upload"
+        );
+        let out = tempfile::tempdir().unwrap();
+        let restored = out.path().join("restored.git");
+        store
+            .archive
+            .as_ref()
+            .unwrap()
+            .download("owner", "repo", &restored)
+            .await
+            .unwrap();
+        assert_eq!(
+            std::fs::read(restored.join("HEAD")).unwrap(),
+            b"ACKED-WRITE\n"
+        );
+    }
+
+    #[tokio::test]
+    async fn stale_pending_marker_without_local_copy_is_dropped() {
+        let store_root = tempfile::tempdir().unwrap();
+        let repos_dir = tempfile::tempdir().unwrap();
+        let store = store_with_fs_archive(repos_dir.path().to_path_buf(), store_root.path());
+
+        let seed = tempfile::tempdir().unwrap();
+        std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap();
+        store
+            .archive
+            .as_ref()
+            .unwrap()
+            .upload("owner", "repo", seed.path())
+            .await
+            .unwrap();
+
+        // Marker exists but the repo dir does not (removed out from under us):
+        // the marker is stale — drop it and download normally.
+        let local = repos_dir.path().join("owner").join("repo.git");
+        std::fs::create_dir_all(local.parent().unwrap()).unwrap();
+        mark_pending_upload(&local, "repo");
+        store
+            .sync_down_if_stale("owner", "repo", &local, true)
+            .await
+            .unwrap();
+        assert_eq!(std::fs::read(local.join("HEAD")).unwrap(), b"v1\n");
+        assert!(!pending_upload_marker(&local).exists());
+    }
+
     #[tokio::test]
     async fn sync_down_if_stale_require_fresh_fails_closed_on_bad_remote() {
         let store_root = tempfile::tempdir().unwrap();

crates/gitlawb-node/src/main.rs

@@ -276,10 +276,16 @@ async fn main() -> Result<()> {
     // connection for its whole lifetime (lock held across receive-pack +
     // upload), so giving locks their own budget keeps a burst of concurrent or
     // slow pushes from draining the main pool and stalling every other DB
-    // handler node-wide. Sized independently of the handler pool.
-    const ADVISORY_LOCK_POOL_SIZE: u32 = 16;
+    // handler node-wide. Sized independently of the handler pool
+    // (GITLAWB_ADVISORY_LOCK_POOL_SIZE). acquire_timeout matches the advisory
+    // lock's own wait budget: with the default 30s, a burst that pins every
+    // connection would fail waiters at 30s even though the lock path is
+    // prepared to outwait a full 300s storage upload.
     let lock_pool = sqlx::postgres::PgPoolOptions::new()
-        .max_connections(ADVISORY_LOCK_POOL_SIZE)
+        .max_connections(config.advisory_lock_pool_size)
+        .acquire_timeout(std::time::Duration::from_secs(
+            git::repo_store::LOCK_ACQUIRE_TIMEOUT_SECS,
+        ))
         .connect(&config.database_url)
         .await
         .context("creating advisory-lock connection pool")?;

crates/gitlawb-node/src/storage/archive.rs

@@ -101,7 +101,6 @@ impl RepoArchive {
     }
 
     /// Delete a repo archive.
-    #[allow(dead_code)]
     pub async fn delete(&self, owner_slug: &str, repo_name: &str) -> Result<()> {
         self.store.delete(&Self::key(owner_slug, repo_name)).await
     }

crates/gitlawb-node/src/storage/fs.rs

@@ -116,7 +116,13 @@ impl BlobStore for FsBlobStore {
             Err(e) => Err(e).context(format!("deleting {}", path.display())),
         }
     }
+}
 
+/// Test-only helper: enumerate stored keys. `list` was dropped from the
+/// `BlobStore` trait until a production consumer (GC/admin/migration) exists;
+/// the tests here still need it to assert on stored state.
+#[cfg(test)]
+impl FsBlobStore {
     async fn list(&self, prefix: &str) -> Result<Vec<String>> {
         let root = self.root.clone();
         let prefix = prefix.to_string();

crates/gitlawb-node/src/storage/ipfs.rs

@@ -163,61 +163,4 @@ impl BlobStore for IpfsBlobStore {
             }
         }
     }
-
-    async fn list(&self, prefix: &str) -> Result<Vec<String>> {
-        // Reject traversal in the prefix before it's spliced into an MFS path
-        // (matches get/put/head/delete, which validate via validate_key).
-        if prefix.split('/').any(|seg| seg == ".." || seg == ".") {
-            anyhow::bail!("list prefix contains traversal segment: {prefix}");
-        }
-        // Best-effort recursive walk of the MFS subtree under `prefix`.
-        let mut keys = Vec::new();
-        let mut stack = vec![prefix.trim_end_matches('/').to_string()];
-        while let Some(rel) = stack.pop() {
-            let url = format!("{}/api/v0/files/ls", self.api);
-            let mfs = if rel.is_empty() {
-                "/gitlawb".to_string()
-            } else {
-                Self::mfs_path(&rel)
-            };
-            let resp = self
-                .client
-                .post(&url)
-                .query(&[("arg", mfs.as_str()), ("long", "true")])
-                .send()
-                .await
-                .context("IPFS files/ls")?;
-            if !resp.status().is_success() {
-                // Distinguish "this subtree doesn't exist" (fine — nothing to
-                // list) from real auth/network/server errors, which must surface
-                // rather than masquerade as an empty listing.
-                let body = resp.text().await.unwrap_or_default();
-                if body.contains("does not exist") || body.contains("no link named") {
-                    continue;
-                }
-                anyhow::bail!("IPFS files/ls {mfs}: {body}");
-            }
-            let v: serde_json::Value = resp.json().await.context("parsing files/ls")?;
-            if let Some(entries) = v.get("Entries").and_then(|e| e.as_array()) {
-                for entry in entries {
-                    let name = entry.get("Name").and_then(|n| n.as_str()).unwrap_or("");
-                    if name.is_empty() {
-                        continue;
-                    }
-                    let child = if rel.is_empty() {
-                        name.to_string()
-                    } else {
-                        format!("{rel}/{name}")
-                    };
-                    // Type 1 = directory in Kubo's MFS ls.
-                    if entry.get("Type").and_then(|t| t.as_u64()) == Some(1) {
-                        stack.push(child);
-                    } else {
-                        keys.push(child);
-                    }
-                }
-            }
-        }
-        Ok(keys)
-    }
 }

crates/gitlawb-node/src/storage/mod.rs

@@ -54,12 +54,6 @@ pub trait BlobStore: Send + Sync {
 
     /// Delete an object. Succeeds (no-op) if the key does not exist.
     async fn delete(&self, key: &str) -> Result<()>;
-
-    /// List object keys under a prefix. Part of the backend interface (and
-    /// implemented by every backend) for future admin/GC/migration use; not yet
-    /// wired to a caller, hence the allow.
-    #[allow(dead_code)]
-    async fn list(&self, prefix: &str) -> Result<Vec<String>>;
 }
 
 /// Build the configured blob store.

crates/gitlawb-node/src/storage/s3.rs

@@ -143,34 +143,4 @@ impl BlobStore for S3BlobStore {
             .context(format!("S3 DELETE {key}"))?;
         Ok(())
     }
-
-    async fn list(&self, prefix: &str) -> Result<Vec<String>> {
-        let mut keys = Vec::new();
-        let mut continuation: Option<String> = None;
-        loop {
-            let mut req = self
-                .s3
-                .list_objects_v2()
-                .bucket(&self.bucket)
-                .prefix(prefix);
-            if let Some(token) = continuation.take() {
-                req = req.continuation_token(token);
-            }
-            let resp = req.send().await.context(format!("S3 LIST {prefix}"))?;
-            for obj in resp.contents() {
-                if let Some(k) = obj.key() {
-                    keys.push(k.to_string());
-                }
-            }
-            if resp.is_truncated().unwrap_or(false) {
-                continuation = resp.next_continuation_token().map(|s| s.to_string());
-                if continuation.is_none() {
-                    break;
-                }
-            } else {
-                break;
-            }
-        }
-        Ok(keys)
-    }
 }