From 979181875dea9e12d1de3f54e69f5a5001aff17b Mon Sep 17 00:00:00 2001 From: Anso Date: Thu, 28 May 2026 14:16:46 -0400 Subject: [PATCH] fix(sidebar): require admin role for Schedule task and debounce search input (#1243) The right-click Schedule task menu item and its keyboard shortcut were gated only on isPaid, but the backend write routes under /api/scheduled-tasks enforce requireAdmin + requirePaid on every action. Non-admin Skipper or Admiral users would see the menu item and hit a 403 on click. The frontend now mirrors the backend by gating Schedule task on isPaid && isAdmin so the affordance only renders for users whose action will actually succeed. Also adds a 120ms keystroke debounce to the sidebar search input. The useStackListState filter rebuild was previously running on every keystroke because disables cmdk's own filter and the existing 250ms timer only debounces state-invalidate events. Visible input stays immediate via local state; the debounced emit drives the filter pass. Adds a regression guard that /api/stacks/statuses is short-circuited by the remote-node proxy (covers the sidebar status poll path) and updates the sidebar feature docs to reflect the admin role requirement on Schedule task. --- .../src/__tests__/proxy-mount-order.test.ts | 15 +++ docs/features/sidebar.mdx | 5 +- frontend/src/components/EditorLayout.tsx | 1 + .../hooks/useSidebarContextMenu.ts | 9 +- .../src/components/sidebar/SidebarSearch.tsx | 36 +++++++- .../sidebar/__tests__/SidebarSearch.test.tsx | 91 +++++++++++++++++++ .../src/components/sidebar/sidebar-types.ts | 1 + .../__tests__/useStackMenuItems.test.tsx | 7 ++ frontend/src/hooks/useStackMenuItems.tsx | 6 +- 9 files changed, 164 insertions(+), 7 deletions(-) create mode 100644 frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx diff --git a/backend/src/__tests__/proxy-mount-order.test.ts b/backend/src/__tests__/proxy-mount-order.test.ts index 804b5e87..bbbbed9d 100644 --- a/backend/src/__tests__/proxy-mount-order.test.ts +++ b/backend/src/__tests__/proxy-mount-order.test.ts @@ -76,6 +76,21 @@ describe('remote proxy mount order', () => { expect(res.status).not.toBe(502); }); + it('short-circuits /api/stacks/statuses (the sidebar status poll) for remote nodes', async () => { + // The sidebar polls /api/stacks/statuses every few seconds; if a future + // refactor accidentally mounted a local fast-path before the proxy, every + // operator viewing a remote node would silently see the central instance's + // own statuses. Asserts the same 502 short-circuit as /api/stacks. + const res = await request(app) + .get('/api/stacks/statuses') + .set('Authorization', authHeader) + .set('x-node-id', String(remoteNodeId)); + + expect(res.status).toBe(502); + expect(res.headers['x-sencho-proxy']).toBeUndefined(); + expect(res.body?.error).toMatch(/unreachable/i); + }); + it('handles proxy-exempt paths locally even when x-node-id targets a remote', async () => { // /api/nodes/:id is in PROXY_EXEMPT_PREFIXES. The proxy must never catch // gateway-level concerns; otherwise a user whose default node is remote diff --git a/docs/features/sidebar.mdx b/docs/features/sidebar.mdx index e0442f80..f42fc198 100644 --- a/docs/features/sidebar.mdx +++ b/docs/features/sidebar.mdx @@ -85,7 +85,7 @@ Right-click any stack (or open the kebab that appears on hover) for its context - **Destructive**: **Delete**. - **Auto-Heal** and **Schedule task** require a **Skipper** or **Admiral** license. + **Auto-Heal** requires a **Skipper** or **Admiral** license. **Schedule task** requires a **Skipper** or **Admiral** license and an **admin** role. @@ -143,4 +143,7 @@ The footer surfaces the most recent stack lifecycle event on the node. Each tick Click the **+** icon to the right of the search box to bring them back. The chip row collapses to a thin **−** / **+** toggle, and the state is remembered in your browser, so an earlier collapse persists across reloads until you expand it again. + + Scheduling requires a **Skipper** or **Admiral** license and an **admin** role. Ask an admin on this node to schedule the task for you, or sign in with an admin account. + diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index 808925ae..6d885ba7 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -176,6 +176,7 @@ export default function EditorLayout() { activeNode, isPaid, isAdmiral, + isAdmin, can, }); diff --git a/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts b/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts index 5c2756fc..c69da470 100644 --- a/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts +++ b/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts @@ -21,6 +21,7 @@ interface UseSidebarContextMenuOptions { activeNode: Node | null | undefined; isPaid: boolean; isAdmiral: boolean; + isAdmin: boolean; can: (action: PermissionAction, resourceType?: string, resourceId?: string) => boolean; } @@ -32,6 +33,7 @@ export function useSidebarContextMenu({ activeNode, isPaid, isAdmiral, + isAdmin, can, }: UseSidebarContextMenuOptions) { const buildMenuCtx = useCallback((file: string): StackMenuCtx => { @@ -42,6 +44,7 @@ export function useSidebarContextMenu({ isBusy: stackListState.isStackBusy(file), isPaid, isAdmiral, + isAdmin, canDelete: can('stack:delete', 'stack', sName), canEditLabels: can('stack:edit', 'stack', sName), // POST /api/labels (the inline "New label" entry) is guarded by the @@ -121,9 +124,13 @@ export function useSidebarContextMenu({ navState.setActiveView('scheduled-ops'); }, }; + // Handlers from useStackActions, useOverlayState, useViewNavigationState are + // useCallback-stabilized at their owner hooks, so listing the menu surface + // values (status maps, role/tier flags, pin state) is sufficient. Exhaustive + // deps would force a rebuild on every parent render and defeat the memo. // eslint-disable-next-line react-hooks/exhaustive-deps }, [ - stackListState.stackStatuses, stackListState.stackPorts, isPaid, isAdmiral, + stackListState.stackStatuses, stackListState.stackPorts, isPaid, isAdmiral, isAdmin, stackListState.isPinned, stackListState.labels, stackListState.stackLabelMap, stackListState.pin, stackListState.unpin, ]); diff --git a/frontend/src/components/sidebar/SidebarSearch.tsx b/frontend/src/components/sidebar/SidebarSearch.tsx index 993ed6b4..6cf7f4ce 100644 --- a/frontend/src/components/sidebar/SidebarSearch.tsx +++ b/frontend/src/components/sidebar/SidebarSearch.tsx @@ -1,3 +1,4 @@ +import { useEffect, useRef, useState } from 'react'; import { CommandInput } from '@/components/ui/command'; interface SidebarSearchProps { @@ -5,13 +6,44 @@ interface SidebarSearchProps { onValueChange: (v: string) => void; } +// 120ms feels instant to a typist (still under the ~150ms human reaction +// floor) while collapsing a burst of keystrokes into one filter rebuild. +// `` means useStackListState owns the actual +// filter pass; debouncing here directly cuts its rebuild count. +const DEBOUNCE_MS = 120; + export function SidebarSearch({ value, onValueChange }: SidebarSearchProps) { + const [local, setLocal] = useState(value); + const timerRef = useRef | null>(null); + const lastEmittedRef = useRef(value); + + useEffect(() => { + // Parent value can move for two reasons: + // 1. Echo of our own debounced emit (lastEmittedRef matches): skip. + // 2. External reset (e.g., clear-on-filter-change): adopt it. + if (value === lastEmittedRef.current) return; + setLocal(value); + }, [value]); + + useEffect(() => () => { + if (timerRef.current) clearTimeout(timerRef.current); + }, []); + + const handleChange = (next: string) => { + setLocal(next); + if (timerRef.current) clearTimeout(timerRef.current); + timerRef.current = setTimeout(() => { + lastEmittedRef.current = next; + onValueChange(next); + }, DEBOUNCE_MS); + }; + return (
diff --git a/frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx b/frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx new file mode 100644 index 00000000..1ff19592 --- /dev/null +++ b/frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, act, fireEvent, cleanup } from '@testing-library/react'; +import { Command } from '@/components/ui/command'; +import { SidebarSearch } from '../SidebarSearch'; + +function renderInsideCommand(props: { value: string; onValueChange: (v: string) => void }) { + return render( + + + , + ); +} + +describe('SidebarSearch', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + cleanup(); + }); + + it('reflects typing immediately in the input but does not emit until the debounce window closes', () => { + const onValueChange = vi.fn(); + const { getByPlaceholderText } = renderInsideCommand({ value: '', onValueChange }); + const input = getByPlaceholderText('Search stacks...') as HTMLInputElement; + + act(() => { + fireEvent.input(input, { target: { value: 'n' } }); + fireEvent.input(input, { target: { value: 'ng' } }); + fireEvent.input(input, { target: { value: 'ngi' } }); + fireEvent.input(input, { target: { value: 'ngin' } }); + fireEvent.input(input, { target: { value: 'nginx' } }); + }); + + expect(input.value).toBe('nginx'); + expect(onValueChange).not.toHaveBeenCalled(); + + act(() => { + vi.advanceTimersByTime(120); + }); + + expect(onValueChange).toHaveBeenCalledTimes(1); + expect(onValueChange).toHaveBeenLastCalledWith('nginx'); + }); + + it('does not clobber in-flight typing when the parent value echoes back the previous debounced emit', () => { + const onValueChange = vi.fn(); + const { getByPlaceholderText, rerender } = renderInsideCommand({ value: '', onValueChange }); + const input = getByPlaceholderText('Search stacks...') as HTMLInputElement; + + act(() => { + fireEvent.input(input, { target: { value: 'web' } }); + }); + act(() => { + vi.advanceTimersByTime(120); + }); + expect(onValueChange).toHaveBeenLastCalledWith('web'); + + // Parent now propagates 'web' back as the controlled value. + rerender( + + + , + ); + + // User keeps typing before the parent's echo settles. + act(() => { + fireEvent.input(input, { target: { value: 'web-api' } }); + }); + + // The echo of 'web' must not overwrite 'web-api' on the input. + expect(input.value).toBe('web-api'); + }); + + it('adopts an external reset of the parent value (e.g., clear)', () => { + const onValueChange = vi.fn(); + const { getByPlaceholderText, rerender } = renderInsideCommand({ value: 'old-query', onValueChange }); + const input = getByPlaceholderText('Search stacks...') as HTMLInputElement; + expect(input.value).toBe('old-query'); + + rerender( + + + , + ); + + expect(input.value).toBe(''); + }); +}); diff --git a/frontend/src/components/sidebar/sidebar-types.ts b/frontend/src/components/sidebar/sidebar-types.ts index fcaa5961..d9d2c3db 100644 --- a/frontend/src/components/sidebar/sidebar-types.ts +++ b/frontend/src/components/sidebar/sidebar-types.ts @@ -27,6 +27,7 @@ export interface StackMenuCtx { isBusy: boolean; isPaid: boolean; isAdmiral: boolean; + isAdmin: boolean; canDelete: boolean; canEditLabels: boolean; canCreateLabels: boolean; diff --git a/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx b/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx index f72c1edb..d89c0c85 100644 --- a/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx +++ b/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx @@ -11,6 +11,7 @@ function makeCtx(overrides: Partial = {}): StackMenuCtx { isBusy: false, isPaid: true, isAdmiral: false, + isAdmin: true, canDelete: true, canEditLabels: true, canCreateLabels: true, @@ -94,6 +95,12 @@ describe('useStackMenuItems', () => { expect(lifecycle.items.some(i => i.id === 'schedule')).toBe(true); }); + it('hides Schedule task when paid but not admin', () => { + const { result } = renderHook(() => useStackMenuItems('web.yml', makeCtx({ isPaid: true, isAdmin: false }))); + const lifecycle = result.current.find(g => g.id === 'lifecycle'); + expect(lifecycle?.items.some(i => i.id === 'schedule')).toBeFalsy(); + }); + it('keeps label assignment available when !isPaid', () => { const { result } = renderHook(() => useStackMenuItems('web.yml', makeCtx({ isPaid: false, diff --git a/frontend/src/hooks/useStackMenuItems.tsx b/frontend/src/hooks/useStackMenuItems.tsx index 7218ed94..96446f89 100644 --- a/frontend/src/hooks/useStackMenuItems.tsx +++ b/frontend/src/hooks/useStackMenuItems.tsx @@ -18,7 +18,7 @@ import type { MenuGroup, MenuItem, StackMenuCtx } from '@/components/sidebar/sid export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] { const { - stackStatus, hasPort, isBusy, isPaid, canDelete, canEditLabels, isPinned, labels, + stackStatus, hasPort, isBusy, isPaid, isAdmin, canDelete, canEditLabels, isPinned, labels, openAlertSheet, openAutoHeal, checkUpdates, openStackApp, deploy, stop, restart, update, remove, pin, unpin, toggleLabel, menuVisibility, openScheduleTask, @@ -68,7 +68,7 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] if (showStop) lifecycle.push({ id: 'stop', label: 'Stop', icon: Square, shortcut: '⌘.', onSelect: stop, disabled: isBusy }); if (showRestart) lifecycle.push({ id: 'restart', label: 'Restart', icon: RotateCw, shortcut: '⌘R', onSelect: restart, disabled: isBusy }); if (showUpdate) lifecycle.push({ id: 'update', label: 'Update', icon: Download, shortcut: '⌘↑', onSelect: update, disabled: isBusy }); - if (isPaid) lifecycle.push({ id: 'schedule', label: 'Schedule task', icon: CalendarClock, onSelect: openScheduleTask }); + if (isPaid && isAdmin) lifecycle.push({ id: 'schedule', label: 'Schedule task', icon: CalendarClock, onSelect: openScheduleTask }); if (lifecycle.length > 0) groups.push({ id: 'lifecycle', items: lifecycle }); if (canDelete) { @@ -80,7 +80,7 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] return groups; }, [ - stackStatus, hasPort, isBusy, isPaid, canDelete, canEditLabels, isPinned, labels, + stackStatus, hasPort, isBusy, isPaid, isAdmin, canDelete, canEditLabels, isPinned, labels, showDeploy, showStop, showRestart, showUpdate, openAlertSheet, openAutoHeal, checkUpdates, openStackApp, deploy, stop, restart, update, remove, pin, unpin, toggleLabel, openScheduleTask,