Conversation
🦋 Changeset detectedLatest commit: 75ec579 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (8)**Public packages** (`packages/*`): Use `build`.📄 CodeRabbit inference engine (AGENTS.md) Files:
If there are false positives, edit ./knip.json so that it passes.📄 CodeRabbit inference engine (AGENTS.md) Files:
Always import from `@trigger.dev/sdk`.📄 CodeRabbit inference engine (packages/trigger-sdk/CLAUDE.md) Files:
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use vitest for all tests in the Trigger.dev repository📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use function declarations instead of default exports📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
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 metr...📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc) Files:
🧠 Learnings (1)📚 Learning: 2026-08-16T18:36:58.179ZApplied to files:
WalkthroughThe SDK now persists stopped-turn boundaries, recovery input sequences, abandoned-turn state, and transcript-reload state. Stop and abort handling preserves successor output and limits state changes to the active session and stream. Message, action, steering, and reconnect operations remain blocked until valid transcript recovery. Priority: ➖ Normal 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 8 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Preserve indeterminate recovery state after a transient stream error. · chat.ts:2593-2595
packages/trigger-sdk/src/v3/chat.ts:2593-2595
🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve indeterminate recovery state after a transient stream error.
prepareTranscriptRecoverysetsisStreamingtoundefinedwhile accepted output can still be pending. A connection or read error then sets it tofalse. The nextreconnectToStreamcall returnsnull, so the client cannot recover that output.Keep
isStreamingundefined when a recovery stream fails before it receives a record. Set it to false only after a settled response or turn completion.Proposed fix
if (this.activeStreams.get(chatId) === internalAbort) { - state.isStreaming = false; + if (!(state.skipSettledPeek && state.isStreaming === undefined)) { + state.isStreaming = false; + } this.notifySessionChange(chatId, state); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: d00b5b24-65ec-4cde-a80c-1031a1b722a7
📒 Files selected for processing (7)
.changeset/chat-stop-successor-boundary.mdpackages/trigger-sdk/src/v3/chat-react.tspackages/trigger-sdk/src/v3/chat-stop.test.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/src/v3/chat.tspackages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/test/use-load-transcript.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/chat-stop-successor-boundary.md
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (38)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: internal / 🧪 Unit Tests: Internal (1)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: internal / 🧪 Unit Tests: Internal (2)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (7)
**Public packages** (`packages/*`): Use `build`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/src/v3/chat-react.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/test/use-load-transcript.test.tspackages/trigger-sdk/src/v3/chat.tspackages/trigger-sdk/src/v3/chat-stop.test.ts
Always import from `@trigger.dev/sdk`.
📄 CodeRabbit inference engine (packages/trigger-sdk/CLAUDE.md)
Files:
packages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/src/v3/chat-react.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/test/use-load-transcript.test.tspackages/trigger-sdk/src/v3/chat.tspackages/trigger-sdk/src/v3/chat-stop.test.ts
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/src/v3/chat-react.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/test/use-load-transcript.test.tspackages/trigger-sdk/src/v3/chat.tspackages/trigger-sdk/src/v3/chat-stop.test.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/test/use-load-transcript.test.tspackages/trigger-sdk/src/v3/chat-stop.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/src/v3/chat-react.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/test/use-load-transcript.test.tspackages/trigger-sdk/src/v3/chat.tspackages/trigger-sdk/src/v3/chat-stop.test.ts
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/src/v3/chat-react.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/test/use-load-transcript.test.tspackages/trigger-sdk/src/v3/chat.tspackages/trigger-sdk/src/v3/chat-stop.test.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 metr...
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/trigger-sdk/test/chat-turn-correlation.test.tspackages/trigger-sdk/src/v3/chat-react.tspackages/trigger-sdk/src/v3/chat.test.tspackages/trigger-sdk/test/use-load-transcript.test.tspackages/trigger-sdk/src/v3/chat.tspackages/trigger-sdk/src/v3/chat-stop.test.ts
🧠 Learnings (1)
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
packages/trigger-sdk/test/use-load-transcript.test.ts
🔇 Additional comments (5)
packages/trigger-sdk/test/chat-turn-correlation.test.ts (1)
123-124: LGTM!packages/trigger-sdk/src/v3/chat.test.ts (1)
1441-1444: LGTM!packages/trigger-sdk/src/v3/chat-stop.test.ts (1)
59-70: LGTM!Also applies to: 77-77, 82-83, 112-112, 117-118, 137-156, 199-216, 266-288, 290-303, 305-318, 320-339, 511-522, 524-553, 555-580, 582-608, 610-636, 638-652, 654-680
packages/trigger-sdk/test/use-load-transcript.test.ts (1)
2-2: LGTM!Also applies to: 59-172
packages/trigger-sdk/src/v3/chat-react.ts (1)
68-69: LGTM!Also applies to: 82-83, 88-89, 92-92, 138-138, 147-157, 175-175
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if ( | ||
| !state.outstandingTurnAbandoned && | ||
| (state.isStreaming || state.activeInputSeq !== undefined) | ||
| (state.isStreaming !== false || state.activeInputSeq !== undefined) |
There was a problem hiding this comment.
🟡 Idle hydrated chats arm stop gate
When hydrated state omits isStreaming, stopGeneration treats an idle chat as active and arms the discard gate. The next send can lose its entire response because no stopped turn boundary exists.
Learn more
isStreaming is optional persisted state, so undefined also occurs for ordinary token-only hydration. This condition now interprets every such session as an outstanding turn. A Stop on an idle session therefore arms skipToTurnComplete, but the output stream has no old turn-complete record to clear it. The next turn's records are discarded until its own boundary, where the transport raises the output-lost error.
Example: Hydrate { publicAccessToken: "pat" }, call stopGeneration("chat") while the session is idle, then send a message. The server emits only the new answer and its completion. The transport discards the answer and rejects the stream instead of rendering it.
Recommended fix: Preserve a distinct marker for an unknown resumed turn rather than inferring activity from absent isStreaming. Arm the boundary only for known active sends, active subscriptions, or that explicit unknown-turn marker, while retaining idle token-only session behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if ( | ||
| !state.outstandingTurnAbandoned && | ||
| (state.isStreaming || state.activeInputSeq !== undefined) | ||
| (state.isStreaming !== false || state.activeInputSeq !== undefined) | ||
| ) { | ||
| state.skipToTurnComplete = true; | ||
| state.supersededInputSeq = state.activeInputSeq; | ||
| this.armStoppedBoundary(state); | ||
| } | ||
| const stoppedBoundary = state.skipToTurnComplete | ||
| ? (state.stoppedBoundary ??= Symbol("stopped-boundary")) | ||
| : undefined; |
There was a problem hiding this comment.
🟡 Stopped boundary is lost across tabs
After one tab stops, stopGeneration keeps the discard boundary only in that tab. ChatTabCoordinator shares only cursors, so another tab can claim the chat and render stopped output.
Learn more
Each browser tab owns a separate TriggerChatTransport and session map. stopGeneration arms the stopped-output boundary locally, then releases the chat claim. The session message carries only lastEventId, so another tab never receives skipToTurnComplete, supersededInputSeq, or the reload guard. That tab can claim the released chat and subscribe from its old cursor without filtering the stopped turn's remaining output.
Example: Tab A and Tab B both open chat c1 with multiTab: true. Tab A receives part of a tool call, stops, and releases its claim. Tab B sends the next prompt. Its local session has no stopped boundary, so the old tool output is delivered as part of the new response.
Recommended fix: Broadcast the persisted stopped-boundary state before releasing the claim, and merge it into peer sessions. Include every field needed to preserve the gate and transcript-recovery guard, not only lastEventId. Add a two-transport regression test covering stop in one tab followed by send in the other.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ); | ||
| } | ||
|
|
||
| describe("Stop with a successor response", () => { |
There was a problem hiding this comment.
🔍 Mock-heavy tests conflict with guidance
The new suites simulate HTTP and transcript loading despite the repository’s no-mocks rule. Review whether integration coverage or an exception is required.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Stop acknowledgments can close a later response, and lost Stop state can replay old tool output after a reload.
The SDK now records Stop before the request and retains stopped and abandoned state through hydration.
Accepted responses without sequence correlation block further sends until
useLoadTranscriptloads a saved transcript that covers the stopped input.✅ Checklist
Testing
RedisCacheStoreConfig, in unchangedinternal-packages/cache/src/stores/redis.ts:7Changelog
New chat responses remain intact after Stop, including slow acknowledgments and page reloads.
Transcript recovery reports missing evidence and empty output polls. An empty poll permits reconnect without another input append.
The internal state shares the persisted type and one field projection.
The recovery callback accepts one cursor object.
seedTranscriptCursorretains its three-argument API.Risk
lastInEventIdandlastOutEventIdfor automatic recovery. Missing evidence reports an error and keeps sends blocked.