scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-07 08:27:12 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Merge origin/main into issue-135

Browse source 
Integrates PR #137 (Extract `fabro resume` subcommand):
- resume.rs: take main's comprehensive extraction (devcontainer support,
  status guards, project config discovery, labels)
- create.rs: preserve issue-135's run_id passthrough for create+start+attach
- run.rs: preserve issue-135's print_run_summary (used by unified path)
- main.rs: take main's verbose flag propagation for resume

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-22 13:48:26 -04:00
commit f28df7e536
9 changed files with 1530 additions and 146 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
.ai/prompts/code-review-deep-1.md Normal file
View file

@ -0,0 +1,35 @@
Provide a code review for this branch relative to the base branch for bugs and defects.
To do this, follow these steps precisely:
1. Use Git to retrieve a list of modified files in this branch.
2. Use a Haiku agent to give you a list of file paths to (but not the contents of) any relevant CLAUDE.md files from the codebase: the root CLAUDE.md file (if one exists), as well as any CLAUDE.md files in the directories whose files the pull request modified
3. Use a Haiku agent to view the branch's diff, and ask the agent to return a summary of the change
4. Then, launch 5 parallel Opus agents to independently code review the change for production bugs and vulnerabilities.
a. Agent #1: Read the git blame and history of the code modified, to identify any bugs in light of that historical context
b. Agent #2: Read code comments in the modified files, and make sure the changes in the pull request comply with any guidance in the comments.
c. Agent #3-5: Read the file changes in this branch, then do a scan for potential bugs. Focus on bugs with production / end-user impact, and avoid small issues and nitpicks.
Output a report with all the bugs using this format:
<code_review>
<bug>
<title>title of bug</title>
<description>brief description of bug</description>
<location>
<file>lib/crates/fabro-cli/src/commands/resume.rs</file>
<start_line>115</start_line>
<end_line>115</end_line>
</location>
<severity>critical/high/medium/low</severity>
</bug>
<bug>...</bug>
</code_review>
Write the report to: `.ai/tmp/candidate_bugs.xml`
Notes:
- Do not check build signal or attempt to build or typecheck the app. These will run separately, and are not relevant to your code review.
- Include all potential bugs of all severity (critical/high/medium/low) that have production / end-user impact. (We will analyze them separately later.)
- Make a todo list first

110
.ai/prompts/code-review-deep-2.md Normal file
View file

@ -0,0 +1,110 @@
Provide a code review for this branch relative to the base branch for bugs and defects.
We have a report of candidate bugs which you need to analyze.
To do this, follow these steps precisely:
1. Use Git to retrieve a list of modified files in this branch.
2. View the branch's diff and understand the changes
3. Then, launch 5 parallel Opus agents to independently assess the candidate bugs. For each bug, investigate it thoroughly in order to produce the report in the format below. If the candidate bug is not valid, then discard it.
Input: Read from `.ai/tmp/candidate_bugs.xml`
Output a report with all the bugs using this format:
```xml
<code_review>
<bug>
<summary>up to 3 sentences</summary>
<severity>important OR nit</severity>
<pre_exiting>yes OR no</pre_existing>
<location>
<file>lib/crates/fabro-cli/src/commands/resume.rs</file>
<start_line>115</start_line>
<end_line>115</end_line>
</location>
<extended_reasoning>
<what_the_bug_is>...</what_the_bug_is>
<the_specific_code_path_that_triggers_it>...</the_specific_code_path_that_triggers_it>
<why_existing_code_does_not_prevent_it>...</why_existing_code_does_not_prevent_it>
<impact>...</impact>
<how_to_fix_it>...</how_to_fix_it>
<step_by_step_proof>
<step>first step</step>
<step>second step</step>
<step>...</step>
<step>bug</step>
</step_by_step_proof>
</extended_reasoning>
</bug>
<bug>...</bug>
</code_review>
```
Here is a real-world example:
```xml
<bug>
<summary>
`prepare_from_checkpoint` unconditionally creates a `LocalSandbox` via `local_sandbox_with_callback`, completely ignoring the `--sandbox` flag and TOML config. A user running `fabro resume --checkpoint logs/checkpoint.json --workflow w.fabro --sandbox docker` will silently get a local sandbox instead of Docker; to fix this, call `resolve_sandbox_provider(args.sandbox.map(Into::into), None, run_defaults)` just as `prepare_from_branch` does.
</summary>
<severity>important</severity>
<pre_exiting>no</pre_existing>
<location>
<file>lib/crates/fabro-cli/src/commands/resume.rs</file>
<start_line>208</start_line>
<end_line>208</end_line>
</location>
<extended_reasoning>
<what_the_bug_is>
`prepare_from_checkpoint` (resume.rs, around line 196) always wires up a `LocalSandbox` regardless of what sandbox the caller requested:
```rust
let sandbox: Arc<dyn Sandbox> = local_sandbox_with_callback(original_cwd, Arc::clone(&emitter));
let sandbox: Arc<dyn Sandbox> = Arc::new(fabro_agent::ReadBeforeWriteSandbox::new(sandbox));
```
The `args.sandbox` field (a `Option<CliSandboxProvider>`) is populated by clap but never read inside this function. No error is raised and no warning is printed.
</what_the_bug_is>
<the_specific_code_path_that_triggers_it>
When a user invokes `fabro resume --checkpoint path/to/checkpoint.json --workflow w.fabro --sandbox docker`, `resume_command` sees `args.checkpoint.is_some()` and dispatches to `prepare_from_checkpoint`. That function builds the `ResumeContext` with a `LocalSandbox` and returns. The `--sandbox docker` value stored in `args.sandbox` is forwarded to `run_resumed` but by then the sandbox is already constructed and the field is never consulted.
</the_specific_code_path_that_triggers_it>
<why_existing_code_does_not_prevent_it>
`prepare_from_branch` — the sibling function for the run-ID path — correctly calls `resolve_sandbox_provider(args.sandbox.map(Into::into), None, run_defaults)` and dispatches through a `match sandbox_provider { ... }` that handles `Local`, `Docker`, `Ssh`, `Exe`, and `Daytona`. The checkpoint-file path was clearly authored separately and the sandbox resolution step was simply omitted. Additionally, the old `fabro run --resume checkpoint.json --sandbox docker` path ran through `run_command`, which performed sandbox resolution before the checkpoint branch — so this is a genuine regression of a previously-working feature.
</why_existing_code_does_not_prevent_it>
<impact>
Any user relying on `--sandbox docker` (for reproducibility, filesystem isolation, or container-specific tooling), `--sandbox ssh` (remote host execution), or `--sandbox exe` when resuming from a checkpoint file will silently run against the local filesystem instead. There is no error, no warning, and the job may produce different results or corrupt local state. The flag is prominently documented in both `docs/reference/cli.mdx` and the `--help` output, so users have every reason to expect it to work.
</impact>
<how_to_fix_it>
Replace the hardcoded `local_sandbox_with_callback` call in `prepare_from_checkpoint` with the same sandbox-resolution logic used by `prepare_from_branch`:
```rust
let sandbox_provider = if args.dry_run {
SandboxProvider::Local
} else {
resolve_sandbox_provider(args.sandbox.map(Into::into), None, run_defaults)?
};
// then match sandbox_provider { ... } as prepare_from_branch does
```
Note that `run_defaults` must also be threaded into `prepare_from_checkpoint` (currently it is not passed to this function), matching the signature of `prepare_from_branch`.
</how_to_fix_it>
<step_by_step_proof>
<step>User runs: `fabro resume --checkpoint ~/.fabro/runs/20260321-01ABC.../checkpoint.json --workflow deploy.fabro --sandbox docker`</step>
<step>`resume_command` evaluates `args.checkpoint.is_some()` → `true` → calls `prepare_from_checkpoint(&args, ...)`.</step>
<step>Inside `prepare_from_checkpoint`, `args.sandbox` holds `Some(CliSandboxProvider::Docker)` but is never read.</step>
<step>Line ~196: `let sandbox = local_sandbox_with_callback(original_cwd, Arc::clone(&emitter));` — a `LocalSandbox` is constructed unconditionally.</step>
<step>`ResumeContext { sandbox, ... }` is returned with the local sandbox.</step>
<step>`run_resumed` receives this context and runs the entire workflow inside the local sandbox.</step>
<step>Docker is never launched; no diagnostic message is emitted.</step>
</step_by_step_proof>
</extended_reasoning>
</bug>
```
Write the output to `.ai/tmp/analyzed_bugs.xml`
Notes:
- Do not check build signal or attempt to build or typecheck the app. These will run separately, and are not relevant to your code review.
- Make a todo list first

33
.ai/prompts/code-review-deep-3.md Normal file
View file

@ -0,0 +1,33 @@
Filter the bugs identified by code review to the bugs worth fixing.
We have a report of analyzed bugs which you need to filter.
To do this, follow these steps precisely:
1. Use Git to retrieve a list of modified files in this branch.
2. Use a Haiku agent to view the branch's diff, and ask the agent to return a summary of the change
3. For each bug assess if it is a false positive based on the criteria below.
Input: Read from `.ai/tmp/analyzed_bugs.xml`
Filter out the false positives. Examples of false positives:
- Nits
- Something that looks like a bug but is not actually a bug
- Pedantic issues that a senior engineer wouldn't call out
- Issues that a linter, typechecker, or compiler would catch (eg. missing or incorrect imports, type errors, broken tests, formatting issues, pedantic style issues like newlines). No need to run these build steps yourself -- it is safe to assume that they will be run separately as part of CI.
- General code quality issues (eg. lack of test coverage, general security issues, poor documentation)
- Maintainability, code smells, etc.
- Changes in functionality that are likely intentional or are directly related to the broader change
- Real issues, but are not related to the changes in the branch
Ouput:
1. Write to `.ai/tmp/valid_bugs.xml` in the same XML format with the false positives filtered out.
2. Write to `.ai/tmp/false_positives.md` a summary of the false positives you filtered out and why.
Notes:
- Do not check build signal or attempt to build or typecheck the app. These will run separately, and are not relevant to your code review.
- Make a todo list first
- It is OK to keep bugs which are pre-existing, if and only if they are both A) important and B) relevant to the changes being made.

92
.ai/prompts/code-review-fast.md Normal file
View file

@ -0,0 +1,92 @@
---
allowed-tools: Bash(gh issue view:*), Bash(gh search:*), Bash(gh issue list:*), Bash(gh pr comment:*), Bash(gh pr diff:*), Bash(gh pr view:*), Bash(gh pr list:*)
description: Code review a pull request
disable-model-invocation: false
---
Provide a code review for the given pull request.
To do this, follow these steps precisely:
1. Use a Haiku agent to check if the pull request (a) is closed, (b) is a draft, (c) does not need a code review (eg. because it is an automated pull request, or is very simple and obviously ok), or (d) already has a code review from you from earlier. If so, do not proceed.
2. Use another Haiku agent to give you a list of file paths to (but not the contents of) any relevant CLAUDE.md files from the codebase: the root CLAUDE.md file (if one exists), as well as any CLAUDE.md files in the directories whose files the pull request modified
3. Use a Haiku agent to view the pull request, and ask the agent to return a summary of the change
4. Then, launch 5 parallel Sonnet agents to independently code review the change. The agents should do the following, then return a list of issues and the reason each issue was flagged (eg. CLAUDE.md adherence, bug, historical git context, etc.):
a. Agent #1: Audit the changes to make sure they compily with the CLAUDE.md. Note that CLAUDE.md is guidance for Claude as it writes code, so not all instructions will be applicable during code review.
b. Agent #2: Read the file changes in the pull request, then do a shallow scan for obvious bugs. Avoid reading extra context beyond the changes, focusing just on the changes themselves. Focus on large bugs, and avoid small issues and nitpicks. Ignore likely false positives.
c. Agent #3: Read the git blame and history of the code modified, to identify any bugs in light of that historical context
d. Agent #4: Read previous pull requests that touched these files, and check for any comments on those pull requests that may also apply to the current pull request.
e. Agent #5: Read code comments in the modified files, and make sure the changes in the pull request comply with any guidance in the comments.
5. For each issue found in #4, launch a parallel Haiku agent that takes the PR, issue description, and list of CLAUDE.md files (from step 2), and returns a score to indicate the agent's level of confidence for whether the issue is real or false positive. To do that, the agent should score each issue on a scale from 0-100, indicating its level of confidence. For issues that were flagged due to CLAUDE.md instructions, the agent should double check that the CLAUDE.md actually calls out that issue specifically. The scale is (give this rubric to the agent verbatim):
a. 0: Not confident at all. This is a false positive that doesn't stand up to light scrutiny, or is a pre-existing issue.
b. 25: Somewhat confident. This might be a real issue, but may also be a false positive. The agent wasn't able to verify that it's a real issue. If the issue is stylistic, it is one that was not explicitly called out in the relevant CLAUDE.md.
c. 50: Moderately confident. The agent was able to verify this is a real issue, but it might be a nitpick or not happen very often in practice. Relative to the rest of the PR, it's not very important.
d. 75: Highly confident. The agent double checked the issue, and verified that it is very likely it is a real issue that will be hit in practice. The existing approach in the PR is insufficient. The issue is very important and will directly impact the code's functionality, or it is an issue that is directly mentioned in the relevant CLAUDE.md.
e. 100: Absolutely certain. The agent double checked the issue, and confirmed that it is definitely a real issue, that will happen frequently in practice. The evidence directly confirms this.
6. Filter out any issues with a score less than 80. If there are no issues that meet this criteria, do not proceed.
7. Use a Haiku agent to repeat the eligibility check from #1, to make sure that the pull request is still eligible for code review.
8. Finally, use the gh bash command to comment back on the pull request with the result. When writing your comment, keep in mind to:
a. Keep your output brief
b. Avoid emojis
c. Link and cite relevant code, files, and URLs
Examples of false positives, for steps 4 and 5:
- Pre-existing issues
- Something that looks like a bug but is not actually a bug
- Pedantic nitpicks that a senior engineer wouldn't call out
- Issues that a linter, typechecker, or compiler would catch (eg. missing or incorrect imports, type errors, broken tests, formatting issues, pedantic style issues like newlines). No need to run these build steps yourself -- it is safe to assume that they will be run separately as part of CI.
- General code quality issues (eg. lack of test coverage, general security issues, poor documentation), unless explicitly required in CLAUDE.md
- Issues that are called out in CLAUDE.md, but explicitly silenced in the code (eg. due to a lint ignore comment)
- Changes in functionality that are likely intentional or are directly related to the broader change
- Real issues, but on lines that the user did not modify in their pull request
Notes:
- Do not check build signal or attempt to build or typecheck the app. These will run separately, and are not relevant to your code review.
- Use `gh` to interact with Github (eg. to fetch a pull request, or to create inline comments), rather than web fetch
- Make a todo list first
- You must cite and link each bug (eg. if referring to a CLAUDE.md, you must link it)
- For your final comment, follow the following format precisely (assuming for this example that you found 3 issues):
---
### Code review
Found 3 issues:
1. <brief description of bug> (CLAUDE.md says "<...>")
<link to file and line with full sha1 + line range for context, note that you MUST provide the full sha and not use bash here, eg. https://github.com/anthropics/claude-code/blob/1d54823877c4de72b2316a64032a54afc404e619/README.md#L13-L17>
2. <brief description of bug> (some/other/CLAUDE.md says "<...>")
<link to file and line with full sha1 + line range for context>
3. <brief description of bug> (bug due to <file and code snippet>)
<link to file and line with full sha1 + line range for context>
🤖 Generated with [Claude Code](https://claude.ai/code)
<sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub>
---
- Or, if you found no issues:
---
### Code review
No issues found. Checked for bugs and CLAUDE.md compliance.
🤖 Generated with [Claude Code](https://claude.ai/code)
- When linking to code, follow the following format precisely, otherwise the Markdown preview won't render correctly: https://github.com/anthropics/claude-cli-internal/blob/c21d3c10bc8e898b7ac1a2d745bdc9bc4e423afe/package.json#L10-L15
- Requires full git sha
- You must provide the full sha. Commands like `https://github.com/owner/repo/blob/$(git rev-parse HEAD)/foo/bar` will not work, since your comment will be directly rendered in Markdown.
- Repo name must match the repo you're code reviewing
- # sign after the file name
- Line range format is L[start]-L[end]
- Provide at least 1 line of context before and after, centered on the line you are commenting about (eg. if you are commenting about lines 5-6, you should link to `L4-7`)

2
.gitignore vendored
View file

@ -8,3 +8,5 @@ evals/swe-bench/results/
evals/swe-bench/dockerfiles/
__pycache__
.ai/plans
.ai/comments
.ai/tmp

1
docs/reference/cli.mdx
View file

@ -92,6 +92,7 @@ fabro resume --checkpoint path/to/checkpoint.json --workflow workflow.fabro
| `--goal-file <FILE>` | Read the goal from a file |
| `--no-retro` | Skip retro generation after the run |
| `--preserve-sandbox` | Keep the sandbox alive after the run finishes |
| `--label <KEY=VALUE>` | Attach a label to this run (repeatable) |
## `fabro ps`

1251
lib/crates/fabro-cli/src/commands/resume.rs
View file

File diff suppressed because it is too large Load diff

149
lib/crates/fabro-cli/src/commands/run.rs
View file

@ -373,7 +373,7 @@ pub(crate) fn resolve_ssh_clone_params(
///
/// `apply_defaults` must be called on `run_cfg` before this — it merges
/// `run_defaults.llm.fallbacks` into `run_cfg.llm.fallbacks` already.
fn resolve_fallback_chain(
pub(crate) fn resolve_fallback_chain(
provider: Provider,
model: &str,
run_cfg: Option<&WorkflowRunConfig>,
@ -392,7 +392,7 @@ fn resolve_fallback_chain(
///
/// Signs a JWT, resolves `owner/repo` from `origin_url`, and requests a
/// scoped token. Returns the token string on success.
async fn mint_github_token(
pub(crate) async fn mint_github_token(
creds: &fabro_github::GitHubAppCredentials,
origin_url: &str,
permissions: &HashMap<String, String>,
@ -430,14 +430,14 @@ enum WorkdirStrategy {
/// Accumulates token usage and cost across all workflow stages.
#[derive(Default)]
struct CostAccumulator {
total_input_tokens: i64,
total_output_tokens: i64,
total_cache_read_tokens: i64,
total_cache_write_tokens: i64,
total_reasoning_tokens: i64,
total_cost: f64,
has_pricing: bool,
pub(crate) struct CostAccumulator {
pub total_input_tokens: i64,
pub total_output_tokens: i64,
pub total_cache_read_tokens: i64,
pub total_cache_write_tokens: i64,
pub total_reasoning_tokens: i64,
pub total_cost: f64,
pub has_pricing: bool,
}
/// Create a [`LocalSandbox`] wired to emit [`WorkflowRunEvent::Sandbox`] events.
@ -479,7 +479,7 @@ pub(crate) async fn write_run_config_snapshot(
Ok(())
}
fn resolve_workflow_source(
pub(crate) fn resolve_workflow_source(
workflow_path: &Path,
) -> anyhow::Result<(PathBuf, Option<WorkflowRunConfig>)> {
let path = project_config::resolve_workflow_arg(workflow_path)?;
@ -521,22 +521,34 @@ pub(crate) struct PreparedWorkflow {
/// Shared between `create_run` (which only persists the spec) and
/// `run_command` (which goes on to execute the workflow).
pub(crate) fn prepare_workflow(
args: &RunArgs,
run_defaults: RunDefaults,
styles: &Styles,
quiet: bool,
) -> anyhow::Result<PreparedWorkflow> {
prepare_workflow_with_project_config(args, run_defaults, styles, quiet, true)
}
pub(crate) fn prepare_workflow_with_project_config(
args: &RunArgs,
mut run_defaults: RunDefaults,
styles: &Styles,
quiet: bool,
apply_project_config: bool,
) -> anyhow::Result<PreparedWorkflow> {
let workflow_path = args
.workflow
.as_ref()
.ok_or_else(|| anyhow::anyhow!("--workflow is required"))?;
// Apply project-level config overrides (fabro.toml) on top of CLI defaults.
if let Ok(Some((_config_path, project_config))) =
project_config::discover_project_config(&std::env::current_dir().unwrap_or_default())
{
tracing::debug!("Applying run defaults from fabro.toml");
run_defaults.merge_overlay(project_config.into_run_defaults());
if apply_project_config {
// Apply project-level config overrides (fabro.toml) on top of CLI defaults.
if let Ok(Some((_config_path, project_config))) =
project_config::discover_project_config(&std::env::current_dir().unwrap_or_default())
{
tracing::debug!("Applying run defaults from fabro.toml");
run_defaults.merge_overlay(project_config.into_run_defaults());
}
}
// Resolve workflow arg, load run config if TOML, apply defaults
@ -2616,6 +2628,109 @@ mod tests {
assert!(result.is_err());
}
#[test]
fn prepare_workflow_with_project_config_resolves_workflow_toml_settings() {
let dir = tempfile::tempdir().unwrap();
std::fs::write(
dir.path().join("workflow.fabro"),
r#"digraph smoke {
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
work [label="Work", prompt="Do the work"]
start -> work -> exit
}"#,
)
.unwrap();
std::fs::write(
dir.path().join("workflow.toml"),
r#"
version = 1
graph = "workflow.fabro"
goal = "toml goal"
[setup]
commands = ["echo from toml"]
[sandbox]
provider = "docker"
[llm]
model = "gpt-5.2"
provider = "openai"
[pull_request]
enabled = true
[assets]
include = ["*.md"]
"#,
)
.unwrap();
let args = RunArgs {
workflow: Some(dir.path().join("workflow.toml")),
run_dir: None,
dry_run: false,
preflight: false,
auto_approve: false,
goal: None,
goal_file: None,
model: None,
provider: None,
verbose: false,
sandbox: None,
label: Vec::new(),
no_retro: false,
preserve_sandbox: false,
detach: false,
run_id: None,
};
let styles = Styles::new(false);
let prepared = prepare_workflow_with_project_config(
&args,
RunDefaults::default(),
&styles,
true,
false,
)
.unwrap();
assert_eq!(prepared.graph.name, "smoke");
assert_eq!(prepared.graph.goal(), "toml goal");
assert_eq!(prepared.sandbox_provider, SandboxProvider::Docker);
assert_eq!(prepared.model, "gpt-5.2");
assert_eq!(prepared.provider.as_deref(), Some("openai"));
let run_cfg = prepared
.run_cfg
.as_ref()
.expect("run config should be loaded");
assert_eq!(
run_cfg
.setup
.as_ref()
.expect("setup config should be preserved")
.commands,
vec!["echo from toml".to_string()]
);
assert!(
run_cfg
.pull_request
.as_ref()
.expect("pull request config should be preserved")
.enabled
);
assert_eq!(
run_cfg
.assets
.as_ref()
.expect("assets config should be preserved")
.include,
vec!["*.md".to_string()]
);
}
#[test]
fn apply_goal_override_cli_wins_over_toml() {
use fabro_graphviz::graph::{AttrValue, Graph};

3
lib/crates/fabro-cli/src/main.rs
View file

@ -937,10 +937,11 @@ async fn main_inner() -> (String, Result<()>) {
commands::secret::set_command(&args)?;
}
},
Command::Resume(args) => {
Command::Resume(mut args) => {
let styles: &'static fabro_util::terminal::Styles =
Box::leak(Box::new(fabro_util::terminal::Styles::detect_stderr()));
let cli_config = cli_config::load_cli_config(None)?;
args.verbose = args.verbose || cli_config.verbose;
#[cfg(feature = "sleep_inhibitor")]
let _sleep_guard = fabro_beastie::guard(cli_config.prevent_idle_sleep);
let github_app = build_github_app_credentials(cli_config.app_id());