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
{{ message }}
Repository navigation
fix: send budget notices newest-first and mark them sent only after a successful request - #11448
Each crossed budget threshold (credits or time) waits in a pending set until it is sent as a notice. This change fixes two problems in how those notices are sent:
Out of order. If one request crosses both 80% and 90%, the 90% notice goes first and the 80% notice follows on the next request. That tells the agent it has more room than it was just told.
Lost. A notice was marked as sent while the request was being rewritten, before the provider answered. If that request then failed, the notice was never sent again.
Notice state (guards/steering-thresholds.js)
Both guards (ai-credits-guard.js, timeout-steering.js) share three new helpers:
selectPendingSteeringThreshold returns the highest pending threshold that is above every notice of the same kind still on an unfinished request. So a request never gets the same notice or an older one while another request is carrying one.
acknowledgeSteeringThreshold marks a notice as sent and removes every pending threshold at or below it, so superseded notices are dropped.
claimSteeringThreshold reserves a notice for one request and returns a handle that resolves once: acknowledge() on success, release() on failure.
Request lifecycle
body-handler.js: when it adds a notice to a request, it now reserves it (req.awfSteeringDelivery = claim(...)) instead of marking it sent.
upstream-response.js: when the upstream response is 2xx and finishes, it calls acknowledge().
proxy-request.js: it calls release() when:
rewriting the request body throws;
a guard blocks the request;
the response closes without finishing (upstream error or client disconnect);
the client disconnected while the body was being rewritten, before dispatch.
The response close comes after the response-end handlers, so the release does nothing on success.
pending {80, 90} → request A carries 90 (reserved)
concurrent B → no notice (80 and 90 are not above the reserved 90)
A → 500 → release; pending stays {80, 90}
C carries 90 → 2xx → acknowledge; pending {} (80 dropped, never sent)
Review notes
Trade-off: a long streaming request carrying 80% holds back the 80% notice from other requests until it ends. A newly crossed 90% can still go out meanwhile. Dropping the reservation would allow duplicate notices under concurrency, which breaks the at-most-once check (T2) in gh-aw's routed-steering smoke test. That smoke test's check T1 still sees an 80% notice, because it crosses 80% on its own.
AI-credit config change: the pending, emitted and in-flight sets are now replaced rather than cleared. A request still holding a notice from before the change therefore cannot change the new state.
Updated tests: four existing tests asserted the old behaviour (older notices following newer ones, and a rejected request using up its notice) and were updated. docs/api-proxy-sidecar.md is updated to describe the new rules.
CopilotAI
changed the title
[WIP] Fix issue with budget notices arriving out of order
api-proxy: send budget notices newest-first and mark them sent only after a successful request
Nov 11, 2026
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Notice acknowledgment and release do not consistently account for downstream completion and streaming terminal outcomes, allowing notices to be lost on disconnects or failures, or repeated after successful delivery.
Updates the API proxy’s time and AI-credit budget notices to prioritize newer thresholds and track delivery across request outcomes, addressing #11444.
Changes:
Add shared threshold selection, supersession, and request-reservation helpers.
Connect notice acknowledgement and release to the request lifecycle.
Update regression coverage and delivery documentation.
File
Description
docs/api-proxy-sidecar.md
Documents notice ordering, delivery, and retry rules.
containers/api-proxy/upstream-response.js
Acknowledges notices when 2xx upstream responses end.
@copilot please address the two open review threads on this PR. Both come down to one rule for when a notice counts as delivered: when a complete, successful reply exists, not when the upstream connection happens to end.
Streaming responses: acknowledge the notice once the upstream stream's terminal success event has been seen (Responses response.completed, Messages message_stop, the final Chat Completions chunk or [DONE]), even if the client closes before upstream EOF. Codex does close early. A 2xx stream that ends with response.failed or an error event, or that is cut off before any terminal event, counts as a failure and releases the notice.
Non-streaming responses: acknowledge only after the 2xx upstream body has ended and the downstream response has finished; if the client disconnects before that, the close handler releases the notice.
Please keep this simple. The token tracker already inspects SSE streams for usage and terminal events, including through the Chat/Responses wire translation, so reuse that detection instead of adding a second stream parser. Keep the existing release paths (transform failure, guard block, close before dispatch). Add regression tests for:
a stream closed by the client after response.completed but before upstream EOF (notice not repeated);
a 200 stream ending in response.failed (notice sent again on the next request);
a non-streaming 2xx where the client disconnects before finish (notice sent again).
Keep the scope to this issue (#11444); #11443 is changing body-transform.js in parallel. Please update the delivery rules in docs/api-proxy-sidecar.md to match.
@copilot please address the two open review threads on this PR. Both come down to one rule for when a notice counts as delivered: when a complete, successful reply exists, not when the upstream co...
Implemented and pushed as 219e1ab. Streaming notices now settle from shared terminal-event inspection; non-streaming notices wait for both upstream end and downstream finish. Added the three requested regressions and updated the delivery docs.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unvalidated SSE payloads can terminate the entire proxy, while incorrect stream-completion handling can prematurely acknowledge or repeatedly resend steering notices.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Unresolved acknowledgement gaps can duplicate steering records for usage-bearing replies and redeliver notices for Gemini JSON streams after client disconnects and retries.
No open issues 🎉
2 previously missed issues
In code that hasn't changed since last review
Defer steering attribution until delivery is acknowledged
containers/api-proxy/upstream-response.js:161
For a usage-bearing non-streaming 2xx reply, token-tracker-http.js records the request's steering field on upstream end, before downstream finish acknowledges the notice. If the client disconnects in that gap, the notice is released and its retry produces a second record for the same threshold, failing the routed-steering T2 at-most-once check. Preserve usage accounting, but defer steering attribution until delivery is acknowledged, and cover this disconnect/retry with usage-bearing responses.
Recognize Gemini JSON stream responses for acknowledgment
containers/api-proxy/upstream-response.js:381
Gemini's :streamGenerateContent also supports streaming JSON responses without ?alt=sse; those arrive as application/json, so isStreaming is false here. A client that consumes the completed stream and closes before EOF releases the already-delivered notice, and the next request repeats it. Recognize this endpoint as streaming for steering acknowledgement while preserving the existing JSON response handling.
The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.
Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
Generated from Security Guard · auto (Claude Sonnet 5.5) · ~11 AIC · ⊞ 9.0K · ◷
Java failure: Maven could not create ~/.m2/repository (LocalRepositoryNotAccessibleException). With a writable repo dir, dependency resolution failed: java.security.InvalidAlgorithmParameterException: the trustAnchors parameter must be non-empty (Java truststore is empty in this environment). The build-test label was not added.
.NET needed a writable DOTNET_CLI_HOME/XDG_RUNTIME_DIR (/run/user/1001 is missing) to pass.
Generated by Build Test Suite for #11448 · copilot · auto (Claude Sonnet 5.5) · ~17 AIC · ⊞ 5.9K · ◷ Add label ready-for-aw to run again
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.
github-actions
Bot
added
the
smoke-copilot-network-isolation
Each crossed budget threshold (credits or time) waits in a pending set until it is sent as a notice. This change fixes two problems in how those notices are sent:
Notice state (
guards/steering-thresholds.js)Both guards (
ai-credits-guard.js,timeout-steering.js) share three new helpers:selectPendingSteeringThresholdreturns the highest pending threshold that is above every notice of the same kind still on an unfinished request. So a request never gets the same notice or an older one while another request is carrying one.acknowledgeSteeringThresholdmarks a notice as sent and removes every pending threshold at or below it, so superseded notices are dropped.claimSteeringThresholdreserves a notice for one request and returns a handle that resolves once:acknowledge()on success,release()on failure.Request lifecycle
body-handler.js: when it adds a notice to a request, it now reserves it (req.awfSteeringDelivery = claim(...)) instead of marking it sent.upstream-response.js: when the upstream response is 2xx and finishes, it callsacknowledge().proxy-request.js: it callsrelease()when:The response close comes after the response-end handlers, so the release does nothing on success.
Review notes
docs/api-proxy-sidecar.mdis updated to describe the new rules.