fix: prevent common config modal infinite reopen loop and add draft editing

The auto-open useEffect in CodexConfigEditor and GeminiConfigEditor
created an inescapable loop: commonConfigError triggered modal open,
closing the modal didn't clear the error, so the effect immediately
reopened it — locking the entire UI.

- Remove auto-open useEffect from both Codex and Gemini config editors
- Convert common config modals to draft editing (edit locally, validate
  before save) instead of persisting on every keystroke
- Add TOML/JSON pre-validation via parseCommonConfigSnippet before any
  merge operation to prevent invalid content from being persisted
- Expose clearCommonConfigError so editors can clear stale errors on
  modal close
This commit is contained in:
Jason
2026-03-11 14:40:14 +08:00
parent 4ac7e28b10
commit 239c6fb2d7
9 changed files with 596 additions and 204 deletions
@@ -0,0 +1,121 @@
import type { ReactNode } from "react";
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";
import CodexConfigEditor from "@/components/providers/forms/CodexConfigEditor";
import GeminiConfigEditor from "@/components/providers/forms/GeminiConfigEditor";
vi.mock("@/components/common/FullScreenPanel", () => ({
FullScreenPanel: ({
isOpen,
title,
onClose,
children,
footer,
}: {
isOpen: boolean;
title: string;
onClose: () => void;
children: ReactNode;
footer?: ReactNode;
}) =>
isOpen ? (
<div data-testid="common-config-panel">
<button type="button" onClick={onClose}>
panel-close
</button>
<h2>{title}</h2>
<div>{children}</div>
<div>{footer}</div>
</div>
) : null,
}));
vi.mock("@/components/JsonEditor", () => ({
default: ({
value,
onChange,
}: {
value: string;
onChange: (value: string) => void;
}) => (
<textarea
value={value}
onChange={(event) => onChange(event.target.value)}
aria-label="mock-editor"
/>
),
}));
describe("Common config modals", () => {
it("keeps the Codex common config modal closed after user closes it with an error present", async () => {
render(
<CodexConfigEditor
authValue="{}"
configValue=""
onAuthChange={() => {}}
onConfigChange={() => {}}
useCommonConfig={false}
onCommonConfigToggle={() => {}}
commonConfigSnippet={`base_url = "https://example.com"`}
onCommonConfigSnippetChange={() => false}
onCommonConfigErrorClear={() => {}}
commonConfigError="Invalid TOML"
authError=""
configError=""
/>,
);
expect(screen.queryByTestId("common-config-panel")).not.toBeInTheDocument();
fireEvent.click(
screen.getByRole("button", { name: /codexConfig.editCommonConfig|编辑通用配置/ }),
);
expect(screen.getByTestId("common-config-panel")).toBeInTheDocument();
fireEvent.click(screen.getByRole("button", { name: "common.cancel" }));
await waitFor(() =>
expect(
screen.queryByTestId("common-config-panel"),
).not.toBeInTheDocument(),
);
});
it("keeps the Gemini common config modal closed after user closes it with an error present", async () => {
render(
<GeminiConfigEditor
envValue="{}"
configValue="{}"
onEnvChange={() => {}}
onConfigChange={() => {}}
useCommonConfig={false}
onCommonConfigToggle={() => {}}
commonConfigSnippet={`{"GEMINI_MODEL":"gemini-2.5-pro"}`}
onCommonConfigSnippetChange={() => false}
onCommonConfigErrorClear={() => {}}
commonConfigError="Invalid JSON"
envError=""
configError=""
/>,
);
expect(screen.queryByTestId("common-config-panel")).not.toBeInTheDocument();
fireEvent.click(
screen.getByRole("button", {
name: /geminiConfig.editCommonConfig|编辑通用配置/,
}),
);
expect(screen.getByTestId("common-config-panel")).toBeInTheDocument();
fireEvent.click(screen.getByRole("button", { name: "common.cancel" }));
await waitFor(() =>
expect(
screen.queryByTestId("common-config-panel"),
).not.toBeInTheDocument(),
);
});
});
+79
View File
@@ -0,0 +1,79 @@
import { act, renderHook, waitFor } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { useCodexCommonConfig } from "@/components/providers/forms/hooks/useCodexCommonConfig";
import { useGeminiCommonConfig } from "@/components/providers/forms/hooks/useGeminiCommonConfig";
const getCommonConfigSnippetMock = vi.fn();
const setCommonConfigSnippetMock = vi.fn();
const extractCommonConfigSnippetMock = vi.fn();
vi.mock("@/lib/api", () => ({
configApi: {
getCommonConfigSnippet: (...args: unknown[]) =>
getCommonConfigSnippetMock(...args),
setCommonConfigSnippet: (...args: unknown[]) =>
setCommonConfigSnippetMock(...args),
extractCommonConfigSnippet: (...args: unknown[]) =>
extractCommonConfigSnippetMock(...args),
},
}));
describe("common config snippet saving", () => {
beforeEach(() => {
getCommonConfigSnippetMock.mockResolvedValue("");
setCommonConfigSnippetMock.mockResolvedValue(undefined);
extractCommonConfigSnippetMock.mockResolvedValue("");
});
it("does not persist an invalid Codex common config snippet", async () => {
const onConfigChange = vi.fn();
const { result } = renderHook(() =>
useCodexCommonConfig({
codexConfig: "model = \"gpt-5\"",
onConfigChange,
}),
);
await waitFor(() => expect(result.current.isLoading).toBe(false));
let saved = false;
act(() => {
saved = result.current.handleCommonConfigSnippetChange(
"base_url = https://bad.example/v1",
);
});
expect(saved).toBe(false);
expect(setCommonConfigSnippetMock).not.toHaveBeenCalled();
expect(onConfigChange).not.toHaveBeenCalled();
expect(result.current.commonConfigError).toContain("invalid value");
});
it("does not persist an invalid Gemini common config snippet", async () => {
const onEnvChange = vi.fn();
const { result } = renderHook(() =>
useGeminiCommonConfig({
envValue: "",
onEnvChange,
envStringToObj: () => ({}),
envObjToString: () => "",
}),
);
await waitFor(() => expect(result.current.isLoading).toBe(false));
let saved = false;
act(() => {
saved = result.current.handleCommonConfigSnippetChange(
JSON.stringify({ GEMINI_MODEL: 123 }),
);
});
expect(saved).toBe(false);
expect(setCommonConfigSnippetMock).not.toHaveBeenCalled();
expect(onEnvChange).not.toHaveBeenCalled();
expect(result.current.commonConfigError).toBe(
"geminiConfig.commonConfigInvalidValues",
);
});
});