From 22fc592ffa08abb2b9a5c212c6e074159fa171d5 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Fri, 1 May 2026 15:22:46 -0700 Subject: [PATCH 1/3] feat(keycardai-oauth)!: pass error context through AccessContext.access(), rename to get_resource_error ResourceAccessError is now populated with resource, error_type ('global_error' | 'resource_error' | 'missing_token'), available_resources, and error_details at the three throw sites in AccessContext.access(). The constructor already accepted these args; this fills the call sites so middleware can surface which resource failed and why. Also rename get_resource_errors -> get_resource_error. Plural method name with a singular return shape was misleading. Companion to keycardai/typescript-sdk#19 (TS commit 317a38b). Addresses review feedback from @jerriclynsjohn. BREAKING CHANGE: AccessContext.get_resource_errors renamed to AccessContext.get_resource_error. --- .../mcp/examples/delegated_access/main.py | 4 +- .../tests/integration/test_grant_decorator.py | 4 +- .../keycardai/oauth/server/access_context.py | 20 ++++- .../oauth/server/test_access_context.py | 80 +++++++++++++++++++ 4 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 packages/oauth/tests/keycardai/oauth/server/test_access_context.py diff --git a/packages/mcp/examples/delegated_access/main.py b/packages/mcp/examples/delegated_access/main.py index a85dbfb..081a3e1 100644 --- a/packages/mcp/examples/delegated_access/main.py +++ b/packages/mcp/examples/delegated_access/main.py @@ -102,7 +102,7 @@ async def list_github_repos(access_ctx: AccessContext, ctx: Context, per_page: i Demonstrates: - Resource-specific error checking with has_resource_error() - - Getting resource-specific errors with get_resource_errors() + - Getting resource-specific errors with get_resource_error() - Parameterized API calls with AccessContext as first parameter Args: @@ -115,7 +115,7 @@ async def list_github_repos(access_ctx: AccessContext, ctx: Context, per_page: i """ # Check for resource-specific error (alternative to has_errors()) if access_ctx.has_resource_error("https://api.github.com"): - resource_errors = access_ctx.get_resource_errors("https://api.github.com") + resource_errors = access_ctx.get_resource_error("https://api.github.com") return { "message": "Token exchange failed for GitHub API", "details": resource_errors, diff --git a/packages/mcp/tests/integration/test_grant_decorator.py b/packages/mcp/tests/integration/test_grant_decorator.py index e5b605c..91b5de6 100644 --- a/packages/mcp/tests/integration/test_grant_decorator.py +++ b/packages/mcp/tests/integration/test_grant_decorator.py @@ -139,7 +139,7 @@ async def test_grant_decorator_token_exchange_failure_with_injected_client(self, def test_function(access_ctx: AccessContext, ctx: Context, user_id: str): # Check if there's a resource error if access_ctx.has_resource_error("https://api.example.com"): - error = access_ctx.get_resource_errors("https://api.example.com") + error = access_ctx.get_resource_error("https://api.example.com") return {"error": error["message"], "isError": True} return {"error": "No error", "isError": False, "access_ctx": access_ctx} @@ -287,7 +287,7 @@ def test_access_context_error_states(self): }) assert access_context.has_resource_error("https://api1.com") assert access_context.get_status() == "partial_error" - assert access_context.get_resource_errors("https://api1.com")["message"] == "Resource failed" + assert access_context.get_resource_error("https://api1.com")["message"] == "Resource failed" def test_access_context_partial_success(self): """Test AccessContext with partial success scenario.""" diff --git a/packages/oauth/src/keycardai/oauth/server/access_context.py b/packages/oauth/src/keycardai/oauth/server/access_context.py index 00e23f0..3549b8e 100644 --- a/packages/oauth/src/keycardai/oauth/server/access_context.py +++ b/packages/oauth/src/keycardai/oauth/server/access_context.py @@ -62,7 +62,7 @@ def get_error(self) -> dict[str, str] | None: """Get global error if any.""" return self._error - def get_resource_errors(self, resource: str) -> dict[str, str] | None: + def get_resource_error(self, resource: str) -> dict[str, str] | None: """Get error for a specific resource.""" return self._resource_errors.get(resource) @@ -96,12 +96,24 @@ def access(self, resource: str) -> TokenResponse: ResourceAccessError: If resource was not granted or has an error """ if self.has_error(): - raise ResourceAccessError() + raise ResourceAccessError( + resource=resource, + error_type="global_error", + error_details=self.get_error(), + ) if self.has_resource_error(resource): - raise ResourceAccessError() + raise ResourceAccessError( + resource=resource, + error_type="resource_error", + error_details=self.get_resource_error(resource), + ) if resource not in self._access_tokens: - raise ResourceAccessError() + raise ResourceAccessError( + resource=resource, + error_type="missing_token", + available_resources=list(self._access_tokens.keys()), + ) return self._access_tokens[resource] diff --git a/packages/oauth/tests/keycardai/oauth/server/test_access_context.py b/packages/oauth/tests/keycardai/oauth/server/test_access_context.py new file mode 100644 index 0000000..46fca6a --- /dev/null +++ b/packages/oauth/tests/keycardai/oauth/server/test_access_context.py @@ -0,0 +1,80 @@ +"""Unit tests for AccessContext. + +Covers the rich-error context surfaced through `access()` and the +get_resource_error getter. +""" + +import pytest + +from keycardai.oauth.server.access_context import AccessContext +from keycardai.oauth.server.exceptions import ResourceAccessError +from keycardai.oauth.types.models import TokenResponse + + +def _token() -> TokenResponse: + return TokenResponse(access_token="tok", token_type="bearer") + + +def test_access_returns_token_when_present(): + ctx = AccessContext() + ctx.set_token("https://api.example.com", _token()) + assert ctx.access("https://api.example.com").access_token == "tok" + + +def test_access_carries_missing_token_context(): + ctx = AccessContext() + ctx.set_token("https://api.example.com", _token()) + + with pytest.raises(ResourceAccessError) as exc_info: + ctx.access("https://missing.example.com") + + err = exc_info.value + assert err.details["error_type"] == "missing_token" + assert err.details["requested_resource"] == "https://missing.example.com" + assert err.details["available_resources"] == ["https://api.example.com"] + + +def test_access_carries_resource_error_context(): + ctx = AccessContext() + detail = {"message": "denied by AS", "code": "access_denied"} + ctx.set_resource_error("https://api.example.com", detail) + + with pytest.raises(ResourceAccessError) as exc_info: + ctx.access("https://api.example.com") + + err = exc_info.value + assert err.details["error_type"] == "resource_error" + assert err.details["requested_resource"] == "https://api.example.com" + assert err.details["error_details"] == detail + + +def test_access_carries_global_error_context(): + ctx = AccessContext() + detail = {"message": "token exchange failed"} + ctx.set_error(detail) + + with pytest.raises(ResourceAccessError) as exc_info: + ctx.access("https://api.example.com") + + err = exc_info.value + assert err.details["error_type"] == "global_error" + assert err.details["requested_resource"] == "https://api.example.com" + assert err.details["error_details"] == detail + + +def test_get_resource_error_returns_stored_detail_or_none(): + ctx = AccessContext() + ctx.set_resource_error("https://api.example.com", {"message": "transient"}) + assert ctx.get_resource_error("https://api.example.com") == {"message": "transient"} + assert ctx.get_resource_error("https://other.example.com") is None + + +def test_status_transitions(): + ctx = AccessContext() + assert ctx.get_status() == "success" + + ctx.set_resource_error("https://api.example.com", {"message": "boom"}) + assert ctx.get_status() == "partial_error" + + ctx.set_error({"message": "global boom"}) + assert ctx.get_status() == "error" From e5a2025cac5de40bc844ef3056c5ec0602832e45 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Fri, 1 May 2026 15:22:46 -0700 Subject: [PATCH 2/3] refactor(keycardai-fastmcp)!: rename AccessContext.get_resource_error to match keycardai-oauth Mirrors the keycardai-oauth rename in the fastmcp provider's parallel AccessContext implementation. Keeps the two surfaces aligned for users who consume both directly. BREAKING CHANGE: AccessContext.get_resource_errors renamed to AccessContext.get_resource_error. --- packages/fastmcp/examples/delegated_access/main.py | 4 ++-- packages/fastmcp/src/keycardai/fastmcp/provider.py | 4 ++-- packages/fastmcp/tests/integration/test_grant_decorator.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/fastmcp/examples/delegated_access/main.py b/packages/fastmcp/examples/delegated_access/main.py index 16ecbae..4f6b3cd 100644 --- a/packages/fastmcp/examples/delegated_access/main.py +++ b/packages/fastmcp/examples/delegated_access/main.py @@ -100,7 +100,7 @@ async def list_github_repos(ctx: Context, per_page: int = 5) -> dict: Demonstrates: - Resource-specific error checking with has_resource_error() - - Getting resource-specific errors with get_resource_errors() + - Getting resource-specific errors with get_resource_error() - Parameterized API calls Args: @@ -114,7 +114,7 @@ async def list_github_repos(ctx: Context, per_page: int = 5) -> dict: # Check for resource-specific error (alternative to has_errors()) if access_context.has_resource_error("https://api.github.com"): - resource_errors = access_context.get_resource_errors("https://api.github.com") + resource_errors = access_context.get_resource_error("https://api.github.com") return { "message": "Token exchange failed for GitHub API", "details": resource_errors, diff --git a/packages/fastmcp/src/keycardai/fastmcp/provider.py b/packages/fastmcp/src/keycardai/fastmcp/provider.py index 95f8f43..8be867c 100644 --- a/packages/fastmcp/src/keycardai/fastmcp/provider.py +++ b/packages/fastmcp/src/keycardai/fastmcp/provider.py @@ -233,7 +233,7 @@ def get_error(self) -> dict[str, str] | None: """Get global error if any.""" return self._error - def get_resource_errors(self, resource: str) -> dict[str, str] | None: + def get_resource_error(self, resource: str) -> dict[str, str] | None: """Get error for a specific resource.""" return self._resource_errors.get(resource) @@ -279,7 +279,7 @@ def access(self, resource: str) -> TokenResponse: raise ResourceAccessError( resource=resource, error_type="resource_error", - error_details=self.get_resource_errors(resource) + error_details=self.get_resource_error(resource) ) # Check if token exists diff --git a/packages/fastmcp/tests/integration/test_grant_decorator.py b/packages/fastmcp/tests/integration/test_grant_decorator.py index bead14f..07cda43 100644 --- a/packages/fastmcp/tests/integration/test_grant_decorator.py +++ b/packages/fastmcp/tests/integration/test_grant_decorator.py @@ -129,7 +129,7 @@ async def test_function(ctx: Context, user_id: str): # Check if there's a resource error access_ctx = await ctx.get_state("keycardai") if access_ctx.has_resource_error("https://api.example.com"): - error = access_ctx.get_resource_errors("https://api.example.com") + error = access_ctx.get_resource_error("https://api.example.com") return {"error": error["message"], "isError": True} return {"error": "No error", "isError": False, "access_ctx": access_ctx} @@ -292,7 +292,7 @@ def test_access_context_error_states(self): }) assert access_context.has_resource_error("https://api1.com") assert access_context.get_status() == "partial_error" - assert access_context.get_resource_errors("https://api1.com")["message"] == "Resource failed" + assert access_context.get_resource_error("https://api1.com")["message"] == "Resource failed" def test_access_context_partial_success(self): """Test AccessContext with partial success scenario.""" From 43fcc10032005a59e488faf975e1f90de6ad05c5 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Wed, 6 May 2026 18:16:33 -0700 Subject: [PATCH 3/3] fix(keycardai-oauth): address review issues on PR #114 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four changes based on code review of the access() rename / rich-error PR: 1. available_resources semantics: for global_error and resource_error, details["available_resources"] is now None instead of []. The empty-list default implied "zero resources available" rather than "field not applicable". Only the missing_token path should carry a list. 2. README sweep for get_resource_error rename: four docs files still called get_resource_errors (plural) after the rename — added to PR #114 scope to prevent AttributeError for any user following those examples. 3. Test assertion fixes: resource_error and global_error tests now assert available_resources is None. missing_token test uses set comparison to avoid fragility when token registration order varies. 4. fastmcp unit tests: AccessContext.access() in provider.py is an independent implementation with the same three error paths. Add tests/test_access_context.py mirroring the oauth test file. --- .../examples/delegated_access/README.md | 2 +- packages/fastmcp/tests/test_access_context.py | 84 +++++++++++++++++++ packages/mcp-fastmcp/README.md | 2 +- packages/mcp/README.md | 2 +- .../mcp/examples/delegated_access/README.md | 2 +- .../src/keycardai/oauth/server/exceptions.py | 2 +- .../oauth/server/test_access_context.py | 4 +- 7 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 packages/fastmcp/tests/test_access_context.py diff --git a/packages/fastmcp/examples/delegated_access/README.md b/packages/fastmcp/examples/delegated_access/README.md index 1cdd50e..3291bc3 100644 --- a/packages/fastmcp/examples/delegated_access/README.md +++ b/packages/fastmcp/examples/delegated_access/README.md @@ -163,7 +163,7 @@ The example demonstrates comprehensive error handling patterns: | `has_errors()` | Check for any errors (global or resource-specific) | | `get_errors()` | Get all error details as a dictionary | | `has_resource_error(url)` | Check for errors on a specific resource | -| `get_resource_errors(url)` | Get errors for a specific resource | +| `get_resource_error(url)` | Get errors for a specific resource | | `has_error()` | Check for global errors only | | `get_error()` | Get global error details | diff --git a/packages/fastmcp/tests/test_access_context.py b/packages/fastmcp/tests/test_access_context.py new file mode 100644 index 0000000..d0011bb --- /dev/null +++ b/packages/fastmcp/tests/test_access_context.py @@ -0,0 +1,84 @@ +"""Unit tests for the fastmcp provider AccessContext. + +The fastmcp AccessContext.access() is an independent implementation in +provider.py with the same three error paths as the oauth AccessContext. +These tests mirror packages/oauth/tests/.../test_access_context.py and +verify the rich-error contract holds for the fastmcp provider too. +""" + +import pytest + +from keycardai.fastmcp.provider import AccessContext +from keycardai.mcp.server.exceptions import ResourceAccessError +from keycardai.oauth.types.models import TokenResponse + + +def _token() -> TokenResponse: + return TokenResponse(access_token="tok", token_type="bearer") + + +def test_access_returns_token_when_present(): + ctx = AccessContext() + ctx.set_token("https://api.example.com", _token()) + assert ctx.access("https://api.example.com").access_token == "tok" + + +def test_access_carries_missing_token_context(): + ctx = AccessContext() + ctx.set_token("https://api.example.com", _token()) + + with pytest.raises(ResourceAccessError) as exc_info: + ctx.access("https://missing.example.com") + + err = exc_info.value + assert err.details["error_type"] == "missing_token" + assert err.details["requested_resource"] == "https://missing.example.com" + assert set(err.details["available_resources"]) == {"https://api.example.com"} + + +def test_access_carries_resource_error_context(): + ctx = AccessContext() + detail = {"message": "denied by AS", "code": "access_denied"} + ctx.set_resource_error("https://api.example.com", detail) + + with pytest.raises(ResourceAccessError) as exc_info: + ctx.access("https://api.example.com") + + err = exc_info.value + assert err.details["error_type"] == "resource_error" + assert err.details["requested_resource"] == "https://api.example.com" + assert err.details["error_details"] == detail + assert err.details["available_resources"] is None + + +def test_access_carries_global_error_context(): + ctx = AccessContext() + detail = {"message": "token exchange failed"} + ctx.set_error(detail) + + with pytest.raises(ResourceAccessError) as exc_info: + ctx.access("https://api.example.com") + + err = exc_info.value + assert err.details["error_type"] == "global_error" + assert err.details["requested_resource"] == "https://api.example.com" + assert err.details["error_details"] == detail + assert err.details["available_resources"] is None + + +def test_get_resource_error_returns_stored_detail_or_none(): + ctx = AccessContext() + ctx.set_resource_error("https://api.example.com", {"message": "transient"}) + assert ctx.get_resource_error("https://api.example.com") == {"message": "transient"} + assert ctx.get_resource_error("https://other.example.com") is None + + +def test_status_transitions(): + ctx = AccessContext() + assert ctx.get_status() == "success" + + ctx.set_resource_error("https://api.example.com", {"message": "boom"}) + assert ctx.get_status() == "partial_error" + + ctx.set_error({"message": "global boom"}) + assert ctx.get_status() == "error" diff --git a/packages/mcp-fastmcp/README.md b/packages/mcp-fastmcp/README.md index a6f3baf..b29a8b0 100644 --- a/packages/mcp-fastmcp/README.md +++ b/packages/mcp-fastmcp/README.md @@ -222,7 +222,7 @@ if access_context.has_error(): # Check for specific resource errors if access_context.has_resource_error("https://api.example.com"): - resource_error = access_context.get_resource_errors("https://api.example.com") + resource_error = access_context.get_resource_error("https://api.example.com") # Get all errors (global + resource-specific) all_errors = access_context.get_errors() diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 44f69b5..4585577 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -620,7 +620,7 @@ def multi_resource_tool(access_ctx: AccessContext, ctx: Context): # Handle failed resources for resource in access_ctx.get_failed_resources(): - error = access_ctx.get_resource_errors(resource) + error = access_ctx.get_resource_error(resource) results[resource] = {"error": error["message"]} return results diff --git a/packages/mcp/examples/delegated_access/README.md b/packages/mcp/examples/delegated_access/README.md index 4b5281c..e04ff55 100644 --- a/packages/mcp/examples/delegated_access/README.md +++ b/packages/mcp/examples/delegated_access/README.md @@ -193,7 +193,7 @@ The example demonstrates comprehensive error handling patterns: | `has_errors()` | Check for any errors (global or resource-specific) | | `get_errors()` | Get all error details as a dictionary | | `has_resource_error(url)` | Check for errors on a specific resource | -| `get_resource_errors(url)` | Get errors for a specific resource | +| `get_resource_error(url)` | Get errors for a specific resource | | `has_error()` | Check for global errors only | | `get_error()` | Get global error details | diff --git a/packages/oauth/src/keycardai/oauth/server/exceptions.py b/packages/oauth/src/keycardai/oauth/server/exceptions.py index 8599e80..91bcde8 100644 --- a/packages/oauth/src/keycardai/oauth/server/exceptions.py +++ b/packages/oauth/src/keycardai/oauth/server/exceptions.py @@ -307,7 +307,7 @@ def __init__( details = { "requested_resource": resource or "unknown", "error_type": error_type or "unknown", - "available_resources": available_resources or [], + "available_resources": available_resources if error_type == "missing_token" else None, "error_details": error_details or {}, "solution": ( "Fix authentication issues before accessing resources" diff --git a/packages/oauth/tests/keycardai/oauth/server/test_access_context.py b/packages/oauth/tests/keycardai/oauth/server/test_access_context.py index 46fca6a..81f2383 100644 --- a/packages/oauth/tests/keycardai/oauth/server/test_access_context.py +++ b/packages/oauth/tests/keycardai/oauth/server/test_access_context.py @@ -31,7 +31,7 @@ def test_access_carries_missing_token_context(): err = exc_info.value assert err.details["error_type"] == "missing_token" assert err.details["requested_resource"] == "https://missing.example.com" - assert err.details["available_resources"] == ["https://api.example.com"] + assert set(err.details["available_resources"]) == {"https://api.example.com"} def test_access_carries_resource_error_context(): @@ -46,6 +46,7 @@ def test_access_carries_resource_error_context(): assert err.details["error_type"] == "resource_error" assert err.details["requested_resource"] == "https://api.example.com" assert err.details["error_details"] == detail + assert err.details["available_resources"] is None def test_access_carries_global_error_context(): @@ -60,6 +61,7 @@ def test_access_carries_global_error_context(): assert err.details["error_type"] == "global_error" assert err.details["requested_resource"] == "https://api.example.com" assert err.details["error_details"] == detail + assert err.details["available_resources"] is None def test_get_resource_error_returns_stored_detail_or_none():