fix(agent): enforce approval gates on final-response tool execution - #28
fix(agent): enforce approval gates on final-response tool execution#28skundu42 wants to merge 1 commit into
Conversation
![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: 4ca60c2737
ℹ️ 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".
| # Prevent PreToolUse from running twice after an approval resume. | ||
| pre_tool_use_applied: Optional[bool] = None | ||
|
|
There was a problem hiding this comment.
When a state contains a pending call, dataclasses.asdict now serializes pre_tool_use_applied even when it is null, while the wire-format version remains 1. An older v1 process in a rolling deployment or rollback passes that object to its three-field ParsedToolCall(**item) constructor and raises TypeError, rather than rejecting a newer state version cleanly. Either keep v1 output backward-compatible or introduce a new serialization version that readers can distinguish.
Useful? React with 👍 / 👎.
| elif outcome["result"] is None: | ||
| # HITL tools cannot produce an unsent result until resumed. | ||
| continue |
There was a problem hiding this comment.
When a PermissionRequest hook allows an approval-gated HITL call while another call still requires user approval, this helper executes the allowed HITL call before saving the gate. If on_tool_called returns None to request a pause, this branch produces neither a result nor a pending call, and the caller subsequently persists only the still-unapproved calls; the HITL call is therefore lost and cannot be resumed. Return paused calls to the caller and retain them in state.
Useful? React with 👍 / 👎.
| effective_call = outcome["call"] | ||
| results.append( | ||
| create_unsent_result(effective_call.id, effective_call.name, outcome["result"].get("result")) | ||
| ) |
There was a problem hiding this comment.
When an ungated tool executes alongside a call that pauses for approval, this success path stores the raw result in UnsentToolResult. On resume, unsent_results_to_api_format JSON-serializes that raw value and never invokes the tool's to_model_output converter, unlike the normal execution path through _tool_result_to_output; tools using content conversion therefore send a different or invalid payload to the model after the approval pause.
Useful? React with 👍 / 👎.
| hook_resolved_unsent.extend( | ||
| await self._execute_calls_to_unsent_results(executable_calls, tools, turn_context) | ||
| ) |
There was a problem hiding this comment.
In a mixed turn containing an executable call and a still-pending approval, this helper executes the former and then the caller immediately saves state and returns. Because the executed calls are not retained and execute_next_turn_params_functions is never run for them, options such as model, temperature, or instructions declared in that tool's next_turn_params are missing from the resumed request, even though the resume is the next model turn after that execution.
Useful? React with 👍 / 👎.
1 participant
Fixes approval-gate bypasses in SDK runtime behavior by: