From a90720dc7e00eefd0a92f32ef8031df5fda10b4a Mon Sep 17 00:00:00 2001 From: Mux Date: Sat, 5 Sep 2026 20:21:57 -0500 Subject: [PATCH 01/24] =?UTF-8?q?[openai]=20=F0=9F=A4=96=20feat:=20add=20p?= =?UTF-8?q?roject-specific=20Codex=20OAuth=20accounts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Support named accounts, project selection, and durable token refresh. --- _Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `high` • Cost: `$45.65`_ --- src/browser/App.tsx | 18 +- src/browser/components/ChatPane/ChatPane.tsx | 16 +- .../ThinkingSelector/ThinkingSelector.tsx | 5 + src/browser/features/ChatInput/index.tsx | 24 +- .../ChatInput/useCreationWorkspace.ts | 1 + .../RightSidebar/ContextUsageSection.tsx | 14 +- .../Settings/Sections/CodexAccounts.tsx | 452 +++++++++++++ .../Settings/Sections/ProvidersSection.tsx | 472 +------------ src/browser/hooks/useContextSwitchWarning.ts | 38 +- src/browser/hooks/useWorkspaceName.ts | 7 +- .../stories/App.codexAccounts.stories.tsx | 298 ++++++++ src/browser/utils/commands/sources.ts | 3 + .../compaction/contextSwitchCheck.test.ts | 18 + .../utils/compaction/contextSwitchCheck.ts | 13 +- src/browser/utils/fastModeServiceTier.test.ts | 24 + src/browser/utils/fastModeServiceTier.ts | 1 + src/cli/run.ts | 18 +- src/cli/trust.test.ts | 57 +- src/cli/trust.ts | 39 +- src/cli/workflow.ts | 33 +- src/common/constants/codexOauthAccounts.ts | 9 + src/common/orpc/schemas/api.test.ts | 33 + src/common/orpc/schemas/api.ts | 51 +- src/common/schemas/project.ts | 2 + .../ai/openaiProviderOptionsAvailability.ts | 2 + .../utils/providers/codexOauthRouting.test.ts | 76 +++ .../utils/providers/codexOauthRouting.ts | 33 +- .../utils/tokens/tokenMeterUtils.test.ts | 31 + src/common/utils/tokens/tokenMeterUtils.ts | 7 +- src/node/config.test.ts | 19 + src/node/config/fileLeaseManager.ts | 16 + src/node/config/index.ts | 13 + src/node/orpc/router.ts | 48 +- src/node/services/agentStatusService.ts | 1 + src/node/services/aiService.test.ts | 51 ++ src/node/services/aiService.ts | 3 +- src/node/services/codexOauthService.test.ts | 637 +++++++++++++++++- src/node/services/codexOauthService.ts | 424 +++++++++--- src/node/services/projectService.test.ts | 36 + src/node/services/projectService.ts | 24 + .../services/providerModelFactory.test.ts | 321 ++++++++- src/node/services/providerModelFactory.ts | 111 +-- src/node/services/providerService.test.ts | 77 +++ src/node/services/providerService.ts | 29 +- .../tools/shared/configRedaction.test.ts | 32 + .../services/tools/shared/configRedaction.ts | 1 + src/node/services/turnRequestBuilder.ts | 12 +- src/node/services/workspaceService.test.ts | 2 + src/node/services/workspaceService.ts | 12 +- .../services/workspaceStatusGenerator.test.ts | 22 +- src/node/services/workspaceStatusGenerator.ts | 2 + .../services/workspaceTitleGenerator.test.ts | 25 +- src/node/services/workspaceTitleGenerator.ts | 4 +- src/node/utils/codexOauthAuth.test.ts | 58 ++ src/node/utils/codexOauthAuth.ts | 48 ++ src/node/utils/providerRequirements.test.ts | 21 + src/node/utils/providerRequirements.ts | 7 +- 57 files changed, 3099 insertions(+), 752 deletions(-) create mode 100644 src/browser/features/Settings/Sections/CodexAccounts.tsx create mode 100644 src/browser/stories/App.codexAccounts.stories.tsx create mode 100644 src/common/constants/codexOauthAccounts.ts create mode 100644 src/common/utils/providers/codexOauthRouting.test.ts create mode 100644 src/node/services/tools/shared/configRedaction.test.ts diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 6fb376dd4c0..5a7615cc946 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -197,6 +197,7 @@ function AppInner() { const { userProjects, + getProjectConfig, refreshProjects, removeProject, openProjectCreateModal, @@ -263,6 +264,10 @@ function AppInner() { ) : null; const creationScopeId = creationScope ? getProjectScopeId(creationScope.projectPath) : null; + const accountProjectPath = selectedWorkspace?.projectPath ?? creationScope?.projectPath; + const codexOauthAccountId = accountProjectPath + ? getProjectConfig(accountProjectPath)?.codexOauthAccountId + : undefined; // History navigation (back/forward) const navigate = useNavigate(); @@ -690,9 +695,17 @@ function AppInner() { const provider = getFastModeProvider(model, { providersConfig, resolvedRouteProvider: getRouteForModel(normalizeToCanonical(model)), + codexOauthAccountId, }); return provider != null && providersConfig[provider]?.serviceTier === "priority"; - }, [creationScopeId, getModelForWorkspace, getRouteForModel, providersConfig, selectedWorkspace]); + }, [ + codexOauthAccountId, + creationScopeId, + getModelForWorkspace, + getRouteForModel, + providersConfig, + selectedWorkspace, + ]); const fastModeToggleInFlightRef = useRef(false); const toggleFastMode = useCallback(async () => { @@ -707,6 +720,7 @@ function AppInner() { const provider = getFastModeProvider(model, { providersConfig, resolvedRouteProvider: getRouteForModel(normalizeToCanonical(model)), + codexOauthAccountId, }); if (provider == null) { fastModeToggleInFlightRef.current = false; @@ -736,6 +750,7 @@ function AppInner() { } }, [ api, + codexOauthAccountId, creationScopeId, getModelForWorkspace, getRouteForModel, @@ -984,6 +999,7 @@ function AppInner() { workspaceMetadata, selectedWorkspace, creationScopeId, + codexOauthAccountId, themePreference, getThinkingLevel: getThinkingLevelForWorkspace, onSetThinkingLevel: setThinkingLevelFromPalette, diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index f1c333bd245..d15d8347778 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -99,6 +99,7 @@ import { useReviews } from "@/browser/hooks/useReviews"; import { ReviewsBanner } from "../ReviewsBanner/ReviewsBanner"; import type { ReviewNoteData } from "@/common/types/review"; import { useWorkspaceContext } from "@/browser/contexts/WorkspaceContext"; +import { useProjectContext } from "@/browser/contexts/ProjectContext"; import { useBackgroundBashActions, useBackgroundBashError, @@ -467,6 +468,8 @@ const ChatPaneContent: React.FC = (props) => { loadingOlderHistory, activeBashMonitorCount, } = workspaceState; + const { getProjectConfig } = useProjectContext(); + const codexOauthAccountId = getProjectConfig(projectPath)?.codexOauthAccountId; const shouldShowPinnedTodoList = workspaceState.todos.length > 0; const shouldShowReviewsBanner = reviews.reviews.length > 0; const shouldRenderLoadOlderMessagesButton = hasOlderHistory && !isPixelSnapshotEnvironment(); @@ -486,6 +489,7 @@ const ChatPaneContent: React.FC = (props) => { api: api ?? undefined, pendingSendOptions, providersConfig, + codexOauthAccountId, }); // Apply message transformations: @@ -577,9 +581,17 @@ const ChatPaneContent: React.FC = (props) => { use1M, autoCompactionThreshold / 100, undefined, - providersConfig + providersConfig, + { codexOauthAccountId } ), - [workspaceUsage, pendingModel, use1M, providersConfig, autoCompactionThreshold] + [ + workspaceUsage, + pendingModel, + use1M, + providersConfig, + autoCompactionThreshold, + codexOauthAccountId, + ] ); // Show warning when: shouldShowWarning flag is true AND not currently compacting. diff --git a/src/browser/components/ThinkingSelector/ThinkingSelector.tsx b/src/browser/components/ThinkingSelector/ThinkingSelector.tsx index a7b1c8c644b..0cef3629df6 100644 --- a/src/browser/components/ThinkingSelector/ThinkingSelector.tsx +++ b/src/browser/components/ThinkingSelector/ThinkingSelector.tsx @@ -56,6 +56,7 @@ export interface ThinkingInheritOption { interface ThinkingSelectorControlProps { modelString: string | undefined; + codexOauthAccountId?: string; /** Delegated preferences may inherit a model that is only known at launch. */ modelCapabilitiesDeferred?: boolean; /** Independent of effort/model inheritance; false denotes an explicit mode override. */ @@ -130,12 +131,14 @@ export const ThinkingSelectorControl: React.FC = ( openaiProModeAvailable(props.modelString, { providersConfig, resolvedRouteProvider: resolvedRoute, + codexOauthAccountId: props.codexOauthAccountId, }), fastModeProvider: props.allowFastMode !== false && providersConfig != null ? getFastModeProvider(props.modelString, { providersConfig, resolvedRouteProvider: resolvedRoute, + codexOauthAccountId: props.codexOauthAccountId, }) : null, }; @@ -437,6 +440,7 @@ export const ThinkingSelectorControl: React.FC = ( interface ThinkingSelectorProps { modelString: string; + codexOauthAccountId?: string; /** Some embedded clients cannot resolve route-aware provider options safely. */ allowProMode?: boolean; /** Some embedded clients do not expose provider configuration mutations. */ @@ -451,6 +455,7 @@ export const ThinkingSelector: React.FC = (props) => { return ( = (props) => { [effectivePolicy] ); const { variant } = props; - const { userProjects } = useProjectContext(); + const { userProjects, getProjectConfig } = useProjectContext(); const creationScope = variant === "creation" ? resolveWorkspaceCreationScope(props.projectPath, userProjects, props.pendingSubProjectPath) @@ -505,7 +506,6 @@ const ChatInputInner: React.FC = (props) => { ensureModelInSettings, defaultModel, setDefaultModel, - codexOauthSet, requiresCodexOauth, } = useModelsFromSettings(); @@ -601,6 +601,15 @@ const ChatInputInner: React.FC = (props) => { const usage = useWorkspaceUsage(workspaceIdForUsage); const { has1MContext } = useProviderOptions(); const { config: providersConfig } = useProvidersConfig(); + const accountProjectPath = + variant === "creation" ? creationParentProjectPath : selectedWorkspace?.projectPath; + const codexOauthAccountId = accountProjectPath + ? getProjectConfig(accountProjectPath)?.codexOauthAccountId + : undefined; + const codexOauthSet = + providersConfig == null + ? null + : hasCodexOauthTokens(providersConfig.openai, codexOauthAccountId); const lastUsage = usage?.liveUsage ?? usage?.lastContextUsage; // Token counts come from usage metadata, but context limits/1M eligibility should // follow the currently selected model unless a stream is actively running. @@ -609,9 +618,11 @@ const ChatInputInner: React.FC = (props) => { const use1M = has1MContext(contextDisplayModel); const contextUsageData = useMemo(() => { return lastUsage - ? calculateTokenMeterData(lastUsage, contextDisplayModel, use1M, false, providersConfig) + ? calculateTokenMeterData(lastUsage, contextDisplayModel, use1M, false, providersConfig, { + codexOauthAccountId, + }) : { segments: [], totalTokens: 0, totalPercentage: 0 }; - }, [lastUsage, contextDisplayModel, use1M, providersConfig]); + }, [lastUsage, contextDisplayModel, use1M, providersConfig, codexOauthAccountId]); const { threshold: autoCompactThreshold, setThreshold: setAutoCompactThreshold } = useAutoCompactionSettings(workspaceIdForUsage, contextDisplayModel); const autoCompactionProps = useMemo( @@ -2765,7 +2776,10 @@ const ChatInputInner: React.FC = (props) => { className="flex shrink-0 items-center" data-component="ThinkingSelectorGroup" > - + diff --git a/src/browser/features/ChatInput/useCreationWorkspace.ts b/src/browser/features/ChatInput/useCreationWorkspace.ts index 05c198db90b..6790697e95c 100644 --- a/src/browser/features/ChatInput/useCreationWorkspace.ts +++ b/src/browser/features/ChatInput/useCreationWorkspace.ts @@ -370,6 +370,7 @@ export function useCreationWorkspace({ debounceMs: 500, userModel, scopeId: workspaceNameScopeId, + projectPath: (subProjectPath ?? projectPath) || undefined, }); // Destructure name state functions for use in callbacks diff --git a/src/browser/features/RightSidebar/ContextUsageSection.tsx b/src/browser/features/RightSidebar/ContextUsageSection.tsx index 68dcaa26703..898a5fe75ac 100644 --- a/src/browser/features/RightSidebar/ContextUsageSection.tsx +++ b/src/browser/features/RightSidebar/ContextUsageSection.tsx @@ -1,4 +1,6 @@ import React from "react"; +import { useProjectContext } from "@/browser/contexts/ProjectContext"; +import { useWorkspaceContext } from "@/browser/contexts/WorkspaceContext"; import { useWorkspaceUsage } from "@/browser/stores/WorkspaceStore"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { AGENT_AI_DEFAULTS_KEY } from "@/common/constants/storage"; @@ -33,6 +35,12 @@ export const ContextUsageSection: React.FC = ({ worksp const { has1MContext } = useProviderOptions(); const pendingSendOptions = useSendMessageOptions(workspaceId); const { config: providersConfig } = useProvidersConfig(); + const { getProjectConfig } = useProjectContext(); + const { workspaceMetadata } = useWorkspaceContext(); + const projectPath = workspaceMetadata.get(workspaceId)?.projectPath; + const codexOauthAccountId = projectPath + ? getProjectConfig(projectPath)?.codexOauthAccountId + : undefined; // Token counts come from usage metadata, but context limits/1M eligibility should // follow the currently selected model unless a stream is actively running. @@ -55,7 +63,8 @@ export const ContextUsageSection: React.FC = ({ worksp contextDisplayModel, has1MContext(contextDisplayModel), false, - providersConfig + providersConfig, + { codexOauthAccountId } ); // Warn when the compaction model can't fit the auto-compact threshold to avoid failures. @@ -67,7 +76,8 @@ export const ContextUsageSection: React.FC = ({ worksp const compactionMaxTokens = getEffectiveContextLimit( effectiveCompactionModel, has1MContext(effectiveCompactionModel), - providersConfig + providersConfig, + { codexOauthAccountId } ); if (compactionMaxTokens && compactionMaxTokens < thresholdTokens) { diff --git a/src/browser/features/Settings/Sections/CodexAccounts.tsx b/src/browser/features/Settings/Sections/CodexAccounts.tsx new file mode 100644 index 00000000000..d9ae367236f --- /dev/null +++ b/src/browser/features/Settings/Sections/CodexAccounts.tsx @@ -0,0 +1,452 @@ +import { useEffect, useRef, useState } from "react"; +import { Loader2 } from "lucide-react"; +import { Button } from "@/browser/components/Button/Button"; +import { useAPI, type APIClient } from "@/browser/contexts/API"; +import { useProjectContext } from "@/browser/contexts/ProjectContext"; +import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; +import type { ProviderConfigInfo } from "@/common/orpc/types"; +import type { Result } from "@/common/types/result"; +import { getErrorMessage } from "@/common/utils/errors"; +import { + CODEX_OAUTH_DEFAULT_ACCOUNT_ID, + CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH, +} from "@/common/constants/codexOauthAccounts"; + +type LoginInput = Parameters[0]; +type Account = NonNullable[number]; +interface LoginFlow { + flowId: string; + url: string; + userCode?: string; + cancel: () => Promise; +} + +const inputClassName = + "bg-background border-border-light text-foreground w-full min-w-0 rounded border px-2 py-1.5 text-xs"; + +function AccountSelect(props: { + label: string; + value: string; + accounts: Account[]; + defaultLabel?: string; + disabled: boolean; + onChange: (value: string) => void; +}) { + // Keep missing selections visible. Selecting another account must require a user action. + const missing = + props.value !== "" && !props.accounts.some((account) => account.id === props.value); + return ( + + ); +} + +export function CodexAccounts() { + const { api } = useAPI(); + const { config, refresh } = useProvidersConfig(); + const { userProjects, refreshProjects } = useProjectContext(); + const [label, setLabel] = useState(""); + const [rename, setRename] = useState(null); + const [busy, setBusy] = useState(false); + const [loginInProgress, setLoginInProgress] = useState(false); + const [error, setError] = useState(null); + const [flow, setFlow] = useState(null); + const attemptRef = useRef(0); + const flowRef = useRef(null); + const openai = config?.openai; + const accounts = + openai?.codexOauthAccounts ?? + (openai?.codexOauthSet ? [{ id: CODEX_OAUTH_DEFAULT_ACCOUNT_ID, label: "Default" }] : []); + const defaultId = openai?.codexOauthDefaultAccountId ?? CODEX_OAUTH_DEFAULT_ACCOUNT_ID; + const defaultLabel = + accounts.find((account) => account.id === defaultId)?.label ?? `Missing account (${defaultId})`; + const isDesktop = !!window.api; + const showBrowser = + isDesktop || ["localhost", "127.0.0.1", "::1"].includes(window.location.hostname); + const disabled = !api || busy; + const authEditable = + accounts.length > 0 && (openai?.apiKeySet === true || !!openai?.apiKeySource); + + // Login owns its server flow. Unmount invalidates late results and cancels the current flow. + useEffect( + () => () => { + attemptRef.current++; + flowRef.current?.cancel().catch(() => undefined); + }, + [] + ); + + function runAction(operation: Promise): void { + operation.catch((err: unknown) => setError(getErrorMessage(err))); + } + + async function saveName() { + if (!api || !rename) return; + const input = { accountId: rename.id, label: rename.label.trim() }; + if (await mutate(() => api.codexOauth.renameAccount(input))) setRename(null); + } + + async function refreshState() { + await Promise.all([refresh(), refreshProjects()]); + } + + async function mutate(operation: () => Promise>) { + setBusy(true); + setError(null); + try { + const result = await operation(); + if (!result.success) { + setError(result.error); + return false; + } + await refreshState(); + return true; + } catch (err) { + setError(getErrorMessage(err)); + return false; + } finally { + setBusy(false); + } + } + + async function connect(device: boolean, input: LoginInput) { + if (!api) return; + const attempt = ++attemptRef.current; + setLoginInProgress(true); + setBusy(true); + setError(null); + try { + let nextFlow: LoginFlow; + if (device || !showBrowser) { + const result = await api.codexOauth.startDeviceFlow(input); + if (!result.success) throw new Error(result.error); + const { flowId, userCode, verifyUrl } = result.data; + nextFlow = { + flowId, + userCode, + url: verifyUrl, + cancel: () => api.codexOauth.cancelDeviceFlow({ flowId }), + }; + } else { + const result = await api.codexOauth.startDesktopFlow(input); + if (!result.success) throw new Error(result.error); + const { flowId, authorizeUrl } = result.data; + nextFlow = { + flowId, + url: authorizeUrl, + cancel: () => api.codexOauth.cancelDesktopFlow({ flowId }), + }; + } + if (attempt !== attemptRef.current) { + await nextFlow.cancel(); + return; + } + flowRef.current = nextFlow; + setFlow(nextFlow); + const result = + nextFlow.userCode != null + ? await api.codexOauth.waitForDeviceFlow({ flowId: nextFlow.flowId }) + : await api.codexOauth.waitForDesktopFlow({ flowId: nextFlow.flowId }); + if (attempt !== attemptRef.current) return; + if (!result.success) throw new Error(result.error); + setLabel(""); + await refreshState(); + } catch (err) { + if (attempt === attemptRef.current) setError(getErrorMessage(err)); + } finally { + if (attempt === attemptRef.current) { + flowRef.current = null; + setFlow(null); + setLoginInProgress(false); + setBusy(false); + } + } + } + + async function cancel() { + attemptRef.current++; + try { + await flowRef.current?.cancel(); + setError(null); + } catch (err) { + setError(getErrorMessage(err)); + } finally { + flowRef.current = null; + setFlow(null); + setLoginInProgress(false); + setBusy(false); + } + } + + const loginInput = label.trim() ? { label: label.trim() } : undefined; + return ( +
+
+

ChatGPT (Codex) OAuth

+

{accounts.length > 0 ? "Connected" : "Not connected"}

+
+
    + {accounts.map((account) => ( +
  • + {rename?.id === account.id ? ( +
    { + event.preventDefault(); + runAction(saveName()); + }} + > + setRename({ ...account, label: event.target.value })} + /> + + +
    + ) : ( + <> +

    + {account.label} + {account.id === defaultId && ( + · Global default + )} +

    +
    + + {showBrowser && ( + + )} + + +
    + + )} +
    • + ))} +
+
{ + event.preventDefault(); + runAction(connect(false, loginInput)); + }} + > + +
+ {showBrowser && ( + + )} + +
+
+ {busy && ( +
+ + + {flow ? "Waiting for authorization..." : loginInProgress ? "Starting..." : "Saving..."} + + {loginInProgress && ( + + )} +
+ )} + {flow && ( +
+ {flow.userCode && ( + <> +

Enter this code on the OpenAI verification page:

+ + {flow.userCode} + + + )} + +
+ )} + {error && ( +

+ {error} +

+ )} + + api && runAction(mutate(() => api.codexOauth.setDefaultAccount({ accountId }))) + } + /> +
+

Project accounts

+

+ Project selections override the global default. Missing accounts do not use another + account. +

+ {Array.from(userProjects, ([projectPath, project]) => ( + + api && + runAction( + mutate(() => + api.projects.setCodexOauthAccount({ projectPath, accountId: accountId || null }) + ) + ) + } + /> + ))} +
+ +

+ ChatGPT OAuth costs use API-equivalent estimates. Your plan may include usage or charge + credits. API keys use OpenAI platform billing. +

+ {!authEditable && ( +

+ Connect ChatGPT OAuth and set an OpenAI API key to change this setting. +

+ )} +
+ ); +} diff --git a/src/browser/features/Settings/Sections/ProvidersSection.tsx b/src/browser/features/Settings/Sections/ProvidersSection.tsx index 75ea804c4af..24849c15553 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.tsx @@ -52,10 +52,6 @@ import { SelectValue, } from "@/browser/components/SelectPrimitive/SelectPrimitive"; import { Switch } from "@/browser/components/Switch/Switch"; -import { - ToggleGroup, - ToggleGroupItem, -} from "@/browser/components/ToggleGroupPrimitive/ToggleGroupPrimitive"; import { HelpIndicator, Tooltip, @@ -80,6 +76,7 @@ import type { AddCustomProviderInput, ProviderConfigInfo } from "@/common/orpc/t import type { ServiceTier, XAIServiceTier } from "@/common/config/schemas/providersConfig"; import type { Result } from "@/common/types/result"; import { CODER_OAUTH_SERVER_START_PATH } from "@/common/constants/coderOAuth"; +import { CodexAccounts } from "./CodexAccounts"; type MuxGatewayLoginStatus = "idle" | "starting" | "waiting" | "success" | "error"; type CodexOauthFlowStatus = "idle" | "starting" | "waiting" | "error"; @@ -104,12 +101,6 @@ function isXAIServiceTier(value: string): value is XAIServiceTier { return value === "default" || value === "priority"; } -interface CodexOauthDeviceFlow { - flowId: string; - userCode: string; - verifyUrl: string; -} - interface OAuthMessage { type?: unknown; state?: unknown; @@ -549,290 +540,9 @@ export function ProvidersSection() { const isDesktop = !!window.api; - // The "Connect (Browser)" OAuth flow requires a redirect back to this origin, - // which only works when the host is the user's local machine. On a remote mux - // server the redirect would land on the server, not the user's browser. - const isRemoteServer = - !isDesktop && !["localhost", "127.0.0.1", "::1"].includes(window.location.hostname); - - const [codexOauthStatus, setCodexOauthStatus] = useState("idle"); - const [codexOauthError, setCodexOauthError] = useState(null); - - const codexOauthAttemptRef = useRef(0); - const [codexOauthDesktopFlowId, setCodexOauthDesktopFlowId] = useState(null); - const [codexOauthDeviceFlow, setCodexOauthDeviceFlow] = useState( - null - ); - const [codexOauthAuthorizeUrl, setCodexOauthAuthorizeUrl] = useState(null); - const codexOauthIsConnected = config?.openai?.codexOauthSet === true; - const openaiApiKeySet = config?.openai?.apiKeySet === true || !!config?.openai?.apiKeySource; - const codexOauthDefaultAuth = - config?.openai?.codexOauthDefaultAuth === "apiKey" ? "apiKey" : "oauth"; - const codexOauthDefaultAuthIsEditable = codexOauthIsConnected && openaiApiKeySet; - - const codexOauthLoginInProgress = - codexOauthStatus === "starting" || codexOauthStatus === "waiting"; - - const startCodexOauthBrowserConnect = async () => { - const attempt = ++codexOauthAttemptRef.current; - - if (!api) { - setCodexOauthStatus("error"); - setCodexOauthError("Xum API not connected."); - return; - } - - // Best-effort: cancel any in-progress flow before starting a new one. - if (codexOauthDesktopFlowId) { - void api.codexOauth.cancelDesktopFlow({ flowId: codexOauthDesktopFlowId }); - } - if (codexOauthDeviceFlow) { - void api.codexOauth.cancelDeviceFlow({ flowId: codexOauthDeviceFlow.flowId }); - } - - setCodexOauthError(null); - setCodexOauthDesktopFlowId(null); - setCodexOauthDeviceFlow(null); - setCodexOauthAuthorizeUrl(null); - - try { - setCodexOauthStatus("starting"); - - if (!isDesktop) { - const startResult = await api.codexOauth.startDeviceFlow(); - - if (attempt !== codexOauthAttemptRef.current) { - if (startResult.success) { - void api.codexOauth.cancelDeviceFlow({ flowId: startResult.data.flowId }); - } - return; - } - - if (!startResult.success) { - setCodexOauthStatus("error"); - setCodexOauthError(startResult.error); - return; - } - - setCodexOauthDeviceFlow({ - flowId: startResult.data.flowId, - userCode: startResult.data.userCode, - verifyUrl: startResult.data.verifyUrl, - }); - setCodexOauthStatus("waiting"); - - // Keep device-code login manual per user request: we only open the - // verification page from the explicit "Copy & Open" action. - const waitResult = await api.codexOauth.waitForDeviceFlow({ - flowId: startResult.data.flowId, - }); - - if (attempt !== codexOauthAttemptRef.current) { - return; - } - - if (!waitResult.success) { - setCodexOauthStatus("error"); - setCodexOauthError(waitResult.error); - return; - } - - setCodexOauthStatus("idle"); - setCodexOauthDeviceFlow(null); - setCodexOauthAuthorizeUrl(null); - await refresh(); - return; - } - - const startResult = await api.codexOauth.startDesktopFlow(); - - if (attempt !== codexOauthAttemptRef.current) { - if (startResult.success) { - void api.codexOauth.cancelDesktopFlow({ flowId: startResult.data.flowId }); - } - return; - } - - if (!startResult.success) { - setCodexOauthStatus("error"); - setCodexOauthError(startResult.error); - return; - } - - const { flowId, authorizeUrl } = startResult.data; - setCodexOauthDesktopFlowId(flowId); - setCodexOauthAuthorizeUrl(authorizeUrl); - setCodexOauthStatus("waiting"); - - const waitResult = await api.codexOauth.waitForDesktopFlow({ flowId }); - - if (attempt !== codexOauthAttemptRef.current) { - return; - } - - if (!waitResult.success) { - setCodexOauthStatus("error"); - setCodexOauthError(waitResult.error); - return; - } - - setCodexOauthStatus("idle"); - setCodexOauthDesktopFlowId(null); - await refresh(); - } catch (err) { - if (attempt !== codexOauthAttemptRef.current) { - return; - } - - setCodexOauthStatus("error"); - setCodexOauthError(getErrorMessage(err)); - } - }; - - const startCodexOauthDeviceConnect = async () => { - const attempt = ++codexOauthAttemptRef.current; - - if (!api) { - setCodexOauthStatus("error"); - setCodexOauthError("Xum API not connected."); - return; - } - - // Best-effort: cancel any in-progress flow before starting a new one. - if (codexOauthDesktopFlowId) { - void api.codexOauth.cancelDesktopFlow({ flowId: codexOauthDesktopFlowId }); - } - if (codexOauthDeviceFlow) { - void api.codexOauth.cancelDeviceFlow({ flowId: codexOauthDeviceFlow.flowId }); - } - - setCodexOauthError(null); - setCodexOauthDesktopFlowId(null); - setCodexOauthDeviceFlow(null); - setCodexOauthAuthorizeUrl(null); - - try { - setCodexOauthStatus("starting"); - const startResult = await api.codexOauth.startDeviceFlow(); - - if (attempt !== codexOauthAttemptRef.current) { - if (startResult.success) { - void api.codexOauth.cancelDeviceFlow({ flowId: startResult.data.flowId }); - } - return; - } - - if (!startResult.success) { - setCodexOauthStatus("error"); - setCodexOauthError(startResult.error); - return; - } - - setCodexOauthDeviceFlow({ - flowId: startResult.data.flowId, - userCode: startResult.data.userCode, - verifyUrl: startResult.data.verifyUrl, - }); - setCodexOauthStatus("waiting"); - - const waitResult = await api.codexOauth.waitForDeviceFlow({ - flowId: startResult.data.flowId, - }); - - if (attempt !== codexOauthAttemptRef.current) { - return; - } - - if (!waitResult.success) { - setCodexOauthStatus("error"); - setCodexOauthError(waitResult.error); - return; - } - - setCodexOauthStatus("idle"); - setCodexOauthDeviceFlow(null); - setCodexOauthAuthorizeUrl(null); - await refresh(); - } catch (err) { - if (attempt !== codexOauthAttemptRef.current) { - return; - } - - setCodexOauthStatus("error"); - setCodexOauthError(getErrorMessage(err)); - } - }; - - const disconnectCodexOauth = async () => { - const attempt = ++codexOauthAttemptRef.current; - - if (!api) { - setCodexOauthStatus("error"); - setCodexOauthError("Xum API not connected."); - return; - } - - // Best-effort: cancel any in-progress flow. - if (codexOauthDesktopFlowId) { - void api.codexOauth.cancelDesktopFlow({ flowId: codexOauthDesktopFlowId }); - } - if (codexOauthDeviceFlow) { - void api.codexOauth.cancelDeviceFlow({ flowId: codexOauthDeviceFlow.flowId }); - } - - setCodexOauthError(null); - setCodexOauthDesktopFlowId(null); - setCodexOauthDeviceFlow(null); - setCodexOauthAuthorizeUrl(null); - - try { - setCodexOauthStatus("starting"); - const result = await api.codexOauth.disconnect(); - - if (attempt !== codexOauthAttemptRef.current) { - return; - } - - if (!result.success) { - setCodexOauthStatus("error"); - setCodexOauthError(result.error); - return; - } - - updateOptimistically("openai", { codexOauthSet: false }); - setCodexOauthStatus("idle"); - await refresh(); - } catch (err) { - if (attempt !== codexOauthAttemptRef.current) { - return; - } - - setCodexOauthStatus("error"); - setCodexOauthError(getErrorMessage(err)); - } - }; const [muxGatewayLoginStatus, setMuxGatewayLoginStatus] = useState("idle"); - const cancelCodexOauth = () => { - codexOauthAttemptRef.current++; - - if (api) { - if (codexOauthDesktopFlowId) { - void api.codexOauth.cancelDesktopFlow({ flowId: codexOauthDesktopFlowId }); - } - if (codexOauthDeviceFlow) { - void api.codexOauth.cancelDeviceFlow({ flowId: codexOauthDeviceFlow.flowId }); - } - } - - setCodexOauthDesktopFlowId(null); - setCodexOauthDeviceFlow(null); - setCodexOauthAuthorizeUrl(null); - setCodexOauthStatus("idle"); - setCodexOauthError(null); - }; - const [muxGatewayLoginError, setMuxGatewayLoginError] = useState(null); const muxGatewayLoginAttemptRef = useRef(0); @@ -2651,185 +2361,7 @@ export function ProvidersSection() { const openAIWebSocketTransportVisible = openAIWireFormat === "responses"; return (
-
- - - {codexOauthStatus === "starting" - ? "Starting..." - : codexOauthStatus === "waiting" - ? "Waiting for login..." - : codexOauthIsConnected - ? "Connected" - : "Not connected"} - -
- -
- {!isRemoteServer && ( - - )} - - - {codexOauthStatus === "waiting" && - !codexOauthDeviceFlow && - codexOauthAuthorizeUrl && ( - - )} - - {codexOauthLoginInProgress && ( - - )} - - {codexOauthIsConnected && ( - - )} -
- - {codexOauthDeviceFlow && ( -
-

- Enter this code on the OpenAI verification page: -

-
- - {codexOauthDeviceFlow.userCode} - - -
-

- - Waiting for authorization... -

-
- )} - - {codexOauthStatus === "waiting" && !codexOauthDeviceFlow && ( -

- - Waiting for authorization... -

- )} - - {codexOauthStatus === "error" && codexOauthError && ( -

{codexOauthError}

- )} - -
-
- -

- Applies to models that support both ChatGPT OAuth and API keys - (e.g. gpt-5.5). -

-
- - { - if (!api) return; - if (next !== "oauth" && next !== "apiKey") { - return; - } - - updateOptimistically("openai", { codexOauthDefaultAuth: next }); - void api.providers.setProviderConfig({ - provider: "openai", - keyPath: ["codexOauthDefaultAuth"], - value: next, - }); - }} - size="sm" - className="h-9" - disabled={!api || !codexOauthDefaultAuthIsEditable} - > - - Use ChatGPT OAuth by default - - - Use OpenAI API key by default - - - -

- ChatGPT OAuth costs use API-equivalent estimates. Your plan may - include usage or charge credits. API keys use OpenAI platform - billing. -

- - {!codexOauthDefaultAuthIsEditable && ( -

- Connect ChatGPT OAuth and set an OpenAI API key to change this - setting. -

- )} -
+
diff --git a/src/browser/hooks/useContextSwitchWarning.ts b/src/browser/hooks/useContextSwitchWarning.ts index a7c48cf73db..0accc42f0d1 100644 --- a/src/browser/hooks/useContextSwitchWarning.ts +++ b/src/browser/hooks/useContextSwitchWarning.ts @@ -37,6 +37,7 @@ interface UseContextSwitchWarningProps { api: RouterClient | undefined; pendingSendOptions: SendMessageOptions; providersConfig: ProvidersConfigMap | null; + codexOauthAccountId?: string; } interface UseContextSwitchWarningResult { @@ -148,11 +149,12 @@ export function useContextSwitchWarning( const checkOptions: ContextSwitchOptions = useMemo( () => ({ providersConfig, + codexOauthAccountId: props.codexOauthAccountId, policy: effectivePolicy, routePriority, routeOverrides, }), - [providersConfig, effectivePolicy, routePriority, routeOverrides] + [providersConfig, props.codexOauthAccountId, effectivePolicy, routePriority, routeOverrides] ); const prevCheckOptionsRef = useRef(checkOptions); @@ -197,14 +199,19 @@ export function useContextSwitchWarning( }); if (suggestion) { - const limit = getEffectiveContextLimit(suggestion.modelId, use1M, providersConfig); + const limit = getEffectiveContextLimit( + suggestion.modelId, + use1M, + providersConfig, + checkOptions + ); if (limit && limit > w.currentTokens) { return { ...w, compactionModel: suggestion.modelId, errorMessage: null }; } } return w; }, - [providersConfig, effectivePolicy, routePriority, routeOverrides, use1M] + [providersConfig, effectivePolicy, routePriority, routeOverrides, use1M, checkOptions] ); const evaluateWarning = useCallback( @@ -413,8 +420,18 @@ export function useContextSwitchWarning( // OFF → ON: may clear warning if context now fits // ON → OFF: may show warning if context no longer fits if (wasEnabled !== use1M) { - const previousLimit = getEffectiveContextLimit(pendingModel, wasEnabled, providersConfig); - const nextLimit = getEffectiveContextLimit(pendingModel, use1M, providersConfig); + const previousLimit = getEffectiveContextLimit( + pendingModel, + wasEnabled, + providersConfig, + checkOptions + ); + const nextLimit = getEffectiveContextLimit( + pendingModel, + use1M, + providersConfig, + checkOptions + ); // Only surface same-model warnings if the effective limit actually changed. if (previousLimit === nextLimit) { @@ -441,7 +458,16 @@ export function useContextSwitchWarning( dispatch({ type: "CLEAR_WARNING" }); } } - }, [use1M, pendingModel, tokens, messages, providersConfig, evaluateWarning, warning]); + }, [ + use1M, + pendingModel, + tokens, + messages, + providersConfig, + evaluateWarning, + warning, + checkOptions, + ]); return { warning, handleModelChange, handleCompact, handleDismiss }; } diff --git a/src/browser/hooks/useWorkspaceName.ts b/src/browser/hooks/useWorkspaceName.ts index 6ebef4260d7..b577496009c 100644 --- a/src/browser/hooks/useWorkspaceName.ts +++ b/src/browser/hooks/useWorkspaceName.ts @@ -49,6 +49,8 @@ export interface UseWorkspaceNameOptions { debounceMs?: number; /** User's selected model to try after preferred models */ userModel?: string; + /** Apply the project account before the workspace exists. */ + projectPath?: string; /** * Optional storage scope for persisting draft name-generation state. * @@ -142,7 +144,7 @@ export function getDisplayTitleFromPersistedState(state: unknown): string { * auto-generation resumes. */ export function useWorkspaceName(options: UseWorkspaceNameOptions): UseWorkspaceNameReturn { - const { message, debounceMs = 500, userModel, scopeId } = options; + const { message, debounceMs = 500, userModel, scopeId, projectPath } = options; const { api } = useAPI(); const candidates = useMemo(() => buildNameGenCandidates(userModel), [userModel]); @@ -235,6 +237,7 @@ export function useWorkspaceName(options: UseWorkspaceNameOptions): UseWorkspace const result = await api.nameGeneration.generate({ message: forMessage, candidates, + projectPath, }); // Check if this request is still current (wasn't cancelled) @@ -288,7 +291,7 @@ export function useWorkspaceName(options: UseWorkspaceNameOptions): UseWorkspace } } }, - [api, setStored, candidates] + [api, setStored, candidates, projectPath] ); // Debounced generation effect diff --git a/src/browser/stories/App.codexAccounts.stories.tsx b/src/browser/stories/App.codexAccounts.stories.tsx new file mode 100644 index 00000000000..a6e75b62960 --- /dev/null +++ b/src/browser/stories/App.codexAccounts.stories.tsx @@ -0,0 +1,298 @@ +import { expect, fn, userEvent, waitFor, within } from "@storybook/test"; +import { appMeta, AppWithMocks, type AppStory } from "./meta"; +import { expandLeftSidebar, selectWorkspace } from "./helpers/uiState"; +import { createMockORPCClient } from "./mocks/orpc"; +import { createWorkspace, groupWorkspacesByProject } from "./mocks/workspaces"; +import type { APIClient } from "@/browser/contexts/API"; +import type { ProvidersConfigMap } from "@/common/orpc/types"; +import { Err, Ok } from "@/common/types/result"; + +export default { ...appMeta, title: "App/CodexAccounts" }; + +const startLogin = fn<(input: Parameters[0]) => void>(); + +const browserLogin = + fn<(input: Parameters[0]) => void>(); +const generateTitle = fn<(input: Parameters[0]) => void>(); + +function setupAccounts() { + expandLeftSidebar(); + startLogin.mockClear(); + browserLogin.mockClear(); + const workspace = createWorkspace({ + id: "codex-accounts", + name: "main", + projectName: "my-app", + projectPath: "/projects/my-app", + }); + selectWorkspace(workspace); + const projects = groupWorkspacesByProject([workspace]); + const providers: ProvidersConfigMap = { + openai: { + apiKeySet: true, + isEnabled: true, + isConfigured: true, + codexOauthSet: true, + codexOauthAccounts: [ + { id: "default", label: "Personal" }, + { id: "work", label: "Work" }, + ], + }, + }; + let slot = 0; + const client = createMockORPCClient({ + projects, + workspaces: [workspace], + providersConfig: providers, + providersList: ["openai"], + }); + const start: APIClient["codexOauth"]["startDeviceFlow"] = (input) => { + startLogin(input); + if (input?.label) { + providers.openai.codexOauthAccounts?.push({ id: "slot-" + ++slot, label: input.label }); + } + return Promise.resolve( + Ok({ + flowId: "login", + userCode: "CODE-1234", + verifyUrl: "https://auth.openai.com/codex/device", + intervalSeconds: 5, + }) + ); + }; + client.codexOauth = { + startDeviceFlow: start, + startDesktopFlow: async (input) => { + browserLogin(input); + await start(input); + return Ok({ flowId: "login", authorizeUrl: "https://auth.openai.com/authorize" }); + }, + waitForDeviceFlow: () => Promise.resolve(Ok(undefined)), + waitForDesktopFlow: () => Promise.resolve(Ok(undefined)), + cancelDeviceFlow: () => Promise.resolve(), + cancelDesktopFlow: () => Promise.resolve(), + disconnect: (input) => { + providers.openai.codexOauthAccounts = providers.openai.codexOauthAccounts?.filter( + (account) => account.id !== (input?.accountId ?? "default") + ); + providers.openai.codexOauthSet = !!providers.openai.codexOauthAccounts?.length; + return Promise.resolve(Ok(undefined)); + }, + renameAccount: (input) => { + const account = providers.openai.codexOauthAccounts?.find( + (item) => item.id === input.accountId + ); + if (!account) return Promise.resolve(Err("Account is missing")); + account.label = input.label; + return Promise.resolve(Ok(undefined)); + }, + setDefaultAccount: (input) => { + providers.openai.codexOauthDefaultAccountId = input.accountId; + return Promise.resolve(Ok(undefined)); + }, + }; + client.providers.getConfig = () => Promise.resolve(structuredClone(providers)); + client.providers.setProviderConfig = (input) => { + if ( + input.keyPath[0] === "codexOauthDefaultAuth" && + (input.value === "apiKey" || input.value === "oauth") + ) { + providers.openai.codexOauthDefaultAuth = input.value; + } + return Promise.resolve(Ok(undefined)); + }; + client.projects.setCodexOauthAccount = (input) => { + const project = projects.get(input.projectPath); + if (!project) return Promise.resolve(Err("Project is missing")); + project.codexOauthAccountId = input.accountId ?? undefined; + return Promise.resolve(Ok(undefined)); + }; + return client; +} + +async function openAccounts(canvasElement: HTMLElement) { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByTestId("settings-button", {}, { timeout: 10000 })); + await userEvent.click(await canvas.findByRole("button", { name: "Providers" })); + const openai = await canvas.findByText("OpenAI", { exact: true }); + await userEvent.click(openai); + const section = await canvas.findByRole("region", { name: "ChatGPT (Codex) accounts" }); + return section; +} + +async function exerciseAccounts(canvasElement: HTMLElement) { + const section = await openAccounts(canvasElement); + const controls = within(section); + const global = controls.getByRole("combobox", { name: "Global default account" }); + const project = controls.getByRole("combobox", { name: "/projects/my-app" }); + await expect(global).toHaveValue("default"); + await expect(project).toHaveValue(""); + await userEvent.selectOptions(global, "work"); + await waitFor(() => expect(global).toHaveValue("work")); + await waitFor(() => expect(project).toBeEnabled()); + await userEvent.selectOptions(project, "work"); + await waitFor(() => expect(project).toHaveValue("work")); + + const work = within(controls.getByRole("listitem", { name: "Work" })); + await userEvent.click(work.getByRole("button", { name: "Rename" })); + const name = controls.getByRole("textbox", { name: "Account name" }); + await userEvent.clear(name); + await userEvent.type(name, "Team{Enter}"); + await controls.findByRole("listitem", { name: "Team" }); + await expect(global).toHaveDisplayValue("Team"); + await expect(project).toHaveDisplayValue("Team"); + + await userEvent.type(controls.getByRole("textbox", { name: "New account name" }), "Lab"); + await userEvent.click(controls.getByRole("button", { name: "Connect (Device)" })); + await controls.findByRole("listitem", { name: "Lab" }); + await expect(startLogin).toHaveBeenLastCalledWith({ label: "Lab" }); + await userEvent.click( + within(controls.getByRole("listitem", { name: "Team" })).getByRole("button", { + name: "Reconnect", + }) + ); + await waitFor(() => expect(global).toBeEnabled()); + await expect(startLogin).toHaveBeenLastCalledWith({ accountId: "work" }); + if (controls.queryByRole("button", { name: "Connect (Browser)" })) { + await expect(browserLogin).toHaveBeenLastCalledWith({ accountId: "work" }); + } + await expect(controls.getAllByRole("listitem")).toHaveLength(3); + + const auth = controls.getByRole("combobox", { name: "Default auth (when both are set)" }); + await userEvent.selectOptions(auth, "apiKey"); + await waitFor(() => expect(auth).toHaveValue("apiKey")); + await userEvent.click( + within(controls.getByRole("listitem", { name: "Team" })).getByRole("button", { + name: "Disconnect", + }) + ); + await waitFor(() => expect(controls.queryByRole("listitem", { name: "Team" })).toBeNull()); + await expect(global).toHaveValue("work"); + await expect(project).toHaveValue("work"); + await expect(global).toHaveDisplayValue(/Missing account/); + await expect(project).toHaveDisplayValue(/Missing account/); + await expect(controls.getAllByRole("listitem")).toHaveLength(2); + await userEvent.selectOptions(global, "default"); + await waitFor(() => expect(global).toHaveValue("default")); + await expect(project).toHaveValue("work"); + await userEvent.selectOptions(project, ""); + await waitFor(() => expect(project).toHaveValue("")); + await expect(project).toHaveDisplayValue(/Personal/); + await userEvent.selectOptions(project, "slot-1"); + await waitFor(() => expect(project).toHaveValue("slot-1")); + section.scrollIntoView({ block: "start" }); + + // The test-runner ignores viewport globals. Pixel and the phone runner enforce these bounds. + if (window.innerWidth < 768) { + await expect(section.getBoundingClientRect().right).toBeLessThanOrEqual(window.innerWidth); + await expect(section.scrollWidth).toBeLessThanOrEqual(section.clientWidth); + } +} + +export const Desktop: AppStory = { + globals: { viewport: { value: "desktop", isRotated: false } }, + parameters: { pixel: { matrix: { themes: ["dark", "light"], viewports: ["desktop"] } } }, + render: () => , + play: async ({ canvasElement }) => exerciseAccounts(canvasElement), +}; + +export const Phone: AppStory = { + ...Desktop, + globals: { viewport: { value: "mobile1", isRotated: false } }, + parameters: { pixel: { matrix: { themes: ["dark", "light"], viewports: ["phone"] } } }, + play: async ({ canvasElement }) => exerciseAccounts(canvasElement), +}; + +function setupLoginFailure() { + const client = setupAccounts(); + let firstAttempt = true; + let finishWait: () => void = () => undefined; + client.codexOauth.startDeviceFlow = () => { + if (firstAttempt) { + firstAttempt = false; + return Promise.resolve(Err("OpenAI login is unavailable. Try again.")); + } + return Promise.resolve( + Ok({ + flowId: "pending-login", + userCode: "CODE-1234", + verifyUrl: "https://auth.openai.com/codex/device", + intervalSeconds: 5, + }) + ); + }; + client.codexOauth.waitForDeviceFlow = () => + new Promise((resolve) => { + finishWait = () => resolve(Err("Login cancelled")); + }); + client.codexOauth.cancelDeviceFlow = () => { + finishWait(); + return Promise.resolve(); + }; + client.codexOauth.setDefaultAccount = () => Promise.resolve(Err("Account update failed")); + return client; +} + +export const LoginFailureAndCancel: AppStory = { + render: () => , + play: async ({ canvasElement }) => { + const section = await openAccounts(canvasElement); + const controls = within(section); + const global = controls.getByRole("combobox", { name: "Global default account" }); + await userEvent.selectOptions(global, "work"); + await controls.findByRole("alert"); + await expect(global).toHaveValue("default"); + const name = controls.getByRole("textbox", { name: "New account name" }); + await userEvent.type(name, "Lab"); + await userEvent.click(controls.getByRole("button", { name: "Connect (Device)" })); + await waitFor(() => + expect(controls.getByRole("alert")).toHaveTextContent(/login is unavailable/) + ); + await expect(controls.getAllByRole("listitem")).toHaveLength(2); + await userEvent.click(controls.getByRole("button", { name: "Connect (Device)" })); + await controls.findByText("CODE-1234"); + await expect(name).toBeDisabled(); + await userEvent.click(controls.getByRole("button", { name: "Cancel" })); + await waitFor(() => expect(name).toBeEnabled()); + await expect(controls.queryByText("CODE-1234")).toBeNull(); + await expect(controls.queryByRole("alert")).toBeNull(); + await expect(controls.getAllByRole("listitem")).toHaveLength(2); + section.scrollIntoView({ block: "start" }); + }, +}; + +function setupProjectTitle() { + const client = setupAccounts(); + generateTitle.mockClear(); + client.nameGeneration.generate = (input) => { + generateTitle(input); + return Promise.resolve( + Ok({ name: "project-title", title: "Project title", modelUsed: "openai:gpt-5.5" }) + ); + }; + return client; +} + +export const TitleUsesProject: AppStory = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const newChat = await canvas.findByRole( + "button", + { name: "New chat in my-app" }, + { timeout: 10000 } + ); + newChat.focus(); + await userEvent.keyboard("{Enter}"); + const message = await canvas.findByRole("textbox", { name: "Message Claude" }); + await userEvent.type(message, "Fix the project title"); + await waitFor(() => + expect(generateTitle).toHaveBeenLastCalledWith( + expect.objectContaining({ projectPath: "/projects/my-app" }) + ) + ); + await waitFor(() => + expect(canvasElement.querySelector("#workspace-name")).toHaveValue("project-title") + ); + }, +}; diff --git a/src/browser/utils/commands/sources.ts b/src/browser/utils/commands/sources.ts index fba9c4fb94e..ebb70ea94fe 100644 --- a/src/browser/utils/commands/sources.ts +++ b/src/browser/utils/commands/sources.ts @@ -99,6 +99,7 @@ export interface BuildSourcesParams { } | null; /** Project-scoped preference ID used while a creation composer is active. */ creationScopeId?: string | null; + codexOauthAccountId?: string; streamingModels?: Map; // UI actions getThinkingLevel: (workspaceId: string) => ThinkingLevel; @@ -1304,6 +1305,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi getFastModeProvider(providerOptionGateModel ?? "", { providersConfig: p.providersConfig, resolvedRouteProvider: providerOptionRoute, + codexOauthAccountId: p.codexOauthAccountId, }) != null ? { id: CommandIds.toggleFastMode(), @@ -1416,6 +1418,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi openaiProModeAvailable(proGateModelString ?? "", { providersConfig: p.providersConfig, resolvedRouteProvider: currentModelRoute, + codexOauthAccountId: p.codexOauthAccountId, }) ) { const proActive = p.getReasoningMode(workspaceId) === "pro"; diff --git a/src/browser/utils/compaction/contextSwitchCheck.test.ts b/src/browser/utils/compaction/contextSwitchCheck.test.ts index 1b4efddff8b..8f7a8ea87d6 100644 --- a/src/browser/utils/compaction/contextSwitchCheck.test.ts +++ b/src/browser/utils/compaction/contextSwitchCheck.test.ts @@ -10,6 +10,24 @@ const OPTIONS = { }; describe("checkContextSwitch", () => { + test("caps the context limit for the project account", () => { + const warning = checkContextSwitch(350_000, "openai:gpt-5.5", "google:gemini-2.5-pro", false, { + ...OPTIONS, + codexOauthAccountId: "work", + providersConfig: { + openai: { + apiKeySet: true, + isEnabled: true, + isConfigured: true, + codexOauthSet: true, + codexOauthAccounts: [{ id: "work", label: "Work" }], + }, + }, + }); + expect(warning).not.toBeNull(); + expect(warning?.targetLimit).toBeLessThan(350_000); + }); + test("returns null when target model matches previous model", () => { const targetModel = "openai:gpt-5.2-codex"; const limit = getEffectiveContextLimit(targetModel, false); diff --git a/src/browser/utils/compaction/contextSwitchCheck.ts b/src/browser/utils/compaction/contextSwitchCheck.ts index 8210bbfe60a..c255318ea4e 100644 --- a/src/browser/utils/compaction/contextSwitchCheck.ts +++ b/src/browser/utils/compaction/contextSwitchCheck.ts @@ -41,6 +41,7 @@ export function findPreviousModel(messages: DisplayedMessage[]): string | null { /** Options for accessibility checks in context switch validation */ export interface ContextSwitchOptions extends CompactionRouteOptions { providersConfig: ProvidersConfigMap | null; + codexOauthAccountId?: string; policy: EffectivePolicy | null; } @@ -63,7 +64,9 @@ function resolveCompactionModel( routePriority: options.routePriority, routeOverrides: options.routeOverrides, }); - const limit = getEffectiveContextLimit(preferred, use1M, options.providersConfig); + const limit = getEffectiveContextLimit(preferred, use1M, options.providersConfig, { + codexOauthAccountId: options.codexOauthAccountId, + }); if (accessible && limit && limit > currentTokens) return preferred; } if (previousModel) { @@ -74,7 +77,9 @@ function resolveCompactionModel( routePriority: options.routePriority, routeOverrides: options.routeOverrides, }); - const limit = getEffectiveContextLimit(previousModel, use1M, options.providersConfig); + const limit = getEffectiveContextLimit(previousModel, use1M, options.providersConfig, { + codexOauthAccountId: options.codexOauthAccountId, + }); if (accessible && limit && limit > currentTokens) return previousModel; } return null; @@ -108,7 +113,9 @@ export function checkContextSwitch( return null; } - const targetLimit = getEffectiveContextLimit(targetModel, use1M, options.providersConfig); + const targetLimit = getEffectiveContextLimit(targetModel, use1M, options.providersConfig, { + codexOauthAccountId: options.codexOauthAccountId, + }); // Unknown model or context fits with 10% buffer - no warning if (!targetLimit || currentTokens <= targetLimit * CONTEXT_FIT_THRESHOLD) { diff --git a/src/browser/utils/fastModeServiceTier.test.ts b/src/browser/utils/fastModeServiceTier.test.ts index ba3820602f2..f139ef00e2d 100644 --- a/src/browser/utils/fastModeServiceTier.test.ts +++ b/src/browser/utils/fastModeServiceTier.test.ts @@ -20,6 +20,30 @@ function createWriter() { } describe("fast mode service tier", () => { + test("uses the project account and preserves the API-key preference", () => { + const providersConfig = { + openai: { + apiKeySet: true, + isEnabled: true, + isConfigured: true, + codexOauthSet: true, + codexOauthAccounts: [{ id: "work", label: "Work" }], + }, + }; + expect( + getFastModeProvider("openai:gpt-5.5", { + providersConfig, + codexOauthAccountId: "work", + }) + ).toBeNull(); + expect( + getFastModeProvider("openai:gpt-5.5", { + providersConfig: { openai: { ...providersConfig.openai, codexOauthDefaultAuth: "apiKey" } }, + codexOauthAccountId: "work", + }) + ).toBe("openai"); + }); + test("resolves direct native providers and rejects gateway routes", () => { expect(getFastModeProvider("openai:gpt-5.6-sol", { resolvedRouteProvider: "direct" })).toBe( "openai" diff --git a/src/browser/utils/fastModeServiceTier.ts b/src/browser/utils/fastModeServiceTier.ts index 3b08ea1255a..ddf5a831a29 100644 --- a/src/browser/utils/fastModeServiceTier.ts +++ b/src/browser/utils/fastModeServiceTier.ts @@ -21,6 +21,7 @@ export interface FastModeServiceTierChange { export interface FastModeAvailabilityOptions { resolvedRouteProvider?: string | null; providersConfig?: ProvidersConfigMap | null; + codexOauthAccountId?: string; } type ProviderConfigWriter = Pick; diff --git a/src/cli/run.ts b/src/cli/run.ts index b5624fc04bd..143de9c0649 100644 --- a/src/cli/run.ts +++ b/src/cli/run.ts @@ -15,7 +15,11 @@ import { z } from "zod"; import * as path from "path"; import * as fs from "fs/promises"; import { createConfigStores } from "../node/config"; -import { materializeResolvedTrust, replaceRunTrustProjects } from "./trust"; +import { + materializeCodexOauthAccount, + materializeResolvedTrust, + replaceRunTrustProjects, +} from "./trust"; import { runBestEffortCleanup } from "./runCleanup"; import { DisposableTempDir } from "../node/services/tempDir"; import { AgentSession, type AgentSessionChatEvent } from "../node/services/agentSession"; @@ -653,7 +657,6 @@ async function main(): Promise { initStateManager, backgroundProcessManager, mcpServerManager, - providerService, workspaceService, workspaceGoalService, idleDispatcher, @@ -681,11 +684,6 @@ async function main(): Promise { : undefined, }); - // `xum run` uses createCoreServices directly (without ServiceContainer), so wire - // Codex OAuth explicitly to ensure Codex-routed OpenAI requests can load/refresh - // OAuth tokens from providers.jsonc. - const codexOauthService = new CodexOauthService(runProvidersStore, providerService); - turnRequestBuilderBindings.codexOauthService = codexOauthService; // Same for Coder OAuth: coder:* models need per-request token loading/refresh. // Bind it to the REAL config (not the ephemeral tempDir copy): Coder rotates // the refresh token on every use, so persisting rotations only to tempDir @@ -698,6 +696,9 @@ async function main(): Promise { realProvidersStore, realFileLeaseManager ); + // Keep Codex token rotations after the temporary CLI root is removed. + const codexOauthService = new CodexOauthService(realProvidersStore, realProviderService); + turnRequestBuilderBindings.codexOauthService = codexOauthService; const coderOauthService = new CoderOauthService( realProvidersStore, realFileLeaseManager, @@ -887,6 +888,9 @@ async function main(): Promise { projectName: path.basename(projectDir), runtimeConfig, }); + const registered = config.findWorkspace(workspaceId); + assert(registered, "CLI workspace registration must exist"); + await materializeCodexOauthAccount(realConfig, config, projectDir, registered.projectPath); // Note: taskService.initialize() is intentionally NOT called. It resumes tasks from a // previous session, but xum run uses an ephemeral Config (temp dir) with no prior state. diff --git a/src/cli/trust.test.ts b/src/cli/trust.test.ts index 11b969f707f..0fa2733f046 100644 --- a/src/cli/trust.test.ts +++ b/src/cli/trust.test.ts @@ -4,7 +4,12 @@ import * as path from "node:path"; import { describe, expect, test } from "bun:test"; import { DisposableTempDir } from "@/node/services/tempDir"; import { Config } from "@/node/config"; -import { materializeResolvedTrust, replaceRunTrustProjects, resolveProjectDir } from "./trust"; +import { + materializeCodexOauthAccount, + materializeResolvedTrust, + replaceRunTrustProjects, + resolveProjectDir, +} from "./trust"; const BUN_EXECUTABLE = process.execPath; const TRUST_ENTRY = path.join(import.meta.dir, "trust.ts"); @@ -98,6 +103,56 @@ describe("xum trust CLI", () => { expect(trustByPath.get(worktree)).toBe(false); }, 15_000); + test("copies Codex overrides without requiring project trust", async () => { + using tmp = new DisposableTempDir("codex-project-copy"); + const real = new Config(path.join(tmp.path, "real")); + const target = new Config(path.join(tmp.path, "target")); + const projectPath = path.join(tmp.path, "project"); + await real.editConfig((config) => { + config.projects.set(projectPath, { workspaces: [], codexOauthAccountId: "work" }); + return config; + }); + await replaceRunTrustProjects(real, target); + expect(target.loadConfigOrDefault().projects.get(projectPath)).toMatchObject({ + codexOauthAccountId: "work", + workspaces: [], + }); + await real.editConfig((config) => { + config.projects.delete(projectPath); + return config; + }); + await replaceRunTrustProjects(real, target); + expect(target.loadConfigOrDefault().projects.has(projectPath)).toBe(false); + }); + + test("materializes subproject accounts onto temporary CLI workspace projects", async () => { + using tmp = new DisposableTempDir("codex-worktree-copy"); + const real = new Config(path.join(tmp.path, "real")); + const target = new Config(path.join(tmp.path, "target")); + const root = path.join(tmp.path, "project"); + const subproject = path.join(root, "subproject"); + const worktree = path.join(tmp.path, "checkout"); + const cliProject = path.join(tmp.path, "cli-project"); + await real.editConfig((config) => { + config.projects.set(root, { + codexOauthAccountId: "personal", + workspaces: [{ path: worktree, id: "test-account-workspace", subProjectPath: subproject }], + }); + config.projects.set(subproject, { workspaces: [], codexOauthAccountId: "work" }); + return config; + }); + await materializeCodexOauthAccount(real, target, worktree, cliProject); + expect(target.loadConfigOrDefault().projects.get(cliProject)?.codexOauthAccountId).toBe("work"); + await real.editConfig((config) => { + delete config.projects.get(subproject)!.codexOauthAccountId; + return config; + }); + await materializeCodexOauthAccount(real, target, worktree, cliProject); + expect( + target.loadConfigOrDefault().projects.get(cliProject)?.codexOauthAccountId + ).toBeUndefined(); + }); + test("replaceRunTrustProjects rebuilds config without foreign settings", async () => { using tmp = new DisposableTempDir("trust-replace-run"); const realConfig = new Config(path.join(tmp.path, "real-root")); diff --git a/src/cli/trust.ts b/src/cli/trust.ts index 5ec85f4b11a..d6294627ead 100644 --- a/src/cli/trust.ts +++ b/src/cli/trust.ts @@ -168,26 +168,61 @@ export async function resolveProjectTrusted( return mainRepoDir != null && isProjectTrusted(realConfig, mainRepoDir); } -/** Replace all run-root trust entries so removed grants cannot survive root reuse. */ +/** Replace run project settings so removed trust grants and account overrides cannot survive root reuse. */ export async function replaceRunTrustProjects( realConfig: Config, targetConfig: Config ): Promise { const trustOnlyProjects = new Map(); for (const [projectPath, projectConfig] of realConfig.loadConfigOrDefault().projects) { - if (projectConfig.trusted === undefined) { + if (projectConfig.trusted === undefined && projectConfig.codexOauthAccountId === undefined) { continue; } trustOnlyProjects.set(projectPath, { workspaces: [], trusted: projectConfig.trusted, + codexOauthAccountId: projectConfig.codexOauthAccountId, }); } await targetConfig.editConfig(() => ({ projects: trustOnlyProjects })); } +/** Copy the source project account onto the CLI workspace project. */ +export async function materializeCodexOauthAccount( + realConfig: Config, + targetConfig: Config, + projectDir: string, + targetProjectPath: string +): Promise { + const projects = realConfig.loadConfigOrDefault().projects; + let sourcePath: string | undefined = projects.has(projectDir) ? projectDir : undefined; + if (sourcePath === undefined) { + for (const [projectPath, project] of projects) { + const workspace = project.workspaces.find((entry) => entry.path === projectDir); + if (workspace) { + sourcePath = + workspace.projects?.[0]?.projectPath ?? workspace.subProjectPath ?? projectPath; + break; + } + } + } + sourcePath ??= + (await findMainRepoDir(projectDir)) ?? (await findGitRoot(projectDir)) ?? projectDir; + const accountId = projects.get(sourcePath)?.codexOauthAccountId; + await targetConfig.editConfig((config) => { + const project = config.projects.get(targetProjectPath) ?? { workspaces: [] }; + if (accountId === undefined) { + delete project.codexOauthAccountId; + } else { + project.codexOauthAccountId = accountId; + } + config.projects.set(targetProjectPath, project); + return config; + }); +} + /** * Copy effective trust onto the target path because the task-spawn gate uses exact lookups. */ diff --git a/src/cli/workflow.ts b/src/cli/workflow.ts index baccd8b04c6..b95ce46f48f 100644 --- a/src/cli/workflow.ts +++ b/src/cli/workflow.ts @@ -9,7 +9,6 @@ import * as path from "node:path"; import { Command } from "commander"; import { EXPERIMENT_IDS, LEGACY_PTC_EXCLUSIVE_EXPERIMENT_ID } from "@/common/constants/experiments"; -import type { ProjectConfig } from "@/common/types/project"; import { parseRuntimeModeAndHost, RUNTIME_MODE, type RuntimeConfig } from "@/common/types/runtime"; import { DEFAULT_THINKING_LEVEL, @@ -46,7 +45,12 @@ import { hasAnyConfiguredProvider, buildProvidersFromEnv } from "@/node/utils/pr import { runBestEffortCleanup } from "./runCleanup"; import { getParseOptions } from "./argv"; import { exitAfterStdoutFlush } from "./processExit"; -import { resolveProjectDir, resolveProjectTrusted } from "./trust"; +import { + materializeCodexOauthAccount, + replaceRunTrustProjects, + resolveProjectDir, + resolveProjectTrusted, +} from "./trust"; const VALID_EXPERIMENT_IDS = new Set(Object.values(EXPERIMENT_IDS)); const THINKING_LABELS_LIST = [...new Set(Object.values(THINKING_DISPLAY_LABELS))].join(", "); @@ -226,17 +230,7 @@ async function copyPersistentConfig( await runStores.secretsStore.saveSecretsConfig(existingSecrets); } - const existingConfig = realConfig.loadConfigOrDefault(); - const trustOnlyProjects = new Map(); - for (const [projectPath, projectConfig] of existingConfig.projects) { - if (projectConfig.trusted !== undefined) { - trustOnlyProjects.set(projectPath, { workspaces: [], trusted: projectConfig.trusted }); - } - } - if (trustOnlyProjects.size > 0) { - // Config.saveConfig is private (lost-update safety); route through the queue. - await config.editConfig((cfg) => ({ ...cfg, projects: trustOnlyProjects })); - } + await replaceRunTrustProjects(realConfig, config); } function buildExperimentsObject(experimentIds: readonly string[]) { @@ -370,8 +364,6 @@ async function createWorkflowContext(options: { extensionMetadataPath: path.join(tempDir.path, "extensionMetadata.json"), mcpConfig: realConfig, }); - codexOauthService = new CodexOauthService(runProvidersStore, services.providerService); - services.turnRequestBuilderBindings.codexOauthService = codexOauthService; // Bind Coder OAuth to the REAL config (not the ephemeral tempDir copy): // Coder rotates the refresh token on every use, so persisting rotations // only to tempDir would strand ~/.xum/providers.jsonc with a consumed @@ -382,6 +374,9 @@ async function createWorkflowContext(options: { realProvidersStore, realFileLeaseManager ); + // Keep Codex token rotations after the temporary CLI root is removed. + codexOauthService = new CodexOauthService(realProvidersStore, realProviderService); + services.turnRequestBuilderBindings.codexOauthService = codexOauthService; coderOauthService = new CoderOauthService( realProvidersStore, realFileLeaseManager, @@ -427,6 +422,14 @@ async function createWorkflowContext(options: { projectName: path.basename(options.projectDir), runtimeConfig, }); + const registered = config.findWorkspace(workspaceId); + assert(registered, "Workflow workspace registration must exist"); + await materializeCodexOauthAccount( + realConfig, + config, + options.projectDir, + registered.projectPath + ); assert(workspacePath.length > 0, "xum workflow workspace path must be non-empty"); return { diff --git a/src/common/constants/codexOauthAccounts.ts b/src/common/constants/codexOauthAccounts.ts new file mode 100644 index 00000000000..7dd2eb0f32b --- /dev/null +++ b/src/common/constants/codexOauthAccounts.ts @@ -0,0 +1,9 @@ +// Local slot IDs differ from ChatGPT account IDs in token claims. +export const CODEX_OAUTH_DEFAULT_ACCOUNT_ID = "default"; +export const CODEX_OAUTH_REFRESH_TIMEOUT_MS = 30_000; +export const CODEX_OAUTH_REFRESH_LOCK_TIMEOUT_MS = 45_000; +export const CODEX_OAUTH_REFRESH_LOCK_STALE_MS = 60_000; +export const CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH = 200; +export const CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH = 100; +export const CODEX_OAUTH_ACCOUNT_ID_PATTERN = /^[a-zA-Z0-9_-]+$/; +export const CODEX_OAUTH_RESERVED_ACCOUNT_IDS = new Set(["__proto__", "constructor", "prototype"]); diff --git a/src/common/orpc/schemas/api.test.ts b/src/common/orpc/schemas/api.test.ts index abee162fbf3..f7099644e1e 100644 --- a/src/common/orpc/schemas/api.test.ts +++ b/src/common/orpc/schemas/api.test.ts @@ -4,6 +4,8 @@ import { ProviderConfigInfoSchema, ProvidersConfigMapSchema, config, + codexOauth, + projects, workspace, } from "./api"; import type { AWSCredentialStatus, ProviderConfigInfo, ProvidersConfigMap } from "../types"; @@ -18,6 +20,35 @@ import type { AWSCredentialStatus, ProviderConfigInfo, ProvidersConfigMap } from * If these tests fail, it means the schema is missing fields that the backend * service returns, which would cause data loss when crossing the IPC boundary. */ +describe("Codex account input validation", () => { + it.each(["__proto__", "constructor", "prototype", "", "work/account"])( + "rejects unsafe slot ID %s at each mutation boundary", + (accountId) => { + expect(codexOauth.startDesktopFlow.input.safeParse({ accountId }).success).toBe(false); + expect(codexOauth.startDeviceFlow.input.safeParse({ accountId }).success).toBe(false); + expect(codexOauth.disconnect.input.safeParse({ accountId }).success).toBe(false); + expect(codexOauth.setDefaultAccount.input.safeParse({ accountId }).success).toBe(false); + expect(codexOauth.renameAccount.input.safeParse({ accountId, label: "Work" }).success).toBe( + false + ); + expect( + projects.setCodexOauthAccount.input.safeParse({ projectPath: "/project", accountId }) + .success + ).toBe(false); + } + ); + + it("allows legacy login and explicit project inheritance", () => { + expect(codexOauth.startDesktopFlow.input.safeParse(undefined).success).toBe(true); + expect(codexOauth.startDeviceFlow.input.safeParse(undefined).success).toBe(true); + expect( + projects.setCodexOauthAccount.input.safeParse({ projectPath: "/project", accountId: null }) + .success + ).toBe(true); + expect(codexOauth.startDesktopFlow.input.safeParse({ label: " " }).success).toBe(false); + }); +}); + describe("ProviderConfigInfoSchema conformance", () => { it("preserves all AWSCredentialStatus fields", () => { const full: AWSCredentialStatus = { @@ -118,6 +149,8 @@ describe("ProviderConfigInfoSchema conformance", () => { cacheTtl: "1h", disableBetaFeatures: true, codexOauthSet: true, + codexOauthAccounts: [{ id: "work", label: "Work" }], + codexOauthDefaultAccountId: "work", codexOauthDefaultAuth: "apiKey", aws: { region: "ap-northeast-1", diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index 5b4d8b73f6b..6b33b259dee 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -6,6 +6,12 @@ import { import { eventIterator } from "@orpc/server"; import { UIModeSchema } from "../../types/mode"; import { z } from "zod"; +import { + CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH, + CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH, + CODEX_OAUTH_ACCOUNT_ID_PATTERN, + CODEX_OAUTH_RESERVED_ACCOUNT_IDS, +} from "@/common/constants/codexOauthAccounts"; import { CODER_ARCHIVE_BEHAVIORS } from "@/common/config/coderArchiveBehavior"; import { WORKTREE_ARCHIVE_BEHAVIORS } from "@/common/config/worktreeArchiveBehavior"; import { HEARTBEAT_MAX_INTERVAL_MS, HEARTBEAT_MIN_INTERVAL_MS } from "@/constants/heartbeat"; @@ -290,6 +296,9 @@ export const ProviderConfigInfoSchema = z.object({ disableBetaFeatures: z.boolean().optional(), /** OpenAI-only: whether Codex OAuth tokens are present in providers.jsonc */ codexOauthSet: z.boolean().optional(), + /** Connected account labels. Credentials stay in the backend. */ + codexOauthAccounts: z.array(z.object({ id: z.string(), label: z.string() })).optional(), + codexOauthDefaultAccountId: z.string().optional(), /** * OpenAI-only: default auth precedence to use for Codex-OAuth-allowed models when BOTH * ChatGPT OAuth and an OpenAI API key are configured. @@ -540,9 +549,28 @@ export const muxGovernorOauth = { }; // Codex OAuth (ChatGPT subscription auth) +const CodexOauthAccountIdSchema = z + .string() + .min(1) + .max(CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH) + .regex(CODEX_OAUTH_ACCOUNT_ID_PATTERN) + .refine((id) => !CODEX_OAUTH_RESERVED_ACCOUNT_IDS.has(id), "Invalid account ID"); +const CodexOauthAccountLabelSchema = z + .string() + .trim() + .min(1) + .max(CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH); +const CodexOauthLoginInputSchema = z + .object({ + label: CodexOauthAccountLabelSchema.optional(), + accountId: CodexOauthAccountIdSchema.optional(), + }) + .strict() + .optional(); + export const codexOauth = { startDesktopFlow: { - input: z.void(), + input: CodexOauthLoginInputSchema, output: ResultSchema(z.object({ flowId: z.string(), authorizeUrl: z.string() }), z.string()), }, waitForDesktopFlow: { @@ -559,7 +587,7 @@ export const codexOauth = { output: z.void(), }, startDeviceFlow: { - input: z.void(), + input: CodexOauthLoginInputSchema, output: ResultSchema( z.object({ flowId: z.string(), @@ -584,7 +612,17 @@ export const codexOauth = { output: z.void(), }, disconnect: { - input: z.void(), + input: z.object({ accountId: CodexOauthAccountIdSchema }).strict().optional(), + output: ResultSchema(z.void(), z.string()), + }, + setDefaultAccount: { + input: z.object({ accountId: CodexOauthAccountIdSchema }).strict(), + output: ResultSchema(z.void(), z.string()), + }, + renameAccount: { + input: z + .object({ accountId: CodexOauthAccountIdSchema, label: CodexOauthAccountLabelSchema }) + .strict(), output: ResultSchema(z.void(), z.string()), }, }; @@ -835,6 +873,12 @@ export const projects = { .passthrough(), output: z.void(), }, + setCodexOauthAccount: { + input: z + .object({ projectPath: z.string(), accountId: CodexOauthAccountIdSchema.nullable() }) + .strict(), + output: ResultSchema(z.void(), z.string()), + }, setCustomInstructions: { input: z .object({ @@ -2279,6 +2323,7 @@ export const nameGeneration = { generate: { input: z.object({ message: z.string(), + projectPath: z.string().optional(), /** Ordered list of model candidates to try (backend resolves gateway routing in createModel) */ candidates: z.array(z.string()), }), diff --git a/src/common/schemas/project.ts b/src/common/schemas/project.ts index bd1c33dc78e..15282f25e0b 100644 --- a/src/common/schemas/project.ts +++ b/src/common/schemas/project.ts @@ -285,6 +285,8 @@ export const WorkspaceConfigSchema = z.object({ }); export const ProjectConfigSchema = z.object({ + /** Use this account for the project. Unset inherits the global default. */ + codexOauthAccountId: z.string().min(1).optional(), displayName: z.string().nullish().meta({ description: "Custom display name for the project", }), diff --git a/src/common/utils/ai/openaiProviderOptionsAvailability.ts b/src/common/utils/ai/openaiProviderOptionsAvailability.ts index 9c69028fc4d..85fc733241d 100644 --- a/src/common/utils/ai/openaiProviderOptionsAvailability.ts +++ b/src/common/utils/ai/openaiProviderOptionsAvailability.ts @@ -15,6 +15,7 @@ export interface OpenAIDirectProviderOptionsAvailability { providersConfig?: ProvidersConfigMap | null; /** Request-level OpenAI wire format; the stored config value wins when set. */ openaiWireFormat?: OpenAIWireFormat | null; + codexOauthAccountId?: string; } export function openaiDirectProviderOptionsAvailable( @@ -55,6 +56,7 @@ export function openaiDirectProviderOptionsAvailable( options?.providersConfig != null && wouldRouteOpenAIThroughCodexOauth(normalized, options.providersConfig, { openaiWireFormat: options.openaiWireFormat, + codexOauthAccountId: options.codexOauthAccountId, }) ); } diff --git a/src/common/utils/providers/codexOauthRouting.test.ts b/src/common/utils/providers/codexOauthRouting.test.ts new file mode 100644 index 00000000000..6886808af1c --- /dev/null +++ b/src/common/utils/providers/codexOauthRouting.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from "bun:test"; +import type { ProvidersConfigMap } from "@/common/orpc/types"; +import { hasCodexOauthTokens, wouldRouteOpenAIThroughCodexOauth } from "./codexOauthRouting"; +import { openaiDirectProviderOptionsAvailable } from "@/common/utils/ai/openaiProviderOptionsAvailability"; + +const auth = { type: "oauth", access: "access", refresh: "refresh", expires: 1000 }; + +describe("Codex OAuth account routing", () => { + it("keeps legacy metadata and raw token defaults compatible", () => { + for (const config of [{ codexOauthSet: true }, { codexOauth: auth }]) { + expect(hasCodexOauthTokens(config)).toBe(true); + expect(hasCodexOauthTokens(config, "default")).toBe(true); + expect(hasCodexOauthTokens(config, "missing")).toBe(false); + } + }); + + it("uses metadata account IDs instead of the aggregate connection flag", () => { + const config = { + codexOauthSet: true, + codexOauthAccounts: [{ id: "work", label: "Work" }], + codexOauthDefaultAccountId: "missing", + }; + expect(hasCodexOauthTokens(config)).toBe(false); + expect(hasCodexOauthTokens(config, "default")).toBe(false); + expect(hasCodexOauthTokens(config, "work")).toBe(true); + expect(hasCodexOauthTokens({ ...config, codexOauthDefaultAccountId: "work" })).toBe(true); + expect(hasCodexOauthTokens({ ...config, codexOauthAccounts: [] }, "work")).toBe(false); + }); + + it("selects raw account maps without substituting legacy tokens", () => { + const config = { + codexOauth: auth, + codexOauthAccounts: { + work: { label: "Work", auth }, + invalid: { label: "Invalid", auth: { ...auth, refresh: "" } }, + }, + codexOauthDefaultAccountId: "work", + }; + expect(hasCodexOauthTokens(config)).toBe(true); + expect(hasCodexOauthTokens(config, "default")).toBe(true); + expect(hasCodexOauthTokens(config, "missing")).toBe(false); + expect(hasCodexOauthTokens(config, "invalid")).toBe(false); + expect(hasCodexOauthTokens({ ...config, codexOauthAccounts: {} })).toBe(false); + }); + + it("threads selected accounts into direct provider option availability", () => { + const providersConfig: ProvidersConfigMap = { + openai: { + apiKeySet: true, + isConfigured: true, + isEnabled: true, + codexOauthSet: true, + codexOauthDefaultAccountId: "deleted", + codexOauthAccounts: [{ id: "work", label: "Work" }], + }, + }; + const model = "openai:gpt-5.5"; + expect(wouldRouteOpenAIThroughCodexOauth(model, providersConfig)).toBe(false); + expect( + wouldRouteOpenAIThroughCodexOauth(model, providersConfig, { codexOauthAccountId: "work" }) + ).toBe(true); + expect( + openaiDirectProviderOptionsAvailable(model, { providersConfig, codexOauthAccountId: "work" }) + ).toBe(false); + expect( + wouldRouteOpenAIThroughCodexOauth(model, providersConfig, { + codexOauthAccountId: "work", + openaiWireFormat: "chatCompletions", + }) + ).toBe(false); + providersConfig.openai.codexOauthDefaultAuth = "apiKey"; + expect( + wouldRouteOpenAIThroughCodexOauth(model, providersConfig, { codexOauthAccountId: "work" }) + ).toBe(false); + }); +}); diff --git a/src/common/utils/providers/codexOauthRouting.ts b/src/common/utils/providers/codexOauthRouting.ts index ad777860236..875d24241b7 100644 --- a/src/common/utils/providers/codexOauthRouting.ts +++ b/src/common/utils/providers/codexOauthRouting.ts @@ -12,6 +12,7 @@ import { isCodexOauthAllowedModel, isCodexOauthRequiredModel } from "@/common/constants/codexOAuth"; import type { ProvidersConfigMap } from "@/common/orpc/types"; import type { OpenAIWireFormat } from "@/common/types/providerOptions"; +import { CODEX_OAUTH_DEFAULT_ACCOUNT_ID } from "@/common/constants/codexOauthAccounts"; /** Request-level inputs the stored providers config cannot carry. */ export interface CodexOauthRoutingOptions { @@ -20,6 +21,8 @@ export interface CodexOauthRoutingOptions { * The stored `openai.wireFormat` wins when set, matching providerModelFactory. */ openaiWireFormat?: OpenAIWireFormat | null; + /** Local account slot selected for this request. */ + codexOauthAccountId?: string; } function asRecord(value: unknown): Record | null { @@ -33,25 +36,41 @@ function hasNonEmptyString(value: unknown): value is string { return typeof value === "string" && value.trim().length > 0; } -export function hasCodexOauthTokens(config: unknown): boolean { +export function hasCodexOauthTokens(config: unknown, accountId?: string): boolean { const record = asRecord(config); if (!record) { return false; } - if (record.codexOauthSet === true) { + const selectedId = + accountId ?? record.codexOauthDefaultAccountId ?? CODEX_OAUTH_DEFAULT_ACCOUNT_ID; + if (Array.isArray(record.codexOauthAccounts)) { + return record.codexOauthAccounts.some( + (account: unknown) => asRecord(account)?.id === selectedId + ); + } + + // Old metadata contains only the legacy connection flag. + if (record.codexOauthSet === true && selectedId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID) { return true; } - // Backend compaction can receive raw providers.jsonc config in older tests/fallback paths. - // Detect the stored token shape without importing node-only OAuth parsing into common code. - const oauth = asRecord(record.codexOauth); + // Raw configs contain tokens. Never substitute another connected account. + const accounts = asRecord(record.codexOauthAccounts); + const selectedAccount = typeof selectedId === "string" ? asRecord(accounts?.[selectedId]) : null; + if (selectedId !== CODEX_OAUTH_DEFAULT_ACCOUNT_ID && !hasNonEmptyString(selectedAccount?.label)) { + return false; + } + const oauth = asRecord( + selectedId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID ? record.codexOauth : selectedAccount?.auth + ); return ( oauth?.type === "oauth" && hasNonEmptyString(oauth.access) && hasNonEmptyString(oauth.refresh) && typeof oauth.expires === "number" && - Number.isFinite(oauth.expires) + Number.isFinite(oauth.expires) && + (oauth.accountId === undefined || hasNonEmptyString(oauth.accountId)) ); } @@ -86,7 +105,7 @@ export function wouldRouteOpenAIThroughCodexOauth( if (!isCodexOauthAllowedModel(model, providersConfig ?? null)) { return false; } - if (!hasCodexOauthTokens(openAIConfig)) { + if (!hasCodexOauthTokens(openAIConfig, options?.codexOauthAccountId)) { return false; } // Codex OAuth serves only the Responses API. With Chat Completions selected, diff --git a/src/common/utils/tokens/tokenMeterUtils.test.ts b/src/common/utils/tokens/tokenMeterUtils.test.ts index 5531640f97c..74080d583c6 100644 --- a/src/common/utils/tokens/tokenMeterUtils.test.ts +++ b/src/common/utils/tokens/tokenMeterUtils.test.ts @@ -76,6 +76,37 @@ describe("calculateTokenMeterData", () => { expect(result.totalPercentage).toBeCloseTo(1.1); }); + test("uses the selected account rather than an unavailable global account", () => { + const providersConfig: ProvidersConfigMap = { + openai: { + apiKeySet: true, + isEnabled: true, + isConfigured: true, + codexOauthSet: true, + codexOauthDefaultAccountId: "missing", + codexOauthAccounts: [{ id: "work", label: "Work" }], + }, + }; + const globalMeter = calculateTokenMeterData( + SAMPLE_USAGE, + "openai:gpt-5.5", + false, + false, + providersConfig + ); + const projectMeter = calculateTokenMeterData( + SAMPLE_USAGE, + "openai:gpt-5.5", + false, + false, + providersConfig, + { codexOauthAccountId: "work" } + ); + expect(projectMeter.maxTokens).toBe(272_000); + expect(globalMeter.maxTokens).toBeGreaterThan(projectMeter.maxTokens!); + expect(projectMeter.totalPercentage).toBeGreaterThan(globalMeter.totalPercentage); + }); + test("uses the Codex OAuth cap for GPT-5.5 token meter percentages", () => { const result = calculateTokenMeterData(SAMPLE_USAGE, "openai:gpt-5.5", false, false, { openai: { diff --git a/src/common/utils/tokens/tokenMeterUtils.ts b/src/common/utils/tokens/tokenMeterUtils.ts index 77b542d9e43..30f5e5833bd 100644 --- a/src/common/utils/tokens/tokenMeterUtils.ts +++ b/src/common/utils/tokens/tokenMeterUtils.ts @@ -1,5 +1,6 @@ import type { ProvidersConfigMap } from "@/common/orpc/types"; import { getEffectiveContextLimit } from "@/common/utils/compaction/contextLimit"; +import type { CodexOauthRoutingOptions } from "@/common/utils/providers/codexOauthRouting"; import type { ChatUsageDisplay } from "./usageAggregator"; // NOTE: Provide theme-matching fallbacks so token meters render consistently @@ -60,11 +61,13 @@ export function calculateTokenMeterData( model: string, use1M: boolean, verticalProportions = false, - providersConfig: ProvidersConfigMap | null = null + providersConfig: ProvidersConfigMap | null = null, + routingOptions?: CodexOauthRoutingOptions ): TokenMeterData { if (!usage) return { segments: [], totalTokens: 0, totalPercentage: 0 }; - const maxTokens = getEffectiveContextLimit(model, use1M, providersConfig) ?? undefined; + const maxTokens = + getEffectiveContextLimit(model, use1M, providersConfig, routingOptions) ?? undefined; // Total tokens used in the request. // For Anthropic prompt caching, cacheCreate tokens are reported separately but still diff --git a/src/node/config.test.ts b/src/node/config.test.ts index 4da8df62961..de7ad5a142e 100644 --- a/src/node/config.test.ts +++ b/src/node/config.test.ts @@ -337,6 +337,25 @@ describe("Config", () => { }); describe("loadConfigOrDefault customInstructions sanitizing", () => { + it("sanitizes malformed Codex selections but retains disconnected account IDs", () => { + fs.writeFileSync( + path.join(tempDir, "config.json"), + JSON.stringify({ + projects: [ + ["/home/user/number", { workspaces: [], codexOauthAccountId: 42 }], + ["/home/user/blank", { workspaces: [], codexOauthAccountId: " " }], + ["/home/user/disconnected", { workspaces: [], codexOauthAccountId: "removed-account" }], + ], + }) + ); + const loaded = config.loadConfigOrDefault(); + expect(loaded.projects.get("/home/user/number")?.codexOauthAccountId).toBeUndefined(); + expect(loaded.projects.get("/home/user/blank")?.codexOauthAccountId).toBeUndefined(); + expect(loaded.projects.get("/home/user/disconnected")?.codexOauthAccountId).toBe( + "removed-account" + ); + }); + it("discards malformed non-string customInstructions and keeps valid ones", () => { // A malformed value must not survive load: it would fail the // projects.list z.string() output schema and brick the project list. diff --git a/src/node/config/fileLeaseManager.ts b/src/node/config/fileLeaseManager.ts index 9a52bd05cb6..bc63e9d0d3e 100644 --- a/src/node/config/fileLeaseManager.ts +++ b/src/node/config/fileLeaseManager.ts @@ -2,6 +2,10 @@ import * as crypto from "crypto"; import * as fs from "fs"; import * as path from "path"; import { getXumHome } from "@/common/constants/paths"; +import { + CODEX_OAUTH_REFRESH_LOCK_TIMEOUT_MS, + CODEX_OAUTH_REFRESH_LOCK_STALE_MS, +} from "@/common/constants/codexOauthAccounts"; import { log } from "@/node/services/log"; import { ensurePrivateDirSync } from "@/node/utils/fs"; @@ -42,6 +46,18 @@ export class FileLeaseManager { return this.withDirLock(`${this.providersFile}.coder-refresh.lock`, 45_000, 60_000, fn); } + /** Serialize each Codex slot across processes before consuming its refresh token. */ + async withCodexOauthRefreshLock(accountId: string, fn: () => Promise | T): Promise { + // Hash slot IDs to keep lock paths bounded and independent of user input. + const slotKey = crypto.createHash("sha256").update(accountId).digest("hex"); + return this.withDirLock( + `${this.providersFile}.codex-refresh-${slotKey}.lock`, + CODEX_OAUTH_REFRESH_LOCK_TIMEOUT_MS, + CODEX_OAUTH_REFRESH_LOCK_STALE_MS, + fn + ); + } + /** Keeps each desktop login rollback snapshot anchored to committed credentials. */ async withCoderOauthLoginCommitLock(fn: () => Promise | T): Promise { return this.withDirLock(`${this.providersFile}.coder-login.lock`, 15_000, 20_000, fn); diff --git a/src/node/config/index.ts b/src/node/config/index.ts index 385d4a64181..f518eeb7768 100644 --- a/src/node/config/index.ts +++ b/src/node/config/index.ts @@ -722,6 +722,7 @@ function normalizeProjectRuntimeSettings(projectConfig: ProjectConfig): ProjectC projectKind?: unknown; customInstructions?: unknown; codeWorkspaceSyncPath?: unknown; + codexOauthAccountId?: unknown; }; const runtimeEnablement = normalizeRuntimeEnablementOverrides(record.runtimeEnablement); const defaultRuntime = normalizeRuntimeEnablementId(record.defaultRuntime); @@ -766,6 +767,13 @@ function normalizeProjectRuntimeSettings(projectConfig: ProjectConfig): ProjectC delete next.customInstructions; } + // Invalid disk values must not break the project list. Preserve missing account IDs for explicit recovery. + if (typeof record.codexOauthAccountId === "string" && record.codexOauthAccountId.trim()) { + next.codexOauthAccountId = record.codexOauthAccountId; + } else { + delete next.codexOauthAccountId; + } + // Same hand-edit hazard as customInstructions above. if (typeof record.codeWorkspaceSyncPath === "string" && record.codeWorkspaceSyncPath.trim()) { next.codeWorkspaceSyncPath = record.codeWorkspaceSyncPath; @@ -2932,6 +2940,7 @@ export class Config { workspacePath: string; projectPath: string; attributionProjectPath?: string; + subProjectPath?: string; projects?: Workspace["projects"]; workspaceName?: string; parentWorkspaceId?: string; @@ -2951,6 +2960,7 @@ export class Config { // config.projects.get(projectPath), even for multi-project workspaces under _multi. projectPath, attributionProjectPath, + subProjectPath: workspace.subProjectPath, projects: workspace.projects, workspaceName: workspace.name, parentWorkspaceId: workspace.parentWorkspaceId, @@ -2993,6 +3003,7 @@ export class Config { workspacePath: workspace.path, projectPath, attributionProjectPath, + subProjectPath: metadata.subProjectPath ?? workspace.subProjectPath, projects: metadata.projects ?? workspace.projects, workspaceName: undefined, parentWorkspaceId: undefined, @@ -3042,6 +3053,7 @@ export class Config { workspacePath: workspace.path, projectPath, attributionProjectPath, + subProjectPath: legacyMetadata.subProjectPath ?? workspace.subProjectPath, projects: legacyMetadata.projects ?? workspace.projects, workspaceName: undefined, parentWorkspaceId: undefined, @@ -3061,6 +3073,7 @@ export class Config { workspacePath: workspace.path, projectPath, attributionProjectPath, + subProjectPath: workspace.subProjectPath, projects: workspace.projects, workspaceName: undefined, parentWorkspaceId: undefined, diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index 02bfc7ae578..b8f2aa5ee66 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -776,8 +776,8 @@ export const router = (authToken?: string) => { .input(schemas.codexOauth.startDesktopFlow.input) .output(schemas.codexOauth.startDesktopFlow.output) .handler( - handlerGen(function* ({ context }) { - return yield* context.codexOauthService.startDesktopFlowEffect(); + handlerGen(function* ({ context }, input) { + return yield* context.codexOauthService.startDesktopFlowEffect(input); }) ), waitForDesktopFlow: t @@ -802,8 +802,8 @@ export const router = (authToken?: string) => { .input(schemas.codexOauth.startDeviceFlow.input) .output(schemas.codexOauth.startDeviceFlow.output) .handler( - handlerGen(function* ({ context }) { - return yield* context.codexOauthService.startDeviceFlowEffect(); + handlerGen(function* ({ context }, input) { + return yield* context.codexOauthService.startDeviceFlowEffect(input); }) ), waitForDeviceFlow: t @@ -828,8 +828,27 @@ export const router = (authToken?: string) => { .input(schemas.codexOauth.disconnect.input) .output(schemas.codexOauth.disconnect.output) .handler( - handlerGen(function* ({ context }) { - return yield* context.codexOauthService.disconnectEffect(); + handlerGen(function* ({ context }, input) { + return yield* context.codexOauthService.disconnectEffect(input?.accountId); + }) + ), + setDefaultAccount: t + .input(schemas.codexOauth.setDefaultAccount.input) + .output(schemas.codexOauth.setDefaultAccount.output) + .handler( + handlerGen(function* ({ context }, input) { + return yield* context.codexOauthService.setDefaultAccountEffect(input.accountId); + }) + ), + renameAccount: t + .input(schemas.codexOauth.renameAccount.input) + .output(schemas.codexOauth.renameAccount.output) + .handler( + handlerGen(function* ({ context }, input) { + return yield* context.codexOauthService.renameAccountEffect( + input.accountId, + input.label + ); }) ), }, @@ -1183,6 +1202,12 @@ export const router = (authToken?: string) => { .handler(({ context, input }) => context.projectService.setColor(input.projectPath, input.color) ), + setCodexOauthAccount: t + .input(schemas.projects.setCodexOauthAccount.input) + .output(schemas.projects.setCodexOauthAccount.output) + .handler(({ context, input }) => + context.projectService.setCodexOauthAccount(input.projectPath, input.accountId) + ), setCustomInstructions: t .input(schemas.projects.setCustomInstructions.input) .output(schemas.projects.setCustomInstructions.output) @@ -1329,7 +1354,16 @@ export const router = (authToken?: string) => { .input(schemas.nameGeneration.generate.input) .output(schemas.nameGeneration.generate.output) .handler(({ context, input }) => - generateWorkspaceIdentity(input.message, input.candidates, context.aiService) + generateWorkspaceIdentity( + input.message, + input.candidates, + context.aiService, + undefined, + undefined, + { + projectPath: input.projectPath, + } + ) ), }, coder: { diff --git a/src/node/services/agentStatusService.ts b/src/node/services/agentStatusService.ts index 83abc8b2c04..1f4f870d128 100644 --- a/src/node/services/agentStatusService.ts +++ b/src/node/services/agentStatusService.ts @@ -405,6 +405,7 @@ export class AgentStatusService { // would leak background LLM work past our lifecycle. if (this.stopped) return; const result = await generateWorkspaceStatus(transcript, candidates, this.aiService, { + workspaceId, streaming, recordUsage: async (modelString, usage, usageOptions) => { const recorded = await this.sessionUsageService?.recordHeadlessUsage( diff --git a/src/node/services/aiService.test.ts b/src/node/services/aiService.test.ts index b85a2ea7143..2e5c02303b1 100644 --- a/src/node/services/aiService.test.ts +++ b/src/node/services/aiService.test.ts @@ -10,6 +10,7 @@ import { describe, it, expect, beforeEach, afterEach, mock, spyOn } from "bun:te import { resolveModelForMetadata } from "@/common/utils/providers/modelEntries"; import { AIService, resolveMuxProjectRootForHostFs } from "./aiService"; +import { getEffectiveContextLimit } from "@/common/utils/compaction/contextLimit"; import { discoverAvailableSubagentsForToolContext } from "./turnContextAssembler"; import { normalizeAnthropicBaseURL, @@ -302,6 +303,7 @@ function stubCommonStreamMessageDependencies(args: { metadata: WorkspaceMetadata; startStreamCalls?: TurnExecutionOptions[]; routeProvider?: ProviderName; + codexOauthAccountId?: string; allTools?: Record; workspacePathOverride?: string; historySequence?: number; @@ -375,6 +377,7 @@ function stubCommonStreamMessageDependencies(args: { wireProviderName: args.canonicalProviderName ?? providerNameFromModelString(canonicalModelString), routedThroughGateway: false, + codexOauthAccountId: args.codexOauthAccountId, ...(args.routeProvider != null ? { routeProvider: args.routeProvider } : {}), }, }); @@ -1087,6 +1090,7 @@ describe("AIService.streamMessage compaction boundary slicing", () => { metadata: WorkspaceMetadata, options?: { routeProvider?: ProviderName; + codexOauthAccountId?: string; allTools?: Record; postPolicyTools?: Record; sessionUsageService?: SessionUsageService; @@ -1122,6 +1126,7 @@ describe("AIService.streamMessage compaction boundary slicing", () => { metadata, startStreamCalls, routeProvider: options?.routeProvider, + codexOauthAccountId: options?.codexOauthAccountId, allTools: options?.allTools, effectiveModelString: options?.effectiveModelString, canonicalProviderName: options?.canonicalProviderName, @@ -1167,6 +1172,52 @@ describe("AIService.streamMessage compaction boundary slicing", () => { }; } + it("pins the turn context limit to the model account rather than the global default", async () => { + using xumHome = new DisposableTempDir("ai-service-codex-account-snapshot"); + const projectPath = path.join(xumHome.path, "project"); + await fs.mkdir(projectPath, { recursive: true }); + const workspaceId = "account-snapshot"; + const harness = createHarness( + xumHome.path, + createLocalWorkspaceMetadata(workspaceId, projectPath), + { + effectiveModelString: "openai:gpt-5.5", + codexOauthAccountId: "work", + } + ); + new ProvidersConfigStore(harness.config.rootDir).saveProvidersConfig({ + openai: { + apiKey: "test-key", + codexOauthDefaultAccountId: "missing", + codexOauthAccounts: { + work: { + label: "Work", + auth: { + type: "oauth", + access: "access", + refresh: "refresh", + expires: Date.now() + 60_000, + }, + }, + }, + }, + }); + const result = await harness.service.streamMessage({ + messages: [createMuxMessage("user", "user", "continue")], + workspaceId, + modelString: "openai:gpt-5.5", + thinkingLevel: "off", + }); + expect(result.success).toBe(true); + const snapshot = harness.startStreamCalls[0]?.providersConfigSnapshot; + expect(snapshot?.openai?.codexOauthDefaultAccountId).toBe("work"); + expect(getEffectiveContextLimit("openai:gpt-5.5", false, snapshot)).toBe(272_000); + expect( + new ProvidersConfigStore(harness.config.rootDir).loadProvidersConfig()?.openai + ?.codexOauthDefaultAccountId + ).toBe("missing"); + }); + interface AdvisorRuntimeForTests { createModel: (modelString: string) => Promise; takeToolCallSnapshot: (toolCallId: string) => diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index 92aaacf28a2..c4c5b8efed0 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -543,6 +543,7 @@ export class AIService extends EventEmitter { opts?: { agentInitiated?: boolean; workspaceId?: string; + projectPath?: string; /** Snapshot pass-through (see ProviderModelFactory.createModel). */ providersConfig?: ProvidersConfig; } @@ -560,7 +561,7 @@ export class AIService extends EventEmitter { */ async createModelWithPinnedMetadata( modelString: string, - opts?: { agentInitiated?: boolean; workspaceId?: string } + opts?: { agentInitiated?: boolean; workspaceId?: string; projectPath?: string } ): Promise> { const providersConfig = this.providersConfigStore.loadProvidersConfig() ?? {}; const result = await this.providerModelFactory.createModel(modelString, undefined, { diff --git a/src/node/services/codexOauthService.test.ts b/src/node/services/codexOauthService.test.ts index df8209b13e9..b308b71a8fe 100644 --- a/src/node/services/codexOauthService.test.ts +++ b/src/node/services/codexOauthService.test.ts @@ -1,10 +1,20 @@ -import type { ProvidersConfigStore } from "@/node/config"; +import { Config, FileLeaseManager, ProvidersConfigStore } from "@/node/config"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; import { describe, it, expect, beforeEach, afterEach } from "bun:test"; -import type { Result } from "@/common/types/result"; -import { Ok } from "@/common/types/result"; +import { Err, Ok } from "@/common/types/result"; +import { Effect } from "effect"; +import { getCodexOauthAccounts, getCodexOauthAuth } from "@/node/utils/codexOauthAuth"; +import { createDeferred } from "@/node/utils/oauthUtils"; +import { + CODEX_OAUTH_TOKEN_URL, + CODEX_OAUTH_DEVICE_USERCODE_URL, + CODEX_OAUTH_DEVICE_TOKEN_POLL_URL, +} from "@/common/constants/codexOAuth"; import type { ProvidersConfig } from "@/node/config"; -import type { ProviderService } from "@/node/services/providerService"; +import { ProviderService } from "@/node/services/providerService"; import type { WindowService } from "@/node/services/windowService"; import type { CodexOauthAuth } from "@/node/utils/codexOauthAuth"; import { CodexOauthService } from "./codexOauthService"; @@ -49,13 +59,17 @@ function mockRefreshResponse(body: Record, status = 200): Respo // --------------------------------------------------------------------------- interface MockDeps { + rootDir: string; providersConfig: ProvidersConfig; setConfigValueCalls: Array<{ provider: string; keyPath: string[]; value: unknown }>; focusCalls: number; + onUpdate?: () => void; + policyDenied?: boolean; } function createMockDeps(): MockDeps { return { + rootDir: fs.mkdtempSync(path.join(os.tmpdir(), "xum-codex-oauth-")), providersConfig: {}, setConfigValueCalls: [], focusCalls: 0, @@ -64,33 +78,45 @@ function createMockDeps(): MockDeps { function createMockProvidersConfigStore( deps: MockDeps -): Pick { +): Pick { return { + rootDir: deps.rootDir, loadProvidersConfig: () => deps.providersConfig, }; } -function createMockProviderService(deps: MockDeps): Pick { +function createMockProviderService( + deps: MockDeps +): Pick { + const setConfigValue: ProviderService["setConfigValue"] = (provider, keyPath, value) => { + deps.setConfigValueCalls.push({ provider, keyPath, value }); + deps.providersConfig[provider] ??= {}; + let current = deps.providersConfig[provider] as Record; + for (const key of keyPath.slice(0, -1)) { + current[key] ??= {}; + current = current[key] as Record; + } + const key = keyPath[keyPath.length - 1]; + if (value === undefined) delete current[key]; + else current[key] = value; + return Promise.resolve(Ok(undefined)); + }; return { - setConfigValue: ( - provider: string, - keyPath: string[], - value: unknown - ): Promise> => { - deps.setConfigValueCalls.push({ provider, keyPath, value }); - // Also update the in-memory config so readStoredAuth() sees the write - if (provider === "openai" && keyPath[0] === "codexOauth") { - if (value === undefined) { - const openai = deps.providersConfig.openai; - if (openai) { - delete openai.codexOauth; - } - } else { - deps.providersConfig.openai ??= {}; - deps.providersConfig.openai.codexOauth = value; - } + setConfigValue, + updateConfigValue: async (provider, keyPath, update, options) => { + if (options?.enforcePolicy && deps.policyDenied) return Err("Provider edits are disabled"); + let current: unknown = deps.providersConfig[provider]; + for (const key of keyPath) { + current = + current !== null && typeof current === "object" + ? (current as Record)[key] + : undefined; } - return Promise.resolve(Ok(undefined)); + const next = update(current); + deps.onUpdate?.(); + if (!next) return Ok({ applied: false }); + await setConfigValue(provider, keyPath, next.value); + return Ok({ applied: true }); }, }; } @@ -111,6 +137,13 @@ function createService(deps: MockDeps): CodexOauthService { ); } +function requestBody(init?: RequestInit): string { + const body = init?.body; + if (typeof body === "string") return body; + if (body instanceof URLSearchParams) return body.toString(); + throw new Error("Expected a string or URLSearchParams request body"); +} + // Helper to mock globalThis.fetch without needing the `preconnect` property. function mockFetch(fn: (input: RequestInfo | URL, init?: RequestInit) => Promise): void { globalThis.fetch = Object.assign(fn, { @@ -137,6 +170,7 @@ describe("CodexOauthService", () => { afterEach(async () => { globalThis.fetch = originalFetch; await service.dispose(); + fs.rmSync(deps.rootDir, { recursive: true, force: true }); }); // ------------------------------------------------------------------------- @@ -451,4 +485,559 @@ describe("CodexOauthService", () => { } }); }); + describe("account selection", () => { + it("uses the global selection and an explicit override without falling back", async () => { + const legacy = validAuth({ access: "legacy" }); + const work = validAuth({ access: "work", accountId: "chatgpt-work" }); + deps.providersConfig = { + openai: { + codexOauth: legacy, + codexOauthAccounts: { work: { label: "Work", auth: work } }, + codexOauthDefaultAccountId: "work", + }, + }; + expect(await service.getValidAuth()).toEqual(Ok(work)); + expect(await service.getValidAuth("default")).toEqual(Ok(legacy)); + expect((await service.getValidAuth("missing")).success).toBe(false); + expect(await service.disconnect("work")).toEqual(Ok(undefined)); + expect((await service.getValidAuth()).success).toBe(false); + expect(await service.getValidAuth("default")).toEqual(Ok(legacy)); + }); + + it("renames each slot and selects defaults without changing credentials", async () => { + const legacy = validAuth(); + const work = validAuth({ refresh: "work" }); + deps.providersConfig = { + openai: { codexOauth: legacy, codexOauthAccounts: { work: { label: "Work", auth: work } } }, + }; + expect(await Effect.runPromise(service.renameAccountEffect("default", " Personal "))).toEqual( + Ok(undefined) + ); + expect(await service.renameAccount("work", " Team ")).toEqual(Ok(undefined)); + expect(await Effect.runPromise(service.setDefaultAccountEffect("work"))).toEqual( + Ok(undefined) + ); + expect(getCodexOauthAccounts(deps.providersConfig.openai)).toEqual([ + { id: "default", label: "Personal", auth: legacy }, + { id: "work", label: "Team", auth: work }, + ]); + expect(await service.getValidAuth()).toEqual(Ok(work)); + expect((await service.setDefaultAccount("missing")).success).toBe(false); + expect((await service.renameAccount("missing", "Name")).success).toBe(false); + }); + + it("enforces policy for account edits and token refresh", async () => { + const stored = expiredAuth(); + deps.providersConfig = { openai: { codexOauth: stored } }; + deps.policyDenied = true; + expect((await service.setDefaultAccount("default")).success).toBe(false); + expect((await service.renameAccount("default", "Renamed")).success).toBe(false); + expect((await service.disconnect()).success).toBe(false); + mockFetch(() => + Promise.resolve(mockRefreshResponse({ access_token: "refreshed", expires_in: 3600 })) + ); + expect((await service.getValidAuth()).success).toBe(false); + expect(getCodexOauthAuth(deps.providersConfig.openai)).toEqual(stored); + expect(deps.setConfigValueCalls).toHaveLength(0); + }); + + it("enforces policy when a revoked refresh attempts to clear credentials", async () => { + const stored = expiredAuth(); + deps.providersConfig = { openai: { codexOauth: stored } }; + deps.policyDenied = true; + mockFetch(() => Promise.resolve(mockRefreshResponse({ error: "invalid_grant" }, 400))); + expect((await service.getValidAuth()).success).toBe(false); + expect(getCodexOauthAuth(deps.providersConfig.openai)).toEqual(stored); + expect(deps.setConfigValueCalls).toHaveLength(0); + }); + + it("rejects unsafe slot IDs and invalid labels before storage writes", async () => { + for (const id of ["", "__proto__", "constructor", "prototype", "../bad", "x".repeat(201)]) { + expect((await service.disconnect(id)).success).toBe(false); + expect((await service.setDefaultAccount(id)).success).toBe(false); + expect((await service.renameAccount(id, "Name")).success).toBe(false); + expect((await service.startDeviceFlow({ accountId: id })).success).toBe(false); + expect((await service.startDesktopFlow({ accountId: id })).success).toBe(false); + } + for (const label of [" ", "x".repeat(101)]) { + expect((await service.renameAccount("default", label)).success).toBe(false); + expect((await service.startDeviceFlow({ label })).success).toBe(false); + } + expect((await service.startDeviceFlow({ accountId: "default", label: "Name" })).success).toBe( + false + ); + expect(deps.setConfigValueCalls).toHaveLength(0); + }); + }); + + describe("account refresh isolation", () => { + it("shares a refresh lease across service instances and persists the rotated token", async () => { + const store = new ProvidersConfigStore(deps.rootDir); + store.saveProvidersConfig({ + openai: { + codexOauthAccounts: { work: { label: "Work", auth: expiredAuth({ refresh: "old" }) } }, + }, + }); + const secondAttempt = createDeferred(); + class ObservedLeaseManager extends FileLeaseManager { + override withCodexOauthRefreshLock( + accountId: string, + fn: () => Promise | T + ): Promise { + secondAttempt.resolve(undefined); + return super.withCodexOauthRefreshLock(accountId, fn); + } + } + const firstService = new CodexOauthService( + store, + new ProviderService(new Config(deps.rootDir)) + ); + const secondService = new CodexOauthService( + new ProvidersConfigStore(deps.rootDir), + new ProviderService(new Config(deps.rootDir)), + undefined, + new ObservedLeaseManager(deps.rootDir) + ); + const refreshStarted = createDeferred(); + const refreshResponse = createDeferred(); + let refreshCount = 0; + mockFetch(() => { + refreshCount++; + refreshStarted.resolve(undefined); + return refreshResponse.promise; + }); + try { + const first = firstService.getValidAuth("work"); + await refreshStarted.promise; + const second = secondService.getValidAuth("work"); + await secondAttempt.promise; + refreshResponse.resolve( + mockRefreshResponse({ + access_token: "refreshed", + refresh_token: "rotated", + expires_in: 3600, + }) + ); + const results = await Promise.all([first, second]); + expect(results[0].success).toBe(true); + expect(results[1]).toEqual(results[0]); + expect(refreshCount).toBe(1); + expect( + getCodexOauthAuth( + new ProvidersConfigStore(deps.rootDir).loadProvidersConfig()?.openai, + "work" + )?.refresh + ).toBe("rotated"); + } finally { + await firstService.dispose(); + await secondService.dispose(); + } + }); + + it("refreshes two accounts concurrently and keeps both writes", async () => { + deps.providersConfig = { + openai: { + codexOauth: expiredAuth({ refresh: "legacy" }), + codexOauthAccounts: { work: { label: "Work", auth: expiredAuth({ refresh: "work" }) } }, + }, + }; + const bothStarted = createDeferred(); + const release = createDeferred(); + let count = 0; + mockFetch(async (_input, init) => { + if (++count === 2) bothStarted.resolve(undefined); + await release.promise; + const token = new URLSearchParams(requestBody(init)).get("refresh_token"); + return mockRefreshResponse({ access_token: token + "-new", expires_in: 3600 }); + }); + const legacy = service.getValidAuth(); + const work = service.getValidAuth("work"); + await bothStarted.promise; + release.resolve(undefined); + expect((await legacy).success).toBe(true); + expect((await work).success).toBe(true); + expect(getCodexOauthAuth(deps.providersConfig.openai, "default")?.access).toBe("legacy-new"); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")?.access).toBe("work-new"); + }); + + it("pins the default while a refresh runs and preserves the ChatGPT identity", async () => { + deps.providersConfig = { + openai: { + codexOauth: expiredAuth({ accountId: "chatgpt-original" }), + codexOauthAccounts: { work: { label: "Work", auth: validAuth({ access: "work" }) } }, + }, + }; + const started = createDeferred(); + const response = createDeferred(); + mockFetch(() => { + started.resolve(undefined); + return response.promise; + }); + const pending = service.getValidAuth(); + await started.promise; + expect(await service.setDefaultAccount("work")).toEqual(Ok(undefined)); + response.resolve( + mockRefreshResponse({ + access_token: fakeJwt({ chatgpt_account_id: "chatgpt-other" }), + expires_in: 3600, + }) + ); + const result = await pending; + expect(result.success).toBe(true); + if (result.success) expect(result.data.accountId).toBe("chatgpt-original"); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")?.access).toBe("work"); + }); + + it.each([200, 400])( + "does not overwrite a reconnect after an old refresh returns %s", + async (status) => { + deps.providersConfig = { openai: { codexOauth: expiredAuth() } }; + const started = createDeferred(); + const response = createDeferred(); + mockFetch(() => { + started.resolve(undefined); + return response.promise; + }); + const pending = service.getValidAuth(); + await started.promise; + // A separate process changes the same slot without this service's revision map. + const reconnected = validAuth({ access: "reconnected", refresh: "new-session" }); + deps.providersConfig.openai.codexOauth = reconnected; + response.resolve( + mockRefreshResponse( + status === 200 + ? { access_token: "stale", expires_in: 3600 } + : { error: "invalid_grant" }, + status + ) + ); + expect((await pending).success).toBe(false); + expect(await service.getValidAuth()).toEqual(Ok(reconnected)); + } + ); + + it("does not restore a disconnected slot after refresh", async () => { + const work = expiredAuth(); + const legacy = validAuth({ access: "legacy" }); + deps.providersConfig = { + openai: { codexOauth: legacy, codexOauthAccounts: { work: { label: "Work", auth: work } } }, + }; + const started = createDeferred(); + const response = createDeferred(); + mockFetch(() => { + started.resolve(undefined); + return response.promise; + }); + const pending = service.getValidAuth("work"); + await started.promise; + await service.disconnect("work"); + response.resolve(mockRefreshResponse({ access_token: "stale", expires_in: 3600 })); + expect((await pending).success).toBe(false); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")).toBeNull(); + expect(await service.getValidAuth("default")).toEqual(Ok(legacy)); + }); + + it("keeps a concurrent rename when refreshed credentials persist", async () => { + deps.providersConfig = { + openai: { codexOauthAccounts: { work: { label: "Work", auth: expiredAuth() } } }, + }; + const started = createDeferred(); + const response = createDeferred(); + mockFetch(() => { + started.resolve(undefined); + return response.promise; + }); + const pending = service.getValidAuth("work"); + await started.promise; + await service.renameAccount("work", "Team"); + response.resolve(mockRefreshResponse({ access_token: "new", expires_in: 3600 })); + expect((await pending).success).toBe(true); + expect(getCodexOauthAccounts(deps.providersConfig.openai)[0].label).toBe("Team"); + }); + }); + + describe("login destinations", () => { + function deviceFetch(exchange?: (init?: RequestInit) => Promise): void { + let nextCode = 0; + mockFetch(async (input, init) => { + const url = input instanceof Request ? input.url : input.toString(); + if (url === CODEX_OAUTH_DEVICE_USERCODE_URL) { + return mockRefreshResponse({ + device_auth_id: "device-" + ++nextCode, + user_code: "code", + interval: 1, + expires_in: 60, + }); + } + if (url === CODEX_OAUTH_DEVICE_TOKEN_POLL_URL) { + const body = JSON.parse(requestBody(init)) as { device_auth_id: string }; + return mockRefreshResponse({ + authorization_code: body.device_auth_id, + code_verifier: "verifier", + }); + } + if (url === CODEX_OAUTH_TOKEN_URL) { + if (exchange) return exchange(init); + return mockRefreshResponse({ + access_token: "access-" + new URLSearchParams(requestBody(init)).get("code"), + refresh_token: "refresh", + expires_in: 3600, + }); + } + throw new Error("Unexpected fetch URL"); + }); + } + + it("does not save login credentials when policy denies provider edits", async () => { + deps.policyDenied = true; + deviceFetch(); + const flow = await service.startDeviceFlow({ label: "Work" }); + if (!flow.success) throw new Error(flow.error); + expect((await service.waitForDeviceFlow(flow.data.flowId)).success).toBe(false); + expect(getCodexOauthAccounts(deps.providersConfig.openai)).toEqual([]); + expect(deps.setConfigValueCalls).toHaveLength(0); + }); + + it("creates a named slot and selects the first account globally", async () => { + deviceFetch(); + const flow = await service.startDeviceFlow({ label: " Personal " }); + if (!flow.success) throw new Error(flow.error); + expect(await service.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + const accounts = getCodexOauthAccounts(deps.providersConfig.openai); + expect(accounts).toHaveLength(1); + expect(accounts[0].id).not.toBe("default"); + expect(accounts[0].label).toBe("Personal"); + expect(await service.getValidAuth()).toEqual(Ok(accounts[0].auth)); + expect(deps.providersConfig.openai?.codexOauth).toBeUndefined(); + }); + + it("keeps concurrent named logins in separate slots", async () => { + deps.providersConfig = { openai: { codexOauth: validAuth({ access: "legacy" }) } }; + deviceFetch(); + const first = await service.startDeviceFlow({ label: "One" }); + const second = await service.startDeviceFlow({ label: "Two" }); + if (!first.success || !second.success) throw new Error("Login start failed"); + expect(await service.setDefaultAccount("default")).toEqual(Ok(undefined)); + const results = await Promise.all([ + service.waitForDeviceFlow(first.data.flowId), + service.waitForDeviceFlow(second.data.flowId), + ]); + expect(results.every((result) => result.success)).toBe(true); + const accounts = getCodexOauthAccounts(deps.providersConfig.openai); + expect(accounts).toHaveLength(3); + expect(new Set(accounts.map((account) => account.id)).size).toBe(3); + expect(accounts.map((account) => account.auth.access).sort()).toEqual([ + "access-device-1", + "access-device-2", + "legacy", + ]); + expect(getCodexOauthAuth(deps.providersConfig.openai)?.access).toBe("legacy"); + }); + + it("selects one account when the first named logins finish concurrently", async () => { + deviceFetch(); + const first = await service.startDeviceFlow({ label: "One" }); + const second = await service.startDeviceFlow({ label: "Two" }); + if (!first.success || !second.success) throw new Error("Login start failed"); + const results = await Promise.all([ + service.waitForDeviceFlow(first.data.flowId), + service.waitForDeviceFlow(second.data.flowId), + ]); + expect(results.every((result) => result.success)).toBe(true); + expect(getCodexOauthAccounts(deps.providersConfig.openai)).toHaveLength(2); + expect((await service.getValidAuth()).success).toBe(true); + }); + + it("pins a named desktop login while the global default changes", async () => { + const legacy = validAuth({ access: "legacy" }); + deps.providersConfig = { openai: { codexOauth: legacy } }; + mockFetch(() => + Promise.resolve( + mockRefreshResponse({ + access_token: "desktop", + refresh_token: "desktop-refresh", + expires_in: 3600, + }) + ) + ); + const flow = await service.startDesktopFlow({ label: "Desktop" }); + if (!flow.success) throw new Error(flow.error); + await service.setDefaultAccount("default"); + const callback = await originalFetch( + "http://localhost:1455/auth/callback?code=auth-code&state=" + flow.data.flowId + ); + expect(callback.ok).toBe(true); + await callback.text(); + expect(await service.waitForDesktopFlow(flow.data.flowId)).toEqual(Ok(undefined)); + const accounts = getCodexOauthAccounts(deps.providersConfig.openai); + expect(accounts).toHaveLength(2); + expect(accounts[1].auth.access).toBe("desktop"); + expect(accounts[1].label).toBe("Desktop"); + expect(await service.getValidAuth()).toEqual(Ok(legacy)); + }); + + it("keeps no-argument login in the legacy slot despite a named default", async () => { + const work = validAuth({ access: "work" }); + deps.providersConfig = { + openai: { + codexOauthAccounts: { work: { label: "Work", auth: work } }, + codexOauthDefaultAccountId: "work", + }, + }; + deviceFetch(); + const flow = await service.startDeviceFlow(); + if (!flow.success) throw new Error(flow.error); + expect(await service.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + expect(getCodexOauthAuth(deps.providersConfig.openai, "default")?.access).toBe( + "access-device-1" + ); + expect(await service.getValidAuth()).toEqual(Ok(work)); + }); + + it.each(["disconnect", "cancel"])("does not persist an exchange after %s", async (action) => { + deps.providersConfig = { + openai: { codexOauthAccounts: { work: { label: "Work", auth: validAuth() } } }, + }; + const started = createDeferred(); + const response = createDeferred(); + deviceFetch(() => { + started.resolve(undefined); + return response.promise; + }); + const flow = await service.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + const pending = service.waitForDeviceFlow(flow.data.flowId); + await started.promise; + if (action === "disconnect") await service.disconnect("work"); + else await service.cancelDeviceFlow(flow.data.flowId); + const writeCount = deps.setConfigValueCalls.length; + const compared = createDeferred(); + deps.onUpdate = () => compared.resolve(undefined); + response.resolve( + mockRefreshResponse({ access_token: "stale", refresh_token: "stale", expires_in: 3600 }) + ); + expect((await pending).success).toBe(false); + await compared.promise; + deps.onUpdate = undefined; + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")?.access).not.toBe("stale"); + if (action === "disconnect") expect(deps.setConfigValueCalls).toHaveLength(writeCount); + }); + + it("keeps refreshed credentials when a concurrent reconnect is cancelled", async () => { + deps.providersConfig = { + openai: { + codexOauthAccounts: { work: { label: "Work", auth: expiredAuth({ refresh: "old" }) } }, + }, + }; + const refreshStarted = createDeferred(); + const refreshResponse = createDeferred(); + deviceFetch(() => { + refreshStarted.resolve(undefined); + return refreshResponse.promise; + }); + const refresh = service.getValidAuth("work"); + await refreshStarted.promise; + const flow = await service.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + await service.cancelDeviceFlow(flow.data.flowId); + refreshResponse.resolve( + mockRefreshResponse({ + access_token: "refreshed", + refresh_token: "rotated", + expires_in: 3600, + }) + ); + const result = await refresh; + expect(result.success).toBe(true); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")?.refresh).toBe("rotated"); + expect(await service.getValidAuth("work")).toEqual(result); + }); + + it("keeps refreshed credentials when reconnect startup fails", async () => { + deps.providersConfig = { openai: { codexOauth: expiredAuth({ refresh: "old" }) } }; + const refreshStarted = createDeferred(); + const refreshResponse = createDeferred(); + mockFetch((input) => { + if (input === CODEX_OAUTH_DEVICE_USERCODE_URL) + return Promise.reject(new Error("Request failed")); + refreshStarted.resolve(undefined); + return refreshResponse.promise; + }); + const refresh = service.getValidAuth(); + await refreshStarted.promise; + expect((await service.startDeviceFlow({ accountId: "default" })).success).toBe(false); + refreshResponse.resolve( + mockRefreshResponse({ + access_token: "refreshed", + refresh_token: "rotated", + expires_in: 3600, + }) + ); + expect((await refresh).success).toBe(true); + expect(getCodexOauthAuth(deps.providersConfig.openai)?.refresh).toBe("rotated"); + }); + + it("lets only the latest reconnect attempt replace the same slot", async () => { + deps.providersConfig = { openai: { codexOauth: validAuth() } }; + deviceFetch(); + const first = await service.startDeviceFlow({ accountId: "default" }); + const second = await service.startDeviceFlow({ accountId: "default" }); + if (!first.success || !second.success) throw new Error("Login start failed"); + expect((await service.waitForDeviceFlow(first.data.flowId)).success).toBe(false); + expect(await service.waitForDeviceFlow(second.data.flowId)).toEqual(Ok(undefined)); + expect(getCodexOauthAuth(deps.providersConfig.openai)?.access).toBe("access-device-2"); + }); + + it("keeps a successful reconnect when an older refresh completes", async () => { + deps.providersConfig = { + openai: { + codexOauthAccounts: { work: { label: "Work", auth: expiredAuth({ refresh: "old" }) } }, + }, + }; + const refreshStarted = createDeferred(); + const refreshResponse = createDeferred(); + deviceFetch((init) => { + const body = new URLSearchParams(requestBody(init)); + if (body.get("grant_type") === "refresh_token") { + refreshStarted.resolve(undefined); + return refreshResponse.promise; + } + return Promise.resolve( + mockRefreshResponse({ + access_token: "reconnected", + refresh_token: "new", + expires_in: 3600, + }) + ); + }); + const refresh = service.getValidAuth("work"); + await refreshStarted.promise; + const flow = await service.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + expect(await service.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + refreshResponse.resolve(mockRefreshResponse({ access_token: "stale", expires_in: 3600 })); + expect((await refresh).success).toBe(false); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")?.access).toBe("reconnected"); + }); + + it("lets a reconnect replace credentials without changing its label or another slot", async () => { + const legacy = validAuth({ access: "legacy" }); + deps.providersConfig = { + openai: { + codexOauth: legacy, + codexOauthAccounts: { work: { label: "Work", auth: validAuth() } }, + }, + }; + deviceFetch(); + const flow = await service.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + await service.renameAccount("work", "Team"); + expect(await service.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + expect(getCodexOauthAccounts(deps.providersConfig.openai)[1].label).toBe("Team"); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")?.access).toBe( + "access-device-1" + ); + expect(await service.getValidAuth()).toEqual(Ok(legacy)); + }); + }); }); diff --git a/src/node/services/codexOauthService.ts b/src/node/services/codexOauthService.ts index 4b03ca842c1..1618519b572 100644 --- a/src/node/services/codexOauthService.ts +++ b/src/node/services/codexOauthService.ts @@ -25,7 +25,15 @@ import { CODEX_OAUTH_DEVICE_VERIFY_URL, CODEX_OAUTH_TOKEN_URL, } from "@/common/constants/codexOAuth"; -import type { ProvidersConfigStore } from "@/node/config"; +import { + CODEX_OAUTH_DEFAULT_ACCOUNT_ID, + CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH, + CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH, + CODEX_OAUTH_ACCOUNT_ID_PATTERN, + CODEX_OAUTH_RESERVED_ACCOUNT_IDS, + CODEX_OAUTH_REFRESH_TIMEOUT_MS, +} from "@/common/constants/codexOauthAccounts"; +import { FileLeaseManager, type ProvidersConfigStore } from "@/node/config"; import type { ProviderService } from "@/node/services/providerService"; import type { WindowService } from "@/node/services/windowService"; import { log } from "@/node/services/log"; @@ -33,6 +41,9 @@ import { sleepWithAbort } from "@/node/utils/abort"; import { AsyncMutex } from "@/node/utils/concurrency/asyncMutex"; import { extractAccountIdFromTokens, + getCodexOauthAccounts, + getCodexOauthAccountId, + getCodexOauthAuth, isCodexOauthAuthExpired, parseCodexOauthAuth, type CodexOauthAuth, @@ -46,7 +57,22 @@ const DEFAULT_DESKTOP_TIMEOUT_MS = 5 * 60 * 1000; const DEFAULT_DEVICE_TIMEOUT_MS = 15 * 60 * 1000; const COMPLETED_FLOW_TTL_MS = 60 * 1000; +export interface CodexOauthLoginOptions { + label?: string; + accountId?: string; +} + +interface AccountSelection { + accountId: string; + revision: number; + loginAttempt?: number; + auth: CodexOauthAuth | null; + label?: string; + selectAsDefault: boolean; +} + interface DeviceFlow { + destination: AccountSelection; flowId: string; deviceAuthId: string; userCode: string; @@ -118,50 +144,102 @@ export class CodexOauthError extends Schema.TaggedError()("Code reason: Schema.String, }) {} +function isValidAccountId(accountId: string): boolean { + return ( + accountId.length <= CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH && + CODEX_OAUTH_ACCOUNT_ID_PATTERN.test(accountId) && + !CODEX_OAUTH_RESERVED_ACCOUNT_IDS.has(accountId) + ); +} + +function isValidLabel(label: string): boolean { + return label.trim().length > 0 && label.trim().length <= CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH; +} + +function matchesAuth(actual: CodexOauthAuth | null, expected: CodexOauthAuth | null): boolean { + if (!actual || !expected) return actual === expected; + return ( + actual.access === expected.access && + actual.refresh === expected.refresh && + actual.expires === expected.expires && + actual.accountId === expected.accountId + ); +} + export class CodexOauthService { private readonly desktopFlows = new OAuthFlowManager(); private readonly deviceFlows = new Map(); - private readonly refreshMutex = new AsyncMutex(); - - // In-memory cache so getValidAuth() skips disk reads when tokens are valid. - // Invalidated on every write (exchange, refresh, disconnect). - private cachedAuth: CodexOauthAuth | null = null; + private readonly refreshMutexes = new Map(); + private readonly accountRevisions = new Map(); + private readonly loginAttempts = new Map(); constructor( private readonly providersConfigStore: ProvidersConfigStore, private readonly providerService: ProviderService, - private readonly windowService?: WindowService + private readonly windowService?: WindowService, + private readonly fileLeaseManager = new FileLeaseManager(providersConfigStore.rootDir) ) {} - async disconnect(): Promise> { - return Effect.runPromise(this.disconnectEffect()); + async disconnect(accountId?: string): Promise> { + return Effect.runPromise(this.disconnectEffect(accountId)); } - /** - * Wire-shaped Effect surface for handlerGen router handlers. Uninterruptible - * (mirrors asAtomicMutation in providerService.ts): a client abort must not - * skip the persisted-credential clear after the in-memory cache was already - * invalidated. - */ - disconnectEffect(): Effect.Effect> { - // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` - const self = this; - return Effect.uninterruptible( - Effect.gen(function* () { - // Clear stored ChatGPT OAuth tokens so Codex-only models are hidden again. - self.cachedAuth = null; - // setConfigValue resolves with a wire Result; a rejection stays a - // defect, matching the previously un-caught await. - return yield* Effect.promise(() => - self.providerService.setConfigValue("openai", ["codexOauth"], undefined) + // No argument retains legacy behavior. Account-aware callers pass the selected slot ID. + disconnectEffect( + accountId = CODEX_OAUTH_DEFAULT_ACCOUNT_ID + ): Effect.Effect> { + return Effect.suspend(() => { + if (!isValidAccountId(accountId)) + return Effect.succeed(Err("Invalid Codex OAuth account ID")); + // Invalidate pending refreshes and logins before clearing this slot. + this.accountRevisions.set(accountId, this.getAccountRevision(accountId) + 1); + return this.updateConfigValueEffect(this.accountPath(accountId), () => ({ + value: undefined, + })); + }); + } + + async setDefaultAccount(accountId: string): Promise> { + return Effect.runPromise(this.setDefaultAccountEffect(accountId)); + } + + setDefaultAccountEffect(accountId: string): Effect.Effect> { + return Effect.suspend(() => { + if (!isValidAccountId(accountId)) + return Effect.succeed(Err("Invalid Codex OAuth account ID")); + return this.updateConfigValueEffect(["codexOauthDefaultAccountId"], () => + this.readStoredAuth(accountId) ? { value: accountId } : null + ); + }); + } + + async renameAccount(accountId: string, label: string): Promise> { + return Effect.runPromise(this.renameAccountEffect(accountId, label)); + } + + renameAccountEffect(accountId: string, label: string): Effect.Effect> { + return Effect.suspend(() => { + if (!isValidAccountId(accountId)) + return Effect.succeed(Err("Invalid Codex OAuth account ID")); + if (!isValidLabel(label)) return Effect.succeed(Err("Invalid Codex OAuth account label")); + if (accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID) { + return this.updateConfigValueEffect(["codexOauthLabel"], () => + this.readStoredAuth(accountId) ? { value: label.trim() } : null ); - }) - ); + } + return this.updateConfigValueEffect(this.accountPath(accountId), (current) => + isPlainObject(current) && parseCodexOauthAuth(current.auth) + ? { value: { ...current, label: label.trim() } } + : null + ); + }); } - async startDesktopFlow(): Promise> { - return Effect.runPromise(this.startDesktopFlowEffect()); + async startDesktopFlow( + options?: CodexOauthLoginOptions + ): Promise> { + return Effect.runPromise(this.startDesktopFlowEffect(options)); } /** @@ -172,19 +250,19 @@ export class CodexOauthService { * and local, so running it to completion on abort is cheap; an abandoned * flow still self-cleans via the registered timeout. */ - startDesktopFlowEffect(): Effect.Effect< - Result<{ flowId: string; authorizeUrl: string }, string> - > { - return Effect.uninterruptible(toWireResult(this.launchDesktopFlowEffect())); + startDesktopFlowEffect( + options?: CodexOauthLoginOptions + ): Effect.Effect> { + return Effect.uninterruptible(toWireResult(this.launchDesktopFlowEffect(options))); } - private launchDesktopFlowEffect(): Effect.Effect< - { flowId: string; authorizeUrl: string }, - CodexOauthError - > { + private launchDesktopFlowEffect( + options?: CodexOauthLoginOptions + ): Effect.Effect<{ flowId: string; authorizeUrl: string }, CodexOauthError> { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; return Effect.gen(function* () { + const destination = yield* self.selectLoginDestination(options); const flowId = randomBase64Url(); const codeVerifier = randomBase64Url(); @@ -233,6 +311,7 @@ export class CodexOauthService { // cancelled. Effect.runFork( self.desktopCallbackPipeline({ + destination, flowId, redirectUri, codeVerifier, @@ -254,6 +333,7 @@ export class CodexOauthService { * dangling on loopback.result. */ private desktopCallbackPipeline(args: { + destination: AccountSelection; flowId: string; redirectUri: string; codeVerifier: string; @@ -277,6 +357,8 @@ export class CodexOauthService { const exchangeResult: Result = yield* toWireResult( self.handleDesktopCallbackAndExchange({ + destination: args.destination, + isActive: () => self.desktopFlows.has(args.flowId), flowId: args.flowId, redirectUri: args.redirectUri, codeVerifier: args.codeVerifier, @@ -338,7 +420,7 @@ export class CodexOauthService { ); } - async startDeviceFlow(): Promise< + async startDeviceFlow(options?: CodexOauthLoginOptions): Promise< Result< { flowId: string; @@ -349,7 +431,7 @@ export class CodexOauthService { string > > { - return Effect.runPromise(this.startDeviceFlowEffect()); + return Effect.runPromise(this.startDeviceFlowEffect(options)); } /** @@ -359,7 +441,9 @@ export class CodexOauthService { * flow record (and its expiry timeout) that lets callers re-attach or the * flow self-clean. */ - startDeviceFlowEffect(): Effect.Effect< + startDeviceFlowEffect( + options?: CodexOauthLoginOptions + ): Effect.Effect< Result<{ flowId: string; userCode: string; verifyUrl: string; intervalSeconds: number }, string> > { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` @@ -367,6 +451,7 @@ export class CodexOauthService { return Effect.uninterruptible( toWireResult( Effect.gen(function* () { + const destination = yield* self.selectLoginDestination(options); const flowId = randomBase64Url(); const { deviceAuthId, userCode, intervalSeconds, expiresAtMs } = @@ -387,6 +472,7 @@ export class CodexOauthService { }, timeoutMs); self.deviceFlows.set(flowId, { + destination, flowId, deviceAuthId, userCode, @@ -503,42 +589,54 @@ export class CodexOauthService { ); } - async getValidAuth(): Promise> { - return Effect.runPromise(this.getValidAuthEffect()); + async getValidAuth(accountId?: string): Promise> { + return Effect.runPromise(this.getValidAuthEffect(accountId)); } - getValidAuthEffect(): Effect.Effect> { - // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` + getValidAuthEffect(accountId?: string): Effect.Effect> { + // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect generators do not inherit this. const self = this; return Effect.gen(function* () { - const stored = self.readStoredAuth(); - if (!stored) { - return Err("Codex OAuth is not configured"); - } - - if (!isCodexOauthAuthExpired(stored)) { - return Ok(stored); + // Resolve once. A default change must not switch an active request to another account. + const selectedId = getCodexOauthAccountId(self.readOpenaiConfig(), accountId); + if (!isValidAccountId(selectedId)) return Err("Invalid Codex OAuth account ID"); + const revision = self.getAccountRevision(selectedId); + const stored = self.readStoredAuth(selectedId); + if (!stored) return Err(`Codex OAuth account "${selectedId}" is not configured`); + if (!isCodexOauthAuthExpired(stored)) return Ok(stored); + + let mutex = self.refreshMutexes.get(selectedId); + if (!mutex) { + mutex = new AsyncMutex(); + self.refreshMutexes.set(selectedId, mutex); } - - // acquireUseRelease guarantees the mutex is released on every exit path - // (refresh success/failure, defects, interruption) — the Effect - // equivalent of the pre-Effect `await using` lock. + const refreshMutex = mutex; return yield* Effect.acquireUseRelease( - Effect.promise(() => self.refreshMutex.acquire()), + Effect.promise(() => refreshMutex.acquire()), () => - Effect.gen(function* () { - // Re-read after acquiring lock in case another caller refreshed first. - const latest = self.readStoredAuth(); - if (!latest) { - return Err("Codex OAuth is not configured"); - } - - if (!isCodexOauthAuthExpired(latest)) { - return Ok(latest); - } - - return yield* toWireResult(self.refreshTokens(latest)); - }), + // Hold the file lease through persistence. Other processes must adopt the rotated token. + Effect.tryPromise({ + try: () => + self.fileLeaseManager.withCodexOauthRefreshLock(selectedId, async () => { + if (revision !== self.getAccountRevision(selectedId)) { + return Err("Codex OAuth account changed during the request"); + } + const latest = self.readStoredAuth(selectedId); + if (!latest) return Err(`Codex OAuth account "${selectedId}" is not configured`); + if (!isCodexOauthAuthExpired(latest)) return Ok(latest); + return await Effect.runPromise( + toWireResult( + self.refreshTokens( + { accountId: selectedId, revision, auth: latest, selectAsDefault: false }, + latest + ) + ) + ); + }), + catch: (error) => getErrorMessage(error), + }).pipe( + Effect.catch((error) => Effect.succeed(Err(`Codex OAuth refresh failed: ${error}`))) + ), (lock) => Effect.promise(() => lock[Symbol.asyncDispose]()) ); }); @@ -562,35 +660,159 @@ export class CodexOauthService { this.deviceFlows.clear(); } - private readStoredAuth(): CodexOauthAuth | null { - if (this.cachedAuth) { - return this.cachedAuth; - } - const providersConfig = this.providersConfigStore.loadProvidersConfig() ?? {}; - const openaiConfig = providersConfig.openai as Record | undefined; - const auth = parseCodexOauthAuth(openaiConfig?.codexOauth); - this.cachedAuth = auth; - return auth; + private readOpenaiConfig(): unknown { + return this.providersConfigStore.loadProvidersConfig()?.openai; } - private persistAuth(auth: CodexOauthAuth): Effect.Effect> { - // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` + private readStoredAuth(accountId: string): CodexOauthAuth | null { + // Read storage so another process cannot leave this service with stale credentials. + return getCodexOauthAuth(this.readOpenaiConfig(), accountId); + } + + private getAccountRevision(accountId: string): number { + return this.accountRevisions.get(accountId) ?? 0; + } + + private accountPath(accountId: string): string[] { + return accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID + ? ["codexOauth"] + : ["codexOauthAccounts", accountId]; + } + + private updateConfigValueEffect( + keyPath: string[], + update: (current: unknown) => { value: unknown } | null + ): Effect.Effect> { + return Effect.uninterruptible( + Effect.tryPromise({ + try: () => + this.providerService.updateConfigValue("openai", keyPath, update, { + enforcePolicy: true, + }), + catch: (error) => getErrorMessage(error), + }).pipe( + Effect.map((result): Result => { + if (!result.success) return result; + return result.data.applied + ? Ok(undefined) + : Err("Codex OAuth account changed or is not configured"); + }), + Effect.catch((error) => Effect.succeed(Err(error))) + ) + ); + } + + private selectLoginDestination( + options?: CodexOauthLoginOptions + ): Effect.Effect { + return Effect.suspend(() => { + if (options?.accountId !== undefined && options.label !== undefined) { + return Effect.fail( + new CodexOauthError({ reason: "Specify an account ID or a label, not both" }) + ); + } + if (options?.label !== undefined && !isValidLabel(options.label)) { + return Effect.fail(new CodexOauthError({ reason: "Invalid Codex OAuth account label" })); + } + const accountId = + options?.accountId ?? + (options?.label !== undefined ? crypto.randomUUID() : CODEX_OAUTH_DEFAULT_ACCOUNT_ID); + if (!isValidAccountId(accountId)) { + return Effect.fail(new CodexOauthError({ reason: "Invalid Codex OAuth account ID" })); + } + const auth = this.readStoredAuth(accountId); + if (options?.accountId !== undefined && !auth) { + return Effect.fail( + new CodexOauthError({ reason: "Codex OAuth account is not configured" }) + ); + } + // A failed or cancelled login must not discard a pending token rotation. + const revision = this.getAccountRevision(accountId); + const loginAttempt = (this.loginAttempts.get(accountId) ?? 0) + 1; + this.loginAttempts.set(accountId, loginAttempt); + return Effect.succeed({ + accountId, + revision, + loginAttempt, + auth, + label: options?.label?.trim(), + selectAsDefault: + options?.label !== undefined && + getCodexOauthAccounts(this.readOpenaiConfig()).length === 0, + }); + }); + } + + private persistAuth( + selection: AccountSelection, + auth: CodexOauthAuth | undefined, + isActive: () => boolean = () => true + ): Effect.Effect> { + return this.updateConfigValueEffect(this.accountPath(selection.accountId), (current) => { + const legacy = selection.accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID; + const stored = parseCodexOauthAuth( + legacy ? current : isPlainObject(current) ? current.auth : undefined + ); + // Compare under the file lock. Old refreshes must not restore deleted or reconnected slots. + if ( + !isActive() || + this.getAccountRevision(selection.accountId) !== selection.revision || + !matchesAuth(stored, selection.auth) + ) + return null; + if (legacy || auth === undefined) return { value: auth }; + return { + value: { + ...(isPlainObject(current) ? current : {}), + label: isPlainObject(current) ? current.label : selection.label, + auth, + }, + }; + }); + } + + private persistLoginAuth( + selection: AccountSelection, + auth: CodexOauthAuth, + isActive: () => boolean + ): Effect.Effect> { + // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect generators do not inherit this. const self = this; return Effect.gen(function* () { - // setConfigValue resolves with a wire Result; a rejection stays a - // defect, matching the previously un-caught await. - const result = yield* Effect.promise(() => - self.providerService.setConfigValue("openai", ["codexOauth"], auth) + const result = yield* self.persistAuth( + selection, + auth, + () => isActive() && self.loginAttempts.get(selection.accountId) === selection.loginAttempt ); - // Invalidate cache so the next readStoredAuth() picks up the persisted value from disk. - // We clear rather than set because setConfigValue may have side-effects (e.g. file-write - // failures) and we want the next read to be authoritative. - self.cachedAuth = null; - return result; + if (!result.success) return result; + self.accountRevisions.set( + selection.accountId, + self.getAccountRevision(selection.accountId) + 1 + ); + if (!selection.selectAsDefault) return result; + // Concurrent first logins share this default write. The first successful write wins. + const selected = yield* Effect.promise(() => + self.providerService.updateConfigValue( + "openai", + ["codexOauthDefaultAccountId"], + (current) => { + const accounts = getCodexOauthAccounts(self.readOpenaiConfig()); + return current === undefined && + accounts.some((account) => account.id === selection.accountId) && + !accounts.some((account) => account.id === CODEX_OAUTH_DEFAULT_ACCOUNT_ID) + ? { value: selection.accountId } + : null; + }, + { enforcePolicy: true } + ) + ); + return selected.success ? Ok(undefined) : selected; }); } private handleDesktopCallbackAndExchange(input: { + destination: AccountSelection; + isActive: () => boolean; flowId: string; redirectUri: string; codeVerifier: string; @@ -618,7 +840,7 @@ export class CodexOauthService { codeVerifier: input.codeVerifier, }); - const persistResult = yield* self.persistAuth(auth); + const persistResult = yield* self.persistLoginAuth(input.destination, auth, input.isActive); if (!persistResult.success) { return yield* Effect.fail(new CodexOauthError({ reason: persistResult.error })); } @@ -712,7 +934,10 @@ export class CodexOauthService { }); } - private refreshTokens(current: CodexOauthAuth): Effect.Effect { + private refreshTokens( + selection: AccountSelection, + current: CodexOauthAuth + ): Effect.Effect { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; return Effect.gen(function* () { @@ -724,6 +949,7 @@ export class CodexOauthService { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, body: buildCodexRefreshBody({ refreshToken: current.refresh }), + signal: AbortSignal.timeout(CODEX_OAUTH_REFRESH_TIMEOUT_MS), }), catch: (error) => new CodexOauthError({ @@ -738,7 +964,7 @@ export class CodexOauthService { // requests fall back to the existing "not connected" behavior. if (isInvalidGrantError(errorText)) { log.debug("[Codex OAuth] Refresh token rejected; clearing stored auth"); - const disconnectResult = yield* self.disconnectEffect(); + const disconnectResult = yield* self.persistAuth(selection, undefined); if (!disconnectResult.success) { log.warn( `[Codex OAuth] Failed to clear stored auth after refresh failure: ${disconnectResult.error}` @@ -782,7 +1008,9 @@ export class CodexOauthService { ); } - const accountId = extractAccountIdFromTokens({ accessToken, idToken }) ?? current.accountId; + // Refresh cannot change the ChatGPT identity for this local slot. + const accountId = + current.accountId ?? extractAccountIdFromTokens({ accessToken, idToken }) ?? undefined; const next: CodexOauthAuth = { type: "oauth", @@ -792,7 +1020,7 @@ export class CodexOauthService { accountId, }; - const persistResult = yield* self.persistAuth(next); + const persistResult = yield* self.persistAuth(selection, next); if (!persistResult.success) { return yield* Effect.fail(new CodexOauthError({ reason: persistResult.error })); } @@ -906,7 +1134,11 @@ export class CodexOauthService { const attempt = yield* self.pollDeviceTokenOnce(flow); if (attempt.kind === "success") { - const persistResult = yield* self.persistAuth(attempt.auth); + const persistResult = yield* self.persistLoginAuth( + flow.destination, + attempt.auth, + () => !flow.settled + ); if (!persistResult.success) { yield* self.finishDeviceFlowEffect(flowId, Err(persistResult.error)); return; diff --git a/src/node/services/projectService.test.ts b/src/node/services/projectService.test.ts index 167fe14dc6a..74d61598af7 100644 --- a/src/node/services/projectService.test.ts +++ b/src/node/services/projectService.test.ts @@ -2280,6 +2280,42 @@ exit 1 }); describe("project settings mutations", () => { + it("persists independent Codex accounts and clears only the selected override", async () => { + const firstPath = path.join(tempDir, "first"); + const secondPath = path.join(tempDir, "second"); + await config.editConfig((current) => { + current.projects.set(firstPath, { workspaces: [] }); + current.projects.set(secondPath, { workspaces: [] }); + return current; + }); + + const results = await Promise.all([ + service.setCodexOauthAccount(firstPath + "/", "personal"), + service.setCodexOauthAccount(secondPath, "work"), + service.setDisplayName(firstPath, "First project"), + ]); + expect(results[0]).toEqual(Ok(undefined)); + expect(results[1]).toEqual(Ok(undefined)); + const reloaded = new Config(tempDir).loadConfigOrDefault(); + expect(reloaded.projects.get(firstPath)).toMatchObject({ + codexOauthAccountId: "personal", + displayName: "First project", + }); + expect(reloaded.projects.get(secondPath)?.codexOauthAccountId).toBe("work"); + + expect(await service.setCodexOauthAccount(firstPath, null)).toEqual(Ok(undefined)); + const afterClear = config.loadConfigOrDefault(); + expect(afterClear.projects.get(firstPath)?.codexOauthAccountId).toBeUndefined(); + expect(afterClear.projects.get(secondPath)?.codexOauthAccountId).toBe("work"); + }); + + it("rejects a Codex account override for an unknown project", async () => { + const missing = path.join(tempDir, "missing"); + const result = await service.setCodexOauthAccount(missing, "work"); + expect(result.success).toBe(false); + expect(config.loadConfigOrDefault().projects.has(missing)).toBe(false); + }); + it("propagates parent trust to MCP state", async () => { const parentPath = path.join(tempDir, "project"); const childPath = path.join(parentPath, "packages", "api"); diff --git a/src/node/services/projectService.ts b/src/node/services/projectService.ts index 28a81f401ea..a6ed50ff527 100644 --- a/src/node/services/projectService.ts +++ b/src/node/services/projectService.ts @@ -1995,6 +1995,30 @@ export class ProjectService { }); } + /** Select a Codex account, or inherit the global default with null. */ + async setCodexOauthAccount(projectPath: string, accountId: string | null): Promise> { + const normalizedPath = stripTrailingSlashes(projectPath); + try { + let result: Result = Err(`Project not found: ${normalizedPath}`); + // Edit fresh state so concurrent project settings remain intact. + await this.config.editConfig((config) => { + const project = config.projects.get(normalizedPath); + if (project) { + if (accountId === null) { + delete project.codexOauthAccountId; + } else { + project.codexOauthAccountId = accountId; + } + result = Ok(undefined); + } + return config; + }); + return result; + } catch (error) { + return Err(`Failed to set Codex account for ${normalizedPath}: ${getErrorMessage(error)}`); + } + } + async setCustomInstructions( projectPath: string, customInstructions: string | null | undefined diff --git a/src/node/services/providerModelFactory.test.ts b/src/node/services/providerModelFactory.test.ts index 9eb498cb9c6..d87e8cd1003 100644 --- a/src/node/services/providerModelFactory.test.ts +++ b/src/node/services/providerModelFactory.test.ts @@ -1062,6 +1062,241 @@ describe("ProviderModelFactory GitHub Copilot", () => { }); }); + it("pins project Codex accounts across requests and rejects deleted slots", async () => { + await withTempConfig(async (config, factory, oauth, providersConfigStore) => { + const personal = { + type: "oauth" as const, + access: "personal-access", + refresh: "personal-refresh", + expires: Date.now() + 3_600_000, + accountId: "chatgpt-personal", + }; + const work = { ...personal, access: "work-access", accountId: "chatgpt-work" }; + providersConfigStore.saveProvidersConfig({ + openai: { + apiKey: "api-key-must-not-win", + codexOauth: personal, + codexOauthAccounts: { work: { label: "Work", auth: work } }, + }, + }); + await config.editConfig((current) => { + current.projects.set("/personal", { + codexOauthAccountId: "default", + workspaces: [{ id: "personal-ws", name: "personal", path: "/personal/ws" }], + }); + current.projects.set("/work", { + codexOauthAccountId: "work", + workspaces: [{ id: "work-ws", name: "work", path: "/work/ws" }], + }); + current.projects.set("/personal/sub", { + parentProjectPath: "/personal", + codexOauthAccountId: "work", + workspaces: [], + }); + current.projects.get("/personal")!.workspaces.push({ + id: "sub-ws", + name: "sub", + path: "/personal/sub-ws", + subProjectPath: "/personal/sub", + }); + current.projects.set("_multi", { + workspaces: [ + { + id: "multi-ws", + name: "multi", + path: "/multi/ws", + projects: [ + { projectPath: "/work", projectName: "work" }, + { projectPath: "/personal", projectName: "personal" }, + ], + }, + ], + }); + return current; + }); + oauth.codexOauthService = new CodexOauthService( + providersConfigStore, + new ProviderService(config, undefined, providersConfigStore) + ); + const originalRegistry = PROVIDER_REGISTRY.openai; + const requests: Headers[] = []; + let capturedFetch: typeof fetch | undefined; + PROVIDER_REGISTRY.openai = async () => { + const module = await originalRegistry(); + return { + ...module, + createOpenAI: (options) => { + capturedFetch = options?.fetch; + return module.createOpenAI(options); + }, + }; + }; + const createFetch = async (context: { workspaceId?: string; projectPath?: string } = {}) => { + const stored = providersConfigStore.loadProvidersConfig() ?? {}; + const result = await factory.createModel("openai:gpt-5.3-codex", undefined, { + ...context, + providersConfig: { + ...stored, + openai: { + ...stored.openai, + fetch: (_input: RequestInfo | URL, init?: RequestInit) => { + requests.push(new Headers(init?.headers)); + return Promise.resolve( + new Response("{}", { headers: { "content-type": "application/json" } }) + ); + }, + }, + }, + }); + expect(result.success).toBe(true); + if (!result.success || !capturedFetch) throw new Error("Expected an OAuth model"); + expect(modelCostsIncluded(result.data)).toBe(true); + return capturedFetch; + }; + const send = (providerFetch: typeof fetch) => + providerFetch("https://api.openai.com/v1/responses", { + method: "POST", + headers: { "ChatGPT-Account-Id": "stale-configured-account" }, + body: JSON.stringify({ model: "gpt-5.3-codex", input: [] }), + }); + try { + const personalFetch = await createFetch({ workspaceId: "personal-ws" }); + const workFetch = await createFetch({ workspaceId: "work-ws" }); + await send(personalFetch); + await send(workFetch); + expect(requests.map((headers) => headers.get("authorization"))).toEqual([ + "Bearer personal-access", + "Bearer work-access", + ]); + expect(requests.map((headers) => headers.get("chatgpt-account-id"))).toEqual([ + "chatgpt-personal", + "chatgpt-work", + ]); + for (const context of [ + { workspaceId: "sub-ws" }, + { workspaceId: "multi-ws" }, + { projectPath: "/work" }, + ]) { + await send(await createFetch(context)); + expect(requests.at(-1)?.get("authorization")).toBe("Bearer work-access"); + } + // Legacy requests use the default slot when no selection exists. + await send(await createFetch()); + expect(requests.at(-1)?.get("authorization")).toBe("Bearer personal-access"); + + const updatedWork = { ...work, access: "work-refreshed", accountId: undefined }; + providersConfigStore.saveProvidersConfig({ + openai: { + codexOauth: personal, + codexOauthDefaultAccountId: "work", + codexOauthAccounts: { work: { label: "Work", auth: updatedWork } }, + }, + }); + await config.editConfig((current) => { + current.projects.get("/work")!.codexOauthAccountId = "default"; + return current; + }); + await send(workFetch); + expect(requests.at(-1)?.get("authorization")).toBe("Bearer work-refreshed"); + expect(requests.at(-1)?.has("chatgpt-account-id")).toBe(false); + await send(personalFetch); + expect(requests.at(-1)?.get("authorization")).toBe("Bearer personal-access"); + await send(await createFetch()); + expect(requests.at(-1)?.get("authorization")).toBe("Bearer work-refreshed"); + await config.editConfig((current) => { + delete current.projects.get("/personal/sub")!.codexOauthAccountId; + return current; + }); + await send(await createFetch({ workspaceId: "sub-ws" })); + expect(requests.at(-1)?.get("authorization")).toBe("Bearer work-refreshed"); + const resolved = await factory.resolveAndCreateModel( + "openai:gpt-5.3-codex", + "off", + undefined, + { + workspaceId: "personal-ws", + } + ); + expect(resolved.success).toBe(true); + if (resolved.success) expect(resolved.data.codexOauthAccountId).toBe("default"); + + providersConfigStore.saveProvidersConfig({ + openai: { + apiKey: "api-key-must-not-win", + codexOauth: personal, + codexOauthDefaultAccountId: "work", + }, + }); + const sentCount = requests.length; + const rejected = await send(workFetch).then( + () => false, + () => true + ); + expect(rejected).toBe(true); + expect(requests).toHaveLength(sentCount); + expect(await factory.createModel("openai:gpt-5.3-codex")).toMatchObject({ + success: false, + error: { type: "oauth_not_connected" }, + }); + } finally { + PROVIDER_REGISTRY.openai = originalRegistry; + } + }); + }); + + it("rejects explicit missing Codex selections without changing API-key precedence", async () => { + await withTempConfig(async (config, factory, _oauth, providersConfigStore) => { + await config.editConfig((current) => { + current.projects.set("/missing", { codexOauthAccountId: "deleted", workspaces: [] }); + return current; + }); + for (const selection of [undefined, { projectPath: "/missing" }]) { + providersConfigStore.saveProvidersConfig({ + openai: { + apiKey: "test-key", + codexOauthDefaultAuth: "oauth", + ...(selection ? {} : { codexOauthDefaultAccountId: "deleted" }), + }, + }); + expect( + await factory.createModel("openai:gpt-5.3-codex", undefined, selection) + ).toMatchObject({ + success: false, + error: { type: "oauth_not_connected" }, + }); + const chat = await factory.createModel( + "openai:gpt-5.3-codex", + { + openai: { wireFormat: "chatCompletions" }, + }, + selection + ); + expect(chat.success).toBe(true); + if (chat.success) expect(modelCostsIncluded(chat.data)).toBe(false); + } + providersConfigStore.saveProvidersConfig({ + openai: { apiKey: "test-key", codexOauthDefaultAuth: "apiKey" }, + }); + const api = await factory.createModel("openai:gpt-5.5", undefined, { + projectPath: "/missing", + }); + expect(api.success).toBe(true); + if (api.success) expect(modelCostsIncluded(api.data)).toBe(false); + providersConfigStore.saveProvidersConfig({ + openai: { codexOauthDefaultAccountId: "deleted" }, + openrouter: { apiKey: "gateway-key" }, + }); + await saveRoutePriority(config, ["openrouter", "direct"]); + const gateway = await factory.resolveAndCreateModel("openai:gpt-5.5", "off", undefined, { + projectPath: "/missing", + }); + expectSuccessfulRouteResult(gateway, { + effectiveModelString: "openrouter:openai/gpt-5.5", + routeProvider: "openrouter", + }); + }); + }); + it("normalizes Request bodies for the Codex OAuth responses endpoint", async () => { await withTempConfig(async (_config, factory, oauth, providersConfigStore) => { const originalOpenAIRegistry = PROVIDER_REGISTRY.openai; @@ -1900,45 +2135,65 @@ describe("ProviderModelFactory routing", () => { }); }); - it("treats OpenAI as available for routing when only Codex OAuth is configured", async () => { - // Temporarily remove OPENAI_API_KEY so the test only succeeds via Codex OAuth, - // not by falling through to an env-var credential path. - const savedKey = process.env.OPENAI_API_KEY; - delete process.env.OPENAI_API_KEY; - try { - await withTempConfig(async (config, factory) => { - new ProvidersConfigStore(config.rootDir).saveProvidersConfig({ - openai: { - // No apiKey — only Codex OAuth credentials. - codexOauth: { - type: "oauth", - access: "test-access-token", - refresh: "test-refresh-token", - expires: Date.now() + 60_000, + it.each(["default", "work"])( + "treats OpenAI as available with only the %s OAuth slot", + async (accountId) => { + // Temporarily remove OPENAI_API_KEY so the test only succeeds via Codex OAuth, + // not by falling through to an env-var credential path. + const savedKey = process.env.OPENAI_API_KEY; + delete process.env.OPENAI_API_KEY; + try { + await withTempConfig(async (config, factory) => { + new ProvidersConfigStore(config.rootDir).saveProvidersConfig({ + openai: { + // No apiKey — only Codex OAuth credentials. + ...(accountId === "default" + ? { + codexOauth: { + type: "oauth" as const, + access: "access", + refresh: "refresh", + expires: Date.now() + 60_000, + }, + } + : { + codexOauthDefaultAccountId: accountId, + codexOauthAccounts: { + [accountId]: { + label: "Work", + auth: { + type: "oauth" as const, + access: "access", + refresh: "refresh", + expires: Date.now() + 60_000, + }, + }, + }, + }), }, - }, - openrouter: { - apiKey: "or-test", - }, - }); + openrouter: { + apiKey: "or-test", + }, + }); - await saveRoutePriority(config, ["direct", "openrouter"]); + await saveRoutePriority(config, ["direct", "openrouter"]); - // Direct OpenAI should win because Codex OAuth makes it available for routing. - // Use a model from CODEX_OAUTH_ALLOWED_MODELS so createModel can route through OAuth. - const result = await factory.resolveAndCreateModel("openai:gpt-5.2", "off"); - expectSuccessfulRouteResult(result, { - effectiveModelString: "openai:gpt-5.2", - routeProvider: "openai", - routedThroughGateway: false, + // Direct OpenAI should win because Codex OAuth makes it available for routing. + // Use a model from CODEX_OAUTH_ALLOWED_MODELS so createModel can route through OAuth. + const result = await factory.resolveAndCreateModel("openai:gpt-5.2", "off"); + expectSuccessfulRouteResult(result, { + effectiveModelString: "openai:gpt-5.2", + routeProvider: "openai", + routedThroughGateway: false, + }); }); - }); - } finally { - if (savedKey !== undefined) { - process.env.OPENAI_API_KEY = savedKey; + } finally { + if (savedKey !== undefined) { + process.env.OPENAI_API_KEY = savedKey; + } } } - }); + ); it("leaves direct-provider model strings unchanged when direct routing wins", async () => { await withTempConfig(async (config, factory) => { diff --git a/src/node/services/providerModelFactory.ts b/src/node/services/providerModelFactory.ts index b9e5fb643b9..83385344c7f 100644 --- a/src/node/services/providerModelFactory.ts +++ b/src/node/services/providerModelFactory.ts @@ -19,7 +19,11 @@ import { isCodexOauthAllowedModel, isCodexOauthRequiredModel, } from "@/common/constants/codexOAuth"; -import { parseCodexOauthAuth } from "@/node/utils/codexOauthAuth"; +import { + getCodexOauthAccountId, + getCodexOauthAccounts, + getCodexOauthAuth, +} from "@/node/utils/codexOauthAuth"; import type { Config, ProviderConfig, ProvidersConfig } from "@/node/config"; import { ProvidersConfigStore } from "@/node/config"; import type { MuxProviderOptions } from "@/common/types/providerOptions"; @@ -1099,6 +1103,8 @@ export interface ResolveAndCreateModelResult { * retagged instance type diverging from the created fallback model. */ coderSelectedInstance?: { name: string; type: string }; + /** Local account slot pinned for request routing and context limits. */ + codexOauthAccountId?: string; /** Whether the request is being routed through the Xum gateway. */ routedThroughGateway: boolean; /** Route provider chosen by backend routing (direct provider or gateway). */ @@ -1108,6 +1114,10 @@ export interface ResolveAndCreateModelResult { interface CreateModelOptions { agentInitiated?: boolean; workspaceId?: string; + /** Project context for requests before workspace creation. */ + projectPath?: string; + /** Account selection snapshot from resolveAndCreateModel. */ + codexOauthSelection?: { accountId: string; explicit: boolean }; routeContext?: RouteContext; /** * Providers-config snapshot to create the model from. Passed by @@ -1192,6 +1202,31 @@ export class ProviderModelFactory { return forced ? { ...providerConfig, deploymentUrl: forced } : providerConfig; } + private resolveCodexOauthSelection( + providerConfig: ProviderConfigRaw, + opts?: CreateModelOptions + ): { accountId: string; explicit: boolean } { + if (opts?.codexOauthSelection) { + return opts.codexOauthSelection; + } + const workspace = opts?.workspaceId ? this.config.findWorkspace(opts.workspaceId) : null; + // Multi-project attribution wins. Single-project workspaces use their registered subproject. + const projectPath = + workspace?.projects?.[0]?.projectPath ?? + workspace?.subProjectPath ?? + workspace?.attributionProjectPath ?? + workspace?.projectPath ?? + opts?.projectPath; + const projectAccountId = projectPath + ? this.config.loadConfigOrDefault().projects.get(projectPath)?.codexOauthAccountId + : undefined; + return { + accountId: getCodexOauthAccountId(providerConfig, projectAccountId), + explicit: + projectAccountId !== undefined || providerConfig.codexOauthDefaultAccountId !== undefined, + }; + } + private isProviderAvailableForRouting( provider: ProviderName, providersConfig: ProvidersConfig, @@ -1206,9 +1241,7 @@ export class ProviderModelFactory { // OpenAI Codex OAuth is a valid credential path even without an API key; // routing should treat it as available so direct OpenAI routes are honored. - const hasCodexOauth = - provider === "openai" && - parseCodexOauthAuth((providerConfig as { codexOauth?: unknown }).codexOauth) !== null; + const hasCodexOauth = provider === "openai" && getCodexOauthAccounts(providerConfig).length > 0; if (!credentials.isConfigured && !hasCodexOauth) { return false; @@ -1284,11 +1317,7 @@ export class ProviderModelFactory { private createModelCoreEffect( modelString: string, muxProviderOptions?: MuxProviderOptions, - opts?: { - agentInitiated?: boolean; - routeContext?: RouteContext; - providersConfig?: ProvidersConfig; - } + opts?: CreateModelOptions ): Effect.Effect> { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; @@ -1568,17 +1597,16 @@ export class ProviderModelFactory { const codexOauthAllowed = isCodexOauthAllowedModel(fullModelId, providersConfig); const codexOauthRequired = isCodexOauthRequiredModel(fullModelId, providersConfig); - const storedCodexOauth = parseCodexOauthAuth( - (providerConfig as { codexOauth?: unknown }).codexOauth - ); + // Pin the local slot, not the ChatGPT header ID, across refreshes and retries. + const selection = self.resolveCodexOauthSelection(providerConfig, opts); + const codexOauthAccountId = selection.accountId; + const storedCodexOauth = getCodexOauthAuth(providerConfig, codexOauthAccountId); // Resolve credentials from config + env so we can decide whether to // route through Codex OAuth or fall back to API key auth. const creds = resolveProviderCredentials("openai", providerConfig); - // When a model requires Codex OAuth but the user hasn't connected it, - // fall back to their API key instead of blocking entirely. If the model - // truly only works through OAuth, OpenAI's API will return a clear error. + // Preserve the legacy API-key path when no OAuth account has been selected. if (codexOauthRequired && !storedCodexOauth && !creds.isConfigured) { return Err({ type: "oauth_not_connected", provider: providerName }); } @@ -1594,32 +1622,20 @@ export class ProviderModelFactory { (providerConfig.wireFormat as string | undefined) ?? muxProviderOptions?.openai?.wireFormat; - // Codex OAuth routing: - // - Chat Completions never routes through OAuth when an API key exists. - // - Required models route through ChatGPT OAuth when connected. - // - If OAuth is not connected, fall back to API key (if available). - // - Allowed models route through OAuth only when: - // - no API key is configured, OR - // - the user prefers OAuth when both are set. - const shouldRouteThroughCodexOauth = (() => { - if (!codexOauthAllowed || !storedCodexOauth) { - return false; - } - - if (earlyWireFormat === "chatCompletions" && creds.isConfigured) { - return false; - } - - if (codexOauthRequired) { - return true; - } - - if (!creds.isConfigured) { - return true; - } + // A missing selected slot must not switch billing to an API key or another account. + const prefersCodexOauth = + codexOauthAllowed && + !(earlyWireFormat === "chatCompletions" && creds.isConfigured) && + (codexOauthRequired || !creds.isConfigured || codexOauthDefaultAuth === "oauth"); + if ( + prefersCodexOauth && + !storedCodexOauth && + (selection.explicit || getCodexOauthAccounts(providerConfig).length > 0) + ) { + return Err({ type: "oauth_not_connected", provider: providerName }); + } - return codexOauthDefaultAuth === "oauth"; - })(); + const shouldRouteThroughCodexOauth = prefersCodexOauth && storedCodexOauth !== null; // OAuth requests use a placeholder key and override auth headers in fetch(). const resolvedApiKey = shouldRouteThroughCodexOauth ? undefined : creds.apiKey; @@ -1734,7 +1750,7 @@ export class ProviderModelFactory { throw new Error("Codex OAuth service not initialized"); } - const authResult = await codexOauthService.getValidAuth(); + const authResult = await codexOauthService.getValidAuth(codexOauthAccountId); if (!authResult.success) { throw new Error(authResult.error); } @@ -1743,6 +1759,9 @@ export class ProviderModelFactory { headers.set("Authorization", `Bearer ${authResult.data.access}`); if (authResult.data.accountId) { headers.set("ChatGPT-Account-Id", authResult.data.accountId); + } else { + // A configured header must not select another ChatGPT account. + headers.delete("ChatGPT-Account-Id"); } nextInput = CODEX_ENDPOINT; @@ -2544,7 +2563,7 @@ export class ProviderModelFactory { modelString: string, thinkingLevel: ThinkingLevel, muxProviderOptions?: MuxProviderOptions, - opts?: { agentInitiated?: boolean; workspaceId?: string } + opts?: Pick ): Promise> { return Effect.runPromise( this.resolveAndCreateModelEffect(modelString, thinkingLevel, muxProviderOptions, opts) @@ -2555,7 +2574,7 @@ export class ProviderModelFactory { modelString: string, thinkingLevel: ThinkingLevel, muxProviderOptions?: MuxProviderOptions, - opts?: { agentInitiated?: boolean; workspaceId?: string } + opts?: Pick ): Effect.Effect> { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; @@ -2790,8 +2809,13 @@ export class ProviderModelFactory { } } + const codexOauthSelection = self.resolveCodexOauthSelection( + providersConfigForShadowCheck.openai ?? {}, + opts + ); const modelResult = yield* self.createModelEffect(effectiveModelString, muxProviderOptions, { ...opts, + codexOauthSelection, routeContext, // ONE config snapshot for the whole resolve+create: the wire snapshot // above and the SDK model must come from the same providers.jsonc read, @@ -2835,6 +2859,7 @@ export class ProviderModelFactory { wireProviderName, coderWire, coderSelectedInstance, + codexOauthAccountId: codexOauthSelection.accountId, routedThroughGateway, // Custom adapters are direct routes: the raw custom id is not a // ProviderName, and leaking it as routeProvider makes downstream diff --git a/src/node/services/providerService.test.ts b/src/node/services/providerService.test.ts index 60a89ace63e..f2f2c37c133 100644 --- a/src/node/services/providerService.test.ts +++ b/src/node/services/providerService.test.ts @@ -314,6 +314,58 @@ describe("ProviderService.getConfig", () => { }); }); + it("exposes account labels without exposing credentials", () => { + withTempConfig((config, service) => { + const auth = { + type: "oauth", + access: "secret-access", + refresh: "secret-refresh", + expires: 12345, + }; + new ProvidersConfigStore(config.rootDir).saveProvidersConfig({ + openai: { + codexOauth: auth, + codexOauthLabel: "Personal", + codexOauthAccounts: { + work: { label: "Work", auth }, + broken: { label: "Broken", auth: {} }, + }, + codexOauthDefaultAccountId: "work", + }, + }); + const result = service.getConfig().openai; + expect(result.codexOauthSet).toBe(true); + expect(result.codexOauthAccounts).toEqual([ + { id: "default", label: "Personal" }, + { id: "work", label: "Work" }, + ]); + expect(result.codexOauthDefaultAccountId).toBe("work"); + expect(JSON.stringify(result)).not.toContain("secret-access"); + expect(JSON.stringify(result)).not.toContain("secret-refresh"); + }); + }); + + it("reports named accounts as connected without legacy credentials", () => { + withTempConfig((config, service) => { + new ProvidersConfigStore(config.rootDir).saveProvidersConfig({ + openai: { + codexOauthAccounts: { + work: { + label: "Work", + auth: { type: "oauth", access: "access", refresh: "refresh", expires: 12345 }, + }, + }, + codexOauthDefaultAccountId: "work", + }, + }); + expect(service.getConfig().openai).toMatchObject({ + codexOauthSet: true, + isConfigured: true, + codexOauthAccounts: [{ id: "work", label: "Work" }], + }); + }); + }); + it("treats disabled OpenAI as unconfigured even when Codex OAuth tokens are stored", () => { withTempConfig((config, service) => { new ProvidersConfigStore(config.rootDir).saveProvidersConfig({ @@ -621,6 +673,31 @@ describe("ProviderService.getConfig", () => { ); }); + it("checks policy for account edits but retains internal credential cleanup", async () => { + await withTempPolicyProviderService( + { policy_format_version: "0.1", provider_access: [{ id: "anthropic" }] }, + async (config, service) => { + const store = new ProvidersConfigStore(config.rootDir); + store.saveProvidersConfig({ openai: { codexOauthDefaultAccountId: "personal" } }); + const denied = await service.updateConfigValue( + "openai", + ["codexOauthDefaultAccountId"], + () => ({ value: "work" }), + { enforcePolicy: true } + ); + expect(denied.success).toBe(false); + expect(store.loadProvidersConfig()?.openai?.codexOauthDefaultAccountId).toBe("personal"); + const cleaned = await service.updateConfigValue( + "openai", + ["codexOauthDefaultAccountId"], + () => ({ value: undefined }) + ); + expect(cleaned).toEqual({ success: true, data: { applied: true } }); + expect(store.loadProvidersConfig()?.openai?.codexOauthDefaultAccountId).toBeUndefined(); + } + ); + }); + it("revalidates policy inside the providers file lock in setModels", async () => { // Regression: policy can refresh while another process holds the // cross-process providers lock. A check done only before the lock wait diff --git a/src/node/services/providerService.ts b/src/node/services/providerService.ts index 6c8088d8e97..1619ca95b00 100644 --- a/src/node/services/providerService.ts +++ b/src/node/services/providerService.ts @@ -63,7 +63,7 @@ import { isProviderAutoRouteEligible, resolveProviderCredentials, } from "@/node/utils/providerRequirements"; -import { parseCodexOauthAuth } from "@/node/utils/codexOauthAuth"; +import { getCodexOauthAccounts, getCodexOauthAccountId } from "@/node/utils/codexOauthAuth"; import { normalizeCoderDeploymentUrl, parseCoderGatewayProviders, @@ -425,8 +425,8 @@ export class ProviderService { config.models === undefined ? undefined : normalizeProviderModelEntries(config.models); const filteredModels = filterProviderModelsByPolicy(normalizedModels, allowedModels); - const codexOauthSet = - provider === "openai" && parseCodexOauthAuth(config.codexOauth) !== null; + const codexOauthAccounts = provider === "openai" ? getCodexOauthAccounts(config) : []; + const codexOauthSet = codexOauthAccounts.length > 0; let isEnabled = !isProviderDisabledInConfig(config); if (provider === "mux-gateway" && mainConfig.muxGatewayEnabled === false) { isEnabled = false; @@ -506,6 +506,11 @@ export class ProviderService { if (provider === "openai") { providerInfo.codexOauthSet = codexOauthSet; + providerInfo.codexOauthAccounts = codexOauthAccounts.map(({ id, label }) => ({ + id, + label, + })); + providerInfo.codexOauthDefaultAccountId = getCodexOauthAccountId(config); const codexOauthDefaultAuth = config.codexOauthDefaultAuth; if (codexOauthDefaultAuth === "oauth" || codexOauthDefaultAuth === "apiKey") { @@ -1447,22 +1452,23 @@ export class ProviderService { * logins/refreshes (e.g. Coder OAuth token rotation across the desktop app * and `mux run`/`mux workflow`). * - * Unlike setConfigValue, this path skips policy gating: it is an internal - * credential-management primitive (clearing dead tokens, persisting - * rotations), not a user-driven config edit. + * Internal credential updates skip policy gating by default. + * User-driven edits must set enforcePolicy to check policy under the file lock. */ public updateConfigValue( provider: string, keyPath: string[], - update: (current: unknown) => { value: unknown } | null + update: (current: unknown) => { value: unknown } | null, + options?: { enforcePolicy?: boolean } ): Promise> { - return Effect.runPromise(this.updateConfigValueEffect(provider, keyPath, update)); + return Effect.runPromise(this.updateConfigValueEffect(provider, keyPath, update, options)); } private updateConfigValueEffect( provider: string, keyPath: string[], - update: (current: unknown) => { value: unknown } | null + update: (current: unknown) => { value: unknown } | null, + options?: { enforcePolicy?: boolean } ): Effect.Effect> { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; @@ -1479,6 +1485,10 @@ export class ProviderService { } const applied = yield* self.providersFileLockEffect(() => { + if (options?.enforcePolicy) { + const denial = self.validateProviderEditPolicy(provider, keyPath); + if (denial != null) return denial; + } // Load, decide, and write under the lock — no awaits in between, so // the predicate result cannot be invalidated by any cooperating writer. const providersConfig = self.providersConfigStore.loadProvidersConfig() ?? {}; @@ -1517,6 +1527,7 @@ export class ProviderService { return true; }); + if (typeof applied === "string") return { success: false as const, error: applied }; yield* self.afterAppliedMutationEffect(provider, applied); return { success: true as const, data: { applied } }; }).pipe( diff --git a/src/node/services/tools/shared/configRedaction.test.ts b/src/node/services/tools/shared/configRedaction.test.ts new file mode 100644 index 00000000000..950a1460e9d --- /dev/null +++ b/src/node/services/tools/shared/configRedaction.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "bun:test"; +import { redactConfigDocument, REDACTED_SECRET_VALUE } from "./configRedaction"; + +describe("Codex account redaction", () => { + it("removes all account credentials without changing the source document", () => { + const auth = { + type: "oauth", + access: "private-access", + refresh: "private-refresh", + expires: 12345, + }; + const document = { + openai: { + codexOauth: auth, + codexOauthAccounts: { work: { label: "Work", auth } }, + codexOauthDefaultAccountId: "work", + }, + }; + + const redacted = redactConfigDocument("providers", document); + expect(redacted).toEqual({ + openai: { + codexOauth: REDACTED_SECRET_VALUE, + codexOauthAccounts: REDACTED_SECRET_VALUE, + codexOauthDefaultAccountId: "work", + }, + }); + expect(JSON.stringify(redacted)).not.toContain(auth.access); + expect(JSON.stringify(redacted)).not.toContain(auth.refresh); + expect(document.openai.codexOauthAccounts.work.auth).toEqual(auth); + }); +}); diff --git a/src/node/services/tools/shared/configRedaction.ts b/src/node/services/tools/shared/configRedaction.ts index 9367bb16708..adc2b170579 100644 --- a/src/node/services/tools/shared/configRedaction.ts +++ b/src/node/services/tools/shared/configRedaction.ts @@ -13,6 +13,7 @@ const PROVIDER_SECRET_KEYS = new Set([ "couponCode", "voucher", "codexOauth", + "codexOauthAccounts", ]); const APP_SECRET_KEYS = new Set(["muxGovernorToken"]); diff --git a/src/node/services/turnRequestBuilder.ts b/src/node/services/turnRequestBuilder.ts index 51c292eb2e3..d240ac92827 100644 --- a/src/node/services/turnRequestBuilder.ts +++ b/src/node/services/turnRequestBuilder.ts @@ -912,11 +912,21 @@ export class TurnRequestBuilder { return resolved; } - const providersConfig = pinCoderInstanceProvidersConfig( + let providersConfig = pinCoderInstanceProvidersConfig( this.dependencies.providerService.getConfig(), options.rawModelString, resolved.data.coderSelectedInstance ); + // Context-limit mirrors must use the account that the model selected. + if (providersConfig.openai && resolved.data.codexOauthAccountId != null) { + providersConfig = { + ...providersConfig, + openai: { + ...providersConfig.openai, + codexOauthDefaultAccountId: resolved.data.codexOauthAccountId, + }, + }; + } const minThinkingLevel = resolveMinimumThinkingLevel( options.rawModelString, options.minimumThinkingLevelOverride, diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index 0a4defe9bc2..59ee63a77a6 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -10138,6 +10138,7 @@ describe("WorkspaceService pending auto-title", () => { expect(metadata?.title).toBe("Harden auth flow"); expect(metadata?.pendingAutoTitle).toBeUndefined(); expect(generateIdentitySpy.mock.calls[0]?.[0]).toBe("Continue with auth hardening"); + expect(generateIdentitySpy.mock.calls[0]?.[5]).toEqual({ workspaceId }); } finally { generateIdentitySpy.mockRestore(); } @@ -17458,6 +17459,7 @@ describe("WorkspaceService regenerateTitle", () => { const call = generateIdentitySpy.mock.calls[0]; expect(call?.[3]).toBeUndefined(); expect(call?.[4]).toBe("Fix CI"); + expect(call?.[5]).toEqual({ workspaceId }); expect(updateTitleSpy).toHaveBeenCalledWith(workspaceId, "Fix CI"); } finally { updateTitleSpy.mockRestore(); diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 819ee590112..361baf5cc86 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -7346,7 +7346,14 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { try { const candidates = await this.getWorkspaceTitleModelCandidates(workspaceId); - const result = await generateWorkspaceIdentity(trimmedMessage, candidates, this.aiService); + const result = await generateWorkspaceIdentity( + trimmedMessage, + candidates, + this.aiService, + undefined, + undefined, + { workspaceId } + ); if (result.success) { const persistResult = await this.updateWorkspaceTitleState(workspaceId, { title: result.data.title, @@ -7711,7 +7718,8 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { candidates, this.aiService, conversationContext, - latestUserText + latestUserText, + { workspaceId } ); if (!result.success) { return Err("Title generation failed"); diff --git a/src/node/services/workspaceStatusGenerator.test.ts b/src/node/services/workspaceStatusGenerator.test.ts index 4ff982f307e..8a60d5f622e 100644 --- a/src/node/services/workspaceStatusGenerator.test.ts +++ b/src/node/services/workspaceStatusGenerator.test.ts @@ -1,4 +1,6 @@ -import { describe, expect, test } from "bun:test"; +import { describe, expect, mock, test } from "bun:test"; +import { Err } from "@/common/types/result"; +import type { AIService } from "./aiService"; import { buildWorkspaceStatusPrompt, generateWorkspaceStatus } from "./workspaceStatusGenerator"; describe("buildWorkspaceStatusPrompt", () => { @@ -56,6 +58,24 @@ describe("buildWorkspaceStatusPrompt", () => { }); describe("generateWorkspaceStatus error paths", () => { + test("preserves workspace account context across candidate failures", async () => { + const createModelWithPinnedMetadata = mock(() => + Promise.resolve(Err({ type: "oauth_not_connected", provider: "openai" })) + ); + const aiService = { createModelWithPinnedMetadata } as unknown as AIService; + const result = await generateWorkspaceStatus( + "Run tests", + ["openai:gpt-5.5", "openai:gpt-5.3-codex"], + aiService, + { workspaceId: "status-workspace" } + ); + expect(result.success).toBe(false); + expect(createModelWithPinnedMetadata).toHaveBeenCalledTimes(2); + for (const call of createModelWithPinnedMetadata.mock.calls) { + expect(call[1]).toEqual({ workspaceId: "status-workspace", agentInitiated: true }); + } + }); + test("returns a configuration error when no candidates are provided", async () => { const fakeAiService = { // Asserting this never gets called is the real point of this test — diff --git a/src/node/services/workspaceStatusGenerator.ts b/src/node/services/workspaceStatusGenerator.ts index 2d51b32bd07..4ae110c8213 100644 --- a/src/node/services/workspaceStatusGenerator.ts +++ b/src/node/services/workspaceStatusGenerator.ts @@ -121,6 +121,7 @@ export function buildWorkspaceStatusPrompt( } export interface GenerateWorkspaceStatusOptions extends BuildWorkspaceStatusPromptOptions { + workspaceId?: string; /** * Best-effort cost telemetry: status generation bypasses StreamManager, * so the caller records the successful candidate's usage into @@ -208,6 +209,7 @@ function generateWorkspaceStatusEffect( const modelResult = yield* Effect.promise(async () => aiService.createModelWithPinnedMetadata(modelString, { agentInitiated: true, + workspaceId: options.workspaceId, }) ); if (!modelResult.success) { diff --git a/src/node/services/workspaceTitleGenerator.test.ts b/src/node/services/workspaceTitleGenerator.test.ts index 089a6b08b2f..6afeb90093c 100644 --- a/src/node/services/workspaceTitleGenerator.test.ts +++ b/src/node/services/workspaceTitleGenerator.test.ts @@ -7,7 +7,7 @@ import { mapModelCreationError, mapNameGenerationError, } from "./workspaceTitleGenerator"; -import { Ok } from "@/common/types/result"; +import { Err, Ok } from "@/common/types/result"; import type { AIService } from "./aiService"; import { attachLanguageModelCleanup } from "./languageModelCleanup"; @@ -83,6 +83,29 @@ describe("generateWorkspaceIdentity cleanup", () => { return { createModel: () => Promise.resolve(Ok(model)) } as unknown as AIService; } + test.each([{ workspaceId: "title-workspace" }, { projectPath: "/new-project" }])( + "preserves title account context across candidate failures: %j", + async (context) => { + const createModel = mock(() => + Promise.resolve(Err({ type: "oauth_not_connected", provider: "openai" })) + ); + const aiService = { createModel } as unknown as AIService; + const result = await generateWorkspaceIdentity( + "Add setting", + ["openai:gpt-5.5", "openai:gpt-5.3-codex"], + aiService, + undefined, + undefined, + context + ); + expect(result.success).toBe(false); + expect(createModel).toHaveBeenCalledTimes(2); + for (const call of createModel.mock.calls) { + expect(call[2]).toEqual({ ...context, agentInitiated: true }); + } + } + ); + test("cleans up the model after a successful title stream", async () => { let cleanupCalls = 0; const model = createTitleModel(); diff --git a/src/node/services/workspaceTitleGenerator.ts b/src/node/services/workspaceTitleGenerator.ts index 0cea63887d3..3096f7b3588 100644 --- a/src/node/services/workspaceTitleGenerator.ts +++ b/src/node/services/workspaceTitleGenerator.ts @@ -185,7 +185,8 @@ export async function generateWorkspaceIdentity( /** Optional conversation turns context used for regenerate-title prompts. */ conversationContext?: string, /** Optional most recent user message; included as additional context only — not given precedence over older turns. */ - latestUserMessage?: string + latestUserMessage?: string, + context?: { workspaceId?: string; projectPath?: string } ): Promise> { if (candidates.length === 0) { return Err({ type: "unknown", raw: "No model candidates provided for name generation" }); @@ -201,6 +202,7 @@ export async function generateWorkspaceIdentity( const modelString = candidates[i]; const modelResult = await aiService.createModel(modelString, undefined, { + ...context, agentInitiated: true, }); if (!modelResult.success) { diff --git a/src/node/utils/codexOauthAuth.test.ts b/src/node/utils/codexOauthAuth.test.ts index 614461e1b1f..3b4b1b35a20 100644 --- a/src/node/utils/codexOauthAuth.test.ts +++ b/src/node/utils/codexOauthAuth.test.ts @@ -2,6 +2,9 @@ import { describe, it, expect } from "bun:test"; import { parseCodexOauthAuth, + getCodexOauthAccounts, + getCodexOauthAccountId, + getCodexOauthAuth, isCodexOauthAuthExpired, parseJwtClaims, extractAccountIdFromClaims, @@ -238,3 +241,58 @@ describe("extractAccountIdFromTokens", () => { expect(extractAccountIdFromTokens({ accessToken, idToken })).toBe("from_access_token"); }); }); + +describe("Codex OAuth account slots", () => { + const legacy = { type: "oauth", access: "legacy", refresh: "legacy-refresh", expires: 1000 }; + const work = { + type: "oauth", + access: "work", + refresh: "work-refresh", + expires: 2000, + accountId: "remote-chatgpt-id", + }; + + it("reads the legacy slot and named slots with separate local identities", () => { + const config = { + codexOauth: legacy, + codexOauthLabel: "Personal", + codexOauthAccounts: { work: { label: "Work", auth: work } }, + }; + expect(getCodexOauthAccounts(config).map(({ id, label }) => ({ id, label }))).toEqual([ + { id: "default", label: "Personal" }, + { id: "work", label: "Work" }, + ]); + expect(getCodexOauthAuth(config, "work")?.accountId).toBe("remote-chatgpt-id"); + expect(getCodexOauthAuth(config, "remote-chatgpt-id")).toBeNull(); + expect(getCodexOauthAuth(config)?.access).toBe("legacy"); + }); + + it("uses explicit selection before the global default and never substitutes missing slots", () => { + const config = { + codexOauth: legacy, + codexOauthAccounts: { work: { label: "Work", auth: work } }, + codexOauthDefaultAccountId: "work", + }; + expect(getCodexOauthAuth(config)?.access).toBe("work"); + expect(getCodexOauthAuth(config, "default")?.access).toBe("legacy"); + expect(getCodexOauthAuth(config, "missing")).toBeNull(); + expect(getCodexOauthAuth({ ...config, codexOauthDefaultAccountId: "missing" })).toBeNull(); + expect(getCodexOauthAccountId(undefined)).toBe("default"); + expect(getCodexOauthAccountId(config, "missing")).toBe("missing"); + }); + + it("filters malformed slots without accepting a duplicate legacy slot", () => { + const config = { + codexOauth: legacy, + codexOauthAccounts: { + default: { label: "Duplicate", auth: work }, + broken: { label: "Broken", auth: {} }, + blank: { label: " ", auth: work }, + work: { label: "Work", auth: work }, + }, + }; + expect(getCodexOauthAccounts(config).map((account) => account.id)).toEqual(["default", "work"]); + expect(getCodexOauthAccounts(null)).toEqual([]); + expect(getCodexOauthAccounts({ codexOauthAccounts: [] })).toEqual([]); + }); +}); diff --git a/src/node/utils/codexOauthAuth.ts b/src/node/utils/codexOauthAuth.ts index 5f188a1984e..f894af5e187 100644 --- a/src/node/utils/codexOauthAuth.ts +++ b/src/node/utils/codexOauthAuth.ts @@ -5,6 +5,8 @@ * extract non-sensitive claims (e.g. ChatGPT-Account-Id) from OAuth responses. */ +import { CODEX_OAUTH_DEFAULT_ACCOUNT_ID } from "@/common/constants/codexOauthAccounts"; + export interface CodexOauthAuth { type: "oauth"; /** OAuth access token (JWT). */ @@ -48,6 +50,52 @@ export function parseCodexOauthAuth(value: unknown): CodexOauthAuth | null { return { type: "oauth", access, refresh, expires, accountId }; } +/** Read connected slots from an OpenAI provider config. */ +export function getCodexOauthAccounts(config: unknown): Array<{ + id: string; + label: string; + auth: CodexOauthAuth; +}> { + if (!isPlainObject(config)) return []; + const accounts: Array<{ id: string; label: string; auth: CodexOauthAuth }> = []; + const legacy = parseCodexOauthAuth(config.codexOauth); + if (legacy) { + accounts.push({ + id: CODEX_OAUTH_DEFAULT_ACCOUNT_ID, + label: + typeof config.codexOauthLabel === "string" && config.codexOauthLabel.trim() + ? config.codexOauthLabel.trim() + : "Default", + auth: legacy, + }); + } + if (isPlainObject(config.codexOauthAccounts)) { + for (const [id, entry] of Object.entries(config.codexOauthAccounts)) { + // The legacy slot owns this ID, even if malformed disk data repeats it. + if (id === CODEX_OAUTH_DEFAULT_ACCOUNT_ID || !isPlainObject(entry)) continue; + const auth = parseCodexOauthAuth(entry.auth); + if (!auth || typeof entry.label !== "string" || !entry.label.trim()) continue; + accounts.push({ id, label: entry.label.trim(), auth }); + } + } + return accounts; +} + +/** Resolve a local slot ID without substituting another connected account. */ +export function getCodexOauthAccountId(config: unknown, override?: string): string { + if (override !== undefined) return override; + if (isPlainObject(config) && typeof config.codexOauthDefaultAccountId === "string") { + return config.codexOauthDefaultAccountId; + } + return CODEX_OAUTH_DEFAULT_ACCOUNT_ID; +} + +/** Read the selected slot. Missing selections do not fall back to another slot. */ +export function getCodexOauthAuth(config: unknown, accountId?: string): CodexOauthAuth | null { + const selected = getCodexOauthAccountId(config, accountId); + return getCodexOauthAccounts(config).find((account) => account.id === selected)?.auth ?? null; +} + export function isCodexOauthAuthExpired( auth: CodexOauthAuth, opts?: { nowMs?: number; skewMs?: number } diff --git a/src/node/utils/providerRequirements.test.ts b/src/node/utils/providerRequirements.test.ts index 6fb8ce9c874..395b6967d70 100644 --- a/src/node/utils/providerRequirements.test.ts +++ b/src/node/utils/providerRequirements.test.ts @@ -76,6 +76,27 @@ describe("hasAnyConfiguredProvider", () => { expect(hasAnyConfiguredProvider(providers)).toBe(true); }); + it("accepts an additional OAuth account even when the global selection is missing", () => { + expect( + hasAnyConfiguredProvider({ + openai: { + codexOauthDefaultAccountId: "deleted", + codexOauthAccounts: { + work: { + label: "Work", + auth: { + type: "oauth", + access: "access", + refresh: "refresh", + expires: Date.now() + 60_000, + }, + }, + }, + }, + }) + ).toBe(true); + }); + it("returns true for OpenAI Codex OAuth-only configuration", () => { const providers: ProvidersConfig = { openai: { diff --git a/src/node/utils/providerRequirements.ts b/src/node/utils/providerRequirements.ts index af26dccbdd0..2a9002ce98c 100644 --- a/src/node/utils/providerRequirements.ts +++ b/src/node/utils/providerRequirements.ts @@ -23,7 +23,7 @@ import type { OpenAIProviderConfig, } from "@/common/config/schemas/providersConfig"; import type { ProviderConfig, ProvidersConfig } from "@/node/config"; -import { parseCodexOauthAuth } from "@/node/utils/codexOauthAuth"; +import { getCodexOauthAccounts } from "@/node/utils/codexOauthAuth"; import { parseCoderOauthAuth } from "@/node/utils/coderOauthAuth"; import { normalizeCoderDeploymentUrl } from "@/common/constants/coderOAuth"; @@ -609,10 +609,7 @@ export function hasAnyConfiguredProvider(providers: ProvidersConfig | null | und } // OpenAI Codex OAuth is a valid credential path even without apiKey. - if ( - providerKey === "openai" && - parseCodexOauthAuth((rawConfig as { codexOauth?: unknown }).codexOauth) !== null - ) { + if (providerKey === "openai" && getCodexOauthAccounts(rawConfig).length > 0) { return true; } From 7af33f3e759cc7cf8df060276fb6b0b575d9da75 Mon Sep 17 00:00:00 2001 From: Mux Date: Sat, 5 Sep 2026 20:42:46 -0500 Subject: [PATCH 02/24] =?UTF-8?q?[openai]=20=F0=9F=A4=96=20fix:=20address?= =?UTF-8?q?=20account=20routing=20and=20persistence=20reviews?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- _Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `high` • Cost: `$56.93`_ --- src/browser/App.tsx | 11 +- src/browser/components/ChatPane/ChatPane.tsx | 6 +- src/browser/features/ChatInput/index.tsx | 15 +- .../RightSidebar/ContextUsageSection.tsx | 3 +- .../stories/App.codexAccounts.stories.tsx | 102 ++++++++- src/cli/trust.test.ts | 37 ++- src/cli/trust.ts | 5 + src/common/utils/ai/cacheStrategy.ts | 6 +- .../ai/openaiProviderOptionsAvailability.ts | 6 +- src/common/utils/compaction/contextLimit.ts | 4 +- .../utils/providers/codexOauthRouting.test.ts | 105 ++++++++- .../utils/providers/codexOauthRouting.ts | 79 +++++-- .../utils/tokens/tokenMeterUtils.test.ts | 6 +- src/node/services/codexOauthService.test.ts | 116 +++++++++- src/node/services/codexOauthService.ts | 212 +++++++++++------- src/node/services/providerModelFactory.ts | 9 +- src/node/services/providerService.test.ts | 53 +++++ src/node/services/providerService.ts | 24 +- src/node/utils/codexOauthAuth.test.ts | 11 + src/node/utils/codexOauthAuth.ts | 23 +- 20 files changed, 695 insertions(+), 138 deletions(-) diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 5a7615cc946..a8f12c0d55e 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -4,6 +4,7 @@ import { useLocation, useNavigate } from "react-router-dom"; import "./styles/globals.css"; import { useWorkspaceContext, toWorkspaceSelection } from "./contexts/WorkspaceContext"; import { useProjectContext } from "./contexts/ProjectContext"; +import { getCodexOauthProjectPath } from "@/common/utils/providers/codexOauthRouting"; import type { WorkspaceSelection } from "./components/ProjectSidebar/ProjectSidebar"; import { LeftSidebar } from "./components/LeftSidebar/LeftSidebar"; import { ProjectCreateModal } from "./components/ProjectCreateModal/ProjectCreateModal"; @@ -264,7 +265,15 @@ function AppInner() { ) : null; const creationScopeId = creationScope ? getProjectScopeId(creationScope.projectPath) : null; - const accountProjectPath = selectedWorkspace?.projectPath ?? creationScope?.projectPath; + const accountWorkspaceId = selectedWorkspace?.workspaceId ?? currentWorkspaceId; + const accountProjectPath = getCodexOauthProjectPath( + accountWorkspaceId + ? (workspaceMetadata.get(accountWorkspaceId) ?? selectedWorkspace) + : { + projectPath: creationScope?.projectPath, + subProjectPath: creationScope?.subProjectPath ?? undefined, + } + ); const codexOauthAccountId = accountProjectPath ? getProjectConfig(accountProjectPath)?.codexOauthAccountId : undefined; diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index d15d8347778..be8ba2d05f6 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -8,6 +8,7 @@ import React, { useMemo, } from "react"; import { Lightbulb } from "lucide-react"; +import { getCodexOauthProjectPath } from "@/common/utils/providers/codexOauthRouting"; import { MessageListProvider } from "@/browser/features/Messages/MessageListContext"; import { cn } from "@/common/lib/utils"; import { ChatInstructionsChatDecoration } from "@/browser/components/InstructionsTab/AdditionalSystemContextScratchpad"; @@ -469,7 +470,10 @@ const ChatPaneContent: React.FC = (props) => { activeBashMonitorCount, } = workspaceState; const { getProjectConfig } = useProjectContext(); - const codexOauthAccountId = getProjectConfig(projectPath)?.codexOauthAccountId; + const accountProjectPath = getCodexOauthProjectPath(meta ?? { projectPath }); + const codexOauthAccountId = accountProjectPath + ? getProjectConfig(accountProjectPath)?.codexOauthAccountId + : undefined; const shouldShowPinnedTodoList = workspaceState.todos.length > 0; const shouldShowReviewsBanner = reviews.reviews.length > 0; const shouldRenderLoadOlderMessagesButton = hasOlderHistory && !isPixelSnapshotEnvironment(); diff --git a/src/browser/features/ChatInput/index.tsx b/src/browser/features/ChatInput/index.tsx index d20be3737f0..3e687167be2 100644 --- a/src/browser/features/ChatInput/index.tsx +++ b/src/browser/features/ChatInput/index.tsx @@ -165,7 +165,10 @@ import type { import { CreationControls } from "./CreationControls"; import { SEND_DISPATCH_MODES } from "./sendDispatchModes"; import { CodexOauthWarningBanner } from "./CodexOauthWarningBanner"; -import { hasCodexOauthTokens } from "@/common/utils/providers/codexOauthRouting"; +import { + getCodexOauthProjectPath, + hasCodexOauthTokens, +} from "@/common/utils/providers/codexOauthRouting"; import { useCreationWorkspace } from "./useCreationWorkspace"; import { useCoderWorkspace } from "@/browser/hooks/useCoderWorkspace"; import { useTutorial } from "@/browser/contexts/TutorialContext"; @@ -492,7 +495,7 @@ const ChatInputInner: React.FC = (props) => { ); const { open } = useSettings(); - const { selectedWorkspace, beginWorkspaceCreation } = useWorkspaceContext(); + const { selectedWorkspace, workspaceMetadata, beginWorkspaceCreation } = useWorkspaceContext(); const { agentId, currentAgent, agents } = useAgent(); // Use current agent's uiColor, or neutral border until agents load @@ -601,8 +604,12 @@ const ChatInputInner: React.FC = (props) => { const usage = useWorkspaceUsage(workspaceIdForUsage); const { has1MContext } = useProviderOptions(); const { config: providersConfig } = useProvidersConfig(); - const accountProjectPath = - variant === "creation" ? creationParentProjectPath : selectedWorkspace?.projectPath; + const accountProjectPath = getCodexOauthProjectPath( + variant === "creation" + ? { projectPath: creationParentProjectPath, subProjectPath: creationSubProjectPath } + : (workspaceMetadata.get(props.workspaceId) ?? + (selectedWorkspace?.workspaceId === props.workspaceId ? selectedWorkspace : undefined)) + ); const codexOauthAccountId = accountProjectPath ? getProjectConfig(accountProjectPath)?.codexOauthAccountId : undefined; diff --git a/src/browser/features/RightSidebar/ContextUsageSection.tsx b/src/browser/features/RightSidebar/ContextUsageSection.tsx index 898a5fe75ac..69318f49483 100644 --- a/src/browser/features/RightSidebar/ContextUsageSection.tsx +++ b/src/browser/features/RightSidebar/ContextUsageSection.tsx @@ -1,4 +1,5 @@ import React from "react"; +import { getCodexOauthProjectPath } from "@/common/utils/providers/codexOauthRouting"; import { useProjectContext } from "@/browser/contexts/ProjectContext"; import { useWorkspaceContext } from "@/browser/contexts/WorkspaceContext"; import { useWorkspaceUsage } from "@/browser/stores/WorkspaceStore"; @@ -37,7 +38,7 @@ export const ContextUsageSection: React.FC = ({ worksp const { config: providersConfig } = useProvidersConfig(); const { getProjectConfig } = useProjectContext(); const { workspaceMetadata } = useWorkspaceContext(); - const projectPath = workspaceMetadata.get(workspaceId)?.projectPath; + const projectPath = getCodexOauthProjectPath(workspaceMetadata.get(workspaceId)); const codexOauthAccountId = projectPath ? getProjectConfig(projectPath)?.codexOauthAccountId : undefined; diff --git a/src/browser/stories/App.codexAccounts.stories.tsx b/src/browser/stories/App.codexAccounts.stories.tsx index a6e75b62960..f883558bddf 100644 --- a/src/browser/stories/App.codexAccounts.stories.tsx +++ b/src/browser/stories/App.codexAccounts.stories.tsx @@ -1,11 +1,17 @@ import { expect, fn, userEvent, waitFor, within } from "@storybook/test"; import { appMeta, AppWithMocks, type AppStory } from "./meta"; -import { expandLeftSidebar, selectWorkspace } from "./helpers/uiState"; +import { + collapseLeftSidebar, + expandLeftSidebar, + expandProjects, + selectWorkspace, +} from "./helpers/uiState"; import { createMockORPCClient } from "./mocks/orpc"; import { createWorkspace, groupWorkspacesByProject } from "./mocks/workspaces"; import type { APIClient } from "@/browser/contexts/API"; import type { ProvidersConfigMap } from "@/common/orpc/types"; import { Err, Ok } from "@/common/types/result"; +import { MULTI_PROJECT_CONFIG_KEY } from "@/common/constants/multiProject"; export default { ...appMeta, title: "App/CodexAccounts" }; @@ -261,6 +267,100 @@ export const LoginFailureAndCancel: AppStory = { }, }; +function setupScopedAccount(kind: "subproject" | "multi" | "creation") { + const root = "/projects/account-root"; + const subproject = root + "/sub"; + const workspace = createWorkspace({ + id: "scoped-account", + name: "main", + projectName: "account-root", + projectPath: root, + }); + if (kind === "subproject") workspace.subProjectPath = subproject; + if (kind === "multi") { + workspace.projectPath = MULTI_PROJECT_CONFIG_KEY; + workspace.projects = [ + { projectPath: subproject, projectName: "sub" }, + { projectPath: root, projectName: "account-root" }, + ]; + } + selectWorkspace(workspace); + if (kind === "creation") { + expandLeftSidebar(); + expandProjects([root, subproject]); + } else { + collapseLeftSidebar(); + } + const projects = groupWorkspacesByProject([workspace]); + projects.set(root, { + ...projects.get(root), + codexOauthAccountId: "default", + workspaces: projects.get(root)?.workspaces ?? [], + }); + projects.set(subproject, { + parentProjectPath: root, + codexOauthAccountId: "deleted", + workspaces: [], + }); + return createMockORPCClient({ + projects, + workspaces: [workspace], + agentAiDefaults: { + exec: { modelString: "openai:gpt-5.3-codex-spark" }, + plan: { modelString: "openai:gpt-5.3-codex-spark" }, + }, + providersList: ["openai"], + providersConfig: { + openai: { + apiKeySet: true, + isConfigured: true, + isEnabled: true, + codexOauthSet: true, + codexOauthAccounts: [{ id: "default", label: "Personal" }], + }, + }, + }); +} + +async function checkScopedAccountWarning(canvasElement: HTMLElement) { + const warning = await within(canvasElement).findByTestId( + "codex-oauth-warning-banner", + {}, + { timeout: 10000 } + ); + await waitFor(() => expect(warning).toBeVisible()); + if (window.innerWidth < 768) { + await expect(warning.getBoundingClientRect().right).toBeLessThanOrEqual(window.innerWidth); + } +} + +export const SubprojectRouting: AppStory = { + render: () => setupScopedAccount("subproject")} />, + play: async ({ canvasElement }) => checkScopedAccountWarning(canvasElement), +}; + +export const MultiProjectRouting: AppStory = { + render: () => setupScopedAccount("multi")} />, + play: SubprojectRouting.play, +}; + +export const SubprojectRoutingPhone: AppStory = { + ...SubprojectRouting, + globals: { viewport: { value: "mobile1", isRotated: false } }, + parameters: { pixel: { matrix: { themes: ["dark"], viewports: ["phone"] } } }, +}; + +export const CreationSubprojectRouting: AppStory = { + render: () => setupScopedAccount("creation")} />, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click( + await canvas.findByRole("button", { name: "New chat in sub-project" }, { timeout: 10000 }) + ); + await checkScopedAccountWarning(canvasElement); + }, +}; + function setupProjectTitle() { const client = setupAccounts(); generateTitle.mockClear(); diff --git a/src/cli/trust.test.ts b/src/cli/trust.test.ts index 0fa2733f046..73aee8f156b 100644 --- a/src/cli/trust.test.ts +++ b/src/cli/trust.test.ts @@ -1,7 +1,7 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; -import { describe, expect, test } from "bun:test"; +import { describe, expect, spyOn, test } from "bun:test"; import { DisposableTempDir } from "@/node/services/tempDir"; import { Config } from "@/node/config"; import { @@ -153,6 +153,41 @@ describe("xum trust CLI", () => { ).toBeUndefined(); }); + test.each(["work", undefined])( + "rejects a lost account selection write: %s", + async (accountId) => { + using tmp = new DisposableTempDir("codex-account-write-failure"); + const real = new Config(path.join(tmp.path, "real")); + const target = new Config(path.join(tmp.path, "target")); + const projectPath = path.join(tmp.path, "project"); + await real.editConfig((config) => { + config.projects.set(projectPath, { workspaces: [], codexOauthAccountId: accountId }); + return config; + }); + await target.editConfig((config) => { + config.projects.set(projectPath, { workspaces: [], codexOauthAccountId: "personal" }); + return config; + }); + // Simulate a config edit that reports success without writing the selection. + const edit = spyOn(target, "editConfig").mockResolvedValue(undefined); + try { + let error: unknown; + try { + await materializeCodexOauthAccount(real, target, projectPath, projectPath); + } catch (caught) { + error = caught; + } + expect(error).toBeInstanceOf(Error); + expect(String(error)).toContain("Failed to persist Codex OAuth account"); + expect(target.loadConfigOrDefault().projects.get(projectPath)?.codexOauthAccountId).toBe( + "personal" + ); + } finally { + edit.mockRestore(); + } + } + ); + test("replaceRunTrustProjects rebuilds config without foreign settings", async () => { using tmp = new DisposableTempDir("trust-replace-run"); const realConfig = new Config(path.join(tmp.path, "real-root")); diff --git a/src/cli/trust.ts b/src/cli/trust.ts index d6294627ead..2325cad7284 100644 --- a/src/cli/trust.ts +++ b/src/cli/trust.ts @@ -221,6 +221,11 @@ export async function materializeCodexOauthAccount( config.projects.set(targetProjectPath, project); return config; }); + // Config.saveConfig swallows write errors. Never send with an unintended account. + const persistedProject = targetConfig.loadConfigOrDefault().projects.get(targetProjectPath); + if (!persistedProject || persistedProject.codexOauthAccountId !== accountId) { + throw new Error(`Failed to persist Codex OAuth account for ${targetProjectPath}`); + } } /** diff --git a/src/common/utils/ai/cacheStrategy.ts b/src/common/utils/ai/cacheStrategy.ts index 32ae7920c41..d3c7f03a19c 100644 --- a/src/common/utils/ai/cacheStrategy.ts +++ b/src/common/utils/ai/cacheStrategy.ts @@ -4,7 +4,7 @@ import { isGpt56FamilyModel } from "@/common/types/thinking"; import assert from "@/common/utils/assert"; import { cloneToolPreservingDescriptors } from "@/common/utils/tools/cloneToolPreservingDescriptors"; import { - wouldRouteOpenAIThroughCodexOauth, + resolveCodexOauthRouting, type CodexOauthRoutingOptions, } from "@/common/utils/providers/codexOauthRouting"; import { resolveCoderWireCanonicalModel } from "@/common/constants/coderOAuth"; @@ -250,7 +250,7 @@ function isOfficialOpenAIBaseUrl(baseUrl: string): boolean { * capability target must itself be an OpenAI GPT-5.6-family model; * - the backend-resolved route provider must be exactly "openai" — missing, * legacy, gateway, or unknown route metadata fails closed; - * - Codex OAuth precedence (mirrored by wouldRouteOpenAIThroughCodexOauth) + * - Codex OAuth precedence (mirrored by resolveCodexOauthRouting) * fails closed because the ChatGPT backend strips these fields; * - a configured custom base URL fails closed unless it is the official * endpoint. Transport-level HTTP proxy env vars are not endpoint overrides @@ -297,7 +297,7 @@ export function openaiExplicitPromptCachingAvailable( return false; } - if (wouldRouteOpenAIThroughCodexOauth(normalized, providersConfig, options)) { + if (resolveCodexOauthRouting(normalized, providersConfig, options) !== "other") { return false; } diff --git a/src/common/utils/ai/openaiProviderOptionsAvailability.ts b/src/common/utils/ai/openaiProviderOptionsAvailability.ts index 85fc733241d..02f9d12ec71 100644 --- a/src/common/utils/ai/openaiProviderOptionsAvailability.ts +++ b/src/common/utils/ai/openaiProviderOptionsAvailability.ts @@ -6,7 +6,7 @@ import type { ProvidersConfigMap } from "@/common/orpc/types"; import type { OpenAIWireFormat } from "@/common/types/providerOptions"; import { PROVIDER_DEFINITIONS } from "@/common/constants/providers"; import { getExplicitGatewayPrefix, normalizeToCanonical } from "@/common/utils/ai/models"; -import { wouldRouteOpenAIThroughCodexOauth } from "@/common/utils/providers/codexOauthRouting"; +import { resolveCodexOauthRouting } from "@/common/utils/providers/codexOauthRouting"; export interface OpenAIDirectProviderOptionsAvailability { /** Settings-resolved route for the canonical model ("direct" = no gateway). */ @@ -54,9 +54,9 @@ export function openaiDirectProviderOptionsAvailable( // API-only provider options, so toggles for those options must fail closed. return !( options?.providersConfig != null && - wouldRouteOpenAIThroughCodexOauth(normalized, options.providersConfig, { + resolveCodexOauthRouting(normalized, options.providersConfig, { openaiWireFormat: options.openaiWireFormat, codexOauthAccountId: options.codexOauthAccountId, - }) + }) !== "other" ); } diff --git a/src/common/utils/compaction/contextLimit.ts b/src/common/utils/compaction/contextLimit.ts index 55929e4e7a6..f1effa7b198 100644 --- a/src/common/utils/compaction/contextLimit.ts +++ b/src/common/utils/compaction/contextLimit.ts @@ -10,7 +10,7 @@ import { getCodexOauthContextWindowOverride, } from "@/common/constants/codexOAuth"; import { - wouldRouteOpenAIThroughCodexOauth, + resolveCodexOauthRouting, type CodexOauthRoutingOptions, } from "@/common/utils/providers/codexOauthRouting"; import type { ProvidersConfigMap } from "@/common/orpc/types"; @@ -36,7 +36,7 @@ function getCodexOauthContextLimit( return null; } - return wouldRouteOpenAIThroughCodexOauth(model, providersConfig, options) ? oauthLimit : null; + return resolveCodexOauthRouting(model, providersConfig, options) !== "other" ? oauthLimit : null; } /** diff --git a/src/common/utils/providers/codexOauthRouting.test.ts b/src/common/utils/providers/codexOauthRouting.test.ts index 6886808af1c..687151e19cf 100644 --- a/src/common/utils/providers/codexOauthRouting.test.ts +++ b/src/common/utils/providers/codexOauthRouting.test.ts @@ -1,11 +1,114 @@ import { describe, expect, it } from "bun:test"; import type { ProvidersConfigMap } from "@/common/orpc/types"; -import { hasCodexOauthTokens, wouldRouteOpenAIThroughCodexOauth } from "./codexOauthRouting"; +import { + getCodexOauthProjectPath, + hasCodexOauthTokens, + resolveCodexOauthRouting, + wouldRouteOpenAIThroughCodexOauth, +} from "./codexOauthRouting"; +import { getEffectiveContextLimit } from "@/common/utils/compaction/contextLimit"; +import { openaiProModeAvailable } from "@/common/utils/ai/proMode"; +import { openaiExplicitPromptCachingAvailable } from "@/common/utils/ai/cacheStrategy"; +import { getFastModeProvider } from "@/browser/utils/fastModeServiceTier"; import { openaiDirectProviderOptionsAvailable } from "@/common/utils/ai/openaiProviderOptionsAvailability"; const auth = { type: "oauth", access: "access", refresh: "refresh", expires: 1000 }; describe("Codex OAuth account routing", () => { + it("uses multi-project attribution before subproject and root scope", () => { + const scope = { + projectPath: "/root", + attributionProjectPath: "/attribution", + subProjectPath: "/root/sub", + projects: [ + { projectPath: "/first", projectName: "first" }, + { projectPath: "/second", projectName: "second" }, + ], + }; + expect(getCodexOauthProjectPath(scope)).toBe("/first"); + expect(getCodexOauthProjectPath({ ...scope, projects: [] })).toBe("/root/sub"); + expect(getCodexOauthProjectPath({ ...scope, projects: [], subProjectPath: undefined })).toBe( + "/attribution" + ); + expect(getCodexOauthProjectPath({ projectPath: "/root" })).toBe("/root"); + expect( + getCodexOauthProjectPath({ projectPath: "/root", subProjectPath: "/root/new-subproject" }) + ).toBe("/root/new-subproject"); + expect(getCodexOauthProjectPath()).toBeUndefined(); + }); + + it.each(["project", "global", "implicit"])( + "fails closed for a missing %s OAuth selection", + (source) => { + const providersConfig: ProvidersConfigMap = { + openai: { + apiKeySet: true, + isConfigured: true, + isEnabled: true, + codexOauthAccounts: [{ id: "work", label: "Work" }], + codexOauthSet: true, + ...(source === "global" ? { codexOauthDefaultAccountId: "deleted" } : {}), + }, + }; + const options = { + providersConfig, + ...(source === "project" ? { codexOauthAccountId: "deleted" } : {}), + }; + const model = "openai:gpt-5.6-sol"; + expect(resolveCodexOauthRouting(model, providersConfig, options)).toBe("missing-account"); + expect(wouldRouteOpenAIThroughCodexOauth(model, providersConfig, options)).toBe(false); + expect(openaiProModeAvailable(model, options)).toBe(false); + expect(getFastModeProvider(model, options)).toBeNull(); + expect(openaiExplicitPromptCachingAvailable(model, "openai", providersConfig, options)).toBe( + false + ); + expect(getEffectiveContextLimit(model, false, providersConfig, options)).toBe(372_000); + expect( + resolveCodexOauthRouting(model, providersConfig, { + ...options, + codexOauthAccountId: "work", + }) + ).toBe("oauth"); + + providersConfig.openai.codexOauthDefaultAuth = "apiKey"; + expect(resolveCodexOauthRouting(model, providersConfig, options)).toBe("other"); + expect(openaiProModeAvailable(model, options)).toBe(true); + expect(getFastModeProvider(model, options)).toBe("openai"); + expect(getEffectiveContextLimit(model, false, providersConfig, options)).toBeGreaterThan( + 372_000 + ); + // Required Codex models still reject the missing selected slot. + expect(resolveCodexOauthRouting("openai:gpt-5.3-codex-spark", providersConfig, options)).toBe( + "missing-account" + ); + } + ); + + it("keeps legacy API keys and gateway routes independent from missing selections", () => { + const providersConfig: ProvidersConfigMap = { + openai: { apiKeySet: true, isConfigured: true, isEnabled: true }, + }; + const model = "openai:gpt-5.6-sol"; + expect(resolveCodexOauthRouting(model, providersConfig)).toBe("other"); + const options = { + providersConfig, + codexOauthAccountId: "deleted", + openaiWireFormat: "chatCompletions" as const, + }; + expect(resolveCodexOauthRouting(model, providersConfig, options)).toBe("other"); + expect( + getFastModeProvider(model, { ...options, resolvedRouteProvider: "openrouter" }) + ).toBeNull(); + expect( + resolveCodexOauthRouting("openrouter:openai/gpt-5.6-sol", providersConfig, options) + ).toBe("other"); + expect( + getEffectiveContextLimit("openrouter:openai/gpt-5.6-sol", false, providersConfig, options) + ).toBeGreaterThan(372_000); + providersConfig.openai.codexOauthDefaultAccountId = "deleted"; + expect(resolveCodexOauthRouting(model, providersConfig)).toBe("missing-account"); + }); + it("keeps legacy metadata and raw token defaults compatible", () => { for (const config of [{ codexOauthSet: true }, { codexOauth: auth }]) { expect(hasCodexOauthTokens(config)).toBe(true); diff --git a/src/common/utils/providers/codexOauthRouting.ts b/src/common/utils/providers/codexOauthRouting.ts index 875d24241b7..c3f2fe6d847 100644 --- a/src/common/utils/providers/codexOauthRouting.ts +++ b/src/common/utils/providers/codexOauthRouting.ts @@ -12,6 +12,7 @@ import { isCodexOauthAllowedModel, isCodexOauthRequiredModel } from "@/common/constants/codexOAuth"; import type { ProvidersConfigMap } from "@/common/orpc/types"; import type { OpenAIWireFormat } from "@/common/types/providerOptions"; +import type { WorkspaceMetadata } from "@/common/types/workspace"; import { CODEX_OAUTH_DEFAULT_ACCOUNT_ID } from "@/common/constants/codexOauthAccounts"; /** Request-level inputs the stored providers config cannot carry. */ @@ -25,6 +26,22 @@ export interface CodexOauthRoutingOptions { codexOauthAccountId?: string; } +/** Use the same account scope as backend model construction. */ +export function getCodexOauthProjectPath( + scope?: + | (Partial> & { + attributionProjectPath?: string; + }) + | null +): string | undefined { + return ( + scope?.projects?.[0]?.projectPath ?? + scope?.subProjectPath ?? + scope?.attributionProjectPath ?? + scope?.projectPath + ); +} + function asRecord(value: unknown): Record | null { if (typeof value !== "object" || value === null || Array.isArray(value)) { return null; @@ -88,38 +105,54 @@ export function hasOpenAIApiKey(config: unknown): boolean { return record.apiKeySet === true || hasNonEmptyString(record.apiKey); } -/** - * Would a direct-OpenAI request for this model route through Codex OAuth? - * - * Mirrors providerModelFactory: allowed model + stored OAuth tokens, then - * Chat Completions with an API key never routes OAuth, required models always - * route OAuth; otherwise OAuth wins when no API key is configured or when - * `codexOauthDefaultAuth` prefers OAuth over a present key. - */ -export function wouldRouteOpenAIThroughCodexOauth( +export type CodexOauthRouting = "oauth" | "missing-account" | "other"; + +/** Resolve OAuth routing without treating a missing selection as API-key fallback. */ +export function resolveCodexOauthRouting( model: string, providersConfig: ProvidersConfigMap | null | undefined, options?: CodexOauthRoutingOptions -): boolean { +): CodexOauthRouting { const openAIConfig = providersConfig?.openai; if (!isCodexOauthAllowedModel(model, providersConfig ?? null)) { - return false; + return "other"; } - if (!hasCodexOauthTokens(openAIConfig, options?.codexOauthAccountId)) { - return false; + const record = asRecord(openAIConfig); + const hasApiKey = hasOpenAIApiKey(openAIConfig); + const hasSelectedAccount = hasCodexOauthTokens(openAIConfig, options?.codexOauthAccountId); + const required = isCodexOauthRequiredModel(model, providersConfig ?? null); + if (required && !hasSelectedAccount && !hasApiKey) { + return "missing-account"; } - // Codex OAuth serves only the Responses API. With Chat Completions selected, - // the factory falls back to the API key whenever one exists. - const wireFormat = asRecord(openAIConfig)?.wireFormat ?? options?.openaiWireFormat; - if (wireFormat === "chatCompletions" && hasOpenAIApiKey(openAIConfig)) { - return false; + // Chat Completions uses the API key, even when OAuth is preferred. + const wireFormat = record?.wireFormat ?? options?.openaiWireFormat; + if (wireFormat === "chatCompletions" && hasApiKey) { + return "other"; } - if (isCodexOauthRequiredModel(model, providersConfig ?? null)) { - return true; + if (!required && hasApiKey && record?.codexOauthDefaultAuth === "apiKey") { + return "other"; } - if (!hasOpenAIApiKey(openAIConfig)) { - return true; + if (hasSelectedAccount) { + return "oauth"; } - return asRecord(openAIConfig)?.codexOauthDefaultAuth !== "apiKey"; + const hasExplicitSelection = + options?.codexOauthAccountId !== undefined || record?.codexOauthDefaultAccountId !== undefined; + const accounts = record?.codexOauthAccounts; + const hasConnectedAccount = Array.isArray(accounts) + ? accounts.length > 0 + : record?.codexOauthSet === true || + hasCodexOauthTokens(openAIConfig, CODEX_OAUTH_DEFAULT_ACCOUNT_ID) || + Object.keys(asRecord(accounts) ?? {}).some((id) => hasCodexOauthTokens(openAIConfig, id)); + // Missing slots must not enable API-only controls or remove the OAuth context cap. + return hasExplicitSelection || hasConnectedAccount ? "missing-account" : "other"; +} + +/** Return true only when the selected OAuth account can serve the request. */ +export function wouldRouteOpenAIThroughCodexOauth( + model: string, + providersConfig: ProvidersConfigMap | null | undefined, + options?: CodexOauthRoutingOptions +): boolean { + return resolveCodexOauthRouting(model, providersConfig, options) === "oauth"; } diff --git a/src/common/utils/tokens/tokenMeterUtils.test.ts b/src/common/utils/tokens/tokenMeterUtils.test.ts index 74080d583c6..e8d72cefa16 100644 --- a/src/common/utils/tokens/tokenMeterUtils.test.ts +++ b/src/common/utils/tokens/tokenMeterUtils.test.ts @@ -76,7 +76,7 @@ describe("calculateTokenMeterData", () => { expect(result.totalPercentage).toBeCloseTo(1.1); }); - test("uses the selected account rather than an unavailable global account", () => { + test("keeps the OAuth cap for an unavailable global account and a connected project account", () => { const providersConfig: ProvidersConfigMap = { openai: { apiKeySet: true, @@ -103,8 +103,8 @@ describe("calculateTokenMeterData", () => { { codexOauthAccountId: "work" } ); expect(projectMeter.maxTokens).toBe(272_000); - expect(globalMeter.maxTokens).toBeGreaterThan(projectMeter.maxTokens!); - expect(projectMeter.totalPercentage).toBeGreaterThan(globalMeter.totalPercentage); + expect(globalMeter.maxTokens).toBe(projectMeter.maxTokens); + expect(projectMeter.totalPercentage).toBe(globalMeter.totalPercentage); }); test("uses the Codex OAuth cap for GPT-5.5 token meter percentages", () => { diff --git a/src/node/services/codexOauthService.test.ts b/src/node/services/codexOauthService.test.ts index b308b71a8fe..5f62e82148b 100644 --- a/src/node/services/codexOauthService.test.ts +++ b/src/node/services/codexOauthService.test.ts @@ -2,11 +2,15 @@ import { Config, FileLeaseManager, ProvidersConfigStore } from "@/node/config"; import * as fs from "fs"; import * as os from "os"; import * as path from "path"; -import { describe, it, expect, beforeEach, afterEach } from "bun:test"; +import { describe, it, expect, beforeEach, afterEach, spyOn } from "bun:test"; import { Err, Ok } from "@/common/types/result"; import { Effect } from "effect"; -import { getCodexOauthAccounts, getCodexOauthAuth } from "@/node/utils/codexOauthAuth"; +import { + getCodexOauthAccounts, + getCodexOauthAccountId, + getCodexOauthAuth, +} from "@/node/utils/codexOauthAuth"; import { createDeferred } from "@/node/utils/oauthUtils"; import { CODEX_OAUTH_TOKEN_URL, @@ -87,7 +91,7 @@ function createMockProvidersConfigStore( function createMockProviderService( deps: MockDeps -): Pick { +): Pick { const setConfigValue: ProviderService["setConfigValue"] = (provider, keyPath, value) => { deps.setConfigValueCalls.push({ provider, keyPath, value }); deps.providersConfig[provider] ??= {}; @@ -103,6 +107,16 @@ function createMockProviderService( }; return { setConfigValue, + updateProviderSection: (provider, update, options) => { + if (options?.enforcePolicy && deps.policyDenied) + return Promise.resolve(Err("Provider edits are disabled")); + const next = update(deps.providersConfig[provider]); + deps.onUpdate?.(); + if (!next) return Promise.resolve(Ok({ applied: false })); + deps.providersConfig[provider] = next.value; + deps.setConfigValueCalls.push({ provider, keyPath: [], value: next.value }); + return Promise.resolve(Ok({ applied: true })); + }, updateConfigValue: async (provider, keyPath, update, options) => { if (options?.enforcePolicy && deps.policyDenied) return Err("Provider edits are disabled"); let current: unknown = deps.providersConfig[provider]; @@ -988,6 +1002,102 @@ describe("CodexOauthService", () => { expect(getCodexOauthAuth(deps.providersConfig.openai)?.access).toBe("access-device-2"); }); + it("completes a reconnect after a concurrent refresh persists first", async () => { + deps.providersConfig = { + openai: { + codexOauthAccounts: { work: { label: "Work", auth: expiredAuth({ refresh: "old" }) } }, + }, + }; + const refreshStarted = createDeferred(); + const refreshResponse = createDeferred(); + const exchangeStarted = createDeferred(); + const exchangeResponse = createDeferred(); + deviceFetch((init) => { + if (new URLSearchParams(requestBody(init)).get("grant_type") === "refresh_token") { + refreshStarted.resolve(undefined); + return refreshResponse.promise; + } + exchangeStarted.resolve(undefined); + return exchangeResponse.promise; + }); + const refresh = service.getValidAuth("work"); + await refreshStarted.promise; + const flow = await service.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + const reconnect = service.waitForDeviceFlow(flow.data.flowId); + await exchangeStarted.promise; + refreshResponse.resolve( + mockRefreshResponse({ + access_token: "refreshed", + refresh_token: "rotated", + expires_in: 3600, + }) + ); + expect((await refresh).success).toBe(true); + exchangeResponse.resolve( + mockRefreshResponse({ + access_token: "reconnected", + refresh_token: "new-login", + expires_in: 3600, + }) + ); + expect(await reconnect).toEqual(Ok(undefined)); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")?.refresh).toBe("new-login"); + }); + + it.each([{ accounts: [] }, { accounts: null }, { accounts: 42 }])( + "repairs a malformed account map during named login: %j", + async ({ accounts }) => { + const provider = new ProviderService(new Config(deps.rootDir)); + const store = provider.providersConfigStore; + store.saveProvidersConfig({ openai: { codexOauthAccounts: accounts, apiKey: "keep-key" } }); + const realService = new CodexOauthService(store, provider); + deviceFetch(); + try { + const flow = await realService.startDeviceFlow({ label: "Work" }); + if (!flow.success) throw new Error(flow.error); + expect(await realService.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + const openai = store.loadProvidersConfig()?.openai; + const connected = getCodexOauthAccounts(openai); + expect(connected).toHaveLength(1); + expect(getCodexOauthAccountId(openai)).toBe(connected[0].id); + expect(openai?.apiKey).toBe("keep-key"); + } finally { + await realService.dispose(); + } + } + ); + + it("does not leave a partial first login when saving the selected account fails", async () => { + const provider = new ProviderService(new Config(deps.rootDir)); + const store = provider.providersConfigStore; + store.saveProvidersConfig({ openai: { apiKey: "keep-key" } }); + const save = store.saveProvidersConfig.bind(store); + const saveSpy = spyOn(store, "saveProvidersConfig").mockImplementation((config) => { + // Fail the write that makes the new account the selected account. + const openai = config.openai; + if ( + getCodexOauthAccounts(openai).some( + (account) => account.id === getCodexOauthAccountId(openai) + ) + ) { + throw new Error("Disk unavailable"); + } + save(config); + }); + const realService = new CodexOauthService(store, provider); + deviceFetch(); + try { + const flow = await realService.startDeviceFlow({ label: "Work" }); + if (!flow.success) throw new Error(flow.error); + expect((await realService.waitForDeviceFlow(flow.data.flowId)).success).toBe(false); + expect(store.loadProvidersConfig()?.openai).toEqual({ apiKey: "keep-key" }); + } finally { + saveSpy.mockRestore(); + await realService.dispose(); + } + }); + it("keeps a successful reconnect when an older refresh completes", async () => { deps.providersConfig = { openai: { diff --git a/src/node/services/codexOauthService.ts b/src/node/services/codexOauthService.ts index 1618519b572..79da534a620 100644 --- a/src/node/services/codexOauthService.ts +++ b/src/node/services/codexOauthService.ts @@ -27,10 +27,7 @@ import { } from "@/common/constants/codexOAuth"; import { CODEX_OAUTH_DEFAULT_ACCOUNT_ID, - CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH, CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH, - CODEX_OAUTH_ACCOUNT_ID_PATTERN, - CODEX_OAUTH_RESERVED_ACCOUNT_IDS, CODEX_OAUTH_REFRESH_TIMEOUT_MS, } from "@/common/constants/codexOauthAccounts"; import { FileLeaseManager, type ProvidersConfigStore } from "@/node/config"; @@ -45,6 +42,7 @@ import { getCodexOauthAccountId, getCodexOauthAuth, isCodexOauthAuthExpired, + isValidCodexOauthAccountId, parseCodexOauthAuth, type CodexOauthAuth, } from "@/node/utils/codexOauthAuth"; @@ -65,7 +63,6 @@ export interface CodexOauthLoginOptions { interface AccountSelection { accountId: string; revision: number; - loginAttempt?: number; auth: CodexOauthAuth | null; label?: string; selectAsDefault: boolean; @@ -144,14 +141,6 @@ export class CodexOauthError extends Schema.TaggedError()("Code reason: Schema.String, }) {} -function isValidAccountId(accountId: string): boolean { - return ( - accountId.length <= CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH && - CODEX_OAUTH_ACCOUNT_ID_PATTERN.test(accountId) && - !CODEX_OAUTH_RESERVED_ACCOUNT_IDS.has(accountId) - ); -} - function isValidLabel(label: string): boolean { return label.trim().length > 0 && label.trim().length <= CODEX_OAUTH_ACCOUNT_LABEL_MAX_LENGTH; } @@ -172,7 +161,8 @@ export class CodexOauthService { private readonly refreshMutexes = new Map(); private readonly accountRevisions = new Map(); - private readonly loginAttempts = new Map(); + private readonly loginSelections = new Map(); + private readonly authMutationMutexes = new Map(); constructor( private readonly providersConfigStore: ProvidersConfigStore, @@ -190,7 +180,7 @@ export class CodexOauthService { accountId = CODEX_OAUTH_DEFAULT_ACCOUNT_ID ): Effect.Effect> { return Effect.suspend(() => { - if (!isValidAccountId(accountId)) + if (!isValidCodexOauthAccountId(accountId)) return Effect.succeed(Err("Invalid Codex OAuth account ID")); // Invalidate pending refreshes and logins before clearing this slot. this.accountRevisions.set(accountId, this.getAccountRevision(accountId) + 1); @@ -206,7 +196,7 @@ export class CodexOauthService { setDefaultAccountEffect(accountId: string): Effect.Effect> { return Effect.suspend(() => { - if (!isValidAccountId(accountId)) + if (!isValidCodexOauthAccountId(accountId)) return Effect.succeed(Err("Invalid Codex OAuth account ID")); return this.updateConfigValueEffect(["codexOauthDefaultAccountId"], () => this.readStoredAuth(accountId) ? { value: accountId } : null @@ -220,7 +210,7 @@ export class CodexOauthService { renameAccountEffect(accountId: string, label: string): Effect.Effect> { return Effect.suspend(() => { - if (!isValidAccountId(accountId)) + if (!isValidCodexOauthAccountId(accountId)) return Effect.succeed(Err("Invalid Codex OAuth account ID")); if (!isValidLabel(label)) return Effect.succeed(Err("Invalid Codex OAuth account label")); if (accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID) { @@ -599,7 +589,7 @@ export class CodexOauthService { return Effect.gen(function* () { // Resolve once. A default change must not switch an active request to another account. const selectedId = getCodexOauthAccountId(self.readOpenaiConfig(), accountId); - if (!isValidAccountId(selectedId)) return Err("Invalid Codex OAuth account ID"); + if (!isValidCodexOauthAccountId(selectedId)) return Err("Invalid Codex OAuth account ID"); const revision = self.getAccountRevision(selectedId); const stored = self.readStoredAuth(selectedId); if (!stored) return Err(`Codex OAuth account "${selectedId}" is not configured`); @@ -674,6 +664,8 @@ export class CodexOauthService { } private accountPath(accountId: string): string[] { + // Do not mirror named credentials into the legacy slot. Older versions refresh that slot independently. + // Keep existing legacy credentials until explicit disconnect; downgrade support does not include named accounts. return accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID ? ["codexOauth"] : ["codexOauthAccounts", accountId]; @@ -682,13 +674,18 @@ export class CodexOauthService { private updateConfigValueEffect( keyPath: string[], update: (current: unknown) => { value: unknown } | null + ): Effect.Effect> { + return this.configMutationEffect(() => + this.providerService.updateConfigValue("openai", keyPath, update, { enforcePolicy: true }) + ); + } + + private configMutationEffect( + mutation: () => Promise> ): Effect.Effect> { return Effect.uninterruptible( Effect.tryPromise({ - try: () => - this.providerService.updateConfigValue("openai", keyPath, update, { - enforcePolicy: true, - }), + try: mutation, catch: (error) => getErrorMessage(error), }).pipe( Effect.map((result): Result => { @@ -702,6 +699,26 @@ export class CodexOauthService { ); } + private withAccountMutationEffect( + accountId: string, + mutation: Effect.Effect + ): Effect.Effect { + let mutex = this.authMutationMutexes.get(accountId); + if (!mutex) { + mutex = new AsyncMutex(); + this.authMutationMutexes.set(accountId, mutex); + } + const mutationMutex = mutex; + // Keep persistence and local snapshot updates together. Network requests stay outside this lock. + return Effect.uninterruptible( + Effect.acquireUseRelease( + Effect.promise(() => mutationMutex.acquire()), + () => mutation, + (lock) => Effect.promise(() => lock[Symbol.asyncDispose]()) + ) + ); + } + private selectLoginDestination( options?: CodexOauthLoginOptions ): Effect.Effect { @@ -717,7 +734,7 @@ export class CodexOauthService { const accountId = options?.accountId ?? (options?.label !== undefined ? crypto.randomUUID() : CODEX_OAUTH_DEFAULT_ACCOUNT_ID); - if (!isValidAccountId(accountId)) { + if (!isValidCodexOauthAccountId(accountId)) { return Effect.fail(new CodexOauthError({ reason: "Invalid Codex OAuth account ID" })); } const auth = this.readStoredAuth(accountId); @@ -728,47 +745,55 @@ export class CodexOauthService { } // A failed or cancelled login must not discard a pending token rotation. const revision = this.getAccountRevision(accountId); - const loginAttempt = (this.loginAttempts.get(accountId) ?? 0) + 1; - this.loginAttempts.set(accountId, loginAttempt); - return Effect.succeed({ + const selection: AccountSelection = { accountId, revision, - loginAttempt, auth, label: options?.label?.trim(), selectAsDefault: options?.label !== undefined && getCodexOauthAccounts(this.readOpenaiConfig()).length === 0, - }); + }; + this.loginSelections.set(accountId, selection); + return Effect.succeed(selection); }); } private persistAuth( selection: AccountSelection, - auth: CodexOauthAuth | undefined, - isActive: () => boolean = () => true + auth: CodexOauthAuth | undefined ): Effect.Effect> { - return this.updateConfigValueEffect(this.accountPath(selection.accountId), (current) => { - const legacy = selection.accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID; - const stored = parseCodexOauthAuth( - legacy ? current : isPlainObject(current) ? current.auth : undefined - ); - // Compare under the file lock. Old refreshes must not restore deleted or reconnected slots. - if ( - !isActive() || - this.getAccountRevision(selection.accountId) !== selection.revision || - !matchesAuth(stored, selection.auth) + return this.withAccountMutationEffect( + selection.accountId, + this.updateConfigValueEffect(this.accountPath(selection.accountId), (current) => { + const legacy = selection.accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID; + const stored = parseCodexOauthAuth( + legacy ? current : isPlainObject(current) ? current.auth : undefined + ); + // Compare under the file lock. Old refreshes must not restore deleted or reconnected slots. + if ( + this.getAccountRevision(selection.accountId) !== selection.revision || + !matchesAuth(stored, selection.auth) + ) + return null; + if (legacy || auth === undefined) return { value: auth }; + return { value: { ...(isPlainObject(current) ? current : {}), auth } }; + }).pipe( + Effect.map((result) => { + const login = this.loginSelections.get(selection.accountId); + // A local refresh changes tokens, not the identity of an in-progress interactive login. + if ( + result.success && + auth && + login?.revision === selection.revision && + matchesAuth(login.auth, selection.auth) + ) { + login.auth = auth; + } + return result; + }) ) - return null; - if (legacy || auth === undefined) return { value: auth }; - return { - value: { - ...(isPlainObject(current) ? current : {}), - label: isPlainObject(current) ? current.label : selection.label, - auth, - }, - }; - }); + ); } private persistLoginAuth( @@ -776,38 +801,71 @@ export class CodexOauthService { auth: CodexOauthAuth, isActive: () => boolean ): Effect.Effect> { - // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect generators do not inherit this. - const self = this; - return Effect.gen(function* () { - const result = yield* self.persistAuth( - selection, - auth, - () => isActive() && self.loginAttempts.get(selection.accountId) === selection.loginAttempt - ); - if (!result.success) return result; - self.accountRevisions.set( - selection.accountId, - self.getAccountRevision(selection.accountId) + 1 - ); - if (!selection.selectAsDefault) return result; - // Concurrent first logins share this default write. The first successful write wins. - const selected = yield* Effect.promise(() => - self.providerService.updateConfigValue( + return this.withAccountMutationEffect( + selection.accountId, + this.configMutationEffect(() => + this.providerService.updateProviderSection( "openai", - ["codexOauthDefaultAccountId"], - (current) => { - const accounts = getCodexOauthAccounts(self.readOpenaiConfig()); - return current === undefined && - accounts.some((account) => account.id === selection.accountId) && - !accounts.some((account) => account.id === CODEX_OAUTH_DEFAULT_ACCOUNT_ID) - ? { value: selection.accountId } - : null; + (section) => { + if ( + !isActive() || + this.loginSelections.get(selection.accountId) !== selection || + this.getAccountRevision(selection.accountId) !== selection.revision + ) + return null; + const current = isPlainObject(section) ? section : {}; + const legacy = selection.accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID; + // Hand-edited arrays cannot retain named properties when JSON serializes the config. + const accounts = isPlainObject(current.codexOauthAccounts) + ? current.codexOauthAccounts + : {}; + const entry = accounts[selection.accountId]; + const stored = parseCodexOauthAuth( + legacy ? current.codexOauth : isPlainObject(entry) ? entry.auth : undefined + ); + if (!matchesAuth(stored, selection.auth)) return null; + const next = { ...current }; + if (legacy) { + next.codexOauth = auth; + } else { + // Do not mirror named credentials into the legacy slot. + // Older versions refresh that slot independently and cannot honor project selection. + // Named login preserves existing legacy credentials until explicit disconnect. + next.codexOauthAccounts = { + ...accounts, + [selection.accountId]: { + ...(isPlainObject(entry) ? entry : {}), + label: isPlainObject(entry) ? entry.label : selection.label, + auth, + }, + }; + // Commit the first slot and its selection together. A failed write must leave neither field. + if ( + selection.selectAsDefault && + current.codexOauthDefaultAccountId === undefined && + !parseCodexOauthAuth(current.codexOauth) + ) { + next.codexOauthDefaultAccountId = selection.accountId; + } + } + return { value: next }; }, { enforcePolicy: true } ) - ); - return selected.success ? Ok(undefined) : selected; - }); + ).pipe( + Effect.map((result) => { + if (result.success) { + this.accountRevisions.set( + selection.accountId, + this.getAccountRevision(selection.accountId) + 1 + ); + if (this.loginSelections.get(selection.accountId) === selection) + this.loginSelections.delete(selection.accountId); + } + return result; + }) + ) + ); } private handleDesktopCallbackAndExchange(input: { diff --git a/src/node/services/providerModelFactory.ts b/src/node/services/providerModelFactory.ts index 83385344c7f..57297bd24c3 100644 --- a/src/node/services/providerModelFactory.ts +++ b/src/node/services/providerModelFactory.ts @@ -29,6 +29,7 @@ import { ProvidersConfigStore } from "@/node/config"; import type { MuxProviderOptions } from "@/common/types/providerOptions"; import type { ServiceTier, XAIServiceTier } from "@/common/config/schemas/providersConfig"; import { resolveConfigBaseUrl } from "@/common/utils/providers/baseUrl"; +import { getCodexOauthProjectPath } from "@/common/utils/providers/codexOauthRouting"; import { isProviderDisabledInConfig } from "@/common/utils/providers/isProviderDisabled"; import { customProviderWireOrigin, @@ -1210,13 +1211,7 @@ export class ProviderModelFactory { return opts.codexOauthSelection; } const workspace = opts?.workspaceId ? this.config.findWorkspace(opts.workspaceId) : null; - // Multi-project attribution wins. Single-project workspaces use their registered subproject. - const projectPath = - workspace?.projects?.[0]?.projectPath ?? - workspace?.subProjectPath ?? - workspace?.attributionProjectPath ?? - workspace?.projectPath ?? - opts?.projectPath; + const projectPath = getCodexOauthProjectPath(workspace) ?? opts?.projectPath; const projectAccountId = projectPath ? this.config.loadConfigOrDefault().projects.get(projectPath)?.codexOauthAccountId : undefined; diff --git a/src/node/services/providerService.test.ts b/src/node/services/providerService.test.ts index f2f2c37c133..f0d64319049 100644 --- a/src/node/services/providerService.test.ts +++ b/src/node/services/providerService.test.ts @@ -2146,6 +2146,59 @@ describe("ProviderService.updateConfigValue", () => { }); }); +describe("ProviderService.updateProviderSection policy", () => { + it("denies user section writes but preserves internal mutation behavior", async () => { + await withTempPolicyProviderService( + { policy_format_version: "0.1", provider_access: [{ id: "anthropic" }] }, + async (config, service) => { + const denied = await service.updateProviderSection( + "openai", + () => ({ value: { codexOauthDefaultAccountId: "work" } }), + { enforcePolicy: true } + ); + expect(denied.success).toBe(false); + expect( + new ProvidersConfigStore(config.rootDir).loadProvidersConfig()?.openai + ).toBeUndefined(); + const internal = await service.updateProviderSection("openai", () => ({ + value: { codexOauthDefaultAccountId: "work" }, + })); + expect(internal).toEqual({ success: true, data: { applied: true } }); + } + ); + }); + + it("allows unchanged locked URLs but rejects section writes that change them", async () => { + await withTempPolicyProviderService( + { + policy_format_version: "0.1", + provider_access: [{ id: "openai", base_url: "https://locked.example.com" }], + }, + async (config, service) => { + const store = new ProvidersConfigStore(config.rootDir); + store.saveProvidersConfig({ + openai: { baseUrl: "https://old.example.com", apiKey: "keep-key" }, + }); + expect( + await service.updateProviderSection( + "openai", + (section) => ({ value: { ...section, codexOauthDefaultAccountId: "work" } }), + { enforcePolicy: true } + ) + ).toEqual({ success: true, data: { applied: true } }); + const before = store.loadProvidersConfig()?.openai; + const denied = await service.updateProviderSection( + "openai", + (section) => ({ value: { ...section, baseUrl: "https://new.example.com" } }), + { enforcePolicy: true } + ); + expect(denied.success).toBe(false); + expect(store.loadProvidersConfig()?.openai).toEqual(before); + } + ); + }); +}); + describe("ProviderService gateway lifecycle", () => { it("auto-inserts gateway into routePriority when configured", async () => { await withTempConfigAsync(async (config, service) => { diff --git a/src/node/services/providerService.ts b/src/node/services/providerService.ts index 1619ca95b00..770ac720138 100644 --- a/src/node/services/providerService.ts +++ b/src/node/services/providerService.ts @@ -1577,28 +1577,33 @@ export class ProviderService { * that fetched it is still the stored credential, and disconnect clears * tokens + models in one write. * - * Internal credential-management primitive: skips policy gating like - * updateConfigValue. + * Internal callers can omit policy checks. User-driven mutations must set enforcePolicy. */ public updateProviderSection( provider: string, update: ( section: Record | undefined - ) => { value: Record } | null + ) => { value: Record } | null, + options?: { enforcePolicy?: boolean } ): Promise> { - return Effect.runPromise(this.updateProviderSectionEffect(provider, update)); + return Effect.runPromise(this.updateProviderSectionEffect(provider, update, options)); } private updateProviderSectionEffect( provider: string, update: ( section: Record | undefined - ) => { value: Record } | null + ) => { value: Record } | null, + options?: { enforcePolicy?: boolean } ): Effect.Effect> { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; return Effect.gen(function* () { const applied = yield* self.providersFileLockEffect(() => { + if (options?.enforcePolicy) { + const denial = self.validateProviderEditPolicy(provider, []); + if (denial != null) return denial; + } const providersConfig = self.providersConfigStore.loadProvidersConfig() ?? {}; const section = providersConfig[provider] as Record | undefined; @@ -1607,6 +1612,14 @@ export class ProviderService { return false; } + if (options?.enforcePolicy) { + for (const key of ["baseUrl", "baseURL"]) { + if (decision.value[key] === section?.[key]) continue; + const denial = self.validateProviderEditPolicy(provider, [key]); + if (denial != null) return denial; + } + } + const deniedKey = Object.keys(decision.value).find((key) => DENIED_KEY_PATH_SEGMENTS.has(key) ); @@ -1619,6 +1632,7 @@ export class ProviderService { return true; }); + if (typeof applied === "string") return { success: false as const, error: applied }; // Best-effort: a landed write must not be reported as failed (see // afterAppliedMutationEffect). yield* self.afterAppliedMutationEffect(provider, applied); diff --git a/src/node/utils/codexOauthAuth.test.ts b/src/node/utils/codexOauthAuth.test.ts index 3b4b1b35a20..c2099cb6630 100644 --- a/src/node/utils/codexOauthAuth.test.ts +++ b/src/node/utils/codexOauthAuth.test.ts @@ -281,6 +281,17 @@ describe("Codex OAuth account slots", () => { expect(getCodexOauthAccountId(config, "missing")).toBe("missing"); }); + it("excludes stored IDs that cannot pass account mutation validation", () => { + const invalidIds = ["", "__proto__", "constructor", "prototype", "../bad", "x".repeat(201)]; + const accounts = Object.fromEntries( + invalidIds.map((id) => [id, { label: "Invalid", auth: work }]) + ); + const config = { codexOauthAccounts: { ...accounts, work: { label: "Work", auth: work } } }; + expect(getCodexOauthAccounts(config).map((account) => account.id)).toEqual(["work"]); + for (const id of invalidIds) expect(getCodexOauthAuth(config, id)).toBeNull(); + expect(getCodexOauthAuth({ ...config, codexOauthDefaultAccountId: "../bad" })).toBeNull(); + }); + it("filters malformed slots without accepting a duplicate legacy slot", () => { const config = { codexOauth: legacy, diff --git a/src/node/utils/codexOauthAuth.ts b/src/node/utils/codexOauthAuth.ts index f894af5e187..bd662211d42 100644 --- a/src/node/utils/codexOauthAuth.ts +++ b/src/node/utils/codexOauthAuth.ts @@ -5,7 +5,12 @@ * extract non-sensitive claims (e.g. ChatGPT-Account-Id) from OAuth responses. */ -import { CODEX_OAUTH_DEFAULT_ACCOUNT_ID } from "@/common/constants/codexOauthAccounts"; +import { + CODEX_OAUTH_DEFAULT_ACCOUNT_ID, + CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH, + CODEX_OAUTH_ACCOUNT_ID_PATTERN, + CODEX_OAUTH_RESERVED_ACCOUNT_IDS, +} from "@/common/constants/codexOauthAccounts"; export interface CodexOauthAuth { type: "oauth"; @@ -50,6 +55,15 @@ export function parseCodexOauthAuth(value: unknown): CodexOauthAuth | null { return { type: "oauth", access, refresh, expires, accountId }; } +/** Validate local slot IDs at input and storage boundaries. */ +export function isValidCodexOauthAccountId(accountId: string): boolean { + return ( + accountId.length <= CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH && + CODEX_OAUTH_ACCOUNT_ID_PATTERN.test(accountId) && + !CODEX_OAUTH_RESERVED_ACCOUNT_IDS.has(accountId) + ); +} + /** Read connected slots from an OpenAI provider config. */ export function getCodexOauthAccounts(config: unknown): Array<{ id: string; @@ -72,7 +86,12 @@ export function getCodexOauthAccounts(config: unknown): Array<{ if (isPlainObject(config.codexOauthAccounts)) { for (const [id, entry] of Object.entries(config.codexOauthAccounts)) { // The legacy slot owns this ID, even if malformed disk data repeats it. - if (id === CODEX_OAUTH_DEFAULT_ACCOUNT_ID || !isPlainObject(entry)) continue; + if ( + id === CODEX_OAUTH_DEFAULT_ACCOUNT_ID || + !isValidCodexOauthAccountId(id) || + !isPlainObject(entry) + ) + continue; const auth = parseCodexOauthAuth(entry.auth); if (!auth || typeof entry.label !== "string" || !entry.label.trim()) continue; accounts.push({ id, label: entry.label.trim(), auth }); From 0219f200db0b72aaa1ac9cb6370d7dbb7e7ede60 Mon Sep 17 00:00:00 2001 From: Mux Date: Sat, 5 Sep 2026 21:01:28 -0500 Subject: [PATCH 03/24] =?UTF-8?q?[openai]=20=F0=9F=A4=96=20fix:=20preserve?= =?UTF-8?q?=20account=20identity=20across=20processes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- _Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `high` • Cost: `$72.93`_ --- src/cli/trust.test.ts | 31 ++++ src/cli/trust.ts | 12 ++ src/node/services/codexOauthService.test.ts | 194 ++++++++++++++++++++ src/node/services/codexOauthService.ts | 115 ++++++++---- src/node/services/providerService.test.ts | 14 ++ src/node/services/providerService.ts | 10 +- src/node/utils/codexOauthAuth.test.ts | 10 + src/node/utils/codexOauthAuth.ts | 18 +- tests/ui/agents/thinkingSelector.test.ts | 6 +- 9 files changed, 375 insertions(+), 35 deletions(-) diff --git a/src/cli/trust.test.ts b/src/cli/trust.test.ts index 73aee8f156b..467ff12aee6 100644 --- a/src/cli/trust.test.ts +++ b/src/cli/trust.test.ts @@ -153,6 +153,37 @@ describe("xum trust CLI", () => { ).toBeUndefined(); }); + test.each(["work", undefined])( + "uses the deepest registered account scope: %s", + async (accountId) => { + using tmp = new DisposableTempDir("codex-nested-account"); + const real = new Config(path.join(tmp.path, "real")); + const target = new Config(path.join(tmp.path, "target")); + const root = path.join(tmp.path, "project"); + const subproject = path.join(root, "packages", "api"); + const targetProject = path.join(tmp.path, "cli-project"); + await real.editConfig((config) => { + config.projects.set(subproject, { workspaces: [], codexOauthAccountId: accountId }); + config.projects.set(root, { workspaces: [], codexOauthAccountId: "personal" }); + return config; + }); + await materializeCodexOauthAccount(real, target, path.join(subproject, "src"), targetProject); + expect(target.loadConfigOrDefault().projects.get(targetProject)?.codexOauthAccountId).toBe( + accountId + ); + + await materializeCodexOauthAccount( + real, + target, + path.join(root, "packages", "api-other", "src"), + targetProject + ); + expect(target.loadConfigOrDefault().projects.get(targetProject)?.codexOauthAccountId).toBe( + "personal" + ); + } + ); + test.each(["work", undefined])( "rejects a lost account selection write: %s", async (accountId) => { diff --git a/src/cli/trust.ts b/src/cli/trust.ts index 2325cad7284..22d37c7a6dc 100644 --- a/src/cli/trust.ts +++ b/src/cli/trust.ts @@ -208,6 +208,18 @@ export async function materializeCodexOauthAccount( } } } + if (sourcePath === undefined) { + // An explicit directory can sit below a registered subproject. Keep its account scope. + for (const projectPath of projects.keys()) { + const relative = path.relative(projectPath, projectDir); + if (relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) { + continue; + } + if (sourcePath === undefined || projectPath.length > sourcePath.length) { + sourcePath = projectPath; + } + } + } sourcePath ??= (await findMainRepoDir(projectDir)) ?? (await findGitRoot(projectDir)) ?? projectDir; const accountId = projects.get(sourcePath)?.codexOauthAccountId; diff --git a/src/node/services/codexOauthService.test.ts b/src/node/services/codexOauthService.test.ts index 5f62e82148b..8c3703f193c 100644 --- a/src/node/services/codexOauthService.test.ts +++ b/src/node/services/codexOauthService.test.ts @@ -1,5 +1,6 @@ import { Config, FileLeaseManager, ProvidersConfigStore } from "@/node/config"; import * as fs from "fs"; +import http from "node:http"; import * as os from "os"; import * as path from "path"; import { describe, it, expect, beforeEach, afterEach, spyOn } from "bun:test"; @@ -38,6 +39,7 @@ function fakeJwt(claims: Record): string { function validAuth(overrides?: Partial): CodexOauthAuth { return { type: "oauth", + credentialId: "1c9c50b0-d777-4dd2-998c-09c156ba9754", access: fakeJwt({ sub: "user" }), refresh: "rt_test", expires: Date.now() + 3_600_000, // 1h from now @@ -1002,6 +1004,198 @@ describe("CodexOauthService", () => { expect(getCodexOauthAuth(deps.providersConfig.openai)?.access).toBe("access-device-2"); }); + function sharedServices(auth: CodexOauthAuth) { + const provider = new ProviderService(new Config(deps.rootDir)); + const store = provider.providersConfigStore; + store.saveProvidersConfig({ + openai: { codexOauthAccounts: { work: { label: "Work", auth } } }, + }); + return { + provider, + store, + first: new CodexOauthService(store, provider), + second: new CodexOauthService( + new ProvidersConfigStore(deps.rootDir), + new ProviderService(new Config(deps.rootDir)) + ), + }; + } + + it("completes reconnect after another service refreshes the same credential", async () => { + const initial = expiredAuth(); + const { store, first, second } = sharedServices(initial); + deviceFetch((init) => + Promise.resolve( + mockRefreshResponse({ + access_token: + new URLSearchParams(requestBody(init)).get("grant_type") === "refresh_token" + ? "rotated-access" + : "login-access", + refresh_token: "new-refresh", + expires_in: 3600, + }) + ) + ); + try { + const flow = await first.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + const refreshed = await second.getValidAuth("work"); + expect(refreshed.success).toBe(true); + expect(getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work")?.credentialId).toBe( + initial.credentialId + ); + expect(await first.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + const reconnected = getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work"); + expect(reconnected?.access).toBe("login-access"); + expect(reconnected?.credentialId).toBeDefined(); + expect(reconnected?.credentialId).not.toBe(initial.credentialId); + } finally { + await first.dispose(); + await second.dispose(); + } + }); + + it("lets the first completed cross-process reconnect invalidate the other login", async () => { + const { store, first, second } = sharedServices(validAuth()); + deviceFetch(); + try { + const older = await first.startDeviceFlow({ accountId: "work" }); + const newer = await second.startDeviceFlow({ accountId: "work" }); + if (!older.success || !newer.success) throw new Error("Login start failed"); + expect(await first.waitForDeviceFlow(older.data.flowId)).toEqual(Ok(undefined)); + const winner = getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work"); + expect((await second.waitForDeviceFlow(newer.data.flowId)).success).toBe(false); + expect(getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work")).toEqual(winner); + } finally { + await first.dispose(); + await second.dispose(); + } + }); + + it.each(["removed", "recreated", "older-client"])( + "rejects stale login after external %s credentials", + async (change) => { + const initial = validAuth(); + const { provider, store, first, second } = sharedServices(initial); + deviceFetch(); + try { + const flow = await first.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + if (change !== "older-client") { + await provider.setConfigValue("openai", ["codexOauthAccounts", "work"], undefined); + } + if (change !== "removed") { + await provider.setConfigValue("openai", ["codexOauthAccounts", "work"], { + label: "Work", + auth: { + ...initial, + credentialId: + change === "recreated" ? "50e00a32-b964-4ce2-b131-6b53356ce2db" : undefined, + }, + }); + } + const current = store.loadProvidersConfig()?.openai; + expect((await first.waitForDeviceFlow(flow.data.flowId)).success).toBe(false); + expect(store.loadProvidersConfig()?.openai).toEqual(current); + } finally { + await first.dispose(); + await second.dispose(); + } + } + ); + + it("waits for an old credential refresh before assigning its durable login ID", async () => { + const initial = expiredAuth({ credentialId: undefined }); + const { store, provider, first, second } = sharedServices(initial); + const leaseAttempt = createDeferred(); + class ObservedLeaseManager extends FileLeaseManager { + override withCodexOauthRefreshLock( + accountId: string, + fn: () => Promise | T + ): Promise { + leaseAttempt.resolve(undefined); + return super.withCodexOauthRefreshLock(accountId, fn); + } + } + const reconnectService = new CodexOauthService( + store, + provider, + undefined, + new ObservedLeaseManager(deps.rootDir) + ); + const refreshStarted = createDeferred(); + const response = createDeferred(); + deviceFetch(() => { + refreshStarted.resolve(undefined); + return response.promise; + }); + try { + const refresh = second.getValidAuth("work"); + await refreshStarted.promise; + const startup = reconnectService.startDeviceFlow({ accountId: "work" }); + await leaseAttempt.promise; + expect( + getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work")?.credentialId + ).toBeUndefined(); + response.resolve( + mockRefreshResponse({ + access_token: "rotated-access", + refresh_token: "rotated-refresh", + expires_in: 3600, + }) + ); + expect((await refresh).success).toBe(true); + const flow = await startup; + if (!flow.success) throw new Error(flow.error); + const stamped = getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work"); + expect(stamped?.access).toBe("rotated-access"); + expect(stamped?.refresh).toBe("rotated-refresh"); + expect(stamped?.credentialId).toBeDefined(); + await reconnectService.cancelDeviceFlow(flow.data.flowId); + expect(await first.getValidAuth("work")).toEqual(Ok(stamped!)); + } finally { + await reconnectService.dispose(); + await first.dispose(); + await second.dispose(); + } + }); + + it("keeps the active device login when a newer device startup fails", async () => { + deps.providersConfig = { openai: { codexOauth: validAuth() } }; + deviceFetch(); + const first = await service.startDeviceFlow({ accountId: "default" }); + if (!first.success) throw new Error(first.error); + const workingFetch = globalThis.fetch; + mockFetch((input, init) => + input === CODEX_OAUTH_DEVICE_USERCODE_URL + ? Promise.reject(new Error("Device startup failed")) + : workingFetch(input, init) + ); + expect((await service.startDeviceFlow({ accountId: "default" })).success).toBe(false); + expect(await service.waitForDeviceFlow(first.data.flowId)).toEqual(Ok(undefined)); + }); + + it("keeps the active desktop login when the new listener cannot start", async () => { + deps.providersConfig = { openai: { codexOauth: validAuth() } }; + deviceFetch(); + const first = await service.startDesktopFlow({ accountId: "default" }); + if (!first.success) throw new Error(first.error); + const createServer = spyOn(http, "createServer").mockImplementationOnce(() => { + throw new Error("Callback listener unavailable"); + }); + try { + expect((await service.startDesktopFlow({ accountId: "default" })).success).toBe(false); + } finally { + createServer.mockRestore(); + } + const response = await originalFetch( + "http://localhost:1455/auth/callback?code=code&state=" + first.data.flowId + ); + await response.text(); + expect(response.ok).toBe(true); + expect(await service.waitForDesktopFlow(first.data.flowId)).toEqual(Ok(undefined)); + }); + it("completes a reconnect after a concurrent refresh persists first", async () => { deps.providersConfig = { openai: { diff --git a/src/node/services/codexOauthService.ts b/src/node/services/codexOauthService.ts index 79da534a620..fd0a0ddd84f 100644 --- a/src/node/services/codexOauthService.ts +++ b/src/node/services/codexOauthService.ts @@ -63,6 +63,7 @@ export interface CodexOauthLoginOptions { interface AccountSelection { accountId: string; revision: number; + credentialId?: string; auth: CodexOauthAuth | null; label?: string; selectAsDefault: boolean; @@ -151,7 +152,8 @@ function matchesAuth(actual: CodexOauthAuth | null, expected: CodexOauthAuth | n actual.access === expected.access && actual.refresh === expected.refresh && actual.expires === expected.expires && - actual.accountId === expected.accountId + actual.accountId === expected.accountId && + actual.credentialId === expected.credentialId ); } @@ -289,6 +291,8 @@ export class CodexOauthService { }, DEFAULT_DESKTOP_TIMEOUT_MS), }); + self.loginSelections.set(destination.accountId, destination); + const authorizeUrl = buildCodexAuthorizeUrl({ redirectUri, state: flowId, @@ -478,6 +482,8 @@ export class CodexOauthService { settled: false, }); + self.loginSelections.set(destination.accountId, destination); + log.debug(`[Codex OAuth] Device flow started (flowId=${flowId})`); return { flowId, userCode, verifyUrl, intervalSeconds }; @@ -719,43 +725,96 @@ export class CodexOauthService { ); } + private initializeCredentialId( + accountId: string + ): Effect.Effect { + return Effect.tryPromise({ + try: () => + this.fileLeaseManager.withCodexOauthRefreshLock(accountId, async () => { + let selected: CodexOauthAuth | null = null; + const result = await this.providerService.updateProviderSection( + "openai", + (section) => { + const current = getCodexOauthAuth(section, accountId); + if (!current) return null; + selected = current; + if (current.credentialId) return null; + selected = { ...current, credentialId: crypto.randomUUID() }; + const next = { ...section }; + if (accountId === CODEX_OAUTH_DEFAULT_ACCOUNT_ID) { + next.codexOauth = selected; + } else { + const accounts = isPlainObject(section?.codexOauthAccounts) + ? section.codexOauthAccounts + : {}; + const entry = accounts[accountId]; + next.codexOauthAccounts = { + ...accounts, + [accountId]: { ...(isPlainObject(entry) ? entry : {}), auth: selected }, + }; + } + return { value: next }; + }, + { enforcePolicy: true } + ); + if (!result.success) throw new Error(result.error); + if (!selected) throw new Error("Codex OAuth account is not configured"); + return selected; + }), + catch: (error) => new CodexOauthError({ reason: getErrorMessage(error) }), + }); + } + private selectLoginDestination( options?: CodexOauthLoginOptions ): Effect.Effect { - return Effect.suspend(() => { + // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect generators do not inherit this. + const self = this; + return Effect.gen(function* () { if (options?.accountId !== undefined && options.label !== undefined) { - return Effect.fail( + return yield* Effect.fail( new CodexOauthError({ reason: "Specify an account ID or a label, not both" }) ); } if (options?.label !== undefined && !isValidLabel(options.label)) { - return Effect.fail(new CodexOauthError({ reason: "Invalid Codex OAuth account label" })); + return yield* Effect.fail( + new CodexOauthError({ reason: "Invalid Codex OAuth account label" }) + ); } const accountId = options?.accountId ?? (options?.label !== undefined ? crypto.randomUUID() : CODEX_OAUTH_DEFAULT_ACCOUNT_ID); if (!isValidCodexOauthAccountId(accountId)) { - return Effect.fail(new CodexOauthError({ reason: "Invalid Codex OAuth account ID" })); + return yield* Effect.fail( + new CodexOauthError({ reason: "Invalid Codex OAuth account ID" }) + ); } - const auth = this.readStoredAuth(accountId); + let auth = self.readStoredAuth(accountId); if (options?.accountId !== undefined && !auth) { - return Effect.fail( + return yield* Effect.fail( new CodexOauthError({ reason: "Codex OAuth account is not configured" }) ); } - // A failed or cancelled login must not discard a pending token rotation. - const revision = this.getAccountRevision(accountId); - const selection: AccountSelection = { + const revision = self.getAccountRevision(accountId); + if (auth && !auth.credentialId) { + // Stamp old credentials under the refresh lease. Stamping during rotation can discard the rotated token. + auth = yield* self.initializeCredentialId(accountId); + } + if (self.getAccountRevision(accountId) !== revision) { + return yield* Effect.fail( + new CodexOauthError({ reason: "Codex OAuth account changed during login startup" }) + ); + } + return { accountId, revision, + credentialId: auth?.credentialId, auth, label: options?.label?.trim(), selectAsDefault: options?.label !== undefined && - getCodexOauthAccounts(this.readOpenaiConfig()).length === 0, + getCodexOauthAccounts(self.readOpenaiConfig()).length === 0, }; - this.loginSelections.set(accountId, selection); - return Effect.succeed(selection); }); } @@ -778,21 +837,7 @@ export class CodexOauthService { return null; if (legacy || auth === undefined) return { value: auth }; return { value: { ...(isPlainObject(current) ? current : {}), auth } }; - }).pipe( - Effect.map((result) => { - const login = this.loginSelections.get(selection.accountId); - // A local refresh changes tokens, not the identity of an in-progress interactive login. - if ( - result.success && - auth && - login?.revision === selection.revision && - matchesAuth(login.auth, selection.auth) - ) { - login.auth = auth; - } - return result; - }) - ) + }) ); } @@ -801,6 +846,7 @@ export class CodexOauthService { auth: CodexOauthAuth, isActive: () => boolean ): Effect.Effect> { + const nextAuth = { ...auth, credentialId: crypto.randomUUID() }; return this.withAccountMutationEffect( selection.accountId, this.configMutationEffect(() => @@ -823,10 +869,16 @@ export class CodexOauthService { const stored = parseCodexOauthAuth( legacy ? current.codexOauth : isPlainObject(entry) ? entry.auth : undefined ); - if (!matchesAuth(stored, selection.auth)) return null; + // Token rotation preserves the login ID. Deletion, replacement, or an older writer cannot match it. + if ( + selection.credentialId === undefined + ? stored !== null + : stored?.credentialId !== selection.credentialId + ) + return null; const next = { ...current }; if (legacy) { - next.codexOauth = auth; + next.codexOauth = nextAuth; } else { // Do not mirror named credentials into the legacy slot. // Older versions refresh that slot independently and cannot honor project selection. @@ -836,7 +888,7 @@ export class CodexOauthService { [selection.accountId]: { ...(isPlainObject(entry) ? entry : {}), label: isPlainObject(entry) ? entry.label : selection.label, - auth, + auth: nextAuth, }, }; // Commit the first slot and its selection together. A failed write must leave neither field. @@ -1072,6 +1124,7 @@ export class CodexOauthService { const next: CodexOauthAuth = { type: "oauth", + credentialId: current.credentialId, access: accessToken, refresh: refreshToken ?? current.refresh, expires: Date.now() + Math.max(0, Math.floor(expiresIn * 1000)), diff --git a/src/node/services/providerService.test.ts b/src/node/services/providerService.test.ts index f0d64319049..9e83256bb31 100644 --- a/src/node/services/providerService.test.ts +++ b/src/node/services/providerService.test.ts @@ -7,6 +7,8 @@ import { writeFile } from "node:fs/promises"; import * as os from "os"; import * as path from "path"; import { CUSTOM_PROVIDER_TYPES } from "@/common/utils/providers/customProviders"; +import { KNOWN_MODELS } from "@/common/constants/knownModels"; +import { resolveCodexOauthRouting } from "@/common/utils/providers/codexOauthRouting"; import type { ProviderModelEntry } from "@/common/orpc/types"; import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults"; import { Config } from "@/node/config"; @@ -345,6 +347,18 @@ describe("ProviderService.getConfig", () => { }); }); + it("preserves implicit and explicit account selections for browser routing", () => { + withTempConfig((config, service) => { + saveOpenAIConfig(config); + expect(resolveCodexOauthRouting(KNOWN_MODELS.GPT.id, service.getConfig())).toBe("other"); + + saveOpenAIConfig(config, { codexOauthDefaultAccountId: "default" }); + expect(resolveCodexOauthRouting(KNOWN_MODELS.GPT.id, service.getConfig())).toBe( + "missing-account" + ); + }); + }); + it("reports named accounts as connected without legacy credentials", () => { withTempConfig((config, service) => { new ProvidersConfigStore(config.rootDir).saveProvidersConfig({ diff --git a/src/node/services/providerService.ts b/src/node/services/providerService.ts index 770ac720138..bd8a22ad1e9 100644 --- a/src/node/services/providerService.ts +++ b/src/node/services/providerService.ts @@ -63,7 +63,7 @@ import { isProviderAutoRouteEligible, resolveProviderCredentials, } from "@/node/utils/providerRequirements"; -import { getCodexOauthAccounts, getCodexOauthAccountId } from "@/node/utils/codexOauthAuth"; +import { getCodexOauthAccounts } from "@/node/utils/codexOauthAuth"; import { normalizeCoderDeploymentUrl, parseCoderGatewayProviders, @@ -510,7 +510,13 @@ export class ProviderService { id, label, })); - providerInfo.codexOauthDefaultAccountId = getCodexOauthAccountId(config); + // Preserve an unset selection. A synthetic default would block API-key-only routing in the renderer. + if ( + "codexOauthDefaultAccountId" in config && + typeof config.codexOauthDefaultAccountId === "string" + ) { + providerInfo.codexOauthDefaultAccountId = config.codexOauthDefaultAccountId; + } const codexOauthDefaultAuth = config.codexOauthDefaultAuth; if (codexOauthDefaultAuth === "oauth" || codexOauthDefaultAuth === "apiKey") { diff --git a/src/node/utils/codexOauthAuth.test.ts b/src/node/utils/codexOauthAuth.test.ts index c2099cb6630..2c48635cf96 100644 --- a/src/node/utils/codexOauthAuth.test.ts +++ b/src/node/utils/codexOauthAuth.test.ts @@ -51,6 +51,16 @@ describe("parseCodexOauthAuth", () => { expect(result).toEqual(input); }); + it("accepts missing login IDs but rejects malformed IDs", () => { + const auth = { type: "oauth", access: "access", refresh: "refresh", expires: 1000 }; + expect(parseCodexOauthAuth(auth)).not.toBeNull(); + const credentialId = "1c9c50b0-d777-4dd2-998c-09c156ba9754"; + expect(parseCodexOauthAuth({ ...auth, credentialId })?.credentialId).toBe(credentialId); + for (const invalid of [null, "", "not-a-uuid", 42]) { + expect(parseCodexOauthAuth({ ...auth, credentialId: invalid })).toBeNull(); + } + }); + it("returns null for non-object values", () => { expect(parseCodexOauthAuth(null)).toBeNull(); expect(parseCodexOauthAuth(undefined)).toBeNull(); diff --git a/src/node/utils/codexOauthAuth.ts b/src/node/utils/codexOauthAuth.ts index bd662211d42..0f7c4fa29eb 100644 --- a/src/node/utils/codexOauthAuth.ts +++ b/src/node/utils/codexOauthAuth.ts @@ -5,6 +5,8 @@ * extract non-sensitive claims (e.g. ChatGPT-Account-Id) from OAuth responses. */ +import { z } from "zod"; + import { CODEX_OAUTH_DEFAULT_ACCOUNT_ID, CODEX_OAUTH_ACCOUNT_ID_MAX_LENGTH, @@ -12,8 +14,12 @@ import { CODEX_OAUTH_RESERVED_ACCOUNT_IDS, } from "@/common/constants/codexOauthAccounts"; +const credentialIdSchema = z.string().uuid().optional(); + export interface CodexOauthAuth { type: "oauth"; + /** Identifies this login across token rotations and processes. */ + credentialId?: string; /** OAuth access token (JWT). */ access: string; /** OAuth refresh token. */ @@ -42,6 +48,7 @@ export function parseCodexOauthAuth(value: unknown): CodexOauthAuth | null { const refresh = value.refresh; const expires = value.expires; const accountId = value.accountId; + const credentialId = credentialIdSchema.safeParse(value.credentialId); if (type !== "oauth") return null; if (typeof access !== "string" || !access) return null; @@ -52,7 +59,16 @@ export function parseCodexOauthAuth(value: unknown): CodexOauthAuth | null { if (typeof accountId !== "string" || !accountId) return null; } - return { type: "oauth", access, refresh, expires, accountId }; + if (!credentialId.success) return null; + + return { + type: "oauth", + access, + refresh, + expires, + accountId, + credentialId: credentialId.data, + }; } /** Validate local slot IDs at input and storage boundaries. */ diff --git a/tests/ui/agents/thinkingSelector.test.ts b/tests/ui/agents/thinkingSelector.test.ts index ed9a6108285..d966dc9c8e7 100644 --- a/tests/ui/agents/thinkingSelector.test.ts +++ b/tests/ui/agents/thinkingSelector.test.ts @@ -95,7 +95,11 @@ describeIntegration("Thinking selector", () => { const harness = await createAppHarness({ branchPrefix: "thinking-selector", beforeRenderEnvironment: async (env) => { - await setupProviders(env, { xai: { apiKey: "dummy" } }); + // Pro and Fast controls require a usable direct API route. + await setupProviders(env, { + openai: { apiKey: "dummy" }, + xai: { apiKey: "dummy" }, + }); }, }); From 5262cbab7856d7386e5be3fc17fb0a040f1e17a2 Mon Sep 17 00:00:00 2001 From: Mux Date: Sat, 5 Sep 2026 21:17:25 -0500 Subject: [PATCH 04/24] =?UTF-8?q?[openai]=20=F0=9F=A4=96=20fix:=20preserve?= =?UTF-8?q?=20reconnect=20identity=20after=20rejected=20refreshes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- _Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `high` • Cost: `$80.74`_ --- src/node/services/codexOauthService.test.ts | 317 ++++++++++++++++---- src/node/services/codexOauthService.ts | 105 ++++--- src/node/utils/codexOauthAuth.test.ts | 10 + src/node/utils/codexOauthAuth.ts | 7 +- 4 files changed, 333 insertions(+), 106 deletions(-) diff --git a/src/node/services/codexOauthService.test.ts b/src/node/services/codexOauthService.test.ts index 8c3703f193c..726ed92ccd0 100644 --- a/src/node/services/codexOauthService.test.ts +++ b/src/node/services/codexOauthService.test.ts @@ -291,73 +291,71 @@ describe("CodexOauthService", () => { // Invalid grant cleanup // ------------------------------------------------------------------------- - describe("invalid grant cleanup", () => { - it("calls disconnect + clears stored auth on invalid_grant response", async () => { - const expired = expiredAuth(); - deps.providersConfig = { openai: { codexOauth: expired } }; - - mockFetch(() => - Promise.resolve( - new Response(JSON.stringify({ error: "invalid_grant" }), { - status: 400, - headers: { "Content-Type": "application/json" }, - }) - ) - ); - - const result = await service.getValidAuth(); - expect(result.success).toBe(false); - - // Should have called setConfigValue to clear auth (disconnect) - const clearCall = deps.setConfigValueCalls.find( - (c) => c.provider === "openai" && c.keyPath[0] === "codexOauth" && c.value === undefined - ); - expect(clearCall).toBeDefined(); - }); - - it("clears auth when error text contains 'revoked'", async () => { - const expired = expiredAuth(); - deps.providersConfig = { openai: { codexOauth: expired } }; - - mockFetch(() => - Promise.resolve( - new Response("Token has been revoked", { - status: 401, - }) - ) - ); - - const result = await service.getValidAuth(); - expect(result.success).toBe(false); - - const clearCall = deps.setConfigValueCalls.find( - (c) => c.provider === "openai" && c.keyPath[0] === "codexOauth" && c.value === undefined - ); - expect(clearCall).toBeDefined(); - }); - - it("subsequent getValidAuth returns error after invalid_grant cleanup", async () => { - const expired = expiredAuth(); - deps.providersConfig = { openai: { codexOauth: expired } }; - - mockFetch(() => - Promise.resolve( - new Response(JSON.stringify({ error: "invalid_grant" }), { - status: 400, - headers: { "Content-Type": "application/json" }, - }) - ) - ); - - // First call triggers disconnect - await service.getValidAuth(); + describe("invalid grant marking", () => { + it.each(["invalid_grant", "Token has been revoked"])( + "retains credential identity after %s", + async (error) => { + const expired = expiredAuth(); + deps.providersConfig = { openai: { codexOauth: expired } }; + mockFetch(() => Promise.resolve(mockRefreshResponse({ error }, 400))); + expect((await service.getValidAuth()).success).toBe(false); + expect(getCodexOauthAuth(deps.providersConfig.openai)).toEqual({ + ...expired, + invalidReason: "invalid_grant", + }); + expect(getCodexOauthAccounts(deps.providersConfig.openai)).toHaveLength(1); + } + ); - // Second call should see no stored auth - const result = await service.getValidAuth(); - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("not configured"); + it.each([false, true])( + "never returns or refreshes marked credentials (expired=%s)", + async (expired) => { + const auth = expired + ? expiredAuth({ invalidReason: "invalid_grant" }) + : validAuth({ invalidReason: "invalid_grant" }); + const legacy = validAuth({ access: "other" }); + deps.providersConfig = { + openai: { + codexOauth: legacy, + codexOauthAccounts: { work: { label: "Work", auth } }, + codexOauthDefaultAccountId: "work", + }, + }; + let fetchCount = 0; + mockFetch(() => { + fetchCount++; + return Promise.reject(new Error("Must not refresh")); + }); + expect((await service.getValidAuth()).success).toBe(false); + expect((await service.getValidAuth("work")).success).toBe(false); + expect(await service.getValidAuth("default")).toEqual(Ok(legacy)); + expect(fetchCount).toBe(0); + expect(getCodexOauthAccounts(deps.providersConfig.openai)).toHaveLength(2); + expect(await service.disconnect("work")).toEqual(Ok(undefined)); + expect(getCodexOauthAuth(deps.providersConfig.openai, "work")).toBeNull(); } + ); + + it("rejects a marker after the local refresh mutex without refreshing again", async () => { + deps.providersConfig = { openai: { codexOauth: expiredAuth() } }; + const started = createDeferred(); + const response = createDeferred(); + let fetchCount = 0; + mockFetch(() => { + fetchCount++; + started.resolve(undefined); + return response.promise; + }); + const first = service.getValidAuth(); + await started.promise; + const second = service.getValidAuth(); + response.resolve(mockRefreshResponse({ error: "invalid_grant" }, 400)); + expect((await first).success).toBe(false); + expect((await second).success).toBe(false); + expect((await service.getValidAuth()).success).toBe(false); + expect(fetchCount).toBe(1); + expect(await service.disconnect()).toEqual(Ok(undefined)); + expect(getCodexOauthAuth(deps.providersConfig.openai)).toBeNull(); }); }); @@ -557,7 +555,7 @@ describe("CodexOauthService", () => { expect(deps.setConfigValueCalls).toHaveLength(0); }); - it("enforces policy when a revoked refresh attempts to clear credentials", async () => { + it("enforces policy when a revoked refresh attempts to mark credentials", async () => { const stored = expiredAuth(); deps.providersConfig = { openai: { codexOauth: stored } }; deps.policyDenied = true; @@ -587,6 +585,102 @@ describe("CodexOauthService", () => { }); describe("account refresh isolation", () => { + it.each([ + { change: "rotation", accepted: true }, + { change: "replacement", accepted: false }, + { change: "expired replacement", accepted: false }, + { change: "dropped ID", accepted: false }, + { change: "legacy rotation", accepted: true }, + { change: "legacy stamp", accepted: false }, + { change: "marked", accepted: false }, + { change: "removed", accepted: false }, + ])("checks the pinned credential after lease wait: $change", async ({ change, accepted }) => { + const initial = expiredAuth(change.startsWith("legacy") ? { credentialId: undefined } : {}); + const latest = validAuth({ + ...initial, + access: "updated", + expires: change === "expired replacement" ? Date.now() - 1000 : Date.now() + 3600000, + credentialId: + change.includes("replacement") || change === "legacy stamp" + ? "50e00a32-b964-4ce2-b131-6b53356ce2db" + : change === "dropped ID" + ? undefined + : initial.credentialId, + invalidReason: change === "marked" ? "invalid_grant" : undefined, + }); + const provider = new ProviderService(new Config(deps.rootDir)); + const store = provider.providersConfigStore; + store.saveProvidersConfig({ openai: { codexOauth: initial } }); + const entered = createDeferred(); + const release = createDeferred(); + const attempted = createDeferred(); + const holder = new FileLeaseManager(deps.rootDir).withCodexOauthRefreshLock( + "default", + async () => { + entered.resolve(undefined); + await release.promise; + } + ); + await entered.promise; + class ObservedLeaseManager extends FileLeaseManager { + override withCodexOauthRefreshLock( + accountId: string, + fn: () => Promise | T + ): Promise { + attempted.resolve(undefined); + return super.withCodexOauthRefreshLock(accountId, fn); + } + } + const waiting = new CodexOauthService( + store, + provider, + undefined, + new ObservedLeaseManager(deps.rootDir) + ); + let fetchCount = 0; + mockFetch(() => { + fetchCount++; + return Promise.reject(new Error("Unexpected refresh")); + }); + try { + const pending = waiting.getValidAuth(); + await attempted.promise; + store.saveProvidersConfig({ openai: change === "removed" ? {} : { codexOauth: latest } }); + release.resolve(undefined); + await holder; + const result = await pending; + expect(result.success).toBe(accepted); + if (accepted) expect(result).toEqual(Ok(latest)); + expect(fetchCount).toBe(0); + } finally { + release.resolve(undefined); + await holder; + await waiting.dispose(); + } + }); + + it("does not adopt a replaced credential after waiting for the local mutex", async () => { + deps.providersConfig = { openai: { codexOauth: expiredAuth() } }; + const started = createDeferred(); + const response = createDeferred(); + let fetchCount = 0; + mockFetch(() => { + fetchCount++; + started.resolve(undefined); + return response.promise; + }); + const first = service.getValidAuth(); + await started.promise; + const waiting = service.getValidAuth(); + deps.providersConfig.openai.codexOauth = validAuth({ + credentialId: "50e00a32-b964-4ce2-b131-6b53356ce2db", + }); + response.resolve(mockRefreshResponse({ access_token: "old-rotation", expires_in: 3600 })); + expect((await first).success).toBe(false); + expect((await waiting).success).toBe(false); + expect(fetchCount).toBe(1); + }); + it("shares a refresh lease across service instances and persists the rotated token", async () => { const store = new ProvidersConfigStore(deps.rootDir); store.saveProvidersConfig({ @@ -1021,6 +1115,97 @@ describe("CodexOauthService", () => { }; } + it("reconnects a marked legacy credential that has no durable ID", async () => { + deps.providersConfig = { + openai: { + codexOauth: expiredAuth({ credentialId: undefined, invalidReason: "invalid_grant" }), + }, + }; + deviceFetch(); + const flow = await service.startDeviceFlow({ accountId: "default" }); + if (!flow.success) throw new Error(flow.error); + const stamped = getCodexOauthAuth(deps.providersConfig.openai); + expect(stamped?.credentialId).toBeDefined(); + expect(stamped?.invalidReason).toBe("invalid_grant"); + expect(await service.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + const reconnected = await service.getValidAuth(); + expect(reconnected.success).toBe(true); + if (reconnected.success) { + expect(reconnected.data.invalidReason).toBeUndefined(); + expect(reconnected.data.credentialId).not.toBe(stamped?.credentialId); + } + }); + + it("completes cross-process reconnect over the same credential's invalid marker", async () => { + const initial = expiredAuth(); + const { store, first, second } = sharedServices(initial); + deviceFetch((init) => + Promise.resolve( + new URLSearchParams(requestBody(init)).get("grant_type") === "refresh_token" + ? mockRefreshResponse({ error: "invalid_grant" }, 400) + : mockRefreshResponse({ + access_token: "reconnected", + refresh_token: "new-login", + expires_in: 3600, + }) + ) + ); + try { + const flow = await first.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + expect((await second.getValidAuth("work")).success).toBe(false); + expect(getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work")).toEqual({ + ...initial, + invalidReason: "invalid_grant", + }); + expect(await first.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + const result = await first.getValidAuth("work"); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.credentialId).not.toBe(initial.credentialId); + expect(result.data.invalidReason).toBeUndefined(); + expect(result.data.access).toBe("reconnected"); + } + } finally { + await first.dispose(); + await second.dispose(); + } + }); + + it("does not invalidate a completed reconnect when an older process refresh fails", async () => { + const { store, first, second } = sharedServices(expiredAuth()); + const started = createDeferred(); + const response = createDeferred(); + deviceFetch((init) => { + if (new URLSearchParams(requestBody(init)).get("grant_type") === "refresh_token") { + started.resolve(undefined); + return response.promise; + } + return Promise.resolve( + mockRefreshResponse({ + access_token: "reconnected", + refresh_token: "new-login", + expires_in: 3600, + }) + ); + }); + try { + const refresh = second.getValidAuth("work"); + await started.promise; + const flow = await first.startDeviceFlow({ accountId: "work" }); + if (!flow.success) throw new Error(flow.error); + expect(await first.waitForDeviceFlow(flow.data.flowId)).toEqual(Ok(undefined)); + const reconnected = getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work"); + response.resolve(mockRefreshResponse({ error: "invalid_grant" }, 400)); + expect((await refresh).success).toBe(false); + expect(getCodexOauthAuth(store.loadProvidersConfig()?.openai, "work")).toEqual(reconnected); + expect((await first.getValidAuth("work")).success).toBe(true); + } finally { + await first.dispose(); + await second.dispose(); + } + }); + it("completes reconnect after another service refreshes the same credential", async () => { const initial = expiredAuth(); const { store, first, second } = sharedServices(initial); diff --git a/src/node/services/codexOauthService.ts b/src/node/services/codexOauthService.ts index fd0a0ddd84f..684360154b1 100644 --- a/src/node/services/codexOauthService.ts +++ b/src/node/services/codexOauthService.ts @@ -153,7 +153,8 @@ function matchesAuth(actual: CodexOauthAuth | null, expected: CodexOauthAuth | n actual.refresh === expected.refresh && actual.expires === expected.expires && actual.accountId === expected.accountId && - actual.credentialId === expected.credentialId + actual.credentialId === expected.credentialId && + actual.invalidReason === expected.invalidReason ); } @@ -593,13 +594,17 @@ export class CodexOauthService { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect generators do not inherit this. const self = this; return Effect.gen(function* () { - // Resolve once. A default change must not switch an active request to another account. + // Pin both the slot and credential. A replacement must not change an active request's account. const selectedId = getCodexOauthAccountId(self.readOpenaiConfig(), accountId); if (!isValidCodexOauthAccountId(selectedId)) return Err("Invalid Codex OAuth account ID"); - const revision = self.getAccountRevision(selectedId); const stored = self.readStoredAuth(selectedId); - if (!stored) return Err(`Codex OAuth account "${selectedId}" is not configured`); - if (!isCodexOauthAuthExpired(stored)) return Ok(stored); + const selection = { + accountId: selectedId, + revision: self.getAccountRevision(selectedId), + credentialId: stored?.credentialId, + }; + const initial = self.validateRequestAuth(stored, selection); + if (!initial.success || !isCodexOauthAuthExpired(initial.data)) return initial; let mutex = self.refreshMutexes.get(selectedId); if (!mutex) { @@ -610,34 +615,54 @@ export class CodexOauthService { return yield* Effect.acquireUseRelease( Effect.promise(() => refreshMutex.acquire()), () => - // Hold the file lease through persistence. Other processes must adopt the rotated token. - Effect.tryPromise({ - try: () => - self.fileLeaseManager.withCodexOauthRefreshLock(selectedId, async () => { - if (revision !== self.getAccountRevision(selectedId)) { - return Err("Codex OAuth account changed during the request"); - } - const latest = self.readStoredAuth(selectedId); - if (!latest) return Err(`Codex OAuth account "${selectedId}" is not configured`); - if (!isCodexOauthAuthExpired(latest)) return Ok(latest); - return await Effect.runPromise( - toWireResult( - self.refreshTokens( - { accountId: selectedId, revision, auth: latest, selectAsDefault: false }, - latest + Effect.gen(function* () { + const afterMutex = self.validateRequestAuth(self.readStoredAuth(selectedId), selection); + if (!afterMutex.success || !isCodexOauthAuthExpired(afterMutex.data)) return afterMutex; + // Hold the file lease through persistence. Other processes adopt only the same credential's rotation. + return yield* Effect.tryPromise({ + try: () => + self.fileLeaseManager.withCodexOauthRefreshLock(selectedId, async () => { + const afterLease = self.validateRequestAuth( + self.readStoredAuth(selectedId), + selection + ); + if (!afterLease.success || !isCodexOauthAuthExpired(afterLease.data)) + return afterLease; + return await Effect.runPromise( + toWireResult( + self.refreshTokens( + { ...selection, auth: afterLease.data, selectAsDefault: false }, + afterLease.data + ) ) - ) - ); - }), - catch: (error) => getErrorMessage(error), - }).pipe( - Effect.catch((error) => Effect.succeed(Err(`Codex OAuth refresh failed: ${error}`))) - ), + ); + }), + catch: (error) => getErrorMessage(error), + }).pipe( + Effect.catch((error) => Effect.succeed(Err(`Codex OAuth refresh failed: ${error}`))) + ); + }), (lock) => Effect.promise(() => lock[Symbol.asyncDispose]()) ); }); } + private validateRequestAuth( + auth: CodexOauthAuth | null, + selection: Pick + ): Result { + if (!auth) return Err(`Codex OAuth account "${selection.accountId}" is not configured`); + if ( + auth.credentialId !== selection.credentialId || + this.getAccountRevision(selection.accountId) !== selection.revision + ) { + return Err("Codex OAuth account changed during the request"); + } + if (auth.invalidReason) + return Err(`Codex OAuth account "${selection.accountId}" needs reconnect`); + return Ok(auth); + } + async dispose(): Promise { await this.desktopFlows.shutdownAll(); @@ -820,7 +845,7 @@ export class CodexOauthService { private persistAuth( selection: AccountSelection, - auth: CodexOauthAuth | undefined + auth: CodexOauthAuth ): Effect.Effect> { return this.withAccountMutationEffect( selection.accountId, @@ -835,7 +860,7 @@ export class CodexOauthService { !matchesAuth(stored, selection.auth) ) return null; - if (legacy || auth === undefined) return { value: auth }; + if (legacy) return { value: auth }; return { value: { ...(isPlainObject(current) ? current : {}), auth } }; }) ); @@ -846,7 +871,7 @@ export class CodexOauthService { auth: CodexOauthAuth, isActive: () => boolean ): Effect.Effect> { - const nextAuth = { ...auth, credentialId: crypto.randomUUID() }; + const nextAuth = { ...auth, credentialId: crypto.randomUUID(), invalidReason: undefined }; return this.withAccountMutationEffect( selection.accountId, this.configMutationEffect(() => @@ -1070,15 +1095,14 @@ export class CodexOauthService { if (!response.ok) { const errorText = yield* Effect.promise(() => response.text().catch(() => "")); - // When the refresh token is invalid/revoked, clear persisted auth so subsequent - // requests fall back to the existing "not connected" behavior. + // Keep the credential identity so an in-progress reconnect can replace rejected tokens. if (isInvalidGrantError(errorText)) { - log.debug("[Codex OAuth] Refresh token rejected; clearing stored auth"); - const disconnectResult = yield* self.persistAuth(selection, undefined); - if (!disconnectResult.success) { - log.warn( - `[Codex OAuth] Failed to clear stored auth after refresh failure: ${disconnectResult.error}` - ); + const invalidationResult = yield* self.persistAuth(selection, { + ...current, + invalidReason: "invalid_grant", + }); + if (!invalidationResult.success) { + log.warn(`[Codex OAuth] Failed to mark rejected auth: ${invalidationResult.error}`); } } @@ -1136,7 +1160,10 @@ export class CodexOauthService { return yield* Effect.fail(new CodexOauthError({ reason: persistResult.error })); } - return next; + const validated = self.validateRequestAuth(next, selection); + if (!validated.success) + return yield* Effect.fail(new CodexOauthError({ reason: validated.error })); + return validated.data; }).pipe( // Mirror the pre-Effect whole-body try/catch: an unexpected throw — // e.g. a rejected persistAuth/disconnect config write, which diff --git a/src/node/utils/codexOauthAuth.test.ts b/src/node/utils/codexOauthAuth.test.ts index 2c48635cf96..60beddeabd9 100644 --- a/src/node/utils/codexOauthAuth.test.ts +++ b/src/node/utils/codexOauthAuth.test.ts @@ -61,6 +61,16 @@ describe("parseCodexOauthAuth", () => { } }); + it("preserves only the supported invalid credential marker", () => { + const auth = { type: "oauth" as const, access: "access", refresh: "refresh", expires: 1000 }; + const marked = parseCodexOauthAuth({ ...auth, invalidReason: "invalid_grant" }); + expect(marked).toEqual({ ...auth, invalidReason: "invalid_grant" }); + expect(getCodexOauthAccounts({ codexOauth: marked })).toHaveLength(1); + for (const invalidReason of [null, "other", 42]) { + expect(parseCodexOauthAuth({ ...auth, invalidReason })).toBeNull(); + } + }); + it("returns null for non-object values", () => { expect(parseCodexOauthAuth(null)).toBeNull(); expect(parseCodexOauthAuth(undefined)).toBeNull(); diff --git a/src/node/utils/codexOauthAuth.ts b/src/node/utils/codexOauthAuth.ts index 0f7c4fa29eb..cade68a4fd3 100644 --- a/src/node/utils/codexOauthAuth.ts +++ b/src/node/utils/codexOauthAuth.ts @@ -20,6 +20,8 @@ export interface CodexOauthAuth { type: "oauth"; /** Identifies this login across token rotations and processes. */ credentialId?: string; + /** Blocks requests while retaining the login identity for reconnect. */ + invalidReason?: "invalid_grant"; /** OAuth access token (JWT). */ access: string; /** OAuth refresh token. */ @@ -49,6 +51,7 @@ export function parseCodexOauthAuth(value: unknown): CodexOauthAuth | null { const expires = value.expires; const accountId = value.accountId; const credentialId = credentialIdSchema.safeParse(value.credentialId); + const invalidReason = value.invalidReason; if (type !== "oauth") return null; if (typeof access !== "string" || !access) return null; @@ -60,6 +63,7 @@ export function parseCodexOauthAuth(value: unknown): CodexOauthAuth | null { } if (!credentialId.success) return null; + if (invalidReason !== undefined && invalidReason !== "invalid_grant") return null; return { type: "oauth", @@ -68,6 +72,7 @@ export function parseCodexOauthAuth(value: unknown): CodexOauthAuth | null { expires, accountId, credentialId: credentialId.data, + invalidReason, }; } @@ -80,7 +85,7 @@ export function isValidCodexOauthAccountId(accountId: string): boolean { ); } -/** Read connected slots from an OpenAI provider config. */ +/** Read stored slots, including invalid credentials that need reconnect. */ export function getCodexOauthAccounts(config: unknown): Array<{ id: string; label: string; From 8a6982fa3628e330e5e9b7a6438a83d3a2d37054 Mon Sep 17 00:00:00 2001 From: Mux Date: Sat, 5 Sep 2026 21:52:40 -0500 Subject: [PATCH 05/24] =?UTF-8?q?[openai]=20=F0=9F=A4=96=20fix:=20complete?= =?UTF-8?q?=20account=20lifecycle=20and=20keyboard=20controls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- _Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `high` • Cost: `$109.41`_ --- .../ProjectSidebar/ProjectSidebar.test.tsx | 2 + src/browser/contexts/SettingsContext.tsx | 26 ++ .../Settings/Sections/CodexAccounts.tsx | 288 ++++++++++++------ .../Sections/ProvidersSection.test.tsx | 190 +++++++++++- .../Settings/Sections/ProvidersSection.tsx | 13 + .../stories/App.codexAccounts.stories.tsx | 174 +++++++++++ src/browser/utils/commandIds.ts | 3 + src/browser/utils/commands/sources.test.ts | 74 +++++ src/browser/utils/commands/sources.ts | 91 +++++- src/common/orpc/schemas/api.ts | 10 +- .../utils/providers/codexOauthRouting.test.ts | 17 ++ .../utils/providers/codexOauthRouting.ts | 18 +- src/node/services/codexOauthService.test.ts | 102 +++++++ src/node/services/codexOauthService.ts | 38 ++- .../services/providerModelFactory.test.ts | 87 +++++- src/node/services/providerModelFactory.ts | 7 +- src/node/services/providerService.test.ts | 89 +++++- src/node/services/providerService.ts | 6 +- 18 files changed, 1123 insertions(+), 112 deletions(-) diff --git a/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx b/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx index 36e52ffab41..6c464bd8925 100644 --- a/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx +++ b/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx @@ -534,6 +534,8 @@ function installProjectSidebarTestDoubles() { providersExpandedProvider: null, setProvidersExpandedProvider: () => undefined, providersStartCoderLogin: false, + codexAccountAction: null, + setCodexAccountAction: () => undefined, setProvidersStartCoderLogin: () => undefined, runtimesProjectPath: null, setRuntimesProjectPath: () => undefined, diff --git a/src/browser/contexts/SettingsContext.tsx b/src/browser/contexts/SettingsContext.tsx index 95869a79f3e..c4057a705d1 100644 --- a/src/browser/contexts/SettingsContext.tsx +++ b/src/browser/contexts/SettingsContext.tsx @@ -7,14 +7,23 @@ import { useRef, useState, type ReactNode, + type Dispatch, + type SetStateAction, } from "react"; import { useRouter } from "@/browser/contexts/RouterContext"; +export type CodexAccountSettingsIntent = + | { type: "add" | "default" } + | { type: "reconnect" | "rename" | "disconnect"; accountId: string } + | { type: "project"; projectPath: string }; + export interface OpenSettingsOptions { /** When opening the Providers settings, expand the given provider. */ expandProvider?: string; /** When opening the Providers settings, start the Coder OAuth login. */ startCoderLogin?: boolean; + /** Open a Codex account operation through the existing settings controls. */ + codexAccountAction?: CodexAccountSettingsIntent; /** When opening the Runtimes settings, pre-select this project scope. */ runtimesProjectPath?: string; /** When opening the Secrets settings, pre-select this project scope. */ @@ -41,6 +50,9 @@ interface SettingsContextValue { providersStartCoderLogin: boolean; setProvidersStartCoderLogin: (start: boolean) => void; + codexAccountAction: CodexAccountSettingsIntent | null; + setCodexAccountAction: Dispatch>; + /** One-shot hint for RuntimesSection to pre-select a project scope. */ runtimesProjectPath: string | null; setRuntimesProjectPath: (path: string | null) => void; @@ -68,6 +80,9 @@ export function SettingsProvider(props: { children: ReactNode }) { const router = useRouter(); const [providersExpandedProvider, setProvidersExpandedProvider] = useState(null); const [providersStartCoderLogin, setProvidersStartCoderLogin] = useState(false); + const [codexAccountAction, setCodexAccountAction] = useState( + null + ); const [runtimesProjectPath, setRuntimesProjectPath] = useState(null); const [secretsProjectPath, setSecretsProjectPath] = useState(null); const [instructionsProjectPath, setInstructionsProjectPath] = useState(null); @@ -83,9 +98,14 @@ export function SettingsProvider(props: { children: ReactNode }) { if (nextSection === "providers") { setProvidersExpandedProvider(options?.expandProvider ?? null); setProvidersStartCoderLogin(options?.startCoderLogin ?? false); + // A fresh identity lets repeated commands reach an already-open settings section. + setCodexAccountAction( + options?.codexAccountAction ? { ...options.codexAccountAction } : null + ); } else { setProvidersExpandedProvider(null); setProvidersStartCoderLogin(false); + setCodexAccountAction(null); } if (nextSection === "runtimes") { setRuntimesProjectPath(options?.runtimesProjectPath ?? null); @@ -121,6 +141,7 @@ export function SettingsProvider(props: { children: ReactNode }) { if (wasOpenRef.current && !isOpen) { setProvidersExpandedProvider(null); setProvidersStartCoderLogin(false); + setCodexAccountAction(null); setRuntimesProjectPath(null); setSecretsProjectPath(null); setInstructionsProjectPath(null); @@ -134,6 +155,7 @@ export function SettingsProvider(props: { children: ReactNode }) { const close = useCallback(() => { setProvidersExpandedProvider(null); setProvidersStartCoderLogin(false); + setCodexAccountAction(null); setRuntimesProjectPath(null); setSecretsProjectPath(null); setInstructionsProjectPath(null); @@ -145,6 +167,7 @@ export function SettingsProvider(props: { children: ReactNode }) { if (section !== "providers") { setProvidersExpandedProvider(null); setProvidersStartCoderLogin(false); + setCodexAccountAction(null); } if (section !== "runtimes") { // Runtime scope hints are one-shot and should not persist across section changes. @@ -173,6 +196,8 @@ export function SettingsProvider(props: { children: ReactNode }) { setProvidersExpandedProvider, providersStartCoderLogin, setProvidersStartCoderLogin, + codexAccountAction, + setCodexAccountAction, runtimesProjectPath, setRuntimesProjectPath, secretsProjectPath, @@ -189,6 +214,7 @@ export function SettingsProvider(props: { children: ReactNode }) { registerOnClose, providersExpandedProvider, providersStartCoderLogin, + codexAccountAction, runtimesProjectPath, secretsProjectPath, instructionsProjectPath, diff --git a/src/browser/features/Settings/Sections/CodexAccounts.tsx b/src/browser/features/Settings/Sections/CodexAccounts.tsx index d9ae367236f..925fc77381e 100644 --- a/src/browser/features/Settings/Sections/CodexAccounts.tsx +++ b/src/browser/features/Settings/Sections/CodexAccounts.tsx @@ -1,4 +1,5 @@ -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState, type Ref } from "react"; +import { useSettings, type CodexAccountSettingsIntent } from "@/browser/contexts/SettingsContext"; import { Loader2 } from "lucide-react"; import { Button } from "@/browser/components/Button/Button"; import { useAPI, type APIClient } from "@/browser/contexts/API"; @@ -21,6 +22,9 @@ interface LoginFlow { cancel: () => Promise; } +const legacyAccounts: Account[] = [{ id: CODEX_OAUTH_DEFAULT_ACCOUNT_ID, label: "Default" }]; +const noAccounts: Account[] = []; + const inputClassName = "bg-background border-border-light text-foreground w-full min-w-0 rounded border px-2 py-1.5 text-xs"; @@ -29,6 +33,7 @@ function AccountSelect(props: { value: string; accounts: Account[]; defaultLabel?: string; + selectRef?: Ref; disabled: boolean; onChange: (value: string) => void; }) { @@ -39,6 +44,7 @@ function AccountSelect(props: {