refactor(run): remove vestigial run dir params

This commit is contained in:
Bryan Helmkamp 2026-04-03 16:16:53 -07:00
parent 27f59d3f6c
commit 0577de38f9
No known key found for this signature in database
7 changed files with 27 additions and 139 deletions

View file

@ -56,7 +56,7 @@ pub(crate) struct EventProjectionCache {
}
impl RunState {
pub fn apply_events(events: &[EventEnvelope]) -> Result<Self> {
pub(crate) fn apply_events(events: &[EventEnvelope]) -> Result<Self> {
let mut state = Self::default();
for event in events {
state.apply_event(event)?;
@ -64,7 +64,7 @@ impl RunState {
Ok(state)
}
pub fn apply_event(&mut self, event: &EventEnvelope) -> Result<()> {
pub(crate) fn apply_event(&mut self, event: &EventEnvelope) -> Result<()> {
let value = event.payload.as_value();
let ts = parse_ts(value)?;
let event_name = value
@ -284,17 +284,6 @@ impl RunState {
self.nodes.get(&(node.node_id.to_string(), node.visit))
}
pub fn list_node_ids(&self) -> Vec<String> {
let mut ids = self
.nodes
.keys()
.map(|(node_id, _)| node_id.clone())
.collect::<Vec<_>>();
ids.sort();
ids.dedup();
ids
}
pub fn list_node_visits(&self, node_id: &str) -> Vec<u32> {
let mut visits = self
.nodes
@ -307,7 +296,7 @@ impl RunState {
visits
}
pub fn build_summary(&self, catalog: &CatalogRecord) -> RunSummary {
pub(crate) fn build_summary(&self, catalog: &CatalogRecord) -> RunSummary {
let workflow_name = self.run.as_ref().map(|run| {
if run.graph.name.is_empty() {
"unnamed".to_string()

View file

@ -184,10 +184,6 @@ impl SlateRunStore {
self.inner.db.list_events_from(1).await
}
pub async fn list_events_from(&self, seq: u32) -> Result<Vec<EventEnvelope>> {
self.inner.db.list_events_from(seq).await
}
pub fn watch_events_from(
&self,
seq: u32,
@ -271,20 +267,6 @@ impl SlateRunStore {
.await
}
pub async fn list_assets(&self, node: &NodeVisitRef<'_>) -> Result<Vec<String>> {
let prefix = format!("{}/", keys::node_asset_prefix(node));
let mut iter = self.inner.db.scan_prefix(prefix.as_bytes()).await?;
let mut assets = Vec::new();
while let Some(entry) = iter.next().await? {
let key = key_to_string(&entry.key)?;
if let Some(asset) = key.strip_prefix(&prefix) {
assets.push(asset.to_string());
}
}
assets.sort();
Ok(assets)
}
pub async fn list_all_assets(&self) -> Result<Vec<(String, u32, String)>> {
self.inner.db.list_all_assets().await
}

View file

@ -49,7 +49,7 @@ impl EventPayload {
Ok(payload)
}
pub fn validate(&self, expected_run_id: &RunId) -> Result<()> {
pub(crate) fn validate(&self, expected_run_id: &RunId) -> Result<()> {
let obj = self.0.as_object().ok_or_else(|| {
StoreError::InvalidEvent("event payload must be a JSON object".into())
})?;

View file

@ -157,7 +157,7 @@ fn test_lifecycle(setup_commands: Vec<String>) -> LifecycleOptions {
}
}
async fn test_run_store(_run_dir: &Path, run_id: &RunId) -> fabro_store::RunStoreHandle {
async fn test_run_store(run_id: &RunId) -> fabro_store::SlateRunStore {
let store: StoreHandle = Arc::new(SlateStore::new(
Arc::new(InMemory::new()),
"",
@ -180,7 +180,7 @@ async fn execute_test_run_with_options(
) -> Executed {
let run_id_value = run_options.run_id;
let git_options = run_options.git.clone();
let run_store = test_run_store(&run_options.run_dir, &run_id_value).await;
let run_store = test_run_store(&run_id_value).await;
let emitter = test_emitter_arc("test-run");
let store_logger = StoreProgressLogger::new(run_store.clone());
store_logger.register(&emitter);
@ -241,7 +241,7 @@ async fn execute_runs_start_to_exit_and_returns_final_context() {
persisted_workflow(graph, source, &run_dir, test_run_id("run-test")),
InitOptions {
run_id: test_run_id("run-test"),
run_store: test_run_store(&run_dir, &test_run_id("run-test")).await,
run_store: test_run_store(&test_run_id("run-test")).await,
dry_run: false,
emitter: test_emitter_arc("run-test"),
sandbox: SandboxSpec::Local {
@ -308,7 +308,7 @@ async fn run_with_lifecycle(
persisted_workflow(graph.clone(), String::new(), &run_dir, run_id),
InitOptions {
run_id,
run_store: test_run_store(&run_dir, &run_id).await,
run_store: test_run_store(&run_id).await,
dry_run: false,
emitter,
sandbox: SandboxSpec::Local {

View file

@ -1,4 +1,3 @@
use std::path::Path;
use std::sync::Arc;
use crate::error::FabroError;
@ -166,24 +165,11 @@ fn build_conclusion_from_parts(
}
}
pub fn persist_terminal_outcome(
_run_dir: &Path,
conclusion: &Conclusion,
run_status: RunStatus,
status_reason: Option<StatusReason>,
) {
let _ = (conclusion, run_status, status_reason);
}
/// Write a finalize commit to the shadow branch with retro.json and final node files.
///
/// This captures the last diff.patch (written after the final checkpoint) and retro.json.
/// Best-effort: errors are logged as warnings.
pub async fn write_finalize_commit(
run_options: &RunOptions,
_run_dir: &Path,
run_store: &SlateRunStore,
) {
pub async fn write_finalize_commit(run_options: &RunOptions, run_store: &SlateRunStore) {
let (Some(meta_branch), Some(repo_path)) = (
run_options
.git
@ -271,7 +257,7 @@ pub async fn finalize(
let (final_status, failure_reason, _run_status, _status_reason) =
classify_engine_result(&outcome);
let conclusion = build_conclusion_from_store(
options.run_store.as_ref(),
&options.run_store,
final_status,
failure_reason,
duration_ms,
@ -279,7 +265,7 @@ pub async fn finalize(
)
.await;
write_finalize_commit(&run_options, &options.run_dir, options.run_store.as_ref()).await;
write_finalize_commit(&run_options, &options.run_store).await;
if options.preserve_sandbox {
let info = sandbox.sandbox_info();

View file

@ -12,9 +12,7 @@ mod validate;
pub use execute::execute;
pub use fabro_types::PullRequestRecord;
pub(crate) use finalize::build_conclusion_from_store;
pub use finalize::{
classify_engine_result, finalize, persist_terminal_outcome, write_finalize_commit,
};
pub use finalize::{classify_engine_result, finalize, write_finalize_commit};
pub use initialize::initialize;
pub use parse::parse;
pub(crate) use persist::persist;

View file

@ -1,5 +1,3 @@
use std::path::Path;
use fabro_config::run::MergeStrategy;
use fabro_store::{RunState, SlateRunStore};
use fabro_types::PullRequestRecord;
@ -201,29 +199,6 @@ fn parse_dot_summary(dot: &str) -> (String, usize, usize) {
}
}
#[cfg(test)]
fn read_dot_source(run_dir: &Path) -> Option<String> {
let workflow_fabro_path = run_dir.join("workflow.fabro");
if let Ok(content) = std::fs::read_to_string(&workflow_fabro_path) {
debug!(path = %workflow_fabro_path.display(), "Read workflow graph for PR body");
return Some(content);
}
let legacy_fabro_path = run_dir.join("graph.fabro");
if let Ok(content) = std::fs::read_to_string(&legacy_fabro_path) {
debug!(path = %legacy_fabro_path.display(), "Read workflow graph for PR body (legacy)");
return Some(content);
}
let dot_path = run_dir.join("graph.dot");
match std::fs::read_to_string(&dot_path) {
Ok(content) => {
debug!(path = %dot_path.display(), "Read workflow graph for PR body (dot fallback)");
Some(content)
}
Err(_) => None,
}
}
/// Read plan text from the first `plan*` node response in run state.
///
/// Nodes are sorted alphabetically so `plan` is preferred over `planning`.
@ -306,8 +281,7 @@ fn emit_run_notice(
});
}
async fn load_pull_request_diff(run_store: &SlateRunStore, run_dir: &Path) -> String {
let _ = run_dir;
async fn load_pull_request_diff(run_store: &SlateRunStore) -> String {
run_store
.state()
.await
@ -326,7 +300,6 @@ pub async fn build_pr_body(
goal: &str,
model: &str,
run_store: &SlateRunStore,
_run_dir: &Path,
conclusion: Option<&Conclusion>,
) -> Result<String, String> {
debug!("Building PR body");
@ -435,7 +408,6 @@ pub async fn maybe_open_pull_request(
draft: bool,
auto_merge: Option<AutoMergeOptions>,
run_store: &SlateRunStore,
run_dir: &Path,
conclusion: Option<&Conclusion>,
) -> Result<Option<PullRequestRecord>, String> {
if diff.is_empty() {
@ -446,7 +418,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, run_store, run_dir, conclusion).await?;
let body = build_pr_body(diff, goal, model, run_store, conclusion).await?;
let body = truncate_pr_body(&body);
let title = pr_title_from_goal(goal);
@ -533,8 +505,7 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
result.status,
StageStatus::Success | StageStatus::PartialSuccess
) {
let diff =
load_pull_request_diff(options.run_store.as_ref(), &options.run_dir).await;
let diff = load_pull_request_diff(&options.run_store).await;
if let (Some(base_branch), Some(run_branch), Some(creds), Some(origin)) = (
&run_options.base_branch,
pushed_branch.as_deref(),
@ -559,8 +530,7 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
&options.model,
pr_cfg.draft,
auto_merge,
options.run_store.as_ref(),
&options.run_dir,
&options.run_store,
Some(&conclusion),
)
.await
@ -1103,8 +1073,7 @@ mod tests {
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"mock-model",
run_store.as_ref(),
tmp.path(),
&run_store,
Some(&conclusion),
)
.await
@ -1144,7 +1113,7 @@ mod tests {
labels: HashMap::new(),
};
append_workflow_event(
run_store.as_ref(),
&run_store,
&fixtures::RUN_1,
&WorkflowRunEvent::RunCreated {
run_id: fixtures::RUN_1,
@ -1164,7 +1133,7 @@ mod tests {
.await
.unwrap();
append_workflow_event(
run_store.as_ref(),
&run_store,
&fixtures::RUN_1,
&WorkflowRunEvent::RetroCompleted {
duration_ms: 1,
@ -1180,8 +1149,7 @@ mod tests {
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"mock-model",
run_store.as_ref(),
tmp.path(),
&run_store,
Some(&conclusion),
)
.await
@ -1221,7 +1189,7 @@ mod tests {
labels: HashMap::new(),
};
append_workflow_event(
run_store.as_ref(),
&run_store,
&fixtures::RUN_1,
&WorkflowRunEvent::RunCreated {
run_id: fixtures::RUN_1,
@ -1241,7 +1209,7 @@ mod tests {
.await
.unwrap();
append_workflow_event(
run_store.as_ref(),
&run_store,
&fixtures::RUN_1,
&WorkflowRunEvent::StageCompleted {
node_id: "plan".to_string(),
@ -1273,8 +1241,7 @@ mod tests {
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"mock-model",
run_store.as_ref(),
tmp.path(),
&run_store,
Some(&make_test_conclusion()),
)
.await
@ -1324,39 +1291,6 @@ mod tests {
assert_eq!(format_duration_ms(0), "0s");
}
// ── read_dot_source tests ───────────────────────────────────────────
#[test]
fn read_dot_source_found() {
let tmp = tempfile::tempdir().unwrap();
std::fs::write(tmp.path().join("workflow.fabro"), "digraph test {}").unwrap();
let result = read_dot_source(tmp.path());
assert_eq!(result, Some("digraph test {}".to_string()));
}
#[test]
fn read_dot_source_legacy_fabro_fallback() {
let tmp = tempfile::tempdir().unwrap();
std::fs::write(tmp.path().join("graph.fabro"), "digraph legacy {}").unwrap();
let result = read_dot_source(tmp.path());
assert_eq!(result, Some("digraph legacy {}".to_string()));
}
#[test]
fn read_dot_source_dot_fallback() {
let tmp = tempfile::tempdir().unwrap();
std::fs::write(tmp.path().join("graph.dot"), "digraph old {}").unwrap();
let result = read_dot_source(tmp.path());
assert_eq!(result, Some("digraph old {}".to_string()));
}
#[test]
fn read_dot_source_not_found() {
let tmp = tempfile::tempdir().unwrap();
let result = read_dot_source(tmp.path());
assert_eq!(result, None);
}
// ── Existing tests ─────────────────────────────────────────────────
#[test]
@ -1458,8 +1392,7 @@ mod tests {
"claude-sonnet-4-20250514",
false,
None,
run_store.as_ref(),
tmp.path(),
&run_store,
None,
)
.await;
@ -1492,7 +1425,7 @@ mod tests {
labels: std::collections::HashMap::new(),
};
append_workflow_event(
run_store.as_ref(),
&run_store,
&fixtures::RUN_1,
&WorkflowRunEvent::RunCreated {
run_id: fixtures::RUN_1,
@ -1512,7 +1445,7 @@ mod tests {
.await
.unwrap();
append_workflow_event(
run_store.as_ref(),
&run_store,
&fixtures::RUN_1,
&WorkflowRunEvent::WorkflowRunCompleted {
duration_ms: 1,
@ -1530,7 +1463,7 @@ mod tests {
.await
.unwrap();
let diff = load_pull_request_diff(run_store.as_ref(), tmp.path()).await;
let diff = load_pull_request_diff(&run_store).await;
assert!(diff.contains("from_store"));
}