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

refactor: drop WorkdirStrategy and RunOptions.checkpoints_disabled

Browse source 
WorkdirStrategy was structurally redundant with the existing
LocalSandboxLayer.worktree_mode config — Local sandboxes always picked
LocalWorktree, everything else picked Cloud, and the LocalDirectory arm
was only ever reachable via the parallel checkpoints_disabled bool.

resolve_worktree_plan now reads worktree_mode directly: Cloud sandboxes
return None with a pre_run_git base sha; Local + Never returns None
with no base sha; Local + non-Never builds the WorktreePlan as before.

RunOptions.checkpoints_disabled drops out: the lifecycle gate becomes
has_run_branch (git: None alone is the canonical "no git checkpoints"
signal), and tests/fixtures stop carrying the field.
This commit is contained in:
Bryan Helmkamp 2026-04-28 09:24:41 -07:00
parent 3f027be220
commit 928b2f585b
No known key found for this signature in database
14 changed files with 1551 additions and 1743 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
lib/crates/fabro-sandbox/src/lib.rs
View file

@ -39,5 +39,5 @@ pub use sandbox::{
};
pub use sandbox_provider::SandboxProvider;
pub use sandbox_record::SandboxRecord;
pub use sandbox_spec::{SandboxSpec, WorkdirStrategy};
pub use sandbox_spec::SandboxSpec;
pub use worktree::{WorktreeEvent, WorktreeEventCallback, WorktreeOptions, WorktreeSandbox};

50
lib/crates/fabro-sandbox/src/sandbox_spec.rs
View file

@ -13,7 +13,6 @@ use fabro_types::RunId;
#[cfg(any(feature = "docker", feature = "daytona"))]
use crate::clone_source;
use crate::config::WorktreeMode;
#[cfg(feature = "daytona")]
use crate::daytona::{DaytonaConfig, DaytonaSandbox, DaytonaSnapshotConfig};
#[cfg(feature = "docker")]
@ -46,13 +45,6 @@ pub enum SandboxSpec {
},
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum WorkdirStrategy {
LocalDirectory,
LocalWorktree,
Cloud,
}
impl SandboxSpec {
pub fn provider_name(&self) -> &'static str {
match self {
@ -130,22 +122,6 @@ impl SandboxSpec {
}
}
pub fn workdir_strategy(
&self,
_worktree_mode: WorktreeMode,
_git_is_clean: bool,
_checkpoint_present: bool,
) -> WorkdirStrategy {
match self {
Self::Local { .. } => WorkdirStrategy::LocalWorktree,
#[allow(
unreachable_patterns,
reason = "Feature-gated variants make this fallback arm reachable on some builds."
)]
_ => WorkdirStrategy::Cloud,
}
}
#[allow(
clippy::unused_async,
reason = "Only Daytona construction awaits; local and Docker builds share the async API."
@ -210,29 +186,3 @@ impl SandboxSpec {
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn local_runs_use_worktrees_for_all_worktree_modes() {
let spec = SandboxSpec::Local {
working_directory: PathBuf::from("/repo"),
};
for mode in [
WorktreeMode::Always,
WorktreeMode::Clean,
WorktreeMode::Dirty,
WorktreeMode::Never,
] {
for git_is_clean in [true, false] {
assert_eq!(
spec.workdir_strategy(mode, git_is_clean, false),
WorkdirStrategy::LocalWorktree
);
}
}
}
}

25
lib/crates/fabro-workflow/src/handler/manager_loop.rs
View file

@ -202,20 +202,19 @@ impl Handler for SubWorkflowHandler {
let child_cancel = Arc::clone(&cancel_token);
let child_run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: child_logs,
cancel_token: Some(cancel_token),
settings: WorkflowSettings::default(),
run_dir: child_logs,
cancel_token: Some(cancel_token),
// Child workflows are part of the parent run's event stream.
run_id: services.run.emitter.run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: true,
base_branch: None,
display_base_sha: None,
git: None,
run_id: services.run.emitter.run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
base_branch: None,
display_base_sha: None,
git: None,
};
// Clone parent context for child; inject parent preamble

3
lib/crates/fabro-workflow/src/lifecycle/mod.rs
View file

@ -106,8 +106,7 @@ impl WorkflowLifecycle {
.as_ref()
.and_then(|g| g.run_branch.as_ref())
.is_some();
let local_git_checkpoint = has_run_branch && !run_options.checkpoints_disabled;
let working_directory = if local_git_checkpoint {
let working_directory = if has_run_branch {
Some(sandbox.working_directory().to_string())
} else {
None

25
lib/crates/fabro-workflow/src/operations/start.rs
View file

@ -691,19 +691,18 @@ impl RunSession {
let record = persisted.run_spec();
let run_options = RunOptions {
settings: record.settings.clone(),
run_dir: persisted.run_dir().to_path_buf(),
cancel_token: self.cancel_token,
run_id: record.run_id,
labels: record.labels.clone(),
workflow_slug: record.workflow_slug.clone(),
github_app: self.github_app.clone(),
pre_run_git: record.pre_run_git.clone(),
fork_source_ref: record.fork_source_ref.clone(),
checkpoints_disabled: record.checkpoints_disabled,
base_branch: record.base_branch.clone(),
display_base_sha: None,
git: self.git.clone(),
settings: record.settings.clone(),
run_dir: persisted.run_dir().to_path_buf(),
cancel_token: self.cancel_token,
run_id: record.run_id,
labels: record.labels.clone(),
workflow_slug: record.workflow_slug.clone(),
github_app: self.github_app.clone(),
pre_run_git: record.pre_run_git.clone(),
fork_source_ref: record.fork_source_ref.clone(),
base_branch: record.base_branch.clone(),
display_base_sha: None,
git: self.git.clone(),
};
let last_git_sha: Arc<Mutex<Option<String>>> = Arc::new(Mutex::new(None));

25
lib/crates/fabro-workflow/src/pipeline/execute/tests.rs
View file

@ -90,19 +90,18 @@ fn test_emitter_arc(label: &str) -> Arc<Emitter> {
fn test_run_options(run_dir: &Path, run_id: &str) -> RunOptions {
RunOptions {
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(run_id),
settings: WorkflowSettings::default(),
git: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
labels: HashMap::new(),
github_app: None,
base_branch: None,
display_base_sha: None,
workflow_slug: None,
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(run_id),
settings: WorkflowSettings::default(),
git: None,
pre_run_git: None,
fork_source_ref: None,
labels: HashMap::new(),
github_app: None,
base_branch: None,
display_base_sha: None,
workflow_slug: None,
}
}

25
lib/crates/fabro-workflow/src/pipeline/finalize.rs
View file

@ -438,19 +438,18 @@ mod tests {
fn test_run_options(run_dir: &std::path::Path) -> RunOptions {
RunOptions {
settings: WorkflowSettings::default(),
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
base_branch: None,
display_base_sha: None,
git: None,
settings: WorkflowSettings::default(),
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
base_branch: None,
display_base_sha: None,
git: None,
}
}

131
lib/crates/fabro-workflow/src/pipeline/initialize.rs
View file

@ -11,9 +11,10 @@ use fabro_auth::{
use fabro_config::RunScratch;
use fabro_graphviz::graph;
use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner};
use fabro_sandbox::config::WorktreeMode;
use fabro_sandbox::{
GitSetupIntent, ReadBeforeWriteSandbox, SandboxEventCallback, SandboxSpec, WorkdirStrategy,
WorktreeOptions, WorktreeSandbox,
GitSetupIntent, ReadBeforeWriteSandbox, SandboxEventCallback, SandboxSpec, WorktreeOptions,
WorktreeSandbox,
};
use fabro_vault::Vault;
use futures::future::try_join_all;
@ -100,17 +101,14 @@ async fn run_hooks(
}
fn resolve_worktree_plan(options: &mut InitOptions) -> Option<WorktreePlan> {
if options.run_options.checkpoints_disabled {
options.run_options.display_base_sha = None;
return None;
}
let Some(worktree_mode) = options.worktree_mode else {
options.run_options.display_base_sha = None;
return None;
};
if options.checkpoint.is_some() && matches!(options.sandbox, SandboxSpec::Local { .. }) {
let is_local = matches!(options.sandbox, SandboxSpec::Local { .. });
if options.checkpoint.is_some() && is_local {
if let Some(fork_source) = options.run_options.fork_source_ref.as_ref() {
let base_sha = fork_source.checkpoint_sha.clone();
options.run_options.display_base_sha = Some(base_sha.clone());
@ -121,9 +119,7 @@ fn resolve_worktree_plan(options: &mut InitOptions) -> Option<WorktreePlan> {
skip_branch_creation: false,
});
}
}
if options.checkpoint.is_some() && matches!(options.sandbox, SandboxSpec::Local { .. }) {
if let Some(git) = options.run_options.git.as_ref() {
if let (Some(run_branch), Some(base_sha)) = (&git.run_branch, &git.base_sha) {
options.run_options.display_base_sha = Some(base_sha.clone());
@ -143,18 +139,14 @@ fn resolve_worktree_plan(options: &mut InitOptions) -> Option<WorktreePlan> {
.pre_run_git
.as_ref()
.map(|git| git.local_dirty);
let git_is_clean =
local_dirty.is_some_and(|status| matches!(status, fabro_types::DirtyStatus::Clean));
let strategy =
options
.sandbox
.workdir_strategy(worktree_mode, git_is_clean, options.checkpoint.is_some());
if matches!(local_dirty, Some(fabro_types::DirtyStatus::Dirty)) {
let env_name = match strategy {
WorkdirStrategy::LocalWorktree => Some("worktree"),
WorkdirStrategy::Cloud => Some("remote sandbox"),
WorkdirStrategy::LocalDirectory => None,
let env_name = if !is_local {
Some("remote sandbox")
} else if worktree_mode == WorktreeMode::Never {
None
} else {
Some("worktree")
};
if let Some(env_name) = env_name {
options.emitter.notice(
@ -165,45 +157,43 @@ fn resolve_worktree_plan(options: &mut InitOptions) -> Option<WorktreePlan> {
}
}
match strategy {
WorkdirStrategy::LocalWorktree => {
let (branch_name, base_sha) =
if let Some(fork_source) = options.run_options.fork_source_ref.as_ref() {
(
format!("{RUN_BRANCH_PREFIX}{}", options.run_id),
Some(fork_source.checkpoint_sha.clone()),
)
} else {
(
format!("{RUN_BRANCH_PREFIX}{}", options.run_id),
options
.run_options
.pre_run_git
.as_ref()
.and_then(|git| git.display_base_sha.clone()),
)
};
options.run_options.display_base_sha = base_sha.clone();
Some(WorktreePlan {
branch_name,
base_sha,
worktree_path: RunScratch::new(&options.run_options.run_dir).worktree_dir(),
skip_branch_creation: false,
})
}
WorkdirStrategy::Cloud => {
options.run_options.display_base_sha = options
.run_options
.pre_run_git
.as_ref()
.and_then(|git| git.display_base_sha.clone());
None
}
WorkdirStrategy::LocalDirectory => {
options.run_options.display_base_sha = None;
None
}
if !is_local {
options.run_options.display_base_sha = options
.run_options
.pre_run_git
.as_ref()
.and_then(|git| git.display_base_sha.clone());
return None;
}
if worktree_mode == WorktreeMode::Never {
options.run_options.display_base_sha = None;
return None;
}
let (branch_name, base_sha) =
if let Some(fork_source) = options.run_options.fork_source_ref.as_ref() {
(
format!("{RUN_BRANCH_PREFIX}{}", options.run_id),
Some(fork_source.checkpoint_sha.clone()),
)
} else {
(
format!("{RUN_BRANCH_PREFIX}{}", options.run_id),
options
.run_options
.pre_run_git
.as_ref()
.and_then(|git| git.display_base_sha.clone()),
)
};
options.run_options.display_base_sha.clone_from(&base_sha);
Some(WorktreePlan {
branch_name,
base_sha,
worktree_path: RunScratch::new(&options.run_options.run_dir).worktree_dir(),
skip_branch_creation: false,
})
}
fn git_setup_intent(run_options: &RunOptions) -> GitSetupIntent {
@ -848,19 +838,18 @@ mod tests {
fn test_settings(run_dir: &std::path::Path) -> RunOptions {
RunOptions {
settings: WorkflowSettings::default(),
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
base_branch: None,
display_base_sha: None,
git: None,
settings: WorkflowSettings::default(),
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
base_branch: None,
display_base_sha: None,
git: None,
}
}

25
lib/crates/fabro-workflow/src/pipeline/retro.rs
View file

@ -279,19 +279,18 @@ mod tests {
fn test_run_options(run_dir: &std::path::Path) -> RunOptions {
RunOptions {
settings: WorkflowSettings::default(),
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
base_branch: None,
display_base_sha: None,
git: None,
settings: WorkflowSettings::default(),
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: test_run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
pre_run_git: None,
fork_source_ref: None,
base_branch: None,
display_base_sha: None,
git: None,
}
}

26
lib/crates/fabro-workflow/src/run_options.rs
View file

@ -19,30 +19,28 @@ pub struct GitCheckpointOptions {
/// Options for a workflow run.
#[derive(Clone)]
pub struct RunOptions {
pub settings: WorkflowSettings,
pub run_dir: PathBuf,
pub cancel_token: Option<Arc<AtomicBool>>,
pub settings: WorkflowSettings,
pub run_dir: PathBuf,
pub cancel_token: Option<Arc<AtomicBool>>,
/// Unique identifier for this workflow run.
pub run_id: RunId,
pub run_id: RunId,
/// User-defined key-value labels for this run.
pub labels: HashMap<String, String>,
pub labels: HashMap<String, String>,
/// Workflow directory slug (e.g. "smoke" from `.fabro/workflows/smoke/`).
pub workflow_slug: Option<String>,
pub workflow_slug: Option<String>,
/// GitHub credentials for pushing metadata branches to origin.
pub github_app: Option<fabro_github::GitHubCredentials>,
pub github_app: Option<fabro_github::GitHubCredentials>,
/// Submitter-side git context captured before the run was created.
pub pre_run_git: Option<PreRunGitContext>,
pub pre_run_git: Option<PreRunGitContext>,
/// Source checkpoint ref used by fork/rewind-created runs.
pub fork_source_ref: Option<ForkSourceRef>,
/// Explicit no-checkpoints mode for in-place local execution.
pub checkpoints_disabled: bool,
pub fork_source_ref: Option<ForkSourceRef>,
/// Name of the branch the run was started from (for PR base).
pub base_branch: Option<String>,
pub base_branch: Option<String>,
/// Base commit SHA to display in lifecycle events/UI even when
/// checkpointing is disabled.
pub display_base_sha: Option<String>,
pub display_base_sha: Option<String>,
/// Git checkpoint options; `None` means checkpointing disabled.
pub git: Option<GitCheckpointOptions>,
pub git: Option<GitCheckpointOptions>,
}
impl RunOptions {

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

@ -128,7 +128,7 @@ async fn initialized(
manifest_blob: None,
pre_run_git: run_options.pre_run_git.clone(),
fork_source_ref: run_options.fork_source_ref.clone(),
checkpoints_disabled: run_options.checkpoints_disabled,
checkpoints_disabled: false,
})
.await
.expect("failed to seed run.created event in run store");

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

@ -511,19 +511,18 @@ async fn daytona_pipeline_artifact_offload_and_sync() {
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone());
let run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
cancel_token: None,
run_id: test_run_id("test-run"),
labels: std::collections::HashMap::new(),
workflow_slug: None,
github_app: None,
base_branch: None,
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
git: None,
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
cancel_token: None,
run_id: test_run_id("test-run"),
labels: std::collections::HashMap::new(),
workflow_slug: None,
github_app: None,
base_branch: None,
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
git: None,
};
let outcome = engine
.run(&graph, &run_options)
@ -692,19 +691,18 @@ async fn daytona_git_checkpoint_remote_emits_events() {
let engine = WorkflowRunner::new(registry, Arc::new(emitter), env.clone());
let run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
cancel_token: None,
run_id: test_run_id("git-cp-test"),
labels: std::collections::HashMap::new(),
workflow_slug: None,
github_app: None,
base_branch: None,
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
git: Some(GitCheckpointOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
cancel_token: None,
run_id: test_run_id("git-cp-test"),
labels: std::collections::HashMap::new(),
workflow_slug: None,
github_app: None,
base_branch: None,
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
git: Some(GitCheckpointOptions {
base_sha: Some(base_sha),
run_branch: Some(branch_name),
meta_branch: None,
@ -876,7 +874,6 @@ async fn daytona_parallel_git_branching_e2e() {
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
git: Some(GitCheckpointOptions {
base_sha: Some(base_sha),
run_branch: Some(branch_name),
@ -1207,7 +1204,6 @@ async fn daytona_git_checkpoint_with_shadow_branch() {
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
git: Some(GitCheckpointOptions {
base_sha: Some(base_sha),
run_branch: Some(branch_name),
@ -1346,7 +1342,7 @@ async fn daytona_asset_collection() {
graph.edges.push(Edge::new("create_assets", "exit"));
let run_options = RunOptions {
settings: WorkflowSettings {
settings: WorkflowSettings {
run: fabro_types::settings::RunNamespace {
artifacts: fabro_types::settings::run::ArtifactsSettings {
include: vec!["test-results/**".to_string()],
@ -1355,18 +1351,17 @@ async fn daytona_asset_collection() {
},
..WorkflowSettings::default()
},
run_dir: dir.path().to_path_buf(),
cancel_token: None,
run_id: test_run_id("artifact-test-daytona"),
labels: std::collections::HashMap::new(),
workflow_slug: None,
github_app: None,
base_branch: None,
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
git: None,
run_dir: dir.path().to_path_buf(),
cancel_token: None,
run_id: test_run_id("artifact-test-daytona"),
labels: std::collections::HashMap::new(),
workflow_slug: None,
github_app: None,
base_branch: None,
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
git: None,
};
let outcome = engine
.run(&graph, &run_options)
@ -1624,7 +1619,6 @@ async fn daytona_git_push_run_branch_to_origin() {
display_base_sha: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
git: Some(GitCheckpointOptions {
base_sha: Some(base_sha),
run_branch: Some(branch_name.clone()),

25
lib/crates/fabro-workflow/tests/it/git_integration.rs
View file

@ -153,19 +153,18 @@ fn make_registry() -> HandlerRegistry {
fn test_run_options(run_dir: &Path) -> RunOptions {
RunOptions {
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: fixtures::RUN_2,
settings: WorkflowSettings::default(),
git: None,
pre_run_git: None,
fork_source_ref: None,
checkpoints_disabled: false,
labels: HashMap::new(),
github_app: None,
base_branch: None,
display_base_sha: None,
workflow_slug: None,
run_dir: run_dir.to_path_buf(),
cancel_token: None,
run_id: fixtures::RUN_2,
settings: WorkflowSettings::default(),
git: None,
pre_run_git: None,
fork_source_ref: None,
labels: HashMap::new(),
github_app: None,
base_branch: None,
display_base_sha: None,
workflow_slug: None,
}
}

2852
lib/crates/fabro-workflow/tests/it/integration.rs
View file

File diff suppressed because it is too large Load diff