test(auth): align copilot-remove test with borrowed-credential policy (#31416)#31946
Merged
Merged
Conversation
…#31416) PR #31416 (avoid persisting borrowed credential secrets) added sanitize_borrowed_credential_payload, which strips access_token from any auth.json pool entry whose (provider, source) isn't in the _PERSISTABLE_PROVIDER_SOURCES allowlist. (copilot, gh_cli) is borrowed (not in the allowlist), so the test fixture's pre-seeded access_token now gets stripped at load_pool() time, leaving the pool empty. resolve_target('1') then fails with 'No credential #1. Provider: copilot.' Fix: align the test with the new contract. At runtime, copilot tokens are hydrated by resolve_copilot_token() — mock that path so the pool gets an entry the test can remove. The behavior under test (suppression of gh_cli + env variants on remove) is unchanged. CI repro on origin/main HEAD; reproduced locally with stock checkout.
Contributor
🔎 Lint report:
|
This was referenced May 25, 2026
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.
Summary
tests/hermes_cli/test_auth_commands.py::test_auth_remove_copilot_suppresses_all_variantsnow passes on currentmain. Pre-existing failure introduced by PR #31416 (avoid persisting borrowed credential secrets, merged May 25 2026).Root cause
PR #31416 added
sanitize_borrowed_credential_payload()toagent/credential_pool.py. The new policy: any(provider, source)tuple NOT in_PERSISTABLE_PROVIDER_SOURCESis "borrowed" and gets itsaccess_tokenstripped at the disk boundary.(copilot, gh_cli)is correctly classified as borrowed —gh auth tokenis the canonical source of truth, notauth.json.The test fixture pre-seeded an
access_tokendirectly into acopilot/gh_clipool entry and expectedload_poolto surface it. Under the new policy that token gets stripped,pool.entries()returns empty, andresolve_target("1")correctly says "No credential #1."The behavior under test (suppression of
gh_cli+ allenv:*variants on remove) is unchanged. Only the fixture's seeding mechanism needs updating to match the new contract.Changes
tests/hermes_cli/test_auth_commands.py—test_auth_remove_copilot_suppresses_all_variantsnow seeds the pool via mockedresolve_copilot_token()/get_copilot_api_token()(the actual runtime hydration path), matching production behavior.Validation
test_auth_remove_copilot_suppresses_all_variantsSystemExit: No credential #1)test_auth_commands.pyReproduced the failure on stock
origin/mainHEAD with no other patches applied. CI run from PR #31929 surfaced it.Why fix-it-first PR
Per
references/green-ci-policy.md: when a pre-existing failure blocks an unrelated PR, the right move is a separate fix-it PR. This test failure was blocking my/resumebracket-stripping salvage (#31929); landing this first lets that PR rebase onto a clean main.Infographic
https://v3b.fal.media/files/b/0a9b99e7/UbnXaIc3cO0K5-hC7YINq_CtkbmY2r.png