fix: confirm deletes and refuse duplicate Provider IDs

frontend/bindings/github.com/MaimoryLab/OneAgent/internal/binding/models.ts

@@ -163,6 +163,13 @@ export interface SaveProviderRequest {
     "base_url": string;
     "anthropic_base_url": string;
     "api_key": string;
+
+    /**
+     * Create refuses an ID that already exists rather than overwriting it. It
+     * defaults to false so an edit, which legitimately writes over an existing
+     * entry, is the behaviour a caller gets without asking.
+     */
+    "create": boolean;
 }
 
 export interface UpdateRequest {

frontend/src/backend/wails.test.ts

@@ -129,7 +129,7 @@ describe("Wails backend adapter", () => {
     await expect(wailsApi.probe({ provider: "custom", apiBaseUrl: "https://proxy.test/v1", apiKey: "secret", model: "m", agents: [] })).resolves.toBe(probe);
     await expect(wailsApi.models({ provider: "ppio", apiBaseUrl: "", apiKey: "secret" })).resolves.toBe(models);
     await expect(wailsApi.getProvider("acme")).resolves.toBe(provider);
-    await expect(wailsApi.saveProvider({ id: "acme", name: "Acme", home: "", base_url: "https://api.acme.test", anthropic_base_url: "", api_key: "secret" })).resolves.toBe(provider);
+    await expect(wailsApi.saveProvider({ id: "acme", name: "Acme", home: "", base_url: "https://api.acme.test", anthropic_base_url: "", api_key: "secret", create: true })).resolves.toBe(provider);
     await expect(wailsApi.deleteProvider("acme")).resolves.toBeUndefined();
     await expect(wailsApi.install({ agents: ["codex"], provider: "ppio", api_key: "secret", model: "m", configure: true, install_agent: false, skip_test: true })).resolves.toBe(install);
     await wailsApi.openRegister("ppio", []);

frontend/src/i18n.tsx

@@ -45,6 +45,14 @@ const english = {
   "编辑配置": "Edit configuration",
   "详情": "Details",
   "删除": "Delete",
+  // Delete confirmations. Both name the record and say the deletion cannot be
+  // undone; the Provider wording also names the API Key, because that is the part
+  // the user cannot recreate from what is on screen.
+  // "删除 Profile" is already defined further down, as the edit-form heading.
+  "确定删除 Profile「{name}」吗？该操作无法撤销。": "Delete the Profile “{name}”? This cannot be undone.",
+  "删除 Provider": "Delete Provider",
+  "确定删除 Provider「{name}」吗？已保存的 API Key 会一并删除，该操作无法撤销。": "Delete the Provider “{name}”? Its saved API key is deleted with it, and this cannot be undone.",
+  "仅供本机识别，可保留默认值。小写字母、数字或连字符": "Identifies this Provider on your machine; the default is fine. Lowercase letters, digits, or hyphens",
   "名称": "Name",
   "模型": "Model",
   "API 类型": "API type",

frontend/src/pages/AgentProfilePage.tsx

@@ -194,7 +194,9 @@ export function AgentProfilePage() {
           <div className="profile-editor-grid">
             <div className="field-stack">
               <label htmlFor="agent-profile-id">Profile ID</label>
-              <input id="agent-profile-id" value={draft.id} pattern="[a-z0-9][a-z0-9_-]{0,63}" onChange={(event) => setDraft({ ...draft, id: event.target.value })} disabled={Boolean(draft.originalId)} required />
+              {/* Escaped hyphen: compiled with the `v` flag, a literal `-` inside
+                  a character class throws and the attribute then accepts anything. */}
+              <input id="agent-profile-id" value={draft.id} pattern="[a-z0-9][a-z0-9_\-]{0,63}" onChange={(event) => setDraft({ ...draft, id: event.target.value })} disabled={Boolean(draft.originalId)} required />
             </div>
             <div className="field-stack">
               <label htmlFor="agent-profile-label">{t("名称")}</label>

frontend/src/pages/ProfilesPage.test.tsx

@@ -9,6 +9,11 @@ import { ProfilesPage } from "./ProfilesPage";
 const refreshStatus = vi.fn<() => Promise<void>>();
 const dispatch = vi.fn();
 
+// Hoisted so the module factory below can close over it: vi.mock is lifted above
+// the imports, so a plain const declared here would not exist yet when it runs.
+const { question } = vi.hoisted(() => ({ question: vi.fn<(options: { Message: string }) => Promise<string>>() }));
+vi.mock("@wailsio/runtime", () => ({ Dialogs: { Question: question } }));
+
 vi.mock("../state/WizardContext", () => ({
   useWizard: () => ({ state: mockState, dispatch, refreshStatus }),
 }));
@@ -99,6 +104,7 @@ describe("ProfilesPage", () => {
     vi.restoreAllMocks();
     refreshStatus.mockReset();
     refreshStatus.mockResolvedValue();
+    question.mockReset();
   });
 
   it("lists public Profile details and reports the Provider's key state", () => {
@@ -128,14 +134,49 @@ describe("ProfilesPage", () => {
     expect(screen.getByTestId("profile-unused").textContent).toContain("暂无 Agent 使用");
   });
 
-  it("deletes a Profile after confirmation", async () => {
-	vi.spyOn(window, "confirm").mockReturnValue(true);
-	const remove = vi.spyOn(api, "deleteProfile").mockResolvedValue();
-	renderPage([profile({ id: "unused", label: "未使用" })]);
+  // These two replace a test that stubbed window.confirm and asserted the delete
+  // went through. The page never called window.confirm, so the stub did nothing
+  // and the test passed against a delete button with no confirmation at all --
+  // exactly the bug it was named for. Asserting the cancel path is what makes the
+  // confirmation load-bearing: without it, deleting unconditionally still passes.
+  it("does not delete a Profile when the confirmation is declined", async () => {
+    question.mockResolvedValue("取消");
+    const remove = vi.spyOn(api, "deleteProfile").mockResolvedValue();
+    renderPage([profile({ id: "unused", label: "未使用" })]);
+
+    fireEvent.click(screen.getByRole("button", { name: "删除 未使用" }));
+    await waitFor(() => expect(question).toHaveBeenCalled());
+    expect(remove).not.toHaveBeenCalled();
+    expect(refreshStatus).not.toHaveBeenCalled();
+  });
 
-	fireEvent.click(screen.getByRole("button", { name: "删除 未使用" }));
-	await waitFor(() => expect(remove).toHaveBeenCalledWith("unused"));
-	expect(refreshStatus).toHaveBeenCalled();
+  it("deletes a Profile once the confirmation is accepted", async () => {
+    question.mockResolvedValue("删除");
+    const remove = vi.spyOn(api, "deleteProfile").mockResolvedValue();
+    renderPage([profile({ id: "unused", label: "未使用" })]);
+
+    fireEvent.click(screen.getByRole("button", { name: "删除 未使用" }));
+    await waitFor(() => expect(remove).toHaveBeenCalledWith("unused"));
+    expect(refreshStatus).toHaveBeenCalled();
+    // The prompt names the Profile: "delete this?" with no subject is how a user
+    // confirms the wrong row.
+    expect(question.mock.calls[0][0].Message).toContain("未使用");
+  });
+
+  it("blocks a second delete while the first is still running", async () => {
+    question.mockResolvedValue("删除");
+    let release = () => {};
+    const remove = vi.spyOn(api, "deleteProfile").mockReturnValue(new Promise<void>((resolve) => { release = resolve; }));
+    renderPage([profile({ id: "unused", label: "未使用" })]);
+
+    const button = screen.getByRole("button", { name: "删除 未使用" });
+    fireEvent.click(button);
+    // Double-clicking sent two deletes, and the second one reported the Profile
+    // it had just removed as unknown.
+    await waitFor(() => expect(button).toBeDisabled());
+    fireEvent.click(button);
+    expect(remove).toHaveBeenCalledTimes(1);
+    release();
   });
 
   it("explains why an in-use Profile cannot be deleted", () => {

frontend/src/pages/ProfilesPage.tsx

@@ -7,6 +7,7 @@ import { PageScaffold } from "../components/PageScaffold";
 import { ProviderSegment } from "../components/ProviderSegment";
 import { SelectField } from "../components/SelectField";
 import { useI18n } from "../i18n";
+import { confirmDelete } from "../state/confirmDelete";
 import { byProviderCreatedAt } from "../state/ranking";
 import { installTaskRoute, taskKey, useTaskCenter } from "../state/TaskCenterContext";
 import { useWizard } from "../state/WizardContext";
@@ -184,6 +185,14 @@ export function ProfilesPage() {
       }));
       return;
     }
+    // Asked after the in-use check, so a Profile that cannot be deleted anyway
+    // explains itself rather than prompting first and refusing afterwards.
+    if (!await confirmDelete({
+      title: t("删除 Profile"),
+      message: t("确定删除 Profile「{name}」吗？该操作无法撤销。", { name: profile.label }),
+      confirmLabel: t("删除"),
+      cancelLabel: t("取消"),
+    })) return;
     setBusy(true);
     setFailure("");
     try {
@@ -226,7 +235,10 @@ export function ProfilesPage() {
                 id="profile-id"
                 value={editor.id}
                 onChange={(event) => setEditor({ ...editor, id: event.target.value })}
-                pattern="[a-z0-9][a-z0-9_-]{0,63}"
+                /* Escaped hyphen: compiled with the `v` flag, a literal `-` in a
+                   character class throws, and that error left the attribute
+                   accepting every value. */
+                pattern="[a-z0-9][a-z0-9_\-]{0,63}"
                 placeholder={t("例如 team-ppio")}
                 disabled={Boolean(editor.originalId)}
                 required
@@ -329,7 +341,10 @@ export function ProfilesPage() {
                     <button className="icon-button" type="button" onClick={() => { setEditor(editDraft(profile, protocolOf(profile))); setFailure(""); }} aria-label={t("编辑 {name}", { name: profile.label })} title={t("编辑")}>
                       <Pencil size={14} />
                     </button>
-                    <button className="icon-button is-danger" type="button" onClick={() => void remove(profile, users)} aria-label={t("删除 {name}", { name: profile.label })} title={users.length ? t("Profile 正在被 {agents} 使用，无法删除", { agents: users.map((agent) => agent.name).join(locale === "en" ? ", " : "、") }) : t("删除")}>
+                    {/* disabled while busy: without it a double-click sent two
+                        deletes, and the second one used to report the Profile it
+                        had just removed as unknown. */}
+                    <button className="icon-button is-danger" type="button" disabled={busy} onClick={() => void remove(profile, users)} aria-label={t("删除 {name}", { name: profile.label })} title={users.length ? t("Profile 正在被 {agents} 使用，无法删除", { agents: users.map((agent) => agent.name).join(locale === "en" ? ", " : "、") }) : t("删除")}>
                       <Trash2 size={14} />
                     </button>
                   </span>

frontend/src/pages/ProvidersPage.test.tsx

@@ -6,6 +6,11 @@ import { api } from "../backend/api";
 import type { StatusResponse } from "../types/api";
 import { ProvidersPage } from "./ProvidersPage";
 
+// Hoisted so the module factory can close over it: vi.mock is lifted above the
+// imports, so a plain const here would not exist yet when the factory runs.
+const { question } = vi.hoisted(() => ({ question: vi.fn<(options: { Message: string }) => Promise<string>>() }));
+vi.mock("@wailsio/runtime", () => ({ Dialogs: { Question: question } }));
+
 vi.mock("../state/WizardContext", () => ({
   useWizard: () => ({ state: mockState, dispatch: vi.fn(), refreshStatus: vi.fn() }),
 }));
@@ -79,7 +84,10 @@ function renderPage(agents: Record<string, string | null>) {
 }
 
 describe("ProvidersPage", () => {
-  afterEach(() => vi.restoreAllMocks());
+  afterEach(() => {
+    vi.restoreAllMocks();
+    question.mockReset();
+  });
 
   it("lists each Provider with its endpoint", () => {
     renderPage({ codex: "ppio" });
@@ -211,21 +219,74 @@ describe("ProvidersPage", () => {
     fireEvent.change(screen.getByLabelText("OpenAI 兼容 Base URL"), { target: { value: "https://api.acme.test" } });
     fireEvent.change(screen.getByLabelText("API Key"), { target: { value: "sk-acme" } });
     fireEvent.click(screen.getByRole("button", { name: /^保存$/ }));
-    await waitFor(() => expect(save).toHaveBeenCalledWith(expect.objectContaining({ id: "acme", base_url: "https://api.acme.test", api_key: "sk-acme" })));
+    await waitFor(() => expect(save).toHaveBeenCalledWith(expect.objectContaining({ id: "acme", base_url: "https://api.acme.test", api_key: "sk-acme", create: true })));
   });
 
-  it("deletes a user Provider", async () => {
+  // The ID is a storage key the user should not have to invent, but a collision is
+  // now refused rather than silently overwriting -- so the suggested value has to
+  // be one that is actually free.
+  it("prefills a free Provider ID and states the rule", () => {
+    renderPage({ codex: null });
+    fireEvent.click(screen.getByRole("button", { name: "新增 Provider" }));
+    const id = screen.getByLabelText("Provider ID") as HTMLInputElement;
+    expect(id.value).toMatch(/^[a-z0-9][a-z0-9-]*$/);
+    expect(Object.keys(mockState.status?.providers ?? {})).not.toContain(id.value);
+    expect(screen.getByText(/小写字母、数字或连字符/)).toBeTruthy();
+  });
+
+  // create separates the two intents. An edit must keep overwriting, or saving a
+  // Provider you opened from the list would be refused as a duplicate of itself.
+  it("saves an edited Provider without the create flag", async () => {
+    const save = vi.spyOn(api, "saveProvider").mockResolvedValue({
+      entry: {
+        id: "ppio", name: "PPIO Cloud", home: "", base_url: "https://api.ppio.com/openai",
+        anthropic_base_url: "", api_key: "", built_in: true,
+      },
+      reapplied: null,
+      failures: null,
+    });
+    vi.spyOn(api, "getProvider").mockResolvedValue({
+      id: "ppio", name: "PPIO", home: "", base_url: "https://api.ppio.com/openai",
+      anthropic_base_url: "", api_key: "", built_in: true,
+    });
+    renderPage({ codex: "ppio" });
+    fireEvent.click(screen.getByRole("button", { name: "编辑 PPIO" }));
+    await waitFor(() => expect(screen.getByLabelText("名称")).toBeTruthy());
+    fireEvent.change(screen.getByLabelText("名称"), { target: { value: "PPIO Cloud" } });
+    fireEvent.click(screen.getByRole("button", { name: /^保存$/ }));
+    await waitFor(() => expect(save).toHaveBeenCalledWith(expect.objectContaining({ id: "ppio", create: false })));
+  });
+
+  // As on the Profiles page, this used to stub window.confirm, which the page
+  // never called -- so it passed against an unconfirmed delete. The declined case
+  // is what holds the confirmation in place.
+  const renderWithUserProvider = () => {
     renderPage({ codex: null });
     if (!mockState.status) throw new Error("missing status");
     mockState.status.providers.acme = { name: "Acme", home: "", base_url: "https://api.acme.test", custom: true };
-    vi.spyOn(window, "confirm").mockReturnValue(true);
-    const remove = vi.spyOn(api, "deleteProvider").mockResolvedValue();
     render(
       <MemoryRouter>
         <ProvidersPage />
       </MemoryRouter>,
     );
+  };
+
+  it("does not delete a Provider when the confirmation is declined", async () => {
+    question.mockResolvedValue("取消");
+    const remove = vi.spyOn(api, "deleteProvider").mockResolvedValue();
+    renderWithUserProvider();
+    fireEvent.click(screen.getByRole("button", { name: "删除 Acme" }));
+    await waitFor(() => expect(question).toHaveBeenCalled());
+    expect(remove).not.toHaveBeenCalled();
+  });
+
+  it("deletes a user Provider once the confirmation is accepted", async () => {
+    question.mockResolvedValue("删除");
+    const remove = vi.spyOn(api, "deleteProvider").mockResolvedValue();
+    renderWithUserProvider();
     fireEvent.click(screen.getByRole("button", { name: "删除 Acme" }));
     await waitFor(() => expect(remove).toHaveBeenCalledWith("acme"));
+    // The saved key going too is the part worth warning about.
+    expect(question.mock.calls[0][0].Message).toContain("API Key");
   });
 });