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>
`Claude5QuestionToolArgs`/`Claude5Question`/`Claude5Option` differed from
the Anthropic trio only in required-ness -- `header: String` rather than
`Option<String>`, same for each option's `description`. The JSON Schema
already enforces that at the model boundary, so the lenient structs
deserialize the strict payload unchanged.
`normalize_claude5_questions` then reproduced `normalize_anthropic_questions`
plus an inlined copy of `options_from_anthropic`, so `option_key`,
`display_text`, and `bounded_display_field` were each applied in two
places and could drift.
Replace both with one normalizer taking a `QuestionLimits`. The genuine
Claude 5 deltas -- at most four questions, two to four options, a
twelve-character header cap, required header and option descriptions, and
no previews on multi-select -- become data rather than a second code path.
Two rules serde used to enforce are now the normalizer's: a missing header
and a missing option description. Both are still rejected, with a clearer
message than serde's "missing field". `multiSelect` now defaults to false
instead of being a deserialization error; the schema still marks it
required, which is where that contract belongs.
Adds tests pinning the strict rules against the shared normalizer, and one
asserting the lenient contract still accepts optional headers and
descriptions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All six provider profiles embed a `BaseProfile` and hand-wrote the same
six delegating accessors -- 24 identical lines each. What actually
distinguishes them is `build_system_prompt`, and for Claude 5,
`register_subagent_tools`.
Replace the copies with one `impl_base_profile_accessors!()` invocation.
A macro rather than trait defaults because three implementors have no
`BaseProfile` to delegate to -- `TestProfile`, the workflow crate's
`ShutdownTestProfile`, and the server's `AskFabroProfile` -- so a default
would need a runtime fallback for a case the compiler can already rule
out. Those three keep their hand-written accessors and are untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`NativeTool` documents itself as "an identity, not a name" whose canonical
form is fabro's own vocabulary, with harness names layered on as aliases:
`to_string = "read_file", serialize = "Read"`.
The four Claude 5 subagent tools inverted that. `ClaudeAgent` declared
`to_string = "Agent"`, making the Anthropic wire name the identity and
leaving `name(ToolVocabulary::Fabro)` returning `"Agent"` -- and pairing a
provider-specific variant name with a generic wire name. It also meant the
`Claude5` arm listed none of them: they fell through to
`canonical_name()` and were correct only by accident.
Rename to `BackgroundAgent` / `AgentOutput` / `StopAgent` / `MessageAgent`
with fabro canonical names, keep the harness names as `serialize` aliases
so `from_any_name` still resolves them, and name them explicitly in the
`Claude5` vocabulary arm. Also map `Grep`/`Glob` there: that arm describes
the vocabulary rather than the profile's registry, and if either were ever
registered it would otherwise reach the harness lowercased.
Records why these are separate identities from
`spawn_agent`/`wait`/`close_agent`/`send_input` rather than aliases of
them, since the capabilities genuinely differ.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task tools scope their list by `root_session_id` -- `Session` documents
this as "a subagent session inherits its parent's `root_session_id` so
todo tools that scope by root (Anthropic tasks) share one list across all
subagents" -- so a root and its children address one logical list.
`build()` runs once per session, though, and `AnthropicProfile`
constructed its own `TodoRuntime` inside that call. Root and child
therefore resolved the same `list_id` through different runtimes: both ID
counters started at zero, so both emitted `todo.created` with id `1` for
the same list, and `TodoListProjection::upsert` matches on id -- the
child's task replaced the parent's in the persisted projection. `TaskGet`
and `TaskList` read the local runtime, so neither session could see the
other's tasks either.
The previous commit's shared runtime fixed this for Claude 5 only,
because `build()` passed dependencies positionally and adding a fourth
argument would have meant touching all six call sites. It grew a second
constructor for Claude 5 instead, leaving the other five on a signature
that could not carry the runtime.
Bundle them into `ProfileDeps` so every profile takes the same
`(model, &deps)`. The duplicate constructor is gone, Anthropic shares the
runtime by construction rather than by opting in, and a future dependency
reaches all six profiles or none.
The existing Claude 5 sharing test is generalized and now also runs for
Anthropic; it fails against a per-profile runtime.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`git add -A` over lib/ and docs/ swept in work that was already untracked in
the working tree before this branch started: lib/crates/, and three docs
files. None of it belongs to this change.
Removed from the index only, so the files stay on disk as the untracked work
they were. They net out of the branch diff entirely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follows the type rename: a rotation chain is now an auth session with its own
row, so the local names and the Repository doc comment should say so rather
than referring to a store that no longer exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records the two new tables in the server configuration reference, and adds a
changelog entry leading with the operator-visible consequence: this upgrade
signs everyone out once, because existing refresh tokens are not migrated.
Also notes the error-code change on concurrent replay, since it is observable
even though the CLI handles both codes identically.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes `slate/auth_tokens.rs`, `Database::refresh_tokens()`, and its
`OnceCell` now that nothing reads them, and clears the retired `auth/refresh`
prefix once at startup. That sweep is not housekeeping we could skip: the
reaper that used to collect those records went with the store, so without it
they would sit in the object store forever. A later boot finds the prefix
empty and does nothing.
`record/transaction.rs` goes too -- rotation was its only caller, and SQLite
transactions replaced it. `KeyedMutex` stays; `AuthCodeStore` still uses it
until auth codes move.
Existing refresh tokens are not migrated. Everyone re-authenticates once on
upgrade.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Points the session listing, revocation, refresh, and logout paths at
`AuthSessionStore`. Listing a user's sessions and revoking one stop scanning
the whole refresh-token keyspace; both are now indexed queries.
Fixes two timestamps that were wrong by construction. `created_at` was fed
from the newest token's `issued_at`, so a session's reported start drifted
forward on every refresh, and `last_seen_at` read a field only ever set at
issue -- so both rendered the same value. They now come from the session row,
where they mean what they say.
Deletes `next_refresh_row`, which had to fabricate an identity of
("https://github.com", "0") and empty profile strings for the no-existing-row
case, because a token was required to carry chain-level fields. Rotation now
takes just the new hash, expiry, and user agent. That also removes the
pre-read it existed to feed, closing the window between that read and the
one `consume_and_rotate` did itself.
Opening the store per request is gone with it: five handlers each had a
500-response arm for "could not open the store", which field access on
AppStores cannot fail.
Drops the replay-revocation cache. Its only effect was reporting `revoked`
rather than `expired` for the third and later presentations in a concurrent
burst, and `fabro-client` (client.rs:508-513) matches both codes in one arm
and treats them identically. Replay detection itself is unaffected: it is
`Reused` into `delete_session`, which lives in the database. The concurrency
test now accepts either code, since losers that arrive after the winner's
revocation find the row already cascaded away.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ParentNotificationHub` kept a second `Mutex` and `watch` channel holding
a copy of each child's terminal result -- data `SubAgent.status` already
owns as `SubAgentStatus::Finished`, and which is never evicted, since
nothing removes entries from `SupervisorState.agents`.
Two of the three bugs fixed in the previous commit were ordering bugs in
the coupling between those two structures: suppress-vs-commit in
`begin_shutdown`, and register-vs-publish in `spawn_inner`. Both were
fixed by ordering the steps correctly. Keeping the registration beside
the status it is delivered with makes that whole class unrepresentable
instead:
- Registration is now a field on the `SubAgent` literal `spawn_inner`
already builds, under the lock that publishes it. There is no window
between publishing an agent and registering its notification.
- Suppression on shutdown happens inside the critical section that
decides the shutdown, after the status transition commits, so a
rejected shutdown cannot discard a result the parent is owed.
- `next_parent_notification_batch` scans agents for a live registration
whose status is `Finished`, and ignores `Closing`/`Closed` outright --
so a shutdown racing delivery can no longer park the parent on a result
that will never arrive, even if suppression were missed.
`spawn_result_monitor` no longer takes the hub; it bumps a single
`watch` counter after committing the status it already commits. Batch
order was the queue's insertion order, so `SubAgent` carries a
`spawn_seq` to keep delivery oldest-first.
Tests move from exercising the hub directly to the supervisor API, and
cover spawn-order batching and the shutdown-races-delivery case that the
old shape could not express.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every operation the SlateDB store answers with a full keyspace scan becomes
an indexed query here: listing a user's sessions joins one row per session
via the partial unique index instead of scanning every token ever issued
and grouping by chain, and revoking one is a single DELETE that cascades.
Rotation is the structural win. Claiming the presented token is one
`UPDATE ... WHERE used_at_ms IS NULL ... RETURNING`, and it is the
transaction's first statement, so SQLite takes the write lock before
anything is read. A concurrent caller blocks on that lock and then sees the
token already spent, which is exactly the replay signal -- so the store
needs no `KeyedMutex` to serialise rotation, and the guarantee survives more
than one server process.
Expiry is checked ahead of reuse on the cold path, preserving the ordering
callers depend on: only replaying a still-live token revokes its chain.
Drops the ordering CHECKs between a session's timestamps and its tokens'.
Rotation stamps `now` from the process clock against rows written by an
earlier request, so an NTP step backwards would have turned a harmless clock
anomaly into refresh failing outright for every affected session.
The store is not wired into the server yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A CLI auth session is a rotation chain, but the SlateDB records that back
it today store identity and profile per token, so a chain has no owner and
nothing stops its rows from disagreeing. These two tables give the chain a
home: `auth_sessions` holds the identity and profile once, `refresh_tokens`
holds only per-token facts.
Two invariants the current code relies on but never states become
constraints. The partial unique index on `(session_id) WHERE used_at_ms IS
NULL` enforces that rotation leaves exactly one live token per chain --
which is what makes the session listing an indexed lookup instead of a
scan-and-group. The foreign key with `ON DELETE CASCADE` makes revoking a
session remove its tokens without a second statement.
Tokens are retained after rotation until they expire so a replayed token
stays distinguishable from a forgery.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
The check tested for `never reconstruct it from memory`, a phrase the
Kimi edit description no longer contains after it was reworded to match
Kimi Code. A one-sided `!contains` against a literal cannot tell "the
phrase is absent because nothing leaked" from "the phrase is absent
everywhere", so it silently stopped protecting anything.
Assert the marker is present in Kimi's own description and absent from
the stock one. Removing the marker from the description now fails the
test instead of quietly disarming it, verified by doing exactly that.
Also correct the grep docs: all three sandbox implementations probe for
`rg` and fall back to POSIX `grep`, so the page should not imply a
single engine. Pre-existing, adjacent to the lines this branch touched.
Reported by Copilot review on #646.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither harness puts read-before-edit mechanics in the system prompt.
Kimi Code's `system.md` has no such section; the rules live in
`edit.md`, `write.md`, and `read.md`. Codex's prompts say nothing about
reading before an edit at all, and its editing guidance is attached to
`apply_patch`.
Drop the `# Reading Before Writing` section from the Kimi prompt and
carry its content in the Edit, Write, and Read descriptions, worded as
Kimi Code words it. Nothing is lost: every bullet in the removed
section was already covered by a tool description.
Two behaviors change to match upstream. Edit now says not to issue
consecutive edits against the same file, since the first invalidates
the second's `old_string` -- Kimi Code's stated reason. Read now says
not to re-read solely to confirm a write landed, which both harnesses
call out as waste; the previous prompt asked for exactly that re-read.
The gpt56 profile already followed the Codex split and is unchanged.
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>
`ReadBeforeWriteSandbox` blocked writes to any existing file the agent
had not read, tracked by a session read set populated only by
`read_file`, `grep`, `read_many_files`, and the Kimi `Read`.
The gpt56 profile has none of those. It mirrors Codex's tool contract --
`shell_command`, `apply_patch`/`edit_file`, `update_plan`, `web_search`
-- and reads through the shell, so its read set stayed permanently
empty and every edit to an existing file failed. In run
01KYD4360GN6SED4BYEVGYP4XT all 28 `edit_file` calls failed, 25 of them
on the guard. The agent read `package.json` with `sed` and `cat`,
hex-dumped it trying to diagnose the rejections, then routed around the
guard with `sed -i`, which the guard never covered. It prevented no
blind write; it converted content-anchored edits into an unreviewed
in-place shell rewrite.
Neither Codex nor Kimi Code enforces read-before-write at runtime.
Codex's `apply_patch` `Add File` overwrites an existing path silently;
Kimi Code's `Write` has no check at all. Both rely on the exact-match
requirement in their edit tools, which is stronger proof of inspection
than a read set, plus per-write approval.
Tool descriptions and the Kimi prompt keep telling the model to read
before editing -- that guidance matches Kimi Code's own `edit.md` and
still prevents `old_string not found` -- but no longer claim the
workspace refuses unread writes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
apps/fabro-web is bundled by a custom Bun script (scripts/build.ts), not
Vite. The stale reference sends agents toward Vite-specific APIs — most
notably `vite:preloadError`, which does not exist in this codebase — when
reasoning about the SPA build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Automation::to_toml_string` and the `to_persisted` helper it wrapped
have had no production callers since automations moved from
`<storage>/automations/*.toml` into SQLite. Writes now serialize through
`canonical_bytes` for revision hashing; nothing renders an `Automation`
back to a TOML document.
Repoint the canonicalization test at `parse_persisted` + `canonical_bytes`
so it exercises the production path that actually produces the bytes the
revision hash is computed over.
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>
The stage-summary preamble rendered per-stage token usage for every
completed LLM stage: "Model: kimi-k3, 92.6k tokens in / 41.1k out" at
compact fidelity and "Tokens: N in / N out" at summary:high. Agents read
that as their own remaining budget.
In run 01KYCM3EG4KMCVRDYNV93PZWBV an implementation stage stopped after 2
of 9 units, reasoning "We have around 100k tokens, but time constraints
are an issue" and recording the rest as halted "within the available
execution window". The 92.6k it saw was the preceding plan stage's
billing telemetry, the only token quantity anywhere in its context. It
had used 11% of a 1,050,000-token window and 0.8% of a 24h stage timeout,
and no harness limit was near.
These counts have no task value to the agent: they describe a different
model's usage on an earlier stage, they are stale by one stage, and
nothing in the preamble distinguishes them from a budget. Keep the model
id and files touched, which carry provenance the agent can act on.
Both tests that asserted the counts now assert their absence, so the
regression is caught rather than re-snapshotted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
