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/gitlawb-node/src/api/changelog.rs

@@ -43,10 +43,7 @@ pub async fn get_changelog(
         .await
         .map_err(|e| AppError::Git(e.to_string()))?;
     let head_ref = store::resolve_head(&disk_path, &record.default_branch);
-    // A read failure is not an empty history: returning a bare 200 with no
-    // events makes a degraded repo look identical to a brand-new one (#400).
-    let commits =
-        store::log(&disk_path, &head_ref, limit).map_err(|e| AppError::Git(e.to_string()))?;
+    let commits = store::log(&disk_path, &head_ref, limit).unwrap_or_default();
 
     let mut events: Vec<serde_json::Value> = commits
         .into_iter()
@@ -63,9 +60,7 @@ pub async fn get_changelog(
         .collect();
 
     // ── Merged PRs ───────────────────────────────────────────────────────
-    // Same for the DB half: an outage must surface as an error (503 when the
-    // pool is unreachable), not an empty timeline (#400).
-    let prs = state.db.list_prs(&record.id).await?;
+    let prs = state.db.list_prs(&record.id).await.unwrap_or_default();
     for pr in prs.iter().filter(|p| p.status == "merged") {
         events.push(serde_json::json!({
             "type": "pr_merged",
@@ -93,212 +88,3 @@ pub async fn get_changelog(
         "count": events.len(),
     })))
 }
-
-/// #400: endpoint-level proof that a degraded store or DB reaches the caller
-/// as an error, not a 200 with an empty timeline.
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use axum::http::Request;
-    use axum::http::StatusCode;
-    use axum::Router;
-    use sqlx::PgPool;
-    use tempfile::TempDir;
-    use tower::ServiceExt;
-
-    fn seed_repo(owner_did: &str, name: &str) -> crate::db::RepoRecord {
-        let now = chrono::Utc::now();
-        crate::db::RepoRecord {
-            id: uuid::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 controls the
-    /// on-disk repo, with the repo record already inserted.
-    async fn seeded_state(
-        pool: &PgPool,
-        repos_dir: &std::path::Path,
-        owner: &str,
-        name: &str,
-    ) -> 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
-            .db
-            .create_repo(&seed_repo(owner, name))
-            .await
-            .expect("seed repo");
-        state
-    }
-
-    fn repo_disk_path(repos_dir: &std::path::Path, owner: &str, name: &str) -> std::path::PathBuf {
-        repos_dir
-            .join(owner.replace([':', '/'], "_"))
-            .join(format!("{name}.git"))
-    }
-
-    fn init_bare(path: &std::path::Path) {
-        std::fs::create_dir_all(path).unwrap();
-        let out = std::process::Command::new("git")
-            .args(["init", "--bare"])
-            .arg(path)
-            .output()
-            .unwrap();
-        assert!(out.status.success());
-    }
-
-    /// A bare repo with one real commit on HEAD.
-    fn bare_repo_with_commit(
-        repos_dir: &std::path::Path,
-        owner: &str,
-        name: &str,
-    ) -> std::path::PathBuf {
-        let scratch = repos_dir.join(format!("scratch-{name}"));
-        let out = std::process::Command::new("git")
-            .args(["init"])
-            .arg(&scratch)
-            .output()
-            .unwrap();
-        assert!(out.status.success());
-        let out = std::process::Command::new("git")
-            .args([
-                "-c",
-                "user.email=t@t",
-                "-c",
-                "user.name=t",
-                "commit",
-                "--allow-empty",
-                "-m",
-                "initial",
-            ])
-            .current_dir(&scratch)
-            .output()
-            .unwrap();
-        assert!(out.status.success());
-        let repo_path = repo_disk_path(repos_dir, owner, name);
-        std::fs::create_dir_all(repo_path.parent().unwrap()).unwrap();
-        let out = std::process::Command::new("git")
-            .args(["clone", "--bare"])
-            .arg(&scratch)
-            .arg(&repo_path)
-            .output()
-            .unwrap();
-        assert!(out.status.success());
-        repo_path
-    }
-
-    /// Delete the object behind HEAD and leave garbage: `git log` fails while
-    /// `rev-parse` still resolves the ref.
-    fn corrupt_head_object(repo_path: &std::path::Path) {
-        let out = std::process::Command::new("git")
-            .args(["rev-parse", "HEAD"])
-            .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();
-    }
-
-    async fn oneshot_changelog(
-        state: AppState,
-        owner: &str,
-        name: &str,
-    ) -> axum::response::Response {
-        Router::new()
-            .route(
-                "/api/v1/repos/{owner}/{repo}/changelog",
-                axum::routing::get(get_changelog),
-            )
-            .with_state(state)
-            .oneshot(
-                Request::builder()
-                    .uri(format!("/api/v1/repos/{owner}/{name}/changelog"))
-                    .body(axum::body::Body::empty())
-                    .unwrap(),
-            )
-            .await
-            .unwrap()
-    }
-
-    /// The git half of the fold: a repo whose HEAD resolves but whose object
-    /// store is corrupt must be a 500, not a 200 with zero events.
-    #[sqlx::test]
-    async fn changelog_on_corrupt_object_store_returns_500_not_empty_200(pool: PgPool) {
-        let owner = "did:key:zCHANGELOGCORRUPTAAAAAAAAAAAAAAAAAAAA";
-        let dir = TempDir::new().unwrap();
-        let state = seeded_state(&pool, dir.path(), owner, "corrupt-log").await;
-        let repo_path = bare_repo_with_commit(dir.path(), owner, "corrupt-log");
-        corrupt_head_object(&repo_path);
-
-        let resp = oneshot_changelog(state, owner, "corrupt-log").await;
-        assert_eq!(
-            resp.status(),
-            StatusCode::INTERNAL_SERVER_ERROR,
-            "a resolving ref whose objects cannot be read is a git error"
-        );
-    }
-
-    /// The DB half of the fold: with the pull_requests table gone, list_prs
-    /// fails and the endpoint must surface it (503) rather than answering a
-    /// 200 with only the git-derived events.
-    #[sqlx::test]
-    async fn changelog_on_pr_table_failure_returns_error_not_empty_200(pool: PgPool) {
-        let owner = "did:key:zCHANGELOGDBBBBBBBBBBBBBBBBBBBBBBBBBBB";
-        let dir = TempDir::new().unwrap();
-        let state = seeded_state(&pool, dir.path(), owner, "db-fail").await;
-        init_bare(&repo_disk_path(dir.path(), owner, "db-fail"));
-        sqlx::query("DROP TABLE pull_requests")
-            .execute(&pool)
-            .await
-            .unwrap();
-
-        let resp = oneshot_changelog(state, owner, "db-fail").await;
-        assert!(
-            resp.status().is_server_error(),
-            "a DB failure must not answer a 200 empty timeline; got {}",
-            resp.status()
-        );
-    }
-
-    /// Must-not direction: a healthy repo with a commit still returns the
-    /// event; an empty repo is still a valid empty timeline.
-    #[sqlx::test]
-    async fn changelog_still_serves_commit_and_empty_repo(pool: PgPool) {
-        let owner = "did:key:zCHANGELOGOKCCCCCCCCCCCCCCCCCCCCCCCCCCC";
-        let dir = TempDir::new().unwrap();
-        let state = seeded_state(&pool, dir.path(), owner, "with-commit").await;
-        bare_repo_with_commit(dir.path(), owner, "with-commit");
-
-        let resp = oneshot_changelog(state, owner, "with-commit").await;
-        assert_eq!(resp.status(), StatusCode::OK);
-        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
-            .await
-            .unwrap();
-        let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
-        assert_eq!(v["count"], 1, "the seeded commit must appear");
-        assert_eq!(v["events"][0]["type"], "commit");
-
-        let state = seeded_state(&pool, dir.path(), owner, "empty-repo").await;
-        init_bare(&repo_disk_path(dir.path(), owner, "empty-repo"));
-        let resp = oneshot_changelog(state, owner, "empty-repo").await;
-        assert_eq!(resp.status(), StatusCode::OK);
-        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
-            .await
-            .unwrap();
-        let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
-        assert_eq!(v["count"], 0, "a genuinely empty repo is still 200/empty");
-    }
-}

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);
+    }
 }

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

@@ -141,29 +141,7 @@ pub fn log(repo_path: &Path, refname: &str, limit: usize) -> Result<Vec<CommitIn
         .context("failed to run git log")?;
 
     if !output.status.success() {
-        // An unresolvable ref means a genuinely empty repo. A ref that
-        // resolves but fails to log is a read failure (corrupt object store, a
-        // gc mid-read): report it rather than passing an empty log off as no
-        // history (#400). rev-parse resolves the name without reading objects.
-        // allow-unbounded-git: failure-path existence recheck; module
-        // convention, at most one extra spawn per failed log.
-        let resolved = Command::new("git")
-            .args(["rev-parse", "--verify", "--quiet", refname])
-            .current_dir(repo_path)
-            .output()
-            .context("failed to run git rev-parse")?;
-        // --quiet exits 1 only for "ref does not resolve"; other failures
-        // (not a repo, broken config) are real errors, not an empty history.
-        match resolved.status.code() {
-            Some(1) => return Ok(vec![]),
-            Some(0) => {}
-            _ => {
-                let stderr = String::from_utf8_lossy(&resolved.stderr);
-                anyhow::bail!("git rev-parse failed for {refname}: {}", stderr.trim());
-            }
-        }
-        let stderr = String::from_utf8_lossy(&output.stderr);
-        anyhow::bail!("git log failed for {refname}: {}", stderr.trim());
+        return Ok(vec![]); // empty repo
     }
 
     let stdout = String::from_utf8_lossy(&output.stdout);
@@ -2047,65 +2025,4 @@ mod tests {
             "a clean `missing` twice on a readable store is a genuine absence; got {res:?}"
         );
     }
-
-    // #400: `log` may fold a failed `git log` into Ok(vec![]) only when the ref
-    // genuinely does not resolve (an empty repo). A ref that resolves but
-    // fails to read is an error, not an empty history.
-    #[test]
-    fn log_empty_is_empty_and_resolved_ref_read_failure_errors() {
-        let td = tempfile::TempDir::new().unwrap();
-        let work: &Path = td.path();
-        let g = |args: &[&str]| {
-            assert!(Command::new("git")
-                .args(args)
-                .current_dir(work)
-                .status()
-                .unwrap()
-                .success());
-        };
-        g(&["init", "-q"]);
-        g(&["config", "user.email", "t@t"]);
-        g(&["config", "user.name", "t"]);
-
-        // No commits: nothing resolves, so a failed log is a real empty repo.
-        assert!(super::log(work, "HEAD", 10).unwrap().is_empty());
-
-        std::fs::write(work.join("f.txt"), b"x\n").unwrap();
-        g(&["add", "."]);
-        g(&["commit", "-qm", "c1"]);
-        assert_eq!(super::log(work, "HEAD", 10).unwrap().len(), 1);
-
-        // Corrupt the object the branch resolves to: `git log` fails but
-        // `rev-parse --verify` still resolves the name, so the failure must
-        // surface as an error rather than an empty history.
-        let oid = {
-            let o = Command::new("git")
-                .args(["rev-parse", "HEAD"])
-                .current_dir(work)
-                .output()
-                .unwrap();
-            String::from_utf8_lossy(&o.stdout).trim().to_string()
-        };
-        let obj = work.join(".git/objects").join(&oid[..2]).join(&oid[2..]);
-        std::fs::remove_file(&obj).unwrap();
-        std::fs::write(&obj, b"garbage").unwrap();
-
-        assert!(super::log(work, "HEAD", 10).is_err());
-    }
-
-    /// The rev-parse recheck inside `log` must distinguish "ref missing"
-    /// (exit 1 -> empty history) from a real probe failure (exit 128 on a
-    /// non-repo -> error), not fold both into `Ok(vec![])`.
-    #[test]
-    fn log_probe_failure_on_a_non_repo_errors_instead_of_empty() {
-        let td = tempfile::TempDir::new().unwrap();
-        // A directory that is not a git repo at all: `git log` fails and the
-        // `rev-parse` recheck fails with 128, which is not "missing ref".
-        let not_a_repo = td.path().join("not-a-repo");
-        std::fs::create_dir_all(&not_a_repo).unwrap();
-        assert!(
-            super::log(&not_a_repo, "HEAD", 10).is_err(),
-            "a rev-parse probe failure must not read as an empty repo"
-        );
-    }
 }

crates/gl/src/clone.rs

@@ -54,13 +54,6 @@ fn emit_warning(line: &str) {
     let _ = writeln!(warn_sink(), "{line}");
 }
 
-/// Warn-and-degrade for the recovery arms in `run`: a failed recovery source
-/// is reported and yields no paths, never silently empty (#400).
-fn warn_recovery_failed(what: &str, e: &anyhow::Error) -> Vec<String> {
-    emit_warning(&format!("warning: {what} failed: {e}"));
-    Vec::new()
-}
-
 #[cfg(not(test))]
 fn warn_sink() -> impl std::io::Write {
     std::io::stderr()
@@ -329,19 +322,7 @@ async fn recover_encrypted_blobs(
         .await
     {
         Ok(r) if r.status().is_success() => r,
-        Ok(r) => {
-            emit_warning(&format!(
-                "warning: could not list encrypted blobs for {owner}/{name}: node returned {}; skipping recovery",
-                r.status()
-            ));
-            return Ok(vec![]);
-        }
-        Err(e) => {
-            emit_warning(&format!(
-                "warning: could not list encrypted blobs for {owner}/{name}: {e}; skipping recovery"
-            ));
-            return Ok(vec![]);
-        }
+        _ => return Ok(vec![]),
     };
     let body: EncryptedBlobsResponse = resp.json().await.context("parsing encrypted-blobs")?;
     if body.blobs.is_empty() {
@@ -855,7 +836,8 @@ pub async fn run(args: CloneArgs) -> Result<()> {
                 // fallback still runs), but the strict /encrypted-blobs parse now
                 // fails closed on schema drift, so surface it rather than letting
                 // `.unwrap_or_default()` silently swallow it into "no paths".
-                warn_recovery_failed("encrypted-blobs recovery", &e)
+                eprintln!("warning: encrypted-blobs recovery failed: {e}");
+                Vec::new()
             });
         let from_arweave = recover_from_arweave(
             &args.arweave_gateway,
@@ -866,10 +848,7 @@ pub async fn run(args: CloneArgs) -> Result<()> {
             &keypair,
         )
         .await
-        .unwrap_or_else(|e| {
-            // Same rule as the node-recovery arm above: warn, never abort.
-            warn_recovery_failed("arweave/ipfs gateway recovery", &e)
-        });
+        .unwrap_or_default();
         paths.extend(from_arweave);
 
         if !paths.is_empty() {
@@ -950,46 +929,6 @@ mod tests {
         WARNINGS.with(|w| w.borrow().clone())
     }
 
-    // #400: the recovery-arm degrade must warn, not silently become "no
-    // paths". `warn_recovery_failed` is the write both arms go through.
-    #[test]
-    fn warn_recovery_failed_warns_and_returns_empty() {
-        reset_warnings();
-        let out =
-            super::warn_recovery_failed("arweave/ipfs gateway recovery", &anyhow::anyhow!("boom"));
-        assert!(out.is_empty());
-        assert!(
-            warnings().contains("arweave/ipfs gateway recovery failed: boom"),
-            "a recovery failure must warn, got: {:?}",
-            warnings()
-        );
-    }
-
-    // #400: the encrypted-blobs LIST fetch must warn on failure instead of
-    // silently returning an empty recovery set. The per-blob stage already
-    // warns; the list fetch was the outlier.
-    #[tokio::test]
-    async fn encrypted_blobs_list_failure_warns_and_returns_empty() {
-        let mut server = mockito::Server::new_async().await;
-        let _m = server
-            .mock("GET", "/api/v1/repos/o/r/encrypted-blobs")
-            .with_status(500)
-            .create_async()
-            .await;
-        let kp = gitlawb_core::identity::Keypair::generate();
-        let dest = TempDir::new().unwrap();
-        reset_warnings();
-        let paths = recover_encrypted_blobs(&server.url(), "o", "r", dest.path(), &kp)
-            .await
-            .unwrap();
-        assert!(paths.is_empty());
-        assert!(
-            warnings().contains("could not list encrypted blobs"),
-            "a failed list fetch must warn, got: {:?}",
-            warnings()
-        );
-    }
-
     fn g(args: &[&str], dir: &Path) {
         assert!(Command::new("git")
             .args(args)