diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index 007d06c66..e46836b04 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -4,8 +4,8 @@ use base64::engine::general_purpose::STANDARD; use chrono::{DateTime, Utc}; use fabro_redact::DisplaySafeUrl; use fabro_static::EnvVars; +use fabro_types::PullRequestGithubDetail; use fabro_types::settings::run::MergeStrategy; -use fabro_types::{GitHubRepositorySlug, PullRequestGithubDetail}; use serde::Deserialize; use tokio::process::Command; @@ -993,29 +993,6 @@ fn normalize_https_host_path(url: &str) -> String { } } -/// Return the commit SHA at the head of a GitHub branch. -/// -/// Returns `None` when GitHub responds with 404. GitHub also uses 404 to hide -/// some private resources from credentials that cannot access them, so `None` -/// means the branch was not observable rather than proving it does not exist. -pub async fn branch_head_sha( - ctx: &GitHubContext<'_>, - owner: &str, - repo: &str, - branch: &str, -) -> anyhow::Result> { - let repository = GitHubRepositorySlug::try_new(&format!("{owner}/{repo}")) - .ok_or_else(|| anyhow!("Invalid GitHub repository coordinate"))?; - let reader = GitHubRepositoryReader::open(ctx, &repository) - .await - .context("Failed to read remote branch head")?; - match reader.resolve_commit(&format!("heads/{branch}")).await { - Ok(sha) => Ok(Some(sha)), - Err(RepositoryReadError::NotFound { .. }) => Ok(None), - Err(error) => Err(anyhow::Error::new(error).context("Failed to read remote branch head")), - } -} - /// Check whether a GitHub App is installed for a specific repository. /// /// Uses the App JWT to query `GET /repos/{owner}/{repo}/installation`. diff --git a/lib/components/fabro-github/tests/integration.rs b/lib/components/fabro-github/tests/integration.rs index c99131393..006f997b3 100644 --- a/lib/components/fabro-github/tests/integration.rs +++ b/lib/components/fabro-github/tests/integration.rs @@ -2,7 +2,7 @@ use std::error::Error as _; use fabro_github::{ GitHubAppCredentials, GitHubContext, GitHubCredentials, GitHubRepositoryReader, - InstallationToken, RepositoryReadError, branch_head_sha, close_pull_request, + InstallationToken, RepositoryReadError, close_pull_request, create_installation_access_token_for_pr, create_pull_request, enable_auto_merge, get_pull_request, merge_pull_request, resolve_authenticated_url, sign_app_jwt, }; @@ -464,27 +464,23 @@ async fn expired_installation_token_keeps_credential_error_source() { } #[fabro_macros::e2e_test(twin)] -async fn branch_head_compatibility_wrapper_uses_repository_reader() { +async fn branch_head_resolution_distinguishes_missing_from_present() { let twin = TwinGitHub::start(standard_app_state()).await; let credentials = github_credentials(); - let context = GitHubContext::with_http_client( - &credentials, - &twin.base_url, - fabro_test::test_http_client(), - ); + let reader = open_reader(&twin.base_url, &credentials).await.unwrap(); assert_eq!( - branch_head_sha(&context, "acme", "widgets", "release") - .await - .unwrap(), - Some(HEAD_SHA.to_string()) - ); - assert_eq!( - branch_head_sha(&context, "acme", "widgets", "missing") - .await - .unwrap(), - None + reader.resolve_commit("heads/release").await.unwrap(), + HEAD_SHA ); + // GitHub answers an absent branch with 404, which the reader reports as + // "not observable" rather than proving the branch does not exist. + assert!(matches!( + reader.resolve_commit("heads/missing").await, + Err(RepositoryReadError::NotFound { + operation: fabro_github::Operation::Revision, + }) + )); twin.shutdown().await; }