Merge pull request #168 from fkukuck/fix/auto-pr-resolved-client

fix(workflow): reuse resolved llm client for auto-pr
This commit is contained in:
Bryan Helmkamp 2026-04-24 11:10:25 -04:00 committed by GitHub
commit ac4f306cc6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
60 changed files with 3079 additions and 1555 deletions

5
Cargo.lock generated
View file

@ -1512,6 +1512,7 @@ dependencies = [
"chrono",
"clap",
"dirs",
"fabro-auth",
"fabro-config",
"fabro-http",
"fabro-llm",
@ -1522,6 +1523,7 @@ dependencies = [
"fabro-test",
"fabro-types",
"fabro-util",
"fabro-vault",
"futures",
"glob",
"htmd",
@ -1570,6 +1572,7 @@ dependencies = [
"fabro-http",
"fabro-model",
"fabro-oauth",
"fabro-util",
"fabro-vault",
"httpmock",
"serde",
@ -1805,6 +1808,7 @@ version = "0.213.0-nightly.0"
dependencies = [
"async-trait",
"fabro-agent",
"fabro-auth",
"fabro-config",
"fabro-http",
"fabro-llm",
@ -2305,6 +2309,7 @@ dependencies = [
"futures",
"git2",
"hex",
"httpmock",
"md5",
"mime_guess",
"object_store",

View file

@ -0,0 +1,32 @@
# LLM Client Resolution
This document defines how Fabro resolves LLM credentials and constructs `fabro-llm` clients.
## Core Rules
- `fabro_auth::CredentialSource` is the credential authority.
- Long-lived runtime contexts store `Arc<dyn CredentialSource>`, not `Client`.
- Call `fabro_llm::client::Client::from_source(&source).await?` at the point of use.
- `GenerateParams::new(model, client)` always receives an explicit `Arc<Client>`.
- When a caller needs diagnostics, call `source.resolve()` directly and consume both `credentials` and `auth_issues`.
- `EnvCredentialSource` is the env-backed source for env-only or no-vault contexts.
- `VaultCredentialSource` is the normal source for vault-backed runtime contexts.
## Why
- Rebuilding a client from the source at point of use preserves OAuth refresh behavior on long-running processes.
- Holding the source on contexts avoids process-global installs and cross-context leakage.
- Requiring an explicit client on `GenerateParams` makes the old silent fallback bug unrepresentable.
## Application
- Workflow state lives on `RunServices.llm_source`.
- Server state lives on `AppState.llm_source`.
- Hooks and other long-lived executors receive a source and derive clients when they actually generate.
- One-shot CLI commands may resolve a source locally, then derive a client once for that operation.
## Enforcement
- Do not add new `Client::from_env`-style shortcuts in production paths.
- Do not cache a long-lived `Client` where OAuth refresh or storage-dir rebinding matters.
- Mirror [server-secrets-strategy.md](/Users/bhelmkamp/p/fabro-sh/fabro-6/docs-internal/server-secrets-strategy.md): production credential resolution should be explicit about where secrets come from and how they flow into subprocesses.

View file

@ -0,0 +1,569 @@
---
title: "refactor: Source-based LLM client resolution + RunServices split"
type: refactor
status: completed
date: 2026-04-23
deepened: 2026-04-23
---
# refactor: Source-based LLM client resolution + RunServices split
## Overview
PR #168 revealed that `fabro-llm::generate::generate()` silently falls back to `Client::from_env()` when no explicit client is passed. Vault-configured credentials (`openai_codex`) get bypassed. PR #168 is a point fix that threads `Option<Client>` through the pipeline. This plan replaces that with the correct architecture.
**Design in one line:** credentials are the long-lived authority; clients are short-lived derivations built at the point of use.
**Greenfield context.** No production installs, no backwards-compat, no migration. The bar is elegance.
**Two changes:**
1. Replace the silent env fallback with `CredentialSource` as the credential authority and `Client::from_source(&source)` as the idiomatic constructor. Every `generate()` requires an explicit `Arc<Client>` (compile-time enforcement). Delete `DEFAULT_CLIENT`, `set_default_client`, `get_default_client`, and `Client::from_env`.
2. Split `EngineServices` into cross-phase `RunServices` (holds `llm_source: Arc<dyn CredentialSource>`) + execute-only `EngineServices` (holds `Arc<RunServices>` + execute-only state). Phase structs carry `Arc<RunServices>` or `Arc<EngineServices>`, not individual service fields. PR #168's `Option<Client>` plumbing unwinds.
**Prerequisite.** PR #168 must merge to main before Phase 2. Phase 1 is independent of PR #168's merge state.
## Problem Frame
Two smells, one architectural root:
- **Silent env fallback.** `generate()` without an explicit client silently uses `Client::from_env()` — env-only, vault-unaware. Every future call site that forgets a client reintroduces the class bug.
- **`Option<Client>` threading.** PR #168 plumbed `Option<Client>` through phase structs to fix one instance. In production the client is always present; the `Option` is a domain lie.
**Root cause:** `Client` plays two roles today — a short-lived RPC handle AND the point of credential resolution. When a caller forgets to pass one, the "resolution" role silently falls through to env. Separating the two roles — `CredentialSource` is the long-lived authority, `Client` is always derived from one — makes the class bug unrepresentable.
## Requirements Trace
- **R1.** `fabro_llm::generate::generate()` fails to compile when no client is supplied. No runtime fallback of any kind.
- **R2.** Every long-lived runtime context that needs LLM access (`RunServices`, `AppState`, `HookExecutor`) holds `Arc<dyn CredentialSource>`, not a pinned `Client`. CLI subcommands resolve sources on demand via a `CommandContext::llm_source()` helper, not a held field.
- **R3.** OAuth refresh behavior is preserved — clients get fresh tokens at point of use, not at process start.
- **R4.** `RunServices` (cross-phase) and `EngineServices` (execute-only) have distinct types. Retro/finalize/PR-body code cannot reach execute-only state.
- **R5.** PR #168's production bug stays fixed, covered by a pipeline-level regression test (run with vault-only `openai_codex`, auto-PR body succeeds).
- **R6.** Duplicate `build_llm_client` in `fabro-workflow/handler/llm/api.rs` and `fabro-server/server_secrets.rs` collapses to one implementation.
## Scope Boundaries
- Don't rework `Client` internals, provider adapters, `from_credentials`, or any `fabro-llm` provider registration logic.
- Don't redesign vault/credential storage — `VaultCredentialSource` adapts existing `CredentialResolver` logic.
- Don't change phase ordering or per-phase behavior — only where shared state lives.
- Don't touch the `fabro-client` crate extraction (`docs/plans/2026-04-20-002`).
- Don't create `docs/solutions/` (separate opportunity).
## Context & Research
### Relevant Code
**`fabro-llm`:**
- `lib/crates/fabro-llm/src/client.rs``Client::new`, `Client::from_env`, `Client::from_credentials`, `provider_names`. `Client` derives `Clone`.
- `lib/crates/fabro-llm/src/generate.rs``DEFAULT_CLIENT: OnceCell<Arc<Client>>`, `set_default_client`, `get_default_client`, `generate`, `stream_with_tool_loop`, `stream_generate`, `generate_object`. `GenerateParams.client: Option<Arc<Client>>`.
**`fabro-auth`:**
- `lib/crates/fabro-auth/src/resolve.rs``CredentialResolver { vault: Arc<AsyncRwLock<Vault>>, env_lookup: EnvLookup }`. `resolve(provider, usage)` returns `ResolvedCredential::Api(ApiCredential)` or error. Handles OAuth refresh and writes back to vault.
- `ApiCredential`, `ApiKeyHeader` — already imported by `fabro-llm::client::Client::from_env`.
**`fabro-workflow`:**
- `lib/crates/fabro-workflow/src/handler/mod.rs``EngineServices` (13 fields today).
- `lib/crates/fabro-workflow/src/handler/llm/api.rs``build_llm_client(resolver: Option<&CredentialResolver>)`; `AgentApiBackend::create_session_for` and `one_shot` rebuild client per session via `build_llm_client(self.resolver.as_ref()).await?.client` at `:268` and `:349`. **This per-session rebuild is load-bearing for OAuth refresh.**
- `lib/crates/fabro-workflow/src/handler/llm/cli.rs` — parallel `AgentCliBackend` with its own `new_from_env`.
- `lib/crates/fabro-workflow/src/pipeline/types.rs` — phase structs and options.
- `lib/crates/fabro-workflow/src/pipeline/initialize.rs:555-566``SandboxReady` hook fires BEFORE `build_registry` at `:587` builds the LLM client. Any design that requires a client at hook time regresses this.
- `lib/crates/fabro-workflow/src/pipeline/{execute,retro,finalize,pull_request}.rs` — phase implementations.
- `lib/crates/fabro-workflow/src/operations/start.rs``RunSession`, `StartServices`. `RunSession.vault: Option<Arc<AsyncRwLock<Vault>>>` threads to `InitOptions.vault` at `:714`.
**Consumers:**
- `lib/crates/fabro-cli/src/main.rs` — CLI entry.
- `lib/crates/fabro-cli/src/command_context.rs:61-72``CommandContext::with_target` and `with_connection` re-derive context per subcommand with different `storage_dir_override`. **A process-global source install is not correct here.**
- `lib/crates/fabro-cli/src/commands/pr/create.rs` — loads vault at `:103`, calls `maybe_open_pull_request(..., None)`.
- `lib/crates/fabro-cli/src/shared/provider_auth.rs` — already passes explicit client.
- `lib/crates/fabro-hooks/src/executor.rs``execute_prompt` (default client today), `execute_agent` (`Client::from_env()` at `:358`).
- `lib/crates/fabro-agent/src/cli.rs:436``Client::from_env()`.
- `lib/crates/fabro-server/src/server.rs:2488``build_app_state`; `Vault::load` at `:2500`.
- `lib/crates/fabro-server/src/server_secrets.rs:87-112` — duplicate `ProviderCredentials::build_llm_client`.
### Institutional Learnings
- **`docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md`** — `CredentialFallback` named-trait precedent. Same rationale applies here: named trait makes the role obvious at the call site.
- **`docs/plans/2026-04-05-server-canonical-secrets-doctor-repo-plan.md:171,485`** — Already flagged `from_env()` as a smell; this plan resolves the deferred follow-up.
- **`docs/plans/2026-04-22-003-refactor-lock-down-server-secrets-plan.md:74-78`** — Credential taxonomy. LLM provider credentials live on the Vault + `ProviderCredentials` track.
- **`docs/plans/2026-04-08-cli-services-command-context-refactor-plan.md:59-61`** and **`docs/plans/2026-04-23-001-refactor-command-context-alignment-plan.md:273-277`** — Repo has twice rejected adding a peer-wrapper alongside an existing context. This plan's `RunServices`/`EngineServices` split is **composition** (`EngineServices` contains `Arc<RunServices>`), not a peer wrapper — and is about the pipeline (`fabro-workflow`), not CLI (`fabro-cli`). The rejections do not apply.
## Key Technical Decisions
### 1. Credentials are the authority; clients are derived
`CredentialSource` trait lives in `fabro-auth`:
```rust
pub struct ResolvedCredentials {
pub credentials: Vec<ApiCredential>,
pub auth_issues: Vec<(Provider, ResolveError)>,
}
pub trait CredentialSource: Send + Sync {
/// Full resolution with OAuth refresh. Expensive. Returns credentials + auth issues.
async fn resolve(&self) -> Result<ResolvedCredentials, Error>;
/// Cheap preflight: which providers have credentials configured at all?
/// No OAuth refresh. Used for model selection and manifest building.
async fn configured_providers(&self) -> Vec<Provider>;
}
```
The richer return type preserves today's partial-resolution diagnostics. `auth_issues` exists because `CredentialResolver::resolve` can fail for one provider (e.g. "OpenAI OAuth refresh failed") while another succeeds — callers produce user-facing messages like "OpenAI requires re-authentication; Anthropic is not configured." This is load-bearing at four sites: `pipeline/initialize.rs:301-318` (error message on "no usable providers"), `server/src/server.rs:6652` (diagnostics endpoint), `server/src/server.rs:6821-6833` (`create_completion` logs `warn!` per issue), `server/src/diagnostics.rs:90`.
Natural home: `fabro-auth` already owns `ApiCredential`, `CredentialResolver`, and `ResolveError`. No new dep cycles. `fabro-llm` exposes `Client::from_source(&dyn CredentialSource) -> Result<Arc<Client>, Error>` — a convenience that discards `auth_issues` and returns only the Client. Callers that need diagnostics call `source.resolve()` directly and inspect both halves.
**Implementors:**
- `VaultCredentialSource` in `fabro-auth` — wraps `CredentialResolver`. Iterates `Provider::ALL` calling `resolver.resolve(provider, CredentialUsage::ApiRequest)`, collecting credentials and auth issues. Two constructors:
- `VaultCredentialSource::new(Arc<AsyncRwLock<Vault>>)` — default env lookup (used by workflow path).
- `VaultCredentialSource::with_env_lookup(Arc<AsyncRwLock<Vault>>, env_lookup)` — server uses this with its own env policy (preserves today's `ProviderCredentials::with_env_lookup` behavior at `server_secrets.rs:67`). Replaces `build_llm_client` in both `fabro-workflow/handler/llm/api.rs` and `fabro-server/server_secrets.rs`.
- `EnvCredentialSource` in `fabro-auth` — reads env vars for each provider, emits `ApiCredential`s. Replaces `Client::from_env`. `auth_issues` is typically empty for this source.
**Server-side implication:** `ProviderCredentials` collapses into a `VaultCredentialSource` constructed via `with_env_lookup`. The server doesn't need its own `CredentialSource` impl — the env-lookup policy is a constructor argument, not a type-level distinction. This is the key insight that lets the consolidation actually happen rather than just moving duplication behind a trait.
### 2. No process-global state
No `DEFAULT_CLIENT`, no `defaults::install`, no `Client::available_default`. Every long-lived runtime context holds its own `Arc<dyn CredentialSource>`:
- `RunServices.llm_source` — built from `InitOptions.vault` at initialize (or `EnvCredentialSource` if vault is `None`; see §3).
- `AppState.llm_source` — built from `Vault::load` at `build_app_state`.
- `HookExecutor.llm_source` — received on construction from the invoker.
- Standalone agent CLI — builds source at startup from resolved vault or env.
- Tests — build stubs directly, no install ceremony.
**CLI subcommands resolve sources on demand.** `CommandContext` does not hold a source as a field. Instead it exposes a lazy helper:
```rust
impl CommandContext {
pub async fn llm_source(&self) -> Result<Arc<dyn CredentialSource>> { ... }
}
```
This mirrors the existing `CommandContext::server()` pattern (`command_context.rs:106-134`). The helper is re-derived per `with_target`/`with_connection` call, so each subcommand gets the right source for its resolved storage dir — no binding hazard.
This design eliminates: CLI re-derivation hazard (`CommandContext::with_connection` can change storage_dir), test cross-contamination on shared statics, double-install policy questions, feature-gate hazards, and the question "which install site wins."
### 3. `RunServices.llm_source` is always `Some` — no `Option`
Today's `RunSession.vault: Option<Arc<AsyncRwLock<Vault>>>` is optional (workflows can run without a vault, e.g. dry-run or env-only). The plan keeps `RunServices.llm_source` **non-optional** to preserve the elegance of "every context has a source." Rule:
- **Vault is `Some`:** build `VaultCredentialSource` from it.
- **Vault is `None`:** build `EnvCredentialSource::new()` — reads env vars; may resolve empty. Same semantics as today's `build_llm_client` with `resolver: None` branch.
- **Dry-run:** `EngineServices.dry_run` (separate flag, as today) is the authoritative signal. When true, handlers skip LLM stages entirely; `llm_source` is constructed anyway but unused.
- **No credentials anywhere:** `source.resolve()` returns empty `Vec<ApiCredential>`. `Client::from_source` returns a Client with `provider_names().is_empty()`. Consumers that need to `generate()` detect this via the same check as today (`initialize.rs:302`) and error with a helpful message built from `auth_issues`.
The error "No usable LLM providers configured" becomes a point-of-use error rather than an initialize-time error, **except** for graphs where `graph::needs_llm_handler_type` is true — for those, `initialize` preflights `source.resolve()` once and errors early with the same message today produces. Preserves UX.
### 4. Clients are built at point of use
`AgentApiBackend` today rebuilds `Client` per session via `build_llm_client(resolver)` — load-bearing for OAuth refresh on long runs. The new design preserves this pattern: every caller that wants a client calls `Client::from_source(&source).await?` at the point of use. Under the hood, `source.resolve()` calls `resolver.resolve()` which refreshes OAuth tokens and writes back to the vault.
No pinned per-run `Client`. No stale-token regression.
### 5. `GenerateParams.client: Arc<Client>` is required
Not `Option`. `GenerateParams::new(model, client)` takes both. Compile error if a future caller forgets.
### 6. Delete `Client::from_env`
`Client::from_env` was the silent-fallback gateway. It's replaced by `Client::from_source(&EnvCredentialSource::new())` — explicit, one-line, same result, and doesn't leave a footgun for future contributors.
`Client::from_credentials` stays as the lowest-level constructor (used internally by `from_source`).
### 7. `SandboxReady` hooks get a source, not a client
Hooks run at `pipeline/initialize.rs:555`, before `build_registry` at `:587`. In the source-on-context design this is fine: `InitOptions.vault` already exists at hook time; `initialize` builds a `VaultCredentialSource` eagerly (or `EnvCredentialSource` if vault is `None`) and passes it to the hook runner. A `SandboxReady` hook that wants to `generate()` calls `Client::from_source(&source).await?` at its point of use.
### 8. `RunServices` / `EngineServices` split is composition, not peer wrapping
Today's single `EngineServices` (13 fields) mixes two lifetimes:
- **Cross-phase** (live past execute into retro/finalize/PR): `run_store`, `emitter`, `sandbox`, `hook_runner`, `cancel_requested`, `provider`, and the new `llm_source`.
- **Execute-only** (die at the execute→retro boundary): `registry`, `inputs`, `workflow_bundle`, `workflow_path`, `dry_run`, `env`, `git_state`.
Split into two structs with `EngineServices { run: Arc<RunServices>, ...execute_only_fields }`. Retroed/Concluded/PullRequestOptions carry `Arc<RunServices>` only — they can't reach execute-only state by construction.
This is composition (`EngineServices` contains `Arc<RunServices>`), not the peer-wrapper pattern the CLI plans rejected. Handlers read `services.run.emitter` for cross-phase state and `services.registry` for execute-only state. One access shape, two concerns.
### 9. Server's `ProviderCredentials` collapses into `VaultCredentialSource::with_env_lookup`
`fabro-server/src/server_secrets.rs` today owns two capabilities:
- `build_llm_client` (`:87-112`) — full resolution with OAuth refresh, plus `auth_issues`.
- `configured_providers` (`:114-119`) — cheap preflight used for model selection at `server.rs:3929` and `run_manifest.rs:355`.
Both collapse into `VaultCredentialSource`: `build_llm_client``source.resolve()`; `configured_providers``source.configured_providers()` (the second trait method). With `VaultCredentialSource::with_env_lookup` covering the server's env policy (§1), `ProviderCredentials` disappears — `build_app_state` constructs `VaultCredentialSource::with_env_lookup(vault, env_lookup)` directly and stores it as `Arc<dyn CredentialSource>` on `AppState`. Callers at `server.rs:3929` and `run_manifest.rs:355` swap `state.provider_credentials.configured_providers().await``state.llm_source.configured_providers().await`. One trait, one type, two capabilities.
## Open Questions
### Resolved During Planning
- **Trait home?** `fabro-auth` (owns `ApiCredential`, `CredentialResolver`). Avoids `fabro-auth → fabro-llm` cycle.
- **Keep `Client::from_env`?** No. Replaced by `EnvCredentialSource` + `Client::from_source`. One construction path.
- **Process-global static?** No. Every context holds its own source.
- **`Client::available_default`?** Not needed. Callers use `Client::from_source(&ctx.llm_source)` where `ctx` is whatever context they hold.
- **Per-run client caching?** No. `Client::from_source` is called at point of use. Cost is acceptable (one resolver iteration + `from_credentials`), and this is where OAuth refresh happens.
- **OAuth refresh preservation?** Automatic. `resolver.resolve()` handles refresh; it runs every time a caller builds a client from a source.
- **CLI install site?** None. Each `CommandContext`-derived command that needs LLM builds its own source from the context's storage dir.
- **Server install site?** None. `build_app_state` constructs `VaultCredentialSource` once and stores it on `AppState`.
- **`SandboxReady` hook ordering?** No change. `InitOptions.vault` already exists at hook time; source is built before the hook runs.
- **Duplicate `build_llm_client` in server?** Delete. Server uses `VaultCredentialSource`. Parity test in Unit 1.2 guards the deletion.
- **`install_mock_llm` test helper?** Delete. Tests construct sources and clients directly.
- **Partial resolution diagnostics (`auth_issues`)?** Preserved. Trait returns `ResolvedCredentials { credentials, auth_issues }`. Four diagnostic call sites (`initialize.rs:301`, `server.rs:6652`, `server.rs:6821-6833` per-issue `warn!`, `diagnostics.rs:90`) consume `auth_issues` directly via `source.resolve()`. `Client::from_source` is the convenience path for callers that don't need diagnostics.
- **What source exists when `InitOptions.vault` is `None`?** `EnvCredentialSource::new()`. `RunServices.llm_source` is never `Option`. Graph-needs-LLM preflight inside `initialize` produces the same user-facing error today produces.
- **CLI source ownership?** `CommandContext::llm_source(&self) -> Result<Arc<dyn CredentialSource>>` — lazy helper, re-derived per `with_target`/`with_connection`. Mirrors existing `CommandContext::server()`.
### Deferred to Implementation
- **Exact error type on the trait.** `Result<ResolvedCredentials, ?>` — use `fabro-auth::Error` or introduce a trait-level `CredentialSourceError`. Decide while writing the trait.
- **Where `CommandContext::llm_source` caches.** `OnceCell<Arc<dyn CredentialSource>>` on the context like `server: OnceCell<Arc<Client>>` today, or freshly built per call. Decide by measuring: if CLI subcommands only call it once per invocation anyway, a `OnceCell` is overkill.
- **`HookExecutor` construction signature.** Takes `Arc<dyn CredentialSource>`. Workflow callers pass `services.run.llm_source.clone()`; standalone callers construct their own.
- **Test harness fixture.** `RunServices::for_test()` builds a stub source + minimal services. Exact surface decided during Unit 2.1.
## High-Level Technical Design
> *Directional guidance for review, not implementation specification.*
### Client resolution flow
```
Caller has long-lived Arc<dyn CredentialSource>
When a generate() is needed:
let client = Client::from_source(&source).await?;
generate(GenerateParams::new(model, client).prompt("..."))
Under the hood:
Client::from_source(&source)
└── source.resolve() → ResolvedCredentials { credentials, auth_issues }
└── VaultCredentialSource: iterate Provider::ALL, resolver.resolve() (with OAuth refresh)
or EnvCredentialSource: iterate Provider::ALL, read env
└── Client::from_credentials(resolved.credentials) // Client::from_source discards auth_issues
For diagnostics-aware callers (initialize preflight, server diag endpoint):
let resolved = source.resolve().await?;
if resolved.credentials.is_empty() && graph_needs_llm {
// build detailed error from resolved.auth_issues
}
```
### Services topology
```
┌────────────────────── RunServices (Arc) ────────────────────────┐
│ run_store, emitter, sandbox, hook_runner, cancel_requested, │
│ provider, llm_source: Arc<dyn CredentialSource>
└───────────────────────────────┬─────────────────────────────────┘
│ Arc<RunServices>
┌──────────────────── EngineServices (Arc) ───────────────────────┐
│ run: Arc<RunServices>, │
│ registry, inputs, workflow_bundle, workflow_path, │
│ dry_run, env, git_state │
└─────────────────────────────────────────────────────────────────┘
Phase data flow:
Persisted → Initialized { engine: Arc<EngineServices> }
→ Executed { engine: Arc<EngineServices> }
→ Retroed { services: Arc<RunServices> } // drops execute-only state
→ Concluded { services: Arc<RunServices> }
→ Finalized { /* no services — terminal */ }
```
### Where sources come from
| Context | Source construction |
|---|---|
| Workflow run | `pipeline/initialize.rs``VaultCredentialSource::new(vault)` if `InitOptions.vault` is `Some`, else `EnvCredentialSource::new()` |
| CLI subcommand | `CommandContext::llm_source().await?` — lazy helper built from the context's resolved storage dir |
| Server | `build_app_state``VaultCredentialSource::with_env_lookup(vault, env_lookup)` from `Vault::load` (preserves today's server env-lookup policy) |
| Standalone `fabro agent` | CLI startup — resolved vault or `EnvCredentialSource` |
| Tests | Inline stub impl |
## Implementation Units
### Phase 1 — Source-based client resolution
- [x] **Unit 1.1: Add `CredentialSource` trait + `VaultCredentialSource` + `EnvCredentialSource` + `Client::from_source`**
**Goal:** Pure addition of the new abstractions. No consumer changes. Existing code compiles and runs unchanged.
**Requirements:** R2, R3 (groundwork).
**Dependencies:** None.
**Files:**
- Modify: `lib/crates/fabro-auth/src/lib.rs` (re-exports)
- Create: `lib/crates/fabro-auth/src/credential_source.rs` (trait)
- Create: `lib/crates/fabro-auth/src/vault_source.rs` (`VaultCredentialSource`)
- Create: `lib/crates/fabro-auth/src/env_source.rs` (`EnvCredentialSource`)
- Modify: `lib/crates/fabro-llm/src/client.rs` (add `Client::from_source`)
- Test: inline unit tests in each new file; `lib/crates/fabro-llm/src/client.rs` test for `from_source`
**Approach:**
- `struct ResolvedCredentials { credentials: Vec<ApiCredential>, auth_issues: Vec<(Provider, ResolveError)> }`.
- `trait CredentialSource: Send + Sync` with `async fn resolve(&self) -> Result<ResolvedCredentials, Error>`. Use `async_trait::async_trait`.
- `VaultCredentialSource::new(Arc<AsyncRwLock<Vault>>) -> Self` wraps `CredentialResolver::new` (default env lookup).
- `VaultCredentialSource::with_env_lookup(Arc<AsyncRwLock<Vault>>, F) -> Self` where `F: Fn(&str) -> Option<String> + Send + Sync + 'static` wraps `CredentialResolver::with_env_lookup` (preserves server env policy).
- `resolve()` iterates `Provider::ALL`, matching today's logic at `handler/llm/api.rs:43-78` byte-for-byte — both credentials accumulation and `auth_issues` accumulation.
- `configured_providers()` delegates to `CredentialResolver::configured_providers(&vault)` at `fabro-auth/src/resolve.rs:140`. Cheap vault read; no OAuth refresh. Matches today's `ProviderCredentials::configured_providers` at `server_secrets.rs:114-119` byte-for-byte.
- For `EnvCredentialSource`, `configured_providers()` iterates `Provider::ALL` checking env-var presence for each (returns providers whose env vars are set).
- `EnvCredentialSource::new()` reads env for each provider in `Provider::ALL`, emitting `ApiCredential`s. `auth_issues` is always empty. Replaces the body of today's `Client::from_env`.
- `Client::from_source(source: &dyn CredentialSource) -> Result<Arc<Client>, Error>` — calls `source.resolve()`, discards `auth_issues`, calls `Client::from_credentials`, wraps in `Arc`. Callers that need diagnostics call `source.resolve()` directly.
- Move `auth_issue_message` helper (today duplicated in `fabro-workflow/handler/llm/api.rs:82` and `fabro-server/server_secrets.rs:127`) to `fabro-auth` alongside the trait.
- `ApiCredential` must have a redacting `Debug` impl (verify/add using `fabro-util::redaction`). Add unit test asserting `format!("{:?}", credential)` does not echo key material.
**Patterns to follow:**
- `CredentialFallback` trait shape in `docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md`.
- Existing `CredentialResolver` iteration in `lib/crates/fabro-workflow/src/handler/llm/api.rs:43-78` (preserve semantics exactly, including partial-resolution `auth_issues` accumulation).
**Test scenarios:**
- Happy path: `VaultCredentialSource::resolve()` with a vault containing `openai_codex` returns `ResolvedCredentials` with the OpenAI entry in `.credentials` and empty `.auth_issues`.
- Happy path: `VaultCredentialSource::configured_providers()` with a vault containing OpenAI + Anthropic returns `[OpenAi, Anthropic]`. No OAuth refresh happens (verified via a stub vault that panics on write).
- Happy path: `EnvCredentialSource::configured_providers()` with `ANTHROPIC_API_KEY` set returns `[Anthropic]`; with nothing set, returns empty.
- Happy path (partial resolution): `VaultCredentialSource::resolve()` with a vault containing `openai_codex` whose refresh token has expired **and** a working Anthropic `ApiKey` returns both — Anthropic in `.credentials`, `(OpenAi, ResolveError::RefreshFailed{...})` in `.auth_issues`. Matches real auth shapes (`CodexOAuth` is OpenAI-only per `fabro-auth/src/credential.rs:69-73`).
- Happy path: `EnvCredentialSource` with `ANTHROPIC_API_KEY` set returns one entry; with nothing set, returns an empty `ResolvedCredentials` (not an error).
- Happy path: `Client::from_source(&stub)` where stub returns a single-provider `ResolvedCredentials` yields a Client with that provider registered.
- Happy path: `Client::from_source(&VaultCredentialSource::new(vault))` with OAuth credential calls through `resolver.resolve` (verified via a stub vault that records calls).
- Edge case: source returning empty credentials yields a Client with `provider_names().is_empty()` — no error.
- Redaction: `format!("{:?}", credential)` does not contain the key/token.
**Verification:**
- `cargo nextest run -p fabro-auth -p fabro-llm` passes.
- `cargo +nightly-2026-04-14 clippy -p fabro-auth -p fabro-llm --all-targets -- -D warnings` clean.
- [x] **Unit 1.2: Migrate every LLM consumer to hold/pass sources and build clients explicitly**
**Goal:** Every `generate()`/`generate_object()`/`stream_generate()` call site constructs its `Arc<Client>` explicitly (via `Client::from_source`). Every long-lived backend/executor/context holds `Arc<dyn CredentialSource>`. `DEFAULT_CLIENT` and the default-client code path remain temporarily so each migration step compiles/tests cleanly.
**Requirements:** R2, R3, R6.
**Dependencies:** Unit 1.1.
**Files:**
- Modify: `lib/crates/fabro-workflow/src/handler/llm/api.rs``AgentApiBackend` holds `Arc<dyn CredentialSource>`; `create_session_for` and `one_shot` call `Client::from_source(&self.source)` per call (matching today's per-session rebuild). Delete `build_llm_client` and the local `auth_issue_message` helper (both moved to `fabro-auth` in Unit 1.1).
- Modify: `lib/crates/fabro-workflow/src/handler/llm/cli.rs` — parallel treatment for `AgentCliBackend`.
- Modify: `lib/crates/fabro-workflow/src/pipeline/initialize.rs``build_registry` takes/holds `Arc<dyn CredentialSource>`. Builds source from `InitOptions.vault` when `Some`, else `EnvCredentialSource::new()`. Passes source to backends. For `graph_needs_llm` paths, preflights `source.resolve()` once at initialize and produces the same "No usable LLM providers configured: ..." error at `:301-318` by consuming `ResolvedCredentials.auth_issues`. Phase 2 moves the source onto `RunServices`; for this unit, hold it alongside the registry return value.
- Modify: `lib/crates/fabro-workflow/src/pipeline/initialize.rs:555-566``SandboxReady` hook invocation passes the already-built source into the hook runner.
- Modify: `lib/crates/fabro-workflow/src/pipeline/pull_request.rs``build_pr_body`/`maybe_open_pull_request` accept an explicit client (PR #168 already added this; here it becomes non-Option).
- Modify: `lib/crates/fabro-hooks/src/executor.rs``HookExecutor::new(..., source: Arc<dyn CredentialSource>)`. `execute_prompt` and `execute_agent` call `Client::from_source(&self.source)` at point of use. Remove `Client::from_env()` at `:358`.
- Modify: `lib/crates/fabro-hooks/src/runner.rs``HookRunner` carries source, forwards to executor.
- Modify: `lib/crates/fabro-cli/src/main.rs` — no process-global install.
- Modify: `lib/crates/fabro-cli/src/command_context.rs` — add lazy helper `pub async fn llm_source(&self) -> Result<Arc<dyn CredentialSource>>`. Resolves from the context's current storage dir; builds `VaultCredentialSource` when a vault is present, `EnvCredentialSource` otherwise. Follow the existing `CommandContext::server()` pattern at `:106-134`.
- Modify: `lib/crates/fabro-cli/src/commands/pr/create.rs` — replace today's direct `Vault::load` at `:103` with `ctx.llm_source().await?`. Call `Client::from_source(&source)`, pass into `maybe_open_pull_request`.
- Modify: `lib/crates/fabro-cli/src/shared/provider_auth.rs` — already passes an explicit client. No change.
- Modify: `lib/crates/fabro-agent/src/cli.rs` — replace `Client::from_env()` with source-based resolution (either via `CommandContext::llm_source` or an equivalent startup helper).
- Modify/delete: `lib/crates/fabro-server/src/server_secrets.rs` — delete `ProviderCredentials` entirely (struct at `:61-75`, `build_llm_client` at `:87-112`, `configured_providers` at `:114-119`, local `auth_issue_message` at `:127`). Both capabilities move to `VaultCredentialSource` via the `CredentialSource` trait. Diagnostic callers at `server.rs:6652`, `server.rs:6821`, and `diagnostics.rs:90` consume `ResolvedCredentials.auth_issues` from `source.resolve()`.
- Modify: `lib/crates/fabro-server/src/server.rs``build_app_state` (`:2488`) constructs `VaultCredentialSource::with_env_lookup(vault, env_lookup)` and stores it as `Arc<dyn CredentialSource>` on `AppState`. `create_completion` (`:6821-6833`) calls `state.llm_source.resolve()` directly (not `Client::from_source`), logs each `auth_issue` via `warn!` (preserving today's observability at `:6831-6833`), then builds the client via `Client::from_credentials(resolved.credentials)`. Server diagnostics endpoint (`:6652`) same pattern. Site at `:3929` swaps `state.provider_credentials.configured_providers().await``state.llm_source.configured_providers().await`.
- Modify: `lib/crates/fabro-server/src/run_manifest.rs:355` — same swap: `state.provider_credentials.configured_providers().await``state.llm_source.configured_providers().await`.
- Test: per-crate unit tests; a workflow-level integration test that runs a minimal pipeline with a vault-only `openai_codex` credential and asserts agent stages use a fresh client each session.
**Approach:**
- Mechanical migration. Each consumer that previously used the default client now constructs explicitly from a source it already has access to.
- `AgentApiBackend` today: `build_llm_client(self.resolver.as_ref()).await?.client` per session. New: `Client::from_source(&self.source).await?` per session. Functionally equivalent; OAuth refresh still happens because `source.resolve()` calls `resolver.resolve()` which refreshes.
- `build_registry` no longer needs to return a client — it returns the registry and the source (passed to backends). Clients are built per session inside the backends.
- Diagnostic sites that previously called `build_llm_client(...).auth_issues` now call `source.resolve()` and consume `.auth_issues` directly. Four sites: `initialize.rs:301` (error message), `server.rs:6652` (diagnostics endpoint), `server.rs:6821-6833` (`create_completion` per-issue `warn!`), `diagnostics.rs:90`. Each of these stays on the `resolve()` path rather than using the `Client::from_source` convenience — they need both halves.
**Patterns to follow:**
- `AgentApiBackend::create_session_for:268` (today's per-session pattern) is the model.
- `CommandContext::server()` (`command_context.rs:106-134`) is the model for the new `llm_source()` helper — lazy, re-derived per context, cached via `OnceCell` per context.
**Test scenarios:**
- Happy path (workflow, vault): pipeline run with vault-configured `openai_codex` produces a Client per session with `openai` registered. Verified via a test that wraps a stub source and counts `resolve()` calls (should equal session count + 1 for retro/PR + 1 for initialize preflight).
- Happy path (workflow, no vault): `InitOptions.vault = None`, graph has no LLM handlers — initialize succeeds; `llm_source` is an `EnvCredentialSource` and is never queried.
- Error path (workflow, no vault, graph needs LLM): `InitOptions.vault = None`, graph has LLM handlers, no env credentials — initialize preflight `source.resolve()` returns empty credentials; error produced is the same "No usable LLM providers configured: ..." message today builds from `auth_issues` (empty for env source → plain "No LLM providers configured" fallback).
- Error path (workflow, vault with one bad credential + one good): partial resolution — `openai_codex` with expired refresh token + working Anthropic `ApiKey`. Initialize preflight succeeds (Anthropic is usable); `auth_issues` is logged or surfaced per today's behavior.
- Happy path (CLI `pr create`): command calls `ctx.llm_source().await?`, builds client, generates PR body. No `None` parameter anywhere.
- Happy path (CLI context re-derivation): `ctx.with_connection(args)` produces a new context with its own `llm_source` bound to the new storage dir. Verified by calling `llm_source` on two derived contexts with different storage dirs.
- Happy path (hooks, workflow-invoked): `SandboxReady` hook that wants to generate receives source from `initialize`; builds a client and calls `generate`.
- Happy path (hooks, standalone agent): `fabro agent` CLI builds source at startup, passes to `HookExecutor`; hooks build clients as needed.
- Regression: server `create_completion` request generates successfully after `build_app_state` stores the source on `AppState`.
- Regression: server diagnostic endpoint at `server.rs:6652` still reports auth issues for misconfigured providers.
- Regression: server `create_completion` at `server.rs:6821-6833` still emits `warn!` per auth issue before building the client. Verified via a test with a partial-resolution fixture source (one working provider, one failing) that captures `warn!` events and asserts the expected log output.
- Regression: `AgentCliBackend` (CLI-mode agent) works with source-based resolution.
- Parity (resolve): `VaultCredentialSource::with_env_lookup(vault, env_lookup).resolve()` output (credentials + auth_issues) matches today's `ProviderCredentials::build_llm_client()` output for the same fixture vault + env policy.
- Parity (configured_providers): `VaultCredentialSource::with_env_lookup(vault, env_lookup).configured_providers()` output matches today's `ProviderCredentials::configured_providers()` output for the same fixture. Required before deleting the old path.
**Verification:**
- `cargo nextest run --workspace` passes.
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` clean.
- `rg 'Client::from_env' lib/crates/` — only `client.rs` (constructor definition), `EnvCredentialSource` (if it reuses the body), and test files.
- `rg 'build_llm_client' lib/crates/` — zero hits.
- [x] **Unit 1.3: Delete the default-client machinery and `Client::from_env`; require `GenerateParams.client`**
**Goal:** Atomic cleanup. Every consumer already passes an explicit client (Unit 1.2). Now remove the fallback paths.
**Requirements:** R1, R6.
**Dependencies:** Unit 1.2.
**Files:**
- Modify: `lib/crates/fabro-llm/src/generate.rs` — delete `DEFAULT_CLIENT`, `set_default_client`, `get_default_client`, the `Client::from_env()` fallback inside `get_default_client`. `GenerateParams.client: Arc<Client>` (not `Option`). `GenerateParams::new(model, client)` takes both. Drop the `.client(...)` builder method.
- Modify: `lib/crates/fabro-llm/src/lib.rs` — drop `set_default_client` re-export.
- Modify: `lib/crates/fabro-llm/src/client.rs` — delete `Client::from_env`. Keep `Client::from_credentials` (used internally by `EnvCredentialSource` and `Client::from_source`).
- Modify: `lib/crates/fabro-workflow/src/pipeline/pull_request.rs` — delete `install_mock_llm` helper and the three `install_mock_llm()` call sites. Tests construct clients directly via a mock source.
- Modify: ~30 test sites in `lib/crates/fabro-llm/src/generate.rs` — use a helper `#[cfg(test)] fn test_params(model: &str) -> GenerateParams` that constructs a mock client.
- Test: new negative test — attempting to build `GenerateParams` without a client is a compile error (trybuild or inspection).
**Approach:**
- Introduce the test helper first, then swap sites mechanically.
- `GenerateParams` spread patterns (`GenerateParams { response_format: ..., ..params }`) continue to work since `client` is a field with a value being propagated.
- `MockProvider` in `pull_request.rs` keeps its `name` field (added by PR #168) — useful for the regression test where one mock is registered under the "openai" provider name.
**Test scenarios:**
- Compile-time enforcement: `GenerateParams::new(model)` without a client arg fails to compile (by the struct shape).
- All existing `fabro-llm` generate tests pass with `test_params(model)` swap.
- Regression (pipeline-level): workflow with vault-only `openai_codex` runs to completion and generates a PR body using vault-resolved credentials. (This is the class-bug fix from PR #168, now type-enforced.)
**Verification:**
- `cargo nextest run --workspace` passes.
- `rg 'DEFAULT_CLIENT|set_default_client|get_default_client|install_mock_llm|Client::from_env' lib/crates/` — zero hits.
- `rg 'Option<Arc<Client>>' lib/crates/` — zero hits in production code.
### Phase 2 — `RunServices` split + PR #168 `Option<Client>` unwind
**Prerequisite:** PR #168 must have merged to main.
- [x] **Unit 2.1: Introduce `RunServices` + `EngineServices` split**
**Goal:** Today's `EngineServices` (13 fields, mixed lifetimes) partitions into cross-phase `RunServices` + execute-only `EngineServices`. Handlers read cross-phase state as `services.run.xxx`.
**Requirements:** R4.
**Dependencies:** Phase 1 complete.
**Files:**
- Create: `lib/crates/fabro-workflow/src/services.rs` (both structs + `RunServices::for_test`)
- Modify: `lib/crates/fabro-workflow/src/handler/mod.rs` — delete old `EngineServices`; re-export from `services`.
- Modify: `lib/crates/fabro-workflow/src/handler/{parallel,manager_loop,prompt,agent,command,fan_in,human}.rs` — mechanical rewrite: `services.run_store``services.run.run_store`, etc.
- Modify: `lib/crates/fabro-workflow/src/node_handler.rs` — same rewrites.
- Modify: `lib/crates/fabro-workflow/src/pipeline/initialize.rs``build_registry` now returns `Arc<EngineServices>` (holding `Arc<RunServices>` with `llm_source`). Dry-run still produces a valid services pair.
- Modify: `lib/crates/fabro-workflow/src/pipeline/execute.rs` — use `Arc<EngineServices>` directly instead of reconstructing it.
- Modify: `lib/crates/fabro-workflow/src/test_support.rs` — test fixture adapts.
- Test: handler unit tests and `fabro-workflow/tests/it/integration.rs` pipeline tests.
**Approach:**
| Field | Lives on |
|---|---|
| `run_store` | `RunServices` |
| `emitter` | `RunServices` |
| `sandbox` | `RunServices` |
| `hook_runner` | `RunServices` |
| `cancel_requested` | `RunServices` |
| `provider` | `RunServices` |
| `llm_source` | `RunServices` (**new**) |
| `registry` | `EngineServices` |
| `inputs` | `EngineServices` |
| `workflow_bundle` | `EngineServices` |
| `workflow_path` | `EngineServices` |
| `dry_run` | `EngineServices` |
| `env` | `EngineServices` |
| `git_state` | `EngineServices` |
- `RunServices::for_test() -> Arc<Self>` builds a minimal services bundle with a stub source. Used by handler and pipeline tests.
- No delegation methods on `EngineServices`; handlers read through `services.run.xxx` explicitly. More lines but clearer boundary.
**Patterns to follow:**
- Existing `Arc<EngineServices>` construction at `pipeline/execute.rs:84`.
**Test scenarios:**
- Happy path: `RunServices::for_test()` constructs cleanly and supports all handler unit tests.
- Happy path: end-to-end pipeline test runs without regression.
- Type-level: a retro-phase test asserting `services.run.emitter` compiles but `services.registry` does not (via a compile-fail test or a comment marker).
- Edge case: `services.run_hooks(&HookContext)` method lives on `RunServices` since `hook_runner` moved there.
- Edge case: `services.git_state()` / `set_git_state(...)` stays on `EngineServices` (execute-only).
**Verification:**
- `cargo nextest run -p fabro-workflow` passes.
- `cargo clippy -p fabro-workflow --all-targets -- -D warnings` clean.
- `rg 'services\.run_store' lib/crates/fabro-workflow/src/handler/` — zero hits (all now `services.run.run_store`).
- [x] **Unit 2.2: Shrink phase structs; unwind PR #168 `Option<Client>`**
**Goal:** `Initialized`/`Executed` carry `Arc<EngineServices>`; `Retroed`/`Concluded` carry `Arc<RunServices>`. Delete `Option<Client>` threading introduced by PR #168.
**Requirements:** R4, R5.
**Dependencies:** Unit 2.1 + PR #168 merged.
**Files:**
- Modify: `lib/crates/fabro-workflow/src/pipeline/types.rs` — shrink phase structs (below), shrink Options structs.
- Modify: `lib/crates/fabro-workflow/src/pipeline/execute.rs` — destructure updates (carry `engine`).
- Modify: `lib/crates/fabro-workflow/src/pipeline/retro.rs` — read `executed.engine.run`; drop `options.llm_client`.
- Modify: `lib/crates/fabro-workflow/src/pipeline/finalize.rs` — destructure updates.
- Modify: `lib/crates/fabro-workflow/src/pipeline/pull_request.rs` — drop `Option<Client>` parameter from `build_pr_body`/`maybe_open_pull_request`/`pull_request`; build via `Client::from_source(&services.run.llm_source)`.
- Modify: `lib/crates/fabro-workflow/src/operations/start.rs``RunSession::run` threads services Arc through phase calls (no longer cloning `executed.llm_client`).
- Modify: `lib/crates/fabro-cli/src/commands/pr/create.rs` — build `RunServices::for_cli(source, run_store, ...)` and pass to `maybe_open_pull_request`.
- Test: `lib/crates/fabro-workflow/tests/it/integration.rs` pipeline-level regression; update phase-level tests.
**Approach — phase structs after the split:**
- `Initialized { graph, source, inputs, run_options, workflow_path, workflow_bundle, checkpoint, seed_context, engine: Arc<EngineServices>, model }`
- `Executed { graph, outcome, run_options, duration_ms, final_context, engine: Arc<EngineServices>, model }`
- `Retroed { graph, outcome, run_options, duration_ms, retro, services: Arc<RunServices> }`
- `Concluded { run_id, outcome, conclusion, pushed_branch, graph, run_options, services: Arc<RunServices> }`
- `Finalized { run_id, outcome, conclusion, pushed_branch, pr_url }` (unchanged, terminal)
- `RetroOptions` drops `llm_client`, `provider`, `sandbox`, `run_store`, `emitter` (read from services); keeps `run_id`, `workflow_name`, `goal`, `run_dir`, `failed`, `run_duration_ms`, `enabled`, `model`.
- `PullRequestOptions` drops `run_store`; keeps `run_dir`, `pr_config`, `github_app`, `origin_url`, `model`.
- `build_pr_body(services: &RunServices, diff, goal, model, conclusion) -> Result<String, String>` — builds client via `Client::from_source(&services.llm_source)`.
All phase structs retain `#[non_exhaustive]` (already present).
**Patterns to follow:**
- Unit 2.1's `Arc<EngineServices>` shape.
**Test scenarios:**
- Regression (the R5 pipeline-level test): workflow run with vault-configured `openai_codex`, no `OPENAI_API_KEY` env, generates a PR body successfully. Previously failed with "Provider 'openai' not registered."
- Happy path: CLI `fabro pr create` resolves vault, builds source, builds `RunServices::for_cli`, calls `maybe_open_pull_request`.
- Edge case: retro when disabled still no-ops (doesn't call `from_source`).
- Edge case: workflow with no LLM stages and no vault initializes cleanly (source built from vault; `Client::from_source` is called lazily by consumers, not eagerly).
**Verification:**
- `cargo nextest run --workspace` passes.
- `rg 'llm_client:' lib/crates/fabro-workflow/` — zero hits in non-test code.
- `rg 'Option<Client>' lib/crates/` — zero production hits.
## System-Wide Impact
- **Interaction graph:** Every `generate`/`generate_object`/`stream_generate` consumer is affected — `fabro-workflow`, `fabro-hooks`, `fabro-agent`, `fabro-cli`, `fabro-server`. Each now builds its client from a source it already holds.
- **Error propagation:** Source-resolution errors (vault lookup failure, OAuth refresh failure) surface at `Client::from_source` call sites, which is the same point they surface today at `build_llm_client` call sites. Error semantics preserved.
- **State lifecycle risks:** `Client::from_source` builds a fresh client per call. For most contexts this runs once per long-running operation (per session, per PR body, per hook invocation). The cost is identical to today's `build_llm_client(resolver)` pattern — unchanged.
- **API surface parity:** `fabro_llm::set_default_client` re-export deletes. `GenerateParams::new` signature changes. `Client::from_env` deletes. Internal-only API shape changes; no external consumers in this monorepo.
- **Integration coverage:** R5's pipeline-level regression test (Unit 2.2) is the critical insurance — proves the class bug is closed at the pipeline shape level, not just at one unit.
- **Credential trust boundary:** `RunServices.llm_source` is shared across all phases by design. Workflow stages run against the same vault as retro/PR body — which matches today's behavior. Callers deploying for multi-tenant workflows must not mix trust levels in a single run. Per-phase credential scoping is out of scope.
- **Unchanged invariants:** `Vault`, `CredentialResolver`, `ApiCredential`, `Provider::ALL`, `ResolvedCredential` enum, phase ordering, `#[non_exhaustive]` on all phase structs, OAuth refresh behavior.
## Risks & Dependencies
| Risk | Mitigation |
|---|---|
| PR #168 lands with `Option<Client>` threading that Phase 2 needs to unwind. If PR #168 stalls, Phase 2 blocks. | Phase 1 is independent of PR #168 and can land first. Phase 2 starts when PR #168 merges. |
| 30+ `GenerateParams::new("mock-model")` test sites in `fabro-llm/src/generate.rs`. | Introduce `test_params(model)` helper before migrating sites; one-line edits thereafter (Unit 1.3). |
| Phase struct destructure patterns in `pipeline/execute.rs`, `pipeline/retro.rs`, `pipeline/finalize.rs` break when struct shape changes. | Unit 2.2 updates them explicitly. `#[non_exhaustive]` protects external crates; in-crate destructures adjust. |
| Server `ProviderCredentials` owns two capabilities (`build_llm_client` + `configured_providers`). Consolidation must preserve both. | Unit 1.2 parity tests on both paths: `resolve()` vs today's `build_llm_client()` (credentials + auth_issues), and `configured_providers()` vs today's `configured_providers()`, against the same fixture vault + env policy. Reconcile any divergence before deletion. |
| `SandboxReady` hooks today have no client; tomorrow they call `Client::from_source(&source)` on demand. Construction cost per hook. | Source already holds the resolver; `source.resolve()` is the same cost as today's `build_llm_client` call that would happen anyway if a hook wanted to generate. No net regression. |
| `AgentApiBackend` per-session client rebuild is load-bearing for OAuth refresh. | Preserved verbatim: backend holds source, calls `Client::from_source(&source)` per session. `source.resolve()``resolver.resolve()` → OAuth refresh. Identical behavior. |
## Documentation / Operational Notes
- Add a short strategy doc at `docs-internal/llm-client-resolution.md` describing the source-on-context rule: `fabro-auth::CredentialSource` is the authority, `Client` is always derived via `Client::from_source`, `generate()` requires an explicit client. Reference `docs-internal/server-secrets-strategy.md` for the adjacent "no `from_env` in production paths" enforcement model.
- A future follow-up should create a `docs/solutions/` entry capturing "pipeline boundary drop" as a pattern learning. Out of scope for this plan.
## Sources & References
- **Related PR:** #168 (`fix(workflow): reuse resolved llm client for auto-pr`) — the point fix this refactor replaces.
- **Related plans:**
- `docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md``CredentialFallback` named-trait precedent.
- `docs/plans/2026-04-05-server-canonical-secrets-doctor-repo-plan.md``from_env()` smell pre-flagged.
- `docs/plans/2026-04-22-003-refactor-lock-down-server-secrets-plan.md` — credential taxonomy.
- `docs/plans/2026-04-08-cli-services-command-context-refactor-plan.md`, `docs/plans/2026-04-23-001-refactor-command-context-alignment-plan.md` — "no peer wrapper" precedent (addressed in Key Technical Decisions).
- **Strategy docs:**
- `docs-internal/server-secrets-strategy.md` — the `set_var`/`remove_var` enforcement model is precedent for enforcing "no `from_env` in production paths."
- **Target files:** `lib/crates/fabro-llm/`, `lib/crates/fabro-auth/`, `lib/crates/fabro-workflow/`, `lib/crates/fabro-cli/`, `lib/crates/fabro-server/`, `lib/crates/fabro-hooks/`, `lib/crates/fabro-agent/`.
## Unresolved questions
- **`docs/solutions/` seeding** — out of scope for this plan; flagged as a follow-up.
- **Observability** — should `VaultCredentialSource::resolve` emit a tracing event on successful resolution? Low-value, but worth deciding while implementing.

View file

@ -16,6 +16,7 @@ The `fabro-agent` crate provides a session-based AI agent that runs an LLM with
```toml title="Cargo.toml"
[dependencies]
fabro-auth = { git = "https://github.com/fabro-sh/fabro" }
fabro-agent = { git = "https://github.com/fabro-sh/fabro" }
fabro-llm = { git = "https://github.com/fabro-sh/fabro" }
tokio = { version = "1", features = ["full"] }
@ -25,18 +26,20 @@ tokio = { version = "1", features = ["full"] }
```rust
use fabro_agent::{
AnthropicProfile, LocalSandbox, Session, SessionConfig,
AnthropicProfile, LocalSandbox, Session, SessionOptions,
};
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use std::path::PathBuf;
use std::sync::Arc;
#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let client = Client::from_env().await?;
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?.as_ref().clone();
let sandbox = Arc::new(LocalSandbox::new(PathBuf::from(".")));
let profile = Arc::new(AnthropicProfile::new("claude-sonnet-4-5"));
let config = SessionConfig::default();
let config = SessionOptions::default();
let mut session = Session::new(client, profile, sandbox, config);
session.initialize().await;
@ -66,9 +69,9 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
```rust
pub fn new(
llm_client: Client,
provider_profile: Arc<dyn ProviderProfile>,
provider_profile: Arc<dyn AgentProfile>,
sandbox: Arc<dyn Sandbox>,
config: SessionConfig,
config: SessionOptions,
) -> Self
```
@ -97,7 +100,7 @@ pub fn new(
| `steer(message)` | Injects a system-level guidance message into the next LLM call. |
| `follow_up(message)` | Queues a follow-up user message after the current turn completes. |
### SessionConfig
### SessionOptions
All fields are public. Key settings with their defaults:
@ -159,10 +162,10 @@ The `DaytonaSandbox` implementation (feature-gated: `daytona`) runs inside a Day
### Provider profiles
The `ProviderProfile` trait encapsulates LLM-specific system prompts, tool definitions, and capability metadata. It controls how the agent presents itself to the model.
The `AgentProfile` trait encapsulates LLM-specific system prompts, tool definitions, and capability metadata. It controls how the agent presents itself to the model.
```rust
pub trait ProviderProfile: Send + Sync {
pub trait AgentProfile: Send + Sync {
fn provider(&self) -> Provider;
fn model(&self) -> &str;
fn tool_registry(&self) -> &ToolRegistry;
@ -250,10 +253,10 @@ impl ToolHookCallback for MyHooks {
}
```
Pass hooks via `SessionConfig`:
Pass hooks via `SessionOptions`:
```rust
let config = SessionConfig {
let config = SessionOptions {
tool_hooks: Some(Arc::new(MyHooks)),
..Default::default()
};
@ -265,7 +268,7 @@ For simple sync approval, use `ToolApprovalAdapter` to wrap a closure:
use fabro_agent::ToolApprovalAdapter;
use std::sync::Arc;
let config = SessionConfig {
let config = SessionOptions {
tool_hooks: Some(Arc::new(ToolApprovalAdapter(Arc::new(|tool_name, _args| {
if tool_name == "shell" {
Err("shell is not allowed".into())
@ -299,6 +302,7 @@ You can use it independently of Fabro's workflow engine — add it as a dependen
```toml title="Cargo.toml"
[dependencies]
fabro-auth = { git = "https://github.com/fabro-sh/fabro" }
fabro-llm = { git = "https://github.com/fabro-sh/fabro" }
tokio = { version = "1", features = ["full"] }
serde_json = "1"
@ -306,20 +310,20 @@ serde_json = "1"
### Quick start
The simplest path is `Client::from_env()`, which auto-registers providers based on environment variables (`ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`, etc.):
The simplest path is an environment-backed `CredentialSource`, then `Client::from_source(&source)`. That keeps credential resolution explicit while still auto-reading environment variables such as `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, and `GEMINI_API_KEY`.
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::generate::{generate, GenerateParams};
use fabro_llm::set_default_client;
#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let client = Client::from_env().await?;
set_default_client(client);
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?;
let result = generate(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("Explain ownership in Rust in two sentences.")
).await?;
@ -333,13 +337,17 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
`Client` is the core type that holds provider adapters and middleware. It routes each request to the appropriate provider.
#### Creating from environment
#### Creating from a credential source
```rust
let client = Client::from_env().await?;
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?;
```
This checks for API key environment variables and registers adapters for each provider found:
For env-backed usage, `EnvCredentialSource` checks for API key environment variables and registers adapters for each provider found:
| Environment variable | Provider |
|---|---|
@ -351,7 +359,7 @@ This checks for API key environment variables and registers adapters for each pr
| `MINIMAX_API_KEY` | Minimax |
| `INCEPTION_API_KEY` | Inception |
The first provider registered becomes the default. Optional base URL overrides (e.g. `ANTHROPIC_BASE_URL`) are also read.
The first provider registered becomes the default. Optional base URL overrides (e.g. `ANTHROPIC_BASE_URL`) are also read. For vault-backed usage inside Fabro, use `fabro_auth::VaultCredentialSource` instead.
#### Creating manually
@ -394,10 +402,14 @@ The `generate()` function wraps the client with automatic tool execution loops,
#### Basic completion
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::generate::{generate, GenerateParams};
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let result = generate(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.system("You are a helpful assistant.")
.prompt("What is the capital of France?")
.temperature(0.0)
@ -411,10 +423,14 @@ println!("{}", result.text());
Use `.messages()` instead of `.prompt()` to pass a full conversation history:
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::types::Message;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let result = generate(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.messages(vec![
Message::user("My name is Alice."),
Message::assistant("Hello Alice! How can I help you?"),
@ -431,7 +447,7 @@ You cannot use both `.prompt()` and `.messages()` on the same request — this r
| Method | Type | Description |
|---|---|---|
| `new(model)` | `impl Into<String>` | Required. Model ID or alias (e.g. `"opus"`, `"claude-sonnet-4-5"`) |
| `new(model, client)` | `(impl Into<String>, Arc<Client>)` | Required. Model ID or alias plus the client to use |
| `.prompt(text)` | `impl Into<String>` | Convenience: sends a single user message |
| `.messages(msgs)` | `Vec<Message>` | Full conversation history |
| `.system(text)` | `impl Into<String>` | System prompt |
@ -446,7 +462,6 @@ You cannot use both `.prompt()` and `.messages()` on the same request — this r
| `.provider(name)` | `impl Into<String>` | Force a specific provider |
| `.max_retries(n)` | `u32` | Retry count for transient errors (default: 2) |
| `.timeout(config)` | `TimeoutConfig` | Total and per-step timeouts |
| `.client(client)` | `Arc<Client>` | Override the default client |
| `.abort_signal(token)` | `CancellationToken` | Cancel generation |
| `.stop_when(f)` | `Fn(&[StepResult]) -> bool` | Custom stop condition after each tool round |
@ -480,6 +495,8 @@ Tools let the model call functions during generation. There are two kinds:
#### Defining an active tool
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::tools::Tool;
use serde_json::json;
@ -503,8 +520,12 @@ let weather = Tool::active(
#### Using tools with generate
```rust
# use fabro_auth::EnvCredentialSource;
# use fabro_llm::client::Client;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let result = generate(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("What's the weather in San Francisco?")
.tools(vec![weather])
.max_tool_rounds(5)
@ -527,18 +548,22 @@ Control how the model selects tools:
use fabro_llm::types::ToolChoice;
// Let the model decide (default)
GenerateParams::new("opus").tool_choice(ToolChoice::Auto);
# use fabro_auth::EnvCredentialSource;
# use fabro_llm::client::Client;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
GenerateParams::new("opus", client.clone()).tool_choice(ToolChoice::Auto);
// Force a specific tool
GenerateParams::new("opus").tool_choice(ToolChoice::Named {
GenerateParams::new("opus", client.clone()).tool_choice(ToolChoice::Named {
tool_name: "get_weather".into()
});
// Force the model to use some tool
GenerateParams::new("opus").tool_choice(ToolChoice::Required);
GenerateParams::new("opus", client.clone()).tool_choice(ToolChoice::Required);
// Prevent tool use
GenerateParams::new("opus").tool_choice(ToolChoice::None);
GenerateParams::new("opus", client.clone()).tool_choice(ToolChoice::None);
```
#### Passive tools
@ -546,6 +571,10 @@ GenerateParams::new("opus").tool_choice(ToolChoice::None);
Passive tools let you handle execution yourself:
```rust
# use fabro_auth::EnvCredentialSource;
# use fabro_llm::client::Client;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let search = Tool::passive(
"search",
"Search the codebase",
@ -559,7 +588,7 @@ let search = Tool::passive(
);
let result = generate(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("Find all uses of the Config struct")
.tools(vec![search])
).await?;
@ -577,11 +606,15 @@ for call in result.tool_calls() {
For simple cases where you only need the text deltas:
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::generate::{stream, GenerateParams};
use futures::StreamExt;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let stream_result = stream(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("Write a haiku about Rust")
).await?;
@ -596,12 +629,16 @@ while let Some(chunk) = text_stream.next().await {
For fine-grained control, consume `StreamEvent` variants directly:
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::generate::{stream, GenerateParams};
use fabro_llm::types::StreamEvent;
use futures::StreamExt;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let mut stream_result = stream(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("Explain monads")
).await?;
@ -646,9 +683,13 @@ while let Some(event) = stream_result.next().await {
Generate typed JSON objects that conform to a JSON Schema:
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::generate::{generate_object, GenerateParams};
use serde_json::json;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let schema = json!({
"type": "object",
"properties": {
@ -663,7 +704,7 @@ let schema = json!({
});
let result = generate_object(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("Generate a profile for a fictional character"),
schema,
).await?;
@ -712,8 +753,15 @@ impl Middleware for LoggingMiddleware {
Add middleware to the client:
```rust
let mut client = Client::from_env().await?;
client.add_middleware(Arc::new(LoggingMiddleware));
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use std::sync::Arc;
let source = EnvCredentialSource::new();
let mut client = Client::from_source(&source).await?;
Arc::get_mut(&mut client)
.expect("install middleware before sharing the client")
.add_middleware(Arc::new(LoggingMiddleware));
```
### Model catalog
@ -820,8 +868,12 @@ Retry only fires when `error.retryable()` returns `true` and respects `Retry-Aft
Pass a `CancellationToken` to interrupt long-running generation:
```rust
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use tokio_util::sync::CancellationToken;
# let source = EnvCredentialSource::new();
# let client = Client::from_source(&source).await?;
let token = CancellationToken::new();
let token_clone = token.clone();
@ -832,7 +884,7 @@ tokio::spawn(async move {
});
let result = generate(
GenerateParams::new("opus")
GenerateParams::new("opus", client.clone())
.prompt("Write a novel")
.abort_signal(token)
).await;

View file

@ -24,6 +24,7 @@ workspace = true
[dependencies]
clap.workspace = true
anyhow.workspace = true
fabro-auth = { path = "../fabro-auth" }
fabro-config = { path = "../fabro-config", features = ["clap"] }
fabro-types = { path = "../fabro-types" }
fabro-llm = { path = "../fabro-llm" }
@ -31,6 +32,7 @@ fabro-model = { path = "../fabro-model" }
fabro-mcp = { path = "../fabro-mcp" }
fabro-sandbox = { path = "../fabro-sandbox" }
fabro-util = { path = "../fabro-util" }
fabro-vault = { path = "../fabro-vault" }
fabro-http.workspace = true
thiserror.workspace = true
serde.workspace = true

View file

@ -8,6 +8,9 @@ use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use clap::{Args, Parser};
use fabro_auth::{CredentialSource, EnvCredentialSource, VaultCredentialSource};
use fabro_config::Storage;
use fabro_config::user::default_storage_dir;
use fabro_llm::Error as LlmError;
use fabro_llm::client::Client;
use fabro_llm::middleware::{Middleware, NextFn, NextStreamFn};
@ -16,9 +19,10 @@ use fabro_llm::types::{Request, Response};
use fabro_mcp::config::McpServerSettings;
use fabro_model::{Catalog, ModelHandle, Provider};
use fabro_util::terminal::Styles;
use fabro_vault::Vault;
use tokio::io::{AsyncWriteExt, stdout};
use tokio::signal;
use tokio::sync::Mutex as AsyncMutex;
use tokio::sync::{Mutex as AsyncMutex, RwLock as AsyncRwLock};
use crate::config::{ToolApprovalAdapter, ToolApprovalFn, ToolHookCallback};
use crate::error::InterruptReason;
@ -215,20 +219,18 @@ fn summarizer_model_id(provider: Provider) -> ModelHandle {
}
}
fn build_summarizer(provider: Provider, llm_client: Option<Client>) -> Option<WebFetchSummarizer> {
let client = llm_client?;
Some(WebFetchSummarizer {
client,
fn build_summarizer(provider: Provider, llm_client: Client) -> WebFetchSummarizer {
WebFetchSummarizer {
client: llm_client,
model_id: summarizer_model_id(provider),
})
}
}
fn build_profile(
provider: Provider,
model: &str,
llm_client: Option<Client>,
summarizer: Option<WebFetchSummarizer>,
) -> Box<dyn AgentProfile> {
let summarizer = build_summarizer(provider, llm_client);
match provider {
Provider::OpenAi => Box::new(OpenAiProfile::with_summarizer(model, summarizer)),
Provider::Kimi
@ -243,6 +245,35 @@ fn build_profile(
}
}
fn parse_provider(args: &AgentArgs) -> anyhow::Result<Provider> {
let provider_str = args.provider.as_deref().unwrap_or("anthropic");
provider_str
.parse()
.map_err(|_| anyhow::anyhow!("unknown provider: {provider_str}"))
}
fn standalone_llm_source() -> Arc<dyn CredentialSource> {
let storage_dir = default_storage_dir();
match Vault::load(Storage::new(storage_dir).secrets_path()) {
Ok(vault) => Arc::new(VaultCredentialSource::new(Arc::new(AsyncRwLock::new(
vault,
)))),
Err(_) => Arc::new(EnvCredentialSource::new()),
}
}
fn ensure_provider_registered(client: &Client, provider: Provider) -> anyhow::Result<()> {
if client
.provider_names()
.iter()
.any(|name| *name == provider.as_str())
{
return Ok(());
}
anyhow::bail!("LLM credentials not configured for provider '{provider}'");
}
fn format_tool_args(args: &serde_json::Value, cwd: &str) -> String {
let cwd_prefix = if cwd.ends_with('/') {
cwd.to_string()
@ -402,7 +433,26 @@ pub async fn run_with_args(
args: AgentArgs,
mcp_servers: Vec<McpServerSettings>,
) -> anyhow::Result<()> {
run_with_args_and_client(args, None, mcp_servers).await
let llm_source = standalone_llm_source();
run_with_args_and_source(args, llm_source, mcp_servers).await
}
#[allow(
clippy::print_stdout,
clippy::print_stderr,
reason = "Assistant output stays on stdout while prompts and diagnostics use stderr."
)]
pub async fn run_with_args_and_source(
args: AgentArgs,
llm_source: Arc<dyn CredentialSource>,
mcp_servers: Vec<McpServerSettings>,
) -> anyhow::Result<()> {
let provider = parse_provider(&args)?;
let client = Client::from_source(llm_source.as_ref())
.await
.map_err(|e| anyhow::anyhow!("Failed to create LLM client: {e}"))?;
ensure_provider_registered(&client, provider)?;
run_with_args_and_client(args, client, mcp_servers).await
}
#[allow(
@ -412,31 +462,15 @@ pub async fn run_with_args(
)]
pub async fn run_with_args_and_client(
args: AgentArgs,
llm_client: Option<Client>,
mut client: Client,
mcp_servers: Vec<McpServerSettings>,
) -> anyhow::Result<()> {
// Resolve color support once, leak to get 'static lifetime for use across
// threads
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
// Parse provider string to enum early for compile-time safety
let provider_str = args.provider.as_deref().unwrap_or("anthropic");
let provider: Provider = provider_str
.parse()
.map_err(|_| anyhow::anyhow!("unknown provider: {provider_str}"))?;
// Build LLM client — use provided client or create from env
let mut client = if let Some(c) = llm_client {
c
} else {
// Validate provider API key only in standalone mode
if !provider.has_api_key() {
anyhow::bail!("API key not set for provider '{provider}'");
}
Client::from_env()
.await
.map_err(|e| anyhow::anyhow!("Failed to create LLM client: {e}"))?
};
let provider = parse_provider(&args)?;
ensure_provider_registered(&client, provider)?;
if args.verbose {
client.add_middleware(Arc::new(VerboseMiddleware { styles }));
@ -458,7 +492,11 @@ pub async fn run_with_args_and_client(
})?
};
eprintln!("{}", styles.dim.apply_to(format!("Using model: {model}")));
let mut profile = build_profile(provider, &model, Some(client.clone()));
let mut profile = build_profile(
provider,
&model,
Some(build_summarizer(provider, client.clone())),
);
// Build sandbox
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
@ -494,7 +532,7 @@ pub async fn run_with_args_and_client(
let factory_env = Arc::clone(&env);
let factory_hooks = config.tool_hooks.clone();
let factory: SessionFactory = Arc::new(move || {
let child_summarizer = build_summarizer(provider, Some(factory_client.clone()));
let child_summarizer = Some(build_summarizer(provider, factory_client.clone()));
let child_profile: Arc<dyn AgentProfile> = match provider {
Provider::OpenAi => Arc::new(OpenAiProfile::with_summarizer(
&factory_model,

View file

@ -2,6 +2,7 @@ use std::collections::{HashMap, VecDeque};
use std::sync::{Arc, Mutex};
use std::time::SystemTime;
use fabro_auth::CredentialSource;
use fabro_llm::client::Client;
use fabro_llm::error::ProviderErrorKind;
use fabro_llm::generate::StreamAccumulator;
@ -91,6 +92,33 @@ impl Session {
}
}
/// Build a session from a credential source. Resolves the LLM client
/// once at construction and caches it for the session's lifetime.
/// Sessions are bounded (≤ 1 hour); cached client is fine within that
/// window. For longer-lived contexts (workflow runs) hold a source,
/// not a session.
///
/// # Errors
///
/// Returns an error if `Client::from_source` fails (e.g. vault unreachable,
/// OAuth refresh failed).
pub async fn from_source(
source: &dyn CredentialSource,
provider_profile: Arc<dyn AgentProfile>,
sandbox: Arc<dyn Sandbox>,
config: SessionOptions,
subagent_manager: Option<Arc<AsyncMutex<SubAgentManager>>>,
) -> Result<Self, LlmError> {
let client = Client::from_source(source).await?;
Ok(Self::new(
client,
provider_profile,
sandbox,
config,
subagent_manager,
))
}
pub fn set_tool_env(&mut self, env: HashMap<String, String>) {
self.tool_env = Some(env);
}

View file

@ -13,6 +13,7 @@ use fabro_agent::{
AgentProfile, AnthropicProfile, GeminiProfile, LocalSandbox, OpenAiProfile, Session,
SessionOptions, SubAgentManager, WebFetchSummarizer,
};
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::provider::{Provider, ProviderAdapter};
use fabro_llm::providers::OpenAiAdapter;
@ -148,7 +149,10 @@ async fn make_client(provider: Provider, twin: Option<&OpenAiTwinOptions>) -> Cl
return make_twin_client(twin.expect("openai twin config should be provided"));
}
Client::from_env().await.expect("Client::from_env failed")
let source = EnvCredentialSource::new();
Client::from_source(&source)
.await
.expect("Client::from_source failed")
}
fn make_twin_client(twin: &OpenAiTwinOptions) -> Client {

View file

@ -17,6 +17,7 @@ chrono = { workspace = true, features = ["serde"] }
fabro-http.workspace = true
fabro-model = { path = "../fabro-model" }
fabro-oauth = { path = "../fabro-oauth" }
fabro-util = { path = "../fabro-util" }
fabro-vault = { path = "../fabro-vault" }
serde.workspace = true
serde_json.workspace = true

View file

@ -1,5 +1,6 @@
use chrono::{DateTime, Duration, Utc};
use fabro_model::Provider;
use fabro_util::redact::redact_string;
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
@ -57,12 +58,37 @@ pub struct OAuthConfig {
pub use_pkce: bool,
}
#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Clone, PartialEq, Eq)]
pub enum ApiKeyHeader {
Bearer(String),
Custom { name: String, value: String },
}
fn redact_for_debug(value: &str) -> String {
let redacted = redact_string(value);
if redacted == value && !value.is_empty() {
"REDACTED".to_string()
} else {
redacted
}
}
impl std::fmt::Debug for ApiKeyHeader {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Bearer(value) => f
.debug_tuple("Bearer")
.field(&redact_for_debug(value))
.finish(),
Self::Custom { name, value } => f
.debug_struct("Custom")
.field("name", name)
.field("value", &redact_for_debug(value))
.finish(),
}
}
}
pub fn credential_id_for(credential: &AuthCredential) -> Result<String, String> {
match (&credential.provider, &credential.details) {
(Provider::OpenAi, AuthDetails::ApiKey { .. }) => Ok("openai".to_string()),
@ -160,4 +186,14 @@ mod tests {
assert!(parse_credential_secret("openai", &json).is_err());
assert!(parse_credential_secret("openai_codex", "{").is_err());
}
#[test]
fn api_key_header_debug_redacts_secret_values() {
let header = ApiKeyHeader::Bearer("sk-test".to_string());
let debug = format!("{header:?}");
assert!(!debug.contains("sk-test"));
assert!(debug.contains("REDACTED"));
}
}

View file

@ -0,0 +1,17 @@
use async_trait::async_trait;
use fabro_model::Provider;
use crate::{ApiCredential, ResolveError};
#[derive(Debug)]
pub struct ResolvedCredentials {
pub credentials: Vec<ApiCredential>,
pub auth_issues: Vec<(Provider, ResolveError)>,
}
#[async_trait]
pub trait CredentialSource: Send + Sync {
async fn resolve(&self) -> anyhow::Result<ResolvedCredentials>;
async fn configured_providers(&self) -> Vec<Provider>;
}

View file

@ -0,0 +1,166 @@
use std::sync::Arc;
use async_trait::async_trait;
use fabro_model::Provider;
use crate::credential_source::{CredentialSource, ResolvedCredentials};
use crate::{ApiCredential, EnvLookup};
#[derive(Clone)]
pub struct EnvCredentialSource {
env_lookup: EnvLookup,
}
impl EnvCredentialSource {
#[must_use]
pub fn new() -> Self {
Self::with_env_lookup(Arc::new(|name| std::env::var(name).ok()))
}
#[must_use]
pub fn with_env_lookup(env_lookup: EnvLookup) -> Self {
Self { env_lookup }
}
fn lookup(&self, name: &str) -> Option<String> {
(self.env_lookup)(name)
}
fn credential_for(&self, provider: Provider) -> Option<ApiCredential> {
let key = provider
.api_key_env_vars()
.iter()
.find_map(|var| self.lookup(var))?;
let mut cred = ApiCredential::from_api_key(provider, key);
match provider {
Provider::Anthropic => {
cred.base_url = self.lookup("ANTHROPIC_BASE_URL");
}
Provider::OpenAi => {
cred.base_url = self.lookup("OPENAI_BASE_URL");
cred.org_id = self.lookup("OPENAI_ORG_ID");
cred.project_id = self.lookup("OPENAI_PROJECT_ID");
if let Some(account_id) = self.lookup("CHATGPT_ACCOUNT_ID") {
cred.base_url = Some("https://chatgpt.com/backend-api/codex".to_string());
cred.codex_mode = true;
cred.extra_headers
.insert("ChatGPT-Account-Id".to_string(), account_id);
cred.extra_headers
.insert("originator".to_string(), "fabro".to_string());
}
}
Provider::Gemini => {
cred.base_url = self.lookup("GEMINI_BASE_URL");
}
Provider::Kimi | Provider::Zai | Provider::Minimax | Provider::Inception => {}
// OpenAiCompatible has no api_key_env_vars, so find_map returned None above.
Provider::OpenAiCompatible => unreachable!(),
}
Some(cred)
}
}
impl std::fmt::Debug for EnvCredentialSource {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("EnvCredentialSource")
.finish_non_exhaustive()
}
}
impl Default for EnvCredentialSource {
fn default() -> Self {
Self::new()
}
}
#[async_trait]
impl CredentialSource for EnvCredentialSource {
async fn resolve(&self) -> anyhow::Result<ResolvedCredentials> {
let credentials = Provider::ALL
.iter()
.copied()
.filter_map(|provider| self.credential_for(provider))
.collect();
Ok(ResolvedCredentials {
credentials,
auth_issues: Vec::new(),
})
}
async fn configured_providers(&self) -> Vec<Provider> {
Provider::ALL
.iter()
.copied()
.filter(|provider| {
provider
.api_key_env_vars()
.iter()
.any(|env_var| self.lookup(env_var).is_some())
})
.collect()
}
}
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::sync::Arc;
use fabro_model::Provider;
use super::EnvCredentialSource;
use crate::CredentialSource;
fn test_source(entries: &[(&str, &str)]) -> EnvCredentialSource {
let entries: HashMap<String, String> = entries
.iter()
.map(|(key, value)| ((*key).to_string(), (*value).to_string()))
.collect();
EnvCredentialSource::with_env_lookup(Arc::new(move |name| entries.get(name).cloned()))
}
#[tokio::test]
async fn configured_providers_reads_injected_env() {
let source = test_source(&[("ANTHROPIC_API_KEY", "anthropic-key")]);
assert_eq!(source.configured_providers().await, vec![
Provider::Anthropic
]);
}
#[tokio::test]
async fn resolve_returns_empty_when_no_keys_are_configured() {
let source = test_source(&[]);
let resolved = source.resolve().await.unwrap();
assert!(resolved.credentials.is_empty());
assert!(resolved.auth_issues.is_empty());
}
#[tokio::test]
async fn resolve_builds_openai_codex_env_credential() {
let source = test_source(&[
("OPENAI_API_KEY", "openai-key"),
("CHATGPT_ACCOUNT_ID", "acct_123"),
("OPENAI_PROJECT_ID", "project_123"),
]);
let resolved = source.resolve().await.unwrap();
let credential = resolved.credentials.first().unwrap();
assert_eq!(credential.provider, Provider::OpenAi);
assert!(credential.codex_mode);
assert_eq!(
credential.base_url.as_deref(),
Some("https://chatgpt.com/backend-api/codex")
);
assert_eq!(
credential.extra_headers.get("ChatGPT-Account-Id"),
Some(&"acct_123".to_string())
);
assert_eq!(credential.project_id.as_deref(), Some("project_123"));
}
}

View file

@ -1,9 +1,12 @@
mod context;
mod credential;
mod credential_source;
mod env_source;
mod refresh;
mod resolve;
mod strategy;
mod vault_ext;
mod vault_source;
pub mod strategies;
@ -12,13 +15,16 @@ pub use credential::{
ApiKeyHeader, AuthCredential, AuthDetails, OAuthConfig, OAuthTokens, credential_id_for,
parse_credential_secret,
};
pub use credential_source::{CredentialSource, ResolvedCredentials};
pub use env_source::EnvCredentialSource;
pub use refresh::refresh_oauth_credential;
pub use resolve::{
ApiCredential, CliAgentKind, CliCredential, CredentialResolver, CredentialUsage, EnvLookup,
ResolveError, ResolvedCredential, configured_providers_from_process_env,
ResolveError, ResolvedCredential, auth_issue_message, configured_providers_from_process_env,
};
pub use strategy::{
AuthMethod, AuthStrategy, CODEX_AUTH_URL, CODEX_CLIENT_ID, CODEX_TOKEN_URL, codex_oauth_config,
strategy_for,
};
pub use vault_ext::{vault_credentials_for_provider, vault_get_credential, vault_set_credential};
pub use vault_source::VaultCredentialSource;

View file

@ -37,6 +37,32 @@ pub struct ApiCredential {
pub project_id: Option<String>,
}
impl ApiCredential {
/// Build an `ApiCredential` from just an API key. Picks the right
/// auth header kind for the provider (Anthropic uses `x-api-key`;
/// everyone else uses `Authorization: Bearer`). All other fields
/// default to empty.
#[must_use]
pub fn from_api_key(provider: Provider, key: String) -> Self {
let auth_header = match provider {
Provider::Anthropic => ApiKeyHeader::Custom {
name: "x-api-key".to_string(),
value: key,
},
_ => ApiKeyHeader::Bearer(key),
};
Self {
provider,
auth_header,
extra_headers: HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
}
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CliCredential {
pub env_vars: HashMap<String, String>,
@ -63,6 +89,24 @@ pub enum ResolveError {
RefreshTokenMissing(Provider),
}
#[must_use]
pub fn auth_issue_message(provider: Provider, err: &ResolveError) -> String {
match err {
ResolveError::NotConfigured(_) => {
format!("{} is not configured", provider.display_name())
}
ResolveError::RefreshFailed { source, .. } => format!(
"{} requires re-authentication: {}",
provider.display_name(),
source
),
ResolveError::RefreshTokenMissing(_) => format!(
"{} requires re-authentication: refresh token missing",
provider.display_name()
),
}
}
#[derive(Clone)]
pub struct CredentialResolver {
vault: Arc<AsyncRwLock<Vault>>,
@ -184,7 +228,6 @@ impl CredentialResolver {
}
fn to_api_credential(&self, vault: &Vault, credential: &AuthCredential) -> ApiCredential {
let mut extra_headers = HashMap::new();
let base_url = match credential.provider {
Provider::Anthropic => self.lookup_env_or_vault(vault, "ANTHROPIC_BASE_URL"),
Provider::OpenAi => self.lookup_env_or_vault(vault, "OPENAI_BASE_URL"),
@ -195,32 +238,19 @@ impl CredentialResolver {
}
};
match &credential.details {
AuthDetails::ApiKey { key } => ApiCredential {
provider: credential.provider,
auth_header: match credential.provider {
Provider::Anthropic => ApiKeyHeader::Custom {
name: "x-api-key".to_string(),
value: key.clone(),
},
_ => ApiKeyHeader::Bearer(key.clone()),
},
extra_headers,
base_url,
codex_mode: false,
org_id: if credential.provider == Provider::OpenAi {
self.lookup_env_or_vault(vault, "OPENAI_ORG_ID")
} else {
None
},
project_id: if credential.provider == Provider::OpenAi {
self.lookup_env_or_vault(vault, "OPENAI_PROJECT_ID")
} else {
None
},
},
AuthDetails::ApiKey { key } => {
let mut cred = ApiCredential::from_api_key(credential.provider, key.clone());
cred.base_url = base_url;
if credential.provider == Provider::OpenAi {
cred.org_id = self.lookup_env_or_vault(vault, "OPENAI_ORG_ID");
cred.project_id = self.lookup_env_or_vault(vault, "OPENAI_PROJECT_ID");
}
cred
}
AuthDetails::CodexOAuth {
tokens, account_id, ..
} => {
let mut extra_headers = HashMap::new();
if let Some(account_id) = account_id {
extra_headers.insert("ChatGPT-Account-Id".to_string(), account_id.clone());
extra_headers.insert("originator".to_string(), "fabro".to_string());
@ -816,4 +846,35 @@ mod tests {
ResolveError::RefreshTokenMissing(Provider::OpenAi)
));
}
#[test]
fn auth_issue_message_formats_refresh_token_missing() {
let message = auth_issue_message(
Provider::OpenAi,
&ResolveError::RefreshTokenMissing(Provider::OpenAi),
);
assert_eq!(
message,
"OpenAI requires re-authentication: refresh token missing"
);
}
#[test]
fn api_credential_debug_redacts_secret_material() {
let credential = ApiCredential {
provider: Provider::OpenAi,
auth_header: ApiKeyHeader::Bearer("sk-test".to_string()),
extra_headers: HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
};
let debug = format!("{credential:?}");
assert!(!debug.contains("sk-test"));
assert!(debug.contains("REDACTED"));
}
}

View file

@ -0,0 +1,185 @@
use std::sync::Arc;
use async_trait::async_trait;
use fabro_model::Provider;
use fabro_vault::Vault;
use tokio::sync::RwLock as AsyncRwLock;
use crate::credential_source::{CredentialSource, ResolvedCredentials};
use crate::{CredentialResolver, CredentialUsage, EnvLookup, ResolveError, ResolvedCredential};
#[derive(Clone)]
pub struct VaultCredentialSource {
vault: Arc<AsyncRwLock<Vault>>,
resolver: CredentialResolver,
}
impl VaultCredentialSource {
#[must_use]
pub fn new(vault: Arc<AsyncRwLock<Vault>>) -> Self {
let resolver = CredentialResolver::new(Arc::clone(&vault));
Self { vault, resolver }
}
#[must_use]
pub fn with_env_lookup<F>(vault: Arc<AsyncRwLock<Vault>>, env_lookup: F) -> Self
where
F: Fn(&str) -> Option<String> + Send + Sync + 'static,
{
let env_lookup: EnvLookup = Arc::new(env_lookup);
let resolver = CredentialResolver::with_env_lookup(Arc::clone(&vault), env_lookup);
Self { vault, resolver }
}
}
impl std::fmt::Debug for VaultCredentialSource {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("VaultCredentialSource")
.finish_non_exhaustive()
}
}
#[async_trait]
impl CredentialSource for VaultCredentialSource {
async fn resolve(&self) -> anyhow::Result<ResolvedCredentials> {
let mut credentials = Vec::new();
let mut auth_issues = Vec::new();
for provider in Provider::ALL {
match self
.resolver
.resolve(*provider, CredentialUsage::ApiRequest)
.await
{
Ok(ResolvedCredential::Api(credential)) => credentials.push(credential),
Ok(ResolvedCredential::Cli(_)) | Err(ResolveError::NotConfigured(_)) => {}
Err(err) => auth_issues.push((*provider, err)),
}
}
Ok(ResolvedCredentials {
credentials,
auth_issues,
})
}
async fn configured_providers(&self) -> Vec<Provider> {
let vault = self.vault.read().await;
self.resolver.configured_providers(&vault)
}
}
#[cfg(test)]
mod tests {
use std::sync::Arc;
use chrono::{Duration, Utc};
use fabro_model::Provider;
use fabro_vault::{SecretType, Vault};
use tokio::sync::RwLock as AsyncRwLock;
use super::VaultCredentialSource;
use crate::credential::{AuthCredential, AuthDetails, OAuthConfig, OAuthTokens};
use crate::{CredentialSource, ResolveError};
fn api_key_credential(provider: Provider, key: &str) -> AuthCredential {
AuthCredential {
provider,
details: AuthDetails::ApiKey {
key: key.to_string(),
},
}
}
fn expired_openai_credential() -> AuthCredential {
AuthCredential {
provider: Provider::OpenAi,
details: AuthDetails::CodexOAuth {
tokens: OAuthTokens {
access_token: "expired-access".to_string(),
refresh_token: Some("refresh-token".to_string()),
expires_at: Utc::now() - Duration::hours(1),
},
config: OAuthConfig {
auth_url: "https://auth.openai.com".to_string(),
token_url: "http://127.0.0.1:9/oauth/token".to_string(),
client_id: "client".to_string(),
scopes: vec!["openid".to_string()],
redirect_uri: Some("https://example.com/callback".to_string()),
use_pkce: true,
},
account_id: Some("acct_123".to_string()),
},
}
}
#[tokio::test]
async fn resolve_returns_credentials_and_auth_issues() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai_codex",
&serde_json::to_string(&expired_openai_credential()).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
vault
.set(
"anthropic",
&serde_json::to_string(&api_key_credential(Provider::Anthropic, "anthropic-key"))
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let source =
VaultCredentialSource::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |_| None);
let resolved = source.resolve().await.unwrap();
assert_eq!(resolved.credentials.len(), 1);
assert_eq!(resolved.credentials[0].provider, Provider::Anthropic);
assert_eq!(resolved.auth_issues.len(), 1);
assert!(matches!(
resolved.auth_issues[0].1,
ResolveError::RefreshFailed {
provider: Provider::OpenAi,
..
}
));
}
#[tokio::test]
async fn configured_providers_reads_from_vault_without_refreshing() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai",
&serde_json::to_string(&api_key_credential(Provider::OpenAi, "openai-key"))
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
vault
.set(
"anthropic",
&serde_json::to_string(&api_key_credential(Provider::Anthropic, "anthropic-key"))
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let source =
VaultCredentialSource::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |_| None);
assert_eq!(source.configured_providers().await, vec![
Provider::Anthropic,
Provider::OpenAi
]);
}
}

View file

@ -2,12 +2,14 @@ use std::path::{Path, PathBuf};
use std::sync::Arc;
use anyhow::{Context as _, Result, bail};
use fabro_config::CliLayer;
use fabro_auth::{CredentialSource, EnvCredentialSource, VaultCredentialSource};
use fabro_config::{CliLayer, Storage};
use fabro_types::settings::RunNamespace;
use fabro_types::settings::cli::{OutputFormat, OutputVerbosity};
use fabro_types::{ServerSettings, UserSettings};
use fabro_util::printer::Printer;
use tokio::sync::OnceCell;
use fabro_vault::Vault;
use tokio::sync::{OnceCell, RwLock as AsyncRwLock};
use crate::args::{
ServerConnectionArgs, ServerTargetArgs, printer_from_verbosity, require_no_json_override,
@ -40,6 +42,7 @@ pub(crate) struct CommandContext {
user_settings: UserSettings,
server_mode: ServerMode,
server: OnceCell<Arc<Client>>,
llm_source: OnceCell<Arc<dyn CredentialSource>>,
}
struct ResolvedCommandSettings {
@ -68,6 +71,7 @@ impl CommandContext {
user_settings: resolved_settings.user_settings,
server_mode: ServerMode::None,
server: OnceCell::new(),
llm_source: OnceCell::new(),
})
}
@ -160,6 +164,26 @@ impl CommandContext {
Ok(Arc::clone(client))
}
pub(crate) async fn llm_source(&self) -> Result<Arc<dyn CredentialSource>> {
let storage_dir = self.storage_dir.clone();
let source = self
.llm_source
.get_or_try_init(|| async move {
let source: Arc<dyn CredentialSource> =
match Vault::load(Storage::new(&storage_dir).secrets_path()) {
Ok(vault) => Arc::new(VaultCredentialSource::new(Arc::new(
AsyncRwLock::new(vault),
))),
Err(_) => Arc::new(EnvCredentialSource::new()),
};
Ok::<Arc<dyn CredentialSource>, anyhow::Error>(source)
})
.await?;
Ok(Arc::clone(source))
}
fn with_server_mode(&self, server_mode: ServerMode) -> Result<Self> {
// Always reload settings for the requested derivation mode so the result
// depends only on the requested mode, not on whichever derived context
@ -178,6 +202,7 @@ impl CommandContext {
user_settings: resolved_settings.user_settings,
server_mode,
server: OnceCell::new(),
llm_source: OnceCell::new(),
})
}
}
@ -251,6 +276,7 @@ mod tests {
user_settings: resolved_settings.user_settings,
server_mode: ServerMode::None,
server: OnceCell::new(),
llm_source: OnceCell::new(),
}
}

View file

@ -2,7 +2,7 @@ use std::collections::HashMap;
use std::sync::Arc;
use anyhow::Result as AnyResult;
use fabro_agent::cli::{OutputFormat, run_with_args, run_with_args_and_client};
use fabro_agent::cli::{OutputFormat, run_with_args_and_client, run_with_args_and_source};
use fabro_llm::client::Client;
use fabro_llm::error::{
Error as LlmError, ProviderErrorDetail, ProviderErrorKind, error_from_status_code,
@ -330,12 +330,13 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu
.register_provider(adapter)
.await
.map_err(|e| anyhow::anyhow!("Failed to register fabro server adapter: {e}"))?;
run_with_args_and_client(args.agent, Some(client), mcp_servers)
run_with_args_and_client(args.agent, client, mcp_servers)
.await
.map_err(classify_server_agent_auth)?;
} else {
tracing::info!(transport = "direct", "Agent session starting");
run_with_args(args.agent, mcp_servers).await?;
let llm_source = ctx.llm_source().await?;
run_with_args_and_source(args.agent, llm_source, mcp_servers).await?;
}
Ok(())

View file

@ -1,14 +1,8 @@
use std::sync::Arc;
use anyhow::{Context, Result, bail};
use fabro_auth::configured_providers_from_process_env;
use fabro_config::Storage;
use fabro_model::Catalog;
use fabro_sandbox::daytona::detect_repo_info;
use fabro_vault::Vault;
use fabro_workflow::outcome::StageStatus;
use fabro_workflow::pull_request::maybe_open_pull_request;
use tokio::sync::RwLock as AsyncRwLock;
use tracing::info;
use crate::args::PrCreateArgs;
@ -97,17 +91,15 @@ pub(super) async fn create_command(args: PrCreateArgs, base_ctx: &CommandContext
);
}
let vault = Vault::load(Storage::new(ctx.storage_dir()).secrets_path())
.ok()
.map(|vault| Arc::new(AsyncRwLock::new(vault)));
let configured = configured_providers_from_process_env(vault.as_ref()).await;
let llm_source = ctx.llm_source().await?;
let configured = llm_source.configured_providers().await;
let model = args.model.unwrap_or_else(|| {
Catalog::builtin()
.default_for_configured(&configured)
.id
.clone()
});
let run_store_handle = run_store.clone().into();
let pull_request = maybe_open_pull_request(
&creds,
&origin_url,
@ -118,7 +110,8 @@ pub(super) async fn create_command(args: PrCreateArgs, base_ctx: &CommandContext
&model,
true,
None,
&run_store.clone().into(),
&run_store_handle,
llm_source.as_ref(),
None,
)
.await

View file

@ -15,8 +15,8 @@ use dialoguer::console::Term;
use dialoguer::theme::ColorfulTheme;
use dialoguer::{Confirm, Password};
use fabro_auth::{
ApiCredential, ApiKeyHeader, AuthContextRequest, AuthContextResponse, AuthCredential,
AuthMethod, codex_oauth_config, strategy_for,
ApiCredential, AuthContextRequest, AuthContextResponse, AuthCredential, AuthMethod,
codex_oauth_config, strategy_for,
};
use fabro_llm::client::Client as LlmClient;
use fabro_llm::generate::{GenerateParams, generate};
@ -78,23 +78,10 @@ pub(crate) enum ApiKeySource {
// ---------------------------------------------------------------------------
pub(crate) async fn validate_api_key(provider: Provider, api_key: &str) -> Result<(), String> {
let auth_header = if provider == Provider::Anthropic {
ApiKeyHeader::Custom {
name: "x-api-key".to_string(),
value: api_key.to_string(),
}
} else {
ApiKeyHeader::Bearer(api_key.to_string())
};
let client = LlmClient::from_credentials(vec![ApiCredential {
let client = LlmClient::from_credentials(vec![ApiCredential::from_api_key(
provider,
auth_header,
extra_headers: std::collections::HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
}])
api_key.to_string(),
)])
.await
.map_err(|e| e.to_string())?;
@ -103,11 +90,10 @@ pub(crate) async fn validate_api_key(provider: Provider, api_key: &str) -> Resul
|model| model.id.clone(),
);
let params = GenerateParams::new(probe_model)
let params = GenerateParams::new(probe_model, Arc::new(client))
.provider(provider.as_str())
.prompt("Say OK")
.max_tokens(16)
.client(Arc::new(client));
.max_tokens(16);
timeout(std::time::Duration::from_secs(30), generate(params))
.await

View file

@ -103,7 +103,7 @@ fn exec_missing_api_key_exits_with_error() {
exit_code: 1
----- stdout -----
----- stderr -----
error: API key not set for provider 'anthropic'
error: LLM credentials not configured for provider 'anthropic'
");
}
@ -129,7 +129,7 @@ fn exec_uses_user_config_defaults() {
exit_code: 1
----- stdout -----
----- stderr -----
error: API key not set for provider 'openai'
error: LLM credentials not configured for provider 'openai'
");
}
@ -205,8 +205,8 @@ fn exec_configured_server_target_alone_does_not_reroute_exec() {
let output = cmd.assert().failure().get_output().clone();
let stderr = String::from_utf8(output.stderr).expect("valid utf8");
assert!(
stderr.contains("API key not set for provider 'openai'"),
"expected local API key validation failure, got: {stderr}"
stderr.contains("LLM credentials not configured for provider 'openai'"),
"expected local credential resolution failure, got: {stderr}"
);
assert!(
!stderr.contains("config-should-not-be-used"),

View file

@ -14,6 +14,7 @@ workspace = true
[dependencies]
fabro-agent = { path = "../fabro-agent" }
fabro-auth = { path = "../fabro-auth" }
fabro-config = { path = "../fabro-config" }
fabro-llm = { path = "../fabro-llm" }
fabro-model = { path = "../fabro-model" }

View file

@ -95,6 +95,7 @@ mod tests {
context: &HookContext,
_sandbox: Arc<dyn Sandbox>,
_work_dir: Option<&Path>,
_llm_source: &dyn fabro_auth::CredentialSource,
) -> HookResult {
self.captured_contexts.lock().unwrap().push(context.clone());
HookResult {

View file

@ -8,6 +8,7 @@ use std::time::Instant;
use async_trait::async_trait;
use fabro_agent::Sandbox;
use fabro_agent::tool_registry::ToolContext;
use fabro_auth::CredentialSource;
use fabro_llm::client::Client as LlmClient;
use fabro_llm::generate::{GenerateParams, generate_object};
use fabro_llm::types::{Message, Request, ToolResult};
@ -48,6 +49,7 @@ pub trait HookExecutor: Send + Sync {
context: &HookContext,
sandbox: Arc<dyn Sandbox>,
work_dir: Option<&Path>,
llm_source: &dyn CredentialSource,
) -> HookResult;
}
@ -288,6 +290,7 @@ impl HookExecutorImpl {
model: Option<&str>,
context: &HookContext,
env: &E,
llm_source: &dyn CredentialSource,
) -> HookDecision
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
@ -301,7 +304,15 @@ impl HookExecutorImpl {
let user_msg = Self::build_hook_user_message(&prompt, context);
Self::execute_llm_with_timeout(definition.timeout(), "prompt", || async move {
let params = GenerateParams::new(&resolved_model)
let client = match LlmClient::from_source(llm_source).await {
Ok(client) => Arc::new(client),
Err(e) => {
tracing::warn!(error = %e, "prompt hook client creation failed, proceeding");
return HookDecision::Proceed;
}
};
let params = GenerateParams::new(&resolved_model, client)
.system(HOOK_EVALUATOR_SYSTEM_PROMPT)
.prompt(user_msg)
.max_tokens(1024);
@ -342,6 +353,7 @@ impl HookExecutorImpl {
context: &HookContext,
sandbox: Arc<dyn Sandbox>,
env: &E,
llm_source: &dyn CredentialSource,
) -> HookDecision
where
E: Env + Clone + Send + Sync + fmt::Debug + 'static,
@ -355,7 +367,7 @@ impl HookExecutorImpl {
let user_msg = Self::build_hook_user_message(&prompt, context);
Self::execute_llm_with_timeout(definition.timeout(), "agent", || async move {
let client = match LlmClient::from_env().await {
let client = match LlmClient::from_source(llm_source).await {
Ok(c) => c,
Err(e) => {
tracing::warn!(error = %e, "agent hook client creation failed, proceeding");
@ -604,6 +616,7 @@ impl HookExecutor for HookExecutorImpl {
context: &HookContext,
sandbox: Arc<dyn Sandbox>,
work_dir: Option<&Path>,
llm_source: &dyn CredentialSource,
) -> HookResult {
use std::sync::OnceLock;
static HTTP_CLIENTS: OnceLock<HttpClientCache> = OnceLock::new();
@ -654,7 +667,17 @@ impl HookExecutor for HookExecutorImpl {
ref prompt,
ref model,
}),
) => Self::execute_prompt(definition, prompt, model.as_deref(), context, &env).await,
) => {
Self::execute_prompt(
definition,
prompt,
model.as_deref(),
context,
&env,
llm_source,
)
.await
}
Some(
Cow::Borrowed(HookType::Agent {
ref prompt,
@ -675,6 +698,7 @@ impl HookExecutor for HookExecutorImpl {
context,
sandbox,
&env,
llm_source,
)
.await
}
@ -694,6 +718,7 @@ impl HookExecutor for HookExecutorImpl {
#[cfg(test)]
mod tests {
use fabro_auth::{CredentialSource, EnvCredentialSource};
use fabro_types::fixtures;
use fabro_util::env::TestEnv;
@ -711,6 +736,10 @@ mod tests {
))
}
fn test_llm_source() -> Arc<dyn CredentialSource> {
Arc::new(EnvCredentialSource::new())
}
fn test_http_client() -> fabro_http::HttpClient {
HookExecutorImpl::build_http_client(TlsMode::Off)
}
@ -791,7 +820,10 @@ mod tests {
let def = make_definition("exit 0");
let ctx = make_context();
let sandbox = make_sandbox();
let result = executor.execute(&def, &ctx, sandbox, None).await;
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
assert_eq!(result.decision, HookDecision::Proceed);
assert_eq!(result.hook_name.as_deref(), Some("test-hook"));
}
@ -802,7 +834,10 @@ mod tests {
let def = make_definition("exit 1");
let ctx = make_context();
let sandbox = make_sandbox();
let result = executor.execute(&def, &ctx, sandbox, None).await;
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
assert!(matches!(result.decision, HookDecision::Block { .. }));
}
@ -812,7 +847,10 @@ mod tests {
let def = make_definition("exit 2");
let ctx = make_context();
let sandbox = make_sandbox();
let result = executor.execute(&def, &ctx, sandbox, None).await;
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
assert!(matches!(result.decision, HookDecision::Block { .. }));
}
@ -822,7 +860,10 @@ mod tests {
let def = make_definition(r#"echo '{"decision": "skip", "reason": "test skip"}'"#);
let ctx = make_context();
let sandbox = make_sandbox();
let result = executor.execute(&def, &ctx, sandbox, None).await;
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
assert_eq!(result.decision, HookDecision::Skip {
reason: Some("test skip".into()),
});
@ -836,7 +877,10 @@ mod tests {
let mut ctx = make_context();
ctx.node_id = Some("plan".into());
let sandbox = make_sandbox();
let result = executor.execute(&def, &ctx, sandbox, None).await;
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
assert_eq!(result.decision, HookDecision::Proceed);
}
@ -855,7 +899,10 @@ mod tests {
};
let ctx = make_context();
let sandbox = make_sandbox();
let result = executor.execute(&def, &ctx, sandbox, None).await;
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
assert!(matches!(result.decision, HookDecision::Block { .. }));
}
@ -1247,7 +1294,10 @@ mod tests {
};
let ctx = make_context();
let sandbox = make_sandbox();
let result = executor.execute(&def, &ctx, sandbox, None).await;
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
mock.assert_async().await;
assert_eq!(result.decision, HookDecision::Proceed);
@ -1278,6 +1328,7 @@ mod tests {
None,
&make_context(),
&test_env(&[]),
test_llm_source().as_ref(),
)
.await;
@ -1294,6 +1345,7 @@ mod tests {
&make_context(),
make_sandbox(),
&test_env(&[]),
test_llm_source().as_ref(),
)
.await;

View file

@ -3,6 +3,9 @@ use std::path::Path;
use std::sync::Arc;
use fabro_agent::Sandbox;
use fabro_auth::CredentialSource;
#[cfg(test)]
use fabro_auth::EnvCredentialSource;
use crate::config::{HookDefinition, HookSettings};
use crate::executor::{HookExecutor, HookExecutorImpl};
@ -13,17 +16,19 @@ use crate::types::{HookContext, HookDecision};
pub struct HookRunner {
config: HookSettings,
executor: Arc<dyn HookExecutor>,
llm_source: Arc<dyn CredentialSource>,
/// Pre-compiled regexes keyed by matcher pattern string.
compiled_matchers: HashMap<String, regex::Regex>,
}
impl HookRunner {
#[must_use]
pub fn new(config: HookSettings) -> Self {
pub fn new(config: HookSettings, llm_source: Arc<dyn CredentialSource>) -> Self {
let compiled_matchers = Self::compile_matchers(&config);
Self {
config,
executor: Arc::new(HookExecutorImpl),
llm_source,
compiled_matchers,
}
}
@ -35,6 +40,7 @@ impl HookRunner {
Self {
config,
executor,
llm_source: Arc::new(EnvCredentialSource::new()),
compiled_matchers,
}
}
@ -140,7 +146,13 @@ impl HookRunner {
);
let result = self
.executor
.execute(hook, context, sandbox.clone(), work_dir)
.execute(
hook,
context,
sandbox.clone(),
work_dir,
self.llm_source.as_ref(),
)
.await;
tracing::debug!(
hook = %hook.effective_name(),
@ -188,7 +200,13 @@ impl HookRunner {
);
let result = self
.executor
.execute(hook, context, sandbox.clone(), work_dir)
.execute(
hook,
context,
sandbox.clone(),
work_dir,
self.llm_source.as_ref(),
)
.await;
tracing::debug!(
hook = %hook.effective_name(),
@ -211,6 +229,7 @@ impl HookRunner {
#[cfg(test)]
mod tests {
use fabro_auth::EnvCredentialSource;
use fabro_types::fixtures;
use super::*;
@ -229,6 +248,7 @@ mod tests {
_context: &HookContext,
_sandbox: Arc<dyn Sandbox>,
_work_dir: Option<&Path>,
_llm_source: &dyn CredentialSource,
) -> HookResult {
HookResult {
hook_name: definition.name.clone(),
@ -248,6 +268,10 @@ mod tests {
HookContext::new(event, fixtures::RUN_1, "test-wf".into())
}
fn test_llm_source() -> Arc<dyn CredentialSource> {
Arc::new(EnvCredentialSource::new())
}
fn make_hook(event: HookEvent, name: &str) -> HookDefinition {
HookDefinition {
name: Some(name.into()),
@ -263,7 +287,7 @@ mod tests {
#[tokio::test]
async fn no_hooks_returns_proceed() {
let runner = HookRunner::new(HookSettings::default());
let runner = HookRunner::new(HookSettings::default(), test_llm_source());
let ctx = make_context(HookEvent::RunStart);
let sandbox = make_sandbox();
let decision = runner.run(&ctx, sandbox.clone(), None).await;
@ -428,7 +452,7 @@ mod tests {
h
}],
};
let runner = HookRunner::new(config);
let runner = HookRunner::new(config, test_llm_source());
let ctx = make_context(HookEvent::RunStart);
let sandbox = make_sandbox();
let decision = runner.run(&ctx, sandbox.clone(), None).await;
@ -444,7 +468,7 @@ mod tests {
h
}],
};
let runner = HookRunner::new(config);
let runner = HookRunner::new(config, test_llm_source());
let ctx = make_context(HookEvent::RunStart);
let sandbox = make_sandbox();
let decision = runner.run(&ctx, sandbox.clone(), None).await;

View file

@ -1,10 +1,10 @@
# unified-llm
# fabro-llm
A unified async Rust client library for multiple LLM providers. Write your LLM integration code once and switch between Anthropic, OpenAI, and Google Gemini without changing your application logic.
## Key concepts
- **Client** -- Routes requests to registered provider adapters. Can be created explicitly or auto-configured from environment variables.
- **Client** -- Routes requests to registered provider adapters. Build it from a `CredentialSource` or explicit typed credentials.
- **ProviderAdapter** -- The trait every provider implements (`complete` and `stream`). Built-in adapters: `AnthropicAdapter`, `OpenAiAdapter`, `GeminiAdapter`, `OpenAiCompatibleAdapter`.
- **Middleware** -- Intercepts requests/responses for logging, caching, or transformation. Supports both blocking and streaming paths.
- **generate()** -- High-level function that wraps `Client.complete()` with automatic tool execution loops, retries, timeouts, and cancellation.
@ -24,13 +24,15 @@ All adapters support streaming, tool calling, structured output (`response_forma
## Usage
### Auto-configure from environment
### Create from an environment-backed credential source
```rust
use unified_llm::client::Client;
use unified_llm::types::{Message, Request};
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::types::{Message, Request};
let client = Client::from_env().await?;
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?;
let request = Request {
model: "claude-sonnet-4-5".to_string(),
@ -55,10 +57,14 @@ println!("{}", response.text());
### High-level generate()
```rust
use unified_llm::generate::{generate, GenerateParams};
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::generate::{generate, GenerateParams};
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?;
let result = generate(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("Explain monads in one sentence")
.system("You are a concise programming tutor.")
.max_tokens(200)
@ -70,10 +76,14 @@ println!("{}", result.text());
### Tool calling
```rust
use unified_llm::generate::{generate, GenerateParams};
use unified_llm::tools::Tool;
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::generate::{generate, GenerateParams};
use fabro_llm::tools::Tool;
use std::sync::Arc;
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?;
let weather_tool = Tool::active(
"get_weather",
"Get the current weather for a city",
@ -91,7 +101,7 @@ let weather_tool = Tool::active(
);
let result = generate(
GenerateParams::new("claude-sonnet-4-5")
GenerateParams::new("claude-sonnet-4-5", client.clone())
.prompt("What's the weather in San Francisco?")
.tools(vec![weather_tool])
.max_tool_rounds(3)
@ -101,11 +111,13 @@ let result = generate(
### Streaming
```rust
use unified_llm::client::Client;
use unified_llm::types::{Message, Request, StreamEvent};
use fabro_auth::EnvCredentialSource;
use fabro_llm::client::Client;
use fabro_llm::types::{Message, Request, StreamEvent};
use futures::StreamExt;
let client = Client::from_env().await?;
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?;
let request = Request {
model: "claude-sonnet-4-5".to_string(),
messages: vec![Message::user("Tell me a joke")],
@ -131,10 +143,10 @@ while let Some(event) = stream.next().await {
### Middleware
```rust
use unified_llm::middleware::{Middleware, NextFn, NextStreamFn};
use unified_llm::types::{Request, Response};
use unified_llm::provider::StreamEventStream;
use unified_llm::error::SdkError;
use fabro_llm::error::Error;
use fabro_llm::middleware::{Middleware, NextFn, NextStreamFn};
use fabro_llm::provider::StreamEventStream;
use fabro_llm::types::{Request, Response};
struct LoggingMiddleware;
@ -144,7 +156,7 @@ impl Middleware for LoggingMiddleware {
&self,
request: Request,
next: NextFn,
) -> Result<Response, SdkError> {
) -> Result<Response, Error> {
eprintln!("Request to model: {}", request.model);
let response = next(request).await?;
eprintln!("Response tokens: {}", response.usage.total_tokens);
@ -155,7 +167,7 @@ impl Middleware for LoggingMiddleware {
&self,
request: Request,
next: NextStreamFn,
) -> Result<StreamEventStream, SdkError> {
) -> Result<StreamEventStream, Error> {
next(request).await
}
}
@ -164,7 +176,7 @@ impl Middleware for LoggingMiddleware {
### OpenAI-compatible providers
```rust
use unified_llm::providers::OpenAiCompatibleAdapter;
use fabro_llm::providers::OpenAiCompatibleAdapter;
use std::sync::Arc;
let adapter = OpenAiCompatibleAdapter::new("your-api-key", "https://api.groq.com/openai/v1")
@ -174,7 +186,7 @@ let adapter = OpenAiCompatibleAdapter::new("your-api-key", "https://api.groq.com
### Model catalog
```rust
use unified_llm::catalog::{get_model_info, list_models, get_latest_model};
use fabro_llm::catalog::{get_latest_model, get_model_info, list_models};
let info = get_model_info("claude-opus-4-6");
let anthropic_models = list_models(Some("anthropic"));
@ -213,7 +225,7 @@ The `retry()` function and `generate()` respect `Retry-After` headers and use ex
Pass provider-specific parameters via `provider_options` without losing portability:
```rust
use unified_llm::types::Request;
use fabro_llm::types::Request;
let request = Request {
provider_options: Some(serde_json::json!({

View file

@ -1,7 +1,7 @@
use std::collections::HashMap;
use std::sync::Arc;
use fabro_auth::{ApiCredential, ApiKeyHeader};
use fabro_auth::{ApiCredential, ApiKeyHeader, CredentialSource};
use tracing::debug;
use crate::error::Error;
@ -38,107 +38,18 @@ impl Client {
}
}
/// Create a Client from environment variables (Section 2.2).
/// Registers providers whose API keys are present in the environment.
/// The first registered provider becomes the default.
/// Create a Client from a credential source.
///
/// # Errors
///
/// Returns `Error` if any provider adapter fails to initialize.
pub async fn from_env() -> Result<Self, Error> {
let mut credentials = Vec::new();
if let Ok(key) = std::env::var("ANTHROPIC_API_KEY") {
credentials.push(ApiCredential {
provider: fabro_model::Provider::Anthropic,
auth_header: ApiKeyHeader::Custom {
name: "x-api-key".to_string(),
value: key,
},
extra_headers: HashMap::new(),
base_url: std::env::var("ANTHROPIC_BASE_URL").ok(),
codex_mode: false,
org_id: None,
project_id: None,
});
}
if let Ok(key) = std::env::var("OPENAI_API_KEY") {
let mut extra_headers = HashMap::new();
let mut base_url = std::env::var("OPENAI_BASE_URL").ok();
let mut codex_mode = false;
if let Ok(account_id) = std::env::var("CHATGPT_ACCOUNT_ID") {
base_url = Some("https://chatgpt.com/backend-api/codex".to_string());
codex_mode = true;
extra_headers.insert("ChatGPT-Account-Id".to_string(), account_id);
extra_headers.insert("originator".to_string(), "fabro".to_string());
}
credentials.push(ApiCredential {
provider: fabro_model::Provider::OpenAi,
auth_header: ApiKeyHeader::Bearer(key),
extra_headers,
base_url,
codex_mode,
org_id: std::env::var("OPENAI_ORG_ID").ok(),
project_id: std::env::var("OPENAI_PROJECT_ID").ok(),
});
}
if let Ok(key) =
std::env::var("GEMINI_API_KEY").or_else(|_| std::env::var("GOOGLE_API_KEY"))
{
credentials.push(ApiCredential {
provider: fabro_model::Provider::Gemini,
auth_header: ApiKeyHeader::Bearer(key),
extra_headers: HashMap::new(),
base_url: std::env::var("GEMINI_BASE_URL").ok(),
codex_mode: false,
org_id: None,
project_id: None,
});
}
if let Ok(key) = std::env::var("KIMI_API_KEY") {
credentials.push(ApiCredential {
provider: fabro_model::Provider::Kimi,
auth_header: ApiKeyHeader::Bearer(key),
extra_headers: HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
});
}
if let Ok(key) = std::env::var("ZAI_API_KEY") {
credentials.push(ApiCredential {
provider: fabro_model::Provider::Zai,
auth_header: ApiKeyHeader::Bearer(key),
extra_headers: HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
});
}
if let Ok(key) = std::env::var("MINIMAX_API_KEY") {
credentials.push(ApiCredential {
provider: fabro_model::Provider::Minimax,
auth_header: ApiKeyHeader::Bearer(key),
extra_headers: HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
});
}
if let Ok(key) = std::env::var("INCEPTION_API_KEY") {
credentials.push(ApiCredential {
provider: fabro_model::Provider::Inception,
auth_header: ApiKeyHeader::Bearer(key),
extra_headers: HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
});
}
Self::from_credentials(credentials).await
/// Returns `Error` if the source cannot resolve credentials or any provider
/// adapter fails to initialize.
pub async fn from_source(source: &dyn CredentialSource) -> Result<Self, Error> {
let resolved = source.resolve().await.map_err(|err| Error::Configuration {
message: format!("Failed to resolve LLM credentials: {err}"),
source: None,
})?;
Self::from_credentials(resolved.credentials).await
}
/// Create a Client from typed provider credentials.
@ -418,6 +329,8 @@ fn auth_value(auth_header: &ApiKeyHeader) -> String {
#[cfg(test)]
mod tests {
use async_trait::async_trait;
use fabro_auth::{CredentialSource, ResolvedCredentials};
use futures::stream;
use super::*;
@ -506,6 +419,27 @@ mod tests {
}
}
struct StubSource {
credentials: Vec<ApiCredential>,
}
#[async_trait]
impl CredentialSource for StubSource {
async fn resolve(&self) -> anyhow::Result<ResolvedCredentials> {
Ok(ResolvedCredentials {
credentials: self.credentials.clone(),
auth_issues: Vec::new(),
})
}
async fn configured_providers(&self) -> Vec<fabro_model::Provider> {
self.credentials
.iter()
.map(|credential| credential.provider)
.collect()
}
}
#[tokio::test]
async fn complete_routes_to_default_provider() {
let mut client = Client::new(HashMap::new(), None, vec![]);
@ -612,6 +546,39 @@ mod tests {
assert_eq!(client.default_provider(), Some("kimi"));
}
#[tokio::test]
async fn from_source_registers_provider_from_resolved_credentials() {
let source = StubSource {
credentials: vec![ApiCredential {
provider: fabro_model::Provider::Anthropic,
auth_header: ApiKeyHeader::Custom {
name: "x-api-key".to_string(),
value: "anthropic-key".to_string(),
},
extra_headers: HashMap::new(),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
}],
};
let client = Client::from_source(&source).await.unwrap();
assert_eq!(client.provider_names(), vec!["anthropic"]);
}
#[tokio::test]
async fn from_source_supports_empty_credentials() {
let source = StubSource {
credentials: Vec::new(),
};
let client = Client::from_source(&source).await.unwrap();
assert!(client.provider_names().is_empty());
}
#[tokio::test]
async fn register_sets_first_as_default() {
let mut client = Client::new(HashMap::new(), None, vec![]);

View file

@ -4,7 +4,7 @@ use std::task::{Context, Poll};
use fabro_util::backoff::BackoffPolicy;
use futures::{Stream, StreamExt, future, stream};
use tokio::sync::{OnceCell, mpsc};
use tokio::sync::mpsc;
use tokio::time;
use tokio_stream::wrappers::ReceiverStream;
use tokio_util::sync::CancellationToken;
@ -21,24 +21,6 @@ use crate::types::{
TokenCounts, ToolCall, ToolChoice, ToolDefinition,
};
/// Module-level default client (Section 2.5).
static DEFAULT_CLIENT: OnceCell<Arc<Client>> = OnceCell::const_new();
/// Set the module-level default client.
pub fn set_default_client(client: Client) {
let _ = DEFAULT_CLIENT.set(Arc::new(client));
}
/// Get the default client, lazily initialized from env.
async fn get_default_client() -> Result<Arc<Client>, Error> {
if let Some(client) = DEFAULT_CLIENT.get() {
return Ok(client.clone());
}
let client = Arc::new(Client::from_env().await?);
let _ = DEFAULT_CLIENT.set(client.clone());
Ok(client)
}
fn build_initial_messages(params: &GenerateParams) -> Result<Vec<Message>, Error> {
let mut messages = Vec::new();
if let Some(system) = &params.system {
@ -109,10 +91,7 @@ fn build_generate_result(steps: Vec<StepResult>, total_usage: TokenCounts) -> Ge
/// Panics if a tool's `execute` handler is `None` when matched during tool
/// execution.
pub async fn generate(params: GenerateParams) -> Result<GenerateResult, Error> {
let client = match params.client.clone() {
Some(c) => c,
None => get_default_client().await?,
};
let client = Arc::clone(&params.client);
let retry_policy = RetryPolicy {
max_retries: params.max_retries,
backoff: BackoffPolicy {
@ -307,7 +286,7 @@ pub struct GenerateParams {
pub metadata: Option<std::collections::HashMap<String, String>>,
pub max_retries: u32,
pub timeout: Option<TimeoutOptions>,
pub client: Option<Arc<Client>>,
pub client: Arc<Client>,
/// Cancellation token to interrupt generation (Section 4.8).
pub abort_signal: Option<CancellationToken>,
/// Custom stop condition checked after each tool round (Section 4.3).
@ -317,30 +296,30 @@ pub struct GenerateParams {
}
impl GenerateParams {
pub fn new(model: impl Into<String>) -> Self {
pub fn new(model: impl Into<String>, client: Arc<Client>) -> Self {
Self {
model: model.into(),
prompt: None,
messages: None,
system: None,
tools: None,
tool_choice: None,
max_tool_rounds: 1,
response_format: None,
temperature: None,
top_p: None,
max_tokens: None,
stop_sequences: None,
model: model.into(),
prompt: None,
messages: None,
system: None,
tools: None,
tool_choice: None,
max_tool_rounds: 1,
response_format: None,
temperature: None,
top_p: None,
max_tokens: None,
stop_sequences: None,
reasoning_effort: None,
speed: None,
provider: None,
speed: None,
provider: None,
provider_options: None,
metadata: None,
max_retries: 2,
timeout: None,
client: None,
abort_signal: None,
stop_when: None,
metadata: None,
max_retries: 2,
timeout: None,
client,
abort_signal: None,
stop_when: None,
repair_tool_call: None,
}
}
@ -363,12 +342,6 @@ impl GenerateParams {
self
}
#[must_use]
pub fn client(mut self, client: Arc<Client>) -> Self {
self.client = Some(client);
self
}
#[must_use]
pub fn tools(mut self, tools: Vec<Tool>) -> Self {
self.tools = Some(tools.into_iter().map(Arc::new).collect());
@ -642,10 +615,7 @@ pub async fn stream(params: GenerateParams) -> Result<StreamResult, Error> {
/// Returns `Error::Configuration` if both `prompt` and `messages` are set,
/// or any provider error encountered during streaming setup.
async fn stream_with_tool_loop(params: GenerateParams) -> Result<StreamEventStream, Error> {
let client = match params.client.clone() {
Some(c) => c,
None => get_default_client().await?,
};
let client = Arc::clone(&params.client);
let mut messages = build_initial_messages(&params)?;
let tool_definitions: Option<Vec<ToolDefinition>> = params
.tools
@ -931,10 +901,7 @@ async fn stream_generate_raw(
/// Returns `Error::Configuration` if both `prompt` and `messages` are set,
/// or any provider error encountered during streaming setup.
pub async fn stream_generate(params: GenerateParams) -> Result<StreamEventStream, Error> {
let client = match params.client.clone() {
Some(c) => c,
None => get_default_client().await?,
};
let client = Arc::clone(&params.client);
let messages = build_initial_messages(&params)?;
let tool_definitions: Option<Vec<ToolDefinition>> = params
.tools
@ -1204,13 +1171,10 @@ mod tests {
#[tokio::test]
async fn generate_simple_text() {
let result = generate(
GenerateParams::new("mock-model")
.prompt("Hello")
.client(mock_client("Hi there!")),
)
.await
.unwrap();
let result =
generate(GenerateParams::new("mock-model", mock_client("Hi there!")).prompt("Hello"))
.await
.unwrap();
assert_eq!(result.text(), "Hi there!");
assert_eq!(result.finish_reason, FinishReason::Stop);
@ -1221,10 +1185,9 @@ mod tests {
#[tokio::test]
async fn generate_with_system_message() {
let result = generate(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", mock_client("Greetings!"))
.system("You are helpful")
.prompt("Hello")
.client(mock_client("Greetings!")),
.prompt("Hello"),
)
.await
.unwrap();
@ -1235,13 +1198,11 @@ mod tests {
#[tokio::test]
async fn generate_with_messages() {
let result = generate(
GenerateParams::new("mock-model")
.messages(vec![
Message::user("Hello"),
Message::assistant("Hi"),
Message::user("How are you?"),
])
.client(mock_client("I'm doing well!")),
GenerateParams::new("mock-model", mock_client("I'm doing well!")).messages(vec![
Message::user("Hello"),
Message::assistant("Hi"),
Message::user("How are you?"),
]),
)
.await
.unwrap();
@ -1255,8 +1216,8 @@ mod tests {
model: "mock-model".into(),
prompt: Some("Hello".into()),
messages: Some(vec![Message::user("World")]),
client: Some(mock_client("test")),
..GenerateParams::new("mock-model")
client: mock_client("test"),
..GenerateParams::new("mock-model", mock_client("base"))
})
.await;
@ -1341,7 +1302,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let result = generate(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather in SF?")
.tools(vec![Tool::active(
"get_weather",
@ -1352,8 +1313,7 @@ mod tests {
Ok(serde_json::json!(format!("72F in {}", city)))
},
)])
.max_tool_rounds(5)
.client(client),
.max_tool_rounds(5),
)
.await
.unwrap();
@ -1418,13 +1378,9 @@ mod tests {
#[tokio::test]
async fn stream_generate_returns_events() {
let client = mock_client("Hello stream!");
let mut stream = stream_generate(
GenerateParams::new("mock-model")
.prompt("Hi")
.client(client),
)
.await
.unwrap();
let mut stream = stream_generate(GenerateParams::new("mock-model", client).prompt("Hi"))
.await
.unwrap();
let first = stream.next().await.unwrap().unwrap();
match &first {
@ -1451,9 +1407,7 @@ mod tests {
});
let result = generate_object(
GenerateParams::new("mock-model")
.prompt("Extract name and age")
.client(client),
GenerateParams::new("mock-model", client).prompt("Extract name and age"),
schema,
)
.await
@ -1470,9 +1424,7 @@ mod tests {
let client = mock_client("not valid json");
let result = generate_object(
GenerateParams::new("mock-model")
.prompt("Extract data")
.client(client),
GenerateParams::new("mock-model", client).prompt("Extract data"),
serde_json::json!({"type": "object"}),
)
.await;
@ -1496,7 +1448,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let result = generate(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather in SF?")
.tools(vec![Tool::active(
"get_weather",
@ -1508,8 +1460,7 @@ mod tests {
},
)])
.max_tool_rounds(5)
.stop_when(|_steps| true) // Stop immediately after first round
.client(client),
.stop_when(|_steps| true), // Stop immediately after first round
)
.await
.unwrap();
@ -1521,7 +1472,7 @@ mod tests {
#[test]
fn generate_params_builder_methods() {
let params = GenerateParams::new("test-model")
let params = GenerateParams::new("test-model", mock_client("builder"))
.prompt("hello")
.system("you are helpful")
.temperature(0.7)
@ -1558,10 +1509,11 @@ mod tests {
#[test]
fn generate_params_timeout_builder() {
let params = GenerateParams::new("test-model").timeout(TimeoutOptions {
total: Some(30.0),
per_step: Some(10.0),
});
let params =
GenerateParams::new("test-model", mock_client("timeout")).timeout(TimeoutOptions {
total: Some(30.0),
per_step: Some(10.0),
});
assert!(params.timeout.is_some());
let t = params.timeout.unwrap();
assert_eq!(t.total, Some(30.0));
@ -1662,9 +1614,7 @@ mod tests {
});
let obj_stream = stream_object(
GenerateParams::new("mock-model")
.prompt("Extract info")
.client(client),
GenerateParams::new("mock-model", client).prompt("Extract info"),
schema,
)
.await
@ -1700,9 +1650,7 @@ mod tests {
});
let obj_stream = stream_object(
GenerateParams::new("mock-model")
.prompt("Extract info")
.client(client),
GenerateParams::new("mock-model", client).prompt("Extract info"),
schema,
)
.await
@ -1748,9 +1696,7 @@ mod tests {
let schema = serde_json::json!({"type": "object"});
let obj_stream = stream_object(
GenerateParams::new("mock-model")
.prompt("Extract info")
.client(client),
GenerateParams::new("mock-model", client).prompt("Extract info"),
schema,
)
.await
@ -1768,9 +1714,8 @@ mod tests {
token.cancel();
let result = generate(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", mock_client("Hi"))
.prompt("Hello")
.client(mock_client("Hi"))
.abort_signal(token),
)
.await;
@ -1839,7 +1784,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let result = generate(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather?")
.tools(vec![Tool::active(
"get_weather",
@ -1848,8 +1793,7 @@ mod tests {
|_args, _ctx| async { Ok(serde_json::json!("72F")) },
)])
.max_tool_rounds(10)
.abort_signal(token)
.client(client),
.abort_signal(token),
)
.await;
@ -1869,9 +1813,8 @@ mod tests {
token_clone.cancel();
let mut stream_result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("Hi")
.client(client)
.abort_signal(token),
)
.await
@ -1897,7 +1840,7 @@ mod tests {
let tool_executed_clone = tool_executed.clone();
let result = generate(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather in SF?")
.tools(vec![Tool::active(
"get_weather",
@ -1911,8 +1854,7 @@ mod tests {
}
},
)])
.max_tool_rounds(0)
.client(client),
.max_tool_rounds(0),
)
.await
.unwrap();
@ -1928,20 +1870,16 @@ mod tests {
#[test]
fn generate_params_abort_signal_builder() {
let token = CancellationToken::new();
let params = GenerateParams::new("test-model").abort_signal(token);
let params = GenerateParams::new("test-model", mock_client("abort")).abort_signal(token);
assert!(params.abort_signal.is_some());
}
#[tokio::test]
async fn stream_result_accumulates_response() {
let client = mock_client("Hello!");
let mut result = stream(
GenerateParams::new("mock-model")
.prompt("Hi")
.client(client),
)
.await
.unwrap();
let mut result = stream(GenerateParams::new("mock-model", client).prompt("Hi"))
.await
.unwrap();
assert!(result.response().is_none());
assert!(result.partial_response().is_none());
@ -1956,13 +1894,9 @@ mod tests {
#[tokio::test]
async fn stream_result_text_stream() {
let client = streaming_json_mock_client(vec!["Hello", " ", "world"]);
let result = stream(
GenerateParams::new("mock-model")
.prompt("Hi")
.client(client),
)
.await
.unwrap();
let result = stream(GenerateParams::new("mock-model", client).prompt("Hi"))
.await
.unwrap();
let texts: Vec<String> = result
.text_stream()
@ -2077,7 +2011,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let mut result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather in SF?")
.tools(vec![Tool::active(
"get_weather",
@ -2085,8 +2019,7 @@ mod tests {
serde_json::json!({"type": "object", "properties": {"city": {"type": "string"}}}),
|_args, _ctx| async { Ok(serde_json::json!("72F")) },
)])
.max_tool_rounds(5)
.client(client),
.max_tool_rounds(5),
)
.await
.unwrap();
@ -2130,7 +2063,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let mut result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather?")
.tools(vec![Tool::active(
"get_weather",
@ -2138,8 +2071,7 @@ mod tests {
serde_json::json!({"type": "object", "properties": {"city": {"type": "string"}}}),
|_args, _ctx| async { Ok(serde_json::json!("72F")) },
)])
.max_tool_rounds(0)
.client(client),
.max_tool_rounds(0),
)
.await
.unwrap();
@ -2205,7 +2137,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let mut result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather in SF?")
.tools(vec![Tool::active(
"get_weather",
@ -2213,8 +2145,7 @@ mod tests {
serde_json::json!({"type": "object", "properties": {"city": {"type": "string"}}}),
|_args, _ctx| async { Ok(serde_json::json!("72F")) },
)])
.max_tool_rounds(5)
.client(client),
.max_tool_rounds(5),
)
.await
.unwrap();
@ -2267,7 +2198,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let mut result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather in SF?")
.tools(vec![Tool::active(
"get_weather",
@ -2276,8 +2207,7 @@ mod tests {
|_args, _ctx| async { Ok(serde_json::json!("72F")) },
)])
.max_tool_rounds(5)
.stop_when(|_steps| true) // Stop immediately after first round
.client(client),
.stop_when(|_steps| true), // Stop immediately after first round
)
.await
.unwrap();
@ -2392,7 +2322,7 @@ mod tests {
// Need active tools so the tool loop path (with retry) is used
let mut result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("Hi")
.tools(vec![Tool::active(
"get_weather",
@ -2401,8 +2331,7 @@ mod tests {
|_args, _ctx| async { Ok(serde_json::json!("72F")) },
)])
.max_tool_rounds(1)
.max_retries(3)
.client(client),
.max_retries(3),
)
.await
.unwrap();
@ -2489,7 +2418,7 @@ mod tests {
// Need active tools so the tool loop path (with timeout) is used
let mut result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("Hi")
.tools(vec![Tool::active(
"get_weather",
@ -2502,8 +2431,7 @@ mod tests {
total: None,
per_step: Some(0.01), // 10ms timeout, provider takes 5s
})
.max_retries(0)
.client(client),
.max_retries(0),
)
.await
.unwrap();
@ -2621,7 +2549,7 @@ mod tests {
let client = Arc::new(Client::new(providers, Some("mock".to_string()), vec![]));
let mut result = stream(
GenerateParams::new("mock-model")
GenerateParams::new("mock-model", client)
.prompt("What's the weather?")
.tools(vec![Tool::active(
"get_weather",
@ -2634,8 +2562,7 @@ mod tests {
total: Some(0.05), // 50ms total timeout
per_step: None,
})
.max_retries(0)
.client(client),
.max_retries(0),
)
.await
.unwrap();

View file

@ -9,7 +9,5 @@ pub mod retry;
pub mod tools;
pub mod types;
// Re-export module-level default client helpers (Section 2.5).
pub use error::{Error, ProviderErrorDetail, ProviderErrorKind, Result};
pub use fabro_model::{ModelHandle, Provider};
pub use generate::set_default_client;

View file

@ -71,22 +71,10 @@ impl ModelTestOutcome {
}
}
pub async fn run_model_test(info: &Model, mode: ModelTestMode) -> ModelTestOutcome {
run_model_test_inner(info, mode, None).await
}
pub async fn run_model_test_with_client(
pub async fn run_model_test(
info: &Model,
mode: ModelTestMode,
client: Arc<Client>,
) -> ModelTestOutcome {
run_model_test_inner(info, mode, Some(client)).await
}
async fn run_model_test_inner(
info: &Model,
mode: ModelTestMode,
client: Option<Arc<Client>>,
) -> ModelTestOutcome {
match mode {
ModelTestMode::Basic => run_basic_test(info, client).await,
@ -94,14 +82,11 @@ async fn run_model_test_inner(
}
}
async fn run_basic_test(info: &Model, client: Option<Arc<Client>>) -> ModelTestOutcome {
let mut params = GenerateParams::new(&info.id)
async fn run_basic_test(info: &Model, client: Arc<Client>) -> ModelTestOutcome {
let params = GenerateParams::new(&info.id, client)
.provider(info.provider.as_str())
.prompt("Say OK")
.max_tokens(16);
if let Some(client) = client {
params = params.client(client);
}
let result = time::timeout(
Duration::from_secs(ModelTestMode::Basic.timeout_secs()),
@ -116,7 +101,7 @@ async fn run_basic_test(info: &Model, client: Option<Arc<Client>>) -> ModelTestO
}
}
async fn run_deep_test(info: &Model, client: Option<Arc<Client>>) -> ModelTestOutcome {
async fn run_deep_test(info: &Model, client: Arc<Client>) -> ModelTestOutcome {
let Some(params) = build_deep_test_params(info, client) else {
return ModelTestOutcome::error("model does not support tools");
};
@ -137,7 +122,7 @@ async fn run_deep_test(info: &Model, client: Option<Arc<Client>>) -> ModelTestOu
}
}
fn build_deep_test_params(info: &Model, client: Option<Arc<Client>>) -> Option<GenerateParams> {
fn build_deep_test_params(info: &Model, client: Arc<Client>) -> Option<GenerateParams> {
if !info.features.tools {
return None;
}
@ -166,7 +151,7 @@ fn build_deep_test_params(info: &Model, client: Option<Arc<Client>>) -> Option<G
},
);
let mut params = GenerateParams::new(&info.id)
let mut params = GenerateParams::new(&info.id, client)
.provider(info.provider.as_str())
.prompt(
"Use the add tool twice: first add 15 and 27, then add that result to 42. \
@ -180,10 +165,6 @@ fn build_deep_test_params(info: &Model, client: Option<Arc<Client>>) -> Option<G
params = params.reasoning_effort(ReasoningEffort::High);
}
if let Some(client) = client {
params = params.client(client);
}
Some(params)
}
@ -205,6 +186,8 @@ fn validate_deep_result(result: &GenerateResult) -> Result<(), String> {
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use fabro_model::{ModelCosts, ModelFeatures, ModelLimits, Provider};
use super::*;
@ -248,6 +231,10 @@ mod tests {
}
}
fn empty_test_client() -> Arc<Client> {
Arc::new(Client::new(HashMap::new(), None, vec![]))
}
#[tokio::test]
async fn run_model_test_deep_errors_when_model_lacks_tools() {
let info = test_model_with(ModelFeatures {
@ -257,7 +244,7 @@ mod tests {
effort: true,
});
let outcome = run_model_test(&info, ModelTestMode::Deep).await;
let outcome = run_model_test(&info, ModelTestMode::Deep, empty_test_client()).await;
assert_eq!(outcome.status, ModelTestStatus::Error);
assert_eq!(

View file

@ -2,6 +2,7 @@ use std::time::Duration;
use base64::Engine as _;
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
use fabro_auth::auth_issue_message;
use fabro_llm::client::Client as LlmClient;
use fabro_llm::types::{Message, Request};
use fabro_model::{Catalog, Provider};
@ -15,7 +16,6 @@ use serde::Serialize;
use tokio::time::timeout;
use crate::server::AppState;
use crate::server_secrets::auth_issue_message;
fn http_client_or_check(
name: &str,
@ -75,7 +75,7 @@ pub async fn run_all(state: &AppState) -> DiagnosticsReport {
}
async fn check_llm_providers(state: &AppState) -> CheckResult {
let result = match state.build_llm_client().await {
let result = match state.resolve_llm_client().await {
Ok(result) => result,
Err(err) => {
return CheckResult {

View file

@ -4,6 +4,7 @@ use std::sync::Arc;
use anyhow::{Result, anyhow, bail};
use fabro_api::types;
use fabro_auth::auth_issue_message;
use fabro_config::{
CliLayer, CliOutputLayer, DaytonaDockerfileLayer, ReplaceMap, RunExecutionLayer, RunLayer,
RunModelLayer, RunSandboxLayer, WorkflowSettingsBuilder,
@ -34,7 +35,6 @@ use fabro_workflow::run_materialization::materialize_run;
use fabro_workflow::workflow_bundle::{BundledWorkflow, WorkflowBundle};
use crate::server::AppState;
use crate::server_secrets::auth_issue_message;
#[derive(Clone)]
pub(crate) struct PreparedManifest {
@ -389,7 +389,7 @@ async fn build_preflight_report(
));
}
let configured_providers = state.provider_credentials.configured_providers().await;
let configured_providers = state.llm_source.configured_providers().await;
let materialized = materialize_run(
prepared.settings.clone(),
graph,
@ -600,7 +600,7 @@ async fn run_llm_check(
let (model, provider) = resolve_model_provider(settings, graph, configured_providers);
let default_provider = provider.as_deref().unwrap_or("anthropic");
match state.build_llm_client().await {
match state.resolve_llm_client().await {
Ok(result) => {
let configured = result
.client

View file

@ -37,14 +37,17 @@ pub use fabro_api::types::{
StageStatus as ApiStageStatus, StartRunRequest, SubmitAnswerRequest, SystemFeatures,
SystemInfoResponse, SystemRunCounts, WriteBlobResponse,
};
use fabro_auth::parse_credential_secret;
use fabro_auth::{
CredentialSource, VaultCredentialSource, auth_issue_message, parse_credential_secret,
};
use fabro_config::daemon::ServerDaemon;
use fabro_config::{RunLayer, RunSettingsBuilder, ServerSettingsBuilder, Storage, envfile};
use fabro_interview::{
Answer, ControlInterviewer, Interviewer, Question, QuestionType, WorkerControlEnvelope,
};
use fabro_llm::client::Client as LlmClient;
use fabro_llm::generate::{GenerateParams, generate_object};
use fabro_llm::model_test::{ModelTestMode, run_model_test_with_client};
use fabro_llm::model_test::{ModelTestMode, run_model_test};
use fabro_llm::types::{
ContentPart, FinishReason, Message as LlmMessage, Request as LlmRequest, Role, ToolChoice,
ToolDefinition,
@ -113,9 +116,7 @@ use crate::ip_allowlist::{IpAllowlistConfig, ip_allowlist_middleware};
use crate::jwt_auth::{self, AuthMode, AuthenticatedService, AuthenticatedSubject};
use crate::run_files::{FilesInFlight, list_run_files, new_files_in_flight};
use crate::run_selector::{ResolveRunError, resolve_run_by_selector};
use crate::server_secrets::{
LlmClientResult, ProviderCredentials, ServerSecrets, auth_issue_message,
};
use crate::server_secrets::{LlmClientResult, ServerSecrets};
use crate::spawn_env::{apply_render_graph_env, apply_worker_env};
use crate::worker_token::{
AuthorizeRunBlob, AuthorizeRunScoped, AuthorizeStageArtifact, WorkerTokenKeys,
@ -548,18 +549,18 @@ pub struct AppState {
/// proceed in parallel. See `crate::run_files` for semantics.
pub(crate) files_in_flight: FilesInFlight,
pub(crate) vault: Arc<AsyncRwLock<Vault>>,
pub(super) server_secrets: ServerSecrets,
pub(crate) provider_credentials: ProviderCredentials,
manifest_run_defaults: RwLock<Arc<RunLayer>>,
manifest_run_settings: RwLock<std::result::Result<RunNamespace, String>>,
pub(crate) server_settings: RwLock<Arc<ServerSettings>>,
pub(crate) env_lookup: EnvLookup,
http_client: Option<fabro_http::HttpClient>,
shutting_down: AtomicBool,
registry_factory_override: Option<Box<RegistryFactoryOverride>>,
slack_service: Option<Arc<SlackService>>,
slack_started: AtomicBool,
pub(crate) vault: Arc<AsyncRwLock<Vault>>,
pub(super) server_secrets: ServerSecrets,
pub(crate) llm_source: Arc<dyn CredentialSource>,
manifest_run_defaults: RwLock<Arc<RunLayer>>,
manifest_run_settings: RwLock<std::result::Result<RunNamespace, String>>,
pub(crate) server_settings: RwLock<Arc<ServerSettings>>,
pub(crate) env_lookup: EnvLookup,
http_client: Option<fabro_http::HttpClient>,
shutting_down: AtomicBool,
registry_factory_override: Option<Box<RegistryFactoryOverride>>,
slack_service: Option<Arc<SlackService>>,
slack_started: AtomicBool,
}
pub(crate) struct AppStateConfig {
@ -664,8 +665,20 @@ impl AppState {
)
}
pub(crate) async fn build_llm_client(&self) -> Result<LlmClientResult, String> {
self.provider_credentials.build_llm_client().await
pub(crate) async fn resolve_llm_client(&self) -> Result<LlmClientResult, String> {
let resolved = self
.llm_source
.resolve()
.await
.map_err(|err| err.to_string())?;
let client = LlmClient::from_credentials(resolved.credentials)
.await
.map_err(|err| err.to_string())?;
Ok(LlmClientResult {
client,
auth_issues: resolved.auth_issues,
})
}
pub(crate) fn vault_or_env(&self, name: &str) -> Option<String> {
@ -2667,10 +2680,13 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppS
} = config;
let vault = Arc::new(AsyncRwLock::new(Vault::load(vault_path)?));
let provider_credentials = ProviderCredentials::with_env_lookup(Arc::clone(&vault), {
let env_lookup = Arc::clone(&env_lookup);
move |name| env_lookup(name)
});
let llm_source: Arc<dyn CredentialSource> = Arc::new(VaultCredentialSource::with_env_lookup(
Arc::clone(&vault),
{
let env_lookup = Arc::clone(&env_lookup);
move |name| env_lookup(name)
},
));
let (global_event_tx, _) = broadcast::channel(4096);
let current_server_settings = Arc::new(resolved_settings.server_settings);
let current_manifest_run_defaults = Arc::new(resolved_settings.manifest_run_defaults);
@ -2713,7 +2729,7 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppS
files_in_flight: new_files_in_flight(),
vault,
server_secrets,
provider_credentials,
llm_source,
manifest_run_defaults: RwLock::new(current_manifest_run_defaults),
manifest_run_settings: RwLock::new(current_manifest_run_settings),
server_settings: RwLock::new(current_server_settings),
@ -4093,7 +4109,7 @@ async fn create_run(
let run_id = prepared.run_id.unwrap_or_else(RunId::new);
info!(run_id = %run_id, "Run created");
let configured_providers = state.provider_credentials.configured_providers().await;
let configured_providers = state.llm_source.configured_providers().await;
let mut create_input = run_manifest::create_run_input(prepared.clone(), configured_providers);
create_input.run_id = Some(run_id);
create_input.provenance = Some(run_provenance(&headers, &subject));
@ -6798,12 +6814,12 @@ async fn test_model(
return ApiError::not_found(format!("Model not found: {id}")).into_response();
};
let llm_result = match state.build_llm_client().await {
let llm_result = match state.resolve_llm_client().await {
Ok(result) => result,
Err(err) => {
return ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
format!("Failed to build LLM client: {err}"),
format!("Failed to resolve LLM client: {err}"),
)
.into_response();
}
@ -6828,7 +6844,7 @@ async fn test_model(
}
let client = Arc::new(llm_result.client);
let outcome = run_model_test_with_client(info, mode, client).await;
let outcome = run_model_test(info, mode, client).await;
Json(serde_json::json!({
"model_id": info.id,
"status": outcome.status.as_str(),
@ -6976,8 +6992,7 @@ async fn create_completion(
// Force non-streaming for structured output
let use_stream = req.stream && req.schema.is_none();
// Get or create LLM client (cached in AppState)
let llm_result = match state.build_llm_client().await {
let llm_result = match state.resolve_llm_client().await {
Ok(result) => result,
Err(err) => {
return ApiError::new(
@ -7041,9 +7056,9 @@ async fn create_completion(
if let Some(schema) = req.schema {
// Structured output uses generate_object for JSON parsing logic
let mut params = GenerateParams::new(&request.model)
.messages(request.messages)
.client(std::sync::Arc::new(client.clone()));
let mut params =
GenerateParams::new(&request.model, std::sync::Arc::new(client.clone()))
.messages(request.messages);
if let Some(ref p) = request.provider {
params = params.provider(p);
}
@ -7316,13 +7331,17 @@ mod tests {
use axum::body::Body;
use axum::http::{Method, Request, header};
use chrono::{Duration as ChronoDuration, Utc};
use fabro_auth::{AuthCredential, AuthDetails};
use fabro_config::bind::Bind;
use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question, QuestionType};
use fabro_llm::types::{Message as LlmMessage, Request as LlmRequest};
use fabro_model::Provider;
use fabro_types::settings::ServerAuthMethod;
use fabro_types::{
InterviewQuestionRecord, InterviewQuestionType, RunAuthMethod, RunBlobId, RunId, fixtures,
};
use httpmock::Method::POST;
use httpmock::MockServer;
use serde_json::json;
use tokio_stream::StreamExt as _;
use tower::ServiceExt;
@ -7381,6 +7400,39 @@ mod tests {
serde_json::from_slice(&bytes).unwrap()
}
fn openai_api_key_credential(key: &str) -> AuthCredential {
AuthCredential {
provider: Provider::OpenAi,
details: AuthDetails::ApiKey {
key: key.to_string(),
},
}
}
fn openai_responses_payload(text: &str) -> serde_json::Value {
json!({
"id": "resp_1",
"model": "gpt-5.4",
"output": [
{
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": text
}
]
}
],
"status": "completed",
"usage": {
"input_tokens": 10,
"output_tokens": 20
}
})
}
macro_rules! assert_status {
($response:expr, $expected:expr) => {
fabro_test::assert_axum_status($response, $expected, concat!(file!(), ":", line!()))
@ -7954,6 +8006,120 @@ provider = "invalid-provider"
assert!(state.vault.read().await.get("openai_codex").is_some());
}
#[tokio::test]
async fn resolve_llm_client_reads_openai_codex_credential_from_vault() {
let state = create_app_state_with_env_lookup(
default_test_server_settings(),
RunLayer::default(),
5,
|_| None,
);
state
.vault
.write()
.await
.set(
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let llm_result = state.resolve_llm_client().await.unwrap();
assert_eq!(llm_result.client.provider_names(), vec!["openai"]);
assert!(llm_result.auth_issues.is_empty());
}
#[tokio::test]
async fn llm_source_configured_providers_reads_openai_codex_from_vault() {
let state = create_app_state_with_env_lookup(
default_test_server_settings(),
RunLayer::default(),
5,
|_| None,
);
state
.vault
.write()
.await
.set(
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
assert_eq!(state.llm_source.configured_providers().await, vec![
Provider::OpenAi
]);
}
#[tokio::test]
async fn resolve_llm_client_uses_env_lookup_for_openai_settings() {
let server = MockServer::start_async().await;
let response_mock = server
.mock_async(|when, then| {
when.method(POST)
.path("/v1/responses")
.header("authorization", "Bearer vault-openai-key")
.header("OpenAI-Organization", "env-org");
then.status(200)
.header("content-type", "application/json")
.json_body(openai_responses_payload("hello from env lookup"));
})
.await;
let base_url = server.url("/v1");
let state = create_app_state_with_env_lookup(
default_test_server_settings(),
RunLayer::default(),
5,
move |name| match name {
"OPENAI_BASE_URL" => Some(base_url.clone()),
"OPENAI_ORG_ID" => Some("env-org".to_string()),
_ => None,
},
);
state
.vault
.write()
.await
.set(
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let llm_result = state.resolve_llm_client().await.unwrap();
let response = llm_result
.client
.complete(&LlmRequest {
model: "gpt-5.4".to_string(),
messages: vec![LlmMessage::user("Hello")],
provider: Some("openai".to_string()),
tools: None,
tool_choice: None,
response_format: None,
temperature: None,
top_p: None,
max_tokens: None,
stop_sequences: None,
reasoning_effort: None,
speed: None,
metadata: None,
provider_options: None,
})
.await
.unwrap();
assert_eq!(response.text(), "hello from env lookup");
response_mock.assert_async().await;
}
#[tokio::test]
async fn list_secrets_includes_credential_metadata() {
let state = create_app_state();
@ -8304,27 +8470,6 @@ allowed_usernames = ["octocat"]
.unwrap_or(EnvOverride::Unchanged)
}
#[test]
fn provider_credentials_resolve_process_env_before_vault() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set("OPENAI_API_KEY", "vault-key", SecretType::Environment, None)
.unwrap();
let provider_credentials =
ProviderCredentials::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |name| {
match name {
"OPENAI_API_KEY" => Some("env-key".to_string()),
_ => None,
}
});
let runtime = tokio::runtime::Runtime::new().unwrap();
let resolved = runtime.block_on(provider_credentials.get("OPENAI_API_KEY"));
assert_eq!(resolved.as_deref(), Some("env-key"));
}
#[tokio::test]
async fn subprocess_answer_transport_cancel_run_enqueues_cancel_message() {
let (control_tx, mut control_rx) = tokio::sync::mpsc::channel(1);

View file

@ -1,15 +1,10 @@
use std::collections::HashMap;
use std::path::Path;
use std::sync::Arc;
use fabro_auth::{CredentialResolver, CredentialUsage, ResolveError, ResolvedCredential};
use fabro_auth::ResolveError;
use fabro_config::envfile;
use fabro_llm::client::Client as LlmClient;
use fabro_llm::client::Client;
use fabro_model::Provider;
use fabro_vault::Vault;
use tokio::sync::RwLock as AsyncRwLock;
type EnvLookup = Arc<dyn Fn(&str) -> Option<String> + Send + Sync>;
pub fn process_env_snapshot() -> HashMap<String, String> {
std::env::vars().collect()
@ -57,150 +52,18 @@ impl std::fmt::Debug for ServerSecrets {
}
}
#[derive(Clone)]
pub(crate) struct ProviderCredentials {
vault: Arc<AsyncRwLock<Vault>>,
env_lookup: EnvLookup,
}
impl ProviderCredentials {
pub(crate) fn with_env_lookup<F>(vault: Arc<AsyncRwLock<Vault>>, env_lookup: F) -> Self
where
F: Fn(&str) -> Option<String> + Send + Sync + 'static,
{
Self {
vault,
env_lookup: Arc::new(env_lookup),
}
}
#[cfg(test)]
pub(crate) async fn get(&self, name: &str) -> Option<String> {
let env_value = (self.env_lookup)(name);
if env_value.is_some() {
return env_value;
}
self.vault.read().await.get(name).map(str::to_string)
}
pub(crate) async fn build_llm_client(&self) -> Result<LlmClientResult, String> {
let resolver =
CredentialResolver::with_env_lookup(Arc::clone(&self.vault), self.env_lookup.clone());
let mut api_credentials = Vec::new();
let mut auth_issues = Vec::new();
for provider in Provider::ALL {
match resolver
.resolve(*provider, CredentialUsage::ApiRequest)
.await
{
Ok(ResolvedCredential::Api(credential)) => api_credentials.push(credential),
Ok(ResolvedCredential::Cli(_)) | Err(ResolveError::NotConfigured(_)) => {}
Err(err) => auth_issues.push((*provider, err)),
}
}
let client = LlmClient::from_credentials(api_credentials)
.await
.map_err(|err| err.to_string())?;
Ok(LlmClientResult {
client,
auth_issues,
})
}
pub(crate) async fn configured_providers(&self) -> Vec<Provider> {
let resolver =
CredentialResolver::with_env_lookup(Arc::clone(&self.vault), self.env_lookup.clone());
let vault = self.vault.read().await;
resolver.configured_providers(&vault)
}
}
pub(crate) struct LlmClientResult {
pub client: LlmClient,
pub client: Client,
pub auth_issues: Vec<(Provider, ResolveError)>,
}
pub(crate) fn auth_issue_message(provider: Provider, err: &ResolveError) -> String {
match err {
ResolveError::NotConfigured(_) => {
format!("{} is not configured", provider.display_name())
}
ResolveError::RefreshFailed { source, .. } => format!(
"{} requires re-authentication: {}",
provider.display_name(),
source
),
ResolveError::RefreshTokenMissing(_) => format!(
"{} requires re-authentication: refresh token missing",
provider.display_name()
),
}
}
impl std::fmt::Debug for ProviderCredentials {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("ProviderCredentials")
.finish_non_exhaustive()
}
}
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::sync::Arc;
use fabro_auth::{AuthCredential, AuthDetails};
use fabro_config::envfile;
use fabro_vault::{SecretType, Vault};
use tokio::sync::RwLock as AsyncRwLock;
use super::{ProviderCredentials, ServerSecrets};
use crate::server_secrets::Provider;
#[tokio::test]
async fn configured_providers_respects_injected_env_lookup() {
let dir = tempfile::tempdir().unwrap();
let vault = Arc::new(AsyncRwLock::new(
Vault::load(dir.path().join("secrets.json")).unwrap(),
));
let credentials = ProviderCredentials::with_env_lookup(Arc::clone(&vault), |name| {
(name == "OPENAI_API_KEY").then(|| "openai-key".to_string())
});
assert_eq!(credentials.configured_providers().await, vec![
Provider::OpenAi
]);
}
#[tokio::test]
async fn configured_providers_includes_vault_credentials() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"anthropic",
&serde_json::to_string(&AuthCredential {
provider: Provider::Anthropic,
details: AuthDetails::ApiKey {
key: "anthropic-key".to_string(),
},
})
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let credentials =
ProviderCredentials::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |_| None);
assert_eq!(credentials.configured_providers().await, vec![
Provider::Anthropic
]);
}
use super::ServerSecrets;
#[test]
fn server_secrets_snapshot_prefers_env_over_file() {

View file

@ -73,6 +73,7 @@ tokio = { workspace = true, features = ["test-util", "macros"] }
object_store.workspace = true
assert_cmd = "2"
predicates = "3"
httpmock = "0.8"
fabro-macros = { path = "../fabro-macros" }
fabro-test = { workspace = true }
fabro-types = { path = "../fabro-types", features = ["test-support"] }

View file

@ -2888,6 +2888,19 @@ impl Emitter {
self.emit_with_scope(event, Some(scope));
}
pub fn notice(
&self,
level: RunNoticeLevel,
code: impl Into<String>,
message: impl Into<String>,
) {
self.emit(&Event::RunNotice {
level,
code: code.into(),
message: message.into(),
});
}
fn emit_with_scope(&self, event: &Event, scope: Option<&StageScope>) {
self.last_event_at.store(epoch_millis(), Ordering::Relaxed);
event.trace();

View file

@ -254,10 +254,10 @@ impl Handler for AgentHandler {
let prompt_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.provider.as_str().to_string()));
.or_else(|| Some(services.run.provider.as_str().to_string()));
let prompt_model = node.model().map(String::from);
let stage_scope = StageScope::for_handler(context, &node.id);
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::Prompt {
stage: node.id.clone(),
visit: stage_scope.visit,
@ -276,10 +276,10 @@ impl Handler for AgentHandler {
.parse::<RunId>()
.map_err(|err| Error::handler(format!("invalid internal run_id: {err}")))?;
let tool_hooks: Option<Arc<dyn fabro_agent::ToolHookCallback>> =
services.hook_runner.as_ref().map(|hr| {
services.run.hook_runner.as_ref().map(|hr| {
Arc::new(fabro_hooks::WorkflowToolHookCallback {
hook_runner: Arc::clone(hr),
sandbox: Arc::clone(&services.sandbox),
sandbox: Arc::clone(&services.run.sandbox),
run_id,
workflow_name: graph.name.clone(),
work_dir: None,
@ -294,8 +294,8 @@ impl Handler for AgentHandler {
&prompt,
context,
thread_id.as_deref(),
&services.emitter,
&services.sandbox,
&services.run.emitter,
&services.run.sandbox,
tool_hooks,
)
.await;
@ -331,9 +331,9 @@ impl Handler for AgentHandler {
let response_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.provider.as_str().to_string()))
.or_else(|| Some(services.run.provider.as_str().to_string()))
.unwrap_or_default();
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::PromptCompleted {
node_id: node.id.clone(),
response: response_text.clone(),
@ -366,6 +366,7 @@ impl Handler for AgentHandler {
if !found_in_response {
let mut found_in_status_json = false;
if let Ok(result) = services
.run
.sandbox
.exec_command("cat status.json", 5_000, None, None, None)
.await
@ -379,6 +380,7 @@ impl Handler for AgentHandler {
let quoted = shlex::try_quote(path).unwrap_or_else(|_| path.into());
let cmd = format!("cat {quoted}");
if let Ok(result) = services
.run
.sandbox
.exec_command(&cmd, 5_000, None, None, None)
.await
@ -435,13 +437,13 @@ mod tests {
) {
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let services = EngineServices {
emitter: Arc::new(crate::event::Emitter::new(fixtures::RUN_1)),
run_store: run_store.clone().into(),
..EngineServices::test_default()
};
let mut services = EngineServices::test_default();
services.run = services
.run
.with_emitter(Arc::new(crate::event::Emitter::new(fixtures::RUN_1)))
.with_run_store(run_store.clone().into());
let logger = crate::event::StoreProgressLogger::new(run_store.clone());
logger.register(services.emitter.as_ref());
logger.register(services.run.emitter.as_ref());
(services, run_store, logger)
}
@ -576,9 +578,12 @@ mod tests {
let tmp = TempDir::new().unwrap();
let mut services = EngineServices::test_default();
services.sandbox = std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
));
services.run =
services
.run
.with_sandbox(std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
)));
let outcome = handler
.execute(&node, &context, &graph, tmp.path(), &services)
@ -634,9 +639,12 @@ mod tests {
let tmp = TempDir::new().unwrap();
let mut services = EngineServices::test_default();
services.sandbox = std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
));
services.run =
services
.run
.with_sandbox(std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
)));
let outcome = handler
.execute(&node, &context, &graph, tmp.path(), &services)
@ -692,9 +700,12 @@ mod tests {
let tmp = TempDir::new().unwrap();
let mut services = EngineServices::test_default();
services.sandbox = std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
));
services.run =
services
.run
.with_sandbox(std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
)));
let outcome = handler
.execute(&node, &context, &graph, tmp.path(), &services)

View file

@ -89,7 +89,7 @@ impl Handler for CommandHandler {
script.to_string()
};
let stage_scope = StageScope::for_handler(context, &node.id);
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::CommandStarted {
node_id: node.id.clone(),
script: script.to_string(),
@ -106,9 +106,10 @@ impl Handler for CommandHandler {
} else {
Some(&services.env)
};
let cancel_token = services.sandbox_cancel_token();
let cancel_token = services.run.sandbox_cancel_token();
let result = services
.run
.sandbox
.exec_command(&command, timeout_ms, None, env_vars, cancel_token.clone())
.await;
@ -117,7 +118,7 @@ impl Handler for CommandHandler {
}
let result = result.map_err(|e| Error::handler(format!("Failed to spawn script: {e}")))?;
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::CommandCompleted {
node_id: node.id.clone(),
stdout: result.stdout.clone(),
@ -205,13 +206,13 @@ mod tests {
) {
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let services = EngineServices {
emitter: Arc::new(crate::event::Emitter::new(fixtures::RUN_1)),
run_store: run_store.clone().into(),
..EngineServices::test_default()
};
let mut services = EngineServices::test_default();
services.run = services
.run
.with_emitter(Arc::new(crate::event::Emitter::new(fixtures::RUN_1)))
.with_run_store(run_store.clone().into());
let logger = crate::event::StoreProgressLogger::new(run_store.clone());
logger.register(services.emitter.as_ref());
logger.register(services.run.emitter.as_ref());
(services, run_store, logger)
}
@ -806,7 +807,7 @@ mod tests {
fn make_spy_services(sandbox: std::sync::Arc<SpySandbox>) -> EngineServices {
let mut services = EngineServices::test_default();
services.sandbox = sandbox;
services.run = services.run.with_sandbox(sandbox);
services
}
@ -952,7 +953,9 @@ mod tests {
let run_dir = tempfile::tempdir().unwrap();
let mut services = make_spy_services(spy.clone());
services.cancel_requested = Some(Arc::new(AtomicBool::new(false)));
services.run = services
.run
.with_cancel_requested(Some(Arc::new(AtomicBool::new(false))));
handler
.execute(&node, &context, &graph, run_dir.path(), &services)

View file

@ -84,8 +84,8 @@ impl Handler for FanInHandler {
context,
run_dir,
&node.id,
&services.emitter,
&services.sandbox,
&services.run.emitter,
&services.run.sandbox,
)
.await?
} else {
@ -116,7 +116,7 @@ impl Handler for FanInHandler {
};
if let (Some(ref sha), Some(_)) = (&best_head_sha, services.git_state()) {
git_merge_ff_only(&*services.sandbox, sha).await;
git_merge_ff_only(&*services.run.sandbox, sha).await;
}
let mut outcome = Outcome::success();

View file

@ -262,7 +262,7 @@ impl Handler for HumanHandler {
let question_id = question.id.clone();
let stage_scope = StageScope::for_handler(context, &node.id);
self.emit(
&services.emitter,
&services.run.emitter,
&Event::InterviewStarted {
question_id: question_id.clone(),
question: question_text.clone(),
@ -282,14 +282,15 @@ impl Handler for HumanHandler {
},
&stage_scope,
);
self.tracker.interview_started(services.emitter.as_ref());
self.tracker
.interview_started(services.run.emitter.as_ref());
let interview_start = Instant::now();
let answer = self.interviewer.ask(question).await;
// 4. Handle timeout
if answer.value == AnswerValue::Timeout {
self.emit(
&services.emitter,
&services.run.emitter,
&Event::InterviewTimeout {
question_id: question_id.clone(),
question: question_text,
@ -298,7 +299,8 @@ impl Handler for HumanHandler {
},
&stage_scope,
);
self.tracker.interview_resolved(services.emitter.as_ref());
self.tracker
.interview_resolved(services.run.emitter.as_ref());
let default_choice = node
.attrs
.get("human.default_choice")
@ -320,6 +322,7 @@ impl Handler for HumanHandler {
// 5. Handle unanswered / interrupted interview sessions.
if answer.value == AnswerValue::Interrupted {
if services
.run
.cancel_requested
.as_ref()
.is_some_and(|flag| flag.load(Ordering::SeqCst))
@ -327,7 +330,7 @@ impl Handler for HumanHandler {
return Err(Error::Cancelled);
}
self.emit(
&services.emitter,
&services.run.emitter,
&Event::InterviewInterrupted {
question_id: question_id.clone(),
question: question_text,
@ -337,14 +340,15 @@ impl Handler for HumanHandler {
},
&stage_scope,
);
self.tracker.interview_resolved(services.emitter.as_ref());
self.tracker
.interview_resolved(services.run.emitter.as_ref());
return Ok(unanswered_human_gate(
"human interaction interrupted before an answer was provided",
));
}
if answer.value == AnswerValue::Skipped {
self.emit(
&services.emitter,
&services.run.emitter,
&Event::InterviewCompleted {
question_id,
question: question_text,
@ -353,13 +357,14 @@ impl Handler for HumanHandler {
},
&stage_scope,
);
self.tracker.interview_resolved(services.emitter.as_ref());
self.tracker
.interview_resolved(services.run.emitter.as_ref());
return Ok(unanswered_human_gate("human skipped interaction"));
}
// Emit interview completed for successful interactions
self.emit(
&services.emitter,
&services.run.emitter,
&Event::InterviewCompleted {
question_id,
question: question_text,
@ -368,7 +373,8 @@ impl Handler for HumanHandler {
},
&stage_scope,
);
self.tracker.interview_resolved(services.emitter.as_ref());
self.tracker
.interview_resolved(services.run.emitter.as_ref());
// 6. Try fixed-choice match
if let Some(selected) = find_choice_match(&answer, &choices) {
@ -477,7 +483,7 @@ mod tests {
.expect("event log lock poisoned")
.push(event.clone());
});
services.emitter = emitter;
services.run = services.run.with_emitter(emitter);
services
}

View file

@ -7,7 +7,7 @@ use fabro_agent::{
AgentEvent, AgentProfile, AnthropicProfile, GeminiProfile, OpenAiProfile, Sandbox, Session,
SessionOptions, Turn,
};
use fabro_auth::{CredentialResolver, CredentialUsage, ResolveError, ResolvedCredential};
use fabro_auth::{CredentialSource, EnvCredentialSource};
use fabro_graphviz::graph::Node;
use fabro_llm::client::Client;
use fabro_llm::types::{Message, Request, TokenCounts};
@ -35,65 +35,6 @@ fn build_profile(model: &str, provider: Provider) -> Box<dyn AgentProfile> {
}
}
pub(crate) struct LlmClientBuildResult {
pub(crate) client: Client,
pub(crate) auth_issues: Vec<(Provider, ResolveError)>,
}
pub(crate) async fn build_llm_client(
resolver: Option<&CredentialResolver>,
) -> Result<LlmClientBuildResult, Error> {
let Some(resolver) = resolver else {
let client = Client::from_env()
.await
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
return Ok(LlmClientBuildResult {
client,
auth_issues: Vec::new(),
});
};
let mut api_credentials = Vec::new();
let mut auth_issues = Vec::new();
for provider in Provider::ALL {
match resolver
.resolve(*provider, CredentialUsage::ApiRequest)
.await
{
Ok(ResolvedCredential::Api(credential)) => api_credentials.push(credential),
Ok(ResolvedCredential::Cli(_)) | Err(ResolveError::NotConfigured(_)) => {}
Err(err) => auth_issues.push((*provider, err)),
}
}
let client = Client::from_credentials(api_credentials)
.await
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
Ok(LlmClientBuildResult {
client,
auth_issues,
})
}
pub(crate) fn auth_issue_message(provider: Provider, err: &ResolveError) -> String {
match err {
ResolveError::NotConfigured(_) => {
format!("{} is not configured", provider.display_name())
}
ResolveError::RefreshFailed { source, .. } => format!(
"{} requires re-authentication: {}",
provider.display_name(),
source
),
ResolveError::RefreshTokenMissing(_) => format!(
"{} requires re-authentication: refresh token missing",
provider.display_name()
),
}
}
/// Shared state for tracking file modifications from agent tool calls.
struct FileTracking {
/// Maps tool_call_id → file_path for in-flight write/edit calls.
@ -180,7 +121,7 @@ pub struct AgentApiBackend {
sessions: Mutex<HashMap<String, Session>>,
env: HashMap<String, String>,
mcp_servers: Vec<McpServerSettings>,
resolver: Option<CredentialResolver>,
source: Arc<dyn CredentialSource>,
}
impl AgentApiBackend {
@ -189,7 +130,7 @@ impl AgentApiBackend {
model: String,
provider: Provider,
fallback_chain: Vec<FallbackTarget>,
resolver: CredentialResolver,
source: Arc<dyn CredentialSource>,
) -> Self {
Self {
model,
@ -198,7 +139,7 @@ impl AgentApiBackend {
sessions: Mutex::new(HashMap::new()),
env: HashMap::new(),
mcp_servers: Vec::new(),
resolver: Some(resolver),
source,
}
}
@ -208,15 +149,12 @@ impl AgentApiBackend {
provider: Provider,
fallback_chain: Vec<FallbackTarget>,
) -> Self {
Self {
Self::new(
model,
provider,
fallback_chain,
sessions: Mutex::new(HashMap::new()),
env: HashMap::new(),
mcp_servers: Vec::new(),
resolver: None,
}
Arc::new(EnvCredentialSource::new()),
)
}
#[must_use]
@ -247,7 +185,7 @@ impl AgentApiBackend {
provider,
node,
sandbox,
self.resolver.as_ref(),
self.source.as_ref(),
&self.env,
tool_hooks,
self.mcp_servers.clone(),
@ -260,12 +198,14 @@ impl AgentApiBackend {
provider: Provider,
node: &Node,
sandbox: &Arc<dyn Sandbox>,
resolver: Option<&CredentialResolver>,
source: &dyn CredentialSource,
env: &HashMap<String, String>,
tool_hooks: Option<Arc<dyn fabro_agent::ToolHookCallback>>,
mcp_servers: Vec<McpServerSettings>,
) -> Result<Session, Error> {
let client = build_llm_client(resolver).await?.client;
let client = Client::from_source(source)
.await
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
let mut profile = build_profile(model, provider);
@ -346,7 +286,9 @@ impl CodergenBackend for AgentApiBackend {
prompt: &str,
system_prompt: Option<&str>,
) -> Result<CodergenResult, Error> {
let client = build_llm_client(self.resolver.as_ref()).await?.client;
let client = Client::from_source(self.source.as_ref())
.await
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
let model = node.model().unwrap_or(&self.model);
let provider = node
@ -587,7 +529,7 @@ impl CodergenBackend for AgentApiBackend {
target_provider,
node,
sandbox,
self.resolver.as_ref(),
self.source.as_ref(),
&self.env,
tool_hooks.clone(),
self.mcp_servers.clone(),
@ -697,7 +639,7 @@ impl CodergenBackend for AgentApiBackend {
#[cfg(test)]
mod tests {
use fabro_agent::subagent::SessionFactory;
use fabro_auth::{AuthCredential, AuthDetails, CredentialResolver};
use fabro_auth::{AuthCredential, AuthDetails, VaultCredentialSource};
use fabro_vault::{SecretType, Vault};
use tokio::sync::RwLock as AsyncRwLock;
@ -847,7 +789,7 @@ mod tests {
}
#[tokio::test]
async fn build_llm_client_uses_resolver_credentials() {
async fn api_backend_uses_source_credentials() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
@ -864,14 +806,18 @@ mod tests {
None,
)
.unwrap();
let resolver = CredentialResolver::with_env_lookup(
Arc::new(AsyncRwLock::new(vault)),
Arc::new(|_| None),
let backend = AgentApiBackend::new(
"claude-opus-4-6".to_string(),
Provider::Anthropic,
Vec::new(),
Arc::new(VaultCredentialSource::with_env_lookup(
Arc::new(AsyncRwLock::new(vault)),
|_| None,
)),
);
let result = build_llm_client(Some(&resolver)).await.unwrap();
let client = Client::from_source(backend.source.as_ref()).await.unwrap();
assert_eq!(result.client.provider_names(), vec!["anthropic"]);
assert!(result.auth_issues.is_empty());
assert_eq!(client.provider_names(), vec!["anthropic"]);
}
}

View file

@ -206,7 +206,7 @@ impl Handler for SubWorkflowHandler {
run_dir: child_logs,
cancel_token: Some(cancel_token),
// Child workflows are part of the parent run's event stream.
run_id: services.emitter.run_id(),
run_id: services.run.emitter.run_id(),
labels: HashMap::new(),
workflow_slug: None,
github_app: None,
@ -227,10 +227,8 @@ impl Handler for SubWorkflowHandler {
}
let before_snapshot = context.snapshot();
let emitter = Arc::clone(&services.emitter);
let sandbox = Arc::clone(&services.sandbox);
let parent_run = Arc::clone(&services.run);
let registry = Arc::clone(&services.registry);
let hook_runner = services.hook_runner.clone();
let env = services.env.clone();
let inputs = services.inputs.clone();
let dry_run = services.dry_run;
@ -250,28 +248,29 @@ impl Handler for SubWorkflowHandler {
// Spawn child engine
let mut child_handle = tokio::spawn(async move {
let child_run = parent_run
.with_run_store(run_store.into())
.with_cancel_requested(None);
let initialized = Initialized {
graph: child_graph,
source: String::new(),
inputs,
run_options: child_run_options,
workflow_path: child_workflow_path,
workflow_bundle,
run_store: run_store.into(),
checkpoint: None,
seed_context: Some(child_context),
emitter,
sandbox,
registry,
on_node: None,
graph: child_graph,
source: String::new(),
run_options: child_run_options,
checkpoint: None,
seed_context: Some(child_context),
on_node: None,
artifact_sink: Some(ArtifactSink::Store(artifact_store)),
run_control: None,
hook_runner,
env,
dry_run,
llm_client: None,
model: String::new(),
provider: fabro_llm::Provider::Anthropic,
run_control: None,
engine: Arc::new(EngineServices {
run: child_run,
registry,
git_state: std::sync::RwLock::new(None),
env,
inputs,
dry_run,
workflow_path: child_workflow_path,
workflow_bundle,
}),
model: String::new(),
};
let executed = pipeline::execute(initialized).await;
Ok::<_, Error>((executed.outcome?, executed.final_context))

View file

@ -13,163 +13,18 @@ pub mod wait;
use std::any::Any;
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::path::Path;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::Duration;
use async_trait::async_trait;
use fabro_agent::Sandbox;
use fabro_graphviz::graph::{Graph, Node, shape_to_handler_type};
use fabro_hooks::{HookContext, HookDecision, HookRunner};
use fabro_interview::Interviewer;
use fabro_model::Provider;
#[cfg(test)]
use fabro_store::Database;
#[cfg(test)]
use object_store::memory::InMemory;
use tokio::time;
use tokio_util::sync::CancellationToken;
use crate::context::Context;
use crate::error::Error;
use crate::event::Emitter;
use crate::outcome::{Outcome, OutcomeExt};
use crate::runtime_store::RunStoreHandle;
use crate::sandbox_git::GitState;
use crate::workflow_bundle::WorkflowBundle;
/// Shared services available to all handlers during execution.
pub struct EngineServices {
pub registry: Arc<HandlerRegistry>,
pub emitter: Arc<Emitter>,
pub sandbox: Arc<dyn Sandbox>,
pub run_store: RunStoreHandle,
/// Git state for the current run. Set via `set_git_state` at the start of
/// `run_via_core` and read by parallel/fan-in handlers.
pub(crate) git_state: std::sync::RwLock<Option<Arc<GitState>>>,
/// Hook runner for user-defined lifecycle hooks.
pub hook_runner: Option<Arc<HookRunner>>,
/// Environment variables from `[sandbox.env]` config, injected into command
/// nodes.
pub env: HashMap<String, String>,
/// Typed values from `[run.inputs]`, available to prompt templates.
pub inputs: HashMap<String, toml::Value>,
/// When true, handlers should skip real execution and return simulated
/// results.
pub dry_run: bool,
/// Optional run-scoped cancellation flag from the core executor.
pub cancel_requested: Option<Arc<AtomicBool>>,
/// Resolved default provider for the current run.
pub provider: Provider,
/// Logical path of the current workflow when running from a bundle.
pub workflow_path: Option<PathBuf>,
/// Bundled workflows available for child-workflow resolution.
pub workflow_bundle: Option<Arc<WorkflowBundle>>,
}
impl EngineServices {
/// Read the current git state (if any).
pub fn git_state(&self) -> Option<Arc<GitState>> {
self.git_state.read().unwrap().clone()
}
/// Set the git state for the current run.
pub fn set_git_state(&self, state: Option<Arc<GitState>>) {
*self.git_state.write().unwrap() = state;
}
/// Bridge the core executor's atomic cancel flag to sandbox command
/// cancellation.
pub fn sandbox_cancel_token(&self) -> Option<CancellationToken> {
sandbox_cancel_token(self.cancel_requested.clone())
}
/// Run lifecycle hooks and return the merged decision.
/// Returns `Proceed` if no hook runner is configured.
pub async fn run_hooks(&self, hook_context: &HookContext) -> HookDecision {
let Some(ref runner) = self.hook_runner else {
return HookDecision::Proceed;
};
runner.run(hook_context, self.sandbox.clone(), None).await
}
/// Test-only default: empty registry, no hooks, local sandbox at cwd.
#[cfg(test)]
#[expect(
clippy::disallowed_methods,
reason = "This test helper must initialize a current-thread runtime safely from both sync tests and #[tokio::test]."
)]
pub fn test_default() -> Self {
let store = Arc::new(Database::new(
Arc::new(InMemory::new()),
"",
Duration::from_millis(1),
None,
));
Self {
registry: Arc::new(HandlerRegistry::new(Box::new(start::StartHandler))),
emitter: Arc::new(Emitter::default()),
sandbox: Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
// Build the test run store on a dedicated runtime so this helper
// remains safe to call from both sync tests and #[tokio::test].
run_store: std::thread::spawn(move || {
tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect("test runtime should initialize")
.block_on(async {
store
.create_run(&fabro_types::RunId::new())
.await
.expect("slate-backed test run store should initialize")
})
})
.join()
.expect("test run store thread should join")
.into(),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
inputs: HashMap::new(),
dry_run: false,
cancel_requested: None,
provider: Provider::Anthropic,
workflow_path: None,
workflow_bundle: None,
}
}
}
pub(crate) fn sandbox_cancel_token(
cancel_requested: Option<Arc<AtomicBool>>,
) -> Option<CancellationToken> {
let cancel_requested = cancel_requested?;
let token = CancellationToken::new();
if cancel_requested.load(Ordering::Relaxed) {
token.cancel();
return Some(token);
}
let token_clone = token.clone();
tokio::spawn(async move {
loop {
if token_clone.is_cancelled() {
return;
}
if cancel_requested.load(Ordering::Relaxed) {
token_clone.cancel();
return;
}
time::sleep(Duration::from_millis(10)).await;
}
});
Some(token)
}
pub(crate) use crate::services::sandbox_cancel_token;
pub use crate::services::{EngineServices, RunServices};
/// The handler interface for node execution.
#[async_trait]

View file

@ -152,7 +152,7 @@ impl Handler for ParallelHandler {
let parallel_stage_scope = StageScope::for_handler(context, &node.id);
let parallel_group_id = StageId::new(node.id.clone(), parallel_stage_scope.visit);
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::ParallelStarted {
node_id: node.id.clone(),
visit: parallel_stage_scope.visit,
@ -169,7 +169,7 @@ impl Handler for ParallelHandler {
let mut hook_ctx =
HookContext::new(HookEvent::ParallelStart, run_id, graph.name.clone());
set_hook_node(&mut hook_ctx, node);
let _ = services.run_hooks(&hook_ctx).await;
let _ = services.run.run_hooks(&hook_ctx).await;
}
let max_parallel = node
.attrs
@ -184,7 +184,7 @@ impl Handler for ParallelHandler {
// --- Git isolation: checkpoint "parallel base" before fan-out ---
let base_sha: Option<String> = if let Some(ref gs) = git_state {
let result = git_checkpoint(
&*services.sandbox,
&*services.run.sandbox,
&gs.run_id.to_string(),
&node.id,
"parallel_base",
@ -239,7 +239,7 @@ impl Handler for ParallelHandler {
);
// Compute worktree path (each sandbox type knows its own path scheme)
let wt_path_str = services.sandbox.parallel_worktree_path(
let wt_path_str = services.run.sandbox.parallel_worktree_path(
run_dir,
&gs.run_id.to_string(),
&node.id,
@ -254,8 +254,10 @@ impl Handler for ParallelHandler {
worktree_path: wt_path_str.clone(),
skip_branch_creation: false,
};
let mut wt_sandbox = WorktreeSandbox::new(Arc::clone(&services.sandbox), wt_config);
wt_sandbox.set_event_callback(Arc::clone(&services.emitter).worktree_callback());
let mut wt_sandbox =
WorktreeSandbox::new(Arc::clone(&services.run.sandbox), wt_config);
wt_sandbox
.set_event_callback(Arc::clone(&services.run.emitter).worktree_callback());
wt_sandbox
.initialize()
.await
@ -267,7 +269,7 @@ impl Handler for ParallelHandler {
let env: Arc<dyn Sandbox> = Arc::new(wt_sandbox);
(env, Some(wt_path))
} else {
(Arc::clone(&services.sandbox), None)
(Arc::clone(&services.run.sandbox), None)
};
branch_setups.push(BranchSetup {
@ -283,15 +285,11 @@ impl Handler for ParallelHandler {
// --- Fan out: concurrent execution ---
let mut handles = Vec::new();
for setup in branch_setups {
let parent_run = Arc::clone(&services.run);
let registry = Arc::clone(&services.registry);
let emitter = Arc::clone(&services.emitter);
let hook_runner = services.hook_runner.clone();
let run_store = services.run_store.clone();
let env = services.env.clone();
let inputs = services.inputs.clone();
let dry_run = services.dry_run;
let cancel_requested = services.cancel_requested.clone();
let provider = services.provider;
let workflow_path = services.workflow_path.clone();
let workflow_bundle = services.workflow_bundle.clone();
let graph = graph.clone();
@ -317,7 +315,7 @@ impl Handler for ParallelHandler {
.await
.map_err(|e| Error::handler(format!("semaphore error: {e}")))?;
emitter.emit_scoped(
parent_run.emitter.emit_scoped(
&Event::ParallelBranchStarted {
parallel_group_id: group_id.clone(),
parallel_branch_id: setup.parallel_branch_id.clone(),
@ -333,7 +331,7 @@ impl Handler for ParallelHandler {
"branch target node not found: {}",
setup.target_id
));
emitter.emit_scoped(
parent_run.emitter.emit_scoped(
&Event::ParallelBranchCompleted {
parallel_group_id: group_id.clone(),
parallel_branch_id: setup.parallel_branch_id.clone(),
@ -354,17 +352,12 @@ impl Handler for ParallelHandler {
};
let branch_services = EngineServices {
run: parent_run.with_sandbox(Arc::clone(&setup.sandbox)),
registry: Arc::clone(&registry),
emitter: Arc::clone(&emitter),
sandbox: Arc::clone(&setup.sandbox),
run_store: run_store.clone(),
git_state: std::sync::RwLock::new(None),
hook_runner: hook_runner.clone(),
env: env.clone(),
inputs: inputs.clone(),
dry_run,
cancel_requested,
provider,
workflow_path,
workflow_bundle,
};
@ -412,7 +405,7 @@ impl Handler for ParallelHandler {
match sha_result {
Ok(r) if r.exit_code == 0 => {
let sha = r.stdout.trim().to_string();
emitter.emit_scoped(
parent_run.emitter.emit_scoped(
&Event::GitCommit {
node_id: Some(setup.target_id.clone()),
sha: sha.clone(),
@ -427,7 +420,7 @@ impl Handler for ParallelHandler {
None
};
emitter.emit_scoped(
parent_run.emitter.emit_scoped(
&Event::ParallelBranchCompleted {
parallel_group_id: group_id.clone(),
parallel_branch_id: setup.parallel_branch_id.clone(),
@ -484,8 +477,9 @@ impl Handler for ParallelHandler {
for result in &results {
if let Some(ref wt_path) = result.worktree_path {
let wt_str = wt_path.to_string_lossy().into_owned();
git_remove_worktree(&*services.sandbox, &wt_str).await;
git_remove_worktree(&*services.run.sandbox, &wt_str).await;
services
.run
.emitter
.emit(&Event::GitWorktreeRemove { path: wt_str });
}
@ -502,7 +496,7 @@ impl Handler for ParallelHandler {
successful.sort_by(|a, b| a.id.cmp(&b.id));
if let Some(winner) = successful.first() {
if let Some(sha) = winner.head_sha.as_ref() {
git_merge_ff_only(&*services.sandbox, sha).await;
git_merge_ff_only(&*services.run.sandbox, sha).await;
}
}
}
@ -535,7 +529,7 @@ impl Handler for ParallelHandler {
context.set(keys::PARALLEL_RESULTS, serde_json::json!(results_json));
context.set(keys::PARALLEL_BRANCH_COUNT, serde_json::json!(total));
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::ParallelCompleted {
node_id: node.id.clone(),
visit: parallel_stage_scope.visit,
@ -554,7 +548,7 @@ impl Handler for ParallelHandler {
let mut hook_ctx =
HookContext::new(HookEvent::ParallelComplete, run_id, graph.name.clone());
set_hook_node(&mut hook_ctx, node);
let _ = services.run_hooks(&hook_ctx).await;
let _ = services.run.run_hooks(&hook_ctx).await;
}
// Evaluate join policy
@ -689,13 +683,13 @@ mod tests {
async fn parallel_handler_with_branches() {
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let services = EngineServices {
emitter: Arc::new(crate::event::Emitter::new(fixtures::RUN_1)),
run_store: run_store.clone().into(),
..EngineServices::test_default()
};
let mut services = EngineServices::test_default();
services.run = services
.run
.with_emitter(Arc::new(crate::event::Emitter::new(fixtures::RUN_1)))
.with_run_store(run_store.clone().into());
let logger = crate::event::StoreProgressLogger::new(run_store.clone());
logger.register(services.emitter.as_ref());
logger.register(services.run.emitter.as_ref());
let mut node = Node::new("par");
node.attrs.insert(
"shape".to_string(),
@ -741,13 +735,13 @@ mod tests {
async fn parallel_handler_stores_results_in_run_store() {
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let services = EngineServices {
emitter: Arc::new(crate::event::Emitter::new(fixtures::RUN_1)),
run_store: run_store.clone().into(),
..EngineServices::test_default()
};
let mut services = EngineServices::test_default();
services.run = services
.run
.with_emitter(Arc::new(crate::event::Emitter::new(fixtures::RUN_1)))
.with_run_store(run_store.clone().into());
let logger = crate::event::StoreProgressLogger::new(run_store.clone());
logger.register(services.emitter.as_ref());
logger.register(services.run.emitter.as_ref());
let mut node = Node::new("par");
node.attrs.insert(
"shape".to_string(),

View file

@ -61,13 +61,13 @@ impl Handler for PromptHandler {
// 1b. Discover project docs for system prompt when project_memory is enabled
let system_prompt = if node.project_memory() {
let working_dir = services.sandbox.working_directory();
let working_dir = services.run.sandbox.working_directory();
let provider = node
.provider()
.and_then(|s| s.parse::<Provider>().ok())
.unwrap_or(services.provider);
.unwrap_or(services.run.provider);
let docs = fabro_agent::discover_memory(
&*services.sandbox,
&*services.run.sandbox,
working_dir,
working_dir,
provider,
@ -86,10 +86,10 @@ impl Handler for PromptHandler {
let prompt_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.provider.as_str().to_string()));
.or_else(|| Some(services.run.provider.as_str().to_string()));
let prompt_model = node.model().map(String::from);
let stage_scope = StageScope::for_handler(context, &node.id);
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::Prompt {
stage: node.id.clone(),
visit: stage_scope.visit,
@ -138,10 +138,10 @@ impl Handler for PromptHandler {
let response_provider = node
.provider()
.map(String::from)
.or_else(|| Some(services.provider.as_str().to_string()))
.or_else(|| Some(services.run.provider.as_str().to_string()))
.unwrap_or_default();
services.emitter.emit_scoped(
services.run.emitter.emit_scoped(
&Event::PromptCompleted {
node_id: node.id.clone(),
response: response_text.clone(),
@ -208,13 +208,13 @@ mod tests {
) {
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let services = EngineServices {
emitter: Arc::new(crate::event::Emitter::new(fixtures::RUN_1)),
run_store: run_store.clone().into(),
..EngineServices::test_default()
};
let mut services = EngineServices::test_default();
services.run = services
.run
.with_emitter(Arc::new(crate::event::Emitter::new(fixtures::RUN_1)))
.with_run_store(run_store.clone().into());
let logger = crate::event::StoreProgressLogger::new(run_store.clone());
logger.register(services.emitter.as_ref());
logger.register(services.run.emitter.as_ref());
(services, run_store, logger)
}

View file

@ -150,6 +150,7 @@ pub mod run_options;
pub mod run_status;
pub mod runtime_store;
pub mod sandbox_git;
pub mod services;
#[doc(hidden)]
pub mod test_support;
#[doc(hidden)]

View file

@ -121,11 +121,11 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
)
.await
{
self.emitter.emit(&Event::RunNotice {
level: RunNoticeLevel::Warn,
code: "artifact_upload_failed".to_string(),
message: format!("[node: {node_id}] artifact upload failed: {err}"),
});
self.emitter.notice(
RunNoticeLevel::Warn,
"artifact_upload_failed",
format!("[node: {node_id}] artifact upload failed: {err}"),
);
return Ok(());
}
let scope = stage_scope_for(state, node_id);
@ -147,11 +147,11 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
}
Ok(_) => {} // no files collected
Err(e) => {
self.emitter.emit(&Event::RunNotice {
level: RunNoticeLevel::Warn,
code: "artifact_collection_failed".to_string(),
message: format!("[node: {node_id}] artifact collection failed: {e}"),
});
self.emitter.notice(
RunNoticeLevel::Warn,
"artifact_collection_failed",
format!("[node: {node_id}] artifact collection failed: {e}"),
);
}
}
@ -170,11 +170,11 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
if let Err(e) =
offload_large_values(&mut result.outcome.context_updates, &self.run_store).await
{
self.emitter.emit(&Event::RunNotice {
level: RunNoticeLevel::Warn,
code: "artifact_offload_failed".to_string(),
message: format!("[node: {node_id}] artifact offload failed: {e}"),
});
self.emitter.notice(
RunNoticeLevel::Warn,
"artifact_offload_failed",
format!("[node: {node_id}] artifact offload failed: {e}"),
);
}
normalize_durable_updates(&mut result.outcome.context_updates);
@ -183,11 +183,11 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
if let Err(e) =
sync_artifacts_to_env(&mut result.outcome.context_updates, &*self.sandbox).await
{
self.emitter.emit(&Event::RunNotice {
level: RunNoticeLevel::Warn,
code: "artifact_sync_failed".to_string(),
message: format!("[node: {node_id}] artifact sync failed: {e}"),
});
self.emitter.notice(
RunNoticeLevel::Warn,
"artifact_sync_failed",
format!("[node: {node_id}] artifact sync failed: {e}"),
);
}
Ok(())

View file

@ -43,8 +43,8 @@ impl NodeHandler<WorkflowGraph> for WorkflowNodeHandler {
let wf_context = artifact::resolve_context_for_execution(
context,
&self.services.run_store,
&*self.services.sandbox,
&self.services.run.run_store,
&*self.services.run.sandbox,
&self.run_dir,
)
.await

View file

@ -723,7 +723,7 @@ impl RunSession {
let mut initialized = Box::pin(pipeline::initialize(persisted, init_options)).await?;
initialized.on_node = on_node;
let sandbox_for_cleanup = Arc::clone(&initialized.sandbox);
let sandbox_for_cleanup = Arc::clone(&initialized.engine.run.sandbox);
let cleanup_guard = scopeguard::guard((), move |()| {
if preserve_sandbox {
return;
@ -745,17 +745,13 @@ impl RunSession {
let retro_opts = RetroOptions {
run_id: executed.run_options.run_id,
run_store: executed.run_store.clone(),
services: Arc::clone(&executed.engine.run),
workflow_name: executed.graph.name.clone(),
goal: executed.graph.goal().to_string(),
run_dir: executed.run_options.run_dir.clone(),
sandbox: Arc::clone(&executed.sandbox),
emitter: Some(Arc::clone(&executed.emitter)),
failed,
run_duration_ms: executed.duration_ms,
enabled: self.retro_enabled,
llm_client: executed.llm_client.clone(),
provider: executed.provider,
model: executed.model.clone(),
};
@ -766,15 +762,11 @@ impl RunSession {
let finalize_opts = FinalizeOptions {
run_dir: retroed.run_options.run_dir.clone(),
run_id: retroed.run_options.run_id,
run_store: retroed.run_store.clone(),
workflow_name: retroed.graph.name.clone(),
hook_runner: retroed.hook_runner.clone(),
preserve_sandbox: self.preserve_sandbox,
last_git_sha: last_git_sha.lock().unwrap().clone(),
};
let pr_opts = PullRequestOptions {
run_dir: retroed.run_options.run_dir.clone(),
run_store: retroed.run_store.clone(),
pr_config: self.pr_config,
github_app: self.pr_github_app,
origin_url: self.pr_origin_url,

View file

@ -13,7 +13,6 @@ use crate::context::{self, Context};
use crate::error::Error;
use crate::event::Event;
use crate::graph::WorkflowGraph;
use crate::handler::EngineServices;
use crate::lifecycle::WorkflowLifecycle;
use crate::node_handler::WorkflowNodeHandler;
use crate::outcome::{Outcome, StageStatus};
@ -37,29 +36,16 @@ pub async fn execute(init: Initialized) -> Executed {
let Initialized {
graph,
source: _,
inputs,
run_options,
workflow_path,
workflow_bundle,
run_store,
checkpoint,
seed_context,
emitter,
sandbox,
registry,
on_node,
artifact_sink,
run_control,
hook_runner,
env,
dry_run,
llm_client,
engine,
model,
provider,
} = init;
let service_inputs = inputs;
let mut checkpoint = checkpoint;
if let Some(cp) = checkpoint.as_mut() {
artifact::normalize_checkpoint_for_resume(cp);
@ -80,37 +66,22 @@ pub async fn execute(init: Initialized) -> Executed {
git_author: run_options.git_author(),
}))
});
let shared_services = Arc::new(EngineServices {
registry,
emitter: Arc::clone(&emitter),
sandbox: Arc::clone(&sandbox),
run_store: run_store.clone(),
git_state: std::sync::RwLock::new(git_state),
hook_runner: hook_runner.clone(),
env,
inputs: service_inputs,
dry_run,
cancel_requested: run_options.cancel_token.clone(),
provider,
workflow_path,
workflow_bundle,
});
engine.set_git_state(git_state);
let handler = Arc::new(WorkflowNodeHandler {
services: shared_services,
services: Arc::clone(&engine),
run_dir: run_options.run_dir.clone(),
graph: Arc::clone(&graph_arc),
});
let settings_arc = Arc::new(run_options.clone());
let lifecycle = WorkflowLifecycle::new(
&emitter,
hook_runner.clone(),
&sandbox,
&engine.run.emitter,
engine.run.hook_runner.clone(),
&engine.run.sandbox,
graph_arc,
&run_options.run_dir,
&run_store,
&engine.run.run_store,
artifact_sink,
&settings_arc,
checkpoint.is_some(),
@ -168,15 +139,10 @@ pub async fn execute(init: Initialized) -> Executed {
graph,
outcome: Err(err),
run_options,
run_store,
hook_runner,
emitter,
sandbox,
duration_ms: crate::millis_u64(start.elapsed()),
final_context: seed_context_from_checkpoint(checkpoint.as_ref()),
llm_client,
engine,
model,
provider,
};
}
}
@ -193,15 +159,10 @@ pub async fn execute(init: Initialized) -> Executed {
graph,
outcome: Err(err),
run_options,
run_store,
hook_runner,
emitter,
sandbox,
duration_ms: crate::millis_u64(start.elapsed()),
final_context: seed,
llm_client,
engine,
model,
provider,
};
}
}
@ -213,15 +174,10 @@ pub async fn execute(init: Initialized) -> Executed {
graph,
outcome: Err(err),
run_options,
run_store,
hook_runner,
emitter,
sandbox,
duration_ms: crate::millis_u64(start.elapsed()),
final_context: Context::new(),
llm_client,
engine,
model,
provider,
};
}
}
@ -243,7 +199,7 @@ pub async fn execute(init: Initialized) -> Executed {
let stall_shutdown =
if let (Some(stall_timeout), Some(ref token)) = (stall_timeout_opt, &stall_token) {
let shutdown = CancellationToken::new();
let emitter = Arc::clone(&emitter);
let emitter = Arc::clone(&engine.run.emitter);
let token_clone = token.clone();
let shutdown_clone = shutdown.clone();
emitter.touch();
@ -316,7 +272,7 @@ pub async fn execute(init: Initialized) -> Executed {
Err(fabro_core::Error::StallTimeout { node_id }) => {
let stall_timeout = graph.stall_timeout().unwrap_or_default();
let idle_secs = stall_timeout.as_secs();
emitter.emit(&Event::StallWatchdogTimeout {
engine.run.emitter.emit(&Event::StallWatchdogTimeout {
node: node_id.clone(),
idle_seconds: idle_secs,
});
@ -340,15 +296,10 @@ pub async fn execute(init: Initialized) -> Executed {
graph,
outcome,
run_options,
run_store,
hook_runner,
emitter,
sandbox,
duration_ms,
final_context,
llm_client,
engine,
model,
provider,
}
}

View file

@ -535,6 +535,8 @@ async fn execute_saves_checkpoint() {
let executed = execute_test_run(dir.path(), simple_graph(), "test-run").await;
assert!(
executed
.engine
.run
.run_store
.state()
.await
@ -586,6 +588,8 @@ async fn execute_mirrors_graph_goal_to_context() {
let dir = tempfile::tempdir().unwrap();
let executed = execute_test_run(dir.path(), simple_graph(), "test-run").await;
let cp = executed
.engine
.run
.run_store
.state()
.await
@ -632,6 +636,8 @@ async fn execute_conditional_routing_uses_unconditional_success_path() {
let executed = execute_test_run(dir.path(), g, "test-run").await;
let cp = executed
.engine
.run
.run_store
.state()
.await
@ -653,7 +659,7 @@ async fn execute_persists_start_record_and_node_status() {
});
let executed = execute_test_run_with_options(run_options, simple_graph(), None).await;
let state = executed.run_store.state().await.unwrap();
let state = executed.engine.run.run_store.state().await.unwrap();
let start = state.start.as_ref().unwrap();
assert_eq!(start.run_id, test_run_id("test-run"));
assert_eq!(
@ -712,7 +718,7 @@ async fn timeout_causes_fail_status_record() {
Some(Arc::new(registry)),
)
.await;
let state = executed.run_store.state().await.unwrap();
let state = executed.engine.run.run_store.state().await.unwrap();
let status = state
.node(&fabro_store::StageId::new("work", 1))
.unwrap()

View file

@ -1,11 +1,9 @@
use std::sync::Arc;
use fabro_hooks::{HookContext, HookEvent, HookRunner};
use fabro_hooks::{HookContext, HookEvent};
use fabro_types::{BilledTokenCounts, EventBody};
use super::types::{Concluded, FinalizeOptions, Retroed};
use crate::error::Error;
use crate::event::{Emitter, Event, RunNoticeLevel};
use crate::event::{Event, RunNoticeLevel};
use crate::git::MetadataStore;
use crate::outcome::{Outcome, OutcomeExt, StageStatus};
use crate::records::{Checkpoint, Conclusion, StageSummary};
@ -14,19 +12,7 @@ use crate::run_options::RunOptions;
use crate::run_status::{FailureReason, RunStatus, SuccessReason};
use crate::runtime_store::RunStoreHandle;
use crate::sandbox_git::{git_diff_with_timeout, git_push_host};
fn emit_run_notice(
emitter: &Emitter,
level: RunNoticeLevel,
code: impl Into<String>,
message: impl Into<String>,
) {
emitter.emit(&Event::RunNotice {
level,
code: code.into(),
message: message.into(),
});
}
use crate::services::RunServices;
pub fn classify_engine_result(
engine_result: &Result<Outcome, Error>,
@ -196,21 +182,19 @@ pub async fn write_finalize_commit(
/// workspace can't stall downstream consumers waiting on the terminal event.
async fn compute_final_patch(
run_options: &RunOptions,
sandbox: &dyn fabro_agent::Sandbox,
services: &RunServices,
status: StageStatus,
emitter: &Emitter,
) -> Option<String> {
let base_sha = run_options.git.as_ref().and_then(|g| g.base_sha.clone())?;
let timeout_ms = match status {
StageStatus::Success | StageStatus::PartialSuccess => 30_000,
_ => 10_000,
};
match git_diff_with_timeout(sandbox, &base_sha, timeout_ms).await {
match git_diff_with_timeout(&*services.sandbox, &base_sha, timeout_ms).await {
Ok(patch) if !patch.is_empty() => Some(patch),
Ok(_) => None,
Err(err) => {
emit_run_notice(
emitter,
services.emitter.notice(
RunNoticeLevel::Warn,
"git_diff_failed",
format!("final diff failed: {err}"),
@ -287,20 +271,8 @@ pub(crate) fn build_terminal_event(
}
}
async fn run_hooks(
hook_runner: Option<&HookRunner>,
hook_context: &HookContext,
sandbox: Arc<dyn fabro_agent::Sandbox>,
) {
let Some(runner) = hook_runner else {
return;
};
let _ = runner.run(hook_context, sandbox, None).await;
}
async fn cleanup_sandbox(
hook_runner: Option<Arc<HookRunner>>,
sandbox: Arc<dyn fabro_agent::Sandbox>,
services: &RunServices,
run_id: &fabro_types::RunId,
workflow_name: &str,
preserve: bool,
@ -310,9 +282,9 @@ async fn cleanup_sandbox(
*run_id,
workflow_name.to_string(),
);
run_hooks(hook_runner.as_deref(), &hook_ctx, Arc::clone(&sandbox)).await;
let _ = services.run_hooks(&hook_ctx).await;
if !preserve {
sandbox.cleanup().await?;
services.sandbox.cleanup().await?;
}
Ok(())
}
@ -331,23 +303,20 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
graph,
outcome,
run_options,
run_store: _run_store,
hook_runner,
emitter,
sandbox,
duration_ms,
services,
retro: _,
} = retroed;
let (final_status, failure_reason, _run_status) = classify_engine_result(&outcome);
let events = options.run_store.list_events().await.unwrap_or_default();
let events = services.run_store.list_events().await.unwrap_or_default();
let stage_durations = crate::extract_stage_durations_from_events(&events);
let artifact_count = events
.iter()
.filter(|envelope| matches!(envelope.event.body, EventBody::ArtifactCaptured(_)))
.count();
let checkpoint = options
let checkpoint = services
.run_store
.state()
.await
@ -362,9 +331,10 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
options.last_git_sha.clone(),
);
let final_patch = compute_final_patch(&run_options, &*sandbox, final_status, &emitter).await;
write_finalize_commit(&run_options, &options.run_store, &conclusion).await;
let (final_patch, ()) = tokio::join!(
compute_final_patch(&run_options, &services, final_status),
write_finalize_commit(&run_options, &services.run_store, &conclusion),
);
let terminal_event = build_terminal_event(
&outcome,
@ -374,29 +344,21 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
final_patch,
conclusion.billing.clone(),
);
emitter.emit(&terminal_event);
services.emitter.emit(&terminal_event);
if options.preserve_sandbox {
let info = sandbox.sandbox_info();
if info.is_empty() {
emit_run_notice(
&emitter,
RunNoticeLevel::Info,
"sandbox_preserved",
"sandbox preserved",
);
let info = services.sandbox.sandbox_info();
let message = if info.is_empty() {
"sandbox preserved".to_string()
} else {
emit_run_notice(
&emitter,
RunNoticeLevel::Info,
"sandbox_preserved",
format!("sandbox preserved: {info}"),
);
}
format!("sandbox preserved: {info}")
};
services
.emitter
.notice(RunNoticeLevel::Info, "sandbox_preserved", message);
}
if let Err(e) = cleanup_sandbox(
options.hook_runner.clone().or(hook_runner),
sandbox,
&services,
&options.run_id,
&options.workflow_name,
options.preserve_sandbox,
@ -404,8 +366,7 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
.await
{
tracing::warn!(error = %e, "Sandbox cleanup failed");
emit_run_notice(
&emitter,
services.emitter.notice(
RunNoticeLevel::Warn,
"sandbox_cleanup_failed",
format!("sandbox cleanup failed: {e}"),
@ -413,13 +374,11 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
}
Ok(Concluded {
run_id: run_options.run_id,
outcome,
conclusion,
pushed_branch: run_options.git.as_ref().and_then(|g| g.run_branch.clone()),
graph,
run_options,
emitter,
services,
})
}
@ -435,7 +394,7 @@ mod tests {
use object_store::memory::InMemory;
use super::*;
use crate::event::StoreProgressLogger;
use crate::event::{Emitter, StoreProgressLogger};
use crate::pipeline::types::Retroed;
use crate::run_options::RunOptions;
@ -478,26 +437,30 @@ mod tests {
let emitter = Arc::new(Emitter::new(test_run_id()));
let store_logger = StoreProgressLogger::new(run_store.clone());
store_logger.register(&emitter);
let services = RunServices::new(
run_store.clone().into(),
Arc::clone(&emitter),
Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap(),
)),
None,
None,
fabro_model::Provider::Anthropic,
Arc::new(fabro_auth::EnvCredentialSource::new()),
);
let retroed = Retroed {
graph: Graph::new("test"),
outcome: Ok(Outcome::success()),
run_options: test_run_options(&run_dir),
run_store: run_store.clone().into(),
hook_runner: None,
emitter,
sandbox: Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap(),
)),
duration_ms: 5,
services,
retro: None,
};
let concluded = finalize(retroed, &FinalizeOptions {
run_dir: run_dir.clone(),
run_id: test_run_id(),
run_store: run_store.clone().into(),
workflow_name: "test".to_string(),
hook_runner: None,
preserve_sandbox: true,
last_git_sha: None,
})

View file

@ -4,16 +4,19 @@ use std::sync::Arc;
use std::time::Instant;
use fabro_agent::Sandbox;
use fabro_auth::CredentialResolver;
use fabro_auth::{
CredentialResolver, CredentialSource, EnvCredentialSource, VaultCredentialSource,
auth_issue_message,
};
use fabro_config::RunScratch;
use fabro_graphviz::graph;
use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner};
use fabro_llm::client::Client;
use fabro_sandbox::{
ReadBeforeWriteSandbox, SandboxEventCallback, SandboxSpec, WorkdirStrategy, WorktreeOptions,
WorktreeSandbox,
};
use fabro_vault::Vault;
use futures::future::try_join_all;
use shlex::try_quote;
use tokio::process::Command as TokioCommand;
use tokio::runtime::Handle;
@ -26,10 +29,10 @@ use crate::devcontainer_bridge::{devcontainer_to_snapshot_config, run_devcontain
use crate::error::Error;
use crate::event::{Emitter, Event, RunNoticeLevel};
use crate::git::{self, GitSyncStatus, MetadataStore};
use crate::handler::llm::api::{auth_issue_message, build_llm_client};
use crate::handler::llm::{AgentApiBackend, AgentCliBackend, BackendRouter};
use crate::handler::{HandlerRegistry, default_registry, sandbox_cancel_token};
use crate::run_options::GitCheckpointOptions;
use crate::services::{EngineServices, RunServices};
struct WorktreePlan {
branch_name: String,
@ -50,19 +53,6 @@ async fn run_hooks(
runner.run(hook_context, sandbox, work_dir).await
}
fn emit_run_notice(
emitter: &Emitter,
level: RunNoticeLevel,
code: impl Into<String>,
message: impl Into<String>,
) {
emitter.emit(&Event::RunNotice {
level,
code: code.into(),
message: message.into(),
});
}
async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<WorktreePlan>, Error> {
let Some(worktree_mode) = options.worktree_mode else {
options.run_options.display_base_sha = None;
@ -111,8 +101,7 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<Workt
WorkdirStrategy::LocalDirectory => None,
};
if let Some(env_name) = env_name {
emit_run_notice(
&options.emitter,
options.emitter.notice(
RunNoticeLevel::Warn,
"dirty_worktree",
format!("Uncommitted changes will not be included in the {env_name}."),
@ -151,14 +140,12 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<Workt
})
.await
{
Ok(()) => emit_run_notice(
&options.emitter,
Ok(()) => options.emitter.notice(
RunNoticeLevel::Info,
"git_push_succeeded",
format!("{branch} (synced local commits to remote)"),
),
Err(e) => emit_run_notice(
&options.emitter,
Err(e) => options.emitter.notice(
RunNoticeLevel::Warn,
"git_push_failed",
format!("Failed to push {branch} to origin: {e}"),
@ -188,8 +175,7 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<Workt
}))
}
Err(e) => {
emit_run_notice(
&options.emitter,
options.emitter.notice(
RunNoticeLevel::Warn,
"worktree_setup_failed",
format!("Git worktree setup failed ({e}), running without worktree."),
@ -264,8 +250,7 @@ async fn build_sandbox_env(
Ok(token) => {
env.insert("GITHUB_TOKEN".to_string(), token);
}
Err(e) => emit_run_notice(
emitter,
Err(e) => emitter.notice(
RunNoticeLevel::Warn,
"github_token_failed",
format!("Failed to mint GitHub token: {e}"),
@ -283,12 +268,13 @@ async fn build_registry(
interviewer: Arc<dyn fabro_interview::Interviewer>,
sandbox_env: &HashMap<String, String>,
graph: &graph::Graph,
vault: Option<Arc<AsyncRwLock<Vault>>>,
) -> Result<(Arc<HandlerRegistry>, Option<Client>, bool), Error> {
llm_source: Arc<dyn CredentialSource>,
cli_resolver: Option<CredentialResolver>,
) -> Result<(Arc<HandlerRegistry>, bool), Error> {
let build_no_backend = || Arc::new(default_registry(Arc::clone(&interviewer), || None));
if spec.dry_run {
return Ok((build_no_backend(), None, true));
return Ok((build_no_backend(), true));
}
let graph_needs_llm = graph
@ -296,10 +282,8 @@ async fn build_registry(
.values()
.any(|n| graph::is_llm_handler_type(n.handler_type()));
let resolver = vault.map(CredentialResolver::new);
match build_llm_client(resolver.as_ref()).await {
Ok(result) if result.client.provider_names().is_empty() => {
match llm_source.resolve().await {
Ok(result) if result.credentials.is_empty() => {
if graph_needs_llm {
let detail = (!result.auth_issues.is_empty()).then(|| {
result
@ -317,38 +301,25 @@ async fn build_registry(
"{prefix}. Set ANTHROPIC_API_KEY or OPENAI_API_KEY, or pass --dry-run to simulate."
)));
}
Ok((build_no_backend(), None, false))
Ok((build_no_backend(), false))
}
Ok(result) => {
Ok(_result) => {
let env = sandbox_env.clone();
let model = spec.model.clone();
let provider = spec.provider;
let fallback_chain = spec.fallback_chain.clone();
let mcp_servers = spec.mcp_servers.clone();
let client = result.client;
let llm_source_for_api = Arc::clone(&llm_source);
let registry = Arc::new(default_registry(interviewer, move || {
let api = resolver
.clone()
.map_or_else(
|| {
AgentApiBackend::new_from_env(
model.clone(),
provider,
fallback_chain.clone(),
)
},
|resolver| {
AgentApiBackend::new(
model.clone(),
provider,
fallback_chain.clone(),
resolver,
)
},
)
.with_env(env.clone())
.with_mcp_servers(mcp_servers.clone());
let cli = resolver
let api = AgentApiBackend::new(
model.clone(),
provider,
fallback_chain.clone(),
Arc::clone(&llm_source_for_api),
)
.with_env(env.clone())
.with_mcp_servers(mcp_servers.clone());
let cli = cli_resolver
.clone()
.map_or_else(
|| AgentCliBackend::new_from_env(model.clone(), provider),
@ -357,7 +328,7 @@ async fn build_registry(
.with_env(env.clone());
Some(Box::new(BackendRouter::new(Box::new(api), cli)))
}));
Ok((registry, Some(client), false))
Ok((registry, false))
}
Err(e) => {
if graph_needs_llm {
@ -365,11 +336,18 @@ async fn build_registry(
"Failed to initialize LLM client: {e}. Set ANTHROPIC_API_KEY or OPENAI_API_KEY, or pass --dry-run to simulate.",
)));
}
Ok((build_no_backend(), None, false))
Ok((build_no_backend(), false))
}
}
}
fn build_llm_source(vault: Option<Arc<AsyncRwLock<Vault>>>) -> Arc<dyn CredentialSource> {
match vault {
Some(vault) => Arc::new(VaultCredentialSource::new(vault)),
None => Arc::new(EnvCredentialSource::new()),
}
}
async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), Error> {
let Some(devcontainer) = options.devcontainer.clone() else {
return Ok(());
@ -397,27 +375,15 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), Error> {
.apply_devcontainer_snapshot(devcontainer_to_snapshot_config(&config));
let timeout = std::time::Duration::from_mins(5);
for command in &config.initialize_commands {
let shell_commands = match command {
fabro_devcontainer::Command::Shell(shell) => vec![shell.clone()],
fabro_devcontainer::Command::Args(args) => {
vec![
args.iter()
.map(|arg| try_quote(arg).unwrap_or_else(|_| arg.into()).to_string())
.collect::<Vec<_>>()
.join(" "),
]
}
fabro_devcontainer::Command::Parallel(commands) => commands.values().cloned().collect(),
};
for shell_command in shell_commands {
let run_shell = |shell_command: String| {
let cwd = devcontainer.resolve_dir.clone();
async move {
let output = tokio_timeout(
timeout,
TokioCommand::new("sh")
.arg("-c")
.arg(&shell_command)
.current_dir(&devcontainer.resolve_dir)
.current_dir(&cwd)
.output(),
)
.await
@ -442,6 +408,25 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), Error> {
"Devcontainer initializeCommand failed (exit code {code}): {shell_command}\n{stderr}"
)));
}
Ok::<(), Error>(())
}
};
for command in &config.initialize_commands {
match command {
fabro_devcontainer::Command::Shell(shell) => run_shell(shell.clone()).await?,
fabro_devcontainer::Command::Args(args) => {
let shell_command = args
.iter()
.map(|arg| try_quote(arg).unwrap_or_else(|_| arg.into()).to_string())
.collect::<Vec<_>>()
.join(" ");
run_shell(shell_command).await?;
}
fabro_devcontainer::Command::Parallel(commands) => {
let futures = commands.values().cloned().map(&run_shell);
try_join_all(futures).await?;
}
}
}
@ -469,10 +454,16 @@ pub async fn initialize(
options.run_options.run_dir = run_dir.clone();
options.run_options.git = options.git.clone();
let llm_source = build_llm_source(options.vault.clone());
let cli_resolver = options.vault.clone().map(CredentialResolver::new);
let hook_runner = if options.hooks.hooks.is_empty() {
None
} else {
Some(Arc::new(HookRunner::new(options.hooks.clone())))
Some(Arc::new(HookRunner::new(
options.hooks.clone(),
Arc::clone(&llm_source),
)))
};
resolve_devcontainer(&mut options).await?;
@ -512,8 +503,7 @@ pub async fn initialize(
Arc::new(ReadBeforeWriteSandbox::new(Arc::new(worktree)))
}
Err(e) => {
emit_run_notice(
&options.emitter,
options.emitter.notice(
RunNoticeLevel::Warn,
"worktree_setup_failed",
format!("Git worktree setup failed ({e}), running without worktree."),
@ -584,20 +574,20 @@ pub async fn initialize(
&options.emitter,
)
.await?;
let (registry, llm_client, effective_dry_run) =
if let Some(registry) = options.registry_override.clone() {
// A caller-supplied registry owns execution behavior for its handlers.
(registry, None, options.dry_run)
} else {
build_registry(
&options.llm,
Arc::clone(&options.interviewer),
&env,
&graph,
options.vault.clone(),
)
.await?
};
let (registry, effective_dry_run) = if let Some(registry) = options.registry_override.clone() {
// A caller-supplied registry owns execution behavior for its handlers.
(registry, options.dry_run)
} else {
build_registry(
&options.llm,
Arc::clone(&options.interviewer),
&env,
&graph,
Arc::clone(&llm_source),
cli_resolver,
)
.await?
};
if effective_dry_run {
use fabro_types::settings::run::RunMode;
@ -710,30 +700,39 @@ pub async fn initialize(
.await?;
}
let run_services = RunServices::new(
options.run_store.clone(),
Arc::clone(&options.emitter),
Arc::clone(&sandbox),
hook_runner.clone(),
options.run_options.cancel_token.clone(),
options.llm.provider,
Arc::clone(&llm_source),
);
let engine = Arc::new(EngineServices {
run: Arc::clone(&run_services),
registry,
git_state: std::sync::RwLock::new(None),
env,
inputs: options.run_options.settings.run.inputs.clone(),
dry_run: options.dry_run,
workflow_path: options.workflow_path.clone(),
workflow_bundle: options.workflow_bundle.clone(),
});
scopeguard::ScopeGuard::into_inner(cleanup_guard);
Ok(Initialized {
graph,
source,
inputs: options.run_options.settings.run.inputs.clone(),
run_options: options.run_options,
workflow_path: options.workflow_path,
workflow_bundle: options.workflow_bundle,
run_store: options.run_store,
checkpoint: options.checkpoint,
seed_context: options.seed_context,
emitter: options.emitter,
sandbox,
registry,
on_node: None,
artifact_sink: options.artifact_sink,
run_control: options.run_control,
hook_runner,
env,
dry_run: options.dry_run,
llm_client,
engine,
model: options.llm.model,
provider: options.llm.provider,
})
}
@ -927,15 +926,28 @@ mod tests {
assert_eq!(initialized.run_options.run_dir, run_dir);
assert_eq!(initialized.source, source);
assert!(initialized.hook_runner.is_none());
assert!(initialized.engine.run.hook_runner.is_none());
assert_eq!(
initialized.env.get("TEST_KEY").map(String::as_str),
initialized.engine.env.get("TEST_KEY").map(String::as_str),
Some("value")
);
assert!(initialized.dry_run);
assert!(initialized.engine.dry_run);
assert_eq!(initialized.model, "test-model");
assert_eq!(initialized.provider, fabro_llm::Provider::Anthropic);
assert!(initialized.llm_client.is_none());
assert_eq!(
initialized.engine.run.provider,
fabro_llm::Provider::Anthropic
);
assert!(
initialized
.engine
.run
.llm_source
.resolve()
.await
.unwrap()
.credentials
.is_empty()
);
}
#[tokio::test]
@ -957,8 +969,9 @@ mod tests {
)
.unwrap();
let (graph, _) = llm_graph();
let vault = Arc::new(AsyncRwLock::new(vault));
let (_, llm_client, effective_dry_run) = build_registry(
let (_registry, effective_dry_run) = build_registry(
&LlmSpec {
model: "claude-opus-4-6".to_string(),
provider: fabro_llm::Provider::Anthropic,
@ -969,13 +982,13 @@ mod tests {
Arc::new(AutoApproveInterviewer),
&HashMap::new(),
&graph,
Some(Arc::new(AsyncRwLock::new(vault))),
Arc::new(VaultCredentialSource::new(Arc::clone(&vault))),
Some(CredentialResolver::new(vault)),
)
.await
.unwrap();
assert!(!effective_dry_run);
assert!(llm_client.unwrap().provider_names().contains(&"anthropic"));
}
#[tokio::test]

View file

@ -1,5 +1,9 @@
use std::sync::Arc;
use fabro_auth::CredentialSource;
use fabro_github::{self as github_app, GitHubCredentials, ssh_url_to_https};
use fabro_graphviz::parser;
use fabro_llm::client::Client;
use fabro_llm::generate::{GenerateParams, generate};
use fabro_retro::retro::Retro;
use fabro_store::RunProjection;
@ -9,7 +13,7 @@ use fabro_util::text::strip_goal_decoration;
use tracing::{debug, info};
use super::types::{Concluded, Finalized, PullRequestOptions};
use crate::event::{Emitter, Event, RunNoticeLevel};
use crate::event::{Event, RunNoticeLevel};
use crate::outcome::{StageStatus, format_cost as outcome_format_cost};
use crate::records::{Conclusion, RunSpec};
use crate::runtime_store::RunStoreHandle;
@ -270,19 +274,6 @@ fn assemble_pr_body(
parts.join("\n")
}
fn emit_run_notice(
emitter: &Emitter,
level: RunNoticeLevel,
code: impl Into<String>,
message: impl Into<String>,
) {
emitter.emit(&Event::RunNotice {
level,
code: code.into(),
message: message.into(),
});
}
async fn load_pull_request_diff(run_store: &RunStoreHandle) -> String {
run_store
.state()
@ -302,23 +293,26 @@ pub async fn build_pr_body(
goal: &str,
model: &str,
run_store: &RunStoreHandle,
llm_source: &dyn CredentialSource,
conclusion: Option<&Conclusion>,
) -> Result<String, String> {
let client = Client::from_source(llm_source)
.await
.map_err(|e| format!("Failed to create LLM client: {e}"))?;
build_pr_body_with_client(diff, goal, model, run_store, conclusion, Arc::new(client)).await
}
async fn build_pr_body_with_client(
diff: &str,
goal: &str,
model: &str,
run_store: &RunStoreHandle,
conclusion: Option<&Conclusion>,
client: Arc<Client>,
) -> Result<String, String> {
debug!("Building PR body");
let loaded_conclusion = if conclusion.is_none() {
run_store
.state()
.await
.inspect_err(|err| {
tracing::warn!(error = %err, "Failed to load conclusion from store for PR body");
})
.ok()
.and_then(|state| state.conclusion)
} else {
None
};
let conclusion = conclusion.or(loaded_conclusion.as_ref());
let run_state = run_store
.state()
.await
@ -326,6 +320,15 @@ pub async fn build_pr_body(
tracing::warn!(error = %err, "Failed to load run state from store for PR body");
})
.ok();
let loaded_conclusion = conclusion
.is_none()
.then(|| {
run_state
.as_ref()
.and_then(|state| state.conclusion.clone())
})
.flatten();
let conclusion = conclusion.or(loaded_conclusion.as_ref());
let plan_text = run_state.as_ref().and_then(read_plan_text);
let retro = run_state.as_ref().and_then(|state| state.retro.clone());
let run_spec = run_state.as_ref().and_then(|state| state.spec.clone());
@ -363,7 +366,9 @@ pub async fn build_pr_body(
format!("Goal: {goal}\n\nDiff:\n```\n{truncated_diff}\n```")
};
let params = GenerateParams::new(model).system(system).prompt(prompt);
let params = GenerateParams::new(model, client)
.system(system)
.prompt(prompt);
let result = generate(params)
.await
@ -413,6 +418,7 @@ pub async fn maybe_open_pull_request(
draft: bool,
auto_merge: Option<AutoMergeOptions>,
run_store: &RunStoreHandle,
llm_source: &dyn CredentialSource,
conclusion: Option<&Conclusion>,
) -> Result<Option<PullRequestRecord>, String> {
if diff.is_empty() {
@ -423,7 +429,7 @@ pub async fn maybe_open_pull_request(
let https_url = ssh_url_to_https(origin_url);
let (owner, repo) = github_app::parse_github_owner_repo(&https_url)?;
let body = build_pr_body(diff, goal, model, run_store, conclusion).await?;
let body = build_pr_body(diff, goal, model, run_store, llm_source, conclusion).await?;
let body = truncate_pr_body(&body);
let title = pr_title_from_goal(goal);
@ -491,13 +497,11 @@ pub async fn maybe_open_pull_request(
/// completes.
pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) -> Finalized {
let Concluded {
run_id,
outcome,
conclusion,
pushed_branch,
graph,
run_options,
emitter,
services,
} = concluded;
let mut pr_url = None;
@ -511,10 +515,10 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
result.status,
StageStatus::Success | StageStatus::PartialSuccess
) {
let diff = load_pull_request_diff(&options.run_store).await;
let diff = load_pull_request_diff(&services.run_store).await;
if let (Some(base_branch), Some(run_branch), Some(creds), Some(origin)) = (
&run_options.base_branch,
pushed_branch.as_deref(),
run_options.run_branch(),
&options.github_app,
&options.origin_url,
) {
@ -536,13 +540,14 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
&options.model,
pr_cfg.draft,
auto_merge,
&options.run_store,
&services.run_store,
services.llm_source.as_ref(),
Some(&conclusion),
)
.await
{
Ok(Some(record)) => {
emitter.emit(&Event::PullRequestCreated {
services.emitter.emit(&Event::PullRequestCreated {
pr_url: record.html_url.clone(),
pr_number: record.number,
owner: record.owner.clone(),
@ -556,9 +561,10 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
}
Ok(None) => {}
Err(e) => {
emitter.emit(&Event::PullRequestFailed { error: e.clone() });
emit_run_notice(
&emitter,
services
.emitter
.emit(&Event::PullRequestFailed { error: e.clone() });
services.emitter.notice(
RunNoticeLevel::Warn,
"pull_request_failed",
format!("PR creation failed: {e}"),
@ -571,10 +577,10 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
}
Finalized {
run_id,
run_id: run_options.run_id,
outcome,
conclusion,
pushed_branch,
pushed_branch: run_options.run_branch().map(str::to_string),
pr_url,
}
}
@ -583,34 +589,43 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
mod tests {
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::{Arc, Once};
use std::sync::Arc;
use std::time::Duration;
use chrono::Utc;
use fabro_auth::{
AuthCredential, AuthDetails, CredentialSource, EnvCredentialSource, VaultCredentialSource,
};
use fabro_graphviz::graph::Graph;
use fabro_llm::Error as LlmError;
use fabro_llm::client::Client;
use fabro_llm::provider::{ProviderAdapter, StreamEventStream};
use fabro_llm::types::{FinishReason, Message, Request, Response, StreamEvent, TokenCounts};
use fabro_llm::{Error as LlmError, set_default_client};
use fabro_retro::retro::{
AggregateStats, FrictionKind, FrictionPoint, OpenItem, OpenItemKind, StageRetro,
};
use fabro_store::Database;
use fabro_types::{BilledTokenCounts, RunSpec, SuccessReason, fixtures};
use fabro_vault::{SecretType, Vault};
use futures::stream;
use httpmock::Method::POST;
use httpmock::MockServer;
use object_store::memory::InMemory;
use tokio::sync::RwLock as AsyncRwLock;
use super::*;
use crate::event::{Event, append_event};
use crate::records::StageSummary;
struct MockProvider {
name: String,
response_text: String,
}
impl MockProvider {
fn new(text: &str) -> Self {
fn new(name: &str, text: &str) -> Self {
Self {
name: name.to_string(),
response_text: text.to_string(),
}
}
@ -619,7 +634,7 @@ mod tests {
#[async_trait::async_trait]
impl ProviderAdapter for MockProvider {
fn name(&self) -> &str {
"mock"
&self.name
}
async fn complete(&self, _request: &Request) -> Result<Response, LlmError> {
@ -681,17 +696,54 @@ mod tests {
))
}
fn install_mock_llm() {
static INIT: Once = Once::new();
fn explicit_client(provider_name: &str, text: &str) -> Arc<Client> {
let mut providers: HashMap<String, Arc<dyn ProviderAdapter>> = HashMap::new();
providers.insert(
provider_name.to_string(),
Arc::new(MockProvider::new(provider_name, text)),
);
Arc::new(Client::new(
providers,
Some(provider_name.to_string()),
vec![],
))
}
INIT.call_once(|| {
let mut providers: HashMap<String, Arc<dyn ProviderAdapter>> = HashMap::new();
providers.insert(
"mock".to_string(),
Arc::new(MockProvider::new("Narrative from mock.")),
);
set_default_client(Client::new(providers, Some("mock".to_string()), vec![]));
});
fn test_llm_source() -> Arc<dyn CredentialSource> {
Arc::new(EnvCredentialSource::new())
}
fn openai_api_key_credential(key: &str) -> AuthCredential {
AuthCredential {
provider: fabro_model::Provider::OpenAi,
details: AuthDetails::ApiKey {
key: key.to_string(),
},
}
}
fn openai_responses_payload(text: &str) -> serde_json::Value {
serde_json::json!({
"id": "resp_1",
"model": "gpt-5.4",
"output": [
{
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": text
}
]
}
],
"status": "completed",
"usage": {
"input_tokens": 10,
"output_tokens": 20
}
})
}
fn make_test_conclusion() -> Conclusion {
@ -1057,17 +1109,15 @@ mod tests {
#[tokio::test]
async fn build_pr_body_uses_in_memory_conclusion() {
install_mock_llm();
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let conclusion = make_test_conclusion();
let body = build_pr_body(
let body = build_pr_body_with_client(
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"mock-model",
&run_store.clone().into(),
Some(&conclusion),
Some(&make_test_conclusion()),
explicit_client("mock", "Narrative from mock."),
)
.await
.unwrap();
@ -1080,8 +1130,6 @@ mod tests {
#[tokio::test]
async fn build_pr_body_uses_store_records_without_legacy_files() {
install_mock_llm();
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
@ -1126,13 +1174,13 @@ mod tests {
.await
.unwrap();
let conclusion = make_test_conclusion();
let body = build_pr_body(
let body = build_pr_body_with_client(
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"mock-model",
&run_store.clone().into(),
Some(&conclusion),
Some(&make_test_conclusion()),
explicit_client("mock", "Narrative from mock."),
)
.await
.unwrap();
@ -1145,8 +1193,6 @@ mod tests {
#[tokio::test]
async fn build_pr_body_uses_plan_text_from_store_without_response_md() {
install_mock_llm();
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
@ -1208,12 +1254,13 @@ mod tests {
.await
.unwrap();
let body = build_pr_body(
let body = build_pr_body_with_client(
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"mock-model",
&run_store.clone().into(),
Some(&make_test_conclusion()),
explicit_client("mock", "Narrative from mock."),
)
.await
.unwrap();
@ -1222,6 +1269,78 @@ mod tests {
assert!(body.contains("Plan from store"));
}
#[tokio::test]
async fn build_pr_body_uses_explicit_llm_client() {
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let body = build_pr_body_with_client(
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"gpt-5.4",
&run_store.clone().into(),
Some(&make_test_conclusion()),
explicit_client("openai", "Narrative from explicit client."),
)
.await
.unwrap();
assert!(body.contains("Narrative from explicit client."));
assert!(!body.contains("Narrative from mock."));
}
#[tokio::test]
async fn build_pr_body_uses_vault_only_openai_codex_source() {
let server = MockServer::start_async().await;
let response_mock = server
.mock_async(|when, then| {
when.method(POST)
.path("/v1/responses")
.header("authorization", "Bearer vault-openai-key");
then.status(200)
.header("content-type", "application/json")
.json_body(openai_responses_payload("Narrative from vault source."));
})
.await;
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let base_url = server.url("/v1");
let llm_source: Arc<dyn CredentialSource> =
Arc::new(VaultCredentialSource::with_env_lookup(
Arc::new(AsyncRwLock::new(vault)),
move |name| match name {
"OPENAI_BASE_URL" => Some(base_url.clone()),
_ => None,
},
));
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let run_store_handle: RunStoreHandle = run_store.into();
let body = build_pr_body(
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"gpt-5.4",
&run_store_handle,
llm_source.as_ref(),
Some(&make_test_conclusion()),
)
.await
.unwrap();
assert!(body.contains("Narrative from vault source."));
response_mock.assert_async().await;
}
// ── parse_dot_summary tests ─────────────────────────────────────────
#[test]
@ -1341,6 +1460,8 @@ mod tests {
async fn empty_diff_returns_none() {
let store = test_store();
let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
let run_store_handle: RunStoreHandle = run_store.into();
let llm_source = test_llm_source();
let creds = GitHubCredentials::App(fabro_github::GitHubAppCredentials {
app_id: "123".to_string(),
private_key_pem: "unused".to_string(),
@ -1355,7 +1476,8 @@ mod tests {
"claude-sonnet-4-20250514",
false,
None,
&run_store.clone().into(),
&run_store_handle,
llm_source.as_ref(),
None,
)
.await;

View file

@ -1,6 +1,7 @@
use std::sync::Arc;
use fabro_agent::SessionEvent;
use fabro_llm::client::Client;
use fabro_retro::retro::{Retro, derive_retro};
use fabro_retro::retro_agent::{
RETRO_DATA_DIR, build_retro_prompt, dry_run_narrative, run_retro_agent,
@ -10,38 +11,40 @@ use super::types::{Executed, RetroOptions, Retroed};
use crate::event::Event;
pub async fn run_retro(options: &RetroOptions, dry_run: bool) -> Option<Retro> {
let state = match options.run_store.state().await {
let services = &options.services;
let state = match services.run_store.state().await {
Ok(state) => state,
Err(e) => {
tracing::warn!(error = %e, "Could not load run state, skipping retro");
if let Some(ref emitter) = options.emitter {
emitter.emit(&Event::RetroFailed {
error: e.to_string(),
duration_ms: 0,
});
}
services.emitter.emit(&Event::RetroFailed {
error: e.to_string(),
duration_ms: 0,
});
return None;
}
};
let Some(ref cp) = state.checkpoint else {
tracing::warn!("Could not load checkpoint, skipping retro");
if let Some(ref emitter) = options.emitter {
emitter.emit(&Event::RetroFailed {
error: "checkpoint not found".to_string(),
duration_ms: 0,
});
}
services.emitter.emit(&Event::RetroFailed {
error: "checkpoint not found".to_string(),
duration_ms: 0,
});
return None;
};
let completed_stages = crate::build_completed_stages(cp, options.failed);
let stage_durations = match options.run_store.list_events().await {
Ok(events) => crate::extract_stage_durations_from_events(&events),
let events = match services.run_store.list_events().await {
Ok(events) => events,
Err(err) => {
tracing::warn!(error = %err, "Could not load events from store, skipping stage durations");
std::collections::HashMap::default()
tracing::warn!(error = %err, "Could not load events from store, skipping retro");
services.emitter.emit(&Event::RetroFailed {
error: err.to_string(),
duration_ms: 0,
});
return None;
}
};
let stage_durations = crate::extract_stage_durations_from_events(&events);
let mut retro = derive_retro(
options.run_id,
&options.workflow_name,
@ -53,81 +56,63 @@ pub async fn run_retro(options: &RetroOptions, dry_run: bool) -> Option<Retro> {
let retro_start = std::time::Instant::now();
let retro_prompt = build_retro_prompt(RETRO_DATA_DIR);
if let Some(ref emitter) = options.emitter {
emitter.emit(&Event::RetroStarted {
prompt: Some(retro_prompt),
provider: Some(options.provider.as_str().to_string()),
model: Some(options.model.clone()),
});
}
services.emitter.emit(&Event::RetroStarted {
prompt: Some(retro_prompt),
provider: Some(services.provider.as_str().to_string()),
model: Some(options.model.clone()),
});
let retro_result = if dry_run {
Ok((dry_run_narrative(), String::new()))
} else if let Some(client) = options.llm_client.as_ref() {
let emitter_clone = options.emitter.clone();
let event_callback: Option<Arc<dyn Fn(SessionEvent) + Send + Sync>> =
emitter_clone.map(|emitter| -> Arc<dyn Fn(SessionEvent) + Send + Sync> {
Arc::new(move |event: SessionEvent| {
emitter.touch();
if !event.event.is_streaming_noise() {
emitter.emit(&Event::Agent {
stage: "retro".to_string(),
visit: 1,
event: event.event.clone(),
session_id: Some(event.session_id.clone()),
parent_session_id: event.parent_session_id.clone(),
});
}
})
});
let events = match options.run_store.list_events().await {
Ok(events) => events,
Err(err) => {
tracing::warn!(error = %err, "Could not load events from store, skipping retro");
if let Some(ref emitter) = options.emitter {
emitter.emit(&Event::RetroFailed {
error: err.to_string(),
duration_ms: 0,
});
}
return None;
}
};
run_retro_agent(
&options.sandbox,
&state,
&events,
&options.run_dir,
client,
options.provider,
&options.model,
event_callback,
)
.await
.map(|result| (result.narrative, result.response))
} else {
Err(anyhow::anyhow!("No LLM client available"))
match Client::from_source(services.llm_source.as_ref()).await {
Ok(client) => {
let emitter = Arc::clone(&services.emitter);
let event_callback: Arc<dyn Fn(SessionEvent) + Send + Sync> =
Arc::new(move |event: SessionEvent| {
emitter.touch();
if !event.event.is_streaming_noise() {
emitter.emit(&Event::Agent {
stage: "retro".to_string(),
visit: 1,
event: event.event.clone(),
session_id: Some(event.session_id.clone()),
parent_session_id: event.parent_session_id.clone(),
});
}
});
run_retro_agent(
&services.sandbox,
&state,
&events,
&options.run_dir,
&client,
services.provider,
&options.model,
Some(event_callback),
)
.await
.map(|result| (result.narrative, result.response))
}
Err(err) => Err(anyhow::anyhow!(err.to_string())),
}
};
let duration_ms = crate::millis_u64(retro_start.elapsed());
match retro_result {
Ok((narrative, response)) => {
retro.apply_narrative(narrative);
if let Some(ref emitter) = options.emitter {
emitter.emit(&Event::RetroCompleted {
duration_ms,
response: Some(response),
retro: serde_json::to_value(&retro).ok(),
});
}
services.emitter.emit(&Event::RetroCompleted {
duration_ms,
response: Some(response),
retro: serde_json::to_value(&retro).ok(),
});
}
Err(e) => {
if let Some(ref emitter) = options.emitter {
emitter.emit(&Event::RetroFailed {
error: e.to_string(),
duration_ms,
});
}
services.emitter.emit(&Event::RetroFailed {
error: e.to_string(),
duration_ms,
});
tracing::debug!(error = %e, "Retro agent skipped");
}
}
@ -144,15 +129,10 @@ pub async fn retro(executed: Executed, options: &RetroOptions) -> Retroed {
graph,
outcome,
run_options,
run_store,
hook_runner,
emitter,
sandbox,
duration_ms,
final_context: _,
llm_client: _,
engine,
model: _,
provider: _,
} = executed;
let dry_run = run_options.dry_run_enabled();
@ -167,11 +147,8 @@ pub async fn retro(executed: Executed, options: &RetroOptions) -> Retroed {
graph,
outcome,
run_options,
run_store,
hook_runner,
emitter,
sandbox,
duration_ms,
services: Arc::clone(&engine.run),
retro,
}
}
@ -182,6 +159,7 @@ mod tests {
use std::sync::{Arc, Mutex};
use std::time::Duration;
use fabro_auth::{CredentialSource, EnvCredentialSource};
use fabro_graphviz::graph::Graph;
use fabro_store::Database;
use fabro_types::{RunId, WorkflowSettings, fixtures};
@ -193,6 +171,7 @@ mod tests {
use crate::pipeline::types::Executed;
use crate::records::{Checkpoint, CheckpointExt, RunSpec};
use crate::run_options::RunOptions;
use crate::services::{EngineServices, RunServices};
fn test_run_id() -> RunId {
fixtures::RUN_1
@ -310,6 +289,10 @@ mod tests {
}
}
fn test_llm_source() -> Arc<dyn CredentialSource> {
Arc::new(EnvCredentialSource::new())
}
#[tokio::test]
async fn retro_phase_persists_retro_in_projection() {
let temp = tempfile::tempdir().unwrap();
@ -324,34 +307,36 @@ mod tests {
let sandbox: Arc<dyn fabro_agent::Sandbox> = Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap(),
));
let services = RunServices::new(
run_store.clone().into(),
Arc::clone(&emitter),
Arc::clone(&sandbox),
None,
None,
fabro_llm::Provider::Anthropic,
test_llm_source(),
);
let mut engine = EngineServices::test_default();
engine.run = Arc::clone(&services);
let executed = Executed {
graph: Graph::new("test"),
outcome: Ok(crate::outcome::Outcome::success()),
run_options: test_run_options(&run_dir),
run_store: run_store.clone().into(),
hook_runner: None,
emitter: Arc::clone(&emitter),
sandbox: Arc::clone(&sandbox),
duration_ms: 1,
final_context: Context::new(),
llm_client: None,
engine: Arc::new(engine),
model: "test-model".to_string(),
provider: fabro_llm::Provider::Anthropic,
};
let retroed = retro(executed, &RetroOptions {
run_id: test_run_id(),
run_store: run_store.into(),
services,
workflow_name: "test".to_string(),
goal: "Ship it".to_string(),
run_dir: run_dir.clone(),
sandbox,
emitter: Some(emitter),
failed: false,
run_duration_ms: 1,
enabled: true,
llm_client: None,
provider: fabro_llm::Provider::Anthropic,
model: "test-model".to_string(),
})
.await;
@ -373,24 +358,29 @@ mod tests {
let seen = Arc::clone(&seen);
move |event| seen.lock().unwrap().push(event.clone())
});
let services = RunServices::new(
test_run_store(&run_dir, &checkpoint).await.into(),
Arc::clone(&emitter),
Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap(),
)),
None,
None,
fabro_llm::Provider::Anthropic,
test_llm_source(),
);
let retro = run_retro(
&RetroOptions {
run_id: test_run_id(),
run_store: test_run_store(&run_dir, &checkpoint).await.into(),
workflow_name: "test".to_string(),
goal: "Ship it".to_string(),
run_dir: run_dir.clone(),
sandbox: Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap(),
)),
emitter: Some(Arc::clone(&emitter)),
failed: false,
run_id: test_run_id(),
services,
workflow_name: "test".to_string(),
goal: "Ship it".to_string(),
run_dir: run_dir.clone(),
failed: false,
run_duration_ms: 1,
enabled: true,
llm_client: None,
provider: fabro_llm::Provider::Anthropic,
model: "test-model".to_string(),
enabled: true,
model: "test-model".to_string(),
},
true,
)

View file

@ -2,12 +2,9 @@ use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use fabro_agent::Sandbox;
use fabro_graphviz::graph::Graph;
use fabro_hooks::HookRunner;
use fabro_interview::Interviewer;
use fabro_llm::Provider;
use fabro_llm::client::Client;
use fabro_mcp::config::McpServerSettings;
use fabro_model::FallbackTarget;
use fabro_retro::retro::Retro;
@ -30,6 +27,7 @@ use crate::records::{Checkpoint, Conclusion, RunSpec};
use crate::run_control::RunControlState;
use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions};
use crate::runtime_store::RunStoreHandle;
use crate::services::{EngineServices, RunServices};
use crate::transforms::Transform;
use crate::workflow_bundle::WorkflowBundle;
@ -263,25 +261,14 @@ pub struct InitOptions {
pub struct Initialized {
pub graph: Graph,
pub source: String,
pub inputs: HashMap<String, toml::Value>,
pub run_options: RunOptions,
pub workflow_path: Option<PathBuf>,
pub workflow_bundle: Option<Arc<WorkflowBundle>>,
pub run_store: RunStoreHandle,
pub(crate) checkpoint: Option<Checkpoint>,
pub(crate) seed_context: Option<Context>,
pub emitter: Arc<Emitter>,
pub sandbox: Arc<dyn Sandbox>,
pub registry: Arc<HandlerRegistry>,
pub on_node: crate::OnNodeCallback,
pub artifact_sink: Option<ArtifactSink>,
pub run_control: Option<Arc<RunControlState>>,
pub hook_runner: Option<Arc<HookRunner>>,
pub env: HashMap<String, String>,
pub dry_run: bool,
pub llm_client: Option<Client>,
pub engine: Arc<EngineServices>,
pub model: String,
pub provider: Provider,
}
/// Output of the EXECUTE phase.
@ -290,15 +277,10 @@ pub struct Executed {
pub graph: Graph,
pub outcome: Result<Outcome, Error>,
pub run_options: RunOptions,
pub run_store: RunStoreHandle,
pub hook_runner: Option<Arc<HookRunner>>,
pub emitter: Arc<Emitter>,
pub sandbox: Arc<dyn Sandbox>,
pub duration_ms: u64,
pub final_context: Context,
pub llm_client: Option<Client>,
pub engine: Arc<EngineServices>,
pub model: String,
pub provider: Provider,
}
/// Output of the RETRO phase.
@ -307,24 +289,19 @@ pub struct Retroed {
pub graph: Graph,
pub outcome: Result<Outcome, Error>,
pub run_options: RunOptions,
pub run_store: RunStoreHandle,
pub hook_runner: Option<Arc<HookRunner>>,
pub emitter: Arc<Emitter>,
pub sandbox: Arc<dyn Sandbox>,
pub duration_ms: u64,
pub services: Arc<RunServices>,
pub retro: Option<Retro>,
}
/// Output of the FINALIZE phase.
#[non_exhaustive]
pub struct Concluded {
pub run_id: RunId,
pub outcome: Result<Outcome, Error>,
pub conclusion: Conclusion,
pub pushed_branch: Option<String>,
pub graph: Graph,
pub run_options: RunOptions,
pub emitter: Arc<Emitter>,
pub outcome: Result<Outcome, Error>,
pub conclusion: Conclusion,
pub graph: Graph,
pub run_options: RunOptions,
pub services: Arc<RunServices>,
}
/// Output of the PULL_REQUEST phase.
@ -348,17 +325,13 @@ pub struct TransformOptions {
/// Options for the RETRO phase.
pub struct RetroOptions {
pub run_id: RunId,
pub run_store: RunStoreHandle,
pub services: Arc<RunServices>,
pub workflow_name: String,
pub goal: String,
pub run_dir: PathBuf,
pub sandbox: Arc<dyn Sandbox>,
pub emitter: Option<Arc<Emitter>>,
pub failed: bool,
pub run_duration_ms: u64,
pub enabled: bool,
pub llm_client: Option<Client>,
pub provider: Provider,
pub model: String,
}
@ -366,17 +339,13 @@ pub struct RetroOptions {
pub struct FinalizeOptions {
pub run_dir: PathBuf,
pub run_id: RunId,
pub run_store: RunStoreHandle,
pub workflow_name: String,
pub hook_runner: Option<Arc<HookRunner>>,
pub preserve_sandbox: bool,
pub last_git_sha: Option<String>,
}
/// Options for the PULL_REQUEST phase.
pub struct PullRequestOptions {
pub run_dir: PathBuf,
pub run_store: RunStoreHandle,
pub pr_config: Option<PullRequestSettings>,
pub github_app: Option<fabro_github::GitHubCredentials>,
pub origin_url: Option<String>,

View file

@ -57,6 +57,11 @@ impl RunOptions {
pub fn artifact_globs(&self) -> Vec<String> {
self.settings.run.artifacts.include.clone()
}
/// Run branch name from git checkpoint options, if set.
pub fn run_branch(&self) -> Option<&str> {
self.git.as_ref().and_then(|g| g.run_branch.as_deref())
}
}
/// Options for sandbox lifecycle management within the engine.

View file

@ -0,0 +1,259 @@
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::Duration;
use fabro_agent::Sandbox;
use fabro_auth::CredentialSource;
#[cfg(test)]
use fabro_auth::ResolvedCredentials;
use fabro_hooks::{HookContext, HookDecision, HookRunner};
use fabro_model::Provider;
use tokio::time;
use tokio_util::sync::CancellationToken;
use crate::event::Emitter;
use crate::handler::HandlerRegistry;
use crate::runtime_store::RunStoreHandle;
use crate::sandbox_git::GitState;
use crate::workflow_bundle::WorkflowBundle;
/// Services shared across workflow phases.
#[derive(Clone)]
pub struct RunServices {
pub run_store: RunStoreHandle,
pub emitter: Arc<Emitter>,
pub sandbox: Arc<dyn Sandbox>,
pub hook_runner: Option<Arc<HookRunner>>,
pub cancel_requested: Option<Arc<AtomicBool>>,
pub provider: Provider,
pub llm_source: Arc<dyn CredentialSource>,
}
impl RunServices {
#[must_use]
pub fn new(
run_store: RunStoreHandle,
emitter: Arc<Emitter>,
sandbox: Arc<dyn Sandbox>,
hook_runner: Option<Arc<HookRunner>>,
cancel_requested: Option<Arc<AtomicBool>>,
provider: Provider,
llm_source: Arc<dyn CredentialSource>,
) -> Arc<Self> {
Arc::new(Self {
run_store,
emitter,
sandbox,
hook_runner,
cancel_requested,
provider,
llm_source,
})
}
/// Bridge the core executor's atomic cancel flag to sandbox command
/// cancellation.
pub fn sandbox_cancel_token(&self) -> Option<CancellationToken> {
sandbox_cancel_token(self.cancel_requested.clone())
}
/// Run lifecycle hooks and return the merged decision.
/// Returns `Proceed` if no hook runner is configured.
pub async fn run_hooks(&self, hook_context: &HookContext) -> HookDecision {
let Some(ref runner) = self.hook_runner else {
return HookDecision::Proceed;
};
runner
.run(hook_context, Arc::clone(&self.sandbox), None)
.await
}
#[must_use]
pub fn with_run_store(self: &Arc<Self>, run_store: RunStoreHandle) -> Arc<Self> {
Arc::new(Self {
run_store,
..self.as_ref().clone()
})
}
#[must_use]
pub fn with_emitter(self: &Arc<Self>, emitter: Arc<Emitter>) -> Arc<Self> {
Arc::new(Self {
emitter,
..self.as_ref().clone()
})
}
#[must_use]
pub fn with_sandbox(self: &Arc<Self>, sandbox: Arc<dyn Sandbox>) -> Arc<Self> {
Arc::new(Self {
sandbox,
..self.as_ref().clone()
})
}
#[must_use]
pub fn with_cancel_requested(
self: &Arc<Self>,
cancel_requested: Option<Arc<AtomicBool>>,
) -> Arc<Self> {
Arc::new(Self {
cancel_requested,
..self.as_ref().clone()
})
}
}
/// Services available only while executing workflow nodes.
pub struct EngineServices {
pub run: Arc<RunServices>,
pub registry: Arc<HandlerRegistry>,
/// Git state for the current run. Set via `set_git_state` at the start of
/// `execute` and read by parallel/fan-in handlers.
pub(crate) git_state: std::sync::RwLock<Option<Arc<GitState>>>,
/// Environment variables from `[sandbox.env]` config, injected into command
/// nodes.
pub env: HashMap<String, String>,
/// Typed values from `[run.inputs]`, available to prompt templates.
pub inputs: HashMap<String, toml::Value>,
/// When true, handlers should skip real execution and return simulated
/// results.
pub dry_run: bool,
/// Logical path of the current workflow when running from a bundle.
pub workflow_path: Option<PathBuf>,
/// Bundled workflows available for child-workflow resolution.
pub workflow_bundle: Option<Arc<WorkflowBundle>>,
}
impl EngineServices {
/// Read the current git state (if any).
pub fn git_state(&self) -> Option<Arc<GitState>> {
self.git_state.read().unwrap().clone()
}
/// Set the git state for the current run.
pub fn set_git_state(&self, state: Option<Arc<GitState>>) {
*self.git_state.write().unwrap() = state;
}
/// Test-only default: empty registry and cross-phase services.
#[cfg(test)]
#[expect(
clippy::disallowed_methods,
reason = "Test scaffolding must build a slate-backed run store from sync code."
)]
pub fn test_default() -> Self {
use fabro_store::Database;
use object_store::memory::InMemory;
use crate::handler::start;
#[derive(Debug, Default)]
struct StubCredentialSource;
#[async_trait::async_trait]
impl CredentialSource for StubCredentialSource {
async fn resolve(&self) -> anyhow::Result<ResolvedCredentials> {
Ok(ResolvedCredentials {
credentials: Vec::new(),
auth_issues: Vec::new(),
})
}
async fn configured_providers(&self) -> Vec<Provider> {
Vec::new()
}
}
let store = Arc::new(Database::new(
Arc::new(InMemory::new()),
"",
Duration::from_millis(1),
None,
));
let run_store = std::thread::spawn(move || {
tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect("test runtime should initialize")
.block_on(async {
store
.create_run(&fabro_types::RunId::new())
.await
.expect("slate-backed test run store should initialize")
})
})
.join()
.expect("test run store thread should join");
Self {
run: RunServices::new(
run_store.into(),
Arc::new(Emitter::default()),
Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
)),
None,
None,
Provider::Anthropic,
Arc::new(StubCredentialSource),
),
registry: Arc::new(HandlerRegistry::new(Box::new(start::StartHandler))),
git_state: std::sync::RwLock::new(None),
env: HashMap::new(),
inputs: HashMap::new(),
dry_run: false,
workflow_path: None,
workflow_bundle: None,
}
}
}
pub(crate) fn sandbox_cancel_token(
cancel_requested: Option<Arc<AtomicBool>>,
) -> Option<CancellationToken> {
let cancel_requested = cancel_requested?;
let token = CancellationToken::new();
if cancel_requested.load(Ordering::Relaxed) {
token.cancel();
return Some(token);
}
let token_clone = token.clone();
tokio::spawn(async move {
loop {
if token_clone.is_cancelled() {
return;
}
if cancel_requested.load(Ordering::Relaxed) {
token_clone.cancel();
return;
}
time::sleep(Duration::from_millis(10)).await;
}
});
Some(token)
}
#[cfg(test)]
mod tests {
use super::EngineServices;
#[tokio::test]
async fn test_default_uses_stub_credential_source() {
let services = EngineServices::test_default();
assert!(
services
.run
.llm_source
.configured_providers()
.await
.is_empty()
);
}
}

View file

@ -5,6 +5,7 @@ use std::sync::Arc;
use std::time::Duration;
use fabro_agent::Sandbox;
use fabro_auth::{CredentialSource, EnvCredentialSource};
use fabro_graphviz::graph::Graph as GvGraph;
use fabro_store::{ArtifactStore, Database, RunProjection};
use object_store::local::LocalFileSystem;
@ -19,6 +20,7 @@ use crate::pipeline::types::{Executed, Initialized};
use crate::pipeline::{billing_from_checkpoint, build_terminal_event};
use crate::records::Checkpoint;
use crate::run_options::RunOptions;
use crate::services::{EngineServices, RunServices};
/// These helpers stop at EXECUTE, so they emit the terminal event here to
/// keep test consumers seeing the same end-of-run signal as production
@ -31,7 +33,7 @@ use crate::run_options::RunOptions;
async fn execute_and_emit_terminal(initialized: InitializedState) -> Executed {
let executed = Box::pin(pipeline::execute(initialized.initialized)).await;
initialized.store_logger.flush().await;
let state = executed.run_store.state().await.ok();
let state = executed.engine.run.run_store.state().await.ok();
let billing = state
.as_ref()
.and_then(|s| s.checkpoint.as_ref())
@ -44,7 +46,7 @@ async fn execute_and_emit_terminal(initialized: InitializedState) -> Executed {
None,
billing,
);
executed.emitter.emit(&event);
executed.engine.run.emitter.emit(&event);
initialized.store_logger.flush().await;
executed
}
@ -62,6 +64,7 @@ struct InitializedOptions {
hook_runner: Option<Arc<fabro_hooks::HookRunner>>,
env: HashMap<String, String>,
checkpoint: Option<Checkpoint>,
llm_source: Option<Arc<dyn CredentialSource>>,
}
struct InitializedState {
@ -143,27 +146,35 @@ async fn initialized(
);
InitializedState {
initialized: Initialized {
graph: graph.clone(),
source: String::new(),
inputs: run_options.settings.run.inputs.clone(),
run_options: run_options.clone(),
workflow_path: None,
workflow_bundle: None,
run_store: run_store.into(),
checkpoint: options.checkpoint,
seed_context: None,
emitter,
sandbox,
registry: Arc::new(registry),
on_node: None,
graph: graph.clone(),
source: String::new(),
run_options: run_options.clone(),
checkpoint: options.checkpoint,
seed_context: None,
on_node: None,
artifact_sink: Some(ArtifactSink::Store(artifact_store)),
run_control: None,
hook_runner: options.hook_runner,
env: options.env,
dry_run: run_options.dry_run_enabled(),
llm_client: None,
model: String::new(),
provider: fabro_llm::Provider::Anthropic,
run_control: None,
engine: Arc::new(EngineServices {
run: RunServices::new(
run_store.into(),
emitter,
sandbox,
options.hook_runner,
run_options.cancel_token.clone(),
fabro_llm::Provider::Anthropic,
options
.llm_source
.unwrap_or_else(|| Arc::new(EnvCredentialSource::new())),
),
registry: Arc::new(registry),
git_state: std::sync::RwLock::new(None),
env: options.env,
inputs: run_options.settings.run.inputs.clone(),
dry_run: run_options.dry_run_enabled(),
workflow_path: None,
workflow_bundle: None,
}),
model: String::new(),
},
store_logger,
}
@ -186,6 +197,7 @@ pub async fn run_graph(
hook_runner: None,
env: HashMap::new(),
checkpoint: None,
llm_source: None,
},
)
.await;
@ -210,12 +222,15 @@ pub async fn run_graph_with_state(
hook_runner: None,
env: HashMap::new(),
checkpoint: None,
llm_source: None,
},
)
.await;
let executed = execute_and_emit_terminal(initialized).await;
let outcome = executed.outcome?;
let state = executed
.engine
.run
.run_store
.state()
.await
@ -242,6 +257,7 @@ pub async fn run_graph_with_hooks(
hook_runner: Some(hook_runner),
env: env.unwrap_or_default(),
checkpoint: None,
llm_source: None,
},
)
.await;
@ -268,12 +284,15 @@ pub async fn run_graph_with_hooks_and_state(
hook_runner: Some(hook_runner),
env: env.unwrap_or_default(),
checkpoint: None,
llm_source: None,
},
)
.await;
let executed = execute_and_emit_terminal(initialized).await;
let outcome = executed.outcome?;
let state = executed
.engine
.run
.run_store
.state()
.await
@ -299,6 +318,7 @@ pub async fn run_graph_from_checkpoint(
hook_runner: None,
env: HashMap::new(),
checkpoint: Some(checkpoint.clone()),
llm_source: None,
},
)
.await;
@ -324,12 +344,50 @@ pub async fn run_graph_from_checkpoint_with_state(
hook_runner: None,
env: HashMap::new(),
checkpoint: Some(checkpoint.clone()),
llm_source: None,
},
)
.await;
let executed = execute_and_emit_terminal(initialized).await;
let outcome = executed.outcome?;
let state = executed
.engine
.run
.run_store
.state()
.await
.map_err(|err| Error::engine(err.to_string()))?;
Ok((outcome, state))
}
pub async fn run_graph_with_state_and_llm_source(
registry: HandlerRegistry,
emitter: Arc<Emitter>,
sandbox: Arc<dyn Sandbox>,
graph: &GvGraph,
run_options: &RunOptions,
llm_source: Arc<dyn CredentialSource>,
) -> Result<(Outcome, RunProjection)> {
let initialized = initialized(
registry,
emitter,
sandbox,
graph,
run_options,
InitializedOptions {
hook_runner: None,
env: HashMap::new(),
checkpoint: None,
llm_source: Some(llm_source),
},
)
.await;
let executed = pipeline::execute(initialized.initialized).await;
initialized.store_logger.flush().await;
let outcome = executed.outcome?;
let state = executed
.engine
.run
.run_store
.state()
.await
@ -395,6 +453,29 @@ impl WorkflowRunner {
.await
}
pub async fn run_with_state_and_llm_source(
&self,
graph: &GvGraph,
run_options: &RunOptions,
llm_source: Arc<dyn CredentialSource>,
) -> Result<(Outcome, RunProjection)> {
let registry = self
.registry
.lock()
.unwrap()
.take()
.expect("WorkflowRunner may only be used once");
Box::pin(run_graph_with_state_and_llm_source(
registry,
Arc::clone(&self.emitter),
Arc::clone(&self.sandbox),
graph,
run_options,
llm_source,
))
.await
}
pub async fn run_from_checkpoint(
&self,
graph: &GvGraph,

View file

@ -571,6 +571,7 @@ impl Handler for FileWriterHandler {
let content = format!("output from {}", node.id);
let cmd = format!("echo '{content}' > {}.txt", node.id);
let _ = services
.run
.sandbox
.exec_command(&cmd, 10_000, None, None, None)
.await;
@ -1288,6 +1289,7 @@ impl Handler for AssetCreatorHandler {
"echo 'test output' > test-results/output.txt"
);
services
.run
.sandbox
.exec_command(script, 30_000, None, None, None)
.await

View file

@ -6141,11 +6141,10 @@ mod real_llm {
}
fabro_test::require_env("ANTHROPIC_API_KEY")?;
Some(Arc::new(
Client::from_env()
.await
.expect("unified-llm client should initialize from env"),
))
let source = fabro_auth::EnvCredentialSource::new();
Some(Arc::new(Client::from_source(&source).await.expect(
"unified-llm client should initialize from env source",
)))
}
fn make_llm_backend(client: Arc<Client>) -> Box<LlmCodergenBackend> {
@ -6602,10 +6601,165 @@ mod real_llm {
}
}
fn openai_api_key_credential(key: &str) -> fabro_auth::AuthCredential {
fabro_auth::AuthCredential {
provider: fabro_model::Provider::OpenAi,
details: fabro_auth::AuthDetails::ApiKey {
key: key.to_string(),
},
}
}
fn openai_responses_payload(text: &str) -> serde_json::Value {
serde_json::json!({
"id": "resp_1",
"model": "gpt-5.4",
"output": [
{
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": text
}
]
}
],
"status": "completed",
"usage": {
"input_tokens": 10,
"output_tokens": 20
}
})
}
// ---------------------------------------------------------------------------
// Wait.human freeform edge integration tests (Section 4.6)
// ---------------------------------------------------------------------------
#[tokio::test]
async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
use chrono::Utc;
use fabro_auth::{CredentialSource, VaultCredentialSource};
use fabro_types::Conclusion;
use fabro_vault::{SecretType, Vault};
use httpmock::Method::POST;
use httpmock::MockServer;
use tokio::sync::RwLock as AsyncRwLock;
let server = MockServer::start_async().await;
let response_mock = server
.mock_async(|when, then| {
when.method(POST)
.path("/v1/responses")
.header("authorization", "Bearer vault-openai-key");
then.status(200)
.header("content-type", "application/json")
.json_body(openai_responses_payload("Narrative from vault source."));
})
.await;
let mut graph = Graph::new("VaultOpenAiCodexPrBody");
graph.attrs.insert(
"goal".to_string(),
AttrValue::String("Verify PR body generation uses vault credentials".to_string()),
);
let mut start = Node::new("start");
start.attrs.insert(
"shape".to_string(),
AttrValue::String("Mdiamond".to_string()),
);
graph.nodes.insert("start".to_string(), start);
let mut exit = Node::new("exit");
exit.attrs.insert(
"shape".to_string(),
AttrValue::String("Msquare".to_string()),
);
graph.nodes.insert("exit".to_string(), exit);
graph.edges.push(Edge::new("start", "exit"));
let vault_dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(vault_dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let base_url = server.url("/v1");
let llm_source: Arc<dyn CredentialSource> = Arc::new(VaultCredentialSource::with_env_lookup(
Arc::new(AsyncRwLock::new(vault)),
move |name| match name {
"OPENAI_BASE_URL" => Some(base_url.clone()),
_ => None,
},
));
let dir = tempfile::tempdir().unwrap();
let mut registry = HandlerRegistry::new(Box::new(StartHandler));
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), local_env());
let run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
cancel_token: None,
run_id: test_run_id("vault-only-openai-codex-pr-body"),
labels: std::collections::HashMap::new(),
workflow_slug: None,
github_app: None,
base_branch: None,
display_base_sha: None,
host_repo_path: None,
git: None,
};
let (outcome, _) = engine
.run_with_state_and_llm_source(&graph, &run_options, Arc::clone(&llm_source))
.await
.expect("workflow run should succeed");
assert_eq!(outcome.status, StageStatus::Success);
let store_dir = test_store_dir(&run_options.run_dir);
let store = Arc::new(Database::new(
Arc::new(LocalFileSystem::new_with_prefix(&store_dir).unwrap()),
"",
Duration::from_millis(1),
None,
));
let run_store = store.open_run_reader(&run_options.run_id).await.unwrap();
let run_store_handle: fabro_workflow::runtime_store::RunStoreHandle = run_store.into();
let body = fabro_workflow::pull_request::build_pr_body(
"diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
"Implement feature",
"gpt-5.4",
&run_store_handle,
llm_source.as_ref(),
Some(&Conclusion {
timestamp: Utc::now(),
status: StageStatus::Success,
duration_ms: 1,
failure_reason: None,
final_git_commit_sha: None,
stages: Vec::new(),
billing: None,
total_retries: 0,
}),
)
.await
.expect("PR body should build from vault-only credentials");
assert!(body.contains("Narrative from vault source."));
response_mock.assert_async().await;
}
/// Freeform-only human gate: free-text input routes through the freeform edge
/// and stores the text in human.gate.text context variable.
#[tokio::test]
@ -7376,9 +7530,10 @@ fn subgraph_without_label_no_class_derived() {
// ---------------------------------------------------------------------------
fn hook_runner_from_defs(hooks: Vec<fabro_hooks::HookDefinition>) -> Arc<fabro_hooks::HookRunner> {
Arc::new(fabro_hooks::HookRunner::new(fabro_hooks::HookSettings {
hooks,
}))
Arc::new(fabro_hooks::HookRunner::new(
fabro_hooks::HookSettings { hooks },
Arc::new(fabro_auth::EnvCredentialSource::new()),
))
}
struct HookTestRunner {
@ -10175,9 +10330,10 @@ impl Handler for FileWriterHandler {
_run_dir: &Path,
services: &fabro_workflow::handler::EngineServices,
) -> Result<Outcome, Error> {
let work_dir = services.sandbox.working_directory().to_string();
let work_dir = services.run.sandbox.working_directory().to_string();
let file_path = format!("{}/{}.txt", work_dir, node.id);
services
.run
.sandbox
.write_file(&file_path, &format!("written by {}", node.id))
.await
@ -12254,7 +12410,7 @@ impl Handler for KeepaliveHandler {
let start = std::time::Instant::now();
while start.elapsed() < std::time::Duration::from_millis(self.total_ms) {
tokio::time::sleep(std::time::Duration::from_millis(self.interval_ms)).await;
services.emitter.emit(&Event::Prompt {
services.run.emitter.emit(&Event::Prompt {
stage: node.id.clone(),
visit: 1,
text: "keepalive".to_string(),
@ -12530,6 +12686,7 @@ impl Handler for AssetCreatorHandler {
"echo 'test output' > test-results/output.txt"
);
services
.run
.sandbox
.exec_command(script, 30_000, None, None, None)
.await