Skip to content

Fix tool callback propagation in Response.reply() - #1836

Open
0d00ciallo0721 wants to merge 1 commit into
simonw:mainfrom
0d00ciallo0721:agent-tasks/1835
Open

0d00ciallo0721 wants to merge 1 commit into
simonw:mainfrom
0d00ciallo0721:agent-tasks/1835

Conversation

@0d00ciallo0721

@0d00ciallo0721 0d00ciallo0721 commented Oct 29, 2026 •

Copy link
Copy Markdown

Problem

Response.reply() automatically executes pending tool calls, but the sync and async paths omit the response's configured before_call and after_call hooks. The returned response also drops them, so later reply rounds do not apply them.

Changes

  • Forward the originating response's hooks to automatic tool execution in sync and async replies.
  • Preserve both hooks on the returned response for subsequent reply rounds.
  • Add sync and async regressions with a rejected tool, a permitted tool, and a second reply round.

Fixes #1835.

Local validation

  • python -m pytest -q: 1,309 passed.
  • Black, Ruff, mypy, Cog, package build, and plugin-loading smoke test passed.
  • The regression fails on the base commit because the rejected tool executes without calling before_call; it passes with this change.

Remote checks

  • Read the Docs: success.
  • GitHub Actions Test and Run Cog: action_required, both with zero jobs; no remote test steps have run yet.

AI assistance

OpenAI Codex assisted with investigation, implementation, and testing. No live model API calls were needed for the reproduction.

@0d00ciallo0721
0d00ciallo0721 marked this pull request as ready for review October 29, 2026 15:55

@Muguett-DBY Muguett-DBY left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I compared this head with the base using offline recording models. In both sync and async replies, the base executes a tool rejected by before_call; this branch cancels it, runs the allowed tool, and retains both callbacks for the next reply.

I also checked a few other paths: a failing before hook stops execution, a failing after hook propagates after the tool runs, and an unknown tool reaches before_call with tool=None and can be cancelled. A sync reply with explicitly supplied tool_results skips auto-execution and keeps the callbacks for the next response.

For the async cases I passed tools explicitly to prompt, keeping the separate conversation-tool inheritance issue in #1834 out of this check. These were the exact model and message sources loaded in memory with a stubbed HTTP client and unused utility helpers; I didn't run a provider integration or the full suite. Codex assisted with the checks.

This branch has not been deployed

No deployments
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.

Response.reply() skips conversation tool callbacks when auto-executing tools

2 participants