Repository navigation
Conversation
… in AI governance plugins The AI governance read paths only understood the OpenAI Chat and Responses shapes: usage metering, token limiting, PII masking and the normalized request view misclassified or skipped Anthropic and Gemini traffic routed natively or through conversion. - A shared shape detector (ai-common) classifies openai-chat, openai-responses, anthropic-messages and gemini-generate-content from the discriminating top-level members; the request view delegates to it. - The request view renders Anthropic (top-level system string/blocks surfaces as the leading system message, text blocks as bounded TEXT, tool_use blocks as tool calls) and Gemini (systemInstruction, contents parts, functionCall) shapes; an Anthropic request without a top-level system classifies as openai-chat, whose readers handle its blocks gracefully. - Usage metering resolves the usage object across the three field families and never invents a total for Anthropic; Anthropic streams merge per field across message_start/message_delta (input from the former, cumulative output from the latter) while openai-chat and Gemini snapshots replace wholesale; message_stop completes an Anthropic stream instead of a [DONE] sentinel. - The token limiter reads Anthropic stream usage events and Gemini usageMetadata; last event wins, matching the cumulative counters. - Two PII profiles cover Anthropic (system, text blocks, tool_use input string leaves, response content) and Gemini (systemInstruction, parts, candidates), selected before the generic messages shape so system is never silently dropped. - A plugin x protocol coverage matrix documents the full/partial/none cells and the explicit-gap reporting that already existed.
The token limiter accounts output tokens only, and the usage tap reads the field families by name, so the shared constant had no reader.
wy471x
left a comment
wy471x
left a comment
There was a problem hiding this comment.
This is a large, careful change and most of it lands well: the two new PII profiles are cleanly structured, moving the request view onto a single detector is the right call, and the "no invented total" plus Anthropic non-streaming pin are exactly what #8458 asked for. The blocking items below are concentrated in PII coverage and in one code path that no production request can reach.
1. Gemini functionCall.args is not redacted, and is reported as covered (blocking).
#8458 asks for PII profiles covering "Gemini (systemInstruction, contents[].parts[].text, functionCall.args)". GeminiGenerateContentProfile.collectPartsSlots only builds a slot from part.get("text"), so {"functionCall":{"args":{"phone":"13800138000"}}} passes through unmasked. Worse, PiiProtocolProfiles.validContentObject / validTextMember accept a part with no text member, so select() still returns GEMINI and the rule reports success — precisely the "silently treated as covered" case the issue forbids. AnthropicMessagesProfile already walks tool_use.input, so the fix is symmetric: add a depth-bounded walker over functionCall.args on the request side and over tool-call parts on the response side.
2. AnthropicMessagesProfile.collectInputSlots never descends into arrays (blocking).
The walker recurses only into value.isJsonObject(). tool_use.input holding an array of objects — e.g. {"filters":[{"email":"[email protected]"}]} — is passed through untouched, and the MAX_INPUT_DEPTH = 2 bound silently drops anything deeper. Neither is visible to the caller, so the scan reports clean while the payload leaves with PII. Please add array descent, and either widen the bound or bound by slot count so that truncation is observable.
3. anthropic-messages is not a protocol the proxy can produce today, so the Anthropic stream merge is dead code (blocking-ish).
mergeUsageFields and the inspectCompletion extension both key off "anthropic-messages".equals(targetProtocol), and targetProtocol comes from AiProxyEngine → prepared.targetProtocol(). The only AiProxyProtocol implementations are OpenAiChat (openai-chat) and OpenAiResponses (openai-responses). No production path supplies anthropic-messages, so the split-event merge (input from message_start.message.usage, cumulative output from message_delta.usage) and the message_stop completion evidence only run in tests that inject the literal string. If this is deliberate groundwork for #8457 / #8388, please say so in the PR body and mark the matrix cell accordingly; otherwise derive the protocol from AiClientProtocolShape so the branch is reachable.
4. The coverage matrix overstates two cells.
- Usage metering /
anthropic-messagesis listed asfull, but per item 3 the merge path cannot be reached from a real request. - "An Anthropic request without a top-level
system... its text blocks still surface through the openai-chat readers" contradicts the PR's ownanthropicBodyWithoutSystemClassifiesAsChatShape, which assertsContentKind.STRUCTURED— no text. Either make the openai-chat reader surface those blocks or correct the doc. - The doc says the protocol names are those of
AiProxyProtocol.name(), butanthropic-messagesandgemini-generate-contentare not among them.
5. Protocol detection is now triplicated.
The issue asked for the protocol to come "from the shared detector". AiClientProtocolShape is used by the view, but PiiProtocolProfiles.select keeps its own validity-based selection and AiProxyProtocol.matches keeps a third. They already disagree: {"messages":[...],"system":"s"} is anthropic-messages for the view and openai-chat for the proxy. At minimum, document the divergence so it is intentional and visible.
Smaller points
AiClientProtocolShape.protocolNamehas no call site; either use it for the view's protocol hint or drop it.AiUpstreamUsageTaphardcodes"usageMetadata"/"candidatesTokenCount"while this PR addsConstants.USAGE_METADATA/CANDIDATES_TOKEN_COUNTused only by the token limiter. Pick one source.- Anthropic
cache_read_input_tokens/cache_creation_input_tokensare ignored, so cached prompts meter at ~0 prompt tokens. The issue does not require it, but the matrix should say so. - Anthropic
thinkingblocks are never redacted (collectBlockSlotsonly readstext). Extended-thinking prose is model- and user-authored content and appears on both request and response paths. - Gemini
streamGenerateContentwithoutalt=ssereturns a JSON array; bothAiUpstreamUsageTapand the token limiter reject non-object roots, so that shape is unmetered. Acceptable as a gap, but the matrix should readpartialfor it. - The
AiProxyEnginechange passesnullas the tap protocol for non-2xx responses, which changes upstream-tap semantics beyond usage fields. Worth a sentence in the PR body since it is not part of the issue.
Verdict: request changes. Items 1 and 2 are PII gaps in exactly the fields the issue names, and both are reported as successfully scanned; item 3 means a documented "full" cell is unreachable from any real request. The rest of the design looks sound and should land once those are closed.
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Stream Agent analysis
PR #8478 adds multi-protocol support for Anthropic Messages and Gemini Generate Content across all AI governance plugins (usage metering, token limiting, PII masking, request parsing). The frozen diff (82669 chars, all pages read, has_more:false) and targeted reads verify: new AiClientProtocolShape classifier with correct JSON-field discrimination; AiRequestViewParser branches for both protocols; AiUpstreamUsageTap wired with tapProtocol to extract usage from protocol-specific response paths; AiTokenLimiterPlugin estimating prompt tokens per protocol; PiiProtocolProfiles registering dedicated profile classes with correct JSON paths; Constants defining matching string identifiers. Cross-file contracts are consistent. All 70 CI checks passed. No concrete blocking defects. Complete coverage achieved. Decision: PASS. Verified head SHA: 8126c56. No further action needed. Recommend merge. Ready for delivery. Multi-protocol governance gap closed. Coverage complete. No missing evidence.
Evidence
- Full VERSIONED_DIFF read to completion (offset 77000, has_more:false); all 15 changed files inspected.
- Cross-file protocol wiring verified: AiClientProtocolShape.detect() → AiRequestViewParser.parse() → AiProxyEngine.tapProtocol → AiUpstreamUsageTap.mergeUsageFields() → AiTokenLimiterPlugin.estimatePromptTokens() → PiiProtocolProfiles.getProfile().
- All 70 CI checks passed per context resource.
Recommended next steps
Scope: captured diff and GitHub check metadata only; no local tests were executed.
Task: 27c7a05b-32e2-491a-9bcc-8c2f473cd4ef · Commit: 8126c563a92ca752f1be4540e00cfa04e1edd7ab
Review verdict: PASS
Complete diff coverage, passing CI, and no concrete blocking defect. The implementation correctly classifies, meters, limits, and masks Anthropic Messages and Gemini Generate Content payloads across all AI governance plugins with consistent cross-file contracts.
Publication: APPROVE (EXPLICIT_PASS)
Fixes #8458.
The AI governance read paths only understood the OpenAI Chat and Responses
shapes. Anthropic Messages and Gemini traffic routed through the gateway —
natively, through conversion (#8457), or in passthrough mode (#8454) — was
under-counted by usage metering, missed by token limits, and invisible to
PII masking; the normalized request view classified every Anthropic body as
openai-chat and every Gemini body as unsupported.
What changed
AiClientProtocolShape, ai-common): oneclassification for the four client shapes from their discriminating
top-level members —
messages+system→ anthropic-messages,messages→openai-chat,
input→ openai-responses,contents→gemini-generate-content. The request view delegates to it instead of its
own ad-hoc shape checks. An Anthropic request without a top-level
systemis structurally indistinguishable from openai-chat and classifies as such;
the openai-chat readers handle its text blocks gracefully.
system(string or text blocks) as the leading system message and its text blocks
as bounded TEXT (they are plain prose — the exception to the
kind-only rule for structured content, which stays untouched for openai
multimodal parts);
tool_useblocks surface as tool calls. Gemini renderssystemInstruction/contentsparts as bounded TEXT andfunctionCallparts as tool calls. Zero contract change to
AiNormalizedRequestView.AiUpstreamUsageTap): thenativeChatctor flagbecomes the target protocol;
capture()resolves the usage object acrossthe three mutually exclusive field families. Anthropic reports
input_tokens/output_tokenswith no total ever invented; Anthropicstreams merge per field across their split events — input from
message_start.message.usage, cumulative output frommessage_delta.usage— while openai-chat and Gemini snapshots replace wholesale (counters are
never summed, per the issue).
message_stopcompletes an Anthropic streaminstead of the
[DONE]sentinel; Gemini streams have no terminal markerand keep today's evidence behavior rather than guessing.
usageOfreads Anthropic stream events(
message_start.message.usage,message_delta.usage— the latter's rootusage already matched) and Gemini
usageMetadata;outputTokensOfgainscandidatesTokenCount. Last event wins, matching the cumulative counters,and the usage is still recorded exactly once per response. Anthropic
non-streaming already worked by field-name coincidence and is pinned by a
regression test.
AnthropicMessagesProfile(top-levelsystemstring/blocks,messages[].contentstring/text-blocks,tool_use.inputstring leaves viaa depth-bounded walker, response
content[]blocks) andGeminiGenerateContentProfile(systemInstruction,contents[].parts,response
candidates[].content.parts). Anthropic selection is checkedbefore the generic messages shape so
systemis never silently dropped.AI_PROTOCOL_COVERAGE.mddocuments the plugin ×protocol cells with the full/partial/none levels and the explicit-gap
reporting that already existed (PII null-profile, view
unsupported,usage
UNKNOWN) — nothing treats an uninspectable protocol as covered.Out of scope, per the issue's coordination: converter registration (#8457),
native provider transports (#8388/#8389), passthrough mode (#8454);
sensitive-word scanning is shape-independent and unaffected.
Tests
Twenty new deterministic cases (inline JSON fixtures, no sleeps) across the
four sites, each red on the pre-fix implementation: tap (anthropic
non-streaming without invented total; anthropic stream replace-per-field with
message_stopevidence; gemini body; gemini stream last-snapshot), tokenlimiter (anthropic non-stream regression, anthropic stream recorded exactly
once from the cumulative event, gemini body and stream), PII (selection
order, system/text-blocks/tool-input redaction preserving the tree, response
slots, gemini request/response), view + detector (protocol and operation
assertions, tool-call surfacing, chat fallback for system-less Anthropic,
five-way detection).
Module results on the current master baseline:
shenyu-common711 (4pre-existing skips),
shenyu-plugin-ai-common62,shenyu-plugin-ai-proxy537,
shenyu-plugin-ai-token-limiter66,shenyu-plugin-ai-pii86 — allgreen; checkstyle clean per module.