capture all tool calls/results via SDK hooks (closes #3) - #4
Draft
jasoncarreira wants to merge 1 commit into
Draft
capture all tool calls/results via SDK hooks (closes #3)#4jasoncarreira wants to merge 1 commit into
jasoncarreira wants to merge 1 commit into
Conversation
Three gaps in the current message-loop based capture in _aquery /
_astream affect downstream consumers that want a complete audit trail
of model tool use:
1. **Built-in tool results are dropped.** Bash/Read/Edit/Write/Glob/
ToolSearch execute entirely inside the claude CLI subprocess. Per
the Anthropic API conversation convention, their ToolResultBlocks
arrive in UserMessage content — but the receive loops in _aquery
and _astream only branch on AssistantMessage and ResultMessage, so
UserMessage is dropped on the floor. Downstream this manifests as
N tool_call events with 0 tool_result events in the recorded turn.
2. **Bridged LangChain tools can't be paired by tool_use_id.** The
_tool_results_var ContextVar capture in _wrap_langchain_tool
records {"name": tool.name, "args": args, "result": result} with
no tool_use_id. Meanwhile the tool_call carries the MCP-bridged
name (mcp__langchain-tools__<name>) and an id. Downstream can't
reliably pair the call with its result.
3. **Events arrive bunched, not interleaved.** _parse_assistant_message
splits content blocks into parallel tool_calls / tool_results lists.
The block-order — the actual call→result→call→result execution
sequence the model produced — is lost.
The SDK has explicit PreToolUse / PostToolUse / PostToolUseFailure
hooks (claude_agent_sdk/types.py:265-292) that fire from the CLI
subprocess via control_protocol (_internal/query.py:389) for EVERY
tool invocation regardless of origin (built-in / bridged / MCP). Each
hook carries tool_name, tool_input / tool_response / error, and
tool_use_id. Registering them solves all three problems in a single
unified path.
This patch adds _install_tool_event_hooks(options): a helper that
registers three closure-based callbacks on options.hooks, returning
the events list they append to. _aquery / _astream call it after
_build_options, then attach the captured list to
generation_info["tool_events"] before returning / yielding the result
chunk. The existing internal_tool_calls / internal_tool_results /
tool_results keys are unchanged — tool_events is purely additive.
User-supplied hooks (e.g. permission gates) are preserved: our
callbacks are appended to any existing options.hooks[event] list, not
replaced. Our callbacks always return {} so they don't influence
control flow.
Verified empirically against a real ClaudeSDKClient + Bash invocation
in our downstream:
pre Bash toolu_01MTaFc6wy73KR7VCdZQZkdu input={'command': 'echo ...'}
post Bash toolu_01MTaFc6wy73KR7VCdZQZkdu response=dict
paired=True
Tested via 4 new tests:
- _install_tool_event_hooks registers all three event types
- User-supplied hooks are preserved
- Callbacks record correctly-shaped events with monotonic timestamps
- End-to-end through ainvoke: tool_events lands on response_metadata
This was referenced May 22, 2026
jasoncarreira
added a commit
to jasoncarreira/mimir
that referenced
this pull request
May 22, 2026
Pins the dep to the ``mimir-bundled-fixes`` integration branch on jasoncarreira/langchain-claude-code, which combines three fixes currently open as PRs upstream: * thehumanworks/langchain-claude-code#2 — preserve full ``ResultMessage`` field set on ``response_metadata`` in both streaming and non-streaming paths * thehumanworks/langchain-claude-code#4 — capture all tool calls/results via SDK hooks instead of inferring from the message stream (closes upstream issue #3) * thehumanworks/langchain-claude-code#6 — skip tools with langgraph-injected args in the bind_tools MCP bridge, fixing ``TypeError: missing 1 required positional argument: 'runtime'`` on every ``mcp__langchain-tools__read_file`` / ``write_file`` / ``edit_file`` invocation when running on deepagents 0.6+ The third one is the load-bearing fix for the production-observed duplicate-tools failure on mimirbot (turn 078eb0f98a1b on 2026-05-22): the framework's middleware-injected filesystem tools were also being bridged through MCP, and every bridged invocation failed with the missing-runtime error. The agent retried a few times before falling back to the native framework tool (``Read``), wasting prompt tokens and turn cycles in the process. Adds ``[tool.hatch.metadata] allow-direct-references = true`` so hatchling permits the ``pkg @ git+https://...`` form in ``optional-dependencies``. Reverts to the registry version (and drops the flag) once upstream PRs land + a new PyPI release ships. Three places in pyproject.toml carry the pin (claude-code extra, dev extra, deepagents extra) — all updated to the same SHA. Co-authored-by: Jason Carreira <jason@visotrust.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jasoncarreira
added a commit
to jasoncarreira/mimir
that referenced
this pull request
May 24, 2026
Pins the dep to the ``mimir-bundled-fixes`` integration branch on jasoncarreira/langchain-claude-code, which combines three fixes currently open as PRs upstream: * thehumanworks/langchain-claude-code#2 — preserve full ``ResultMessage`` field set on ``response_metadata`` in both streaming and non-streaming paths * thehumanworks/langchain-claude-code#4 — capture all tool calls/results via SDK hooks instead of inferring from the message stream (closes upstream issue #3) * thehumanworks/langchain-claude-code#6 — skip tools with langgraph-injected args in the bind_tools MCP bridge, fixing ``TypeError: missing 1 required positional argument: 'runtime'`` on every ``mcp__langchain-tools__read_file`` / ``write_file`` / ``edit_file`` invocation when running on deepagents 0.6+ The third one is the load-bearing fix for the production-observed duplicate-tools failure on mimirbot (turn 078eb0f98a1b on 2026-05-22): the framework's middleware-injected filesystem tools were also being bridged through MCP, and every bridged invocation failed with the missing-runtime error. The agent retried a few times before falling back to the native framework tool (``Read``), wasting prompt tokens and turn cycles in the process. Adds ``[tool.hatch.metadata] allow-direct-references = true`` so hatchling permits the ``pkg @ git+https://...`` form in ``optional-dependencies``. Reverts to the registry version (and drops the flag) once upstream PRs land + a new PyPI release ships. Three places in pyproject.toml carry the pin (claude-code extra, dev extra, deepagents extra) — all updated to the same SHA. Co-authored-by: Jason Carreira <jason@visotrust.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jasoncarreira
added a commit
to jasoncarreira/mimir
that referenced
this pull request
May 25, 2026
Pins the dep to the ``mimir-bundled-fixes`` integration branch on jasoncarreira/langchain-claude-code, which combines three fixes currently open as PRs upstream: * thehumanworks/langchain-claude-code#2 — preserve full ``ResultMessage`` field set on ``response_metadata`` in both streaming and non-streaming paths * thehumanworks/langchain-claude-code#4 — capture all tool calls/results via SDK hooks instead of inferring from the message stream (closes upstream issue #3) * thehumanworks/langchain-claude-code#6 — skip tools with langgraph-injected args in the bind_tools MCP bridge, fixing ``TypeError: missing 1 required positional argument: 'runtime'`` on every ``mcp__langchain-tools__read_file`` / ``write_file`` / ``edit_file`` invocation when running on deepagents 0.6+ The third one is the load-bearing fix for the production-observed duplicate-tools failure on mimirbot (turn 078eb0f98a1b on 2026-05-22): the framework's middleware-injected filesystem tools were also being bridged through MCP, and every bridged invocation failed with the missing-runtime error. The agent retried a few times before falling back to the native framework tool (``Read``), wasting prompt tokens and turn cycles in the process. Adds ``[tool.hatch.metadata] allow-direct-references = true`` so hatchling permits the ``pkg @ git+https://...`` form in ``optional-dependencies``. Reverts to the registry version (and drops the flag) once upstream PRs land + a new PyPI release ships. Three places in pyproject.toml carry the pin (claude-code extra, dev extra, deepagents extra) — all updated to the same SHA. Co-authored-by: Jason Carreira <jason@visotrust.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #3 — adds
_install_tool_event_hooks(options)which registersPreToolUse/PostToolUse/PostToolUseFailurecallbacks that capture every tool invocation into a single ordered, id-paired list._aqueryand_astreamattach the list togeneration_info[\"tool_events\"]. Built-in tools (Bash/Read/Edit/Write/Glob/ToolSearch), bridged LangChain tools, and MCP tools all surface here — the existing path loses results for built-in tools entirely (theirToolResultBlocks arrive inUserMessagecontent, which the message loops don't iterate), and bridged-tool capture has notool_use_idto pair on.Draft because:
tool_eventskey vs. fixing the existing keys vs. handlingUserMessageparsing) is a maintainer call.What's in this PR
src/langchain_claude_code/claude_chat_model.py(+101): new_install_tool_event_hookshelper, plus 3-line additions to_aqueryand_astreamto call it and attach the result.tests/test_claude_chat_model.py(+154): 4 new tests:ainvokewith stubbed SDK:tool_eventslands onresponse_metadataExisting 11 tests continue to pass; new tests use the existing
StubClaudeSDKClientpattern.Design notes
internal_tool_calls/internal_tool_results/tool_resultsare unchanged. Consumers that don't care abouttool_eventssee no behavioral change. Consumers that want unified, ordered, paired tool data can prefer it._aquery/_astreamcall. Hook callbacks close over it, so concurrent invocations stay isolated without any ContextVar plumbing.options.hooksthe caller supplied (e.g. permission gates) are kept; our callbacks are appended to the matcher lists. Our callbacks always return{}so they never influence control flow even when chained.tool_call/tool_resultsuccess /tool_resultfailure), withtool_use_idalways present andts_mono_nsfor ordering.Test plan
python -m pytest tests/→ 15 passed (11 existing + 4 new)ClaudeSDKClient+Bashinvocation in our downstream: paired pre/post events with matchingtool_use_id, including for built-in tools that the existing path misses entirely🤖 Generated with Claude Code