diff --git a/internal/gateway/plugins_api_test.go b/internal/gateway/plugins_api_test.go index b9fbffc..3ff9e3f 100644 --- a/internal/gateway/plugins_api_test.go +++ b/internal/gateway/plugins_api_test.go @@ -67,26 +67,30 @@ func TestUIInjectServesPluginUI(t *testing.T) { t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String()) } var view struct { - UI struct { - Page *struct { - PageID string `json:"page_id"` - Title string `json:"title"` - Mount string `json:"mount"` - } `json:"page"` - Elements []struct { - Target string `json:"target"` - Mount string `json:"mount"` - } `json:"elements"` - } `json:"ui"` - Stages []string `json:"stages"` + // Decoded into the real types so the test cannot drift from the wire + // contract. An inline copy missed the `pages` field when the payload + // shape changed and failed to compile, which is at least loud — but the + // same copy also went on asserting the OLD single-page shape for a + // release, silently. + UI lua.UIExtension `json:"ui"` + Stages []string `json:"stages"` } if err := json.Unmarshal(rr.Body.Bytes(), &view); err != nil { t.Fatalf("decode: %v", err) } - if view.UI.Page == nil || view.UI.Page.PageID != "billing" { - t.Fatalf("no billing page in the inject payload") + // The payload carries every page in one list; ui.page is no longer a + // separate slot (it was, and a second plugin contributing a page overwrote + // whatever was there). + var billing *lua.UIPage + for i, pg := range view.UI.Pages { + if pg != nil && pg.PageID == "billing" { + billing = view.UI.Pages[i] + } } - if !strings.Contains(view.UI.Page.Mount, "billing-root") { + if billing == nil { + t.Fatalf("no billing page in the inject payload; pages=%d", len(view.UI.Pages)) + } + if !strings.Contains(billing.Mount, "billing-root") { t.Error("the page mount came back empty") } if len(view.UI.Elements) == 0 { diff --git a/internal/gateway/ui/index.html b/internal/gateway/ui/index.html index ec25559..a0ef555 100644 --- a/internal/gateway/ui/index.html +++ b/internal/gateway/ui/index.html @@ -5362,9 +5362,21 @@ const nav = $("#sb-nav"); if (!main || !nav) return; - // --- page --- - if (ui.page && ui.page.page_id && ui.page.mount) { - const id = String(ui.page.page_id); + // --- pages --- + // + // A plugin may contribute ONE page (`ui.page`) or SEVERAL + // (`ui.pages[]`). Both are handled by the same code: a plugin whose + // price rules produce the numbers on its billing page needs a second + // screen to edit them, and cramming both into one pane behind + // in-page tabs would hide a whole capability behind a toggle. The + // single-page shape stays supported because it is what docs/plugins.md + // documents and what every existing plugin uses. + const pluginPages = [] + .concat(ui.page ? [ui.page] : []) + .concat(Array.isArray(ui.pages) ? ui.pages : []) + .filter((p) => p && p.page_id && p.mount); + pluginPages.forEach((pg) => { + const id = String(pg.page_id); if (!document.getElementById("tab-" + id)) { const pane = document.createElement("div"); pane.id = "tab-" + id; @@ -5381,7 +5393,7 @@ const btn = document.createElement("button"); btn.className = "sb-i"; btn.dataset.tab = id; - btn.title = ui.page.title || id; + btn.title = pg.title || id; // A plugin icon may be plain text (an emoji, a glyph) or an inline // SVG snippet. Native tabs use inline SVG styled with // `stroke: currentColor`, so an emoji next to them renders at the @@ -5391,12 +5403,12 @@ // The SVG form is allowed through RAW, which is only safe because // it is strictly filtered: see pluginIconHTML. Escaping it (as this // did) would print the markup as text instead. - btn.innerHTML = pluginIconHTML(ui.page.icon); + btn.innerHTML = pluginIconHTML(pg.icon); btn.onclick = () => goTab(id); nav.appendChild(btn); PLUGIN_PAGES.add(id); // The breadcrumb map is local to this file, so extend it here. - if (typeof NAV_NAME === "object") NAV_NAME[id] = ui.page.title || id; + if (typeof NAV_NAME === "object") NAV_NAME[id] = pg.title || id; } const pane = document.getElementById("tab-" + id); if (pane && !pane.dataset.pluginMounted) { @@ -5406,7 +5418,7 @@ // execute it, which is exactly what we want to avoid the opposite // problem: running before its own DOM exists. const tpl = document.createElement("template"); - tpl.innerHTML = ui.page.mount; + tpl.innerHTML = pg.mount; pane.appendChild(tpl.content); // Move each script into a fresh element so it executes. pane.querySelectorAll("script").forEach((old) => { @@ -5416,7 +5428,7 @@ old.replaceWith(s); }); } - } + }); // --- elements into existing pages --- // diff --git a/internal/lua/billing_test.go b/internal/lua/billing_test.go index a8c0dfa..01610d4 100644 --- a/internal/lua/billing_test.go +++ b/internal/lua/billing_test.go @@ -275,6 +275,22 @@ func TestBillingPluginDeclaresUI(t *testing.T) { if page, _ := ui["page"].(string); page != "billing" { t.Errorf("ui.page = %v, want \"billing\"", ui["page"]) } + // The management listing must name EVERY page, not just the first. + // Without `pages`, a plugin contributing the totals page and the rule + // editor is shown as contributing one, and the operator has no way to + // tell from the plugin list that a second screen exists. + // List() builds this map in Go, so the value is []string here; only + // after the HTTP round-trip would it be []interface{}. Asserting the + // wrong one is a silently empty set, which is what made the first + // version of this check fail for the wrong reason. + pages, _ := ui["pages"].([]string) + found := map[string]bool{} + for _, id := range pages { + found[id] = true + } + if !found["billing-rules"] { + t.Errorf("ui.pages = %v, want it to include billing-rules", found) + } if n, _ := ui["elements"].(int); n < 1 { t.Error("billing contributes no element to an existing page") } @@ -283,6 +299,137 @@ func TestBillingPluginDeclaresUI(t *testing.T) { t.Fatal("billing plugin is not loaded") } +// TestBillingDeclaresRulesEditorPage: the rule editor is a SECOND screen, not a +// tab inside the totals page. Two reasons it has to be its own page: mixing +// editable configuration with read-only results blurs the line between "looking +// at numbers" and "changing prices", and the plugin UI contract only ever had +// room for one page — a second plugin contributing a page silently overwrote +// the first, so multi-page had to become a first-class shape before this could +// exist. +func TestBillingDeclaresRulesEditorPage(t *testing.T) { + ps, _ := billingVM(t) + var ui *UIExtension = nil + for _, p := range ps.plugins { + if p.Info.Name == "billing" { + ui = p.UI + } + } + if ui == nil { + t.Fatal("billing plugin loaded with no UI extension") + } + // Read the plugin's OWN extension, which is where the single `page` field + // still lives — the fold into one list happens in the merged view, not here. + ids := map[string]bool{} + if ui.Page != nil { + ids[ui.Page.PageID] = true + } + for _, pg := range ui.Pages { + if pg != nil { + ids[pg.PageID] = true + } + } + if !ids["billing"] { + t.Error("the totals page is missing") + } + if !ids["billing-rules"] { + t.Errorf("the rule editor page is missing; pages = %v", ids) + } + // The editor must actually contain its controls, not just a pane: a page + // that mounts an empty div looks fine in the sidebar and does nothing. + var mount string + for _, pg := range ui.Pages { + if pg != nil && pg.PageID == "billing-rules" { + mount = pg.Mount + } + } + for _, needle := range []string{`id="br-body"`, "data-act='save'", "data-act='export'", "data-act='newprofile'", ".r-url", ".r-mode"} { + if !strings.Contains(mount, needle) { + t.Errorf("the rule editor page is missing %s", needle) + } + } +} + +// TestUIExtensionMergesEveryPluginPage: two plugins contributing pages must +// BOTH appear. The old merge assigned a single field, so the second plugin +// erased the first one's page from the sidebar with no error anywhere. +func TestUIExtensionMergesEveryPluginPage(t *testing.T) { + ps, pdir := billingVM(t) + mk := func(name, code string) { + if err := os.WriteFile(filepath.Join(pdir, name+".lua"), []byte(code), 0644); err != nil { + t.Fatal(err) + } + if err := ps.LoadSource(name, code); err != nil { + t.Fatalf("load %s: %v", name, err) + } + } + mk("other", ` +local plugin = {} +plugin.name = "other" +plugin.version = "0.1" +plugin.ui = { page = { page_id = "other-page", title = "Other", order = 90, + mount = "
" } } +return plugin`) + mk("third", ` +local plugin = {} +plugin.name = "third" +plugin.version = "0.1" +plugin.ui = { pages = { + { page_id = "third-a", title = "Third A", order = 80, mount = "" }, + { page_id = "third-b", title = "Third B", order = 81, mount = "" }, +} } +return plugin`) + + ui := ps.UI() + if ui == nil { + t.Fatal("no merged UI") + } + got := map[string]bool{} + for _, pg := range ui.Pages { + got[pg.PageID] = true + } + for _, want := range []string{"billing", "billing-rules", "other-page", "third-a", "third-b"} { + if !got[want] { + t.Errorf("merged UI lost page %q; has %v", want, got) + } + } + // A duplicate page_id must not appear twice. Two plugins claiming the same + // id collide in the DOM (getElementById returns the first, the second pane + // is silently unreachable), so the merge keeps the first and drops the + // later one. + mk("collide", ` +local plugin = {} +plugin.name = "collide" +plugin.version = "0.1" +plugin.ui = { pages = { + { page_id = "third-a", title = "Impostor", order = 79, mount = "" }, +} } +return plugin`) + ui = ps.UI() + counts := map[string]int{} + for _, pg := range ui.Pages { + counts[pg.PageID]++ + } + if counts["third-a"] != 1 { + t.Errorf("page id third-a appears %d times; a duplicate id collides in the DOM", counts["third-a"]) + } + for _, pg := range ui.Pages { + if pg.PageID == "third-a" && pg.Title == "Impostor" { + t.Error("the LATER plugin won the id; first writer should keep it") + } + } + + // Order must be honoured so the sidebar is predictable. + if len(ui.Pages) != 5 { + t.Fatalf("expected all 5 pages to merge, got %d: %v", len(ui.Pages), got) + } + for i := 1; i < len(ui.Pages); i++ { + if ui.Pages[i].Order < ui.Pages[i-1].Order { + t.Errorf("pages out of order at %d: %d before %d", + i, ui.Pages[i-1].Order, ui.Pages[i].Order) + } + } +} + // TestBillingPluginLoadedByDefault: the shipped plugin must load with no // configuration, since seeding only happens on a fresh plugin dir. func TestBillingPluginLoadedByDefault(t *testing.T) { diff --git a/internal/lua/hook_guard_test.go b/internal/lua/hook_guard_test.go new file mode 100644 index 0000000..3bab5b1 --- /dev/null +++ b/internal/lua/hook_guard_test.go @@ -0,0 +1,76 @@ +package lua + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// A plugin hook that RAISES must not take the process down. This is the +// production outage: a Lua error anywhere in a hook made golua's callEx call +// L.StackTrace(), which calls lua_getinfo and SIGSEGVs on a deep enough stack — +// a C-level signal Go cannot recover from, so one buggy plugin killed the whole +// gateway and took every in-flight request with it. +// +// The fix routes hook calls through a Lua-side pcall guard, so the error comes +// back as an ordinary return value. This test fires hooks that raise on purpose +// and asserts three things: the process survives, the failure is RECORDED, and +// a well-behaved plugin on the same state keeps working afterwards (the error +// must not poison the Lua state). +func TestHookThatRaisesDoesNotCrashTheProcess(t *testing.T) { + ps, pdir := billingVM(t) + boom := ` +local plugin = {} +plugin.name = "boom" +plugin.version = "0.1" +function plugin.request_end(payload) + -- Raise on a table index, the exact shape of the billing bug that caused the + -- outage. Deliberately NOT a syntax error: this must load fine and fail only + -- when invoked. + local x = nil + return x.field +end +return plugin` + if err := os.WriteFile(filepath.Join(pdir, "boom.lua"), []byte(boom), 0644); err != nil { + t.Fatal(err) + } + if err := ps.LoadSource("boom", boom); err != nil { + t.Fatalf("load boom: %v", err) + } + + payload := map[string]interface{}{ + "model": "m", "source": "s", "ok": true, + "prompt_tokens": 100, "completion_tokens": 10, "time": 1750000000000, + } + // Fire many times: a single call could pass by luck, but if the error ever + // escapes into golua's C path the process dies and this test never returns. + for i := 0; i < 50; i++ { + ps.Fire(StageRequestEnd, payload) + } + // Reaching this line at all is the primary assertion. + + errs := ps.HookErrors() + end, ok := errs[string(StageRequestEnd)] + if !ok { + t.Fatal("a raising hook left no record — failures must be observable, not swallowed") + } + if end["count"] == nil || end["count"].(int) == 0 { + t.Error("hook error count is zero despite 50 raising calls") + } + msg, _ := end["last_error"].(string) + if !strings.Contains(msg, "boom") { + t.Errorf("last_error does not name the offending plugin: %q", msg) + } + + // The billing plugin shares the same Plugins registry and must still work: + // one broken plugin may not disable the others. + st, _ := ps.State("billing").(map[string]interface{}) + if st == nil || st["total"] == nil { + t.Fatalf("the healthy plugin stopped working after another plugin raised") + } + tot, _ := st["total"].(map[string]interface{}) + if tot == nil || tot["requests"] == nil || tot["requests"].(float64) == 0 { + t.Errorf("billing recorded no requests after the raising plugin ran: %v", st["total"]) + } +} diff --git a/internal/lua/plugins.go b/internal/lua/plugins.go index e31926c..2afc8ad 100644 --- a/internal/lua/plugins.go +++ b/internal/lua/plugins.go @@ -115,6 +115,14 @@ type UIExtension struct { // The kernel renders Page's HTML into a pane whose id is "tab-"+PageID and // adds a sidebar button with data-tab="