fix: immediate token exchange and simplified storage API (#34)

Author: koistya

.vscode/settings.json

@@ -3,9 +3,12 @@
   "editor.defaultFormatter": "esbenp.prettier-vscode",
   "cSpell.words": [
     "bunx",
+    "EADDRINUSE",
     "konstantin",
     "kriasoft",
     "modelcontextprotocol",
+    "myapp",
+    "PKCE",
     "publint",
     "srcpack",
     "streamable",

CLAUDE.md

@@ -1,5 +1,10 @@
 # OAuth Callback Project Guide
 
+## Documentation
+
+**ADRs** (`docs/adr/NNN-slug.md`): Architectural decisions (reference as ADR-NNN)  
+**SPECs** (`docs/specs/slug.md`): Design specifications (reference as SPEC-slug)
+
 ## Project Structure
 
 ```bash
@@ -56,7 +61,7 @@ oauth-callback/
 - `browserAuth()` - MCP SDK-compatible OAuth provider
 - `inMemoryStore()` - Ephemeral token storage
 - `fileStore()` - Persistent file-based token storage
-- Type exports: `BrowserAuthOptions`, `Tokens`, `TokenStore`, `ClientInfo`, `OAuthSession`, `OAuthStore`
+- Type exports: `BrowserAuthOptions`, `Tokens`, `TokenStore`, `ClientInfo`, `OAuthStore`
 
 ## Key Constraints
 

README.md

@@ -127,6 +127,8 @@ import { browserAuth, inMemoryStore } from "oauth-callback/mcp";
 import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
 
+const serverUrl = new URL("https://mcp.notion.com/mcp");
+
 // Create MCP-compatible OAuth provider
 const authProvider = browserAuth({
   port: 3000,
@@ -135,18 +137,29 @@ const authProvider = browserAuth({
   store: inMemoryStore(), // Or fileStore() for persistence
 });
 
-// Use with MCP SDK transport
-const transport = new StreamableHTTPClientTransport(
-  new URL("https://mcp.notion.com/mcp"),
-  { authProvider },
-);
-
 const client = new Client(
   { name: "my-app", version: "1.0.0" },
   { capabilities: {} },
 );
 
-await client.connect(transport);
+// Connect with OAuth retry: first attempt completes OAuth and saves tokens,
+// but SDK returns before checking them. Second attempt succeeds.
+async function connectWithOAuthRetry() {
+  const transport = new StreamableHTTPClientTransport(serverUrl, {
+    authProvider,
+  });
+  try {
+    await client.connect(transport);
+  } catch (error: any) {
+    if (error.message === "Unauthorized") {
+      await client.connect(
+        new StreamableHTTPClientTransport(serverUrl, { authProvider }),
+      );
+    } else throw error;
+  }
+}
+
+await connectWithOAuthRetry();
 ```
 
 #### Token Storage Options
@@ -275,7 +288,7 @@ class OAuthError extends Error {
 
 ### `browserAuth(options)`
 
-Available from `oauth-callback/mcp`. Creates an MCP SDK-compatible OAuth provider for browser-based flows. Handles Dynamic Client Registration (DCR), token storage, and automatic refresh.
+Available from `oauth-callback/mcp`. Creates an MCP SDK-compatible OAuth provider for browser-based flows. Handles Dynamic Client Registration (DCR) and token storage. Expired tokens trigger re-authentication.
 
 #### Parameters
 

docs/.vitepress/config.ts

@@ -58,7 +58,7 @@ export default withMermaid(
         { text: "API", link: "/api/get-auth-code" },
         { text: "Examples", link: "/examples/notion" },
         {
-          text: "v2.0.0",
+          text: "v2.2.0",
           items: [
             {
               text: "Release Notes",
@@ -82,6 +82,7 @@ export default withMermaid(
             },
             { text: "Getting Started", link: "/getting-started" },
             { text: "Core Concepts", link: "/core-concepts" },
+            { text: "ADRs", link: "/adr/" },
           ],
         },
         {

docs/adr/000-template.md

@@ -0,0 +1,28 @@
+# ADR-NNN Title
+
+**Status:** Proposed | Accepted | Deprecated | Superseded  
+**Date:** YYYY-MM-DD  
+**Tags:** tag1, tag2
+
+## Problem
+
+- One or two sentences on the decision trigger or constraint.
+
+## Decision
+
+- The chosen approach in a short paragraph.
+
+## Alternatives (brief)
+
+- Option A — why not.
+- Option B — why not.
+
+## Impact
+
+- Positive:
+- Negative/Risks:
+
+## Links
+
+- Code/Docs:
+- Related ADRs:

docs/adr/001-no-refresh-tokens.md

@@ -0,0 +1,37 @@
+# ADR-001: No Refresh Tokens in browserAuth
+
+**Status:** Accepted
+**Date:** 2025-01-25
+**Tags:** oauth, mcp, security
+
+## Problem
+
+- Should `browserAuth` handle OAuth refresh tokens and automatic token renewal?
+
+## Decision
+
+- `browserAuth` intentionally does not request or handle refresh tokens.
+- When tokens expire, `tokens()` returns `undefined`, signaling the MCP SDK to re-authenticate.
+- The SDK's built-in retry logic handles re-auth transparently.
+
+Rationale:
+
+- **MCP SDK lifecycle**: The SDK expects auth providers to return `undefined` for expired tokens, triggering its standard re-auth flow.
+- **CLI/desktop UX**: Interactive re-consent is acceptable and often preferred over silent background refresh.
+- **Simplicity**: Avoiding refresh eliminates token rotation complexity, race conditions, and concurrent refresh handling.
+- **Security**: No long-lived refresh tokens stored; each session requires explicit user consent.
+
+## Alternatives (brief)
+
+- **Implement refresh flow** — Adds complexity (token rotation, concurrency), conflicts with SDK's re-auth expectations, stores long-lived credentials.
+- **Optional refresh via config** — Increases API surface, creates two divergent code paths to maintain.
+
+## Impact
+
+- Positive: Simpler implementation, predictable behavior, aligns with MCP SDK design.
+- Negative/Risks: More frequent browser prompts for long-running sessions (acceptable for CLI tools).
+
+## Links
+
+- Code: `src/auth/browser-auth.ts`
+- Related: MCP SDK auth interface in `@modelcontextprotocol/sdk/client/auth`

docs/adr/002-immediate-token-exchange.md

@@ -0,0 +1,67 @@
+# ADR-002: Immediate Token Exchange in redirectToAuthorization()
+
+**Status:** Accepted
+**Date:** 2025-01-25
+**Tags:** oauth, mcp, sdk-integration
+
+## Problem
+
+The MCP SDK's `auth()` flow works as follows:
+
+1. Check `provider.tokens()` — if valid tokens exist, return `'AUTHORIZED'`
+2. If no tokens, start authorization: call `redirectToAuthorization(url)`
+3. **Immediately** return `'REDIRECT'` (without re-checking tokens)
+
+For web-based OAuth, this makes sense: `redirectToAuthorization()` triggers a page redirect and control never returns synchronously. The SDK expects authentication to complete in a subsequent request.
+
+For CLI/desktop apps using `browserAuth()`, control **does** return synchronously—we capture the callback in-process via a local HTTP server. We exchange tokens inside `redirectToAuthorization()`, but the SDK has already decided to return `'REDIRECT'`, causing `UnauthorizedError`.
+
+## Decision
+
+Exchange tokens **inside** `redirectToAuthorization()` and document the retry pattern as the expected usage:
+
+```typescript
+// First connect triggers OAuth flow and saves tokens, but SDK returns
+// 'REDIRECT' before checking. Second connect finds valid tokens.
+async function connectWithOAuthRetry(client, serverUrl, authProvider) {
+  const transport = new StreamableHTTPClientTransport(serverUrl, {
+    authProvider,
+  });
+  try {
+    await client.connect(transport);
+  } catch (error) {
+    if (error.message === "Unauthorized") {
+      await client.connect(
+        new StreamableHTTPClientTransport(serverUrl, { authProvider }),
+      );
+    } else throw error;
+  }
+}
+```
+
+**Why a new transport on retry?** The transport caches connection state internally. A fresh transport ensures clean reconnection.
+
+## Rationale
+
+- **SDK constraint**: No hook exists between redirect completion and the `'REDIRECT'` return. The SDK interface (`Promise<void>`) cannot signal "auth completed."
+- **In-process capture**: CLI apps don't have page redirects that would trigger a fresh auth check cycle.
+- **Correctness over elegance**: The retry is unusual but reliable—tokens are always saved before the error.
+
+## Alternatives Considered
+
+| Alternative                                    | Why Rejected                                                  |
+| ---------------------------------------------- | ------------------------------------------------------------- |
+| `transport.finishAuth(callbackUrl)`            | Breaks provider encapsulation; doesn't fit in-process capture |
+| Return tokens from `redirectToAuthorization()` | SDK interface expects `Promise<void>`                         |
+| Upstream SDK change                            | Not viable for library consumers                              |
+
+## Impact
+
+- **Positive**: Self-contained auth flow; no external coordination needed
+- **Negative**: First connection always throws `UnauthorizedError` after OAuth—must be documented clearly
+
+## Links
+
+- Code: `src/auth/browser-auth.ts` lines 254-368
+- MCP SDK auth interface: `@modelcontextprotocol/sdk/client/auth.js`
+- Related: ADR-001 (no refresh tokens)

docs/adr/003-stable-client-metadata.md

@@ -0,0 +1,31 @@
+# ADR-003: Stable Client Metadata Across DCR
+
+**Status:** Accepted
+**Date:** 2025-01-25
+**Tags:** oauth, dcr, security
+
+## Problem
+
+- During Dynamic Client Registration (DCR), the authorization server may return different capabilities than requested (e.g., `token_endpoint_auth_method`).
+- If `clientMetadata` adapts to DCR responses, subsequent token requests may fail when the AS caches the original registration metadata.
+
+## Decision
+
+- `clientMetadata` is immutable after construction.
+- `token_endpoint_auth_method` is determined at construction: `client_secret_post` if `clientSecret` is provided, `none` otherwise. DCR responses never change this value.
+- DCR credentials (`client_id`, `client_secret`) are stored separately and never mutate the auth method.
+
+## Alternatives (brief)
+
+- **Dynamic metadata evolution** — Adapting to DCR responses seems flexible but causes cache mismatches with AS that remember original registration.
+- **Per-request method detection** — Adds complexity and non-deterministic behavior across retries.
+
+## Impact
+
+- Positive: Predictable behavior with all AS implementations; eliminates cache-related auth failures.
+- Negative/Risks: None identified; the fixed method (`client_secret_post`) has universal support.
+
+## Links
+
+- Code: `src/auth/browser-auth.ts`
+- Related ADRs: ADR-001 (No Refresh Tokens), ADR-002 (Immediate Token Exchange)

docs/adr/004-conditional-state-validation.md

@@ -0,0 +1,40 @@
+# ADR-004: Conditional OAuth State Validation
+
+**Status:** Accepted
+**Date:** 2025-01-25
+**Tags:** oauth, security, csrf
+
+## Problem
+
+- RFC 6749 recommends `state` for CSRF protection, but RFC 8252 (native apps) relies on loopback redirect for security.
+- Some authorization servers don't echo `state` back; others require it.
+- Strict validation breaks compatibility; no validation weakens security.
+
+## Decision
+
+- Validate `state` only if it was present in the authorization URL.
+- If the auth URL includes `state` and the callback doesn't match, reject as CSRF.
+- If the auth URL omits `state`, accept callbacks without state validation.
+
+Rationale:
+
+- **Defense-in-depth**: Loopback binding (127.0.0.1) prevents network CSRF, but state adds protection against local attacks (malicious apps, browser extensions intercepting localhost).
+- **CLI threat model**: Unlike web apps, CLI tools face local machine threats—other processes can probe localhost ports. State validation detects if a callback arrives from an unrelated auth flow.
+- **Compatibility**: Authorization servers have inconsistent state handling. Conditional validation works with all servers while providing protection when available.
+
+## Alternatives (brief)
+
+- **Always require state** — Breaks servers that don't echo state or don't support it.
+- **Never validate state** — Loopback provides baseline security, but ignores state when the AS cooperates.
+- **Generate state internally always** — Conflicts with auth URLs that already include state from the MCP SDK.
+
+## Impact
+
+- Positive: Maximum security when AS supports state; universal compatibility otherwise.
+- Negative/Risks: If an AS echoes arbitrary state values without validation, the protection is weaker (rare edge case).
+
+## Links
+
+- Code: `src/auth/browser-auth.ts:297-300`
+- RFC 6749 Section 10.12 (CSRF Protection)
+- RFC 8252 Section 8.1 (Loopback Redirect)

docs/adr/005-store-responsibility-reduction.md

@@ -0,0 +1,36 @@
+# ADR-005: OAuthStore Responsibility Reduction
+
+**Status:** Accepted
+**Date:** 2025-01-25
+**Tags:** api, storage, simplification
+
+## Problem
+
+The store was accumulating OAuth flow state (session, nonce, state parameter) alongside persistent data (tokens, client registration). This blurred the line between "what survives a crash" and "what's ephemeral by design," making the API harder to reason about and test.
+
+## Decision
+
+The store is responsible **only** for data that must survive process restarts:
+
+| Stored                | Not Stored        |
+| --------------------- | ----------------- |
+| `tokens`              | `state` parameter |
+| `client` (DCR result) | `nonce`           |
+| `codeVerifier` (PKCE) | session objects   |
+
+The `codeVerifier` is the sole flow artifact persisted—it enables completing an in-progress authorization if the process crashes after browser launch but before callback.
+
+## Alternatives (brief)
+
+- **Full session persistence** — Would enable crash-recovery at any point, but adds complexity for a rare edge case. Users can simply restart the flow.
+- **No verifier persistence** — Simpler, but loses the most common crash scenario (user switches apps, process dies).
+
+## Impact
+
+- Positive: Cleaner mental model; store implementations are trivial to write and test.
+- Negative: If the process crashes before `codeVerifier` is saved, the flow must restart. This is acceptable—it's a sub-second window.
+
+## Links
+
+- Code: `src/storage/`, `src/mcp-types.ts`
+- Related: ADR-002 (Immediate Token Exchange)