fix(openrouter): allowlist metadata pass-through, drop eager serialization#608
Merged
Abhijeet Prasad (AbhiPrasad) merged 1 commit intoJul 21, 2026
Merged
Conversation
…ation Aligns the OpenRouter integration with `.agents/skills/sdk-integrations/SKILL.md`, following #604 (mistral), #603 (livekit_agents), #594 (litellm), #593 (langchain), and #585 (claude_agent_sdk). Two issues from the audit: - **Excess serialization.** `sanitize_openrouter_logged_value` called `bt_safe_deep_copy` on every span input, every kwargs dict, every response (for both output and metadata extraction), and every usage dict. Braintrust already runs `bt_safe_deep_copy` at log time in `Span.log_internal`, so every one of those was a duplicate walk. Dropped the wrapper; reads now go through a small `_get_field(obj, key)` helper (dict-or-attribute) and pydantic responses flow straight to `span.log(...)`. - **Denylist metadata → per-surface allowlist.** `_OMITTED_KEYS` was a small denylist (`execute`, `render`, `nextTurnParams`, `requireApproval`); the old `_build_openrouter_metadata` then dumped every other kwarg — including `messages`, `api_key`, `extra_headers`, and unknown kwargs — into `metadata`. Replaced with three request allowlists (`_CHAT_REQUEST_KEYS`, `_EMBEDDINGS_REQUEST_KEYS`, `_RESPONSES_REQUEST_KEYS`) and three response allowlists, mirroring `litellm/_SAFE_METADATA_KEYS` and mistral's `_*_METADATA_KEYS` family. `is_byok` is still extracted from `usage`. Also collapsed the three near-identical `_finalize_*_response` functions into one parameterized `_finalize_response(..., *, output, response_keys)`, and factored the shared model/provider stamping (`_stamp_model_and_provider`) used by both request and response metadata builders. ## Test plan - [x] **No new mocks/fakes.** All coverage stays on the existing `@pytest.mark.vcr` cassette-backed tests in `integrations/openrouter/test_openrouter.py` (chat sync/stream/async, embeddings, responses sync + async-stream, `setup_*_creates_spans`, `setup_*_is_idempotent`, auto_instrument subprocess). - [x] **No cassette re-recording.** HTTP behavior is unchanged; only in-process span shaping was touched. - [x] `cd py && nox -s "test_openrouter(latest)"` — 9/9 passed. - [x] `cd py && nox -s "test_openrouter(0.6.0)"` — 9/9 passed. - [x] `ruff check` + `ruff format --check` — clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Abhijeet Prasad (AbhiPrasad)
approved these changes
Jul 21, 2026
Abhijeet Prasad (AbhiPrasad)
enabled auto-merge (squash)
July 21, 2026 17:19
Abhijeet Prasad (AbhiPrasad)
deleted the
fix/openrouter-allowlist-metadata
branch
July 21, 2026 17:28
6 tasks
Abhijeet Prasad (AbhiPrasad)
pushed a commit
that referenced
this pull request
Jul 21, 2026
…eshape (#610) ## Summary Aligns the pydantic_ai integration with `.agents/skills/sdk-integrations/SKILL.md`, following the same pattern as #606 (openai), #608 (openrouter), and #604/#603/#594 (anthropic/livekit/litellm/mistral). Audit turned up two issues: - **Denylist metadata pass-through (real leak vector).** `_build_agent_input_and_metadata` and `_build_direct_model_input_and_metadata` did `for key, value in kwargs.items(): if key not in [...]: input_data[key] = value` — every non-`deps` / non-`{model,messages}` kwarg landed in span input. That flowed `usage` (a mutable counter), `event_stream_handler` (a callable), `infer_name`, `instrument`, and anything pydantic_ai adds in future SDK versions straight into telemetry. `deps` (user's app-level container — commonly holds API keys / DB connections) was the sole excluded key on the agent path; on the direct-model path there was no exclusion at all. Replaced with per-surface allowlist tuples (`_AGENT_INPUT_ALLOWLIST`, `_DIRECT_MODEL_INPUT_ALLOWLIST`) covering the known kwargs across `Agent.run` / `run_sync` / `run_stream` / `run_stream_events` / `to_cli_sync` and `direct.model_request*`. Explicitly excludes `deps`, `usage`, `event_stream_handler`, `infer_name`, `instrument`. - **Eager message reshaping dropping fields.** `_shape_message` / `_shape_content_part` / `_shape_model_response` reshaped every message into shallow dicts via a hard-coded field allowlist (`_MESSAGE_FIELDS`, `_PART_FIELDS`, `_RESPONSE_FIELDS`) even when no binary content needed materialization. That dropped real pydantic_ai v2 fields — `instructions`, `run_id`, `conversation_id`, `metadata`, `state`, `provider_details`, `outcome`, ... — and burned CPU rebuilding dicts that `bt_safe_deep_copy` (which already handles dataclasses + Pydantic models) would produce anyway at log time. Added `_has_binary_leaf` walker; the four shape helpers now short-circuit to the raw object when no binary is present. Reshape (and its field allowlist) only runs when a binary attachment actually needs materialization. Also: - Extracted `_shape_toolset(ts)` from a nested 20-line block inside `_build_agent_input_and_metadata`. - Trimmed docstrings/comments that just restated the helper name. Kept the ones explaining non-obvious *why* (portal-thread context propagation, wrapper-vs-leaf token ownership, 1.78.0 ToolManager alias, `_system_prompts` private-API access). - Hoisted `import inspect` from inside `_shape_type` to module top. ## Test plan - [x] **No new mocks/fakes.** All provider-behavior coverage stays on the existing `@pytest.mark.vcr` cassette-backed tests. - [x] **No cassette re-recording.** HTTP behavior unchanged; only in-process input/shape logic was touched. - [x] `nox -s "test_pydantic_ai_integration(latest)"` — 68/68 pass (`pydantic-ai==2.13.0`) - [x] `nox -s "test_pydantic_ai_integration(1.10.0)"` — 68/68 pass (min-supported matrix version) - [x] `nox -s "test_pydantic_ai_wrap_openai(latest)"` — green - [x] `nox -s "test_pydantic_ai_logfire(latest)"` — 1/1 pass Two new tests pin the security invariant: - `test_agent_input_kwargs_are_allowlisted` — asserts `deps` / `usage` / `event_stream_handler` / `infer_name` never land in span input; allowlisted kwargs (`instructions`, `usage_limits`, `retries`, `conversation_id`, `metadata`) do. - `test_direct_model_request_kwargs_are_allowlisted` — asserts `instrument` never leaks; `model_settings` / `model_request_parameters` do. One existing test replaced: `test_v2_message_and_response_fields_are_shaped` → `test_shape_message_and_response_pass_through_when_no_binary` — the old test was locking in the reshape that was losing v2 fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- sfk:created-approved-by --> Created by abhijeet <!-- sfk:slack-thread --> [Slack thread](https://starfolkai.slack.com/archives/C0AQDETAVT3/p1784659060950489?thread_ts=1784659060.950489&cid=C0AQDETAVT3) --------- Co-authored-by: Starfolk <noreply@starfolk.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Aligns the OpenRouter integration with
.agents/skills/sdk-integrations/SKILL.md, following #604 (mistral), #603 (livekit_agents), #594 (litellm), #593 (langchain), and #585 (claude_agent_sdk). Audit found two issues:Excess serialization.
sanitize_openrouter_logged_valuecalledbt_safe_deep_copyon every span input, every kwargs dict, every response (for both output and metadata extraction), and every usage dict. Braintrust already runsbt_safe_deep_copyat log time inSpan.log_internal(logger.py:4699), so every one of those was a duplicate walk. Dropped the wrapper; reads now go through a small_get_field(obj, key)helper (dict-or-attribute) and pydantic responses flow straight tospan.log(...).Denylist metadata → per-surface allowlist.
_OMITTED_KEYSwas a small denylist (execute,render,nextTurnParams,requireApproval); the old_build_openrouter_metadatathen dumped every other kwarg — includingmessages,api_key,extra_headers, and unknown kwargs — intometadata. Replaced with three request allowlists (_CHAT_REQUEST_KEYS,_EMBEDDINGS_REQUEST_KEYS,_RESPONSES_REQUEST_KEYS) and three response allowlists (_CHAT_RESPONSE_KEYS,_EMBEDDINGS_RESPONSE_KEYS,_RESPONSES_RESPONSE_KEYS), mirroringlitellm/_SAFE_METADATA_KEYSand mistral's_*_METADATA_KEYSfamily.is_byokis still extracted fromusage(it's still nested there).Also collapsed the three near-identical
_finalize_*_responsefunctions into one parameterized_finalize_response(..., *, output, response_keys), and factored the shared model/provider stamping (_stamp_model_and_provider) used by both request and response metadata builders.Net: 1 file changed, +262 / −178 (net −0 after adding the allowlist tuples, but the tracing surface itself shrank meaningfully).
Test plan
@pytest.mark.vcrcassette-backed tests inintegrations/openrouter/test_openrouter.py(chat sync/stream/async, embeddings, responses sync + async-stream,setup_*_creates_spans,setup_*_is_idempotent, auto_instrument subprocess).cd py && nox -s "test_openrouter(latest)"— 9/9 passed.cd py && nox -s "test_openrouter(0.6.0)"— 9/9 passed.ruff check+ruff format --check— clean.🤖 Generated with Claude Code
Created by abhijeet
Slack thread