Merge pull request #404 from ahrav/feature/wire-pg-git-persistence

Feature/wire pg git persistence

Author: ahrav

.beads/issues.jsonl

@@ -1,5 +1,37 @@
 **NEVER auto-commit, auto-add, or auto-push code to git. Only perform git operations when explicitly asked by the user.**
 
+<!-- autoresearch-git-exception-v1 -->
+
+### Exception: Autoresearch Commit/Revert Cycle
+
+The `/autoresearch` family of commands (and their OpenCode `_` variants) are
+**exempt** from the no-auto-commit policy during an active autoresearch loop.
+Autoresearch requires git commits as a core mechanism — commit-before-verify
+enables clean rollback, and git history serves as the agent's memory of what
+was tried and what failed.
+
+**Allowed git operations inside an autoresearch loop only:**
+
+| Operation | Purpose |
+| --------- | ------- |
+| `git add` + `git commit -m "experiment: ..."` | Record each atomic change before verification |
+| `git revert HEAD --no-edit` | Roll back a failed experiment |
+| `git log` / `git diff` | Read own experiment history for learning |
+
+**Still prohibited, even inside autoresearch:**
+
+- `git push` (never push experiment commits automatically)
+- `git commit --amend` or `git rebase` (never rewrite experiment history)
+- `git reset --hard` (always use `git revert` to preserve history)
+- Committing to `main`/`master` directly (create a dedicated experiment branch first)
+
+**Activation:** This exception applies only when the user explicitly invokes
+an autoresearch command (`/autoresearch`, `/autoresearch:debug`,
+`/autoresearch_fix`, etc.). Normal agent operations outside the autoresearch
+loop must still follow the no-auto-commit policy.
+
+<!-- end-autoresearch-git-exception -->
+
 <!-- comment-policy-v2 -->
 
 ## Comment Policy — MANDATORY, HOOK-ENFORCED

AGENTS.md

@@ -1,5 +1,37 @@
 **NEVER auto-commit, auto-add, or auto-push code to git. Only perform git operations when explicitly asked by the user.**
 
+<!-- autoresearch-git-exception-v1 -->
+
+### Exception: Autoresearch Commit/Revert Cycle
+
+The `/autoresearch` family of commands (and their OpenCode `_` variants) are
+**exempt** from the no-auto-commit policy during an active autoresearch loop.
+Autoresearch requires git commits as a core mechanism — commit-before-verify
+enables clean rollback, and git history serves as the agent's memory of what
+was tried and what failed.
+
+**Allowed git operations inside an autoresearch loop only:**
+
+| Operation | Purpose |
+| --------- | ------- |
+| `git add` + `git commit -m "experiment: ..."` | Record each atomic change before verification |
+| `git revert HEAD --no-edit` | Roll back a failed experiment |
+| `git log` / `git diff` | Read own experiment history for learning |
+
+**Still prohibited, even inside autoresearch:**
+
+- `git push` (never push experiment commits automatically)
+- `git commit --amend` or `git rebase` (never rewrite experiment history)
+- `git reset --hard` (always use `git revert` to preserve history)
+- Committing to `main`/`master` directly (create a dedicated experiment branch first)
+
+**Activation:** This exception applies only when the user explicitly invokes
+an autoresearch command (`/autoresearch`, `/autoresearch:debug`,
+`/autoresearch_fix`, etc.). Normal agent operations outside the autoresearch
+loop must still follow the no-auto-commit policy.
+
+<!-- end-autoresearch-git-exception -->
+
 ## Agent Model Inheritance — MANDATORY
 
 When dispatching subagents via the Agent tool, **NEVER set the `model` parameter**.

CLAUDE.md

@@ -26,7 +26,7 @@ use gossip_contracts::{
     },
 };
 use gossip_coordination::{AcquireScratch, CoordinationFacade, CursorSemantics};
-use scanner_git::{FinalizeOutcome, GitEventOutput};
+use scanner_git::{FinalizeOutcome, GitEventOutput, PersistenceStore};
 use scanner_scheduler::events::EventOutput;
 
 use super::commit_bridge::{
@@ -550,9 +550,10 @@ pub(super) struct GitRepoPersistenceInput<'a> {
     pub(super) tenant_secret_key: TenantSecretKey,
     pub(super) rule_fingerprint: &'a dyn Fn(u32) -> RuleFingerprint,
     pub(super) claim_time: LogicalTime,
-    /// Wall-clock timestamp captured after scan execution *and* persistence
-    /// finalize complete. The `(claim_time, complete_time)` interval therefore
-    /// measures claim-to-durable-finalize, not claim-to-scan-completion alone.
+    /// Wall-clock timestamp captured after scan execution completes but before
+    /// persistence submissions. The `(claim_time, complete_time)` interval
+    /// measures claim-to-scan-completion; it does not include persistence
+    /// latency.
     pub(super) complete_time: LogicalTime,
 }
 
@@ -912,11 +913,11 @@ where
         )));
     }
 
-    // Findings must be durable BEFORE shard checkpoint advances. The in-memory
-    // GitPersistenceBackend makes post-execution batch write safe: a crash
-    // clears in-memory watermarks, causing full re-scan on re-claim. A durable
-    // GitPersistenceBackend would require splitting finalize into: seen-bitmaps
-    // → findings persistence → watermark commit.
+    // External findings and done-ledger state must land before a complete Git
+    // finalize advances the repo's durable scan state. The scan phase buffers
+    // complete finalizes via `DeferredCompleteFinalizeStore` so the git-kv
+    // commit can run after these receipts succeed; partial finalizes remain
+    // inline because they never advance watermarks.
     let captured_findings = capture_sink.take_captured_findings();
     let detected_count = capture_sink.detected_finding_count();
     if detected_count != captured_findings.len() as u64 {
@@ -966,6 +967,33 @@ where
         receipts: GIT_REPO_RECEIPT_FAMILIES,
     });
 
+    // At-least-once guarantee: findings and done-ledger records are already
+    // durable at this point. If commit_finalize fails (connection drop,
+    // constraint violation) or the process is killed before it completes,
+    // watermarks remain at their pre-scan position. The next lease re-scans
+    // the same blobs and re-emits findings. Done-ledger and findings
+    // consumers must tolerate duplicate submissions.
+    if matches!(execution.finalize_outcome, FinalizeOutcome::Complete)
+        && execution.deferred_finalize.is_none()
+    {
+        return Err(DistributedRuntimeError::Durability(anyhow!(
+            "complete finalize for shard '{}' must produce a deferred finalize output; \
+             watermarks would be silently dropped",
+            stage_sink.redacted_shard_id()
+        )));
+    }
+    if let Some(finalize) = execution.deferred_finalize.as_ref() {
+        execution
+            .persistence
+            .commit_finalize(finalize)
+            .map_err(|error| {
+                DistributedRuntimeError::Durability(AnyError::new(error).context(format!(
+                    "git repo-frontier shard '{}' git state finalize commit failed",
+                    stage_sink.redacted_shard_id()
+                )))
+            })?;
+    }
+
     tracing::debug!(
         shard_id = %stage_sink.redacted_shard_id(),
         detected_findings = detected_count,

Cargo.lock

@@ -2019,6 +2019,58 @@ fn run_git_repo_worker_fails_cleanly_on_persistence_error() {
     );
 }
 
+/// The post-receipt `commit_finalize` failure must surface as a `Durability`
+/// error while leaving findings and done-ledger rows already persisted (the
+/// at-least-once guarantee). Scan-phase persistence succeeds; only the
+/// deferred finalize commit that advances watermarks and clears staging is
+/// injected to fail.
+#[test]
+fn run_git_repo_worker_surfaces_durability_error_on_post_receipt_finalize_failure() {
+    let repo = create_clean_git_repo_fixture();
+    let mirror_root = tempdir().expect("mirror root");
+    let mut mirrors = LocalMirrorManager::new(mirror_root.path()).expect("mirror manager");
+    let backend = TestGitBackend::default();
+    backend.fail_on_finalize_commit();
+    let mut coordinator =
+        setup_coordinator_with_git_shard(repo.path(), CoordCursorUpdate::initial(), 30_000);
+
+    let findings = InMemoryFindingsSink::new();
+    let done_ledger = InMemoryDoneLedger::new();
+    let err = run_git_repo_worker(
+        &mut coordinator,
+        &mut mirrors,
+        git_worker_identity(repo.path()),
+        backend.clone(),
+        DistributedPersistence::new(findings.clone(), done_ledger.clone()),
+        DistributedRuntimeConfig::default(),
+    )
+    .expect_err("post-receipt finalize commit failure should propagate");
+
+    assert!(
+        matches!(err, DistributedRuntimeError::Durability(_)),
+        "expected Durability error variant, got: {err:?}"
+    );
+    assert!(
+        err.to_string().contains("finalize commit failed"),
+        "error message should identify the finalize commit failure: {err}"
+    );
+
+    // Scan-phase spill writes must have succeeded before the finalize failed.
+    assert!(
+        backend.batch_call_count() > 0,
+        "scan-phase persistence batches should have succeeded before the finalize failure"
+    );
+
+    // Done-ledger and findings are submitted BEFORE commit_finalize, so they
+    // must be durable despite the finalize failure (at-least-once guarantee).
+    let done_rows = done_ledger.snapshot().expect("done-ledger snapshot");
+    assert_eq!(
+        done_rows.len(),
+        1,
+        "done-ledger row must be durable even when finalize commit fails"
+    );
+}
+
 /// Git events remain observable when the repo-frontier worker persists
 /// findings durably.
 #[test]

crates/gossip-scanner-runtime/src/distributed/execution.rs

@@ -104,6 +104,12 @@ pub(super) struct TestGitBackendState {
     pub(super) kv: BTreeMap<Vec<u8>, Vec<u8>>,
     pub(super) batch_call_count: usize,
     pub(super) fail_after_n_batches: Option<usize>,
+    /// Fail any `apply_batch` call that contains a `Delete` op AND has more
+    /// than two operations. Complete-finalize commits include data ops,
+    /// checkpoint deletes, staging delete, and watermark puts (4+ ops).
+    /// Scan-phase checkpoint writes have at most 2 ops. This flag targets
+    /// post-scan finalize failures without knowledge of batch counts.
+    pub(super) fail_on_finalize_commit: bool,
 }
 
 #[derive(Debug, Clone, Default)]
@@ -126,6 +132,13 @@ impl TestGitBackend {
             .fail_after_n_batches = Some(n);
     }
 
+    pub(super) fn fail_on_finalize_commit(&self) {
+        self.state
+            .lock()
+            .expect("git backend state lock")
+            .fail_on_finalize_commit = true;
+    }
+
     pub(super) fn stored_keys(&self) -> Vec<Vec<u8>> {
         self.state
             .lock()
@@ -159,6 +172,16 @@ impl GitPersistenceBackend for TestGitBackend {
                 message: "injected persistence failure",
             });
         }
+        if state.fail_on_finalize_commit
+            && ops.len() > 2
+            && ops
+                .iter()
+                .any(|op| matches!(op, GitPersistenceOp::Delete { .. }))
+        {
+            return Err(TestGitBackendError {
+                message: "injected finalize commit failure",
+            });
+        }
         state.batch_call_count += 1;
         for op in ops {
             match op {