Wrap root CLI errors at the main boundary so fatal diagnostics use miette's styled renderer while preserving existing telemetry, exit codes, and auth help hints.
Add fabro-static::EnvVars as the shared registry for fixed environment variable names and migrate env reads, clap env bindings, and subprocess/test allowlists to use it.
Add clippy bans for raw std::env lookup APIs so future dynamic env facades must be documented explicitly.
Rewind now creates a resumable replacement run from the selected checkpoint, archives the source run, and records run.superseded_by for auditability. Fork, rewind, and timeline listing now share server-backed git-store plumbing, with generated API clients and docs updated for the new contract.
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.
Abandoning `fabro pr list`. Deletes:
- lib/crates/fabro-cli/src/commands/pr/list.rs
- lib/crates/fabro-cli/tests/it/cmd/pr_list.rs
- PrListArgs struct + PrCommand::List variant + dispatch arm + name
- The `client()` accessor + `client` field on ServerSummaryLookup
(`pr list` was the only consumer)
- `### fabro pr list` section in docs/reference/cli.mdx
- `list` row + alias from the `fabro pr --help` snapshot test
Server side untouched: there was no `/pull_requests` endpoint to
remove. Historical changelog and plan docs left as-is — they record
when the command shipped, not its current existence.
Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4581 passed (down from 4584 by the
3 deleted pr_list tests), 182 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
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>
FINALIZE loaded the run event log twice: once via build_conclusion_from_store
for stage durations, then again to count ArtifactCaptured events. Merged into
a single walk feeding both the conclusion and the artifact count.
Collapsed six near-identical pipeline::execute + emit_terminal + flush blocks
in test_support into one execute_and_emit_terminal helper. Also trimmed
narrative comments that described caller ordering, control flow, or the fix
commit rather than non-obvious invariants.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `WorkflowRunCompleted` / `WorkflowRunFailed` event was emitted from
`EventLifecycle::on_run_end`, a callback the executor fires at the end of
the EXECUTE phase. But the run isn't done at that point — RETRO and
FINALIZE still need to run, and FINALIZE writes the meta branch's finalize
commit. Observers that treat the event as "done" (CLI attach, daemon SSE
consumers) could observe terminal state and act on it before the worker
flushed its remaining writes.
The recovery scenario test exposed this: it deletes the meta branch
right after `fabro run` returns, then asserts the branch is empty. On
loaded CI runners the worker's finalize commit landed after the delete,
recreating the branch and failing the assertion.
Move the terminal event emission to `pipeline::finalize::finalize`, after
`write_finalize_commit`. The lifecycle's `on_run_end` overrides for event
and git become empty (deleted — the trait already provides a no-op
default). Three pieces of cross-cutting state (`final_patch`,
`captured_artifact_count`, the dead `EventLifecycle` reads of
`last_git_sha`) only existed to ferry data from EXECUTE to the terminal
event; deleted those too. The aggregator collapses to a one-line
delegate to `hook.on_run_end`.
`write_finalize_commit` now takes the conclusion as a parameter and
injects it into the projection copy, since the terminal event hasn't run
through the run store yet when the meta branch is written.
`build_terminal_event` is `pub(crate)` so `test_support` helpers (which
stop at EXECUTE) can mirror the production payload.
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>
- fabro-client: collapse identical match arms for DevToken/Worker bearer
- fabro-hooks: rewrite filter_map(bool::then) as filter().map() chain
- fabro-sandbox: import WORKER_SECRET_ENV_DENYLIST rather than absolute path
- fabro-server: box large execute_run_in_process future; take path: &str in
test-only bearer_request; use let-else in session-secret test; replace unit
pattern _ with () in worker_token request_parts helper; import StatusCode
- fabro-cli run/mod.rs: box large runner::execute future
- fabro-cli worker_auth.rs: drop unused async on shutdown, allow
clippy::unwrap_used at file level for subprocess test harness setup
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `fabro store dump` -> `fabro dump` rename left `commands/store/` as a
vestigial directory with a stale one-line `StoreRunExport` alias. Move
`dump.rs` and `rebuild.rs` up to `commands/`, import `RunDump` directly,
rename `dump::dump_command` -> `dump::run`, and clean up stale docs and
a noise test that only asserted clap's default error output.
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>
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>
The dev-token gating commit added ensure_home_server_auth_methods only to
run_cmd/create_cmd helpers, but many integration tests use context.command()
directly to invoke run/start/attach/etc. Patch the offenders rather than
hoisting auth-injection into command() itself, since command() is also used
by tests (e.g. uninstall) that explicitly want a stable settings file.
- attach, start, scenario lifecycle/recovery, json_global graph: call
context.ensure_home_server_auth_methods() up front
- validate(): hoist into the helper itself, since every validate test
needs it
- server_status, uninstall legacy-record tests: bake methods=["dev-token"]
into their hand-written settings.toml fixtures and pass FABRO_DEV_TOKEN
via env so the spawned server actually boots
- install: write_artifact_store_metadata_creates_marker test fixture also
needs explicit methods after the resolver became strict
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>
`--local` is supposed to render the settings that apply to the local CLI
/ client side, so it has no business resolving server settings. Drop the
server section and the warning path, return only project/workflow/run/
cli/features. Removes the boundary violation that was about to break the
CI boundary check, and restores the legacy_*_silently_ignored tests to
their original silent assertion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
