fix(keycardai-agents): restore test suite (ACC-236)#100
Merged
Conversation
Closes ACC-236. Surfaced by ACC-234 wait-and-see verification: keycardai-agents tests would not run on a fresh checkout. Three pre-existing breaks fixed: 1. a2a-sdk constraint pinned to <1.0. The unbounded >=0.3.22 resolved to 1.0.x today; a2a-sdk 1.0 moved A2AStarletteApplication out of a2a.server.apps.jsonrpc, which keycardai-agents/server/app.py imports. Pinning is the cheap fix because keycardai-agents is being decomposed and archived in ACC-229..232; the replacement keycardai-a2a package will be written against a2a-sdk 1.x natively. 2. tests/integrations/test_crewai_a2a.py imported from keycardai.agents.integrations.crewai_a2a, which does not exist. The module is keycardai.agents.integrations.crewai. Three import/patch references updated. 3. test_tool_run_with_task_and_inputs asserted "pr_number" was a top-level key in the task dict; the actual contract puts task_inputs under task["inputs"]. Assertion updated to match the implementation. Verified: 85/85 agents tests pass. mcp 560/560, starlette 49/49, oauth 208/208 unaffected.
📦 Release PreviewThis analysis shows the expected release impact: 📈 Expected Version Changes📋 Package Details[
{
"package_name": "keycardai-agents",
"package_dir": "packages/agents",
"has_changes": true,
"current_version": "0.1.1",
"next_version": "0.2.0",
"increment": "MINOR"
}
]📝 Changelog PreviewThis comment was automatically generated by the release preview workflow. |
6 tasks
kamil-keycard
approved these changes
Apr 27, 2026
Larry-Osakwe
added a commit
that referenced
this pull request
Apr 28, 2026
First step of the keycardai-agents decomposition (ACC-229..232). Per the revised KEP, the OAuth PKCE user-login flow is generic OAuth code with no agents-specific concerns and belongs in keycardai-oauth next to the rest of the OAuth client primitives. New module keycardai.oauth.pkce: - PKCEClient orchestrates the full authorization-code-with-PKCE flow: parse the WWW-Authenticate challenge (RFC 9728), fetch protected resource and authorization server metadata (RFC 8414), open the browser at the authorize endpoint, capture the redirect via a local callback server, and exchange the code at the token endpoint. Returns the token endpoint response dict directly. - OAuthCallbackServer is the loopback redirect catcher (RFC 8252) used by PKCEClient; exported separately so callers running their own flow on top of the lower-level PKCEGenerator + build_authorize_url primitives can reuse the callback machinery. - 7 new tests cover header parsing, discovery error paths, the happy-path flow, and confidential vs public client auth on the token endpoint. keycardai-agents changes: - AgentClient now composes PKCEClient instead of carrying its own copy of the auth flow. AgentClient.authenticate(...) is preserved as a thin shim that returns the access_token string and updates the per-service token cache, so existing /invoke retry-on-401 behavior is unchanged. - AgentClient drops ~370 lines of duplicated PKCE/discovery/callback code. - keycardai.agents.client.oauth re-exports OAuthCallbackServer through a module __getattr__ that emits a DeprecationWarning pointing at the new canonical import path. - Stale tests in test_agent_client_oauth.py that exercised AgentClient private methods (_extract_resource_metadata_url, _fetch_resource_metadata, _fetch_authorization_server_metadata) removed; equivalent contracts now live in the keycardai-oauth PKCE test suite. Verified: oauth 215/215 (was 208 + 7 new), agents 81/81 (was 85 - 4 removed implementation tests), mcp 560/560, starlette 49/49, ruff clean. Stacked on #100 (ACC-236 a2a-sdk pin) so the agents test suite can run during validation.
Larry-Osakwe
added a commit
that referenced
this pull request
Apr 28, 2026
…101) * feat(keycardai-oauth): add high-level PKCE flow client (ACC-229) First step of the keycardai-agents decomposition (ACC-229..232). Per the revised KEP, the OAuth PKCE user-login flow is generic OAuth code with no agents-specific concerns and belongs in keycardai-oauth next to the rest of the OAuth client primitives. New module keycardai.oauth.pkce: - PKCEClient orchestrates the full authorization-code-with-PKCE flow: parse the WWW-Authenticate challenge (RFC 9728), fetch protected resource and authorization server metadata (RFC 8414), open the browser at the authorize endpoint, capture the redirect via a local callback server, and exchange the code at the token endpoint. Returns the token endpoint response dict directly. - OAuthCallbackServer is the loopback redirect catcher (RFC 8252) used by PKCEClient; exported separately so callers running their own flow on top of the lower-level PKCEGenerator + build_authorize_url primitives can reuse the callback machinery. - 7 new tests cover header parsing, discovery error paths, the happy-path flow, and confidential vs public client auth on the token endpoint. keycardai-agents changes: - AgentClient now composes PKCEClient instead of carrying its own copy of the auth flow. AgentClient.authenticate(...) is preserved as a thin shim that returns the access_token string and updates the per-service token cache, so existing /invoke retry-on-401 behavior is unchanged. - AgentClient drops ~370 lines of duplicated PKCE/discovery/callback code. - keycardai.agents.client.oauth re-exports OAuthCallbackServer through a module __getattr__ that emits a DeprecationWarning pointing at the new canonical import path. - Stale tests in test_agent_client_oauth.py that exercised AgentClient private methods (_extract_resource_metadata_url, _fetch_resource_metadata, _fetch_authorization_server_metadata) removed; equivalent contracts now live in the keycardai-oauth PKCE test suite. Verified: oauth 215/215 (was 208 + 7 new), agents 81/81 (was 85 - 4 removed implementation tests), mcp 560/560, starlette 49/49, ruff clean. Stacked on #100 (ACC-236 a2a-sdk pin) so the agents test suite can run during validation. * fix(keycardai-oauth): address review findings on PKCE move Three small fixes from the review of #101: 1. PKCEClient now accepts an optional injected httpx.AsyncClient. AgentClient passes its existing http_client through, so a single connection pool covers both the agent service calls and the OAuth flow. close() only closes the client it owns. Restores the one-pool-per-AgentClient behavior from main. 2. Drop the no-op rstrip("/") + "/" round-trip in PKCEClient.authenticate when building the authorization server discovery URL. 3. Assert the discovery URL path in test_authenticate_completes_full_flow. The previous test stubbed http_mock.get with side_effects but never verified what URLs were passed; a typo from oauth-authorization-server to openid-configuration would have gone unnoticed. * refactor(keycardai-oauth): collapse PKCEClient into a flow function on AsyncClient Per Kamil review feedback (#101): a separate PKCEClient sitting next to AsyncClient invited "which client do I use?" The OAuth-server-facing operations belong on the existing AsyncClient. Changes: - keycardai.oauth.pkce.PKCEClient (class) -> keycardai.oauth.pkce.authenticate (module-level async function). One-shot per user login, no state worth preserving across calls. - The function uses AsyncClient internally for server metadata discovery (RFC 8414) and code exchange. AsyncClient is now the only thing in keycardai.oauth that talks to OAuth servers as a client. - AsyncClient.exchange_authorization_code (and Client + the underlying operations._authorize helpers) gain an optional resource= parameter so RFC 8707 tokens still work through the canonical path. - The pkce module retains the user-flow concerns: RFC 9728 challenge parsing, resource metadata fetch (paired with the protected resource, not the OAuth server), browser launch, and the loopback callback server (RFC 8252). - AgentClient drops the cached _pkce instance and just calls the function per /invoke retry, passing its own httpx.AsyncClient through for the resource metadata fetch. - Tests rewritten for the function shape: 7/7 passing, same coverage (header parsing, discovery error paths, happy path with resource indicator, public vs confidential auth on the token endpoint). Verified: oauth 215/215, agents 81/81, mcp 560/560, starlette 49/49. ruff clean.
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
Closes ACC-236. Surfaced by ACC-234 wait-and-see verification:
keycardai-agentstests do not run on a fresh checkout. Three pre-existing breaks fixed:a2a-sdkpinned<1.0. The unbounded>=0.3.22resolved to1.0.x.a2a-sdk1.0 movedA2AStarletteApplicationout ofa2a.server.apps.jsonrpc, whichkeycardai-agents/server/app.pyimports. Pinning is the cheap fix becausekeycardai-agentsis being decomposed and archived in ACC-229..232; the replacementkeycardai-a2a(ACC-230) will be written againsta2a-sdk1.x natively.crewai_a2areferences in tests.tests/integrations/test_crewai_a2a.pyimported fromkeycardai.agents.integrations.crewai_a2a, which does not exist (the module iskeycardai.agents.integrations.crewai). Three import/patch sites updated.test_tool_run_with_task_and_inputs. Asserted"pr_number" in task; the implementation contract putstask_inputsundertask["inputs"]. Assertion updated to match.Test plan
cd packages/agents && uv run --extra test pytest→ 85/85 passingcd packages/mcp && uv run --extra test pytest→ 560/560 (unaffected)cd packages/starlette && uv run --extra test pytest→ 49/49 (unaffected)cd packages/oauth && uv run --extra test pytest→ 208/208 (unaffected)uv run ruff checkcleanWhy pin and not upgrade
keycardai-agentsis being decomposed (ACC-229..232). The replacementkeycardai-a2apackage will adopta2a-sdk1.x natively. Upgrading the existing package is throwaway work; pinning unblocks the suite cheaply until decomposition lands.