Repository navigation
fix(frontend): render a session that is parked on a tool approval - #6202
Merged
Merged
Conversation
Opening a session that waits on an approval showed an empty pane: the agent card, the Templates grid and the composer, but none of the session's turns and no Approve/Deny controls. The backend returned the turns and the pending user_approval row the whole time. A user was told something waited on them and had no way to act on it. The transcript was mapped correctly and then discarded. #5942 added an `awaitingUser` input to `shouldAdoptServerTranscript` so a background poll cannot adopt the durable log over a card the user is still filling in. The guard was applied unconditionally, including on a COLD open, where `localMessageCount` is 0. There is no local card to protect there, so refusing the server transcript protects nothing and leaves the pane blank. Nothing else calls `setMessages` on that path, and `hydratedEmpty` stays false because the transcript was not empty, so the user does not even get the history-unavailable notice. Pair the guard with `localMessageCount` so it only fires when there is a rendered transcript to preserve. Every background and revalidation path runs with one, so #5942 keeps its protection. Regression in v0.113.0. v0.112.3 has no `awaitingUser` input at all; it came in with c08a824, merged in PR #6065. The @agenta/chat copy of the hook never passed `awaitingUser`, so mobile is unaffected. Tests: 4 cases in transcriptAdoptionAwaiting.test.ts driving a pending user_approval row through hasWaitingInteraction into the guard. The cold-open case fails without this change and the #5942 case stays green in both directions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe transcript adoption guard now allows empty clients to load durable server transcripts when a pending approval exists. Clients with rendered transcript content still refuse adoption. Unit tests cover both cases and related safeguards. ChangesTranscript adoption
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This restores cold-open rendering for sessions waiting on tool approval while preserving protection for approvals already visible on screen; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
Contributor
Railway Preview Environment
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Opening a session that is waiting on a tool approval showed an empty pane. You got the agent card, the Templates grid and the composer, but none of the session's turns and no Approve/Deny controls. This happened from the sessions list and from Home's "Waiting on you" entry. QA reproduced it 3/3 on staging; the five sessions on the same account without a pending approval rendered normally in the same pane.
The backend was never the problem.
POST /api/sessions/interactions/queryreturned the turns and the pendinguser_approvalrow the whole time. The transcript was mapped correctly and then thrown away.#5942 added an
awaitingUserinput toshouldAdoptServerTranscript. Its job is to stop a background poll from adopting the durable log over a card the user is still filling in, which would discard whatever they typed into its form. That intent is right. The problem is that the guard ran on every path, including a cold open, wherelocalMessageCountis 0. There is no card on screen to protect there, so refusing the server transcript protects nothing and leaves the pane blank. Nothing else callssetMessageson that path.hydratedEmptyalso stays false, because the transcript was not empty, so the user does not even get the "history unavailable" notice. The Templates grid only renders whenmessages.length === 0, which is exactly the reported symptom.That is also why the split QA saw is 1 versus 5. All six sessions took the same cold-hydration path. The five without a pending interaction had
awaitingUser: falseand were adopted. The one parked on an approval was refused.Changes
shouldAdoptServerTranscriptnow pairs the guard withlocalMessageCount, so it only fires when there is a rendered transcript to preserve.Before:
After:
Every background and revalidation path runs with a rendered transcript, so #5942 keeps its protection. The only case that changes is the one that was broken: a first paint with nothing on screen.
Regression or pre-existing
This is a regression in v0.113.0. v0.112.3 has no
awaitingUserinput at all:It arrived in commit
c08a8248bc("rebuild #5942's adoption guard in the lane's idiom"), merged in PR #6065. The v0.112.3 release solved #5942 with different machinery in the app hook. The rebuild moved the rule into the shared predicate and lost the "only when we render something" precondition.Blast radius is the desktop OSS and EE chat slice only. The
@agenta/chatcopy of the hook (useAgentConversation.ts) never passedawaitingUser, so mobile was never affected.Tests
Four cases added to
transcriptAdoptionAwaiting.test.ts. They drive a real pendinguser_approvalrow throughhasWaitingInteractioninto the guard, so the wiring is covered, not just the predicate.expected false to be true) and passes with it.localMessageCount: 0did not turn the guard into a blanket "always adopt":busy, an empty server transcript, and a watermark that has not moved all still refuse.Verified:
@agenta/entities1334 unit tests pass,@agenta/chat378 pass,types:checkclean for both@agenta/ossand@agenta/ee, prettier 3.7.4 clean, and one production build viapnpm build-oss(10/10 tasks).Not verified live. Reproducing a pending approval needs a stack, and every local stack is in use by another release task. The click path below is the handoff.
Why it can look intermittent
It is not intermittent. It is deterministic for the first open of a session on a device, and it disappears the moment anything caches that session locally.
QA hit the empty pane 3/3 on cold opens. They then opened the same session on
/mand went back to the desktop, where it rendered 7/7. That is the fix working by accident, not the bug going away. The mobile app drives the same session throughuseAgentConversation(mobile/src/features/chat/LiveConversation.tsx:71), and that hook never passesawaitingUserinto the guard. So/madopted the transcript and persisted it. Both apps read and write the same store,agenta:agent-chat:messages:v2, declared once inpackages/agenta-chat/src/state/sessionMessages.ts:52./mis abasePathon the same deployment, not a separate host, so it is the same origin and the samelocalStoragepartition. After that write, every desktop open of that session takes the warm path:initialMessagesis non-empty, the pane paints from cache, and the guard is never the thing standing between the user and their turns.One detail if you try to reproduce the sequence.
tabLocalStoragedeletes jotai'ssubscribe, so a desktop tab that was already open when/mwrote does not pick the value up live. The warm path needs a fresh page load after the/mvisit.The practical consequence for review: test COLD. Use a fresh browser profile, or clear
agenta:agent-chat:messages:v2before opening the session. A warm open exercises a path that was never broken and will pass either way.What to QA
localStoragefirst. The key isagenta:agent-chat:messages:v2. This is what makes it a cold open, and it is the only shape that was broken.