Deflate flushes its output in 8 KiB blocks, and each write became its own
allocation, channel send, and HTTP body frame. A 64 KiB BufWriter in
front of the sink cuts all three by eight.
An artifact deleted between the listing and its read no longer aborts the
whole archive. That race is a run being pruned mid-download; leaving the
file out beats handing back a truncated ZIP missing everything after it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Path safety now lives in one place. The NUL-byte and drive-letter rules
move from a server-only helper into the store's own filename validation,
so uploads reject those paths at write time instead of only the ZIP read
path catching them. The download still re-checks, because artifacts
stored before the rule existed can still carry an unsafe path, but it
now skips a bad path rather than failing the whole archive.
Promote is_boundary_stage to RunProjection and drop the three identical
private copies. The ZIP download used a node-name match instead, which
would have dropped artifacts from a working node that happened to be
named "start".
Compress the archive. Entries were Stored while the response was also
excluded from transfer compression, so text artifacts moved at full
size. async_zip gains the deflate feature; async-compression and flate2
were already in the lock file.
Log archive failures unconditionally. The send-succeeded guard meant a
client that had already disconnected left no record at all, which is the
case where the log is the only evidence.
Also: collapse the duplicate 500 arms, drop the dead stage-ID tiebreaker
and the cached order in the selection map, name the accessible label
after the visible one, share the run URL prefix between the two download
href builders, and document the mid-stream truncation behavior in the
OpenAPI description.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Conflicts were between this branch's parallel-branch identity work and
main's stage billing, review targets, and live stage timing.
- Stage fixtures: main added `billing` to each per-file `makeStage`; this
branch had hoisted one builder into `lib/test-utils`. Kept the hoisted
builder and gave it `billing: makeBilledTokenCounts()`, so both intents
hold and the field list stays in one place. `stage-sidebar.test.ts` also
builds raw `RunStage` wire payloads, so it keeps importing
`makeBilledTokenCounts` directly.
- Import lists (`run_projection.rs`, `fabro-api/src/lib.rs`,
`run_state.rs`, `stage_projection_round_trip.rs`): unioned both sides —
`ParallelBranchId` alongside `timing`, `ReviewTarget`,
`ReviewTargetKind`, `AttrValue`, `Node`, and
`StageToolBatchProjection`.
- `fabro-server` tests: git interleaved two unrelated new tests into one
body. Split them back into
`list_run_stages_exposes_parallel_branch_identity` and
`run_billing_includes_live_stage_timing_in_rows_and_totals`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Splitting on the first colon in FromStr broke bare model IDs that
legitimately contain one. A reference like "llama3:8b" parsed as
provider "llama3" selector "8b", and since "llama3" is not a provider
the lookup failed instead of passing the ID through to the pinned
provider. Verified against origin/main: canonical_session_model with
"future-model:latest" pinned to openrouter returned the passthrough
before and a 400 after.
This is not fixable by choosing a different separator. Bedrock
inference-profile ARNs contain both colons and slashes, and
docs/public/integrations/bedrock.mdx tells users to put arbitrary
inference-profile IDs in api_id. Only the registry can tell a provider
prefix from a model ID that happens to contain the separator.
FromStr now leaves colon-bearing tokens bare, and ModelRef::qualify
promotes only those whose prefix names a known provider. resolve()
applies it, so the fallback path is covered; sessions.rs applies it
before its own match so it keeps its tailored ambiguity messages.
ModelRegistry is now implemented for Catalog in fabro-types, replacing
the CatalogModelRegistry wrapper that existed only in start.rs, so both
call sites share one registry view.
Covered by regression tests at both surfaces, plus qualify unit tests
for ollama tags and Bedrock ARNs. The pre-existing passthrough test
canonical_session_model_preserves_unknown_passthrough_on_selected_provider
passes again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up cleanup on the publish-failures change.
Error model:
- Collapse `Error::{Engine, Publish, Handler}` into one `Error::Stage` with an
`ErrorStage` discriminator. The three shared a field shape and had to be
edited together in four match groups; nine near-identical constructors
become two private helpers.
- Add `Error::failure_reason()`, replacing the same error -> FailureReason
mapping written out in four places.
- Publish errors are now terminal. Publish runs once, after execution, so no
caller could ever act on the retryable classification.
Publish phase:
- Fix: a branch that was pushed is now still reported when pull request
creation fails afterwards. `PublishOutcome` records what happened and
carries the error separately, instead of hiding both behind a `Result`.
- Drop `PublishOutcome::NoChanges`, which no consumer distinguished from
`Published { pr_url: None }`.
- Move publish onto `Concluded` as methods and replace three near-identical
precondition guards with one `publish_target()`.
Pull requests:
- `maybe_open_pull_request` -> `open_pull_request` returning the record
directly. Both callers already reject empty diffs, so the `Ok(None)` path
was unreachable.
- Drop `CreatedPullRequest.head_sha`, which echoed back its own input.
GitHub client:
- Delete `branch_exists`, which had no callers and duplicated
`branch_head_sha`. Give `branch_head_sha` the `_with_client` split every
sibling has and port the tests to `MockHttpClient`.
- Collapse the copy-pasted credential match in `resolve_clone_credentials`.
Events:
- `PullRequestCreated.head_sha` is `Option<String>` instead of using an empty
string to mean absent.
- Centralize the run-branch refspec in `lifecycle::push_run_branch`, so
`git.push` reports a branch name from both emitters as documented.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- 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>
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>
`active_time_ms` was only ever computed from terminal stage events, so a
stage still running contributed zero to the run rollup. A run parked in one
long agent stage reported 2m 8s of active time against 16m 53s of wall
clock — the two finished stages — while the running stage had been doing
continuous inference and tool work for over 14 minutes.
`live_run_timing` summed `filter_map(|stage| stage.timing)`, and
`stage.timing` is only written at finalization. Wall time ticked live off
`start_time`; active time did not tick at all.
Stage projections now accumulate brackets from the event log:
- Closing an inference bracket folds its span into `live_inference_ms`
instead of discarding it, including across retries, matching the
in-process stopwatch.
- Tool calls open a batch on the first outstanding call and close it when
the last one drains, so tools running concurrently within a turn count
once — the same span `execute_tool_calls` is bracketed by. Summing
per-call durations would over-count parallel tool use. Subagent tool
events are excluded; they run inside the root call's span already.
- `StageProjection::live_timing(now)` composes accumulators with any open
bracket, per handler: agent stages use the brackets, prompt and command
stages count elapsed time as inference and tool respectively, and
handlers that wait on a human, timer, condition, or child branches
report zero.
Active is clamped to wall per stage. A worker killed mid-turn leaves its
bracket open forever, and without the clamp it would tick up unbounded.
The clamp does not need to detect the dead worker: a stage cannot have been
active longer than it has existed. `watchdog.timeout` remains the authority
on whether a run is stuck. The clamp is deliberately not applied at run
level, where concurrent branches can legitimately sum past run wall time.
Timing is derived from events rather than emitted by the worker, so this
needs no event-schema change and applies to runs already stored.
`StageProjection.timing` keeps its terminal-only meaning, and the
authoritative breakdown still replaces the live estimate at terminal
events.
The billing endpoint had the same hole behind its `wall_only` fallback:
running stages reported zero inference/tool/active. Not visible in the
product, which renders only `wall_time_ms`, but wrong for any other
consumer of `GET /runs/{id}/billing`.
Parallel branch stages lose their breakdown permanently, even after
completion, because `parallel.branch.completed` carries only `duration_ms`.
That is a separate data-loss bug, tracked in #644.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex drives the GPT-5.6 models with a much narrower tool set than the
other OpenAI models: a shell, `apply_patch`, and `update_plan`. It has no
file-read, file-write, grep, glob, or fetch tool at all -- reading and
searching go through the shell, and every write goes through
`apply_patch`. Offering 5.6 fabro's extra tools advertises affordances its
instructions never mention, so this adds a profile that registers only
what Codex does.
The profile is selected per model via `agent_profile = "gpt56"` on the six
5.6 rows (three each on `openai` and `openrouter`), following the existing
Kimi-over-a-gateway pattern. Every other model on those providers keeps
its provider default, with no code branch and no version sniffing.
- `ToolVocabulary::Codex` renames `shell` to `shell_command`; a strum
alias keeps `from_any_name` resolving it to `NativeTool::Shell`, so
permissions, categories, and telemetry still key on the canonical name.
- `shell_command` gains `workdir`, passed to the `cwd` argument
`execute_shell_command` already accepted, with Codex's "always set
`workdir`, do not `cd`" guidance.
- `prompts/gpt56.md.j2` is adapted from Codex's 5.6 `base_instructions`,
which are byte-identical across Sol, Terra, and Luna. A header comment
records provenance and the departures fabro's harness forces.
This is an alignment-only pass: it matches Codex's tool contract while
keeping direct tool calls. Codex actually drives 5.6 in code mode, with a
single `exec` tool taking JavaScript and every other tool reached through
a `tools` object inside a V8 isolate. That is deliberately out of scope.
Luna's `multi_agent_version: v1` (vs v2 on Sol and Terra) is also out of
scope. It only changes the sub-agent tool set, which fabro registers from
the caller rather than the profile, and fabro's current set matches
neither version exactly.
Two server cancel-timing tests are adjusted. `gpt-5.6-sol` is the
`openai` provider's default model, so runs that name no model now build a
3-tool profile instead of an 8-tool one and reach their first stage
sooner. `full_http_lifecycle_cancel` asserted `status.kind == "blocked"`
at the instant of cancel, which the worker is free to change the moment it
is signaled; it now accepts either live state, matching the tolerance its
own comment already documents for `pending_control`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tool names were string literals matched in several places, which made
renaming a tool for one profile unsafe: `tool_category` falls back to `Shell`
for an unrecognized name, so exposing `Read` instead of `read_file` would have
silently demanded shell-level approval for every file read.
Introduce `NativeTool`, the closed set of tools fabro implements, with strum
string conversions per the repo convention. A tool is an identity; a name is
one rendering of it. `ToolVocabulary` names the renderings -- fabro's own, and
Kimi Code's -- and `NativeTool::from_any_name` resolves a name in any
vocabulary back to the identity. Permissions, categories, and telemetry go
through that resolution, so behavior no longer depends on which profile is
running.
`known_tool_category` is now an exhaustive match on the enum rather than a
string match, so a new built-in tool has to state its category instead of
silently inheriting the unknown-tool default. Tools that are uncategorized
today stay uncategorized: giving them a category would change the CLI
permission gate, which is a behavior change rather than a cleanup.
MCP, skill, and run-scoped tools keep arbitrary string names, so
`ToolDefinition.name` and the registry keys stay `String`. The enum covers the
closed set only.
With that in place, the Kimi profile exposes its tools under Kimi Code's
vocabulary -- Read, Write, Edit, Bash, Grep, Glob, WebSearch, FetchURL -- and
its prompt and tool descriptions use those names. Tools with no Kimi Code
counterpart of the same shape keep fabro's names. Ask Fabro's tool policy
resolves through the canonical name so a Kimi-model run is not denied its
whole tool set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kimi models ran on the OpenAI profile, which exists to look like Codex. Give
them their own profile derived from Kimi Code's system prompt.
Routing is per model, not per provider, because Kimi models are served both
directly by Moonshot and through gateways. `kimi` sets agent_profile at the
provider level; the Kimi model rows on `openrouter` set it individually, so a
gateway route behaves like the direct one while other OpenRouter models keep
the provider's OpenAI profile.
The profile targets a measured failure. Across two observed K3 implementation
stages, 32 of 35 tool failures were the same thing: writes to files the model
had not read, rejected by the workspace read-before-write guard, or
`old_string` values reconstructed from memory rather than taken from a read.
Kimi Code drills this rule in its own tool descriptions, so the profile does
too -- `edit_file` and `write_file` carry Kimi-specific descriptions naming the
guard and the failure text the model will see, alongside a "Reading Before
Writing" section in the system prompt. Profiles own their tool registries, so
this re-describes the tools for Kimi only; every other profile is untouched and
the executors and JSON schemas are shared unchanged.
Tool names stay fabro's existing snake_case. Whether Kimi Code's PascalCase
vocabulary measurably helps is untested, and renaming would also mean updating
the name-keyed categories in tool_permissions.rs, where an unknown tool falls
back to Shell. That is a separate change to make on evidence.
The prompt is a subtractive port: capabilities fabro does not have -- plan
mode, background tasks, cron, subagent swarms, the cwd tree listing -- are
dropped rather than promised. The shell timeout default matches Kimi Code's 60s
and memory discovery reads AGENTS.md, which is the only instruction file Kimi
Code looks for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit introduced render_prompt and splice_optional_section, a
second templating mechanism in a workspace that already standardizes on
MiniJinja behind fabro-template. Drop both and render the profile prompts the
same way fabro-workflow and fabro-manifest render theirs.
Expressing the conditionals as {% if %} lets every profile collapse to a single
template, since the optional blocks no longer need to be separate files spliced
in from Rust:
before: 6 files + 2 splice helpers, prompt prose split across .md and .rs
after: 3 files, one per profile, all prose in the template
Rust now passes only facts -- provider name, which file-edit tool is active,
and whether web search and subagents are available. Values land under `vars`,
so templates read {{ vars.env_block }}. Booleans are passed as "true"/"false"
and compared explicitly via the bool_var helper, because the shared
TemplateContext types vars as strings and a bare {% if %} on the string
"false" would be truthy.
Also converts fabro-server's Ask Fabro prompt, which is assembled at runtime.
Its tool guidance now arrives as a template variable instead of being
interpolated into the template text. That guidance carries tool names and
descriptions that can originate from MCP servers, and MiniJinja does not
re-render substituted values, so a tool description containing {{ ... }} stays
inert rather than being evaluated.
Output is unchanged. Verified by diffing all ten prompt variants against the
same unmodified origin/main worktree used for the previous commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Guard EventScan seeks against sequences past MAX_EVENT_SEQ: a
seven-digit start prefix sorts below six-digit event keys, so an
unvalidated since_seq like 5000000 returned an incorrect slice of
history instead of an empty page. An end bound past MAX_EVENT_SEQ now
delegates to the unbounded scan, which is equivalent because no stored
sequence exceeds it.
Clamp the descending exclusive end to just past the newest stored
event, so an oversized before_seq cursor pages from the newest event
instead of probing empty key space and returning nothing.
Split RunEventListParams out of EventListParams so before_seq and
order are only accepted by /runs/{id}/events; the session, stage,
pair transcript, and demo endpoints go back to ignoring them instead
of accepting order=desc while returning ascending results.
Co-Authored-By: Claude Fable 5 <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>
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>
Resolved conflict in run_store.rs tests: kept both the new
list_events_before_with_limit tests from this branch and the
append_event_rejects_sequences_beyond_key_order_limit test from main.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add AppState::cached_run with the standard 500/404 mapping and use it
everywhere handlers read the shared run-projection cache. This also
normalizes two inconsistencies: graph-source cache errors now map to
500 (was 502), and a missing projection in PR create/unlink now maps
to the canonical 404 (was a bespoke 500).
- Extract an EventScan cursor shared by the four run-event scan loops,
delegate list_events_from to the paginated variant, and stop the
stage-event scan once its page is full instead of walking the rest of
the log.
- Hold Arc<RunProjection> in the local projection cache so opening a run
no longer deep-copies the projection (copy-on-write via Arc::make_mut),
and drop the now-unreachable shared-cache branch in last_event_seq.
- Trim hot-path clones: run_files serves the projection Arc directly,
run-state serializes by reference, artifacts only checks existence, and
the command-log handler opens a reader only for the CAS-blob branch.
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>
