mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-06 02:48:25 +00:00
refactor(pipeline): split build_pr_body signature; drop RunServices::for_cli
build_pr_body and maybe_open_pull_request now take the two things they actually need — run_store: &RunStoreHandle and llm_source: &dyn CredentialSource — instead of services: &RunServices. The workflow PULL_REQUEST phase decomposes services at the callsite; the standalone fabro pr create command passes its own directly. This removes RunServices::for_cli, a stub constructor that fabricated an emitter, sandbox, and provider just to satisfy the RunServices type for two fields it cared about. The "leaky fake" is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b01e08a52d
commit
7e83cd38e5
4 changed files with 22 additions and 42 deletions
|
|
@ -3,7 +3,6 @@ use fabro_model::Catalog;
|
|||
use fabro_sandbox::daytona::detect_repo_info;
|
||||
use fabro_workflow::outcome::StageStatus;
|
||||
use fabro_workflow::pull_request::maybe_open_pull_request;
|
||||
use fabro_workflow::services::RunServices;
|
||||
use tracing::info;
|
||||
|
||||
use crate::args::PrCreateArgs;
|
||||
|
|
@ -100,8 +99,7 @@ pub(super) async fn create_command(args: PrCreateArgs, base_ctx: &CommandContext
|
|||
.id
|
||||
.clone()
|
||||
});
|
||||
let pr_services = RunServices::for_cli(run_store.clone().into(), llm_source);
|
||||
|
||||
let run_store_handle = run_store.clone().into();
|
||||
let pull_request = maybe_open_pull_request(
|
||||
&creds,
|
||||
&origin_url,
|
||||
|
|
@ -112,7 +110,8 @@ pub(super) async fn create_command(args: PrCreateArgs, base_ctx: &CommandContext
|
|||
&model,
|
||||
true,
|
||||
None,
|
||||
pr_services.as_ref(),
|
||||
&run_store_handle,
|
||||
llm_source.as_ref(),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
use std::sync::Arc;
|
||||
|
||||
use fabro_auth::CredentialSource;
|
||||
use fabro_github::{self as github_app, GitHubCredentials, ssh_url_to_https};
|
||||
use fabro_graphviz::parser;
|
||||
use fabro_llm::client::Client;
|
||||
|
|
@ -16,7 +17,6 @@ use crate::event::{Event, RunNoticeLevel};
|
|||
use crate::outcome::{StageStatus, format_cost as outcome_format_cost};
|
||||
use crate::records::{Conclusion, RunSpec};
|
||||
use crate::runtime_store::RunStoreHandle;
|
||||
use crate::services::RunServices;
|
||||
|
||||
/// Derive a PR title from the workflow goal.
|
||||
///
|
||||
|
|
@ -292,22 +292,15 @@ pub async fn build_pr_body(
|
|||
diff: &str,
|
||||
goal: &str,
|
||||
model: &str,
|
||||
services: &RunServices,
|
||||
run_store: &RunStoreHandle,
|
||||
llm_source: &dyn CredentialSource,
|
||||
conclusion: Option<&Conclusion>,
|
||||
) -> Result<String, String> {
|
||||
let client = Client::from_source(services.llm_source.as_ref())
|
||||
let client = Client::from_source(llm_source)
|
||||
.await
|
||||
.map_err(|e| format!("Failed to create LLM client: {e}"))?;
|
||||
|
||||
build_pr_body_with_client(
|
||||
diff,
|
||||
goal,
|
||||
model,
|
||||
&services.run_store,
|
||||
conclusion,
|
||||
Arc::new(client),
|
||||
)
|
||||
.await
|
||||
build_pr_body_with_client(diff, goal, model, run_store, conclusion, Arc::new(client)).await
|
||||
}
|
||||
|
||||
async fn build_pr_body_with_client(
|
||||
|
|
@ -424,7 +417,8 @@ pub async fn maybe_open_pull_request(
|
|||
model: &str,
|
||||
draft: bool,
|
||||
auto_merge: Option<AutoMergeOptions>,
|
||||
services: &RunServices,
|
||||
run_store: &RunStoreHandle,
|
||||
llm_source: &dyn CredentialSource,
|
||||
conclusion: Option<&Conclusion>,
|
||||
) -> Result<Option<PullRequestRecord>, String> {
|
||||
if diff.is_empty() {
|
||||
|
|
@ -435,7 +429,7 @@ pub async fn maybe_open_pull_request(
|
|||
let https_url = ssh_url_to_https(origin_url);
|
||||
let (owner, repo) = github_app::parse_github_owner_repo(&https_url)?;
|
||||
|
||||
let body = build_pr_body(diff, goal, model, services, conclusion).await?;
|
||||
let body = build_pr_body(diff, goal, model, run_store, llm_source, conclusion).await?;
|
||||
let body = truncate_pr_body(&body);
|
||||
|
||||
let title = pr_title_from_goal(goal);
|
||||
|
|
@ -546,7 +540,8 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
|
|||
&options.model,
|
||||
pr_cfg.draft,
|
||||
auto_merge,
|
||||
&services,
|
||||
&services.run_store,
|
||||
services.llm_source.as_ref(),
|
||||
Some(&conclusion),
|
||||
)
|
||||
.await
|
||||
|
|
@ -1329,13 +1324,14 @@ mod tests {
|
|||
|
||||
let store = test_store();
|
||||
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
|
||||
let services = RunServices::for_cli(run_store.into(), llm_source);
|
||||
let run_store_handle: RunStoreHandle = run_store.into();
|
||||
|
||||
let body = build_pr_body(
|
||||
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
|
||||
"Implement feature",
|
||||
"gpt-5.4",
|
||||
services.as_ref(),
|
||||
&run_store_handle,
|
||||
llm_source.as_ref(),
|
||||
Some(&make_test_conclusion()),
|
||||
)
|
||||
.await
|
||||
|
|
@ -1464,8 +1460,8 @@ mod tests {
|
|||
async fn empty_diff_returns_none() {
|
||||
let store = test_store();
|
||||
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
|
||||
let run_store_handle: RunStoreHandle = run_store.into();
|
||||
let llm_source = test_llm_source();
|
||||
let services = RunServices::for_cli(run_store.clone().into(), llm_source);
|
||||
let creds = GitHubCredentials::App(fabro_github::GitHubAppCredentials {
|
||||
app_id: "123".to_string(),
|
||||
private_key_pem: "unused".to_string(),
|
||||
|
|
@ -1480,7 +1476,8 @@ mod tests {
|
|||
"claude-sonnet-4-20250514",
|
||||
false,
|
||||
None,
|
||||
services.as_ref(),
|
||||
&run_store_handle,
|
||||
llm_source.as_ref(),
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
|
|
|
|||
|
|
@ -70,23 +70,6 @@ impl RunServices {
|
|||
.await
|
||||
}
|
||||
|
||||
/// CLI helper: minimal cross-phase services for PR generation and similar
|
||||
/// source-backed operations outside the workflow executor.
|
||||
#[must_use]
|
||||
pub fn for_cli(run_store: RunStoreHandle, llm_source: Arc<dyn CredentialSource>) -> Arc<Self> {
|
||||
Self::new(
|
||||
run_store,
|
||||
Arc::new(Emitter::default()),
|
||||
Arc::new(fabro_agent::LocalSandbox::new(
|
||||
std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
)),
|
||||
None,
|
||||
None,
|
||||
Provider::Anthropic,
|
||||
llm_source,
|
||||
)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_run_store(self: &Arc<Self>, run_store: RunStoreHandle) -> Arc<Self> {
|
||||
Arc::new(Self {
|
||||
|
|
|
|||
|
|
@ -6734,13 +6734,14 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
|
|||
None,
|
||||
));
|
||||
let run_store = store.open_run_reader(&run_options.run_id).await.unwrap();
|
||||
let services = fabro_workflow::services::RunServices::for_cli(run_store.into(), llm_source);
|
||||
let run_store_handle: fabro_workflow::runtime_store::RunStoreHandle = run_store.into();
|
||||
|
||||
let body = fabro_workflow::pull_request::build_pr_body(
|
||||
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
|
||||
"Implement feature",
|
||||
"gpt-5.4",
|
||||
services.as_ref(),
|
||||
&run_store_handle,
|
||||
llm_source.as_ref(),
|
||||
Some(&Conclusion {
|
||||
timestamp: Utc::now(),
|
||||
status: StageStatus::Success,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue