Skip to content

Commit a88604e

Browse files
bugfix/5155-roots-panel-openmenu-undefined (#5291)
* bugfix/5155-roots-panel-openmenu-undefined Signed-off-by: NAYANA.R <nayana.r7813@gmail.com> * addind test case Signed-off-by: NAYANA.R <nayana.r7813@gmail.com> * adding test Signed-off-by: NAYANA.R <nayana.r7813@gmail.com> --------- Signed-off-by: NAYANA.R <nayana.r7813@gmail.com>
1 parent 4999e17 commit a88604e

4 files changed

Lines changed: 120 additions & 4 deletions

File tree

mcpgateway/admin_ui/components/overflow-menu.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
* - The dropdown container must have `x-ref="menu"` and `role="menu"`.
1111
* - Each action inside the menu must carry `role="menuitem"`.
1212
*
13-
* Usage:
14-
* <div x-data="Admin.overflowMenu('tools-table-wrapper')"
13+
* Usage (in HTML templates):
14+
* <div x-data="overflowMenu('tools-table-wrapper')"
1515
* @click.away="menuOpen = false"
1616
* @keydown.escape="menuOpen = false; $refs.trigger.focus()">
1717
* <button x-ref="trigger" @click="menuOpen ? (menuOpen = false) : openMenu()" ...>…</button>
@@ -20,6 +20,12 @@
2020
* </div>
2121
* </div>
2222
*
23+
* NOTE: overflowMenu is registered via Alpine.data('overflowMenu', overflowMenu)
24+
* in alpine-setup.js. Templates must reference it directly as `overflowMenu('...')`,
25+
* NOT via window.Admin.overflowMenu(...) — the latter pattern causes Alpine to
26+
* initialize with an empty scope (no menuOpen, no openMenu) because optional
27+
* chaining returns undefined when the property is missing on window.Admin.
28+
*
2329
* @param {string|null} wrapperId - Optional id of a scroll-constrained wrapper
2430
* (e.g. `tools-table-wrapper`) whose `overflow` should also be pinned while
2531
* the menu is open so the fixed-positioned dropdown isn't clipped.

mcpgateway/templates/admin.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7935,7 +7935,7 @@ <h2 class="text-2xl font-bold dark:text-gray-200">
79357935
<tr>
79367936
<td class="px-6 py-4 whitespace-nowrap text-sm font-medium">
79377937
<div class="relative"
7938-
x-data="window.Admin?.overflowMenu?.('roots-table-wrapper')"
7938+
x-data="overflowMenu('roots-table-wrapper')"
79397939
@click.away="menuOpen = false"
79407940
@keydown.escape="menuOpen = false; $refs.trigger.focus()">
79417941
<button

tests/unit/js/overflow-menu.test.js

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,116 @@ describe("dispatch", () => {
712712
});
713713
});
714714

715+
// ─── Regression: x-data access pattern (issue #5155) ─────────────────────────
716+
//
717+
// The bug: admin.html used window.Admin?.overflowMenu?.('roots-table-wrapper')
718+
// as the x-data expression. Since overflowMenu is registered via Alpine.data()
719+
// (not attached to window.Admin), optional chaining returned undefined and
720+
// Alpine initialized the component with an empty scope — menuOpen and openMenu
721+
// were never defined, causing "Undefined variable: openMenu" on click.
722+
//
723+
// All other sections (gateways, tools, agents, resources, servers, prompts)
724+
// use the Alpine-registered name directly: x-data="overflowMenu(...)".
725+
// These tests confirm that pattern works and the broken one does not.
726+
727+
describe("Regression: x-data access pattern (issue #5155)", () => {
728+
afterEach(() => {
729+
delete window.Admin;
730+
});
731+
732+
test("overflowMenu is NOT on window.Admin (Alpine.data, not window.Admin)", () => {
733+
window.Admin = {};
734+
expect(window.Admin.overflowMenu).toBeUndefined();
735+
});
736+
737+
test("window.Admin?.overflowMenu?.() evaluates to undefined (the bug)", () => {
738+
window.Admin = {};
739+
const result = window.Admin?.overflowMenu?.("roots-table-wrapper");
740+
expect(result).toBeUndefined();
741+
});
742+
743+
test("window.Admin?.overflowMenu?.() evaluates to undefined even with null Admin", () => {
744+
window.Admin = null;
745+
const result = window.Admin?.overflowMenu?.("roots-table-wrapper");
746+
expect(result).toBeUndefined();
747+
});
748+
749+
test("window.Admin?.overflowMenu?.() evaluates to undefined when Admin is absent", () => {
750+
delete window.Admin;
751+
const result = window.Admin?.overflowMenu?.("roots-table-wrapper");
752+
expect(result).toBeUndefined();
753+
});
754+
755+
test("overflowMenu('...') called directly returns a valid component with menuOpen/openMenu", () => {
756+
const component = overflowMenu("roots-table-wrapper");
757+
expect(component).toBeDefined();
758+
expect(component.menuOpen).toBe(false);
759+
expect(typeof component.openMenu).toBe("function");
760+
expect(typeof component.init).toBe("function");
761+
});
762+
763+
test("overflowMenu('...') called directly works when window.Admin is absent (full isolation)", () => {
764+
delete window.Admin;
765+
const component = overflowMenu("roots-table-wrapper");
766+
expect(component).toBeDefined();
767+
expect(component.menuOpen).toBe(false);
768+
expect(typeof component.openMenu).toBe("function");
769+
770+
// Simulate a real click flow (as Alpine would)
771+
const { menu } = createMenuItems(1);
772+
const trigger = document.createElement("button");
773+
trigger.getBoundingClientRect = vi.fn(() => ({ bottom: 100, left: 50, top: 80 }));
774+
component.$refs = { trigger, menu };
775+
component.$watch = vi.fn();
776+
component.$nextTick = vi.fn((cb) => cb());
777+
component.init();
778+
779+
component.openMenu();
780+
expect(component.menuOpen).toBe(true);
781+
expect(component.menuTop).toBe(104);
782+
});
783+
784+
test("overflowMenu('...') returns component with closeMenu and navigate", () => {
785+
// These are used in the template markup alongside overflowMenu
786+
const component = overflowMenu("roots-table-wrapper");
787+
expect(typeof component.closeMenu).toBe("function");
788+
expect(typeof component.navigate).toBe("function");
789+
expect(typeof component.dispatch).toBe("function");
790+
expect(typeof component.destroy).toBe("function");
791+
});
792+
793+
test("BUG REPRODUCTION: inline x-data expression matches broken pattern", () => {
794+
// This is what admin.html line 7938 *was* before the fix:
795+
// x-data="window.Admin?.overflowMenu?.('roots-table-wrapper')"
796+
// Alpine evaluates that as a JS expression. We confirm the result is
797+
// always undefined regardless of window.Admin state.
798+
window.Admin = {};
799+
const expression1 = window.Admin?.overflowMenu?.("roots-table-wrapper");
800+
expect(expression1).toBeUndefined();
801+
802+
window.Admin = { overflowMenu: undefined };
803+
const expression2 = window.Admin?.overflowMenu?.("roots-table-wrapper");
804+
expect(expression2).toBeUndefined();
805+
806+
// With a real function on window.Admin.overflowMenu it would work,
807+
// but that's not how overflowMenu is registered.
808+
});
809+
810+
test("BUG REPRODUCTION: undefined x-data scope cannot provide openMenu", () => {
811+
// When x-data="window.Admin?.overflowMenu?.('...')" evaluates to undefined,
812+
// Alpine initializes the component with an empty/undefined scope.
813+
// In that scenario, calling openMenu is impossible because it doesn't exist.
814+
const brokenExpression = window.Admin?.overflowMenu?.("roots-table-wrapper");
815+
expect(brokenExpression).toBeUndefined();
816+
817+
// In contrast, the correct pattern returns a component with openMenu.
818+
const fixedExpression = overflowMenu("roots-table-wrapper");
819+
expect(fixedExpression).toBeDefined();
820+
expect(typeof fixedExpression.openMenu).toBe("function");
821+
expect(typeof fixedExpression.menuOpen).toBe("boolean");
822+
});
823+
});
824+
715825
// ─── Multi-menu coordination ──────────────────────────────────────────────────
716826

717827
describe("Multi-menu coordination", () => {

tests/unit/js/tokens.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ describe("showTokenDetailsModal", () => {
278278
document.body.innerHTML = "";
279279
});
280280

281-
test("creates and appends modal to body", () => {
281+
test("creates and appends modal to body", { timeout: 15000 }, () => {
282282
window.USERTEAMSDATA = [];
283283
showTokenDetailsModal({
284284
id: "tok-123",

0 commit comments

Comments
 (0)