feat(provider): add Claude subscription OAuth#45
Open
CreatorGhost wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughAdds Anthropic Claude Pro/Max subscription OAuth support across core integrations, LLM routing, OpenCode credential management, session execution, provider discovery, and TUI selection. It includes refresh deduplication, PKCE authorization, token persistence, request transformation, and streaming response handling. ChangesAnthropic subscription support
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TUI
participant OpenCode
participant CredentialService
participant AnthropicAPI
User->>TUI: select Claude Pro/Max provider
TUI->>OpenCode: start OAuth authorization
OpenCode-->>User: authorization URL and PKCE state
User->>OpenCode: complete OAuth callback
OpenCode->>CredentialService: store OAuth credential
OpenCode->>AnthropicAPI: send transformed authenticated request
AnthropicAPI-->>OpenCode: stream response
OpenCode-->>User: display restored model response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
packages/opencode/src/cli/cmd/providers.ts (1)
306-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated synthetic-subscription-entry injection block.
The "fetch latest subscription credential, inject a synthetic OAuth entry if not already present" block (lines 306-320) is duplicated almost verbatim in
ProvidersLogoutCommand(566-579). Consider extracting a small helper (e.g.withSyntheticSubscriptionAuth(entries)) shared by both commands to avoid the two copies drifting.♻️ Proposed helper extraction
+const withSyntheticSubscriptionAuth = Effect.fn("Cli.providers.withSyntheticSubscriptionAuth")(function* ( + entries: Array<[string, Auth.Info]>, +) { + const subscription = yield* Effect.gen(function* () { + return (yield* (yield* Credential.Service).list(AnthropicSubscriptionIntegrationID)).at(-1) + }).pipe(Effect.provide(credentialLayer)) + if (subscription?.value.type !== "oauth" || entries.some(([id]) => id === AnthropicSubscriptionProviderID)) { + return entries + } + return [ + ...entries, + [ + AnthropicSubscriptionProviderID, + { type: "oauth", access: subscription.value.access, refresh: subscription.value.refresh, expires: subscription.value.expires }, + ] as [string, Auth.Info], + ] +})Also applies to: 566-585
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/cli/cmd/providers.ts` around lines 306 - 321, Extract the duplicated subscription-auth injection logic from the current command and ProvidersLogoutCommand into a shared helper such as withSyntheticSubscriptionAuth(entries). Have the helper fetch the latest Anthropic subscription credential, add the synthetic OAuth entry only when appropriate and absent, then update both commands to use it while preserving their existing results flow.packages/core/test/session-runner-anthropic-subscription.test.ts (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the star import with the exported namespace name.
Import the module’s named
SessionRunnerLLMnamespace directly, adding the prescribed self-reexport if it is not currently exposed.As per coding guidelines, “Never use star imports, including type star imports. For namespace-style values, import the module's exported namespace by name and reference it through that namespace.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/test/session-runner-anthropic-subscription.test.ts` at line 25, Replace the star import of the LLM runner with the module’s exported SessionRunnerLLM namespace and continue referencing it through that namespace. If the namespace is not currently exported by the runner module, add the prescribed self-reexport there before updating the test import.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/integration.ts`:
- Around line 420-425: Update both compare-and-swap guards in the OAuth refresh
flow around the visible token comparisons to also compare the current OAuth
value’s methodID with oauth.methodID. Return the latest value whenever access,
refresh, expires, or methodID differs, preventing stale refresh results from
overwriting a concurrent OAuth-method change.
In `@packages/core/src/plugin/provider/anthropic-subscription.ts`:
- Around line 39-56: The shared request created by
refreshAnthropicSubscriptionToken must be bounded so a stalled token endpoint
cannot leave the cached Promise pending. Add one timeout around
tokenRequestPromise, abort the underlying request when it expires, ensure
cleanup still removes the matching pending entry, and add coverage using a
never-resolving endpoint to verify the Promise and integration Deferred settle.
In `@packages/core/src/session/runner/model.ts`:
- Around line 167-173: Update the credential guard in the model execution flow
to require both OAuth type and credential.methodID ===
AnthropicSubscriptionMethodID before forwarding the token; otherwise return the
existing CredentialRequiredError with the current providerID and modelID values.
In `@packages/llm/src/providers/anthropic-subscription.ts`:
- Line 3: Replace the star import for AnthropicMessages with a named namespace
import from the public anthropic-messages module, preserving the existing
AnthropicMessages references in the provider.
- Around line 161-165: Update restoreToolNames so only mcp_ names present in the
names mapping are restored; when no mapping exists, preserve the original mcp_
tool name unchanged. Keep the existing mapped-name replacement behavior and
avoid applying unprefixToolName as a fallback.
In `@packages/llm/test/provider/anthropic-subscription.test.ts`:
- Around line 7-11: Replace the layer-backed test usage in the “Anthropic
subscription provider” suite with the prescribed testEffect helper: import
testEffect from the existing effect test utility and update both test
declarations currently using it.effect, preserving their test bodies and layer
setup.
In `@packages/opencode/src/cli/cmd/providers.ts`:
- Around line 43-87: Three implementations of Anthropic subscription credential
persistence and legacy-auth retirement have diverged in rollback behavior. Add a
shared helper in anthropic-subscription-credential.ts that owns the locked
create-or-update, legacy-auth removal, and rollback semantics, then replace the
anthropic branch of putOAuth in packages/opencode/src/cli/cmd/providers.ts lines
43-87, the OAuth callback branch in packages/opencode/src/provider/auth.ts lines
219-260, and the storedAuth legacy-migration branch in
packages/opencode/src/provider/provider.ts lines 1388-1424 with calls to that
helper.
- Around line 604-633: Update the Anthropic subscription logout flow around the
stored credential collection and authSvc.remove rollback so every credential
removed from stored is recreated when removal fails, preserving each
credential’s integrationID, label, and value. Replace the single stored.at(-1)
rollback input with iteration over all stored rows while retaining the existing
credentialLayer and no-op behavior when none were removed.
In `@packages/opencode/src/plugin/anthropic/subscription.ts`:
- Around line 254-259: Update the 401 retry handling around getAuth so an
undefined latest credential is treated as disconnected and fails immediately.
Only call refresh and send with latest.refresh or auth.refresh when an existing
credential is available; preserve the access-token replacement path for valid
changed OAuth credentials.
In `@packages/opencode/src/provider/provider.ts`:
- Around line 1388-1424: Update Provider.storedAuth so the common
already-migrated OAuth credential path reads and returns the credential without
entering withAnthropicSubscriptionCredentialLock. Keep the lock only around
legacy-auth migration and its credential creation/removal writes, and remove any
unnecessary auth.get cleanup from the fast path; preserve existing behavior for
non-Anthropic providers and non-OAuth credentials.
In `@packages/opencode/src/server/routes/instance/httpapi/handlers/control.ts`:
- Line 22: Update updateDurable and its related credential replacement flow to
snapshot the complete list returned by credentials.list rather than only
.at(-1), then perform removal and restoration atomically in a single transaction
where supported. If deletion or auth.remove fails, restore every original
credential and preserve the existing replacement behavior on success.
In `@packages/opencode/test/server/httpapi-provider.test.ts`:
- Line 450: Extend the final DELETE test around the auth credential request to
verify that the updated credential is actually removed from durable state, not
only that the response status is 200. Reuse the existing credential retrieval or
persistence assertion symbols in the test to confirm the credential is absent
after the DELETE.
---
Nitpick comments:
In `@packages/core/test/session-runner-anthropic-subscription.test.ts`:
- Line 25: Replace the star import of the LLM runner with the module’s exported
SessionRunnerLLM namespace and continue referencing it through that namespace.
If the namespace is not currently exported by the runner module, add the
prescribed self-reexport there before updating the test import.
In `@packages/opencode/src/cli/cmd/providers.ts`:
- Around line 306-321: Extract the duplicated subscription-auth injection logic
from the current command and ProvidersLogoutCommand into a shared helper such as
withSyntheticSubscriptionAuth(entries). Have the helper fetch the latest
Anthropic subscription credential, add the synthetic OAuth entry only when
appropriate and absent, then update both commands to use it while preserving
their existing results flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31f8fb8c-af57-4093-ab68-19b6e293f863
📒 Files selected for processing (30)
packages/core/src/integration.tspackages/core/src/plugin/provider.tspackages/core/src/plugin/provider/anthropic-subscription.tspackages/core/src/session/runner/model.tspackages/core/test/plugin/provider-anthropic-subscription.test.tspackages/core/test/session-runner-anthropic-subscription.test.tspackages/core/test/session-runner-model.test.tspackages/llm/package.jsonpackages/llm/src/providers/anthropic-subscription.tspackages/llm/src/providers/index.tspackages/llm/test/provider/anthropic-subscription.test.tspackages/opencode/src/cli/cmd/providers.tspackages/opencode/src/plugin/anthropic/subscription.tspackages/opencode/src/plugin/index.tspackages/opencode/src/plugin/openai/codex.tspackages/opencode/src/provider/anthropic-subscription-credential.tspackages/opencode/src/provider/auth.tspackages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.tspackages/opencode/src/server/routes/instance/httpapi/handlers/control.tspackages/opencode/src/server/routes/instance/httpapi/handlers/provider.tspackages/opencode/src/session/llm/native-runtime.tspackages/opencode/test/plugin/anthropic-subscription.test.tspackages/opencode/test/plugin/codex.test.tspackages/opencode/test/provider/provider.test.tspackages/opencode/test/server/httpapi-provider.test.tspackages/opencode/test/session/llm-native.test.tspackages/tui/src/component/dialog-provider.tsxpackages/tui/src/context/sync.tsxpackages/tui/test/cli/cmd/tui/provider-options.test.ts
Comment on lines
+420
to
+425
| if ( | ||
| latest.value.access !== oauth.access || | ||
| latest.value.refresh !== oauth.refresh || | ||
| latest.value.expires !== oauth.expires | ||
| ) | ||
| return latest.value |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Include methodID in both compare-and-swap checks.
A concurrent OAuth-method change can retain identical token fields, pass these guards, and then be reverted by the stale refresh result.
Proposed fix
if (
+ latest.value.methodID !== oauth.methodID ||
latest.value.access !== oauth.access ||
latest.value.refresh !== oauth.refresh ||
latest.value.expires !== oauth.expires
)
if (
+ current.value.methodID !== latest.value.methodID ||
current.value.access !== latest.value.access ||
current.value.refresh !== latest.value.refresh ||
current.value.expires !== latest.value.expires
)Also applies to: 436-443
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/integration.ts` around lines 420 - 425, Update both
compare-and-swap guards in the OAuth refresh flow around the visible token
comparisons to also compare the current OAuth value’s methodID with
oauth.methodID. Return the latest value whenever access, refresh, expires, or
methodID differs, preventing stale refresh results from overwriting a concurrent
OAuth-method change.
Comment on lines
+39
to
+56
| export function refreshAnthropicSubscriptionToken(input: { | ||
| readonly refresh: string | ||
| readonly request?: Fetch | ||
| readonly tokenEndpoint?: string | ||
| }) { | ||
| const endpoint = input.tokenEndpoint ?? tokenEndpoint | ||
| const key = `${endpoint}\u0000${input.refresh}` | ||
| const current = refreshRequests.get(key) | ||
| if (current) return current | ||
| const pending = tokenRequestPromise(input.request ?? fetch, endpoint, { | ||
| grant_type: "refresh_token", | ||
| refresh_token: input.refresh, | ||
| client_id: clientID, | ||
| }).finally(() => { | ||
| if (refreshRequests.get(key) === pending) refreshRequests.delete(key) | ||
| }) | ||
| refreshRequests.set(key, pending) | ||
| return pending |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the shared refresh request with a timeout.
This request has no cancellation or timeout. If the token endpoint stalls, the shared Promise and integration Deferred never settle, so every later credential resolution hangs. Apply one timeout to the shared request, abort it on expiry, and test a never-resolving endpoint.
Also applies to: 111-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/plugin/provider/anthropic-subscription.ts` around lines 39
- 56, The shared request created by refreshAnthropicSubscriptionToken must be
bounded so a stalled token endpoint cannot leave the cached Promise pending. Add
one timeout around tokenRequestPromise, abort the underlying request when it
expires, ensure cleanup still removes the matching pending entry, and add
coverage using a never-resolving endpoint to verify the Promise and integration
Deferred settle.
Comment on lines
+167
to
+173
| if (credential?.type !== "oauth") | ||
| return Effect.fail( | ||
| new CredentialRequiredError({ | ||
| providerID: resolved.providerID, | ||
| modelID: resolved.id, | ||
| }), | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate the OAuth method before forwarding the token.
Checking only credential.type allows an OAuth token from another integration to be sent to Anthropic. Require credential.methodID === AnthropicSubscriptionMethodID; otherwise return CredentialRequiredError.
Proposed guard
- if (credential?.type !== "oauth")
+ if (
+ credential?.type !== "oauth" ||
+ credential.methodID !== AnthropicSubscriptionMethodID
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (credential?.type !== "oauth") | |
| return Effect.fail( | |
| new CredentialRequiredError({ | |
| providerID: resolved.providerID, | |
| modelID: resolved.id, | |
| }), | |
| ) | |
| if ( | |
| credential?.type !== "oauth" || | |
| credential.methodID !== AnthropicSubscriptionMethodID | |
| ) | |
| return Effect.fail( | |
| new CredentialRequiredError({ | |
| providerID: resolved.providerID, | |
| modelID: resolved.id, | |
| }), | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/session/runner/model.ts` around lines 167 - 173, Update the
credential guard in the model execution flow to require both OAuth type and
credential.methodID === AnthropicSubscriptionMethodID before forwarding the
token; otherwise return the existing CredentialRequiredError with the current
providerID and modelID values.
| @@ -0,0 +1,249 @@ | |||
| import { Effect, Stream } from "effect" | |||
| import { Headers } from "effect/unstable/http" | |||
| import * as AnthropicMessages from "../protocols/anthropic-messages" | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace the star import with the exported namespace.
Import the module’s named namespace from its public module rather than using import * as.
As per coding guidelines, “Never use star imports… For namespace-style values, import the module's exported namespace by name.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/llm/src/providers/anthropic-subscription.ts` at line 3, Replace the
star import for AnthropicMessages with a named namespace import from the public
anthropic-messages module, preserving the existing AnthropicMessages references
in the provider.
Source: Coding guidelines
Comment on lines
+161
to
+165
| export const restoreToolNames = (frame: string, names?: ReadonlyMap<string, string>) => | ||
| frame.replace( | ||
| /"name"\s*:\s*"mcp_([^"]+)"/g, | ||
| (_match, name: string) => `"name":${JSON.stringify(names?.get(`mcp_${name}`) ?? unprefixToolName(name))}`, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Leave unmapped mcp_ tool names unchanged.
The fallback renames every unknown mcp_Foo to foo, potentially changing a hosted or provider-defined tool’s identity. Only restore names found in the request’s mapping.
Proposed fix
- (_match, name: string) => `"name":${JSON.stringify(names?.get(`mcp_${name}`) ?? unprefixToolName(name))}`,
+ (match, name: string) => {
+ const restored = names?.get(`mcp_${name}`)
+ if (restored === undefined && names !== undefined) return match
+ return `"name":${JSON.stringify(restored ?? unprefixToolName(name))}`
+ },As per coding guidelines, “Provider-defined or hosted tools must pass through untouched.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/llm/src/providers/anthropic-subscription.ts` around lines 161 - 165,
Update restoreToolNames so only mcp_ names present in the names mapping are
restored; when no mapping exists, preserve the original mcp_ tool name
unchanged. Keep the existing mapped-name replacement behavior and avoid applying
unprefixToolName as a fallback.
Source: Coding guidelines
Comment on lines
+604
to
+633
| if (provider === AnthropicSubscriptionProviderID) { | ||
| yield* withAnthropicSubscriptionCredentialLock( | ||
| Effect.gen(function* () { | ||
| const previous = yield* Effect.gen(function* () { | ||
| const credentials = yield* Credential.Service | ||
| const stored = yield* credentials.list(AnthropicSubscriptionIntegrationID) | ||
| for (const credential of stored) { | ||
| yield* credentials.remove(credential.id) | ||
| } | ||
| return stored.at(-1) | ||
| }).pipe(Effect.provide(credentialLayer)) | ||
| yield* Effect.orDie(authSvc.remove(provider)).pipe( | ||
| Effect.onError(() => | ||
| previous | ||
| ? Effect.gen(function* () { | ||
| const credentials = yield* Credential.Service | ||
| yield* credentials.create({ | ||
| integrationID: previous.integrationID, | ||
| label: previous.label, | ||
| value: previous.value, | ||
| }) | ||
| }).pipe(Effect.provide(credentialLayer)) | ||
| : Effect.void, | ||
| ), | ||
| ) | ||
| }), | ||
| ) | ||
| yield* Prompt.outro("Logout successful") | ||
| return | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Logout rollback only restores the last of possibly several deleted credential rows.
The code removes every row returned by credentials.list(AnthropicSubscriptionIntegrationID) but the rollback-on-auth.remove-failure path only recreates stored.at(-1). If more than one row existed (e.g. leftover from an earlier race), an auth.remove failure permanently loses the earlier rows even though the operation is meant to be reversible.
🐛 Proposed fix to restore all removed rows
- const previous = yield* Effect.gen(function* () {
- const credentials = yield* Credential.Service
- const stored = yield* credentials.list(AnthropicSubscriptionIntegrationID)
- for (const credential of stored) {
- yield* credentials.remove(credential.id)
- }
- return stored.at(-1)
- }).pipe(Effect.provide(credentialLayer))
+ const previous = yield* Effect.gen(function* () {
+ const credentials = yield* Credential.Service
+ const stored = yield* credentials.list(AnthropicSubscriptionIntegrationID)
+ for (const credential of stored) {
+ yield* credentials.remove(credential.id)
+ }
+ return stored
+ }).pipe(Effect.provide(credentialLayer))
yield* Effect.orDie(authSvc.remove(provider)).pipe(
Effect.onError(() =>
- previous
+ previous.length > 0
? Effect.gen(function* () {
const credentials = yield* Credential.Service
- yield* credentials.create({
- integrationID: previous.integrationID,
- label: previous.label,
- value: previous.value,
- })
+ for (const p of previous) {
+ yield* credentials.create({ integrationID: p.integrationID, label: p.label, value: p.value })
+ }
}).pipe(Effect.provide(credentialLayer))
: Effect.void,
),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (provider === AnthropicSubscriptionProviderID) { | |
| yield* withAnthropicSubscriptionCredentialLock( | |
| Effect.gen(function* () { | |
| const previous = yield* Effect.gen(function* () { | |
| const credentials = yield* Credential.Service | |
| const stored = yield* credentials.list(AnthropicSubscriptionIntegrationID) | |
| for (const credential of stored) { | |
| yield* credentials.remove(credential.id) | |
| } | |
| return stored.at(-1) | |
| }).pipe(Effect.provide(credentialLayer)) | |
| yield* Effect.orDie(authSvc.remove(provider)).pipe( | |
| Effect.onError(() => | |
| previous | |
| ? Effect.gen(function* () { | |
| const credentials = yield* Credential.Service | |
| yield* credentials.create({ | |
| integrationID: previous.integrationID, | |
| label: previous.label, | |
| value: previous.value, | |
| }) | |
| }).pipe(Effect.provide(credentialLayer)) | |
| : Effect.void, | |
| ), | |
| ) | |
| }), | |
| ) | |
| yield* Prompt.outro("Logout successful") | |
| return | |
| } | |
| if (provider === AnthropicSubscriptionProviderID) { | |
| yield* withAnthropicSubscriptionCredentialLock( | |
| Effect.gen(function* () { | |
| const previous = yield* Effect.gen(function* () { | |
| const credentials = yield* Credential.Service | |
| const stored = yield* credentials.list(AnthropicSubscriptionIntegrationID) | |
| for (const credential of stored) { | |
| yield* credentials.remove(credential.id) | |
| } | |
| return stored | |
| }).pipe(Effect.provide(credentialLayer)) | |
| yield* Effect.orDie(authSvc.remove(provider)).pipe( | |
| Effect.onError(() => | |
| previous.length > 0 | |
| ? Effect.gen(function* () { | |
| const credentials = yield* Credential.Service | |
| for (const p of previous) { | |
| yield* credentials.create({ integrationID: p.integrationID, label: p.label, value: p.value }) | |
| } | |
| }).pipe(Effect.provide(credentialLayer)) | |
| : Effect.void, | |
| ), | |
| ) | |
| }), | |
| ) | |
| yield* Prompt.outro("Logout successful") | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/opencode/src/cli/cmd/providers.ts` around lines 604 - 633, Update
the Anthropic subscription logout flow around the stored credential collection
and authSvc.remove rollback so every credential removed from stored is recreated
when removal fails, preserving each credential’s integrationID, label, and
value. Replace the single stored.at(-1) rollback input with iteration over all
stored rows while retaining the existing credentialLayer and no-op behavior when
none were removed.
Comment on lines
+254
to
+259
| const latest = await getAuth() | ||
| if (latest?.type === "oauth" && latest.access && latest.access !== auth.access) { | ||
| response = await send(latest.access) | ||
| } else { | ||
| const refreshed = await refresh(latest?.type === "oauth" ? latest.refresh : auth.refresh) | ||
| response = await send(refreshed.access) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Stop the 401 retry when the credential has been removed.
When getAuth() returns undefined after a concurrent logout, this branch refreshes using auth.refresh. That continues using a removed credential and contacts the token endpoint after logout. Fail as disconnected before refreshing.
Proposed fix
const latest = await getAuth()
-if (latest?.type === "oauth" && latest.access && latest.access !== auth.access) {
+if (latest?.type !== "oauth") {
+ throw new Error("Anthropic subscription is disconnected; reconnect with /connect")
+}
+if (latest.access && latest.access !== auth.access) {
response = await send(latest.access)
} else {
- const refreshed = await refresh(latest?.type === "oauth" ? latest.refresh : auth.refresh)
+ const refreshed = await refresh(latest.refresh)
response = await send(refreshed.access)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const latest = await getAuth() | |
| if (latest?.type === "oauth" && latest.access && latest.access !== auth.access) { | |
| response = await send(latest.access) | |
| } else { | |
| const refreshed = await refresh(latest?.type === "oauth" ? latest.refresh : auth.refresh) | |
| response = await send(refreshed.access) | |
| const latest = await getAuth() | |
| if (latest?.type !== "oauth") { | |
| throw new Error("Anthropic subscription is disconnected; reconnect with /connect") | |
| } | |
| if (latest.access && latest.access !== auth.access) { | |
| response = await send(latest.access) | |
| } else { | |
| const refreshed = await refresh(latest.refresh) | |
| response = await send(refreshed.access) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/opencode/src/plugin/anthropic/subscription.ts` around lines 254 -
259, Update the 401 retry handling around getAuth so an undefined latest
credential is treated as disconnected and fails immediately. Only call refresh
and send with latest.refresh or auth.refresh when an existing credential is
available; preserve the access-token replacement path for valid changed OAuth
credentials.
Comment on lines
+1388
to
+1424
| const storedAuth = Effect.fn("Provider.storedAuth")(function* (providerID: ProviderV2.ID) { | ||
| if (providerID !== AnthropicSubscriptionProviderID) return yield* auth.get(providerID).pipe(Effect.orDie) | ||
| return yield* withAnthropicSubscriptionCredentialLock( | ||
| Effect.gen(function* () { | ||
| const credential = (yield* credentials.list(AnthropicSubscriptionIntegrationID)).at(-1) | ||
| if (credential?.value.type === "oauth") { | ||
| if (yield* auth.get(providerID).pipe(Effect.orDie)) yield* auth.remove(providerID).pipe(Effect.orDie) | ||
| return { | ||
| type: "oauth" as const, | ||
| access: credential.value.access, | ||
| refresh: credential.value.refresh, | ||
| expires: credential.value.expires, | ||
| } | ||
| } | ||
| const legacy = yield* auth.get(providerID).pipe(Effect.orDie) | ||
| if (legacy?.type !== "oauth") return legacy | ||
| yield* credentials.create({ | ||
| integrationID: AnthropicSubscriptionIntegrationID, | ||
| label: "Claude Pro/Max", | ||
| value: Credential.OAuth.make({ | ||
| type: "oauth", | ||
| methodID: AnthropicSubscriptionMethodID, | ||
| access: legacy.access, | ||
| refresh: legacy.refresh, | ||
| expires: legacy.expires, | ||
| }), | ||
| }) | ||
| yield* auth.remove(providerID).pipe(Effect.orDie) | ||
| return { | ||
| type: "oauth" as const, | ||
| access: legacy.access, | ||
| refresh: legacy.refresh, | ||
| expires: legacy.expires, | ||
| } | ||
| }), | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Lock and DB read on every Claude Pro/Max request serialize concurrent traffic.
storedAuth is passed as the plugin's getAuth callback (line 1630), which the anthropic-subscription plugin invokes on every outgoing request (including inside its fetch wrapper). For AnthropicSubscriptionProviderID, the entire body — including the common already-migrated fast path — runs under withAnthropicSubscriptionCredentialLock, a process-wide 1-permit semaphore, plus does a credentials.list read (and sometimes an extra auth.get) every call. This serializes all concurrent Claude Pro/Max requests (e.g. parallel subagents/tool calls) through a single lock even though the common case is a pure read. Line 1623's extra storedAuth(providerID) call (on top of what the loader itself invokes via getAuth) compounds this. Consider only locking the legacy-migration write path and letting the fast, already-migrated read bypass the semaphore.
⚡ Proposed narrower locking
const storedAuth = Effect.fn("Provider.storedAuth")(function* (providerID: ProviderV2.ID) {
if (providerID !== AnthropicSubscriptionProviderID) return yield* auth.get(providerID).pipe(Effect.orDie)
- return yield* withAnthropicSubscriptionCredentialLock(
- Effect.gen(function* () {
- const credential = (yield* credentials.list(AnthropicSubscriptionIntegrationID)).at(-1)
- if (credential?.value.type === "oauth") {
- if (yield* auth.get(providerID).pipe(Effect.orDie)) yield* auth.remove(providerID).pipe(Effect.orDie)
- return {
- type: "oauth" as const,
- access: credential.value.access,
- refresh: credential.value.refresh,
- expires: credential.value.expires,
- }
- }
- const legacy = yield* auth.get(providerID).pipe(Effect.orDie)
+ const credential = (yield* credentials.list(AnthropicSubscriptionIntegrationID)).at(-1)
+ if (credential?.value.type === "oauth") {
+ return {
+ type: "oauth" as const,
+ access: credential.value.access,
+ refresh: credential.value.refresh,
+ expires: credential.value.expires,
+ }
+ }
+ return yield* withAnthropicSubscriptionCredentialLock(
+ Effect.gen(function* () {
+ const legacy = yield* auth.get(providerID).pipe(Effect.orDie)
if (legacy?.type !== "oauth") return legacy
yield* credentials.create({ /* ... */ })
yield* auth.remove(providerID).pipe(Effect.orDie)
return { type: "oauth" as const, access: legacy.access, refresh: legacy.refresh, expires: legacy.expires }
}),
)
})(Residual legacy-auth cleanup on the already-migrated path can be dropped or moved to a rare, separately-locked reconciliation step.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/opencode/src/provider/provider.ts` around lines 1388 - 1424, Update
Provider.storedAuth so the common already-migrated OAuth credential path reads
and returns the credential without entering
withAnthropicSubscriptionCredentialLock. Keep the lock only around legacy-auth
migration and its credential creation/removal writes, and remove any unnecessary
auth.get cleanup from the fast path; preserve existing behavior for
non-Anthropic providers and non-OAuth credentials.
| function updateDurable(value?: Auth.Info) { | ||
| return Effect.gen(function* () { | ||
| const credentials = yield* Credential.Service | ||
| const previous = (yield* credentials.list(AnthropicSubscriptionIntegrationID)).at(-1) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make durable credential replacement atomic and restore the complete snapshot.
updateDurable() remembers only .at(-1) but removes every credential individually. A deletion failure, or a later auth.remove failure, can permanently lose all records except the last one. Snapshot and restore the full list, ideally within one transaction.
Also applies to: 40-58, 75-77, 94-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/opencode/src/server/routes/instance/httpapi/handlers/control.ts` at
line 22, Update updateDurable and its related credential replacement flow to
snapshot the complete list returned by credentials.list rather than only
.at(-1), then perform removal and restoration atomically in a single transaction
where supported. If deletion or auth.remove fails, restore every original
credential and preserve the existing replacement behavior on success.
| access: "access-from-control", | ||
| refresh: "refresh-from-control", | ||
| }) | ||
| expect((yield* request("/auth/anthropic-subscription", { method: "DELETE", headers })).status).toBe(200) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that the final DELETE removes the updated credential.
A 200 response alone does not verify durable state after the PUT path.
Proposed assertion
expect((yield* request("/auth/anthropic-subscription", { method: "DELETE", headers })).status).toBe(200)
+expect(
+ yield* Effect.gen(function* () {
+ return yield* (yield* Credential.Service).list(AnthropicSubscriptionIntegrationID)
+ }).pipe(Effect.provide(AppNodeBuilder.build(Credential.node))),
+).toEqual([])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect((yield* request("/auth/anthropic-subscription", { method: "DELETE", headers })).status).toBe(200) | |
| expect((yield* request("/auth/anthropic-subscription", { method: "DELETE", headers })).status).toBe(200) | |
| expect( | |
| yield* Effect.gen(function* () { | |
| return yield* (yield* Credential.Service).list(AnthropicSubscriptionIntegrationID) | |
| }).pipe(Effect.provide(AppNodeBuilder.build(Credential.node))), | |
| ).toEqual([]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/opencode/test/server/httpapi-provider.test.ts` at line 450, Extend
the final DELETE test around the auth credential request to verify that the
updated credential is actually removed from durable state, not only that the
response status is 200. Reuse the existing credential retrieval or persistence
assertion symbols in the test to confirm the credential is absent after the
DELETE.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
anthropic-subscriptionproviderValidation
packages/llm: typecheck + 303 tests passed (30 skipped)packages/tui: typecheck + 209 tests passed (1 skipped)packages/core: typecheck + 29 focused tests passedpackages/opencode: typecheck + 45 focused tests passedgit diff --checkpassedNotes
The request cannot be processed./connect.Summary by CodeRabbit
New Features
Bug Fixes
Tests