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>
`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>
Command node `script` attributes were literal text: a `{{ inputs.x }}`
reached bash verbatim, and the only signal was a `detemplated_attribute`
warning. Scripts now substitute `{{ goal }}`, `{{ inputs.NAME }}`, and
`{{ vars.NAME }}` at run creation, alongside goals and prompts.
Scripts use `InterpString` token substitution rather than the MiniJinja
pass that renders prompts. Shell source is full of brace syntax that must
survive untouched — jq filters, awk programs, Go templates, brace
expansion — and `InterpString` claims only the narrow token forms,
leaving everything else literal.
`env` and `secrets` are deliberately not wired and now fail loudly
instead of passing through as text. A script reads the environment with
`$NAME`, which needs no interpolation, and a resolved secret would be
baked into the `CommandStarted` event that records the script verbatim.
The error points at `[environments.<slug>.env]` for the secret case.
`ResolveCtx` gains opt-in `with_inputs` and `with_goal`. Namespace
availability stays scope-determined per call site, so every existing
config-layer context leaves both unwired and keeps its current behavior.
`goal` names a single value rather than a namespace of them, so it has
no dotted form: only the exact body `goal` produces a token and
`{{ goal.title }}` stays literal.
Values substitute verbatim without shell quoting, matching
`[[run.prepare.steps]].script` where the snippet is the author's to
quote. Substituted text is never rescanned.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The model indicator on a stage page hovered to provider, model, and
reasoning effort only. Seeing what a stage actually spent meant leaving
for the Billing tab, which reports per node rather than per visit.
The stage list had no token data to show, so add a per-visit `billing`
block to `GET /runs/{id}/stages`. The Billing tab's pricing rule (a
provider-reported cost wins, otherwise the server catalog prices the
tokens) was private to `billing_rollup`; move it to
`StageProjection::billed_usage` and drive both call sites from it so the
two views cannot drift.
The popover's buckets use the Billing tab's labels verbatim. It stays
scoped to one visit, so a looped node's row on the Billing tab is the sum
of what each of its visits shows here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
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>
`ReadBeforeWriteSandbox` blocked writes to any existing file the agent
had not read, tracked by a session read set populated only by
`read_file`, `grep`, `read_many_files`, and the Kimi `Read`.
The gpt56 profile has none of those. It mirrors Codex's tool contract --
`shell_command`, `apply_patch`/`edit_file`, `update_plan`, `web_search`
-- and reads through the shell, so its read set stayed permanently
empty and every edit to an existing file failed. In run
01KYD4360GN6SED4BYEVGYP4XT all 28 `edit_file` calls failed, 25 of them
on the guard. The agent read `package.json` with `sed` and `cat`,
hex-dumped it trying to diagnose the rejections, then routed around the
guard with `sed -i`, which the guard never covered. It prevented no
blind write; it converted content-anchored edits into an unreviewed
in-place shell rewrite.
Neither Codex nor Kimi Code enforces read-before-write at runtime.
Codex's `apply_patch` `Add File` overwrites an existing path silently;
Kimi Code's `Write` has no check at all. Both rely on the exact-match
requirement in their edit tools, which is stronger proof of inspection
than a read set, plus per-write approval.
Tool descriptions and the Kimi prompt keep telling the model to read
before editing -- that guidance matches Kimi Code's own `edit.md` and
still prevents `old_string not found` -- but no longer claim the
workspace refuses unread writes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The stage-summary preamble rendered per-stage token usage for every
completed LLM stage: "Model: kimi-k3, 92.6k tokens in / 41.1k out" at
compact fidelity and "Tokens: N in / N out" at summary:high. Agents read
that as their own remaining budget.
In run 01KYCM3EG4KMCVRDYNV93PZWBV an implementation stage stopped after 2
of 9 units, reasoning "We have around 100k tokens, but time constraints
are an issue" and recording the rest as halted "within the available
execution window". The 92.6k it saw was the preceding plan stage's
billing telemetry, the only token quantity anywhere in its context. It
had used 11% of a 1,050,000-token window and 0.8% of a 24h stage timeout,
and no harness limit was near.
These counts have no task value to the agent: they describe a different
model's usage on an earlier stage, they are stale by one stage, and
nothing in the preamble distinguishes them from a budget. Keep the model
id and files touched, which carry provenance the agent can act on.
Both tests that asserted the counts now assert their absence, so the
regression is caught rather than re-snapshotted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Fabro advertised Bash while its three backends implemented three
different contracts: Daytona evaluated commands through `sh`, and
Docker's streaming, stdio, and setup paths used a login shell. Bash-only
syntax silently misbehaved depending on provider and code path, and
login profiles could change PATH and command behavior per image.
Make `bash -c` the enforced interpreter for every command string the
Unix sandbox API accepts, on every production backend and through both
buffered and streaming execution. This selects the interpreter only —
no `errexit`, no `pipefail`, no login mode — so `false | true` still
succeeds and a workflow that wants other semantics writes them into its
own command.
Local resolves `bash` through the worker's PATH (NixOS has no
/bin/bash) and reuses that one executable across all three command
paths. Docker and Daytona require /bin/bash with no `sh` fallback.
Fresh initialization and resume/start now verify Bash through a shared
marker-validating probe before reporting the sandbox usable, so a
missing or non-Bash interpreter fails at the lifecycle boundary with
provider-specific remediation instead of on the first command. The
probe also rejects Bash in POSIX mode, which an image whose `bash` is
really `sh` would otherwise pass.
Sandbox MCP scripts and the detached launch wrapper move under the same
contract; host-side stdio MCP scripts, hooks, and interactive terminals
are separate executors and keep their existing `sh` behavior.
The `shell` tool's name and JSON schema are unchanged across providers;
only its prose now identifies `command` as Bash source.
BREAKING CHANGE: sandbox commands no longer load login-shell profiles,
so environment set in /etc/profile.d/*.sh, ~/.bash_profile, or
nvm/rbenv/sdkman initializers is gone. Move those exports into the
Dockerfile's ENV or the Daytona snapshot image.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shell executor rendered every returned ExecResult and returned
Ok(output), so nonzero exits, timeouts, and cancellations reached
execute_one_tool() as successes. That false ToolResult propagated
consistently: agent.tool.completed recorded is_error: false, the success
post-tool hook ran, Anthropic saw is_error: false, OpenAI Responses saw a
completed function-call output, and CLI/web rendered a successful tool
call.
ExecResult::is_success() is now the authoritative predicate. The executor
runs through exec_command_streaming() with a sink callback, so it keeps
the production providers' stream provenance and partial-output capture,
and drops the exec 2>&1 prefix that merged stderr into stdout before
Fabro could report it. Model-facing text labels termination, exit code,
duration, and either separate stdout/stderr sections or one combined
section when the provider cannot separate streams.
Session-bound dispatch also emits a typed agent.tool.process.completed
event carrying the process metadata, streams_separated, and bounded
redacted output tails. It is subordinate diagnostic data: the following
agent.tool.completed remains the one tool-protocol completion and the
authoritative owner of is_error, so consumers need no new row.
Nonzero, timed-out, and cancelled commands intentionally change from
successful to failed tool results, and PostToolUseFailure replaces
PostToolUse for them. On Docker the agent shell tool now uses the
streaming path's bash -lc supervisor, which terminates the process group
on timeout instead of leaving container-side processes running.
The public shell schema is unchanged and pinned by an exact assertion.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Follow-up cleanup on the profile-builder refactor.
AgentProfileBuilder::build now borrows instead of consuming, removing the
builder.clone().build() dance at all seven call sites. Deletes
with_command_timeouts, which had no caller but its own test, and the
with_summarizer constructors on all three profiles, whose only remaining
caller was each profile's own new().
Replaces the fifth copy of the profile-kind match (guardrails.rs) with the
builder, and swaps the parity matrix's hand-maintained provider list for
Catalog::effective_agent_profile so a new catalog provider cannot silently
skip the matrix. Collapses web_search_provider_test! into a secrets = arm
on provider_test! and uses EnvVars::BRAVE_SEARCH_API_KEY over a literal.
Drops the Brave key from the Ask Fabro session: AskFabroToolAccessPolicy
denies web_search, and both tools() and the prompt are filtered through
that policy, so the vault read only registered an uncallable tool.
Makes NativeToolOptions::for_profile match exhaustively so a new profile
kind must state its timeout, restores Anthropic's borrowed prompt sections
and Gemini's static prompt (placeholder substitution rather than format!
over 110 lines with doubled braces), and introduces WEB_SEARCH_TOOL_NAME
for the registry lookups that keep tool availability and prompt guidance
in sync.
Updates the product docs, which still described web_search as always
registered and as erroring at call time when unconfigured.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
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>
Resolves conflicts with main's typed reasoning_effort field (#609) and
the usage-buckets test (#616). The handler's manual string parse is
superseded by serde-level validation of the typed enum, so it is
removed along with its test; the client-side unsupported-effort
validation and 400 error mapping from this branch are kept.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
The fallback provider set was always catalog.all_provider_ids(), computed
at every call site and threaded through five layers alongside the catalog
itself. Fold it into Catalog::resolve_selection_with_catalog_fallback and
carry only a catalog_fallback flag through the transform/validate/
materialize entry points.
- materialize_run delegates to resolve_run_model again instead of
re-inlining its provider normalization and selection
- run_preflight derives ready providers from llm_result instead of
taking both, so callers cannot pass inconsistent pairs; the legacy
tests now exercise the production ready-first routing path
- AppState::resolve_llm_client_with_ready_ids replaces three copies of
resolve-then-extract-provider-ids, and ready_llm_provider_ids
delegates to it
- the unreachable "model resolution failed" preflight check becomes an
invariant error where the materialized run is produced
- validate_prepared_manifest_with_vars/_for_preflight share the
ValidateInput construction
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Return InvalidRequest (400) for unsupported speed too, matching the
reasoning_effort check and the complete()/stream() doc comments
- Centralize fabro_llm::Error -> ApiError mapping in a From impl so the
completions handler, playground handler, and Error::Llm arm agree on
the InvalidRequest -> 400 / else -> 502 split
- Reject unparseable reasoning_effort values with 400 instead of
silently dropping them
- Add classify_sdk_invalid_request test per fabro-workflow convention
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Extract emit_branch_completed() to replace three near-identical
ParallelBranchCompleted constructions; status now reads consistently
from outcome.status
- Add context_diff_public() so parallel.rs and manager_loop.rs share the
diff-minus-engine-internal-keys step; move context_diff tests next to
the function in context.rs
- Replace fan_in's dead BranchShape struct with the canonical
Vec<ParallelBranchResult> (from_value moves, so no payload cloning)
- Narrow parseParallelOverview to ParallelBranchSummary {id, status};
its only consumer renders just those fields
- Drop helpers.test.ts's duplicate envelope() fixture in favor of the
shared makeEventEnvelope
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
