Skip to content

fix(extension): holistic review fixes for #241 - #248

Merged
yuanhao merged 4 commits into
mainfrom
fix/extension-review
Oct 6, 2026
Merged

yuanhao merged 4 commits into
mainfrom
fix/extension-review

Conversation

@yuanhao

@yuanhao yuanhao commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Fixes everything the holistic five-reviewer pass over the merged #241 work found (#242–#247). The extension API is unreleased, so the reshaping happens now, before 0.25.

API (commit 1)

  • impl Extension for Arc<T>: share an extension, read Budget::spent_usd() after installing, pass tree extensions on.
  • AgentLoopConfig::delegated_from(&ToolContext) / Agent::delegated_from: a hand-written delegation tool's child now gets the tree properly: depth, label, inherited extensions whose tools are not offered. Before, tree policy failed open for these tools. SubAgentTool uses it too.
  • on_event moves to RunHooks (sync, &self), so per-run state stays in the hooks.
  • RunOutcome: private fields, end() -> RunEnd { Completed, Stopped, Rejected, Cancelled, Failed { error, extension } }, stop_reason().
  • Stateless becomes ClonedHooks, with .filters_tool_output() and .rechecks_modified_calls().
  • EXTENSION_MESSAGE_PREFIX = "[Extension message: " ("[Extension " also matched real user text).
  • Budget:
    • rejects a negative or NaN limit, and gains with_name;
    • a per-run budget counts a sub-agent's reported spend;
    • no leaked per-run entries.

Safety

  • A pending required failure stops tool calls not started yet: before a response's tools, and between sequential calls and batches.
  • Every hook races the run's cancel, and finish is bounded (FINISH_TIMEOUT).
  • A run cancelled before its first turn closes it.
  • on_input sees the input without the filters' warnings.
  • name() / mode() panics are contained.
  • Logs carry run_id / tool_call_id; a dead event observer is an error!; an advisory on_event that panics is switched off for the run.
  • SubAgentTool fails when its event forwarder fails, instead of reporting an empty success.

Tests and docs (commit 2)

Tests:

  • extension_test grows from 37 to 58 cases and extension_budget_test from 5 to 11, plus one decision test and one wasm32 smoke test (run under Node, 9/9).
  • The key new behaviours are mutation-checked: the per-call failure check, the first-turn close, the cancel race.
  • Multi-thread variants were added for the observer.

Docs:

  • extensions.md rewritten for the final API. Examples now compile, the guard's position is right, the on_stop/before_model rules are right, turn-hook note order is stated, plus delegation, cancellation, wasm32 panics and Length.
  • rustdoc for the gate, guard, advisor, is_loop_injected and ToolContext; struct listings; CLAUDE.md.
  • CHANGELOG: Loop: tools run after cancel, limit-stopped turns stay open, messages appended without events #243's added events and skipped tools are flagged as behaviour changes.

Deliberately not done: deprecating ToolGate's ToolMiddleware impl and InputGuard's filter impl. That's a separate decision for 0.25.

Checks

cargo test --all-features (58 binaries; hook_order unchanged), clippy with --all-features and --no-default-features, wasm32 clippy (lib with decision, and the test target), the wasm32 tests under Node, cargo doc -Dwarnings, fmt. The extension tests passed 10 repeated runs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG

yuanhao and others added 4 commits October 7, 2026 00:52
API (unreleased, reshaped before 0.25):
- impl Extension for Arc<T>: share an extension, read Budget::spent_usd
  after installing, pass ToolContext::tree_extensions() on.
- AgentLoopConfig::delegated_from(&ToolContext) / Agent::delegated_from:
  a custom delegation tool's child gets the tree (depth, label, inherited
  extensions whose tools are not offered); SubAgentTool uses it.
- on_event moves to RunHooks (sync, &self), so per-run state stays in the
  hooks; an advisory on_event that panics is not called again.
- RunOutcome: private fields, end() -> RunEnd { Completed, Stopped,
  Rejected, Cancelled, Failed { error, extension } }, stop_reason().
- Stateless -> ClonedHooks, with filters_tool_output() and
  rechecks_modified_calls() builders.
- EXTENSION_MESSAGE_PREFIX = "[Extension message: ".
- Budget: rejects a negative or NaN limit, with_name, counts a sub-agent's
  reported spend unless it runs in the child too; per-run spend lives in
  the run's hooks (no leak on a dropped run).

Safety:
- A pending required failure stops tool calls not started yet (before
  tools, between sequential calls and batches).
- Every hook races the run's cancel; finish is bounded (FINISH_TIMEOUT).
- A run cancelled before its first turn closes that turn.
- InputContext.text excludes the filters' warnings.
- name()/mode() panics are contained; logs carry run_id and tool_call_id;
  a dead observer is an error; SubAgentTool fails when its event forwarder
  fails instead of reporting an empty success.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
Tests (extension_test 58, budget 11, decision +1, wasm32 +1):
- a hand-written delegation tool runs its child under the tree (an Arc'd
  tree policy, depth 1, tools not offered);
- a required failure on a response stops its tool calls, and after_tool
  failing stops the next sequential call;
- a run cancelled before its first turn closes it; a hung hook does not
  hang a cancelled run (each mutation-checked);
- loop-injected prefix, an advisory on_event switched off after a panic,
  on_input without filter warnings, ClonedHooks filtering output;
- a tree redactor across a sub-agent, the output store holding only the
  filtered result, required panics in on_input/after_tool/finish,
  recheck scope, older hooks first, on_stop with follow-ups and limits,
  finish seeing every event, after_tool skipped for denied/unknown calls,
  a provider error in finish, on_event before the consumer,
  multi-thread variants;
- Budget: read through an Arc, a per-run budget counting a sub-agent's
  spend, the exact limit, a named budget, negative/NaN refused;
- a tree gate rechecking a rewritten call;
- wasm32: an extension gating and observing a run on the host.

Docs: extensions.md rewritten for the final API (examples compile:
ModelConfig import, no shadowed Budget), the input guard's position, the
on_stop/before_model rules, turn-hook note order, delegation via
delegated_from, cancellation, wasm32 panics, Length; rustdoc for the gate,
guard, advisor, is_loop_injected, ToolContext; struct listings;
TurnEnd row; CLAUDE.md; CHANGELOG (behaviour changes from #243 flagged).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
- A cancel during the input stage is not a rejection: a cancelled on_input
  lets the run start, and it ends Cancelled at its first check, before any
  model request (was InputRejected).
- Budget decides per delegation which child spend it already counted: a
  child run records parent_run_id/call_id (RunContext::{parent_run_id,
  delegated_by}, new), instead of one sticky flag on the shared Budget
  that stopped another agent counting its sub-agents' spend.
- run_outcome: a run whose model finished stays Completed even if the token
  is cancelled afterwards.
- Docs: on_event can wait behind a running &mut hook; cancel semantics.
- Tests: cancel before the input check ends Cancelled (mutation-checked);
  on_event strictly before the consumer (multi-thread); a shared budget
  across a tree agent and a plain agent counts each child once.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
- A SubAgentTool child cancelled before any assistant message fails the
  delegation with ToolError::Cancelled instead of reporting an empty
  success (the pinned test_sub_agent_cancellation expected the old
  behaviour and is updated; docs/sub-agents.md and CHANGELOG say so).
- RunContext::inherited (in start_run): the extension came from the
  calling run's tree. Budget records a child delegation only then, so no
  key is left behind when the parent does not hold the budget.
- The on_event ordering test sleeps before checking the consumer, so a
  forward-first observer fails it (mutation-checked).
- Stale doc comment on ActiveExtensions::on_input.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
@yuanhao
yuanhao merged commit e63cc71 into main Oct 6, 2026
14 checks passed
@yuanhao
yuanhao deleted the fix/extension-review branch October 6, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant