fix(oauth,settings): forward upstream oauth error contract and persist recovered config - #331
Merged
Merged
Conversation
The route boundary collapsed every upstream OAuth failure into a single generic message, leaving all six mappings in the frontend's mapOAuthError unreachable. Classify the failure and forward the matched category so clients surface specific guidance. Hoist the category list into @opencode-manager/shared as the single source of truth for both sides; the frontend map is now typed Record<OAuthErrorCategory, string> so a missing message fails typecheck.
…bstrings
The previous mapping matched invented substrings ("invalid code", "expired",
"access denied", ...) that opencode never emits, so every real failure fell
through to the generic message and left the frontend map unreachable. It also
returned 500 for what upstream reports as 400.
opencode serialises these failures as a structured contract: ProviderAuthError
({ name, data }) or InvalidRequestError ({ _tag, message }) at HTTP 400. Model
both shapes in shared, discriminate on name/_tag, and forward the exact code
plus upstream detail so clients can map errors precisely.
Pin the contract to upstream with a compile-time conformance assertion against
@opencode-ai/sdk's generated ProviderOauthAuthorizeErrors[400] and
ProviderOauthCallbackErrors[400], added as a type-only devDependency. Any
added, removed, renamed, or retyped upstream variant now fails typecheck.
Also stop discarding the structured error at the frontend boundary, where
handleApiError rewrapped FetchError in a bare Error and dropped the code that
fetchWrapper had already parsed.
Verified against a live opencode 1.18.7 server: callback without a pending
authorization returns 400 ProviderAuthOauthMissing and is now forwarded as
such. Tests are pinned to captured responses, including the multiline message
BadRequest carries.…api/npm fields Applying a default config with auto-removed fields wrote the stripped content to disk but left the stored record holding the rejected fields, so the database and the on-disk config disagreed until the next write. Persist the applied content and return 409 when the config disappears mid-recovery. Add the provider-level api, npm, whitelist, and blacklist fields so valid opencode provider configs survive a parse round-trip instead of being stripped.
Contributor
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughOAuth failures now use shared typed contracts, backend status/detail translation, and frontend code-based messages. Provider configuration schemas preserve additional fields, while recovered default-config updates persist sanitized content and handle concurrent removal with ChangesOAuth error flow
Configuration management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OAuthClient
participant oauthApi
participant OAuthRoutes
participant openCodeClient
OAuthClient->>oauthApi: Start authorize or callback request
oauthApi->>OAuthRoutes: Send OAuth payload
OAuthRoutes->>openCodeClient: Forward OAuth operation
openCodeClient-->>OAuthRoutes: Return upstream response
OAuthRoutes-->>oauthApi: Return success or structured failure
oauthApi-->>OAuthClient: Expose response or FetchError
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The OAuth route collapsed every upstream failure into a single generic message and matched invented substrings opencode never emits, leaving the frontend error map unreachable. Model opencode's structured ProviderAuthError / InvalidRequestError contract in @opencode-manager/shared, discriminate by name/_tag, and forward the exact code plus upstream detail so clients can map precisely. Pin the contract to @opencode-ai/sdk with a compile-time conformance assertion.
Separately, applying a default config with auto-removed fields wrote the stripped content to disk but left the stored record holding the rejected fields, so disk and database disagreed. Persist the applied content, return 409 when the config disappears mid-recovery, and accept provider-level api/npm/whitelist/blacklist fields so valid opencode provider configs survive a parse round-trip.
Summary
Type of Change
Checklist
pnpm lintpasses locallypnpm typecheckpasses locallySummary by CodeRabbit
New Features
Bug Fixes
Tests