feat: expose deterministic action_ref on ToolContext - #4549
feat: expose deterministic action_ref on ToolContext#4549sylvesterkaczmarek wants to merge 5 commits 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: bd08fcc265
ℹ️ 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".
| self.run_config = None | ||
| self._action_ref = _compute_tool_action_ref( | ||
| agent.name if agent is not None else None, | ||
| self.qualified_tool_name, |
There was a problem hiding this comment.
When an agent has a supported visible bare tool and deferred-loading sibling with the same name, the deferred call carries the synthetic namespace == name. qualified_tool_name uses tool_trace_name, which deliberately collapses that namespace, so the bare and deferred calls produce the same action_ref when their arguments match. Hook consumers can therefore conflate two distinct tool actions; commit to the canonical lookup kind and namespace instead of the trace display name.
AGENTS.md reference: AGENTS.md:L159-L159
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. action_ref now commits to the canonical function-tool lookup identity, preserving the bare/namespaced/deferred-top-level distinction. Added regression coverage for a bare tool and deferred sibling with the same visible name.
| ) | ||
| except (json.JSONDecodeError, TypeError, ValueError): | ||
| return b"raw\0" + tool_arguments.encode("utf-8") | ||
| return b"json\0" + canonical_arguments.encode("utf-8") |
There was a problem hiding this comment.
When model-generated arguments contain a lone surrogate escape such as {"x":"\ud800"}, json.loads succeeds and ensure_ascii=False leaves the surrogate in canonical_arguments, but this UTF-8 encoding raises UnicodeEncodeError. Because ToolContext is constructed before normal tool invocation and its error conversion, the new reference calculation aborts the entire run; preserve ASCII escapes or otherwise handle surrogate encoding without raising.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Canonical JSON now preserves ASCII escapes, and the raw fallback safely handles surrogate code points. Added regression coverage for \ud800.
| - `action_ref` – a deterministic `act_v1_...` commitment for the action, or `None` when active agent metadata is unavailable | ||
|
|
||
| For SDK-native agents, `action_ref` uses the configured `Agent.name` as the agent identifier and commits to that identifier, the qualified tool name, and a SHA-256 digest of the request arguments. Valid JSON arguments are normalized by sorting object keys and removing insignificant whitespace before hashing; malformed JSON falls back to the exact raw argument bytes. The reference is computed when `ToolContext` is created, does not depend on the provider-generated `tool_call_id`, and remains the same for the corresponding `on_tool_start` and `on_tool_end` hook payloads. |
There was a problem hiding this comment.
This commit introduces action_ref and simultaneously documents it in docs/, although the behavior is not present in the latest published release. The repository requires documentation for unreleased feature behavior to land in a separate docs-only change so its merge can be coordinated with the release; remove these lines from this feature commit.
AGENTS.md reference: AGENTS.md:L76-L78
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Removed the unreleased action_ref documentation from this feature PR; documentation can land separately in a docs-only change coordinated with the release.
bd08fcc to
847a5be
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a00826e402
ℹ️ 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".
| self.run_config = None | ||
| self._action_ref = _compute_tool_action_ref( | ||
| agent.name if agent is not None else None, | ||
| get_function_tool_lookup_key(self.tool_name, self.tool_namespace), |
There was a problem hiding this comment.
When a Responses CustomTool is invoked, CustomToolAction.execute also constructs a ToolContext with agent metadata, so this line assigns it a function-tool ("bare", name) identity. Because the runtime routes function and custom calls by their call type, an agent can expose both families under the same name; identical input text then produces the same action_ref for different handlers. JSON-looking custom inputs are also normalized despite CustomTool explicitly receiving raw text, so inputs such as {"x":1} and { "x": 1 } can behave differently while sharing a reference. Include the tool family and raw-input semantics in the commitment, or leave action_ref unset for non-function tools.
Useful? React with 👍 / 👎.
| except (json.JSONDecodeError, TypeError, ValueError): | ||
| return b"raw\0" + tool_arguments.encode("utf-8", errors="surrogatepass") |
There was a problem hiding this comment.
When model-generated arguments contain sufficiently deep but syntactically valid nesting, json.loads or json.dumps raises RecursionError, which this exception list does not catch. ToolContext is constructed before the function-tool invocation and its existing input-error conversion, so action-ref calculation aborts the entire run instead of following the normal tool failure path. The surrogate case is fixed, but deeply nested JSON is fresh evidence of another parser-accepted input path that still escapes construction; route this exception through the raw commitment fallback as well.
Useful? React with 👍 / 👎.
| ensure_ascii=True, | ||
| sort_keys=True, | ||
| separators=(",", ":"), | ||
| allow_nan=False, |
There was a problem hiding this comment.
For a valid JSON number that overflows Python's float range, such as {"x":1e400}, json.loads produces infinity and allow_nan=False makes json.dumps raise ValueError; the code then hashes the raw string. Consequently {"x":1e400} and { "x": 1e400 } receive different references even though they differ only by insignificant whitespace, violating the stated normalization behavior for valid JSON. Parse or serialize numeric tokens without float overflow so these requests remain canonicalized.
Useful? React with 👍 / 👎.
|
Thanks for narrowing the request from #4543. The existing function-tool hook boundary already exposes ToolContext.tool_name, tool_namespace, tool_arguments, and the active agent, so an integration can compute its own commitment using its stable application identity, policy scope, and canonicalization rules. The SDK does not currently have a stable agent identity contract: Agent.name is a display name and may be duplicated or changed. Consequently, the proposed action_ref does not provide the claimed third-party verification property. The current implementation also assigns function-tool identity and JSON normalization semantics to custom tools, whose inputs are raw text. I am going to close this PR. We can reconsider a narrower SDK change if a concrete workflow identifies information that is unavailable through the existing hooks. |
2 participants
This pull request adds a deterministic
action_reftoToolContextso function-tool lifecycle hooks can bind an action to its request content without relying on the provider-generatedtool_call_id.For SDK-native agents, the v1 commitment uses the configured
Agent.nameas the available agent identifier, the canonical function-tool lookup identity (including bare, namespaced, and deferred-top-level distinctions), and a SHA-256 digest of the request arguments. Valid JSON arguments are normalized by sorting object keys and removing insignificant whitespace before hashing; malformed JSON falls back to the raw argument representation. JSON serialization preserves ASCII escapes so parser-accepted surrogate escapes cannot abortToolContextconstruction.The resulting
act_v1_...value is computed when theToolContextis created and is reused unchanged by the correspondingon_tool_startandon_tool_endpayloads. It does not depend on the provider-generatedtool_call_id.The change deliberately stays at the existing
ToolContextboundary. It does not add signing, persistence, policy semantics, or a new agent identity API.Regression coverage checks equivalent JSON normalization, surrogate escapes, binding to agent/tool/request inputs, bare vs deferred-top-level identity separation, namespace separation, independence from tool call IDs, and stability across start/end hooks.
Documentation is intentionally omitted from this feature PR because the behavior is unreleased; the repository's documentation release-timing policy requires that documentation to land separately when maintainers coordinate it with the release.
This pull request resolves #4543.