feat(frontend): Surface parked connect prompts in an interaction dock - #5521
feat(frontend): Surface parked connect prompts in an interaction dock#5521ashrafchowdury wants to merge 4 commits into
Conversation
- Added InteractionDock component to manage parked client-tool interactions, mirroring ApprovalDock's functionality. - Introduced WaitingForInput component to indicate when the agent is waiting for user input. - Updated AgentConversation to integrate InteractionDock and display waiting states appropriately. - Enhanced QueuedMessages to reflect held messages when the interaction is pending. - Refactored ConnectToolWidget to utilize useConnectFlow for managing connection states and actions. - Added useConnectFlow hook to centralize connection logic for both InteractionDock and inline components. - Updated state management to include isPendingClientToolInteraction for better handling of client-tool interactions.
dosubot
Bot
added
size:XL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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: Organization 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:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
10bb97f to
4b9e0e4
Compare
ashrafchowdury
changed the title
Railway Preview Environment
|
…n-dock # Conflicts: # web/packages/agenta-playground/src/index.ts # web/packages/agenta-playground/src/state/execution/index.ts # web/packages/agenta-playground/src/state/index.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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/components/clientTools/ConnectToolWidget.tsx (1)
63-108: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
phase === "error"branch is unreachable — retry failures never show the specific reason.Every reachable
runConnect(false)call comes from the Retry button at line 93, which only renders oncemeta.settled || outcomeis alreadytrue(that's the whole premise of "retry after settle"). So when that retry fails and setsphase = "error"+errorText, the render still hits themeta.settled || outcomecheck at line 64 first and returns the generic "Connection not completed" chip (line 88-95) — the newphase === "error"block at line 98 can never execute. The specific failure reason from a failed retry is silently dropped.Move the
phase === "error"check ahead of the settled check (same pattern already used forphase === "connecting").🐛 Proposed fix: reorder the branches
- // ── Settled: the result chip (U1). `outcome` covers the render before `meta.settled` flips. ── - if (meta.settled || outcome) { + // ── Error on a manual retry (create failed, popup blocked): show reason + Retry ────────────── + if (phase === "error") { + return ( + <ChipRow icon={<Warning size={13} weight="fill" className="text-colorError" />}> + <Text type="danger" className="!text-xs truncate" title={errorText ?? undefined}> + {errorText ?? "Connection failed."} + </Text> + <RetryButton onClick={() => runConnect(false)} /> + </ChipRow> + ) + } + + // ── Settled: the result chip (U1). `outcome` covers the render before `meta.settled` flips. ── + if (meta.settled || outcome) { const output = (meta.output ?? {}) as ConnectOutput ... return ( <ChipRow icon={<Warning size={13} weight="fill" className="text-colorWarning" />}> <Text type="secondary" className="!text-xs"> Connection not completed </Text> <RetryButton onClick={() => runConnect(false)} /> </ChipRow> ) } - - // ── Error on a manual retry (create failed, popup blocked): show reason + Retry ────────────── - if (phase === "error") { - return ( - <ChipRow icon={<Warning size={13} weight="fill" className="text-colorError" />}> - <Text type="danger" className="!text-xs truncate" title={errorText ?? undefined}> - {errorText ?? "Connection failed."} - </Text> - <RetryButton onClick={() => runConnect(false)} /> - </ChipRow> - ) - }
🧹 Nitpick comments (2)
web/oss/src/components/AgentChatSlice/components/clientTools/useConnectFlow.ts (2)
174-190: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider validating
redirectUrl's scheme before opening it.
redirectUrlcomes straight fromresult.connection?.data?.redirect_urland is passed towindow.openunchecked. It's backend-controlled today, but a cheaphttp(s):-only guard adds defense-in-depth against a misbehaving/compromised response opening ajavascript:/data:URL in the opener's context.🛡️ Proposed guard
if (!redirectUrl) { // No OAuth step (e.g. api_key created inline): the connection already exists. onSuccess() return } + if (!/^https?:\/\//i.test(redirectUrl)) { + setPhase("error") + setErrorText("Received an unexpected connection URL.") + return + } const popup = window.open(
70-304: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftNo unit tests for this race-prone state machine.
useConnectFlowcoordinates popup lifecycle,postMessageorigin validation, polling, and a timeout backstop with a one-shot settle guard — exactly the kind of logic where a small ordering mistake (as found inConnectToolWidget.tsx's render branches) goes unnoticed without tests. Consider adding unit tests aroundfinish/runConnect/cancel/decline(mockingwindow.open,postMessage, timers).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 55573f52-cd9d-412e-ab59-654c60dae71d
📒 Files selected for processing (8)
web/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/components/InteractionDock.tsxweb/oss/src/components/AgentChatSlice/components/QueuedMessages.tsxweb/oss/src/components/AgentChatSlice/components/clientTools/ConnectToolWidget.tsxweb/oss/src/components/AgentChatSlice/components/clientTools/useConnectFlow.tsweb/packages/agenta-playground/src/index.tsweb/packages/agenta-playground/src/state/execution/index.tsweb/packages/agenta-playground/src/state/index.ts
2 participants
Context
When the runner parks a
request_connectioncall, the stream genuinely ends.useChatreportsready, so nothing busy-derived (working dots, the stop button) signals that the run is paused on you. Meanwhile the message queue quietly holds every send.The result: the turn looked finished, anything you typed disappeared into a queue with no explanation, and a parked connection had no way to say no, so the conversation stayed frozen.
Changes
InteractionDocksits in the composer region, between the transcript and the input, so a paused run cannot scroll out of reach. It follows the same contract asApprovalDock. The dock owns the actions and the inline transcript row is only a marker. It also adds the escape hatch a parked connection lacked: "Not now" settles the call as a structured decline, so the run resumes and the conversation unfreezes.Three smaller pieces make the pause legible:
WorkingDotsanimates because the agent is working. This one is deliberately static, because here it is your move.ConnectToolWidgetdrops about 280 lines. The OAuth flow moved into a newuseConnectFlowhook so the dock and the inline chip share one implementation instead of two that could both settle the same call. The hook settles on every terminal path, so a run cannot hang:It settles with a reference, never a secret. The runner re-resolves the credential from the project vault on resume.
Scope is the connect interaction only. Elicitation stays inline, since it is a form the user fills in the transcript and it already carries its own decline and dismiss actions. The composer waiting state covers its visibility.
Tests / notes
tsc --noEmitonweb/osspasses with 0 errors, satisfying the type-regression gate added in [chore] Zero out web tsc errors and fail builds on type regressions #5464.pnpm lint-fixis clean and changed no files.release/v0.105.6and had diverged from its own remote, which was built onbig-agents. The commits were cherry-picked onto a fresh branch off the release instead.What to QA
ApprovalDockas before, and an ordinary tool-using turn shows no interaction dock and does not auto-resend.Previews
Before:
After: