-
Notifications
You must be signed in to change notification settings - Fork 472
fix(codex): preserve external model providers #333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ff077f0
821f9fa
b3d1bc6
d1469d8
c2caf45
759b403
a9575ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,24 @@ | ||
| import { existsSync, readFileSync, unlinkSync } from "node:fs"; | ||
| import { atomicWriteFile, loadConfig, websocketsEnabled } from "../config"; | ||
| import { markJournalInjectedState, restoreJournalState, writeJournal } from "./journal"; | ||
| import { markJournalInjectedState, removeJournal, restoreJournalState, writeJournal } from "./journal"; | ||
| import { restoreCodexCatalog } from "./catalog"; | ||
| import { migrateHistoryToOpenai, syncCodexHistoryProvider } from "./history-provider"; | ||
| import { CODEX_CONFIG_PATH, CODEX_PROFILE_PATH, DEFAULT_CATALOG_PATH, parseTomlString, readRootTomlString, resolveCodexConfigPath, tomlString } from "./paths"; | ||
| import { resolveEffectiveProjectModelProvider } from "./project-config-warnings"; | ||
| import type { OcxConfig } from "../types"; | ||
|
|
||
| const OCX_SECTION_MARKER = "# Auto-injected by opencodex"; | ||
|
|
||
| export function externalCodexModelProvider(content: string): string | null { | ||
| const provider = resolveEffectiveProjectModelProvider(content).provider; | ||
| return provider && provider !== "openai" && provider !== "opencodex" ? provider : null; | ||
| } | ||
|
|
||
| export function currentExternalCodexModelProvider(): string | null { | ||
| if (!existsSync(CODEX_CONFIG_PATH)) return null; | ||
| return externalCodexModelProvider(readFileSync(CODEX_CONFIG_PATH, "utf8")); | ||
| } | ||
|
|
||
| /** | ||
| * Detect the file's dominant line ending. Every transform in this module is LF-pure | ||
| * (split("\n") + hard "\n" joins), so CRLF configs (Windows-edited config.toml) are | ||
|
|
@@ -365,8 +376,23 @@ export async function injectCodexConfig(port: number, config?: OcxConfig, option | |
| return { success: false, message: `Codex config not found at ${CODEX_CONFIG_PATH}. Is Codex installed?` }; | ||
| } | ||
|
|
||
| writeJournal(); | ||
| const rawContent = readFileSync(CODEX_CONFIG_PATH, "utf-8"); | ||
| const activeProvider = externalCodexModelProvider(rawContent); | ||
| if (activeProvider) { | ||
| // A launcher may have journaled before the provider manager took ownership. Never let shutdown | ||
| // replay that stale snapshot over externally managed config. | ||
| removeJournal(); | ||
| return { | ||
| success: true, | ||
| message: `⚠️ Codex routing NOT injected: config.toml selects the external model_provider ${tomlString(activeProvider)}.\n` + | ||
| ` OpenCodex preserves external provider configuration so existing ${tomlString(activeProvider)} session history stays visible.\n` + | ||
| ` Configure that provider for Responses passthrough at http://${providerBaseHost(config?.hostname)}:${port}/v1` + | ||
| `${shouldInjectApiAuthHeader(config) ? ` with x-opencodex-api-key from OPENCODEX_API_AUTH_TOKEN` : ""}.\n` + | ||
| ` For direct injection, switch to the built-in openai provider, remove any user-owned root openai_base_url, and rerun 'ocx start'.`, | ||
| }; | ||
| } | ||
|
|
||
| writeJournal(); | ||
| // EOL boundary: transforms below are LF-pure; preserve the file's dominant ending on write. | ||
| const eol = dominantEol(rawContent); | ||
| let content = applyEol(rawContent, "\n"); | ||
|
|
@@ -543,6 +569,11 @@ export function removeCodexConfig(options: { preserveProfile?: boolean } = {}): | |
| * handler, and `ocx restore`. Idempotent + atomic. | ||
| */ | ||
| export function restoreNativeCodex(): { success: boolean; message: string } { | ||
| const activeProvider = currentExternalCodexModelProvider(); | ||
| if (activeProvider) { | ||
| removeJournal(); | ||
| return { success: true, message: `External Codex provider ${tomlString(activeProvider)} preserved; no native restore was needed.` }; | ||
| } | ||
|
Comment on lines
+572
to
+576
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Add a restore-path preservation regression test. The new early return prevents config, catalog, and history restoration for an external provider, but the new integration coverage exercises only As per path instructions, “A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem.” 🤖 Prompt for AI AgentsSource: Path instructions |
||
| const journal = restoreJournalState(); | ||
| const cfg = journal.configRestored | ||
| ? { success: true, message: "Codex config restored from opencodex journal." } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.