diff --git a/frontend/src/v2/__tests__/V2AgentListingRow.test.tsx b/frontend/src/v2/__tests__/V2AgentListingRow.test.tsx index 82c99621d..092bbe79c 100644 --- a/frontend/src/v2/__tests__/V2AgentListingRow.test.tsx +++ b/frontend/src/v2/__tests__/V2AgentListingRow.test.tsx @@ -132,4 +132,17 @@ describe('V2AgentListingRow — registry entry owner controls (TASK-419)', () => expect(await screen.findByRole('alert')).toHaveTextContent("Couldn't update this listing. Try again."); expect(retireButton()).toBeInTheDocument(); }); + + test('after a failed save the listing toggle still closes its panel', async () => { + axios.get.mockResolvedValue(entry('owner-1')); + axios.patch.mockRejectedValue(new Error('500')); + renderSection('owner-1'); + await openOwnerControls(); + fireEvent.click(await screen.findByRole('button', { name: 'Retire listing' })); + expect(await screen.findByRole('alert')).toBeInTheDocument(); + const toggle = screen.getByRole('button', { name: 'Manage listing' }); + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + expect(screen.queryByRole('alert')).toBeNull(); + }); }); diff --git a/frontend/src/v2/__tests__/V2AgentRuntimeGrants.test.tsx b/frontend/src/v2/__tests__/V2AgentRuntimeGrants.test.tsx index a65637d4b..77e8e5567 100644 --- a/frontend/src/v2/__tests__/V2AgentRuntimeGrants.test.tsx +++ b/frontend/src/v2/__tests__/V2AgentRuntimeGrants.test.tsx @@ -239,6 +239,8 @@ test('grants this viewer may not see read as owner-only, with no alert and no re expect(screen.queryByText('Could not load connector grants.')).toBeNull(); expect(screen.queryByText('None')).toBeNull(); expect(screen.queryByRole('button', { name: 'Try again' })).toBeNull(); + // Nothing to list, so no toggle that would open an empty panel. + expect(screen.queryByRole('button', { name: 'Connector grants' })).toBeNull(); }); test('a failed grant load is one row with Try again, which reloads the list', async () => { @@ -256,6 +258,41 @@ test('a failed grant load is one row with Try again, which reloads the list', as fireEvent.click(within(grantsSection).getByRole('button', { name: 'Try again' })); expect(await within(grantsSection).findByText(/Work · Granted by Owner/)).toBeInTheDocument(); expect(within(grantsSection).queryByRole('alert')).toBeNull(); + // Try again is gone once the list loads, so focus lands on the row's toggle, not the page. + await waitFor(() => expect(screen.getByRole('button', { name: 'Connector grants' })).toHaveFocus()); +}); + +test('a revoke whose reload is forbidden leaves no grant rows or revoke controls behind', async () => { + let forbid = false; + axios.get.mockImplementation((url) => { + if (url.includes('/agent-binding/placement')) return Promise.resolve({ data: { boundMachineId: null, requestedMachineId: null, boundMachineRevoked: false, activeMachines: [] } }); + if (url.includes('/grants') && forbid) return Promise.reject({ response: { status: 403, data: { error: 'Forbidden' } } }); + return Promise.resolve({ data: response }); + }); + renderSection(); + openGrants(); + expect(await screen.findByText(/Work · Granted by Owner/)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Revoke' })); + forbid = true; + fireEvent.click(screen.getByRole('button', { name: 'Yes, revoke it' })); + expect(await screen.findByText('Owner and admins only')).toBeInTheDocument(); + expect(screen.queryByText(/Work · Granted by Owner/)).toBeNull(); + expect(screen.queryByRole('button', { name: /Revoke/ })).toBeNull(); + expect(screen.queryByRole('region', { name: 'Connector grants' })).toBeNull(); +}); + +test('a failed grant load opens the list, and its toggle still closes it', async () => { + axios.get.mockImplementation((url) => { + if (url.includes('/agent-binding/placement')) return Promise.resolve({ data: { boundMachineId: null, requestedMachineId: null, boundMachineRevoked: false, activeMachines: [] } }); + if (url.includes('/grants')) return Promise.reject(new Error('offline')); + return Promise.resolve({ data: response }); + }); + renderSection(); + expect(await screen.findByRole('region', { name: 'Connector grants' })).toBeInTheDocument(); + const toggle = screen.getByRole('button', { name: 'Connector grants' }); + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + expect(screen.queryByRole('region', { name: 'Connector grants' })).toBeNull(); }); test('a failed revoke says so above the rows, keeps the grant revocable and offers no reload', async () => { diff --git a/frontend/src/v2/__tests__/V2AgentSettingsPage.test.tsx b/frontend/src/v2/__tests__/V2AgentSettingsPage.test.tsx index 9e6b1e6d4..9869217bc 100644 --- a/frontend/src/v2/__tests__/V2AgentSettingsPage.test.tsx +++ b/frontend/src/v2/__tests__/V2AgentSettingsPage.test.tsx @@ -773,7 +773,12 @@ describe('one Identity & access list (d215 frame)', () => { renderPage(); expect(await screen.findByText('Connected · 1 pod')).toBeInTheDocument(); await waitFor(() => expect(document.getElementById('agent-skills-panel')).not.toHaveAttribute('hidden')); - expect(screen.getByRole('button', { name: 'Skills' })).toHaveAttribute('aria-expanded', 'true'); + const toggle = screen.getByRole('button', { name: 'Skills' }); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + // Opened by the error, but still the reader's to close. + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + expect(document.getElementById('agent-skills-panel')).toHaveAttribute('hidden'); }); test('a healthy skills read leaves the Skills panel closed', async () => { @@ -826,16 +831,38 @@ describe('one Identity & access list (d215 frame)', () => { fireEvent.click(screen.getByRole('button', { name: /replace the running agent/i })); expect(await screen.findByTestId('agent-one-time-key')).toHaveTextContent('cm_agent_once'); const toggle = screen.getByRole('button', { name: 'Key actions' }); - fireEvent.click(toggle); - // Closing the toggle cannot hide a key that is shown only once. - expect(screen.getByTestId('agent-one-time-key')).toBeInTheDocument(); + // A key shown only once can't be folded away, and the toggle says so rather than doing nothing. + expect(toggle).toBeDisabled(); expect(toggle).toHaveAttribute('aria-expanded', 'true'); fireEvent.click(screen.getByRole('button', { name: 'Done' })); expect(screen.queryByTestId('agent-one-time-key')).toBeNull(); expect(screen.queryByText('Runtime key · Laptop')).toBeNull(); + // Done closes the panel and hands focus back to the row, not to the page. + expect(toggle).toBeEnabled(); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + expect(toggle).toHaveFocus(); }); - test('a key error forces the panel open', async () => { + test('a key that arrives after the panel was closed mid-replace still opens it', async () => { + axios.get.mockImplementation(fleet()); + let resolvePost: (value: unknown) => void = () => {}; + axios.post.mockReturnValue(new Promise((resolve) => { resolvePost = resolve; })); + renderPage(); + await openKeys(); + fireEvent.click(screen.getByRole('button', { name: 'Regenerate key' })); + fireEvent.click(screen.getByRole('button', { name: /replace the running agent/i })); + const toggle = screen.getByRole('button', { name: 'Key actions' }); + // The reader folds the panel while the replace is still in flight. + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + await act(async () => { resolvePost({ data: { token: 'cm_agent_late' } }); }); + // The old key is already gone, so the new one must be in view, with Done the way out. + expect(await screen.findByTestId('agent-one-time-key')).toHaveTextContent('cm_agent_late'); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + expect(toggle).toBeDisabled(); + }); + + test('a key error opens the panel, and its toggle still closes it', async () => { axios.get.mockImplementation(fleet()); axios.post.mockRejectedValue({ response: { data: { error: 'Replacement refused' } } }); renderPage(); @@ -843,7 +870,11 @@ describe('one Identity & access list (d215 frame)', () => { fireEvent.click(screen.getByRole('button', { name: 'Regenerate key' })); fireEvent.click(screen.getByRole('button', { name: /replace the running agent/i })); expect(await screen.findByText('Replacement refused')).toBeInTheDocument(); - fireEvent.click(screen.getByRole('button', { name: 'Key actions' })); + const toggle = screen.getByRole('button', { name: 'Key actions' }); + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + expect(screen.queryByText('Replacement refused')).toBeNull(); + fireEvent.click(toggle); expect(screen.getByText('Replacement refused')).toBeInTheDocument(); }); diff --git a/frontend/src/v2/__tests__/useOpensOnDemand.test.tsx b/frontend/src/v2/__tests__/useOpensOnDemand.test.tsx new file mode 100644 index 000000000..172258f48 --- /dev/null +++ b/frontend/src/v2/__tests__/useOpensOnDemand.test.tsx @@ -0,0 +1,31 @@ +import { renderHook, act } from '@testing-library/react'; +import { useOpensOnDemand } from '../hooks/useOpensOnDemand'; + +type Demands = { a: unknown; b?: unknown }; +const render = (initial: Demands) => renderHook(({ a, b }: Demands) => useOpensOnDemand(a, b), { initialProps: initial }); + +describe('useOpensOnDemand', () => { + it('starts open when a demand already holds, closed otherwise', () => { + expect(render({ a: 'error' }).result.current[0]).toBe(true); + expect(render({ a: null }).result.current[0]).toBe(false); + }); + + it('stays closed while the demand holds after the reader closes it, and reopens when it rises again', () => { + const { result, rerender } = render({ a: null }); + rerender({ a: 'error' }); + expect(result.current[0]).toBe(true); + act(() => result.current[1](false)); + rerender({ a: 'error' }); + expect(result.current[0]).toBe(false); + rerender({ a: null }); + rerender({ a: 'again' }); + expect(result.current[0]).toBe(true); + }); + + it('a second demand rising opens the panel even while the first still holds', () => { + const { result, rerender } = render({ a: 'confirm', b: null }); + act(() => result.current[1](false)); + rerender({ a: 'confirm', b: 'key' }); + expect(result.current[0]).toBe(true); + }); +}); diff --git a/frontend/src/v2/agents/V2AgentListingRow.tsx b/frontend/src/v2/agents/V2AgentListingRow.tsx index 27668454f..f72cfc39e 100644 --- a/frontend/src/v2/agents/V2AgentListingRow.tsx +++ b/frontend/src/v2/agents/V2AgentListingRow.tsx @@ -4,6 +4,7 @@ import { useTranslation } from 'react-i18next'; import axios from '../../utils/axiosConfig'; import { useAuth } from '../../context/AuthContext'; import V2AgentSettingsIcon from './V2AgentSettingsIcon'; +import { useOpensOnDemand } from '../hooks/useOpensOnDemand'; interface Props { agentName: string; @@ -35,7 +36,8 @@ const V2AgentListingRow: React.FC = ({ agentName, instanceId }) => { const [listingName, setListingName] = useState(''); const [listingBusy, setListingBusy] = useState(false); const [listingError, setListingError] = useState(null); - const [open, setOpen] = useState(false); + // A failed save opens the panel where its message is. + const [open, setOpen] = useOpensOnDemand(listingError); useEffect(() => { let active = true; @@ -74,7 +76,7 @@ const V2AgentListingRow: React.FC = ({ agentName, instanceId }) => { } }; const panelId = `agent-listing-owner-${instanceId}`; - const panelOpen = ownsListing && (open || Boolean(listingError)); + const panelOpen = ownsListing && open; return <>
{t('agentSettings.listingTitle')} diff --git a/frontend/src/v2/agents/V2AgentSettingsPage.tsx b/frontend/src/v2/agents/V2AgentSettingsPage.tsx index 1db5f3dfe..3169a19d2 100644 --- a/frontend/src/v2/agents/V2AgentSettingsPage.tsx +++ b/frontend/src/v2/agents/V2AgentSettingsPage.tsx @@ -7,6 +7,7 @@ import V2AgentRuntimeBody, { Runtime, V2AgentConnectorsRow, describeRuntime } fr import { AgentControlLoader } from '../components/V2AgentControlSection'; import { AgentConnection, connectionLabel } from '../utils/agentConnection'; import { usePhone } from '../connectors/usePhone'; +import { useOpensOnDemand } from '../hooks/useOpensOnDemand'; import { localizeRelativeTime } from '../utils/localizeRelativeTime'; import { useAgentSettings } from './useAgentSettings'; import { useTalkToAgent } from './useTalkToAgent'; @@ -60,9 +61,20 @@ const V2AgentSettingsPage: React.FC = () => { const [oneTimeKey, setOneTimeKey] = useState(null); const [connection, setConnection] = useState(null); const connectionState = connection?.state ?? null; - const [keyOpen, setKeyOpen] = useState(false); - const [skillsOpen, setSkillsOpen] = useState(false); + // The key panel opens by itself for anything the owner must see or answer; a failed skills read opens + // its panel, where the error is. Either way the row's toggle keeps working. + const [keyOpen, setKeyOpen] = useOpensOnDemand(oneTimeKey, pendingAction, error); + const keyToggleRef = useRef(null); + // Set by Done; the toggle is disabled until the key clears, so focus moves after that render. + const refocusKeyToggle = useRef(false); + useEffect(() => { + if (oneTimeKey || !refocusKeyToggle.current) return; + refocusKeyToggle.current = false; + keyToggleRef.current?.focus(); + }, [oneTimeKey]); + // undefined while the skills panel loads, null if its read failed. const [skillCount, setSkillCount] = useState(undefined); + const [skillsOpen, setSkillsOpen] = useOpensOnDemand(skillCount === null); const [runsNeedAttention, setRunsNeedAttention] = useState(false); const runsRef = useRef(null); // Open How it runs when its body starts needing action, and leave closing to the person, @@ -301,10 +313,6 @@ const V2AgentSettingsPage: React.FC = () => { const displayName = access.displayName || agentName; const newestUse = activeKeys.map((key) => key.lastUsedAt).filter((value): value is string => Boolean(value)).sort().pop(); - // The key panel stays open whenever it holds something the owner must see or answer. - const keyPanelOpen = keyOpen || Boolean(oneTimeKey || pendingAction || error); - // A failed skills read opens its panel, where the error is. - const skillsShown = skillsOpen || skillCount === null; const runtimeDetails = describeRuntime(runtime); const runtimeSummary = [runtimeDetails.adapter || runtime.adapter || runtime.runtimeType, runtime.model].filter(Boolean).join(' · '); const processState = runtime.processConnected === true ? 'connected' : runtime.processConnected === false ? 'disconnected' : 'notReported'; @@ -374,13 +382,15 @@ const V2AgentSettingsPage: React.FC = () => { {!keysForbidden && ( )}
- {keyPanelOpen &&
+ {keyOpen &&
{error &&

{error}

} {!keysForbidden && (installation?.runtime?.runtimeType === 'wrapper' ? (

{t('agentSettings.wrapperReconnect')}

@@ -404,7 +414,12 @@ const V2AgentSettingsPage: React.FC = () => {
{t('agentSettings.copyNow')} {oneTimeKey} - +
)} {!keysForbidden && ( @@ -445,13 +460,13 @@ const V2AgentSettingsPage: React.FC = () => {
{/* Mounted while closed, so the row can count what the panel loads. */} - ; const activeGrants = grants.filter((grant) => !grantEnded(grant)).length; const endedGrants = grants.length - activeGrants; - // A failed load or revoke, or a pending confirm, keeps the list open so its message and action stay in view. - const panelOpen = open || Boolean(loadError || revokeError || confirmGrantId); const panelId = `agent-grants-${instanceId}`; const title = t('agentManage.grants.title', { defaultValue: 'Connector grants' }); @@ -278,14 +282,14 @@ export const V2AgentConnectorsRow: React.FC = ({ name, instance - + }
- {panelOpen &&
- {loadError && alertRow(loadError, 'muted', )} + {open && !grantsPrivate &&
+ {loadError && alertRow(loadError, 'muted', )} {loading && !grants.length &&
{t('agentManage.grants.loading', { defaultValue: 'Loading grants…' })}
} diff --git a/frontend/src/v2/hooks/useOpensOnDemand.ts b/frontend/src/v2/hooks/useOpensOnDemand.ts new file mode 100644 index 000000000..a9c163e68 --- /dev/null +++ b/frontend/src/v2/hooks/useOpensOnDemand.ts @@ -0,0 +1,21 @@ +import { useLayoutEffect, useRef, useState } from 'react'; +import type { Dispatch, SetStateAction } from 'react'; + +// A disclosure that opens by itself when something needs the reader (an error, a +// key to copy, a confirm) and stays a working toggle afterwards. Rendering it as +// `open || demand` instead pins it open, so its toggle does nothing for as long +// as the demand holds. +// +// Each demand opens the panel on its own rising edge, not on the OR of them: a +// key that arrives while a confirm already held the panel open, after the reader +// closed it, must still open it. The layout effect opens it before paint. +export const useOpensOnDemand = (...demands: unknown[]): [boolean, Dispatch>] => { + const [open, setOpen] = useState(() => demands.some(Boolean)); + const previous = useRef(demands.map(Boolean)); + useLayoutEffect(() => { + const now = demands.map(Boolean); + if (now.some((demand, index) => demand && !previous.current[index])) setOpen(true); + previous.current = now; + }); + return [open, setOpen]; +};