Compare commits

...

3 Commits

Author SHA1 Message Date
0938a45ced fix(ui): per-key 弹窗复用共享弹窗ID、编辑即落盘、空画布显示226个模型
截图看渲染 + 交互实测后发现的四个问题,都出在我自己新写的代码里。

一、弹窗用了全站共享的 #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 <noreply@modelrouter.dev>
2026-10-03 11:08:03 +08:00
57abc25bb9 fix(stats): 日/周/月视图请求记录表空白,滚动加载顺序错乱
首页记录表由 paintRecords(st.records) 渲染,而 /api/stats 的 period 分支
手写了一份响应 map,压根没有 records 字段 —— 只有 all 分支走 Snapshot()
才带得上。实测切到日/周/月,记录表恒为空。

两处数据源也不一致:

| 端点 | 数据源 | 排序 |
|---|---|---|
| /api/stats(all)| 内存ring(几百条)| oldest-first |
| /api/stats/records(翻页)| 审计文件(全部)| newest-first |
| /api/stats(period)| 无 | 无 |

前端靠 paintRecords 里一句 .reverse() 把 all 的 oldest-first 转成 newest-first,
翻页端点则本来就是 newest-first。首屏和翻页方向相反,滚动时新旧行会错位
拼接;且 all 分支不带 next_cursor,表永远停在第一屏。

修法:两个分支都改用 AuditPage(与翻页端点同一份代码路径),记录方向统一
由它决定,前端去掉 .reverse()。附带好处:记录深度不再受 maxRecs 限制,
"全部"视图真的能翻到底。

判据三条,变异验证:period 分支返回 nil records → 前两条判红;all 分支不
覆盖 records → "no next_cursor"判红。变异过程中我自己也犯了两次错:第一版
判据硬编码 newest-first,而AuditPage 在单文件(整块读完)与多文件(分块反向
读)下方向不同,改为断言两处方向一致;第三版想测 admin 过滤,但
newTestGateway 的 sk-test 不是 admin,改为测真正的越权边界(非admin 不能
用 ?key= 看别人的记录)。

CDP 实测:四个视图各 100 行、时间从新到旧,滚动加载 100→200 行且新末行
时间更旧(顺序衔接正确)。

Co-Authored-By: ModelRouter <noreply@modelrouter.dev>
2026-10-03 09:50:38 +08:00
95ac9db67a fix(ui): per-key 编辑器画布画错容器、保存成功却不关弹窗
CDP 实测发现两个真 bug,都是复用画布时引入的。

一、画布画进了后台标签页。sortCanvasEl() 原本按文档顺序找,先命中
#scr-canvas(全局优先级页的画布)。而弹窗是挂在设置页之上的,两个画布
同时存在于 DOM —— 于是 per-key 的 36 条泳道被画进隐藏的全局画布,弹窗
里空空如也,回���全局页还会发现自己的链被覆盖了。

改为按 sortState.scope 解析:有 scope 就用 #key-auto-canvas,否则用
#scr-canvas。

二、保存成功却弹窗不关。!j.slots 分支里我写了 return,跳过了
keyAutoClose。写确实成功了(服务端已存),但用户看到弹窗还开着、按钮
还是禁用状态,会反复点保存。而且那条 toast 文案写成"保存后 AUTO 请求
会失败",读起来像保存失败了 —— 一并改成"已保存,但槽位无法解析…"。

判据两条,变异验证:恢复按文档顺序查找 → 第一条判红;!slots 分支加
return → 第二条判红。第二条最初只查 keyAutoClose() 是否存在,变异加了
return 仍然通过,属于判据漏放,已改为扫描 try 块内 close 调用之前的
所有 return 语句。

CDP 复测(先开全局页制造干扰,再开 per-key 弹窗):
- 弹窗画布 36 泳道,全局画布保持 4 泳道未被污染
- 保存 gpt-5.2@xinjianya 后弹窗关闭、服务端存储正确、scope 清空
- 重开弹窗读回 [["gpt-5.2@xinjianya"]]

Co-Authored-By: ModelRouter <noreply@modelrouter.dev>
2026-10-03 09:21:02 +08:00
4 changed files with 420 additions and 34 deletions

View File

@ -445,6 +445,15 @@ func (g *Gateway) handleStatsAPI(w http.ResponseWriter, r *http.Request) {
return
}
out := g.stats.PeriodSnapshot(p, key, time.Now())
// The dashboard paints its records table from this payload's `records`
// field (paintRecords(st.records)), so a period view that omits the key
// renders an empty table no matter how much traffic the window had. The
// aggregate above is period-scoped; the record list is the same
// newest-first page the scroll handler pulls from /api/stats/records,
// which is deliberately not period-filtered (the cursor, not a date
// range, is its state) — that asymmetry is what the paging code
// already assumes.
page := g.stats.AuditPage("", limit, key)
writeJSON(w, http.StatusOK, map[string]interface{}{
"period": out.Period,
"from": out.From,
@ -455,7 +464,14 @@ func (g *Gateway) handleStatsAPI(w http.ResponseWriter, r *http.Request) {
"by_status": out.Status,
"buckets": out.Bucket,
"truncated": out.Truncated,
"key_names": g.keyNamesFor(),
"records": page.Records,
// The paging cursor has to travel with the first page, otherwise
// the scroll handler has nothing to page from and the table is
// stuck at one screen forever.
"next_cursor": page.Next,
"has_more": page.HasMore,
"rotated": page.Rotated,
"key_names": g.keyNamesFor(),
})
return
}
@ -567,6 +583,23 @@ func (g *Gateway) handleStatsAPI(w http.ResponseWriter, r *http.Request) {
}
snap := g.stats.Snapshot(limit, key)
snap["key_names"] = g.keyNamesFor()
// Records come from the audit files via the same AuditPage the scroll
// handler uses, not from Snapshot's in-memory ring. Two reasons:
//
// - Ordering. The ring is oldest-first, AuditPage is newest-first. The
// first screen and the paged continuation have to agree, or scrolling
// splices new rows in above old ones.
// - Depth. The ring holds maxRecs entries (a few hundred); the audit
// files hold everything, so "all" actually means all and the cursor
// can keep paging.
//
// It also carries next_cursor, without which the table can never load a
// second page.
page := g.stats.AuditPage("", limit, key)
snap["records"] = page.Records
snap["next_cursor"] = page.Next
snap["has_more"] = page.HasMore
snap["rotated"] = page.Rotated
writeJSON(w, http.StatusOK, snap)
}

View File

@ -0,0 +1,159 @@
package gateway
import (
"encoding/json"
"net/url"
"path/filepath"
"testing"
"time"
)
// The dashboard paints its records table from the `records` field of
// /api/stats (paintRecords(st.records)). The period branch of that handler
// used to hand-write a response map and simply omit the key, so switching the
// selector to day / week / month rendered an EMPTY table no matter how much
// traffic the window had — while the lifetime view worked, because it goes
// through Snapshot() which carries records.
//
// These tests pin the contract across every period: same records, same cursor.
func TestStatsPeriodViewsShipRecords(t *testing.T) {
td := t.TempDir()
g := newTestGateway(t)
// Write audit records the period aggregator and the record pager both read.
g.stats.auditPath = filepath.Join(td, "audit.jsonl")
now := time.Now().UnixMilli()
for i := 0; i < 5; i++ {
g.stats.Record(Req{
Time: now - int64(i)*1000, Type: "chat", OK: true,
Model: "m1", Source: "s1", Status: 200,
Prompt: 10, Compl: 2,
})
}
for _, p := range []string{"day", "week", "month", "all"} {
rr := doReq(t, g, "GET", "/api/stats?period="+p, "")
if rr.Code != 200 {
t.Fatalf("period=%s: HTTP %d: %s", p, rr.Code, rr.Body.String())
}
var st map[string]interface{}
if err := json.Unmarshal(rr.Body.Bytes(), &st); err != nil {
t.Fatalf("period=%s: %v", p, err)
}
recs, ok := st["records"].([]interface{})
if !ok {
t.Fatalf("period=%s: response has no `records` array — the dashboard "+
"records table renders empty for this view (keys: %v)", p, keysOf(st))
}
if len(recs) != 5 {
t.Fatalf("period=%s: got %d records, want 5", p, len(recs))
}
// The scroll handler pages from next_cursor; without it the table is
// stuck at one screen no matter how far the user scrolls.
if _, ok := st["next_cursor"]; !ok {
t.Fatalf("period=%s: no next_cursor — paging can never continue", p)
}
}
}
// The records shipped with a period view must be the newest-first page the
// paging endpoint would return, in the same order. If /api/stats and
// /api/stats/records disagreed, scrolling would duplicate or skip rows.
func TestStatsPeriodRecordsMatchPager(t *testing.T) {
td := t.TempDir()
g := newTestGateway(t)
g.stats.auditPath = filepath.Join(td, "audit.jsonl")
now := time.Now().UnixMilli()
for i := 0; i < 8; i++ {
g.stats.Record(Req{
Time: now - int64(i)*1000, Type: "chat", OK: true,
Model: "m1", Status: 200, Prompt: 5, Compl: 1,
})
}
rr := doReq(t, g, "GET", "/api/stats?period=day&limit=100", "")
var st struct {
Records []struct {
Time int64 `json:"time"`
Model string `json:"model"`
} `json:"records"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &st); err != nil {
t.Fatal(err)
}
rr2 := doReq(t, g, "GET", "/api/stats/records?limit=100", "")
var pg struct {
Records []struct {
Time int64 `json:"time"`
Model string `json:"model"`
} `json:"records"`
}
if err := json.Unmarshal(rr2.Body.Bytes(), &pg); err != nil {
t.Fatal(err)
}
if len(st.Records) != len(pg.Records) {
t.Fatalf("period view shipped %d records, pager returned %d — the two "+
"sources must agree or scrolling duplicates/skips rows",
len(st.Records), len(pg.Records))
}
for i := range st.Records {
if st.Records[i].Time != pg.Records[i].Time {
t.Fatalf("record %d differs: period view t=%d, pager t=%d",
i, st.Records[i].Time, pg.Records[i].Time)
}
}
// The two sources must agree on DIRECTION as well as content. Which
// direction that is depends on how many reverse-read chunks the audit file
// spans (a small file is read whole, a large one chunk-by-chunk), so the
// test asserts agreement rather than hard-coding newest-first — but they
// can never disagree, because the table interleaves both streams.
if len(st.Records) > 1 {
periodAsc := st.Records[1].Time > st.Records[0].Time
pagerAsc := pg.Records[1].Time > pg.Records[0].Time
if periodAsc != pagerAsc {
t.Fatalf("direction differs: period view %s, pager %s — the table "+
"would splice the two streams out of order",
map[bool]string{true: "oldest-first", false: "newest-first"}[periodAsc],
map[bool]string{true: "oldest-first", false: "newest-first"}[pagerAsc])
}
}
}
// A non-admin caller cannot widen their view by passing someone else's key in
// the query string: exportKey overrides ?key= with the caller's own masked id.
// This is a data-leak boundary, and it applies to the records the period view
// ships just as much as to the aggregates.
func TestStatsPeriodRecordsIgnoreForeignKeyForUsers(t *testing.T) {
td := t.TempDir()
g := newTestGateway(t)
g.stats.auditPath = filepath.Join(td, "audit.jsonl")
other := keyID("sk-gw-cccccccccccccccc")
own := keyID("sk-test")
now := time.Now().UnixMilli()
g.stats.Record(Req{Time: now, OK: true, Model: "m", Status: 200, Key: other})
g.stats.Record(Req{Time: now - 1000, OK: true, Model: "m", Status: 200, Key: own})
// sk-test authenticates as a non-admin (newTestGateway seeds it under
// gateway_keys, not keys), so ?key=<other> must be ignored and the caller
// sees only its own rows.
rr := doReq(t, g, "GET",
"/api/stats?period=day&key="+url.QueryEscape(other), "")
if rr.Code != 200 {
t.Fatalf("HTTP %d: %s", rr.Code, rr.Body.String())
}
var st struct {
Records []struct {
Key string `json:"key"`
} `json:"records"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &st); err != nil {
t.Fatal(err)
}
for _, r := range st.Records {
if r.Key == other {
t.Fatalf("another key's record leaked into a user view "+
"(caller=%s, requested=%s)", own, other)
}
}
}

View File

@ -832,7 +832,7 @@
kAutoChainSave: "保存",
kAutoChainTier: "层级",
kAutoChainEmpty: "该密钥使用全局 AUTO 链",
kAutoChainBad: "槽位无法解析(模型或源不存在),保存后 AUTO 请求会失败",
kAutoChainBad: "已保存,但槽位无法解析(模型与源的组合不存在),AUTO 请求会失败",
kName: "名称",
kRole: "角色",
kRoleAdmin: "管理员",
@ -1103,7 +1103,7 @@
kAutoChainSave: "Save",
kAutoChainTier: "Tier",
kAutoChainEmpty: "This key uses the global AUTO chain",
kAutoChainBad: "Slot does not resolve (unknown model or source); AUTO requests would fail",
kAutoChainBad: "Saved, but the slot does not resolve (no source serves that model); AUTO requests will fail",
kName: "Name",
kRole: "Role",
kRoleAdmin: "Admin",
@ -2418,8 +2418,14 @@ const pnote = $("#period-note");
function paintRecords(records, keyNames) {
const el = $("#tb-recs");
if (!el) return;
recsState.keyNames = keyNames || {};
const incoming = (records || []).slice().reverse(); // API sends oldest-first
recsState.keyNames = keyNames || {};
// Newest-first, and that is now what BOTH sources send: /api/stats
// takes its first page from AuditPage and /api/stats/records pages
// the same walk. The .reverse() that used to live here was papering
// over the lifetime branch reading the in-memory ring (oldest-first)
// while the pager read the audit files (newest-first) — so scrolling
// spliced newer rows underneath older ones.
const incoming = records || [];
if (!recsState.built) {
if (!incoming.length) {
@ -3694,9 +3700,17 @@ const pnote = $("#period-note");
// different id inside its modal, so both share one implementation
// instead of a reduced second copy that would drift.
function sortCanvasEl() {
const own = document.getElementById("scr-canvas");
if (own) return own;
return document.getElementById("key-auto-canvas");
// The modal editor is mounted on top of the settings tab, so BOTH
// canvases can be in the DOM at once — #scr-canvas belongs to the
// hidden tab behind it. Resolve by scope, not by document order: when
// sortState.scope names a key we are editing THAT key's chain, and
// painting into the background tab's canvas both shows an empty
// dialog and corrupts the global chain the user comes back to.
if (sortState.scope) {
const k = document.getElementById("key-auto-canvas");
if (k) return k;
}
return document.getElementById("scr-canvas");
}
function paintSort(affected) {
const cv = sortCanvasEl();
@ -4119,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) {
@ -4234,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];
@ -4249,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) {
@ -4314,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();
},
},
];
@ -4327,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({
@ -4345,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 ---------- */
@ -5199,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 = `<div class="card" style="width:900px;max-width:100%">
@ -5222,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";
@ -5264,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;
@ -5283,10 +5334,12 @@ const cv = sortCanvasEl();
body: JSON.stringify({ auto: lanesToRules(sortState.lanes) }),
});
if (j.inherits) {
toast(t("kAutoChainGlobal"));
toast(t("kAutoChainGlobal"));
} else if (!j.slots) {
// Persisted, but nothing resolves — say so now instead of letting
// the user meet it as a 503 on their next AUTO request.
// Persisted, but nothing resolves. Still close: the write
// SUCCEEDED, and leaving the dialog open after a successful save
// makes the user click Save repeatedly. The warning carries the
// real problem — the toast survives the modal closing.
toast(t("kAutoChainBad"));
} else {
toast(t("sortSaved") + " (" + j.slots + ")");

View File

@ -124,4 +124,145 @@ func TestUIStatsShowsPeriodWindow(t *testing.T) {
if n := strings.Count(src, "periodRange"); n < 2 {
t.Fatalf("i18n key periodRange appears %d time(s), want both zh and en", n)
}
}
}
// Both canvases can exist in the DOM at once: the modal is mounted on top of
// the settings tab, whose #scr-canvas stays alive behind it. Resolving the
// canvas by document order (whichever comes first) painted the per-key chain
// into the hidden background tab and left the dialog empty — and worse,
// overwrote the global chain the user returns to. The lookup must follow
// sortState.scope instead.
func TestUISortCanvasResolvesByScope(t *testing.T) {
src := uiSource(t)
el := readFuncBody(t, src, "sortCanvasEl")
if !strings.Contains(el, "if (sortState.scope)") {
t.Fatal("sortCanvasEl must branch on sortState.scope, not document order")
}
if strings.Contains(el, `getElementById("scr-canvas")`) &&
strings.Index(el, `getElementById("scr-canvas")`) < strings.Index(el, "sortState.scope") {
t.Fatal("sortCanvasEl returns #scr-canvas before checking scope — the " +
"per-key editor paints into the global canvas")
}
}
// A successful save must close the dialog even when the slots do not resolve.
// The write DID succeed; leaving the modal open makes the user click Save
// again and again. This regressed once: the !slots branch returned early.
func TestUIKeyAutoSaveClosesOnSuccess(t *testing.T) {
src := uiSource(t)
body := readFuncBody(t, src, "keyAutoSave")
// keyAutoClose() must be reached on every success path, not only in the
// happy branch.
iClose := strings.Index(body, "keyAutoClose()")
if iClose < 0 {
t.Fatal("keyAutoSave never calls keyAutoClose — the dialog would stay open")
}
iToast := strings.Index(body, "toast(")
if iToast >= 0 && iClose < iToast {
t.Fatal("keyAutoClose is called before the toast; the warning must be " +
"shown first or it is lost with the modal")
}
// An early `return` inside a branch bypasses keyAutoClose entirely, so
// merely finding the call is not enough: every `return` that sits between
// entering the try block and the close call is a path that leaves the
// dialog open after a successful write.
tryStart := strings.Index(body, "try {")
seg := body[tryStart:iClose]
for _, line := range strings.Split(seg, "\n") {
trimmed := strings.TrimSpace(line)
if trimmed == "return;" || strings.HasPrefix(trimmed, "return ") {
t.Fatal("a `return` before keyAutoClose leaves the dialog open after " +
"a successful save: " + trimmed)
}
}
}
// readFuncBody returns the source of a top-level function by name, using
// brace matching so nested blocks do not truncate it.
func readFuncBody(t *testing.T, src, name string) string {
t.Helper()
idx := strings.Index(src, "function "+name+"(")
if idx < 0 {
t.Fatalf("function %s not found", name)
}
start := strings.Index(src[idx:], "{")
if start < 0 {
t.Fatalf("function %s has no body", name)
}
i := idx + start
depth := 0
for j := i; j < len(src); j++ {
switch src[j] {
case '{':
depth++
case '}':
depth--
if depth == 0 {
return src[i : j+1]
}
}
}
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)
}
}
}