fix(azure): 401 fallback to anonymous for read-only requests

[MementoRC]

Summary

PR #165 enabled anonymous access to public Azure DevOps projects when AZURE_DEVOPS_TOKEN is unset. However, live testing against build 1517588 exposed a remaining gap: when a stale or expired PAT is present in the environment, the client still attaches BasicAuth and Azure returns 401 ("Personal Access Token has expired") before the anonymous code path can engage.

This patch makes GET requests robust to stale credentials by transparently retrying without auth on 401.

Gap Left by PR #165

The anonymous path in #165 is gated on token is None. If AZURE_DEVOPS_TOKEN is set (even to an expired value), _auth() returns a BasicAuth object and the request is sent with credentials. Azure then rejects it at the auth layer — the public-project anonymous path is never tried.

Build 1517588 returned 401 from the merged code precisely because an expired PAT was still set in the env.

Fix

In AzureClient.get(), after a first-attempt 401 with auth attached:

1. Release the 401 response.
2. Log a warning.
3. Retry the GET without the auth header.
4. Return the second response (success for public projects; 401 for private).

POST and PATCH are unchanged — writes require valid auth.

Behavior Table

Scenario	Behaviour
GET with auth -> 200	Returned directly, no retry
GET with auth -> 401 (expired/stale PAT)	Retried anonymously; second response returned
GET without auth -> 401	Returned directly (already anonymous, no retry)
POST/PATCH -> 401	Propagated unchanged (writes require valid auth)

Test Coverage

New class TestAzureClient401Fallback in tests/unit/azure/test_azure_client.py covers all four paths:

• test_get_retries_without_auth_on_401_when_token_present — verifies 2 calls, release awaited, second call has no auth kwarg, returns 200 response
• test_get_does_not_retry_on_401_when_anonymous — verifies only 1 call when token is None
• test_get_returns_first_response_on_200 — verifies no retry when first response is 200
• test_post_does_not_retry_on_401 — verifies POST 401 propagated, GET never called

Files Changed

• src/mcp_server_git/azure/client.py — GET retry logic (6 lines added)
• tests/unit/azure/test_azure_client.py — TestAzureClient401Fallback class (4 tests)

[github-actions]

Claude finished @MementoRC's task —— View job

---

PR Review Complete ✅

Todo List

• Read and analyze changed files
• Review implementation logic and code quality
• Analyze test coverage and test quality
• Check for potential bugs and edge cases
• Review security and performance considerations
• Provide comprehensive feedback

Review Feedback

This is a well-crafted fix for a real-world issue with expired Azure DevOps PATs. The implementation is clean, focused, and includes excellent test coverage.

✅ Strengths

Implementation Quality:

• Clean, minimal change that addresses the specific gap left by PR feat(azure): anonymous access to public Azure DevOps projects #165
• Proper resource cleanup with await response.release() before retry - crucial for preventing memory leaks
• Clear logging that helps with debugging expired token scenarios
• Follows existing code patterns and style consistently

Logic & Design:

• Smart conditional retry logic - only retries GET requests with auth that fail with 401
• Preserves existing behavior for all other scenarios (200 responses, anonymous requests, POST/PATCH)
• Explicit auth parameter handling (auth=None vs omitting auth kwarg) shows good understanding of aiohttp behavior

Test Coverage:

• Comprehensive test suite covering all four behavioral paths from the behavior table
• Tests verify the exact behavior described (2 calls vs 1 call, auth kwarg presence/absence)
• Good use of mocks to isolate the retry logic without external dependencies
• Test names clearly describe the scenario being tested

🔍 Technical Review

Security Considerations:

• ✅ No security concerns identified
• The retry only happens on 401 (authentication failure), not other errors
• Expired credentials are properly handled without exposure
• Write operations (POST/PATCH) still require valid authentication

Performance Considerations:

• ✅ Minimal performance impact
• Only adds one additional request in the specific case of expired tokens
• Resource cleanup properly implemented
• No unnecessary retries (only on 401 with auth present)

Error Handling:

• ✅ Proper async resource management with await response.release()
• Clear warning message for debugging
• Maintains original error responses when appropriate

🎯 Specific Code Review

src/mcp_server_git/azure/client.py:75-83:
The retry logic is well-implemented:

if response.status == 401:
    await response.release()  # ✅ Proper cleanup
    logger.warning("Azure 401 with auth — retrying anonymously for public project access")
    response = await self.session.get(url, headers=headers, **kwargs)  # ✅ No auth kwarg

Test Quality (test_azure_client.py:156-238):

• test_get_retries_without_auth_on_401_when_token_present - ✅ Verifies core functionality
• test_get_does_not_retry_on_401_when_anonymous - ✅ Prevents infinite retry loops
• test_get_returns_first_response_on_200 - ✅ Ensures no unnecessary retries
• test_post_does_not_retry_on_401 - ✅ Confirms write operations unchanged

🏆 Overall Assessment

This PR demonstrates excellent software engineering practices:

• Problem-focused: Solves the specific issue without over-engineering
• Backwards compatible: No breaking changes to existing functionality
• Well-tested: Comprehensive test coverage with clear test cases
• Production-ready: Proper error handling and resource management

The implementation elegantly handles the edge case where expired tokens prevent fallback to anonymous access, which is exactly what was needed after PR #165.

Recommendation: ✅ Approve - This is ready to merge.

---