fix: what the whole-PR review and Copilot found

- A redirect followed for a custom base URL also drops the key headers of
  providers that do not use Authorization (x-api-key, x-goog-api-key,
  api-key) when it goes to another origin.
- A second Enter or click while a message is being prepared (attachments
  read, diagram exported) no longer sends it twice.
- The admin Test on the deployment's own endpoints (EdgeOne, the server's
  keyless Ollama, an address on the server's network) counts toward the
  quota like a chat; the chat and the Test share one rule for it. The
  Test of an Azure entry set up by AZURE_RESOURCE_NAME only goes where chat
  goes.
- EdgeOne's function is called at the site root again, as on main: EdgeOne
  serves edge functions there, outside Next's base path.
- MCP History: the state before a write is kept unless the browser saved
  no change of the user's since the last server write (draw.io's sync copy
  of it adds no entry), and the dedupe compares the exact text again, so a
  change of page size or other settings only is its own version.
- MCP: an edit keeps untouched labels as draw.io shows them (a literal line
  break in an attribute is a space); a new document of empty pages the
  user named is auto-saved; load_diagram reads only regular files, so a
  pipe cannot hold up the other write tools; the preview does not load
  back its own push still on its way (an undo made meanwhile is saved).
- Two overlapping saves of a new chat no longer reload the canvas from the
  older copy.
- At most three screenshot checks per user turn, passed or failed, as
  documented.
- Desktop: the main window navigates only within the app (draw.io stays in
  its frame); a presets file that is not JSON and cannot be moved aside is
  not overwritten.
- A last self-closing cell with a raw "<" in a value is not taken for cut
  off output.
- README: Material Design shapes load their icons from fonts.gstatic.com.
This commit is contained in:
dayuan.jiang
2026-10-05 21:19:32 +09:00
parent 69c7896002
commit b8b851fc5b
29 changed files with 476 additions and 90 deletions
+16 -1
View File
@@ -20,7 +20,13 @@ vi.mock("@/lib/admin/settings", () => ({
import { POST as testModel } from "@/app/api/admin/test-model/route"
const ENV = ["OPENAI_BASE_URL", "SGLANG_BASE_URL", "AI_GATEWAY_BASE_URL"]
const ENV = [
"OPENAI_BASE_URL",
"SGLANG_BASE_URL",
"AI_GATEWAY_BASE_URL",
"AZURE_BASE_URL",
"AZURE_RESOURCE_NAME",
]
const saved: Record<string, string | undefined> = {}
beforeEach(() => {
envFallback.values = {}
@@ -91,6 +97,15 @@ describe("admin Test of an entry without a URL", () => {
}
})
it("tests Azure set up by resource name where chat goes", async () => {
process.env.AZURE_RESOURCE_NAME = "team-openai"
await test({ provider: "azure", apiKey: "k" })
expect(sent.body.baseUrl).toBe(
"https://team-openai.openai.azure.com/openai",
)
expect(sent.body.serverBaseUrl).toBe(true)
})
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)
+2 -2
View File
@@ -119,7 +119,7 @@ describe("EdgeOne endpoints", () => {
)
})
it("keeps the deployment's base path", async () => {
it("calls the function at the site root, also with a base path", async () => {
const savedPath = process.env.NEXT_PUBLIC_BASE_PATH
process.env.NEXT_PUBLIC_BASE_PATH = "/draw"
try {
@@ -128,7 +128,7 @@ describe("EdgeOne endpoints", () => {
"x-ai-model": "@tx/deepseek-ai/deepseek-v3-0324",
})
expect(calls[0]?.url).toBe(
"http://localhost/draw/api/edgeai/chat/completions",
"http://localhost/api/edgeai/chat/completions",
)
} finally {
if (savedPath === undefined)
+26
View File
@@ -3,6 +3,7 @@ import {
existsSync,
mkdtempSync,
readdirSync,
readFileSync,
rmSync,
writeFileSync,
} from "node:fs"
@@ -19,6 +20,8 @@ vi.mock("electron", () => ({
// Make the next read of the presets file fail, like a file an antivirus
// scanner holds on Windows
const readFails = vi.hoisted(() => ({ next: false }))
// Make renaming the presets file fail, as when a sync tool holds it
const renameFails = vi.hoisted(() => ({ on: false }))
vi.mock("node:fs", async (importOriginal) => {
const fs = await importOriginal<typeof import("node:fs")>()
return {
@@ -32,6 +35,17 @@ vi.mock("node:fs", async (importOriginal) => {
}
return fs.readFileSync(...args)
}) as typeof fs.readFileSync,
renameSync: ((...args: Parameters<typeof fs.renameSync>) => {
if (renameFails.on && String(args[0]).endsWith(".json")) {
throw Object.assign(
new Error("EPERM: operation not permitted"),
{
code: "EPERM",
},
)
}
return fs.renameSync(...args)
}) as typeof fs.renameSync,
}
})
@@ -42,6 +56,7 @@ const presetsFile = () => join(userData.dir, "config-presets.json")
beforeEach(() => {
userData.dir = mkdtempSync(join(tmpdir(), "config-manager-"))
readFails.next = false
renameFails.on = false
})
describe("config presets file", () => {
@@ -70,6 +85,17 @@ describe("config presets file", () => {
expect(loadPresets().presets.map((p) => p.name)).toEqual(["New"])
})
it("keeps a file that is not JSON when it cannot be moved aside", () => {
writeFileSync(presetsFile(), "{not json")
renameFails.on = true
expect(loadPresets().presets).toEqual([])
// A save based on that empty read must not replace it
expect(() =>
createPreset({ name: "New", config: { AI_PROVIDER: "openai" } }),
).toThrow()
expect(readFileSync(presetsFile(), "utf-8")).toBe("{not json")
})
it("moves a file that is not JSON aside", () => {
writeFileSync(presetsFile(), "{not json")
expect(loadPresets().presets).toEqual([])
+23
View File
@@ -259,6 +259,29 @@ describe("MCP preview after the server recreated its session", () => {
expect(t.calls.filter((c) => c.method === "POST")).toHaveLength(0)
})
it("keeps an undo when a poll sees the tab's own push first", 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")
// Undo back to A while B is on its way (equal to the saved A: not sent)
t.fromDrawio({ event: "autosave", xml: "<mxfile>A</mxfile>" })
// The server already has B, and the poll's answer comes first
const loadsBefore = t.toDrawio.filter((m) => m.action === "load").length
const poll = t.page.poll()
t.next("GET").answer(state("S1", 3, "<mxfile>B</mxfile>"))
await poll
expect(t.toDrawio.filter((m) => m.action === "load")).toHaveLength(
loadsBefore,
)
pushB.answer({ status: 200, body: { success: true, version: 3 } })
await t.settle()
await t.settle()
// The undo is saved
expect(t.next("POST").body.xml).toBe("<mxfile>A</mxfile>")
})
it("sends nothing more after a sync reply", async () => {
const t = await inStep()
const poll = t.page.poll()
+6
View File
@@ -177,6 +177,9 @@ describe("redirectGuardedFetch with the quota on", () => {
headers: {
Authorization: "Bearer user-key",
Cookie: "eo_token=1",
"x-api-key": "anthropic-key",
"x-goog-api-key": "google-key",
"api-key": "azure-key",
"Content-Type": "application/json",
},
})
@@ -192,6 +195,9 @@ describe("redirectGuardedFetch with the quota on", () => {
expect(sent(0).get("authorization")).toBe("Bearer user-key")
expect(sent(1).get("authorization")).toBeNull()
expect(sent(1).get("cookie")).toBeNull()
for (const name of ["x-api-key", "x-goog-api-key", "api-key"]) {
expect(sent(1).get(name)).toBeNull()
}
expect(sent(1).get("content-type")).toBe("application/json")
})
+21 -1
View File
@@ -50,6 +50,8 @@ describe("the screenshot check and Stop", () => {
watchStop: () => () => boolean
validateDiagram: () => Promise<any>
captureValidationPng?: () => Promise<string>
// Checks already made in this user turn
retryCount?: { current: number }
}) => {
const onValidationStateChange = vi.fn()
const { result } = renderHook(() =>
@@ -57,7 +59,7 @@ describe("the screenshot check and Stop", () => {
partialXmlRef: { current: "" },
editDiagramOriginalXmlRef: { current: new Map() },
processedToolCallsRef: { current: new Set() },
validationRetryCountRef: { current: 0 },
validationRetryCountRef: opts.retryCount ?? { current: 0 },
chartXMLRef: { current: "" },
onDisplayChart: () => null,
onFetchChart: async () => "",
@@ -118,6 +120,24 @@ describe("the screenshot check and Stop", () => {
expect(addToolOutput.mock.lastCall?.[0].state).toBeUndefined()
})
it("checks at most three diagrams in one user turn", async () => {
const validateDiagram = vi.fn(async () => ({
valid: true,
issues: [],
suggestions: [],
}))
const retryCount = { current: 0 }
for (let i = 0; i < 4; i++) {
await draw({
watchStop: () => () => false,
validateDiagram,
retryCount,
})
}
// Passed checks count too
expect(validateDiagram).toHaveBeenCalledTimes(3)
})
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
+6
View File
@@ -44,6 +44,12 @@ describe("isMxCellXmlComplete", () => {
const xml =
'<mxCell id="2" value="Hello" style="rounded=1;" vertex="1" parent="1"/>'
expect(isMxCellXmlComplete(xml)).toBe(true)
// A raw "<" in a value (escaped later by the auto-fix)
expect(
isMxCellXmlComplete(
'<mxCell id="3" value="<b>Title</b>" style="text;html=1;" vertex="1" parent="1"/>',
),
).toBe(true)
})
it("returns true for mxCell with closing tag", () => {
+52
View File
@@ -18,8 +18,22 @@ vi.mock("@/lib/ssrf-protection", async (importOriginal) => ({
isPrivateUrl: async () => privateUrls.all,
}))
// The quota, off unless a test turns it on; every request is refused
const quota = vi.hoisted(() => ({ enabled: false, checks: 0 }))
vi.mock("@/lib/dynamo-quota-manager", () => ({
isQuotaEnabled: () => quota.enabled,
checkAndIncrementRequest: async () => {
quota.checks++
return { allowed: false, error: "Daily limit reached" }
},
}))
vi.mock("@/lib/user-id", () => ({ getUserIdFromRequest: () => "user-1" }))
afterEach(() => {
delete process.env.ALLOW_PRIVATE_URLS
quota.enabled = false
quota.checks = 0
privateUrls.all = false
vi.unstubAllGlobals()
})
@@ -314,3 +328,41 @@ describe("the admin panel's Test button", () => {
}
})
})
describe("the Test on the deployment's own endpoints", () => {
const test = (body: object) =>
validateModel(
new Request("http://localhost/api/validate-model", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ modelId: "m", ...body }),
}),
)
it("counts as a chat request with the quota on", async () => {
quota.enabled = true
// EdgeOne, and a model server on the server's network with a dummy key
const edgeone = await test({ provider: "edgeone" })
expect(edgeone.status).toBe(429)
privateUrls.all = true
const internal = await test({
provider: "openai",
apiKey: "x",
baseUrl: "http://10.0.0.5:8000/v1",
})
expect(internal.status).toBe(429)
expect(quota.checks).toBe(2)
})
it("does not count a user's own endpoint", async () => {
quota.enabled = true
streamReply({ role: "assistant", content: "OK" })
const res = await test({
provider: "openai",
apiKey: "user-key",
baseUrl: "https://api.example.com/v1",
})
expect(res.status).toBe(200)
expect(quota.checks).toBe(0)
})
})