diff --git a/app/src/components/chat/IntegrationConnectCard.tsx b/app/src/components/chat/IntegrationConnectCard.tsx new file mode 100644 index 000000000..c1677fa81 --- /dev/null +++ b/app/src/components/chat/IntegrationConnectCard.tsx @@ -0,0 +1,378 @@ +import debug from 'debug'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; + +import { authorize, listConnections } from '../../lib/composio/composioApi'; +import { canonicalizeComposioToolkitSlug } from '../../lib/composio/toolkitSlug'; +import { deriveComposioState } from '../../lib/composio/types'; +import { useT } from '../../lib/i18n/I18nContext'; +import { callCoreRpc } from '../../services/coreRpcClient'; +import { clearPendingApprovalForThread, type PendingApproval } from '../../store/chatRuntimeSlice'; +import { useAppDispatch } from '../../store/hooks'; +import { openUrl } from '../../utils/openUrl'; +import { + getRequiredFieldsForToolkit, + validateRequiredFieldValues, +} from '../composio/toolkitRequiredFields'; +import Button from '../ui/Button'; + +/** + * Inline OAuth connect card (#3993). + * + * Rendered in place of {@link ApprovalRequestCard} when the agent calls the + * `composio_connect` tool — that tool parks on the same ApprovalGate, so the + * request arrives over the identical `approval_request` socket path, but the + * surface is a **Connect** button rather than approve/deny. Clicking it runs + * `composio_authorize`, opens the OAuth handoff in the browser, and polls + * `composio_list_connections` until the toolkit flips to ACTIVE — at which + * point it resolves the parked tool call with `approve_once` so the agent + * resumes in the same turn. Cancel (or the gate's 10-minute TTL) resolves it + * as `deny`. + * + * Provider-specific required fields (WhatsApp `waba_id`, Jira `subdomain`, + * Dynamics 365 `org_name`) are collected inline from the + * [`toolkitRequiredFields`] registry before the OAuth handoff — so even + * field-gated toolkits connect entirely in-chat rather than failing with a + * raw `ConnectedAccount_MissingRequiredFields` (code 612) error. Mirrors the + * polling + field-collection contract of `ComposioConnectModal` so the two + * connect surfaces behave identically. + */ +const log = debug('openhuman:chat:integration-connect-card'); + +const POLL_INTERVAL_MS = 4_000; +const POLL_TIMEOUT_MS = 5 * 60 * 1_000; + +/** + * Composio error slug for missing required fields (code 612). Defensive + * recovery path: if the backend starts requiring a field the registry hasn't + * caught up on, surface a clear message instead of a dead retry loop. + */ +const MISSING_REQUIRED_FIELDS_SLUG = 'ConnectedAccount_MissingRequiredFields'; + +type Phase = 'idle' | 'connecting' | 'error'; + +interface Props { + threadId: string; + approval: PendingApproval; +} + +function errorText(e: unknown): string { + return e instanceof Error ? e.message : String(e); +} + +export const IntegrationConnectCard: React.FC = ({ threadId, approval }) => { + const { t } = useT(); + const dispatch = useAppDispatch(); + // Canonicalize the slug the agent supplied (e.g. `google_drive` → + // `googledrive`) so authorize / list-connections hit the form Composio's + // backend expects (#3993). Defensive: the core already canonicalizes too. + const toolkit = canonicalizeComposioToolkitSlug(approval.toolkit ?? ''); + + const [phase, setPhase] = useState('idle'); + const [errorMsg, setErrorMsg] = useState(null); + // Cleared to false on a permanent backend rejection (no auth config, unknown + // toolkit, 400) so the card drops its Retry affordance — retrying won't help. + const [retryable, setRetryable] = useState(true); + + // Provider-specific required fields are sourced from the declarative + // registry — no per-toolkit branches here (mirrors ComposioConnectModal). + const requiredFields = useMemo(() => getRequiredFieldsForToolkit(toolkit), [toolkit]); + const [fieldValues, setFieldValues] = useState>({}); + const [fieldErrors, setFieldErrors] = useState>({}); + + const pollTimerRef = useRef(null); + const pollDeadlineRef = useRef(0); + const isPollingRef = useRef(false); + const inFlightRef = useRef(false); + // Set once the card is dismissed (Deny) or unmounted, so an `authorize()` + // call still in flight doesn't open OAuth / start polling afterwards. + const cancelledRef = useRef(false); + + const stopPolling = useCallback(() => { + isPollingRef.current = false; + if (pollTimerRef.current != null) { + window.clearTimeout(pollTimerRef.current); + pollTimerRef.current = null; + } + }, []); + + // Stop polling if the card unmounts (turn ended, thread switched, decided), + // and mark cancelled so any in-flight authorize continuation aborts. + useEffect( + () => () => { + cancelledRef.current = true; + stopPolling(); + }, + [stopPolling] + ); + + // Resolve the parked `composio_connect` tool call. `approve_once` once the + // connection is live; `deny` when the user cancels. Clears the card on a + // successful decide — ChatRuntimeProvider also clears on turn end. + const resolveGate = useCallback( + async (decision: 'approve_once' | 'deny') => { + try { + await callCoreRpc({ + method: 'openhuman.approval_decide', + params: { request_id: approval.requestId, decision }, + }); + } catch (e) { + // The backend request is still parked. Clearing the card here would + // drop the only surface that can retry/deny it, blocking the thread + // until the gate TTL expires — so keep the card mounted and surface + // the failure instead of clearing it (#4062, coderabbit review). + log('approval_decide(%s) failed: %o', decision, e); + setPhase('error'); + setErrorMsg(t('chat.approval.error')); + return; + } + dispatch(clearPendingApprovalForThread({ threadId })); + }, + [approval.requestId, dispatch, threadId, t] + ); + + const startPolling = useCallback(() => { + stopPolling(); + isPollingRef.current = true; + pollDeadlineRef.current = Date.now() + POLL_TIMEOUT_MS; + + const scheduleNext = () => { + if (!isPollingRef.current) return; + pollTimerRef.current = window.setTimeout(() => void tick(), POLL_INTERVAL_MS); + }; + + const tick = async () => { + if (inFlightRef.current || !isPollingRef.current) return; + if (Date.now() > pollDeadlineRef.current) { + stopPolling(); + setPhase('error'); + setErrorMsg(t('composio.connect.oauthTimeout')); + // Resolve the parked tool call now so the agent resumes immediately + // instead of blocking until the 10-min gate TTL (#3993). The agent + // relays the timeout and the user can ask to connect again. + await resolveGate('deny'); + return; + } + inFlightRef.current = true; + try { + const resp = await listConnections(); + // Scan ALL rows for this toolkit — list_connections returns every row + // (failed / pending / multiple accounts), so the freshly-authorized + // ACTIVE row can sit behind an older FAILED or pending one (#3993, + // codex review). Approve if any row is connected. + const matches = resp.connections.filter( + c => c.toolkit.toLowerCase() === toolkit.toLowerCase() + ); + if (matches.some(c => deriveComposioState(c) === 'connected')) { + stopPolling(); + await resolveGate('approve_once'); + return; + } + // Keep waiting while any handoff is still in flight; only surface an + // error once a failed row exists and nothing is pending. + const pending = matches.some(c => deriveComposioState(c) === 'pending'); + const errored = matches.find(c => deriveComposioState(c) === 'error'); + if (errored && !pending) { + stopPolling(); + setPhase('error'); + setErrorMsg( + t('composio.connect.connectionFailed').replace('{status}', String(errored.status)) + ); + return; + } + } catch (err) { + // Transient poll failures are expected mid-handoff — retry next tick. + log('connection poll failed: %o', err); + } finally { + inFlightRef.current = false; + } + scheduleNext(); + }; + + void tick(); + }, [resolveGate, stopPolling, t, toolkit]); + + const connect = useCallback(async () => { + if (phase === 'connecting' || !toolkit) return; + // A prior Deny (or a failed decide that kept the card mounted) may have set + // cancelledRef — clear it so this fresh, user-initiated attempt isn't + // aborted by the post-authorize cancellation guard. + cancelledRef.current = false; + + // Collect + validate provider-specific required fields before the OAuth + // handoff so field-gated toolkits don't hit a 612 error mid-flow. + let extraParams: Record | undefined; + if (requiredFields.length > 0) { + const errors = validateRequiredFieldValues(requiredFields, fieldValues); + if (Object.keys(errors).length > 0) { + setFieldErrors(errors); + return; + } + setFieldErrors({}); + extraParams = {}; + for (const f of requiredFields) { + extraParams[f.key] = (fieldValues[f.key] ?? '').trim(); + } + } + + setPhase('connecting'); + setErrorMsg(null); + setRetryable(true); + try { + const resp = await authorize(toolkit, extraParams); + // The user may have hit Deny / dismissed the card while authorize was in + // flight — abort so we don't open OAuth or start polling after the gate + // was already denied, nor race a second approval_decide (codex review). + if (cancelledRef.current) return; + try { + await openUrl(resp.connectUrl); + } catch (openErr) { + // Opening the browser failed, but the handoff may still be reachable; + // keep polling and let the user reopen if needed. + log('openUrl failed: %o', openErr); + } + startPolling(); + } catch (e) { + log('authorize failed: %o', e); + setPhase('error'); + // Defensive: backend reports a required field the registry lacks. Without + // a field definition we can't collect it inline, so surface a clear + // "needs extra setup" message rather than a retry that will fail again. + if (errorText(e).includes(MISSING_REQUIRED_FIELDS_SLUG) && requiredFields.length === 0) { + setErrorMsg(t('composio.connect.additionalConfigRequired')); + } else { + // Surface the backend's actual reason (e.g. "toolkit not in allowlist") + // so a failed connect is diagnosable — a bare "Connection failed." hides + // whether the toolkit is unsupported, mis-slugged, or a transient error. + // Composio authorize errors are connection diagnostics (no PII); bound + // the length and collapse whitespace before showing. + // Strip the "(status: {status})" clause locale-robustly — the English + // literal differs per locale (de "Status", fr "statut", …), so match the + // parenthetical containing the {status} token rather than the English + // wording, else non-English users see a literal "{status}" (#3993). + const base = t('composio.connect.connectionFailed') + .replace(/\s*\([^)]*\{status\}[^)]*\)/, '') + .trim(); + const reason = errorText(e).replace(/\s+/g, ' ').trim().slice(0, 240); + setErrorMsg(reason ? `${base} ${reason}` : base); + // Permanent backend rejections won't change on retry — drop the Retry + // affordance so the user isn't looped on a doomed connect (#3993). + if (/no auth config|not a valid toolkit|unknown toolkit|not found|\b400\b/i.test(reason)) { + setRetryable(false); + } + } + } + }, [phase, requiredFields, fieldValues, startPolling, t, toolkit]); + + const cancel = useCallback(async () => { + cancelledRef.current = true; + stopPolling(); + await resolveGate('deny'); + }, [resolveGate, stopPolling]); + + const connecting = phase === 'connecting'; + const showFields = requiredFields.length > 0 && !connecting; + + return ( +
+
+ + 🔗 + +
+

+ {approval.message || t('chat.approval.fallback')} +

+ + {showFields && ( +
+ {requiredFields.map(field => ( + + ))} +
+ )} + + {connecting && ( +

+ + {t('composio.connect.waitingHint')} +

+ )} + +

+ {t('chat.approval.tool')}{' '} + + {approval.toolName} + +

+ + {errorMsg && ( +

⚠ {errorMsg}

+ )} + +
+ {/* Hide Connect/Retry on a permanent rejection — retrying a toolkit + the backend can't authorize just loops. Dismiss stays. */} + {!(phase === 'error' && !retryable) && ( + + )} + +
+
+
+
+ ); +}; + +export default IntegrationConnectCard; diff --git a/app/src/components/chat/__tests__/IntegrationConnectCard.test.tsx b/app/src/components/chat/__tests__/IntegrationConnectCard.test.tsx new file mode 100644 index 000000000..71cae8a24 --- /dev/null +++ b/app/src/components/chat/__tests__/IntegrationConnectCard.test.tsx @@ -0,0 +1,340 @@ +import { configureStore } from '@reduxjs/toolkit'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { authorize, listConnections } from '../../../lib/composio/composioApi'; +import { deriveComposioState } from '../../../lib/composio/types'; +import { callCoreRpc } from '../../../services/coreRpcClient'; +import chatRuntimeReducer, { + type PendingApproval, + setPendingApprovalForThread, +} from '../../../store/chatRuntimeSlice'; +import { openUrl } from '../../../utils/openUrl'; +import IntegrationConnectCard from '../IntegrationConnectCard'; + +vi.mock('../../../services/coreRpcClient', () => ({ callCoreRpc: vi.fn() })); +vi.mock('../../../utils/openUrl', () => ({ openUrl: vi.fn() })); +vi.mock('../../../lib/composio/composioApi', () => ({ + authorize: vi.fn(), + listConnections: vi.fn(), +})); +vi.mock('../../../lib/composio/types', () => ({ deriveComposioState: vi.fn() })); + +const THREAD = 't1'; +const approval: PendingApproval = { + requestId: 'req-connect-1', + toolName: 'composio_connect', + message: 'Connect gmail to complete your task', + toolkit: 'gmail', +}; + +function renderCard() { + const store = configureStore({ reducer: { chatRuntime: chatRuntimeReducer } }); + store.dispatch(setPendingApprovalForThread({ threadId: THREAD, approval })); + const utils = render( + + + + ); + return { store, ...utils }; +} + +describe('IntegrationConnectCard', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('renders the connect prompt, Connect button, and tool name', () => { + renderCard(); + // The action message leads the card (no alarming "Approval needed" title). + expect(screen.getByText('Connect gmail to complete your task')).toBeInTheDocument(); + expect(screen.getByText('Connect')).toBeInTheDocument(); + expect(screen.getByText('composio_connect')).toBeInTheDocument(); + }); + + it('Connect authorizes, opens the OAuth url, polls, and resolves approve_once on active', async () => { + vi.mocked(authorize).mockResolvedValueOnce({ + connectUrl: 'https://hosted.composio.dev/abc', + connectionId: 'conn-1', + } as Awaited>); + vi.mocked(listConnections).mockResolvedValue({ + connections: [{ toolkit: 'gmail', status: 'ACTIVE' }], + } as Awaited>); + // First poll tick sees the toolkit ACTIVE. + vi.mocked(deriveComposioState).mockReturnValue('connected'); + vi.mocked(callCoreRpc).mockResolvedValue({}); + + const { store } = renderCard(); + fireEvent.click(screen.getByText('Connect')); + + // No required fields for gmail → authorize called with no extra params. + await waitFor(() => expect(authorize).toHaveBeenCalledWith('gmail', undefined)); + await waitFor(() => expect(openUrl).toHaveBeenCalledWith('https://hosted.composio.dev/abc')); + // Polling detected the live connection → parked tool call resolved as approved. + await waitFor(() => + expect(callCoreRpc).toHaveBeenCalledWith({ + method: 'openhuman.approval_decide', + params: { request_id: 'req-connect-1', decision: 'approve_once' }, + }) + ); + await waitFor(() => + expect(store.getState().chatRuntime.pendingApprovalByThread[THREAD]).toBeUndefined() + ); + }); + + it('Cancel resolves the parked call as deny', async () => { + vi.mocked(callCoreRpc).mockResolvedValueOnce({}); + const { store } = renderCard(); + + fireEvent.click(screen.getByText('Deny')); + + expect(callCoreRpc).toHaveBeenCalledWith({ + method: 'openhuman.approval_decide', + params: { request_id: 'req-connect-1', decision: 'deny' }, + }); + await waitFor(() => + expect(store.getState().chatRuntime.pendingApprovalByThread[THREAD]).toBeUndefined() + ); + }); + + it('keeps the card mounted and surfaces an error when approval_decide fails', async () => { + // The decide RPC throws — the backend request is still parked, so clearing + // the card would strand the thread until the gate TTL expires. The card + // must stay (so the user can retry/deny) and surface the failure (#4062). + vi.mocked(callCoreRpc).mockRejectedValueOnce(new Error('rpc down')); + const { store } = renderCard(); + + fireEvent.click(screen.getByText('Deny')); + + await waitFor(() => + expect(callCoreRpc).toHaveBeenCalledWith({ + method: 'openhuman.approval_decide', + params: { request_id: 'req-connect-1', decision: 'deny' }, + }) + ); + // The parked approval survives the failed decide — not cleared. + expect(store.getState().chatRuntime.pendingApprovalByThread[THREAD]).toBeDefined(); + // The failure is shown rather than silently swallowed. + await waitFor(() => + expect(screen.getByText(/Could not record your decision/)).toBeInTheDocument() + ); + }); + + it('collects required fields inline before authorizing (whatsapp waba_id)', async () => { + vi.mocked(authorize).mockResolvedValue({ + connectUrl: 'https://hosted.composio.dev/wa', + connectionId: 'conn-wa', + } as Awaited>); + // Not connected yet — poll keeps waiting; we only assert the authorize args. + vi.mocked(listConnections).mockResolvedValue({ connections: [] } as Awaited< + ReturnType + >); + + const waApproval: PendingApproval = { + requestId: 'req-wa', + toolName: 'composio_connect', + message: 'Connect whatsapp to complete your task', + toolkit: 'whatsapp', + }; + const store = configureStore({ reducer: { chatRuntime: chatRuntimeReducer } }); + store.dispatch(setPendingApprovalForThread({ threadId: THREAD, approval: waApproval })); + render( + + + + ); + + // The required field is rendered inline. + expect(screen.getByText('WhatsApp Business Account ID (WABA ID)')).toBeInTheDocument(); + + // Connecting without filling it blocks authorize and shows a field error. + fireEvent.click(screen.getByText('Connect')); + expect(authorize).not.toHaveBeenCalled(); + expect(screen.getByText('This field is required.')).toBeInTheDocument(); + + // Filling it forwards the value as an extra_param to authorize. + fireEvent.change(screen.getByRole('textbox'), { target: { value: '123456789012345' } }); + fireEvent.click(screen.getByText('Connect')); + await waitFor(() => + expect(authorize).toHaveBeenCalledWith('whatsapp', { waba_id: '123456789012345' }) + ); + }); + + it('canonicalizes the toolkit slug before authorizing (google_drive → googledrive)', async () => { + vi.mocked(authorize).mockResolvedValueOnce({ + connectUrl: 'https://hosted.composio.dev/gd', + connectionId: 'conn-gd', + } as Awaited>); + vi.mocked(listConnections).mockResolvedValue({ connections: [] } as Awaited< + ReturnType + >); + + const gdApproval: PendingApproval = { + requestId: 'req-gd', + toolName: 'composio_connect', + message: 'Connect googledrive to complete your task', + toolkit: 'google_drive', + }; + const store = configureStore({ reducer: { chatRuntime: chatRuntimeReducer } }); + store.dispatch(setPendingApprovalForThread({ threadId: THREAD, approval: gdApproval })); + render( + + + + ); + + fireEvent.click(screen.getByText('Connect')); + // The card hits the canonical Composio slug, not the agent's guess. + await waitFor(() => expect(authorize).toHaveBeenCalledWith('googledrive', undefined)); + }); + + it('shows an error and a Retry affordance when authorize fails', async () => { + vi.mocked(authorize).mockRejectedValueOnce(new Error('backend down')); + renderCard(); + + fireEvent.click(screen.getByText('Connect')); + + // Raw error text is not surfaced; the localized connection-failed string is. + await waitFor(() => expect(screen.getByText('Retry connection')).toBeInTheDocument()); + // The parked call is NOT resolved on a local authorize failure — the user + // can retry without the agent giving up. + expect(callCoreRpc).not.toHaveBeenCalled(); + }); + + it('surfaces the backend reason and drops Retry on a permanent rejection', async () => { + vi.mocked(authorize).mockRejectedValueOnce( + new Error( + '[composio] authorize failed: Backend returned 400 Bad Request: No auth config found for toolkit "googledrive"' + ) + ); + renderCard(); + + fireEvent.click(screen.getByText('Connect')); + + // The actual backend reason is shown (diagnosable), not a bare "failed". + await waitFor(() => expect(screen.getByText(/No auth config found/)).toBeInTheDocument()); + // Retry is gone (it would loop); only Dismiss remains. + expect(screen.queryByText('Retry connection')).not.toBeInTheDocument(); + expect(screen.getByText('Deny')).toBeInTheDocument(); + }); + + it('surfaces the status when polling finds an errored connection', async () => { + vi.mocked(authorize).mockResolvedValueOnce({ + connectUrl: 'https://hosted.composio.dev/e', + connectionId: 'conn-e', + } as Awaited>); + vi.mocked(listConnections).mockResolvedValue({ + connections: [{ toolkit: 'gmail', status: 'FAILED' }], + } as Awaited>); + vi.mocked(deriveComposioState).mockReturnValue('error'); + + renderCard(); + fireEvent.click(screen.getByText('Connect')); + + await waitFor(() => expect(screen.getByText(/Connection failed/)).toBeInTheDocument()); + // A poll-detected error leaves the card for the user to retry/dismiss; + // it does NOT auto-resolve the gate. + expect(callCoreRpc).not.toHaveBeenCalled(); + }); + + it('shows "additional config required" when authorize reports missing fields for a field-less toolkit', async () => { + vi.mocked(authorize).mockRejectedValueOnce( + new Error('400: ConnectedAccount_MissingRequiredFields') + ); + renderCard(); // gmail has no entry in the required-fields registry + + fireEvent.click(screen.getByText('Connect')); + + // Error line renders as "⚠ {msg}", so match the substring. + await waitFor(() => expect(screen.getByText(/Additional config required/)).toBeInTheDocument()); + }); + + it('approves when any matching row is ACTIVE even behind a stale FAILED row', async () => { + vi.mocked(authorize).mockResolvedValueOnce({ + connectUrl: 'https://hosted.composio.dev/m', + connectionId: 'conn-m', + } as Awaited>); + // First row is an old FAILED handoff; the freshly-authorized row is ACTIVE. + vi.mocked(listConnections).mockResolvedValue({ + connections: [ + { toolkit: 'gmail', status: 'FAILED' }, + { toolkit: 'gmail', status: 'ACTIVE' }, + ], + } as Awaited>); + vi.mocked(deriveComposioState).mockImplementation((c?: { status: string }) => + c?.status === 'ACTIVE' ? 'connected' : 'error' + ); + vi.mocked(callCoreRpc).mockResolvedValue({}); + + renderCard(); + fireEvent.click(screen.getByText('Connect')); + + // The ACTIVE row wins over the stale FAILED row → approve. + await waitFor(() => + expect(callCoreRpc).toHaveBeenCalledWith({ + method: 'openhuman.approval_decide', + params: { request_id: 'req-connect-1', decision: 'approve_once' }, + }) + ); + }); + + it('aborts the authorize continuation if the card is dismissed mid-flight', async () => { + let resolveAuthorize!: (v: Awaited>) => void; + vi.mocked(authorize).mockReturnValueOnce( + new Promise(resolve => { + resolveAuthorize = resolve; + }) + ); + vi.mocked(callCoreRpc).mockResolvedValue({}); + + renderCard(); + fireEvent.click(screen.getByText('Connect')); + // Deny while authorize is still in flight. + fireEvent.click(screen.getByText('Deny')); + await waitFor(() => + expect(callCoreRpc).toHaveBeenCalledWith({ + method: 'openhuman.approval_decide', + params: { request_id: 'req-connect-1', decision: 'deny' }, + }) + ); + + // authorize finally resolves — the continuation must NOT open OAuth. + resolveAuthorize({ + connectUrl: 'https://hosted.composio.dev/x', + connectionId: 'conn-x', + } as Awaited>); + await Promise.resolve(); + await Promise.resolve(); + expect(openUrl).not.toHaveBeenCalled(); + }); + + it('resolves the gate as deny when the OAuth poll times out', async () => { + vi.useFakeTimers(); + try { + vi.mocked(authorize).mockResolvedValueOnce({ + connectUrl: 'https://hosted.composio.dev/t', + connectionId: 'conn-t', + } as Awaited>); + // Never connects → poll runs until the 5-min deadline. + vi.mocked(listConnections).mockResolvedValue({ connections: [] } as Awaited< + ReturnType + >); + vi.mocked(callCoreRpc).mockResolvedValue({}); + + renderCard(); + fireEvent.click(screen.getByText('Connect')); + + // Flush authorize + run polling past the 5-min deadline. + await vi.advanceTimersByTimeAsync(5 * 60 * 1000 + 5000); + + // Timeout resolves the parked tool call as deny so the agent resumes. + expect(callCoreRpc).toHaveBeenCalledWith({ + method: 'openhuman.approval_decide', + params: { request_id: 'req-connect-1', decision: 'deny' }, + }); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/app/src/components/composio/ComposioConnectModal.tsx b/app/src/components/composio/ComposioConnectModal.tsx index b11a47f72..046ed6976 100644 --- a/app/src/components/composio/ComposioConnectModal.tsx +++ b/app/src/components/composio/ComposioConnectModal.tsx @@ -1022,7 +1022,7 @@ function RequiredFieldsForm({ value={value} autoFocus={autoFocusFirst && idx === 0} onChange={(e: ChangeEvent) => onChange(field.key, e.target.value)} - placeholder={field.placeholder} + placeholder={field.placeholderKey ? t(field.placeholderKey) : undefined} aria-describedby={hintId} aria-invalid={!!errorText} className="flex-1 min-w-0 px-3 py-2 text-sm text-stone-900 dark:text-neutral-100 placeholder:text-stone-400 dark:placeholder:text-neutral-500 bg-transparent focus:outline-none" diff --git a/app/src/components/composio/toolkitRequiredFields.test.ts b/app/src/components/composio/toolkitRequiredFields.test.ts index 410505bb9..83a617b59 100644 --- a/app/src/components/composio/toolkitRequiredFields.test.ts +++ b/app/src/components/composio/toolkitRequiredFields.test.ts @@ -12,7 +12,7 @@ describe('toolkitRequiredFields registry', () => { expect(fields).toHaveLength(1); expect(fields[0].key).toBe('org_name'); expect(fields[0].suffix).toBe('.crm.dynamics.com'); - expect(fields[0].placeholder).toBe('myorg'); + expect(fields[0].placeholderKey).toBe('composio.connect.dynamicsOrgNamePlaceholder'); }); it('exposes the Jira subdomain field with the .atlassian.net suffix', () => { diff --git a/app/src/components/composio/toolkitRequiredFields.ts b/app/src/components/composio/toolkitRequiredFields.ts index c9566dbda..01946e572 100644 --- a/app/src/components/composio/toolkitRequiredFields.ts +++ b/app/src/components/composio/toolkitRequiredFields.ts @@ -32,8 +32,12 @@ export interface ToolkitRequiredField { labelKey: TranslationKey; /** Optional i18n key for the hint paragraph rendered below the input. */ hintKey?: TranslationKey; - /** Optional placeholder text shown when the input is empty. */ - placeholder?: string; + /** + * Optional i18n key for the placeholder shown when the input is empty. + * Keyed (not raw text) so the placeholder stays inside the i18n pipeline + * like `labelKey` / `hintKey` — all UI text goes through `useT()`. + */ + placeholderKey?: TranslationKey; /** * Optional fixed suffix rendered inside the input (e.g. `.atlassian.net`). * Purely cosmetic — never included in the submitted value. @@ -73,7 +77,7 @@ export const TOOLKIT_REQUIRED_FIELDS: Readonly = { }; export function canonicalizeComposioToolkitSlug(slug: string): string { - const key = slug.toLowerCase(); + // `.trim()` keeps this in sync with the Rust `canonicalize_toolkit_slug` + // (src/openhuman/composio/tools.rs) so a stray-whitespace slug can't diverge. + const key = slug.trim().toLowerCase(); return TOOLKIT_ALIASES[key] ?? key; } diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 8ff44f514..59068537e 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -2939,6 +2939,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'احصل عليه عبر GET /me/businesses ثم GET /{business_id}/owned_whatsapp_business_accounts باستخدام رمز وصول Meta الخاص بك.', 'composio.connect.wabaIdLabel': 'تسمية معرف WABA', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'يرجى إدخال معرف حساب WhatsApp Business (WABA ID) للمتابعة.', 'composio.connect.waitingFor': 'بانتظار', 'composio.connect.waitingHint': 'تلميح الانتظار', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index a6eade320..524f8322c 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -3004,6 +3004,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'আপনার Meta অ্যাক্সেস টোকেন ব্যবহার করে GET /me/businesses তারপর GET /{business_id}/owned_whatsapp_business_accounts এর মাধ্যমে এটি খুঁজে পান।', 'composio.connect.wabaIdLabel': 'WABA ID লেবেল', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'চালিয়ে যেতে আপনার WhatsApp Business Account ID (WABA ID) দিন।', 'composio.connect.waitingFor': 'অপেক্ষা করছে', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 3b94e1bb4..319dc784e 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -3075,6 +3075,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'Finde es über GET /me/businesses und dann GET /{business_id}/owned_whatsapp_business_accounts mit deinem Meta-Zugriffstoken.', 'composio.connect.wabaIdLabel': 'Waba-ID-Etikett', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Bitte gib deine WhatsApp Geschäftskonto-ID (WABA ID) ein, um fortzufahren.', 'composio.connect.waitingFor': 'Warten auf', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index fc8708ae2..7c696ce66 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -3566,6 +3566,9 @@ const en: TranslationMap = { 'composio.connect.wabaIdHint': 'Find it via GET /me/businesses then GET /{business_id}/owned_whatsapp_business_accounts using your Meta access token.', 'composio.connect.wabaIdLabel': 'WhatsApp Business Account ID (WABA ID)', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Please enter your WhatsApp Business Account ID (WABA ID) to continue.', 'composio.connect.waitingFor': 'Waiting for', diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index bb8bd2ed4..e5e7c7735 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -3053,6 +3053,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'Encuéntralo mediante GET /me/businesses y luego GET /{business_id}/owned_whatsapp_business_accounts usando tu token de acceso de Meta.', 'composio.connect.wabaIdLabel': 'Etiqueta de ID de WABA', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Ingresa tu ID de cuenta de WhatsApp Business (WABA ID) para continuar.', 'composio.connect.waitingFor': 'Esperando a', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index 05679e30a..710a3dd26 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -3068,6 +3068,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': "Trouve-le via GET /me/businesses puis GET /{business_id}/owned_whatsapp_business_accounts en utilisant ton jeton d'accès Meta.", 'composio.connect.wabaIdLabel': "Libellé de l'identifiant WABA", + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Saisis ton identifiant WhatsApp Business Account (WABA ID) pour continuer.', 'composio.connect.waitingFor': 'En attente de', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index a771258a1..9c16843a2 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -3004,6 +3004,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'अपने Meta एक्सेस टोकन का उपयोग करके GET /me/businesses फिर GET /{business_id}/owned_whatsapp_business_accounts के माध्यम से इसे प्राप्त करें।', 'composio.connect.wabaIdLabel': 'Waba आईडी लेबल', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'जारी रखने के लिए अपना WhatsApp Business Account ID (WABA ID) डालें।', 'composio.connect.waitingFor': 'प्रतीक्षा कर रहा है', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 9b1c16574..1bee6e442 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -3007,6 +3007,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'Temukan melalui GET /me/businesses lalu GET /{business_id}/owned_whatsapp_business_accounts menggunakan token akses Meta Anda.', 'composio.connect.wabaIdLabel': 'Label ID WABA', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Masukkan ID Akun Bisnis WhatsApp (WABA ID) Anda untuk melanjutkan.', 'composio.connect.waitingFor': 'Menunggu', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index ee42f92f9..2ce02735b 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -3047,6 +3047,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'Trovalo tramite GET /me/businesses poi GET /{business_id}/owned_whatsapp_business_accounts usando il tuo token di accesso Meta.', 'composio.connect.wabaIdLabel': 'Etichetta WABA ID', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Inserisci il tuo ID WhatsApp Business Account (WABA ID) per continuare.', 'composio.connect.waitingFor': 'In attesa di', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index 83f2473f6..b8a3e51e6 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -2977,6 +2977,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'GET /me/businesses를 통해 찾은 다음 Meta 액세스 토큰을 사용하여 /{business_id}/owned_whatsapp_business_accounts를 GET하세요.', 'composio.connect.wabaIdLabel': 'WABA ID 라벨', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': '계속하려면 WhatsApp Business Account ID (WABA ID)를 입력하세요.', 'composio.connect.waitingFor': '대기 중:', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index a5e4e771b..ee271494c 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -3034,6 +3034,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'Znajdziesz je przez GET /me/businesses, a następnie GET /{business_id}/owned_whatsapp_business_accounts z tokenem dostępu Meta.', 'composio.connect.wabaIdLabel': 'ID WhatsApp Business Account', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Wprowadź swoje ID WhatsApp Business Account (WABA ID), aby kontynuować.', 'composio.connect.waitingFor': 'Oczekiwanie na', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index d84cd1b8b..aedb7496d 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -3052,6 +3052,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'Encontre-o via GET /me/businesses depois GET /{business_id}/owned_whatsapp_business_accounts usando seu token de acesso do Meta.', 'composio.connect.wabaIdLabel': 'Rótulo de ID WABA', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Por favor, insira seu ID de Conta Empresarial do WhatsApp (WABA ID) para continuar.', 'composio.connect.waitingFor': 'Aguardando', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index f02fe40b3..6c82da9cc 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -3026,6 +3026,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': 'Найдите его через GET /me/businesses, затем GET /{business_id}/owned_whatsapp_business_accounts, используя ваш токен доступа Meta.', 'composio.connect.wabaIdLabel': 'ID аккаунта WhatsApp Business', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': 'Введи ID аккаунта WhatsApp Business (WABA ID) для продолжения.', 'composio.connect.waitingFor': 'Ожидание', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index 5b1a9d896..61d2202d9 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -2857,6 +2857,9 @@ const messages: TranslationMap = { 'composio.connect.wabaIdHint': '通过 Meta 访问令牌调用 GET /me/businesses,然后 GET /{business_id}/owned_whatsapp_business_accounts 获取。', 'composio.connect.wabaIdLabel': 'WhatsApp 企业账户 ID', + 'composio.connect.atlassianSubdomainPlaceholder': 'your-subdomain', + 'composio.connect.dynamicsOrgNamePlaceholder': 'myorg', + 'composio.connect.wabaIdPlaceholder': 'e.g. 123456789012345', 'composio.connect.wabaIdRequired': '请输入你的 WhatsApp 企业账户 ID(WABA ID)以继续。', 'composio.connect.waitingFor': '等待中', 'composio.connect.waitingHint': '请在浏览器中完成授权', diff --git a/app/src/pages/Conversations.tsx b/app/src/pages/Conversations.tsx index 9e64deb25..b88214c78 100644 --- a/app/src/pages/Conversations.tsx +++ b/app/src/pages/Conversations.tsx @@ -11,6 +11,7 @@ import ChatComposer from '../components/chat/ChatComposer'; import ChatFilesChip from '../components/chat/ChatFilesChip'; import ChatNewWindowHero from '../components/chat/ChatNewWindowHero'; import ComposerTokenStats from '../components/chat/ComposerTokenStats'; +import IntegrationConnectCard from '../components/chat/IntegrationConnectCard'; import QueuedFollowups from '../components/chat/QueuedFollowups'; import { ConfirmationModal } from '../components/intelligence/ConfirmationModal'; import { SidebarContent } from '../components/layout/shell/SidebarSlot'; @@ -2527,11 +2528,31 @@ const Conversations = ({ const pendingApproval = approvalThreadId ? pendingApprovalByThread[approvalThreadId] : undefined; - return pendingApproval && approvalThreadId ? ( + if (!pendingApproval || !approvalThreadId) return null; + // `composio_connect` parks on the same gate but needs a Connect + // button + OAuth poll rather than approve/deny (#3993). + const isConnect = pendingApproval.toolName === 'composio_connect'; + return (
- + {isConnect ? ( + // Key by requestId so switching from one parked approval to + // another remounts the card with fresh local state (phase, + // field values, cancellation refs, poll timers) instead of + // bleeding the previous request's state in (#4062, coderabbit). + + ) : ( + + )}
- ) : null; + ); })()} {(() => { diff --git a/app/src/providers/ChatRuntimeProvider.tsx b/app/src/providers/ChatRuntimeProvider.tsx index 3097700ce..325b152e6 100644 --- a/app/src/providers/ChatRuntimeProvider.tsx +++ b/app/src/providers/ChatRuntimeProvider.tsx @@ -961,6 +961,9 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => { firstString(a.path) ?? firstString(a.url) ?? firstString(a.target); + // `composio_connect` carries the toolkit slug so the inline connect + // card (#3993) knows which integration to authorize. + const toolkit = firstString(a.toolkit); dispatch( setPendingApprovalForThread({ threadId: event.thread_id, @@ -969,6 +972,7 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => { toolName: event.tool_name, message: event.message, command, + toolkit, }, }) ); diff --git a/app/src/store/chatRuntimeSlice.ts b/app/src/store/chatRuntimeSlice.ts index 81decd673..99799134f 100644 --- a/app/src/store/chatRuntimeSlice.ts +++ b/app/src/store/chatRuntimeSlice.ts @@ -221,6 +221,13 @@ export interface PendingApproval { * extracted from the event's redacted args for display. Empty if unavailable. */ command?: string; + /** + * Toolkit slug carried on `composio_connect` requests (#3993). Present only + * when `toolName === 'composio_connect'`; the inline connect card uses it to + * run the OAuth handoff and poll for completion. The slug is a public + * identifier (not PII), so it survives arg redaction unchanged. + */ + toolkit?: string; } /** diff --git a/src/openhuman/agent_registry/agents/integrations_agent/agent.toml b/src/openhuman/agent_registry/agents/integrations_agent/agent.toml index de2375d15..420a1a5fd 100644 --- a/src/openhuman/agent_registry/agents/integrations_agent/agent.toml +++ b/src/openhuman/agent_registry/agents/integrations_agent/agent.toml @@ -22,6 +22,11 @@ named = [ "composio_list_tools", "file_read", "composio_execute", + # Inline-in-chat OAuth connect card (#3993). When the bound toolkit is not + # connected — or an action fails with a true auth/connection error — raise + # the connect card with this tool and await the result, instead of bubbling + # up "Connection error" and sending the user to Settings → Connections. + "composio_connect", # Deterministic time resolver. Composio actions take time-window args # (Slack/Gmail/Calendar `oldest`/`latest`/`since`/`after`) as raw # timestamps; the model must NEVER hand-compute epoch seconds (a real diff --git a/src/openhuman/agent_registry/agents/integrations_agent/prompt.md b/src/openhuman/agent_registry/agents/integrations_agent/prompt.md index 2f1168081..a9f183adb 100644 --- a/src/openhuman/agent_registry/agents/integrations_agent/prompt.md +++ b/src/openhuman/agent_registry/agents/integrations_agent/prompt.md @@ -6,6 +6,7 @@ You are the **Integrations Agent**. You interact with one connected external ser - **`composio_list_tools`** — inspect the action catalogue for your bound toolkit. Returns the `function.name` slug + JSON schema for each action. - **`composio_execute`** — run a Composio action: `{ tool: "", arguments: {...} }`. +- **`composio_connect`** — raise an **inline connect card** in the chat for your bound toolkit and wait for the user to authorize in one click. Use this the moment you detect the toolkit is not connected, or after a true auth/connection error. Never tell the user to open Settings → Connections yourself while this tool is available. - **`extract_from_result`** — runtime-provided system tool for oversized-result runs. Use it when a tool returned too much data to inspect directly: pass the prior `result_id` plus a narrow `query`, and it will return only the requested slice from that oversized result. - **Per-action tools** — the toolkit's individual action tools are already registered in your tool list with typed schemas (e.g. `GMAIL_SEND_EMAIL`, `NOTION_CREATE_PAGE`). Prefer calling these directly over the generic `composio_execute`. @@ -13,17 +14,21 @@ You do **not** have shell, file I/O, or any other capability beyond these permit ## Typical flow +0. **Connect first if needed.** If the caller's objective is simply to connect/authorize this toolkit, or you already know it isn't connected, call `composio_connect { toolkit }`, await the result, and report it. `{ connected: true }` → proceed (or you're done, if connecting was the whole task); `{ connected: false }` → the user declined: report that plainly, note they can still connect later via Settings → Connections, and stop — do **not** retry `composio_connect`. 1. You already have the toolkit's action tools in your tool list — start there. If you need a schema reminder or a slug you don't see, call `composio_list_tools`. 2. Call the per-action tool (or `composio_execute` with the slug) using the caller's task as your guide. 3. If the call fails with `[composio:error:insufficient_scope]`, `insufficient authentication scopes`, or `missing required permissions`, do **not** call the service disconnected. Say the connected account is missing the permissions needed for the requested action and point the user to Settings → Connections → the toolkit to reconnect or enable the required scope. -4. If the call fails with a true authentication / authorization / connection error that is **not** a scope or permission error, stop and return: **"Connection error, try to authenticate"** — the orchestrator will take over and route the user to settings. +4. If the call fails with a true authentication / authorization / connection error that is **not** a scope or permission error, the toolkit is not connected. Call **`composio_connect`** with your bound `toolkit` to raise an inline connect card and **await its result**: + - `{ connected: true }` → the user authorized; retry the original action **once** and continue. + - `{ connected: false, declined: true }` (or an error) → the user declined or the card could not be raised. **Only then** return **"Connection error, try to authenticate"** so the orchestrator can route the user to settings. + Do **not** print a Settings → Connections instruction yourself when `composio_connect` is available. ## Rules - **Never fabricate action slugs.** Pull them from `composio_list_tools` or use the per-action tools already in your list. - **Respect rate limits** — Composio and upstream providers both throttle. Back off on errors rather than retrying tightly. - **Scope errors are not disconnections.** If Gmail or another connected toolkit returns insufficient scope / missing permissions, report the missing permission plainly and direct the user to Settings → Connections → that toolkit. Never say the toolkit is disconnected for this case. -- **Auth errors bubble up.** On true auth / connection failures only, reply exactly: `Connection error, try to authenticate`. Do not retry, do not attempt to re-authorise yourself — you have no tools for that. +- **Auth errors → connect inline first.** On a true auth / connection failure (not a scope error), call `composio_connect { toolkit }` to raise the inline connect card and await it. If it returns `connected: true`, retry the action once. Only if the user declines or the card can't be raised, reply exactly: `Connection error, try to authenticate`. Never paste OAuth URLs or name Composio to the user. - **Be precise** — every action expects a specific argument shape. Validate against the schema before calling. - **Report results** — state what action was taken and the outcome, including any cost reported by Composio. diff --git a/src/openhuman/agent_registry/agents/orchestrator/agent.toml b/src/openhuman/agent_registry/agents/orchestrator/agent.toml index 0245629e4..916094fe6 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/agent.toml +++ b/src/openhuman/agent_registry/agents/orchestrator/agent.toml @@ -171,12 +171,14 @@ hint = "chat" # Direct tools — things the orchestrator calls itself rather than # delegating. # -# `composio_list_connections` is the orchestrator's only composio_* -# tool: it exists so the agent can detect newly-authorised integrations -# mid-session (the session-start fetch froze the Delegation Guide's -# connected list). Authorisation, toolkit discovery, action listing, -# and action execution all live downstream in `integrations_agent` — -# the orchestrator never calls composio_authorize / composio_list_tools +# `composio_list_connections` lets the agent detect newly-authorised +# integrations mid-session (the session-start fetch froze the Delegation +# Guide's connected list). `composio_connect` lets it raise an inline +# connect card for ANY toolkit the user asks to connect — including ones +# not yet connected (#3993); the tool validates the slug against the +# backend allowlist and the OAuth handoff itself runs inside the card. +# Toolkit *action* listing and execution still live downstream in +# `integrations_agent` — the orchestrator never calls composio_list_tools # / composio_execute directly. named = [ "read_workspace_state", @@ -190,6 +192,11 @@ named = [ "spawn_async_subagent", "spawn_parallel_agents", "composio_list_connections", + # Inline OAuth connect card (#3993). Free-form `toolkit` slug — the + # orchestrator calls this directly to connect a service the user names, + # whether or not it is already connected. Validates against the backend + # allowlist and raises the in-chat Connect card. + "composio_connect", # Lightweight MCP discovery — the orchestrator's only `mcp_registry_*` # tool, mirroring `composio_list_connections`. Lists the user's installed # MCP servers and their live connection state + tool counts so the agent diff --git a/src/openhuman/agent_registry/agents/orchestrator/prompt.md b/src/openhuman/agent_registry/agents/orchestrator/prompt.md index a3e327564..73f6c14e0 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/prompt.md +++ b/src/openhuman/agent_registry/agents/orchestrator/prompt.md @@ -26,7 +26,7 @@ Follow this sequence for every user message: - Words like "email/inbox/gmail", "calendar", "notion doc", "drive file", "slack/whatsapp/telegram message", "linear ticket", "send to X", "check X", etc. mean the user wants the **live** service. - Find the matching toolkit in the **Connected Integrations** section and call `delegate_to_integrations_agent` with that `toolkit`. - **Do this even if `memory_tree` could plausibly answer.** The user wants the live source of truth, not a stale summary. - - If the relevant toolkit is not in **Connected Integrations**, tell the user to connect it via Settings → Connections → [Service] (see "Connecting external services" below). Do **not** silently fall back to `memory_tree`. + - If the relevant toolkit is **not** in **Connected Integrations**, call `composio_connect { toolkit: "" }` **directly** to raise an **inline connect card** so the user can authorize in one click, then continue the task once it returns `connected: true`. Do **not** refuse based on the Connected Integrations list (that is only what is *already* connected, not what is *connectable*), do **not** make "go to Settings → Connections" your first move, and do **not** silently fall back to `memory_tree` (see "Connecting external services" below). 3. **Can I solve this with direct tools?** - Yes: use direct tools (`query_memory`, `read_workspace_state`, `composio_list_connections`, task tools, etc.). - No: continue. @@ -102,7 +102,9 @@ When the user asks to connect a service (Gmail, Notion, WhatsApp, Calendar, Driv - **Never** paste external URLs (e.g. `app.composio.dev`, provider OAuth pages, dashboards). - **Never** explain OAuth, Composio, or any backend mechanic by name. -- Reply with one short bubble pointing to the in-app path: **Settings → Connections → [Service]**. Example: `head to Settings → Connections → Gmail to hook it up, ping me when it's connected`. +- **Connect inline, don't redirect.** Call `composio_connect { toolkit: "" }` **directly** to raise an **inline connect card** in the chat — this works for **any** service the user names (gmail, notion, whatsapp, youtube, …), not just ones already connected. The card *is* the confirmation: when the user asks to connect/authorize a service, or wants to use one that isn't connected, just call `composio_connect` — don't ask "want me to raise a card?" first. The user authorizes in one click and the task continues in the same turn. +- **Don't confabulate "unsupported".** You do **not** have the list of connectable toolkits in your prompt — only the *connected* ones. Never tell the user a service "isn't available to connect" from memory. `composio_connect` checks the real backend allowlist: if it returns that the toolkit isn't an available integration, relay that message (and the list it provides). That is the only honest "I can't connect this". +- **On decline / fallback.** If `composio_connect` reports the user declined (`connected: false`) or that it couldn't raise the card, acknowledge it and offer `head to Settings → Connections → [Service]` as the alternative. - If the user already said they connected it, call `composio_list_connections` to verify before continuing. - Do **not** apply this rule to scope / permission failures such as `[composio:error:insufficient_scope]` or "missing required permissions". For those, say the connection exists but needs additional permissions in **Settings → Connections → [Service]**. diff --git a/src/openhuman/approval/redact.rs b/src/openhuman/approval/redact.rs index ba458e3ba..c2fbed9bc 100644 --- a/src/openhuman/approval/redact.rs +++ b/src/openhuman/approval/redact.rs @@ -183,6 +183,15 @@ fn match_home_prefix(s: &str) -> Option { /// the user knows *what* the agent wants to do without exposing the /// content. pub fn summarize_action(tool_name: &str, args: &Value) -> String { + // Friendly, human-readable summaries for tools whose approval card reads + // better as a sentence than a `key=value` dump (#3993). `entry_id` is a + // public catalog slug, not PII, so it is safe to surface verbatim. + if tool_name == "skill_registry_install" { + if let Some(id) = args.get("entry_id").and_then(|v| v.as_str()) { + return format!("Install the \"{id}\" skill to complete your task"); + } + } + let safe_fields: &[&str] = &[ "action", "tool_slug", @@ -444,4 +453,36 @@ mod tests { assert!(summary.contains("pushover")); assert!(summary.contains("bytes")); } + + #[test] + fn summarize_action_skill_install_is_human_readable() { + let args = json!({ "entry_id": "notion" }); + let summary = summarize_action("skill_registry_install", &args); + // Friendly sentence, not a key=value/byte dump (#3993). + assert_eq!( + summary, + "Install the \"notion\" skill to complete your task" + ); + assert!(!summary.contains("bytes")); + } + + #[test] + fn redact_preserves_toolkit_slug_for_connect_card() { + // The inline connect card (#3993) reads `toolkit` out of the redacted + // args to drive the OAuth handoff, so a non-sensitive slug must survive + // redaction verbatim while real PII alongside it is still scrubbed. + let args = json!({ "toolkit": "gmail", "body": "secret message" }); + let redacted = redact_args(&args); + assert_eq!(redacted["toolkit"], json!("gmail")); + assert_ne!(redacted["body"], json!("secret message")); + } + + #[test] + fn summarize_action_skill_install_without_entry_id_falls_back() { + let args = json!({}); + let summary = summarize_action("skill_registry_install", &args); + // Missing slug → generic fallback so we never panic or mislabel. + assert!(summary.contains("skill_registry_install")); + assert!(summary.contains("bytes")); + } } diff --git a/src/openhuman/composio/tools.rs b/src/openhuman/composio/tools.rs index ed5dbe56b..f7464b8f2 100644 --- a/src/openhuman/composio/tools.rs +++ b/src/openhuman/composio/tools.rs @@ -666,6 +666,277 @@ impl Tool for ComposioAuthorizeTool { } } +// ── composio_connect (inline approval card, #3993) ────────────────── + +/// Canonicalize an agent/user-supplied toolkit slug to the form Composio's +/// backend expects. Mirrors `canonicalizeComposioToolkitSlug` on the FE +/// (`app/src/lib/composio/toolkitSlug.ts`) — **keep the alias maps in sync**. +/// The agent frequently guesses `google_drive` where Composio uses +/// `googledrive` (#3993); without this the OAuth handoff fails with an opaque +/// error. +fn canonicalize_toolkit_slug(slug: &str) -> String { + let key = slug.trim().to_ascii_lowercase(); + match key.as_str() { + "feishu" | "lark" => "larksuite".to_string(), + "google_calendar" => "googlecalendar".to_string(), + "google_drive" => "googledrive".to_string(), + "google_sheets" => "googlesheets".to_string(), + _ => key, + } +} + +/// Fresh (uncached) liveness check for `toolkit`. +/// +/// Tri-state via `Result`: +/// - `Ok(true)` — a connection is verified ACTIVE. +/// - `Ok(false)` — the read succeeded but no ACTIVE connection exists. +/// - `Err(_)` — the state could **not** be verified (client construction or +/// the list call failed). +/// +/// Used to confirm liveness after the approval gate resolves `Allow`. The +/// card-driven path only approves once it has polled the connection ACTIVE, +/// but other approval surfaces (a typed `yes`, Telegram's approval prompt, or +/// an existing auto-approve entry) resolve `Allow` with no OAuth poll — so +/// `Allow` alone must NOT be treated as "connected" (#3993, codex review). +/// +/// Distinguishing `Err` from `Ok(false)` lets the caller fail closed on a +/// transient backend/auth failure **without** fabricating an "OAuth not +/// complete" reason that wrongly blames the user (#4062, coderabbit review). +async fn connection_is_active(config: &Config, toolkit: &str) -> anyhow::Result { + let active_match = |connections: &[super::types::ComposioConnection]| { + connections + .iter() + .any(|c| c.is_active() && c.normalized_toolkit().eq_ignore_ascii_case(toolkit)) + }; + match create_composio_client(config)? { + ComposioClientKind::Backend(client) => { + Ok(active_match(&client.list_connections().await?.connections)) + } + ComposioClientKind::Direct(direct) => Ok(active_match( + &direct_list_connections(&direct).await?.connections, + )), + } +} + +/// Connect a Composio integration **inline in the chat** instead of +/// sending the user off to Settings → Connections. +/// +/// Unlike [`ComposioAuthorizeTool`] (which hands the agent a raw +/// `connectUrl` it is not allowed to paste), this tool raises an +/// approval card via the process-global `ApprovalGate`. The frontend +/// recognises `tool_name == "composio_connect"` and renders a **Connect** +/// button: clicking it runs the existing `composio_authorize` RPC, opens +/// the OAuth handoff, and polls `composio_list_connections` until the +/// toolkit is ACTIVE — at which point it resolves the gate. The agent +/// then resumes the original task in the same turn. +pub struct ComposioConnectTool { + config: Arc, +} + +impl ComposioConnectTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for ComposioConnectTool { + fn name(&self) -> &str { + "composio_connect" + } + fn description(&self) -> &str { + "Connect a Composio integration (OAuth) for the user **inline in the chat**. \ + Raises an approval card with a Connect button — the user authorizes in one \ + click without leaving the conversation, and this tool returns once the \ + connection is active (or the user declines). ALWAYS prefer this over telling \ + the user to open Settings → Connections. Returns {toolkit, connected}." + } + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "toolkit": { + "type": "string", + "description": "Toolkit slug to connect, e.g. 'gmail' or 'notion'." + } + }, + "required": ["toolkit"], + "additionalProperties": false + }) + } + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + fn category(&self) -> ToolCategory { + ToolCategory::Workflow + } + // NOTE: `external_effect` deliberately stays `false`. Gating happens + // *inside* `execute` via a manual `ApprovalGate` intercept so we can + // (a) skip the card when the toolkit is already connected and (b) carry + // the toolkit slug into the card for the inline Connect button. The + // engine's auto-gate is unconditional and would double-prompt. + async fn execute(&self, args: Value) -> anyhow::Result { + let raw_toolkit = args + .get("toolkit") + .and_then(|v| v.as_str()) + .unwrap_or("") + .trim(); + if raw_toolkit.is_empty() { + return Ok(ToolResult::error("composio_connect: 'toolkit' is required")); + } + // Canonicalize before any backend call — the agent often guesses + // `google_drive` where Composio expects `googledrive` (#3993). + let toolkit = canonicalize_toolkit_slug(raw_toolkit); + tracing::debug!(raw = %raw_toolkit, toolkit = %toolkit, "[composio] tool connect.execute"); + + // The inline connect card only has a surface on an interactive chat + // turn (the web-chat path installs `APPROVAL_CHAT_CONTEXT`). On + // background / cron turns there is no UI to click Connect, so fail + // closed with a clear message rather than parking forever — mirrors + // the `install_tool` guard (#3993). + if crate::openhuman::approval::APPROVAL_CHAT_CONTEXT + .try_with(|_| ()) + .is_err() + { + return Ok(ToolResult::error(format!( + "[policy-denied] composio_connect needs an interactive chat turn. \ + Ask the user to connect '{toolkit}' in Settings → Connections." + ))); + } + + // Reload config per call so a mid-session `composio.mode` toggle is + // honoured (#1710), then skip the card entirely if the toolkit is + // already connected — avoids a flash of a Connect card that would + // immediately resolve. + let live_config = + match config_rpc::reload_config_snapshot_with_timeout(self.config.as_ref()).await { + Ok(c) => c, + Err(e) => { + tracing::warn!(error = %e, "[composio] connect.execute: load_config failed"); + self.config.as_ref().clone() + } + }; + let already_connected = super::fetch_connected_integrations(&live_config) + .await + .into_iter() + .any(|ci| ci.connected && ci.toolkit.eq_ignore_ascii_case(&toolkit)); + if already_connected { + tracing::debug!(toolkit = %toolkit, "[composio] connect.execute: already connected"); + return Ok(ToolResult::success(serde_json::to_string(&json!({ + "toolkit": toolkit, + "connected": true, + "already_connected": true, + }))?)); + } + + // Validate the toolkit against the *connectable* catalog before raising + // a card. The orchestrator only knows which toolkits are already + // connected — it must NOT confabulate "unsupported" from that list + // (#3993). This grounds the answer: a backend-allowlisted toolkit gets + // a card; a genuinely unsupported one gets a clear, listed refusal + // instead of a card that would fail on Connect. + match create_composio_client(&live_config) { + Ok(ComposioClientKind::Backend(client)) => { + if let Ok(resp) = client.list_toolkits().await { + // Empty allowlist = backend predates the catalog / unknown; + // don't block — let the OAuth handoff report support. + if !resp.toolkits.is_empty() + && !resp + .toolkits + .iter() + .any(|s| s.eq_ignore_ascii_case(&toolkit)) + { + let available = resp.toolkits.join(", "); + tracing::info!(toolkit = %toolkit, "[composio] connect.execute: toolkit not in allowlist"); + return Ok(ToolResult::error(format!( + "composio_connect: '{toolkit}' is not an available integration. \ + Connectable toolkits: {available}" + ))); + } + } + } + Ok(ComposioClientKind::Direct(_)) => { + // Personal-tenant (direct) mode performs OAuth at app.composio.dev, + // not via the backend handoff the card drives — so an inline card + // can't complete it. Point the user to Settings instead. + return Ok(ToolResult::error(format!( + "composio_connect: direct Composio mode is active — connect '{toolkit}' \ + in Settings → Connections (your personal Composio account)." + ))); + } + Err(e) => { + return Ok(ToolResult::error(format!( + "composio_connect: Composio is unavailable: {e}" + ))); + } + } + + // Raise the inline connect card via the approval gate. The frontend + // resolves it with `approve_once` once it polls the connection ACTIVE; + // an explicit decline or the 10-minute TTL resolves it as denied. + let gate = match crate::openhuman::approval::ApprovalGate::try_global() { + Some(g) => g, + None => { + return Ok(ToolResult::error( + "composio_connect: approval gate unavailable in this environment", + )); + } + }; + let summary = format!("Connect {toolkit} to complete your task"); + let (outcome, _request_id) = gate + .intercept_audited("composio_connect", &summary, json!({ "toolkit": toolkit })) + .await; + match outcome { + crate::openhuman::approval::GateOutcome::Allow => { + // `Allow` only means the prompt was approved — re-check liveness + // with a fresh read, because non-card approval surfaces (typed + // "yes", Telegram, auto-approve) resolve Allow without running + // the OAuth poll (#3993, codex review). + match connection_is_active(&live_config, &toolkit).await { + Ok(true) => { + tracing::debug!(toolkit = %toolkit, "[composio] connect.execute: connection active"); + Ok(ToolResult::success(serde_json::to_string(&json!({ + "toolkit": toolkit, + "connected": true, + }))?)) + } + Ok(false) => { + tracing::info!(toolkit = %toolkit, "[composio] connect.execute: approved but not yet active"); + Ok(ToolResult::success(serde_json::to_string(&json!({ + "toolkit": toolkit, + "connected": false, + "reason": "Approved, but the connection is not active yet — the user still needs to complete the OAuth flow.", + }))?)) + } + Err(e) => { + // Couldn't verify liveness — a transient backend/auth + // failure, NOT proof the user skipped OAuth. Fail closed + // (connected:false) but report a verification error + // rather than fabricating an "OAuth incomplete" reason + // that blames the user and can drive the agent into + // reconnect loops (#4062, coderabbit review). + tracing::warn!(toolkit = %toolkit, error = %e, "[composio] connect.execute: liveness check failed"); + Ok(ToolResult::success(serde_json::to_string(&json!({ + "toolkit": toolkit, + "connected": false, + "reason": "Approved, but the connection state could not be verified right now (a temporary problem reaching Composio). Please try connecting again in a moment.", + }))?)) + } + } + } + crate::openhuman::approval::GateOutcome::Deny { reason } => { + tracing::info!(toolkit = %toolkit, reason = %reason, "[composio] connect.execute: declined"); + Ok(ToolResult::success(serde_json::to_string(&json!({ + "toolkit": toolkit, + "connected": false, + "declined": true, + "reason": reason, + }))?)) + } + } + } +} + // ── composio_list_tools ───────────────────────────────────────────── pub struct ComposioListToolsTool { @@ -1255,6 +1526,9 @@ pub fn all_composio_agent_tools(config: &crate::openhuman::config::Config) -> Ve Box::new(ComposioListToolkitsTool::new(config_arc.clone())), Box::new(ComposioListConnectionsTool::new(config_arc.clone())), Box::new(ComposioAuthorizeTool::new(config_arc.clone())), + // Inline-in-chat OAuth connect card (#3993). Raises an approval card + // with a Connect button instead of handing the agent a raw URL. + Box::new(ComposioConnectTool::new(config_arc.clone())), Box::new(ComposioListToolsTool::new(config_arc.clone())), Box::new(ComposioExecuteTool::new(config_arc)), // Pref-elevation is intentionally NOT an agent-callable tool; diff --git a/src/openhuman/composio/tools_tests.rs b/src/openhuman/composio/tools_tests.rs index c239e8e2e..22e89b67f 100644 --- a/src/openhuman/composio/tools_tests.rs +++ b/src/openhuman/composio/tools_tests.rs @@ -125,6 +125,7 @@ fn all_composio_tools_are_in_skill_category() { Box::new(ComposioListToolkitsTool::new(config.clone())), Box::new(ComposioListConnectionsTool::new(config.clone())), Box::new(ComposioAuthorizeTool::new(config.clone())), + Box::new(ComposioConnectTool::new(config.clone())), Box::new(ComposioListToolsTool::new(config.clone())), Box::new(ComposioExecuteTool::new(config)), ]; @@ -144,6 +145,7 @@ fn all_composio_tools_are_in_skill_category() { assert!(names.contains(&"composio_list_toolkits")); assert!(names.contains(&"composio_list_connections")); assert!(names.contains(&"composio_authorize")); + assert!(names.contains(&"composio_connect")); assert!(names.contains(&"composio_list_tools")); assert!(names.contains(&"composio_execute")); } @@ -217,6 +219,119 @@ async fn authorize_tool_execute_rejects_whitespace_toolkit() { assert!(result.is_error); } +// ── composio_connect (inline approval card, #3993) ────────────────── + +fn tool_result_text(result: &crate::openhuman::tools::traits::ToolResult) -> String { + result + .content + .iter() + .filter_map(|c| match c { + crate::openhuman::tools::traits::ToolContent::Text { text } => Some(text.clone()), + _ => None, + }) + .collect::>() + .join(" ") +} + +#[test] +fn connect_tool_metadata_requires_toolkit_and_is_not_auto_gated() { + let t = ComposioConnectTool::new(fake_config_arc()); + assert_eq!(t.name(), "composio_connect"); + // Gating is done manually inside execute (so it can skip the card when + // already connected) — the engine must NOT auto-gate it. + assert!(!t.external_effect()); + let schema = t.parameters_schema(); + let required: Vec = schema["required"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect(); + assert_eq!(required, vec!["toolkit"]); +} + +#[tokio::test] +async fn connect_tool_execute_rejects_missing_toolkit() { + let t = ComposioConnectTool::new(fake_config_arc()); + let result = t.execute(serde_json::json!({})).await.unwrap(); + assert!(result.is_error); + assert!(tool_result_text(&result).contains("'toolkit' is required")); +} + +#[tokio::test] +async fn connect_tool_refuses_without_interactive_chat_context() { + // No `APPROVAL_CHAT_CONTEXT` in scope → background/cron turn. There is no + // surface for the Connect card, so the tool must fail closed (it returns + // before touching the network). + let t = ComposioConnectTool::new(fake_config_arc()); + let result = t + .execute(serde_json::json!({ "toolkit": "gmail" })) + .await + .unwrap(); + assert!(result.is_error); + assert!(tool_result_text(&result).contains("[policy-denied]")); +} + +#[test] +fn canonicalize_toolkit_slug_maps_known_aliases_and_passes_through() { + // Mirrors the FE `canonicalizeComposioToolkitSlug` map (#3993). + assert_eq!( + super::canonicalize_toolkit_slug("google_drive"), + "googledrive" + ); + assert_eq!( + super::canonicalize_toolkit_slug("Google_Calendar"), + "googlecalendar" + ); + assert_eq!( + super::canonicalize_toolkit_slug("google_sheets"), + "googlesheets" + ); + assert_eq!(super::canonicalize_toolkit_slug("feishu"), "larksuite"); + assert_eq!(super::canonicalize_toolkit_slug("lark"), "larksuite"); + // Unknown slugs are trimmed + lowercased, not rewritten. + assert_eq!(super::canonicalize_toolkit_slug(" Notion "), "notion"); + assert_eq!(super::canonicalize_toolkit_slug("gmail"), "gmail"); +} + +#[tokio::test] +async fn connect_tool_validates_before_gating_in_chat_context() { + use crate::openhuman::approval::{ApprovalChatContext, APPROVAL_CHAT_CONTEXT}; + // With a chat context the interactive guard passes; with no composio + // credentials the client factory errors — so execute canonicalizes the + // slug, reloads config, checks connected state, and returns a clean error + // at the client-resolution step, exercising the pre-gate body with no + // network (#3993). + let tool = ComposioConnectTool::new(fake_config_arc()); + let ctx = ApprovalChatContext { + thread_id: "t-test".into(), + client_id: "c-test".into(), + }; + let result = APPROVAL_CHAT_CONTEXT + .scope( + ctx, + tool.execute(serde_json::json!({ "toolkit": "google_drive" })), + ) + .await + .unwrap(); + assert!(result.is_error); + // Backend mode without creds → "Composio is unavailable" (not the + // non-interactive policy refusal, which the chat context bypassed). + let txt = tool_result_text(&result).to_lowercase(); + assert!(txt.contains("composio"), "{txt}"); + assert!(!txt.contains("[policy-denied]"), "{txt}"); +} + +#[tokio::test] +async fn connection_is_active_errs_without_a_client() { + // Liveness re-check (#3993): with no composio client (no creds) we cannot + // confirm a connection. This must surface as `Err` (state unverifiable) — + // NOT `Ok(false)` — so the caller fails closed without fabricating an + // "OAuth incomplete" reason that blames the user (#4062, coderabbit). + let cfg = crate::openhuman::config::Config::default(); + assert!(super::connection_is_active(&cfg, "gmail").await.is_err()); +} + #[test] fn list_tools_tool_metadata_accepts_optional_toolkits_filter() { let t = ComposioListToolsTool::new(fake_config_arc()); @@ -302,10 +417,10 @@ fn agent_tools_register_when_backend_signed_in() { let tools = all_composio_agent_tools(&config); assert_eq!( tools.len(), - 5, - "backend session present → all 5 generic composio agent tools should register \ - (list_toolkits, list_connections, authorize, list_tools, execute). Scope \ - elevation is intentionally NOT exposed as an agent tool — the user must \ + 6, + "backend session present → all 6 generic composio agent tools should register \ + (list_toolkits, list_connections, authorize, connect, list_tools, execute). \ + Scope elevation is intentionally NOT exposed as an agent tool — the user must \ flip scopes themselves in the Connections UI." ); } @@ -329,9 +444,9 @@ fn agent_tools_register_when_direct_mode_with_stored_key_and_no_backend_session( let tools = all_composio_agent_tools(&config); assert_eq!( tools.len(), - 5, + 6, "direct mode with stored API key (no backend session) must still register \ - all 5 generic composio agent tools — the pre-Option-C bug returned 0 here" + all 6 generic composio agent tools — the pre-Option-C bug returned 0 here" ); } diff --git a/src/openhuman/skill_registry/agent/skill_setup/prompt.md b/src/openhuman/skill_registry/agent/skill_setup/prompt.md index 08b5e2c3d..9442ef4a1 100644 --- a/src/openhuman/skill_registry/agent/skill_setup/prompt.md +++ b/src/openhuman/skill_registry/agent/skill_setup/prompt.md @@ -21,8 +21,8 @@ You help the user find and install skills from three registries: 1. When the user asks to find a skill, search across registries. 2. Present results clearly: name, description, source, install count. 3. Ask the user which skill(s) to install if multiple match. -4. Install the selected skill and confirm it was added. -5. If installation fails, explain the error and suggest alternatives. +4. Install the selected skill and confirm it was added. Installing raises an **inline approval card** the user accepts in the chat — you do not navigate away or hand out manual setup steps. +5. If the user **declines** the approval card, or installation fails, acknowledge it plainly and suggest alternatives. Do **not** silently retry the same install — one declined/failed attempt is enough. ## Important rules diff --git a/src/openhuman/skill_registry/tools.rs b/src/openhuman/skill_registry/tools.rs index e991c6072..3433b53ca 100644 --- a/src/openhuman/skill_registry/tools.rs +++ b/src/openhuman/skill_registry/tools.rs @@ -167,6 +167,16 @@ impl Tool for SkillRegistryInstallTool { PermissionLevel::Write } + /// Installing a skill mutates the user's local skill set and fetches a + /// remote `SKILL.md`, so it routes through the process-global + /// `ApprovalGate` (#3993). On an interactive chat turn the user sees an + /// inline approval card and approves before anything is written; on + /// background/cron turns (no `APPROVAL_CHAT_CONTEXT`) the gate is bypassed, + /// matching every other external-effect tool. + fn external_effect(&self) -> bool { + true + } + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { let entry_id = args .get("entry_id") @@ -286,3 +296,28 @@ impl Tool for SkillRegistryUninstallTool { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn install_tool_is_external_effect_so_it_routes_through_approval_gate() { + let tool = SkillRegistryInstallTool::new(Arc::new(Config::default())); + assert_eq!(tool.name(), "skill_registry_install"); + // #3993: installs must raise an inline approval card before writing. + assert!( + tool.external_effect(), + "skill_registry_install must declare external_effect so the harness gates it" + ); + assert!(matches!(tool.permission_level(), PermissionLevel::Write)); + } + + #[test] + fn read_only_skill_tools_are_not_gated() { + // Browse/search/sources stay ungated — they only read the catalog. + assert!(!SkillRegistryBrowseTool.external_effect()); + assert!(!SkillRegistrySearchTool.external_effect()); + assert!(!SkillRegistrySourcesTool.external_effect()); + } +} diff --git a/tests/composio_ops_credentials_appstate_raw_coverage_e2e.rs b/tests/composio_ops_credentials_appstate_raw_coverage_e2e.rs index da6b89aea..aff8c3ff4 100644 --- a/tests/composio_ops_credentials_appstate_raw_coverage_e2e.rs +++ b/tests/composio_ops_credentials_appstate_raw_coverage_e2e.rs @@ -308,7 +308,8 @@ async fn round18_composio_ops_and_agent_tools_cover_backend_errors_and_metadata( .contains("not in the curated whitelist")); let registered_tools = all_composio_agent_tools(&harness.config); - assert_eq!(registered_tools.len(), 5); + // 6 generic composio agent tools incl. the inline-connect card tool (#3993). + assert_eq!(registered_tools.len(), 6); *state.scenario.lock().expect("scenario") = Scenario::ToolkitsFail; let toolkits_error = composio_list_toolkits(&harness.config) diff --git a/tests/composio_raw_coverage_e2e.rs b/tests/composio_raw_coverage_e2e.rs index 6b0fe9af7..d5c21feb8 100644 --- a/tests/composio_raw_coverage_e2e.rs +++ b/tests/composio_raw_coverage_e2e.rs @@ -1119,6 +1119,7 @@ async fn composio_agent_tools_cover_metadata_missing_params_and_scope_helpers() "composio_list_toolkits", "composio_list_connections", "composio_authorize", + "composio_connect", "composio_list_tools", "composio_execute", ] diff --git a/tests/tools_approval_channels_raw_coverage_e2e.rs b/tests/tools_approval_channels_raw_coverage_e2e.rs index c9d29003c..9bbf27f8e 100644 --- a/tests/tools_approval_channels_raw_coverage_e2e.rs +++ b/tests/tools_approval_channels_raw_coverage_e2e.rs @@ -781,6 +781,7 @@ async fn composio_agent_tools_cover_backend_discovery_markdown_and_execution_pat "composio_list_toolkits", "composio_list_connections", "composio_authorize", + "composio_connect", "composio_list_tools", "composio_execute", ]