feat: Sessions dashboard, task_kind, and chat-ready hardening (1/4)#3542
Conversation
🦋 Changeset detectedLatest commit: be1a6cf 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 |
|
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 unit tests (beta)
|
b84d537 to
ed7bf97
Compare
ed7bf97 to
365e73b
Compare
365e73b to
b4a0986
Compare
1712b59 to
3721c34
Compare
3721c34 to
f240799
Compare
9a09da4 to
84c717c
Compare
…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.
…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.
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.
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.
…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.
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.
37ea386 to
32b0e42
Compare
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.
219e550 to
be1a6cf
Compare
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: