diff --git a/tests/test_tenants_endpoint.py b/tests/test_tenants_endpoint.py index 21d3bb663554..55f0bd5c1654 100644 --- a/tests/test_tenants_endpoint.py +++ b/tests/test_tenants_endpoint.py @@ -229,3 +229,63 @@ def test_fe_aggregate_and_deeplink_pins(): "cost_ladder_by_tenant block — sole canonical per-tenant " "cost surface" ) + + +def test_fe_keyboard_nav_and_url_toggle_and_tab_title_pins(): + """KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — + pin the keyboard contract + URL-toggle storage key + browser-tab + title format. Each of these is part of the operator-facing + contract; renaming a key on one side without the other silently + breaks documented behavior. + """ + repo = Path(__file__).parent.parent + + # A.4 — keyboard shortcuts pin. The TENANT_PICKER_KEYBOARD_SHORTCUTS + # object is the single source of truth; the implementation key + # checks below assert the documented keys actually appear in + # the handler. + picker_src = ( + repo / "web" / "src" / "components" / "TenantPicker.tsx" + ).read_text(encoding="utf-8") + assert "TENANT_PICKER_KEYBOARD_SHORTCUTS" in picker_src + # Implementation handles each of the documented keys. + for key in ('"ArrowDown"', '"ArrowUp"', '"Enter"', '"Escape"'): + assert key in picker_src, ( + f"TenantPicker keyboard handler must reference {key} — " + "pinned by TENANT_PICKER_KEYBOARD_SHORTCUTS" + ) + # Letter-jump cycling exists (matches §4 STOP-ASK resolution + # to prefer cycling over first-match-only). + assert "cycleLetterJump" in picker_src + + # B.1 — URL-toggle storage key pinned in the hook + checkbox + # rendered in the picker. + hook_src = ( + repo / "web" / "src" / "hooks" / "useActiveTenant.ts" + ).read_text(encoding="utf-8") + assert ( + 'TENANT_PICKER_URL_TOGGLE_STORAGE_KEY =\n "kora_tenant_picker_update_url"' + in hook_src + or 'TENANT_PICKER_URL_TOGGLE_STORAGE_KEY = "kora_tenant_picker_update_url"' + in hook_src + ), ( + "URL-toggle localStorage key must be 'kora_tenant_picker_update_url' " + "— pinned for operator-facing localStorage stability" + ) + assert "useTenantUrlToggle" in hook_src + assert "useTenantUrlToggle" in picker_src, ( + "TenantPicker must render the URL-toggle checkbox via " + "useTenantUrlToggle" + ) + + # C.1 — tab-title prefix format. Pin the brand suffix + + # bracket-prefix shape so a future refactor can't silently + # change what shows up in the browser tab. + pageheader_src = ( + repo / "web" / "src" / "contexts" / "PageHeaderProvider.tsx" + ).read_text(encoding="utf-8") + assert 'TAB_TITLE_SUFFIX = "Hermes Agent"' in pageheader_src + assert "formatBrowserTabTitle" in pageheader_src + # Format pins: `[] · Hermes Agent`. + assert "[all tenants]" in pageheader_src + assert "document.title = formatBrowserTabTitle(" in pageheader_src diff --git a/web/src/components/TenantPicker.tsx b/web/src/components/TenantPicker.tsx index 0639c1fdc1e1..eca61acf540b 100644 --- a/web/src/components/TenantPicker.tsx +++ b/web/src/components/TenantPicker.tsx @@ -1,24 +1,31 @@ // KR-FE-TENANT-PICKER-COCKPIT-CHROME — cockpit-chrome dropdown for // switching the active tenant view across cost/audit/promotion pages. // -// Placement (per A.1): sidebar header section, directly below the -// "Hermes Agent" branding. Always visible when the sidebar is open -// (lg+: always; mobile: when the menu is toggled). Top-bar would -// also work but the sidebar slot is already present in App.tsx and +// Placement: sidebar header section, directly below the "Hermes +// Agent" branding. Always visible when the sidebar is open (lg+: +// always; mobile: when the menu is toggled). Top-bar would also +// work but the sidebar slot is already present in App.tsx and // avoids a layout-chrome refactor for the first cut. // -// Single-tenant degradation (A.6): renders nothing when the cost- -// holder registry contains < 2 tenants. The picker re-appears -// automatically when a second tenant emerges (re-fetched on focus -// by useActiveTenant). Keeps single-tenant operators (Joshua today) -// from seeing UI clutter. +// Single-tenant degradation: renders nothing when the cost-holder +// registry contains < 2 tenants. // // Aggregate-view option ("All tenants"): present iff there are ≥ 2 -// real tenants. Stored as ALL_TENANTS_SENTINEL — pages branch on -// useActiveTenant().isAllTenants to render aggregate vs single- -// tenant views. +// real tenants. Stored as ALL_TENANTS_SENTINEL. +// +// KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — +// keyboard nav + opt-in URL-toggle layered on top of the #207/#208 +// chrome. Keyboard contract pinned via TENANT_PICKER_KEYBOARD_SHORTCUTS +// + asserted by the cross-stack drift-guard test. -import { useCallback, useEffect, useState } from "react"; +import { + useCallback, + useEffect, + useMemo, + useRef, + useState, + type KeyboardEvent, +} from "react"; import { ChevronDown, Users } from "lucide-react"; import { cn } from "@/lib/utils"; import { @@ -26,8 +33,24 @@ import { DEFAULT_TENANT_ID, OPEN_TENANT_PICKER_EVENT, useActiveTenant, + useTenantUrlToggle, } from "@/hooks/useActiveTenant"; +// KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — +// pinned shortcut contract. Drift-guard test asserts this object's +// keys/values match the implementation below (a rename of one side +// without the other silently breaks documented operator behavior). +export const TENANT_PICKER_KEYBOARD_SHORTCUTS = { + open: ["Enter", " "] as const, // trigger button focused + navDown: "ArrowDown" as const, // wraps at end + navUp: "ArrowUp" as const, // wraps at start + select: "Enter" as const, // confirms highlighted option + close: "Escape" as const, // returns focus to trigger + // Letter-jump: any printable single-char key. Cycles through + // tenants whose id starts with that letter on repeated press. + letterJump: "<printable-single-char>" as const, +} as const; + export function TenantPicker() { const { activeTenant, @@ -37,8 +60,31 @@ export function TenantPicker() { isMultiTenant, loadingTenants, } = useActiveTenant(); + const [urlToggle, setUrlToggle] = useTenantUrlToggle(); const [open, setOpen] = useState(false); + // Render order mirrors the dropdown: real tenants in the order + // /api/tenants/list returned (default-first), then the aggregate + // sentinel as the final pseudo-row. + const options = useMemo( + () => [...availableTenants, ALL_TENANTS_SENTINEL], + [availableTenants], + ); + + // Index of the highlighted option for keyboard nav. -1 ≡ none + // highlighted (closed-state default; reset when picker closes). + // When picker opens we point this at the currently-active tenant + // so ↑/↓ start from where the operator already is. + const [highlight, setHighlight] = useState(-1); + const triggerRef = useRef<HTMLButtonElement | null>(null); + const listRef = useRef<HTMLDivElement | null>(null); + const optionRefs = useRef<Array<HTMLButtonElement | null>>([]); + // Track repeated-key presses for letter-jump cycling. Reset by a + // timer so a fresh keystroke after a pause starts from the top. + const letterCycleRef = useRef<{ letter: string; lastIdx: number } | null>( + null, + ); + // KR-FE-MULTI-TENANT-COCKPIT-AGGREGATE-AND-DEEPLINK — listen for // open-requests from page-header tenant badges. Lets the badge // open the picker without prop-drilling through Layout. @@ -48,12 +94,133 @@ export function TenantPicker() { return () => window.removeEventListener(OPEN_TENANT_PICKER_EVENT, handler); }, []); + // When picker opens, highlight the currently-active option so + // ↑/↓ start from there. When picker closes, drop highlight + + // return focus to the trigger so keyboard flow continues. + useEffect(() => { + if (open) { + const activeIdx = options.indexOf(activeTenant); + setHighlight(activeIdx >= 0 ? activeIdx : 0); + } else { + setHighlight(-1); + // Defer focus restoration to the next tick — Enter/Esc may + // have just landed on the option button; refocusing the + // trigger synchronously fights React's commit ordering. + const t = window.setTimeout(() => triggerRef.current?.focus(), 0); + return () => window.clearTimeout(t); + } + }, [open, activeTenant, options]); + + // Scroll the highlighted option into view on highlight change + // (large tenant lists may exceed the dropdown's max-h). + useEffect(() => { + if (highlight < 0) return; + const el = optionRefs.current[highlight]; + el?.scrollIntoView({ block: "nearest" }); + }, [highlight]); + + const closePicker = useCallback(() => setOpen(false), []); + const onPick = useCallback( (next: string) => { setActiveTenant(next); - setOpen(false); + closePicker(); + }, + [setActiveTenant, closePicker], + ); + + const onTriggerKeyDown = useCallback( + (e: KeyboardEvent<HTMLButtonElement>) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + setOpen(true); + } else if (e.key === "ArrowDown") { + e.preventDefault(); + setOpen(true); + } }, - [setActiveTenant], + [], + ); + + const cycleLetterJump = useCallback( + (letter: string) => { + const lower = letter.toLowerCase(); + const matches: number[] = []; + options.forEach((id, idx) => { + const label = + id === ALL_TENANTS_SENTINEL ? "all" : id.toLowerCase(); + if (label.startsWith(lower)) matches.push(idx); + }); + if (matches.length === 0) return; + const prev = letterCycleRef.current; + let nextIdx: number; + if (prev && prev.letter === lower) { + // Repeated press on the same letter → cycle to the next + // match (wrap at end). Resolves the dup-prefix case the + // bucket called out in §4 with the preferred "cycle" + // behavior. + const currentPos = matches.indexOf(prev.lastIdx); + const nextPos = currentPos < 0 ? 0 : (currentPos + 1) % matches.length; + nextIdx = matches[nextPos]; + } else { + nextIdx = matches[0]; + } + letterCycleRef.current = { letter: lower, lastIdx: nextIdx }; + setHighlight(nextIdx); + }, + [options], + ); + + const onListKeyDown = useCallback( + (e: KeyboardEvent<HTMLDivElement>) => { + if (e.key === "Escape") { + e.preventDefault(); + closePicker(); + return; + } + if (e.key === "ArrowDown") { + e.preventDefault(); + setHighlight((idx) => (idx + 1) % options.length); + return; + } + if (e.key === "ArrowUp") { + e.preventDefault(); + setHighlight((idx) => + idx <= 0 ? options.length - 1 : idx - 1, + ); + return; + } + if (e.key === "Home") { + e.preventDefault(); + setHighlight(0); + return; + } + if (e.key === "End") { + e.preventDefault(); + setHighlight(options.length - 1); + return; + } + if (e.key === "Enter") { + e.preventDefault(); + if (highlight >= 0 && highlight < options.length) { + onPick(options[highlight]); + } + return; + } + // Letter-jump: any single printable character (length 1, no + // modifier keys). Modifier check avoids hijacking Cmd-A etc. + if ( + e.key.length === 1 && + !e.ctrlKey && + !e.metaKey && + !e.altKey && + /[\w]/.test(e.key) + ) { + e.preventDefault(); + cycleLetterJump(e.key); + } + }, + [closePicker, highlight, onPick, options, cycleLetterJump], ); // A.6: only render when we have observed ≥ 2 tenants. Hides @@ -66,8 +233,10 @@ export function TenantPicker() { return ( <div className="relative px-4 pb-2"> <button + ref={triggerRef} type="button" onClick={() => setOpen((v) => !v)} + onKeyDown={onTriggerKeyDown} aria-haspopup="listbox" aria-expanded={open} aria-label={`Active tenant: ${label}`} @@ -96,28 +265,72 @@ export function TenantPicker() { {open && ( <div + // Combined ref callback: store + auto-focus. Auto-focus + // is what makes ↑/↓/Enter/letter-jump work without a + // second click after open. + ref={(node) => { + listRef.current = node; + node?.focus(); + }} role="listbox" aria-label="Select active tenant" + aria-activedescendant={ + highlight >= 0 ? `tenant-option-${highlight}` : undefined + } + tabIndex={-1} + onKeyDown={onListKeyDown} className={cn( "absolute left-4 right-4 z-50 mt-1", "rounded border border-current/20 bg-popover shadow-lg", "max-h-64 overflow-auto", + "focus-visible:outline-none", )} > - {availableTenants.map((t) => ( + {options.map((id, idx) => ( <TenantOption - key={t} - tenantId={t} - active={t === activeTenant} + key={id} + id={`tenant-option-${idx}`} + tenantId={id} + active={id === activeTenant} + highlighted={idx === highlight} onPick={onPick} + onHover={() => setHighlight(idx)} + optionRef={(node) => { + optionRefs.current[idx] = node; + }} /> ))} - <TenantOption - tenantId={ALL_TENANTS_SENTINEL} - label="All tenants (aggregate)" - active={isAllTenants} - onPick={onPick} - /> + + {/* KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — + opt-in URL-toggle. Persisted via useTenantUrlToggle; + default off. Keeps the picker-vs-URL precedence from + #207 intact for everyone who doesn't opt in. */} + <div className="border-t border-current/10 px-3 py-2"> + <label + className={cn( + "flex items-center gap-2 cursor-pointer", + "text-[10px] uppercase tracking-wide", + "text-muted-foreground hover:text-foreground", + )} + title="When on, picker selections also update the ?tenant URL param" + > + <input + type="checkbox" + checked={urlToggle} + onChange={(e) => setUrlToggle(e.target.checked)} + onKeyDown={(e) => { + // Keep Esc/Tab functional from the checkbox; let + // Space toggle natively (don't preventDefault). + if (e.key === "Escape") { + e.preventDefault(); + closePicker(); + } + }} + className="h-3 w-3" + /> + <span>Also update URL</span> + </label> + </div> </div> )} </div> @@ -125,26 +338,50 @@ export function TenantPicker() { } interface TenantOptionProps { + id: string; tenantId: string; - label?: string; active: boolean; + highlighted: boolean; onPick: (id: string) => void; + onHover: () => void; + optionRef: (node: HTMLButtonElement | null) => void; } -function TenantOption({ tenantId, label, active, onPick }: TenantOptionProps) { +function TenantOption({ + id, + tenantId, + active, + highlighted, + onPick, + onHover, + optionRef, +}: TenantOptionProps) { const display = - label ?? - (tenantId === DEFAULT_TENANT_ID ? `${tenantId} (canonical)` : tenantId); + tenantId === ALL_TENANTS_SENTINEL + ? "All tenants (aggregate)" + : tenantId === DEFAULT_TENANT_ID + ? `${tenantId} (canonical)` + : tenantId; return ( <button + ref={optionRef} + id={id} type="button" role="option" aria-selected={active} + // Mouse pick — keyboard pick goes through onListKeyDown. onClick={() => onPick(tenantId)} + onMouseEnter={onHover} + // Keep the listbox focused on hover so a keyboard nav after + // a stray mouse-over still picks up the right element. + tabIndex={-1} className={cn( "block w-full px-3 py-1.5 text-left text-xs font-mono", "hover:bg-accent/40", - active && "bg-accent/30 text-accent-foreground font-semibold", + // Highlighted state is the keyboard-driven "where would + // Enter land" indicator. Distinct from active (selected). + highlighted && "bg-accent/60", + active && "text-accent-foreground font-semibold", )} > {display} diff --git a/web/src/contexts/PageHeaderProvider.tsx b/web/src/contexts/PageHeaderProvider.tsx index 9fdd6215e343..55485d940a19 100644 --- a/web/src/contexts/PageHeaderProvider.tsx +++ b/web/src/contexts/PageHeaderProvider.tsx @@ -1,9 +1,53 @@ -import { useLayoutEffect, useMemo, useState, type ReactNode } from "react"; +import { + useEffect, + useLayoutEffect, + useMemo, + useState, + type ReactNode, +} from "react"; import { useLocation } from "react-router-dom"; import { PageHeaderContext } from "./page-header-context"; import { resolvePageTitle } from "@/lib/resolve-page-title"; import { cn } from "@/lib/utils"; import { useI18n } from "@/i18n"; +import { + DEFAULT_TENANT_ID, + useActiveTenant, +} from "@/hooks/useActiveTenant"; + +// KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — +// browser-tab title suffix. Mirrors the static <title> in +// web/index.html so per-page document.title updates keep the +// "· Kora" / "Hermes Agent" anchor a screen-reader / tab-switcher +// expects. Single point of change if the brand suffix ever moves. +const TAB_TITLE_SUFFIX = "Hermes Agent" as const; + +/** + * Format a per-page browser-tab title: + * single-tenant or default-active → ``"Probe Investigations · Hermes Agent"`` + * tenant-active → ``"[marvin] Probe Investigations · Hermes Agent"`` + * aggregate-active → ``"[all tenants] Probe Investigations · Hermes Agent"`` + * + * Exported for the drift-guard test so renaming the brand suffix + * or changing the prefix format trips the cross-stack pin. + */ +export function formatBrowserTabTitle( + displayTitle: string, + options: { + activeTenant: string; + isAllTenants: boolean; + isMultiTenant: boolean; + }, +): string { + const base = `${displayTitle} · ${TAB_TITLE_SUFFIX}`; + // Hide prefix on single-tenant deployments (no value — nothing + // to switch to) and on the canonical "default" tenant (every + // operator's idle baseline). + if (!options.isMultiTenant) return base; + if (options.isAllTenants) return `[all tenants] ${base}`; + if (options.activeTenant === DEFAULT_TENANT_ID) return base; + return `[${options.activeTenant}] ${base}`; +} export function PageHeaderProvider({ children, @@ -34,6 +78,22 @@ export function PageHeaderProvider({ ); const displayTitle = titleOverride ?? defaultTitle; + // KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — + // sync document.title to the active page header + active-tenant + // prefix. Lets operators distinguish ProbeInvestigations-for- + // Kora vs ProbeInvestigations-for-Marvin tabs at a glance in + // their browser tab strip. No-op until at least 2 tenants exist + // OR the active tenant is non-default — keeps the original + // static title intact for single-tenant operators. + const { activeTenant, isAllTenants, isMultiTenant } = useActiveTenant(); + useEffect(() => { + document.title = formatBrowserTabTitle(displayTitle, { + activeTenant, + isAllTenants, + isMultiTenant, + }); + }, [displayTitle, activeTenant, isAllTenants, isMultiTenant]); + const isChatRoute = pathname === "/chat" || pathname === "/chat/"; /** Env jump-nav is wide — stack below title on small screens so KEYS stays readable. */ const isEnvRoute = diff --git a/web/src/hooks/useActiveTenant.ts b/web/src/hooks/useActiveTenant.ts index 360d44e1540e..255c89f085ff 100644 --- a/web/src/hooks/useActiveTenant.ts +++ b/web/src/hooks/useActiveTenant.ts @@ -124,6 +124,14 @@ function useResolvedTenant(): [string, (next: string) => void] { const setActiveTenant = useCallback((next: string) => { writeStored(next); setStored(next); + // KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — + // when the operator opted in, also mirror the pick into the URL. + // Read the toggle live (not via the hook) so this stays usable + // from callers outside React (e.g., the badge's share-URL flow + // could trigger this in principle). + if (readUrlToggle()) { + updateUrlTenantParam(next); + } }, []); return [resolved, setActiveTenant]; @@ -151,6 +159,99 @@ export function requestOpenTenantPicker(): void { window.dispatchEvent(new Event(OPEN_TENANT_PICKER_EVENT)); } +// KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — +// opt-in "also update URL when picking a tenant" preference. When +// enabled, picker selections write the ``?tenant=`` query param to +// the current URL (preserving every other param) in addition to +// localStorage. Useful for power-operators who want their browser +// history / open-tab URLs to reflect the active tenant. +// +// Default is off — preserves the #207 picker-vs-URL precedence +// (URL anchors a deep-link; operator picks update only their own +// localStorage and don't mutate a shared link's URL). +export const TENANT_PICKER_URL_TOGGLE_STORAGE_KEY = + "kora_tenant_picker_update_url" as const; + +function readUrlToggle(): boolean { + if (typeof window === "undefined") return false; + try { + return ( + window.localStorage.getItem(TENANT_PICKER_URL_TOGGLE_STORAGE_KEY) === + "1" + ); + } catch { + return false; + } +} + +function writeUrlToggle(enabled: boolean): void { + if (typeof window === "undefined") return; + try { + if (enabled) { + window.localStorage.setItem( + TENANT_PICKER_URL_TOGGLE_STORAGE_KEY, + "1", + ); + } else { + window.localStorage.removeItem(TENANT_PICKER_URL_TOGGLE_STORAGE_KEY); + } + window.dispatchEvent(new Event("kora:tenant-url-toggle-changed")); + } catch { + // Best-effort. + } +} + +/** + * KR-FE-TENANT-PICKER-KEYBOARD-NAV-AND-URL-TOGGLE-AND-TAB-TITLE — + * write ``?tenant=<value>`` to the current URL while preserving + * every other query param. Uses ``history.replaceState`` so the + * change doesn't push a new entry onto the back-stack (operators + * picking through tenants shouldn't pollute history). ``default`` + * removes the param entirely so the URL stays clean. + */ +function updateUrlTenantParam(next: string): void { + if (typeof window === "undefined") return; + try { + const url = new URL(window.location.href); + if (next === DEFAULT_TENANT_ID) { + url.searchParams.delete(TENANT_ID_QUERY_PARAM); + } else { + url.searchParams.set(TENANT_ID_QUERY_PARAM, tenantToUrlValue(next)); + } + window.history.replaceState( + window.history.state, + "", + url.pathname + url.search + url.hash, + ); + } catch { + // history API restrictions (rare) — silent. localStorage + // still updated, so the picker still works as before. + } +} + +/** + * Read + persist the "also update URL" toggle. Listens for in-tab + * + cross-tab changes so the picker checkbox stays in sync if the + * operator changes the setting from another tab. + */ +export function useTenantUrlToggle(): [boolean, (enabled: boolean) => void] { + const [enabled, setEnabled] = useState(() => readUrlToggle()); + useEffect(() => { + const reread = () => setEnabled(readUrlToggle()); + window.addEventListener("storage", reread); + window.addEventListener("kora:tenant-url-toggle-changed", reread); + return () => { + window.removeEventListener("storage", reread); + window.removeEventListener("kora:tenant-url-toggle-changed", reread); + }; + }, []); + const set = useCallback((nextEnabled: boolean) => { + writeUrlToggle(nextEnabled); + setEnabled(nextEnabled); + }, []); + return [enabled, set]; +} + export function useActiveTenant(): UseActiveTenantResult { const [activeTenant, setActiveTenant] = useResolvedTenant(); const [availableTenants, setAvailableTenants] = useState<string[]>([