Commit graph

86 commits

Author SHA1 Message Date
Bryan Helmkamp
09ef90ba1e
refactor: pass the catalog by value and correct the RenderMode docs
- Take `Arc<Catalog>` by value again through the validation entry points.
  `AppState::catalog()` returns an owned `Arc`, so `&state.catalog()` was
  cloning, borrowing the temporary, then cloning again at the leaf. Every
  consumer ends up owning the `Arc`, so by-value is the honest shape and it
  drops one clone per call. The one caller holding the catalog in a field
  now says `Arc::clone(&self.catalog)` explicitly.
- Correct the `RenderMode` doc comment. It claimed `Strict` is "used by
  run-create", but run-create renders `Structural` and promotes the
  resulting warnings to errors itself; `Strict` has no production caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 14:30:48 -04:00
Bryan Helmkamp
c5d3345144
Merge pull request #651 from fabro-sh/fix/offline-validation-catalog-free
fix(cli): keep offline validation catalog-free
2026-07-28 14:28:35 -04:00
Bryan Helmkamp
04c931e9d3
Simplify edge-only node validation 2026-07-28 14:14:32 -04:00
Bryan Helmkamp
8592a34968
refactor: collapse the split validation paths
Follow-up cleanup on the catalog-free validation split. Same behavior,
fewer parallel code paths.

- Make the catalog an explicit `Option<&Catalog>` on `pipeline::validate`
  instead of a `validate` / `validate_with_catalog` pair, so each call
  site states whether catalog rules run.
- Collapse `preprocess_and_validate`, `preprocess_and_validate_structural`,
  and `preprocess` into one function that takes `TransformOptions`. Its
  `model_resolution` field is now the single source of truth for catalog
  awareness, which drops a 12-argument signature and the
  `too_many_arguments` allow.
- Replace the duplicated resolve-and-preprocess block in
  `operations::validate` with one `validate_in_scope` helper, and drop the
  HashSet -> Vec -> HashSet round trip on the catalog path.
- Extract `configured_default_provider`, previously duplicated between
  `operations::create` and `operations::validate`.
- Delete `validate_manifest_with_environment_defaults`, which had no
  callers outside its own module.
- Share the `server-model.fabro` fixture between the two CLI tests instead
  of inlining it twice. The validate test now asserts the rendered output
  through the usual snapshot helper, which also removes a hand-rolled
  `std::fs::write` and its clippy allow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 14:13:45 -04:00
Bryan Helmkamp
010c8d50c1
Add structured review targets to human gates 2026-07-28 13:24:12 -04:00
Bryan Helmkamp
f0a7423b51
refactor(config): stop resolving {{ env.* }} in interpolated config
The process environment is no longer a configuration source. `{{ vars.NAME }}`
(non-sensitive, server-stored) and `{{ secrets.NAME }}` (vault-backed) cover
both cases, and reading the worker's ambient environment made a run's inputs
depend on how its process happened to be launched.

`Namespace::Env` is kept but wired to nothing, so `{{ env.NAME }}` still
parses and fails with a message naming its replacement rather than reaching
a consumer as literal text. `ResolveCtx::with_env` is gone, so no call site
can opt back in.

Two long-standing warts were env-only and go with it:

- `InterpString::resolve_or_source`, the "fall back to the raw template
  source on failure" path, which let an unresolved token reach a sandbox or
  the GitHub API as literal `{{ ... }}` text. Its own comment noted it was
  slated for hard-error semantics.
- `RunEnvironmentSettings::resolve_env`'s matching source fallback for
  env-only values.

Both carried `#[expect(clippy::disallowed_methods)]` escape hatches. Every
run-boundary resolver — sandbox env, prepare steps, MCP transports, GitHub
permissions, Slack channels, run goal files, provider extra_headers — now
fails closed instead.

Hooks lose their `allowed_env_vars` allowlist, `resolve_header`, and
`HeaderResolveError` along with the `E: Env` generic threaded through the
executor. They keep `{{ vars.* }}`, which `RunSettings::substitute_variables`
already substitutes server-side at run creation.

`allowed_env_vars` is removed from the OpenAPI spec and the generated
TypeScript client. The docs example showing `{{ env.* }}` in
`[server.slatedb.s3].bucket` was already wrong — that field is a plain
String and never interpolated — and is now a literal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 21:09:35 -04:00
Bryan Helmkamp
e54fef760a
refactor(auth): remove EnvCredentialSource and make the run vault required
`EnvCredentialSource` resolved provider credentials from the process
environment. It had no production entry point of its own — it was only
ever reached as the `None` arm of an `Option<Vault>` in three places:
`build_llm_source`, `configured_providers_for_start`, and
`configured_providers_from_process_env`.

That optional vault is not a state the product can be in. Every run has a
server behind it, the server always spawns workers with `--storage-dir`
(`worker_runtime.rs`), and `SqlVaultCredentialSource` backs both the
server and the CLI. So the fallback only served to silently degrade
credential resolution to whatever the worker process happened to have in
its environment.

Make the vault required across the run path — `RunOptions`,
`StartServices`, `build_llm_source`, `tool_secrets_from_configured_sources`,
`vault_token_lookup`, and the CLI GitHub helpers — so the invariant is
enforced by types rather than assumed. A worker spawned without
`--storage-dir` now fails with a clear message instead of quietly
continuing without a vault.

`configured_providers_from_process_env` had no callers at all and is
deleted. `AgentApiBackend::new_from_env` was public but only ever called
from its own tests; it is deleted too.

Test-only credential sources move to a feature-gated
`fabro_auth::test_support`, wired through dev-dependencies so they never
link into production builds. The CLI worker tests now pass
`--storage-dir`, matching what the server actually does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 20:36:54 -04:00
Release Repro
69a51e65b9
feat(workflow): infer command nodes from the script attribute
A node with no `shape` defaulted to `box`, which resolves to the agent
handler. That made a shapeless `script` node run as an LLM call prompted
with its own label, while the `script` was reported as inert — wrong
behavior behind a warning.

`script` is read by the command handler and by nothing else, so a
shapeless node that sets it is unambiguously a command node. `shape()`
now infers `parallelogram` in that case. An explicit `shape` still wins.

Two rules keep the inference honest:

- `script_prompt_conflict` — setting both `script` and `prompt` is an
  error. No handler reads both. It fires regardless of shape so that
  adding one cannot downgrade the error to a warning.
- `command_requires_script` — a command node without a script is an
  error. Without this the original trap just moves: a node meant as a
  command that omits its script silently becomes an agent again.

Also drops the `tool_command` alias in favor of `script` alone, routing
the six read sites through a new `Node::script()` accessor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 14:59:31 -04:00
Bryan Helmkamp
c501c67185
Show each diagnostic's suggested fix in CLI output
Diagnostics have carried a `fix` field all along, but the CLI renderer
never printed it — the suggestion was only reachable through --json. The
actionable half of every validation failure was invisible to the person
running the command.

print_diagnostics now emits the fix as a dim-labelled continuation line
under any diagnostic that has one, at both error and warning severity.
Gating it behind --verbose would defeat the point, and printing it only
for errors would read as "this warning has no fix" — the warning
suggestions are useful on their own. Diagnostics that set no fix simply
omit the line.

The severity match moved into print_diagnostic so the fix line is
appended once in the loop rather than copied into all five arms; the
rest of the diff is reindentation.

print_diagnostics is shared by validate, preflight, graph, exec, and
dry-run, so this covers all five. Eleven inline snapshots across four
files gain a fix line; every change is additive.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 14:06:31 -04:00
Bryan Helmkamp
59b1c2e59f
Reject nodes referenced by an edge but never declared
The DOT parser created a node for every edge endpoint, and nothing
recorded whether a node came from a declaration or was synthesized from
an edge. The edge_target_exists rule only checked whether the node id
was present in the graph, which was always true by then, so a misspelled
endpoint became an attribute-free node that defaulted to shape=box — an
LLM stage. Validation emitted a prompt_on_llm_nodes warning and exited 0.

Node now carries `implicit`, set only when the parser synthesizes the
node from an edge endpoint. A declaration anywhere in the workflow
clears it, so order does not matter and subgraph declarations count.
Node::new leaves it false, so programmatic construction and graphs
deserialized from older checkpoints read as declared.

edge_target_exists treats an endpoint as valid only when it exists and
is declared, reporting each undeclared node once. The near-identical
missing-source and missing-target branches collapse into one path. The
import transform copies the flag onto spliced nodes so an edge-only node
inside an imported fragment is caught too.

parse_and_validate_human_gate had two edge-only nodes and now declares
them; it was an instance of the bug rather than a casualty of the fix.
No shipped workflow, docs example, or CLI fixture relied on the old
behavior.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 13:53:54 -04:00
Bryan Helmkamp
b53045a1ac
Add runtime for_each item injection 2026-07-27 11:55:55 -04:00
Bryan Helmkamp
1c82bd9008
fix(workflow): make publish failures terminal 2026-07-27 11:25:18 -04:00
Bryan Helmkamp
0b24649e76
fix(cli): keep offline validation catalog-free 2026-07-26 09:25:47 -04:00
Bryan Helmkamp
c81ea69c73
Merge origin/main into feat/inference-observability 2026-07-24 22:55:15 -04:00
Bryan Helmkamp
d4f619bc2a
fix: clean up inference observability 2026-07-24 22:50:00 -04:00
Bryan Helmkamp
6659ae768a
feat(events): make inference in-flight state observable
During a long LLM turn the durable event stream was silent: between
`agent.tool.completed` and the next `agent.message` nothing was emitted,
so "the model is generating" and "the worker is wedged" were
indistinguishable from the run store, SSE, or the UI.

The signal already existed. `AssistantTextStart` fired at exactly the
right point — after `build_request()`, after compaction, immediately
before the stream opens — then was classified as streaming noise and
thrown away. This promotes it rather than inventing a new one.

Two events, each asserting only what is provable when it is emitted:

- `agent.llm.started` carries the *requested* provider/model. No usage,
  no cost, no context window: none of it exists yet, and failover can
  re-target, so `agent.message` stays authoritative for what answered.
- `agent.llm.first_output` is edge-triggered on the first output of an
  attempt and names what arrived. `ToolCall` is required, not optional:
  a turn that opens with a tool call produces no text or reasoning
  delta, so a latch keyed on those two would stay silent for exactly
  the tool-heavy rounds where liveness matters most.

`agent.llm.retry` now also fires on the one previously invisible
mid-turn path — a stream that ends without a finish event, which
replays the turn and discards its output with nothing to show for it.
Its `attempt` field was already fed by two independent counters, so an
optional `phase` (open | consume) names which loop it counts.

`StageProjection.inference` projects the open bracket. `Some` means
"the event log contains an unclosed inference bracket", not "the model
is computing now" — a SIGKILLed worker leaves it open, which is the
truthful statement of what we know, and `watchdog.timeout` remains the
authority on actually-stuck.

The close is the subtle part. Terminal cancel and wall-clock timeout
tear the session down through `discard_session` without emitting a
message, error, or interrupt, so a session-lifecycle backstop is
required. It has to be `agent.session.ended`, not
`agent.session.deactivated`: deactivation is emitted by `lease.release()`
*before* the forwarder drains queued agent events, so a queued
`agent.llm.started` can arrive after it and re-open the bracket. But
`agent.session.ended` carries no stage identity, so the close takes
ordering from the event and identity from the projection, scanning for
brackets the ending session opened. A normal stage lookup there finds
no target and silently no-ops.

Presentation states what the log proves and nothing more: no progress
bar or ETA (no completion estimate exists), "reasoning" only when the
provider sent reasoning output, elapsed counted since the request
opened, and no live animation once the run is terminal.

Scope is session-backed agent stages. One-shot completions call
`client.complete` directly and never build a session; covering them
means moving the emit point into `fabro-llm`, filed as a follow-up.

`agent.output.start` was never persisted — it existed in a name map,
an `unreachable!` arm, and docs — so the rename carries no migration
risk. Corrects `events.md`, which documented it as a real emitted
event, and the v2 proposal, which mapped it to `message.part.started`
despite it firing before the request opens.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 21:47:51 -04:00
Release Repro
af647aba4b
fix(cli): avoid duplicate compaction error prefix 2026-07-24 21:24:31 -04:00
Release Repro
7eef7652d3
fix(agent): harden compaction failure handling
Accept every nonblank summary instead of applying an arbitrary length heuristic. Preserve typed compaction failures and their source chains, suppress repeat attempts within one input, and clear the CLI compaction indicator when the existing agent error event arrives.
2026-07-24 20:57:57 -04:00
Bryan Helmkamp
e7740b4acb
feat(reasoning): capture provider reasoning in agent.message
Normalize the readable reasoning providers already return into a
canonical `ReasoningOutput` and carry it through the `agent.message`
run event to storage, SSE, and JSONL.

The shape is derived from the final response's canonical message
content rather than stored a second time, so there is no duplicate
source of truth and retried or replaced streaming buffers never
become durable reasoning. OpenAI-compatible `reasoning_details` are
now preserved verbatim as an opaque content part; only known readable
members are normalized out of them, leaving encrypted entries for a
later provider-aware replay phase.

This phase is passive: no request parameters change, no capability
guessing, and no newly observed provider field is replayed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 14:21:15 -04:00
Bryan Helmkamp
c9900b2bfa
Merge branch 'main' into feat/backward-event-pagination 2026-07-24 10:14:57 -04:00
Bryan Helmkamp
eb83539a18
Merge origin/main into feat/stage-execution-identity-on-resume
Resolves conflicts with the shared-checkout parallel rewrite (#607) and the
cached-run/billing dedup (de60eb900):

- handler/parallel.rs: rebuilt on main's shared-checkout version. Branch
  ordinals are still reserved inside the branch task right before
  ParallelBranchStarted (with graph_visit/resumed_from_stage_id), and the
  reserved StageScope is shared with post-await error paths via a OnceLock
  slot instead of main's dispatch-time visit=1 scope, so completion events
  are never emitted under a guessed ordinal.
- billing.rs: keep this branch's run_stage_from_projection (RunStage grew
  graph_visit/resumed_from_stage_id and a typed id), adopt main's
  state.cached_run() and drop the removed run_stage_from_stage_id import.
- run_projection.rs: adopt main's typed parallel_results
  (Option<Vec<ParallelBranchResult>>).
- run_event/misc.rs: union of both sides' imports.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 10:07:35 -04:00
Bryan Helmkamp
ec100fca2b
Merge remote-tracking branch 'origin/main' into feat/backward-event-pagination
Resolved conflicts against main's shared-projection-cache rework:
- projection_cache.rs: kept main's projection_snapshot and dropped this
  branch's last_seq accessor, which it subsumes; latest_event_seq now
  reads the sequence from projection_snapshot.
- run_store.rs: kept main's EventScan cursor and added a seek_before
  constructor so the backward-pagination range scan bounds its end key
  through the same abstraction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 09:57:15 -04:00
Bryan Helmkamp
8394eb2723
Merge pull request #619 from fabro-sh/feat/fireworks-provider
feat(llm): add Fireworks AI as an opt-in provider
2026-07-24 09:46:03 -04:00
Bryan Helmkamp
78fea736e3
fix: harden stage execution identity on resume 2026-07-24 09:37:05 -04:00
Bryan Helmkamp
415e9e5cd8
Merge pull request #617 from fabro-sh/feat/expose-model-controls
Expose model reasoning effort controls
2026-07-24 09:34:55 -04:00
Bryan Helmkamp
5d8befa6ac
Reuse canonical ModelControls in fabro-api and tighten serde contract
Add the missing with_replacement for ModelControls so progenitor reuses
fabro_model::ModelControls instead of generating a dead parallel DTO,
re-export it from fabro_api::types, and assert type identity in the
round-trip test.

Drop #[serde(default)] from Model.controls and
ModelControls.reasoning_effort: the OpenAPI spec marks both required,
matching the strict deserialization of the sibling features/costs
fields. Update CLI stub payloads to include the now-required field.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 09:15:18 -04:00
Bryan Helmkamp
cd706646c6
feat: treat resumed in-flight nodes as new stage executions
A node cancelled (or lost to a crash) mid-flight and then resumed now
starts a new stage execution with the next StageId ordinal (work@2)
instead of reusing and clearing the cancelled execution's projection.
The old execution stays immutable with its own events, session, output,
timing, billing, and termination state.

Engine:
- Add a run-scoped StageExecutionTracker on RunServices with per-node
  high-water marks. Ordinals are reserved after the StageStart hook
  passes on the first attempt (retries reuse the reservation), ensured
  at the composite checkpoint pre-step for hook-skips, and reserved in
  on_terminal_reached for terminal nodes' synthetic events.
- Keep three concepts distinct: graph visit (max_visits/checkpoints,
  unchanged), stage execution ordinal (the @N in StageId), and handler
  attempt. The tracker is not checkpointed; the append-only stage event
  history is its durable source of truth.
- resume() seeds the allocator from the run projection and computes a
  node -> StageId provenance map of executions observed after the
  selected checkpoint, threaded through execute_persisted_run,
  RunSession, and InitOptions.

Events and projections:
- stage.started, parallel.branch.started, and checkpoint.completed
  carry optional graph_visit and resumed_from_stage_id; StageProjection
  stores both. Old events deserialize with None and legacy duplicate
  stage.started replays keep last-attempt behavior.
- The CheckpointCompleted reducer is envelope-first: diffs and
  skipped-stage synthesis attach to the exact execution StageId, an
  existing Retrying projection finalizes as Skipped without losing
  identity, and historical node_outcomes no longer create or collide
  with newer ordinals (node_visits remains a legacy fallback).

Handlers:
- Parallel fan-out reserves child ordinals through the shared tracker,
  derives worktree pass{N} from the parent's execution ordinal, and
  seeds branch contexts with explicit child stage scopes so branch
  lifecycle and nested handler events agree.
- Artifact capture and manager-loop child logs follow the ordinal.

API and UI:
- RunStage documents visit as the execution ordinal and adds optional
  graph_visit and resumed_from_stage_id; Rust and TypeScript clients
  regenerated.
- The web sidebar lists both executions chronologically; resumed stages
  show a "Resumed from" link in the stage detail header and hover
  popover, with the graph visit surfaced when it diverges.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 09:00:37 -04:00
Bryan Helmkamp
c3cdefa5ea
refactor(events): simplify backward pagination internals
- Extract a shared fetch_run_events_page helper so the three client
  paging loops (full list, until, tail) no longer repeat the request/
  convert/has_more skeleton; fold the tail loop's two descending-order
  checks into one and drop its redundant had_events flag.
- Skip the latest-seq lookup in list_events_before_with_limit when the
  caller supplies a before_seq cursor, so a cold projection cache costs
  at most one full history scan per pagination session instead of one
  per page.
- Remove the dead before_seq max(1) clamp and the passthrough order()
  accessor from EventListParams.
- Document the CLI --tail 0 --follow seeding trick and the reader
  event_seq placeholder invariant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 08:34:02 -04:00
Bryan Helmkamp
9708ca8177
feat(llm): add Fireworks AI as an opt-in provider
Adds a disabled-by-default `fireworks` provider to the built-in catalog,
served through the existing openai_compatible adapter/codec. The curated
roster covers Kimi K2.7 Code (default), Kimi K2.6, DeepSeek V4 Pro/Flash,
GLM 5.2, MiniMax M2.7, Qwen 3.7 Plus, and GPT-OSS 120B/20B (small
default + probe), with serverless pricing including cached-input rates.

All api_ids were verified live against /chat/completions (Fireworks'
GET /v1/models only returns a featured subset), and serverless responses
were confirmed to report prompt_tokens_details.cached_tokens, so cache
billing works through the existing codec path.

FIREWORKS_API_KEY is registered as an optional vault secret; provider
login, vault storage, and diagnostics probing are catalog-driven and
need no code changes. Includes catalog/install tests, two live e2e
tests, an integrations docs page, and a provider logo for the web UI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 08:03:39 -04:00
Bryan Helmkamp
4bd9753217
Expose model reasoning effort controls 2026-07-24 07:44:40 -04:00
Bryan Helmkamp
bb1afae363
feat(events): add backward cursor pagination 2026-07-24 07:23:23 -04:00
Bryan Helmkamp
4621149b6e
Merge remote-tracking branch 'origin/main' into feat/shared-checkout-parallel 2026-07-24 06:54:32 -04:00
Bryan Helmkamp
85f3286c66
Merge branch 'main' into feat/shared-checkout-parallel 2026-07-24 06:29:57 -04:00
Bryan Helmkamp
84c5468722
Merge remote-tracking branch 'origin/main' into fix/cancellation-interrupt-lifecycle
# Conflicts:
#	lib/components/fabro-agent/src/subagent.rs
#	lib/components/fabro-agent/tests/it/parity_matrix.rs
2026-07-23 20:55:25 -04:00
Bryan Helmkamp
3c33e0fb69
Merge origin/main (three-layer crate layout) into fix/openrouter-anthropic-prompt-caching
The crate reorganization renamed lib/crates/ to lib/apps|components|foundation/.
Git followed all modified files across the rename; the only conflict was the
newly added codec/cache.rs, now placed at lib/components/fabro-llm/src/codec/cache.rs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-23 19:30:20 -04:00
Scott Werner
47bc772f7b refactor: organize crates into three layers 2026-07-23 17:59:34 -04:00