Commit graph

1528 commits

Author SHA1 Message Date
Bryan Helmkamp
30faaebe2f Fix workflow-relative config resolution 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
95d8b32919 Refactor config layering into combine plus settings 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
aec2d5b437 Rename --run-dir to --storage-dir, unify with data_dir
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
b93e1b0109 Add goal_file and labels to FabroConfig
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
03f73bf83c Add fabro config show command 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
9ec57b1012 Merge detached_support.rs into detached.rs
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
8b78d33795 Extract --preflight into fabro preflight subcommand
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
f05ccb89f3 Extract runs list and rm commands into dedicated files
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
4ac227f57c Move fabro-cli cp under run commands 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
f56faaff52 Refactor fabro-cli run command layout 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
cece073052 refactor(fabro-cli): slim down main 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
4dcd7f49da fix: detect early init failures in attach and roll back partial worktree state
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
2f479dc970 fix(operations): thread git checkpoint options through RunOptions
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
9cedd9fc0a Fix verification regressions after runtime init refactor 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
6120b38e2d Refactor workflow runtime initialization 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
88427c5b19 Guard resume against completed runs 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
bd2142c75b fix(resume): reject succeeded runs and check PID before checkpoint parse
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
93448aa357 fix(resume): validate checkpoint before cleanup and clear progress.jsonl
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
beb0f5653a fix: resume skips worktree creation and keeps sleep inhibitor alive
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
551d16b74c refactor: clean CREATE/START/RESUME separation
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
2d0b719f1c Add restore operation for resumed runs 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
452624e699 refactor: rename local variables/fields to align with Options suffix
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
2f017a4a64 refactor: rename Settings/Config structs to Options suffix
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
20df53b77c Add pull_request pipeline stage 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
8e1c3bc4e9 refactor(operations): make create own the full create lifecycle
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
2d60bb3954 Fix persisted resume boundary gaps 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
c5def1e8ed refactor(workflows): hide internal modules from public API
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
6a70909b00 Add workflow persist stage 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
a7774d1735 refactor(sandbox): move SandboxRecord and sandbox_reconnect to fabro-sandbox
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>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
a438f9bc8d Refactor fabro-workflows graph ops modules 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
b2e78639ee refactor(sandbox): move SandboxProvider from fabro-workflows to fabro-sandbox
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>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
fca50d3336 refactor(workflows): merge cost.rs into outcome.rs
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>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
505b99c83f refactor(workflows): move checkpoint, run_record, start_record, sandbox_record into records/
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>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
b0ff8d3c40 refactor(workflows): split transform.rs into transforms/ directory
Move each transformer into its own file under transforms/, move
stylesheet.rs into the directory, and fold vars.rs into
variable_expansion.rs. Backward-compat re-exports in lib.rs keep all
external paths working.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
e744467830 refactor(workflows): move conclusion into records module and preamble into handler/llm
Relocate conclusion.rs to records/conclusion.rs behind a new records
module, and move preamble.rs into handler/llm/preamble.rs where it is
actually used. Update all imports across fabro-cli and fabro-workflows.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
2882ab3640 refactor(graphviz): move graph_render module from fabro-workflows to fabro-graphviz
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
d1ba181501 style: sort import statements alphabetically
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
7a1271d4e6 refactor(workflows): move core_adapter contents up one level
Promote graph, lifecycle, and node_handler to top-level modules,
removing the unnecessary core_adapter grouping layer.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
fbf1306fbd refactor(workflows): consolidate context/ directory into context.rs
Combine context/mod.rs and context/keys.rs into a single context.rs
file with keys as an inline pub mod. No API changes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
90c55919b3 refactor(workflows): move backend module to handler/llm
Co-locate LLM backend implementations (AgentApiBackend, AgentCliBackend,
BackendRouter) under handler/ since they implement the CodergenBackend
trait defined in handler/agent.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
9ed3a88834 test(workflows): relocate execute pipeline tests 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
4bc73614ea refactor(workflows): remove legacy engine module 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
e84c3bea5a Fix audit regressions from pipeline migration
- Gate pr_config on dry_run_mode to prevent PR creation during dry runs
- Restore em dash (—) separator in retro output
- Print "Retro unavailable" when retro is enabled but returns None
- Fix pre-existing clippy warnings (derivable_impls, needless_borrow)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
3788b8872e Fix operations start follow-up issues 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
0744d98624 Refactor workflow lifecycle into operations 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
e844c371d8 Co-locate unit tests with extracted helper modules
Move unit tests from engine.rs to their respective modules:
- 72 tests to graph_ops.rs (retry policy, edge selection, fidelity, thread_id, etc.)
- 5 tests to run_dir.rs (node_dir, visit_from_context)
- 1 test to sandbox_git.rs (git_checkpoint_includes_builtin_excludes)

Fix clippy needless_borrow in pipeline/finalize.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
8a2fd11735 Extract engine helpers and implement pipeline phases 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
041c2b7c2f Clean up RunSettings migration: fix naming, dedup, error handling
- Rename `settings: mut config` binding to `mut settings` in resume.rs
  and update all 8 downstream references
- Deduplicate normalize_config call in run.rs by reusing the result
  computed for RunRecord
- Replace expect() with graceful error handling when loading RunRecord
  in the API server's execute_run

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
0fe05e09c7 Replace RunConfig with RunSettings 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
d6aff051a5 Align run persistence with run record plan 2026-03-29 13:47:06 -04:00