fix(frontend): Make parked connection requests visible and declinable - #5251
fix(frontend): Make parked connection requests visible and declinable
#5251ashrafchowdury wants to merge 6 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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
size:XXL
📝 WalkthroughWalkthroughThe PR adds parked connect-interaction handling and waiting-state UI to agent chat. It also adds linear revision selection and agent-specific commit, deployment, navigation, workflow-state, and public export behavior in the playground. ChangesParked chat interactions
Playground agent revision UX
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AgentConversation
participant InteractionDock
participant useConnectFlow
participant OAuthPopup
participant ClientTool
AgentConversation->>InteractionDock: pass pending interaction
InteractionDock->>useConnectFlow: runConnect, cancel, or decline
useConnectFlow->>OAuthPopup: open and monitor OAuth flow
OAuthPopup-->>useConnectFlow: validated completion message
useConnectFlow->>ClientTool: settle output or error
ClientTool-->>AgentConversation: update interaction state
Possibly related PRs
🚥 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 |
dosubot
Bot
added
size:XL
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Thanks @ashrafchowdury I think it makes sense and it unifies the UI for approvals. The copy can be improved though.
@ardaerzin wdyt?
389d3e7 to
c5fafd2
Compare
|
Updated the copy: Connect Gmail to let the agent continue, or continue without the connection. |
- reframe default variant as history, nothing else changed - hide the deployment option for the agent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/oss/src/components/AgentChatSlice/components/clientTools/useConnectFlow.ts (1)
36-41: 📐 Maintainability & Code Quality | 🔵 TrivialUnresolved TODO left in shipped code.
CONNECT_TIMEOUT_MS's bound is flagged as an open question ("NOTE for Mahmoud: confirm the bound") rather than a settled value. Worth confirming/closing before merge so a stale note doesn't linger in production code.Want me to open a follow-up issue to track confirming this timeout, or help track down the referenced open question?
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6fdcc7dc-34f9-43c8-ba3c-585199dab1bd
📒 Files selected for processing (17)
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/oss/src/components/Playground/Components/AgentRevisionSelector/index.tsxweb/oss/src/components/Playground/Components/Menus/SelectVariant/components/RevisionChildTitle.tsxweb/oss/src/components/Playground/Components/Menus/SelectVariant/index.tsxweb/oss/src/components/Playground/Components/Menus/SelectVariant/types.d.tsweb/oss/src/components/Playground/Components/Modals/CommitVariantChangesModal/index.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/PlaygroundVariantConfigHeader.tsxweb/oss/src/components/Sidebar/hooks/useSidebarConfig/index.tsxweb/oss/src/state/workflow/hooks.tsweb/packages/agenta-entity-ui/src/selection/components/UnifiedEntityPicker/variants/TreeSelectPopupContent.tsxweb/packages/agenta-playground/src/index.tsweb/packages/agenta-playground/src/state/execution/index.tsweb/packages/agenta-playground/src/state/index.ts
| * Settle on every terminal path (design §"Settle on every path"), so the run never hangs: | ||
| * success → {connected:true, integration, slug} · decline → {connected:false, reason:"declined"} | ||
| * · cancel/abandon → {connected:false, reason:"cancelled"} · timeout → {connected:false, | ||
| * reason:"timeout"} · failure → errorText. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Create-failure path violates the file's own "failure → errorText" contract.
The docstring promises failure → errorText, but the catch block's finish({connected: false, integration, slug, reason: message}) (Line 258) routes through the ConnectOutput/output branch of finish(), not the errorText branch. In fact, no call site in this file ever constructs {errorText: ...} — that branch of finish() (Lines 120-122) is dead code.
Practical effect: a genuine API/network failure while creating the connection settles as output: {connected:false, reason:"<raw error message>"}, indistinguishable in shape from a structured decline/cancel/timeout. Downstream, ConnectToolWidget distinguishes output-error state meaningfully elsewhere (deferredByRunner), so the agent/runner likely can't tell "user declined" apart from "connection creation actually failed."
🐛 Proposed fix
} catch (err) {
if (settleParkedCall && !activeRef.current) return
const message = err instanceof Error ? err.message : "Connection failed."
// A create failure is terminal for the parked call: settle so the run resumes; for a
// manual retry just surface the reason with another Retry.
setPhase("error")
setErrorText(message)
- if (settleParkedCall) finish({connected: false, integration, slug, reason: message})
+ if (settleParkedCall) finish({errorText: message})
}Also applies to: 251-259
| className={cn( | ||
| "tree-popup-compact pb-2", | ||
| flattenSingleParent ? "tree-popup-flat px-1.5 pt-2.5" : "px-2", | ||
| )} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply flattened layout classes only when the tree is actually flattened.
The class tree-popup-flat removes the left padding from leaf nodes. Currently, it is applied whenever the flattenSingleParent prop is true, even if the tree was not actually flattened (e.g., if treeData.length > 1). This would incorrectly strip the indentation from leaf nodes in a multi-parent tree.
Update the condition to ensure the styling is only applied when the tree data is successfully flattened.
🐛 Proposed fix
className={cn(
"tree-popup-compact pb-2",
- flattenSingleParent ? "tree-popup-flat px-1.5 pt-2.5" : "px-2",
+ flattenSingleParent && treeData.length === 1 ? "tree-popup-flat px-1.5 pt-2.5" : "px-2",
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={cn( | |
| "tree-popup-compact pb-2", | |
| flattenSingleParent ? "tree-popup-flat px-1.5 pt-2.5" : "px-2", | |
| )} | |
| > | |
| className={cn( | |
| "tree-popup-compact pb-2", | |
| flattenSingleParent && treeData.length === 1 ? "tree-popup-flat px-1.5 pt-2.5" : "px-2", | |
| )} | |
| > |
|
@ashrafchowdury I think this is still relevant can you please resolve the conflicts and check whether still works and add it to the merge queue in the 106 thread |
3 participants
Context
When the agent asks for a connection in the playground chat (a parked
request_connectionclient tool), the runner legitimately ends the stream. Every "in progress" signal in the chat was derived from stream status, so the UI looked idle: no working dots, a plain send button. Meanwhile the queue gate (isHitlPending) held every new message, so sends piled up as "1 queued" with no visible reason. Users read this as the agent being stuck and kept resending. There was also no way to refuse the request: the inline chip only offered Connect, and the timeout only arms after the popup opens, so an ignored request froze the conversation forever (a reload restored the same frozen state).Changes
The paused run is now a first-class "waiting for you" state, following the existing ApprovalDock contract (the dock owns the actions, the inline row is a marker).
InteractionDock (new). A persistent card between the transcript and the composer: "The agent is waiting for you", with Connect and "Not now" buttons. It cannot scroll out of reach. "Not now" settles the parked call with
{connected: false, reason: "declined"}(distinct from"cancelled", which means an abandoned popup), so the run resumes, the agent can respond gracefully, and queued messages release.Shared OAuth flow. The popup flow (create connection, origin-validated callback, closed-popup poll, timeout backstop) moved from
ConnectToolWidgetintouseConnectFlow. The dock runs the live parked call; the inline chip keeps the post-settle states (result chip, Retry). Both instances can be mounted for the same call, so every live-settle path now also checksmeta.settled; a secondaddToolOutputfor the same call is impossible.The composer tells the truth while paused:
Before: placeholder "Ask the agent...", plain send button, pill "1 queued".
After: placeholder says the agent is waiting and new messages will be queued, the last turn shows a static "Waiting for your input" chip in the working-dots slot, and the pill reads "1 queued · waiting on you" (popover: "Held until you answer the agent").
isPendingClientToolInteractionis now exported from@agenta/playground, so the dock's visibility and the queue's hold derive from the same predicate and cannot disagree.Tests / notes
@agenta/playgroundbuilds; all 198 package unit tests pass.@agenta/ossand@agenta/playground; tsc reports no errors in touched files (the twoElicitationWidgeterrors are pre-existing on the branch).What to QA
Previews
Before:
After: