fix(credential_pool): check copilot suppression before token exchange (salvage #76341) - #77561
Merged
kshitijk4poor merged 3 commits intoAug 3, 2026
Merged
Conversation
The copilot branch of _seed_from_singletons ran the suppression gate _after get_copilot_api_token(), which retries the network exchange 3x with backoff (~13s worst case). A source the user already suppressed (hermes auth remove copilot gh_cli) still burned the full exchange dead time on every pool load — model picker open, /model, agent startup — only to have the entry discarded afterwards. Move the _is_suppressed() gate ahead of the network call, matching the early-gate pattern every other singleton branch uses. Suppressed copilot sources now skip the exchange entirely. Measured: model.options payload build drops from ~13s to ~0.2-0.4s for a user with copilot suppressed. Add regression test test_load_pool_skips_exchange_for_suppressed_copilot asserting the exchange is never invoked for a suppressed source.
…ppressed The all-sources suppression gate now runs before resolve_copilot_token(), which shells out to `gh auth token` (~30ms) on every pool load. A user who suppressed every copilot source (hermes auth remove copilot gh_cli suppresses gh_cli + all env variants) still paid the subprocess spawn on every load — model picker open, /model, agent startup. Enumerate the same source space credential_sources._remove_copilot_gh suppresses and bail before any work when all are suppressed. Measured: model.options payload build drops from ~0.46s to ~0.26s cold for an all-suppressed user; resolve_copilot_token() is no longer called at all.
Review fold on the NousResearch#76341 salvage: the substring test ('gh' in source.lower()) classified GH_TOKEN and GITHUB_TOKEN as gh_cli, so a user's env-var-specific suppression was silently bypassed (and suppressing gh_cli silently dropped env tokens). Pre-existing bug on main, but the PR's early gate makes the classification decide whether the exchange runs at all. Match resolve_copilot_token's exact 'gh auth token' sentinel instead. Adds 3 regression tests: env-var suppression gates the exchange, gh_cli suppression doesn't swallow env tokens, all-sources suppression skips the resolve subprocess entirely. Also corrects the ~13s comment (actual worst case ~35s: 3x10s timeouts + 4.5s backoff).
kshitijk4poor
enabled auto-merge (rebase)
August 3, 2026 10:23
alt-glitch
added
type/bug
This was referenced Aug 3, 2026
feat: reset-aware primary restore — stay on fallback until the window resets (salvage #67642)
#77631
Merged
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.
Salvages #76341 by @wangyunyou — both commits cherry-picked to preserve authorship, plus one review-fold commit fixing a classification bug the early gate made decisive.
Context — what this fixes, for whom
Anyone with a suppressed copilot source (
hermes auth remove copilot gh_cli): everyload_pool("copilot")— model picker open,/model, agent startup — ran the full token exchange (3 retries x 10s timeouts + 4.5s backoff, ~35s worst case) and THEN discarded the result at the post-exchange suppression check. The PR moves the gate before the network call and adds an all-sources early exit that skips even thegh auth tokensubprocess.Review fold (the salvage's addition)
The per-source gate classifies with
"gh" in source.lower()— which classifiesGH_TOKENandGITHUB_TOKENasgh_cli(substring match). Pre-existing on main, but this PR makes the classification decide whether the exchange runs at all: a user who suppressedenv:GH_TOKENwould have the suppression silently bypassed, and one who suppressedgh_cliwould silently lose env-sourced tokens. Fixed to matchresolve_copilot_token's exact"gh auth token"sentinel, with 3 regression tests (env-var suppression gates the exchange / gh_cli suppression doesn't swallow env tokens / all-sources suppression skips the resolve subprocess). Also corrected the "~13s" worst-case comment (actual: ~35s from copilot_auth's constants).Verification
tests/agent/test_credential_pool.py -k "copilot or suppress": 5 passed (PR's regression test + 3 new + 1 adjacent)_seed_from_singletons, no overlap with the other two.Closes #76341 (superseded by this salvage — original author credited via cherry-pick authorship).