fix(#1704): clear stale tab on node switch + sync tree focusedId (#1706)

Two root causes of the explorer selection appearing stuck:

1. writeUrl in app/units/page.tsx preserved the ?tab= param when only
   a new node was selected. Switching from a Unit with ?tab=Agents to an
   Agent node left ?tab=Agents in the URL; DetailPane's tab-correction
   useEffect then fired with a potentially-stale selectedId closure and
   could overwrite a subsequent click's URL update.

   Fix: delete the tab param whenever writeUrl is called with node but
   not tab. Also hoist the useCallback hooks above the early returns to
   comply with rules-of-hooks, and wrap the handlers in useCallback so
   the correction effect runs less often.

2. UnitTree initialised focusedId (the roving-tabindex anchor) from the
   selectedId prop via useState but never re-synced it on prop changes.
   After URL-driven selection changes (Cmd-K, deep-links, dashboard nav),
   the keyboard tabstop stayed on the old row. Pressing Enter then
   re-selected the old node, making selection look "snapped back".

   Fix: useEffect(() => setFocusedId(selectedId), [selectedId]).

Closes #1704.

Co-authored-by: savasp-agent[bot] <275188714+savasp-agent[bot]@users.noreply.github.com>

Author: savasp-agent[bot]

src/Cvoya.Spring.Web/src/app/units/page.tsx

@@ -10,7 +10,7 @@
 // route stays until `DEL-units-id` lands because its tabs still host
 // content the EXP-tab-unit-* issues are migrating into the Explorer.
 
-import { Suspense } from "react";
+import { Suspense, useCallback } from "react";
 import Link from "next/link";
 import { AlertCircle, Loader2, Plus } from "lucide-react";
 import { usePathname, useRouter, useSearchParams } from "next/navigation";
@@ -39,6 +39,42 @@ function UnitExplorerRoute() {
 
   const treeQuery = useTenantTree();
 
+  // Hooks must be declared before any early return (#1704, react-hooks/rules-of-hooks).
+  // `writeUrl`, `handleSelectNode`, and `handleTabChange` only depend on URL
+  // state that is available on every render path.
+  const writeUrl = useCallback(
+    (next: { node?: string; tab?: TabName }) => {
+      const params = new URLSearchParams(searchParams.toString());
+      if (next.node !== undefined) {
+        params.set("node", next.node);
+        // #1704: clear a stale `tab` when only switching the node. Keeping
+        // the old tab across a node switch makes `DetailPane` see an invalid
+        // tab and fire its correction effect with a potentially-stale
+        // `selectedId` closure, which can overwrite a subsequent click.
+        if (next.tab === undefined) params.delete("tab");
+      }
+      if (next.tab !== undefined) params.set("tab", next.tab);
+      const qs = params.toString();
+      // #1039: Next.js 16's `router.replace("?foo=bar")` with a bare
+      // query-only relative URL doesn't update the canonical URL — the
+      // reconciler's `replaceState` call fires with the stale query, so
+      // the URL (and controlled `tab`/`node` props derived from it) snap
+      // back to the prior value the moment React commits. Passing the
+      // full pathname alongside the query restores the intended navigation.
+      router.replace(qs ? `${pathname}?${qs}` : pathname, { scroll: false });
+    },
+    [searchParams, pathname, router],
+  );
+
+  const handleSelectNode = useCallback(
+    (id: string) => writeUrl({ node: id }),
+    [writeUrl],
+  );
+  const handleTabChange = useCallback(
+    (id: string, nextTab: TabName) => writeUrl({ node: id, tab: nextTab }),
+    [writeUrl],
+  );
+
   if (treeQuery.isError) {
     return (
       <Card
@@ -80,20 +116,6 @@ function UnitExplorerRoute() {
 
   const tree = adaptValidatedNode(treeQuery.data);
 
-  const writeUrl = (next: { node?: string; tab?: TabName }) => {
-    const params = new URLSearchParams(searchParams.toString());
-    if (next.node !== undefined) params.set("node", next.node);
-    if (next.tab !== undefined) params.set("tab", next.tab);
-    const qs = params.toString();
-    // #1039: Next.js 16's `router.replace("?foo=bar")` with a bare
-    // query-only relative URL doesn't update the canonical URL — the
-    // reconciler's `replaceState` call fires with the stale query, so
-    // the URL (and controlled `tab`/`node` props derived from it) snap
-    // back to the prior value the moment React commits. Passing the
-    // full pathname alongside the query restores the intended navigation.
-    router.replace(qs ? `${pathname}?${qs}` : pathname, { scroll: false });
-  };
-
   return (
     <div
       data-testid="unit-explorer-route"
@@ -107,9 +129,9 @@ function UnitExplorerRoute() {
         <UnitExplorer
           tree={tree}
           selectedId={selectedId}
-          onSelectNode={(id) => writeUrl({ node: id })}
+          onSelectNode={handleSelectNode}
           tab={tab ?? undefined}
-          onTabChange={(id, nextTab) => writeUrl({ node: id, tab: nextTab })}
+          onTabChange={handleTabChange}
         />
       </div>
     </div>

src/Cvoya.Spring.Web/src/app/units/units-page.test.tsx

@@ -91,6 +91,8 @@ vi.mock("@/lib/api/queries", () => ({
   // with "no data" so Explorer page tests don't need to model
   // execution defaults.
   useUnitExecution: () => ({ data: null, isLoading: false }),
+  // Agents tab — MembershipDialog reads the agent-runtimes catalog.
+  useAgentRuntimes: () => ({ data: [], isLoading: false }),
 }));
 
 function wrap(node: ReactNode) {
@@ -213,6 +215,32 @@ describe("UnitsPage — Explorer route (EXP-route)", () => {
     expect(replaceMock.mock.calls.at(-1)?.[0]).toMatch(/node=engineering/);
   });
 
+  it("#1704: clears a stale ?tab when switching to a different node kind", async () => {
+    // Start with the tenant root showing and a Unit-only tab (Agents) in the
+    // URL from a prior navigation. We simulate this by seeding the URL with
+    // the tab but NOT the node so the page renders at the root (Tenant) —
+    // this avoids rendering the full Unit Agents tab which needs extra mocks.
+    currentSearchParams = new URLSearchParams("tab=Agents");
+    useTenantTreeMock.mockReturnValue({
+      data: sampleTree,
+      isLoading: false,
+      isError: false,
+    });
+    render(wrap(<UnitsPage />));
+    await screen.findByTestId("unit-explorer");
+
+    // Click the Engineering unit. The stale ?tab=Agents is valid for a Unit,
+    // so it carries over (correct). Then click Marketing — we just need the
+    // first write to confirm the node switches without re-carrying the tab.
+    replaceMock.mockClear();
+    fireEvent.click(screen.getByTestId("tree-row-marketing"));
+    await waitFor(() => expect(replaceMock).toHaveBeenCalled());
+    const urlAfterSwitch = replaceMock.mock.calls.at(-1)?.[0] ?? "";
+    // Node must update and the stale cross-kind tab must be cleared.
+    expect(urlAfterSwitch).toMatch(/node=marketing/);
+    expect(urlAfterSwitch).not.toMatch(/tab=/);
+  });
+
   it("renders a 'New unit' link in the page header pointing to /units/create (#1069)", async () => {
     useTenantTreeMock.mockReturnValue({
       data: sampleTree,

src/Cvoya.Spring.Web/src/components/units/unit-tree.test.tsx

@@ -353,6 +353,41 @@ describe("UnitTree", () => {
     });
   });
 
+  it("#1704: re-anchors keyboard tabstop when selectedId changes externally", () => {
+    const { rerender } = render(
+      <UnitTree
+        tree={tree}
+        selectedId="tenant-acme"
+        onSelect={vi.fn()}
+        defaultExpanded={{ "tenant-acme": true }}
+      />,
+    );
+    expect(screen.getByTestId("tree-row-tenant-acme")).toHaveAttribute(
+      "tabindex",
+      "0",
+    );
+
+    // Simulate an external selection change (URL navigation / Cmd-K / deep-link).
+    rerender(
+      <UnitTree
+        tree={tree}
+        selectedId="unit-eng"
+        onSelect={vi.fn()}
+        defaultExpanded={{ "tenant-acme": true }}
+      />,
+    );
+
+    // The new selection should now hold tabIndex=0 so keyboard Enter targets it.
+    expect(screen.getByTestId("tree-row-unit-eng")).toHaveAttribute(
+      "tabindex",
+      "0",
+    );
+    expect(screen.getByTestId("tree-row-tenant-acme")).toHaveAttribute(
+      "tabindex",
+      "-1",
+    );
+  });
+
   it("surfaces a worst-status buried four levels deep on the collapsed top-level row", () => {
     // Fixture independent of the file-scoped `tree` above: a
     // Tenant → Unit → Unit → Unit → Agent(error) chain where only the leaf

src/Cvoya.Spring.Web/src/components/units/unit-tree.tsx

@@ -4,6 +4,7 @@ import { Bot, Globe, Layers } from "lucide-react";
 import {
   type KeyboardEvent,
   useCallback,
+  useEffect,
   useMemo,
   useRef,
   useState,
@@ -76,6 +77,15 @@ export function UnitTree({
   // the selected row, but tracks arrow-key movement separately so operators
   // can survey the tree without shifting selection.
   const [focusedId, setFocusedId] = useState<string>(selectedId);
+
+  // #1704: re-anchor the tabstop when `selectedId` changes externally (URL
+  // navigation, Cmd-K teleport, dashboard deep-link). Without this, keyboard
+  // Enter would re-select the previously-focused row, making selection appear
+  // to "snap back" to the old node.
+  useEffect(() => {
+    setFocusedId(selectedId);
+  }, [selectedId]);
+
   const containerRef = useRef<HTMLDivElement>(null);
   // Short-lived type-ahead buffer: printable keys append; ~500 ms of
   // silence resets.