Skip to content

Allow client settings to configure application-owned token managers - #11

Merged
kdubb merged 1 commit into
mainfrom
codex/issue-253-token-persistence
Oct 6, 2026
Merged

kdubb merged 1 commit into
mainfrom
codex/issue-253-token-persistence

Conversation

@kdubb

@kdubb kdubb commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

ClientSettings previously always created an in-memory token manager, so configuration-created clients could not reopen an application's saved session. Add an optional TokenManagerFactory to construction and resolution that receives the resolved provider map and creates exactly one application-configured native manager.

The default stays memory-only and the hook is skipped for public settings. Applications retain control of storage, native clock/skew options and lifecycle. Document logout, session/environment isolation and the scope of single-flight renewal.

Regression coverage executes initial acquisition, reuse by a new settings instance, expiry/skew-triggered refresh, persisted refresh-token rotation, concurrent renewal, isolation and logout. It also checks lazy storage access, constructor/resolver forwarding, skipped hooks for public/invalid settings and factory errors.

Validation: 486 tests passed with 92.45% coverage; Ruff format/lint, strict mypy, wheel/sdist build and Twine checks passed. Cross-repository generated per-service and aggregate persistence tests also pass against this worktree.

Closes #10. Runtime dependency for outfoxx/sunday-generator#253; publish a companion release before the generator forwarding is released.

Expose a factory over resolved providers while retaining lazy in-memory defaults.
Test saved-session reuse, refresh rotation, isolation, logout and factory behavior.
Document native manager configuration and application lifecycle ownership.
@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds optional token manager factory hook to client settings.

The PR appears safe to merge; no actionable issues were found.

What we checked:

  • Invalid settings reach the factory: Credential checks finish before manager creation. Provider conflicts also raise before the factory runs.
  • Construction reads saved tokens: TokenManager only retains the store during construction. It loads tokens when a caller asks for credentials.

Summary

Adds an optional TokenManagerFactory to ClientSettings construction and resolution so applications can reopen saved token sessions.

  • Keeps the in-memory default and skips the hook when no providers are selected.
  • Exports the factory type and documents storage ownership, session isolation, logout, and shared renewal.
  • Adds regression tests for reuse, refresh rotation, concurrent renewal, isolation, and factory errors.
  • No actionable issues found. Tests were inspected, not executed during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Construct or resolve settings] --> B[Validate credentials and resolve providers]
    B --> C{Providers selected?}
    C -- No --> D[No token manager]
    C -- Yes --> E{Factory supplied?}
    E -- No --> F[Create in-memory manager]
    E -- Yes --> G[Call application factory once]
    F --> H[Settings retain manager]
    G --> H
    H --> I[Requests load or renew tokens]
Loading

Reviews (1) · Last reviewed commit: "Allow client settings to configure appli..."

@kdubb
kdubb merged commit 4423ae2 into main Oct 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow ClientSettings to use application-owned token storage

1 participant