Commit graph

416 commits

Author SHA1 Message Date
Bryan Helmkamp
bb791e98b0 Refactor workflow runtime initialization 2026-03-26 22:18:07 -04:00
Bryan Helmkamp
571ac0e1f3 Guard resume against completed runs 2026-03-26 13:53:12 -04:00
Bryan Helmkamp
7c906fc570 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-26 13:19:41 -04:00
Bryan Helmkamp
cb55fd3cd1 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-26 13:08:36 -04:00
Bryan Helmkamp
7c74c0a417 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-26 13:07:15 -04:00
Bryan Helmkamp
13e394fa5b 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-26 12:58:47 -04:00
Bryan Helmkamp
97e2ff7882 Add restore operation for resumed runs 2026-03-26 10:30:30 -04:00
Bryan Helmkamp
e6e913eabc 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-26 09:24:11 -04:00
Bryan Helmkamp
fafc0a3c39 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-26 08:52:15 -04:00
Bryan Helmkamp
c81e61bd12 Add pull_request pipeline stage 2026-03-26 08:38:23 -04:00
Bryan Helmkamp
89ad849208 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-25 23:09:16 -04:00
Bryan Helmkamp
5a8eff0d63 Fix persisted resume boundary gaps 2026-03-25 14:06:19 -04:00
Bryan Helmkamp
4522762cba 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-25 13:54:08 -04:00
Bryan Helmkamp
e9cf3c374d Add workflow persist stage 2026-03-25 13:46:34 -04:00
Bryan Helmkamp
62b3a0e1a0 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-25 12:12:32 -04:00
Bryan Helmkamp
f5217b92e8 Refactor fabro-workflows graph ops modules 2026-03-25 11:57:11 -04:00
Bryan Helmkamp
e9d758d0cb 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-25 11:38:34 -04:00
Bryan Helmkamp
6386621416 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-25 11:29:53 -04:00
Bryan Helmkamp
b784946aee 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-25 11:21:16 -04:00
Bryan Helmkamp
ad1fadc338 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-25 11:18:06 -04:00
Bryan Helmkamp
4e7f854d1b 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-25 11:05:17 -04:00
Bryan Helmkamp
73b8fc8cb9 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-25 10:58:37 -04:00
Bryan Helmkamp
b6ea9e1584 style: sort import statements alphabetically
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 10:49:30 -04:00
Bryan Helmkamp
4d01287719 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-25 10:36:26 -04:00
Bryan Helmkamp
526fe6229e 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-25 10:31:37 -04:00
Bryan Helmkamp
5765c2078f 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-25 10:26:41 -04:00
Bryan Helmkamp
3e1603f64b test(workflows): relocate execute pipeline tests 2026-03-25 10:22:39 -04:00
Bryan Helmkamp
a5bc58d18b refactor(workflows): remove legacy engine module 2026-03-25 10:17:07 -04:00
Bryan Helmkamp
55b036ebc2 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-25 09:47:37 -04:00
Bryan Helmkamp
159640bb55 Fix operations start follow-up issues 2026-03-25 09:44:31 -04:00
Bryan Helmkamp
378ad22ffe Refactor workflow lifecycle into operations 2026-03-25 09:29:26 -04:00
Bryan Helmkamp
b53f9d4977 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-25 00:14:45 -04:00
Bryan Helmkamp
e55c9a3189 Extract engine helpers and implement pipeline phases 2026-03-24 22:31:08 -04:00
Bryan Helmkamp
606af8b1ea 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-24 21:15:11 -04:00
Bryan Helmkamp
70a89f6f53 Replace RunConfig with RunSettings 2026-03-24 21:11:16 -04:00
Bryan Helmkamp
1d456e5021 Align run persistence with run record plan 2026-03-24 20:15:59 -04:00
Bryan Helmkamp
450a9c61ba 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-24 19:53:58 -04:00
Bryan Helmkamp
16bfb84b40 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-24 19:30:18 -04:00
Bryan Helmkamp
2aa6476437 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-24 19:23:54 -04:00
Bryan Helmkamp
b0907ff90b 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-24 19:06:37 -04:00
Bryan Helmkamp
ddb499c319 Remove core-engine feature flag and old execution loop
Fix parity gaps (handler errors → fail outcomes, panic.txt, goal gate
message, fail-with-no-edge message, visit limit source, terminal
completion normalization) then delete ~1,250 lines of old-path code
(LoopState, run_failed_hook, mirror_graph_attributes, execute_with_retry,
run_internal) and all cfg gating.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 17:49:44 -04:00
Bryan Helmkamp
a02e3533e4 Clarify visit count semantics 2026-03-24 16:40:34 -04:00
Bryan Helmkamp
a452456cd9 Simplify Context: inline HashMap in fabro-core, extension trait in fabro-workflows
Remove the ContextStore trait, InMemoryStore, and Context::with_store() from
fabro-core — put the HashMap directly in Context. Replace the duplicate
fabro-workflows Context struct with a re-export of fabro_core::Context, and
move domain accessors (fidelity, run_id, preamble, thread_id) to a
WorkflowContext extension trait. Eliminate the bridge layer (WfContextStore,
bridge_context, WorkflowContextExt) entirely since there is now one Context
type. Rename clone_context() to fork() for clarity.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 16:26:05 -04:00
Bryan Helmkamp
f3f15967e2 Add fabro-core after_record lifecycle hook 2026-03-24 16:18:49 -04:00
Bryan Helmkamp
b536564d5e 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-24 15:45:43 -04:00
Bryan Helmkamp
668b70842e Align core adapter lifecycle with final plan 2026-03-24 15:29:16 -04:00
Bryan Helmkamp
d285965596 Implement all sub-lifecycles fully per plan specification
Fill in the previously stubbed ArtifactLifecycle and GitLifecycle, and
complete FidelityLifecycle and CircuitBreakerLifecycle with their full
behavior. Wire the orchestrator with context seeding, shared state, and
all callback orderings matching the plan.

FidelityLifecycle: use resolve_fidelity/resolve_thread_id for full
resolution chains, add preamble building via build_preamble, set
thread.{tid}.current_node key, store raw Edge for proper resolution.

CircuitBreakerLifecycle: add on_edge_selected with TransientInfra guard
and restart_failure_signatures tracking for loop_restart edges.

EventLifecycle: add Skipped guard in after_node (engine.rs:2080 parity),
read GitCheckpointResult for GitCommit/GitPush events in on_checkpoint,
read artifact_store count and last_git_sha in on_run_end.

HookLifecycle: add Skipped guard in after_node, add on_checkpoint for
CheckpointSaved hook.

DiskLifecycle: add on_run_start with write_manifest + write_run_status,
use write_node_status with visit-based directory naming.

GitLifecycle: full implementation — on_run_start resets last_git_sha and
inits metadata branch; on_checkpoint does shadow commit, run branch
commit, checkpoint re-save with SHA, push, and diff.patch; on_run_end
writes final.patch.

ArtifactLifecycle: full implementation — on_run_start swaps fresh store,
before_attempt records epoch, after_attempt collects assets and emits
AssetsCaptured, after_node offloads large values and syncs to sandbox.

Orchestrator: context seeding (mirror_graph_attributes, INTERNAL_RUN_ID,
INTERNAL_WORK_DIR) with is_initial_resume gating, shared state for
checkpoint_git_result/last_git_sha/artifact_store, full callback wiring.

Promote write_manifest, write_node_status, git_diff to pub(crate).
Add Clone to RunConfig. Constructor takes Arc<RunConfig> + is_resume.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 15:15:46 -04:00
Bryan Helmkamp
5e6eebaff8 Decompose monolithic WorkflowLifecycle into 8 focused sub-lifecycles
Split the 564-line core_adapter/lifecycle.rs into a lifecycle/ directory
with dedicated structs for each domain concern (event, hook, fidelity,
auto_status, circuit_breaker, disk, git, artifact), orchestrated by a
WorkflowLifecycle that enforces explicit per-callback ordering.

Also fixes core adapter boundary gaps:
- Handler now uses per-call snapshot/apply context bridge and real graph
  instead of STUB_GRAPH
- Executor::run() returns (Outcome, RunState) so run_via_core can
  extract the final context instead of returning an empty one
- run_via_core populates git_state on EngineServices for handlers
- Checkpoint resume gains stage_index, next_node_id fallback, and
  node_visits reconstruction for old checkpoints

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 14:46:10 -04:00
Bryan Helmkamp
4a7433a031 Deduplicate panic formatting, backoff construction, and stub graph allocation
- Extract format_panic_message() helper, used by both engine.rs and core_adapter
- Add RetryPolicy::DEFAULT_BACKOFF const, replacing 6 identical BackoffPolicy literals
- Cache stub graph via LazyLock to avoid per-call allocation in core_adapter handler
- Replace magic "success" string with StageStatus::Success.to_string()
- Bind graph.stall_timeout() once in run_via_core instead of calling twice

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 11:27:31 -04:00
Bryan Helmkamp
5822ba0260 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-24 11:02:35 -04:00