Commit graph

99 commits

Author SHA1 Message Date
Bryan Helmkamp
501f0e76d1 fmt: apply nightly rustfmt and document nightly requirement
The rustfmt.toml uses nightly-only options (struct_field_align_threshold,
imports_granularity, etc.) so stable rustfmt silently skips them,
producing different output. Use cargo +nightly fmt going forward.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 12:22:51 -04:00
Bryan Helmkamp
1a5d8e3bf8 fix: preserve file path in config parse errors and fix test assertion
The error standardization lost the file path from parse error messages
when anyhow::Context was removed. Add path field to ParseSettings
variant so errors like "Failed to parse settings file at /path: ..."
include the file location. Also fix test that expected capitalized
"Workflow not found" to match the new lowercase error message.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 12:20:17 -04:00
Bryan Helmkamp
f76e28b7a7 refactor: standardize crate error types 2026-04-11 11:53:53 -04:00
Bryan Helmkamp
5eeacd7864 fmt 2026-04-11 11:27:46 -04:00
Bryan Helmkamp
5ed04c3c57 feat(template): unify workflow and config template syntax
Add a shared MiniJinja-based template crate and migrate workflow prompts,
imports, hooks, and InterpString env references to the new {{ ... }}
syntax. This also threads typed run inputs through workflow rendering and
updates docs and tests to match the new templating model.
2026-04-11 10:58:50 -04:00
Bryan Helmkamp
8014f61054 chore(lint): fix all clippy warnings including --tests
Resolve every clippy warning across the workspace when running with
--tests enabled. Previously only library code was lint-clean; test
code had accumulated issues that were invisible without --tests.

Fixes:
- redundant_closure_for_method_calls: |s| s.as_source() -> InterpString::as_source
  (effective_settings, resolve_cli/root/server/features, run_event/record_serde,
  materialize_run) — add InterpString imports where needed
- absolute_paths: inline fabro_types::settings::* paths -> use imports;
  add #![allow(clippy::absolute_paths)] to fabro-cli and fabro-server
  IT test harnesses (matching the existing pattern in integration.rs)
- bool_assert_comparison: assert_eq!(x, true) -> assert!(x)
- needless_raw_string_hashes: r#"..."# -> r"..." where no inner quotes
- field_reassign_with_default: mut + field assign -> struct literal with ..Default
- match_same_arms: merge Timeout | Disconnected arms in attach.rs
- needless_pass_by_value: signal_rx by ref in attach.rs
- unreadable_literal: 9999999999 -> 9_999_999_999
- default_trait_access: Default::default() -> BTreeMap::default()
- items_after_statements: move use to function top
- large_futures: allow in integration.rs test module (test-only, not prod)
- filter_map_bool_then: .filter_map(bool::then) -> .filter().map()

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 10:16:24 -04:00
Bryan Helmkamp
67077d1d4d refactor(config): use lookup injection for active_settings_path env test
The active_settings_path_honors_fabro_config_env test was using an
EnvGuard that called std::env::set_var/remove_var — unsafe shared
mutable state in a parallel test binary.

Extract active_settings_path_with_lookup that takes an env-lookup
closure (same pattern as resolve_auth_mode_with_lookup in jwt_auth.rs).
Rewrite the test to inject the env value via the closure. Delete the
EnvGuard struct — no remaining callers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 08:49:31 -04:00
Bryan Helmkamp
98df4a82c9 refactor(settings): remove bridge shims and restore contracts
Drop the dead sandbox and hook bridge helpers that no longer have runtime
callers, and move the run settings serde coverage into fabro-types where the
wire types live. Add the missing /api/v1/runs/:id/settings contract test so the
outward sparse settings shape stays covered after the refactor.
2026-04-10 08:35:08 -04:00
Bryan Helmkamp
c5c81d2985 refactor(settings): rename settings layer and move parsing 2026-04-10 08:10:06 -04:00
Bryan Helmkamp
990812db3f refactor(settings): remove sparse settings compatibility layer 2026-04-10 07:58:46 -04:00
Bryan Helmkamp
238fd76cfc refactor(settings): resolve feature flags 2026-04-10 07:16:10 -04:00
Bryan Helmkamp
0563f0c123 refactor(settings): resolve workflow settings 2026-04-10 07:12:29 -04:00
Bryan Helmkamp
f716ffc929 refactor(settings): resolve project settings 2026-04-10 07:10:32 -04:00
Bryan Helmkamp
a93bf1df9c refactor(settings): resolve cli settings 2026-04-10 07:04:34 -04:00
Bryan Helmkamp
5dd6b88415 refactor(settings): resolve run settings and materialize defaults
Add the resolved run namespace, materialize persisted run defaults at create
time, and migrate the main workflow/server/CLI runtime paths off the old
run bridges.
2026-04-10 06:54:31 -04:00
Bryan Helmkamp
6572761a82 refactor(settings): resolve server settings in fabro-config
Add the server-side resolved settings view and move server startup,
auth, OAuth, TLS, and settings redaction paths onto that validated
shape. This lands the server pilot slice of the settings refactor
without changing the sparse persisted/API settings model.
2026-04-09 22:37:02 -04:00
Bryan Helmkamp
872783b4ed chore(simplify): cleanup from review of recent commits
- Use FABRO_LOCAL_NO_AUTH_ENV const in start.rs and tests instead of
  the literal it was hoisted from.
- Preserve error chain in resolve_goal_override via anyhow::Error::from
  rather than stringifying through anyhow!.
- Drop {source} from ResolveGoalError::Io Display to avoid duplicate
  text under anyhow's chain formatter.
- Fail loud in setup_register when ConfigLayer reload or parent dir
  creation errors instead of silently leaving stale state.
- Promote resolve_goal_file_path to pub and call it from fabro-config
  to dedupe the absolute-or-base.join logic.
- Trim narrator-voice paragraphs from tls_config and web_auth comments.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:03:46 -04:00
Bryan Helmkamp
fcf3152daf fix(config): route project/workflow loaders through ConfigLayer::load
The Stage 6 audit caught that `load_project_config` and `load_run_config`
bypassed `ConfigLayer::load` and called `parse_project_config` /
`ConfigLayer::parse` directly. As a result, `resolve_goal_file_paths` —
which rewrites relative `[run.goal] file = "..."` paths to absolute
against the declaring file's directory — only fired for
`~/.fabro/settings.toml`, never for `fabro.toml` or `workflow.toml`.

That meant a project author writing

    [run.goal]
    file = "prompts/goal.md"

would have the relative path survive all the way to consume time and
get resolved against the run's `working_directory` instead of the
config-file directory, contradicting the agreed "config-file rooted"
rule and breaking the most common case.

Both loaders now delegate to `ConfigLayer::load(path)`, which performs
the load-time rewrite. The user-settings path was already correct.

## Tests

- `load_project_config_rewrites_relative_goal_file_path`
- `load_run_config_rewrites_relative_goal_file_path`
- `load_run_config_leaves_absolute_goal_file_untouched`
- `build_manifest_resolves_relative_goal_file_in_project_config` —
  end-to-end via `build_run_manifest`, asserting the absolute path lands
  in `manifest.goal.path` and the file contents land in
  `manifest.goal.text`.
- `build_manifest_resolves_relative_goal_file_in_workflow_config` — same
  shape but exercising `workflow.toml`-declared goal files, which
  resolve relative to the much deeper workflow directory rather than
  the project root.

3,787 workspace tests pass (was 3,782, +5 new). `cargo fmt --check
--all` and `cargo clippy --workspace -- -D warnings` are clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 20:41:00 -04:00
Bryan Helmkamp
2b2f158c16 feat(settings): run.goal tagged union (inline | file)
`--goal-file` was broken in the v2 path: `TryFrom<&RunArgs> for ConfigLayer`
did `let _ = &args.goal_file;`, so clap accepted the flag listed in
`--help` and then silently dropped it. Users running
`fabro run demo --goal-file prompts/goal.md` ended up with no goal at
all (or the DOT graph-level fallback), a regression from the legacy
flat `Settings` shape.

This commit adds first-class support for both inline and file-sourced
goals via a tagged union on `run.goal`. Greenfield decisions:

- **Single field, two variants.** `RunGoalLayer` is an untagged enum
  of `Inline(InterpString)` and `File { file: InterpString }`. Makes
  `goal XOR goal_file` un-representable in the type system and lets
  the v2 merge matrix treat `run.goal` as a single scalar
  (last-writer-wins) instead of needing a custom mutual-exclusion
  merge rule. Matches the existing `DaytonaDockerfileLayer` pattern.

- **Relative paths are anchored at the file that declared them.**
  `ConfigLayer::load(path)` walks the just-parsed `SettingsFile` and
  rewrites any literal relative `run.goal.file` path to absolute
  using `path.parent()` as the base, via new
  `fabro_config::config::resolve_goal_file_paths`. CLI-sourced paths
  via `--goal-file` are anchored at CWD in
  `overrides::goal_layer_from_args`. Env-interpolated paths
  (`${env.GOALS_DIR}/goal.md`) are left unresolved until consume time
  and then resolved against the run's working_directory.

- **New accessors, no shims.**
  - `run_goal_layer() -> Option<&RunGoalLayer>` — raw variant access.
  - `run_goal_inline_str() -> Option<String>` — inline-only, returns
    `None` for file-sourced goals.
  - `resolve_run_goal(base_dir) -> Result<Option<ResolvedRunGoal>>` —
    reads the file from disk if needed, returns text + provenance
    (`ResolvedGoalSource::Inline | File { path }`).
  - New `ResolveGoalError` enum covers env-lookup and I/O failures.
  - Old `run_goal() / run_goal_str()` are **deleted** outright; every
    call site has been updated to pick the right variant.

- **CLI wiring (the actual bug fix).** `overrides::goal_layer_from_args`
  replaces the two `let _ = &args.goal_file;` lines with real
  resolution: `(Some(text), None)` → `Inline`, `(None, Some(path))` →
  `File { file: absolute }`. Both-set is rejected by a helper error
  and clap already had `conflicts_with = "goal"` as a belt-and-
  braces check. Applied to both `RunArgs` and `PreflightArgs`.

- **Manifest builder.** `resolve_manifest_goal` now calls
  `args_layer.as_v2().resolve_run_goal()` and
  `settings.resolve_run_goal()` in precedence order, then falls
  through to the graph-level `@file` sugar if both are absent. The
  resolved goal is translated to a `ManifestGoal { text, type_, path }`
  by a new `resolved_goal_to_manifest` helper — inline goals get
  `type = Value`, file-sourced goals get `type = File` with the
  absolute path echoed for provenance.

- **Workflow pipeline.** `fabro-workflow::operations::source::
  resolve_goal_override` is rewritten to use `resolve_run_goal`
  against the working_directory. The orphaned helper `resolve_goal_file`
  (a stub from Stage 4 that was always called with `None`) is
  deleted.

- **Server-side manifest.** `fabro-server::run_manifest::
  prepare_manifest` stores the CLI-resolved goal as
  `RunGoalLayer::Inline`, matching the Stage 4 plan's "CLI owns goal
  file reads; server never touches the filesystem for goals"
  contract.

## Tests

**Schema** (`fabro-types::settings::accessors`):
- `run_goal_inline_str_returns_source_value` — literal inline variant
- `run_goal_inline_str_is_none_for_file_variant` — file variant
  explicitly yields `None` from the inline accessor
- `resolve_run_goal_reads_file_variant_from_disk` — end-to-end file
  read with provenance assertion
- `resolve_run_goal_inline_passes_text_through` — inline passthrough

**Config load** (`fabro-config::config`):
- `parse_accepts_inline_goal` + `parse_accepts_file_variant`
- `parse_rejects_goal_with_unknown_sibling_fields` — untagged enum
  correctly rejects mixed-shape TOML
- `combine_replaces_file_goal_with_inline_from_higher_layer` and the
  reverse — confirms the tagged union merges as a single scalar with
  no custom rule needed
- `load_rewrites_relative_goal_file_to_absolute`
- `load_leaves_absolute_goal_file_untouched`
- `load_leaves_env_interpolated_goal_file_untouched`

**CLI overrides** (`fabro-cli::commands::run::overrides`):
- `goal_and_goal_file_together_is_rejected`
- `goal_file_is_anchored_at_cwd_when_relative`
- `absolute_goal_file_is_preserved`
- `inline_goal_builds_inline_variant`
- `empty_args_produce_no_goal_layer`

**CLI integration** (`fabro-cli::tests:🇮🇹:cmd::run`):
- `dry_run_with_goal_file_reads_contents_into_goal` — end-to-end
  `fabro run --dry-run --auto-approve --goal-file <path>` and asserts
  the file contents appear in the preflight summary. Explicit
  regression test for the silently-ignored flag.
- `dry_run_rejects_goal_and_goal_file_together` — clap conflicts_with

## Callsite churn

Every `run_goal() / run_goal_str()` call site updated:
- `fabro-config/src/effective_settings.rs` — 2 test assertions →
  `run_goal_inline_str()`
- `fabro-cli/tests/it/cmd/{config,create}.rs` — 3 sites → inline
- `fabro-cli/src/manifest_builder.rs` — rewritten to use
  `resolve_run_goal`
- `fabro-workflow/src/operations/create.rs` — 2 sites, test + set
- `fabro-workflow/src/operations/source.rs` — rewritten
- `fabro-server/src/{run_manifest,server}.rs` — set + test assertion

3,782 workspace tests pass (was 3,765, +17 new). `cargo fmt
--check --all` and `cargo clippy --workspace -- -D warnings` are
clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 19:47:12 -04:00
Bryan Helmkamp
4e7839c202 refactor(settings): stage 6.5b sweep ::v2:: prefix out of consumers
Final mechanical pass: replaces every remaining
`fabro_types::settings::v2::*` import path with
`fabro_types::settings::*` (or the appropriate submodule) across 53
files in 10 crates, then deletes the transitional
`pub mod v2 { pub use super::*; }` alias from
`fabro-types/src/settings/mod.rs`.

No functional changes — all touches are `sed s|settings::v2::|settings::|g`
on import statements and fully-qualified type paths. The v2
namespace is now fully gone; the authoritative module path is
`fabro_types::settings::{accessors, cli, duration, features, interp,
model_ref, project, run, server, size, splice_array, tree, version,
workflow}`.

All 3,758 workspace tests pass. `cargo fmt --check --all` and
`cargo clippy --workspace -- -D warnings` are clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 18:42:05 -04:00
Bryan Helmkamp
581e063883 refactor(settings): stage 6.3b promote hook + project runtime types
Two more legacy runtime type modules deleted from `fabro-types`:

**project.rs** (19 LOC): `ProjectSettings` was a trivial one-field
struct with a `pub use` re-export in `fabro-config/src/project.rs`.
Nothing else referenced it. Deleted outright; `fabro-config/src/project.rs`
drops the re-export and fixes up a `v2::` import path.

**hook.rs** (230 LOC): `HookDefinition`, `HookEvent`, `HookSettings`,
`HookType`, `TlsMode` plus the `resolved_hook_type` / `is_blocking`
/ `timeout` / `runs_in_sandbox` / `effective_name` behavior methods
are **moved** (not just re-exported) into
`fabro-hooks/src/config.rs`. They're runtime shapes owned by the
hook executor, so they belong in the consumer crate.

`bridge_hook` (and its private `resolve_hook_type` /
`bridge_hook_event` helpers) also moved from
`fabro-types/src/settings/v2/to_runtime.rs` into
`fabro-hooks/src/config.rs`, because the target type is now local
to `fabro-hooks`. `fabro-workflow/src/operations/start.rs` now
imports `bridge_hook` from `fabro_hooks::config::bridge_hook`
instead of the v2 `to_runtime` module.

`fabro-hooks/src/types.rs` re-export of `HookEvent` switches from
the deleted `fabro_types::settings::hook` path to the new
crate-local `crate::config::HookEvent`.

`settings/mod.rs` drops `pub mod {hook, project}` and the
corresponding `pub use` re-exports. Three of the seven legacy
runtime type modules are now gone; four remain (mcp, run, sandbox,
server).

3,758 workspace tests pass. `cargo fmt --check --all` and
`cargo clippy --workspace -- -D warnings` are clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 18:07:04 -04:00
Bryan Helmkamp
eb7f99310c refactor(config): stage 6.4 delete fabro-config re-export shims
fabro-config no longer carries the legacy pass-through shims that
forwarded type re-exports from `fabro_types::settings::{hook,mcp,sandbox,
server,user,run}`. Consumers now import the runtime types directly
from `fabro_types::settings::*`, which is the only definitional
location.

Deleted files:
- `fabro-config/src/hook.rs` (1 LOC glob re-export)
- `fabro-config/src/mcp.rs`   (1 LOC glob re-export)
- `fabro-config/src/sandbox.rs` (~8 LOC re-export list)
- `fabro-config/src/server.rs`  (re-exports + `resolve_storage_dir`;
  the `resolve_storage_dir` helper moved to `fabro_config`'s crate root
  and takes `&SettingsFile` directly)

Shrunk files:
- `fabro-config/src/run.rs` lost the `ArtifactsSettings` /
  `CheckpointSettings` / `GitHubSettings` / `LlmSettings` /
  `MergeStrategy` / `PullRequestSettings` / `SetupSettings` re-export
  block and the unused `resolve_env_refs` helper. What remains is just
  the workflow TOML loader helpers (`parse_run_config`, `load_run_config`,
  `resolve_graph_path`).
- `fabro-config/src/user.rs` lost the `ClientTlsSettings` /
  `ExecSettings` / `OutputFormat` / `PermissionLevel` /
  `ServerSettings` re-export block. The settings-path helpers and
  legacy-config warning logic stay. `fabro-cli/src/user_config.rs`
  now imports `ClientTlsSettings` directly from fabro_types.

Callers updated to use the canonical paths:
- `fabro-agent/src/cli.rs` imports `{OutputFormat, PermissionLevel}`
  from `fabro_types::settings::user`; added `fabro-types` dep.
- `fabro-hooks/src/{config,types}.rs` re-export from
  `fabro_types::settings::hook`.
- `fabro-mcp/src/config.rs` re-exports from `fabro_types::settings::mcp`.
- `fabro-sandbox/src/daytona/mod.rs` re-exports from
  `fabro_types::settings::sandbox`.
- `fabro-server/src/{lib,jwt_auth,tls,serve,demo}.rs` +
  `tests/it/openapi_conformance.rs` import server types from
  `fabro_types::settings::server` and call `fabro_config::resolve_storage_dir`
  from the crate root.
- `fabro-workflow/src/{operations/start,pipeline/types,pipeline/pull_request}.rs`
  import sandbox / pull_request types from `fabro_types::settings::*`.

Build, clippy, fmt, and 3756 / 3756 tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 16:30:38 -04:00
Bryan Helmkamp
884fa329ba feat(settings): stage 6.2 delete bridge_to_old seam
bridge.rs (818 LOC) is gone. Production consumers no longer produce a
full legacy `Settings` from v2 state; every read path walks the v2 tree
directly or uses one of the narrow v2->runtime helpers in the new
`settings::v2::to_runtime` module.

Core moves:

fabro-types
- Delete `settings::v2::bridge::bridge_to_old` and the whole bridge.rs
  file.
- Relocate the narrow v2->runtime helpers (`bridge_sandbox`,
  `bridge_mcp_entry`, `bridge_mcps`, `bridge_hook`, `bridge_worktree_mode`,
  `bridge_merge_strategy`, `bridge_pull_request`, `bridge_run_artifacts`)
  into a new `settings::v2::to_runtime` module. Each helper takes a
  single v2 subtree and produces the corresponding runtime shape;
  nothing assembles a full legacy `Settings` anymore.
- `settings/mod.rs` doc comment rewritten to describe `Settings` as a
  runtime shape, not a resolved parse target. Stage 6.3 deletes it.

fabro-config
- `ConfigLayer::resolve` is gone along with the `TryFrom<ConfigLayer>
  for Settings` impls. Consumers call `.into()` for a `SettingsFile`,
  or `.as_v2()` to borrow one.
- `fabro_config::server::resolve_storage_dir` now takes `&SettingsFile`.

fabro-server
- `api_server_settings` emits the v2 `SettingsFile` JSON shape
  directly instead of bridging to the legacy flat DTO. Stage 6.6
  replaces the shape again with an explicit allow-list DTO.
- `serve.rs`: `load_settings` returns `SettingsFile`;
  `apply_serve_overrides` / `apply_runtime_settings` mutate v2
  subtrees directly; `build_artifact_object_store` walks
  `server.artifacts`; `build_legacy_api_settings` projects the v2
  auth/listen/api subtrees down to the legacy `ApiSettings` shape for
  the (still-legacy) auth resolver.
- `diagnostics::check_crypto` walks `server.auth.api.{jwt,mtls}` and
  `server.listen.tls` directly.
- `web_auth.rs` oauth / register / setup-status / auth-me flows all
  read `server.web`, `server.integrations.github`, and
  `server.auth.web` directly via the v2 accessors. `merge_settings_keys`
  now writes v2 TOML (with `[server.web]`, `[server.integrations.github]`,
  etc.) instead of the legacy v1 top-level keys, and the register
  handler re-parses the freshly-written file back into the in-memory
  `SettingsFile` state.

fabro-cli
- `CommandContext::machine_settings` returns `&SettingsFile`.
- `user_config::load_settings` and friends return `SettingsFile`.
- `user_config::resolve_server_target` / `exec_server_target` /
  `configured_server_target` walk `cli.target.{http,unix}` directly.
  Tests rewritten against v2 TOML fixtures.
- `main.rs` logging init reads `cli.logging.level` / `server.logging.level`
  via v2 accessors.
- `commands/exec.rs` reads `cli.exec.{model,agent}` and builds mcps
  from `cli.exec.agent.mcps` (falling back to `run.agent.mcps`) via
  `to_runtime::bridge_mcp_entry`.
- `commands/pr/mod.rs` calls `github_app_id_str()`.
- `commands/run/create.rs` drops the legacy `.resolve()` call and uses
  `Into::<SettingsFile>::into(...)`.
- `commands/config/mod.rs::legacy_settings_to_v2` is now a real
  reverse-mapping helper that covers `storage`, `scheduler`,
  `integrations.{github,slack}`, `run.model`, `run.inputs`, and
  `cli.output.verbosity`. Stage 6.6 deletes it when the API client
  returns v2 natively.
- `tests/it/cmd/config.rs` tests now walk the v2 tree directly (via
  `cfg.run_model_name_str()`, `cfg.run_inputs()`, `cfg.run_sandbox()`,
  `cfg.run_hooks()`, `cfg.run_agent_mcps()`, `cfg.run_prepare_commands()`,
  `cfg.server_storage_root_str()`, etc.). The `bridge_to_old` test
  helper is gone.
- `tests/it/api/settings.rs` asserts against the v2 JSON shape.

Build, test, and quality gates all green:
- `cargo build --workspace --tests`
- `cargo clippy --workspace -- -D warnings`
- `cargo fmt --check --all`
- `cargo nextest run --workspace`: 3758 / 3758 passed, 182 skipped.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 16:12:59 -04:00
Bryan Helmkamp
41ab919959 feat(settings): stage 6.1 consumer migration builds workspace-wide
Extends the stage 6.1 WIP into a compiling state across the workspace.
Most crates and their unit/integration tests now read run.* / cli.* /
server.* v2 layers directly or through targeted bridge helpers.

Key moves in this commit:

fabro-server
- AppState.settings: Arc<RwLock<SettingsFile>> -- all helpers,
  create_app_state_with_* factories, and tests updated.
- api_server_settings bridges SettingsFile -> legacy Settings via the
  transitional bridge so /api/v1/settings still emits the legacy DTO
  shape until Stage 6.6 replaces it with an allow-list DTO.
- get_system_info, get_system_df, get_github_repo, webhook startup, and
  other read sites use the v2 accessors (github_app_id_str,
  server_web, run_sandbox, run_model_*).
- web_auth.rs wraps each oauth / register / setup-status handler in a
  local `bridged` helper that produces a legacy Settings from the v2
  state, so the complex oauth mutation flow keeps working until its
  Stage 6.6 rewrite.
- diagnostics::check_github_app reads via github_*_str accessors;
  check_crypto bridges to the legacy shape inline.
- serve.rs: load_settings returns SettingsFile; apply_serve_overrides /
  apply_runtime_settings mutate v2 subtrees directly; the config poll
  loop and TLS/webhook startup use bridged() for legacy-shape reads.
- Tests in tests/it/{helpers,api/*,scenario/*} rewritten to construct
  SettingsFile via ConfigLayer::parse or v2 struct literals.

fabro-workflow
- Every test fixture in pipeline/{finalize,initialize,pull_request,retro,
  execute,persist}, operations/{create,rebuild_meta,start}, run_lookup,
  runtime_store, handler/manager_loop, and tests/it/{integration,
  daytona_integration}.rs now uses SettingsFile.
- start.rs hooks into the bridge helpers directly via use-imports.
- run_graph / run_graph_from_checkpoint / initialize / finalize /
  pull_request calls are Box::pin'd to stay under clippy's large-future
  threshold after the v2 tree brought RunOptions size up.
- resolve_run_settings writes resolved model/provider back into
  run.model as InterpStrings; tests assert via run_model_*_str().
- preprocess_and_validate pulls vars from run_inputs_as_strings().

fabro-cli
- manifest_builder uses ConfigLayer.combine(...).into() to get a v2
  SettingsFile for the manifest goal resolution path; file-based
  goal_file handling is deferred to 6.6 when the manifest schema catches
  up.
- runner::maybe_build_github_app_credentials and
  tests/it/cmd/{create,runner}.rs read from v2 accessors.
- commands/config/mod.rs::merged_config returns SettingsFile; the
  server-side retrieve_server_settings is bridged via a stopgap
  legacy_settings_to_v2 shim that Stage 6.6 replaces.
- commands/store/dump.rs sample_run_record constructs SettingsFile.

fabro-store, fabro-checkpoint
- Test fixtures constructing RunRecord values updated to SettingsFile.
- fabro-checkpoint/src/author.rs stays (v2 From impl landed in a
  previous additive commit).

fabro-config
- effective_settings.rs rewrite compiles and passes its unit tests.
- project::resolve_working_directory takes &SettingsFile.

Build status: `cargo build --workspace --tests`, `cargo clippy
--workspace -- -D warnings`, and `cargo fmt --check --all` all pass.
`cargo nextest run --workspace` passes 3,749 of 3,764 tests; the 15
remaining failures are fabro-cli integration tests whose snapshot +
TOML fixture shapes still need manual updates:

- cmd::config::* (seven tests): fixture TOML files still use v1
  top-level keys and the snapshot outputs expect the legacy flat JSON
  shape.
- cmd::inspect::* (four tests): run-record JSON snapshots embed the
  flat Settings shape.
- cmd::run::dry_run_persists_event_history_in_store and
  json_run_implies_auto_approve_for_human_gates: check `settings.dry_run
  == Some(true)` directly on the v2 file; should assert
  dry_run_enabled() instead.
- cmd::attach::attach_json_errors_without_prompting_for_human_input:
  unrelated insta snapshot drift caused by the new SettingsFile JSON
  shape leaking into an events-log snapshot.

Follow-up work for this stage also includes:
- Rewriting web_auth.rs register flow to emit v2 TOML directly and to
  re-parse the written file back into state.settings so in-memory
  state doesn't lag the on-disk file.
- Removing the legacy_settings_to_v2 shim in fabro-cli/config once
  the server-side settings endpoint returns v2 shapes (Stage 6.6).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 15:25:59 -04:00
Bryan Helmkamp
d0802fbfb1 wip(settings): stage 6.1 consumer migration (broken build)
Partial Stage 6.1 migration of consumers off the legacy flat Settings
shape to v2 SettingsFile. Commits the in-flight work so subsequent
sessions can resume from here. Workspace currently does NOT build --
fabro-server still has ~60 consumer sites that reference state.settings
as legacy Settings, and fabro-cli is entirely untouched.

Landed in this commit:

fabro-types
- RunRecord.settings: Settings -> SettingsFile
- RunCreatedProps.settings: Settings -> SettingsFile

fabro-config
- effective_settings: full rewrite. resolve_settings now returns
  SettingsFile; apply_server_defaults / apply_local_daemon_overrides
  are v2-native and use the v2 merge matrix for server-owned domains.
- project::resolve_working_directory takes &SettingsFile and reads
  run.working_dir as an InterpString.

fabro-workflow
- start.rs, create.rs, source.rs, validate.rs, run_options.rs, git.rs,
  initialize.rs, manager_loop.rs all migrated to &SettingsFile reads.
- resolve_sandbox_provider / resolve_worktree_mode / resolve_daytona_config
  / resolve_fallback_chain walk v2 trees using the bridge helper fns.
- LifecycleOptions built from run_prepare_commands() / run_prepare_timeout_ms().
- Hooks built via bridge_hook on v2 HookEntry.
- MCPs built via bridge_mcp_entry on v2 McpEntryLayer.
- resolve_run_settings writes resolved model/provider back into
  run.model (InterpString), not the flat llm struct.
- preprocess_and_validate pulls var expansion from run_inputs_as_strings.

fabro-server/run_manifest.rs
- PreparedManifest.settings -> SettingsFile.
- prepare_manifest_with_mode takes &SettingsFile.
- build_preflight_report / run_llm_check / resolve_model_provider /
  run_github_token_check / resolve_sandbox_provider / resolve_daytona_config
  all migrated.
- Tests rewritten to use v2 fixtures via ConfigLayer::parse.

fabro-server/server.rs
- AppState.settings type changed to Arc<RwLock<SettingsFile>>.
- github_app_credentials call site uses settings.github_app_id_str()
  accessor instead of the flat app_id().

Known remaining errors:
- fabro-server/server.rs: ~60 state.settings.read() sites still
  reference legacy Settings fields (llm, sandbox, setup, git, etc.).
- fabro-server/web_auth.rs: heavy git settings usage, tests.
- fabro-server/serve.rs: state mutation of flat llm/sandbox fields.
- fabro-server/diagnostics.rs: app_id / api auth strategies.
- fabro-cli: manifest_builder, commands, tests all untouched.
- Test fixtures across the workspace still construct Settings literals.
- insta snapshots will need bulk-accept after the runtime shape stabilizes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 12:52:03 -04:00
Bryan Helmkamp
143ed5b417 fix(lint): clean up fabro-config test clippy warnings
- Drop the fabro_types::Combine re-export from fabro-config/lib.rs
  (unused externally after Stage 3 replaced the legacy Combine-based
  merge with the v2 merge matrix)
- Replace absolute `fabro_types::settings::v2::InterpString` paths in
  fabro-config/src/config.rs and merge.rs test blocks with a scoped
  `use` import, satisfying clippy::absolute_paths
- fabro-config/src/merge.rs tests: use `!contains_key`, drop redundant
  closures around InterpString::as_source, prefer indexing over
  get().unwrap() on the notifications HashMap
- fabro-config/src/project.rs tests: switch the run.execution.retros
  fixture off raw string literal hashes (only simple content inside)
  and use ToString::to_string in the error-chain join expression
2026-04-09 11:44:56 -04:00
Bryan Helmkamp
d8389025a8 docs(config): point new code at ConfigLayer::as_v2 rather than the bridge 2026-04-09 11:38:00 -04:00
Bryan Helmkamp
b0fff693e6 refactor(config): delete unused legacy shim modules, document transitional seam
Stage 6 initial cleanup. Removes two fabro-config shim modules that no
longer hold any code and adds a module-level comment to
fabro-types/src/settings/mod.rs documenting the transitional seam
between the flat legacy Settings shape and the authoritative v2
namespaced schema in fabro_types::settings::v2.

Deleted:
- fabro-config/src/combine.rs: was a one-line re-export of
  fabro_types::combine::Combine; nothing imports it anymore
- fabro-config/src/settings.rs: was reduced to a header comment
  after Stage 3 replaced TryFrom<ConfigLayer> for Settings with
  ConfigLayer::resolve via the v2 bridge

Stage 6 full deletion (legacy flat Settings type, the bridge, the
old settings/{hook,mcp,project,run,sandbox,server,user}.rs modules,
plus the Combine trait derive) is scheduled for a follow-up PR that
migrates every consumer call site from the flat settings.llm /
.vars / .sandbox / .setup / .hooks / .mcp_servers / .goal / .work_dir
/ .github / .git / .pull_request / .checkpoint / .artifacts fields to
the v2 SettingsFile tree. That touches ~128 call sites across ~15
files and is a mechanical but large follow-up; the current bridge is
the safe intermediate state.
2026-04-09 11:35:44 -04:00
Bryan Helmkamp
3eabc013d1 test(migration): land final Stage 4 fixes — 100% workspace tests green
Close out consumer migration with targeted behavior fixes and the
remaining integration-test fixture rewrites. The full workspace
nextest run now reports 3,760 passed / 0 failed / 182 skipped.

Runtime fixes:
- effective_settings::apply_server_defaults now propagates the full
  server-side Settings shape (llm, sandbox, setup, checkpoint,
  pull_request, artifacts, hooks, mcp_servers, github, slack, fabro)
  into the resolved CLI settings, matching the pre-Stage-3 'merge
  everything server' behavior for RemoteServer/LocalDaemon modes
- fabro-cli commands/run/overrides: route --verbose through
  cli.output.verbosity = verbose instead of a run.metadata stash,
  so it resolves to settings.verbose via the bridge
- fabro-server run_manifest manifest_args_layer: same — emit a
  CliLayer with cli.output.verbosity rather than stuffing the flag
  into run.metadata
- fabro-test settings_storage_dir: detect the managed marker and
  return None instead of parsing the injected server.storage.root,
  so isolated_server correctly spins up a new storage dir
- fabro-server run_manifest_local_daemon test now passes with full
  server-side settings snapshot propagation

Test fixture + assertion updates:
- cmd::config::settings_local_explicit_workflow_path_uses_workflow_project_layers:
  assertion updated for v2 R30 whole-list replacement of
  run.prepare.steps across layers (only workflow-setup survives)
- cmd::config::create_explicit_workflow_path_uses_project_config_relative_to_workflow:
  same correction for the persisted run.settings.setup.commands
- cmd::attach::attach_json_errors_without_prompting_for_human_input
  and cmd::run::json_run_implies_auto_approve_for_human_gates: strip
  the bridge-emitted settings.server and settings.version fields from
  the JSON snapshot so the randomised unix-socket path does not flap
  the insta snapshot
- cmd::server_start::concurrent_autostart_converges_on_one_shared_daemon_and_cleans_up:
  rewrite the injected settings.toml to v2 shape with
  [server.storage] root and [cli.target] type = unix path
- scenario::smoke::attach_smoke_covers_arg_validation_and_remote_server_behaviors:
  two [server] target fixtures rewritten to [cli.target]
  type = http url

Accepted insta snapshots for attach and run JSON outputs. Workspace
build + clippy both clean under -D warnings.
2026-04-09 11:07:18 -04:00
Bryan Helmkamp
16204acf0d fix(effective_settings): keep cli/server stanzas from user settings.toml
LocalDaemon and RemoteServer modes were stripping owner-specific
domains (cli, server) from the user layer as well as from fabro.toml
and workflow.toml. Per the plan's trust boundary rule, owner-specific
domains should only be consumed from ~/.fabro/settings.toml, so the
user layer is the one place they MUST survive. Strip only the
workflow and project layers.
2026-04-09 10:19:18 -04:00
Bryan Helmkamp
d951d0bf82 fix(lint): clippy cleanup for Stage 3/4 consumer migration
- effective_settings server_defaults_layer: drop Result wrapper since
  the body never fails after the v2 switch
- merge.rs: allow needless_pass_by_value module-wide since every
  merge helper consumes both sides by design
- fabro-cli overrides.rs: replace &Option<String> sigs with
  Option<&str>, collapse Default-plus-assignment into struct literal
  (avoid clippy::field_reassign_with_default), and box the metadata
  HashMap inline
- fabro-cli manifest_builder.rs: pull DaytonaDockerfileLayer into
  scope so the pattern match stays absolute-path-clean
- fabro-cli main.rs + commands/config/mod.rs: Box::pin the settings
  subcommand future so clippy::large_futures stays happy
2026-04-09 10:16:47 -04:00
Bryan Helmkamp
eb077ed3a2 feat(config): switch parser and layering to v2 schema
Stage 3 of the settings TOML redesign. Switches the core parse/merge/
resolve path to the v2 namespaced schema while keeping the legacy flat
Settings shape accessible via the bridge for not-yet-migrated consumers.

Parser and layering:
- ConfigLayer is now a newtype around v2 SettingsFile. Loading via
  ConfigLayer::parse/load/settings/for_workflow/project now hard-fails
  on legacy top-level keys (version, llm, vars, sandbox, etc.) with
  targeted rename hints emitted by fabro_types::settings::v2::tree
- new fabro_config::merge module encodes the merge matrix directly:
  replace-by-default maps, sticky merge for run.sandbox.env and
  provider-native labels, splice-aware string arrays for
  run.model.fallbacks and notification route events, whole-list
  replacement for run.prepare.steps, field-merge keyed objects for
  notifications/MCPs/web-auth providers, and ordered hook id-aware
  replacement
- ConfigLayer::resolve delegates to fabro_types::settings::v2::bridge
  so consumers keep reading through the legacy Settings shape until
  Stage 4 migrates them off it
- effective_settings::resolve_settings now treats project/workflow/
  run/features as shared layered domains and strips cli/server from
  non-local layers before merging, fulfilling the owner-first trust
  boundary rule

Consumer migration (Stage 4 preview, kept to the files that block
the workspace build):
- fabro-server run_manifest builds v2 RunLayer from ManifestArgs and
  resolves manifest dockerfile references through the v2 sandbox
  daytona snapshot tree
- fabro-cli manifest_builder consults run.goal via v2; user_config
  writes the v2 server.storage.root field under the CLI storage-dir
  override; run/overrides constructs a v2 RunLayer from RunArgs
- fabro-cli scaffolds (repo init, workflow create) emit _version = 1
  with project.directory/workflow.graph/run.sandbox etc.

fabro-config / fabro-types legacy parse-time types (ProjectConfig,
LlmConfig, SandboxConfig, PullRequestConfig, ExecConfig, SettingsFile
try_into, etc.) are deleted from the parse path; the resolved type
re-exports (LlmSettings, SandboxSettings, etc.) remain as shims so
unmigrated consumers keep compiling.

fabro-test helper: settings.toml fixtures now use _version = 1 plus
[server.storage] root and [cli.target] type = "unix" path. Legacy
flat storage_dir/server.target handling removed from the sync path.

Known Stage 4/5 follow-ups:
- fabro-cli integration test fixtures still use legacy-shape TOML
  (version = 1, [llm], [sandbox], [vars], [exec], [fabro], etc.);
  tests currently fail to parse against the v2 schema as intended.
  Migrating them is the bulk of Stage 4 and lands in subsequent
  commits.
- OpenAPI ServerSettings schema, generated clients, apps/fabro-web
  workflowData fallback, and docs/reference examples are unchanged
  and land in Stage 5.
2026-04-09 09:49:23 -04:00
Bryan Helmkamp
fa3507d8c7 feat(artifacts): remove scratch artifact cache staging
Stage captured artifacts in per-attempt tempdirs and persist them through an
explicit artifact sink instead of writing into run scratch cache.

Server-managed and test-owned runs now write directly to ArtifactStore, while
CLI worker runs keep the staged upload path. The local run summary now prints
durable artifact identifiers and copy hints rather than scratch-cache paths,
and the run-directory docs and integration coverage were updated to match.
2026-04-08 17:42:55 -04:00
Bryan Helmkamp
30e63b270f Merge remote-tracking branch 'origin/main' 2026-04-08 16:30:26 -04:00
Bryan Helmkamp
e098d8d4c0 refactor: remove negative scratch assertions and rename child dir to stages/
Remove test assertions that verified legacy files (final.patch,
workflow_bundle.json, manifest.json, cache/artifacts/values/) do not
exist in scratch directories — these are a test smell since the code
that wrote them is long gone.

Also rename child workflow scratch path from nodes/{id}_{visit}/child
to stages/{id}@{visit}/child to align with stage_id convention.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 16:23:02 -04:00
Bryan Helmkamp
45675fc42d feat(server): make web ui optional
Add a server-side web.enabled toggle and CLI overrides so Fabro can run
with API and health only while disabling the embedded SPA, browser auth
routes, and web-only helper endpoints.
2026-04-08 16:22:41 -04:00
Bryan Helmkamp
50070ad75a refactor(scratch): remove store-backed diff and blob cache writes
Stop writing scratch final.patch files now that diffs are projected from
run state, and remove the unused cache/artifacts/values plumbing while
keeping runtime/blobs materialization intact.

Update tests and run-directory docs to match the current scratch contract.
2026-04-07 22:31:59 -04:00
Bryan Helmkamp
87bc42be70 refactor(interview): move run answers onto control channels
Persist pending interviews in run state, deliver accepted answers to workers
through the server-owned control path, and remove the old scratch-file and
WebInterviewer transports.

This also moves Slack onto the canonical server answer flow, adds richer
question metadata to the API and run events, and covers the subprocess
question lifecycle with end-to-end tests.
2026-04-07 19:23:36 -04:00
Bryan Helmkamp
494a7fe1cc feat(artifacts): finish object-backed artifact uploads
Add scoped worker upload tokens and HTTP artifact upload clients.

Support manifest-first multipart stage artifact uploads with validation and checksums.

Gate artifact reads by run capability while preserving legacy scratch fallback.
2026-04-07 16:56:37 -04:00
Bryan Helmkamp
5abf775cf5 fix(logging): use stable server log paths 2026-04-06 16:18:13 -04:00
Bryan Helmkamp
c75349aa12 Merge remote-tracking branch 'origin/main' 2026-04-06 15:25:43 -04:00
Bryan Helmkamp
83eabb5afe fix(storage): use Storage/RunScratch accessors instead of raw path joins
Four callsites were bypassing existing Storage/RunScratch methods:

1. install.rs — .join("secrets.json") → Storage::secrets_path()
2. initialize.rs — .join("worktree") → RunScratch::worktree_dir()
3. git.rs — .join("final.patch") → RunScratch::final_patch()
4. create.rs — duplicated date-format logic → RunScratch::for_run()

Adds RunScratch::for_run(scratch_dir, run_id) to centralize the
date-prefixed directory name, used by both Storage::run_scratch()
and make_run_dir().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 15:12:19 -04:00
Bryan Helmkamp
72c5c30c11 refactor(home): remove dead settings path fallbacks
default_settings_path() and active_settings_path() always return a
value (Home::from_env() never fails), so unwrap_or_else fallbacks to
".fabro/settings.toml" were dead code. Change both functions to return
PathBuf instead of Option<PathBuf> and remove the unreachable branches
in server_client, serve, and user config.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 13:59:17 -04:00
Bryan Helmkamp
fa6a41f995 fix(home): migrate all runtime consumers to use Home
Replaces every dirs::home_dir().join(".fabro") in production code with
Home::from_env() accessors, so FABRO_HOME is respected everywhere:

- fabro-config: project workflows dir, legacy .env path
- fabro-cli: logging dir, install root + cert defaults, upgrade check
  state, workflow list, doctor/install/login legacy_env callers
- fabro-telemetry: tmp dir for spawn, anonymous ID file
- fabro-workflow: file resolver fallback in source resolution
- fabro-agent: skill discovery (session + default_skill_dirs API)

Remaining dirs::home_dir() calls are legitimate: tilde expansion,
display path shortening, non-.fabro paths (e.g. ~/.claude/skills).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 13:36:43 -04:00
Bryan Helmkamp
e862581fc8 refactor(settings): make settings command server-backed
Default `fabro settings` now resolves effective runtime settings against the
selected server, while `--local` preserves local-only inspection. This also
extracts shared settings resolution logic so CLI output, manifest preparation,
and the `/api/v1/settings` contract stay aligned.
2026-04-06 13:34:28 -04:00
Bryan Helmkamp
5fe4a2e7d3 refactor(home): move Home to fabro-util so all crates can share it
Home lived in fabro-config, which meant fabro-types (a dependency of
fabro-config) could not use it — forcing Settings::storage_dir() to
duplicate the FABRO_HOME / dirs::home_dir() fallback logic. Moving Home
to the leaf crate fabro-util breaks this layering constraint and lets
Settings::storage_dir() delegate to Home::from_env().storage_dir().

Also adds stable accessors: storage_dir, socket_path, workflows_dir,
logs_dir, tmp_dir. fabro-config re-exports Home for API compatibility.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 13:08:35 -04:00
Bryan Helmkamp
23231eb1ae refactor(cli): separate local socket and storage defaults
Keep local server targeting based on explicit server targets instead of
implicitly deriving a socket from storage_dir. This makes ~/.fabro/fabro.sock
the default local socket again, keeps storage under ~/.fabro/storage, threads
FABRO_CONFIG through server autostart paths, and updates the CLI test harness
for the new split.
2026-04-06 11:57:14 -04:00
Bryan Helmkamp
fe8fa627a0 fix(storage): satisfy clippy in home and artifact paths 2026-04-06 10:31:08 -04:00
Bryan Helmkamp
bd8f0fe5ee refactor(storage): unify scratch paths and key schema
- centralize FABRO_HOME and storage path resolution in fabro-config
- rename store types, extract ArtifactStore, and simplify run key layout
- switch run scratch to scratch/, remove RuntimeState, and refresh docs/clients
2026-04-06 10:13:37 -04:00
Bryan Helmkamp
6d94f93e44 refactor(config): unify machine config as settings.toml
Consolidate CLI and server machine defaults under settings.toml,
including loader renames, writer preservation fixes, same-machine
manifest handling, and docs/test updates for the new config model.
2026-04-05 23:55:28 -04:00