Skip to content

test(agent): make Claude Code credential tests hermetic on macOS with Keychain entries - #43344

Closed
bionicbutterfly13 wants to merge 1 commit into
NousResearch:mainfrom
bionicbutterfly13:test/anthropic-adapter-keychain-hermetic
Closed

test(agent): make Claude Code credential tests hermetic on macOS with Keychain entries#43344
bionicbutterfly13 wants to merge 1 commit into
NousResearch:mainfrom
bionicbutterfly13:test/anthropic-adapter-keychain-hermetic

Conversation

@bionicbutterfly13

Copy link
Copy Markdown
Contributor

What does this PR do?

Makes the Claude Code credential-resolution tests in tests/agent/test_anthropic_adapter.py hermetic on macOS machines that have real Claude Code Keychain credentials.

Today, 5 tests fail out of the box on any contributor Mac where Claude Code is (or has been) logged in:

FAILED tests/agent/test_anthropic_adapter.py::TestResolveAnthropicToken::test_falls_back_to_claude_code_credentials
FAILED tests/agent/test_anthropic_adapter.py::TestResolveAnthropicToken::test_prefers_refreshable_claude_code_credentials_over_static_anthropic_token
FAILED tests/agent/test_anthropic_adapter.py::TestRunOauthSetupToken::test_returns_token_from_credential_files
FAILED tests/agent/test_anthropic_adapter.py::TestRunOauthSetupToken::test_returns_token_from_env_var
FAILED tests/agent/test_anthropic_adapter.py::TestRunOauthSetupToken::test_returns_none_when_no_creds_found

Root cause: these tests fake ~/.claude/.credentials.json by monkeypatching agent.anthropic_adapter.Path.home to a tmp_path, but read_claude_code_credentials() (agent/anthropic_adapter.py) checks the macOS Keychain first via _read_claude_code_credentials_from_keychain() — which the tests never mocked. The developer's real (possibly expired) Keychain entry shadows the fake credentials file, so resolution returns the wrong token (or None after a failed live refresh) and the assertions fail, e.g. AssertionError: assert None == 'cc-auto-token'.

The remaining tests in TestResolveAnthropicToken, TestResolveWithRefresh, and TestRunOauthSetupToken currently pass only by luck of the local Keychain state (token expiry, refreshability), so the whole group is covered, not just the 5 currently-red tests.

Fix: reuse the exact pattern the file already established — TestReadClaudeCodeCredentials has an autouse no_keychain fixture that monkeypatches _read_claude_code_credentials_from_keychain to return None. This PR adds the same fixture to the three affected classes. No production code changes.

The other test files that touch this code path were audited and are already hermetic: they mock resolve_anthropic_token / read_claude_code_credentials at the function level, and tests/agent/test_anthropic_keychain.py mocks subprocess.run directly.

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tests/agent/test_anthropic_adapter.py: add an autouse no_keychain fixture (monkeypatch agent.anthropic_adapter._read_claude_code_credentials_from_keychainNone) to TestResolveAnthropicToken, TestResolveWithRefresh, and TestRunOauthSetupToken, mirroring the existing fixture on TestReadClaudeCodeCredentials. 21 insertions, no production code touched.

How to Test

  1. On a Mac where Claude Code is logged in (Keychain has a Claude Code-credentials entry), check out main and run pytest tests/agent/test_anthropic_adapter.py -q → 5 failures as listed above.
  2. Check out this branch and run pytest tests/agent/test_anthropic_adapter.py -q → all 159 tests pass.
  3. On Linux/CI (no Keychain), the suite passes both before and after — this change only removes host-state dependence.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (with Claude Code logged in — the failing configuration)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Before (clean upstream/main, macOS 15, Claude Code logged in):

======================== 5 failed, 154 passed in 8.06s =========================

After (this branch, same machine):

============================= 159 passed in 6.14s ==============================

… Keychain entries

Several tests in tests/agent/test_anthropic_adapter.py fail on any
contributor Mac where Claude Code is logged in:

  TestResolveAnthropicToken::test_falls_back_to_claude_code_credentials
  TestResolveAnthropicToken::test_prefers_refreshable_claude_code_credentials_over_static_anthropic_token
  TestRunOauthSetupToken::test_returns_token_from_credential_files
  TestRunOauthSetupToken::test_returns_token_from_env_var
  TestRunOauthSetupToken::test_returns_none_when_no_creds_found

Root cause: these tests fake ~/.claude/.credentials.json by
monkeypatching agent.anthropic_adapter.Path.home to a tmp_path, but
read_claude_code_credentials() consults the macOS Keychain FIRST via
_read_claude_code_credentials_from_keychain(), which the tests did not
mock. Real (possibly expired) Keychain credentials shadow the fake
file, so resolution returns the wrong token or None and the assertions
fail. The remaining tests in TestResolveAnthropicToken,
TestResolveWithRefresh, and TestRunOauthSetupToken only pass by luck
of the developer's local Keychain state.

Fix: add the same autouse no_keychain fixture that
TestReadClaudeCodeCredentials already uses (monkeypatch the keychain
reader to return None) to TestResolveAnthropicToken,
TestResolveWithRefresh, and TestRunOauthSetupToken. No production
code changes.

Verified on macOS 15 with Claude Code logged in:
before 5 failed / 154 passed, after 159 passed.
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/anthropic Anthropic native Messages API labels Jun 10, 2026
@NarahariRaghava

Copy link
Copy Markdown

Hi @bionicbutterfly13 - I independently ran into this exact bug today and arrived at the same fix (autouse no_keychain fixture on TestResolveAnthropicToken, TestResolveWithRefresh, and TestRunOauthSetupToken, mocking _read_claude_code_credentials_from_keychain to None).

I verified it on a real Mac with Claude Code installed: before the fix, 5-6 of 160 tests in tests/agent/test_anthropic_adapter.py fail non-deterministically (either real Keychain tokens leaking into assertions, or TypeError: the JSON object must be str, bytes or bytearray, not MagicMock when an unrelated subprocess.run mock intercepts the Keychain's own subprocess call). After the fix, all 160 tests pass consistently via scripts/run_tests.sh.

Just wanted to add a second independent confirmation that this is a real bug and the fix is correct - might help this get unstuck. Closed my duplicate (#49796) in favor of this one.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tightening this macOS-specific test isolation. Current main still calls _read_claude_code_credentials_from_keychain() independently of the monkeypatched Path.home() (agent/anthropic_adapter.py:979-995), while the three target classes have no equivalent fixture (tests/agent/test_anthropic_adapter.py:295, :613, :660). The proposed fixtures mirror the existing established pattern at tests/agent/test_anthropic_adapter.py:227-233 and do not alter production behavior.

No problems found in the PR diff. GitHub reports the PR as mergeable, so this should be a mechanical salvage.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @bionicbutterfly13 — closing as resolved on main: PR #74517 landed a suite-wide autouse Keychain guard (_neutralize_macos_keychain_creds, from #35464 by @y0shua1ee) that covers your three per-class fixtures as a strict subset. You were the earliest PR targeting the anthropic-adapter Keychain class (June 10) — credited here. Verified post-merge: the adapter tests can no longer read a developer's real Keychain entries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have provider/anthropic Anthropic native Messages API sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants