Skip to content

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

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: 786 tests passed; ESLint, TypeScript source/test checking and package build passed. Cross-repository generated per-service and aggregate persistence tests also pass against this worktree.

Closes #82. 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.
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

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

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

What we checked:

  • Invalid credentials reach the factory: ClientSettings checks each selected credential before preparing the manager. Missing or incompatible credentials stop construction first.
  • Construction reads saved tokens: The constructor only stores its options. Storage reads happen later when callers request credentials.

Summary

Adds an optional TokenManagerFactory to ClientSettings construction and resolution so applications can supply a native manager with their own storage and clock options.

  • Keeps the in-memory default and skips the factory when no providers are selected.
  • Calls the factory once, after credential checks, with the resolved provider map.
  • Adds tests for saved-session reuse, refresh rotation, concurrent renewal, isolation and logout.
  • Documents application ownership of storage and session cleanup.

No actionable issues found. Tests and builds were not run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Construct or resolve ClientSettings] --> B[Check credentials and prepare providers]
  B --> C{Any selected providers?}
  C -->|No| D[No token manager]
  C -->|Yes| E{Factory supplied?}
  E -->|No| F[Create memory-only TokenManager]
  E -->|Yes| G[Call application factory once]
  F --> H[Retain manager for all operations]
  G --> H
Loading

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

@kdubb
kdubb merged commit 707f99b into main Oct 6, 2026
3 checks passed
@kdubb
kdubb deleted the codex/issue-253-token-persistence branch October 6, 2026 13:42
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.

Expose token-store and manager options through ClientSettings

1 participant