Commit graph

182 commits

Author SHA1 Message Date
Bryan Helmkamp
f0a7423b51
refactor(config): stop resolving {{ env.* }} in interpolated config
The process environment is no longer a configuration source. `{{ vars.NAME }}`
(non-sensitive, server-stored) and `{{ secrets.NAME }}` (vault-backed) cover
both cases, and reading the worker's ambient environment made a run's inputs
depend on how its process happened to be launched.

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

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

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

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

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

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

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

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

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

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

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 20:36:54 -04:00
Bryan Helmkamp
9e744f8072
Keep server tests off the developer's real ~/.fabro/storage
Test settings usually omit `[server.storage] root`, so it resolved to the
production default. Handlers that walk that tree read whatever the machine
happened to have.

That is why all_spec_routes_are_routable was slow. Timing every request in
it showed 91% of the runtime in two routes:

  6304ms  GET /api/v1/system/resources
  4574ms  GET /api/v1/system/df
   583ms  POST /api/v1/system/prune/runs
  ...
  the remaining 134 operations: 8ms combined

Both size Fabro-managed storage. On this machine that meant 193MB and 90,795
entries under scratch/, so the test's duration tracked how long the developer
had been running Fabro locally. Run-creating tests were writing there too.

Redirect settings that still carry the production default to a `storage`
directory beside the test vault, alongside the existing `server.env` and
`settings.toml` siblings. A test that chose its own root keeps it.

all_spec_routes_are_routable drops from ~15s to 0.6s, and the full workspace
run from ~33s to ~21s. All 7402 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 18:45:27 -04:00
Bryan Helmkamp
99dd7718c0
Keep server tests off the developer's real ~/.fabro/storage
Test settings usually omit `[server.storage] root`, so it resolved to the
production default. Handlers that walk that tree read whatever the machine
happened to have.

That is why all_spec_routes_are_routable was slow. Timing every request in
it showed 91% of the runtime in two routes:

  6304ms  GET /api/v1/system/resources
  4574ms  GET /api/v1/system/df
   583ms  POST /api/v1/system/prune/runs
  ...
  the remaining 134 operations: 8ms combined

Both size Fabro-managed storage. On this machine that meant 193MB and 90,795
entries under scratch/, so the test's duration tracked how long the developer
had been running Fabro locally. Run-creating tests were writing there too.

Redirect settings that still carry the production default to a `storage`
directory beside the test vault, alongside the existing `server.env` and
`settings.toml` siblings. A test that chose its own root keeps it.

all_spec_routes_are_routable drops from ~15s to 0.6s, and the full workspace
run from ~33s to ~21s. All 7402 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 18:39:25 -04:00
Bryan Helmkamp
713d340db7
Render the run title prompt with Jinja instead of string replacement
The prompt used hand-rolled `{placeholder}` substitution via `str::replace`.
The app already has a MiniJinja layer for exactly this, and every other
checked-in prompt uses it, so use it here too.

`prompts/run_title.md` becomes `prompts/run_title.md.j2` with `{{ inputs.* }}`
variables, rendered through `fabro_template::render_named`. Strict undefined
handling now catches a variable the template asks for and the caller does not
supply, which the old `.replace()` chain silently left as literal text.

`build_title_prompt` returns `Result` accordingly. A checked-in template that
will not render is a bug rather than a transient failure, so the caller logs
it at `warn` — louder than the `debug` used for a generation miss — and keeps
the deterministic title.

Re-checked against claude-haiku-4-5: same titles as before the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 17:23:16 -04:00
Bryan Helmkamp
88ca4b1d03
Teach the run title model the title shape we want
Move the run title prompt into `src/prompts/run_title.md` and load it with
`include_str!`, matching the playground and ask-fabro prompts. Placeholder
substitution replaces `format!`, so the literal `{"title":"..."}` in the
prompt no longer needs brace escaping.

The old instructions only said "concise" and "preserve ticket IDs", so a
work order run titled itself with the raw file path. The prompt now asks for
a pull-request-shaped title: leading verb, identifier in canonical uppercase,
then a description with paths, date prefixes, and extensions stripped and
slug hyphens turned back into words. Three worked examples carry the shape.

Checked against claude-haiku-4-5 at the existing 64-token budget:

  Implement Conveyor Work Order docs/planning/orders/2026-07-22-wrk-004-operational-diagnostics.md
    -> Implement WRK-004: Operational diagnostics
  Fix flaky checkout test (input branch release-9.2)
    -> Fix flaky checkout test on release-9.2

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 17:11:52 -04:00
Release Repro
c812274db8
Show live status for parallel branches 2026-07-27 17:08:08 -04:00
Bryan Helmkamp
7841a77f2c
feat(web): show stage tokens and cost in the model popover
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>
2026-07-27 16:06:19 -04:00
Release Repro
69a51e65b9
feat(workflow): infer command nodes from the script attribute
A node with no `shape` defaulted to `box`, which resolves to the agent
handler. That made a shapeless `script` node run as an LLM call prompted
with its own label, while the `script` was reported as inert — wrong
behavior behind a warning.

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

Two rules keep the inference honest:

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

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

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

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

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

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

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

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

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

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 13:53:54 -04:00
Bryan Helmkamp
b53045a1ac
Add runtime for_each item injection 2026-07-27 11:55:55 -04:00
Bryan Helmkamp
1c82bd9008
fix(workflow): make publish failures terminal 2026-07-27 11:25:18 -04:00
Bryan Helmkamp
0b24649e76
fix(cli): keep offline validation catalog-free 2026-07-26 09:25:47 -04:00
Bryan Helmkamp
dd9f75fb05
fix(timing): harden live active projections 2026-07-25 23:43:43 -04:00
Bryan Helmkamp
d669a2d55c
fix(agent): correct background-agent notification delivery
Three correctness fixes in the Claude 5 background-agent path, plus
cleanups from a reuse/quality/efficiency review pass.

Fixes:

- Background-agent output was run through skill expansion. A child that
  wrote a bare path ("cleaned up /tmp") failed the whole parent turn with
  `Unknown skill: /tmp`, and a child whose output happened to name a real
  skill had its report replaced by that skill's template. Synthesized
  harness turns now skip expansion; only text the user typed can invoke a
  skill.

- `begin_shutdown` suppressed the pending notification before deciding
  whether a shutdown would happen. Stopping an agent that had just
  finished rejected the stop *and* discarded the result the parent was
  owed. Suppression now happens only once shutdown is committed.

- `spawn_inner` registered the notification after publishing the agent in
  `state.agents`, so a concurrent `shutdown_all` in that window left a
  pending entry the monitor never completes, and the parent's drain loop
  would never see the queue as drained. Registration now precedes
  publication.

- `TaskOutput.timeout` was declared `number` but parsed with `as_u64`, so
  a schema-valid `30000.0` failed at runtime.

- Update the fabro-server alias test for the `sonnet` alias moving to
  Claude Sonnet 5.

Cleanups:

- The supervisor renders the notification turn; `Session` no longer knows
  the envelope format.
- Replace six near-identical prompt snapshots with a property test over
  all eight conditional combinations, keeping the default and
  all-conditionals snapshots for wording.
- Collapse `TodoRuntime`'s two mutexes into one.
- Read the prompt vocabulary from the registry instead of hardcoding it.
- Drop internal vocabulary from the `SendMessage` tool description.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-25 15:15:39 -04:00
Bryan Helmkamp
c4971b93d3
fix(timing): accumulate active time for in-flight stages
`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>
2026-07-25 14:54:38 -04:00
Bryan Helmkamp
695a981f42
feat(web): tell open tabs when a new build ships
A tab left open across a deploy keeps running the previous build's
JavaScript indefinitely. index.html is fetched only on a full page load,
all later navigation is client-side, and hashed bundles are served
`immutable`, so nothing reveals that the code is stale. This produced a
false-positive bug report where two correctly-deployed fixes appeared to
be missing.

Publishes a build id and offers a reload when the running document falls
behind. The toast never reloads on its own; the only automatic reload is
recovery from a chunk that no longer exists.

Build id derivation
-------------------
The obvious approach — hash the emitted asset filenames, which already
embed content hashes — does not work: Bun's minified identifier naming is
not deterministic. Building an unchanged tree twice produces byte-different
output roughly one run in three (same length, ~100k differing bytes, all of
it mangled names). Output hashes therefore move with no source change,
which would fire the toast on redeploys of identical code and train people
to ignore it.

The id is instead derived from the bundle's source inputs, so it changes if
and only if something we control changed. Verified stable across eight
consecutive builds while the entry hash flipped between both variants.

This non-determinism also means two builds of the same commit embed
different bytes into the server binary, which is worth addressing
separately for reproducible builds.

Detection
---------
SWR with `refreshInterval` + `revalidateOnFocus`, per the repo's React
effects policy. SWR does not poll while the document is hidden, so
background tabs stay quiet without extra gating. Unknown state on either
side — missing meta tag, failed fetch, 503 during a dev rebuild — never
produces a prompt.

Stylesheet hashing
------------------
Tailwind's output was stable-named and therefore served `no-cache`, letting
a tab revalidate into new CSS while running old JS. Tailwind purges unused
classes per build, so classes the old bundle still emits could silently
lose their styles. It is now content-hashed and moves with the build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-25 14:45:26 -04:00
Bryan Helmkamp
e2df6e68c7
Merge pull request #640 from fabro-sh/codex/workspace-glob-semantics
Unify workspace glob semantics across sandboxes and artifacts
2026-07-25 12:05:17 -04:00
Bryan Helmkamp
dec67ec92e
fix(glob): harden artifact traversal 2026-07-25 11:56:57 -04:00
Bryan Helmkamp
d931ae6105
fix(agent): simplify GPT-5.6 tool routing 2026-07-25 11:49:40 -04:00
Bryan Helmkamp
c7ad387d3e
feat(agent): add gpt56 profile for GPT-5.6 Sol, Terra, and Luna
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>
2026-07-25 10:42:32 -04:00
Bryan Helmkamp
c81ea69c73
Merge origin/main into feat/inference-observability 2026-07-24 22:55:15 -04:00
Bryan Helmkamp
d4f619bc2a
fix: clean up inference observability 2026-07-24 22:50:00 -04:00
Bryan Helmkamp
fea249b4b6
Merge pull request #631 from fabro-sh/feat/kimi-agent-profile
feat(agent): add a Kimi agent profile for Moonshot and gateway routes
2026-07-24 22:20:50 -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
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
Release Repro
af647aba4b
fix(cli): avoid duplicate compaction error prefix 2026-07-24 21:24:31 -04:00
Release Repro
7eef7652d3
fix(agent): harden compaction failure handling
Accept every nonblank summary instead of applying an arbitrary length heuristic. Preserve typed compaction failures and their source chains, suppress repeat attempts within one input, and clear the CLI compaction indicator when the existing agent error event arrives.
2026-07-24 20:57:57 -04:00
Bryan Helmkamp
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
fd6f14f107
refactor(agent): simplify prompt template rendering 2026-07-24 19:10:48 -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
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
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
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
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
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
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
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
Bryan Helmkamp
94e4a38375
Merge remote-tracking branch 'origin/main' into fix/shared-run-projection-cache
# Conflicts:
#	lib/components/fabro-store/src/slate/run_store.rs
2026-07-24 09:41:58 -04:00
Bryan Helmkamp
78fea736e3
fix: harden stage execution identity on resume 2026-07-24 09:37:05 -04:00