scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Drop the branch-head compatibility wrapper

Browse source 
`branch_head_sha` existed to keep callers compiling while the branch-head
lookup moved onto the repository reader. Its last caller now opens a
reader directly, so the wrapper only made the typed API worse: it joined
an already-validated owner and repo into a slug so the reader could split
them apart again, invented an "invalid repository coordinate" error for a
value validated upstream, opened a fresh credential session per call, and
flattened `RepositoryReadError` into `anyhow` while keeping one variant —
leaving callers unable to tell a rate limit from a rejected token.

Its integration test pinned the wrapper rather than the behavior. Replace
it with one that asserts the same 404-means-not-observable semantics
through `resolve_commit`, which is where that contract actually lives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-08-19 15:43:08 -04:00
parent 8dfda7ede0
commit 3ab584691d
2 changed files with 14 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
lib/components/fabro-github/src/lib.rs
View file

@ -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<Option<String>> {
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`.

30
lib/components/fabro-github/tests/integration.rs
View file

@ -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;
}