You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Runnable resources already carry the caller identity as x-user-id and the
run's session as x-session-id; add the calling agent's name so downstream
services can attribute a call to the agent that made it.
agent_name -> x-agent, header-only for every runnable like session_id
(no run params type declares it as a body field), so the base
_RUN_CONTROL_KEYS excludes it and Model/Tool also strip it from the v1/URL
builder paths. Declared on ModelRunParams, not BaseRunParams, for the same
reason as session_id: AgentRunParams must stay free of look-alike keys.
Collapse the header list into one source of truth,
RunnableResourceMixin._RUN_HEADER_KEYS. _headers_for_run now iterates it and
Model._SDK_ONLY_PARAMS derives its header half from it, instead of both
re-listing the same kwargs.
The refactor is motivated by 9cba90d: the merge of 'test' into development
(dc8c4c2) resolved the _SDK_ONLY_PARAMS hunk in favour of the older frozenset,
dropping session_id while keeping its tests and comments -- so the value rode
in the request body until someone noticed. With the keys derived rather than
duplicated, the payload filters can no longer drift from the wire.
Adds unit coverage for x-agent emission and payload exclusion (v2 + v1
builders), all three headers together, and
TestRunHeaderKeysAreSingleSourceOfTruth -- which asserts every _RUN_HEADER_KEYS
kwarg is stripped from both payload paths, and would have caught the regression
above.
Reviewed together with aixplain/aixplain-agents#306. The _RUN_HEADER_KEYS single-source-of-truth refactor is the right call — it makes the 9cba90d class of merge regression structurally impossible, and TestRunHeaderKeysAreSingleSourceOfTruth proves it. Coverage is thorough. One change requested, one nit.
1. Non-ASCII agent_name surfaces as a deep UnicodeEncodeError at send time — validate and raise a clear error instead
Unlike identifier and session_id (platform-generated, ASCII-safe ids), agent_name is user-authored free text. The header builder passes it straight through:
Verified: requests/urllib3 accepts the header at prepare time but crashes at send with UnicodeEncodeError: 'latin-1' codec can't encode character 'ı'... for a name like "Ürün Asistanı" (any CJK/Arabic name fails too). That error gives the caller no hint that the agent's name is the problem.
Decision: fail fast with a descriptive error — do not sanitize or alter the name. In _headers_for_run (or a small validator it calls), reject a non-ASCII header value with a ValueError naming the offending kwarg and value, e.g. ValueError("agent_name 'Ürün Asistanı' cannot be sent as the x-agent header: header values must be ASCII"). Please add a test with a non-ASCII name asserting the error.
agent_name: Name of the agent making this call, emitted as the ``x-agent``
header so downstream services can attribute it to the calling agent
(for a team run, the specific sub-agent — not the team). Header-only,
on the same terms as ``session_id``.
"for a team run, the specific sub-agent — not the team" is true for delegated calls, but the orchestrator's own direct tool calls carry the root/team name. Worth a small wording tweak so consumers of the header aren't surprised.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Runnable resources already carry the caller identity as
x-user-idand therun's session as
x-session-id; add the calling agent's name so downstreamservices can attribute a call to the agent that made it.
agent_name->x-agent, header-only for every runnable likesession_id(no run params type declares it as a body field), so the base
_RUN_CONTROL_KEYS excludes it and Model/Tool also strip it from the v1/URL
builder paths. Declared on ModelRunParams, not BaseRunParams, for the same
reason as
session_id: AgentRunParams must stay free of look-alike keys.RunnableResourceMixin._RUN_HEADER_KEYS. _headers_for_run now iterates it and
Model._SDK_ONLY_PARAMS derives its header half from it, instead of both
re-listing the same kwargs.
The refactor is motivated by 9cba90d: the merge of 'test' into development
(dc8c4c2) resolved the _SDK_ONLY_PARAMS hunk in favour of the older frozenset,
dropping
session_idwhile keeping its tests and comments -- so the value rodein the request body until someone noticed. With the keys derived rather than
duplicated, the payload filters can no longer drift from the wire.
Adds unit coverage for x-agent emission and payload exclusion (v2 + v1
builders), all three headers together, and
TestRunHeaderKeysAreSingleSourceOfTruth -- which asserts every _RUN_HEADER_KEYS
kwarg is stripped from both payload paths, and would have caught the regression
above.