From a500209168623cd15ec29caa8a55f3f4016d4dbf Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Wed, 30 Sep 2026 13:50:09 -0500 Subject: [PATCH] [Fix] Auto no longer denies calls in the first seconds of an open session --- .../fast-agent-tool-approvals.test.ts | 45 ++++++++++++++++++- .../fast-agent/fast-agent-tool-approvals.ts | 14 ++++++ .../lib/__tests__/task-tool-approvals.test.ts | 26 ++++++++++- .../sdk/src/server/lib/task-tool-approvals.ts | 18 ++++++++ 4 files changed, 101 insertions(+), 2 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 58693052e7..37437bdb1d 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,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('../../integration-tool-auto-evaluation', () => ({ describeIntegrationToolAutoDeny: vi.fn( @@ -952,6 +952,10 @@ describe('tool approval bridge', () => { ); }); + afterEach(() => { + vi.useRealTimers(); + }); + beforeEach(() => { vi.clearAllMocks(); databaseMocks.recentApprovalOutcomes = []; @@ -1342,6 +1346,7 @@ describe('tool approval bridge', () => { ])( 'denies an Auto %s call when the Session owner is absent', async (_label, evaluation, reason) => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); redisMocks.isPresent.mockResolvedValue(false); databaseMocks.recentApprovalOutcomes = [ { @@ -1367,6 +1372,10 @@ describe('tool approval bridge', () => { 'Please post the release update once.', ], }).handleAsk({ ...ask, requestId: `absent-${_label}` }, helperMocks); + // Away only after a second lookup a full presence renewal later. + for (let second = 0; second < 12; second += 1) { + await vi.advanceTimersByTimeAsync(1_000); + } await vi.waitFor(() => expect(helperMocks.reply).toHaveBeenCalledWith( @@ -1395,6 +1404,40 @@ describe('tool approval bridge', () => { }, ); + it('asks when the owner is back by the second presence lookup', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + vi.mocked(isSessionUserPresent) + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true); + vi.mocked(resolveIntegrationToolAutoDecision).mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation: { recommendation: 'ask', evaluatedAt: '' }, + }); + vi.mocked(getIntegrationToolApproval).mockResolvedValue({ + status: 'rejected', + } as never); + const h = helpers(); + createFastAgentToolApprovalBridge({ + sessionId: 'session-id', + userId: 'user-id', + surface: 'web', + integrations, + autoToolKeys: new Set([JSON.stringify(['mock-slack', 'post_message'])]), + }).handleAsk({ ...ask, requestId: 'back-1' }, h); + for (let second = 0; second < 12; second += 1) { + await vi.advanceTimersByTimeAsync(1_000); + } + await vi.waitFor(() => + expect(insertIntegrationToolApproval).toHaveBeenCalledWith( + { sessionId: 'session-id', userId: 'user-id' }, + expect.objectContaining({ nativeRequestId: 'back-1' }), + ), + ); + expect(insertAutoRejectedIntegrationToolApproval).not.toHaveBeenCalled(); + expect(isSessionUserPresent).toHaveBeenCalledTimes(2); + }); + it('asks when the Fast Session presence lookup fails', async () => { const evaluation = { recommendation: 'ask' as const, evaluatedAt: '' }; redisMocks.isPresent.mockRejectedValueOnce(new Error('redis unavailable')); 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 9db66546fb..fad3a999ee 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,12 @@ import { buildFastAgentCodeModeServerNames } from './fast-agent-tool-policy'; */ const INTEGRATION_TOOL_APPROVAL_POLL_MS = 1_500; const SESSION_PRESENCE_LOOKUP_TIMEOUT_MS = 2_000; +/** + * An open session page renews its presence every 10 seconds, and a page that + * just opened can briefly drop it. The owner counts as away only when a + * second lookup, a full renewal later, still finds nobody. + */ +const SESSION_PRESENCE_RECHECK_MS = 11_000; async function isFastAgentLaunchedTask( sessionId: string, @@ -427,6 +433,14 @@ export function createFastAgentToolApprovalBridge(input: { const ownerIsPresent = async (): Promise => { if (isFastAgentApprovalChatSurface(input.surface)) return true; + if (await lookUpOwnerPresence()) return true; + await new Promise((resolve) => { + const timer = setTimeout(resolve, SESSION_PRESENCE_RECHECK_MS); + timer.unref?.(); + }); + return lookUpOwnerPresence(); + }; + const lookUpOwnerPresence = async (): Promise => { 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 25217bf8e1..6cfd722d75 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 @@ -340,6 +340,7 @@ describe('requestTaskToolApproval', () => { ])( 'denies an Auto %s call when the Session owner is absent', async (_label, evaluation, reason) => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); mocks.isPresent.mockResolvedValue(false); mocks.resolveAuto.mockResolvedValue({ action: 'ask', @@ -347,10 +348,15 @@ describe('requestTaskToolApproval', () => { evaluation, }); - await expect(requestTaskToolApproval(ask)).resolves.toEqual({ + const result = requestTaskToolApproval(ask); + // Away only after a second lookup a full presence renewal later. + await vi.advanceTimersByTimeAsync(11_000); + await expect(result).resolves.toEqual({ outcome: 'denied', reason, }); + vi.useRealTimers(); + expect(isSessionUserPresent).toHaveBeenCalledTimes(2); expect(isSessionUserPresent).toHaveBeenCalledWith({ sessionId: 'session-1', userId: 'owner-1', @@ -366,6 +372,24 @@ describe('requestTaskToolApproval', () => { }, ); + it('asks when the owner is back by the second presence lookup', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + mocks.isPresent.mockResolvedValueOnce(false).mockResolvedValueOnce(true); + mocks.resolveAuto.mockResolvedValue({ + action: 'ask', + mode: 'on', + evaluation: { recommendation: 'ask', evaluatedAt: '' }, + }); + const result = requestTaskToolApproval(ask); + await vi.advanceTimersByTimeAsync(11_000); + await expect(result).resolves.toEqual({ + outcome: 'pending', + approvalId: 'approval-1', + }); + vi.useRealTimers(); + expect(mocks.insertAutoRejected).not.toHaveBeenCalled(); + }); + it('asks when the task presence lookup fails', async () => { const evaluation = { recommendation: 'ask', evaluatedAt: '' }; mocks.resolveAuto.mockResolvedValue({ diff --git a/packages/sdk/src/server/lib/task-tool-approvals.ts b/packages/sdk/src/server/lib/task-tool-approvals.ts index 4d4e3e326b..7b153b5c7c 100644 --- a/packages/sdk/src/server/lib/task-tool-approvals.ts +++ b/packages/sdk/src/server/lib/task-tool-approvals.ts @@ -108,6 +108,12 @@ async function publishTaskToolApproval(input: { } const SESSION_PRESENCE_LOOKUP_TIMEOUT_MS = 2_000; +/** + * An open session page renews its presence every 10 seconds, and a page that + * just opened can briefly drop it. The owner counts as away only when a + * second lookup, a full renewal later, still finds nobody. + */ +const SESSION_PRESENCE_RECHECK_MS = 11_000; async function isTaskSessionOwnerPresent(input: { sessionId: string; @@ -122,6 +128,18 @@ async function isTaskSessionOwnerPresent(input: { ) { return true; } + if (await lookUpTaskSessionOwnerPresence(input)) return true; + await new Promise((resolve) => { + const timer = setTimeout(resolve, SESSION_PRESENCE_RECHECK_MS); + timer.unref?.(); + }); + return lookUpTaskSessionOwnerPresence(input); +} + +async function lookUpTaskSessionOwnerPresence(input: { + sessionId: string; + userId: string; +}): Promise { let timeout: ReturnType | undefined; try { return await Promise.race([