Commit graph

1551 commits

Author SHA1 Message Date
Bryan Helmkamp
752d04cbcb Enable additional pedantic clippy lints and fix violations
Enables cast_possible_truncation, cast_sign_loss, items_after_statements,
needless_pass_by_value, return_self_not_must_use, uninlined_format_args,
unreadable_literal, and unnested_or_patterns. Keeps doc_markdown disabled.

Replaces unsafe `as` casts with try_from().unwrap() throughout, using
#[allow] only for f64-to-integer casts which have no try_from equivalent.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
1d304b771a Enable clippy pedantic lints and restriction lints workspace-wide
Adopts uv's clippy lint configuration: pedantic group at warn priority,
with noisy lints allowed, plus restriction lints for print/dbg/exit/use_self.
Fixes all violations across the workspace.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
97214c7d83 Apply rustfmt 2024 style edition across workspace
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
0b90305432 Enforce no-inline-qualified-paths via clippy absolute_paths lint
Add clippy.toml with absolute-paths-max-segments = 2 (allowing std/core/alloc)
and enable the absolute_paths = "warn" lint workspace-wide. Fix all ~300
violations across the codebase: replace 3+-segment inline paths with use
statements so call sites read as operations::create() rather than
fabro_workflows::operations::create(). The demo module gets an allow
attribute since it constructs many API types by design.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
41e1809b21 Enforce no-wildcard-imports via clippy workspace lint
Configure clippy `wildcard_imports = "warn"` at the workspace level and
opt all 28 crates in via `[lints] workspace = true`. Fix the three
production glob imports that triggered warnings: fabro-sandbox
read_guard, fabro-cli main, and fabro-api demo module (allowed via
attribute since it constructs many API types by design). Document the
import style convention in CLAUDE.md.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
423c4b4688 Extract RunSession from run_engine to eliminate intermediary option types
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>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
6cf4be6354 Extract push_run_branches helper from rewind.rs and fork.rs
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>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
4b1caafcc8 Fix leading space in artifact path doc example
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
65b8cb6a31 Clean up runtime path docs and test comment 2026-03-29 13:47:08 -04:00
Bryan Helmkamp
5a09b4c328 Remove unused goal_override and base_dir from PersistCreateOptions
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>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
bf9fd2cea2 Create RuntimeState for run runtime and cache paths 2026-03-29 13:47:08 -04:00
Bryan Helmkamp
ffd0e1de11 Deduplicate timeline resolution and test helpers in operations
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>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
69d00ef149 Fix Slate store fencing and restore status shape 2026-03-29 13:47:08 -04:00
Bryan Helmkamp
ce33e880ba Add InMemoryStore create_run retry and conflict test
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>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
e417901a3e Add fabro-store with in-memory and SlateDB backends 2026-03-29 13:47:08 -04:00
Bryan Helmkamp
55e792b885 Split validate.rs and resume.rs out of operations
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>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
48c21ed521 Create fabro-types and rewire shared types 2026-03-29 13:47:08 -04:00
Bryan Helmkamp
c192c106f6 Rename fabro-types to fabro-api-types
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>
2026-03-29 13:47:08 -04:00
Bryan Helmkamp
58bb577d57 Refactor workflow operations and config resolution 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
590d237844 Tighten workflows request context and launcher cleanup 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
db3231e987 Thin CLI run commands and move execution into workflows operations 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
defcf9c746 Clean up resolved settings interfaces 2026-03-29 13:47:07 -04:00
Bryan Helmkamp
72acee6c8a 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
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