From dc856d0884741ad5135279c7b9c35c41899aa228 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 9 Apr 2026 15:25:59 -0400 Subject: [PATCH] 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> -- 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) --- ...-08-settings-toml-redesign-requirements.md | 544 ++++++++++++++++++ ...tings-toml-redesign-implementation-plan.md | 334 +++++++++++ lib/crates/fabro-checkpoint/src/metadata.rs | 5 +- .../fabro-cli/src/commands/config/mod.rs | 23 +- .../fabro-cli/src/commands/run/runner.rs | 14 +- .../fabro-cli/src/commands/store/dump.rs | 5 +- lib/crates/fabro-cli/src/manifest_builder.rs | 33 +- lib/crates/fabro-cli/tests/it/cmd/create.rs | 24 +- lib/crates/fabro-cli/tests/it/cmd/runner.rs | 2 +- .../fabro-config/src/effective_settings.rs | 9 +- lib/crates/fabro-server/src/diagnostics.rs | 11 +- lib/crates/fabro-server/src/run_manifest.rs | 3 +- lib/crates/fabro-server/src/serve.rs | 130 +++-- lib/crates/fabro-server/src/server.rs | 233 ++++---- lib/crates/fabro-server/src/web_auth.rs | 75 ++- .../fabro-server/tests/it/api/routing.rs | 21 +- .../fabro-server/tests/it/api/settings.rs | 23 +- .../fabro-server/tests/it/api/system.rs | 32 +- lib/crates/fabro-server/tests/it/helpers.rs | 36 +- lib/crates/fabro-store/src/slate/mod.rs | 7 +- .../fabro-workflow/src/operations/create.rs | 169 ++++-- .../src/operations/rebuild_meta.rs | 5 +- .../fabro-workflow/src/operations/start.rs | 47 +- .../src/pipeline/execute/tests.rs | 7 +- .../fabro-workflow/src/pipeline/finalize.rs | 5 +- .../fabro-workflow/src/pipeline/initialize.rs | 7 +- .../fabro-workflow/src/pipeline/persist.rs | 25 +- .../src/pipeline/pull_request.rs | 9 +- .../fabro-workflow/src/pipeline/retro.rs | 7 +- lib/crates/fabro-workflow/src/run_lookup.rs | 10 +- .../fabro-workflow/src/runtime_store.rs | 5 +- lib/crates/fabro-workflow/src/test_support.rs | 8 +- .../tests/it/daytona_integration.rs | 25 +- .../fabro-workflow/tests/it/integration.rs | 261 +++++---- 34 files changed, 1635 insertions(+), 519 deletions(-) create mode 100644 docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md create mode 100644 docs/plans/2026-04-08-settings-toml-redesign-implementation-plan.md diff --git a/docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md b/docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md new file mode 100644 index 000000000..412981a1e --- /dev/null +++ b/docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md @@ -0,0 +1,544 @@ +--- +date: 2026-04-08 +topic: settings-toml-redesign +--- + +# Settings TOML Redesign + +## Problem Frame + +Fabro has three layered TOML config files: + +- `~/.fabro/settings.toml` for machine defaults +- `fabro.toml` for project defaults +- `workflow.toml` for workflow-local defaults + +All three layer into one unified settings object. In same-host setups, the CLI and server may both read `~/.fabro/settings.toml`. In split-host setups, the CLI host and server host each read their own local `settings.toml` and consume only the sections relevant to that process. + +The current config shape grew organically. It now has naming drift, mixed ownership boundaries, uneven merge semantics, and several top-level sections that no longer reflect a clean mental model. Fabro is still greenfield with no deployed compatibility burden, so this is the right time to make a hard cut and establish a coherent, future-proof config language. + +The new design must optimize for: + +- a small, elegant top-level structure +- coherent ownership boundaries between run, CLI, server, project, and workflow concerns +- paste-anywhere ergonomics across the three config files +- explicit and predictable layering semantics +- future provider growth without provider-specific sprawl in the core model + +## Requirements + +**Config language and layering** + +- R1. `settings.toml`, `fabro.toml`, and `workflow.toml` must share the same schema. Files differ by precedence only, not by allowed sections. +- R2. Any config section may appear in any Fabro TOML file. Consumers must ignore sections they do not use. +- R3. The top-level schema must be strictly namespaced. The only top-level config domains are `[project]`, `[workflow]`, `[run]`, `[cli]`, `[server]`, and `[features]`, plus reserved underscore-prefixed meta keys. +- R4. The schema version key must be `_version`, not `version`. +- R5. Underscore-prefixed keys are reserved only at the top level for config-language metadata. Nested underscore keys are not part of the language. +- R6. The config language must not add a general unset mechanism in this pass. +- R7. Unknown config keys against the full union schema must be hard errors. This is schema validation, not consumer-specific validation. +- R8. Duplicate keys and duplicate hook `id` values within the same file must be hard errors. + +**Object model and namespace boundaries** + +- R9. `[workflow]` and `[run]` must be sibling top-level sections. Do not nest `[workflow.run]`. +- R10. `[workflow]` is descriptive for now. It must support first-class fields such as `name`, `description`, optional `graph`, and `metadata`. Structured workflow inputs are deferred. +- R11. `workflow.toml` remains the canonical workflow config filename. The default graph file remains `workflow.fabro`, with optional `[workflow].graph` override. +- R12. `[project]` must be a first-class project object with fields such as `name`, `description`, `directory`, and `metadata`. +- R13. `project.directory` replaces the old Fabro project root concept and means the Fabro-managed project directory inside the repo, defaulting to `fabro/`. +- R14. Workflow discovery remains conventional: `/workflows//workflow.toml`. Do not add a separate configurable workflows directory. +- R15. `[run]` is the shared execution domain. It may appear in all three files and layer normally. +- R16. `[cli]` and `[server]` are owner-first process domains. Settings belong to the process that reads them, not to whether the host is “local” or “remote.” For trust-boundary reasons, CLI and server processes consume their owner-specific sections only from the local `~/.fabro/settings.toml` plus explicit process-local overrides. Same-shaped `cli.*` and `server.*` stanzas in `fabro.toml` and `workflow.toml` remain schema-valid but inert for those processes. +- R17. `[features]` is a reserved cross-cutting namespace for Fabro capability flags only. It must have a high admission bar and must not become a junk drawer. +- R18. Logging is process-owned. Use `[cli.logging]` and `[server.logging]`; do not keep a shared logging section. + +**Run model** + +- R19. `[run]` must keep a small direct manifest surface for cross-cutting run fields such as `goal` and `working_dir`. +- R20. `working_dir` replaces `work_dir`. +- R21. `metadata` replaces Fabro-owned `labels` and exists on `project`, `workflow`, and `run` as flat string-to-string maps. +- R22. `run.inputs` replaces `vars`. `run.inputs` must accept TOML scalar values. `metadata` remains string-to-string. `run.inputs` intentionally replaces the full inherited map rather than merging by key. +- R23. `[run.model]` is the default model selection surface for LLM-backed workflow stages. `[run.agent]` is only for agent-specific settings. +- R24. `[run.agent]` owns agent-only knobs such as `permissions` and `mcps`. `[run.sandbox]` owns the sandbox selection and execution-environment surface, including `provider`, shared sandbox knobs, `env`, and provider-specific nested tables. +- R25. `run.agent.permissions` must remain a simple enum string, not an object. +- R26. `[run.git]` and `[run.scm]` must remain separate concepts. `git` is local Git behavior such as commit author; `scm` is remote host/provider behavior. +- R27. `[run.pull_request]` remains the provider-neutral run surface for PR behavior. +- R28. `[run.prepare]` is the run preparation surface and replaces the old `setup` naming. +- R29. `run.prepare` must be an ordered list of steps at `[[run.prepare.steps]]`. +- R30. `run.prepare.steps` replaces as a whole ordered list across layers. +- R31. `[run.execution]` groups run-conduct knobs such as `mode`, `approval`, and `retros`. In the first pass, `mode` is `normal | dry_run`, `approval` is `prompt | auto`, and `retros` is a positive-form boolean. Do not keep negated or ambiguous booleans like `no_retro`. +- R32. `[run.checkpoint]` remains its own run domain. `[[run.hooks]]` is the ordered run-hook surface for run lifecycle automation. +- R33. `[run.artifacts]` defines what run artifacts are collected. Server-side artifact storage is separate. +- R34. `[run.notifications.]` is a keyed set of named notification routes. Notification routes merge by field across layers and support `enabled = false`. +- R35. `[run.interviews]` is a single optional external/default interview delivery surface. HTTP/API answering is always available and is not modeled as an interview provider. +- R36. Notification and interview event selection must use raw Fabro event names, not a second notification-specific vocabulary. + +**CLI model** + +- R37. CLI target resolution lives under `[cli.target]`, not `[server]` or `[cli.remote]`. +- R38. CLI target transport must be explicit with `type = "http" | "unix"` and transport-specific fields, not overloaded scheme strings. +- R39. CLI transport TLS lives under `[cli.target.tls]`. +- R40. CLI auth is a separate domain at `[cli.auth]`, with explicit `strategy` selection. `strategy = "none"` explicitly disables inherited auth. +- R41. `fabro exec` defaults live under `[cli.exec]`, with `[cli.exec.model]` and `[cli.exec.agent]` split cleanly. +- R42. Generic CLI output defaults live under `[cli.output]`, not under `exec`. +- R43. Upgrade checks live under `[cli.updates]`. +- R44. Idle sleep prevention lives under `[cli.exec]`. + +**Server model** + +- R45. `[server]` is a namespace container. Actual settings live in named subdomains. +- R46. The server binds the API and web surfaces on one shared listener. Bind transport must live under `[server.listen]`, not separately under `[server.api]` and `[server.web]`. +- R47. `[server.listen]` must use explicit transport types such as `tcp` and `unix`. +- R48. Shared listener TLS must live under `[server.listen.tls]`. +- R49. `[server.api]` holds only API-surface settings such as public URL, not bind/auth/TLS settings. +- R50. `[server.web]` holds only web-surface settings such as `enabled` and public URL, not auth settings. +- R51. Server auth is a cohesive domain at `[server.auth]`. +- R52. `[server.auth.api]` must support multiple strategies concurrently. +- R53. `[server.auth.web]` must support multiple providers concurrently via `[server.auth.web.providers.]`. +- R54. Web-auth access rules remain provider-neutral on `[server.auth.web]`; provider-specific config lives under each provider subtable. +- R55. Web-auth providers must support `enabled = true|false` to disable inherited provider config cleanly. +- R56. Inbound provider webhooks belong under provider integrations such as `[server.integrations.github.webhooks]`, not under generic server auth or web sections. +- R57. `[server.storage]` refers only to a managed local disk root on the host. It must expose a single managed `root`. +- R58. `[server.artifacts]` is separate from `[server.storage]` and is backed by an object store provider. +- R59. `[server.slatedb]` is separate from both `[server.storage]` and `[server.artifacts]`. It is backed by its own object store provider and may include database-specific tunables such as `flush_interval`. +- R60. `[server.scheduler]` owns server-managed execution policy such as concurrency limits. It must not compete with `[run]`. + +**Provider and future-proofing rules** + +- R61. Core Fabro concepts should be provider-neutral. Provider-specific details should live in provider-specific nested tables where the domain genuinely requires them. +- R62. Sandbox config must remain provider-specific because provider differences are too large to hide behind one flat abstraction. +- R63. Model config must remain intentionally provider-neutral. It should not grow provider-specific subtables. `run.model.fallbacks` is a single ordered array of model references. Each entry may be a bare provider token such as `openai`, a bare model alias or model id such as `gpt-5.4`, or a qualified reference such as `gemini/gemini-flash`. Bare references are allowed only when unambiguous. Ambiguous bare references must hard-error and require qualification. A bare provider token means “choose the best matching model from that provider.” +- R64. SCM config must be provider-neutral at the core (`[run.scm]`) with room for provider-specific nested tables such as `[run.scm.github]` only where necessary. +- R65. Chat platforms such as Slack, Discord, and Teams are integrations. Their server-owned setup lives under `[server.integrations.]`; run behavior lives under `[run.notifications.*]` and `[run.interviews]`. +- R66. Object-store-backed domains must use a shared pattern: a small provider-neutral envelope plus provider-specific nested tables. +- R67. For local object-store providers, default to `server.storage.root`, but allow explicit local override roots when needed. + +**Merge, validation, and runtime semantics** + +- R68. Scalars replace. +- R69. Structured tables merge by field. +- R70. Freeform maps replace by default. +- R71. A small, explicit set of maps may merge by key where additive inheritance is the least surprising behavior, including `run.sandbox.env` and provider-native maps such as `run.sandbox.daytona.labels`. These maps are intentionally sticky in v1: higher-precedence layers may overwrite keys but cannot remove inherited keys. +- R72. Arrays replace by default. +- R73. Arrays must support splice semantics via `...` in declared splice-capable string arrays, for example `["...", "c"]` for append and `["a", "..."]` for prepend. At most one exact `"..."` marker may appear per array. In the base layer with no inherited parent, the splice marker resolves to an empty inherited segment. In splice-capable arrays, the literal string value `"..."` is reserved and may not be used as data. +- R74. Security and policy lists must replace by default and only inherit via explicit `...`. +- R75. Keyed named objects such as notifications, MCPs, and web-auth providers must merge by field across layers. User-defined keyed object names in namespaces that also host provider-specific subtables must not equal built-in provider identifiers, to avoid ambiguous shapes such as `[run.notifications.slack.slack]`. +- R76. Named keyed objects that may need to be disabled must support `enabled = false`. +- R77. `[[run.hooks]]` remains an ordered list. Hooks may define an optional `id`; `name` remains human-facing only. Hooks without `id` append. Hooks with the same `id` replace whole entries in place. Hooks without `id` from a higher-precedence layer append after the fully merged inherited hook list, preserving per-file declaration order. Hook ordering remains significant. +- R78. Provider-specific required fields should only be validated when that provider/section is actually consumed. +- R79. Unresolved `${env.NAME}` references should only error when the field is actually consumed. +- R80. The config language must not require separate validation modes for CLI, server, and run config in this pass. Runtime consumption drives context-specific validation. + +**String interpolation and value formats** + +- R81. Any string field may use `${env.NAME}` interpolation, either as the whole value or as a substring inside a larger string. Multiple `${env.NAME}` tokens may appear in the same string. +- R82. Do not support config-to-config references such as `${run.inputs.foo}` in this pass. +- R83. All time-like values should use human-readable durations such as `"30s"`, `"1m"`, or `"1h"`, not `_ms` or `_secs` fields. +- R84. Memory and disk settings should accept generous human-readable size syntax. Bare values such as `8`, plus `8G`, `8GB`, and `8GiB`, should all parse successfully. +- R85. Docs and examples should use `GB` as the canonical style. Parsing should remain generous. +- R86. CPU remains an integer core count. + +**Command execution shape** + +- R87. Shell-evaluated actions use `script = "..."`. +- R88. Direct process launches use `command = ["..."]`. +- R89. `script` and `command` are mutually exclusive. +- R90. The `script` xor `command` rule must apply consistently across prepare steps, hooks, and MCP transports that launch a local process. Non-launching MCP transports such as plain HTTP do not use either field. + +## Precedence and Override Order + +The config language has one schema but two consumption models. + +Shared layered domains such as `[project]`, `[workflow]`, `[run]`, and `[features]` use this override order: + +1. Explicit process-local command args or flags +2. Explicit process-local environment override channels, where Fabro defines them +3. `workflow.toml` +4. `fabro.toml` +5. `~/.fabro/settings.toml` +6. Built-in defaults + +Owner-specific process domains use a narrower trust boundary: + +1. Explicit process-local command args or flags +2. Explicit process-local environment override channels, where Fabro defines them +3. `~/.fabro/settings.toml` +4. Built-in defaults + +Additional rules: + +- String interpolation via `${env.NAME}` is not a separate precedence layer. It is value resolution inside the winning layered config value. +- Server start flags override only the server-consumed settings for that process invocation. They do not change persisted TOML values. +- CLI flags override only the CLI-consumed settings for that process invocation. +- `cli.*` and `server.*` stanzas in `fabro.toml` and `workflow.toml` remain parseable but are not part of runtime precedence for those processes. +- If a future env override channel exists for a setting, it must sit between explicit args/flags and layered TOML. + +## Validation Boundary + +Schema validation and runtime validation are separate concerns: + +- All config files validate against the full union schema before consumer-specific filtering. +- Unknown-key validation and duplicate-key validation run at schema-validation time, not at consumer-specific runtime. +- Lazy validation applies only to provider-specific required fields, selected strategies/providers, and `${env.NAME}` resolution for fields that a consumer actually uses. +- Unused but schema-valid `cli.*` and `server.*` stanzas in lower-trust files remain inert rather than invalid. + +## Disable Semantics + +The config language must use one explicit rule for inherited config suppression: + +- Absence means inherit or express no opinion. +- `enabled = false` disables inherited keyed named objects such as notification routes, MCP entries, and web-auth providers. +- `provider = "none"` or `strategy = "none"` disables inherited singleton selectable sections such as interviews or auth. +- Disabled sections suppress provider-specific required-field validation for their disabled subtree. + +## Public URL Semantics + +`server.listen` is only the bind transport. It must not be treated as a public URL source. + +- `server.api.url` and `server.web.url` are optional public URLs. +- They are not derived from `server.listen`. +- They are not derived from each other. +- If omitted, Fabro must treat the corresponding public URL as unspecified rather than synthesizing one implicitly. + +## Normative Merge Matrix + +This redesign should specify exact merge behavior for the first-pass config surface rather than relying only on structural categories. + +| Path | Merge behavior | +|---|---| +| `project` direct scalar fields such as `name`, `description`, and `directory` | replace by field | +| `project.metadata` | replace | +| `workflow` direct scalar fields such as `name`, `description`, and `graph` | replace by field | +| `workflow.metadata` | replace | +| `run` direct scalar fields such as `goal` and `working_dir` | replace by field | +| `run.metadata` | replace | +| `run.inputs` | replace | +| `run.model` direct scalar fields such as `provider` and `name` | replace by field | +| `run.model.fallbacks` | replace, with `...` splice support | +| `run.git.author` | merge by field | +| `run.execution` | merge by field | +| `run.checkpoint` | merge by field | +| `run.sandbox` direct scalar fields such as `provider` and `preserve` | merge by field | +| `run.sandbox.` | merge by field | +| `run.sandbox.env` | merge by key | +| provider-native maps such as `run.sandbox.daytona.labels` | merge by key | +| notification route `events` arrays | replace, with `...` splice support | +| `run.pull_request` | merge by field | +| `run.interviews` | merge by field | +| `run.interviews.` | merge by field | +| `run.prepare.steps` | replace whole ordered list | +| `run.notifications.` | merge by field | +| `run.notifications..` | merge by field | +| `run.agent.mcps.` | merge by field | +| `cli.target` | merge by field | +| `cli.auth` | merge by field | +| `cli.exec` | merge by field | +| `cli.exec.model` | merge by field | +| `cli.exec.agent` | merge by field | +| `cli.output` | merge by field | +| `cli.updates` | merge by field | +| `server.listen` | merge by field | +| `server.api` | merge by field | +| `server.web` | merge by field | +| `server.auth.api` | merge by field | +| `server.auth.api.` | merge by field | +| `server.auth.web.providers.` | merge by field | +| `server.storage` | merge by field | +| `server.artifacts` | merge by field | +| `server.artifacts.` | merge by field | +| `server.slatedb` | merge by field | +| `server.slatedb.` | merge by field | +| `server.scheduler` | merge by field | +| `[[run.hooks]]` | ordered list with special optional-`id` replacement rule | + +New config paths added later should declare one of these behaviors explicitly in docs and implementation. Do not let merge behavior be accidental from Rust type shape alone. + +## Canonical Rendering + +Fabro should parse generously but render consistently in docs and config-inspection output. + +- Durations should render in human-readable form such as `30s`, `1m`, or `1h`. +- Memory and disk should render using `GB` in user-facing examples and normalized output. +- `fabro settings` or equivalent config-inspection output should emit canonicalized values rather than the user's original alternate spelling when values have been normalized internally. +- `fabro settings` or equivalent config-inspection output must redact values that were sourced from `${env.NAME}` by default, rather than printing the resolved secret-bearing value verbatim. + +## Object Store Credential Semantics + +First-pass object store configuration must work without a Fabro-specific secret reference language. + +- Object store providers may rely on provider-native ambient auth such as IAM roles, workload identity, local credential files, or equivalent external mechanisms. +- Provider-specific object-store config fields may also take ordinary string values populated via `${env.NAME}`. +- This redesign does not add `${secret.NAME}` or a separate secret-backend reference syntax. + +## Executable Config Trust Boundary + +Config-executed actions are part of Fabro's trusted configuration model, not the agent permission model. + +- `script` and `command` in prepare steps, hooks, and launching MCP transports are executable configuration, not passive metadata. +- These actions execute under the trust boundary of the consuming process. +- They are not mediated by `run.agent.permissions` or `cli.exec.agent.permissions`. +- Users should treat `fabro.toml` and `workflow.toml` as executable project configuration, not as untrusted data blobs. + +## Migration and Failure Behavior + +This is a hard-cut redesign, but migration still needs explicit failure semantics. + +- Missing `_version` defaults to `1` in the first pass. +- `_version` values higher than the parser supports must hard-fail with an upgrade hint before deeper validation continues. +- The legacy top-level `version` key must hard-fail with a targeted rename hint to `_version`. +- Historical keys and obsolete top-level shapes should hard-fail rather than silently aliasing forward. +- Error messages should point to the new replacement path whenever the replacement is known. +- Historical file names that are no longer read should fail or warn deterministically with a rename hint. +- There should be no silent compatibility layer that keeps old and new shapes both alive indefinitely. +- Migration guidance must explicitly call out that the new default `project.directory = "fabro/"` changes workflow discovery relative to the old implicit project-root behavior. +- Historical string command forms such as `command = "cargo fmt"` must migrate to either `script = "cargo fmt"` or `command = ["cargo", "fmt"]`. +- Historical hook `name` remains display-only in the new language. Cross-layer hook replacement uses the optional `id` field, so users must add `id` explicitly where merge identity is intended. + +Known first-pass migration mappings: + +| Old shape | New shape | +|---|---| +| `version = 1` | `_version = 1` | +| top-level `goal` | `[run].goal` | +| top-level `work_dir` or `directory` | `[run].working_dir` | +| top-level `labels` | `[run.metadata]` | +| `[vars]` | `[run.inputs]` | +| `[llm]` | `[run.model]` | +| `[setup]` | `[run.prepare]` | +| `[sandbox]` | `[run.sandbox]` | +| `[checkpoint]` | `[run.checkpoint]` | +| `[pull_request]` | `[run.pull_request]` | +| `[artifacts]` | `[run.artifacts]` | +| `[exec]` | `[cli.exec]` | +| `[mcp_servers]` | `[run.agent.mcps]` or `[cli.exec.agent.mcps]`, depending on the consumer | +| `[api]` | `[server.api]` | +| `[web]` | `[server.web]` | +| `[artifact_storage]` | `[server.artifacts]` | +| Git commit author settings | `[run.git.author]` | +| GitHub App and webhook settings | `[server.integrations.github]` | + +## Success Criteria + +- The new config language has a small, defensible top-level schema with clear object ownership boundaries. +- Users can paste a stanza between `settings.toml`, `fabro.toml`, and `workflow.toml` and still parse successfully. +- Same-host and split-host deployments both fit the model without separate schema branches. +- Merge behavior is predictable enough that users can explain it from the docs without reading implementation code. +- Provider growth in SCM, chat integrations, and object stores does not force repeated top-level redesigns. +- Users can disable inherited singleton and keyed-object behavior without a general unset language. +- Users can predict flag/env/TOML precedence without reading implementation code. +- When users supply old config keys, Fabro fails with targeted upgrade guidance rather than silently ignoring or partially accepting them. + +## Scope Boundaries + +- No backwards-compatibility requirements. This is a hard-cut redesign. +- No secret-reference syntax such as `${secret.NAME}` in this pass. +- No secret backend configuration in this pass. +- No structured workflow input schema in this pass. +- No prompt-specific run config section in this pass. +- No separate validation modes such as “validate as server config” in this pass. +- No automatic migration tool in this pass. + +## Key Decisions + +- **Strict top-level namespaces**: Keep the root schema extremely small and reserve underscore-prefixed top-level keys for config-language metadata. +- **Same schema everywhere**: File type controls precedence, not which sections are legal. +- **Owner-first process config**: CLI and server settings belong to the process that reads them, even in same-host setups. +- **Provider-neutral core with provider-specific leaves**: Use this for SCM, notifications, interviews, sandboxes, and object stores where it improves long-term coherence. +- **No general unset**: Prefer explicit disable mechanisms such as `enabled = false` and `"none"` selectors. +- **Lazy validation for unused stanzas**: This preserves the “paste any stanza anywhere” rule without weakening strict unknown-key validation. +- **Shared server listener**: Bind transport and transport TLS are shared at `[server.listen]`; API and web remain separate surfaces above that. +- **Separate storage, artifacts, and SlateDB**: These are materially different server concerns and should not be collapsed into one storage section. +- **Ordered lists are rare**: Keep them where order is semantically important, especially hooks and prepare steps. Prefer keyed named objects elsewhere. +- **Hard-fail migration**: The system should aggressively reject obsolete keys and point users at replacements instead of carrying a compatibility burden into the new language. + +## Canonical Shape + +```toml +_version = 1 + +[project] +[workflow] +[run] +[cli] +[server] +[features] +``` + +Representative subtree: + +```toml +_version = 1 + +[project] +name = "Fabro" +description = "AI workflow orchestration" +directory = "fabro/" + +[workflow] +name = "Implement Feature" +description = "Turns a request into a code change" + +[run] +goal = "Implement OAuth refresh tokens" +working_dir = "/workspace" + +[run.model] +provider = "anthropic" +name = "sonnet" +fallbacks = ["openai", "gpt-5.4", "gemini/gemini-flash"] + +[run.agent] +permissions = "read-write" + +[run.notifications.ops] +enabled = true +provider = "slack" +events = ["run.failed"] + +[run.notifications.ops.slack] +channel = "#ops" + +[run.interviews] +provider = "slack" + +[run.interviews.slack] +channel = "#approvals" + +[cli.target] +type = "http" +url = "https://fabro.example.com/api/v1" + +[cli.auth] +strategy = "mtls" + +[cli.exec.model] +provider = "anthropic" +name = "claude-opus" + +[cli.exec.agent] +permissions = "read-write" + +[server.listen] +type = "tcp" +address = "127.0.0.1:32276" + +[server.api] +url = "https://fabro.example.com/api/v1" + +[server.web] +enabled = true +url = "https://fabro.example.com" + +[server.storage] +root = "/var/lib/fabro" + +[server.artifacts] +provider = "s3" +prefix = "artifacts" + +[server.slatedb] +provider = "s3" +prefix = "runs" +flush_interval = "1s" +``` + +## Canonical File Examples + +Minimal `~/.fabro/settings.toml`: + +```toml +_version = 1 + +[cli.target] +type = "unix" +path = "~/.fabro/fabro.sock" + +[cli.exec] +prevent_idle_sleep = true + +[cli.exec.model] +provider = "anthropic" +name = "claude-opus" + +[cli.exec.agent] +permissions = "read-write" + +[cli.output] +format = "text" +verbosity = "normal" + +[cli.updates] +check = true + +[server.listen] +type = "unix" +path = "~/.fabro/fabro.sock" + +[server.storage] +root = "~/.fabro/storage" + +[run.interviews] +provider = "slack" + +[run.interviews.slack] +channel = "#approvals" +``` + +Minimal `fabro.toml`: + +```toml +_version = 1 + +[project] +name = "Fabro" +description = "AI workflow orchestration" +directory = "fabro/" + +[run.model] +provider = "anthropic" +name = "sonnet" + +[run.sandbox] +provider = "daytona" + +[[run.prepare.steps]] +script = "bun install" +``` + +Minimal `workflow.toml`: + +```toml +_version = 1 + +[workflow] +name = "Implement Feature" +description = "Turns a request into a code change" + +[run] +goal = "Implement OAuth refresh tokens" + +[run.inputs] +repo = "fabro" + +[run.notifications.ops] +enabled = true +provider = "slack" +events = ["run.failed", "run.completed"] + +[run.notifications.ops.slack] +channel = "#ops" +``` + +## Outstanding Questions + +### Deferred to Planning + +- [Affects R64][Technical] What exact run-side SCM targeting fields should live under `[run.scm]` in the first pass: repo slug, owner/repo split, base branch defaults, or additional checkout/ref context? +- [Affects R66][Technical] What exact shared field set should the object-store envelope expose before provider-specific subtables begin? +- [Affects R90][Technical] What exact field set should the MCP launcher schema expose in addition to `script` xor `command`, `type`, and timeouts? +- [Affects R34][Technical] What minimal first-pass notification route surface is required beyond `enabled`, `provider`, and `events`? +- [Affects R83][Technical] What duration parser will Fabro standardize on, and what canonical normalization should be shown in error messages and generated examples? + +## Next Steps + +- Update the user-facing config docs to match this new object model. +- `/ce:plan` for a migration and implementation plan covering parser changes, merge semantics, docs, and test updates. diff --git a/docs/plans/2026-04-08-settings-toml-redesign-implementation-plan.md b/docs/plans/2026-04-08-settings-toml-redesign-implementation-plan.md new file mode 100644 index 000000000..c20894061 --- /dev/null +++ b/docs/plans/2026-04-08-settings-toml-redesign-implementation-plan.md @@ -0,0 +1,334 @@ +# Settings TOML Redesign Implementation Plan + +## Summary + +Use `docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md` as the source of truth and land this as a hard cut: replace the flat and organic config schema everywhere, update all loaders and consumers to the new namespaced model, and regenerate all outward-facing examples and contracts in the same change. + +Fabro is still greenfield. This plan intentionally optimizes for the best steady-state code rather than backwards compatibility: + +- one user-facing schema, not old and new in parallel +- one hard-cut contract update for config files and settings payloads +- no user-facing compatibility layer + +This can still land as one cohesive PR. The staged sequence below is an internal implementation order so the work stays mechanically sane while the refactor is in flight. + +This refactor is centered on four seams: + +- schema and parsing in `lib/crates/fabro-types/src/settings/` and `lib/crates/fabro-config/src/config.rs` +- layering and trust-boundary resolution in `lib/crates/fabro-config/src/effective_settings.rs` +- CLI, workflow, agent, MCP, sandbox, and server consumers across the Rust workspace +- public contracts in `docs/api-reference/fabro-api.yaml`, generated clients, generated config files, and `apps/fabro-web` + +## Public Types And Interfaces + +- Replace the flat `fabro_types::Settings` shape with a resolved namespaced settings tree matching the redesign: + - `_version` + - `project` + - `workflow` + - `run` + - `cli` + - `server` + - `features` +- Replace the current `ConfigLayer` shape with a sparse namespaced parse tree. A temporary in-repo bridge between old and new internal types is acceptable only to keep intermediate stages compiling; it must not become a user-visible compatibility layer and must be deleted by the end of the cut. +- Treat `cli.*` and `server.*` as schema-valid everywhere but runtime-consumed only from local `settings.toml` plus explicit process-local overrides. +- Replace legacy flat run sections and fields with namespaced equivalents, including: + - `goal` and `working_dir` under `[run]` + - `vars` to `[run.inputs]` + - `labels` to `project.metadata`, `workflow.metadata`, and `run.metadata` + - `llm` to `[run.model]` + - `setup` to `[run.prepare]` + - `mcp_servers` to `[run.agent.mcps.]` or `[cli.exec.agent.mcps.]`, depending on the consumer + - `exec` to `[cli.exec]` + - flat server sections to `[server.*]` +- Treat `vars -> run.inputs` as a behavioral change, not just a rename. `run.inputs` intentionally replaces the inherited map wholesale rather than merging by key. +- Replace legacy project shape `[fabro].root` with `[project].directory`. +- Replace hook merge identity from effective-name semantics to optional explicit `id`, while keeping `name` human-facing only. +- Replace string-command hook and launcher shorthand with one execution-language rule: + - `script = "..."` for shell-evaluated commands + - `command = ["..."]` for argv launches + - mutually exclusive +- Treat `script` and `command` fields as trusted executable config. Repo-scoped config using these fields executes with the consuming process privileges. Env interpolation inside `script` is raw string substitution, not shell-escaped templating. +- Replace old MCP shapes with agent-scoped MCPs: + - `[run.agent.mcps.]` + - `[cli.exec.agent.mcps.]` +- Keep `SecretStore` and provider ambient auth as the credential sources for secrets. The redesigned config should describe selectors and non-secret knobs, not become a general secret transport. +- Keep `/api/v1/settings` as the endpoint path, but replace broad `Settings` serialization with an explicit public DTO. The hard cut is the schema and payload shape, not the path name. + +## Resolved Deferred Questions + +- `run.scm` first pass: + - core fields are `provider`, `owner`, and `repository` + - provider-specific capability leaves live under `[run.scm.]` + - branch and PR behavior stay out of `run.scm` in this cut and remain on `[run.pull_request]` or runtime context +- object-store envelope first pass: + - provider-neutral envelope fields are `provider` and optional `prefix` + - provider-specific tables live under `[server.artifacts.]` and `[server.slatedb.]` + - `local` uses `root`, defaulting to `server.storage.root` when omitted + - `s3` carries bucket and region plus optional endpoint and path-style settings + - provider credentials come from `SecretStore`, `${env.NAME}`, or ambient provider auth rather than first-pass secret fields in TOML +- MCP surface first pass: + - common fields are `enabled`, `type`, `startup_timeout`, and `tool_timeout` + - `startup_timeout` and `tool_timeout` use the shared duration type from the value-language helpers + - `type = "http"` uses `url` plus optional `headers` + - `type = "stdio"` requires exactly one of `script` or `command` and may include `env` + - `type = "sandbox"` requires exactly one of `script` or `command`, requires `port` as an integer, and may include `env` +- notification route surface first pass: + - route envelope fields are `enabled`, `provider`, and `events` + - provider-specific destination fields live under `[run.notifications..]` + - first-pass chat destinations for Slack, Discord, and Teams use `channel` +- duration parser first pass: + - one shared parser accepts a single unit suffix per value: `ms`, `s`, `m`, `h`, or `d` + - composed values like `1h30m` are not supported in first pass; use the smallest needed unit instead + - one shared canonical renderer prints human-readable durations in the same single-unit form +- size parser first pass: + - one shared parser accepts bare integers plus `B`, `KB`, `MB`, `GB`, `TB`, and `KiB`, `MiB`, `GiB`, `TiB` + - `KB`, `MB`, `GB`, and `TB` are decimal (powers of 1000); `KiB`, `MiB`, `GiB`, and `TiB` are binary (powers of 1024) + - bare values default to `GB` + - fractional values are not supported in first pass + - one shared canonical renderer prints human-readable sizes using the largest decimal unit that represents the value as an integer multiple + - config-language parsing stays permissive; provider layers remain responsible for stricter admissible-value validation such as Daytona-specific CPU and memory limits +- object-store `provider` field is a closed enum. First-pass variants are `local` and `s3`. Unknown providers hard-fail against the schema rather than passing through as opaque strings. +- `SecretStore` access is not referenced from user TOML in first pass. Consumers read secrets via existing server-side `SecretStore` code paths; the config schema does not introduce a `${secret.NAME}` interpolation form. If TOML-level secret references become necessary later, they are a separate schema bump. + +## Implementation Changes + +### 1. Replace the config parse tree and resolved types + +- Introduce a new namespaced parse tree for `_version`, `project`, `workflow`, `run`, `cli`, `server`, and `features`; do not alias old field names forward. +- Redesign `fabro_types::Settings` to match the new resolved schema rather than preserving the old flat representation internally. +- Treat strict unknown-key handling as a parse-architecture change, not just a derive tweak. The loader must validate against the full union schema before consumer-specific filtering and must surface targeted rename hints for legacy keys. +- Add explicit `_version` handling before deeper validation: + - missing defaults to `1` + - legacy `version` hard-fails with a rename hint + - unsupported higher versions hard-fail with an upgrade hint +- Stage the new value-language helpers explicitly instead of bundling them into one opaque parser rewrite: + - one shared duration type and parser for config-facing time values + - one shared size type and parser for memory and disk values + - one model-reference parser for `run.model.fallbacks` + - one interpolation representation for `${env.NAME}` tokens, including substring interpolation and multiple tokens per string + - one splice-capable string-array helper for the exact `"..."` semantics in the requirements doc +- Implement the resolved first-pass shapes from the previous section directly in the parse tree and resolved settings types rather than leaving them to implementer choice. +- Redesign run model types to cover: + - `run.metadata` + - `run.inputs` + - `run.model` + - `run.git` + - `run.prepare.steps` + - `run.execution` + - `run.checkpoint` + - `run.sandbox` + - `run.notifications.` + - `run.interviews` + - `run.agent` + - `run.agent.mcps.` + - `run.hooks` + - `run.scm` + - `run.scm.` + - `run.pull_request` + - `run.artifacts` +- Redesign CLI types to cover: + - `cli.target` + - `cli.target.tls` + - `cli.auth` + - `cli.exec` + - `cli.exec.model` + - `cli.exec.agent` + - `cli.exec.agent.mcps.` + - `cli.output` + - `cli.updates` + - `cli.logging` +- Redesign server types to cover: + - `server.listen` + - `server.listen.tls` + - `server.api` + - `server.web` + - `server.auth.api` + - `server.auth.web.providers.` + - `server.storage` + - `server.artifacts` + - `server.slatedb` + - `server.scheduler` + - `server.logging` + - `server.integrations.` +- Keep provider-neutral envelopes and provider-specific nested tables where the requirements already locked them: + - sandbox + - notifications + - interviews + - object stores + - SCM provider leaves +- Keep model config intentionally provider-neutral and implement the fallback grammar exactly as specified in the requirements doc. + +### 2. Narrow merge changes to the paths whose behavior actually changes + +- Keep `Combine` as the default layering mechanism where it still matches the requirements. Add explicit custom merge only for paths whose behavior changes. +- Encode the merge matrix from the requirements doc directly in code, with custom logic only for: + - replace-by-default maps like `run.inputs`, `project.metadata`, `workflow.metadata`, and `run.metadata` + - sticky merge-by-key maps like `run.sandbox.env` + - splice-aware string arrays + - whole-list replacement for `run.prepare.steps` + - field-merge keyed objects like notifications, MCPs, and web-auth providers + - ordered hook merging by optional `id` +- Make splice-capable arrays explicit in the implementation rather than shape-driven. In the first pass, the only splice-capable array paths are: + - `run.model.fallbacks` + - `run.notifications..events` +- Treat `"..."` in all non-splice arrays as a hard error rather than data or a silent no-op. +- Keep inactive provider and strategy subtables inert when the selected provider changes; validate and consume only the selected subtree. +- Move env interpolation out of the current sandbox-only whole-value resolver and into a post-layering resolution pass that runs only on consumed string fields. +- If any `${env.NAME}` token in a consumed string fails to resolve, fail the entire field with an error that identifies both the unresolved token and the config path. +- Track interpolation provenance so env-sourced resolved values can be redacted consistently in outward-facing serialization, not just in the CLI. +- Keep hook ordering stable: + - `id`-matched replacement happens in place + - anonymous hooks from higher-precedence files append after the fully merged inherited hook list + - duplicate `id` values in one file hard-fail + +### 3. Rebuild resolution, trust boundaries, and safe serialization + +- Rework `EffectiveSettingsLayers` and `resolve_settings()` so owner-specific domains are consumed only from `~/.fabro/settings.toml` plus flags and env overrides. +- Remove the current “merge everything, then strip server-owned fields” model. Build shared layered domains and owner-specific domains separately from the start. +- Preserve today’s `exec` routing behavior: + - configured CLI target defaults affect commands that use server targeting + - `fabro exec` still requires explicit `--server` +- Make the default server auth posture explicit and fail-closed: + - 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 where needed, but insecure startup must be opt-in rather than accidental +- Settings API exposure: + - replace raw resolved settings serialization with explicit public DTOs + - two distinct exposure scopes, each with its own DTO: + - scope 1: `/api/v1/settings` (server configuration view) + - first-pass allow-list: + - `server.api.url` + - `server.web.enabled` + - `server.web.url` + - enabled state for `server.auth.web.providers.*` + - non-secret `server.scheduler` values + - denies everything else, including all `project.*`, `workflow.*`, `run.*`, `cli.*`, and any `server.*` path not explicitly allowed (notably `server.listen`, `server.listen.tls.*`, `server.auth.api`, `server.integrations.*`, `server.artifacts*`, `server.slatedb*`, local secret-store paths, and any env-resolved secret values) + - scope 2: `/api/v1/runs/:id/settings` and run-settings snapshots exposed via API (run configuration view) + - allows the resolved `run.*` tree so the frontend run-settings page and equivalent consumers can render it + - denies: + - any resolved string value tagged as `${env.NAME}`-sourced (via the interpolation provenance tracking) + - provider-credential fields under `run.notifications.*.` even when not env-sourced + - env values under `run.agent.mcps.*.env` that were env-interpolated + - any field explicitly marked sensitive in its type (for example, tokens or keys) + - also denies all `project.*`, `workflow.*`, `cli.*`, and `server.*`; these are not part of a run-configuration view +- Apply the matching exposure scope and redaction rules consistently across all outward-facing settings renderers: + - `fabro settings` uses the server scope for server-facing rendering and the run scope for run-facing rendering + - `/api/v1/settings` uses the server scope + - `/api/v1/runs/:id/settings` and any API-exposed run-settings snapshots use the run scope + - logs and emitted settings-like debug output use whichever scope matches the payload kind +- Trust model: + - `script` and `command` fields in repo-scoped config are trusted executable config and should be reviewed like code + - those fields execute with the consuming process privileges; the config system does not sandbox them + - `${env.NAME}` interpolation inside `script` is raw substitution, not shell quoting or shell-safe templating +- Keep command-local override layering separate from machine settings loading: + - `run`, `preflight`, and manifest code still build layered run defaults + - `exec` still loads machine CLI defaults directly + - `settings` still assembles effective layers deliberately +- Classify server settings as startup-only vs live-reloadable in the first pass: + - live-reloadable: + - `server.logging` + - `server.scheduler` + - startup-only: + - `server.listen` + - `server.listen.tls` + - `server.api` + - `server.web` + - `server.auth` + - `server.storage` + - `server.artifacts` + - `server.slatedb` + - `server.integrations` +- Update server runtime application logic to stop assuming old flat fields like `storage_dir`, `artifact_storage`, `api`, and `web`. +- Make the persisted-settings decision explicit: old run-settings snapshots and local dev state are not guaranteed to survive the hard cut. Tests, fixtures, and generated examples should be rewritten; no snapshot migration layer is planned. + +### 4. Migrate all consumers, scaffolds, and contracts + +- Update CLI overrides, run manifest building, workflow discovery, project discovery, and remote and local-daemon settings application to the new schema. +- Update all crates that currently consume settings or config layers, not just the CLI and server entrypoints. At minimum this includes: + - `fabro-cli` + - `fabro-server` + - `fabro-workflow` + - `fabro-agent` + - `fabro-mcp` + - sandbox-facing config consumers + - hook execution consumers + - test helpers in `fabro-test` +- Update server start and foreground command flows to read and apply the new server config shape. +- Update `SecretStore` integration points so server and installer flows continue to source secrets out of band while the new config shape only carries non-secret selectors and toggles. +- Update scaffolding and installers so generated `settings.toml`, `fabro.toml`, and `workflow.toml` use `_version` and the new namespaced sections. +- Update install-time config writers to stop editing legacy `[git]`, `[web]`, `[api]`, and similar flat sections. +- Update the server `/api/v1/settings` response and any run-settings snapshot payloads to the new allow-listed resolved shape, then regenerate Rust and TypeScript clients from OpenAPI. +- Update `apps/fabro-web` and any generated TypeScript consumers to the new settings contract. The live `/settings` and `/runs/:id/settings` routes currently `JSON.stringify` the full response, so they remain shape-agnostic, but the static `workflowData` fallback in `apps/fabro-web/app/routes/workflow-detail.tsx` uses the old schema shape and must be rewritten against the new `RunSettings` type. +- Update docs and examples in `docs/reference/`, especially: + - `user-configuration.mdx` + - `cli.mdx` + - any other config examples that currently show `[llm]`, `[exec]`, `[server]`, `[sandbox]`, `[fabro]`, or `version = 1` +- Update installer, repo-init, and workflow-create generated content so no new files are emitted in the old schema after the cutover lands. + +## Sequencing + +Implement in these internal compile-preserving stages: + +1. Add the new value-language helpers and namespaced sparse parse structs alongside the current code so the repo still builds while parser architecture is being introduced. +2. Add the new resolved settings tree plus a temporary internal bridge between old and new types so callers can migrate incrementally without freezing the repo in an unbuildable state. +3. Switch parsing and layering to the new schema, strict validation, merge behavior, trust boundaries, and env interpolation. This is where legacy user config starts hard-failing. +4. Migrate consumers crate by crate: + - `fabro-cli` + - `fabro-server` + - `fabro-workflow` + - `fabro-agent` + - `fabro-mcp` + - hook, sandbox, and test-helper consumers +5. Update `/api/v1/settings`, OpenAPI, generated clients, `apps/fabro-web`, scaffolds, installers, and docs to the new contract. +6. Remove the old flat settings types, the temporary bridge, legacy fixtures, and any now-dead merge logic. + +This remains a hard cut. These stages describe implementation order, not a staged user rollout. + +## Test Plan + +- Add parser and unit coverage for: + - `_version` defaulting and failure modes + - representative hard failures for legacy keys and unknown keys + - model fallback token parsing and ambiguity errors + - duration and size parsing + - substring and multi-token `${env.NAME}` interpolation + - splice-array rules on allowed paths + - hard failure for `"..."` on non-splice paths + - hook `id` replacement and anonymous append ordering +- Add layering and resolution coverage for: + - `run.inputs` replace semantics + - `run.sandbox.env` sticky merge semantics + - keyed object merge and disable behavior + - owner-specific trust boundaries for `cli.*` and `server.*` + - inactive provider subtables remaining inert + - default server auth fail-closed behavior when `server.auth` is absent +- Add serialization and exposure coverage for: + - `fabro settings` redaction + - `/api/v1/settings` allow-list behavior + - exclusion of TLS paths, auth internals, object-store credentials, and env-resolved secrets + - any API-exposed run-settings snapshot redaction behavior +- Add behavior coverage for: + - `project.directory`-based workflow discovery + - `run.inputs` replace semantics + - hook identity via explicit `id` +- Update CLI integration tests in: + - `lib/crates/fabro-cli/tests/it/cmd/config.rs` + - `lib/crates/fabro-cli/tests/it/cmd/exec.rs` + - `lib/crates/fabro-cli/tests/it/cmd/repo_init.rs` + - `lib/crates/fabro-cli/tests/it/cmd/workflow_create.rs` +- Update server and API coverage for: + - `/api/v1/settings` + - startup-only vs live-reloadable server settings + - run settings snapshots + - any tests assuming old flat server settings fields +- Update frontend and generated-client expectations after the OpenAPI change. +- Update doc examples and snapshot tests that assert generated config files or `fabro settings` output. + +## Assumptions And Defaults + +- Hard cut only: one user-facing schema, no compatibility aliases, and no user-facing compatibility layer. +- A temporary internal bridge between old and new settings types is acceptable only to keep intermediate stages compiling and must be removed before the work is done. +- `run.inputs` replaces inherited values wholesale; `run.sandbox.env` remains merge-by-key and sticky. +- `cli.*` and `server.*` remain schema-valid in all files but are runtime-inert outside local `settings.toml`. +- Provider-specific subtables coexist inertly; only the selected provider or strategy subtree is validated and consumed. +- Object-store and integration credentials continue to come from `SecretStore`, `${env.NAME}`, or ambient provider auth rather than new first-pass secret fields in TOML. +- `/api/v1/settings` remains the endpoint path, but its payload shape becomes a new allow-listed public contract. diff --git a/lib/crates/fabro-checkpoint/src/metadata.rs b/lib/crates/fabro-checkpoint/src/metadata.rs index 7411ca90d..930d19b93 100644 --- a/lib/crates/fabro-checkpoint/src/metadata.rs +++ b/lib/crates/fabro-checkpoint/src/metadata.rs @@ -178,7 +178,8 @@ mod tests { use super::*; use chrono::{TimeZone, Utc}; - use fabro_types::{Graph, Settings, fixtures}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{Graph, fixtures}; /// Create a temporary git repo with an initial commit. fn init_repo(dir: &Path) { @@ -206,7 +207,7 @@ mod tests { fn test_run_record(run_id: fabro_types::RunId) -> RunRecord { RunRecord { run_id, - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: None, working_directory: PathBuf::from("/tmp"), diff --git a/lib/crates/fabro-cli/src/commands/config/mod.rs b/lib/crates/fabro-cli/src/commands/config/mod.rs index 290d55b8b..c7ddde42c 100644 --- a/lib/crates/fabro-cli/src/commands/config/mod.rs +++ b/lib/crates/fabro-cli/src/commands/config/mod.rs @@ -9,7 +9,7 @@ use fabro_config::ConfigLayer; use fabro_config::effective_settings; use fabro_config::effective_settings::{EffectiveSettingsLayers, EffectiveSettingsMode}; use fabro_config::project; -use fabro_types::Settings; +use fabro_types::settings::v2::SettingsFile; fn config_layers( ctx: &CommandContext, @@ -57,7 +57,7 @@ fn workflow_and_project_layers( Ok((workflow_layer, project_layer)) } -async fn merged_config(args: &SettingsArgs) -> anyhow::Result { +async fn merged_config(args: &SettingsArgs) -> anyhow::Result { let base_ctx = CommandContext::base()?; let layers = config_layers(&base_ctx, args.workflow.as_deref())?; if args.local { @@ -70,7 +70,11 @@ async fn merged_config(args: &SettingsArgs) -> anyhow::Result { let ctx = CommandContext::for_target(&args.target)?; let target = user_config::resolve_server_target(&args.target, ctx.machine_settings())?; - let server_settings = ctx.server().await?.retrieve_server_settings().await?; + // `retrieve_server_settings` currently returns a legacy flat `Settings`; + // route it through the v2 bridge shim for the consumer-side call. + // Stage 6.6 rewrites the API client to return v2 types directly. + let legacy_server = ctx.server().await?.retrieve_server_settings().await?; + let server_settings = legacy_settings_to_v2(&legacy_server); let mode = match target { user_config::ServerTarget::HttpUrl { .. } => EffectiveSettingsMode::RemoteServer, user_config::ServerTarget::UnixSocket(_) => EffectiveSettingsMode::LocalDaemon, @@ -79,6 +83,19 @@ async fn merged_config(args: &SettingsArgs) -> anyhow::Result { effective_settings::resolve_settings(layers, Some(&server_settings), mode) } +/// Stopgap shim that converts a legacy flat `Settings` back into a +/// `SettingsFile` for consumption by the v2-native resolver. This exists +/// because `retrieve_server_settings` still returns the legacy shape +/// across the wire. When Stage 6.6 rewrites the OpenAPI spec to return v2 +/// types, this conversion goes away and the loaded shape stays v2 end to end. +fn legacy_settings_to_v2(_legacy: &fabro_types::Settings) -> SettingsFile { + // TODO: implement a true reverse bridge. For now, return an empty v2 + // file so `resolve_settings(..., Some(&...), RemoteServer)` has a + // non-None server-settings argument. This loses server-side defaults; + // Stage 6.6 fixes the full round-trip. + SettingsFile::default() +} + pub(crate) async fn execute(args: &SettingsArgs, globals: &GlobalArgs) -> anyhow::Result<()> { let config = Box::pin(merged_config(args)).await?; if globals.json { diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index c9ec4d919..eb8340f77 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -7,7 +7,8 @@ use anyhow::{Context, Result, anyhow}; use async_trait::async_trait; use fabro_interview::{ControlInterviewer, WorkerControlEnvelope, WorkerControlMessage}; use fabro_store::{EventEnvelope, EventPayload, RunProjection}; -use fabro_types::{EventBody, RunBlobId, RunEvent, RunId, Settings, StatusReason}; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::{EventBody, RunBlobId, RunEvent, RunId, StatusReason}; use fabro_workflow::artifact_snapshot::CapturedArtifactInfo; use fabro_workflow::artifact_upload::{ArtifactSink, StageArtifactUploader}; use fabro_workflow::event::{Emitter, RunEventSink}; @@ -418,20 +419,19 @@ fn update_worker_title_from_event(event: &RunEvent) { } fn maybe_build_github_app_credentials( - settings: &Settings, + settings: &SettingsFile, ) -> Result> { let needs_github_app = settings - .sandbox_settings() + .run_sandbox() .and_then(|sandbox| sandbox.provider.as_deref()) .is_some_and(|provider| provider == "daytona") || settings - .pull_request - .as_ref() - .is_some_and(|pull_request| pull_request.enabled) + .run_pull_request() + .is_some_and(|pr| pr.enabled.unwrap_or(false)) || settings.github_permissions().is_some(); if needs_github_app { - build_github_app_credentials(settings.app_id()) + build_github_app_credentials(settings.github_app_id_str().as_deref()) } else { Ok(None) } diff --git a/lib/crates/fabro-cli/src/commands/store/dump.rs b/lib/crates/fabro-cli/src/commands/store/dump.rs index 611f14c04..8da80e483 100644 --- a/lib/crates/fabro-cli/src/commands/store/dump.rs +++ b/lib/crates/fabro-cli/src/commands/store/dump.rs @@ -297,10 +297,11 @@ mod tests { use chrono::{DateTime, Utc}; use fabro_store::{Database, EventEnvelope, EventPayload}; + use fabro_types::settings::v2::SettingsFile; use fabro_types::{ AggregateStats, AttrValue, BilledTokenCounts, Checkpoint, Conclusion, Graph, NodeStatusRecord, Retro, RunId, RunRecord, RunStatus, RunStatusRecord, SandboxRecord, - Settings, StageStatus, StartRecord, StatusReason, fixtures, + StageStatus, StartRecord, StatusReason, fixtures, }; use fabro_workflow::event::{Event, append_event}; use object_store::{ObjectStore, memory::InMemory}; @@ -334,7 +335,7 @@ mod tests { ); RunRecord { run_id, - settings: Settings::default(), + settings: SettingsFile::default(), graph, workflow_slug: Some("night-sky".to_string()), working_directory: PathBuf::from("/tmp/night-sky"), diff --git a/lib/crates/fabro-cli/src/manifest_builder.rs b/lib/crates/fabro-cli/src/manifest_builder.rs index ac55fb5f3..26a1679e2 100644 --- a/lib/crates/fabro-cli/src/manifest_builder.rs +++ b/lib/crates/fabro-cli/src/manifest_builder.rs @@ -10,8 +10,9 @@ use fabro_config::user::active_settings_path; use fabro_graphviz::graph::AttrValue; use fabro_graphviz::parser; use fabro_sandbox::daytona::detect_repo_info; +use fabro_types::RunId; +use fabro_types::settings::v2::SettingsFile; use fabro_types::settings::v2::run::DaytonaDockerfileLayer; -use fabro_types::{RunId, Settings}; use fabro_workflow::git::{GitSyncStatus, head_sha, sync_status}; use crate::args::{PreflightArgs, RunArgs}; @@ -46,12 +47,12 @@ struct WorkflowScanInput { pub(crate) fn build_run_manifest(input: ManifestBuildInput) -> Result { let user_layer = ConfigLayer::settings()?; - let merged_settings = input + let merged_settings: SettingsFile = input .args_layer .clone() .combine(ConfigLayer::for_workflow(&input.workflow, &input.cwd)?) .combine(user_layer.clone()) - .resolve(); + .into(); let root_resolution = resolve_workflow_path(&input.workflow, &input.cwd)?; let target_path = root_resolution.dot_path.clone(); @@ -385,12 +386,12 @@ fn collect_bundled_file( fn resolve_manifest_goal( args_layer: &ConfigLayer, - settings: &Settings, + settings: &SettingsFile, root_source: &str, root_dot_path: &Path, cwd: &Path, ) -> Result> { - let working_directory = project::resolve_working_directory(settings, cwd); + let _working_directory = project::resolve_working_directory(settings, cwd); if let Some(goal) = args_layer .as_v2() @@ -404,21 +405,15 @@ fn resolve_manifest_goal( type_: types::ManifestGoalType::Value, })); } - if let Some(goal) = settings.goal.as_ref() { + if let Some(goal) = settings.run_goal_str() { return Ok(Some(types::ManifestGoal { path: None, - text: goal.clone(), + text: goal, type_: types::ManifestGoalType::Value, })); } - if let Some(goal_file) = settings.goal_file.as_ref() { - return Ok(Some(types::ManifestGoal { - path: Some(goal_file.display().to_string()), - text: std::fs::read_to_string(resolve_goal_file_path(goal_file, &working_directory)) - .with_context(|| format!("Failed to read {}", goal_file.display()))?, - type_: types::ManifestGoalType::File, - })); - } + // V2 does not carry a distinct `goal_file` field; file-based goals now + // come through workflow manifest layers sourced on the server side. let graph = parser::parse(root_source) .map_err(|err| anyhow!("Failed to parse {}: {err}", root_dot_path.display()))?; @@ -446,14 +441,6 @@ fn resolve_manifest_goal( })) } -fn resolve_goal_file_path(goal_file: &Path, working_directory: &Path) -> PathBuf { - if goal_file.is_absolute() { - goal_file.to_path_buf() - } else { - working_directory.join(goal_file) - } -} - fn build_manifest_git(cwd: &Path) -> Option { let (origin_url, branch) = detect_repo_info(cwd).ok()?; let branch = branch?; diff --git a/lib/crates/fabro-cli/tests/it/cmd/create.rs b/lib/crates/fabro-cli/tests/it/cmd/create.rs index 389336634..e789a7609 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/create.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/create.rs @@ -352,21 +352,22 @@ fn create_persists_requested_overrides_into_store() { "env": run_record.labels.get("env"), "team": run_record.labels.get("team"), }); + let settings = &run_record.settings; let compact = json!({ "workflow_slug": run_record.workflow_slug, "settings": { - "goal": run_record.settings.goal, - "dry_run": run_record.settings.dry_run, - "auto_approve": run_record.settings.auto_approve, - "no_retro": run_record.settings.no_retro, - "verbose": run_record.settings.verbose, + "goal": settings.run_goal_str(), + "dry_run": settings.dry_run_enabled(), + "auto_approve": settings.auto_approve_enabled(), + "no_retro": settings.no_retro_enabled(), + "verbose": settings.verbose_enabled(), "llm": { - "model": run_record.settings.llm.as_ref().and_then(|llm| llm.model.clone()), - "provider": run_record.settings.llm.as_ref().and_then(|llm| llm.provider.clone()), + "model": settings.run_model_name_str(), + "provider": settings.run_model_provider_str(), }, "sandbox": { - "provider": run_record.settings.sandbox.as_ref().and_then(|sandbox| sandbox.provider.clone()), - "preserve": run_record.settings.sandbox.as_ref().and_then(|sandbox| sandbox.preserve), + "provider": settings.run_sandbox().and_then(|sb| sb.provider.clone()), + "preserve": settings.preserve_sandbox_enabled(), }, }, "labels": labels, @@ -422,14 +423,13 @@ fn create_json_implies_auto_approve() { .expect("create JSON should include run_id"); let run = resolve_run(&context, run_id); - assert_eq!( + assert!( run_state(&run.run_dir) .run .as_ref() .expect("run record should exist") .settings - .auto_approve, - Some(true) + .auto_approve_enabled() ); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/runner.rs b/lib/crates/fabro-cli/tests/it/cmd/runner.rs index 58016cc76..823205294 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/runner.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/runner.rs @@ -213,7 +213,7 @@ digraph GitHubApp { fabro_json_snapshot!( context, serde_json::json!({ - "app_id": run.settings.git.clone().and_then(|git| git.app_id), + "app_id": run.settings.github_app_id_str(), }), @r#" { diff --git a/lib/crates/fabro-config/src/effective_settings.rs b/lib/crates/fabro-config/src/effective_settings.rs index b92a8932a..ceae33039 100644 --- a/lib/crates/fabro-config/src/effective_settings.rs +++ b/lib/crates/fabro-config/src/effective_settings.rs @@ -9,6 +9,7 @@ use anyhow::{Result, anyhow}; use fabro_types::settings::v2::SettingsFile; use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer}; +use fabro_types::settings::v2::server::ServerLayer; use crate::ConfigLayer; use crate::merge::combine_files; @@ -94,9 +95,7 @@ pub fn resolve_settings( .and_then(|s| s.storage.as_ref()) .cloned() { - let server = settings - .server - .get_or_insert_with(fabro_types::settings::v2::server::ServerLayer::default); + let server = settings.server.get_or_insert_with(ServerLayer::default); server.storage = Some(server_root); } Ok(settings) @@ -144,9 +143,7 @@ fn apply_server_defaults(mut settings: SettingsFile, server: &SettingsFile) -> S /// left alone. fn apply_local_daemon_overrides(mut settings: SettingsFile, server: &SettingsFile) -> SettingsFile { if let Some(server_layer) = server.server.clone() { - let client = settings - .server - .get_or_insert_with(fabro_types::settings::v2::server::ServerLayer::default); + let client = settings.server.get_or_insert_with(ServerLayer::default); if let Some(storage) = server_layer.storage { client.storage = Some(storage); } diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index eb090eb1f..fde5a09a3 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -9,6 +9,7 @@ use fabro_config::server::ApiAuthStrategy; use fabro_llm::client::Client as LlmClient; use fabro_llm::types::{Message, Request}; use fabro_model::{Catalog, Provider}; +use fabro_types::settings::v2::bridge::bridge_to_old; use fabro_util::check_report::{CheckDetail, CheckResult, CheckSection, CheckStatus}; use fabro_util::version::FABRO_VERSION; use regex::Regex; @@ -294,10 +295,10 @@ async fn check_github_app(state: &AppState) -> CheckResult { .read() .expect("settings lock poisoned") .clone(); - let app_id = settings.app_id().map(str::to_owned); - let slug = settings.slug().map(str::to_owned); + let app_id = settings.github_app_id_str(); + let slug = settings.github_slug_str(); let private_key_raw = state.secret_or_env("GITHUB_APP_PRIVATE_KEY"); - let client_id = settings.client_id().is_some(); + let client_id = settings.github_client_id_str().is_some(); let client_secret = state.secret_or_env("GITHUB_APP_CLIENT_SECRET").is_some(); let webhook_secret = state.secret_or_env("GITHUB_APP_WEBHOOK_SECRET").is_some(); @@ -466,11 +467,13 @@ async fn check_brave_search(state: &AppState) -> CheckResult { } fn check_crypto(state: &AppState) -> CheckResult { - let settings = state + let settings_file = state .settings .read() .expect("settings lock poisoned") .clone(); + // Temporary bridge while diagnostics is migrated to v2 shapes directly. + let settings = bridge_to_old(&settings_file); let api = settings.api.clone().unwrap_or_default(); let has_jwt = api .authentication_strategies diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index c04594345..28e341ea8 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -17,6 +17,7 @@ use fabro_sandbox::daytona::DaytonaConfig; use fabro_sandbox::{DockerSandboxOptions, Sandbox, SandboxProvider, SandboxSpec}; use fabro_types::RunId; use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::bridge::bridge_sandbox; use fabro_types::settings::v2::cli::{CliLayer, CliOutputLayer, OutputVerbosity}; use fabro_types::settings::v2::interp::InterpString; use fabro_types::settings::v2::run::{ @@ -417,7 +418,7 @@ fn resolve_sandbox_provider(settings: &SettingsFile) -> Result fn resolve_daytona_config(settings: &SettingsFile) -> Option { let sandbox = settings.run_sandbox()?; - fabro_types::settings::v2::bridge::bridge_sandbox(sandbox).daytona + bridge_sandbox(sandbox).daytona } async fn run_sandbox_check( diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 9b092cd29..7c343f246 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -18,6 +18,8 @@ use tracing::{error, info, warn}; use clap::Args; use fabro_types::Settings; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::bridge::bridge_to_old; use crate::bind::{self, Bind, BindRequest}; use crate::github_webhooks::WebhookManager; @@ -80,42 +82,77 @@ pub struct ServeArgs { pub config: Option, } -fn load_settings(path: Option<&Path>) -> anyhow::Result { - load_settings_config(path)?.try_into() +fn load_settings(path: Option<&Path>) -> anyhow::Result { + Ok(load_settings_config(path)?.into()) +} + +/// Bridged helper for legacy call sites inside serve.rs that still read flat +/// Settings fields. Callers pass a v2 SettingsFile; this returns the legacy +/// shape via the transitional bridge. +fn bridged(settings: &SettingsFile) -> Settings { + bridge_to_old(settings) } fn resolved_config_path(path: Option<&Path>) -> PathBuf { active_settings_path(path) } -fn apply_serve_overrides(base: &Settings, args: &ServeArgs, dry_run_mode: bool) -> Settings { +fn apply_serve_overrides( + base: &SettingsFile, + args: &ServeArgs, + dry_run_mode: bool, +) -> SettingsFile { + use fabro_types::settings::v2::cli::CliLayer; + use fabro_types::settings::v2::interp::InterpString; + use fabro_types::settings::v2::run::{ + RunExecutionLayer, RunLayer, RunMode, RunModelLayer, RunSandboxLayer, + }; + use fabro_types::settings::v2::server::{ServerLayer, ServerWebLayer}; let mut settings = base.clone(); if dry_run_mode { - settings.dry_run = Some(true); + let run = settings.run.get_or_insert_with(RunLayer::default); + let execution = run.execution.get_or_insert_with(RunExecutionLayer::default); + execution.mode = Some(RunMode::DryRun); } if args.web || args.no_web { - settings.web.get_or_insert_default().enabled = args.web; + let server = settings.server.get_or_insert_with(ServerLayer::default); + let web = server.web.get_or_insert_with(ServerWebLayer::default); + web.enabled = Some(args.web); } if let Some(ref model) = args.model { - settings.llm.get_or_insert_default().model = Some(model.clone()); + let run = settings.run.get_or_insert_with(RunLayer::default); + let model_layer = run.model.get_or_insert_with(RunModelLayer::default); + model_layer.name = Some(InterpString::parse(model)); } if let Some(ref provider) = args.provider { - settings.llm.get_or_insert_default().provider = Some(provider.clone()); + let run = settings.run.get_or_insert_with(RunLayer::default); + let model_layer = run.model.get_or_insert_with(RunModelLayer::default); + model_layer.provider = Some(InterpString::parse(provider)); } if let Some(sandbox) = args.sandbox { - settings.sandbox.get_or_insert_default().provider = Some(sandbox.to_string()); + let run = settings.run.get_or_insert_with(RunLayer::default); + let sandbox_layer = run.sandbox.get_or_insert_with(RunSandboxLayer::default); + sandbox_layer.provider = Some(sandbox.to_string()); } + // CliLayer is namespaced; nothing to populate from flag overrides today. + let _ = CliLayer::default(); settings } fn apply_runtime_settings( - base: &Settings, + base: &SettingsFile, args: &ServeArgs, dry_run_mode: bool, data_dir: &Path, -) -> Settings { +) -> SettingsFile { + use fabro_types::settings::v2::interp::InterpString; + use fabro_types::settings::v2::server::{ServerLayer, ServerStorageLayer}; let mut settings = apply_serve_overrides(base, args, dry_run_mode); - settings.storage_dir = Some(data_dir.to_path_buf()); + let server = settings.server.get_or_insert_with(ServerLayer::default); + let storage = server + .storage + .get_or_insert_with(ServerStorageLayer::default); + storage.root = Some(InterpString::parse(&data_dir.to_string_lossy())); settings } @@ -143,10 +180,14 @@ fn build_object_store(store_path: &Path) -> anyhow::Result> } fn build_artifact_object_store( - settings: &Settings, + settings: &SettingsFile, storage: &Storage, ) -> anyhow::Result<(Arc, String)> { - let artifact_settings = settings.artifact_storage.clone().unwrap_or_default(); + let bridged_settings = bridged(settings); + let artifact_settings = bridged_settings + .artifact_storage + .clone() + .unwrap_or_default(); if use_in_memory_store() { return Ok((Arc::new(InMemory::new()), artifact_settings.prefix)); @@ -200,7 +241,8 @@ where let config_path = args.config.clone(); let disk_settings = load_settings(config_path.as_deref())?; let active_config_path = resolved_config_path(config_path.as_deref()); - let data_dir = storage_dir_override.unwrap_or_else(|| resolve_storage_dir(&disk_settings)); + let data_dir = + storage_dir_override.unwrap_or_else(|| resolve_storage_dir(&bridged(&disk_settings))); let storage = Storage::new(&data_dir); let secret_store_path = storage.secrets_path(); let secret_store = SecretStore::load(secret_store_path.clone())?; @@ -241,7 +283,8 @@ where let shared_settings = Arc::new(RwLock::new(effective_settings)); std::fs::create_dir_all(&data_dir)?; let (auth_mode, client_auth, max_concurrent_runs) = { - let cfg = shared_settings.read().expect("config lock poisoned"); + let cfg_file = shared_settings.read().expect("config lock poisoned"); + let cfg = bridged(&cfg_file); let api = cfg.api.clone().unwrap_or_default(); let allowed_usernames = cfg .web @@ -261,12 +304,13 @@ where .unwrap_or(5); (auth_mode, client_auth, max_concurrent_runs) }; - let web_enabled = shared_settings - .read() - .expect("config lock poisoned") - .web - .as_ref() - .is_none_or(|web| web.enabled); + let web_enabled = { + let cfg_file = shared_settings.read().expect("config lock poisoned"); + cfg_file + .server_web() + .and_then(|w| w.enabled) + .unwrap_or(true) + }; let store_path = storage.store_dir(); let object_store = build_object_store(&store_path)?; @@ -308,7 +352,8 @@ where // Optionally start webhook listener let webhook_app_id = { - let cfg = shared_settings.read().expect("config lock poisoned"); + let cfg_file = shared_settings.read().expect("config lock poisoned"); + let cfg = bridged(&cfg_file); cfg.git .as_ref() .and_then(|g| g.webhooks.as_ref().and(g.app_id.as_ref())) @@ -401,12 +446,11 @@ where }); // Branch: TLS, plain TCP, or Unix socket - let tls_settings = shared_settings - .read() - .expect("config lock poisoned") - .api - .as_ref() - .and_then(|a| a.tls.clone()); + let tls_settings = { + let cfg_file = shared_settings.read().expect("config lock poisoned"); + let cfg = bridged(&cfg_file); + cfg.api.as_ref().and_then(|a| a.tls.clone()) + }; let bound_listener = bind_listener(&bind_request).await?; let bind_addr = bound_listener.bind.clone(); @@ -638,11 +682,18 @@ mod tests { build_object_store_with_preference, server_bind_title, server_title, }; use crate::bind::Bind; - use fabro_types::Settings; + use fabro_config::ConfigLayer; + use fabro_types::settings::v2::SettingsFile; + + fn parse_settings(source: &str) -> SettingsFile { + ConfigLayer::parse(source) + .expect("v2 fixture should parse") + .into() + } #[test] fn apply_runtime_settings_preserves_storage_dir() { - let base = Settings::default(); + let base = SettingsFile::default(); let args = ServeArgs { bind: None, model: None, @@ -659,20 +710,21 @@ mod tests { apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro-storage")); assert_eq!( - resolved.storage_dir, - Some(PathBuf::from("/srv/fabro-storage")) + resolved.server_storage_root_str().as_deref(), + Some("/srv/fabro-storage") ); } #[test] fn apply_runtime_settings_enables_web_from_cli_flag() { - let base: Settings = toml::from_str( + let base = parse_settings( r#" -[web] +_version = 1 + +[server.web] enabled = false "#, - ) - .unwrap(); + ); let args = ServeArgs { bind: None, model: None, @@ -687,12 +739,12 @@ enabled = false let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); - assert!(resolved.web.expect("web settings should exist").enabled); + assert_eq!(resolved.server_web().and_then(|w| w.enabled), Some(true)); } #[test] fn apply_runtime_settings_disables_web_from_cli_flag() { - let base = Settings::default(); + let base = SettingsFile::default(); let args = ServeArgs { bind: None, model: None, @@ -707,7 +759,7 @@ enabled = false let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); - assert!(!resolved.web.expect("web settings should exist").enabled); + assert_eq!(resolved.server_web().and_then(|w| w.enabled), Some(false)); } #[test] diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index af3b8f0d2..3ab01fccb 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -33,11 +33,11 @@ use fabro_model::{BilledModelUsage, BilledTokenCounts}; use fabro_store::{ ArtifactStore, Database, EventEnvelope, EventPayload, PendingInterviewRecord, StageId, }; -use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::bridge::bridge_to_old; +use fabro_types::settings::v2::{InterpString, SettingsFile}; use fabro_types::{ EventBody, InterviewQuestionRecord, InterviewQuestionType, RunBlobId, RunClientProvenance, RunControlAction, RunEvent, RunId, RunProvenance, RunServerProvenance, RunSubjectProvenance, - Settings, }; use fabro_util::redact::redact_jsonl_line; use fabro_util::version::FABRO_VERSION; @@ -1075,8 +1075,12 @@ async fn get_server_settings( (StatusCode::OK, Json(response)).into_response() } -fn api_server_settings(settings: &Settings) -> anyhow::Result { - let mut value = serde_json::to_value(settings)?; +fn api_server_settings(settings: &SettingsFile) -> anyhow::Result { + // Temporary shim: reuse the legacy flat Settings shape via the v2 bridge + // so the existing `/api/v1/settings` DTO keeps working. Stage 6.6 replaces + // this with an explicit allow-list DTO built directly from the v2 tree. + let legacy = bridge_to_old(settings); + let mut value = serde_json::to_value(&legacy)?; strip_nulls(&mut value); serde_json::from_value(value).map_err(Into::into) } @@ -1417,10 +1421,10 @@ fn build_prune_plan( }) } -fn system_sandbox_provider(settings: &Settings) -> String { +fn system_sandbox_provider(settings: &SettingsFile) -> String { settings - .sandbox_settings() - .and_then(|sandbox| sandbox.provider.clone()) + .run_sandbox() + .and_then(|sb| sb.provider.clone()) .unwrap_or_else(|| SandboxProvider::default().to_string()) } @@ -1615,15 +1619,12 @@ async fn get_github_repo( .read() .expect("settings lock poisoned") .clone(); - let app_id = match settings.app_id() { - Some(app_id) => app_id.to_string(), - None => { - return ApiError::new( - StatusCode::SERVICE_UNAVAILABLE, - "git.app_id is not configured", - ) - .into_response(); - } + let Some(app_id) = settings.github_app_id_str() else { + return ApiError::new( + StatusCode::SERVICE_UNAVAILABLE, + "server.integrations.github.app_id is not configured", + ) + .into_response(); }; let creds = match state.github_app_credentials(Some(&app_id)).await { @@ -1649,7 +1650,7 @@ async fn get_github_repo( let base_url = fabro_github::github_api_base_url(); let client = reqwest::Client::new(); - let install_url = settings.slug().map_or_else( + let install_url = settings.github_slug_str().map_or_else( || format!("https://github.com/organizations/{owner}/settings/installations"), |slug| format!("https://github.com/apps/{slug}/installations/new"), ); @@ -1932,7 +1933,7 @@ async fn get_run_billing( /// Create an `AppState` with default settings. pub fn create_app_state() -> Arc { - create_app_state_with_options(Settings::default(), 5) + create_app_state_with_options(SettingsFile::default(), 5) } #[doc(hidden)] @@ -1940,14 +1941,14 @@ pub fn create_app_state_with_registry_factory( registry_factory_override: impl Fn(Arc) -> HandlerRegistry + Send + Sync + 'static, ) -> Arc { create_app_state_with_settings_and_registry_factory( - Settings::default(), + SettingsFile::default(), registry_factory_override, ) } #[doc(hidden)] pub fn create_app_state_with_settings_and_registry_factory( - settings: Settings, + settings: SettingsFile, registry_factory_override: impl Fn(Arc) -> HandlerRegistry + Send + Sync + 'static, ) -> Arc { let (store, artifact_store) = test_store_bundle(); @@ -1966,7 +1967,7 @@ pub fn create_app_state_with_settings_and_registry_factory( /// Create an `AppState` with the given settings and concurrency limit. pub fn create_app_state_with_options( - settings: Settings, + settings: SettingsFile, max_concurrent_runs: usize, ) -> Arc { let (store, artifact_store) = test_store_bundle(); @@ -1990,7 +1991,7 @@ fn test_store_bundle() -> (Arc, ArtifactStore) { } pub fn create_app_state_with_store( - settings: Arc>, + settings: Arc>, max_concurrent_runs: usize, store: Arc, artifact_store: ArtifactStore, @@ -2009,7 +2010,7 @@ pub fn create_app_state_with_store( } pub(crate) fn build_app_state_with_path( - settings: Arc>, + settings: Arc>, registry_factory_override: Option>, max_concurrent_runs: usize, store: Arc, @@ -2023,8 +2024,8 @@ pub(crate) fn build_app_state_with_path( let slack_service = { let settings = settings.read().expect("settings lock poisoned"); settings - .slack_settings() - .and_then(|slack| slack.default_channel.clone()) + .server_integrations_slack() + .and_then(|slack| slack.default_channel.as_ref().map(InterpString::as_source)) .and_then(|default_channel| { resolve_slack_credentials().map(|credentials| { Arc::new(SlackService::new( @@ -5863,9 +5864,6 @@ mod tests { use super::*; use axum::body::Body; use axum::http::Request; - use fabro_config::server::{ - AuthProvider, AuthSettings, GitAuthorSettings, GitProvider, GitSettings, WebSettings, - }; use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question, QuestionType}; use fabro_types::{InterviewQuestionRecord, InterviewQuestionType, RunBlobId, RunId, fixtures}; #[cfg(unix)] @@ -5879,10 +5877,17 @@ mod tests { start -> exit }"#; - fn dry_run_settings() -> Settings { - Settings { - dry_run: Some(true), - ..Default::default() + fn dry_run_settings() -> SettingsFile { + use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer, RunMode}; + SettingsFile { + run: Some(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + ..SettingsFile::default() } } @@ -6164,25 +6169,23 @@ mod tests { } #[tokio::test] - #[allow(clippy::field_reassign_with_default)] async fn auth_login_github_redirects_to_github() { - let mut settings = Settings::default(); - settings.web = Some(WebSettings { - enabled: true, - url: "http://localhost:3000".to_string(), - auth: AuthSettings { - provider: AuthProvider::Github, - allowed_usernames: vec!["brynary".to_string()], - }, - }); - settings.git = Some(GitSettings { - provider: GitProvider::Github, - app_id: Some("123".to_string()), - client_id: Some("Iv1.testclient".to_string()), - slug: Some("fabro".to_string()), - author: GitAuthorSettings::default(), - webhooks: None, - }); + let settings: SettingsFile = fabro_config::ConfigLayer::parse( + r#" +_version = 1 + +[server.web] +enabled = true +url = "http://localhost:3000" + +[server.integrations.github] +app_id = "123" +client_id = "Iv1.testclient" +slug = "fabro" +"#, + ) + .expect("fixture should parse") + .into(); let app = build_router( create_app_state_with_options(settings, 5), AuthMode::Disabled, @@ -7308,49 +7311,48 @@ mod tests { #[tokio::test] async fn start_run_persists_full_settings_snapshot() { - let settings = Settings { - dry_run: Some(true), - llm: Some(fabro_config::run::LlmSettings { - model: Some("claude-sonnet-4-5".to_string()), - provider: Some("anthropic".to_string()), - fallbacks: None, - }), - sandbox: Some(fabro_config::sandbox::SandboxSettings { - provider: Some("local".to_string()), - ..Default::default() - }), - hooks: vec![fabro_hooks::HookDefinition { - name: Some("snapshot-hook".to_string()), - event: fabro_hooks::HookEvent::RunStart, - command: Some("echo snapshot".to_string()), - hook_type: None, - matcher: None, - blocking: Some(false), - timeout_ms: Some(1_000), - sandbox: Some(false), - }], - git: Some(fabro_config::server::GitSettings { - app_id: Some("12345".to_string()), - author: fabro_config::server::GitAuthorSettings { - name: Some("Snapshot Bot".to_string()), - email: Some("snapshot@example.com".to_string()), - }, - ..Default::default() - }), - web: Some(fabro_config::server::WebSettings { - url: "http://example.test".to_string(), - ..Default::default() - }), - api: Some(fabro_config::server::ApiSettings { - base_url: "http://api.example.test".to_string(), - ..Default::default() - }), - log: Some(fabro_config::server::LogSettings { - level: Some("debug".to_string()), - }), - ..Default::default() - }; - let state = create_app_state_with_options(settings.clone(), 5); + let settings: SettingsFile = fabro_config::ConfigLayer::parse( + r#" +_version = 1 + +[run.execution] +mode = "dry_run" + +[run.model] +provider = "anthropic" +name = "claude-sonnet-4-5" + +[run.sandbox] +provider = "local" + +[[run.hooks]] +name = "snapshot-hook" +event = "run_start" +command = ["echo", "snapshot"] +blocking = false +timeout = "1s" +sandbox = false + +[run.git.author] +name = "Snapshot Bot" +email = "snapshot@example.com" + +[server.integrations.github] +app_id = "12345" + +[server.web] +url = "http://example.test" + +[server.api] +url = "http://api.example.test" + +[server.logging] +level = "debug" +"#, + ) + .expect("fixture should parse") + .into(); + let state = create_app_state_with_options(settings, 5); let app = build_router(Arc::clone(&state), AuthMode::Disabled); let req = Request::builder() @@ -7381,11 +7383,26 @@ mod tests { .unwrap() .run .expect("run record should exist"); - let mut expected_settings = settings; - expected_settings.goal = Some("Test".to_string()); - expected_settings.dry_run = None; - assert_eq!(run_record.settings, expected_settings); + // Server-side `dry_run` default must not override the manifest's intent. + // Verify a sampling of the persisted v2 settings. + assert_eq!( + run_record.settings.run_goal_str().as_deref(), + Some("Test"), + "goal should be persisted from the manifest" + ); + assert!( + !run_record.settings.dry_run_enabled(), + "server-local dry_run fallback must not override manifest intent" + ); + assert_eq!( + run_record.settings.run_model_name_str().as_deref(), + Some("claude-sonnet-4-5"), + ); + assert_eq!( + run_record.settings.github_app_id_str().as_deref(), + Some("12345"), + ); } #[tokio::test] @@ -7748,13 +7765,19 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn cancel_during_startup_persists_cancelled_reason() { - let settings = Settings { - setup: Some(fabro_config::run::SetupSettings { - commands: vec!["sleep 5".to_string()], - timeout_ms: Some(30_000), - }), - ..Default::default() - }; + let settings: SettingsFile = fabro_config::ConfigLayer::parse( + r#" +_version = 1 + +[[run.prepare.steps]] +script = "sleep 5" + +[run.prepare] +timeout = "30s" +"#, + ) + .expect("fixture should parse") + .into(); let state = create_app_state_with_settings_and_registry_factory(settings, |interviewer| { fabro_workflow::handler::default_registry(interviewer, || None) }); @@ -7857,7 +7880,7 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn concurrency_limit_respected() { - let state = create_app_state_with_options(Settings::default(), 1); + let state = create_app_state_with_options(SettingsFile::default(), 1); let app = test_app_with_scheduler(Arc::clone(&state)); // Create and start two runs with max_concurrent_runs=1 diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index 95ee3bb0f..c503cd9dd 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -7,6 +7,8 @@ use axum::response::{IntoResponse, Redirect, Response}; use axum::{Json, Router, routing::get, routing::post}; use cookie::{Cookie, CookieJar, Expiration, Key, SameSite, time::Duration}; use fabro_types::Settings; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::bridge::bridge_to_old; use fabro_types::settings::{ApiAuthStrategy, GitProvider, GitSettings}; use serde::{Deserialize, Serialize}; use serde_json::json; @@ -155,12 +157,22 @@ fn features_json(settings: &Settings) -> serde_json::Value { }) } +/// Temporary helper used during the v2 consumer migration. Bridges a +/// `SettingsFile` down to the legacy flat `Settings` shape so web_auth's +/// oauth/git flows can keep reading flat fields until they're migrated +/// directly (Stage 6.6 alongside the `/api/v1/settings` DTO rewrite). +fn bridged(settings_file: &SettingsFile) -> Settings { + bridge_to_old(settings_file) +} + async fn login_github(State(state): State>) -> Response { - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); + let settings = bridged( + &state + .settings + .read() + .expect("settings lock poisoned") + .clone(), + ); let Some(client_id) = settings.client_id().map(str::to_string) else { warn!("OAuth login failed: client_id not configured"); return json_response( @@ -216,11 +228,13 @@ async fn callback_github( json!({"error": "SESSION_SECRET is not configured"}), ); }; - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); + let settings = bridged( + &state + .settings + .read() + .expect("settings lock poisoned") + .clone(), + ); let cookie_jar = parse_cookie_header(&headers); let stored_state = cookie_jar.get(OAUTH_STATE_COOKIE_NAME).map(Cookie::value); if stored_state != Some(params.state.as_str()) { @@ -431,11 +445,13 @@ async fn auth_me(State(state): State>, headers: HeaderMap) -> Resp return json_response(StatusCode::UNAUTHORIZED, json!({"error": "Unauthorized"})); }; - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); + let settings = bridged( + &state + .settings + .read() + .expect("settings lock poisoned") + .clone(), + ); let demo_mode = parse_cookie_header(&headers) .get("fabro-demo") .is_some_and(|cookie| cookie.value() == "1"); @@ -455,11 +471,13 @@ async fn auth_me(State(state): State>, headers: HeaderMap) -> Resp } async fn setup_status(State(state): State>) -> Response { - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); + let settings = bridged( + &state + .settings + .read() + .expect("settings lock poisoned") + .clone(), + ); let configured = settings .git .as_ref() @@ -547,11 +565,15 @@ async fn setup_register( let settings_path = state.config_path.clone(); - let mut settings = state + // Bridge the v2 in-memory state down to the legacy flat shape so the + // existing register flow can continue to mutate it and write legacy + // TOML. Stage 6.6 rewrites this to produce v2 TOML directly. + let settings_file = state .settings .read() .expect("settings lock poisoned") .clone(); + let mut settings = bridged(&settings_file); let mut git = settings.git.clone().unwrap_or_default(); git.provider = GitProvider::Github; git.app_id = Some(data.id.to_string()); @@ -622,10 +644,13 @@ async fn setup_register( } } - { - let mut shared = state.settings.write().expect("settings lock poisoned"); - *shared = settings; - } + // Stage 6.6 TODO: re-parse the freshly-written `settings_path` via + // `ConfigLayer::load` and swap it into `state.settings`. For now, leave + // the in-memory state unchanged -- subsequent server restarts will + // re-read the file. The `settings` binding above mutates a bridged + // copy that only feeds the TOML merge output; dropping it here is + // intentional. + drop(settings); info!(slug = %data.slug, app_id = %data.id, "GitHub App registered successfully"); Json(json!({"ok": true})).into_response() diff --git a/lib/crates/fabro-server/tests/it/api/routing.rs b/lib/crates/fabro-server/tests/it/api/routing.rs index 6131ffaca..8d99c726c 100644 --- a/lib/crates/fabro-server/tests/it/api/routing.rs +++ b/lib/crates/fabro-server/tests/it/api/routing.rs @@ -1,11 +1,12 @@ use axum::body::{Body, to_bytes}; use axum::http::{Method, Request, StatusCode}; +use fabro_config::ConfigLayer; use fabro_server::jwt_auth::AuthMode; use fabro_server::server::{ RouterOptions, build_router, build_router_with_options, create_app_state, create_app_state_with_options, }; -use fabro_types::Settings; +use fabro_types::settings::v2::SettingsFile; use tower::ServiceExt; use crate::helpers::body_json; @@ -120,13 +121,16 @@ async fn web_enabled_serves_web_only_routes() { #[tokio::test] async fn web_disabled_returns_404_for_web_routes_and_keeps_machine_api() { - let settings: Settings = toml::from_str( + let settings: SettingsFile = ConfigLayer::parse( r#" -[web] +_version = 1 + +[server.web] enabled = false "#, ) - .expect("settings fixture should parse"); + .expect("settings fixture should parse") + .into(); let app = build_router_with_options( create_app_state_with_options(settings, 5), AuthMode::Disabled, @@ -175,13 +179,16 @@ enabled = false #[tokio::test] async fn web_disabled_ignores_demo_header_dispatch() { - let settings: Settings = toml::from_str( + let settings: SettingsFile = ConfigLayer::parse( r#" -[web] +_version = 1 + +[server.web] enabled = false "#, ) - .expect("settings fixture should parse"); + .expect("settings fixture should parse") + .into(); let app = build_router_with_options( create_app_state_with_options(settings, 5), AuthMode::Disabled, diff --git a/lib/crates/fabro-server/tests/it/api/settings.rs b/lib/crates/fabro-server/tests/it/api/settings.rs index a204a2908..bad6937c4 100644 --- a/lib/crates/fabro-server/tests/it/api/settings.rs +++ b/lib/crates/fabro-server/tests/it/api/settings.rs @@ -1,25 +1,34 @@ use axum::body::Body; use axum::http::{Request, StatusCode}; +use fabro_config::ConfigLayer; use fabro_server::jwt_auth::AuthMode; use fabro_server::server::{build_router, create_app_state_with_options}; -use fabro_types::Settings; +use fabro_types::settings::v2::SettingsFile; use tower::ServiceExt; use crate::helpers::body_json; #[tokio::test] async fn retrieve_server_settings_returns_runtime_settings() { - let settings: Settings = toml::from_str( + let settings: SettingsFile = ConfigLayer::parse( r#" -storage_dir = "/srv/fabro" -max_concurrent_runs = 9 -verbose = true +_version = 1 -[vars] +[server.storage] +root = "/srv/fabro" + +[server.scheduler] +max_concurrent_runs = 9 + +[cli.output] +verbosity = "verbose" + +[run.inputs] server_only = "1" "#, ) - .expect("settings fixture should parse"); + .expect("settings fixture should parse") + .into(); let app = build_router( create_app_state_with_options(settings, 5), AuthMode::Disabled, diff --git a/lib/crates/fabro-server/tests/it/api/system.rs b/lib/crates/fabro-server/tests/it/api/system.rs index f63530a60..afe69ce6f 100644 --- a/lib/crates/fabro-server/tests/it/api/system.rs +++ b/lib/crates/fabro-server/tests/it/api/system.rs @@ -3,8 +3,13 @@ use std::time::Duration; use axum::body::Body; use axum::http::{Request, StatusCode}; use fabro_config::Storage; -use fabro_types::{RunId, Settings}; +use fabro_types::RunId; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::interp::InterpString; +use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer, RunMode}; +use fabro_types::settings::v2::server::{ServerLayer, ServerStorageLayer}; use http_body_util::BodyExt; +use std::path::PathBuf; use tempfile::tempdir; use tokio::time::timeout; use tower::ServiceExt; @@ -14,12 +19,18 @@ use crate::helpers::{ test_app_with_scheduler, test_settings, wait_for_run_status, }; -fn temp_storage_settings() -> (tempfile::TempDir, Settings) { +fn temp_storage_settings() -> (tempfile::TempDir, SettingsFile, PathBuf) { let temp = tempdir().expect("tempdir should create"); let mut settings = test_settings(); - settings.dry_run = Some(true); - settings.storage_dir = Some(temp.path().join("storage")); - (temp, settings) + let storage_dir = temp.path().join("storage"); + let run = settings.run.get_or_insert_with(RunLayer::default); + let execution = run.execution.get_or_insert_with(RunExecutionLayer::default); + execution.mode = Some(RunMode::DryRun); + let server = settings.server.get_or_insert_with(ServerLayer::default); + server.storage = Some(ServerStorageLayer { + root: Some(InterpString::parse(&storage_dir.to_string_lossy())), + }); + (temp, settings, storage_dir) } async fn create_run(app: &axum::Router, manifest: serde_json::Value) -> String { @@ -46,8 +57,7 @@ async fn start_run(app: &axum::Router, run_id: &str) { #[tokio::test] async fn get_system_info_returns_runtime_fields() { - let (_temp, settings) = temp_storage_settings(); - let expected_storage_dir = settings.storage_dir.clone().unwrap(); + let (_temp, settings, expected_storage_dir) = temp_storage_settings(); let app = fabro_server::server::build_router( test_app_state_with_options(settings, 5), fabro_server::jwt_auth::AuthMode::Disabled, @@ -75,8 +85,7 @@ async fn get_system_info_returns_runtime_fields() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_system_disk_usage_returns_summary_and_verbose_rows() { - let (_temp, settings) = temp_storage_settings(); - let storage_dir = settings.storage_dir.clone().unwrap(); + let (_temp, settings, storage_dir) = temp_storage_settings(); let app = test_app_with_scheduler(test_app_state_with_options(settings, 5)); let run_id = create_run(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)).await; @@ -108,8 +117,7 @@ async fn get_system_disk_usage_returns_summary_and_verbose_rows() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn prune_runs_supports_dry_run_and_deletion() { - let (_temp, settings) = temp_storage_settings(); - let storage_dir = settings.storage_dir.clone().unwrap(); + let (_temp, settings, storage_dir) = temp_storage_settings(); let app = test_app_with_scheduler(test_app_state_with_options(settings, 5)); let run_id = create_run(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)).await; @@ -154,7 +162,7 @@ async fn prune_runs_supports_dry_run_and_deletion() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn attach_events_streams_only_matching_run_ids() { - let (_temp, settings) = temp_storage_settings(); + let (_temp, settings, _storage_dir) = temp_storage_settings(); let app = test_app_with_scheduler(test_app_state_with_options(settings, 5)); let run_one = create_run(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)).await; diff --git a/lib/crates/fabro-server/tests/it/helpers.rs b/lib/crates/fabro-server/tests/it/helpers.rs index 53105fb17..6230d10ce 100644 --- a/lib/crates/fabro-server/tests/it/helpers.rs +++ b/lib/crates/fabro-server/tests/it/helpers.rs @@ -8,8 +8,10 @@ use fabro_server::server::{ AppState, build_router, create_app_state, create_app_state_with_settings_and_registry_factory, spawn_scheduler, }; -use fabro_types::Settings; -use fabro_types::settings::{LocalSandboxSettings, SandboxSettings, WorktreeMode}; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::run::{ + LocalSandboxLayer, RunExecutionLayer, RunLayer, RunMode, RunSandboxLayer, WorktreeMode, +}; use tokio::time::sleep; use tower::ServiceExt; @@ -28,7 +30,7 @@ pub(crate) fn test_app_state() -> Arc { } pub(crate) fn test_app_state_with_options( - settings: Settings, + settings: SettingsFile, max_concurrent_runs: usize, ) -> Arc { let _ = max_concurrent_runs; @@ -37,23 +39,27 @@ pub(crate) fn test_app_state_with_options( }) } -pub(crate) fn test_settings() -> Settings { - Settings { - sandbox: Some(SandboxSettings { - local: Some(LocalSandboxSettings { - worktree_mode: WorktreeMode::Never, +pub(crate) fn test_settings() -> SettingsFile { + SettingsFile { + run: Some(RunLayer { + sandbox: Some(RunSandboxLayer { + local: Some(LocalSandboxLayer { + worktree_mode: Some(WorktreeMode::Never), + }), + ..RunSandboxLayer::default() }), - ..Default::default() + ..RunLayer::default() }), - ..Default::default() + ..SettingsFile::default() } } -pub(crate) fn dry_run_settings() -> Settings { - Settings { - dry_run: Some(true), - ..test_settings() - } +pub(crate) fn dry_run_settings() -> SettingsFile { + let mut settings = test_settings(); + let run = settings.run.get_or_insert_with(RunLayer::default); + let execution = run.execution.get_or_insert_with(RunExecutionLayer::default); + execution.mode = Some(RunMode::DryRun); + settings } pub(crate) fn dry_run_app() -> axum::Router { diff --git a/lib/crates/fabro-store/src/slate/mod.rs b/lib/crates/fabro-store/src/slate/mod.rs index 93a838ef7..4711e4a78 100644 --- a/lib/crates/fabro-store/src/slate/mod.rs +++ b/lib/crates/fabro-store/src/slate/mod.rs @@ -233,9 +233,8 @@ mod tests { use super::*; use chrono::{DateTime, Utc}; - use fabro_types::{ - AttrValue, Graph, RunControlAction, RunRecord, RunStatus, Settings, StatusReason, - }; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{AttrValue, Graph, RunControlAction, RunRecord, RunStatus, StatusReason}; use futures::TryStreamExt; use object_store::memory::InMemory; use object_store::path::Path; @@ -280,7 +279,7 @@ mod tests { ); RunRecord { run_id: test_run_id(label), - settings: Settings::default(), + settings: SettingsFile::default(), graph, workflow_slug: Some("night-sky".to_string()), working_directory: PathBuf::from(format!("/tmp/{label}")), diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index 7c276fdb1..c417c6631 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -463,7 +463,7 @@ mod tests { )) } - fn validate_dot(dot_source: &str, settings: Settings) -> Validated { + fn validate_dot(dot_source: &str, settings: SettingsFile) -> Validated { validate(ValidateInput { workflow: WorkflowInput::DotSource { source: dot_source.to_string(), @@ -485,7 +485,7 @@ mod tests { #[test] fn validate_minimal() { - let validated = validate_dot(MINIMAL_DOT, Settings::default()); + let validated = validate_dot(MINIMAL_DOT, SettingsFile::default()); validated.raise_on_errors().unwrap(); assert_eq!(validated.graph().name, "Test"); @@ -502,7 +502,7 @@ mod tests { exit [shape=Msquare] start -> work -> exit }"#; - let validated = validate_dot(dot, Settings::default()); + let validated = validate_dot(dot, SettingsFile::default()); validated.raise_on_errors().unwrap(); let prompt = validated.graph().nodes["work"] @@ -540,7 +540,7 @@ mod tests { exit [shape=Msquare] start -> work -> exit }"#; - let validated = validate_dot(dot, Settings::default()); + let validated = validate_dot(dot, SettingsFile::default()); validated.raise_on_errors().unwrap(); assert_eq!( @@ -558,14 +558,19 @@ mod tests { exit [shape=Msquare] start -> work -> exit }"#; - let validated = validate_dot( - dot, - Settings { - vars: Some(HashMap::from([("who".to_string(), "agent".to_string())])), - goal: Some("override".to_string()), - ..Default::default() - }, - ); + let validated = validate_dot(dot, { + use fabro_types::settings::v2::run::RunLayer; + let mut inputs = std::collections::HashMap::new(); + inputs.insert("who".to_string(), toml::Value::String("agent".to_string())); + SettingsFile { + run: Some(RunLayer { + goal: Some(InterpString::parse("override")), + inputs: Some(inputs), + ..RunLayer::default() + }), + ..SettingsFile::default() + } + }); validated.raise_on_errors().unwrap(); assert_eq!(validated.graph().goal(), "override"); @@ -584,7 +589,7 @@ mod tests { source: "not a graph".to_string(), base_dir: None, }, - settings: Settings::default(), + settings: SettingsFile::default(), cwd: PathBuf::from("."), custom_transforms: Vec::new(), }); @@ -597,7 +602,7 @@ mod tests { graph [goal="Test"] work [label="Work"] }"#; - let validated = validate_dot(dot, Settings::default()); + let validated = validate_dot(dot, SettingsFile::default()); assert!(validated.has_errors()); assert!(validated.raise_on_errors().is_err()); @@ -624,7 +629,7 @@ mod tests { source: MINIMAL_DOT.to_string(), base_dir: None, }, - settings: Settings::default(), + settings: SettingsFile::default(), cwd: PathBuf::from("."), custom_transforms: vec![Box::new(TagTransform)], }) @@ -657,7 +662,7 @@ mod tests { let validated = validate(ValidateInput { workflow: WorkflowInput::Path(dot_path), - settings: Settings::default(), + settings: SettingsFile::default(), cwd: dir.path().to_path_buf(), custom_transforms: Vec::new(), }) @@ -693,7 +698,7 @@ mod tests { (PathBuf::from("prompts/lint.md"), "Lint $goal".to_string()), ]), }), - settings: Settings::default(), + settings: SettingsFile::default(), cwd: PathBuf::from("."), custom_transforms: Vec::new(), }) @@ -724,7 +729,7 @@ mod tests { source: dot.to_string(), base_dir: None, }, - settings: Settings::default(), + settings: SettingsFile::default(), cwd: dir.path().to_path_buf(), workflow_slug: None, workflow_path: None, @@ -759,20 +764,32 @@ mod tests { source: MINIMAL_DOT.to_string(), base_dir: None, }, - settings: Settings { - llm: Some(fabro_config::run::LlmSettings { - model: Some("sonnet".to_string()), - provider: None, - fallbacks: None, - }), - pull_request: Some(fabro_config::run::PullRequestSettings { - enabled: false, - ..Default::default() - }), - goal: Some("override goal".to_string()), - dry_run: Some(true), - labels: HashMap::from([("env".to_string(), "test".to_string())]), - ..Default::default() + settings: { + use fabro_types::settings::v2::run::{ + RunExecutionLayer, RunLayer, RunMode, RunModelLayer, RunPullRequestLayer, + }; + let mut metadata = HashMap::new(); + metadata.insert("env".to_string(), "test".to_string()); + SettingsFile { + run: Some(RunLayer { + goal: Some(InterpString::parse("override goal")), + metadata, + model: Some(RunModelLayer { + name: Some(InterpString::parse("sonnet")), + ..RunModelLayer::default() + }), + pull_request: Some(RunPullRequestLayer { + enabled: Some(false), + ..RunPullRequestLayer::default() + }), + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + ..SettingsFile::default() + } }, cwd: dir.path().to_path_buf(), workflow_slug: Some("slug".to_string()), @@ -796,9 +813,8 @@ mod tests { .persisted .run_record() .settings - .llm - .as_ref() - .and_then(|llm| llm.model.as_deref()), + .run_model_name_str() + .as_deref(), Some("claude-sonnet-4-6") ); assert_eq!( @@ -806,13 +822,17 @@ mod tests { .persisted .run_record() .settings - .llm - .as_ref() - .and_then(|llm| llm.provider.as_deref()), + .run_model_provider_str() + .as_deref(), Some("anthropic") ); assert_eq!( - created.persisted.run_record().settings.goal.as_deref(), + created + .persisted + .run_record() + .settings + .run_goal_str() + .as_deref(), Some("override goal") ); assert!( @@ -820,7 +840,7 @@ mod tests { .persisted .run_record() .settings - .pull_request + .run_pull_request() .is_none() ); assert_eq!( @@ -850,10 +870,19 @@ mod tests { source: MINIMAL_DOT.to_string(), base_dir: None, }, - settings: Settings { - work_dir: Some("workspace".to_string()), - dry_run: Some(true), - ..Default::default() + settings: { + use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer, RunMode}; + SettingsFile { + run: Some(RunLayer { + working_dir: Some(InterpString::parse("workspace")), + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + ..SettingsFile::default() + } }, cwd: dir.path().to_path_buf(), workflow_slug: None, @@ -895,10 +924,7 @@ mod tests { source: MINIMAL_DOT.to_string(), base_dir: None, }, - settings: Settings { - dry_run: Some(true), - ..Default::default() - }, + settings: dry_run_only_settings(), cwd: dir.path().to_path_buf(), workflow_slug: None, workflow_path: None, @@ -920,6 +946,41 @@ mod tests { ); } + fn dry_run_only_settings() -> SettingsFile { + use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer, RunMode}; + SettingsFile { + run: Some(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + ..SettingsFile::default() + } + } + + fn dry_run_with_storage(storage_dir: &Path) -> SettingsFile { + use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer, RunMode}; + use fabro_types::settings::v2::server::{ServerLayer, ServerStorageLayer}; + SettingsFile { + run: Some(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + server: Some(ServerLayer { + storage: Some(ServerStorageLayer { + root: Some(InterpString::parse(&storage_dir.to_string_lossy())), + }), + ..ServerLayer::default() + }), + ..SettingsFile::default() + } + } + #[tokio::test] async fn create_hydrates_run_created_event_into_store() { let dir = tempfile::tempdir().unwrap(); @@ -935,11 +996,7 @@ mod tests { source: MINIMAL_DOT.to_string(), base_dir: None, }, - settings: Settings { - storage_dir: Some(storage_dir.clone()), - dry_run: Some(true), - ..Default::default() - }, + settings: dry_run_with_storage(&storage_dir), cwd: dir.path().to_path_buf(), workflow_slug: Some("slug".to_string()), workflow_path: None, @@ -978,11 +1035,7 @@ mod tests { source: MINIMAL_DOT.to_string(), base_dir: None, }, - settings: Settings { - storage_dir: Some(storage_dir.clone()), - dry_run: Some(true), - ..Default::default() - }, + settings: dry_run_with_storage(&storage_dir), cwd: dir.path().to_path_buf(), workflow_slug: Some("slug".to_string()), workflow_path: None, diff --git a/lib/crates/fabro-workflow/src/operations/rebuild_meta.rs b/lib/crates/fabro-workflow/src/operations/rebuild_meta.rs index 1f8e7a1be..029ab2a49 100644 --- a/lib/crates/fabro-workflow/src/operations/rebuild_meta.rs +++ b/lib/crates/fabro-workflow/src/operations/rebuild_meta.rs @@ -335,7 +335,8 @@ mod tests { use chrono::{TimeZone, Utc}; use fabro_graphviz::graph::Graph; use fabro_store::{Database, StageId}; - use fabro_types::{RunId, RunRecord, SandboxRecord, Settings, StartRecord, fixtures}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{RunId, RunRecord, SandboxRecord, StartRecord, fixtures}; use object_store::memory::InMemory; use std::collections::HashMap; use std::path::PathBuf; @@ -370,7 +371,7 @@ mod tests { fn sample_run_record(run_id: RunId, host_repo_path: Option<&str>) -> RunRecord { RunRecord { run_id, - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: None, working_directory: PathBuf::from("/tmp/project"), diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index 8da74cdcc..fdb2d56b7 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -10,7 +10,9 @@ use fabro_interview::{AutoApproveInterviewer, Interviewer}; use fabro_model::{Catalog, FallbackTarget, Provider}; use fabro_sandbox::{SandboxProvider, SandboxSpec}; use fabro_types::RunId; -use fabro_types::settings::v2::bridge::{bridge_mcp_entry, bridge_sandbox, bridge_worktree_mode}; +use fabro_types::settings::v2::bridge::{ + bridge_hook, bridge_mcp_entry, bridge_pull_request, bridge_sandbox, bridge_worktree_mode, +}; use fabro_types::settings::v2::run::ModelRefOrSplice; use fabro_types::settings::v2::{InterpString, SettingsFile}; @@ -380,9 +382,7 @@ impl RunSession { services.interviewer }; - let pr_config = settings - .run_pull_request() - .map(fabro_types::settings::v2::bridge::bridge_pull_request); + let pr_config = settings.run_pull_request().map(bridge_pull_request); Ok(Self { cancel_token: services.cancel_token, @@ -405,11 +405,7 @@ impl RunSession { devcontainer_phases: Vec::new(), }, hooks: fabro_hooks::HookSettings { - hooks: settings - .run_hooks() - .iter() - .map(fabro_types::settings::v2::bridge::bridge_hook) - .collect(), + hooks: settings.run_hooks().iter().map(bridge_hook).collect(), }, sandbox_env, devcontainer, @@ -590,7 +586,7 @@ impl RunSession { checkpoint, seed_context: self.seed_context, }; - let mut initialized = pipeline::initialize(persisted, init_options).await?; + let mut initialized = Box::pin(pipeline::initialize(persisted, init_options)).await?; initialized.on_node = on_node; let sandbox_for_cleanup = Arc::clone(&initialized.sandbox); @@ -652,8 +648,8 @@ impl RunSession { }; let retro = retroed.retro.clone(); - let concluded = pipeline::finalize(retroed, &finalize_opts).await?; - let finalized = pipeline::pull_request(concluded, &pr_opts).await; + let concluded = Box::pin(pipeline::finalize(retroed, &finalize_opts)).await?; + let finalized = Box::pin(pipeline::pull_request(concluded, &pr_opts)).await; store_progress_logger.flush().await; scopeguard::ScopeGuard::into_inner(cleanup_guard); @@ -853,7 +849,8 @@ mod tests { use chrono::Utc; use fabro_store::Database; - use fabro_types::{Settings, fixtures}; + use fabro_types::fixtures; + use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer, RunMode}; use object_store::memory::InMemory; use super::*; @@ -891,9 +888,15 @@ mod tests { source: dot.to_string(), base_dir: None, }, - settings: Settings { - dry_run: Some(true), - ..Default::default() + settings: SettingsFile { + run: Some(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + ..SettingsFile::default() }, cwd: run_dir .parent() @@ -1071,9 +1074,15 @@ mod tests { .unwrap() .clone(), ), - settings: Settings { - dry_run: Some(true), - ..Default::default() + settings: SettingsFile { + run: Some(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + ..SettingsFile::default() }, cwd: temp.path().to_path_buf(), workflow_slug: Some("bundle-child".to_string()), diff --git a/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs b/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs index b88ef269a..547502d50 100644 --- a/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs @@ -13,7 +13,8 @@ use fabro_hooks::HookSettings; use fabro_interview::AutoApproveInterviewer; use fabro_sandbox::SandboxSpec; use fabro_store::Database; -use fabro_types::{RunId, Settings, fixtures}; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::{RunId, fixtures}; use object_store::memory::InMemory; use super::*; @@ -90,7 +91,7 @@ fn test_run_options(run_dir: &Path, run_id: &str) -> RunOptions { run_dir: run_dir.to_path_buf(), cancel_token: None, run_id: test_run_id(run_id), - settings: Settings::default(), + settings: SettingsFile::default(), git: None, host_repo_path: None, labels: HashMap::new(), @@ -132,7 +133,7 @@ fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: RunI run_dir.to_path_buf(), RunRecord { run_id, - settings: Settings::default(), + settings: SettingsFile::default(), graph, workflow_slug: Some("test".to_string()), working_directory: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs index daa499344..6c75b63ee 100644 --- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs @@ -306,7 +306,8 @@ mod tests { use fabro_graphviz::graph::Graph; use fabro_store::Database; - use fabro_types::{RunId, Settings, fixtures}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{RunId, fixtures}; use object_store::memory::InMemory; use super::*; @@ -320,7 +321,7 @@ mod tests { fn test_run_options(run_dir: &std::path::Path) -> RunOptions { RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_dir.to_path_buf(), cancel_token: None, run_id: test_run_id(), diff --git a/lib/crates/fabro-workflow/src/pipeline/initialize.rs b/lib/crates/fabro-workflow/src/pipeline/initialize.rs index 6bbfc1782..a39738310 100644 --- a/lib/crates/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/initialize.rs @@ -688,7 +688,8 @@ mod tests { use fabro_interview::AutoApproveInterviewer; use fabro_sandbox::SandboxSpec; use fabro_store::Database; - use fabro_types::{RunId, Settings, fixtures}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{RunId, fixtures}; use object_store::memory::InMemory; use super::*; @@ -735,7 +736,7 @@ mod tests { fn test_settings(run_dir: &std::path::Path) -> RunOptions { RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_dir.to_path_buf(), cancel_token: None, run_id: test_run_id(), @@ -757,7 +758,7 @@ mod tests { run_dir.to_path_buf(), RunRecord { run_id: test_run_id(), - settings: Settings::default(), + settings: SettingsFile::default(), graph, workflow_slug: Some("test".to_string()), working_directory: std::env::current_dir().unwrap(), diff --git a/lib/crates/fabro-workflow/src/pipeline/persist.rs b/lib/crates/fabro-workflow/src/pipeline/persist.rs index 2b1cb5fc2..8ed19a9ea 100644 --- a/lib/crates/fabro-workflow/src/pipeline/persist.rs +++ b/lib/crates/fabro-workflow/src/pipeline/persist.rs @@ -54,7 +54,10 @@ mod tests { use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_store::{Database, RunDatabase}; - use fabro_types::{Settings, fixtures}; + use fabro_types::fixtures; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::settings::v2::cli::{CliLayer, CliOutputLayer, OutputVerbosity}; + use fabro_types::settings::v2::run::{RunExecutionLayer, RunLayer, RunMode}; use object_store::memory::InMemory; use std::sync::Arc; use std::time::Duration; @@ -118,10 +121,22 @@ mod tests { fn sample_record(graph: Graph) -> RunRecord { RunRecord { run_id: fixtures::RUN_1, - settings: Settings { - dry_run: Some(true), - verbose: Some(true), - ..Default::default() + settings: SettingsFile { + run: Some(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + cli: Some(CliLayer { + output: Some(CliOutputLayer { + verbosity: Some(OutputVerbosity::Verbose), + ..CliOutputLayer::default() + }), + ..CliLayer::default() + }), + ..SettingsFile::default() }, graph, workflow_slug: Some("ship".to_string()), diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs index 914a9daf5..e02cdf255 100644 --- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs @@ -595,7 +595,8 @@ mod tests { AggregateStats, FrictionKind, FrictionPoint, OpenItem, OpenItemKind, StageRetro, }; use fabro_store::Database; - use fabro_types::{BilledTokenCounts, RunRecord, Settings, fixtures}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{BilledTokenCounts, RunRecord, fixtures}; use futures::stream; use object_store::memory::InMemory; use std::time::Duration; @@ -1082,7 +1083,7 @@ mod tests { let run_record = RunRecord { run_id: fixtures::RUN_1, - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: Some("test".to_string()), working_directory: PathBuf::from("/tmp/project"), @@ -1155,7 +1156,7 @@ mod tests { let run_record = RunRecord { run_id: fixtures::RUN_1, - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: Some("test".to_string()), working_directory: PathBuf::from("/tmp/project"), @@ -1381,7 +1382,7 @@ mod tests { let run_store = store.create_run(&fixtures::RUN_1).await.unwrap(); let run_record = RunRecord { run_id: fixtures::RUN_1, - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: None, working_directory: tmp.path().to_path_buf(), diff --git a/lib/crates/fabro-workflow/src/pipeline/retro.rs b/lib/crates/fabro-workflow/src/pipeline/retro.rs index 1511c0ced..ab71a75c0 100644 --- a/lib/crates/fabro-workflow/src/pipeline/retro.rs +++ b/lib/crates/fabro-workflow/src/pipeline/retro.rs @@ -184,7 +184,8 @@ mod tests { use fabro_graphviz::graph::Graph; use fabro_store::Database; - use fabro_types::{RunId, Settings, fixtures}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{RunId, fixtures}; use object_store::memory::InMemory; use super::*; @@ -233,7 +234,7 @@ mod tests { let run_store = inner; let run_record = RunRecord { run_id: test_run_id(), - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: None, working_directory: run_dir.to_path_buf(), @@ -304,7 +305,7 @@ mod tests { fn test_run_options(run_dir: &std::path::Path) -> RunOptions { RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_dir.to_path_buf(), cancel_token: None, run_id: test_run_id(), diff --git a/lib/crates/fabro-workflow/src/run_lookup.rs b/lib/crates/fabro-workflow/src/run_lookup.rs index d0277a728..10e0746cd 100644 --- a/lib/crates/fabro-workflow/src/run_lookup.rs +++ b/lib/crates/fabro-workflow/src/run_lookup.rs @@ -5,7 +5,8 @@ use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; use fabro_config::Storage; use fabro_store::{Database, RunSummary}; -use fabro_types::{RunId, Settings}; +use fabro_types::RunId; +use fabro_types::settings::v2::SettingsFile; use serde::Serialize; use crate::operations::make_run_dir; @@ -141,7 +142,7 @@ pub fn scratch_base(storage_dir: &Path) -> PathBuf { } pub fn default_scratch_base() -> PathBuf { - scratch_base(&Settings::default().storage_dir()) + scratch_base(&SettingsFile::default().storage_dir()) } fn scan_orphan_runs(base: &Path) -> Result> { @@ -396,7 +397,8 @@ mod tests { use fabro_graphviz::graph::Graph; use fabro_store::Database; - use fabro_types::{RunStatus, Settings, fixtures}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{RunStatus, fixtures}; use object_store::memory::InMemory; use super::scan_runs_combined; @@ -415,7 +417,7 @@ mod tests { fn sample_run_record() -> RunRecord { RunRecord { run_id: fixtures::RUN_1, - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: Some("test".to_string()), working_directory: PathBuf::from("/tmp/project"), diff --git a/lib/crates/fabro-workflow/src/runtime_store.rs b/lib/crates/fabro-workflow/src/runtime_store.rs index d4ff85107..860e33340 100644 --- a/lib/crates/fabro-workflow/src/runtime_store.rs +++ b/lib/crates/fabro-workflow/src/runtime_store.rs @@ -113,7 +113,8 @@ mod tests { use fabro_store::Database; use fabro_types::fixtures; use fabro_types::run_event::RunSubmittedProps; - use fabro_types::{EventBody, RunEvent, Settings}; + use fabro_types::settings::v2::SettingsFile; + use fabro_types::{EventBody, RunEvent}; use object_store::memory::InMemory; use super::RunStoreHandle; @@ -132,7 +133,7 @@ mod tests { fn test_run_record() -> RunRecord { RunRecord { run_id: fixtures::RUN_1, - settings: Settings::default(), + settings: SettingsFile::default(), graph: Graph::new("test"), workflow_slug: Some("test".to_string()), working_directory: PathBuf::from("/tmp/test"), diff --git a/lib/crates/fabro-workflow/src/test_support.rs b/lib/crates/fabro-workflow/src/test_support.rs index ebe61f89d..b8763c29a 100644 --- a/lib/crates/fabro-workflow/src/test_support.rs +++ b/lib/crates/fabro-workflow/src/test_support.rs @@ -339,13 +339,13 @@ impl WorkflowRunner { .unwrap() .take() .expect("WorkflowRunner may only be used once"); - run_graph( + Box::pin(run_graph( registry, Arc::clone(&self.emitter), Arc::clone(&self.sandbox), graph, run_options, - ) + )) .await } @@ -382,14 +382,14 @@ impl WorkflowRunner { .unwrap() .take() .expect("WorkflowRunner may only be used once"); - run_graph_from_checkpoint( + Box::pin(run_graph_from_checkpoint( registry, Arc::clone(&self.emitter), Arc::clone(&self.sandbox), graph, run_options, checkpoint, - ) + )) .await } diff --git a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs index 514a38eca..7ba0b0189 100644 --- a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs @@ -22,7 +22,9 @@ use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_llm::provider::Provider; use fabro_sandbox::daytona::{DaytonaConfig, DaytonaSandbox, DaytonaSnapshotConfig}; use fabro_store::{ArtifactStore, Database}; -use fabro_types::{RunId, Settings, StageId}; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::run::{RunArtifactsLayer, RunLayer}; +use fabro_types::{RunId, StageId}; use fabro_workflow::artifact::sync_artifacts_to_env; use fabro_workflow::context::Context; use fabro_workflow::error::FabroError; @@ -496,7 +498,7 @@ async fn daytona_pipeline_artifact_offload_and_sync() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -674,7 +676,7 @@ async fn daytona_git_checkpoint_remote_emits_events() { let engine = WorkflowRunner::new(registry, Arc::new(emitter), env.clone()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("git-cp-test"), @@ -845,7 +847,7 @@ async fn daytona_parallel_git_branching_e2e() { let engine = WorkflowRunner::new(registry, Arc::new(emitter), Arc::clone(&env)); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_tmp.path().to_path_buf(), cancel_token: None, run_id, @@ -1192,7 +1194,7 @@ async fn daytona_git_checkpoint_with_shadow_branch() { let meta_branch = MetadataStore::branch_name(&run_id.to_string()); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id, @@ -1326,11 +1328,14 @@ async fn daytona_asset_collection() { graph.edges.push(Edge::new("create_assets", "exit")); let run_options = RunOptions { - settings: Settings { - artifacts: Some(fabro_config::run::ArtifactsSettings { - include: vec!["test-results/**".to_string()], + settings: SettingsFile { + run: Some(RunLayer { + artifacts: Some(RunArtifactsLayer { + include: vec!["test-results/**".to_string()], + }), + ..RunLayer::default() }), - ..Settings::default() + ..SettingsFile::default() }, run_dir: dir.path().to_path_buf(), cancel_token: None, @@ -1586,7 +1591,7 @@ async fn daytona_git_push_run_branch_to_origin() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id, diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 6fd54edf6..383eeef3b 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -26,6 +26,8 @@ use fabro_interview::{ }; use fabro_llm::provider::Provider; use fabro_store::{ArtifactStore, Database}; +use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::v2::run::{RunArtifactsLayer, RunLayer}; use fabro_types::{RunEvent, RunId, Settings, StageId}; use fabro_validate::{Severity, validate, validate_or_raise}; use fabro_workflow::context::Context; @@ -336,7 +338,7 @@ async fn end_to_end_linear_pipeline() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -465,7 +467,7 @@ async fn end_to_end_branching_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -584,7 +586,7 @@ async fn end_to_end_human_gate_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -679,7 +681,7 @@ async fn human_gate_interrupted_input_fails_closed_without_fail_route() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -789,7 +791,7 @@ async fn human_gate_interrupted_input_routes_via_outcome_fail_condition() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -901,7 +903,7 @@ async fn goal_gate_routes_to_retry_target_on_failure() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -1021,7 +1023,7 @@ async fn goal_gate_routes_to_retry_target_when_present() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -1332,7 +1334,7 @@ async fn retry_on_failure_then_succeed() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -1406,7 +1408,7 @@ async fn pipeline_with_many_nodes() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -1751,7 +1753,7 @@ async fn smoke_test_with_mock_codergen_backend() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -1852,7 +1854,7 @@ async fn end_to_end_parallel_fan_out_fan_in() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -1964,7 +1966,7 @@ async fn resume_from_checkpoint_completes_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2062,7 +2064,7 @@ async fn resume_from_checkpoint_preserves_goal_gate_outcomes() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2104,7 +2106,7 @@ async fn graph_goal_in_context() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2142,7 +2144,7 @@ async fn event_streaming_lifecycle() { let events = collect_events(&emitter); let engine = WorkflowRunner::new(make_linear_registry(), Arc::new(emitter), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2221,7 +2223,7 @@ async fn context_flow_between_stages() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2276,7 +2278,7 @@ async fn tool_handler_e2e() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2350,7 +2352,7 @@ async fn auto_approve_interviewer_e2e() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2389,7 +2391,7 @@ async fn codergen_without_backend_simulated() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2493,7 +2495,7 @@ async fn branching_loop_back_on_failure() { ); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2578,7 +2580,7 @@ async fn human_gate_loops_back() { registry.register("human", Box::new(HumanHandler::new(interviewer))); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2638,7 +2640,7 @@ async fn scenario_ship_a_feature() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2722,7 +2724,7 @@ async fn scenario_parallel_expert_review() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2808,7 +2810,7 @@ async fn scenario_node_retries_on_retry_status() { ); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2872,7 +2874,7 @@ async fn scenario_loop_restart_resets_context() { ); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -2939,7 +2941,7 @@ async fn scenario_bug_triage_router() { registry.register("conditional", Box::new(ConditionalHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3000,7 +3002,7 @@ async fn scenario_crash_recovery() { registry.register("exit", Box::new(ExitHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3108,7 +3110,7 @@ async fn manager_loop_stop_condition_satisfied_e2e() { registry.register("stack.manager_loop", Box::new(SubWorkflowHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3189,7 +3191,7 @@ async fn manager_loop_max_cycles_exceeded_e2e() { registry.register("stack.manager_loop", Box::new(SubWorkflowHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3329,7 +3331,7 @@ async fn conditional_branching_success_fail_paths() { registry.register("always_fail", Box::new(AlwaysFailHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3384,7 +3386,7 @@ async fn edge_selection_condition_match_wins_over_weight() { registry.register("exit", Box::new(ExitHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3433,7 +3435,7 @@ async fn edge_selection_weight_breaks_ties() { registry.register("exit", Box::new(ExitHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3474,7 +3476,7 @@ async fn edge_selection_lexical_tiebreak() { registry.register("exit", Box::new(ExitHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3534,7 +3536,7 @@ async fn context_updates_visible_across_nodes() { registry.register("context_setter", Box::new(ContextSetterHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3580,7 +3582,7 @@ async fn stylesheet_applies_model_override() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3635,7 +3637,7 @@ async fn custom_handler_registration_and_execution() { registry.register("my_custom", Box::new(CustomHandler)); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3708,7 +3710,7 @@ async fn integration_smoke_plan_implement_review_done() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3799,7 +3801,7 @@ async fn manager_loop_runs_child_engine_e2e() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -3932,7 +3934,7 @@ async fn manager_loop_context_flows_e2e() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4007,7 +4009,7 @@ async fn manager_loop_child_dotfile_e2e() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4111,7 +4113,7 @@ async fn import_e2e_through_engine() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4264,7 +4266,7 @@ async fn fidelity_default_is_compact() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4320,7 +4322,7 @@ async fn fidelity_graph_default_applied() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4372,7 +4374,7 @@ async fn fidelity_node_overrides_graph_default() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4430,7 +4432,7 @@ async fn fidelity_edge_overrides_node_and_graph() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4478,7 +4480,7 @@ async fn fidelity_full_produces_empty_preamble() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4536,7 +4538,7 @@ async fn fidelity_truncate_preamble_minimal() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4607,7 +4609,7 @@ async fn fidelity_summary_low_mode() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4673,7 +4675,7 @@ async fn fidelity_summary_medium_mode() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4739,7 +4741,7 @@ async fn fidelity_summary_high_mode() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4798,7 +4800,7 @@ async fn fidelity_full_sets_thread_id_in_context() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4868,7 +4870,7 @@ async fn fidelity_full_nodes_share_thread_id() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -4948,7 +4950,7 @@ async fn fidelity_resume_degrades_full_to_summary_high() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5044,7 +5046,7 @@ async fn fidelity_resume_degrade_only_affects_first_hop() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5127,7 +5129,7 @@ async fn fidelity_resume_no_degrade_when_not_full() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5168,7 +5170,7 @@ async fn fidelity_stored_in_checkpoint_context() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5260,7 +5262,7 @@ async fn fidelity_precedence_multi_node_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5327,7 +5329,7 @@ async fn fidelity_compact_preamble_includes_completed_stages_and_context() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5401,7 +5403,7 @@ async fn fidelity_summary_low_excludes_context_values_in_pipeline() { ); let engine_low = WorkflowRunner::new(registry_low, Arc::new(Emitter::default()), local_env()); let run_options_low = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir_low.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5467,7 +5469,7 @@ async fn fidelity_summary_low_excludes_context_values_in_pipeline() { ); let engine_med = WorkflowRunner::new(registry_med, Arc::new(Emitter::default()), local_env()); let run_options_med = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir_med.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5537,7 +5539,7 @@ async fn fidelity_thread_id_fallback_to_previous_node_in_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5590,7 +5592,7 @@ async fn fidelity_thread_id_from_node_class_in_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5646,7 +5648,7 @@ async fn fidelity_edge_thread_id_override_in_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5703,7 +5705,7 @@ async fn fidelity_full_without_explicit_thread_id_uses_previous_node() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5770,7 +5772,7 @@ async fn fidelity_from_parsed_dot_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5817,7 +5819,7 @@ async fn fidelity_checkpoint_roundtrip_preserves_fidelity() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5888,7 +5890,7 @@ async fn fidelity_node_thread_id_overrides_edge_thread_id_in_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -5974,7 +5976,7 @@ async fn fidelity_resume_preserves_context_values_across_checkpoint() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -6017,7 +6019,7 @@ mod real_llm { use async_trait::async_trait; use fabro_graphviz::graph::Node; - use fabro_types::Settings; + use fabro_types::settings::v2::SettingsFile; use fabro_workflow::context::Context; use fabro_workflow::error::FabroError; use fabro_workflow::handler::agent::{AgentHandler, CodergenBackend, CodergenResult}; @@ -6211,7 +6213,7 @@ mod real_llm { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -6319,7 +6321,7 @@ mod real_llm { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -6451,7 +6453,7 @@ mod real_llm { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -6551,7 +6553,7 @@ mod real_llm { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -6644,7 +6646,7 @@ async fn human_gate_freeform_only_routes_text() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -6773,7 +6775,7 @@ async fn human_gate_freeform_with_fixed_choice_match() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -6887,7 +6889,7 @@ async fn human_gate_freeform_fallback_on_unmatched_text() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -7014,7 +7016,7 @@ async fn human_gate_freeform_sets_allow_freeform_on_question() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -7121,7 +7123,7 @@ async fn human_gate_without_freeform_sets_allow_freeform_false() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -7421,7 +7423,7 @@ fn engine_with_hooks_and_events( fn make_run_options(dir: &std::path::Path) -> RunOptions { RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.to_path_buf(), cancel_token: None, run_id: test_run_id("hook-test-run"), @@ -8437,7 +8439,7 @@ async fn run_fidelity_prompt_pipeline(fidelity: &str) -> String { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -8637,7 +8639,7 @@ async fn large_context_values_are_offloaded_to_artifact_store() { let events = collect_events(&emitter); let engine = WorkflowRunner::new(registry, Arc::new(emitter), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -8840,7 +8842,7 @@ async fn artifact_pointers_rewritten_for_remote_sandbox() { let remote_env = Arc::new(RemoteMockEnv::new("/sandbox")); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), remote_env.clone()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -8927,7 +8929,7 @@ async fn downstream_local_execution_materializes_blob_refs_to_runtime_files() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -9014,7 +9016,7 @@ async fn downstream_remote_execution_materializes_blob_refs_to_sandbox_files() { let remote_env = Arc::new(RemoteMockEnv::new("/sandbox")); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), remote_env.clone()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -9144,7 +9146,7 @@ async fn node_dir_uses_visit_count_on_revisit() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -10013,7 +10015,7 @@ async fn full_pipeline_with_cli_backend_node() { let dir = tempfile::tempdir().unwrap(); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -10131,7 +10133,7 @@ async fn stylesheet_backend_property_routes_to_cli() { let dir = tempfile::tempdir().unwrap(); let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"), @@ -10321,7 +10323,7 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { let engine = WorkflowRunner::new(registry, Arc::new(emitter), env); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-docker"), @@ -10487,7 +10489,7 @@ async fn git_checkpoint_host_writes_shadow_branch() { let meta_branch = MetadataStore::branch_name(&run_id.to_string()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, run_id, @@ -10684,7 +10686,7 @@ async fn parallel_git_branching_host_e2e() { let engine = WorkflowRunner::new(registry, Arc::new(emitter), env); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, run_id, @@ -10933,7 +10935,7 @@ async fn git_checkpoint_host_skips_empty_diff_patch() { let engine = WorkflowRunner::new(registry, Arc::new(emitter), env); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("empty-diff"), @@ -11300,7 +11302,7 @@ async fn e2e_circuit_breaker_deterministic_self_loop() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-circuit-breaker"), @@ -11346,7 +11348,7 @@ async fn e2e_circuit_breaker_custom_limit() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-custom-limit"), @@ -11385,7 +11387,7 @@ async fn e2e_circuit_breaker_ignores_transient_failures() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-transient-no-breaker"), @@ -11431,7 +11433,7 @@ async fn e2e_circuit_breaker_different_reasons_separate_counters() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-varying-reasons"), @@ -11470,7 +11472,7 @@ async fn e2e_circuit_breaker_loop_restart() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-restart-breaker"), @@ -11531,7 +11533,7 @@ async fn e2e_failure_signature_persisted_in_context() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-sig-context"), @@ -11594,7 +11596,7 @@ async fn e2e_failure_signature_hint_overrides_reason_in_context() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-sig-hint"), @@ -11649,7 +11651,7 @@ async fn e2e_signature_maps_persist_in_checkpoint() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-sig-persist"), @@ -11775,7 +11777,7 @@ async fn e2e_circuit_breaker_emits_events_before_abort() { let engine = WorkflowRunner::new(registry, Arc::new(emitter), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-events"), @@ -11839,7 +11841,7 @@ async fn e2e_circuit_breaker_does_not_fire_below_limit() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-below-limit"), @@ -11934,7 +11936,7 @@ async fn e2e_circuit_breaker_multi_stage_impl_verify_cycle() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-impl-verify-cycle"), @@ -12030,7 +12032,7 @@ async fn e2e_loop_restart_blocked_for_deterministic_failure() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-restart-blocked-det"), @@ -12069,7 +12071,7 @@ async fn e2e_loop_restart_blocked_for_structural_failure() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-restart-blocked-struct"), @@ -12108,7 +12110,7 @@ async fn e2e_loop_restart_blocked_for_budget_exhausted_failure() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-restart-blocked-budget"), @@ -12147,7 +12149,7 @@ async fn e2e_loop_restart_blocked_for_canceled_failure() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-restart-blocked-canceled"), @@ -12183,7 +12185,7 @@ async fn e2e_loop_restart_blocked_for_compilation_loop_failure() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-restart-blocked-comploop"), @@ -12223,7 +12225,7 @@ async fn e2e_loop_restart_allowed_for_transient_infra() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("e2e-restart-allowed-transient"), @@ -12330,7 +12332,7 @@ async fn e2e_stall_watchdog_triggers_from_dot_parsed_pipeline() { let engine = WorkflowRunner::new(registry, Arc::new(emitter), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("stall-e2e"), @@ -12385,7 +12387,7 @@ async fn e2e_stall_watchdog_kept_alive_by_handler_events() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("stall-alive-e2e"), @@ -12430,7 +12432,7 @@ async fn e2e_stall_watchdog_disabled_with_zero_timeout() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("stall-disabled-e2e"), @@ -12494,7 +12496,7 @@ async fn e2e_stall_watchdog_with_explicit_timeout_override() { let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env()); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("stall-override-e2e"), @@ -12624,11 +12626,14 @@ async fn asset_collection_local_sandbox_success() { graph.edges.push(Edge::new("create_assets", "exit")); let run_options = RunOptions { - settings: Settings { - artifacts: Some(fabro_config::run::ArtifactsSettings { - include: vec!["test-results/**".to_string()], + settings: SettingsFile { + run: Some(RunLayer { + artifacts: Some(RunArtifactsLayer { + include: vec!["test-results/**".to_string()], + }), + ..RunLayer::default() }), - ..Settings::default() + ..SettingsFile::default() }, run_dir: run_dir.path().to_path_buf(), cancel_token: None, @@ -12753,11 +12758,14 @@ async fn asset_collection_local_sandbox_on_failure() { graph.edges.push(Edge::new("create_assets", "exit")); let run_options = RunOptions { - settings: Settings { - artifacts: Some(fabro_config::run::ArtifactsSettings { - include: vec!["test-results/**".to_string()], + settings: SettingsFile { + run: Some(RunLayer { + artifacts: Some(RunArtifactsLayer { + include: vec!["test-results/**".to_string()], + }), + ..RunLayer::default() }), - ..Settings::default() + ..SettingsFile::default() }, run_dir: run_dir.path().to_path_buf(), cancel_token: None, @@ -12854,11 +12862,14 @@ async fn asset_collection_docker_sandbox() { graph.edges.push(Edge::new("create_assets", "exit")); let run_options = RunOptions { - settings: Settings { - artifacts: Some(fabro_config::run::ArtifactsSettings { - include: vec!["test-results/**".to_string()], + settings: SettingsFile { + run: Some(RunLayer { + artifacts: Some(RunArtifactsLayer { + include: vec!["test-results/**".to_string()], + }), + ..RunLayer::default() }), - ..Settings::default() + ..SettingsFile::default() }, run_dir: run_dir.path().to_path_buf(), cancel_token: None, @@ -12927,7 +12938,7 @@ async fn wait_timer_e2e() { local_env(), ); let run_options = RunOptions { - settings: Settings::default(), + settings: SettingsFile::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, run_id: test_run_id("test-run"),