Review pass over the reuse change. No intended behavior changes.
- share one definition of the initial generation from fabro-types instead
of three copies across fabro-types, fabro-agent, and the supervisor
- give each child one SubAgentHandle instead of threading the supervisor's
state, callback, and notification sender through five functions, and
collapse the repeated signal-then-drain pairs into publish()
- move `reusable` inside SubAgentStatus::Finished so a closed agent can no
longer be marked reusable
- clear the lifecycle draining flag with an RAII guard, so one panicking
callback cannot silence every later lifecycle event
- tear down a session that failed to initialize right away rather than
holding it and its sandbox until the parent closes the agent
- look agents up through SupervisorState::agent/agent_mut instead of five
copies of the same not-found error
- drop the unreachable cleanup_started branch and the test-only emit_event
whose only caller was its own test
- render subagent starts from one ProgressEvent and one display method,
deriving the spawn/turn distinction from the generation
- set projected subagent status through one helper instead of four
identical reducer arms
- drive the generation-pinned wait test through spawn/send_input rather
than hand-writing private supervisor state
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second pass, from the remaining review findings.
- Wrap the stall watchdog in a `StallWatchdog` type. The call site kept
two parallel `Option`s derived from the same condition and threaded out
an `Option<(CancellationToken, JoinHandle<()>)>`. `monitor_for_stall`
also took two same-typed `CancellationToken` params pointing opposite
directions, where swapping them compiles and yields a run that silently
never stalls.
- Rename `WorkflowAgentQuestionRuntime::stage_id` and
`PendingAgentQuestionBatch::stage_id` to `node_id`. They hold
`node.id`, and the previous commit put them two lines from
`stage_scope.stage_id()`, which returns a real `StageId`.
- Widen the two real-time interview tests. `node_timeout_excludes_
human_input_wait` allowed 20ms of active work against a 50ms budget,
which is tight enough to flake under parallel nextest load. The blocked
wait still outruns the timeout, so both still fail if the pause
regresses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One conflict, in StructuredOutputError::repair_message. main (#709)
added a `previous_error` parameter and richer validation-error
rendering; this branch had replaced the inline expectation match with
OutputSchemaKind::expectation().
Resolved by keeping both: main's new signature and section assembly,
calling schema.expectation() for the expectation text. The method
already supersedes main's inline match and carries this branch's intent
of embedding the resolved JSON Schema instead of naming it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up cleanup on the human-input timeout work.
- Drop the `unresolved_interviews` counter from `InterviewBlockState`. It
duplicated `blocked_stages`, which is non-empty exactly when the run is
blocked.
- Publish block state before emitting `run.blocked` / `run.unblocked` in
both directions, so a listener reading `subscribe()` from an event
callback never sees state that disagrees with the event. The watchdog
still gets a full fresh deadline because it restarts on the unblock
transition.
- Stop panicking in `InterviewBlockState::resolve`. It runs from `Drop`,
where a panic during unwind aborts the process.
- Replace the emitter's `activity_revision` watch channel with a
monotonic timestamp. `record_activity` runs on every agent stream
delta, and the channel woke the watchdog task and re-armed its timer
per event. The watchdog now samples `last_activity()` when its deadline
fires and re-arms only if the run was active, so the hot path is one
clock read and one relaxed store.
- Remove the now-unused `last_event_at()` and `epoch_millis()`.
- Collapse the duplicated blocked/unblocked `select!` arms in
`monitor_for_stall` and `timeout_excluding_interview_wait` into one
loop each, using a branch precondition to park the timer while blocked.
- Handle a dropped block-state sender in
`timeout_excluding_interview_wait` by falling back to a plain deadline
instead of panicking, which also removes a potential busy loop.
- Only compute `stage_id` when the node actually has a timeout.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the two envelope-level tests with focused `ApiUsage` tests that
match the file's existing `token_counts_*` convention.
The streaming and non-streaming tests were the same test paid for twice:
`ApiResponse::usage` and `StreamChunk::usage` are both `Option<ApiUsage>`,
so the envelope cannot change the result. Envelope-level usage decoding is
already covered by `stream_chunk_usage_parsing`.
Also pin the precedence rule this change introduces — nested detail wins
over the flat spelling, and an empty `completion_tokens_details` still
falls back — and document it on `token_counts`. Revert the unrelated
`cost` doc edit that dropped the OpenRouter reference.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both contract helpers dispatched on OutputSchemaKind, so they belong on
the type. Moves expectation() and agent_prompt() into an impl block and
drops the free functions.
Splits the combined agent test: assertions no longer run inside the
backend's run(), where a failure surfaces as a panic from execute().
Adds coverage for the Routing branch of the contract, which was
previously untested.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolve the model catalog table conflict in docs/public/core-concepts/models.mdx
by keeping both changes: this branch's `kimi` -> `moonshot` provider rename for
the Kimi rows, and main's new DeepSeek V4 rows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses Copilot review feedback on the repeated-failure check.
serde_json runs with preserve_order, and jsonschema builds the
additionalProperties `unexpected` list by walking the instance in
document order. So the same leftover keys emitted in a different order
produced a different Vec and compared as a different problem, which
suppressed the "unchanged from your previous repair" nudge.
Sorting at capture also makes the MAX_UNEXPECTED_PROPERTIES truncation
pick the same subset every time instead of an order-dependent one, and
stabilizes the rendered message.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Path safety now lives in one place. The NUL-byte and drive-letter rules
move from a server-only helper into the store's own filename validation,
so uploads reject those paths at write time instead of only the ZIP read
path catching them. The download still re-checks, because artifacts
stored before the rule existed can still carry an unsafe path, but it
now skips a bad path rather than failing the whole archive.
Promote is_boundary_stage to RunProjection and drop the three identical
private copies. The ZIP download used a node-name match instead, which
would have dropped artifacts from a working node that happened to be
named "start".
Compress the archive. Entries were Stored while the response was also
excluded from transfer compression, so text artifacts moved at full
size. async_zip gains the deflate feature; async-compression and flate2
were already in the lock file.
Log archive failures unconditionally. The send-succeeded guard meant a
client that had already disconnected left no record at all, which is the
case where the log is the only evidence.
Also: collapse the duplicate 500 arms, drop the dead stage-ID tiebreaker
and the cached order in the selection map, name the accessible label
after the visible one, share the run URL prefix between the two download
href builders, and document the mid-stream truncation behavior in the
OpenAPI description.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three places translated a provider error code into a ProviderErrorKind:
error_from_status_code for HTTP error bodies, and a private table in each
of the openai_responses and anthropic_messages stream decoders. The tables
disagreed, so the same failure classified differently depending on which
path saw it.
Most visibly, OpenAI returns HTTP 429 with error.type "insufficient_quota"
when an account is out of credit. The streaming decoder mapped that to
QuotaExceeded, but the non-streaming path fell through to the plain
429 => RateLimit arm, so a spent quota was retried with backoff and never
triggered failover.
Move the code table into error.rs as kind_from_error_code, returning None
when the code says nothing so each caller keeps its own default. All three
call sites now share it.
In error_from_status_code, unambiguous statuses (401, 403, 404, 408, 413,
5xx) still win outright. A 429 defers to the code only when it reports a
spent quota. Ambiguous statuses (400, 422, ...) prefer the structured code
over the existing message-substring guessing, which now runs only when
there is no code.
Two classifications improve as a side effect of merging the tables:
not_found_error now maps to NotFound rather than Server for openai, and
request_too_large maps to ContextLength rather than InvalidRequest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up review of the repair-error work. Behavior is the same or better;
the machinery is smaller.
Fixes a false "unchanged from your previous repair" nudge. same_problem_as
fell through to `_ => true`, so any two non-Required issues at the same
instance path, schema path and keyword compared equal. A model that removed
one unexpected property and added another was told it had changed nothing.
SchemaValidationIssue already derives PartialEq, so the 17-line comparison
is now `previous.contains(issue)`.
Drops the hand-written Type and Enum rendering. jsonschema already renders
both, and its messages name the offending value, which the hand-written
ones did not. Also switches masked() back to to_string(): masking replaced
the bad value with a placeholder, working against the goal of an actionable
message, and buys no privacy since the full response is already in the
prompt.
Resolves the schema fragment when the issue is captured rather than
threading Option<&OutputSchemaKind> through rendering. That reverts the
command.rs change and drops the test-only messages() shim. The fragment is
now attached only to Other, where it adds information; for required, type,
enum and additionalProperties it just repeated the prose.
Also: caps the model-controlled unexpected-property list so a wide object
cannot turn the repair prompt into megabytes; drops evaluation_path, which
was dead except under $ref, where it printed a pointer that does not
resolve; drops the keyword field, already named by the schema path; and
records the previous error only after the agent session accepted the
repair, since failover rebuilds the session from the original prompt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The parallel stage summary rendered a Duration tile directly below
StageMetaBar, which already shows the same stage's duration with a live
ticking clock and a started-at tooltip. The two disagreed while running:
the meta bar counted up, the tile showed the static word "running". The
cancelled-stage bug lived only in the duplicate.
Drop the tile. The meta bar owns duration for every stage renderer, and
it was already correct for cancelled, pending and skipped stages. That
removes the three-way duration branch, the "--" sentinel decode, and the
ACTIVE_STAGE_STATES and formatDurationMs imports.
With the tile gone, ParallelOverview.durationMs is dead, as were
successCount, failureCount and isComplete — the renderer counts the
branch rows it draws. ParallelOverview reduces to branch identity.
For run event write failures, log the first at error with run_id and
event name, the rest at debug, and summarize new losses at flush. A
broken sink fails for every event, so a bare error would emit one
"investigate me" line per event for the life of the run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Return String from the sanitize helpers instead of Cow: every call
site feeds the result into json!, which allocates anyway, so the
borrowed fast path only cost extra branches and Cow-variant tests.
- Route all toolUse/toolResult construction through private
tool_use_block/tool_result_block constructors that own the sanitize
calls, so the toolUse/toolResult pairing invariant is enforced by
construction rather than by call-site discipline.
- Drop a test assertion the type system already guarantees (encoding
takes &Request, so it cannot mutate the input) and assert wiring
tests against the sanitize helpers instead of re-pinning the exact
replacement literals in a second file.
No wire-format changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A fork carries a checkpoint from its source run, but its first
run.created event contains only a sandbox plan. Resume previously tried
to reconnect that planned sandbox and failed because no instance
exists. Now a fork resume with a Planned sandbox record builds a fresh
sandbox instead; later fork resumes still reconnect the ready instance,
and a same-run resume with an uninitialized sandbox still fails the
precondition check.
Also consolidates the test module's three near-identical InitOptions
literals into a shared test_init_options helper.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
