Commit graph

43 commits

Author SHA1 Message Date
Bryan Helmkamp
baa20ca375
Merge remote-tracking branch 'origin/main' 2026-04-24 11:23:34 -04:00
Bryan Helmkamp
176c0c0159
refactor: GitHubContext + drop dead CommandContext methods
Three cleanups in one pass:

1. Bundle GitHub creds + base URL into a GitHubContext<'_>:
   Defined in fabro-github and threaded through create_pull_request,
   enable_auto_merge, get_pull_request, merge_pull_request, and
   close_pull_request (plus their _with_client variants). Each function
   loses its trailing `base_url: &str` and replaces `creds:
   &GitHubCredentials` with `ctx: GitHubContext<'_>`. Bundle propagates
   into OpenPullRequestRequest as a single `github` field instead of
   the prior split `creds` + `github_api_base_url`.

2. Delete dead CommandContext::storage_dir() and ::server_settings():
   Origin added these for client-side PR commands that no longer exist
   after the server-side migration. Field `server_settings` removed
   from CommandContext (only the deleted method read it). Same field
   pruned from ResolvedCommandSettings; one test that verified the
   underlying loader behavior was rewired to read LoadedSettings
   directly via load_resolved_settings_from_toml.

3. Audit *_error helpers in fabro-server: no remaining single-use
   factories. The previous inlining pass left a tidy surface. No diff.

Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4587 passed, 182 skipped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-24 00:54:11 -04:00
Bryan Helmkamp
4e9dd6fb52
Merge origin/main (preserving RunServices architecture)
Reconciles 61 origin commits (settings/config architectural reshape:
sparse layers → dense snapshots via builders, WorkflowSettings rename,
RunLayer/CliLayer moves, workflow builders, drop of public load wrappers)
with our LLM credential + RunServices refactor.

Our architecture preserved where it conflicted with origin's:

- RunServices / EngineServices stay (services.rs does not exist on
  origin, which inlined the fields onto Initialized). Origin's new
  Initialized fields (inputs, run_store, emitter, sandbox, registry,
  env, dry_run, llm_client, provider) are absorbed through RunServices
  and EngineServices instead of being inlined.
- llm_source: Arc<dyn CredentialSource> stays on AppState and
  RunServices. Origin had a parallel ProviderCredentials struct in
  fabro-server; our CredentialSource trait is more general and
  complies with docs-internal/llm-client-resolution.md. Point-of-use
  Client::from_source(...) rebuild preserves OAuth refresh.
- CommandContext.llm_source() uses self.storage_dir (origin's direct
  field) instead of self.machine_settings (our side's field, removed
  by origin).
- standalone_llm_source in fabro-agent drops the dead Result wrap and
  uses fabro_config::user::default_storage_dir (origin's entrypoint)
  instead of the removed load_settings_user/resolve_storage_root.

Absorbed from origin wholesale:

- SettingsLayer → WorkflowSettings rename everywhere
- Dense run settings: RunOptions.settings is WorkflowSettings, inputs
  read via settings.run.inputs directly (not Option<RunLayer>)
- AppState.manifest_run_defaults / manifest_run_settings
- fabro_config re-exports of CliLayer/RunLayer/CliOutputLayer/etc.
- Lifecycle terminal-event changes, finalize dedup, list_events
  consolidation — already brought in on the previous merge, kept

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-24 00:10:01 -04:00
Bryan Helmkamp
600720423c
Merge remote-tracking branch 'origin/main'
Merge origin's fabro-config types boundary refactor (dense settings
migration: WorkflowSettings/UserSettings/ServerSettings moved to
fabro-types; SettingsLayer made pub(crate) inside fabro-config) into
local PR-refactor branch.

Conflict resolution intent:

- lib/crates/fabro-cli/src/commands/pr/{create,mod}.rs — kept HEAD's
  server-side PR command implementations; origin still carried the
  pre-refactor client-side helpers (build_github_credentials,
  load_pr_record, branch_exists pre-check) that local commits had
  already migrated to the server.
- lib/crates/fabro-cli/src/user_config.rs — took origin's resolution
  (load_resolved_settings_from_toml + storage_dir_from_document tests),
  which implements the same dead-storage_dir-wrapper cleanup local had
  done via local_server::storage_dir.
- lib/crates/fabro-server/src/server.rs — kept HEAD's PullRequestRecord
  import alongside origin's added ServerSettings; rewrote test helpers
  github_token_settings + create_github_token_app_state to use origin's
  ServerSettingsBuilder + AppStateConfig dense-settings shape (replaces
  HEAD's parse_settings_layer + Arc<RwLock<SettingsLayer>>); switched
  RunSpec.settings fixture from SettingsLayer::default() to
  WorkflowSettings::default() per origin's RunSpec retype.
- Suppressed dead_code on CommandContext::storage_dir() and
  ::server_settings() (added by origin for use by client-side PR
  commands that no longer exist after local's server-side migration);
  gated load_resolved_settings_from_toml on cfg(test).

Verified post-merge: workspace fmt clean, clippy --all-targets
-D warnings clean, cargo nextest run --workspace 4587 passed,
182 skipped.
2026-04-24 00:01:09 -04:00
Bryan Helmkamp
64dbdf2500
fix(ci): resolve workspace test and lint regressions 2026-04-23 20:49:30 -04:00
Bryan Helmkamp
ec3d65928f
style(rustfmt): format remaining llm refactor files 2026-04-23 19:51:45 -04:00
Bryan Helmkamp
cb14dc43d1
refactor(llm): use credential sources and split run services 2026-04-23 18:54:27 -04:00
Bryan Helmkamp
db132d11a7
migrate cli install tests off sparse settings layers 2026-04-23 18:37:37 -04:00
Bryan Helmkamp
9a898b12cd
move sparse settings layers into fabro-config 2026-04-23 18:10:12 -04:00
Bryan Helmkamp
83fc1602ea
route cli settings loads through dense config 2026-04-23 17:09:21 -04:00
Bryan Helmkamp
134d8c32d5
add dense run settings builder 2026-04-23 16:37:00 -04:00
Bryan Helmkamp
3667330e79
cache dense run settings in command context 2026-04-23 16:28:56 -04:00
Bryan Helmkamp
daf8c7fb10
split cli command context off sparse machine settings 2026-04-23 15:44:29 -04:00
Bryan Helmkamp
30986207cb
trim config wrapper and storage override helpers 2026-04-23 12:01:49 -04:00
Bryan Helmkamp
9220af6e80
migrate remaining run settings consumers to dense snapshots 2026-04-23 11:55:12 -04:00
Bryan Helmkamp
b2bcf0d5a8
refactor settings builders and dense run snapshots 2026-04-23 11:35:21 -04:00
Bryan Helmkamp
e210fe9fed
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-cli/src/command_context.rs
#	lib/crates/fabro-cli/src/main.rs
2026-04-23 08:56:40 -04:00
Bryan Helmkamp
da9453cbd1
refactor(config): move settings combine into types 2026-04-23 08:42:19 -04:00
Bryan Helmkamp
3bfd93aa60
simplify: add CommandContext::verbose() accessor
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>
2026-04-23 08:36:52 -04:00
Bryan Helmkamp
829a02f1f8
fix: reorder test imports to satisfy rustfmt
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>
2026-04-23 08:23:39 -04:00
Bryan Helmkamp
4318e07ad7
simplify: drop with_server_mode_from_loaded_settings test seam
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>
2026-04-23 08:21:48 -04:00
Bryan Helmkamp
c52e89438b
simplify: fold ResolvedBaseContext into CommandContext
`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>
2026-04-23 08:17:27 -04:00
Bryan Helmkamp
72894b3c46
simplify: drop CommandContext::base in favor of ResolvedBaseContext
`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>
2026-04-23 07:54:53 -04:00
Bryan Helmkamp
c87e282f8c
simplify: gate with_server_mode_from_loaded_settings behind cfg(test)
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>
2026-04-23 07:48:04 -04:00
Bryan Helmkamp
897b818a9e
simplify: introduce CommandContext::json_output() accessor
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>
2026-04-23 07:34:11 -04:00
Bryan Helmkamp
4fa5896d78
refactor(cli): narrow base context construction 2026-04-23 07:33:21 -04:00
Bryan Helmkamp
07d4aee67c
simplify: drop redundant CommandContext derivations and guards
- 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>
2026-04-23 07:28:54 -04:00
Bryan Helmkamp
2b933597b2
refactor(cli): finish command context cleanup 2026-04-23 07:14:32 -04:00
Bryan Helmkamp
fbe0bdfbc1
refactor: align cli command boundaries on CommandContext 2026-04-23 01:08:14 -04:00
Bryan Helmkamp
93b6577cd3
simplify: drop duplicate settings plumbing from cli/server refactor
- 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>
2026-04-22 21:02:37 -04:00
Bryan Helmkamp
ebb8bf7add
refactor settings API entrypoints 2026-04-22 18:58:47 -04:00
Bryan Helmkamp
aa8cdd6986
refactor(cli): rename ServerStoreClient -> Client, fabro_api::Client -> ApiClient
The CLI façade is the primary type callers reach for, so it deserves
the bare `Client` name (per `reqwest::Client`, `hyper::Client`
convention). The raw generated HTTP binding is secondary and is more
accurately named `ApiClient`. "Store" in `ServerStoreClient` was
leftover from the SlateDB-ownership refactor and no longer describes
the type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 08:56:09 -04:00
Bryan Helmkamp
ad0d532691
chore(clippy): require reasons on allow attributes
Enable clippy::allow_attributes_without_reason at the workspace level.
Add concise, callsite-specific reasons to existing allow attributes, including generated code paths.
2026-04-19 20:24:24 -04:00
Bryan Helmkamp
99ce9a66c4 refactor(cli): route global args through settings layer
Resolve CLI settings once from user config plus process-local overrides
and pass the resolved view through command dispatch and CommandContext.

This keeps config-driven cli.output, cli.updates, and cli.logging
behavior working while preserving commands that only reject explicit
--json overrides. It also removes the implicit auto-approve coupling
from JSON run output.
2026-04-14 12:27:59 -04:00
Bryan Helmkamp
4b30a5f16e refactor(cli): route command output through Printer
Plumb the CLI printer through command dispatch, replace direct stdout and
stderr writes with printer helpers, preserve important stdout in quiet
mode, and update the Claude Rust formatting hook to use cargo +nightly
fmt.
2026-04-11 14:40:33 -04:00
Bryan Helmkamp
6a87f0a071 fmt: apply nightly rustfmt after merge
Restore a clean nightly rustfmt baseline on the merged main branch so
cargo +nightly fmt --check --all passes again after bringing in
origin/main.
2026-04-11 13:43:30 -04:00
Bryan Helmkamp
007cfed240 refactor: remove backwards-compat error type aliases
No production deployments exist, so there's no need for migration shims.
Remove all six backwards-compat type aliases (AgentError, SdkError,
CoreError, GraphvizError, StoreError, FabroError) and migrate ~880
callsites to use the canonical Error name directly within each crate,
or qualified imports (e.g., `use fabro_llm::Error as LlmError`) for
cross-crate references. Also fix a pre-existing absolute-path clippy
lint in fabro-server error.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 12:51:42 -04:00
Bryan Helmkamp
5eeacd7864 fmt 2026-04-11 11:27:46 -04:00
Bryan Helmkamp
c5c81d2985 refactor(settings): rename settings layer and move parsing 2026-04-10 08:10:06 -04:00
Bryan Helmkamp
a93bf1df9c refactor(settings): resolve cli settings 2026-04-10 07:04:34 -04:00
Bryan Helmkamp
4e7839c202 refactor(settings): stage 6.5b sweep ::v2:: prefix out of consumers
Final mechanical pass: replaces every remaining
`fabro_types::settings::v2::*` import path with
`fabro_types::settings::*` (or the appropriate submodule) across 53
files in 10 crates, then deletes the transitional
`pub mod v2 { pub use super::*; }` alias from
`fabro-types/src/settings/mod.rs`.

No functional changes — all touches are `sed s|settings::v2::|settings::|g`
on import statements and fully-qualified type paths. The v2
namespace is now fully gone; the authoritative module path is
`fabro_types::settings::{accessors, cli, duration, features, interp,
model_ref, project, run, server, size, splice_array, tree, version,
workflow}`.

All 3,758 workspace tests pass. `cargo fmt --check --all` and
`cargo clippy --workspace -- -D warnings` are clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 18:42:05 -04:00
Bryan Helmkamp
884fa329ba feat(settings): stage 6.2 delete bridge_to_old seam
bridge.rs (818 LOC) is gone. Production consumers no longer produce a
full legacy `Settings` from v2 state; every read path walks the v2 tree
directly or uses one of the narrow v2->runtime helpers in the new
`settings::v2::to_runtime` module.

Core moves:

fabro-types
- Delete `settings::v2::bridge::bridge_to_old` and the whole bridge.rs
  file.
- Relocate the narrow v2->runtime helpers (`bridge_sandbox`,
  `bridge_mcp_entry`, `bridge_mcps`, `bridge_hook`, `bridge_worktree_mode`,
  `bridge_merge_strategy`, `bridge_pull_request`, `bridge_run_artifacts`)
  into a new `settings::v2::to_runtime` module. Each helper takes a
  single v2 subtree and produces the corresponding runtime shape;
  nothing assembles a full legacy `Settings` anymore.
- `settings/mod.rs` doc comment rewritten to describe `Settings` as a
  runtime shape, not a resolved parse target. Stage 6.3 deletes it.

fabro-config
- `ConfigLayer::resolve` is gone along with the `TryFrom<ConfigLayer>
  for Settings` impls. Consumers call `.into()` for a `SettingsFile`,
  or `.as_v2()` to borrow one.
- `fabro_config::server::resolve_storage_dir` now takes `&SettingsFile`.

fabro-server
- `api_server_settings` emits the v2 `SettingsFile` JSON shape
  directly instead of bridging to the legacy flat DTO. Stage 6.6
  replaces the shape again with an explicit allow-list DTO.
- `serve.rs`: `load_settings` returns `SettingsFile`;
  `apply_serve_overrides` / `apply_runtime_settings` mutate v2
  subtrees directly; `build_artifact_object_store` walks
  `server.artifacts`; `build_legacy_api_settings` projects the v2
  auth/listen/api subtrees down to the legacy `ApiSettings` shape for
  the (still-legacy) auth resolver.
- `diagnostics::check_crypto` walks `server.auth.api.{jwt,mtls}` and
  `server.listen.tls` directly.
- `web_auth.rs` oauth / register / setup-status / auth-me flows all
  read `server.web`, `server.integrations.github`, and
  `server.auth.web` directly via the v2 accessors. `merge_settings_keys`
  now writes v2 TOML (with `[server.web]`, `[server.integrations.github]`,
  etc.) instead of the legacy v1 top-level keys, and the register
  handler re-parses the freshly-written file back into the in-memory
  `SettingsFile` state.

fabro-cli
- `CommandContext::machine_settings` returns `&SettingsFile`.
- `user_config::load_settings` and friends return `SettingsFile`.
- `user_config::resolve_server_target` / `exec_server_target` /
  `configured_server_target` walk `cli.target.{http,unix}` directly.
  Tests rewritten against v2 TOML fixtures.
- `main.rs` logging init reads `cli.logging.level` / `server.logging.level`
  via v2 accessors.
- `commands/exec.rs` reads `cli.exec.{model,agent}` and builds mcps
  from `cli.exec.agent.mcps` (falling back to `run.agent.mcps`) via
  `to_runtime::bridge_mcp_entry`.
- `commands/pr/mod.rs` calls `github_app_id_str()`.
- `commands/run/create.rs` drops the legacy `.resolve()` call and uses
  `Into::<SettingsFile>::into(...)`.
- `commands/config/mod.rs::legacy_settings_to_v2` is now a real
  reverse-mapping helper that covers `storage`, `scheduler`,
  `integrations.{github,slack}`, `run.model`, `run.inputs`, and
  `cli.output.verbosity`. Stage 6.6 deletes it when the API client
  returns v2 natively.
- `tests/it/cmd/config.rs` tests now walk the v2 tree directly (via
  `cfg.run_model_name_str()`, `cfg.run_inputs()`, `cfg.run_sandbox()`,
  `cfg.run_hooks()`, `cfg.run_agent_mcps()`, `cfg.run_prepare_commands()`,
  `cfg.server_storage_root_str()`, etc.). The `bridge_to_old` test
  helper is gone.
- `tests/it/api/settings.rs` asserts against the v2 JSON shape.

Build, test, and quality gates all green:
- `cargo build --workspace --tests`
- `cargo clippy --workspace -- -D warnings`
- `cargo fmt --check --all`
- `cargo nextest run --workspace`: 3758 / 3758 passed, 182 skipped.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 16:12:59 -04:00
Bryan Helmkamp
367fd9302b refactor(cli): centralize command settings and server access
Add CommandContext to load machine settings once per invocation, cache
server access, and route migrated commands through the shared
ServerStoreClient path instead of reloading settings and reconnecting ad
hoc.
2026-04-08 16:27:25 -04:00