fix(node): satisfy clippy on stale-local download test path join

Under-lock GET failures now always shed as RepoUnavailable, even when a local
copy exists, so a transient download error cannot publish over a newer stored
generation. PublishDurabilitySlot only synthesizes UploadUnknowable after
release starts, and receive-pack marks that boundary before spawning the tail.

Adds regression tests for both paths; inv22 F4 gate accepts the release wrapper.

.env.example

@@ -24,11 +24,26 @@ 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. 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).
+# remembering admin tooling opens its own pool.
 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
@@ -103,13 +118,6 @@ GITLAWB_REQUIRE_SIGNED_PEER_WRITES=false
 # is on. See docs/RUN-A-NODE.md.
 GITLAWB_ENFORCE_OWNER_PUSH=true
 
-# Require a `nonce` parameter on signatures that authorize a write. Without one
-# the replay ledger keys on the signing-string hash, which rejects a repeated
-# byte-identical mutation inside the same second. Keep false until every client
-# signs a nonce; with this and GITLAWB_REQUIRE_SIGNED_PEER_WRITES both true, a
-# peer still on a pre-nonce binary can no longer write to this node.
-GITLAWB_REQUIRE_SIGNATURE_NONCE=false
-
 # Comma-separated libp2p multiaddrs.
 # Example: /ip4/1.2.3.4/udp/7546/quic-v1/p2p/12D3KooW...
 GITLAWB_P2P_BOOTSTRAP=
@@ -288,17 +296,6 @@ GITLAWB_IPFS_RATE_LIMIT=600
 # the client IP. 0 disables. Default 120.
 GITLAWB_CREATE_RATE_LIMIT=120
 
-# ── Signed-write rate limiting (per client IP, uses GITLAWB_TRUSTED_PROXY) ────
-# Max requests per client IP per hour on the write routes that require an HTTP
-# Signature (tasks, PR merge/close/review/comment, webhooks, branch protection,
-# stars, replicas, labels, visibility, agent deregistration, bounties, profile,
-# issue close/comment). Every signed attempt spends a consumed_signatures row
-# before the handler checks ownership or existence, and any freshly minted
-# did:key can sign, so this brake is what bounds the durable writes an
-# unregistered caller can force. Separate bucket from the creation, push and
-# peer-sync limits. 0 disables. Default 600.
-GITLAWB_SIGNED_WRITE_RATE_LIMIT=600
-
 # ── Peer-sync rate limiting (per client IP, uses GITLAWB_TRUSTED_PROXY below) ─
 # /api/v1/peers/announce and /api/v1/sync/notify accept unsigned requests from
 # known peers and run at higher frequency, so a generous bucket. Separate from

Cargo.lock

@@ -3467,7 +3467,6 @@ dependencies = [
  "libp2p-swarm",
  "mockito",
  "multiaddr",
- "multibase",
  "prometheus",
  "reqwest",
  "serde",

README.md

@@ -369,14 +369,6 @@ GITLAWB_REQUIRE_SIGNED_PEER_WRITES=true
 
 `POST /api/v1/sync/trigger` is not part of the staged rollout: it always requires a signature in both config modes and returns 401 without one, because each call drives an O(peers) outbound fan-out.
 
-A verified signature is spent once on the write routes that require one: repo, issue, PR, task, bounty, profile, webhook, label, star, replica, visibility and agent mutations, plus git push and `POST /api/v1/sync/trigger`. A captured request to any of those cannot be replayed. The two peer-write routes are the exception. `POST /api/v1/peers/announce` and `POST /api/v1/sync/notify` reach the ledger only when `GITLAWB_REQUIRE_SIGNED_PEER_WRITES=true`; under the default (`false`) they verify a signature when the headers are present but never spend it, so a captured signed peer write is still replayable there until the fleet upgrades and the flag is turned on. The ledger keys on the RFC 9421 `nonce` parameter when the client signs one and falls back to the signing-string hash when it does not. The fallback is correct but coarser: a client that repeats a byte-identical mutation within the same second is rejected as a replay and has to re-sign. `GITLAWB_REQUIRE_SIGNATURE_NONCE` closes that fallback once every client emits a nonce:
-
-```bash
-GITLAWB_REQUIRE_SIGNATURE_NONCE=true
-```
-
-With it on, a nonce-less signature on a write route is rejected with 400 and `X-Gitlawb-Error: signature_nonce_required`. Signed reads are never affected. Flipping it on has a federation precondition: three of the node's own outbound signed calls (peer announce, replica registration, sync-notify) are peer traffic, so a node running with both this and `GITLAWB_REQUIRE_SIGNED_PEER_WRITES` set to true will reject peer writes from any node still on a pre-nonce binary. Upgrade the fleet before enabling it.
-
 ---
 
 ## Configuration
@@ -401,9 +393,6 @@ Important node settings:
 | `GITLAWB_P2P_BOOTSTRAP` | Comma-separated libp2p multiaddrs. |
 | `GITLAWB_BOOTSTRAP_DISABLE_SEEDS` | Disable embedded seed peers for isolated dev/test networks. |
 | `GITLAWB_REQUIRE_SIGNED_PEER_WRITES` | Require signed peer announce/sync writes. Defaults to `false` during the staged rollout below. |
-| `GITLAWB_REQUIRE_SIGNATURE_NONCE` | Require a signed `nonce` on write routes. Default false; see the staged rollout note above. |
-| `GITLAWB_SIGNED_WRITE_RATE_LIMIT` | Per-client-IP requests per hour on the signed write routes (tasks, PR merge/close/review/comment, webhooks, branch protection, stars, replicas, labels, visibility, agent deregistration, bounties, profile, issue close/comment). Each signed attempt spends a ledger row before the handler checks anything, so the brake bounds what an unregistered caller can write. Own bucket; `0` disables. Default 600. |
-<!-- verbatim -->
 | `GITLAWB_ENFORCE_OWNER_PUSH` | Require the authenticated pusher to be the repo owner on `git-receive-pack`. **Defaults to `true`.** A `did:key` signature is authentication, not authorization — anyone can mint a key and sign — so with this off every signed caller may push to every repository, private ones included. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on. Set `false` only for a rolling upgrade; see [`docs/RUN-A-NODE.md`](docs/RUN-A-NODE.md). |
 | `GITLAWB_AUTO_SYNC` | Enable automatic sync from known peers. |
 | `GITLAWB_MAX_PACK_BYTES` | Max git pack body size for smart-HTTP routes. |

crates/gitlawb-core/src/http_sig.rs

@@ -8,16 +8,11 @@
 //!
 //! RFC 9421 headers produced by `sign_request`:
 //!   Content-Digest:   sha-256=:base64hash:
-//!   Signature-Input:  sig1=("@method" "@path" "content-digest");keyid="did:key:z6Mk...";alg="ed25519";created=<unix>;nonce="<32 hex chars>"
+//!   Signature-Input:  sig1=("@method" "@path" "content-digest");keyid="did:key:z6Mk...";alg="ed25519";created=<unix>
 //!   Signature:        sig1=:base64signature:
-//!
-//! The `nonce` is a per-request 128-bit value. It sits in the parameter tail,
-//! which is what `@signature-params` is built from, so it is covered by the
-//! signature without appearing in the covered-component list.
 
 use base64::{engine::general_purpose::STANDARD, Engine};
 use chrono::Utc;
-use rand::{rngs::OsRng, RngCore};
 use sha2::{Digest, Sha256};
 use std::collections::HashMap;
 
@@ -28,28 +23,15 @@ use crate::{Error, Result};
 /// The component identifiers covered by every gitlawb signature.
 pub const COVERED_COMPONENTS: &[&str] = &["@method", "@path", "content-digest"];
 
-/// How old a signature may be and still verify.
-///
-/// Do not shrink this without measuring large-pack pushes: the verifier
-/// buffers the entire request body before checking `created`, so a multi-GB
-/// pack spends its upload time inside this budget.
-pub const MAX_SIGNATURE_AGE_SECS: i64 = 300;
-
-/// How far into the future a signature's `created` may sit. Absorbs ordinary
-/// clock skew without letting a signer pre-date its way to a wider window.
-pub const MAX_FUTURE_SKEW_SECS: i64 = 30;
-
 /// The three headers produced by RFC 9421 signing.
 #[derive(Debug, Clone)]
 pub struct SignedHeaders {
     /// `Content-Digest: sha-256=:base64:`
     pub content_digest: String,
-    /// `Signature-Input: sig1=(...);keyid="...";alg="ed25519";created=<unix>;nonce="..."`
+    /// `Signature-Input: sig1=(...);keyid="...";alg="ed25519";created=<unix>`
     pub signature_input: String,
     /// `Signature: sig1=:base64:`
     pub signature: String,
-    /// The per-request nonce carried in `Signature-Input`.
-    pub nonce: String,
 }
 
 /// A parsed RFC 9421 signature (from Signature-Input + Signature headers).
@@ -60,8 +42,6 @@ pub struct HttpSignature {
     pub created: i64,
     pub components: Vec<String>,
     pub signature_bytes: Vec<u8>,
-    /// The `nonce` param, absent on signatures from a pre-nonce signer.
-    pub nonce: Option<String>,
 }
 
 impl HttpSignature {
@@ -84,15 +64,6 @@ impl HttpSignature {
             .find(')')
             .ok_or_else(|| Error::HttpSignature("missing ')' in Signature-Input".into()))?;
 
-        // Both parentheses are located independently, so a header carrying them
-        // out of order (`sig1=)(`) would build a reversed range and panic. This
-        // runs on unauthenticated header text, so it has to be a rejection.
-        if close < open {
-            return Err(Error::HttpSignature(
-                "'(' must precede ')' in Signature-Input".into(),
-            ));
-        }
-
         let components_str = &rest[open + 1..close];
         let params_str = &rest[close + 1..]; // starts with ';'
 
@@ -122,9 +93,6 @@ impl HttpSignature {
             .parse()
             .map_err(|_| Error::HttpSignature("invalid created timestamp".into()))?;
 
-        // Optional: absent on signatures from a signer that predates the nonce.
-        let nonce = params.get("nonce").map(|v| v.trim_matches('"').to_string());
-
         // Signature: sig1=:base64bytes:
         let sig_b64 = sig_header
             .trim()
@@ -142,39 +110,18 @@ impl HttpSignature {
             created,
             components,
             signature_bytes,
-            nonce,
         })
     }
 
-    /// Reject a `created` timestamp outside the acceptance window.
-    ///
-    /// The window is deliberately asymmetric. A signature may be up to
-    /// [`MAX_SIGNATURE_AGE_SECS`] old, because the verifier buffers the whole
-    /// request body before this check runs and the git routes accept packs up
-    /// to gigabytes, so upload time is charged against that budget. It may be
-    /// only [`MAX_FUTURE_SKEW_SECS`] in the future, which is enough to absorb
-    /// ordinary clock skew.
-    ///
-    /// This used to compare `(now - created).abs()` against a single bound,
-    /// which let a signer pre-date `created` and hold a signature valid for
-    /// twice the intended span (#253).
+    /// Reject if the `created` timestamp is more than 5 minutes from now.
     pub fn check_created(&self) -> Result<()> {
         let now = Utc::now().timestamp();
-
-        if self.created > now + MAX_FUTURE_SKEW_SECS {
+        let skew = (now - self.created).abs();
+        if skew > 300 {
             return Err(Error::HttpSignature(format!(
-                "signature is dated {}s in the future (max {MAX_FUTURE_SKEW_SECS}s)",
-                self.created - now
+                "clock skew too large: {skew}s (max 300s)"
             )));
         }
-
-        let age = now - self.created;
-        if age > MAX_SIGNATURE_AGE_SECS {
-            return Err(Error::HttpSignature(format!(
-                "signature is {age}s old (max {MAX_SIGNATURE_AGE_SECS}s)"
-            )));
-        }
-
         Ok(())
     }
 
@@ -211,36 +158,6 @@ pub fn build_signing_string(
     Ok(lines.join("\n"))
 }
 
-/// Render `COVERED_COMPONENTS` as the parenthesised component list that goes
-/// inside `Signature-Input`, e.g. `"@method" "@path" "content-digest"`.
-///
-/// Both the wire header and [`build_signing_string`]'s input are built from
-/// this one source. Writing the list out by hand in the header while passing
-/// the const to the signing-string builder lets a client sign over one set
-/// while advertising another, which no verifier can reconcile.
-fn covered_components_list() -> String {
-    COVERED_COMPONENTS
-        .iter()
-        .map(|c| format!("\"{c}\""))
-        .collect::<Vec<_>>()
-        .join(" ")
-}
-
-/// The request-derived value for every component this signer knows how to
-/// cover. Kept alongside [`covered_components_list`] so the guard test can
-/// check the two agree without signing anything.
-fn request_values_for(
-    method: &str,
-    path_and_query: &str,
-    content_digest: &str,
-) -> HashMap<String, String> {
-    let mut values = HashMap::new();
-    values.insert("@method".to_string(), method.to_uppercase());
-    values.insert("@path".to_string(), path_and_query.to_string());
-    values.insert("content-digest".to_string(), content_digest.to_string());
-    values
-}
-
 /// Sign an HTTP request per RFC 9421 and return the three headers to inject.
 pub fn sign_request(
     keypair: &Keypair,
@@ -252,32 +169,22 @@ pub fn sign_request(
     let content_digest = compute_content_digest(body);
     let did = keypair.did();
 
-    // Full Signature-Input header value. The advertised component list is
-    // derived from COVERED_COMPONENTS rather than written out, so the wire
-    // header and the list we actually sign over cannot drift apart.
-    let advertised = covered_components_list();
-    // 128 bits from the OS CSPRNG, appended to the parameter tail. The tail is
-    // sliced into `sig_params_value` below and so lands in the
-    // `@signature-params` line, which puts the nonce under the signature
-    // (RFC 9421 §2.3) without touching COVERED_COMPONENTS. Verifiers that
-    // predate the nonce rebuild `@signature-params` from the received header
-    // text, so they keep verifying an unknown parameter unchanged.
-    let nonce = generate_nonce();
+    // Full Signature-Input header value
     let signature_input = format!(
-        r#"sig1=({advertised});keyid="{did}";alg="ed25519";created={created};nonce="{nonce}""#
+        r#"sig1=("@method" "@path" "content-digest");keyid="{did}";alg="ed25519";created={created}"#
     );
 
     // The @signature-params component value is the part after "sig1="
     let sig_params_value = &signature_input["sig1=".len()..];
 
-    let request_values = request_values_for(method, path_and_query, &content_digest);
+    let mut request_values = HashMap::new();
+    request_values.insert("@method".to_string(), method.to_uppercase());
+    request_values.insert("@path".to_string(), path_and_query.to_string());
+    request_values.insert("content-digest".to_string(), content_digest.clone());
 
-    // Guarded by `sign_request_supplies_every_covered_component`: that test
-    // fails cleanly if COVERED_COMPONENTS gains an entry `request_values_for`
-    // cannot supply, which is the only way this could fail.
     let signing_string =
         build_signing_string(COVERED_COMPONENTS, sig_params_value, &request_values)
-            .expect("request_values_for covers COVERED_COMPONENTS (see guard test)");
+            .expect("required components always present when building");
 
     let sig_bytes = keypair.sign(signing_string.as_bytes());
     let sig_b64 = STANDARD.encode(sig_bytes.to_bytes());
@@ -286,17 +193,9 @@ pub fn sign_request(
         content_digest,
         signature_input,
         signature: format!("sig1=:{sig_b64}:"),
-        nonce,
     }
 }
 
-/// Draw a fresh 128-bit nonce from the OS CSPRNG, hex-encoded.
-fn generate_nonce() -> String {
-    let mut bytes = [0u8; 16];
-    OsRng.fill_bytes(&mut bytes);
-    hex::encode(bytes)
-}
-
 /// Compute RFC 9421 Content-Digest value: `sha-256=:base64(sha256(body)):`
 pub fn compute_content_digest(body: &[u8]) -> String {
     let mut hasher = Sha256::new();
@@ -424,95 +323,6 @@ mod tests {
         assert!(d.len() > 12);
     }
 
-    /// U1 guard: the component list on the wire must be exactly what we sign
-    /// over. Load-bearing by mutation, not by red-then-green: adding a fourth
-    /// entry to COVERED_COMPONENTS turns this red.
-    #[test]
-    fn signature_input_advertises_exactly_covered_components() {
-        let kp = Keypair::generate();
-        let headers = sign_request(&kp, "POST", "/api/test", b"body");
-        let parsed = HttpSignature::parse(&headers.signature_input, &headers.signature)
-            .expect("emitted Signature-Input must parse");
-        assert_eq!(
-            parsed.components, COVERED_COMPONENTS,
-            "the advertised component list drifted from COVERED_COMPONENTS"
-        );
-    }
-
-    /// U1 guard: every covered component must have a request-derived value.
-    /// This is the check that fails *cleanly* when COVERED_COMPONENTS grows,
-    /// instead of letting sign_request panic inside build_signing_string.
-    #[test]
-    fn sign_request_supplies_every_covered_component() {
-        let values = request_values_for("POST", "/api/test", "sha-256=:abc:");
-        let missing: Vec<&str> = COVERED_COMPONENTS
-            .iter()
-            .copied()
-            .filter(|c| !values.contains_key(*c))
-            .collect();
-        assert!(
-            missing.is_empty(),
-            "COVERED_COMPONENTS entries with no value in request_values_for: {missing:?}. \
-             sign_request would panic on every call; add the value or drop the component."
-        );
-    }
-
-    /// U3 boundary: the forward bound is real and tight.
-    #[test]
-    fn future_dated_created_outside_skew_is_rejected() {
-        let kp = Keypair::generate();
-        let mut headers = sign_request(&kp, "GET", "/api/v1/agents", b"");
-        let future = Utc::now().timestamp() + MAX_FUTURE_SKEW_SECS + 1;
-        headers.signature_input = headers
-            .signature_input
-            .split(";created=")
-            .next()
-            .map(|head| format!("{head};created={future}"))
-            .expect("signature_input carries created");
-        let sig = HttpSignature::parse(&headers.signature_input, &headers.signature).unwrap();
-        assert!(
-            sig.check_created().is_err(),
-            "created beyond the forward skew allowance must be rejected"
-        );
-    }
-
-    /// U3 boundary: just inside the forward allowance still verifies, so the
-    /// bound absorbs real clock skew rather than being effectively zero.
-    #[test]
-    fn future_dated_created_within_skew_is_accepted() {
-        let kp = Keypair::generate();
-        let mut headers = sign_request(&kp, "GET", "/api/v1/agents", b"");
-        let future = Utc::now().timestamp() + MAX_FUTURE_SKEW_SECS - 1;
-        headers.signature_input = headers
-            .signature_input
-            .split(";created=")
-            .next()
-            .map(|head| format!("{head};created={future}"))
-            .expect("signature_input carries created");
-        let sig = HttpSignature::parse(&headers.signature_input, &headers.signature).unwrap();
-        assert!(sig.check_created().is_ok(), "within skew must be accepted");
-    }
-
-    /// U3 regression: the backward budget must not be narrowed. A large pack
-    /// spends this long uploading before check_created ever runs.
-    #[test]
-    fn backward_budget_still_accepts_a_nearly_stale_signature() {
-        let kp = Keypair::generate();
-        let mut headers = sign_request(&kp, "POST", "/api/test", b"body");
-        let old = Utc::now().timestamp() - (MAX_SIGNATURE_AGE_SECS - 20);
-        headers.signature_input = headers
-            .signature_input
-            .split(";created=")
-            .next()
-            .map(|head| format!("{head};created={old}"))
-            .expect("signature_input carries created");
-        let sig = HttpSignature::parse(&headers.signature_input, &headers.signature).unwrap();
-        assert!(
-            sig.check_created().is_ok(),
-            "a signature well inside the age budget must still verify"
-        );
-    }
-
     #[test]
     fn clock_skew_rejection() {
         let kp = Keypair::generate();
@@ -574,173 +384,6 @@ mod tests {
         assert_ne!(d1, d2);
     }
 
-    /// U2: the nonce is per-call, so two signatures over an identical request
-    /// differ and so do the `Signature-Input` values they were computed over.
-    #[test]
-    fn two_signatures_over_an_identical_request_differ() {
-        let kp = Keypair::generate();
-        let a = sign_request(&kp, "POST", "/api/register", b"same body");
-        let b = sign_request(&kp, "POST", "/api/register", b"same body");
-
-        assert_ne!(
-            a.nonce, b.nonce,
-            "each sign_request call must draw a fresh nonce"
-        );
-        assert_ne!(
-            a.signature_input, b.signature_input,
-            "the nonce must reach the wire header, not just the struct"
-        );
-        assert_ne!(
-            a.signature, b.signature,
-            "a covered nonce must change the signature bytes"
-        );
-    }
-
-    /// U2: `parse` round-trips the nonce, and a nonce-less header still parses.
-    #[test]
-    fn parse_exposes_nonce_and_tolerates_its_absence() {
-        let kp = Keypair::generate();
-        let headers = sign_request(&kp, "POST", "/api/register", b"body");
-        let parsed = HttpSignature::parse(&headers.signature_input, &headers.signature).unwrap();
-        assert_eq!(
-            parsed.nonce.as_deref(),
-            Some(headers.nonce.as_str()),
-            "the parsed nonce must be the one that was signed, unquoted"
-        );
-
-        let did = kp.did();
-        let nonceless = format!(
-            r#"sig1=("@method" "@path" "content-digest");keyid="{did}";alg="ed25519";created=1000"#
-        );
-        let parsed = HttpSignature::parse(&nonceless, &headers.signature).unwrap();
-        assert_eq!(
-            parsed.nonce, None,
-            "a pre-U2 signer's header must parse with no nonce"
-        );
-    }
-
-    /// U2: 128 bits of entropy, hex-encoded.
-    #[test]
-    fn nonce_is_128_bits() {
-        let kp = Keypair::generate();
-        let headers = sign_request(&kp, "GET", "/api/v1/agents", b"");
-        let raw = hex::decode(&headers.nonce).expect("nonce must be hex");
-        assert_eq!(raw.len(), 16, "nonce must carry exactly 128 bits");
-    }
-
-    /// U2 backward compatibility, the load-bearing one: a nonce-bearing
-    /// signature still verifies through the same reconstruct-and-verify flow
-    /// the node runs, with no verifier change.
-    #[test]
-    fn nonce_bearing_signature_verifies_through_the_unchanged_flow() {
-        use crate::identity::verify;
-
-        let kp = Keypair::generate();
-        let body = b"{\"did\":\"did:key:z6Mk\"}";
-        let headers = sign_request(&kp, "POST", "/api/register", body);
-        assert!(
-            headers.signature_input.contains(";nonce=\""),
-            "this test is only meaningful once the header carries a nonce"
-        );
-
-        let sig = HttpSignature::parse(&headers.signature_input, &headers.signature).unwrap();
-        assert_eq!(
-            sig.components, COVERED_COMPONENTS,
-            "the nonce must not disturb the covered-component list"
-        );
-
-        // Exactly what the node does: rebuild @signature-params from the
-        // received header text and verify over it.
-        let sig_params_value = headers.signature_input.strip_prefix("sig1=").unwrap();
-        let mut request_values = HashMap::new();
-        request_values.insert("@method".to_string(), "POST".to_string());
-        request_values.insert("@path".to_string(), "/api/register".to_string());
-        request_values.insert("content-digest".to_string(), headers.content_digest.clone());
-        let components_ref: Vec<&str> = sig.components.iter().map(String::as_str).collect();
-        let signing_string =
-            build_signing_string(&components_ref, sig_params_value, &request_values).unwrap();
-
-        let vk = sig.key_id.to_verifying_key().unwrap();
-        let sig_bytes: [u8; 64] = sig.signature_bytes.clone().try_into().unwrap();
-        assert!(
-            verify(&vk, signing_string.as_bytes(), &sig_bytes).is_ok(),
-            "a nonce-bearing signature must verify unchanged"
-        );
-
-        // And the nonce is genuinely covered: strip it and verification fails.
-        let without_nonce = sig_params_value
-            .split(";nonce=")
-            .next()
-            .unwrap()
-            .to_string();
-        let tampered =
-            build_signing_string(&components_ref, &without_nonce, &request_values).unwrap();
-        assert!(
-            verify(&vk, tampered.as_bytes(), &sig_bytes).is_err(),
-            "the nonce must be inside @signature-params, not decoration"
-        );
-    }
-
-    /// A `)` before the `(` must be rejected, not sliced backwards. The two
-    /// parentheses are found independently, so an out-of-order pair used to
-    /// build a reversed range and panic inside `parse` — reachable with two
-    /// headers and no credentials at all.
-    #[test]
-    fn reversed_parentheses_are_rejected_not_panicked() {
-        let err = HttpSignature::parse("sig1=)(", "sig1=:AAAA:")
-            .expect_err("reversed parentheses must return an error");
-        let msg = err.to_string();
-        assert!(
-            msg.contains("'(' must precede ')'"),
-            "expected the paren-ordering error, got: {msg}"
-        );
-    }
-
-    /// `parse` runs on attacker-controlled header text before any
-    /// authentication, so no input may reach a panic. Every entry here must
-    /// come back as `Err`; the test failing by panic is the case it exists for.
-    #[test]
-    fn malformed_signature_input_always_errors_never_panics() {
-        let did = Keypair::generate().did();
-        let long = format!("sig1=({}", "(".repeat(4096));
-        let cases: Vec<(&str, &str)> = vec![
-            ("", "sig1=:AAAA:"),
-            ("sig1=", "sig1=:AAAA:"),
-            ("sig1=()", "sig1=:AAAA:"),
-            ("sig1=)(", "sig1=:AAAA:"),
-            ("sig1=(", "sig1=:AAAA:"),
-            ("sig1=)", "sig1=:AAAA:"),
-            ("sig1=))((", "sig1=:AAAA:"),
-            ("sig1=;", "sig1=:AAAA:"),
-            (";", "sig1=:AAAA:"),
-            ("sig1=)(", ""),
-            ("sig1=)(", "sig1=:not base64!:"),
-            ("sig1=(\"@method\");created=notanumber", "sig1=:AAAA:"),
-            ("sig1=(\"@method\");created=", "sig1=:AAAA:"),
-            ("sig1=(é)é;created=1000", "sig1=:AAAA:"),
-            ("sig1=)é(", "sig1=:AAAA:"),
-            ("sig1=é)(é", "sig1=:AAAA:"),
-            (&long, "sig1=:AAAA:"),
-        ];
-
-        for (input, header) in cases {
-            let out = HttpSignature::parse(input, header);
-            assert!(
-                out.is_err(),
-                "malformed Signature-Input {input:?} parsed successfully"
-            );
-        }
-
-        // An unbalanced quote inside an otherwise well-formed header is
-        // tolerated: `trim_matches('"')` strips whatever quotes are there.
-        // The invariant that matters is that it returns rather than panics, so
-        // assert it parses and leaves the quote-stripped component behind.
-        let unbalanced = format!(r#"sig1=("@method);keyid="{did}";alg="ed25519";created=1000"#);
-        let parsed = HttpSignature::parse(&unbalanced, "sig1=:AAAA:")
-            .expect("an unbalanced quote is tolerated, not a parse failure");
-        assert_eq!(parsed.components, vec!["@method".to_string()]);
-    }
-
     #[test]
     fn method_uppercased_in_signing_string() {
         let kp = Keypair::generate();

crates/gitlawb-core/src/lib.rs

@@ -5,7 +5,6 @@ pub mod encrypt;
 pub mod error;
 pub mod http_sig;
 pub mod identity;
-pub mod node_denial;
 // `url` is the one dependency here that drags a tail (idna, then the icu
 // crates), and gitlawb-core is allowlisted to stay embeddable. Every client that
 // needs this predicate already parses URLs, so they opt in and nothing else

crates/gitlawb-core/src/node_denial.rs

@@ -1,229 +0,0 @@
-//! The denial codes the node puts in `X-Gitlawb-Error`, in one place.
-//!
-//! When the node refuses a write it answers with an `X-Gitlawb-Error` header
-//! and a matching `error` field in the JSON body. The `gl` client keys off that
-//! header to turn a denial into a hard error instead of handing the caller a
-//! response that pretty-prints like a success.
-//!
-//! # Why this is not named for the signature ledger
-//!
-//! It was, and only the ledger's `consume_signature` middleware built these
-//! responses. Then the per-client flood brake in `rate_limit` started answering
-//! 429 on the same five route groups the ledger's `signature_ledger_full` 429
-//! already covered, and the only thing telling the two apart was the *absence*
-//! of a header — the same signal we already accept a proxy may strip. Two
-//! unrelated refusals were indistinguishable on the wire.
-//!
-//! The fix is to give the brake a code, which means this type's subject is the
-//! wire vocabulary rather than one middleware. Putting [`RateLimited`] here
-//! instead of in a sibling enum is deliberate: the exhaustive match in `gl` is
-//! the whole reason the type exists, and a parallel type would need its own
-//! parallel match, which is exactly the duplication that let
-//! `signature_nonce_too_short` drift in the first place. One enum, one match,
-//! one place to add a code.
-//!
-//! Both halves used to carry their own list of string literals, agreeing only
-//! by having been typed the same way twice in two crates. They drifted exactly
-//! as you would expect: `signature_nonce_too_short` was added to the node after
-//! the client's list was written, so for a while the client returned `Ok(400)`
-//! for that denial and the write looked like it might have happened.
-//!
-//! This enum is the single source of truth. The node builds its responses from
-//! it, so the wire strings and statuses come from one place, and `gl` matches
-//! it with no wildcard arm, so adding a variant here is a compile error in the
-//! client until the client handles it.
-//!
-//! # Deliberately not `#[non_exhaustive]`
-//!
-//! Marking this `#[non_exhaustive]` would force every downstream crate to add a
-//! wildcard arm, which is precisely the silent fallthrough this type exists to
-//! prevent. The compile-time guarantee is worth more here than the freedom to
-//! add a variant without touching the client: the client and the node ship from
-//! this same repo and are versioned together.
-//!
-//! The guarantee is over the codes *this repo's node emits*, not over arbitrary
-//! input. A client talks to nodes it does not control, so [`from_code`] returns
-//! `Option` and an unrecognised string stays unrecognised at runtime.
-//!
-//! [`from_code`]: NodeDenial::from_code
-//! [`RateLimited`]: NodeDenial::RateLimited
-
-/// A refusal the node names in `X-Gitlawb-Error`.
-///
-/// The `as_str` value is the wire contract: it appears verbatim in the
-/// `X-Gitlawb-Error` response header and in the body's `error` field, and
-/// scripts match on it. Never change one.
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-pub enum NodeDenial {
-    /// 400 — the node requires a `nonce` in `Signature-Input` and the request
-    /// carried none. A pre-nonce client; the caller must upgrade.
-    NonceRequired,
-    /// 400 — the request carried a `nonce`, but too short to be unique. The
-    /// client already emits the parameter, it just does not fill it properly.
-    NonceTooShort,
-    /// 409 — the node already admitted a request bearing this signature.
-    Replayed,
-    /// 429 — too many unexpired signatures for this identity. A rate condition,
-    /// not a permanent refusal.
-    LedgerFull,
-    /// 500 — the ledger was reached without a verified identity, meaning the
-    /// node's own auth layer order is wrong. The one code that is not the
-    /// caller's fault.
-    IdentityMissing,
-    /// 503 — the ledger backend is down and the node is failing closed.
-    LedgerUnavailable,
-    /// 429 — the node's per-client flood brake refused the request before any
-    /// handler ran. Not a ledger outcome: it is keyed on the caller's resolved
-    /// network address, not on an identity or a signature, so it clears when
-    /// the address's window ages out and no amount of re-signing helps. Shares
-    /// its status with [`LedgerFull`](Self::LedgerFull), which is precisely why
-    /// it needs a code of its own.
-    RateLimited,
-}
-
-impl NodeDenial {
-    /// Every variant. Kept in the same order as the declaration so a reader can
-    /// check it by eye; the `all_is_exhaustive` test below asserts it
-    /// is complete.
-    pub const ALL: [Self; 7] = [
-        Self::NonceRequired,
-        Self::NonceTooShort,
-        Self::Replayed,
-        Self::LedgerFull,
-        Self::IdentityMissing,
-        Self::LedgerUnavailable,
-        Self::RateLimited,
-    ];
-
-    /// The wire code, as it appears in `X-Gitlawb-Error` and in the body.
-    pub const fn as_str(self) -> &'static str {
-        match self {
-            Self::NonceRequired => "signature_nonce_required",
-            Self::NonceTooShort => "signature_nonce_too_short",
-            Self::Replayed => "signature_replayed",
-            Self::LedgerFull => "signature_ledger_full",
-            Self::IdentityMissing => "signature_identity_missing",
-            Self::LedgerUnavailable => "signature_ledger_unavailable",
-            Self::RateLimited => "rate_limited",
-        }
-    }
-
-    /// The HTTP status the node pairs with this code.
-    ///
-    /// A `u16` rather than an `http::StatusCode` because this crate carries no
-    /// HTTP dependency; the node converts once, and its own test proves every
-    /// value here is a valid status.
-    pub const fn status(self) -> u16 {
-        match self {
-            Self::NonceRequired | Self::NonceTooShort => 400,
-            Self::Replayed => 409,
-            Self::LedgerFull | Self::RateLimited => 429,
-            Self::IdentityMissing => 500,
-            Self::LedgerUnavailable => 503,
-        }
-    }
-
-    /// Parse a code off the wire.
-    ///
-    /// Returns `None` for anything this build does not know, because the string
-    /// comes from a node the caller does not control. An unknown code is not an
-    /// error to report; it is a denial this client cannot describe, and the
-    /// caller decides what to do with the raw response.
-    pub fn from_code(code: &str) -> Option<Self> {
-        Self::ALL.into_iter().find(|d| d.as_str() == code)
-    }
-}
-
-impl std::fmt::Display for NodeDenial {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(self.as_str())
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    /// Guards [`NodeDenial::ALL`] against a variant added above it and
-    /// forgotten here: the match is exhaustive, so a new variant stops this
-    /// compiling, and the assertion catches the case where someone adds the arm
-    /// but not the `ALL` entry.
-    #[test]
-    fn all_is_exhaustive() {
-        fn tag(d: NodeDenial) -> u8 {
-            match d {
-                NodeDenial::NonceRequired => 0,
-                NodeDenial::NonceTooShort => 1,
-                NodeDenial::Replayed => 2,
-                NodeDenial::LedgerFull => 3,
-                NodeDenial::IdentityMissing => 4,
-                NodeDenial::LedgerUnavailable => 5,
-                NodeDenial::RateLimited => 6,
-            }
-        }
-        let mut tags: Vec<u8> = NodeDenial::ALL.iter().copied().map(tag).collect();
-        tags.sort_unstable();
-        assert_eq!(
-            tags,
-            (0..NodeDenial::ALL.len() as u8).collect::<Vec<_>>(),
-            "NodeDenial::ALL must list every variant exactly once",
-        );
-    }
-
-    /// The wire contract, pinned literally. These strings and statuses are what
-    /// deployed nodes emit and what deployed clients and scripts match on, so a
-    /// change here is a protocol break, not a rename.
-    #[test]
-    fn wire_codes_and_statuses_are_pinned() {
-        let pinned: [(NodeDenial, &str, u16); 7] = [
-            (NodeDenial::NonceRequired, "signature_nonce_required", 400),
-            (NodeDenial::NonceTooShort, "signature_nonce_too_short", 400),
-            (NodeDenial::Replayed, "signature_replayed", 409),
-            (NodeDenial::LedgerFull, "signature_ledger_full", 429),
-            (
-                NodeDenial::IdentityMissing,
-                "signature_identity_missing",
-                500,
-            ),
-            (
-                NodeDenial::LedgerUnavailable,
-                "signature_ledger_unavailable",
-                503,
-            ),
-            (NodeDenial::RateLimited, "rate_limited", 429),
-        ];
-        assert_eq!(pinned.len(), NodeDenial::ALL.len());
-        for (denial, code, status) in pinned {
-            assert_eq!(denial.as_str(), code);
-            assert_eq!(denial.status(), status);
-        }
-    }
-
-    #[test]
-    fn from_code_round_trips_every_variant() {
-        for denial in NodeDenial::ALL {
-            assert_eq!(NodeDenial::from_code(denial.as_str()), Some(denial));
-        }
-    }
-
-    #[test]
-    fn from_code_rejects_what_this_build_does_not_know() {
-        for unknown in [
-            "",
-            "signature_",
-            "signature_nonce",
-            "signature_replayed_",
-            " signature_replayed",
-            "SIGNATURE_REPLAYED",
-            "repo_exists",
-            "human_detected",
-            "signature_seventh_code_from_a_newer_node",
-        ] {
-            assert_eq!(
-                NodeDenial::from_code(unknown),
-                None,
-                "must not recognise {unknown:?}",
-            );
-        }
-    }
-}

crates/gitlawb-node/Cargo.toml

@@ -75,7 +75,6 @@ alloy = { version = "1", default-features = false, features = [
 libp2p-dns = { version = "0.44.0", features = ["tokio"] }
 
 [dev-dependencies]
-multibase = { workspace = true }
 mockito = "1"
 tempfile = "3"
 # `test-util` for `#[tokio::test(start_paused = true)]`: lets the deadline tests assert

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

@@ -3758,9 +3758,13 @@ 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)
-                .await;
-        state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool);
+            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.push_limiter_trust = crate::rate_limit::TrustedProxy::None;
         let mut cfg = (*state.config).clone();
         cfg.git_acquire_timeout_secs = 1;
@@ -3831,9 +3835,13 @@ 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)
-                .await;
-        state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool);
+            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),
+        );
         let mut cfg = (*state.config).clone();
         cfg.git_acquire_timeout_secs = 1;
         state.config = Arc::new(cfg);
@@ -7956,9 +7964,18 @@ 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)
-                .await;
-        state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool);
+            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();
         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,20 +61,30 @@ 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(&disk_path, &issue_id, &json_str);
+    let create_result = git_issues::create_issue(guard.path(), &issue_id, &json_str);
 
-    // Always release the advisory lock — even on error; upload to Tigris only on success.
-    guard.release(create_result.is_ok()).await;
+    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?;
 
     create_result.map_err(|e| AppError::Git(e.to_string()))?;
 
@@ -225,6 +235,8 @@ 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
@@ -232,8 +244,161 @@ pub async fn close_issue(
         .await?
         .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
 
-    // Same capacity shed as create_issue above (#173 F1): an exhausted write-lock
-    // pool is a 503 + Retry-After, not a 500 git error.
+    // 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.
     let guard = state
         .repo_store
         .acquire_write(&record.owner_did, &record.name)
@@ -241,37 +406,56 @@ 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();
 
-    // 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),
+    // 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(),
+                ));
+            }
+        }
         Ok(None) => {
-            guard.release(false).await;
-            return Err(AppError::NotFound(format!("issue {issue_id} not found")));
+            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(),
+                )
+            });
         }
         Err(e) => {
-            guard.release(false).await;
+            let _ = 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.
-    guard.release(close_result.is_ok()).await;
+    // Same short-circuit as create_issue, and before the 200 body below.
+    guard.release(close_result.is_ok()).await.into_result()?;
 
     let updated = close_result
         .map_err(|e| AppError::Git(e.to_string()))?
@@ -285,12 +469,344 @@ pub async fn close_issue(
     Ok(Json(issue))
 }
 
-/// #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 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.
 #[cfg(test)]
 mod lock_pool_shed_tests {
     use super::*;
@@ -314,22 +830,17 @@ 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!(
@@ -346,10 +857,6 @@ 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";
@@ -380,9 +887,7 @@ 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");
 
-        // 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 _ = held.release(false).await;
         let admitted = create_issue(
             State(state.clone()),
             Extension(AuthenticatedDid(owner.to_string())),
@@ -401,7 +906,38 @@ mod lock_pool_shed_tests {
         );
     }
 
-    /// RED-before/GREEN-after for `close_issue`, same two directions.
+    #[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:?}"
+        );
+    }
+
     #[sqlx::test]
     async fn close_issue_lock_pool_exhaustion_sheds_503_not_500(pool: PgPool) {
         let owner = "did:key:zISSUECLOSELOCKPOOLBBBBBBBBBBBBBBBBBBBBB";
@@ -426,12 +962,14 @@ 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");
 
-        held.release(false).await;
+        let _ = held.release(false).await;
         let admitted = close_issue(
             State(state.clone()),
             Extension(AuthenticatedDid(owner.to_string())),
@@ -440,6 +978,8 @@ mod lock_pool_shed_tests {
                 "lp-close".to_string(),
                 "deadbeef".to_string(),
             )),
+            axum::http::HeaderMap::new(),
+            crate::rate_limit::PeerAddr(None),
         )
         .await;
         assert!(
@@ -448,4 +988,244 @@ 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/peers.rs

@@ -514,32 +514,9 @@ pub async fn ping_peer(
     })))
 }
 
-/// The `x-gitlawb-error` code a peer put on a rejection, if it sent one.
-///
-/// Kept verbatim: it is what separates a ledger rejection from a replay
-/// rejection on the same-ish status, and it is what an operator greps for. A
-/// peer behind a proxy that strips unknown `X-` headers returns `None`, which
-/// is why every caller logs the status too rather than relying on this alone.
-pub(crate) fn peer_error_code(headers: &reqwest::header::HeaderMap) -> Option<&str> {
-    headers.get("x-gitlawb-error")?.to_str().ok()
-}
-
 #[cfg(test)]
 mod tests {
     use super::is_public_http_url;
-    use super::peer_error_code;
-
-    #[test]
-    fn peer_error_code_reads_the_header_when_present() {
-        let mut headers = reqwest::header::HeaderMap::new();
-        headers.insert("x-gitlawb-error", "signature_ledger_full".parse().unwrap());
-        assert_eq!(peer_error_code(&headers), Some("signature_ledger_full"));
-    }
-
-    #[test]
-    fn peer_error_code_is_none_without_the_header() {
-        assert_eq!(peer_error_code(&reqwest::header::HeaderMap::new()), None);
-    }
 
     #[test]
     fn accepts_public_https_and_http() {
@@ -717,7 +694,6 @@ mod tests {
             .layer(Extension(IpRateLimiter {
                 limiter: RateLimiter::new(60, Duration::from_secs(3600)),
                 trust: TrustedProxy::None,
-                skip_reads: false,
             }))
             .with_state(state);
         let mut req =
@@ -766,7 +742,6 @@ mod tests {
             .layer(Extension(IpRateLimiter {
                 limiter: RateLimiter::new(1, Duration::from_secs(3600)),
                 trust: TrustedProxy::None,
-                skip_reads: false,
             }))
             .with_state(state);
         let peer = "203.0.113.5:5000".parse::<SocketAddr>().unwrap();
@@ -1452,15 +1427,6 @@ mod tests {
             .await
             .unwrap();
         assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS);
-        // The brake shares 429 with the ledger's `signature_ledger_full` on
-        // this route group, so the code is what tells a client which one it
-        // hit. Asserted end-to-end here, through the real router.
-        assert_eq!(
-            resp.headers()
-                .get("x-gitlawb-error")
-                .and_then(|v| v.to_str().ok()),
-            Some("rate_limited"),
-        );
         let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
             .await
             .unwrap();

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

@@ -209,9 +209,6 @@ 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)
@@ -228,7 +225,10 @@ pub async fn merge_pr(
     );
 
     // Always release the advisory lock — even on error; upload to Tigris only on success.
-    guard.release(merge_result.is_ok()).await;
+    // 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()?;
 
     let merge_sha = merge_result.map_err(|e| AppError::Git(e.to_string()))?;
 
@@ -428,12 +428,8 @@ pub async fn list_comments(
     Ok(Json(serde_json::json!({ "comments": comments })))
 }
 
-/// #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.
+/// #173 F1 follow-up: merge holds no admission permit, so an exhausted write-lock
+/// pool must shed 503 + Retry-After.
 #[cfg(test)]
 mod lock_pool_shed_tests {
     use super::*;
@@ -457,20 +453,15 @@ 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");
@@ -524,9 +515,7 @@ mod lock_pool_shed_tests {
             "merge_pr: a capacity shed must tell the client when to retry"
         );
 
-        // 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 _ = held.release(false).await;
         let admitted = merge_pr(
             State(state.clone()),
             Extension(AuthenticatedDid(owner.to_string())),

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

@@ -1,16 +1,14 @@
 use axum::body::Body;
 use axum::extract::{Request, State};
-use axum::http::{Method, StatusCode};
+use axum::http::StatusCode;
 use axum::middleware::Next;
 use axum::response::{IntoResponse, Response};
 use axum::Json;
 use http_body_util::BodyExt;
 use serde_json::json;
-use sha2::{Digest, Sha256};
 use std::collections::HashMap;
 
 use gitlawb_core::did::Did;
-use gitlawb_core::node_denial::NodeDenial;
 use gitlawb_core::ucan::Ucan;
 
 use crate::state::AppState;
@@ -19,39 +17,6 @@ use crate::state::AppState;
 #[derive(Clone, Debug)]
 pub struct AuthenticatedDid(pub String);
 
-/// The canonical identity of a verified HTTP signature, published into request
-/// extensions by [`require_signature`] so a spent-signature ledger can be keyed
-/// without re-parsing headers.
-///
-/// `signing_string_hash` is a hex SHA-256 of the *reconstructed* signing string,
-/// never of the raw `Signature` header. `HttpSignature::parse` trims, and the
-/// header is not a covered component, so whitespace variants are distinct header
-/// bytes that reconstruct to one signing string; hashing the reconstruction is
-/// what collapses those variants to a single ledger key. The signing string also
-/// embeds `keyid` and `created` through its `@signature-params` line, so the
-/// hash cannot collide across DIDs.
-#[derive(Clone, Debug)]
-pub struct SignatureIdentity {
-    /// The signing DID, as it appeared in the `keyid` parameter. Human-readable
-    /// and good for logs; NOT an identity key — see [`Self::key_fingerprint`].
-    pub keyid: String,
-    /// Hex of the 32 resolved Ed25519 public key bytes: the canonical identity.
-    ///
-    /// `Did` stores the wire string verbatim while `to_verifying_key` resolves
-    /// it through `multibase::decode`, which accepts any multibase prefix. One
-    /// keypair therefore has many valid `did:key` spellings (`z6Mk…` base58btc,
-    /// `f…` base16, `m…` base64url, and more) that are all distinct strings and
-    /// all resolve to the same key. Anything counting per identity by string
-    /// equality hands that keypair a fresh budget per spelling, so per-identity
-    /// accounting keys on this instead.
-    pub key_fingerprint: String,
-    /// The `nonce` parameter, absent on signatures from a pre-nonce signer.
-    /// Present but short is not the same as unique: see [`unique_nonce`].
-    pub nonce: Option<String>,
-    /// Hex SHA-256 of the reconstructed signing string: exactly 64 characters.
-    pub signing_string_hash: String,
-}
-
 /// Whether `caller` is authorized to push to `record`.
 ///
 /// Phase 1 (`GITLAWB_ENFORCE_OWNER_PUSH`): owner-only, via the canonical
@@ -66,7 +31,6 @@ pub fn caller_authorized_to_push(record: &crate::db::RepoRecord, caller: &str) -
 
 use gitlawb_core::http_sig::{
     build_signing_string, compute_content_digest, HttpSignature, COVERED_COMPONENTS,
-    MAX_FUTURE_SKEW_SECS, MAX_SIGNATURE_AGE_SECS,
 };
 use gitlawb_core::identity::verify;
 
@@ -74,7 +38,7 @@ use gitlawb_core::identity::verify;
 ///
 /// Every write request must carry:
 ///   Content-Digest:   sha-256=:base64hash:
-///   Signature-Input:  sig1=("@method" "@path" "content-digest");keyid="did:key:...";alg="ed25519";created=<unix>;nonce="..."
+///   Signature-Input:  sig1=("@method" "@path" "content-digest");keyid="did:key:...";alg="ed25519";created=<unix>
 ///   Signature:        sig1=:base64signature:
 ///
 /// The middleware:
@@ -278,15 +242,6 @@ pub async fn require_signature(request: Request, next: Next) -> Response {
     request
         .extensions_mut()
         .insert(AuthenticatedDid(sig.key_id.to_string()));
-    // Publish the canonical ledger key while the verified signing string is still
-    // in hand. Hashing `signing_string` (the reconstruction that was just verified
-    // against) rather than any raw header is deliberate: see `SignatureIdentity`.
-    request.extensions_mut().insert(SignatureIdentity {
-        keyid: sig.key_id.to_string(),
-        key_fingerprint: hex::encode(verifying_key.to_bytes()),
-        nonce: sig.nonce.clone(),
-        signing_string_hash: hex::encode(Sha256::digest(signing_string.as_bytes())),
-    });
     next.run(request).await
 }
 
@@ -411,251 +366,6 @@ pub async fn require_ucan_chain(
     next.run(request).await
 }
 
-/// How long a spent signature stays in the ledger, in seconds.
-///
-/// Retention must cover the whole window in which the signature would still be
-/// accepted, or a key is evicted while its signature still passes the time
-/// check. `check_created` accepts a request when `created` falls in
-/// `[now - 300, now + 30]`, so for one fixed signature the acceptable *arrival*
-/// window is `[created - 30, created + 300]`: 330 seconds wide. Expiry is
-/// computed from arrival rather than from `created`, which can only over-retain
-/// (an early arrival is charged the full 330s from when it landed), never
-/// under-retain.
-///
-/// 330 + [`CROSS_INSTANCE_SKEW_MARGIN_SECS`] = 390. The margin is not slack:
-/// single-node a flush 330 is exactly right (at `now = created + 300` the sweep
-/// predicate `expires_at < now` is false so the row survives, and at
-/// `created + 301` the signature is already too old), but the sweep runs on
-/// whichever instance's timer fires, using ITS clock. With instance B running
-/// `d` seconds ahead of A, a flush TTL has B delete the row at true time
-/// `created + 300 - d` while A still accepts the replay until `created + 300`,
-/// a replay window of exactly `d`. 60s covers the disagreement an NTP-synced
-/// fleet can actually reach.
-///
-/// Derived rather than written out so it cannot drift from the window it has to
-/// cover: widening either skew bound in `gitlawb-core` carries the TTL with it.
-const SIGNATURE_LEDGER_TTL_SECS: i64 =
-    MAX_SIGNATURE_AGE_SECS + MAX_FUTURE_SKEW_SECS + CROSS_INSTANCE_SKEW_MARGIN_SECS;
-
-/// Clock disagreement between two instances that the ledger TTL must absorb.
-const CROSS_INSTANCE_SKEW_MARGIN_SECS: i64 = 60;
-
-/// The shortest nonce this node will treat as a unique ledger key.
-///
-/// `HttpSignature::parse` maps the raw parameter, so `nonce=""` arrives as
-/// `Some("")` rather than `None`. Length is the only property a verifier can
-/// check (entropy is not observable), so it is the floor: 16 characters is 64
-/// bits even on the weakest plausible alphabet, hex at 4 bits per character. At
-/// the enforced ceiling of `MAX_LIVE_SIGNATURES_PER_KEYID` (512) live rows per
-/// identity, the birthday probability of an accidental collision within one
-/// identity is about `512^2 / 2^65`, roughly 1e-14.
-///
-/// Set below the 32 hex characters (128 bits) our own `sign_request` emits so
-/// the floor costs no upgrade churn, yet still admits a third-party client
-/// signing a 22-character base64 UUID or a 16-character hex draw.
-const MIN_NONCE_CHARS: usize = 16;
-
-/// The nonce, but only when it is long enough to stand in as a unique key.
-///
-/// Without this filter an empty nonce is `Some("")`, which both satisfies the
-/// staged `require a nonce` flag (whose stated purpose is that every client
-/// signs one) and puts [`ledger_key`] on its `(key, nonce)` arm with a value
-/// that is CONSTANT per identity: every mutation from that keypair would
-/// collapse onto one ledger key, so the first one in a retention window would
-/// succeed and every later one would be refused as a replay.
-fn unique_nonce(identity: &SignatureIdentity) -> Option<&str> {
-    identity
-        .nonce
-        .as_deref()
-        .filter(|nonce| nonce.chars().count() >= MIN_NONCE_CHARS)
-}
-
-/// The ledger key for a verified signature: always a 64-character hex SHA-256,
-/// which is what the `consumed_signatures` CHECK constraint requires.
-///
-/// Two disjoint schemes, kept apart by a domain tag so a nonce key can never
-/// collide with a signing-string key:
-///   * with a nonce of usable width, `(key_fingerprint, nonce)` — short,
-///     fixed-width, and it lets two legitimately identical requests be told
-///     apart;
-///   * otherwise the signing-string hash, which is canonical by construction
-///     and collapses whitespace variants of the `Signature` header onto one key.
-///
-/// A too-short nonce takes the second arm rather than the first. That is the
-/// safe direction: the hash arm is unique by construction, so a client with a
-/// weak nonce loses only the ability to repeat byte-identical requests inside
-/// one second, whereas trusting the nonce would collapse its whole traffic onto
-/// one key. The fingerprint rather than `keyid` keeps the nonce arm on the
-/// resolved key, so two spellings of one DID cannot reuse a nonce.
-fn ledger_key(identity: &SignatureIdentity) -> String {
-    let mut hasher = Sha256::new();
-    match unique_nonce(identity) {
-        Some(nonce) => {
-            hasher.update(b"gitlawb/sig-nonce\x00");
-            hasher.update(identity.key_fingerprint.as_bytes());
-            hasher.update(b"\x00");
-            hasher.update(nonce.as_bytes());
-        }
-        None => {
-            hasher.update(b"gitlawb/sig-string\x00");
-            hasher.update(identity.signing_string_hash.as_bytes());
-        }
-    }
-    hex::encode(hasher.finalize())
-}
-
-/// Refuse a signed write with one of the shared ledger denial codes.
-///
-/// The code and status both come from [`NodeDenial`], which `gl` matches
-/// exhaustively, so the two halves cannot drift: adding a variant there is a
-/// compile error in the client until it handles it.
-fn ledger_rejection(denial: NodeDenial, message: &str) -> Response {
-    let code = denial.as_str();
-    (
-        // Infallible: every `NodeDenial::status` is a real HTTP status,
-        // which `denial_statuses_are_valid_http_codes` proves over every
-        // variant.
-        StatusCode::from_u16(denial.status()).expect("NodeDenial status is a valid HTTP code"),
-        [("X-Gitlawb-Error", code)],
-        Json(json!({ "error": code, "message": message })),
-    )
-        .into_response()
-}
-
-/// Axum middleware that spends a verified HTTP signature exactly once.
-///
-/// Layer it so it runs *after* [`require_signature`] (which publishes the
-/// [`SignatureIdentity`] read here) and after [`require_ucan_chain`], but before
-/// the handler. Consuming last means only a request that cleared every auth
-/// check spends its signature, so a valid signature paired with a rejected UCAN
-/// can be retried with the same bytes; consuming before the handler is what
-/// closes the concurrent-replay race.
-pub async fn consume_signature(
-    State(state): State<AppState>,
-    request: Request,
-    next: Next,
-) -> Response {
-    // Skip reads. `write_routes` chains `PUT/DELETE/GET` on one path
-    // (`/api/v1/repos/{owner}/{repo}/visibility`, `server.rs`), so
-    // `list_visibility` is served by a router inside `add_auth_layers` and `gl`
-    // drives it through `get_signed`. Ledgering there would put a database write
-    // on the signed read path, which R7 forbids. A method check is the general
-    // form of that exclusion and needs no router split: replaying a read has no
-    // side effect to spend.
-    if matches!(*request.method(), Method::GET | Method::HEAD) {
-        crate::metrics::record_signature_ledger("skipped_read");
-        return next.run(request).await;
-    }
-
-    let identity = match request.extensions().get::<SignatureIdentity>() {
-        Some(identity) => identity.clone(),
-        None => {
-            crate::metrics::record_signature_ledger("identity_missing");
-            // Fail closed. If this passed the request through, a wrong layer
-            // order would silently delete the replay defense while every test
-            // that exercises the correct stack kept passing.
-            tracing::error!(
-                path = %request.uri().path(),
-                "signature ledger reached without a verified SignatureIdentity — check the layer order",
-            );
-            return ledger_rejection(
-                NodeDenial::IdentityMissing,
-                "the request reached the signature ledger without a verified identity",
-            );
-        }
-    };
-
-    // Staged rollout of R6: once every client signs a nonce, an operator closes
-    // the signing-string fallback here. This runs after the method skip above,
-    // so it never reaches a signed read, and before the ledger is charged, so a
-    // refused request spends nothing.
-    if state.config.require_signature_nonce && unique_nonce(&identity).is_none() {
-        // A present-but-short nonce gets its own code. It is a different
-        // client bug from a pre-nonce signer (the client already emits the
-        // parameter, it just does not fill it), and the two need different
-        // instructions.
-        return match identity.nonce {
-            None => {
-                crate::metrics::record_signature_ledger("nonce_required");
-                tracing::warn!(did = %identity.keyid, "rejected a nonce-less signature: a nonce is required");
-                ledger_rejection(
-                    NodeDenial::NonceRequired,
-                    "this node requires a `nonce` parameter in Signature-Input — upgrade your client",
-                )
-            }
-            Some(nonce) => {
-                crate::metrics::record_signature_ledger("nonce_too_short");
-                tracing::warn!(
-                    did = %identity.keyid,
-                    len = nonce.chars().count(),
-                    "rejected a signature whose nonce is too short to be unique",
-                );
-                ledger_rejection(
-                    NodeDenial::NonceTooShort,
-                    &format!(
-                        "the `nonce` parameter in Signature-Input must be at least \
-                         {MIN_NONCE_CHARS} characters drawn from a CSPRNG — \
-                         `gl` signs 32 hex characters",
-                    ),
-                )
-            }
-        };
-    }
-
-    let key = ledger_key(&identity);
-    let now = chrono::Utc::now().timestamp();
-    // Charge the cap against the RESOLVED key, never the wire DID: see
-    // `SignatureIdentity::key_fingerprint`. This does not weaken single-use,
-    // which never depended on the identity column — a replay has to reproduce
-    // the signed bytes, and `keyid` sits inside `@signature-params`, so
-    // re-spelling the DID changes the signing string and needs a fresh
-    // signature. It fixes the per-identity cap only.
-    match state
-        .db
-        .consume_signature(
-            &key,
-            &identity.key_fingerprint,
-            now,
-            now + SIGNATURE_LEDGER_TTL_SECS,
-        )
-        .await
-    {
-        Ok(crate::db::ConsumeSignature::Inserted) => {
-            crate::metrics::record_signature_ledger("admitted");
-            next.run(request).await
-        }
-        Ok(crate::db::ConsumeSignature::Replayed) => {
-            crate::metrics::record_signature_ledger("replayed");
-            tracing::warn!(did = %identity.keyid, "rejected a replayed HTTP signature");
-            ledger_rejection(
-                NodeDenial::Replayed,
-                "this signature has already been used — sign a fresh request",
-            )
-        }
-        Ok(crate::db::ConsumeSignature::IdentityLedgerFull) => {
-            // A rate condition, not a permanent rejection: the caller's live
-            // rows drain as they expire, so this is retryable.
-            crate::metrics::record_signature_ledger("identity_ledger_full");
-            tracing::warn!(did = %identity.keyid, "signature ledger full for this identity");
-            ledger_rejection(
-                NodeDenial::LedgerFull,
-                "too many unexpired signatures for this identity — retry shortly",
-            )
-        }
-        Err(e) => {
-            // Fail closed (KTD5). An outage is exactly when a holder of a
-            // captured signature would try, and the mutation handlers all need
-            // the same database anyway.
-            crate::metrics::record_signature_ledger("unavailable");
-            tracing::error!(did = %identity.keyid, err = %e, "signature ledger unavailable");
-            ledger_rejection(
-                NodeDenial::LedgerUnavailable,
-                "the signature ledger is unavailable — retry shortly",
-            )
-        }
-    }
-}
-
 fn human_detected(message: &str) -> impl IntoResponse {
     (
         StatusCode::UNAUTHORIZED,
@@ -699,39 +409,6 @@ mod tests {
         .unwrap()
     }
 
-    /// `ledger_rejection` converts `NodeDenial::status` with an `expect`.
-    /// This is what makes that infallible: every variant is a real HTTP status,
-    /// and it is the status the node has always paired with that code.
-    #[test]
-    fn denial_statuses_are_valid_http_codes() {
-        for denial in NodeDenial::ALL {
-            let status = StatusCode::from_u16(denial.status())
-                .unwrap_or_else(|e| panic!("{denial} has an invalid status: {e}"));
-            assert_eq!(status.as_u16(), denial.status());
-        }
-    }
-
-    /// The exact bytes `consume_signature` puts on the wire for each denial:
-    /// the `X-Gitlawb-Error` header, the body's `error` field, and the status
-    /// must all agree, because `gl` keys on the header and `gl init` keys on the
-    /// body field.
-    #[tokio::test]
-    async fn ledger_rejection_emits_the_code_in_both_the_header_and_the_body() {
-        for denial in NodeDenial::ALL {
-            let resp = ledger_rejection(denial, "why it was refused");
-            assert_eq!(resp.status().as_u16(), denial.status(), "{denial}");
-            assert_eq!(
-                resp.headers().get("X-Gitlawb-Error").unwrap(),
-                denial.as_str(),
-                "{denial}",
-            );
-            let body = resp.into_body().collect().await.unwrap().to_bytes();
-            let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
-            assert_eq!(json["error"], denial.as_str(), "{denial}");
-            assert_eq!(json["message"], "why it was refused", "{denial}");
-        }
-    }
-
     #[test]
     fn validate_ucan_chain_valid() {
         let node = Keypair::generate();
@@ -840,6 +517,7 @@ 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,
@@ -853,7 +531,6 @@ mod tests {
             push_limiter_trust: crate::rate_limit::TrustedProxy::None,
             sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),
             peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
-            signed_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
             shutdown_tx: tokio::sync::watch::channel(false).0,
             git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
             git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
@@ -873,221 +550,6 @@ mod tests {
         }
     }
 
-    /// The ledger TTL must keep a spent signature past the last instant any
-    /// instance would still accept it, INCLUDING one whose clock disagrees.
-    /// The sweep runs on whichever instance's timer fires, using its own clock,
-    /// so a TTL flush against the acceptance window lets an instance running
-    /// ahead delete a row that a lagging instance would still honour.
-    #[test]
-    fn ledger_ttl_keeps_a_margin_over_the_acceptance_window() {
-        // For one fixed signature the arrival window is
-        // `[created - MAX_FUTURE_SKEW_SECS, created + MAX_SIGNATURE_AGE_SECS]`.
-        let arrival_window = MAX_SIGNATURE_AGE_SECS + MAX_FUTURE_SKEW_SECS;
-        assert_eq!(arrival_window, 330, "the window this TTL is derived from");
-
-        // The floor is written out rather than read from
-        // `CROSS_INSTANCE_SKEW_MARGIN_SECS`, so zeroing that constant is caught
-        // here instead of quietly satisfying the comparison against itself.
-        let margin = SIGNATURE_LEDGER_TTL_SECS - arrival_window;
-        assert!(
-            margin >= 60,
-            "TTL {SIGNATURE_LEDGER_TTL_SECS}s leaves {margin}s over a {arrival_window}s \
-             arrival window, under the 60s skew budget: an instance running ahead \
-             would sweep a row still accepted elsewhere"
-        );
-    }
-
-    // ── `gitlawb_signature_ledger_total{outcome}` ────────────────────────────
-    //
-    // The ledger fails CLOSED, so a fault local to it (statement timeout, lock
-    // contention) turns every REST mutation into a 503 while the handlers' own
-    // queries stay healthy. These assert the counter is really charged on each
-    // terminal outcome, driven through the layer and read back through the same
-    // gather/encode path `/metrics` serves.
-    //
-    // Each assertion is a strict increase rather than an exact delta: the
-    // counter is process-wide, `metrics::init` is a `OnceLock`, and the ledger
-    // tests in `test_support` run in the same binary, so a concurrent test can
-    // add to any outcome. Pollution can only ever add, never subtract, so a
-    // strict increase never flakes; to attribute a delta exactly (e.g. when
-    // mutating out a `record_*` call to confirm RED), run these serially:
-    // `cargo test --bin gitlawb-node ledger_metrics -- --test-threads=1`.
-
-    fn ledger_metric(outcome: &str) -> u64 {
-        let needle = format!("gitlawb_signature_ledger_total{{outcome=\"{outcome}\"}} ");
-        crate::metrics::encode()
-            .expect("encode should succeed after init")
-            .lines()
-            .find_map(|line| line.strip_prefix(needle.as_str()))
-            .map(|value| value.trim().parse().expect("counter value is an integer"))
-            .unwrap_or(0)
-    }
-
-    /// One route, mounted behind `consume_signature` alone. The layer reads the
-    /// `SignatureIdentity` extension, so tests inject it directly rather than
-    /// producing real RFC-9421 signatures.
-    fn ledger_app(state: crate::state::AppState) -> Router {
-        Router::new()
-            .route(
-                "/probe",
-                axum::routing::post(|| async { StatusCode::OK }).get(|| async { StatusCode::OK }),
-            )
-            .layer(middleware::from_fn_with_state(state, consume_signature))
-    }
-
-    fn ledger_identity(seed: &str, nonce: Option<&str>) -> SignatureIdentity {
-        SignatureIdentity {
-            keyid: format!("did:key:zLedgerMetrics{seed}"),
-            key_fingerprint: hex::encode(Sha256::digest(format!("key/{seed}").as_bytes())),
-            nonce: nonce.map(str::to_owned),
-            signing_string_hash: hex::encode(Sha256::digest(format!("sig/{seed}").as_bytes())),
-        }
-    }
-
-    fn ledger_request(identity: Option<SignatureIdentity>) -> Request {
-        let builder = Request::builder().method(Method::POST).uri("/probe");
-        let builder = match identity {
-            Some(identity) => builder.extension(identity),
-            None => builder,
-        };
-        builder.body(Body::empty()).expect("request builder")
-    }
-
-    async fn ledger_status(state: crate::state::AppState, request: Request) -> StatusCode {
-        ledger_app(state)
-            .oneshot(request)
-            .await
-            .expect("router response")
-            .status()
-    }
-
-    #[tokio::test]
-    async fn ledger_metrics_count_a_skipped_read() {
-        crate::metrics::init("0.0.0-test", "did:key:test");
-        let before = ledger_metric("skipped_read");
-
-        let request = Request::builder()
-            .method(Method::GET)
-            .uri("/probe")
-            .body(Body::empty())
-            .expect("request builder");
-        let status = ledger_status(make_test_state(Keypair::generate().did()), request).await;
-
-        assert_eq!(status, StatusCode::OK, "a read must still pass through");
-        assert!(
-            ledger_metric("skipped_read") > before,
-            "the read skip must be counted, or the outcomes do not sum to the layer's traffic"
-        );
-    }
-
-    #[tokio::test]
-    async fn ledger_metrics_count_a_missing_identity() {
-        crate::metrics::init("0.0.0-test", "did:key:test");
-        let before = ledger_metric("identity_missing");
-
-        let status = ledger_status(
-            make_test_state(Keypair::generate().did()),
-            ledger_request(None),
-        )
-        .await;
-
-        assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR);
-        assert!(ledger_metric("identity_missing") > before);
-    }
-
-    #[tokio::test]
-    async fn ledger_metrics_count_the_nonce_rejections() {
-        use clap::Parser;
-
-        crate::metrics::init("0.0.0-test", "did:key:test");
-        let mut state = make_test_state(Keypair::generate().did());
-        state.config = Arc::new(crate::config::Config::parse_from([
-            "gitlawb-node",
-            "--require-signature-nonce",
-        ]));
-
-        let before_required = ledger_metric("nonce_required");
-        let status = ledger_status(
-            state.clone(),
-            ledger_request(Some(ledger_identity("no-nonce", None))),
-        )
-        .await;
-        assert_eq!(status, StatusCode::BAD_REQUEST);
-        assert!(ledger_metric("nonce_required") > before_required);
-
-        let before_short = ledger_metric("nonce_too_short");
-        let status = ledger_status(
-            state,
-            ledger_request(Some(ledger_identity("short-nonce", Some("abc")))),
-        )
-        .await;
-        assert_eq!(status, StatusCode::BAD_REQUEST);
-        assert!(ledger_metric("nonce_too_short") > before_short);
-    }
-
-    /// The fail-closed 503 is the outcome an operator most needs a numerator
-    /// for: it is returned by the ledger while the handlers are healthy. The
-    /// state here carries a lazy pool pointed at a database that does not
-    /// exist, so `consume_signature` returns `Err`.
-    #[tokio::test]
-    async fn ledger_metrics_count_a_ledger_error() {
-        crate::metrics::init("0.0.0-test", "did:key:test");
-        let before = ledger_metric("unavailable");
-
-        let status = ledger_status(
-            make_test_state(Keypair::generate().did()),
-            ledger_request(Some(ledger_identity("db-down", None))),
-        )
-        .await;
-
-        assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE);
-        assert!(ledger_metric("unavailable") > before);
-    }
-
-    #[sqlx::test]
-    async fn ledger_metrics_count_admitted_and_replayed(pool: sqlx::PgPool) {
-        crate::metrics::init("0.0.0-test", "did:key:test");
-        let state = crate::test_support::test_state(pool).await;
-        let identity = ledger_identity("spend-once", None);
-
-        let before_admitted = ledger_metric("admitted");
-        let status = ledger_status(state.clone(), ledger_request(Some(identity.clone()))).await;
-        assert_eq!(status, StatusCode::OK, "a fresh signature is admitted");
-        assert!(ledger_metric("admitted") > before_admitted);
-
-        let before_replayed = ledger_metric("replayed");
-        let status = ledger_status(state, ledger_request(Some(identity))).await;
-        assert_eq!(status, StatusCode::CONFLICT, "the same signature replays");
-        assert!(ledger_metric("replayed") > before_replayed);
-    }
-
-    #[sqlx::test]
-    async fn ledger_metrics_count_a_full_identity_ledger(pool: sqlx::PgPool) {
-        crate::metrics::init("0.0.0-test", "did:key:test");
-        let state = crate::test_support::test_state(pool.clone()).await;
-        let identity = ledger_identity("capped", None);
-
-        // Seeded through SQL rather than 512 requests. The cap is charged
-        // against the resolved key, which is the `keyid` column's value.
-        sqlx::query(
-            "INSERT INTO consumed_signatures (sig_hash, keyid, expires_at)
-             SELECT md5(i::text) || md5((i + 1)::text), $1, $2
-             FROM generate_series(1, $3) AS i",
-        )
-        .bind(&identity.key_fingerprint)
-        .bind(9_000_000_000i64)
-        .bind(crate::db::MAX_LIVE_SIGNATURES_PER_KEYID)
-        .execute(&pool)
-        .await
-        .expect("seed a full ledger for this identity");
-
-        let before = ledger_metric("identity_ledger_full");
-        let status = ledger_status(state, ledger_request(Some(identity))).await;
-
-        assert_eq!(status, StatusCode::TOO_MANY_REQUESTS);
-        assert!(ledger_metric("identity_ledger_full") > before);
-    }
-
     #[tokio::test]
     async fn require_ucan_chain_no_header_passes_through() {
         let state = make_test_state(Keypair::generate().did());

crates/gitlawb-node/src/config.rs

@@ -76,14 +76,6 @@ pub struct Config {
     )]
     pub require_signed_peer_writes: bool,
 
-    /// Require an RFC 9421 `nonce` parameter on signatures that authorize a
-    /// mutation. Keep false until every client signs one: nodes fall back to
-    /// keying the replay ledger on the signing-string hash without it, which
-    /// rejects a byte-identical mutation repeated inside the same second.
-    /// Enforced on write routes only, so signed reads are never affected.
-    #[arg(long, env = "GITLAWB_REQUIRE_SIGNATURE_NONCE", default_value_t = false)]
-    pub require_signature_nonce: bool,
-
     /// Require the authenticated pusher to be the repo owner on `git-receive-pack`.
     /// Authentication (a valid did:key signature) is not authorization on its own:
     /// any party can mint a did:key and sign as it, so with this off every signed
@@ -205,19 +197,6 @@ pub struct Config {
     #[arg(long, env = "GITLAWB_PEER_WRITE_RATE_LIMIT", default_value_t = 600)]
     pub peer_write_rate_limit: usize,
 
-    /// Per-client-IP rate limit for the signed write routes (tasks, pull-request
-    /// merge/close/review/comment, webhooks, branch protection, stars, replicas,
-    /// labels, visibility, agent deregistration, bounties, profile, issue
-    /// close/comment), in requests per hour. Every signed attempt on these routes
-    /// commits a `consumed_signatures` row before the handler runs any
-    /// authorization or existence check, and a signature proves nothing about
-    /// registration (any freshly minted `did:key` self-signs), so without a brake
-    /// an unregistered caller can force a durable write per request. Its own
-    /// bucket, so a flood here cannot drain the creation, push, or peer-sync
-    /// quotas. `0` disables. Default: 600.
-    #[arg(long, env = "GITLAWB_SIGNED_WRITE_RATE_LIMIT", default_value_t = 600)]
-    pub signed_write_rate_limit: usize,
-
     /// Optional address to bind a Prometheus `/metrics` exposition endpoint on.
     /// Example: `127.0.0.1:9091`. Leave empty (default) to disable.
     /// Bind to localhost or a private interface — the metrics endpoint is
@@ -273,11 +252,9 @@ 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. 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`.
+    /// 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`).
     #[arg(
         long,
         env = "GITLAWB_DB_MAX_CONNECTIONS",
@@ -286,6 +263,46 @@ 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(
@@ -380,15 +397,14 @@ 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 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.
+    /// 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.
     #[arg(
         long,
         env = "GITLAWB_MAX_CONCURRENT_GIT_PUSHES",
@@ -746,25 +762,37 @@ 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> {
-        // 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 {
+        // 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 {
             return Err(format!(
-                "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,
+                "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,
                 self.max_concurrent_git_pushes,
-                Self::DB_POOL_APP_HEADROOM,
-                floor
+                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
             ));
         }
         Ok(())
@@ -775,6 +803,25 @@ 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!(
@@ -792,75 +839,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn require_signature_nonce_ships_off() {
-        assert!(
-            !Config::parse_from(["gitlawb-node"]).require_signature_nonce,
-            "the staged flag must default to false"
-        );
-        assert!(
-            Config::parse_from(["gitlawb-node", "--require-signature-nonce"])
-                .require_signature_nonce
-        );
-    }
-
-    /// Child half of [`nonce_flag_env_values_never_silently_enforce`]. It runs in
-    /// a subprocess with `GITLAWB_REQUIRE_SIGNATURE_NONCE` set, because setting
-    /// that variable in-process would race every other test that parses a
-    /// `Config` and would take the whole binary down if the value were invalid.
-    #[test]
-    #[ignore = "driven as a subprocess by nonce_flag_env_values_never_silently_enforce"]
-    fn nonce_flag_env_probe() {
-        match Config::try_parse_from(["gitlawb-node"]) {
-            Ok(config) => println!("PROBE:ok:{}", config.require_signature_nonce),
-            Err(_) => println!("PROBE:err"),
-        }
-    }
-
-    /// The flag is read as a boolean, not as "is the variable present". An
-    /// operator writing `GITLAWB_REQUIRE_SIGNATURE_NONCE=0` (or leaving it
-    /// empty) must never end up enforcing.
-    #[test]
-    fn nonce_flag_env_values_never_silently_enforce() {
-        fn probe(value: &str) -> String {
-            let out = std::process::Command::new(
-                std::env::current_exe().expect("path to the test binary"),
-            )
-            .args([
-                "--exact",
-                "config::tests::nonce_flag_env_probe",
-                "--ignored",
-                "--nocapture",
-            ])
-            .env("GITLAWB_REQUIRE_SIGNATURE_NONCE", value)
-            .output()
-            .expect("run the env probe");
-            let stdout = String::from_utf8_lossy(&out.stdout);
-            stdout
-                .lines()
-                .find(|l| l.starts_with("PROBE:"))
-                .unwrap_or("PROBE:missing")
-                .to_string()
-        }
-
-        // The probe can observe enforcement, so a "never true" assertion below
-        // is not vacuous.
-        assert_eq!(probe("true"), "PROBE:ok:true");
-
-        for value in ["false", "0", ""] {
-            let observed = probe(value);
-            assert_ne!(
-                observed, "PROBE:ok:true",
-                "GITLAWB_REQUIRE_SIGNATURE_NONCE={value:?} must not enforce, got {observed}"
-            );
-        }
-        assert_eq!(
-            probe("false"),
-            "PROBE:ok:false",
-            "the documented disable value must parse to false, not error"
-        );
-    }
-
     /// #174 (RED-before/GREEN-after): the upper bound is what keeps every duration
     /// derived from this knob in range — the lease steal bound's `* 2 + 60` on the write
     /// path, and the `Instant::now() + Duration::from_secs(..)` deadlines in
@@ -1453,41 +1431,61 @@ mod tests {
         );
     }
 
-    /// #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.
+    /// #174 F1: concurrent git writes pin the dedicated lock pool, not the main
+    /// pool. `validate()` must reject an under-sized lock pool at boot.
     #[test]
     fn db_pool_must_clear_the_git_push_cap() {
-        // Shipped defaults validate (48 >= 32 + 8).
+        // Shipped defaults validate (lock pool 40 >= pushes 32 + non-push headroom 8).
         Config::parse_from(["gitlawb-node"])
             .validate()
             .expect("default config must validate");
 
-        // An under-sized pool relative to the push cap is rejected (20 < 32 + 8).
-        let under = Config::parse_from([
+        // An under-sized lock pool relative to the push cap is rejected.
+        let under_lock = Config::parse_from([
             "gitlawb-node",
-            "--db-max-connections",
-            "20",
+            "--db-lock-pool-max-connections",
+            "16",
             "--max-concurrent-git-pushes",
             "32",
         ]);
         assert!(
-            under.validate().is_err(),
-            "db_max_connections 20 below max_concurrent_git_pushes 32 + headroom must be rejected"
+            under_lock.validate().is_err(),
+            "db_lock_pool_max_connections below max_concurrent_git_pushes + headroom must be rejected"
         );
 
-        // Exactly at the floor validates (40 == 32 + 8).
-        let at_floor = Config::parse_from([
+        // 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([
             "gitlawb-node",
             "--db-max-connections",
+            "16",
+            "--db-lock-pool-max-connections",
             "40",
             "--max-concurrent-git-pushes",
             "32",
         ]);
         assert!(
-            at_floor.validate().is_ok(),
-            "db_max_connections at the floor (pushes + headroom) must validate"
+            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"
         );
     }
 

crates/gitlawb-node/src/error.rs

@@ -69,6 +69,15 @@ 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),
 
@@ -110,7 +119,31 @@ impl From<anyhow::Error> for AppError {
     fn from(err: anyhow::Error) -> Self {
         match err.downcast::<sqlx::Error>() {
             Ok(sql) => AppError::Db(sql),
-            Err(err) => AppError::Internal(err),
+            // 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),
+                        },
+                    },
+                },
+            },
         }
     }
 }
@@ -165,13 +198,9 @@ impl IntoResponse for AppError {
             AppError::UnresolvableDid(msg) => {
                 (StatusCode::BAD_REQUEST, "unresolvable_did", msg.clone())
             }
-            // Same wire code as the flood brake's `too_many_requests`, taken
-            // from the shared enum rather than retyped so the two cannot drift.
-            AppError::TooManyRequests(msg) => (
-                StatusCode::TOO_MANY_REQUESTS,
-                gitlawb_core::node_denial::NodeDenial::RateLimited.as_str(),
-                msg.clone(),
-            ),
+            AppError::TooManyRequests(msg) => {
+                (StatusCode::TOO_MANY_REQUESTS, "rate_limited", msg.clone())
+            }
             AppError::Incomplete(msg) => {
                 (StatusCode::UNPROCESSABLE_ENTITY, "incomplete", msg.clone())
             }
@@ -188,6 +217,29 @@ 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,
@@ -242,20 +294,17 @@ impl IntoResponse for AppError {
         }
 
         let mut resp = (status, Json(body)).into_response();
-        if matches!(self, AppError::TooManyRequests(_)) {
-            use axum::http::HeaderValue;
-            let code = gitlawb_core::node_denial::NodeDenial::RateLimited.as_str();
-            if let Ok(v) = HeaderValue::from_str(code) {
-                resp.headers_mut().insert("X-Gitlawb-Error", v);
-            }
-        }
         // Both retryable 503s advertise when to retry: Overloaded (capacity shed) and
         // SearchIncomplete (a bounded CID search cut short by a cap — retry may complete
         // it). They ride the shared tail above for body/status, so the header is attached
         // here rather than in bespoke early returns, keeping each variant handled once.
         if matches!(
             self,
-            AppError::Overloaded(_) | AppError::SearchIncomplete { .. }
+            AppError::Overloaded(_)
+                | AppError::SearchIncomplete { .. }
+                | AppError::RepoBusy
+                | AppError::RepoUnavailable
+                | AppError::RepoWriteFenced
         ) {
             resp.headers_mut().insert(
                 axum::http::header::RETRY_AFTER,
@@ -285,17 +334,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn too_many_requests_carries_rate_limited_header() {
-        use axum::http::HeaderValue;
-        let resp = AppError::TooManyRequests("slow down".into()).into_response();
-        assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS);
-        assert_eq!(
-            resp.headers().get("X-Gitlawb-Error"),
-            Some(&HeaderValue::from_static("rate_limited"))
-        );
-    }
-
     #[test]
     fn overloaded_maps_to_503_with_retry_after() {
         let resp = AppError::Overloaded("x".into()).into_response();
@@ -384,4 +422,31 @@ 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,6 +61,27 @@ 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/
@@ -278,6 +299,27 @@ 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,6 +870,7 @@ 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,12 +5,41 @@
 
 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 {
@@ -31,21 +60,29 @@ impl TigrisClient {
         })
     }
 
-    /// 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.
+    /// 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.
     #[cfg(test)]
-    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;
+    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();
         Self {
-            s3: S3Client::new(&config),
+            s3: S3Client::from_conf(config),
             bucket: bucket.to_string(),
         }
     }
@@ -77,8 +114,47 @@ impl TigrisClient {
         }
     }
 
-    /// 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<()> {
+    /// 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> {
         let key = Self::repo_key(owner_slug, repo_name);
         debug!(key = %key, path = %local_path.display(), "uploading repo to tigris");
 
@@ -93,15 +169,57 @@ impl TigrisClient {
 
         let body = aws_sdk_s3::primitives::ByteStream::from(archive_bytes);
 
-        self.s3
+        let mut req = self
+            .s3
             .put_object()
             .bucket(&self.bucket)
             .key(&key)
             .body(body)
-            .content_type("application/zstd")
-            .send()
-            .await
-            .context(format!("tigris PUT {key}"))?;
+            .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}")),
+            ));
+        }
 
         info!(key = %key, "uploaded repo to tigris");
         Ok(())
@@ -112,10 +230,32 @@ impl TigrisClient {
         &self,
         owner_slug: &str,
         repo_name: &str,
-        local_path: &Path,
+        local_path: &super::repo_store::ValidatedRepoDiskPath,
+        swap_authority: Option<Arc<AtomicBool>>,
     ) -> 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 = %local_path.display(), "downloading repo from tigris");
+        debug!(key = %key, path = %target.as_path().display(), "downloading repo from tigris");
 
         let resp = self
             .s3
@@ -133,21 +273,62 @@ impl TigrisClient {
             .context("reading tigris response body")?
             .into_bytes();
 
-        // Extract tar.zst to local path
-        tokio::task::spawn_blocking({
-            let local_path = local_path.to_path_buf();
-            move || decompress_repo(&data, &local_path)
+        // 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
+            }
         })
         .await
         .context("extract task panicked")?
         .context("extracting repo")?;
 
-        info!(key = %key, path = %local_path.display(), "downloaded repo from tigris");
-        Ok(())
+        info!(key = %key, path = %target.as_path().display(), "downloaded repo from tigris");
+        Ok(extracted)
     }
 
     /// 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
@@ -180,7 +361,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.
-fn publish_lock(local_path: &Path) -> Arc<Mutex<()>> {
+pub(crate) 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
@@ -193,6 +374,35 @@ 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
@@ -200,11 +410,16 @@ fn publish_lock(local_path: &Path) -> Arc<Mutex<()>> {
 /// 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: &Path) -> Result<()> {
-    let parent = local_path.parent().context("repo path has no parent")?;
+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")?;
     std::fs::create_dir_all(parent).context("creating parent dir")?;
 
-    let file_name = local_path
+    let file_name = live
         .file_name()
         .context("repo path has no file name")?
         .to_string_lossy();
@@ -229,17 +444,218 @@ fn decompress_repo(data: &[u8], local_path: &Path) -> Result<()> {
         return Err(e);
     }
 
-    // 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")?;
+    // 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)?;
 
     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)),
+                Ok(p) if p.is_dir() => out.push((repo, created_at_key, p.into_path_buf())),
                 Ok(_) => {}
                 Err(e) => {
                     tracing::warn!(repo_id = %repo.id, err = %e, "sweep discovery: rejected unsafe repo path");

crates/gitlawb-node/src/main.rs

@@ -32,7 +32,7 @@ use axum::routing::get;
 use axum::{Json, Router};
 use clap::Parser;
 use std::collections::HashSet;
-use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
+use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::Arc;
 use tokio::net::TcpListener;
 use tokio::sync::watch;
@@ -51,59 +51,12 @@ struct DegradedState {
     db_startup: Arc<DbStartupStatus>,
 }
 
-/// Independent fields with no cross-field invariant — atomics, not a lock, so
-/// the retry loop and the degraded handlers never contend.
+/// Two independent counters with no cross-field invariant — atomics, not a
+/// lock, so the retry loop and the degraded handlers never contend.
 #[derive(Default)]
 struct DbStartupStatus {
     attempts: AtomicU64,
     next_retry_secs: AtomicU64,
-    /// Set once the retry loop sees a migration version collision. Latched:
-    /// the condition is a code-level numbering bug that cannot heal between
-    /// attempts, and it changes what the readiness payload must say.
-    schema_conflict: AtomicBool,
-}
-
-impl DbStartupStatus {
-    fn mark_schema_conflict(&self) {
-        self.schema_conflict.store(true, Ordering::Relaxed);
-    }
-
-    fn schema_conflict(&self) -> bool {
-        self.schema_conflict.load(Ordering::Relaxed)
-    }
-}
-
-/// Distinct from `error::DB_UNAVAILABLE_CODE`: the database is reachable and
-/// healthy, the schema it holds disagrees with this build. Retrying will never
-/// clear it, so the readiness payload must not read as "still initializing".
-const DB_SCHEMA_CONFLICT_CODE: &str = "db_schema_conflict";
-const DB_SCHEMA_CONFLICT_MESSAGE: &str =
-    "database schema conflicts with this build; operator action is required";
-
-/// Hard ceiling on the advisory-lock pool's `max_connections`.
-///
-/// `max_concurrent_git_pushes` is validated all the way up to 1_048_576, and the lock
-/// pool used to derive its size straight from that knob, so raising the push cap
-/// silently raised the node's Postgres connection ceiling with no CLI error and no
-/// relation to the server's own `max_connections` (#173 F4). The node's total budget is
-/// now bounded: `db_max_connections` (default 48) + at most this.
-const LOCK_POOL_MAX_CONNECTIONS: u32 = 64;
-
-/// Connections the lock pool keeps above the push cap. Covers the three non-push
-/// `acquire_write` callers (`api/issues.rs` x2, `api/pulls.rs`), which hold no
-/// concurrency permit, so a push never queues here for a connection where it did not
-/// before.
-const LOCK_POOL_PUSH_HEADROOM: u8 = 8;
-
-/// Size the advisory-lock pool for a given push cap: the cap plus
-/// [`LOCK_POOL_PUSH_HEADROOM`], clamped to [`LOCK_POOL_MAX_CONNECTIONS`]. Past the
-/// clamp a push may wait for a lock-pool connection, which is a bounded wait that sheds
-/// a clean 503 (see `LockPoolBusy`), not an unbounded hang.
-fn lock_pool_size(max_concurrent_git_pushes: usize) -> u32 {
-    u32::try_from(max_concurrent_git_pushes)
-        .unwrap_or(u32::MAX)
-        .saturating_add(u32::from(LOCK_POOL_PUSH_HEADROOM))
-        .min(LOCK_POOL_MAX_CONNECTIONS)
 }
 
 #[tokio::main]
@@ -137,8 +90,6 @@ async fn main() -> Result<()> {
 
     // Load or generate the node's identity keypair
     let keypair = load_or_create_keypair(&config)?;
-    // Sealing key for the legacy-scan continuation tokens, DERIVED from the identity
-    // just loaded so it is the same key after a restart (see `derive_scan_token_key`).
     let scan_token_key = AppState::derive_scan_token_key(&keypair);
     let node_did = keypair.did();
 
@@ -337,20 +288,20 @@ async fn main() -> Result<()> {
         None
     };
 
-    // Repo write locks run on their own pool, never the main query pool: each push
-    // holds its connection for the whole receive-pack, so a burst of concurrent
-    // pushes drawing from the main pool would park that many connections for the
-    // duration of their receive-packs and starve every other query. That holds
-    // whatever the two pools are sized at, which is why the separation is
-    // structural rather than a consequence of the defaults; config validate()
-    // separately requires db_max_connections >= max_concurrent_git_pushes + 8. See
-    // build_lock_pool for the cancellation semantics (#173).
-    let lock_pool = git::repo_store::build_lock_pool(
-        db.pool(),
-        lock_pool_size(config.max_concurrent_git_pushes),
+    // Advisory-lock connections come from their own pool: a write guard pins one
+    // for its whole lifetime, and sharing the application pool would let a push
+    // burst starve ordinary request handlers.
+    let lock_pool = db::Db::lock_pool(
+        &config.database_url,
+        config.db_lock_pool_max_connections,
         std::time::Duration::from_secs(config.db_acquire_timeout_secs),
+    )?;
+    let repo_store = git::repo_store::RepoStore::new(
+        config.repos_dir.clone(),
+        tigris,
+        lock_pool,
+        std::time::Duration::from_secs(config.lock_held_transfer_timeout_secs),
     );
-    let repo_store = git::repo_store::RepoStore::new(config.repos_dir.clone(), tigris, lock_pool);
 
     // Per-DID limiter for the creation endpoints. Keyed on the authenticated
     // DID (attacker-varied), so bound its key set to cap memory.
@@ -395,6 +346,23 @@ async fn main() -> Result<()> {
         tracing::warn!("GITLAWB_PUSH_RATE_LIMIT=0 — per-IP push rate limiting disabled");
     }
 
+    // close_issue drives a full archive download for non-owner author pre-checks.
+    // Keep it on its own bucket so it cannot drain the receive-pack limit.
+    let close_issue_limit = std::env::var("GITLAWB_CLOSE_ISSUE_RATE_LIMIT")
+        .ok()
+        .and_then(|v| v.trim().parse::<usize>().ok())
+        .unwrap_or(120);
+    let close_issue_rate_limiter = rate_limit::RateLimiter::new_bounded(
+        close_issue_limit,
+        std::time::Duration::from_secs(3600),
+        200_000,
+    );
+    if close_issue_limit == 0 {
+        tracing::warn!(
+            "GITLAWB_CLOSE_ISSUE_RATE_LIMIT=0 — per-IP close_issue rate limiting disabled"
+        );
+    }
+
     // Which forwarded header the edge is trusted to set. Default None (trust
     // nothing, key on the socket peer). Fly nodes set GITLAWB_TRUSTED_PROXY=fly;
     // a node behind Caddy/NGINX sets it to x-forwarded-for.
@@ -417,22 +385,6 @@ async fn main() -> Result<()> {
         std::time::Duration::from_secs(3600),
         200_000,
     );
-    // Flood brake for the signed write routes. Every signed attempt there spends
-    // a `consumed_signatures` row before the handler checks anything, and any
-    // freshly minted did:key signs, so the brake is what bounds the durable
-    // writes an unregistered caller can force. Its own bucket so it cannot drain
-    // the creation, push, or peer-sync quotas. Bounded key set (a
-    // client-influenced IP); 0 disables.
-    let signed_write_rate_limiter = rate_limit::RateLimiter::new_bounded(
-        config.signed_write_rate_limit,
-        std::time::Duration::from_secs(3600),
-        200_000,
-    );
-    if config.signed_write_rate_limit == 0 {
-        tracing::warn!(
-            "GITLAWB_SIGNED_WRITE_RATE_LIMIT=0 — signed-write IP rate limiting disabled"
-        );
-    }
     if config.sync_trigger_rate_limit == 0 {
         tracing::warn!(
             "GITLAWB_SYNC_TRIGGER_RATE_LIMIT=0 — /sync/trigger IP rate limiting disabled"
@@ -460,18 +412,10 @@ async fn main() -> Result<()> {
         rate_limiter,
         create_ip_rate_limiter,
         push_rate_limiter,
+        close_issue_rate_limiter,
         ipfs_max_history_walks: crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST,
-        // The legacy-probe budget is operator-tunable via GITLAWB_IPFS_MAX_LEGACY_PROBES
-        // (R5), which is what `ipfs_legacy_probe_budget` reads. NOT
-        // GITLAWB_IPFS_MAX_REPOS_WALKED, which this comment used to name: that is the
-        // separate cap on expensive visibility walks, so an operator following the old
-        // text tuned the walk cap and left this fan-out unchanged. The history-walk
-        // ceiling above stays constant (a smaller value false-503s a provenanced
-        // request). Default 256 preserves the shipped behaviour.
         ipfs_max_legacy_probes: AppState::ipfs_legacy_probe_budget(&config),
         ipfs_legacy_scan_page_rows: crate::api::ipfs::LEGACY_SCAN_PAGE_ROWS,
-        // Operator-tunable via GITLAWB_IPFS_MAX_LEGACY_SCAN_ROWS, read through the same
-        // helper shape as the probe budget so the knob cannot be a silent no-op.
         ipfs_max_legacy_scan_rows: AppState::ipfs_legacy_scan_row_budget(&config),
         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),
@@ -479,7 +423,6 @@ async fn main() -> Result<()> {
         push_limiter_trust,
         sync_trigger_rate_limiter,
         peer_write_rate_limiter,
-        signed_write_rate_limiter,
         shutdown_tx: shutdown_tx.clone(),
         git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(config.max_concurrent_git_ops)),
         git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(
@@ -551,10 +494,6 @@ async fn main() -> Result<()> {
             std::time::Duration::from_secs(3600),
             200_000,
         ),
-        // Separate WORK-budget bucket for the resolver's per-probe/per-walk charges (R6).
-        // Its capacity is DERIVED from the route limit (no new knob) and floored at the
-        // legacy-probe budget, so one full default-config legacy scan never self-throttles
-        // mid-request while the route brake above stays the pure once-per-request cap.
         ipfs_work_rate_limiter: rate_limit::RateLimiter::new_bounded(
             AppState::ipfs_work_budget(&config),
             std::time::Duration::from_secs(3600),
@@ -592,17 +531,16 @@ async fn main() -> Result<()> {
         });
     }
 
-    // Periodic cleanup of expired rate limit entries, the consumed-proof
-    // ledger, and the HTTP-signature replay ledger
+    // Periodic cleanup of expired rate limit entries + consumed-proof ledger
     {
-        let cleanup_state = state.clone();
+        let sweep_state = state.clone();
         let db = state.db.clone();
         let mut shutdown_rx = state.subscribe_shutdown();
         tokio::spawn(async move {
             loop {
                 tokio::select! {
                     _ = tokio::time::sleep(std::time::Duration::from_secs(300)) => {
-                        cleanup_state.sweep_rate_limiters().await;
+                        sweep_state.sweep_rate_limiters().await;
                         let now = std::time::SystemTime::now()
                             .duration_since(std::time::UNIX_EPOCH)
                             .map(|d| d.as_secs() as i64)
@@ -610,9 +548,6 @@ async fn main() -> Result<()> {
                         if let Err(e) = db.sweep_expired_proofs(now).await {
                             tracing::warn!(err = %e, "failed to sweep expired iCaptcha proofs");
                         }
-                        if let Err(e) = db.sweep_expired_signatures(now).await {
-                            tracing::warn!(err = %e, "failed to sweep expired signature ledger rows");
-                        }
                     }
                     _ = shutdown_rx.changed() => {
                         if *shutdown_rx.borrow() {
@@ -748,44 +683,6 @@ async fn main() -> Result<()> {
     Ok(())
 }
 
-/// U4 (#173): spawn the periodic legacy provider-CID repair sweep. Releases before this
-/// version stored the PROVIDER CID (Kubo dag-pb / Pinata CIDv0) in `pinned_cids.cid`,
-/// and this version's `/ipfs/{cid}` resolver withholds any row whose stored key is not
-/// the raw-content CID. The opportunistic repair on the pin path only fires when a push
-/// re-carries the object, which normal git negotiation makes it not do, so those rows
-/// need a walk. DETACHED, never on the boot path: the caller keeps serving while this
-/// runs, and the sweep's own batch bound plus inter-batch delay keep it off the DB's
-/// critical path. Its cursor is durable, so a restart mid-walk resumes instead of
-/// rewinding. It re-arms after every run, including a failed one, so it never returns
-/// and there is no awaited value to log here; the shutdown watcher below is what ends
-/// it.
-///
-/// A named function rather than an inline block in `main` so the WIRING has a seam a
-/// test can call: that the task is spawned at all, that it reads its batch and delay
-/// from the config knobs rather than some other field, that the caller is not blocked
-/// on it, and that the shutdown watcher actually ends it mid-walk. The sweep's own
-/// behavior is covered elsewhere; this is the boot-path half.
-fn spawn_legacy_cid_sweep(state: &AppState, config: &Config) -> tokio::task::JoinHandle<()> {
-    let db = state.db.clone();
-    let repos_dir = config.repos_dir.clone();
-    let git_bin = state.git_bin.clone();
-    let git_timeout = std::time::Duration::from_secs(config.git_service_timeout_secs);
-    let batch = config.pin_repair_sweep_batch;
-    let delay = std::time::Duration::from_secs(config.pin_repair_sweep_delay_secs);
-    let mut shutdown_rx = state.subscribe_shutdown();
-    tokio::spawn(async move {
-        tokio::select! {
-            _ = ipfs_pin::run_sweep_rearmed(
-                &repos_dir, &git_bin, git_timeout, batch, delay,
-                ipfs_pin::SWEEP_REARM_DELAY, &db,
-            ) => {}
-            // Shutdown mid-walk simply drops the run; the persisted cursor means the
-            // next boot picks up where this one stopped.
-            _ = shutdown_rx.changed() => {}
-        }
-    })
-}
-
 fn spawn_shutdown_signal(tx: watch::Sender<bool>) {
     tokio::spawn(async move {
         #[cfg(unix)]
@@ -861,14 +758,7 @@ async fn connect_db_with_retry(
                 // the provider — and take liveness down with it), but log at
                 // error level and skip straight to the maximum backoff; the
                 // /ready health check is what surfaces this to deploys.
-                // One call decides the backoff, the log level and the readiness
-                // signal, and applies the latch. Everything below reads from
-                // its result, so the branch cannot drift away from the latch.
-                let fault = record_db_startup_failure(&err, &db_startup);
-                let DbStartupFault {
-                    permanent,
-                    schema_conflict,
-                } = fault;
+                let permanent = is_likely_permanent_db_error(&err);
                 let retry_secs = if permanent {
                     max_retry_secs
                 } else {
@@ -877,14 +767,7 @@ async fn connect_db_with_retry(
                 db_startup
                     .next_retry_secs
                     .store(retry_secs, Ordering::Relaxed);
-                if schema_conflict {
-                    tracing::error!(
-                        attempts,
-                        retry_secs,
-                        err = %err,
-                        "database schema conflicts with this build's migration catalogue; this will not heal on retry; operator action is required"
-                    );
-                } else if permanent {
+                if permanent {
                     tracing::error!(
                         attempts,
                         retry_secs,
@@ -913,52 +796,12 @@ async fn connect_db_with_retry(
     }
 }
 
-/// What a failed startup attempt means for the retry loop and for what the
-/// node tells the outside world.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-struct DbStartupFault {
-    /// Jump straight to the maximum backoff and log at error level.
-    permanent: bool,
-    /// The schema disagrees with this build. Latched into `DbStartupStatus`,
-    /// so the degraded 503 body stops saying "initializing".
-    schema_conflict: bool,
-}
-
-/// Classify a failed startup attempt **and** record its consequence on
-/// `db_startup`. The two are one function on purpose: a classification that
-/// never reaches the readiness payload leaves /ready reporting "database
-/// initializing" forever for a fault that cannot heal, and separate
-/// classify-then-latch steps can silently drift apart. `connect_db_with_retry`
-/// derives its backoff and log level from the returned value, so the call site
-/// cannot keep the decision while dropping the latch.
-fn record_db_startup_failure(err: &anyhow::Error, db_startup: &DbStartupStatus) -> DbStartupFault {
-    let schema_conflict = err
-        .downcast_ref::<db::MigrationVersionCollision>()
-        .is_some();
-    if schema_conflict {
-        db_startup.mark_schema_conflict();
-    }
-    DbStartupFault {
-        permanent: is_likely_permanent_db_error(err),
-        schema_conflict,
-    }
-}
-
 /// Errors that indicate misconfiguration rather than a transient outage: a
-/// malformed DATABASE_URL, a server that answered and rejected us — Postgres
-/// error class 28xxx (invalid authorization) or 3D000 (database does not
-/// exist), or a migration version collision. Best-effort: an error that anyhow
-/// can't downcast back to a known type just counts as transient.
+/// malformed DATABASE_URL, or a server that answered and rejected us —
+/// Postgres error class 28xxx (invalid authorization) or 3D000 (database
+/// does not exist). Best-effort: an error that anyhow can't downcast back to
+/// sqlx just counts as transient.
 fn is_likely_permanent_db_error(err: &anyhow::Error) -> bool {
-    // A version collision is a code-level numbering bug. It is the loudest
-    // thing the migration guard can report, so it must never be filed under
-    // "database unavailable, retrying".
-    if err
-        .downcast_ref::<db::MigrationVersionCollision>()
-        .is_some()
-    {
-        return true;
-    }
     match err.downcast_ref::<sqlx::Error>() {
         Some(sqlx::Error::Configuration(_)) => true,
         Some(sqlx::Error::Database(db)) => db
@@ -1022,27 +865,11 @@ fn build_degraded_router(node_did: String, db_startup: Arc<DbStartupStatus>) ->
 /// vocabulary with error.rs so clients see the same code/message for
 /// "database unavailable" regardless of which phase produced it.
 fn degraded_body(db_startup: &DbStartupStatus) -> serde_json::Value {
-    // A schema conflict is not a database that is still coming up. Saying
-    // "initializing" for it hides a permanent, operator-actionable fault behind
-    // a status that reads as "wait longer".
-    let (state, code, message) = if db_startup.schema_conflict() {
-        (
-            "schema_conflict",
-            DB_SCHEMA_CONFLICT_CODE,
-            DB_SCHEMA_CONFLICT_MESSAGE,
-        )
-    } else {
-        (
-            "initializing",
-            error::DB_UNAVAILABLE_CODE,
-            error::DB_UNAVAILABLE_MESSAGE,
-        )
-    };
     serde_json::json!({
         "status": "degraded",
-        "database": state,
-        "error": code,
-        "message": message,
+        "database": "initializing",
+        "error": error::DB_UNAVAILABLE_CODE,
+        "message": error::DB_UNAVAILABLE_MESSAGE,
         "db_attempts": db_startup.attempts.load(Ordering::Relaxed),
         "db_next_retry_secs": db_startup.next_retry_secs.load(Ordering::Relaxed),
     })
@@ -1140,40 +967,6 @@ fn build_operator_client(
     Ok(operator::OperatorClient::new(cfg))
 }
 
-/// A bootstrap peer's answer to our signed announce, reduced to what the
-/// caller has to act on.
-#[derive(Debug, PartialEq, Eq)]
-enum AnnounceOutcome {
-    Accepted,
-    /// Any non-2xx. Carries the peer's `x-gitlawb-error` code when it sent
-    /// one, which is what makes a spent-signature-ledger rejection (429
-    /// `signature_ledger_full`, 409 `signature_replayed`) diagnosable rather
-    /// than just "some 4xx".
-    Rejected {
-        status: u16,
-        error_code: Option<String>,
-    },
-}
-
-/// Classify a bootstrap announce response.
-///
-/// Split out from `gossip_task` so the rejection path is unit-testable: the
-/// whole defect was that a non-2xx fell off the end of an `if` with no `else`,
-/// leaving a node whose announces are being refused looking exactly like one
-/// succeeding.
-fn classify_announce_response(
-    status: reqwest::StatusCode,
-    headers: &reqwest::header::HeaderMap,
-) -> AnnounceOutcome {
-    if status.is_success() {
-        return AnnounceOutcome::Accepted;
-    }
-    AnnounceOutcome::Rejected {
-        status: status.as_u16(),
-        error_code: api::peers::peer_error_code(headers).map(str::to_string),
-    }
-}
-
 /// Announce to bootstrap peers on startup, then periodically ping all known peers.
 async fn gossip_task(
     state: AppState,
@@ -1238,8 +1031,8 @@ async fn gossip_task(
         )
         .await
         {
-            Ok(Ok(resp)) => match classify_announce_response(resp.status(), resp.headers()) {
-                AnnounceOutcome::Accepted => {
+            Ok(Ok(resp)) => {
+                if resp.status().is_success() {
                     if let Ok(json) = resp.json::<serde_json::Value>().await {
                         // Add them back to our peer list
                         if let (Some(their_did), Some(their_url)) = (
@@ -1278,19 +1071,7 @@ async fn gossip_task(
                         }
                     }
                 }
-                // Without this arm a refused announce produced no log line at
-                // all, so a node whose peers reject it looked identical to one
-                // succeeding. Same shape as the two sibling signed calls
-                // (sync.rs replica registration, repos.rs peer sync notify).
-                AnnounceOutcome::Rejected { status, error_code } => {
-                    warn!(
-                        url = %announce_url,
-                        status = status,
-                        error_code = error_code.as_deref().unwrap_or("none"),
-                        "bootstrap peer announce rejected"
-                    );
-                }
-            },
+            }
             Ok(Err(e)) => {
                 tracing::warn!(url = %announce_url, err = %e, "failed to announce to bootstrap peer")
             }
@@ -1343,20 +1124,20 @@ mod rate_limiter_sweep_tests {
         state.rate_limiter = RateLimiter::new(10, window);
         state.create_ip_rate_limiter = RateLimiter::new(10, window);
         state.push_rate_limiter = RateLimiter::new(10, window);
+        state.close_issue_rate_limiter = RateLimiter::new(10, window);
         state.sync_trigger_rate_limiter = RateLimiter::new(10, window);
         state.peer_write_rate_limiter = RateLimiter::new(10, window);
         state.ipfs_rate_limiter = RateLimiter::new(10, window);
-        state.ipfs_work_rate_limiter = RateLimiter::new(10, window);
 
         let limiters = |s: &crate::state::AppState| {
             [
                 s.rate_limiter.clone(),
                 s.create_ip_rate_limiter.clone(),
                 s.push_rate_limiter.clone(),
+                s.close_issue_rate_limiter.clone(),
                 s.sync_trigger_rate_limiter.clone(),
                 s.peer_write_rate_limiter.clone(),
                 s.ipfs_rate_limiter.clone(),
-                s.ipfs_work_rate_limiter.clone(),
             ]
         };
         for l in limiters(&state) {
@@ -1373,6 +1154,26 @@ mod rate_limiter_sweep_tests {
     }
 }
 
+/// it.
+fn spawn_legacy_cid_sweep(state: &AppState, config: &Config) -> tokio::task::JoinHandle<()> {
+    let db = state.db.clone();
+    let repos_dir = config.repos_dir.clone();
+    let git_bin = state.git_bin.clone();
+    let git_timeout = std::time::Duration::from_secs(config.git_service_timeout_secs);
+    let batch = config.pin_repair_sweep_batch;
+    let delay = std::time::Duration::from_secs(config.pin_repair_sweep_delay_secs);
+    let mut shutdown_rx = state.subscribe_shutdown();
+    tokio::spawn(async move {
+        tokio::select! {
+            _ = ipfs_pin::run_sweep_rearmed(
+                &repos_dir, &git_bin, git_timeout, batch, delay,
+                ipfs_pin::SWEEP_REARM_DELAY, &db,
+            ) => {}
+            _ = shutdown_rx.changed() => {}
+        }
+    })
+}
+
 async fn gossip_ping_round(
     db: &Db,
     client: &reqwest::Client,
@@ -1549,140 +1350,6 @@ fn load_or_create_keypair(config: &Config) -> Result<Keypair> {
     }
 }
 
-#[cfg(test)]
-mod legacy_cid_sweep_wiring_tests {
-    use super::spawn_legacy_cid_sweep;
-    use sqlx::PgPool;
-    use std::time::Duration;
-
-    /// Seed `count` `pinned_cids` rows whose keys are already canonical raw CIDv1, in a
-    /// known `sha256_hex` order. The sweep's own cost gate skips a raw-CIDv1 row without
-    /// reading bytes or resolving a repo, so each row is SCANNED (it advances the cursor)
-    /// and nothing else. That is what makes the cursor a clean readout of how far the
-    /// walk got, with no dependency on repos on disk.
-    async fn seed_scannable_rows(pool: &PgPool, count: usize) -> Vec<String> {
-        let mut shas = Vec::new();
-        for i in 1..=count {
-            let sha = format!("wire{i:02}");
-            let cid = gitlawb_core::cid::Cid::from_git_object_bytes(sha.as_bytes()).to_string();
-            assert!(
-                gitlawb_core::cid::is_raw_cidv1(&cid),
-                "the seeded key must hit the sweep's raw-CIDv1 skip, not a repair attempt"
-            );
-            sqlx::query("INSERT INTO pinned_cids (sha256_hex, cid, pinned_at) VALUES ($1, $2, $3)")
-                .bind(&sha)
-                .bind(&cid)
-                .bind("2020-01-01T00:00:00Z")
-                .execute(pool)
-                .await
-                .unwrap();
-            shas.push(sha);
-        }
-        shas
-    }
-
-    /// Poll the persisted sweep cursor until it reaches `want`, or give up.
-    async fn cursor_reaches(db: &crate::db::Db, want: &str, within: Duration) -> String {
-        let deadline = std::time::Instant::now() + within;
-        loop {
-            let c = db.pin_repair_cursor().await.unwrap();
-            if c == want || std::time::Instant::now() >= deadline {
-                return c;
-            }
-            tokio::time::sleep(Duration::from_millis(20)).await;
-        }
-    }
-
-    /// #173 U4, the BOOT-PATH half. The sweep's own logic (batching, cursor resumption,
-    /// terminal vs retryable skips) is covered in `test_support`; what this covers is the
-    /// wiring `main` performs, which nothing else executes: the task is spawned at all,
-    /// it takes its batch and delay from the two `pin_repair_sweep_*` knobs rather than
-    /// some other config field, the caller is not blocked on the walk, and the shutdown
-    /// watcher ends the run mid-walk.
-    ///
-    /// Six scannable rows, batch 2, delay 30s. One pass must land the cursor on exactly
-    /// the second row and the task must then still be alive in its inter-batch sleep,
-    /// which pins both knobs at once: a different batch stops at a different row, and a
-    /// delay that did not come from the knob either finishes the table or leaves the task
-    /// gone. Shutdown must then end it while four rows are still unwalked.
-    #[sqlx::test]
-    async fn the_boot_path_spawns_the_sweep_detached_with_its_configured_knobs(pool: PgPool) {
-        let state = crate::test_support::test_state(pool.clone()).await;
-        let shas = seed_scannable_rows(&pool, 6).await;
-        let repos_dir = tempfile::TempDir::new().unwrap();
-
-        let mut config = (*state.config).clone();
-        config.repos_dir = repos_dir.path().to_path_buf();
-        config.pin_repair_sweep_batch = 2;
-        // Far longer than this test runs, so a task still alive after the first pass can
-        // only be one that is honoring the configured inter-batch delay.
-        config.pin_repair_sweep_delay_secs = 30;
-
-        let started = std::time::Instant::now();
-        let handle = spawn_legacy_cid_sweep(&state, &config);
-        let spawn_cost = started.elapsed();
-
-        let cursor = cursor_reaches(&state.db, &shas[1], Duration::from_secs(10)).await;
-        assert_eq!(
-            cursor, shas[1],
-            "the spawned sweep must run and stop its first pass at the CONFIGURED batch \
-             bound (2), leaving the cursor on the second row"
-        );
-        assert!(
-            spawn_cost < Duration::from_secs(1),
-            "the sweep must be detached, not awaited on the boot path; the spawn took \
-             {spawn_cost:?}"
-        );
-        assert!(
-            !handle.is_finished(),
-            "with a 30s inter-batch delay the task must still be sleeping between passes, \
-             not finished: a finished task means the delay was not the configured one"
-        );
-
-        state.shutdown();
-        tokio::time::timeout(Duration::from_secs(10), handle)
-            .await
-            .expect("the shutdown watcher must end the sweep, and not after its 30s delay")
-            .expect("the sweep task must not panic");
-
-        assert_eq!(
-            state.db.pin_repair_cursor().await.unwrap(),
-            shas[1],
-            "shutdown must have ended the run MID-walk, with the remaining rows unwalked"
-        );
-    }
-}
-
-#[cfg(test)]
-mod lock_pool_sizing_tests {
-    use super::{lock_pool_size, LOCK_POOL_MAX_CONNECTIONS, LOCK_POOL_PUSH_HEADROOM};
-
-    /// The default push cap gets its cap plus headroom, so no push ever queues for a
-    /// lock-pool connection where it did not before.
-    #[test]
-    fn default_push_cap_gets_headroom_over_the_cap() {
-        assert_eq!(lock_pool_size(32), 32 + u32::from(LOCK_POOL_PUSH_HEADROOM));
-        assert_eq!(lock_pool_size(1), 1 + u32::from(LOCK_POOL_PUSH_HEADROOM));
-    }
-
-    /// #173 F4: `max_concurrent_git_pushes` is validated all the way to 1_048_576, so an
-    /// operator raising it used to raise the node's Postgres connection ceiling with it,
-    /// silently and without bound. The lock pool is CLAMPED instead.
-    #[test]
-    fn an_oversized_push_cap_is_clamped_not_propagated() {
-        assert_eq!(lock_pool_size(1_048_576), LOCK_POOL_MAX_CONNECTIONS);
-        assert_eq!(lock_pool_size(usize::MAX), LOCK_POOL_MAX_CONNECTIONS);
-        // The largest cap that still fits under the clamp keeps its full headroom.
-        let widest = (LOCK_POOL_MAX_CONNECTIONS - u32::from(LOCK_POOL_PUSH_HEADROOM)) as usize;
-        assert_eq!(lock_pool_size(widest), LOCK_POOL_MAX_CONNECTIONS);
-        assert_eq!(
-            lock_pool_size(widest - 1),
-            LOCK_POOL_MAX_CONNECTIONS - 1,
-            "values below the clamp must not be rounded up to it"
-        );
-    }
-}
-
 #[cfg(test)]
 mod gossip_ssrf_tests {
     use super::{
@@ -1985,199 +1652,3 @@ mod gossip_ssrf_tests {
         assert!(!ok, "a connection error must count as an unready peer");
     }
 }
-
-#[cfg(test)]
-mod gossip_announce_tests {
-    use super::{classify_announce_response, AnnounceOutcome};
-    use reqwest::header::HeaderMap;
-    use reqwest::StatusCode;
-
-    fn headers(code: Option<&str>) -> HeaderMap {
-        let mut h = HeaderMap::new();
-        if let Some(c) = code {
-            h.insert("x-gitlawb-error", c.parse().unwrap());
-        }
-        h
-    }
-
-    #[test]
-    fn a_2xx_announce_is_accepted() {
-        assert_eq!(
-            classify_announce_response(StatusCode::OK, &headers(None)),
-            AnnounceOutcome::Accepted
-        );
-    }
-
-    // The defect: every non-2xx fell off the end of an `if` with no `else`, so
-    // a node whose announces were being refused logged nothing at all. Each of
-    // these must come back Rejected, carrying the status and, when the peer
-    // sent one, the x-gitlawb-error code that names the reason.
-    #[test]
-    fn a_ledger_rejection_is_reported_with_its_error_code() {
-        assert_eq!(
-            classify_announce_response(
-                StatusCode::TOO_MANY_REQUESTS,
-                &headers(Some("signature_ledger_full"))
-            ),
-            AnnounceOutcome::Rejected {
-                status: 429,
-                error_code: Some("signature_ledger_full".to_string()),
-            }
-        );
-        assert_eq!(
-            classify_announce_response(StatusCode::CONFLICT, &headers(Some("signature_replayed"))),
-            AnnounceOutcome::Rejected {
-                status: 409,
-                error_code: Some("signature_replayed".to_string()),
-            }
-        );
-        assert_eq!(
-            classify_announce_response(
-                StatusCode::SERVICE_UNAVAILABLE,
-                &headers(Some("signature_ledger_unavailable"))
-            ),
-            AnnounceOutcome::Rejected {
-                status: 503,
-                error_code: Some("signature_ledger_unavailable".to_string()),
-            }
-        );
-    }
-
-    // A proxy that strips unknown X- headers, or a peer that never sets one,
-    // must still produce a rejection with its status rather than silence.
-    #[test]
-    fn a_rejection_without_an_error_code_is_still_reported() {
-        assert_eq!(
-            classify_announce_response(StatusCode::FORBIDDEN, &headers(None)),
-            AnnounceOutcome::Rejected {
-                status: 403,
-                error_code: None,
-            }
-        );
-        assert_eq!(
-            classify_announce_response(StatusCode::INTERNAL_SERVER_ERROR, &headers(None)),
-            AnnounceOutcome::Rejected {
-                status: 500,
-                error_code: None,
-            }
-        );
-    }
-}
-
-#[cfg(test)]
-mod db_startup_signal_tests {
-    use super::{
-        degraded_body, is_likely_permanent_db_error, record_db_startup_failure, DbStartupStatus,
-    };
-    use crate::db::MigrationVersionCollision;
-
-    fn collision_error() -> anyhow::Error {
-        anyhow::Error::new(MigrationVersionCollision {
-            version: 16,
-            recorded_name: "pinned_cids_repo_provenance".to_string(),
-            build_name: "consumed_signatures",
-        })
-        // Wrapped the way the real call path wraps it, so the classifier has
-        // to look through the context chain.
-        .context("connecting to postgres")
-    }
-
-    // The wire between the classifier and the readiness payload. Classifying a
-    // collision correctly is useless if nothing latches it: the backoff and the
-    // log level would be right while /ready still reported "initializing"
-    // forever, which is the defect. This drives the same function
-    // `connect_db_with_retry` calls on every failed attempt, so the latch is
-    // what is under test, not just its two endpoints.
-    #[test]
-    fn a_collision_latches_the_readiness_signal() {
-        let status = DbStartupStatus::default();
-        assert!(!status.schema_conflict(), "precondition: not yet latched");
-
-        let fault = record_db_startup_failure(&collision_error(), &status);
-
-        assert!(fault.schema_conflict, "the fault must name the conflict");
-        assert!(fault.permanent, "and classify it as permanent");
-        assert!(
-            status.schema_conflict(),
-            "the collision must latch the startup status, or the readiness \
-             payload keeps claiming the database is initializing"
-        );
-        assert_eq!(
-            degraded_body(&status)["database"],
-            "schema_conflict",
-            "the latch must reach the payload the degraded router serves"
-        );
-    }
-
-    #[test]
-    fn an_ordinary_outage_does_not_latch_a_schema_conflict() {
-        let status = DbStartupStatus::default();
-
-        let fault = record_db_startup_failure(
-            &anyhow::Error::new(sqlx::Error::PoolTimedOut).context("connecting to postgres"),
-            &status,
-        );
-
-        assert!(!fault.schema_conflict);
-        assert!(!fault.permanent);
-        assert!(
-            !status.schema_conflict(),
-            "a transient outage must not latch the conflict signal"
-        );
-        assert_eq!(degraded_body(&status)["database"], "initializing");
-    }
-
-    // Defect 1: a schema collision never heals on its own, so it must classify
-    // as permanent and take the error-level, operator-action branch, not the
-    // "database unavailable during startup; retrying" warn branch.
-    #[test]
-    fn a_migration_collision_is_permanent() {
-        assert!(
-            is_likely_permanent_db_error(&collision_error()),
-            "a migration version collision must classify as permanent"
-        );
-    }
-
-    #[test]
-    fn an_ordinary_connection_error_is_still_transient() {
-        let err = anyhow::Error::new(sqlx::Error::PoolTimedOut).context("connecting to postgres");
-        assert!(
-            !is_likely_permanent_db_error(&err),
-            "a pool timeout is a transient outage, not a permanent misconfiguration"
-        );
-        let io = anyhow::Error::new(sqlx::Error::Io(std::io::Error::new(
-            std::io::ErrorKind::ConnectionRefused,
-            "connection refused",
-        )))
-        .context("connecting to postgres");
-        assert!(
-            !is_likely_permanent_db_error(&io),
-            "a refused TCP connection is a transient outage"
-        );
-    }
-
-    // Defect 1, readiness half: the payload must not claim the database is
-    // merely initializing when the real cause is a schema conflict.
-    #[test]
-    fn the_degraded_body_distinguishes_a_schema_conflict_from_an_ordinary_wait() {
-        let waiting = DbStartupStatus::default();
-        let body = degraded_body(&waiting);
-        assert_eq!(body["database"], "initializing");
-        assert_eq!(body["error"], crate::error::DB_UNAVAILABLE_CODE);
-
-        let conflicted = DbStartupStatus::default();
-        conflicted.mark_schema_conflict();
-        let body = degraded_body(&conflicted);
-        assert_eq!(
-            body["database"], "schema_conflict",
-            "a schema conflict must not read as 'initializing'"
-        );
-        assert_eq!(body["error"], super::DB_SCHEMA_CONFLICT_CODE);
-        assert_ne!(
-            body["error"],
-            crate::error::DB_UNAVAILABLE_CODE,
-            "the conflict must carry its own code"
-        );
-        assert_eq!(body["status"], "degraded");
-    }
-}

crates/gitlawb-node/src/metrics.rs

@@ -7,8 +7,6 @@
 //!   * is fetch traffic flowing? — `gitlawb_fetches_total`
 //!   * are signature checks passing or failing? —
 //!     `gitlawb_auth_successes_total` / `gitlawb_auth_failures_total`
-//!   * is the spent-signature replay ledger admitting or refusing writes? —
-//!     `gitlawb_signature_ledger_total{outcome}`
 //!   * is the sync worker making progress? —
 //!     `gitlawb_sync_queue_processed_total{status}`
 //!   * are webhooks reaching their endpoints? —
@@ -49,7 +47,6 @@ static PUSHES: OnceLock<IntCounterVec> = OnceLock::new();
 static FETCHES: OnceLock<IntCounterVec> = OnceLock::new();
 static AUTH_SUCCESSES: OnceLock<IntCounterVec> = OnceLock::new();
 static AUTH_FAILURES: OnceLock<IntCounterVec> = OnceLock::new();
-static SIGNATURE_LEDGER: OnceLock<IntCounterVec> = OnceLock::new();
 static SYNC_PROCESSED: OnceLock<IntCounterVec> = OnceLock::new();
 static WEBHOOK_DELIVERIES: OnceLock<IntCounterVec> = OnceLock::new();
 static PACK_SIZE: OnceLock<Histogram> = OnceLock::new();
@@ -60,17 +57,6 @@ static PEERS_CONNECTED: OnceLock<IntGauge> = OnceLock::new();
 /// more than once is a silent no-op. MUST be called from `main()` after
 /// the node DID is known.
 pub fn init(version: &str, node_did: &str) {
-    // `Once` rather than a bare `REGISTRY.get().is_some()` check: the registry
-    // is published last, so that check leaves a window in which a second
-    // concurrent caller walks into the `set(..).expect(..)` calls below and
-    // panics on an already-set `OnceLock`. `main` calls this once, but tests
-    // call it from several threads. `call_once` makes the later callers block
-    // and then return, which is what the `expect`s already assume.
-    static INIT: std::sync::Once = std::sync::Once::new();
-    INIT.call_once(|| init_once(version, node_did));
-}
-
-fn init_once(version: &str, node_did: &str) {
     // Guard with Once so two concurrent callers cannot both pass an unsynchronized
     // `REGISTRY.get()` check and then race on `OnceLock::set(...).expect(...)`,
     // panicking the loser (#192 F4). Once runs the body exactly once, so the
@@ -152,21 +138,6 @@ fn init_inner(version: &str, node_did: &str) {
         .set(auth_failures)
         .expect("set AUTH_FAILURES once");
 
-    let signature_ledger = IntCounterVec::new(
-        Opts::new(
-            "gitlawb_signature_ledger_total",
-            "Total requests seen by the spent-signature replay ledger, by outcome",
-        ),
-        &["outcome"],
-    )
-    .expect("gitlawb_signature_ledger_total definition");
-    registry
-        .register(Box::new(signature_ledger.clone()))
-        .expect("register gitlawb_signature_ledger_total");
-    SIGNATURE_LEDGER
-        .set(signature_ledger)
-        .expect("set SIGNATURE_LEDGER once");
-
     let sync_processed = IntCounterVec::new(
         Opts::new(
             "gitlawb_sync_queue_processed_total",
@@ -280,25 +251,6 @@ pub fn record_auth_failure(route: &str, reason: &str) {
     }
 }
 
-/// Record one terminal outcome of the spent-signature replay ledger layer.
-///
-/// Every request that enters the layer lands on exactly one outcome, so the
-/// series sum is the layer's request count and any one outcome is a rate over
-/// it. `outcome` is a fixed set of literals chosen at the call sites in
-/// `auth::consume_signature` — never a DID, keyid, or path, which would be
-/// attacker-controlled and unbounded.
-///
-/// The sum is the LAYER's request count, not the routes' traffic. The per-client
-/// flood brake (`rate_limit`) rejects upstream of this layer, so its 429s appear
-/// in no series here even though they are served on the same routes. An operator
-/// counting refusals has to read `rate_limited` responses separately; this
-/// counter answers "what is the ledger doing", not "how many 429s went out".
-pub fn record_signature_ledger(outcome: &str) {
-    if let Some(c) = SIGNATURE_LEDGER.get() {
-        c.with_label_values(&[outcome]).inc();
-    }
-}
-
 /// Record one sync_queue item outcome. `status` ∈ {done, failed, skipped,
 /// rejected, deferred}. `rejected` is terminal and caused by a guard (a
 /// malformed slug, or a path resolving outside repos_dir); `deferred` leaves
@@ -411,7 +363,6 @@ mod tests {
         record_fetch("test/repo");
         record_auth_success("test/route");
         record_auth_failure("test/route", "test_reason");
-        record_signature_ledger("test_outcome");
         record_sync_processed("done");
         record_webhook_delivery("ok");
         observe_pack_size(1024.0);

crates/gitlawb-node/src/rate_limit.rs

@@ -4,12 +4,9 @@ use std::sync::Arc;
 use std::time::{Duration, Instant};
 
 use axum::extract::{ConnectInfo, Request};
-use axum::http::{HeaderMap, Method, StatusCode};
+use axum::http::{HeaderMap, StatusCode};
 use axum::middleware::Next;
 use axum::response::{IntoResponse, Response};
-use axum::Json;
-use gitlawb_core::node_denial::NodeDenial;
-use serde_json::json;
 use tokio::sync::Mutex;
 
 use crate::auth::AuthenticatedDid;
@@ -279,10 +276,12 @@ pub async fn rate_limit_by_did(request: Request, next: Next) -> Response {
 
     if let (Some(limiter), Some(did)) = (limiter, did) {
         if !limiter.check(&did).await {
-            // Same response as the per-IP brake: a client cannot act on the
-            // difference between the two buckets, and both need the code that
-            // tells them apart from the ledger's 429.
-            return too_many_requests();
+            return (
+                StatusCode::TOO_MANY_REQUESTS,
+                [("retry-after", "60")],
+                "rate limit exceeded — try again later",
+            )
+                .into_response();
         }
     }
 
@@ -361,18 +360,6 @@ pub fn client_key(
 pub struct IpRateLimiter {
     pub limiter: RateLimiter,
     pub trust: TrustedProxy,
-    /// Exempt GET/HEAD from this bucket. The signed-write brake classifies
-    /// whole routers, but `write_routes` chains PUT/DELETE/GET on one path
-    /// (`/api/v1/repos/{owner}/{repo}/visibility`, `server.rs`), and `gl
-    /// visibility list` deliberately signs that owner-only GET — so without
-    /// this the read spends the write quota and a normal listing 429s after a
-    /// few writes from the same IP (one NAT egress makes the coupling vivid).
-    /// The method check is the same general form `consume_signature` uses for
-    /// its ledger skip: a read has no write side effect for the brake to
-    /// protect. Leave this `false` on any limiter whose job IS braking reads —
-    /// the `/ipfs/{cid}` brake exists precisely to cap GET-driven history
-    /// walks, so a blanket skip inside `rate_limit_by_ip` would delete it.
-    pub skip_reads: bool,
 }
 
 /// Infallible extractor for the socket peer address from `ConnectInfo`. Yields
@@ -397,24 +384,14 @@ impl<S: Send + Sync> axum::extract::FromRequestParts<S> for PeerAddr {
     }
 }
 
-/// The shared 429 response for the flood brakes. Route-agnostic: this serves
-/// the push path AND the peer-sync routes, so the message stays generic (the
-/// offending path is recorded in the warn log at the call site).
-///
-/// Carries `X-Gitlawb-Error: rate_limited` and the same JSON shape the ledger's
-/// `ledger_rejection` uses, because the brake shares 429 with the ledger's
-/// `signature_ledger_full` on every route group the brake covers. Without a
-/// code the only discriminator between two unrelated conditions is the
-/// *absence* of a header, which is exactly what a proxy stripping unknown `X-`
-/// headers produces. A client cannot act on a difference it cannot see: `gl`
-/// would print "invalid JSON response" for a rate limit, and a peer's sync
-/// sender would retry a brake it can never clear inside its budget.
+/// The shared 429 response for the per-IP flood brakes. Route-agnostic: this
+/// middleware now serves the push path AND the peer-sync routes, so the message
+/// stays generic (the offending path is recorded in the warn log below).
 pub fn too_many_requests() -> Response {
-    let code = NodeDenial::RateLimited.as_str();
     (
         StatusCode::TOO_MANY_REQUESTS,
-        [("retry-after", "60"), ("X-Gitlawb-Error", code)],
-        Json(json!({ "error": code, "message": "rate limit exceeded — try again later" })),
+        [("retry-after", "60")],
+        "rate limit exceeded — try again later",
     )
         .into_response()
 }
@@ -430,16 +407,10 @@ pub async fn rate_limit_by_ip(request: Request, next: Next) -> Response {
         .map(|c| c.0);
 
     if let Some(limiter) = limiter {
-        // Opt-in read exemption (see `IpRateLimiter::skip_reads`): only a
-        // limiter that declares itself write-only skips GET/HEAD, so the /ipfs
-        // read brake and every other read-facing bucket keep braking reads.
-        let is_read = matches!(*request.method(), Method::GET | Method::HEAD);
-        if !(limiter.skip_reads && is_read) {
-            if let Some(key) = client_key(request.headers(), peer, limiter.trust) {
-                if !limiter.limiter.check(&key).await {
-                    tracing::warn!(key = %key, path = %request.uri().path(), "per-IP rate limit exceeded");
-                    return too_many_requests();
-                }
+        if let Some(key) = client_key(request.headers(), peer, limiter.trust) {
+            if !limiter.limiter.check(&key).await {
+                tracing::warn!(key = %key, path = %request.uri().path(), "per-IP rate limit exceeded");
+                return too_many_requests();
             }
         }
     }
@@ -451,41 +422,6 @@ pub async fn rate_limit_by_ip(request: Request, next: Next) -> Response {
 mod tests {
     use super::*;
 
-    /// The brake shares 429 with the spent-signature ledger's
-    /// `signature_ledger_full`, so it has to be told apart by something other
-    /// than the status. It carries the same shape the ledger uses: an
-    /// `X-Gitlawb-Error` header and a JSON body whose `error` matches it.
-    #[tokio::test]
-    async fn brake_429_carries_a_machine_readable_code_and_json_body() {
-        let resp = too_many_requests();
-        assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS);
-        assert_eq!(
-            resp.headers()
-                .get("retry-after")
-                .and_then(|v| v.to_str().ok()),
-            Some("60"),
-        );
-        assert_eq!(
-            resp.headers()
-                .get("x-gitlawb-error")
-                .and_then(|v| v.to_str().ok()),
-            Some("rate_limited"),
-            "a brake 429 must be distinguishable from a ledger 429 by its code",
-        );
-        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
-            .await
-            .expect("brake body");
-        let body: serde_json::Value =
-            serde_json::from_slice(&bytes).expect("brake body must be JSON, not text/plain");
-        assert_eq!(body["error"], "rate_limited");
-        assert!(
-            body["message"]
-                .as_str()
-                .is_some_and(|m| m.contains("rate limit exceeded")),
-            "body: {body}",
-        );
-    }
-
     #[test]
     fn per_caller_concurrency_caps_one_caller_and_frees_on_drop() {
         let lim = PerCallerConcurrency::new(2, 100);
@@ -775,7 +711,6 @@ mod tests {
         let router = ip_limited_router(IpRateLimiter {
             limiter: RateLimiter::new(2, Duration::from_secs(60)),
             trust: TrustedProxy::None,
-            skip_reads: false,
         });
         let peer: SocketAddr = "203.0.113.7:5000".parse().unwrap();
         // Two requests inside the budget, the third over it (shared Arc state).
@@ -792,7 +727,6 @@ mod tests {
         let router = ip_limited_router(IpRateLimiter {
             limiter: RateLimiter::new(1, Duration::from_secs(60)),
             trust: TrustedProxy::None,
-            skip_reads: false,
         });
         let a: SocketAddr = "203.0.113.1:1".parse().unwrap();
         let b: SocketAddr = "203.0.113.2:1".parse().unwrap();
@@ -801,70 +735,6 @@ mod tests {
         assert_eq!(post_from(&router, a).await, StatusCode::TOO_MANY_REQUESTS);
     }
 
-    /// `skip_reads` mechanics, isolated from the production router: on a
-    /// limiter that declares itself write-only, GET and HEAD never touch the
-    /// bucket — they pass an already-exhausted one AND never debit it — while
-    /// a mutation from the same peer stays braked. On a limiter that does not
-    /// opt in (the /ipfs read brake), GET is braked exactly as before. The
-    /// production wiring (the signed-write brake on `write_routes`, whose one
-    /// GET is the signed visibility listing) is proven end-to-end by
-    /// `signed_write_brake_rejects_writes_but_serves_visibility_get` in
-    /// `test_support`; this pins the middleware seam it rides on.
-    #[tokio::test]
-    async fn skip_reads_exempts_get_and_head_but_still_brakes_writes() {
-        use tower::ServiceExt;
-        let router = |skip_reads: bool| {
-            axum::Router::new()
-                .route(
-                    "/thing",
-                    axum::routing::get(|| async { StatusCode::OK })
-                        .head(|| async { StatusCode::OK })
-                        .put(|| async { StatusCode::OK }),
-                )
-                .layer(axum::middleware::from_fn(rate_limit_by_ip))
-                .layer(axum::Extension(IpRateLimiter {
-                    limiter: RateLimiter::new(1, Duration::from_secs(60)),
-                    trust: TrustedProxy::None,
-                    skip_reads,
-                }))
-        };
-        let peer: SocketAddr = "203.0.113.9:5000".parse().unwrap();
-        let send = |router: &axum::Router, method: axum::http::Method| {
-            let mut req = axum::http::Request::builder()
-                .method(method)
-                .uri("/thing")
-                .body(axum::body::Body::empty())
-                .unwrap();
-            req.extensions_mut().insert(ConnectInfo(peer));
-            let router = router.clone();
-            async move { router.oneshot(req).await.unwrap().status() }
-        };
-
-        // Opted-in limiter: the PUT spends the single slot...
-        let r = router(true);
-        assert_eq!(send(&r, axum::http::Method::PUT).await, StatusCode::OK);
-        // ...reads from the same peer still pass the exhausted bucket...
-        assert_eq!(send(&r, axum::http::Method::GET).await, StatusCode::OK);
-        assert_eq!(send(&r, axum::http::Method::HEAD).await, StatusCode::OK);
-        // ...and did not debit it behind the scenes some other way — the next
-        // mutation is still the one that gets braked.
-        assert_eq!(
-            send(&r, axum::http::Method::PUT).await,
-            StatusCode::TOO_MANY_REQUESTS,
-            "the write brake must keep braking writes while exempting reads"
-        );
-
-        // Non-opted-in limiter: GET is braked like any other method, so the
-        // exemption cannot silently spread to read-braking buckets like /ipfs.
-        let r = router(false);
-        assert_eq!(send(&r, axum::http::Method::GET).await, StatusCode::OK);
-        assert_eq!(
-            send(&r, axum::http::Method::GET).await,
-            StatusCode::TOO_MANY_REQUESTS,
-            "a limiter that did not opt in must keep braking reads"
-        );
-    }
-
     // ── creation-route layering: IP brake runs BEFORE auth ──────────────
 
     /// A minimal stand-in for the creation route group: an inner "auth" layer
@@ -910,7 +780,6 @@ mod tests {
         let router = ip_limited_over_auth_router(IpRateLimiter {
             limiter: RateLimiter::new(1, Duration::from_secs(60)),
             trust: TrustedProxy::None,
-            skip_reads: false,
         });
         let peer: SocketAddr = "203.0.113.42:7000".parse().unwrap();
 

crates/gitlawb-node/src/server.rs

@@ -42,33 +42,12 @@ async fn graphql_playground() -> impl IntoResponse {
     ))
 }
 
-/// Applies the standard auth middleware stack to a router. Axum runs the LAST
-/// `.layer` first, so the three below run in this order for every matched
-/// request: `require_signature` (verifies the RFC 9421 signature, sets
-/// `AuthenticatedDid` and `SignatureIdentity`), then `require_ucan_chain`
-/// (reads the DID), then `consume_signature` (spends the signature).
-///
-/// The ledger runs last of the three on purpose. If it ran before
-/// `require_ucan_chain`, a request with a good signature but a rejected UCAN
-/// would burn its key and the client could not retry those exact bytes.
-/// Consuming before the handler, rather than inside it, is what closes the
-/// concurrent-replay race: two copies of the same signature race the ledger, not
-/// the handler.
-///
-/// It is not the last check overall, so some rejections do still cost a ledger
-/// row. Any layer added inside the router passed in runs AFTER the ledger
-/// (`creation_routes` puts `rate_limit_by_did` there), and every check the
-/// handler itself makes (the iCaptcha 403, ownership, existence) runs later
-/// still. A request refused by the per-DID throttle, by iCaptcha, or by the
-/// handler has already spent its signature and must re-sign to retry. Only a
-/// per-IP brake layered outside this stack rejects a request before the ledger
-/// is charged.
+/// Applies the standard auth middleware pair to a router: HTTP Signature verification
+/// followed by UCAN chain validation. The two layers run in this order for every
+/// matched request: `require_signature` first (sets `AuthenticatedDid`), then
+/// `require_ucan_chain` (reads it).
 fn add_auth_layers(router: Router<AppState>, state: AppState) -> Router<AppState> {
     router
-        .layer(middleware::from_fn_with_state(
-            state.clone(),
-            auth::consume_signature,
-        ))
         .layer(middleware::from_fn_with_state(
             state,
             auth::require_ucan_chain,
@@ -87,28 +66,6 @@ pub fn build_router(state: AppState) -> Router {
         .layer(middleware::from_fn(auth::optional_signature))
         .route_service("/graphql/ws", GraphQLSubscription::new(schema));
 
-    // ── Per-IP flood brake for the signed write routes ────────────────────
-    // Every signed attempt on a route wrapped in `add_auth_layers` commits a
-    // `consumed_signatures` row before the handler runs any authorization or
-    // existence check, and `require_signature` resolves the key from the did:key
-    // itself, so any freshly minted keypair can force that durable write on a
-    // repo it does not own or that does not exist. The five groups below carried
-    // no brake at all; this one caps them on the resolved client IP. Its own
-    // bucket, so a flood here cannot drain the creation, push, or peer-sync
-    // quotas. Each group layers it OUTSIDE `add_auth_layers` (axum runs the last
-    // `.layer` first) so an over-limit request is rejected before signature
-    // verification burns CPU and before the ledger is charged.
-    // `skip_reads`: `write_routes` chains PUT/DELETE/GET on the visibility
-    // path, and `gl visibility list` signs that owner-only GET. A write-only
-    // brake must not let reads spend the write quota (or a listing starts
-    // 429ing behind a shared NAT after a handful of writes), mirroring the
-    // method skip `consume_signature` applies to the ledger for the same GET.
-    let signed_write_ip_limiter = rate_limit::IpRateLimiter {
-        limiter: state.signed_write_rate_limiter.clone(),
-        trust: state.push_limiter_trust,
-        skip_reads: true,
-    };
-
     // ── Task routes (write — require HTTP Signature) ───────────────────────
     let task_write_routes = add_auth_layers(
         Router::new()
@@ -117,9 +74,7 @@ pub fn build_router(state: AppState) -> Router {
             .route("/api/v1/tasks/{id}/complete", post(tasks::complete_task))
             .route("/api/v1/tasks/{id}/fail", post(tasks::fail_task)),
         state.clone(),
-    )
-    .layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
-    .layer(axum::Extension(signed_write_ip_limiter.clone()));
+    );
 
     // ── Task routes (read — open) ──────────────────────────────────────────
     let task_read_routes = Router::new()
@@ -138,7 +93,6 @@ pub fn build_router(state: AppState) -> Router {
     let create_ip_limiter = rate_limit::IpRateLimiter {
         limiter: state.create_ip_rate_limiter.clone(),
         trust: state.push_limiter_trust,
-        skip_reads: false,
     };
     let creation_routes = add_auth_layers(
         Router::new()
@@ -157,7 +111,7 @@ pub fn build_router(state: AppState) -> Router {
     .layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
     .layer(axum::Extension(create_ip_limiter));
 
-    // ── Write routes — require HTTP Signature, plus the signed-write IP brake ─
+    // ── Write routes — require HTTP Signature (no rate limit) ─────────────
     let write_routes = add_auth_layers(
         Router::new()
             .route(
@@ -227,9 +181,7 @@ pub fn build_router(state: AppState) -> Router {
                 axum::routing::delete(agents::deregister_agent),
             ),
         state.clone(),
-    )
-    .layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
-    .layer(axum::Extension(signed_write_ip_limiter.clone()));
+    );
 
     // Body limit is raised to GITLAWB_MAX_PACK_BYTES (default 2 GB) for git
     // routes only — all other API routes keep axum's default 2 MB cap.
@@ -243,7 +195,6 @@ pub fn build_router(state: AppState) -> Router {
     let push_limiter = rate_limit::IpRateLimiter {
         limiter: state.push_rate_limiter.clone(),
         trust: state.push_limiter_trust,
-        skip_reads: false,
     };
     let git_write_routes = add_auth_layers(
         Router::new()
@@ -271,7 +222,6 @@ pub fn build_router(state: AppState) -> Router {
     let ipfs_limiter = rate_limit::IpRateLimiter {
         limiter: state.ipfs_rate_limiter.clone(),
         trust: state.push_limiter_trust,
-        skip_reads: false,
     };
     let ipfs_routes = Router::new()
         .route("/ipfs/{cid}", get(ipfs::get_by_cid))
@@ -308,9 +258,7 @@ pub fn build_router(state: AppState) -> Router {
                 post(bounties::dispute_bounty),
             ),
         state.clone(),
-    )
-    .layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
-    .layer(axum::Extension(signed_write_ip_limiter.clone()));
+    );
 
     // ── Bounty routes (read — open) ──────────────────────────────────────
     let bounty_read_routes = Router::new()
@@ -331,11 +279,9 @@ pub fn build_router(state: AppState) -> Router {
     let profile_write_routes = add_auth_layers(
         Router::new().route("/api/v1/profile", axum::routing::put(profiles::set_profile)),
         state.clone(),
-    )
-    .layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
-    .layer(axum::Extension(signed_write_ip_limiter.clone()));
+    );
 
-    // ── Issue routes (write — require HTTP Signature) ────────────────────
+    // ── Issue routes (write — require HTTP Signature, no rate limit) ─────
     let issue_write_routes = add_auth_layers(
         Router::new()
             .route(
@@ -347,9 +293,7 @@ pub fn build_router(state: AppState) -> Router {
                 post(issues::create_issue_comment),
             ),
         state.clone(),
-    )
-    .layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
-    .layer(axum::Extension(signed_write_ip_limiter));
+    );
 
     // ── Peer discovery routes ─────────────────────────────────────────────
     // Peer writes accept signatures when present and can require them after a
@@ -372,7 +316,6 @@ pub fn build_router(state: AppState) -> Router {
     .layer(axum::Extension(rate_limit::IpRateLimiter {
         limiter: state.sync_trigger_rate_limiter.clone(),
         trust: state.push_limiter_trust,
-        skip_reads: false,
     }));
 
     // announce + notify keep their rolling-upgrade signature behavior (unsigned
@@ -393,7 +336,6 @@ pub fn build_router(state: AppState) -> Router {
         .layer(axum::Extension(rate_limit::IpRateLimiter {
             limiter: state.peer_write_rate_limiter.clone(),
             trust: state.push_limiter_trust,
-            skip_reads: false,
         }));
 
     // ── Read routes — open for public repos ───────────────────────────────

crates/gitlawb-node/src/state.rs

@@ -79,6 +79,10 @@ 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
@@ -186,16 +190,6 @@ pub struct AppState {
     /// sink as trigger and accepts unsigned requests from known peers, so it is
     /// braked too; each peer's distinct IP gets its own bucket.
     pub peer_write_rate_limiter: RateLimiter,
-    /// Per-client-IP limiter for the signed write routes (tasks, pull-request
-    /// merge/close/review/comment, webhooks, branch protection, stars, replicas,
-    /// labels, visibility, agent deregistration, bounties, profile, issue
-    /// close/comment). Every signed attempt on those routes commits a
-    /// `consumed_signatures` row before the handler checks ownership or
-    /// existence, and `require_signature` resolves the key from the did:key
-    /// itself, so any freshly minted keypair can force that durable write. Its
-    /// own bucket so a flood here cannot drain the creation, push, or peer-sync
-    /// quotas. Keyed by `push_limiter_trust`.
-    pub signed_write_rate_limiter: RateLimiter,
     /// Process-wide graceful-shutdown signal. Sending `true` causes every
     /// task that holds a `watch::Receiver` to exit at its next await point.
     /// Used by:
@@ -352,6 +346,7 @@ 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/tests/inv22_gates.rs

@@ -225,7 +225,8 @@ 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(mut self")
+        .find("pub async fn release(self")
+        .or_else(|| repo_store.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/gl/src/http.rs

@@ -3,32 +3,18 @@
 //! Writes are signed with RFC 9421 HTTP Signatures. When the node gates a write
 //! behind iCaptcha (HTTP 403 `icaptcha_proof_required`, advertised via the
 //! `x-icaptcha-url` / `x-icaptcha-level` headers), the client transparently
-//! solves the challenge and re-signs the write with the `x-icaptcha-proof`
-//! header attached (see `crates/icaptcha-client`).
+//! solves the challenge and retries the same signed request with the
+//! `x-icaptcha-proof` header — see `crates/icaptcha-client`.
 
 use anyhow::{Context, Result};
 use gitlawb_core::http_sig::sign_request;
 use gitlawb_core::identity::Keypair;
-use gitlawb_core::node_denial::NodeDenial;
 use icaptcha_client::IcaptchaCfg;
 
 /// Max times we'll fetch a fresh proof and retry a 403-iCaptcha response
 /// (absorbs proof expiry / first-seen replay).
 const MAX_ICAPTCHA_RETRIES: usize = 2;
 
-/// Max bytes buffered from a denial body before we give up on parsing it. Node
-/// error bodies are a few hundred bytes of JSON; anything past this is not one,
-/// and reading it unbounded is exactly the allocation a hostile node would aim
-/// for.
-pub(crate) const DENIAL_BODY_CAP: usize = 64 * 1024;
-
-/// Max bytes read from a git smart-HTTP ref advertisement. Real ads are small;
-/// this bounds hostile 200 responses on the MCP `git_refs` path.
-pub(crate) const GIT_REFS_BODY_CAP: usize = 256 * 1024;
-
-/// Max characters of a node-supplied message we will echo to the terminal.
-const NODE_MSG_CHARS: usize = 200;
-
 /// Total request timeout: from the start of connecting through the end of the
 /// response body, so it bounds a slow download and not just a slow handshake.
 const TOTAL_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
@@ -172,12 +158,9 @@ impl NodeClient {
     }
 
     /// Sign + send a write. On a 403 iCaptcha challenge (detected via the
-    /// `x-icaptcha-*` headers) attach the proof and send the write again, up to
-    /// [`MAX_ICAPTCHA_RETRIES`]. Each attempt goes through `send_once`, which
-    /// signs afresh, so the retry is a new signature over the same bytes, not a
-    /// resend of the original one. Emits an actionable hint on a 401 "not an
-    /// agent" (the old-CLI / unregistered failure mode), and converts every
-    /// denial the node names in `x-gitlawb-error` into an error.
+    /// `x-icaptcha-*` headers) solve it and retry the same signed request with
+    /// the proof header, up to [`MAX_ICAPTCHA_RETRIES`]. Emits an actionable
+    /// hint on a 401 "not an agent" (the old-CLI / unregistered failure mode).
     async fn send_signed(
         &self,
         method: &str,
@@ -211,10 +194,6 @@ impl NodeClient {
                     continue;
                 }
             }
-
-            if let Some(rejection) = signature_rejection(&resp) {
-                return Err(rejection.into_error(method, path, resp).await);
-            }
             return Ok(resp);
         }
     }
@@ -270,145 +249,6 @@ impl NodeClient {
     }
 }
 
-/// A write the node refused with a code this build recognises.
-struct SignatureRejection {
-    /// Which denial it was. Its `as_str` is the `x-gitlawb-error` code, printed
-    /// verbatim so scripts can match on it.
-    denial: NodeDenial,
-    /// What the user should actually do next.
-    hint: &'static str,
-}
-
-impl SignatureRejection {
-    /// Consume the response to fold the node's own message into the error.
-    /// Called only after [`signature_rejection`] has decided from the status and
-    /// header, since reading the body takes the response by value.
-    ///
-    /// The message is attacker controlled (any node the user points `gl` at
-    /// wrote it), so the read is capped and the string is stripped of control
-    /// characters and bidi overrides before it can reach a terminal: without
-    /// that, the text whose whole job is to say "your write was REFUSED" can
-    /// clear the screen and print a fake success line.
-    async fn into_error(self, method: &str, path: &str, resp: reqwest::Response) -> anyhow::Error {
-        let status = resp.status();
-        let capped = read_body_capped(resp, DENIAL_BODY_CAP).await;
-        let node_msg = serde_json::from_str::<serde_json::Value>(&capped.text)
-            .ok()
-            .and_then(|b| b["message"].as_str().map(sanitize_node_msg))
-            .filter(|m| !m.is_empty());
-        let Self { denial, hint } = self;
-        let code = denial.as_str();
-        match node_msg {
-            Some(m) => anyhow::anyhow!("{method} {path} rejected ({status} {code}): {hint} ({m})"),
-            None => anyhow::anyhow!("{method} {path} rejected ({status} {code}): {hint}"),
-        }
-    }
-}
-
-/// Recognise a node rejection from the status and the
-/// `x-gitlawb-error` header, before anything reads the body.
-///
-/// Known limit: the node also puts the same code in the body's `error` field,
-/// and this does not look at it, so a proxy or CDN that strips unknown `X-`
-/// headers switches the detection off. Reading the body to recover the second
-/// signal is destructive, and handing the caller back a response it can still
-/// parse needs `http::Response::builder` (the only constructor
-/// `reqwest::Response::from` takes), which is a dependency this crate does not
-/// carry. The commands that must not mistake a denial for a success do not rely
-/// on this alone: `gl init` compares the structured error code from the body,
-/// and the `gl task` writes check the status before parsing.
-///
-/// None of these is ever retried. A `signature_replayed` 409 means the node
-/// already admitted a request carrying this signature, so re-sending the same
-/// bytes risks applying the mutation twice, which is the duplicate write the
-/// ledger exists to prevent. (The iCaptcha 403 retry above does not have that
-/// problem: `verify_request` runs inside the handler, so the ledger has already
-/// been charged by the time the challenge is returned, but `send_once` signs
-/// every attempt afresh and each signature carries its own nonce, so the retry
-/// arrives under a ledger key the node has never seen.) The 400, 429, 500 and
-/// 503 cases did not apply the write; 429 and 503 are retryable, but doing it
-/// automatically would only hammer a node that is already saying "not now", so
-/// they surface to the user instead.
-///
-/// The set of codes is [`NodeDenial`], shared with the node so both halves
-/// spell them the same way. The `match` below has no wildcard arm on purpose:
-/// adding a denial to the node stops this crate compiling until someone writes
-/// the caller's instructions for it. That is the whole point of the type, since
-/// the previous arrangement (a list of literals in each crate) silently missed
-/// `signature_nonce_too_short` for several commits and returned `Ok(400)` for it.
-fn signature_rejection(resp: &reqwest::Response) -> Option<SignatureRejection> {
-    let code = resp.headers().get("x-gitlawb-error")?.to_str().ok()?;
-    // A code this build does not know: not our denial to describe. The header
-    // came from a node the user chose, which may be newer, older, or hostile,
-    // so the response goes back to the caller untouched.
-    let denial = NodeDenial::from_code(code)?;
-    // The status has to agree with the code, so a stray header on a 200 cannot
-    // turn a success into a refusal.
-    if resp.status().as_u16() != denial.status() {
-        return None;
-    }
-    let hint = match denial {
-        NodeDenial::NonceRequired => {
-            "this node requires a nonce in the request signature and the request did not carry \
-             one. The write did not happen; upgrade `gl` and run the command again"
-        }
-        NodeDenial::NonceTooShort => {
-            "this node rejected the nonce in the request signature as too short to be unique. \
-             The write did not happen; upgrade `gl` and run the command again"
-        }
-        NodeDenial::Replayed => {
-            "the node already admitted a request with this signature and will not take it twice. \
-             Do not resend: check whether the change took effect before running the command again"
-        }
-        NodeDenial::LedgerFull => {
-            "this identity has too many unexpired signatures on the node, so it is refusing more \
-             signed writes for now. The write did not happen; wait a moment and run the command \
-             again"
-        }
-        NodeDenial::IdentityMissing => {
-            "the node reached its signature ledger without a verified identity and refused the \
-             write. The write did not happen; this is a fault on the node, so report it to the \
-             operator"
-        }
-        NodeDenial::LedgerUnavailable => {
-            "the node's signature ledger is unavailable, so it is refusing signed writes. \
-             The write did not happen; retry later or ask the node operator to check the node"
-        }
-        NodeDenial::RateLimited => {
-            "the node is throttling requests from your network address, so it refused this one \
-             before running it. The write did not happen; this is not about your identity or \
-             your signature, so re-running immediately will not help — wait and try again"
-        }
-    };
-    Some(SignatureRejection { denial, hint })
-}
-
-/// Read a node reply, turning a non-2xx into an error instead of handing the
-/// caller an error body that pretty-prints like a success. The message is read
-/// under a cap and sanitized, same as a signature denial.
-pub(crate) async fn json_or_denial<T: serde::de::DeserializeOwned>(
-    what: &str,
-    resp: reqwest::Response,
-) -> Result<T> {
-    let status = resp.status();
-    if !status.is_success() {
-        let capped = read_body_capped(resp, DENIAL_BODY_CAP).await;
-        let msg = serde_json::from_str::<serde_json::Value>(&capped.text)
-            .ok()
-            .and_then(|v| {
-                v.get("message")
-                    .or_else(|| v.get("error"))
-                    .and_then(|m| m.as_str())
-                    .map(str::to_string)
-            })
-            .unwrap_or(capped.text);
-        anyhow::bail!("{what} failed ({status}): {}", sanitize_node_msg(&msg));
-    }
-    resp.json::<T>()
-        .await
-        .with_context(|| format!("invalid JSON response from {what}"))
-}
-
 /// Run the (blocking) iCaptcha solve loop off the async runtime.
 async fn obtain_proof(cfg: IcaptchaCfg) -> Result<String> {
     tokio::task::spawn_blocking(move || icaptcha_client::obtain_proof(&cfg, None))
@@ -429,41 +269,21 @@ pub(crate) struct CappedBody {
     pub(crate) read_failed: bool,
 }
 
-/// The byte twin of [`CappedBody`]: the same completion flags, but the payload is
-/// left as the exact bytes that arrived. Byte-oriented protocol consumers (the git
-/// smart-HTTP ref advertisement on the MCP `git_refs` path) must read through this
-/// one: git ref names are byte strings that need not be UTF-8, so a lossy decode
-/// followed by `as_bytes()` is not a display nicety but corruption — each invalid
-/// byte becomes the 3-byte U+FFFD, changing both the ref name and the pkt-line
-/// length accounting, and a valid repository stops validating.
-pub(crate) struct CappedBytes {
-    /// The bytes read, exactly as they arrived.
-    pub(crate) bytes: Vec<u8>,
-    /// The cap cut the body short.
-    pub(crate) truncated: bool,
-    /// A chunk read FAILED part-way through, so the body is not merely short, it is
-    /// unfinished and the node may have had more to say.
-    pub(crate) read_failed: bool,
-}
-
-/// Read at most `cap` bytes of a response body, raw. Bounds the allocation from a
-/// hostile or broken node returning a huge body — the display is capped
+/// Read at most `cap` bytes of a response body. Bounds the allocation from a
+/// hostile or broken node returning a huge error body — the display is capped
 /// separately, but the read itself must not be unbounded (INV-6, read half).
 ///
 /// `truncated` reports whether the cap cut the body short. A caller that CLASSIFIES
 /// on the body needs it: a cut body fails JSON parse, and a parse failure is
 /// indistinguishable from a node that sent no code at all, so without this flag an
-/// oversized body silently picks a different arm. A protocol consumer needs it even
-/// more: a cut pkt-line stream can still be a syntactically valid *prefix*, so
-/// without the flag a bounded read is indistinguishable from a clean EOF and the
-/// prefix parses as the complete answer.
+/// oversized body silently picks a different arm.
 ///
 /// `read_failed` reports the other way a body can end early. A mid-body read error
 /// used to leave through the same exit as a clean end of stream, so a 500 whose body
 /// died in transit surfaced as an empty message and the caller was told
 /// `node returned 500: ` with nothing after the colon. That is a report of what the
 /// node said, and the node never got to say it.
-pub(crate) async fn read_body_capped_bytes(mut resp: reqwest::Response, cap: usize) -> CappedBytes {
+pub(crate) async fn read_body_capped(mut resp: reqwest::Response, cap: usize) -> CappedBody {
     let mut buf: Vec<u8> = Vec::new();
     let mut truncated = false;
     let mut read_failed = false;
@@ -489,27 +309,13 @@ pub(crate) async fn read_body_capped_bytes(mut resp: reqwest::Response, cap: usi
     if buf.len() >= cap {
         truncated = true;
     }
-    CappedBytes {
-        bytes: buf,
+    CappedBody {
+        text: String::from_utf8_lossy(&buf).into_owned(),
         truncated,
         read_failed,
     }
 }
 
-/// Text view of [`read_body_capped_bytes`], for callers that go on to display or
-/// JSON-parse node-supplied *messages* (denial bodies). The decode is lossy on
-/// purpose there — a denial is prose for a terminal, and a hostile byte becomes a
-/// visible U+FFFD instead of raw output. Anything that parses a byte protocol out
-/// of the body must take the bytes variant instead; see [`CappedBytes`].
-pub(crate) async fn read_body_capped(resp: reqwest::Response, cap: usize) -> CappedBody {
-    let raw = read_body_capped_bytes(resp, cap).await;
-    CappedBody {
-        text: String::from_utf8_lossy(&raw.bytes).into_owned(),
-        truncated: raw.truncated,
-        read_failed: raw.read_failed,
-    }
-}
-
 /// Strip terminal-dangerous characters from (and cap the length of) a
 /// node-supplied error string before surfacing it. The node a caller talks to
 /// could be hostile and embed escape sequences in its error body; those must not
@@ -519,7 +325,7 @@ pub(crate) async fn read_body_capped(resp: reqwest::Response, cap: usize) -> Cap
 pub(crate) fn sanitize_node_msg(s: &str) -> String {
     s.chars()
         .filter(|c| !c.is_control() && !gitlawb_core::sanitize::is_bidi_format(*c))
-        .take(NODE_MSG_CHARS)
+        .take(200)
         .collect()
 }
 
@@ -757,349 +563,6 @@ mod tests {
         m.assert();
     }
 
-    // ── send_signed signature-ledger rejections ─────────────────────────
-
-    /// Mock one node reply and send a signed POST through `send_signed`,
-    /// asserting the node was called exactly once (i.e. nothing retried).
-    async fn send_signed_once(
-        status: usize,
-        headers: &[(&str, &str)],
-        body: &str,
-    ) -> (Result<reqwest::Response>, mockito::ServerGuard) {
-        let mut server = Server::new_async().await;
-        let mut m = server
-            .mock("POST", "/api/v1/repos")
-            .with_status(status)
-            .with_header("content-type", "application/json");
-        for (k, v) in headers {
-            m = m.with_header(*k, v);
-        }
-        let m = m.with_body(body).expect(1).create_async().await;
-        let client = NodeClient::new(server.url(), Some(test_keypair()));
-        let resp = client.send_signed("POST", "/api/v1/repos", b"{}").await;
-        m.assert_async().await;
-        (resp, server)
-    }
-
-    #[tokio::test]
-    async fn send_signed_errors_on_replayed_signature_and_never_retries() {
-        let (resp, _server) = send_signed_once(
-            409,
-            &[("x-gitlawb-error", "signature_replayed")],
-            r#"{"error":"signature_replayed","message":"this signature was already used"}"#,
-        )
-        .await;
-        let err = resp
-            .expect_err("a replayed signature must surface as an error, not Ok(409)")
-            .to_string();
-        assert!(err.contains("signature_replayed"), "got: {err}");
-        assert!(
-            err.contains("already"),
-            "message must tell the user the node already applied it, got: {err}"
-        );
-        assert!(
-            err.contains("this signature was already used"),
-            "must surface the node's message, got: {err}"
-        );
-    }
-
-    #[tokio::test]
-    async fn send_signed_errors_on_ledger_full_and_never_retries() {
-        let (resp, _server) = send_signed_once(
-            429,
-            &[("x-gitlawb-error", "signature_ledger_full")],
-            r#"{"error":"signature_ledger_full","message":"ledger at capacity"}"#,
-        )
-        .await;
-        let err = resp
-            .expect_err("a full ledger must surface as an error")
-            .to_string();
-        assert!(err.contains("signature_ledger_full"), "got: {err}");
-        assert!(
-            !err.contains("already"),
-            "must not claim the write was applied, got: {err}"
-        );
-    }
-
-    #[tokio::test]
-    async fn send_signed_errors_on_ledger_unavailable_and_never_retries() {
-        let (resp, _server) = send_signed_once(
-            503,
-            &[("x-gitlawb-error", "signature_ledger_unavailable")],
-            r#"{"error":"signature_ledger_unavailable","message":"ledger down"}"#,
-        )
-        .await;
-        let err = resp
-            .expect_err("an unavailable ledger must surface as an error")
-            .to_string();
-        assert!(err.contains("signature_ledger_unavailable"), "got: {err}");
-    }
-
-    #[tokio::test]
-    async fn send_signed_errors_on_nonce_required_and_never_retries() {
-        let (resp, _server) = send_signed_once(
-            400,
-            &[("x-gitlawb-error", "signature_nonce_required")],
-            r#"{"error":"signature_nonce_required","message":"this node requires a nonce"}"#,
-        )
-        .await;
-        let err = resp
-            .expect_err("a nonce-less signature must surface as an error, not Ok(400)")
-            .to_string();
-        assert!(err.contains("signature_nonce_required"), "got: {err}");
-        assert!(
-            !err.contains("already"),
-            "must not claim the write was applied, got: {err}"
-        );
-    }
-
-    #[tokio::test]
-    async fn send_signed_errors_on_identity_missing_and_never_retries() {
-        let (resp, _server) = send_signed_once(
-            500,
-            &[("x-gitlawb-error", "signature_identity_missing")],
-            r#"{"error":"signature_identity_missing","message":"no verified identity"}"#,
-        )
-        .await;
-        let err = resp
-            .expect_err("a missing signature identity must surface as an error, not Ok(500)")
-            .to_string();
-        assert!(err.contains("signature_identity_missing"), "got: {err}");
-        assert!(
-            !err.contains("already"),
-            "must not claim the write was applied, got: {err}"
-        );
-    }
-
-    #[tokio::test]
-    async fn send_signed_errors_on_nonce_too_short_and_never_retries() {
-        let (resp, _server) = send_signed_once(
-            400,
-            &[("x-gitlawb-error", "signature_nonce_too_short")],
-            r#"{"error":"signature_nonce_too_short","message":"the nonce must be at least 16 characters"}"#,
-        )
-        .await;
-        let err = resp
-            .expect_err("a too-short nonce must surface as an error, not Ok(400)")
-            .to_string();
-        assert!(err.contains("signature_nonce_too_short"), "got: {err}");
-        assert!(
-            !err.contains("already"),
-            "must not claim the write was applied, got: {err}"
-        );
-        assert!(
-            err.contains("the nonce must be at least 16 characters"),
-            "must surface the node's message, got: {err}"
-        );
-    }
-
-    /// Two unrelated conditions now share 429: the node's per-client flood
-    /// brake and the spent-signature ledger's per-identity cap. They must reach
-    /// the user as different errors, and neither may be retried.
-    #[tokio::test]
-    async fn send_signed_tells_a_rate_limit_brake_apart_from_a_full_ledger() {
-        let (brake, _s1) = send_signed_once(
-            429,
-            &[("x-gitlawb-error", "rate_limited")],
-            r#"{"error":"rate_limited","message":"rate limit exceeded — try again later"}"#,
-        )
-        .await;
-        let brake = brake
-            .expect_err("a rate-limit brake must surface as an error, not Ok(429)")
-            .to_string();
-
-        let (ledger, _s2) = send_signed_once(
-            429,
-            &[("x-gitlawb-error", "signature_ledger_full")],
-            r#"{"error":"signature_ledger_full","message":"ledger at capacity"}"#,
-        )
-        .await;
-        let ledger = ledger
-            .expect_err("a full ledger must surface as an error")
-            .to_string();
-
-        assert!(brake.contains("rate_limited"), "got: {brake}");
-        assert!(
-            !brake.contains("signature_ledger_full"),
-            "a brake must not be described as a ledger denial: {brake}"
-        );
-        assert!(
-            brake.contains("network address"),
-            "the brake hint must say it is keyed on the client address: {brake}"
-        );
-        assert!(ledger.contains("signature_ledger_full"), "got: {ledger}");
-        assert!(
-            !ledger.contains("rate_limited"),
-            "a ledger denial must not be described as a brake: {ledger}"
-        );
-        assert!(
-            ledger.contains("unexpired signatures"),
-            "the ledger hint must say it is keyed on the identity: {ledger}"
-        );
-        assert_ne!(brake, ledger);
-    }
-
-    /// Every signature denial the node can return, as (status, code). Derived
-    /// from the shared enum rather than retyped, so it cannot drift from what
-    /// the node emits.
-    fn all_denials() -> Vec<(usize, &'static str)> {
-        NodeDenial::ALL
-            .iter()
-            .map(|d| (d.status() as usize, d.as_str()))
-            .collect()
-    }
-
-    #[tokio::test]
-    async fn send_signed_errors_on_every_denial_carrying_the_header() {
-        for (status, code) in all_denials() {
-            let body = format!(r#"{{"error":"{code}","message":"node says no"}}"#);
-            let (resp, _server) =
-                send_signed_once(status, &[("x-gitlawb-error", code)], &body).await;
-            let err = match resp {
-                Ok(r) => panic!("{code} returned Ok({}) instead of an error", r.status()),
-                Err(e) => e.to_string(),
-            };
-            assert!(err.contains(code), "wrong code surfaced for {code}: {err}");
-            assert!(
-                err.contains("node says no"),
-                "node message dropped for {code}: {err}"
-            );
-        }
-    }
-
-    #[tokio::test]
-    async fn send_signed_does_not_see_a_denial_carried_only_in_the_body() {
-        // Documents a known limit rather than a wanted behaviour: detection is
-        // header-keyed, so a proxy stripping `x-gitlawb-error` leaves the denial
-        // to the caller (see `signature_rejection`). The callers that must not
-        // read one as a success handle it themselves: `gl init` compares
-        // `error` from the body, `gl task` checks the status before parsing.
-        for (status, code) in all_denials() {
-            let body = format!(r#"{{"error":"{code}","message":"node says no"}}"#);
-            let (resp, _server) = send_signed_once(status, &[], &body).await;
-            let resp = resp.unwrap_or_else(|e| panic!("{code} without the header: {e}"));
-            assert_eq!(resp.status().as_u16(), status as u16);
-            // The body is untouched, which is what lets the caller decide.
-            let payload: serde_json::Value = resp.json().await.unwrap();
-            assert_eq!(payload["error"], code);
-        }
-    }
-
-    #[tokio::test]
-    async fn send_signed_sanitizes_and_caps_a_hostile_denial_message() {
-        // The message that exists to say "your write was REFUSED" must not be
-        // able to clear the screen and print a fake success line.
-        let hostile = format!(
-            "\u{1b}[2J\u{1b}[31mSUCCESS: task created\u{202e}{}",
-            "A".repeat(5000)
-        );
-        let body = serde_json::json!({
-            "error": "signature_replayed",
-            "message": hostile,
-        })
-        .to_string();
-        let (resp, _server) =
-            send_signed_once(409, &[("x-gitlawb-error", "signature_replayed")], &body).await;
-        let err = resp.expect_err("a replay must error").to_string();
-        assert!(
-            !err.contains('\u{1b}'),
-            "ESC leaked to the terminal: {err:?}"
-        );
-        assert!(
-            !err.contains('\u{202e}'),
-            "RLO bidi override leaked: {err:?}"
-        );
-        assert!(err.contains("signature_replayed"), "got: {err}");
-        assert!(
-            err.chars().count() < 600,
-            "denial message not bounded: {} chars",
-            err.chars().count()
-        );
-    }
-
-    #[tokio::test]
-    async fn send_signed_returns_repo_exists_409_unchanged() {
-        // The pre-existing 409. Callers (`gl init`, `gl repo create`) inspect the
-        // status themselves, so it must keep returning Ok(resp).
-        let (resp, _server) = send_signed_once(
-            409,
-            &[("x-node-marker", "kept")],
-            r#"{"error":"repo_exists","message":"already exists"}"#,
-        )
-        .await;
-        let resp = resp.expect("a non-replay 409 must still return Ok");
-        assert_eq!(resp.status(), 409);
-        // Status, headers and body must all survive the denial check: `gl init`
-        // reads the code out of the body to decide "already exists, continue".
-        assert_eq!(resp.headers().get("x-node-marker").unwrap(), "kept");
-        let payload: serde_json::Value = resp.json().await.expect("body must survive intact");
-        assert_eq!(payload["error"], "repo_exists");
-        assert_eq!(payload["message"], "already exists");
-    }
-
-    #[tokio::test]
-    async fn send_signed_returns_409_with_other_error_header_unchanged() {
-        // Only the three signature_* codes are converted; any other
-        // x-gitlawb-error on a 409 is left to the caller.
-        let (resp, _server) = send_signed_once(
-            409,
-            &[("x-gitlawb-error", "repo_exists")],
-            r#"{"error":"repo_exists"}"#,
-        )
-        .await;
-        let resp = resp.expect("an unrelated 409 error code must still return Ok");
-        assert_eq!(resp.status(), 409);
-    }
-
-    #[tokio::test]
-    async fn send_signed_returns_401_invalid_signature_unchanged() {
-        let (resp, _server) = send_signed_once(
-            401,
-            &[("x-gitlawb-error", "invalid_signature")],
-            r#"{"error":"invalid_signature"}"#,
-        )
-        .await;
-        let resp = resp.expect("a 401 must still return Ok, distinct from a replay error");
-        assert_eq!(resp.status(), 401);
-    }
-
-    #[tokio::test]
-    async fn send_signed_returns_401_unsigned_request_unchanged() {
-        // An unsigned write is rejected with human_detected; the client prints a
-        // hint and returns the response, and must not be mistaken for a replay.
-        let mut server = Server::new_async().await;
-        let m = server
-            .mock("POST", "/api/v1/repos")
-            .with_status(401)
-            .with_header("content-type", "application/json")
-            .with_header("x-gitlawb-error", "human_detected")
-            .with_body(r#"{"error":"human_detected"}"#)
-            .expect(1)
-            .create_async()
-            .await;
-        let client = NodeClient::new(server.url(), None);
-        let resp = client
-            .send_signed("POST", "/api/v1/repos", b"{}")
-            .await
-            .expect("an unsigned-request rejection must still return Ok");
-        assert_eq!(resp.status(), 401);
-        m.assert_async().await;
-    }
-
-    #[tokio::test]
-    async fn send_signed_does_not_convert_a_409_on_another_status() {
-        // The header alone must not trigger the conversion: the status has to
-        // match too, so a 200 carrying a stray header stays a success.
-        let (resp, _server) = send_signed_once(
-            200,
-            &[("x-gitlawb-error", "signature_replayed")],
-            r#"{"ok":true}"#,
-        )
-        .await;
-        let resp = resp.expect("a 200 must stay Ok regardless of headers");
-        assert_eq!(resp.status(), 200);
-    }
-
     // ── send_signed iCaptcha retry (full integration) ────────────────────
 
     /// Set GITLAWB_ICAPTCHA_URL and GITLAWB_ICAPTCHA_INSECURE so the iCaptcha
@@ -1220,77 +683,6 @@ mod tests {
         ic.answer.assert();
     }
 
-    #[tokio::test]
-    async fn send_signed_signs_the_icaptcha_retry_afresh() {
-        // What makes the 403 retry safe is that it is a NEW signature, not a
-        // resend: `verify_request` runs inside the handler, so the ledger was
-        // already charged for the first attempt. Each `send_once` signs again
-        // and `sign_request` draws a fresh nonce, so the retry lands on a ledger
-        // key the node has not seen. Pin that: the two attempts must not carry
-        // the same `Signature-Input`.
-        let mut node = Server::new_async().await;
-        let mut icaptcha = Server::new_async().await;
-        let ic = MockIcaptcha::new(&mut icaptcha, 1).await;
-
-        let seen: std::sync::Arc<Mutex<Vec<String>>> = Default::default();
-        let record = {
-            let seen = seen.clone();
-            move |req: &mockito::Request| {
-                let v = req
-                    .header("signature-input")
-                    .first()
-                    .map(|h| String::from_utf8_lossy(h.as_bytes()).into_owned())
-                    .unwrap_or_default();
-                seen.lock().unwrap().push(v);
-                true
-            }
-        };
-
-        let n1 = node
-            .mock("POST", "/api/register")
-            .match_header("x-icaptcha-proof", mockito::Matcher::Missing)
-            .match_request(record.clone())
-            .with_status(403)
-            .with_header("content-type", "application/json")
-            .with_header("x-icaptcha-url", &ic.url)
-            .with_header("x-icaptcha-level", "3")
-            .with_body(r#"{"error":"icaptcha_proof_required"}"#)
-            .expect(1)
-            .create_async()
-            .await;
-        let n2 = node
-            .mock("POST", "/api/register")
-            .match_header("x-icaptcha-proof", "mock.proof")
-            .match_request(record)
-            .with_status(201)
-            .with_header("content-type", "application/json")
-            .with_body(r#"{"status":"created"}"#)
-            .expect(1)
-            .create_async()
-            .await;
-
-        let client = NodeClient::new(node.url(), Some(test_keypair()));
-        let resp = client
-            .send_signed("POST", "/api/register", b"{}")
-            .await
-            .unwrap();
-        assert_eq!(resp.status(), 201);
-        n1.assert();
-        n2.assert();
-
-        let seen = seen.lock().unwrap();
-        assert_eq!(seen.len(), 2, "expected two attempts, got {seen:?}");
-        assert!(
-            !seen[0].is_empty() && !seen[1].is_empty(),
-            "both attempts must be signed: {seen:?}"
-        );
-        assert_ne!(
-            seen[0], seen[1],
-            "the retry reused the first signature, so it would land on the same \
-             ledger key the node already spent"
-        );
-    }
-
     #[tokio::test]
     async fn send_signed_returns_403_after_icaptcha_retries_exhausted() {
         let mut node = Server::new_async().await;

crates/gl/src/init.rs

@@ -9,7 +9,7 @@ use clap::Args;
 use serde_json::{json, Value};
 use std::path::PathBuf;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 #[derive(Args)]
@@ -33,14 +33,6 @@ pub struct InitArgs {
 
 pub async fn run(args: InitArgs) -> Result<()> {
     let cwd = std::env::current_dir().context("cannot determine current directory")?;
-    run_in(&cwd, args).await
-}
-
-/// The body of [`run`], with the working directory passed in. Split out so the
-/// whole flow (including what it prints on a node denial) is testable without
-/// mutating the process-wide current directory.
-async fn run_in(cwd: &std::path::Path, args: InitArgs) -> Result<()> {
-    let cwd = cwd.to_path_buf();
 
     // 1. Ensure git repo exists
     let git_dir = cwd.join(".git");
@@ -95,12 +87,16 @@ async fn run_in(cwd: &std::path::Path, args: InitArgs) -> Result<()> {
         .post("/api/register", &body)
         .await
         .context("failed to connect to node")?;
-    // No tolerated failure here: `register_agent` upserts, so re-registering a
-    // known DID is a 201, not a conflict (node `api/register.rs`, and the ON
-    // CONFLICT clause in `db::register_agent`). An older check let any message
-    // containing "already" through, which included the replay denial "this
-    // signature has already been used".
-    let payload: Value = json_or_denial("registration", resp).await?;
+    let status = resp.status();
+    let payload: Value = resp.json().await.context("invalid JSON from register")?;
+
+    if !status.is_success() {
+        let msg = payload["message"].as_str().unwrap_or("unknown error");
+        // "already registered" is fine
+        if !msg.contains("already") {
+            anyhow::bail!("registration failed ({status}): {msg}");
+        }
+    }
 
     // Save UCAN if returned
     if let Some(ucan) = payload.get("ucan").and_then(|v| v.as_str()) {
@@ -143,22 +139,15 @@ async fn run_in(cwd: &std::path::Path, args: InitArgs) -> Result<()> {
         .await
         .context("failed to create repo")?;
     let repo_status = resp.status();
+    let repo_result: Value = resp.json().await.context("invalid JSON from create repo")?;
+
     if !repo_status.is_success() {
-        let capped = crate::http::read_body_capped(resp, crate::http::DENIAL_BODY_CAP).await;
-        let repo_result: Value = serde_json::from_str(&capped.text).unwrap_or(Value::Null);
-        if !repo_already_exists(&repo_result) {
-            let msg = repo_result["message"]
-                .as_str()
-                .or_else(|| repo_result["error"].as_str())
-                .unwrap_or("unknown error");
-            anyhow::bail!(
-                "create repo failed ({repo_status}): {}",
-                crate::http::sanitize_node_msg(msg)
-            );
+        let msg = repo_result["message"].as_str().unwrap_or("unknown error");
+        if !msg.contains("exists") && !msg.contains("already") {
+            anyhow::bail!("create repo failed ({repo_status}): {msg}");
         }
         println!("  Repository already exists — continuing.");
     } else {
-        let _repo_result: Value = resp.json().await.context("invalid JSON from create repo")?;
         println!("  Repository created.");
     }
 
@@ -231,14 +220,6 @@ async fn run_in(cwd: &std::path::Path, args: InitArgs) -> Result<()> {
     Ok(())
 }
 
-/// Is this failed create-repo reply the benign "you already own that repo" case?
-/// `AppError::RepoExists` is the only thing the node renders as `repo_exists`
-/// (node `error.rs`), and it is the only non-success reply `gl init` may treat
-/// as "keep going".
-fn repo_already_exists(payload: &Value) -> bool {
-    payload["error"].as_str() == Some("repo_exists")
-}
-
 fn generate_identity(dir: Option<&std::path::Path>) -> Result<gitlawb_core::identity::Keypair> {
     let base = if let Some(d) = dir {
         d.to_path_buf()
@@ -351,155 +332,56 @@ mod tests {
         assert_eq!(resp["name"], "test-repo");
     }
 
-    /// A work dir with a git repo in it, plus an identity dir, wired to `node`.
-    fn init_args(node: String, id_dir: &TempDir) -> (InitArgs, TempDir) {
-        let work = TempDir::new().unwrap();
-        std::process::Command::new("git")
-            .args(["init", "-b", "main"])
-            .current_dir(work.path())
-            .stdout(std::process::Stdio::null())
-            .stderr(std::process::Stdio::null())
-            .status()
-            .unwrap();
-        let args = InitArgs {
-            name: Some("test-repo".to_string()),
-            node,
-            dir: Some(id_dir.path().to_path_buf()),
-            description: None,
-        };
-        (args, work)
-    }
-
-    fn has_gitlawb_remote(work: &TempDir) -> bool {
-        std::process::Command::new("git")
-            .args(["remote", "get-url", "gitlawb"])
-            .current_dir(work.path())
-            .stdout(std::process::Stdio::null())
-            .stderr(std::process::Stdio::null())
-            .status()
-            .map(|s| s.success())
-            .unwrap_or(false)
-    }
-
-    async fn mock_register(server: &mut mockito::ServerGuard) -> mockito::Mock {
-        server
-            .mock("POST", "/api/register")
-            .with_status(201)
-            .with_header("content-type", "application/json")
-            .with_body(r#"{"status":"accepted","message":"welcome","ucan":"test.token"}"#)
-            .create_async()
-            .await
-    }
-
     #[tokio::test]
-    async fn init_fails_on_replay_409_without_the_error_header() {
-        // The node refused the create (the signature was already spent) and a
-        // proxy stripped `x-gitlawb-error`, so the client layer cannot see the
-        // denial and hands the 409 straight back. `gl init` used to read
-        // "already" out of the prose, print "Repository already exists —
-        // continuing", add the remote, print "Ready! Push with", and exit 0 for
-        // a repo that does not exist. Comparing the structured error code is
-        // what makes it fail instead, before any of that.
-        let id_dir = TempDir::new().unwrap();
-        write_identity(&id_dir);
-        let mut server = mockito::Server::new_async().await;
-        let _reg = mock_register(&mut server).await;
-        let _repo = server
-            .mock("POST", "/api/v1/repos")
-            .with_status(409)
-            .with_header("content-type", "application/json")
-            .with_body(
-                r#"{"error":"signature_replayed","message":"this signature has already been used - sign a fresh request"}"#,
-            )
-            .create_async()
-            .await;
+    async fn test_init_handles_already_registered() {
+        let dir = TempDir::new().unwrap();
+        let kp = write_identity(&dir);
 
-        let (args, work) = init_args(server.url(), &id_dir);
-        let err = run_in(work.path(), args)
-            .await
-            .expect_err("a replayed signature must fail `gl init`, not report success");
-        let err = format!("{err:#}");
-        assert!(err.contains("create repo failed"), "got: {err}");
-        assert!(err.contains("409"), "status not surfaced: {err}");
-        // The remote is added several lines after the point where the old code
-        // decided "already exists, continuing", so its absence proves we bailed
-        // before printing "Ready! Push with".
-        assert!(
-            !has_gitlawb_remote(&work),
-            "init continued past the denial and configured the remote"
-        );
-    }
-
-    #[tokio::test]
-    async fn init_still_continues_on_a_real_repo_exists_409() {
-        // The regression guard for the fix above: a genuine repo_exists conflict
-        // must behave exactly as before (continue, add the remote, succeed).
-        let id_dir = TempDir::new().unwrap();
-        write_identity(&id_dir);
-        let mut server = mockito::Server::new_async().await;
-        let _reg = mock_register(&mut server).await;
-        let _repo = server
-            .mock("POST", "/api/v1/repos")
-            .with_status(409)
-            .with_header("content-type", "application/json")
-            .with_body(
-                r#"{"error":"repo_exists","message":"repository 'test-repo' already exists"}"#,
-            )
-            .create_async()
-            .await;
-
-        let (args, work) = init_args(server.url(), &id_dir);
-        run_in(work.path(), args)
-            .await
-            .expect("an existing repo must still be a successful `gl init`");
-        assert!(
-            has_gitlawb_remote(&work),
-            "init must still configure the remote for an existing repo"
-        );
-    }
-
-    #[tokio::test]
-    async fn init_fails_when_registration_is_refused() {
-        // Registration is an upsert on the node, so any non-success is a real
-        // failure. The old `contains("already")` tolerance also swallowed the
-        // replay denial here.
-        let id_dir = TempDir::new().unwrap();
-        write_identity(&id_dir);
         let mut server = mockito::Server::new_async().await;
         let _reg = server
             .mock("POST", "/api/register")
             .with_status(409)
             .with_header("content-type", "application/json")
-            .with_body(
-                r#"{"error":"signature_replayed","message":"this signature has already been used"}"#,
-            )
+            .with_body(r#"{"message":"already registered"}"#)
             .create_async()
             .await;
 
-        let (args, work) = init_args(server.url(), &id_dir);
-        let err = run_in(work.path(), args)
-            .await
-            .expect_err("a refused registration must fail `gl init`");
-        let err = format!("{err:#}");
-        assert!(err.contains("registration failed"), "got: {err}");
-        assert!(err.contains("409"), "status not surfaced: {err}");
-        assert!(!has_gitlawb_remote(&work));
+        let client = NodeClient::new(server.url(), Some(kp.clone()));
+        let body = serde_json::to_vec(&json!({
+            "did": kp.did().to_string(),
+            "capabilities": ["git:push"],
+        }))
+        .unwrap();
+        let resp = client.post("/api/register", &body).await.unwrap();
+        let status = resp.status();
+        let payload: Value = resp.json().await.unwrap();
+        let msg = payload["message"].as_str().unwrap_or("");
+        // Should not bail because message contains "already"
+        assert!(!status.is_success());
+        assert!(msg.contains("already"));
     }
 
-    #[test]
-    fn repo_already_exists_matches_the_code_not_the_prose() {
-        assert!(repo_already_exists(
-            &json!({"error":"repo_exists","message":"repository 'x' already exists"})
-        ));
-        // The replay denial's message contains "already" and "exists" is one
-        // word away; only the code separates them.
-        assert!(!repo_already_exists(&json!({
-            "error": "signature_replayed",
-            "message": "this signature has already been used - sign a fresh request"
-        })));
-        assert!(!repo_already_exists(
-            &json!({"message":"repository already exists"})
-        ));
-        assert!(!repo_already_exists(&json!({})));
+    #[tokio::test]
+    async fn test_init_handles_repo_exists() {
+        let dir = TempDir::new().unwrap();
+        let kp = write_identity(&dir);
+
+        let mut server = mockito::Server::new_async().await;
+        let _repo = server
+            .mock("POST", "/api/v1/repos")
+            .with_status(409)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"message":"repository already exists"}"#)
+            .create_async()
+            .await;
+
+        let client = NodeClient::new(server.url(), Some(kp.clone()));
+        let body = serde_json::to_vec(&json!({"name": "existing", "is_public": true})).unwrap();
+        let resp = client.post("/api/v1/repos", &body).await.unwrap();
+        let status = resp.status();
+        let result: Value = resp.json().await.unwrap();
+        let msg = result["message"].as_str().unwrap_or("");
+        assert!(!status.is_success());
+        assert!(msg.contains("exists") || msg.contains("already"));
     }
 }

crates/gl/src/issue.rs

@@ -7,7 +7,7 @@ use serde_json::{json, Value};
 use std::path::PathBuf;
 use uuid::Uuid;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 fn signed_client(node: &str, dir: Option<&std::path::Path>) -> NodeClient {
@@ -198,7 +198,13 @@ async fn cmd_create(
         .post(&path, &request_body)
         .await
         .context("failed to connect to node")?;
-    let result: Value = json_or_denial("create issue", resp).await?;
+    let status = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("create issue failed ({status}): {msg}");
+    }
 
     let id = result["id"].as_str().unwrap_or("?");
     println!("✓ Created issue #{id}");
@@ -258,7 +264,13 @@ async fn cmd_show(repo: String, id: String, node: String, dir: Option<PathBuf>)
         .get_authed(&path)
         .await
         .context("failed to connect to node")?;
-    let issue: Value = json_or_denial("show", resp).await?;
+    let status = resp.status();
+    let issue: Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = issue["message"].as_str().unwrap_or("issue not found");
+        anyhow::bail!("show failed ({status}): {msg}");
+    }
 
     let title = issue["title"].as_str().unwrap_or("(no title)");
     let status = issue["status"].as_str().unwrap_or("?");
@@ -289,8 +301,13 @@ async fn cmd_close(repo: String, id: String, node: String, dir: Option<PathBuf>)
         .post(&format!("{path}/close"), &body)
         .await
         .context("failed to connect to node")?;
-    // Parsed only to reject a denial; the success body carries nothing to print.
-    json_or_denial::<Value>("close issue", resp).await?;
+    let status = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("close issue failed ({status}): {msg}");
+    }
 
     println!("✗ Closed issue {id}");
     Ok(())
@@ -315,7 +332,13 @@ async fn cmd_issue_comment(
         )
         .await
         .context("failed to connect to node")?;
-    json_or_denial::<Value>("comment", resp).await?;
+    let code = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON")?;
+
+    if !code.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("comment failed ({code}): {msg}");
+    }
 
     println!("· Comment posted on issue {id}");
     Ok(())

crates/gl/src/pr.rs

@@ -5,7 +5,7 @@ use clap::{Args, Subcommand};
 use serde_json::Value;
 use std::path::PathBuf;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 #[derive(Args)]
@@ -233,7 +233,13 @@ async fn cmd_create(
         .post(&format!("/api/v1/repos/{owner}/{repo}/pulls"), &payload)
         .await
         .context("failed to connect to node")?;
-    let pr: Value = json_or_denial("create PR", resp).await?;
+    let status = resp.status();
+    let pr: Value = resp.json().await.context("invalid JSON")?;
+
+    if !status.is_success() {
+        let msg = pr["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("create PR failed ({status}): {msg}");
+    }
 
     let number = pr["number"].as_i64().unwrap_or(0);
     println!("✓ Opened PR #{number}: {title}");
@@ -409,7 +415,13 @@ async fn cmd_merge(repo: String, number: u64, node: String, dir: Option<PathBuf>
         )
         .await
         .context("failed to connect to node")?;
-    let result: Value = json_or_denial("merge", resp).await?;
+    let status = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON")?;
+
+    if !status.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("merge failed ({status}): {msg}");
+    }
 
     let sha = result["merge_sha"].as_str().unwrap_or("?");
     println!("✓ Merged PR #{number}");
@@ -441,7 +453,13 @@ async fn cmd_review(
         )
         .await
         .context("failed to connect to node")?;
-    json_or_denial::<Value>("review", resp).await?;
+    let code = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON")?;
+
+    if !code.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("review failed ({code}): {msg}");
+    }
 
     let icon = match status.as_str() {
         "approved" => "✓",
@@ -471,7 +489,13 @@ async fn cmd_comment(
         )
         .await
         .context("failed to connect to node")?;
-    json_or_denial::<Value>("comment", resp).await?;
+    let code = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON")?;
+
+    if !code.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("comment failed ({code}): {msg}");
+    }
 
     println!("· Comment posted on PR #{number}");
     Ok(())

crates/gl/src/profile.rs

@@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};
 use serde_json::json;
 use std::path::PathBuf;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 #[derive(Args)]
@@ -248,10 +248,16 @@ async fn cmd_set(
         .await
         .context("failed to update profile")?;
 
-    // Status before body. A denial need not be JSON at all — the per-client
-    // rate-limit brake used to answer text/plain — and parsing first turned a
-    // 429 into "invalid JSON response", which names the wrong problem.
-    let resp_body: serde_json::Value = json_or_denial("profile update", resp).await?;
+    let status = resp.status();
+    let resp_body: serde_json::Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = resp_body
+            .get("message")
+            .and_then(|v| v.as_str())
+            .unwrap_or("unknown error");
+        anyhow::bail!("profile update failed ({status}): {msg}");
+    }
 
     println!();
     println!("✓ Profile updated");
@@ -392,8 +398,16 @@ async fn cmd_import(path: PathBuf, pin: bool, node: String, dir: Option<PathBuf>
         .await
         .context("failed to import profile")?;
 
-    // Parsed only to reject a denial; the success body carries nothing to print.
-    json_or_denial::<serde_json::Value>("profile import", resp).await?;
+    let status = resp.status();
+    let resp_body: serde_json::Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = resp_body
+            .get("message")
+            .and_then(|v| v.as_str())
+            .unwrap_or("unknown error");
+        anyhow::bail!("profile import failed ({status}): {msg}");
+    }
 
     println!("✓ Profile imported successfully");
     Ok(())
@@ -442,66 +456,6 @@ fn did_short(did: &str) -> &str {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use tempfile::TempDir;
-
-    fn write_identity(dir: &TempDir) {
-        let kp = gitlawb_core::identity::Keypair::generate();
-        std::fs::write(
-            dir.path().join("identity.pem"),
-            kp.to_pem().unwrap().as_bytes(),
-        )
-        .unwrap();
-    }
-
-    async fn set_against(status: usize, headers: &[(&str, &str)], body: &str) -> String {
-        let dir = TempDir::new().unwrap();
-        write_identity(&dir);
-        let mut server = mockito::Server::new_async().await;
-        let mut m = server.mock("PUT", "/api/v1/profile").with_status(status);
-        for (k, v) in headers {
-            m = m.with_header(*k, v);
-        }
-        let _m = m.with_body(body).create_async().await;
-        let err = cmd_set(
-            Some("Axiom".into()),
-            None,
-            None,
-            None,
-            None,
-            None,
-            None,
-            None,
-            false,
-            server.url(),
-            Some(dir.path().to_path_buf()),
-        )
-        .await
-        .expect_err("a 429 must not be reported as success");
-        format!("{err:#}")
-    }
-
-    /// A per-IP brake 429 is newly reachable on this route. Parsing the body
-    /// before checking the status turned it into "invalid JSON response",
-    /// which tells the user nothing about the real condition.
-    #[tokio::test]
-    async fn profile_set_surfaces_a_rate_limit_brake_not_invalid_json() {
-        // Header present: the client recognises the code up front.
-        let err = set_against(
-            429,
-            &[("x-gitlawb-error", "rate_limited")],
-            r#"{"error":"rate_limited","message":"rate limit exceeded — try again later"}"#,
-        )
-        .await;
-        assert!(!err.contains("invalid JSON"), "got: {err}");
-        assert!(err.contains("rate_limited"), "got: {err}");
-
-        // Header stripped by a proxy, body not JSON: the status still has to
-        // decide, so this must read as a 429, not as a parse failure.
-        let err = set_against(429, &[], "rate limit exceeded — try again later").await;
-        assert!(!err.contains("invalid JSON"), "got: {err}");
-        assert!(err.contains("429"), "got: {err}");
-        assert!(err.contains("rate limit exceeded"), "got: {err}");
-    }
 
     #[test]
     fn test_did_short_extracts_suffix() {

crates/gl/src/register.rs

@@ -8,7 +8,7 @@ use clap::Args;
 use serde_json::{json, Value};
 use std::path::PathBuf;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 #[derive(Args)]
@@ -55,7 +55,16 @@ pub async fn run(args: RegisterArgs) -> Result<()> {
         .await
         .context("failed to connect to node")?;
 
-    let payload: Value = json_or_denial("registration", resp).await?;
+    let status = resp.status();
+    let payload: Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = payload
+            .get("message")
+            .and_then(|v| v.as_str())
+            .unwrap_or("unknown error");
+        anyhow::bail!("registration failed ({status}): {msg}");
+    }
 
     // Save bootstrap UCAN
     let ucan = payload.get("ucan").and_then(|v| v.as_str()).unwrap_or("");

crates/gl/src/repo.rs

@@ -5,7 +5,7 @@ use clap::{Args, Subcommand};
 use serde_json::{json, Value};
 use std::path::PathBuf;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 #[derive(Args)]
@@ -251,7 +251,13 @@ async fn cmd_create(
         .post("/api/v1/repos", &body)
         .await
         .context("failed to connect to node")?;
-    let payload: Value = json_or_denial("create", resp).await?;
+    let status = resp.status();
+    let payload: Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = payload["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("create failed ({status}): {msg}");
+    }
 
     let clone_url = payload["clone_url"].as_str().unwrap_or("");
     let gitlawb_url = format!("gitlawb://{owner_did}/{name}");
@@ -573,9 +579,13 @@ async fn cmd_fork(
         .post(&format!("/api/v1/repos/{owner}/{repo_name}/fork"), &body)
         .await
         .context("failed to connect to node")?;
-    // Status before body: a rate-limit brake or any other non-JSON denial must
-    // read as the denial, not as "invalid JSON response".
-    let result: Value = json_or_denial("fork", resp).await?;
+    let status = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON response")?;
+
+    if !status.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("fork failed ({status}): {msg}");
+    }
 
     let fork_name = result["name"].as_str().unwrap_or(&repo_name);
     let owner_did = result["owner_did"].as_str().unwrap_or("?");
@@ -613,7 +623,13 @@ async fn cmd_label_add(
         .post(&format!("/api/v1/repos/{owner}/{name}/labels"), &body)
         .await
         .context("failed to connect to node")?;
-    let result: Value = json_or_denial("add label", resp).await?;
+    let status = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON")?;
+
+    if !status.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("add label failed ({status}): {msg}");
+    }
 
     let added = result["added"].as_bool().unwrap_or(true);
     if added {
@@ -638,8 +654,13 @@ async fn cmd_label_remove(
         .delete(&format!("/api/v1/repos/{owner}/{name}/labels/{label}"), &[])
         .await
         .context("failed to connect to node")?;
-    // Parsed only to reject a denial; the success body carries nothing to print.
-    json_or_denial::<Value>("remove label", resp).await?;
+    let status = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON")?;
+
+    if !status.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("remove label failed ({status}): {msg}");
+    }
 
     println!("- Label removed: {label} from {owner}/{name}");
     Ok(())
@@ -799,48 +820,6 @@ mod tests {
         assert!(!owner.contains(':'));
     }
 
-    async fn fork_against(status: usize, headers: &[(&str, &str)], body: &str) -> String {
-        let dir = TempDir::new().unwrap();
-        write_identity(&dir);
-        let mut server = mockito::Server::new_async().await;
-        let mut m = server
-            .mock("POST", "/api/v1/repos/alice/myrepo/fork")
-            .with_status(status);
-        for (k, v) in headers {
-            m = m.with_header(*k, v);
-        }
-        let _m = m.with_body(body).create_async().await;
-        let err = cmd_fork(
-            "alice/myrepo".to_string(),
-            None,
-            server.url(),
-            Some(dir.path().to_path_buf()),
-        )
-        .await
-        .expect_err("a 429 must not be reported as success");
-        format!("{err:#}")
-    }
-
-    /// Same shape as `gl profile set`: the fork route gained a per-IP brake, so
-    /// a 429 with a non-JSON body has to read as a rate limit rather than as a
-    /// parse failure.
-    #[tokio::test]
-    async fn fork_surfaces_a_rate_limit_brake_not_invalid_json() {
-        let err = fork_against(
-            429,
-            &[("x-gitlawb-error", "rate_limited")],
-            r#"{"error":"rate_limited","message":"rate limit exceeded — try again later"}"#,
-        )
-        .await;
-        assert!(!err.contains("invalid JSON"), "got: {err}");
-        assert!(err.contains("rate_limited"), "got: {err}");
-
-        let err = fork_against(429, &[], "rate limit exceeded — try again later").await;
-        assert!(!err.contains("invalid JSON"), "got: {err}");
-        assert!(err.contains("429"), "got: {err}");
-        assert!(err.contains("rate limit exceeded"), "got: {err}");
-    }
-
     #[tokio::test]
     async fn test_cmd_create_success() {
         let dir = TempDir::new().unwrap();

crates/gl/src/task.rs

@@ -5,7 +5,7 @@ use clap::{Args, Subcommand};
 use serde_json::{json, Value};
 use std::path::PathBuf;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 #[derive(Args)]
@@ -166,14 +166,13 @@ async fn cmd_create(
         "delegator_did": delegator_did,
     }))?;
 
-    let resp = client
+    let resp: Value = client
         .post("/api/v1/tasks", &body)
         .await
-        .context("failed to create task")?;
-    // Check the status BEFORE parsing. Without this a denial (a 400
-    // `signature_nonce_required`, a 500, a 404) pretty-prints the node's error
-    // JSON and exits 0, so a script keying on `$?` treats the task as created.
-    let resp: Value = json_or_denial("create task", resp).await?;
+        .context("failed to create task")?
+        .json()
+        .await
+        .context("invalid JSON response")?;
     print_json(&resp);
     Ok(())
 }
@@ -222,11 +221,13 @@ async fn cmd_claim(id: String, node: String, dir: Option<PathBuf>) -> Result<()>
     let client = NodeClient::new(&node, Some(keypair));
 
     let body = serde_json::to_vec(&json!({ "assignee_did": assignee_did }))?;
-    let resp = client
+    let resp: Value = client
         .post(&format!("/api/v1/tasks/{}/claim", id), &body)
         .await
-        .context("failed to claim task")?;
-    let resp: Value = json_or_denial("claim task", resp).await?;
+        .context("failed to claim task")?
+        .json()
+        .await
+        .context("invalid JSON response")?;
     print_json(&resp);
     Ok(())
 }
@@ -242,11 +243,13 @@ async fn cmd_complete(
     let client = NodeClient::new(&node, Some(keypair));
 
     let body = serde_json::to_vec(&json!({ "result": result, "by_did": by_did }))?;
-    let resp = client
+    let resp: Value = client
         .post(&format!("/api/v1/tasks/{}/complete", id), &body)
         .await
-        .context("failed to complete task")?;
-    let resp: Value = json_or_denial("complete task", resp).await?;
+        .context("failed to complete task")?
+        .json()
+        .await
+        .context("invalid JSON response")?;
     print_json(&resp);
     Ok(())
 }
@@ -262,11 +265,13 @@ async fn cmd_fail(
     let client = NodeClient::new(&node, Some(keypair));
 
     let body = serde_json::to_vec(&json!({ "reason": reason, "by_did": by_did }))?;
-    let resp = client
+    let resp: Value = client
         .post(&format!("/api/v1/tasks/{}/fail", id), &body)
         .await
-        .context("failed to fail task")?;
-    let resp: Value = json_or_denial("fail task", resp).await?;
+        .context("failed to fail task")?
+        .json()
+        .await
+        .context("invalid JSON response")?;
     print_json(&resp);
     Ok(())
 }
@@ -334,129 +339,6 @@ mod tests {
         assert!(err.to_string().contains("no identity found"));
     }
 
-    /// Write an identity into a fresh dir so the signed task commands run.
-    fn identity_dir() -> tempfile::TempDir {
-        let dir = tempfile::TempDir::new().unwrap();
-        let kp = gitlawb_core::identity::Keypair::generate();
-        std::fs::write(
-            dir.path().join("identity.pem"),
-            kp.to_pem().unwrap().as_bytes(),
-        )
-        .unwrap();
-        dir
-    }
-
-    /// Every denial a task write can come back with, as (status, body).
-    /// The last entry is a plain node failure with no signature code: the task
-    /// commands must fail on that too, not just on the ledger denials.
-    fn denials() -> Vec<(usize, String)> {
-        gitlawb_core::node_denial::NodeDenial::ALL
-            .iter()
-            .map(|d| {
-                (
-                    d.status() as usize,
-                    format!(
-                        r#"{{"error":"{}","message":"node refused the write"}}"#,
-                        d.as_str()
-                    ),
-                )
-            })
-            .chain(std::iter::once((
-                500,
-                r#"{"error":"internal_error","message":"boom"}"#.to_string(),
-            )))
-            .collect()
-    }
-
-    #[tokio::test]
-    async fn task_writes_fail_on_every_denial() {
-        for (status, body) in denials() {
-            let dir = identity_dir();
-            let mut server = mockito::Server::new_async().await;
-            let _m = server
-                .mock("POST", mockito::Matcher::Any)
-                .with_status(status)
-                .with_header("content-type", "application/json")
-                .with_body(&body)
-                .expect_at_least(1)
-                .create_async()
-                .await;
-            let path = Some(dir.path().to_path_buf());
-
-            let e = cmd_create(
-                "deploy".into(),
-                "agent:task".into(),
-                None,
-                None,
-                None,
-                None,
-                None,
-                server.url(),
-                path.clone(),
-            )
-            .await;
-            assert!(
-                e.is_err(),
-                "task create returned Ok for {status} {body}: a script keying on $? \
-                 would treat the task as created"
-            );
-
-            for (name, res) in [
-                (
-                    "claim",
-                    cmd_claim("t1".into(), server.url(), path.clone()).await,
-                ),
-                (
-                    "complete",
-                    cmd_complete("t1".into(), None, server.url(), path.clone()).await,
-                ),
-                (
-                    "fail",
-                    cmd_fail("t1".into(), None, server.url(), path.clone()).await,
-                ),
-            ] {
-                assert!(res.is_err(), "task {name} returned Ok for {status} {body}");
-            }
-        }
-    }
-
-    #[tokio::test]
-    async fn task_create_denial_message_is_sanitized_and_bounded() {
-        let dir = identity_dir();
-        let mut server = mockito::Server::new_async().await;
-        let hostile = format!("\u{1b}[2Jfake success\u{202e}{}", "A".repeat(5000));
-        let body = serde_json::json!({"error": "internal_error", "message": hostile}).to_string();
-        let _m = server
-            .mock("POST", "/api/v1/tasks")
-            .with_status(500)
-            .with_header("content-type", "application/json")
-            .with_body(&body)
-            .create_async()
-            .await;
-
-        let err = cmd_create(
-            "deploy".into(),
-            "agent:task".into(),
-            None,
-            None,
-            None,
-            None,
-            None,
-            server.url(),
-            Some(dir.path().to_path_buf()),
-        )
-        .await
-        .unwrap_err()
-        .to_string();
-        assert!(!err.contains('\u{1b}'), "ESC leaked: {err:?}");
-        assert!(!err.contains('\u{202e}'), "RLO leaked: {err:?}");
-        assert!(
-            err.chars().count() < 600,
-            "not bounded: {} chars",
-            err.chars().count()
-        );
-    }
-
     #[tokio::test]
     async fn test_create_task_server_error() {
         let mut server = mockito::Server::new_async().await;
@@ -476,7 +358,8 @@ mod tests {
             .create_async()
             .await;
 
-        let err = cmd_create(
+        // Should still succeed (prints JSON, doesn't check status code)
+        cmd_create(
             "deploy".to_string(),
             "agent:task".to_string(),
             None,
@@ -488,10 +371,7 @@ mod tests {
             Some(dir.path().to_path_buf()),
         )
         .await
-        .unwrap_err()
-        .to_string();
-        assert!(err.contains("500"), "status not surfaced: {err}");
-        assert!(err.contains("internal error"), "message dropped: {err}");
+        .unwrap();
     }
 
     // ── list ─────────────────────────────────────────────────────────

crates/gl/src/webhook.rs

@@ -5,7 +5,7 @@ use clap::{Args, Subcommand};
 use serde_json::Value;
 use std::path::PathBuf;
 
-use crate::http::{json_or_denial, NodeClient};
+use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
 
 #[derive(Args)]
@@ -102,7 +102,13 @@ async fn cmd_create(
         .post(&format!("/api/v1/repos/{owner}/{name}/hooks"), &payload)
         .await
         .context("failed to connect to node")?;
-    let hook: Value = json_or_denial("create webhook", resp).await?;
+    let status = resp.status();
+    let hook: Value = resp.json().await.context("invalid JSON")?;
+
+    if !status.is_success() {
+        let msg = hook["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("create webhook failed ({status}): {msg}");
+    }
 
     let id = hook["id"].as_str().unwrap_or("?");
     let hook_events = hook["events"]
@@ -142,7 +148,13 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
     let resp = client
         .get_signed(&format!("/api/v1/repos/{owner}/{name}/hooks"))
         .await?;
-    let body: Value = json_or_denial("list webhooks", resp).await?;
+    let status = resp.status();
+    let body: Value = resp.json().await.context("invalid JSON")?;
+
+    if !status.is_success() {
+        let msg = body["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("list webhooks failed ({status}): {msg}");
+    }
 
     let hooks = body
         .get("webhooks")
@@ -190,8 +202,13 @@ async fn cmd_delete(repo: String, id: String, node: String, dir: Option<PathBuf>
         )
         .await
         .context("failed to connect to node")?;
-    // Parsed only to reject a denial; the success body carries nothing to print.
-    json_or_denial::<Value>("delete webhook", resp).await?;
+    let status = resp.status();
+    let result: Value = resp.json().await.context("invalid JSON")?;
+
+    if !status.is_success() {
+        let msg = result["message"].as_str().unwrap_or("unknown error");
+        anyhow::bail!("delete webhook failed ({status}): {msg}");
+    }
 
     println!("✓ Webhook {id} deleted");
     Ok(())