diff --git a/apps/sim/app/api/v1/auth.ts b/apps/sim/app/api/v1/auth.ts index bb94b6952ea..1d9d4d14d1d 100644 --- a/apps/sim/app/api/v1/auth.ts +++ b/apps/sim/app/api/v1/auth.ts @@ -74,7 +74,7 @@ export async function authenticateV1Request(request: NextRequest): Promise { describe('updateApiKeyLastUsed', () => { it('only writes when the stored lastUsed is missing or stale', async () => { - await updateApiKeyLastUsed('key-1') + updateApiKeyLastUsed('key-1') + await flushMacrotask() expect(dbChainMockFns.update).toHaveBeenCalledTimes(1) expect(dbChainMockFns.set).toHaveBeenCalledWith({ lastUsed: expect.any(Date) }) @@ -113,12 +115,12 @@ describe('updateApiKeyLastUsed', () => { }) }) - it('swallows database errors instead of failing the request', async () => { + it('logs database errors instead of failing the request', async () => { dbChainMockFns.update.mockImplementationOnce(() => { throw new Error('connection lost') }) - await expect(updateApiKeyLastUsed('key-1')).resolves.toBeUndefined() - expect(serviceLogger.error).toHaveBeenCalled() + expect(() => updateApiKeyLastUsed('key-failing')).not.toThrow() + await vi.waitFor(() => expect(serviceLogger.error).toHaveBeenCalled()) }) }) diff --git a/apps/sim/lib/api-key/service.ts b/apps/sim/lib/api-key/service.ts index 0ca2c46abb7..d8edaa27d00 100644 --- a/apps/sim/lib/api-key/service.ts +++ b/apps/sim/lib/api-key/service.ts @@ -3,6 +3,7 @@ import { apiKey as apiKeyTable, user as userTable } from '@sim/db/schema' import { createLogger, setRequestAuth } from '@sim/logger' import { and, eq, isNull, lt, or } from 'drizzle-orm' import { hashApiKey } from '@/lib/api-key/crypto' +import { createDetachedTouch } from '@/lib/core/utils/background' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' import { getWorkspaceBillingSettings, type WorkspaceBillingSettings } from '@/lib/workspaces/utils' @@ -136,17 +137,26 @@ export async function authenticateApiKeyFromHeader( const LAST_USED_STALENESS_WINDOW_MS = 10 * 60 * 1000 +const touchApiKeyLastUsed = createDetachedTouch({ + label: 'API key last used', + write: writeLastUsed, + debounce: { intervalMs: LAST_USED_STALENESS_WINDOW_MS, maxKeys: 10_000 }, +}) + /** - * Update the last used timestamp for an API key. + * Record that an API key was used, without delaying the request. * - * `lastUsed` is display-only, so the write uses a staleness window: it only - * fires when the stored value is older than - * {@link LAST_USED_STALENESS_WINDOW_MS}. High-traffic keys otherwise rewrite - * the same row on every request, serializing concurrent requests behind row - * locks. The 10-minute window matches GitLab's personal-access-token - * last-used tracking. + * `lastUsed` is display-only, so the write is detached (see {@link createDetachedTouch}): a + * commit waiting on the database must never hold up an authenticated request. Across processes + * the write only fires when the stored value is older than {@link LAST_USED_STALENESS_WINDOW_MS}, + * so high-traffic keys do not rewrite the same row on every request. The 10-minute window matches + * GitLab's personal-access-token last-used tracking. */ -export async function updateApiKeyLastUsed(keyId: string): Promise { +export function updateApiKeyLastUsed(keyId: string): void { + touchApiKeyLastUsed(keyId) +} + +async function writeLastUsed(keyId: string): Promise { try { const staleBefore = new Date(Date.now() - LAST_USED_STALENESS_WINDOW_MS) await db diff --git a/apps/sim/lib/api/server/routes/v2-api-key-auth.ts b/apps/sim/lib/api/server/routes/v2-api-key-auth.ts index e84b81a73df..133d60a9310 100644 --- a/apps/sim/lib/api/server/routes/v2-api-key-auth.ts +++ b/apps/sim/lib/api/server/routes/v2-api-key-auth.ts @@ -120,7 +120,7 @@ async function authenticateApiKey(apiKeyHeader: string): Promise { it('swallows rejections so they do not surface as unhandled', async () => { @@ -20,3 +21,65 @@ describe('runDetached', () => { await flushMacrotask() }) }) + +describe('createDetachedTouch', () => { + const pendingWrite = () => { + let settle = () => {} + const promise = new Promise((resolve) => { + settle = resolve + }) + return { promise, settle } + } + + it('returns before the write settles and skips repeats inside the interval', () => { + const write = vi.fn(() => new Promise(() => {})) + const touch = createDetachedTouch({ + label: 'test', + write, + debounce: { intervalMs: 60_000, maxKeys: 10 }, + }) + + expect(touch('a')).toBeUndefined() + touch('a') + touch('b') + + return flushMacrotask().then(() => { + expect(write.mock.calls).toEqual([['a'], ['b']]) + }) + }) + + it('never starts a second write for a key while its first is still pending', async () => { + const first = pendingWrite() + const write = vi.fn().mockReturnValueOnce(first.promise).mockResolvedValue(undefined) + const touch = createDetachedTouch({ + label: 'test', + write, + debounce: { intervalMs: 1, maxKeys: 1 }, + }) + + touch('a') + await sleep(5) + touch('b') + touch('a') + await flushMacrotask() + expect(write.mock.calls).toEqual([['a'], ['b']]) + + first.settle() + await flushMacrotask() + await sleep(5) + touch('a') + await flushMacrotask() + expect(write.mock.calls).toEqual([['a'], ['b'], ['a']]) + }) + + it('releases a key whose write failed', async () => { + const write = vi.fn().mockRejectedValueOnce(new Error('boom')).mockResolvedValue(undefined) + const touch = createDetachedTouch({ label: 'test', write }) + + touch('a') + await flushMacrotask() + touch('a') + await flushMacrotask() + expect(write).toHaveBeenCalledTimes(2) + }) +}) diff --git a/apps/sim/lib/core/utils/background.ts b/apps/sim/lib/core/utils/background.ts index 5f3d4f7d6c4..5c54fa72e83 100644 --- a/apps/sim/lib/core/utils/background.ts +++ b/apps/sim/lib/core/utils/background.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' +import { LRUCache } from 'lru-cache' const logger = createLogger('BackgroundTask') @@ -24,3 +25,46 @@ export function runDetached(label: string, work: () => Promise): void { logger.error(`Background task failed: ${label}`, toError(error)) }) } + +interface DetachedTouchOptions { + /** Identifier used in the failure log line. */ + label: string + /** The write itself, usually a guarded single-row timestamp update. */ + write: (key: string) => Promise + /** + * Skip a key written by this process within `intervalMs`, remembering at most `maxKeys`. Only + * for a value nothing reads as fresh: the process-local window adds to whatever staleness the + * write's own guard allows. + */ + debounce?: { intervalMs: number; maxKeys: number } +} + +/** + * A best-effort per-key write (a last-used or last-seen timestamp) that the caller never waits + * on, and that never starts while the same key's previous write is still pending: a commit + * stalled on the database would otherwise hold every caller that touches the row, and each repeat + * would queue behind its row lock holding a pool connection. In-flight keys are tracked apart + * from the optional debounce cache, so neither expiry nor eviction can start an overlapping write. + */ +export function createDetachedTouch({ + label, + write, + debounce, +}: DetachedTouchOptions): (key: string) => void { + const recent = debounce + ? new LRUCache({ max: debounce.maxKeys, ttl: debounce.intervalMs }) + : null + const inFlight = new Set() + return (key) => { + if (inFlight.has(key) || recent?.has(key)) return + recent?.set(key, true) + inFlight.add(key) + runDetached(label, async () => { + try { + await write(key) + } finally { + inFlight.delete(key) + } + }) + } +} diff --git a/apps/sim/lib/desktop/application/executor.ts b/apps/sim/lib/desktop/application/executor.ts index cc0d2e19bae..493f1a4f145 100644 --- a/apps/sim/lib/desktop/application/executor.ts +++ b/apps/sim/lib/desktop/application/executor.ts @@ -5,6 +5,7 @@ import { generateId } from '@sim/utils/id' import { isPlainRecord, omit } from '@sim/utils/object' import { defineOperation } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { createDetachedTouch } from '@/lib/core/utils/background' import { type CredentialUserAuditEntry, defineAuthorizedCredentialUserUseCase, @@ -58,6 +59,16 @@ import { const logger = createLogger('DesktopExecutor') +/** + * An inbox poll never waits on its `lastSeenAt` write or stacks one behind another. Not debounced + * in-process: the offline check reads `lastSeenAt`, and the write's own SQL guard already bounds + * how often it lands. + */ +const touchDeviceLastSeen = createDetachedTouch({ + label: 'desktop device last seen', + write: touchDesktopDevice, +}) + type DeviceInput = { deviceId: string } /** Every executor request must come from the session the device registered under. */ @@ -169,10 +180,10 @@ export const listDesktopInbox = defineAuthorizedCredentialUserUseCase({ input: DeviceInput }): Promise<{ items: DesktopInboxEntry[] }> { await requireBoundDevice(principal, input.deviceId) + touchDeviceLastSeen(input.deviceId) const [rows] = await Promise.all([ listDesktopInboxRows({ deviceId: input.deviceId, userId: principal.userId }), markDesktopPresent(input.deviceId), - touchDesktopDevice(input.deviceId), ]) return { items: classifyDesktopInbox(rows) } }, diff --git a/apps/sim/lib/workflows/api/route-policies.ts b/apps/sim/lib/workflows/api/route-policies.ts index c60e1fe7308..b777a8a1a27 100644 --- a/apps/sim/lib/workflows/api/route-policies.ts +++ b/apps/sim/lib/workflows/api/route-policies.ts @@ -103,7 +103,7 @@ async function authenticateWorkflowApiKey(rawApiKey: string): Promise