Keep the Copilot run alive on rate-limit errors eligible for auto-switch - #714
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request updates the Copilot Go provider’s session event handling so that session.error events representing rate-limit errors that the runtime will auto-recover from (via EligibleForAutoSwitch) do not terminate the run loop. This aligns provider behavior with the Copilot SDK/runtime contract by allowing the session to continue through subsequent auto-switch events and complete normally on session.idle.
Changes:
- Treat
SessionErrorData.EligibleForAutoSwitch == trueas a non-terminal session notification (no error returned, run loop continues). - Add an end-to-end fake-runtime test asserting the run survives an eligible rate-limit error and still emits the assistant message.
- Add a test asserting a rate-limit error without the eligibility flag remains terminal and returns the session error.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| provider/copilotprovider/copilot.go | Makes eligible rate-limit session.error non-terminal so the run loop can process auto-switch and reach session.idle. |
| provider/copilotprovider/copilot_test.go | Adds regression tests covering eligible vs non-eligible rate-limit errors through the full run loop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
21294c1 to
38bdd70
Compare
This comment has been minimized.
This comment has been minimized.
responseUpdateForSessionEvent mapped every session.error to a terminal error, aborting the run. The SDK sets SessionErrorData.EligibleForAutoSwitch only on errorType "rate_limit" to signal that the runtime will follow the error with an auto_mode_switch.requested event and keep the session alive. When that flag is set, surface the error as a non-terminal notification so the run loop keeps pumping the auto-switch events and the eventual idle completion instead of terminating. All other session errors keep the existing terminal behavior.
38bdd70 to
da3804b
Compare
Parity Review: No Issues FoundThis PR modifies only an unexported internal function ( The behavioral change is specific to the GitHub Copilot SDK ( Verdict: This PR stays within Go-internal implementation details. Cross-repo parity is preserved. The Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
What
responseUpdateForSessionEventinprovider/copilotprovider/copilot.gomapped every*copilot.SessionErrorDatato a terminal(done=true, error), so the run loop yielded the error and returned, terminating the session.The Copilot SDK sets
SessionErrorData.EligibleForAutoSwitchonly onerrorType: "rate_limit", documented as: whentrue, the runtime will follow this error with anauto_mode_switch.requestedevent and keep the session alive (or silently switch). The follow-upauto_mode_switch.requested/completedevents are already handled by the provider's default (non-terminal) case, and the run naturally completes on the subsequentsession.idle.This change dereferences
EligibleForAutoSwitchin theSessionErrorDatacase: when non-nil andtrue, the error is surfaced as a non-terminal raw notification (done=false, err=nil) so the loop keeps pumping the auto-switch events and the eventual idle completion. Every other session error (nil/false flag, or any non-rate-limit type) keeps the existing terminal behavior. Recovery is confined to exactly the SDK-documented rate-limit auto-switch case.Why
This aligns the Go provider with the runtime contract the SDK exposes via
EligibleForAutoSwitch: a rate-limit that the runtime is about to auto-recover from should not surface to the caller as a fatal run error. Treating these as terminal caused spurious run failures on transient, auto-recoverable rate limits.Tests
Added to the canonical
copilot_test.go, driving the full run loop through the existing fake-runtime harness:TestRun_WithRateLimitEligibleForAutoSwitch_KeepsSessionAlive— asession.errorwitherrorType: "rate_limit"andeligibleForAutoSwitch: true, followed byauto_mode_switch.requested, anassistant.message, andsession.idle. Asserts the run does not fail and emits the assistant message.TestRun_WithRateLimitNotEligibleForAutoSwitch_ReturnsError— a rate-limit error without the flag still terminates with the session error.Both tests fail before the fix and pass after.
go build ./...,go vet ./provider/copilotprovider/..., andgo test ./provider/copilotprovider/...all pass.