mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-12 23:02:41 +00:00
parent
6272a66f77
commit
7b249487c8
5 changed files with 242 additions and 8 deletions
File diff suppressed because one or more lines are too long
121
nodes/implement/prompt.md
Normal file
121
nodes/implement/prompt.md
Normal file
|
|
@ -0,0 +1,121 @@
|
|||
Goal: # Plan: Extract `fabro resume` subcommand
|
||||
|
||||
## Context
|
||||
|
||||
Resume functionality is currently embedded in `fabro run` via `--resume` (checkpoint file) and `--run-branch` (git branch). This makes the `run` command's arg surface complex with `conflicts_with` annotations, and the UX is unintuitive — users must construct `fabro/run/RUN_ID` branch names manually. The new `fabro resume` subcommand provides a cleaner interface: `fabro resume RUN_ID_OR_PREFIX`.
|
||||
|
||||
## New `ResumeArgs` struct
|
||||
|
||||
```rust
|
||||
pub struct ResumeArgs {
|
||||
/// Run ID, prefix, or branch (fabro/run/...)
|
||||
#[arg(required_unless_present = "checkpoint")]
|
||||
pub run: Option<String>,
|
||||
|
||||
/// Resume from a checkpoint file (requires --workflow)
|
||||
#[arg(long)]
|
||||
pub checkpoint: Option<PathBuf>,
|
||||
|
||||
/// Override workflow graph (required with --checkpoint)
|
||||
#[arg(long)]
|
||||
pub workflow: Option<PathBuf>,
|
||||
|
||||
// Shared run options: run_dir, dry_run, auto_approve, goal, goal_file,
|
||||
// model, provider, verbose, sandbox, no_retro, ssh, preserve_sandbox
|
||||
}
|
||||
```
|
||||
|
||||
**Run ID resolution** (at top of `resume_command()`):
|
||||
- If `run` starts with `fabro/run/` → strip prefix to get run_id
|
||||
- Otherwise → call `find_run_id_by_prefix(&repo, &run)` (same as `rewind`/`fork`)
|
||||
- Then construct branch name as `fabro/run/{run_id}`
|
||||
|
||||
## Files to modify
|
||||
|
||||
### 1. New: `lib/crates/fabro-cli/src/commands/resume.rs`
|
||||
- Define `ResumeArgs` struct
|
||||
- Move `run_from_branch()` body (~315 lines, `run.rs:1811-2125`) into `pub async fn resume_command()`
|
||||
- Add run ID resolution logic at top (prefix → full ID via `find_run_id_by_prefix`)
|
||||
- Add `--checkpoint` path: validate `--workflow` is present, load graph via `prepare_from_file()`, load checkpoint via `Checkpoint::load()`, then run engine
|
||||
|
||||
### 2. `lib/crates/fabro-cli/src/commands/run.rs`
|
||||
- **Remove from `RunArgs`**: `resume` field (line 97-99), `run_branch` field (line 101-103)
|
||||
- **Simplify `workflow`**: remove `required_unless_present = "run_branch"` — it's now always required
|
||||
- **Update `conflicts_with_all`**: remove `"resume"`/`"run_branch"` from `preflight` (line 90) and `detach` (line 146)
|
||||
- **Remove** `run_from_branch()` function (lines 1811-2125)
|
||||
- **Remove** the `run_branch` early-return at top of `run_command()` (lines 602-604)
|
||||
- **Simplify** engine call: remove `if let Some(ref checkpoint_path) = args.resume` branch (lines 1467-1476), always pass `None` for checkpoint
|
||||
- **Widen visibility** of helpers used by `resume.rs`:
|
||||
- `local_sandbox_with_callback` (line 439) → `pub(crate)`
|
||||
- `resolve_ssh_config` (line 341) → `pub(crate)`
|
||||
- `resolve_ssh_clone_params` (line 355) → `pub(crate)`
|
||||
- `resolve_exe_config` (line 313) → `pub(crate)`
|
||||
- `resolve_exe_clone_params` (line 328) → `pub(crate)`
|
||||
- `resolve_preserve_sandbox` (line 261) → `pub(crate)`
|
||||
- `generate_retro` (line 2560) → `pub(crate)`
|
||||
- `write_finalize_commit` (line 2523) → `pub(crate)`
|
||||
- `print_final_output` (line 2128) → `pub(crate)`
|
||||
- `print_assets` (line 2149) → `pub(crate)`
|
||||
|
||||
### 3. `lib/crates/fabro-cli/src/commands/mod.rs`
|
||||
- Add `pub mod resume;`
|
||||
|
||||
### 4. `lib/crates/fabro-cli/src/main.rs`
|
||||
- Add `Resume(commands::resume::ResumeArgs)` to `Command` enum (near line 170, alongside `Rewind`/`Fork`)
|
||||
- Add `Command::Resume(_) => "resume"` to command_name match
|
||||
- Add dispatch handler (pattern follows `Rewind`/`Fork`/`Wait` — create styles, load cli_config, build github_app/git_author, call `resume_command()`)
|
||||
|
||||
### 5. `lib/crates/fabro-workflows/src/run_spec.rs`
|
||||
- Remove `resume` and `run_branch` fields from `RunSpec`
|
||||
- Add `#[serde(default)]` to `RunSpec` for backward compat with existing `spec.json` files
|
||||
- Update `sample_spec()` in tests
|
||||
|
||||
### 6. `lib/crates/fabro-cli/src/commands/create.rs`
|
||||
- Remove lines 86-87 that set `resume` and `run_branch` in the spec
|
||||
|
||||
### 7. `lib/crates/fabro-cli/src/main.rs` (`_run_engine` handler)
|
||||
- Remove lines setting `resume` and `run_branch` when reconstructing `RunArgs` from `RunSpec`
|
||||
|
||||
### 8. `lib/crates/fabro-cli/src/commands/rewind.rs` (line 48-52)
|
||||
- Change hint: `"To resume: fabro resume {run_id}"` (use short prefix)
|
||||
|
||||
### 9. `lib/crates/fabro-cli/src/commands/fork.rs` (line 56-60)
|
||||
- Change hint: `"To resume: fabro resume {new_run_id}"` (use short prefix)
|
||||
|
||||
### 10. `lib/crates/fabro-cli/tests/cli.rs`
|
||||
- Update/remove tests referencing `--resume` or `--run-branch` on `fabro run`
|
||||
- Add basic parse test for `fabro resume`
|
||||
|
||||
### 11. Documentation (`docs/`)
|
||||
- Update `docs/reference/cli.mdx`: add `fabro resume` section, remove `--resume`/`--run-branch` from `fabro run`
|
||||
- Update `docs/execution/checkpoints.mdx`: change resume examples
|
||||
- Update any other docs referencing `fabro run --run-branch` or `fabro run --resume`
|
||||
|
||||
## Verification
|
||||
|
||||
1. `cargo build --workspace` — compiles cleanly
|
||||
2. `cargo test --workspace` — all tests pass
|
||||
3. `cargo clippy --workspace -- -D warnings` — no warnings
|
||||
4. Manual: `fabro resume --help` shows expected args
|
||||
5. Manual: `fabro run --help` no longer shows `--resume` or `--run-branch`
|
||||
|
||||
|
||||
## Completed stages
|
||||
- **toolchain**: success
|
||||
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
|
||||
- Stdout:
|
||||
```
|
||||
cargo 1.94.0 (85eff7c80 2026-01-15)
|
||||
```
|
||||
- Stderr: (empty)
|
||||
- **preflight_compile**: success
|
||||
- Script: `cargo check -q --workspace 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
- **preflight_lint**: success
|
||||
- Script: `cargo clippy -q --workspace -- -D warnings 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
|
||||
|
||||
Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.
|
||||
5
nodes/implement/provider_used.json
Normal file
5
nodes/implement/provider_used.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"mode": "agent",
|
||||
"provider": "anthropic",
|
||||
"model": "claude-opus-4-6"
|
||||
}
|
||||
60
nodes/implement/response.md
Normal file
60
nodes/implement/response.md
Normal file
|
|
@ -0,0 +1,60 @@
|
|||
These are all valid usages - the PR code uses `run_branch` from the manifest (different from the CLI arg), and resume.rs naturally uses `run_branch` as a local variable. The `run.rs` references are the engine's `RunConfig.run_branch` field which is still needed. All correct.
|
||||
|
||||
Let me do a final summary of all the changes made:
|
||||
|
||||
## Summary of Changes
|
||||
|
||||
### New file: `lib/crates/fabro-cli/src/commands/resume.rs`
|
||||
- New `ResumeArgs` struct with `run`, `checkpoint`, `workflow`, and all shared run options
|
||||
- `resume_command()` function with two paths:
|
||||
- **Checkpoint path**: loads checkpoint file + workflow, runs engine locally
|
||||
- **Run ID path**: resolves run ID by prefix (like rewind/fork), reads checkpoint/graph from git metadata branch, resumes with worktree
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/src/commands/run.rs`
|
||||
- Removed `resume` and `run_branch` fields from `RunArgs`
|
||||
- Simplified `workflow` arg (no longer `required_unless_present`)
|
||||
- Simplified `preflight` conflicts (removed `resume`/`run_branch`)
|
||||
- Simplified `detach` conflicts (removed `resume`/`run_branch`)
|
||||
- Removed `run_branch` early-return at top of `run_command()`
|
||||
- Simplified engine call: always passes `None` for checkpoint
|
||||
- Removed entire `run_from_branch()` function (~320 lines)
|
||||
- Made 10 helper functions `pub(crate)`: `local_sandbox_with_callback`, `resolve_ssh_config`, `resolve_ssh_clone_params`, `resolve_exe_config`, `resolve_exe_clone_params`, `resolve_preserve_sandbox`, `print_final_output`, `print_assets`, `write_finalize_commit`, `generate_retro`
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/src/commands/mod.rs`
|
||||
- Added `pub mod resume;`
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/src/main.rs`
|
||||
- Added `Resume(commands::resume::ResumeArgs)` to `Command` enum
|
||||
- Added `Command::Resume(_) => "resume"` to command_name match
|
||||
- Added dispatch handler for `Command::Resume`
|
||||
- Removed `resume` and `run_branch` from `_run_engine` `RunArgs` reconstruction
|
||||
|
||||
### Modified: `lib/crates/fabro-workflows/src/run_spec.rs`
|
||||
- Removed `resume` and `run_branch` fields from `RunSpec`
|
||||
- Added `#[serde(default)]` for backward compat with existing spec.json files
|
||||
- Added `Default` impl for `RunSpec`
|
||||
- Updated `sample_spec()` in tests
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/src/commands/create.rs`
|
||||
- Removed lines setting `resume` and `run_branch` in the spec
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/src/commands/start.rs`
|
||||
- Removed `resume` and `run_branch` from test spec construction
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/src/commands/rewind.rs`
|
||||
- Changed hint to `"To resume: fabro resume {short_prefix}"`
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/src/commands/fork.rs`
|
||||
- Changed hint to `"To resume: fabro resume {short_prefix}"`
|
||||
|
||||
### Modified: `lib/crates/fabro-cli/tests/cli.rs`
|
||||
- Removed `detach_conflicts_with_resume` test
|
||||
- Removed `resume`/`run_branch` from spec JSON in test helpers
|
||||
- Added `resume_help_shows_expected_args` test
|
||||
- Added `resume_requires_run_or_checkpoint` test
|
||||
- Added `run_help_no_longer_shows_resume_or_run_branch` test
|
||||
|
||||
### Documentation updates:
|
||||
- **`docs/reference/cli.mdx`**: Added `fabro resume` section, removed `--resume`/`--run-branch` from `fabro run`, updated rewind/fork examples
|
||||
- **`docs/execution/checkpoints.mdx`**: Updated all resume examples to use `fabro resume`
|
||||
- **`docs/core-concepts/how-fabro-works.mdx`**: Updated resume examples to use `fabro resume`
|
||||
6
nodes/implement/status.json
Normal file
6
nodes/implement/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"status": "success",
|
||||
"notes": "Stage completed: implement",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-03-21T14:35:45.956453+00:00"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue