fix: marking executed tool calls on webhooks#2479
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where tool calls executed in webhook contexts (where save_conversation=False and no message is reserved) would be left in the executed state forever — since the message-finalize path that flips executed → confirmed never runs without a message. The repository now writes such rows directly as confirmed, allowing the reconciler/journal to reflect the true terminal state.
Changes:
mark_executedandupsert_executednow writestatus='confirmed'instead of'executed'when nomessage_idis supplied.- Updated existing tests and docstrings to reflect the new semantics, and added new tests for the with-message and without-message paths (including the
upsert_executedfallback).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| application/storage/db/repositories/tool_call_attempts.py | Parameterizes status in both mark_executed and upsert_executed, choosing confirmed when message_id is None. |
| tests/agents/test_tool_executor_three_phase.py | Renames and rewrites tests to cover the new no-message → confirmed behavior; deduplicates tools_dict into a module-level fixture; adds repository-level tests for the with/without message_id branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2479 +/- ##
==========================================
+ Coverage 91.34% 91.37% +0.03%
==========================================
Files 248 265 +17
Lines 20709 22653 +1944
==========================================
+ Hits 18916 20699 +1783
- Misses 1793 1954 +161 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Why was this change needed? (You can also link to an open issue here)
Other information: