SettingsLayer::{test_default, ensure_test_auth_methods} are `pub(crate)`
and only called from fabro-config's own in-crate tests, but their impl
block was gated on `cfg(any(test, feature = "test-support"))`. No
external crate enabled the `test-support` feature, so under
`--all-features` the methods compiled in without reachable callers and
clippy flagged them as dead code. Narrow the gate to `cfg(test)` and
drop the vestigial feature entry.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After Unit 3.1 of the config boundary refactor, fabro-types no longer has
any #[derive(Combine)] sites — the fabro-macros dep is unused. Likewise
`Duration as DurationLayer` was an artifact from when layer and vocabulary
types lived side-by-side; the resolved Duration type has no Layer form now,
so the alias was misleading.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Collapse splice_model_fallbacks and splice_events into a single
generic splice_combine guarded by a new SpliceMarker trait; the
Combine impl for Vec<T: SpliceMarker> replaces the two per-enum impls.
- Drop inherent iter/iter_mut (redundant with Deref/DerefMut),
AsRef/AsMut, and IntoIterator for &_/&mut _ on ReplaceMap/StickyMap/
MergeMap — they had zero external callers. DerefMut and IntoIterator
for Self stay because labels.extend(...) relies on both.
- Drop unused _api parameter on resolve_web.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fresh `Resolver` API takes a `&SettingsLayer`, not a file path, so
its constructor should match the existing `ServerSettings::from_layer`
and `UserSettings::from_layer` naming. The `*_from_file` suffix on the
older free helpers is a legacy choice (their input was historically
loaded from a file); leave those names alone since they're a stable
public API used across many call sites.
Also drop two doc-comment references to specific call sites (the simplify
guidelines treat those as rot bait — call sites move, the doc lies).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously, each per-namespace `resolve_*_from_file` helper, together with
`resolve_storage_root` and the `*Settings::from_layer` constructors, called
`apply_builtin_defaults(file.clone())` independently. That meant every
batch resolve cloned the entire `SettingsLayer` (including hooks, MCPs,
sandbox config, etc.) and merged the static defaults layer once per call.
The worst offender, `fabro_workflow::operations::create::resolve_settings_tree`,
ran that pipeline four times back-to-back per `create_run` request.
Add `fabro_config::Resolver`, which applies builtin defaults exactly once
on construction and exposes per-namespace methods (`server`, `cli`,
`features`, `project`, `run`, `workflow`, `storage_root`) plus low-level
`*_into(&mut errors)` variants for callers that want to merge errors
across multiple namespaces. The standalone `resolve_*_from_file` helpers
and `resolve_storage_root` remain on the public API, but each is now a
one-liner that delegates to `Resolver::from_file(...)` so single-namespace
callers see no behavior change.
Migrate the multi-namespace consumers:
- `ServerSettings::from_layer` and `UserSettings::from_layer` build one
`Resolver` and call the `*_into` pair, preserving the original
"surface all errors from both namespaces" semantics.
- `resolve_settings_tree` builds one `Resolver` and pulls all four
namespaces from it, dropping three redundant defaulting+clone passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three byte-identical copies of `render_resolve_errors` had drifted into
`fabro-server/src/run_manifest.rs`, `fabro-workflow/src/operations/start.rs`,
and `fabro-workflow/src/operations/create.rs`. Each one folded a
`&[ResolveError]` into a semicolon-separated string for surfacing through
`anyhow!` / `Error::Precondition` envelopes.
Promote the helper to `fabro_config::render_resolve_errors` (it lives next
to `ResolveError`, the type it acts on) and rewrite the four call sites
in workflow ops plus the one in run_manifest to call the shared version.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Promote `apply_storage_dir_override` to `fabro_config::user` so the
serve startup path stops carrying its own copy of the storage-root
mutation that already lived in `fabro-cli/user_config.rs`.
- Inline the `load_settings` and `router_web_enabled` one-liner wrappers
in `fabro-server/src/serve.rs` and drop the dead
`let _ = CliLayer::default()` marker.
- Cache the demo `server_settings()` JSON in a `OnceLock` so the demo
mode stops re-parsing TOML, re-resolving, and re-serializing the same
static fixture on every `GET /api/v1/settings` request.
- Standardize the four `state.settings.read().unwrap()` callsites in
`fabro-server/src/server.rs` on `.expect("settings lock poisoned")`
to match the existing convention.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related correctness bugs surfaced by the failing test suite:
1. Server-owned settings didn't flow into run settings, and the few
server-only fields that did leak in made run snapshots bulky and let
callers re-resolve server state from the run layer.
- effective_settings::materialize_settings_layer now treats the
server's run/features stanzas as base defaults (client layers
still win where set), and enforce_server_authority keeps the
original cherry-pick of storage/scheduler/artifacts/web/api but
no longer lets the rest of the server namespace propagate. auth,
listen, ip_allowlist, slatedb, logging, and integrations stay on
the server, where AppState::server_settings() already has them.
- run_preflight, the scheduler start-path, and operations::start
now read GitHub integrations from state.server_settings() (or
StartServices::github_permissions, which the server populates)
instead of re-resolving the server namespace from the run's
settings layer.
- create_app_state{_with_options,_with_env_lookup,_with_options_and_registry_factory}
and create_app_state_with_store_and_env_lookup all route through
ensure_test_auth_methods so the strict resolver accepts
SettingsLayer::default() in tests.
- Fixed the start_run_persists_full_settings_snapshot assertion
that expected server.integrations.github.app_id in the run's
persisted settings — the new design deliberately omits it.
2. Unit and integration tests were hitting live AWS S3.
- Added a NoProxyReqwestConnector (behind a dedicated reqwest 0.12
dep aliased as object_store_reqwest) and wired it through
AmazonS3Builder::with_http_connector. macOS SystemConfiguration
proxy discovery in the default reqwest client was blowing past
nextest's 20s kill timeout on serve.rs's S3 builder unit tests;
the no-proxy connector brings them under 15ms.
- InstallAppState::for_test_with_paths now sets
FABRO_TEST_IN_MEMORY_STORE=1 so /install/finish's artifact-metadata
sentinel write short-circuits to the in-memory object store and
never contacts AWS. The install integration tests verify
persistence/redaction, not S3 reachability.
`cargo nextest run --workspace`: 4495/4495 passing.
`cargo +nightly-2026-04-14 fmt --check --all`: clean.
`cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings`: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Import serde:🇩🇪:Error trait so the `custom` fn pointer uses `D::Error`
instead of the absolute `serde:🇩🇪:Error::custom` path.
- Import `fabro_api::types::ServerSettings` / `fabro_config::UserSettings`
directly rather than through absolute paths.
- Gate sync `std::fs::write` fixture setup in new config resolver tests
with a file-level `#![expect(clippy::disallowed_methods, …)]`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Remove CommandContext::cli_settings and cascade through 11 functions
whose only use of `cli: &CliNamespace` was constructing it; dispatchers
now forward only cli_layer.
- Drop `ServerSettings as CurrentServerSettings` /
`ServerNamespace as ResolvedServerSettings` rename aliases; use the
canonical type names in fabro-server.
- Inline `local_server::server_settings` and `user_config::{resolve_user_settings,
resolve_cli_settings}` wrappers; callers use `ServerSettings::from_layer`
/ `UserSettings::from_layer` directly (anyhow converts via `?`).
- Trim narrative module doc in fabro-config/src/lib.rs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Clean up clippy -D warnings violations that slipped through in 4bf0c4031:
- envfile.rs: extend the existing module-level #![expect] to also cover clippy::disallowed_types so the intentional std::io::Write usage stops tripping the workspace lint.
- install.rs: import ServerSecrets and EnvFileUpdate at the top and drop the fully-qualified call sites (unused_qualifications); collapse the nested if-let around the post-finish manual-credentials cleanup (collapsible_match); gate the test's std::fs::write with #[expect(clippy::disallowed_methods, reason="...")].
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Bind` and `ServerDaemon` are serde-serialized descriptions of on-disk
server state (the `server.json` record). They belong with
`RuntimeDirectory` in fabro-config rather than in fabro-server's web
layer.
The practical payoff: fabro-test was hand-parsing `server.json` via
`serde_json::Value["pid"]` because fabro-server already depends on
fabro-test (cycle blocked the reverse edge). Moving these types into
fabro-config lets fabro-test call `ServerDaemon::{load_running, read,
remove}` directly, dropping ~20 lines of duplicated record parsing.
fabro-config gains `fabro-proc` and `tempfile` as deps to cover
`ServerDaemon::{is_running, write}`. All 16 `fabro_server::{bind,
daemon}` import sites in fabro-server and fabro-cli are rewritten to
`fabro_config::{bind, daemon}`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add SettingsLayer::test_default() and SettingsLayer::ensure_test_auth_methods()
to fabro-types behind a "test-support" feature, then collapse the five
near-identical ensure_fixture_auth_methods/default_settings/test_default_settings
helpers that the dev-token gating cleanup spread across fabro-config,
fabro-server, and fabro-workflow.
Why: the next required SettingsLayer field would otherwise need updating in
five places. With the canonical helper in fabro-types, adding a required field
becomes a one-line change.
The cfg(any(test, feature = "test-support")) gate keeps the helpers out of
production builds. Consumer crates enable the feature via dev-dependencies.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After removing the implicit [server.auth] dev-token default, several unit
tests still passed empty SettingsLayer values into paths that resolve
server settings, so they panicked with "server.auth.methods: field is
required". Restore them by injecting dev-token methods in test fixtures
(consistent with the existing fabro-config resolve_server test pattern),
and rescue create_test_app_state_with_session_key, which bypassed the
existing ensure_test_auth_methods helper.
Clippy clean-ups unblock `cargo clippy --workspace -- -D warnings`:
- fabro-config: bring SettingsLayer into scope, flatten single-arm match
- fabro-cli: gate storage_dir unit tests with allow(deprecated), drop
unnecessary borrow, scope effective_settings imports, drop needless
raw-string hashes
- fabro-server: replace Option<Option<String>> test helper with an
EnvOverride enum, widen test unwrap → expect
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
