fix(storage): use Storage/RunScratch accessors instead of raw path joins

Four callsites were bypassing existing Storage/RunScratch methods:

1. install.rs — .join("secrets.json") → Storage::secrets_path()
2. initialize.rs — .join("worktree") → RunScratch::worktree_dir()
3. git.rs — .join("final.patch") → RunScratch::final_patch()
4. create.rs — duplicated date-format logic → RunScratch::for_run()

Adds RunScratch::for_run(scratch_dir, run_id) to centralize the
date-prefixed directory name, used by both Storage::run_scratch()
and make_run_dir().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-06 15:12:19 -04:00
parent c57bf24410
commit b29a582d31
No known key found for this signature in database
5 changed files with 22 additions and 14 deletions

View file

@ -13,6 +13,7 @@ use dialoguer::console::Term;
use dialoguer::theme::ColorfulTheme;
use dialoguer::{MultiSelect, Select};
use fabro_api::types::SetSecretRequest;
use fabro_config::Storage;
use fabro_config::legacy_env;
use fabro_config::user::SETTINGS_CONFIG_FILENAME;
use fabro_model::Provider;
@ -556,7 +557,7 @@ async fn persist_install_secrets(
return Ok(());
}
let mut store = SecretStore::load(storage_dir.join("secrets.json"))?;
let mut store = SecretStore::load(Storage::new(storage_dir).secrets_path())?;
for (name, value) in secrets {
store.set(name, value)?;
}
@ -840,7 +841,7 @@ pub(crate) async fn run_install(args: &InstallArgs, globals: &GlobalArgs) -> Res
" {} Saved {} secrets to {}",
s.green.apply_to(""),
secret_pairs.len(),
storage_dir.join("secrets.json").display()
Storage::new(&storage_dir).secrets_path().display()
);
if server_was_running {
eprintln!(

View file

@ -46,11 +46,7 @@ impl Storage {
#[must_use]
pub fn run_scratch(&self, run_id: &RunId) -> RunScratch {
let local_dt = run_id.created_at().with_timezone(&Local);
RunScratch::new(
self.scratch_dir()
.join(format!("{}-{run_id}", local_dt.format("%Y%m%d"))),
)
RunScratch::for_run(&self.scratch_dir(), run_id)
}
#[must_use]
@ -92,6 +88,13 @@ impl RunScratch {
Self { root: root.into() }
}
/// Create a `RunScratch` for a given run under a scratch directory.
#[must_use]
pub fn for_run(scratch_dir: &Path, run_id: &RunId) -> Self {
let local_dt = run_id.created_at().with_timezone(&Local);
Self::new(scratch_dir.join(format!("{}-{run_id}", local_dt.format("%Y%m%d"))))
}
#[must_use]
pub fn root(&self) -> &Path {
&self.root

View file

@ -3,6 +3,7 @@ use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use async_trait::async_trait;
use fabro_config::RunScratch;
use fabro_store::RunDatabase;
use fabro_types::RunId;
use tokio::fs;
@ -311,7 +312,9 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
match git_diff(&*self.sandbox, &base_sha).await {
Ok(patch) if !patch.is_empty() => {
*self.final_patch.lock().unwrap() = Some(patch.clone());
if let Err(err) = fs::write(self.run_dir.join("final.patch"), patch).await {
if let Err(err) =
fs::write(RunScratch::new(&self.run_dir).final_patch(), patch).await
{
self.emitter.emit(&Event::RunNotice {
level: RunNoticeLevel::Warn,
code: "final_patch_write_failed".to_string(),

View file

@ -1,4 +1,3 @@
use chrono::Local;
use fabro_config::Storage;
use fabro_graphviz::graph::{AttrValue, Graph};
use fabro_model::{Catalog, Provider};
@ -393,14 +392,15 @@ pub(crate) fn default_run_dir(run_id: &RunId) -> PathBuf {
}
pub fn make_run_dir(scratch_base: &Path, run_id: &RunId) -> PathBuf {
let local_dt = run_id.created_at().with_timezone(&Local);
scratch_base.join(format!("{}-{run_id}", local_dt.format("%Y%m%d")))
fabro_config::RunScratch::for_run(scratch_base, run_id)
.root()
.to_path_buf()
}
#[cfg(test)]
mod tests {
use super::*;
use chrono::{TimeZone, Utc};
use chrono::{Local, TimeZone, Utc};
use fabro_graphviz::graph::AttrValue;
use fabro_store::Database;
use fabro_types::fixtures;

View file

@ -4,6 +4,7 @@ use std::sync::Arc;
use std::time::Instant;
use fabro_agent::Sandbox;
use fabro_config::RunScratch;
use fabro_graphviz::graph;
use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner};
use fabro_llm::client::Client;
@ -74,7 +75,7 @@ async fn resolve_worktree_plan(
return Ok(Some(WorktreePlan {
branch_name: run_branch.clone(),
base_sha: base_sha.clone(),
worktree_path: options.run_options.run_dir.join("worktree"),
worktree_path: RunScratch::new(&options.run_options.run_dir).worktree_dir(),
skip_branch_creation: true,
}));
}
@ -173,7 +174,7 @@ async fn resolve_worktree_plan(
Ok(Some(WorktreePlan {
branch_name: format!("{}{}", git::RUN_BRANCH_PREFIX, options.run_id),
base_sha,
worktree_path: options.run_options.run_dir.join("worktree"),
worktree_path: RunScratch::new(&options.run_options.run_dir).worktree_dir(),
skip_branch_creation: false,
}))
}