The check `ctx.user_settings().cli.output.verbosity == OutputVerbosity::Verbose`
repeated 6 times across doctor, preflight, run command/resume/mod. Add a
`verbose()` method and replace every call site. `cargo fix` handles the
now-unused `OutputVerbosity` imports.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-applied by `cargo +nightly-2026-04-14 fmt --all`. `cargo fix`
earlier inserted the import in a position that violated the grouped
ordering.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cfg(test) helper only re-implemented the production `with_server_mode`
struct literal so tests could inject settings without disk I/O. Three
tests used it, but each one was self-referential — asserting what the
helper itself does rather than exercising production code. Remove the
helper and those three tests.
`synthetic_context_with_settings` is no longer needed either (its
flexibility only mattered for the removed tests); collapse it into
`synthetic_context`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ResolvedBaseContext` was a staging struct that held pre-context
settings so main.rs could read `user_settings.cli` before committing
to a full `CommandContext`. Now that install no longer needs the
indirection, the staging step doesn't earn its weight.
Give `CommandContext` a direct `from_disk(cli_layer, process_local_json)`
constructor that does the load + printer derivation + struct build in
one shot. main.rs builds one `base_ctx` before the dispatch match and
every arm borrows it — the `build_base_ctx` closure and ~20 duplicate
`let base_ctx = build_base_ctx()?;` lines disappear.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Public entry points (`execute`, `run_install`, `run_install_github_command`,
`run_install_inner`, `run_install_github_inner`) now take `&CommandContext`
instead of a 4-tuple of `(cli, cli_layer, process_local_json, printer)`.
Extract cli/printer/json from the context once at the top of each.
The nested doctor invocation inside run_install_inner previously built a
fresh `ResolvedBaseContext::from_disk(...).to_context()` with
`process_local_json = false`. Doctor only reads the resolved output
format (`base_ctx.json_output()`), not the invocation flag, so passing
the parent ctx through is behaviorally equivalent and avoids a second
disk load.
main.rs drops the `cli_settings` local entirely — no remaining command
needs it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These top-level commands still took `&CliNamespace` + `Printer`
separately. Thread `&CommandContext` through the public entry points
and pull what's needed (`user_settings().cli`, `printer()`,
`json_output()`) from the context:
- parse: both args were unused — drop entirely.
- workflow list/create: use `ctx.json_output()` / `ctx.printer()`.
- exec: bind `cli = &ctx.user_settings().cli` at the top; drop unused
printer param.
- upgrade: extract cli/printer inside run_upgrade; leave the private
run_upgrade_brew helper with its existing signature (unit tests use
`CliNamespace::default()` directly).
- uninstall: use `ctx.json_output()` / `ctx.printer()`.
Install remains on the old signature — its nested callback structure
makes a larger refactor than this simplification pass warrants.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The helper was a 2-line indirection with a single caller. Inline its
body so the whole command fits in one function.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`load_pr_record` built a `with_target`-derived context internally and
threw it away, forcing callers to re-derive or fall back to `base_ctx`.
Return the context alongside the record and let close/merge/view use
it directly for printer/json access and github-credentials lookup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`CommandContext::base` had a single caller (install.rs) and duplicated
the disk-load path that `ResolvedBaseContext::from_disk` already
provides. Route install through `ResolvedBaseContext::from_disk(...).to_context()`
and drop the standalone constructor. `base_with_settings` stays as the
private shared helper behind both `to_context` entry points.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `cli_settings` local was a standalone clone of `user_settings.cli`.
Drop the clone and rebind it as `&resolved_base.user_settings().cli`
inside the async block — all callers already borrowed it anyway.
Pre-async uses inline `resolved_base.user_settings().cli.<field>`
directly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The secret dispatcher pre-computed json/printer and threaded them into
every subcommand. Pass the context directly so each subcommand pulls
what it needs, dropping the fabro_util:🖨️:Printer import from
three files along the way.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`run_bulk` and `remove_from` took separate `json: bool` + `printer`
parameters. Thread the context through instead and pull json/printer
out of it inside the helper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The helper only exists to let tests inject pre-loaded settings. Inline
the struct-literal into the sole production caller and mark the helper
`#[cfg(test)]` so the test-seam intent is explicit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The check `ctx.user_settings().cli.output.format == OutputFormat::Json`
(and its `!=` variant) was repeated 51 times across 36 files. Add a
`json_output()` method on CommandContext and replace every call site.
`cargo fix` handles the now-unused `OutputFormat` imports.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- ssh/graph: remove `explicit_json_requested() &&` guard before
`require_no_json_override()` (the call already no-ops without --json).
- pr close/merge/view: drop the outer `with_target` derivation that was
used only for `printer()` and the output format — both match base_ctx,
so the derivation was an unused disk-read + settings re-merge.
- command_context tests: collapse `synthetic_context` to delegate to
`synthetic_context_with_settings`, removing duplicated struct literals.
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>
- Derive strum::IntoStaticStr on InstallObjectStoreProvider/CredentialMode and use it in as_session_value instead of a hand-written match.
- Split resolve_install_object_store_state: extract resolve_s3_manual_credentials and fold the redundant outer "missing credentials" guard into its (None, None) arm.
- Replace the per-endpoint installFetch boilerplate with installRequest / installJsonRequest<T> so each install-api wrapper is a single call.
- Extract runStepSubmit inside InstallApp; the LLM, server, object-store, and GitHub step handlers now share the setSubmitting / try / refresh-session / navigate / finally scaffolding.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Derive install stepper's current step from INSTALL_STEPS instead of a hand-maintained pathname if-chain.
- Replace four near-identical picker components with one generic CardPicker plus per-flow option arrays.
- Extract repeated object-store validation error strings into constants and a small helper.
- Run the S3 artifacts/ and slatedb/ prefix probes concurrently via tokio::try_join!.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace hand-written Display/FromStr/as_str boilerplate with strum
derives on Provider, RunStatus, StatusReason, Speed, ReasoningEffort,
SandboxProvider, Fidelity, ModelTestMode, ModelTestStatus. Update a few
downstream callers whose FromStr::Err = String assumption no longer
holds. Net -172 lines, zero wire-format change.
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>
Collapses the identical scopeguard + serve_command block in
`start.rs::execute_foreground` into a single `foreground::serve_with_daemon_record`
helper shared with `server::dispatch`.
Changes `prepare_foreground_server_log`, `acquire_lock`, and
`load_or_create_local_session_secret` to take `&RuntimeDirectory` instead
of `&Path storage_dir`, since each only consumed the path to immediately
rebuild a `RuntimeDirectory`. Callers that still need the raw storage
path for child processes keep it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deduplicates the `match bind { Unix(p) => p.to_string_lossy(), Tcp(a) => format!("http://{a}") }`
formatting shared between `worker_command` and the `server_target` test helper
by moving it onto `Bind` itself. Also surfaces unexpected errors from
`ServerDaemon::remove` via `tracing::warn!` instead of silently discarding
them, while still short-circuiting the common `NotFound` path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace hand-rolled `format!("{}@{}", node_id_segment.display(), stage_id.visit())`
with `stage_id.to_string()`, matching the pattern already used for the
stages directory at line 61. `StageId`'s Display impl already produces
`{node_id}@{visit}`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The local ensure_test_auth_methods helper in server.rs is reachable from
the pub fn create_app_state_with_store chain, which is compiled in
release builds even though only integration tests call it. After
2cb623561, the helper called SettingsLayer::ensure_test_auth_methods()
— which is gated behind #[cfg(any(test, feature = "test-support"))] —
so cargo build --release broke with E0599.
Inline the auth-methods setup locally. This one helper only needs the
ServerAuthMethod::DevToken default; it doesn't share the "new required
SettingsLayer field" concern that motivated the centralization.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Unauthenticated commands surfaced only `error: Authentication required.`
with no remediation. Add a cyan-bold `hint:` line pointing at
`fabro auth login` in the top-level error printer, keyed off
`ExitClass::AuthRequired` so it covers every command that hits the
server (run, exec, ps, system info, etc.). Suppressed when `--json` is
set.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The assets committed in 537a5125c drifted from what bun 1.3.13 produces
for the current TS source, which broke the nightly release verifier.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
