Repository navigation
fix(core,api): harden cancellation and tool telemetry - #122
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c739a91801
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raise MaxStepsExceeded(self.config.max_steps) | ||
| return res_max | ||
|
|
||
| except asyncio.CancelledError: |
There was a problem hiding this comment.
If cancellation lands while get_messages() is awaiting, it happens after create_run() has persisted the row but before the try guarded by this handler starts, so the cancellation escapes with the run still running and the session history unrepaired. Extend cancellation finalization to cover this setup path and add the core-loop regression test required for this change.
AGENTS.md reference: AGENTS.md:L84-L84
Useful? React with 👍 / 👎.
Cancellation delivered while get_messages() was still awaiting escaped finalization entirely: the run row already existed in 'running', but the only CancelledError handler sat inside the reasoning-loop try block, so no terminal write (and no history repair happened at all) before re-raising. Route both cancellation windows through one shielded _finalize_canceled() helper. During setup the history save is skipped: the local snapshot is still empty and writing it would destroy the stored conversation rather than repair it. Adds the core-loop regression tests pinned to the post-create_run window and records the broadened rule in ADR-021.
test_tools_endpoint_exposes_builtin_registry asserted web_search unconditionally, but the real-infra CI job never exports SEARCH_API_KEY, so the conditional registration change failed that suite at 144bc97. The new parametrized tools_client fixture builds one application per case with SEARCH_API_KEY passed explicitly: web_search is expected present with a key configured and absent without one, independent of the ambient environment, so a developer with the variable set cannot mask the key-missing path (or vice versa).
Summary
canceledrun status and repaired session history under shielded finalization when a run is canceled, then propagate cancellation.web_searchonly whenSEARCH_API_KEYis configured and document the behavior.Closes #93, #98, and #99.
Verification
ruff check .ruff format --check .mypy agentkit