Reuse the existing merge strategy type across CLI/API/GitHub paths, consolidate repeated PR command setup, and serialize server-side PR creation per run to avoid duplicate external work.
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>
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.
Introduce PullRequestApiError with a structured NotFound variant and an
Other(String) catch-all for non-classified failures. Update
get_pull_request, merge_pull_request, and close_pull_request to return
the new type so callers can branch on shape rather than substring.
Server PR handlers now match Err(PullRequestApiError::NotFound { .. })
to map a missing GitHub PR to the existing github_pull_request_not_found
ApiError, removing three err.contains("not found") substring checks.
The Display impl for NotFound preserves the prior message format
("Pull request #N not found in owner/repo") so logging and the
catch-all BAD_GATEWAY response keep their human-readable text.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reconciles origin's "emit terminal event from FINALIZE" refactor
(41c47dbe1, e8a89ac39, 904c8842f) with the local RunServices refactor.
finalize() now performs origin's single list_events walk for stage
durations + artifact count, origin's compute_final_patch, deduped
stages/billing via billing_from_checkpoint, and origin's terminal event
emission — but reads run_store/sandbox/emitter from the shared
RunServices instead of individual Retroed fields. services.emitter.notice
replaces origin's local emit_run_notice helper.
test_support's execute_and_emit_terminal (added by origin) now accesses
run_store/emitter via executed.engine.run.* since Executed bundles
EngineServices. execute/tests.rs drops the terminal-event status
assertion origin deleted — status is no longer set at EXECUTE end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move PullRequestDetail, PullRequestGithubDetail, PullRequestUser,
PullRequestRef, and MergeMethod into fabro-types. Register them as
fabro-api with_replacement targets so the OpenAPI client and the server
share one canonical type per concept.
PullRequestDetail composes a stored PullRequestRecord with a flattened
PullRequestGithubDetail mirroring GitHub's REST payload, removing the
hand-rolled pull_request_detail_json builder in the server. Change the
PullRequestRef wire field from `ref_name` to `ref` so the same Rust
type round-trips through both GitHub and our API without aliases.
The server now uses fabro_api::types::{Create,Merge,Close}* directly,
deleting the hand-defined request/response shadows and the
`body.method.parse::<...>()` call (the typed MergeMethod enum drives
deserialization). Drops fabro-cli's `i64::try_from(record.number)`
panic path and the AutoMergeMethod enum (replaced by MergeMethod).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Integrates origin's worker-JWT-auth work (commits 8a6f83bb0..c847a828d)
with the config-boundary refactor that landed locally. Conflicts
resolved:
- commands/dump.rs: take origin's removal of the 500-line in-process
test block (replaced by real-server integration coverage).
- commands/run/runner.rs: keep local's dense WorkflowSettings import,
drop dead SettingsLayer import, pull in origin's ActorRef.
- manifest_builder.rs: adopt origin's lifted working_directory
resolution (fixes#159 - manifest git detection in nested repos),
but via local's resolve_working_directory_from_run API that takes
the dense RunNamespace. Update the regression test's
ManifestBuildInput literal to local's run_overrides/cli_overrides
field shape.
- server.rs: keep origin's jwt_auth_mode/jwt_auth_state/
test_user_subject/issue_test_user_jwt/issue_test_worker_token/
create_run_with_bearer/bearer_request test helpers, adapt
jwt_auth_state to local's create_test_app_state_with_session_key
signature (ServerSettings + RunLayer), keep local's dense
canonical_origin_settings that returns ServerSettings via
server_settings_from_toml. Rewrite
build_app_state_requires_session_secret_for_worker_tokens against
the dense AppStateConfig (resolved_settings +
resolved_runtime_settings_for_tests).
Post-merge verification: workspace builds clean, cargo +nightly
fmt --check all clean, cargo +nightly clippy --workspace
--all-targets -- -D warnings clean, cargo nextest run --workspace
4560 tests passed.
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>
The worker subprocess is spawned with env_clear+allowlist by the server, so
the only sensitive value in its env is FABRO_WORKER_TOKEN itself. Read the
token and remove_var it from the process env in main() before Tokio starts
worker threads, then thread it explicitly through runner::execute(&str).
Every descendant (hooks, local sandbox, devcontainer initializeCommand,
MCP stdio, etc.) now inherits a worker env with no bearer in it, so an
unscrubbed spawn site cannot leak the token. This makes the prior denylist
scrub in fabro-hooks and fabro-sandbox redundant — delete it and the shared
WORKER_SECRET_ENV_DENYLIST constant. The sandbox keeps its _api_key/_secret/
_token/_password/_credential suffix heuristic for user-supplied env_vars
hygiene.
Extend the server-dispatched-worker env-leak integration test to also
assert a Bash stage running in the worker does not observe FABRO_WORKER_TOKEN.
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>
- 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>
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>
Add FABRO_SUPPRESS_OPEN_BROWSER env knob via fabro_util::browser::try_open.
apply_test_isolation now sets it, so install-mode and auth-login tests that
spawn a real fabro binary no longer pop real browser windows. All six
open::that call sites route through the helper; consolidates the direct
open crate dep into fabro-util.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Returns exit code 4 whenever the CLI fails because the user needs to run
fabro auth login, so scripts and the install wizard can distinguish
re-auth from generic failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Use hex crate for [u8; 32] RecordId instead of hand-rolled loops
- Drop dead prefix_segments cache field; key assembly consumes
R::PREFIX.split('/') directly, removing an intermediate Vec<&str>
- Cache BlobStore on RunDatabaseInner (built once in open_writer/
open_reader via a new build() helper) instead of per-blob construction
- Trim the replay_revocations doc comment to drop a stale plan reference
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces hand-written K/V stores in fabro-store with a shared Record trait
plus Repository<R> typed K/V layer. Adds KeyedMutex for per-key serialization
and transaction() for all-or-nothing WriteBatch commits. Renames
SlateAuthCodeStore/SlateAuthTokenStore to AuthCodeStore/RefreshTokenStore and
adds BlobStore and RunCatalogIndex wrappers on top of Repository. Deletes
catalog.rs in favor of RunCatalogIndex. Database gains blobs() and
catalog_index() accessors; auth_tokens() is renamed refresh_tokens().
Plan: docs/plans/2026-04-20-003-refactor-fabro-store-record-abstractions-plan.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the plan at
docs/plans/2026-04-20-003-refactor-unify-run-vocabulary-metadata-plan.md.
- Rename RunRecord to RunSpec and RunProjection.run to .spec everywhere
in Rust source, tests, helpers, test names, and error messages.
- Introduce SerializableProjection wrapper that trims bulky node text
fields (prompt, response, diff, stdout, stderr) for run.json snapshots.
- Collapse metadata-branch and CLI export to one RunDump::from_projection
builder emitting run.json + graph.fabro + stages/{stage_id}/... and
drop legacy top-level start/status/checkpoint/sandbox/retro/conclusion
split files.
- Replace MetadataStore::write_checkpoint with write_snapshot returning
the commit SHA; add read_run_projection/read_run_spec; demote
read_checkpoint/read_start_record to projection-field extractors.
- Switch fork, rewind, rebuild_meta, CLI rewind recovery, and retro
upload to read the unified projection layout.
- Add additive query methods on RunSpec and RunProjection.
Serde-level `alias = "spec"` shim dropped; `rename = "run"` retained to
keep the server API wire format stable per the plan's scope boundary.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lift shared client DTOs into fabro-types, move auth/target/error/session
logic into fabro-client, and reduce fabro-cli to orchestration around the
builder-based client path.
This also lands the remaining plan cleanup for ApiError, ServerTarget
canonicalization, and the RunEventStream rename at the CLI boundary.
Delete the CliTargetTls settings, ClientTlsSettings struct, rustls-pemfile
dep, and the fabro-http wrapper methods (use_rustls_tls/identity/
add_root_certificate) that only the CLI client-auth path used. The server
no longer terminates TLS in-process and the CliAuthStrategy::Mtls variant
had no construction or match sites.
Also simplify ServerTarget::HttpUrl to a tuple variant (HttpUrl(String))
now that tls is gone, removing the struct-variant ceremony across 22
construction and destructure sites.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Harden the CLI browser auth flow by moving auth-code issuance behind
an explicit same-origin confirmation step, and update the real-browser
test harness to submit the confirmation page.
Add the missing refresh transport guard, actionable auth-store lock errors
for unsupported filesystems, explicit OAuth state expiry, and the remaining
CLI auth regression coverage around replay revocation, HTML headers, and
secret-safe logging.
