scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-09 03:20:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

refactor(github): take GitHubContext by reference in public API

Browse source 
Public functions now take ctx: &GitHubContext<'_> instead of by-value
GitHubContext<'_>. Matches the surrounding &str / &GitHubCredentials
convention. The type stays Copy so internal call sites that pass `ctx`
through still work without explicit reborrows.

Touched: 8 fabro-github functions + matching _with_client variants,
plus call sites in fabro-server, fabro-workflow, fabro-sandbox, and
fabro-github's integration + unit tests. Pure mechanical change.

Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4581 passed, 182 skipped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-24 09:15:25 -04:00
parent 042fca0dec
commit 12ca595f54
No known key found for this signature in database
7 changed files with 41 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
lib/crates/fabro-github/src/lib.rs
View file

@ -471,7 +471,7 @@ pub struct CreatedPullRequest {
reason = "Creating a pull request needs explicit repo, branch, and body fields."
)]
pub async fn create_pull_request(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
base: &str,
@ -568,7 +568,7 @@ fn merge_method_as_graphql_value(method: fabro_types::MergeMethod) -> &'static s
/// Requires the PR's `node_id` (from the REST API response) and a merge method.
/// The repository must have auto-merge enabled in its settings.
pub async fn enable_auto_merge(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
pr_node_id: &str,
@ -695,7 +695,7 @@ fn normalize_https_host_path(url: &str) -> String {
/// Uses a GitHub App installation token to query the branches API.
/// Returns `true` if the branch exists, `false` if it doesn't (404).
pub async fn branch_exists(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
branch: &str,
@ -706,7 +706,7 @@ pub async fn branch_exists(
async fn branch_exists_with_client(
client: &impl HttpClient,
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
branch: &str,
@ -848,7 +848,7 @@ pub async fn update_app_webhook_config(
/// Always generates a token regardless of repo visibility, since the token
/// is needed for pushing from the sandbox.
pub async fn resolve_clone_credentials(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
) -> Result<(Option<String>, Option<String>), String> {
@ -883,7 +883,7 @@ pub fn embed_token_in_url(url: &str, token: &str) -> String {
/// Parses owner/repo from the URL, obtains a fresh installation access token,
/// and returns the URL with embedded credentials.
pub async fn resolve_authenticated_url(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
url: &str,
) -> Result<String, String> {
let (owner, repo) = parse_github_owner_repo(url)?;
@ -896,7 +896,7 @@ pub async fn resolve_authenticated_url(
/// Fetch detailed information about a pull request.
pub async fn get_pull_request(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
number: u64,
@ -907,7 +907,7 @@ pub async fn get_pull_request(
async fn get_pull_request_with_client(
client: &impl HttpClient,
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
number: u64,
@ -963,7 +963,7 @@ async fn get_pull_request_with_client(
/// Merge a pull request.
pub async fn merge_pull_request(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
number: u64,
@ -975,7 +975,7 @@ pub async fn merge_pull_request(
async fn merge_pull_request_with_client(
client: &impl HttpClient,
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
number: u64,
@ -1029,7 +1029,7 @@ async fn merge_pull_request_with_client(
/// Close a pull request.
pub async fn close_pull_request(
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
number: u64,
@ -1040,7 +1040,7 @@ pub async fn close_pull_request(
async fn close_pull_request_with_client(
client: &impl HttpClient,
ctx: GitHubContext<'_>,
ctx: &GitHubContext<'_>,
owner: &str,
repo: &str,
number: u64,
@ -1549,7 +1549,7 @@ mod tests {
});
let result = branch_exists_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
"my-branch",
@ -1587,7 +1587,7 @@ mod tests {
});
let result = branch_exists_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
"no-such-branch",
@ -1625,7 +1625,7 @@ mod tests {
});
let result = branch_exists_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
"broken",
@ -1648,7 +1648,7 @@ mod tests {
let creds = GitHubCredentials::Token("ghu_test".to_string());
let result = branch_exists_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
"my-branch",
@ -1786,7 +1786,7 @@ mod tests {
});
let detail = get_pull_request_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
42,
@ -1831,7 +1831,7 @@ mod tests {
});
let err = get_pull_request_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
999,
@ -1865,7 +1865,7 @@ mod tests {
let creds = GitHubCredentials::Token("ghu_test".to_string());
let detail = get_pull_request_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
42,
@ -1881,7 +1881,7 @@ mod tests {
let creds = GitHubCredentials::Token("ghu_test".to_string());
let credentials =
resolve_clone_credentials(GitHubContext::new(&creds, ""), "owner", "repo")
resolve_clone_credentials(&GitHubContext::new(&creds, ""), "owner", "repo")
.await
.unwrap();
@ -1927,7 +1927,7 @@ mod tests {
});
merge_pull_request_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
42,
@ -1961,7 +1961,7 @@ mod tests {
});
let err = merge_pull_request_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
42,
@ -1996,7 +1996,7 @@ mod tests {
});
let err = merge_pull_request_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
42,
@ -2038,7 +2038,7 @@ mod tests {
app_id: "test".to_string(),
private_key_pem: pem.to_string(),
});
close_pull_request_with_client(&mock, GitHubContext::new(&creds, ""), "owner", "repo", 42)
close_pull_request_with_client(&mock, &GitHubContext::new(&creds, ""), "owner", "repo", 42)
.await
.unwrap();
}
@ -2067,7 +2067,7 @@ mod tests {
});
let err = close_pull_request_with_client(
&mock,
GitHubContext::new(&creds, ""),
&GitHubContext::new(&creds, ""),
"owner",
"repo",
999,

12
lib/crates/fabro-github/tests/integration.rs
View file

@ -39,7 +39,7 @@ fn standard_app_state() -> GitHubAppState {
async fn create_and_get_pull_request() {
let twin = TwinGitHub::start(standard_app_state()).await;
let creds = github_credentials();
let ctx = GitHubContext::new(&creds, &twin.base_url);
let ctx = &GitHubContext::new(&creds, &twin.base_url);
let created = create_pull_request(
ctx,
@ -70,7 +70,7 @@ async fn create_and_get_pull_request() {
async fn create_merge_and_verify_state() {
let twin = TwinGitHub::start(standard_app_state()).await;
let creds = github_credentials();
let ctx = GitHubContext::new(&creds, &twin.base_url);
let ctx = &GitHubContext::new(&creds, &twin.base_url);
let created = create_pull_request(
ctx, "acme", "widgets", "main", "feature", "Merge me", "PR body", false,
@ -97,7 +97,7 @@ async fn create_close_and_verify_state() {
let twin = TwinGitHub::start(standard_app_state()).await;
let creds = github_credentials();
let ctx = GitHubContext::new(&creds, &twin.base_url);
let ctx = &GitHubContext::new(&creds, &twin.base_url);
let created = create_pull_request(
ctx, "acme", "widgets", "main", "feature", "Close me", "PR body", false,
@ -123,7 +123,7 @@ async fn enable_auto_merge_persists() {
let twin = TwinGitHub::start(standard_app_state()).await;
let creds = github_credentials();
let ctx = GitHubContext::new(&creds, &twin.base_url);
let ctx = &GitHubContext::new(&creds, &twin.base_url);
let created = create_pull_request(
ctx,
@ -187,7 +187,7 @@ async fn resolve_authenticated_url_embeds_token() {
let creds = github_credentials();
let url = resolve_authenticated_url(
GitHubContext::new(&creds, &twin.base_url),
&GitHubContext::new(&creds, &twin.base_url),
"https://github.com/acme/widgets.git",
)
.await
@ -205,7 +205,7 @@ async fn resolve_authenticated_url_errors_on_non_github_url() {
let creds = github_credentials();
let error = resolve_authenticated_url(
GitHubContext::new(&creds, &twin.base_url),
&GitHubContext::new(&creds, &twin.base_url),
"https://gitlab.com/foo/bar",
)
.await

4
lib/crates/fabro-sandbox/src/daytona/mod.rs
View file

@ -534,7 +534,7 @@ impl Sandbox for DaytonaSandbox {
err
})?;
fabro_github::resolve_clone_credentials(
fabro_github::GitHubContext::new(
&fabro_github::GitHubContext::new(
creds,
&fabro_github::github_api_base_url(),
),
@ -816,7 +816,7 @@ impl Sandbox for DaytonaSandbox {
};
let auth_url = fabro_github::resolve_authenticated_url(
fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()),
&fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()),
origin_url,
)
.await

6
lib/crates/fabro-server/src/server.rs
View file

@ -5438,7 +5438,7 @@ async fn get_run_pull_request(
};
match fabro_github::get_pull_request(
fabro_github::GitHubContext::new(&ctx.creds, state.github_api_base_url.as_str()),
&fabro_github::GitHubContext::new(&ctx.creds, state.github_api_base_url.as_str()),
&ctx.owner,
&ctx.repo,
ctx.record.number,
@ -5468,7 +5468,7 @@ async fn merge_run_pull_request(
};
match fabro_github::merge_pull_request(
fabro_github::GitHubContext::new(&ctx.creds, state.github_api_base_url.as_str()),
&fabro_github::GitHubContext::new(&ctx.creds, state.github_api_base_url.as_str()),
&ctx.owner,
&ctx.repo,
ctx.record.number,
@ -5500,7 +5500,7 @@ async fn close_run_pull_request(
};
match fabro_github::close_pull_request(
fabro_github::GitHubContext::new(&ctx.creds, state.github_api_base_url.as_str()),
&fabro_github::GitHubContext::new(&ctx.creds, state.github_api_base_url.as_str()),
&ctx.owner,
&ctx.repo,
ctx.record.number,

4
lib/crates/fabro-workflow/src/pipeline/pull_request.rs
View file

@ -467,7 +467,7 @@ pub async fn maybe_open_pull_request(
let title = pr_title_from_goal(req.goal);
let created = github_app::create_pull_request(
req.github,
&req.github,
&owner,
&repo,
req.base_branch,
@ -487,7 +487,7 @@ pub async fn maybe_open_pull_request(
MergeStrategy::Rebase => fabro_types::MergeMethod::Rebase,
};
match github_app::enable_auto_merge(
req.github,
&req.github,
&owner,
&repo,
&created.node_id,

2
lib/crates/fabro-workflow/src/sandbox_git.rs
View file

@ -151,7 +151,7 @@ pub async fn git_push_host(
let https_url = fabro_github::ssh_url_to_https(&origin_url);
let push_url = if let Some(creds) = github_app {
match fabro_github::resolve_authenticated_url(
fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()),
&fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()),
&https_url,
)
.await

4
lib/crates/fabro-workflow/tests/it/daytona_integration.rs
View file

@ -1493,7 +1493,7 @@ async fn daytona_clone_public_repo_gets_credentials() {
// Directly test resolve_clone_credentials against a repo in an org where the
// app is installed
let (username, password) = fabro_github::resolve_clone_credentials(
fabro_github::GitHubContext::new(&creds, &fabro_github::github_api_base_url()),
&fabro_github::GitHubContext::new(&creds, &fabro_github::github_api_base_url()),
"fabro-sh",
"fabro",
)
@ -1518,7 +1518,7 @@ async fn daytona_iat_not_installed_gives_clear_error() {
let creds = load_github_app_credentials();
let result = fabro_github::resolve_clone_credentials(
fabro_github::GitHubContext::new(&creds, &fabro_github::github_api_base_url()),
&fabro_github::GitHubContext::new(&creds, &fabro_github::github_api_base_url()),
"torvalds",
"linux",
)