Commit graph

155 commits

Author SHA1 Message Date
Bryan Helmkamp
9a43606759
refactor(settings): rename settings layer and move parsing 2026-04-10 08:10:06 -04:00
Bryan Helmkamp
fab67ad31f
refactor(settings): remove sparse settings compatibility layer 2026-04-10 07:58:46 -04:00
Bryan Helmkamp
7796c4d4e1
refactor(settings): resolve feature flags 2026-04-10 07:16:10 -04:00
Bryan Helmkamp
e1ea66a833
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
856c0f1c68
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
09b616cb12
fix(server): convert auth resolver panics to fail-closed errors
Completes the R52/R53 fail-closed posture from d4fb73d61. The jwt and
mtls strategy branches were still using panic!/expect/assert! when
their required material was missing or malformed, which would crash
the server binary instead of returning a clean startup error.

- decode_pem_env: return anyhow::Result<String> instead of panicking
  on invalid base64 or invalid UTF-8.
- resolve_auth_mode_with_lookup: convert the missing-FABRO_JWT_PUBLIC_KEY,
  invalid-PEM, and missing-[server.listen.tls]-for-mtls cases from
  panics to anyhow::Err returns prefixed with "Fabro server refuses
  to start".
- Update the resolve_auth_mode doc to drop the "Panics if..." caveat.
- Add three fail-closed tests covering each new error path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:53:52 -04:00
Bryan Helmkamp
f79ca80591
Merge origin/main into main
Brings in the events schema v2 work (RunEvent envelope fields, ActorRef,
parallel branch ids, flattened EventEnvelope wire JSON) on top of the
local Stage 6 settings TOML redesign.

Conflict resolutions:
- fabro-types/src/lib.rs: keep new ParallelBranchId re-export from
  origin; drop the legacy Settings/ArtifactStorage* re-exports (the
  flat Settings struct was deleted in Stage 6.3b).
- fabro-server/src/server.rs: keep new ActorRef import from origin;
  drop the unused legacy Settings import that came along with it.
- fabro-api-client/src/models/web-settings.ts: keep our deletion. The
  remote modification was an incidental TS-client regeneration that
  Stage 6.6 already invalidated by collapsing settings DTOs to a
  freeform v2 shape.
- fabro-workflow/src/event.rs: rewrite the run_created actor test to
  use SettingsFile::default() instead of the deleted Settings type.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:11:55 -04:00
Bryan Helmkamp
2d4c0945bb
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
ce1696706c
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
fac0b10244
feat(server): preserve comments in setup_register TOML edits
`setup_register` in `web_auth.rs` used to round-trip the user's
settings file through `toml::Value` + `toml::to_string_pretty`, which
strips every comment, blank line, and explicit key ordering on the
way out. A user who'd hand-commented their `~/.fabro/settings.toml`
would see all of that lost on the next GitHub App registration.

Switches the edit path to `toml_edit::DocumentMut`, which preserves
prefix decoration (comments, blank lines) on every key. Adds
`toml_edit = "0.22"` as a workspace dependency (already pulled in
transitively via `toml 0.8`) and declares it in `fabro-server`.

Implementation notes:

- New `ensure_nested_table(doc, &["server", "web"])` walks a dotted
  path and `or_insert`s missing intermediate tables without touching
  existing ones.
- New `set_preserving_decor(table, key, value)` replaces an entry's
  value while copying the old key's `leaf_decor` forward. Without
  that workaround, `toml_edit::Table::insert` drops the prefix
  decoration of the replaced key -- which would strip a top-of-file
  comment attached to `_version = 1` or any other value we update.
- `_version` is only inserted when missing; it's always `1` today, so
  rewriting it every time is unnecessary and would trample its decor.
- `merge_settings_keys` now takes `&mut toml_edit::DocumentMut`
  instead of `&mut toml::Value`. The flow in `setup_register` parses
  the file on disk into a `DocumentMut`, applies the merge, and
  writes `doc.to_string()` back.

Adds a new test
`merge_settings_keys_preserves_comments_and_unrelated_keys` that
round-trips a fixture file containing:

- A top-of-file comment attached to `_version`
- A comment above `[server.storage]`
- A comment above a pre-existing `[server.integrations.slack]` table
- Unrelated keys in `[server.storage]`, `[server.integrations.slack]`,
  and `[run.model]`

and asserts that every comment and every unrelated key survives the
merge, that the new GitHub App keys are present, and that the final
output still parses as a valid v2 `SettingsFile` via
`fabro_config::ConfigLayer::parse`.

Also strengthens the existing
`merge_settings_keys_writes_v2_server_integrations_github` test with
a round-trip parse of the emitted TOML through `ConfigLayer::parse`
to ensure the output is real v2 config, not just a JSON-shaped blob.

3,765 workspace tests pass (+1 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:18:10 -04:00
Bryan Helmkamp
d4fb73d614
feat(server): fail-closed auth posture per R52/R53
`resolve_auth_mode_with_lookup` now returns `anyhow::Result<AuthMode>`
and refuses to return success when `server.auth` resolves to zero
enabled strategies. Startup propagates the error via `?` and aborts
with a descriptive message pointing at the three configuration
escape hatches.

Previously the resolver logged a warning and returned
`AuthMode::Strategies(empty)`, which meant an unconfigured server
would start and then reject every request — accidental
misconfigurations produced a silently-broken process rather than a
clean startup failure. The new behavior matches the implementation
plan's explicit guidance: "if `server.auth` is absent or resolves to
no enabled API or web auth configuration, normal server startup
must refuse to start. Demo and test helpers may continue to inject
explicit insecure settings, but insecure startup must be opt-in
rather than accidental."

The single opt-in path is the `FABRO_LOCAL_NO_AUTH` env var set to
the literal string `"1"`, now hoisted into a module-level
`FABRO_LOCAL_NO_AUTH_ENV` constant. `fabro server start --bind
<unix-socket>` already sets this implicitly in `start.rs:232-234`,
so local daemon usage is unchanged. TCP binds now require either
real auth config or an explicit `FABRO_LOCAL_NO_AUTH=1` — arguably
a security improvement for TCP.

Detailed error message lists the three configuration options:

    Configure at least one of the following in `[server.auth]`:
      - `[server.auth.api.jwt]` (requires `FABRO_JWT_PUBLIC_KEY` env)
      - `[server.auth.api.mtls]` (requires `[server.listen.tls]` ...)
      - `SESSION_SECRET` env (enables cookie-based web auth)

Adds six new unit tests covering the full decision matrix:

- `fail_closed_when_server_auth_absent`
- `fail_closed_when_all_strategies_disabled`
- `opt_in_insecure_startup_via_env`
- `insecure_startup_flag_any_other_value_still_fails_closed`
- `cookie_strategy_alone_unlocks_startup`
- `mtls_strategy_resolves_when_enabled_with_listen_tls`

Also adds `#[derive(Debug)]` to `AuthMode` and `AuthStrategy` so the
tests can `expect_err()` on the resolver result.

Two existing `fabro-cli` integration tests for TCP bind resolution
(`start_with_tcp_host_only_bind_resolves_to_host_and_port` and
`start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unavailable`)
now set `FABRO_LOCAL_NO_AUTH=1` in the test environment. They were
exercising bind-address resolution, not auth, so opting into
insecure startup explicitly keeps their focus narrow.

3,764 workspace tests pass (was 3,758, +6 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:03:51 -04:00
Bryan Helmkamp
747d9e8fcb
refactor(server): move TlsSettings into its own tls_config module
`TlsSettings` and its `from_settings(&SettingsFile)` constructor
lived in `jwt_auth.rs` as a historical artifact from the Stage 6.6g
rewrite — the auth resolver only needs to know *whether* TLS is
present (for mTLS support), not the contents of the triple. The
type is really a listen-side concern that belongs next to the
rustls builder.

Moves the type into a new `fabro-server/src/tls_config.rs` module
(35 LOC). Updates three importers:

- `jwt_auth.rs` — imports `TlsSettings` from `crate::tls_config`;
  drops the `std::path::PathBuf` / `InterpString` / `ServerListenLayer`
  / `serde::Deserialize` imports that are no longer used after the
  type moved.
- `serve.rs` — splits the multi-item `use crate::jwt_auth::{...}`
  line so `TlsSettings` comes from `crate::tls_config`.
- `tls.rs` — same split.
- `tests/it/api/mtls.rs` — same split.

Pure relocation; no behavioral change. 156 fabro-server 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:54:04 -04:00
Bryan Helmkamp
c625747e0c
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
d82d167f07
refactor(settings): stage 6.6g rewrite auth resolver for v2
Replaces the `build_legacy_api_settings` + `resolve_auth_mode_with_lookup(&ApiSettings, &[String], lookup)`
shim path with a direct `resolve_auth_mode_with_lookup(&SettingsFile, lookup)`
that walks the v2 `server.auth.api.{jwt,mtls}` and
`server.auth.web.allowed_usernames` subtrees directly:

- Each strategy subtree is considered enabled when present unless
  `enabled = false` is explicit (R52).
- `allowed_usernames` is read from `server.auth.web.allowed_usernames`
  instead of a separate caller-supplied `&[String]` slice.
- The FABRO_LOCAL_NO_AUTH escape hatch and
  "no strategies configured; rejecting everything" warnings are
  preserved.

Deletes the `ApiAuthStrategy` and `ApiSettings` transitional shim
types from `fabro-server/src/jwt_auth.rs`. `TlsSettings` survives
(it's the resolved `(cert, key, ca)` triple that `tls.rs`'s rustls
builder still consumes), with a new
`TlsSettings::from_settings(&SettingsFile)` constructor that
projects `server.listen.tls` into the runtime shape.

`serve.rs` drops its `build_legacy_api_settings` helper entirely
(~60 LOC). The serve bootstrap now calls
`resolve_auth_mode_with_lookup(&cfg_file, ...)` directly and uses
`TlsSettings::from_settings(&cfg_file)` for the TCP-vs-Unix branch.

The `build_legacy_api_settings` TODO-2 from handoff-2 is resolved.
TlsSettings uses `is_some_and` instead of `map_or(false, ...)` to
satisfy the clippy `unnecessary_map_or` lint.

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:38:22 -04:00
Bryan Helmkamp
15b799fb3f
refactor(settings): stage 6.3b + 6.5b finish — delete last legacy server types and flatten v2/
**6.3b finishing touch:** relocates the last three transitional server
runtime types (`ApiAuthStrategy`, `TlsSettings`, `ApiSettings`) out of
`fabro-types` into `fabro-server/src/jwt_auth.rs` — the only crate
that consumes them. `serve.rs`, `tls.rs`, and the mTLS integration
test now import from `crate::jwt_auth` / `fabro_server::jwt_auth`
instead of `fabro_types::settings::server`.

`lib/crates/fabro-types/src/settings/server.rs` (the legacy one) and
the `pub mod server_config { pub use fabro_types::settings::server::*; }`
block in `fabro-server/src/lib.rs` are both deleted. The legacy
runtime type module tree under `fabro-types/src/settings/{hook,
mcp, project, run, sandbox, server, user}.rs` is now fully gone —
nothing left to promote.

**6.5b flatten:** `git mv` the fourteen v2 modules up one directory:

- `settings/v2/accessors.rs` → `settings/accessors.rs`
- `settings/v2/cli.rs` → `settings/cli.rs`
- `settings/v2/duration.rs` → `settings/duration.rs`
- `settings/v2/features.rs` → `settings/features.rs`
- `settings/v2/interp.rs` → `settings/interp.rs`
- `settings/v2/model_ref.rs` → `settings/model_ref.rs`
- `settings/v2/project.rs` → `settings/project.rs`
- `settings/v2/run.rs` → `settings/run.rs`
- `settings/v2/server.rs` → `settings/server.rs` (name no longer
  collides with the deleted legacy `server.rs`)
- `settings/v2/size.rs` → `settings/size.rs`
- `settings/v2/splice_array.rs` → `settings/splice_array.rs`
- `settings/v2/tree.rs` → `settings/tree.rs`
- `settings/v2/version.rs` → `settings/version.rs`
- `settings/v2/workflow.rs` → `settings/workflow.rs`
- `settings/v2/mod.rs` — deleted (its `pub mod` / `pub use` block
  moved into `settings/mod.rs`).

`settings/mod.rs` picks up those `pub mod` declarations and the
accompanying `pub use <module>::*` re-exports, plus a transitional
`pub mod v2 { pub use super::*; }` alias so that existing
`fabro_types::settings::v2::*` import paths across the workspace
keep compiling. A follow-up sweep will drop the `::v2::` prefix from
every consumer and then the alias can go away.

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:32:54 -04:00
Bryan Helmkamp
3ac7ab9035
refactor(settings): stage 6.3b shrink server runtime types + delete Combine
Prunes `fabro-types/src/settings/server.rs` down to just the three
types that still have live consumers:

- `ApiAuthStrategy` — used by `fabro-server::jwt_auth::resolve_auth_mode_with_lookup`
- `TlsSettings` — used by `fabro-server::tls::*` and the mTLS integration test
- `ApiSettings` — the shim struct built by
  `fabro-server::serve::build_legacy_api_settings` so the pre-v2
  `resolve_auth_mode_with_lookup` signature still compiles

Deletes the rest as dead code (all unreferenced in the workspace):
`AuthProvider`, `AuthSettings`, `GitProvider`, `GitSettings`,
`GitAuthorSettings`, `WebSettings`, `WebhookSettings`,
`WebhookStrategy`, `SlackSettings`, `FeaturesSettings`, `LogSettings`,
`ArtifactStorageBackend`, `ArtifactStorageSettings`. Trims the
`ApiSettings` struct itself to just the two fields the auth resolver
reads; drops the never-used `base_url` field and the
`build_legacy_api_settings` lines that were computing it.

Drops `pub use settings::{ArtifactStorageBackend, ArtifactStorageSettings}`
from `fabro-types/src/lib.rs`.

Also deletes the dead `Combine` trait machinery alongside its only
remaining consumers:

- `lib/crates/fabro-types/src/combine.rs` — deleted.
- `pub mod combine;` / `pub use fabro_macros::Combine;` removed from
  `fabro-types/src/lib.rs`.
- `#[proc_macro_derive(Combine)] fn derive_combine` — deleted from
  `fabro-macros/src/lib.rs` along with its `syn::{Data, DeriveInput,
  Fields}` imports. The `e2e_test` proc-macro is untouched.

The seven legacy runtime type modules
(`hook`, `mcp`, `project`, `run`, `sandbox`, `user`, plus now the
bulk of `server`) are effectively all gone. Only a tiny `server.rs`
remains as a transitional home for the three auth-resolver types
until Stage 6.6g rewrites `resolve_auth_mode_with_lookup` to walk
the v2 `server.auth.api` subtree directly.

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:27:46 -04:00
Bryan Helmkamp
6df8bbeb3c
refactor(settings): stage 6.3b promote sandbox runtime types into fabro-sandbox
Moves the sandbox runtime types from `fabro-types/src/settings/sandbox.rs`
into a new `fabro-sandbox/src/config.rs` module:

- `SandboxSettings`, `LocalSandboxSettings`, `DaytonaSettings`,
  `DaytonaSnapshotSettings`, `DaytonaNetwork`, `DockerfileSource`,
  `WorktreeMode` (with the custom serde `DaytonaNetwork`
  serialize/deserialize impls intact).
- `bridge_sandbox` and `bridge_worktree_mode` (v2
  `RunSandboxLayer` → `SandboxSettings` converters) also move from
  `fabro-types/src/settings/v2/to_runtime.rs` into the new config
  module.

`fabro-sandbox/src/daytona/mod.rs` and `sandbox_spec.rs` update to
import from the crate-local `config` module instead of
`fabro_types::settings::sandbox`. The daytona module still re-exports
`DaytonaSettings as DaytonaConfig` etc., so no breaking changes for
callers of `fabro_sandbox::daytona::*`.

Consumer updates:

- `fabro-workflow/src/operations/start.rs` and `pipeline/types.rs`
  now import `WorktreeMode`, `SandboxSettings` (as `sandbox_config`
  alias), `bridge_sandbox`, and `bridge_worktree_mode` from
  `fabro_sandbox::config`.
- `fabro-server/src/run_manifest.rs` imports `bridge_sandbox` from
  `fabro_sandbox::config`.

`to_runtime.rs` in fabro-types shrinks to just the three remaining
helpers tied to the legacy `run.rs` module types
(`bridge_merge_strategy`, `bridge_pull_request`, `bridge_run_artifacts`).
Those move out in the next 6.3b pass when the `run.rs` module itself
moves.

Five of the seven legacy runtime type modules are now gone; two
remain (run, 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:17:06 -04:00
Bryan Helmkamp
4a40c73b71
refactor(settings): stage 6.3b delete legacy flat Settings struct
Deletes `fabro_types::Settings` — the ~65-field legacy flat view that
has been read-only since Stage 6.1 migrated all production read sites
to the v2 `SettingsFile`.

The last remaining readers all fall out of this commit:

- `fabro-server/src/demo/mod.rs` — the two demo settings fixtures
  (`runs::settings()` and `settings::server_settings()`) are rewritten
  as `serde_json::json!(...)` literals in the v2 `SettingsFile` shape.
  They produce the same wire bytes as the real handlers now return, so
  the demo page keeps rendering identically.
- `fabro-server/src/lib.rs::server_config` — drops the
  `pub use fabro_types::Settings` re-export. Only the inner
  `fabro_types::settings::server::*` module (still around until the
  full runtime-type cleanup) remains.
- `fabro-server/tests/it/openapi_conformance.rs` — drops the
  `server_settings_keys_match_openapi_spec` schema-drift test and all
  of its legacy type imports. The new freeform-object DTO in the spec
  (`type: object, additionalProperties: true`) has no `properties` to
  diff against, so the test was already a no-op. Leaves
  `all_spec_routes_are_routable` in place.
- `fabro-store/src/run_state.rs` — test fixture was building a
  `Settings::default()` JSON payload; switched to `SettingsFile::default()`.
- `fabro-types/src/run_event/mod.rs` — two `EventBody::RunCreated`
  round-trip tests were constructing `Settings::default()`; switched
  to `SettingsFile::default()`.
- `fabro-workflow/tests/it/integration.rs` — the two
  `hook_toml_*_parsing` tests decoded top-level `[[hooks]]` into a
  legacy `Settings`. That parse path was removed in Stage 6.1; the
  tests are deleted and replaced with a comment pointing at the v2
  `settings::v2::tree::tests` fixtures that cover the same ground.

The legacy flat struct's module-level doc comment in
`settings/mod.rs` is updated to explain the transitional runtime
shapes that still live under `hook`, `mcp`, `project`, `run`,
`sandbox`, `server`, and `user` — a follow-up pass will either
promote them into their consumer crates or inline them at the call
sites so the whole `settings/*.rs` file set can go away and 6.5b
flattening can happen.

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 17:32:03 -04:00
Bryan Helmkamp
eae89a6f53
refactor(events): dedupe schema v2 plumbing
Mostly consolidation of code added in the recent schema v2 work:

- Share a single ActorRef::user() constructor between server control
  actions and workflow provenance conversions.
- Share StageScope::from_context() between current_stage_scope and
  StageScope::for_handler so the 4-field construction lives in one place.
- Collapse RunEvent::to_value's if-let chain into an insert_opt helper.
- Use Value::String(id.to_string()) instead of serde_json::to_value for
  StageId/ParallelBranchId when seeding the parallel branch context.
- Share parse_event_envelopes via tests/it/support/mod.rs instead of
  duplicating the parsing block in two CLI run_events helpers.

Also fix parallel-branch git.commit to emit via emit_scoped with a
branch-specific StageScope so it carries stage_id / parallel_group_id /
parallel_branch_id alongside the other stage-scoped events.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 17:23:54 -04:00
Bryan Helmkamp
f5b9f82a27
feat(settings): stage 6.6 wire server + CLI to v2 SettingsFile DTO
Replaces the Stage 6.2 stopgap `strip_nulls(serde_json::to_value(full
SettingsFile))` path in `get_server_settings` with an explicit
redaction pass in the new `fabro_server::settings_view` module.

The redaction drops the narrow set of fields that leak operational
secrets or host filesystem layout:

- `server.listen.*` (bind + TLS material)
- `server.auth.api.jwt.{issuer, audience}` (auth topology)
- `server.auth.api.mtls.ca` (filesystem path)
- `server.auth.web.providers.github.client_secret`

Every other field is preserved. `InterpString` values that reference
`${env.NAME}` already serialize to their unresolved template form, so
no additional env-provenance walk is needed in this pass.

Implements the real `/api/v1/runs/:id/settings` handler — previously
wired to `not_implemented` — by opening the run reader, reading the
persisted `RunRecord.settings`, running it through the same
redaction, and serializing. The demo route still points at
`demo::get_run_settings`, unchanged.

Updates `fabro-cli` to deserialize the new wire shape as
`SettingsFile` directly:

- `server_client::retrieve_server_settings` now returns
  `SettingsFile` (no longer the legacy flat `Settings`) by decoding
  the progenitor `types::ServerSettings` newtype map into a
  `serde_json::Value` and then into `SettingsFile`.
- `commands/config/mod.rs::legacy_settings_to_v2` shim (TODO-1)
  **deleted**; `merged_config` passes the v2 file straight into
  `effective_settings::resolve_settings`.
- The `fabro-cli` integration tests rewrite their mock `/api/v1/settings`
  payloads as v2 TOML via `ConfigLayer::parse` instead of hand-rolling
  the legacy TOML shape.

All 3,761 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 17:16:26 -04:00
Bryan Helmkamp
8097c224ec
feat(events): populate actor on control-action events
Per the schema v2 spec (docs-internal/fabro-event-schema-v2-concrete-shape.md:208-229),
`actor` is expected on control actions like `run.cancel.requested` to
identify the user who initiated the request. Before this commit, the
three Event::Run{Cancel,Pause,Unpause}Requested variants were bare
unit variants and the cancel/pause/unpause HTTP handlers used the
_auth: AuthenticatedService ZST extractor which discards user
identity.

- fabro-workflow/src/event.rs: add `actor: Option<ActorRef>` to
  Event::RunCancelRequested, Event::RunPauseRequested,
  Event::RunUnpauseRequested. Add a stored_event_fields_for_variant
  match arm that copies the actor into the envelope. Update
  event_body_from_event, event_name, and the trace! debug arm to
  ignore the new field via `{ .. }`.
- fabro-server/src/server.rs: switch cancel_run, pause_run,
  unpause_run from _auth: AuthenticatedService to
  subject: AuthenticatedSubject (which handles cookie/JWT/mTLS
  identity uniformly via lib/crates/fabro-server/src/jwt_auth.rs).
  Add an actor_from_subject helper that mirrors the existing
  actor_from_provenance in fabro-workflow -- both produce an
  ActorRef { kind: User, id: login, display: login }.
  append_control_request takes a new Option<ActorRef> argument and
  constructs the variants with it. Test call sites pass None.

Test: new unit test control_action_events_carry_actor_in_envelope
in event.rs covering cancel/pause/unpause with Some(actor) and
unpause with None. Run mode AuthMode::Disabled returns
subject.login = None, so actor ends up None in that path -- matches
the spec's "actor is optional" guidance.

Wire format is backward compatible: actor uses
#[serde(default, skip_serializing_if = "Option::is_none")] so old
persisted events without the field still parse cleanly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 16:42:24 -04:00
Bryan Helmkamp
a3fd3b002b
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
ea206e0e40
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
dc856d0884
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
5d9aad85a3
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
ea84d9bc72
refactor(store): flatten EventEnvelope wire shape via serde
Replace the hand-written to_wire_value / from_wire_value helpers
and the wire_event_envelope_from_generated bridge with
#[serde(flatten)] on EventEnvelope.payload. Derived serde now
produces and accepts the wire shape natively:

  { "seq": 42, "id": "...", "event": "...", ... }

instead of the nested { "seq": 42, "payload": { ... } } the
derive would otherwise emit. #[serde(flatten)] composes fine with
the #[serde(transparent)] EventPayload(Value) wrapper, so the
inner payload object is merged into the outer map on both sides.

- fabro-store/src/types.rs: add #[serde(flatten)]; delete the two
  wire helpers (33 lines of Value-map poking); update the
  round-trip test to assert the shape is actually flat.
- fabro-server/src/server.rs: sse_event_from_store serializes
  the envelope directly; api_event_envelope_from_store pipelines
  to_value into from_value.
- fabro-cli/src/server_client.rs: buffer_sse_events parses
  straight into EventEnvelope via serde_json::from_str;
  list_run_events uses the existing convert_type helper in place
  of the deleted wire_event_envelope_from_generated bridge.
- fabro-cli tests: helpers that called from_wire_value now call
  serde_json::from_value.

Drops the shape check that from_wire_value used to perform on
parse (id/ts/run_id/event must exist as strings): that check
extracted run_id from the payload and then validated it against
itself, so it only guaranteed presence, not correctness.
EventPayload::new(value, expected_run_id) still runs the same
check where a caller has a real external run_id to cross-match.

Generated code and the OpenAPI allOf(seq, RunEvent) schema are
untouched; the wire JSON is byte-identical before and after.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 12:09:00 -04:00
Bryan Helmkamp
747ccc0383
refactor(events): tidy schema v2 plumbing
Quality cleanup on top of the v2 envelope commits:

- fabro-workflow/src/event.rs: add ActorKind/ActorRef/RunProvenance
  to the existing ::fabro_types import block so call sites can use
  unqualified names (restores CLAUDE.md import style). Extract a
  node_stored_fields helper to collapse 4 near-identical match arms
  in stored_event_fields. Drop the no-op ..default() from the Agent
  arm where all 9 fields are set explicitly.
- fabro-types/src/run_event/mod.rs: collapse 9 copies of the
  obj.get/as_str/to_string chain in from_ref behind an opt_str
  closure.
- fabro-server/src/server.rs: dedupe the two identical error
  closures in api_event_envelope_from_store. Skip the typed
  ApiEventEnvelope roundtrip in sse_event_from_store so streamed
  events go straight from the wire Value to a JSON string.
- fabro-workflow/src/handler/llm/api.rs: inline current_visit into
  its sole caller current_stage_event_scope.

Also fixes pre-existing test compile breakage carried in by the
v2 commits: restore the fabro_types::RunId import in support.rs
(removed by b51403ae but still referenced by find_run_dir), and
thread parallel_group_id/parallel_branch_id: None through 9
Event::Stage*/Event::Agent constructors in run_progress and
store/dump tests that 91d61016 missed.

No behavior change aside from the SSE hot path avoiding one full
strong-type deserialize + reserialize per event.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 11:42:02 -04:00
Bryan Helmkamp
b51403ae64
refactor(events): simplify envelope metadata plumbing
Centralize flattened EventEnvelope conversion in fabro-store so the CLI,
server, and test helpers reuse one wire-shape path. Also thread parallel
group and branch ids through nested stage and agent events so the new
envelope fields stay populated inside parallel branches.
2026-04-09 11:08:38 -04:00
Bryan Helmkamp
b572482361
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
51dc4350a5
feat(api): flatten EventEnvelope wire JSON (schema v2)
Wire EventEnvelope now inlines the RunEvent payload fields alongside
seq at the top level of the JSON object. The internal Rust
EventEnvelope { seq, payload } stays structurally unchanged; only the
API/SSE serialization layer flattens for clients.

- OpenAPI spec: add stage_id, parallel_group_id, parallel_branch_id,
  tool_call_id, actor to RunEvent; model EventEnvelope as allOf(seq,
  RunEvent); introduce ActorRef/ActorKind schemas.
- fabro-server: rewrite api_event_envelope_from_store to merge seq
  into the payload JSON value before returning the generated flat
  type; remove the now-unused nested ApiRunEvent conversion helper.
- fabro-cli server_client: add wire_event_envelope_into_store helper
  that turns flat wire JSON back into fabro_store::EventEnvelope
  { seq, payload } for internal consumers.
- Regenerate progenitor Rust types and typescript-axios client.
- Update demo stubs, SSE tests, CLI test helpers, and insta
  snapshots to expect the flattened shape and the new stage_id field.

Incidental: the typescript regeneration also picked up prior-merged
spec fields (ApiQuestion stage/timeout/context, upload manifest
batches, web-settings) that were stale in the TS client.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 10:40:21 -04:00
Bryan Helmkamp
a6047250cf
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
eabbca649b
feat(tests): migrate fabro-cli fixtures and repo fabro.toml to v2
Stage 4 consumer migration: rewrite test fixtures across fabro-cli
integration tests and the repo's own fabro.toml + workflow.toml files
to use the v2 namespaced schema.

Fixtures migrated:
- repo fabro.toml: [fabro] root → [project] directory, [pull_request]
  → [run.pull_request], [sandbox] → [run.sandbox], daytona labels
  and snapshot moved under [run.sandbox.daytona], [[hooks]] →
  [[run.hooks]] with id, integer memory/disk → '8GB'/'20GB' Size
  values
- fabro/workflows/{implement-issue,implement-plan,gh-triage,smoke}/
  workflow.toml: version → _version, [github] →
  [server.integrations.github]
- fabro-cli integration tests: config.rs (settings/external fixtures),
  repo.rs, repo_init.rs, runner.rs, run.rs, store_dump.rs,
  workflow.rs, workflow_create.rs, support.rs
- fabro-server run_manifest.rs: prepare_manifest test constructs v2
  manifest configs (run.prepare.steps + server.integrations.github)
  and updated assertion to reflect v2 whole-list replacement of
  run.prepare.steps across layers

Validate command tests all green; ~10 tests remain that need targeted
fixes for specific behaviors that shifted between schemas.
2026-04-09 09:57:30 -04:00
Bryan Helmkamp
a0eec6aee1
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
bc7a926e26
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
8710643fc9
fix(test): remove dist/ dependency from source_maps_are_not_served test
Use a synthetic .map path instead of scanning apps/fabro-web/dist at
runtime, which requires a prior bun build and breaks on fresh checkouts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 16:51:23 -04:00
Bryan Helmkamp
7dd93b7a28
fix(test): remove workspace test build warnings
Drop the unused test-only import in fabro-server and the unused test
binding in fabro-workflow so workspace test builds stay warning-free.
2026-04-08 16:47:23 -04:00
Bryan Helmkamp
622760952d
Merge remote-tracking branch 'origin/main' 2026-04-08 16:30:26 -04:00
Bryan Helmkamp
13bcac88dc
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
b101bfd5d5
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
552d46398e
Merge remote-tracking branch 'origin/main' into production-web-ui 2026-04-08 16:07:38 -04:00
Bryan Helmkamp
a54e27f42e
fix: resolve clippy warnings across workspace
- fabro-types: remove redundant "freeform" match arm (match_same_arms)
- fabro-server: use let...else and remove needless return
- fabro-cli/runner: use while-let instead of match loop, unwrap Option
  from build_artifact_uploader return type
- fabro-cli/attach: introduce AttachOptions struct to reduce bool
  parameter count (fn_params_excessive_bools)
- fabro-test: fix unused variable and needless continue in session lock

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 15:56:51 -04:00
Bryan Helmkamp
fdb1297442
feat(server): support host-only tcp binds
Accept `--bind <ip>` as a TCP bind request while keeping the default
Unix socket behavior unchanged. Resolve host-only TCP binds inside the
serving process so startup output, server metadata, and status always
reflect the concrete host:port, preferring 32276 and falling back to a
random port with a warning when needed.
2026-04-08 15:43:09 -04:00
Bryan Helmkamp
8bab1335c9
Merge remote-tracking branch 'origin/main' into production-web-ui 2026-04-08 14:44:35 -04:00
Bryan Helmkamp
c58b731171
feat: embed fabro spa and align interruption semantics
Move the built web bundle into an embedded fabro-spa crate so Cargo and
release builds no longer depend on Bun at build time, and preserve the
local dev override path for fast UI iteration.

At the same time, rename interview and agent-level aborted flows to
interrupted, keep cancelled for run-level shutdown, and stop reporting
skipped answers as interruptions in the run event stream.
2026-04-08 14:43:44 -04:00
Bryan Helmkamp
c4b51e44ea
fix: session cookie decryption and add HTTP endpoint logging
Cookie auth was broken because parse_cookie_header used Cookie::parse
which does not percent-decode values. The cookie crate's private jar
percent-encodes on Set-Cookie but Cookie::parse leaves %2F/%3D intact,
making base64 decryption fail silently. Switch to Cookie::parse_encoded.

Also:
- Add tower-http TraceLayer for request/response logging (DEBUG for
  requests, INFO for responses with status and latency)
- Add structured tracing to all web_auth handlers per logging strategy
- Replace eprintln debug calls with tracing::warn
- Update GitHub App manifest homepage URL to https://fabro.sh

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 11:46:30 -04:00
Bryan Helmkamp
90b373c366
fix(test): eliminate recovery, cancel, and label flakes
Stabilize the recovery scenario around rebuilt metadata timing and node
ordinals, make in-process run cancellation converge on a cancelled
reason, and keep the label assertion unit test out of the shared
TestContext session lifecycle.
2026-04-08 10:13:17 -04:00
Bryan Helmkamp
c663beb18a
fix: GitHub App setup flow — nullable webhook_secret, double POST, stale restart copy, wrong port
- Change webhook_secret to Option<String> in GitHubManifestConversion since
  GitHub's API returns null when no webhook URL is configured
- Use useRef guard to prevent React StrictMode from firing the one-time
  manifest conversion POST twice
- Remove fake "restart required" flow — server reads auth config lazily so
  no restart is needed after setup
- Derive web.url and api.base_url from the request Origin header instead of
  hardcoding port 3000
- Add error logging for manifest conversion parse failures

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 08:34:39 -04:00
Bryan Helmkamp
007128ceb9
feat: wire web UI to real server with demo mode toggle
Server changes:
- Add /boards/runs to demo routes (delegates to list_runs)
- Fix demo get_run_status to return StoreRunSummary shape matching OpenAPI spec
- Enrich real /boards/runs to return RunListItem shape with board column mapping
  (Running->working, Paused->pending, Completed->merge; others excluded)
- Update existing tests that asserted old RunStatusResponse fields from /boards/runs

Web UI changes:
- Add DemoModeProvider context and useDemoMode hook
- Hide Workflows/Insights nav items in production mode via getVisibleNavigation
- Change run-detail loader to use /runs/{id} directly instead of searching /boards/runs
- Add mapRunSummaryToRunItem for mapping server response to UI shape
- Add Graph tab, hide Stages tab in production mode, always hide Files tab
- Make run-overview and run-graph loaders resilient to 501 via apiJsonOrNull
- Add isNotImplemented and apiJsonOrNull helpers to api.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 04:46:08 -04:00
Bryan Helmkamp
57ec693a90
test(cli): replace slow active-run coverage
Replace the slow CLI integration tests that waited on worker shutdown
grace periods with focused coverage that still checks the important
behavior. The attach JSON test now finishes the gated run cleanly,
the rm force test uses a mocked server contract, and the Ctrl-C cancel
path is covered at the attach layer instead of through a full live run.

Add a cooperative subprocess cancel control message so cancel and delete
can abort pending interviews without relying only on the 5 second hard
kill fallback.
2026-04-08 03:47:35 -04:00
Bryan Helmkamp
57cc38894b
fix(server): skip delete grace for terminal runs
Completed runs can briefly retain a stale worker PID after their terminal
state is visible. Using the full 5s worker cancellation grace in that window
made rm and prune pay an avoidable delay.

Keep the existing grace for active runs, but use a short delete grace for
already-terminal runs so completed-run cleanup stays fast.
2026-04-07 23:43:06 -04:00