fix(node): address four reviewer findings on #26 split 1/4

- P1-A: add startup reconcile step that promotes `prepared` rows to
  `applied` when the on-disk ref matches the row's `new_sha`. The
  recovery drain (which only reads `applied` rows) can now pick up
  a ref that landed when the live handler's
  `mark_pending_ref_transitions_applied` call errored or was
  interrupted. Strict SHA equality is the load-bearing check — a
  `prepared` row whose target did NOT actually land stays
  `prepared`.
- P1-B: route the live handler's cert issuance through
  `cert::issue_ref_certificate` (the upsert) instead of
  `issue_ref_certificate_idempotent` (DO NOTHING). A re-push to the
  same ref now updates the cert's `old_sha` / `new_sha` /
  `pusher_did` / `issued_at` / `signature` to the new transition
  while preserving the deterministic `cert_id`. The recovery drain
  keeps the idempotent variant; both paths collapse to one row.
- P2-A: refactor the drain into a `drain_pending_ref_transitions_with`
  testable seam that does per-row log-and-continue, and add
  `drain_pending_ref_transitions_all` that loops
  `DRAIN_PER_PASS_LIMIT=1000` rows for `DRAIN_MAX_PASSES=10` passes.
  A failing row no longer stalls the batch; a backlog above 1000
  rows is fully processed across passes.
- P2-B: add a `first_ref_name` column to `pending_ref_transitions`
  via migration v28. The live handler hoists a `first_ref_name`
  local and persists it on every row of the same `request_id`. The
  drain's `derive_one` keys the push event id on
  `row.first_ref_name` instead of `row.ref_name`, so live and
  recovery produce the same id and `ON CONFLICT (id) DO NOTHING`
  collapses a multi-ref push to one push event row (and one trust-
  score bump). Cert and anchor ids stay per-ref / per-transition.

Author: Gravirei

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

@@ -2279,6 +2279,20 @@ pub async fn git_receive_pack(
         .and_then(|v| v.to_str().ok())
         .unwrap_or("")
         .to_string();
+    // The first ref name is hoisted here so it is the SAME value
+    // persisted on every `pending_ref_transitions` row of this request
+    // (request-scoped) AND the SAME value used to derive the push event
+    // id below. The recovery drain reads `first_ref_name` from each row
+    // and keys `push_event_id_for` on it, so the live and recovery
+    // paths produce the same id and the ON CONFLICT (id) DO NOTHING in
+    // `record_push_with_id` collapses a live push followed by a recovery
+    // pass into a single push event row. An empty `ref_updates` is
+    // defensive: receive-pack on a push with no refs would have already
+    // been rejected upstream, so this branch is unreachable in practice.
+    let first_ref_name = ref_updates
+        .first()
+        .map(|u| u.ref_name.clone())
+        .unwrap_or_default();
     if let Err(e) = state
         .db
         .insert_pending_ref_transitions(
@@ -2290,6 +2304,7 @@ pub async fn git_receive_pack(
             &signature_header,
             &signature_input,
             &content_digest,
+            &first_ref_name,
         )
         .await
     {
@@ -2464,11 +2479,12 @@ pub async fn git_receive_pack(
         // The push event is keyed on the FIRST ref's name so a
         // multi-ref push collapses to one push event row, not N. The
         // deterministic id is the same one the recovery drain
-        // derives.
-        let first_ref_name = ref_updates
-            .first()
-            .map(|u| u.ref_name.clone())
-            .unwrap_or_else(|| "refs/heads/main".to_string());
+        // derives, because the drain reads `first_ref_name` from the
+        // outbox row (persisted above on every row of this request) and
+        // uses it in the same `push_event_id_for` call. The outer
+        // `first_ref_name` local was hoisted above the
+        // `insert_pending_ref_transitions` call so this id matches
+        // the persisted value exactly.
         let push_event_id = crate::db::push_event_id_for(&request_id, &first_ref_name);
         let _ = state
             .db
@@ -2484,9 +2500,25 @@ pub async fn git_receive_pack(
         // Issue a signed certificate for every ref this push advanced, each
         // carrying that ref's real old→new transition. A multi-ref push must
         // not collapse to a single cert covering only the first ref.
+        //
+        // P1-B: this routes through `issue_ref_certificate` (the
+        // upsert, `ON CONFLICT (repo_id, ref_name) DO UPDATE`) so a
+        // re-push to the same ref updates the row's
+        // `old_sha` / `new_sha` / `pusher_did` / `issued_at` /
+        // `signature` to the new transition while preserving the
+        // original `id` (the `insert_ref_certificate` upsert's
+        // `EXCLUDED.issued_at > ref_certificates.issued_at` guard).
+        // The previous `issue_ref_certificate_idempotent` call
+        // (DO NOTHING) left the first cert's fields frozen on every
+        // later push. The deterministic `cert_id` makes a recovery
+        // re-pass safe: the recovery's `insert_ref_certificate_idempotent`
+        // (DO NOTHING) is a no-op when the live handler has already
+        // written a row, and the live handler's upsert preserves the
+        // original `id` so the deterministic id survives across the
+        // push / recovery / re-push cycle.
         for update in &ref_updates {
             let cert_id = crate::db::ref_cert_id_for(&request_id, &update.ref_name);
-            match cert::issue_ref_certificate_idempotent(
+            match cert::issue_ref_certificate(
                 &state,
                 &record.id,
                 &update.ref_name,
@@ -2497,12 +2529,9 @@ pub async fn git_receive_pack(
             )
             .await
             {
-                Ok(Some(c)) => {
+                Ok(c) => {
                     tracing::info!(cert_id = %c.id, repo = %record.name, ref_name = %update.ref_name, pusher = %did, "issued ref certificate")
                 }
-                Ok(None) => {
-                    tracing::debug!(ref_name = %update.ref_name, repo = %record.name, "ref certificate already exists for this ref, idempotent skip")
-                }
                 Err(e) => {
                     tracing::warn!(err = %e, ref_name = %update.ref_name, "failed to issue ref certificate")
                 }

crates/gitlawb-node/src/cert.rs

@@ -11,29 +11,57 @@ use uuid::Uuid;
 use crate::db::RefCertificate;
 use crate::state::AppState;
 
-/// Issue a signed ref-update certificate for a successful push.
+/// Issue a signed ref-update certificate for a successful push. The
+/// live receive-pack handler calls this on every successful push.
 ///
-/// Builds a canonical JSON payload, signs it with the node's Ed25519 key,
-/// persists the certificate, and returns it.
+/// `cert_id` is the deterministic id derived from `(request_id,
+/// ref_name)` (see [`crate::db::ref_cert_id_for`]). It is required so
+/// the recovery drain and the live handler produce the same primary
+/// key: a live push followed by a recovery pass collapses to a
+/// single cert row, and a re-push to the same `(repo, ref)` updates
+/// the existing row's `old_sha` / `new_sha` / `pusher_did` /
+/// `issued_at` / `signature` to the new transition while preserving
+/// the original `id` (the `insert_ref_certificate` upsert is
+/// keyed on `(repo_id, ref_name)` and only updates fields when the
+/// new `issued_at` is strictly greater).
 ///
-/// #26 Split PR 1: the live handler now uses
-/// [`issue_ref_certificate_idempotent`] so the cert id is deterministic
-/// and recovery re-derives the same primary key. This legacy entry
-/// point remains for callers that prefer a fresh UUID per cert (it
-/// keeps the older `insert_ref_certificate` upsert semantics); Split
-/// PR 3 owns the cert/CLI compatibility decision of whether to keep
-/// it or remove it.
-#[allow(dead_code)] // kept for the PR 3 cert/CLI compat pass
+/// #26 Split PR 1 P1-B: the live handler routes through this
+/// function (the upsert), NOT through
+/// [`issue_ref_certificate_idempotent`] (DO NOTHING). The
+/// idempotent variant is reserved for the recovery drain. Both
+/// paths use the same deterministic `cert_id` so a re-pass is
+/// always safe:
+///
+/// - Live handler → live upsert: re-push updates the row, preserves
+///   the original `id`. The contract pinned by
+///   `insert_ref_certificate_upserts_on_repo_ref` is restored.
+/// - Live handler → recovery: live's `ON CONFLICT (id) DO UPDATE`
+///   preserves the original `id`; the recovery's
+///   `ON CONFLICT (repo_id, ref_name) DO NOTHING` is a no-op.
+/// - Recovery → live handler: the recovery wrote a row with the
+///   deterministic `id`; the live upsert (which preserves `id` and
+///   only updates other fields when `issued_at` is strictly newer)
+///   is a no-op for an equal-`issued_at` re-run and a refresh for
+///   a strictly-newer one.
 pub async fn issue_ref_certificate(
     state: &AppState,
     repo_id: &str,
     ref_name: &str,
     old_sha: &str,
     new_sha: &str,
     pusher_did: &str,
+    cert_id: &str,
 ) -> Result<RefCertificate> {
-    let cert =
-        build_ref_certificate(state, repo_id, ref_name, old_sha, new_sha, pusher_did, None).await?;
+    let cert = build_ref_certificate(
+        state,
+        repo_id,
+        ref_name,
+        old_sha,
+        new_sha,
+        pusher_did,
+        Some(cert_id.to_string()),
+    )
+    .await?;
     state.db.insert_ref_certificate(&cert).await
 }