fix(node): propagate for-each-ref failures in resolve_issue_id

get_issue folded every cat-file failure into Ok(None), so a corrupt or
unreadable object store looked identical to a missing issue. close_issue
then hit that None right after resolve_issue_id had proven the ref
exists and panicked on the expect.

A shared read_issue_blob helper now re-resolves the ref with rev-parse
(which does not read the object) when cat-file fails: unresolved means
genuinely absent (Ok(None)), resolved means a real read failure
(propagated as Err). list_issues applies the same split so a degraded
store errors the listing instead of silently under-reporting, and
close_issue treats a ref deleted between resolve and read as not-found
instead of panicking.

Closes #426.

crates/git-remote-gitlawb/src/main.rs

@@ -287,10 +287,8 @@ fn handle_connect<R: Read>(
     //    this into one POST deadlocks a multi-round fetch (#117).
     //
     //  git-receive-pack (push):
-    //    Git sends the ref-update command list (pkt-lines ending in a flush),
-    //    then the complete PACK blob only when a command has a non-zero new-oid.
-    //    A delete-only push sends no pack, and git holds its write pipe open for
-    //    the report-status response, so EOF cannot delimit the request (#369).
+    //    Git sends ref-update commands + the complete PACK blob, then closes its
+    //    write pipe. read_to_end is safe and correct here, a single POST.
 
     let post_url = format!("{}/{}", repo_base, service);
 
@@ -298,7 +296,10 @@ fn handle_connect<R: Read>(
         return negotiate_upload_pack(&client, &post_url, service, signing_key, stdin, &mut stdout);
     }
 
-    let request_body = read_receive_pack_request(stdin)?;
+    let mut request_body = Vec::new();
+    stdin
+        .read_to_end(&mut request_body)
+        .context("reading receive-pack request")?;
     tracing::debug!("pack request: {} bytes from git", request_body.len());
 
     if request_body.is_empty() {
@@ -534,108 +535,6 @@ fn read_upload_pack_round<R: Read>(stdin: &mut R) -> Result<(Vec<u8>, RoundEnd)>
     }
 }
 
-/// Read one git-receive-pack request: optional `shallow` lines, then EITHER a
-/// command list (pkt-lines terminated by a flush) or a push-cert block
-/// (pkt-lines terminated by a "push-cert-end" line), then the negotiated
-/// push-options block, then the pack ONLY when a command carries a non-zero
-/// new-oid.
-///
-/// A delete-only push sends no pack, and git holds its write pipe open waiting
-/// for the report-status response, so a plain read_to_end deadlocks the two
-/// (#369). A signed push is the other trap: when the client sends a push-cert
-/// it sends NO command list at all (the commands live inside the cert), so the
-/// request starts with a "push-cert" line and no flush ever arrives. The
-/// negotiated capability list is on the push-cert line, or on the first
-/// command line after its NUL when there is no cert. EOF at any point also
-/// ends the read, covering git versions that close the pipe early.
-fn read_receive_pack_request<R: Read>(stdin: &mut R) -> Result<Vec<u8>> {
-    /// Read one pkt-line into `body` (length prefix + data), returning its data
-    /// or `None` on flush or EOF. Malformed lengths bail rather than collapse
-    /// into a synthesized flush (#192 F5).
-    fn read_pkt<R: Read>(stdin: &mut R, body: &mut Vec<u8>) -> Result<Option<Vec<u8>>> {
-        let mut len_bytes = [0u8; 4];
-        match stdin.read_exact(&mut len_bytes) {
-            Ok(()) => {}
-            Err(e) if e.kind() == io::ErrorKind::UnexpectedEof => return Ok(None),
-            Err(e) => return Err(e.into()),
-        }
-        let Ok(len_hex) = std::str::from_utf8(&len_bytes) else {
-            bail!("malformed pkt-line length prefix: non-UTF-8 bytes {len_bytes:02x?}");
-        };
-        let Ok(pkt_len) = usize::from_str_radix(len_hex, 16) else {
-            bail!("malformed pkt-line length prefix: non-hex {len_hex:?}");
-        };
-        body.extend_from_slice(&len_bytes);
-        if pkt_len == 0 {
-            return Ok(None);
-        }
-        if pkt_len < 4 {
-            bail!("invalid pkt-line length: {pkt_len}");
-        }
-        let mut data = vec![0u8; pkt_len - 4];
-        stdin
-            .read_exact(&mut data)
-            .context("reading pkt-line data")?;
-        body.extend_from_slice(&data);
-        Ok(Some(data))
-    }
-
-    /// "<40-hex old> <40-hex new> <refname>" — the command shape, whether it
-    /// appears in a command list or inside a push-cert.
-    fn is_command(data: &[u8]) -> bool {
-        data.len() >= 82
-            && data[..40].iter().all(|b| b.is_ascii_hexdigit())
-            && data[40] == b' '
-            && data[41..81].iter().all(|b| b.is_ascii_hexdigit())
-            && data[81] == b' '
-    }
-
-    let mut body = Vec::new();
-    let mut caps = String::new();
-    let mut pack_expected = false;
-    let mut in_cert = false;
-    loop {
-        let Some(data) = read_pkt(stdin, &mut body)? else {
-            // Flush ends a command list; EOF ends anything. A cert has no
-            // legal flush inside it, so the same break covers malformed
-            // input without hanging.
-            break;
-        };
-        if !in_cert && data.starts_with(b"push-cert") {
-            in_cert = true;
-            if let Some(nul) = data.iter().position(|&b| b == 0) {
-                caps = String::from_utf8_lossy(&data[nul + 1..]).into_owned();
-            }
-            continue;
-        }
-        if in_cert && data == b"push-cert-end\n" {
-            break;
-        }
-        if is_command(&data) {
-            if caps.is_empty() {
-                if let Some(nul) = data.iter().position(|&b| b == 0) {
-                    caps = String::from_utf8_lossy(&data[nul + 1..]).into_owned();
-                }
-            }
-            if data[41..81].iter().any(|&b| b != b'0') {
-                pack_expected = true;
-            }
-        }
-    }
-    // When both are negotiated the push-options block follows the cert; when
-    // only options are negotiated it follows the command flush. Either way it
-    // is pkt-lines to a flush.
-    if caps.split(' ').any(|c| c.starts_with("push-options")) {
-        while read_pkt(stdin, &mut body)?.is_some() {}
-    }
-    if pack_expected {
-        stdin
-            .read_to_end(&mut body)
-            .context("reading receive-pack pack")?;
-    }
-    Ok(body)
-}
-
 /// POST one self-contained stateless-RPC request body and stream the response to
 /// `stdout`. Signs the request when `signing_key` is present, so every round of a
 /// private-repo fetch carries the caller's signature (the node visibility-gates
@@ -2118,184 +2017,6 @@ mod tests {
         assert!(help.ends_with('\n'));
     }
 
-    // ── #369 delete-only push deadlock ───────────────────────────────────────
-
-    /// A delete-only receive-pack request ends at the command flush; git then
-    /// holds the write pipe open for the report and sends no pack. A reader that
-    /// blocks after the command list reproduces the hang that a Cursor (EOF)
-    /// hides. The read must return at the flush, inside the timeout.
-    #[test]
-    fn delete_only_receive_pack_request_returns_at_command_flush() {
-        let old = "a".repeat(40);
-        let zero = "0".repeat(40);
-        let mut seed = Vec::new();
-        seed.extend_from_slice(&pkt(&format!("{old} {zero} refs/heads/gone\n")));
-        seed.extend_from_slice(b"0000"); // command-list flush; git then blocks
-
-        let (tx, rx) = std::sync::mpsc::channel::<()>();
-        let mut reader = BlockAfterSeed {
-            seed: io::Cursor::new(seed),
-            gate: rx,
-        };
-
-        let (done_tx, done_rx) = std::sync::mpsc::channel::<Vec<u8>>();
-        let handle = std::thread::spawn(move || {
-            let _ = done_tx.send(read_receive_pack_request(&mut reader).unwrap());
-        });
-
-        let got = done_rx.recv_timeout(std::time::Duration::from_secs(2));
-        let _ = tx.send(()); // unblock the reader so the thread can exit
-        let _ = handle.join();
-
-        let body = got.expect(
-            "read_receive_pack_request blocked past the command flush waiting for \
-             a pack a delete-only push never sends (#369)",
-        );
-        assert!(
-            body.ends_with(b"0000"),
-            "the request body keeps the command list and its flush terminator"
-        );
-    }
-
-    /// A push carrying objects still reads the pack through to EOF, and the
-    /// command flush sits between the commands and the pack in the body.
-    #[test]
-    fn receive_pack_request_with_pack_reads_through_eof() {
-        let old = "a".repeat(40);
-        let new = "b".repeat(40);
-        let mut stream = Vec::new();
-        stream.extend_from_slice(&pkt(&format!(
-            "{old} {new} refs/heads/main\0 report-status-v2\n"
-        )));
-        stream.extend_from_slice(b"0000");
-        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
-
-        let mut stdin = io::Cursor::new(stream);
-        let body = read_receive_pack_request(&mut stdin).unwrap();
-        let pos = body.windows(4).position(|w| w == b"0000").unwrap();
-        assert_eq!(
-            &body[pos + 4..],
-            b"PACK\x00\x00\x00\x02fakepack",
-            "a non-zero new-oid means the pack follows the flush"
-        );
-    }
-
-    /// A signed delete-only push sends NO command list: the request starts
-    /// with the "push-cert" line, the commands live inside the cert, and the
-    /// cert ends with a "push-cert-end" pkt-line (not a flush). The reader
-    /// must return after push-cert-end without waiting on the pipe.
-    #[test]
-    fn signed_delete_only_push_returns_after_push_cert_end() {
-        let old = "a".repeat(40);
-        let zero = "0".repeat(40);
-        let mut seed = Vec::new();
-        seed.extend_from_slice(&pkt("push-cert\0 report-status-v2 push-cert\n"));
-        seed.extend_from_slice(&pkt("certificate version 0.1\n"));
-        seed.extend_from_slice(&pkt("pusher did:key:z6Mk\n"));
-        seed.extend_from_slice(&pkt("\n")); // empty line ends the headers
-        seed.extend_from_slice(&pkt(&format!("{old} {zero} refs/heads/gone\n")));
-        seed.extend_from_slice(&pkt("sig-line\n"));
-        seed.extend_from_slice(&pkt("push-cert-end\n"));
-        // git now blocks for the report; the pipe stays open.
-
-        let (tx, rx) = std::sync::mpsc::channel::<()>();
-        let mut reader = BlockAfterSeed {
-            seed: io::Cursor::new(seed),
-            gate: rx,
-        };
-
-        let (done_tx, done_rx) = std::sync::mpsc::channel::<Vec<u8>>();
-        let handle = std::thread::spawn(move || {
-            let _ = done_tx.send(read_receive_pack_request(&mut reader).unwrap());
-        });
-        let got = done_rx.recv_timeout(std::time::Duration::from_secs(2));
-        let _ = tx.send(());
-        let _ = handle.join();
-
-        let body = got.expect(
-            "read_receive_pack_request blocked past push-cert-end on a signed \
-             delete-only push (#369)",
-        );
-        assert!(
-            body.windows(6).any(|w| w == b"pusher"),
-            "the cert block is part of the request body"
-        );
-    }
-
-    /// A signed pack-bearing push: cert with the commands inside, then the
-    /// pack. Everything must land in the body.
-    #[test]
-    fn signed_pack_push_keeps_cert_and_pack() {
-        let old = "a".repeat(40);
-        let new = "b".repeat(40);
-        let mut stream = Vec::new();
-        stream.extend_from_slice(&pkt("push-cert\0 report-status-v2 push-cert\n"));
-        stream.extend_from_slice(&pkt("certificate version 0.1\n"));
-        stream.extend_from_slice(&pkt("\n"));
-        stream.extend_from_slice(&pkt(&format!("{old} {new} refs/heads/main\n")));
-        stream.extend_from_slice(&pkt("push-cert-end\n"));
-        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
-
-        let mut stdin = io::Cursor::new(stream);
-        let body = read_receive_pack_request(&mut stdin).unwrap();
-        assert!(
-            body.windows(5).any(|w| w == b"PACK\x00"),
-            "the pack follows the cert into the body"
-        );
-    }
-
-    /// A negotiated push-options block is pkt-lines ending in its own flush.
-    /// With no cert it follows the command flush; with a cert it follows
-    /// push-cert-end.
-    #[test]
-    fn receive_pack_request_consumes_a_push_options_block() {
-        let old = "a".repeat(40);
-        let new = "b".repeat(40);
-        let mut stream = Vec::new();
-        stream.extend_from_slice(&pkt(&format!(
-            "{old} {new} refs/heads/main\0 report-status-v2 push-options\n"
-        )));
-        stream.extend_from_slice(b"0000"); // command flush
-        stream.extend_from_slice(&pkt("ci.skip\n"));
-        stream.extend_from_slice(b"0000"); // options flush
-        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
-
-        let mut stdin = io::Cursor::new(stream);
-        let body = read_receive_pack_request(&mut stdin).unwrap();
-        assert!(body.windows(7).any(|w| w == b"ci.skip"), "options survived");
-        assert!(
-            body.ends_with(b"PACK\x00\x00\x00\x02fakepack"),
-            "the pack lands after the options block"
-        );
-    }
-
-    /// Cert and options together: the cert comes first and the options block
-    /// sits between push-cert-end and the pack.
-    #[test]
-    fn signed_push_with_options_reads_cert_then_options_then_pack() {
-        let old = "a".repeat(40);
-        let new = "b".repeat(40);
-        let mut stream = Vec::new();
-        stream.extend_from_slice(&pkt(
-            "push-cert\0 report-status-v2 push-cert push-options\n",
-        ));
-        stream.extend_from_slice(&pkt("certificate version 0.1\n"));
-        stream.extend_from_slice(&pkt("\n"));
-        stream.extend_from_slice(&pkt(&format!("{old} {new} refs/heads/main\n")));
-        stream.extend_from_slice(&pkt("push-cert-end\n"));
-        stream.extend_from_slice(&pkt("ci.skip\n"));
-        stream.extend_from_slice(b"0000"); // options flush
-        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
-
-        let mut stdin = io::Cursor::new(stream);
-        let body = read_receive_pack_request(&mut stdin).unwrap();
-        assert!(body.windows(7).any(|w| w == b"ci.skip"), "options survived");
-        assert!(
-            body.ends_with(b"PACK\x00\x00\x00\x02fakepack"),
-            "the pack lands after the options block"
-        );
-    }
-
     // ── #117 multi-round fetch negotiation ───────────────────────────────────
 
     /// Encode a git pkt-line: 4-byte hex length (incl. the 4 bytes) + data.

crates/git-remote-gitlawb/tests/real_git_fetch.rs

@@ -95,36 +95,6 @@ fn upload_pack(repo: &Path, advertise: bool, input: &[u8]) -> Vec<u8> {
     out.stdout
 }
 
-/// Run `git receive-pack --stateless-rpc [--advertise-refs] <repo>`, optionally
-/// feeding `input` on stdin. Returns raw stdout (the wire bytes). The receive
-/// half of `upload_pack`, used so the shim can serve a real push.
-fn receive_pack(repo: &Path, advertise: bool, input: &[u8]) -> Vec<u8> {
-    let mut cmd = Command::new("git");
-    cmd.arg("receive-pack").arg("--stateless-rpc");
-    if advertise {
-        cmd.arg("--advertise-refs");
-    }
-    cmd.arg(repo)
-        .stdin(Stdio::piped())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped());
-    let mut child = cmd.spawn().expect("spawn git receive-pack");
-    let mut stdin = child.stdin.take().unwrap();
-    let input = input.to_vec();
-    let writer = std::thread::spawn(move || {
-        let _ = stdin.write_all(&input);
-        // drop closes stdin
-    });
-    let out = child.wait_with_output().expect("wait receive-pack");
-    writer.join().ok();
-    assert!(
-        out.status.success(),
-        "git receive-pack failed: {}",
-        String::from_utf8_lossy(&out.stderr)
-    );
-    out.stdout
-}
-
 #[derive(Clone, Copy)]
 enum ShimMode {
     /// Faithful v0 serving: pipe each POST to `git upload-pack --stateless-rpc`.
@@ -248,21 +218,7 @@ fn handle_conn(stream: TcpStream, repo: &Path, mode: ShimMode, posts: &AtomicUsi
         reader.read_exact(&mut body).ok();
     }
 
-    let (content_type, payload) = if method == "GET" && target.contains("service=git-receive-pack")
-    {
-        // Receive-pack advertisement, same wrapper shape as upload-pack.
-        let adv = receive_pack(repo, true, b"");
-        let mut wrapped = pkt(b"# service=git-receive-pack\n");
-        wrapped.extend_from_slice(b"0000");
-        wrapped.extend_from_slice(&adv);
-        ("application/x-git-receive-pack-advertisement", wrapped)
-    } else if method == "POST" && target.ends_with("/git-receive-pack") {
-        posts.fetch_add(1, Ordering::SeqCst);
-        (
-            "application/x-git-receive-pack-result",
-            receive_pack(repo, false, &body),
-        )
-    } else if method == "GET" && target.contains("/info/refs") {
+    let (content_type, payload) = if method == "GET" && target.contains("/info/refs") {
         // v0 advertisement, wrapped exactly as the node's info_refs does.
         let adv = upload_pack(repo, true, b"");
         let mut wrapped = pkt(b"# service=git-upload-pack\n");
@@ -337,34 +293,6 @@ fn fetch_with_helper(clone: &Path, node_url: &str) -> (bool, std::process::Outpu
     run_bounded(cmd, Duration::from_secs(30))
 }
 
-/// Run `git push` in `local` through the helper, same wiring and hard timeout as
-/// [`fetch_with_helper`]. `args` carries the push spec, e.g.
-/// `["--delete", "doomed"]`.
-fn push_with_helper(local: &Path, node_url: &str, args: &[&str]) -> (bool, std::process::Output) {
-    let helper_bin = PathBuf::from(env!("CARGO_BIN_EXE_git-remote-gitlawb"));
-    let helper_dir = helper_bin.parent().unwrap().to_path_buf();
-    let path_env = match std::env::var_os("PATH") {
-        Some(p) => {
-            let mut dirs = vec![helper_dir.clone()];
-            dirs.extend(std::env::split_paths(&p));
-            std::env::join_paths(dirs).unwrap()
-        }
-        None => helper_dir.clone().into_os_string(),
-    };
-
-    let mut cmd = Command::new("git");
-    cmd.arg("-C")
-        .arg(local)
-        .arg("push")
-        .arg("origin")
-        .args(args)
-        .env("PATH", path_env)
-        .env("LC_ALL", "C")
-        .env("GITLAWB_NODE", node_url)
-        .env("GITLAWB_KEY", "/nonexistent-key-for-anon-push");
-    run_bounded(cmd, Duration::from_secs(30))
-}
-
 /// A synthetic non-success `ExitStatus` used only on the unix timeout path, where
 /// `reap_group` has already consumed the child and the later `wait` returns ECHILD.
 /// The timeout branch already reports `completed == false`, so the exact status is
@@ -1314,77 +1242,3 @@ fn run_bounded_bounds_join_when_leader_exits_leaving_a_pipe_holder() {
          held pipes do not stall the joins)"
     );
 }
-
-/// #369 end-to-end: a real `git push --delete` through the helper completes.
-/// The receive-pack request for a delete-only push is commands + flush with NO
-/// pack, and git holds its write pipe open for the report — so a helper that
-/// reads stdin to EOF deadlocks against it. The shim pipes the POST to a real
-/// `git receive-pack --stateless-rpc`, so the branch actually has to be deleted
-/// server-side for the assertions to pass.
-#[test]
-fn real_git_push_delete_completes() {
-    let dir = tempfile::tempdir().unwrap();
-    let server = dir.path().join("server.git");
-    let local = dir.path().join("local");
-
-    // A bare repo holding the doomed branch, plus a live one.
-    git(
-        dir.path(),
-        &[
-            "init",
-            "--bare",
-            server.to_str().unwrap(),
-            "--initial-branch=main",
-        ],
-    );
-    let work = dir.path().join("work");
-    git(
-        dir.path(),
-        &["init", work.to_str().unwrap(), "--initial-branch=main"],
-    );
-    git(&work, &["config", "user.email", "t@t"]);
-    git(&work, &["config", "user.name", "t"]);
-    std::fs::write(work.join("f"), "x").unwrap();
-    git(&work, &["add", "f"]);
-    git(&work, &["commit", "-m", "c"]);
-    git(&work, &["branch", "doomed"]);
-    git(&work, &["push", server.to_str().unwrap(), "main", "doomed"]);
-
-    // The pusher: a local repo whose origin is the gitlawb remote pointing at
-    // the shim. Nothing needs to be fetched first; git takes the old-oid from
-    // the receive-pack advertisement.
-    git(
-        dir.path(),
-        &["init", local.to_str().unwrap(), "--initial-branch=main"],
-    );
-    git(
-        &local,
-        &["remote", "add", "origin", "gitlawb://did:key:z6MkTest/repo"],
-    );
-
-    let shim = start_shim(server.clone(), ShimMode::Normal);
-    let (completed, out) = push_with_helper(&local, &shim.base_url, &["--delete", "doomed"]);
-    let posts = shim.posts.load(Ordering::SeqCst);
-
-    assert!(
-        completed,
-        "git push --delete did not complete within the timeout (deadlock signature). stderr:\n{}",
-        String::from_utf8_lossy(&out.stderr)
-    );
-    assert!(
-        out.status.success(),
-        "git push --delete failed. stderr:\n{}",
-        String::from_utf8_lossy(&out.stderr)
-    );
-    assert_eq!(posts, 1, "a delete-only push is exactly one POST");
-    let gone = Command::new("git")
-        .arg("-C")
-        .arg(&server)
-        .args(["rev-parse", "--verify", "--quiet", "refs/heads/doomed"])
-        .output()
-        .unwrap();
-    assert!(
-        !gone.status.success(),
-        "refs/heads/doomed must be deleted in the server repo"
-    );
-}

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

@@ -449,3 +449,198 @@ mod lock_pool_shed_tests {
         );
     }
 }
+
+/// #426: end-to-end at the handler seam. A corrupt object store behind a
+/// resolving issue ref used to look identical to an absent issue: get returned
+/// a fake 404, close returned a fake 404 after releasing the write guard, and
+/// list silently under-reported. All three must now surface a git error.
+#[cfg(test)]
+mod corrupt_store_tests {
+    use super::*;
+    use axum::response::IntoResponse;
+    use sqlx::PgPool;
+    use tempfile::TempDir;
+
+    fn seed_repo(owner_did: &str, name: &str) -> crate::db::RepoRecord {
+        let now = Utc::now();
+        crate::db::RepoRecord {
+            id: Uuid::new_v4().to_string(),
+            name: name.to_string(),
+            owner_did: owner_did.to_string(),
+            description: None,
+            is_public: true,
+            default_branch: "main".to_string(),
+            created_at: now,
+            updated_at: now,
+            disk_path: format!("/tmp/{name}"),
+            forked_from: None,
+            machine_id: None,
+        }
+    }
+
+    /// A state whose repo store roots in `repos_dir`, so the test can lay down
+    /// a real repo and then corrupt it without touching /tmp.
+    async fn state_with_repos_dir(pool: &PgPool, repos_dir: &std::path::Path) -> AppState {
+        let mut state = crate::test_support::test_state(pool.clone()).await;
+        state.repo_store =
+            crate::git::repo_store::RepoStore::for_testing(repos_dir.to_path_buf(), pool.clone());
+        state
+    }
+
+    /// Bare repo at the store's expected path, holding one issue ref.
+    fn bare_repo_with_issue(
+        repos_dir: &std::path::Path,
+        owner: &str,
+        name: &str,
+    ) -> (std::path::PathBuf, String) {
+        let repo_path = repos_dir
+            .join(owner.replace([':', '/'], "_"))
+            .join(format!("{name}.git"));
+        std::fs::create_dir_all(&repo_path).unwrap();
+        let out = std::process::Command::new("git")
+            .args(["init", "--bare"])
+            .arg(&repo_path)
+            .output()
+            .unwrap();
+        assert!(out.status.success());
+        let issue_id = "aaaabbbb-0000-0000-0000-000000000000".to_string();
+        git_issues::create_issue(
+            &repo_path,
+            &issue_id,
+            &serde_json::json!({
+                "id": issue_id,
+                "title": "t",
+                "status": "open",
+            })
+            .to_string(),
+        )
+        .unwrap();
+        (repo_path, issue_id)
+    }
+
+    /// Delete the blob object behind the issue ref and put garbage in its
+    /// place: `cat-file` fails while `rev-parse` still resolves the ref.
+    fn corrupt_issue_object(repo_path: &std::path::Path, issue_id: &str) {
+        let out = std::process::Command::new("git")
+            .args(["rev-parse", &format!("refs/gitlawb/issues/{issue_id}")])
+            .current_dir(repo_path)
+            .output()
+            .unwrap();
+        let oid = String::from_utf8(out.stdout).unwrap().trim().to_string();
+        let obj = repo_path.join("objects").join(&oid[..2]).join(&oid[2..]);
+        std::fs::remove_file(&obj).unwrap();
+        std::fs::write(&obj, b"garbage").unwrap();
+    }
+
+    /// The regression: closing over a corrupt store used to return a fake 404
+    /// (or panic deeper in). It must reach the caller as a git 500, and the
+    /// handler must return rather than unwind.
+    #[sqlx::test]
+    async fn close_issue_on_corrupt_store_returns_git_error_not_panic(pool: PgPool) {
+        let owner = "did:key:zISSUECORRUPTCCCCCCCCCCCCCCCCCCCCCCCCC";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "corrupt-close"))
+            .await
+            .expect("seed repo");
+        let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-close");
+        corrupt_issue_object(&repo_path, &issue_id);
+
+        let err = close_issue(
+            State(state),
+            Extension(AuthenticatedDid(owner.to_string())),
+            Path((owner.to_string(), "corrupt-close".to_string(), issue_id)),
+        )
+        .await
+        .expect_err("a corrupt store must error, not panic or 404");
+        assert_eq!(
+            err.into_response().status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "a read failure is a git 500, not a fake not-found"
+        );
+    }
+
+    /// get on a corrupt store: 500, not the 404 the fold used to produce.
+    #[sqlx::test]
+    async fn get_issue_on_corrupt_store_returns_500_not_404(pool: PgPool) {
+        let owner = "did:key:zISSUECORRUPTGETDDDDDDDDDDDDDDDDDDDDDDD";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "corrupt-get"))
+            .await
+            .expect("seed repo");
+        let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-get");
+        corrupt_issue_object(&repo_path, &issue_id);
+
+        let err = get_issue(
+            State(state),
+            Path((owner.to_string(), "corrupt-get".to_string(), issue_id)),
+            None,
+        )
+        .await
+        .expect_err("a corrupt store must error, not 404");
+        assert_eq!(
+            err.into_response().status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+        );
+    }
+
+    /// list on a corrupt store must fail outright; a 200 that silently drops
+    /// the unreadable issue is the other half of the fold.
+    #[sqlx::test]
+    async fn list_issues_on_corrupt_store_returns_git_error(pool: PgPool) {
+        let owner = "did:key:zISSUECORRUPTLISTEEEEEEEEEEEEEEEEEEEEEE";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "corrupt-list"))
+            .await
+            .expect("seed repo");
+        let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-list");
+        corrupt_issue_object(&repo_path, &issue_id);
+
+        let err = list_issues(
+            State(state),
+            Path((owner.to_string(), "corrupt-list".to_string())),
+            None,
+        )
+        .await
+        .expect_err("a corrupt store must error, not under-report");
+        assert_eq!(
+            err.into_response().status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+        );
+    }
+
+    /// Must-not direction: a genuinely absent issue still gets the 404.
+    #[sqlx::test]
+    async fn get_issue_on_absent_ref_still_returns_404(pool: PgPool) {
+        let owner = "did:key:zISSUEABSENTFFFFFFFFFFFFFFFFFFFFFFFFFFF";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "absent-get"))
+            .await
+            .expect("seed repo");
+        bare_repo_with_issue(dir.path(), owner, "absent-get");
+
+        let err = get_issue(
+            State(state),
+            Path((
+                owner.to_string(),
+                "absent-get".to_string(),
+                "ffffffff-0000-0000-0000-000000000000".to_string(),
+            )),
+            None,
+        )
+        .await
+        .expect_err("a missing issue is still not-found");
+        assert_eq!(err.into_response().status(), StatusCode::NOT_FOUND);
+    }
+}

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

@@ -61,6 +61,119 @@ pub fn create_issue(repo_path: &Path, issue_id: &str, json: &str) -> Result<()>
     Ok(())
 }
 
+/// Issue ids are UUIDs; refs under `refs/gitlawb/issues/` only ever carry
+/// `[0-9a-zA-Z_-]` names. Anything else is not a value we pass to git.
+fn issue_ref_name_is_safe(ref_name: &str) -> bool {
+    let Some(id) = ref_name.strip_prefix("refs/gitlawb/issues/") else {
+        return false;
+    };
+    !id.is_empty()
+        && id
+            .chars()
+            .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_')
+}
+
+/// Resolve a ref name without putting it on a command line: enumerate refs
+/// (constant argv) and match in memory. A nonzero exit propagates, so "cannot
+/// determine" is an error rather than a quiet `false`.
+fn ref_name_resolves(repo_path: &Path, ref_name: &str) -> Result<bool> {
+    // allow-unbounded-git: failure-path existence recheck, module convention.
+    let out = Command::new("git")
+        .args(["for-each-ref", "--format=%(refname)"])
+        .current_dir(repo_path)
+        .output()
+        .context("failed to run git for-each-ref")?;
+    if !out.status.success() {
+        let stderr = String::from_utf8_lossy(&out.stderr);
+        anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
+    }
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    Ok(stdout.lines().any(|line| line.trim() == ref_name))
+}
+
+/// Read the blob behind an issue ref.
+///
+/// `Ok(None)` means the ref itself is absent. A ref that resolves but whose
+/// object cannot be read (corrupt object, unreadable store, a mid-read gc) is
+/// an error, not an absence (#426).
+///
+/// The ref name reaches git on stdin, never argv: `cat-file --batch` answers
+/// `<oid> <type> <size>\n<body>\n` for a readable object and
+/// `<spec> missing\n` for an absent ref or an unreadable object alike, so the
+/// `missing` case is re-resolved by name enumeration to tell them apart.
+fn read_issue_blob(repo_path: &Path, ref_name: &str) -> Result<Option<String>> {
+    if !issue_ref_name_is_safe(ref_name) {
+        anyhow::bail!("refusing issue ref outside the issues namespace: {ref_name}");
+    }
+
+    // allow-unbounded-git: this module's spawns predate the bounded runner and
+    // its callers run inside issue handlers that already hold the repo guard.
+    let mut child = Command::new("git")
+        .args(["cat-file", "--batch"])
+        .current_dir(repo_path)
+        .stdin(std::process::Stdio::piped())
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped())
+        .spawn()
+        .context("failed to spawn git cat-file --batch")?;
+
+    use std::io::Write;
+    // Reap the child even when the write fails so a stdin error cannot drop an
+    // unwaited Child and leak a zombie (#53).
+    let write_result = match child.stdin.take() {
+        Some(mut stdin) => stdin
+            .write_all(ref_name.as_bytes())
+            .and_then(|()| stdin.write_all(b"\n")),
+        None => Err(std::io::Error::new(
+            std::io::ErrorKind::BrokenPipe,
+            "git cat-file --batch stdin unavailable",
+        )),
+    };
+    let output = child
+        .wait_with_output()
+        .context("git cat-file --batch failed")?;
+    write_result.context("failed to write ref name to git cat-file stdin")?;
+
+    if !output.status.success() {
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        anyhow::bail!("git cat-file --batch failed: {}", stderr.trim());
+    }
+
+    // "<spec> missing\n" covers an absent ref and an unreadable object; only a
+    // ref that still resolves is the error case.
+    let missing = format!("{ref_name} missing\n");
+    if output.stdout.as_slice() == missing.as_bytes() {
+        return match ref_name_resolves(repo_path, ref_name)? {
+            true => anyhow::bail!("issue ref resolves but its object is unreadable: {ref_name}"),
+            false => Ok(None),
+        };
+    }
+
+    // Header: "<oid> <type> <size>\n" then exactly <size> bytes plus a newline.
+    let header_end = output
+        .stdout
+        .iter()
+        .position(|b| *b == b'\n')
+        .context("unexpected git cat-file --batch output")?;
+    let header = String::from_utf8_lossy(&output.stdout[..header_end]);
+    let mut parts = header.split(' ');
+    parts.next();
+    let obj_type = parts.next().unwrap_or("");
+    if obj_type != "blob" {
+        anyhow::bail!("issue ref {ref_name} resolves to a non-blob object");
+    }
+    let size: usize = parts
+        .next()
+        .and_then(|s| s.parse().ok())
+        .context("unexpected git cat-file --batch header")?;
+    let body = &output.stdout[header_end + 1..];
+    anyhow::ensure!(
+        body.len() >= size,
+        "truncated git cat-file --batch body for {ref_name}"
+    );
+    Ok(Some(String::from_utf8_lossy(&body[..size]).to_string()))
+}
+
 /// 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/
@@ -74,9 +187,11 @@ pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
         .output()
         .context("failed to run git for-each-ref")?;
 
+    // for-each-ref exits 0 with empty output when no refs match, so a nonzero
+    // exit is a real enumeration failure, not "no issues yet".
     if !list_output.status.success() {
-        // No issues yet
-        return Ok(vec![]);
+        let stderr = String::from_utf8_lossy(&list_output.stderr);
+        anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
     }
 
     let refs_str = String::from_utf8_lossy(&list_output.stdout);
@@ -88,16 +203,12 @@ pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
             continue;
         }
 
-        // Read the blob content
-        let cat_output = Command::new("git")
-            .args(["cat-file", "blob", ref_name])
-            .current_dir(repo_path)
-            .output()
-            .context("failed to run git cat-file")?;
-
-        if cat_output.status.success() {
-            let content = String::from_utf8_lossy(&cat_output.stdout).to_string();
-            issues.push(content);
+        // Read the blob content. A ref that vanished between for-each-ref and
+        // here is skipped; a ref that resolves but fails to read propagates so
+        // a degraded object store errors the listing instead of under-reporting.
+        match read_issue_blob(repo_path, ref_name)? {
+            Some(content) => issues.push(content),
+            None => continue,
         }
     }
 
@@ -127,8 +238,12 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result<Option<S
         .output()
         .context("failed to run git for-each-ref")?;
 
+    // for-each-ref exits 0 with empty output when nothing matches, so a
+    // nonzero exit is a real enumeration failure (e.g. not a repo), not
+    // "issue not found".
     if !list.status.success() {
-        return Ok(None);
+        let stderr = String::from_utf8_lossy(&list.stderr);
+        anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
     }
 
     let output = String::from_utf8_lossy(&list.stdout);
@@ -161,8 +276,12 @@ pub fn close_issue(repo_path: &Path, issue_id: &str) -> Result<Option<String>> {
         None => return Ok(None),
     };
 
-    let raw = get_issue(repo_path, &full_id)?
-        .expect("ref existed in resolve but not in get — should be impossible");
+    // A ref that resolved and is now absent was deleted between the two calls;
+    // any other read failure propagates as an error rather than panic (#426).
+    let raw = match get_issue(repo_path, &full_id)? {
+        Some(raw) => raw,
+        None => return Ok(None),
+    };
 
     let mut issue: serde_json::Value =
         serde_json::from_str(&raw).context("invalid issue JSON in git ref")?;
@@ -181,18 +300,7 @@ pub fn get_issue(repo_path: &Path, issue_id: &str) -> Result<Option<String>> {
     };
 
     let ref_name = format!("refs/gitlawb/issues/{full_id}");
-    let cat_output = Command::new("git")
-        .args(["cat-file", "blob", &ref_name])
-        .current_dir(repo_path)
-        .output()
-        .context("failed to run git cat-file")?;
-
-    if !cat_output.status.success() {
-        return Ok(None);
-    }
-
-    let content = String::from_utf8_lossy(&cat_output.stdout).to_string();
-    Ok(Some(content))
+    read_issue_blob(repo_path, &ref_name)
 }
 
 #[cfg(test)]
@@ -294,4 +402,109 @@ mod tests {
         let v: serde_json::Value = serde_json::from_str(&updated).unwrap();
         assert_eq!(v["status"], "closed");
     }
+
+    /// Overwrite the loose object behind an issue ref with garbage, so
+    /// `cat-file` fails while `rev-parse` still resolves the ref (#426).
+    fn corrupt_issue_object(dir: &TempDir, full_id: &str) {
+        let oid_out = Command::new("git")
+            .args(["rev-parse", &format!("refs/gitlawb/issues/{full_id}")])
+            .current_dir(dir.path())
+            .output()
+            .unwrap();
+        let oid = String::from_utf8(oid_out.stdout)
+            .unwrap()
+            .trim()
+            .to_string();
+        let obj = dir
+            .path()
+            .join(".git/objects")
+            .join(&oid[..2])
+            .join(&oid[2..]);
+        std::fs::remove_file(&obj).unwrap();
+        std::fs::write(&obj, b"garbage").unwrap();
+    }
+
+    #[test]
+    fn test_get_and_close_missing_issue_return_none() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        assert_eq!(get_issue(dir.path(), "nosuch00").unwrap(), None);
+        assert_eq!(close_issue(dir.path(), "nosuch00").unwrap(), None);
+    }
+
+    // #426: a directory that is not a repo makes `cat-file --batch` exit
+    // nonzero; that failure is an error, not a quiet None.
+    #[test]
+    fn test_issue_read_on_non_repo_errors_instead_of_none() {
+        let dir = TempDir::new().unwrap();
+        let not_a_repo = dir.path().join("not-a-repo");
+        std::fs::create_dir_all(&not_a_repo).unwrap();
+        assert!(read_issue_blob(&not_a_repo, "refs/gitlawb/issues/deadbeef").is_err());
+    }
+
+    // #426: the ref name is constrained to the issues namespace and a safe
+    // charset before it is fed to git, even on stdin (a newline would smuggle
+    // a second `--batch` spec).
+    #[test]
+    fn test_issue_read_rejects_ref_outside_issues_namespace() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        assert!(read_issue_blob(dir.path(), "refs/heads/main").is_err());
+        assert!(read_issue_blob(dir.path(), "refs/gitlawb/issues/../x").is_err());
+    }
+
+    // #426: a failed for-each-ref enumeration is an error, not an empty list.
+    #[test]
+    fn test_list_issues_on_non_repo_errors_instead_of_empty() {
+        let dir = TempDir::new().unwrap();
+        let not_a_repo = dir.path().join("not-a-repo");
+        std::fs::create_dir_all(&not_a_repo).unwrap();
+        assert!(list_issues(&not_a_repo).is_err());
+    }
+
+    // #426: the same enumeration failure inside resolve_issue_id must reach
+    // get_issue/close_issue as an error, not a "not found" None.
+    #[test]
+    fn test_get_and_close_on_non_repo_error_instead_of_none() {
+        let dir = TempDir::new().unwrap();
+        let not_a_repo = dir.path().join("not-a-repo");
+        std::fs::create_dir_all(&not_a_repo).unwrap();
+        assert!(get_issue(&not_a_repo, "nosuch00").is_err());
+        assert!(close_issue(&not_a_repo, "nosuch00").is_err());
+    }
+
+    // #426: a ref that resolves but whose object is corrupt is a read error,
+    // not an absent issue. `close_issue` used to `expect()` on that None and
+    // panic inside a request handler holding the write guard.
+    #[test]
+    fn test_corrupt_issue_object_errors_instead_of_panicking() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        let full_id = "bad00000-0000-0000-0000-000000000000";
+        create_issue(dir.path(), full_id, r#"{"status":"open"}"#).unwrap();
+        corrupt_issue_object(&dir, full_id);
+
+        assert!(get_issue(dir.path(), full_id).is_err());
+        assert!(close_issue(dir.path(), full_id).is_err());
+        assert!(list_issues(dir.path()).is_err());
+    }
+
+    #[test]
+    fn test_list_issues_returns_all() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        create_issue(
+            dir.path(),
+            "aaa00000-0000-0000-0000-000000000000",
+            r#"{"status":"open"}"#,
+        )
+        .unwrap();
+        create_issue(
+            dir.path(),
+            "bbb00000-0000-0000-0000-000000000000",
+            r#"{"status":"open"}"#,
+        )
+        .unwrap();
+        assert_eq!(list_issues(dir.path()).unwrap().len(), 2);
+    }
 }