Commit graph

137 commits

Author SHA1 Message Date
Bryan Helmkamp
33f4da33c1
Clean up resolved settings interfaces 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
d5287a1af5
Fold fabro-beastie into fabro-cli as internal sleep_inhibitor module
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>
2026-03-29 13:47:07 -04:00
Bryan Helmkamp
68ca9e892a
Fix workflow-relative config resolution 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
8a256bb68e
Refactor config layering into combine plus settings 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
126ece3d71
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
833574ec4b
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
960ae3b3ac
Add fabro config show command 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
5947513217
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
fc3d364c84
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
e5c60a84b3
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
37cd04afa0
Move fabro-cli cp under run commands 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
c871ecb274
Refactor fabro-cli run command layout 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
241f0192f2
refactor(fabro-cli): slim down main 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
b1fb172dde
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
0e076d5594
Fix verification regressions after runtime init refactor 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
205d40d687
Refactor workflow runtime initialization 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
b748c7e6ea
Guard resume against completed runs 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
75f2148f43
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
4c6b846fb9
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
25b9d24d9e
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
d45b4516ab
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
f4f4762ab4
Add restore operation for resumed runs 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
1a91a9b916
refactor: rename local variables/fields to align with Options suffix
Follow-up to f37345d5. 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
f3bc47e9f0
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
ed18bb6250
Add pull_request pipeline stage 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
a8fa5256b4
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
c1d840e792
Fix persisted resume boundary gaps 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
b57bc52416
Add workflow persist stage 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
974fa5519f
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
2203bb8d9e
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
1dfcc699c6
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
365461e51d
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
a23822bbbb
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
08f4a6d978
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
0c5bbf5bc1
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
937ff79575
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
6fae59a9a5
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
5cedd970d0
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
6ba28531ad
Refactor workflow lifecycle into operations 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
1a1c18fa7c
Extract engine helpers and implement pipeline phases 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
cc7ab268aa
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
7b7a7d829b
Replace RunConfig with RunSettings 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
f487fc0959
Align run persistence with run record plan 2026-03-29 13:47:06 -04:00
Bryan Helmkamp
88ec53e632
Complete remaining RunRecord plan gaps: run_from_record, TOML rename, API RunRecord
Phase 5: Rename debug artifacts from run.toml/graph.fabro to
workflow.toml/workflow.fabro. Change write_run_config_snapshot to
byte-for-byte copy of the original TOML instead of re-serialization.

Phase 6: Add run_from_record() that builds execution state directly
from a RunRecord, bypassing prepare_workflow(). Refactor run_command
into run_command + run_command_impl to share execution logic. Simplify
run_engine_entrypoint to call run_from_record() instead of
reconstructing RunArgs and re-parsing the workflow.

Step 7k: Write RunRecord in the API server's execute_run() for
observability, enabling fabro ps/inspect for API-initiated runs.

Fix stale manifest.json reference in docs/agents/outputs.mdx.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
b3382aa734
Simplify MetadataStore init_run API and resume graph loading
Collapse init_run/init_run_with_records/init_run_inner into a single
init_run(run_id, files) that takes all files as a flat slice. Resume
from metadata branch now uses RunRecord's embedded graph directly
when available, falling back to graph.fabro DOT parsing for old runs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
e7a36a1feb
Remove RunSpec + Manifest types and all remaining references
Delete run_spec.rs and manifest.rs modules. Remove write_manifest()
from the engine, update DiskLifecycle and GitLifecycle to only write
StartRecord. Remove read_manifest() from MetadataStore. Update
run_fork to only handle run.json/start.json. Convert resume.rs to
use RunRecord/StartRecord from the metadata branch. Update all tests
and integration tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
08dbcee01e
Add RunRecord + StartRecord alongside RunSpec + Manifest
Introduce two new persistence types aligned to the CREATE/START lifecycle:
- RunRecord (run.json): written at CREATE with merged FabroConfig, fully
  transformed Graph, and run metadata
- StartRecord (start.json): written at START with start_time, run_branch,
  and base_sha

All readers (run_lookup, inspect, diff, pr, attach, detached_support,
start, run_fork, pull_request, run_rewind, resume) now read from the
new types first. Legacy manifest.json + spec.json are still written
for backward compatibility (removal in follow-up).

Also adds dry_run, auto_approve, no_retro fields to FabroConfig, derives
Default on LlmConfig and Graph, and updates docs + tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
0466f94239
Fix CLI integration test timeout by skipping upgrade check
The dry_run_writes_jsonl_and_live_json test was timing out at 4s because
the arc() helper didn't pass --no-upgrade-check, causing every test run
to await a background GitHub API call before process exit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:06 -04:00
Bryan Helmkamp
ef583c35ac
Add explicit 7-phase pipeline module with typestate lifecycle
Introduce `fabro_workflows::pipeline` module defining typed phases:
PARSE → TRANSFORM → VALIDATE → INITIALIZE → EXECUTE → RETRO → FINALIZE.

Each phase is a standalone function with `#[non_exhaustive]` input/output
types so the compiler enforces ordering. `Validated` uses private fields
with read-only accessors to guarantee immutability post-validation.

Split `engine.run_with_lifecycle()` into `prepare_sandbox()` +
`execute_graph()` (backward-compatible wrapper preserved). Rewrite
`WorkflowBuilder::prepare_inner()` and CLI `prepare_workflow()` to use
pipeline functions. `PreparedWorkflow` now carries a `Validated` with
accessor methods instead of raw `graph`/`source` fields.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:05 -04:00
Bryan Helmkamp
b1693c34a8
Unify Outcome types between fabro-core and fabro-workflows
Make fabro-core's Outcome generic over a usage/metadata type parameter
(OutcomeMeta trait), allowing fabro-workflows to use core's types
directly via a type alias instead of maintaining duplicate Outcome,
StageStatus, and FailureDetail types with bidirectional conversions.

Key changes:
- Add FailureCategory enum to fabro-core (moved from fabro-workflows'
  FailureClass), with Display/FromStr/is_signature_tracked
- Add OutcomeMeta supertrait + blanket impl for the generic parameter
- Make Outcome<M>, NodeResult<M>, RunState<M>, NodeDecision<M> generic
  with default type parameter M=()
- Add Graph::Meta associated type
- Update FailureDetail with serde renames (category→"failure_class",
  signature→"failure_signature") for checkpoint backward compat
- Replace fabro-workflows' Outcome with type alias to
  fabro_core::Outcome<Option<StageUsage>>
- Add OutcomeExt extension trait for wf-specific factory methods
  (fail_classify, fail_deterministic, retry_classify, simulated, etc.)
- Delete core_adapter/outcome.rs (~170 lines of conversion functions)
- Replace FailureClass with FailureCategory throughout fabro-workflows
- Fix timeout handler to use TransientInfra category, panic handler to
  use Deterministic category

Net: -144 lines, zero-cost type unification with no runtime conversions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:05 -04:00