fix: what the batch C review found

- A redirect followed for a custom base URL (quota on) no longer carries
  the user's key or cookies to another origin, as fetch itself does, and
  a private address may redirect to another private one (already counted).
- The admin Test of an Ollama or Vertex AI entry without a URL goes where
  chat sends that entry's key: the environment's own URL variable, for
  Ollama else the local default. The Test of an Ollama Cloud key without
  a URL went to the cloud while chat went to local Ollama.
- Chat saves: each save notes the chat on screen and the order of the
  reads before reading its data. A save read before switching chats no
  longer writes into the chat switched to, and a copy that waited for its
  thumbnail no longer replaces a newer one.
- "Continue without saving" keeps its button when a later auto-save fails,
  and goes away when a new message is sent.
- A screenshot check that was waiting for its image when the user pressed
  Stop stays skipped after the next message.
- MCP History: draw.io's own copy of a diagram (after get_diagram) no
  longer adds an entry without a picture; a change of background is still
  its own version. The tab ignores an edit's answer that arrives after a
  newer AI write loaded.
- Desktop: a deleted preset is not brought back by a failed switch, and a
  request naming no preset does not stop a rollback. An origin keeping
  an access code counts as having settings.
- .env: a quoted value ending in a backslash ("C:\dir\") is read as dotenv
  reads it.
- The tool card no longer crashes on an id that does not turn into text;
  an older Test's success timer no longer ends a newer Test's spinner.
- Tests that passed without their fix now check it.
This commit is contained in:
dayuan.jiang
2026-10-05 20:09:42 +09:00
parent c0fa997186
commit 69c7896002
23 changed files with 461 additions and 63 deletions
+24 -1
View File
@@ -11,13 +11,19 @@ vi.mock("@/app/api/validate-model/route", () => ({
},
}))
vi.mock("@/lib/admin/auth", () => ({ checkAdminAuth: () => null }))
vi.mock("@/lib/admin/settings", () => ({ loadSettings: () => ({}) }))
// The environment's own values, under the panel's settings
const envFallback = vi.hoisted(() => ({ values: {} as Record<string, string> }))
vi.mock("@/lib/admin/settings", () => ({
loadSettings: () => ({}),
getEnvFallback: (key: string) => envFallback.values[key] ?? null,
}))
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(() => {
envFallback.values = {}
for (const k of ENV) {
saved[k] = process.env[k]
delete process.env[k]
@@ -74,8 +80,25 @@ describe("admin Test of an entry without a URL", () => {
try {
await test({ provider: "vertexai", vertexApiKey: "new-key" })
expect(sent.body.baseUrl).toBeUndefined()
// The environment's own URL, which chat uses once it is saved
envFallback.values.GOOGLE_VERTEX_BASE_URL =
"https://vertex-proxy.example.com"
await test({ provider: "vertexai", vertexApiKey: "new-key" })
expect(sent.body.baseUrl).toBe("https://vertex-proxy.example.com")
expect(sent.body.serverBaseUrl).toBe(true)
} finally {
delete process.env.GOOGLE_VERTEX_BASE_URL
}
})
it("tests Ollama where chat sends the entry's key", async () => {
// Chat on the saved entry: OLLAMA_BASE_URL of the environment, else
// the SDK's local default (the Test used to go to Ollama Cloud)
await test({ provider: "ollama", apiKey: "k" })
expect(sent.body.baseUrl).toBe("http://127.0.0.1:11434/api")
expect(sent.body.serverBaseUrl).toBe(true)
envFallback.values.OLLAMA_BASE_URL = "http://gpu:11434/api"
await test({ provider: "ollama", apiKey: "k" })
expect(sent.body.baseUrl).toBe("http://gpu:11434/api")
})
})
@@ -11,6 +11,7 @@ const settings = vi.hoisted(() => ({ values: {} as Record<string, string> }))
vi.mock("@/lib/admin/settings", () => ({
loadSettings: () => settings.values,
getEnvFallback: (key: string) => process.env[key] ?? null,
}))
vi.mock("@ai-sdk/google-vertex", () => {
+21
View File
@@ -21,6 +21,7 @@ const state = vi.hoisted(() => ({
}))
vi.mock("@/electron/main/config-manager", () => ({
applyPresetToEnv: (id: string) => {
if (id === "missing") return null
state.current = id
return { AI_PROVIDER: id }
},
@@ -80,4 +81,24 @@ describe("switchPreset", () => {
await third
expect(state.current).toBe("B")
})
it("does not bring back the old preset over a deletion", async () => {
const toB = switchPreset("B").catch(() => {})
// B is deleted while its restart is pending
state.current = null
state.restarts[0].reject(new Error("timed out"))
await toB
expect(state.current).toBeNull()
expect(state.restarts).toHaveLength(1)
})
it("still rolls back when a later request named no preset", async () => {
const toB = switchPreset("B").catch(() => {})
await expect(switchPreset("missing")).rejects.toThrow("not found")
state.restarts[0].reject(new Error("timed out"))
await new Promise((r) => setTimeout(r, 0))
state.restarts[1]?.resolve()
await toB
expect(state.current).toBe("A")
})
})
+3 -1
View File
@@ -163,11 +163,13 @@ describe("chat quota", () => {
it("does not count a provider that never uses the base URL header", async () => {
// Bedrock on the user's own AWS keys goes to AWS, whatever the
// leftover base URL says
// leftover base URL says (with a key header too, so the request gets
// past the custom URL check to the quota decision)
const res = await send({
"x-ai-provider": "bedrock",
"x-ai-model": "amazon.nova-lite-v1:0",
"x-ai-base-url": "http://127.0.0.1:8080",
"x-ai-api-key": "leftover",
"x-aws-access-key-id": "id",
"x-aws-secret-access-key": "secret",
"x-aws-region": "us-east-1",
+16
View File
@@ -28,6 +28,8 @@ const KEYS = [
"T_ESC_HASH",
"T_ESC_INNER",
"T_ESC_COMMENT",
"T_DIR",
"T_DIR_COMMENT",
]
afterEach(() => {
for (const k of KEYS) delete process.env[k]
@@ -92,4 +94,18 @@ describe("loadEnvFile", () => {
expect(process.env.T_ESC_INNER).toBe('a # \\"b\\"')
expect(process.env.T_ESC_COMMENT).toBe('x\\"y')
})
it("keeps a backslash before the closing quote, like dotenv", () => {
dir.path = mkdtempSync(join(tmpdir(), "env-loader-"))
writeFileSync(
join(dir.path, ".env"),
['T_DIR="C:\\dir\\"', 'T_DIR_COMMENT="C:\\data\\" # dir'].join(
"\n",
),
)
loadEnvFile()
// Windows folders; dotenv 16.6.1 reads them the same
expect(process.env.T_DIR).toBe("C:\\dir\\")
expect(process.env.T_DIR_COMMENT).toBe("C:\\data\\")
})
})
+44 -3
View File
@@ -238,6 +238,27 @@ describe("MCP preview after the server recreated its session", () => {
expect(t.next("POST").body.xml).toBe("<mxfile>A</mxfile>")
})
it("ignores an edit's answer that comes after a newer AI write loaded", async () => {
const t = await inStep()
t.fromDrawio({ event: "autosave", xml: "<mxfile>B</mxfile>" })
t.fromDrawio({ event: "export", data: "<svg/>" })
await t.settle()
const pushB = t.next("POST")
// The AI wrote X after B; the poll's answer comes first
const poll = t.page.poll()
t.next("GET").answer(state("S1", 4, "<mxfile>X</mxfile>"))
await poll
pushB.answer({ status: 200, body: { success: true, version: 3 } })
await t.settle()
await t.settle()
expect(t.page.read()).toMatchObject({
currentVersion: 4,
lastXml: "<mxfile>X</mxfile>",
})
// No push of the AI's diagram as the user's edit
expect(t.calls.filter((c) => c.method === "POST")).toHaveLength(0)
})
it("sends nothing more after a sync reply", async () => {
const t = await inStep()
const poll = t.page.poll()
@@ -298,13 +319,27 @@ describe("MCP preview thumbnails and downloads", () => {
it("drops the reply to an older thumbnail export", async () => {
const { t, n } = await loadedB()
// The next AI write loads before draw.io answered the first export
const poll = t.page.poll()
t.next("GET").answer(state("S1", 4, "<mxfile>C</mxfile>"))
await poll
await new Promise((r) => setTimeout(r, 600))
const newer = t.toDrawio.at(-1).thumbExport
expect(newer).toBeGreaterThan(n)
t.fromDrawio({
event: "export",
data: "<svg/>",
message: { thumbExport: n - 1 },
data: "<svg>B</svg>",
message: { thumbExport: n },
})
await t.settle()
expect(thumbnailPosts(t)).toHaveLength(0)
t.fromDrawio({
event: "export",
data: "<svg>C</svg>",
message: { thumbExport: newer },
})
await t.settle()
expect(thumbnailPosts(t).map((c) => c.body.version)).toEqual([4])
})
it("drops the image when the user changed the canvas since the load", async () => {
@@ -312,11 +347,17 @@ describe("MCP preview thumbnails and downloads", () => {
t.fromDrawio({ event: "autosave", xml: "<mxfile>B edited</mxfile>" })
t.fromDrawio({
event: "export",
data: "<svg/>",
data: "<svg>thumbnail</svg>",
message: { thumbExport: n },
})
await t.settle()
expect(thumbnailPosts(t)).toHaveLength(0)
// The edit is saved with the image of its own export
t.fromDrawio({ event: "export", data: "<svg>edit</svg>" })
await t.settle()
const push = t.next("POST")
expect(push.body.xml).toBe("<mxfile>B edited</mxfile>")
expect(atob(push.body.svg.split(",")[1])).toBe("<svg>edit</svg>")
})
it("downloads the canvas with an edit the server did not get", async () => {
+52
View File
@@ -143,6 +143,58 @@ describe("redirectGuardedFetch with the quota on", () => {
expect(fetch).toHaveBeenCalledTimes(1)
})
it("follows a private address's redirect to another one", async () => {
// Counted as the server's from the start
vi.stubGlobal(
"fetch",
answers({
"http://10.0.0.5:4000/v1/chat": new Response(null, {
status: 307,
headers: { location: "http://10.0.0.6:4000/v1/chat" },
}),
"http://10.0.0.6:4000/v1/chat": new Response("ok"),
}),
)
const res = await redirectGuardedFetch()?.(
"http://10.0.0.5:4000/v1/chat",
{ method: "POST", body: "{}" },
)
expect(await res?.text()).toBe("ok")
})
it("sends no credentials to another origin", async () => {
const fetchMock = answers({
"https://proxy.example/v1/chat": new Response(null, {
status: 307,
headers: { location: "https://other.example/v1/chat" },
}),
"https://other.example/v1/chat": new Response("ok"),
})
vi.stubGlobal("fetch", fetchMock)
await redirectGuardedFetch()?.("https://proxy.example/v1/chat", {
method: "POST",
body: "{}",
headers: {
Authorization: "Bearer user-key",
Cookie: "eo_token=1",
"Content-Type": "application/json",
},
})
const sent = (call: number) =>
new Headers(
(
fetchMock.mock.calls[call] as unknown as [
string,
RequestInit,
]
)[1].headers,
)
expect(sent(0).get("authorization")).toBe("Bearer user-key")
expect(sent(1).get("authorization")).toBeNull()
expect(sent(1).get("cookie")).toBeNull()
expect(sent(1).get("content-type")).toBe("application/json")
})
it("is not used without the quota", () => {
delete process.env.DYNAMODB_QUOTA_TABLE
expect(redirectGuardedFetch()).toBeUndefined()
+2
View File
@@ -16,6 +16,8 @@ describe("ToolCallCard", () => {
null,
{ operation: {} },
{ operation: "add", cell_id: {} },
// JSON can hold an object that does not turn into text
JSON.parse('{"operation":"add","cell_id":{"toString":null}}'),
{ operation: "add", cell_id: "2", new_xml: {} },
{ operation: "update", cell_id: "3", new_xml: '<mxCell id="3"/>' },
]
+36 -5
View File
@@ -47,8 +47,9 @@ function setup(partialXml: string) {
describe("the screenshot check and Stop", () => {
const draw = async (opts: {
isStopped: () => boolean
watchStop: () => () => boolean
validateDiagram: () => Promise<any>
captureValidationPng?: () => Promise<string>
}) => {
const onValidationStateChange = vi.fn()
const { result } = renderHook(() =>
@@ -62,9 +63,11 @@ describe("the screenshot check and Stop", () => {
onFetchChart: async () => "",
onExport: () => {},
enableVlmValidation: true,
captureValidationPng: async () => "data:image/png;base64,AA",
captureValidationPng:
opts.captureValidationPng ??
(async () => "data:image/png;base64,AA"),
validateDiagram: opts.validateDiagram,
isStopped: opts.isStopped,
watchStop: opts.watchStop,
onValidationStateChange,
}),
)
@@ -89,7 +92,7 @@ describe("the screenshot check and Stop", () => {
suggestions: [],
}))
const { addToolOutput, onValidationStateChange } = await draw({
isStopped: () => true,
watchStop: () => () => true,
validateDiagram,
})
expect(validateDiagram).not.toHaveBeenCalled()
@@ -103,7 +106,7 @@ describe("the screenshot check and Stop", () => {
it("ends with the diagram's result when Stop cancels a running check", async () => {
const { addToolOutput, onValidationStateChange } = await draw({
isStopped: () => false,
watchStop: () => () => false,
validateDiagram: async () => {
throw new DOMException("Validation cancelled", "AbortError")
},
@@ -114,6 +117,34 @@ describe("the screenshot check and Stop", () => {
expect(addToolOutput).toHaveBeenCalledTimes(1)
expect(addToolOutput.mock.lastCall?.[0].state).toBeUndefined()
})
it("skips the check when Stop came during the screenshot", async () => {
// As the chat panel counts it: the next message already cleared
// the stop flag when the screenshot arrives
let stops = 0
let stoppedNow = false
const validateDiagram = vi.fn(async () => ({
valid: true,
issues: [],
suggestions: [],
}))
const { onValidationStateChange } = await draw({
watchStop: () => {
const before = stops
return () => stoppedNow || stops !== before
},
captureValidationPng: async () => {
stops++ // Stop
stoppedNow = false // the next message
return "data:image/png;base64,AA"
},
validateDiagram,
})
expect(validateDiagram).not.toHaveBeenCalled()
expect(onValidationStateChange.mock.lastCall?.[1].status).toBe(
"skipped",
)
})
})
describe("append_diagram and the stored previews", () => {
+64 -2
View File
@@ -96,7 +96,7 @@ describe("saving the chat on screen", () => {
it("drops a save scheduled before New Chat", async () => {
const { result } = await setup()
const scheduled = result.current.getChatGeneration()
const scheduled = result.current.getSaveTicket()
act(() => result.current.clearCurrentSession())
let save!: Promise<boolean>
act(() => {
@@ -110,7 +110,7 @@ describe("saving the chat on screen", () => {
it("drops a save of the old chat waiting behind New Chat's save", async () => {
const { result } = await setup()
// The auto-save is scheduled, then New Chat saves and clears
const scheduled = result.current.getChatGeneration()
const scheduled = result.current.getSaveTicket()
let newChatSave!: Promise<boolean>
let autoSave!: Promise<boolean>
act(() => {
@@ -168,3 +168,65 @@ describe("saving the chat on screen", () => {
expect(hook.result.current.currentSessionId).toBeNull()
})
})
describe("save tickets", () => {
beforeEach(() => {
stored.clear()
pendingWrites = []
})
const textOf = (session: any) => session?.messages[0].parts[0].text
const said = (text: string) => ({
...data,
messages: [{ ...data.messages[0], parts: [{ type: "text", text }] }],
})
it("never put an older copy of a chat over a newer one", async () => {
const { result } = await setup()
let first!: Promise<boolean>
act(() => {
first = result.current.saveCurrentSession(said("first"))
})
await finishWrites()
await first
// An auto-save read its data, then waits for its thumbnail; a save
// without a thumbnail reads newer data and is done first
const older = result.current.getSaveTicket()
const newer = result.current.getSaveTicket()
let saves!: Promise<boolean[]>
act(() => {
saves = Promise.all([
result.current.saveCurrentSession(said("newer"), newer),
result.current.saveCurrentSession(said("older"), older),
])
})
await finishWrites()
await saves
expect(textOf([...stored.values()][0])).toBe("newer")
})
it("keep a chat read before a switch out of the chat switched to", async () => {
stored.set("other", {
...said("other chat"),
id: "other",
title: "Other",
})
const { result } = await setup()
// New Chat reads this chat, then waits for its thumbnail
const ticket = result.current.getSaveTicket()
// Meanwhile the user opens the other chat
let open!: Promise<unknown>
act(() => {
open = result.current.switchSession("other")
})
await finishWrites()
await open
let late!: Promise<boolean>
act(() => {
late = result.current.saveCurrentSession(said("this chat"), ticket)
})
await finishWrites()
await late
expect(textOf(stored.get("other"))).toBe("other chat")
expect(stored.size).toBe(1)
})
})
+4 -1
View File
@@ -6,7 +6,10 @@ import { POST as validateModel } from "@/app/api/validate-model/route"
import { getAIModel } from "@/lib/ai-providers"
// No saved admin providers
vi.mock("@/lib/admin/settings", () => ({ loadSettings: () => ({}) }))
vi.mock("@/lib/admin/settings", () => ({
loadSettings: () => ({}),
getEnvFallback: (key: string) => process.env[key] ?? null,
}))
// Every URL is public (no DNS in tests), unless a test says otherwise
const privateUrls = vi.hoisted(() => ({ all: false }))