Cut PR and diff readers over to the run store

This commit is contained in:
Bryan Helmkamp 2026-04-01 20:51:36 -04:00
parent d71a5c5d23
commit 991c5eb956
No known key found for this signature in database
10 changed files with 180 additions and 33 deletions

View file

@ -83,8 +83,17 @@ async fn create_from(
.as_deref()
.context("Run has no run_branch — was it run with git push enabled?")?;
let diff = std::fs::read_to_string(run_dir.join("final.patch"))
.context("Failed to read final.patch — no diff available")?;
let diff = match run_store.as_ref() {
Some(run_store) => run_store
.get_final_patch()
.await
.ok()
.flatten()
.or_else(|| std::fs::read_to_string(run_dir.join("final.patch")).ok())
.context("Failed to read final.patch — no diff available")?,
None => std::fs::read_to_string(run_dir.join("final.patch"))
.context("Failed to read final.patch — no diff available")?,
};
if diff.trim().is_empty() {
bail!("final.patch is empty — nothing to create a PR for");
}
@ -148,6 +157,11 @@ async fn create_from(
match record {
Some(record) => {
info!(pr_url = %record.html_url, "Pull request created");
if let Some(run_store) = run_store.as_ref() {
if let Err(err) = run_store.put_pull_request(&record).await {
tracing::warn!(error = %err, "Failed to persist pull request in run store");
}
}
if let Err(err) = record.save(&run_dir.join("pull_request.json")) {
tracing::warn!(error = %err, "Failed to save pull_request.json");
}

View file

@ -2,7 +2,7 @@ use std::path::Path;
use anyhow::{Context, Result};
use fabro_config::FabroSettingsExt;
use fabro_workflow::pull_request::PullRequestRecord;
use fabro_types::PullRequestRecord;
use fabro_workflow::run_lookup::{runs_base, scan_runs_combined};
use futures::future::join_all;
use serde::Serialize;
@ -50,6 +50,12 @@ async fn list_from(
let mut entries: Vec<(String, PullRequestRecord)> = Vec::new();
for run in &runs {
if let Ok(Some(run_store)) = store.open_run_reader(&run.run_id).await {
if let Ok(Some(record)) = run_store.get_pull_request().await {
entries.push((run.run_id.to_string(), record));
continue;
}
}
let pr_path = run.path.join("pull_request.json");
if let Ok(content) = std::fs::read_to_string(&pr_path) {
if let Ok(record) = serde_json::from_str::<PullRequestRecord>(&content) {

View file

@ -8,7 +8,7 @@ use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use fabro_workflow::pull_request::PullRequestRecord;
use fabro_types::PullRequestRecord;
use fabro_workflow::run_lookup::resolve_run_combined;
use crate::args::{GlobalArgs, PrCommand, PrNamespace};
@ -35,9 +35,14 @@ pub(crate) async fn load_pr_record(
) -> Result<(PullRequestRecord, PathBuf)> {
let storage_dir = base.parent().unwrap_or(base);
let store = store::build_store(storage_dir)?;
let run_dir = resolve_run_combined(store.as_ref(), base, run_id)
.await?
.path;
let run = resolve_run_combined(store.as_ref(), base, run_id).await?;
let run_dir = run.path;
let run_store = store::open_run_reader(storage_dir, &run.run_id).await?;
if let Some(run_store) = run_store {
if let Some(record) = run_store.get_pull_request().await.ok().flatten() {
return Ok((record, run_dir));
}
}
let pr_path = run_dir.join("pull_request.json");
let content = std::fs::read_to_string(&pr_path).with_context(|| {
format!(

View file

@ -82,6 +82,13 @@ async fn resolve_diff(
.as_deref()
.ok_or_else(|| anyhow::anyhow!("This run was not git-checkpointed; no diff available"))?;
if let Some(run_store) = run_store {
if let Ok(Some(patch)) = run_store.get_final_patch().await {
debug!("Reading final.patch from store");
return Ok(patch);
}
}
let final_patch_path = run_dir.join("final.patch");
if final_patch_path.exists() {
debug!("Reading final.patch");

View file

@ -3,12 +3,12 @@ use std::time::Duration;
use fabro_graphviz::graph::Graph;
use fabro_store::RuntimeState;
use fabro_types::PullRequestRecord;
use fabro_util::terminal::Styles;
use fabro_util::text::strip_goal_decoration;
use fabro_workflow::asset_snapshot::collect_asset_paths;
use fabro_workflow::outcome::{StageStatus, format_cost};
use fabro_workflow::pipeline::{Persisted, Validated};
use fabro_workflow::pull_request::PullRequestRecord;
use fabro_workflow::records::{Checkpoint, CheckpointExt, Conclusion, ConclusionExt};
use indicatif::HumanDuration;

View file

@ -1,7 +1,28 @@
use std::sync::Arc;
use fabro_store::Store;
use fabro_test::{fabro_snapshot, test_context};
use fabro_types::RunId;
use object_store::local::LocalFileSystem;
use super::support::{git_filters, setup_git_backed_changed_run, setup_git_backed_noop_run};
fn with_runtime<T>(f: impl FnOnce(&tokio::runtime::Runtime) -> T) -> T {
let runtime = tokio::runtime::Runtime::new().unwrap();
f(&runtime)
}
fn build_store(storage_dir: &std::path::Path) -> Arc<fabro_store::SlateStore> {
let store_path = storage_dir.join("store");
std::fs::create_dir_all(&store_path).unwrap();
let object_store = Arc::new(LocalFileSystem::new_with_prefix(&store_path).unwrap());
Arc::new(fabro_store::SlateStore::new(
object_store,
"",
std::time::Duration::from_millis(5),
))
}
#[test]
fn help() {
let context = test_context!();
@ -89,6 +110,41 @@ fn diff_completed_run_with_changes_prints_patch() {
");
}
#[test]
fn diff_completed_run_reads_store_final_patch_without_disk_file() {
let context = test_context!();
let setup = setup_git_backed_changed_run(&context);
let run_id: RunId = setup.run.run_id.parse().unwrap();
let patch = std::fs::read_to_string(setup.run.run_dir.join("final.patch")).unwrap();
std::fs::remove_file(setup.run.run_dir.join("final.patch")).unwrap();
with_runtime(|runtime| {
runtime.block_on(async {
let store = build_store(&context.storage_dir);
let run_store = store.open_run(&run_id).await.unwrap().unwrap();
run_store.put_final_patch(&patch).await.unwrap();
});
});
let mut cmd = context.command();
cmd.args(["diff", &setup.run.run_id]);
fabro_snapshot!(git_filters(&context), cmd, @"
success: true
exit_code: 0
----- stdout -----
diff --git a/story.txt b/story.txt
index [SHA]..[SHA] 100644
--- a/story.txt
+++ b/story.txt
@@ -1 +1,3 @@
line 1
+line 2
+line 3
----- stderr -----
");
}
#[test]
fn diff_node_outputs_specific_patch() {
let context = test_context!();

View file

@ -44,7 +44,6 @@ fn pr_create_unfinished_run_errors_before_network() {
----- stdout -----
----- stderr -----
error: Failed to load start.json
> I/O error: No such file or directory (os error 2)
");
}
@ -63,3 +62,21 @@ fn pr_create_completed_dry_run_without_run_branch_errors() {
error: Run has no run_branch was it run with git push enabled?
");
}
#[test]
fn pr_create_uses_store_run_record_without_run_json() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
std::fs::remove_file(run.run_dir.join("run.json")).unwrap();
let mut cmd = context.command();
cmd.args(["pr", "create", &run.run_id]);
fabro_snapshot!(context.filters(), cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
error: Run has no run_branch was it run with git push enabled?
");
}

View file

@ -1,7 +1,28 @@
use std::sync::Arc;
use fabro_store::Store;
use fabro_test::{fabro_snapshot, test_context};
use fabro_types::{PullRequestRecord, RunId};
use object_store::local::LocalFileSystem;
use super::support::setup_completed_dry_run;
fn with_runtime<T>(f: impl FnOnce(&tokio::runtime::Runtime) -> T) -> T {
let runtime = tokio::runtime::Runtime::new().unwrap();
f(&runtime)
}
fn build_store(storage_dir: &std::path::Path) -> Arc<fabro_store::SlateStore> {
let store_path = storage_dir.join("store");
std::fs::create_dir_all(&store_path).unwrap();
let object_store = Arc::new(LocalFileSystem::new_with_prefix(&store_path).unwrap());
Arc::new(fabro_store::SlateStore::new(
object_store,
"",
std::time::Duration::from_millis(5),
))
}
#[test]
fn help() {
let context = test_context!();
@ -46,3 +67,45 @@ fn pr_view_missing_pull_request_json_errors() {
> No such file or directory (os error 2)
");
}
#[test]
fn pr_view_reads_pull_request_from_store_without_pull_request_json() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
let run_id: RunId = run.run_id.parse().unwrap();
with_runtime(|runtime| {
runtime.block_on(async {
let store = build_store(&context.storage_dir);
let run_store = store.open_run(&run_id).await.unwrap().unwrap();
run_store
.put_pull_request(&PullRequestRecord {
html_url: "https://github.com/fabro-sh/fabro/pull/123".to_string(),
number: 123,
owner: "fabro-sh".to_string(),
repo: "fabro".to_string(),
base_branch: "main".to_string(),
head_branch: "fabro/run/demo".to_string(),
title: "Map the constellations".to_string(),
})
.await
.unwrap();
});
});
let pr_path = run.run_dir.join("pull_request.json");
if pr_path.exists() {
std::fs::remove_file(pr_path).unwrap();
}
let mut cmd = context.command();
cmd.args(["pr", "view", &run.run_id]);
fabro_snapshot!(context.filters(), cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
error: GitHub App credentials required set GITHUB_APP_PRIVATE_KEY and configure app_id
");
}

View file

@ -10,6 +10,7 @@ pub(crate) mod types;
mod validate;
pub use execute::execute;
pub use fabro_types::PullRequestRecord;
pub(crate) use finalize::build_conclusion_from_store;
pub use finalize::{
build_conclusion, classify_engine_result, finalize, persist_terminal_outcome,
@ -18,9 +19,7 @@ pub use finalize::{
pub use initialize::initialize;
pub use parse::parse;
pub(crate) use persist::persist;
pub use pull_request::{
AutoMergeOptions, PullRequestRecord, build_pr_body, maybe_open_pull_request, pull_request,
};
pub use pull_request::{AutoMergeOptions, build_pr_body, maybe_open_pull_request, pull_request};
pub use retro::{retro, run_retro};
pub use transform::transform;
pub use types::{

View file

@ -2,7 +2,7 @@ use std::path::Path;
use fabro_config::run::MergeStrategy;
use fabro_store::RunStore;
use serde::{Deserialize, Serialize};
use fabro_types::PullRequestRecord;
use tracing::{debug, info};
use fabro_github::{self as github_app, GitHubAppCredentials, ssh_url_to_https};
@ -19,26 +19,6 @@ use tokio::fs::read_to_string;
use super::types::{Concluded, Finalized, PullRequestOptions};
/// Record of a pull request created for a workflow run.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct PullRequestRecord {
pub html_url: String,
pub number: u64,
pub owner: String,
pub repo: String,
pub base_branch: String,
pub head_branch: String,
pub title: String,
}
impl PullRequestRecord {
pub fn save(&self, path: &Path) -> Result<(), String> {
let json = serde_json::to_string_pretty(self)
.map_err(|e| format!("Failed to serialize pull_request.json: {e}"))?;
std::fs::write(path, json).map_err(|e| format!("Failed to write pull_request.json: {e}"))
}
}
/// Derive a PR title from the workflow goal.
///
/// Uses the first line, truncated to 120 characters for readability.