diff --git a/apps/sim/lib/core/config/trigger-runtime.test.ts b/apps/sim/lib/core/config/trigger-runtime.test.ts new file mode 100644 index 00000000000..bb905e0667f --- /dev/null +++ b/apps/sim/lib/core/config/trigger-runtime.test.ts @@ -0,0 +1,56 @@ +/** + * @vitest-environment node + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockTaskContext } = vi.hoisted(() => ({ + mockTaskContext: { isInsideTask: false }, +})) + +vi.mock('@trigger.dev/core/v3', () => ({ + taskContext: mockTaskContext, +})) + +import { + isInsideTriggerRun, + markInsideTriggerRun, + resetInsideTriggerRunForTests, +} from '@/lib/core/config/trigger-runtime' + +describe('trigger runtime detection', () => { + beforeEach(() => { + mockTaskContext.isInsideTask = false + resetInsideTriggerRunForTests() + }) + + afterEach(() => { + mockTaskContext.isInsideTask = false + resetInsideTriggerRunForTests() + }) + + it('reports no run when neither signal is present', () => { + expect(isInsideTriggerRun()).toBe(false) + }) + + it('reports a run from the SDK ambient task context alone', () => { + mockTaskContext.isInsideTask = true + expect(isInsideTriggerRun()).toBe(true) + }) + + it('reports a run from the init-hook marker alone', () => { + markInsideTriggerRun() + expect(isInsideTriggerRun()).toBe(true) + }) + + it('is idempotent when marked repeatedly', () => { + markInsideTriggerRun() + markInsideTriggerRun() + expect(isInsideTriggerRun()).toBe(true) + }) + + it('keeps the marker on globalThis so a duplicated bundle still sees it', () => { + markInsideTriggerRun() + const carrier = globalThis as Record + expect(carrier[Symbol.for('sim.trigger-dev.inside-run')]).toBe(true) + }) +}) diff --git a/apps/sim/lib/core/config/trigger-runtime.ts b/apps/sim/lib/core/config/trigger-runtime.ts new file mode 100644 index 00000000000..edac6f20238 --- /dev/null +++ b/apps/sim/lib/core/config/trigger-runtime.ts @@ -0,0 +1,56 @@ +import { taskContext } from '@trigger.dev/core/v3' + +/** + * Carrier key for the marker written by the global `init` lifecycle hook. + * + * Parked on `globalThis` under a registered symbol rather than held in module + * scope: the Trigger.dev build bundles the config entrypoint alongside the task + * graph, and a module-level binding duplicated across bundles would let the + * hook write one copy while dispatch code reads another. + */ +const INSIDE_TRIGGER_RUN = Symbol.for('sim.trigger-dev.inside-run') + +interface TriggerRunCarrier { + [INSIDE_TRIGGER_RUN]?: true +} + +/** + * Records that this process is executing a Trigger.dev run. Idempotent; called + * from the global `init` lifecycle hook in `trigger.config.ts`, which + * Trigger.dev documents as running before any task run. + * + * @see https://trigger.dev/docs/config/config-file#lifecycle-functions + */ +export function markInsideTriggerRun(): void { + ;(globalThis as TriggerRunCarrier)[INSIDE_TRIGGER_RUN] = true +} + +/** + * Whether this process is executing a Trigger.dev run — the question that makes + * dispatch decisions independent of which environment variables a given + * container happens to have. + * + * Two independent signals, because this has been got wrong twice and either one + * alone is a single point of failure: + * + * 1. `taskContext.isInsideTask`, the SDK runtime's own ambient flag. Already + * load-bearing in `getAsyncBackendType` for the same carve-out, so it is + * proven in this codebase rather than assumed. + * 2. The `init`-hook marker, which uses only the public, documented lifecycle + * surface and so holds even if the internal `taskContext` shape moves. + * + * Neither signal is derived from `TRIGGER_SECRET_KEY` or `TRIGGER_DEV_ENABLED`. + * Both are guesses about a process that Trigger.dev has already proven it owns + * by being the thing running it. + */ +export function isInsideTriggerRun(): boolean { + return taskContext.isInsideTask || (globalThis as TriggerRunCarrier)[INSIDE_TRIGGER_RUN] === true +} + +/** + * Clears the `init`-hook marker. Test-only: production processes are either a + * Trigger.dev worker for their whole lifetime or never one. + */ +export function resetInsideTriggerRunForTests(): void { + delete (globalThis as TriggerRunCarrier)[INSIDE_TRIGGER_RUN] +} diff --git a/apps/sim/lib/knowledge/documents/processing-queue.test.ts b/apps/sim/lib/knowledge/documents/processing-queue.test.ts index 5e090255f0a..1490a48f50a 100644 --- a/apps/sim/lib/knowledge/documents/processing-queue.test.ts +++ b/apps/sim/lib/knowledge/documents/processing-queue.test.ts @@ -8,9 +8,13 @@ import { resetEnvFlagsMock, setEnvFlags, } from '@sim/testing' -import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' import { env } from '@/lib/core/config/env' +import { + markInsideTriggerRun, + resetInsideTriggerRunForTests, +} from '@/lib/core/config/trigger-runtime' const { mockBatchTrigger } = vi.hoisted(() => ({ mockBatchTrigger: vi.fn(), @@ -153,3 +157,94 @@ describe('processDocumentsWithQueue billing attribution', () => { expect(jobs[0].payload).not.toHaveProperty('billingAttribution') }) }) + +/** + * The per-document fan-out was inert in production because `isTriggerAvailable()` + * inferred availability from environment variables that the app container sets + * and the Trigger.dev worker does not. A run process is authoritative about its + * own runtime, so the marker has to beat both environment conjuncts. + */ +describe('processDocumentsWithQueue dispatch backend', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + resetInsideTriggerRunForTests() + mockBatchTrigger.mockResolvedValue({ batchId: 'batch-1' }) + for (const key of Object.keys(env)) { + delete (env as Record)[key] + } + Object.assign(env, { ...defaultMockEnv }) + ;(env as Record).TRIGGER_SECRET_KEY = undefined + dbChainMockFns.limit.mockResolvedValue([ + { userId: 'knowledge-owner', workspaceId: 'workspace-1' }, + ]) + }) + + afterEach(() => { + resetInsideTriggerRunForTests() + setEnvFlags({ isTriggerDevEnabled: true }) + }) + + it('dispatches via Trigger.dev inside a run with neither env conjunct satisfied', async () => { + setEnvFlags({ isTriggerDevEnabled: false }) + markInsideTriggerRun() + + await processDocumentsWithQueue( + [DOCUMENT], + 'knowledge-base-1', + {}, + 'request-1', + BILLING_ATTRIBUTION + ) + + expect(mockBatchTrigger).toHaveBeenCalledTimes(1) + }) + + it('dispatches via Trigger.dev inside a run when only the secret key is missing', async () => { + setEnvFlags({ isTriggerDevEnabled: true }) + markInsideTriggerRun() + + await processDocumentsWithQueue( + [DOCUMENT], + 'knowledge-base-1', + {}, + 'request-1', + BILLING_ATTRIBUTION + ) + + expect(mockBatchTrigger).toHaveBeenCalledTimes(1) + }) + + it('does not dispatch via Trigger.dev outside a run when the secret key is missing', async () => { + setEnvFlags({ isTriggerDevEnabled: true }) + + await expect( + processDocumentsWithQueue( + [DOCUMENT], + 'knowledge-base-1', + {}, + 'request-1', + BILLING_ATTRIBUTION + ) + ).rejects.toThrow() + + expect(mockBatchTrigger).not.toHaveBeenCalled() + }) + + it('does not dispatch via Trigger.dev outside a run when the deployment flag is off', async () => { + setEnvFlags({ isTriggerDevEnabled: false }) + Object.assign(env, { TRIGGER_SECRET_KEY: 'trigger-secret' }) + + await expect( + processDocumentsWithQueue( + [DOCUMENT], + 'knowledge-base-1', + {}, + 'request-1', + BILLING_ATTRIBUTION + ) + ).rejects.toThrow() + + expect(mockBatchTrigger).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/knowledge/documents/service.ts b/apps/sim/lib/knowledge/documents/service.ts index ffdb9a9366d..d3df5ea79cd 100644 --- a/apps/sim/lib/knowledge/documents/service.ts +++ b/apps/sim/lib/knowledge/documents/service.ts @@ -56,6 +56,7 @@ import type { ChunkingStrategy, StrategyOptions } from '@/lib/chunkers/types' import { resolveTriggerRegion } from '@/lib/core/async-jobs/region' import { env, envNumber } from '@/lib/core/config/env' import { getCostMultiplier, isTriggerDevEnabled } from '@/lib/core/config/env-flags' +import { isInsideTriggerRun } from '@/lib/core/config/trigger-runtime' import { OrchestrationError } from '@/lib/core/orchestration/types' import { mapWithConcurrency } from '@/lib/core/utils/concurrency' import { @@ -1315,8 +1316,42 @@ export async function processDocumentAsync( } } +let triggerAvailabilityLogged = false + +/** + * Whether background work may be dispatched to Trigger.dev rather than run + * in-process. + * + * Inside a Trigger.dev run the answer is unconditionally yes: the platform is + * what is executing this process, so no environment guess can be more reliable + * than the run marker. Outside a run the deployment must both enable + * Trigger.dev and hold the secret key the SDK authenticates with. + * + * Resolving `true` inside a run is safe even if the run process turns out not + * to expose `TRIGGER_SECRET_KEY`: the SDK would then reject the batch trigger + * and `dispatchViaBatchTrigger` falls back to processing in-process, which is + * exactly where a `false` predicate lands anyway. + * + * The first evaluation in a process logs the resolved inputs. That is once per + * worker process rather than once per dispatch, and it is the signal that makes + * an app-vs-worker asymmetry visible without reading a crashed run's spans. + */ export function isTriggerAvailable(): boolean { - return Boolean(env.TRIGGER_SECRET_KEY) && isTriggerDevEnabled + const insideRun = isInsideTriggerRun() + const hasSecretKey = Boolean(env.TRIGGER_SECRET_KEY) + const available = insideRun || (hasSecretKey && isTriggerDevEnabled) + + if (!triggerAvailabilityLogged) { + triggerAvailabilityLogged = true + logger.info('Resolved Trigger.dev dispatch availability', { + available, + insideTriggerRun: insideRun, + triggerDevEnabled: isTriggerDevEnabled, + hasSecretKey, + }) + } + + return available } type DocumentStorageBilling = diff --git a/apps/sim/trigger.config.ts b/apps/sim/trigger.config.ts index 160c2808518..27d8746d536 100644 --- a/apps/sim/trigger.config.ts +++ b/apps/sim/trigger.config.ts @@ -9,6 +9,7 @@ import { } from '@trigger.dev/build/extensions/core' import { defineConfig } from '@trigger.dev/sdk' import { env } from './lib/core/config/env' +import { markInsideTriggerRun } from './lib/core/config/trigger-runtime' import { parseOtlpHeaders } from './lib/monitoring/otlp' const grafanaEndpoint = env.GRAFANA_OTLP_ENDPOINT @@ -58,6 +59,17 @@ export default defineConfig({ }, }, dirs: ['./background'], + /** + * Runs before any task run, in the run process. Marks the process so that + * dispatch decisions further down the call graph stop inferring from + * environment variables whether Trigger.dev is available: a process that + * Trigger.dev is executing has Trigger.dev available by definition. + * + * @see https://trigger.dev/docs/config/config-file#lifecycle-functions + */ + init: () => { + markInsideTriggerRun() + }, ...(grafanaTelemetry ? { telemetry: grafanaTelemetry } : {}), build: { external: [ @@ -77,10 +89,9 @@ export default defineConfig({ { name: 'DB_APP_NAME', value: 'sim-trigger' }, /** * Workers run Trigger.dev by definition, but the flag saying so was only - * set on the app container, so `isTriggerAvailable()` was false in every - * task run and dispatched work silently took the in-process fallback. - * Ineffective where dispatching is impossible: the check also requires - * TRIGGER_SECRET_KEY, which only the Trigger.dev runtime provides. + * set on the app container. Syncing it keeps the deployment flag honest + * inside runs; the dispatch decision itself no longer depends on it, + * because the `init` hook above marks the run process directly. */ { name: 'TRIGGER_DEV_ENABLED', value: 'TRUE' }, ]),