test(webapp): chat.agent durability regression suite - #4550
Conversation
A chat/session belongs to one (org, user) pair. Pin that every store read — getChatMessages, getSession, chatExists, listChats, countUserMessages — and appendChatMessageOnce refuse a foreign tenant, so a chatId from another org reads as not-found and never leaks a transcript or the session's public access token. TRI-11166.
The primitive resumes a turn by replaying its snapshot; pin the store seam that replay lands on: a streamed-then-resumed turn is not double-appended, a crash mid-turn keeps the mid-turn append and rebuilds the session cursor, a failed write commits nothing and the retry replays with no loss, and an OOM restart replays idempotently. TRI-11166.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
…o test/chat-agent-durability-tri-11166
@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: |
…o test/chat-agent-durability-tri-11166
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // The next turn's write fails partway — a malformed message with no id throws inside the | ||
| // transaction, after the (would-be) settlement/message work has begun. | ||
| await expect( | ||
| persistTurn(agentDb, { | ||
| chatId, | ||
| messages: [ | ||
| textMessage("u1", "user"), | ||
| textMessage("a1"), | ||
| textMessage("a2"), | ||
| { role: "assistant", parts: [] } as unknown as { id: string; role: string }, | ||
| ], | ||
| session: { publicAccessToken: "pat_torn", lastEventId: "2", runId: "run_torn" }, | ||
| }) | ||
| ).rejects.toThrow(/handed a message with no id/); | ||
|
|
||
| // The whole turn rolled back: no new rows, allocator untouched, and — the version- | ||
| // mismatch case — the session cursor is still the first turn's, not the torn one's. | ||
| expect((await transcript(chatId)).map((m) => m.id)).toEqual(["u1", "a1"]); | ||
| expect(await nextPosition(prisma, chatId)).toBe(positionBefore); | ||
| expect( | ||
| await getSession(agentDb, { chatId, organizationId: ORG, userId: USER }) | ||
| ).toMatchObject({ publicAccessToken: "pat1", lastEventId: "1" }); |
There was a problem hiding this comment.
🔍 The "write fails partway" test never actually reaches a write, so it can't detect a non-atomic persistTurn
The comment claims the malformed message throws "inside the transaction, after the (would-be) settlement/message work has begun", but persistTurn (internal-packages/dashboard-agent-db/src/queries.ts:592-680) runs the settlements loop (empty here, since no settlements are passed) and then calls storeChatMessages, whose very first action is the dedup loop that calls messageIdOf on each message (internal-packages/dashboard-agent-db/src/queries.ts:386-399). The throw therefore happens before the SELECT ... FOR UPDATE, before any position reservation, and before the chat_sessions upsert. The subsequent assertions (no new rows, allocator untouched, session still pat1) would pass even if persistTurn were not transactional at all, so this case does not pin the rollback/atomicity property the PR body claims ("a persistTurn that throws commits nothing"). To actually exercise rollback, the failure needs to occur after some write has landed — e.g. a valid message batch plus a settlement whose state isn't renderable (the throw new Error("Investigation ... settled to a state that isn't renderable") path), or a finalisation with a mismatched role, so message rows/positions are written first and then rolled back.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
1 participant
What & why
Test-only hardening for chat.agent durability. chat.agent gets its durability from the primitive — object-store snapshot + S2
.in/.outreplay + continuation boot — but several store-level mechanisms that replay lands on had no regression test, and two of them were criticals in the 2026-06-10 chat.agent audit: cross-tenant isolation and no duplicate mid-stream turn. This adds those cases against a real Postgres table (testcontainers, no mocks). TRI-11166.Stack
Stacked on #4549 (query boundary). Merge that first.
What's inside
dashboardAgentTenantIsolation.test.tsdashboardAgentDurableResume.test.tsdashboardAgentDurableResume.test.tsdashboardAgentDurableResume.test.tspackages/trigger-sdk/src/v3/chat.test.ts— not duplicateddashboardAgentDurableResume.test.ts.outtrimming / OOM retry restarts cleanlydashboardAgentDurableResume.test.ts.outtrimming and the OOM restart itself are inside the closed primitive (not reachable)The "Full" vs "Seam-only" column is the honest distinction: full means the whole mechanism is exercised from the repos we own; seam-only means we pin the store contract the primitive depends on, and the primitive-internal half lives where we can't reach it.
Key decisions
organizationIdfilter fromgetChatMessagesleaks org A's transcript to the owner's user id under another org — the test fails attoBeNull(). No-duplicate: removing the stored-id drop instoreChatMessagesmakes a replayed persistTurn over-reserve positions (next free slot jumps 3 → 7) — the test fails on the allocator assertion. Both reverted.Residual follow-ups
These live inside the closed chat.agent primitive package and can't be unit-tested from the repos we own; the tests above are the store-level backstop they depend on:
.in/.outreplay at the transport level..outtrimming never dropping in-flight data, and the OOM restart mechanism itself.Testing
pnpm run test --filter webapp—dashboardAgentTenantIsolation.test.tsanddashboardAgentDurableResume.test.ts.