diff --git a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.accountChoice.test.tsx b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.accountChoice.test.tsx index 8c452a251..1cf4bba76 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.accountChoice.test.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.accountChoice.test.tsx @@ -12,6 +12,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { Status } from "@/api/types"; import { FeatureFlagContext } from "@/hooks/useFeatureFlag"; +import { useCopilotHeaderStore } from "@/store/useCopilotHeaderStore"; type StreamBody = { message: string; @@ -346,7 +347,9 @@ describe("WorkflowCopilotChat connected account choices", () => { }).disabled, ).toBe(true); if (accountQuestion) { - expect(screen.getByText("Choose a Google account")).toBeTruthy(); + expect( + screen.getByRole("group", { name: "Connected Google accounts" }), + ).toBeTruthy(); expect( screen.getByRole("button", { name: /Connection …goac_1/, @@ -400,6 +403,28 @@ describe("WorkflowCopilotChat connected account choices", () => { }, ); + it("docks an actionable choice above the composer and picks an active row by number", async () => { + await renderChat(); + await finishChoiceAsk(); + const tray = screen.getByRole("group", { + name: "Connected Google accounts", + }); + expect( + Boolean( + screen.getByText(/choose below/).compareDocumentPosition(tray) & + Node.DOCUMENT_POSITION_FOLLOWING, + ), + ).toBe(true); + expect(useCopilotHeaderStore.getState().attention).toBe("account"); + + // The first number is the first active row; the reconnect-only row above it has no key. + await act(async () => { + fireEvent.keyDown(tray, { key: "1" }); + }); + await waitFor(() => expect(postStreaming).toHaveBeenCalledTimes(2)); + expect(streamCalls[1]?.body.selected_connected_account_id).toBe("goac_1"); + }); + it("renders canonical rows and sends one exact active id despite a same-tick double click", async () => { await renderChat(); await finishChoiceAsk(); diff --git a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.credentialCard.test.tsx b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.credentialCard.test.tsx index 3f8729bb4..7a2d22a3f 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.credentialCard.test.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.credentialCard.test.tsx @@ -15,6 +15,7 @@ import { canonicalRecoveriesByWorkflow, } from "./WorkflowCopilotChat"; +import { useCopilotHeaderStore } from "@/store/useCopilotHeaderStore"; import { useRecordingRefinementEvidenceStore } from "@/store/RecordingRefinementEvidenceStore"; import { beginYamlCommit, @@ -580,6 +581,85 @@ describe("WorkflowCopilotChat — activity log", () => { }); describe("WorkflowCopilotChat — credential receipt placement", () => { + it("releases the dock and header flag when a docked sign-in request times out", async () => { + await streamScoutTurn(); + vi.useFakeTimers(); + await act(async () => { + streamCalls[0]!.onMessage( + credentialFrame({ anchor_tool_call_id: "tc-1" }), + ); + await vi.advanceTimersByTimeAsync(0); + }); + expect(screen.getByRole("group", { name: "Sign-in request" })).toBeTruthy(); + expect(useCopilotHeaderStore.getState().attention).toBe("credential"); + + // The server sends no frame on timeout, so the deadline alone has to release the dock. + await act(async () => vi.advanceTimersByTimeAsync(300_001)); + expect(screen.queryByRole("group", { name: "Sign-in request" })).toBeNull(); + expect(screen.queryByText(/continue below/)).toBeNull(); + expect(screen.getAllByText("Timed out").length).toBeGreaterThan(0); + expect(useCopilotHeaderStore.getState().attention).toBeNull(); + }); + + it("docks a live sign-in request above the composer and hands the receipt back to the transcript", async () => { + credentialsData.current = [ + { credential_id: "cred-hn", name: "HN Login", tested_url: null }, + ]; + await streamScoutTurn(); + await act(async () => { + streamCalls[0]!.onMessage( + credentialFrame({ anchor_tool_call_id: "tc-1" }), + ); + }); + const precedes = (a: Node, b: Node) => + Boolean(a.compareDocumentPosition(b) & Node.DOCUMENT_POSITION_FOLLOWING); + const tray = await screen.findByRole("group", { name: "Sign-in request" }); + const marker = screen.getByText(/continue below/); + expect( + precedes( + document.querySelector('[data-activity-row-id="tc-1"]')!, + marker, + ), + ).toBe(true); + expect(precedes(marker, tray)).toBe(true); + expect( + precedes( + tray, + screen.getByRole("group", { name: "Copilot message composer" }), + ), + ).toBe(true); + expect(screen.getAllByRole("button", { name: "Skip for now" })).toEqual([ + within(tray).getByRole("button", { name: "Skip for now" }), + ]); + expect(useCopilotHeaderStore.getState().attention).toBe("credential"); + + // Minimized, the tray keeps its name on the control that restores it. + fireEvent.click( + within(tray).getByRole("button", { name: "Minimize sign-in request" }), + ); + fireEvent.click( + screen.getByRole("button", { name: "Show Sign-in request" }), + ); + expect(screen.getByRole("group", { name: "Sign-in request" })).toBeTruthy(); + + await act(async () => { + streamCalls[0]!.onMessage({ + type: "credential_pause_resolved", + turn_id: "turn-1", + workflow_copilot_chat_id: "chat-1", + resume_token: "rt-abc", + outcome: "connected", + credential_id: "cred-hn", + name: "HN Login", + timestamp: new Date().toISOString(), + }); + }); + expect(await screen.findByText("Credential 'HN Login' added")).toBeTruthy(); + expect(screen.queryByRole("group", { name: "Sign-in request" })).toBeNull(); + expect(screen.queryByText(/continue below/)).toBeNull(); + expect(useCopilotHeaderStore.getState().attention).toBeNull(); + }); + it("keeps a mid-turn credential card after the row it was raised on, live and once the turn ends", async () => { credentialsData.current = [ { credential_id: "cred-hn", name: "HN Login", tested_url: null }, @@ -2090,9 +2170,14 @@ describe("WorkflowCopilotChat — credential card wiring", () => { await waitFor(() => expect(credentialsGets().length).toBeGreaterThanOrEqual(1), ); + // The notice sits in the tray, and a second copy of the ask in the transcript would add a button. + const tray = await screen.findByRole("group", { name: "Sign-in request" }); expect( - await screen.findByText(/Couldn't load your saved logins/), - ).toBeTruthy(); + within(tray).getAllByText(/Couldn't load your saved logins/), + ).toHaveLength(1); + expect( + screen.getAllByRole("button", { name: "Connect credential" }), + ).toHaveLength(1); expect(screen.getByRole("button", { name: "Retry" })).toBeTruthy(); expect( screen.getByRole("button", { name: "Connect credential" }), @@ -2322,6 +2407,9 @@ describe("WorkflowCopilotChat — credential card wiring", () => { }); expect(await screen.findByRole("combobox")).toBeTruthy(); expect(screen.queryByText("Continuing with 'HN login'…")).toBeNull(); + // A stranded ask is no longer the tail turn, so it stays inline instead of holding the dock. + expect(screen.queryByRole("group", { name: "Sign-in request" })).toBeNull(); + expect(useCopilotHeaderStore.getState().attention).toBeNull(); }); it("hides a stale terminal ask once it is no longer the last message", async () => { diff --git a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.historyRace.test.tsx b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.historyRace.test.tsx index 39781e142..4ae1192b4 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.historyRace.test.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.historyRace.test.tsx @@ -2921,7 +2921,7 @@ describe("WorkflowCopilotChat — question transport", () => { expect(tray.nextElementSibling).toBe( screen.getByRole("group", { name: "Copilot message composer" }), ); - expect(useCopilotHeaderStore.getState().awaitingAnswer).toBe(true); + expect(useCopilotHeaderStore.getState().attention).toBe("question"); cancelPost.mockResolvedValueOnce({ data: { @@ -2962,7 +2962,7 @@ describe("WorkflowCopilotChat — question transport", () => { screen.queryByRole("group", { name: "Question parts" }), ).toBeNull(), ); - expect(useCopilotHeaderStore.getState().awaitingAnswer).toBe(false); + expect(useCopilotHeaderStore.getState().attention).toBeNull(); }); it("waits for an answer before the composer sends, and locks Cancel while it posts", async () => { diff --git a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.tsx b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.tsx index 827c6a908..4c1c5e3a7 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/WorkflowCopilotChat.tsx @@ -25,6 +25,7 @@ import { import { ArtifactDownloadLink } from "@/components/ArtifactDownloadLink"; import { Button } from "@/components/ui/button"; import { + Fragment, useState, useEffect, useLayoutEffect, @@ -69,7 +70,10 @@ import { } from "@/store/WorkflowHasChangesStore"; import { useWorkflowTitleStore } from "@/store/WorkflowTitleStore"; import { useCopilotActionStore } from "@/store/useCopilotActionStore"; -import { useCopilotHeaderStore } from "@/store/useCopilotHeaderStore"; +import { + type CopilotAttention, + useCopilotHeaderStore, +} from "@/store/useCopilotHeaderStore"; import { buildWorkflowCopilotContext, buildWorkflowYamlDocument, @@ -172,7 +176,10 @@ import { import { useRunLifecycleAnnouncements } from "./useRunLifecycleAnnouncements"; import { useHistoryLoad } from "./useHistoryLoad"; import { ConfirmCard, shouldShowConfirmCard } from "./cards/ConfirmCard"; -import { ConnectedAccountChoiceCard } from "./cards/ConnectedAccountChoiceCard"; +import { + ConnectedAccountChoiceCard, + ConnectedAccountChoiceMarker, +} from "./cards/ConnectedAccountChoiceCard"; import { QuestionReceipt } from "./cards/QuestionReceipt"; import { QUESTION_PROMPT_ID, QuestionTray } from "./cards/QuestionTray"; import { useQuestionStepper } from "./useQuestionStepper"; @@ -188,9 +195,11 @@ import { connectedAccountChoiceLabel } from "./cards/connectedAccountChoiceLabel import { shouldShowDiffCard } from "./cards/DiffCard"; import { ReviewGateCard, getReviewGateVerdict } from "./cards/ReviewGateCard"; import { TURN_ROW_INSET } from "./cards/cardLayout"; +import { type AttentionTrayPresentation } from "./cards/AttentionTray"; import { TestRunOutputCard } from "./cards/TestRunOutputCard"; import { GoogleReconnectCard } from "./cards/GoogleReconnectCard"; import { + CredentialAskMarker, CredentialCard, type CredentialRequiredFrame, type CredentialRequiredReason, @@ -1036,6 +1045,63 @@ type CredentialResolution = CredentialPauseHistorical & { continued?: boolean; }; +type TerminalCredentialAsk = { + turnId: string; + frame: CredentialRequiredFrame; + localResolution: CredentialResolution | undefined; + resolvedOutcome: CredentialPauseHistorical | undefined; + canContinue: boolean; +}; + +// The actionable ask is only live on the tail message; a resolved receipt renders on any message so +// a scrolled-back turn keeps its outcome. A stranded ask (its auto-continue failed) stays actionable off-tail. +function terminalCredentialAskFor( + message: ChatMessage, + isLastMessage: boolean, + credentialResolutions: Record, + pauseCardResolutions: Record, + strandedTerminalContinuations: ReadonlySet, +): TerminalCredentialAsk | null { + const turnId = message.narrative?.turnId ?? null; + if (!message.narrative || turnId === null) return null; + const frame = credentialCardFrameFor(message.narrative); + if (!frame) return null; + const localResolution = credentialResolutions[turnId]; + // The persisted pause verdict outranks the optimistic click, so a pick the server did not admit + // never reads as connected; the click's name survives via pauseCardResolutions. + const resolvedOutcome = + historicalCredentialOutcome(message.narrative, pauseCardResolutions) ?? + localResolution; + const canContinue = + isLastMessage || strandedTerminalContinuations.has(turnId); + if (!resolvedOutcome && !canContinue) return null; + return { turnId, frame, localResolution, resolvedOutcome, canContinue }; +} + +function connectedAccountChoiceStateFor( + messages: ChatMessage[], + index: number, +) { + const choices = messages[index]?.narrative?.connectedAccountChoices ?? []; + const adjacentMessage = nextAnsweringMessage(messages, index); + const selectedConnectionId = + adjacentMessage?.sender === "user" && + choices.some((choice) => choice.connection_id === adjacentMessage.content) + ? adjacentMessage.content + : null; + // Once any later conversation message exists, this account-choice turn is historical. Only an + // exact structured selection may render a receipt; prose must never make the old card actionable + // again between stream completion and the next assistant response. + const hasUnconsumedAdjacentMessage = + adjacentMessage !== undefined && selectedConnectionId === null; + return { + choices, + adjacentMessage, + selectedConnectionId, + hasUnconsumedAdjacentMessage, + }; +} + // Append a resolution under its key (a turn or a card), capping the map with oldest-eviction like // the sibling per-turn maps (turnSnapshots/turnOwnedRunIds). delete-then-set // re-inserts an existing key as newest so an active one isn't evicted. @@ -6701,11 +6767,126 @@ export function WorkflowCopilotChat({ setInputValue(settled); }); }, [inputValue, isSpeechListening, stopSpeech, trayQuestionId]); + + // What Copilot is waiting on the user for, docked above the composer one at a time. A question + // goes first because the composer answers it; the others wait behind it as "Up next". + const visibleRecoveredPauseFrames = isLoadingHistory + ? [] + : recoveredPauseFrames.filter( + (frame) => + frame.workflow_copilot_chat_id === workflowCopilotChatId && + frame.turn_id !== livePauseFrame?.turn_id, + ); + // The server sends nothing when a pause times out, so expiry is read from the frame itself; a + // missing or malformed expires_at counts as expired, matching the card's countdown. + const openPauseFrames = isLoadingHistory + ? [] + : [ + ...(livePauseFrame && + livePauseFrame.turn_id === narrative.turnId && + narrative.terminal === null + ? [livePauseFrame] + : []), + ...visibleRecoveredPauseFrames, + ].filter( + (frame) => + !pauseCardResolutions[frame.resume_token] && + Date.parse(frame.expires_at ?? "") > Date.now(), + ); + const trayPauseFrame = openPauseFrames[0] ?? null; + const nextPauseExpiry = openPauseFrames.length + ? Math.min( + ...openPauseFrames.map((frame) => Date.parse(frame.expires_at ?? "")), + ) + : null; + const [, setPauseExpiryTick] = useState(0); + useEffect(() => { + if (nextPauseExpiry === null) return; + const timer = window.setTimeout( + () => setPauseExpiryTick((tick) => tick + 1), + Math.max(0, nextPauseExpiry - Date.now()) + 1, + ); + return () => window.clearTimeout(timer); + }, [nextPauseExpiry]); + const lastTurnIndex = findLastTurnIndex(messages); + // Only the tail turn's ask docks; a stranded one further up stays actionable inline. + const tailTerminalAsk = + trayPauseFrame || isLoadingHistory || lastTurnIndex < 0 + ? null + : terminalCredentialAskFor( + messages[lastTurnIndex]!, + true, + credentialResolutions, + pauseCardResolutions, + strandedTerminalContinuations, + ); + const trayTerminalAsk = + tailTerminalAsk && !tailTerminalAsk.resolvedOutcome + ? tailTerminalAsk + : null; + const trayCredentialKey = trayPauseFrame + ? `pause:${trayPauseFrame.resume_token}` + : trayTerminalAsk + ? `terminal:${trayTerminalAsk.turnId}` + : null; + const trayAccountChoice = (() => { + const tail = messages[lastTurnIndex]; + const turnId = tail?.narrative?.turnId ?? null; + // A pick waiting in the queue already answered it, so the dock steps aside for the inline card. + if ( + tail?.sender !== "ai" || + turnId === null || + queuedPrompt?.selectedConnectedAccountId !== undefined + ) { + return null; + } + const { choices, adjacentMessage } = connectedAccountChoiceStateFor( + messages, + lastTurnIndex, + ); + // A picker with only reconnect links has nothing to choose, so it stays inline. + return adjacentMessage === undefined && + choices.some((choice) => choice.state === "active") + ? { turnId, choices } + : null; + })(); + // The question always leads, so it never needs an "Up next" label. + const attentionQueue: { kind: CopilotAttention; upNext?: string }[] = []; + if (trayQuestion) { + attentionQueue.push({ kind: "question" }); + } + if (trayCredentialKey) { + attentionQueue.push({ + kind: "credential", + upNext: "Copilot needs to sign in", + }); + } + if (trayAccountChoice) { + attentionQueue.push({ kind: "account", upNext: "Choose a Google account" }); + } + const activeAttention = attentionQueue[0]?.kind ?? null; + const attentionUpNext = attentionQueue[1]?.upNext ?? null; + // Only the open tray replaces its transcript card; queued items stay actionable inline. + const dockedPauseFrame = + activeAttention === "credential" ? trayPauseFrame : null; + const dockedTerminalAsk = + activeAttention === "credential" ? trayTerminalAsk : null; + const dockedAccountChoice = + activeAttention === "account" ? trayAccountChoice : null; + const [collapsedAttentionKey, setCollapsedAttentionKey] = useState< + string | null + >(null); + const attentionTray = (key: string): AttentionTrayPresentation => ({ + collapsed: collapsedAttentionKey === key, + onCollapsedChange: (collapsed) => + setCollapsedAttentionKey(collapsed ? key : null), + upNext: attentionUpNext, + }); useEffect(() => { const store = useCopilotHeaderStore.getState(); - store.setAwaitingAnswer(hasPendingQuestion); - return () => store.setAwaitingAnswer(false); - }, [hasPendingQuestion]); + store.setAttention(activeAttention); + return () => store.setAttention(null); + }, [activeAttention]); const uploadDroppedAttachments = useCallback( (files: FileList) => { const droppedFiles = Array.from(files); @@ -9128,6 +9309,68 @@ export function WorkflowCopilotChat({ return null; }; + const pauseCredentialCard = ( + frame: WorkflowCopilotCredentialRequiredUpdate, + tray?: AttentionTrayPresentation, + ) => ( + + openCredentialModal(frame, frame.turn_id, false, credential) + } + // A picked credential (id + name from the fetched list) answers through the typed resume + // POST, which origin-binds; the Add-credential CTA (no id) opens the modal. + onConnect={(credentialId, name) => + credentialId + ? void respondToCredentialPause( + frame, + "connected", + credentialId, + name, + ) + : openCredentialModal(frame, frame.turn_id) + } + onSkip={() => void respondToCredentialPause(frame, "skip")} + tray={tray} + /> + ); + const terminalCredentialCard = ( + ask: TerminalCredentialAsk, + tray?: AttentionTrayPresentation, + ) => ( + + credentialId + ? continueAfterTerminalConnect( + ask.turnId, + credentialId, + name ?? ask.localResolution?.name, + ask.canContinue, + ) + : openCredentialModal(null, ask.turnId, ask.canContinue) + } + onSkip={() => resolveTerminalCredential(ask.turnId, "skip")} + tray={tray} + /> + ); + const accountChoiceBusy = (turnId: string) => + isLoading || + hasPendingQuestion || + acceptUnresolved || + connectedAccountChoicePendingTurnId === turnId; + // The live turn places its plan and its credential card exactly where the finished turn will, so // nothing moves when the turn ends. const liveAnchored: AnchoredTurnItem[] = []; @@ -9154,36 +9397,15 @@ export function WorkflowCopilotChat({ narrative, livePauseFrame.anchor_tool_call_id, ), - node: ( - - openCredentialModal( - livePauseFrame, - livePauseFrame.turn_id, - false, - credential, - ) - } - // A picked credential (id + name from the fetched list) answers through the typed - // resume POST, which origin-binds; the Add-credential CTA (no id) opens the modal. - onConnect={(credentialId, name) => - credentialId - ? void respondToCredentialPause( - livePauseFrame, - "connected", - credentialId, - name, - ) - : openCredentialModal(livePauseFrame, livePauseFrame.turn_id) - } - onSkip={() => void respondToCredentialPause(livePauseFrame, "skip")} - /> - ), + node: + dockedPauseFrame?.resume_token === livePauseFrame.resume_token ? ( + + ) : ( + pauseCredentialCard(livePauseFrame) + ), }); } @@ -9303,7 +9525,6 @@ export function WorkflowCopilotChat({ ? `Listening… · ${browserStatusText}` : "Listening…" : browserStatusText; - const lastTurnIndex = findLastTurnIndex(messages); // The composer is the text field for the pending question, so while one is pending it says so // rather than inviting a new request. const latestTurnIsAsk = questionInteractions.some( @@ -9817,73 +10038,29 @@ export function WorkflowCopilotChat({ if (message.sender === "ai" && message.narrative) { questionsPlaced = true; const turnId = message.narrative.turnId; - const choices = message.narrative.connectedAccountChoices; - const adjacentMessage = nextAnsweringMessage(messages, index); - const selectedConnectionId = - adjacentMessage?.sender === "user" && - choices.some( - (choice) => - choice.connection_id === adjacentMessage.content, - ) - ? adjacentMessage.content - : null; - // Once any later conversation message exists, this account-choice turn is - // historical. Only an exact structured selection may render a receipt; - // prose must never make the old card actionable again between stream - // completion and the next assistant response. - const hasUnconsumedAdjacentMessage = - adjacentMessage !== undefined && - selectedConnectionId === null; + const { + choices, + adjacentMessage, + selectedConnectionId, + hasUnconsumedAdjacentMessage, + } = connectedAccountChoiceStateFor(messages, index); const showReviewGate = shouldShowDiffCard(message.narrative) || (turnId !== null && turnId === pendingProposalTurnId); const credentialCard = (() => { - if (isLoadingHistory || turnId === null) return null; - const credFrame = credentialCardFrameFor(message.narrative); - if (!credFrame) return null; - const localResolution = credentialResolutions[turnId]; - // The persisted pause verdict outranks the optimistic click, so a pick the server - // did not admit never reads as connected; the click's name survives via pauseCardResolutions. - const resolvedOutcome = - historicalCredentialOutcome( - message.narrative, - pauseCardResolutions, - ) ?? localResolution; - // The actionable ask is only live on the tail message; a resolved receipt still - // renders on any message so a scrolled-back turn keeps its outcome. Without this, - // picking on a stale card would show a receipt with no backend call or continue. - // A stranded ask (its auto-continue failed) stays actionable off-tail for a retry. - if ( - !resolvedOutcome && - !isLastMessage && - !strandedTerminalContinuations.has(turnId) - ) - return null; - return ( - { - const canContinue = - isLastMessage || - strandedTerminalContinuations.has(turnId); - return credentialId - ? continueAfterTerminalConnect( - turnId, - credentialId, - name ?? localResolution?.name, - canContinue, - ) - : openCredentialModal(null, turnId, canContinue); - }} - onSkip={() => resolveTerminalCredential(turnId, "skip")} - /> + if (isLoadingHistory) return null; + const ask = terminalCredentialAskFor( + message, + isLastMessage, + credentialResolutions, + pauseCardResolutions, + strandedTerminalContinuations, + ); + if (!ask) return null; + return dockedTerminalAsk?.turnId === ask.turnId ? ( + + ) : ( + terminalCredentialCard(ask) ); })(); const autoBoundCard = (() => { @@ -10013,18 +10190,19 @@ export function WorkflowCopilotChat({ a Google account.

) : null} - {turnId !== null && choices.length > 0 ? ( + {turnId !== null && + choices.length > 0 && + dockedAccountChoice?.turnId === turnId ? ( + + ) : turnId !== null && choices.length > 0 ? ( handleConnectedAccountChoice(turnId, connectionId) @@ -10278,41 +10456,16 @@ export function WorkflowCopilotChat({ RESPONSE has frozen the narrative into the latest AI message — otherwise the same turn would render twice. */} - {!isLoadingHistory && - recoveredPauseFrames - .filter( - (frame) => - frame.workflow_copilot_chat_id === workflowCopilotChatId && - frame.turn_id !== livePauseFrame?.turn_id, - ) - .map((frame) => ( - - credentialId - ? void respondToCredentialPause( - frame, - "connected", - credentialId, - name, - ) - : openCredentialModal(frame, frame.turn_id) - } - onUpdateCredential={(credential) => - openCredentialModal( - frame, - frame.turn_id, - false, - credential, - ) - } - onSkip={() => void respondToCredentialPause(frame, "skip")} - /> - ))} + {visibleRecoveredPauseFrames.map((frame) => + frame.resume_token === dockedPauseFrame?.resume_token ? ( + + ) : ( + pauseCredentialCard(frame) + ), + )} {narrative.turnId !== null && narrative.terminal === null && (
+ ) : null} + {trayCredentialKey && (dockedPauseFrame || dockedTerminalAsk) ? ( + // Keyed so moving between asks never carries one ask's picker state into the next. + + {dockedPauseFrame + ? pauseCredentialCard( + dockedPauseFrame, + attentionTray(trayCredentialKey), + ) + : dockedTerminalAsk + ? terminalCredentialCard( + dockedTerminalAsk, + attentionTray(trayCredentialKey), + ) + : null} + + ) : null} + {dockedAccountChoice ? ( + + handleConnectedAccountChoice( + dockedAccountChoice.turnId, + connectionId, + ) + } + tray={attentionTray(`account:${dockedAccountChoice.turnId}`)} /> ) : null} {showQueuedStrip && queuedPrompt ? ( @@ -10605,8 +10789,8 @@ export function WorkflowCopilotChat({ onDrop={handleComposerDrop} className={cn( "relative flex items-end gap-1.5 rounded-lg border border-input bg-slate-elevation2 py-1.5 pl-3 pr-2 transition-colors focus-within:border-ring", - (showQueuedStrip || trayQuestion) && "rounded-t-none", - trayQuestion && "border-amber-500/50", + (showQueuedStrip || activeAttention) && "rounded-t-none", + activeAttention && "border-amber-500/50", )} > {isFileDragging ? ( diff --git a/skyvern-frontend/src/routes/workflows/copilot/cards/AttentionTray.tsx b/skyvern-frontend/src/routes/workflows/copilot/cards/AttentionTray.tsx new file mode 100644 index 000000000..9e384ee66 --- /dev/null +++ b/skyvern-frontend/src/routes/workflows/copilot/cards/AttentionTray.tsx @@ -0,0 +1,161 @@ +import type { HTMLAttributes, ReactNode } from "react"; +import { ChevronDownIcon, ChevronUpIcon } from "@radix-ui/react-icons"; + +import { Button } from "@/components/ui/button"; +import { cn } from "@/util/utils"; + +const TRAY_FRAME = + "rounded-t-lg border border-b-0 border-amber-500/50 bg-amber-500/[0.06]"; +const TRAY_TITLE = "font-semibold text-amber-700 dark:text-yellow-400"; + +// How the chat asks a request card to render itself as the docked tray. +export interface AttentionTrayPresentation { + collapsed: boolean; + onCollapsedChange: (collapsed: boolean) => void; + upNext?: string | null; +} + +type AttentionTrayProps = Omit, "title"> & { + title: ReactNode; + titleId?: string; + // For a request whose title names what it needs, so an ellipsis cannot drop the site or account. + wrapTitle?: boolean; + meta?: ReactNode; + collapsedTitle: ReactNode; + collapsedMeta?: ReactNode; + collapsed: boolean; + onCollapsedChange: (collapsed: boolean) => void; + minimizeLabel: string; + // Another request waiting behind this one; the chat shows one at a time. + upNext?: string | null; + children: ReactNode; +}; + +// The docked frame above the composer for anything Copilot is waiting on the user for. +export function AttentionTray({ + title, + titleId, + wrapTitle = false, + meta, + collapsedTitle, + collapsedMeta, + collapsed, + onCollapsedChange, + minimizeLabel, + upNext, + children, + className, + ...groupProps +}: AttentionTrayProps) { + if (collapsed) { + return ( +
+ + + {collapsedTitle} + + {collapsedMeta} + +
+ ); + } + + return ( +
+ {upNext ? ( +
+ + Up next · + + {upNext} + +
+ ) : null} +
+ + + {title} + +
+ {meta} + +
+
+ {children} +
+ ); +} + +// Where a docked request was raised in the transcript. While it is pending the answer happens in +// the tray, so this row only points there. +export function AttentionMarker({ + icon, + title, + hint, +}: { + icon: ReactNode; + title: string; + hint: string; +}) { + return ( +
+ {icon} + + {title} · {hint} + +
+ ); +} diff --git a/skyvern-frontend/src/routes/workflows/copilot/cards/ConnectedAccountChoiceCard.tsx b/skyvern-frontend/src/routes/workflows/copilot/cards/ConnectedAccountChoiceCard.tsx index f37909811..4dee73ae1 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/cards/ConnectedAccountChoiceCard.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/cards/ConnectedAccountChoiceCard.tsx @@ -1,6 +1,13 @@ +import type { KeyboardEvent } from "react"; import { CheckIcon, Link2Icon } from "@radix-ui/react-icons"; import type { ConnectedAccountChoice } from "../workflowCopilotTypes"; +import { + AttentionMarker, + AttentionTray, + type AttentionTrayPresentation, +} from "./AttentionTray"; +import { keyedChoiceFor, MAX_KEYED_CHOICES } from "./keyedChoice"; import { connectedAccountChoiceLabel } from "./connectedAccountChoiceLabel"; type ConnectedAccountChoiceCardProps = { @@ -8,6 +15,8 @@ type ConnectedAccountChoiceCardProps = { selectedConnectionId: string | null; disabled: boolean; onSelect: (connectionId: string) => void; + // Set when the chat docks this picker above the composer instead of in the transcript. + tray?: AttentionTrayPresentation; }; export function ConnectedAccountChoiceCard({ @@ -15,7 +24,108 @@ export function ConnectedAccountChoiceCard({ selectedConnectionId, disabled, onSelect, + tray, }: ConnectedAccountChoiceCardProps) { + // Only active rows send a choice; an inactive one is a link to reconnect it. + const keyed = tray + ? choices + .filter((choice) => choice.state === "active") + .slice(0, MAX_KEYED_CHOICES) + : []; + const rows = ( +
+ {choices.map((choice) => { + const selected = selectedConnectionId === choice.connection_id; + const accountLabel = connectedAccountChoiceLabel(choice, choices); + const keyIndex = keyed.indexOf(choice); + const content = ( + <> + {keyIndex >= 0 ? ( + + {keyIndex + 1} + + ) : ( + + )} + + + {choice.name} + + + {accountLabel} + + + + + {selected ? ( + + + ) : null} + + ); + if (choice.state !== "active") { + return ( + + {content} + + ); + } + return ( + + ); + })} +
+ ); + + if (tray) { + const onKeyDown = (event: KeyboardEvent) => { + if (disabled || selectedConnectionId !== null) return; + const choice = keyedChoiceFor(event, keyed); + if (!choice) return; + event.preventDefault(); + onSelect(choice.connection_id); + }; + return ( + +
{rows}
+ + ); + } + return (
-
- {choices.map((choice) => { - const selected = selectedConnectionId === choice.connection_id; - const accountLabel = connectedAccountChoiceLabel(choice, choices); - const content = ( - <> - - - - {choice.name} - - - {accountLabel} - - - - - {selected ? ( - - - ) : null} - - ); - if (choice.state !== "active") { - return ( - - {content} - - ); - } - return ( - - ); - })} -
+ {rows}
); } + +export function ConnectedAccountChoiceMarker() { + return ( + + } + title="Copilot needs a Google account" + hint="choose below" + /> + ); +} diff --git a/skyvern-frontend/src/routes/workflows/copilot/cards/CredentialCard.tsx b/skyvern-frontend/src/routes/workflows/copilot/cards/CredentialCard.tsx index 61c4b4ea4..6546de725 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/cards/CredentialCard.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/cards/CredentialCard.tsx @@ -4,6 +4,7 @@ import { useRef, useState, type ReactNode, + type RefObject, } from "react"; import { ChevronDownIcon, LockClosedIcon } from "@radix-ui/react-icons"; import { useDebounce } from "use-debounce"; @@ -38,6 +39,11 @@ import { CopilotCard, GutterRow, } from "./cardChrome"; +import { + AttentionMarker, + AttentionTray, + type AttentionTrayPresentation, +} from "./AttentionTray"; import { TURN_ROW_INSET } from "./cardLayout"; // Union of both a request-policy-time classifier's real reason tokens and a @@ -119,6 +125,8 @@ export interface CredentialCardProps { // "auto-bound" mode only: whether the Change affordance is live (the tail turn). A scrollback // receipt stays read-only so a pick can't optimistically resolve without an actual continuation. canChange?: boolean; + // Set when the chat docks this ask above the composer instead of in the transcript. + tray?: AttentionTrayPresentation; } const SIGN_IN_WHY_LINE = @@ -473,11 +481,13 @@ function CredentialUpdateAsk({ credentialId, onUpdateCredential, onSkip, + tray, }: { frame: CredentialRequiredFrame; credentialId: string; onUpdateCredential?: (credential: CredentialApiResponse) => void; onSkip: () => void; + tray?: AttentionTrayPresentation; }) { const { remainingMs, expired } = useCountdown(frame.expires_at ?? "", true); const [rootRef, insideLiveRegion] = useInsideLiveRegion(); @@ -495,29 +505,28 @@ function CredentialUpdateAsk({ const status = credential ? "" : (loadFailure ?? "Loading saved login…"); return ( -
- - - } - wrapTitle - title={ - rejected - ? `Update ${credential ? `'${credential.name}'` : "your saved login"} to sign in to ${site}` - : credential - ? `Add 2FA to '${credential.name}' to sign in to ${site}` - : `Add 2FA to your saved login for ${site}` - } - right={} - /> - - - - {CREDENTIAL_WHY_LINE_BY_REASON[frame.reason]} - - - - + } + lines={[ + + {CREDENTIAL_WHY_LINE_BY_REASON[frame.reason]} + , + ]} + footer={ + <>
- {/* Mounted for the card's lifetime so only its text changes; inside an outer live region - the visible status is already announced. */} - - {insideLiveRegion ? "" : status} - + + } + announcer={ + // Mounted for the card's lifetime so only its text changes; inside an outer live region + // the visible status is already announced. + + {insideLiveRegion ? "" : status} + + } + /> + ); +} + +// One unresolved ask, drawn either as a transcript card or as the tray docked above the composer. +// The tray leaves the assistant's words to the transcript marker, which sits where the ask was raised. +function AskChrome({ + tray, + rootRef, + message, + title, + countdown, + lines, + footer, + announcer, +}: { + tray?: AttentionTrayPresentation; + rootRef: RefObject; + message?: string; + title: string; + countdown: ReactNode; + lines: ReactNode[]; + footer: ReactNode; + // Outside the collapsible body, so a minimized tray still announces a failure. + announcer: ReactNode; +}) { + if (tray) { + return ( +
+ +
+ {lines.map((line, index) => ( +
{line}
+ ))} +
+
+ {footer} +
+ + {announcer} +
+ ); + } + return ( +
+ + + } + wrapTitle + title={title} + right={countdown} + /> + + {lines.map((line, index) => ( + {line} + ))} + + + {footer} + {announcer} +
+ ); +} + +// Where a docked ask was raised: the assistant's words for it, then a pointer to the tray. +export function CredentialAskMarker({ message }: { message?: string }) { + return ( +
+ + + } + title="Copilot needs to sign in" + hint="continue below" + />
); } @@ -631,6 +735,7 @@ export function CredentialCard(props: Readonly) { credentialId={updateTargetId} onUpdateCredential={props.onUpdateCredential} onSkip={props.onSkip} + tray={props.tray} /> ); } @@ -647,6 +752,7 @@ function CredentialAskCard({ reloadKey, autoBound, canChange = false, + tray, }: Readonly) { // Terminal mode never expires by design: its signal carries no timeout/expiry // semantics at all, so there is nothing to compare "now" against. Only a @@ -907,34 +1013,36 @@ function CredentialAskCard({ const site = siteFromLoginPageUrls(frame.login_page_urls); return ( -
- - - } - wrapTitle - title={`Copilot needs to sign in to ${site}`} - right={ - countdownActive ? ( - - ) : null - } - /> - - - - {CREDENTIAL_WHY_LINE_BY_REASON[frame.reason] ?? SIGN_IN_WHY_LINE} - - - {mode === "terminal" ? ( - - + + ) : null + } + lines={[ + + {CREDENTIAL_WHY_LINE_BY_REASON[frame.reason] ?? SIGN_IN_WHY_LINE} + , + ...(mode === "terminal" + ? [ + Connect a credential and I'll continue. - - - ) : null} - - + , + ] + : []), + ]} + footer={ + <>
- {/* Mounted for the card's lifetime so only its text changes: a live region that appears - already holding its text is read as ordinary new content and never announced. Inside - an outer live region it takes no role and repeats nothing that region already - contains, only the search failure, whose visible text sits in a portaled popover. */} - - {searchFailed - ? SEARCH_FAILED_ANNOUNCEMENT - : insideLiveRegion - ? "" - : orgCredentials.status === "loading" - ? "Loading saved logins…" - : orgCredentials.status === "error" - ? "Couldn't load your saved logins." - : ""} - - - - + + } + announcer={ + // Mounted for the card's lifetime so only its text changes: a live region that appears + // already holding its text is read as ordinary new content and never announced. Inside an + // outer live region it takes no role and repeats nothing that region already contains, + // only the search failure, whose visible text sits in a portaled popover. + + {searchFailed + ? SEARCH_FAILED_ANNOUNCEMENT + : insideLiveRegion + ? "" + : orgCredentials.status === "loading" + ? "Loading saved logins…" + : orgCredentials.status === "error" + ? "Couldn't load your saved logins." + : ""} + + } + /> ); } diff --git a/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionPartsCard.tsx b/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionPartsCard.tsx index 92b636746..8ae5a9cf5 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionPartsCard.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionPartsCard.tsx @@ -2,7 +2,7 @@ import { CheckIcon } from "@radix-ui/react-icons"; import { cn } from "@/util/utils"; import type { QuestionInteraction } from "../workflowCopilotTypes"; import { hasAnswer } from "./questionAnswers"; -import { MAX_KEYED_CHOICES } from "./QuestionTray"; +import { MAX_KEYED_CHOICES } from "./keyedChoice"; const TYPED_BUBBLE = "max-w-full self-start whitespace-pre-wrap break-words rounded-[10px] border border-border bg-slate-elevation4 dark:border-white/5 px-2.5 py-1.5 text-[12.5px] leading-[1.45] text-foreground"; diff --git a/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionTray.tsx b/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionTray.tsx index 92fa2770d..38a6fb200 100644 --- a/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionTray.tsx +++ b/skyvern-frontend/src/routes/workflows/copilot/cards/QuestionTray.tsx @@ -1,15 +1,11 @@ import { useEffect, useId, useRef, type KeyboardEvent } from "react"; -import { - CheckIcon, - ChevronDownIcon, - ChevronUpIcon, -} from "@radix-ui/react-icons"; +import { CheckIcon } from "@radix-ui/react-icons"; import { Button } from "@/components/ui/button"; import { cn } from "@/util/utils"; import type { QuestionStepper } from "../useQuestionStepper"; import type { QuestionInteraction } from "../workflowCopilotTypes"; - -export const MAX_KEYED_CHOICES = 9; +import { AttentionTray } from "./AttentionTray"; +import { keyedChoiceFor, MAX_KEYED_CHOICES } from "./keyedChoice"; // One tray renders at a time, so the composer can name the prompt it is answering. export const QUESTION_PROMPT_ID = "copilot-question-prompt"; @@ -26,6 +22,7 @@ export function QuestionTray({ onCancel, cancelDisabled, cancelTitle, + upNext, }: { interaction: QuestionInteraction; stepper: QuestionStepper; @@ -40,6 +37,7 @@ export function QuestionTray({ onCancel?: () => void; cancelDisabled?: boolean; cancelTitle?: string; + upNext?: string | null; }) { const titleId = useId(); const advanceRef = useRef(null); @@ -55,77 +53,35 @@ export function QuestionTray({ const part = interaction.parts[stepper.index]; const noun = total === 1 ? "question" : "questions"; - if (collapsed) { - return ( -
- - - Copilot is waiting on {total} {noun} - - -
- ); - } - const onKeyDown = (event: KeyboardEvent) => { if (disabled || !part || part.choices.length > MAX_KEYED_CHOICES) return; - if (event.metaKey || event.ctrlKey || event.altKey) return; - const choice = part.choices[Number(event.key) - 1]; - if (!/^[1-9]$/.test(event.key) || !choice) return; + const choice = keyedChoiceFor(event, part.choices); + if (!choice) return; event.preventDefault(); stepper.toggleChoice(part.part_id, choice.choice_id); }; return ( -
1 ? ( + + {stepper.index + 1} of {total} + + ) : null + } + collapsedTitle={`Copilot is waiting on ${total} ${noun}`} + collapsed={collapsed} + onCollapsedChange={onCollapsedChange} + minimizeLabel="Minimize question" + upNext={upNext} > -
- - - Copilot needs your answer - -
- {total > 1 ? ( - - {stepper.index + 1} of {total} - - ) : null} - -
-
{/* Stays mounted across steps so Back and Next read the new question to a screen reader whose focus is still in the composer. */} @@ -271,6 +227,6 @@ export function QuestionTray({ )}
- + ); } diff --git a/skyvern-frontend/src/routes/workflows/copilot/cards/keyedChoice.ts b/skyvern-frontend/src/routes/workflows/copilot/cards/keyedChoice.ts new file mode 100644 index 000000000..5121eb0e9 --- /dev/null +++ b/skyvern-frontend/src/routes/workflows/copilot/cards/keyedChoice.ts @@ -0,0 +1,13 @@ +import type { KeyboardEvent } from "react"; + +export const MAX_KEYED_CHOICES = 9; + +// The item a bare digit key picks from a tray's keyed list, or null for any other key. +export function keyedChoiceFor( + event: KeyboardEvent, + keyed: readonly T[], +): T | null { + if (event.metaKey || event.ctrlKey || event.altKey) return null; + if (!/^[1-9]$/.test(event.key)) return null; + return keyed[Number(event.key) - 1] ?? null; +} diff --git a/skyvern-frontend/src/routes/workflows/studio/CopilotPaneHeader.tsx b/skyvern-frontend/src/routes/workflows/studio/CopilotPaneHeader.tsx index 42fd7ac99..467832b36 100644 --- a/skyvern-frontend/src/routes/workflows/studio/CopilotPaneHeader.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/CopilotPaneHeader.tsx @@ -1,6 +1,9 @@ import { PlusIcon } from "@radix-ui/react-icons"; -import { useCopilotHeaderStore } from "@/store/useCopilotHeaderStore"; +import { + COPILOT_ATTENTION_CHIP, + useCopilotHeaderStore, +} from "@/store/useCopilotHeaderStore"; import { useRecordingStore } from "@/store/useRecordingStore"; import { cn } from "@/util/utils"; @@ -85,13 +88,20 @@ export function CopilotActiveDot() { ); } -export function CopilotRecordingStatus() { +export function CopilotPaneStatus() { const recording = useRecordingStore((state) => state.isRecording); const finishing = useRecordingStore( (state) => state.finishRequested || state.isCommitting, ); + const attention = useCopilotHeaderStore((state) => state.attention); if (!recording && !finishing) { - return null; + // The tray it points at can sit below the fold of a short pane, or under a collapsed line. + return attention ? ( + + + ) : null; } return ( diff --git a/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.test.tsx b/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.test.tsx index 43efc5a88..1c0bd87af 100644 --- a/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.test.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.test.tsx @@ -526,12 +526,12 @@ describe("StudioPaneToggles browser activity", () => { }); describe("StudioPaneToggles Copilot question", () => { - afterEach(() => useCopilotHeaderStore.getState().setAwaitingAnswer(false)); + afterEach(() => useCopilotHeaderStore.getState().setAttention(null)); test.each(["copilot", "browser"])( "flags the Copilot tab while a question waits (panes=%s)", (panes) => { - useCopilotHeaderStore.getState().setAwaitingAnswer(true); + useCopilotHeaderStore.getState().setAttention("question"); renderAt(`/workflows/wpid_abc/studio?panes=${panes}`); expect( screen.getByRole("button", { @@ -541,10 +541,18 @@ describe("StudioPaneToggles Copilot question", () => { }, ); + test("names a sign-in request instead of a question", () => { + useCopilotHeaderStore.getState().setAttention("credential"); + renderAt("/workflows/wpid_abc/studio?panes=browser"); + expect( + screen.getByRole("button", { name: "Copilot, needs to sign in" }), + ).toBeTruthy(); + }); + test("drops the flag once the question is answered", () => { - useCopilotHeaderStore.getState().setAwaitingAnswer(true); + useCopilotHeaderStore.getState().setAttention("question"); renderAt("/workflows/wpid_abc/studio?panes=copilot"); - act(() => useCopilotHeaderStore.getState().setAwaitingAnswer(false)); + act(() => useCopilotHeaderStore.getState().setAttention(null)); expect( screen.queryByRole("button", { name: "Copilot, waiting for your answer", diff --git a/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.tsx b/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.tsx index 5e4244f8b..b02f28b4f 100644 --- a/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/StudioPaneToggles.tsx @@ -18,7 +18,10 @@ import { TooltipContent, TooltipTrigger, } from "@/components/ui/tooltip"; -import { useCopilotHeaderStore } from "@/store/useCopilotHeaderStore"; +import { + COPILOT_ATTENTION_LABEL, + useCopilotHeaderStore, +} from "@/store/useCopilotHeaderStore"; import { useStudioBrowserStore } from "@/store/useStudioBrowserStore"; import { cn } from "@/util/utils"; @@ -93,7 +96,7 @@ export function StudioPaneToggles() { (s) => s.hasUnseenActivity, ); const clearBrowserActivity = useStudioBrowserStore((s) => s.clearActivity); - const copilotAwaitingAnswer = useCopilotHeaderStore((s) => s.awaitingAnswer); + const copilotAttention = useCopilotHeaderStore((s) => s.attention); const { runId, runStatus } = useStudioRunSignals(); const labelsCollapsed = useLabelsCollapsed(); @@ -176,13 +179,13 @@ export function StudioPaneToggles() { const disabled = blockedByDeletion; const showActivityDot = id === "browser" && hasUnseenBrowserActivity && !open; - // Shown even with the pane open: the question can still be off-screen behind other panes. - const showAwaitingDot = id === "copilot" && copilotAwaitingAnswer; + // Shown even with the pane open: the request can still be off-screen behind other panes. + const awaiting = id === "copilot" ? copilotAttention : null; const showRunStatusDot = isRunControl && Boolean(runStatus); const ariaLabel = showActivityDot ? "Browser, new activity" - : showAwaitingDot - ? "Copilot, waiting for your answer" + : awaiting + ? `Copilot, ${COPILOT_ATTENTION_LABEL[awaiting]}` : isRunControl && runStatus ? `${label}, ${runStatusLabel(runStatus)}` : label; @@ -199,10 +202,10 @@ export function StudioPaneToggles() { - ) : showAwaitingDot ? ( + ) : awaiting ? ( diff --git a/skyvern-frontend/src/routes/workflows/studio/StudioShell.tsx b/skyvern-frontend/src/routes/workflows/studio/StudioShell.tsx index b7a2ccebe..df51bc38c 100644 --- a/skyvern-frontend/src/routes/workflows/studio/StudioShell.tsx +++ b/skyvern-frontend/src/routes/workflows/studio/StudioShell.tsx @@ -38,7 +38,7 @@ import { BrowserPaneActions, BrowserPaneViewPills } from "./BrowserPaneHeader"; import { CopilotActiveDot, CopilotPaneControls, - CopilotRecordingStatus, + CopilotPaneStatus, } from "./CopilotPaneHeader"; import { EditorPaneBlockSearch, @@ -1082,7 +1082,7 @@ function StudioStage(props: StudioWorkspaceProps) { > } + headerExtras={} headerActions={} iconBadge={} > diff --git a/skyvern-frontend/src/store/useCopilotHeaderStore.ts b/skyvern-frontend/src/store/useCopilotHeaderStore.ts index a092e32a3..7df1eecb9 100644 --- a/skyvern-frontend/src/store/useCopilotHeaderStore.ts +++ b/skyvern-frontend/src/store/useCopilotHeaderStore.ts @@ -13,6 +13,21 @@ export type CopilotHeaderControls = { navigationLockedReason: string | null; }; +// What the docked Copilot chat is waiting on the user for. +export type CopilotAttention = "question" | "credential" | "account"; + +export const COPILOT_ATTENTION_LABEL: Record = { + question: "waiting for your answer", + credential: "needs to sign in", + account: "needs a Google account", +}; + +export const COPILOT_ATTENTION_CHIP: Record = { + question: "Needs your answer", + credential: "Needs sign-in", + account: "Choose an account", +}; + /** * Bridges the docked copilot chat and the studio's Copilot pane header: the * chat registers its History/New-chat controls here so the header (rendered @@ -22,15 +37,14 @@ export type CopilotHeaderControls = { type CopilotHeaderState = { controls: CopilotHeaderControls | null; setControls: (controls: CopilotHeaderControls | null) => void; - // A Copilot question is waiting on the user. The chat stays mounted while its pane is closed, - // so the studio's pane toggle can flag it. - awaitingAnswer: boolean; - setAwaitingAnswer: (awaitingAnswer: boolean) => void; + // The chat stays mounted while its pane is closed, so the studio's pane toggle can flag it. + attention: CopilotAttention | null; + setAttention: (attention: CopilotAttention | null) => void; }; export const useCopilotHeaderStore = create((set) => ({ controls: null, setControls: (controls) => set({ controls }), - awaitingAnswer: false, - setAwaitingAnswer: (awaitingAnswer) => set({ awaitingAnswer }), + attention: null, + setAttention: (attention) => set({ attention }), })); diff --git a/skyvern/forge/agent.py b/skyvern/forge/agent.py index f464a8277..8a2b85c1e 100644 --- a/skyvern/forge/agent.py +++ b/skyvern/forge/agent.py @@ -2270,45 +2270,31 @@ class ForgeAgent: from skyvern.forge.taskv3.workflow_position import PreviousBlockHandoff, select_previous_block from skyvern.utils.token_counter import approx_count_tokens - # Workflow-block tasks re-resolve the live working page on every tool call, so a click that - # opens a new tab/popup is followed (mirrors the step engine's get_working_page re-fetch). - # Bare tasks keep today's exact semantics: one page grabbed up front, for the run's duration. + # Every task re-resolves the live working page on every tool call, so a click that opens a + # new tab/popup is followed (mirrors the step engine's get_working_page re-fetch). + # Fail fast (with the recovery attempt must_get makes) when the page is already gone at the + # start; mid-run losses surface through the per-call provider instead. + await browser_state.must_get_working_page() # A block reuses the browser across blocks, so neither the landing status nor its URL belongs # to this block's start; only a bare task names a starting page in the dead-end verdict. initial_navigation_url: str | None = None - follows_working_page = task_block is not None or workflow_owned_recovery - if follows_working_page: - # Fail fast (with the same recovery attempt bare tasks get) when the page is already - # gone at block start; mid-run losses surface through the per-call provider instead. - await browser_state.must_get_working_page() - - async def _page_provider() -> Any: - # must_get (not get): recovers a crashed/closed page via _reopen_lost_working_page, - # matching the step engine's per-action re-acquisition; its raise on unrecoverable - # loss is contained by the loop's per-tool-call error handling. - return await browser_state.must_get_working_page() - - async def _fingerprint_page() -> Any: - # get (not must_get): recovery at finish time could navigate and induce duplicate - # actions, so a lost page yields None and the verdict is accepted as-is. - return await browser_state.get_working_page() - - else: - initial_page = await browser_state.must_get_working_page() + if task_block is None and not workflow_owned_recovery: # The pair setup recorded, not the page's URL now: the status belongs to the landed response # (page.goto returns the last redirect hop), and the settle and challenge-solver waits that # follow it give a client-side redirect time to move the page somewhere the status was never # about. Reading the page here would name that destination as the page that 404ed. initial_navigation_url = browser_state.last_navigation_url - async def _page_provider() -> Any: - return initial_page + async def _page_provider() -> Any: + # must_get (not get): recovers a crashed/closed page via _reopen_lost_working_page, + # matching the step engine's per-action re-acquisition; its raise on unrecoverable + # loss is contained by the loop's per-tool-call error handling. + return await browser_state.must_get_working_page() - async def _fingerprint_page() -> Any: - # The fingerprint MUST sample the page the tools act on. Bare tasks pin one page for - # the run, and browser_state.get_working_page() would both return the newest tab - # (wrong page after any popup) and repoint the working page as a side effect. - return None if initial_page.is_closed() else initial_page + async def _fingerprint_page() -> Any: + # get (not must_get): recovery at finish time could navigate and induce duplicate + # actions, so a lost page yields None and the verdict is accepted as-is. + return await browser_state.get_working_page() llm_caller = LLMCaller(llm_key=await _resolve_task_v3_llm_key(task)) workflow_run_context = ( @@ -2991,8 +2977,7 @@ class ForgeAgent: async def _reload_page() -> None: # Observed by the action policy like the legacy internal refresh; the loop records it in - # the action round. A bare task pins one page, so it is passed explicitly. - pinned = None if task_block is not None else await _page_provider() + # the action round. preflight_action( ReloadPageAction( reasoning="a page-level handler requested a refresh", @@ -3001,10 +2986,10 @@ class ForgeAgent: task_id=task.task_id, step_id=step.step_id, ), - pinned if pinned is not None else await _fingerprint_page(), + await _fingerprint_page(), site="internal_refresh", ) - await browser_state.reload_page(page=pinned) + await browser_state.reload_page() async def _restore_page_url(page: Any, url: str) -> None: # Not `reload_page`: the tab is blanked in place, so reloading it reloads `about:blank`. @@ -3289,8 +3274,7 @@ class ForgeAgent: elif not page_free_validation and not download_finalized: blank_page = None try: - # A block's tools act on the working page the gate just read; a bare task's on its pinned page. - first_sample = gate_page if follows_working_page else await _fingerprint_page() + first_sample = gate_page async def _settle_should_stop() -> bool: if time.monotonic() >= loop_deadline_at: diff --git a/tests/unit/test_agent_task_v3.py b/tests/unit/test_agent_task_v3.py index 121385eaa..043daa0d1 100644 --- a/tests/unit/test_agent_task_v3.py +++ b/tests/unit/test_agent_task_v3.py @@ -119,6 +119,10 @@ async def _run_execute_task_v3( provider_probe_calls: int = 0, get_working_page_side_effect: list[Any] | None = None, must_get_working_page_side_effect: BaseException | list[Any] | None = None, + # Both page accessors return this page, for tests that probe what the run's closures read. + working_page: Any = None, + # A real browser state whose accessors stand in for the mocked ones. + real_browser_state: Any = None, loop_raises: BaseException | None = None, update_task_side_effect: BaseException | None = None, completion_gate_vetoes: bool = False, @@ -132,14 +136,12 @@ async def _run_execute_task_v3( page_url_after_settle: str | None = None, # Where the page is once the loop returns -- the loop itself can leave the tab somewhere else. page_url_after_loop: str | None = None, - # A tab the run opened that is newer than the page the tools act on, at this URL once the loop returns. - newest_tab_url_after_loop: str | None = None, - pinned_page_open: bool = False, # The block's own (url, navigation_goal) as the AUTHOR typed them, pinned through the real block # seam before the render that produced the task fields above. unrendered_block_fields: tuple[str | None, str | None] | None = None, # Called with the loop's kwargs before the loop returns, to drive state the loop's tools own. on_loop: Callable[[dict[str, Any]], None] | None = None, + on_loop_async: Callable[[dict[str, Any]], Awaitable[None]] | None = None, # A prompt the fake loop hands the goal judge, the way the finish gate would, when one was built. goal_judge_prompt: str | None = None, # Leave the credential-TOTP candidate gate reading the real workflow-run context. @@ -153,8 +155,6 @@ async def _run_execute_task_v3( step = make_step(now, task, step_id="step-v3", status=StepStatus.created, order=0, output=None) browser_state, _, page = make_browser_state() - if pinned_page_open: - page.is_closed = MagicMock(return_value=False) # What setup's navigation RECORDED: the response's own URL beside the status it came back with, # which after a redirect is not the URL it asked for. browser_state.last_navigation_status = navigation_status @@ -167,6 +167,12 @@ async def _run_execute_task_v3( browser_state.get_working_page = AsyncMock(side_effect=get_working_page_side_effect) else: browser_state.get_working_page = AsyncMock(return_value=page) + if working_page is not None: + browser_state.must_get_working_page = AsyncMock(return_value=working_page) + browser_state.get_working_page = AsyncMock(return_value=working_page) + if real_browser_state is not None: + browser_state.must_get_working_page = AsyncMock(side_effect=real_browser_state.must_get_working_page) + browser_state.get_working_page = AsyncMock(side_effect=real_browser_state.get_working_page) browser_state.take_post_action_screenshot = AsyncMock( return_value=b"png-bytes", side_effect=RuntimeError("screenshot boom") if screenshot_raises else None, @@ -187,6 +193,8 @@ async def _run_execute_task_v3( await cb(round_actions, turn_text) if on_loop is not None: on_loop(kwargs) + if on_loop_async is not None: + await on_loop_async(kwargs) if goal_judge_prompt is not None and kwargs.get("goal_judge") is not None: page.is_closed = MagicMock(return_value=False) loop_mock.goal_judge_response = await kwargs["goal_judge"](goal_judge_prompt) @@ -197,10 +205,6 @@ async def _run_execute_task_v3( raise loop_raises if page_url_after_loop is not None: page.url = page_url_after_loop - if newest_tab_url_after_loop is not None: - newest_tab = MagicMock(url=newest_tab_url_after_loop) - newest_tab.main_frame.child_frames = [] - browser_state.get_working_page = AsyncMock(return_value=newest_tab) return outcome loop_mock = AsyncMock(side_effect=_loop) @@ -3137,29 +3141,10 @@ async def test_execute_task_v3_should_cancel_skips_workflow_read_for_bare_task( # --------------------------------------------------------------------------- -# P4: the page provider (live re-resolution for workflow blocks, once for bare tasks) +# P4: the page provider (live re-resolution on every tool call) # --------------------------------------------------------------------------- -@pytest.mark.asyncio -async def test_execute_task_v3_bare_task_provider_resolves_page_once(monkeypatch: pytest.MonkeyPatch) -> None: - # A bare task must preserve today's exact semantics: must_get_working_page grabs the page once - # up front, and every later provider call returns that same object, not a re-resolved one. - outcome = LoopOutcome(status="completed", reason="done", billable_actions=[]) - _step, _task, loop_mock, _post = await _run_execute_task_v3( - monkeypatch, - outcome, - provider_probe_calls=3, - data_extraction_goal=None, - extracted_information_schema=None, - ) - assert loop_mock.resolved_pages == [loop_mock.resolved_pages[0]] * 3 - loop_mock.browser_state.must_get_working_page.assert_awaited_once() - # The completion gate reads the page once on a completed outcome; the PROVIDER itself never - # consults get_working_page for a bare task. - assert loop_mock.browser_state.get_working_page.await_count <= 1 - - @pytest.mark.asyncio async def test_a_workflow_owned_recovery_keeps_its_caller_s_step_cap(monkeypatch: pytest.MonkeyPatch) -> None: # The code block AI fallback sizes its own budget from the org's cap, so the v3 floor must not @@ -3946,36 +3931,6 @@ async def test_execute_task_v3_completed_on_a_page_still_loading_completes(monke assert task.status == TaskStatus.completed -@pytest.mark.asyncio -async def test_execute_task_v3_bare_task_judges_its_pinned_page_not_the_newest_tab( - monkeypatch: pytest.MonkeyPatch, -) -> None: - # A bare task's tools act on one pinned page; a blank popup it opened along the way is not that page. - monkeypatch.setattr(agent_module, "asyncio", ScopedAsyncio(sleep=AsyncMock())) - outcome = LoopOutcome(status="completed", reason="done", billable_actions=["click"]) - _step, task, _loop_mock, _post = await _run_execute_task_v3( - monkeypatch, - outcome, - pinned_page_open=True, - newest_tab_url_after_loop="about:blank", - data_extraction_goal=None, - extracted_information_schema=None, - ) - assert task.status == TaskStatus.completed - - _step, task, _loop_mock, _post = await _run_execute_task_v3( - monkeypatch, - outcome, - pinned_page_open=True, - page_url_after_loop="about:blank", - newest_tab_url_after_loop="https://example.com/results", - data_extraction_goal=None, - extracted_information_schema=None, - ) - assert task.status == TaskStatus.failed - assert "blank page" in (task.failure_reason or "") - - @pytest.mark.asyncio async def test_v3_blank_page_settle_stops_at_the_deadline_and_judges_the_last_sample( monkeypatch: pytest.MonkeyPatch, @@ -5091,44 +5046,6 @@ async def test_execute_task_v3_settle_completion_fenced_to_block_tasks( assert bare_loop_mock.await_args.kwargs["max_settle_deferrals"] == 0 -@pytest.mark.asyncio -async def test_execute_task_v3_bare_task_fingerprint_samples_the_pinned_page( - monkeypatch: pytest.MonkeyPatch, -) -> None: - # A bare task pins one page for the run, so its fingerprint must sample THAT page. Going through - # browser_state.get_working_page() would return the newest tab after any popup — sampling a page - # the model never acted on — and would repoint the working page as a side effect, which is a - # behaviour change to the live bare-task arm rather than the scoped one this gate intends. - outcome = LoopOutcome(status="completed", reason="done", billable_actions=[]) - pinned = MagicMock() - pinned.is_closed = MagicMock(return_value=False) - pinned.evaluate = AsyncMock(return_value="pinned-hash:100:10") - popup = MagicMock() - popup.is_closed = MagicMock(return_value=False) - popup.evaluate = AsyncMock(return_value="popup-hash:1:1") - - _step, _task, loop_mock, _post = await _run_execute_task_v3( - monkeypatch, - outcome, - must_get_working_page_side_effect=[pinned], - get_working_page_side_effect=[popup, popup, popup], - data_extraction_goal=None, - extracted_information_schema=None, - ) - fingerprint = loop_mock.await_args.kwargs["page_fingerprint"] - before = loop_mock.browser_state.get_working_page.await_count - assert await fingerprint() == "pinned-hash:100:10" - # The sampler probed the pinned page and never the popup, and did not consult (or repoint) the - # browser's working page to do it. Counting the delta rather than asserting never-awaited: the - # post-loop completion-veto gate legitimately calls get_working_page once, before this point. - popup.evaluate.assert_not_awaited() - assert loop_mock.browser_state.get_working_page.await_count == before - - # A closed pinned page yields None rather than silently falling back to another tab. - pinned.is_closed = MagicMock(return_value=True) - assert await fingerprint() is None - - @pytest.mark.asyncio async def test_execute_task_v3_page_fingerprint_samples_child_frames(monkeypatch: pytest.MonkeyPatch) -> None: # The settle deferral is a live gate: a main-frame-only fingerprint reads a page whose child frame @@ -5152,7 +5069,7 @@ async def test_execute_task_v3_page_fingerprint_samples_child_frames(monkeypatch _step, _task, loop_mock, _post = await _run_execute_task_v3( monkeypatch, outcome, - must_get_working_page_side_effect=[pinned], + working_page=pinned, data_extraction_goal=None, extracted_information_schema=None, ) @@ -5200,7 +5117,7 @@ async def test_execute_task_v3_document_identity_changes_with_an_acted_in_frame_ _step, _task, loop_mock, _post = await _run_execute_task_v3( monkeypatch, outcome, - must_get_working_page_side_effect=[pinned], + working_page=pinned, data_extraction_goal=None, extracted_information_schema=None, ) @@ -5279,7 +5196,7 @@ async def test_execute_task_v3_document_identity_raises_when_a_realm_is_unidenti _step, _task, loop_mock, _post = await _run_execute_task_v3( monkeypatch, outcome, - must_get_working_page_side_effect=[pinned], + working_page=pinned, data_extraction_goal=None, extracted_information_schema=None, ) @@ -5302,7 +5219,7 @@ async def test_execute_task_v3_document_identity_raises_when_a_realm_is_unidenti _step, _task, loop_mock2, _post = await _run_execute_task_v3( monkeypatch, outcome, - must_get_working_page_side_effect=[pinned2], + working_page=pinned2, data_extraction_goal=None, extracted_information_schema=None, )