fix(mcp): address oauth review notes

Author: zerob13

docs/issues/pr-1870-review-fixes/plan.md

@@ -0,0 +1,20 @@
+# Plan
+
+## Implementation
+
+1. Patch the smallest root locations:
+   - `mcpOAuthManager.ts` for OAuth error shape checks and stale-flow guard.
+   - `mcpOAuthProvider.ts` for authorization URL scheme validation.
+   - `oauthCredentialStore.ts` for scoped deletion persistence.
+   - renderer OAuth callback handlers for in-flight guards.
+   - `McpServerCard.vue` for the auth-error label.
+   - shared contracts for stricter callback/status validation.
+2. Replace the source-text sheet test with a mounted DOM assertion.
+3. Update the new OpenAI Codex/MCP auth strings in affected locale JSON files.
+
+## Test Strategy
+
+- Extend MCP OAuth manager tests for HTTP status classification.
+- Add a credential store scoped-clear test.
+- Update the SheetContent drag test to mount the component.
+- Run focused tests for MCP OAuth/store/renderer components plus format, i18n, lint, and typecheck.

docs/issues/pr-1870-review-fixes/spec.md

@@ -0,0 +1,34 @@
+# PR 1870 Review Fixes
+
+## User Need
+
+PR review comments on MCP and Codex OAuth should be verified against the current code and fixed
+where they point to real bugs or user-visible polish issues.
+
+## Goal
+
+Address valid review feedback for OAuth error classification, stale OAuth flow handling, callback
+submission guards, URL validation, credential clearing, status labels, rendered drag-region tests,
+and newly added locale strings.
+
+## Acceptance Criteria
+
+- MCP OAuth classifies HTTP 401 errors from common error shapes.
+- Superseded MCP OAuth flows cannot complete and overwrite the active flow.
+- Invalid pasted MCP callback URLs return the error status created by the failure path.
+- MCP OAuth authorization opens only `http:` or `https:` URLs.
+- Scoped MCP OAuth credential clearing actually removes the targeted scope.
+- OpenAI Codex and MCP callback submissions cannot double-submit while already in flight.
+- MCP auth error cards show the failure label, not the required-auth label.
+- The Sheet no-drag regression test checks rendered DOM.
+- Newly added auth strings are localized in the affected locale files.
+
+## Non-Goals
+
+- Do not redesign OAuth.
+- Do not add a new permission model.
+- Do not translate unrelated legacy strings.
+
+## Open Questions
+
+None.

docs/issues/pr-1870-review-fixes/tasks.md

@@ -0,0 +1,9 @@
+# Tasks
+
+- [x] Fetch and verify PR review comments.
+- [x] Patch OAuth manager/provider/store issues.
+- [x] Patch renderer duplicate-submit and status-label issues.
+- [x] Tighten route/event schemas.
+- [x] Replace sheet source-text test with DOM assertion.
+- [x] Localize new auth strings in affected locales.
+- [x] Run focused tests and required validation commands.

src/main/presenter/mcpPresenter/mcpOAuthManager.ts

@@ -66,9 +66,21 @@ function isOAuthError(error: unknown): boolean {
     return true
   }
 
-  const status = (error as { code?: unknown; status?: unknown; httpStatus?: unknown } | undefined)
-    ?.code
-  if (status === 401) {
+  const errorLike = error as
+    | {
+        code?: unknown
+        status?: unknown
+        httpStatus?: unknown
+        response?: { status?: unknown }
+      }
+    | undefined
+  const statuses = [
+    errorLike?.code,
+    errorLike?.status,
+    errorLike?.httpStatus,
+    errorLike?.response?.status
+  ]
+  if (statuses.some((status) => status === 401 || status === '401')) {
     return true
   }
 
@@ -261,14 +273,15 @@ export class McpOAuthManager {
 
     const resolution = flow.callbackSession.resolveCallbackUrl(callbackUrl)
     if (resolution.kind === 'not-found') {
-      this.setStatus({
+      const status: McpServerAuthStatus = {
         serverName,
         state: 'error',
         authenticated: false,
         error: 'MCP OAuth callback URL is invalid',
         storage: this.store.getStorageState()
-      })
-      return this.getStatus(serverName, config)
+      }
+      this.setStatus(status)
+      return status
     }
 
     await flow.flowPromise
@@ -302,6 +315,10 @@ export class McpOAuthManager {
   }
 
   private finishAuthenticatedFlow(flow: PendingMcpOAuthFlow): void {
+    if (this.pendingFlows.get(flow.serverName) !== flow) {
+      return
+    }
+
     this.pendingFlows.delete(flow.serverName)
     flow.callbackSession.close()
     const entry = this.store.load(flow.credentialKey)

src/main/presenter/mcpPresenter/mcpOAuthProvider.ts

@@ -69,6 +69,9 @@ export class DeepChatMcpOAuthProvider implements OAuthClientProvider {
     if (!this.interactive) {
       throw new Error('MCP OAuth authentication is required')
     }
+    if (authorizationUrl.protocol !== 'http:' && authorizationUrl.protocol !== 'https:') {
+      throw new Error('MCP OAuth authorization URL must use http or https')
+    }
 
     await shell.openExternal(authorizationUrl.toString())
   }

src/main/presenter/mcpPresenter/oauthCredentialStore.ts

@@ -82,7 +82,8 @@ export class McpOAuthCredentialStore {
       return
     }
 
-    const current = this.load(key)
+    const entries = this.loadAll()
+    const current = entries[key]
     if (!current) {
       return
     }
@@ -98,7 +99,8 @@ export class McpOAuthCredentialStore {
       delete next.discoveryState
     }
 
-    this.saveEntry(key, next)
+    entries[key] = next
+    this.saveAll(entries)
   }
 
   private loadAll(): McpOAuthCredentialData {

src/renderer/settings/components/OpenAICodexOAuth.vue

@@ -285,6 +285,10 @@ const startBrowserLogin = () =>
   runAuthAction('browser', () => oauthClient.startOpenAICodexBrowserLogin())
 
 const completeBrowserLoginFromUrl = () => {
+  if (busyAction.value === 'callback') {
+    return
+  }
+
   const url = callbackUrl.value.trim()
   if (!url) {
     return

src/renderer/src/components/mcp-config/components/McpServerCard.vue

@@ -116,7 +116,7 @@ const statusConfig = computed(() => {
     case 'auth-error':
       return {
         dot: 'bg-red-500',
-        text: t('settings.mcp.authRequired'),
+        text: t('settings.mcp.authFailed'),
         color: 'text-red-600 dark:text-red-400'
       }
     case 'error':

src/renderer/src/components/mcp-config/components/McpServers.vue

@@ -248,6 +248,10 @@ const handleAuthenticateServer = async (serverName: string) => {
 }
 
 const submitAuthCallbackUrl = async () => {
+  if (isSubmittingAuthCallback.value) {
+    return
+  }
+
   const serverName = selectedServerForAuth.value
   const callbackUrl = authCallbackUrl.value.trim()
   if (!serverName || !callbackUrl) {

src/renderer/src/i18n/da-DK/settings.json

@@ -1064,11 +1064,11 @@
         "advanced": "Advanced"
       }
     },
-    "openaiCodexPasteCallback": "Paste callback URL",
-    "openaiCodexCallbackTitle": "Complete authentication",
-    "openaiCodexCallbackDescription": "After the browser finishes, return to DeepChat. If DeepChat does not update, paste the full callback URL here.",
-    "openaiCodexCallbackPlaceholder": "Paste the full callback URL",
-    "openaiCodexCompleteAuthentication": "Complete authentication"
+    "openaiCodexPasteCallback": "Indsæt callback-URL",
+    "openaiCodexCallbackTitle": "Fuldfør godkendelse",
+    "openaiCodexCallbackDescription": "Når browseren er færdig, skal du vende tilbage til DeepChat. Hvis DeepChat ikke opdateres, skal du indsætte den fulde callback-URL her.",
+    "openaiCodexCallbackPlaceholder": "Indsæt den fulde callback-URL",
+    "openaiCodexCompleteAuthentication": "Fuldfør godkendelse"
   },
   "knowledgeBase": {
     "title": "Indstillinger for vidensbase",
@@ -1387,15 +1387,15 @@
         "custom": "Custom"
       }
     },
-    "authRequired": "Authentication required",
-    "authenticate": "Authenticate",
-    "authFailed": "Authentication failed",
-    "authCallbackTitle": "Complete authentication",
-    "authCallbackDescription": "After the browser finishes, return to DeepChat. If DeepChat does not update, paste the full callback URL here.",
-    "authCallbackPlaceholder": "Paste the full callback URL",
-    "completeAuthentication": "Complete authentication",
-    "saveSuccess": "Configuration saved",
-    "saveFailed": "Failed to save configuration"
+    "authRequired": "Godkendelse påkrævet",
+    "authenticate": "Godkend",
+    "authFailed": "Godkendelse mislykkedes",
+    "authCallbackTitle": "Fuldfør godkendelse",
+    "authCallbackDescription": "Når browseren er færdig, skal du vende tilbage til DeepChat. Hvis DeepChat ikke opdateres, skal du indsætte den fulde callback-URL her.",
+    "authCallbackPlaceholder": "Indsæt den fulde callback-URL",
+    "completeAuthentication": "Fuldfør godkendelse",
+    "saveSuccess": "Konfiguration gemt",
+    "saveFailed": "Kunne ikke gemme konfiguration"
   },
   "scheduledTasks": {
     "title": "Planlagte opgaver",