Commit graph

2582 commits

Author SHA1 Message Date
Bryan Helmkamp
eddee10b35
fix(agent): harden Kimi profile tool contracts 2026-07-24 22:05:48 -04:00
Bryan Helmkamp
e1d0b1af4f
refactor(llm): make StreamStart a universal liveness edge
`StreamStart` was supposed to mean "the provider is responding", but
each decoder decided for itself when to emit it, so it meant something
different per dialect:

  anthropic       on the `message_start` frame
  bedrock         on the `messageStart` frame
  openai_responses  latched on the first SSE event
  gemini          latched on the first chunk
  openai_compatible  never — Chat Completions has no opening frame

A consumer could not rely on it, which is why the inference-bracket
work keyed its first-output edge on content kind instead.

Ownership moves to the two loops that drive decoders — the shared SSE
loop in `transport.rs` and the AWS event-stream loop in the bedrock
provider — each emitting exactly one `StreamStart` immediately before
handing over the first framed event. The invariant is now structural:
it cannot depend on a dialect having a particular opening frame,
because no decoder is involved in producing it. `StreamDecoder`
documents that decoders must not emit it, and the four that did no
longer do.

Only `openai_compatible` changes observably, gaining the event it never
had; the other three dialects' snapshots are byte-identical, because
their opening frame was already the first framed event. The six
updated `openai_compatible` snapshots each differ by exactly one
leading `stream_start` and nothing else.

Each dialect also gets an explicit `stream_opens_with_stream_start`
assertion. The snapshots already cover this, but a snapshot can be
re-accepted silently, and this is the one event a liveness consumer
needs to hold for every provider.

No behavior change to the agent's inference bracket:
`first_output_kind()` maps `StreamStart` to `None`, so the bracket
still opens on observed content and keeps reporting which kind
arrived. The point of this change is that a content-agnostic edge now
exists at all — the one-shot coverage follow-up needs it, and it is
strictly earlier than first content.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 22:05:26 -04:00
Release Repro
5d0617f547
fix(agent): harden compaction reasoning budgets
Model default reasoning explicitly at the provider-route level so always-reasoning endpoints without effort controls receive summary headroom. Cap all summary requests at model output limits and bound retained visible summaries to the original allowance. Reuse builtin catalog fixtures and named budget constants in tests, and document the new model setting.
2026-07-24 21:58:23 -04:00
Bryan Helmkamp
1ca9fe977d
fix(sandbox): simplify Bash contract implementation 2026-07-24 21:55:36 -04:00
Bryan Helmkamp
c803354309
refactor(agent): streamline shell outcome reporting 2026-07-24 21:50:07 -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
51a775ea22
Merge remote-tracking branch 'origin/main' into fix/compaction-reasoning-token-budget 2026-07-24 21:37:05 -04:00
Bryan Helmkamp
cbd257c016
feat(agent): give the Kimi profile Grep's output modes and paging
Kimi Code's Grep returns matching lines, matching file names, or per-file
counts, and pages results with `head_limit` and `offset`. All four are shapes
of the result list the Sandbox trait already returns, so the Kimi profile gets
them without any provider work.

Scoped to the Kimi profile. The other profiles keep fabro's grep tool: these
options exist because Kimi models are trained against them, not because every
model should be handed more knobs.

Two details worth knowing when reading it. Extracting a file path means
parsing the `<path>:<line>:<content>` prefix, which the underlying search omits
when scanning a single file, so the search root is the fallback; the parser
also walks candidate separators so a colon inside matched content is not
mistaken for the line-number field. And `head_limit` is only pushed down to the
search as a result cap in `content` mode, where results and lines are the same
thing -- capping lines early would undercount files for the other two modes.

Kimi Code's `type`, `multiline`, and `include_ignored` are still absent. They
would have to reach ripgrep flags through new Sandbox trait methods
implemented across the local, Docker, and Daytona providers, and a parameter
that is advertised but ignored is worse than one that is missing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 21:35:02 -04:00
Bryan Helmkamp
c464e1b91c
feat(agent): implement Read, Write, and Bash to Kimi Code's contract
Three Kimi Code tools differ from fabro's built-ins in what their parameters
mean, not just what they are called. Renaming fabro's parameters would have
advertised behavior fabro does not have, so these are separate tools:

- Bash takes `timeout` in SECONDS where fabro takes milliseconds, and accepts a
  `cwd`. A rename alone would have made every timeout 1000x wrong -- silently,
  since nothing validates the magnitude.
- Read accepts a NEGATIVE `line_offset`, meaning "read the last N lines".
  Fabro's `offset` has no such meaning, so the tool counts the file's lines and
  converts to an absolute start.
- Write takes a `mode`, so it can append. The Sandbox trait has no append, so
  append is read-modify-write, which keeps every provider working and stays
  inside path policy.

Everything reaches the environment through the same Sandbox methods the
built-ins use, so sandbox behavior, path policy, and the read-before-write
guard are unchanged. Tools register under their canonical names and the
registry's vocabulary renames them, so the Kimi profile does not special-case
naming twice.

Edit needed no new tool: `old_string`, `new_string`, and `replace_all` already
match Kimi Code exactly, and `file_path` versus `path` is a pure rename.

Grep and Glob are not converted. Their shared parameters already behave
identically; the gap is optional capability fabro lacks -- Grep's `type`,
`multiline`, and `include_ignored`, and Glob's `include_dirs` and
`include_ignored` -- which needs new Sandbox trait methods implemented across
the local, Docker, and Daytona providers. Omitting an optional parameter is
honest; renaming one whose semantics differ is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 21:28:47 -04:00
Bryan Helmkamp
7501ada9e6
Merge pull request #629 from fabro-sh/fix/compaction-empty-summary-guard
fix(agent): refuse to truncate history on a degenerate compaction summary
2026-07-24 21:27:51 -04:00
Bryan Helmkamp
3606ba6a0f
feat(sandbox): standardize command execution on non-login Bash
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>
2026-07-24 21:26:05 -04:00
Release Repro
af647aba4b
fix(cli): avoid duplicate compaction error prefix 2026-07-24 21:24:31 -04:00
Bryan Helmkamp
392b8dd27b
fix(agent): report real shell process outcomes
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>
2026-07-24 21:24:16 -04:00
Bryan Helmkamp
21b90bad00
test(agent): pin that Kimi tool descriptions stay scoped to the Kimi profile
The Kimi profile rewrites several built-in tool descriptions. Every profile
builds its registry from the same factories, so a change made in the wrong
place would reword tools for models that were never meant to see it, and
nothing would fail.

Assert the isolation directly: for each shared built-in, Kimi's description
differs from Anthropic's, OpenAI and Gemini match Anthropic's stock wording,
and the read-before-write phrasing appears nowhere but Kimi.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 21:19:28 -04:00
Bryan Helmkamp
5349e9d99c
feat(agent): give the Kimi profile Kimi Code's tool descriptions
Edit and Write already carried Kimi-specific descriptions; the other four
built-ins were still fabro's one-liners, roughly 150-220 characters against
Kimi Code's 1-5KB. Port Bash, Read, Grep, and Glob the same way.

Bash is the largest and the most useful: most of its length is an explicit
translation table steering shell usage to the dedicated tools -- cat to Read,
sed to Edit, find to Glob, grep to Grep -- under the names this profile
exposes. It also states that each call runs in a fresh bash process, so `cd`
and environment variables do not persist, and that a command which timed out
needs a raised `timeout_ms` rather than a retry. Two of the observed K3 tool
failures were shell timeouts.

The port stays subtractive. Kimi Code's Bash documents background execution,
TaskOutput, TaskStop, and a `cwd` argument; fabro's shell has none of those, so
none of it is claimed. Read drops Kimi Code's media and paging specifics that
do not match fabro's offset/limit, and gains the fact that reading a file is
what clears it for writing. Grep deliberately does not promise ripgrep syntax:
fabro falls back to POSIX grep when rg is absent, so the description asks for
portable patterns instead.

Bash quotes the timeouts this profile actually enforces by interpolating them
from NativeToolOptions, so the description cannot drift from behavior. Tests
assert the interpolation rendered, that the translation table names the exposed
tools, and that no background-execution guidance leaked in.

Parameter names stay fabro's. Kimi Code's differ (`path` and `line_offset`
where fabro has `file_path` and `offset`), but across roughly 1200 tool calls
in two observed K3 runs there were no schema or missing-parameter errors, so
the model reads the schema it is given. Renaming parameters would be churn
against a hypothesis the data does not support.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 21:14:23 -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
9604d2ac9d
fix(agent): apply the Kimi vocabulary to every tool, not just the early ones
Renaming ran as a pass at the end of profile construction, so it only covered
tools registered by that point. Subagent tools arrive later via
`register_subagent_tools`, and the skill tool is registered when a session
discovers skills, so a Kimi profile actually exposed a mixed set:

  Read Write Edit Bash Grep Glob FetchURL TodoList     renamed
  spawn_agent send_input close_agent wait use_skill    missed

Move the vocabulary into ToolRegistry instead of applying it as a pass.
`register` renames built-ins on the way in, so registration order stops
mattering and a late registration cannot slip through. `ToolRegistry::new`
keeps the fabro vocabulary, so no other profile changes.

`use_skill` now exposes as `Skill`, matching Kimi Code, which has the same
semantics. The subagent tools stay under fabro's names on purpose: Kimi Code's
`Agent` launches a subagent and returns its result, while fabro's spawn_agent
returns a handle that send_input, wait, and close_agent drive. Borrowing the
name without the semantics would promise a result the tool does not return --
the same mistake as exposing incremental task tools under a whole-list name.

The skills prompt section hardcoded `use_skill`, which under this vocabulary
names a tool the model was not given. It takes the exposed name now, threaded
through EmbeddedPrompt so a profile's prompt and its registry cannot disagree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 20:27:53 -04:00
Bryan Helmkamp
ddddc5bb33
feat(agent): give the Kimi profile Kimi Code's TodoList tool
The Kimi profile was registering the Anthropic task tools. Both persist
through the same TodoRuntime, but they model opposite interactions: TaskCreate
and TaskUpdate mutate individual tasks against tracked ids, while Kimi Code's
TodoList replaces the whole list in one call. Of the two surfaces fabro already
had, Kimi was given the one furthest from what its models are trained on.

Add TodoListKind::KimiTodos and a TodoList tool matching Kimi Code's contract
exactly:

  TodoList({ todos?: [{ title, status: pending | in_progress | done }] })

Omitting `todos` reads the list, an empty array clears it, and a list replaces
it. Reconciliation mirrors update_plan -- items are identified by their text,
so re-submitting a list preserves identity for unchanged entries -- and the
runtime, projections, and events are unchanged.

Two differences from the existing surfaces were behavioral rather than
cosmetic. Items carry only `title`, where TaskCreate requires both `subject`
and `description`, so a model with nothing to say for a description had to
invent one. And the terminal status is spelled `done`; `completed` is the
Anthropic and Codex spelling, and a model emitting `done` against the old
schema got a validation error rather than a todo. The internal representation
stays TodoStatus::Completed; only the wire vocabulary differs.

Kimi todo lists are session-scoped like OpenAI plans, so the root-agent
projection excludes subagent lists the same way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 20:18:29 -04:00
Bryan Helmkamp
fbff6f5774
refactor(agent): model built-in tools as an enum with per-profile vocabularies
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>
2026-07-24 20:04:11 -04:00
Bryan Helmkamp
c08e5c5490
feat(agent): add a Kimi agent profile for Moonshot and gateway routes
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>
2026-07-24 19:50:58 -04:00
Bryan Helmkamp
63c952e380
Merge pull request #628 from fabro-sh/refactor/agent-md-prompts
Some checks are pending
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
TypeScript / Build (push) Waiting to run
refactor(agent): render profile system prompts with minijinja templates
2026-07-24 19:31:48 -04:00
Bryan Helmkamp
f94955ede5
fix(agent): budget compaction summaries for reasoning models
Compaction summarizes the conversation with the session's own model, but
hard-coded `max_tokens: Some(4096)` and sent no `reasoning_effort`. On a
reasoning model that ceiling covers thinking *and* visible output, so a
long conversation can exhaust it on reasoning alone and return a
successful response with empty content — silently replacing the compacted
history with an empty summary.

The Anthropic codec's existing clamp does not cover this path: it only
runs when the request carries a `reasoning_effort` and the model has no
native effort parameter. Compaction sends `reasoning_effort: None`, so
encoding falls through to the branch that injects `{"type": "adaptive"}`
for `levels` models with no clamp at all, and the openai_compatible and
openai_responses codecs pass `max_tokens` straight through.

Resolve the budget from the catalog instead. Models whose endpoint
reasons without being asked (`always_adaptive` natively, `levels` via
default adaptive thinking or the provider's default effort) get 16K of
reasoning headroom above the 4096-token summary allowance, capped at the
model's own `max_output`. Models with no reasoning-effort feature never
reason on this path and keep the existing 4096.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 19:11:25 -04:00
Bryan Helmkamp
fd6f14f107
refactor(agent): simplify prompt template rendering 2026-07-24 19:10:48 -04:00
Bryan Helmkamp
b17b8aeaed
fix(agent): refuse to truncate history on a degenerate compaction summary
When the summarization LLM call returned an empty completion, compaction
truncated the conversation anyway. `history.compact_from` discarded the
summarized turns irreversibly, `CompactionCompleted` was emitted as if
nothing had gone wrong, and the replacement system turn contained only the
handoff preamble: "A different assistant began this task and produced the
following summary" followed by nothing.

The agent then continued with zero context while having been explicitly
told a handoff summary existed. It presents to a user as the agent
suddenly forgetting everything, and the only trace was a `debug!` line
that is off by default, so there was nothing in production logs to
correlate against.

This is provider-independent. Any completion that comes back empty
triggers it: a truncated stream, a reasoning model that spends its whole
token budget on reasoning, or a rate-limit edge.

Validate the summary before mutating history. A summary that is empty,
whitespace-only, or shorter than 32 bytes after trimming is refused: the
history is left fully intact and an error is returned instead. The
threshold is deliberately far below any genuine summary — 32 bytes is
shorter than a single source file path — because this guards against
degenerate responses, not summary quality, and a false refusal would let
the context keep growing. Structure is not validated, since a model may
legitimately vary the requested section format.

Returning `Err` is sufficient to surface the failure. `compact_if_needed`
already converts it into an `AgentEvent::Error`, which lands in the run
event stream and logs at ERROR via `AgentEvent::trace`, and the session
continues rather than dying — behavior already covered by
`compaction_failure_is_non_fatal`.

The canned summary in `compaction_includes_structured_prompt_and_file_tracking`
was 26 bytes, which the new guard rejects. That test verifies the
summarization request prompt and file tracking, not minimum summary
length, so its fixture is now a realistic summary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 19:10:13 -04:00
Bryan Helmkamp
1d685cbea4
refactor(agent): render profile prompts with minijinja via fabro-template
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>
2026-07-24 18:23:47 -04:00
Bryan Helmkamp
2c13e2da5e
refactor(agent): load profile system prompts from .md files
The three profile prompts lived as multi-line Rust string literals with
backslash continuations, which made them awkward to read, diff, and review.
Move them to profiles/prompts/*.md loaded via include_str!, matching the
existing convention in fabro-workflow's pipeline prompts and the server's
playground prompt.

Two helpers back the templates, both following the {env_block} convention
already used by assemble_system_prompt rather than adding a template engine:

- render_prompt substitutes {name} placeholders and leaves the rest intact,
  for values spliced inline (provider name, the web-search bullet)
- splice_optional_section handles whole blocks that come and go, dropping the
  blank line ahead of the placeholder when the block is empty

Gemini already carried a {web_search_section} placeholder in its literal, so
that one maps onto render_prompt unchanged. Anthropic's per-section functions
collapse into a single template plus a subagent fragment. OpenAI keeps its two
one-line file-edit failure hints inline, since they are bound to the tool name
and would not read well as standalone files; the multi-line usage blocks they
pair with become fragments.

Output is unchanged. Verified by capturing all ten prompt variants -- Anthropic
across subagent x web-search, OpenAI across apply_patch/edit_file x web-search,
Gemini across web-search -- from an unmodified worktree at origin/main, then
diffing them byte for byte against this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 17:55:08 -04:00
Bryan Helmkamp
8921fc533f
Merge pull request #622 from fabro-sh/feat/stage-chat-view
feat(web): add Chat view for agent stages
2026-07-24 17:54:19 -04:00
Bryan Helmkamp
7044dd57fa
Merge pull request #627 from fabro-sh/fix/openai-compatible-stream-usage
fix(llm): request streaming usage on openai_compatible providers
2026-07-24 17:53:45 -04:00
Bryan Helmkamp
ae3b6702e2
Merge pull request #626 from fabro-sh/feat/passive-reasoning-capture
feat(reasoning): passive reasoning capture in agent.message
2026-07-24 17:49:46 -04:00
Bryan Helmkamp
53a92f75f4
test(llm): model streamed usage in OpenAI twin 2026-07-24 17:47:05 -04:00
Release Repro
512ab50f9c
fix(reasoning): align stream and client invariants 2026-07-24 17:42:44 -04:00
Bryan Helmkamp
e739f86f6a
Merge remote-tracking branch 'origin/main' into feat/stage-chat-view
# Conflicts:
#	apps/fabro-web/app/routes/run-stages.test.ts
#	apps/fabro-web/app/routes/run-stages.tsx
2026-07-24 17:39:48 -04:00
Bryan Helmkamp
2c6fd5d798
Merge pull request #625 from fabro-sh/fix/brave-search-profile-config
Fix Brave Search tool secret propagation
2026-07-24 17:36:48 -04:00
Bryan Helmkamp
4ce57f8aae
refactor: simplify profile builder and drop dead tool plumbing
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>
2026-07-24 17:32:33 -04:00
Release Repro
4d3de5f564
fix(reasoning): tighten capture normalization 2026-07-24 17:31:14 -04:00
Bryan Helmkamp
0169725b4e
fix(llm): request streaming usage on openai_compatible providers
Chat Completions only emits the trailing usage chunk when the request sets
`stream_options: {"include_usage": true}`. The openai_compatible codec never
sent it, so providers that follow the spec strictly returned no usage at all
on streamed responses. Every message came back with zero tokens, and the
catalog cost estimate multiplied those zeros into $0.

Kimi is the visible case: a run's kimi-k3 stages report 0 tokens and no
dollars, while an openrouter stage in the same run bills normally because
OpenRouter volunteers usage (and an in-band cost) without being asked.

Send the opt-in whenever we stream. Providers that already volunteer usage
accept the field and are unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-24 16:21:00 -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
e15defe4c5
refactor: centralize agent profile tool configuration 2026-07-24 14:06:29 -04:00
Release Repro
0b58d087ee
feat(model): add Claude Opus 5 2026-07-24 13:43:08 -04:00
Bryan Helmkamp
fec5021a29
fix: pass tool secrets to agent profiles 2026-07-24 13:36:45 -04:00
Bryan Helmkamp
2539e3a661
feat(web): add Chat view for agent stages
Adds a Chat tab to agent stage pages alongside Thread and Debug, styled
after the Ask Fabro sidebar: agent messages render as first-class chat
bubbles (the narration between tool batches is the content that matters),
the stage prompt is a collapsed user-side card, and each run of
consecutive tool calls collapses to a wrench-icon count chip. While the
stage is running, in-flight tool calls (agent.tool.started without a
completed event) show as a live spinner line with the tool name and input
preview — data the Thread view drops today.

Thread remains the default tab; Chat becomes the default only after
production testing.

Also fixes the demo dataset: detect-drift carries the agent-flavored
stage events (prompt, agent messages, tool calls) but was labeled a
command stage, so its Thread/Chat views were unreachable in demo mode.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 13:17:57 -04:00
Bryan Helmkamp
71549f0917
Merge remote-tracking branch 'origin/feat/backward-event-pagination' into feat/backward-event-pagination 2026-07-24 10:21:40 -04:00
Bryan Helmkamp
b77116994f
Harden event pagination bounds and scope desc params to the run route
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>
2026-07-24 10:21:25 -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
7e7d7e4457
Recover latest event seq with bounded probes instead of a full scan
On a projection-cache miss, descending pagination recovered the latest
sequence by scanning the run's entire event prefix, making a cold-cache
order=desc request O(total_events). Binary-search the zero-padded
sequence key space with single-entry probes instead, bounding recovery
to O(log MAX_EVENT_SEQ) reads. The probe predicate (smallest stored
sequence at or above a bound) stays monotone across gaps left by
failed appends.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 10:05:01 -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
b886f82622
Clamp backward pagination end bound to the event key-order limit
Event keys zero-pad seq to six digits, so an exclusive end bound past
MAX_EVENT_SEQ formatted as a seven-digit prefix that sorts before real
event keys, producing an inverted scan range. This made the newest page
come back empty once a run reached MAX_EVENT_SEQ, and let a client
supplied before_seq beyond MAX_EVENT_SEQ garble the range. Clamp the
bound and treat anything past MAX_EVENT_SEQ as unbounded; no stored
sequence exceeds it, so the results are equivalent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 09:50:40 -04:00
Bryan Helmkamp
17cf8d710e
Merge pull request #618 from fabro-sh/fix/shared-run-projection-cache
Reuse shared projections for current run state
2026-07-24 09:47:29 -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