diff --git a/cli/src/codex/codexAppServerClient.test.ts b/cli/src/codex/codexAppServerClient.test.ts new file mode 100644 index 00000000..b144265e --- /dev/null +++ b/cli/src/codex/codexAppServerClient.test.ts @@ -0,0 +1,64 @@ +import { EventEmitter } from 'node:events'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { execFileSyncMock, spawnMock } = vi.hoisted(() => ({ + execFileSyncMock: vi.fn(() => 'codex-cli 1.0.0'), + spawnMock: vi.fn() +})); + +vi.mock('node:child_process', async () => { + const actual = await vi.importActual('node:child_process'); + return { + ...actual, + execFileSync: execFileSyncMock, + spawn: spawnMock + }; +}); + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs'); + return { ...actual, existsSync: vi.fn(() => false) }; +}); + +vi.mock('@/utils/process', () => ({ + killProcessByChildProcess: vi.fn(async () => true) +})); + +vi.mock('@/ui/logger', () => ({ + logger: { debug: vi.fn() } +})); + +import { CodexAppServerClient } from './codexAppServerClient'; + +function fakeStream(): EventEmitter & { setEncoding: ReturnType } { + return Object.assign(new EventEmitter(), { setEncoding: vi.fn() }); +} + +function fakeChild() { + return Object.assign(new EventEmitter(), { + stdin: { end: vi.fn(), write: vi.fn() }, + stdout: fakeStream(), + stderr: fakeStream() + }); +} + +describe('CodexAppServerClient process cwd', () => { + beforeEach(() => { + execFileSyncMock.mockClear(); + spawnMock.mockReset(); + }); + + it('passes an explicit neutral cwd to the app-server process', async () => { + spawnMock.mockReturnValue(fakeChild()); + const client = new CodexAppServerClient({ cwd: '/neutral-home' }); + + await client.connect(); + + expect(spawnMock).toHaveBeenCalledWith( + 'codex', + ['app-server'], + expect.objectContaining({ cwd: '/neutral-home' }) + ); + await client.disconnect(); + }); +}); diff --git a/cli/src/codex/codexAppServerClient.ts b/cli/src/codex/codexAppServerClient.ts index 5c0d6a77..a70cf06c 100644 --- a/cli/src/codex/codexAppServerClient.ts +++ b/cli/src/codex/codexAppServerClient.ts @@ -66,6 +66,10 @@ type PendingRequest = { cleanup: () => void; }; +type CodexAppServerClientOptions = { + cwd?: string; +}; + function asRecord(value: unknown): Record | null { if (!value || typeof value !== 'object') { return null; @@ -169,6 +173,10 @@ export class CodexAppServerClient extends JsonLineParser { static readonly DEFAULT_TIMEOUT_MS = 14 * 24 * 60 * 60 * 1000; + constructor(private readonly options: CodexAppServerClientOptions = {}) { + super(); + } + setStderrHandler(handler: ((text: string) => void) | null): void { this.stderrHandler = handler; } @@ -181,6 +189,7 @@ export class CodexAppServerClient extends JsonLineParser { const codexCommand = resolveCodexAppServerCommand(); logger.debug(`[CodexAppServer] Starting ${codexCommand} app-server`); this.process = spawn(codexCommand, ['app-server'], { + cwd: this.options.cwd, env: Object.keys(process.env).reduce((acc, key) => { const value = process.env[key]; if (typeof value === 'string') acc[key] = value; diff --git a/cli/src/modules/common/codexModels.test.ts b/cli/src/modules/common/codexModels.test.ts new file mode 100644 index 00000000..759254d8 --- /dev/null +++ b/cli/src/modules/common/codexModels.test.ts @@ -0,0 +1,39 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { constructorOptions } = vi.hoisted(() => ({ + constructorOptions: [] as unknown[] +})); + +vi.mock('node:os', async () => { + const actual = await vi.importActual('node:os'); + return { ...actual, homedir: vi.fn(() => '/neutral-home') }; +}); + +vi.mock('@/codex/codexAppServerClient', () => ({ + CodexAppServerClient: class { + constructor(options: unknown) { + constructorOptions.push(options); + } + + async connect(): Promise {} + async initialize(): Promise {} + async listModels(): Promise<{ data: unknown[] }> { + return { data: [] }; + } + async disconnect(): Promise {} + } +})); + +import { listCodexModels } from './codexModels'; + +describe('listCodexModels cwd', () => { + beforeEach(() => { + constructorOptions.length = 0; + }); + + it('starts discovery from the user home instead of the caller cwd', async () => { + await listCodexModels(); + + expect(constructorOptions).toEqual([{ cwd: '/neutral-home' }]); + }); +}); diff --git a/cli/src/modules/common/codexModels.ts b/cli/src/modules/common/codexModels.ts index fa51d052..6d42872a 100644 --- a/cli/src/modules/common/codexModels.ts +++ b/cli/src/modules/common/codexModels.ts @@ -1,3 +1,4 @@ +import { homedir } from 'node:os'; import type { CodexModelsResponse, CodexModelSummary } from '@hapi/protocol/apiTypes'; import { CodexAppServerClient } from '@/codex/codexAppServerClient'; import { getErrorMessage } from './rpcResponses'; @@ -81,7 +82,9 @@ function normalizeModel(entry: unknown): CodexModelSummary | null { } export async function listCodexModels(includeHidden: boolean = false): Promise { - const client = new CodexAppServerClient(); + // Model discovery is account-scoped. Never inherit a session/runner cwd: + // project config or a deleted worktree must not alter or break the catalog. + const client = new CodexAppServerClient({ cwd: homedir() }); try { await client.connect(); diff --git a/hub/src/sync/rpcGateway.test.ts b/hub/src/sync/rpcGateway.test.ts index d565a78b..4f72d599 100644 --- a/hub/src/sync/rpcGateway.test.ts +++ b/hub/src/sync/rpcGateway.test.ts @@ -67,6 +67,15 @@ describe('RpcGateway RPC timeouts', () => { expect(timeouts).toEqual([120_000]) }) + it('uses the session RPC for explicit Codex fallback discovery', async () => { + const { gateway, calls, timeouts } = createGateway() + + await gateway.listCodexModelsForSession('session-1') + + expect(calls.map((call) => call.method)).toEqual(['session-1:listCodexModels']) + expect(timeouts).toEqual([120_000]) + }) + it('uses an extended RPC timeout when listing Cursor models for a machine', async () => { const { gateway, timeouts } = createGateway() diff --git a/hub/src/sync/rpcGateway.ts b/hub/src/sync/rpcGateway.ts index 406bb784..259155a8 100644 --- a/hub/src/sync/rpcGateway.ts +++ b/hub/src/sync/rpcGateway.ts @@ -339,6 +339,15 @@ export class RpcGateway { return await this.machineRpc(machineId, RPC_METHODS.ListCodexModels, {}, MODEL_LIST_RPC_TIMEOUT_MS) as RpcListCodexModelsResponse } + async listCodexModelsForSession(sessionId: string): Promise { + return await this.sessionRpc( + sessionId, + RPC_METHODS.ListCodexModels, + {}, + MODEL_LIST_RPC_TIMEOUT_MS + ) as RpcListCodexModelsResponse + } + async listCodexSessionsForMachine(machineId: string, cwd?: string | null, sessionIds?: string[]): Promise { const result = await this.machineRpc(machineId, RPC_METHODS.ListCodexSessions, { cwd: cwd ?? null, sessionIds }, MODEL_LIST_RPC_TIMEOUT_MS) return ListCodexSessionsRpcResponseSchema.parse(result) diff --git a/hub/src/sync/syncEngine.ts b/hub/src/sync/syncEngine.ts index 4e0644bc..8cdffa69 100644 --- a/hub/src/sync/syncEngine.ts +++ b/hub/src/sync/syncEngine.ts @@ -3736,6 +3736,10 @@ async uploadScratchlistAttachment( return await this.rpcGateway.listCodexModelsForMachine(machineId) } + async listCodexModelsForSession(sessionId: string): Promise { + return await this.rpcGateway.listCodexModelsForSession(sessionId) + } + async listCodexSessionsForMachine(machineId: string, cwd?: string | null, sessionIds?: string[]) { return await this.rpcGateway.listCodexSessionsForMachine(machineId, cwd, sessionIds) } diff --git a/hub/src/web/routes/machines.test.ts b/hub/src/web/routes/machines.test.ts index 844702d0..845da593 100644 --- a/hub/src/web/routes/machines.test.ts +++ b/hub/src/web/routes/machines.test.ts @@ -3,6 +3,7 @@ import { Hono } from 'hono' import type { Machine, SyncEngine } from '../../sync/syncEngine' import type { WebAppEnv } from '../middleware/auth' import { createMachinesRoutes } from './machines' +import { RpcTargetMissingError } from '../../sync/rpcGateway' function createMachine(overrides?: Partial): Machine { return { @@ -102,6 +103,36 @@ describe('machines routes', () => { }) }) + it('returns a stable code when the Codex machine RPC target is absent', async () => { + const machine = createMachine() + const engine = { + getMachine: () => machine, + getMachineByNamespace: () => machine, + listCodexModelsForMachine: async () => { + throw new RpcTargetMissingError( + 'machine-1:listCodexModels', + 'handler-not-registered' + ) + } + } as Partial + + const app = new Hono() + app.use('*', async (c, next) => { + c.set('namespace', 'default') + await next() + }) + app.route('/api', createMachinesRoutes(() => engine as SyncEngine)) + + const response = await app.request('/api/machines/machine-1/codex-models') + + expect(response.status).toBe(503) + expect(await response.json()).toEqual({ + success: false, + error: 'RPC handler not registered: machine-1:listCodexModels', + code: 'rpc_target_missing' + }) + }) + it('forwards startingMode "pty" to SyncEngine.spawnSession in the startingMode slot', async () => { const machine = createMachine() let captured: unknown[] | null = null diff --git a/hub/src/web/routes/machines.ts b/hub/src/web/routes/machines.ts index 38952a37..10abbeaa 100644 --- a/hub/src/web/routes/machines.ts +++ b/hub/src/web/routes/machines.ts @@ -6,7 +6,9 @@ import { SpawnSessionRequestSchema } from '@hapi/protocol' import { Hono } from 'hono' +import { RPC_TARGET_MISSING_ERROR_CODE } from '@hapi/protocol/rpcMethods' import type { SyncEngine } from '../../sync/syncEngine' +import { RpcTargetMissingError } from '../../sync/rpcGateway' import type { WebAppEnv } from '../middleware/auth' import { requireMachine } from './guards' @@ -203,6 +205,13 @@ export function createMachinesRoutes(getSyncEngine: () => SyncEngine | null): Ho const result = await engine.listCodexModelsForMachine(machineId) return c.json(result) } catch (error) { + if (error instanceof RpcTargetMissingError) { + return c.json({ + success: false, + error: error.message, + code: RPC_TARGET_MISSING_ERROR_CODE + }, 503) + } return c.json({ success: false, error: error instanceof Error ? error.message : 'Failed to list Codex models' diff --git a/hub/src/web/routes/sessions.test.ts b/hub/src/web/routes/sessions.test.ts index 779d3b91..365b5260 100644 --- a/hub/src/web/routes/sessions.test.ts +++ b/hub/src/web/routes/sessions.test.ts @@ -64,6 +64,7 @@ function createApp(session: Session, opts?: { sessionExists?: boolean archiveSession?: (sessionId: string) => Promise getCursorChatStoreStatus?: SyncEngine['getCursorChatStoreStatus'] + listCodexModelsForSession?: SyncEngine['listCodexModelsForSession'] forkConversation?: SyncEngine['forkConversation'] rewindConversation?: SyncEngine['rewindConversation'] setSessionPinned?: (sessionId: string, pinned: boolean) => void @@ -127,6 +128,10 @@ function createApp(session: Session, opts?: { : { ok: false, reason: 'not-found' }, applySessionConfig, listCursorModelsForSession, + listCodexModelsForSession: opts?.listCodexModelsForSession ?? (async () => ({ + success: true, + models: [] + })), listOpencodeModelsForSession, listOpencodeReasoningEffortOptionsForSession, listGrokModelsForSession, @@ -196,6 +201,36 @@ describe('sessions routes', () => { expect(response.status).toBe(400) }) + it('uses session-scoped Codex model discovery for the fallback endpoint', async () => { + const session = createSession({ + metadata: { + path: '/tmp/project', + host: 'localhost', + flavor: 'codex', + machineId: 'machine-1' + } + }) + const captured: string[] = [] + const { app } = createApp(session, { + listCodexModelsForSession: async (sessionId) => { + captured.push(sessionId) + return { + success: true, + models: [{ id: 'gpt-5.5', displayName: 'GPT-5.5', isDefault: true }] + } + } + }) + + const response = await app.request('/api/sessions/session-1/codex-models') + + expect(response.status).toBe(200) + expect(captured).toEqual(['session-1']) + expect(await response.json()).toEqual({ + success: true, + models: [{ id: 'gpt-5.5', displayName: 'GPT-5.5', isDefault: true }] + }) + }) + it('returns the machine-scoped Cursor chat store status', async () => { const session = createSession({ active: false, diff --git a/hub/src/web/routes/sessions.ts b/hub/src/web/routes/sessions.ts index ab3dafd2..e935ac8f 100644 --- a/hub/src/web/routes/sessions.ts +++ b/hub/src/web/routes/sessions.ts @@ -1253,6 +1253,36 @@ export function createSessionsRoutes(getSyncEngine: () => SyncEngine | null): Ho } }) + app.get('/sessions/:id/codex-models', async (c) => { + const engine = requireSyncEngine(c, getSyncEngine) + if (engine instanceof Response) { + return engine + } + + const sessionResult = requireSessionFromParam(c, engine, { requireActive: true }) + if (sessionResult instanceof Response) { + return sessionResult + } + + const flavor = sessionResult.session.metadata?.flavor ?? 'claude' + if (flavor !== 'codex') { + return c.json({ + success: false, + error: 'Codex models are only available for Codex sessions' + }, 400) + } + + try { + const result = await engine.listCodexModelsForSession(sessionResult.sessionId) + return c.json(result) + } catch (error) { + return c.json({ + success: false, + error: error instanceof Error ? error.message : 'Failed to list Codex models' + }, 500) + } + }) + app.get('/sessions/:id/opencode-models', async (c) => { const engine = requireSyncEngine(c, getSyncEngine) if (engine instanceof Response) { diff --git a/shared/src/rpcMethods.ts b/shared/src/rpcMethods.ts index 120eddce..ef57eaa7 100644 --- a/shared/src/rpcMethods.ts +++ b/shared/src/rpcMethods.ts @@ -46,4 +46,6 @@ export const RPC_METHODS = { RewindConversation: 'rewind-conversation', } as const +export const RPC_TARGET_MISSING_ERROR_CODE = 'rpc_target_missing' as const + export type RpcMethod = typeof RPC_METHODS[keyof typeof RPC_METHODS] diff --git a/web/src/api/client.ts b/web/src/api/client.ts index de06d4a9..08650fdd 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -822,6 +822,12 @@ export class ApiClient { ) } + async getSessionCodexModels(sessionId: string): Promise { + return await this.request( + `/api/sessions/${encodeURIComponent(sessionId)}/codex-models` + ) + } + async getSessionOpencodeModels(sessionId: string): Promise { return await this.request( `/api/sessions/${encodeURIComponent(sessionId)}/opencode-models` diff --git a/web/src/components/SessionChat.tsx b/web/src/components/SessionChat.tsx index 7c8fb153..52bcbdd6 100644 --- a/web/src/components/SessionChat.tsx +++ b/web/src/components/SessionChat.tsx @@ -838,6 +838,7 @@ function SessionChatInner(props: SessionChatProps) { const codexCollaborationModeSupported = agentFlavor === 'codex' && !controlledByUser const codexModelsState = useCodexModels({ api: props.api, + sessionId: props.session.id, machineId: props.session.metadata?.machineId ?? null, enabled: agentFlavor === 'codex' && props.session.active && !controlledByUser }) diff --git a/web/src/hooks/queries/useCodexModels.test.tsx b/web/src/hooks/queries/useCodexModels.test.tsx new file mode 100644 index 00000000..e0d0094e --- /dev/null +++ b/web/src/hooks/queries/useCodexModels.test.tsx @@ -0,0 +1,99 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { renderHook, waitFor } from '@testing-library/react' +import type { PropsWithChildren } from 'react' +import { describe, expect, it, vi } from 'vitest' +import { RPC_TARGET_MISSING_ERROR_CODE } from '@hapi/protocol/rpcMethods' +import { ApiError, type ApiClient } from '@/api/client' +import { useCodexModels } from './useCodexModels' + +function wrapper(queryClient: QueryClient) { + return ({ children }: PropsWithChildren) => ( + {children} + ) +} + +function createQueryClient(): QueryClient { + return new QueryClient({ + defaultOptions: { + queries: { retry: false } + } + }) +} + +describe('useCodexModels query scoping', () => { + it('shares machine-backed discovery across sessions on the same machine', async () => { + const getMachineCodexModels = vi.fn(async () => ({ + success: true, + models: [{ id: 'gpt-5.5', displayName: 'GPT-5.5', isDefault: true }] + })) + const getSessionCodexModels = vi.fn() + const api = { + getMachineCodexModels, + getSessionCodexModels + } as unknown as ApiClient + const queryClient = createQueryClient() + const sharedWrapper = wrapper(queryClient) + + const first = renderHook(() => useCodexModels({ + api, + sessionId: 'session-1', + machineId: 'machine-1', + enabled: true + }), { wrapper: sharedWrapper }) + const second = renderHook(() => useCodexModels({ + api, + sessionId: 'session-2', + machineId: 'machine-1', + enabled: true + }), { wrapper: sharedWrapper }) + + await waitFor(() => { + expect(first.result.current.models[0]?.id).toBe('gpt-5.5') + expect(second.result.current.models[0]?.id).toBe('gpt-5.5') + }) + expect(getMachineCodexModels).toHaveBeenCalledTimes(1) + expect(getSessionCodexModels).not.toHaveBeenCalled() + }) + + it('isolates session fallback catalogs after the machine target is missing', async () => { + const getMachineCodexModels = vi.fn(async () => { + throw new ApiError( + 'machine RPC unavailable', + 503, + RPC_TARGET_MISSING_ERROR_CODE + ) + }) + const getSessionCodexModels = vi.fn(async (sessionId: string) => ({ + success: true, + models: [{ id: sessionId, displayName: sessionId, isDefault: true }] + })) + const api = { + getMachineCodexModels, + getSessionCodexModels + } as unknown as ApiClient + const queryClient = createQueryClient() + const sharedWrapper = wrapper(queryClient) + + const first = renderHook(() => useCodexModels({ + api, + sessionId: 'session-1', + machineId: 'machine-1', + enabled: true + }), { wrapper: sharedWrapper }) + const second = renderHook(() => useCodexModels({ + api, + sessionId: 'session-2', + machineId: 'machine-1', + enabled: true + }), { wrapper: sharedWrapper }) + + await waitFor(() => { + expect(first.result.current.models[0]?.id).toBe('session-1') + expect(second.result.current.models[0]?.id).toBe('session-2') + }) + expect(getSessionCodexModels.mock.calls.map(([sessionId]) => sessionId).sort()).toEqual([ + 'session-1', + 'session-2' + ]) + }) +}) diff --git a/web/src/hooks/queries/useCodexModels.ts b/web/src/hooks/queries/useCodexModels.ts index b0847086..17aef52e 100644 --- a/web/src/hooks/queries/useCodexModels.ts +++ b/web/src/hooks/queries/useCodexModels.ts @@ -1,10 +1,12 @@ import { useQuery } from '@tanstack/react-query' -import type { ApiClient } from '@/api/client' +import { RPC_TARGET_MISSING_ERROR_CODE } from '@hapi/protocol/rpcMethods' +import { ApiError, type ApiClient } from '@/api/client' import type { CodexModelSummary } from '@/types/api' import { queryKeys } from '@/lib/query-keys' export function useCodexModels(args: { api: ApiClient | null + sessionId?: string | null machineId?: string | null enabled?: boolean }): { @@ -12,12 +14,11 @@ export function useCodexModels(args: { isLoading: boolean error: string | null } { - const { api, machineId } = args - const enabled = Boolean(args.enabled && api && machineId) - const queryKey = queryKeys.machineCodexModels(machineId ?? 'unknown') + const { api, sessionId, machineId } = args + const enabled = Boolean(args.enabled && api && (sessionId || machineId)) - const query = useQuery({ - queryKey, + const machineQuery = useQuery({ + queryKey: queryKeys.machineCodexModels(machineId ?? 'unknown'), queryFn: async () => { if (!api) { throw new Error('API unavailable') @@ -27,11 +28,38 @@ export function useCodexModels(args: { } throw new Error('Codex models target unavailable') }, - enabled, + enabled: Boolean(enabled && machineId), staleTime: 30_000, retry: false, }) + // Successful machine discovery stays shared across chats and New Session. + // Only an absent machine RPC unlocks the per-session neutral fallback. + const useSessionFallback = Boolean( + enabled + && sessionId + && (!machineId || ( + machineQuery.error instanceof ApiError + && machineQuery.error.code === RPC_TARGET_MISSING_ERROR_CODE + )) + ) + const sessionQuery = useQuery({ + queryKey: queryKeys.sessionCodexModels(sessionId ?? 'unknown'), + queryFn: async () => { + if (!api) { + throw new Error('API unavailable') + } + if (sessionId) { + return await api.getSessionCodexModels(sessionId) + } + throw new Error('Codex models fallback target unavailable') + }, + enabled: useSessionFallback, + staleTime: 30_000, + retry: false, + }) + const query = useSessionFallback ? sessionQuery : machineQuery + return { models: query.data?.models ?? [], isLoading: query.isLoading, diff --git a/web/src/lib/query-keys.ts b/web/src/lib/query-keys.ts index cc00e0f6..00cd3a02 100644 --- a/web/src/lib/query-keys.ts +++ b/web/src/lib/query-keys.ts @@ -7,6 +7,7 @@ export const queryKeys = { hubSettings: ['hub-settings'] as const, usageSummary: (range: string, timeZone: string) => ['usage-summary', range, timeZone] as const, machineCodexModels: (machineId: string) => ['machine-codex-models', machineId] as const, + sessionCodexModels: (sessionId: string) => ['session-codex-models', sessionId] as const, gitStatus: (sessionId: string) => ['git-status', sessionId] as const, sessionFiles: (sessionId: string, query: string) => ['session-files', sessionId, query] as const, sessionDirectory: (sessionId: string, path: string) => ['session-directory', sessionId, path] as const,