fix: what the third round broke, and the first batch's review

MCP preview after the server lost a session (it expired, or the MCP process restarted): - Every server state has an id, made when the state is created. The tab notices a new id even when the version numbers happen to match, and every push names the state it was based on, so one based on a lost state is refused, also when it comes before the tab's first poll (the server recovers the saved file first). - The tab keeps the newest canvas XML, saved or not. When the server knows nothing (no file) or exactly what the tab last saved, the canvas wins and is saved, so edits made while the server was down are kept. Otherwise the server's diagram (an AI write the tab missed, a cleared document that was saved) is shown and the tab's copy goes to History. - Late answers to an old state's push or poll are dropped; a failed push says the server is unreachable; Download as .drawio saves the canvas. Settings and server: - Saved providers this version does not know stay in storage with their keys, and sending no longer trips over them. - The desktop "Ollama (Local)" preset with a key goes to local Ollama again; a server model's Ollama URL variable is read; the admin panel writes Ollama Cloud's URL for a key without one. - Provider error texts show again in the desktop app and for EdgeOne. - .env: a quoted value followed by a comment ending in a quote is read as dotenv reads it; unquoted values are unchanged. - Desktop app: the next launch opens the port where a chat was last saved; a launch elsewhere that saves nothing does not move it, and a page with no chats lets the next launch try the other port once. - The Test button no longer stays busy after another tab changed the key. - A completed append_diagram is no longer undone by an earlier failed edit's preview; a file read once in vain is saved again once it is read or gone. From the first batch's review: - The admin panel's Test of an entry without a URL now tests the server's <P>_BASE_URL, where chat sends the entry's key; chat is unchanged (the first fix rerouted working setups). - The model list ends downloads that are too large, accepts answers without a body, and keeps the "redirects are not allowed" explanation. - A test covers the preview's History rendering.
2026-10-08 10:47:48 +08:00 · 2026-10-05 17:33:36 +09:00
parent 4731394f32
commit c75f74a5a0
36 changed files with 1179 additions and 188 deletions
@@ -271,4 +271,60 @@ test("a test result for an old API key is dropped", async ({ page }) => {
    release()
    await page.waitForTimeout(500)
    await expect(dialog.locator('[title="1.0 s"]')).toHaveCount(0)
+    // The test is over: the button works again and nothing spins
+    await expect(
+        dialog.getByRole("button", { name: "Test", exact: true }),
+    ).toBeEnabled()
+    await expect(dialog.locator(".animate-spin")).toHaveCount(0)
+})
+
+test("an older test does not end a newer one's spinners", async ({ page }) => {
+    // Each validate request waits for its own release
+    const releases: Array<() => void> = []
+    await page.route("**/api/validate-model", async (route) => {
+        await new Promise<void>((r) => releases.push(r))
+        await route.fulfill({
+            status: 200,
+            json: { valid: true, responseTime: 1000 },
+        })
+    })
+    const dialog = await openQwenSettings(page, TWO_PROVIDERS)
+    await dialog.getByRole("button", { name: "Test", exact: true }).click()
+    await expect.poll(() => releases.length).toBe(1)
+    // The user corrects the key and tests again
+    await dialog.locator("#api-key").fill("new-key")
+    await dialog.getByRole("button", { name: "Test", exact: true }).click()
+    await expect.poll(() => releases.length).toBe(2)
+    releases[0]()
+    await page.waitForTimeout(500)
+    await expect(dialog.locator(".animate-spin").first()).toBeVisible()
+    releases[1]()
+    await expect(dialog.locator('[title="1.0 s"]')).toHaveCount(1)
+})
+
+test("no spinner stays after another tab's change while elsewhere", async ({
+    page,
+}) => {
+    const release = await holdRoute(page, "**/api/validate-model", {
+        valid: true,
+        responseTime: 1000,
+    })
+    const dialog = await openQwenSettings(page, TWO_PROVIDERS)
+    await dialog.getByRole("button", { name: "Test", exact: true }).click()
+    // The user looks at the other provider while another tab changes the key
+    await dialog.getByText("GLM (Zhipu)").first().click()
+    await page.evaluate(() => {
+        const key = "next-ai-draw-io-model-configs"
+        const config = JSON.parse(localStorage.getItem(key) ?? "{}")
+        config.providers[0].apiKey = "key-from-another-tab"
+        const value = JSON.stringify(config)
+        localStorage.setItem(key, value)
+        window.dispatchEvent(
+            new StorageEvent("storage", { key, newValue: value }),
+        )
+    })
+    release()
+    await page.waitForTimeout(500)
+    await dialog.getByText("Qwen (Alibaba)").first().click()
+    await expect(dialog.locator(".animate-spin")).toHaveCount(0)
 })
@@ -69,6 +69,34 @@ describe("deriveEnvUpdates", () => {
        expect(updates.ADMIN_OPENAI_API_KEY_2).toBe("sk-second")
    })

+    it("sends an Ollama key without a URL to Ollama Cloud, like its Test", () => {
+        // Chat sends a server Ollama key to OLLAMA_BASE_URL, or to local
+        // Ollama without one; the Test sends it to Ollama Cloud
+        const cloud = deriveEnvUpdates(
+            [provider({ provider: "ollama", apiKey: "ollama-key" })],
+            [],
+        )
+        expect(cloud.OLLAMA_API_KEY).toBe("ollama-key")
+        expect(cloud.OLLAMA_BASE_URL).toBe("https://ollama.com/api")
+        const own = deriveEnvUpdates(
+            [
+                provider({
+                    provider: "ollama",
+                    apiKey: "k",
+                    baseUrl: "https://ollama.internal/api",
+                }),
+            ],
+            [],
+        )
+        expect(own.OLLAMA_BASE_URL).toBe("https://ollama.internal/api")
+        // No key: local Ollama, nothing to write
+        const local = deriveEnvUpdates(
+            [provider({ provider: "ollama", apiKey: undefined })],
+            [],
+        )
+        expect(local.OLLAMA_BASE_URL ?? null).toBeNull()
+    })
+
    it("maps bedrock credentials to ADMIN_AWS_* env vars", () => {
        const updates = deriveEnvUpdates(
            [
@@ -156,29 +184,6 @@ describe("adminProvidersToConfig", () => {
        expect(config.providers[1].apiKeyEnv).toBe("ADMIN_OPENAI_API_KEY_2")
    })

-    it("names its own URL variable when it has its own key, even empty", () => {
-        // Otherwise chat reads the global OPENAI_BASE_URL, which may be a
-        // proxy for another key, while the Test used the official endpoint
-        const own = adminProvidersToConfig([provider()]).providers[0]
-        expect(own.baseUrlEnv).toBe("ADMIN_OPENAI_BASE_URL")
-        // Without a key or URL of its own: the global key and URL, a pair
-        const shared = adminProvidersToConfig([provider({ apiKey: undefined })])
-            .providers[0]
-        expect(shared.baseUrlEnv).toBeUndefined()
-        // An Azure key belongs to one resource: AZURE_BASE_URL stays
-        const azure = adminProvidersToConfig([provider({ provider: "azure" })])
-            .providers[0]
-        expect(azure.baseUrlEnv).toBeUndefined()
-        expect(
-            adminProvidersToConfig([
-                provider({
-                    provider: "azure",
-                    baseUrl: "https://r.openai.azure.com/openai",
-                }),
-            ]).providers[0].baseUrlEnv,
-        ).toBe("ADMIN_AZURE_BASE_URL")
-    })
-
    it("skips providers without models and carries the default flag", () => {
        const config = adminProvidersToConfig([
            provider({ id: "p1", models: [] }),
@@ -0,0 +1,68 @@
+// @vitest-environment node
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
+
+// The request the admin Test hands to validate-model
+const sent = vi.hoisted(() => ({ body: null as any, headers: null as any }))
+vi.mock("@/app/api/validate-model/route", () => ({
+    POST: async (req: Request) => {
+        sent.body = await req.json()
+        sent.headers = Object.fromEntries(req.headers)
+        return Response.json({ valid: true })
+    },
+}))
+vi.mock("@/lib/admin/auth", () => ({ checkAdminAuth: () => null }))
+vi.mock("@/lib/admin/settings", () => ({ loadSettings: () => ({}) }))
+
+import { POST as testModel } from "@/app/api/admin/test-model/route"
+
+const ENV = ["OPENAI_BASE_URL", "SGLANG_BASE_URL", "AI_GATEWAY_BASE_URL"]
+const saved: Record<string, string | undefined> = {}
+beforeEach(() => {
+    for (const k of ENV) {
+        saved[k] = process.env[k]
+        delete process.env[k]
+    }
+})
+afterEach(() => {
+    for (const k of ENV) {
+        if (saved[k] === undefined) delete process.env[k]
+        else process.env[k] = saved[k]
+    }
+})
+
+const test = (provider: Record<string, unknown>) =>
+    testModel(
+        new Request("http://localhost/api/admin/test-model", {
+            method: "POST",
+            headers: { "Content-Type": "application/json" },
+            body: JSON.stringify({
+                provider: { id: "p1", models: ["m"], ...provider },
+                modelId: "m",
+            }),
+        }),
+    )
+
+describe("admin Test of an entry without a URL", () => {
+    it("tests the server's <P>_BASE_URL, where chat sends the entry's key", async () => {
+        // A server model without baseUrlEnv reads the global variable
+        process.env.OPENAI_BASE_URL = "https://operator-proxy.example.com/v1"
+        await test({ provider: "openai", apiKey: "panel-key" })
+        expect(sent.body.baseUrl).toBe("https://operator-proxy.example.com/v1")
+
+        process.env.AI_GATEWAY_BASE_URL = "https://gateway.example.com/v3/ai"
+        await test({ provider: "gateway", apiKey: "k" })
+        expect(sent.body.baseUrl).toBe("https://gateway.example.com/v3/ai")
+    })
+
+    it("keeps the entry's own URL, and none when the server has none", async () => {
+        process.env.SGLANG_BASE_URL = "http://gpu-box:8000/v1"
+        await test({
+            provider: "sglang",
+            apiKey: "k",
+            baseUrl: "http://other:8000/v1",
+        })
+        expect(sent.body.baseUrl).toBe("http://other:8000/v1")
+        await test({ provider: "deepseek", apiKey: "k" })
+        expect(sent.body.baseUrl).toBeUndefined()
+    })
+})
@@ -378,25 +378,6 @@ describe("whose keys a request uses", () => {
        expect(provider.chat).not.toHaveBeenCalled()
    })

-    it("sends an admin OpenAI key without a URL to the official endpoint", () => {
-        // Its URL variable is named but empty; the SDK would otherwise read
-        // the server's OPENAI_BASE_URL, a proxy for another key
-        process.env.OPENAI_BASE_URL = "https://operator-proxy.example.com/v1"
-        process.env.ADMIN_OPENAI_API_KEY = "panel-key"
-        getAIModel({
-            provider: "openai",
-            modelId: "gpt-5.5",
-            apiKeyEnv: "ADMIN_OPENAI_API_KEY",
-            baseUrlEnv: "ADMIN_OPENAI_BASE_URL",
-        })
-        expect(createOpenAI).toHaveBeenLastCalledWith(
-            expect.objectContaining({
-                apiKey: "panel-key",
-                baseURL: "https://api.openai.com/v1",
-            }),
-        )
-    })
-
    it("uses Chat Completions for any configured base URL", () => {
        // The settings form fills in the official URL for a new provider
        getAIModel({
@@ -425,21 +406,44 @@ describe("whose keys a request uses", () => {
        )
    })

-    it("sends the server's Ollama key without a base URL to Ollama Cloud", async () => {
-        // An Ollama key is an Ollama Cloud key: local Ollama has none.
-        // The admin panel saves it as OLLAMA_API_KEY, without a base URL.
+    it("sends the server's Ollama key where OLLAMA_BASE_URL says, or to local Ollama", async () => {
+        // The desktop app's "Ollama (Local)" preset puts its API Key field
+        // into OLLAMA_API_KEY; with no base URL that is the local Ollama
        process.env.OLLAMA_API_KEY = "server-key"
        const { createOllama } = await import("ollama-ai-provider-v2")
        getAIModel({ provider: "ollama", modelId: "m" })
-        expect(createOllama).toHaveBeenLastCalledWith(
-            expect.objectContaining({ baseURL: "https://ollama.com/api" }),
-        )
-        // Without a key: the SDK's local default
-        delete process.env.OLLAMA_API_KEY
-        getAIModel({ provider: "ollama", modelId: "m" })
        expect(vi.mocked(createOllama).mock.lastCall?.[0]).not.toHaveProperty(
            "baseURL",
        )
+        process.env.OLLAMA_BASE_URL = "https://ollama.com/api"
+        getAIModel({ provider: "ollama", modelId: "m" })
+        expect(createOllama).toHaveBeenLastCalledWith(
+            expect.objectContaining({ baseURL: "https://ollama.com/api" }),
+        )
+    })
+
+    it("uses a server model's own Ollama URL variable", async () => {
+        process.env.OLLAMA_BASE_URL = "http://other.internal:11434/api"
+        process.env.MY_OLLAMA_URL = "https://ollama.proxy.example/api"
+        process.env.MY_OLLAMA_KEY = "proxy-key"
+        try {
+            const { createOllama } = await import("ollama-ai-provider-v2")
+            getAIModel({
+                provider: "ollama",
+                modelId: "m",
+                apiKeyEnv: "MY_OLLAMA_KEY",
+                baseUrlEnv: "MY_OLLAMA_URL",
+            })
+            expect(createOllama).toHaveBeenLastCalledWith(
+                expect.objectContaining({
+                    baseURL: "https://ollama.proxy.example/api",
+                    headers: { Authorization: "Bearer proxy-key" },
+                }),
+            )
+        } finally {
+            delete process.env.MY_OLLAMA_URL
+            delete process.env.MY_OLLAMA_KEY
+        }
    })

    it("needs a base URL with a user's Azure key", () => {
@@ -462,11 +462,11 @@ describe("Ollama API key security", () => {

        expect(createOllamaMock).toHaveBeenCalledTimes(1)
        const callArgs = createOllamaMock.mock.calls[0][0]
-        // As env.example says: without OLLAMA_BASE_URL, Ollama Cloud (the
-        // SDK's default is the local server, which has no keys)
+        // The SDK's local default: the desktop app's "Ollama (Local)"
+        // preset puts its API Key field into OLLAMA_API_KEY
+        expect(callArgs).not.toHaveProperty("baseURL")
        expect(callArgs).toEqual(
            expect.objectContaining({
-                baseURL: "https://ollama.com/api",
                headers: { Authorization: "Bearer server-key" },
            }),
        )
@@ -0,0 +1,126 @@
+// @vitest-environment node
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
+
+// No DNS in tests: only loopback addresses are private
+vi.mock("@/lib/ssrf-protection", async (importOriginal) => ({
+    ...(await importOriginal<typeof import("@/lib/ssrf-protection")>()),
+    isPrivateUrl: async (url: string) =>
+        /^https?:\/\/(127\.0\.0\.1|localhost)\b/.test(url),
+}))
+
+import { POST as chat } from "@/app/api/chat/route"
+
+const ENV = [
+    "AI_PROVIDER",
+    "AI_MODEL",
+    "OPENAI_API_KEY",
+    "OLLAMA_BASE_URL",
+    "OLLAMA_API_KEY",
+    "NEXT_AI_DRAWIO_DESKTOP",
+]
+const saved: Record<string, string | undefined> = {}
+
+beforeEach(() => {
+    for (const k of ENV) saved[k] = process.env[k]
+    for (const k of ENV) delete process.env[k]
+})
+
+afterEach(() => {
+    for (const k of ENV) {
+        if (saved[k] === undefined) delete process.env[k]
+        else process.env[k] = saved[k]
+    }
+    vi.unstubAllGlobals()
+})
+
+/** Every provider request answers with this status and text */
+const providerAnswers = (status: number, body: string) =>
+    vi.stubGlobal(
+        "fetch",
+        vi.fn(
+            async () =>
+                new Response(body, {
+                    status,
+                    headers: { "Content-Type": "application/json" },
+                }),
+        ),
+    )
+
+/** The error text the chat panel gets from the stream */
+async function streamedError(headers: Record<string, string>) {
+    const res = await chat(
+        new Request("http://localhost/api/chat", {
+            method: "POST",
+            headers: { "Content-Type": "application/json", ...headers },
+            body: JSON.stringify({
+                messages: [
+                    {
+                        id: "u1",
+                        role: "user",
+                        parts: [{ type: "text", text: "Draw two boxes" }],
+                    },
+                ],
+                xml: "",
+            }),
+        }),
+    )
+    const text = await res.text()
+    const line = text.split("\n").find((l) => l.includes('"type":"error"'))
+    return line ? JSON.parse(JSON.parse(line.slice(6)).errorText).message : ""
+}
+
+describe("provider error texts in the stream", () => {
+    it("shows the user's own local Ollama error in the desktop app", async () => {
+        process.env.NEXT_AI_DRAWIO_DESKTOP = "1"
+        process.env.AI_PROVIDER = "ollama"
+        process.env.AI_MODEL = "llama3"
+        // Ollama is not running
+        vi.stubGlobal(
+            "fetch",
+            vi.fn(async () => {
+                throw Object.assign(new TypeError("fetch failed"), {
+                    cause: new Error("connect ECONNREFUSED 127.0.0.1:11434"),
+                })
+            }),
+        )
+        expect(await streamedError({})).toMatch(
+            /127\.0\.0\.1:11434|fetch failed/,
+        )
+        // The SDK retries a refused connection twice, waiting between
+    }, 20_000)
+
+    it("shows EdgeOne's own daily quota explanation", async () => {
+        // The function answers 429, which the SDK retries with a wait;
+        // the status does not decide whether the text is shown
+        providerAnswers(
+            400,
+            JSON.stringify({
+                error: {
+                    message:
+                        "The daily public quota has been exhausted. After deployment, you can enjoy a personal daily exclusive quota.",
+                },
+            }),
+        )
+        expect(
+            await streamedError({
+                "x-ai-provider": "edgeone",
+                "x-ai-model": "@tx/deepseek-ai/deepseek-v3-0324",
+            }),
+        ).toMatch(/daily public quota/)
+    })
+
+    it("hides the provider's text on the server's own key", async () => {
+        process.env.AI_PROVIDER = "openai"
+        process.env.AI_MODEL = "gpt-5.5"
+        process.env.OPENAI_API_KEY = "server-key"
+        providerAnswers(
+            403,
+            JSON.stringify({
+                error: { message: "Organization org-operator is suspended" },
+            }),
+        )
+        const message = await streamedError({})
+        expect(message).not.toMatch(/org-operator/)
+        expect(message).toBe("The provider returned an error.")
+    })
+})
@@ -15,7 +15,17 @@ vi.mock("electron", () => ({

 import { loadEnvFile } from "@/electron/main/env-loader"

-const KEYS = ["T_JSON", "T_COMMENT", "T_PLAIN", "T_DOUBLE"]
+const KEYS = [
+    "T_JSON",
+    "T_COMMENT",
+    "T_PLAIN",
+    "T_DOUBLE",
+    "T_QUOTED_COMMENT",
+    "T_KEY_COMMENT",
+    "T_HASH",
+    "T_AFTER",
+    "T_JOINED",
+]
 afterEach(() => {
    for (const k of KEYS) delete process.env[k]
 })
@@ -39,4 +49,27 @@ describe("loadEnvFile", () => {
        expect(process.env.T_PLAIN).toBe("plain")
        expect(process.env.T_DOUBLE).toBe(`say "hi"`)
    })
+
+    it("drops a comment that ends with a quote, like dotenv", () => {
+        // Expected values checked against dotenv 16.6.1's parse
+        dir.path = mkdtempSync(join(tmpdir(), "env-loader-"))
+        writeFileSync(
+            join(dir.path, ".env"),
+            [
+                `T_QUOTED_COMMENT="gpt-5" # pick "fast"`,
+                `T_KEY_COMMENT="sk-abc" # from "Team A"`,
+                `T_AFTER='a' b`,
+                `T_JOINED="a"b`,
+                // Unquoted: a # without a space before it stays in the value
+                // (dotenv would cut it; this loader never did)
+                "T_HASH=http://host/#/x",
+            ].join("\n"),
+        )
+        loadEnvFile()
+        expect(process.env.T_QUOTED_COMMENT).toBe("gpt-5")
+        expect(process.env.T_KEY_COMMENT).toBe("sk-abc")
+        expect(process.env.T_AFTER).toBe(`'a' b`)
+        expect(process.env.T_JOINED).toBe(`"a"b`)
+        expect(process.env.T_HASH).toBe("http://host/#/x")
+    })
 })
@@ -0,0 +1,46 @@
+import { readFileSync } from "node:fs"
+import { join } from "node:path"
+import { describe, expect, it } from "vitest"
+
+// The MCP preview page, as the server fills it in, with its script run in
+// this document (no session id, so it does not poll)
+const dir = join(process.cwd(), "packages/mcp-server/src/preview")
+const html = readFileSync(join(dir, "index.html"), "utf8")
+    .replace("{{CSS}}", "")
+    .replace("{{SESSION_BADGE}}", "")
+    .replaceAll("{{DISABLED}}", "")
+    .replace("{{DRAWIO_URL}}", "about:blank")
+    .replace("{{SESSION_JSON}}", '""')
+    .replace("{{ORIGIN_JSON}}", '"https://embed.diagrams.net"')
+const scripts = [...html.matchAll(/<script>([\s\S]*?)<\/script>/g)].map((m) =>
+    m[1].replace("{{SCRIPT}}", ""),
+)
+const preview = readFileSync(join(dir, "preview.js"), "utf8")
+
+function renderHistory(entries: unknown[]): HTMLElement {
+    document.body.innerHTML = html.replace(/<script>[\s\S]*?<\/script>/g, "")
+    // One scope, as the page's scripts share one; returns its renderHistory
+    const run = new Function(
+        `${scripts.join("\n")}\n${preview}\nreturn (d) => { historyData = d; renderHistory(); }`,
+    )
+    run()(entries)
+    return document.getElementById("history-grid") as HTMLElement
+}
+
+describe("MCP preview History", () => {
+    it("never reads a stored thumbnail as HTML", () => {
+        const grid = renderHistory([
+            { id: 1, index: 0, svg: 'x" onerror="window.__xss=1' },
+            { id: 2, index: 1, svg: "javascript:window.__xss=2" },
+            { id: 3, index: 2, svg: "data:image/svg+xml;base64,PHN2Zy8+" },
+        ])
+        const images = [...grid.querySelectorAll("img")]
+        expect(images.map((i) => i.getAttribute("src"))).toEqual([
+            "data:image/svg+xml;base64,PHN2Zy8+",
+        ])
+        expect(grid.querySelector("[onerror]")).toBeNull()
+        // Entries without a usable picture show their number
+        expect(grid.textContent).toContain("#0")
+        expect(grid.textContent).toContain("#1")
+    })
+})
@@ -34,6 +34,8 @@ vi.mock("node:net", () => ({

 import {
    findAvailablePort,
+    noteNoChats,
+    rememberChatPort,
    resetAllocatedPort,
 } from "@/electron/main/port-manager"

@@ -97,3 +99,55 @@ describe("findAvailablePort", () => {
        expect(await launch()).toBe(13371)
    })
 })
+
+describe("the port where chats were last saved", () => {
+    it("opens there first", async () => {
+        // Windows reserved 61337 for a while, and the user kept working
+        storeData(61337)
+        busy.ports[61337] = "EACCES"
+        expect(await launch()).toBe(13370)
+        storeData(13370)
+        rememberChatPort()
+        busy.ports = {}
+        expect(await launch()).toBe(13370)
+    })
+
+    it("does not move after a launch elsewhere that saved nothing", async () => {
+        storeData(61337)
+        expect(await launch()).toBe(61337)
+        rememberChatPort()
+        busy.ports[61337] = "EADDRINUSE"
+        expect(await launch()).toBe(13370)
+        storeData(13370)
+        busy.ports = {}
+        expect(await launch()).toBe(61337)
+    })
+
+    it("never stores a last-resort port, which changes between launches", async () => {
+        busy.ports[61337] = "EACCES"
+        busy.ports[13370] = "EADDRINUSE"
+        expect(await launch()).toBe(13371)
+        rememberChatPort()
+        busy.ports = {}
+        expect(await launch()).toBe(61337)
+    })
+
+    it("tries the other port after opening on one without chats", async () => {
+        // Split before this version: chats only on 13370, and a launch on
+        // 61337 created that origin's folder
+        storeData(13370)
+        storeData(61337)
+        expect(await launch()).toBe(61337)
+        noteNoChats()
+        expect(await launch()).toBe(13370)
+        // Once a choice is stored, an empty page changes nothing
+        noteNoChats()
+        expect(await launch()).toBe(13370)
+    })
+
+    it("stays put for a new user", async () => {
+        expect(await launch()).toBe(61337)
+        noteNoChats()
+        expect(await launch()).toBe(61337)
+    })
+})
@@ -1,5 +1,13 @@
 // @vitest-environment node
 import { afterEach, describe, expect, it, vi } from "vitest"
+
+// No DNS in tests: only loopback addresses are private
+vi.mock("@/lib/ssrf-protection", async (importOriginal) => ({
+    ...(await importOriginal<typeof import("@/lib/ssrf-protection")>()),
+    isPrivateUrl: async (url: string) =>
+        /^https?:\/\/(127\.0\.0\.1|localhost)\b/.test(url),
+}))
+
 import { POST as providerModels } from "@/app/api/provider-models/route"
 import {
    canListModels,
@@ -227,6 +235,64 @@ describe("POST /api/provider-models", () => {
        }
    })

+    it("ends the download of a list that is too large", async () => {
+        // The answer announces 4 MB and never finishes
+        let signal: AbortSignal | undefined
+        vi.stubGlobal(
+            "fetch",
+            vi.fn(async (_url: string, init?: RequestInit) => {
+                signal = init?.signal ?? undefined
+                const body = new ReadableStream({ start() {} })
+                return new Response(body, {
+                    headers: { "content-length": String(4 * 1024 * 1024) },
+                })
+            }),
+        )
+        const data = await (
+            await post({
+                provider: "ollama",
+                baseUrl: "https://big.example.com",
+            })
+        ).json()
+        expect(data.error).toBe("The model list is too large.")
+        expect(signal?.aborted).toBe(true)
+    })
+
+    it("handles answers without a body", async () => {
+        for (const status of [204, 304]) {
+            vi.stubGlobal(
+                "fetch",
+                vi.fn(async () => new Response(null, { status })),
+            )
+            const data = await (
+                await post({ provider: "ollama", baseUrl: "https://x.example" })
+            ).json()
+            expect(data.error).toMatch(/not valid JSON|failed \(304\)/)
+        }
+    })
+
+    it("explains a refused redirect", async () => {
+        process.env.ALLOW_PRIVATE_URLS = "false"
+        try {
+            vi.stubGlobal(
+                "fetch",
+                vi.fn(
+                    async () =>
+                        new Response(null, {
+                            status: 301,
+                            headers: { location: "https://elsewhere.example" },
+                        }),
+                ),
+            )
+            const data = await (
+                await post({ provider: "ollama", baseUrl: "https://x.example" })
+            ).json()
+            expect(data.error).toMatch(/Redirects are not allowed/)
+        } finally {
+            delete process.env.ALLOW_PRIVATE_URLS
+        }
+    })
+
    it("keeps its own explanations and hides other error texts", async () => {
        // Our own: no base URL for SGLang
        const own = await (
@@ -0,0 +1,82 @@
+import { renderHook } from "@testing-library/react"
+import { describe, expect, it, vi } from "vitest"
+import { useDiagramToolHandlers } from "@/hooks/use-diagram-tool-handlers"
+
+const geometry =
+    '<mxGeometry x="0" y="0" width="80" height="40" as="geometry"/>'
+const box = (id: string) =>
+    `<mxCell id="${id}" value="${id}" vertex="1" parent="1">${geometry}</mxCell>`
+
+function setup(partialXml: string) {
+    const refs = {
+        partialXmlRef: { current: partialXml },
+        // A failed edit's preview is still on the canvas, its original kept
+        editDiagramOriginalXmlRef: {
+            current: new Map([["edit-1", "<mxfile>original</mxfile>"]]),
+        },
+        processedToolCallsRef: { current: new Set<string>() },
+        validationRetryCountRef: { current: 0 },
+        chartXMLRef: { current: "" },
+    }
+    const onDisplayChart = vi.fn(
+        (_xml: string, _skipValidation?: boolean): string | null => null,
+    )
+    const { result } = renderHook(() =>
+        useDiagramToolHandlers({
+            ...refs,
+            onDisplayChart,
+            onFetchChart: async () => "",
+            onExport: () => {},
+            enableVlmValidation: false,
+        }),
+    )
+    const addToolOutput = vi.fn()
+    const append = (xml: string) =>
+        result.current.handleToolCall(
+            {
+                toolCall: {
+                    toolCallId: "append-1",
+                    toolName: "append_diagram",
+                    input: { xml },
+                },
+            },
+            addToolOutput,
+        )
+    return { refs, onDisplayChart, addToolOutput, append }
+}
+
+describe("append_diagram and the stored previews", () => {
+    it("takes the stored originals when it draws the completed diagram", async () => {
+        // Otherwise the preview code later loads the failed edit's original
+        // over the completed diagram
+        const { refs, onDisplayChart, append } = setup(
+            `${box("2")}<mxCell id="3" value="3" vertex="1" parent="1"><mxGeometry x="0" y="0" width="8`,
+        )
+        await append('0" height="40" as="geometry"/></mxCell>')
+        expect(onDisplayChart).toHaveBeenCalledTimes(1)
+        expect(onDisplayChart.mock.calls[0][0]).toContain('id="3"')
+        expect(refs.editDiagramOriginalXmlRef.current.size).toBe(0)
+        expect(refs.processedToolCallsRef.current.has("edit-1")).toBe(true)
+    })
+
+    it("leaves them while the diagram is still incomplete", async () => {
+        // Nothing is drawn, so the failed edit's preview must still be undone
+        const { refs, onDisplayChart, append } = setup(
+            `${box("2")}<mxCell id="3" value="3" vertex="1" parent="1"><mxGeometry x="0" y="0" width="8`,
+        )
+        await append('0" height="40"')
+        expect(onDisplayChart).not.toHaveBeenCalled()
+        expect(refs.editDiagramOriginalXmlRef.current.size).toBe(1)
+        expect(refs.processedToolCallsRef.current.has("edit-1")).toBe(false)
+    })
+
+    it("leaves them when the assembled diagram is invalid", async () => {
+        const { refs, onDisplayChart, addToolOutput, append } = setup(
+            `<mxCell id="1" value="root id" vertex="1" parent="1">${geometry}</mxCell><mxCell id="3" value="3" vertex="1" parent="1"><mxGeometry x="0" y="0" width="8`,
+        )
+        await append('0" height="40" as="geometry"/></mxCell>')
+        expect(onDisplayChart).not.toHaveBeenCalled()
+        expect(addToolOutput.mock.calls[0][0].state).toBe("output-error")
+        expect(refs.editDiagramOriginalXmlRef.current.size).toBe(1)
+    })
+})
@@ -1,6 +1,6 @@
 import { act, cleanup, renderHook, waitFor } from "@testing-library/react"
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
-import { useModelConfig } from "@/hooks/use-model-config"
+import { getSelectedAIConfig, useModelConfig } from "@/hooks/use-model-config"
 import type { FlattenedServerModel } from "@/lib/server-model-config"
 import { STORAGE_KEYS } from "@/lib/storage"
 import type { MultiModelConfig } from "@/lib/types/model-config"
@@ -126,6 +126,43 @@ describe("useModelConfig server model selection", () => {
        expect(result.current.models.map((m) => m.id)).toContain("m1")
    })

+    it("keeps an unknown provider and its key in storage", async () => {
+        // The version that saved it may be opened again (an older desktop
+        // build, another tab): the provider must still be there
+        storeConfig({
+            ...USER_CONFIG,
+            providers: [
+                ...USER_CONFIG.providers,
+                {
+                    id: "p9",
+                    provider: "not-a-provider" as any,
+                    apiKey: "k9",
+                    models: [{ id: "m9", modelId: "x" }],
+                },
+            ],
+            selectedModelId: "m1",
+        })
+        const { result } = await renderLoaded()
+        act(() => result.current.setSelectedModelId(undefined))
+        await waitFor(() => {
+            const stored = JSON.parse(
+                localStorage.getItem(STORAGE_KEYS.modelConfigs) ?? "{}",
+            )
+            expect(stored.selectedModelId).toBeUndefined()
+            expect(stored.providers.map((p: { id: string }) => p.id)).toEqual([
+                "p1",
+                "p9",
+            ])
+            expect(stored.providers[1].apiKey).toBe("k9")
+        })
+        // Sending reads the stored config too, and must not trip over it
+        act(() => result.current.setSelectedModelId("m1"))
+        expect(getSelectedAIConfig()).toMatchObject({
+            aiProvider: "openai",
+            aiModel: "gpt-4o",
+        })
+    })
+
    it("keeps a selected user model", async () => {
        storeConfig({ ...USER_CONFIG, selectedModelId: "m1" })
        const { result } = await renderLoaded()