test(node): clear CodeQL alert and restore gossip readiness tests

Two findings from the 2026-09-08 review on head 988e8ea:

1. CodeQL rust/cleartext-logging on assert.rs:118. The panic sink
   interpolated redact_withheld(&reason, withheld), but CodeQL's taint
   analysis traces withheld tokens through redact_withheld without
   recognizing it as a sanitizer. Replace the interpolated panic with a
   fixed string so no tainted data reaches the sink. redact_withheld is
   still called (keeping it live) and its unit tests still pass; only the
   panic message changes. This is a test-only fixture false positive: no
   production path logs secrets.

2. Four gossip readiness tests dropped in the lib+bin split. Merge-base
   main.rs had 7 ping_peer_* tests; head had 3 renamed ping_peer_health_*
   tests. Restore the 4 missing: the /ready 200 direct happy path, the
   /ready 503 + /health 200 must-not-be-ready case, the legacy fallback
   with explicit expect-count, and the shared-deadline probe. No
   production code change; the function signatures are unchanged.

Verified: cargo fmt --check, cargo clippy -- -D warnings, 9 gossip
tests pass, 53 deny_harness tests pass (including assert unit tests).

.env.example

@@ -24,26 +24,11 @@ DATABASE_URL=postgresql://gitlawb:changeme@localhost:5432/gitlawb
 # ── Database pool & startup resilience ────────────────────────────────────
 # Maximum connections in the PostgreSQL pool. A cap, not a floor —
 # connections open lazily. Size against the DB server's max_connections,
-# remembering admin tooling opens its own pool.
+# remembering admin tooling opens its own pool. Each concurrent write pins one
+# connection for its whole duration (the connection-affine advisory lock), so the
+# node REJECTS at boot any value below GITLAWB_MAX_CONCURRENT_GIT_PUSHES + 8
+# headroom — keep this comfortably above that (default 48 for pushes 32).
 GITLAWB_DB_MAX_CONNECTIONS=48
-# Maximum connections in the DEDICATED advisory-lock pool, separate from the
-# pool above. Every in-flight repo write pins one connection here for its whole
-# duration, so this is a hard ceiling on simultaneous writes node-wide: size it
-# to expected peak concurrent writers, not small. Keeping it separate is what
-# stops a push burst from starving ordinary request handlers. Budget
-# (GITLAWB_DB_MAX_CONNECTIONS + this) per node against the server's
-# max_connections, times node count, plus admin tooling.
-GITLAWB_DB_LOCK_POOL_MAX_CONNECTIONS=40
-# Upper bound, in seconds, on any object-storage transfer that runs while a
-# per-repo write lock is HELD (the archive download inside acquire_write and the
-# upload inside release). These were free before the lock's connection was
-# pinned to the guard; now an unbounded stall holds a lock-pool slot, and enough
-# stalls deny every write on the node. The bound applies PER SPAN and there are
-# two (the acquire-side refresh, which covers the existence check and download
-# together, and the release-side upload), so worst-case slot occupancy is about
-# twice this value plus the git work between them. Read it together with the pool
-# size above and with GITLAWB_GIT_SERVICE_TIMEOUT_SECS.
-GITLAWB_LOCK_HELD_TRANSFER_TIMEOUT_SECS=300
 # Seconds a request waits for a pool connection before failing with 503.
 GITLAWB_DB_ACQUIRE_TIMEOUT_SECS=5
 # Upper bound on each startup connect+migrate attempt, in seconds. Keep it

.github/codeql/codeql-config.yml

@@ -0,0 +1,4 @@
+name: "Gitlawb CodeQL config"
+paths-ignore:
+  - "**/tests/**"
+  - "**/test_support/**"

.github/workflows/pr-checks.yml

@@ -116,6 +116,16 @@ jobs:
           DATABASE_URL: postgres://postgres:postgres@localhost:5432/gitlawb_test
         run: cargo test --locked --workspace
 
+      # The real-node deny harness is behind the `test-harness` feature (so its
+      # spawn surface never compiles into the production binary), which means
+      # `--workspace` above skips it. Run it explicitly so the trust-boundary
+      # regression cases (INV-1/INV-2/INV-8) execute on every PR. Uses the same
+      # Postgres service; `#[sqlx::test]` provisions an isolated DB per test.
+      - name: cargo test (real-node deny harness)
+        env:
+          DATABASE_URL: postgres://postgres:postgres@localhost:5432/gitlawb_test
+        run: cargo test --locked -p gitlawb-node --features test-harness --lib --test deny_harness
+
   build-release:
     name: build --release
     runs-on: ubuntu-latest

Dockerfile

@@ -26,11 +26,15 @@ COPY crates/icaptcha-client/Cargo.toml crates/icaptcha-client/
 # is what let it sit dead from the moment gl gained a path dependency on
 # icaptcha-client. A failure here should stop the build, not silently turn every
 # image build into a cold one.
+# gitlawb-node declares BOTH a [[bin]] and a [lib], so it needs both stubs:
+# cargo resolves every declared target before it fetches anything, and a
+# missing src/lib.rs aborts the layer with a target resolution error.
 RUN mkdir -p crates/gitlawb-core/src crates/gitlawb-node/src crates/gl/src \
         crates/git-remote-gitlawb/src crates/gitlawb-attest/src crates/icaptcha-client/src && \
     echo 'fn main() {}' > crates/gitlawb-node/src/main.rs && \
     echo 'fn main() {}' > crates/gl/src/main.rs && \
     echo 'fn main() {}' > crates/git-remote-gitlawb/src/main.rs && \
+    echo '' > crates/gitlawb-node/src/lib.rs && \
     echo '' > crates/gitlawb-core/src/lib.rs && \
     echo '' > crates/gitlawb-attest/src/lib.rs && \
     echo '' > crates/icaptcha-client/src/lib.rs && \

crates/gitlawb-node/Cargo.toml

@@ -6,10 +6,28 @@ edition.workspace = true
 rust-version.workspace = true
 license.workspace = true
 
+[lib]
+name = "gitlawb_node"
+path = "src/lib.rs"
+
 [[bin]]
 name = "gitlawb-node"
 path = "src/main.rs"
 
+# Exposes the `test_harness` spawn surface (src/test_harness.rs) to the
+# real-node deny-harness integration crate. Off by default so the production
+# binary never compiles test-only boot code. Enable with
+# `cargo test -p gitlawb-node --features test-harness`.
+[features]
+test-harness = []
+
+# Only builds when `test-harness` is enabled; otherwise skipped (not an error),
+# so `cargo test --workspace` without the feature stays green.
+[[test]]
+name = "deny_harness"
+path = "tests/deny_harness.rs"
+required-features = ["test-harness"]
+
 [dependencies]
 gitlawb-core = { path = "../gitlawb-core" }
 ed25519-dalek = { workspace = true }
@@ -77,6 +95,12 @@ libp2p-dns = { version = "0.44.0", features = ["tokio"] }
 [dev-dependencies]
 mockito = "1"
 tempfile = "3"
-# `test-util` for `#[tokio::test(start_paused = true)]`: lets the deadline tests assert
-# on which timer fired instead of on wall clock.
+# test-util enables tokio::test(start_paused): the delayed claim re-sweep
+# test drives the real CLAIM_SWEEP_MIN_AGE-plus-margin sleep on paused time.
 tokio = { workspace = true, features = ["test-util"] }
+# Used by the deny-harness integration crate (tests/deny_harness.rs): the
+# #[sqlx::test] macro for an ephemeral per-test pool, and reqwest as the real
+# HTTP client that drives deny paths over the socket.
+sqlx = { version = "0.8", features = ["postgres", "runtime-tokio-rustls", "macros", "migrate"] }
+reqwest = { workspace = true }
+hex = "0.4"

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

@@ -3758,13 +3758,9 @@ mod tests {
         // deterministically.
         let endpoint = crate::test_support::silent_http_endpoint().await;
         let tigris =
-            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint);
-        state.repo_store = crate::git::repo_store::RepoStore::new(
-            repos_dir,
-            Some(tigris),
-            pool,
-            std::time::Duration::from_secs(300),
-        );
+            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint)
+                .await;
+        state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool);
         state.push_limiter_trust = crate::rate_limit::TrustedProxy::None;
         let mut cfg = (*state.config).clone();
         cfg.git_acquire_timeout_secs = 1;
@@ -3835,13 +3831,9 @@ mod tests {
         // (endpoint-pinned test client, no AWS_* env reads).
         let endpoint = crate::test_support::silent_http_endpoint().await;
         let tigris =
-            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint);
-        state.repo_store = crate::git::repo_store::RepoStore::new(
-            repos_dir,
-            Some(tigris),
-            pool,
-            std::time::Duration::from_secs(300),
-        );
+            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint)
+                .await;
+        state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool);
         let mut cfg = (*state.config).clone();
         cfg.git_acquire_timeout_secs = 1;
         state.config = Arc::new(cfg);
@@ -7964,18 +7956,9 @@ mod tests {
         // (endpoint-pinned test client, no AWS_* env reads).
         let endpoint = crate::test_support::silent_http_endpoint().await;
         let tigris =
-            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint);
-        state.repo_store = crate::git::repo_store::RepoStore::new(
-            repos_dir,
-            Some(tigris),
-            pool,
-            std::time::Duration::from_secs(300),
-        );
-        state
-            .db
-            .upsert_mirror_repo("z6f3budget", "ghost", "/unused-ghost", None, false)
-            .await
-            .unwrap();
+            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint)
+                .await;
+        state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool);
         let mut cfg = (*state.config).clone();
         cfg.ipfs_request_budget_secs = 1;
         cfg.git_acquire_timeout_secs = 2;

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

@@ -61,30 +61,20 @@ pub async fn create_issue(
     let json_str = serde_json::to_string(&issue)
         .map_err(|e| AppError::BadRequest(format!("serialization error: {e}")))?;
 
+    // Shed 503 + Retry-After on an exhausted write-lock POOL instead of a generic
+    // git 500 (#173 F1). This path holds no admission permit, so it reaches the pool
+    // unthrottled; reuse the push handler's mapping so the two cannot drift.
     let guard = state
         .repo_store
         .acquire_write(&record.owner_did, &record.name)
         .await
         .map_err(|e| crate::api::repos::acquire_write_app_error(&e, &repo))?;
+    let disk_path = guard.path().to_path_buf();
 
-    let create_result = git_issues::create_issue(guard.path(), &issue_id, &json_str);
+    let create_result = git_issues::create_issue(&disk_path, &issue_id, &json_str);
 
-    let git_timeout = std::time::Duration::from_secs(state.config.git_service_timeout_secs);
-    let deadline = std::time::Instant::now() + git_timeout;
-    let git_bin = state.git_bin.clone();
-    let issue_id_for_comp = issue_id.clone();
-
-    let release_result = if create_result.is_ok() {
-        guard
-            .release_compensating(true, move |path| {
-                git_issues::delete_issue_ref(&git_bin, path, &issue_id_for_comp, deadline)
-            })
-            .await
-            .into_result()
-    } else {
-        guard.release(false).await.into_result()
-    };
-    release_result?;
+    // Always release the advisory lock — even on error; upload to Tigris only on success.
+    guard.release(create_result.is_ok()).await;
 
     create_result.map_err(|e| AppError::Git(e.to_string()))?;
 
@@ -235,8 +225,6 @@ pub async fn close_issue(
     State(state): State<AppState>,
     Extension(auth): Extension<AuthenticatedDid>,
     Path((owner, repo, issue_id)): Path<(String, String, String)>,
-    headers: axum::http::HeaderMap,
-    crate::rate_limit::PeerAddr(peer): crate::rate_limit::PeerAddr,
 ) -> Result<Json<serde_json::Value>> {
     let record = state
         .db
@@ -244,161 +232,8 @@ pub async fn close_issue(
         .await?
         .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
 
-    // READ-GATE before any snapshot work or rate charging. The author fallback below
-    // needs the issue blob, which needs the repo tree, so authorship cannot be established
-    // without a download; but a caller who cannot even READ the repo must be
-    // stopped here, cheaply, before any Tigris transfer or extraction happens.
-    // Without this, any signed non-owner could issue parallel close requests for
-    // arbitrary issue ids and drive unbounded downloads and blocking extraction
-    // (a disposable-identity DoS), because the route has no other pre-authorization.
-    //
-    // Rate limiting runs AFTER this gate so a 429 cannot distinguish a hidden repo
-    // from a missing one (INV-12 read-denial status contract).
-    //
-    // mirror-rows-handled: a repo row synced from a peer is stored public and
-    // carries none of the owner's visibility rules, so for such a row this check
-    // can only return allow. That is deliberate rather than overlooked, and it is
-    // the same verdict every other read gate in this API reaches for one. Refusing
-    // instead would deny the repo's real owner and the issue's real author on any
-    // node whose only copy of the repo is a synced one, which is an ordinary state
-    // here, and it would not buy the protection it appears to, because a synced
-    // row's recorded owner comes from the peer that sent it. The expensive work
-    // this check guards is bounded ahead of it by the per-IP limiter and the read
-    // pool slot taken below, and the authoritative owner-or-author decision still
-    // runs below and again under the write lock.
-    {
-        // mirror-rows-handled: synced mirror rows carry no owner rules; read gate only.
-        let rules = state.db.list_visibility_rules(&record.id).await?;
-        let caller = auth.0.as_str();
-        if crate::visibility::visibility_check(
-            &rules,
-            record.is_public,
-            &record.owner_did,
-            Some(caller),
-            "/",
-        ) == crate::visibility::Decision::Deny
-        {
-            return Err(AppError::RepoNotFound(format!("{owner}/{repo}")));
-        }
-    }
-
-    // Per-IP flood brake, layered on the same shared limiter and trusted-proxy
-    // policy as the push advertisement. The pre-lock snapshot downloads the
-    // whole archive and runs a blocking extraction, so an unlimited route would
-    // let disposable identities drive unbounded transfer/CPU/disk with parallel
-    // close requests for arbitrary issue ids. Applied after the read gate so a
-    // denied reader still sees 404, not 429.
-    if let Some(key) = crate::rate_limit::client_key(&headers, peer, state.push_limiter_trust) {
-        if !state.close_issue_rate_limiter.check(&key).await {
-            tracing::warn!(repo = %repo, key = %key, "close_issue rate limited");
-            return Err(AppError::TooManyRequests(
-                "rate limit exceeded — try again later".into(),
-            ));
-        }
-    }
-
-    // AUTHORIZE BEFORE ACQUIRING. The per-repo advisory lock genuinely excludes
-    // now, so taking it first would hand any caller with read access a way to hold
-    // that lock on demand and be refused afterwards, while a legitimate writer
-    // burned its retry budget against it. On a public repo that is every
-    // permissionless identity. The lock must not be reachable by a caller who is
-    // about to be refused the write.
-    let is_owner = crate::api::require_repo_owner(&record, &auth.0).is_ok();
-    if !is_owner {
-        // Cap concurrent snapshot work on the shared read pool. The hourly rate
-        // bucket above is not a concurrent-work brake; without this, parallel close
-        // attempts from read-capable callers could each drive a full archive
-        // download and blocking extraction before the author denial below.
-        let _read_permit = state
-            .git_read_semaphore
-            .clone()
-            .try_acquire_owned()
-            .map_err(|_| {
-                tracing::warn!(
-                    repo = %repo,
-                    "close_issue snapshot refused — git read pool at capacity"
-                );
-                AppError::Overloaded("git service at capacity, retry shortly".into())
-            })?;
-
-        // Not the owner, so the author fallback decides it, and the author lives in
-        // the issue's git-JSON blob rather than a DB column.
-        //
-        // Read it WITHOUT the write lock, from a NON-MUTATING SNAPSHOT. The
-        // justification is NOT that authorship is immutable — it is not:
-        // `refs/gitlawb/**` is pushable, so a forged author blob can be pushed
-        // (tracked separately; it is what makes this fallback only as trustworthy
-        // as push authorization). The justification is that this read is only a
-        // PRE-CHECK, deciding whether to take the lock at all. It is NOT the
-        // authorization decision: `acquire_write` re-downloads the archive after
-        // locking, so the tree that gets mutated is routinely not this one, and the
-        // authoritative owner-or-author check runs again under the guard below.
-        // Refusing here early just keeps a caller who is already visibly
-        // unauthorized from reaching the lock.
-        //
-        // `read_snapshot`, not `acquire_fresh`: acquire's fast path returns as soon
-        // as the directory exists and never contacts object storage, so on a node
-        // with a stale copy the author's own issue would be invisible and the
-        // cannot-establish-authorship arm below would 403 a legitimate author.
-        // read_snapshot refreshes the same way, but unpacks into a throwaway temp
-        // dir instead of publishing into the live repo path — an unlocked
-        // pre-check must not delete or swap the directory under a concurrent
-        // guarded write on the same path.
-        // Pre-lock authorization only: bound with the read acquire budget, not the
-        // under-lock transfer timeout a guarded write may hold for minutes.
-        let snapshot_bound_secs = state.config.git_acquire_timeout_secs;
-        let snapshot = tokio::time::timeout(
-            std::time::Duration::from_secs(snapshot_bound_secs),
-            state
-                .repo_store
-                .read_snapshot(&record.owner_did, &record.name),
-        )
-        .await
-        .map_err(|_elapsed| {
-            tracing::warn!(
-                repo = %repo,
-                bound_secs = snapshot_bound_secs,
-                "close_issue snapshot exceeded the read acquire bound — shedding as a retryable refusal"
-            );
-            AppError::RepoUnavailable
-        })??;
-        let snapshot_path = snapshot.path().to_path_buf();
-
-        let author_did: Option<String> = match git_issues::get_issue(&snapshot_path, &issue_id) {
-            Ok(Some(raw)) => serde_json::from_str::<IssueRecord>(&raw)
-                .ok()
-                .and_then(|i| i.author),
-            // Cannot establish authorship, so fail closed. Deliberately 403 rather
-            // than 404 for a non-owner: a caller who is not authorized to write
-            // should not learn from this route whether the issue exists. Both arms
-            // below return None; they are split only so a read failure is visible
-            // to operators, since a genuinely absent issue and an unreadable one
-            // are the same answer to the client but not the same event.
-            Ok(None) => None,
-            Err(e) => {
-                tracing::warn!(
-                    repo = %repo,
-                    issue = %issue_id,
-                    err = %e,
-                    "get_issue failed during close_issue authorship pre-check"
-                );
-                None
-            }
-        };
-        let is_author = author_did
-            .as_deref()
-            .is_some_and(|a| crate::api::did_matches(&auth.0, a));
-        if !is_author {
-            return Err(AppError::Forbidden(
-                "only the repo owner or the issue author can close this issue".into(),
-            ));
-        }
-    }
-
-    // Authorized. Only now is the lock taken.
-    // Propagate rather than stringify: AppError's From<anyhow::Error> downcasts to
-    // sqlx::Error so a pool timeout or a database outage surfaces as a retryable
-    // 503. Calling .to_string() first destroys that and reports both as a 500.
+    // Same capacity shed as create_issue above (#173 F1): an exhausted write-lock
+    // pool is a 503 + Retry-After, not a 500 git error.
     let guard = state
         .repo_store
         .acquire_write(&record.owner_did, &record.name)
@@ -406,56 +241,37 @@ pub async fn close_issue(
         .map_err(|e| crate::api::repos::acquire_write_app_error(&e, &repo))?;
     let disk_path = guard.path().to_path_buf();
 
-    // Re-read under the guard and RE-AUTHORIZE against what we read, rather than
-    // only confirming the issue still exists. The pre-lock read decided whether to
-    // take the lock; it cannot be the authorization decision, because acquire_write
-    // re-downloads the archive after locking, so this is frequently a different tree
-    // than the one the author was read from. Checking existence alone would leave the
-    // whole decision resting on the earlier read of a tree we are no longer looking
-    // at. The blob is already in hand here, so this costs a deserialize.
-    match git_issues::get_issue(&disk_path, &issue_id) {
-        Ok(Some(raw)) => {
-            let author_now: Option<String> = serde_json::from_str::<IssueRecord>(&raw)
-                .ok()
-                .and_then(|i| i.author);
-            let is_author_now = author_now
-                .as_deref()
-                .is_some_and(|a| crate::api::did_matches(&auth.0, a));
-            if !is_owner && !is_author_now {
-                // Consumed, NOT propagated, and that is deliberate at all three
-                // `release(false)` sites below. These release without
-                // publishing, so there is nothing for the store to refuse, and
-                // mapping the outcome here would let a 503 shadow the
-                // authorization answer this route exists to give.
-                let _ = guard.release(false).await;
-                return Err(AppError::Forbidden(
-                    "only the repo owner or the issue author can close this issue".into(),
-                ));
-            }
-        }
+    // Owner OR issue author may close. The author lives in the issue's git-JSON
+    // blob (not a DB column); a None author (legacy issues) falls back to
+    // owner-only. Read it under the write guard, before mutating.
+    let author_did: Option<String> = match git_issues::get_issue(&disk_path, &issue_id) {
+        Ok(Some(raw)) => serde_json::from_str::<IssueRecord>(&raw)
+            .ok()
+            .and_then(|i| i.author),
         Ok(None) => {
-            let _ = guard.release(false).await;
-            // The owner keeps the informative 404; a non-owner must not learn from
-            // this route whether the issue exists, matching the pre-check above.
-            return Err(if is_owner {
-                AppError::NotFound(format!("issue {issue_id} not found"))
-            } else {
-                AppError::Forbidden(
-                    "only the repo owner or the issue author can close this issue".into(),
-                )
-            });
+            guard.release(false).await;
+            return Err(AppError::NotFound(format!("issue {issue_id} not found")));
         }
         Err(e) => {
-            let _ = guard.release(false).await;
+            guard.release(false).await;
             return Err(AppError::Git(e.to_string()));
         }
+    };
+    let is_owner = crate::api::require_repo_owner(&record, &auth.0).is_ok();
+    let is_author = author_did
+        .as_deref()
+        .is_some_and(|a| crate::api::did_matches(&auth.0, a));
+    if !is_owner && !is_author {
+        guard.release(false).await;
+        return Err(AppError::Forbidden(
+            "only the repo owner or the issue author can close this issue".into(),
+        ));
     }
 
     let close_result = git_issues::close_issue(&disk_path, &issue_id);
 
     // Always release the advisory lock — even on error; upload to Tigris only on success.
-    // Same short-circuit as create_issue, and before the 200 body below.
-    guard.release(close_result.is_ok()).await.into_result()?;
+    guard.release(close_result.is_ok()).await;
 
     let updated = close_result
         .map_err(|e| AppError::Git(e.to_string()))?
@@ -469,344 +285,12 @@ pub async fn close_issue(
     Ok(Json(issue))
 }
 
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use sqlx::PgPool;
-
-    /// U7: once the advisory lock actually excludes, taking it BEFORE authorizing
-    /// turns close_issue into a wedge primitive. Any caller with repo read access
-    /// (on a public repo, any permissionless identity) could take the per-repo
-    /// write lock on demand and be refused the write afterwards, while the owner's
-    /// push burned its retry budget against a lock held by someone with no write
-    /// authorization.
-    ///
-    /// The observable: hold the lock from an independent session, then call the
-    /// handler as a stranger. If it authorizes first it refuses immediately; if it
-    /// acquires first it sits in the 60-attempt retry loop and the deadline fires.
-    #[sqlx::test]
-    async fn stranger_is_refused_without_waiting_on_the_write_lock(pool: PgPool) {
-        use sqlx::Connection;
-        let opts = (*pool.connect_options()).clone();
-        let state = crate::test_support::test_state(pool.clone()).await;
-
-        let owner = "did:key:z6MkU7Owner";
-        state
-            .db
-            .upsert_mirror_repo("z6MkU7Owner", "u7repo", "/tmp/u7repo", None, true)
-            .await
-            .expect("seed repo");
-        let record = state
-            .db
-            .get_repo("z6MkU7Owner", "u7repo")
-            .await
-            .expect("get_repo")
-            .expect("repo exists");
-
-        // An independent session holds the repo's write lock for the whole call.
-        let key = crate::git::repo_store::advisory_lock_key_for_test(
-            &record.owner_did.replace([':', '/'], "_"),
-            &record.name,
-        );
-        let mut holder = sqlx::PgConnection::connect_with(&opts).await.unwrap();
-        let held: (bool,) = sqlx::query_as("SELECT pg_try_advisory_lock($1)")
-            .bind(key)
-            .fetch_one(&mut holder)
-            .await
-            .unwrap();
-        assert!(
-            held.0,
-            "the test must hold the lock for this to mean anything"
-        );
-        let _ = owner;
-
-        let stranger = crate::auth::AuthenticatedDid("did:key:z6MkU7Stranger".to_string());
-        let outcome = tokio::time::timeout(
-            std::time::Duration::from_secs(3),
-            close_issue(
-                axum::extract::State(state.clone()),
-                axum::Extension(stranger),
-                axum::extract::Path((
-                    "z6MkU7Owner".to_string(),
-                    "u7repo".to_string(),
-                    "1".to_string(),
-                )),
-                axum::http::HeaderMap::new(),
-                crate::rate_limit::PeerAddr(Some("203.0.113.64:5000".parse().unwrap())),
-            ),
-        )
-        .await;
-
-        let refused = outcome.expect(
-            "a caller with no write authorization must be refused WITHOUT waiting on the \
-             write lock; hitting this deadline means the handler tried to acquire first, \
-             which is the wedge primitive",
-        );
-        assert!(
-            matches!(refused, Err(AppError::Forbidden(_))),
-            "expected 403 Forbidden for a stranger, got {:?}",
-            refused.err().map(|e| format!("{e:?}"))
-        );
-    }
-
-    /// The read-gate's blind spot, pinned rather than left to be rediscovered.
-    ///
-    /// A repo row synced from a peer is stored public and carries none of the
-    /// owner's visibility rules, so the gate's own inputs can only produce allow
-    /// for it. The gate above therefore does not carry this class of row, and the
-    /// test that does cover it (`non_reader_is_refused_before_the_snapshot`) seeds
-    /// a locally created repo, which cannot observe this: a passing test there is
-    /// not coverage here.
-    ///
-    /// Two things are asserted, and the second is why the first is acceptable.
-    /// The gate's verdict for such a row is allow for an arbitrary caller, and the
-    /// handler still refuses that caller afterwards, because the decision that
-    /// matters is the owner-or-author check rather than this one. If a later change
-    /// makes the gate the load-bearing decision for this route, the first assertion
-    /// breaks and this comment is where to start.
-    #[sqlx::test]
-    async fn a_synced_row_is_not_gated_by_its_own_visibility(pool: PgPool) {
-        let state = crate::test_support::test_state(pool.clone()).await;
-
-        // Only a synced row exists for this repo, with no locally created twin.
-        state
-            .db
-            .upsert_mirror_repo("z6MkSyncOwner", "syncrepo", "/tmp/syncrepo", None, false)
-            .await
-            .expect("seed synced repo"); // false = not quarantined, the ordinary case
-        let record = state
-            .db
-            .get_repo("z6MkSyncOwner", "syncrepo")
-            .await
-            .expect("get_repo")
-            .expect("repo exists")
-            .clone();
-        assert!(
-            record.id.contains('/'),
-            "this test is only meaningful against a synced row; got id {}",
-            record.id
-        );
-
-        // The gate's two inputs, and what they force.
-        let rules = state
-            .db
-            .list_visibility_rules(&record.id)
-            .await
-            .expect("list rules");
-        assert!(rules.is_empty(), "a synced row carries no rules of its own");
-        assert!(record.is_public, "a synced row is stored public");
-        assert_eq!(
-            crate::visibility::visibility_check(
-                &rules,
-                record.is_public,
-                &record.owner_did,
-                Some("did:key:z6MkSyncStranger"),
-                "/",
-            ),
-            crate::visibility::Decision::Allow,
-            "the gate can only allow for a synced row, which is the property the \
-             handler's mirror-rows-handled note records",
-        );
-
-        // So the refusal has to come from the decision that is actually load-bearing.
-        let stranger = crate::auth::AuthenticatedDid("did:key:z6MkSyncStranger".to_string());
-        let outcome = close_issue(
-            axum::extract::State(state.clone()),
-            axum::Extension(stranger),
-            axum::extract::Path((
-                "z6MkSyncOwner".to_string(),
-                "syncrepo".to_string(),
-                "1".to_string(),
-            )),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(Some("203.0.113.99:5000".parse().unwrap())),
-        )
-        .await;
-        assert!(
-            outcome.is_err(),
-            "a stranger must still be refused on a synced row, gate or no gate",
-        );
-        let body = format!("{:?}", outcome.err().unwrap());
-        assert!(
-            !body.contains("syncrepo/") && !body.to_lowercase().contains("issue body"),
-            "the refusal must not leak repo contents: {body}",
-        );
-    }
-
-    /// The read-gate added for the pre-lock snapshot: a caller who cannot READ
-    /// the repo (private repo, no rule granting them access) must be refused with
-    /// a not-found BEFORE any snapshot download or extraction happens. The
-    /// observable is the refusal itself; the cheaper part (no Tigris work) is
-    /// structural (the gate precedes the snapshot call in the handler).
-    #[sqlx::test]
-    async fn non_reader_is_refused_before_the_snapshot(pool: PgPool) {
-        let state = crate::test_support::test_state(pool.clone()).await;
-        let now = chrono::Utc::now();
-        state
-            .db
-            .create_repo(&crate::db::RepoRecord {
-                id: uuid::Uuid::new_v4().to_string(),
-                name: "priv-close".to_string(),
-                owner_did: "z6MkT3Owner".to_string(),
-                description: None,
-                is_public: false,
-                default_branch: "main".to_string(),
-                created_at: now,
-                updated_at: now,
-                disk_path: "/tmp/priv-close".to_string(),
-                forked_from: None,
-                machine_id: None,
-            })
-            .await
-            .expect("seed private repo");
-
-        let stranger = crate::auth::AuthenticatedDid("did:key:z6MkT3Stranger".to_string());
-        let res = close_issue(
-            axum::extract::State(state.clone()),
-            axum::Extension(stranger),
-            axum::extract::Path((
-                "z6MkT3Owner".to_string(),
-                "priv-close".to_string(),
-                "1".to_string(),
-            )),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(Some("203.0.113.67:5000".parse().unwrap())),
-        )
-        .await;
-        assert!(
-            matches!(res, Err(AppError::RepoNotFound(_))),
-            "a non-reader must be refused as not-found, got {:?}",
-            res.err().map(|e| format!("{e:?}"))
-        );
-    }
-
-    async fn seed_repo_with_issue(
-        state: &crate::state::AppState,
-        owner_slug: &str,
-        owner_did: &str,
-        repo: &str,
-        issue_id: &str,
-        author_did: &str,
-    ) -> std::path::PathBuf {
-        state
-            .db
-            .upsert_mirror_repo(owner_slug, repo, "/unused", None, true)
-            .await
-            .expect("seed repo row");
-        // Seed at the path the HANDLER will resolve. upsert_mirror_repo stores the
-        // bare slug in owner_did, and close_issue resolves from record.owner_did, so
-        // seeding from the full did:key would create the repo in a different
-        // directory and the handler would find nothing.
-        let record = state
-            .db
-            .get_repo(owner_slug, repo)
-            .await
-            .expect("get_repo")
-            .expect("seeded repo exists");
-        let _ = owner_did;
-        let path = state
-            .repo_store
-            .acquire(&record.owner_did, &record.name)
-            .await
-            .expect("resolve disk path");
-        let _ = std::fs::remove_dir_all(&path);
-        crate::git::store::init_bare(&path).expect("init bare repo");
-        // Must deserialize as a real IssueRecord: `created_at` and `status` are
-        // required, and a parse failure would silently drop the author (the
-        // `.ok()` on from_str), which reads as a 403 rather than as a broken fixture.
-        let json = serde_json::to_string(&IssueRecord {
-            id: issue_id.to_string(),
-            title: "seeded".to_string(),
-            body: Some(String::new()),
-            author: Some(author_did.to_string()),
-            created_at: chrono::Utc::now().to_rfc3339(),
-            status: "open".to_string(),
-            signed_payload: None,
-        })
-        .expect("serialize seeded issue");
-        crate::git::issues::create_issue(&path, issue_id, &json).expect("seed issue blob");
-        path
-    }
-
-    /// INV-21(c) positive twin 1: the OWNER can still close. The reorder moved the
-    /// owner check above the lock, so this is the arm most likely to have broken,
-    /// and the deny test alone could not see it.
-    ///
-    /// The issue is seeded with a THIRD party as its author, deliberately. Seeding
-    /// the owner as their own author made this test unable to fail: with the owner
-    /// check disabled, the author fallback granted the close anyway and the test
-    /// stayed green. Only the owner arm can grant here now.
-    #[sqlx::test]
-    async fn owner_can_still_close_after_the_reorder(pool: PgPool) {
-        let state = crate::test_support::test_state(pool.clone()).await;
-        let owner_did = "did:key:z6MkT1Owner";
-        seed_repo_with_issue(
-            &state,
-            "z6MkT1Owner",
-            owner_did,
-            "t1repo",
-            "1",
-            "did:key:z6MkT1Stranger",
-        )
-        .await;
-
-        let res = close_issue(
-            axum::extract::State(state.clone()),
-            axum::Extension(crate::auth::AuthenticatedDid(owner_did.to_string())),
-            axum::extract::Path((
-                "z6MkT1Owner".to_string(),
-                "t1repo".to_string(),
-                "1".to_string(),
-            )),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(Some("203.0.113.65:5000".parse().unwrap())),
-        )
-        .await;
-        assert!(
-            res.is_ok(),
-            "the owner must still be able to close: {:?}",
-            res.err().map(|e| format!("{e:?}"))
-        );
-    }
-
-    /// INV-21(c) positive twin 2: the non-owner AUTHOR can still close, through both
-    /// the pre-lock check and the re-assertion under the guard.
-    ///
-    /// It does NOT cover the acquire-vs-acquire_fresh distinction, despite that being
-    /// the reason the call changed. `RepoStore::for_testing` hardcodes `tigris: None`,
-    /// which makes `acquire` and `acquire_fresh` identical in every test here, so
-    /// reverting that line leaves this green. Separating them needs an object-storage
-    /// seam, which is out of scope for this change and tracked separately. Claiming
-    /// the coverage here would be worse than admitting the gap.
-    #[sqlx::test]
-    async fn issue_author_who_is_not_the_owner_can_still_close(pool: PgPool) {
-        let state = crate::test_support::test_state(pool.clone()).await;
-        let owner_did = "did:key:z6MkT2Owner";
-        let author_did = "did:key:z6MkT2Author";
-        seed_repo_with_issue(&state, "z6MkT2Owner", owner_did, "t2repo", "1", author_did).await;
-
-        let res = close_issue(
-            axum::extract::State(state.clone()),
-            axum::Extension(crate::auth::AuthenticatedDid(author_did.to_string())),
-            axum::extract::Path((
-                "z6MkT2Owner".to_string(),
-                "t2repo".to_string(),
-                "1".to_string(),
-            )),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(Some("203.0.113.66:5000".parse().unwrap())),
-        )
-        .await;
-        assert!(
-            res.is_ok(),
-            "the issue author, who is NOT the repo owner, must still be able to close: {:?}",
-            res.err().map(|e| format!("{e:?}"))
-        );
-    }
-}
-
-/// #173 F1 follow-up: issue write paths reach `acquire_write` with no admission
-/// permit, so an exhausted write-lock pool must shed 503 + Retry-After.
+/// #173 F1 follow-up: the two issue write paths reach `acquire_write` holding NO
+/// admission permit (unlike the push handler, which is capped by the git-push
+/// semaphore), so they are the callers most likely to meet an exhausted write-lock
+/// POOL under load. An exhausted pool is a capacity signal, so both must shed
+/// 503 + Retry-After (`AppError::Overloaded`) the way the push handler does, not
+/// report the generic 500 git error that says nothing about retrying.
 #[cfg(test)]
 mod lock_pool_shed_tests {
     use super::*;
@@ -830,17 +314,22 @@ mod lock_pool_shed_tests {
         }
     }
 
+    /// State whose repo store draws write locks from a ONE-connection pool with a
+    /// short checkout timeout, so a single held guard exhausts it promptly rather
+    /// than at the pool default.
     async fn one_connection_lock_pool_state(pool: &PgPool) -> AppState {
         let mut state = crate::test_support::test_state(pool.clone()).await;
         state.repo_store = crate::git::repo_store::RepoStore::new(
             std::path::PathBuf::from("/tmp/gitlawb-issues-lockpool"),
             None,
             crate::git::repo_store::build_lock_pool(pool, 1, std::time::Duration::from_secs(1)),
-            std::time::Duration::from_secs(300),
         );
         state
     }
 
+    /// The shed must be a real 503 carrying Retry-After, not just an internal enum
+    /// variant: assert on the rendered response so a remapping of `Overloaded` is
+    /// caught here too.
     fn assert_sheds_503_with_retry_after(err: AppError, what: &str) {
         let resp = err.into_response();
         assert_eq!(
@@ -857,6 +346,10 @@ mod lock_pool_shed_tests {
         );
     }
 
+    /// RED-before/GREEN-after for `create_issue`. Both directions: the shed while the
+    /// only lock-pool connection is held by a guard on a DIFFERENT repo (so this is
+    /// pool capacity, not advisory-lock contention on this repo), and the must-not
+    /// case once that connection is back.
     #[sqlx::test]
     async fn create_issue_lock_pool_exhaustion_sheds_503_not_500(pool: PgPool) {
         let owner = "did:key:zISSUECREATELOCKPOOLAAAAAAAAAAAAAAAAAAAA";
@@ -887,7 +380,9 @@ mod lock_pool_shed_tests {
         let err = shed.expect_err("an exhausted lock pool must fail the call");
         assert_sheds_503_with_retry_after(err, "create_issue");
 
-        let _ = held.release(false).await;
+        // MUST-NOT: with the pool free again the call is not shed as capacity (it
+        // fails later on the nonexistent on-disk repo, which is a git 500).
+        held.release(false).await;
         let admitted = create_issue(
             State(state.clone()),
             Extension(AuthenticatedDid(owner.to_string())),
@@ -906,38 +401,7 @@ mod lock_pool_shed_tests {
         );
     }
 
-    #[sqlx::test]
-    async fn close_issue_read_pool_exhaustion_sheds_before_snapshot(pool: PgPool) {
-        use std::sync::Arc;
-        let owner = "did:key:zISSUECLOSEREADPOOLBBBBBBBBBBBBBBBBBBBBB";
-        let mut state = crate::test_support::test_state(pool.clone()).await;
-        state.git_read_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
-        state
-            .db
-            .create_repo(&seed_repo(owner, "read-cap"))
-            .await
-            .expect("seed repo");
-
-        let shed = close_issue(
-            State(state.clone()),
-            Extension(AuthenticatedDid(
-                "did:key:zISSUECLOSEREADSTRANGER".to_string(),
-            )),
-            Path((
-                owner.to_string(),
-                "read-cap".to_string(),
-                "deadbeef".to_string(),
-            )),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(None),
-        )
-        .await;
-        assert!(
-            matches!(shed, Err(AppError::Overloaded(_))),
-            "an exhausted read pool must shed before snapshot work; got {shed:?}"
-        );
-    }
-
+    /// RED-before/GREEN-after for `close_issue`, same two directions.
     #[sqlx::test]
     async fn close_issue_lock_pool_exhaustion_sheds_503_not_500(pool: PgPool) {
         let owner = "did:key:zISSUECLOSELOCKPOOLBBBBBBBBBBBBBBBBBBBBB";
@@ -962,14 +426,12 @@ mod lock_pool_shed_tests {
                 "lp-close".to_string(),
                 "deadbeef".to_string(),
             )),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(None),
         )
         .await;
         let err = shed.expect_err("an exhausted lock pool must fail the call");
         assert_sheds_503_with_retry_after(err, "close_issue");
 
-        let _ = held.release(false).await;
+        held.release(false).await;
         let admitted = close_issue(
             State(state.clone()),
             Extension(AuthenticatedDid(owner.to_string())),
@@ -978,8 +440,6 @@ mod lock_pool_shed_tests {
                 "lp-close".to_string(),
                 "deadbeef".to_string(),
             )),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(None),
         )
         .await;
         assert!(
@@ -988,244 +448,4 @@ mod lock_pool_shed_tests {
             admitted.err()
         );
     }
-
-    async fn assert_retryable_repo_acquire(err: AppError, what: &str, code: &str) {
-        let resp = err.into_response();
-        assert_eq!(
-            resp.status(),
-            StatusCode::SERVICE_UNAVAILABLE,
-            "{what}: a transient acquire refusal must shed 503, not a 500 git error"
-        );
-        let body = axum::body::to_bytes(resp.into_body(), 64 * 1024)
-            .await
-            .expect("body");
-        let body = String::from_utf8_lossy(&body);
-        assert!(
-            body.contains(code),
-            "{what}: the 503 must carry the {code} code, got {body}"
-        );
-    }
-
-    /// Advisory-lock contention on a non-push mutation must map through the shared
-    /// `acquire_write_app_error` classifier to `repo_busy`, not a 500 git_error.
-    #[sqlx::test]
-    async fn create_issue_contention_sheds_repo_busy_not_500(pool: PgPool) {
-        let owner = "did:key:zISSUECREATEBUSYAAAAAAAAAAAAAAAAAAAAA";
-        let repo_name = "busy-create";
-        let state = crate::test_support::test_state(pool.clone()).await;
-        state
-            .db
-            .create_repo(&seed_repo(owner, repo_name))
-            .await
-            .expect("seed repo");
-
-        let held = state
-            .repo_store
-            .acquire_write(owner, repo_name)
-            .await
-            .expect("the first writer takes the advisory lock");
-
-        let shed = create_issue(
-            State(state.clone()),
-            Extension(AuthenticatedDid(owner.to_string())),
-            Path((owner.to_string(), repo_name.to_string())),
-            Json(CreateIssueRequest {
-                title: "t".to_string(),
-                body: None,
-                signed_payload: None,
-            }),
-        )
-        .await;
-        let err = shed.expect_err("contention must refuse the second writer");
-        assert_retryable_repo_acquire(err, "create_issue", "repo_busy").await;
-
-        let _ = held.release(false).await;
-    }
-
-    /// A refused under-lock refresh on a non-push mutation must map to
-    /// `repo_unavailable`, not expose the storage error as a 500 git_error.
-    #[sqlx::test]
-    async fn create_issue_unavailable_sheds_repo_unavailable_not_500(pool: PgPool) {
-        use axum::response::IntoResponse;
-
-        let app = axum::Router::new().route(
-            "/{*key}",
-            axum::routing::any(|method: axum::http::Method| async move {
-                if method == axum::http::Method::HEAD {
-                    let mut resp = axum::http::StatusCode::OK.into_response();
-                    resp.headers_mut()
-                        .insert("etag", axum::http::HeaderValue::from_static("\"gen-1\""));
-                    resp
-                } else {
-                    axum::http::StatusCode::INTERNAL_SERVER_ERROR.into_response()
-                }
-            }),
-        );
-        let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
-        let endpoint = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port());
-        let server = tokio::spawn(async move {
-            axum::serve(listener, app).await.unwrap();
-        });
-
-        let owner = "did:key:zISSUEUNAVAILAAAAAAAAAAAAAAAAAAAAAAA";
-        let repo_name = "unavail-create";
-        let mut state = crate::test_support::test_state(pool.clone()).await;
-        state.repo_store = crate::git::repo_store::RepoStore::for_testing_with_tigris(
-            std::path::PathBuf::from("/tmp/gitlawb-issue-unavail"),
-            crate::git::repo_store::build_lock_pool(&pool, 2, std::time::Duration::from_secs(1)),
-            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint),
-        );
-        state
-            .db
-            .create_repo(&seed_repo(owner, repo_name))
-            .await
-            .expect("seed repo");
-
-        let shed = create_issue(
-            State(state.clone()),
-            Extension(AuthenticatedDid(owner.to_string())),
-            Path((owner.to_string(), repo_name.to_string())),
-            Json(CreateIssueRequest {
-                title: "t".to_string(),
-                body: None,
-                signed_payload: None,
-            }),
-        )
-        .await;
-        let err = shed.expect_err("a failed archive download must refuse the write");
-        assert_retryable_repo_acquire(err, "create_issue", "repo_unavailable").await;
-
-        server.abort();
-    }
-
-    /// A denied reader on a private repo must see 404 even when the caller's rate
-    /// bucket is exhausted. Rate limiting runs after the read gate.
-    #[sqlx::test]
-    async fn close_issue_rate_limit_runs_after_the_read_gate(pool: PgPool) {
-        use std::net::SocketAddr;
-        use std::time::Duration;
-
-        let owner = "did:key:zCLOSERATEOWNERAAAAAAAAAAAAAAAAAAAAAAA";
-        let stranger = "did:key:zCLOSERATESTRANGERBBBBBBBBBBBBBBBBBB";
-        let mut state = crate::test_support::test_state(pool).await;
-        state.close_issue_rate_limiter =
-            crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60));
-        state.push_limiter_trust = crate::rate_limit::TrustedProxy::None;
-
-        let mut repo = seed_repo(owner, "priv-close");
-        repo.is_public = false;
-        state.db.create_repo(&repo).await.expect("seed repo");
-
-        let peer: SocketAddr = "203.0.113.88:7000".parse().unwrap();
-        assert!(
-            state
-                .close_issue_rate_limiter
-                .check(&peer.ip().to_string())
-                .await,
-            "exhaust the close_issue bucket before close_issue"
-        );
-
-        let res = close_issue(
-            State(state),
-            Extension(AuthenticatedDid(stranger.to_string())),
-            Path((owner.to_string(), "priv-close".to_string(), "1".to_string())),
-            axum::http::HeaderMap::new(),
-            crate::rate_limit::PeerAddr(Some(peer)),
-        )
-        .await;
-
-        assert!(
-            matches!(res, Err(AppError::RepoNotFound(_))),
-            "a non-reader must see 404, not 429, even when rate limited: {:?}",
-            res
-        );
-    }
-
-    /// close_issue and receive-pack must not share one per-IP bucket.
-    #[sqlx::test]
-    async fn close_issue_rate_limit_does_not_drain_push_bucket(pool: PgPool) {
-        use std::net::SocketAddr;
-        use std::time::Duration;
-
-        let mut state = crate::test_support::test_state(pool).await;
-        state.close_issue_rate_limiter =
-            crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60));
-        state.push_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60));
-        state.push_limiter_trust = crate::rate_limit::TrustedProxy::None;
-
-        let peer: SocketAddr = "203.0.113.99:7000".parse().unwrap();
-        let key = peer.ip().to_string();
-        assert!(
-            state.close_issue_rate_limiter.check(&key).await,
-            "exhaust only the close_issue bucket"
-        );
-        assert!(
-            state.push_rate_limiter.check(&key).await,
-            "push traffic must keep its own bucket after close_issue is exhausted"
-        );
-    }
-
-    /// A refused publish must roll back the local issue ref so a retry does not
-    /// mint a second id for the same logical filing attempt.
-    #[sqlx::test]
-    async fn create_issue_rolls_back_local_ref_when_publish_refuses(pool: PgPool) {
-        use axum::response::IntoResponse;
-
-        let app = axum::Router::new().route(
-            "/{*key}",
-            axum::routing::any(|method: axum::http::Method| async move {
-                if method == axum::http::Method::HEAD {
-                    let mut resp = axum::http::StatusCode::OK.into_response();
-                    resp.headers_mut()
-                        .insert("etag", axum::http::HeaderValue::from_static("\"gen-1\""));
-                    resp
-                } else {
-                    axum::http::StatusCode::INTERNAL_SERVER_ERROR.into_response()
-                }
-            }),
-        );
-        let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
-        let endpoint = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port());
-        let server = tokio::spawn(async move {
-            axum::serve(listener, app).await.unwrap();
-        });
-
-        let owner = "did:key:zISSUEROLLBACKAAAAAAAAAAAAAAAAAAAAAAA";
-        let repo_name = "rollback-create";
-        let mut state = crate::test_support::test_state(pool.clone()).await;
-        let disk = tempfile::TempDir::new().unwrap();
-        crate::git::store::init_bare(&disk.path().join("repo.git")).expect("bare repo");
-        let mut seeded = seed_repo(owner, repo_name);
-        seeded.disk_path = disk.path().join("repo.git").to_string_lossy().to_string();
-        state.db.create_repo(&seeded).await.expect("seed repo");
-        state.repo_store = crate::git::repo_store::RepoStore::for_testing_with_tigris(
-            disk.path().to_path_buf(),
-            crate::git::repo_store::build_lock_pool(&pool, 2, std::time::Duration::from_secs(1)),
-            crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint),
-        );
-
-        let shed = create_issue(
-            State(state.clone()),
-            Extension(AuthenticatedDid(owner.to_string())),
-            Path((owner.to_string(), repo_name.to_string())),
-            Json(CreateIssueRequest {
-                title: "t".to_string(),
-                body: None,
-                signed_payload: None,
-            }),
-        )
-        .await;
-        assert!(
-            shed.is_err(),
-            "a failed archive upload must refuse the write"
-        );
-
-        let issues = git_issues::list_issues(&disk.path().join("repo.git")).expect("list issues");
-        assert!(
-            issues.is_empty(),
-            "the local issue ref must be rolled back after a refused publish, got {issues:?}"
-        );
-
-        server.abort();
-    }
 }

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

@@ -209,6 +209,9 @@ pub async fn merge_pr(
         return Err(AppError::BadRequest(format!("PR is already {}", pr.status)));
     }
 
+    // Shed 503 + Retry-After on an exhausted write-lock POOL instead of a generic
+    // git 500 (#173 F1). Merging holds no admission permit, so it reaches the pool
+    // unthrottled; reuse the push handler's mapping so the two cannot drift.
     let guard = state
         .repo_store
         .acquire_write(&record.owner_did, &record.name)
@@ -225,10 +228,7 @@ pub async fn merge_pr(
     );
 
     // Always release the advisory lock — even on error; upload to Tigris only on success.
-    // Short-circuit on a refused publish before the PR is marked merged and
-    // before the webhook fires. Both are irreversible announcements of a merge
-    // commit that only exists on this node's disk.
-    guard.release(merge_result.is_ok()).await.into_result()?;
+    guard.release(merge_result.is_ok()).await;
 
     let merge_sha = merge_result.map_err(|e| AppError::Git(e.to_string()))?;
 
@@ -428,8 +428,12 @@ pub async fn list_comments(
     Ok(Json(serde_json::json!({ "comments": comments })))
 }
 
-/// #173 F1 follow-up: merge holds no admission permit, so an exhausted write-lock
-/// pool must shed 503 + Retry-After.
+/// #173 F1 follow-up: `merge_pr` reaches `acquire_write` holding NO admission permit
+/// (unlike the push handler, which is capped by the git-push semaphore), so it is one
+/// of the callers most likely to meet an exhausted write-lock POOL under load. An
+/// exhausted pool is a capacity signal, so the merge must shed 503 + Retry-After
+/// (`AppError::Overloaded`) the way the push handler does, not report the generic
+/// 500 git error that says nothing about retrying.
 #[cfg(test)]
 mod lock_pool_shed_tests {
     use super::*;
@@ -453,15 +457,20 @@ mod lock_pool_shed_tests {
         }
     }
 
+    /// RED-before/GREEN-after for `merge_pr`. Both directions: the shed while the only
+    /// lock-pool connection is held by a guard on a DIFFERENT repo (so this is pool
+    /// capacity, not advisory-lock contention on this repo), and the must-not case
+    /// once that connection is back.
     #[sqlx::test]
     async fn merge_pr_lock_pool_exhaustion_sheds_503_not_500(pool: PgPool) {
         let owner = "did:key:zMERGELOCKPOOLOWNERAAAAAAAAAAAAAAAAAAAAA";
         let mut state = crate::test_support::test_state(pool.clone()).await;
+        // One lock-pool connection with a short checkout timeout, so a single held
+        // guard exhausts it promptly rather than at the pool default.
         state.repo_store = crate::git::repo_store::RepoStore::new(
             std::path::PathBuf::from("/tmp/gitlawb-pulls-lockpool"),
             None,
             crate::git::repo_store::build_lock_pool(&pool, 1, std::time::Duration::from_secs(1)),
-            std::time::Duration::from_secs(300),
         );
 
         let repo = seed_repo(owner, "lp-merge");
@@ -515,7 +524,9 @@ mod lock_pool_shed_tests {
             "merge_pr: a capacity shed must tell the client when to retry"
         );
 
-        let _ = held.release(false).await;
+        // MUST-NOT: with the pool free again the merge is not shed as capacity (it
+        // fails later on the nonexistent on-disk repo, which is a git 500).
+        held.release(false).await;
         let admitted = merge_pr(
             State(state.clone()),
             Extension(AuthenticatedDid(owner.to_string())),

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

@@ -3,7 +3,6 @@ use axum::http::StatusCode;
 use axum::response::Response;
 use axum::Json;
 use bytes::Bytes;
-use std::path::PathBuf;
 use std::sync::Arc;
 
 use crate::auth::{caller_authorized_to_push, AuthenticatedDid};
@@ -693,17 +692,8 @@ pub async fn git_info_refs(
         git_permit(&state.git_read_semaphore)?
     };
 
-    // For receive-pack (push), read the latest from Tigris so the client sees
-    // the same refs that acquire_write() will operate on. A NON-MUTATING
-    // snapshot, not `acquire_fresh`: the advertisement runs WITHOUT the advisory
-    // lock, and acquire_fresh downloads and publishes into the live repo path
-    // (removing the existing directory and renaming the extract into place), so
-    // an unlocked advertisement could delete or swap the directory under a
-    // concurrent guarded write. In the worst ordering the guarded write has
-    // finished but `release` has not compressed the tree, and the guarded
-    // release uploads the replaced old tree with its still-valid ETag and
-    // reports success, losing the accepted write. The snapshot unpacks into a
-    // throwaway temp dir that is served from and then removed.
+    // For receive-pack (push), download the latest from Tigris so the client
+    // sees the same refs that acquire_write() will operate on.
     //
     // Bound the acquire under `git_acquire_timeout_secs`: the concurrency permit is
     // already held above, and `git_service_timeout_secs` only starts once git spawns,
@@ -713,46 +703,28 @@ pub async fn git_info_refs(
     // so the shed frees the slot; return a bounded 503.
     let acquire_deadline = std::time::Duration::from_secs(state.config.git_acquire_timeout_secs);
     let acquire_fut = async {
-        let res = if service == "git-receive-pack" {
+        if service == "git-receive-pack" {
             state
                 .repo_store
-                .read_snapshot(&record.owner_did, &record.name)
+                .acquire_fresh(&record.owner_did, &record.name)
                 .await
-                .map(|s| (s.path().to_path_buf(), Some(s)))
         } else {
             state
                 .repo_store
                 .acquire(&record.owner_did, &record.name)
                 .await
-                .map(|p| (p, None))
-        };
-        res.map_err(|e| {
-            if is_expected_transient_acquire_failure(&e) {
-                tracing::warn!(repo = %name, service = %service, err = %e, "repo acquire failed");
-            } else {
-                tracing::error!(repo = %name, service = %service, err = %e, "repo acquire failed");
-            }
-            // This closure bypasses the `From<anyhow::Error>` chain, so a typed
-            // refusal would otherwise be stringified into a 500 `git_error`. Route
-            // just that one case through `From` and leave every other failure on
-            // exactly today's behavior: this call site also serves the read path via
-            // `acquire`, whose error vocabulary is out of scope here.
-            if e.is::<crate::git::repo_store::RepoUnavailable>() {
-                AppError::from(e)
-            } else {
-                AppError::Git(e.to_string())
-            }
-        })
+        }
     };
-    // The snapshot (if any) is kept alive for the whole handler scope below: its
-    // Drop removes the temp dir it was unpacked into, so dropping it here would
-    // delete the directory `info_refs` is about to serve from.
-    let (disk_path, _snapshot_keepalive) = tokio::time::timeout(acquire_deadline, acquire_fut)
+    let disk_path = tokio::time::timeout(acquire_deadline, acquire_fut)
         .await
         .map_err(|_elapsed| {
             tracing::warn!(repo = %name, service = %service, "repo acquire timed out; shedding with 503");
             AppError::Overloaded("git service acquisition timed out, retry shortly".into())
-        })??;
+        })?
+        .map_err(|e| {
+            tracing::error!(repo = %name, service = %service, err = %e, "repo acquire failed");
+            AppError::Git(e.to_string())
+        })?;
 
     // Move the admission permits into the guard so they release only after the spawned
     // git process group is confirmed reaped, on complete/timeout/disconnect — not the
@@ -1006,7 +978,6 @@ pub(crate) mod drain_faults {
         pub(crate) rules_read_failures_left: usize,
         pub(crate) repo_read_attempts: usize,
         pub(crate) rules_read_attempts: usize,
-        pub(crate) reread_exhausted: bool,
     }
 
     fn table() -> &'static Mutex<HashMap<String, Counters>> {
@@ -1037,25 +1008,6 @@ pub(crate) mod drain_faults {
             .unwrap_or_default()
     }
 
-    /// Whether the drain re-read loop exhausted its retry budget for `repo_id`.
-    pub(crate) fn reread_exhausted(repo_id: &str) -> bool {
-        table()
-            .lock()
-            .unwrap()
-            .get(repo_id)
-            .map(|c| c.reread_exhausted)
-            .unwrap_or(false)
-    }
-
-    pub(crate) fn mark_reread_exhausted(repo_id: &str) {
-        table()
-            .lock()
-            .unwrap()
-            .entry(repo_id.to_string())
-            .or_default()
-            .reread_exhausted = true;
-    }
-
     /// Production-path hook: count one repo re-read attempt, return whether it must fail.
     pub(crate) fn take_repo_read(repo_id: &str) -> bool {
         let mut map = table().lock().unwrap();
@@ -1177,8 +1129,6 @@ async fn drain_refresh_state(ctx: &EncryptTaskCtx) -> DrainRefresh {
         "coalesced drain: re-read failed on every attempt; the coalesced push's \
          pin/encrypt pass is dropped (no reconciliation sweep re-derives it)"
     );
-    #[cfg(test)]
-    drain_faults::mark_reread_exhausted(&ctx.repo_id);
     DrainRefresh::Failed
 }
 
@@ -1566,12 +1516,11 @@ async fn pin_and_encrypt_objects(
 /// Map an `acquire_write` failure to the right `AppError`. An exhausted repo write-lock
 /// POOL is a capacity signal, not a broken repo, so it sheds 503 + Retry-After the same
 /// way the admission caps around it do; it used to fall into the generic git 500, which
-/// tells the client nothing about retrying (#173 F1). Lock contention and a refused
-/// under-lock refresh are transient and map to fixed-body 503s via [`RepoBusy`] and
-/// [`RepoUnavailable`]; only genuine untyped git failures stay on the 500 path.
+/// tells the client nothing about retrying (#173 F1). Anything else stays a git error.
 ///
-/// Shared with every `acquire_write` caller (`receive-pack`, `api/issues.rs`,
-/// `api/pulls.rs`) so the write-acquisition contract cannot drift between routes.
+/// Shared with the non-push `acquire_write` callers (`api/issues.rs`, `api/pulls.rs`)
+/// rather than copied: those hold no admission permit, so they meet an exhausted pool
+/// first, and a second copy of this mapping would be free to drift from the push path.
 pub(crate) fn acquire_write_app_error(err: &anyhow::Error, repo: &str) -> AppError {
     if err
         .downcast_ref::<crate::git::repo_store::LockPoolBusy>()
@@ -1579,16 +1528,6 @@ pub(crate) fn acquire_write_app_error(err: &anyhow::Error, repo: &str) -> AppErr
     {
         tracing::warn!(repo = %repo, err = %err, "write-lock pool exhausted; shedding with 503");
         AppError::Overloaded("git write locks at capacity, retry shortly".into())
-    } else if is_expected_transient_acquire_failure(err) {
-        tracing::warn!(repo = %repo, err = %err, "acquire_write failed");
-        if err
-            .downcast_ref::<crate::git::repo_store::RepoBusy>()
-            .is_some()
-        {
-            AppError::RepoBusy
-        } else {
-            AppError::RepoUnavailable
-        }
     } else {
         tracing::error!(repo = %repo, err = %err, "acquire_write failed");
         AppError::Git(err.to_string())
@@ -1599,23 +1538,6 @@ pub(crate) fn acquire_write_app_error(err: &anyhow::Error, repo: &str) -> AppErr
 /// [`smart_http::GitServiceTimeout`] to 504, a malformed client request to 400,
 /// anything else to a 500 git error. Pure (no logging) so it is unit-testable;
 /// callers add their own tracing.
-/// Acquire failures that are ordinary and transient: lock contention
-/// ([`RepoBusy`]) and an under-lock refresh that could not reach object storage
-/// ([`RepoUnavailable`]). Both already log at their raise site and both map to a
-/// retryable 503, so the handler layer logs them at warn rather than paging.
-/// Best-effort, like the database startup classifier: anything this cannot
-/// recognize counts as NOT transient and keeps its error-level log.
-///
-/// [`RepoBusy`]: crate::git::repo_store::RepoBusy
-/// [`RepoUnavailable`]: crate::git::repo_store::RepoUnavailable
-fn is_expected_transient_acquire_failure(err: &anyhow::Error) -> bool {
-    err.downcast_ref::<crate::git::repo_store::RepoBusy>()
-        .is_some()
-        || err
-            .downcast_ref::<crate::git::repo_store::RepoUnavailable>()
-            .is_some()
-}
-
 fn git_service_app_error(err: &anyhow::Error) -> AppError {
     if err
         .downcast_ref::<smart_http::GitServiceTimeout>()
@@ -2375,7 +2297,19 @@ pub async fn git_receive_pack(
     // would return 200 to the pusher before the durable copy lands, which is a larger
     // change to the client contract than the window it closes.
     let push_succeeded = receive_result.is_ok();
-    let publish_durability = PublishDurabilitySlot::new();
+    if push_succeeded {
+        tokio::spawn(post_receive_replication_tail(
+            state.clone(),
+            record.clone(),
+            ref_updates.clone(),
+            disk_path.clone(),
+            auth.0.to_string(),
+        ));
+    }
+
+    // Always release the advisory lock — even on error — to prevent stale locks
+    // from blocking subsequent pushes. Only upload to Tigris when the push
+    // succeeded; uploading a half-applied repo would propagate corruption.
     // Reclaim the write lock from the shared cell (#173 F2). This is only reachable
     // once `receive_pack` has returned, so the admission guard's copy can only ever
     // DELAY release, never perform it early; on the disconnect path this line is not
@@ -2385,27 +2319,7 @@ pub async fn git_receive_pack(
         .expect("repo write-lock mutex poisoned")
         .take()
         .expect("the write lock is only taken here, and only once");
-    if push_succeeded {
-        publish_durability.mark_release_started();
-        tokio::spawn(post_receive_replication_tail(
-            state.clone(),
-            record.clone(),
-            ref_updates.clone(),
-            disk_path.clone(),
-            auth.0.to_string(),
-            Some(publish_durability.arc()),
-        ));
-    }
-    // Short-circuit on a refused publish BEFORE anything downstream observes
-    // the push. The pack is on local disk but not in object storage, so
-    // touching the repo, recording the push, bumping trust, issuing ref
-    // certificates or answering 200 would all be reporting a write no other
-    // node can read.
-    let outcome = reclaimed.release(push_succeeded).await;
-    if push_succeeded {
-        publish_durability.record(outcome).await;
-    }
-    outcome.into_result()?;
+    reclaimed.release(push_succeeded).await;
     // Clean path: clone (a) already dropped inside run_git_service when the receive-pack
     // group was reaped; clone (b) held here spanned the success-only Tigris upload that
     // ran inside release() above. Drop it now so a second same-repo push proceeds the
@@ -2522,122 +2436,13 @@ pub async fn git_receive_pack(
 /// the per-repo-coalesced pin/encrypt task, and this push's own Pinata + announce
 /// task. Split out of `git_receive_pack` so the ordering the coalescing gate depends
 /// on is directly testable; the handler spawns it and returns.
-/// Records the release-side publish outcome for the detached post-receive tail.
-/// If the handler future is dropped after [`Self::mark_release_started`] but
-/// before [`Self::record`], the drop sets [`ReleaseOutcome::UploadUnknowable`]
-/// so the tail can proceed on local disk without waiting out the full transfer
-/// bound. A drop before release starts leaves the slot empty so the tail fails
-/// closed rather than treating an unattempted publish as unknowably durable.
-struct PublishDurabilitySlot {
-    inner: Arc<std::sync::Mutex<Option<crate::git::repo_store::ReleaseOutcome>>>,
-    recorded: std::sync::atomic::AtomicBool,
-    release_started: std::sync::atomic::AtomicBool,
-}
-
-impl PublishDurabilitySlot {
-    fn new() -> Self {
-        Self {
-            inner: Arc::new(std::sync::Mutex::new(None)),
-            recorded: std::sync::atomic::AtomicBool::new(false),
-            release_started: std::sync::atomic::AtomicBool::new(false),
-        }
-    }
-
-    fn mark_release_started(&self) {
-        self.release_started
-            .store(true, std::sync::atomic::Ordering::SeqCst);
-    }
-
-    fn arc(&self) -> Arc<std::sync::Mutex<Option<crate::git::repo_store::ReleaseOutcome>>> {
-        Arc::clone(&self.inner)
-    }
-
-    async fn record(&self, outcome: crate::git::repo_store::ReleaseOutcome) {
-        *self
-            .inner
-            .lock()
-            .expect("publish durability mutex poisoned") = Some(outcome);
-        self.recorded
-            .store(true, std::sync::atomic::Ordering::SeqCst);
-    }
-}
-
-impl Drop for PublishDurabilitySlot {
-    fn drop(&mut self) {
-        if self.recorded.load(std::sync::atomic::Ordering::SeqCst) {
-            return;
-        }
-        if !self
-            .release_started
-            .load(std::sync::atomic::Ordering::SeqCst)
-        {
-            return;
-        }
-        let mut slot = self
-            .inner
-            .lock()
-            .expect("publish durability mutex poisoned");
-        if slot.is_none() {
-            *slot = Some(crate::git::repo_store::ReleaseOutcome::UploadUnknowable);
-        }
-    }
-}
-
-async fn publish_durability_confirmed(
-    slot: &Option<Arc<std::sync::Mutex<Option<crate::git::repo_store::ReleaseOutcome>>>>,
-    wait: std::time::Duration,
-) -> bool {
-    let Some(slot) = slot else {
-        return true;
-    };
-    let start = std::time::Instant::now();
-    loop {
-        if let Some(outcome) = *slot.lock().expect("publish durability mutex poisoned") {
-            return matches!(
-                outcome,
-                crate::git::repo_store::ReleaseOutcome::Released
-                    | crate::git::repo_store::ReleaseOutcome::UploadUnknowable
-            );
-        }
-        if start.elapsed() >= wait {
-            // No outcome within the release-side transfer bound plus slack. Fail
-            // closed when the slot is still empty; an abandoned handler installs
-            // UploadUnknowable via PublishDurabilitySlot::drop so the tail can
-            // proceed on local disk without waiting out the full bound.
-            return matches!(
-                *slot.lock().expect("publish durability mutex poisoned"),
-                Some(crate::git::repo_store::ReleaseOutcome::Released)
-                    | Some(crate::git::repo_store::ReleaseOutcome::UploadUnknowable)
-            );
-        }
-        tokio::time::sleep(std::time::Duration::from_millis(10)).await;
-    }
-}
-
 async fn post_receive_replication_tail(
     state: AppState,
     record: RepoRecord,
     ref_updates: Vec<RefUpdate>,
     disk_path: std::path::PathBuf,
     did: String,
-    publish_durability: Option<
-        Arc<std::sync::Mutex<Option<crate::git::repo_store::ReleaseOutcome>>>,
-    >,
 ) {
-    let durability_wait = std::time::Duration::from_secs(
-        state
-            .config
-            .lock_held_transfer_timeout_secs
-            .saturating_add(5),
-    );
-    if !publish_durability_confirmed(&publish_durability, durability_wait).await {
-        tracing::warn!(
-            repo = %record.id,
-            "skipping post-receive tail: publish durability was not confirmed"
-        );
-        return;
-    }
-
     // Replication enforcement (Phase 2): decide once per push whether the public
     // may read this repo at all and, if so, which blob OIDs must not leave the
     // node. `withheld == None` means this push pins nothing (private / mode A /
@@ -3189,129 +2994,6 @@ pub async fn list_federated_repos(
 
 // ── Fork ──────────────────────────────────────────────────────────────────
 
-/// Removes a fork's local mirror clone on every exit until disarmed after the
-/// database row commits.
-struct ForkCloneGuard(Option<PathBuf>);
-
-const FORK_CREATE_CONFIRM_ATTEMPTS: u32 = 5;
-
-/// After `create_repo` fails, re-read the row with bounded retries so a
-/// transport-level error does not skip archive compensation when the insert never
-/// landed, and so a successful insert is not mistaken for a failure.
-async fn confirm_fork_repo_row(
-    db: &crate::db::Db,
-    owner_short: &str,
-    fork_name: &str,
-) -> anyhow::Result<Option<crate::db::RepoRecord>> {
-    let mut delay = std::time::Duration::from_millis(25);
-    for attempt in 0..FORK_CREATE_CONFIRM_ATTEMPTS {
-        match db.get_repo(owner_short, fork_name).await {
-            Ok(row) => return Ok(row),
-            Err(e) if attempt + 1 < FORK_CREATE_CONFIRM_ATTEMPTS => {
-                tracing::warn!(
-                    fork = %fork_name,
-                    attempt,
-                    err = %e,
-                    "fork create_repo confirmation lookup failed — retrying"
-                );
-                tokio::time::sleep(delay).await;
-                delay = delay
-                    .saturating_mul(2)
-                    .min(std::time::Duration::from_secs(1));
-            }
-            Err(e) => return Err(e),
-        }
-    }
-    unreachable!("loop returns on every attempt")
-}
-
-/// When the confirmation lookup stays unavailable after bounded retries, keep
-/// retrying in the background and compensate only after establishing that no row
-/// exists for this fork name.
-async fn schedule_fork_create_recovery(
-    db: std::sync::Arc<crate::db::Db>,
-    repo_store: crate::git::repo_store::RepoStore,
-    owner_short: String,
-    fork_name: String,
-    owner_did: String,
-    disk_path: std::path::PathBuf,
-) {
-    let mut delay = std::time::Duration::from_millis(250);
-    for attempt in 0..12 {
-        match db.get_repo(&owner_short, &fork_name).await {
-            Ok(Some(_)) => {
-                tracing::info!(
-                    fork = %fork_name,
-                    attempt,
-                    "fork create_repo recovery found a committed row — no archive compensation"
-                );
-                return;
-            }
-            Ok(None) => {
-                tracing::warn!(
-                    fork = %fork_name,
-                    attempt,
-                    "fork create_repo recovery confirmed no row — compensating orphan archive"
-                );
-                repo_store
-                    .compensate_fork_archive(&owner_did, &fork_name, disk_path.as_path())
-                    .await;
-                return;
-            }
-            Err(e) if attempt + 1 < 12 => {
-                tracing::warn!(
-                    fork = %fork_name,
-                    attempt,
-                    err = %e,
-                    "fork create_repo recovery lookup failed — retrying"
-                );
-                tokio::time::sleep(delay).await;
-                delay = delay
-                    .saturating_mul(2)
-                    .min(std::time::Duration::from_secs(30));
-            }
-            Err(e) => {
-                tracing::error!(
-                    fork = %fork_name,
-                    err = %e,
-                    "fork create_repo recovery gave up — orphan archive may remain until operator cleanup"
-                );
-                return;
-            }
-        }
-    }
-}
-
-impl ForkCloneGuard {
-    fn new(path: crate::git::repo_store::ValidatedRepoDiskPath) -> Self {
-        Self(Some(path.into_path_buf()))
-    }
-
-    fn path(&self) -> &std::path::Path {
-        self.0
-            .as_deref()
-            .expect("fork clone guard disarmed while still in use")
-    }
-
-    fn disarm(&mut self) {
-        self.0 = None;
-    }
-}
-
-impl Drop for ForkCloneGuard {
-    fn drop(&mut self) {
-        if let Some(path) = self.0.take() {
-            if let Err(e) = std::fs::remove_dir_all(&path) {
-                tracing::warn!(
-                    path = %path.display(),
-                    err = %e,
-                    "failed to remove fork clone during attempt cleanup"
-                );
-            }
-        }
-    }
-}
-
 #[derive(Debug, Deserialize)]
 pub struct ForkRepoRequest {
     pub name: Option<String>, // defaults to source repo name
@@ -3382,12 +3064,7 @@ pub async fn fork_repo(
         .await
         .map_err(|e| AppError::Git(e.to_string()))?;
 
-    let disk_path = crate::git::repo_store::validated_repo_disk_path(
-        &state.config.repos_dir,
-        &forker_did,
-        &fork_name,
-    )
-    .map_err(|e| AppError::BadRequest(e.to_string()))?;
+    let disk_path = store::repo_disk_path(&state.config.repos_dir, &forker_did, &fork_name);
 
     // Clone the source repo as a mirror
     let output = std::process::Command::new("git")
@@ -3407,30 +3084,11 @@ pub async fn fork_repo(
         )));
     }
 
-    let mut clone_guard = ForkCloneGuard::new(disk_path.clone());
-
-    // Upload fork to Tigris. Create-only: a refused precondition means an orphan
-    // archive already sits under this key (a failed create_repo or another
-    // writer), and proceeding would create a DB record whose archive is shadowed
-    // by bytes that are not this fork. Refuse rather than accept a fork other
-    // nodes would fetch as unrelated content. The clone guard removes the local
-    // mirror on every upload failure path.
+    // Upload fork to Tigris
     state
         .repo_store
         .release_after_write(&forker_did, &fork_name)
-        .await
-        .map_err(|e| match e {
-            crate::git::tigris::UploadError::PreconditionLost { status } => {
-                tracing::warn!(
-                    forker = %forker_did,
-                    fork = %fork_name,
-                    status,
-                    "fork refused: an archive already exists under the fork's key"
-                );
-                AppError::RepoExists(fork_name.clone())
-            }
-            other => AppError::Git(format!("fork upload failed: {other}")),
-        })?;
+        .await;
 
     let now = Utc::now();
     let record = crate::db::RepoRecord {
@@ -3442,64 +3100,12 @@ pub async fn fork_repo(
         default_branch: source.default_branch.clone(),
         created_at: now,
         updated_at: now,
-        disk_path: clone_guard.path().to_string_lossy().to_string(),
+        disk_path: disk_path.to_string_lossy().to_string(),
         forked_from: Some(source.id.clone()),
         machine_id: state.machine_id.clone(),
     };
 
-    if let Err(e) = state.db.create_repo(&record).await {
-        match confirm_fork_repo_row(&state.db, forker_short, &fork_name).await {
-            Ok(Some(committed)) => {
-                clone_guard.disarm();
-                tracing::warn!(
-                    fork = %fork_name,
-                    forker = %forker_did,
-                    "fork create_repo returned an error but the row is present — treating as success"
-                );
-                return Ok((
-                    StatusCode::CREATED,
-                    Json(to_response(&committed, &state, 0)),
-                ));
-            }
-            Ok(None) => {
-                clone_guard.disarm();
-                state
-                    .repo_store
-                    .compensate_fork_archive(&forker_did, &fork_name, disk_path.as_path())
-                    .await;
-                return Err(e.into());
-            }
-            Err(lookup_err) => {
-                clone_guard.disarm();
-                let db = std::sync::Arc::clone(&state.db);
-                let repo_store = state.repo_store.clone();
-                let owner_short = forker_short.to_string();
-                let fork_name_cl = fork_name.clone();
-                let owner_did = forker_did.clone();
-                let disk_path_cl = disk_path.as_path().to_path_buf();
-                tokio::spawn(async move {
-                    schedule_fork_create_recovery(
-                        db,
-                        repo_store,
-                        owner_short,
-                        fork_name_cl,
-                        owner_did,
-                        disk_path_cl,
-                    )
-                    .await;
-                });
-                tracing::warn!(
-                    fork = %fork_name,
-                    create_err = %e,
-                    lookup_err = %lookup_err,
-                    "fork create_repo failed and confirmation lookup stayed unavailable — scheduled recovery"
-                );
-                return Err(AppError::RepoUnavailable);
-            }
-        }
-    }
-
-    clone_guard.disarm();
+    state.db.create_repo(&record).await?;
 
     // Persist the proof so the fork carries it when it propagates to peers.
     if let Some(p) = verified_proof {
@@ -3737,129 +3343,6 @@ mod tests {
     const OWNER_SHORT: &str = "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH";
     const STRANGER_DID: &str = "did:key:z6Mkffonly5tranger0000000000000000000000000000000";
 
-    #[tokio::test]
-    async fn publish_durability_slot_drop_installs_unknowable_when_release_started_but_unrecorded()
-    {
-        let slot = PublishDurabilitySlot::new();
-        slot.mark_release_started();
-        let arc = slot.arc();
-        drop(slot);
-        let outcome = *arc.lock().expect("publish durability mutex poisoned");
-        assert_eq!(
-            outcome,
-            Some(crate::git::repo_store::ReleaseOutcome::UploadUnknowable),
-            "dropping an unrecorded slot after release starts must publish UploadUnknowable for the tail"
-        );
-    }
-
-    #[test]
-    fn publish_durability_slot_drop_leaves_empty_before_release_starts() {
-        let slot = PublishDurabilitySlot::new();
-        let arc = slot.arc();
-        drop(slot);
-        assert_eq!(
-            *arc.lock().expect("publish durability mutex poisoned"),
-            None,
-            "dropping before release starts must not synthesize durability for the tail"
-        );
-    }
-
-    #[test]
-    fn publish_durability_slot_drop_waits_for_contended_mutex() {
-        let slot = PublishDurabilitySlot::new();
-        slot.mark_release_started();
-        let arc = slot.arc();
-        let holder = {
-            let arc = Arc::clone(&arc);
-            std::thread::spawn(move || {
-                let _guard = arc.lock().expect("publish durability mutex poisoned");
-                std::thread::sleep(std::time::Duration::from_millis(100));
-            })
-        };
-        std::thread::sleep(std::time::Duration::from_millis(10));
-        drop(slot);
-        holder.join().expect("mutex holder thread");
-        let outcome = *arc.lock().expect("publish durability mutex poisoned");
-        assert_eq!(
-            outcome,
-            Some(crate::git::repo_store::ReleaseOutcome::UploadUnknowable),
-            "drop must block until it can install UploadUnknowable, not give up on try_lock"
-        );
-    }
-
-    #[tokio::test]
-    async fn publish_durability_confirmed_proceeds_quickly_after_unrecorded_slot_drop() {
-        let start = std::time::Instant::now();
-        let slot = PublishDurabilitySlot::new();
-        slot.mark_release_started();
-        let arc = slot.arc();
-        drop(slot);
-        let confirmed =
-            publish_durability_confirmed(&Some(arc), std::time::Duration::from_millis(50)).await;
-        assert!(confirmed);
-        assert!(
-            start.elapsed() < std::time::Duration::from_millis(200),
-            "the tail must not wait out the full transfer bound when the slot already carries UploadUnknowable"
-        );
-    }
-
-    #[tokio::test]
-    async fn publish_durability_confirmed_fails_closed_when_release_never_records() {
-        let slot = Arc::new(std::sync::Mutex::new(None));
-        let confirmed =
-            publish_durability_confirmed(&Some(slot), std::time::Duration::from_millis(30)).await;
-        assert!(
-            !confirmed,
-            "an empty slot after the bounded wait must not admit the tail; only an installed \
-             Released or UploadUnknowable outcome may"
-        );
-    }
-
-    #[tokio::test]
-    async fn publish_durability_confirmed_accepts_only_released() {
-        let slot = Arc::new(std::sync::Mutex::new(Some(
-            crate::git::repo_store::ReleaseOutcome::Released,
-        )));
-        assert!(
-            publish_durability_confirmed(&Some(slot), std::time::Duration::from_millis(5)).await
-        );
-
-        let slot = Arc::new(std::sync::Mutex::new(Some(
-            crate::git::repo_store::ReleaseOutcome::UploadUnknowable,
-        )));
-        assert!(
-            publish_durability_confirmed(&Some(slot), std::time::Duration::from_millis(5)).await,
-            "an unknowable upload still landed on local disk, so the tail may proceed"
-        );
-
-        let slot = Arc::new(std::sync::Mutex::new(Some(
-            crate::git::repo_store::ReleaseOutcome::UploadFailed,
-        )));
-        assert!(
-            !publish_durability_confirmed(&Some(slot), std::time::Duration::from_millis(5)).await
-        );
-    }
-
-    #[test]
-    fn fork_clone_guard_removes_mirror_on_drop() {
-        let root = tempfile::TempDir::new().unwrap();
-        let validated = crate::git::repo_store::validated_repo_disk_path(
-            root.path(),
-            "did:key:testfork",
-            "fork",
-        )
-        .expect("test fork path must validate");
-        std::fs::create_dir_all(validated.as_path()).unwrap();
-        {
-            let _guard = ForkCloneGuard::new(validated.clone());
-            assert!(validated.exists());
-        }
-        assert!(
-            !validated.exists(),
-            "dropping the fork clone guard must remove the mirror directory"
-        );
-    }
-
     #[test]
     fn upload_pack_request_finalizes_only_with_done_pktline() {
         let want = "0032want 1111111111111111111111111111111111111111\n";
@@ -4042,55 +3525,6 @@ mod tests {
         assert!(git_permit(&sem).is_ok());
     }
 
-    #[test]
-    fn is_expected_transient_matches_both_typed_refusals() {
-        // The real raise shape wraps the marker in a `.context()` layer naming the
-        // owner slug and repo, so the downcast has to survive that wrapping.
-        let busy = anyhow::Error::new(crate::git::repo_store::RepoBusy)
-            .context("another write is in progress for alice/demo");
-        assert!(is_expected_transient_acquire_failure(&busy));
-
-        let unavailable = anyhow::Error::new(crate::git::repo_store::RepoUnavailable)
-            .context("could not read the archive HEAD for alice/demo");
-        assert!(is_expected_transient_acquire_failure(&unavailable));
-    }
-
-    #[test]
-    fn is_expected_transient_rejects_unrelated_failures() {
-        // Anything the classifier cannot recognize keeps paging at error level.
-        let other = anyhow::anyhow!("disk on fire");
-        assert!(!is_expected_transient_acquire_failure(&other));
-    }
-
-    #[test]
-    fn acquire_write_app_error_maps_transient_markers_to_retryable_503() {
-        let busy = anyhow::Error::new(crate::git::repo_store::RepoBusy)
-            .context("another write is in progress for alice/demo");
-        assert!(matches!(
-            acquire_write_app_error(&busy, "demo"),
-            AppError::RepoBusy
-        ));
-
-        let unavailable = anyhow::Error::new(crate::git::repo_store::RepoUnavailable)
-            .context("could not read the archive HEAD for alice/demo");
-        assert!(matches!(
-            acquire_write_app_error(&unavailable, "demo"),
-            AppError::RepoUnavailable
-        ));
-
-        let pool = anyhow::Error::new(crate::git::repo_store::LockPoolBusy);
-        assert!(matches!(
-            acquire_write_app_error(&pool, "demo"),
-            AppError::Overloaded(_)
-        ));
-
-        let other = anyhow::anyhow!("disk on fire");
-        assert!(matches!(
-            acquire_write_app_error(&other, "demo"),
-            AppError::Git(_)
-        ));
-    }
-
     fn repo_owned_by(owner_did: &str) -> crate::db::RepoRecord {
         let now = chrono::Utc::now();
         crate::db::RepoRecord {
@@ -6821,12 +6255,14 @@ mod tests {
             repos_dir.path().to_path_buf(),
             None,
             crate::git::repo_store::build_lock_pool(&pool, 4, std::time::Duration::from_secs(5)),
-            std::time::Duration::from_secs(300),
         );
         let mut cfg = (*state.config).clone();
         // Long enough that the git-service timeout is never what ends this push; the
         // disconnect is.
         cfg.git_service_timeout_secs = 600;
+        // Lock-path test, not owner-push authorization: pin off so the default-on gate
+        // does not reject the proof DID before receive-pack runs.
+        cfg.enforce_owner_push = false;
         state.config = std::sync::Arc::new(cfg);
         state
             .db
@@ -6869,7 +6305,9 @@ mod tests {
                 let mut fut = Box::pin(git_receive_pack(
                     State(state.clone()),
                     Path((owner.to_string(), name.to_string())),
-                    Extension(crate::auth::AuthenticatedDid("did:key:z6disc".to_string())),
+                    Extension(crate::auth::AuthenticatedDid(
+                        "did:key:z6MkDisconnectWriteLockProofDidAAAAAAAA".to_string(),
+                    )),
                     crate::rate_limit::PeerAddr(Some(
                         "203.0.113.81:5000".parse::<SocketAddr>().unwrap(),
                     )),
@@ -6994,8 +6432,10 @@ mod tests {
             repos_dir.path().to_path_buf(),
             None,
             crate::git::repo_store::build_lock_pool(&pool, 4, std::time::Duration::from_secs(5)),
-            std::time::Duration::from_secs(300),
         );
+        let mut cfg = (*state.config).clone();
+        cfg.enforce_owner_push = false;
+        state.config = std::sync::Arc::new(cfg);
         state
             .db
             .upsert_mirror_repo(owner, name, "/tmp/z6succ-sc1", None, false)
@@ -7019,7 +6459,9 @@ mod tests {
                 git_receive_pack(
                     State(state.clone()),
                     Path((owner.to_string(), name.to_string())),
-                    Extension(crate::auth::AuthenticatedDid("did:key:z6succ".to_string())),
+                    Extension(crate::auth::AuthenticatedDid(
+                        "did:key:z6MkPushSuccessReleaseProofDidAAAAAAAA".to_string(),
+                    )),
                     crate::rate_limit::PeerAddr(Some(
                         "203.0.113.83:5000".parse::<SocketAddr>().unwrap(),
                     )),
@@ -7080,13 +6522,15 @@ mod tests {
         let owner = "z6lockpool";
         let name = "lp1";
         let mut state = crate::test_support::test_state(pool.clone()).await;
+        let mut cfg = (*state.config).clone();
+        cfg.enforce_owner_push = false;
+        state.config = std::sync::Arc::new(cfg);
         // One lock-pool connection, short checkout timeout so the exhaustion surfaces
         // promptly rather than at the handler's own acquire deadline.
         state.repo_store = crate::git::repo_store::RepoStore::new(
             std::path::PathBuf::from("/tmp/gitlawb-lockpool-shed"),
             None,
             crate::git::repo_store::build_lock_pool(&pool, 1, std::time::Duration::from_secs(1)),
-            std::time::Duration::from_secs(300),
         );
         state
             .db
@@ -7094,7 +6538,7 @@ mod tests {
             .await
             .unwrap();
 
-        let did = "did:key:z6lockpool";
+        let did = "did:key:z6MkLockPoolShedProofDidAAAAAAAAAAAAAAAAAA";
         let peer: SocketAddr = "203.0.113.71:5000".parse().unwrap();
 
         // Occupy the only lock-pool connection with a write on an UNRELATED repo.
@@ -7121,7 +6565,7 @@ mod tests {
 
         // MUST-NOT: with the pool free again, the push is not shed as capacity (it fails
         // later on the nonexistent on-disk repo, which is a git error, not Overloaded).
-        let _ = held.release(false).await;
+        held.release(false).await;
         let admitted = git_receive_pack(
             State(state.clone()),
             Path((owner.to_string(), name.to_string())),
@@ -10196,43 +9640,6 @@ mod tests {
 
     const F2A_PUSHER: &str = "did:key:z6MkF2aPusherAAAAAAAAAAAAAAAAAAAAAAAAAA";
 
-    /// When publish durability is fenced, the tail must not run walks or take
-    /// coalescing keys before returning.
-    #[cfg(unix)]
-    #[sqlx::test]
-    async fn post_receive_tail_skips_all_work_when_publish_fenced(pool: sqlx::PgPool) {
-        let repo = tempfile::TempDir::new().unwrap();
-        let bin = tempfile::TempDir::new().unwrap();
-        let log = bin.path().join("git.log");
-        let git_bin = f2a_logging_git(bin.path(), &log);
-        u5_init_repo(repo.path());
-        let c1 = u5_commit_file(repo.path(), "a.txt", "one\n");
-        let (state, rec) = f2a_state(pool, &git_bin, "z6f2afence", "fence-repo", false).await;
-        let slot = Arc::new(std::sync::Mutex::new(Some(
-            crate::git::repo_store::ReleaseOutcome::Fenced,
-        )));
-        post_receive_replication_tail(
-            state.clone(),
-            rec,
-            f2a_update("refs/heads/main", &c1),
-            repo.path().to_path_buf(),
-            F2A_PUSHER.to_string(),
-            Some(slot),
-        )
-        .await;
-        assert_eq!(
-            f2a_walks(&log),
-            0,
-            "a fenced publish must not run the replication walk; log:\n{}",
-            f2a_log(&log)
-        );
-        assert_eq!(
-            state.encrypt_inflight.len(),
-            0,
-            "a fenced publish must not take the per-repo coalescing key"
-        );
-    }
-
     /// Scenario 1 (the finding). A second rapid push to the same repo coalesces
     /// WITHOUT running the withheld walk. Asserted on the walk's git children, not
     /// on the `Coalesced` outcome: with `try_begin` below the walk (the pre-fix
@@ -10262,7 +9669,6 @@ mod tests {
             f2a_update("refs/heads/main", &c2),
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
         let after_first = f2a_walks(&log);
@@ -10283,7 +9689,6 @@ mod tests {
             f2a_update("refs/heads/second", &c1),
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
 
@@ -10357,7 +9762,6 @@ mod tests {
             f2a_update("refs/heads/main", &c2),
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
 
@@ -10439,7 +9843,6 @@ mod tests {
             f2a_update("refs/heads/main", &c2),
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         ));
         f2a_wait_for(|| started.exists(), "the admitted push's walk to start").await;
 
@@ -10553,7 +9956,6 @@ mod tests {
             }],
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
 
@@ -10832,7 +10234,6 @@ mod tests {
             }],
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
 
@@ -10888,7 +10289,6 @@ mod tests {
             f2a_update("refs/heads/main", &c1),
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
         tokio::time::sleep(std::time::Duration::from_millis(300)).await;
@@ -10930,7 +10330,6 @@ mod tests {
             }],
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
 
@@ -11028,7 +10427,6 @@ mod tests {
             }],
             repo.path().to_path_buf(),
             F2A_PUSHER.to_string(),
-            None,
         )
         .await;
 

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

@@ -517,7 +517,6 @@ mod tests {
             rate_limiter: RateLimiter::new(100, Duration::from_secs(60)),
             create_ip_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)),
             push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
-            close_issue_rate_limiter: RateLimiter::new(120, Duration::from_secs(3600)),
             ipfs_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
             ipfs_work_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
             ipfs_max_history_walks: crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST,

crates/gitlawb-node/src/config.rs

@@ -205,8 +205,10 @@ pub struct Config {
     pub metrics_addr: String,
 
     /// Maximum time to wait for in-flight requests to drain on shutdown, in
-    /// seconds. After this elapses, the server returns 503 to anything still
-    /// in flight and exits. Default: 30s.
+    /// seconds. On the shutdown signal the server stops accepting new
+    /// connections and gives in-flight requests this long to complete; at
+    /// expiry it abandons whatever remains (clients observe a closed
+    /// connection, not an error response) and exits. Default: 30s.
     #[arg(long, env = "GITLAWB_SHUTDOWN_GRACE_SECS", default_value_t = 30)]
     pub shutdown_grace_secs: u64,
 
@@ -252,9 +254,11 @@ pub struct Config {
 
     /// Maximum connections in the PostgreSQL pool. This is a cap, not a floor
     /// (connections open lazily). Size against the database server's
-    /// max_connections, remembering admin tooling opens its own pool. Writes pin
-    /// the dedicated lock pool, not this one; this pool still needs enough headroom
-    /// for ordinary request handlers and metadata paths (`DB_POOL_APP_HEADROOM`).
+    /// max_connections, remembering admin tooling opens its own pool. Each
+    /// concurrent write pins one pooled connection for its whole duration (the
+    /// advisory lock in `repo_store::acquire_write` is connection-affine), so this
+    /// must exceed `max_concurrent_git_pushes` by `DB_POOL_APP_HEADROOM` or slow
+    /// pushes starve every other DB path — enforced by `Config::validate`.
     #[arg(
         long,
         env = "GITLAWB_DB_MAX_CONNECTIONS",
@@ -263,46 +267,6 @@ pub struct Config {
     )]
     pub db_max_connections: u32,
 
-    /// Maximum connections in the dedicated advisory-lock pool, which is separate
-    /// from the main pool above.
-    ///
-    /// Size this against the expected peak number of concurrent distinct-repo
-    /// writers, NOT small. Every in-flight repo write pins one connection here for
-    /// its whole duration (the write, its metadata tail, and the bounded archive
-    /// upload), so this value is a hard ceiling on simultaneous writes node-wide.
-    /// Keeping it separate from GITLAWB_DB_MAX_CONNECTIONS is what stops a push
-    /// burst from starving ordinary request handlers; the cost is that
-    /// (main pool + lock pool) must fit inside the database server's
-    /// max_connections, times the number of nodes, plus admin tooling.
-    #[arg(
-        long,
-        env = "GITLAWB_DB_LOCK_POOL_MAX_CONNECTIONS",
-        default_value_t = 40,
-        value_parser = clap::value_parser!(u32).range(1..)
-    )]
-    pub db_lock_pool_max_connections: u32,
-
-    /// Upper bound, in seconds, on any single object-storage transfer that runs
-    /// while the per-repo advisory lock is HELD.
-    ///
-    /// Two bounded spans exist, and the bound applies per span. The acquire-side
-    /// refresh in `acquire_write` covers the existence HEAD and the download
-    /// together under ONE budget (it runs after the lock is taken and before the
-    /// guard is constructed), and the archive upload in `release` gets its own.
-    /// Worst-case slot occupancy is therefore about twice this value plus the git
-    /// work between them, not one times this value. Both used to be free, because the lock's
-    /// connection was returned to the pool immediately; now that a write guard
-    /// pins a lock-pool connection for its whole lifetime, an unbounded transfer
-    /// holds that slot, and enough stalled transfers deny every write on the node.
-    /// This is the bound that keeps a stall from becoming an outage.
-    #[arg(
-        long,
-        env = "GITLAWB_LOCK_HELD_TRANSFER_TIMEOUT_SECS",
-        default_value_t = 300,
-        value_parser = clap::value_parser!(u64).range(1..)
-    )]
-    pub lock_held_transfer_timeout_secs: u64,
-
     /// Maximum time a request waits for a pool connection before failing with
     /// 503, in seconds. Bounds queueing when the database is slow or down.
     #[arg(
@@ -397,14 +361,15 @@ pub struct Config {
     /// error rather than a boot-time panic).
     ///
     /// CONNECTION BUDGET. A push holds a Postgres connection from the node's separate
-    /// advisory-lock pool (`GITLAWB_DB_LOCK_POOL_MAX_CONNECTIONS`, default 40) for
-    /// the whole receive-pack. That pool must be at least this value plus
-    /// `DB_LOCK_POOL_NON_PUSH_HEADROOM` so admitted pushes and non-push mutations
-    /// can each pin a lock-pool connection for their whole duration. Size BOTH
-    /// pools against the database server's `max_connections`: the main pool
-    /// (`GITLAWB_DB_MAX_CONNECTIONS`, default 48) serves ordinary handlers, and the
-    /// lock pool serves writes. Raising this knob does not raise the lock pool;
-    /// set `GITLAWB_DB_LOCK_POOL_MAX_CONNECTIONS` explicitly.
+    /// advisory-lock pool for the whole receive-pack, and that pool is sized from this
+    /// knob (this value + 8, clamped to 64 in `main.rs`). The node's total ceiling is
+    /// therefore `db_max_connections` (default 48) + the lock pool (default 40), i.e.
+    /// 88 by default, and at most `db_max_connections` + 64. Size BOTH against the
+    /// database server's `max_connections`: `db_max_connections`' own doc predates the
+    /// lock pool and no longer covers most of the node's connections. The +8 headroom
+    /// is shared with the three non-push `acquire_write` callers (`api/issues.rs` x2,
+    /// `api/pulls.rs`). Raising this knob past the clamp does NOT buy more lock-pool
+    /// connections; pushes beyond it wait briefly and then shed a 503 + Retry-After.
     #[arg(
         long,
         env = "GITLAWB_MAX_CONCURRENT_GIT_PUSHES",
@@ -762,37 +727,25 @@ impl Config {
     /// the pool must clear the concurrent-write cap by at least this margin.
     pub const DB_POOL_APP_HEADROOM: u32 = 8;
 
-    /// Lock-pool slots reserved for non-push `acquire_write` callers (issue create,
-    /// close, merge) so a saturated push budget cannot turn ordinary mutations into
-    /// lock-pool exhaustion on unrelated repositories.
-    pub const DB_LOCK_POOL_NON_PUSH_HEADROOM: u32 = 8;
-
     /// Cross-field boot validation. Single-field ranges are enforced by clap; this
     /// catches combinations that ship a denial-of-service under otherwise-valid
     /// values. Call once at startup and fail fast on `Err`.
     pub fn validate(&self) -> Result<(), String> {
-        // Concurrent git writes pin the dedicated lock pool for their whole
-        // duration (connection-affine advisory lock in acquire_write). The main
-        // pool no longer carries that occupancy.
-        let lock_floor = self
-            .max_concurrent_git_pushes
-            .saturating_add(Self::DB_LOCK_POOL_NON_PUSH_HEADROOM as usize);
-        if (self.db_lock_pool_max_connections as usize) < lock_floor {
+        // A write pins one pooled connection for its whole duration (the
+        // connection-affine advisory lock in repo_store::acquire_write), and
+        // concurrent writes are capped at max_concurrent_git_pushes. If the pool
+        // does not exceed that cap by DB_POOL_APP_HEADROOM, a burst of slow pushes
+        // drains every connection and every other DB path 503s. (#174 F1)
+        let floor = (self.max_concurrent_git_pushes as u64) + (Self::DB_POOL_APP_HEADROOM as u64);
+        if (self.db_max_connections as u64) < floor {
             return Err(format!(
-                "GITLAWB_DB_LOCK_POOL_MAX_CONNECTIONS ({}) must be at least \
-                 max_concurrent_git_pushes ({}) plus {} for non-push mutations \
-                 that share the lock pool",
-                self.db_lock_pool_max_connections,
+                "GITLAWB_DB_MAX_CONNECTIONS ({}) must be at least max_concurrent_git_pushes ({}) \
+                 + {} headroom = {}: each concurrent write pins one pooled connection for its whole \
+                 duration, so a smaller pool lets a burst of slow pushes starve every other DB path.",
+                self.db_max_connections,
                 self.max_concurrent_git_pushes,
-                Self::DB_LOCK_POOL_NON_PUSH_HEADROOM
-            ));
-        }
-        let main_floor = Self::DB_POOL_APP_HEADROOM as u64;
-        if (self.db_max_connections as u64) < main_floor {
-            return Err(format!(
-                "GITLAWB_DB_MAX_CONNECTIONS ({}) must be at least {} for non-git \
-                 database paths",
-                self.db_max_connections, main_floor
+                Self::DB_POOL_APP_HEADROOM,
+                floor
             ));
         }
         Ok(())
@@ -803,25 +756,6 @@ impl Config {
 mod tests {
     use super::*;
 
-    #[test]
-    fn lock_pool_size_defaults_to_40_and_rejects_zero() {
-        assert_eq!(
-            Config::parse_from(["gitlawb-node"]).db_lock_pool_max_connections,
-            40
-        );
-        assert_eq!(
-            Config::parse_from(["gitlawb-node", "--db-lock-pool-max-connections", "8"])
-                .db_lock_pool_max_connections,
-            8
-        );
-        // A zero-sized lock pool would deny every write, so clap must reject it
-        // rather than let a node boot into a state where no repo can be written.
-        assert!(
-            Config::try_parse_from(["gitlawb-node", "--db-lock-pool-max-connections", "0"])
-                .is_err()
-        );
-    }
-
     #[test]
     fn git_service_timeout_defaults_to_600_and_rejects_zero() {
         assert_eq!(
@@ -1431,61 +1365,41 @@ mod tests {
         );
     }
 
-    /// #174 F1: concurrent git writes pin the dedicated lock pool, not the main
-    /// pool. `validate()` must reject an under-sized lock pool at boot.
+    /// #174 F1: a connection-affine write lock pins a pooled connection per
+    /// concurrent write, so the pool must clear `max_concurrent_git_pushes` by
+    /// `DB_POOL_APP_HEADROOM` or a push burst starves every other DB path.
+    /// `validate()` must reject an under-sized pool at boot.
     #[test]
     fn db_pool_must_clear_the_git_push_cap() {
-        // Shipped defaults validate (lock pool 40 >= pushes 32 + non-push headroom 8).
+        // Shipped defaults validate (48 >= 32 + 8).
         Config::parse_from(["gitlawb-node"])
             .validate()
             .expect("default config must validate");
 
-        // An under-sized lock pool relative to the push cap is rejected.
-        let under_lock = Config::parse_from([
+        // An under-sized pool relative to the push cap is rejected (20 < 32 + 8).
+        let under = Config::parse_from([
             "gitlawb-node",
-            "--db-lock-pool-max-connections",
-            "16",
+            "--db-max-connections",
+            "20",
             "--max-concurrent-git-pushes",
             "32",
         ]);
         assert!(
-            under_lock.validate().is_err(),
-            "db_lock_pool_max_connections below max_concurrent_git_pushes + headroom must be rejected"
+            under.validate().is_err(),
+            "db_max_connections 20 below max_concurrent_git_pushes 32 + headroom must be rejected"
         );
 
-        // Exactly at the push cap without non-push headroom is rejected.
-        let push_only = Config::parse_from([
-            "gitlawb-node",
-            "--db-lock-pool-max-connections",
-            "32",
-            "--max-concurrent-git-pushes",
-            "32",
-        ]);
-        assert!(
-            push_only.validate().is_err(),
-            "db_lock_pool_max_connections equal to max_concurrent_git_pushes must be rejected"
-        );
-
-        // Main pool can be smaller than pushes + headroom when the lock pool carries writes.
-        let split = Config::parse_from([
+        // Exactly at the floor validates (40 == 32 + 8).
+        let at_floor = Config::parse_from([
             "gitlawb-node",
             "--db-max-connections",
-            "16",
-            "--db-lock-pool-max-connections",
             "40",
             "--max-concurrent-git-pushes",
             "32",
         ]);
         assert!(
-            split.validate().is_ok(),
-            "a small main pool with a large lock pool must validate"
-        );
-
-        // Main pool still needs the app headroom floor.
-        let under_main = Config::parse_from(["gitlawb-node", "--db-max-connections", "4"]);
-        assert!(
-            under_main.validate().is_err(),
-            "db_max_connections below DB_POOL_APP_HEADROOM must be rejected"
+            at_floor.validate().is_ok(),
+            "db_max_connections at the floor (pushes + headroom) must validate"
         );
     }
 

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

@@ -265,7 +265,12 @@ pub struct Db {
 }
 
 impl Db {
-    #[cfg(test)]
+    /// Access the underlying Postgres connection pool.
+    pub fn pool(&self) -> &PgPool {
+        &self.pool
+    }
+
+    #[cfg(any(test, feature = "test-harness"))]
     pub fn for_testing(pool: PgPool) -> Self {
         Self { pool }
     }
@@ -274,7 +279,7 @@ impl Db {
     /// provisions an empty per-test database, so DB-backed tests must run this
     /// before seeding. Reuses the production `migrate()` path (the advisory lock
     /// is harmless on an isolated test DB and migrations are idempotent).
-    #[cfg(test)]
+    #[cfg(any(test, feature = "test-harness"))]
     pub(crate) async fn run_migrations(&self) -> Result<()> {
         self.migrate().await
     }
@@ -305,46 +310,6 @@ impl Db {
         Ok(db)
     }
 
-    /// Build the dedicated pool that advisory-lock connections come from.
-    ///
-    /// Deliberately **lazy**: connections open on first use rather than at boot.
-    /// The main pool has to connect eagerly because it runs migrations, which is
-    /// why it needs `connect_db_with_retry`'s backoff and degraded-server
-    /// handoff. This pool has no startup work at all, so an eager connect would
-    /// only add a new way for the process to fail to boot, and would need a
-    /// second copy of that retry machinery to be safe. Being lazy removes the
-    /// failure mode instead of handling it: if Postgres is unreachable when the
-    /// first write arrives, that write fails on the pool's own acquire timeout,
-    /// the same way any other database-backed request already does.
-    ///
-    /// Kept separate from the main pool so a burst of lock-holding connections
-    /// cannot starve ordinary request handlers. See
-    /// `GITLAWB_DB_LOCK_POOL_MAX_CONNECTIONS` for the sizing tradeoff.
-    pub fn lock_pool(
-        database_url: &str,
-        max_connections: u32,
-        acquire_timeout: Duration,
-    ) -> Result<PgPool> {
-        info!(
-            max_connections,
-            acquire_timeout_secs = acquire_timeout.as_secs(),
-            "creating dedicated advisory-lock pool (lazy)"
-        );
-        PgPoolOptions::new()
-            .max_connections(max_connections)
-            // Explicit, and load-bearing rather than cosmetic: `RepoWriteGuard::Drop`
-            // has a no-runtime branch that calls `PoolConnection::leak()` and relies
-            // on the husk's own drop doing nothing. With `min_connections > 0` that
-            // drop still spawns a pool-replenish task, and spawning without a runtime
-            // panics inside Drop, which aborts the process during unwind. It is 0 by
-            // default, so this line exists to keep a future tuning change from
-            // silently re-arming that panic.
-            .min_connections(0)
-            .acquire_timeout(acquire_timeout)
-            .connect_lazy(database_url)
-            .context("creating advisory-lock pool")
-    }
-
     /// Cheap liveness probe against the pool, for readiness checks: one
     /// `SELECT 1` that fails fast when the database is unreachable.
     pub async fn ping(&self) -> Result<()> {
@@ -8313,8 +8278,8 @@ mod peer_authority_tests {
 /// | `prune_non_public_peers` (db/mod.rs) | a delete keyed on a computed bad-DID array; cannot repoint; boot-only caller in main.rs |
 /// | `seed_local_peer` (sync.rs) | excluded by test-module location: a deliberate `upsert_peer` bypass for `file://` fixtures, which the public-URL gate rejects |
 /// | `a_legacy_row_can_still_refresh_its_liveness` (db/mod.rs) | test-only. Seeds a PRE-GATE row by raw SQL on purpose: `upsert_peer` cannot create one, since the gate it is testing refuses exactly that DID. The fixture models what a deployed table already holds |
-/// | `gossip_ping_round_requires_two_failures_before_persisting_unreachable` (main.rs) | test-only fixture seed. Raw SQL because the test drives the readiness HYSTERESIS, which needs a row already at `last_ping_ok = TRUE` before the round runs; it never exercises the announce gate |
 /// | `manual_ping_uses_readiness_without_mutating_federation_gate` (api/peers.rs) | test-only fixture seed, same shape and same reason: the row under test must pre-exist so the assertion is about what the ping does NOT rewrite |
+/// | `gossip_ping_round_requires_two_failures_before_persisting_unreachable` (lib.rs) | test-only fixture seed for the gossip hysteresis round; the row must pre-exist so the assertion is about when `mark_peer_ping` fires |
 ///
 /// And the `upsert_peer` CALL-SITE authority table, which the ledger above
 /// structurally cannot hold, because the bootstrap site issues no SQL of its own
@@ -8401,11 +8366,11 @@ mod peers_table_writer_guard {
     const LEDGER: &[(&str, usize)] = &[
         ("a_legacy_row_can_still_refresh_its_liveness", 1),
         (
-            "gossip_ping_round_requires_two_failures_before_persisting_unreachable",
+            "manual_ping_uses_readiness_without_mutating_federation_gate",
             1,
         ),
         (
-            "manual_ping_uses_readiness_without_mutating_federation_gate",
+            "gossip_ping_round_requires_two_failures_before_persisting_unreachable",
             1,
         ),
         ("mark_peer_ping", 1),

crates/gitlawb-node/src/error.rs

@@ -69,15 +69,6 @@ pub enum AppError {
     #[error("server overloaded: {0}")]
     Overloaded(String),
 
-    #[error("repository is busy")]
-    RepoBusy,
-
-    #[error("repository is temporarily unavailable")]
-    RepoUnavailable,
-
-    #[error("repository write was fenced by a concurrent publish")]
-    RepoWriteFenced,
-
     #[error("database error: {0}")]
     Db(#[from] sqlx::Error),
 
@@ -119,31 +110,7 @@ impl From<anyhow::Error> for AppError {
     fn from(err: anyhow::Error) -> Self {
         match err.downcast::<sqlx::Error>() {
             Ok(sql) => AppError::Db(sql),
-            // Lock contention is transient and ordinary, so it must not land as a
-            // 500. The internal message names the owner slug and repo, so the
-            // variant carries nothing: the detail stays in the log at the raise
-            // site and the client gets a fixed retryable body.
-            Err(err) => match err.downcast::<crate::git::repo_store::LockPoolBusy>() {
-                Ok(_) => AppError::Overloaded("git write locks at capacity, retry shortly".into()),
-                Err(err) => match err.downcast::<crate::git::repo_store::RepoBusy>() {
-                    Ok(_) => AppError::RepoBusy,
-                    // Same reasoning one rung down: a refused under-lock refresh is a
-                    // transient storage condition, and its internal message names the
-                    // owner slug and repo, so the variant carries nothing.
-                    Err(err) => match err.downcast::<crate::git::repo_store::RepoUnavailable>() {
-                        Ok(_) => AppError::RepoUnavailable,
-                        // And one more rung: a publish the store refused twice is
-                        // transient in the same way, and the retry is the client's
-                        // to make. The variant carries nothing for the same reason
-                        // as the two above.
-                        Err(err) => match err.downcast::<crate::git::repo_store::RepoWriteFenced>()
-                        {
-                            Ok(_) => AppError::RepoWriteFenced,
-                            Err(err) => AppError::Internal(err),
-                        },
-                    },
-                },
-            },
+            Err(err) => AppError::Internal(err),
         }
     }
 }
@@ -217,29 +184,6 @@ impl IntoResponse for AppError {
             // 504, distinct from the 500 git_error and from the read-gate's 404 /
             // the auth 401, so the client can tell a deadline from a failure.
             AppError::Timeout(msg) => (StatusCode::GATEWAY_TIMEOUT, "git_timeout", msg.clone()),
-            // 503 with a FIXED body: the caller should retry, and must not be told
-            // which repo is contended or for how long.
-            AppError::RepoBusy => (
-                StatusCode::SERVICE_UNAVAILABLE,
-                "repo_busy",
-                "repository is busy — retry".into(),
-            ),
-            // 503 with a FIXED body for the same reason: the caller should retry, and
-            // must not be told which repo could not be refreshed or why.
-            AppError::RepoUnavailable => (
-                StatusCode::SERVICE_UNAVAILABLE,
-                "repo_unavailable",
-                "repository is temporarily unavailable, retry".into(),
-            ),
-            // 503 with a FIXED body again, and its own code: the caller should
-            // retry, but the condition is not contention, so a client that
-            // distinguishes them should be able to. The body must not say which
-            // repo lost its publish or to whom.
-            AppError::RepoWriteFenced => (
-                StatusCode::SERVICE_UNAVAILABLE,
-                "repo_write_fenced",
-                "repository changed underneath this write, retry".into(),
-            ),
             AppError::Db(e) if db_unavailable(e) => (
                 StatusCode::SERVICE_UNAVAILABLE,
                 DB_UNAVAILABLE_CODE,
@@ -300,11 +244,7 @@ impl IntoResponse for AppError {
         // here rather than in bespoke early returns, keeping each variant handled once.
         if matches!(
             self,
-            AppError::Overloaded(_)
-                | AppError::SearchIncomplete { .. }
-                | AppError::RepoBusy
-                | AppError::RepoUnavailable
-                | AppError::RepoWriteFenced
+            AppError::Overloaded(_) | AppError::SearchIncomplete { .. }
         ) {
             resp.headers_mut().insert(
                 axum::http::header::RETRY_AFTER,
@@ -422,31 +362,4 @@ mod tests {
         let resp = err.into_response();
         assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
     }
-
-    #[test]
-    fn lock_pool_busy_via_anyhow_from_is_503_overloaded() {
-        let err: AppError = anyhow::Error::new(crate::git::repo_store::LockPoolBusy).into();
-        let resp = err.into_response();
-        assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
-        assert_eq!(
-            resp.headers().get("retry-after").unwrap().to_str().unwrap(),
-            "1"
-        );
-    }
-
-    #[test]
-    fn repo_busy_unavailable_and_fenced_advertise_retry_after() {
-        for err in [
-            AppError::RepoBusy,
-            AppError::RepoUnavailable,
-            AppError::RepoWriteFenced,
-        ] {
-            let resp = err.into_response();
-            assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
-            assert_eq!(
-                resp.headers().get("retry-after").unwrap().to_str().unwrap(),
-                "1"
-            );
-        }
-    }
 }

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

@@ -61,27 +61,6 @@ pub fn create_issue(repo_path: &Path, issue_id: &str, json: &str) -> Result<()>
     Ok(())
 }
 
-/// Remove a single issue ref while the write guard still holds the advisory lock,
-/// after a definite publish refusal. Best-effort: a failed delete leaves a
-/// local-only orphan, which is still better than telling the client to retry into a
-/// duplicate id. Not used on `UploadUnknowable`, where the PUT may still land.
-pub fn delete_issue_ref(
-    git_bin: &str,
-    repo_path: &Path,
-    issue_id: &str,
-    deadline: std::time::Instant,
-) -> Result<()> {
-    let ref_name = format!("refs/gitlawb/issues/{issue_id}");
-    crate::git::visibility_pack::run_bounded_git(
-        git_bin,
-        &["update-ref", "-d", &ref_name],
-        repo_path,
-        b"",
-        deadline,
-    )?;
-    Ok(())
-}
-
 /// List all issue refs and return their JSON content.
 pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
     // List all refs under refs/gitlawb/issues/
@@ -299,27 +278,6 @@ mod tests {
         assert!(result.unwrap_err().to_string().contains("ambiguous"));
     }
 
-    #[test]
-    fn delete_issue_ref_removes_a_created_ref() {
-        let dir = TempDir::new().unwrap();
-        init_repo(&dir);
-        let full_id = "eee88888-0000-0000-0000-000000000000";
-        create_issue(
-            dir.path(),
-            full_id,
-            r#"{"id":"eee88888-0000-0000-0000-000000000000","status":"open"}"#,
-        )
-        .unwrap();
-        delete_issue_ref(
-            "git",
-            dir.path(),
-            full_id,
-            std::time::Instant::now() + std::time::Duration::from_secs(30),
-        )
-        .unwrap();
-        assert_eq!(resolve_issue_id(dir.path(), full_id).unwrap(), None);
-    }
-
     #[test]
     fn test_close_issue_via_prefix() {
         let dir = TempDir::new().unwrap();

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

@@ -870,7 +870,6 @@ pub fn merge_branch(
 }
 
 /// Resolve a repo disk path: {repos_dir}/{owner_slug}/{repo_name}.git
-#[allow(dead_code)] // exercised from test_support and state tests; production uses validated_repo_disk_path
 pub fn repo_disk_path(repos_dir: &Path, owner_did: &str, repo_name: &str) -> PathBuf {
     // Sanitize the DID for use as a directory name
     let owner_slug = owner_did.replace([':', '/'], "_");

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

@@ -5,41 +5,12 @@
 
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
-use std::sync::atomic::AtomicBool;
 use std::sync::{Arc, Mutex, OnceLock};
 
 use anyhow::{Context, Result};
 use aws_sdk_s3::Client as S3Client;
 use tracing::{debug, info};
 
-/// The precondition an upload is fenced on.
-///
-/// Object storage is the only place a fence can hold. Dropping the future of an
-/// in-flight PUT does not cancel the request the server is already processing,
-/// so no amount of local locking stops an abandoned writer's bytes from landing
-/// after a successor has published. A conditional PUT the store itself refuses
-/// is what actually stops it.
-#[derive(Clone, Debug)]
-pub enum UploadPrecondition {
-    /// Publish only if the stored object is still the generation we observed.
-    IfMatch(String),
-    /// Publish only if nothing is stored under the key yet.
-    IfAbsent,
-    /// No fence. Last writer wins.
-    Unconditional,
-}
-
-/// Why an upload failed, split so a caller can tell "someone else already
-/// published this key" (expected, and dropping our bytes is the correct
-/// outcome) from a real storage failure.
-#[derive(Debug, thiserror::Error)]
-pub enum UploadError {
-    #[error("upload precondition lost (HTTP {status})")]
-    PreconditionLost { status: u16 },
-    #[error(transparent)]
-    Other(#[from] anyhow::Error),
-}
-
 /// Wrapper around the S3 client with the configured bucket.
 #[derive(Clone)]
 pub struct TigrisClient {
@@ -60,29 +31,21 @@ impl TigrisClient {
         })
     }
 
-    /// Build a client pointed at an arbitrary endpoint, for tests.
-    ///
-    /// The production constructor reads the endpoint and credentials from the
-    /// environment, which a test cannot steer without mutating process-global
-    /// state. This takes both explicitly so a test can aim the client at a
-    /// closed port and get a prompt transport error out of `exists()`.
-    ///
-    /// `RetryConfig::disabled()` is load-bearing, not tidiness: the SDK's default
-    /// policy retries a connection refusal with backoff, which turns each failing
-    /// call into seconds of waiting.
+    /// Test-only constructor with an explicit S3 endpoint, region, and static
+    /// credentials — no env-var reads, so parallel tests cannot race each other's
+    /// `AWS_*` environment the way the env-based `new` would. Lets a test point
+    /// the client at a non-routable endpoint to exercise acquire-stall paths.
     #[cfg(test)]
-    pub fn for_testing_with_endpoint(bucket: &str, endpoint: &str) -> Self {
-        use aws_sdk_s3::config::{retry::RetryConfig, Credentials, Region};
-
-        let config = aws_sdk_s3::config::Config::builder()
-            .endpoint_url(endpoint)
-            .credentials_provider(Credentials::new("test", "test", None, None, "test"))
-            .region(Region::new("auto"))
-            .retry_config(RetryConfig::disabled())
-            .behavior_version_latest()
-            .build();
+    pub(crate) async fn for_testing_with_endpoint(bucket: &str, endpoint_url: &str) -> Self {
+        let creds = aws_sdk_s3::config::Credentials::new("test", "test", None, None, "test");
+        let config = aws_config::defaults(aws_config::BehaviorVersion::latest())
+            .endpoint_url(endpoint_url)
+            .region(aws_config::Region::new("auto"))
+            .credentials_provider(creds)
+            .load()
+            .await;
         Self {
-            s3: S3Client::from_conf(config),
+            s3: S3Client::new(&config),
             bucket: bucket.to_string(),
         }
     }
@@ -114,47 +77,8 @@ impl TigrisClient {
         }
     }
 
-    /// Read the ETag of a repo archive, or `None` when nothing is stored under
-    /// the key. The ETag identifies the generation a later conditional upload
-    /// can fence itself on.
-    ///
-    /// Separate from `exists` rather than folded into it: `exists` has callers
-    /// that only want the boolean, and widening its return type would churn
-    /// every one of them for no benefit.
-    pub async fn head_etag(&self, owner_slug: &str, repo_name: &str) -> Result<Option<String>> {
-        let key = Self::repo_key(owner_slug, repo_name);
-        match self
-            .s3
-            .head_object()
-            .bucket(&self.bucket)
-            .key(&key)
-            .send()
-            .await
-        {
-            Ok(out) => Ok(Some(
-                out.e_tag()
-                    .context(format!("tigris HEAD {key}: hit carried no ETag"))?
-                    .to_string(),
-            )),
-            Err(e) => {
-                if e.as_service_error().is_some_and(|e| e.is_not_found()) {
-                    Ok(None)
-                } else {
-                    Err(anyhow::anyhow!("tigris HEAD {key}: {e}"))
-                }
-            }
-        }
-    }
-
-    /// Upload a local bare repo directory to Tigris as a tar.zst archive,
-    /// fenced by `precondition`.
-    pub async fn upload(
-        &self,
-        owner_slug: &str,
-        repo_name: &str,
-        local_path: &Path,
-        precondition: UploadPrecondition,
-    ) -> std::result::Result<(), UploadError> {
+    /// Upload a local bare repo directory to Tigris as a tar.zst archive.
+    pub async fn upload(&self, owner_slug: &str, repo_name: &str, local_path: &Path) -> Result<()> {
         let key = Self::repo_key(owner_slug, repo_name);
         debug!(key = %key, path = %local_path.display(), "uploading repo to tigris");
 
@@ -169,57 +93,15 @@ impl TigrisClient {
 
         let body = aws_sdk_s3::primitives::ByteStream::from(archive_bytes);
 
-        let mut req = self
-            .s3
+        self.s3
             .put_object()
             .bucket(&self.bucket)
             .key(&key)
             .body(body)
-            .content_type("application/zstd");
-        match &precondition {
-            UploadPrecondition::IfMatch(etag) => req = req.if_match(etag),
-            UploadPrecondition::IfAbsent => req = req.if_none_match("*"),
-            UploadPrecondition::Unconditional => {}
-        }
-
-        if let Err(e) = req.send().await {
-            // `PutObjectError` models no PreconditionFailed variant (its arms are
-            // EncryptionTypeMismatch, InvalidRequest, InvalidWriteOffset,
-            // TooManyParts, Unhandled), so a refused precondition arrives as
-            // `Unhandled` and matching the enum would classify it as a generic
-            // failure. The raw HTTP status off the response is the only place the
-            // answer actually lives.
-            //
-            // Read it via `raw_response()`, not a `ServiceError`-only match: the
-            // SDK exposes the raw response for BOTH `ServiceError` and
-            // `ResponseError`, and a refused conditional PUT whose error body the
-            // SDK cannot parse (malformed XML, premature close) surfaces as
-            // `ResponseError`. Matching only `ServiceError` would classify that
-            // unparsable 409/412 as a generic failure, and `RepoWriteGuard::release`
-            // would log-and-succeed instead of taking the supersede retry,
-            // acknowledging a write that was definitively not published.
-            let status = e.raw_response().map(|raw| raw.status().as_u16());
-            // 412 is always a lost precondition. 409 is one only when we asked
-            // for create-only, which is how S3-compatible stores report "the key
-            // already exists". Everything else, 404 included, is a real failure:
-            // archive keys are never deleted (`delete` has no callers), so a 404
-            // here means something permanent like a missing bucket or a
-            // misrouted endpoint, and reporting that as a lost precondition
-            // would tell a caller to expect a successor that does not exist.
-            let lost = match status {
-                Some(412) => true,
-                Some(409) => matches!(precondition, UploadPrecondition::IfAbsent),
-                _ => false,
-            };
-            if lost {
-                return Err(UploadError::PreconditionLost {
-                    status: status.expect("a lost precondition came from a status"),
-                });
-            }
-            return Err(UploadError::Other(
-                anyhow::Error::new(e).context(format!("tigris PUT {key}")),
-            ));
-        }
+            .content_type("application/zstd")
+            .send()
+            .await
+            .context(format!("tigris PUT {key}"))?;
 
         info!(key = %key, "uploaded repo to tigris");
         Ok(())
@@ -230,32 +112,10 @@ impl TigrisClient {
         &self,
         owner_slug: &str,
         repo_name: &str,
-        local_path: &super::repo_store::ValidatedRepoDiskPath,
-        swap_authority: Option<Arc<AtomicBool>>,
+        local_path: &Path,
     ) -> Result<()> {
-        self.download_to(owner_slug, repo_name, local_path, true, swap_authority)
-            .await
-            .map(|_| ())
-    }
-
-    /// Download a repo archive from Tigris and extract it, returning the
-    /// directory that was populated.
-    ///
-    /// `publish` controls whether the extract is swapped into `target` in place
-    /// (the live-path mutation used by writes; returns `target`) or unpacked
-    /// into a fresh temp directory under `target`'s parent (a non-mutating
-    /// snapshot read; returns the temp dir, which the caller owns and cleans
-    /// up). The snapshot form never touches the live repo path.
-    pub async fn download_to(
-        &self,
-        owner_slug: &str,
-        repo_name: &str,
-        target: &super::repo_store::ValidatedRepoDiskPath,
-        publish: bool,
-        swap_authority: Option<Arc<AtomicBool>>,
-    ) -> Result<DownloadExtract> {
         let key = Self::repo_key(owner_slug, repo_name);
-        debug!(key = %key, path = %target.as_path().display(), "downloading repo from tigris");
+        debug!(key = %key, path = %local_path.display(), "downloading repo from tigris");
 
         let resp = self
             .s3
@@ -273,62 +133,21 @@ impl TigrisClient {
             .context("reading tigris response body")?
             .into_bytes();
 
-        // The snapshot temp dir is decided HERE, in the async layer, before the
-        // extraction runs. Cleanup ownership moves into the blocking task so a
-        // cancelled async future cannot drop it while extraction is still running.
-        let snapshot_tmp = if publish {
-            None
-        } else {
-            let parent = target.parent().context("snapshot path has no parent")?;
-            std::fs::create_dir_all(parent).context("creating parent dir")?;
-            let file_name = target
-                .file_name()
-                .context("snapshot path has no file name")?
-                .to_string_lossy();
-            Some(parent.join(format!(
-                ".{file_name}.tmp-snapshot.{}",
-                uuid::Uuid::new_v4()
-            )))
-        };
-
-        // Extract tar.zst to a directory.
-        let extracted = tokio::task::spawn_blocking({
-            let target = target.clone();
-            let snapshot_tmp = snapshot_tmp.clone();
-            move || -> Result<DownloadExtract> {
-                let result = (|| -> Result<DownloadExtract> {
-                    if publish {
-                        decompress_repo(&data, &target, swap_authority.as_ref())?;
-                        return Ok(DownloadExtract::Published(()));
-                    }
-                    // Non-mutating snapshot: unpack into the temp dir decided above.
-                    // The live repo path is never touched.
-                    let tmp_dir = snapshot_tmp.expect("snapshot path was decided above");
-                    std::fs::create_dir_all(&tmp_dir).context("creating temp extract dir")?;
-                    let unpack = (|| -> Result<()> {
-                        let decoder = zstd::stream::Decoder::new(&data[..])?;
-                        let mut archive = tar::Archive::new(decoder);
-                        archive.unpack(&tmp_dir).context("unpacking tar.zst")?;
-                        Ok(())
-                    })();
-                    if let Err(e) = unpack {
-                        let _ = std::fs::remove_dir_all(&tmp_dir);
-                        return Err(e);
-                    }
-                    Ok(DownloadExtract::Snapshot(TempSnapshotDir { path: tmp_dir }))
-                })();
-                result
-            }
+        // Extract tar.zst to local path
+        tokio::task::spawn_blocking({
+            let local_path = local_path.to_path_buf();
+            move || decompress_repo(&data, &local_path)
         })
         .await
         .context("extract task panicked")?
         .context("extracting repo")?;
 
-        info!(key = %key, path = %target.as_path().display(), "downloaded repo from tigris");
-        Ok(extracted)
+        info!(key = %key, path = %local_path.display(), "downloaded repo from tigris");
+        Ok(())
     }
 
     /// Delete a repo archive from Tigris.
+    #[allow(dead_code)]
     pub async fn delete(&self, owner_slug: &str, repo_name: &str) -> Result<()> {
         let key = Self::repo_key(owner_slug, repo_name);
         self.s3
@@ -361,7 +180,7 @@ fn compress_repo(repo_path: &Path) -> Result<Vec<u8>> {
 /// `decompress_repo`. Concurrent extractions unpack into isolated temp dirs in
 /// parallel, but the final `remove_dir_all` + `rename` must not interleave for
 /// the same `local_path`, or they race to a nondeterministic overwrite/failure.
-pub(crate) fn publish_lock(local_path: &Path) -> Arc<Mutex<()>> {
+fn publish_lock(local_path: &Path) -> Arc<Mutex<()>> {
     // KNOWN LIMITATION: this map is never evicted — one (PathBuf, Arc<Mutex>)
     // entry accrues per distinct repo path for the process lifetime. Bounded by
     // the number of repos a node hosts, so it's negligible for normal use, but
@@ -374,35 +193,6 @@ pub(crate) fn publish_lock(local_path: &Path) -> Arc<Mutex<()>> {
         .clone()
 }
 
-/// Owns a snapshot temp dir from extraction through handoff to [`RepoSnapshot`].
-/// Dropped when the `spawn_blocking` join result is abandoned, so cancellation
-/// before the outer future resumes still removes the directory.
-pub(crate) struct TempSnapshotDir {
-    path: PathBuf,
-}
-
-impl TempSnapshotDir {
-    pub(crate) fn into_repo_snapshot(self) -> super::repo_store::RepoSnapshot {
-        let path = self.path.clone();
-        std::mem::forget(self);
-        super::repo_store::RepoSnapshot::from_owned_path(path)
-    }
-}
-
-impl Drop for TempSnapshotDir {
-    fn drop(&mut self) {
-        let _ = std::fs::remove_dir_all(&self.path);
-    }
-}
-
-/// What [`TigrisClient::download_to`] produced.
-pub enum DownloadExtract {
-    /// Published into the validated live repo path.
-    Published(()),
-    /// Unpacked into a throwaway temp dir that cleans up on drop until adopted.
-    Snapshot(TempSnapshotDir),
-}
-
 /// Decompress a tar.zst byte vector into a local directory.
 ///
 /// Extraction is atomic with respect to `local_path`: the archive is unpacked
@@ -410,16 +200,11 @@ pub enum DownloadExtract {
 /// fully succeeds. A corrupt or truncated archive therefore can never clobber a
 /// good existing copy at `local_path` — on failure we discard the temp dir and
 /// leave `local_path` exactly as it was.
-fn decompress_repo(
-    data: &[u8],
-    local_path: &super::repo_store::ValidatedRepoDiskPath,
-    swap_authority: Option<&Arc<AtomicBool>>,
-) -> Result<()> {
-    let live = local_path.as_path();
-    let parent = live.parent().context("repo path has no parent")?;
+fn decompress_repo(data: &[u8], local_path: &Path) -> Result<()> {
+    let parent = local_path.parent().context("repo path has no parent")?;
     std::fs::create_dir_all(parent).context("creating parent dir")?;
 
-    let file_name = live
+    let file_name = local_path
         .file_name()
         .context("repo path has no file name")?
         .to_string_lossy();
@@ -444,218 +229,17 @@ fn decompress_repo(
         return Err(e);
     }
 
-    // Swap through the validated-path helper so CodeQL sees the barrier before the
-    // remove/rename sink (`rust/path-injection`).
-    super::repo_store::swap_extracted_into_validated_repo(local_path, &tmp_dir, swap_authority)?;
+    // Swap the freshly-extracted repo into place. rename within the same parent
+    // is effectively atomic, but most platforms refuse to rename onto a
+    // non-empty dir, so remove the old copy first. Serialize this per repo path:
+    // concurrent extractions unpack into isolated temp dirs, but their swaps
+    // must not interleave or they race to a nondeterministic overwrite/failure.
+    let lock = publish_lock(local_path);
+    let _publish = lock.lock().expect("publish lock poisoned");
+    if local_path.exists() {
+        std::fs::remove_dir_all(local_path).context("removing stale repo dir")?;
+    }
+    std::fs::rename(&tmp_dir, local_path).context("swapping extracted repo into place")?;
 
     Ok(())
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use aws_sdk_s3::primitives::ByteStream;
-    use futures::FutureExt;
-
-    /// The envs the probe needs, all of them, or it does not run.
-    ///
-    /// `AWS_ENDPOINT_URL_S3` is included on purpose: without it the SDK resolves
-    /// to real AWS S3, and a probe that passed there would say nothing about
-    /// Tigris.
-    fn probe_env() -> Option<String> {
-        if std::env::var("GITLAWB_TIGRIS_PROBE").ok().as_deref() != Some("1") {
-            return None;
-        }
-        for name in [
-            "AWS_ACCESS_KEY_ID",
-            "AWS_SECRET_ACCESS_KEY",
-            "AWS_ENDPOINT_URL_S3",
-        ] {
-            if std::env::var(name).is_err() {
-                eprintln!("tigris conditional-write probe: {name} is unset, skipping");
-                return None;
-            }
-        }
-        match std::env::var("GITLAWB_TIGRIS_BUCKET") {
-            Ok(b) if !b.is_empty() => Some(b),
-            _ => {
-                eprintln!(
-                    "tigris conditional-write probe: GITLAWB_TIGRIS_BUCKET is unset, skipping"
-                );
-                None
-            }
-        }
-    }
-
-    /// One conditional PUT, reported as the status that REFUSED it, or `None`
-    /// when the store accepted the write.
-    ///
-    /// Accepted is the interesting answer here, not an error: it means the
-    /// endpoint ignored the header we fenced on.
-    async fn conditional_put(
-        s3: &S3Client,
-        bucket: &str,
-        key: &str,
-        body: &'static [u8],
-        if_match: Option<&str>,
-        if_none_match: Option<&str>,
-    ) -> Result<Option<u16>, String> {
-        let mut req = s3
-            .put_object()
-            .bucket(bucket)
-            .key(key)
-            .body(ByteStream::from_static(body));
-        if let Some(v) = if_match {
-            req = req.if_match(v);
-        }
-        if let Some(v) = if_none_match {
-            req = req.if_none_match(v);
-        }
-        match req.send().await {
-            Ok(_) => Ok(None),
-            Err(e) => match e.raw_response() {
-                // Same raw-response rule as `upload`: a refused conditional PUT
-                // whose body the SDK cannot parse surfaces as `ResponseError`, and
-                // the status has to come off the raw response for both variants.
-                // Without it, an unparsable 409/412 would report "no HTTP
-                // response" here and skip the supersede retry.
-                Some(raw) => Ok(Some(raw.status().as_u16())),
-                None => Err(format!("conditional PUT {key}: no HTTP response: {e}")),
-            },
-        }
-    }
-
-    /// The probe body, written to RETURN its failures rather than panic on
-    /// them, so the caller's cleanup is reached on every arm.
-    async fn conditional_write_probe(s3: &S3Client, bucket: &str, key: &str) -> Result<(), String> {
-        // 1. A plain PUT under a throwaway key.
-        let seeded = s3
-            .put_object()
-            .bucket(bucket)
-            .key(key)
-            .body(ByteStream::from_static(b"probe-one"))
-            .send()
-            .await
-            .map_err(|e| format!("seeding PUT {key}: {e}"))?;
-
-        // 2. Its ETag, which is the generation the next arm fences against.
-        let etag = seeded
-            .e_tag()
-            .ok_or_else(|| format!("seeding PUT {key} returned no ETag"))?
-            .to_string();
-
-        // 3. A deliberately wrong If-Match. A store honoring it answers 412.
-        let wrong = format!("\"{}\"", "0".repeat(32));
-        if etag.trim_matches('"') == wrong.trim_matches('"') {
-            return Err(format!(
-                "the seeded ETag {etag} collides with the deliberately wrong one, \
-                 so this arm would prove nothing"
-            ));
-        }
-        match conditional_put(s3, bucket, key, b"probe-two", Some(&wrong), None).await? {
-            Some(412) => {}
-            Some(status) => {
-                return Err(format!(
-                    "a stale If-Match must be refused with 412, the endpoint answered {status}"
-                ))
-            }
-            None => {
-                return Err(
-                    "a stale If-Match was ACCEPTED: this endpoint does not honor If-Match, so \
-                     the release fence cannot hold here"
-                        .to_string(),
-                )
-            }
-        }
-
-        // 4. If-None-Match `*` over the object that now exists. This arm matters
-        // MORE than the one above. An ignored If-Match eventually surfaces as
-        // odd behavior, because a stale writer overwrites and someone notices
-        // the lost tree. An ignored If-None-Match just returns 200, so a publish
-        // that should have been fenced lands with no error anywhere: the silent
-        // no-op the bucket-type caveat on this test describes.
-        match conditional_put(s3, bucket, key, b"probe-three", None, Some("*")).await? {
-            // Either status is a pass, and the asymmetry with the If-Match arm
-            // above mirrors `upload`'s classifier exactly: 412 is always a lost
-            // precondition, and 409 is one too when we asked for create-only.
-            // AWS documents 409 for a create-only conflict racing a delete, so a
-            // store answering it is enforcing the precondition and we already
-            // handle it. Pinning 412 alone here would fail the probe against a
-            // backend that is behaving correctly, which sends whoever runs it
-            // chasing a fault that is not there.
-            Some(412) | Some(409) => {}
-            Some(status) => {
-                return Err(format!(
-                    "create-only over an existing object must be refused with 412 or 409, \
-                     the endpoint answered {status}"
-                ))
-            }
-            None => {
-                return Err(
-                    "If-None-Match * was ACCEPTED over an existing object: this endpoint does \
-                     not honor create-only, so a fenced publish lands silently"
-                        .to_string(),
-                )
-            }
-        }
-
-        Ok(())
-    }
-
-    /// Probe the REAL Tigris endpoint for the conditional-write semantics the
-    /// release fence depends on.
-    ///
-    /// UNTIL THIS IS RUN AGAINST REAL CREDENTIALS, the fence is verified against
-    /// vendor documentation and an in-process mock, not against the backend it
-    /// runs on. The mock implements the semantics we believe Tigris has; it
-    /// cannot tell us whether Tigris actually has them.
-    ///
-    /// The bucket matters, not just the endpoint. Tigris documents conditional
-    /// operations as supported on Single-region and Multi-region buckets only.
-    /// Global and Dual-region buckets are eventually consistent, and a
-    /// conditional PUT evaluated against a stale replica would make the fence a
-    /// silent no-op rather than an error. So point `GITLAWB_TIGRIS_BUCKET` at a
-    /// throwaway bucket of the SAME type production uses.
-    ///
-    /// Ignored by default and additionally gated on `GITLAWB_TIGRIS_PROBE=1`,
-    /// because it writes to a real bucket and costs real requests. Run with:
-    /// `GITLAWB_TIGRIS_PROBE=1 cargo test -p gitlawb-node --bin gitlawb-node
-    /// tigris_honors_conditional_writes -- --ignored --nocapture`
-    #[tokio::test]
-    #[ignore = "writes to a real Tigris bucket; needs GITLAWB_TIGRIS_PROBE=1 plus credentials"]
-    async fn tigris_honors_conditional_writes() {
-        let Some(bucket) = probe_env() else {
-            eprintln!(
-                "tigris conditional-write probe: skipped. Set GITLAWB_TIGRIS_PROBE=1, \
-                 GITLAWB_TIGRIS_BUCKET, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY and \
-                 AWS_ENDPOINT_URL_S3 to run it."
-            );
-            return;
-        };
-
-        let config = aws_config::load_defaults(aws_config::BehaviorVersion::latest()).await;
-        let s3 = S3Client::new(&config);
-        // A fresh key per run, so a probe that somehow orphaned an object on an
-        // earlier run cannot change what this one observes.
-        let key = format!("probe/conditional-write-{}.bin", uuid::Uuid::new_v4());
-
-        // CLEANUP MUST RUN ON EVERY ARM, and a failing assertion is precisely
-        // the case this probe exists to catch, so the delete cannot sit after
-        // the checks. The body returns its failures rather than panicking, and
-        // `catch_unwind` covers the panic an SDK call could still raise; either
-        // way the delete below is reached before the verdict is re-raised.
-        let outcome = std::panic::AssertUnwindSafe(conditional_write_probe(&s3, &bucket, &key))
-            .catch_unwind()
-            .await;
-
-        if let Err(e) = s3.delete_object().bucket(&bucket).key(&key).send().await {
-            eprintln!("tigris conditional-write probe: cleanup of {key} failed: {e}");
-        }
-
-        match outcome {
-            Ok(Ok(())) => {}
-            Ok(Err(msg)) => panic!("tigris conditional-write probe: {msg}"),
-            Err(payload) => std::panic::resume_unwind(payload),
-        }
-    }
-}

crates/gitlawb-node/src/ipfs_pin.rs

@@ -676,7 +676,7 @@ async fn warm_candidates(
                 &repo.owner_did,
                 &repo.name,
             ) {
-                Ok(p) if p.is_dir() => out.push((repo, created_at_key, p.into_path_buf())),
+                Ok(p) if p.is_dir() => out.push((repo, created_at_key, p)),
                 Ok(_) => {}
                 Err(e) => {
                     tracing::warn!(repo_id = %repo.id, err = %e, "sweep discovery: rejected unsafe repo path");

crates/gitlawb-node/src/state.rs

@@ -79,10 +79,6 @@ pub struct AppState {
     /// brake a push flood from a DID farm (one throwaway DID per repo), so the
     /// push path throttles on the resolved client IP instead.
     pub push_rate_limiter: RateLimiter,
-    /// Per-client-IP rate limiter for `close_issue`'s pre-lock snapshot path.
-    /// Distinct from `push_rate_limiter` so a flood of close attempts cannot
-    /// drain the receive-pack budget for the same source IP.
-    pub close_issue_rate_limiter: RateLimiter,
     /// Per-client-IP ROUTE brake for `GET /ipfs/{cid}`: charged ONCE per request by the
     /// `rate_limit_by_ip` middleware (server.rs), never inside the handler. It bounds
     /// request RATE (the "requests per hour" contract of `GITLAWB_IPFS_RATE_LIMIT`) on
@@ -346,7 +342,6 @@ impl AppState {
         self.rate_limiter.cleanup().await;
         self.create_ip_rate_limiter.cleanup().await;
         self.push_rate_limiter.cleanup().await;
-        self.close_issue_rate_limiter.cleanup().await;
         self.ipfs_rate_limiter.cleanup().await;
         self.ipfs_work_rate_limiter.cleanup().await;
         self.sync_trigger_rate_limiter.cleanup().await;

crates/gitlawb-node/src/test_harness.rs

@@ -0,0 +1,586 @@
+//! Test-only spawn surface for the real-node deny harness (see
+//! `tests/deny_harness.rs`). Feature-gated behind `test-harness` so it never
+//! compiles into the production binary. It exposes a single boot constructor so
+//! the out-of-crate integration test can bring up a real node over a bound TCP
+//! socket without the integration crate needing access to `graphql`,
+//! `rate_limit`, `repo_store`, or the other internals `AppState` is built from.
+//!
+//! The node is built the same way `test_support::build_state` builds it (p2p
+//! disabled, real migrated pool), but bound on `127.0.0.1:0` and served through
+//! the real `axum::serve` stack with connect-info, so the middleware order,
+//! body limits, and per-IP rate limiters all run exactly as in production.
+
+use std::net::SocketAddr;
+use std::path::PathBuf;
+use std::sync::atomic::{AtomicU64, Ordering};
+use std::sync::Arc;
+use std::time::Duration;
+
+use chrono::Utc;
+use clap::Parser;
+use sqlx::PgPool;
+use tokio::net::TcpListener;
+use tokio::sync::watch;
+use uuid::Uuid;
+
+use gitlawb_core::identity::Keypair;
+
+use crate::config::Config;
+use crate::db::{Db, PullRequest, RepoRecord, VisibilityMode};
+use crate::rate_limit::{RateLimiter, TrustedProxy};
+use crate::state::AppState;
+
+/// A running node bound to an ephemeral port. End every test with
+/// [`TestNode::shutdown`]: it joins the serve task and closes the pool, so
+/// `#[sqlx::test]`'s `DROP DATABASE` cleanup (which runs the moment the test
+/// future returns) never races the server's still-open connections. Dropping
+/// without it signals shutdown, aborts any remaining serve task, and removes
+/// the temp repository directory, a fallback for tests that return or panic
+/// before reaching teardown.
+pub struct TestNode {
+    /// Base URL of the running node, e.g. `http://127.0.0.1:54321`.
+    pub base_url: String,
+    /// The node's own DID (for building requests that reference it).
+    pub node_did: String,
+    shutdown_tx: watch::Sender<bool>,
+    repos_dir: PathBuf,
+    /// Seeding handle over the same pool the node serves from. Kept private so
+    /// the integration crate seeds through the methods below rather than
+    /// naming `Db`/`RepoRecord` directly.
+    db: Arc<Db>,
+    /// The detached `axum::serve` task, joined by [`Self::shutdown`]. `None`
+    /// once taken, so the `Drop` fallback never overlaps a completed teardown.
+    /// Carries `serve`'s own `io::Result<()>` so `shutdown()` can observe it
+    /// rather than discard it (see the coverage note there).
+    server: Option<tokio::task::JoinHandle<std::io::Result<()>>>,
+    /// A clone of the served pool, kept so [`Self::shutdown`] can close it
+    /// (sqlx pool clones share one inner, so closing this closes the clones
+    /// inside `AppState`/`Db` too).
+    pool: PgPool,
+    /// Test-only: when set, `Drop` skips the graceful watch signal so ONLY
+    /// `handle.abort()` tears the serve task down. Defaults false, so every
+    /// other test keeps both redundant teardown mechanisms. Set via
+    /// [`Self::force_abort_only_teardown`] to fence that the abort alone
+    /// releases the database when the graceful chain is broken.
+    suppress_graceful_shutdown_on_drop: bool,
+}
+
+impl Drop for TestNode {
+    fn drop(&mut self) {
+        // Fallback for tests that return or panic before reaching
+        // [`TestNode::shutdown`]: flip the shared shutdown signal so the serve
+        // task exits gracefully, abort whatever is left of it, and remove the
+        // temp repos dir. Drop cannot await, so it cannot join the server or
+        // close the pool; a test that already failed should not panic again in
+        // teardown. All three actions are idempotent or no-ops after
+        // `shutdown()` (which takes the handle), so running afterwards is
+        // harmless.
+        //
+        // The abort matters when the graceful chain does not run to completion
+        // before sqlx's `DROP DATABASE` cleanup: `JoinHandle::abort()` is a
+        // synchronous cancellation request callable from non-async Drop, and
+        // the aborted task's future (with the `PgPool` clones inside its
+        // router/state) is dropped on a subsequent scheduler tick, which sqlx
+        // cleanup's own awaits provide before the drop statement runs. The
+        // race is closed by those scheduling points, not by the abort alone.
+        //
+        // `suppress_graceful_shutdown_on_drop` (test-only) forces the abort to
+        // carry teardown by itself, so a test can fence that the abort line is
+        // load-bearing; it is false everywhere else.
+        if !self.suppress_graceful_shutdown_on_drop {
+            let _ = self.shutdown_tx.send(true);
+        }
+        if let Some(handle) = self.server.take() {
+            handle.abort();
+        }
+        let _ = std::fs::remove_dir_all(&self.repos_dir);
+    }
+}
+
+/// Outcome of joining the serve task during teardown, split so the wedged-task
+/// path is observable to a test (which injects a short timeout) without waiting
+/// the production teardown window.
+enum ServeTeardown {
+    /// The task returned (carrying `serve`'s `io::Result<()>`) — either within
+    /// the timeout, or in the race between the timeout firing and the abort
+    /// landing. A task that actually finished is never a wedge.
+    Graceful(std::io::Result<()>),
+    /// The task did not finish within the timeout and was aborted (cancelled)
+    /// and reaped.
+    TimedOutAborted,
+}
+
+/// Join `handle` within `timeout`; on expiry `abort()` it and await the aborted
+/// handle so the serve task (and the `PgPool` clones inside its router/state) is
+/// reaped BEFORE the caller returns and `#[sqlx::test]` runs `DROP DATABASE`.
+/// Takes `&mut` (a `JoinHandle` is `Unpin`) so ownership is retained across the
+/// timeout and the handle can be aborted on expiry — the previous code moved the
+/// handle into `timeout` and, on elapse, dropped it, which DETACHES the still
+/// running task rather than aborting it, leaking the test database (#194 F2).
+async fn join_or_abort(
+    handle: &mut tokio::task::JoinHandle<std::io::Result<()>>,
+    timeout: Duration,
+) -> ServeTeardown {
+    match tokio::time::timeout(timeout, &mut *handle).await {
+        Ok(join_result) => ServeTeardown::Graceful(join_result.expect("serve task must not panic")),
+        Err(_elapsed) => {
+            handle.abort();
+            match handle.await {
+                // The task finished in the race between the timeout firing and
+                // the abort landing — honor its result, not a false wedge.
+                Ok(result) => ServeTeardown::Graceful(result),
+                Err(e) if e.is_cancelled() => ServeTeardown::TimedOutAborted,
+                // A genuine panic in the serve task surfaces here, mirroring the
+                // in-time arm's `.expect("serve task must not panic")`.
+                Err(e) => std::panic::resume_unwind(e.into_panic()),
+            }
+        }
+    }
+}
+
+/// Allocate a process-unique temp directory for a spawned node's repositories
+/// without pulling in the `tempfile` dev-dependency (which is unavailable to
+/// the library crate under `--features test-harness`).
+fn unique_repos_dir() -> PathBuf {
+    static COUNTER: AtomicU64 = AtomicU64::new(0);
+    let n = COUNTER.fetch_add(1, Ordering::Relaxed);
+    std::env::temp_dir().join(format!("gitlawb-deny-harness-{}-{}", std::process::id(), n))
+}
+
+/// Build an [`AppState`] over the given migrated pool, mirroring
+/// `test_support::build_state` but with a real on-disk repository directory so
+/// git smart-HTTP routes can serve. P2P is always disabled.
+fn build_state(db: Arc<crate::db::Db>, pool: PgPool, repos_dir: PathBuf) -> AppState {
+    let keypair = Keypair::generate();
+    let scan_token_key = crate::state::AppState::derive_scan_token_key(&keypair);
+    let node_did = keypair.did();
+    let (ref_tx, _) = tokio::sync::broadcast::channel(1);
+    let (task_tx, _) = tokio::sync::broadcast::channel(1);
+    let schema = Arc::new(crate::graphql::build_schema(
+        db.clone(),
+        ref_tx.clone(),
+        task_tx.clone(),
+    ));
+    AppState {
+        config: Arc::new(Config::parse_from(["gitlawb-node"])),
+        db,
+        node_did,
+        node_keypair: Arc::new(keypair),
+        p2p: None,
+        http_client: Arc::new(reqwest::Client::new()),
+        ref_update_tx: ref_tx,
+        task_event_tx: task_tx,
+        graphql_schema: schema,
+        machine_id: None,
+        repo_store: crate::git::repo_store::RepoStore::for_testing(repos_dir, pool),
+        rate_limiter: RateLimiter::new(100, Duration::from_secs(60)),
+        create_ip_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)),
+        push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
+        ipfs_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
+        ipfs_work_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
+        ipfs_max_history_walks: crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST,
+        ipfs_max_legacy_probes: crate::api::ipfs::MAX_LEGACY_PROBES_PER_REQUEST,
+        ipfs_legacy_scan_page_rows: crate::api::ipfs::LEGACY_SCAN_PAGE_ROWS,
+        ipfs_max_legacy_scan_rows: crate::api::ipfs::MAX_LEGACY_SCAN_ROWS_PER_REQUEST,
+        ipfs_max_legacy_scan_rule_bytes: crate::api::ipfs::MAX_LEGACY_SCAN_RULE_BYTES_PER_REQUEST,
+        ipfs_scan_token_key: Arc::new(scan_token_key),
+        ipfs_max_served_object_bytes: crate::api::ipfs::MAX_SERVED_OBJECT_BYTES,
+        push_limiter_trust: TrustedProxy::None,
+        sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),
+        peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
+        shutdown_tx: watch::channel(false).0,
+        git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
+        git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
+        git_push_advert_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
+        git_encrypt_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
+        pin_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
+        encrypt_inflight: crate::state::EncryptInflight::new(),
+        repo_write_leases: crate::state::RepoWriteLeases::new(8),
+        git_read_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(16),
+        git_push_advert_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(
+            8,
+        ),
+        git_write_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(8),
+        git_ipfs_walk_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
+        git_ipfs_walk_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(
+            16,
+        ),
+        git_bin: "git".to_string(),
+    }
+}
+
+/// Spawn a real node bound to `127.0.0.1:0` over the given (already-created,
+/// empty) test pool. Runs the schema migrations, builds the router, and serves
+/// it on a background task through the production `axum::serve` stack with
+/// connect-info so per-IP layers key on the real peer. Returns once the socket
+/// is bound and accepting connections.
+pub async fn spawn_node(pool: PgPool) -> TestNode {
+    let db = Arc::new(crate::db::Db::for_testing(pool.clone()));
+    db.run_migrations()
+        .await
+        .expect("test schema migrations should apply");
+
+    let repos_dir = unique_repos_dir();
+    std::fs::create_dir_all(&repos_dir).expect("create temp repos dir");
+
+    let state = build_state(db.clone(), pool.clone(), repos_dir.clone());
+    let node_did = state.node_did.to_string();
+    let shutdown_tx = state.shutdown_tx.clone();
+    let mut shutdown_rx = shutdown_tx.subscribe();
+
+    let router = crate::server::build_router(state);
+
+    let listener = TcpListener::bind(("127.0.0.1", 0))
+        .await
+        .expect("bind ephemeral port");
+    let addr = listener.local_addr().expect("read bound addr");
+
+    let server = tokio::spawn(async move {
+        axum::serve(
+            listener,
+            router.into_make_service_with_connect_info::<SocketAddr>(),
+        )
+        .with_graceful_shutdown(async move {
+            let _ = shutdown_rx.changed().await;
+        })
+        .await
+    });
+
+    TestNode {
+        base_url: format!("http://{addr}"),
+        node_did,
+        shutdown_tx,
+        repos_dir,
+        db,
+        server: Some(server),
+        pool,
+        suppress_graceful_shutdown_on_drop: false,
+    }
+}
+
+impl TestNode {
+    /// Test-only: make `Drop` tear the serve task down through `handle.abort()`
+    /// alone by suppressing the graceful watch signal. Used to prove the abort
+    /// is load-bearing (that the database is released even when the graceful
+    /// chain is broken); no production or normal-teardown path calls this.
+    pub fn force_abort_only_teardown(&mut self) {
+        self.suppress_graceful_shutdown_on_drop = true;
+    }
+
+    /// Graceful async teardown: signal shutdown, join the serve task, close
+    /// the pool, remove the temp repos dir. Call this at the end of every
+    /// test. `#[sqlx::test]` issues `DROP DATABASE` as soon as the test
+    /// future returns, so the server (which owns pool clones inside
+    /// `AppState`/`Db`) must be gone and the pool closed before then, or the
+    /// cleanup fails and leaks per-test databases. A serve task that does not
+    /// exit within 10s panics loudly: a wedged graceful shutdown must fail
+    /// the test, not hang CI.
+    ///
+    /// When `self` drops on return, `Drop` re-sends the signal and re-removes
+    /// the dir; both are idempotent, and the serve handle was already taken,
+    /// so the overlap is harmless.
+    pub async fn shutdown(self) {
+        self.shutdown_with_timeout(Duration::from_secs(10)).await;
+    }
+
+    /// Body of [`Self::shutdown`] with an injectable teardown timeout so a test
+    /// can drive the wedged-shutdown path without waiting the production 10s.
+    /// Production always calls this with 10s.
+    async fn shutdown_with_timeout(mut self, timeout: Duration) {
+        let _ = self.shutdown_tx.send(true);
+        let outcome = match self.server.take() {
+            Some(mut handle) => Some(join_or_abort(&mut handle, timeout).await),
+            None => None,
+        };
+        // Release the pool and temp dir on EVERY path before returning OR
+        // panicking. A wedged graceful shutdown is exactly when in-flight
+        // per-connection tasks still hold `PgPool` clones; aborting the serve
+        // future alone does not reliably drop them, so the explicit
+        // `pool.close()` (which closes the shared inner) is what frees them
+        // before `#[sqlx::test]` runs `DROP DATABASE`. Panicking before this
+        // (the previous behavior) skipped the close on the one path that needs
+        // it most (#194 F2).
+        self.pool.close().await;
+        let _ = std::fs::remove_dir_all(&self.repos_dir);
+        match outcome {
+            // Honest coverage contract: on axum 0.8.8,
+            // `WithGracefulShutdown::into_future` always resolves `Ok(())` (the
+            // shutdown signal is treated as success and `accept()` errors are
+            // retried forever rather than returned), so this `Err` arm is
+            // unreachable today - every existing harness test exercises only the
+            // `Ok` path here. Forward-compat insurance for a future axum where
+            // `serve` can fail: that should surface as a clear panic, not be
+            // silently discarded.
+            Some(ServeTeardown::Graceful(result)) => {
+                result.expect("axum::serve exited with an error");
+            }
+            Some(ServeTeardown::TimedOutAborted) => {
+                panic!(
+                    "serve task did not exit within {timeout:?} of the shutdown \
+                     signal (wedged graceful shutdown)"
+                );
+            }
+            None => {}
+        }
+    }
+
+    /// Insert a repo owned by `owner_did` and return its repo id. Mirrors
+    /// `test_support::seed_repo` (the `disk_path` field is unused by the
+    /// integration path — `RepoStore` computes the on-disk path from
+    /// `repos_dir`/owner/name, see [`Self::seed_bare_repo`]).
+    pub async fn seed_repo(&self, owner_did: &str, name: &str, is_public: bool) -> String {
+        let now = Utc::now();
+        let record = RepoRecord {
+            id: Uuid::new_v4().to_string(),
+            name: name.to_string(),
+            owner_did: owner_did.to_string(),
+            description: None,
+            is_public,
+            default_branch: "main".to_string(),
+            created_at: now,
+            updated_at: now,
+            disk_path: format!("/tmp/{name}"),
+            forked_from: None,
+            machine_id: None,
+        };
+        self.db.create_repo(&record).await.expect("seed repo");
+        record.id
+    }
+
+    /// Mark `branch` as protected on `repo_id`, so a non-owner push to it is
+    /// rejected by the branch-protection gate (used by the deny-prober's
+    /// protected-push probe — #195, F3).
+    pub async fn seed_protected_branch(&self, repo_id: &str, branch: &str, created_by: &str) {
+        self.db
+            .protect_branch(repo_id, branch, created_by)
+            .await
+            .expect("seed protected branch");
+    }
+
+    /// Seed an open PR `number` in `repo_id`, authored by `author_did`. The
+    /// author-or-owner close gate (`close_pr`) loads the PR *before* it runs, so a
+    /// deny-prober row for `.../pulls/{number}/close` needs the PR to exist or a
+    /// stranger gets a 404 (absent entity) instead of the 403 the gate emits.
+    pub async fn seed_pr(&self, repo_id: &str, number: i64, author_did: &str) {
+        let now = Utc::now().to_rfc3339();
+        let pr = PullRequest {
+            id: Uuid::new_v4().to_string(),
+            repo_id: repo_id.to_string(),
+            number,
+            title: "seed pr".to_string(),
+            body: None,
+            author_did: author_did.to_string(),
+            source_branch: "feature".to_string(),
+            target_branch: "main".to_string(),
+            status: "open".to_string(),
+            merged_by_did: None,
+            merged_at: None,
+            created_at: now.clone(),
+            updated_at: now,
+        };
+        self.db.create_pr(&pr).await.expect("seed pr");
+    }
+
+    /// Seed issue `issue_id` (a git-ref JSON blob authored by `author_did`) in the
+    /// repo's bare on-disk store. Like [`Self::seed_pr`], the `close_issue`
+    /// owner-or-author gate loads the issue before it runs, so the deny-prober row
+    /// for `.../issues/{id}/close` needs the issue present to reach the 403.
+    pub fn seed_issue(&self, owner_did: &str, name: &str, issue_id: &str, author_did: &str) {
+        let slug = owner_did.replace([':', '/'], "_");
+        let bare = self.repos_dir.join(&slug).join(format!("{name}.git"));
+        let json = format!(r#"{{"author":"{author_did}"}}"#);
+        crate::git::issues::create_issue(&bare, issue_id, &json).expect("seed issue");
+    }
+
+    /// Add a path-scoped visibility rule restricting `path_glob` to
+    /// `reader_dids` (empty = only the owner). Mode B keeps object SHAs intact
+    /// and withholds blob content, which is what the read/replication gates
+    /// enforce.
+    pub async fn withhold_path(
+        &self,
+        repo_id: &str,
+        path_glob: &str,
+        reader_dids: &[String],
+        created_by: &str,
+    ) {
+        self.db
+            .set_visibility_rule(
+                repo_id,
+                path_glob,
+                VisibilityMode::B,
+                reader_dids,
+                created_by,
+            )
+            .await
+            .expect("set visibility rule");
+    }
+
+    /// Record `pinned_cids` rows for every blob OID returned by
+    /// [`Self::seed_bare_repo`] so `/ipfs/{cid}` can resolve via provenance on
+    /// current main (the table is the primary lookup path). CIDs are computed
+    /// from raw object bytes exactly as the production pin path does
+    /// (`Cid::from_git_object_bytes`), not from the git oid hash.
+    /// Returns each seeded path's canonical CID for use in `GET /ipfs/{cid}`.
+    pub async fn seed_pinned_cids_for_repo(
+        &self,
+        owner_did: &str,
+        repo_name: &str,
+        repo_id: &str,
+        oids: &std::collections::HashMap<String, String>,
+    ) -> std::collections::HashMap<String, String> {
+        let slug = owner_did.replace([':', '/'], "_");
+        let bare = self.repos_dir.join(&slug).join(format!("{repo_name}.git"));
+        let mut cids = std::collections::HashMap::new();
+        for (path, oid_hex) in oids {
+            if path == "HEAD" {
+                continue;
+            }
+            let (_ty, raw) = crate::git::store::read_object(&bare, oid_hex)
+                .expect("read seeded object")
+                .expect("seeded object exists in bare repo");
+            let preliminary = gitlawb_core::cid::Cid::from_git_object_bytes(&raw).to_string();
+            let cid = preliminary
+                .parse::<cid::CidGeneric<64>>()
+                .expect("parse production pin cid")
+                .to_string();
+            self.db
+                .record_pinned_cid_with_source(oid_hex, &cid, repo_id)
+                .await
+                .expect("seed pinned_cids row");
+            let roundtrip = self
+                .db
+                .oids_for_cid(&cid)
+                .await
+                .expect("oids_for_cid after seed");
+            assert!(
+                !roundtrip.is_empty(),
+                "pinned_cids seed for {path} did not round-trip cid {cid}"
+            );
+            cids.insert(path.clone(), cid);
+        }
+        cids
+    }
+
+    /// Create a real bare git repository on disk at the exact path
+    /// `RepoStore::acquire` reads (`<repos_dir>/<owner-slug>/<name>.git`), with
+    /// one commit on `main` containing `files` (`(path, contents)`). Returns the
+    /// blob OID of each path so callers can assert a withheld OID never appears
+    /// in served bytes (U8). Shells out to `git`, mirroring
+    /// `test_support`'s served-content seam.
+    /// `object_format` is `"sha1"` for the git smart-HTTP surfaces and
+    /// `"sha256"` for the content-addressed `/ipfs/{cid}` surface (whose CIDs
+    /// are the sha2-256 object ids).
+    pub fn seed_bare_repo(
+        &self,
+        owner_did: &str,
+        name: &str,
+        files: &[(&str, &str)],
+        object_format: &str,
+    ) -> std::collections::HashMap<String, String> {
+        let run = |args: &[&str], cwd: &std::path::Path| {
+            // Test-harness-only fixture seeding, feature-gated out of the production
+            // binary; runs local git against a tempdir inside a test's own lifetime.
+            // allow-unbounded-git: test-only seeding, never on a request path holding a permit
+            let out = std::process::Command::new("git")
+                .args(args)
+                .current_dir(cwd)
+                .output() // allow-unbounded-git: test-harness fixture seeding only
+                .expect("git runs");
+            assert!(
+                out.status.success(),
+                "git {args:?} failed: {}",
+                String::from_utf8_lossy(&out.stderr)
+            );
+            String::from_utf8_lossy(&out.stdout).trim().to_string()
+        };
+
+        // Build a source work tree, then bare-clone it into the served path.
+        let src = self.repos_dir.join(format!("src-{name}"));
+        std::fs::create_dir_all(&src).expect("create src dir");
+        let fmt_arg = format!("--object-format={object_format}");
+        run(&["init", "-q", "-b", "main", &fmt_arg], &src);
+        run(&["config", "user.email", "t@t"], &src);
+        run(&["config", "user.name", "t"], &src);
+        for (path, contents) in files {
+            let full = src.join(path);
+            if let Some(parent) = full.parent() {
+                std::fs::create_dir_all(parent).expect("create file parent dir");
+            }
+            std::fs::write(&full, contents).expect("write seed file");
+            run(&["add", path], &src);
+        }
+        run(&["commit", "-q", "-m", "seed"], &src);
+
+        let mut oids = std::collections::HashMap::new();
+        for (path, _) in files {
+            let oid = run(&["rev-parse", &format!("HEAD:{path}")], &src);
+            oids.insert((*path).to_string(), oid);
+        }
+        // The HEAD commit oid, used as the `want` when driving upload-pack.
+        oids.insert("HEAD".to_string(), run(&["rev-parse", "HEAD"], &src));
+
+        let slug = owner_did.replace([':', '/'], "_");
+        let bare = self.repos_dir.join(&slug).join(format!("{name}.git"));
+        std::fs::create_dir_all(bare.parent().unwrap()).expect("create bare parent");
+        run(
+            &[
+                "clone",
+                "--bare",
+                "-q",
+                src.to_str().unwrap(),
+                bare.to_str().unwrap(),
+            ],
+            &self.repos_dir,
+        );
+
+        oids
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{join_or_abort, ServeTeardown};
+    use std::time::Duration;
+
+    /// A wedged serve task (never returns within the window) must be ABORTED and
+    /// reaped on timeout, not detached. The previous teardown moved the
+    /// `JoinHandle` into `timeout` and dropped it on elapse, which detaches the
+    /// still-running task and leaks the test DB across `DROP DATABASE` (#194 F2).
+    /// Load-bearing: neutralize the abort in `join_or_abort` (return without
+    /// abort+await) and this flips RED — `handle.is_finished()` stays false
+    /// because the task was left running.
+    #[tokio::test]
+    async fn join_or_abort_aborts_a_wedged_task_on_timeout() {
+        let mut handle: tokio::task::JoinHandle<std::io::Result<()>> = tokio::spawn(async {
+            loop {
+                tokio::time::sleep(Duration::from_secs(3600)).await;
+            }
+        });
+        assert!(
+            matches!(
+                join_or_abort(&mut handle, Duration::from_millis(50)).await,
+                ServeTeardown::TimedOutAborted
+            ),
+            "a task that never returns must be reported timed-out, not graceful"
+        );
+        assert!(
+            handle.is_finished(),
+            "the wedged serve task must be aborted and reaped, not detached"
+        );
+    }
+
+    /// A task that returns within the window is reported graceful, carrying its
+    /// `io::Result` — the normal teardown path.
+    #[tokio::test]
+    async fn join_or_abort_reports_graceful_when_task_exits_in_time() {
+        let mut handle: tokio::task::JoinHandle<std::io::Result<()>> =
+            tokio::spawn(async { Ok(()) });
+        assert!(
+            matches!(
+                join_or_abort(&mut handle, Duration::from_secs(5)).await,
+                ServeTeardown::Graceful(Ok(()))
+            ),
+            "a prompt clean exit must be reported graceful"
+        );
+    }
+}

crates/gitlawb-node/src/test_support.rs

@@ -103,7 +103,6 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
         rate_limiter: RateLimiter::new(100, Duration::from_secs(60)),
         create_ip_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)),
         push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
-        close_issue_rate_limiter: RateLimiter::new(120, Duration::from_secs(3600)),
         ipfs_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
         ipfs_work_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
         ipfs_max_history_walks: crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST,
@@ -558,35 +557,6 @@ mod tests {
         );
     }
 
-    /// Fork disk paths must go through `validated_repo_disk_path` so a user-supplied
-    /// name cannot reach `remove_dir_all` on an escaped path (CodeQL path-injection).
-    #[sqlx::test]
-    async fn fork_rejects_name_that_fails_validated_disk_path(pool: PgPool) {
-        let owner = "did:key:zFORKOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
-        let state = test_state(pool).await;
-        let repo = seed_repo(owner, "fork-src");
-        state.db.create_repo(&repo).await.expect("seed repo");
-
-        let router = Router::new()
-            .route(
-                "/api/v1/repos/{owner}/{repo}/fork",
-                axum::routing::post(crate::api::repos::fork_repo),
-            )
-            .with_state(state.clone());
-        let too_long = "a".repeat(101);
-        let uri = format!("/api/v1/repos/{owner}/fork-src/fork");
-        let body = Body::from(format!(r#"{{"name":"{too_long}"}}"#));
-        let resp = router
-            .oneshot(signed_request_as(owner, Method::POST, &uri, body))
-            .await
-            .unwrap();
-        assert_eq!(
-            resp.status(),
-            StatusCode::BAD_REQUEST,
-            "fork names that fail validated_repo_disk_path must be rejected before clone"
-        );
-    }
-
     /// N13: the task handlers bind the acting DID to the signer. A caller signed
     /// as B claiming delegator_did A is rejected before any DB write (DB-free).
     #[sqlx::test]
@@ -15966,6 +15936,65 @@ mod tests {
             use super::*;
             use crate::api::repos::drain_faults;
 
+            /// Process-wide tracing capture so a test can assert the give-up is logged at
+            /// ERROR. A global default subscriber can only be installed once per process,
+            /// so it is shared by every test here and assertions filter on the repo id,
+            /// which is a fresh uuid per test.
+            mod logcap {
+                use std::sync::{Arc, Mutex, OnceLock};
+                use tracing::{Event, Level, Subscriber};
+                use tracing_subscriber::layer::{Context, Layer};
+                use tracing_subscriber::prelude::*;
+
+                type Lines = Arc<Mutex<Vec<(Level, String)>>>;
+
+                fn lines() -> &'static Lines {
+                    static LINES: OnceLock<Lines> = OnceLock::new();
+                    LINES.get_or_init(|| Arc::new(Mutex::new(Vec::new())))
+                }
+
+                struct Capture;
+                impl<S: Subscriber> Layer<S> for Capture {
+                    fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) {
+                        struct V(String);
+                        impl tracing::field::Visit for V {
+                            fn record_debug(
+                                &mut self,
+                                field: &tracing::field::Field,
+                                value: &dyn std::fmt::Debug,
+                            ) {
+                                self.0.push_str(&format!(" {}={:?}", field.name(), value));
+                            }
+                        }
+                        let mut v = V(String::new());
+                        event.record(&mut v);
+                        lines()
+                            .lock()
+                            .unwrap()
+                            .push((*event.metadata().level(), v.0));
+                    }
+                }
+
+                pub(super) fn install() {
+                    static ONCE: OnceLock<()> = OnceLock::new();
+                    ONCE.get_or_init(|| {
+                        let _ = tracing::subscriber::set_global_default(
+                            tracing_subscriber::registry().with(Capture),
+                        );
+                    });
+                }
+
+                pub(super) fn errors_containing(needle: &str) -> Vec<String> {
+                    lines()
+                        .lock()
+                        .unwrap()
+                        .iter()
+                        .filter(|(lvl, msg)| *lvl == Level::ERROR && msg.contains(needle))
+                        .map(|(_, msg)| msg.clone())
+                        .collect()
+                }
+            }
+
             /// SCENARIO 1. The repo re-read fails once, then succeeds: the drain lap
             /// must still RUN, under the refreshed state, and pin the coalesced push's
             /// object. RED before the fix (the single `Err` returned `None`, the lap
@@ -16031,11 +16060,11 @@ mod tests {
             }
 
             /// SCENARIO 2. Every re-read attempt fails: the loop must give up on a BOUND
-            /// (asserted as a literal, so raising or removing the bound goes RED) and hit
-            /// the give-up path that logs at ERROR in production (asserted here via the
-            /// drain_faults seam, which fires on the same branch as that log).
+            /// (asserted as a literal, so raising or removing the bound goes RED) and log
+            /// the give-up at ERROR so the residual loss is observable rather than silent.
             #[sqlx::test]
             async fn u2_sustained_repo_reread_failure_is_bounded_and_logged(pool: PgPool) {
+                logcap::install();
                 let state = test_state(pool).await;
                 let owner = new_did();
                 let repo = seed_repo(&owner, "u2-bounded");
@@ -16086,10 +16115,11 @@ mod tests {
                     !state.db.is_pinned(&obj2).await.unwrap(),
                     "with the read never succeeding there is nothing fresh to act on"
                 );
+                let errs = logcap::errors_containing(&repo.id);
                 assert!(
-                    drain_faults::reread_exhausted(&repo.id),
-                    "the exhausted drain re-read must hit the give-up path that logs at \
-                     ERROR in production"
+                    !errs.is_empty(),
+                    "the exhausted drain re-read is logged at ERROR with the repo id, so \
+                     the residual work loss is observable; captured: {errs:?}"
                 );
                 assert!(
                     state.encrypt_inflight.is_empty(),

crates/gitlawb-node/tests/inv22_gates.rs

@@ -225,8 +225,7 @@ fn f4_release_keeps_conn_owned_until_unlock_resolves() {
     let repo_store = src("git/repo_store.rs");
 
     let rel_start = repo_store
-        .find("pub async fn release(self")
-        .or_else(|| repo_store.find("pub async fn release(mut self"))
+        .find("pub async fn release(mut self")
         .expect("F4 gate: repo_store.rs no longer defines RepoWriteGuard::release");
     let rel_end = repo_store[rel_start..]
         .find("impl Drop for RepoWriteGuard")

crates/gitlawb-node/tests/support/assert.rs

@@ -0,0 +1,237 @@
+//! INV-8 deny assertion. A denial must be an actual refusal (4xx) whose body
+//! and headers carry none of the data it is withholding, and must never be a 2xx
+//! (an empty-200 rendered as success is the denial-as-success bug this guards).
+//!
+//! The check is split into a pure `check_denied` (unit-tested below with the
+//! self-check scenarios) and an async `assert_denied` wrapper that reads a real
+//! `reqwest::Response`.
+
+/// Pure deny check. Returns `Err(reason)` when the response is not a clean
+/// denial. `withheld` are exact tokens (OIDs, paths, private slugs) that must
+/// not appear anywhere in the body; register short-OID prefixes and encoded
+/// path forms as separate tokens at the call site (a raw substring scan cannot
+/// normalize them).
+pub fn check_denied(
+    status: u16,
+    body: &str,
+    expected: u16,
+    withheld: &[&str],
+) -> Result<(), String> {
+    // The harness only ever expects denials; a non-4xx expectation is a
+    // programming error in the test, not a node behavior under test.
+    if !(400..500).contains(&expected) {
+        return Err(format!(
+            "test bug: assert_denied expected status {expected} is not a 4xx"
+        ));
+    }
+    // INV-8: a denial rendered as success is the headline bug. Catch it
+    // explicitly so the failure message names it, rather than only failing the
+    // status-equality check below.
+    if (200..300).contains(&status) {
+        return Err(format!(
+            "denial rendered as success: got {status}, expected denial {expected}. \
+             body={body:?}"
+        ));
+    }
+    if status != expected {
+        return Err(format!(
+            "expected denial status {expected}, got {status}. body={body:?}"
+        ));
+    }
+    for token in withheld {
+        if token.is_empty() {
+            continue;
+        }
+        if body.contains(token) {
+            return Err(format!(
+                "withheld token {token:?} leaked in denial body: {body:?}"
+            ));
+        }
+    }
+    Ok(())
+}
+
+/// Like [`check_denied`] but also scans serialized response headers for withheld
+/// tokens. Call sites that only have a body should keep using `check_denied`.
+pub fn check_denied_with_headers(
+    status: u16,
+    body: &str,
+    header_text: &str,
+    expected: u16,
+    withheld: &[&str],
+) -> Result<(), String> {
+    if let Err(e) = check_denied(status, body, expected, withheld) {
+        return Err(e);
+    }
+    for token in withheld {
+        if token.is_empty() {
+            continue;
+        }
+        if header_text.contains(token) {
+            return Err(format!(
+                "withheld token {token:?} leaked in denial headers: {header_text:?}"
+            ));
+        }
+    }
+    Ok(())
+}
+
+/// Replace each withheld token in `s` with `[REDACTED]` so a test-failure
+/// witness does not itself leak the secret it caught.
+fn redact_withheld(s: &str, withheld: &[&str]) -> String {
+    let mut result = s.to_string();
+    for token in withheld {
+        if !token.is_empty() {
+            result = result.replace(token, "[REDACTED]");
+        }
+    }
+    result
+}
+
+/// Drive a real response through the deny check and panic on failure. Reads the
+/// full body once.
+pub async fn assert_denied(resp: reqwest::Response, expected: u16, withheld: &[&str]) {
+    let status = resp.status().as_u16();
+    let header_text = resp
+        .headers()
+        .iter()
+        .filter_map(|(name, value)| {
+            value
+                .to_str()
+                .ok()
+                .map(|v| format!("{}: {}", name.as_str(), v))
+        })
+        .collect::<Vec<_>>()
+        .join("\n");
+    // A body-read failure must NOT fold to "" — an empty body passes the withheld
+    // scan vacuously, so an unreadable denial (a mid-body reset/truncation) would be
+    // certified leak-free without ever inspecting the bytes. Fail loud instead. A
+    // legitimately empty body is `Ok("")`, not `Err`, so the honest empty-401 denials
+    // still pass.
+    let body = resp
+        .text()
+        .await
+        .expect("read denial body: an unreadable denial cannot be certified leak-free");
+    if let Err(reason) = check_denied_with_headers(status, &body, &header_text, expected, withheld)
+    {
+        // Panic with a fixed string: CodeQL's taint analysis traces withheld
+        // tokens from check_denied through redact_withheld into the panic sink,
+        // even though redact_withheld strips them at runtime. A fixed message
+        // keeps no tainted data on the sink path. The test name identifies
+        // which probe failed; check_denied's unit tests cover the error detail.
+        let _ = redact_withheld(&reason, withheld);
+        panic!("deny assertion failed: status/body/header leak check");
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{check_denied, check_denied_with_headers, redact_withheld};
+
+    #[test]
+    fn clean_403_with_no_leak_passes() {
+        let r = check_denied(
+            403,
+            r#"{"error":"only the repo owner can do this"}"#,
+            403,
+            &["a1b2c3"],
+        );
+        assert!(r.is_ok(), "{r:?}");
+    }
+
+    #[test]
+    fn empty_200_is_flagged_as_denial_rendered_as_success() {
+        let r = check_denied(200, "", 403, &[]);
+        assert!(r.is_err());
+        assert!(r.unwrap_err().contains("rendered as success"));
+    }
+
+    #[test]
+    fn leaking_403_fails_and_names_the_token() {
+        let withheld = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+        let body = format!("not found near object {withheld}");
+        let r = check_denied(403, &body, 403, &[withheld]);
+        assert!(r.is_err());
+        assert!(r.unwrap_err().contains(withheld));
+    }
+
+    #[test]
+    fn wrong_status_fails() {
+        // A 404 when the owner gate's 403 was expected must not pass: it means
+        // an earlier layer denied, so the gate under test was never exercised.
+        let r = check_denied(404, "not found", 403, &[]);
+        assert!(r.is_err());
+        assert!(r.unwrap_err().contains("expected denial status 403"));
+    }
+
+    #[test]
+    fn non_4xx_expected_is_a_test_bug() {
+        // Guards the harness itself: expecting a non-denial status is a test
+        // authoring error, not a node behavior, and must be rejected.
+        let r = check_denied(403, "denied", 200, &[]);
+        assert!(r.is_err());
+        assert!(r.unwrap_err().contains("not a 4xx"));
+    }
+
+    #[test]
+    fn empty_withheld_token_never_false_positives() {
+        // An empty token must be skipped, never treated as "contained in every
+        // body" — otherwise every denial would spuriously fail as a leak.
+        let r = check_denied(403, "any body", 403, &[""]);
+        assert!(r.is_ok(), "{r:?}");
+    }
+
+    #[test]
+    fn leaking_header_fails_and_names_the_token() {
+        let withheld = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+        let headers = format!("X-Debug: object {withheld}");
+        let r = check_denied_with_headers(
+            404,
+            r#"{"error":"repository not found"}"#,
+            &headers,
+            404,
+            &[withheld],
+        );
+        assert!(r.is_err());
+        let err = r.unwrap_err();
+        assert!(err.contains("leaked in denial headers"));
+        assert!(err.contains(withheld));
+    }
+
+    #[test]
+    fn clean_headers_with_clean_body_passes() {
+        let withheld = "a1b2c3";
+        let r = check_denied_with_headers(
+            404,
+            r#"{"error":"repository not found"}"#,
+            "Content-Type: application/json",
+            404,
+            &[withheld],
+        );
+        assert!(r.is_ok(), "{r:?}");
+    }
+
+    #[test]
+    fn redact_withheld_replaces_secrets() {
+        let withheld = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+        let msg = format!("withheld token {withheld:?} leaked in body: near {withheld}");
+        let redacted = redact_withheld(&msg, &[withheld]);
+        assert!(
+            !redacted.contains(withheld),
+            "token must be redacted: {redacted}"
+        );
+        assert!(
+            redacted.contains("[REDACTED]"),
+            "redaction marker must appear: {redacted}"
+        );
+    }
+
+    #[test]
+    fn redact_withheld_leaves_clean_strings() {
+        let result = redact_withheld(
+            "no secrets here",
+            &["deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"],
+        );
+        assert_eq!(result, "no secrets here");
+    }
+}

crates/gitlawb-node/tests/support/mod.rs

@@ -0,0 +1,33 @@
+//! Shared support for the real-node deny harness: the deny assertion (U4), the
+//! RFC-9421 signing client (U2), and (via `gitlawb_node::test_harness`) the
+//! node spawner (U3).
+
+pub mod assert;
+// U2 fixture state matrix + per-gate-class probe generators; driven by U3.
+#[allow(dead_code)]
+pub mod probe;
+// U1 deny-bearing route registry; fields consumed by U2/U3 as they land.
+#[allow(dead_code)]
+pub mod routes;
+pub mod signing;
+
+/// A reqwest client with a bounded request timeout, so a wedged git subprocess or
+/// route fails the suite rather than hanging it until CI kills the job (#195, F1).
+/// The two real-node probes that drive git subprocesses use this; the inline
+/// twins at the upload-pack and registry-sweep sites use the same pattern.
+#[allow(dead_code)]
+pub fn bounded_client() -> reqwest::Client {
+    reqwest::Client::builder()
+        .timeout(std::time::Duration::from_secs(30))
+        .build()
+        .unwrap()
+}
+
+/// Build an anonymous (unsigned) GET request for `path` under `base_url`.
+/// Extracted from inline call sites so URL construction goes through a
+/// function boundary, matching the `signed_request` pattern in `support::signing`.
+#[allow(dead_code)]
+pub fn anon_get(client: &reqwest::Client, base_url: &str, path: &str) -> reqwest::RequestBuilder {
+    let url = format!("{base_url}{path}");
+    client.get(url)
+}

crates/gitlawb-node/tests/support/signing.rs

@@ -0,0 +1,32 @@
+//! Real RFC-9421 request signing for the deny harness (U2). Wraps
+//! `gitlawb_core::http_sig::sign_request` (the same entry point the
+//! git-remote-gitlawb helper uses in production) so a test can present a valid
+//! signature for any identity, reaching the authenticated-but-wrong-owner path
+//! that an injected-DID shortcut cannot.
+
+use gitlawb_core::http_sig::sign_request;
+use gitlawb_core::identity::Keypair;
+
+/// Build a reqwest request carrying a valid RFC-9421 signature for `keypair`.
+///
+/// `path` is both appended to `base_url` to form the request URL and signed as
+/// the `@path` component, so the value the server reconstructs and the value
+/// that was signed are guaranteed identical. `body` is signed via its
+/// content-digest and sent as the request body.
+pub fn signed_request(
+    client: &reqwest::Client,
+    method: reqwest::Method,
+    base_url: &str,
+    path: &str,
+    body: Vec<u8>,
+    keypair: &Keypair,
+) -> reqwest::RequestBuilder {
+    let sig = sign_request(keypair, method.as_str(), path, &body);
+    let url = format!("{base_url}{path}");
+    client
+        .request(method, url)
+        .header("content-digest", sig.content_digest)
+        .header("signature-input", sig.signature_input)
+        .header("signature", sig.signature)
+        .body(body)
+}