mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
feat(SKY-11150): give terminate actions a distinct terminated visual (amber triangle) (#6693)
Some checks are pending
Run tests and pre-commit / Run tests and pre-commit hooks (push) Waiting to run
Run tests and pre-commit / Frontend Lint and Build (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.11) (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.13) (push) Waiting to run
Publish Fern Docs / run (push) Waiting to run
Some checks are pending
Run tests and pre-commit / Run tests and pre-commit hooks (push) Waiting to run
Run tests and pre-commit / Frontend Lint and Build (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.11) (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.13) (push) Waiting to run
Publish Fern Docs / run (push) Waiting to run
Co-authored-by: Suchintan Singh <suchintan@skyvern.com>
This commit is contained in:
parent
a824fb9915
commit
b5ad775942
12 changed files with 222 additions and 43 deletions
|
|
@ -8,7 +8,6 @@ import {
|
|||
CrossCircledIcon,
|
||||
MinusCircledIcon,
|
||||
PauseIcon,
|
||||
StopIcon,
|
||||
StopwatchIcon,
|
||||
UpdateIcon,
|
||||
} from "@radix-ui/react-icons";
|
||||
|
|
@ -16,6 +15,7 @@ import {
|
|||
import { Status } from "@/api/types";
|
||||
import { cn } from "@/util/utils";
|
||||
|
||||
import { TerminatedIcon } from "./terminatedVisual";
|
||||
import { Badge } from "./ui/badge";
|
||||
|
||||
type StatusVariant =
|
||||
|
|
@ -98,7 +98,7 @@ function iconForStatus(status: Status | "pending") {
|
|||
case Status.TimedOut:
|
||||
return <StopwatchIcon className={cls} />;
|
||||
case Status.Terminated:
|
||||
return <StopIcon className={cls} />;
|
||||
return <TerminatedIcon className={cls} />;
|
||||
case Status.Skipped:
|
||||
return <MinusCircledIcon className={cls} />;
|
||||
case Status.Paused:
|
||||
|
|
|
|||
9
skyvern-frontend/src/components/terminatedVisual.tsx
Normal file
9
skyvern-frontend/src/components/terminatedVisual.tsx
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
import { ExclamationTriangleIcon } from "@radix-ui/react-icons";
|
||||
|
||||
// Single source of truth for how a "terminated" outcome looks across the app
|
||||
// (run timeline, action cards, and the run-history status badge). Swap
|
||||
// TerminatedIcon / the tone classes here to change it everywhere.
|
||||
export const TerminatedIcon = ExclamationTriangleIcon;
|
||||
export const terminatedTone = "text-amber-500";
|
||||
export const terminatedBorder = "border-l-amber-500";
|
||||
export const terminatedDot = "bg-amber-500";
|
||||
|
|
@ -3,8 +3,9 @@ import {
|
|||
type ActionType,
|
||||
ActionTypes,
|
||||
ReadableActionTypes,
|
||||
Status,
|
||||
} from "@/api/types";
|
||||
import { getActionDisplayKind } from "@/routes/workflows/components/actionStatus";
|
||||
import { terminatedDot } from "@/components/terminatedVisual";
|
||||
import { Collapsible, CollapsibleContent } from "@/components/ui/collapsible";
|
||||
import {
|
||||
Tooltip,
|
||||
|
|
@ -59,11 +60,7 @@ function ActionCardCompact({
|
|||
onToggleExpanded,
|
||||
cardClassName,
|
||||
}: Props) {
|
||||
// wait actions return ActionFailure despite succeeding
|
||||
const success =
|
||||
action.action_type === ActionTypes.wait ||
|
||||
action.status === Status.Completed ||
|
||||
action.status === Status.Skipped;
|
||||
const kind = getActionDisplayKind(action);
|
||||
|
||||
const reasoningPreview = action.reasoning?.trim() ?? "";
|
||||
const fromScript = action.created_by === "script";
|
||||
|
|
@ -88,7 +85,7 @@ function ActionCardCompact({
|
|||
<div
|
||||
data-slot="action-card-compact"
|
||||
data-active={active ? "true" : "false"}
|
||||
data-status={success ? "success" : "failure"}
|
||||
data-status={kind}
|
||||
className={cn(
|
||||
"group relative rounded-md bg-slate-elevation4 ring-1 ring-transparent transition-all duration-200",
|
||||
{
|
||||
|
|
@ -111,9 +108,10 @@ function ActionCardCompact({
|
|||
<div className="flex min-h-[24px] items-center gap-2">
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className={cn("h-2 w-2 shrink-0 rounded-full", {
|
||||
"bg-success": success,
|
||||
"bg-destructive": !success,
|
||||
className={cn("h-2 w-2 shrink-0", {
|
||||
"rounded-full bg-success": kind === "success",
|
||||
"rounded-full bg-destructive": kind === "failure",
|
||||
[`rounded-[2px] ${terminatedDot}`]: kind === "terminated",
|
||||
})}
|
||||
/>
|
||||
<span className="shrink-0 text-xs tabular-nums text-slate-500">
|
||||
|
|
|
|||
|
|
@ -4,10 +4,10 @@ import {
|
|||
ActionApiResponse,
|
||||
ActionsApiResponse,
|
||||
ActionTypes,
|
||||
Status,
|
||||
StepApiResponse,
|
||||
TaskApiResponse,
|
||||
} from "@/api/types";
|
||||
import { isActionSuccess } from "@/routes/workflows/components/actionStatus";
|
||||
import { useCredentialGetter } from "@/hooks/useCredentialGetter";
|
||||
import { keepPreviousData, useQuery } from "@tanstack/react-query";
|
||||
import { statusIsNotFinalized } from "../../types";
|
||||
|
|
@ -106,11 +106,12 @@ function useActions({ id }: Props): {
|
|||
confidence: action.confidence_float,
|
||||
input: getActionInput(action),
|
||||
type: action.action_type,
|
||||
// Wait actions always succeed — they intentionally return ActionFailure
|
||||
// from the backend but completing a wait is expected, not a failure.
|
||||
// wait reports ActionFailure but completing it is expected; terminate
|
||||
// reports ActionSuccess but means the agent gave up — a failure.
|
||||
success:
|
||||
action.action_type === ActionTypes.wait ||
|
||||
(actionResult?.[0]?.success ?? false),
|
||||
action.action_type !== ActionTypes.terminate &&
|
||||
(action.action_type === ActionTypes.wait ||
|
||||
(actionResult?.[0]?.success ?? false)),
|
||||
stepId: step.step_id,
|
||||
index,
|
||||
created_by: action.created_by,
|
||||
|
|
@ -125,11 +126,7 @@ function useActions({ id }: Props): {
|
|||
confidence: action.confidence_float ?? undefined,
|
||||
input: action.response ?? "",
|
||||
type: action.action_type,
|
||||
// Wait actions always succeed — see comment in legacy path above.
|
||||
success:
|
||||
action.action_type === ActionTypes.wait ||
|
||||
action.status === Status.Completed ||
|
||||
action.status === Status.Skipped,
|
||||
success: isActionSuccess(action),
|
||||
stepId: action.step_id ?? "",
|
||||
index: index,
|
||||
created_by: action.created_by,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,112 @@
|
|||
import { describe, expect, test } from "vitest";
|
||||
|
||||
import {
|
||||
ActionTypes,
|
||||
Status,
|
||||
type ActionsApiResponse,
|
||||
type ActionType,
|
||||
} from "@/api/types";
|
||||
|
||||
import {
|
||||
getActionDisplayKind,
|
||||
getActionDisplayStatus,
|
||||
isActionSuccess,
|
||||
} from "./actionStatus";
|
||||
|
||||
function buildAction(
|
||||
action_type: ActionType,
|
||||
status: Status,
|
||||
): ActionsApiResponse {
|
||||
return {
|
||||
action_id: "act_1",
|
||||
action_type,
|
||||
status,
|
||||
task_id: null,
|
||||
step_id: null,
|
||||
step_order: null,
|
||||
action_order: null,
|
||||
confidence_float: null,
|
||||
description: null,
|
||||
reasoning: null,
|
||||
intention: null,
|
||||
response: null,
|
||||
created_by: null,
|
||||
text: null,
|
||||
};
|
||||
}
|
||||
|
||||
describe("getActionDisplayStatus", () => {
|
||||
test("terminate is shown as terminated even when persisted as completed", () => {
|
||||
expect(
|
||||
getActionDisplayStatus(
|
||||
buildAction(ActionTypes.terminate, Status.Completed),
|
||||
),
|
||||
).toBe(Status.Terminated);
|
||||
});
|
||||
|
||||
test("wait passes its persisted status through (timeline shows real status)", () => {
|
||||
expect(
|
||||
getActionDisplayStatus(buildAction(ActionTypes.wait, Status.Failed)),
|
||||
).toBe(Status.Failed);
|
||||
});
|
||||
|
||||
test("other actions pass their persisted status through", () => {
|
||||
expect(
|
||||
getActionDisplayStatus(buildAction(ActionTypes.Click, Status.Failed)),
|
||||
).toBe(Status.Failed);
|
||||
expect(
|
||||
getActionDisplayStatus(buildAction(ActionTypes.Click, Status.Completed)),
|
||||
).toBe(Status.Completed);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isActionSuccess", () => {
|
||||
test("terminate is a failure", () => {
|
||||
expect(
|
||||
isActionSuccess(buildAction(ActionTypes.terminate, Status.Completed)),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test("wait is a success", () => {
|
||||
expect(isActionSuccess(buildAction(ActionTypes.wait, Status.Failed))).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("completed and skipped are successes, failed is not", () => {
|
||||
expect(
|
||||
isActionSuccess(buildAction(ActionTypes.Click, Status.Completed)),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isActionSuccess(buildAction(ActionTypes.Click, Status.Skipped)),
|
||||
).toBe(true);
|
||||
expect(isActionSuccess(buildAction(ActionTypes.Click, Status.Failed))).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getActionDisplayKind", () => {
|
||||
test("terminate is its own kind, not failure", () => {
|
||||
expect(
|
||||
getActionDisplayKind(
|
||||
buildAction(ActionTypes.terminate, Status.Completed),
|
||||
),
|
||||
).toBe("terminated");
|
||||
});
|
||||
|
||||
test("wait is a success", () => {
|
||||
expect(
|
||||
getActionDisplayKind(buildAction(ActionTypes.wait, Status.Failed)),
|
||||
).toBe("success");
|
||||
});
|
||||
|
||||
test("completed is success, failed is failure", () => {
|
||||
expect(
|
||||
getActionDisplayKind(buildAction(ActionTypes.Click, Status.Completed)),
|
||||
).toBe("success");
|
||||
expect(
|
||||
getActionDisplayKind(buildAction(ActionTypes.Click, Status.Failed)),
|
||||
).toBe("failure");
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,39 @@
|
|||
import { ActionsApiResponse, ActionTypes, Status } from "@/api/types";
|
||||
|
||||
// A terminate action executes "successfully" (the backend persists it with
|
||||
// status "completed"), but it means the agent gave up — show it as a failure.
|
||||
// Every other status passes through unchanged.
|
||||
export function getActionDisplayStatus(action: ActionsApiResponse): Status {
|
||||
if (action.action_type === ActionTypes.terminate) {
|
||||
return Status.Terminated;
|
||||
}
|
||||
return action.status;
|
||||
}
|
||||
|
||||
export function isActionSuccess(action: ActionsApiResponse): boolean {
|
||||
// terminate is not a success even though it is persisted as completed — it is
|
||||
// its own "terminated" outcome (see getActionDisplayKind for the distinction).
|
||||
if (action.action_type === ActionTypes.terminate) {
|
||||
return false;
|
||||
}
|
||||
// wait reports ActionFailure from the backend, but completing a wait is
|
||||
// expected, not a failure.
|
||||
if (action.action_type === ActionTypes.wait) {
|
||||
return true;
|
||||
}
|
||||
return action.status === Status.Completed || action.status === Status.Skipped;
|
||||
}
|
||||
|
||||
export type ActionDisplayKind = "success" | "failure" | "terminated";
|
||||
|
||||
// terminate is its own outcome — the agent deliberately gave up — so it gets a
|
||||
// distinct visual (matching the "terminated" status badge) rather than the red
|
||||
// failure treatment used for errors.
|
||||
export function getActionDisplayKind(
|
||||
action: ActionsApiResponse,
|
||||
): ActionDisplayKind {
|
||||
if (action.action_type === ActionTypes.terminate) {
|
||||
return "terminated";
|
||||
}
|
||||
return isActionSuccess(action) ? "success" : "failure";
|
||||
}
|
||||
|
|
@ -1,4 +1,6 @@
|
|||
import { ActionsApiResponse, ActionTypes, Status } from "@/api/types";
|
||||
import { ActionsApiResponse, ActionTypes } from "@/api/types";
|
||||
import { getActionDisplayKind } from "@/routes/workflows/components/actionStatus";
|
||||
import { TerminatedIcon, terminatedTone } from "@/components/terminatedVisual";
|
||||
import { StatusPill } from "@/components/ui/status-pill";
|
||||
import {
|
||||
Tooltip,
|
||||
|
|
@ -24,17 +26,12 @@ type Props = {
|
|||
};
|
||||
|
||||
function ActionCard({ action, onClick, active, index, cardClassName }: Props) {
|
||||
// Wait actions always succeed — they intentionally return ActionFailure
|
||||
// from the backend but completing a wait is expected, not a failure.
|
||||
const success =
|
||||
action.action_type === ActionTypes.wait ||
|
||||
action.status === Status.Completed ||
|
||||
action.status === Status.Skipped;
|
||||
const kind = getActionDisplayKind(action);
|
||||
|
||||
return (
|
||||
<RunCard
|
||||
active={active}
|
||||
status={success ? "success" : "failure"}
|
||||
status={kind}
|
||||
onClick={onClick}
|
||||
className={cardClassName ? `flex ${cardClassName}` : "flex"}
|
||||
>
|
||||
|
|
@ -61,10 +58,16 @@ function ActionCard({ action, onClick, active, index, cardClassName }: Props) {
|
|||
</Tooltip>
|
||||
</TooltipProvider>
|
||||
)}
|
||||
{success ? (
|
||||
{kind === "success" ? (
|
||||
<StatusPill
|
||||
icon={<CheckCircledIcon className="h-4 w-4 text-success" />}
|
||||
/>
|
||||
) : kind === "terminated" ? (
|
||||
<StatusPill
|
||||
icon={
|
||||
<TerminatedIcon className={`h-4 w-4 ${terminatedTone}`} />
|
||||
}
|
||||
/>
|
||||
) : (
|
||||
<StatusPill
|
||||
icon={<CrossCircledIcon className="h-4 w-4 text-destructive" />}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { LightningBoltIcon } from "@radix-ui/react-icons";
|
||||
import { ActionsApiResponse, ActionTypes, Status } from "@/api/types";
|
||||
import { ActionsApiResponse } from "@/api/types";
|
||||
import { getActionDisplayKind } from "@/routes/workflows/components/actionStatus";
|
||||
import {
|
||||
Tooltip,
|
||||
TooltipContent,
|
||||
|
|
@ -14,15 +15,15 @@ type Props = {
|
|||
};
|
||||
|
||||
function ActionCardMinimal({ action }: Props) {
|
||||
// Wait actions always succeed — they intentionally return ActionFailure
|
||||
// from the backend but completing a wait is expected, not a failure.
|
||||
const success =
|
||||
action.action_type === ActionTypes.wait ||
|
||||
action.status === Status.Completed ||
|
||||
action.status === Status.Skipped;
|
||||
const kind = getActionDisplayKind(action);
|
||||
|
||||
return (
|
||||
<ItemStatusIndicator failure={!success} success={success} offset="-0.7rem">
|
||||
<ItemStatusIndicator
|
||||
success={kind === "success"}
|
||||
failure={kind === "failure"}
|
||||
terminated={kind === "terminated"}
|
||||
offset="-0.7rem"
|
||||
>
|
||||
<div className="flex items-center justify-center gap-2">
|
||||
<ActionTypePillMinimal actionType={action.action_type} />
|
||||
{action.created_by === "script" && (
|
||||
|
|
|
|||
|
|
@ -1,10 +1,13 @@
|
|||
import { CheckIcon, Cross2Icon } from "@radix-ui/react-icons";
|
||||
|
||||
import { TerminatedIcon, terminatedTone } from "@/components/terminatedVisual";
|
||||
|
||||
interface Props {
|
||||
children: React.ReactNode;
|
||||
offset?: string;
|
||||
failure?: boolean;
|
||||
success?: boolean;
|
||||
terminated?: boolean;
|
||||
}
|
||||
|
||||
function ItemStatusIndicator({
|
||||
|
|
@ -12,6 +15,7 @@ function ItemStatusIndicator({
|
|||
offset = "-0.6rem",
|
||||
failure,
|
||||
success,
|
||||
terminated,
|
||||
}: Props) {
|
||||
return (
|
||||
<div className="relative flex items-center justify-center overflow-visible">
|
||||
|
|
@ -22,6 +26,12 @@ function ItemStatusIndicator({
|
|||
style={{ right: offset, top: offset }}
|
||||
/>
|
||||
)}
|
||||
{terminated && (
|
||||
<TerminatedIcon
|
||||
className={`absolute h-[0.65rem] w-[0.65rem] ${terminatedTone}`}
|
||||
style={{ right: offset, top: offset }}
|
||||
/>
|
||||
)}
|
||||
{failure && (
|
||||
<Cross2Icon
|
||||
className="absolute h-[0.65rem] w-[0.65rem] text-destructive"
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
import { cn } from "@/util/utils";
|
||||
import { useEffect, useRef } from "react";
|
||||
|
||||
type RunCardStatus = "success" | "failure";
|
||||
import { terminatedBorder } from "@/components/terminatedVisual";
|
||||
|
||||
type RunCardStatus = "success" | "failure" | "terminated";
|
||||
|
||||
type RunCardProps = {
|
||||
active?: boolean;
|
||||
|
|
@ -40,6 +42,7 @@ function RunCard({
|
|||
!!onClick,
|
||||
"border-l-destructive": status === "failure" && !active,
|
||||
"border-l-success": status === "success" && !active,
|
||||
[terminatedBorder]: status === "terminated" && !active,
|
||||
"ring-2 ring-neutral-500/45 hover:ring-neutral-500/45 dark:ring-white/55 dark:hover:ring-white/55":
|
||||
active,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ import { cn } from "@/util/utils";
|
|||
import { workflowBlockTitle } from "../editor/nodes/types";
|
||||
import { WorkflowBlockIcon } from "../editor/nodes/WorkflowBlockIcon";
|
||||
import { actionTypeIcons as timelineActionIcons } from "../components/actionTypeIcons";
|
||||
import { getActionDisplayStatus } from "../components/actionStatus";
|
||||
import { TerminatedIcon, terminatedTone } from "@/components/terminatedVisual";
|
||||
import {
|
||||
isAction,
|
||||
isBlockItem,
|
||||
|
|
@ -99,9 +101,9 @@ function StatusDot({
|
|||
isFinalized: boolean;
|
||||
}) {
|
||||
const isCompleted = status === Status.Completed;
|
||||
const isTerminated = status === Status.Terminated;
|
||||
const isFailure =
|
||||
status === Status.Failed ||
|
||||
status === Status.Terminated ||
|
||||
status === Status.TimedOut ||
|
||||
status === Status.Canceled;
|
||||
const isRunning = status === Status.Running && !isFinalized;
|
||||
|
|
@ -109,6 +111,9 @@ function StatusDot({
|
|||
if (isCompleted) {
|
||||
return <CheckCircledIcon className="size-3.5 shrink-0 text-success" />;
|
||||
}
|
||||
if (isTerminated) {
|
||||
return <TerminatedIcon className={`size-3.5 shrink-0 ${terminatedTone}`} />;
|
||||
}
|
||||
if (isFailure) {
|
||||
return <CrossCircledIcon className="size-3.5 shrink-0 text-destructive" />;
|
||||
}
|
||||
|
|
@ -490,7 +495,7 @@ function TimelineActionRows({
|
|||
className="flex min-w-0 flex-1 cursor-pointer items-center gap-1.5 rounded text-left outline-none focus-visible:ring-1 focus-visible:ring-white/40"
|
||||
>
|
||||
<StatusDot
|
||||
status={action.status}
|
||||
status={getActionDisplayStatus(action)}
|
||||
isFinalized={!!workflowRunIsFinalized}
|
||||
/>
|
||||
<span className="shrink-0 text-slate-400" aria-hidden="true">
|
||||
|
|
|
|||
|
|
@ -23,10 +23,11 @@ function WorkflowRunTimelineBlockItemMinimal({ block, subItems }: Props) {
|
|||
const showStatusIndicator = block.status !== null;
|
||||
const showSuccessIndicator =
|
||||
showStatusIndicator && block.status === Status.Completed;
|
||||
const showTerminatedIndicator =
|
||||
showStatusIndicator && block.status === Status.Terminated;
|
||||
const showFailureIndicator =
|
||||
showStatusIndicator &&
|
||||
(block.status === Status.Failed ||
|
||||
block.status === Status.Terminated ||
|
||||
block.status === Status.TimedOut ||
|
||||
block.status === Status.Canceled);
|
||||
|
||||
|
|
@ -43,6 +44,7 @@ function WorkflowRunTimelineBlockItemMinimal({ block, subItems }: Props) {
|
|||
<ItemStatusIndicator
|
||||
failure={showFailureIndicator}
|
||||
success={showSuccessIndicator}
|
||||
terminated={showTerminatedIndicator}
|
||||
>
|
||||
<WorkflowBlockIcon workflowBlockType={block.block_type} />
|
||||
</ItemStatusIndicator>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue