[fix] Waiting cards refuse transcript adoption without proof they ended - #5942
Conversation
dosubot
Bot
added
the
size:M
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughSession loading now carries interaction-row states into hydration. Transcript adoption uses terminal and pending row states to settle or retain local client-tool cards. Regression tests cover replay and no-growth cases. ChangesSession hydration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant adoptServerTranscript
participant loadSession
participant shouldAdoptTranscript
adoptServerTranscript->>loadSession: Load SessionTranscript
loadSession-->>adoptServerTranscript: Return messages and interactionRows
adoptServerTranscript->>shouldAdoptTranscript: Evaluate transcript adoption
shouldAdoptTranscript-->>adoptServerTranscript: Adopt or retain local cards
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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}
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 (1)
web/oss/src/components/AgentChatSlice/assets/loadSession.ts (1)
61-70: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLoad interaction rows for the refreshed record snapshot.
Line 64 builds refreshed messages with
interactionRowStatescaptured beforerefreshedresolves. Line 69 forwards that stale state to adoption.If a row changes from
pendingto terminal during record revalidation, the refreshed transcript can still block adoption of the settled card. Fetch a matching current row snapshot forfresh, or return records and interaction rows as one refreshed snapshot. Add a race regression test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 575027f9-d6f1-4d93-8caf-0e79de20bb21
📒 Files selected for processing (3)
web/oss/src/components/AgentChatSlice/assets/loadSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.test.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.ts
Railway Preview Environment
|
1 participant
Context
After #5919, the user-request form card in the agent chat kept refreshing while the user typed, and the typed input was lost. The chain: the browser now re-reads the conversation from the server the instant a card parks (the new interaction-event listener). The guard that should protect a waiting card during that re-read counted any server part it could not recognize as "settled". But the server's replay of a freshly parked card is built from the durable record's harness-wrapped tool name (
mcp.agenta-tools.request_input) with no render marker, so the guard failed to recognize it, wrongly concluded the card had ended, and let the server copy replace the live chat. The replacement carries different internal ids, so React remounted the form widget and destroyed the draft. A background poll repeated this.Changes
settledLocallyWaitingIdsnow demands positive proof that a waiting card ended before it counts as settled: the server part sits in a terminal state (output-available,output-error,output-denied), or the card's interaction row has moved pastpending(responded,resolved,cancelled). A part that is present but merely unrecognized now refuses adoption, exactly as an absent part always did.The no-growth adoption path (which retires zombie cards in dead sessions) additionally refuses while any waiting card's row still reads
pending. In a dead session the row is terminal, so that path keeps working; for a freshly parked live card the row ispending, so the card can no longer be swept away.To make row evidence available,
SessionTranscriptnow carries the interaction rows the transcript was replayed against (loadSession.tsalready fetched them; it stopped discarding them). Rows and messages always come from the same fetch, so the two kinds of evidence cannot disagree across generations.Before: a parked form card was replaced seconds after appearing, and typing was lost.
After: the card is replaced only when the server positively shows it ended.
Tests
useSessionHydration.test.ts, derived from the realarabicPoetrySessiongolden records cut at the exact record boundary that reproduces the unrecognizable replay. With the old guard logic, the three refusal tests fail; with the new logic all 26 pass.cancelledrow.@agenta/entities: 974 passed. Mobile transcript adoption: 6 passed.cancelled, fetch-failure degradation): no confirmed defects.What to QA