diff --git a/apps/server/src/guide/topics.ts b/apps/server/src/guide/topics.ts index d8d6b5064..ff4cc1877 100644 --- a/apps/server/src/guide/topics.ts +++ b/apps/server/src/guide/topics.ts @@ -250,7 +250,7 @@ A Claude sign-in made with Claude's own sign-in (not an API key or a long-lived When a Codex ChatGPT sign-in belongs to more than one workspace (a personal one and a company's, say), the Codex row shows the one in use, and its Account & Sign-in has Workspace: pick another and Switch. Switching first stops that Codex's running chats and holds new ones until it's done. A workspace behind company sign-on opens a sign-in page; Cancel keeps the current workspace. A chat whose data the old workspace keeps in another region may not continue after a switch; start a new thread for it. It needs Codex 0.167 or later. -When Codex's own config points it at a gateway or another model provider instead of OpenAI, the Codex row says Runs on and the provider's name, and Account & Sign-in has a Model provider line. Threadlines only shows it; change it in Codex's config. A ChatGPT sign-in beside it keeps its email, workspace and Sign out. It needs Codex 0.168 or later. +When Codex's own config points it at a gateway or another model provider instead of OpenAI, the Codex row says Runs on and the provider's name, and Account & Sign-in has a Model provider line. Threadlines only shows it; change it in Codex's config. A ChatGPT sign-in beside it keeps its email, workspace and Sign out. The model Codex's config names is in the model picker and is the default for new threads, and titles and commit messages written by Codex go through the gateway too. A Codex agent answering on the side in a room still runs on OpenAI. It needs Codex 0.168 or later. When OpenAI asks a ChatGPT account to do something about its security, the Codex row says Security notice, and Account & Sign-in shows OpenAI's words with a button that opens the page on OpenAI's site. It needs Codex 0.169 or later. diff --git a/apps/server/src/provider/Drivers/CodexDriver.ts b/apps/server/src/provider/Drivers/CodexDriver.ts index 711dc024d..f6eb741c3 100644 --- a/apps/server/src/provider/Drivers/CodexDriver.ts +++ b/apps/server/src/provider/Drivers/CodexDriver.ts @@ -24,8 +24,10 @@ import { CodexSettings, ProviderDriverKind, type ServerProvider } from "@threadlines/contracts"; import * as Clock from "effect/Clock"; import * as DateTime from "effect/DateTime"; +import * as Deferred from "effect/Deferred"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; @@ -45,6 +47,11 @@ import { makeCodexAdapter } from "../Layers/CodexAdapter.ts"; import { freshOrCurrentSnapshot } from "../freshProviderSnapshot.ts"; import { checkCodexProviderStatus, + codexGatewayForCheck, + currentCodexModelProvider, + isCodexModelProviderKnown, + probeCodexGateway, + type CodexModelProviderMemory, type CodexSecurityNoticeMemory, type CodexWorkspaceMemory, consumeCodexRateLimitResetCredit, @@ -83,6 +90,14 @@ const decodeCodexSettings = Schema.decodeSync(CodexSettings); const DRIVER_KIND = ProviderDriverKind.make("codex"); const SNAPSHOT_REFRESH_INTERVAL = Duration.minutes(5); +/** + * How long a title or commit message waits for the instance's first status + * check, which normally takes a second or two. Past it the run asks Codex + * itself which provider it is on, rather than hold the user's commit. + */ +const FIRST_CHECK_WAIT = Duration.seconds(5); +/** How long that question may take: two reads of Codex's own config. */ +const GATEWAY_PROBE_TIMEOUT = Duration.seconds(10); const WINDOWS_NATIVE_INSTALL = makeWindowsNativeInstaller({ url: "https://chatgpt.com/codex/install.ps1", lockKey: "codex-native-win32", @@ -193,7 +208,37 @@ export const CodexDriver: ProviderDriver = { // here; the registry only has to worry about snapshot-build and // spawner-availability failures surfaced from `checkCodexProviderStatus` // below. - const textGeneration = yield* makeCodexTextGeneration(effectiveConfig, processEnv); + // Filled in by each status check; titles and commit messages read it + // at the moment they run. Without an answer they would go to OpenAI, + // gateway or not, so a run that finds none waits a moment for the first + // check (it starts with the instance) and then asks Codex itself. + const modelProviderMemory: CodexModelProviderMemory = {}; + const firstCheckEnded = yield* Deferred.make(); + const modelProviderLaunch = Effect.gen(function* () { + yield* Deferred.await(firstCheckEnded).pipe(Effect.timeoutOption(FIRST_CHECK_WAIT)); + const nowMs = yield* Clock.currentTimeMillis; + if (!isCodexModelProviderKnown(modelProviderMemory, nowMs)) { + const reads = yield* probeCodexGateway({ + binaryPath: effectiveConfig.binaryPath, + ...(effectiveConfig.homePath ? { homePath: effectiveConfig.homePath } : {}), + cwd: process.cwd(), + environment: processEnv, + }).pipe( + Effect.scoped, + Effect.timeoutOption(GATEWAY_PROBE_TIMEOUT), + Effect.map(Option.getOrUndefined), + Effect.orElseSucceed(() => undefined), + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + ); + if (reads) { + codexGatewayForCheck(reads, modelProviderMemory, nowMs); + } + } + return currentCodexModelProvider(modelProviderMemory, nowMs)?.launch; + }); + const textGeneration = yield* makeCodexTextGeneration(effectiveConfig, processEnv, { + modelProviderLaunch, + }); // Build a managed snapshot whose settings never change — mutations come // in as instance rebuilds from the registry rather than in-place @@ -207,9 +252,11 @@ export const CodexDriver: ProviderDriver = { processEnv, workspaceMemory, securityNoticeMemory, + modelProviderMemory, ).pipe( Effect.map(stampIdentity), Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.ensuring(Deferred.succeed(firstCheckEnded, undefined)), ); const snapshot = yield* makeManagedServerProvider({ get maintenanceCapabilities() { diff --git a/apps/server/src/provider/Layers/CodexProvider.ts b/apps/server/src/provider/Layers/CodexProvider.ts index 1b3e08c20..80a814747 100644 --- a/apps/server/src/provider/Layers/CodexProvider.ts +++ b/apps/server/src/provider/Layers/CodexProvider.ts @@ -1,3 +1,4 @@ +import * as Clock from "effect/Clock"; import * as DateTime from "effect/DateTime"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; @@ -38,6 +39,10 @@ import { createModelCapabilities } from "@threadlines/shared/model"; import { isCommandAvailable } from "@threadlines/shared/shell"; import { DEFAULT_CODEX_SERVICE_TIER_SELECTION } from "../../codexServiceTier.ts"; import { CODEX_APP_SERVER_ARGS, codexAppServerCommandOptions } from "../codexAppServerArgs.ts"; +import { + codexModelProviderLaunch, + type CodexModelProviderLaunch, +} from "../codexModelProviderLaunch.ts"; import { detectProviderBinary } from "../providerDetection.ts"; import { AUTH_PROBE_TIMEOUT_MS, @@ -70,6 +75,12 @@ export interface CodexAppServerProviderSnapshot { readonly tokenUsage?: CodexSchema.V2GetAccountTokenUsageResponse; /** Codex's model providers and which one new chats use (`modelProvider/list`). */ readonly modelProviders?: CodexSchema.V2ModelProviderListResponse; + /** + * What Codex's config says about the provider its chats run on, when that + * is not OpenAI (`config/read`). Absent on OpenAI, or when Codex gave no + * answer. + */ + readonly gateway?: CodexGatewayConfig; /** * What OpenAI asks this account to do about its security * (`account/securitySetup/read`). Absent when Codex gave no answer; a @@ -81,6 +92,24 @@ export interface CodexAppServerProviderSnapshot { readonly skills: ReadonlyArray; } +/** The parts of Codex's config that matter when its chats run on another provider. */ +export interface CodexGatewayConfig { + readonly providerId: string; + /** + * Whether Codex answered with its config. When it did not, the model and + * definition below are unknown, not absent. + */ + readonly configRead: boolean; + /** The config's `model`: what Codex itself uses on that provider. */ + readonly model?: string; + /** + * The provider's entry in the config's `model_providers`, as Codex reports + * it. It can hold secrets, so it stays on the server. Absent for a provider + * Codex has built in. + */ + readonly definition?: unknown; +} + const REASONING_EFFORT_LABELS: Record = { none: "None", minimal: "Minimal", @@ -772,42 +801,42 @@ const probeCodexAppServerProvider = Effect.fn("probeCodexAppServerProvider")(fun const versionMatch = initialize.userAgent.match(/\/([^\s]+)/); const version = versionMatch ? versionMatch[1] : undefined; - const accountResponse = yield* client.request("account/read", {}); + // The provider list is read beside the account, ahead of the signed-out + // return below: a Codex moved back to OpenAI while signed out must still + // say so, or the gateway it left would stay remembered. + const [accountResponse, modelProviders] = yield* Effect.all( + [client.request("account/read", {}), readCodexModelProviders(client)], + { concurrency: "unbounded" }, + ); if (!accountResponse.account && accountResponse.requiresOpenaiAuth) { return { account: accountResponse, + ...(modelProviders ? { modelProviders } : {}), version, models: appendCustomCodexModels([], input.customModels ?? []), skills: [], } satisfies CodexAppServerProviderSnapshot; } - const [ - skillsResponse, - models, - rateLimits, - tokenUsage, - accountSessions, - modelProviders, - securitySetup, - ] = yield* Effect.all( - [ - client.request("skills/list", { - cwds: [input.cwd], - }), - requestAllCodexModels(client), - client.request("account/rateLimits/read", null).pipe(Effect.orElseSucceed(() => undefined)), - client.request("account/usage/read", {}).pipe(Effect.orElseSucceed(() => undefined)), - accountResponse.account?.type === "chatgpt" - ? readCodexAccountSessions(client) - : Effect.succeed(undefined), - readCodexModelProviders(client), - accountResponse.account?.type === "chatgpt" - ? readCodexSecuritySetup(client) - : Effect.succeed(undefined), - ], - { concurrency: "unbounded" }, - ); + const [skillsResponse, models, rateLimits, tokenUsage, accountSessions, gateway, securitySetup] = + yield* Effect.all( + [ + client.request("skills/list", { + cwds: [input.cwd], + }), + requestAllCodexModels(client), + client.request("account/rateLimits/read", null).pipe(Effect.orElseSucceed(() => undefined)), + client.request("account/usage/read", {}).pipe(Effect.orElseSucceed(() => undefined)), + accountResponse.account?.type === "chatgpt" + ? readCodexAccountSessions(client) + : Effect.succeed(undefined), + readCodexGatewayConfig(client, modelProviders), + accountResponse.account?.type === "chatgpt" + ? readCodexSecuritySetup(client) + : Effect.succeed(undefined), + ], + { concurrency: "unbounded" }, + ); return { account: accountResponse, @@ -816,6 +845,7 @@ const probeCodexAppServerProvider = Effect.fn("probeCodexAppServerProvider")(fun ...(tokenUsage ? { tokenUsage } : {}), ...(modelProviders ? { modelProviders } : {}), ...(securitySetup ? { securitySetup } : {}), + ...(gateway ? { gateway } : {}), version, models: appendCustomCodexModels(models, input.customModels ?? []), skills: parseCodexSkillsListResponse(skillsResponse, input.cwd), @@ -872,6 +902,205 @@ export const readCodexSecuritySetup = (client: CodexClient.CodexAppServerClientS Effect.orElseSucceed(() => undefined), ); +/** Codex's default model provider, when it is not OpenAI's own. */ +function codexNonOpenAiDefaultProvider( + providers: CodexSchema.V2ModelProviderListResponse | undefined, +) { + const provider = providers?.data.find((entry) => entry.id === providers.defaultProviderId); + return provider && provider.settings.type !== "openai" && provider.id.trim() !== "" + ? provider + : undefined; +} + +/** + * What Codex's config says about the provider its chats run on: the model it + * names, and the provider's own definition. `config` is the answer to + * `config/read`, or undefined when Codex gave none. Undefined on OpenAI. + */ +export function codexGatewayConfigFromReads( + providers: CodexSchema.V2ModelProviderListResponse | undefined, + config: CodexSchema.V2ConfigReadResponse | undefined, +): CodexGatewayConfig | undefined { + const provider = codexNonOpenAiDefaultProvider(providers); + if (!provider) return undefined; + if (!config) return { providerId: provider.id, configRead: false }; + const model = optionalString(config.config.model); + const definitions: unknown = config.config.model_providers; + const definition = + typeof definitions === "object" && definitions !== null + ? (definitions as { readonly [id: string]: unknown })[provider.id] + : undefined; + return { + providerId: provider.id, + configRead: true, + ...(model ? { model } : {}), + ...(definition !== undefined && definition !== null ? { definition } : {}), + }; +} + +/** Reads Codex's config only when its chats run somewhere other than OpenAI. */ +const readCodexGatewayConfig = ( + client: CodexClient.CodexAppServerClientShape, + providers: CodexSchema.V2ModelProviderListResponse | undefined, +) => + codexNonOpenAiDefaultProvider(providers) === undefined + ? Effect.succeed(undefined) + : client.request("config/read", {}).pipe( + Effect.timeoutOption(CODEX_OPTIONAL_READ_TIMEOUT), + Effect.map(Option.getOrUndefined), + Effect.orElseSucceed(() => undefined), + Effect.map((config) => codexGatewayConfigFromReads(providers, config)), + ); + +/** + * The model list with the model Codex's config names, as the default. On a + * gateway Codex still lists OpenAI's models and marks one of them default, + * which the gateway may not serve; the config's model is the one known to + * work there. Added when the list lacks it, shown when the list hides it, + * and no longer a custom model when the user had added it by hand. + */ +export function withCodexConfigModel( + models: ReadonlyArray, + configModel: string | undefined, +): ReadonlyArray { + const slug = configModel?.trim(); + if (!slug) return models; + const withoutDefault = models.map(({ isDefault: _isDefault, ...model }) => model); + const index = withoutDefault.findIndex((model) => model.slug === slug); + if (index >= 0) { + return withoutDefault.map((model, position) => { + if (position !== index) return model; + // One the user also added by hand is Codex's own from here on: only a + // model that is not custom can be the default. + const { isHidden: _isHidden, ...shown } = model; + return { ...shown, isCustom: false, isDefault: true }; + }); + } + return [ + ...withoutDefault, + { + slug, + name: slug, + description: "The model set in Codex's config.", + isCustom: false, + isDefault: true, + capabilities: models.find((model) => model.capabilities)?.capabilities ?? null, + }, + ]; +} + +/** + * What the last status checks learned about the provider Codex's chats run + * on, for what reads it between checks: one-shot runs (titles, commit + * messages) and the model list. One per Codex instance. Read it through + * `currentCodexModelProvider`, which lets it lapse. + */ +export interface CodexModelProviderMemory { + /** When Codex last said which providers it has, whatever the answer. */ + answeredAtMs?: number | undefined; + last?: + | { + readonly providerId: string; + /** When Codex last named this provider as its default. */ + readonly confirmedAtMs: number; + readonly model?: string | undefined; + /** Absent when the config was never read, or the provider needs Codex's stored sign-in. */ + readonly launch?: CodexModelProviderLaunch | undefined; + } + | undefined; +} + +/** + * How long a gateway stays remembered without Codex naming it again: three + * scheduled checks. Past it, a Codex that can no longer be asked (an older + * version put back, a check that keeps failing) is not assumed to still be on + * a gateway the user may have removed. + */ +const CODEX_MODEL_PROVIDER_MEMORY_MS = 15 * 60_000; + +/** The remembered gateway, while Codex named it recently enough to go by. */ +export function currentCodexModelProvider( + memory: CodexModelProviderMemory, + nowMs: number, +): CodexModelProviderMemory["last"] { + const last = memory.last; + if (last && nowMs - last.confirmedAtMs > CODEX_MODEL_PROVIDER_MEMORY_MS) { + memory.last = undefined; + } + return memory.last; +} + +/** + * Whether Codex said recently enough which providers it has for the memory to + * be gone by. Not so right after a start, or while its status checks fail. + */ +export function isCodexModelProviderKnown( + memory: CodexModelProviderMemory, + nowMs: number, +): boolean { + return ( + memory.answeredAtMs !== undefined && + nowMs - memory.answeredAtMs <= CODEX_MODEL_PROVIDER_MEMORY_MS + ); +} + +/** + * Asks Codex only which provider its chats run on: two reads of its own + * config, with none of a status check's calls to OpenAI. For a title or + * commit message that cannot wait out a slow check and must not guess. + * Undefined when Codex does not say. + */ +export const probeCodexGateway = Effect.fn("probeCodexGateway")(function* (input: { + readonly binaryPath: string; + readonly homePath?: string; + readonly cwd: string; + readonly environment?: NodeJS.ProcessEnv; +}) { + const client = yield* makeCodexAppServerClient(input); + yield* initializeCodexAppServerClient(client); + const modelProviders = yield* readCodexModelProviders(client); + if (!modelProviders) return undefined; + const gateway = yield* readCodexGatewayConfig(client, modelProviders); + return { modelProviders, ...(gateway ? { gateway } : {}) } satisfies Pick< + CodexAppServerProviderSnapshot, + "modelProviders" | "gateway" + >; +}); + +/** + * The gateway a check goes by, remembered for the next one. A check that + * could not ask Codex which providers it has says nothing, so what was known + * stands for a while; one that got the provider but not its config keeps + * what was known of that same provider. Codex's own answer that it is on + * OpenAI forgets it at once. + */ +export function codexGatewayForCheck( + snapshot: Pick, + memory: CodexModelProviderMemory, + nowMs: number, +): CodexModelProviderMemory["last"] { + if (snapshot.modelProviders === undefined) return currentCodexModelProvider(memory, nowMs); + memory.answeredAtMs = nowMs; + const gateway = snapshot.gateway; + if (!gateway) { + memory.last = undefined; + } else if (gateway.configRead) { + memory.last = { + providerId: gateway.providerId, + confirmedAtMs: nowMs, + model: gateway.model, + launch: codexModelProviderLaunch(gateway), + }; + } else { + memory.last = { + ...(memory.last?.providerId === gateway.providerId ? memory.last : {}), + providerId: gateway.providerId, + confirmedAtMs: nowMs, + }; + } + return memory.last; +} + /** * Where new Codex chats run, when that is not OpenAI: the default model * provider of the user's Codex config, such as a company gateway. None for @@ -885,10 +1114,9 @@ export function codexModelProviderForCheck( if (!providers) return undefined; const accountType = account.account?.type; if (accountType === "amazonBedrock" || accountType === "gcp") return undefined; - const provider = providers.data.find((entry) => entry.id === providers.defaultProviderId); - if (!provider || provider.settings.type === "openai") return undefined; + const provider = codexNonOpenAiDefaultProvider(providers); + if (!provider) return undefined; const id = provider.id.trim(); - if (!id) return undefined; return { id, name: singleLine(provider.name, MAX_MODEL_PROVIDER_NAME) ?? id }; } @@ -1209,6 +1437,8 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu workspaceMemory?: CodexWorkspaceMemory, /** The instance's last security notice, kept the same way. */ securityNoticeMemory?: CodexSecurityNoticeMemory, + /** What the instance knows of the provider Codex is on, kept between checks. */ + modelProviderMemory: CodexModelProviderMemory = {}, ): Effect.fn.Return< ServerProviderDraft, ServerSettingsError, @@ -1294,6 +1524,11 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu } const snapshot = probeResult.success.value; + const gateway = codexGatewayForCheck( + snapshot, + modelProviderMemory, + yield* Clock.currentTimeMillis, + ); const accountStatus = accountProbeStatus(snapshot, workspaceMemory, securityNoticeMemory); const accountUsage = normalizeCodexAccountUsage( snapshot.rateLimits, @@ -1305,7 +1540,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu presentation: CODEX_PRESENTATION, enabled: codexSettings.enabled, checkedAt, - models: snapshot.models, + models: withCodexConfigModel(snapshot.models, gateway?.model), skills: snapshot.skills, probe: { installed: true, diff --git a/apps/server/src/provider/Layers/ProviderRegistry.test.ts b/apps/server/src/provider/Layers/ProviderRegistry.test.ts index a1e58db7e..8e958ba21 100644 --- a/apps/server/src/provider/Layers/ProviderRegistry.test.ts +++ b/apps/server/src/provider/Layers/ProviderRegistry.test.ts @@ -39,7 +39,10 @@ import { applyServerSettingsPatch } from "@threadlines/shared/serverSettings"; import { checkCodexProviderStatus, parseCodexModelListResponse, + codexGatewayConfigFromReads, + withCodexConfigModel, type CodexAppServerProviderSnapshot, + type CodexModelProviderMemory, type CodexSecurityNoticeMemory, type CodexWorkspaceMemory, } from "./CodexProvider.ts"; @@ -524,6 +527,210 @@ it.layer(Layer.mergeAll(NodeServices.layer, ServerSettingsService.layerTest(), T }), ); + it("makes the model Codex's config names the one default on a gateway", () => { + const catalog = [ + { slug: "gpt-6.1-sol", name: "GPT-6.1-Sol", isCustom: false, isDefault: true }, + { slug: "gpt-6-astra", name: "GPT-6-Astra", isCustom: false, isHidden: true }, + ].map((model) => ({ ...model, capabilities: null })); + + // Not in Codex's list: added, and the only default. + const added = withCodexConfigModel(catalog, " acme-coder-1 "); + assert.deepStrictEqual( + added.map((model) => [model.slug, model.isDefault === true, model.isCustom]), + [ + ["gpt-6.1-sol", false, false], + ["gpt-6-astra", false, false], + ["acme-coder-1", true, false], + ], + ); + + // Already listed, even hidden: shown, and the only default. + const listed = withCodexConfigModel(catalog, "gpt-6-astra"); + assert.deepStrictEqual( + listed.map((model) => [model.slug, model.isDefault === true, model.isHidden === true]), + [ + ["gpt-6.1-sol", false, false], + ["gpt-6-astra", true, false], + ], + ); + + // Added by hand before: Codex's own from here on, so it can be the default. + const custom = withCodexConfigModel( + [ + ...catalog, + { slug: "acme-coder-1", name: "acme-coder-1", isCustom: true, capabilities: null }, + ], + "acme-coder-1", + ); + assert.deepStrictEqual( + custom + .filter((model) => model.isDefault === true) + .map((model) => [model.slug, model.isCustom]), + [["acme-coder-1", false]], + ); + + // No model in the config: Codex's own list stands. + assert.strictEqual(withCodexConfigModel(catalog, undefined), catalog); + }); + + it("reads the gateway's model and definition out of Codex's config", () => { + const providers = { + canChangeDefault: true, + defaultProviderId: "acme", + data: [ + { + id: "acme", + name: "Acme Gateway", + canEdit: true, + settings: { + type: "customGateway" as const, + auth: { mode: "externallyConfigured" as const, envKey: null }, + }, + }, + { id: "openai", name: "OpenAI", canEdit: false, settings: { type: "openai" as const } }, + ], + }; + const definition = { + name: "Acme Gateway", + base_url: "https://llm.acme.example/v1", + env_key: null, + experimental_bearer_token: "sk-secret-token", + }; + // As `config/read` answers: the keys Codex's schema names, and the rest as JSON. + const config = { + config: { + model: "acme-coder-1", + model_provider: "acme", + model_providers: { acme: definition }, + }, + origins: {}, + }; + + assert.deepStrictEqual(codexGatewayConfigFromReads(providers, config), { + providerId: "acme", + configRead: true, + model: "acme-coder-1", + definition, + }); + // No answer from `config/read`: the provider is known, the rest is not. + assert.deepStrictEqual(codexGatewayConfigFromReads(providers, undefined), { + providerId: "acme", + configRead: false, + }); + // On OpenAI, or with no provider list at all, there is no gateway. + assert.strictEqual( + codexGatewayConfigFromReads({ ...providers, defaultProviderId: "openai" }, config), + undefined, + ); + assert.strictEqual(codexGatewayConfigFromReads(undefined, config), undefined); + }); + + it.effect("remembers the gateway until Codex itself says it is on OpenAI", () => + Effect.gen(function* () { + const modelProviderMemory: CodexModelProviderMemory = {}; + const check = (probe: Parameters[1]) => + checkCodexProviderStatus( + defaultCodexSettings, + probe, + {}, + undefined, + undefined, + modelProviderMemory, + ); + const providers = (defaultProviderId: string) => ({ + canChangeDefault: true, + defaultProviderId, + data: [ + { + id: "acme", + name: "Acme Gateway", + canEdit: true, + settings: { + type: "customGateway" as const, + auth: { mode: "environmentVariable" as const, envKey: "ACME_API_KEY" }, + }, + }, + { + id: "openai", + name: "OpenAI", + canEdit: false, + settings: { type: "openai" as const }, + }, + ], + }); + const defaultModels = (status: Pick) => + status.models.filter((model) => model.isDefault === true).map((model) => model.slug); + const onGateway = makeCodexProbeSnapshot({ + modelProviders: providers("acme"), + gateway: { + providerId: "acme", + configRead: true, + model: "acme-coder-1", + definition: { base_url: "https://llm.acme.example/v1", env_key: "ACME_API_KEY" }, + }, + }); + + const first = yield* check(() => Effect.succeed(onGateway)); + assert.deepStrictEqual(defaultModels(first), ["acme-coder-1"]); + assert.strictEqual( + modelProviderMemory.last?.launch?.config, + [ + 'model_provider = "acme"', + 'model_providers = {"acme" = {"base_url" = "https://llm.acme.example/v1", "env_key" = "ACME_API_KEY"}}', + "", + ].join("\n"), + ); + + // Codex did not say which providers it has: nothing is known to have changed. + const noProviderList = yield* check(() => Effect.succeed(makeCodexProbeSnapshot())); + assert.deepStrictEqual(defaultModels(noProviderList), ["acme-coder-1"]); + assert.strictEqual(modelProviderMemory.last?.launch?.providerId, "acme"); + + // It named the gateway but did not answer with its config: what was + // known of that gateway stands. + const noConfig = yield* check(() => + Effect.succeed( + makeCodexProbeSnapshot({ + modelProviders: providers("acme"), + gateway: { providerId: "acme", configRead: false }, + }), + ), + ); + assert.deepStrictEqual(defaultModels(noConfig), ["acme-coder-1"]); + assert.strictEqual(modelProviderMemory.last?.launch?.providerId, "acme"); + + // A check that could not reach Codex at all says nothing either. + yield* check(() => + Effect.fail( + new CodexErrors.CodexAppServerSpawnError({ command: "codex", cause: "missing" }), + ), + ); + assert.strictEqual(modelProviderMemory.last?.launch?.providerId, "acme"); + + // Codex's own answer that it is on OpenAI forgets the gateway. + const onOpenAi = yield* check(() => + Effect.succeed(makeCodexProbeSnapshot({ modelProviders: providers("openai") })), + ); + assert.strictEqual(modelProviderMemory.last, undefined); + assert.strictEqual( + onOpenAi.models.some((model) => model.slug === "acme-coder-1"), + false, + ); + + // A gateway Codex has not named for a while is not assumed to be + // there still: the user may have removed it behind a Codex that can + // no longer be asked. + yield* check(() => Effect.succeed(onGateway)); + yield* TestClock.adjust("16 minutes"); + const lapsed = yield* check(() => Effect.succeed(makeCodexProbeSnapshot())); + assert.strictEqual(modelProviderMemory.last, undefined); + assert.strictEqual( + lapsed.models.some((model) => model.slug === "acme-coder-1"), + false, + ); + }), + ); + it.effect("shows OpenAI's security notice only to the account it is for", () => Effect.gen(function* () { const securityNoticeMemory: CodexSecurityNoticeMemory = {}; diff --git a/apps/server/src/provider/codexModelProviderLaunch.test.ts b/apps/server/src/provider/codexModelProviderLaunch.test.ts new file mode 100644 index 000000000..e74234475 --- /dev/null +++ b/apps/server/src/provider/codexModelProviderLaunch.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { + codexModelProviderLaunch, + redactCodexProviderSecrets, +} from "./codexModelProviderLaunch.ts"; + +describe("codexModelProviderLaunch", () => { + it("writes a config with the provider alone, its definition whole", () => { + const launch = codexModelProviderLaunch({ + providerId: "acme.gw v2", + model: "acme-coder-1", + definition: { + name: 'Acme "GW"', + base_url: "https://llm.acme.example/v1", + wire_api: "responses", + env_key: null, + model_catalog_url: null, + experimental_bearer_token: "sk-secret-token", + http_headers: { "X-Org": "org-secret" }, + query_params: { "api-version": "2026-01-01", code: "fn-key-secret" }, + request_max_retries: 2, + requires_openai_auth: false, + gateway_oauth: null, + auth: { command: "acme-token", args: ["--audience", "llm"] }, + }, + }); + + expect(launch).toEqual({ + providerId: "acme.gw v2", + model: "acme-coder-1", + config: [ + 'model_provider = "acme.gw v2"', + 'model_providers = {"acme.gw v2" = {"name" = "Acme \\"GW\\"", "base_url" = "https://llm.acme.example/v1", "wire_api" = "responses", "experimental_bearer_token" = "sk-secret-token", "http_headers" = {"X-Org" = "org-secret"}, "query_params" = {"api-version" = "2026-01-01", "code" = "fn-key-secret"}, "request_max_retries" = 2, "requires_openai_auth" = false, "auth" = {"command" = "acme-token", "args" = ["--audience", "llm"]}}}', + "", + ].join("\n"), + // Longest first, so one is never struck out from inside another. + secrets: ["sk-secret-token", "fn-key-secret", "org-secret", "2026-01-01"], + }); + }); + + // A run's own home has no sign-in: these providers use what the user's + // Codex home stores, so their runs stay as they were. + it("gives no launch for a provider that needs Codex's stored sign-in", () => { + for (const definition of [ + { base_url: "https://proxy.acme.example/v1", requires_openai_auth: true }, + { base_url: "https://llm.acme.example/v1", gateway_oauth: { issuer: "https://id.acme" } }, + { aws: { profile: "work", region: "us-east-1" } }, + { project_id: "acme-ml", location: "us-central1" }, + ]) { + expect(codexModelProviderLaunch({ providerId: "acme", definition })).toBeUndefined(); + } + }); + + // No definition to hand over: it runs in the user's own home, where any + // sign-in Codex keeps for it is, named beside --ignore-user-config. + it("names a built-in provider by its id alone, as an argument", () => { + expect(codexModelProviderLaunch({ providerId: "ollama" })).toEqual({ + providerId: "ollama", + args: ["-c", 'model_provider="ollama"'], + secrets: [], + }); + expect(codexModelProviderLaunch({ providerId: 'odd" & id' })).toBeUndefined(); + }); + + // Codex's errors repeat the address of the request that failed. + it("strikes the provider's secrets from text, also where an address encodes them", () => { + const launch = codexModelProviderLaunch({ + providerId: "acme", + definition: { + base_url: "https://user:p%40ssw0rd@llm.acme.example/v1?sig=abc123def", + query_params: { code: "fn key/secret" }, + experimental_bearer_token: "sk-secret-token", + }, + }); + expect( + redactCodexProviderSecrets( + "ERROR: 401 for https://llm.acme.example/v1/responses?code=fn%20key%2Fsecret&sig=abc123def (Bearer sk-secret-token, p@ssw0rd)", + launch, + ), + ).toBe( + "ERROR: 401 for https://llm.acme.example/v1/responses?code=[redacted]&sig=[redacted] (Bearer [redacted], [redacted])", + ); + expect(redactCodexProviderSecrets("ERROR: nothing to strike", undefined)).toBe( + "ERROR: nothing to strike", + ); + }); +}); diff --git a/apps/server/src/provider/codexModelProviderLaunch.ts b/apps/server/src/provider/codexModelProviderLaunch.ts new file mode 100644 index 000000000..b3f43ea4a --- /dev/null +++ b/apps/server/src/provider/codexModelProviderLaunch.ts @@ -0,0 +1,204 @@ +/** + * Points a one-shot `codex exec` at the model provider the user's Codex runs + * on, when that is not OpenAI (a company gateway, say). + * + * Titles, commit messages and the like run `codex exec --ignore-user-config`, + * so that the user's MCP servers, notify hooks and memories stay out of a + * one-line answer. That also leaves out `model_provider`, and the run goes to + * OpenAI whatever Codex's chats use. Loading the whole config back in brings + * the MCP servers with it (overriding `mcp_servers` on the command line does + * not stop them). + * + * So the run gets a Codex home of its own, holding one file: a config with + * the provider alone, its id and its definition as Codex's own `config/read` + * reports it. A definition can hold secrets (a token, headers, a key in its + * address), which is why it travels in a file only its owner can read and + * never as `-c` arguments, which show in the process list. + * + * A home of its own has no sign-in. A provider that signs in with what + * Codex's home stores (`requires_openai_auth`, a gateway's OAuth, Amazon + * Bedrock, Google Cloud) therefore gets no launch, and its runs go to OpenAI + * as they did before. A provider Codex has built in has no definition to + * hand over: it runs in the user's home, named by its id alone. + * + * @module codexModelProviderLaunch + */ + +/** What a `codex exec` run needs to use the provider: `config` or `args`, never both. */ +export interface CodexModelProviderLaunch { + readonly providerId: string; + /** The model Codex's config names: the one known to work on this provider. */ + readonly model?: string; + /** + * The text of the `config.toml` for a home of the run's own. It can hold + * the provider's secrets: keep it out of logs, arguments and anything sent + * to a client. + */ + readonly config?: string; + /** + * For a provider Codex has built in: arguments beside `--ignore-user-config` + * in the user's own home. Only the provider's id, never a secret. + */ + readonly args?: ReadonlyArray; + /** + * The definition's values that may be secret (its token, headers, query + * parameters, a password in its address). Codex's error output can repeat + * them, in a request's address say: strike them from anything shown or + * logged with `redactCodexProviderSecrets`. + */ + readonly secrets: ReadonlyArray; +} + +type JsonRecord = { readonly [key: string]: unknown }; + +function isRecord(value: unknown): value is JsonRecord { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * A TOML basic string. JSON's escapes are all TOML's; DEL is the one control + * character JSON leaves bare and TOML refuses. + */ +function tomlString(value: string): string { + return JSON.stringify(value).replaceAll("\u007f", "\\u007F"); +} + +/** + * A JSON value as inline TOML, or undefined for what TOML cannot hold (null, + * a number that is not finite). Tables and arrays leave such entries out. + */ +function tomlValue(value: unknown): string | undefined { + switch (typeof value) { + case "string": + return tomlString(value); + case "boolean": + return value ? "true" : "false"; + case "number": + return Number.isFinite(value) ? String(value) : undefined; + case "object": { + if (value === null) return undefined; + if (Array.isArray(value)) { + const items = value.flatMap((item) => tomlValue(item) ?? []); + return `[${items.join(", ")}]`; + } + const entries = Object.entries(value).flatMap(([key, entry]) => { + const encoded = tomlValue(entry); + return encoded === undefined ? [] : [`${tomlString(key)} = ${encoded}`]; + }); + return `{${entries.join(", ")}}`; + } + default: + return undefined; + } +} + +/** + * Whether the provider signs in with what the user's own Codex home stores: + * a ChatGPT or API-key sign-in, a gateway's OAuth tokens, or the Amazon + * Bedrock or Google Cloud credentials Codex keeps there. A home made for one + * run holds none of them. + */ +function needsStoredSignIn(definition: JsonRecord): boolean { + const set = (value: unknown) => value !== undefined && value !== null; + return ( + definition.requires_openai_auth === true || + set(definition.gateway_oauth) || + set(definition.aws) || + set(definition.project_id) || + set(definition.location) || + set(definition.credential_file) + ); +} + +/** Shorter than this, a value is not a secret worth striking, and striking it would mangle text. */ +const MIN_SECRET_LENGTH = 6; + +function stringValues(table: unknown): ReadonlyArray { + return isRecord(table) + ? Object.values(table).filter((value): value is string => typeof value === "string") + : []; +} + +/** A password, user name or query values carried in an address. */ +function addressSecrets(value: unknown): ReadonlyArray { + if (typeof value !== "string") return []; + try { + const url = new URL(value); + return [ + decodeURIComponent(url.username), + decodeURIComponent(url.password), + ...url.searchParams.values(), + ]; + } catch { + return []; + } +} + +/** The definition's values that may be secret, longest first so one never hides inside another. */ +function definitionSecrets(definition: JsonRecord): ReadonlyArray { + const candidates = [ + ...(typeof definition.experimental_bearer_token === "string" + ? [definition.experimental_bearer_token] + : []), + ...stringValues(definition.http_headers), + ...stringValues(definition.query_params), + ...addressSecrets(definition.base_url), + ...addressSecrets(definition.model_catalog_url), + ]; + return [...new Set(candidates.filter((value) => value.length >= MIN_SECRET_LENGTH))].toSorted( + (left, right) => right.length - left.length, + ); +} + +/** + * Text with the provider's secrets struck out, also where they appear + * percent-encoded, as in the address of a failed request. + */ +export function redactCodexProviderSecrets( + text: string, + launch: Pick | undefined, +): string { + let redacted = text; + for (const secret of launch?.secrets ?? []) { + for (const form of new Set([secret, encodeURIComponent(secret)])) { + redacted = redacted.replaceAll(form, "[redacted]"); + } + } + return redacted; +} + +/** A built-in provider's id, made of plain characters: safe as an argument on any platform. */ +const PLAIN_PROVIDER_ID = /^[A-Za-z0-9_-]+$/u; + +/** + * What a `codex exec` run on provider `providerId` needs. `definition` is the + * provider's entry in `config/read`'s `model_providers`; leave it out for a + * provider Codex has built in. Undefined when the provider needs the sign-in + * stored in the user's own Codex home. + */ +export function codexModelProviderLaunch(input: { + readonly providerId: string; + readonly model?: string | undefined; + readonly definition?: unknown; +}): CodexModelProviderLaunch | undefined { + const base = { + providerId: input.providerId, + ...(input.model ? { model: input.model } : {}), + }; + if (!isRecord(input.definition)) { + return PLAIN_PROVIDER_ID.test(input.providerId) + ? { ...base, args: ["-c", `model_provider=${tomlString(input.providerId)}`], secrets: [] } + : undefined; + } + if (needsStoredSignIn(input.definition)) { + return undefined; + } + // The whole table on one line, keyed by the quoted id: a `[model_providers.id]` + // header would split an id that has a dot in it. + const table = `{${tomlString(input.providerId)} = ${tomlValue(input.definition) ?? "{}"}}`; + return { + ...base, + config: `model_provider = ${tomlString(input.providerId)}\nmodel_providers = ${table}\n`, + secrets: definitionSecrets(input.definition), + }; +} diff --git a/apps/server/src/provider/providerStatusCache.test.ts b/apps/server/src/provider/providerStatusCache.test.ts index 7422be5cd..79da99414 100644 --- a/apps/server/src/provider/providerStatusCache.test.ts +++ b/apps/server/src/provider/providerStatusCache.test.ts @@ -243,6 +243,46 @@ it.layer(NodeServices.layer)("providerStatusCache", (it) => { ); }); + // On a gateway, Codex's default is the model its config names, which the + // user may also have added by hand. Before the first check the fallback + // holds only that custom entry: the cached default must not be lost to it. + it("keeps a cached default that shares its slug with a custom model", () => { + const cachedCodex = makeProvider(CODEX_DRIVER, { + models: [ + { slug: "gpt-5.4", name: "GPT-5.4", isCustom: false, capabilities: emptyCapabilities }, + { + slug: "acme-coder-1", + name: "acme-coder-1", + isCustom: false, + isDefault: true, + capabilities: emptyCapabilities, + }, + ], + }); + const fallbackCodex = makeProvider(CODEX_DRIVER, { + models: [ + { + slug: "acme-coder-1", + name: "acme-coder-1", + isCustom: true, + capabilities: emptyCapabilities, + }, + ], + }); + + const hydrated = hydrateCachedProvider({ + cachedProvider: cachedCodex, + fallbackProvider: fallbackCodex, + }); + assert.deepStrictEqual( + hydrated.models.map((model) => [model.slug, model.isDefault === true, model.isCustom]), + [ + ["acme-coder-1", true, false], + ["gpt-5.4", false, false], + ], + ); + }); + it("ignores stale cached enabled state when the provider is now disabled", () => { const cachedCodex = makeProvider(CODEX_DRIVER, { checkedAt: "2026-04-10T12:00:00.000Z", diff --git a/apps/server/src/provider/providerStatusCache.ts b/apps/server/src/provider/providerStatusCache.ts index b0c3b7b3b..ec8daac66 100644 --- a/apps/server/src/provider/providerStatusCache.ts +++ b/apps/server/src/provider/providerStatusCache.ts @@ -21,7 +21,16 @@ const mergeProviderModels = ( cachedModels: ReadonlyArray, ): ReadonlyArray => { const fallbackSlugs = new Set(fallbackModels.map((model) => model.slug)); - return [...fallbackModels, ...cachedModels.filter((model) => !fallbackSlugs.has(model.slug))]; + // The fallback names no default of its own (Codex's is just the user's + // custom models): a cached default with the same slug as one of them stays + // the default, or the list would come back with none until the first check. + const cachedDefault = fallbackModels.some((model) => model.isDefault === true) + ? undefined + : cachedModels.find((model) => model.isDefault === true && fallbackSlugs.has(model.slug)); + return [ + ...fallbackModels.map((model) => (model.slug === cachedDefault?.slug ? cachedDefault : model)), + ...cachedModels.filter((model) => !fallbackSlugs.has(model.slug)), + ]; }; export const orderProviderSnapshots = ( diff --git a/apps/server/src/textGeneration/CodexTextGeneration.test.ts b/apps/server/src/textGeneration/CodexTextGeneration.test.ts index d640b13a7..2f02ea444 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.test.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.test.ts @@ -20,6 +20,7 @@ import { import { ServerConfig } from "../config.ts"; import { type TextGenerationShape } from "./TextGeneration.ts"; import { makeCodexTextGeneration } from "./CodexTextGeneration.ts"; +import { codexModelProviderLaunch } from "../provider/codexModelProviderLaunch.ts"; const decodeCodexSettings = Schema.decodeSync(CodexSettings); const DEFAULT_TEST_MODEL_SELECTION = createModelSelection( @@ -56,6 +57,7 @@ function makeFakeCodexBinary( failForModelExitCode?: number; stdinMustContain?: string; stdinMustNotContain?: string; + recordInvocationTo?: string; }, ) { return Effect.gen(function* () { @@ -268,6 +270,20 @@ function makeFakeCodexBinary( codexPath, [ "#!/bin/sh", + ...(input.recordInvocationTo !== undefined + ? [ + // The arguments one per line, the home the run was given, its + // config, and who may read them. + // @effect-diagnostics-next-line preferSchemaOverJson:off + `printf "%s\n" "$@" > ${JSON.stringify(`${input.recordInvocationTo}.args`)}`, + // @effect-diagnostics-next-line preferSchemaOverJson:off + `printf "%s" "$CODEX_HOME" > ${JSON.stringify(`${input.recordInvocationTo}.home`)}`, + // @effect-diagnostics-next-line preferSchemaOverJson:off + `cat "$CODEX_HOME/config.toml" > ${JSON.stringify(`${input.recordInvocationTo}.config`)}`, + // @effect-diagnostics-next-line preferSchemaOverJson:off + `ls -ld "$CODEX_HOME" "$CODEX_HOME/config.toml" | cut -c1-10 > ${JSON.stringify(`${input.recordInvocationTo}.modes`)}`, + ] + : []), 'output_path=""', 'seen_image="0"', 'seen_fast_service_tier="0"', @@ -471,15 +487,18 @@ function withFakeCodexEnv( failForModelExitCode?: number; stdinMustContain?: string; stdinMustNotContain?: string; + /** POSIX only: writes the run's arguments, home, config and their modes to `.*`. */ + recordInvocationTo?: string; }, effectFn: (textGeneration: TextGenerationShape) => Effect.Effect, + options?: Parameters[2], ) { return Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const tempDir = yield* fs.makeTempDirectoryScoped({ prefix: "threadlines-codex-text-" }); const codexPath = yield* makeFakeCodexBinary(tempDir, input); const config = decodeCodexSettings({ binaryPath: codexPath }); - const textGeneration = yield* makeCodexTextGeneration(config); + const textGeneration = yield* makeCodexTextGeneration(config, process.env, options); return yield* effectFn(textGeneration); }).pipe(Effect.scoped); } @@ -673,6 +692,67 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGeneration", (it) => { ), ); + // The built-in pick for titles is an OpenAI model, which a gateway may not + // serve. The model Codex's own config names is the one known to work there. + it.effect("falls back to the gateway's own model when the picked one is refused", () => + withFakeCodexEnv( + { + output: JSON.stringify({ title: "Investigate reconnect regressions" }), + failForModel: DEFAULT_GIT_TEXT_GENERATION_MODEL, + failForModelStderr: "ERROR: unknown model", + requireModel: "acme-coder-1", + }, + (textGeneration) => + Effect.gen(function* () { + const generated = yield* textGeneration.generateThreadTitle({ + cwd: process.cwd(), + message: "Please investigate websocket reconnect regressions.", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }); + + expect(generated.title).toBe("Investigate reconnect regressions"); + }), + { + modelProviderLaunch: Effect.succeed( + codexModelProviderLaunch({ providerId: "acme", model: "acme-coder-1" }), + ), + }, + ), + ); + + // Another model would not help a failed sign-in: the failure is reported as + // it is. Codex prints the prompt above its error, and a line of the prompt + // that reads like a model error is not one. + it.effect("does not try the gateway's model for a failure that names no model", () => + withFakeCodexEnv( + { + output: JSON.stringify({ title: "unused" }), + failForModel: DEFAULT_GIT_TEXT_GENERATION_MODEL, + failForModelStderr: "user\nERROR: model cache is corrupt\nERROR: 401 Unauthorized", + }, + (textGeneration) => + Effect.gen(function* () { + const result = yield* textGeneration + .generateThreadTitle({ + cwd: process.cwd(), + message: "Please investigate websocket reconnect regressions.", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }) + .pipe(Effect.result); + + expect(Result.isFailure(result)).toBe(true); + if (Result.isFailure(result)) { + expect(result.failure.detail).toContain("401 Unauthorized"); + } + }), + { + modelProviderLaunch: Effect.succeed( + codexModelProviderLaunch({ providerId: "acme", model: "acme-coder-1" }), + ), + }, + ), + ); + it.effect("generates commit message with branch when includeBranch is true", () => withFakeCodexEnv( { @@ -769,6 +849,134 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGeneration", (it) => { ), ); + // Titles and commit messages leave the user's Codex config out, and with it + // the gateway Codex runs on. The run gets a home of its own with the gateway + // alone, in a file only this user can read: its secrets are never arguments. + it.effect.skipIf(process.platform === "win32")( + "runs on the gateway Codex is configured for, from a private home of its own", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const recordDir = yield* fs.makeTempDirectoryScoped({ prefix: "threadlines-codex-args-" }); + const record = path.join(recordDir, "run"); + const launch = codexModelProviderLaunch({ + providerId: "acme", + definition: { + name: "Acme Gateway", + base_url: "https://llm.acme.example/v1", + experimental_bearer_token: "sk-secret-token", + query_params: { code: "fn-key" }, + }, + }); + yield* withFakeCodexEnv( + { + output: JSON.stringify({ title: "Investigate reconnect regressions" }), + recordInvocationTo: record, + }, + (textGeneration) => + textGeneration.generateThreadTitle({ + cwd: process.cwd(), + message: "Please investigate websocket reconnect regressions.", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }), + { modelProviderLaunch: Effect.succeed(launch) }, + ); + + const args = yield* fs.readFileString(`${record}.args`); + expect(args).not.toContain("sk-secret-token"); + expect(args).not.toContain("fn-key"); + expect(args).not.toContain("acme"); + // The home holds no user config to ignore. + expect(args.split("\n")).not.toContain("--ignore-user-config"); + expect(yield* fs.readFileString(`${record}.config`)).toBe(launch?.config); + expect((yield* fs.readFileString(`${record}.modes`)).split("\n").slice(0, 2)).toEqual([ + "drwx------", + "-rw-------", + ]); + // Gone with the run. + const home = yield* fs.readFileString(`${record}.home`); + expect(home).not.toBe(""); + expect(yield* fs.exists(home)).toBe(false); + }).pipe(Effect.scoped), + ); + + // Codex's error names the address of the request that failed, and this text + // is logged and shown to the user. + it.effect("strikes the gateway's secrets from a failed run's error", () => + withFakeCodexEnv( + { + output: JSON.stringify({ title: "unused" }), + exitCode: 1, + stderr: + "ERROR: unexpected status 503 for https://llm.acme.example/v1/responses?code=fn-key-secret", + }, + (textGeneration) => + Effect.gen(function* () { + const result = yield* textGeneration + .generateThreadTitle({ + cwd: process.cwd(), + message: "Please investigate websocket reconnect regressions.", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }) + .pipe(Effect.result); + + expect(Result.isFailure(result)).toBe(true); + if (Result.isFailure(result)) { + expect(result.failure.detail).toContain("responses?code=[redacted]"); + expect(result.failure.detail).not.toContain("fn-key-secret"); + } + }), + { + modelProviderLaunch: Effect.succeed( + codexModelProviderLaunch({ + providerId: "acme", + definition: { + base_url: "https://llm.acme.example/v1", + query_params: { code: "fn-key-secret" }, + }, + }), + ), + }, + ), + ); + + // A provider Codex has built in has no definition to hand over, and may + // sign in with what Codex's own home stores: it runs there, named by id. + it.effect.skipIf(process.platform === "win32")( + "runs a built-in provider in the user's own home, named beside --ignore-user-config", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const recordDir = yield* fs.makeTempDirectoryScoped({ prefix: "threadlines-codex-args-" }); + const record = path.join(recordDir, "run"); + yield* withFakeCodexEnv( + { + output: JSON.stringify({ title: "Investigate reconnect regressions" }), + requireIgnoreUserConfig: true, + recordInvocationTo: record, + }, + (textGeneration) => + textGeneration.generateThreadTitle({ + cwd: process.cwd(), + message: "Please investigate websocket reconnect regressions.", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }), + { + modelProviderLaunch: Effect.succeed(codexModelProviderLaunch({ providerId: "ollama" })), + }, + ); + + const args = (yield* fs.readFileString(`${record}.args`)).split("\n"); + expect(args).toContain('model_provider="ollama"'); + // No home of its own was made for it. + expect(yield* fs.readFileString(`${record}.home`)).not.toContain( + "threadlines-side-textgen-", + ); + }).pipe(Effect.scoped), + ); + it.effect("retries without --ignore-user-config when the codex CLI rejects the flag", () => withFakeCodexEnv( { diff --git a/apps/server/src/textGeneration/CodexTextGeneration.ts b/apps/server/src/textGeneration/CodexTextGeneration.ts index d4e1c5efb..aa8816332 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.ts @@ -20,6 +20,11 @@ import { sanitizeBranchFragment, sanitizeFeatureBranchName } from "@threadlines/ import { resolveAttachmentPath } from "../attachmentStore.ts"; import { ServerConfig } from "../config.ts"; import { expandHomePath } from "../pathExpansion.ts"; +import { + redactCodexProviderSecrets, + type CodexModelProviderLaunch, +} from "../provider/codexModelProviderLaunch.ts"; +import { sweepStaleCodexSideAnswerHomes } from "../provider/codexSideAnswerHome.ts"; import { TextGenerationError } from "@threadlines/contracts"; import { type BranchNameGenerationInput, @@ -66,6 +71,8 @@ type RunCodexJsonInput = { imagePaths?: ReadonlyArray; cleanupPaths?: ReadonlyArray; modelSelection: ModelSelection; + /** The gateway to run on, asked once for the whole generation. */ + providerLaunch?: CodexModelProviderLaunch | undefined; }; function isCodexModelCapacityError(error: TextGenerationError): boolean { @@ -76,6 +83,22 @@ function isCodexModelCapacityError(error: TextGenerationError): boolean { ); } +/** + * Whether Codex ran and the service refused the model: its closing `ERROR:` + * line names the model. A failed sign-in or a broken config ends on another + * line, and another model would not help those. Only the last line counts: + * Codex prints the prompt above it, which can hold anything. + */ +function isCodexModelRefusal(error: TextGenerationError): boolean { + const prefix = "Codex CLI command failed: "; + if (!error.detail.startsWith(prefix)) return false; + const lastLine = error.detail.slice(prefix.length).trimEnd().split("\n").at(-1)?.trim() ?? ""; + return /^ERROR:.*\bmodel\b/iu.test(lastLine); +} + +/** A model id made of the characters model ids are made of, safe as an argument on any platform. */ +const PLAIN_MODEL_NAME = /^[A-Za-z0-9_\-+=.,:/@]+$/u; + function isUnsupportedIgnoreUserConfigFlagError(error: TextGenerationError): boolean { const detail = error.detail.toLowerCase(); return ( @@ -105,6 +128,14 @@ function withDefaultCodexModel(modelSelection: ModelSelection): ModelSelection { export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(function* ( codexConfig: CodexSettings, environment: NodeJS.ProcessEnv = process.env, + textGenerationOptions?: { + /** + * The provider Codex's chats run on when it is not OpenAI, as the last + * status check found it, asked at each run. Without it a run goes to + * OpenAI, which is where `--ignore-user-config` leaves it. + */ + readonly modelProviderLaunch?: Effect.Effect; + }, ) { const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -204,6 +235,41 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func return { imagePaths }; }); + /** + * A Codex home for one run on a gateway: a folder only this user can open, + * with the gateway's config in it, removed when the run's scope closes. It + * is named like a side-answer home so that the sweep of those, run here + * before the first one is made, also removes any a killed server left. + */ + let sweptGatewayHomes = false; + const makeGatewayHome = ( + operation: CodexTextGenerationOperation, + config: string, + ): Effect.Effect => + Effect.gen(function* () { + if (!sweptGatewayHomes) { + sweptGatewayHomes = true; + yield* sweepStaleCodexSideAnswerHomes.pipe(Effect.forkDetach); + } + const home = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-side-textgen-", + }); + yield* fileSystem.chmod(home, 0o700); + yield* fileSystem.writeFileString(path.join(home, "config.toml"), config, { + mode: 0o600, + }); + return home; + }).pipe( + Effect.mapError( + (cause) => + new TextGenerationError({ + operation, + detail: "Failed to prepare a Codex home for the gateway", + cause, + }), + ), + ); + const runCodexJson = Effect.fn("runCodexJson")(function* ({ operation, cwd, @@ -212,6 +278,7 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func imagePaths = [], cleanupPaths = [], modelSelection, + providerLaunch: gatewayLaunch, }: RunCodexJsonInput): Effect.fn.Return< S["Type"], TextGenerationError, @@ -237,9 +304,20 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func getModelSelectionStringOptionValue(modelSelection, "reasoningEffort") ?? DEFAULT_GIT_TEXT_GENERATION_REASONING_EFFORT; const serviceTier = resolveCodexCliServiceTier(modelSelection); + // On a gateway the run gets a home of its own, holding the provider + // alone (see codexModelProviderLaunch.ts). That stands in for + // --ignore-user-config: there is no user config in it to ignore. + const gatewayHome = + options.ignoreUserConfig && gatewayLaunch?.config !== undefined + ? yield* makeGatewayHome(operation, gatewayLaunch.config) + : undefined; const env = { ...environment, - ...(codexConfig.homePath ? { CODEX_HOME: expandHomePath(codexConfig.homePath) } : {}), + ...(gatewayHome !== undefined + ? { CODEX_HOME: gatewayHome } + : codexConfig.homePath + ? { CODEX_HOME: expandHomePath(codexConfig.homePath) } + : {}), }; const plan = planCliSpawn( codexConfig.binaryPath || "codex", @@ -249,7 +327,11 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func // One-shot text generation should not inherit agent-oriented user // config (MCP servers, notify hooks, memories) from // $CODEX_HOME/config.toml; auth still uses CODEX_HOME. - ...(options.ignoreUserConfig ? ["--ignore-user-config"] : []), + // A built-in provider is named beside the flag, in the user's own + // home: it has no definition to give a home of its own. + ...(options.ignoreUserConfig && gatewayHome === undefined + ? ["--ignore-user-config", ...(gatewayLaunch?.args ?? [])] + : []), "--skip-git-repo-check", ...(operation === "generateCommitMessage" ? ["--ignore-rules"] : []), "-s", @@ -305,7 +387,12 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func if (exitCode !== 0) { const stderrDetail = stderr.trim(); const stdoutDetail = stdout.trim(); - const detail = stderrDetail.length > 0 ? stderrDetail : stdoutDetail; + // Codex's errors can repeat a gateway's address, and with it a key the + // address carries. This text is logged and shown to the user. + const detail = redactCodexProviderSecrets( + stderrDetail.length > 0 ? stderrDetail : stdoutDetail, + gatewayLaunch, + ); return yield* new TextGenerationError({ operation, detail: @@ -374,7 +461,40 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func >( input: RunCodexJsonInput, ): Effect.fn.Return { - return yield* runCodexJson(input).pipe( + const providerLaunch = textGenerationOptions?.modelProviderLaunch + ? yield* textGenerationOptions.modelProviderLaunch + : undefined; + const run = { ...input, providerLaunch }; + // On a gateway, the model picked for titles and commit messages may be one + // only OpenAI serves (the built-in pick is). The model Codex's config names + // is known to work there, so a run refused for its model is tried once + // more on that one. + // The name comes from the user's Codex config and becomes an argument: + // one with anything but plain characters is left alone. + const gatewayModel = + providerLaunch?.model !== undefined && PLAIN_MODEL_NAME.test(providerLaunch.model) + ? providerLaunch.model + : undefined; + return yield* runCodexJson(run).pipe( + Effect.catchIf( + (error) => + gatewayModel !== undefined && + input.modelSelection.model !== gatewayModel && + isCodexModelRefusal(error), + () => + Effect.logWarning("retrying Codex text generation with the gateway's model", { + operation: input.operation, + failedModel: input.modelSelection.model, + fallbackModel: gatewayModel, + }).pipe( + Effect.andThen( + runCodexJson({ + ...run, + modelSelection: { ...input.modelSelection, model: gatewayModel ?? DEFAULT_MODEL }, + }), + ), + ), + ), Effect.catchIf( (error) => shouldRetryWithDefaultCodexModel(input.modelSelection, error), () => @@ -385,7 +505,7 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func }).pipe( Effect.andThen( runCodexJson({ - ...input, + ...run, modelSelection: withDefaultCodexModel(input.modelSelection), }), ), diff --git a/apps/web/src/components/settings/ProviderInstanceCard.test.ts b/apps/web/src/components/settings/ProviderInstanceCard.test.ts index 7c35dae55..93a464914 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.test.ts +++ b/apps/web/src/components/settings/ProviderInstanceCard.test.ts @@ -47,6 +47,24 @@ describe("deriveProviderModelsForDisplay", () => { }).map((model) => model.slug), ).toEqual(["server-model", "kept-custom"]); }); + + // Codex on a gateway lists the model from its own config, which the user + // may have added by hand before: one row, the provider's. + it("shows a model the provider lists itself once, not again as a custom one", () => { + const liveModels: ReadonlyArray = [ + { slug: "acme-coder-1", name: "acme-coder-1", isCustom: false, capabilities: null }, + ]; + + expect( + deriveProviderModelsForDisplay({ + liveModels, + customModels: ["acme-coder-1", "acme-coder-mini"], + }).map((model) => [model.slug, model.isCustom]), + ).toEqual([ + ["acme-coder-1", false], + ["acme-coder-mini", true], + ]); + }); }); describe("Claude credential preference helpers", () => { diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index 54eff9d23..187fa42fd 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -211,15 +211,20 @@ export function deriveProviderModelsForDisplay(input: { .map((model) => [model.slug, model] as const), ); const serverModels = input.liveModels?.filter((model) => !model.isCustom) ?? []; - const customModels = input.customModels.map( - (slug) => - liveCustomModelsBySlug.get(slug) ?? { - slug, - name: slug, - isCustom: true, - capabilities: null, - }, - ); + // A model the provider now lists itself (Codex's own config model, say) is + // shown once, as the provider's: the user's entry of the same id is spent. + const serverSlugs = new Set(serverModels.map((model) => model.slug)); + const customModels = input.customModels + .filter((slug) => !serverSlugs.has(slug)) + .map( + (slug) => + liveCustomModelsBySlug.get(slug) ?? { + slug, + name: slug, + isCustom: true, + capabilities: null, + }, + ); return [...serverModels, ...customModels]; } diff --git a/docs/providers/codex.md b/docs/providers/codex.md index 3beb096ee..ca9c10315 100644 --- a/docs/providers/codex.md +++ b/docs/providers/codex.md @@ -85,10 +85,26 @@ later. - A ChatGPT sign-in can sit beside a gateway. Its email, workspace and `Sign out` stay on the row. - With no account at all the row reads `No sign-in needed`: Codex is ready on the gateway alone. Threadlines can't check the gateway's key, so a wrong key shows up when a chat fails. -- Codex lists OpenAI's models whatever the provider. If your gateway serves other models, add - their ids under the Codex row's `Models` as custom models. -- Titles and commit messages written by Codex still go to OpenAI, not the gateway. Pick another - agent for them in `Settings` if Codex has no OpenAI sign-in. +- The model Codex's config names (`model`) is in the model picker and is the default for new + threads. Codex lists OpenAI's models beside it whatever the provider. If your gateway serves + others, add their ids under the Codex row's `Models` as custom models. +- Titles and commit messages written by Codex go through the gateway too. If the gateway refuses + the model picked for them in `Settings`, Threadlines tries once more with the model from + Codex's config. +- Those short runs leave the rest of your Codex config out (MCP servers, notify hooks), as they + always have. Each gets a Codex folder of its own holding the provider's settings alone, + readable only by you and removed when the run ends. Nothing from those settings is passed on + the command line. +- One setup keeps those short runs on OpenAI, as before: a provider you add that signs in with + what Codex's own folder stores (a ChatGPT sign-in, a gateway's OAuth, Amazon Bedrock or Google + Cloud credentials). A folder made for one run has no sign-in. A provider built into Codex runs + in Codex's own folder, so its sign-in is there. +- If such a run fails, the error shown has the provider's token, headers and address keys struck + out. +- If Codex can't be asked which provider it is on for 15 minutes, Threadlines stops assuming the + gateway until Codex names it again. +- A Codex agent answering on the side in a room still runs on OpenAI with your ChatGPT sign-in, + not on the gateway. ## Security Notices From OpenAI