Replace four private types (InternalStartOptions, StartRetroOptions,
StartFinalizeOptions, StartPullRequestConfig) with a single RunSession
struct. Convert derive_start_options and run_engine into RunSession::new
and RunSession::run methods, flattening the nested config fields.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both rewind_to_entry and fork_from_entry had nearly identical 25-line
blocks resolving the repo path, checking for a remote tracking branch,
and pushing run+meta refspecs. Extract Store::repo_dir() and a shared
push_run_branches() helper to eliminate the duplication.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These fields were only consumed by create_from_source before calling
persist_validated, which immediately destructured them to _. Pass them
as explicit parameters to create_from_source instead.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move resolve_target into RunTimeline::resolve() method and extract
shared test helpers (temp_repo, test_sig, make_checkpoint_json) into
a test_support module used by both fork.rs and rewind.rs.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers the idempotent retry path (same run_id + same created_at) and
the conflict rejection path (same run_id + different created_at returns
RunAlreadyExists). This was already tested in the SlateStore suite but
missing from the InMemoryStore tests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract validate() into its own file from create.rs and resume() into
its own file from start.rs, maintaining one public operation per file.
Shared helpers (preprocess_and_validate, execute_persisted_run) become
pub(super) so the new modules can call them.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Better reflects that this crate contains auto-generated types scoped
to the API layer. Pure rename with no behavior change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fabro-beastie had no consumers other than fabro-cli behind a feature
flag. Absorbing it as an internal module reduces workspace crate count
without changing any behavior.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the per-run `--run-dir` CLI flag with `--storage-dir` which sets
the base storage directory (default ~/.fabro). Runs are now created under
`<storage-dir>/runs/` automatically. This unifies the server's `data_dir`
config with the CLI by renaming `FabroConfig.data_dir` to `storage_dir`
and adding a `storage_dir()` convenience method.
Key changes:
- FabroConfig: `data_dir` → `storage_dir` (serde alias preserves compat)
- CLI: `--run-dir` → `--storage-dir` on `fabro run`
- `__detached`: now takes `--storage-dir` + `--run-id` instead of `--run-dir`
- All ~20 CLI commands derive runs base from config instead of hardcoded default
- Added parameterized `runs_base(storage_dir)` and `make_run_dir()` helpers
- Updated OpenAPI spec, docs, and all tests
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Allow workflow authors to set default goal files and labels in
workflow.toml/fabro.toml, reducing repetitive CLI flags. CLI flags
override config values; labels are deep-merged with CLI winning on
key collision.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The _support suffix was a naming smell — guards, failure persistence,
and progress helpers are all detached-run infrastructure and belong
alongside the detached run entry point.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Preflight validation is conceptually distinct from running a workflow —
it deserves its own top-level command rather than being a flag on `run`.
- Add `PreflightArgs` struct and `Commands::Preflight` variant
- Create `commands/preflight.rs` with dedicated `execute()` function
- Remove `--preflight` flag from `RunArgs`
- Refactor `load_workflow_source_input` to take individual params
instead of `&RunArgs`
- Refactor `resolve_cli_goal` to take `Option<&str>` / `Option<&Path>`
- Refactor `run_preflight` to take `cli_model`/`cli_provider` instead
of `&RunArgs`, make `pub(crate)`
- Update docs and skills references
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two pre-existing issues fixed:
1. attach: while waiting for progress.jsonl, check for terminal
status.json and engine child death. A detached run that dies during
early init (before any event fires) now surfaces the real failure
instead of timing out after 10s.
2. worktree: when `git worktree add` fails after branch creation,
roll back the branch with `git branch -D` to avoid leaking partial
git state.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RunOptions.git was hardcoded to None in run_engine(), relying on
initialize to overwrite it from InitOptions.git. Pass options.git
directly for consistency — initialize still owns the final decision
(clearing it on worktree failure).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Succeeded runs now rejected — a completed run keeps checkpoint.json
around, so resume would happily restart and overwrite start.json and
conclusion.json. Now checks status.json and bails on Succeeded.
2. PID liveness check moved before checkpoint validation. The engine
writes checkpoint.json with a plain fs::write, so a concurrent
resume could see a half-written file and report "corrupt" for a
run that is simply still alive. Order is now: PID → status →
checkpoint parse → cleanup → spawn.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two issues in the resume cleanup logic:
1. progress.jsonl was not in the stale artifact list, so attach and
logs would replay the previous attempt's events before the new run.
Added it to the cleanup list.
2. Cleanup ran before validating the checkpoint was parseable. A
crash during the original run can leave a truncated checkpoint.json
that passes exists() but fails to parse. We now load and parse the
checkpoint first; if it's corrupt we bail with the old conclusion
and failure evidence intact.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two bugs from the refactoring:
1. (High) On resume, run_command_impl would create a fresh worktree
with skip_branch_creation=false, force-resetting the run branch
and losing file changes from the original run. Fix: force
workdir_strategy to LocalDirectory when resume=true.
2. (Low) Sleep inhibitor guard was created inside a #[cfg] block
scope, so it was dropped before resume_command ran. Fix: use
`let _guard = { ... }` pattern to keep it alive for the arm.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resume now follows the same subprocess pattern as run: look up run
directory by ID prefix, validate checkpoint exists, clean stale
artifacts, reset status to Submitted, spawn _run_engine --resume, and
attach. This eliminates ~1600 lines of duplicated env/sandbox setup
from resume.rs.
Key changes:
- operations::start() and operations::resume() take run_dir instead
of Persisted, loading state from disk internally
- run_engine() builds RunOptions from RunRecord on disk, so callers
no longer extract record fields manually
- StartOptions flattened (no more nested InitOptions)
- FabroError::Precondition variant for start/resume guard checks
- _run_engine accepts --resume flag to dispatch to resume path
- operations::restore removed (no longer needed)
- Resume CLI stripped to just <RUN_ID> + --detach
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Follow-up to fafc0a3c. Renames local variables, function parameters,
and struct fields that hold renamed types (ExecutorOptions, RunCreateOptions,
RunOptions) from config/settings to options/run_options for consistency.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Standardize naming so all "bag of options" structs use the Options
suffix: ExecutorSettings→ExecutorOptions, RunSettings→RunOptions,
GitCheckpointSettings→GitCheckpointOptions, LifecycleConfig→LifecycleOptions,
RunCreateSettings→RunCreateOptions, StartRetroConfig→StartRetroOptions,
StartFinalizeConfig→StartFinalizeOptions, AutoMergeConfig→AutoMergeOptions.
Also renames the run_settings module to run_options.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
operations::create now handles the full pipeline: var expansion, parse,
goal override, transform, validate, config normalization, and persist.
This eliminates duplicated RunRecord construction and pipeline::persist
calls across CLI and API callers.
Key changes:
- Rename operations::create → validate, CreateOptions → ValidateOptions
- New operations::create returns Persisted, with RunCreateSettings
- Add ValidationFailed error variant with diagnostics
- Move normalize_config, default_run_dir into operations
- Delete prepare_workflow, PreparedWorkflow, CliFlags from CLI
- Make pipeline::persist and types module pub(crate)
- API catches both Parse and ValidationFailed as 400
- CLI prints diagnostics directly from error (no re-validation)
- ExecutionOverrides struct replaces 9-param function
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Make 7 modules pub(crate) (condition, graph, lifecycle, node_handler,
run_dir) and 4 modules #[doc(hidden)] (artifact, test_support,
transforms, stylesheet) to reduce the public surface of fabro-workflows.
Internal crate::transform alias replaced with crate::transforms.
External consumers still access what they need via narrowed re-exports.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These are purely sandbox concerns — they serialize/deserialize sandbox
connection info and reconstruct sandbox instances. Moving them to
fabro-sandbox improves cohesion and removes workflow-layer coupling
from sandbox lifecycle logic.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The enum had zero internal usage in fabro-workflows and naturally belongs
in fabro-sandbox alongside the sandbox implementations. Removed cfg
gates from the Exe variant (it's just a tag) and added non-exedev
fallback arms in fabro-cli to handle feature unification from fabro-api.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Colocate compute_stage_cost and format_cost with StageUsage, eliminating a
thin module that only imported from outcome.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate all record types under the records module. Files are renamed
to drop the _record suffix (run_record→run, start_record→start,
sandbox_record→sandbox) since the module path provides that context.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
