Merge master into 3ca0-announcements-purgatory

4 files changed, 203 insertions, 15 deletions

diff --git a/src/git/authorization.rs b/src/git/authorization.rs
index 69a0751..df780bb 100644
--- a/src/git/authorization.rs
+++ b/src/git/authorization.rs
@@ -609,6 +609,28 @@ pub async fn get_state_authorization_for_specific_owner_repo(
         owner_pubkey
     );
 
+    // Accept pushes where all refs are already at the desired state (old_oid == new_oid)
+    // This handles race conditions where state events are applied between fetch and push
+    if !pushed_refs.is_empty() {
+        let all_refs_unchanged = pushed_refs
+            .iter()
+            .all(|(old_oid, new_oid, _)| old_oid == new_oid);
+
+        if all_refs_unchanged {
+            debug!(
+                "All pushed refs unchanged (old_oid == new_oid) for {} owned by {}, accepting without purgatory check",
+                identifier, owner_pubkey
+            );
+            return Ok(AuthorizationResult {
+                authorized: true,
+                reason: "Push accepted: all refs already at desired state (no-op)".to_string(),
+                state: None,
+                maintainers: authorized.into_iter().collect(),
+                purgatory_events: vec![],
+            });
+        }
+    }
+
     // Check purgatory for matching state events
     // Convert pushed refs to RefUpdate (filter out refs/nostr/* refs)
     let pushed_updates: Vec<RefUpdate> = pushed_refs
@@ -699,12 +721,88 @@ pub async fn get_state_authorization_for_specific_owner_repo(
             debug!("Purgatory events found but none from authorized authors");
         }
     } else {
-        debug!("No matching state events found in purgatory");
+        // Check if there are ANY state events in purgatory for this identifier
+        let all_purgatory_states = purgatory.find_state(identifier);
+
+        if !all_purgatory_states.is_empty() {
+            // There are state events but none match the push - diagnose why
+            debug!(
+                "Found {} state event(s) in purgatory for {} but none match the push",
+                all_purgatory_states.len(),
+                identifier
+            );
+
+            // Count authorized state events and collect diagnostic info
+            let mut authorized_count = 0;
+            let mut diagnostic_reasons = Vec::new();
+
+            // Diagnose why each authorized state event doesn't match
+            for entry in all_purgatory_states.iter() {
+                let author_hex = entry.event.pubkey.to_hex();
+                if authorized.contains(&author_hex) {
+                    authorized_count += 1;
+                    if let Some(reason) = crate::purgatory::diagnose_state_mismatch(
+                        &entry.event,
+                        &pushed_updates,
+                        &local_refs,
+                    ) {
+                        debug!(
+                            "State event {} from authorized author {} doesn't match push: {}",
+                            entry.event.id,
+                            entry
+                                .event
+                                .pubkey
+                                .to_bech32()
+                                .unwrap_or_else(|_| author_hex.clone()),
+                            reason
+                        );
+                        diagnostic_reasons.push(reason);
+                    }
+                }
+            }
+
+            // Create concise WARN message summarizing the rejection
+            let summary = if authorized_count > 0 {
+                let reason_summary = if !diagnostic_reasons.is_empty() {
+                    // Take the first diagnostic reason as representative
+                    format!(" ({})", diagnostic_reasons[0])
+                } else {
+                    String::new()
+                };
+                format!(
+                    "{} state event{} in purgatory from authorized publisher{} but doesn't match push{}",
+                    authorized_count,
+                    if authorized_count == 1 { "" } else { "s" },
+                    if authorized_count == 1 { "" } else { "s" },
+                    reason_summary
+                )
+            } else {
+                format!(
+                    "{} state event{} in purgatory but none from authorized publishers",
+                    all_purgatory_states.len(),
+                    if all_purgatory_states.len() == 1 {
+                        ""
+                    } else {
+                        "s"
+                    }
+                )
+            };
+
+            warn!("Push rejected for {}: {}", identifier, summary);
+            return Ok(AuthorizationResult::denied(summary));
+        } else {
+            debug!("No state events found in purgatory for {}", identifier);
+            warn!(
+                "Push rejected for {}: No state events in purgatory",
+                identifier
+            );
+            return Ok(AuthorizationResult::denied("No state events in purgatory"));
+        }
     }
 
     // No matching state found in purgatory
     Ok(AuthorizationResult::denied(
-        "No state event found in purgatory from authorized publishers",
+        "No matching state event found in purgatory from authorized publishers",
     ))
 }
 
diff --git a/src/git/handlers.rs b/src/git/handlers.rs
index 13d6ba0..f43cbb6 100644
--- a/src/git/handlers.rs
+++ b/src/git/handlers.rs
@@ -100,6 +100,42 @@ pub async fn handle_info_refs(
         .unwrap())
 }
 
+/// Build an HTTP 200 OK response with an ERR pkt-line for git protocol errors.
+///
+/// Per the git smart HTTP protocol spec, protocol-level errors (like "not our ref")
+/// should be returned as HTTP 200 OK with the error message in pkt-line format:
+/// `PKT-LINE("ERR" SP explanation-text)`
+///
+/// This allows git clients to properly parse and display the error message.
+fn build_git_protocol_error_response(
+    service: GitService,
+    error_message: &str,
+) -> Response<Full<Bytes>> {
+    // Format: "ERR <message>\n"
+    let err_content = format!("ERR {}\n", error_message.trim());
+    let err_pktline = PktLine::data(err_content.as_bytes()).encode();
+
+    Response::builder()
+        .status(StatusCode::OK)
+        .header("content-type", service.result_content_type())
+        .header("cache-control", "no-cache")
+        .body(Full::new(Bytes::from(err_pktline)))
+        .unwrap()
+}
+
+/// Check if a git process failure is a protocol error (vs transport error).
+///
+/// Protocol errors are communicated via stderr when git exits with code 128.
+/// These should be returned to the client as HTTP 200 with ERR pkt-line.
+///
+/// Transport errors (process spawn failures, I/O errors, signals) should
+/// remain as HTTP 500 errors.
+fn is_git_protocol_error(exit_code: Option<i32>, stderr: &[u8]) -> bool {
+    // Git uses exit code 128 for protocol/usage errors
+    // If there's stderr content, it's a protocol error message
+    exit_code == Some(128) && !stderr.is_empty()
+}
+
 /// Handle POST /git-upload-pack (clone/fetch)
 pub async fn handle_upload_pack(
     repo_path: PathBuf,
@@ -121,7 +157,10 @@ pub async fn handle_upload_pack(
         stdin
             .write_all(&request_body)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to write to git upload-pack stdin: {}", e);
+                GitError::IoError(e)
+            })?;
         // Close stdin to signal end of input
         drop(stdin);
     }
@@ -135,7 +174,10 @@ pub async fn handle_upload_pack(
         stdout
             .read_to_end(&mut output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git upload-pack stdout: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     if let Some(stderr) = git.take_stderr() {
@@ -143,14 +185,35 @@ pub async fn handle_upload_pack(
         stderr
             .read_to_end(&mut stderr_output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git upload-pack stderr: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     // Wait for process
-    let status = git.wait().await.map_err(GitError::IoError)?;
+    let status = git.wait().await.map_err(|e| {
+        error!("Failed to wait for git upload-pack process: {}", e);
+        GitError::IoError(e)
+    })?;
 
     if !status.success() {
         let stderr_str = String::from_utf8_lossy(&stderr_output);
+
+        // Check if this is a git protocol error (exit code 128 with stderr)
+        // Protocol errors should be returned as HTTP 200 with ERR pkt-line
+        if is_git_protocol_error(status.code(), &stderr_output) {
+            warn!(
+                "Git upload-pack protocol error (returning ERR pkt-line): {}",
+                stderr_str
+            );
+            return Ok(build_git_protocol_error_response(
+                GitService::UploadPack,
+                &stderr_str,
+            ));
+        }
+
+        // Transport errors (spawn failures, signals, etc.) remain as HTTP 500
         error!("Git upload-pack failed: {}", stderr_str);
         return Err(GitError::GitFailed(status.code()));
     }
@@ -206,7 +269,7 @@ pub async fn handle_receive_pack(
     }
 
     // GRASP Authorization Check
-    info!(
+    debug!(
         "Authorizing push for {} owned by {} via database query",
         identifier, owner_pubkey
     );
@@ -251,7 +314,10 @@ pub async fn handle_receive_pack(
         stdin
             .write_all(&request_body)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to write to git receive-pack stdin: {}", e);
+                GitError::IoError(e)
+            })?;
         drop(stdin);
     }
 
@@ -264,7 +330,10 @@ pub async fn handle_receive_pack(
         stdout
             .read_to_end(&mut output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git receive-pack stdout: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     if let Some(stderr) = git.take_stderr() {
@@ -272,14 +341,35 @@ pub async fn handle_receive_pack(
         stderr
             .read_to_end(&mut stderr_output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git receive-pack stderr: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     // Wait for process
-    let status = git.wait().await.map_err(GitError::IoError)?;
+    let status = git.wait().await.map_err(|e| {
+        error!("Failed to wait for git receive-pack process: {}", e);
+        GitError::IoError(e)
+    })?;
 
     if !status.success() {
         let stderr_str = String::from_utf8_lossy(&stderr_output);
+
+        // Check if this is a git protocol error (exit code 128 with stderr)
+        // Protocol errors should be returned as HTTP 200 with ERR pkt-line
+        if is_git_protocol_error(status.code(), &stderr_output) {
+            warn!(
+                "Git receive-pack protocol error (returning ERR pkt-line): {}",
+                stderr_str
+            );
+            return Ok(build_git_protocol_error_response(
+                GitService::ReceivePack,
+                &stderr_str,
+            ));
+        }
+
+        // Transport errors (spawn failures, signals, etc.) remain as HTTP 500
         error!("Git receive-pack failed: {}", stderr_str);
         return Err(GitError::GitFailed(status.code()));
     }
diff --git a/src/git/mod.rs b/src/git/mod.rs
index b3fee69..1255b6f 100644
--- a/src/git/mod.rs
+++ b/src/git/mod.rs
@@ -253,7 +253,7 @@ pub fn update_ref(repo_path: &Path, ref_name: &str, commit_hash: &str) -> Result
         return Err(format!("git update-ref failed: {}", stderr));
     }
 
-    info!(
+    debug!(
         "Updated ref {} to {} in {}",
         ref_name,
         commit_hash,
diff --git a/src/git/sync.rs b/src/git/sync.rs
index 8401736..c24d16b 100644
--- a/src/git/sync.rs
+++ b/src/git/sync.rs
@@ -673,7 +673,7 @@ pub fn align_repository_with_state(repo_path: &Path, state: &RepositoryState) ->
         match git::update_ref(repo_path, ref_name, expected_commit) {
             Ok(()) => {
                 if current_commit.is_some() {
-                    info!(
+                    debug!(
                         "Updated {} to {} in {}",
                         ref_name,
                         expected_commit,
@@ -681,7 +681,7 @@ pub fn align_repository_with_state(repo_path: &Path, state: &RepositoryState) ->
                     );
                     result.refs_updated += 1;
                 } else {
-                    info!(
+                    debug!(
                         "Created {} at {} in {}",
                         ref_name,
                         expected_commit,
@@ -707,7 +707,7 @@ pub fn align_repository_with_state(repo_path: &Path, state: &RepositoryState) ->
             if let Some(head_commit) = state.get_branch_commit(branch_name) {
                 match git::try_set_head_if_available(repo_path, head_ref, head_commit) {
                     Ok(true) => {
-                        info!("Set HEAD to {} in {}", head_ref, repo_path.display());
+                        debug!("Set HEAD to {} in {}", head_ref, repo_path.display());
                         result.head_set = true;
                     }
                     Ok(false) => {