From 4c542d0015a58fc4274f04eb06abcaf50017e786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Mon, 30 Mar 2026 11:49:05 +0200 Subject: [PATCH] fix(assistant): dedupe missing approval storage warnings --- packages/api/src/__tests__/assistant-router.test.ts | 7 +++++++ packages/api/src/router/assistant.ts | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/packages/api/src/__tests__/assistant-router.test.ts b/packages/api/src/__tests__/assistant-router.test.ts index 3e7c515..d6ea721 100644 --- a/packages/api/src/__tests__/assistant-router.test.ts +++ b/packages/api/src/__tests__/assistant-router.test.ts @@ -10,9 +10,11 @@ import { getAvailableAssistantTools, listPendingAssistantApprovals, peekPendingAssistantApproval, + resetAssistantApprovalStorageWarningStateForTests, selectAssistantToolsForRequest, } from "../router/assistant.js"; import { TOOL_DEFINITIONS } from "../router/assistant-tools.js"; +import { logger } from "../lib/logger.js"; function getToolNames( permissions: PermissionKeyValue[], @@ -205,6 +207,7 @@ describe("assistant router tool gating", () => { beforeEach(() => { approvalStore = createApprovalStoreMock(); apiRateLimiter.reset(); + resetAssistantApprovalStorageWarningStateForTests(); }); it("hides advanced tools unless the dedicated assistant permission is granted", () => { @@ -863,6 +866,7 @@ describe("assistant router tool gating", () => { }); it("degrades approval reads gracefully when approval storage is missing", async () => { + const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => logger); const missingTableError = createMissingApprovalTableError(); const missingStore = { assistantApproval: { @@ -885,9 +889,11 @@ describe("assistant router tool gating", () => { await expect(peekPendingAssistantApproval(missingStore, TEST_USER_ID, TEST_CONVERSATION_ID)).resolves.toBeNull(); await expect(consumePendingAssistantApproval(missingStore, TEST_USER_ID, TEST_CONVERSATION_ID)).resolves.toBeNull(); await expect(clearPendingAssistantApproval(missingStore, TEST_USER_ID, TEST_CONVERSATION_ID)).resolves.toBeUndefined(); + expect(warnSpy).toHaveBeenCalledTimes(1); }); it("returns an explicit error when approval storage is missing for mutation confirmation", async () => { + const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => logger); const missingTableError = createMissingApprovalTableError(); const missingStore = { assistantApproval: { @@ -913,6 +919,7 @@ describe("assistant router tool gating", () => { "create_project", JSON.stringify({ name: "Apollo" }), )).rejects.toThrow("Assistant approval storage is unavailable"); + expect(warnSpy).toHaveBeenCalledTimes(1); }); it("does not require confirmation for read-only assistant tools", () => { diff --git a/packages/api/src/router/assistant.ts b/packages/api/src/router/assistant.ts index b50948c..13ca0a1 100644 --- a/packages/api/src/router/assistant.ts +++ b/packages/api/src/router/assistant.ts @@ -21,6 +21,7 @@ const PENDING_APPROVAL_TTL_MS = 15 * 60 * 1000; export const ASSISTANT_CONFIRMATION_PREFIX = "CONFIRMATION_REQUIRED:"; const ASSISTANT_APPROVALS_TABLE_NAME = "public.assistant_approvals"; const MAX_OPENAI_TOOL_DEFINITIONS = 128; +let hasLoggedAssistantApprovalStorageUnavailable = false; const ALWAYS_INCLUDED_TOOL_NAMES = new Set([ "get_current_user", @@ -623,6 +624,10 @@ function isAssistantApprovalTableMissingError(error: unknown): boolean { } function logAssistantApprovalStorageUnavailable(error: unknown) { + if (hasLoggedAssistantApprovalStorageUnavailable) { + return; + } + hasLoggedAssistantApprovalStorageUnavailable = true; logger.warn( { err: error, @@ -645,6 +650,10 @@ async function withAssistantApprovalFallback( } } +export function resetAssistantApprovalStorageWarningStateForTests(): void { + hasLoggedAssistantApprovalStorageUnavailable = false; +} + export async function listPendingAssistantApprovals( db: AssistantApprovalStore, userId: string,