fix(assistant): dedupe missing approval storage warnings
This commit is contained in:
@@ -10,9 +10,11 @@ import {
|
|||||||
getAvailableAssistantTools,
|
getAvailableAssistantTools,
|
||||||
listPendingAssistantApprovals,
|
listPendingAssistantApprovals,
|
||||||
peekPendingAssistantApproval,
|
peekPendingAssistantApproval,
|
||||||
|
resetAssistantApprovalStorageWarningStateForTests,
|
||||||
selectAssistantToolsForRequest,
|
selectAssistantToolsForRequest,
|
||||||
} from "../router/assistant.js";
|
} from "../router/assistant.js";
|
||||||
import { TOOL_DEFINITIONS } from "../router/assistant-tools.js";
|
import { TOOL_DEFINITIONS } from "../router/assistant-tools.js";
|
||||||
|
import { logger } from "../lib/logger.js";
|
||||||
|
|
||||||
function getToolNames(
|
function getToolNames(
|
||||||
permissions: PermissionKeyValue[],
|
permissions: PermissionKeyValue[],
|
||||||
@@ -205,6 +207,7 @@ describe("assistant router tool gating", () => {
|
|||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
approvalStore = createApprovalStoreMock();
|
approvalStore = createApprovalStoreMock();
|
||||||
apiRateLimiter.reset();
|
apiRateLimiter.reset();
|
||||||
|
resetAssistantApprovalStorageWarningStateForTests();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("hides advanced tools unless the dedicated assistant permission is granted", () => {
|
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 () => {
|
it("degrades approval reads gracefully when approval storage is missing", async () => {
|
||||||
|
const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => logger);
|
||||||
const missingTableError = createMissingApprovalTableError();
|
const missingTableError = createMissingApprovalTableError();
|
||||||
const missingStore = {
|
const missingStore = {
|
||||||
assistantApproval: {
|
assistantApproval: {
|
||||||
@@ -885,9 +889,11 @@ describe("assistant router tool gating", () => {
|
|||||||
await expect(peekPendingAssistantApproval(missingStore, TEST_USER_ID, TEST_CONVERSATION_ID)).resolves.toBeNull();
|
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(consumePendingAssistantApproval(missingStore, TEST_USER_ID, TEST_CONVERSATION_ID)).resolves.toBeNull();
|
||||||
await expect(clearPendingAssistantApproval(missingStore, TEST_USER_ID, TEST_CONVERSATION_ID)).resolves.toBeUndefined();
|
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 () => {
|
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 missingTableError = createMissingApprovalTableError();
|
||||||
const missingStore = {
|
const missingStore = {
|
||||||
assistantApproval: {
|
assistantApproval: {
|
||||||
@@ -913,6 +919,7 @@ describe("assistant router tool gating", () => {
|
|||||||
"create_project",
|
"create_project",
|
||||||
JSON.stringify({ name: "Apollo" }),
|
JSON.stringify({ name: "Apollo" }),
|
||||||
)).rejects.toThrow("Assistant approval storage is unavailable");
|
)).rejects.toThrow("Assistant approval storage is unavailable");
|
||||||
|
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not require confirmation for read-only assistant tools", () => {
|
it("does not require confirmation for read-only assistant tools", () => {
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ const PENDING_APPROVAL_TTL_MS = 15 * 60 * 1000;
|
|||||||
export const ASSISTANT_CONFIRMATION_PREFIX = "CONFIRMATION_REQUIRED:";
|
export const ASSISTANT_CONFIRMATION_PREFIX = "CONFIRMATION_REQUIRED:";
|
||||||
const ASSISTANT_APPROVALS_TABLE_NAME = "public.assistant_approvals";
|
const ASSISTANT_APPROVALS_TABLE_NAME = "public.assistant_approvals";
|
||||||
const MAX_OPENAI_TOOL_DEFINITIONS = 128;
|
const MAX_OPENAI_TOOL_DEFINITIONS = 128;
|
||||||
|
let hasLoggedAssistantApprovalStorageUnavailable = false;
|
||||||
|
|
||||||
const ALWAYS_INCLUDED_TOOL_NAMES = new Set([
|
const ALWAYS_INCLUDED_TOOL_NAMES = new Set([
|
||||||
"get_current_user",
|
"get_current_user",
|
||||||
@@ -623,6 +624,10 @@ function isAssistantApprovalTableMissingError(error: unknown): boolean {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function logAssistantApprovalStorageUnavailable(error: unknown) {
|
function logAssistantApprovalStorageUnavailable(error: unknown) {
|
||||||
|
if (hasLoggedAssistantApprovalStorageUnavailable) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
hasLoggedAssistantApprovalStorageUnavailable = true;
|
||||||
logger.warn(
|
logger.warn(
|
||||||
{
|
{
|
||||||
err: error,
|
err: error,
|
||||||
@@ -645,6 +650,10 @@ async function withAssistantApprovalFallback<T>(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function resetAssistantApprovalStorageWarningStateForTests(): void {
|
||||||
|
hasLoggedAssistantApprovalStorageUnavailable = false;
|
||||||
|
}
|
||||||
|
|
||||||
export async function listPendingAssistantApprovals(
|
export async function listPendingAssistantApprovals(
|
||||||
db: AssistantApprovalStore,
|
db: AssistantApprovalStore,
|
||||||
userId: string,
|
userId: string,
|
||||||
|
|||||||
Reference in New Issue
Block a user