From f9cd0770a92c069b4ec1a8cb343edf9d23ff962b Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Sat, 3 Oct 2026 11:08:03 +0800 Subject: [PATCH] =?UTF-8?q?fix(ui):=20per-key=20=E5=BC=B9=E7=AA=97?= =?UTF-8?q?=E5=A4=8D=E7=94=A8=E5=85=B1=E4=BA=AB=E5=BC=B9=E7=AA=97ID?= =?UTF-8?q?=E3=80=81=E7=BC=96=E8=BE=91=E5=8D=B3=E8=90=BD=E7=9B=98=E3=80=81?= =?UTF-8?q?=E7=A9=BA=E7=94=BB=E5=B8=83=E6=98=BE=E7=A4=BA226=E4=B8=AA?= =?UTF-8?q?=E6=A8=A1=E5=9E=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 截图看渲染 + 交互实测后发现的四个问题，都出在我自己新写的代码里。 一、弹窗用了全站共享的 #modal-wrap。 这个 id 是 App 内所有弹窗共用的，且允许多个同时存在（"添加档位"选择器就 开在本弹窗之上）。既有代码靠 .closest() 或 closeTopModal() 从按钮往上找， 是有效的——但本弹窗还要从画布侧被查（sortCanvasEl / keyAutoClose / keyAutoInherit），那里没有按钮可走。实测点"添加档位"后 DOM 里同时有 modal-wrap=1 和 key-auto-modal=1，getElementById 会命中文档里的第一个， 也就是用户没在看的那一个：取消关掉错误的框，画布也可能解析到选择器上。 改为独立 id #key-auto-modal。 二、编辑即落盘，保存/取消形同虚设。 scrAddFromForm / sortScopeSave / scrDelSlot 等 6 处编辑后立刻调 persistAuto()。全局页这样是有意的（改动立即生效），但它和自己页面上那句 "点击保存排序后生效"的提示自相矛盾；在 per-key 弹窗里更严重——加一个槽位 就已经写进那个用户的链了，取消也无法撤销。收敛到 afterChainEdit()： 有 scope 时只提示、不落盘，保存是唯一写入口。 三、无独立链时画布显示全部 226 个模型。 注释写的是"显示空画布"，实现却是 buildLanes(idx, [], 'chat')——空 rules 会走 discovery 兜底，把每个源的模型全填进去。用户打开弹窗看到 226 个色块， 会以为这些就是这个密钥的链。改为空 rules 直接给 []。 四、"使用全局链"提示不随画布变化。 加完槽位后仍显示"该密钥使用全局 AUTO 链"，与旁边的色块直接矛盾。改为由 afterChainEdit 触发、按画布是否有内容决定去留。 另：批量替换时正则的缩进假设错了一次，把 afterChainEdit 自己改成了自调用 （无限递归），node --check 抓到后修掉。这类批量替换必须先过语法检查。 判据两条 + 变异：弹窗回到共享 id → 判红；去掉 scope 分支 → 判红。 CDP 实测：弹窗 id 唯一、添加后画布 0→1 块、添加后服务端 inherits=true （未落盘）、取消后仍 inherits=true、选择器与弹窗并存互不干扰。 Co-Authored-By: ModelRouter --- internal/gateway/ui/index.html | 83 ++++++++++++++++++++-------- internal/gateway/ui_key_auto_test.go | 62 +++++++++++++++++++++ 2 files changed, 122 insertions(+), 23 deletions(-) diff --git a/internal/gateway/ui/index.html b/internal/gateway/ui/index.html index bff34e1..9bd187d 100644 --- a/internal/gateway/ui/index.html +++ b/internal/gateway/ui/index.html @@ -3710,7 +3710,7 @@ recsState.keyNames = keyNames || {}; const k = document.getElementById("key-auto-canvas"); if (k) return k; } - return document.getElementById("scr-canvas"); +return document.getElementById("scr-canvas"); } function paintSort(affected) { const cv = sortCanvasEl(); @@ -4133,6 +4133,25 @@ const cv = sortCanvasEl(); // editor means a per-key chain gets exactly the features the global one // has (drag ordering, per-slot quota/period/hours) instead of a reduced // second implementation that would drift. + // afterChainEdit is the single place an in-canvas edit decides whether to + // write through. On the GLOBAL page edits take effect immediately (that + // page has always behaved that way). In the per-key dialog they must NOT: + // that dialog has Save and Cancel, and auto-persisting meant a slot added + // (or dragged, or deleted) was already committed to the user's chain + // before Save — Cancel could not undo it, and the "Click Save for it to + // take effect" hint was a lie. +function afterChainEdit() { + // Every in-canvas edit lands here, so this is the one place that keeps + // the "uses the global chain" notice honest while the user works. + if (typeof keyAutoInheritNotice === "function") keyAutoInheritNotice(); + if (sortState.scope) { + toast(t("sortHintSave")); + return; + } + persistAuto() + .then(() => toast(t("sortHintSave"))) + .catch((e) => toast(e.message)); + } async function persistAuto() { const rules = lanesToRules(sortState.lanes); if (sortState.scope) { @@ -4248,9 +4267,7 @@ const cv = sortCanvasEl(); if (btn) btn.closest("#modal-wrap")?.remove(); else closeTopModal(); paintSort([li]); - persistAuto() - .then(() => toast(t("sortHintSave"))) - .catch((e) => toast(e.message)); + afterChainEdit(); } function sortScopeSave(li, ji, btn) { const it = sortState.lanes[li].models[ji]; @@ -4263,9 +4280,7 @@ const cv = sortCanvasEl(); it.meta = { quota: q, period: p, hours: p || q ? h : 0 }; (btn ? btn.closest("#modal-wrap") : null) || closeTopModal(); paintSort([li]); - persistAuto() - .then(() => toast(t("sortHintSave"))) - .catch((e) => toast(e.message)); + afterChainEdit(); } function scrCtx(e, li, ji) { if (e.button === 2) { @@ -4328,9 +4343,7 @@ const cv = sortCanvasEl(); meta: it.meta ? { ...it.meta } : null, }); paintSort([li]); - persistAuto() - .then(() => toast(t("sortHintSave"))) - .catch((e) => toast(e.message)); + afterChainEdit(); }, }, ]; @@ -4341,9 +4354,7 @@ const cv = sortCanvasEl(); fn: () => { it.meta = null; paintSort([li]); - persistAuto() - .then(() => toast(t("sortHintSave"))) - .catch((e) => toast(e.message)); + afterChainEdit(); }, }); items.push({ @@ -4359,9 +4370,7 @@ const cv = sortCanvasEl(); if (!sortState.lanes[li].models.length && sortState.lanes.length > 1) sortState.lanes.splice(li, 1); paintSort([li]); - persistAuto() - .then(() => toast(t("sortHintSave"))) - .catch((e) => toast(e.message)); + afterChainEdit(); } /* ---------- adapters tab ---------- */ @@ -5213,8 +5222,16 @@ const cv = sortCanvasEl(); // /api/keys/{key}/auto instead of /api/auto. A separate reduced editor // would drift from the global one and lose features. async function openKeyAutoModal(key, name) { + // This dialog deliberately does NOT reuse #modal-wrap. That id is + // shared by every dialog in the app and more than one can be open at + // once, so a lookup by id resolves to whichever comes first in the + // document — not the one the user is looking at. That is fine for + // handlers that can use `.closest()` from their own button, but this + // dialog also has to be found from the CANVAS side (sortCanvasEl, + // keyAutoClose, keyAutoInherit), so it needs an identity of its own. + document.getElementById("key-auto-modal")?.remove(); const wrap = document.createElement("div"); - wrap.id = "modal-wrap"; + wrap.id = "key-auto-modal"; wrap.style.cssText = "position:fixed;inset:0;background:rgba(15,22,44,.45);display:flex;align-items:flex-start;justify-content:center;overflow:auto;padding:48px 20px;z-index:50"; wrap.innerHTML = `
@@ -5236,25 +5253,45 @@ const cv = sortCanvasEl(); const cur = await api("/api/keys/" + encodeURIComponent(key) + "/auto"); keyAutoState.key = key; keyAutoState.inherits = !!cur.inherits; - // A key with no own chain shows an empty canvas rather than the - // discovery fallback: filling it with every available slot would - // suggest those slots are already this user's chain. + // A key with no own chain shows an EMPTY canvas, never the discovery + // fallback buildLanes uses for an unconfigured global chain: that + // fallback fills every source's models (226 blocks on a real + // deployment), and showing them inside "this key's chain" dialog + // says they already belong to this user. They do not. sortState.scope = key; sortState.kind = "chat"; - sortState.lanes = buildLanes(idx, cur.auto || [], "chat"); + sortState.lanes = + cur.auto && cur.auto.length ? buildLanes(idx, cur.auto, "chat") : []; sortState.origin = JSON.stringify(sortState.lanes); await loadModelPairs(); renderSortEditor($("#ka-canvas-host"), { canvasId: "key-auto-canvas" }); + // The notice is the only thing telling the user what an empty + // canvas means here. Without it the dialog is just a blank box and + // "Use global chain" looks like a button that already did + // something. It disappears as soon as the chain has slots of its + // own, which keyAutoSave's reload then confirms. if (keyAutoState.inherits) keyAutoInheritNotice(); } catch (e) { toast(String(e)); } } const keyAutoState = { key: null, inherits: true }; - function keyAutoInheritNotice() { + // The notice describes what an EMPTY canvas means, so it must track the + // canvas rather than the state the dialog opened with: adding the first + // slot makes "this key uses the global chain" false, and leaving it up + // reads as a contradiction right next to the slot the user just added. + function keyAutoInheritNotice(showing) { const host = $("#ka-canvas-host"); if (!host) return; let n = host.querySelector(".ka-inherit-note"); + const hasSlots = (sortState.lanes || []).some( + (l) => (l.models || []).length, + ); + const want = showing === undefined ? !hasSlots : showing && !hasSlots; + if (!want) { + if (n) n.remove(); + return; + } if (!n) { n = document.createElement("div"); n.className = "muted ka-inherit-note"; @@ -5278,7 +5315,7 @@ const cv = sortCanvasEl(); keyAutoInheritNotice(); } function keyAutoClose() { - const m = document.getElementById("modal-wrap"); + const m = document.getElementById("key-auto-modal"); if (m) m.remove(); // Leaving scope set would make the global editor save to this key. sortState.scope = null; diff --git a/internal/gateway/ui_key_auto_test.go b/internal/gateway/ui_key_auto_test.go index a9a4206..ba72a15 100644 --- a/internal/gateway/ui_key_auto_test.go +++ b/internal/gateway/ui_key_auto_test.go @@ -204,3 +204,65 @@ func readFuncBody(t *testing.T, src, name string) string { t.Fatalf("function %s body is unterminated", name) return "" } + +// The per-key dialog must not share the app-wide #modal-wrap id. +// +// Every dialog in this file uses that id, and more than one can be open at +// once (the "add slot" picker opens ON TOP of this one). Handlers resolve +// their own dialog via `.closest()` from their button, which works — but this +// dialog is also looked up from the CANVAS side (sortCanvasEl, keyAutoClose, +// keyAutoInherit), where there is no button to walk up from. +// getElementById("#modal-wrap") then returns whichever dialog comes first in +// the document, i.e. not the one being edited: Cancel closed the wrong box and +// the canvas could resolve to the picker's. +func TestUIKeyAutoModalHasOwnID(t *testing.T) { + src := uiSource(t) + body := readFuncBody(t, src, "openKeyAutoModal") + if !strings.Contains(body, `wrap.id = "key-auto-modal"`) { + t.Fatal("openKeyAutoModal must give its dialog its own id, not the shared " + + "#modal-wrap (another dialog may be open on top of it)") + } + close := readFuncBody(t, src, "keyAutoClose") + if strings.Contains(close, `"modal-wrap"`) { + t.Fatal("keyAutoClose resolves the shared #modal-wrap; it would close " + + "whichever dialog comes first in the document") + } + if !strings.Contains(close, `"key-auto-modal"`) { + t.Fatal("keyAutoClose must resolve the per-key dialog by its own id") + } +} + +// Editing a chain inside the per-key dialog must NOT persist it. That dialog +// has Save and Cancel, so auto-writing on add/drag/delete meant the change was +// already committed before Save and Cancel could not undo it. +// +// The global page is the opposite: it has always applied edits immediately. +func TestUIKeyAutoEditsDoNotAutoPersist(t *testing.T) { + src := uiSource(t) + body := readFuncBody(t, src, "afterChainEdit") + if !strings.Contains(body, "if (sortState.scope)") { + t.Fatal("afterChainEdit must branch on sortState.scope") + } + // The scoped branch must return before reaching persistAuto. + iScope := strings.Index(body, "if (sortState.scope)") + iReturn := strings.Index(body[iScope:], "return") + iPersist := strings.Index(body, "persistAuto()") + if iScope < 0 || iReturn < 0 || iPersist < 0 { + t.Fatal("afterChainEdit no longer has the expected shape") + } + if iScope+iReturn > iPersist { + t.Fatal("the scoped branch reaches persistAuto() — a per-key edit would " + + "be written before Save") + } + // And it must not call itself (a regex-driven rewrite did exactly that). + if strings.Contains(body, "afterChainEdit()") { + t.Fatal("afterChainEdit calls itself: infinite recursion") + } + // Every in-canvas edit goes through it, so none of them bypass the check. + for _, fn := range []string{"scrAddFromForm", "sortScopeSave", "scrDelSlot"} { + b := readFuncBody(t, src, fn) + if strings.Contains(b, "persistAuto()") { + t.Fatalf("%s calls persistAuto() directly, bypassing afterChainEdit", fn) + } + } +}