diff --git a/src/shared/toml-object.ts b/src/shared/toml-object.ts index cefa4d62..018a4744 100644 --- a/src/shared/toml-object.ts +++ b/src/shared/toml-object.ts @@ -26,6 +26,75 @@ export function parseTomlObject(rawText: string): Record { return parsed; } +export interface TomlNewlineRepairProposal { + repairedText: string; + insertionOffset: number; +} + +const CODEX_MODEL_MIGRATION_TABLE = /^[ \t]*\[notice\.model_migrations\][ \t]*(?:#.*)?$/; +const QUOTED_MIGRATION_JOIN = + /^[ \t]*(?:"(?:\\.|[^"\\])*"|'[^']*')[ \t]*=[ \t]*(?:"(?:\\.|[^"\\])*"|'[^']*')(?=\[)/; +const TOML_TABLE_HEADER_SUFFIX = /^(?:\[\[[^\]\r\n]+\]\]|\[[^\]\r\n]+\])[ \t]*(?:#.*)?$/; + +type TomlMultilineDelimiter = '"""' | "'''"; + +function isEscapedAt(text: string, offset: number): boolean { + let backslashCount = 0; + for (let index = offset - 1; index >= 0 && text[index] === '\\'; index -= 1) { + backslashCount += 1; + } + return backslashCount % 2 === 1; +} + +function updateTomlMultilineDelimiter( + line: string, + initialDelimiter: TomlMultilineDelimiter | null +): TomlMultilineDelimiter | null { + let delimiter = initialDelimiter; + let cursor = 0; + + while (cursor < line.length) { + if (delimiter) { + const closingOffset = line.indexOf(delimiter, cursor); + if (closingOffset === -1) return delimiter; + if (delimiter === '"""' && isEscapedAt(line, closingOffset)) { + cursor = closingOffset + 1; + continue; + } + cursor = closingOffset + delimiter.length; + delimiter = null; + continue; + } + + const char = line[cursor]; + if (char === '#') return null; + if (line.startsWith('"""', cursor) || line.startsWith("'''", cursor)) { + delimiter = line.slice(cursor, cursor + 3) as TomlMultilineDelimiter; + cursor += 3; + continue; + } + if (char === '"') { + cursor += 1; + while (cursor < line.length) { + if (line[cursor] === '"' && !isEscapedAt(line, cursor)) { + cursor += 1; + break; + } + cursor += 1; + } + continue; + } + if (char === "'") { + const closingOffset = line.indexOf("'", cursor + 1); + cursor = closingOffset === -1 ? line.length : closingOffset + 1; + continue; + } + cursor += 1; + } + + return delimiter; +} + export function safeParseTomlObject(rawText: string): SafeTomlObjectParseResult { try { return { @@ -39,3 +108,61 @@ export function safeParseTomlObject(rawText: string): SafeTomlObjectParseResult }; } } + +export function proposeJoinedCodexModelMigrationRepair( + rawText: string +): TomlNewlineRepairProposal | null { + if (!safeParseTomlObject(rawText).parseError) return null; + + const candidates: Array<{ insertionOffset: number; lineEnding: string }> = []; + let cursor = 0; + let inModelMigrationTable = false; + let previousLineEnding = '\n'; + let multilineDelimiter: TomlMultilineDelimiter | null = null; + + while (cursor < rawText.length) { + const newlineOffset = rawText.indexOf('\n', cursor); + const rawLineEnd = newlineOffset === -1 ? rawText.length : newlineOffset; + const usesCrLf = newlineOffset !== -1 && rawText[rawLineEnd - 1] === '\r'; + const contentEnd = usesCrLf ? rawLineEnd - 1 : rawLineEnd; + const lineEnding = newlineOffset === -1 ? '' : usesCrLf ? '\r\n' : '\n'; + const rawLine = rawText.slice(cursor, contentEnd); + const line = cursor === 0 && rawLine.charCodeAt(0) === 0xfeff ? rawLine.slice(1) : rawLine; + + if (!multilineDelimiter) { + if (CODEX_MODEL_MIGRATION_TABLE.test(line)) { + inModelMigrationTable = true; + } else if (/^[ \t]*\[\[?/.test(line)) { + inModelMigrationTable = false; + } else if (inModelMigrationTable) { + const joinedMapping = line.match(QUOTED_MIGRATION_JOIN); + if (joinedMapping) { + const suffix = line.slice(joinedMapping[0].length); + if (TOML_TABLE_HEADER_SUFFIX.test(suffix)) { + candidates.push({ + insertionOffset: cursor + joinedMapping[0].length, + lineEnding: lineEnding || previousLineEnding, + }); + } + } + } + } + multilineDelimiter = updateTomlMultilineDelimiter(line, multilineDelimiter); + + if (lineEnding) previousLineEnding = lineEnding; + if (newlineOffset === -1) break; + cursor = newlineOffset + 1; + } + + if (candidates.length !== 1) return null; + + const [{ insertionOffset, lineEnding }] = candidates; + const repairedText = `${rawText.slice(0, insertionOffset)}${lineEnding}${rawText.slice(insertionOffset)}`; + + try { + parseTomlObject(repairedText); + return { repairedText, insertionOffset }; + } catch { + return null; + } +} diff --git a/tests/unit/shared/toml-object.test.ts b/tests/unit/shared/toml-object.test.ts new file mode 100644 index 00000000..2f7d624b --- /dev/null +++ b/tests/unit/shared/toml-object.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from 'bun:test'; + +import { + parseTomlObject, + proposeJoinedCodexModelMigrationRepair, +} from '../../../src/shared/toml-object'; + +describe('joined Codex model migration TOML recovery', () => { + it('proposes the exact one-newline repair for a joined following table header', () => { + const rawText = `model = "gpt-5.4" + +[notice.model_migrations] +"gpt-5.3-codex" = "gpt-5.4"[agents.code_simplifier] +description = "Keep this exact text" +`; + + const proposal = proposeJoinedCodexModelMigrationRepair(rawText); + const insertionOffset = rawText.indexOf('[agents.code_simplifier]'); + + expect(proposal).toEqual({ + repairedText: `${rawText.slice(0, insertionOffset)}\n${rawText.slice(insertionOffset)}`, + insertionOffset, + }); + expect(() => parseTomlObject(proposal?.repairedText ?? '')).not.toThrow(); + }); + + it('preserves a UTF-8 BOM, CRLF endings, comments, spacing, and quoting', () => { + const rawText = + "\uFEFF# exact comment\r\n[notice.model_migrations]\r\n 'gpt-5.3-codex' = 'gpt-5.4'[[agents]]\r\nname = \"one\"\r\n"; + + const proposal = proposeJoinedCodexModelMigrationRepair(rawText); + const insertionOffset = rawText.indexOf('[[agents]]'); + + expect(proposal?.repairedText).toBe( + `${rawText.slice(0, insertionOffset)}\r\n${rawText.slice(insertionOffset)}` + ); + expect(proposal?.insertionOffset).toBe(insertionOffset); + }); + + it('does not propose a repair for valid TOML', () => { + const rawText = `[notice.model_migrations] +"gpt-5.3-codex" = "gpt-5.4" + +[agents.code_simplifier] +description = "valid" +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); + + it('does not generically repair a joined boundary outside the migration table', () => { + const rawText = `model = "gpt-5.4"[agents.code_simplifier] +description = "invalid" +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); + + it('ignores a migration table lookalike inside a multiline TOML string', () => { + const rawText = `description = """ +[notice.model_migrations] +""" +"old" = "new"[agents.outside] +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); + + it('does not propose a repair for unrelated invalid TOML', () => { + expect(proposeJoinedCodexModelMigrationRepair('model = "gpt-5.4"\n[features\n')).toBeNull(); + }); + + it('requires a quoted string-to-string migration mapping', () => { + const rawText = `[notice.model_migrations] +gpt-5 = 54[agents.code_simplifier] +description = "invalid" +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); + + it('does not propose a repair when the following text is not a valid table header', () => { + const rawText = `[notice.model_migrations] +"gpt-5.3-codex" = "gpt-5.4"[agents.code_simplifier +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); + + it('does not propose a repair when another syntax error would remain', () => { + const rawText = `[notice.model_migrations] +"gpt-5.3-codex" = "gpt-5.4"[agents.code_simplifier] +description = "valid" +[features +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); + + it('does not propose an ambiguous repair when multiple joined boundaries match', () => { + const rawText = `[notice.model_migrations] +"gpt-5.2" = "gpt-5.3"[agents.first] +name = "first" + +[notice.model_migrations] +"gpt-5.3" = "gpt-5.4"[agents.second] +name = "second" +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); + + it('does not treat a joined-looking string inside a multiline value as a repair candidate', () => { + const rawText = `[notice.model_migrations] +note = """ +"gpt-5.3-codex" = "gpt-5.4"[agents.code_simplifier] +""" +[features +`; + + expect(proposeJoinedCodexModelMigrationRepair(rawText)).toBeNull(); + }); +}); diff --git a/ui/src/pages/codex.tsx b/ui/src/pages/codex.tsx index eb04fca3..0175daae 100644 --- a/ui/src/pages/codex.tsx +++ b/ui/src/pages/codex.tsx @@ -10,6 +10,7 @@ import { useCodex } from '@/hooks/use-codex'; import { isApiConflictError } from '@/lib/api-client'; import { CodexOverviewTab } from '@/components/compatible-cli/codex-overview-tab'; import { RawConfigEditorPanel } from '@/components/compatible-cli/raw-json-settings-editor-panel'; +import { Button } from '@/components/ui/button'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; import { getKnownCodexFeatures, @@ -20,7 +21,13 @@ import { readCodexProjectTrust, readCodexTopLevelSettings, } from '@/lib/codex-config'; -import { safeParseTomlObject } from '@shared/toml-object'; +import { proposeJoinedCodexModelMigrationRepair, safeParseTomlObject } from '@shared/toml-object'; + +interface RawConfigDraft { + text: string; + baseText: string; + expectedMtime?: number; +} export function CodexPage() { const { t } = useTranslation(); @@ -40,10 +47,14 @@ export function CodexPage() { isPatchingConfig, } = useCodex(); - const [rawDraftText, setRawDraftText] = useState(null); + const [rawDraft, setRawDraft] = useState(null); const rawBaseText = rawConfig?.rawText ?? ''; - const rawEditorText = rawDraftText ?? rawBaseText; - const rawConfigDirty = rawDraftText !== null && rawDraftText !== rawBaseText; + const rawEditorText = rawDraft?.text ?? rawBaseText; + const rawConfigDirty = rawDraft !== null; + const rawBaseRepairProposal = useMemo(() => { + if (rawConfigDirty || !rawConfig?.parseError || rawConfig.readError) return null; + return proposeJoinedCodexModelMigrationRepair(rawBaseText); + }, [rawBaseText, rawConfig?.parseError, rawConfig?.readError, rawConfigDirty]); const rawEditorParsed = safeParseTomlObject(rawEditorText); const rawEditorValidation = rawEditorParsed.parseError ? { valid: false as const, error: rawEditorParsed.parseError } @@ -84,11 +95,20 @@ export function CodexPage() { const featureState = useMemo(() => readCodexFeatureState(controlsConfig), [controlsConfig]); const setRawEditorDraftText = (nextText: string) => { - if (nextText === rawBaseText) { - setRawDraftText(null); - return; - } - setRawDraftText(nextText); + setRawDraft((currentDraft) => { + const baseText = currentDraft?.baseText ?? rawBaseText; + if (nextText === baseText) return null; + + return { + text: nextText, + baseText, + expectedMtime: currentDraft + ? currentDraft.expectedMtime + : rawConfig?.exists + ? rawConfig.mtime + : undefined, + }; + }); }; const refreshAll = async () => { @@ -103,24 +123,24 @@ export function CodexPage() { return; } - setRawDraftText(null); + setRawDraft(null); } catch (error) { toast.error((error as Error).message || t('toasts.codexRefreshError')); } }; const handleSaveRawConfig = async () => { - if (!rawEditorValidation.valid) { + if (!rawDraft || !rawEditorValidation.valid) { toast.error(t('toasts.codexFixToml')); return; } try { await saveRawConfigAsync({ - rawText: rawEditorText, - expectedMtime: rawConfig?.exists ? rawConfig.mtime : undefined, + rawText: rawDraft.text, + expectedMtime: rawDraft.expectedMtime, }); - setRawDraftText(null); + setRawDraft(null); toast.success(t('toasts.codexSaved')); await refetchDiagnostics(); } catch (error) { @@ -141,7 +161,7 @@ export function CodexPage() { ...patch, expectedMtime: rawConfig?.exists ? rawConfig.mtime : undefined, }); - setRawDraftText(null); + setRawDraft(null); toast.success(successMessage); } catch (error) { if (isApiConflictError(error)) { @@ -259,7 +279,7 @@ export function CodexPage() { }} onSave={handleSaveRawConfig} onRefresh={refreshAll} - onDiscard={() => setRawDraftText(null)} + onDiscard={() => setRawDraft(null)} language="toml" exactText /* TODO i18n: missing key for "Loading config.toml..." */ @@ -267,16 +287,40 @@ export function CodexPage() { /* TODO i18n: missing key for "TOML warning" */ parseWarningLabel="TOML warning" ownershipNotice={ -
- {/* TODO i18n: missing keys for ownership notice paragraphs */} -

This file is upstream-owned by Codex CLI.

-

- CCS does not keep ~/.codex/config.toml in sync for you. -

-

- CCS-backed Codex launches may apply transient -c overrides and - CCS_CODEX_API_KEY; those effective values may not appear here. -

+
+ {rawBaseRepairProposal ? ( +
+

Safe repair available

+

+ Codex joined a model migration to the next table header. Previewing inserts + exactly one newline in this unsaved editor draft. Review it, then select Save + to write config.toml. +

+ +
+ ) : null} +
+ {/* TODO i18n: missing keys for ownership notice paragraphs */} +

This file is upstream-owned by Codex CLI.

+

+ CCS does not keep ~/.codex/config.toml in sync for you. +

+

+ CCS-backed Codex launches may apply transient -c overrides and + CCS_CODEX_API_KEY; those effective values may not appear here. +

+
} /> diff --git a/ui/tests/unit/ui/pages/codex-page.test.tsx b/ui/tests/unit/ui/pages/codex-page.test.tsx index cae3fa89..1b450456 100644 --- a/ui/tests/unit/ui/pages/codex-page.test.tsx +++ b/ui/tests/unit/ui/pages/codex-page.test.tsx @@ -40,7 +40,11 @@ vi.mock('@/components/shared/code-editor', () => ({ })); vi.mock('@/components/compatible-cli/codex-control-center-tab', () => ({ - CodexControlCenterTab: () =>
Control Center
, + CodexControlCenterTab: ({ disabled }: { disabled?: boolean }) => ( +
+ Control Center +
+ ), })); vi.mock('@/components/compatible-cli/codex-docs-tab', () => ({ @@ -53,6 +57,15 @@ vi.mock('@/components/compatible-cli/codex-overview-tab', () => ({ import { CodexPage } from '@/pages/codex'; +const joinedMigrationRawText = `[notice.model_migrations] +"gpt-5.3-codex" = "gpt-5.4"[agents.code_simplifier] +description = "Keep exact text" +`; +const repairedMigrationRawText = joinedMigrationRawText.replace( + '"gpt-5.4"[agents.code_simplifier]', + '"gpt-5.4"\n[agents.code_simplifier]' +); + const diagnostics = { binary: { installed: true, @@ -228,5 +241,184 @@ describe('CodexPage', () => { expect(screen.getByText(/Read-only: Refusing symlink file for safety\./)).toBeInTheDocument(); expect(screen.getByLabelText('codex raw editor')).toHaveAttribute('readonly'); + expect( + screen.queryByRole('button', { name: 'Preview one-newline repair' }) + ).not.toBeInTheDocument(); + }); + + it('previews the one-newline repair as an unsaved draft before explicitly saving it', async () => { + mocks.saveRawConfigAsync.mockResolvedValue({ success: true, mtime: 101 }); + mocks.useCodex.mockReturnValue( + buildUseCodexResult({ + rawConfig: { + path: '$CODEX_HOME/config.toml', + resolvedPath: '/tmp/.codex/config.toml', + exists: true, + mtime: 100, + rawText: joinedMigrationRawText, + config: null, + parseError: 'Invalid TOML', + readError: null, + }, + }) + ); + + render(); + + await userEvent.click(screen.getByRole('tab', { name: 'Control Center' })); + expect(screen.getByTestId('codex-control-center')).toHaveAttribute('data-disabled', 'true'); + await userEvent.click(screen.getByRole('button', { name: 'Preview one-newline repair' })); + + expect(screen.getByLabelText('codex raw editor')).toHaveValue(repairedMigrationRawText); + expect(screen.getByText('Unsaved')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Save' })).toBeEnabled(); + expect(screen.getByTestId('codex-control-center')).toHaveAttribute('data-disabled', 'true'); + expect(mocks.saveRawConfigAsync).not.toHaveBeenCalled(); + + await userEvent.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => + expect(mocks.saveRawConfigAsync).toHaveBeenCalledWith({ + rawText: repairedMigrationRawText, + expectedMtime: 100, + }) + ); + }); + + it('keeps the repair draft bound to its original mtime across a fetched snapshot update', async () => { + mocks.saveRawConfigAsync.mockResolvedValue({ success: true, mtime: 201 }); + let rawConfig = { + path: '$CODEX_HOME/config.toml', + resolvedPath: '/tmp/.codex/config.toml', + exists: true, + mtime: 100, + rawText: joinedMigrationRawText, + config: null, + parseError: 'Invalid TOML', + readError: null, + }; + mocks.useCodex.mockImplementation(() => buildUseCodexResult({ rawConfig })); + + const view = render(); + await userEvent.click(screen.getByRole('tab', { name: 'Control Center' })); + await userEvent.click(screen.getByRole('button', { name: 'Preview one-newline repair' })); + + rawConfig = { + ...rawConfig, + mtime: 200, + rawText: 'model = "gpt-5.4-mini"\n', + config: { model: 'gpt-5.4-mini' }, + parseError: null, + }; + view.rerender(); + + expect(screen.getByLabelText('codex raw editor')).toHaveValue(repairedMigrationRawText); + expect(screen.getByTestId('codex-control-center')).toHaveAttribute('data-disabled', 'true'); + await userEvent.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => + expect(mocks.saveRawConfigAsync).toHaveBeenCalledWith({ + rawText: repairedMigrationRawText, + expectedMtime: 100, + }) + ); + }); + + it('keeps manual raw edits bound to the snapshot mtime captured on first edit', async () => { + mocks.saveRawConfigAsync.mockResolvedValue({ success: true, mtime: 201 }); + let rawConfig = { + path: '$CODEX_HOME/config.toml', + resolvedPath: '/tmp/.codex/config.toml', + exists: true, + mtime: 100, + rawText: 'model = "gpt-5.4"\n', + config: { model: 'gpt-5.4' }, + parseError: null, + readError: null, + }; + mocks.useCodex.mockImplementation(() => buildUseCodexResult({ rawConfig })); + + const view = render(); + const editor = screen.getByLabelText('codex raw editor'); + await userEvent.clear(editor); + await userEvent.type(editor, 'model = "gpt-5.4-mini"'); + + rawConfig = { + ...rawConfig, + mtime: 200, + rawText: 'model = "gpt-5.4-high"\n', + config: { model: 'gpt-5.4-high' }, + }; + view.rerender(); + + expect(screen.getByLabelText('codex raw editor')).toHaveValue('model = "gpt-5.4-mini"'); + await userEvent.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => + expect(mocks.saveRawConfigAsync).toHaveBeenCalledWith({ + rawText: 'model = "gpt-5.4-mini"', + expectedMtime: 100, + }) + ); + }); + + it('does not offer the targeted repair for generic invalid TOML', () => { + mocks.useCodex.mockReturnValue( + buildUseCodexResult({ + rawConfig: { + path: '$CODEX_HOME/config.toml', + resolvedPath: '/tmp/.codex/config.toml', + exists: true, + mtime: 100, + rawText: 'model = "gpt-5.4"\n[features\n', + config: null, + parseError: 'Invalid TOML', + readError: null, + }, + }) + ); + + render(); + + expect( + screen.queryByRole('button', { name: 'Preview one-newline repair' }) + ).not.toBeInTheDocument(); + }); + + it('re-enables structured controls only after the saved valid snapshot is loaded', async () => { + mocks.saveRawConfigAsync.mockResolvedValue({ success: true, mtime: 101 }); + let rawConfig = { + path: '$CODEX_HOME/config.toml', + resolvedPath: '/tmp/.codex/config.toml', + exists: true, + mtime: 100, + rawText: joinedMigrationRawText, + config: null, + parseError: 'Invalid TOML', + readError: null, + }; + mocks.useCodex.mockImplementation(() => buildUseCodexResult({ rawConfig })); + + const view = render(); + await userEvent.click(screen.getByRole('tab', { name: 'Control Center' })); + await userEvent.click(screen.getByRole('button', { name: 'Preview one-newline repair' })); + expect(screen.getByTestId('codex-control-center')).toHaveAttribute('data-disabled', 'true'); + + await userEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => expect(mocks.saveRawConfigAsync).toHaveBeenCalledTimes(1)); + + rawConfig = { + ...rawConfig, + mtime: 101, + rawText: repairedMigrationRawText, + config: { + notice: { model_migrations: { 'gpt-5.3-codex': 'gpt-5.4' } }, + agents: { code_simplifier: { description: 'Keep exact text' } }, + }, + parseError: null, + }; + view.rerender(); + + expect(screen.getByTestId('codex-control-center')).toHaveAttribute('data-disabled', 'false'); }); });