From 90b911c92775652fff70cc5151a12c53c190836a Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 20 Apr 2026 18:26:46 -0400 Subject: [PATCH 1/5] docs(plan): clarify unknown-event handling in client extraction Document that unknown Fabro run events already fall back through EventBody::Unknown, and that the refactor only needs to preserve that behavior during the EventEnvelope move. --- ...20-002-refactor-extract-fabro-client-crate-plan.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md b/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md index 931b9d497..9a431d4f5 100644 --- a/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md +++ b/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md @@ -57,7 +57,7 @@ These were all reasonable while the client had exactly one caller. They prevent - Writing a second consumer of `fabro-client` (SDK example, IDE integration, etc.) — the crate exists to enable that, not to deliver it. - Changing wire semantics (OpenAPI contract stays identical). - Migrating existing `~/.fabro/auth.json` entries. The canonical key format for HTTP targets remains identical (same `normalized_http_base_url` logic); Unix-socket canonicalization changes from `fs::canonicalize` (symlink-resolving) to lexical-only (`.`/`..` cleanup, no symlink chasing). Existing sessions against socket paths written as absolute non-symlinked paths continue to match. Users with symlinked socket paths may need to re-login — acceptable per `CLAUDE.md` ("We don't care about migration"). -- (Previously noted as out of scope; now in scope per review feedback.) `RunEvent` unknown-variant handling: add a fallback `Unknown` variant so older CLIs against newer servers survive deserialization. Covered under Unit 2. +- `RunEvent` unknown-variant handling. Already present today via `EventBody::Unknown` at `lib/crates/fabro-types/src/run_event/mod.rs:695` (covered by `:974`). Unit 2 verifies the `EventEnvelope` restructure preserves this existing behavior — no new variant, no new code. - Splitting the ~40 client methods into separate `client/runs.rs`, `client/secrets.rs`, etc. modules. A single `client.rs` is acceptable at ~900 lines; cosmetic domain splitting can happen later if the file grows. ## Context & Research @@ -107,7 +107,7 @@ None. Internal refactor; the patterns are all established locally. ``` On refresh failure modes that previously triggered fallback, `OAuthSession` calls `fallback.resolve()`. If `Some(Credential)`, the session rebuilds the client bundle with that credential; if `None`, it surfaces "session expired." The CLI provides a concrete impl in `fabro-cli` that wraps the existing dev-token-loading logic. Named trait (vs bare `Fn` closure) makes the role obvious at the `OAuthSession::builder` call site. -- **`fabro_types::RunEvent` gains unknown-variant tolerance.** Add `#[serde(other)] Unknown` (or `#[serde(untagged)] Unknown(serde_json::Value)` depending on the enum shape) so a newer server's unknown event type doesn't break an older CLI at deserialize. Preserves today's accidental forward-compat that came from storing events as raw JSON. Small addition to the event enum; benefit extends beyond this plan (any event-consuming path becomes version-mismatch-tolerant). +- **`fabro_types::RunEvent` already tolerates unknown event names.** `EventBody::Unknown { name, properties }` at `lib/crates/fabro-types/src/run_event/mod.rs:695` is the existing fallback — a newer server's unknown event type deserializes into this variant without error. Unit 2's responsibility is to *preserve* this behavior across the `EventEnvelope { seq, #[serde(flatten)] event: RunEvent }` restructure, not to add a new variant. - **Phased delivery.** DTO lift must land before the client extraction — otherwise `fabro-client`'s deps can't be correct. We split the work into three phases (DTO lift → `fabro-client` extraction → `fabro-cli` rewire) and land each phase as a coherent chunk. Each phase compiles and tests pass at its boundary. ## Open Questions @@ -117,7 +117,7 @@ None. Internal refactor; the patterns are all established locally. - **Where do `apply_events`/`apply_event` live after `RunProjection` moves?** In `fabro-store` as an extension trait `RunProjectionReducer` implemented on `fabro_types::RunProjection`. Preserves OOP-style method-call syntax at call sites via `use fabro_store::RunProjectionReducer;`. Chosen over free functions because the workspace leans toward methods over free helpers. - **HTTP canonicalization scope in `ServerTarget::from_url`.** Preserve today's full normalization: lowercase scheme + lowercase host + strip default ports + strip trailing `/` + strip `/api/v1` suffix + rebuild authority. Matches today's `canonical_http_target` in `auth_store.rs:350-383` and the existing `https_normalization_collapses_equivalent_urls` test at line 507. Simpler "strip slash + api-v1 only" form was rejected because it would silently invalidate AuthStore entries for users whose URLs differed only in case or default-port. - **OAuthSession refresh fallback mechanism.** Add a `CredentialFallback` trait; `fabro-cli` implements it against its dev-token sources; `OAuthSession` holds `Option>` and calls `fallback.resolve()` on the failure modes that previously fell back to dev-token. Named trait (not bare `Fn`) chosen for role clarity. -- **`RunEvent` unknown-variant forward-compat.** Add an `Unknown` fallback variant in `fabro_types::RunEvent` so the `EventEnvelope` restructure doesn't regress today's accidental pass-through tolerance for unknown event types. Addressed in Unit 2. +- **`RunEvent` unknown-variant forward-compat.** Already handled today via `EventBody::Unknown` at `lib/crates/fabro-types/src/run_event/mod.rs:695` (tested at `:974`). Unit 2 verifies the `EventEnvelope` restructure preserves this existing behavior — no new variant, no new code. - **Does `EventEnvelope` need `EventPayload` to travel with it?** No. OpenAPI already defines the wire shape as `seq + RunEvent flattened`. `EventPayload` is a storage-internal validation helper and stays behind. - **Does `AuthStore` need a trait-based abstraction?** No. Concrete type with a configurable file path is sufficient; we extract a trait the day a second implementation exists. - **Does `ArtifactUpload` live in `fabro-types` or `fabro-client`?** `fabro-types`. Source is `fabro-workflow` (capture) → sink is `fabro-client` (upload). Placing the DTO in `fabro-types` prevents `fabro-workflow` from having to depend on `fabro-client`. @@ -301,7 +301,6 @@ Client::builder() Construction sites (switch to `fabro_types::EventEnvelope { seq, event }`): - Create: `lib/crates/fabro-types/src/event_envelope.rs` - Modify: `lib/crates/fabro-types/src/lib.rs` (module + re-export) -- Modify: `lib/crates/fabro-types/src/run_event/` (add `Unknown` fallback variant — exact form depends on current enum tagging) - Modify: `lib/crates/fabro-store/src/types.rs` (delete old `EventEnvelope`; keep `EventPayload` internal; update tests at lines 127 and 171) - Modify: `lib/crates/fabro-store/src/slate/run_store.rs` (production constructors at lines 194 and 342) - Modify: `lib/crates/fabro-store/src/run_state.rs` (apply_event body at line 79; test fixtures at lines 728, 741, 1086, 1112; test assertions at lines 1133, 1137) @@ -343,7 +342,7 @@ Write-path `EventPayload::new` sites (UNAFFECTED — stay in fabro-store as inte pub event: RunEvent, } ``` -- Add unknown-variant tolerance to `fabro_types::RunEvent` so the read-path typed deserialization doesn't regress today's forward-compat pass-through behavior. Shape depends on `RunEvent`'s current enum tagging — likely a catch-all `Unknown(serde_json::Value)` variant with `#[serde(other)]` or an `#[serde(untagged)]` fallback. Inspect the existing `fabro_types::RunEvent` definition during implementation and pick the minimal form that preserves lossless round-trip of unknown payloads. +- Confirm today's `EventBody::Unknown { name, properties }` fallback (at `lib/crates/fabro-types/src/run_event/mod.rs:695`) continues to round-trip through the new `EventEnvelope { seq, #[serde(flatten)] event: RunEvent }` shape — the `#[serde(flatten)]` interacts cleanly with the existing catch-all. No new variant is added; we are only confirming the existing forward-compat behavior survives. - `fabro-store` keeps `EventPayload` strictly internal — every existing write path continues to call `EventPayload::new(value, run_id)?` for `expected_run_id` validation before persisting (unchanged behavior). No write path bypasses that check. Only the external-facing read-path return type changes. - At the store's read boundary (`slate/run_store.rs:194, 342` — where the old `EventEnvelope { seq, payload }` was constructed by pairing a separately-tracked seq with decoded payload bytes): deserialize the raw bytes as `RunEvent` (the on-disk format is raw RunEvent JSON without `seq`), then wrap in `fabro_types::EventEnvelope { seq, event }`. The write path at `slate/run_store.rs:201-203` continues to persist `serde_json::to_vec(payload)?` of the `EventPayload`; the `seq` comes from a separate counter, same as today. - CLI callers that did `event.payload.as_value()` + `RunEvent::from_ref(...)` → change to `&event.event`. This is a win at every call site (fewer conversions). @@ -357,7 +356,7 @@ Write-path `EventPayload::new` sites (UNAFFECTED — stay in fabro-store as inte - Happy path: `EventEnvelope` round-trips through `serde_json` preserving both `seq` and all `RunEvent` fields, and deserializes the existing wire format identically. - Integration: existing `fabro-store` test `wire_event_envelope_round_trips` still passes after porting (adapt it to construct the new struct shape). - Integration: CLI integration tests that consume events over SSE or HTTP (e.g., `fabro-cli/tests/it/workflow/mod.rs`, `cmd/attach.rs`) continue to pass — proves the wire shape is unchanged. -- Forward-compat: an `EventEnvelope` JSON payload whose `type` (or equivalent discriminator) is unknown to this build of `fabro-types` deserializes into a `RunEvent::Unknown` fallback variant without error, and re-serializes losslessly. +- Forward-compat regression guard: an `EventEnvelope` JSON payload whose event name is unknown to this build of `fabro-types` deserializes into the existing `EventBody::Unknown { name, properties }` without error and re-serializes losslessly. (Covers: the `EventEnvelope` restructure did not break today's fallback at `run_event/mod.rs:695`.) **Verification:** - `cargo build --workspace` succeeds. From f0f04abf44aa80c69f969673a83ff1cd474612bc Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 20 Apr 2026 20:42:20 -0400 Subject: [PATCH 2/5] refactor(client): extract fabro-client crate 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. --- Cargo.lock | 27 + ...efactor-extract-fabro-client-crate-plan.md | 25 +- lib/crates/fabro-cli/Cargo.toml | 1 + .../fabro-cli/src/commands/auth/login.rs | 24 +- .../fabro-cli/src/commands/auth/logout.rs | 74 +- .../fabro-cli/src/commands/auth/status.rs | 15 +- lib/crates/fabro-cli/src/commands/doctor.rs | 2 +- lib/crates/fabro-cli/src/commands/exec.rs | 4 +- .../fabro-cli/src/commands/run/attach.rs | 14 +- .../fabro-cli/src/commands/run/create.rs | 9 +- lib/crates/fabro-cli/src/commands/run/logs.rs | 8 +- .../fabro-cli/src/commands/run/runner.rs | 15 +- .../fabro-cli/src/commands/store/rebuild.rs | 2 +- .../fabro-cli/src/commands/system/events.rs | 2 +- lib/crates/fabro-cli/src/commands/version.rs | 10 +- lib/crates/fabro-cli/src/main.rs | 3 - lib/crates/fabro-cli/src/server_client.rs | 1750 ++--------------- lib/crates/fabro-cli/src/server_runs.rs | 3 +- lib/crates/fabro-cli/src/user_config.rs | 73 +- lib/crates/fabro-cli/tests/it/cmd/attach.rs | 1 + lib/crates/fabro-cli/tests/it/cmd/rewind.rs | 17 +- lib/crates/fabro-cli/tests/it/cmd/runner.rs | 2 +- lib/crates/fabro-cli/tests/it/cmd/support.rs | 9 +- lib/crates/fabro-cli/tests/it/workflow/mod.rs | 11 +- lib/crates/fabro-client/Cargo.toml | 37 + .../src/auth_store.rs | 229 +-- lib/crates/fabro-client/src/client.rs | 1392 +++++++++++++ lib/crates/fabro-client/src/credential.rs | 40 + lib/crates/fabro-client/src/error.rs | 184 ++ lib/crates/fabro-client/src/lib.rs | 26 + .../src/loopback.rs} | 64 +- lib/crates/fabro-client/src/session.rs | 48 + .../{fabro-cli => fabro-client}/src/sse.rs | 2 +- lib/crates/fabro-client/src/target.rs | 220 +++ lib/crates/fabro-retro/src/retro_agent.rs | 2 +- lib/crates/fabro-server/src/server.rs | 48 +- lib/crates/fabro-store/src/lib.rs | 8 +- lib/crates/fabro-store/src/run_state.rs | 387 ++-- lib/crates/fabro-store/src/slate/mod.rs | 9 +- lib/crates/fabro-store/src/slate/run_store.rs | 12 +- lib/crates/fabro-store/src/types.rs | 130 +- lib/crates/fabro-types/src/artifact.rs | 30 + lib/crates/fabro-types/src/event_envelope.rs | 135 ++ lib/crates/fabro-types/src/lib.rs | 8 + lib/crates/fabro-types/src/run_projection.rs | 109 + lib/crates/fabro-types/src/run_summary.rs | 56 + .../fabro-workflow/src/artifact_snapshot.rs | 22 +- .../fabro-workflow/src/artifact_upload.rs | 6 +- lib/crates/fabro-workflow/src/event.rs | 2 +- lib/crates/fabro-workflow/src/lib.rs | 18 +- .../fabro-workflow/src/lifecycle/artifact.rs | 10 +- .../fabro-workflow/src/operations/create.rs | 5 +- lib/crates/fabro-workflow/src/run_lookup.rs | 4 +- 53 files changed, 2890 insertions(+), 2454 deletions(-) create mode 100644 lib/crates/fabro-client/Cargo.toml rename lib/crates/{fabro-cli => fabro-client}/src/auth_store.rs (68%) create mode 100644 lib/crates/fabro-client/src/client.rs create mode 100644 lib/crates/fabro-client/src/credential.rs create mode 100644 lib/crates/fabro-client/src/error.rs create mode 100644 lib/crates/fabro-client/src/lib.rs rename lib/crates/{fabro-cli/src/loopback_target.rs => fabro-client/src/loopback.rs} (71%) create mode 100644 lib/crates/fabro-client/src/session.rs rename lib/crates/{fabro-cli => fabro-client}/src/sse.rs (94%) create mode 100644 lib/crates/fabro-client/src/target.rs create mode 100644 lib/crates/fabro-types/src/artifact.rs create mode 100644 lib/crates/fabro-types/src/event_envelope.rs create mode 100644 lib/crates/fabro-types/src/run_projection.rs create mode 100644 lib/crates/fabro-types/src/run_summary.rs diff --git a/Cargo.lock b/Cargo.lock index c779b34bf..7af5fd082 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1616,6 +1616,7 @@ dependencies = [ "fabro-api", "fabro-auth", "fabro-checkpoint", + "fabro-client", "fabro-config", "fabro-devcontainer", "fabro-github", @@ -1682,6 +1683,32 @@ dependencies = [ "x509-parser", ] +[[package]] +name = "fabro-client" +version = "0.208.0-nightly.1" +dependencies = [ + "anyhow", + "bytes", + "chrono", + "fabro-api", + "fabro-http", + "fabro-model", + "fabro-types", + "fabro-util", + "fs2", + "futures", + "libc", + "progenitor-client", + "rand 0.9.4", + "serde", + "serde_json", + "tempfile", + "thiserror 2.0.18", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "fabro-config" version = "0.208.0-nightly.1" diff --git a/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md b/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md index 9a431d4f5..04faa0652 100644 --- a/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md +++ b/docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md @@ -41,6 +41,19 @@ These were all reasonable while the client had exactly one caller. They prevent - R8. `fabro-cli` keeps CLI-owned orchestration: subprocess autostart, server-record lookup, dev-token-from-disk loading, `[cli.target]` TOML resolution, and the wrapper that stitches these together into a ready-to-use `fabro_client::Client`. - R9. The full workspace build passes (`cargo build --workspace`), clippy is clean (`cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings`), rustfmt check passes, and all workspace tests continue to pass (`cargo nextest run --workspace`). +## Review Adjustments + +- Remaining gaps against this plan in the current landed code: + - `HttpResponseFailure` → `ApiError` rename is still open. + - `ServerTarget` canonical-by-construction / lexical-only Unix-path canonicalization is still open; canonicalization still partly lives in `AuthStore`. + - `RunAttachEventStream` → `RunEventStream` rename is still open at the CLI boundary. + - Because the two rename cleanups above are still open, Unit 11's "no leaked old names" grep is currently expected to fail until that follow-up cleanup lands. +- Clarifications after implementation review: + - `RunProjection` ownership moved to `fabro-types`; external callers may continue to import it through `fabro_store`'s re-export. Verification should check type ownership and dependency boundaries, not the literal import spelling. + - `Client::from_http_client(...)` is the required public constructor. `Client::new_no_proxy(...)` may remain as a convenience wrapper, primarily for tests. + - `TransportConnector` is an acceptable internal helper inside `fabro-client` when needed to preserve caller-specific transport configuration across refresh/rebuild flows. + - `fabro-cli` may retain multiple thin `connect_*` convenience wrappers so long as they preserve CLI-only orchestration and funnel into `Client::builder()` under the hood. + ## Scope Boundaries **In scope:** @@ -96,7 +109,8 @@ None. Internal refactor; the patterns are all established locally. - **`ServerTarget` canonical-by-construction, lexical only.** `ServerTarget::from_url` applies today's full HTTP canonicalization inline: lowercase scheme, lowercase host, strip default ports (`:443` on https, `:80` on http), strip trailing `/`, strip `/api/v1` suffix, rebuild authority as `{scheme}://{host}[:{port}]`. `ServerTarget::from_unix_path` applies lexical `.`/`..` resolution (no FS access, no symlink chasing). `PartialEq`/`Eq`/`Hash` operate on the canonical form directly. `ServerTargetKey` is deleted; `ServerTarget` itself serves as the `AuthStore` map key. In OOP style: construction *is* the canonicalizer — no separate `canonicalize()` method. Related helpers attach to `ServerTarget` as inherent methods (`target.loopback_classification()`, `target.build_public_http_client()`) rather than free functions. - **`AuthStore` moves to `fabro-client`.** Default path remains `~/.fabro/auth.json` via `fabro_util::Home`. The public API (`get`/`put`/`remove`/`list`) is narrow enough that we keep it concrete — no `TokenRefresher` trait abstraction. Callers wanting alternative storage can pass an explicit path to `AuthStore::new`. If external-consumer flexibility becomes a real need later, we extract a trait then, not now. - **Renames applied inline with moves.** We don't do a separate rename pass — the DTOs and internal types are renamed as they move. This keeps the compiler-driven find-all-callers loop honest: every broken import is both a move and a rename in one commit. -- **Connection API collapses to one builder.** Today's four `connect_*` functions in `server_client.rs` are all CLI-opinionated. `fabro-client` exposes a single `Client::builder().target(t).credential(c).oauth_session(s).connect().await?`. The CLI's `connect_server_with_settings` becomes a thin orchestrator: resolve target → autostart if needed → build `Credential` from dev-token/env/AuthStore → call `Client::builder()`. +- **Connection API centers on one builder.** Today's `connect_*` functions in `server_client.rs` are CLI-opinionated convenience wrappers. `fabro-client` exposes `Client::builder().target(t).credential(c).oauth_session(s).connect().await?` as the underlying connection API. `fabro-cli` may keep several thin `connect_*` wrappers, but they must remain orchestration-only and funnel into the builder instead of duplicating transport/session assembly logic. +- **Refresh rebuilds may use a transport connector hook.** If the CLI needs request-transport customization across OAuth refresh rebuilds (for example, preserving a CLI-specific user-agent), `fabro-client` may carry a `TransportConnector`-style helper as an internal implementation detail. This does not count as a second public connection API. - **`OAuthSession` refresh fallback via `CredentialFallback` trait.** Today's refresh flow falls back to a dev-token-from-disk when the OAuth entry is missing, expired, or revoked. That fallback lookup reads CLI-owned sources (`FABRO_DEV_TOKEN` env, `~/.fabro/dev-token`, storage-dir dev-token file, fabro-server pidfile record) that don't belong in `fabro-client`. `OAuthSession` takes an optional `Box` at build time: ```text @@ -121,7 +135,7 @@ None. Internal refactor; the patterns are all established locally. - **Does `EventEnvelope` need `EventPayload` to travel with it?** No. OpenAPI already defines the wire shape as `seq + RunEvent flattened`. `EventPayload` is a storage-internal validation helper and stays behind. - **Does `AuthStore` need a trait-based abstraction?** No. Concrete type with a configurable file path is sufficient; we extract a trait the day a second implementation exists. - **Does `ArtifactUpload` live in `fabro-types` or `fabro-client`?** `fabro-types`. Source is `fabro-workflow` (capture) → sink is `fabro-client` (upload). Placing the DTO in `fabro-types` prevents `fabro-workflow` from having to depend on `fabro-client`. -- **How do we handle the `Client::new_no_proxy(base_url)` constructor used by CLI tests today?** Expose `Client::from_http_client(base_url, http_client)` as a public `pub fn`; the CLI's `new_no_proxy` wrapper stays in `fabro-cli` test code. +- **How do we handle the `Client::new_no_proxy(base_url)` constructor used by CLI tests today?** Expose `Client::from_http_client(base_url, http_client)` as the stable public `pub fn`. `Client::new_no_proxy(base_url)` may remain as a small convenience wrapper (in `fabro-client` or CLI-local test code) if it continues to earn its keep. - **`convert_type` serde round-trip helper — does it stay in the CLI or move with the client?** Moves with the client. It's how the client bridges `fabro_api::types::RunSummary` (wire) → `fabro_types::RunSummary` (domain) at response boundaries. ### Deferred to Implementation @@ -421,7 +435,7 @@ Write-path `EventPayload::new` sites (UNAFFECTED — stay in fabro-store as inte **Verification:** - `cargo build --workspace` succeeds. - `cargo nextest run --workspace` passes. -- `grep -rn "fabro_store::RunProjection" lib/` returns only re-export lines and internal fabro-store uses; external callers use `fabro_types::RunProjection`. +- `grep -rn "struct RunProjection" lib/crates/fabro-store lib/crates/fabro-types` shows the concrete struct definition only in `fabro-types`; external callers may import either `fabro_types::RunProjection` or the `fabro-store` re-export. --- @@ -628,11 +642,12 @@ Write-path `EventPayload::new` sites (UNAFFECTED — stay in fabro-store as inte } ``` The old `ClientBundle` name disappears; `ClientState` is private to the module. -- `Client::builder()`: new public API. Replaces today's four `connect_*` functions that blend CLI opinions with transport. The CLI's own `connect_server_with_settings` becomes a thin orchestrator over `Client::builder()` (handled in Unit 9). +- `Client::builder()`: new public API. It becomes the underlying connection API. The CLI may keep thin `connect_*` orchestration wrappers around it (handled in Unit 9), but transport/session assembly should live in the builder path rather than being duplicated across wrappers. - `RunEventStream` rename: the struct, its `next_event`/`buffer_sse_events` methods, and the `VecDeque` field. `EventEnvelope` is now `fabro_types::EventEnvelope`. - Method bodies: the 40 wrappers move verbatim. They call `send_api(|client| ...)` — `client` is the `fabro_api::ApiClient` from `ClientState`. `convert_type::<_, fabro_types::RunSummary>(...)` continues to bridge wire → domain. -- `Client::from_http_client(base_url, http_client)` — public `pub fn` constructor for test use (replaces today's `new_no_proxy`). The CLI's test code can still build one via this with a `no_proxy()` builder. +- `Client::from_http_client(base_url, http_client)` — public `pub fn` constructor for test use and non-builder callers. `Client::new_no_proxy(base_url)` may remain as a small convenience wrapper built on top of it. - Preserve `send_api`'s 401 → refresh → retry auto-logic. `OAuthSession` owns the refresh state it needs (`target`, `auth_store`, optional `fallback`); the actual refresh HTTP call uses a bespoke HTTP client built via `target.build_public_http_client()` (method on `ServerTarget`, not a free function — OOP style). +- If preserving caller-specific transport behavior across refresh rebuilds requires it, `Client` may carry an internal `TransportConnector` helper that can rebuild the transport with the same customization after credentials change. - `CredentialFallback` trait lives in `fabro-client::credential`: ```text // Directional — not implementation diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index 6aadb6032..4687f7df9 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -39,6 +39,7 @@ graphviz-sys.workspace = true fabro-validate = { path = "../fabro-validate" } fabro-workflow = { path = "../fabro-workflow" } fabro-server = { path = "../fabro-server" } +fabro-client = { path = "../fabro-client" } fabro-api = { path = "../fabro-api" } fabro-telemetry = { path = "../fabro-telemetry" } fabro-store = { path = "../fabro-store" } diff --git a/lib/crates/fabro-cli/src/commands/auth/login.rs b/lib/crates/fabro-cli/src/commands/auth/login.rs index b71437d1f..227ff8936 100644 --- a/lib/crates/fabro-cli/src/commands/auth/login.rs +++ b/lib/crates/fabro-cli/src/commands/auth/login.rs @@ -3,6 +3,7 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use chrono::{DateTime, Utc}; use fabro_api::types; +use fabro_client::{AuthEntry, AuthStore, StoredSubject}; use fabro_http::header::CONTENT_TYPE; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::CliLayer; @@ -11,9 +12,7 @@ use serde::Deserialize; use tokio::time::timeout; use crate::args::{AuthLoginArgs, require_no_json_override}; -use crate::auth_store::{AuthEntry, AuthStore, ServerTargetKey, StoredSubject}; use crate::command_context::CommandContext; -use crate::loopback_target::{LoopbackClassification, is_loopback_or_unix_socket}; use crate::user_config; use crate::user_config::ServerTarget; @@ -56,7 +55,6 @@ pub(super) async fn login_command( { let ctx = CommandContext::base(printer, cli.clone(), cli_layer)?; let target = user_config::resolve_server_target(&args.server, ctx.machine_settings())?; - let server_key = ServerTargetKey::new(&target)?; let config = fetch_cli_auth_config(&target).await?; if !config.enabled { bail!("{}", cli_auth_unavailable_message(config.reason.as_deref())); @@ -98,11 +96,11 @@ pub(super) async fn login_command( } }; - match is_loopback_or_unix_socket(&target)? { - LoopbackClassification::Https - | LoopbackClassification::LoopbackHttp - | LoopbackClassification::UnixSocket => {} - LoopbackClassification::Rejected => { + match target.loopback_classification()? { + fabro_client::LoopbackClassification::Https + | fabro_client::LoopbackClassification::LoopbackHttp + | fabro_client::LoopbackClassification::UnixSocket => {} + fabro_client::LoopbackClassification::Rejected => { bail!("{}", token_transport_error(&target)); } } @@ -123,8 +121,8 @@ pub(super) async fn login_command( logged_in_at: Utc::now(), }; let summary = identity_summary(&entry.subject); - AuthStore::default().put(&server_key, entry)?; - fabro_util::printerr!(printer, "Logged in to {} as {}", server_key, summary); + AuthStore::default().put(&target, entry)?; + fabro_util::printerr!(printer, "Logged in to {} as {}", target, summary); Ok(()) } } @@ -270,11 +268,11 @@ struct OAuthErrorBody { mod tests { use base64::Engine as _; use base64::engine::general_purpose::URL_SAFE_NO_PAD; + use fabro_client::LoopbackClassification; use insta::assert_snapshot; use sha2::{Digest, Sha256}; use super::{build_browser_url, cli_auth_unavailable_message, login_failure_message}; - use crate::loopback_target::{LoopbackClassification, is_loopback_or_unix_socket}; use crate::user_config::ServerTarget; #[test] @@ -343,9 +341,9 @@ mod tests { #[test] fn token_transport_accepts_only_https_loopback_or_unix() { - let target = ServerTarget::HttpUrl("https://fabro.example.com".to_string()); + let target = ServerTarget::http_url("https://fabro.example.com").unwrap(); assert_eq!( - is_loopback_or_unix_socket(&target).unwrap(), + target.loopback_classification().unwrap(), LoopbackClassification::Https ); } diff --git a/lib/crates/fabro-cli/src/commands/auth/logout.rs b/lib/crates/fabro-cli/src/commands/auth/logout.rs index db18379fe..4743eb42b 100644 --- a/lib/crates/fabro-cli/src/commands/auth/logout.rs +++ b/lib/crates/fabro-cli/src/commands/auth/logout.rs @@ -1,11 +1,11 @@ use anyhow::{Result, bail}; +use fabro_client::{AuthEntry, AuthStore}; use fabro_http::header::AUTHORIZATION; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::CliLayer; use fabro_util::printer::Printer; use crate::args::{AuthLogoutArgs, require_no_json_override}; -use crate::auth_store::{AuthEntry, AuthStore, ServerTargetKey}; use crate::command_context::CommandContext; use crate::user_config; use crate::user_config::ServerTarget; @@ -29,13 +29,11 @@ pub(super) async fn logout_command( } let mut warnings = Vec::new(); - for (key, entry) in entries { - if let Ok(target) = server_target_from_key(&key) { - if let Err(error) = revoke_remote_session(&target, &entry).await { - warnings.push(format_warning(&key, &error.to_string())); - } + for (target, entry) in entries { + if let Err(error) = revoke_remote_session(&target, &entry).await { + warnings.push(format_warning(&target, &error.to_string())); } - store.remove(&key)?; + store.remove(&target)?; } for warning in warnings { @@ -46,17 +44,16 @@ pub(super) async fn logout_command( } let target = user_config::resolve_server_target(&args.server, ctx.machine_settings())?; - let key = ServerTargetKey::new(&target)?; - let Some(entry) = store.get(&key)? else { - fabro_util::printerr!(printer, "Not logged in to {}.", key); + let Some(entry) = store.get(&target)? else { + fabro_util::printerr!(printer, "Not logged in to {}.", target); return Ok(()); }; if let Err(error) = revoke_remote_session(&target, &entry).await { - fabro_util::printerr!(printer, "{}", format_warning(&key, &error.to_string())); + fabro_util::printerr!(printer, "{}", format_warning(&target, &error.to_string())); } - store.remove(&key)?; - fabro_util::printerr!(printer, "Logged out from {}.", key); + store.remove(&target)?; + fabro_util::printerr!(printer, "Logged out from {}.", target); Ok(()) } @@ -79,62 +76,21 @@ async fn revoke_remote_session(target: &ServerTarget, entry: &AuthEntry) -> Resu bail!("request failed with status {status}: {body}") } -fn server_target_from_key(key: &ServerTargetKey) -> Result { - let value = key.to_string(); - if let Some(path) = value.strip_prefix("unix://") { - return Ok(ServerTarget::UnixSocket(path.into())); - } - if value.starts_with("http://") || value.starts_with("https://") { - return Ok(ServerTarget::HttpUrl(value)); - } - bail!("invalid auth store server key `{value}`") -} - -fn format_warning(key: &ServerTargetKey, error: &str) -> String { +fn format_warning(target: &ServerTarget, error: &str) -> String { format!( - "Warning: removed local session for {key}, but remote revocation failed: {error}. The refresh token may remain valid until it expires." + "Warning: removed local session for {target}, but remote revocation failed: {error}. The refresh token may remain valid until it expires." ) } #[cfg(test)] mod tests { - use std::path::PathBuf; - - use super::{format_warning, server_target_from_key}; - use crate::auth_store::ServerTargetKey; + use super::format_warning; use crate::user_config::ServerTarget; - #[test] - fn rebuilds_server_target_from_http_key() { - let key = ServerTargetKey::new(&ServerTarget::HttpUrl( - "https://fabro.example.com/api/v1".to_string(), - )) - .unwrap(); - - assert_eq!( - server_target_from_key(&key).unwrap(), - ServerTarget::HttpUrl("https://fabro.example.com".to_string()) - ); - } - - #[test] - fn rebuilds_server_target_from_unix_key() { - let key = ServerTargetKey::new(&ServerTarget::UnixSocket(PathBuf::from("/tmp/fabro.sock"))) - .unwrap(); - - assert_eq!( - server_target_from_key(&key).unwrap(), - ServerTarget::UnixSocket(PathBuf::from("/tmp/fabro.sock")) - ); - } - #[test] fn warning_mentions_local_removal_and_remote_failure() { - let key = ServerTargetKey::new(&ServerTarget::HttpUrl( - "https://fabro.example.com".to_string(), - )) - .unwrap(); - let warning = format_warning(&key, "request failed with status 500"); + let target = ServerTarget::http_url("https://fabro.example.com").unwrap(); + let warning = format_warning(&target, "request failed with status 500"); assert!(warning.contains("removed local session")); assert!(warning.contains("remote revocation failed")); } diff --git a/lib/crates/fabro-cli/src/commands/auth/status.rs b/lib/crates/fabro-cli/src/commands/auth/status.rs index ccdcc2023..de81e60c2 100644 --- a/lib/crates/fabro-cli/src/commands/auth/status.rs +++ b/lib/crates/fabro-cli/src/commands/auth/status.rs @@ -1,5 +1,6 @@ use anyhow::Result; use chrono::{DateTime, Utc}; +use fabro_client::{AuthEntry, AuthStore}; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::CliLayer; use fabro_util::dev_token::{read_dev_token_file, validate_dev_token_format}; @@ -7,7 +8,6 @@ use fabro_util::printer::Printer; use serde::Serialize; use crate::args::AuthStatusArgs; -use crate::auth_store::{AuthEntry, AuthStore, ServerTargetKey}; use crate::command_context::CommandContext; use crate::shared::print_json_pretty; use crate::user_config; @@ -126,7 +126,7 @@ fn all_rows(store: &AuthStore, now: DateTime) -> Result> { Ok(store .list()? .into_iter() - .map(|(key, entry)| status_row(&key, entry, now)) + .map(|(target, entry)| status_row(&target, entry, now)) .collect()) } @@ -135,17 +135,16 @@ fn filter_rows( target: &ServerTarget, now: DateTime, ) -> Result> { - let key = ServerTargetKey::new(target)?; Ok(store - .get(&key)? + .get(target)? .into_iter() - .map(|entry| status_row(&key, entry, now)) + .map(|entry| status_row(target, entry, now)) .collect()) } -fn status_row(key: &ServerTargetKey, entry: AuthEntry, now: DateTime) -> StatusRow { +fn status_row(target: &ServerTarget, entry: AuthEntry, now: DateTime) -> StatusRow { StatusRow { - server: key.to_string(), + server: target.to_string(), oauth_state: oauth_state(&entry, now), access_token_expires_at: entry.access_token_expires_at, refresh_token_expires_at: entry.refresh_token_expires_at, @@ -187,9 +186,9 @@ fn load_dev_token_if_available() -> bool { #[cfg(test)] mod tests { use chrono::Duration; + use fabro_client::{AuthEntry, StoredSubject}; use super::{OAuthState, human_state, oauth_state}; - use crate::auth_store::{AuthEntry, StoredSubject}; fn entry(access_offset_secs: i64, refresh_offset_secs: i64) -> AuthEntry { let now = chrono::Utc::now(); diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index f7543e41d..2a5fc1ed5 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -343,7 +343,7 @@ pub(crate) async fn run_doctor( checks: vec![CheckResult { name: "Location".to_string(), status: CheckStatus::Pass, - summary: server.base_url().to_string(), + summary: server.base_url().clone(), details: vec![], remediation: None, }], diff --git a/lib/crates/fabro-cli/src/commands/exec.rs b/lib/crates/fabro-cli/src/commands/exec.rs index 7d3026557..2a63e9337 100644 --- a/lib/crates/fabro-cli/src/commands/exec.rs +++ b/lib/crates/fabro-cli/src/commands/exec.rs @@ -115,7 +115,7 @@ struct AuthenticatedFabroServerAdapter { impl AuthenticatedFabroServerAdapter { fn new(client: server_client::Client, provider_name: impl Into) -> Self { - let base_url = client.base_url().to_string(); + let base_url = client.base_url().clone(); Self { client, base_url, @@ -212,7 +212,7 @@ fn transport_error(provider: &str, err: &anyhow::Error) -> LlmError { } } -fn map_response_failure(provider: &str, failure: &server_client::HttpResponseFailure) -> LlmError { +fn map_response_failure(provider: &str, failure: &fabro_client::ApiError) -> LlmError { let retry_after = parse_retry_after(&failure.headers); let (message, code, raw) = parse_server_error_body(&failure.body); error_from_status_code( diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index c39fc605c..5b3eca612 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -21,7 +21,7 @@ use fabro_interview::{AnswerValue, ConsoleInterviewer, Question, QuestionOption, use fabro_store::EventEnvelope; use fabro_types::settings::cli::OutputVerbosity; use fabro_types::settings::run::ApprovalMode; -use fabro_types::{EventBody, RunEvent, RunId}; +use fabro_types::{EventBody, RunId}; use fabro_util::json::normalize_json_value; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; @@ -155,7 +155,7 @@ async fn attach_live_run_with_client( client: &server_client::Client, run_id: &RunId, existing_events: Vec, - mut stream: server_client::RunAttachEventStream, + mut stream: server_client::RunEventStream, styles: &'static Styles, opts: AttachOptions, printer: Printer, @@ -392,7 +392,7 @@ fn show_progress(progress_ui: &mut run_progress::ProgressUI, json_output: bool) } fn event_payload_line(event: &EventEnvelope) -> Result { - let mut value = normalize_json_value(event.payload.as_value().clone()); + let mut value = normalize_json_value(event.event.to_value()?); restore_empty_run_properties(&mut value); serde_json::to_string(&value).map_err(Into::into) } @@ -462,8 +462,7 @@ fn state_exit_code(state: &server_client::RunProjection) -> Option { } fn event_exit_code(event: &EventEnvelope) -> Option { - let run_event = RunEvent::try_from(&event.payload).ok()?; - match run_event.body { + match &event.event.body { EventBody::RunCompleted(props) => Some( if props.status == "success" || props.status == "partial_success" { ExitCode::from(0) @@ -477,10 +476,7 @@ fn event_exit_code(event: &EventEnvelope) -> Option { } fn event_starts_interview(event: &EventEnvelope) -> bool { - let Ok(run_event) = RunEvent::try_from(&event.payload) else { - return false; - }; - matches!(run_event.body, EventBody::InterviewStarted(_)) + matches!(event.event.body, EventBody::InterviewStarted(_)) } #[cfg(test)] diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index 48685ee1d..6ee251680 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -73,14 +73,15 @@ pub(crate) async fn create_run( } let created_run_id = client.create_run_from_manifest(built.manifest).await?; - let local_run_dir = match &target { - ServerTarget::UnixSocket(_) => Some( + let local_run_dir = if target.is_unix_socket() { + Some( Storage::new(user_config::storage_dir(ctx.machine_settings())?) .run_scratch(&created_run_id) .root() .to_path_buf(), - ), - ServerTarget::HttpUrl(_) => None, + ) + } else { + None }; Ok(CreatedRun { diff --git a/lib/crates/fabro-cli/src/commands/run/logs.rs b/lib/crates/fabro-cli/src/commands/run/logs.rs index dae03d121..a732bc023 100644 --- a/lib/crates/fabro-cli/src/commands/run/logs.rs +++ b/lib/crates/fabro-cli/src/commands/run/logs.rs @@ -87,8 +87,8 @@ pub(crate) async fn run( Ok(()) } -fn event_name(event: &fabro_store::EventEnvelope) -> Option<&str> { - event.payload.as_value().get("event")?.as_str() +fn event_name(event: &fabro_store::EventEnvelope) -> &str { + event.event.event_name() } fn apply_filters( @@ -175,7 +175,7 @@ async fn follow_store_logs( let had_events = !events.is_empty(); let saw_terminal = events .iter() - .any(|event| matches!(event_name(event), Some("run.completed" | "run.failed"))); + .any(|event| matches!(event_name(event), "run.completed" | "run.failed")); for event in events { let line = event_payload_line(&event)?; if pretty { @@ -268,7 +268,7 @@ async fn flush_remaining_store_events( } fn event_payload_line(event: &fabro_store::EventEnvelope) -> Result { - let mut value = normalize_json_value(event.payload.as_value().clone()); + let mut value = normalize_json_value(event.event.to_value()?); restore_empty_run_properties(&mut value); let line = serde_json::to_string(&value)?; Ok(redact_jsonl_line(&line)) diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index bfc4e60be..7b6033dce 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -14,12 +14,11 @@ use anyhow::{Context, Result, anyhow}; use async_trait::async_trait; use fabro_config::Storage; use fabro_interview::{ControlInterviewer, WorkerControlEnvelope, WorkerControlMessage}; -use fabro_store::{EventEnvelope, EventPayload, RunProjection}; +use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer}; use fabro_types::settings::run::RunMode; use fabro_types::settings::{InterpString, SettingsLayer}; -use fabro_types::{EventBody, RunBlobId, RunEvent, RunId, StatusReason}; +use fabro_types::{ArtifactUpload, EventBody, RunBlobId, RunEvent, RunId, StatusReason}; use fabro_vault::Vault; -use fabro_workflow::artifact_snapshot::CapturedArtifactInfo; use fabro_workflow::artifact_upload::{ArtifactSink, StageArtifactUploader}; use fabro_workflow::event::{Emitter, RunEventSink}; use fabro_workflow::operations::{self, StartServices}; @@ -264,7 +263,7 @@ impl StageArtifactUploader for HttpArtifactUploader { &self, stage_id: &fabro_types::StageId, artifact_capture_dir: &Path, - artifacts: &[CapturedArtifactInfo], + artifacts: &[ArtifactUpload], ) -> Result<()> { if artifacts.is_empty() { return Ok(()); @@ -306,7 +305,7 @@ impl StageArtifactUploader for MissingArtifactUploadTokenUploader { &self, _stage_id: &fabro_types::StageId, _artifact_capture_dir: &Path, - _artifacts: &[CapturedArtifactInfo], + _artifacts: &[ArtifactUpload], ) -> Result<()> { Err(anyhow!( "run {} could not upload artifacts because the worker did not receive an artifact upload token", @@ -369,8 +368,10 @@ impl HttpRunStore { } async fn apply_acknowledged_event(&self, seq: u32, event: &RunEvent) -> Result<()> { - let payload = EventPayload::new(event.to_value()?, &self.run_id)?; - let envelope = EventEnvelope { seq, payload }; + let envelope = EventEnvelope { + seq, + event: event.clone(), + }; { let mut state = self.state.lock().await; diff --git a/lib/crates/fabro-cli/src/commands/store/rebuild.rs b/lib/crates/fabro-cli/src/commands/store/rebuild.rs index 91003770b..526fc8ff2 100644 --- a/lib/crates/fabro-cli/src/commands/store/rebuild.rs +++ b/lib/crates/fabro-cli/src/commands/store/rebuild.rs @@ -17,7 +17,7 @@ pub(crate) async fn rebuild_run_store( )); let run_store = store.create_run(run_id).await?; for event in events { - let payload = EventPayload::new(event.payload.as_value().clone(), run_id)?; + let payload = EventPayload::new(event.event.to_value()?, run_id)?; run_store.append_event(&payload).await?; } Ok(run_store) diff --git a/lib/crates/fabro-cli/src/commands/system/events.rs b/lib/crates/fabro-cli/src/commands/system/events.rs index bba7bebc3..da5bdad57 100644 --- a/lib/crates/fabro-cli/src/commands/system/events.rs +++ b/lib/crates/fabro-cli/src/commands/system/events.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use fabro_client::sse; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::{CliLayer, OutputFormat}; use fabro_util::printer::Printer; @@ -6,7 +7,6 @@ use futures::StreamExt; use crate::args::SystemEventsArgs; use crate::command_context::CommandContext; -use crate::sse; pub(super) async fn events_command( args: &SystemEventsArgs, diff --git a/lib/crates/fabro-cli/src/commands/version.rs b/lib/crates/fabro-cli/src/commands/version.rs index a4eeb3dcd..a58e0541f 100644 --- a/lib/crates/fabro-cli/src/commands/version.rs +++ b/lib/crates/fabro-cli/src/commands/version.rs @@ -132,9 +132,13 @@ fn is_non_release_profile(profile: &str) -> bool { } fn format_server_target(target: &ServerTarget) -> String { - match target { - ServerTarget::HttpUrl(api_url) => api_url.clone(), - ServerTarget::UnixSocket(path) => path.display().to_string(), + if let Some(api_url) = target.as_http_url() { + api_url.to_string() + } else { + target + .as_unix_socket_path() + .map(|path| path.display().to_string()) + .unwrap_or_default() } } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 4d7924742..fc64910b4 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -4,20 +4,17 @@ )] mod args; -mod auth_store; mod command_context; mod commands; mod gh; mod landing; mod logging; -mod loopback_target; mod manifest_builder; mod server_client; mod server_runs; mod shared; #[cfg(feature = "sleep_inhibitor")] mod sleep_inhibitor; -mod sse; mod user_config; #[cfg(test)] diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 730dc1db8..4f12f2c0c 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -1,199 +1,63 @@ -use std::collections::VecDeque; -use std::num::NonZeroU64; -use std::path::Path; -use std::sync::{Arc, RwLock}; +use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; -use bytes::Bytes; -use fabro_api::types; +use fabro_client::{ + AuthStore, Credential, CredentialFallback, OAuthSession, ServerTarget, TransportConnector, +}; +pub(crate) use fabro_client::{Client, RunEventStream}; use fabro_config::Storage; -use fabro_http::header::{AUTHORIZATION, CONTENT_LENGTH, CONTENT_TYPE}; -use fabro_http::multipart::{Form, Part}; -use fabro_model::Model; +use fabro_http::header::AUTHORIZATION; use fabro_server::bind::Bind; -use fabro_store::{EventEnvelope, RunSummary, StageId}; +pub(crate) use fabro_types::RunProjection; use fabro_types::settings::SettingsLayer; -use fabro_types::{RunBlobId, RunEvent, RunId}; use fabro_util::dev_token::validate_dev_token_format; use fabro_util::{Home, dev_token}; -use fabro_workflow::artifact_snapshot::CapturedArtifactInfo; -use futures::StreamExt; -use serde::de::DeserializeOwned; -use serde::{Deserialize, Serialize}; -use tokio::fs::File; -use tokio::sync::Mutex; use tokio::time::sleep; -use tokio_util::io::ReaderStream; use crate::args::ServerTargetArgs; -use crate::auth_store::{AuthEntry, AuthStore, ServerTargetKey, StoredSubject}; use crate::commands::server::{record, start}; -use crate::loopback_target::{LoopbackClassification, is_loopback_or_unix_socket}; -use crate::user_config::cli_http_client_builder; -use crate::{sse, user_config}; +use crate::user_config::{self, cli_http_client_builder}; -#[derive(Clone)] -pub(crate) struct Client { - state: Arc>, - base_url: String, - refreshable_oauth: Option, - refresh_lock: Arc>, +#[derive(Debug)] +struct CliDevTokenFallback { + storage_dir: Option, } -#[derive(Clone)] -struct ClientBundle { - client: fabro_api::ApiClient, - http_client: fabro_http::HttpClient, - bearer_token: Option, -} - -#[derive(Debug, Clone)] -enum ResolvedBearer { - DevToken(String), - OAuth(AuthEntry), -} - -impl ResolvedBearer { - fn bearer_token(&self) -> &str { - match self { - Self::DevToken(token) => token, - Self::OAuth(entry) => &entry.access_token, - } - } -} - -#[derive(Debug, Clone)] -struct RefreshableOAuth { - target: user_config::ServerTarget, - key: ServerTargetKey, - auth_store: AuthStore, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -struct ApiFailure { - status: fabro_http::StatusCode, - code: Option, -} - -struct StructuredApiError { - error: anyhow::Error, - failure: Option, -} - -#[derive(Debug, Deserialize)] -struct CliTokenResponse { - access_token: String, - access_token_expires_at: chrono::DateTime, - refresh_token: String, - refresh_token_expires_at: chrono::DateTime, - subject: CliTokenSubject, -} - -#[derive(Debug, Deserialize)] -struct CliTokenSubject { - idp_issuer: String, - idp_subject: String, - login: String, - name: String, - email: String, -} - -#[derive(Debug, Deserialize)] -struct OAuthErrorBody { - error: String, - #[serde(default)] - error_description: Option, -} - -pub(crate) struct RunAttachEventStream { - stream: progenitor_client::ByteStream, - pending_bytes: Vec, - buffered_events: VecDeque, -} - -impl RunAttachEventStream { - fn new(stream: progenitor_client::ByteStream) -> Self { - Self { - stream, - pending_bytes: Vec::new(), - buffered_events: VecDeque::new(), - } - } - - pub(crate) async fn next_event(&mut self) -> Result> { - loop { - if let Some(event) = self.buffered_events.pop_front() { - return Ok(Some(event)); - } - - if let Some(chunk) = self.stream.next().await { - let chunk = chunk.map_err(|err| anyhow!("{err}"))?; - self.pending_bytes.extend_from_slice(&chunk); - self.buffer_sse_events(false)?; - } else { - self.buffer_sse_events(true)?; - return Ok(self.buffered_events.pop_front()); - } - } - } - - fn buffer_sse_events(&mut self, finalize: bool) -> Result<()> { - for payload in sse::drain_sse_payloads(&mut self.pending_bytes, finalize) { - self.buffered_events - .push_back(serde_json::from_str(&payload)?); - } - Ok(()) - } -} - -pub(crate) use fabro_store::RunProjection; - -fn client_bundle( - base_url: &str, - http_client: fabro_http::HttpClient, - bearer_token: Option, -) -> ClientBundle { - let client = fabro_api::ApiClient::new_with_client(base_url, http_client.clone()); - ClientBundle { - client, - http_client, - bearer_token, +impl CredentialFallback for CliDevTokenFallback { + fn resolve(&self) -> Option { + load_dev_token_if_available(self.storage_dir.as_deref()).map(Credential::DevToken) } } fn refreshable_oauth( - target: &user_config::ServerTarget, - bearer: Option<&ResolvedBearer>, -) -> Result> { - if matches!(bearer, Some(ResolvedBearer::OAuth(_))) { - return Ok(Some(RefreshableOAuth { - target: target.clone(), - key: ServerTargetKey::new(target)?, - auth_store: AuthStore::default(), - })); + target: &ServerTarget, + credential: Option<&Credential>, +) -> Option { + if matches!(credential, Some(Credential::OAuth(_))) { + let session = OAuthSession::new(target.clone(), AuthStore::default()); + if local_dev_token_fallback(target) { + return Some( + session.with_fallback(Arc::new(CliDevTokenFallback { storage_dir: None })), + ); + } + return Some(session); } - Ok(None) + None } pub(crate) async fn connect_server(storage_dir: &Path) -> Result { - connect_api_client_bundle(storage_dir).await + connect_local_api_client_bundle(storage_dir, &user_config::active_settings_path(None)).await } -pub(crate) async fn connect_server_target(target: &user_config::ServerTarget) -> Result { +pub(crate) async fn connect_server_target(target: &ServerTarget) -> Result { connect_target_api_client_bundle(target).await } pub(crate) async fn connect_server_target_direct(target: &str) -> Result { - if target.starts_with("http://") || target.starts_with("https://") { - connect_server_target(&user_config::ServerTarget::HttpUrl(target.to_string())).await - } else { - let path = Path::new(target); - if !path.is_absolute() { - bail!("server target must be an http(s) URL or absolute Unix socket path"); - } - connect_server_target(&user_config::ServerTarget::UnixSocket(path.to_path_buf())).await - } + let target = target.parse::()?; + connect_server_target(&target).await } pub(crate) async fn connect_server_with_settings( @@ -202,7 +66,7 @@ pub(crate) async fn connect_server_with_settings( base_config_path: &Path, ) -> Result { if let Some(target) = user_config::resolve_nondefault_server_target(args, settings)? { - if let user_config::ServerTarget::UnixSocket(path) = &target { + if let Some(path) = target.as_unix_socket_path() { return connect_managed_unix_socket_api_client_bundle( path, &user_config::storage_dir(settings)?, @@ -221,33 +85,35 @@ async fn connect_managed_unix_socket_api_client_bundle( storage_dir: &Path, active_config_path: &Path, ) -> Result { - let target = user_config::ServerTarget::UnixSocket(path.to_path_buf()); - let bearer = resolve_target_bearer( + let target = ServerTarget::unix_socket_path(path)?; + let credential = resolve_target_credential( &target, Some(storage_dir), local_dev_token_fallback(&target), )?; - let refreshable_oauth = refreshable_oauth(&target, bearer.as_ref())?; - let bearer_token = bearer.as_ref().map(ResolvedBearer::bearer_token); + let oauth_session = refreshable_oauth(&target, credential.as_ref()); + let bearer_token = credential.as_ref().map(Credential::bearer_token); - let bundle = if let Ok(bundle) = - try_connect_unix_socket_api_client_bundle(path, Some(storage_dir), bearer_token).await + let http_client = if let Ok(http_client) = + try_connect_unix_socket_http_client(path, Some(storage_dir), bearer_token).await { - bundle + http_client } else { start::ensure_server_running_on_socket(path, active_config_path, storage_dir) .await .with_context(|| format!("Failed to start fabro server for {}", path.display()))?; - connect_unix_socket_api_client_bundle(path, Some(storage_dir), bearer_token) + connect_unix_socket_http_client(path, Some(storage_dir), bearer_token) .await .with_context(|| format!("Failed to connect to fabro server at {}", path.display()))? }; - Ok(Client::from_bundle( - bundle, - "http://fabro".to_string(), - refreshable_oauth, - )) + build_client( + target, + credential, + oauth_session, + Some(("http://fabro".to_string(), http_client)), + ) + .await } async fn connect_local_api_client_bundle( @@ -259,96 +125,94 @@ async fn connect_local_api_client_bundle( .with_context(|| format!("Failed to start fabro server for {}", storage_dir.display()))?; match bind { Bind::Unix(path) => { - let bundle = - connect_unix_socket_api_client_bundle(&path, Some(storage_dir), None).await?; - Ok(Client::from_bundle( - bundle, - "http://fabro".to_string(), - None, - )) + let http_client = + connect_unix_socket_http_client(&path, Some(storage_dir), None).await?; + Ok(Client::from_http_client("http://fabro", http_client)) } Bind::Tcp(addr) => { let token = wait_for_local_dev_token(storage_dir).await?; let builder = cli_http_client_builder().no_proxy(); let http_client = apply_bearer_token_auth(builder, &token)?.build()?; let base_url = format!("http://{addr}"); - Ok(Client::from_bundle( - client_bundle(&base_url, http_client, Some(token)), - base_url, - None, - )) + Ok(Client::from_http_client(base_url, http_client)) } } } -async fn connect_api_client_bundle(storage_dir: &Path) -> Result { - connect_local_api_client_bundle(storage_dir, &user_config::active_settings_path(None)).await -} - #[allow( dead_code, reason = "Retained for pending storage-backed internal callers and referenced in existing design docs." )] pub(crate) async fn connect_api_client(storage_dir: &Path) -> Result { - connect_api_client_bundle(storage_dir) + connect_local_api_client_bundle(storage_dir, &user_config::active_settings_path(None)) .await - .map(|client| client.client_bundle().client) + .map(|client| client.api_client()) } -async fn connect_target_api_client_bundle(target: &user_config::ServerTarget) -> Result { - match target { - user_config::ServerTarget::HttpUrl(api_url) => { - let bearer = resolve_target_bearer(target, None, local_dev_token_fallback(target))?; - let refreshable_oauth = refreshable_oauth(target, bearer.as_ref())?; - let bundle = connect_remote_api_client_bundle( - api_url, - bearer.as_ref().map(ResolvedBearer::bearer_token), - )?; - Ok(Client::from_bundle( - bundle, - user_config::normalized_http_base_url(api_url).to_string(), - refreshable_oauth, - )) - } - user_config::ServerTarget::UnixSocket(path) => { - let bearer = resolve_target_bearer(target, None, local_dev_token_fallback(target))?; - let refreshable_oauth = refreshable_oauth(target, bearer.as_ref())?; - let bundle = try_connect_unix_socket_api_client_bundle( - path, - None, - bearer.as_ref().map(ResolvedBearer::bearer_token), - ) - .await - .with_context(|| format!("Failed to connect to fabro server at {}", path.display()))?; - Ok(Client::from_bundle( - bundle, - "http://fabro".to_string(), - refreshable_oauth, - )) - } +async fn connect_target_api_client_bundle(target: &ServerTarget) -> Result { + let credential = resolve_target_credential(target, None, local_dev_token_fallback(target))?; + let oauth_session = refreshable_oauth(target, credential.as_ref()); + build_client(target.clone(), credential, oauth_session, None).await +} + +async fn build_client( + target: ServerTarget, + credential: Option, + oauth_session: Option, + transport: Option<(String, fabro_http::HttpClient)>, +) -> Result { + let mut builder = Client::builder() + .target(target.clone()) + .transport_connector(build_cli_transport_connector(target)); + if let Some((base_url, http_client)) = transport { + builder = builder.transport(base_url, http_client); } + if let Some(credential) = credential { + builder = builder.credential(credential); + } + if let Some(oauth_session) = oauth_session { + builder = builder.oauth_session(oauth_session); + } + builder.connect().await } -fn connect_remote_api_client_bundle( - api_url: &str, +fn build_cli_transport_connector(target: ServerTarget) -> TransportConnector { + TransportConnector::new(move |bearer_token| { + let target = target.clone(); + async move { connect_cli_target_transport(&target, bearer_token.as_deref()) } + }) +} + +fn connect_cli_target_transport( + target: &ServerTarget, bearer_token: Option<&str>, -) -> Result { - let normalized = user_config::normalized_http_base_url(api_url); - let mut builder = user_config::cli_http_client_builder(); +) -> Result<(fabro_http::HttpClient, String)> { + if let Some(api_url) = target.as_http_url() { + let mut builder = cli_http_client_builder(); + builder = match bearer_token { + Some(token) => apply_bearer_token_auth(builder, token)?, + None => builder, + }; + let http_client = builder.build()?; + return Ok((http_client, api_url.to_string())); + } + + let Some(path) = target.as_unix_socket_path() else { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + }; + let mut builder = cli_http_client_builder().unix_socket(path).no_proxy(); builder = match bearer_token { Some(token) => apply_bearer_token_auth(builder, token)?, None => builder, }; - let http_client = builder.build()?; - Ok(client_bundle( - normalized, - http_client, - bearer_token.map(ToOwned::to_owned), - )) + let http_client = builder + .build() + .context("Failed to build Unix-socket HTTP client for fabro server")?; + Ok((http_client, "http://fabro".to_string())) } -fn local_dev_token_fallback(target: &user_config::ServerTarget) -> bool { - matches!(target, user_config::ServerTarget::UnixSocket(_)) +fn local_dev_token_fallback(target: &ServerTarget) -> bool { + target.is_unix_socket() } fn load_dev_token_if_available(storage_dir: Option<&Path>) -> Option { @@ -413,52 +277,24 @@ fn apply_bearer_token_auth( Ok(builder.default_headers(headers)) } -fn apply_dev_token_auth( - builder: fabro_http::HttpClientBuilder, - storage_dir: Option<&Path>, -) -> Result { - let Some(token) = load_dev_token_if_available(storage_dir) else { - return Ok(builder); - }; - apply_bearer_token_auth(builder, &token) -} - -fn unix_socket_api_client_bundle( - http_client: fabro_http::HttpClient, - bearer_token: Option, -) -> ClientBundle { - client_bundle("http://fabro", http_client, bearer_token) -} - -async fn build_authed_unix_socket_client( +async fn build_authed_unix_socket_http_client( path: &Path, storage_dir: Option<&Path>, bearer_token: Option<&str>, -) -> Result { - let http_client = if let Some(token) = bearer_token { - apply_bearer_token_auth( - cli_http_client_builder().unix_socket(path).no_proxy(), - token, - )? - .build() - .context("Failed to build Unix-socket HTTP client for fabro server")? +) -> Result { + let builder = cli_http_client_builder().unix_socket(path).no_proxy(); + let builder = if let Some(token) = bearer_token { + apply_bearer_token_auth(builder, token)? } else if let Some(storage_dir) = storage_dir { let token = wait_for_local_dev_token(storage_dir).await?; - apply_bearer_token_auth( - cli_http_client_builder().unix_socket(path).no_proxy(), - &token, - )? - .build() - .context("Failed to build Unix-socket HTTP client for fabro server")? + apply_bearer_token_auth(builder, &token)? } else { - apply_dev_token_auth(cli_http_client_builder().unix_socket(path).no_proxy(), None)? - .build() - .context("Failed to build Unix-socket HTTP client for fabro server")? + builder }; - Ok(unix_socket_api_client_bundle( - http_client, - bearer_token.map(ToOwned::to_owned), - )) + + builder + .build() + .context("Failed to build Unix-socket HTTP client for fabro server") } fn build_unix_socket_probe_client(path: &Path) -> Result { @@ -469,47 +305,46 @@ fn build_unix_socket_probe_client(path: &Path) -> Result .context("Failed to build Unix-socket HTTP client for fabro server") } -async fn try_connect_unix_socket_api_client_bundle( +async fn try_connect_unix_socket_http_client( path: &Path, storage_dir: Option<&Path>, bearer_token: Option<&str>, -) -> Result { +) -> Result { check_server_ready(&build_unix_socket_probe_client(path)?).await?; - build_authed_unix_socket_client(path, storage_dir, bearer_token).await + build_authed_unix_socket_http_client(path, storage_dir, bearer_token).await } -async fn connect_unix_socket_api_client_bundle( +async fn connect_unix_socket_http_client( path: &Path, storage_dir: Option<&Path>, bearer_token: Option<&str>, -) -> Result { +) -> Result { wait_for_server_ready(&build_unix_socket_probe_client(path)?).await?; - build_authed_unix_socket_client(path, storage_dir, bearer_token).await + build_authed_unix_socket_http_client(path, storage_dir, bearer_token).await } -fn resolve_target_bearer( - target: &user_config::ServerTarget, +fn resolve_target_credential( + target: &ServerTarget, storage_dir: Option<&Path>, allow_local_dev_token_fallback: bool, -) -> Result> { +) -> Result> { if let Some(token) = std::env::var("FABRO_DEV_TOKEN") .ok() .filter(|token| validate_dev_token_format(token)) { - return Ok(Some(ResolvedBearer::DevToken(token))); + return Ok(Some(Credential::DevToken(token))); } let store = AuthStore::default(); - let key = ServerTargetKey::new(target)?; - if let Some(entry) = store.get(&key)? { + if let Some(entry) = store.get(target)? { let now = chrono::Utc::now(); if entry.access_token_expires_at > now || entry.refresh_token_expires_at > now { - return Ok(Some(ResolvedBearer::OAuth(entry))); + return Ok(Some(Credential::OAuth(entry))); } } if allow_local_dev_token_fallback { - return Ok(load_dev_token_if_available(storage_dir).map(ResolvedBearer::DevToken)); + return Ok(load_dev_token_if_available(storage_dir).map(Credential::DevToken)); } Ok(None) @@ -530,9 +365,7 @@ async fn wait_for_server_ready(http_client: &fabro_http::HttpClient) -> Result<( while std::time::Instant::now() < deadline { match check_server_ready(http_client).await { Ok(()) => return Ok(()), - Err(err) => { - last_error = Some(err); - } + Err(err) => last_error = Some(err), } sleep(Duration::from_millis(50)).await; } @@ -540,1269 +373,13 @@ async fn wait_for_server_ready(http_client: &fabro_http::HttpClient) -> Result<( Err(last_error.unwrap_or_else(|| anyhow!("server did not become ready in time"))) } -#[derive(Debug, Serialize)] -struct ArtifactBatchUploadManifest { - entries: Vec, -} - -#[derive(Debug, Serialize)] -struct ArtifactBatchUploadEntry { - part: String, - path: String, - #[serde(skip_serializing_if = "Option::is_none")] - sha256: Option, - #[serde(skip_serializing_if = "Option::is_none")] - expected_bytes: Option, - #[serde(skip_serializing_if = "Option::is_none")] - content_type: Option, -} - -impl Client { - fn from_bundle( - bundle: ClientBundle, - base_url: String, - refreshable_oauth: Option, - ) -> Self { - Self { - state: Arc::new(RwLock::new(bundle)), - base_url, - refreshable_oauth, - refresh_lock: Arc::new(Mutex::new(())), - } - } - - fn client_bundle(&self) -> ClientBundle { - self.state - .read() - .expect("server client state lock should not be poisoned") - .clone() - } - - fn replace_client_bundle(&self, bundle: ClientBundle) { - *self - .state - .write() - .expect("server client state lock should not be poisoned") = bundle; - } - - /// Build a client for tests that bypasses proxy discovery. - #[cfg(test)] - pub(crate) fn new_no_proxy(base_url: &str) -> Result { - let http_client = cli_http_client_builder().no_proxy().build()?; - Ok(Self::from_bundle( - client_bundle(base_url, http_client, None), - base_url.to_string(), - None, - )) - } - - pub(crate) fn clone_for_reuse(&self) -> Self { - self.clone() - } - - pub(crate) async fn send_api( - &self, - request: F, - ) -> Result> - where - F: FnOnce(fabro_api::ApiClient) -> Fut + Clone, - Fut: std::future::Future< - Output = std::result::Result< - progenitor_client::ResponseValue, - progenitor_client::Error, - >, - >, - E: serde::Serialize + std::fmt::Debug, - { - let bundle = self.client_bundle(); - match request.clone()(bundle.client.clone()).await { - Ok(response) => Ok(response), - Err(err) => { - let mapped = classify_api_error(err).await; - if self.should_refresh(mapped.failure.as_ref()) { - if let Some(failed_token) = bundle.bearer_token.as_deref() { - self.refresh_access_token(failed_token).await?; - let bundle = self.client_bundle(); - return request(bundle.client.clone()).await.map_err(map_api_error); - } - } - Err(mapped.error) - } - } - } - - fn should_refresh(&self, failure: Option<&ApiFailure>) -> bool { - self.refreshable_oauth.is_some() - && failure.is_some_and(|failure| { - failure.status == fabro_http::StatusCode::UNAUTHORIZED - && failure.code.as_deref() == Some("access_token_expired") - }) - } - - async fn refresh_access_token(&self, failed_access_token: &str) -> Result<()> { - let Some(refreshable) = &self.refreshable_oauth else { - bail!("CLI session has expired. Run `fabro auth login` again."); - }; - let _guard = self.refresh_lock.lock().await; - let current_bundle = self.client_bundle(); - if current_bundle.bearer_token.as_deref() != Some(failed_access_token) { - return Ok(()); - } - - let Some(entry) = refreshable.auth_store.get(&refreshable.key)? else { - let fallback = resolve_target_bearer( - &refreshable.target, - None, - local_dev_token_fallback(&refreshable.target), - )?; - self.rebuild_client_for_target( - &refreshable.target, - fallback.as_ref().map(ResolvedBearer::bearer_token), - ) - .await?; - bail!("CLI session has expired. Run `fabro auth login` again."); - }; - if entry.refresh_token_expires_at <= chrono::Utc::now() { - refreshable.auth_store.remove(&refreshable.key)?; - let fallback = resolve_target_bearer( - &refreshable.target, - None, - local_dev_token_fallback(&refreshable.target), - )?; - self.rebuild_client_for_target( - &refreshable.target, - fallback.as_ref().map(ResolvedBearer::bearer_token), - ) - .await?; - bail!("CLI session has expired. Run `fabro auth login` again."); - } - ensure_refresh_target_transport(&refreshable.target)?; - - let (http_client, base_url) = user_config::build_public_http_client(&refreshable.target)?; - let response = http_client - .post(format!("{base_url}/auth/cli/refresh")) - .header(AUTHORIZATION, format!("Bearer {}", entry.refresh_token)) - .send() - .await?; - - if response.status().is_success() { - let tokens = response - .json::() - .await - .context("failed to parse CLI auth refresh response")?; - let entry = AuthEntry { - access_token: tokens.access_token.clone(), - access_token_expires_at: tokens.access_token_expires_at, - refresh_token: tokens.refresh_token.clone(), - refresh_token_expires_at: tokens.refresh_token_expires_at, - subject: StoredSubject { - idp_issuer: tokens.subject.idp_issuer, - idp_subject: tokens.subject.idp_subject, - login: tokens.subject.login, - name: tokens.subject.name, - email: tokens.subject.email, - }, - logged_in_at: entry.logged_in_at, - }; - refreshable - .auth_store - .put(&refreshable.key, entry.clone()) - .context("failed to persist refreshed CLI auth tokens")?; - self.rebuild_client_for_target(&refreshable.target, Some(&entry.access_token)) - .await?; - return Ok(()); - } - - let status = response.status(); - let body = response.text().await.unwrap_or_default(); - let parsed_error = serde_json::from_str::(&body).ok(); - if parsed_error.as_ref().is_some_and(|error| { - matches!( - error.error.as_str(), - "refresh_token_expired" | "refresh_token_revoked" - ) - }) { - refreshable.auth_store.remove(&refreshable.key)?; - let fallback = resolve_target_bearer( - &refreshable.target, - None, - local_dev_token_fallback(&refreshable.target), - )?; - self.rebuild_client_for_target( - &refreshable.target, - fallback.as_ref().map(ResolvedBearer::bearer_token), - ) - .await?; - } - - if let Some(parsed_error) = parsed_error { - let message = parsed_error - .error_description - .filter(|value| !value.is_empty()) - .unwrap_or_else(|| format!("request failed with status {status}")); - bail!("{message}"); - } - if body.is_empty() { - bail!("request failed with status {status}"); - } - bail!("request failed with status {status}: {body}"); - } - - async fn rebuild_client_for_target( - &self, - target: &user_config::ServerTarget, - bearer_token: Option<&str>, - ) -> Result<()> { - let bundle = match target { - user_config::ServerTarget::HttpUrl(api_url) => { - connect_remote_api_client_bundle(api_url, bearer_token)? - } - user_config::ServerTarget::UnixSocket(path) => { - connect_unix_socket_api_client_bundle(path, None, bearer_token).await? - } - }; - self.replace_client_bundle(bundle); - Ok(()) - } - - pub(crate) async fn send_http_response( - &self, - request: F, - ) -> Result> - where - F: FnOnce(fabro_http::HttpClient) -> Fut + Clone, - Fut: std::future::Future>, - T: Into, - { - let bundle = self.client_bundle(); - let response = request.clone()(bundle.http_client.clone()) - .await - .map_err(Into::into)?; - match classify_http_response(response).await? { - Ok(response) => Ok(Ok(response)), - Err(failure) => { - if self.should_refresh(Some(&failure.failure)) { - if let Some(failed_token) = bundle.bearer_token.as_deref() { - self.refresh_access_token(failed_token).await?; - let bundle = self.client_bundle(); - let response = request(bundle.http_client.clone()) - .await - .map_err(Into::into)?; - return classify_http_response(response).await; - } - } - Ok(Err(failure)) - } - } - } - - async fn send_http(&self, request: F) -> Result - where - F: FnOnce(fabro_http::HttpClient) -> Fut + Clone, - Fut: std::future::Future>, - T: Into, - { - match self.send_http_response(request).await? { - Ok(response) => Ok(response), - Err(failure) => Err(raw_response_failure_error(&failure)), - } - } - - #[allow( - dead_code, - reason = "This accessor is kept for tests and pending callers." - )] - pub(crate) fn http_client(&self) -> fabro_http::HttpClient { - self.client_bundle().http_client - } - - #[allow( - dead_code, - reason = "This accessor is kept for tests and pending callers." - )] - pub(crate) fn base_url(&self) -> &str { - &self.base_url - } - - pub(crate) async fn retrieve_resolved_server_settings(&self) -> Result { - let url = format!("{}/api/v1/settings?view=resolved", self.base_url); - let response = self - .send_http(|http_client| async move { http_client.get(&url).send().await }) - .await?; - - let marker = response - .headers() - .get("x-fabro-settings-view") - .and_then(|value| value.to_str().ok()); - if marker != Some("resolved") { - bail!( - "server does not support resolved settings view; upgrade the server or use --local" - ); - } - - response - .json::() - .await - .context("server returned invalid JSON for the resolved settings view") - } - - pub(crate) async fn create_run_from_manifest( - &self, - manifest: types::RunManifest, - ) -> Result { - let response = self - .send_api( - |client| async move { client.create_run().body(manifest.clone()).send().await }, - ) - .await?; - let status = response.into_inner(); - status - .id - .parse() - .map_err(|err| anyhow!("invalid run ID from server: {err}")) - } - - pub(crate) async fn list_secrets(&self) -> Result> { - let response = self - .send_api(|client| async move { client.list_secrets().send().await }) - .await?; - Ok(response.into_inner().data) - } - - pub(crate) async fn create_secret( - &self, - body: types::CreateSecretRequest, - ) -> Result { - let response = self - .send_api( - |client| async move { client.create_secret().body(body.clone()).send().await }, - ) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn delete_secret_by_name(&self, name: &str) -> Result<()> { - self.send_api(|client| async move { - client - .delete_secret_by_name() - .body(types::DeleteSecretRequest { - name: name.to_string(), - }) - .send() - .await - }) - .await?; - Ok(()) - } - - pub(crate) async fn list_models( - &self, - provider: Option<&str>, - query: Option<&str>, - ) -> Result> { - let mut offset = 0u64; - let mut models = Vec::new(); - - loop { - let response = self - .send_api(|client| async move { - let mut request = client.list_models().page_limit(100u64).page_offset(offset); - if let Some(provider) = provider { - request = request.provider(provider.to_string()); - } - if let Some(query) = query { - request = request.query(query.to_string()); - } - request.send().await - }) - .await?; - let parsed = response.into_inner(); - let count = parsed.data.len() as u64; - models.extend(convert_type::<_, Vec>(parsed.data)?); - if !parsed.meta.has_more { - break; - } - offset += count; - } - - Ok(models) - } - - pub(crate) async fn test_model( - &self, - id: &str, - mode: Option, - ) -> Result { - let response = self - .send_api(|client| async move { - let mut request = client.test_model().id(id.to_string()); - if let Some(mode) = mode { - request = request.mode(mode); - } - request.send().await - }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn attach_events( - &self, - run_ids: &[String], - ) -> Result { - let response = self - .send_api(|client| async move { - let mut request = client.attach_events(); - if !run_ids.is_empty() { - request = request.run_id(run_ids.join(",")); - } - request.send().await - }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn get_system_info(&self) -> Result { - let response = self - .send_api(|client| async move { client.get_system_info().send().await }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn get_system_disk_usage( - &self, - verbose: bool, - ) -> Result { - let response = self - .send_api(|client| async move { - client.get_system_disk_usage().verbose(verbose).send().await - }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn prune_runs( - &self, - body: types::PruneRunsRequest, - ) -> Result { - let response = self - .send_api(|client| async move { client.prune_runs().body(body.clone()).send().await }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn get_health(&self) -> Result<()> { - self.send_api(|client| async move { client.get_health().send().await }) - .await?; - Ok(()) - } - - pub(crate) async fn run_diagnostics(&self) -> Result { - let response = self - .send_api(|client| async move { client.run_diagnostics().send().await }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn get_github_repo( - &self, - owner: &str, - name: &str, - ) -> Result { - let response = self - .send_api(|client| async move { - client - .get_github_repo() - .owner(owner.to_string()) - .name(name.to_string()) - .send() - .await - }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn run_preflight( - &self, - manifest: types::RunManifest, - ) -> Result { - self.send_api( - |client| async move { client.run_preflight().body(manifest.clone()).send().await }, - ) - .await - .map(progenitor_client::ResponseValue::into_inner) - } - - pub(crate) async fn render_workflow_graph( - &self, - request: types::RenderWorkflowGraphRequest, - ) -> Result> { - let response = self - .send_api(|client| async move { - client - .render_workflow_graph() - .body(request.clone()) - .send() - .await - }) - .await?; - let mut stream = response.into_inner(); - let mut bytes = Vec::new(); - while let Some(chunk) = stream.next().await { - let chunk = chunk.map_err(|err| anyhow!("{err}"))?; - bytes.extend_from_slice(&chunk); - } - Ok(bytes) - } - - pub(crate) async fn start_run(&self, run_id: &RunId, resume: bool) -> Result<()> { - self.send_api(|client| async move { - client - .start_run() - .id(run_id.to_string()) - .body(types::StartRunRequest { resume }) - .send() - .await - }) - .await?; - Ok(()) - } - - pub(crate) async fn cancel_run(&self, run_id: &RunId) -> Result<()> { - self.send_api( - |client| async move { client.cancel_run().id(run_id.to_string()).send().await }, - ) - .await?; - Ok(()) - } - - pub(crate) async fn archive_run(&self, run_id: &RunId) -> Result<()> { - self.send_api( - |client| async move { client.archive_run().id(run_id.to_string()).send().await }, - ) - .await?; - Ok(()) - } - - pub(crate) async fn unarchive_run(&self, run_id: &RunId) -> Result<()> { - self.send_api(|client| async move { - client.unarchive_run().id(run_id.to_string()).send().await - }) - .await?; - Ok(()) - } - - pub(crate) async fn list_store_runs(&self) -> Result> { - let mut all_runs = Vec::new(); - let mut offset = 0_u64; - let limit = 100_u64; - - loop { - let response = self - .send_api(|client| async move { - client - .list_runs() - .page_limit(limit) - .page_offset(offset) - .include_archived(true) - .send() - .await - }) - .await?; - let parsed = response.into_inner(); - let batch = parsed - .data - .into_iter() - .map(convert_type) - .collect::>>()?; - let batch_len = batch.len() as u64; - all_runs.extend(batch); - - if !parsed.meta.has_more || batch_len == 0 { - break; - } - offset += batch_len; - } - - Ok(all_runs) - } - - pub(crate) async fn retrieve_run(&self, run_id: &RunId) -> Result { - let response = self - .send_api( - |client| async move { client.retrieve_run().id(run_id.to_string()).send().await }, - ) - .await?; - convert_type(response.into_inner()) - } - - pub(crate) async fn resolve_run(&self, selector: &str) -> Result { - let response = self - .send_api(|client| async move { - client - .resolve_run() - .selector(selector.to_string()) - .send() - .await - }) - .await?; - convert_type(response.into_inner()) - } - - pub(crate) async fn get_run_state(&self, run_id: &RunId) -> Result { - let response = self - .send_api( - |client| async move { client.get_run_state().id(run_id.to_string()).send().await }, - ) - .await?; - convert_type(response.into_inner()) - } - - pub(crate) async fn list_run_events( - &self, - run_id: &RunId, - since_seq: Option, - limit: Option, - ) -> Result> { - let mut next_since_seq = since_seq; - let mut all_events = Vec::new(); - - loop { - let response = self - .send_api(|client| async move { - let mut request = client.list_run_events().id(run_id.to_string()); - if let Some(seq) = next_since_seq.and_then(non_zero_u64_from_u32) { - request = request.since_seq(seq); - } - if let Some(limit) = limit.and_then(non_zero_u64_from_usize) { - request = request.limit(limit); - } - request.send().await - }) - .await?; - let parsed = response.into_inner(); - let page_events = parsed - .data - .into_iter() - .map(convert_type::<_, EventEnvelope>) - .collect::>>()?; - let next_page_since_seq = page_events.last().map(|event| event.seq.saturating_add(1)); - all_events.extend(page_events); - - if limit.is_some() || !parsed.meta.has_more || next_page_since_seq.is_none() { - break; - } - next_since_seq = next_page_since_seq; - } - - Ok(all_events) - } - - pub(crate) async fn attach_run_events( - &self, - run_id: &RunId, - since_seq: Option, - ) -> Result { - let response = self - .send_api(|client| async move { - let mut request = client.attach_run_events().id(run_id.to_string()); - if let Some(seq) = since_seq.and_then(non_zero_u64_from_u32) { - request = request.since_seq(seq); - } - request.send().await - }) - .await?; - Ok(RunAttachEventStream::new(response.into_inner())) - } - - pub(crate) async fn list_run_questions( - &self, - run_id: &RunId, - ) -> Result> { - let response = self - .send_api(|client| async move { - client - .list_run_questions() - .id(run_id.to_string()) - .page_limit(100) - .page_offset(0) - .send() - .await - }) - .await?; - Ok(response.into_inner().data) - } - - pub(crate) async fn submit_run_answer( - &self, - run_id: &RunId, - qid: &str, - value: Option, - selected_option_key: Option, - selected_option_keys: Vec, - ) -> Result<()> { - self.send_api(|client| async move { - client - .submit_run_answer() - .id(run_id.to_string()) - .qid(qid) - .body(types::SubmitAnswerRequest { - value: value.clone(), - selected_option_key: selected_option_key.clone(), - selected_option_keys: selected_option_keys.clone(), - }) - .send() - .await - }) - .await?; - Ok(()) - } - - pub(crate) async fn append_run_event(&self, run_id: &RunId, event: &RunEvent) -> Result { - let body: types::RunEvent = convert_type(event)?; - let response = self - .send_api(|client| async move { - client - .append_run_event() - .id(run_id.to_string()) - .body(body.clone()) - .send() - .await - }) - .await?; - u32::try_from(response.into_inner().seq).context("append_run_event returned invalid seq") - } - - pub(crate) async fn write_run_blob(&self, run_id: &RunId, data: &[u8]) -> Result { - let response = self - .send_api(|client| async move { - client - .write_run_blob() - .id(run_id.to_string()) - .body(data.to_vec()) - .send() - .await - }) - .await?; - response - .into_inner() - .id - .parse() - .context("write_run_blob returned invalid blob id") - } - - pub(crate) async fn read_run_blob( - &self, - run_id: &RunId, - blob_id: &RunBlobId, - ) -> Result> { - let response = self - .client_bundle() - .client - .read_run_blob() - .id(run_id.to_string()) - .blob_id(blob_id.to_string()) - .send() - .await; - match response { - Ok(response) => { - let mut stream = response.into_inner(); - let mut bytes = Vec::new(); - while let Some(chunk) = stream.next().await { - let chunk = chunk.map_err(|err| anyhow!("{err}"))?; - bytes.extend_from_slice(&chunk); - } - Ok(Some(Bytes::from(bytes))) - } - Err(err) => { - if is_not_found_error(&err) { - Ok(None) - } else { - Err(map_api_error(err)) - } - } - } - } - - pub(crate) async fn delete_store_run(&self, run_id: &RunId, force: bool) -> Result<()> { - let mut url = fabro_http::Url::parse(&self.base_url) - .with_context(|| format!("invalid server base URL {}", self.base_url))?; - url.path_segments_mut() - .map_err(|()| anyhow!("server base URL cannot accept path segments"))? - .extend(["api", "v1", "runs", &run_id.to_string()]); - if force { - url.query_pairs_mut().append_pair("force", "true"); - } - - self.send_http(|http_client| async move { http_client.delete(url.clone()).send().await }) - .await?; - Ok(()) - } - - pub(crate) async fn list_run_artifacts( - &self, - run_id: &RunId, - ) -> Result> { - let response = self - .send_api(|client| async move { - client - .list_run_artifacts() - .id(run_id.to_string()) - .send() - .await - }) - .await?; - Ok(response.into_inner().data) - } - - pub(crate) async fn download_stage_artifact( - &self, - run_id: &RunId, - stage_id: &StageId, - filename: &str, - ) -> Result> { - let response = self - .send_api(|client| async move { - client - .get_stage_artifact() - .id(run_id.to_string()) - .stage_id(stage_id.to_string()) - .filename(filename) - .send() - .await - }) - .await?; - let mut stream = response.into_inner(); - let mut bytes = Vec::new(); - while let Some(chunk) = stream.next().await { - let chunk = chunk.map_err(|err| anyhow!("{err}"))?; - bytes.extend_from_slice(&chunk); - } - Ok(bytes) - } - - fn stage_artifacts_url(&self, run_id: &RunId, stage_id: &StageId) -> Result { - let mut url = fabro_http::Url::parse(&self.base_url) - .with_context(|| format!("invalid server base URL {}", self.base_url))?; - url.path_segments_mut() - .map_err(|()| anyhow!("server base URL cannot accept path segments"))? - .extend([ - "api", - "v1", - "runs", - &run_id.to_string(), - "stages", - &stage_id.to_string(), - "artifacts", - ]); - Ok(url) - } - - pub(crate) async fn upload_stage_artifact_file( - &self, - run_id: &RunId, - stage_id: &StageId, - filename: &str, - path: &Path, - bearer_token: &str, - ) -> Result<()> { - let mut url = self.stage_artifacts_url(run_id, stage_id)?; - url.query_pairs_mut().append_pair("filename", filename); - - let file = File::open(path) - .await - .with_context(|| format!("failed to open artifact {}", path.display()))?; - let content_length = file - .metadata() - .await - .with_context(|| format!("failed to stat artifact {}", path.display()))? - .len(); - let body = fabro_http::Body::wrap_stream(ReaderStream::new(file)); - - let response = self - .client_bundle() - .http_client - .post(url) - .bearer_auth(bearer_token) - .header(CONTENT_TYPE, "application/octet-stream") - .header(CONTENT_LENGTH, content_length.to_string()) - .body(body) - .send() - .await - .with_context(|| format!("failed to upload artifact {}", path.display()))?; - classify_http_response(response) - .await? - .map(|_| ()) - .map_err(|failure| raw_response_failure_error(&failure)) - } - - pub(crate) async fn upload_stage_artifact_batch( - &self, - run_id: &RunId, - stage_id: &StageId, - artifact_capture_dir: &Path, - artifacts: &[CapturedArtifactInfo], - bearer_token: &str, - ) -> Result<()> { - let url = self.stage_artifacts_url(run_id, stage_id)?; - let mut manifest_entries = Vec::with_capacity(artifacts.len()); - let mut file_parts = Vec::with_capacity(artifacts.len()); - - for (index, artifact) in artifacts.iter().enumerate() { - let part_name = format!("file{}", index + 1); - let path = artifact_capture_dir.join(&artifact.path); - let file = File::open(&path) - .await - .with_context(|| format!("failed to open artifact {}", path.display()))?; - let content_length = file - .metadata() - .await - .with_context(|| format!("failed to stat artifact {}", path.display()))? - .len(); - - manifest_entries.push(ArtifactBatchUploadEntry { - part: part_name.clone(), - path: artifact.path.clone(), - sha256: Some(artifact.content_sha256.clone()), - expected_bytes: Some(artifact.bytes), - content_type: Some(artifact.mime.clone()), - }); - - file_parts.push(( - part_name, - Part::stream_with_length( - fabro_http::Body::wrap_stream(ReaderStream::new(file)), - content_length, - ) - .file_name(artifact.path.clone()), - )); - } - - let manifest = ArtifactBatchUploadManifest { - entries: manifest_entries, - }; - let manifest_part = - Part::text(serde_json::to_string(&manifest)?).mime_str("application/json")?; - let mut form = Form::new().part("manifest", manifest_part); - for (part_name, part) in file_parts { - form = form.part(part_name, part); - } - - let response = self - .client_bundle() - .http_client - .post(url) - .bearer_auth(bearer_token) - .multipart(form) - .send() - .await - .context("failed to upload artifact batch")?; - classify_http_response(response) - .await? - .map(|_| ()) - .map_err(|failure| raw_response_failure_error(&failure)) - } - - pub(crate) async fn generate_preview_url( - &self, - run_id: &RunId, - port: u16, - expires_in_secs: u64, - signed: bool, - ) -> Result { - let expires_in_secs = NonZeroU64::new(expires_in_secs) - .ok_or_else(|| anyhow!("preview expiry must be greater than zero"))?; - let response = self - .send_api(|client| async move { - client - .generate_preview_url() - .id(run_id.to_string()) - .body(types::PreviewUrlRequest { - expires_in_secs, - port: i64::from(port), - signed, - }) - .send() - .await - }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn create_run_ssh_access( - &self, - run_id: &RunId, - ttl_minutes: f64, - ) -> Result { - let response = self - .send_api(|client| async move { - client - .create_run_ssh_access() - .id(run_id.to_string()) - .body(types::SshAccessRequest { ttl_minutes }) - .send() - .await - }) - .await?; - Ok(response.into_inner()) - } - - pub(crate) async fn list_sandbox_files( - &self, - run_id: &RunId, - path: &str, - depth: Option, - ) -> Result> { - let response = self - .send_api(|client| async move { - let mut request = client - .list_sandbox_files() - .id(run_id.to_string()) - .path(path); - if let Some(depth) = depth.and_then(non_zero_u64_from_u32) { - request = request.depth(depth); - } - request.send().await - }) - .await?; - Ok(response.into_inner().data) - } - - pub(crate) async fn get_sandbox_file(&self, run_id: &RunId, path: &str) -> Result> { - let response = self - .send_api(|client| async move { - client - .get_sandbox_file() - .id(run_id.to_string()) - .path(path) - .send() - .await - }) - .await?; - let mut stream = response.into_inner(); - let mut bytes = Vec::new(); - while let Some(chunk) = stream.next().await { - let chunk = chunk.map_err(|err| anyhow!("{err}"))?; - bytes.extend_from_slice(&chunk); - } - Ok(bytes) - } - - pub(crate) async fn put_sandbox_file( - &self, - run_id: &RunId, - path: &str, - bytes: Vec, - ) -> Result<()> { - self.send_api(|client| async move { - client - .put_sandbox_file() - .id(run_id.to_string()) - .path(path) - .body(bytes.clone()) - .send() - .await - }) - .await?; - Ok(()) - } -} - -fn ensure_refresh_target_transport(target: &user_config::ServerTarget) -> Result<()> { - match is_loopback_or_unix_socket(target)? { - LoopbackClassification::Https - | LoopbackClassification::LoopbackHttp - | LoopbackClassification::UnixSocket => Ok(()), - LoopbackClassification::Rejected => bail!(refresh_transport_error(target)), - } -} - -fn refresh_transport_error(target: &user_config::ServerTarget) -> String { - format!( - "Refusing to send refresh-token credentials over plaintext HTTP to a non-loopback host ({target}). Use HTTPS, or bind the server to 127.0.0.1 / ::1." - ) -} - -fn parse_error_response_value(value: &serde_json::Value) -> (Option, Option) { - let first = value - .get("errors") - .and_then(serde_json::Value::as_array) - .and_then(|errors| errors.first()); - let detail = first - .and_then(|entry| entry.get("detail")) - .and_then(serde_json::Value::as_str) - .map(ToOwned::to_owned); - let code = first - .and_then(|entry| entry.get("code")) - .and_then(serde_json::Value::as_str) - .map(ToOwned::to_owned); - (detail, code) -} - -async fn classify_api_error(err: progenitor_client::Error) -> StructuredApiError -where - E: serde::Serialize + std::fmt::Debug, -{ - match err { - progenitor_client::Error::UnexpectedResponse(response) => { - let status = response.status(); - let body = response.text().await.unwrap_or_default(); - let mut code = None; - if let Ok(value) = serde_json::from_str::(&body) { - let (detail, parsed_code) = parse_error_response_value(&value); - code = parsed_code; - if let Some(detail) = detail { - return StructuredApiError { - error: anyhow!("{detail}"), - failure: Some(ApiFailure { status, code }), - }; - } - } - let error = if body.is_empty() { - anyhow!("request failed with status {status}") - } else { - anyhow!("request failed with status {status}: {body}") - }; - StructuredApiError { - error, - failure: Some(ApiFailure { status, code }), - } - } - other => map_api_error_structured(other), - } -} - -fn map_api_error_structured(err: progenitor_client::Error) -> StructuredApiError -where - E: serde::Serialize + std::fmt::Debug, -{ - match err { - progenitor_client::Error::ErrorResponse(response) => { - let status = response.status(); - let mut code = None; - if let Ok(value) = serde_json::to_value(response.into_inner()) { - let (detail, parsed_code) = parse_error_response_value(&value); - code = parsed_code; - if let Some(detail) = detail { - return StructuredApiError { - error: anyhow!("{detail}"), - failure: Some(ApiFailure { status, code }), - }; - } - } - StructuredApiError { - error: anyhow!("request failed with status {status}"), - failure: Some(ApiFailure { status, code }), - } - } - progenitor_client::Error::UnexpectedResponse(response) => StructuredApiError { - error: anyhow!("request failed with status {}", response.status()), - failure: Some(ApiFailure { - status: response.status(), - code: None, - }), - }, - other => StructuredApiError { - error: anyhow!("{other}"), - failure: None, - }, - } -} - -pub(crate) fn map_api_error(err: progenitor_client::Error) -> anyhow::Error -where - E: serde::Serialize + std::fmt::Debug, -{ - map_api_error_structured(err).error -} - -pub(crate) struct HttpResponseFailure { - pub(crate) status: fabro_http::StatusCode, - pub(crate) headers: fabro_http::HeaderMap, - pub(crate) body: String, - failure: ApiFailure, -} - -fn raw_response_failure_error(failure: &HttpResponseFailure) -> anyhow::Error { - if let Ok(value) = serde_json::from_str::(&failure.body) { - let (detail, _) = parse_error_response_value(&value); - if let Some(detail) = detail { - return anyhow!("{detail}"); - } - } - - if failure.body.is_empty() { - return anyhow!("request failed with status {}", failure.status); - } - - anyhow!( - "request failed with status {}: {}", - failure.status, - failure.body - ) -} - -async fn classify_http_response( - response: fabro_http::Response, -) -> Result> { - if response.status().is_success() { - return Ok(Ok(response)); - } - let status = response.status(); - let headers = response.headers().clone(); - let body = response.text().await.unwrap_or_default(); - let mut code = None; - if let Ok(value) = serde_json::from_str::(&body) { - let (_, parsed_code) = parse_error_response_value(&value); - code = parsed_code; - } - - Ok(Err(HttpResponseFailure { - status, - headers, - body, - failure: ApiFailure { status, code }, - })) -} - -fn is_not_found_error(err: &progenitor_client::Error) -> bool -where - E: serde::Serialize + std::fmt::Debug, -{ - match err { - progenitor_client::Error::ErrorResponse(response) => { - response.status() == fabro_http::StatusCode::NOT_FOUND - } - progenitor_client::Error::UnexpectedResponse(response) => { - response.status() == fabro_http::StatusCode::NOT_FOUND - } - _ => false, - } -} -fn convert_type(value: TInput) -> Result -where - TInput: serde::Serialize, - TOutput: DeserializeOwned, -{ - serde_json::from_value(serde_json::to_value(value)?).map_err(Into::into) -} - -fn non_zero_u64_from_u32(value: u32) -> Option { - NonZeroU64::new(u64::from(value)) -} - -fn non_zero_u64_from_usize(value: usize) -> Option { - u64::try_from(value).ok().and_then(NonZeroU64::new) -} - #[cfg(test)] #[expect( clippy::disallowed_methods, reason = "server-client tests stage local dev-token fixtures with sync std::fs::write" )] mod tests { - use std::path::PathBuf; - - use chrono::Duration as ChronoDuration; + use chrono::Utc; use super::*; @@ -1859,7 +436,7 @@ mod tests { .server_state() .log_path(), dev_token_path: Some(token_path), - started_at: chrono::Utc::now(), + started_at: Utc::now(), }) .unwrap(); @@ -1874,68 +451,13 @@ mod tests { #[test] fn explicit_http_targets_do_not_allow_local_dev_token_fallback() { - let target = - user_config::ServerTarget::HttpUrl("https://fabro.example.com/api/v1".to_string()); + let target = ServerTarget::http_url("https://fabro.example.com/api/v1").unwrap(); assert!(!local_dev_token_fallback(&target)); } #[test] fn unix_socket_targets_keep_local_dev_token_fallback() { - let target = user_config::ServerTarget::UnixSocket(PathBuf::from("/tmp/fabro.sock")); + let target = ServerTarget::unix_socket_path("/tmp/fabro.sock").unwrap(); assert!(local_dev_token_fallback(&target)); } - - fn oauth_entry(login: &str) -> AuthEntry { - let now = chrono::Utc::now(); - AuthEntry { - access_token: format!("access-{login}"), - access_token_expires_at: now + ChronoDuration::minutes(10), - refresh_token: format!("refresh-{login}"), - refresh_token_expires_at: now + ChronoDuration::days(30), - subject: StoredSubject { - idp_issuer: "https://github.com".to_string(), - idp_subject: "12345".to_string(), - login: login.to_string(), - name: format!("Name {login}"), - email: format!("{login}@example.com"), - }, - logged_in_at: now, - } - } - - #[cfg(unix)] - #[tokio::test] - async fn refresh_access_token_rejects_plain_http_non_loopback_targets() { - let temp = tempfile::tempdir().unwrap(); - let auth_store = AuthStore::new(temp.path().join("auth.json")); - let target = user_config::ServerTarget::HttpUrl("http://fabro.example.com".to_string()); - let key = ServerTargetKey::new(&target).unwrap(); - auth_store.put(&key, oauth_entry("octocat")).unwrap(); - - let http_client = cli_http_client_builder().no_proxy().build().unwrap(); - let client = Client { - state: Arc::new(RwLock::new(client_bundle( - "http://fabro.example.com", - http_client, - Some("access-octocat".to_string()), - ))), - base_url: "http://fabro.example.com".to_string(), - refreshable_oauth: Some(RefreshableOAuth { - target: target.clone(), - key: key.clone(), - auth_store: auth_store.clone(), - }), - refresh_lock: Arc::new(Mutex::new(())), - }; - - let err = client - .refresh_access_token("access-octocat") - .await - .unwrap_err(); - assert!( - err.to_string() - .contains("Refusing to send refresh-token credentials over plaintext HTTP") - ); - assert!(auth_store.get(&key).unwrap().is_some()); - } } diff --git a/lib/crates/fabro-cli/src/server_runs.rs b/lib/crates/fabro-cli/src/server_runs.rs index 99010132f..eae615923 100644 --- a/lib/crates/fabro-cli/src/server_runs.rs +++ b/lib/crates/fabro-cli/src/server_runs.rs @@ -3,8 +3,7 @@ use std::sync::Arc; use anyhow::Result; use chrono::{DateTime, Utc}; -use fabro_store::RunSummary; -use fabro_types::{RunId, RunStatus, StatusReason}; +use fabro_types::{RunId, RunStatus, RunSummary, StatusReason}; use crate::server_client::Client; diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index e82cf759c..c151caa4e 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -1,7 +1,8 @@ -use std::fmt; use std::path::{Path, PathBuf}; +use std::str::FromStr; -use anyhow::{Result, bail}; +use anyhow::Result; +pub(crate) use fabro_client::ServerTarget; pub(crate) use fabro_config::user::*; use fabro_types::settings::cli::CliTargetSettings; use fabro_types::settings::{CliSettings, SettingsLayer}; @@ -60,50 +61,10 @@ pub(crate) fn apply_storage_dir_override( layer } -#[derive(Debug, Clone, PartialEq)] -pub(crate) enum ServerTarget { - HttpUrl(String), - UnixSocket(PathBuf), -} - -impl fmt::Display for ServerTarget { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::HttpUrl(api_url) => f.write_str(api_url), - Self::UnixSocket(path) => write!(f, "unix://{}", path.display()), - } - } -} - -pub(crate) fn normalized_http_base_url(api_url: &str) -> &str { - let trimmed = api_url.trim_end_matches('/'); - trimmed.strip_suffix("/api/v1").unwrap_or(trimmed) -} - pub(crate) fn build_public_http_client( target: &ServerTarget, ) -> Result<(fabro_http::HttpClient, String)> { - match target { - ServerTarget::HttpUrl(api_url) => { - let http_client = cli_http_client_builder().build()?; - Ok((http_client, normalized_http_base_url(api_url).to_string())) - } - ServerTarget::UnixSocket(path) => { - #[cfg(unix)] - { - let http_client = cli_http_client_builder() - .unix_socket(path) - .no_proxy() - .build()?; - Ok((http_client, "http://fabro".to_string())) - } - #[cfg(not(unix))] - { - let _ = path; - bail!("Unix-socket HTTP client is not supported on this platform") - } - } - } + target.build_public_http_client() } /// Pull the resolved CLI target configuration out of `[cli.target]`. @@ -125,7 +86,7 @@ fn configured_server_target(settings: &SettingsLayer) -> Result ServerTarget { - ServerTarget::UnixSocket(default_socket_path()) + ServerTarget::unix_socket_path(default_socket_path()).expect("default socket path is absolute") } pub(crate) fn storage_dir(settings: &SettingsLayer) -> anyhow::Result { @@ -153,16 +114,7 @@ pub(crate) fn storage_dir(settings: &SettingsLayer) -> anyhow::Result { } fn parse_server_target(value: &str) -> Result { - if value.starts_with("http://") || value.starts_with("https://") { - return Ok(ServerTarget::HttpUrl(value.to_string())); - } - - let path = Path::new(value); - if path.is_absolute() { - return Ok(ServerTarget::UnixSocket(path.to_path_buf())); - } - - bail!("server target must be an http(s) URL or absolute Unix socket path") + ServerTarget::from_str(value) } fn explicit_server_target(args: &ServerTargetArgs) -> Result> { @@ -219,7 +171,7 @@ mod tests { fn exec_uses_cli_server_target() { assert_eq!( exec_server_target(&server_target_args(Some("https://cli.example.com"))).unwrap(), - Some(ServerTarget::HttpUrl("https://cli.example.com".to_string())) + Some(ServerTarget::http_url("https://cli.example.com").unwrap()) ); } @@ -227,7 +179,7 @@ mod tests { fn exec_supports_explicit_unix_socket_target() { assert_eq!( exec_server_target(&server_target_args(Some("/tmp/fabro.sock"))).unwrap(), - Some(ServerTarget::UnixSocket(PathBuf::from("/tmp/fabro.sock"))) + Some(ServerTarget::unix_socket_path("/tmp/fabro.sock").unwrap()) ); } @@ -249,7 +201,7 @@ url = "https://config.example.com" ); assert_eq!( resolve_server_target(&server_target_args(None), &settings).unwrap(), - ServerTarget::HttpUrl("https://config.example.com".to_string()) + ServerTarget::http_url("https://config.example.com").unwrap() ); } @@ -270,7 +222,7 @@ url = "https://config.example.com" &settings ) .unwrap(), - ServerTarget::HttpUrl("https://cli.example.com".to_string()) + ServerTarget::http_url("https://cli.example.com").unwrap() ); } @@ -279,7 +231,8 @@ url = "https://config.example.com" let settings = SettingsLayer::default(); assert_eq!( resolve_server_target(&server_target_args(None), &settings).unwrap(), - ServerTarget::UnixSocket(dirs::home_dir().unwrap().join(".fabro/fabro.sock")) + ServerTarget::unix_socket_path(dirs::home_dir().unwrap().join(".fabro/fabro.sock")) + .unwrap() ); } @@ -300,7 +253,7 @@ url = "https://config.example.com" &settings ) .unwrap(), - ServerTarget::HttpUrl("https://cli.example.com".to_string()) + ServerTarget::http_url("https://cli.example.com").unwrap() ); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/attach.rs b/lib/crates/fabro-cli/tests/it/cmd/attach.rs index 52b10c838..46f1e53cb 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/attach.rs @@ -635,6 +635,7 @@ fn attach_json_errors_without_prompting_for_human_input() { { "event": "run.queued", "id": "[EVENT_ID]", + "properties": {}, "run_id": "[ULID]", "ts": "[TIMESTAMP]" }, diff --git a/lib/crates/fabro-cli/tests/it/cmd/rewind.rs b/lib/crates/fabro-cli/tests/it/cmd/rewind.rs index 3122c81e7..997b16a7f 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/rewind.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/rewind.rs @@ -129,7 +129,7 @@ fn rewind_preserves_event_history_and_clears_terminal_snapshot_state() { assert!( before_events .iter() - .any(|event| event.payload.as_value()["event"] == "run.completed"), + .any(|event| event.event.event_name() == "run.completed"), "setup run should be completed before rewind" ); @@ -153,28 +153,31 @@ fn rewind_preserves_event_history_and_clears_terminal_snapshot_state() { assert_eq!( after_events[..before_events.len()] .iter() - .map(|event| event.payload.as_value()["event"].as_str().unwrap()) + .map(|event| event.event.event_name()) .collect::>(), before_events .iter() - .map(|event| event.payload.as_value()["event"].as_str().unwrap()) + .map(|event| event.event.event_name()) .collect::>(), "rewind should preserve the prior event prefix" ); assert_eq!( - after_events[before_events.len()].payload.as_value()["event"], + after_events[before_events.len()].event.event_name(), "run.rewound" ); assert_eq!( - after_events[before_events.len() + 1].payload.as_value()["event"], + after_events[before_events.len() + 1].event.event_name(), "checkpoint.completed" ); assert_eq!( - after_events[before_events.len() + 2].payload.as_value()["event"], + after_events[before_events.len() + 2].event.event_name(), "run.submitted" ); assert!( - after_events[before_events.len() + 2].payload.as_value()["properties"]["definition_blob"] + after_events[before_events.len() + 2] + .event + .properties() + .unwrap()["definition_blob"] .is_string(), "rewind should re-emit run.submitted with the definition_blob" ); diff --git a/lib/crates/fabro-cli/tests/it/cmd/runner.rs b/lib/crates/fabro-cli/tests/it/cmd/runner.rs index 947e74ab0..8ac91e074 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/runner.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/runner.rs @@ -29,7 +29,7 @@ fn stored_worker_events(run_dir: &std::path::Path) -> Vec { } fn run_event(event: &EventEnvelope) -> RunEvent { - RunEvent::try_from(&event.payload).expect("stored event should parse") + event.event.clone() } fn assert_worker_succeeded(run_dir: &std::path::Path, stdout: &[u8]) { diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index 679d0d8e4..50499f17a 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -789,14 +789,7 @@ pub(crate) fn wait_for_event_names(run_dir: &Path, expected: &[&str]) { loop { let event_names = run_events(run_dir) .into_iter() - .filter_map(|event| { - event - .payload - .as_value() - .get("event") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string) - }) + .map(|event| event.event.event_name().to_string()) .collect::>(); if expected diff --git a/lib/crates/fabro-cli/tests/it/workflow/mod.rs b/lib/crates/fabro-cli/tests/it/workflow/mod.rs index adcd4103f..d8cac011a 100644 --- a/lib/crates/fabro-cli/tests/it/workflow/mod.rs +++ b/lib/crates/fabro-cli/tests/it/workflow/mod.rs @@ -54,14 +54,9 @@ pub(super) fn completed_nodes(run_dir: &Path) -> Vec { } pub(super) fn has_event(run_dir: &Path, event_name: &str) -> bool { - run_events(run_dir).into_iter().any(|event| { - event - .payload - .as_value() - .get("event") - .and_then(Value::as_str) - == Some(event_name) - }) + run_events(run_dir) + .into_iter() + .any(|event| event.event.event_name() == event_name) } pub(super) fn store_dump_export(context: &TestContext, run_id: &str) -> PathBuf { diff --git a/lib/crates/fabro-client/Cargo.toml b/lib/crates/fabro-client/Cargo.toml new file mode 100644 index 000000000..338496b20 --- /dev/null +++ b/lib/crates/fabro-client/Cargo.toml @@ -0,0 +1,37 @@ +[package] +name = "fabro-client" +edition.workspace = true +version.workspace = true +publish = false +license.workspace = true +description = "Typed HTTP client for the Fabro API" + +[lib] +doctest = false + +[lints] +workspace = true + +[dependencies] +anyhow.workspace = true +bytes.workspace = true +chrono = { workspace = true, features = ["serde"] } +fabro-api = { path = "../fabro-api" } +fabro-http.workspace = true +fabro-model = { path = "../fabro-model" } +fabro-types = { path = "../fabro-types" } +fabro-util = { path = "../fabro-util" } +fs2.workspace = true +futures.workspace = true +libc = "0.2" +progenitor-client = "0.13" +rand.workspace = true +serde.workspace = true +serde_json.workspace = true +thiserror.workspace = true +tokio.workspace = true +tokio-util.workspace = true +tracing.workspace = true + +[dev-dependencies] +tempfile = "3" diff --git a/lib/crates/fabro-cli/src/auth_store.rs b/lib/crates/fabro-client/src/auth_store.rs similarity index 68% rename from lib/crates/fabro-cli/src/auth_store.rs rename to lib/crates/fabro-client/src/auth_store.rs index f88291b1c..01185ac72 100644 --- a/lib/crates/fabro-cli/src/auth_store.rs +++ b/lib/crates/fabro-client/src/auth_store.rs @@ -8,9 +8,9 @@ )] use std::collections::BTreeMap; +use std::fs; use std::io::Write as _; use std::path::{Path, PathBuf}; -use std::{fmt, fs}; use chrono::{DateTime, Utc}; use fs2::FileExt; @@ -18,68 +18,31 @@ use rand::Rng; use serde::{Deserialize, Serialize}; use thiserror::Error; -use crate::user_config::{ServerTarget, normalized_http_base_url}; +use crate::target::ServerTarget; const AUTH_FILE_ENV: &str = "FABRO_AUTH_FILE"; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub(crate) struct StoredSubject { - pub(crate) idp_issuer: String, - pub(crate) idp_subject: String, - pub(crate) login: String, - pub(crate) name: String, - pub(crate) email: String, +pub struct StoredSubject { + pub idp_issuer: String, + pub idp_subject: String, + pub login: String, + pub name: String, + pub email: String, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub(crate) struct AuthEntry { - pub(crate) access_token: String, - pub(crate) access_token_expires_at: DateTime, - pub(crate) refresh_token: String, - pub(crate) refresh_token_expires_at: DateTime, - pub(crate) subject: StoredSubject, - pub(crate) logged_in_at: DateTime, -} - -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub(crate) struct ServerTargetKey(String); - -impl ServerTargetKey { - pub(crate) fn new(target: &ServerTarget) -> Result { - match target { - ServerTarget::HttpUrl(api_url) => canonical_http_target(api_url).map(Self), - ServerTarget::UnixSocket(path) => Ok(Self(format!( - "unix://{}", - canonical_socket_path(path)?.display() - ))), - } - } - - fn from_canonical(canonical: String) -> Self { - Self(canonical) - } - - pub(crate) fn as_str(&self) -> &str { - &self.0 - } -} - -impl fmt::Display for ServerTargetKey { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(&self.0) - } -} - -impl TryFrom<&ServerTarget> for ServerTargetKey { - type Error = AuthStoreError; - - fn try_from(value: &ServerTarget) -> Result { - Self::new(value) - } +pub struct AuthEntry { + pub access_token: String, + pub access_token_expires_at: DateTime, + pub refresh_token: String, + pub refresh_token_expires_at: DateTime, + pub subject: StoredSubject, + pub logged_in_at: DateTime, } #[derive(Debug, Error)] -pub(crate) enum AuthStoreError { +pub enum AuthStoreError { #[allow( dead_code, reason = "This platform-gated variant is exercised on non-Unix targets." @@ -118,7 +81,7 @@ pub(crate) enum AuthStoreError { } #[derive(Debug, Error)] -pub(crate) enum LockError { +pub enum LockError { #[error( "the filesystem backing {path} does not support file locking; move the auth store to a local filesystem or set {AUTH_FILE_ENV} to a local path" )] @@ -131,7 +94,7 @@ pub(crate) enum LockError { } #[derive(Debug, Clone)] -pub(crate) struct AuthStore { +pub struct AuthStore { path: PathBuf, } @@ -152,47 +115,45 @@ impl Default for AuthStore { } impl AuthStore { - pub(crate) fn new(path: PathBuf) -> Self { + pub fn new(path: PathBuf) -> Self { Self { path } } - pub(crate) fn get(&self, key: &ServerTargetKey) -> Result, AuthStoreError> { + pub fn get(&self, target: &ServerTarget) -> Result, AuthStoreError> { if !self.path.exists() { return Ok(None); } + let key = key_for_target(target); self.with_shared_lock(|| { let file = self.read_auth_file()?; - Ok(file.servers.get(key.as_str()).cloned()) + Ok(file.servers.get(&key).cloned()) }) } - pub(crate) fn put( - &self, - key: &ServerTargetKey, - entry: AuthEntry, - ) -> Result<(), AuthStoreError> { + pub fn put(&self, target: &ServerTarget, entry: AuthEntry) -> Result<(), AuthStoreError> { #[cfg(not(unix))] { - let _ = (key, entry); + let _ = (target, entry); Err(AuthStoreError::UnsupportedPlatform) } #[cfg(unix)] { + let key = key_for_target(target); self.ensure_parent_dir()?; self.with_exclusive_lock(|| { let mut file = self.read_auth_file_if_exists()?; - file.servers.insert(key.to_string(), entry); + file.servers.insert(key, entry); self.write_auth_file(&file) }) } } - pub(crate) fn remove(&self, key: &ServerTargetKey) -> Result { + pub fn remove(&self, target: &ServerTarget) -> Result { #[cfg(not(unix))] { - let _ = key; + let _ = target; Err(AuthStoreError::UnsupportedPlatform) } @@ -202,28 +163,28 @@ impl AuthStore { return Ok(false); } + let key = key_for_target(target); self.ensure_parent_dir()?; self.with_exclusive_lock(|| { let mut file = self.read_auth_file_if_exists()?; - let removed = file.servers.remove(key.as_str()).is_some(); + let removed = file.servers.remove(&key).is_some(); self.write_auth_file(&file)?; Ok(removed) }) } } - pub(crate) fn list(&self) -> Result, AuthStoreError> { + pub fn list(&self) -> Result, AuthStoreError> { if !self.path.exists() { return Ok(Vec::new()); } self.with_shared_lock(|| { let file = self.read_auth_file()?; - Ok(file - .servers + file.servers .into_iter() - .map(|(key, entry)| (ServerTargetKey::from_canonical(key), entry)) - .collect()) + .map(|(key, entry)| Ok((parse_stored_target(&key)?, entry))) + .collect::, AuthStoreError>>() }) } @@ -347,48 +308,26 @@ impl AuthStore { } } -fn canonical_http_target(api_url: &str) -> Result { - let trimmed = api_url.trim(); - let normalized = normalized_http_base_url(trimmed); - let url = - fabro_http::Url::parse(normalized).map_err(|_| AuthStoreError::InvalidServerTarget { - value: api_url.to_string(), - })?; - let scheme = url.scheme().to_ascii_lowercase(); - let Some(host) = url.host_str() else { - return Err(AuthStoreError::InvalidServerTarget { - value: api_url.to_string(), - }); - }; - let host = host.to_ascii_lowercase(); - let Some(port) = url.port_or_known_default() else { - return Err(AuthStoreError::InvalidServerTarget { - value: api_url.to_string(), - }); - }; - let default_port = match scheme.as_str() { - "http" => 80, - "https" => 443, - _ => { - return Err(AuthStoreError::InvalidServerTarget { - value: api_url.to_string(), - }); - } - }; - if port == default_port { - Ok(format!("{scheme}://{host}")) - } else { - Ok(format!("{scheme}://{host}:{port}")) - } +fn key_for_target(target: &ServerTarget) -> String { + target.to_string() } -fn canonical_socket_path(path: &Path) -> Result { - if !path.is_absolute() { - return Err(AuthStoreError::InvalidServerTarget { - value: path.display().to_string(), +fn parse_stored_target(value: &str) -> Result { + if let Some(path) = value.strip_prefix("unix://") { + return ServerTarget::unix_socket_path(path).map_err(|_| { + AuthStoreError::InvalidServerTarget { + value: value.to_string(), + } }); } - Ok(fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf())) + if value.starts_with("http://") || value.starts_with("https://") { + return ServerTarget::http_url(value).map_err(|_| AuthStoreError::InvalidServerTarget { + value: value.to_string(), + }); + } + Err(AuthStoreError::InvalidServerTarget { + value: value.to_string(), + }) } #[cfg(unix)] @@ -438,8 +377,8 @@ mod tests { #[cfg(unix)] use super::{AUTH_FILE_ENV, LockError, classify_lock_error}; - use super::{AuthEntry, AuthStore, ServerTargetKey, StoredSubject}; - use crate::user_config::ServerTarget; + use super::{AuthEntry, AuthStore, StoredSubject, key_for_target}; + use crate::target::ServerTarget; fn entry(login: &str) -> AuthEntry { let now = chrono::Utc::now(); @@ -460,7 +399,7 @@ mod tests { } fn https_target(value: &str) -> ServerTarget { - ServerTarget::HttpUrl(value.to_string()) + ServerTarget::http_url(value).unwrap() } #[cfg(unix)] @@ -468,11 +407,11 @@ mod tests { fn round_trips_https_entry() { let temp = tempfile::tempdir().unwrap(); let store = AuthStore::new(temp.path().join("auth.json")); - let key = ServerTargetKey::new(&https_target("https://fabro.example.com")).unwrap(); + let target = https_target("https://fabro.example.com"); - store.put(&key, entry("octocat")).unwrap(); + store.put(&target, entry("octocat")).unwrap(); - let saved = store.get(&key).unwrap().unwrap(); + let saved = store.get(&target).unwrap().unwrap(); assert_eq!(saved.subject.login, "octocat"); } @@ -481,11 +420,11 @@ mod tests { fn round_trips_loopback_http_entry() { let temp = tempfile::tempdir().unwrap(); let store = AuthStore::new(temp.path().join("auth.json")); - let key = ServerTargetKey::new(&https_target("http://127.0.0.1:3000")).unwrap(); + let target = https_target("http://127.0.0.1:3000"); - store.put(&key, entry("alice")).unwrap(); + store.put(&target, entry("alice")).unwrap(); - let saved = store.get(&key).unwrap().unwrap(); + let saved = store.get(&target).unwrap().unwrap(); assert_eq!(saved.subject.login, "alice"); } @@ -496,19 +435,19 @@ mod tests { let socket = temp.path().join("fabro.sock"); std::fs::write(&socket, "").unwrap(); let store = AuthStore::new(temp.path().join("auth.json")); - let key = ServerTargetKey::new(&ServerTarget::UnixSocket(socket)).unwrap(); + let target = ServerTarget::unix_socket_path(socket).unwrap(); - store.put(&key, entry("unix")).unwrap(); + store.put(&target, entry("unix")).unwrap(); - let saved = store.get(&key).unwrap().unwrap(); + let saved = store.get(&target).unwrap().unwrap(); assert_eq!(saved.subject.login, "unix"); } #[test] fn https_normalization_collapses_equivalent_urls() { - let a = ServerTargetKey::new(&https_target("https://EXAMPLE.COM/")).unwrap(); - let b = ServerTargetKey::new(&https_target("https://example.com:443")).unwrap(); - let c = ServerTargetKey::new(&https_target("https://example.com")).unwrap(); + let a = key_for_target(&https_target("https://EXAMPLE.COM/")); + let b = key_for_target(&https_target("https://example.com:443")); + let c = key_for_target(&https_target("https://example.com")); assert_eq!(a, b); assert_eq!(b, c); @@ -516,36 +455,34 @@ mod tests { #[test] fn distinct_unix_socket_paths_do_not_collide() { - let a = - ServerTargetKey::new(&ServerTarget::UnixSocket(PathBuf::from("/tmp/a.sock"))).unwrap(); - let b = - ServerTargetKey::new(&ServerTarget::UnixSocket(PathBuf::from("/tmp/b.sock"))).unwrap(); + let a = key_for_target(&ServerTarget::unix_socket_path("/tmp/a.sock").unwrap()); + let b = key_for_target(&ServerTarget::unix_socket_path("/tmp/b.sock").unwrap()); assert_ne!(a, b); } #[cfg(unix)] #[test] - fn canonicalizes_symlinked_socket_paths() { + fn preserves_distinct_symlinked_socket_paths() { let temp = tempfile::tempdir().unwrap(); let socket = temp.path().join("fabro.sock"); let link = temp.path().join("fabro-link.sock"); std::fs::write(&socket, "").unwrap(); std::os::unix::fs::symlink(&socket, &link).unwrap(); - let direct = ServerTargetKey::new(&ServerTarget::UnixSocket(socket)).unwrap(); - let via_link = ServerTargetKey::new(&ServerTarget::UnixSocket(link)).unwrap(); + let direct = key_for_target(&ServerTarget::unix_socket_path(socket).unwrap()); + let via_link = key_for_target(&ServerTarget::unix_socket_path(link).unwrap()); - assert_eq!(direct, via_link); + assert_ne!(direct, via_link); } #[test] fn missing_file_returns_empty_results() { let temp = tempfile::tempdir().unwrap(); let store = AuthStore::new(temp.path().join("auth.json")); - let key = ServerTargetKey::new(&https_target("https://fabro.example.com")).unwrap(); + let target = https_target("https://fabro.example.com"); - assert!(store.get(&key).unwrap().is_none()); + assert!(store.get(&target).unwrap().is_none()); assert!(store.list().unwrap().is_empty()); } @@ -555,9 +492,9 @@ mod tests { let path = temp.path().join("auth.json"); std::fs::write(&path, "{not-json").unwrap(); let store = AuthStore::new(path.clone()); - let key = ServerTargetKey::new(&https_target("https://fabro.example.com")).unwrap(); + let target = https_target("https://fabro.example.com"); - let err = store.get(&key).unwrap_err(); + let err = store.get(&target).unwrap_err(); assert!(err.to_string().contains(&path.display().to_string())); } @@ -566,21 +503,21 @@ mod tests { fn concurrent_puts_do_not_corrupt_file() { let temp = tempfile::tempdir().unwrap(); let store = Arc::new(AuthStore::new(temp.path().join("auth.json"))); - let key = ServerTargetKey::new(&https_target("https://fabro.example.com")).unwrap(); + let target = https_target("https://fabro.example.com"); let mut tasks = Vec::new(); for login in ["alice", "bob"] { let store = Arc::clone(&store); - let key = key.clone(); + let target = target.clone(); tasks.push(thread::spawn(move || { - store.put(&key, entry(login)).unwrap(); + store.put(&target, entry(login)).unwrap(); })); } for task in tasks { task.join().unwrap(); } - let saved = store.get(&key).unwrap().unwrap(); + let saved = store.get(&target).unwrap().unwrap(); assert!(matches!(saved.subject.login.as_str(), "alice" | "bob")); } @@ -591,9 +528,9 @@ mod tests { let temp = tempfile::tempdir().unwrap(); let store = AuthStore::new(temp.path().join("auth.json")); - let key = ServerTargetKey::new(&https_target("https://fabro.example.com")).unwrap(); + let target = https_target("https://fabro.example.com"); - store.put(&key, entry("octocat")).unwrap(); + store.put(&target, entry("octocat")).unwrap(); let mode = std::fs::metadata(temp.path().join("auth.json")) .unwrap() @@ -608,9 +545,9 @@ mod tests { fn put_returns_unsupported_platform() { let temp = tempfile::tempdir().unwrap(); let store = AuthStore::new(temp.path().join("auth.json")); - let key = ServerTargetKey::new(&https_target("https://fabro.example.com")).unwrap(); + let target = https_target("https://fabro.example.com"); - let err = store.put(&key, entry("octocat")).unwrap_err(); + let err = store.put(&target, entry("octocat")).unwrap_err(); assert!(err.to_string().contains("not supported on this platform")); } diff --git a/lib/crates/fabro-client/src/client.rs b/lib/crates/fabro-client/src/client.rs new file mode 100644 index 000000000..57098e055 --- /dev/null +++ b/lib/crates/fabro-client/src/client.rs @@ -0,0 +1,1392 @@ +use std::collections::VecDeque; +use std::future::Future; +use std::num::NonZeroU64; +use std::path::Path; +use std::sync::{Arc, RwLock}; + +use anyhow::{Context as _, Result, anyhow, bail}; +use bytes::Bytes; +use fabro_api::types; +use fabro_http::header::{AUTHORIZATION, CONTENT_LENGTH, CONTENT_TYPE}; +use fabro_http::multipart::{Form, Part}; +use fabro_model::Model; +use fabro_types::{ + ArtifactUpload, EventEnvelope, RunBlobId, RunEvent, RunId, RunProjection, RunSummary, StageId, +}; +use futures::StreamExt; +use futures::future::BoxFuture; +use serde::{Deserialize, Serialize}; +use tokio::fs::File; +use tokio::sync::Mutex; +use tokio_util::io::ReaderStream; + +use crate::credential::Credential; +use crate::error::{ + ApiError, ApiFailure, classify_api_error, classify_http_response, convert_type, + is_not_found_error, map_api_error, raw_response_failure_error, +}; +use crate::loopback::LoopbackClassification; +use crate::session::OAuthSession; +use crate::target::ServerTarget; +use crate::{AuthEntry, StoredSubject, sse}; + +type TransportFuture = BoxFuture<'static, Result<(fabro_http::HttpClient, String)>>; + +pub struct RunEventStream { + stream: progenitor_client::ByteStream, + pending_bytes: Vec, + buffered_events: VecDeque, +} + +#[derive(Clone)] +struct ClientState { + client: fabro_api::ApiClient, + http_client: fabro_http::HttpClient, + bearer_token: Option, + base_url: String, +} + +#[derive(Clone)] +pub struct Client { + state: Arc>, + oauth_session: Option, + refresh_lock: Arc>, + transport_connector: Option, +} + +#[derive(Clone)] +pub struct TransportConnector { + connect: Arc) -> TransportFuture + Send + Sync>, +} + +#[derive(Default)] +pub struct ClientBuilder { + target: Option, + credential: Option, + oauth_session: Option, + transport: Option<(String, fabro_http::HttpClient)>, + transport_connector: Option, +} + +#[derive(Debug, Deserialize)] +struct CliTokenResponse { + access_token: String, + access_token_expires_at: chrono::DateTime, + refresh_token: String, + refresh_token_expires_at: chrono::DateTime, + subject: CliTokenSubject, +} + +#[derive(Debug, Deserialize)] +struct CliTokenSubject { + idp_issuer: String, + idp_subject: String, + login: String, + name: String, + email: String, +} + +#[derive(Debug, Deserialize)] +struct OAuthErrorBody { + error: String, + #[serde(default)] + error_description: Option, +} + +#[derive(Debug, Serialize)] +struct ArtifactBatchUploadManifest { + entries: Vec, +} + +#[derive(Debug, Serialize)] +struct ArtifactBatchUploadEntry { + part: String, + path: String, + #[serde(skip_serializing_if = "Option::is_none")] + sha256: Option, + #[serde(skip_serializing_if = "Option::is_none")] + expected_bytes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + content_type: Option, +} + +impl RunEventStream { + #[must_use] + pub fn new(stream: progenitor_client::ByteStream) -> Self { + Self { + stream, + pending_bytes: Vec::new(), + buffered_events: VecDeque::new(), + } + } + + pub async fn next_event(&mut self) -> Result> { + loop { + if let Some(event) = self.buffered_events.pop_front() { + return Ok(Some(event)); + } + + if let Some(chunk) = self.stream.next().await { + let chunk = chunk.map_err(|err| anyhow!("{err}"))?; + self.pending_bytes.extend_from_slice(&chunk); + self.buffer_sse_events(false)?; + } else { + self.buffer_sse_events(true)?; + return Ok(self.buffered_events.pop_front()); + } + } + } + + fn buffer_sse_events(&mut self, finalize: bool) -> Result<()> { + for payload in sse::drain_sse_payloads(&mut self.pending_bytes, finalize) { + self.buffered_events + .push_back(serde_json::from_str(&payload)?); + } + Ok(()) + } +} + +impl TransportConnector { + pub fn new(connect: F) -> Self + where + F: Fn(Option) -> Fut + Send + Sync + 'static, + Fut: Future> + Send + 'static, + { + Self { + connect: Arc::new(move |bearer_token| Box::pin(connect(bearer_token))), + } + } + + pub async fn connect( + &self, + bearer_token: Option, + ) -> Result<(fabro_http::HttpClient, String)> { + (self.connect)(bearer_token).await + } +} + +impl ClientBuilder { + #[must_use] + pub fn target(mut self, target: ServerTarget) -> Self { + self.target = Some(target); + self + } + + #[must_use] + pub fn credential(mut self, credential: Credential) -> Self { + self.credential = Some(credential); + self + } + + #[must_use] + pub fn oauth_session(mut self, oauth_session: OAuthSession) -> Self { + self.oauth_session = Some(oauth_session); + self + } + + #[must_use] + pub fn transport( + mut self, + base_url: impl Into, + http_client: fabro_http::HttpClient, + ) -> Self { + self.transport = Some((base_url.into(), http_client)); + self + } + + #[must_use] + pub fn transport_connector(mut self, transport_connector: TransportConnector) -> Self { + self.transport_connector = Some(transport_connector); + self + } + + pub async fn connect(self) -> Result { + let bearer_token = self + .credential + .as_ref() + .map(Credential::bearer_token) + .map(ToOwned::to_owned); + let target = self.target.clone().or_else(|| { + self.oauth_session + .as_ref() + .map(|session| session.target.clone()) + }); + let transport_connector = self + .transport_connector + .or_else(|| target.map(default_transport_connector)); + + let state = if let Some((base_url, http_client)) = self.transport { + client_state(base_url, http_client, bearer_token.clone()) + } else { + let Some(transport_connector) = transport_connector.clone() else { + bail!("client builder requires a target, transport, or transport connector"); + }; + let (http_client, base_url) = transport_connector.connect(bearer_token.clone()).await?; + client_state(base_url, http_client, bearer_token.clone()) + }; + + Ok(Client { + state: Arc::new(RwLock::new(state)), + oauth_session: self.oauth_session, + refresh_lock: Arc::new(Mutex::new(())), + transport_connector, + }) + } +} + +impl Client { + #[must_use] + pub fn builder() -> ClientBuilder { + ClientBuilder::default() + } + + #[must_use] + pub fn from_http_client( + base_url: impl Into, + http_client: fabro_http::HttpClient, + ) -> Self { + Self { + state: Arc::new(RwLock::new(client_state( + base_url.into(), + http_client, + None, + ))), + oauth_session: None, + refresh_lock: Arc::new(Mutex::new(())), + transport_connector: None, + } + } + + pub fn new_no_proxy(base_url: &str) -> Result { + let http_client = fabro_http::HttpClientBuilder::new().no_proxy().build()?; + Ok(Self::from_http_client(base_url.to_string(), http_client)) + } + + #[must_use] + pub fn clone_for_reuse(&self) -> Self { + self.clone() + } + + #[must_use] + pub fn api_client(&self) -> fabro_api::ApiClient { + self.current_state().client + } + + #[must_use] + pub fn http_client(&self) -> fabro_http::HttpClient { + self.current_state().http_client + } + + #[must_use] + pub fn base_url(&self) -> String { + self.current_state().base_url + } + + fn current_state(&self) -> ClientState { + self.state + .read() + .expect("client state lock should not be poisoned") + .clone() + } + + fn replace_state(&self, state: ClientState) { + *self + .state + .write() + .expect("client state lock should not be poisoned") = state; + } + + async fn send_api( + &self, + request: F, + ) -> Result> + where + F: FnOnce(fabro_api::ApiClient) -> Fut + Clone, + Fut: Future< + Output = std::result::Result< + progenitor_client::ResponseValue, + progenitor_client::Error, + >, + >, + E: serde::Serialize + std::fmt::Debug, + { + let state = self.current_state(); + match request.clone()(state.client.clone()).await { + Ok(response) => Ok(response), + Err(err) => { + let mapped = classify_api_error(err).await; + if self.should_refresh(mapped.failure.as_ref()) { + if let Some(failed_token) = state.bearer_token.as_deref() { + self.refresh_access_token(failed_token).await?; + let state = self.current_state(); + return request(state.client.clone()).await.map_err(map_api_error); + } + } + Err(mapped.error) + } + } + } + + fn should_refresh(&self, failure: Option<&ApiFailure>) -> bool { + self.oauth_session.is_some() + && failure.is_some_and(|failure| { + failure.status == fabro_http::StatusCode::UNAUTHORIZED + && failure.code.as_deref() == Some("access_token_expired") + }) + } + + async fn refresh_access_token(&self, failed_access_token: &str) -> Result<()> { + let Some(oauth_session) = &self.oauth_session else { + bail!("CLI session has expired. Run `fabro auth login` again."); + }; + + let _guard = self.refresh_lock.lock().await; + let current_state = self.current_state(); + if current_state.bearer_token.as_deref() != Some(failed_access_token) { + return Ok(()); + } + + let Some(entry) = oauth_session.auth_store.get(&oauth_session.target)? else { + self.rebuild_with_fallback(oauth_session).await?; + bail!("CLI session has expired. Run `fabro auth login` again."); + }; + if entry.refresh_token_expires_at <= chrono::Utc::now() { + oauth_session.auth_store.remove(&oauth_session.target)?; + self.rebuild_with_fallback(oauth_session).await?; + bail!("CLI session has expired. Run `fabro auth login` again."); + } + ensure_refresh_target_transport(&oauth_session.target)?; + + let (http_client, base_url) = oauth_session.target.build_public_http_client()?; + let response = http_client + .post(format!("{base_url}/auth/cli/refresh")) + .header(AUTHORIZATION, format!("Bearer {}", entry.refresh_token)) + .send() + .await?; + + if response.status().is_success() { + let tokens = response + .json::() + .await + .context("failed to parse CLI auth refresh response")?; + let entry = AuthEntry { + access_token: tokens.access_token.clone(), + access_token_expires_at: tokens.access_token_expires_at, + refresh_token: tokens.refresh_token.clone(), + refresh_token_expires_at: tokens.refresh_token_expires_at, + subject: StoredSubject { + idp_issuer: tokens.subject.idp_issuer, + idp_subject: tokens.subject.idp_subject, + login: tokens.subject.login, + name: tokens.subject.name, + email: tokens.subject.email, + }, + logged_in_at: entry.logged_in_at, + }; + oauth_session + .auth_store + .put(&oauth_session.target, entry.clone()) + .context("failed to persist refreshed CLI auth tokens")?; + self.rebuild_client(Some(entry.access_token)).await?; + return Ok(()); + } + + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + let parsed_error = serde_json::from_str::(&body).ok(); + if parsed_error.as_ref().is_some_and(|error| { + matches!( + error.error.as_str(), + "refresh_token_expired" | "refresh_token_revoked" + ) + }) { + oauth_session.auth_store.remove(&oauth_session.target)?; + self.rebuild_with_fallback(oauth_session).await?; + } + + if let Some(parsed_error) = parsed_error { + let message = parsed_error + .error_description + .filter(|value| !value.is_empty()) + .unwrap_or_else(|| format!("request failed with status {status}")); + bail!("{message}"); + } + if body.is_empty() { + bail!("request failed with status {status}"); + } + bail!("request failed with status {status}: {body}"); + } + + async fn rebuild_with_fallback(&self, oauth_session: &OAuthSession) -> Result<()> { + let credential = oauth_session.resolve_fallback(); + self.rebuild_client( + credential + .as_ref() + .map(Credential::bearer_token) + .map(ToOwned::to_owned), + ) + .await + } + + async fn rebuild_client(&self, bearer_token: Option) -> Result<()> { + let Some(transport_connector) = &self.transport_connector else { + bail!("client transport cannot be rebuilt"); + }; + let (http_client, base_url) = transport_connector.connect(bearer_token.clone()).await?; + self.replace_state(client_state(base_url, http_client, bearer_token)); + Ok(()) + } + + pub async fn send_http_response( + &self, + request: F, + ) -> Result> + where + F: FnOnce(fabro_http::HttpClient) -> Fut + Clone, + Fut: Future>, + T: Into, + { + let state = self.current_state(); + let response = request.clone()(state.http_client.clone()) + .await + .map_err(Into::into)?; + match classify_http_response(response).await? { + Ok(response) => Ok(Ok(response)), + Err(failure) => { + if self.should_refresh(Some(failure.api_failure())) { + if let Some(failed_token) = state.bearer_token.as_deref() { + self.refresh_access_token(failed_token).await?; + let state = self.current_state(); + let response = request(state.http_client.clone()) + .await + .map_err(Into::into)?; + return classify_http_response(response).await; + } + } + Ok(Err(failure)) + } + } + } + + async fn send_http(&self, request: F) -> Result + where + F: FnOnce(fabro_http::HttpClient) -> Fut + Clone, + Fut: Future>, + T: Into, + { + match self.send_http_response(request).await? { + Ok(response) => Ok(response), + Err(failure) => Err(raw_response_failure_error(&failure)), + } + } + + pub async fn retrieve_resolved_server_settings(&self) -> Result { + let url = format!("{}/api/v1/settings?view=resolved", self.base_url()); + let response = self + .send_http(|http_client| async move { http_client.get(&url).send().await }) + .await?; + + let marker = response + .headers() + .get("x-fabro-settings-view") + .and_then(|value| value.to_str().ok()); + if marker != Some("resolved") { + bail!( + "server does not support resolved settings view; upgrade the server or use --local" + ); + } + + response + .json::() + .await + .context("server returned invalid JSON for the resolved settings view") + } + + pub async fn create_run_from_manifest(&self, manifest: types::RunManifest) -> Result { + let response = self + .send_api( + |client| async move { client.create_run().body(manifest.clone()).send().await }, + ) + .await?; + let status = response.into_inner(); + status + .id + .parse() + .map_err(|err| anyhow!("invalid run ID from server: {err}")) + } + + pub async fn list_secrets(&self) -> Result> { + let response = self + .send_api(|client| async move { client.list_secrets().send().await }) + .await?; + Ok(response.into_inner().data) + } + + pub async fn create_secret( + &self, + body: types::CreateSecretRequest, + ) -> Result { + let response = self + .send_api( + |client| async move { client.create_secret().body(body.clone()).send().await }, + ) + .await?; + Ok(response.into_inner()) + } + + pub async fn delete_secret_by_name(&self, name: &str) -> Result<()> { + self.send_api(|client| async move { + client + .delete_secret_by_name() + .body(types::DeleteSecretRequest { + name: name.to_string(), + }) + .send() + .await + }) + .await?; + Ok(()) + } + + pub async fn list_models( + &self, + provider: Option<&str>, + query: Option<&str>, + ) -> Result> { + let mut offset = 0u64; + let mut models = Vec::new(); + + loop { + let response = self + .send_api(|client| async move { + let mut request = client.list_models().page_limit(100u64).page_offset(offset); + if let Some(provider) = provider { + request = request.provider(provider.to_string()); + } + if let Some(query) = query { + request = request.query(query.to_string()); + } + request.send().await + }) + .await?; + let parsed = response.into_inner(); + let count = parsed.data.len() as u64; + models.extend(convert_type::<_, Vec>(parsed.data)?); + if !parsed.meta.has_more { + break; + } + offset += count; + } + + Ok(models) + } + + pub async fn test_model( + &self, + id: &str, + mode: Option, + ) -> Result { + let response = self + .send_api(|client| async move { + let mut request = client.test_model().id(id.to_string()); + if let Some(mode) = mode { + request = request.mode(mode); + } + request.send().await + }) + .await?; + Ok(response.into_inner()) + } + + pub async fn attach_events(&self, run_ids: &[String]) -> Result { + let response = self + .send_api(|client| async move { + let mut request = client.attach_events(); + if !run_ids.is_empty() { + request = request.run_id(run_ids.join(",")); + } + request.send().await + }) + .await?; + Ok(response.into_inner()) + } + + pub async fn get_system_info(&self) -> Result { + let response = self + .send_api(|client| async move { client.get_system_info().send().await }) + .await?; + Ok(response.into_inner()) + } + + pub async fn get_system_disk_usage(&self, verbose: bool) -> Result { + let response = self + .send_api(|client| async move { + client.get_system_disk_usage().verbose(verbose).send().await + }) + .await?; + Ok(response.into_inner()) + } + + pub async fn prune_runs( + &self, + body: types::PruneRunsRequest, + ) -> Result { + let response = self + .send_api(|client| async move { client.prune_runs().body(body.clone()).send().await }) + .await?; + Ok(response.into_inner()) + } + + pub async fn get_health(&self) -> Result<()> { + self.send_api(|client| async move { client.get_health().send().await }) + .await?; + Ok(()) + } + + pub async fn run_diagnostics(&self) -> Result { + let response = self + .send_api(|client| async move { client.run_diagnostics().send().await }) + .await?; + Ok(response.into_inner()) + } + + pub async fn get_github_repo( + &self, + owner: &str, + name: &str, + ) -> Result { + let response = self + .send_api(|client| async move { + client + .get_github_repo() + .owner(owner.to_string()) + .name(name.to_string()) + .send() + .await + }) + .await?; + Ok(response.into_inner()) + } + + pub async fn run_preflight( + &self, + manifest: types::RunManifest, + ) -> Result { + self.send_api( + |client| async move { client.run_preflight().body(manifest.clone()).send().await }, + ) + .await + .map(progenitor_client::ResponseValue::into_inner) + } + + pub async fn render_workflow_graph( + &self, + request: types::RenderWorkflowGraphRequest, + ) -> Result> { + let response = self + .send_api(|client| async move { + client + .render_workflow_graph() + .body(request.clone()) + .send() + .await + }) + .await?; + let mut stream = response.into_inner(); + let mut bytes = Vec::new(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|err| anyhow!("{err}"))?; + bytes.extend_from_slice(&chunk); + } + Ok(bytes) + } + + pub async fn start_run(&self, run_id: &RunId, resume: bool) -> Result<()> { + self.send_api(|client| async move { + client + .start_run() + .id(run_id.to_string()) + .body(types::StartRunRequest { resume }) + .send() + .await + }) + .await?; + Ok(()) + } + + pub async fn cancel_run(&self, run_id: &RunId) -> Result<()> { + self.send_api( + |client| async move { client.cancel_run().id(run_id.to_string()).send().await }, + ) + .await?; + Ok(()) + } + + pub async fn archive_run(&self, run_id: &RunId) -> Result<()> { + self.send_api( + |client| async move { client.archive_run().id(run_id.to_string()).send().await }, + ) + .await?; + Ok(()) + } + + pub async fn unarchive_run(&self, run_id: &RunId) -> Result<()> { + self.send_api(|client| async move { + client.unarchive_run().id(run_id.to_string()).send().await + }) + .await?; + Ok(()) + } + + pub async fn list_store_runs(&self) -> Result> { + let mut all_runs = Vec::new(); + let mut offset = 0_u64; + let limit = 100_u64; + + loop { + let response = self + .send_api(|client| async move { + client + .list_runs() + .page_limit(limit) + .page_offset(offset) + .include_archived(true) + .send() + .await + }) + .await?; + let parsed = response.into_inner(); + let batch = parsed + .data + .into_iter() + .map(convert_type) + .collect::>>()?; + let batch_len = batch.len() as u64; + all_runs.extend(batch); + + if !parsed.meta.has_more || batch_len == 0 { + break; + } + offset += batch_len; + } + + Ok(all_runs) + } + + pub async fn retrieve_run(&self, run_id: &RunId) -> Result { + let response = self + .send_api( + |client| async move { client.retrieve_run().id(run_id.to_string()).send().await }, + ) + .await?; + convert_type(response.into_inner()) + } + + pub async fn resolve_run(&self, selector: &str) -> Result { + let response = self + .send_api(|client| async move { + client + .resolve_run() + .selector(selector.to_string()) + .send() + .await + }) + .await?; + convert_type(response.into_inner()) + } + + pub async fn get_run_state(&self, run_id: &RunId) -> Result { + let response = self + .send_api( + |client| async move { client.get_run_state().id(run_id.to_string()).send().await }, + ) + .await?; + convert_type(response.into_inner()) + } + + pub async fn list_run_events( + &self, + run_id: &RunId, + since_seq: Option, + limit: Option, + ) -> Result> { + let mut next_since_seq = since_seq; + let mut all_events = Vec::new(); + + loop { + let response = self + .send_api(|client| async move { + let mut request = client.list_run_events().id(run_id.to_string()); + if let Some(seq) = next_since_seq.and_then(non_zero_u64_from_u32) { + request = request.since_seq(seq); + } + if let Some(limit) = limit.and_then(non_zero_u64_from_usize) { + request = request.limit(limit); + } + request.send().await + }) + .await?; + let parsed = response.into_inner(); + let page_events = parsed + .data + .into_iter() + .map(convert_type::<_, EventEnvelope>) + .collect::>>()?; + let next_page_since_seq = page_events.last().map(|event| event.seq.saturating_add(1)); + all_events.extend(page_events); + + if limit.is_some() || !parsed.meta.has_more || next_page_since_seq.is_none() { + break; + } + next_since_seq = next_page_since_seq; + } + + Ok(all_events) + } + + pub async fn attach_run_events( + &self, + run_id: &RunId, + since_seq: Option, + ) -> Result { + let response = self + .send_api(|client| async move { + let mut request = client.attach_run_events().id(run_id.to_string()); + if let Some(seq) = since_seq.and_then(non_zero_u64_from_u32) { + request = request.since_seq(seq); + } + request.send().await + }) + .await?; + Ok(RunEventStream::new(response.into_inner())) + } + + pub async fn list_run_questions(&self, run_id: &RunId) -> Result> { + let response = self + .send_api(|client| async move { + client + .list_run_questions() + .id(run_id.to_string()) + .page_limit(100) + .page_offset(0) + .send() + .await + }) + .await?; + Ok(response.into_inner().data) + } + + pub async fn submit_run_answer( + &self, + run_id: &RunId, + qid: &str, + value: Option, + selected_option_key: Option, + selected_option_keys: Vec, + ) -> Result<()> { + self.send_api(|client| async move { + client + .submit_run_answer() + .id(run_id.to_string()) + .qid(qid) + .body(types::SubmitAnswerRequest { + value: value.clone(), + selected_option_key: selected_option_key.clone(), + selected_option_keys: selected_option_keys.clone(), + }) + .send() + .await + }) + .await?; + Ok(()) + } + + pub async fn append_run_event(&self, run_id: &RunId, event: &RunEvent) -> Result { + let body: types::RunEvent = convert_type(event)?; + let response = self + .send_api(|client| async move { + client + .append_run_event() + .id(run_id.to_string()) + .body(body.clone()) + .send() + .await + }) + .await?; + u32::try_from(response.into_inner().seq).context("append_run_event returned invalid seq") + } + + pub async fn write_run_blob(&self, run_id: &RunId, data: &[u8]) -> Result { + let response = self + .send_api(|client| async move { + client + .write_run_blob() + .id(run_id.to_string()) + .body(data.to_vec()) + .send() + .await + }) + .await?; + response + .into_inner() + .id + .parse() + .context("write_run_blob returned invalid blob id") + } + + pub async fn read_run_blob( + &self, + run_id: &RunId, + blob_id: &RunBlobId, + ) -> Result> { + let response = self + .current_state() + .client + .read_run_blob() + .id(run_id.to_string()) + .blob_id(blob_id.to_string()) + .send() + .await; + match response { + Ok(response) => { + let mut stream = response.into_inner(); + let mut bytes = Vec::new(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|err| anyhow!("{err}"))?; + bytes.extend_from_slice(&chunk); + } + Ok(Some(Bytes::from(bytes))) + } + Err(err) => { + if is_not_found_error(&err) { + Ok(None) + } else { + Err(map_api_error(err)) + } + } + } + } + + pub async fn delete_store_run(&self, run_id: &RunId, force: bool) -> Result<()> { + let base_url = self.base_url(); + let mut url = fabro_http::Url::parse(&base_url) + .with_context(|| format!("invalid server base URL {base_url}"))?; + url.path_segments_mut() + .map_err(|()| anyhow!("server base URL cannot accept path segments"))? + .extend(["api", "v1", "runs", &run_id.to_string()]); + if force { + url.query_pairs_mut().append_pair("force", "true"); + } + + self.send_http(|http_client| async move { http_client.delete(url.clone()).send().await }) + .await?; + Ok(()) + } + + pub async fn list_run_artifacts(&self, run_id: &RunId) -> Result> { + let response = self + .send_api(|client| async move { + client + .list_run_artifacts() + .id(run_id.to_string()) + .send() + .await + }) + .await?; + Ok(response.into_inner().data) + } + + pub async fn download_stage_artifact( + &self, + run_id: &RunId, + stage_id: &StageId, + filename: &str, + ) -> Result> { + let response = self + .send_api(|client| async move { + client + .get_stage_artifact() + .id(run_id.to_string()) + .stage_id(stage_id.to_string()) + .filename(filename) + .send() + .await + }) + .await?; + let mut stream = response.into_inner(); + let mut bytes = Vec::new(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|err| anyhow!("{err}"))?; + bytes.extend_from_slice(&chunk); + } + Ok(bytes) + } + + fn stage_artifacts_url(&self, run_id: &RunId, stage_id: &StageId) -> Result { + let base_url = self.base_url(); + let mut url = fabro_http::Url::parse(&base_url) + .with_context(|| format!("invalid server base URL {base_url}"))?; + url.path_segments_mut() + .map_err(|()| anyhow!("server base URL cannot accept path segments"))? + .extend([ + "api", + "v1", + "runs", + &run_id.to_string(), + "stages", + &stage_id.to_string(), + "artifacts", + ]); + Ok(url) + } + + pub async fn upload_stage_artifact_file( + &self, + run_id: &RunId, + stage_id: &StageId, + filename: &str, + path: &Path, + bearer_token: &str, + ) -> Result<()> { + let mut url = self.stage_artifacts_url(run_id, stage_id)?; + url.query_pairs_mut().append_pair("filename", filename); + + let file = File::open(path) + .await + .with_context(|| format!("failed to open artifact {}", path.display()))?; + let content_length = file + .metadata() + .await + .with_context(|| format!("failed to stat artifact {}", path.display()))? + .len(); + let body = fabro_http::Body::wrap_stream(ReaderStream::new(file)); + + let response = self + .current_state() + .http_client + .post(url) + .bearer_auth(bearer_token) + .header(CONTENT_TYPE, "application/octet-stream") + .header(CONTENT_LENGTH, content_length.to_string()) + .body(body) + .send() + .await + .with_context(|| format!("failed to upload artifact {}", path.display()))?; + classify_http_response(response) + .await? + .map(|_| ()) + .map_err(|failure| raw_response_failure_error(&failure)) + } + + pub async fn upload_stage_artifact_batch( + &self, + run_id: &RunId, + stage_id: &StageId, + artifact_capture_dir: &Path, + artifacts: &[ArtifactUpload], + bearer_token: &str, + ) -> Result<()> { + let url = self.stage_artifacts_url(run_id, stage_id)?; + let mut manifest_entries = Vec::with_capacity(artifacts.len()); + let mut file_parts = Vec::with_capacity(artifacts.len()); + + for (index, artifact) in artifacts.iter().enumerate() { + let part_name = format!("file{}", index + 1); + let path = artifact_capture_dir.join(&artifact.path); + let file = File::open(&path) + .await + .with_context(|| format!("failed to open artifact {}", path.display()))?; + let content_length = file + .metadata() + .await + .with_context(|| format!("failed to stat artifact {}", path.display()))? + .len(); + + manifest_entries.push(ArtifactBatchUploadEntry { + part: part_name.clone(), + path: artifact.path.clone(), + sha256: Some(artifact.content_sha256.clone()), + expected_bytes: Some(artifact.bytes), + content_type: Some(artifact.mime.clone()), + }); + + file_parts.push(( + part_name, + Part::stream_with_length( + fabro_http::Body::wrap_stream(ReaderStream::new(file)), + content_length, + ) + .file_name(artifact.path.clone()), + )); + } + + let manifest = ArtifactBatchUploadManifest { + entries: manifest_entries, + }; + let manifest_part = + Part::text(serde_json::to_string(&manifest)?).mime_str("application/json")?; + let mut form = Form::new().part("manifest", manifest_part); + for (part_name, part) in file_parts { + form = form.part(part_name, part); + } + + let response = self + .current_state() + .http_client + .post(url) + .bearer_auth(bearer_token) + .multipart(form) + .send() + .await + .context("failed to upload artifact batch")?; + classify_http_response(response) + .await? + .map(|_| ()) + .map_err(|failure| raw_response_failure_error(&failure)) + } + + pub async fn generate_preview_url( + &self, + run_id: &RunId, + port: u16, + expires_in_secs: u64, + signed: bool, + ) -> Result { + let expires_in_secs = NonZeroU64::new(expires_in_secs) + .ok_or_else(|| anyhow!("preview expiry must be greater than zero"))?; + let response = self + .send_api(|client| async move { + client + .generate_preview_url() + .id(run_id.to_string()) + .body(types::PreviewUrlRequest { + expires_in_secs, + port: i64::from(port), + signed, + }) + .send() + .await + }) + .await?; + Ok(response.into_inner()) + } + + pub async fn create_run_ssh_access( + &self, + run_id: &RunId, + ttl_minutes: f64, + ) -> Result { + let response = self + .send_api(|client| async move { + client + .create_run_ssh_access() + .id(run_id.to_string()) + .body(types::SshAccessRequest { ttl_minutes }) + .send() + .await + }) + .await?; + Ok(response.into_inner()) + } + + pub async fn list_sandbox_files( + &self, + run_id: &RunId, + path: &str, + depth: Option, + ) -> Result> { + let response = self + .send_api(|client| async move { + let mut request = client + .list_sandbox_files() + .id(run_id.to_string()) + .path(path); + if let Some(depth) = depth.and_then(non_zero_u64_from_u32) { + request = request.depth(depth); + } + request.send().await + }) + .await?; + Ok(response.into_inner().data) + } + + pub async fn get_sandbox_file(&self, run_id: &RunId, path: &str) -> Result> { + let response = self + .send_api(|client| async move { + client + .get_sandbox_file() + .id(run_id.to_string()) + .path(path) + .send() + .await + }) + .await?; + let mut stream = response.into_inner(); + let mut bytes = Vec::new(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|err| anyhow!("{err}"))?; + bytes.extend_from_slice(&chunk); + } + Ok(bytes) + } + + pub async fn put_sandbox_file(&self, run_id: &RunId, path: &str, bytes: Vec) -> Result<()> { + self.send_api(|client| async move { + client + .put_sandbox_file() + .id(run_id.to_string()) + .path(path) + .body(bytes.clone()) + .send() + .await + }) + .await?; + Ok(()) + } +} + +fn client_state( + base_url: String, + http_client: fabro_http::HttpClient, + bearer_token: Option, +) -> ClientState { + let client = fabro_api::ApiClient::new_with_client(&base_url, http_client.clone()); + ClientState { + client, + http_client, + bearer_token, + base_url, + } +} + +fn default_transport_connector(target: ServerTarget) -> TransportConnector { + TransportConnector::new(move |bearer_token| { + let target = target.clone(); + async move { connect_target_transport(&target, bearer_token.as_deref()) } + }) +} + +fn connect_target_transport( + target: &ServerTarget, + bearer_token: Option<&str>, +) -> Result<(fabro_http::HttpClient, String)> { + if let Some(api_url) = target.as_http_url() { + let mut builder = fabro_http::HttpClientBuilder::new(); + builder = match bearer_token { + Some(token) => apply_bearer_token_auth(builder, token)?, + None => builder, + }; + let http_client = builder.build()?; + return Ok((http_client, api_url.to_string())); + } + + let Some(path) = target.as_unix_socket_path() else { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + }; + let mut builder = fabro_http::HttpClientBuilder::new() + .unix_socket(path) + .no_proxy(); + builder = match bearer_token { + Some(token) => apply_bearer_token_auth(builder, token)?, + None => builder, + }; + let http_client = builder.build()?; + Ok((http_client, "http://fabro".to_string())) +} + +fn apply_bearer_token_auth( + builder: fabro_http::HttpClientBuilder, + token: &str, +) -> Result { + let mut headers = fabro_http::HeaderMap::new(); + headers.insert( + AUTHORIZATION, + fabro_http::HeaderValue::from_str(&format!("Bearer {token}")) + .context("invalid bearer token header value")?, + ); + Ok(builder.default_headers(headers)) +} + +fn ensure_refresh_target_transport(target: &ServerTarget) -> Result<()> { + match target.loopback_classification()? { + LoopbackClassification::Https + | LoopbackClassification::LoopbackHttp + | LoopbackClassification::UnixSocket => Ok(()), + LoopbackClassification::Rejected => bail!(refresh_transport_error(target)), + } +} + +fn refresh_transport_error(target: &ServerTarget) -> String { + format!( + "Refusing to send refresh-token credentials over plaintext HTTP to a non-loopback host ({target}). Use HTTPS, or bind the server to 127.0.0.1 / ::1." + ) +} + +fn non_zero_u64_from_u32(value: u32) -> Option { + NonZeroU64::new(u64::from(value)) +} + +fn non_zero_u64_from_usize(value: usize) -> Option { + u64::try_from(value).ok().and_then(NonZeroU64::new) +} + +#[cfg(test)] +mod tests { + use chrono::Duration as ChronoDuration; + + use super::*; + use crate::AuthStore; + + fn oauth_entry(login: &str) -> AuthEntry { + let now = chrono::Utc::now(); + AuthEntry { + access_token: format!("access-{login}"), + access_token_expires_at: now + ChronoDuration::minutes(10), + refresh_token: format!("refresh-{login}"), + refresh_token_expires_at: now + ChronoDuration::days(30), + subject: StoredSubject { + idp_issuer: "https://github.com".to_string(), + idp_subject: "12345".to_string(), + login: login.to_string(), + name: format!("Name {login}"), + email: format!("{login}@example.com"), + }, + logged_in_at: now, + } + } + + #[cfg(unix)] + #[tokio::test] + async fn refresh_access_token_rejects_plain_http_non_loopback_targets() { + let temp = tempfile::tempdir().unwrap(); + let auth_store = AuthStore::new(temp.path().join("auth.json")); + let target = ServerTarget::http_url("http://fabro.example.com").unwrap(); + let entry = oauth_entry("octocat"); + auth_store.put(&target, entry.clone()).unwrap(); + + let client = Client::builder() + .target(target.clone()) + .credential(Credential::OAuth(entry)) + .oauth_session(OAuthSession::new(target.clone(), auth_store.clone())) + .transport( + "http://fabro.example.com", + fabro_http::HttpClientBuilder::new() + .no_proxy() + .build() + .unwrap(), + ) + .connect() + .await + .unwrap(); + + let err = client + .refresh_access_token("access-octocat") + .await + .unwrap_err(); + assert!( + err.to_string() + .contains("Refusing to send refresh-token credentials over plaintext HTTP") + ); + assert!(auth_store.get(&target).unwrap().is_some()); + } +} diff --git a/lib/crates/fabro-client/src/credential.rs b/lib/crates/fabro-client/src/credential.rs new file mode 100644 index 000000000..c0a194069 --- /dev/null +++ b/lib/crates/fabro-client/src/credential.rs @@ -0,0 +1,40 @@ +use std::fmt; + +use crate::AuthEntry; + +#[derive(Clone)] +pub enum Credential { + DevToken(String), + OAuth(AuthEntry), +} + +pub trait CredentialFallback: Send + Sync { + fn resolve(&self) -> Option; +} + +impl CredentialFallback for F +where + F: Fn() -> Option + Send + Sync, +{ + fn resolve(&self) -> Option { + self() + } +} + +impl Credential { + pub fn bearer_token(&self) -> &str { + match self { + Self::DevToken(token) => token, + Self::OAuth(entry) => &entry.access_token, + } + } +} + +impl fmt::Debug for Credential { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::DevToken(_) => f.write_str("Credential::DevToken()"), + Self::OAuth(_) => f.write_str("Credential::OAuth()"), + } + } +} diff --git a/lib/crates/fabro-client/src/error.rs b/lib/crates/fabro-client/src/error.rs new file mode 100644 index 000000000..d41d61fa7 --- /dev/null +++ b/lib/crates/fabro-client/src/error.rs @@ -0,0 +1,184 @@ +use anyhow::{Result, anyhow}; +use serde::de::DeserializeOwned; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ApiFailure { + pub status: fabro_http::StatusCode, + pub code: Option, +} + +pub struct StructuredApiError { + pub error: anyhow::Error, + pub failure: Option, +} + +pub struct ApiError { + pub status: fabro_http::StatusCode, + pub headers: fabro_http::HeaderMap, + pub body: String, + failure: ApiFailure, +} + +impl ApiError { + pub fn api_failure(&self) -> &ApiFailure { + &self.failure + } +} + +pub fn parse_error_response_value(value: &serde_json::Value) -> (Option, Option) { + let first = value + .get("errors") + .and_then(serde_json::Value::as_array) + .and_then(|errors| errors.first()); + let detail = first + .and_then(|entry| entry.get("detail")) + .and_then(serde_json::Value::as_str) + .map(ToOwned::to_owned); + let code = first + .and_then(|entry| entry.get("code")) + .and_then(serde_json::Value::as_str) + .map(ToOwned::to_owned); + (detail, code) +} + +pub async fn classify_api_error(err: progenitor_client::Error) -> StructuredApiError +where + E: serde::Serialize + std::fmt::Debug, +{ + match err { + progenitor_client::Error::UnexpectedResponse(response) => { + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + let mut code = None; + if let Ok(value) = serde_json::from_str::(&body) { + let (detail, parsed_code) = parse_error_response_value(&value); + code = parsed_code; + if let Some(detail) = detail { + return StructuredApiError { + error: anyhow!("{detail}"), + failure: Some(ApiFailure { status, code }), + }; + } + } + let error = if body.is_empty() { + anyhow!("request failed with status {status}") + } else { + anyhow!("request failed with status {status}: {body}") + }; + StructuredApiError { + error, + failure: Some(ApiFailure { status, code }), + } + } + other => map_api_error_structured(other), + } +} + +fn map_api_error_structured(err: progenitor_client::Error) -> StructuredApiError +where + E: serde::Serialize + std::fmt::Debug, +{ + match err { + progenitor_client::Error::ErrorResponse(response) => { + let status = response.status(); + let mut code = None; + if let Ok(value) = serde_json::to_value(response.into_inner()) { + let (detail, parsed_code) = parse_error_response_value(&value); + code = parsed_code; + if let Some(detail) = detail { + return StructuredApiError { + error: anyhow!("{detail}"), + failure: Some(ApiFailure { status, code }), + }; + } + } + StructuredApiError { + error: anyhow!("request failed with status {status}"), + failure: Some(ApiFailure { status, code }), + } + } + progenitor_client::Error::UnexpectedResponse(response) => StructuredApiError { + error: anyhow!("request failed with status {}", response.status()), + failure: Some(ApiFailure { + status: response.status(), + code: None, + }), + }, + other => StructuredApiError { + error: anyhow!("{other}"), + failure: None, + }, + } +} + +pub fn map_api_error(err: progenitor_client::Error) -> anyhow::Error +where + E: serde::Serialize + std::fmt::Debug, +{ + map_api_error_structured(err).error +} + +pub fn raw_response_failure_error(failure: &ApiError) -> anyhow::Error { + if let Ok(value) = serde_json::from_str::(&failure.body) { + let (detail, _) = parse_error_response_value(&value); + if let Some(detail) = detail { + return anyhow!("{detail}"); + } + } + + if failure.body.is_empty() { + return anyhow!("request failed with status {}", failure.status); + } + + anyhow!( + "request failed with status {}: {}", + failure.status, + failure.body + ) +} + +pub async fn classify_http_response( + response: fabro_http::Response, +) -> Result> { + if response.status().is_success() { + return Ok(Ok(response)); + } + let status = response.status(); + let headers = response.headers().clone(); + let body = response.text().await.unwrap_or_default(); + let mut code = None; + if let Ok(value) = serde_json::from_str::(&body) { + let (_, parsed_code) = parse_error_response_value(&value); + code = parsed_code; + } + + Ok(Err(ApiError { + status, + headers, + body, + failure: ApiFailure { status, code }, + })) +} + +pub fn is_not_found_error(err: &progenitor_client::Error) -> bool +where + E: serde::Serialize + std::fmt::Debug, +{ + match err { + progenitor_client::Error::ErrorResponse(response) => { + response.status() == fabro_http::StatusCode::NOT_FOUND + } + progenitor_client::Error::UnexpectedResponse(response) => { + response.status() == fabro_http::StatusCode::NOT_FOUND + } + _ => false, + } +} + +pub fn convert_type(value: TInput) -> Result +where + TInput: serde::Serialize, + TOutput: DeserializeOwned, +{ + serde_json::from_value(serde_json::to_value(value)?).map_err(Into::into) +} diff --git a/lib/crates/fabro-client/src/lib.rs b/lib/crates/fabro-client/src/lib.rs new file mode 100644 index 000000000..248987500 --- /dev/null +++ b/lib/crates/fabro-client/src/lib.rs @@ -0,0 +1,26 @@ +//! Typed HTTP client for the Fabro API. +//! +//! This crate hosts the reusable client and auth/session plumbing that was +//! previously embedded in `fabro-cli`. + +pub mod auth_store; +pub mod client; +pub mod credential; +pub mod error; +pub mod loopback; +pub mod session; +pub mod sse; +pub mod target; + +pub use auth_store::{AuthEntry, AuthStore, AuthStoreError, LockError, StoredSubject}; +pub use client::{Client, RunEventStream, TransportConnector}; +pub use credential::{Credential, CredentialFallback}; +pub use error::{ + ApiError, ApiFailure, StructuredApiError, classify_api_error, classify_http_response, + convert_type, is_not_found_error, map_api_error, parse_error_response_value, + raw_response_failure_error, +}; +pub use fabro_api::types; +pub use loopback::{LoopbackClassification, TargetSchemeError}; +pub use session::OAuthSession; +pub use target::ServerTarget; diff --git a/lib/crates/fabro-cli/src/loopback_target.rs b/lib/crates/fabro-client/src/loopback.rs similarity index 71% rename from lib/crates/fabro-cli/src/loopback_target.rs rename to lib/crates/fabro-client/src/loopback.rs index 96903111b..20a28b900 100644 --- a/lib/crates/fabro-cli/src/loopback_target.rs +++ b/lib/crates/fabro-client/src/loopback.rs @@ -2,10 +2,10 @@ use std::net::IpAddr; use thiserror::Error; -use crate::user_config::{self, ServerTarget}; +use crate::target::ServerTarget; #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum LoopbackClassification { +pub enum LoopbackClassification { Https, LoopbackHttp, UnixSocket, @@ -13,7 +13,7 @@ pub(crate) enum LoopbackClassification { } #[derive(Debug, Error)] -pub(crate) enum TargetSchemeError { +pub enum TargetSchemeError { #[error("invalid server URL `{value}`: {reason}")] InvalidUrl { value: String, reason: String }, #[error("unsupported server URL scheme `{scheme}`")] @@ -22,22 +22,25 @@ pub(crate) enum TargetSchemeError { MissingHost { value: String }, } -pub(crate) fn is_loopback_or_unix_socket( +pub(crate) fn classify_target( target: &ServerTarget, ) -> Result { - match target { - ServerTarget::UnixSocket(_) => Ok(LoopbackClassification::UnixSocket), - ServerTarget::HttpUrl(api_url) => classify_http_target(api_url), + if target.is_unix_socket() { + Ok(LoopbackClassification::UnixSocket) + } else if let Some(api_url) = target.as_http_url() { + classify_http_target(api_url) + } else { + Err(TargetSchemeError::MissingHost { + value: target.to_string(), + }) } } fn classify_http_target(api_url: &str) -> Result { - let normalized = user_config::normalized_http_base_url(api_url); - let url = - fabro_http::Url::parse(normalized).map_err(|source| TargetSchemeError::InvalidUrl { - value: api_url.to_string(), - reason: source.to_string(), - })?; + let url = fabro_http::Url::parse(api_url).map_err(|source| TargetSchemeError::InvalidUrl { + value: api_url.to_string(), + reason: source.to_string(), + })?; match url.scheme() { "https" => Ok(LoopbackClassification::Https), @@ -50,7 +53,7 @@ fn classify_http_target(api_url: &str) -> Result bool { mod tests { use std::path::PathBuf; - use super::{LoopbackClassification, is_loopback_or_unix_socket}; - use crate::user_config::ServerTarget; + use super::LoopbackClassification; + use crate::target::ServerTarget; #[test] fn classifies_https_loopback_and_unix_targets() { let cases = [ ( - ServerTarget::HttpUrl("https://fabro.example.com".to_string()), + ServerTarget::http_url("https://fabro.example.com").unwrap(), LoopbackClassification::Https, ), ( - ServerTarget::HttpUrl("http://127.0.0.1:3000".to_string()), + ServerTarget::http_url("http://127.0.0.1:3000").unwrap(), LoopbackClassification::LoopbackHttp, ), ( - ServerTarget::HttpUrl("http://[::1]:3000".to_string()), + ServerTarget::http_url("http://[::1]:3000").unwrap(), LoopbackClassification::LoopbackHttp, ), ( - ServerTarget::HttpUrl("http://[::ffff:127.0.0.1]:3000".to_string()), + ServerTarget::http_url("http://[::ffff:127.0.0.1]:3000").unwrap(), LoopbackClassification::LoopbackHttp, ), ( - ServerTarget::UnixSocket(PathBuf::from("/tmp/fabro.sock")), + ServerTarget::unix_socket_path("/tmp/fabro.sock").unwrap(), LoopbackClassification::UnixSocket, ), ]; for (target, expected) in cases { - assert_eq!(is_loopback_or_unix_socket(&target).unwrap(), expected); + assert_eq!(target.loopback_classification().unwrap(), expected); } } @@ -167,18 +170,23 @@ mod tests { ]; for api_url in cases { - let target = ServerTarget::HttpUrl(api_url.to_string()); + let target = ServerTarget::http_url(api_url).unwrap(); assert_eq!( - is_loopback_or_unix_socket(&target).unwrap(), + target.loopback_classification().unwrap(), LoopbackClassification::Rejected ); } } #[test] - fn rejects_unsupported_schemes() { - let target = ServerTarget::HttpUrl("ftp://fabro.example.com".to_string()); - let error = is_loopback_or_unix_socket(&target).unwrap_err(); - assert!(error.to_string().contains("unsupported server URL scheme")); + fn rejects_non_http_server_targets_at_parse_time() { + let error = "ftp://fabro.example.com" + .parse::() + .unwrap_err(); + assert!( + error + .to_string() + .contains("server target must be an http(s) URL or absolute Unix socket path") + ); } } diff --git a/lib/crates/fabro-client/src/session.rs b/lib/crates/fabro-client/src/session.rs new file mode 100644 index 000000000..55435b9e8 --- /dev/null +++ b/lib/crates/fabro-client/src/session.rs @@ -0,0 +1,48 @@ +use std::fmt; +use std::sync::Arc; + +use crate::{AuthStore, Credential, CredentialFallback, ServerTarget}; + +#[derive(Clone)] +pub struct OAuthSession { + pub target: ServerTarget, + pub auth_store: AuthStore, + pub fallback: Option>, +} + +impl OAuthSession { + #[must_use] + pub fn new(target: ServerTarget, auth_store: AuthStore) -> Self { + Self { + target, + auth_store, + fallback: None, + } + } + + #[must_use] + pub fn with_fallback(mut self, fallback: Arc) -> Self { + self.fallback = Some(fallback); + self + } + + #[must_use] + pub fn resolve_fallback(&self) -> Option { + self.fallback + .as_ref() + .and_then(|fallback| fallback.resolve()) + } +} + +impl fmt::Debug for OAuthSession { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("OAuthSession") + .field("target", &self.target) + .field("auth_store", &self.auth_store) + .field( + "fallback", + &self.fallback.as_ref().map(|_| ""), + ) + .finish() + } +} diff --git a/lib/crates/fabro-cli/src/sse.rs b/lib/crates/fabro-client/src/sse.rs similarity index 94% rename from lib/crates/fabro-cli/src/sse.rs rename to lib/crates/fabro-client/src/sse.rs index 7208e625d..10b72f2d3 100644 --- a/lib/crates/fabro-cli/src/sse.rs +++ b/lib/crates/fabro-client/src/sse.rs @@ -1,4 +1,4 @@ -pub(crate) fn drain_sse_payloads(buffer: &mut Vec, finalize: bool) -> Vec { +pub fn drain_sse_payloads(buffer: &mut Vec, finalize: bool) -> Vec { let mut payloads = Vec::new(); while let Some(pos) = buffer.iter().position(|byte| *byte == b'\n') { diff --git a/lib/crates/fabro-client/src/target.rs b/lib/crates/fabro-client/src/target.rs new file mode 100644 index 000000000..e710ca732 --- /dev/null +++ b/lib/crates/fabro-client/src/target.rs @@ -0,0 +1,220 @@ +use std::fmt; +use std::path::{Component, Path, PathBuf}; +use std::str::FromStr; + +use anyhow::{Result, bail}; + +use crate::loopback::{LoopbackClassification, TargetSchemeError, classify_target}; + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum ServerTarget { + HttpUrl(CanonicalHttpUrl), + UnixSocket(CanonicalUnixSocketPath), +} + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct CanonicalHttpUrl(String); + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct CanonicalUnixSocketPath(PathBuf); + +impl ServerTarget { + pub fn http_url(value: impl AsRef) -> Result { + Ok(Self::HttpUrl(CanonicalHttpUrl::new(value.as_ref())?)) + } + + pub fn unix_socket_path(path: impl AsRef) -> Result { + Ok(Self::UnixSocket(CanonicalUnixSocketPath::new( + path.as_ref(), + )?)) + } + + #[must_use] + pub fn as_http_url(&self) -> Option<&str> { + match self { + Self::HttpUrl(url) => Some(url.as_str()), + Self::UnixSocket(_) => None, + } + } + + #[must_use] + pub fn as_unix_socket_path(&self) -> Option<&Path> { + match self { + Self::HttpUrl(_) => None, + Self::UnixSocket(path) => Some(path.as_path()), + } + } + + #[must_use] + pub fn is_unix_socket(&self) -> bool { + matches!(self, Self::UnixSocket(_)) + } + + pub fn build_public_http_client(&self) -> Result<(fabro_http::HttpClient, String)> { + if let Some(api_url) = self.as_http_url() { + let http_client = fabro_http::HttpClientBuilder::new().build()?; + return Ok((http_client, api_url.to_string())); + } + + let Some(path) = self.as_unix_socket_path() else { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + }; + + #[cfg(unix)] + { + let http_client = fabro_http::HttpClientBuilder::new() + .unix_socket(path) + .no_proxy() + .build()?; + Ok((http_client, "http://fabro".to_string())) + } + #[cfg(not(unix))] + { + let _ = path; + bail!("Unix-socket HTTP client is not supported on this platform") + } + } + + pub fn loopback_classification(&self) -> Result { + classify_target(self) + } +} + +impl fmt::Display for ServerTarget { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(api_url) = self.as_http_url() { + return f.write_str(api_url); + } + let Some(path) = self.as_unix_socket_path() else { + return Err(fmt::Error); + }; + write!(f, "unix://{}", path.display()) + } +} + +impl FromStr for ServerTarget { + type Err = anyhow::Error; + + fn from_str(value: &str) -> Result { + if value.starts_with("http://") || value.starts_with("https://") { + return Self::http_url(value); + } + + let path = Path::new(value); + if path.is_absolute() { + return Self::unix_socket_path(path); + } + + bail!("server target must be an http(s) URL or absolute Unix socket path") + } +} + +impl CanonicalHttpUrl { + fn new(value: &str) -> Result { + Ok(Self(canonical_http_url(value)?)) + } + + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl CanonicalUnixSocketPath { + fn new(path: &Path) -> Result { + let normalized = lexical_normalize_absolute_path(path)?; + Ok(Self(normalized)) + } + + #[must_use] + pub fn as_path(&self) -> &Path { + &self.0 + } +} + +fn canonical_http_url(value: &str) -> Result { + let trimmed = value.trim(); + let normalized = trim_api_path_suffix(trimmed); + let url = fabro_http::Url::parse(normalized).map_err(|_| { + anyhow::anyhow!("server target must be an http(s) URL or absolute Unix socket path") + })?; + + let scheme = url.scheme().to_ascii_lowercase(); + let default_port = match scheme.as_str() { + "http" => 80, + "https" => 443, + _ => bail!("server target must be an http(s) URL or absolute Unix socket path"), + }; + + let Some(host) = url.host_str() else { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + }; + let host = host.to_ascii_lowercase(); + let Some(port) = url.port_or_known_default() else { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + }; + + if port == default_port { + Ok(format!("{scheme}://{host}")) + } else { + Ok(format!("{scheme}://{host}:{port}")) + } +} + +fn trim_api_path_suffix(value: &str) -> &str { + let trimmed = value.trim_end_matches('/'); + trimmed.strip_suffix("/api/v1").unwrap_or(trimmed) +} + +fn lexical_normalize_absolute_path(path: &Path) -> Result { + if !path.is_absolute() { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + } + + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::Prefix(prefix) => normalized.push(prefix.as_os_str()), + Component::RootDir => normalized.push(component.as_os_str()), + Component::CurDir => {} + Component::ParentDir => { + let _ = normalized.pop(); + } + Component::Normal(part) => normalized.push(part), + } + } + + Ok(normalized) +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::ServerTarget; + + #[test] + fn canonicalizes_http_targets_at_construction() { + let target = ServerTarget::http_url("https://EXAMPLE.COM:443/api/v1/").unwrap(); + + assert_eq!(target.as_http_url(), Some("https://example.com")); + assert_eq!(target.to_string(), "https://example.com"); + } + + #[test] + fn canonicalizes_http_targets_by_rebuilding_authority() { + let target = ServerTarget::http_url("http://Example.com:3000/nested/path").unwrap(); + + assert_eq!(target.as_http_url(), Some("http://example.com:3000")); + } + + #[test] + fn lexically_normalizes_unix_socket_paths_without_fs_access() { + let target = ServerTarget::unix_socket_path("/tmp/fabro/../fabro.sock").unwrap(); + + assert_eq!( + target.as_unix_socket_path(), + Some(PathBuf::from("/tmp/fabro.sock").as_path()) + ); + } +} diff --git a/lib/crates/fabro-retro/src/retro_agent.rs b/lib/crates/fabro-retro/src/retro_agent.rs index 2529c436a..7fef673d4 100644 --- a/lib/crates/fabro-retro/src/retro_agent.rs +++ b/lib/crates/fabro-retro/src/retro_agent.rs @@ -308,7 +308,7 @@ async fn upload_data_files( let progress_content = { let lines: Vec = events .iter() - .filter_map(|env| serde_json::to_string(env.payload.as_value()).ok()) + .filter_map(|env| serde_json::to_string(&env.event).ok()) .collect(); if lines.is_empty() { None diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 81fe6e50f..3a13fd504 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -885,9 +885,7 @@ fn start_optional_slack_service(state: &Arc) { loop { match rx.recv().await { Ok(envelope) => { - if let Ok(event) = RunEvent::try_from(&envelope.payload) { - event_service.handle_event(&event).await; - } + event_service.handle_event(&envelope.event).await; } Err(RecvError::Lagged(_)) => {} Err(RecvError::Closed) => break, @@ -1553,7 +1551,7 @@ struct PrunePlan { reason = "sync helper invoked from async handler via spawn_blocking (see callers at :1301 / :1341)" )] fn build_disk_usage_response( - summaries: &[fabro_store::RunSummary], + summaries: &[fabro_types::RunSummary], storage_dir: &std::path::Path, verbose: bool, ) -> anyhow::Result { @@ -1626,7 +1624,7 @@ fn build_disk_usage_response( fn build_prune_plan( request: &PruneRunsRequest, - summaries: &[fabro_store::RunSummary], + summaries: &[fabro_types::RunSummary], storage_dir: &std::path::Path, ) -> anyhow::Result { let scratch_base_dir = scratch_base(storage_dir); @@ -1772,16 +1770,7 @@ fn event_matches_run_filter(event: &EventEnvelope, run_filter: Option<&HashSet().ok()) - else { - return false; - }; - run_filter.contains(&run_id) + run_filter.contains(&event.event.run_id) } fn sse_event_from_store(event: &EventEnvelope) -> Option { @@ -1791,11 +1780,8 @@ fn sse_event_from_store(event: &EventEnvelope) -> Option { } fn attach_event_is_terminal(event: &EventEnvelope) -> bool { - let Ok(run_event) = RunEvent::try_from(&event.payload) else { - return false; - }; matches!( - run_event.body, + &event.event.body, EventBody::RunCompleted(_) | EventBody::RunFailed(_) ) } @@ -2771,7 +2757,7 @@ fn elapsed_secs(duration_ms: Option) -> Option { duration_ms.map(|ms| ms as f64 / 1000.0) } -fn summary_to_api_run_summary(summary: fabro_store::RunSummary) -> serde_json::Value { +fn summary_to_api_run_summary(summary: fabro_types::RunSummary) -> serde_json::Value { let goal = summary.goal.unwrap_or_default(); let title = truncate_goal(&goal); let repository = repository_name(summary.host_repo_path.as_deref()); @@ -3302,9 +3288,13 @@ fn octet_stream_response(bytes: Bytes) -> Response { reason = "Stored event conversion surfaces HTTP errors directly." )] fn api_event_envelope_from_store(event: &EventEnvelope) -> Result { - // The payload is already a serde_json::Value; merge `seq` into it - // instead of serializing the whole envelope and re-parsing. - let mut obj = event.payload.as_value().clone(); + let mut obj = event.event.to_value().map_err(|err| { + ApiError::new( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Failed to serialize stored event: {err}"), + ) + .into_response() + })?; if let serde_json::Value::Object(ref mut map) = obj { map.insert("seq".into(), serde_json::Value::from(event.seq)); } @@ -3578,11 +3568,9 @@ async fn forward_run_events_to_global( loop { match run_events.recv().await { Ok(event) => { - if let Ok(run_event) = RunEvent::try_from(&event.payload) { - let mut runs = state.runs.lock().expect("runs lock poisoned"); - if let Some(managed_run) = runs.get_mut(&run_id) { - reconcile_live_interview_state_for_event(managed_run, &run_event); - } + let mut runs = state.runs.lock().expect("runs lock poisoned"); + if let Some(managed_run) = runs.get_mut(&run_id) { + reconcile_live_interview_state_for_event(managed_run, &event.event); } let _ = state.global_event_tx.send(event); } @@ -8767,8 +8755,8 @@ slug = "fabro" let run_store = state.store.open_run_reader(&run_id).await.unwrap(); let events = run_store.list_events().await.unwrap(); - let created = events[0].payload.as_value(); - let submitted = events[1].payload.as_value(); + let created = events[0].event.to_value().unwrap(); + let submitted = events[1].event.to_value().unwrap(); let manifest_blob = created["properties"]["manifest_blob"] .as_str() .expect("run.created should carry manifest_blob") diff --git a/lib/crates/fabro-store/src/lib.rs b/lib/crates/fabro-store/src/lib.rs index 55ff6a427..42a2cef9f 100644 --- a/lib/crates/fabro-store/src/lib.rs +++ b/lib/crates/fabro-store/src/lib.rs @@ -9,13 +9,15 @@ mod types; pub use artifact_store::{ArtifactStore, NodeArtifact}; pub use error::{Error, Result}; -pub use fabro_types::{RunBlobId, StageId}; -pub use run_state::{NodeState, PendingInterviewRecord, RunProjection}; +pub use fabro_types::{ + EventEnvelope, NodeState, PendingInterviewRecord, RunBlobId, RunProjection, StageId, +}; +pub use run_state::RunProjectionReducer; pub use slate::{ AuthCode, ConsumeOutcome, Database, RefreshToken, RunDatabase, Runs, SlateAuthCodeStore, SlateAuthTokenStore, }; -pub use types::{EventEnvelope, EventPayload, RunSummary}; +pub use types::EventPayload; #[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct ListRunsQuery { diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index 8ae25dc96..bd8cdbb9d 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -9,56 +9,13 @@ use fabro_types::run_event::{ }; use fabro_types::{ BilledModelUsage, BlockedReason, Checkpoint, Conclusion, EventBody, FailureSignature, - InterviewQuestionRecord, InterviewQuestionType, NodeStatusRecord, Outcome, PullRequestRecord, - Retro, RunControlAction, RunEvent, RunId, RunRecord, RunStatus, RunStatusRecord, SandboxRecord, - StageStatus, StartRecord, StatusReason, + InterviewQuestionRecord, InterviewQuestionType, NodeStatusRecord, Outcome, + PendingInterviewRecord, PullRequestRecord, RunControlAction, RunId, RunProjection, RunRecord, + RunStatus, RunStatusRecord, RunSummary, SandboxRecord, StageStatus, StartRecord, StatusReason, }; use serde_json::Value; -use crate::{Error, EventEnvelope, Result, RunSummary, StageId}; - -#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] -#[serde(default)] -pub struct RunProjection { - pub run: Option, - pub graph_source: Option, - pub start: Option, - pub status: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub prior_status: Option, - pub pending_control: Option, - pub checkpoint: Option, - pub checkpoints: Vec<(u32, Checkpoint)>, - pub conclusion: Option, - pub retro: Option, - pub retro_prompt: Option, - pub retro_response: Option, - pub sandbox: Option, - pub final_patch: Option, - pub pull_request: Option, - pub pending_interviews: BTreeMap, - nodes: HashMap, -} - -#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] -pub struct PendingInterviewRecord { - pub question: InterviewQuestionRecord, - pub started_at: Option>, -} - -#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] -pub struct NodeState { - pub prompt: Option, - pub response: Option, - pub status: Option, - pub provider_used: Option, - pub diff: Option, - pub script_invocation: Option, - pub script_timing: Option, - pub parallel_results: Option, - pub stdout: Option, - pub stderr: Option, -} +use crate::{Error, EventEnvelope, Result}; #[derive(Debug, Clone, Default)] pub(crate) struct EventProjectionCache { @@ -66,8 +23,16 @@ pub(crate) struct EventProjectionCache { pub state: RunProjection, } -impl RunProjection { - pub fn apply_events(events: &[EventEnvelope]) -> Result { +pub trait RunProjectionReducer { + fn apply_events(events: &[EventEnvelope]) -> Result + where + Self: Sized; + + fn apply_event(&mut self, event: &EventEnvelope) -> Result<()>; +} + +impl RunProjectionReducer for RunProjection { + fn apply_events(events: &[EventEnvelope]) -> Result { let mut state = Self::default(); for event in events { state.apply_event(event)?; @@ -75,9 +40,8 @@ impl RunProjection { Ok(state) } - pub fn apply_event(&mut self, event: &EventEnvelope) -> Result<()> { - let stored = RunEvent::from_ref(event.payload.as_value()) - .map_err(|err| Error::InvalidEvent(format!("invalid stored event: {err}")))?; + fn apply_event(&mut self, event: &EventEnvelope) -> Result<()> { + let stored = &event.event; let ts = stored.ts; let run_id = stored.run_id; @@ -420,107 +384,57 @@ impl RunProjection { Ok(()) } +} - pub fn node(&self, node: &StageId) -> Option<&NodeState> { - self.nodes.get(node) - } - - pub fn iter_nodes(&self) -> impl Iterator { - self.nodes.iter() - } - - pub fn is_empty(&self) -> bool { - self.nodes.is_empty() - } - - pub fn set_node(&mut self, node: StageId, state: NodeState) { - self.nodes.insert(node, state); - } - - pub fn list_node_visits(&self, node_id: &str) -> Vec { - let mut visits = self - .nodes - .keys() - .filter(|node| node.node_id() == node_id) - .map(StageId::visit) - .collect::>(); - visits.sort_unstable(); - visits.dedup(); - visits - } - - pub(crate) fn build_summary(&self, run_id: &RunId) -> RunSummary { - let workflow_name = self.run.as_ref().map(|run| { - if run.graph.name.is_empty() { - "unnamed".to_string() - } else { - run.graph.name.clone() - } - }); - let goal = self.run.as_ref().and_then(|run| { - let goal = run.graph.goal(); - (!goal.is_empty()).then(|| goal.to_string()) - }); - RunSummary { - run_id: *run_id, - workflow_name, - workflow_slug: self.run.as_ref().and_then(|run| run.workflow_slug.clone()), - goal, - labels: self - .run - .as_ref() - .map(|run| run.labels.clone()) - .unwrap_or_default(), - host_repo_path: self.run.as_ref().and_then(|run| run.host_repo_path.clone()), - start_time: self.start.as_ref().map(|start| start.start_time), - status: self - .status - .as_ref() - .map_or(RunStatus::Submitted, |status| status.status), - status_reason: self.status.as_ref().and_then(|status| status.status_reason), - blocked_reason: self - .status - .as_ref() - .and_then(|status| status.blocked_reason), - pending_control: self.pending_control, - duration_ms: self - .conclusion - .as_ref() - .map(|conclusion| conclusion.duration_ms), - total_usd_micros: self - .conclusion - .as_ref() - .and_then(|conclusion| conclusion.billing.as_ref()) - .and_then(|billing| billing.total_usd_micros), +pub(crate) fn build_summary(state: &RunProjection, run_id: &RunId) -> RunSummary { + let workflow_name = state.run.as_ref().map(|run| { + if run.graph.name.is_empty() { + "unnamed".to_string() + } else { + run.graph.name.clone() } - } - - fn node_mut(&mut self, node_id: &str, visit: u32) -> &mut NodeState { - self.nodes.entry(StageId::new(node_id, visit)).or_default() - } - - fn current_visit_for(&self, node_id: &str) -> Option { - self.nodes - .keys() - .filter(|node| node.node_id() == node_id) - .map(StageId::visit) - .max() - } - - fn reset_for_rewind(&mut self) { - self.status = None; - self.pending_control = None; - self.checkpoint = None; - self.checkpoints.clear(); - self.conclusion = None; - self.retro = None; - self.retro_prompt = None; - self.retro_response = None; - self.sandbox = None; - self.final_patch = None; - self.pull_request = None; - self.pending_interviews.clear(); - self.nodes.clear(); + }); + let goal = state.run.as_ref().and_then(|run| { + let goal = run.graph.goal(); + (!goal.is_empty()).then(|| goal.to_string()) + }); + RunSummary { + run_id: *run_id, + workflow_name, + workflow_slug: state.run.as_ref().and_then(|run| run.workflow_slug.clone()), + goal, + labels: state + .run + .as_ref() + .map(|run| run.labels.clone()) + .unwrap_or_default(), + host_repo_path: state + .run + .as_ref() + .and_then(|run| run.host_repo_path.clone()), + start_time: state.start.as_ref().map(|start| start.start_time), + status: state + .status + .as_ref() + .map_or(RunStatus::Submitted, |status| status.status), + status_reason: state + .status + .as_ref() + .and_then(|status| status.status_reason), + blocked_reason: state + .status + .as_ref() + .and_then(|status| status.blocked_reason), + pending_control: state.pending_control, + duration_ms: state + .conclusion + .as_ref() + .map(|conclusion| conclusion.duration_ms), + total_usd_micros: state + .conclusion + .as_ref() + .and_then(|conclusion| conclusion.billing.as_ref()) + .and_then(|billing| billing.total_usd_micros), } } @@ -700,13 +614,13 @@ mod tests { }; use fabro_types::settings::SettingsLayer; use fabro_types::{ - Checkpoint, EventBody, InterviewQuestionType, RunBlobId, RunControlAction, RunEvent, - fixtures, + Checkpoint, EventBody, InterviewQuestionType, NodeState, RunBlobId, RunControlAction, + RunEvent, fixtures, }; use serde_json::json; - use super::{NodeState, RunProjection}; - use crate::{EventEnvelope, EventPayload, StageId}; + use super::{RunProjection, RunProjectionReducer, build_summary}; + use crate::{EventEnvelope, StageId}; fn test_event(seq: u32, body: EventBody, node_id: Option<&str>) -> EventEnvelope { let event = RunEvent { @@ -725,11 +639,7 @@ mod tests { body, }; - EventEnvelope { - seq, - payload: EventPayload::new(serde_json::to_value(event).unwrap(), &fixtures::RUN_1) - .unwrap(), - } + EventEnvelope { seq, event } } fn test_raw_event( @@ -740,17 +650,14 @@ mod tests { ) -> EventEnvelope { EventEnvelope { seq, - payload: EventPayload::new( - json!({ - "id": format!("evt-{seq}"), - "ts": Utc::now().to_rfc3339(), - "run_id": fixtures::RUN_1, - "event": event, - "node_id": node_id, - "properties": properties, - }), - &fixtures::RUN_1, - ) + event: RunEvent::from_value(json!({ + "id": format!("evt-{seq}"), + "ts": Utc::now().to_rfc3339(), + "run_id": fixtures::RUN_1, + "event": event, + "node_id": node_id, + "properties": properties, + })) .unwrap(), } } @@ -813,23 +720,21 @@ mod tests { #[test] fn set_node_round_trips_through_json() { - let mut state = RunProjection { - pending_control: Some(RunControlAction::Unpause), - checkpoints: vec![(7, Checkpoint { - timestamp: "2026-04-07T12:00:00Z".parse().unwrap(), - current_node: "build".to_string(), - completed_nodes: vec!["build".to_string()], - node_retries: HashMap::new(), - context_values: HashMap::new(), - node_outcomes: HashMap::new(), - next_node_id: None, - git_commit_sha: None, - loop_failure_signatures: HashMap::new(), - restart_failure_signatures: HashMap::new(), - node_visits: HashMap::from([("build".to_string(), 2usize)]), - })], - ..RunProjection::default() - }; + let mut state = RunProjection::default(); + state.pending_control = Some(RunControlAction::Unpause); + state.checkpoints = vec![(7, Checkpoint { + timestamp: "2026-04-07T12:00:00Z".parse().unwrap(), + current_node: "build".to_string(), + completed_nodes: vec!["build".to_string()], + node_retries: HashMap::new(), + context_values: HashMap::new(), + node_outcomes: HashMap::new(), + next_node_id: None, + git_commit_sha: None, + loop_failure_signatures: HashMap::new(), + restart_failure_signatures: HashMap::new(), + node_visits: HashMap::from([("build".to_string(), 2usize)]), + })]; state.set_node(StageId::new("build", 2), NodeState { stdout: Some("done".to_string()), ..NodeState::default() @@ -955,7 +860,7 @@ mod tests { assert_eq!(status_json["status_reason"], serde_json::Value::Null); assert_eq!(status_json["blocked_reason"], "human_input_required"); - let summary = state.build_summary(&fixtures::RUN_1); + let summary = build_summary(&state, &fixtures::RUN_1); let summary_json = serde_json::to_value(summary).unwrap(); assert_eq!(summary_json["status"], "paused"); assert_eq!(summary_json["status_reason"], serde_json::Value::Null); @@ -1053,25 +958,23 @@ mod tests { #[test] fn summary_synthesizes_submitted_when_run_exists_without_status() { - let state = RunProjection { - run: Some(fabro_types::RunRecord { - run_id: fixtures::RUN_1, - settings: SettingsLayer::default(), - graph: fabro_types::Graph::new("test"), - workflow_slug: Some("test".to_string()), - working_directory: std::path::PathBuf::from("/tmp/run"), - host_repo_path: Some("/tmp/repo".to_string()), - repo_origin_url: None, - base_branch: None, - labels: HashMap::new(), - provenance: None, - manifest_blob: None, - definition_blob: None, - }), - ..RunProjection::default() - }; + let mut state = RunProjection::default(); + state.run = Some(fabro_types::RunRecord { + run_id: fixtures::RUN_1, + settings: SettingsLayer::default(), + graph: fabro_types::Graph::new("test"), + workflow_slug: Some("test".to_string()), + working_directory: std::path::PathBuf::from("/tmp/run"), + host_repo_path: Some("/tmp/repo".to_string()), + repo_origin_url: None, + base_branch: None, + labels: HashMap::new(), + provenance: None, + manifest_blob: None, + definition_blob: None, + }); - let summary_json = serde_json::to_value(state.build_summary(&fixtures::RUN_1)).unwrap(); + let summary_json = serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap(); assert_eq!(summary_json["status"], "submitted"); } @@ -1082,45 +985,39 @@ mod tests { RunBlobId::new(br#"{"version":1,"workflow_path":"workflow.fabro"}"#).to_string(); let events = vec![ EventEnvelope { - seq: 1, - payload: EventPayload::new( - json!({ - "id": "evt-run-created", - "ts": "2026-04-07T12:00:00Z", - "run_id": fixtures::RUN_1, - "event": "run.created", - "properties": { - "settings": SettingsLayer::default(), - "graph": { - "name": "test", - "nodes": {}, - "edges": [], - "attrs": {} - }, - "labels": {}, - "run_dir": "/tmp/run", - "working_directory": "/tmp/run", - "manifest_blob": manifest_blob - } - }), - &fixtures::RUN_1, - ) + seq: 1, + event: RunEvent::from_value(json!({ + "id": "evt-run-created", + "ts": "2026-04-07T12:00:00Z", + "run_id": fixtures::RUN_1, + "event": "run.created", + "properties": { + "settings": SettingsLayer::default(), + "graph": { + "name": "test", + "nodes": {}, + "edges": [], + "attrs": {} + }, + "labels": {}, + "run_dir": "/tmp/run", + "working_directory": "/tmp/run", + "manifest_blob": manifest_blob + } + })) .unwrap(), }, EventEnvelope { - seq: 2, - payload: EventPayload::new( - json!({ - "id": "evt-run-submitted", - "ts": "2026-04-07T12:00:01Z", - "run_id": fixtures::RUN_1, - "event": "run.submitted", - "properties": { - "definition_blob": definition_blob - } - }), - &fixtures::RUN_1, - ) + seq: 2, + event: RunEvent::from_value(json!({ + "id": "evt-run-submitted", + "ts": "2026-04-07T12:00:01Z", + "run_id": fixtures::RUN_1, + "event": "run.submitted", + "properties": { + "definition_blob": definition_blob + } + })) .unwrap(), }, ]; @@ -1130,11 +1027,11 @@ mod tests { assert_eq!( value["run"]["manifest_blob"], - events[0].payload.as_value()["properties"]["manifest_blob"] + events[0].event.properties().unwrap()["manifest_blob"] ); assert_eq!( value["run"]["definition_blob"], - events[1].payload.as_value()["properties"]["definition_blob"] + events[1].event.properties().unwrap()["definition_blob"] ); } diff --git a/lib/crates/fabro-store/src/slate/mod.rs b/lib/crates/fabro-store/src/slate/mod.rs index e368970b0..d5e02d668 100644 --- a/lib/crates/fabro-store/src/slate/mod.rs +++ b/lib/crates/fabro-store/src/slate/mod.rs @@ -10,14 +10,15 @@ use std::time::Duration; pub use auth_codes::{AuthCode, SlateAuthCodeStore}; pub use auth_tokens::{ConsumeOutcome, RefreshToken, SlateAuthTokenStore}; -use fabro_types::RunId; +use fabro_types::{RunId, RunSummary}; use object_store::ObjectStore; pub use run_store::RunDatabase; use run_store::RunDatabaseInner; use slatedb::config::{CompressionCodec, Settings}; use tokio::sync::{Mutex, OnceCell}; -use crate::{Error, ListRunsQuery, Result, RunSummary, keys}; +use crate::run_state::build_summary; +use crate::{Error, ListRunsQuery, Result, keys}; #[derive(Clone)] pub struct Database { @@ -170,7 +171,7 @@ impl Database { let mut summaries = Vec::new(); for run_id in run_ids { if let Some(active) = self.get_active_run(&run_id).await { - summaries.push(active.state().await?.build_summary(&run_id)); + summaries.push(build_summary(&active.state().await?, &run_id)); continue; } if !RunDatabase::has_any_events(&db, &run_id).await? { @@ -245,7 +246,7 @@ impl Runs { pub async fn find(&self, run_id: &RunId) -> Result> { match self.db.open_run_reader(run_id).await { - Ok(run_db) => Ok(Some(run_db.state().await?.build_summary(run_id))), + Ok(run_db) => Ok(Some(build_summary(&run_db.state().await?, run_id))), Err(Error::RunNotFound(_)) => Ok(None), Err(err) => Err(err), } diff --git a/lib/crates/fabro-store/src/slate/run_store.rs b/lib/crates/fabro-store/src/slate/run_store.rs index 388e5a188..16f21e3e2 100644 --- a/lib/crates/fabro-store/src/slate/run_store.rs +++ b/lib/crates/fabro-store/src/slate/run_store.rs @@ -4,14 +4,14 @@ use std::sync::atomic::{AtomicU32, Ordering}; use bytes::Bytes; use chrono::Utc; -use fabro_types::{RunBlobId, RunId}; +use fabro_types::{RunBlobId, RunEvent, RunId, RunSummary}; use futures::Stream; use slatedb::{Db, DbRead}; use tokio::sync::{Mutex, broadcast, mpsc}; use tokio_stream::wrappers::UnboundedReceiverStream; -use crate::run_state::EventProjectionCache; -use crate::{Error, EventEnvelope, EventPayload, Result, RunProjection, RunSummary, keys}; +use crate::run_state::{EventProjectionCache, RunProjectionReducer, build_summary}; +use crate::{Error, EventEnvelope, EventPayload, Result, RunProjection, keys}; const DEFAULT_EVENT_TAIL_LIMIT: usize = 1024; #[derive(Clone)] @@ -131,7 +131,7 @@ impl RunDatabase { { let events = list_events_from(db, run_id, 1).await?; let state = RunProjection::apply_events(&events)?; - Ok(state.build_summary(run_id)) + Ok(build_summary(&state, run_id)) } async fn projected_state(&self) -> Result { @@ -193,7 +193,7 @@ impl RunDatabase { let seq = self.inner.event_seq.fetch_add(1, Ordering::SeqCst); let event = EventEnvelope { seq, - payload: payload.clone(), + event: RunEvent::try_from(payload)?, }; self.inner .db @@ -341,7 +341,7 @@ where } events.push(EventEnvelope { seq, - payload: serde_json::from_slice(&entry.value)?, + event: serde_json::from_slice(&entry.value)?, }); } events.sort_by_key(|event| event.seq); diff --git a/lib/crates/fabro-store/src/types.rs b/lib/crates/fabro-store/src/types.rs index 6023d49d6..65235a55b 100644 --- a/lib/crates/fabro-store/src/types.rs +++ b/lib/crates/fabro-store/src/types.rs @@ -1,28 +1,8 @@ -use std::collections::HashMap; - -use chrono::{DateTime, Utc}; -use fabro_types::{BlockedReason, RunControlAction, RunEvent, RunId, RunStatus, StatusReason}; +use fabro_types::{RunEvent, RunId}; use serde::{Deserialize, Serialize}; use crate::{Error, Result}; -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct RunSummary { - pub run_id: RunId, - pub workflow_name: Option, - pub workflow_slug: Option, - pub goal: Option, - pub labels: HashMap, - pub host_repo_path: Option, - pub start_time: Option>, - pub status: RunStatus, - pub status_reason: Option, - pub blocked_reason: Option, - pub pending_control: Option, - pub duration_ms: Option, - pub total_usd_micros: Option, -} - #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(transparent)] pub struct EventPayload(serde_json::Value); @@ -81,111 +61,3 @@ impl TryFrom<&EventPayload> for RunEvent { .map_err(|err| Error::InvalidEvent(format!("invalid stored event: {err}"))) } } - -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct EventEnvelope { - pub seq: u32, - #[serde(flatten)] - pub payload: EventPayload, -} - -#[cfg(test)] -mod tests { - use chrono::{TimeZone, Utc}; - use fabro_types::run_event::RunCompletedProps; - use fabro_types::{ActorRef, EventBody, ParallelBranchId, RunEvent, StageId, fixtures}; - - use super::{EventEnvelope, EventPayload}; - - #[test] - fn wire_event_envelope_round_trips() { - let event = RunEvent { - id: "evt_1".to_string(), - ts: Utc.with_ymd_and_hms(2026, 4, 9, 12, 0, 0).unwrap(), - run_id: fixtures::RUN_1, - node_id: Some("code".to_string()), - node_label: Some("Code".to_string()), - stage_id: Some(StageId::new("code", 1)), - parallel_group_id: None, - parallel_branch_id: None, - session_id: None, - parent_session_id: None, - tool_call_id: None, - actor: None, - body: EventBody::RunCompleted(RunCompletedProps { - duration_ms: 42, - artifact_count: 0, - status: "success".to_string(), - reason: None, - total_usd_micros: None, - final_git_commit_sha: None, - final_patch: None, - billing: None, - }), - }; - let payload = EventPayload::new(event.to_value().unwrap(), &fixtures::RUN_1).unwrap(); - let envelope = EventEnvelope { seq: 7, payload }; - - let wire = serde_json::to_value(&envelope).unwrap(); - assert_eq!(wire["seq"], 7); - assert_eq!(wire["id"], "evt_1"); - assert_eq!(wire["event"], "run.completed"); - assert!(wire.get("payload").is_none(), "wire shape must be flat"); - - let parsed: EventEnvelope = serde_json::from_value(wire).unwrap(); - assert_eq!(parsed, envelope); - } - - #[test] - fn wire_event_envelope_round_trips_with_all_envelope_fields() { - let group = StageId::new("review", 2); - let branch = ParallelBranchId::new(group.clone(), 3); - let event = RunEvent { - id: "evt_2".to_string(), - ts: Utc.with_ymd_and_hms(2026, 4, 9, 13, 0, 0).unwrap(), - run_id: fixtures::RUN_1, - node_id: Some("review".to_string()), - node_label: Some("Review".to_string()), - stage_id: Some(StageId::new("review", 2)), - parallel_group_id: Some(group), - parallel_branch_id: Some(branch), - session_id: Some("ses_42".to_string()), - parent_session_id: Some("ses_root".to_string()), - tool_call_id: Some("tool_call_xyz".to_string()), - actor: Some(ActorRef::agent( - Some("ses_42".to_string()), - Some("claude-sonnet".to_string()), - )), - body: EventBody::RunCompleted(RunCompletedProps { - duration_ms: 100, - artifact_count: 1, - status: "success".to_string(), - reason: None, - total_usd_micros: None, - final_git_commit_sha: None, - final_patch: None, - billing: None, - }), - }; - let payload = EventPayload::new(event.to_value().unwrap(), &fixtures::RUN_1).unwrap(); - let envelope = EventEnvelope { seq: 99, payload }; - - let wire = serde_json::to_value(&envelope).unwrap(); - assert_eq!(wire["seq"], 99); - assert_eq!(wire["id"], "evt_2"); - assert_eq!(wire["stage_id"], "review@2"); - assert_eq!(wire["parallel_group_id"], "review@2"); - assert_eq!(wire["parallel_branch_id"], "review@2:3"); - assert_eq!(wire["session_id"], "ses_42"); - assert_eq!(wire["parent_session_id"], "ses_root"); - assert_eq!(wire["tool_call_id"], "tool_call_xyz"); - assert_eq!(wire["actor"]["kind"], "agent"); - assert_eq!(wire["actor"]["id"], "ses_42"); - assert_eq!(wire["actor"]["display"], "claude-sonnet"); - assert_eq!(wire["event"], "run.completed"); - assert!(wire.get("payload").is_none(), "wire shape must be flat"); - - let parsed: EventEnvelope = serde_json::from_value(wire).unwrap(); - assert_eq!(parsed, envelope); - } -} diff --git a/lib/crates/fabro-types/src/artifact.rs b/lib/crates/fabro-types/src/artifact.rs new file mode 100644 index 000000000..0cd6af42a --- /dev/null +++ b/lib/crates/fabro-types/src/artifact.rs @@ -0,0 +1,30 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ArtifactUpload { + pub path: String, + pub mime: String, + pub content_md5: String, + pub content_sha256: String, + pub bytes: u64, +} + +#[cfg(test)] +mod tests { + use super::ArtifactUpload; + + #[test] + fn round_trips_through_serde_json() { + let artifact = ArtifactUpload { + path: "artifacts/log.txt".to_string(), + mime: "text/plain".to_string(), + content_md5: "md5".to_string(), + content_sha256: "sha256".to_string(), + bytes: 42, + }; + + let value = serde_json::to_value(&artifact).unwrap(); + let parsed: ArtifactUpload = serde_json::from_value(value).unwrap(); + assert_eq!(parsed, artifact); + } +} diff --git a/lib/crates/fabro-types/src/event_envelope.rs b/lib/crates/fabro-types/src/event_envelope.rs new file mode 100644 index 000000000..16de63da5 --- /dev/null +++ b/lib/crates/fabro-types/src/event_envelope.rs @@ -0,0 +1,135 @@ +use serde::{Deserialize, Serialize}; + +use crate::RunEvent; + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct EventEnvelope { + pub seq: u32, + #[serde(flatten)] + pub event: RunEvent, +} + +#[cfg(test)] +mod tests { + use chrono::{TimeZone, Utc}; + + use super::EventEnvelope; + use crate::run_event::RunCompletedProps; + use crate::{ActorRef, EventBody, ParallelBranchId, RunEvent, StageId, fixtures}; + + #[test] + fn wire_event_envelope_round_trips() { + let event = RunEvent { + id: "evt_1".to_string(), + ts: Utc.with_ymd_and_hms(2026, 4, 9, 12, 0, 0).unwrap(), + run_id: fixtures::RUN_1, + node_id: Some("code".to_string()), + node_label: Some("Code".to_string()), + stage_id: Some(StageId::new("code", 1)), + parallel_group_id: None, + parallel_branch_id: None, + session_id: None, + parent_session_id: None, + tool_call_id: None, + actor: None, + body: EventBody::RunCompleted(RunCompletedProps { + duration_ms: 42, + artifact_count: 0, + status: "success".to_string(), + reason: None, + total_usd_micros: None, + final_git_commit_sha: None, + final_patch: None, + billing: None, + }), + }; + let envelope = EventEnvelope { seq: 7, event }; + + let wire = serde_json::to_value(&envelope).unwrap(); + assert_eq!(wire["seq"], 7); + assert_eq!(wire["id"], "evt_1"); + assert_eq!(wire["event"], "run.completed"); + + let parsed: EventEnvelope = serde_json::from_value(wire).unwrap(); + assert_eq!(parsed, envelope); + } + + #[test] + fn wire_event_envelope_round_trips_with_all_envelope_fields() { + let group = StageId::new("review", 2); + let branch = ParallelBranchId::new(group.clone(), 3); + let event = RunEvent { + id: "evt_2".to_string(), + ts: Utc.with_ymd_and_hms(2026, 4, 9, 13, 0, 0).unwrap(), + run_id: fixtures::RUN_1, + node_id: Some("review".to_string()), + node_label: Some("Review".to_string()), + stage_id: Some(StageId::new("review", 2)), + parallel_group_id: Some(group), + parallel_branch_id: Some(branch), + session_id: Some("ses_42".to_string()), + parent_session_id: Some("ses_root".to_string()), + tool_call_id: Some("tool_call_xyz".to_string()), + actor: Some(ActorRef::agent( + Some("ses_42".to_string()), + Some("claude-sonnet".to_string()), + )), + body: EventBody::RunCompleted(RunCompletedProps { + duration_ms: 100, + artifact_count: 1, + status: "success".to_string(), + reason: None, + total_usd_micros: None, + final_git_commit_sha: None, + final_patch: None, + billing: None, + }), + }; + let envelope = EventEnvelope { seq: 99, event }; + + let wire = serde_json::to_value(&envelope).unwrap(); + assert_eq!(wire["seq"], 99); + assert_eq!(wire["id"], "evt_2"); + assert_eq!(wire["stage_id"], "review@2"); + assert_eq!(wire["parallel_group_id"], "review@2"); + assert_eq!(wire["parallel_branch_id"], "review@2:3"); + assert_eq!(wire["session_id"], "ses_42"); + assert_eq!(wire["parent_session_id"], "ses_root"); + assert_eq!(wire["tool_call_id"], "tool_call_xyz"); + assert_eq!(wire["actor"]["kind"], "agent"); + assert_eq!(wire["actor"]["id"], "ses_42"); + assert_eq!(wire["actor"]["display"], "claude-sonnet"); + assert_eq!(wire["event"], "run.completed"); + + let parsed: EventEnvelope = serde_json::from_value(wire).unwrap(); + assert_eq!(parsed, envelope); + } + + #[test] + fn preserves_unknown_event_names_and_properties() { + let wire = serde_json::json!({ + "seq": 7, + "id": "evt_unknown", + "ts": "2026-04-20T12:00:00.000Z", + "run_id": fixtures::RUN_1, + "event": "vendor.custom.event", + "properties": { + "answer": 42, + "nested": { "ok": true } + } + }); + + let parsed: EventEnvelope = serde_json::from_value(wire.clone()).unwrap(); + let serialized = serde_json::to_value(&parsed).unwrap(); + + assert_eq!(serialized["seq"], wire["seq"]); + assert_eq!(serialized["id"], wire["id"]); + assert_eq!(serialized["run_id"], wire["run_id"]); + assert_eq!(serialized["event"], wire["event"]); + assert_eq!(serialized["properties"], wire["properties"]); + assert_eq!( + chrono::DateTime::parse_from_rfc3339(serialized["ts"].as_str().unwrap()).unwrap(), + chrono::DateTime::parse_from_rfc3339(wire["ts"].as_str().unwrap()).unwrap(), + ); + } +} diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 6ff85e5b1..99d4ae9c4 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -1,10 +1,12 @@ extern crate self as fabro_types; +pub mod artifact; pub mod auth; pub mod billing; pub mod blob_ref; pub mod checkpoint; pub mod conclusion; +pub mod event_envelope; pub mod failure_signature; pub mod graph; pub mod interview; @@ -16,12 +18,15 @@ pub mod run; pub mod run_blob_id; pub mod run_event; pub mod run_id; +pub mod run_projection; +pub mod run_summary; pub mod sandbox_record; pub mod settings; pub mod stage_id; pub mod start; pub mod status; +pub use artifact::ArtifactUpload; pub use auth::{IdpIdentity, IdpIdentityError}; pub use billing::{ AnthropicBillingFacts, AnthropicModelPricing, BilledModelUsage, BilledTokenCounts, @@ -34,6 +39,7 @@ pub use blob_ref::{ }; pub use checkpoint::Checkpoint; pub use conclusion::{Conclusion, StageSummary}; +pub use event_envelope::EventEnvelope; pub use failure_signature::FailureSignature; pub use graph::{AttrValue, Edge, Graph, Node, is_llm_handler_type, shape_to_handler_type}; pub use interview::{InterviewQuestionRecord, InterviewQuestionType}; @@ -51,6 +57,8 @@ pub use run::{ pub use run_blob_id::RunBlobId; pub use run_event::{ActorKind, ActorRef, EventBody, RunEvent, RunNoticeLevel}; pub use run_id::{RunId, fixtures}; +pub use run_projection::{NodeState, PendingInterviewRecord, RunProjection}; +pub use run_summary::RunSummary; pub use sandbox_record::SandboxRecord; pub use stage_id::{ParallelBranchId, StageId}; pub use start::StartRecord; diff --git a/lib/crates/fabro-types/src/run_projection.rs b/lib/crates/fabro-types/src/run_projection.rs new file mode 100644 index 000000000..4fcebdc10 --- /dev/null +++ b/lib/crates/fabro-types/src/run_projection.rs @@ -0,0 +1,109 @@ +use std::collections::{BTreeMap, HashMap}; + +use chrono::{DateTime, Utc}; + +use crate::{ + Checkpoint, Conclusion, InterviewQuestionRecord, NodeStatusRecord, PullRequestRecord, Retro, + RunControlAction, RunRecord, RunStatus, RunStatusRecord, SandboxRecord, StageId, StartRecord, +}; + +#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] +#[serde(default)] +pub struct RunProjection { + pub run: Option, + pub graph_source: Option, + pub start: Option, + pub status: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub prior_status: Option, + pub pending_control: Option, + pub checkpoint: Option, + pub checkpoints: Vec<(u32, Checkpoint)>, + pub conclusion: Option, + pub retro: Option, + pub retro_prompt: Option, + pub retro_response: Option, + pub sandbox: Option, + pub final_patch: Option, + pub pull_request: Option, + pub pending_interviews: BTreeMap, + nodes: HashMap, +} + +#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] +pub struct PendingInterviewRecord { + pub question: InterviewQuestionRecord, + pub started_at: Option>, +} + +#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] +pub struct NodeState { + pub prompt: Option, + pub response: Option, + pub status: Option, + pub provider_used: Option, + pub diff: Option, + pub script_invocation: Option, + pub script_timing: Option, + pub parallel_results: Option, + pub stdout: Option, + pub stderr: Option, +} + +impl RunProjection { + pub fn node(&self, node: &StageId) -> Option<&NodeState> { + self.nodes.get(node) + } + + pub fn iter_nodes(&self) -> impl Iterator { + self.nodes.iter() + } + + pub fn is_empty(&self) -> bool { + self.nodes.is_empty() + } + + pub fn set_node(&mut self, node: StageId, state: NodeState) { + self.nodes.insert(node, state); + } + + pub fn list_node_visits(&self, node_id: &str) -> Vec { + let mut visits = self + .nodes + .keys() + .filter(|node| node.node_id() == node_id) + .map(StageId::visit) + .collect::>(); + visits.sort_unstable(); + visits.dedup(); + visits + } + + pub fn node_mut(&mut self, node_id: &str, visit: u32) -> &mut NodeState { + self.nodes.entry(StageId::new(node_id, visit)).or_default() + } + + pub fn current_visit_for(&self, node_id: &str) -> Option { + self.nodes + .keys() + .filter(|node| node.node_id() == node_id) + .map(StageId::visit) + .max() + } + + pub fn reset_for_rewind(&mut self) { + self.status = None; + self.pending_control = None; + self.checkpoint = None; + self.checkpoints.clear(); + self.conclusion = None; + self.retro = None; + self.retro_prompt = None; + self.retro_response = None; + self.sandbox = None; + self.final_patch = None; + self.pull_request = None; + self.pending_interviews.clear(); + self.nodes.clear(); + } +} diff --git a/lib/crates/fabro-types/src/run_summary.rs b/lib/crates/fabro-types/src/run_summary.rs new file mode 100644 index 000000000..0e025ae6b --- /dev/null +++ b/lib/crates/fabro-types/src/run_summary.rs @@ -0,0 +1,56 @@ +use std::collections::HashMap; + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +use crate::{BlockedReason, RunControlAction, RunId, RunStatus, StatusReason}; + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct RunSummary { + pub run_id: RunId, + pub workflow_name: Option, + pub workflow_slug: Option, + pub goal: Option, + pub labels: HashMap, + pub host_repo_path: Option, + pub start_time: Option>, + pub status: RunStatus, + pub status_reason: Option, + pub blocked_reason: Option, + pub pending_control: Option, + pub duration_ms: Option, + pub total_usd_micros: Option, +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use chrono::{TimeZone, Utc}; + + use super::RunSummary; + use crate::{BlockedReason, RunControlAction, RunStatus, StatusReason, fixtures}; + + #[test] + fn round_trips_through_serde_json() { + let summary = RunSummary { + run_id: fixtures::RUN_1, + workflow_name: Some("workflow".to_string()), + workflow_slug: Some("workflow".to_string()), + goal: Some("ship it".to_string()), + labels: HashMap::from([("team".to_string(), "core".to_string())]), + host_repo_path: Some("/tmp/repo".to_string()), + start_time: Some(Utc.with_ymd_and_hms(2026, 4, 20, 12, 0, 0).unwrap()), + status: RunStatus::Blocked, + status_reason: Some(StatusReason::SandboxInitializing), + blocked_reason: Some(BlockedReason::HumanInputRequired), + pending_control: Some(RunControlAction::Pause), + duration_ms: Some(42), + total_usd_micros: Some(123), + }; + + let value = serde_json::to_value(&summary).unwrap(); + let parsed: RunSummary = serde_json::from_value(value).unwrap(); + assert_eq!(parsed, summary); + } +} diff --git a/lib/crates/fabro-workflow/src/artifact_snapshot.rs b/lib/crates/fabro-workflow/src/artifact_snapshot.rs index 78f8ac5df..ce9abeb47 100644 --- a/lib/crates/fabro-workflow/src/artifact_snapshot.rs +++ b/lib/crates/fabro-workflow/src/artifact_snapshot.rs @@ -2,7 +2,7 @@ use std::path::Path; use fabro_agent::Sandbox; use fabro_sandbox::shell_quote; -use serde::{Deserialize, Serialize}; +use fabro_types::ArtifactUpload; use sha2::{Digest, Sha256}; use tokio::fs; use tracing::{debug, warn}; @@ -15,25 +15,15 @@ pub struct DiscoveredFile { pub mtime_epoch_secs: f64, } -/// Metadata for a single captured artifact file. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct CapturedArtifactInfo { - pub path: String, - pub mime: String, - pub content_md5: String, - pub content_sha256: String, - pub bytes: u64, -} - /// Summary of an artifact collection run. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct ArtifactCollectionSummary { pub files_copied: usize, pub total_bytes: u64, pub files_skipped: usize, pub download_errors: usize, pub hash_errors: usize, - pub captured_assets: Vec, + pub captured_assets: Vec, } /// Directories to exclude from the find search and checkpoint commits. @@ -261,7 +251,7 @@ fn normalize_paths(discovered: Vec, root: &str) -> Vec std::result::Result { +) -> std::result::Result { let mime = mime_guess::from_path(relative_path) .first_or_octet_stream() .to_string(); @@ -271,7 +261,7 @@ async fn compute_artifact_info( let bytes = u64::try_from(data.len()).unwrap_or(u64::MAX); let content_md5 = format!("{:x}", md5::compute(&data)); let content_sha256 = hex::encode(Sha256::digest(&data)); - Ok(CapturedArtifactInfo { + Ok(ArtifactUpload { path: relative_path.to_string(), mime, content_md5, @@ -308,7 +298,7 @@ pub async fn collect_artifacts( let mut total_bytes: u64 = 0; let mut download_errors: usize = 0; let mut hash_errors: usize = 0; - let mut captured_assets: Vec = Vec::new(); + let mut captured_assets: Vec = Vec::new(); for file in &to_collect { let dest = artifact_capture_dir.join(&file.relative_path); diff --git a/lib/crates/fabro-workflow/src/artifact_upload.rs b/lib/crates/fabro-workflow/src/artifact_upload.rs index e6f28d59e..2fb9d3c9a 100644 --- a/lib/crates/fabro-workflow/src/artifact_upload.rs +++ b/lib/crates/fabro-workflow/src/artifact_upload.rs @@ -4,9 +4,7 @@ use std::sync::Arc; use anyhow::Result; use async_trait::async_trait; use fabro_store::ArtifactStore; -use fabro_types::StageId; - -use crate::artifact_snapshot::CapturedArtifactInfo; +use fabro_types::{ArtifactUpload, StageId}; #[async_trait] pub trait StageArtifactUploader: Send + Sync { @@ -14,7 +12,7 @@ pub trait StageArtifactUploader: Send + Sync { &self, stage_id: &StageId, artifact_capture_dir: &Path, - artifacts: &[CapturedArtifactInfo], + artifacts: &[ArtifactUpload], ) -> Result<()>; } diff --git a/lib/crates/fabro-workflow/src/event.rs b/lib/crates/fabro-workflow/src/event.rs index 31116566c..947ffd593 100644 --- a/lib/crates/fabro-workflow/src/event.rs +++ b/lib/crates/fabro-workflow/src/event.rs @@ -3166,7 +3166,7 @@ mod tests { let line = events .into_iter() .next() - .map(|event| event.payload.as_value().clone()) + .map(|event| event.event.to_value().unwrap()) .unwrap(); assert!(line.get("id").is_some()); assert_eq!(line["event"], "run.notice"); diff --git a/lib/crates/fabro-workflow/src/lib.rs b/lib/crates/fabro-workflow/src/lib.rs index ceada6f1a..1d05277b3 100644 --- a/lib/crates/fabro-workflow/src/lib.rs +++ b/lib/crates/fabro-workflow/src/lib.rs @@ -92,19 +92,19 @@ pub fn build_completed_stages(cp: &records::Checkpoint, run_failed: bool) -> Vec pub fn extract_stage_durations_from_events(events: &[EventEnvelope]) -> HashMap { let mut durations = HashMap::new(); for envelope in events { - let value = envelope.payload.as_value(); - let event_name = value.get("event").and_then(serde_json::Value::as_str); - if event_name != Some("stage.completed") && event_name != Some("stage.failed") { + let event = &envelope.event; + let event_name = event.event_name(); + if event_name != "stage.completed" && event_name != "stage.failed" { continue; } - let Some(node_id) = value.get("node_id").and_then(serde_json::Value::as_str) else { + let Some(node_id) = event.node_id.as_deref() else { continue; }; - let Some(duration_ms) = value - .get("properties") - .and_then(serde_json::Value::as_object) - .and_then(|properties| properties.get("duration_ms")) - .and_then(serde_json::Value::as_u64) + let Some(duration_ms) = event + .properties() + .ok() + .and_then(|properties| properties.get("duration_ms").cloned()) + .and_then(|duration| duration.as_u64()) else { continue; }; diff --git a/lib/crates/fabro-workflow/src/lifecycle/artifact.rs b/lib/crates/fabro-workflow/src/lifecycle/artifact.rs index 8b5e70037..88e35ab6a 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/artifact.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/artifact.rs @@ -9,12 +9,12 @@ use fabro_core::lifecycle::{AttemptContext, AttemptResultContext, NodeDecision, use fabro_core::outcome::NodeResult; use fabro_core::state::ExecutionState; use fabro_store::ArtifactStore; -use fabro_types::{RunId, StageId}; +use fabro_types::{ArtifactUpload, RunId, StageId}; use tokio::fs; use tokio::time::sleep; use crate::artifact::{normalize_durable_updates, offload_large_values, sync_artifacts_to_env}; -use crate::artifact_snapshot::{CapturedArtifactInfo, collect_artifacts}; +use crate::artifact_snapshot::collect_artifacts; use crate::artifact_upload::ArtifactSink; use crate::event::{Emitter, Event, RunNoticeLevel}; use crate::graph::{WorkflowGraph, WorkflowNode}; @@ -209,7 +209,7 @@ impl ArtifactLifecycle { &self, stage_id: &StageId, artifact_capture_dir: &std::path::Path, - artifacts: &[CapturedArtifactInfo], + artifacts: &[ArtifactUpload], ) -> Result<(), String> { let Some(sink) = self.artifact_sink.as_ref() else { return Err("artifact sink is not configured".to_string()); @@ -238,7 +238,7 @@ impl ArtifactLifecycle { sink: &ArtifactSink, stage_id: &StageId, artifact_capture_dir: &std::path::Path, - artifacts: &[CapturedArtifactInfo], + artifacts: &[ArtifactUpload], ) -> Result<(), String> { match sink { ArtifactSink::Store(store) => { @@ -257,7 +257,7 @@ impl ArtifactLifecycle { store: &ArtifactStore, stage_id: &StageId, artifact_capture_dir: &std::path::Path, - artifacts: &[CapturedArtifactInfo], + artifacts: &[ArtifactUpload], ) -> Result<(), String> { for artifact in artifacts { let local_path = artifact_capture_dir.join(&artifact.path); diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index a00695024..0ea901856 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -1022,10 +1022,7 @@ mod tests { let run_store = store.open_run_reader(&created.run_id).await.unwrap(); let events = run_store.list_events().await.unwrap(); - assert_eq!( - events.first().unwrap().payload.as_value()["event"], - "run.created" - ); + assert_eq!(events.first().unwrap().event.event_name(), "run.created"); } #[tokio::test] diff --git a/lib/crates/fabro-workflow/src/run_lookup.rs b/lib/crates/fabro-workflow/src/run_lookup.rs index 86bc556b6..8d9ae49e5 100644 --- a/lib/crates/fabro-workflow/src/run_lookup.rs +++ b/lib/crates/fabro-workflow/src/run_lookup.rs @@ -11,8 +11,8 @@ use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; use fabro_config::Storage; use fabro_config::user::default_storage_dir; -use fabro_store::{Database, RunSummary}; -use fabro_types::RunId; +use fabro_store::Database; +use fabro_types::{RunId, RunSummary}; use serde::Serialize; use crate::operations::make_run_dir; From eb8ea317ec69062991e69a35ef1dca4c3ae2eec6 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 20 Apr 2026 21:44:09 -0400 Subject: [PATCH 3/5] refactor(client): dedupe helpers and fix TOCTOU in auth store - Expose apply_bearer_token_auth and ensure_refresh_target_transport from fabro-client; drop the CLI's duplicate copies. - Collapse AuthStore's two read paths into one NotFound-tolerant reader and drop the pre-existence checks in get/remove/list. - Avoid rewriting auth.json when remove found nothing. - Inline the one-line user_config::build_public_http_client wrapper. - Trim unused pub use fabro_api::types re-export and the narrating doc comment in fabro-client/src/lib.rs. - Clean up pre-existing unused imports in run/create.rs and loopback.rs tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../fabro-cli/src/commands/auth/login.rs | 21 ++------- .../fabro-cli/src/commands/auth/logout.rs | 2 +- .../fabro-cli/src/commands/run/create.rs | 2 +- lib/crates/fabro-cli/src/server_client.rs | 15 +----- lib/crates/fabro-cli/src/user_config.rs | 6 --- lib/crates/fabro-client/src/auth_store.rs | 46 +++++++------------ lib/crates/fabro-client/src/client.rs | 4 +- lib/crates/fabro-client/src/lib.rs | 9 ++-- lib/crates/fabro-client/src/loopback.rs | 2 - 9 files changed, 30 insertions(+), 77 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/auth/login.rs b/lib/crates/fabro-cli/src/commands/auth/login.rs index 227ff8936..c7c85f42d 100644 --- a/lib/crates/fabro-cli/src/commands/auth/login.rs +++ b/lib/crates/fabro-cli/src/commands/auth/login.rs @@ -3,7 +3,7 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use chrono::{DateTime, Utc}; use fabro_api::types; -use fabro_client::{AuthEntry, AuthStore, StoredSubject}; +use fabro_client::{AuthEntry, AuthStore, StoredSubject, ensure_refresh_target_transport}; use fabro_http::header::CONTENT_TYPE; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::CliLayer; @@ -96,14 +96,7 @@ pub(super) async fn login_command( } }; - match target.loopback_classification()? { - fabro_client::LoopbackClassification::Https - | fabro_client::LoopbackClassification::LoopbackHttp - | fabro_client::LoopbackClassification::UnixSocket => {} - fabro_client::LoopbackClassification::Rejected => { - bail!("{}", token_transport_error(&target)); - } - } + ensure_refresh_target_transport(&target)?; let tokens = exchange_cli_token(&target, &code, &pkce.verifier, &redirect_uri).await?; let entry = AuthEntry { @@ -129,7 +122,7 @@ pub(super) async fn login_command( #[cfg(unix)] async fn fetch_cli_auth_config(target: &ServerTarget) -> Result { - let (http_client, base_url) = user_config::build_public_http_client(target)?; + let (http_client, base_url) = target.build_public_http_client()?; let client = fabro_api::ApiClient::new_with_client(&base_url, http_client); client .get_cli_auth_config() @@ -146,7 +139,7 @@ async fn exchange_cli_token( code_verifier: &str, redirect_uri: &str, ) -> Result { - let (http_client, base_url) = user_config::build_public_http_client(target)?; + let (http_client, base_url) = target.build_public_http_client()?; let response = http_client .post(format!("{base_url}/auth/cli/token")) .header(CONTENT_TYPE, "application/json") @@ -241,12 +234,6 @@ fn login_failure_message(error_code: &str, error_description: Option<&str>) -> S } } -fn token_transport_error(target: &ServerTarget) -> String { - format!( - "Refusing to send refresh-token credentials over plaintext HTTP to a non-loopback host ({target}). Use HTTPS, or bind the server to 127.0.0.1 / ::1." - ) -} - fn identity_summary(subject: &StoredSubject) -> String { if !subject.name.is_empty() && !subject.email.is_empty() { format!("{} ({} <{}>)", subject.login, subject.name, subject.email) diff --git a/lib/crates/fabro-cli/src/commands/auth/logout.rs b/lib/crates/fabro-cli/src/commands/auth/logout.rs index 4743eb42b..4b72ef5c1 100644 --- a/lib/crates/fabro-cli/src/commands/auth/logout.rs +++ b/lib/crates/fabro-cli/src/commands/auth/logout.rs @@ -58,7 +58,7 @@ pub(super) async fn logout_command( } async fn revoke_remote_session(target: &ServerTarget, entry: &AuthEntry) -> Result<()> { - let (http_client, base_url) = user_config::build_public_http_client(target)?; + let (http_client, base_url) = target.build_public_http_client()?; let response = http_client .post(format!("{base_url}/auth/cli/logout")) .header(AUTHORIZATION, format!("Bearer {}", entry.refresh_token)) diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index 6ee251680..e1c93496c 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -13,7 +13,7 @@ use super::overrides::run_args_layer; use crate::args::RunArgs; use crate::command_context::CommandContext; use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, run_manifest_args}; -use crate::user_config::{self, ServerTarget}; +use crate::user_config; pub(crate) struct CreatedRun { pub(crate) run_id: RunId, diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 4f12f2c0c..337a34530 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -5,10 +5,10 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use fabro_client::{ AuthStore, Credential, CredentialFallback, OAuthSession, ServerTarget, TransportConnector, + apply_bearer_token_auth, }; pub(crate) use fabro_client::{Client, RunEventStream}; use fabro_config::Storage; -use fabro_http::header::AUTHORIZATION; use fabro_server::bind::Bind; pub(crate) use fabro_types::RunProjection; use fabro_types::settings::SettingsLayer; @@ -264,19 +264,6 @@ async fn wait_for_local_dev_token(storage_dir: &Path) -> Result { ); } -fn apply_bearer_token_auth( - builder: fabro_http::HttpClientBuilder, - token: &str, -) -> Result { - let mut headers = fabro_http::HeaderMap::new(); - headers.insert( - AUTHORIZATION, - fabro_http::HeaderValue::from_str(&format!("Bearer {token}")) - .context("invalid dev token header value")?, - ); - Ok(builder.default_headers(headers)) -} - async fn build_authed_unix_socket_http_client( path: &Path, storage_dir: Option<&Path>, diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index c151caa4e..3fa3710da 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -61,12 +61,6 @@ pub(crate) fn apply_storage_dir_override( layer } -pub(crate) fn build_public_http_client( - target: &ServerTarget, -) -> Result<(fabro_http::HttpClient, String)> { - target.build_public_http_client() -} - /// Pull the resolved CLI target configuration out of `[cli.target]`. /// Returns either an http(s) URL or a unix socket path. fn cli_target_from_settings(settings: &CliSettings) -> Option { diff --git a/lib/crates/fabro-client/src/auth_store.rs b/lib/crates/fabro-client/src/auth_store.rs index 01185ac72..e436e3e78 100644 --- a/lib/crates/fabro-client/src/auth_store.rs +++ b/lib/crates/fabro-client/src/auth_store.rs @@ -120,10 +120,6 @@ impl AuthStore { } pub fn get(&self, target: &ServerTarget) -> Result, AuthStoreError> { - if !self.path.exists() { - return Ok(None); - } - let key = key_for_target(target); self.with_shared_lock(|| { let file = self.read_auth_file()?; @@ -143,7 +139,7 @@ impl AuthStore { let key = key_for_target(target); self.ensure_parent_dir()?; self.with_exclusive_lock(|| { - let mut file = self.read_auth_file_if_exists()?; + let mut file = self.read_auth_file()?; file.servers.insert(key, entry); self.write_auth_file(&file) }) @@ -159,26 +155,20 @@ impl AuthStore { #[cfg(unix)] { - if !self.path.exists() { - return Ok(false); - } - let key = key_for_target(target); self.ensure_parent_dir()?; self.with_exclusive_lock(|| { - let mut file = self.read_auth_file_if_exists()?; + let mut file = self.read_auth_file()?; let removed = file.servers.remove(&key).is_some(); - self.write_auth_file(&file)?; + if removed { + self.write_auth_file(&file)?; + } Ok(removed) }) } } pub fn list(&self) -> Result, AuthStoreError> { - if !self.path.exists() { - return Ok(Vec::new()); - } - self.with_shared_lock(|| { let file = self.read_auth_file()?; file.servers @@ -189,21 +179,19 @@ impl AuthStore { } fn read_auth_file(&self) -> Result { - let contents = fs::read_to_string(&self.path).map_err(|source| AuthStoreError::Read { - path: self.path.clone(), - source, - })?; - serde_json::from_str(&contents).map_err(|source| AuthStoreError::Corrupt { - path: self.path.clone(), - source, - }) - } - - fn read_auth_file_if_exists(&self) -> Result { - if !self.path.exists() { - return Ok(AuthFile::default()); + match fs::read_to_string(&self.path) { + Ok(contents) => { + serde_json::from_str(&contents).map_err(|source| AuthStoreError::Corrupt { + path: self.path.clone(), + source, + }) + } + Err(source) if source.kind() == std::io::ErrorKind::NotFound => Ok(AuthFile::default()), + Err(source) => Err(AuthStoreError::Read { + path: self.path.clone(), + source, + }), } - self.read_auth_file() } #[cfg(unix)] diff --git a/lib/crates/fabro-client/src/client.rs b/lib/crates/fabro-client/src/client.rs index 57098e055..3cc165c28 100644 --- a/lib/crates/fabro-client/src/client.rs +++ b/lib/crates/fabro-client/src/client.rs @@ -1294,7 +1294,7 @@ fn connect_target_transport( Ok((http_client, "http://fabro".to_string())) } -fn apply_bearer_token_auth( +pub fn apply_bearer_token_auth( builder: fabro_http::HttpClientBuilder, token: &str, ) -> Result { @@ -1307,7 +1307,7 @@ fn apply_bearer_token_auth( Ok(builder.default_headers(headers)) } -fn ensure_refresh_target_transport(target: &ServerTarget) -> Result<()> { +pub fn ensure_refresh_target_transport(target: &ServerTarget) -> Result<()> { match target.loopback_classification()? { LoopbackClassification::Https | LoopbackClassification::LoopbackHttp diff --git a/lib/crates/fabro-client/src/lib.rs b/lib/crates/fabro-client/src/lib.rs index 248987500..e8a78d380 100644 --- a/lib/crates/fabro-client/src/lib.rs +++ b/lib/crates/fabro-client/src/lib.rs @@ -1,7 +1,4 @@ //! Typed HTTP client for the Fabro API. -//! -//! This crate hosts the reusable client and auth/session plumbing that was -//! previously embedded in `fabro-cli`. pub mod auth_store; pub mod client; @@ -13,14 +10,16 @@ pub mod sse; pub mod target; pub use auth_store::{AuthEntry, AuthStore, AuthStoreError, LockError, StoredSubject}; -pub use client::{Client, RunEventStream, TransportConnector}; +pub use client::{ + Client, RunEventStream, TransportConnector, apply_bearer_token_auth, + ensure_refresh_target_transport, +}; pub use credential::{Credential, CredentialFallback}; pub use error::{ ApiError, ApiFailure, StructuredApiError, classify_api_error, classify_http_response, convert_type, is_not_found_error, map_api_error, parse_error_response_value, raw_response_failure_error, }; -pub use fabro_api::types; pub use loopback::{LoopbackClassification, TargetSchemeError}; pub use session::OAuthSession; pub use target::ServerTarget; diff --git a/lib/crates/fabro-client/src/loopback.rs b/lib/crates/fabro-client/src/loopback.rs index 20a28b900..f980c9f43 100644 --- a/lib/crates/fabro-client/src/loopback.rs +++ b/lib/crates/fabro-client/src/loopback.rs @@ -122,8 +122,6 @@ fn ip_is_loopback(ip: &IpAddr) -> bool { #[cfg(test)] mod tests { - use std::path::PathBuf; - use super::LoopbackClassification; use crate::target::ServerTarget; From 1214bebd1410e6867d4257adc26c5243a74969f8 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 20 Apr 2026 21:49:09 -0400 Subject: [PATCH 4/5] fix(client): reject obfuscated IPv4 literals at target parse time url::Url normalizes decimal (http://2130706433) and hex (http://0x7f000001) IPv4 host forms into 127.0.0.1, so after canonical_http_url rewrites the target the loopback classifier cannot tell them apart from a legitimate http://127.0.0.1 and lets a refresh token ride plaintext HTTP. Detect these forms on the raw input string and bail out before the url crate can hide them, and split the loopback test to cover the parse-time rejection path. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/crates/fabro-client/src/loopback.rs | 12 ++++++++-- lib/crates/fabro-client/src/target.rs | 30 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/lib/crates/fabro-client/src/loopback.rs b/lib/crates/fabro-client/src/loopback.rs index f980c9f43..bda6f0b25 100644 --- a/lib/crates/fabro-client/src/loopback.rs +++ b/lib/crates/fabro-client/src/loopback.rs @@ -163,8 +163,6 @@ mod tests { "http://127.0.0.1:1@attacker.com", "http://localhost", "http://localhost.evil.com", - "http://2130706433", - "http://0x7f000001", ]; for api_url in cases { @@ -176,6 +174,16 @@ mod tests { } } + #[test] + fn rejects_obfuscated_ipv4_literals_at_parse_time() { + for api_url in ["http://2130706433", "http://0x7f000001"] { + assert!( + ServerTarget::http_url(api_url).is_err(), + "{api_url} should not parse as a server target" + ); + } + } + #[test] fn rejects_non_http_server_targets_at_parse_time() { let error = "ftp://fabro.example.com" diff --git a/lib/crates/fabro-client/src/target.rs b/lib/crates/fabro-client/src/target.rs index e710ca732..4ad663ac5 100644 --- a/lib/crates/fabro-client/src/target.rs +++ b/lib/crates/fabro-client/src/target.rs @@ -146,6 +146,10 @@ fn canonical_http_url(value: &str) -> Result { _ => bail!("server target must be an http(s) URL or absolute Unix socket path"), }; + if raw_url_host(normalized).is_some_and(is_obfuscated_ipv4_literal) { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + } + let Some(host) = url.host_str() else { bail!("server target must be an http(s) URL or absolute Unix socket path"); }; @@ -166,6 +170,32 @@ fn trim_api_path_suffix(value: &str) -> &str { trimmed.strip_suffix("/api/v1").unwrap_or(trimmed) } +/// Extract the host substring from `value` without going through +/// [`fabro_http::Url`]. `Url` normalizes decimal/hex IPv4 literals into dotted +/// form, which hides the original input from later inspection. +fn raw_url_host(value: &str) -> Option<&str> { + let (_, remainder) = value.split_once("://")?; + let authority_end = remainder.find(['/', '?', '#']).unwrap_or(remainder.len()); + let authority = &remainder[..authority_end]; + let after_userinfo = authority + .rsplit_once('@') + .map_or(authority, |(_, host)| host); + if after_userinfo.starts_with('[') { + return None; + } + let host = after_userinfo + .split_once(':') + .map_or(after_userinfo, |(host, _)| host); + (!host.is_empty()).then_some(host) +} + +fn is_obfuscated_ipv4_literal(host: &str) -> bool { + if host.starts_with("0x") || host.starts_with("0X") { + return true; + } + !host.contains('.') && !host.is_empty() && host.bytes().all(|b| b.is_ascii_digit()) +} + fn lexical_normalize_absolute_path(path: &Path) -> Result { if !path.is_absolute() { bail!("server target must be an http(s) URL or absolute Unix socket path"); From 125a73aae3ec0dfb389285ccb24eaaa585292918 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 20 Apr 2026 22:18:44 -0400 Subject: [PATCH 5/5] fix(client): reject all obfuscated IPv4 host forms at parse time Replace the narrow decimal/hex obfuscation check with a general comparison: if the parsed host is an IPv4 literal and the raw input host differs from the canonical dotted-quad form, the user supplied an obfuscated variant (octal, short-form, mixed radix, leading zeros, decimal integer, hex integer) that url::Url has already normalized to 127.0.0.1. All such variants are rejected. Test now covers decimal, hex, octal, two-/three-part short, mixed hex/ decimal, and leading-zero octets. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/crates/fabro-client/src/loopback.rs | 12 +++++++++++- lib/crates/fabro-client/src/target.rs | 23 ++++++++++------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/crates/fabro-client/src/loopback.rs b/lib/crates/fabro-client/src/loopback.rs index bda6f0b25..cea27093f 100644 --- a/lib/crates/fabro-client/src/loopback.rs +++ b/lib/crates/fabro-client/src/loopback.rs @@ -176,7 +176,17 @@ mod tests { #[test] fn rejects_obfuscated_ipv4_literals_at_parse_time() { - for api_url in ["http://2130706433", "http://0x7f000001"] { + let cases = [ + "http://2130706433", // decimal integer + "http://0x7f000001", // hex integer + "http://0177.0.0.1", // octal dotted + "http://127.1", // two-part short + "http://127.0.1", // three-part short + "http://0x7f.0.0.1", // mixed hex/decimal + "http://127.00.0.1", // leading-zero octet + "http://127.0.0.001", // leading-zero octet + ]; + for api_url in cases { assert!( ServerTarget::http_url(api_url).is_err(), "{api_url} should not parse as a server target" diff --git a/lib/crates/fabro-client/src/target.rs b/lib/crates/fabro-client/src/target.rs index 4ad663ac5..b590bcb0a 100644 --- a/lib/crates/fabro-client/src/target.rs +++ b/lib/crates/fabro-client/src/target.rs @@ -146,13 +146,16 @@ fn canonical_http_url(value: &str) -> Result { _ => bail!("server target must be an http(s) URL or absolute Unix socket path"), }; - if raw_url_host(normalized).is_some_and(is_obfuscated_ipv4_literal) { - bail!("server target must be an http(s) URL or absolute Unix socket path"); - } - let Some(host) = url.host_str() else { bail!("server target must be an http(s) URL or absolute Unix socket path"); }; + + if host.parse::().is_ok() + && raw_url_host(normalized).is_some_and(|raw| raw != host) + { + bail!("server target must be an http(s) URL or absolute Unix socket path"); + } + let host = host.to_ascii_lowercase(); let Some(port) = url.port_or_known_default() else { bail!("server target must be an http(s) URL or absolute Unix socket path"); @@ -171,8 +174,9 @@ fn trim_api_path_suffix(value: &str) -> &str { } /// Extract the host substring from `value` without going through -/// [`fabro_http::Url`]. `Url` normalizes decimal/hex IPv4 literals into dotted -/// form, which hides the original input from later inspection. +/// [`fabro_http::Url`]. `Url` normalizes IPv4 literals (decimal/hex/octal/short +/// form) into dotted quads, which hides the original input from later +/// inspection. fn raw_url_host(value: &str) -> Option<&str> { let (_, remainder) = value.split_once("://")?; let authority_end = remainder.find(['/', '?', '#']).unwrap_or(remainder.len()); @@ -189,13 +193,6 @@ fn raw_url_host(value: &str) -> Option<&str> { (!host.is_empty()).then_some(host) } -fn is_obfuscated_ipv4_literal(host: &str) -> bool { - if host.starts_with("0x") || host.starts_with("0X") { - return true; - } - !host.contains('.') && !host.is_empty() && host.bytes().all(|b| b.is_ascii_digit()) -} - fn lexical_normalize_absolute_path(path: &Path) -> Result { if !path.is_absolute() { bail!("server target must be an http(s) URL or absolute Unix socket path");