feat: Sessions dashboard, task_kind, and chat-ready hardening (1/4)#3542
feat: Sessions dashboard, task_kind, and chat-ready hardening (1/4)#3542ericallam wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: f240799 The changes in this PR will be included in the next version bump. This PR includes changesets to release 29 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces sessions listing/detail UI and server presenters, realtime session-stream SSE/JSON endpoints, and a session stream manager in the core SDK. Adds run “source” filtering with taskKind propagation end-to-end, including icons and filters in the runs UI, repositories, and presenters. Extends API client with sessions lifecycle and stream operations. Updates ClickHouse and Prisma schemas for task kind and playground conversations. Adds input/session streams management APIs, improved SSE retry logic, and tests. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 getTaskQueueInfo early-return path omits taskKind, misclassifying AGENT/SCHEDULED runs as STANDARD
When getTaskQueueInfo is called for a non-locked-worker trigger that provides both a queue override and a per-trigger TTL, the early return at line 219 skips the DB query entirely and returns { queueName, taskTtl: undefined } without taskKind. The caller in triggerTask.server.ts:316 then falls through to taskKind ?? "STANDARD", permanently stamping AGENT or SCHEDULED runs as STANDARD in the ClickHouse task_kind column.
This directly contradicts the PR's own intent — the comment block at queues.server.ts:110-116 on the locked-worker path explicitly says "Always fetch the task so we can resolve triggerSource" to avoid exactly this bug. But the non-locked-worker path has the same early-return optimization from before the PR that was never updated to also fetch triggerSource.
Affected scenario: any AGENT task triggered via the API/SDK with queue: { name: "custom" } and ttl: "5m" (both set). The run will appear as "Standard" in the dashboard's Source filter and won't match a sources=["AGENT"] filter.
(Refers to lines 218-219)
Prompt for agents
In `getTaskQueueInfo` at apps/webapp/app/runEngine/concerns/queues.server.ts, the early-return at line 218-219 fires when both `overriddenQueueName` and `body.options?.ttl` are set and skips the DB query entirely — returning no `taskKind`. This means AGENT and SCHEDULED runs hitting this path get misclassified as STANDARD in ClickHouse.
The fix should either:
1. Remove this early-return optimization entirely (simplest — the subsequent DB queries are cheap and already cached by the replica). The locked-worker path at line 110-129 already does this: it always fetches the task row.
2. Or keep the optimization but still query `backgroundWorkerTask.findFirst` for just `triggerSource` so `taskKind` is populated. This is what the locked-worker path does at lines 117-129.
The same issue affects the `no worker found` fallback at line 231 (`return { queueName: overriddenQueueName ?? defaultQueueName, taskTtl: undefined }`) and the `no task found` fallback at line 265 — though those are edge cases where there's no task row to look up. Still, the line-218 path is the common case and must be fixed.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (11)
packages/core/src/v3/test/test-realtime-streams-manager.ts-158-161 (1)
158-161:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
reset()leakswriteListenersacross tests.
reset()clearsbuffersandpipeWaitsbut leaveswriteListenersintact. For a manager whose primary purpose is per-test isolation, any listener registered in one test (and not explicitly unsubscribed) will continue to fire for writes in subsequent tests that share this instance, leading to flaky/cross-talking tests.🧹 Proposed fix
reset(): void { this.buffers.clear(); this.pipeWaits.clear(); + this.writeListeners.clear(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/test/test-realtime-streams-manager.ts` around lines 158 - 161, The reset() method currently clears buffers and pipeWaits but leaves writeListeners populated, causing listeners to leak between tests; update the reset() implementation (the reset() method in the test realtime streams manager) to also remove all registered write listeners (e.g., clear or reinitialize the writeListeners collection) so any callbacks registered via writeListeners are unsubscribed/cleared between tests to ensure per-test isolation.packages/core/src/v3/apiClient/runStream.ts-480-491 (1)
480-491:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAbort during backoff is not wakeable
The retry delay promise only listens to
retryNowController. If callersignalaborts during backoff, shutdown can be delayed until the timer expires.Suggested fix
this.retryNowController = new AbortController(); await new Promise<void>((resolve) => { + if (this.options.signal?.aborted) { + resolve(); + return; + } const timer = setTimeout(() => { this.retryNowController?.signal.removeEventListener("abort", onAbort); + this.options.signal?.removeEventListener("abort", onUserAbort); resolve(); }, delay); const onAbort = () => { clearTimeout(timer); + this.options.signal?.removeEventListener("abort", onUserAbort); + resolve(); + }; + const onUserAbort = () => { + clearTimeout(timer); + this.retryNowController?.signal.removeEventListener("abort", onAbort); resolve(); }; this.retryNowController!.signal.addEventListener("abort", onAbort, { once: true }); + this.options.signal?.addEventListener("abort", onUserAbort, { once: true }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/apiClient/runStream.ts` around lines 480 - 491, The backoff promise only listens to this.retryNowController and ignores the caller's abort signal, causing shutdown to wait for the timer; update the wait logic in runStream.ts to also listen to the external/caller signal (e.g., this.signal or the passed-in signal) alongside this.retryNowController by adding an event listener on that signal which clears the timeout, resolves the promise, and removes both listeners (mirror the existing onAbort cleanup for retryNowController.signal and the timer) to ensure immediate wake-up on caller abort while preventing listener leaks.packages/core/src/v3/inputStreams/manager.ts-181-188 (1)
181-188:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
disconnectStreammay be silently undone by auto-reconnect when handlers or waiters still exist.The tail's
.finallyblock auto-reconnects wheneverthis.handlersorthis.onceWaitersstill have entries forstreamId. If any.on()handler or pending.once()waiter is active whendisconnectStreamruns, the abort will complete but the tail immediately reconnects—negating the contract: "disconnect before.wait()suspends so the tail doesn't buffer duplicates delivered through the waitpoint path."Add an
explicitlyDisconnectedflag (as already implemented insessionStreams/manager.ts) to prevent auto-reconnect after intentional disconnect. Mark the stream before abort, check the flag in the.finallyreconnect branch, and clear it on the nexton()/once()call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/inputStreams/manager.ts` around lines 181 - 188, The disconnectStream implementation currently aborts and removes the tail but can be immediately undone by the tail's .finally auto-reconnect if handlers or onceWaiters still reference streamId; add an explicitlyDisconnected flag (same pattern used in sessionStreams/manager.ts) to the manager: set explicitlyDisconnected[streamId] = true before calling tail.abortController.abort() in disconnectStream, check explicitlyDisconnected inside the tail's .finally reconnect branch to skip reconnect when true, and ensure explicitlyDisconnected[streamId] is cleared when a new .on() or .once() call for that streamId occurs so reconnects resume normally.packages/core/src/v3/test/test-session-stream-manager.ts-215-247 (1)
215-247:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandler invocation in
__sendFromTestdoesn't use the safeinvokehelper — and the helper itself is dead.
__sendFromTestcalls handlers directly viaArray.from(handlers).map((h) => Promise.resolve().then(() => h(data)))(Line 225). A thrown/rejected handler will reject thePromise.alland propagate out of__sendFromTest, which is the opposite of the comment on Line 278 ("Never let a handler error break test state"). Meanwhile, theinvokeprivate method defined on Lines 270-279 (which does exactly that catch) is never referenced anywhere in the file — dead code.Either drop
invoke, or route handler dispatch through it so a misbehaving handler doesn't blow up the test harness.🛡️ Suggested fix
const handlers = this.handlers.get(key); if (handlers && handlers.size > 0) { - await Promise.all( - Array.from(handlers).map((h) => Promise.resolve().then(() => h(data))) - ); + await Promise.all( + Array.from(handlers).map(async (h) => { + try { + await h(data); + } catch { + // Never let a handler error break test state + } + }) + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/test/test-session-stream-manager.ts` around lines 215 - 247, The handler loop in __sendFromTest currently calls handlers directly with Promise.all(Array.from(handlers).map((h) => Promise.resolve().then(() => h(data)))) so a thrown/rejected handler will reject the whole send; instead reuse the existing private invoke helper (or restore its implementation) to swallow or handle handler errors as intended. Replace the direct handler invocation in __sendFromTest to call this.invoke(h, data) for each handler (i.e., Promise.resolve().then(() => this.invoke(h, data)) or map to this.invoke) so individual handler failures are caught and do not break test state; if invoke is currently dead/incorrect, fix its implementation to catch errors (and optionally log) and return a resolved promise. Ensure the change references the handlers map and the invoke method so behavior is consistent with the "Never let a handler error break test state" comment.packages/core/src/v3/sessionStreams/manager.ts-121-145 (1)
121-145:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAbort listener and timeout handle leak on normal
once()resolution.When a
once()waiter resolves via#dispatch(Line 359), neither thesignal'sabortlistener nor the unused timeout handle are removed/cleared. Theabortlistener is registered with{ once: true }, so it self-clears if the signal eventually fires, but in the common case where the signal outlives thisonce()(e.g., a long-lived run-levelAbortSignalreused across manyonce()calls), the listener — which captures thewaiterandthis— accumulates on the signal until it finally aborts.#dispatchshould also clear the timeout handle (it currently only clears it on Line 358 for the leading waiter viaclearTimeout) — wait, it does — but it never invokessignal.removeEventListener.Compare with
TestSessionStreamManagerwhich keepsabortHandleron the waiter and removes it on dispatch (packages/core/src/v3/test/test-session-stream-manager.ts, Lines 234-236).🛡️ Suggested fix
type OnceWaiter = { resolve: (result: InputStreamOnceResult<unknown>) => void; reject: (error: Error) => void; timeoutHandle?: ReturnType<typeof setTimeout>; + signal?: AbortSignal; + abortHandler?: () => void; };return new InputStreamOncePromise<unknown>((resolve, reject) => { const waiter: OnceWaiter = { resolve, reject }; if (options?.signal) { if (options.signal.aborted) { reject(new Error("Aborted")); return; } - options.signal.addEventListener( - "abort", - () => { - if (waiter.timeoutHandle) clearTimeout(waiter.timeoutHandle); - this.#removeOnceWaiter(key, waiter); - reject(new Error("Aborted")); - }, - { once: true } - ); + const abortHandler = () => { + if (waiter.timeoutHandle) clearTimeout(waiter.timeoutHandle); + this.#removeOnceWaiter(key, waiter); + reject(new Error("Aborted")); + }; + waiter.signal = options.signal; + waiter.abortHandler = abortHandler; + options.signal.addEventListener("abort", abortHandler, { once: true }); }And in
#dispatch(and the buffered-shift path) clear the listener when the waiter is resolved:const waiter = waiters.shift()!; if (waiters.length === 0) this.onceWaiters.delete(key); if (waiter.timeoutHandle) clearTimeout(waiter.timeoutHandle); + if (waiter.signal && waiter.abortHandler) { + waiter.signal.removeEventListener("abort", waiter.abortHandler); + } waiter.resolve({ ok: true, output: data });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/sessionStreams/manager.ts` around lines 121 - 145, The once() waiter installs an abort listener and timeout but does not remove the abort listener when the waiter is resolved via `#dispatch`, causing listener/closure leaks; update once() to store the abort handler on the waiter (e.g., waiter.abortHandler) when adding options.signal.addEventListener, and in `#dispatch` (and any buffered-shift resolution path) call options.signal.removeEventListener("abort", waiter.abortHandler) and clearTimeout(waiter.timeoutHandle) when resolving the waiter so both the listener and timeout are cleaned up; reference the waiter object used in once() and the `#dispatch` method to apply these removals.apps/webapp/app/services/runsRepository/runsRepository.server.ts-45-46 (1)
45-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConstrain
taskKindsto known values.Line 45 currently accepts any string, so invalid values can silently flow into filtering and produce confusing empty results. Tightening this to known task kinds improves input correctness.
Suggested diff
const RunStatus = z.enum(Object.values(TaskRunStatus) as [TaskRunStatus, ...TaskRunStatus[]]); +const TaskKind = z.enum(["TASK", "AGENT", "SCHEDULED"]); const RunListInputOptionsSchema = z.object({ @@ - taskKinds: z.array(z.string()).optional(), + taskKinds: z.array(TaskKind).optional(), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/services/runsRepository/runsRepository.server.ts` around lines 45 - 46, The taskKinds schema currently allows any string (taskKinds: z.array(z.string()).optional()), which lets invalid values pass; change it to constrain to the known task kinds (e.g., replace z.string() with z.enum([...]) or z.nativeEnum(TaskKind) referencing your central TaskKind enum) so only valid task kind values are accepted; update imports to bring in the TaskKind enum or explicitly list allowed strings and adjust any callers/types if needed.apps/webapp/app/routes/resources.sessions.$sessionParam.close.ts-10-14 (1)
10-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReuse the shared close-session schema here.
This local schema has already drifted from the public contract: it accepts arbitrarily long
reasonstrings, whileCloseSessionRequestBodycaps them at 256 chars. Import the shared schema (or at least mirror its limit) so the dashboard and API reject the same payloads.As per coding guidelines:
apps/webapp/**/*.{ts,tsx}should use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepath.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/resources.sessions`.$sessionParam.close.ts around lines 10 - 14, The local closeSessionSchema has drifted from the shared CloseSessionRequestBody contract (it allows unlimited reason length); update closeSessionSchema to match the shared schema by either importing the shared CloseSessionRequestBody schema from the `@trigger.dev/core` subpath export and using it directly, or at minimum constrain reason to z.string().max(256). Ensure the import uses a subpath export from `@trigger.dev/core` (e.g. import { CloseSessionRequestBody } from '@trigger.dev/core/…') rather than importing from the package root, and replace or remove the local schema definition accordingly.apps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.ts-18-23 (1)
18-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject oversized cursors before coercing them to
Number.
afterEventIdcurrently accepts any digit string viaz.string().regex(/^\d+$/), but line 74 coerces it unsafely withNumber(searchParams.afterEventId). Long inputs can becomeInfinityor lose precision, causing the endpoint to skip/duplicate records or pass an invalid cursor downstream. TightenSearchSchemato validate finite safe integers before conversion.Suggested fix
const SearchSchema = z.object({ // S2 sequence number — same cursor format as the SSE Last-Event-ID // (the SSE `id:` field on session-channel events is the seq_num, // stringified). Records returned have `seqNum > afterEventId`. - afterEventId: z.string().regex(/^\d+$/).optional(), + afterEventId: z + .string() + .regex(/^\d+$/) + .refine((value) => Number.isSafeInteger(Number(value)), { + message: "afterEventId must be a safe integer", + }) + .optional(), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/realtime.v1.sessions`.$session.$io.records.ts around lines 18 - 23, SearchSchema currently allows any digit string for afterEventId but the code later does Number(searchParams.afterEventId) (see usage of Number in the handler), which can produce Infinity or lose precision for oversized values; update SearchSchema to reject values outside JavaScript safe integer range (or use a z.preprocess that coerces the string to a number and validates Number.isFinite() && Number.isSafeInteger() and >= 0) so only finite safe non-negative integers are accepted before conversion, then replace the unsafe Number(...) usage with the validated numeric value from the parsed schema (keep references to SearchSchema and afterEventId and the Number(...) conversion to locate the change).apps/webapp/app/components/runs/v3/TaskRunsTable.tsx-357-361 (1)
357-361:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the unsafe type cast for
taskKind.The
taskKindfield is typed asTaskKind, which allows arbitrary strings via.or(anyString)in the Zod schema, whileTaskTriggerSourceis limited to three specific enum values:STANDARD,SCHEDULED, andAGENT. The cast bypasses type safety and could pass invalid values toTaskTriggerSourceIcon. Either narrow theTaskKindschema to exclude arbitrary strings, or validate the value before casting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/components/runs/v3/TaskRunsTable.tsx` around lines 357 - 361, The code unsafely casts run.taskKind to TaskTriggerSource before passing it to TaskTriggerSourceIcon; instead validate or narrow the value: check that run.taskKind is one of the TaskTriggerSource enum values (e.g., compare against Object.values(TaskTriggerSource) or use a helper isTaskTriggerSource) and only pass it when valid, otherwise pass a safe fallback (or undefined) or render a default icon; alternatively tighten the TaskKind schema to only allow the TaskTriggerSource union so no cast is needed. Ensure references: TaskRunsTable.tsx, run.taskKind, TaskTriggerSource, TaskTriggerSourceIcon, and TaskKind.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.ts-61-69 (1)
61-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject zero and partially-numeric
Timeout-Secondsvalues.Line 63 uses
parseInt(), so values like"10foo"are accepted, and Line 66’s truthy guard lets"0"through even though this route documents a1..600range.Suggested fix
- const timeoutInSecondsRaw = request.headers.get("Timeout-Seconds") ?? undefined; - const timeoutInSeconds = timeoutInSecondsRaw ? parseInt(timeoutInSecondsRaw) : undefined; + const timeoutInSecondsRaw = request.headers.get("Timeout-Seconds") ?? undefined; + const timeoutInSeconds = + timeoutInSecondsRaw === undefined ? undefined : Number(timeoutInSecondsRaw); - if ( - timeoutInSeconds && - (isNaN(timeoutInSeconds) || timeoutInSeconds < 1 || timeoutInSeconds > 600) - ) { + if ( + timeoutInSeconds !== undefined && + (!Number.isInteger(timeoutInSeconds) || timeoutInSeconds < 1 || timeoutInSeconds > 600) + ) { return new Response("Invalid timeout", { status: 400 }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/resources.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.ts around lines 61 - 69, The timeout parsing currently uses parseInt on timeoutInSecondsRaw (from the "Timeout-Seconds" header) which accepts partially-numeric strings like "10foo" and treats "0" as falsy; change the validation in the timeoutInSeconds calculation (and the subsequent if) to first verify the header matches a pure integer regex (e.g. /^\d+$/) before parsing, then parse to a number (Number or parseInt) and explicitly reject 0 and values outside 1..600; update the checks around timeoutInSecondsRaw/timeoutInSeconds and the error return to ensure partially-numeric and "0" values return 400.apps/webapp/app/components/runs/v3/RunFilters.tsx-194-196 (1)
194-196:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
sourcesagainst the supported trigger-source set.Right now
sourcesaccepts any non-empty string, so?sources=foosurvives parsing, shows up as an applied filter, and gets passed downstream even though this UI only supportsSTANDARD,SCHEDULED, andAGENT.Suggested fix
+const RunSource = z.enum(["STANDARD", "SCHEDULED", "AGENT"]); + export const TaskRunListSearchFilters = z.object({ cursor: z.string().optional().describe("Cursor for pagination - used internally for navigation"), direction: z .enum(["forward", "backward"]) .optional() @@ - sources: StringOrStringArray.describe( + sources: z + .preprocess((value) => { + if (typeof value === "string") { + return value.length > 0 ? [value] : undefined; + } + + if (Array.isArray(value)) { + return value.filter((v) => typeof v === "string" && v.length > 0); + } + + return undefined; + }, RunSource.array().optional()) + .describe( "Task trigger sources to filter by (STANDARD, SCHEDULED, AGENT)" - ), + ), });As per coding guidelines,
{packages/core,apps/webapp}/**/*.{ts,tsx}: Use zod for validation in packages/core and apps/webapp.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/components/runs/v3/RunFilters.tsx` around lines 194 - 196, Replace the loose StringOrStringArray.describe for the "sources" field with a zod-based schema that only allows the supported trigger sources: "STANDARD", "SCHEDULED", "AGENT". Specifically, change the "sources" schema in RunFilters.tsx to use zod.enum (or z.union of z.string and z.array(z.enum(...)) if you need single-or-array behavior) instead of StringOrStringArray.describe, and ensure parsing/validation uses that zod schema so queries like ?sources=foo are rejected/normalized before becoming an applied filter or passed downstream.
🧹 Nitpick comments (13)
packages/core/src/v3/test/test-realtime-streams-manager.ts (1)
22-22: 💤 Low value
pipeWaitsis write-only dead state.
pipeWaitsis populated inpipe()(lines 83-84) and cleared inreset()(line 160), but nothing ever reads from it. Either remove it, or expose thewaitAll(key?)helper this state seems to anticipate (useful for tests that fan out multiplepipe()calls and want to await them collectively without holding each returned instance).♻️ Option A — drop the unused field
- private pipeWaits = new Map<string, Promise<void>[]>(); private writeListeners = new Set<WriteListener>();- if (!this.pipeWaits.has(key)) this.pipeWaits.set(key, []); - this.pipeWaits.get(key)!.push(done); - return {reset(): void { this.buffers.clear(); - this.pipeWaits.clear(); }Option B — keep it and add a
__waitAllFromTest(key?)helper that awaits the trackeddonepromises so tests can drain before assertions.Also applies to: 83-84, 160-160
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/test/test-realtime-streams-manager.ts` at line 22, The pipeWaits Map field is never read (populated in pipe() and cleared in reset()) so either remove pipeWaits entirely or implement a test-facing waiter so it’s useful; to fix, choose one: (A) delete the private pipeWaits property and any code that pushes into it inside pipe() and reset(), or (B) keep pipeWaits and add a method like __waitAllFromTest(key?: string) / waitAll(key?: string) that returns Promise<void> which awaits Promise.all(this.pipeWaits.get(key) || []) and clears the stored array, and update reset() to use that helper to drain entries; reference the pipeWaits field, the pipe() method where entries are added, and the reset() method where they’re cleared when making the change.packages/core/src/v3/test/test-input-stream-manager.ts (1)
117-121: ⚡ Quick winTest manager's
shiftBuffer/disconnectStreamdiverge from real manager semantics.
shiftBufferalways returnsfalseanddisconnectStreamis a complete no-op, but the realStandardInputStreamManagershifts/clears the buffered head used byonce(). SincependingSendsplays exactly that role in this test fixture (drained byonce()at lines 62–68), tests that exercise.wait()-style flows (drop-the-duplicate / disconnect-before-suspend) will not observe correct behavior here.Aligning the stubs would make tests faithfully reproduce production semantics.
♻️ Suggested alignment
- shiftBuffer(_streamId: string): boolean { - return false; - } - - disconnectStream(_streamId: string): void {} + shiftBuffer(streamId: string): boolean { + const buffered = this.pendingSends.get(streamId); + if (buffered && buffered.length > 0) { + buffered.shift(); + if (buffered.length === 0) this.pendingSends.delete(streamId); + return true; + } + return false; + } + + disconnectStream(streamId: string): void { + this.pendingSends.delete(streamId); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/test/test-input-stream-manager.ts` around lines 117 - 121, The test fixture's shiftBuffer and disconnectStream do not mimic production semantics: update shiftBuffer(streamId) to remove/shift the buffered head from the pendingSends structure for that stream and return true when it actually removed an entry (false otherwise), and update disconnectStream(streamId) to clear any buffered entries in pendingSends for that stream (and/or call shiftBuffer repeatedly) so that the once() consumer behavior that drains pendingSends is reproduced; reference the shiftBuffer and disconnectStream methods and the pendingSends collection and once() consumer when making the changes.packages/core/src/v3/sessionStreams/types.ts (1)
24-76: 💤 Low valuePrefer
typealias overinterfacefor the newSessionStreamManagerdeclaration.Per the repository's TypeScript guideline, types are preferred over interfaces. Since this is a brand-new declaration in a new file, it's a good opportunity to follow the guideline (the pre-existing
InputStreamManagerinterface can be migrated separately).♻️ Proposed change
-export interface SessionStreamManager { - /** Register a handler that fires every time data arrives on the given channel. */ - on( - sessionId: string, - io: SessionChannelIO, - handler: (data: unknown) => void | Promise<void> - ): { off: () => void }; - ... -} +export type SessionStreamManager = { + /** Register a handler that fires every time data arrives on the given channel. */ + on( + sessionId: string, + io: SessionChannelIO, + handler: (data: unknown) => void | Promise<void> + ): { off: () => void }; + ... +};As per coding guidelines: "Use types over interfaces for TypeScript".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/sessionStreams/types.ts` around lines 24 - 76, The declaration for SessionStreamManager should be converted from an interface to a type alias per repository TypeScript guidelines: replace the "interface SessionStreamManager { ... }" with "type SessionStreamManager = { ... }" preserving all member signatures (on, once, peek, lastSeqNum, setLastSeqNum, setMinTimestamp, shiftBuffer, disconnectStream, clearHandlers, reset, disconnect) and exported name; no API or runtime behavior should change, just the syntactic form (leave InputStreamManager untouched for separate migration).packages/core/src/v3/sessionStreams/manager.ts (1)
137-145: 💤 Low value
timeoutMs: 0silently disables the timeout.The truthy check on Line 137 treats
timeoutMs: 0as "no timeout", butTestSessionStreamManager.once(test-session-stream-manager.tsLine 102) usesoptions?.timeoutMs !== undefinedand would arm a 0ms timer that resolves immediately. The two implementations therefore disagree on0. If0is not a meaningful "fire immediately" value here, leave a comment; otherwise switch to an explicit!== undefinedcheck for parity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/sessionStreams/manager.ts` around lines 137 - 145, The current truthy check in the once waiter (where waiter.timeoutHandle is set) treats timeoutMs: 0 as unset and thus disables the timer; align behavior with TestSessionStreamManager.once by checking explicitly for undefined (use options.timeoutMs !== undefined) so a 0ms timeout is honored; update the condition around waiter.timeoutHandle and ensure the timeout callback still uses options.timeoutMs (and constructs the InputStreamTimeoutError(key, options.timeoutMs!)) and leaves `#removeOnceWaiter` and resolve logic unchanged.packages/core/src/v3/test/test-session-stream-manager.ts (2)
144-146: 💤 Low value
setLastSeqNumis unconditionally monotonic in production but not here.
StandardSessionStreamManager.setLastSeqNumonly advances the stored seqNum whenseqNum > current(packages/core/src/v3/sessionStreams/manager.tsLines 168-171). The test manager overwrites unconditionally, which can mask a class of regressions where production-side regression is the whole point of the test (e.g., resume-after-reconnect with out-of-order acks). Consider matching production semantics unless there's a specific reason to diverge.♻️ Suggested fix
setLastSeqNum(sessionId: string, io: SessionChannelIO, seqNum: number): void { - this.seqNums.set(keyFor(sessionId, io), seqNum); + const key = keyFor(sessionId, io); + const current = this.seqNums.get(key); + if (current === undefined || seqNum > current) { + this.seqNums.set(key, seqNum); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/test/test-session-stream-manager.ts` around lines 144 - 146, The test implementation of setLastSeqNum currently overwrites seqNums unconditionally; change it to match production behavior in StandardSessionStreamManager by only updating this.seqNums for keyFor(sessionId, io) when the incoming seqNum is greater than the existing value (i.e., retrieve current via this.seqNums.get(...), compare and only call this.seqNums.set(...) if seqNum > current), preserving monotonic advancement semantics used for resume-after-reconnect/out-of-order ack tests.
197-214: 💤 Low valueDocstring misstates production behavior.
The comment on Lines 207-213 says "Production discards records that only match handlers — but in production the SSE tail introduces enough latency that the next
.onceis usually registered before the next record arrives." Looking atStandardSessionStreamManager.#dispatch(packages/core/src/v3/sessionStreams/manager.tsLines 364-378), production also buffers the record when there's no waiter (handlers fire and buffer is appended). The test manager's behavior actually mirrors production here; the only deviation is that production buffers fromonPart, not from a test-driver entry point. Update the comment so future readers don't get a misleading mental model.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/test/test-session-stream-manager.ts` around lines 197 - 214, The docstring in test-session-stream-manager.ts incorrectly claims production discards records that only match handlers; update the comment to reflect that StandardSessionStreamManager.#dispatch also appends to the buffer when no waiter is present (handlers fire and the record is buffered), and clarify that the real difference is where buffering is triggered (production buffers from onPart while the test manager buffers from the test-driver entry point). Edit the block describing dispatch rules (the paragraph starting with "If no waiter, the record is buffered...") to state this corrected behavior and mention the actual deviation between production and tests (buffer origin), referencing the TestSessionStreamManager and StandardSessionStreamManager.#dispatch to guide readers.packages/core/src/v3/sessionStreams/manager.test.ts (2)
101-134: 💤 Low valueTest name vs. assertion is slightly off; consider tightening.
The test claims it verifies that a filter on
"in"doesn't bleed into"out", but the firstmanageronly ever asserts on"out"and never proves the"in"filter on that same manager (Line 113 receives{ kind: "in-record" }, which the singleShot mock delivers to whichever io subscribes first). The actual filter assertion uses a second manager (manager2) with separate state (Lines 120-127). The proof you want — same manager, same session, different io, only one is filtered — isn't actually exercised. Consider building one mock that routes records per io (e.g., emit different fixtures on different_iocalls) so a single manager can demonstrate the per-io isolation end-to-end.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/sessionStreams/manager.test.ts` around lines 101 - 134, The test currently proves filtering on "in" using a separate manager (manager2) instead of demonstrating per-(sessionId, io) isolation on the same StandardSessionStreamManager; update the test to use a single StandardSessionStreamManager and a single mock API that emits different fixture records depending on the requested _io so that the same manager subscribes to both "in" and "out" and you can call setMinTimestamp(sessionId, "in", 5000) then assert once(sessionId, "out") returns the expected record while once(sessionId, "in") times out; modify or replace singleShotApiClient to inspect the _io argument and return distinct records for "in" vs "out", then keep using manager.disconnectStream/manager.disconnect to clean up.
13-44: 💤 Low valueMock cast bypasses
ApiClienttype checking.
as unknown as ApiClient(Line 43) silently absorbs any breaking change to theApiClientshape, and the inner cast on Line 41 papers over the async-generator vsAsyncIterableStreammismatch. IfsubscribeToSessionStreamever gains required surface (or its return type changes), these tests will keep compiling while the manager breaks at runtime against the real client. Consider typing the partial mock asPick<ApiClient, "subscribeToSessionStream">and avoiding the outer double-cast — the manager only depends on that one method.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/sessionStreams/manager.test.ts` around lines 13 - 44, The mock currently double-casts to ApiClient (singleShotApiClient) which hides shape mismatches; change the mock's type to return Pick<ApiClient, "subscribeToSessionStream"> instead of ApiClient and remove the outer `as unknown as ApiClient` cast; ensure the subscribeToSessionStream method signature and its returned async-iterable value match ApiClient's actual return type (use ReturnType<ApiClient["subscribeToSessionStream"]> / Awaited<...> for the function return) so you only need to satisfy the single method the manager uses (subscribeToSessionStream) and avoid masking future API shape changes; keep SSEStreamPart usage and the delivered logic intact.packages/core/src/v3/realtimeStreams/streamsWriterV2.test.ts (1)
5-16: ⚡ Quick winAssert
lastEventIdon the success path.The new contract here is
StreamWriteResult, but this test only proves thatwait()resolves. Ifwait()regresses back to{}orundefined, this suite still passes. SeedlastAckedPosition()with a knownseqNumand assert the returned{ lastEventId }.Also applies to: 132-148
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/realtimeStreams/streamsWriterV2.test.ts` around lines 5 - 16, The test currently stubs lastAckedPosition and appendSession but never asserts the StreamWriteResult.lastEventId; update the mock for lastAckedPosition (e.g., have lastAckedPosition return a known seqNum/object) and add an assertion after calling wait() that the resolved value includes lastEventId equal to that known seqNum; apply the same change to the second occurrence referenced (the block around lines 132-148) so both success-path tests verify lastEventId from lastAckedPosition via the appendSession mock and wait() result.packages/core/src/v3/realtimeStreams/sessionStreamInstance.ts (2)
45-50: 💤 Low valueBest practice: Specify radix parameter for parseInt.
Lines 46 and 49 call
parseIntwithout the radix parameter. While the header values should be base-10 strings, explicitly passing10as the radix prevents potential issues and follows best practices.♻️ Add radix parameter
const flushIntervalMs = headers["x-s2-flush-interval-ms"] - ? parseInt(headers["x-s2-flush-interval-ms"]) + ? parseInt(headers["x-s2-flush-interval-ms"], 10) : undefined; const maxRetries = headers["x-s2-max-retries"] - ? parseInt(headers["x-s2-max-retries"]) + ? parseInt(headers["x-s2-max-retries"], 10) : undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/realtimeStreams/sessionStreamInstance.ts` around lines 45 - 50, The parseInt calls used to parse headers["x-s2-flush-interval-ms"] and headers["x-s2-max-retries"] (producing flushIntervalMs and maxRetries) should specify a radix to avoid ambiguous parsing; update those parseInt invocations to pass 10 as the second argument (e.g., parseInt(..., 10)) so the header strings are parsed explicitly as base-10 integers.
29-38: 💤 Low valueOptional: Remove redundant optional chaining.
Line 37 uses
this.options?.requestOptions, butoptionsis a required constructor parameter and cannot be undefined. The?.operator can be simplified to..♻️ Simplify optional chaining
const response = await this.options.apiClient.initializeSessionStream( this.options.sessionId, this.options.io, - this.options?.requestOptions + this.options.requestOptions );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/realtimeStreams/sessionStreamInstance.ts` around lines 29 - 38, The code uses redundant optional chaining for requestOptions in initializeWriter; since options is required on the SessionStreamInstance constructor (type SessionStreamInstanceOptions<T>), replace this.options?.requestOptions with this.options.requestOptions inside the initializeWriter method (the call to this.options.apiClient.initializeSessionStream) and remove the unnecessary ?. to simplify the expression.apps/webapp/app/presenters/v3/SessionPresenter.server.ts (1)
99-109: 💤 Low valueOptional: Consider extracting currentRun lookup for clarity.
The ternary and nullish coalescing on lines 99-108 are correct but require a non-null assertion on line 105. While this is safe (the outer ternary ensures
session.currentRunIdis truthy), extracting this logic into a helper or using an if-statement could improve readability.♻️ Alternative: Extract to helper or use if-statement
- const currentRun = session.currentRunId - ? runsById.get(session.currentRunId) ?? - (await startActiveSpan( - "SessionPresenter.findCurrentRunFallback", - () => - this.replica.taskRun.findFirst({ - where: { id: session.currentRunId! }, - select: { id: true, friendlyId: true, status: true }, - }) - )) - : null; + let currentRun = null; + if (session.currentRunId) { + currentRun = runsById.get(session.currentRunId); + if (!currentRun) { + currentRun = await startActiveSpan( + "SessionPresenter.findCurrentRunFallback", + () => + this.replica.taskRun.findFirst({ + where: { id: session.currentRunId }, + select: { id: true, friendlyId: true, status: true }, + }) + ); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/presenters/v3/SessionPresenter.server.ts` around lines 99 - 109, The currentRun lookup uses a nested ternary/nullish coalescing with a non-null assertion on session.currentRunId; extract this into a small helper or an if-statement to improve readability and remove the need for the assertion. Specifically, replace the inline expression that builds currentRun (which references session.currentRunId, runsById, startActiveSpan("SessionPresenter.findCurrentRunFallback", ...), and this.replica.taskRun.findFirst) with a clear helper function (e.g., findCurrentRun) or an explicit if-block that: 1) checks if session.currentRunId is set, 2) returns runsById.get(session.currentRunId) if present, and 3) otherwise calls startActiveSpan(...)/this.replica.taskRun.findFirst to fetch and return the run; update the currentRun assignment to call that helper or use the if-block so no non-null assertion is required.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1)
348-348: ⚡ Quick winThe
as anycast bypasses type safety.Casting
task.agentConfig as anyremoves compile-time guarantees that the config is JSON-serializable. While this may be necessary for Prisma'sJsontype, consider using a Zod schema to validate the config structure before persistence, or at minimum add a comment explaining why the cast is safe.🛡️ Safer alternative with Zod validation
As per coding guidelines, use Zod for validation in apps/webapp:
+import { z } from "zod"; + +const AgentConfigSchema = z.record(z.unknown()).optional(); + async function createWorkerTask( task: TaskResource, ... ) { ... + const validatedConfig = task.agentConfig + ? AgentConfigSchema.parse(task.agentConfig) + : undefined; + await prisma.backgroundWorkerTask.create({ data: { ... - config: task.agentConfig ? (task.agentConfig as any) : undefined, + config: validatedConfig as any, }, }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/v3/services/createBackgroundWorker.server.ts` at line 348, The cast "task.agentConfig as any" in the createBackgroundWorker flow removes type safety and should be replaced by runtime validation: define a Zod schema (e.g., AgentConfigSchema) that matches the expected JSON-serializable shape, call AgentConfigSchema.parse or safeParse on task.agentConfig inside the function that builds the Prisma payload (the spot assigning config), and use the validated value (or undefined) instead of the as any cast; if validation fails, handle the error (reject/create a clear log and abort persisting) or default to undefined. If a Zod schema cannot be introduced now, add a clear comment explaining why the cast is safe and what invariants guarantee JSON-serializability, referencing the config assignment (config: task.agentConfig ...) so future reviewers know why it's exempt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9a6d7e41-895b-4cd9-874d-48dd9f05eb37
📒 Files selected for processing (88)
.changeset/sessions-primitive.md.gitignoreCLAUDE.mdapps/webapp/app/components/BulkActionFilterSummary.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/components/runs/v3/TaskTriggerSource.tsxapps/webapp/app/components/sessions/v1/CloseSessionDialog.tsxapps/webapp/app/components/sessions/v1/SessionFilters.tsxapps/webapp/app/components/sessions/v1/SessionStatus.tsxapps/webapp/app/components/sessions/v1/SessionsTable.tsxapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/presenters/SessionFilters.server.tsapps/webapp/app/presenters/v3/ApiRunListPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.tsapps/webapp/app/presenters/v3/SessionListPresenter.server.tsapps/webapp/app/presenters/v3/SessionPresenter.server.tsapps/webapp/app/presenters/v3/TaskListPresenter.server.tsapps/webapp/app/presenters/v3/TestPresenter.server.tsapps/webapp/app/presenters/v3/TestTaskPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsxapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.tsapps/webapp/app/routes/resources.sessions.$sessionParam.close.tsapps/webapp/app/routes/runs.$runParam.tsapps/webapp/app/runEngine/concerns/queues.server.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/runEngine/types.tsapps/webapp/app/services/apiRateLimit.server.tsapps/webapp/app/services/realtime/mintRunToken.server.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tsapps/webapp/app/services/realtime/sessionRunManager.server.tsapps/webapp/app/services/runsReplicationService.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.tsapps/webapp/app/services/sessionsRepository/sessionsRepository.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/v3/services/createBackgroundWorker.server.tsinternal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sqlinternal-packages/clickhouse/src/taskRuns.test.tsinternal-packages/clickhouse/src/taskRuns.tsinternal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sqlinternal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sqlinternal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sqlinternal-packages/database/prisma/schema.prismapackages/core/src/v3/apiClient/errors.tspackages/core/src/v3/apiClient/runStream.test.tspackages/core/src/v3/apiClient/runStream.tspackages/core/src/v3/inputStreams/index.tspackages/core/src/v3/inputStreams/manager.tspackages/core/src/v3/inputStreams/noopManager.tspackages/core/src/v3/inputStreams/types.tspackages/core/src/v3/realtime-streams-api.tspackages/core/src/v3/realtimeStreams/index.tspackages/core/src/v3/realtimeStreams/manager.tspackages/core/src/v3/realtimeStreams/noopManager.tspackages/core/src/v3/realtimeStreams/sessionStreamInstance.tspackages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/realtimeStreams/streamsWriterV1.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.test.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.tspackages/core/src/v3/realtimeStreams/types.tspackages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/build.tspackages/core/src/v3/schemas/resources.tspackages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/schemas.tspackages/core/src/v3/semanticInternalAttributes.tspackages/core/src/v3/session-streams-api.tspackages/core/src/v3/sessionStreams/index.tspackages/core/src/v3/sessionStreams/manager.test.tspackages/core/src/v3/sessionStreams/manager.tspackages/core/src/v3/sessionStreams/noopManager.tspackages/core/src/v3/sessionStreams/types.tspackages/core/src/v3/test/test-input-stream-manager.tspackages/core/src/v3/test/test-realtime-streams-manager.tspackages/core/src/v3/test/test-run-metadata-manager.tspackages/core/src/v3/test/test-session-stream-manager.tspackages/core/src/v3/types/tasks.tspackages/core/src/v3/utils/globals.tspackages/core/src/v3/workers/index.tspackages/core/src/v3/workers/taskExecutor.tspackages/core/test/runStream.test.ts
| import { CheckCircleIcon, ClockIcon } from "@heroicons/react/20/solid"; | ||
| import assertNever from "assert-never"; | ||
| import { type SessionStatus } from "~/services/sessionsRepository/sessionsRepository.server"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the server-only import and locate a shared non-server SessionStatus definition.
rg -n 'sessionsRepository\.server' apps/webapp/app/components/sessions/v1/SessionStatus.tsx
rg -n '\b(SessionStatus|allSessionStatuses)\b' apps/webapp/app packages/core -g '!**/*.server.ts'Repository: triggerdotdev/trigger.dev
Length of output: 3838
Replace the .server module import with the type from @trigger.dev/core.
SessionStatus.tsx is a client-side component but imports SessionStatus from a .server module, which violates Remix's server/client boundary and will fail bundling. Import the type from packages/core/src/v3/schemas/api.ts instead, which provides the same SessionStatus type definition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/webapp/app/components/sessions/v1/SessionStatus.tsx` around lines 1 - 3,
SessionStatus.tsx currently imports the SessionStatus type from a ".server"
module which breaks the client/server boundary; change the import to pull the
type from the core package (import type { SessionStatus } from
"@trigger.dev/core") so the component remains client-safe, and use a type-only
import to avoid bundling server code while keeping existing symbols like
SessionStatus, CheckCircleIcon, and ClockIcon unchanged.
| ? this.replica.taskRun.findMany({ | ||
| where: { id: { in: currentRunIds } }, | ||
| select: { id: true, friendlyId: true }, | ||
| }) |
There was a problem hiding this comment.
Scope the current-run lookup to the same project/environment.
Session.currentRunId is a plain string column with no FK, so resolving runs by id alone can surface another tenant’s run if that pointer is ever stale or corrupted. This follow-up lookup should be constrained to the same projectId and environmentId.
Suggested fix
return currentRunIds.length > 0
? this.replica.taskRun.findMany({
- where: { id: { in: currentRunIds } },
+ where: {
+ id: { in: currentRunIds },
+ projectId,
+ runtimeEnvironmentId: environmentId,
+ },
select: { id: true, friendlyId: true },
})
: [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/webapp/app/presenters/v3/SessionListPresenter.server.ts` around lines
163 - 166, The current lookup using this.replica.taskRun.findMany({ where: { id:
{ in: currentRunIds } }, ... }) is unsafe because Session.currentRunId has no FK
and can return runs from other projects/environments; update the where clause to
also constrain by projectId and environmentId (e.g. include projectId: { in:
currentProjectIds } and environmentId: { in: currentEnvironmentIds } or match
the session's projectId/environmentId context) so the findMany call only returns
taskRun rows belonging to the same project/environment as the sessions; adjust
how currentRunIds are grouped/mapped if needed to ensure correct scoping for
this.replica.taskRun.findMany and references to Session.currentRunId.
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | ||
| if (!environment) { | ||
| throw new Error("Environment not found"); |
There was a problem hiding this comment.
Return a user-facing 404 here instead of throwing a generic error.
Line 42 turns a bad or unauthorized environment slug into an unhandled route error, while the adjacent project-not-found path already uses user-facing handling.
Suggested fix
const environment = await findEnvironmentBySlug(project.id, envParam, userId);
if (!environment) {
- throw new Error("Environment not found");
+ throw new Response("Environment not found", { status: 404 });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | |
| if (!environment) { | |
| throw new Error("Environment not found"); | |
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | |
| if (!environment) { | |
| throw new Response("Environment not found", { status: 404 }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
around lines 40 - 42, Replace the generic throw in the environment lookup with a
user-facing 404 response: when findEnvironmentBySlug(project.id, envParam,
userId) returns falsy, throw or return a Response with status 404 (e.g., throw
new Response("Environment not found", { status: 404 })) so the route renders a
proper Not Found page instead of bubbling a generic Error; update the branch
that currently throws new Error("Environment not found") to use this 404
Response (referencing findEnvironmentBySlug and the environment check in the
route handler).
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | ||
| if (!environment) { | ||
| throw new Error("Environment not found"); | ||
| } |
There was a problem hiding this comment.
Handle a missing environment as a 404, not a generic error.
Line 74 makes an invalid or unauthorized environment slug fall into the route error boundary/500 path instead of returning a user-facing not-found response.
Suggested fix
const environment = await findEnvironmentBySlug(project.id, envParam, userId);
if (!environment) {
- throw new Error("Environment not found");
+ throw new Response("Environment not found", { status: 404 });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | |
| if (!environment) { | |
| throw new Error("Environment not found"); | |
| } | |
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | |
| if (!environment) { | |
| throw new Response("Environment not found", { status: 404 }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsx
around lines 72 - 75, The code currently throws a generic Error when
findEnvironmentBySlug returns null; change this to return a 404 response so
missing/unauthorized env slugs hit the not-found UI instead of the error
boundary. Replace the throw new Error("Environment not found") in the block
after await findEnvironmentBySlug(project.id, envParam, userId) with a 404
response (e.g., throw new Response("Not Found", { status: 404 }) or use your
framework's notFound() helper) so the route returns a proper 404 for a missing
environment.
| select: { | ||
| deployment: { | ||
| select: { | ||
| friendlyId: true, | ||
| createdAt: true, | ||
| shortCode: true, | ||
| version: true, | ||
| runtime: true, | ||
| runtimeVersion: true, | ||
| status: true, | ||
| deployedAt: true, | ||
| git: true, | ||
| errorData: true, | ||
| }, |
There was a problem hiding this comment.
Return updatedAt to match the shared response schema.
RetrieveCurrentDeploymentResponseBody now reuses ApiDeploymentListResponseItem, which requires updatedAt, but this loader neither selects nor returns it. Clients validating against the shared schema will fail on this endpoint until the field is included.
Suggested fix
select: {
deployment: {
select: {
friendlyId: true,
createdAt: true,
+ updatedAt: true,
shortCode: true,
version: true,
runtime: true,
runtimeVersion: true,
status: true,
@@
return json({
id: deployment.friendlyId,
createdAt: deployment.createdAt,
+ updatedAt: deployment.updatedAt,
shortCode: deployment.shortCode,
version: deployment.version,
runtime: deployment.runtime,Also applies to: 42-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/webapp/app/routes/api.v1.deployments.current.ts` around lines 20 - 33,
The loader is missing the updatedAt field required by
ApiDeploymentListResponseItem/ RetrieveCurrentDeploymentResponseBody; update the
deployment select in this route so the nested deployment.select includes
updatedAt (add updatedAt: true wherever deployment is being selected in this
file) so the returned object matches the shared response schema.
| vi.mock("@s2-dev/streamstore", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@s2-dev/streamstore")>(); | ||
| return { | ||
| ...actual, | ||
| S2: class FakeS2 { | ||
| basin() { | ||
| return { | ||
| stream: () => ({ | ||
| appendSession, | ||
| }), | ||
| }; | ||
| } | ||
| }, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Avoid vi.mock() here; test this path without module mocking.
This mock sidesteps the exact appendSession() / lastAckedPosition() behavior that StreamsWriterV2.wait() now depends on, and it also conflicts with the repo’s test rules. A small extraction of the JSON-envelope/size-check logic into a pure helper would let you cover the oversize branch without mocking @s2-dev/streamstore.
As per coding guidelines, "Use vitest exclusively for testing. Never mock anything - use testcontainers instead."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/v3/realtimeStreams/streamsWriterV2.test.ts` around lines 18
- 32, Remove the vi.mock block and test this path without mocking; instead
extract the JSON-envelope/size-check logic from StreamsWriterV2.wait() into a
pure helper (e.g., computeEnvelopeSize or isEnvelopeOversize) and update
StreamsWriterV2.wait() to call that helper so you can unit-test the oversize
branch directly in streamsWriterV2.test.ts. For tests that require real
appendSession/lastAckedPosition behavior, replace the mocked module usage with
an integration test using testcontainers to start the real `@s2-dev/streamstore`
service and assert StreamsWriterV2.wait() behavior against the real
appendSession/lastAckedPosition semantics. Ensure references to appendSession,
lastAckedPosition, and StreamsWriterV2.wait are updated to call the new helper
and to use testcontainers-based integration tests instead of vi.mock.
| /** Per-run wall-clock cap (seconds). Forwarded to `TaskRunOptions.maxDuration`. */ | ||
| maxDuration: z.number().int().positive().optional(), | ||
| /** Pin every run to a specific worker version. Forwarded to `TaskRunOptions.lockToVersion`. */ | ||
| lockToVersion: z.string().optional(), | ||
| /** Region to schedule runs in. Forwarded to `TaskRunOptions.region`. */ | ||
| region: z.string().optional(), |
There was a problem hiding this comment.
Match session maxDuration to the existing 5-second floor.
TriggerOptions.maxDuration is documented in this file as requiring at least 5 seconds, but SessionTriggerConfig.maxDuration currently accepts 1..4. That lets callers persist a session config that only fails later when the session tries to trigger a run.
Suggested fix
- maxDuration: z.number().int().positive().optional(),
+ maxDuration: z.number().int().min(5).optional(),As per coding guidelines: {packages/core,apps/webapp}/**/*.{ts,tsx} should use Zod for validation in packages/core and apps/webapp.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Per-run wall-clock cap (seconds). Forwarded to `TaskRunOptions.maxDuration`. */ | |
| maxDuration: z.number().int().positive().optional(), | |
| /** Pin every run to a specific worker version. Forwarded to `TaskRunOptions.lockToVersion`. */ | |
| lockToVersion: z.string().optional(), | |
| /** Region to schedule runs in. Forwarded to `TaskRunOptions.region`. */ | |
| region: z.string().optional(), | |
| /** Per-run wall-clock cap (seconds). Forwarded to `TaskRunOptions.maxDuration`. */ | |
| maxDuration: z.number().int().min(5).optional(), | |
| /** Pin every run to a specific worker version. Forwarded to `TaskRunOptions.lockToVersion`. */ | |
| lockToVersion: z.string().optional(), | |
| /** Region to schedule runs in. Forwarded to `TaskRunOptions.region`. */ | |
| region: z.string().optional(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/v3/schemas/api.ts` around lines 1498 - 1503,
SessionTriggerConfig.maxDuration allows values 1–4 but must match
TriggerOptions.maxDuration's 5-second minimum; update the Zod schema for
SessionTriggerConfig.maxDuration in packages/core/src/v3/schemas/api.ts to
enforce an integer minimum of 5 seconds (e.g., use .int().min(5) or
.int().gte(5) before .optional()), ensuring the same validation as
TriggerOptions.maxDuration so invalid configs are rejected at persist time.
| async #runTail( | ||
| sessionId: string, | ||
| io: SessionChannelIO, | ||
| signal: AbortSignal | ||
| ): Promise<void> { | ||
| const key = keyFor(sessionId, io); | ||
| try { | ||
| const lastSeq = this.seqNums.get(key); | ||
| // Dispatch is driven from `onPart` (not the for-await loop) so each | ||
| // record reaches dispatch with its full SSE metadata in scope — | ||
| // specifically the timestamp, which we need for the per-stream | ||
| // min-timestamp filter. The for-await loop below just drains the | ||
| // pipeThrough output to keep the source flowing. | ||
| const stream = await this.apiClient.subscribeToSessionStream<unknown>(sessionId, io, { | ||
| signal, | ||
| baseUrl: this.baseUrl, | ||
| timeoutInSeconds: 600, | ||
| lastEventId: lastSeq !== undefined ? String(lastSeq) : undefined, | ||
| onPart: (part) => { | ||
| if (signal.aborted) return; | ||
| const seqNum = parseInt(part.id, 10); | ||
| if (Number.isFinite(seqNum)) { | ||
| this.seqNums.set(key, seqNum); | ||
| } | ||
|
|
||
| // Min-timestamp filter: drop records older than (or at) the | ||
| // bound. Used to skip already-processed records on OOM-retry | ||
| // boot. | ||
| const minTs = this.minTimestamps.get(key); | ||
| if (minTs !== undefined && part.timestamp <= minTs) { | ||
| return; | ||
| } | ||
|
|
||
| let data: unknown = part.chunk; | ||
| if (typeof data === "string") { | ||
| try { | ||
| data = JSON.parse(data); | ||
| } catch { | ||
| // keep as string | ||
| } | ||
| } | ||
| this.#dispatch(key, data); | ||
| }, | ||
| onComplete: () => { | ||
| if (this.debug) { | ||
| console.log(`[SessionStreamManager] Tail completed for "${key}"`); | ||
| } | ||
| }, | ||
| onError: (error) => { | ||
| if (this.debug) { | ||
| console.error(`[SessionStreamManager] Tail error for "${key}":`, error); | ||
| } | ||
| }, | ||
| }); | ||
|
|
||
| // Drain to keep the pipeThrough flowing. Records were already | ||
| // dispatched in `onPart`, so the body here is a no-op. | ||
| for await (const _record of stream) { | ||
| if (signal.aborted) break; | ||
| } | ||
| } catch (error) { | ||
| if (error instanceof Error && error.name === "AbortError") return; | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare reconnect/backoff behavior between InputStreamManager and SessionStreamManager
fd -t f 'manager.ts' packages/core/src/v3/inputStreams packages/core/src/v3/sessionStreams --exec cat {}Repository: triggerdotdev/trigger.dev
Length of output: 27244
🏁 Script executed:
# Check if there's any backoff/retry delay mechanism in either manager or apiClient
rg -t ts "backoff|delay|setTimeout.*reconnect|minDelay|exponential" packages/core/src/v3/Repository: triggerdotdev/trigger.dev
Length of output: 16648
🏁 Script executed:
# Find subscribeToSessionStream definition
rg -t ts "subscribeToSessionStream" packages/core/src/v3 -A 5 | head -50Repository: triggerdotdev/trigger.dev
Length of output: 2242
🏁 Script executed:
# Find the actual subscribeToSessionStream definition in apiClient
fd -t f -name "*.ts" packages/core/src/v3/apiClient --exec grep -l "subscribeToSessionStream" {}Repository: triggerdotdev/trigger.dev
Length of output: 300
🏁 Script executed:
# Find the subscribeToSessionStream method definition
rg -t ts "subscribeToSessionStream\s*\(" packages/core/src/v3/apiClient -A 30Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Search for ApiClient class and subscribeToSessionStream method
rg -t ts "class ApiClient|subscribeToSessionStream" packages/core/src/v3 -B 2 -A 10Repository: triggerdotdev/trigger.dev
Length of output: 6608
🏁 Script executed:
# Get the full ApiClient class to find subscribeToSessionStream method
rg -t ts "subscribeToSessionStream" packages/core/src/v3/apiClient/index.ts -A 25 -B 2Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Search all files for subscribeToSessionStream definition
rg -t ts "subscribeToSessionStream.*\{" packages/core/src/v3 -A 30Repository: triggerdotdev/trigger.dev
Length of output: 2591
🏁 Script executed:
# Search for the method signature in ApiClient
rg -t ts "async subscribeToSessionStream|subscribeToSessionStream\s*<" packages/core/src/v3/apiClient/ -A 20Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Search for subscribeToSessionStream in type definitions
rg -t ts "subscribeToSessionStream" packages/core/src/v3 --type-list | head -20Repository: triggerdotdev/trigger.dev
Length of output: 459
🏁 Script executed:
# Look at the ApiClient index file structure
head -200 packages/core/src/v3/apiClient/index.ts | tail -100Repository: triggerdotdev/trigger.dev
Length of output: 2479
🏁 Script executed:
# Find where subscribeToSessionStream is defined in apiClient
grep -n "subscribeToSessionStream" packages/core/src/v3/apiClient/index.tsRepository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# List all files in apiClient directory
ls -la packages/core/src/v3/apiClient/Repository: triggerdotdev/trigger.dev
Length of output: 887
🏁 Script executed:
# Search for subscribeToSessionStream method in index.ts
rg "subscribeToSessionStream" packages/core/src/v3/apiClient/index.tsRepository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Search all of packages/core for the definition of subscribeToSessionStream
rg "subscribeToSessionStream.*=" packages/core/src/v3 -A 5Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Search for subscribeToSessionStream definition with more flexibility
rg "subscribeToSessionStream" packages/core/src/v3 -B 2 -A 15 | head -60Repository: triggerdotdev/trigger.dev
Length of output: 5405
🏁 Script executed:
# Search entire codebase for the method implementation
rg "subscribeToSessionStream\s*\(" packages/core/src -A 20 | grep -v "test.ts" | grep -v "await this.apiClient"Repository: triggerdotdev/trigger.dev
Length of output: 51
Add backoff and error handling to prevent reconnect loops on persistent failures.
In #runTail (lines 335-339), onError only logs (when debug is on) and never aborts or rethrows. If subscribeToSessionStream fails with a non-abort error, the for-await loop exits normally, #runTail returns, and the .finally reconnect path at lines 251-260 immediately re-attaches whenever hasHandlers || hasWaiters. With a persistent failure (auth rejected, 5xx, DNS, etc.) and an on() handler installed, this creates an unbounded tight reconnect loop with no backoff.
The same issue exists in StandardInputStreamManager—both managers lack exponential backoff on reconnect failures. Consider adding minimum backoff before reconnect, capping retry attempts, or surfacing terminal errors through persistent handlers / pending once() waiters so callers can react.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/v3/sessionStreams/manager.ts` around lines 287 - 351, The
tail loop in `#runTail` uses subscribeToSessionStream with an onError that only
logs, causing immediate reconnection loops when persistent errors occur; update
onError to surface terminal failures by aborting the provided AbortSignal or
throwing a distinct error so the outer caller can detect a failure, and
implement exponential backoff with a capped delay and retry limit in the
reconnect logic that currently decides to re-attach when hasHandlers ||
hasWaiters (the same pattern should be applied to StandardInputStreamManager).
Concretely: modify the onError handler passed into subscribeToSessionStream to
call signal.abort() or forward a non-Abort terminal Error to cause `#runTail` to
exit with an error, then in the surrounding reconnect/finally path add a backoff
loop (backoff base, multiplier, maxDelay, and maxAttempts) before re-invoking
`#runTail`; ensure seqNums/minTimestamps/#dispatch behavior is unchanged and that
aborts still return cleanly for AbortError.
| /** | ||
| * Trigger a task and subscribe to its updates via realtime. Unlike `triggerAndWait`, | ||
| * this does NOT suspend the parent run — the parent stays alive and polls for updates. | ||
| * This enables parallel tool calls and proper abort signal handling. | ||
| * | ||
| * @param payload | ||
| * @param options - Options for the task run, including an optional `signal` to cancel the subscription and child run | ||
| * @returns TaskRunPromise | ||
| * @example | ||
| * ``` | ||
| * const result = await task.triggerAndSubscribe({ foo: "bar" }, { signal: abortSignal }); | ||
| * | ||
| * if (result.ok) { | ||
| * console.log(result.output); | ||
| * } else { | ||
| * console.error(result.error); | ||
| * } | ||
| * ``` | ||
| */ | ||
| triggerAndSubscribe: ( | ||
| payload: TInput, | ||
| options?: TriggerAndSubscribeOptions, | ||
| ) => TaskRunPromise<TIdentifier, TOutput>; |
There was a problem hiding this comment.
Add a changeset for this SDK surface change.
triggerAndSubscribe()/TriggerAndSubscribeOptions extend the public packages/core API, so this PR needs a changeset before merge or consumers won't get a versioned release note for the new surface.
Based on learnings: When modifying any public package (packages/* or integrations/*), add a changeset using pnpm run changeset:add.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/v3/types/tasks.ts` around lines 650 - 672, This PR adds a
new public API surface (triggerAndSubscribe and TriggerAndSubscribeOptions) in
packages/core, so add a changeset describing this public API addition: run `pnpm
run changeset:add`, select the "packages/core" package, choose a version bump
(likely patch), and write a short summary noting the new
Task.triggerAndSubscribe and TriggerAndSubscribeOptions export so consumers
receive a release note; commit the generated changeset file with the PR.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
b84d537 to
ed7bf97
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/v3/schemas/runEngine.ts`:
- Around line 18-19: The TaskKind schema currently uses .or(anyString) which
widens the canonical TaskKind to any string and destroys the literal type;
revert TaskKind to just the z.enum(["STANDARD","SCHEDULED","AGENT"]) definition
so TaskKind infers the union of those three literals, and remove the
.or(anyString) usage; if you need a permissive parser keep a separate exported
schema (e.g., TaskKindLoose = TaskKind.or(z.string())) for forward-compat
parsing while leaving TaskKind strict; update references that relied on the
previous loose schema to use TaskKindLoose where appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 13f751b0-d010-4fef-b3c2-a1cb754be003
📒 Files selected for processing (88)
.changeset/sessions-primitive.md.gitignoreCLAUDE.mdapps/webapp/app/components/BulkActionFilterSummary.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/components/runs/v3/TaskTriggerSource.tsxapps/webapp/app/components/sessions/v1/CloseSessionDialog.tsxapps/webapp/app/components/sessions/v1/SessionFilters.tsxapps/webapp/app/components/sessions/v1/SessionStatus.tsxapps/webapp/app/components/sessions/v1/SessionsTable.tsxapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/presenters/SessionFilters.server.tsapps/webapp/app/presenters/v3/ApiRunListPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.tsapps/webapp/app/presenters/v3/SessionListPresenter.server.tsapps/webapp/app/presenters/v3/SessionPresenter.server.tsapps/webapp/app/presenters/v3/TaskListPresenter.server.tsapps/webapp/app/presenters/v3/TestPresenter.server.tsapps/webapp/app/presenters/v3/TestTaskPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsxapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.tsapps/webapp/app/routes/resources.sessions.$sessionParam.close.tsapps/webapp/app/routes/runs.$runParam.tsapps/webapp/app/runEngine/concerns/queues.server.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/runEngine/types.tsapps/webapp/app/services/apiRateLimit.server.tsapps/webapp/app/services/realtime/mintRunToken.server.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tsapps/webapp/app/services/realtime/sessionRunManager.server.tsapps/webapp/app/services/runsReplicationService.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.tsapps/webapp/app/services/sessionsRepository/sessionsRepository.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/v3/services/createBackgroundWorker.server.tsinternal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sqlinternal-packages/clickhouse/src/taskRuns.test.tsinternal-packages/clickhouse/src/taskRuns.tsinternal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sqlinternal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sqlinternal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sqlinternal-packages/database/prisma/schema.prismapackages/core/src/v3/apiClient/errors.tspackages/core/src/v3/apiClient/runStream.test.tspackages/core/src/v3/apiClient/runStream.tspackages/core/src/v3/inputStreams/index.tspackages/core/src/v3/inputStreams/manager.tspackages/core/src/v3/inputStreams/noopManager.tspackages/core/src/v3/inputStreams/types.tspackages/core/src/v3/realtime-streams-api.tspackages/core/src/v3/realtimeStreams/index.tspackages/core/src/v3/realtimeStreams/manager.tspackages/core/src/v3/realtimeStreams/noopManager.tspackages/core/src/v3/realtimeStreams/sessionStreamInstance.tspackages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/realtimeStreams/streamsWriterV1.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.test.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.tspackages/core/src/v3/realtimeStreams/types.tspackages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/build.tspackages/core/src/v3/schemas/resources.tspackages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/schemas.tspackages/core/src/v3/semanticInternalAttributes.tspackages/core/src/v3/session-streams-api.tspackages/core/src/v3/sessionStreams/index.tspackages/core/src/v3/sessionStreams/manager.test.tspackages/core/src/v3/sessionStreams/manager.tspackages/core/src/v3/sessionStreams/noopManager.tspackages/core/src/v3/sessionStreams/types.tspackages/core/src/v3/test/test-input-stream-manager.tspackages/core/src/v3/test/test-realtime-streams-manager.tspackages/core/src/v3/test/test-run-metadata-manager.tspackages/core/src/v3/test/test-session-stream-manager.tspackages/core/src/v3/types/tasks.tspackages/core/src/v3/utils/globals.tspackages/core/src/v3/workers/index.tspackages/core/src/v3/workers/taskExecutor.tspackages/core/test/runStream.test.ts
✅ Files skipped from review due to trivial changes (10)
- packages/core/src/v3/semanticInternalAttributes.ts
- packages/core/src/v3/workers/index.ts
- CLAUDE.md
- .gitignore
- internal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sql
- internal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sql
- .changeset/sessions-primitive.md
- internal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sql
- packages/core/src/v3/realtimeStreams/index.ts
- internal-packages/clickhouse/src/taskRuns.test.ts
🚧 Files skipped from review as they are similar to previous changes (76)
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
- apps/webapp/app/components/BulkActionFilterSummary.tsx
- apps/webapp/app/presenters/v3/TaskListPresenter.server.ts
- internal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sql
- apps/webapp/app/presenters/SessionFilters.server.ts
- apps/webapp/app/services/runsReplicationService.server.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV1.ts
- packages/core/src/v3/realtimeStreams/noopManager.ts
- packages/core/src/v3/realtimeStreams/streamInstance.ts
- packages/core/src/v3/realtime-streams-api.ts
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts
- packages/core/src/v3/schemas/resources.ts
- packages/core/src/v3/inputStreams/noopManager.ts
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
- apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
- apps/webapp/app/presenters/RunFilters.server.ts
- packages/core/src/v3/inputStreams/manager.ts
- packages/core/src/v3/schemas/schemas.ts
- packages/core/src/v3/workers/taskExecutor.ts
- packages/core/src/v3/realtimeStreams/manager.ts
- packages/core/src/v3/inputStreams/types.ts
- packages/core/src/v3/apiClient/errors.ts
- packages/core/src/v3/utils/globals.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV2.test.ts
- apps/webapp/app/services/realtime/mintRunToken.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsx
- apps/webapp/app/routes/runs.$runParam.ts
- apps/webapp/app/presenters/v3/SessionPresenter.server.ts
- packages/core/src/v3/schemas/api.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
- apps/webapp/app/components/sessions/v1/CloseSessionDialog.tsx
- apps/webapp/app/routes/api.v1.deployments.current.ts
- packages/core/src/v3/sessionStreams/types.ts
- apps/webapp/app/components/sessions/v1/SessionStatus.tsx
- apps/webapp/app/services/runsRepository/runsRepository.server.ts
- apps/webapp/app/presenters/v3/TestPresenter.server.ts
- packages/core/src/v3/sessionStreams/manager.test.ts
- packages/core/test/runStream.test.ts
- packages/core/src/v3/types/tasks.ts
- apps/webapp/app/routes/resources.sessions.$sessionParam.close.ts
- apps/webapp/app/components/runs/v3/TaskTriggerSource.tsx
- apps/webapp/app/services/realtime/s2realtimeStreams.server.ts
- apps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.ts
- apps/webapp/app/utils/pathBuilder.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsx
- packages/core/src/v3/session-streams-api.ts
- apps/webapp/app/presenters/v3/SessionListPresenter.server.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV2.ts
- apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
- packages/core/src/v3/test/test-realtime-streams-manager.ts
- packages/core/src/v3/sessionStreams/index.ts
- apps/webapp/app/services/apiRateLimit.server.ts
- packages/core/src/v3/inputStreams/index.ts
- apps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.ts
- packages/core/src/v3/sessionStreams/noopManager.ts
- internal-packages/clickhouse/src/taskRuns.ts
- internal-packages/database/prisma/schema.prisma
- apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts
- apps/webapp/app/services/realtime/sessionRunManager.server.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.ts
- apps/webapp/app/runEngine/services/triggerTask.server.ts
- apps/webapp/app/services/sessionsRepository/sessionsRepository.server.ts
- apps/webapp/app/runEngine/types.ts
- packages/core/src/v3/test/test-input-stream-manager.ts
- packages/core/src/v3/test/test-run-metadata-manager.ts
- packages/core/src/v3/realtimeStreams/sessionStreamInstance.ts
- apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
- packages/core/src/v3/sessionStreams/manager.ts
- packages/core/src/v3/apiClient/runStream.test.ts
- apps/webapp/app/components/sessions/v1/SessionFilters.tsx
- apps/webapp/app/components/runs/v3/RunFilters.tsx
- apps/webapp/app/components/sessions/v1/SessionsTable.tsx
- packages/core/src/v3/apiClient/runStream.ts
- apps/webapp/app/runEngine/concerns/queues.server.ts
- packages/core/src/v3/realtimeStreams/types.ts
- packages/core/src/v3/test/test-session-stream-manager.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadImport from
@trigger.dev/coresubpaths only, never from the root of the package
Files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Use
pnpm run typecheck --filter <package>for verification in apps and internal packages, notbuild. Building proves almost nothing about correctness.
Files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queriesAlways import from
@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecatedclient.defineJobAdd
//@Crumbsmarkers or wrap blocks in `// `#region` `@crumbsfor debug tracing as you write code, not just when debugging. These are stripped before merge.
Files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-11T19:01:52.274Z
Learning: This is a pnpm 10.33.2 monorepo using Turborepo. Run commands from root with `pnpm run`.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-11T19:01:52.274Z
Learning: Use vitest exclusively for testing. Run tests using `pnpm run test --filter <package>` or `pnpm run test ./path/to/test.ts --run` for single files.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-11T19:01:52.274Z
Learning: Request flow architecture: User API call -> Webapp routes -> Services -> RunEngine -> Redis Queue -> Supervisor -> Container execution -> Results back through RunEngine -> ClickHouse (analytics) + PostgreSQL (state)
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-11T19:01:52.274Z
Learning: The `apps/webapp/app/v3/` directory contains mostly V2 code that is actively used. Only specific files are V1-only legacy. When encountering V1/V2 branching in services, only modify V2 code paths. All new work uses Run Engine 2.0 and redis-worker.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-11T19:01:52.274Z
Learning: Do NOT update `rules/` directory or `.claude/skills/trigger-dev-tasks/` unless explicitly asked - these are maintained in separate dedicated passes.
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
packages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/build.ts
🔇 Additional comments (3)
packages/core/src/v3/schemas/build.ts (2)
3-9: Import update looks correct and properly scoped.
SkillManifestis imported from the existing local schema module and is used by the new manifest fields.
79-80:skillsschema additions are consistent and backward-compatible.Adding
skillsas optional on bothBuildManifestandWorkerManifestkeeps old manifests valid while enabling the new capability.Also applies to: 98-98
packages/core/src/v3/schemas/runEngine.ts (1)
26-26: No additional concern here beyond theTaskKindschema issue above.
| export const TaskKind = z.enum(["STANDARD", "SCHEDULED", "AGENT"]).or(anyString); | ||
| export type TaskKind = z.infer<typeof TaskKind>; |
There was a problem hiding this comment.
TaskKind validation is currently non-restrictive and loses type safety.
TaskKind.or(anyString) makes the schema accept any string, so TaskKind infers to string and no longer enforces "STANDARD" | "SCHEDULED" | "AGENT" at boundaries.
Suggested fix
-export const TaskKind = z.enum(["STANDARD", "SCHEDULED", "AGENT"]).or(anyString);
+export const TaskKind = z.enum(["STANDARD", "SCHEDULED", "AGENT"]);
export type TaskKind = z.infer<typeof TaskKind>;If you still need forward-compat parsing in select readers, keep a separate loose schema instead of broadening the canonical one:
export const TaskKindLoose = TaskKind.or(z.string());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const TaskKind = z.enum(["STANDARD", "SCHEDULED", "AGENT"]).or(anyString); | |
| export type TaskKind = z.infer<typeof TaskKind>; | |
| export const TaskKind = z.enum(["STANDARD", "SCHEDULED", "AGENT"]); | |
| export type TaskKind = z.infer<typeof TaskKind>; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/v3/schemas/runEngine.ts` around lines 18 - 19, The TaskKind
schema currently uses .or(anyString) which widens the canonical TaskKind to any
string and destroys the literal type; revert TaskKind to just the
z.enum(["STANDARD","SCHEDULED","AGENT"]) definition so TaskKind infers the union
of those three literals, and remove the .or(anyString) usage; if you need a
permissive parser keep a separate exported schema (e.g., TaskKindLoose =
TaskKind.or(z.string())) for forward-compat parsing while leaving TaskKind
strict; update references that relied on the previous loose schema to use
TaskKindLoose where appropriate.
ed7bf97 to
365e73b
Compare
ericallam
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/v3/apiClient/index.ts (1)
1291-1291: 💤 Low valueMinor: Explicitly passing undefined is redundant.
The second parameter can be omitted since
mergeRequestOptionshandlesundefinedby returningdefaultOptions.♻️ Simplify by omitting the undefined parameter
- mergeRequestOptions(this.defaultRequestOptions, undefined) + this.defaultRequestOptions🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/v3/apiClient/index.ts` at line 1291, Call site passes an explicit undefined to mergeRequestOptions which is redundant; update the invocation that currently uses mergeRequestOptions(this.defaultRequestOptions, undefined) to call mergeRequestOptions(this.defaultRequestOptions) instead so mergeRequestOptions can rely on its own undefined handling (refer to the mergeRequestOptions function and defaultRequestOptions field to locate and verify the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/v3/errors.ts`:
- Around line 634-648: The error message in ChatChunkTooLargeError (constructor,
chunkSize/maxSize/chunkType) embeds a dead docs link; update the constructor's
message to either point to a valid documentation URL (or the docs root) or
remove the link and expand the inline guidance so users get actionable steps
(e.g., "write oversized payloads to your own store and emit only an id/url
through the chat stream") until the referenced page
https://trigger.dev/docs/ai-chat/patterns/large-payloads is published; ensure
the change is made on the string returned by the ChatChunkTooLargeError
constructor so users get a working help path.
---
Nitpick comments:
In `@packages/core/src/v3/apiClient/index.ts`:
- Line 1291: Call site passes an explicit undefined to mergeRequestOptions which
is redundant; update the invocation that currently uses
mergeRequestOptions(this.defaultRequestOptions, undefined) to call
mergeRequestOptions(this.defaultRequestOptions) instead so mergeRequestOptions
can rely on its own undefined handling (refer to the mergeRequestOptions
function and defaultRequestOptions field to locate and verify the change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 56e37b2f-7656-4945-b7b8-db790a69f2fa
📒 Files selected for processing (91)
.changeset/sessions-primitive.md.gitignoreCLAUDE.mdapps/webapp/app/components/BulkActionFilterSummary.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/components/runs/v3/TaskTriggerSource.tsxapps/webapp/app/components/sessions/v1/CloseSessionDialog.tsxapps/webapp/app/components/sessions/v1/SessionFilters.tsxapps/webapp/app/components/sessions/v1/SessionStatus.tsxapps/webapp/app/components/sessions/v1/SessionsTable.tsxapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/presenters/SessionFilters.server.tsapps/webapp/app/presenters/v3/ApiRunListPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.tsapps/webapp/app/presenters/v3/SessionListPresenter.server.tsapps/webapp/app/presenters/v3/SessionPresenter.server.tsapps/webapp/app/presenters/v3/TaskListPresenter.server.tsapps/webapp/app/presenters/v3/TestPresenter.server.tsapps/webapp/app/presenters/v3/TestTaskPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsxapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.tsapps/webapp/app/routes/resources.sessions.$sessionParam.close.tsapps/webapp/app/routes/runs.$runParam.tsapps/webapp/app/runEngine/concerns/queues.server.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/runEngine/types.tsapps/webapp/app/services/apiRateLimit.server.tsapps/webapp/app/services/realtime/mintRunToken.server.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tsapps/webapp/app/services/realtime/sessionRunManager.server.tsapps/webapp/app/services/runsReplicationService.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.tsapps/webapp/app/services/sessionsRepository/sessionsRepository.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/v3/services/createBackgroundWorker.server.tsinternal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sqlinternal-packages/clickhouse/src/taskRuns.test.tsinternal-packages/clickhouse/src/taskRuns.tsinternal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sqlinternal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sqlinternal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sqlinternal-packages/database/prisma/schema.prismapackages/core/src/v3/apiClient/errors.tspackages/core/src/v3/apiClient/index.tspackages/core/src/v3/apiClient/runStream.test.tspackages/core/src/v3/apiClient/runStream.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/inputStreams/index.tspackages/core/src/v3/inputStreams/manager.tspackages/core/src/v3/inputStreams/noopManager.tspackages/core/src/v3/inputStreams/types.tspackages/core/src/v3/realtime-streams-api.tspackages/core/src/v3/realtimeStreams/index.tspackages/core/src/v3/realtimeStreams/manager.tspackages/core/src/v3/realtimeStreams/noopManager.tspackages/core/src/v3/realtimeStreams/sessionStreamInstance.tspackages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/realtimeStreams/streamsWriterV1.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.test.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.tspackages/core/src/v3/realtimeStreams/types.tspackages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/build.tspackages/core/src/v3/schemas/resources.tspackages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/schemas.tspackages/core/src/v3/semanticInternalAttributes.tspackages/core/src/v3/session-streams-api.tspackages/core/src/v3/sessionStreams/index.tspackages/core/src/v3/sessionStreams/manager.test.tspackages/core/src/v3/sessionStreams/manager.tspackages/core/src/v3/sessionStreams/noopManager.tspackages/core/src/v3/sessionStreams/types.tspackages/core/src/v3/test/test-input-stream-manager.tspackages/core/src/v3/test/test-realtime-streams-manager.tspackages/core/src/v3/test/test-run-metadata-manager.tspackages/core/src/v3/test/test-session-stream-manager.tspackages/core/src/v3/types/tasks.tspackages/core/src/v3/utils/globals.tspackages/core/src/v3/workers/index.tspackages/core/src/v3/workers/taskExecutor.tspackages/core/test/runStream.test.ts
✅ Files skipped from review due to trivial changes (11)
- .gitignore
- CLAUDE.md
- packages/core/src/v3/semanticInternalAttributes.ts
- apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
- apps/webapp/app/presenters/RunFilters.server.ts
- packages/core/src/v3/realtime-streams-api.ts
- packages/core/src/v3/realtimeStreams/index.ts
- .changeset/sessions-primitive.md
- packages/core/src/v3/schemas/build.ts
- apps/webapp/app/runEngine/types.ts
- packages/core/src/v3/sessionStreams/types.ts
🚧 Files skipped from review as they are similar to previous changes (75)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsx
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
- apps/webapp/app/services/sessionsRepository/sessionsRepository.server.ts
- apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
- internal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sql
- internal-packages/clickhouse/src/taskRuns.ts
- apps/webapp/app/services/runsReplicationService.server.ts
- packages/core/src/v3/workers/taskExecutor.ts
- apps/webapp/app/components/BulkActionFilterSummary.tsx
- packages/core/src/v3/session-streams-api.ts
- packages/core/src/v3/inputStreams/noopManager.ts
- apps/webapp/app/presenters/v3/TaskListPresenter.server.ts
- apps/webapp/app/services/runsRepository/runsRepository.server.ts
- apps/webapp/app/routes/api.v1.deployments.current.ts
- apps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.ts
- packages/core/src/v3/apiClient/errors.ts
- packages/core/src/v3/realtimeStreams/manager.ts
- packages/core/src/v3/inputStreams/manager.ts
- packages/core/src/v3/realtimeStreams/noopManager.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV2.ts
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts
- apps/webapp/app/components/sessions/v1/CloseSessionDialog.tsx
- packages/core/src/v3/schemas/resources.ts
- apps/webapp/app/services/realtime/mintRunToken.server.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV1.ts
- internal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sql
- packages/core/src/v3/workers/index.ts
- packages/core/src/v3/utils/globals.ts
- packages/core/src/v3/schemas/api.ts
- apps/webapp/app/services/apiRateLimit.server.ts
- internal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sql
- apps/webapp/app/presenters/SessionFilters.server.ts
- packages/core/src/v3/types/tasks.ts
- apps/webapp/app/runEngine/services/triggerTask.server.ts
- apps/webapp/app/components/runs/v3/TaskTriggerSource.tsx
- internal-packages/clickhouse/src/taskRuns.test.ts
- apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts
- apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.ts
- packages/core/src/v3/schemas/schemas.ts
- apps/webapp/app/routes/resources.sessions.$sessionParam.close.ts
- apps/webapp/app/utils/pathBuilder.ts
- packages/core/src/v3/schemas/runEngine.ts
- packages/core/src/v3/sessionStreams/manager.test.ts
- apps/webapp/app/runEngine/concerns/queues.server.ts
- apps/webapp/app/services/realtime/s2realtimeStreams.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
- packages/core/src/v3/realtimeStreams/sessionStreamInstance.ts
- packages/core/src/v3/apiClient/runStream.test.ts
- apps/webapp/app/components/runs/v3/RunFilters.tsx
- apps/webapp/app/components/sessions/v1/SessionStatus.tsx
- packages/core/src/v3/realtimeStreams/types.ts
- packages/core/test/runStream.test.ts
- apps/webapp/app/presenters/v3/TestPresenter.server.ts
- apps/webapp/app/presenters/v3/SessionListPresenter.server.ts
- packages/core/src/v3/inputStreams/index.ts
- packages/core/src/v3/test/test-realtime-streams-manager.ts
- packages/core/src/v3/sessionStreams/manager.ts
- packages/core/src/v3/inputStreams/types.ts
- packages/core/src/v3/sessionStreams/index.ts
- packages/core/src/v3/test/test-run-metadata-manager.ts
- packages/core/src/v3/sessionStreams/noopManager.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV2.test.ts
- apps/webapp/app/services/realtime/sessionRunManager.server.ts
- packages/core/src/v3/apiClient/runStream.ts
- apps/webapp/app/presenters/v3/SessionPresenter.server.ts
- packages/core/src/v3/realtimeStreams/streamInstance.ts
- packages/core/src/v3/test/test-input-stream-manager.ts
- internal-packages/database/prisma/schema.prisma
- apps/webapp/app/components/sessions/v1/SessionsTable.tsx
- packages/core/src/v3/test/test-session-stream-manager.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsx
- apps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.ts
- apps/webapp/app/components/sessions/v1/SessionFilters.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: typecheck / typecheck
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadAlways import from
@trigger.dev/sdk, never from@trigger.dev/sdk/v3or use deprecatedclient.defineJobUse the syntax
import { task } from '@trigger.dev/sdk'to define tasks with anidandrunasync functionImport
@trigger.dev/coreusing subpath imports only, never the root import
Files:
apps/webapp/app/routes/runs.$runParam.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/runs.$runParam.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Add crumbs as you write code using
//@Crumbscomments or `// `#region` `@crumbsblocks for debug tracing with agentcrumbs
Files:
apps/webapp/app/routes/runs.$runParam.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/routes/runs.$runParam.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/routes/runs.$runParam.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
apps/webapp/app/routes/runs.$runParam.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
internal-packages/database/**/prisma/migrations/*/*.sql
📄 CodeRabbit inference engine (internal-packages/database/CLAUDE.md)
internal-packages/database/**/prisma/migrations/*/*.sql: Clean up generated Prisma migrations by removing extraneous lines for junction tables (_BackgroundWorkerToBackgroundWorkerFile,_BackgroundWorkerToTaskQueue,_TaskRunToTaskRunTag,_WaitpointRunConnections,_completedWaitpoints) and indexes (SecretStore_key_idx, variousTaskRunindexes) unless explicitly added
When adding indexes to existing tables, useCREATE INDEX CONCURRENTLY IF NOT EXISTSto avoid table locks in production, and place each concurrent index in its own separate migration file
Indexes on newly created tables can useCREATE INDEXwithout CONCURRENTLY and can be combined in the same migration file as theCREATE TABLEstatement
When adding an index on a new column in an existing table, use two separate migrations: first forALTER TABLE ... ADD COLUMN IF NOT EXISTS ..., then forCREATE INDEX CONCURRENTLY IF NOT EXISTS ...in its own file
Files:
internal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sql
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: Use `typecheck` for verifying changes in apps and internal packages (`apps/*`, `internal-packages/*`), not `build`
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: Use `build` to verify changes in public packages (`packages/*`)
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: When modifying any public package (`packages/*` or `integrations/*`), add a changeset using `pnpm run changeset:add`
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: Default to **patch** for changeset versions for bug fixes and minor changes; confirm with maintainers before selecting **minor** or **major**
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: Do not update `rules/` directory or `.claude/skills/trigger-dev-tasks/` unless explicitly asked - these are maintained in separate dedicated passes
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: Only run typecheck/build after major changes (new files, significant refactors, schema changes), not for small edits
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: When modifying server components only (`apps/webapp/`, `apps/supervisor/`, etc.) with no package changes, add a `.server-changes/` file instead of a changeset
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:24:01.399Z
Learning: Only modify V2 code paths when encountering V1/V2 branching in services; never modify V1-only legacy code paths (MarQS queue, triggerTaskV1, cancelTaskRunV1, etc.)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/routes/runs.$runParam.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/routes/runs.$runParam.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/apiClient/index.ts
📚 Learning: 2026-02-03T18:48:31.790Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: internal-packages/database/prisma/migrations/20260129162810_add_integration_deployment/migration.sql:14-18
Timestamp: 2026-02-03T18:48:31.790Z
Learning: For Prisma migrations targeting PostgreSQL: - When adding indexes to existing tables, create the index in a separate migration file and include CONCURRENTLY to avoid locking the table. - For indexes on newly created tables (in CREATE TABLE statements), you can create the index in the same migration file without CONCURRENTLY. This reduces rollout complexity for new objects while protecting uptime for existing structures.
Applied to files:
internal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sql
📚 Learning: 2026-03-22T13:49:20.068Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: internal-packages/database/prisma/migrations/20260318114244_add_prompt_friendly_id/migration.sql:5-5
Timestamp: 2026-03-22T13:49:20.068Z
Learning: For Prisma migration SQL files under `internal-packages/database/prisma/migrations/`, it is acceptable to create indexes with `CREATE INDEX` / `CREATE UNIQUE INDEX` (i.e., without `CONCURRENTLY`) when the parent table is introduced in the same PR and has no existing production rows yet. Only require `CREATE INDEX CONCURRENTLY` (or otherwise account for existing production data/locks) when the table already exists in production with data.
Applied to files:
internal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sql
🔇 Additional comments (10)
packages/core/src/v3/index.ts (1)
24-24: LGTM!Also applies to: 84-84
apps/webapp/app/routes/runs.$runParam.ts (1)
31-31: LGTM!Also applies to: 61-67, 73-74
packages/core/src/v3/errors.ts (1)
650-652: LGTM!internal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sql (1)
1-5: LGTM!packages/core/src/v3/apiClient/index.ts (6)
1115-1212: LGTM!
1218-1241: LGTM!
1243-1259: LGTM!
1300-1336: LGTM!
1584-1594: LGTM!Also applies to: 1715-1730
2114-2114: The type cast on line 2114 is necessary and correct.ListSessionsOptions.statusis typed asSessionStatus | SessionStatus[] | undefined(whereSessionStatusis the Zod enumz.enum(["ACTIVE", "CLOSED", "EXPIRED"])), which requires explicit widening tostring | string[] | undefinedto match theappendManyfunction signature. This differs from fields likeoptions.typeandoptions.tag, which are already defined asz.union([z.string(), z.array(z.string())])and thus don't need casts.
| export class ChatChunkTooLargeError extends Error { | ||
| constructor( | ||
| public readonly chunkSize: number, | ||
| public readonly maxSize: number, | ||
| public readonly chunkType?: string | ||
| ) { | ||
| super( | ||
| `chat.agent chunk${chunkType ? ` of type "${chunkType}"` : ""} is ${chunkSize} bytes, ` + | ||
| `over the realtime stream's per-record cap of ${maxSize} bytes. ` + | ||
| `For oversized payloads (e.g. large tool outputs), write the value to your own store and ` + | ||
| `emit only an id/url through the chat stream — see https://trigger.dev/docs/ai-chat/patterns/large-payloads.` | ||
| ); | ||
| this.name = "ChatChunkTooLargeError"; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if the large-payloads docs URL returns a valid response
curl -s -o /dev/null -w "%{http_code}" https://trigger.dev/docs/ai-chat/patterns/large-payloadsRepository: triggerdotdev/trigger.dev
Length of output: 72
Ensure the documentation URL exists before this feature ships to users.
The error message references https://trigger.dev/docs/ai-chat/patterns/large-payloads, which currently returns a 404. Either stage this documentation page to deploy alongside this feature, or provide inline guidance (or defer the URL) until the docs are available. Users encountering this error should receive actionable help, not a broken link.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/v3/errors.ts` around lines 634 - 648, The error message in
ChatChunkTooLargeError (constructor, chunkSize/maxSize/chunkType) embeds a dead
docs link; update the constructor's message to either point to a valid
documentation URL (or the docs root) or remove the link and expand the inline
guidance so users get actionable steps (e.g., "write oversized payloads to your
own store and emit only an id/url through the chat stream") until the referenced
page https://trigger.dev/docs/ai-chat/patterns/large-payloads is published;
ensure the change is made on the string returned by the ChatChunkTooLargeError
constructor so users get a working help path.
365e73b to
b4a0986
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/webapp/app/routes/api.v1.deployments.current.ts (2)
20-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
updatedAtto the deployment select.The database query is missing the
updatedAtfield, which is required by theRetrieveCurrentDeploymentResponseBodyschema. Clients validating the response will fail without this field.🔧 Add the missing field to the select
select: { deployment: { select: { friendlyId: true, createdAt: true, + updatedAt: true, shortCode: true, version: true,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/api.v1.deployments.current.ts` around lines 20 - 33, The deployment select in the query (the nested select under deployment in routes/api.v1.deployments.current.ts) is missing the updatedAt field required by the RetrieveCurrentDeploymentResponseBody schema; add "updatedAt: true" to that deployment select object alongside existing fields (friendlyId, createdAt, shortCode, version, runtime, runtimeVersion, status, deployedAt, git, errorData) so the returned payload matches the schema.
42-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
updatedAtin the response object.The JSON response is missing the
updatedAtfield required byRetrieveCurrentDeploymentResponseBody. This must be added to match the API schema contract.🔧 Add the missing field to the response
return json({ id: deployment.friendlyId, createdAt: deployment.createdAt, + updatedAt: deployment.updatedAt, shortCode: deployment.shortCode, version: deployment.version,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/api.v1.deployments.current.ts` around lines 42 - 53, Response object returned by json(...) is missing the updatedAt field required by RetrieveCurrentDeploymentResponseBody; add updatedAt: deployment.updatedAt ?? undefined to the returned object (the same place where createdAt, deployedAt, git, and error are set) so the JSON matches the API schema.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx (1)
40-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn a user-facing 404 Response instead of throwing a generic Error.
The environment-not-found case should throw a Response with status 404 to provide proper error handling, matching the pattern used for the project-not-found case above.
🔧 Proposed fix
const environment = await findEnvironmentBySlug(project.id, envParam, userId); if (!environment) { - throw new Error("Environment not found"); + throw new Response("Environment not found", { status: 404 }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx around lines 40 - 42, Replace the generic throw new Error("Environment not found") with a user-facing 404 Response so the route mirrors the project-not-found pattern; specifically, when findEnvironmentBySlug(project.id, envParam, userId) returns falsy, throw new Response("Environment not found", { status: 404 }) (or the app's standard 404 Response helper) instead of throwing an Error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx:
- Line 78: The "Sessions docs" link in route.tsx uses
docsPath("/ai-chat/overview") which points to AI Chat docs, so either update the
docsPath argument to the correct sessions documentation route (replace
"/ai-chat/overview" with the real sessions docs path) or change the link text to
match the AI Chat destination (e.g., "AI Chat docs"); locate the anchor using
docsPath("/ai-chat/overview") and the visible text "Sessions docs" in the
sessions route(s) and make the swap consistently in both sessions route files.
---
Duplicate comments:
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx:
- Around line 40-42: Replace the generic throw new Error("Environment not
found") with a user-facing 404 Response so the route mirrors the
project-not-found pattern; specifically, when findEnvironmentBySlug(project.id,
envParam, userId) returns falsy, throw new Response("Environment not found", {
status: 404 }) (or the app's standard 404 Response helper) instead of throwing
an Error.
In `@apps/webapp/app/routes/api.v1.deployments.current.ts`:
- Around line 20-33: The deployment select in the query (the nested select under
deployment in routes/api.v1.deployments.current.ts) is missing the updatedAt
field required by the RetrieveCurrentDeploymentResponseBody schema; add
"updatedAt: true" to that deployment select object alongside existing fields
(friendlyId, createdAt, shortCode, version, runtime, runtimeVersion, status,
deployedAt, git, errorData) so the returned payload matches the schema.
- Around line 42-53: Response object returned by json(...) is missing the
updatedAt field required by RetrieveCurrentDeploymentResponseBody; add
updatedAt: deployment.updatedAt ?? undefined to the returned object (the same
place where createdAt, deployedAt, git, and error are set) so the JSON matches
the API schema.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 935320d9-506e-4dd2-84a6-6426641bb164
📒 Files selected for processing (91)
.changeset/chat-ready-core-additions.md.gitignoreCLAUDE.mdapps/webapp/app/components/BulkActionFilterSummary.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/components/runs/v3/TaskTriggerSource.tsxapps/webapp/app/components/sessions/v1/CloseSessionDialog.tsxapps/webapp/app/components/sessions/v1/SessionFilters.tsxapps/webapp/app/components/sessions/v1/SessionStatus.tsxapps/webapp/app/components/sessions/v1/SessionsTable.tsxapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/presenters/SessionFilters.server.tsapps/webapp/app/presenters/v3/ApiRunListPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.tsapps/webapp/app/presenters/v3/SessionListPresenter.server.tsapps/webapp/app/presenters/v3/SessionPresenter.server.tsapps/webapp/app/presenters/v3/TaskListPresenter.server.tsapps/webapp/app/presenters/v3/TestPresenter.server.tsapps/webapp/app/presenters/v3/TestTaskPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsxapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.tsapps/webapp/app/routes/resources.sessions.$sessionParam.close.tsapps/webapp/app/routes/runs.$runParam.tsapps/webapp/app/runEngine/concerns/queues.server.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/runEngine/types.tsapps/webapp/app/services/apiRateLimit.server.tsapps/webapp/app/services/realtime/mintRunToken.server.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tsapps/webapp/app/services/realtime/sessionRunManager.server.tsapps/webapp/app/services/runsReplicationService.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.tsapps/webapp/app/services/sessionsRepository/sessionsRepository.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/v3/services/createBackgroundWorker.server.tsinternal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sqlinternal-packages/clickhouse/src/taskRuns.test.tsinternal-packages/clickhouse/src/taskRuns.tsinternal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sqlinternal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sqlinternal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sqlinternal-packages/database/prisma/schema.prismapackages/core/src/v3/apiClient/errors.tspackages/core/src/v3/apiClient/index.tspackages/core/src/v3/apiClient/runStream.test.tspackages/core/src/v3/apiClient/runStream.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/inputStreams/index.tspackages/core/src/v3/inputStreams/manager.tspackages/core/src/v3/inputStreams/noopManager.tspackages/core/src/v3/inputStreams/types.tspackages/core/src/v3/realtime-streams-api.tspackages/core/src/v3/realtimeStreams/index.tspackages/core/src/v3/realtimeStreams/manager.tspackages/core/src/v3/realtimeStreams/noopManager.tspackages/core/src/v3/realtimeStreams/sessionStreamInstance.tspackages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/realtimeStreams/streamsWriterV1.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.test.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.tspackages/core/src/v3/realtimeStreams/types.tspackages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/build.tspackages/core/src/v3/schemas/resources.tspackages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/schemas.tspackages/core/src/v3/semanticInternalAttributes.tspackages/core/src/v3/session-streams-api.tspackages/core/src/v3/sessionStreams/index.tspackages/core/src/v3/sessionStreams/manager.test.tspackages/core/src/v3/sessionStreams/manager.tspackages/core/src/v3/sessionStreams/noopManager.tspackages/core/src/v3/sessionStreams/types.tspackages/core/src/v3/test/test-input-stream-manager.tspackages/core/src/v3/test/test-realtime-streams-manager.tspackages/core/src/v3/test/test-run-metadata-manager.tspackages/core/src/v3/test/test-session-stream-manager.tspackages/core/src/v3/types/tasks.tspackages/core/src/v3/utils/globals.tspackages/core/src/v3/workers/index.tspackages/core/src/v3/workers/taskExecutor.tspackages/core/test/runStream.test.ts
✅ Files skipped from review due to trivial changes (9)
- internal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sql
- .changeset/chat-ready-core-additions.md
- packages/core/src/v3/realtimeStreams/index.ts
- packages/core/src/v3/workers/index.ts
- CLAUDE.md
- .gitignore
- packages/core/src/v3/session-streams-api.ts
- internal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sql
- packages/core/src/v3/realtimeStreams/manager.ts
🚧 Files skipped from review as they are similar to previous changes (77)
- apps/webapp/app/services/runsReplicationService.server.ts
- apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
- packages/core/src/v3/index.ts
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
- apps/webapp/app/routes/runs.$runParam.ts
- packages/core/src/v3/realtime-streams-api.ts
- apps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.ts
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts
- apps/webapp/app/presenters/RunFilters.server.ts
- apps/webapp/app/services/apiRateLimit.server.ts
- packages/core/src/v3/apiClient/errors.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV1.ts
- packages/core/src/v3/realtimeStreams/noopManager.ts
- packages/core/src/v3/inputStreams/noopManager.ts
- packages/core/src/v3/schemas/resources.ts
- apps/webapp/app/services/realtime/mintRunToken.server.ts
- packages/core/src/v3/inputStreams/manager.ts
- apps/webapp/app/presenters/v3/TestPresenter.server.ts
- apps/webapp/app/components/BulkActionFilterSummary.tsx
- packages/core/src/v3/semanticInternalAttributes.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsx
- packages/core/src/v3/utils/globals.ts
- packages/core/src/v3/types/tasks.ts
- packages/core/src/v3/inputStreams/index.ts
- apps/webapp/app/services/sessionsRepository/sessionsRepository.server.ts
- apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
- apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
- apps/webapp/app/utils/pathBuilder.ts
- apps/webapp/app/components/runs/v3/TaskTriggerSource.tsx
- packages/core/src/v3/schemas/runEngine.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.ts
- internal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sql
- internal-packages/clickhouse/src/taskRuns.ts
- apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts
- apps/webapp/app/components/sessions/v1/SessionStatus.tsx
- internal-packages/clickhouse/src/taskRuns.test.ts
- apps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.ts
- packages/core/src/v3/sessionStreams/index.ts
- packages/core/src/v3/sessionStreams/noopManager.ts
- packages/core/src/v3/workers/taskExecutor.ts
- internal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sql
- apps/webapp/app/services/runsRepository/runsRepository.server.ts
- apps/webapp/app/runEngine/concerns/queues.server.ts
- packages/core/src/v3/schemas/api.ts
- packages/core/src/v3/test/test-input-stream-manager.ts
- apps/webapp/app/presenters/SessionFilters.server.ts
- apps/webapp/app/presenters/v3/SessionPresenter.server.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV2.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV2.test.ts
- packages/core/src/v3/sessionStreams/manager.test.ts
- packages/core/src/v3/inputStreams/types.ts
- apps/webapp/app/routes/resources.sessions.$sessionParam.close.ts
- apps/webapp/app/runEngine/services/triggerTask.server.ts
- apps/webapp/app/presenters/v3/TaskListPresenter.server.ts
- packages/core/src/v3/schemas/build.ts
- packages/core/src/v3/realtimeStreams/types.ts
- apps/webapp/app/components/sessions/v1/SessionsTable.tsx
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
- apps/webapp/app/presenters/v3/SessionListPresenter.server.ts
- apps/webapp/app/components/sessions/v1/CloseSessionDialog.tsx
- packages/core/test/runStream.test.ts
- packages/core/src/v3/test/test-realtime-streams-manager.ts
- packages/core/src/v3/test/test-run-metadata-manager.ts
- internal-packages/database/prisma/schema.prisma
- apps/webapp/app/runEngine/types.ts
- apps/webapp/app/services/realtime/sessionRunManager.server.ts
- packages/core/src/v3/apiClient/index.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsx
- apps/webapp/app/services/realtime/s2realtimeStreams.server.ts
- packages/core/src/v3/realtimeStreams/sessionStreamInstance.ts
- packages/core/src/v3/sessionStreams/types.ts
- packages/core/src/v3/sessionStreams/manager.ts
- packages/core/src/v3/apiClient/runStream.ts
- apps/webapp/app/components/sessions/v1/SessionFilters.tsx
- packages/core/src/v3/schemas/schemas.ts
- apps/webapp/app/components/runs/v3/RunFilters.tsx
- packages/core/src/v3/test/test-session-stream-manager.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tsapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxpackages/core/src/v3/apiClient/runStream.test.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tsapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxpackages/core/src/v3/apiClient/runStream.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Always import tasks from
@trigger.dev/sdk- never use@trigger.dev/sdk/v3or deprecatedclient.defineJobWhen importing from
@trigger.dev/core, import subpaths only (never the root) to maintain proper module structure
Files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tsapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxpackages/core/src/v3/apiClient/runStream.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tsapps/webapp/app/routes/api.v1.deployments.current.tspackages/core/src/v3/apiClient/runStream.test.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tspackages/core/src/v3/apiClient/runStream.test.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tsapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxpackages/core/src/v3/apiClient/runStream.test.ts
{packages,integrations}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Add a changeset when modifying any public package - use patch for bug fixes and minor changes, confirm with maintainers before selecting minor or major versions
Files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tspackages/core/src/v3/apiClient/runStream.test.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
Files:
packages/core/src/v3/apiClient/runStream.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx,js}: Use vitest for unit testing and run tests withpnpm run test
Test files should live beside the files under test with descriptivedescribeanditblocks
Tests should avoid mocks or stubs and use helpers from@internal/testcontainerswhen Redis or Postgres are needed
Files:
packages/core/src/v3/apiClient/runStream.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use vitest exclusively for testing - never mock anything; use testcontainers instead for Redis/PostgreSQL
Place test files next to source files with the naming convention:
SourceFile.ts→SourceFile.test.ts
Files:
packages/core/src/v3/apiClient/runStream.test.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:35:56.399Z
Learning: Use Run Engine 2.0 (`internal/run-engine`) and redis-worker for all new work - avoid legacy V1 engine code except where explicitly required
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:35:56.399Z
Learning: For V1/V2 branching in services, only modify V2 code paths - V1-only legacy code is restricted to specific files in `apps/webapp/`
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:35:56.399Z
Learning: Add agentcrumbs debug markers (`// crumbs` or `// `#region` crumbs`) as you write code during development, not just when debugging
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:35:56.399Z
Learning: Do not flag `// crumbs` markers, `#region crumbs` blocks, or agentcrumbs imports in code reviews - these are temporary debug instrumentation stripped before merge
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tsapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxpackages/core/src/v3/apiClient/runStream.test.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
packages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/errors.tsapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxpackages/core/src/v3/apiClient/runStream.test.ts
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
📚 Learning: 2026-04-02T19:18:26.255Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions/route.tsx:179-189
Timestamp: 2026-04-02T19:18:26.255Z
Learning: In this repo’s route components that render the Inspector `ResizablePanelGroup` panels, it’s acceptable to pass `collapsed={!isShowingInspector}` together with a no-op `onCollapseChange={() => {}}` when panel visibility is intentionally controlled only by route parameters (e.g., `*Param` search/route params) rather than user drag/collapse interactions. Do not flag an empty/no-op `onCollapseChange` as “missing wiring” in these cases; only flag it when collapse state is expected to change based on user interaction.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
📚 Learning: 2026-05-08T21:00:20.973Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3538
File: apps/webapp/app/components/primitives/Resizable.tsx:60-78
Timestamp: 2026-05-08T21:00:20.973Z
Learning: In the triggerdotdev/trigger.dev codebase, treat Zod as a boundary validation tool (API handlers, request/response validation, and storage/DB read/write validation), not as inline render-time validation inside React components/primitive UI code. For render-time guards, prefer small manual type-narrowing checks (e.g., a short predicate like ~10–20 lines) over importing Zod into UI primitives, to avoid per-render schema-parse overhead and unnecessary abstraction. Use the manual guard approach unless you truly need schema validation at a boundary; only then introduce Zod.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
🔇 Additional comments (7)
packages/core/src/v3/realtimeStreams/streamInstance.ts (1)
6-6: LGTM!Also applies to: 66-69
packages/core/src/v3/errors.ts (2)
634-648: The documentation URL issue remains unresolved.The past review correctly identified that the URL
https://trigger.dev/docs/ai-chat/patterns/large-payloadsreturns a 404. Before this feature ships to users, either stage the documentation to deploy alongside this PR or update the error message to remove the URL and expand the inline guidance.
650-652: LGTM!apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx (3)
1-22: LGTM!
23-29: LGTM!
66-77: LGTM!Also applies to: 79-107
packages/core/src/v3/apiClient/runStream.test.ts (1)
1-444: ⚖️ Poor tradeoffThe fetch mocking pattern used throughout this test file is consistent with the established approach in the apiClient folder—
streamBatchItems.test.tsuses the same pattern. This is the standard way to unit test HTTP client retry logic, backoff, timeouts, and abort handling without requiring actual HTTP servers or network simulation. While the guideline states "never mock anything," the context (mentioning testcontainers for Redis/PostgreSQL) suggests the concern is infrastructure testing rather than HTTP client unit tests. The mocking approach here is appropriate for testing SSEStreamSubscription's network-level retry behavior at the unit level.
| <LinkButton | ||
| variant={"docs/small"} | ||
| LeadingIcon={BookOpenIcon} | ||
| to={docsPath("/ai-chat/overview")} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if there's a sessions-specific docs path in the codebase
# Search for sessions-related doc paths
rg -n "docsPath.*session" --type ts --type tsx -C2
# Also check if ai-chat/overview is the standard path for sessions
rg -n "/ai-chat/overview" --type ts --type tsx -C2Repository: triggerdotdev/trigger.dev
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Search for sessions-related doc paths using correct rg syntax
echo "=== Searching for sessions doc paths ==="
rg "docsPath.*session" -g '*.ts' -g '*.tsx' -C2
echo ""
echo "=== Searching for ai-chat/overview references ==="
rg "ai-chat/overview" -g '*.ts' -g '*.tsx' -C2
echo ""
echo "=== Examining the specific file ==="
cat "apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.sessions._index/route.tsx" | head -100Repository: triggerdotdev/trigger.dev
Length of output: 5596
🏁 Script executed:
#!/bin/bash
# Find all unique docsPath calls to see what paths are used
echo "=== All docsPath usages in codebase ==="
rg "docsPath\(" -g '*.ts' -g '*.tsx' -o | sed 's/.*docsPath("\([^"]*\)").*/\1/' | sort -u
echo ""
echo "=== Checking for sessions documentation path ==="
rg "sessions|/session" -g '*.ts' -g '*.tsx' | grep -i "docs\|path" | head -20Repository: triggerdotdev/trigger.dev
Length of output: 4507
🏁 Script executed:
#!/bin/bash
# Extract the actual paths used in docsPath() calls
echo "=== Actual docsPath arguments ==="
rg 'docsPath\("([^"]+)"\)' -r '$1' -g '*.ts' -g '*.tsx' | sort -u
echo ""
echo "=== Lines around sessions docsPath calls ==="
rg -B2 -A2 'docsPath.*ai-chat/overview' -g '*.ts' -g '*.tsx'Repository: triggerdotdev/trigger.dev
Length of output: 13815
Update the sessions documentation link to point to the correct path or change the link text.
The link text displays "Sessions docs" but points to /ai-chat/overview, which is the AI Chat documentation. Either locate the correct sessions documentation path and update the to attribute, or if sessions documentation doesn't exist yet, change the link text to reflect where it actually links.
This mismatch exists in both sessions routes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
at line 78, The "Sessions docs" link in route.tsx uses
docsPath("/ai-chat/overview") which points to AI Chat docs, so either update the
docsPath argument to the correct sessions documentation route (replace
"/ai-chat/overview" with the real sessions docs path) or change the link text to
match the AI Chat destination (e.g., "AI Chat docs"); locate the anchor using
docsPath("/ai-chat/overview") and the visible text "Sessions docs" in the
sessions route(s) and make the swap consistently in both sessions route files.
1712b59 to
3721c34
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trigger-sdk/src/v3/tasks.ts (1)
1-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a changeset for the new
@trigger.dev/trigger-sdkexports.
triggerAndSubscribeis a new exported API on@trigger.dev/sdk(exported fromtasks.*and as both a function export and instance method inshared.ts). Per repo conventions, public-package changes require an accompanying changeset entry — for a new feature this typically warrants aminorbump, but confirm the version bump level with maintainers before adding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trigger-sdk/src/v3/tasks.ts` around lines 1 - 115, The PR added a new public API (triggerAndSubscribe exported as a top-level export and via tasks.triggerAndSubscribe) but did not include a changeset; add a changeset file describing the new feature for the `@trigger.dev/trigger-sdk` package (include the package name, the new export triggerAndSubscribe, and mark the bump level—typically "minor" for a new public API unless maintainers instruct otherwise), commit it alongside the code changes so the release tooling will pick up the version bump.
🧹 Nitpick comments (2)
packages/trigger-sdk/src/v3/shared.ts (1)
218-237: 💤 Low valueInstance methods silently drop
signallifetime /requestOptions.The instance-method shape
triggerAndSubscribe: (payload, options) => ...is fine for parity withcreateSchemaTask.triggerAndWait, but note that theTaskRunPromisewrapper does not currently relayoptions.signalfor promise-level cancellation semantics (i.e., callers awaitingtask.triggerAndSubscribe(...)cannot abort the awaited promise itself short of awaitingsubscribeToRun's own teardown). That's consistent with the rest of the file, but worth confirming this is intended for the chat.agent use case where abort propagation is a stated goal oftriggerAndSubscribe(per the new JSDoc at lines 514-516).If/when the public
triggerAndSubscribelearns aboutrequestOptions(see the other comment on lines 534-554), please add the same parameter to these instance methods for symmetry withcreateTask.triggerAndWait(line 197).Also applies to: 372-391
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trigger-sdk/src/v3/shared.ts` around lines 218 - 237, The instance method triggerAndSubscribe currently drops options.signal / requestOptions when wrapping triggerAndSubscribe_internal in a new TaskRunPromise; update the signature of the instance method(s) (triggerAndSubscribe at ~218 and the analogous one at ~372) to accept a requestOptions or signal parameter, thread that signal into the TaskRunPromise so the promise-level cancellation is supported, and forward the same requestOptions/signal through to triggerAndSubscribe_internal (and to any TaskRunPromise constructor hook that can cancel the inner subscription) to maintain parity with createTask.triggerAndWait and ensure abort propagation to awaiting callers.packages/trigger-sdk/src/v3/streams.ts (1)
768-909: ⚡ Quick winAdd
@crumbsaround the new wait lifecycle branches.This adds a lot of new idle/suspend/resume control flow in a public SDK path, but none of the new branches are breadcrumbed. Please mark the waitpoint creation,
skipSuspend, suspend, and resume paths so rollout debugging can tell which branch fired.As per coding guidelines, "Add crumbs as you write code using
//@Crumbscomments or wrap blocks in `// `#region` `@crumbs."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trigger-sdk/src/v3/streams.ts` around lines 768 - 909, The new wait lifecycle branches in wait() and waitWithIdleTimeout() lack breadcrumb markers; add // `@crumbs` comments (or // `#region` `@crumbs` blocks) around the waitpoint creation (after apiClient.createInputStreamWaitpoint/response), around the skip-suspend short-circuit (inside waitWithIdleTimeout() when options.skipSuspend is true), around the suspend path (before calling runtime.waitUntil in wait() and before calling self.wait(...) with spanName "suspended" in waitWithIdleTimeout()), and around the resume path (after successful waitResult.ok where inputStreams.setLastSeqNum and options.onResume are invoked) so rollout debugging can identify which branch executed. Ensure crumbs reference the unique symbols response.waitpointId, runtime.waitUntil, self.wait, inputStreams.lastSeqNum/setLastSeqNum, and options.onResume for easy locating.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/trigger-sdk/src/v3/shared.ts`:
- Around line 2594-2605: The abort listener added when options.signal is present
(using onAbort and options.signal.addEventListener) is never removed on normal
completion, causing listener leaks and retained closures; update the flow in the
trigger/subscribe routine so that when the run finishes (success, failure, or
cancel) you explicitly remove the listener via
options.signal.removeEventListener(onAbort) (or register cleanup in a finally
block) and then proceed to normal resolution; also replace the non-idiomatic
throw new Error("Aborted") with an abort-style error (e.g., a
DOMException("Aborted", "AbortError") or an Error with name "AbortError") so
callers can detect aborts consistently; ensure references to
apiClient.cancelRun(response.id) remain wrapped in the onAbort handler and that
removeEventListener runs before releasing references to apiClient/response.id.
- Around line 534-554: Public triggerAndSubscribe is missing a requestOptions
parameter; update its signature to accept a RequestOptions (e.g.,
TriggerApiRequestOptions | ApiRequestOptions) and pass it into
triggerAndSubscribe_internal so per-call clientConfig/timeout/headers can be
applied. Specifically, change triggerAndSubscribe to add the requestOptions
argument and thread it into
triggerAndSubscribe_internal("tasks.triggerAndSubscribe()", id, payload,
requestOptions, options), then inside triggerAndSubscribe_internal use
apiClientManager.clientOrThrow(requestOptions?.clientConfig) and forward
requestOptions into apiClient.triggerTask(..., requestOptions) (mirroring
triggerAndWait_internal).
- Around line 2607-2631: The error path in triggerAndSubscribe currently builds
a plain Error from the SerializedError returned by subscribeToRun, losing
TaskRunError discriminators and diverging from triggerAndWait's use of
createErrorTaskError; update the failure branch in triggerAndSubscribe to
convert the SerializedError (run.error) into the same TaskRunError shape
expected by createErrorTaskError (or call createErrorTaskError with a mapped
TaskRunError) so you preserve
BUILT_IN_ERROR/CUSTOM_ERROR/INTERNAL_ERROR/STRING_ERROR discriminators and
stack/name/message; locate the failure handling inside the async loop over
apiClient.subscribeToRun and replace the new Error(...) construction with a call
to createErrorTaskError(mappedError) where mappedError is created from run.error
(SerializedError) following the same mapping logic used by
triggerAndWait/createJsonErrorObject.
In `@packages/trigger-sdk/src/v3/streams.ts`:
- Around line 873-877: The skipSuspend branch currently returns { ok: false,
error: undefined } which breaks callers expecting a concrete
WaitpointTimeoutError; update that branch in the wait() implementation to
construct and return a real WaitpointTimeoutError (or the same error type used
elsewhere) — e.g., create new WaitpointTimeoutError("suspend skipped" or
similar), keep the span.setAttribute("wait.resolved","skipped") and return { ok:
false, error: thatError } so waitWithIdleTimeout() and callers that do throw
result.error or access result.error.message receive a proper Error instance.
---
Outside diff comments:
In `@packages/trigger-sdk/src/v3/tasks.ts`:
- Around line 1-115: The PR added a new public API (triggerAndSubscribe exported
as a top-level export and via tasks.triggerAndSubscribe) but did not include a
changeset; add a changeset file describing the new feature for the
`@trigger.dev/trigger-sdk` package (include the package name, the new export
triggerAndSubscribe, and mark the bump level—typically "minor" for a new public
API unless maintainers instruct otherwise), commit it alongside the code changes
so the release tooling will pick up the version bump.
---
Nitpick comments:
In `@packages/trigger-sdk/src/v3/shared.ts`:
- Around line 218-237: The instance method triggerAndSubscribe currently drops
options.signal / requestOptions when wrapping triggerAndSubscribe_internal in a
new TaskRunPromise; update the signature of the instance method(s)
(triggerAndSubscribe at ~218 and the analogous one at ~372) to accept a
requestOptions or signal parameter, thread that signal into the TaskRunPromise
so the promise-level cancellation is supported, and forward the same
requestOptions/signal through to triggerAndSubscribe_internal (and to any
TaskRunPromise constructor hook that can cancel the inner subscription) to
maintain parity with createTask.triggerAndWait and ensure abort propagation to
awaiting callers.
In `@packages/trigger-sdk/src/v3/streams.ts`:
- Around line 768-909: The new wait lifecycle branches in wait() and
waitWithIdleTimeout() lack breadcrumb markers; add // `@crumbs` comments (or //
`#region` `@crumbs` blocks) around the waitpoint creation (after
apiClient.createInputStreamWaitpoint/response), around the skip-suspend
short-circuit (inside waitWithIdleTimeout() when options.skipSuspend is true),
around the suspend path (before calling runtime.waitUntil in wait() and before
calling self.wait(...) with spanName "suspended" in waitWithIdleTimeout()), and
around the resume path (after successful waitResult.ok where
inputStreams.setLastSeqNum and options.onResume are invoked) so rollout
debugging can identify which branch executed. Ensure crumbs reference the unique
symbols response.waitpointId, runtime.waitUntil, self.wait,
inputStreams.lastSeqNum/setLastSeqNum, and options.onResume for easy locating.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b9fc1d21-ac87-4eca-a7e9-7b16f61dbf81
📒 Files selected for processing (94)
.changeset/chat-ready-core-additions.md.gitignoreCLAUDE.mdapps/webapp/app/components/BulkActionFilterSummary.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/components/runs/v3/TaskTriggerSource.tsxapps/webapp/app/components/sessions/v1/CloseSessionDialog.tsxapps/webapp/app/components/sessions/v1/SessionFilters.tsxapps/webapp/app/components/sessions/v1/SessionStatus.tsxapps/webapp/app/components/sessions/v1/SessionsTable.tsxapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/presenters/SessionFilters.server.tsapps/webapp/app/presenters/v3/ApiRunListPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.tsapps/webapp/app/presenters/v3/SessionListPresenter.server.tsapps/webapp/app/presenters/v3/SessionPresenter.server.tsapps/webapp/app/presenters/v3/TaskListPresenter.server.tsapps/webapp/app/presenters/v3/TestPresenter.server.tsapps/webapp/app/presenters/v3/TestTaskPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsxapps/webapp/app/routes/api.v1.deployments.current.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.tsapps/webapp/app/routes/resources.sessions.$sessionParam.close.tsapps/webapp/app/routes/runs.$runParam.tsapps/webapp/app/runEngine/concerns/queues.server.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/runEngine/types.tsapps/webapp/app/services/apiRateLimit.server.tsapps/webapp/app/services/realtime/mintRunToken.server.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tsapps/webapp/app/services/realtime/sessionRunManager.server.tsapps/webapp/app/services/runsReplicationService.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.tsapps/webapp/app/services/sessionsRepository/sessionsRepository.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/v3/services/createBackgroundWorker.server.tsinternal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sqlinternal-packages/clickhouse/src/taskRuns.test.tsinternal-packages/clickhouse/src/taskRuns.tsinternal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sqlinternal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sqlinternal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sqlinternal-packages/database/prisma/schema.prismapackages/core/src/v3/apiClient/errors.tspackages/core/src/v3/apiClient/index.tspackages/core/src/v3/apiClient/runStream.test.tspackages/core/src/v3/apiClient/runStream.tspackages/core/src/v3/errors.tspackages/core/src/v3/index.tspackages/core/src/v3/inputStreams/index.tspackages/core/src/v3/inputStreams/manager.tspackages/core/src/v3/inputStreams/noopManager.tspackages/core/src/v3/inputStreams/types.tspackages/core/src/v3/realtime-streams-api.tspackages/core/src/v3/realtimeStreams/index.tspackages/core/src/v3/realtimeStreams/manager.tspackages/core/src/v3/realtimeStreams/noopManager.tspackages/core/src/v3/realtimeStreams/sessionStreamInstance.tspackages/core/src/v3/realtimeStreams/streamInstance.tspackages/core/src/v3/realtimeStreams/streamsWriterV1.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.test.tspackages/core/src/v3/realtimeStreams/streamsWriterV2.tspackages/core/src/v3/realtimeStreams/types.tspackages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/build.tspackages/core/src/v3/schemas/resources.tspackages/core/src/v3/schemas/runEngine.tspackages/core/src/v3/schemas/schemas.tspackages/core/src/v3/semanticInternalAttributes.tspackages/core/src/v3/session-streams-api.tspackages/core/src/v3/sessionStreams/index.tspackages/core/src/v3/sessionStreams/manager.test.tspackages/core/src/v3/sessionStreams/manager.tspackages/core/src/v3/sessionStreams/noopManager.tspackages/core/src/v3/sessionStreams/types.tspackages/core/src/v3/test/test-input-stream-manager.tspackages/core/src/v3/test/test-realtime-streams-manager.tspackages/core/src/v3/test/test-run-metadata-manager.tspackages/core/src/v3/test/test-session-stream-manager.tspackages/core/src/v3/types/tasks.tspackages/core/src/v3/utils/globals.tspackages/core/src/v3/workers/index.tspackages/core/src/v3/workers/taskExecutor.tspackages/core/test/runStream.test.tspackages/trigger-sdk/src/v3/shared.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/tasks.ts
✅ Files skipped from review due to trivial changes (13)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions/route.tsx
- packages/core/src/v3/session-streams-api.ts
- packages/core/src/v3/index.ts
- .changeset/chat-ready-core-additions.md
- .gitignore
- packages/core/src/v3/workers/taskExecutor.ts
- packages/core/src/v3/realtimeStreams/index.ts
- internal-packages/clickhouse/src/taskRuns.test.ts
- apps/webapp/app/presenters/v3/TaskListPresenter.server.ts
- apps/webapp/app/services/realtime/mintRunToken.server.ts
- CLAUDE.md
- packages/core/src/v3/test/test-run-metadata-manager.ts
- internal-packages/clickhouse/schema/029_add_task_kind_to_task_runs_v2.sql
🚧 Files skipped from review as they are similar to previous changes (76)
- packages/core/src/v3/realtimeStreams/noopManager.ts
- apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
- apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
- apps/webapp/app/presenters/RunFilters.server.ts
- apps/webapp/app/components/runs/v3/TaskTriggerSource.tsx
- apps/webapp/app/services/apiRateLimit.server.ts
- apps/webapp/app/routes/runs.$runParam.ts
- apps/webapp/app/services/sessionsRepository/sessionsRepository.server.ts
- apps/webapp/app/components/BulkActionFilterSummary.tsx
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
- packages/core/src/v3/workers/index.ts
- packages/core/src/v3/realtimeStreams/streamInstance.ts
- packages/core/src/v3/schemas/runEngine.ts
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts
- apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts
- packages/core/src/v3/semanticInternalAttributes.ts
- packages/core/src/v3/errors.ts
- packages/core/src/v3/realtimeStreams/manager.ts
- packages/core/src/v3/realtime-streams-api.ts
- packages/core/src/v3/schemas/schemas.ts
- apps/webapp/app/services/sessionsRepository/clickhouseSessionsRepository.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx
- packages/core/src/v3/types/tasks.ts
- internal-packages/clickhouse/src/taskRuns.ts
- apps/webapp/app/utils/pathBuilder.ts
- apps/webapp/app/routes/resources.sessions.$sessionParam.close.ts
- apps/webapp/app/presenters/SessionFilters.server.ts
- packages/core/src/v3/apiClient/errors.ts
- apps/webapp/app/components/sessions/v1/CloseSessionDialog.tsx
- packages/core/src/v3/schemas/resources.ts
- apps/webapp/app/services/runsRepository/runsRepository.server.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV2.test.ts
- packages/core/src/v3/sessionStreams/index.ts
- apps/webapp/app/components/sessions/v1/SessionStatus.tsx
- packages/core/src/v3/realtimeStreams/sessionStreamInstance.ts
- packages/core/src/v3/inputStreams/manager.ts
- packages/core/src/v3/sessionStreams/noopManager.ts
- packages/core/src/v3/utils/globals.ts
- packages/core/src/v3/inputStreams/types.ts
- internal-packages/database/prisma/migrations/20260329100903_add_agent_trigger_source_and_task_config/migration.sql
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
- apps/webapp/app/routes/api.v1.deployments.current.ts
- internal-packages/database/prisma/migrations/20260330113734_add_playground_conversation/migration.sql
- packages/core/src/v3/realtimeStreams/streamsWriterV2.ts
- internal-packages/database/prisma/migrations/20260330135232_add_messages_and_last_event_id_to_playground/migration.sql
- packages/core/test/runStream.test.ts
- packages/core/src/v3/realtimeStreams/streamsWriterV1.ts
- apps/webapp/app/presenters/v3/SessionListPresenter.server.ts
- apps/webapp/app/services/runsReplicationService.server.ts
- apps/webapp/app/runEngine/concerns/queues.server.ts
- packages/core/src/v3/sessionStreams/manager.test.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam.realtime.v1.$io.ts
- apps/webapp/app/runEngine/services/triggerTask.server.ts
- packages/core/src/v3/inputStreams/index.ts
- apps/webapp/app/presenters/v3/SessionPresenter.server.ts
- packages/core/src/v3/realtimeStreams/types.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions.$sessionParam/route.tsx
- apps/webapp/app/runEngine/types.ts
- apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
- packages/core/src/v3/test/test-input-stream-manager.ts
- packages/core/src/v3/schemas/build.ts
- apps/webapp/app/presenters/v3/TestPresenter.server.ts
- packages/core/src/v3/test/test-realtime-streams-manager.ts
- apps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.ts
- packages/core/src/v3/schemas/api.ts
- apps/webapp/app/components/runs/v3/RunFilters.tsx
- apps/webapp/app/components/sessions/v1/SessionFilters.tsx
- packages/core/src/v3/apiClient/index.ts
- packages/core/src/v3/apiClient/runStream.test.ts
- internal-packages/database/prisma/schema.prisma
- packages/core/src/v3/sessionStreams/types.ts
- apps/webapp/app/services/realtime/sessionRunManager.server.ts
- packages/core/src/v3/sessionStreams/manager.ts
- packages/core/src/v3/test/test-session-stream-manager.ts
- packages/core/src/v3/apiClient/runStream.ts
- apps/webapp/app/components/sessions/v1/SessionsTable.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/core/src/v3/inputStreams/noopManager.tspackages/trigger-sdk/src/v3/tasks.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/inputStreams/noopManager.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Always import tasks from
@trigger.dev/sdk, never from@trigger.dev/sdk/v3or use deprecatedclient.defineJob.Import from
@trigger.dev/coreusing subpaths only, never from the root.Add crumbs as you write code using
//@Crumbscomments or wrap blocks in `// `#region` `@crumbs. They stay on the branch throughout development and are stripped byagentcrumbs stripbefore merge.
Files:
packages/core/src/v3/inputStreams/noopManager.tspackages/trigger-sdk/src/v3/tasks.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
packages/core/src/v3/inputStreams/noopManager.tspackages/trigger-sdk/src/v3/tasks.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/inputStreams/noopManager.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
packages/core/src/v3/inputStreams/noopManager.tspackages/trigger-sdk/src/v3/tasks.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
packages/trigger-sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Files:
packages/trigger-sdk/src/v3/tasks.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
packages/trigger-sdk/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/trigger-sdk/CLAUDE.md)
Always import from
@trigger.dev/sdk. Never use@trigger.dev/sdk/v3(deprecated path alias)
Files:
packages/trigger-sdk/src/v3/tasks.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/services/realtime/s2realtimeStreams.server.ts
apps/webapp/**/*.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.ts: Never userequest.signalfor detecting client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.tsinstead, which is wired directly to Expressres.on('close')and fires reliably
Access environment variables viaenvexport fromapp/env.server.ts. Never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries.findUniquehas an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance).findFirstis never batched and avoids this entire class of issues
Files:
apps/webapp/app/services/realtime/s2realtimeStreams.server.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:40:33.677Z
Learning: Edit `package.json` directly instead of using `pnpm add` when adding dependencies, then run `pnpm i` from the repo root. See `.claude/rules/package-installation.md` for the full process.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:40:33.677Z
Learning: For Apps and internal packages (`apps/*`, `internal-packages/*`), use `typecheck` for verification after changes. Never use `build` for these — building proves almost nothing about correctness.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:40:33.677Z
Learning: For Public packages (`packages/*`), use `build` for verification after changes.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:40:33.677Z
Learning: When modifying any public package (`packages/*` or `integrations/*`), add a changeset using `pnpm run changeset:add`. Default to patch for bug fixes and minor changes, confirm with maintainers before selecting minor for new features, and never select major without explicit approval.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:40:33.677Z
Learning: When modifying only server components (`apps/webapp/`, `apps/supervisor/`, etc.) with no package changes, add a `.server-changes/` file instead of a changeset.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:40:33.677Z
Learning: Do not update `rules/` directory or `.claude/skills/trigger-dev-tasks/` unless explicitly asked - these are maintained in separate dedicated passes.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-12T08:40:33.677Z
Learning: Do not flag `// crumbs` markers, `#region crumbs` blocks, or agentcrumbs imports in PR reviews - these are temporary debug instrumentation stripped before merge.
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
packages/core/src/v3/inputStreams/noopManager.tspackages/trigger-sdk/src/v3/tasks.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
packages/core/src/v3/inputStreams/noopManager.tspackages/trigger-sdk/src/v3/tasks.tsapps/webapp/app/services/realtime/s2realtimeStreams.server.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2026-03-31T21:37:27.212Z
Learnt from: isshaddad
Repo: triggerdotdev/trigger.dev PR: 3283
File: docs/migration-n8n.mdx:19-21
Timestamp: 2026-03-31T21:37:27.212Z
Learning: When reviewing code in `packages/trigger-sdk/src/v3`, treat `tasks.triggerAndWait()` and `tasks.batchTriggerAndWait()` as real exported APIs. They are defined in `shared.ts` and re-exported via the `tasks` object in `tasks.ts`, and they take the task ID string as their first argument (not a task instance). This is distinct from the instance methods `yourTask.triggerAndWait()` and `yourTask.batchTriggerAndWait()`. Do not flag calls to `tasks.triggerAndWait()` or `tasks.batchTriggerAndWait()` as non-existent or incorrectly invoked.
Applied to files:
packages/trigger-sdk/src/v3/tasks.tspackages/trigger-sdk/src/v3/streams.tspackages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2026-03-26T09:02:07.973Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:07.973Z
Learning: When parsing Trigger.dev task run annotations in server-side services, keep `TaskRun.annotations` strictly conforming to the `RunAnnotations` schema from `trigger.dev/core/v3`. If the code already uses `RunAnnotations.safeParse` (e.g., in a `#parseAnnotations` helper), treat that as intentional/necessary for atomic, schema-accurate annotation handling. Do not recommend relaxing the annotation payload schema or using a permissive “passthrough” parse path, since the annotations are expected to be written atomically in one operation and should not contain partial/legacy payloads that would require a looser parser.
Applied to files:
apps/webapp/app/services/realtime/s2realtimeStreams.server.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/services/realtime/s2realtimeStreams.server.ts
🔇 Additional comments (8)
apps/webapp/app/services/realtime/s2realtimeStreams.server.ts (2)
443-516: LGTM!
619-655: LGTM!packages/trigger-sdk/src/v3/shared.ts (2)
285-285: LGTM!
259-260: ⚡ Quick winNo changes needed.
triggerSourceandagentConfigare properly defined as optional fields inCommonTaskOptions(the base type forTaskOptions,TaskOptionsWithSchema, andTaskWithSchemaOptions), which are used as parameters for bothcreateTaskandcreateSchemaTask. The Zod schemas inregisterTaskMetadataalso include these fields. No type-checking failures or undefined-out issues will occur.packages/trigger-sdk/src/v3/tasks.ts (1)
23-23: LGTM!Also applies to: 100-100
packages/core/src/v3/inputStreams/noopManager.ts (1)
25-29: LGTM!packages/trigger-sdk/src/v3/streams.ts (2)
28-31: LGTM!Also applies to: 144-145, 172-173, 200-202, 648-648, 664-675, 731-731, 779-779, 834-834
792-797: The implementation already addresses this concern. ThedisconnectStream()method inpackages/core/src/v3/inputStreams/manager.ts(line 181–187) explicitly clears the buffer withthis.buffer.delete(streamId)before returning. This ensures buffered entries are dropped during the suspension window, preventing duplicate delivery on resume as intended by the comment at lines 792–795.> Likely an incorrect or invalid review comment.
| export function triggerAndSubscribe<TTask extends AnyTask>( | ||
| id: TaskIdentifier<TTask>, | ||
| payload: TaskPayload<TTask>, | ||
| options?: TriggerAndSubscribeOptions | ||
| ): TaskRunPromise<TaskIdentifier<TTask>, TaskOutput<TTask>> { | ||
| return new TaskRunPromise<TaskIdentifier<TTask>, TaskOutput<TTask>>((resolve, reject) => { | ||
| triggerAndSubscribe_internal<TaskIdentifier<TTask>, TaskPayload<TTask>, TaskOutput<TTask>>( | ||
| "tasks.triggerAndSubscribe()", | ||
| id, | ||
| payload, | ||
| undefined, | ||
| options | ||
| ) | ||
| .then((result) => { | ||
| resolve(result); | ||
| }) | ||
| .catch((error) => { | ||
| reject(error); | ||
| }); | ||
| }, id); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Public triggerAndSubscribe is missing a requestOptions parameter.
Sibling APIs (trigger, triggerAndWait, batchTrigger, etc.) all accept an ApiRequestOptions/TriggerApiRequestOptions argument so callers can pass clientConfig, custom request timeouts, headers, etc. triggerAndSubscribe doesn't, and triggerAndSubscribe_internal hard-codes apiClientManager.clientOrThrow() (no clientConfig). This makes the SDK inconsistent and prevents per-call overrides (notably multi-project/multi-API-key setups).
♻️ Suggested signature change
export function triggerAndSubscribe<TTask extends AnyTask>(
id: TaskIdentifier<TTask>,
payload: TaskPayload<TTask>,
- options?: TriggerAndSubscribeOptions
+ options?: TriggerAndSubscribeOptions,
+ requestOptions?: ApiRequestOptions
): TaskRunPromise<TaskIdentifier<TTask>, TaskOutput<TTask>> {
return new TaskRunPromise<TaskIdentifier<TTask>, TaskOutput<TTask>>((resolve, reject) => {
triggerAndSubscribe_internal<TaskIdentifier<TTask>, TaskPayload<TTask>, TaskOutput<TTask>>(
"tasks.triggerAndSubscribe()",
id,
payload,
undefined,
- options
+ options,
+ requestOptions
)Then thread requestOptions through triggerAndSubscribe_internal (use it in apiClientManager.clientOrThrow(requestOptions?.clientConfig) and in the apiClient.triggerTask(..., requestOptions) call, mirroring triggerAndWait_internal).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/trigger-sdk/src/v3/shared.ts` around lines 534 - 554, Public
triggerAndSubscribe is missing a requestOptions parameter; update its signature
to accept a RequestOptions (e.g., TriggerApiRequestOptions | ApiRequestOptions)
and pass it into triggerAndSubscribe_internal so per-call
clientConfig/timeout/headers can be applied. Specifically, change
triggerAndSubscribe to add the requestOptions argument and thread it into
triggerAndSubscribe_internal("tasks.triggerAndSubscribe()", id, payload,
requestOptions, options), then inside triggerAndSubscribe_internal use
apiClientManager.clientOrThrow(requestOptions?.clientConfig) and forward
requestOptions into apiClient.triggerTask(..., requestOptions) (mirroring
triggerAndWait_internal).
| // Optionally cancel the child run when the abort signal fires (default: true) | ||
| const cancelOnAbort = options?.cancelOnAbort !== false; | ||
| if (options?.signal && cancelOnAbort) { | ||
| const onAbort = () => { | ||
| apiClient.cancelRun(response.id).catch(() => {}); | ||
| }; | ||
| if (options.signal.aborted) { | ||
| await apiClient.cancelRun(response.id).catch(() => {}); | ||
| throw new Error("Aborted"); | ||
| } | ||
| options.signal.addEventListener("abort", onAbort, { once: true }); | ||
| } |
There was a problem hiding this comment.
AbortSignal listener leak on normal completion.
When signal is provided and the run completes (success/failure/cancel) without the signal firing, the abort listener is never removed. Since { once: true } only auto-removes after the listener fires, a long-lived signal (e.g., a parent-run controller shared across many triggerAndSubscribe calls) accumulates dead listeners that retain apiClient and response.id until the signal is GC'd.
Also note: throw new Error("Aborted") is not an idiomatic abort error — callers commonly check err.name === "AbortError". Consider using a DOMException("Aborted", "AbortError") (or matching whatever convention subscribeToRun propagates on abort).
🔒 Suggested cleanup pattern
- // Optionally cancel the child run when the abort signal fires (default: true)
- const cancelOnAbort = options?.cancelOnAbort !== false;
- if (options?.signal && cancelOnAbort) {
- const onAbort = () => {
- apiClient.cancelRun(response.id).catch(() => {});
- };
- if (options.signal.aborted) {
- await apiClient.cancelRun(response.id).catch(() => {});
- throw new Error("Aborted");
- }
- options.signal.addEventListener("abort", onAbort, { once: true });
- }
-
- for await (const run of apiClient.subscribeToRun(response.id, {
- closeOnComplete: true,
- signal: options?.signal,
- skipColumns: ["payload"],
- })) {
- ...
- }
-
- throw new Error(`Task ${id}: subscription ended without completion`);
+ // Optionally cancel the child run when the abort signal fires (default: true)
+ const cancelOnAbort = options?.cancelOnAbort !== false;
+ let onAbort: (() => void) | undefined;
+ if (options?.signal && cancelOnAbort) {
+ if (options.signal.aborted) {
+ await apiClient.cancelRun(response.id).catch(() => {});
+ throw new DOMException("Aborted", "AbortError");
+ }
+ onAbort = () => {
+ apiClient.cancelRun(response.id).catch(() => {});
+ };
+ options.signal.addEventListener("abort", onAbort, { once: true });
+ }
+
+ try {
+ for await (const run of apiClient.subscribeToRun(response.id, {
+ closeOnComplete: true,
+ signal: options?.signal,
+ skipColumns: ["payload"],
+ })) {
+ ...
+ }
+ throw new Error(`Task ${id}: subscription ended without completion`);
+ } finally {
+ if (onAbort) options!.signal!.removeEventListener("abort", onAbort);
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/trigger-sdk/src/v3/shared.ts` around lines 2594 - 2605, The abort
listener added when options.signal is present (using onAbort and
options.signal.addEventListener) is never removed on normal completion, causing
listener leaks and retained closures; update the flow in the trigger/subscribe
routine so that when the run finishes (success, failure, or cancel) you
explicitly remove the listener via options.signal.removeEventListener(onAbort)
(or register cleanup in a finally block) and then proceed to normal resolution;
also replace the non-idiomatic throw new Error("Aborted") with an abort-style
error (e.g., a DOMException("Aborted", "AbortError") or an Error with name
"AbortError") so callers can detect aborts consistently; ensure references to
apiClient.cancelRun(response.id) remain wrapped in the onAbort handler and that
removeEventListener runs before releasing references to apiClient/response.id.
| for await (const run of apiClient.subscribeToRun(response.id, { | ||
| closeOnComplete: true, | ||
| signal: options?.signal, | ||
| skipColumns: ["payload"], | ||
| })) { | ||
| if (run.isSuccess) { | ||
| // run.output from subscribeToRun is already deserialized | ||
| return { | ||
| ok: true as const, | ||
| id: response.id, | ||
| taskIdentifier: id as TIdentifier, | ||
| output: run.output as TOutput, | ||
| }; | ||
| } | ||
| if (run.isFailed || run.isCancelled) { | ||
| const error = new Error(run.error?.message ?? `Task ${id} failed (${run.status})`); | ||
| if (run.error?.name) error.name = run.error.name; | ||
|
|
||
| return { | ||
| ok: false as const, | ||
| id: response.id, | ||
| taskIdentifier: id as TIdentifier, | ||
| error, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) How does subscribeToRun shape run.output for "application/store" / serialized payloads?
fd -t f -e ts | xargs rg -nP -l 'subscribeToRun' 2>/dev/null
rg -nP -C8 '\bsubscribeToRun\s*\(' --type=ts -g '!**/node_modules/**'
rg -nP -C5 '\b(isSuccess|isFailed|isCancelled)\b' --type=ts -g 'packages/core/**'
# 2) How is run.output produced inside subscribeToRun (is parsePacket / conditionallyImportPacket called)?
rg -nP -C5 'application/store|conditionallyImportPacket|parsePacket' --type=ts -g 'packages/core/**'
# 3) Confirm createErrorTaskError contract used by triggerAndWait
ast-grep --pattern 'export function createErrorTaskError($_) {
$$$
}'Repository: triggerdotdev/trigger.dev
Length of output: 29769
🏁 Script executed:
# 1. Check how subscribeToRun constructs run.output in runStream.ts
sed -n '680,750p' packages/core/src/v3/apiClient/runStream.tsRepository: triggerdotdev/trigger.dev
Length of output: 2516
🏁 Script executed:
# 2. Check what happens with output/outputType in the run shape
rg -nP -A 3 'output.*outputType' packages/core/src/v3/apiClient/runStream.tsRepository: triggerdotdev/trigger.dev
Length of output: 287
🏁 Script executed:
# 3. Read handleTaskRunExecutionResult to see error handling
sed -n '2933,3000p' packages/trigger-sdk/src/v3/shared.tsRepository: triggerdotdev/trigger.dev
Length of output: 2074
🏁 Script executed:
# 4. Find createErrorTaskError function
rg -nP -B 2 -A 10 'export.*createErrorTaskError' packages/trigger-sdk/src/v3/Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Search for createErrorTaskError more broadly
rg -nP 'createErrorTaskError' packages/trigger-sdk/src/v3/shared.ts | head -20Repository: triggerdotdev/trigger.dev
Length of output: 153
🏁 Script executed:
# Find where createErrorTaskError is defined
rg -nP 'function createErrorTaskError|export.*createErrorTaskError' --type=tsRepository: triggerdotdev/trigger.dev
Length of output: 169
🏁 Script executed:
# Check the run object type returned from subscribeToRun in the code under review
sed -n '2600,2640p' packages/trigger-sdk/src/v3/shared.tsRepository: triggerdotdev/trigger.dev
Length of output: 1387
🏁 Script executed:
# Search for how the run object from apiClient.subscribeToRun is typed
rg -nP 'subscribeToRun.*:.*=' packages/core/src/v3/apiClient/index.ts -A 5 | head -30Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Get the full implementation of createErrorTaskError
sed -n '248,280p' packages/core/src/v3/errors.tsRepository: triggerdotdev/trigger.dev
Length of output: 880
🏁 Script executed:
# Check what TaskRunError is and what fields it has
rg -nP 'interface TaskRunError|type TaskRunError' packages/core/src/v3/ -A 8Repository: triggerdotdev/trigger.dev
Length of output: 1334
🏁 Script executed:
# Check if run.error in the subscribeToRun result is typed as TaskRunError
rg -nP 'error.*:.*TaskRunError|error\?' packages/core/src/v3/apiClient/runStream.ts -B 3 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 456
🏁 Script executed:
# Check the SerializedError type to understand what fields are available
rg -nP 'type SerializedError|interface SerializedError' packages/core/src/v3/ -A 8Repository: triggerdotdev/trigger.dev
Length of output: 707
🏁 Script executed:
# Verify what the run.error object contains when coming from subscribeToRun
sed -n '650,750p' packages/core/src/v3/apiClient/runStream.ts | grep -A 20 'createJsonErrorObject'Repository: triggerdotdev/trigger.dev
Length of output: 928
🏁 Script executed:
# Check if there are tests or examples showing the error behavior difference
rg -nP 'triggerAndSubscribe.*error|\.unwrap' packages/trigger-sdk/src/v3/ -B 2 -A 5 | head -50Repository: triggerdotdev/trigger.dev
Length of output: 2842
🏁 Script executed:
# Get the full SerializedError type definition
sed -n '540,560p' packages/core/src/v3/schemas/common.tsRepository: triggerdotdev/trigger.dev
Length of output: 524
🏁 Script executed:
# Verify the error type in subscribeToRun return - check RunShape definition
rg -nP 'type RunShape|interface RunShape' packages/core/src/v3/apiClient/runStream.ts -A 20 | head -40Repository: triggerdotdev/trigger.dev
Length of output: 1307
🏁 Script executed:
# Check the imports in shared.ts to see if createErrorTaskError is used
sed -n '1,50p' packages/trigger-sdk/src/v3/shared.ts | grep -i importRepository: triggerdotdev/trigger.dev
Length of output: 260
🏁 Script executed:
# Check what createJsonErrorObject does and how it relates to createErrorTaskError
sed -n '274,320p' packages/core/src/v3/errors.tsRepository: triggerdotdev/trigger.dev
Length of output: 1352
🏁 Script executed:
# Verify the flow: TaskRunError -> SerializedError in runStream.ts
sed -n '720,730p' packages/core/src/v3/apiClient/runStream.tsRepository: triggerdotdev/trigger.dev
Length of output: 590
🏁 Script executed:
# Check if there's an inverse function to createJsonErrorObject that reconstructs TaskRunError
rg -nP 'function.*Error.*|export.*Error.*' packages/core/src/v3/errors.ts | head -20Repository: triggerdotdev/trigger.dev
Length of output: 1277
🏁 Script executed:
# Check what skipColumns affects in subscribeToRun
rg -nP 'skipColumns' packages/core/src/v3/apiClient/runStream.ts -B 5 -A 5Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Double-check the condition for deserializing output in runStream.ts
sed -n '675,690p' packages/core/src/v3/apiClient/runStream.tsRepository: triggerdotdev/trigger.dev
Length of output: 620
🏁 Script executed:
# Verify if payload skip affects output processing
rg -nP 'payload.*skip|skip.*payload' packages/core/src/v3/apiClient/runStream.ts -B 3 -A 3Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Check the full context around output deserialization in runStream.ts
sed -n '668,720p' packages/core/src/v3/apiClient/runStream.tsRepository: triggerdotdev/trigger.dev
Length of output: 1611
🏁 Script executed:
# See what columns can be skipped in the API
rg -nP 'RealtimeRunSkipColumns|skipColumns' packages/core/src/v3/ -A 5 | head -40Repository: triggerdotdev/trigger.dev
Length of output: 2599
🏁 Script executed:
# Understand the error type mismatch - check what TaskRunError is vs SerializedError
sed -n '170,230p' packages/core/src/v3/schemas/common.tsRepository: triggerdotdev/trigger.dev
Length of output: 2149
🏁 Script executed:
# Final verification: confirm subscribeToRun returns SerializedError not TaskRunError
rg -nP 'error\?' packages/core/src/v3/apiClient/runStream.ts | grep -A 2 -B 2 'error'Repository: triggerdotdev/trigger.dev
Length of output: 125
🏁 Script executed:
# Check if there's any way to get TaskRunError from SerializedError (is there a reverse conversion?)
rg -nP 'SerializedError.*TaskRunError|function.*reconstruct.*error' packages/core/src/v3/Repository: triggerdotdev/trigger.dev
Length of output: 51
Error handling diverges from triggerAndWait — loss of error type information.
The error path (lines 2625–2627) creates a plain new Error() from run.error, which is a SerializedError (containing only message, name, and stackTrace). By contrast, triggerAndWait (line 2950) calls createErrorTaskError(execution.error) where execution.error is a full TaskRunError with type discriminators (BUILT_IN_ERROR, CUSTOM_ERROR, INTERNAL_ERROR, STRING_ERROR).
Since subscribeToRun returns SerializedError (created by createJsonErrorObject), the type information needed by createErrorTaskError is lost and cannot be reconstructed. This means error matching and .unwrap()-style consumers will behave inconsistently: triggerAndSubscribe always returns plain Error objects, while triggerAndWait reconstructs the original error types. Align error handling to use the same createErrorTaskError pathway or document the intentional divergence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/trigger-sdk/src/v3/shared.ts` around lines 2607 - 2631, The error
path in triggerAndSubscribe currently builds a plain Error from the
SerializedError returned by subscribeToRun, losing TaskRunError discriminators
and diverging from triggerAndWait's use of createErrorTaskError; update the
failure branch in triggerAndSubscribe to convert the SerializedError (run.error)
into the same TaskRunError shape expected by createErrorTaskError (or call
createErrorTaskError with a mapped TaskRunError) so you preserve
BUILT_IN_ERROR/CUSTOM_ERROR/INTERNAL_ERROR/STRING_ERROR discriminators and
stack/name/message; locate the failure handling inside the async loop over
apiClient.subscribeToRun and replace the new Error(...) construction with a call
to createErrorTaskError(mappedError) where mappedError is created from run.error
(SerializedError) following the same mapping logic used by
triggerAndWait/createJsonErrorObject.
| // Skip suspend if requested — return as if timed out | ||
| if (options.skipSuspend) { | ||
| span.setAttribute("wait.resolved", "skipped"); | ||
| return { ok: false as const, error: undefined }; | ||
| } |
There was a problem hiding this comment.
Return a concrete error from the skipSuspend branch.
wait() returns a real WaitpointTimeoutError on non-OK results, but this branch returns error: undefined. That gives waitWithIdleTimeout() a one-off failure shape and will break callers that do throw result.error or read result.error.message.
Suggested fix
// Skip suspend if requested — return as if timed out
if (options.skipSuspend) {
span.setAttribute("wait.resolved", "skipped");
- return { ok: false as const, error: undefined };
+ return {
+ ok: false as const,
+ error: new WaitpointTimeoutError("Suspension was skipped"),
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Skip suspend if requested — return as if timed out | |
| if (options.skipSuspend) { | |
| span.setAttribute("wait.resolved", "skipped"); | |
| return { ok: false as const, error: undefined }; | |
| } | |
| // Skip suspend if requested — return as if timed out | |
| if (options.skipSuspend) { | |
| span.setAttribute("wait.resolved", "skipped"); | |
| return { | |
| ok: false as const, | |
| error: new WaitpointTimeoutError("Suspension was skipped"), | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/trigger-sdk/src/v3/streams.ts` around lines 873 - 877, The
skipSuspend branch currently returns { ok: false, error: undefined } which
breaks callers expecting a concrete WaitpointTimeoutError; update that branch in
the wait() implementation to construct and return a real WaitpointTimeoutError
(or the same error type used elsewhere) — e.g., create new
WaitpointTimeoutError("suspend skipped" or similar), keep the
span.setAttribute("wait.resolved","skipped") and return { ok: false, error:
thatError } so waitWithIdleTimeout() and callers that do throw result.error or
access result.error.message receive a proper Error instance.
Adds Sessions, a durable, run-aware stream primitive that scopes session.in / session.out records to a session (not a single run). Records survive run boundaries; reconnect-from-last-event-id is built in. Server foundation: - New /realtime/v1/sessions/:session/:io/append + /records routes - sessionRunManager + sessionsRepository + clickhouseSessionsRepository - mintRunToken for short-lived per-session tokens - s2Append retry-with-backoff + undici cause diagnostics - /api/v[12]/packets/* exempt from customer rate limits - BackgroundWorker schema gains taskKind enum (TASK, AGENT, SCHEDULED) - TaskRun.taskKind column + clickhouse 029_add_task_kind_to_task_runs_v2 Core types: - new sessionStreams, inputStreams, realtimeStreams packages in @trigger.dev/core - session-streams-api / realtime-streams-api surface Sessions dashboard UI (the primitive's own viewer): - /sessions index + detail routes - SessionsTable, SessionFilters, SessionStatus, CloseSessionDialog - AGENT/SCHEDULED filter in RunFilters + TaskTriggerSource Includes the sessions-primitive changeset.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
3721c34 to
f240799
Compare
ericallam
changed the title
1 participant
Summary
A
/sessionsdashboard for inspecting durable Sessions, anAGENT/SCHEDULEDtask-kind filter for the runs list, and the server-side hardening (rate-limit exemption for packets, retry-with-backoff on stream appends, typed too-large-chunk error) that thechat.agentruntime in #3543 needs. Builds on the Sessions primitive shipped in #3417.Design
The Sessions list + detail routes mirror the run inspector pattern.
TaskTriggerSourcegainsAGENTandSCHEDULEDvalues, persisted onBackgroundWorker.taskKindandTaskRun.taskKind(plus a matching Clickhouse column), so the runs list can filter by kind.New
@trigger.dev/coremodules —sessionStreams,inputStreams, asessionStreamInstancefor realtime streams, and therealtime-streams-api/session-streams-apisurfaces — expose the typed shapes that chat.agent will use to drivesession.out.ChatChunkTooLargeErrorlets the runtime drop oversized chunks with a typed surface instead of failing the run.s2Appendretries transient failures with exponential backoff./api/v[12]/packets/*is exempt from customer rate limits so chat snapshot reads and writes don't get throttled under load.Test plan
/sessions, run a session via the SDK, verify it appears in the list with record counts.in/.outrecords renderAGENTkind, verify only agent-shaped runs appearStack
Part of a 4-PR stack. Merge bottom-up.
mainchat.agentruntime + browser transportReplaces #3173 (closed).