From 54e9e8a335db9b1be17648f3bd0f8d89c3e4ac37 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:56:01 +0100 Subject: [PATCH] Keep the In my chats switch and sign in again from cards Tool tiles keep the caller's own In my chats switch, with the same visibility and revert-on-failure as before, but without the label text beside it; screen readers still get its name. A connected tool whose connection needs signing in again has Sign in again in its menu, and a paused synced source has Reconnect on its tile, both for the reader's own connection and in place where the wizard can do it. --- frontend/DESIGN.md | 11 +-- frontend/src/settings/Sources.test.tsx | 89 +++++++++++++++----- frontend/src/settings/Sources.tsx | 49 ++++++++--- frontend/src/settings/Tools.test.tsx | 112 +++++++++++++++++++++---- frontend/src/settings/Tools.tsx | 109 ++++++++++++++++++++---- 5 files changed, 303 insertions(+), 67 deletions(-) diff --git a/frontend/DESIGN.md b/frontend/DESIGN.md index 0ea49eb7..aece8ffc 100644 --- a/frontend/DESIGN.md +++ b/frontend/DESIGN.md @@ -647,11 +647,12 @@ description use a Switch in a SettingRow instead. ### Switch, TimePicker and Calendar (`ui/switch.tsx`, `ui/time-picker.tsx`, `ui/calendar.tsx`) `Switch` (Radix) is an on/off setting, placed inside a `SettingRow` that names -it. A tool row's own on/off in a connection's drawer ("In my chats") is the -one exception: a `Label text-muted-foreground text-xs font-normal` + `Switch` -pair at the row's end. It is the caller's own preference, never a switch that -turns the tool off for everyone. The Tools page's tiles carry no switch; the -chat's Tools picker is where a tool goes in or out of the caller's chats. The track is `primary` when on and `bg-input` when off, with a white +it. A tool's own "In my chats" on/off is the one exception. On a Tools page +tile it is a bare `Switch` at the tile's bottom-right, named only by its +`aria-label` (no visible label); in a connection's drawer, a tool row ends in +a `Label text-muted-foreground text-xs font-normal` + `Switch` pair. Either way +it is the caller's own preference, never a switch that turns the tool off for +everyone. The track is `primary` when on and `bg-input` when off, with a white thumb in both themes (see Elevation). `TimePicker` picks a time of day as two `SelectTrigger`s, hours and minutes, diff --git a/frontend/src/settings/Sources.test.tsx b/frontend/src/settings/Sources.test.tsx index b8d511ee..b2249849 100644 --- a/frontend/src/settings/Sources.test.tsx +++ b/frontend/src/settings/Sources.test.tsx @@ -2,25 +2,27 @@ import { act, useState } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { MemoryRouter, useLocation } from 'react-router-dom'; -const { dispatch, service, view, connectors, uploadProps } = vi.hoisted(() => ({ - uploadProps: vi.fn(), - connectors: { connections: [] as Record[] }, - // The heavy children: each view reports the canEdit it was given. - view: - (testId: string) => - ({ canEdit }: { canEdit?: boolean }) => ( -
- ), - dispatch: vi.fn(), - service: { - getConfig: vi.fn(), - manageSync: vi.fn(), - syncSource: vi.fn(), - syncConnector: vi.fn(), - reingestSource: vi.fn(), - getDirectoryStructure: vi.fn(), - }, -})); +const { dispatch, service, view, connectors, uploadProps, reconnect } = + vi.hoisted(() => ({ + reconnect: vi.fn(), + uploadProps: vi.fn(), + connectors: { connections: [] as Record[] }, + // The heavy children: each view reports the canEdit it was given. + view: + (testId: string) => + ({ canEdit }: { canEdit?: boolean }) => ( +
+ ), + dispatch: vi.fn(), + service: { + getConfig: vi.fn(), + manageSync: vi.fn(), + syncSource: vi.fn(), + syncConnector: vi.fn(), + reingestSource: vi.fn(), + getDirectoryStructure: vi.fn(), + }, + })); vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }), @@ -71,6 +73,9 @@ vi.mock('./WikiSettingsModal', () => ({ })); vi.mock('./EnableGraphRAGModal', () => ({ default: () => null })); vi.mock('../teams/ShareToTeamModal', () => ({ default: () => null })); +vi.mock('../connectors/SignInAgainNotice', () => ({ + useSignInAgain: () => ({ reconnect, modals: null }), +})); vi.mock('../upload/Upload', () => ({ default: (props: unknown) => { uploadProps(props); @@ -210,6 +215,52 @@ describe('Sources access', () => { expect(items).toContain('convTile.delete'); }); + // A source whose connection needs signing in again says so on its tile + // and signs in again from there, without opening the source. + it('reconnects a paused synced source from its tile', async () => { + connectors.connections = [ + { + id: 'conn-1', + connector_key: 'google_drive', + name: 'Google Drive', + icon: 'drive', + status: 'reconnect_needed', + account_label: 'alex@example.com', + }, + ]; + reconnect.mockClear(); + await render(doc({ connectionId: 'conn-1' } as Partial)); + connectors.connections = []; + const button = Array.from(container.querySelectorAll('button')).find( + (b) => b.textContent === 'settings.connectors.status.reconnect', + )!; + expect(button).toBeDefined(); + await act(async () => button.click()); + expect(reconnect).toHaveBeenCalledWith( + expect.objectContaining({ id: 'conn-1', connector_key: 'google_drive' }), + ); + // Only the reconnect: the source view stays closed. + expect(container.querySelector('[data-testid="chunks"]')).toBeNull(); + }); + + it('offers no Reconnect on a synced source that is running', async () => { + connectors.connections = [ + { + id: 'conn-1', + connector_key: 'google_drive', + name: 'Google Drive', + status: 'connected', + }, + ]; + await render(doc({ connectionId: 'conn-1' } as Partial)); + connectors.connections = []; + expect( + Array.from(container.querySelectorAll('button')).some( + (b) => b.textContent === 'settings.connectors.status.reconnect', + ), + ).toBe(false); + }); + // Leaving Knowledge loses nothing, so its Add knowledge may browse the // whole Connectors page; the other openers keep the list in the dialog. it('lets Add knowledge browse the syncing connectors from Knowledge', async () => { diff --git a/frontend/src/settings/Sources.tsx b/frontend/src/settings/Sources.tsx index 08bbe018..c3b836c6 100644 --- a/frontend/src/settings/Sources.tsx +++ b/frontend/src/settings/Sources.tsx @@ -58,6 +58,7 @@ import { formatDate } from '../utils/dateTimeUtils'; import FileTree from '../components/FileTree'; import ConnectorTree from '../components/ConnectorTree'; import ConnectorIcon from '../connectors/ConnectorIcon'; +import { useSignInAgain } from '../connectors/SignInAgainNotice'; import { loadConnectors, selectConnections, @@ -100,6 +101,8 @@ export default function Sources({ const token = useSelector(selectToken); const uploadTasks = useSelector(selectUploadTasks); const connections = useSelector(selectConnections); + // Signing in again reloads the connections, which lifts the pause. + const { reconnect, modals: signInModals } = useSignInAgain(); const connectorsLoaded = useSelector(selectConnectorsLoaded); useEffect(() => { @@ -744,19 +747,37 @@ export default function Sources({
{connection && paused && ( - - - - {t('settings.connectors.detail.paused')} - - - - {t('settings.sources.paused', { - name: connection.name, - interpolation: { escapeValue: false }, - })} - - +
+ + + + {t('settings.connectors.detail.paused')} + + + + {t('settings.sources.paused', { + name: connection.name, + interpolation: { escapeValue: false }, + })} + + + {/* The reader's own connection (only theirs are + loaded): sign in again right here, without + opening the source. */} + +
)} {document.ingestStatus === 'failed' && ( @@ -871,6 +892,8 @@ export default function Sources({ /> )} + {signInModals} + {deleteModalState === 'ACTIVE' && documentToDelete && ( ({ })); vi.mock('../api/services/devicesService', () => ({ default: {} })); +const reconnect = vi.fn(); +vi.mock('../connectors/SignInAgainNotice', () => ({ + useSignInAgain: () => ({ reconnect, modals: null }), +})); + const getUserTools = vi.fn(); const updateToolStatus = vi.fn(); vi.mock('../api/services/userService', () => ({ @@ -198,6 +203,8 @@ describe('Tools', () => { Array.from(card(id).querySelectorAll('[data-testid="menu"] button')).map( (b) => b.textContent, ); + const switchOf = (id: string) => + card(id).querySelector('[role="switch"]')!; it('shows Edit, Reconnect, Share and Delete to the owner', async () => { await render([ownTool]); @@ -259,13 +266,53 @@ describe('Tools', () => { }); }); - // Whether a tool is in the caller's own chats is set from the chat's - // Tools picker; the page's cards carry no switch for it. - it('puts no In my chats switch on any card', async () => { - await render([ownTool, editorTool, viewerTool]); - expect(container.querySelector('[role="switch"]')).toBeNull(); + // The caller's own "In my chats" switch stays on the tile, named for + // screen readers only: the tile shows no label text beside it. + it('keeps the In my chats switch, bound to in_chat, without its label text', async () => { + await render([ownTool, editorTool]); + const sw = switchOf('ed'); + expect(sw.getAttribute('aria-checked')).toBe('false'); + expect(switchOf('own').getAttribute('aria-checked')).toBe('true'); + expect(sw.getAttribute('aria-label')).toBe( + 'settings.tools.useInMyChatsAria:{"toolName":"ed"}', + ); + expect(card('ed').querySelector(`label[for="${sw.id}"]`)).toBeNull(); expect(container.textContent).not.toContain('settings.tools.inMyChats'); - expect(updateToolStatus).not.toHaveBeenCalled(); + }); + + // The tool can't be in the caller's chats at all (the composer picker + // hides it too), so there is no state to show: no switch, no bare grey one. + it('hides the switch for a shared tool without use_in_own', async () => { + await render([viewerTool]); + expect(card('vw').querySelector('[role="switch"]')).toBeNull(); + }); + + it('reverts the switch and shows an error toast when the update fails', async () => { + await render([editorTool]); + updateToolStatus.mockImplementation(() => + jsonResponse({ success: false, message: 'Forbidden' }, false, 403), + ); + await act(async () => switchOf('ed').click()); + expect(updateToolStatus).toHaveBeenCalledWith( + { id: 'ed', status: true }, + 'token', + ); + expect(switchOf('ed').getAttribute('aria-checked')).toBe('false'); + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + payload: expect.objectContaining({ variant: 'destructive' }), + }), + ); + }); + + it('keeps the new value when the update succeeds', async () => { + await render([editorTool]); + // Mounting loads the connectors; only a toast would come after it. + dispatch.mockClear(); + updateToolStatus.mockImplementation(() => jsonResponse({ success: true })); + await act(async () => switchOf('ed').click()); + expect(switchOf('ed').getAttribute('aria-checked')).toBe('true'); + expect(dispatch).not.toHaveBeenCalled(); }); it('shows the role on a shared tile as a neutral Users Badge', async () => { @@ -401,11 +448,11 @@ describe('Tools page connections', () => { it('shows each connected account as its own tool, the account on its footer', async () => { await renderTools(root); const telegram = card('Telegram'); - expect(telegram.querySelector('[role="switch"]')).toBeNull(); - // Which account this card is, and the catalog's plain description. - expect( - telegram.querySelector('[data-slot="card-footer"]')?.textContent, - ).toBe('Alerts bot'); + // Which account this card is beside its own switch, and the catalog's + // plain description. + const footer = telegram.querySelector('[data-slot="card-footer"]')!; + expect(footer.textContent).toBe('Alerts bot'); + expect(footer.querySelector('[role="switch"]')).not.toBeNull(); expect(telegram.textContent).toContain( 'settings.connectors.descriptions.telegram', ); @@ -420,13 +467,48 @@ describe('Tools page connections', () => { expect(menuItem('Linear', 'settings.tools.reconnect')).toBeUndefined(); }); + // The owner signs in again right from the card; an MCP preset re-signs + // its existing tool. + it('offers the owner Sign in again on a connected tool that needs it', async () => { + reconnect.mockClear(); + await renderTools(root); + expect( + menuItem('Telegram', 'settings.connectors.health.signInAgain'), + ).toBeUndefined(); + await act(async () => + menuItem('Linear', 'settings.connectors.health.signInAgain')!.click(), + ); + expect(reconnect).toHaveBeenCalledWith( + expect.objectContaining({ id: 'conn-2', connector_key: 'mcp:linear' }), + 'lin', + ); + }); + + it("offers no Sign in again on a teammate's connected tool", async () => { + getUserTools.mockImplementation(() => + jsonResponse({ + tools: [ + { + ...TOOLS[2], + customName: 'Linear (shared)', + connection_id: 'owner-conn', + access: 'editor', + ownership: 'team', + allowed_actions: ['edit', 'use', 'use_in_own'], + }, + ], + }), + ); + await renderTools(root); + expect( + menuItem('Linear (shared)', 'settings.connectors.health.signInAgain'), + ).toBeUndefined(); + }); + it('keeps Edit for a tool that is not from a connection', async () => { await renderTools(root); expect(menuItem('My API', 'settings.tools.edit')).toBeDefined(); - // No account line and nothing else for the footer: no footer. - expect( - card('My API').querySelector('[data-slot="card-footer"]'), - ).toBeNull(); + expect(card('My API').querySelector('[role="switch"]')).not.toBeNull(); }); it("gives a viewer of a teammate's connected tool View", async () => { diff --git a/frontend/src/settings/Tools.tsx b/frontend/src/settings/Tools.tsx index 46c78bbf..a68b23fe 100644 --- a/frontend/src/settings/Tools.tsx +++ b/frontend/src/settings/Tools.tsx @@ -20,6 +20,7 @@ import { CardTitle, } from '../components/ui/card'; import { ActionMenu, type MenuOption } from '../components/ui/dropdown-menu'; +import { Switch } from '../components/ui/switch'; import { EmptyState } from '../components/ui/empty-state'; import ConnectorIcon from '../connectors/ConnectorIcon'; import { @@ -30,6 +31,7 @@ import { } from '../connectors/connectorsSlice'; import { connectorDescription } from '../connectors/i18n'; import type { Connection } from '../connectors/types'; +import { useSignInAgain } from '../connectors/SignInAgainNotice'; import { toolServiceOf } from '../connectors/toolService'; import { useLoaderState } from '../hooks'; import type { AvailableToolType } from '../modals/types'; @@ -44,7 +46,11 @@ import ShareToTeamModal, { type ShareCredentials, } from '../teams/ShareToTeamModal'; import { can, isOwner, roleOf } from '../utils/accessUtils'; -import { isSharedOAuthMcp } from '../utils/toolUtils'; +import { + canAddToolToOwn, + isSharedOAuthMcp, + toolInChat, +} from '../utils/toolUtils'; import RemoteDeviceConfig from './RemoteDeviceConfig'; import ToolConfig from './ToolConfig'; import { APIToolType, UserToolType } from './types'; @@ -175,6 +181,17 @@ export default function Tools() { const getMenuOptions = (tool: UserToolType): MenuOption[] => { const canEdit = can(tool, 'edit') || can(tool, 'edit_credentials'); const options: MenuOption[] = []; + // The caller's own connection that needs signing in again (the badge + // says so) is signed in again from here, in place where it can be. + const connection = connectionOf(tool); + if (connection && connectionNeedsSignIn(connection)) + options.push({ + icon: RefreshCw, + label: t('settings.connectors.health.signInAgain'), + onClick: () => + reconnect(connection, tool.name === 'mcp_tool' ? tool.id : undefined), + variant: 'default', + }); // The owner's connected tool (its connection is theirs) is managed on // the Connectors page, so it has no editor here; a teammate's opens the // tool editor like any other shared tool. @@ -243,6 +260,10 @@ export default function Tools() { .catch(() => {}); }, [token]); + const { reconnect, modals: signInModals } = useSignInAgain({ + onConnected: () => getUserTools(), + }); + const getUserTools = () => { setLoading(true); userService @@ -268,6 +289,40 @@ export default function Tools() { }); }; + const setToolInChat = (toolId: string, value: boolean) => + setUserTools((prevTools) => + prevTools.map((tool) => + tool.id !== toolId + ? tool + : isOwner(tool) + ? { ...tool, status: value, in_chat: value } + : { ...tool, in_chat: value }, + ), + ); + + // The switch moves at once and flips back when the server refuses it. + const updateToolStatus = (toolId: string, newStatus: boolean) => { + setToolInChat(toolId, newStatus); + const fail = () => { + setToolInChat(toolId, !newStatus); + dispatch( + showActionToast({ + variant: 'destructive', + message: t('settings.tools.statusUpdateFailed'), + }), + ); + }; + userService + .updateToolStatus({ id: toolId, status: newStatus }, token) + .then((response: Response) => { + if (!response.ok) fail(); + }) + .catch((error: unknown) => { + console.error('Failed to update tool status:', error); + fail(); + }); + }; + // The caller's own connection behind a connected tool (only their own // connections are loaded): the card names its account. const connectionOf = (tool: UserToolType) => @@ -528,22 +583,45 @@ export default function Tools() {
- {/* Which account this is: each account of a - service is its own tool. */} - {connection && ( + {/* Which account this is (each account of a + service is its own tool) and the caller's own + "In my chats" switch, named for screen readers + only, share the meta row. A shared tool without + use_in_own can't be in the caller's chats at + all, so it has no switch. */} + {(connection || canAddToolToOwn(tool)) && ( - - - - {accountLine(connection)} + {connection && ( + + + + {accountLine(connection)} + - + )} + {canAddToolToOwn(tool) && ( + + updateToolStatus(tool.id, checked) + } + aria-label={t( + 'settings.tools.useInMyChatsAria', + { + interpolation: { escapeValue: false }, + toolName: + tool.customName || tool.displayName, + }, + )} + /> + )} )} @@ -574,6 +652,7 @@ export default function Tools() { submitLabel={t('settings.tools.delete')} variant="destructive" /> + {signInModals}