SEP-2207: Refresh token guidance #1523
+263
−6
Open
Claude / Claude Code Review
completed
Mar 27, 2026 in 5m 41s
Code review found 3 potential issues
Found 5 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | packages/client/src/client/auth.ts:590-599 |
fetchToken bypasses determineScope for non-interactive flows |
| 🟣 Pre-existing | packages/client/src/client/auth.ts:493 |
Inconsistent offline_access check: substring vs exact token match |
Annotations
Check warning on line 599 in packages/client/src/client/auth.ts
claude / Claude Code Review
fetchToken bypasses determineScope for non-interactive flows
Nit: `fetchToken()` (line 650) receives the raw `scope` parameter instead of `resolvedScope`, so non-interactive flows using `prepareTokenRequest()` miss the PRM `scopes_supported` fallback and the `offline_access` augmentation from `determineScope()`. In practice the impact is minimal — authorization_code flows don't send scope in the token request at all, and `client_credentials` + refresh tokens is not a valid combination per RFC 6749 §4.4.3 — but passing `resolvedScope` here would be more co
Check notice on line 493 in packages/client/src/client/auth.ts
claude / Claude Code Review
Inconsistent offline_access check: substring vs exact token match
Nit (pre-existing): `startAuthorization()` at line 1259 uses `scope?.includes('offline_access')` (substring match on the string), while the new `determineScope()` correctly uses `.split(' ').includes('offline_access')` (exact token match). A hypothetical scope like `"no_offline_access"` would incorrectly trigger `prompt=consent`. Consider updating line 1259 to `scope?.split(' ').includes('offline_access')` for consistency.
Loading