Fix/pr 3542 sessions#3557
Conversation
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.
- Fix early-return path when both queue override and TTL are provided - Fix no-worker-found fallback to query taskKind - Fix no-task-found fallback to include taskKind This ensures AGENT/SCHEDULED runs are correctly classified in ClickHouse.
🦋 Changeset detectedLatest commit: 1ee2dd8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 29 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi @deepshekhardas, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (88)
WalkthroughIntroduces Sessions to the dashboard and SDK: adds session list and inspector routes, close-session action, SSE endpoints for session channels, and server/session presenters. Adds SDK session stream managers and S2-only writers, enhances SSE retry/timeout logic, and extends input/session stream APIs. Propagates taskKind across schema, presenters, repositories, UI, and ClickHouse, with migrations. Adds run source filters and icons, playground models, rate-limit exemptions, JWT minting, and extensive tests. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 STANDARD case in TaskTriggerSourceIcon ignores the className prop, causing oversized icons in the runs table
The STANDARD branch at TaskTriggerSource.tsx:14-15 renders <TaskIconSmall className="size-[1.125rem] ..." /> with a hardcoded className, ignoring the className prop entirely. The SCHEDULED and AGENT branches use cn(...) to merge the prop. The new call site in TaskRunsTable.tsx:357-360 passes className="size-3.5 flex-none" expecting uniform sizing across all source types — but for STANDARD tasks (the majority), the icon renders at 18px (size-[1.125rem]) instead of 14px (size-3.5) and lacks flex-none, causing a visible size mismatch in the task column.
(Refers to lines 14-15)
Was this helpful? React with 👍 or 👎 to provide feedback.
| // When caller provides both a queue override and a per-trigger TTL, | ||
| // we still need to fetch taskKind to correctly classify AGENT/SCHEDULED runs | ||
| if (overriddenQueueName && body.options?.ttl !== undefined) { | ||
| const worker = await findCurrentWorkerFromEnvironment(environment, this.prisma); | ||
| if (worker) { | ||
| const task = await this.replicaPrisma.backgroundWorkerTask.findFirst({ | ||
| where: { | ||
| workerId: worker.id, | ||
| runtimeEnvironmentId: environment.id, | ||
| slug: taskId, | ||
| }, | ||
| select: { triggerSource: true }, | ||
| }); | ||
| return { queueName: overriddenQueueName, taskTtl: undefined, taskKind: task?.triggerSource }; | ||
| } | ||
| return { queueName: overriddenQueueName, taskTtl: undefined }; |
There was a problem hiding this comment.
🔴 New DB queries added to previously zero-query hot path in queue concern, violating CLAUDE.md trigger-path guidance
The apps/webapp/CLAUDE.md explicitly states: "Piggyback on the existing query instead of adding new ones" for the queue concern, and "Do NOT add database queries" to the trigger hot path. The overriddenQueueName && body.options?.ttl !== undefined branch at queues.server.ts:218-231 previously returned immediately with zero database queries (a deliberate optimization). It now calls findCurrentWorkerFromEnvironment (1 query) plus backgroundWorkerTask.findFirst (1 query) — adding up to 2 new DB round-trips. Similarly, the !worker fallback at lines 237-255 adds a new backgroundWorkerTask.findFirst query where none existed before. These queries execute on every trigger call that matches these conditions.
Prompt for agents
The trigger hot path guidance in apps/webapp/CLAUDE.md says to piggyback on existing queries rather than adding new ones. The taskKind/triggerSource information could instead be resolved at dequeue time (like retry config, machine config, maxDuration — which are all resolved via the full BackgroundWorkerTask load in dequeueSystem.ts). Alternatively, if trigger-time resolution is needed, the taskKind could be passed in via the TriggerTaskRequestBody options from the SDK side (the SDK already knows whether it's an agent task at define-time), avoiding any new server-side queries. A third option: since the branch at line 218 only fires when both overriddenQueueName AND ttl are explicitly provided by the caller, and agents typically set triggerSource via the SDK task definition, the caller could also supply taskKind in the trigger request options.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const task = await this.replicaPrisma.backgroundWorkerTask.findFirst({ | ||
| where: { | ||
| runtimeEnvironmentId: environment.id, | ||
| slug: taskId, | ||
| }, | ||
| select: { triggerSource: true }, | ||
| }); | ||
|
|
||
| return { | ||
| queueName: overriddenQueueName ?? defaultQueueName, | ||
| taskTtl: undefined, | ||
| taskKind: task?.triggerSource, | ||
| }; |
There was a problem hiding this comment.
🚩 no-worker fallback in getTaskQueueInfo queries backgroundWorkerTask without workerId constraint
At queues.server.ts:243-249, when no current worker is found, the code queries backgroundWorkerTask filtered only by runtimeEnvironmentId and slug (no workerId). Without orderBy, findFirst returns an arbitrary row — potentially from an old worker version. In practice triggerSource is stable across versions for a given task slug (a task doesn't change from STANDARD to AGENT between deployments without a code change), so this is unlikely to return a wrong value. Still, adding orderBy: { createdAt: 'desc' } would make the query deterministic.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Closes #
✅ Checklist
Testing
[Describe the steps you took to test this change]
Changelog
[Short description of what has changed]
Screenshots
[Screenshots]
💯