Skip to content

Commit 40b4b0a

Browse files
committed
refactor(db): tighten pool-isolation follow-ups
Assert each frozen-schema mutation throws, rank BM25 by query terms instead of copying each tool's vocabulary, gate the reporting usage cache on isPrimaryPoolClient like the auth caches, and trim comments.
1 parent dc65283 commit 40b4b0a

8 files changed

Lines changed: 29 additions & 26 deletions

File tree

‎apps/sim/lib/auth/database-context.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
11
import { AsyncLocalStorage } from 'node:async_hooks'
2-
import { type db, dbFor } from '@sim/db'
2+
import { dbFor } from '@sim/db'
33
import type { DbOrTx } from '@/lib/db/types'
44

55
const authDatabase = new AsyncLocalStorage<DbOrTx>()
66

7-
/**
8-
* The auth pool's root client. Session, adapter, and API-key lookups gate every
9-
* authenticated request, so they keep their own connections when slow work
10-
* saturates the process pool.
11-
*/
12-
export function getAuthPool(): typeof db {
7+
/** The auth pool's root client (see `DB_POOL_PROFILES.auth`). */
8+
export function getAuthPool() {
139
return dbFor('auth')
1410
}
1511

‎apps/sim/lib/billing/core/reporting-usage-cache.integration.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,15 @@ const { transaction, summedPeriods } = vi.hoisted(() => ({
2323
const databaseUrl = readTestDatabaseUrl()
2424
const redisUrl = readTestRedisUrl()
2525

26-
vi.mock('@sim/db', () => ({ db: { transaction }, dbFor: () => ({ transaction }), dbReplica: {} }))
26+
vi.mock('@sim/db', () => {
27+
const db = { transaction }
28+
return {
29+
db,
30+
dbFor: () => ({ transaction }),
31+
dbReplica: {},
32+
isPrimaryPoolClient: (executor: object) => executor === db,
33+
}
34+
})
2735
vi.mock('@/lib/core/config/redis', () => redisConfigMock)
2836
vi.mock('@/lib/billing/core/usage-log', async (importOriginal) => {
2937
const actual = await importOriginal<typeof import('@/lib/billing/core/usage-log')>()

‎apps/sim/lib/billing/core/reporting-usage-cache.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { db } from '@sim/db'
1+
import { db, isPrimaryPoolClient } from '@sim/db'
22
import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { generateShortId } from '@sim/utils/id'
@@ -417,7 +417,7 @@ export function readSoftGateUsageCost(
417417
period: UsageQueryPeriod,
418418
executor: DbClient = db
419419
): Promise<number> {
420-
if (isReportingPeriod(period) && executor === db) {
420+
if (isReportingPeriod(period) && isPrimaryPoolClient(executor)) {
421421
return readCachedReportingUsageCost(entity, period)
422422
}
423423
return getBillingPeriodUsageCost(entity, period, undefined, executor)

‎apps/sim/lib/mothership/chat/payload.test.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,11 +292,13 @@ describe('buildIntegrationToolSchemas', () => {
292292
})
293293
const first = await buildIntegrationToolSchemas('user-mutating')
294294
first.push({ name: 'mcp-private-tool', description: '', input_schema: {} })
295-
try {
295+
expect(() => {
296296
;(first[0].input_schema.properties as Record<string, unknown>).recipients = { type: 'number' }
297-
;(first[0].input_schema.required as string[]).push('forged')
297+
}).toThrow(TypeError)
298+
expect(() => (first[0].input_schema.required as string[]).push('forged')).toThrow(TypeError)
299+
expect(() => {
298300
first[0].description = 'forged'
299-
} catch {}
301+
}).toThrow(TypeError)
300302

301303
const second = await buildIntegrationToolSchemas('user-reading')
302304
expect(second.map((tool) => tool.name)).toEqual([

‎apps/sim/lib/mothership/chat/payload.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,9 @@ type IntegrationToolSchemaVariantOptions = Required<
138138
/**
139139
* Every exposed integration tool's schema for one surface and mode, by tool id.
140140
*
141-
* Depends only on the tool registry and deployment config, never on the viewer, so it is
142-
* built once per process and shared: the entries are frozen and must not be mutated.
141+
* Depends only on the tool registry and deployment config (`isHosted`, OAuth availability),
142+
* fixed for the process's life, never on the viewer, so it is built once per process and
143+
* shared: the entries are frozen and must not be mutated.
143144
* Viewer gating (block visibility, permission groups, the deployment allowlist) picks
144145
* which entries a request returns. Tools whose description gains the free-plan email
145146
* tagline also get a tagged copy in `withEmailTagline`, sharing the same input schema.

‎apps/sim/lib/mothership/integrations/application/catalog.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,10 @@ function rankByBm25(tools: ToolSchema[], queryTerms: string[]): number[] {
135135
const documents = tools.map((tool) => {
136136
const stats = getOperationTermStats(tool)
137137
const frequencies = new Map<string, number>()
138-
for (const [term, frequency] of stats.frequencies)
139-
if (query.has(term)) frequencies.set(term, frequency)
138+
for (const term of query) {
139+
const frequency = stats.frequencies.get(term)
140+
if (frequency) frequencies.set(term, frequency)
141+
}
140142
return { length: stats.length, frequencies }
141143
})
142144
const averageLength =

‎apps/sim/lib/workflows/deployment-status.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,8 @@ import type { WorkflowState } from '@/stores/workflows/workflow/types'
2020
* draft, so that the server and the client answered the same question
2121
* differently for the same workflow.
2222
*
23-
* Every read — the active version row, the normalized draft, its variables and
24-
* the credential rows the block migrations consult — runs in one REPEATABLE READ
25-
* transaction so both operands come from the same snapshot. The diff does not:
26-
* it is pure CPU over values already read, and running it after commit returns
27-
* the pooled connection instead of holding it idle in transaction.
23+
* All reads share one REPEATABLE READ snapshot; the CPU-only diff runs after
24+
* commit so the connection is not held idle in transaction.
2825
*/
2926
export async function checkNeedsRedeployment(workflowId: string): Promise<boolean> {
3027
const snapshot = await db.transaction(async (tx) => {

‎packages/testing/src/mocks/database.mock.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,10 +477,7 @@ function resolveDbUrl(
477477
*/
478478
export const databaseMockFns = {
479479
mockResolveDbUrl: vi.fn(resolveDbUrl),
480-
/**
481-
* Recognizes only the shared instance by default; a test that needs a distinct
482-
* sub-pool client marks it as a primary root client here.
483-
*/
480+
/** Recognizes only the shared instance unless a test marks its own client. */
484481
mockIsPrimaryPoolClient,
485482
mockEnsureRowCountTriggers: vi.fn(async (): Promise<void> => {}),
486483
mockVerifyRowCountTriggers: vi.fn(async () => ({

0 commit comments

Comments
 (0)