feat: Sessions dashboard, task_kind, and chat-ready hardening (1/4)#3542
Open
ericallam wants to merge 9 commits into
Open
feat: Sessions dashboard, task_kind, and chat-ready hardening (1/4)#3542ericallam wants to merge 9 commits into
ericallam wants to merge 9 commits into
Conversation
🦋 Changeset detectedLatest commit: 37ea386 The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 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 |
Contributor
|
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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
b84d537 to
ed7bf97
Compare
Contributor
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.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
ed7bf97 to
365e73b
Compare
ericallam
changed the title
This was referenced May 12, 2026
Contributor
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.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
365e73b to
b4a0986
Compare
Contributor
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.
ericallam
force-pushed
the
feature/sessions-primitive
branch
2 times, most recently
from
1712b59 to
3721c34
Compare
Contributor
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.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
3721c34 to
f240799
Compare
ericallam
changed the title
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.
Session.triggerConfig.basePayload is the boot payload for every Run
triggered by ensureRunForSession / swapSessionRun. When a customer's
first-run payload includes `message` / `messages` / `trigger:
"submit-message"` (the chat.createStartSessionAction({ basePayload:
{ message } }) pattern designed to boot the FIRST run straight into a
turn), those fields are sticky — every continuation run replays them
and produces a phantom turn re-processing the original first message.
Continuation overrides now clear those three fields so the new run
boots clean. ensureRunForSession strips on continuation (prior run
dead); swapSessionRun strips unconditionally (every swap is a
deliberate handoff). Pairs with the SDK's new continuation-wait boot
branch — together, the new run waits silently on session.in for the
next user message instead of replaying stale basePayload.
… shape
The auth-consolidation commit on main rewrote createLoaderApiRoute's
authorization contract:
- `resource` returns a typed RbacResource ({ type, id }) instead of an
untyped record ({ deployments: 'current' } / { sessions: [...] })
- multi-resource auth uses `anyResource(...)` instead of an array literal
- `superScopes` is no longer a field — super-scopes are resolved at the
JWT ability layer
- the `findResource` resolver must return `T | undefined`, not `T | null`
Our Sessions PR added two routes (api.v1.deployments.current.ts and
realtime.v1.sessions.$session.$io.records.ts) using the pre-consolidation
shape. The rebase had no textual conflict because the files didn't exist
on main, but typecheck fails because the new contract doesn't accept the
old shape. Migrate both routes to match.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
9a09da4 to
84c717c
Compare
2 tasks
matt-aitken
pushed a commit
that referenced
this pull request
May 12, 2026
…mpling (#3567) ## Summary Follow-up to #3561. The drift-audit workflow timed out on PR #3542 (92 files, +5962 lines) by hitting `--max-turns 15` before reaching a verdict, leaving a red ❌ on that PR with no sticky comment. ## Changes - `--max-turns` bumped from 15 to 30. - Prompt now opens with an explicit "Strategy" section: read REVIEW.md once, scan the file-list only, open at most 5 files (3-5 on PRs >50 files), and bias toward finishing over exploring. - Final rule: *"when in doubt between one more file read and finish now — finish now."* The audit is allowed to miss things. It is not allowed to time out and leave a red X. ## Test plan - [ ] Verify this PR's audit posts `✅ REVIEW.md looks current for this PR.` (small diff) - [ ] After merge, retry the audit on #3542 or a similarly large PR and confirm it completes
Four fixes from the #3542 review pass. webapp/runEngine/queues.server.ts — non-locked-worker path of getTaskQueueInfo skipped the task lookup when the caller provided both a queue override and a per-trigger TTL, leaving `taskKind` undefined. AGENT/SCHEDULED runs hitting this path got stamped as STANDARD in ClickHouse and disappeared from the dashboard's Source filter. Mirrors the locked-worker fix above (always fetch triggerSource). webapp/presenters/v3/SessionListPresenter.server.ts — current-run lookup wasn't scoped to projectId + runtimeEnvironmentId. Session .currentRunId has no FK, so a stale or corrupted pointer could surface another tenant's run. The list query is env-scoped; this adds the same fence to the run lookup. webapp/services/realtime/sessionRunManager.server.ts — after a lost claim race, the post-reload probe of fresh.currentRunId went through getRunStatusAndFriendlyId which reads from $replica. The replica can lag behind the writer the winner just wrote to, miss the live run, and force another trigger+recurse up to ENSURE_RUN_FOR_SESSION_MAX_ATTEMPTS. Probe the writer for the same read-after-write reason the fresh reload already used. trigger-sdk/v3/shared.ts — triggerAndSubscribe leaked the abort listener on normal completion. `{ once: true }` only auto-removes after firing; long-lived signals shared across many calls accumulated dead listeners pinning apiClient + response.id until GC. Wrap the subscribe loop in try/finally and removeEventListener on every exit path. Also switched the synchronous-pre-aborted throw to a DOMException with name AbortError so callers can detect the abort with the standard err.name === 'AbortError' check.
shared.ts — triggerAndSubscribe was missing the requestOptions
parameter that every sibling task-trigger API (trigger, triggerAndWait,
batchTrigger) accepts. Without it callers couldn't pass clientConfig,
forcing the internal to hardcode apiClientManager.clientOrThrow() with
no override. Added requestOptions?: TriggerApiRequestOptions to both
the public and internal signatures and threaded clientConfig through
to clientOrThrow.
streams.ts — the skipSuspend branch of waitWithIdleTimeout returned
{ ok: false, error: undefined } instead of a WaitpointTimeoutError, so
the result shape didn't match the cold-phase wait() path below.
Callers that did `throw result.error` would have thrown undefined.
Now returns a real WaitpointTimeoutError with a descriptive message.
changeset — adds .changeset/sessions-primitive.md for the new public
SDK surface (Sessions primitive + tasks.triggerAndSubscribe + chat.agent
on top). Marked as minor on @trigger.dev/sdk.
ericallam
added a commit
that referenced
this pull request
May 13, 2026
…ign-note divergence Three more #3542 review fixes addressing the design-question bucket. sessionStreams/manager.ts + inputStreams/manager.ts — both #runTail loops swallowed errors and the .finally reconnected immediately whenever hasHandlers || hasWaiters. A persistent backend failure (auth rejection, 5xx, DNS) would reconnect in a tight loop with no rate limiting. Both managers now exponentially back off: 1s base, doubling per attempt, capped at 30s, plus 0–1s jitter. A reconnectAttempts counter resets to 0 on every successful #dispatch (any record flowing through = healthy connection), so transient blips don't accumulate. Per-waiter timeouts still bound how long any once() waits regardless. realtimeStreams/streamsWriterV2.ts + .test.ts — extracted the size-check + discriminant-extraction logic into encodeChunkOrError, a pure helper. Tests now exercise it directly, no `vi.mock("@s2-dev/ streamstore")` shim. The original vi.mock conflicted with the codebase rule of using testcontainers / not mocking; the new tests are framework-pure and faster. trigger-sdk/v3/shared.ts — added an in-code comment in triggerAndSubscribe explaining the error-shape divergence from triggerAndWait. The SerializedError surfaced by subscribeToRun strips the TaskRunError type discriminator at the server boundary (createJsonErrorObject in errors.ts:274), so the SDK can't reconstruct the discriminator on the receive side. Callers needing exact error-type matching should use triggerAndWait.
…ign-note divergence Three more #3542 review fixes addressing the design-question bucket. sessionStreams/manager.ts + inputStreams/manager.ts — both #runTail loops swallowed errors and the .finally reconnected immediately whenever hasHandlers || hasWaiters. A persistent backend failure (auth rejection, 5xx, DNS) would reconnect in a tight loop with no rate limiting. Both managers now exponentially back off: 1s base, doubling per attempt, capped at 30s, plus 0–1s jitter. A reconnectAttempts counter resets to 0 on every successful #dispatch (any record flowing through = healthy connection), so transient blips don't accumulate. Per-waiter timeouts still bound how long any once() waits regardless. realtimeStreams/streamsWriterV2.ts + .test.ts — extracted the size-check + discriminant-extraction logic into encodeChunkOrError, a pure helper. Tests now exercise it directly, no `vi.mock("@s2-dev/ streamstore")` shim. The original vi.mock conflicted with the codebase rule of using testcontainers / not mocking; the new tests are framework-pure and faster. trigger-sdk/v3/shared.ts — added an in-code comment in triggerAndSubscribe explaining the error-shape divergence from triggerAndWait. The SerializedError surfaced by subscribeToRun strips the TaskRunError type discriminator at the server boundary (createJsonErrorObject in errors.ts:274), so the SDK can't reconstruct the discriminator on the receive side. Callers needing exact error-type matching should use triggerAndWait.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
2218110 to
5359eda
Compare
Four follow-up nits from the second-pass review on #3542. .server-changes/sessions-dashboard-and-task-source-filter.md — adds the missing high-level entry for the webapp surface (Sessions page + task-source filter on Runs). The two existing changesets only cover @trigger.dev/sdk and @trigger.dev/core, so the dashboard work wouldn't have shown up in a future server changelog. apps/webapp/app/routes/realtime.v1.sessions.$session.$io.records.ts — switched `const loader = ...; export { loader }` to `export const loader = ...` to match the sibling `api.v1.deployments.current.ts` and the rest of the route file convention. Functionally identical. packages/core/src/v3/sessionStreams/manager.ts + packages/core/src/v3/inputStreams/manager.ts — two clarifications: (1) added a JSDoc to `disconnect()` documenting that it intentionally leaves handlers and waiters in place, so any registered listener will trigger an auto-reconnect with backoff. Distinguishes from `reset()` (full clean state, rejects waiters) and `disconnectStream` (single key, stays down until fresh `on()`/`once()`). (2) `disconnectStream` now clears `reconnectAttempts` for the key — an explicit teardown is not evidence of a broken backend, and a future re-attach should start the backoff at attempt 0.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).
This is part 5 of 5 in a stack made with GitButler: