Uh oh!
There was an error while loading. Please reload this page.
fix(responses): monitor-mode output guardrails must not hold back or fail streaming closed - #760
Merged
Merged
Conversation
…fail streaming closed The streaming /v1/responses path (both the verbatim OpenAI forward and the cross-provider bridge) entered the whole-response hold-back branch whenever ANY output-hook guardrail was attached, ignoring the chain's resolved stream_output_policy. A monitor-only chain resolves to EndOfStreamCheck — it can never block by definition — yet its stream was fully buffered (no bytes until end of generation) and, past the 256 KiB cap, rejected 422 content_filter. Monitor mode could therefore block exactly the long generations Codex produces: 422 + 0 tokens + tens of seconds latency, intermittently. chat.rs and messages.rs already gate hold-back on the policy; /v1/responses was the one deviating surface. Now hold-back engages only when the resolved policy holds back (Window/BufferFull — any block-capable chain, unchanged fail-closed secure default). An EndOfStreamCheck chain forwards the SSE live and runs the same two-phase scan (blob check + segment pass) at end-of-stream so would-block / would-mask monitor hits still reach telemetry; a Block verdict there (only reachable via the documented mandatory-unavailability composition) is signalled with a trailing error frame on the bridge path, mirroring chat's EndOfStreamCheck behavior. LiteLLM baseline: logging-only / on_flagged=monitor guardrails never withhold or fail a stream, and no scan-buffer size cap exists at all — the live-forward + observe-at-end behavior matches; keeping the fail-closed cap for blocking chains is our stricter (OOM-bounding) divergence, unchanged here. Fixesapi7/AISIX-Cloud#1010
📝 WalkthroughWalkthrough
ChangesResponses streaming guardrails
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ResponsesHandler
participant StreamingPath
participant Guardrail
participant UsageEvent
Client->>ResponsesHandler: request streaming /v1/responses
ResponsesHandler->>StreamingPath: apply output policy
StreamingPath-->>Client: forward live SSE or hold frames
StreamingPath->>Guardrail: scan output at stream end
Guardrail-->>StreamingPath: monitor hits or redaction result
StreamingPath->>UsageEvent: emit usage and monitor hits
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…d bridge live-scan text Audit findings on the live-forward path: - H1: the explicit completion took the guard slot BEFORE awaiting the end-of-stream observation. SDK clients close the connection right after the terminal frame, dropping the generator at that await — a fully-delivered 200 stream then emitted no UsageEvent at all (billing/logs/TPM post-stream accounting lost) whenever the monitor chain contained a remote provider. The scan now runs while the guard stays armed (reading the captured text via a non-consuming clone), so a mid-scan disconnect falls back to the guard's Drop emit, and only then does the explicit completion take the slot. Regression test parks the scan on a delayed moderation backend, drops the body, and asserts the event still arrives (mutation-verified). - M1: the bridge live path fed the unbounded assembled text to the scan (the hold-back cap no longer applies there); it is now truncated to DEFAULT_STREAM_OUTPUT_BUFFER_BYTES on a char boundary, matching the verbatim path's EosOutputScan bound. - L1: the masked-segment capture rebuild is gated on hold-back mode — the live walk is read-only, so a masked outcome there must not clobber the capture from the empty joined buffer. - L4: correct the capture-cap comment (terminal text is bounded by the SSE frame cap and re-truncated per consumer, not by the scan bound).
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
AISIX-Cloud#1010: a customer running 0.3.1 with an aliyun output guardrail in monitor mode saw intermittent
422rejections on Codex traffic — dashboard rows showingcontent_filter, 0 tokens, $0.0000, and 16–26 s latency. Monitor mode is documented to observe and never block.Root cause: the streaming
/v1/responsespath (both the verbatim OpenAI forward and the cross-provider bridge) entered the whole-response hold-back branch whenever any output-hook guardrail was attached, ignoring the chain's resolvedstream_output_policy:EndOfStreamCheck(can never block), yet its stream was fully buffered — the client saw nothing until the generation finished;DEFAULT_STREAM_OUTPUT_BUFFER_BYTES= 256 KiB, hit by exactly the long SSE streams Codex produces), the request was rejected422 content_filterbefore any verdict ran — MonitorGuardrail never got the chance to downgrade.chat.rsandmessages.rsalready gate hold-back on the policy (holds_back()/ BufferFull-only);/v1/responseswas the one deviating surface (family audit: completions/audio/passthrough/realtime have no streaming output-buffer path).Fix
Window/BufferFull— any block-capable chain). Block-mode behavior is unchanged, including the fail-closed cap.EndOfStreamCheckchain (monitor-only) forwards the SSE live and runs the same two-phase scan (blob check + segment pass) at end-of-stream, sowould_block/would_maskmonitor hits still reach telemetry. The scan runs while the completion guard stays armed: SDK clients close the connection right after the terminal frame, and a disconnect mid-scan must fall back to the guard's Drop emit rather than lose the usage event for a fully-delivered stream. Scan input is bounded to the same 256 KiB on both paths so observation provider calls stay bounded.Blockverdict on the live path (only reachable via the documentedmandatory-unavailability composition) is logged and, on the bridge path, signalled with a trailing error frame — mirroring chat'sEndOfStreamCheckbehavior.Behavior change
Monitor-mode-only output chains on streaming
/v1/responses: clients now receive tokens live (no whole-response buffering latency) and oversized responses are no longer rejected. Blocking chains: no change.LiteLLM baseline
LiteLLM never withholds or fails a stream for logging-only /
on_flagged: monitorguardrails, and has no scan-buffer size cap at all — live-forward + observe-at-end matches the baseline. Keeping the fail-closed cap for blocking chains is our stricter, OOM-bounding divergence (pre-existing, unchanged).Tests
responses.rs): oversized (300 KB) stream + monitor guardrail released with 200 on both paths (verbatim + cross-provider bridge) — both fail before the fix (mutation-verified); live-pathwould_blockobservation recorded on the usage event; disconnect-during-scan still emits the usage event (parks the scan on a delayed moderation backend, drops the body — mutation-verified against the take-before-await shape); existing block-mode oversized fail-closed tests still pass unchanged.tests/e2e): self-gating block→monitor flip on a >256 KiB/v1/responsesstream — block mode 422s (pins the secure default), monitor mode releases the full SSE live.Fixes api7/AISIX-Cloud#1010