mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(SKY-11623): preserve /discover prompt in Copilot handoff (#7216)
This commit is contained in:
parent
d72b6a1ba0
commit
ac7ffaf6eb
5 changed files with 424 additions and 5 deletions
|
|
@ -23,6 +23,10 @@ const { mockNavigate, mockPost, mockSetAutoplay } = vi.hoisted(() => ({
|
|||
mockSetAutoplay: vi.fn(),
|
||||
}));
|
||||
|
||||
const { studioState } = vi.hoisted(() => ({
|
||||
studioState: { enabled: false },
|
||||
}));
|
||||
|
||||
vi.mock("@/api/AxiosClient", () => ({
|
||||
getClient: async () => ({
|
||||
post: mockPost,
|
||||
|
|
@ -33,6 +37,10 @@ vi.mock("@/hooks/useCredentialGetter", () => ({
|
|||
useCredentialGetter: () => undefined,
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/useWorkflowStudioEnabled", () => ({
|
||||
useWorkflowStudioEnabled: () => studioState.enabled,
|
||||
}));
|
||||
|
||||
vi.mock("@/store/useAutoplayStore", () => ({
|
||||
useAutoplayStore: () => ({
|
||||
setAutoplay: mockSetAutoplay,
|
||||
|
|
@ -136,6 +144,8 @@ function renderPromptBox(enableCopilotHandoff = false) {
|
|||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
sessionStorage.clear();
|
||||
studioState.enabled = false;
|
||||
mockNavigate.mockReset();
|
||||
mockPost.mockReset();
|
||||
mockSetAutoplay.mockReset();
|
||||
|
|
@ -170,7 +180,7 @@ describe("PromptBox", () => {
|
|||
expect(body.request.url).toBe("https://google.com");
|
||||
});
|
||||
|
||||
test("hands Discover prompts to workflow copilot with agent execution", async () => {
|
||||
test("hands Discover prompts to the legacy build path via route state only", async () => {
|
||||
mockPost.mockResolvedValue({
|
||||
data: {
|
||||
workflow_permanent_id: "wpid_copilot",
|
||||
|
|
@ -199,5 +209,39 @@ describe("PromptBox", () => {
|
|||
state: { copilotMessage: "Build this workflow" },
|
||||
},
|
||||
);
|
||||
// The legacy /build (Debugger) surface never mounts the recovery hook, so
|
||||
// no session-scoped copy is written — writing one would strand a dead entry.
|
||||
expect(
|
||||
sessionStorage.getItem("skyvern.discoverCopilotHandoff:wpid_copilot"),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
test("hands Discover prompts to workflow studio with recoverable prompt state", async () => {
|
||||
studioState.enabled = true;
|
||||
mockPost.mockResolvedValue({
|
||||
data: {
|
||||
workflow_permanent_id: "wpid_studio",
|
||||
workflow_definition: { blocks: [] },
|
||||
},
|
||||
});
|
||||
|
||||
renderPromptBox(true);
|
||||
|
||||
fireEvent.change(screen.getByPlaceholderText("Enter your prompt..."), {
|
||||
target: { value: "Build this in studio" },
|
||||
});
|
||||
fireEvent.click(screen.getByLabelText("submit-prompt"));
|
||||
|
||||
await waitFor(() => expect(mockPost).toHaveBeenCalledTimes(1));
|
||||
|
||||
expect(mockNavigate).toHaveBeenCalledWith(
|
||||
"/agents/wpid_studio/studio?via=discover",
|
||||
{
|
||||
state: { copilotMessage: "Build this in studio" },
|
||||
},
|
||||
);
|
||||
expect(
|
||||
sessionStorage.getItem("skyvern.discoverCopilotHandoff:wpid_studio"),
|
||||
).toBe("Build this in studio");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ import { SpeechInputButton } from "@/components/SpeechInputButton";
|
|||
import { cn } from "@/util/utils";
|
||||
import { useSpeechToTextField } from "@/hooks/useSpeechToTextField";
|
||||
import { useWorkflowStudioEnabled } from "@/hooks/useWorkflowStudioEnabled";
|
||||
import { rememberDiscoverCopilotPrompt } from "@/routes/workflows/discoverCopilotHandoff";
|
||||
import { workflowEditorPath } from "@/routes/workflows/studioNavigation";
|
||||
|
||||
const exampleCases = [
|
||||
|
|
@ -280,6 +281,11 @@ function PromptBox({ enableCopilotHandoff = false }: PromptBoxProps) {
|
|||
onSuccess: ({ data: workflow, prompt }) => {
|
||||
queryClient.invalidateQueries({ queryKey: ["workflows"] });
|
||||
queryClient.invalidateQueries({ queryKey: ["folders"] });
|
||||
// Only the studio handoff writes the recovery key. The legacy /build path
|
||||
// can mount Workspace, but it never consumes this stored discover seed.
|
||||
if (studioEnabled) {
|
||||
rememberDiscoverCopilotPrompt(workflow.workflow_permanent_id, prompt);
|
||||
}
|
||||
// `?via=discover` is what makes WorkflowEditor fire
|
||||
// `copilot.discover.started` with entry_point=discover on mount.
|
||||
navigate(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,217 @@
|
|||
import {
|
||||
act,
|
||||
cleanup,
|
||||
fireEvent,
|
||||
render,
|
||||
renderHook,
|
||||
screen,
|
||||
waitFor,
|
||||
} from "@testing-library/react";
|
||||
import { createElement, useEffect, useMemo, useRef, useState } from "react";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
forgetDiscoverCopilotPrompt,
|
||||
readDiscoverCopilotPrompt,
|
||||
rememberDiscoverCopilotPrompt,
|
||||
useDiscoverCopilotPromptRecovery,
|
||||
withoutDiscoverViaParam,
|
||||
} from "./discoverCopilotHandoff";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
sessionStorage.clear();
|
||||
});
|
||||
|
||||
function InitialMessageHarness({ sentMessages }: { sentMessages: string[] }) {
|
||||
const [routeMessage, setRouteMessage] = useState<string | null>(
|
||||
"Discover seed",
|
||||
);
|
||||
const { storedInitialCopilotMessage, clearStoredInitialCopilotMessage } =
|
||||
useDiscoverCopilotPromptRecovery({
|
||||
shouldRead: true,
|
||||
workflowPermanentId: "wpid_1",
|
||||
});
|
||||
const initialMessage = useMemo(
|
||||
() => routeMessage ?? storedInitialCopilotMessage,
|
||||
[routeMessage, storedInitialCopilotMessage],
|
||||
);
|
||||
const hasAutoSentRef = useRef(false);
|
||||
|
||||
useEffect(() => {
|
||||
hasAutoSentRef.current = false;
|
||||
}, [initialMessage]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!initialMessage || hasAutoSentRef.current) {
|
||||
return;
|
||||
}
|
||||
hasAutoSentRef.current = true;
|
||||
sentMessages.push(initialMessage);
|
||||
clearStoredInitialCopilotMessage();
|
||||
setRouteMessage(null);
|
||||
}, [clearStoredInitialCopilotMessage, initialMessage, sentMessages]);
|
||||
|
||||
return createElement(
|
||||
"button",
|
||||
{ onClick: () => setRouteMessage("Fix seed"), type: "button" },
|
||||
"send fix seed",
|
||||
);
|
||||
}
|
||||
|
||||
function RecoveryHarness({
|
||||
shouldRead = true,
|
||||
workflowPermanentId,
|
||||
}: {
|
||||
shouldRead?: boolean;
|
||||
workflowPermanentId: string | undefined;
|
||||
}) {
|
||||
const { storedInitialCopilotMessage, clearStoredInitialCopilotMessage } =
|
||||
useDiscoverCopilotPromptRecovery({
|
||||
shouldRead,
|
||||
workflowPermanentId,
|
||||
});
|
||||
|
||||
return createElement(
|
||||
"div",
|
||||
null,
|
||||
createElement(
|
||||
"span",
|
||||
{ "data-testid": "stored-prompt" },
|
||||
storedInitialCopilotMessage ?? "",
|
||||
),
|
||||
createElement(
|
||||
"button",
|
||||
{ onClick: clearStoredInitialCopilotMessage, type: "button" },
|
||||
"clear stored prompt",
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
describe("discoverCopilotHandoff", () => {
|
||||
it("stores and clears the prompt for one workflow", () => {
|
||||
rememberDiscoverCopilotPrompt("wpid_1", "Build the workflow");
|
||||
|
||||
expect(readDiscoverCopilotPrompt("wpid_1")).toBe("Build the workflow");
|
||||
expect(readDiscoverCopilotPrompt("wpid_1")).toBe("Build the workflow");
|
||||
|
||||
forgetDiscoverCopilotPrompt("wpid_1");
|
||||
expect(readDiscoverCopilotPrompt("wpid_1")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps prompts scoped by workflow id", () => {
|
||||
rememberDiscoverCopilotPrompt("wpid_1", "First prompt");
|
||||
rememberDiscoverCopilotPrompt("wpid_2", "Second prompt");
|
||||
|
||||
expect(readDiscoverCopilotPrompt("wpid_2")).toBe("Second prompt");
|
||||
expect(readDiscoverCopilotPrompt("wpid_1")).toBe("First prompt");
|
||||
});
|
||||
|
||||
it("reads a new stored prompt when the recovery workflow changes", async () => {
|
||||
rememberDiscoverCopilotPrompt("wpid_1", "First prompt");
|
||||
rememberDiscoverCopilotPrompt("wpid_2", "Second prompt");
|
||||
|
||||
const { rerender } = render(
|
||||
createElement(RecoveryHarness, { workflowPermanentId: "wpid_1" }),
|
||||
);
|
||||
|
||||
expect(screen.getByTestId("stored-prompt").textContent).toBe(
|
||||
"First prompt",
|
||||
);
|
||||
|
||||
rerender(createElement(RecoveryHarness, { workflowPermanentId: "wpid_2" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("stored-prompt").textContent).toBe(
|
||||
"Second prompt",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores a stored prompt when recovery reads are disabled", () => {
|
||||
rememberDiscoverCopilotPrompt("wpid_1", "Stored prompt");
|
||||
|
||||
render(
|
||||
createElement(RecoveryHarness, {
|
||||
shouldRead: false,
|
||||
workflowPermanentId: "wpid_1",
|
||||
}),
|
||||
);
|
||||
|
||||
expect(screen.getByTestId("stored-prompt").textContent).toBe("");
|
||||
expect(readDiscoverCopilotPrompt("wpid_1")).toBe("Stored prompt");
|
||||
});
|
||||
|
||||
it("does not restore a consumed Discover prompt after a later Fix seed", async () => {
|
||||
rememberDiscoverCopilotPrompt("wpid_1", "Discover seed");
|
||||
const sentMessages: string[] = [];
|
||||
|
||||
render(createElement(InitialMessageHarness, { sentMessages }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(sentMessages).toEqual(["Discover seed"]);
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "send fix seed" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(sentMessages).toEqual(["Discover seed", "Fix seed"]);
|
||||
});
|
||||
expect(readDiscoverCopilotPrompt("wpid_1")).toBeNull();
|
||||
});
|
||||
|
||||
it("does not resurrect a consumed prompt when the recovery hook remounts", () => {
|
||||
rememberDiscoverCopilotPrompt("wpid_1", "Discover seed");
|
||||
|
||||
const first = renderHook(() =>
|
||||
useDiscoverCopilotPromptRecovery({
|
||||
shouldRead: true,
|
||||
workflowPermanentId: "wpid_1",
|
||||
}),
|
||||
);
|
||||
expect(first.result.current.storedInitialCopilotMessage).toBe(
|
||||
"Discover seed",
|
||||
);
|
||||
|
||||
act(() => {
|
||||
first.result.current.clearStoredInitialCopilotMessage();
|
||||
});
|
||||
expect(first.result.current.storedInitialCopilotMessage).toBeNull();
|
||||
expect(readDiscoverCopilotPrompt("wpid_1")).toBeNull();
|
||||
first.unmount();
|
||||
|
||||
// A fresh mount re-runs the useState initializer that reads sessionStorage;
|
||||
// clearing must have removed the entry so the consumed prompt stays gone.
|
||||
const second = renderHook(() =>
|
||||
useDiscoverCopilotPromptRecovery({
|
||||
shouldRead: true,
|
||||
workflowPermanentId: "wpid_1",
|
||||
}),
|
||||
);
|
||||
expect(second.result.current.storedInitialCopilotMessage).toBeNull();
|
||||
second.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
describe("withoutDiscoverViaParam", () => {
|
||||
it("strips a via=discover param but keeps other params", () => {
|
||||
expect(withoutDiscoverViaParam("?via=discover&cache-key-value=abc")).toBe(
|
||||
"?cache-key-value=abc",
|
||||
);
|
||||
});
|
||||
|
||||
it("drops the leading '?' when no params remain", () => {
|
||||
expect(withoutDiscoverViaParam("?via=discover")).toBe("");
|
||||
});
|
||||
|
||||
it("leaves non-discover via values untouched", () => {
|
||||
expect(withoutDiscoverViaParam("?via=onboarding")).toBe("?via=onboarding");
|
||||
});
|
||||
|
||||
it("is a no-op when there is no via param", () => {
|
||||
expect(withoutDiscoverViaParam("?cache-key-value=abc")).toBe(
|
||||
"?cache-key-value=abc",
|
||||
);
|
||||
expect(withoutDiscoverViaParam("")).toBe("");
|
||||
});
|
||||
});
|
||||
132
skyvern-frontend/src/routes/workflows/discoverCopilotHandoff.ts
Normal file
132
skyvern-frontend/src/routes/workflows/discoverCopilotHandoff.ts
Normal file
|
|
@ -0,0 +1,132 @@
|
|||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
|
||||
const DISCOVER_COPILOT_HANDOFF_STORAGE_PREFIX =
|
||||
"skyvern.discoverCopilotHandoff";
|
||||
|
||||
function storageKey(workflowPermanentId: string): string {
|
||||
return `${DISCOVER_COPILOT_HANDOFF_STORAGE_PREFIX}:${workflowPermanentId}`;
|
||||
}
|
||||
|
||||
function getSessionStorage(): Storage | null {
|
||||
if (typeof window === "undefined") {
|
||||
return null;
|
||||
}
|
||||
return window.sessionStorage;
|
||||
}
|
||||
|
||||
function recoveryWorkflowId(
|
||||
shouldRead: boolean,
|
||||
workflowPermanentId: string | undefined,
|
||||
): string | null {
|
||||
return shouldRead && workflowPermanentId ? workflowPermanentId : null;
|
||||
}
|
||||
|
||||
export function rememberDiscoverCopilotPrompt(
|
||||
workflowPermanentId: string | undefined,
|
||||
prompt: string,
|
||||
): void {
|
||||
if (!workflowPermanentId || !prompt) {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
getSessionStorage()?.setItem(storageKey(workflowPermanentId), prompt);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
export function readDiscoverCopilotPrompt(
|
||||
workflowPermanentId: string | undefined,
|
||||
): string | null {
|
||||
if (!workflowPermanentId) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// sessionStorage access can throw SecurityError in some private-browsing
|
||||
// modes; the catch below degrades to the pre-fix manual-retype path.
|
||||
try {
|
||||
const storage = getSessionStorage();
|
||||
if (!storage) {
|
||||
return null;
|
||||
}
|
||||
return storage.getItem(storageKey(workflowPermanentId));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export function useDiscoverCopilotPromptRecovery({
|
||||
shouldRead,
|
||||
workflowPermanentId,
|
||||
}: {
|
||||
shouldRead: boolean;
|
||||
workflowPermanentId: string | undefined;
|
||||
}) {
|
||||
const initialRecoveryWorkflowId = recoveryWorkflowId(
|
||||
shouldRead,
|
||||
workflowPermanentId,
|
||||
);
|
||||
const recoveryWorkflowIdRef = useRef(initialRecoveryWorkflowId);
|
||||
const [storedInitialCopilotMessage, setStoredInitialCopilotMessage] =
|
||||
useState(() =>
|
||||
initialRecoveryWorkflowId
|
||||
? readDiscoverCopilotPrompt(initialRecoveryWorkflowId)
|
||||
: null,
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
const nextRecoveryWorkflowId = recoveryWorkflowId(
|
||||
shouldRead,
|
||||
workflowPermanentId,
|
||||
);
|
||||
if (recoveryWorkflowIdRef.current === nextRecoveryWorkflowId) {
|
||||
return;
|
||||
}
|
||||
recoveryWorkflowIdRef.current = nextRecoveryWorkflowId;
|
||||
setStoredInitialCopilotMessage(
|
||||
nextRecoveryWorkflowId
|
||||
? readDiscoverCopilotPrompt(nextRecoveryWorkflowId)
|
||||
: null,
|
||||
);
|
||||
}, [shouldRead, workflowPermanentId]);
|
||||
|
||||
const clearStoredInitialCopilotMessage = useCallback(() => {
|
||||
recoveryWorkflowIdRef.current = null;
|
||||
setStoredInitialCopilotMessage(null);
|
||||
forgetDiscoverCopilotPrompt(workflowPermanentId);
|
||||
}, [workflowPermanentId]);
|
||||
|
||||
return {
|
||||
clearStoredInitialCopilotMessage,
|
||||
storedInitialCopilotMessage,
|
||||
};
|
||||
}
|
||||
|
||||
// Drops `via=discover` from a location.search string once the seed prompt it
|
||||
// pointed at has been consumed, so a later remount of the editor (e.g. via
|
||||
// browser back/forward) can't make useViaEntryPointCapture re-fire
|
||||
// copilot.discover.started for a turn that already happened.
|
||||
export function withoutDiscoverViaParam(search: string): string {
|
||||
const params = new URLSearchParams(search);
|
||||
if (params.get("via") !== "discover") {
|
||||
return search;
|
||||
}
|
||||
params.delete("via");
|
||||
const next = params.toString();
|
||||
return next ? `?${next}` : "";
|
||||
}
|
||||
|
||||
export function forgetDiscoverCopilotPrompt(
|
||||
workflowPermanentId: string | undefined,
|
||||
): void {
|
||||
if (!workflowPermanentId) {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
getSessionStorage()?.removeItem(storageKey(workflowPermanentId));
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
|
@ -172,6 +172,10 @@ import { constructCacheKeyValue, getInitialParameters } from "./utils";
|
|||
import { WorkflowCopilotChat } from "../copilot/WorkflowCopilotChat";
|
||||
import { useStudioRunId } from "../studio/useStudioRunId";
|
||||
import { copilotRunId } from "./copilotRunId";
|
||||
import {
|
||||
useDiscoverCopilotPromptRecovery,
|
||||
withoutDiscoverViaParam,
|
||||
} from "../discoverCopilotHandoff";
|
||||
import {
|
||||
initialEditorAutoOpenState,
|
||||
shouldAutoOpenEditor,
|
||||
|
|
@ -365,23 +369,39 @@ function Workspace({
|
|||
);
|
||||
const location = useLocation();
|
||||
const navigate = useNavigate();
|
||||
const [searchParams] = useSearchParams();
|
||||
const locationState = location.state as {
|
||||
copilotMessage?: unknown;
|
||||
copilotFixOrigin?: unknown;
|
||||
} | null;
|
||||
const initialCopilotMessage =
|
||||
const routeInitialCopilotMessage =
|
||||
typeof locationState?.copilotMessage === "string"
|
||||
? locationState.copilotMessage
|
||||
: null;
|
||||
const { storedInitialCopilotMessage, clearStoredInitialCopilotMessage } =
|
||||
useDiscoverCopilotPromptRecovery({
|
||||
shouldRead: searchParams.get("via") === "discover",
|
||||
workflowPermanentId,
|
||||
});
|
||||
const initialCopilotMessage = useMemo(
|
||||
() => routeInitialCopilotMessage ?? storedInitialCopilotMessage,
|
||||
[routeInitialCopilotMessage, storedInitialCopilotMessage],
|
||||
);
|
||||
const initialCopilotFixOrigin = locationState?.copilotFixOrigin === true;
|
||||
const handleInitialCopilotMessageConsumed = useCallback(() => {
|
||||
if (!initialCopilotMessage) return;
|
||||
navigate(location.pathname + location.search, {
|
||||
clearStoredInitialCopilotMessage();
|
||||
navigate(location.pathname + withoutDiscoverViaParam(location.search), {
|
||||
replace: true,
|
||||
state: null,
|
||||
});
|
||||
}, [initialCopilotMessage, location.pathname, location.search, navigate]);
|
||||
const [searchParams] = useSearchParams();
|
||||
}, [
|
||||
initialCopilotMessage,
|
||||
clearStoredInitialCopilotMessage,
|
||||
location.pathname,
|
||||
location.search,
|
||||
navigate,
|
||||
]);
|
||||
const cacheKeyValueParam = searchParams.get("cache-key-value");
|
||||
const headlessTurnDrainEnabled = ["1", "true"].includes(
|
||||
(searchParams.get("copilotHeadlessTurnDrain") ?? "").toLowerCase(),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue