Skip to content

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

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

kdubb merged 2 commits 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: 490 tests; complete ./gradlew check passed, including lint and coverage checks. Cross-repository generated per-service and aggregate persistence tests also pass against this worktree.

Closes #65. 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

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

The PR appears safe to merge; the close-and-reopen test addresses the earlier suggestion without introducing a new issue.

What we checked:

  • Closing one manager stops later work: TokenManager creates its own child scope. Closing it leaves the parent scope available for later managers.

Summary

Adds an optional TokenManagerFactory to ClientSettings construction and resolution. Applications can supply storage and lifecycle settings while the default remains memory-only.

  • The latest change closes the first manager before reopening the saved session.
  • The test then checks that the saved token is reused without another acquisition, addressing the earlier review suggestion.
  • No new issues were found in the changes since the previous review.

Reviews (2) 路 Last reviewed commit: "Strengthen persisted-session lifecycle r..."

Close the first manager before reopening persisted credentials. Verify overlapping Swift refresh calls with a test-controlled gate.
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

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

ClientSettings cannot accept application-owned token storage for generated factories

1 participant