From db2a15766a4cebe2f0b30a85b0faba0cf9e548a8 Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 16:50:32 +0000 Subject: [PATCH 1/4] feat: enforce allow/deny in Auto integration tool approval mode --- .changeset/auto-approval-allow-deny.md | 5 ++ ...ngIntegrationToolApprovals.client.test.tsx | 23 ------ .../PendingIntegrationToolApprovals.tsx | 28 ------- ...rationToolApprovalsExperimentalSetting.tsx | 6 +- .../IntegrationToolAutoModeSetting.tsx | 6 +- .../opencode-server-tool-approvals.test.ts | 18 +++++ .../opencode-server/tool-approvals.ts | 8 ++ .../integration-tool-auto-evaluation.test.ts | 50 +++++++++---- .../fast-agent-tool-approvals.test.ts | 72 +++++++++++++----- .../fast-agent/fast-agent-tool-approvals.ts | 45 ++++++++++- .../integration-tool-auto-evaluation.ts | 74 ++++++++++++++----- .../integration-tool-approvals.test.ts | 46 ++++++++++++ .../db/src/lib/integration-tool-approvals.ts | 45 +++++++++++ .../lib/__tests__/task-tool-approvals.test.ts | 51 +++++++++---- .../sdk/src/server/lib/task-tool-approvals.ts | 35 +++++++-- .../types/src/integration-tool-approvals.ts | 18 +++-- 16 files changed, 391 insertions(+), 139 deletions(-) create mode 100644 .changeset/auto-approval-allow-deny.md diff --git a/.changeset/auto-approval-allow-deny.md b/.changeset/auto-approval-allow-deny.md new file mode 100644 index 0000000000..de6073e2ba --- /dev/null +++ b/.changeset/auto-approval-allow-deny.md @@ -0,0 +1,5 @@ +--- +"@roomote/web": patch +--- + +Auto mode for integration tool approvals now enforces allow/deny: a call the decision model does not approve is blocked with a tool error returned to the agent instead of pausing on an approval card, and any evaluation failure or missing decision model fails closed to the same denial. diff --git a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx index 57805a6c4d..1dd855f11f 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx @@ -140,27 +140,4 @@ describe('PendingIntegrationToolApprovals', () => { expect(screen.getByText('No additional details.')).toBeInTheDocument(); expect(screen.queryByText('No arguments')).not.toBeInTheDocument(); }); - - it('tells the person what Auto made of the call, when it looked', () => { - render( - - - , - ); - expect(screen.getByTestId('auto-evaluation')).toHaveTextContent( - 'Auto flagged this call as risky and asked you.', - ); - }); }); diff --git a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx index ccb8de1190..b8e6b247d5 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx @@ -14,7 +14,6 @@ import { import { MCP_INTEGRATIONS, type IntegrationToolApprovalMetadata, - type IntegrationToolAutoEvaluation, } from '@roomote/types'; const integrationNames = new Map( @@ -65,25 +64,6 @@ 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. - */ -function describeAutoEvaluation( - evaluation: IntegrationToolAutoEvaluation, -): string { - if (evaluation.unavailable === 'no_model') { - return 'Auto couldn’t check this call because no decision model is available.'; - } - if (evaluation.unavailable === 'error') { - return 'Auto couldn’t check this call.'; - } - return evaluation.recommendation === 'approve' - ? 'Auto would have run this call.' - : 'Auto flagged this call as risky and asked you.'; -} - function summarizeArgs(argsSummary: unknown): string | null { if ( argsSummary === null || @@ -161,14 +141,6 @@ export function PendingIntegrationToolApprovals({

Roomote is waiting for your approval to continue.

- {item.autoEvaluation ? ( -

- {describeAutoEvaluation(item.autoEvaluation)} -

- ) : null} diff --git a/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx b/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx index 8a96609a11..a2daf839b7 100644 --- a/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx +++ b/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx @@ -29,9 +29,9 @@ 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 + work automatically and block 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 diff --git a/apps/web/src/components/settings/IntegrationToolAutoModeSetting.tsx b/apps/web/src/components/settings/IntegrationToolAutoModeSetting.tsx index 81b9f98ad4..80a16d90ee 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 block anything risky. 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 block anything risky.', disclosure: 'Additional instructions', guidanceLabel: 'Additional instructions', guidanceHelp: @@ -46,7 +46,7 @@ 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. + * or block it; it never asks a person. */ 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..264f82c173 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', + ), + }), + ); + }); + 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..89e337e626 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}. The call was not run, and this call cannot proceed.`, + ); + 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..c7821e6784 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 @@ -68,7 +68,7 @@ beforeEach(() => { }); describe('recommendFromAutoAnswers', () => { - it('runs only a routine call the user asked for; every doubt asks', () => { + it('runs only a routine call the user asked for; every doubt denies', () => { expect(recommendFromAutoAnswers(routine)).toBe('approve'); // With no request to judge against, the other signals decide. expect( @@ -82,7 +82,7 @@ describe('recommendFromAutoAnswers', () => { { steeredByUntrustedContent: 0.4 }, { guidanceFlagsRisk: 0.5 }, ] satisfies Partial[]) { - expect(recommendFromAutoAnswers({ ...routine, ...doubt })).toBe('ask'); + expect(recommendFromAutoAnswers({ ...routine, ...doubt })).toBe('deny'); } }); }); @@ -146,7 +146,7 @@ describe('evaluateIntegrationToolAutoDecision', () => { ); const guided = await evaluateIntegrationToolAutoDecision(call); expect(guided).toMatchObject({ - recommendation: 'ask', + recommendation: 'deny', answers: { guidanceFlagsRisk: 0.9 }, }); const { state, questions } = mocks.evaluate.mock.calls[1]![0]; @@ -156,24 +156,24 @@ describe('evaluateIntegrationToolAutoDecision', () => { expect(questions.guidanceFlagsRisk).toBeDefined(); }); - it('falls back to asking with no model or a failed evaluation', async () => { + it('fails closed to deny with no model or a failed evaluation', async () => { mocks.evaluate.mockResolvedValue(null); await expect( evaluateIntegrationToolAutoDecision(call), ).resolves.toMatchObject({ - recommendation: 'ask', + recommendation: 'deny', unavailable: 'no_model', }); mocks.evaluate.mockRejectedValue(new Error('timeout')); await expect( evaluateIntegrationToolAutoDecision(call), - ).resolves.toMatchObject({ recommendation: 'ask', unavailable: 'error' }); + ).resolves.toMatchObject({ recommendation: 'deny', unavailable: 'error' }); }); }); 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 fail closed + // 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,20 +283,38 @@ describe('resolveIntegrationToolAutoDecision', () => { 'Reads are routine.', ); - // Risky, or no model at all: the card shows. + // Risky, or a failed evaluation: the call is denied, never asked. mocks.evaluate.mockResolvedValue( modelAnswers({ ...routine, risk: { score: 2, confidence: 0.9 } }), ); await expect( resolveIntegrationToolAutoDecision(call), - ).resolves.toMatchObject({ action: 'ask', mode: 'on' }); + ).resolves.toMatchObject({ action: 'deny', mode: 'on' }); mocks.evaluate.mockResolvedValue(null); await expect( resolveIntegrationToolAutoDecision(call), ).resolves.toMatchObject({ - action: 'ask', + action: 'deny', mode: 'on', evaluation: { unavailable: 'no_model' }, }); }); + + it('fails closed to deny 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: 'deny', + mode: 'on', + evaluation: { recommendation: 'deny', 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: 'deny', 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..a748c2af6e 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' + ? 'no decision model is configured to check it' + : 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 () => []), @@ -37,6 +48,7 @@ import { getIntegrationToolApproval, getSessionForFastConversation, insertAutoApprovedIntegrationToolApproval, + insertAutoRejectedIntegrationToolApproval, insertIntegrationToolApproval, isDeploymentExperimentEnabled, listIntegrationToolPolicies, @@ -792,7 +804,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, + ), }; } @@ -820,7 +838,7 @@ describe('tool approval bridge', () => { }); }); - it('runs a default tool Auto finds routine, and asks about one it finds risky', async () => { + it('runs a default tool Auto finds routine, and denies one it finds risky', async () => { const evaluation = { recommendation: 'approve' as const, answers: {}, @@ -865,36 +883,56 @@ describe('tool approval bridge', () => { }), ); - // Risky: the card, with the model's view on it. + // Risky: no card, a terminal auto_rejected audit row, and a tool error + // to the model naming Auto mode and the reason. + const riskyEvaluation = { ...evaluation, recommendation: 'deny' as const }; vi.mocked(resolveIntegrationToolAutoDecision).mockResolvedValue({ - action: 'ask', + action: 'deny', 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()); - expect(insertIntegrationToolApproval).toHaveBeenCalledWith( - expect.anything(), + await vi.waitFor(() => + expect(unsure.reply).toHaveBeenCalledWith( + 'req-2', + 'reject', + expect.stringContaining('Auto mode blocked this tool call'), + ), + ); + expect(unsure.reply.mock.calls[0]![2]).toContain('assessed as risky'); + expect(insertIntegrationToolApproval).not.toHaveBeenCalled(); + expect(insertAutoRejectedIntegrationToolApproval).toHaveBeenCalledWith( + { sessionId: 'session-id', userId: 'user-id' }, expect.objectContaining({ nativeRequestId: 'req-2', - autoEvaluation: { ...evaluation, recommendation: 'ask' }, + autoEvaluation: riskyEvaluation, }), ); - // Auto failing outright asks a person. + // Auto failing outright fails closed to the same denial. 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()); - expect(insertIntegrationToolApproval).toHaveBeenCalledWith( + await vi.waitFor(() => + expect(failing.reply).toHaveBeenCalledWith( + 'req-3', + 'reject', + expect.stringContaining('Auto mode blocked this tool call'), + ), + ); + expect(insertIntegrationToolApproval).not.toHaveBeenCalled(); + expect(insertAutoRejectedIntegrationToolApproval).toHaveBeenCalledWith( expect.anything(), - expect.objectContaining({ nativeRequestId: 'req-3' }), + expect.objectContaining({ + nativeRequestId: 'req-3', + autoEvaluation: expect.objectContaining({ + recommendation: 'deny', + unavailable: 'error', + }), + }), ); // A manual Ask first tool (not in the Auto set) never reaches the model. 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..931ce99957 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, @@ -28,6 +29,7 @@ import { } from '@roomote/types'; import { + describeIntegrationToolAutoDeny, resolveIntegrationToolAutoDecision, resolveIntegrationToolAutoState, } from '../integration-tool-auto-evaluation'; @@ -505,7 +507,9 @@ 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. Auto never asks a person: anything it does not + // approve — a risky call, an evaluation error, or no decision model — + // is denied, and the denial is returned to the model as a tool error. const autoAssessed = !overrideForSession && input.autoToolKeys?.has( @@ -519,15 +523,49 @@ export function createFastAgentToolApprovalBridge(input: { args, userRequest: input.userRequest, userId: input.userId, - }).catch(() => ({ action: 'ask' as const, mode: 'failed' as const })) + }).catch(() => ({ + action: 'deny' as const, + mode: 'on' as const, + evaluation: { + recommendation: 'deny' 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 === 'deny') { + // The audit row is born terminal `auto_rejected` with the model's + // assessment; if it cannot be written the outer handler rejects the + // ask instead of denying it unrecorded. No card is ever shown. + 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, + )}. The call was not run, and this call cannot proceed.`, + ) + .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 +613,6 @@ export function createFastAgentToolApprovalBridge(input: { nativeRequestId: ask.requestId, argsFingerprint, argsSummary: args ?? null, - ...(auto?.mode === 'on' ? { 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..95eec16760 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,8 @@ 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 is blocked. The model can only ever run the call or deny it, + * never ask a person. */ export function recommendFromAutoAnswers( answers: AutoRiskAnswers, @@ -94,7 +94,7 @@ export function recommendFromAutoAnswers( (answers.matchesRequest ?? 1) >= YES && answers.steeredByUntrustedContent <= NO && (answers.guidanceFlagsRisk ?? 0) <= NO; - return routine ? 'approve' : 'ask'; + return routine ? 'approve' : 'deny'; } export async function evaluateIntegrationToolAutoDecision(input: { @@ -142,7 +142,7 @@ export async function evaluateIntegrationToolAutoDecision(input: { taskId: input.taskId, }); if (!answers) { - return { recommendation: 'ask', unavailable: 'no_model', evaluatedAt }; + return { recommendation: 'deny', unavailable: 'no_model', evaluatedAt }; } const riskAnswers: AutoRiskAnswers = { risk: { @@ -173,16 +173,18 @@ export async function evaluateIntegrationToolAutoDecision(input: { evaluatedAt, }; } catch { - return { recommendation: 'ask', unavailable: 'error', evaluatedAt }; + return { recommendation: 'deny', unavailable: 'error', evaluatedAt }; } } /** - * 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 fail-closed deny rather than + * silently running calls. `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 +199,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,28 +251,59 @@ export function recordIntegrationToolShadowEvaluationInBackground(input: { } export type IntegrationToolAutoDecision = - | { action: 'ask'; mode: 'off' } + | { action: 'run'; mode: 'off' } | { - action: 'approve' | 'ask'; + action: 'approve' | 'deny'; 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 'no decision model is configured to check it'; + } + 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 — is `deny`, + * and the call is blocked with a tool error to the model. 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') { + // Fail closed: 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. + return { + action: 'deny', + mode: 'on', + evaluation: { + recommendation: 'deny', + unavailable: 'no_model', + evaluatedAt: new Date().toISOString(), + }, + }; + } const evaluation = await evaluateIntegrationToolAutoDecision({ ...input, deploymentGuidance: state.settings.policy, }); return evaluation.recommendation === 'approve' ? { action: 'approve', mode: 'on', evaluation } - : { action: 'ask', mode: 'on', evaluation }; + : { action: 'deny', mode: 'on', evaluation }; } 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..cf44874fa8 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: 'deny' 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: 'deny' }); + // 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..293bd7d348 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,6 +9,7 @@ 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), @@ -17,6 +18,12 @@ const mocks = vi.hoisted(() => ({ vi.mock( '@roomote/cloud-agents/server/integration-tool-auto-evaluation', () => ({ + describeIntegrationToolAutoDeny: (evaluation: { unavailable?: string }) => + evaluation.unavailable === 'no_model' + ? 'no decision model is configured to check it' + : evaluation.unavailable === 'error' + ? 'the automatic check failed' + : 'it was assessed as risky', resolveIntegrationToolAutoDecision: mocks.resolveAuto, resolveIntegrationToolAutoState: mocks.autoState, }), @@ -33,6 +40,7 @@ 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, @@ -74,7 +82,7 @@ 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' }); }); @@ -195,33 +203,48 @@ describe('requestTaskToolApproval', () => { expect(mocks.claimAuto).not.toHaveBeenCalled(); expect(mocks.insert).not.toHaveBeenCalled(); - // Risky: the card, with the model's view on it. + // Risky: denied with a terminal audit row and the reason for the + // model; no card is ever created. + const riskyEvaluation = { ...evaluation, recommendation: 'deny' }; mocks.resolveAuto.mockResolvedValue({ - action: 'ask', + action: 'deny', mode: 'on', - evaluation: { ...evaluation, recommendation: 'ask' }, + evaluation: riskyEvaluation, }); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ - outcome: 'pending', - approvalId: 'approval-1', + outcome: 'denied', + reason: 'it was assessed as risky', }); - expect(mocks.insert).toHaveBeenLastCalledWith( - expect.anything(), + expect(mocks.insertAutoRejected).toHaveBeenCalledWith( + { sessionId: 'session-1', userId: 'owner-1' }, expect.objectContaining({ - autoEvaluation: { ...evaluation, recommendation: 'ask' }, + taskId: 'task-1', + nativeRequestId: 'per_1', + autoEvaluation: riskyEvaluation, }), ); + expect(mocks.insert).not.toHaveBeenCalled(); - // Auto failing outright asks a person. + // Auto failing outright fails closed to the same denial. mocks.resolveAuto.mockRejectedValue(new Error('settings unavailable')); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ - outcome: 'pending', - approvalId: 'approval-1', + outcome: 'denied', + reason: 'the automatic check failed', }); + expect(mocks.insertAutoRejected).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ + autoEvaluation: expect.objectContaining({ + recommendation: 'deny', + unavailable: 'error', + }), + }), + ); + expect(mocks.insert).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', }); diff --git a/packages/sdk/src/server/lib/task-tool-approvals.ts b/packages/sdk/src/server/lib/task-tool-approvals.ts index e4b44ee6b7..1789c5271a 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, @@ -15,6 +16,7 @@ import { taskRuns, } from '@roomote/db/server'; import { + describeIntegrationToolAutoDeny, resolveIntegrationToolAutoDecision, resolveIntegrationToolAutoState, } from '@roomote/cloud-agents/server/integration-tool-auto-evaluation'; @@ -119,6 +121,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 +175,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. Auto never asks a person: anything it + // does not approve is denied, and any failure on this path denies too. const auto = integrationToolModeIsAutoAssessed({ policyMode, sessionOverrideMode: overrideForSession, @@ -183,10 +188,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: 'deny' as const, + mode: 'on' as const, + evaluation: { + recommendation: 'deny' 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,10 +223,18 @@ export async function requestTaskToolApproval(input: { }); return { outcome: 'approved' }; } - const approval = await insertIntegrationToolApproval(context, { - ...call, - ...(auto?.mode === 'on' ? { autoEvaluation: auto.evaluation } : {}), - }); + if (auto?.action === 'deny') { + // Born-terminal audit row; no card is ever shown for an Auto denial. + await insertAutoRejectedIntegrationToolApproval(context, { + ...call, + autoEvaluation: auto.evaluation, + }); + return { + outcome: 'denied', + reason: describeIntegrationToolAutoDeny(auto.evaluation), + }; + } + const approval = await insertIntegrationToolApproval(context, call); 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..11bde8c6f1 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's decision model denied the call, or + // the assessment failed closed. 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,8 +79,9 @@ 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 + * first; a routine call runs, anything else is blocked with a tool error + * returned to the model — Auto never 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 * runs in the background and is recorded, so its judgment can be checked @@ -108,12 +112,12 @@ 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 model can only + * ever run the call or block it; it never asks a person. */ export interface IntegrationToolAutoEvaluation { - recommendation: 'approve' | 'ask'; + recommendation: 'approve' | 'deny'; /** * The raw judgments the recommendation was computed from: the risk level * (`riskScore`, a weighted position on the ordered risk levels, with From 1e2e46fd2eaa5658e5f28f0d04c87f49719c86bd Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:08:20 +0000 Subject: [PATCH 2/4] fix: make Auto approval presence-aware --- .changeset/auto-approval-allow-deny.md | 2 +- ...ngIntegrationToolApprovals.client.test.tsx | 51 +++++ .../PendingIntegrationToolApprovals.tsx | 24 +++ ...rationToolApprovalsExperimentalSetting.tsx | 19 +- ...grationToolAutoModeSetting.client.test.tsx | 5 + .../IntegrationToolAutoModeSetting.tsx | 8 +- .../opencode-server-tool-approvals.test.ts | 2 +- .../opencode-server/tool-approvals.ts | 2 +- .../integration-tool-auto-evaluation.test.ts | 32 ++-- .../fast-agent-tool-approvals.test.ts | 176 ++++++++++++++++-- .../server/fast-agent/fast-agent-service.ts | 10 +- .../fast-agent/fast-agent-tool-approvals.ts | 61 +++++- .../integration-tool-auto-evaluation.ts | 39 ++-- .../integration-tool-approvals.test.ts | 4 +- .../lib/__tests__/task-tool-approvals.test.ts | 120 ++++++++++-- .../sdk/src/server/lib/task-tool-approvals.ts | 81 ++++++-- .../types/src/integration-tool-approvals.ts | 20 +- 17 files changed, 537 insertions(+), 119 deletions(-) diff --git a/.changeset/auto-approval-allow-deny.md b/.changeset/auto-approval-allow-deny.md index de6073e2ba..2d5b0a335e 100644 --- a/.changeset/auto-approval-allow-deny.md +++ b/.changeset/auto-approval-allow-deny.md @@ -2,4 +2,4 @@ "@roomote/web": patch --- -Auto mode for integration tool approvals now enforces allow/deny: a call the decision model does not approve is blocked with a tool error returned to the agent instead of pausing on an approval card, and any evaluation failure or missing decision model fails closed to the same denial. +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 1dd855f11f..6f1c5afd05 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.client.test.tsx @@ -140,4 +140,55 @@ describe('PendingIntegrationToolApprovals', () => { expect(screen.getByText('No additional details.')).toBeInTheDocument(); expect(screen.queryByText('No arguments')).not.toBeInTheDocument(); }); + + 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 b8e6b247d5..c4d4e07183 100644 --- a/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx +++ b/apps/web/src/components/sessions/PendingIntegrationToolApprovals.tsx @@ -13,6 +13,7 @@ import { } from '@/components/system'; import { MCP_INTEGRATIONS, + type IntegrationToolAutoEvaluation, type IntegrationToolApprovalMetadata, } from '@roomote/types'; @@ -64,6 +65,21 @@ function approvalPrompt(item: IntegrationToolApprovalMetadata): string { return `Let ${name} use this tool?`; } +/** 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 an automatic check wasn't available, so it asked you."; + } + if (evaluation.unavailable === 'error') { + return "Auto couldn't check this call, so it asked you."; + } + return evaluation.recommendation === 'approve' + ? 'Auto would have run this call.' + : 'Auto flagged this call as risky and asked you.'; +} + function summarizeArgs(argsSummary: unknown): string | null { if ( argsSummary === null || @@ -141,6 +157,14 @@ export function PendingIntegrationToolApprovals({

Roomote is waiting for your approval to continue.

+ {item.autoEvaluation ? ( +

+ {describeAutoEvaluation(item.autoEvaluation)} +

+ ) : null} diff --git a/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx b/apps/web/src/components/settings/IntegrationToolApprovalsExperimentalSetting.tsx index a2daf839b7..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 block 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 80a16d90ee..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 block 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 block 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 block it; it never asks a person. + * 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 264f82c173..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 @@ -118,7 +118,7 @@ describe('createTaskToolApprovalRelay', () => { expect.objectContaining({ reply: 'reject', message: expect.stringContaining( - 'Auto mode blocked this tool call because it was assessed as risky', + 'Auto mode blocked this tool call because it was assessed as risky and the session owner was away', ), }), ); 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 89e337e626..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 @@ -222,7 +222,7 @@ export function createTaskToolApprovalRelay(options: { await reply( ask, 'reject', - `Auto mode blocked this tool call because ${result.reason}. The call was not run, and this call cannot proceed.`, + `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; } 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 c7821e6784..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 @@ -68,7 +68,7 @@ beforeEach(() => { }); describe('recommendFromAutoAnswers', () => { - it('runs only a routine call the user asked for; every doubt denies', () => { + it('runs only a routine call the user asked for; every doubt asks', () => { expect(recommendFromAutoAnswers(routine)).toBe('approve'); // With no request to judge against, the other signals decide. expect( @@ -82,7 +82,7 @@ describe('recommendFromAutoAnswers', () => { { steeredByUntrustedContent: 0.4 }, { guidanceFlagsRisk: 0.5 }, ] satisfies Partial[]) { - expect(recommendFromAutoAnswers({ ...routine, ...doubt })).toBe('deny'); + expect(recommendFromAutoAnswers({ ...routine, ...doubt })).toBe('ask'); } }); }); @@ -146,7 +146,7 @@ describe('evaluateIntegrationToolAutoDecision', () => { ); const guided = await evaluateIntegrationToolAutoDecision(call); expect(guided).toMatchObject({ - recommendation: 'deny', + recommendation: 'ask', answers: { guidanceFlagsRisk: 0.9 }, }); const { state, questions } = mocks.evaluate.mock.calls[1]![0]; @@ -156,19 +156,19 @@ describe('evaluateIntegrationToolAutoDecision', () => { expect(questions.guidanceFlagsRisk).toBeDefined(); }); - it('fails closed to deny 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), ).resolves.toMatchObject({ - recommendation: 'deny', + recommendation: 'ask', unavailable: 'no_model', }); mocks.evaluate.mockRejectedValue(new Error('timeout')); await expect( evaluateIntegrationToolAutoDecision(call), - ).resolves.toMatchObject({ recommendation: 'deny', unavailable: 'error' }); + ).resolves.toMatchObject({ recommendation: 'ask', unavailable: 'error' }); }); }); @@ -181,9 +181,9 @@ describe('resolveIntegrationToolAutoState', () => { settings: { policy: 'Reads are fine.' }, }); - // The setting can outlive the model: on stays on so callers fail closed - // instead of silently running unassessed calls. 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: 'on', @@ -283,38 +283,38 @@ describe('resolveIntegrationToolAutoDecision', () => { 'Reads are routine.', ); - // Risky, or a failed evaluation: the call is denied, never asked. + // Risky, or a failed evaluation: the call asks its owner. mocks.evaluate.mockResolvedValue( modelAnswers({ ...routine, risk: { score: 2, confidence: 0.9 } }), ); await expect( resolveIntegrationToolAutoDecision(call), - ).resolves.toMatchObject({ action: 'deny', mode: 'on' }); + ).resolves.toMatchObject({ action: 'ask', mode: 'on' }); mocks.evaluate.mockResolvedValue(null); await expect( resolveIntegrationToolAutoDecision(call), ).resolves.toMatchObject({ - action: 'deny', + action: 'ask', mode: 'on', evaluation: { unavailable: 'no_model' }, }); }); - it('fails closed to deny when Auto is on but no judgment model is configured', async () => { + 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: 'deny', + action: 'ask', mode: 'on', - evaluation: { recommendation: 'deny', unavailable: 'no_model' }, + 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: 'deny', mode: 'on' }); + ).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 a748c2af6e..726b085527 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 @@ -4,7 +4,7 @@ vi.mock('../../integration-tool-auto-evaluation', () => ({ describeIntegrationToolAutoDeny: vi.fn( (evaluation: { unavailable?: string }) => evaluation.unavailable === 'no_model' - ? 'no decision model is configured to check it' + ? 'an automatic check is not available' : evaluation.unavailable === 'error' ? 'the automatic check failed' : 'it was assessed as risky', @@ -41,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, @@ -56,6 +62,7 @@ import { listIntegrationToolUserPolicies, markIntegrationToolApprovalConsumed, } from '@roomote/db/server'; +import { isSessionUserPresent } from '@roomote/redis'; import { buildIntegrationToolApprovalRules, @@ -818,6 +825,7 @@ describe('tool approval bridge', () => { return createFastAgentToolApprovalBridge({ sessionId: 'session-id', userId: 'user-id', + surface: 'web', integrations, }); } @@ -826,6 +834,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', @@ -838,7 +847,7 @@ describe('tool approval bridge', () => { }); }); - it('runs a default tool Auto finds routine, and denies 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: {}, @@ -853,6 +862,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.', @@ -883,34 +893,36 @@ describe('tool approval bridge', () => { }), ); - // Risky: no card, a terminal auto_rejected audit row, and a tool error - // to the model naming Auto mode and the reason. - const riskyEvaluation = { ...evaluation, recommendation: 'deny' as const }; + // 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: 'deny', + action: 'ask', mode: 'on', 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).toHaveBeenCalledWith( 'req-2', 'reject', - expect.stringContaining('Auto mode blocked this tool call'), + 'The requester rejected this tool call.', ), ); - expect(unsure.reply.mock.calls[0]![2]).toContain('assessed as risky'); - expect(insertIntegrationToolApproval).not.toHaveBeenCalled(); - expect(insertAutoRejectedIntegrationToolApproval).toHaveBeenCalledWith( + expect(insertIntegrationToolApproval).toHaveBeenCalledWith( { sessionId: 'session-id', userId: 'user-id' }, expect.objectContaining({ nativeRequestId: 'req-2', autoEvaluation: riskyEvaluation, }), ); + expect(insertAutoRejectedIntegrationToolApproval).not.toHaveBeenCalled(); - // Auto failing outright fails closed to the same denial. + // An evaluation failure also asks while the owner is present. vi.mocked(resolveIntegrationToolAutoDecision).mockRejectedValue( new Error('settings unavailable'), ); @@ -920,20 +932,20 @@ describe('tool approval bridge', () => { expect(failing.reply).toHaveBeenCalledWith( 'req-3', 'reject', - expect.stringContaining('Auto mode blocked this tool call'), + 'The requester rejected this tool call.', ), ); - expect(insertIntegrationToolApproval).not.toHaveBeenCalled(); - expect(insertAutoRejectedIntegrationToolApproval).toHaveBeenCalledWith( - expect.anything(), + expect(insertIntegrationToolApproval).toHaveBeenCalledWith( + { sessionId: 'session-id', userId: 'user-id' }, expect.objectContaining({ nativeRequestId: 'req-3', autoEvaluation: expect.objectContaining({ - recommendation: 'deny', + 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(); @@ -943,6 +955,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', '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' }, @@ -954,6 +1094,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); @@ -1243,6 +1384,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(() => @@ -1286,6 +1428,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(() => @@ -1307,6 +1450,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..b2fd23656b 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, @@ -6282,16 +6283,21 @@ 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' + conversation.surface === 'discord' || + conversation.surface === 'telegram' ? { notify: async (approval) => { const sessionUrl = buildFastSessionUrl( - conversation.surface as 'slack' | 'discord', + conversation.surface as + | 'slack' + | 'discord' + | 'telegram', 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 931ce99957..b92cf876db 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 @@ -18,11 +18,13 @@ 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, @@ -60,6 +62,7 @@ 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; /** * OpenCode flattens every MCP tool to `_`. The server @@ -367,6 +370,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 @@ -389,6 +393,41 @@ export function createFastAgentToolApprovalBridge(input: { const handledRequestIds = new Set(); const notifiedApprovalIds = new Set(); + const ownerIsPresent = async (): Promise => { + if ( + input.surface === 'slack' || + input.surface === 'discord' || + input.surface === '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( + `[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. @@ -507,9 +546,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. Auto never asks a person: anything it does not - // approve — a risky call, an evaluation error, or no decision model — - // is denied, and the denial is returned to the model as a tool error. + // 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( @@ -524,10 +562,10 @@ export function createFastAgentToolApprovalBridge(input: { userRequest: input.userRequest, userId: input.userId, }).catch(() => ({ - action: 'deny' as const, + action: 'ask' as const, mode: 'on' as const, evaluation: { - recommendation: 'deny' as const, + recommendation: 'ask' as const, unavailable: 'error' as const, evaluatedAt: new Date().toISOString(), }, @@ -540,10 +578,10 @@ export function createFastAgentToolApprovalBridge(input: { await helpers.reply(ask.requestId, 'once'); return; } - if (auto?.action === 'deny') { - // The audit row is born terminal `auto_rejected` with the model's - // assessment; if it cannot be written the outer handler rejects the - // ask instead of denying it unrecorded. No card is ever shown. + 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 }, { @@ -561,7 +599,7 @@ export function createFastAgentToolApprovalBridge(input: { 'reject', `Auto mode blocked this tool call because ${describeIntegrationToolAutoDeny( auto.evaluation, - )}. The call was not run, and this call cannot proceed.`, + )} 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; @@ -613,6 +651,9 @@ export function createFastAgentToolApprovalBridge(input: { nativeRequestId: ask.requestId, argsFingerprint, argsSummary: args ?? null, + ...(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 95eec16760..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 is blocked. The model can only ever run the call or deny it, - * never ask a person. + * 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, @@ -94,7 +95,7 @@ export function recommendFromAutoAnswers( (answers.matchesRequest ?? 1) >= YES && answers.steeredByUntrustedContent <= NO && (answers.guidanceFlagsRisk ?? 0) <= NO; - return routine ? 'approve' : 'deny'; + return routine ? 'approve' : 'ask'; } export async function evaluateIntegrationToolAutoDecision(input: { @@ -142,7 +143,7 @@ export async function evaluateIntegrationToolAutoDecision(input: { taskId: input.taskId, }); if (!answers) { - return { recommendation: 'deny', unavailable: 'no_model', evaluatedAt }; + return { recommendation: 'ask', unavailable: 'no_model', evaluatedAt }; } const riskAnswers: AutoRiskAnswers = { risk: { @@ -173,7 +174,7 @@ export async function evaluateIntegrationToolAutoDecision(input: { evaluatedAt, }; } catch { - return { recommendation: 'deny', unavailable: 'error', evaluatedAt }; + return { recommendation: 'ask', unavailable: 'error', evaluatedAt }; } } @@ -182,9 +183,10 @@ export async function evaluateIntegrationToolAutoDecision(input: { * 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 fail-closed deny rather than - * silently running calls. `shadow` is the same assessment recorded without - * acting, while Auto is off and a hosted model is there to do it cheaply. + * 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'; @@ -253,7 +255,7 @@ export function recordIntegrationToolShadowEvaluationInBackground(input: { export type IntegrationToolAutoDecision = | { action: 'run'; mode: 'off' } | { - action: 'approve' | 'deny'; + action: 'approve' | 'ask'; mode: 'on'; evaluation: IntegrationToolAutoEvaluation; }; @@ -266,7 +268,7 @@ export function describeIntegrationToolAutoDeny( evaluation: IntegrationToolAutoEvaluation, ): string { if (evaluation.unavailable === 'no_model') { - return 'no decision model is configured to check it'; + return 'an automatic check is not available'; } if (evaluation.unavailable === 'error') { return 'the automatic check failed'; @@ -277,9 +279,9 @@ export function describeIntegrationToolAutoDeny( /** * How Auto treats one call to a default tool. `approve` means the call is * routine enough to run; anything else — a risky assessment, an evaluation - * error, or Auto on without a judgment model to assess with — is `deny`, - * and the call is blocked with a tool error to the model. Only `off` (Auto - * disabled) lets the call run unassessed. + * 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], @@ -287,13 +289,14 @@ export async function resolveIntegrationToolAutoDecision( const state = await resolveIntegrationToolAutoState(); if (state.mode !== 'on') return { action: 'run', mode: 'off' }; if (state.model !== 'judgment') { - // Fail closed: 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. + // 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: 'deny', + action: 'ask', mode: 'on', evaluation: { - recommendation: 'deny', + recommendation: 'ask', unavailable: 'no_model', evaluatedAt: new Date().toISOString(), }, @@ -305,5 +308,5 @@ export async function resolveIntegrationToolAutoDecision( }); return evaluation.recommendation === 'approve' ? { action: 'approve', mode: 'on', evaluation } - : { action: 'deny', mode: 'on', evaluation }; + : { action: 'ask', mode: 'on', evaluation }; } 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 cf44874fa8..5c9605ad35 100644 --- a/packages/db/src/lib/__tests__/integration-tool-approvals.test.ts +++ b/packages/db/src/lib/__tests__/integration-tool-approvals.test.ts @@ -595,7 +595,7 @@ describe('auto-rejected audit rows', () => { const userId = await user(); const sessionId = await ownedSession(userId); const evaluation = { - recommendation: 'deny' as const, + recommendation: 'ask' as const, answers: { riskScore: 0.9 }, evaluatedAt: new Date().toISOString(), }; @@ -615,7 +615,7 @@ describe('auto-rejected audit rows', () => { expect(row?.status).toBe('auto_rejected'); expect(row?.decidedByUserId).toBeNull(); expect(row?.decidedAt).not.toBeNull(); - expect(row?.autoEvaluation).toMatchObject({ recommendation: 'deny' }); + expect(row?.autoEvaluation).toMatchObject({ recommendation: 'ask' }); // Terminal: nothing can claim, decide, or cancel it into a run. await expect( claimAutoApprovedIntegrationToolApproval({ 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 293bd7d348..7955fafc90 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 @@ -13,6 +13,7 @@ const mocks = vi.hoisted(() => ({ claimAuto: vi.fn(async () => true), getApproval: vi.fn(async () => undefined as unknown), expire: vi.fn(async () => undefined), + isPresent: vi.fn(async () => true), })); vi.mock( @@ -20,7 +21,7 @@ vi.mock( () => ({ describeIntegrationToolAutoDeny: (evaluation: { unavailable?: string }) => evaluation.unavailable === 'no_model' - ? 'no decision model is configured to check it' + ? 'an automatic check is not available' : evaluation.unavailable === 'error' ? 'the automatic check failed' : 'it was assessed as risky', @@ -46,17 +47,22 @@ vi.mock('@roomote/db/server', () => ({ 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, @@ -84,6 +90,7 @@ beforeEach(() => { mocks.claimAuto.mockResolvedValue(true); mocks.resolveAuto.mockResolvedValue({ action: 'run', mode: 'off' }); mocks.autoState.mockResolvedValue({ mode: 'off' }); + mocks.isPresent.mockResolvedValue(true); }); describe('resolveTaskIntegrationToolApprovals', () => { @@ -203,19 +210,18 @@ describe('requestTaskToolApproval', () => { expect(mocks.claimAuto).not.toHaveBeenCalled(); expect(mocks.insert).not.toHaveBeenCalled(); - // Risky: denied with a terminal audit row and the reason for the - // model; no card is ever created. - const riskyEvaluation = { ...evaluation, recommendation: 'deny' }; + // Risky: the present owner gets a card with the assessment. + const riskyEvaluation = { ...evaluation, recommendation: 'ask' }; mocks.resolveAuto.mockResolvedValue({ - action: 'deny', + action: 'ask', mode: 'on', evaluation: riskyEvaluation, }); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ - outcome: 'denied', - reason: 'it was assessed as risky', + outcome: 'pending', + approvalId: 'approval-1', }); - expect(mocks.insertAutoRejected).toHaveBeenCalledWith( + expect(mocks.insert).toHaveBeenCalledWith( { sessionId: 'session-1', userId: 'owner-1' }, expect.objectContaining({ taskId: 'task-1', @@ -223,24 +229,24 @@ describe('requestTaskToolApproval', () => { autoEvaluation: riskyEvaluation, }), ); - expect(mocks.insert).not.toHaveBeenCalled(); + expect(mocks.insertAutoRejected).not.toHaveBeenCalled(); - // Auto failing outright fails closed to the same denial. + // Auto failing outright still asks a present owner. mocks.resolveAuto.mockRejectedValue(new Error('settings unavailable')); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ - outcome: 'denied', - reason: 'the automatic check failed', + outcome: 'pending', + approvalId: 'approval-1', }); - expect(mocks.insertAutoRejected).toHaveBeenLastCalledWith( + expect(mocks.insert).toHaveBeenLastCalledWith( expect.anything(), expect.objectContaining({ autoEvaluation: expect.objectContaining({ - recommendation: 'deny', + recommendation: 'ask', unavailable: 'error', }), }), ); - expect(mocks.insert).not.toHaveBeenCalled(); + expect(mocks.insertAutoRejected).not.toHaveBeenCalled(); }); it('runs a default tool asked under a stale rule once Auto is off', async () => { @@ -251,6 +257,90 @@ describe('requestTaskToolApproval', () => { 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('treats a chat-surface task Session as present without browser presence', async () => { + mocks.sessionForTask.mockResolvedValue({ + ...ownedSession, + sourceSurface: 'slack', + }); + 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 1789c5271a..84e3594d62 100644 --- a/packages/sdk/src/server/lib/task-tool-approvals.ts +++ b/packages/sdk/src/server/lib/task-tool-approvals.ts @@ -15,6 +15,7 @@ import { listIntegrationToolUserPolicies, taskRuns, } from '@roomote/db/server'; +import { isSessionUserPresent } from '@roomote/redis'; import { describeIntegrationToolAutoDeny, resolveIntegrationToolAutoDecision, @@ -30,6 +31,47 @@ 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 === '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, @@ -50,6 +92,7 @@ async function resolveTaskApprovalSession(runId: number) { sessionId: session.id, ownerUserId: session.ownerKind === 'user' ? (session.ownerUserId ?? null) : null, + sourceSurface: session.sourceSurface, }; } @@ -175,8 +218,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. Auto never asks a person: anything it - // does not approve is denied, and any failure on this path denies too. + // 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, @@ -189,10 +232,10 @@ export async function requestTaskToolApproval(input: { userId: session.ownerUserId, taskId: session.taskId, }).catch(() => ({ - action: 'deny' as const, + action: 'ask' as const, mode: 'on' as const, evaluation: { - recommendation: 'deny' as const, + recommendation: 'ask' as const, unavailable: 'error' as const, evaluatedAt: new Date().toISOString(), }, @@ -223,18 +266,28 @@ export async function requestTaskToolApproval(input: { }); return { outcome: 'approved' }; } - if (auto?.action === 'deny') { - // Born-terminal audit row; no card is ever shown for an Auto denial. - await insertAutoRejectedIntegrationToolApproval(context, { - ...call, - autoEvaluation: auto.evaluation, + if (auto?.action === 'ask') { + const ownerPresent = await isTaskSessionOwnerPresent({ + sessionId: session.sessionId, + userId: session.ownerUserId, + sourceSurface: session.sourceSurface, }); - return { - outcome: 'denied', - reason: describeIntegrationToolAutoDeny(auto.evaluation), - }; + 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); + const approval = await insertIntegrationToolApproval(context, { + ...call, + ...(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 11bde8c6f1..df97008afb 100644 --- a/packages/types/src/integration-tool-approvals.ts +++ b/packages/types/src/integration-tool-approvals.ts @@ -49,8 +49,8 @@ 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's decision model denied the call, or - // the assessment failed closed. Born terminal; `decidedByUserId` is null. + // 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 = @@ -79,11 +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, anything else is blocked with a tool error - * returned to the model — Auto never 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. */ @@ -113,11 +113,11 @@ export const integrationToolAutoSettingsSchema = z.object({ /** * 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 model can only - * ever run the call or block it; it never asks a person. + * 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' | 'deny'; + recommendation: 'approve' | 'ask'; /** * The raw judgments the recommendation was computed from: the risk level * (`riskScore`, a weighted position on the ordered risk levels, with From 1a7cd468b918eec10bcbcf10c6edc86b099abf04 Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:37:38 +0000 Subject: [PATCH 3/4] fix: include Teams in Auto approval presence --- .../fast-agent-tool-approvals.test.ts | 19 +++++++++- .../server/fast-agent/fast-agent-service.ts | 14 +++---- .../fast-agent/fast-agent-tool-approvals.ts | 24 ++++++++---- .../lib/__tests__/task-tool-approvals.test.ts | 37 ++++++++++--------- .../sdk/src/server/lib/task-tool-approvals.ts | 1 + 5 files changed, 63 insertions(+), 32 deletions(-) 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 726b085527..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 @@ -71,6 +71,7 @@ import { extractApprovalCallArgs, hashIntegrationToolApprovalRules, integrationToolApprovalRulesToConfig, + isFastAgentApprovalChatSurface, resolveFastAgentToolApprovalRules, resolveFastAgentToolApprovalSession, shouldDisposeInstanceForToolApprovalRules, @@ -797,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', @@ -1054,7 +1071,7 @@ describe('tool approval bridge', () => { expect(insertAutoRejectedIntegrationToolApproval).not.toHaveBeenCalled(); }); - it.each(['slack', 'discord', 'telegram'] as const)( + it.each(['slack', 'discord', 'teams', 'telegram'] as const)( 'treats a %s Session as present without browser presence', async (surface) => { redisMocks.isPresent.mockResolvedValue(false); 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 b2fd23656b..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 @@ -226,6 +226,7 @@ import { } from './fast-agent-tool-policy'; import { createFastAgentToolApprovalBridge, + isFastAgentApprovalChatSurface, resolveFastAgentToolApprovalSession, integrationToolApprovalRulesToConfig, resolveFastAgentToolApprovalRules, @@ -6270,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 @@ -6288,16 +6293,11 @@ export async function answerFastAgentQuestion({ autoToolKeys: toolApprovalRules.autoToolKeys, userRequest: question, signal: promptSignal, - ...(conversation.surface === 'slack' || - conversation.surface === 'discord' || - conversation.surface === 'telegram' + ...(approvalNotificationSurface ? { notify: async (approval) => { const sessionUrl = buildFastSessionUrl( - conversation.surface as - | 'slack' - | 'discord' - | 'telegram', + 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 b92cf876db..6031afecec 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 @@ -64,6 +64,22 @@ const INTEGRATION_TOOL_APPROVAL_CANCEL_EXPERIMENT_DISABLED = 'experiment_disabled'; const SESSION_PRESENCE_LOOKUP_TIMEOUT_MS = 2_000; +export 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 * name is the integration's code-mode mount name, which is the sanitized @@ -394,13 +410,7 @@ export function createFastAgentToolApprovalBridge(input: { const notifiedApprovalIds = new Set(); const ownerIsPresent = async (): Promise => { - if ( - input.surface === 'slack' || - input.surface === 'discord' || - input.surface === 'telegram' - ) { - return true; - } + if (isFastAgentApprovalChatSurface(input.surface)) return true; let timeout: ReturnType | undefined; try { return await Promise.race([ 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 7955fafc90..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 @@ -322,24 +322,27 @@ describe('requestTaskToolApproval', () => { expect(mocks.insertAutoRejected).not.toHaveBeenCalled(); }); - it('treats a chat-surface task Session as present without browser presence', async () => { - mocks.sessionForTask.mockResolvedValue({ - ...ownedSession, - sourceSurface: 'slack', - }); - const evaluation = { recommendation: 'ask', evaluatedAt: '' }; - mocks.resolveAuto.mockResolvedValue({ - action: 'ask', - mode: 'on', - evaluation, - }); + 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(); - }); + 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([ diff --git a/packages/sdk/src/server/lib/task-tool-approvals.ts b/packages/sdk/src/server/lib/task-tool-approvals.ts index 84e3594d62..cd416e6827 100644 --- a/packages/sdk/src/server/lib/task-tool-approvals.ts +++ b/packages/sdk/src/server/lib/task-tool-approvals.ts @@ -41,6 +41,7 @@ async function isTaskSessionOwnerPresent(input: { if ( input.sourceSurface === 'slack' || input.sourceSurface === 'discord' || + input.sourceSurface === 'teams' || input.sourceSurface === 'telegram' ) { return true; From e2599b0444d46b0ced16e3109f7f7e7892ed20e7 Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:38:28 +0000 Subject: [PATCH 4/4] fix: keep Teams chat surface helper internal --- .../src/server/fast-agent/fast-agent-tool-approvals.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6031afecec..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 @@ -64,7 +64,7 @@ const INTEGRATION_TOOL_APPROVAL_CANCEL_EXPERIMENT_DISABLED = 'experiment_disabled'; const SESSION_PRESENCE_LOOKUP_TIMEOUT_MS = 2_000; -export type FastAgentApprovalChatSurface = Extract< +type FastAgentApprovalChatSurface = Extract< FastAgentSurface, 'slack' | 'discord' | 'teams' | 'telegram' >;