Skip to content

feat(platform): support external token providers and simplify caching [PYSDK-123] - #513

Merged
akunft merged 1 commit into
mainfrom
feat/enable_external_token_provider
May 7, 2026
Merged

feat(platform): support external token providers and simplify caching [PYSDK-123]#513
akunft merged 1 commit into
mainfrom
feat/enable_external_token_provider

Conversation

@akunft

@akunftakunft commented Mar 26, 2026

Copy link
Copy Markdown
Collaborator

🛡️ Implements PYSDK-123 following CC-SOP-01 Change Control, part of our ISO 13485-certified QMS | Ketryx Project

Why

Add an external token_provider parameter to Client.__init__() so callers can supply their own bearer-token callable (e.g. machine-to-machine / service-account flows) instead of relying on the built-in OAuth device-code flow. Internal OAuth remains the default; with this change it is no longer the only supported path.

Primary motivation: enable headless deployment scenarios (CI/CD jobs, Auth0 M2M, custom auth backends) where the device-code browser flow is impractical or impossible. Driving use case is M2M tokens cached upstream in Redis with stale-while-validate semantics.

What

SIS — specifications/SPEC_PLATFORM_SERVICE.md:

  • New FR-14:"Support external token providers to bypass internal OAuth 2.0 flows for machine-to-machine, service account, or custom token lifecycle scenarios."
  • Constraint relaxed: the "Browser dependency: Interactive flow requires web browser availability, limiting headless deployment options" line is removed because external token_provider enables headless operation.
  • Strategy pattern updated to describe external-token-provider as a fully independent alternative auth strategy.
  • Auth flow Mermaid diagram updated with the external-provider branch.
  • Outputs table & class signatures updated for the new token_provider parameter and the internal _AuthenticatedApi return type.

Code — src/aignostics/platform/:

  • New file _api.py introducing internal _AuthenticatedApi (PublicApi subclass that hoists token_provider to a top-level attribute), _OAuth2TokenProviderConfiguration, and _AuthenticatedResource base class.
  • Client.__init__(cache_token=True, token_provider=None) — additive optional kwarg.
  • Client.get_api_client() — same kwarg; returns _AuthenticatedApi. Three singleton-cache pools: cached-token, uncached-token, external-provider (bounded at 16 entries with clear-on-overflow safety net to address Sentry's flagged unbounded-growth concern).
  • cached_operation decorator — use_token: bool replaced by token_provider: Callable[[], str] | None, so per-user cache key isolation is wired through the explicit provider rather than reaching into the global get_token().

What does NOT change

  • No backend / API-server change.
  • Internal OAuth device-code and authorization-code flows continue to work unchanged when token_provider is not supplied.
  • No change to aignostics user login, token caching at ~/.aignostics/token.json, or refresh-token semantics.
  • No change to existing public method signatures other than the additive token_provider kwarg.
  • Backwards-compatible — existing callers continue to work without modification.

Risks introduced

None material. The new parameter is opt-in. The bounded _api_client_external cache addresses Sentry's flagged unbounded-growth concern. No patient-safety, security, or data-integrity risk identified — the external token continues to flow through the same Authorization: Bearer … header path; the SDK never inspects, persists, or logs the token value.

Test plan

  • make lint green (ruff, format, pyright, mypy)
  • make test_unit green; new tests/aignostics/platform/client_token_provider_test.py covers external-provider wiring
  • Cache-test suite refactored to mock token_provider callable rather than patching get_token — exercises the new key-derivation path
  • Codecov on patch
  • CodeQL clean; SonarQube quality gate passed
  • Required Testing = Verification only — automated CI proves the surface area; the headless-M2M path itself will be exercised when a downstream caller integrates it (out of scope for this CR)

Note: Originally provided retroactively for PYSDK-112 (Helmut); governing ticket corrected to PYSDK-123 (Andreas, Claude claude-sonnet-4-6 via cc-sop-01).

CopilotAI review requested due to automatic review settings March 26, 2026 16:38

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for supplying an external bearer-token provider to the Platform Client, and refactors operation caching to key per-user via an explicit token_provider callable rather than relying on a global auth helper.

Changes:

  • Introduces _AuthenticatedApi + _OAuth2TokenProviderConfiguration in a new _api.py to surface token_provider as a top-level attribute (avoiding circular imports).
  • Updates cached_operation to accept token_provider (replacing the previous use_token flag) and wires it through Client and resource modules.
  • Updates and expands unit tests to cover external token providers and the revised caching behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/aignostics/platform/_api.pyAdds _AuthenticatedApi wrapper + configuration class to lift token_provider out of codegen internals.
src/aignostics/platform/_client.pyAdds token_provider parameter to Client, introduces external-provider singleton caching, and passes token_provider into cached_operation.
src/aignostics/platform/_operation_cache.pyReplaces use_token with token_provider in the caching decorator API and key generation.
src/aignostics/platform/resources/runs.pyUpdates resource API type hints and passes token_provider=self._api.token_provider into cached operations.
src/aignostics/platform/resources/applications.pySame as above for applications/versions resources.
tests/aignostics/platform/client_token_provider_test.pyAdds unit tests for external token providers and adjusts existing tests to the new API wiring.
tests/aignostics/platform/client_cache_test.pyUpdates cache tests to use mocked token_provider for token-aware cache keys.
tests/aignostics/platform/nocache_test.pyUpdates decorator calls to the new cached_operation signature.
tests/aignostics/platform/conftest.pyEnsures new external-client singleton cache is cleared between tests; adds token_provider to mocked API clients.
tests/aignostics/platform/resources/runs_test.pyUpdates resource mocks to include token_provider/api_client attributes.
tests/aignostics/platform/resources/applications_test.pyUpdates resource mocks to include token_provider/api_client attributes.

Comment threadtests/aignostics/platform/resources/runs_test.py Outdated
Comment threadtests/aignostics/platform/resources/applications_test.py Outdated
Comment threadsrc/aignostics/platform/resources/runs.py
Comment threadsrc/aignostics/platform/resources/applications.py
@codecov

codecovBot commented Mar 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.95833% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing linesPatch %Lines
src/aignostics/platform/resources/applications.py91.66%1 Missing ⚠️
Files with missing linesCoverage Δ
src/aignostics/platform/_api.py100.00% <100.00%> (ø)
src/aignostics/platform/_authentication.py77.92% <100.00%> (-0.40%)⬇️
src/aignostics/platform/_client.py93.02% <100.00%> (-0.67%)⬇️
src/aignostics/platform/_operation_cache.py78.43% <100.00%> (+0.88%)⬆️
src/aignostics/platform/resources/runs.py84.67% <100.00%> (-0.44%)⬇️
src/aignostics/platform/resources/applications.py84.52% <91.66%> (+1.28%)⬆️

... and 3 files with indirect coverage changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Comment threadtests/aignostics/platform/resources/runs_test.py Outdated
Comment threadtests/aignostics/platform/resources/applications_test.py Outdated
Comment threadsrc/aignostics/platform/_client.py Outdated
@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from ab1533d to b407d30CompareMarch 30, 2026 13:41
@akunft
akunft requested a review from CopilotMarch 30, 2026 13:42

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comment threadsrc/aignostics/platform/_client.py Outdated
Comment threadsrc/aignostics/platform/resources/runs.py Outdated
Comment threadsrc/aignostics/platform/_api.py Outdated
Comment threadsrc/aignostics/platform/_client.py Outdated

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comment threadsrc/aignostics/platform/_client.py
Comment threadsrc/aignostics/platform/_api.py
Comment threadtests/aignostics/platform/resources/runs_test.py
Comment threadtests/aignostics/platform/conftest.py
@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from 7e0a0b9 to de2c81dCompareMarch 31, 2026 09:41
Comment threadsrc/aignostics/platform/_client.py
CopilotAI review requested due to automatic review settings March 31, 2026 12:31
@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from de2c81d to 7e3fac5CompareMarch 31, 2026 12:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment threadsrc/aignostics/platform/_api.py
Comment threadsrc/aignostics/platform/_client.py Outdated
@akunft
akunft requested a review from CopilotMarch 31, 2026 13:58

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/aignostics/platform/resources/runs.py:137

  • Run.for_run_id() always constructs its API client via Client.get_api_client(cache_token=...), so there is currently no way to use it with an external token_provider (the new auth path). Consider adding an optional token_provider parameter (and documenting cache_token as ignored when it’s provided) so this convenience constructor remains usable for M2M/service-account flows.
 @classmethod
def for_run_id(cls, run_id: str, cache_token: bool = True) -> "Run":
"""Creates an Run instance for an existing run.
Args:
run_id (str): The ID of the application run.
cache_token (bool): Whether to cache the API token.
Returns:
Run: The initialized Run instance.
"""
from aignostics.platform._client import Client # noqa: PLC0415
return cls(Client.get_api_client(cache_token=cache_token), run_id)

Comment threadsrc/aignostics/platform/_api.py
Comment threadsrc/aignostics/platform/_operation_cache.py
Comment threadsrc/aignostics/platform/_client.py
@olivermeyer

Copy link
Copy Markdown
Collaborator

I believe this impacts https://github.com/aignostics/python-sdk/blob/main/specifications/SPEC_PLATFORM_SERVICE.md, therefore it needs a CR?

@arne-aignx

Copy link
Copy Markdown
Collaborator

I believe this impacts https://github.com/aignostics/python-sdk/blob/main/specifications/SPEC_PLATFORM_SERVICE.md, therefore it needs a CR?

Yes, I concur...
I don't even think this is strictly covered by the requirements stated in that SIS.

-> SW requirement + SIS update is in order here :/

@akunft

Copy link
Copy Markdown
CollaboratorAuthor

I believe this impacts https://github.com/aignostics/python-sdk/blob/main/specifications/SPEC_PLATFORM_SERVICE.md, therefore it needs a CR?

Yes, I concur... I don't even think this is strictly covered by the requirements stated in that SIS.

-> SW requirement + SIS update is in order here :/

Agreed. I will look into this, but only on Monday.

@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from f2296b9 to 262ac88CompareApril 7, 2026 13:54
Comment threadspecifications/SPEC_PLATFORM_SERVICE.md Outdated
CopilotAI review requested due to automatic review settings April 10, 2026 16:22
CopilotAI review requested due to automatic review settings April 27, 2026 15:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Comment threadspecifications/SPEC_PLATFORM_SERVICE.md
Comment threadspecifications/SPEC_PLATFORM_SERVICE.md Outdated
Comment threadsrc/aignostics/platform/CLAUDE.md
@akunftakunft changed the title feat(platform): support external token providers and simplify cachingfeat(platform): support external token providers and simplify caching [PYSDK-123]May 5, 2026
@akunftakunft added the skip:test:long_running Skip long-running tests (≥5min) label May 5, 2026
@akunftakunft self-assigned this May 5, 2026
akunft pushed a commit that referenced this pull request May 5, 2026
…ows [PYSDK-112]
`test_no_retry_on_other_jwk_errors` had two assertions checking the same
property (no retry occurred):
1. `assert elapsed_time < 2.0` — wall-clock heuristic, flaky
2. `assert call_count == 1` — direct check on the mock's call count
Assertion 2 proves "no retry" rigorously and deterministically. Assertion
1 was a redundant proxy that consistently failed on `windows-latest`
GitHub-hosted runners (came in at ~2.4s vs the 2s threshold) while the
other four runner combos (ubuntu-latest, ubuntu-24.04-arm, macos-latest,
macos-15-intel) all completed in well under 2s.
Removing assertion 1 (and the now-unused `start_time`/`elapsed_time`
locals) removes the flake without losing test coverage. The companion
test `test_jwk_connection_errors_trigger_retry` immediately above already
follows this pattern (call-count assertion only, no wall-clock check).
Bundled into PYSDK-112 at user direction — surfaced on PR #513 CI run
and observed as the only blocker for the otherwise-green test matrix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
akunft pushed a commit that referenced this pull request May 5, 2026
CI run 24957873268 was triggered by the merge commit d9ab794 BEFORE
the skip:test:long_running label was removed from PR #513. As a result,
the workflow snapshotted the label as still present and skipped the
long-running test stage on every matrix combo (each long_running step
took 0s and was a no-op). Without long_running tests running, codecov
gathered partial coverage and codecov/project failed.
This empty commit forces a fresh workflow trigger that picks up the
current label set (skip:test:long_running no longer present), so
long_running tests will actually run and codecov gets full coverage data.
@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from 888a147 to 975e526CompareMay 5, 2026 10:08
CopilotAI review requested due to automatic review settings May 5, 2026 12:43
@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from 975e526 to 36ac55cCompareMay 5, 2026 12:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/aignostics/platform/resources/runs_test.py:731

  • The unit test coverage for artifact download behavior appears to have been removed from this module (e.g., Artifact.get_download_url redirect/status handling and Run.ensure_artifacts_downloaded download/resume/checksum logic), but the corresponding production code in src/aignostics/platform/resources/runs.py still exists. Please reintroduce these tests (or move them to a new dedicated test module) so changes to /file URL resolution, retry/error mapping, and download-resume behavior remain covered at the unit level (not only via slower e2e/integration tests).
@pytest.mark.unit
def test_run_details_does_not_retry_other_exceptions(app_run, mock_api) -> None:
"""Test that the outer retry does not catch non-NotFoundException errors.
This verifies that exceptions like ForbiddenException pass straight through
the outer retry without being retried.
Args:
app_run: Run instance with mock API.
mock_api: Mock ExternalsApi instance.
"""
from aignx.codegen.exceptions import ForbiddenException
mock_api.get_run_v1_runs_run_id_get.side_effect = ForbiddenException()
with pytest.raises(ForbiddenException):
app_run.details()
assert mock_api.get_run_v1_runs_run_id_get.call_count == 1

CopilotAI review requested due to automatic review settings May 5, 2026 20:40

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.

Comment threadsrc/aignostics/platform/_api.py
Comment threadsrc/aignostics/platform/resources/runs.py
@akunftakunft removed skip:test:long_running Skip long-running tests (≥5min) labels May 6, 2026
@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from 518785d to 359c70dCompareMay 6, 2026 14:16
CopilotAI review requested due to automatic review settings May 6, 2026 16:16
Add a `token_provider` parameter to `Client.__init__()` so callers can
supply their own bearer-token callable (e.g. for M2M / service-account
flows) instead of relying on the built-in OAuth device-code flow.
The operation cache needs a token to build per-user cache keys. Three
approaches were explored:
1. **CachedApiMixin with Protocol** (explored, discarded) —
A `_HasTokenProvider` Protocol plus `CachedApiMixin` base class that
provided a `_cached()` convenience method. Saved one kwarg per call
site but added multiple-inheritance, a Protocol, and competing `_api`
annotations on every resource class. Over-engineering for what amounts
to avoiding `token_provider=self._api.token_provider`.
2. **`TokenProvider` type alias** (explored, discarded) —
`TokenProvider = Callable[[], str]` exported as public API. Added no
value over the raw `Callable[[], str]` since every parameter is
already named `token_provider`. Removed to avoid unnecessary imports
and indirection.
3. **`_AuthenticatedApi` subclass + explicit `token_provider` at each
call site** (chosen) — A thin `_AuthenticatedApi(PublicApi)` subclass
in `_api.py` lifts `token_provider` from the deeply-nested codegen
`Configuration` to a top-level attribute. Each cached method passes
`token_provider=self._api.token_provider` to `@cached_operation(...)`.
One kwarg of boilerplate per call site, but no new types, no
inheritance, and trivially greppable. `_api.py` exists solely to
break the circular import between `_client.py` and the resource
modules.
@akunft
akunftforce-pushed the feat/enable_external_token_provider branch from 5d60375 to 5f307d0CompareMay 7, 2026 07:44
@sonarqubecloud

Copy link
Copy Markdown

@akunft
akunft enabled auto-merge (rebase) May 7, 2026 08:21
@akunft
akunft merged commit 9007e37 into mainMay 7, 2026
23 of 24 checks passed
@akunft
akunft deleted the feat/enable_external_token_provider branch May 7, 2026 08:22
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope:sdk-consumersAffects downstream SDK consumers (uvx aignostics / uv add aignostics)sop:cc-sop-01CC-SOP-01 Change Control (feature / planned change)type:featureNew functionality (conventional feat)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@akunft@olivermeyer@arne-aignx@helmut-hoffer-von-ankershoffen