fix(a11y): capture overlay invokers before async loads

OnlineChef · cursoragent · OnlineChef · commit 831ba670dfd3 · 2026-08-01T21:39:10.000+02:00
Pass pre-fetch invokers into openDetail/openCredentialsModal, drop stale openServerDetail responses via a generation counter, and cover the async refresh contract in the Node harness so CodeRabbit findings on tip 40a6658 stay closed. Co-authored-by: Cursor <cursoragent@cursor.com>
diff --git a/src/kater/web/dashboard.py b/src/kater/web/dashboard.py
@@ -1616,6 +1616,9 @@
 let selectedNode = null;
 let detailInvoker = null;
 let credInvoker = null;
+// Bumps on every openServerDetail call so a late response from an older
+// fetch cannot reopen or overwrite the panel after the user moved on.
+let detailRequestGen = 0;
 
 // Overview live state.
 let routeFilter = 'all';
@@ -2320,14 +2323,15 @@ class ApiError extends Error {
 
 async function openSelectedRoute() {
   if (routeSel < 0 || !routeRows[routeSel]) return;
-  const name = routeRows[routeSel].dataset.name;
+  const invoker = routeRows[routeSel];
+  const name = invoker.dataset.name;
   let node = servers.find(s => s.name === name);
   if (!node) return;
   if (!node.mcp) {
     try { node = await api('/api/mcp/servers/' + encodeURIComponent(node.name)); }
     catch (err) { /* fall back */ }
   }
-  openDetail(node);
+  openDetail(node, false, invoker);
 }
 
 function buildNodes() {
@@ -2351,7 +2355,7 @@ class ApiError extends Error {
     try { node = await api('/api/mcp/servers/' + encodeURIComponent(node.name)); }
     catch (err) { /* fall back */ }
   }
-  openDetail(node);
+  openDetail(node, false, row);
 }
 
 function startAnimationLoop() {}
@@ -2366,14 +2370,16 @@ class ApiError extends Error {
   return '-';
 }
 
-function openDetail(node, refresh) {
+function openDetail(node, refresh, invoker) {
   // Track the latest trigger from outside the panel: selecting another server
   // while the panel is open must move the return target to that row, but focus
   // already inside the panel (close button, actions) is not a return target.
   // Background refreshes (WebSocket updates, post-action reloads) pass
   // refresh=true and must not overwrite the invoker: whatever happens to hold
   // focus then (e.g. the command bar) never opened the panel.
-  const trigger = document.activeElement;
+  // Callers that await a fetch pass the pre-captured invoker so a focus move
+  // during the request cannot steal the return target.
+  const trigger = invoker !== undefined ? invoker : document.activeElement;
   const openPanel = document.getElementById('detail-panel');
   if (!refresh && trigger && trigger.tagName !== 'BODY'
       && !openPanel.contains(trigger)) {
@@ -2487,21 +2493,23 @@ class ApiError extends Error {
   if (selectedNode && selectedNode.name) promptCredentials(selectedNode.name);
 }
 
-async function promptCredentials(name) {
+async function promptCredentials(name, invoker) {
   // Always work from the full server doc: the catalog payload omits the list
   // of required env vars, the detail endpoint has it.
+  // Capture the trigger before the await — focus may move while loading.
+  const captured = invoker !== undefined ? invoker : document.activeElement;
   try {
     const server = await api('/api/mcp/servers/' + encodeURIComponent(name));
-    if (server && !server.error) openCredentialsModal(server);
+    if (server && !server.error) openCredentialsModal(server, captured);
   } catch (e) {
     toast('Could not load ' + name + ': ' + (e.message || 'failed'), 'error');
   }
 }
 
-function openCredentialsModal(server) {
-  if (!credInvoker && document.activeElement
-      && document.activeElement.tagName !== 'BODY') {
-    credInvoker = document.activeElement;
+function openCredentialsModal(server, invoker) {
+  const trigger = invoker !== undefined ? invoker : document.activeElement;
+  if (!credInvoker && trigger && trigger.tagName !== 'BODY') {
+    credInvoker = trigger;
   }
   credServer = server;
   const reqs = server.env_required || [];
@@ -2683,7 +2691,7 @@ class ApiError extends Error {
       return;
     }
     const card = e.target.closest('.server-card');
-    if (card && card.dataset.name) openServerDetail(card.dataset.name);
+    if (card && card.dataset.name) openServerDetail(card.dataset.name, false, card);
   });
   grid.addEventListener('keydown', (e) => {
     if (e.key === 'Enter' || e.key === ' ') {
@@ -2697,7 +2705,7 @@ class ApiError extends Error {
       const card = e.target.closest('.server-card');
       if (card && card.dataset.name && e.target === card) {
         e.preventDefault();
-        openServerDetail(card.dataset.name);
+        openServerDetail(card.dataset.name, false, card);
       }
     }
   });
@@ -3572,10 +3580,25 @@ class ApiError extends Error {
   }
 }
 
-async function openServerDetail(name, refresh) {
+async function openServerDetail(name, refresh, invoker) {
+  // Capture before the await so a focus move during the fetch cannot steal
+  // the return target. Background refreshes skip capture entirely.
+  const captured = refresh
+    ? null
+    : (invoker !== undefined ? invoker : document.activeElement);
+  const gen = ++detailRequestGen;
   try {
     const data = await api('/api/mcp/servers/' + encodeURIComponent(name));
-    openDetail(data, refresh);
+    if (gen !== detailRequestGen) return;
+    if (refresh) {
+      // Drop stale refreshes: panel closed, or user selected another server.
+      const panel = document.getElementById('detail-panel');
+      if (!panel || !panel.classList.contains('open')) return;
+      if (!selectedNode || selectedNode.name !== name) return;
+      openDetail(data, true);
+      return;
+    }
+    openDetail(data, false, captured);
   } catch (e) {
     toast(name + ': ' + (e.message || 'not found'), 'error');
   }
diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py
@@ -251,6 +251,9 @@ def test_pr_view_has_accessible_refresh_button_and_status_region():
 def _extract_js_function(source: str, name: str) -> str:
     """Slice a whole `function name(...) {...}` declaration out of the JS."""
     start = source.index(f"function {name}(")
+    # Keep a leading `async` so awaited helpers stay valid when re-run in Node.
+    if start >= 6 and source[start - 6 : start] == "async ":
+        start -= 6
     depth = 0
     for pos in range(source.index("{", start), len(source)):
         if source[pos] == "{":
@@ -485,38 +488,69 @@ def test_focus_restoration_logic_is_present():
     html = render_dashboard()
     assert "let detailInvoker = null;" in html
     assert "let credInvoker = null;" in html
+    assert "let detailRequestGen = 0;" in html
     assert "function openDetail" in html
-    assert "const trigger = document.activeElement;" in html
+    assert "const trigger = invoker !== undefined ? invoker : document.activeElement;" in html
     assert "detailInvoker = trigger;" in html
     assert "function closeDetail" in html
     assert "invoker.focus()" in html
     assert "function openCredentialsModal" in html
-    assert "credInvoker = document.activeElement" in html
+    assert "credInvoker = trigger;" in html
     assert "function closeCredentialsModal" in html
+    assert "function openServerDetail" in html
+    assert "if (gen !== detailRequestGen) return;" in html
+    # Real callers must keep refresh=true on background reloads.
+    assert "openServerDetail(name, true)" in html
+    assert "openServerDetail(data.name, true)" in html
 
 
 _FOCUS_HARNESS = r"""
+const panelClassList = {
+  open: false,
+  add(c) { if (c === 'open') this.open = true; },
+  remove(c) { if (c === 'open') this.open = false; },
+  contains(c) { return c === 'open' ? this.open : false; },
+};
+const detailPanel = {
+  classList: panelClassList,
+  appendChild() {},
+  contains() { return false; },
+  innerHTML: '',
+  textContent: '',
+  style: {},
+};
 const document = {
   activeElement: null,
   contains(el) { return true; },
   getElementById(id) {
+    if (id === 'detail-panel') return detailPanel;
     return {
-      classList: { remove() {}, add() {} },
+      classList: { remove() {}, add() {}, contains() { return false; } },
       appendChild() {},
       contains() { return false; },
       innerHTML: '',
       textContent: '',
       style: {},
+      disabled: false,
     };
   }
 };
 let selectedNode = null;
 let writeUrlState = () => {};
 const makeBadge = () => ({ classList: { remove() {} } });
 const formatLaunch = () => '-';
+function toast() {}
 
 let detailInvoker = null;
 let credInvoker = null;
+let detailRequestGen = 0;
+
+let pendingApi = null;
+function api(url) {
+  return new Promise((resolve, reject) => {
+    pendingApi = { url, resolve, reject };
+  });
+}
 
 /*__DASHBOARD_JS__*/
 
@@ -527,36 +561,79 @@ def test_focus_restoration_logic_is_present():
 });
 const cardA = makeCard();
 const cardB = makeCard();
+const commandBar = {
+  tagName: 'INPUT',
+  focusCalled: false,
+  focus() { this.focusCalled = true; },
+};
 
-document.activeElement = cardA;
-openDetail({ name: 'a' });
-const afterOpenDetailInvoker = detailInvoker;
-
-// Selecting another server while the panel stays open must retarget focus.
-document.activeElement = cardB;
-openDetail({ name: 'b' });
-const afterReselectInvoker = detailInvoker;
-
-// A background refresh (WebSocket update) while the user works elsewhere
-// (e.g. the command bar) must not steal the return target from the last
-// explicitly selected row.
-const commandBar = { tagName: 'INPUT', focusCalled: false, focus() { this.focusCalled = true; } };
-document.activeElement = commandBar;
-openDetail({ name: 'b' }, true);
-const afterRefreshInvoker = detailInvoker;
-
-closeDetail();
-const afterCloseDetailInvoker = detailInvoker;
-
-process.stdout.write(JSON.stringify({
-  detailInvokerSet: afterOpenDetailInvoker === cardA,
-  detailInvokerFollowsSelection: afterReselectInvoker === cardB,
-  detailInvokerSurvivesRefresh: afterRefreshInvoker === cardB,
-  detailInvokerCleared: afterCloseDetailInvoker === null,
-  focusCalled: cardB.focusCalled,
-  staleInvokerFocused: cardA.focusCalled,
-  refreshFocusStolen: commandBar.focusCalled,
-}));
+(async () => {
+  document.activeElement = cardA;
+  openDetail({ name: 'a' });
+  const afterOpenDetailInvoker = detailInvoker;
+
+  // Selecting another server while the panel stays open must retarget focus.
+  document.activeElement = cardB;
+  openDetail({ name: 'b' });
+  const afterReselectInvoker = detailInvoker;
+
+  // A background refresh (WebSocket update) while the user works elsewhere
+  // (e.g. the command bar) must not steal the return target from the last
+  // explicitly selected row.
+  document.activeElement = commandBar;
+  openDetail({ name: 'b' }, true);
+  const afterRefreshInvoker = detailInvoker;
+
+  // Real caller: openServerDetail captures the invoker before the await.
+  // Move focus to the command bar while the API response is still pending.
+  detailInvoker = cardB;
+  selectedNode = { name: 'b' };
+  panelClassList.open = true;
+  document.activeElement = cardB;
+  const openPromise = openServerDetail('b', false, cardB);
+  document.activeElement = commandBar;
+  pendingApi.resolve({
+    name: 'b', env_required: [], env_configured: true, enabled: true,
+  });
+  await openPromise;
+  const afterAsyncInvoker = detailInvoker;
+
+  // Background refresh via the real caller must pass refresh=true and keep
+  // the prior invoker even when focus sits on the command bar.
+  document.activeElement = commandBar;
+  const refreshPromise = openServerDetail('b', true);
+  pendingApi.resolve({
+    name: 'b', env_required: [], env_configured: true, enabled: true,
+  });
+  await refreshPromise;
+  const afterCallerRefreshInvoker = detailInvoker;
+
+  // Stale refresh after the panel closed must not reopen it.
+  closeDetail();
+  const afterCloseDetailInvoker = detailInvoker;
+  panelClassList.open = false;
+  selectedNode = null;
+  const staleRefresh = openServerDetail('b', true);
+  pendingApi.resolve({ name: 'b', env_required: [], env_configured: true });
+  await staleRefresh;
+  const staleRefreshReopened = panelClassList.open;
+
+  process.stdout.write(JSON.stringify({
+    detailInvokerSet: afterOpenDetailInvoker === cardA,
+    detailInvokerFollowsSelection: afterReselectInvoker === cardB,
+    detailInvokerSurvivesRefresh: afterRefreshInvoker === cardB,
+    detailInvokerSurvivesAsyncFetch: afterAsyncInvoker === cardB,
+    detailInvokerSurvivesCallerRefresh: afterCallerRefreshInvoker === cardB,
+    detailInvokerCleared: afterCloseDetailInvoker === null,
+    focusCalled: cardB.focusCalled,
+    staleInvokerFocused: cardA.focusCalled,
+    refreshFocusStolen: commandBar.focusCalled,
+    staleRefreshDropped: staleRefreshReopened === false,
+  }));
+})().catch((err) => {
+  console.error(err);
+  process.exit(1);
+});
 """
 
 
@@ -567,7 +644,8 @@ def test_focus_restoration_behavior_node(tmp_path):
     assert node is not None
     html = render_dashboard()
     dashboard_js = "\n".join(
-        _extract_js_function(html, name) for name in ("openDetail", "closeDetail")
+        _extract_js_function(html, name)
+        for name in ("openDetail", "closeDetail", "openServerDetail")
     )
     script = tmp_path / "focus_restoration.cjs"
     script.write_text(
@@ -582,10 +660,13 @@ def test_focus_restoration_behavior_node(tmp_path):
     assert res["detailInvokerSet"] is True
     assert res["detailInvokerFollowsSelection"] is True
     assert res["detailInvokerSurvivesRefresh"] is True
+    assert res["detailInvokerSurvivesAsyncFetch"] is True
+    assert res["detailInvokerSurvivesCallerRefresh"] is True
     assert res["detailInvokerCleared"] is True
     assert res["focusCalled"] is True
     assert res["staleInvokerFocused"] is False
     assert res["refreshFocusStolen"] is False
+    assert res["staleRefreshDropped"] is True
 
 
 # The credentials modal builds real DOM, so it needs the element shim rather
