refactor: extract RunPrInputs validation; drop dead is_app_public

Two cleanups:

1. Extracted the 8 sequential let-Some-else-return validations from
   create_run_pull_request into a server-local RunPrInputs struct with
   an extract(&run_state, force) -> Result<RunPrInputs, ApiError>
   constructor. The handler shrinks from ~85 lines of validation +
   build to a single match RunPrInputs::extract(...) followed by
   creds + model + request build. All error codes/messages preserved.

2. Deleted is_app_public from fabro-github plus its 3 unit tests and
   the now-unused MockHeaderCheck::Missing / with_req_header_missing
   test-helper variants. No production caller remained after the
   server-side install flow stopped checking app visibility client-side.

Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4584 passed (down from 4587 by the 3
deleted is_app_public tests), 182 skipped.

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

lib/crates/fabro-github/src/lib.rs

@@ -842,38 +842,6 @@ pub async fn update_app_webhook_config(
     Ok(())
 }
 
-/// Check whether a GitHub App is publicly visible.
-///
-/// Calls `GET /apps/{slug}` **without** authentication. Public apps return 200,
-/// private apps return 404 to unauthenticated requests.
-pub async fn is_app_public(
-    client: &impl HttpClient,
-    slug: &str,
-    base_url: &str,
-) -> Result<bool, String> {
-    let url = format!("{base_url}/apps/{slug}");
-    let resp = client
-        .request(
-            HttpMethod::Get,
-            &url,
-            &[
-                ("Accept", "application/vnd.github+json"),
-                ("User-Agent", "fabro"),
-            ],
-            None,
-        )
-        .await
-        .map_err(|e| format!("Failed to check GitHub App visibility: {e}"))?;
-
-    match resp.status {
-        200 => Ok(true),
-        404 => Ok(false),
-        status => Err(format!(
-            "Unexpected status {status} checking GitHub App visibility"
-        )),
-    }
-}
-
 /// Resolve git clone credentials for a GitHub repository.
 ///
 /// Returns `(username, password)` for authenticated cloning.
@@ -1354,7 +1322,6 @@ mod tests {
 
     enum MockHeaderCheck {
         Equals(String),
-        Missing,
     }
 
     struct MockHttpClient {
@@ -1384,12 +1351,6 @@ mod tests {
             self
         }
 
-        fn with_req_header_missing(mut self, name: &str) -> Self {
-            self.routes.last_mut().unwrap().assert_header =
-                Some((name.to_string(), MockHeaderCheck::Missing));
-            self
-        }
-
         fn with_req_body(mut self, json_str: &str) -> Self {
             self.routes.last_mut().unwrap().assert_body_json =
                 Some(serde_json::from_str(json_str).unwrap());
@@ -1407,26 +1368,14 @@ mod tests {
         ) -> Result<HttpResponse, String> {
             for route in &self.routes {
                 if method == route.method && url.ends_with(&route.path) {
-                    if let Some((name, check)) = &route.assert_header {
-                        let found = headers.iter().find(|(k, _)| *k == name.as_str());
-                        match check {
-                            MockHeaderCheck::Equals(expected) => {
-                                let (_, v) = found.unwrap_or_else(|| {
-                                    panic!("Expected header '{name}' not found in request to {url}")
-                                });
-                                assert_eq!(
-                                    *v,
-                                    expected.as_str(),
-                                    "Header '{name}' mismatch for {url}"
-                                );
-                            }
-                            MockHeaderCheck::Missing => {
-                                assert!(
-                                    found.is_none(),
-                                    "Header '{name}' should be absent for {url}"
-                                );
-                            }
-                        }
+                    if let Some((name, MockHeaderCheck::Equals(expected))) = &route.assert_header {
+                        let (_, v) = headers
+                            .iter()
+                            .find(|(k, _)| *k == name.as_str())
+                            .unwrap_or_else(|| {
+                                panic!("Expected header '{name}' not found in request to {url}")
+                            });
+                        assert_eq!(*v, expected.as_str(), "Header '{name}' mismatch for {url}");
                     }
                     if let Some(expected_body) = &route.assert_body_json {
                         let actual = body.expect("Expected request body");
@@ -1782,46 +1731,6 @@ mod tests {
         );
     }
 
-    // -----------------------------------------------------------------------
-    // is_app_public
-    // -----------------------------------------------------------------------
-
-    #[tokio::test]
-    async fn is_app_public_returns_true_on_200() {
-        let mock = MockHttpClient::new().on(
-            HttpMethod::Get,
-            "/apps/my-fabro-app",
-            200,
-            r#"{"slug": "my-fabro-app"}"#,
-        );
-
-        let result = is_app_public(&mock, "my-fabro-app", "").await;
-        assert!(result.unwrap());
-    }
-
-    #[tokio::test]
-    async fn is_app_public_returns_false_on_404() {
-        let mock = MockHttpClient::new().on(HttpMethod::Get, "/apps/my-private-app", 404, "");
-
-        let result = is_app_public(&mock, "my-private-app", "").await;
-        assert!(!result.unwrap());
-    }
-
-    #[tokio::test]
-    async fn is_app_public_no_auth_header() {
-        let mock = MockHttpClient::new()
-            .on(
-                HttpMethod::Get,
-                "/apps/my-app",
-                200,
-                r#"{"slug": "my-app"}"#,
-            )
-            .with_req_header_missing("Authorization");
-
-        let result = is_app_public(&mock, "my-app", "").await;
-        assert!(result.unwrap());
-    }
-
     // -----------------------------------------------------------------------
     // get_pull_request
     // -----------------------------------------------------------------------

lib/crates/fabro-server/src/server.rs

@@ -5267,6 +5267,95 @@ async fn load_pull_request_github_context(
     })
 }
 
+struct RunPrInputs<'a> {
+    run_spec:          &'a fabro_types::RunSpec,
+    base_branch:       &'a str,
+    run_branch:        &'a str,
+    diff:              &'a str,
+    conclusion:        &'a fabro_types::Conclusion,
+    normalized_origin: String,
+}
+
+impl<'a> RunPrInputs<'a> {
+    fn extract(run_state: &'a fabro_store::RunProjection, force: bool) -> Result<Self, ApiError> {
+        if let Some(record) = run_state.pull_request.as_ref() {
+            return Err(ApiError::with_code(
+                StatusCode::CONFLICT,
+                format!("Pull request already exists at {}", record.html_url),
+                "pull_request_exists",
+            ));
+        }
+        let run_spec = run_state.spec.as_ref().ok_or_else(|| {
+            ApiError::new(
+                StatusCode::INTERNAL_SERVER_ERROR,
+                "Run spec missing from store.",
+            )
+        })?;
+        let origin_url = run_spec.repo_origin_url.as_deref().ok_or_else(|| {
+            ApiError::with_code(
+                StatusCode::BAD_REQUEST,
+                "Run has no repo origin URL — pull request creation requires git metadata.",
+                "missing_repo_origin",
+            )
+        })?;
+        let base_branch = run_spec.base_branch.as_deref().ok_or_else(|| {
+            ApiError::with_code(
+                StatusCode::BAD_REQUEST,
+                "Run has no base branch — pull request creation requires git metadata.",
+                "missing_base_branch",
+            )
+        })?;
+        let run_branch = run_state
+            .start
+            .as_ref()
+            .and_then(|start| start.run_branch.as_deref())
+            .ok_or_else(|| {
+                ApiError::with_code(
+                    StatusCode::BAD_REQUEST,
+                    "Run has no run_branch — was it run with git push enabled?",
+                    "missing_run_branch",
+                )
+            })?;
+        let diff = run_state
+            .final_patch
+            .as_deref()
+            .filter(|d| !d.trim().is_empty())
+            .ok_or_else(empty_pull_request_diff_error)?;
+        let conclusion = run_state.conclusion.as_ref().ok_or_else(|| {
+            ApiError::with_code(
+                StatusCode::BAD_REQUEST,
+                "Run is not finished yet.",
+                "run_not_finished",
+            )
+        })?;
+        if !force
+            && !matches!(
+                conclusion.status,
+                fabro_types::StageStatus::Success | fabro_types::StageStatus::PartialSuccess
+            )
+        {
+            return Err(ApiError::with_code(
+                StatusCode::BAD_REQUEST,
+                format!(
+                    "Run status is '{}', expected success or partial_success",
+                    conclusion.status
+                ),
+                "run_not_successful",
+            ));
+        }
+        let normalized_origin = fabro_github::ssh_url_to_https(origin_url);
+        parse_github_owner_repo_from_url(&normalized_origin, "repo origin URL")?;
+        Ok(Self {
+            run_spec,
+            base_branch,
+            run_branch,
+            diff,
+            conclusion,
+            normalized_origin,
+        })
+    }
+}
+
 async fn create_run_pull_request(
     AuthorizeRunScoped(id): AuthorizeRunScoped,
     State(state): State<Arc<AppState>>,
@@ -5275,7 +5364,6 @@ async fn create_run_pull_request(
     let Ok(run_store) = state.store.open_run(&id).await else {
         return ApiError::not_found("Run not found.").into_response();
     };
-
     let run_state = match run_store.state().await {
         Ok(run_state) => run_state,
         Err(err) => {
@@ -5283,94 +5371,14 @@ async fn create_run_pull_request(
                 .into_response();
         }
     };
-
-    if let Some(record) = run_state.pull_request.as_ref() {
-        return ApiError::with_code(
-            StatusCode::CONFLICT,
-            format!("Pull request already exists at {}", record.html_url),
-            "pull_request_exists",
-        )
-        .into_response();
-    }
-
-    let Some(run_spec) = run_state.spec.as_ref() else {
-        return ApiError::new(
-            StatusCode::INTERNAL_SERVER_ERROR,
-            "Run spec missing from store.",
-        )
-        .into_response();
+    let inputs = match RunPrInputs::extract(&run_state, body.force) {
+        Ok(inputs) => inputs,
+        Err(err) => return err.into_response(),
     };
-
-    let Some(origin_url) = run_spec.repo_origin_url.as_deref() else {
-        return ApiError::with_code(
-            StatusCode::BAD_REQUEST,
-            "Run has no repo origin URL — pull request creation requires git metadata.",
-            "missing_repo_origin",
-        )
-        .into_response();
-    };
-    let Some(base_branch) = run_spec.base_branch.as_deref() else {
-        return ApiError::with_code(
-            StatusCode::BAD_REQUEST,
-            "Run has no base branch — pull request creation requires git metadata.",
-            "missing_base_branch",
-        )
-        .into_response();
-    };
-    let Some(run_branch) = run_state
-        .start
-        .as_ref()
-        .and_then(|start| start.run_branch.as_deref())
-    else {
-        return ApiError::with_code(
-            StatusCode::BAD_REQUEST,
-            "Run has no run_branch — was it run with git push enabled?",
-            "missing_run_branch",
-        )
-        .into_response();
-    };
-
-    let Some(diff) = run_state.final_patch.as_deref() else {
-        return empty_pull_request_diff_error().into_response();
-    };
-    if diff.trim().is_empty() {
-        return empty_pull_request_diff_error().into_response();
-    }
-
-    let Some(conclusion) = run_state.conclusion.as_ref() else {
-        return ApiError::with_code(
-            StatusCode::BAD_REQUEST,
-            "Run is not finished yet.",
-            "run_not_finished",
-        )
-        .into_response();
-    };
-    if !body.force
-        && !matches!(
-            conclusion.status,
-            fabro_types::StageStatus::Success | fabro_types::StageStatus::PartialSuccess
-        )
-    {
-        return ApiError::with_code(
-            StatusCode::BAD_REQUEST,
-            format!(
-                "Run status is '{}', expected success or partial_success",
-                conclusion.status
-            ),
-            "run_not_successful",
-        )
-        .into_response();
-    }
-
-    let normalized_origin = fabro_github::ssh_url_to_https(origin_url);
-    if let Err(err) = parse_github_owner_repo_from_url(&normalized_origin, "repo origin URL") {
-        return err.into_response();
-    }
     let creds = match load_server_github_credentials(state.as_ref()) {
         Ok(creds) => creds,
         Err(err) => return err.into_response(),
     };
-
     let model = if let Some(model) = body.model {
         model
     } else {
@@ -5384,14 +5392,14 @@ async fn create_run_pull_request(
     let run_store_handle = run_store.clone().into();
     let request = pull_request::OpenPullRequestRequest::from_run_state(
         fabro_github::GitHubContext::new(&creds, state.github_api_base_url.as_str()),
-        run_spec,
-        run_branch,
-        &normalized_origin,
-        base_branch,
-        diff,
+        inputs.run_spec,
+        inputs.run_branch,
+        &inputs.normalized_origin,
+        inputs.base_branch,
+        inputs.diff,
         &model,
         &run_store_handle,
-        conclusion,
+        inputs.conclusion,
     );
     let pull_request = match pull_request::maybe_open_pull_request(request).await {
         Ok(Some(record)) => record,