diff --git a/.changeset/auto-approval-allow-deny.md b/.changeset/auto-approval-allow-deny.md new file mode 100644 index 0000000000..2d5b0a335e --- /dev/null +++ b/.changeset/auto-approval-allow-deny.md @@ -0,0 +1,5 @@ +--- +"@roomote/web": patch +--- + +Auto mode for integration tool approvals now runs routine calls automatically and asks before anything risky when the session owner is present. If the owner is away, the call is blocked with a tool error and recorded as an automatic rejection; unavailable checks follow the same present-to-ask, absent-to-block behavior. Chat-originated sessions are treated as present because their approval card is linked from the conversation. diff --git a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx index 57805a6c4d..6f1c5afd05 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx @@ -141,7 +141,7 @@ describe('PendingIntegrationToolApprovals', () => { expect(screen.queryByText('No arguments')).not.toBeInTheDocument(); }); - it('tells the person what Auto made of the call, when it looked', () => { + it('tells the person when Auto flagged a risky call', () => { render( { /> , ); + expect(screen.getByTestId('auto-evaluation')).toHaveTextContent( 'Auto flagged this call as risky and asked you.', ); }); + + it('explains an unavailable Auto check without exposing implementation terms', () => { + render( + + + , + ); + + expect(screen.getByTestId('auto-evaluation')).toHaveTextContent( + "Auto couldn't check this call because an automatic check wasn't available, so it asked you.", + ); + expect( + screen.queryByText(/decision model|judgment model|logs/i), + ).toBeNull(); + }); }); diff --git a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx index ccb8de1190..c4d4e07183 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx @@ -13,8 +13,8 @@ import { } from '@/components/system'; import { MCP_INTEGRATIONS, - type IntegrationToolApprovalMetadata, type IntegrationToolAutoEvaluation, + type IntegrationToolApprovalMetadata, } from '@roomote/types'; const integrationNames = new Map( @@ -65,19 +65,15 @@ function approvalPrompt(item: IntegrationToolApprovalMetadata): string { return `Let ${name} use this tool?`; } -/** - * One line on what Auto made of the call, so the person deciding knows - * why they are being asked. Auto never rejects, so this only ever explains - * why it did not run the call on its own. - */ +/** Explain why Auto handed a call to the Session owner. */ function describeAutoEvaluation( evaluation: IntegrationToolAutoEvaluation, ): string { if (evaluation.unavailable === 'no_model') { - return 'Auto couldn’t check this call because no decision model is available.'; + return "Auto couldn't check this call because an automatic check wasn't available, so it asked you."; } if (evaluation.unavailable === 'error') { - return 'Auto couldn’t check this call.'; + return "Auto couldn't check this call, so it asked you."; } return evaluation.recommendation === 'approve' ? 'Auto would have run this call.' diff --git a/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx b/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx index 8a96609a11..1a7f0d1e92 100644 --- a/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx +++ b/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx @@ -29,15 +29,16 @@ export function IntegrationToolApprovalsExperimentalSetting() { tools dialog in Settings → Integrations offers Auto (default, shown with no choice selected), Always allow, Always ask, and Disable per tool. Set up Auto mode in Settings → Integrations to handle routine - work automatically and ask before anything risky. Always ask pauses - each call until the session owner allows it once, stops the asks for - the rest of that session, or declines it; Disable hides the tool and - blocks it outright. A task asks the owner of its session the same way, - and a task nobody can answer for, such as one an automation started, - cannot run an Always ask tool. Session owners can also ask to be asked - about any tool from its call in the transcript. Tools left on Auto - with Auto mode off run exactly as before. Policies are deployment-wide - and apply from the next session turn. + work automatically and ask before anything risky. If the session owner + is away, risky calls are blocked. Always ask pauses each call until + the session owner allows it once, stops the asks for the rest of that + session, or declines it; Disable hides the tool and blocks it + outright. A task asks the owner of its session the same way, and a + task nobody can answer for, such as one an automation started, cannot + run an Always ask tool. Session owners can also ask to be asked about + any tool from its call in the transcript. Tools left on Auto with Auto + mode off run exactly as before. Policies are deployment-wide and apply + from the next session turn.

diff --git a/apps/web/src/components/settings/IntegrationToolAutoModeSetting.client.test.tsx b/apps/web/src/components/settings/IntegrationToolAutoModeSetting.client.test.tsx index 6d585e98be..7a752ddf3c 100644 --- a/apps/web/src/components/settings/IntegrationToolAutoModeSetting.client.test.tsx +++ b/apps/web/src/components/settings/IntegrationToolAutoModeSetting.client.test.tsx @@ -48,6 +48,11 @@ describe('IntegrationToolAutoModeSetting', () => { it('shows the current mode and keeps the guidance behind a disclosure', () => { render(); expect(screen.getByRole('radio', { name: /^Off/ })).toBeChecked(); + expect( + screen.getByText( + "Let Roomote handle routine work and ask before anything risky. When you're away, risky calls are blocked. Your other tool choices stay the same.", + ), + ).toBeInTheDocument(); expect( screen.queryByLabelText('Additional instructions'), ).not.toBeInTheDocument(); diff --git a/apps/web/src/components/settings/IntegrationToolAutoModeSetting.tsx b/apps/web/src/components/settings/IntegrationToolAutoModeSetting.tsx index 81b9f98ad4..b51c08c38d 100644 --- a/apps/web/src/components/settings/IntegrationToolAutoModeSetting.tsx +++ b/apps/web/src/components/settings/IntegrationToolAutoModeSetting.tsx @@ -24,9 +24,9 @@ import { /** Customer-facing copy: what Auto mode does for the person, no mechanics. */ const COPY = { description: - 'Let Roomote handle routine work and ask before anything risky. Your other tool choices stay the same.', + "Let Roomote handle routine work and ask before anything risky. When you're away, risky calls are blocked. Your other tool choices stay the same.", off: 'Keep each tool’s current choice.', - on: 'Handle routine work automatically and ask before anything risky.', + on: "Handle routine work automatically and ask before anything risky. Risky calls are blocked when you're away.", disclosure: 'Additional instructions', guidanceLabel: 'Additional instructions', guidanceHelp: @@ -45,8 +45,8 @@ const MODES: { mode: IntegrationToolAutoMode; label: string; hint: string }[] = /** * Deployment-wide Auto mode for tool approvals. Rendered on the - * Integrations page, only while the experiment is on. Reject is never affected, and the model can only ever run a call - * or ask; it never rejects one. + * Integrations page, only while the experiment is on. Reject is never + * affected, and Auto only runs routine calls; it asks about anything risky. */ export function IntegrationToolAutoModeSetting() { const trpc = useTRPC(); diff --git a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-tool-approvals.test.ts b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-tool-approvals.test.ts index 7cb333a887..719f4df31d 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-tool-approvals.test.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-tool-approvals.test.ts @@ -106,6 +106,24 @@ describe('createTaskToolApprovalRelay', () => { }, ); + it('rejects with the Auto mode tool error when the outcome is denied', async () => { + const { client, api, relay } = setup([], { + outcome: 'denied', + reason: 'it was assessed as risky', + }); + relay.handleAsk(ask); + await replied(client); + expect(api.status).not.toHaveBeenCalled(); + expect(client.replyPermission).toHaveBeenCalledWith( + expect.objectContaining({ + reply: 'reject', + message: expect.stringContaining( + 'Auto mode blocked this tool call because it was assessed as risky and the session owner was away', + ), + }), + ); + }); + it('rejects when nobody can approve, the tool is unknown, or the relay fails', async () => { const unavailable = setup([], { outcome: 'unavailable' }); unavailable.relay.handleAsk(ask); diff --git a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/tool-approvals.ts b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/tool-approvals.ts index f931a99b69..08f49f2650 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/tool-approvals.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/tool-approvals.ts @@ -218,6 +218,14 @@ export function createTaskToolApprovalRelay(options: { await reply(ask, 'once'); return; } + if (result.outcome === 'denied') { + await reply( + ask, + 'reject', + `Auto mode blocked this tool call because ${result.reason} and the session owner was away. The call was not run. The session owner can allow this tool from its call in the transcript.`, + ); + return; + } if (result.outcome === 'unavailable') { await reply( ask, diff --git a/packages/cloud-agents/src/server/__tests__/integration-tool-auto-evaluation.test.ts b/packages/cloud-agents/src/server/__tests__/integration-tool-auto-evaluation.test.ts index d52861b6e5..a2fb144705 100644 --- a/packages/cloud-agents/src/server/__tests__/integration-tool-auto-evaluation.test.ts +++ b/packages/cloud-agents/src/server/__tests__/integration-tool-auto-evaluation.test.ts @@ -156,7 +156,7 @@ describe('evaluateIntegrationToolAutoDecision', () => { expect(questions.guidanceFlagsRisk).toBeDefined(); }); - it('falls back to asking with no model or a failed evaluation', async () => { + it('asks when no model or a failed evaluation leaves Auto unable to check', async () => { mocks.evaluate.mockResolvedValue(null); await expect( evaluateIntegrationToolAutoDecision(call), @@ -173,7 +173,7 @@ describe('evaluateIntegrationToolAutoDecision', () => { }); describe('resolveIntegrationToolAutoState', () => { - it('is on only with the experiment, the setting, and a hosted judgment model', async () => { + it('is on with the experiment and the setting, even when no judgment model is left', async () => { mocks.settings.mockResolvedValue({ mode: 'on', policy: 'Reads are fine.' }); await expect(resolveIntegrationToolAutoState()).resolves.toMatchObject({ mode: 'on', @@ -181,15 +181,17 @@ describe('resolveIntegrationToolAutoState', () => { settings: { policy: 'Reads are fine.' }, }); - // The helper-model fallback is an LLM call per tool call: never implied. + // The setting can outlive the model: on stays on so callers ask or deny + // based on presence instead of silently running unassessed calls. The + // helper-model fallback is an LLM call per tool call: never implied. mocks.resolveModel.mockResolvedValue({ kind: 'helper', model: 'm' }); await expect(resolveIntegrationToolAutoState()).resolves.toMatchObject({ - mode: 'off', + mode: 'on', model: 'helper', }); mocks.resolveModel.mockResolvedValue(null); await expect(resolveIntegrationToolAutoState()).resolves.toMatchObject({ - mode: 'off', + mode: 'on', model: null, }); @@ -256,9 +258,9 @@ describe('recordIntegrationToolShadowEvaluationInBackground', () => { }); describe('resolveIntegrationToolAutoDecision', () => { - it('asks unless on, and then runs only a routine call', async () => { + it('runs unassessed unless on, and then runs only a routine call', async () => { await expect(resolveIntegrationToolAutoDecision(call)).resolves.toEqual({ - action: 'ask', + action: 'run', mode: 'off', }); expect(mocks.evaluate).not.toHaveBeenCalled(); @@ -281,7 +283,7 @@ describe('resolveIntegrationToolAutoDecision', () => { 'Reads are routine.', ); - // Risky, or no model at all: the card shows. + // Risky, or a failed evaluation: the call asks its owner. mocks.evaluate.mockResolvedValue( modelAnswers({ ...routine, risk: { score: 2, confidence: 0.9 } }), ); @@ -297,4 +299,22 @@ describe('resolveIntegrationToolAutoDecision', () => { evaluation: { unavailable: 'no_model' }, }); }); + + it('asks when Auto is on but no judgment model is configured', async () => { + mocks.settings.mockResolvedValue({ mode: 'on', policy: '' }); + mocks.resolveModel.mockResolvedValue(null); + await expect( + resolveIntegrationToolAutoDecision(call), + ).resolves.toMatchObject({ + action: 'ask', + mode: 'on', + evaluation: { recommendation: 'ask', unavailable: 'no_model' }, + }); + // The helper model is never used for tool-call assessment. + mocks.resolveModel.mockResolvedValue({ kind: 'helper', model: 'm' }); + await expect( + resolveIntegrationToolAutoDecision(call), + ).resolves.toMatchObject({ action: 'ask', mode: 'on' }); + expect(mocks.evaluate).not.toHaveBeenCalled(); + }); }); diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts index a04b032d70..07d8fdd081 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts @@ -1,8 +1,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('../../integration-tool-auto-evaluation', () => ({ + describeIntegrationToolAutoDeny: vi.fn( + (evaluation: { unavailable?: string }) => + evaluation.unavailable === 'no_model' + ? 'an automatic check is not available' + : evaluation.unavailable === 'error' + ? 'the automatic check failed' + : 'it was assessed as risky', + ), resolveIntegrationToolAutoDecision: vi.fn(async () => ({ - action: 'ask', + action: 'run', mode: 'off', })), resolveIntegrationToolAutoState: vi.fn(async () => ({ @@ -23,6 +31,9 @@ vi.mock('@roomote/db/server', () => ({ insertAutoApprovedIntegrationToolApproval: vi.fn(async () => ({ approvalId: 'auto-approval-1', })), + insertAutoRejectedIntegrationToolApproval: vi.fn(async () => ({ + approvalId: 'auto-rejection-1', + })), insertIntegrationToolApproval: vi.fn(), isDeploymentExperimentEnabled: vi.fn(async () => true), listIntegrationToolPolicies: vi.fn(async () => []), @@ -30,6 +41,12 @@ vi.mock('@roomote/db/server', () => ({ listIntegrationToolUserPolicies: vi.fn(async () => []), markIntegrationToolApprovalConsumed: vi.fn(async () => true), })); +const redisMocks = vi.hoisted(() => ({ + isPresent: vi.fn(async () => true), +})); +vi.mock('@roomote/redis', () => ({ + isSessionUserPresent: redisMocks.isPresent, +})); import { claimAutoApprovedIntegrationToolApproval, @@ -37,6 +54,7 @@ import { getIntegrationToolApproval, getSessionForFastConversation, insertAutoApprovedIntegrationToolApproval, + insertAutoRejectedIntegrationToolApproval, insertIntegrationToolApproval, isDeploymentExperimentEnabled, listIntegrationToolPolicies, @@ -44,6 +62,7 @@ import { listIntegrationToolUserPolicies, markIntegrationToolApprovalConsumed, } from '@roomote/db/server'; +import { isSessionUserPresent } from '@roomote/redis'; import { buildIntegrationToolApprovalRules, @@ -52,6 +71,7 @@ import { extractApprovalCallArgs, hashIntegrationToolApprovalRules, integrationToolApprovalRulesToConfig, + isFastAgentApprovalChatSurface, resolveFastAgentToolApprovalRules, resolveFastAgentToolApprovalSession, shouldDisposeInstanceForToolApprovalRules, @@ -778,6 +798,22 @@ describe('resolveFastAgentToolApprovalSession', () => { }); }); +describe('approval chat surfaces', () => { + it.each(['slack', 'discord', 'teams', 'telegram'] as const)( + 'treats %s as a present approval surface', + (surface) => { + expect(isFastAgentApprovalChatSurface(surface)).toBe(true); + }, + ); + + it.each(['web', 'agentmail', 'automation'] as const)( + 'does not treat %s as a chat approval surface', + (surface) => { + expect(isFastAgentApprovalChatSurface(surface)).toBe(false); + }, + ); +}); + describe('tool approval bridge', () => { const ask = { requestId: 'req-1', @@ -792,7 +828,13 @@ describe('tool approval bridge', () => { fetchCallArgs: vi.fn(async () => ({ input: { channel: 'C1', text: 'hi' }, })), - reply: vi.fn(async () => undefined), + reply: vi.fn( + async ( + _requestId: string, + _response: 'once' | 'reject', + _message?: string, + ) => undefined, + ), }; } @@ -800,6 +842,7 @@ describe('tool approval bridge', () => { return createFastAgentToolApprovalBridge({ sessionId: 'session-id', userId: 'user-id', + surface: 'web', integrations, }); } @@ -808,6 +851,7 @@ describe('tool approval bridge', () => { vi.clearAllMocks(); vi.mocked(isDeploymentExperimentEnabled).mockResolvedValue(true); vi.mocked(listIntegrationToolSessionOverrides).mockResolvedValue([]); + redisMocks.isPresent.mockResolvedValue(true); vi.mocked(insertIntegrationToolApproval).mockResolvedValue({ approvalId: 'approval-1', integrationId: 'mock-slack', @@ -820,7 +864,7 @@ describe('tool approval bridge', () => { }); }); - it('runs a default tool Auto finds routine, and asks about one it finds risky', async () => { + it('runs a routine call and asks a present owner about a risky call', async () => { const evaluation = { recommendation: 'approve' as const, answers: {}, @@ -835,6 +879,7 @@ describe('tool approval bridge', () => { createFastAgentToolApprovalBridge({ sessionId: 'session-id', userId: 'user-id', + surface: 'web', integrations, autoToolKeys: new Set([JSON.stringify(['mock-slack', 'post_message'])]), userRequest: 'Tell the team we shipped.', @@ -865,37 +910,59 @@ describe('tool approval bridge', () => { }), ); - // Risky: the card, with the model's view on it. + // Risky while the owner is present: the pending row carries the + // assessment, and the existing card is allowed to explain it. + const riskyEvaluation = { ...evaluation, recommendation: 'ask' as const }; vi.mocked(resolveIntegrationToolAutoDecision).mockResolvedValue({ action: 'ask', mode: 'on', - evaluation: { ...evaluation, recommendation: 'ask' }, + evaluation: riskyEvaluation, }); vi.mocked(getIntegrationToolApproval).mockResolvedValue({ status: 'rejected', } as never); const unsure = helpers(); autoBridge().handleAsk({ ...ask, requestId: 'req-2' }, unsure); - await vi.waitFor(() => expect(unsure.reply).toHaveBeenCalled()); + await vi.waitFor(() => + expect(unsure.reply).toHaveBeenCalledWith( + 'req-2', + 'reject', + 'The requester rejected this tool call.', + ), + ); expect(insertIntegrationToolApproval).toHaveBeenCalledWith( - expect.anything(), + { sessionId: 'session-id', userId: 'user-id' }, expect.objectContaining({ nativeRequestId: 'req-2', - autoEvaluation: { ...evaluation, recommendation: 'ask' }, + autoEvaluation: riskyEvaluation, }), ); + expect(insertAutoRejectedIntegrationToolApproval).not.toHaveBeenCalled(); - // Auto failing outright asks a person. + // An evaluation failure also asks while the owner is present. vi.mocked(resolveIntegrationToolAutoDecision).mockRejectedValue( new Error('settings unavailable'), ); const failing = helpers(); autoBridge().handleAsk({ ...ask, requestId: 'req-3' }, failing); - await vi.waitFor(() => expect(failing.reply).toHaveBeenCalled()); + await vi.waitFor(() => + expect(failing.reply).toHaveBeenCalledWith( + 'req-3', + 'reject', + 'The requester rejected this tool call.', + ), + ); expect(insertIntegrationToolApproval).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ nativeRequestId: 'req-3' }), + { sessionId: 'session-id', userId: 'user-id' }, + expect.objectContaining({ + nativeRequestId: 'req-3', + autoEvaluation: expect.objectContaining({ + recommendation: 'ask', + unavailable: 'error', + }), + }), ); + expect(insertAutoRejectedIntegrationToolApproval).not.toHaveBeenCalled(); // A manual Ask first tool (not in the Auto set) never reaches the model. vi.mocked(resolveIntegrationToolAutoDecision).mockClear(); @@ -905,6 +972,134 @@ describe('tool approval bridge', () => { expect(resolveIntegrationToolAutoDecision).not.toHaveBeenCalled(); }); + it.each([ + [ + 'risky', + { recommendation: 'ask' as const, answers: {}, evaluatedAt: '' }, + 'it was assessed as risky', + ], + [ + 'evaluation error', + { + recommendation: 'ask' as const, + unavailable: 'error' as const, + evaluatedAt: '', + }, + 'the automatic check failed', + ], + [ + 'no model', + { + recommendation: 'ask' as const, + unavailable: 'no_model' as const, + evaluatedAt: '', + }, + 'an automatic check is not available', + ], + ])( + 'denies an Auto %s call when the Session owner is absent', + async (_label, evaluation, reason) => { + redisMocks.isPresent.mockResolvedValue(false); + vi.mocked(resolveIntegrationToolAutoDecision).mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation, + }); + const helperMocks = helpers(); + createFastAgentToolApprovalBridge({ + sessionId: 'session-id', + userId: 'user-id', + surface: 'web', + integrations, + autoToolKeys: new Set([JSON.stringify(['mock-slack', 'post_message'])]), + }).handleAsk({ ...ask, requestId: `absent-${_label}` }, helperMocks); + + await vi.waitFor(() => + expect(helperMocks.reply).toHaveBeenCalledWith( + `absent-${_label}`, + 'reject', + expect.stringContaining('Auto mode blocked this tool call'), + ), + ); + expect(helperMocks.reply.mock.calls[0]![2]).toContain(reason); + expect(helperMocks.reply.mock.calls[0]![2]).toContain( + 'the session owner was away', + ); + expect(isSessionUserPresent).toHaveBeenCalledWith({ + sessionId: 'session-id', + userId: 'user-id', + }); + expect(insertAutoRejectedIntegrationToolApproval).toHaveBeenCalledWith( + { sessionId: 'session-id', userId: 'user-id' }, + expect.objectContaining({ autoEvaluation: evaluation }), + ); + expect(insertIntegrationToolApproval).not.toHaveBeenCalled(); + }, + ); + + it('asks when the Fast Session presence lookup fails', async () => { + const evaluation = { recommendation: 'ask' as const, evaluatedAt: '' }; + redisMocks.isPresent.mockRejectedValueOnce(new Error('redis unavailable')); + vi.mocked(resolveIntegrationToolAutoDecision).mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation, + }); + vi.mocked(getIntegrationToolApproval).mockResolvedValue({ + status: 'rejected', + } as never); + const helperMocks = helpers(); + createFastAgentToolApprovalBridge({ + sessionId: 'session-id', + userId: 'user-id', + surface: 'web', + integrations, + autoToolKeys: new Set([JSON.stringify(['mock-slack', 'post_message'])]), + }).handleAsk({ ...ask, requestId: 'presence-failure' }, helperMocks); + + await vi.waitFor(() => + expect(helperMocks.reply).toHaveBeenCalledWith( + 'presence-failure', + 'reject', + 'The requester rejected this tool call.', + ), + ); + expect(insertIntegrationToolApproval).toHaveBeenCalledWith( + { sessionId: 'session-id', userId: 'user-id' }, + expect.objectContaining({ autoEvaluation: evaluation }), + ); + expect(insertAutoRejectedIntegrationToolApproval).not.toHaveBeenCalled(); + }); + + it.each(['slack', 'discord', 'teams', 'telegram'] as const)( + 'treats a %s Session as present without browser presence', + async (surface) => { + redisMocks.isPresent.mockResolvedValue(false); + const evaluation = { recommendation: 'ask' as const, evaluatedAt: '' }; + vi.mocked(resolveIntegrationToolAutoDecision).mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation, + }); + vi.mocked(getIntegrationToolApproval).mockResolvedValue({ + status: 'rejected', + } as never); + const helperMocks = helpers(); + createFastAgentToolApprovalBridge({ + sessionId: 'session-id', + userId: 'user-id', + surface, + integrations, + autoToolKeys: new Set([JSON.stringify(['mock-slack', 'post_message'])]), + }).handleAsk({ ...ask, requestId: `chat-${surface}` }, helperMocks); + + await vi.waitFor(() => expect(helperMocks.reply).toHaveBeenCalled()); + expect(isSessionUserPresent).not.toHaveBeenCalled(); + expect(insertIntegrationToolApproval).toHaveBeenCalled(); + expect(insertAutoRejectedIntegrationToolApproval).not.toHaveBeenCalled(); + }, + ); + it('never consults Auto for a tool the requester asked to decide themselves', async () => { vi.mocked(listIntegrationToolSessionOverrides).mockResolvedValue([ { integrationId: 'mock-slack', toolName: 'post_message', mode: 'ask' }, @@ -916,6 +1111,7 @@ describe('tool approval bridge', () => { createFastAgentToolApprovalBridge({ sessionId: 'session-id', userId: 'user-id', + surface: 'web', integrations, autoToolKeys: new Set([JSON.stringify(['mock-slack', 'post_message'])]), }).handleAsk(ask, helperMocks); @@ -1205,6 +1401,7 @@ describe('tool approval bridge', () => { createFastAgentToolApprovalBridge({ sessionId: 'session-id', userId: 'user-id', + surface: 'web', integrations: colliding, }).handleAsk({ ...ask, permission: 'a_b_c' }, helperMocks as never); await vi.waitFor(() => @@ -1248,6 +1445,7 @@ describe('tool approval bridge', () => { createFastAgentToolApprovalBridge({ sessionId: 'session-id', userId: 'user-id', + surface: 'web', integrations: colliding, }).handleAsk({ ...ask, permission: 'a_b_c' }, helperMocks as never); await vi.waitFor(() => @@ -1269,6 +1467,7 @@ describe('tool approval bridge', () => { createFastAgentToolApprovalBridge({ sessionId: 'session-id', userId: 'user-id', + surface: 'web', integrations, notify, }).handleAsk(ask, helperMocks); diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts index b1916a48a7..c21bca9844 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts @@ -57,6 +57,7 @@ import { type EnvironmentRecipe, type IntegrationToolCandidate, type DataVisualizationInput, + type FastAgentSurface, CALL_INTEGRATION_TOOL_TOOL, FIND_INTEGRATION_TOOLS_TOOL, LIST_REPOSITORIES_MAX_LIMIT, @@ -225,6 +226,7 @@ import { } from './fast-agent-tool-policy'; import { createFastAgentToolApprovalBridge, + isFastAgentApprovalChatSurface, resolveFastAgentToolApprovalSession, integrationToolApprovalRulesToConfig, resolveFastAgentToolApprovalRules, @@ -6269,6 +6271,10 @@ export async function answerFastAgentQuestion({ inferenceAttemptNumber += 1; resolvedInferenceModel = undefined; captureInferenceContext('prompt_submission'); + const approvalNotificationSurface = + isFastAgentApprovalChatSurface(conversation.surface) + ? conversation.surface + : undefined; // Native per-tool approval bridge for gated code-mode // integration calls. Web conversations surface the pending // card in the Session transcript; chat-originated @@ -6282,16 +6288,16 @@ export async function answerFastAgentQuestion({ sessionId: toolApprovalSessionId, // The Session owner decides, even on a participant's turn. userId: toolApprovalDeciderUserId, + surface: conversation.surface as FastAgentSurface, integrations: availableIntegrations, autoToolKeys: toolApprovalRules.autoToolKeys, userRequest: question, signal: promptSignal, - ...(conversation.surface === 'slack' || - conversation.surface === 'discord' + ...(approvalNotificationSurface ? { notify: async (approval) => { const sessionUrl = buildFastSessionUrl( - conversation.surface as 'slack' | 'discord', + approvalNotificationSurface, session.id, ); await adapter.postReply({ diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts index 841a0ead79..dbe4711f23 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts @@ -10,6 +10,7 @@ import { getIntegrationToolApproval, getSessionForFastConversation, insertAutoApprovedIntegrationToolApproval, + insertAutoRejectedIntegrationToolApproval, insertIntegrationToolApproval, isDeploymentExperimentEnabled, listIntegrationToolPolicies, @@ -17,17 +18,20 @@ import { listIntegrationToolUserPolicies, markIntegrationToolApprovalConsumed, } from '@roomote/db/server'; +import { isSessionUserPresent } from '@roomote/redis'; import { integrationToolModeIsAutoAssessed, integrationToolPolicyKey, resolveEffectiveIntegrationToolMode, resolveGoverningIntegrationToolPolicies, + type FastAgentSurface, type IntegrationToolApprovalMetadata, type IntegrationToolPolicyMetadata, type IntegrationToolSessionOverrideMetadata, } from '@roomote/types'; import { + describeIntegrationToolAutoDeny, resolveIntegrationToolAutoDecision, resolveIntegrationToolAutoState, } from '../integration-tool-auto-evaluation'; @@ -58,6 +62,23 @@ import { buildFastAgentCodeModeServerNames } from './fast-agent-tool-policy'; const INTEGRATION_TOOL_APPROVAL_POLL_MS = 1_500; const INTEGRATION_TOOL_APPROVAL_CANCEL_EXPERIMENT_DISABLED = 'experiment_disabled'; +const SESSION_PRESENCE_LOOKUP_TIMEOUT_MS = 2_000; + +type FastAgentApprovalChatSurface = Extract< + FastAgentSurface, + 'slack' | 'discord' | 'teams' | 'telegram' +>; + +export function isFastAgentApprovalChatSurface( + surface: FastAgentSurface, +): surface is FastAgentApprovalChatSurface { + return ( + surface === 'slack' || + surface === 'discord' || + surface === 'teams' || + surface === 'telegram' + ); +} /** * OpenCode flattens every MCP tool to `_`. The server @@ -365,6 +386,7 @@ export async function resolveFastAgentToolApprovalSession( export function createFastAgentToolApprovalBridge(input: { sessionId: string; userId: string; + surface: FastAgentSurface; integrations: FastAgentIntegration[]; /** * Tools that ask only because Auto mode is on. Their asks are assessed @@ -387,6 +409,35 @@ export function createFastAgentToolApprovalBridge(input: { const handledRequestIds = new Set(); const notifiedApprovalIds = new Set(); + const ownerIsPresent = async (): Promise => { + if (isFastAgentApprovalChatSurface(input.surface)) return true; + let timeout: ReturnType | undefined; + try { + return await Promise.race([ + isSessionUserPresent({ + sessionId: input.sessionId, + userId: input.userId, + }), + new Promise((resolve) => { + timeout = setTimeout(() => { + console.warn( + `[Fast Agent] Presence lookup timed out for Session ${input.sessionId}; asking defensively.`, + ); + resolve(true); + }, SESSION_PRESENCE_LOOKUP_TIMEOUT_MS); + timeout.unref?.(); + }), + ]); + } catch (error) { + console.warn( + `[Fast Agent] Presence lookup failed for Session ${input.sessionId}; asking defensively: ${error instanceof Error ? error.message : String(error)}`, + ); + return true; + } finally { + if (timeout) clearTimeout(timeout); + } + }; + // A code-mode child call's dotted name (`server.tool`) is unambiguous even // when its flattened permission key (`server_tool`) is not, so it is the // identity source for colliding keys. Flattening replaces the first dot. @@ -505,7 +556,8 @@ export function createFastAgentToolApprovalBridge(input: { // Auto mode: a call to a default tool is risk-assessed, and a routine // one runs without a card. A tool someone made a choice about (a // stored mode or a session override) is theirs to decide, so it never - // reaches the model. Any failure on this path asks a person. + // reaches the model. A risky or unavailable assessment asks the Session + // owner when present and is denied when they are away. const autoAssessed = !overrideForSession && input.autoToolKeys?.has( @@ -519,15 +571,49 @@ export function createFastAgentToolApprovalBridge(input: { args, userRequest: input.userRequest, userId: input.userId, - }).catch(() => ({ action: 'ask' as const, mode: 'failed' as const })) + }).catch(() => ({ + action: 'ask' as const, + mode: 'on' as const, + evaluation: { + recommendation: 'ask' as const, + unavailable: 'error' as const, + evaluatedAt: new Date().toISOString(), + }, + })) : undefined; // A default tool asked under a rule compiled while Auto was on, after // Auto went off: it runs as it always has, and there is nothing to - // record. A failed assessment asks a person instead. + // record. if (auto?.mode === 'off') { await helpers.reply(ask.requestId, 'once'); return; } + if (auto?.action === 'ask' && !(await ownerIsPresent())) { + // The audit row is born terminal `auto_rejected` with the assessment; + // if it cannot be written the outer handler rejects the ask instead + // of denying it unrecorded. No card is shown while the owner is away. + await insertAutoRejectedIntegrationToolApproval( + { sessionId: input.sessionId, userId: input.userId }, + { + integrationId: tool.integrationId, + toolName: tool.toolName, + nativeRequestId: ask.requestId, + argsFingerprint, + argsSummary: args ?? null, + autoEvaluation: auto.evaluation, + }, + ); + await helpers + .reply( + ask.requestId, + 'reject', + `Auto mode blocked this tool call because ${describeIntegrationToolAutoDeny( + auto.evaluation, + )} and the session owner was away. The call was not run. The session owner can allow this tool from its call in the transcript.`, + ) + .catch(() => undefined); + return; + } if (allowedForSession || auto?.action === 'approve') { // The audit row starts as an unrelayed `approved` decision; claiming // it is the atomic reservation. The claim reads the experiment under @@ -575,7 +661,9 @@ export function createFastAgentToolApprovalBridge(input: { nativeRequestId: ask.requestId, argsFingerprint, argsSummary: args ?? null, - ...(auto?.mode === 'on' ? { autoEvaluation: auto.evaluation } : {}), + ...(auto?.action === 'ask' + ? { autoEvaluation: auto.evaluation } + : {}), }, ); if (input.notify && !notifiedApprovalIds.has(approval.approvalId)) { diff --git a/packages/cloud-agents/src/server/integration-tool-auto-evaluation.ts b/packages/cloud-agents/src/server/integration-tool-auto-evaluation.ts index e94b4a5c82..390acc4e73 100644 --- a/packages/cloud-agents/src/server/integration-tool-auto-evaluation.ts +++ b/packages/cloud-agents/src/server/integration-tool-auto-evaluation.ts @@ -82,8 +82,9 @@ export type AutoRiskAnswers = { * Run without a person only when the call reads and changes nothing (with * confidence), is what the user asked for when that is known, is not steered * by untrusted content, and the deployment's guidance does not flag it. - * Anything less asks. The model can only ever recommend running the call or - * asking, never rejecting. + * Anything less asks a person. The model can only ever recommend running the + * call or asking a person; presence decides whether that ask becomes a card + * or a denial. */ export function recommendFromAutoAnswers( answers: AutoRiskAnswers, @@ -178,11 +179,14 @@ export async function evaluateIntegrationToolAutoDecision(input: { } /** - * What Auto mode is doing right now. `on` needs the experiment, the setting, - * and a hosted judgment model: the helper-model fallback is an LLM call per - * tool call, which is never turned on implicitly. `shadow` is the same - * assessment recorded without acting, while Auto is off and a hosted model - * is there to do it cheaply. + * What Auto mode is doing right now. `on` is the experiment plus the setting: + * every default tool call is gated and must be assessed before it runs. `on` + * does not imply a hosted judgment model is configured — the On control is + * disabled without one, but the setting can outlive the model, and callers + * must treat `on` without a judgment model as an ask for a present owner and + * a denial for an absent owner. `shadow` is the same assessment recorded + * without acting, while Auto is off and a hosted model is there to do it + * cheaply. */ export type IntegrationToolAutoState = { mode: 'off' | 'shadow' | 'on'; @@ -197,8 +201,13 @@ export async function resolveIntegrationToolAutoState(): Promise null), ]); const hosted = model?.kind === 'judgment'; - const mode = - !enabled || !hosted ? 'off' : settings.mode === 'on' ? 'on' : 'shadow'; + const mode = !enabled + ? 'off' + : settings.mode === 'on' + ? 'on' + : hosted + ? 'shadow' + : 'off'; return { mode, settings, model: model?.kind ?? null }; } @@ -244,23 +253,55 @@ export function recordIntegrationToolShadowEvaluationInBackground(input: { } export type IntegrationToolAutoDecision = - | { action: 'ask'; mode: 'off' } + | { action: 'run'; mode: 'off' } | { action: 'approve' | 'ask'; mode: 'on'; evaluation: IntegrationToolAutoEvaluation; }; +/** + * The short, model-facing reason an Auto denial happened, suitable for the + * tool error returned to the agent. + */ +export function describeIntegrationToolAutoDeny( + evaluation: IntegrationToolAutoEvaluation, +): string { + if (evaluation.unavailable === 'no_model') { + return 'an automatic check is not available'; + } + if (evaluation.unavailable === 'error') { + return 'the automatic check failed'; + } + return 'it was assessed as risky'; +} + /** * How Auto treats one call to a default tool. `approve` means the call is - * routine enough to run without a card. Only `on` can produce it, and only - * after the assessment has actually run, so an error still asks. + * routine enough to run; anything else — a risky assessment, an evaluation + * error, or Auto on without a judgment model to assess with — asks the owner. + * Presence decides whether that ask becomes a card or a denial. Only `off` + * (Auto disabled) lets the call run unassessed. */ export async function resolveIntegrationToolAutoDecision( input: Parameters[0], ): Promise { const state = await resolveIntegrationToolAutoState(); - if (state.mode !== 'on') return { action: 'ask', mode: 'off' }; + if (state.mode !== 'on') return { action: 'run', mode: 'off' }; + if (state.model !== 'judgment') { + // Auto is on but nothing can assess the call. The helper model fallback is + // an LLM call per tool call and is never used here; ask/deny is decided by + // the Session owner's presence at the enforcement point. + return { + action: 'ask', + mode: 'on', + evaluation: { + recommendation: 'ask', + unavailable: 'no_model', + evaluatedAt: new Date().toISOString(), + }, + }; + } const evaluation = await evaluateIntegrationToolAutoDecision({ ...input, deploymentGuidance: state.settings.policy, diff --git a/packages/db/src/lib/__tests__/integration-tool-approvals.test.ts b/packages/db/src/lib/__tests__/integration-tool-approvals.test.ts index 5555018248..5c9605ad35 100644 --- a/packages/db/src/lib/__tests__/integration-tool-approvals.test.ts +++ b/packages/db/src/lib/__tests__/integration-tool-approvals.test.ts @@ -17,6 +17,7 @@ import { fingerprintIntegrationToolCall, getIntegrationToolApproval, insertAutoApprovedIntegrationToolApproval, + insertAutoRejectedIntegrationToolApproval, insertIntegrationToolApproval, IntegrationToolApprovalUnavailableError, listIntegrationToolPolicies, @@ -589,6 +590,51 @@ describe('auto-approved reservations', () => { }); }); +describe('auto-rejected audit rows', () => { + it('inserts a born-terminal auto_rejected row with the model assessment and no decider', async () => { + const userId = await user(); + const sessionId = await ownedSession(userId); + const evaluation = { + recommendation: 'ask' as const, + answers: { riskScore: 0.9 }, + evaluatedAt: new Date().toISOString(), + }; + const denial = await insertAutoRejectedIntegrationToolApproval( + { sessionId, userId }, + { + integrationId: call.integrationId, + toolName: call.toolName, + nativeRequestId: nextNativeRequestId(), + argsFingerprint: fingerprint(), + argsSummary: call.args, + autoEvaluation: evaluation, + }, + ); + expect(denial.status).toBe('auto_rejected'); + const row = await getIntegrationToolApproval(denial.approvalId); + expect(row?.status).toBe('auto_rejected'); + expect(row?.decidedByUserId).toBeNull(); + expect(row?.decidedAt).not.toBeNull(); + expect(row?.autoEvaluation).toMatchObject({ recommendation: 'ask' }); + // Terminal: nothing can claim, decide, or cancel it into a run. + await expect( + claimAutoApprovedIntegrationToolApproval({ + approvalId: denial.approvalId, + requesterUserId: userId, + }), + ).resolves.toBe(false); + await expect( + decideIntegrationToolApproval( + { sessionId, userId }, + { approvalId: denial.approvalId, decision: 'approved' }, + ), + ).rejects.toThrow(); + expect((await getIntegrationToolApproval(denial.approvalId))?.status).toBe( + 'auto_rejected', + ); + }); +}); + describe('Auto mode', () => { it('keeps the deployment Auto settings, defaulting to off', async () => { await expect(getIntegrationToolAutoSettings()).resolves.toEqual({ diff --git a/packages/db/src/lib/integration-tool-approvals.ts b/packages/db/src/lib/integration-tool-approvals.ts index c647b2a969..3d4267191d 100644 --- a/packages/db/src/lib/integration-tool-approvals.ts +++ b/packages/db/src/lib/integration-tool-approvals.ts @@ -750,6 +750,51 @@ export async function insertAutoApprovedIntegrationToolApproval( }); } +/** + * Audit row for a call Auto mode denied: born terminal `auto_rejected`, so + * no later decision or relay can ever run it, and the model's assessment is + * recorded beside the denial. `decidedByUserId` stays null: the decision + * model denied, not a person. + */ +export async function insertAutoRejectedIntegrationToolApproval( + context: { sessionId: string; userId: string }, + input: { + integrationId: string; + toolName: string; + nativeRequestId: string; + argsFingerprint: string; + argsSummary: unknown; + /** Set when a task's agent asked; see `claimTaskIntegrationToolCall`. */ + taskId?: string; + /** The decision model's assessment, or why it could not assess. */ + autoEvaluation: IntegrationToolAutoEvaluation; + }, +): Promise { + return db.transaction(async (tx) => { + const owner = await requireSessionOwner(tx, context); + const [row] = await tx + .insert(integrationToolApprovalRequests) + .values({ + sessionId: context.sessionId, + requesterUserId: owner.id, + taskId: input.taskId ?? null, + autoEvaluation: input.autoEvaluation, + integrationId: input.integrationId, + toolName: input.toolName, + nativeRequestId: input.nativeRequestId, + argsFingerprint: input.argsFingerprint, + argsSummary: redactIntegrationToolArgs(input.argsSummary), + status: 'auto_rejected', + decidedByUserId: null, + decidedAt: sql`clock_timestamp()`, + expiresAt: sql`clock_timestamp()`, + }) + .returning(); + if (!row) throw new IntegrationToolApprovalUnavailableError('write_failed'); + return approvalMetadata(row); + }); +} + /** * Atomically claim an unrelayed auto-approval for relay: only an `approved`, * unclaimed row transitions to terminal `auto_approved`. The experiment diff --git a/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts b/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts index fa735bb964..7e69511650 100644 --- a/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts +++ b/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts @@ -1,5 +1,5 @@ const mocks = vi.hoisted(() => ({ - resolveAuto: vi.fn(async () => ({ action: 'ask', mode: 'off' }) as unknown), + resolveAuto: vi.fn(async () => ({ action: 'run', mode: 'off' }) as unknown), autoState: vi.fn(async () => ({ mode: 'off' }) as unknown), experiment: vi.fn(async () => true), findRun: vi.fn(async () => ({ taskId: 'task-1' }) as unknown), @@ -9,14 +9,22 @@ const mocks = vi.hoisted(() => ({ overrides: vi.fn(async () => [] as unknown[]), insert: vi.fn(async () => ({ approvalId: 'approval-1' })), insertAuto: vi.fn(async () => ({ approvalId: 'approval-auto' })), + insertAutoRejected: vi.fn(async () => ({ approvalId: 'approval-denied' })), claimAuto: vi.fn(async () => true), getApproval: vi.fn(async () => undefined as unknown), expire: vi.fn(async () => undefined), + isPresent: vi.fn(async () => true), })); vi.mock( '@roomote/cloud-agents/server/integration-tool-auto-evaluation', () => ({ + describeIntegrationToolAutoDeny: (evaluation: { unavailable?: string }) => + evaluation.unavailable === 'no_model' + ? 'an automatic check is not available' + : evaluation.unavailable === 'error' + ? 'the automatic check failed' + : 'it was assessed as risky', resolveIntegrationToolAutoDecision: mocks.resolveAuto, resolveIntegrationToolAutoState: mocks.autoState, }), @@ -33,22 +41,28 @@ vi.mock('@roomote/db/server', () => ({ listIntegrationToolSessionOverrides: mocks.overrides, insertIntegrationToolApproval: mocks.insert, insertAutoApprovedIntegrationToolApproval: mocks.insertAuto, + insertAutoRejectedIntegrationToolApproval: mocks.insertAutoRejected, claimAutoApprovedIntegrationToolApproval: mocks.claimAuto, getIntegrationToolApproval: mocks.getApproval, expireIntegrationToolApproval: mocks.expire, fingerprintIntegrationToolCall: (input: unknown) => JSON.stringify(input), })); +vi.mock('@roomote/redis', () => ({ + isSessionUserPresent: mocks.isPresent, +})); import { getTaskToolApprovalStatus, requestTaskToolApproval, resolveTaskIntegrationToolApprovals, } from '../task-tool-approvals'; +import { isSessionUserPresent } from '@roomote/redis'; const ownedSession = { id: 'session-1', ownerKind: 'user', ownerUserId: 'owner-1', + sourceSurface: 'web', }; const ask = { runId: 7, @@ -74,8 +88,9 @@ beforeEach(() => { mocks.userPolicies.mockResolvedValue([]); mocks.overrides.mockResolvedValue([]); mocks.claimAuto.mockResolvedValue(true); - mocks.resolveAuto.mockResolvedValue({ action: 'ask', mode: 'off' }); + mocks.resolveAuto.mockResolvedValue({ action: 'run', mode: 'off' }); mocks.autoState.mockResolvedValue({ mode: 'off' }); + mocks.isPresent.mockResolvedValue(true); }); describe('resolveTaskIntegrationToolApprovals', () => { @@ -195,39 +210,140 @@ describe('requestTaskToolApproval', () => { expect(mocks.claimAuto).not.toHaveBeenCalled(); expect(mocks.insert).not.toHaveBeenCalled(); - // Risky: the card, with the model's view on it. + // Risky: the present owner gets a card with the assessment. + const riskyEvaluation = { ...evaluation, recommendation: 'ask' }; mocks.resolveAuto.mockResolvedValue({ action: 'ask', mode: 'on', - evaluation: { ...evaluation, recommendation: 'ask' }, + evaluation: riskyEvaluation, }); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ outcome: 'pending', approvalId: 'approval-1', }); - expect(mocks.insert).toHaveBeenLastCalledWith( - expect.anything(), + expect(mocks.insert).toHaveBeenCalledWith( + { sessionId: 'session-1', userId: 'owner-1' }, expect.objectContaining({ - autoEvaluation: { ...evaluation, recommendation: 'ask' }, + taskId: 'task-1', + nativeRequestId: 'per_1', + autoEvaluation: riskyEvaluation, }), ); + expect(mocks.insertAutoRejected).not.toHaveBeenCalled(); - // Auto failing outright asks a person. + // Auto failing outright still asks a present owner. mocks.resolveAuto.mockRejectedValue(new Error('settings unavailable')); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ outcome: 'pending', approvalId: 'approval-1', }); + expect(mocks.insert).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ + autoEvaluation: expect.objectContaining({ + recommendation: 'ask', + unavailable: 'error', + }), + }), + ); + expect(mocks.insertAutoRejected).not.toHaveBeenCalled(); }); it('runs a default tool asked under a stale rule once Auto is off', async () => { - mocks.resolveAuto.mockResolvedValue({ action: 'ask', mode: 'off' }); + mocks.resolveAuto.mockResolvedValue({ action: 'run', mode: 'off' }); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ outcome: 'not_required', }); expect(mocks.insert).not.toHaveBeenCalled(); }); + it.each([ + [ + 'risky', + { recommendation: 'ask', answers: { riskScore: 0.9 }, evaluatedAt: '' }, + 'it was assessed as risky', + ], + [ + 'evaluation error', + { recommendation: 'ask', unavailable: 'error', evaluatedAt: '' }, + 'the automatic check failed', + ], + [ + 'no model', + { recommendation: 'ask', unavailable: 'no_model', evaluatedAt: '' }, + 'an automatic check is not available', + ], + ])( + 'denies an Auto %s call when the Session owner is absent', + async (_label, evaluation, reason) => { + mocks.isPresent.mockResolvedValue(false); + mocks.resolveAuto.mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation, + }); + + await expect(requestTaskToolApproval(ask)).resolves.toEqual({ + outcome: 'denied', + reason, + }); + expect(isSessionUserPresent).toHaveBeenCalledWith({ + sessionId: 'session-1', + userId: 'owner-1', + }); + expect(mocks.insertAutoRejected).toHaveBeenCalledWith( + { sessionId: 'session-1', userId: 'owner-1' }, + expect.objectContaining({ + taskId: 'task-1', + autoEvaluation: evaluation, + }), + ); + expect(mocks.insert).not.toHaveBeenCalled(); + }, + ); + + it('asks when the task presence lookup fails', async () => { + const evaluation = { recommendation: 'ask', evaluatedAt: '' }; + mocks.resolveAuto.mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation, + }); + mocks.isPresent.mockRejectedValueOnce(new Error('redis unavailable')); + + await expect(requestTaskToolApproval(ask)).resolves.toEqual({ + outcome: 'pending', + approvalId: 'approval-1', + }); + expect(mocks.insert).toHaveBeenCalledWith( + { sessionId: 'session-1', userId: 'owner-1' }, + expect.objectContaining({ autoEvaluation: evaluation }), + ); + expect(mocks.insertAutoRejected).not.toHaveBeenCalled(); + }); + + it.each(['slack', 'discord', 'teams', 'telegram'] as const)( + 'treats a %s task Session as present without browser presence', + async (sourceSurface) => { + mocks.sessionForTask.mockResolvedValue({ + ...ownedSession, + sourceSurface, + }); + const evaluation = { recommendation: 'ask', evaluatedAt: '' }; + mocks.resolveAuto.mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation, + }); + + await expect(requestTaskToolApproval(ask)).resolves.toEqual({ + outcome: 'pending', + approvalId: 'approval-1', + }); + expect(isSessionUserPresent).not.toHaveBeenCalled(); + }, + ); + it('runs a tool the owner chose to always allow, without the model', async () => { mocks.userPolicies.mockResolvedValue([ policy('save_issue', 'always_allow'), diff --git a/packages/sdk/src/server/lib/task-tool-approvals.ts b/packages/sdk/src/server/lib/task-tool-approvals.ts index e4b44ee6b7..cd416e6827 100644 --- a/packages/sdk/src/server/lib/task-tool-approvals.ts +++ b/packages/sdk/src/server/lib/task-tool-approvals.ts @@ -7,6 +7,7 @@ import { getIntegrationToolApproval, getSessionForTask, insertAutoApprovedIntegrationToolApproval, + insertAutoRejectedIntegrationToolApproval, insertIntegrationToolApproval, isDeploymentExperimentEnabled, listIntegrationToolPolicies, @@ -14,7 +15,9 @@ import { listIntegrationToolUserPolicies, taskRuns, } from '@roomote/db/server'; +import { isSessionUserPresent } from '@roomote/redis'; import { + describeIntegrationToolAutoDeny, resolveIntegrationToolAutoDecision, resolveIntegrationToolAutoState, } from '@roomote/cloud-agents/server/integration-tool-auto-evaluation'; @@ -28,6 +31,48 @@ import { type TaskIntegrationToolApprovals, } from '@roomote/types'; +const SESSION_PRESENCE_LOOKUP_TIMEOUT_MS = 2_000; + +async function isTaskSessionOwnerPresent(input: { + sessionId: string; + userId: string; + sourceSurface: string | null | undefined; +}): Promise { + if ( + input.sourceSurface === 'slack' || + input.sourceSurface === 'discord' || + input.sourceSurface === 'teams' || + input.sourceSurface === 'telegram' + ) { + return true; + } + let timeout: ReturnType | undefined; + try { + return await Promise.race([ + isSessionUserPresent({ + sessionId: input.sessionId, + userId: input.userId, + }), + new Promise((resolve) => { + timeout = setTimeout(() => { + console.warn( + `[Task tool approvals] Presence lookup timed out for Session ${input.sessionId}; asking defensively.`, + ); + resolve(true); + }, SESSION_PRESENCE_LOOKUP_TIMEOUT_MS); + timeout.unref?.(); + }), + ]); + } catch (error) { + console.warn( + `[Task tool approvals] Presence lookup failed for Session ${input.sessionId}; asking defensively: ${error instanceof Error ? error.message : String(error)}`, + ); + return true; + } finally { + if (timeout) clearTimeout(timeout); + } +} + /** * Experiment-gated (`integrationToolApprovals`) approvals for a task's agent. * A task belongs to one Session, so its asks are recorded on that Session, @@ -48,6 +93,7 @@ async function resolveTaskApprovalSession(runId: number) { sessionId: session.id, ownerUserId: session.ownerKind === 'user' ? (session.ownerUserId ?? null) : null, + sourceSurface: session.sourceSurface, }; } @@ -119,6 +165,8 @@ type TaskToolApprovalRequestResult = | { outcome: 'unavailable' } /** The Session owner already chose not to be asked about this tool. */ | { outcome: 'approved' } + /** Auto mode blocked the call; the reason goes back to the model. */ + | { outcome: 'denied'; reason: string } | { outcome: 'pending'; approvalId: string }; /** Record one native ask from a task's agent. */ @@ -171,7 +219,8 @@ export async function requestTaskToolApproval(input: { const allowedForSession = overrideForSession === 'allow' || effectiveMode === 'always_allow'; // Auto mode assesses a call to a default tool only; a tool someone made a - // choice about is theirs to decide. Any failure on this path asks a person. + // choice about is theirs to decide. A risky or unavailable assessment asks + // the Session owner when present and is denied when they are away. const auto = integrationToolModeIsAutoAssessed({ policyMode, sessionOverrideMode: overrideForSession, @@ -183,10 +232,18 @@ export async function requestTaskToolApproval(input: { userRequest: input.userRequest, userId: session.ownerUserId, taskId: session.taskId, - }).catch(() => ({ action: 'ask' as const, mode: 'failed' as const })) + }).catch(() => ({ + action: 'ask' as const, + mode: 'on' as const, + evaluation: { + recommendation: 'ask' as const, + unavailable: 'error' as const, + evaluatedAt: new Date().toISOString(), + }, + })) : undefined; // A default tool asked while Auto is off (a stale native rule) runs as it - // always has; a failed assessment asks a person instead. + // always has. if (auto?.mode === 'off') return { outcome: 'not_required' }; if (allowedForSession) { // Same reservation-and-claim audit path as a Session's own agent. @@ -210,9 +267,27 @@ export async function requestTaskToolApproval(input: { }); return { outcome: 'approved' }; } + if (auto?.action === 'ask') { + const ownerPresent = await isTaskSessionOwnerPresent({ + sessionId: session.sessionId, + userId: session.ownerUserId, + sourceSurface: session.sourceSurface, + }); + if (!ownerPresent) { + // Born-terminal audit row; no card is shown while the owner is away. + await insertAutoRejectedIntegrationToolApproval(context, { + ...call, + autoEvaluation: auto.evaluation, + }); + return { + outcome: 'denied', + reason: describeIntegrationToolAutoDeny(auto.evaluation), + }; + } + } const approval = await insertIntegrationToolApproval(context, { ...call, - ...(auto?.mode === 'on' ? { autoEvaluation: auto.evaluation } : {}), + ...(auto?.action === 'ask' ? { autoEvaluation: auto.evaluation } : {}), }); return { outcome: 'pending', approvalId: approval.approvalId }; } diff --git a/packages/types/src/integration-tool-approvals.ts b/packages/types/src/integration-tool-approvals.ts index 69034a630f..df97008afb 100644 --- a/packages/types/src/integration-tool-approvals.ts +++ b/packages/types/src/integration-tool-approvals.ts @@ -49,6 +49,9 @@ export const INTEGRATION_TOOL_APPROVAL_STATUSES = [ // session" for the tool, or Auto mode's decision model approved the call // (then `decidedByUserId` is null). Its own status for the audit trail. 'auto_approved', + // Blocked without a card: Auto mode asked, but the Session owner was away. + // Born terminal; `decidedByUserId` is null. + 'auto_rejected', ] as const; export type IntegrationToolApprovalStatus = (typeof INTEGRATION_TOOL_APPROVAL_STATUSES)[number]; @@ -66,7 +69,7 @@ export interface IntegrationToolApprovalMetadata { status: IntegrationToolApprovalStatus; /** The task whose agent asked; null when the Session's own agent did. */ taskId: string | null; - /** Auto mode's view of the call, when it was consulted before this card. */ + /** Auto mode's assessment of the call, on the rows it decided. */ autoEvaluation?: IntegrationToolAutoEvaluation; /** When this approval stops accepting a decision and fails closed. */ expiresAt: string; @@ -76,10 +79,11 @@ export interface IntegrationToolApprovalMetadata { /** * Deployment-wide Auto mode. `on`: every call to a tool nobody has made a * choice about (the default mode) is risk-assessed by the decision model - * first; a routine call runs, a risky one asks a person. A manual choice - * always wins: Always allow is never assessed, Ask first always asks, Reject - * always blocks. `off`: default tools run as they always have. While off, - * and only with a hosted judgment model configured, the assessment still + * first; a routine call runs, anything else asks the Session owner when they + * are present and is blocked with a tool error when they are away. A manual + * choice always wins: Always allow is never assessed, Ask first always asks, + * Reject always blocks. `off`: default tools run as they always have. While + * off, and only with a hosted judgment model configured, the assessment still * runs in the background and is recorded, so its judgment can be checked * against real calls before it is turned on. */ @@ -108,9 +112,9 @@ export const integrationToolAutoSettingsSchema = z.object({ }); /** - * A decision model's risk assessment of one paused Ask first call, recorded - * beside the decision. In shadow mode it decides nothing. The model can only - * ever recommend running the call or asking, never rejecting. + * A decision model's risk assessment of one Auto-gated call, recorded on the + * call's audit row. In shadow mode it decides nothing. The assessment can + * recommend running the call or asking its owner. */ export interface IntegrationToolAutoEvaluation { recommendation: 'approve' | 'ask';