fix(workflow): verify remote head before generating PR content, tolerate replica lag

Three follow-ups from the efficiency review of the publish pipeline.

Check the branch before spending an LLM call:
`open_pull_request` generated the PR title and body first and only then
verified the remote branch pointed at the run's final commit. Every stale
branch therefore cost a full content generation before failing. The
verification is the cheap check, so it now runs first.

Tolerate GitHub read-after-write lag:
`GET /repos/{owner}/{repo}/branches/{branch}` is replica-served and can briefly
report the previous commit, or 404 for a branch that is new on the remote,
right after the push publish just made. It was read once with no retry. Since
publish failures are terminal, a replica that had not caught up yet would
discard a fully successful run. It is now read up to three times.

These two land together on purpose: the LLM call was the only thing buying
slack against the race, so reordering without the retry would have made it
more likely.

Keep commit SHAs out of failure classification:
`classify_failure_reason` substring-matches bare "500", "502", "503" and "504"
as transient-infra hints. Both publish messages embed a commit SHA, and a
40-char hex string contains one of those often enough to matter, so a
deterministic failure could be reported as transient. Long hex runs are now
masked before matching; the three-digit status codes those hints look for are
too short to be affected. The hex regex is shared with
`normalize_failure_reason`, which already had its own copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

lib/components/fabro-workflow/src/error.rs

@@ -1,5 +1,5 @@
 use std::fmt;
-use std::sync::Arc;
+use std::sync::{Arc, LazyLock};
 
 use fabro_graphviz::Error as GraphvizError;
 use fabro_llm::{Error as LlmError, ProviderErrorKind};
@@ -11,6 +11,7 @@ use fabro_types::settings::AmbiguousModelRef;
 use fabro_types::{ExecOutputTail, FailureReason, RunFailure};
 use fabro_util::error::{SharedError, collect_causes, collect_chain, render_with_causes};
 use fabro_validate::Diagnostic;
+use regex::Regex;
 use thiserror::Error as ThisError;
 
 use crate::outcome::{FailureDetail, Outcome, StageOutcome};
@@ -153,13 +154,21 @@ impl miette::Diagnostic for SharedTemplateError {
     }
 }
 
+/// Matches git SHAs and other long hex blobs.
+static HEX_RE: LazyLock<Regex> =
+    LazyLock::new(|| Regex::new(r"\b[0-9a-f]{7,64}\b").expect("hardcoded regex should compile"));
+
 /// Classify a failure reason string using heuristics.
 ///
 /// This is the fallback when structured error information is not available
 /// (e.g. for `Handler(String)` or `Engine(String)` errors).
 #[must_use]
 pub fn classify_failure_reason(reason: &str) -> FailureCategory {
-    let lower = reason.to_lowercase();
+    // Mask commit SHAs first. They are hex, so one contains "500" or "503"
+    // often enough to matter, which would read as a transient infra hint. The
+    // bare status codes those hints look for are too short to be masked.
+    let lowered = reason.to_lowercase();
+    let lower = HEX_RE.replace_all(&lowered, "<hex>");
 
     if lower.contains("interrupt")
         || (lower.contains("cancel")
@@ -196,13 +205,6 @@ pub fn classify_failure_reason(reason: &str) -> FailureCategory {
 /// semantically identical errors produce the same signature regardless of
 /// line numbers, commit hashes, or timestamps.
 pub fn normalize_failure_reason(reason: &str) -> String {
-    use std::sync::LazyLock;
-
-    use regex::Regex;
-
-    static HEX_RE: LazyLock<Regex> = LazyLock::new(|| {
-        Regex::new(r"\b[0-9a-f]{7,64}\b").expect("hardcoded regex should compile")
-    });
     static DIGITS_RE: LazyLock<Regex> =
         LazyLock::new(|| Regex::new(r"\b\d+\b").expect("hardcoded regex should compile"));
     static COMMA_SPACE_RE: LazyLock<Regex> =
@@ -2085,6 +2087,23 @@ mod tests {
         }
     }
 
+    /// Commit SHAs are hex, so they contain digit runs like "503" often enough
+    /// to matter. Masking them keeps a deterministic failure from being
+    /// reported as transient just because of the SHA it names.
+    #[test]
+    fn commit_shas_do_not_trip_transient_infra_hints() {
+        let sha = "a503b1c9d4e2f7a8b6c3d0e1f2a3b4c5d6e7f8a9";
+        assert_eq!(
+            classify_failure_reason(&format!("failed to push final commit {sha} to branch 'x'")),
+            FailureCategory::Deterministic
+        );
+        // A real status code is still a transient hint.
+        assert_eq!(
+            classify_failure_reason("push rejected with 503"),
+            FailureCategory::TransientInfra
+        );
+    }
+
     #[test]
     fn to_fail_outcome_preserves_class() {
         let err = Error::handler("timeout");

lib/components/fabro-workflow/src/pipeline/pull_request.rs

@@ -1,5 +1,6 @@
 use std::collections::HashSet;
 use std::sync::{Arc, LazyLock};
+use std::time::Duration;
 
 use fabro_auth::CredentialSource;
 use fabro_github::{self as github_app, ssh_url_to_https};
@@ -11,6 +12,7 @@ use fabro_store::RunProjection;
 use fabro_types::PullRequestLink;
 use fabro_types::settings::run::MergeStrategy;
 use fabro_util::text::strip_goal_decoration;
+use tokio::time::sleep;
 use tracing::{debug, info, warn};
 
 use crate::outcome::format_cost as outcome_format_cost;
@@ -470,6 +472,54 @@ pub struct CreatedPullRequest {
     pub head_branch: String,
 }
 
+/// How many times to read the remote branch head before giving up.
+///
+/// `GET /repos/{owner}/{repo}/branches/{branch}` is replica-served, so shortly
+/// after the push that publish just made it can still report the previous
+/// commit — or 404 for a branch that is new on the remote.
+const BRANCH_HEAD_ATTEMPTS: u32 = 3;
+const BRANCH_HEAD_RETRY_DELAY: Duration = Duration::from_millis(500);
+
+/// Confirm the remote branch points at the run's final commit.
+///
+/// Publish failures are terminal, so a replica that has not caught up yet must
+/// not be mistaken for a genuinely stale branch.
+async fn verify_remote_head(
+    req: &OpenPullRequestRequest<'_>,
+    owner: &str,
+    repo: &str,
+) -> Result<(), String> {
+    let mut last_seen = Ok(None);
+    for attempt in 1..=BRANCH_HEAD_ATTEMPTS {
+        last_seen = github_app::branch_head_sha(&req.github, owner, repo, req.head_branch).await;
+        match &last_seen {
+            Ok(Some(head)) if head == req.expected_head_sha => return Ok(()),
+            Ok(head) => debug!(
+                attempt,
+                head = ?head,
+                expected = req.expected_head_sha,
+                "Remote branch head does not match the final commit yet"
+            ),
+            Err(err) => debug!(attempt, error = %err, "Failed to read remote branch head"),
+        }
+        if attempt < BRANCH_HEAD_ATTEMPTS {
+            sleep(BRANCH_HEAD_RETRY_DELAY).await;
+        }
+    }
+
+    Err(match last_seen {
+        Ok(Some(head)) => format!(
+            "remote branch '{}' points to commit {head}, expected final commit {}",
+            req.head_branch, req.expected_head_sha
+        ),
+        Ok(None) => format!(
+            "remote branch '{}' does not exist; expected final commit {}",
+            req.head_branch, req.expected_head_sha
+        ),
+        Err(err) => format!("failed to verify remote branch head: {err:#}"),
+    })
+}
+
 /// Open a pull request for a completed run.
 ///
 /// Callers are responsible for skipping runs with an empty diff; reaching here
@@ -481,6 +531,10 @@ pub async fn open_pull_request(
     let (owner, repo) =
         github_app::parse_github_owner_repo(&https_url).map_err(|err| format!("{err:#}"))?;
 
+    // Verify before generating content: this is the cheap check, and a stale
+    // branch would otherwise cost a full LLM call before failing.
+    verify_remote_head(&req, &owner, &repo).await?;
+
     let content = build_pr_content(
         req.diff,
         req.goal,
@@ -496,25 +550,6 @@ pub async fn open_pull_request(
     let body = truncate_pr_body(&content.body);
     let title = content.title;
 
-    let remote_head = github_app::branch_head_sha(&req.github, &owner, &repo, req.head_branch)
-        .await
-        .map_err(|err| format!("failed to verify remote branch head: {err:#}"))?;
-    match remote_head {
-        Some(remote_head) if remote_head == req.expected_head_sha => {}
-        Some(remote_head) => {
-            return Err(format!(
-                "remote branch '{}' points to commit {remote_head}, expected final commit {}",
-                req.head_branch, req.expected_head_sha
-            ));
-        }
-        None => {
-            return Err(format!(
-                "remote branch '{}' does not exist; expected final commit {}",
-                req.head_branch, req.expected_head_sha
-            ));
-        }
-    }
-
     let created = github_app::create_pull_request(
         &req.github,
         &owner,
@@ -1412,11 +1447,13 @@ mod tests {
 
         assert!(error.contains("stale-sha"));
         assert!(error.contains("final-sha"));
-        httpmock::Mock::new(harness.openai_mock_id, &harness.openai_server)
-            .assert_async()
-            .await;
+        // The branch is re-read to ride out replica lag...
         httpmock::Mock::new(harness.branch_mock_id, &harness.github_server)
-            .assert_async()
+            .assert_calls_async(BRANCH_HEAD_ATTEMPTS as usize)
+            .await;
+        // ...but the check runs first, so no LLM call and no PR creation.
+        httpmock::Mock::new(harness.openai_mock_id, &harness.openai_server)
+            .assert_calls_async(0)
             .await;
         httpmock::Mock::new(harness.github_mock_id, &harness.github_server)
             .assert_calls_async(0)