Reuse credential-source APIs across provider discovery and tests, target known-provider LLM client resolution, and remove duplicated hook/source plumbing from the review cleanup.
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>
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>
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.
Add shared axum/reqwest response assertion helpers in fabro-test,
migrate the Rust HTTP test surface to use them, and document the
new rule in the testing strategy.
Add the server-side CLI OAuth endpoints and token persistence needed to
mint JWT access tokens and rotating refresh tokens from the existing
GitHub web auth flow.
Add CLI auth storage plus `fabro auth login`, `logout`, and `status`, and
prefer stored OAuth access tokens when building target clients.
Two coverage gaps closed:
1. `parse_head_show_output` — extracted from `resolve_head_sha_and_time`
as a pure function so it can be tested without a sandbox. Six tests
cover: well-formed sha+iso line, non-UTC timezone normalization,
sha-only output (missing %cI), malformed date (parser tolerates
and returns sha with None date), empty-input rejection, and
surrounding-whitespace tolerance. New code from the simplify pass,
previously unverified.
2. `fetch_blob_table` two-phase error isolation — `ScriptedBlobSandbox`
(hand-written minimal Sandbox impl) returns different exec responses
for `cat-file --batch-check` vs `cat-file --batch`. The phase-2
failure test proves that a malformed --batch parse outcome doesn't
corrupt phase-1-classified oversized entries — the doc-comment's
promise that the two phases are isolated now has a regression test
behind it. The phase-1-skip test enforces the
METADATA_PHASE_SHA_THRESHOLD contract by making phase 1's
batch-check response an error: if the threshold logic regressed
and phase 1 ran, the test would fail with a 503.
Also adds `Debug` to `ApiError` (required by `Result::expect` in the
new tests) and adds `async-trait`/`tokio-util` as dev-dependencies
plus the `test-support` feature on fabro-sandbox.
Total workspace test count: 4173 -> 4180.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three follow-ups from verification against the actual @pierre/diffs
1.1.15 type definitions:
1. Deep-link expand uses `options.expandUnchanged: true` on the
targeted MultiFileDiff rather than firing `el.click()` on the outer
wrapper. Pierre 1.1.x exposes no imperative expand API — click on
the row container was a no-op. Per-file expansion now fires on
mount when the file name matches the URL hash.
2. Enter/Space binding removed from useFileKeyboardNav — click on the
outer row doesn't trigger anything in pierre's model, and binding
it just delayed default browser scroll behavior on Space. j/k
focus navigation remains the working keyboard affordance. When a
pierre imperative expand API appears, Enter/Space can be re-added
to call it.
3. normalize_for_match strip loop now iterates to a fixed point
against the fully-lowercased string so repeated `./` / `../` / `/`
prefixes are all stripped. Added Windows-path and Unicode-uppercase
regression tests for is_sensitive to verify basename matching
survives both.
Virtualizer usage verified against the 1.1.x type definitions: the
`{ children: ReactNode }` signature accepts the wrapped file list
directly with no Virtualizer.Item wrapper needed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves the sensitive-path denylist, sandbox-git env helper, and metrics
emitter into a dedicated run_files_security module so the Run Files
Changed endpoint has a single, testable surface for security controls.
Denylist upgrades to globset::GlobSet with two explicit lists:
- Basename globs: .env, .env.*, *.pem, id_rsa, id_rsa.*, id_ed25519*,
*.p12, *.keystore, *.key
- Path-suffix globs: .aws/credentials, .git/config, .ssh/**
Matching semantics explicitly pinned:
- Case-insensitive via lowercased normalization
- Path traversal (`../`, `./`, leading `/`) stripped before match
- Basename globs match the final segment only — prevents
`log/.env_audit/data.txt` from matching `.env.*`
- Empty/pathological paths fail closed (sensitive=true safe default)
Also ships:
- sandbox_git_env() returning the env-hardening map
- RunFilesMetrics struct + emit() so tracing never leaks paths/contents
Handler migrates to consume the new module; inline denylist and inline
info!() call removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
