Commit graph

2078 commits

Author SHA1 Message Date
Bryan Helmkamp
a64f4d0cd8
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
3db0c84385
test optimizations 2026-04-10 18:08:44 -04:00
Bryan Helmkamp
0221805a73
fix(cli): let detached workers exit after post-run shutdown
Some checks are pending
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
TypeScript / Build (push) Waiting to run
Move worker control stdin handling off Tokio's blocking shutdown path so
subprocess workers can exit cleanly after success or cooperative
cancellation even when the parent still holds stdin open.

Add regression coverage for retro-enabled success and SIGTERM-driven
cancellation with stdin intentionally left open.
2026-04-10 15:20:41 -04:00
Bryan Helmkamp
9b0eeb3814
Merge remote-tracking branch 'origin/main' 2026-04-10 12:02:05 -04:00
Bryan Helmkamp
4a872633ce
fix(core): prevent infinite loop when goal-gate retry target is terminal
Skip retry when get_retry_target points at a terminal node — retrying
into a terminal re-triggers the same goal-gate failure endlessly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 12:01:46 -04:00
Bryan Helmkamp
7fc133a2b1
chore(simplify): remove unnecessary comment and avoid double-serialization in event API
- Remove narrating comment in run_manifest.rs (code is self-explanatory)
- Optimize api_event_envelope_from_store: reuse payload's existing
  serde_json::Value instead of serialize-then-deserialize round-trip

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 11:47:15 -04:00
Bryan Helmkamp
76089bd8aa
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
3085ad56f8
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-cli/src/commands/install.rs
2026-04-10 10:10:23 -04:00
Bryan Helmkamp
9dea052093
feat(install): let user choose GitHub App owner (personal or org)
Add GhCli wrapper for best-effort gh CLI detection and org discovery.
During `fabro install`, prompt the user to create the GitHub App under
their personal account or an org they admin, with a manual entry fallback
for org app managers. App name defaults to `{owner}-fabro`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 09:32:05 -04:00
Bryan Helmkamp
42f8ec271b
test(fabro-test): scrub FABRO_* env from spawned subprocesses
TestContext::command() was inheriting all parent env vars, so a
developer (or CI) running with FABRO_CONFIG set would pollute child
test subprocesses, causing settings_local_* IT tests to fail with
opaque assertion errors.

Iterate std::env::vars_os() and env_remove every FABRO_* key before
re-adding the controlled set (FABRO_NO_UPGRADE_CHECK, etc.). Safe to
iterate because the prior two commits eliminated all std::env::set_var
callers in fabro-cli and fabro-config tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 08:49:44 -04:00
Bryan Helmkamp
0a57367fee
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
06d9ef6118
refactor(cli): inject user settings layer into build_run_manifest
build_run_manifest was reading FABRO_CONFIG env and ~/.fabro/settings.toml
internally, which forced its 3 unit tests to use unsafe std::env::set_var
to isolate from the developer's real config. This violates the project rule
against mutating shared mutable state in tests.

Add user_layer: SettingsLayer and user_settings_path: Option<PathBuf> to
ManifestBuildInput so callers pass the user layer explicitly.

- Production callers (graph, preflight, validate, run/create) load via
  load_settings_user() + active_settings_path(None) at the command boundary.
- Tests pass SettingsLayer::default() and None, needing no env access.
- Delete all unsafe { set_var/remove_var } blocks and #[allow(unsafe_code)]
  attributes from the 3 manifest_builder tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 08:49:22 -04:00
Bryan Helmkamp
1dfb8fc272
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
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
e7d890bbf5
refactor(settings): resolve workflow settings 2026-04-10 07:12:29 -04:00
Bryan Helmkamp
48c5737bd5
refactor(settings): resolve project settings 2026-04-10 07:10:32 -04:00
Bryan Helmkamp
a61d9f84ae
refactor(settings): resolve cli settings 2026-04-10 07:04:34 -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
f414a7d719
chore(simplify): events schema v2 cleanup from review
Cleanup pass on the events schema v2 work merged from origin/main.

Quality fixes:
- prompt.rs: drop dead `_visit` local; use stage_scope.visit at the
  emit site (the value was being recomputed inline next to a scope
  that already had it).
- llm/cli.rs: rename `_context` to `context` in CodergenBackend::run
  (it's actually used now); delete the lingering `current_visit`
  helper that was deleted from llm/api.rs in c6a78a428 but missed
  here; use stage_scope.visit at the emit site.
- llm/api.rs: rename `event_scope` to `stage_scope` for consistency
  with every other handler.
- agent.rs, fan_in.rs, parallel.rs: same `visit_from_context` →
  `stage_scope.visit` substitution at every event-emit site.
- parallel.rs: switch ParallelStarted/ParallelCompleted from `emit`
  to `emit_scoped` so they carry stage_id in the envelope.
- event.rs: fix the StageScope::for_handler docstring — the lifecycle
  hook is `before_node`, not `before_attempt`.

Reuse fixes:
- run_event/mod.rs: add `ActorRef::agent(session_id, display)` symmetric
  with the existing `ActorRef::user`; use it from agent_actor_for_event
  in workflow event.rs.

Correctness fixes:
- event.rs: introduce `StageScope::for_parallel_branch` to name the
  "branch starts at visit 1" invariant the parallel handler was
  hardcoding via a struct literal at parallel.rs:307. This makes
  the assumption auditable and gives a single place to fix when
  parallel nodes ever loop.

Efficiency fixes:
- stage_id.rs: switch StageId/ParallelBranchId Serialize impls from
  `serializer.serialize_str(&self.to_string())` to `collect_str(self)`,
  removing one transient String allocation per ID per emitted event.

Hardening:
- event.rs: add `#[must_use]` on `to_run_event`, `to_run_event_at`,
  and `event_name`.
- store/types.rs: add a second wire-envelope round-trip test that
  populates stage_id, parallel_group_id, parallel_branch_id,
  session_id, parent_session_id, tool_call_id, and actor — the
  existing test only exercised stage_id, so a regression in any of
  the other envelope fields' #[serde(flatten)] interaction would
  have been silent.

All 3810 workspace tests pass; clippy and fmt clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 22:11:20 -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
003b691de5
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
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
194fe7997d
docs(plans): commit stage 6 handoff 2 (previously untracked)
This doc was written at the end of the session that landed Stages
6.1-6.5 but never committed; it's been sitting untracked for three
follow-up sessions. Handoff docs 3 and 4 both point at it as their
predecessor, so it belongs in the tree alongside them.

No content change; the file is committed as originally written.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 18:51:44 -04:00
Bryan Helmkamp
345d43721a
docs(plans): write Stage 6 wrap-up handoff — all substages complete
Captures the full end-state after this session finished the
consumer-migration pass through 6.3b, flattened the v2 directory
(6.5b), rewrote the auth resolver (6.6g), and closed out the last
scoped TODOs from handoff-2.

Nothing left in Stage 6. Next work is either from the deferred list
(setup_register toml_edit upgrade, ModelRegistry for fallback
chains, goal_file schema decision, fail-closed server posture,
centralized env interp pass, optional OpenAPI formalization) or
driven by new requirements.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 18:44:13 -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
7f9640aac7
refactor(settings): stage 6.3b promote run runtime types + delete to_runtime
Moves the only actively-used types from
`fabro-types/src/settings/run.rs` — `PullRequestSettings`,
`MergeStrategy`, `ArtifactsSettings` — into a new
`fabro-workflow/src/config.rs` module. The other types in that file
(`LlmSettings`, `SetupSettings`, `CheckpointSettings`,
`GitHubSettings`) had no remaining consumers in the workspace and
are deleted outright.

`bridge_pull_request`, `bridge_merge_strategy`, and
`bridge_run_artifacts` move along with them into
`fabro-workflow/src/config.rs`. That empties
`fabro-types/src/settings/v2/to_runtime.rs`, so the file is deleted
and its `pub mod` declaration removed from `v2/mod.rs`. Stage 6.2's
"narrow runtime-type conversion helpers" module is completely gone.

Consumer updates:

- `fabro-workflow/src/lib.rs` exposes `pub mod config`.
- `fabro-workflow/src/operations/start.rs` imports
  `PullRequestSettings` and `bridge_pull_request` from
  `crate::config`.
- `fabro-workflow/src/pipeline/types.rs` imports
  `PullRequestSettings` from `crate::config`.
- `fabro-workflow/src/pipeline/pull_request.rs` imports
  `MergeStrategy` from `crate::config`.

`fabro-types/src/settings/mod.rs` drops `pub mod run` and the
corresponding `pub use run::{ArtifactsSettings, ...}` re-export.

Six of the seven legacy runtime type modules are now gone; only
`server.rs` remains. 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:21:59 -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
38dacb8744
refactor(settings): stage 6.3b promote mcp runtime types into fabro-mcp
Moves `McpServerEntry`, `McpServerSettings`, `McpTransport`, plus the
`default_startup_timeout_secs` / `default_tool_timeout_secs` helpers
from `fabro-types/src/settings/mcp.rs` into
`fabro-mcp/src/config.rs`. fabro-mcp was already the only crate that
re-exported them, so this deletes the `fabro-types` module entirely
and drops the `pub use mcp::*` re-export from `settings/mod.rs`.

`bridge_mcps` / `bridge_mcp_entry` (v2 `McpEntryLayer` → runtime
`McpServerEntry` converters) also move to `fabro-mcp/src/config.rs`.
`fabro-workflow::operations::start` and `fabro-cli::commands::exec`
now import `bridge_mcp_entry` from `fabro_mcp::config::bridge_mcp_entry`
instead of the v2 `to_runtime` module.

Four of the seven legacy runtime type modules are now gone; three
remain (run, sandbox, server). The `to_runtime.rs` module is down to
just sandbox, pull-request, merge-strategy, artifacts, and
worktree-mode helpers.

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:12:00 -04:00
Bryan Helmkamp
2016c8e948
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
db45511ff5
refactor(settings): stage 6.3b promote user runtime types into consumers
First consumer migration pass. Deletes
`lib/crates/fabro-types/src/settings/user.rs` outright:

- `OutputFormat`, `PermissionLevel`: moved into `fabro-agent/src/cli.rs`
  where they are actually consumed as `AgentArgs` fields. They carry
  clap `ValueEnum` derives so `fabro-cli` keeps importing them via the
  `fabro_agent::cli::{OutputFormat, PermissionLevel}` public path.
- `ClientTlsSettings`: moved into `fabro-cli/src/user_config.rs` as a
  crate-private struct. Only `fabro-cli` references it (via
  `cli_target_from_v2` when building the HTTP client).
- `ExecSettings`, legacy `ServerSettings` (from `settings::user`):
  deleted outright — no callers remained.

Also removes the now-dead `From<&GitAuthorSettings> for GitAuthor`
impl in `fabro-checkpoint/src/author.rs`. The v2 `GitAuthorLayer`
conversion is the only path `fabro-workflow::git::git_author_from_settings`
uses. Drops the `fabro_types::settings::server::GitAuthorSettings`
import along with it.

`settings/mod.rs` drops the `pub mod user` declaration and the
`pub use user::*` re-export line. One of the seven legacy runtime
type modules is now gone; six remain.

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:59:49 -04:00
Bryan Helmkamp
2986c1055f
docs(plans): write stage 6.6 + 6.3b-partial handoff
Captures what landed in this session:

- Stage 6.6a/b/c: OpenAPI DTO collapse (commit 7c8448ece)
- Stage 6.6d/e/f/i: Server handlers + CLI + demo migration (f5b9f82a2)
- Stage 6.6h: fabro-web literal rewrite (65a9fd137)
- Stage 6.3b first pass: delete fabro_types::Settings (4a40c73b7)

Plus what still remains:

- Stage 6.3b runtime type module cleanup (blocked on consumer migration)
- Stage 6.5b directory flatten (blocked on 6.3b)
- Stage 6.6g auth resolver rewrite
- Stage 6.6j setup_register review
- 5 of 12 scoped TODOs still open; 7 resolved

Also records the consumer migration map — ~33 import sites across
8 crates that need individual per-crate migration. This is the bulk
of the remaining Stage 6 work.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 17:35:23 -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
cbbbc6f90f
docs
Some checks are pending
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
TypeScript / Build (push) Waiting to run
2026-04-09 17:24:30 -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
65a9fd137b
refactor(fabro-web): stage 6.6 rewrite workflowData literal to v2 shape
The hardcoded sample workflow entries in `workflow-detail.tsx` still
embedded the legacy flat `RunSettings` shape (top-level `llm`, `vars`,
`sandbox`, `setup`) — a visible mismatch with what the server now
returns on `/api/v1/runs/:id/settings`.

Rewrites the four static literals (fix_build, implement, sync_drift,
expand) to mirror the v2 `SettingsFile` tree: `_version`, `run.goal`,
`run.inputs`, `run.model`, `run.sandbox`, `run.prepare.steps`,
`run.prepare.timeout`, etc. Duration and size fields now use the
human-readable forms (`"120s"`, `"8GB"`, `"10GB"`) per R83 / R84.

Adds a module-level doc comment pointing readers at the
`fabro_types::settings::SettingsFile` Rust type as the source of truth
for the shape. `RunSettings` stays as `Record<string, unknown>`, so
the literal typechecks without needing a formal type assertion on
each entry.

fabro-web `typecheck` / `test` / `build` stay green.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 17:18:34 -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
7c8448ece8
refactor(api): stage 6.6 collapse settings DTOs to freeform v2 shape
Replaces the legacy flat `ServerSettings` / `RunSettings` schemas in
`docs/api-reference/fabro-api.yaml` and 20+ supporting nested type
schemas (LlmSettings, SandboxSettings, HookDefinition, WebSettings,
ApiSettings, GitSettings, McpServerEntry, etc.) with two simple
`type: object, additionalProperties: true` schemas that declare the
wire shape as the v2 `SettingsFile` tree with secret-bearing subtrees
dropped before serialization.

Regenerates the Rust progenitor and TypeScript Axios clients against
the new spec. The progenitor generates `RunSettings` / `ServerSettings`
as `#[serde(transparent)]` newtypes over `serde_json::Map<String,
Value>`; the openapi-generator emits `{ [key: string]: any; }` inlined
into the API method signatures and no longer exports named model
types.

Updates fabro-web to define local `type ServerSettings =
Record<string, unknown>` / `type RunSettings = Record<string,
unknown>` aliases since the generated client no longer exports them.
The UI only `JSON.stringify`s these payloads into a CollapsibleFile,
so the opaque shape is fine.

All 3,756 workspace tests remain green. The OpenAPI conformance test
`server_settings_keys_match_openapi_spec` still passes because
`compare_schema` short-circuits on pure-map schemas (no `properties`);
it becomes a no-op that will be removed entirely when Stage 6.3b
deletes the legacy flat `Settings` struct it still builds.

Unblocks the server handler + CLI migration in the next commits of
Stage 6.6.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 17:04:33 -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
c6a78a4286
refactor(events): thread stage scope through emitter
Populate stage_id / parallel_group_id / parallel_branch_id on every
event tied to a concrete stage execution, per the spec at
docs-internal/fabro-event-schema-v2-concrete-shape.md:223-279.

Before this commit, stored_event_fields() only set stage_id for the
four Event::Stage* variants and Event::Agent -- the only variants
that carried visit/parallel_group_id/parallel_branch_id in their
payload. Every other stage-scoped event (Checkpoint*, PromptCompleted,
Command*, AgentCli*, Prompt, Interview*, Failover, StallWatchdog,
GitCommit, ArtifactCaptured) fell through to node_stored_fields()
and left stage_id as None.

New approach: scope is carried alongside the event, not on the
variant.

- fabro-workflow/src/event.rs: new StageScope type
  { node_id, visit, parallel_group_id, parallel_branch_id }. New
  Emitter::emit_scoped(&event, &scope) for stage-level emission.
  to_run_event_at and stored_event_fields take an
  Option<&StageScope> that merges into the returned envelope
  fields. StageScope::for_handler(context, node_id) is the
  canonical handler-side constructor -- prefers
  context.current_stage_scope() set by the fidelity lifecycle,
  falls back to a scope synthesized from the node_id + context
  visit count for tests that don't go through the full lifecycle.
- fabro-workflow/src/context.rs: new
  WorkflowContext::current_stage_scope() method reads CURRENT_NODE,
  internal.node_visit_count, internal.parallel_group_id,
  internal.parallel_branch_id from the context.
- Remove the now-redundant visit/parallel_group_id/parallel_branch_id
  fields from Event::Stage{Started,Completed,Failed,Retrying} and
  the parallel_* fields from Event::Agent. These existed only to
  feed stored_event_fields() and are obsolete once scope is
  threaded through the emitter.

Emission site migration (all stage-scoped handlers now use
emit_scoped):

- lifecycle/event.rs: StageStarted, StageCompleted, StageFailed,
  StageRetrying, CheckpointCompleted, GitCommit (from on_checkpoint)
- lifecycle/git.rs: CheckpointFailed
- lifecycle/artifact.rs: ArtifactCaptured
- handler/command.rs: CommandStarted, CommandCompleted
- handler/prompt.rs: Prompt, PromptCompleted
- handler/agent.rs: Prompt, PromptCompleted
- handler/fan_in.rs: Prompt, PromptCompleted
- handler/human.rs: InterviewStarted, InterviewTimeout,
  InterviewInterrupted, InterviewCompleted
- handler/llm/api.rs: Failover, Agent (via spawn_event_forwarder
  which now carries a StageScope across the tokio::spawn boundary)
- handler/llm/cli.rs: AgentCliStarted, AgentCliCompleted
- handler/parallel.rs: ParallelBranchStarted, ParallelBranchCompleted

StallWatchdogTimeout stays on plain emit() because the watchdog
fires from an error path without a live stage context.

Deleted the local StageEventScope struct + current_stage_event_scope
helper from handler/llm/api.rs; it's generalized into StageScope.

Tests: two new unit tests in event.rs --
stage_scope_populates_stage_id_on_non_stage_events verifies
CommandStarted / Prompt / GitCommit all pick up stage_id from scope,
run_level_events_without_scope_leave_stage_id_absent confirms
run.* events still get no stage scope. Updated all test fixtures
across fabro-workflow, fabro-cli to drop the removed Event variant
fields. Accepted two insta snapshot updates in
fabro-cli/tests/it/cmd/{attach,run}.rs that now include the
formerly-missing stage_id fields on checkpoint and interview events.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 16:34:59 -04:00