feat(web): warn about workflow replay cycles (#1720)
This commit is contained in:
Carlos Florêncio
@@ -1,20 +1,18 @@
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { beforeEach, expect, it, vi } from "vitest";
|
||||
import {
|
||||
createWorkflowAction,
|
||||
createWorkflowStepAction,
|
||||
deleteWorkflowStepAction,
|
||||
getWorkflowTaskCount,
|
||||
getStepTaskCount,
|
||||
listWorkflowStepsAction,
|
||||
reorderWorkflowStepsAction,
|
||||
updateWorkflowStepAction,
|
||||
} from "@/app/actions/workspaces";
|
||||
import type { OnTurnCompleteAction } from "@/lib/types/workflow-actions";
|
||||
import type { Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import {
|
||||
useWorkflowDeleteHandlers,
|
||||
useWorkflowSaveActions,
|
||||
useWorkflowStepActions,
|
||||
} from "./workflow-card-actions";
|
||||
import { useWorkflowSaveActions, useWorkflowStepActions } from "./workflow-card-actions";
|
||||
import { useWorkflowMutationGuard } from "./workflow-mutation-guard";
|
||||
|
||||
vi.mock("@/app/actions/workspaces", () => ({
|
||||
createWorkflowAction: vi.fn(),
|
||||
@@ -29,6 +27,18 @@ vi.mock("@/app/actions/workspaces", () => ({
|
||||
bulkMoveTasks: vi.fn(),
|
||||
}));
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(createWorkflowStepAction).mockImplementation(async (payload) =>
|
||||
step(`created-${payload.position}`, payload.name, payload.position, false),
|
||||
);
|
||||
vi.mocked(updateWorkflowStepAction).mockImplementation(async (stepId, updates) => ({
|
||||
...step(stepId, stepId, updates.position ?? 0, updates.is_start_step ?? false),
|
||||
...updates,
|
||||
}));
|
||||
vi.mocked(reorderWorkflowStepsAction).mockResolvedValue({ steps: [], total: 0 });
|
||||
});
|
||||
|
||||
const workflow = {
|
||||
id: "wf-1",
|
||||
workspace_id: "ws-1",
|
||||
@@ -37,7 +47,13 @@ const workflow = {
|
||||
updated_at: "",
|
||||
} as Workflow;
|
||||
|
||||
function step(id: string, name: string, position: number, isStartStep: boolean): WorkflowStep {
|
||||
function step(
|
||||
id: string,
|
||||
name: string,
|
||||
position: number,
|
||||
isStartStep: boolean,
|
||||
options: { autoStart?: boolean; onTurnComplete?: OnTurnCompleteAction[] } = {},
|
||||
): WorkflowStep {
|
||||
return {
|
||||
id,
|
||||
workflow_id: workflow.id,
|
||||
@@ -46,6 +62,10 @@ function step(id: string, name: string, position: number, isStartStep: boolean):
|
||||
color: "bg-slate-500",
|
||||
allow_manual_move: true,
|
||||
is_start_step: isStartStep,
|
||||
events: {
|
||||
on_enter: options.autoStart ? [{ type: "auto_start_agent" }] : [],
|
||||
on_turn_complete: options.onTurnComplete,
|
||||
},
|
||||
created_at: "",
|
||||
updated_at: "",
|
||||
};
|
||||
@@ -58,214 +78,587 @@ function renderNewWorkflowStepActions(initialSteps: WorkflowStep[]) {
|
||||
steps = typeof updater === "function" ? updater(steps) : updater;
|
||||
},
|
||||
);
|
||||
const view = renderHook(() =>
|
||||
useWorkflowStepActions({
|
||||
const view = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(steps);
|
||||
return useWorkflowStepActions({
|
||||
workflow,
|
||||
isNewWorkflow: true,
|
||||
workflowSteps: steps,
|
||||
setWorkflowSteps,
|
||||
refreshWorkflowSteps: vi.fn(),
|
||||
setStepToDelete: vi.fn(),
|
||||
setStepTaskCount: vi.fn(),
|
||||
setTargetStepForMigration: vi.fn(),
|
||||
setStepDeleteOpen: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
}),
|
||||
);
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
return { ...view, getSteps: () => steps };
|
||||
}
|
||||
|
||||
function renderReadOnlyWorkflowStepActions(initialSteps: WorkflowStep[]) {
|
||||
let steps = initialSteps;
|
||||
function renderPersistedWorkflowStepActions(steps: WorkflowStep[]) {
|
||||
let currentSteps = steps;
|
||||
const setWorkflowSteps = vi.fn(
|
||||
(updater: ((prev: WorkflowStep[]) => WorkflowStep[]) | WorkflowStep[]) => {
|
||||
steps = typeof updater === "function" ? updater(steps) : updater;
|
||||
currentSteps = typeof updater === "function" ? updater(currentSteps) : updater;
|
||||
},
|
||||
);
|
||||
const refreshWorkflowSteps = vi.fn();
|
||||
const view = renderHook(() =>
|
||||
useWorkflowStepActions({
|
||||
const setStepDeleteOpen = vi.fn();
|
||||
const view = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(steps);
|
||||
const actions = useWorkflowStepActions({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
readOnly: true,
|
||||
workflowSteps: steps,
|
||||
setWorkflowSteps,
|
||||
refreshWorkflowSteps,
|
||||
setStepToDelete: vi.fn(),
|
||||
setStepTaskCount: vi.fn(),
|
||||
setTargetStepForMigration: vi.fn(),
|
||||
setStepDeleteOpen,
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
return { actions, mutationGuard };
|
||||
});
|
||||
return {
|
||||
...view,
|
||||
setWorkflowSteps,
|
||||
setStepDeleteOpen,
|
||||
getSteps: () => currentSteps,
|
||||
};
|
||||
}
|
||||
|
||||
function deferredPromise<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
const promise = new Promise<T>((resolvePromise) => {
|
||||
resolve = resolvePromise;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
it("keeps one start step while editing a new workflow locally", async () => {
|
||||
const { result, getSteps } = renderNewWorkflowStepActions([
|
||||
step("step-1", "Todo", 0, true),
|
||||
step("step-2", "Plan", 1, false),
|
||||
]);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleUpdateWorkflowStep("step-2", { is_start_step: true });
|
||||
});
|
||||
|
||||
expect(
|
||||
getSteps()
|
||||
.filter((s) => s.is_start_step)
|
||||
.map((s) => s.id),
|
||||
).toEqual(["step-2"]);
|
||||
});
|
||||
|
||||
it("does not update a persisted step when the proposal introduces a blocking cycle", async () => {
|
||||
const steps = [
|
||||
step("build", "Build", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false, { autoStart: true }),
|
||||
];
|
||||
const { result } = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(steps);
|
||||
return useWorkflowStepActions({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
workflowSteps: steps,
|
||||
setWorkflowSteps: vi.fn(),
|
||||
setStepToDelete: vi.fn(),
|
||||
setStepTaskCount: vi.fn(),
|
||||
setTargetStepForMigration: vi.fn(),
|
||||
setStepDeleteOpen: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleUpdateWorkflowStep("review", {
|
||||
events: {
|
||||
...steps[1].events,
|
||||
on_turn_complete: [{ type: "move_to_previous" }],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not persist a second topology edit before the first mutation completes", async () => {
|
||||
const steps = [
|
||||
step("work", "Work", 0, true, { autoStart: true }),
|
||||
step("review", "Review", 1, false),
|
||||
];
|
||||
const update = deferredPromise<WorkflowStep>();
|
||||
vi.mocked(updateWorkflowStepAction).mockReturnValueOnce(update.promise);
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
let firstEdit!: Promise<void>;
|
||||
await act(async () => {
|
||||
firstEdit = result.current.actions.handleUpdateWorkflowStep("work", {
|
||||
events: { on_turn_complete: [{ type: "move_to_next" }] },
|
||||
});
|
||||
await vi.waitFor(() => expect(updateWorkflowStepAction).toHaveBeenCalledTimes(1));
|
||||
await result.current.actions.handleUpdateWorkflowStep("review", {
|
||||
events: { on_turn_complete: [{ type: "move_to_previous" }] },
|
||||
});
|
||||
});
|
||||
|
||||
expect(updateWorkflowStepAction).toHaveBeenCalledTimes(1);
|
||||
expect(result.current.mutationGuard.isMutationPending).toBe(true);
|
||||
|
||||
update.resolve({
|
||||
...steps[0],
|
||||
events: { on_turn_complete: [{ type: "move_to_next" }] },
|
||||
});
|
||||
await act(async () => {
|
||||
await firstEdit;
|
||||
});
|
||||
expect(result.current.mutationGuard.isMutationPending).toBe(false);
|
||||
});
|
||||
|
||||
it("does not replace a pending warning proposal with a second edit", async () => {
|
||||
const steps = [
|
||||
step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
);
|
||||
return { ...view, getSteps: () => steps, refreshWorkflowSteps, setWorkflowSteps };
|
||||
}
|
||||
step("review", "Review", 1, false),
|
||||
];
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
describe("useWorkflowStepActions", () => {
|
||||
it("keeps one start step while editing a new workflow locally", async () => {
|
||||
const { result, getSteps } = renderNewWorkflowStepActions([
|
||||
step("step-1", "Todo", 0, true),
|
||||
step("step-2", "Plan", 1, false),
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("review", {
|
||||
events: {
|
||||
...steps[1].events,
|
||||
on_turn_complete: [{ type: "move_to_previous" }],
|
||||
},
|
||||
});
|
||||
});
|
||||
const proposal = result.current.mutationGuard.proposal;
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("work", { name: "Building" });
|
||||
});
|
||||
|
||||
expect(result.current.mutationGuard.proposal).toBe(proposal);
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not replace a pending blocking proposal with a second edit", async () => {
|
||||
const steps = [
|
||||
step("build", "Build", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false, { autoStart: true }),
|
||||
];
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("review", {
|
||||
events: { on_turn_complete: [{ type: "move_to_previous" }] },
|
||||
});
|
||||
});
|
||||
const proposal = result.current.mutationGuard.proposal;
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("build", { name: "Building" });
|
||||
});
|
||||
|
||||
expect(result.current.mutationGuard.proposal).toBe(proposal);
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("releases a blocking proposal if confirmation is invoked defensively", async () => {
|
||||
const steps = [
|
||||
step("build", "Build", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false, { autoStart: true }),
|
||||
];
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("review", {
|
||||
events: {
|
||||
...steps[1].events,
|
||||
on_turn_complete: [{ type: "move_to_previous" }],
|
||||
},
|
||||
});
|
||||
});
|
||||
expect(result.current.mutationGuard.proposal?.severity).toBe("blocking");
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutationGuard.confirmProposal();
|
||||
});
|
||||
|
||||
expect(result.current.mutationGuard.proposal).toBeNull();
|
||||
expect(result.current.mutationGuard.isMutationPending).toBe(false);
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("holds a warning update until confirmation and executes it exactly once", async () => {
|
||||
const steps = [
|
||||
step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false),
|
||||
];
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("review", {
|
||||
events: { on_turn_complete: [{ type: "move_to_previous" }] },
|
||||
});
|
||||
});
|
||||
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(result.current.mutationGuard.proposal).toMatchObject({
|
||||
intent: "apply",
|
||||
severity: "warning",
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutationGuard.confirmProposal();
|
||||
await result.current.mutationGuard.confirmProposal();
|
||||
});
|
||||
|
||||
expect(updateWorkflowStepAction).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("discards a warning update when confirmation is cancelled", async () => {
|
||||
const steps = [
|
||||
step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false),
|
||||
];
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("review", {
|
||||
events: { on_turn_complete: [{ type: "move_to_previous" }] },
|
||||
});
|
||||
result.current.mutationGuard.cancelProposal();
|
||||
});
|
||||
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(result.current.mutationGuard.proposal).toBeNull();
|
||||
});
|
||||
|
||||
it("does not gate an unchanged diagnostic identity or an edit that removes a cycle", async () => {
|
||||
const steps = [
|
||||
step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false, {
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
];
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("work", { prompt: "New prompt" });
|
||||
await result.current.actions.handleUpdateWorkflowStep("review", {
|
||||
events: { on_turn_complete: [] },
|
||||
});
|
||||
});
|
||||
|
||||
expect(updateWorkflowStepAction).toHaveBeenCalledTimes(2);
|
||||
expect(result.current.mutationGuard.proposal).toBeNull();
|
||||
});
|
||||
|
||||
it("preflights a delete shape before any side effect", async () => {
|
||||
const work = step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
});
|
||||
const middle = step("middle", "Middle", 1, false);
|
||||
const review = step("review", "Review", 2, false, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
});
|
||||
const { result, setWorkflowSteps } = renderPersistedWorkflowStepActions([work, middle, review]);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleRemoveWorkflowStep("middle");
|
||||
});
|
||||
|
||||
expect(getStepTaskCount).not.toHaveBeenCalled();
|
||||
expect(deleteWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(setWorkflowSteps).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("preflights a reorder shape with an idle guard before any side effect", async () => {
|
||||
const work = step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
});
|
||||
const middle = step("middle", "Middle", 1, false);
|
||||
const review = step("review", "Review", 2, false, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
});
|
||||
const { result, setWorkflowSteps } = renderPersistedWorkflowStepActions([work, middle, review]);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleReorderWorkflowSteps([
|
||||
work,
|
||||
{ ...review, position: 1 },
|
||||
{ ...middle, position: 2 },
|
||||
]);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleUpdateWorkflowStep("step-2", { is_start_step: true });
|
||||
});
|
||||
|
||||
expect(
|
||||
getSteps()
|
||||
.filter((s) => s.is_start_step)
|
||||
.map((s) => s.id),
|
||||
).toEqual(["step-2"]);
|
||||
});
|
||||
|
||||
it("refuses to add, update, remove, or reorder steps when readOnly", async () => {
|
||||
const initialSteps = [step("step-1", "Todo", 0, true), step("step-2", "Plan", 1, false)];
|
||||
const { result, getSteps, refreshWorkflowSteps } =
|
||||
renderReadOnlyWorkflowStepActions(initialSteps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleAddWorkflowStep();
|
||||
});
|
||||
await act(async () => {
|
||||
await result.current.handleUpdateWorkflowStep("step-2", { name: "Renamed" });
|
||||
});
|
||||
await act(async () => {
|
||||
await result.current.handleRemoveWorkflowStep("step-1");
|
||||
});
|
||||
await act(async () => {
|
||||
await result.current.handleReorderWorkflowSteps([initialSteps[1], initialSteps[0]]);
|
||||
});
|
||||
|
||||
expect(getSteps()).toEqual(initialSteps);
|
||||
expect(createWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(deleteWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(reorderWorkflowStepsAction).not.toHaveBeenCalled();
|
||||
expect(refreshWorkflowSteps).not.toHaveBeenCalled();
|
||||
});
|
||||
expect(result.current.mutationGuard.proposal?.severity).toBe("blocking");
|
||||
expect(reorderWorkflowStepsAction).not.toHaveBeenCalled();
|
||||
expect(setWorkflowSteps).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("useWorkflowDeleteHandlers", () => {
|
||||
it("refuses to open the delete-workflow dialog when readOnly", async () => {
|
||||
const wfDel = {
|
||||
setDeleteOpen: vi.fn(),
|
||||
setWorkflowTaskCount: vi.fn(),
|
||||
setWorkflowDeleteLoading: vi.fn(),
|
||||
setTargetWorkflowId: vi.fn(),
|
||||
setTargetWorkflowSteps: vi.fn(),
|
||||
setTargetStepId: vi.fn(),
|
||||
targetWorkflowId: "",
|
||||
targetStepId: "",
|
||||
setMigrateLoading: vi.fn(),
|
||||
};
|
||||
const { result } = renderHook(() =>
|
||||
useWorkflowDeleteHandlers({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
readOnly: true,
|
||||
otherWorkflows: [],
|
||||
wfDel,
|
||||
deleteWorkflowRun: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
}),
|
||||
);
|
||||
it("reconciles a successful update from its authoritative response", async () => {
|
||||
const steps = [step("work", "Work", 0, true)];
|
||||
const updated = { ...steps[0], name: "Building" };
|
||||
vi.mocked(updateWorkflowStepAction).mockResolvedValue(updated);
|
||||
const { result, getSteps } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleDeleteWorkflowClick();
|
||||
});
|
||||
|
||||
expect(getWorkflowTaskCount).not.toHaveBeenCalled();
|
||||
expect(wfDel.setDeleteOpen).not.toHaveBeenCalled();
|
||||
await act(async () => {
|
||||
await result.current.actions.handleUpdateWorkflowStep("work", { name: "Building" });
|
||||
});
|
||||
|
||||
expect(getSteps()).toEqual([updated]);
|
||||
expect(listWorkflowStepsAction).not.toHaveBeenCalled();
|
||||
expect(result.current.mutationGuard.isMutationPending).toBe(false);
|
||||
});
|
||||
|
||||
describe("useWorkflowSaveActions", () => {
|
||||
it("remaps template workflow pull sources between backend-created and added steps", async () => {
|
||||
const createdWorkflowId = "wf-created" as Workflow["id"];
|
||||
const templateName = "Template Step";
|
||||
const addedName = "Added Step";
|
||||
const backendTemplateId = "backend-template";
|
||||
const backendAddedId = "backend-added";
|
||||
const draftTemplateId = "draft-template";
|
||||
it("appends a successful add from its authoritative response", async () => {
|
||||
const steps = [step("work", "Work", 0, true)];
|
||||
const created = step("created", "Server Step", 1, false);
|
||||
vi.mocked(createWorkflowStepAction).mockResolvedValue(created);
|
||||
const { result, getSteps } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
vi.mocked(createWorkflowAction).mockResolvedValue({
|
||||
...workflow,
|
||||
id: createdWorkflowId,
|
||||
workflow_template_id: "template-1",
|
||||
});
|
||||
vi.mocked(listWorkflowStepsAction).mockResolvedValue({
|
||||
steps: [
|
||||
{
|
||||
...step(backendTemplateId, templateName, 0, true),
|
||||
id: backendTemplateId,
|
||||
workflow_id: createdWorkflowId,
|
||||
},
|
||||
],
|
||||
total: 1,
|
||||
});
|
||||
vi.mocked(createWorkflowStepAction).mockResolvedValue({
|
||||
...step(backendAddedId, addedName, 1, false),
|
||||
id: backendAddedId,
|
||||
workflow_id: createdWorkflowId,
|
||||
});
|
||||
vi.mocked(updateWorkflowStepAction).mockResolvedValue(
|
||||
step(backendAddedId, addedName, 1, false),
|
||||
);
|
||||
await act(async () => {
|
||||
await result.current.actions.handleAddWorkflowStep();
|
||||
});
|
||||
|
||||
const templateStep = {
|
||||
...step(draftTemplateId, templateName, 0, true),
|
||||
pull_from_step_id: "draft-added",
|
||||
};
|
||||
const addedStep = {
|
||||
...step("draft-added", addedName, 1, false),
|
||||
pull_from_step_id: draftTemplateId,
|
||||
};
|
||||
const { result } = renderHook(() =>
|
||||
useWorkflowSaveActions({
|
||||
workflow: { ...workflow, workflow_template_id: "template-1" },
|
||||
isNewWorkflow: true,
|
||||
workflowSteps: [templateStep, addedStep],
|
||||
templateStepCount: 1,
|
||||
onSaveWorkflow: vi.fn(),
|
||||
onWorkflowCreated: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
}),
|
||||
);
|
||||
expect(getSteps()).toEqual([...steps, created]);
|
||||
expect(listWorkflowStepsAction).not.toHaveBeenCalled();
|
||||
expect(result.current.mutationGuard.isMutationPending).toBe(false);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleSaveWorkflow();
|
||||
});
|
||||
it("reconciles a successful reorder from its authoritative response", async () => {
|
||||
const work = step("work", "Work", 0, true);
|
||||
const review = step("review", "Review", 1, false);
|
||||
const reordered = [
|
||||
{ ...review, position: 0 },
|
||||
{ ...work, position: 1 },
|
||||
];
|
||||
vi.mocked(reorderWorkflowStepsAction).mockResolvedValue({ steps: reordered, total: 2 });
|
||||
const { result, getSteps } = renderPersistedWorkflowStepActions([work, review]);
|
||||
|
||||
expect(createWorkflowStepAction).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ pull_from_step_id: "" }),
|
||||
);
|
||||
expect(updateWorkflowStepAction).toHaveBeenCalledWith(backendAddedId, {
|
||||
pull_from_step_id: backendTemplateId,
|
||||
});
|
||||
expect(updateWorkflowStepAction).toHaveBeenCalledWith(backendTemplateId, {
|
||||
pull_from_step_id: backendAddedId,
|
||||
await act(async () => {
|
||||
await result.current.actions.handleReorderWorkflowSteps([review, work]);
|
||||
});
|
||||
|
||||
expect(getSteps()).toEqual(reordered);
|
||||
expect(listWorkflowStepsAction).not.toHaveBeenCalled();
|
||||
expect(result.current.mutationGuard.isMutationPending).toBe(false);
|
||||
});
|
||||
|
||||
it("holds the task migration delete path until a warning is confirmed", async () => {
|
||||
vi.mocked(getStepTaskCount).mockResolvedValue({ task_count: 2 });
|
||||
const work = step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
});
|
||||
const middle = step("middle", "Middle", 1, false);
|
||||
const review = step("review", "Review", 2, false, {
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
});
|
||||
const { result, setStepDeleteOpen } = renderPersistedWorkflowStepActions([work, middle, review]);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleRemoveWorkflowStep("middle");
|
||||
});
|
||||
expect(getStepTaskCount).not.toHaveBeenCalled();
|
||||
expect(result.current.mutationGuard.proposal?.severity).toBe("warning");
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutationGuard.confirmProposal();
|
||||
});
|
||||
|
||||
expect(getStepTaskCount).toHaveBeenCalledTimes(1);
|
||||
expect(setStepDeleteOpen).toHaveBeenCalledWith(true);
|
||||
expect(deleteWorkflowStepAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("adds a step without reconfirming an existing diagnostic", async () => {
|
||||
const steps = [
|
||||
step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_step", config: { step_id: "work" } }],
|
||||
}),
|
||||
];
|
||||
const { result } = renderPersistedWorkflowStepActions(steps);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.actions.handleAddWorkflowStep();
|
||||
});
|
||||
|
||||
expect(createWorkflowStepAction).toHaveBeenCalledTimes(1);
|
||||
expect(result.current.mutationGuard.proposal).toBeNull();
|
||||
});
|
||||
|
||||
it("does not create a draft workflow with a blocking cycle", async () => {
|
||||
const draftSteps = [
|
||||
step("build", "Build", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
];
|
||||
const { result } = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(draftSteps);
|
||||
return useWorkflowSaveActions({
|
||||
workflow: { ...workflow, id: "temp-workflow" as Workflow["id"] },
|
||||
isNewWorkflow: true,
|
||||
workflowSteps: draftSteps,
|
||||
templateStepCount: 0,
|
||||
onSaveWorkflow: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses to save changes to an existing workflow when readOnly", async () => {
|
||||
const onSaveWorkflow = vi.fn().mockResolvedValue(undefined);
|
||||
const { result } = renderHook(() =>
|
||||
useWorkflowSaveActions({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
readOnly: true,
|
||||
workflowSteps: [],
|
||||
templateStepCount: 0,
|
||||
onSaveWorkflow,
|
||||
toast: vi.fn(),
|
||||
}),
|
||||
);
|
||||
await act(async () => {
|
||||
await result.current.handleSaveWorkflow();
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleSaveWorkflow();
|
||||
expect(createWorkflowAction).not.toHaveBeenCalled();
|
||||
expect(result.current.mutationGuard.proposal?.severity).toBe("blocking");
|
||||
});
|
||||
|
||||
it("cancels a warning draft with no requests and creates it once after confirmation", async () => {
|
||||
const createdWorkflow = { ...workflow, id: "created-workflow" as Workflow["id"] };
|
||||
vi.mocked(createWorkflowAction).mockResolvedValue(createdWorkflow);
|
||||
vi.mocked(createWorkflowStepAction).mockImplementation(async (payload) => ({
|
||||
...step(`created-${payload.position}`, payload.name, payload.position, false),
|
||||
workflow_id: createdWorkflow.id,
|
||||
}));
|
||||
const draftSteps = [
|
||||
step("work", "Work", 0, true, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", "Review", 1, false, {
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
];
|
||||
const { result } = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(draftSteps);
|
||||
return useWorkflowSaveActions({
|
||||
workflow: { ...workflow, id: "temp-workflow" as Workflow["id"] },
|
||||
isNewWorkflow: true,
|
||||
workflowSteps: draftSteps,
|
||||
templateStepCount: 0,
|
||||
onSaveWorkflow: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
|
||||
expect(onSaveWorkflow).not.toHaveBeenCalled();
|
||||
await act(async () => {
|
||||
await result.current.handleSaveWorkflow();
|
||||
result.current.mutationGuard.cancelProposal();
|
||||
});
|
||||
expect(createWorkflowAction).not.toHaveBeenCalled();
|
||||
expect(createWorkflowStepAction).not.toHaveBeenCalled();
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleSaveWorkflow();
|
||||
await result.current.mutationGuard.confirmProposal();
|
||||
await result.current.mutationGuard.confirmProposal();
|
||||
});
|
||||
|
||||
expect(createWorkflowAction).toHaveBeenCalledTimes(1);
|
||||
expect(createWorkflowStepAction).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("remaps template workflow pull sources between backend-created and added steps", async () => {
|
||||
const createdWorkflowId = "wf-created" as Workflow["id"];
|
||||
const templateName = "Template Step";
|
||||
const addedName = "Added Step";
|
||||
const backendTemplateId = "backend-template";
|
||||
const backendAddedId = "backend-added";
|
||||
const draftTemplateId = "draft-template";
|
||||
|
||||
vi.mocked(createWorkflowAction).mockResolvedValue({
|
||||
...workflow,
|
||||
id: createdWorkflowId,
|
||||
workflow_template_id: "template-1",
|
||||
});
|
||||
vi.mocked(listWorkflowStepsAction).mockResolvedValue({
|
||||
steps: [
|
||||
{
|
||||
...step(backendTemplateId, templateName, 0, true),
|
||||
id: backendTemplateId,
|
||||
workflow_id: createdWorkflowId,
|
||||
},
|
||||
],
|
||||
total: 1,
|
||||
});
|
||||
vi.mocked(createWorkflowStepAction).mockResolvedValue({
|
||||
...step(backendAddedId, addedName, 1, false),
|
||||
id: backendAddedId,
|
||||
workflow_id: createdWorkflowId,
|
||||
});
|
||||
vi.mocked(updateWorkflowStepAction).mockResolvedValue(step(backendAddedId, addedName, 1, false));
|
||||
|
||||
const templateStep = {
|
||||
...step(draftTemplateId, templateName, 0, true),
|
||||
pull_from_step_id: "draft-added",
|
||||
};
|
||||
const addedStep = {
|
||||
...step("draft-added", addedName, 1, false),
|
||||
pull_from_step_id: draftTemplateId,
|
||||
};
|
||||
const workflowSteps = [templateStep, addedStep];
|
||||
const { result } = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(workflowSteps);
|
||||
return useWorkflowSaveActions({
|
||||
workflow: { ...workflow, workflow_template_id: "template-1" },
|
||||
isNewWorkflow: true,
|
||||
workflowSteps,
|
||||
templateStepCount: 1,
|
||||
onSaveWorkflow: vi.fn(),
|
||||
onWorkflowCreated: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleSaveWorkflow();
|
||||
});
|
||||
|
||||
expect(createWorkflowStepAction).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ pull_from_step_id: "" }),
|
||||
);
|
||||
expect(updateWorkflowStepAction).toHaveBeenCalledWith(backendAddedId, {
|
||||
pull_from_step_id: backendTemplateId,
|
||||
});
|
||||
expect(updateWorkflowStepAction).toHaveBeenCalledWith(backendTemplateId, {
|
||||
pull_from_step_id: backendAddedId,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,7 +4,6 @@ import { useEffect } from "react";
|
||||
import type { Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import { useToast } from "@/components/toast-provider";
|
||||
import { useRequest } from "@/lib/http/use-request";
|
||||
import { generateUUID } from "@/lib/utils";
|
||||
import {
|
||||
createWorkflowAction,
|
||||
createWorkflowStepAction,
|
||||
@@ -17,33 +16,35 @@ import {
|
||||
exportWorkflowAction,
|
||||
bulkMoveTasks,
|
||||
} from "@/app/actions/workspaces";
|
||||
import type { WorkflowMutationGuardController } from "./workflow-mutation-guard";
|
||||
import {
|
||||
addLocalStep,
|
||||
addRemoteStep,
|
||||
applyWorkflowStepUpdates,
|
||||
newWorkflowStep,
|
||||
removeLocalStep,
|
||||
updateRemoteWorkflowStep,
|
||||
} from "./workflow-step-mutations";
|
||||
|
||||
const FALLBACK_ERROR_MESSAGE = "Request failed";
|
||||
|
||||
type WorkflowStepActionsParams = {
|
||||
workflow: Workflow;
|
||||
isNewWorkflow: boolean;
|
||||
/**
|
||||
* Workflows synced from GitHub (`workflow.source === "github"`) are
|
||||
* read-only in the UI; the backend also rejects step mutations with a 409.
|
||||
* Gating here is defense-in-depth in case a disabled control is somehow
|
||||
* still triggered.
|
||||
*/
|
||||
readOnly?: boolean;
|
||||
workflowSteps: WorkflowStep[];
|
||||
setWorkflowSteps: (updater: ((prev: WorkflowStep[]) => WorkflowStep[]) | WorkflowStep[]) => void;
|
||||
refreshWorkflowSteps: () => Promise<void>;
|
||||
setStepToDelete: (id: string | null) => void;
|
||||
setStepTaskCount: (count: number | null) => void;
|
||||
setTargetStepForMigration: (id: string) => void;
|
||||
setStepDeleteOpen: (open: boolean) => void;
|
||||
toast: ReturnType<typeof useToast>["toast"];
|
||||
mutationGuard: WorkflowMutationGuardController;
|
||||
};
|
||||
|
||||
type RemoveStepParams = {
|
||||
stepId: string;
|
||||
workflowSteps: WorkflowStep[];
|
||||
refreshWorkflowSteps: () => Promise<void>;
|
||||
setWorkflowSteps: (updater: (prev: WorkflowStep[]) => WorkflowStep[]) => void;
|
||||
setStepToDelete: (id: string | null) => void;
|
||||
setStepTaskCount: (count: number | null) => void;
|
||||
setTargetStepForMigration: (id: string) => void;
|
||||
@@ -54,7 +55,7 @@ type RemoveStepParams = {
|
||||
async function removeWorkflowStep({
|
||||
stepId,
|
||||
workflowSteps,
|
||||
refreshWorkflowSteps,
|
||||
setWorkflowSteps,
|
||||
setStepToDelete,
|
||||
setStepTaskCount,
|
||||
setTargetStepForMigration,
|
||||
@@ -65,7 +66,11 @@ async function removeWorkflowStep({
|
||||
const { task_count } = await getStepTaskCount(stepId);
|
||||
if (task_count === 0) {
|
||||
await deleteWorkflowStepAction(stepId);
|
||||
await refreshWorkflowSteps();
|
||||
setWorkflowSteps((previous) =>
|
||||
previous
|
||||
.filter((step) => step.id !== stepId)
|
||||
.map((step, position) => ({ ...step, position })),
|
||||
);
|
||||
return;
|
||||
}
|
||||
setStepToDelete(stepId);
|
||||
@@ -82,73 +87,18 @@ async function removeWorkflowStep({
|
||||
}
|
||||
}
|
||||
|
||||
const NEW_STEP_DEFAULTS = { name: "New Step", color: "bg-slate-500" } as const;
|
||||
|
||||
function addLocalStep(
|
||||
workflow: Workflow,
|
||||
setWorkflowSteps: WorkflowStepActionsParams["setWorkflowSteps"],
|
||||
) {
|
||||
setWorkflowSteps((prev) => [
|
||||
...prev,
|
||||
{
|
||||
id: `temp-step-${generateUUID()}`,
|
||||
workflow_id: workflow.id,
|
||||
...NEW_STEP_DEFAULTS,
|
||||
position: prev.length,
|
||||
allow_manual_move: true,
|
||||
created_at: "",
|
||||
updated_at: "",
|
||||
},
|
||||
]);
|
||||
}
|
||||
|
||||
async function addRemoteStep(
|
||||
workflow: Workflow,
|
||||
stepCount: number,
|
||||
refreshWorkflowSteps: () => Promise<void>,
|
||||
toast: WorkflowStepActionsParams["toast"],
|
||||
) {
|
||||
try {
|
||||
await createWorkflowStepAction({
|
||||
workflow_id: workflow.id,
|
||||
...NEW_STEP_DEFAULTS,
|
||||
position: stepCount,
|
||||
});
|
||||
await refreshWorkflowSteps();
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Failed to add workflow step",
|
||||
description: error instanceof Error ? error.message : FALLBACK_ERROR_MESSAGE,
|
||||
variant: "error",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
function applyWorkflowStepUpdates(
|
||||
steps: WorkflowStep[],
|
||||
stepId: string,
|
||||
updates: Partial<WorkflowStep>,
|
||||
): WorkflowStep[] {
|
||||
const isSettingStartStep = updates.is_start_step === true;
|
||||
return steps.map((step) => {
|
||||
if (step.id === stepId) return { ...step, ...updates };
|
||||
if (isSettingStartStep) return { ...step, is_start_step: false };
|
||||
return step;
|
||||
});
|
||||
}
|
||||
|
||||
export function useWorkflowStepActions({
|
||||
workflow,
|
||||
isNewWorkflow,
|
||||
readOnly = false,
|
||||
workflowSteps,
|
||||
setWorkflowSteps,
|
||||
refreshWorkflowSteps,
|
||||
setStepToDelete,
|
||||
setStepTaskCount,
|
||||
setTargetStepForMigration,
|
||||
setStepDeleteOpen,
|
||||
toast,
|
||||
mutationGuard,
|
||||
}: WorkflowStepActionsParams) {
|
||||
const handleUpdateWorkflowStep = async (stepId: string, updates: Partial<WorkflowStep>) => {
|
||||
if (readOnly) return;
|
||||
@@ -156,66 +106,79 @@ export function useWorkflowStepActions({
|
||||
setWorkflowSteps((prev) => applyWorkflowStepUpdates(prev, stepId, updates));
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await updateWorkflowStepAction(stepId, updates);
|
||||
await refreshWorkflowSteps();
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Failed to update workflow step",
|
||||
description: error instanceof Error ? error.message : FALLBACK_ERROR_MESSAGE,
|
||||
variant: "error",
|
||||
});
|
||||
}
|
||||
const proposedSteps = applyWorkflowStepUpdates(workflowSteps, stepId, updates);
|
||||
await mutationGuard.guardMutation({
|
||||
proposedSteps,
|
||||
operation: () => updateRemoteWorkflowStep({ stepId, updates, setWorkflowSteps, toast }),
|
||||
});
|
||||
};
|
||||
|
||||
const handleAddWorkflowStep = async () => {
|
||||
if (readOnly) return;
|
||||
if (isNewWorkflow) {
|
||||
addLocalStep(workflow, setWorkflowSteps);
|
||||
return;
|
||||
}
|
||||
await addRemoteStep(workflow, workflowSteps.length, refreshWorkflowSteps, toast);
|
||||
const proposedSteps = [
|
||||
...workflowSteps,
|
||||
newWorkflowStep(workflow, workflowSteps.length, `proposed-step-${workflow.id}`),
|
||||
];
|
||||
await mutationGuard.guardMutation({
|
||||
proposedSteps,
|
||||
operation: () => addRemoteStep(workflow, workflowSteps.length, setWorkflowSteps, toast),
|
||||
});
|
||||
};
|
||||
|
||||
const handleRemoveWorkflowStep = async (stepId: string) => {
|
||||
if (readOnly) return;
|
||||
if (isNewWorkflow) {
|
||||
setWorkflowSteps((prev) =>
|
||||
prev.filter((s) => s.id !== stepId).map((s, i) => ({ ...s, position: i })),
|
||||
);
|
||||
removeLocalStep(stepId, setWorkflowSteps);
|
||||
return;
|
||||
}
|
||||
await removeWorkflowStep({
|
||||
stepId,
|
||||
workflowSteps,
|
||||
refreshWorkflowSteps,
|
||||
setStepToDelete,
|
||||
setStepTaskCount,
|
||||
setTargetStepForMigration,
|
||||
setStepDeleteOpen,
|
||||
toast,
|
||||
const proposedSteps = workflowSteps
|
||||
.filter((step) => step.id !== stepId)
|
||||
.map((step, position) => ({ ...step, position }));
|
||||
await mutationGuard.guardMutation({
|
||||
proposedSteps,
|
||||
operation: () =>
|
||||
removeWorkflowStep({
|
||||
stepId,
|
||||
workflowSteps,
|
||||
setWorkflowSteps,
|
||||
setStepToDelete,
|
||||
setStepTaskCount,
|
||||
setTargetStepForMigration,
|
||||
setStepDeleteOpen,
|
||||
toast,
|
||||
}),
|
||||
});
|
||||
};
|
||||
|
||||
const handleReorderWorkflowSteps = async (reorderedSteps: WorkflowStep[]) => {
|
||||
if (readOnly) return;
|
||||
setWorkflowSteps(reorderedSteps);
|
||||
if (isNewWorkflow) return;
|
||||
try {
|
||||
await reorderWorkflowStepsAction(
|
||||
workflow.id,
|
||||
reorderedSteps.map((s) => s.id),
|
||||
);
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Failed to reorder workflow steps",
|
||||
description: error instanceof Error ? error.message : FALLBACK_ERROR_MESSAGE,
|
||||
variant: "error",
|
||||
});
|
||||
await refreshWorkflowSteps();
|
||||
const proposedSteps = reorderedSteps.map((step, position) => ({ ...step, position }));
|
||||
if (isNewWorkflow) {
|
||||
setWorkflowSteps(proposedSteps);
|
||||
return;
|
||||
}
|
||||
await mutationGuard.guardMutation({
|
||||
proposedSteps,
|
||||
operation: async () => {
|
||||
setWorkflowSteps(proposedSteps);
|
||||
try {
|
||||
const response = await reorderWorkflowStepsAction(
|
||||
workflow.id,
|
||||
proposedSteps.map((step) => step.id),
|
||||
);
|
||||
setWorkflowSteps(response.steps);
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Failed to reorder workflow steps",
|
||||
description: error instanceof Error ? error.message : FALLBACK_ERROR_MESSAGE,
|
||||
variant: "error",
|
||||
});
|
||||
setWorkflowSteps(workflowSteps);
|
||||
}
|
||||
},
|
||||
});
|
||||
};
|
||||
|
||||
return {
|
||||
handleUpdateWorkflowStep,
|
||||
handleAddWorkflowStep,
|
||||
@@ -227,7 +190,6 @@ export function useWorkflowStepActions({
|
||||
type WorkflowDeleteHandlersParams = {
|
||||
workflow: Workflow;
|
||||
isNewWorkflow: boolean;
|
||||
/** See `WorkflowStepActionsParams.readOnly` — defense-in-depth for synced workflows. */
|
||||
readOnly?: boolean;
|
||||
otherWorkflows: Workflow[];
|
||||
wfDel: {
|
||||
@@ -303,8 +265,6 @@ export function useWorkflowDeleteHandlers({
|
||||
};
|
||||
|
||||
const handleDeleteWorkflow = async () => {
|
||||
// A background sync can flip the workflow read-only while the delete
|
||||
// dialog is already open; re-check at confirm time.
|
||||
if (readOnly) return;
|
||||
try {
|
||||
await deleteWorkflowRun();
|
||||
@@ -343,7 +303,6 @@ export function useWorkflowDeleteHandlers({
|
||||
|
||||
return { handleDeleteWorkflowClick, handleDeleteWorkflow, handleMigrateAndDeleteWorkflow };
|
||||
}
|
||||
|
||||
type StepDeleteHandlersParams = {
|
||||
workflow: Workflow;
|
||||
stepDel: {
|
||||
@@ -353,28 +312,33 @@ type StepDeleteHandlersParams = {
|
||||
setStepDeleteOpen: (v: boolean) => void;
|
||||
setStepToDelete: (v: string | null) => void;
|
||||
};
|
||||
refreshWorkflowSteps: () => Promise<void>;
|
||||
setWorkflowSteps: (updater: (prev: WorkflowStep[]) => WorkflowStep[]) => void;
|
||||
toast: ReturnType<typeof useToast>["toast"];
|
||||
};
|
||||
|
||||
export function useStepDeleteHandlers({
|
||||
workflow,
|
||||
stepDel,
|
||||
refreshWorkflowSteps,
|
||||
setWorkflowSteps,
|
||||
toast,
|
||||
}: StepDeleteHandlersParams) {
|
||||
const handleMigrateAndDeleteStep = async () => {
|
||||
if (!stepDel.stepToDelete || !stepDel.targetStepForMigration) return;
|
||||
const stepId = stepDel.stepToDelete;
|
||||
stepDel.setStepMigrateLoading(true);
|
||||
try {
|
||||
await bulkMoveTasks({
|
||||
source_workflow_id: workflow.id,
|
||||
source_step_id: stepDel.stepToDelete,
|
||||
source_step_id: stepId,
|
||||
target_workflow_id: workflow.id,
|
||||
target_step_id: stepDel.targetStepForMigration,
|
||||
});
|
||||
await deleteWorkflowStepAction(stepDel.stepToDelete);
|
||||
await refreshWorkflowSteps();
|
||||
await deleteWorkflowStepAction(stepId);
|
||||
setWorkflowSteps((previous) =>
|
||||
previous
|
||||
.filter((step) => step.id !== stepId)
|
||||
.map((step, position) => ({ ...step, position })),
|
||||
);
|
||||
stepDel.setStepDeleteOpen(false);
|
||||
stepDel.setStepToDelete(null);
|
||||
} catch (error) {
|
||||
@@ -390,10 +354,15 @@ export function useStepDeleteHandlers({
|
||||
|
||||
const handleDeleteStepAndTasks = async () => {
|
||||
if (!stepDel.stepToDelete) return;
|
||||
const stepId = stepDel.stepToDelete;
|
||||
stepDel.setStepMigrateLoading(true);
|
||||
try {
|
||||
await deleteWorkflowStepAction(stepDel.stepToDelete);
|
||||
await refreshWorkflowSteps();
|
||||
await deleteWorkflowStepAction(stepId);
|
||||
setWorkflowSteps((previous) =>
|
||||
previous
|
||||
.filter((step) => step.id !== stepId)
|
||||
.map((step, position) => ({ ...step, position })),
|
||||
);
|
||||
stepDel.setStepDeleteOpen(false);
|
||||
stepDel.setStepToDelete(null);
|
||||
} catch (error) {
|
||||
@@ -525,13 +494,13 @@ async function reconcileTemplateSteps(
|
||||
type WorkflowSaveActionsParams = {
|
||||
workflow: Workflow;
|
||||
isNewWorkflow: boolean;
|
||||
/** See `WorkflowStepActionsParams.readOnly` — defense-in-depth for synced workflows. */
|
||||
readOnly?: boolean;
|
||||
workflowSteps: WorkflowStep[];
|
||||
templateStepCount: number;
|
||||
onSaveWorkflow: () => Promise<unknown>;
|
||||
onWorkflowCreated?: (created: Workflow) => void;
|
||||
toast: ReturnType<typeof useToast>["toast"];
|
||||
mutationGuard: WorkflowMutationGuardController;
|
||||
};
|
||||
|
||||
export function useWorkflowSaveActions({
|
||||
@@ -543,6 +512,7 @@ export function useWorkflowSaveActions({
|
||||
onSaveWorkflow,
|
||||
onWorkflowCreated,
|
||||
toast,
|
||||
mutationGuard,
|
||||
}: WorkflowSaveActionsParams) {
|
||||
const saveWorkflowRequest = useRequest(onSaveWorkflow);
|
||||
|
||||
@@ -567,8 +537,7 @@ export function useWorkflowSaveActions({
|
||||
|
||||
const activeSaveRequest = isNewWorkflow ? saveNewWorkflowRequest : saveWorkflowRequest;
|
||||
|
||||
const handleSaveWorkflow = async () => {
|
||||
if (readOnly) return;
|
||||
const runSaveWorkflow = async () => {
|
||||
try {
|
||||
if (isNewWorkflow) await saveNewWorkflowRequest.run();
|
||||
else await saveWorkflowRequest.run();
|
||||
@@ -581,7 +550,21 @@ export function useWorkflowSaveActions({
|
||||
}
|
||||
};
|
||||
|
||||
return { activeSaveRequest, handleSaveWorkflow };
|
||||
const handleSaveWorkflow = async () => {
|
||||
if (readOnly) return;
|
||||
if (!isNewWorkflow) {
|
||||
await runSaveWorkflow();
|
||||
return;
|
||||
}
|
||||
await mutationGuard.guardMutation({
|
||||
baselineSteps: [],
|
||||
proposedSteps: workflowSteps,
|
||||
intent: "create",
|
||||
operation: runSaveWorkflow,
|
||||
});
|
||||
};
|
||||
|
||||
return { activeSaveRequest, handleSaveWorkflow, mutationGuard };
|
||||
}
|
||||
|
||||
type WorkflowExportActionsParams = {
|
||||
|
||||
@@ -3,13 +3,14 @@
|
||||
import { useState, useEffect } from "react";
|
||||
import { IconDownload, IconTrash } from "@tabler/icons-react";
|
||||
import { Card, CardContent } from "@kandev/ui/card";
|
||||
import { Badge } from "@kandev/ui/badge";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import { Badge } from "@kandev/ui/badge";
|
||||
import { Input } from "@kandev/ui/input";
|
||||
import { Label } from "@kandev/ui/label";
|
||||
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import type { Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import type { WorkflowReplayCycleDiagnostic } from "@/lib/workflows/replay-cycle-analysis";
|
||||
import { useHealthyAgentProfiles } from "@/hooks/domains/settings/use-healthy-agent-profiles";
|
||||
import { useRequest } from "@/lib/http/use-request";
|
||||
import { useToast } from "@/components/toast-provider";
|
||||
@@ -26,6 +27,8 @@ import {
|
||||
useWorkflowSaveActions,
|
||||
handleExportWorkflow,
|
||||
} from "./workflow-card-actions";
|
||||
import { useWorkflowMutationGuard } from "./workflow-mutation-guard";
|
||||
import { WorkflowCycleGuardDialog } from "./workflow-cycle-diagnostic";
|
||||
|
||||
type WorkflowCardProps = {
|
||||
workflow: Workflow;
|
||||
@@ -76,16 +79,7 @@ function useWorkflowSteps(
|
||||
};
|
||||
}, [workflowId, initialSteps, isNewWorkflow, toast]);
|
||||
|
||||
const refreshWorkflowSteps = async () => {
|
||||
try {
|
||||
const res = await listWorkflowStepsAction(workflowId);
|
||||
setWorkflowSteps(res.steps ?? []);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
};
|
||||
|
||||
return { workflowSteps, setWorkflowSteps, workflowLoading, refreshWorkflowSteps };
|
||||
return { workflowSteps, setWorkflowSteps, workflowLoading };
|
||||
}
|
||||
|
||||
type WorkflowDeleteState = {
|
||||
@@ -176,17 +170,13 @@ type WorkflowCardActionsProps = {
|
||||
};
|
||||
|
||||
const SYNCED_READ_ONLY_REASON =
|
||||
"Managed by workflow sync — edit or remove it in the synced repository";
|
||||
"Managed by workflow sync - edit or remove it in the synced repository";
|
||||
|
||||
function DeleteWorkflowButton({
|
||||
onDeleteClick,
|
||||
deleteDisabled,
|
||||
readOnly,
|
||||
}: {
|
||||
onDeleteClick: () => Promise<void>;
|
||||
deleteDisabled: boolean;
|
||||
readOnly: boolean;
|
||||
}) {
|
||||
}: Pick<WorkflowCardActionsProps, "onDeleteClick" | "deleteDisabled" | "readOnly">) {
|
||||
const button = (
|
||||
<Button
|
||||
type="button"
|
||||
@@ -202,7 +192,6 @@ function DeleteWorkflowButton({
|
||||
);
|
||||
|
||||
if (!readOnly) return button;
|
||||
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
@@ -264,10 +253,12 @@ type WorkflowCardDialogsProps = {
|
||||
handleMigrateAndDeleteStep: () => Promise<void>;
|
||||
handleDeleteStepAndTasks: () => Promise<void>;
|
||||
};
|
||||
mutationGuard: ReturnType<typeof useWorkflowMutationGuard>;
|
||||
};
|
||||
|
||||
type WorkflowCardBodyProps = {
|
||||
workflow: Workflow;
|
||||
readOnly: boolean;
|
||||
isWorkflowDirty: boolean;
|
||||
onUpdateWorkflow: (updates: {
|
||||
name?: string;
|
||||
@@ -276,21 +267,18 @@ type WorkflowCardBodyProps = {
|
||||
}) => void;
|
||||
activeSaveRequest: { isLoading: boolean; status: "idle" | "loading" | "success" | "error" };
|
||||
handleSaveWorkflow: () => Promise<void>;
|
||||
mutationPending: boolean;
|
||||
workflowLoading: boolean;
|
||||
workflowSteps: WorkflowStep[];
|
||||
diagnostics: WorkflowReplayCycleDiagnostic[];
|
||||
stepActions: {
|
||||
handleUpdateWorkflowStep: (id: string, updates: Partial<WorkflowStep>) => Promise<void>;
|
||||
handleAddWorkflowStep: () => Promise<void>;
|
||||
handleRemoveWorkflowStep: (id: string) => Promise<void>;
|
||||
handleReorderWorkflowSteps: (steps: WorkflowStep[]) => Promise<void>;
|
||||
};
|
||||
readOnly: boolean;
|
||||
};
|
||||
|
||||
// SyncedBadge marks workflows managed by workflow sync (workflow.source ===
|
||||
// "github"). Hovering / focusing shows which repo-relative file it came from
|
||||
// so users don't accidentally edit a workflow that the next poll will
|
||||
// overwrite.
|
||||
function SyncedBadge({ sourcePath }: { sourcePath?: string }) {
|
||||
return (
|
||||
<Tooltip>
|
||||
@@ -305,7 +293,7 @@ function SyncedBadge({ sourcePath }: { sourcePath?: string }) {
|
||||
</Badge>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>
|
||||
Read-only — managed by workflow sync from {sourcePath || "a configured repository"}. Edit or
|
||||
Read-only - managed by workflow sync from {sourcePath || "a configured repository"}. Edit or
|
||||
remove it in the synced repository.
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
@@ -314,14 +302,16 @@ function SyncedBadge({ sourcePath }: { sourcePath?: string }) {
|
||||
|
||||
function WorkflowCardBody({
|
||||
workflow,
|
||||
readOnly,
|
||||
isWorkflowDirty,
|
||||
onUpdateWorkflow,
|
||||
activeSaveRequest,
|
||||
handleSaveWorkflow,
|
||||
mutationPending,
|
||||
workflowLoading,
|
||||
workflowSteps,
|
||||
diagnostics,
|
||||
stepActions,
|
||||
readOnly,
|
||||
}: WorkflowCardBodyProps) {
|
||||
const healthyProfiles = useHealthyAgentProfiles(workflow.agent_profile_id);
|
||||
|
||||
@@ -333,11 +323,6 @@ function WorkflowCardBody({
|
||||
<span>Workflow Name</span>
|
||||
{isWorkflowDirty && <UnsavedChangesBadge />}
|
||||
{readOnly && <SyncedBadge sourcePath={workflow.source_path} />}
|
||||
{readOnly && (
|
||||
<span className="text-xs text-muted-foreground">
|
||||
Read-only — managed by workflow sync
|
||||
</span>
|
||||
)}
|
||||
</Label>
|
||||
<Input
|
||||
value={workflow.name}
|
||||
@@ -352,10 +337,10 @@ function WorkflowCardBody({
|
||||
</Label>
|
||||
<Select
|
||||
value={workflow.agent_profile_id || "none"}
|
||||
disabled={readOnly}
|
||||
onValueChange={(value) =>
|
||||
onUpdateWorkflow({ agent_profile_id: value === "none" ? "" : value })
|
||||
}
|
||||
disabled={readOnly}
|
||||
>
|
||||
<SelectTrigger
|
||||
className="w-full cursor-pointer"
|
||||
@@ -380,7 +365,7 @@ function WorkflowCardBody({
|
||||
isLoading={activeSaveRequest.isLoading}
|
||||
status={activeSaveRequest.status}
|
||||
onClick={handleSaveWorkflow}
|
||||
disabled={readOnly}
|
||||
disabled={mutationPending || readOnly}
|
||||
/>
|
||||
</div>
|
||||
<div className="space-y-2">
|
||||
@@ -390,11 +375,12 @@ function WorkflowCardBody({
|
||||
) : (
|
||||
<WorkflowPipelineEditor
|
||||
steps={workflowSteps}
|
||||
diagnostics={diagnostics}
|
||||
onUpdateStep={stepActions.handleUpdateWorkflowStep}
|
||||
onAddStep={stepActions.handleAddWorkflowStep}
|
||||
onRemoveStep={stepActions.handleRemoveWorkflowStep}
|
||||
onReorderSteps={stepActions.handleReorderWorkflowSteps}
|
||||
readOnly={readOnly}
|
||||
readOnly={mutationPending || readOnly}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
@@ -413,6 +399,7 @@ function WorkflowCardDialogs({
|
||||
stepDel,
|
||||
stepsForStepMigration,
|
||||
stepDeleteHandlers,
|
||||
mutationGuard,
|
||||
}: WorkflowCardDialogsProps) {
|
||||
return (
|
||||
<>
|
||||
@@ -448,6 +435,11 @@ function WorkflowCardDialogs({
|
||||
onMigrateAndDelete={stepDeleteHandlers.handleMigrateAndDeleteStep}
|
||||
onDeleteAndTasks={stepDeleteHandlers.handleDeleteStepAndTasks}
|
||||
/>
|
||||
<WorkflowCycleGuardDialog
|
||||
proposal={mutationGuard.proposal}
|
||||
onCancel={mutationGuard.cancelProposal}
|
||||
onConfirm={mutationGuard.confirmProposal}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -461,25 +453,27 @@ function useWorkflowCardState(props: WorkflowCardProps) {
|
||||
const wfDel = useWorkflowDeleteState();
|
||||
const stepDel = useStepDeleteState();
|
||||
const isNewWorkflow = workflow.id.startsWith("temp-");
|
||||
// Workflows synced from a configured GitHub repo are read-only: the
|
||||
// backend rejects definition mutations with a 409, so the UI disables the
|
||||
// matching affordances (name/agent-profile/steps/delete) up front.
|
||||
const readOnly = workflow.source === "github";
|
||||
const deleteWorkflowRequest = useRequest(onDeleteWorkflow);
|
||||
const { workflowSteps, setWorkflowSteps, workflowLoading, refreshWorkflowSteps } =
|
||||
useWorkflowSteps(workflow.id, initialWorkflowSteps, isNewWorkflow, toast);
|
||||
const { workflowSteps, setWorkflowSteps, workflowLoading } = useWorkflowSteps(
|
||||
workflow.id,
|
||||
initialWorkflowSteps,
|
||||
isNewWorkflow,
|
||||
toast,
|
||||
);
|
||||
const mutationGuard = useWorkflowMutationGuard(workflowSteps);
|
||||
const stepActions = useWorkflowStepActions({
|
||||
workflow,
|
||||
isNewWorkflow,
|
||||
readOnly,
|
||||
workflowSteps,
|
||||
setWorkflowSteps,
|
||||
refreshWorkflowSteps,
|
||||
setStepToDelete: stepDel.setStepToDelete,
|
||||
setStepTaskCount: stepDel.setStepTaskCount,
|
||||
setTargetStepForMigration: stepDel.setTargetStepForMigration,
|
||||
setStepDeleteOpen: stepDel.setStepDeleteOpen,
|
||||
toast,
|
||||
mutationGuard,
|
||||
});
|
||||
const { activeSaveRequest, handleSaveWorkflow } = useWorkflowSaveActions({
|
||||
workflow,
|
||||
@@ -490,6 +484,7 @@ function useWorkflowCardState(props: WorkflowCardProps) {
|
||||
onSaveWorkflow,
|
||||
onWorkflowCreated,
|
||||
toast,
|
||||
mutationGuard,
|
||||
});
|
||||
const wfDeleteHandlers = useWorkflowDeleteHandlers({
|
||||
workflow,
|
||||
@@ -503,7 +498,7 @@ function useWorkflowCardState(props: WorkflowCardProps) {
|
||||
const stepDeleteHandlers = useStepDeleteHandlers({
|
||||
workflow,
|
||||
stepDel,
|
||||
refreshWorkflowSteps,
|
||||
setWorkflowSteps,
|
||||
toast,
|
||||
});
|
||||
const stepsForStepMigration = stepDel.stepToDelete
|
||||
@@ -522,6 +517,7 @@ function useWorkflowCardState(props: WorkflowCardProps) {
|
||||
deleteWorkflowRequest,
|
||||
workflowSteps,
|
||||
workflowLoading,
|
||||
mutationGuard,
|
||||
stepActions,
|
||||
activeSaveRequest,
|
||||
handleSaveWorkflow,
|
||||
@@ -544,14 +540,16 @@ export function WorkflowCard(props: WorkflowCardProps) {
|
||||
<div className="space-y-4">
|
||||
<WorkflowCardBody
|
||||
workflow={workflow}
|
||||
readOnly={s.readOnly}
|
||||
isWorkflowDirty={isWorkflowDirty}
|
||||
onUpdateWorkflow={onUpdateWorkflow}
|
||||
activeSaveRequest={s.activeSaveRequest}
|
||||
handleSaveWorkflow={s.handleSaveWorkflow}
|
||||
mutationPending={s.mutationGuard.isMutationPending}
|
||||
workflowLoading={s.workflowLoading}
|
||||
workflowSteps={s.workflowSteps}
|
||||
diagnostics={s.mutationGuard.diagnostics}
|
||||
stepActions={s.stepActions}
|
||||
readOnly={s.readOnly}
|
||||
/>
|
||||
<WorkflowCardActions
|
||||
isNewWorkflow={s.isNewWorkflow}
|
||||
@@ -561,7 +559,10 @@ export function WorkflowCard(props: WorkflowCardProps) {
|
||||
toast={s.toast}
|
||||
onDeleteClick={s.wfDeleteHandlers.handleDeleteWorkflowClick}
|
||||
deleteDisabled={
|
||||
s.deleteWorkflowRequest.isLoading || s.wfDel.workflowDeleteLoading || s.readOnly
|
||||
s.mutationGuard.isMutationPending ||
|
||||
s.deleteWorkflowRequest.isLoading ||
|
||||
s.wfDel.workflowDeleteLoading ||
|
||||
s.readOnly
|
||||
}
|
||||
readOnly={s.readOnly}
|
||||
/>
|
||||
@@ -578,6 +579,7 @@ export function WorkflowCard(props: WorkflowCardProps) {
|
||||
stepDel={s.stepDel}
|
||||
stepsForStepMigration={s.stepsForStepMigration}
|
||||
stepDeleteHandlers={s.stepDeleteHandlers}
|
||||
mutationGuard={s.mutationGuard}
|
||||
/>
|
||||
</Card>
|
||||
);
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, fireEvent, render, screen, within } from "@testing-library/react";
|
||||
import type { WorkflowStep } from "@/lib/types/http";
|
||||
import type { WorkflowReplayCycleDiagnostic } from "@/lib/workflows/replay-cycle-analysis";
|
||||
import { WorkflowPipelineEditor } from "./workflow-pipeline-editor";
|
||||
import { WorkflowCycleDiagnostic, WorkflowCycleGuardDialog } from "./workflow-cycle-diagnostic";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
const AUTO_START_STEP_NAME = "In Progress";
|
||||
|
||||
const diagnostic: WorkflowReplayCycleDiagnostic = {
|
||||
identity: "cycle-work-review",
|
||||
severity: "warning",
|
||||
autoStartStepId: "work",
|
||||
autoStartStepName: AUTO_START_STEP_NAME,
|
||||
affectedStepIds: ["work", "review", "done"],
|
||||
trace: [
|
||||
{
|
||||
sourceStepId: "work",
|
||||
sourceStepName: AUTO_START_STEP_NAME,
|
||||
trigger: "on_turn_complete",
|
||||
actionKind: "move_to_next",
|
||||
destinationStepId: "review",
|
||||
destinationStepName: "Review",
|
||||
requiresUserInvolvement: false,
|
||||
},
|
||||
{
|
||||
sourceStepId: "review",
|
||||
sourceStepName: "Review",
|
||||
trigger: "on_turn_start",
|
||||
actionKind: "move_to_step",
|
||||
destinationStepId: "done",
|
||||
destinationStepName: "Done",
|
||||
requiresUserInvolvement: true,
|
||||
},
|
||||
{
|
||||
sourceStepId: "done",
|
||||
sourceStepName: "Done",
|
||||
trigger: "on_turn_complete",
|
||||
actionKind: "move_to_previous",
|
||||
destinationStepId: "work",
|
||||
destinationStepName: AUTO_START_STEP_NAME,
|
||||
requiresUserInvolvement: true,
|
||||
},
|
||||
],
|
||||
promptSource: "task_description",
|
||||
};
|
||||
|
||||
function workflowStep(id: string, name: string, position: number): WorkflowStep {
|
||||
return {
|
||||
id,
|
||||
workflow_id: "workflow-1",
|
||||
name,
|
||||
position,
|
||||
color: "bg-slate-500",
|
||||
created_at: "2026-07-15T00:00:00.000Z",
|
||||
updated_at: "2026-07-15T00:00:00.000Z",
|
||||
} as WorkflowStep;
|
||||
}
|
||||
|
||||
describe("WorkflowCycleDiagnostic", () => {
|
||||
it("renders severity, the exact ordered trace, user-required hops, and prompt source", () => {
|
||||
render(<WorkflowCycleDiagnostic diagnostic={diagnostic} />);
|
||||
|
||||
expect(screen.getByText("Potential repeated agent run")).toBeTruthy();
|
||||
const hops = screen.getAllByRole("listitem");
|
||||
expect(hops).toHaveLength(3);
|
||||
expect(hops[0].textContent).toContain("In ProgressReviewOn turn completeMove to next step");
|
||||
expect(hops[1].textContent).toContain("ReviewDoneOn turn startMove to specific step");
|
||||
expect(hops[2].textContent).toContain("DoneIn ProgressOn turn completeMove to previous step");
|
||||
expect(within(hops[0]).queryByText("User action required")).toBeNull();
|
||||
expect(within(hops[1]).getByText("User action required")).toBeTruthy();
|
||||
expect(within(hops[2]).getByText("User action required")).toBeTruthy();
|
||||
expect(
|
||||
screen.getByText(/has no step prompt, so re-entering it sends the task description/i),
|
||||
).toBeTruthy();
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
"step_prompt_with_task_description" as const,
|
||||
/rendered step prompt including the task description/i,
|
||||
],
|
||||
["step_prompt" as const, /step prompt instead of the task description/i],
|
||||
])("explains the %s prompt source", (promptSource, expected) => {
|
||||
render(<WorkflowCycleDiagnostic diagnostic={{ ...diagnostic, promptSource }} />);
|
||||
expect(screen.getByText(expected)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("renders blocking severity separately from a warning", () => {
|
||||
render(<WorkflowCycleDiagnostic diagnostic={{ ...diagnostic, severity: "blocking" }} />);
|
||||
expect(screen.getByText("Automatic workflow cycle")).toBeTruthy();
|
||||
expect(screen.getByText(/without another user action/i)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("WorkflowCycleGuardDialog", () => {
|
||||
it("offers no override for a blocking proposal", () => {
|
||||
const onCancel = vi.fn();
|
||||
render(
|
||||
<WorkflowCycleGuardDialog
|
||||
proposal={{
|
||||
diagnostics: [{ ...diagnostic, severity: "blocking" }],
|
||||
intent: "apply",
|
||||
severity: "blocking",
|
||||
}}
|
||||
onCancel={onCancel}
|
||||
onConfirm={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByRole("heading", { name: "Workflow cycle blocked" })).toBeTruthy();
|
||||
expect(screen.queryByRole("button", { name: /anyway/i })).toBeNull();
|
||||
fireEvent.click(screen.getByRole("button", { name: "Return to workflow" }));
|
||||
expect(onCancel).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["apply" as const, "Apply anyway"],
|
||||
["create" as const, "Create anyway"],
|
||||
])("confirms a warning proposal with %s intent", (intent, actionLabel) => {
|
||||
const onConfirm = vi.fn();
|
||||
const onCancel = vi.fn();
|
||||
render(
|
||||
<WorkflowCycleGuardDialog
|
||||
proposal={{ diagnostics: [diagnostic], intent, severity: "warning" }}
|
||||
onCancel={onCancel}
|
||||
onConfirm={onConfirm}
|
||||
/>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: actionLabel }));
|
||||
expect(onConfirm).toHaveBeenCalledOnce();
|
||||
expect(onCancel).not.toHaveBeenCalled();
|
||||
expect(
|
||||
screen.getByRole("button", { name: actionLabel }).hasAttribute("data-dialog-default-action"),
|
||||
).toBe(true);
|
||||
expect(screen.getByRole("button", { name: "Cancel" })).toBeTruthy();
|
||||
});
|
||||
|
||||
it("uses the semantic dialog action for Enter confirmation", () => {
|
||||
const onConfirm = vi.fn();
|
||||
render(
|
||||
<WorkflowCycleGuardDialog
|
||||
proposal={{ diagnostics: [diagnostic], intent: "apply", severity: "warning" }}
|
||||
onCancel={vi.fn()}
|
||||
onConfirm={onConfirm}
|
||||
/>,
|
||||
);
|
||||
|
||||
fireEvent.keyDown(screen.getByTestId("workflow-cycle-guard-dialog"), { key: "Enter" });
|
||||
expect(onConfirm).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
describe("WorkflowPipelineEditor replay-cycle marker", () => {
|
||||
it("marks affected nodes with visible and accessible text", () => {
|
||||
render(
|
||||
<WorkflowPipelineEditor
|
||||
steps={[workflowStep("work", AUTO_START_STEP_NAME, 0), workflowStep("review", "Review", 1)]}
|
||||
diagnostics={[diagnostic]}
|
||||
onUpdateStep={() => {}}
|
||||
onAddStep={() => {}}
|
||||
onRemoveStep={() => {}}
|
||||
onReorderSteps={() => {}}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByLabelText("In Progress is part of a replay cycle")).toBeTruthy();
|
||||
expect(screen.getByLabelText("Review is part of a replay cycle")).toBeTruthy();
|
||||
expect(screen.getAllByText("Cycle")).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,188 @@
|
||||
"use client";
|
||||
|
||||
import { useRef } from "react";
|
||||
import { IconAlertTriangle, IconArrowRight, IconUser } from "@tabler/icons-react";
|
||||
import { Alert, AlertDescription, AlertTitle } from "@kandev/ui/alert";
|
||||
import {
|
||||
AlertDialog,
|
||||
AlertDialogAction,
|
||||
AlertDialogCancel,
|
||||
AlertDialogContent,
|
||||
AlertDialogDescription,
|
||||
AlertDialogFooter,
|
||||
AlertDialogHeader,
|
||||
AlertDialogTitle,
|
||||
} from "@kandev/ui/alert-dialog";
|
||||
import type {
|
||||
WorkflowReplayCycleDiagnostic,
|
||||
WorkflowReplayCycleHop,
|
||||
} from "@/lib/workflows/replay-cycle-analysis";
|
||||
import type { WorkflowMutationProposal } from "./workflow-mutation-guard";
|
||||
import { cn } from "@/lib/utils";
|
||||
|
||||
const PROMPT_SOURCE_TEXT = {
|
||||
task_description: (stepName: string) =>
|
||||
`"${stepName}" has no step prompt, so re-entering it sends the task description.`,
|
||||
step_prompt_with_task_description: (stepName: string) =>
|
||||
`Re-entering "${stepName}" sends its rendered step prompt including the task description.`,
|
||||
step_prompt: (stepName: string) =>
|
||||
`Re-entering "${stepName}" sends its step prompt instead of the task description.`,
|
||||
} as const;
|
||||
|
||||
const TRIGGER_LABELS = {
|
||||
on_turn_start: "On turn start",
|
||||
on_turn_complete: "On turn complete",
|
||||
} as const;
|
||||
|
||||
const ACTION_LABELS = {
|
||||
move_to_next: "Move to next step",
|
||||
move_to_previous: "Move to previous step",
|
||||
move_to_step: "Move to specific step",
|
||||
} as const;
|
||||
|
||||
function CycleHop({ hop, index }: { hop: WorkflowReplayCycleHop; index: number }) {
|
||||
return (
|
||||
<li className="min-w-0 rounded-md border bg-background/60 p-3">
|
||||
<div className="grid min-w-0 grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)] items-center gap-2">
|
||||
<span className="min-w-0 break-words font-medium">{hop.sourceStepName}</span>
|
||||
<IconArrowRight className="size-4 shrink-0 text-muted-foreground" aria-hidden="true" />
|
||||
<span className="min-w-0 break-words text-right font-medium">
|
||||
{hop.destinationStepName}
|
||||
</span>
|
||||
</div>
|
||||
<div className="mt-2 flex min-w-0 flex-wrap items-center gap-1.5 text-xs text-muted-foreground">
|
||||
<span className="whitespace-nowrap rounded bg-muted px-2 py-1">
|
||||
{TRIGGER_LABELS[hop.trigger]}
|
||||
</span>
|
||||
<span className="whitespace-nowrap rounded bg-muted px-2 py-1">
|
||||
{ACTION_LABELS[hop.actionKind]}
|
||||
</span>
|
||||
</div>
|
||||
{hop.requiresUserInvolvement && (
|
||||
<div className="mt-2 flex items-center gap-1.5 text-xs font-medium text-foreground">
|
||||
<IconUser className="size-3.5 shrink-0" aria-hidden="true" />
|
||||
<span>User action required</span>
|
||||
</div>
|
||||
)}
|
||||
<span className="sr-only">Hop {index + 1}</span>
|
||||
</li>
|
||||
);
|
||||
}
|
||||
|
||||
export function WorkflowCycleDiagnostic({
|
||||
diagnostic,
|
||||
}: {
|
||||
diagnostic: WorkflowReplayCycleDiagnostic;
|
||||
}) {
|
||||
const isBlocking = diagnostic.severity === "blocking";
|
||||
const promptText = PROMPT_SOURCE_TEXT[diagnostic.promptSource](diagnostic.autoStartStepName);
|
||||
|
||||
return (
|
||||
<Alert
|
||||
variant={isBlocking ? "destructive" : "default"}
|
||||
className={cn(
|
||||
"min-w-0 overflow-hidden p-3 text-sm",
|
||||
!isBlocking && "border-amber-500/60 bg-amber-500/5",
|
||||
)}
|
||||
data-testid={`workflow-cycle-diagnostic-${diagnostic.autoStartStepId}`}
|
||||
>
|
||||
<IconAlertTriangle className="mt-0.5 size-4" aria-hidden="true" />
|
||||
<AlertTitle className="text-sm">
|
||||
{isBlocking ? "Automatic workflow cycle" : "Potential repeated agent run"}
|
||||
</AlertTitle>
|
||||
<AlertDescription className="min-w-0 space-y-3 text-left text-sm text-pretty">
|
||||
<p>
|
||||
{isBlocking
|
||||
? `This path re-enters "${diagnostic.autoStartStepName}" and can start the agent again without another user action.`
|
||||
: `This path can re-enter "${diagnostic.autoStartStepName}" and start the agent again after a user action.`}
|
||||
</p>
|
||||
<ol
|
||||
aria-label={`Replay path for ${diagnostic.autoStartStepName}`}
|
||||
className="grid min-w-0 gap-2"
|
||||
>
|
||||
{diagnostic.trace.map((hop, index) => (
|
||||
<CycleHop
|
||||
key={`${diagnostic.identity}-${hop.sourceStepId}-${hop.trigger}-${index}`}
|
||||
hop={hop}
|
||||
index={index}
|
||||
/>
|
||||
))}
|
||||
</ol>
|
||||
<p className="break-words">{promptText}</p>
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
);
|
||||
}
|
||||
|
||||
type WorkflowCycleGuardDialogProps = {
|
||||
proposal: WorkflowMutationProposal | null;
|
||||
onCancel: () => void;
|
||||
onConfirm: () => void | Promise<void>;
|
||||
};
|
||||
|
||||
export function WorkflowCycleGuardDialog({
|
||||
proposal,
|
||||
onCancel,
|
||||
onConfirm,
|
||||
}: WorkflowCycleGuardDialogProps) {
|
||||
const isBlocking = proposal?.severity === "blocking";
|
||||
const actionLabel = proposal?.intent === "create" ? "Create anyway" : "Apply anyway";
|
||||
const confirming = useRef(false);
|
||||
|
||||
const handleConfirm = async () => {
|
||||
confirming.current = true;
|
||||
try {
|
||||
await onConfirm();
|
||||
} finally {
|
||||
confirming.current = false;
|
||||
}
|
||||
};
|
||||
|
||||
return (
|
||||
<AlertDialog
|
||||
open={proposal !== null}
|
||||
onOpenChange={(open) => !open && !confirming.current && onCancel()}
|
||||
>
|
||||
<AlertDialogContent
|
||||
className="max-h-[calc(100dvh-2rem)] max-w-[calc(100vw-2rem)] grid-rows-[auto_minmax(0,1fr)_auto] gap-0 overflow-hidden p-0 sm:max-w-2xl"
|
||||
enterConfirms={!isBlocking}
|
||||
data-testid="workflow-cycle-guard-dialog"
|
||||
>
|
||||
<AlertDialogHeader className="place-items-start p-4 pb-3 text-left sm:p-6 sm:pb-4">
|
||||
<AlertDialogTitle className="text-lg">
|
||||
{isBlocking ? "Workflow cycle blocked" : "Confirm workflow cycle"}
|
||||
</AlertDialogTitle>
|
||||
<AlertDialogDescription className="text-left text-sm">
|
||||
{isBlocking
|
||||
? "Change the workflow steps to remove the automatic cycle before continuing."
|
||||
: "Review the repeated agent run before continuing."}
|
||||
</AlertDialogDescription>
|
||||
</AlertDialogHeader>
|
||||
<div
|
||||
className="min-h-0 overflow-x-hidden overflow-y-auto px-4 pb-4 sm:px-6"
|
||||
data-testid="workflow-cycle-guard-scroll"
|
||||
>
|
||||
<div className="grid min-w-0 gap-3">
|
||||
{proposal?.diagnostics.map((diagnostic) => (
|
||||
<WorkflowCycleDiagnostic key={diagnostic.identity} diagnostic={diagnostic} />
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
<AlertDialogFooter className="border-t bg-background p-4 sm:px-6">
|
||||
<AlertDialogCancel className="min-h-12 w-full cursor-pointer sm:w-auto">
|
||||
{isBlocking ? "Return to workflow" : "Cancel"}
|
||||
</AlertDialogCancel>
|
||||
{!isBlocking && (
|
||||
<AlertDialogAction
|
||||
data-dialog-default-action
|
||||
className="min-h-12 w-full cursor-pointer sm:w-auto"
|
||||
onClick={handleConfirm}
|
||||
>
|
||||
{actionLabel}
|
||||
</AlertDialogAction>
|
||||
)}
|
||||
</AlertDialogFooter>
|
||||
</AlertDialogContent>
|
||||
</AlertDialog>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,118 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useMemo, useRef, useState } from "react";
|
||||
import type { WorkflowStep } from "@/lib/types/http";
|
||||
import {
|
||||
analyzeIntroducedWorkflowReplayCycles,
|
||||
analyzeWorkflowReplayCycles,
|
||||
type WorkflowReplayCycleDiagnostic,
|
||||
type WorkflowReplayCycleSeverity,
|
||||
} from "@/lib/workflows/replay-cycle-analysis";
|
||||
|
||||
export type WorkflowMutationIntent = "apply" | "create";
|
||||
|
||||
export type WorkflowMutationProposal = {
|
||||
diagnostics: WorkflowReplayCycleDiagnostic[];
|
||||
intent: WorkflowMutationIntent;
|
||||
severity: WorkflowReplayCycleSeverity;
|
||||
};
|
||||
|
||||
type GuardMutationParams = {
|
||||
proposedSteps: WorkflowStep[];
|
||||
operation: () => Promise<void>;
|
||||
intent?: WorkflowMutationIntent;
|
||||
baselineSteps?: WorkflowStep[];
|
||||
};
|
||||
|
||||
export type WorkflowMutationGuardController = {
|
||||
diagnostics: WorkflowReplayCycleDiagnostic[];
|
||||
proposal: WorkflowMutationProposal | null;
|
||||
isMutationPending: boolean;
|
||||
guardMutation: (params: GuardMutationParams) => Promise<void>;
|
||||
confirmProposal: () => Promise<void>;
|
||||
cancelProposal: () => void;
|
||||
};
|
||||
|
||||
type MutationPhase = "idle" | "proposal" | "running";
|
||||
|
||||
export function useWorkflowMutationGuard(
|
||||
displayedSteps: WorkflowStep[],
|
||||
): WorkflowMutationGuardController {
|
||||
const diagnostics = useMemo(() => analyzeWorkflowReplayCycles(displayedSteps), [displayedSteps]);
|
||||
const [proposal, setProposal] = useState<WorkflowMutationProposal | null>(null);
|
||||
const [isMutationPending, setIsMutationPending] = useState(false);
|
||||
const mutationPhase = useRef<MutationPhase>("idle");
|
||||
const pendingOperation = useRef<(() => Promise<void>) | null>(null);
|
||||
|
||||
const releaseMutation = useCallback(() => {
|
||||
mutationPhase.current = "idle";
|
||||
setIsMutationPending(false);
|
||||
}, []);
|
||||
|
||||
const cancelProposal = useCallback(() => {
|
||||
if (mutationPhase.current !== "proposal") return;
|
||||
pendingOperation.current = null;
|
||||
setProposal(null);
|
||||
releaseMutation();
|
||||
}, [releaseMutation]);
|
||||
|
||||
const confirmProposal = useCallback(async () => {
|
||||
if (mutationPhase.current !== "proposal") return;
|
||||
const operation = pendingOperation.current;
|
||||
if (!operation) {
|
||||
setProposal(null);
|
||||
releaseMutation();
|
||||
return;
|
||||
}
|
||||
pendingOperation.current = null;
|
||||
mutationPhase.current = "running";
|
||||
setProposal(null);
|
||||
try {
|
||||
await operation();
|
||||
} finally {
|
||||
releaseMutation();
|
||||
}
|
||||
}, [releaseMutation]);
|
||||
|
||||
const guardMutation = useCallback(
|
||||
async ({
|
||||
proposedSteps,
|
||||
operation,
|
||||
intent = "apply",
|
||||
baselineSteps = displayedSteps,
|
||||
}: GuardMutationParams) => {
|
||||
if (mutationPhase.current !== "idle") return;
|
||||
mutationPhase.current = "running";
|
||||
setIsMutationPending(true);
|
||||
|
||||
try {
|
||||
const introduced = analyzeIntroducedWorkflowReplayCycles(baselineSteps, proposedSteps);
|
||||
if (introduced.length === 0) {
|
||||
await operation();
|
||||
releaseMutation();
|
||||
return;
|
||||
}
|
||||
|
||||
const severity = introduced.some((diagnostic) => diagnostic.severity === "blocking")
|
||||
? "blocking"
|
||||
: "warning";
|
||||
pendingOperation.current = severity === "warning" ? operation : null;
|
||||
mutationPhase.current = "proposal";
|
||||
setProposal({ diagnostics: introduced, intent, severity });
|
||||
} catch (error) {
|
||||
releaseMutation();
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
[displayedSteps, releaseMutation],
|
||||
);
|
||||
|
||||
return {
|
||||
diagnostics,
|
||||
proposal,
|
||||
isMutationPending,
|
||||
guardMutation,
|
||||
confirmProposal,
|
||||
cancelProposal,
|
||||
};
|
||||
}
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
IconTrash,
|
||||
IconChevronRight,
|
||||
IconRosetteNumber1,
|
||||
IconAlertTriangle,
|
||||
} from "@tabler/icons-react";
|
||||
import { StepCapabilityIcons } from "@/components/step-capability-icons";
|
||||
import {
|
||||
@@ -37,8 +38,10 @@ import {
|
||||
import { ScrollArea, ScrollBar } from "@kandev/ui/scroll-area";
|
||||
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import type { WorkflowStep } from "@/lib/types/http";
|
||||
import type { WorkflowReplayCycleDiagnostic } from "@/lib/workflows/replay-cycle-analysis";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { StepConfigPanel } from "./workflow-pipeline-editor-panels";
|
||||
import { WorkflowCycleDiagnostic } from "./workflow-cycle-diagnostic";
|
||||
|
||||
type WorkflowPipelineEditorProps = {
|
||||
steps: WorkflowStep[];
|
||||
@@ -46,6 +49,7 @@ type WorkflowPipelineEditorProps = {
|
||||
onAddStep: () => void;
|
||||
onRemoveStep: (stepId: string) => void;
|
||||
onReorderSteps: (steps: WorkflowStep[]) => void;
|
||||
diagnostics?: WorkflowReplayCycleDiagnostic[];
|
||||
readOnly?: boolean;
|
||||
};
|
||||
|
||||
@@ -73,6 +77,7 @@ type PipelineNodeProps = {
|
||||
isSelected: boolean;
|
||||
onSelect: () => void;
|
||||
onRemove: () => void;
|
||||
isReplayCycleAffected: boolean;
|
||||
readOnly?: boolean;
|
||||
};
|
||||
|
||||
@@ -81,6 +86,7 @@ function PipelineNode({
|
||||
isSelected,
|
||||
onSelect,
|
||||
onRemove,
|
||||
isReplayCycleAffected,
|
||||
readOnly = false,
|
||||
}: PipelineNodeProps) {
|
||||
const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({
|
||||
@@ -98,6 +104,7 @@ function PipelineNode({
|
||||
isSelected
|
||||
? "border-primary bg-primary/5"
|
||||
: "border-border bg-card hover:border-primary/50",
|
||||
isReplayCycleAffected && "border-dashed border-amber-500 ring-1 ring-amber-500/30",
|
||||
isDragging && "opacity-50 z-50",
|
||||
)}
|
||||
onClick={onSelect}
|
||||
@@ -137,6 +144,15 @@ function PipelineNode({
|
||||
agentProfileId={step.agent_profile_id}
|
||||
fallback={<span className="text-xs text-muted-foreground/50">manual</span>}
|
||||
/>
|
||||
{isReplayCycleAffected && (
|
||||
<span
|
||||
className="flex items-center gap-1 text-xs font-medium text-amber-700 dark:text-amber-300"
|
||||
aria-label={`${step.name} is part of a replay cycle`}
|
||||
>
|
||||
<IconAlertTriangle className="size-3 shrink-0" aria-hidden="true" />
|
||||
Cycle
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
{!readOnly && (
|
||||
<button
|
||||
@@ -176,6 +192,7 @@ type PipelineAreaProps = {
|
||||
onSelectStep: (stepId: string) => void;
|
||||
onRemoveStep: (stepId: string) => void;
|
||||
onAddStep: () => void;
|
||||
affectedStepIds: Set<string>;
|
||||
readOnly: boolean;
|
||||
};
|
||||
|
||||
@@ -185,6 +202,7 @@ function PipelineArea({
|
||||
onSelectStep,
|
||||
onRemoveStep,
|
||||
onAddStep,
|
||||
affectedStepIds,
|
||||
readOnly,
|
||||
}: PipelineAreaProps) {
|
||||
return (
|
||||
@@ -197,6 +215,7 @@ function PipelineArea({
|
||||
isSelected={selectedStepId === step.id}
|
||||
onSelect={() => onSelectStep(step.id)}
|
||||
onRemove={() => onRemoveStep(step.id)}
|
||||
isReplayCycleAffected={affectedStepIds.has(step.id)}
|
||||
readOnly={readOnly}
|
||||
/>
|
||||
</div>
|
||||
@@ -252,6 +271,23 @@ function StepDeleteConfirmation({
|
||||
);
|
||||
}
|
||||
|
||||
function affectedStepIds(diagnostics: WorkflowReplayCycleDiagnostic[]): Set<string> {
|
||||
return new Set(diagnostics.flatMap((diagnostic) => diagnostic.affectedStepIds));
|
||||
}
|
||||
|
||||
function WorkflowCycleAlerts({ diagnostics }: { diagnostics: WorkflowReplayCycleDiagnostic[] }) {
|
||||
if (diagnostics.length === 0) return null;
|
||||
return (
|
||||
<div className="grid min-w-0 gap-3">
|
||||
{diagnostics.map((diagnostic) => (
|
||||
<WorkflowCycleDiagnostic key={diagnostic.identity} diagnostic={diagnostic} />
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
const EMPTY_DIAGNOSTICS: WorkflowReplayCycleDiagnostic[] = [];
|
||||
|
||||
// --- Main Pipeline Editor ---
|
||||
|
||||
export function WorkflowPipelineEditor({
|
||||
@@ -260,6 +296,7 @@ export function WorkflowPipelineEditor({
|
||||
onAddStep,
|
||||
onRemoveStep,
|
||||
onReorderSteps,
|
||||
diagnostics = EMPTY_DIAGNOSTICS,
|
||||
readOnly = false,
|
||||
}: WorkflowPipelineEditorProps) {
|
||||
const [selectedStepId, setSelectedStepId] = useState<string | null>(null);
|
||||
@@ -273,6 +310,7 @@ export function WorkflowPipelineEditor({
|
||||
}
|
||||
|
||||
const stepItems = useMemo(() => steps.map((step) => step.id), [steps]);
|
||||
const affectedIds = useMemo(() => affectedStepIds(diagnostics), [diagnostics]);
|
||||
const isMounted = useSyncExternalStore(
|
||||
() => () => {},
|
||||
() => true,
|
||||
@@ -317,12 +355,14 @@ export function WorkflowPipelineEditor({
|
||||
onSelectStep={handleSelectStep}
|
||||
onRemoveStep={requestRemoveStep}
|
||||
onAddStep={onAddStep}
|
||||
affectedStepIds={affectedIds}
|
||||
readOnly={readOnly}
|
||||
/>
|
||||
);
|
||||
|
||||
return (
|
||||
<div className="space-y-3">
|
||||
<WorkflowCycleAlerts diagnostics={diagnostics} />
|
||||
<ScrollArea className="w-full pb-1">
|
||||
{isMounted ? (
|
||||
<DndContext
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { beforeEach, expect, it, vi } from "vitest";
|
||||
import {
|
||||
createWorkflowStepAction,
|
||||
deleteWorkflowStepAction,
|
||||
getWorkflowTaskCount,
|
||||
listWorkflowStepsAction,
|
||||
reorderWorkflowStepsAction,
|
||||
updateWorkflowStepAction,
|
||||
} from "@/app/actions/workspaces";
|
||||
import type { Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import {
|
||||
useWorkflowDeleteHandlers,
|
||||
useWorkflowSaveActions,
|
||||
useWorkflowStepActions,
|
||||
} from "./workflow-card-actions";
|
||||
import { useWorkflowMutationGuard } from "./workflow-mutation-guard";
|
||||
|
||||
vi.mock("@/app/actions/workspaces", () => ({
|
||||
createWorkflowAction: vi.fn(),
|
||||
createWorkflowStepAction: vi.fn(),
|
||||
updateWorkflowStepAction: vi.fn(),
|
||||
deleteWorkflowStepAction: vi.fn(),
|
||||
reorderWorkflowStepsAction: vi.fn(),
|
||||
listWorkflowStepsAction: vi.fn(),
|
||||
getStepTaskCount: vi.fn(),
|
||||
getWorkflowTaskCount: vi.fn(),
|
||||
exportWorkflowAction: vi.fn(),
|
||||
bulkMoveTasks: vi.fn(),
|
||||
}));
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(listWorkflowStepsAction).mockResolvedValue({ steps: [], total: 0 });
|
||||
});
|
||||
|
||||
const workflow = {
|
||||
id: "wf-1",
|
||||
workspace_id: "ws-1",
|
||||
name: "Synced workflow",
|
||||
source: "github",
|
||||
source_path: ".kandev/workflows/review.yaml",
|
||||
created_at: "",
|
||||
updated_at: "",
|
||||
} as Workflow;
|
||||
|
||||
function step(id: string, position: number): WorkflowStep {
|
||||
return {
|
||||
id,
|
||||
workflow_id: workflow.id,
|
||||
name: id,
|
||||
position,
|
||||
color: "bg-slate-500",
|
||||
allow_manual_move: true,
|
||||
is_start_step: position === 0,
|
||||
events: { on_enter: [] },
|
||||
created_at: "",
|
||||
updated_at: "",
|
||||
};
|
||||
}
|
||||
|
||||
it("refuses every step mutation when the workflow is read-only", async () => {
|
||||
const initialSteps = [step("todo", 0), step("review", 1)];
|
||||
let currentSteps = initialSteps;
|
||||
const setWorkflowSteps = vi.fn(
|
||||
(updater: ((previous: WorkflowStep[]) => WorkflowStep[]) | WorkflowStep[]) => {
|
||||
currentSteps = typeof updater === "function" ? updater(currentSteps) : updater;
|
||||
},
|
||||
);
|
||||
const { result } = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(initialSteps);
|
||||
return useWorkflowStepActions({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
readOnly: true,
|
||||
workflowSteps: initialSteps,
|
||||
setWorkflowSteps,
|
||||
setStepToDelete: vi.fn(),
|
||||
setStepTaskCount: vi.fn(),
|
||||
setTargetStepForMigration: vi.fn(),
|
||||
setStepDeleteOpen: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleAddWorkflowStep();
|
||||
await result.current.handleUpdateWorkflowStep("review", { name: "Renamed" });
|
||||
await result.current.handleRemoveWorkflowStep("todo");
|
||||
await result.current.handleReorderWorkflowSteps([...initialSteps].reverse());
|
||||
});
|
||||
|
||||
expect(currentSteps).toEqual(initialSteps);
|
||||
expect(setWorkflowSteps).not.toHaveBeenCalled();
|
||||
expect(createWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(updateWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(deleteWorkflowStepAction).not.toHaveBeenCalled();
|
||||
expect(reorderWorkflowStepsAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("refuses to open or confirm workflow deletion when read-only", async () => {
|
||||
const deleteWorkflowRun = vi.fn();
|
||||
const wfDel = {
|
||||
setDeleteOpen: vi.fn(),
|
||||
setWorkflowTaskCount: vi.fn(),
|
||||
setWorkflowDeleteLoading: vi.fn(),
|
||||
setTargetWorkflowId: vi.fn(),
|
||||
setTargetWorkflowSteps: vi.fn(),
|
||||
setTargetStepId: vi.fn(),
|
||||
targetWorkflowId: "target-workflow",
|
||||
targetStepId: "target-step",
|
||||
setMigrateLoading: vi.fn(),
|
||||
};
|
||||
const { result } = renderHook(() =>
|
||||
useWorkflowDeleteHandlers({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
readOnly: true,
|
||||
otherWorkflows: [],
|
||||
wfDel,
|
||||
deleteWorkflowRun,
|
||||
toast: vi.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleDeleteWorkflowClick();
|
||||
await result.current.handleDeleteWorkflow();
|
||||
await result.current.handleMigrateAndDeleteWorkflow();
|
||||
});
|
||||
|
||||
expect(getWorkflowTaskCount).not.toHaveBeenCalled();
|
||||
expect(deleteWorkflowRun).not.toHaveBeenCalled();
|
||||
expect(wfDel.setDeleteOpen).not.toHaveBeenCalled();
|
||||
expect(wfDel.setMigrateLoading).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("refuses to save an existing workflow when read-only", async () => {
|
||||
const onSaveWorkflow = vi.fn();
|
||||
const { result } = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard([]);
|
||||
return useWorkflowSaveActions({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
readOnly: true,
|
||||
workflowSteps: [],
|
||||
templateStepCount: 0,
|
||||
onSaveWorkflow,
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleSaveWorkflow();
|
||||
});
|
||||
|
||||
expect(onSaveWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -0,0 +1,128 @@
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { beforeEach, expect, it, vi } from "vitest";
|
||||
import { reorderWorkflowStepsAction } from "@/app/actions/workspaces";
|
||||
import type { Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import { useWorkflowStepActions } from "./workflow-card-actions";
|
||||
import { useWorkflowMutationGuard } from "./workflow-mutation-guard";
|
||||
|
||||
vi.mock("@/app/actions/workspaces", () => ({
|
||||
createWorkflowAction: vi.fn(),
|
||||
createWorkflowStepAction: vi.fn(),
|
||||
updateWorkflowStepAction: vi.fn(),
|
||||
deleteWorkflowStepAction: vi.fn(),
|
||||
reorderWorkflowStepsAction: vi.fn(),
|
||||
listWorkflowStepsAction: vi.fn(),
|
||||
getStepTaskCount: vi.fn(),
|
||||
getWorkflowTaskCount: vi.fn(),
|
||||
exportWorkflowAction: vi.fn(),
|
||||
bulkMoveTasks: vi.fn(),
|
||||
}));
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
const workflow = {
|
||||
id: "wf-1",
|
||||
workspace_id: "ws-1",
|
||||
name: "Workflow",
|
||||
created_at: "",
|
||||
updated_at: "",
|
||||
} as Workflow;
|
||||
|
||||
function step(id: string, name: string, position: number, isStartStep: boolean): WorkflowStep {
|
||||
return {
|
||||
id,
|
||||
workflow_id: workflow.id,
|
||||
name,
|
||||
position,
|
||||
color: "bg-slate-500",
|
||||
allow_manual_move: true,
|
||||
is_start_step: isStartStep,
|
||||
events: { on_enter: [] },
|
||||
created_at: "",
|
||||
updated_at: "",
|
||||
};
|
||||
}
|
||||
|
||||
function renderReorderActions(initialSteps: WorkflowStep[]) {
|
||||
let currentSteps = initialSteps;
|
||||
const setWorkflowSteps = vi.fn(
|
||||
(updater: ((previous: WorkflowStep[]) => WorkflowStep[]) | WorkflowStep[]) => {
|
||||
currentSteps = typeof updater === "function" ? updater(currentSteps) : updater;
|
||||
},
|
||||
);
|
||||
const view = renderHook(() => {
|
||||
const mutationGuard = useWorkflowMutationGuard(initialSteps);
|
||||
return useWorkflowStepActions({
|
||||
workflow,
|
||||
isNewWorkflow: false,
|
||||
workflowSteps: initialSteps,
|
||||
setWorkflowSteps,
|
||||
setStepToDelete: vi.fn(),
|
||||
setStepTaskCount: vi.fn(),
|
||||
setTargetStepForMigration: vi.fn(),
|
||||
setStepDeleteOpen: vi.fn(),
|
||||
toast: vi.fn(),
|
||||
mutationGuard,
|
||||
});
|
||||
});
|
||||
return { ...view, setWorkflowSteps, getSteps: () => currentSteps };
|
||||
}
|
||||
|
||||
function deferredPromise<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
const promise = new Promise<T>((resolvePromise) => {
|
||||
resolve = resolvePromise;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
it("keeps a reorder optimistic until the authoritative response arrives", async () => {
|
||||
const work = step("work", "Work", 0, true);
|
||||
const review = step("review", "Review", 1, false);
|
||||
const proposed = [
|
||||
{ ...review, position: 0 },
|
||||
{ ...work, position: 1 },
|
||||
];
|
||||
const authoritative = proposed.map((workflowStep) => ({
|
||||
...workflowStep,
|
||||
updated_at: "server-update",
|
||||
}));
|
||||
const response = deferredPromise<{ steps: WorkflowStep[]; total: number }>();
|
||||
vi.mocked(reorderWorkflowStepsAction).mockReturnValue(response.promise);
|
||||
const { result, getSteps } = renderReorderActions([work, review]);
|
||||
|
||||
let request!: Promise<void>;
|
||||
await act(async () => {
|
||||
request = result.current.handleReorderWorkflowSteps([review, work]);
|
||||
await vi.waitFor(() => expect(reorderWorkflowStepsAction).toHaveBeenCalledTimes(1));
|
||||
});
|
||||
expect(getSteps()).toEqual(proposed);
|
||||
|
||||
await act(async () => {
|
||||
response.resolve({ steps: authoritative, total: 2 });
|
||||
await request;
|
||||
});
|
||||
expect(getSteps()).toEqual(authoritative);
|
||||
});
|
||||
|
||||
it("rolls an optimistic reorder back when persistence fails", async () => {
|
||||
const work = step("work", "Work", 0, true);
|
||||
const review = step("review", "Review", 1, false);
|
||||
const initial = [work, review];
|
||||
const proposed = [
|
||||
{ ...review, position: 0 },
|
||||
{ ...work, position: 1 },
|
||||
];
|
||||
vi.mocked(reorderWorkflowStepsAction).mockRejectedValue(new Error("reorder failed"));
|
||||
const { result, setWorkflowSteps, getSteps } = renderReorderActions(initial);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleReorderWorkflowSteps([review, work]);
|
||||
});
|
||||
|
||||
expect(setWorkflowSteps).toHaveBeenNthCalledWith(1, proposed);
|
||||
expect(setWorkflowSteps).toHaveBeenNthCalledWith(2, initial);
|
||||
expect(getSteps()).toEqual(initial);
|
||||
});
|
||||
@@ -0,0 +1,95 @@
|
||||
import type { Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import type { useToast } from "@/components/toast-provider";
|
||||
import { generateUUID } from "@/lib/utils";
|
||||
import { createWorkflowStepAction, updateWorkflowStepAction } from "@/app/actions/workspaces";
|
||||
|
||||
const FALLBACK_ERROR_MESSAGE = "Request failed";
|
||||
const NEW_STEP_DEFAULTS = { name: "New Step", color: "bg-slate-500" } as const;
|
||||
|
||||
type WorkflowStepsSetter = (
|
||||
updater: ((previous: WorkflowStep[]) => WorkflowStep[]) | WorkflowStep[],
|
||||
) => void;
|
||||
type Toast = ReturnType<typeof useToast>["toast"];
|
||||
|
||||
export function newWorkflowStep(workflow: Workflow, position: number, id: string): WorkflowStep {
|
||||
return {
|
||||
id,
|
||||
workflow_id: workflow.id,
|
||||
...NEW_STEP_DEFAULTS,
|
||||
position,
|
||||
allow_manual_move: true,
|
||||
created_at: "",
|
||||
updated_at: "",
|
||||
};
|
||||
}
|
||||
|
||||
export function addLocalStep(workflow: Workflow, setWorkflowSteps: WorkflowStepsSetter) {
|
||||
setWorkflowSteps((previous) => [
|
||||
...previous,
|
||||
newWorkflowStep(workflow, previous.length, `temp-step-${generateUUID()}`),
|
||||
]);
|
||||
}
|
||||
|
||||
export function removeLocalStep(stepId: string, setWorkflowSteps: WorkflowStepsSetter) {
|
||||
setWorkflowSteps((previous) =>
|
||||
previous.filter((step) => step.id !== stepId).map((step, position) => ({ ...step, position })),
|
||||
);
|
||||
}
|
||||
|
||||
export async function addRemoteStep(
|
||||
workflow: Workflow,
|
||||
stepCount: number,
|
||||
setWorkflowSteps: WorkflowStepsSetter,
|
||||
toast: Toast,
|
||||
) {
|
||||
try {
|
||||
const created = await createWorkflowStepAction({
|
||||
workflow_id: workflow.id,
|
||||
...NEW_STEP_DEFAULTS,
|
||||
position: stepCount,
|
||||
});
|
||||
setWorkflowSteps((previous) => [...previous, created]);
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Failed to add workflow step",
|
||||
description: error instanceof Error ? error.message : FALLBACK_ERROR_MESSAGE,
|
||||
variant: "error",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
export function applyWorkflowStepUpdates(
|
||||
steps: WorkflowStep[],
|
||||
stepId: string,
|
||||
updates: Partial<WorkflowStep>,
|
||||
): WorkflowStep[] {
|
||||
const isSettingStartStep = updates.is_start_step === true;
|
||||
return steps.map((step) => {
|
||||
if (step.id === stepId) return { ...step, ...updates };
|
||||
if (isSettingStartStep) return { ...step, is_start_step: false };
|
||||
return step;
|
||||
});
|
||||
}
|
||||
|
||||
export async function updateRemoteWorkflowStep({
|
||||
stepId,
|
||||
updates,
|
||||
setWorkflowSteps,
|
||||
toast,
|
||||
}: {
|
||||
stepId: string;
|
||||
updates: Partial<WorkflowStep>;
|
||||
setWorkflowSteps: WorkflowStepsSetter;
|
||||
toast: Toast;
|
||||
}) {
|
||||
try {
|
||||
const updated = await updateWorkflowStepAction(stepId, updates);
|
||||
setWorkflowSteps((previous) => applyWorkflowStepUpdates(previous, stepId, updated));
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Failed to update workflow step",
|
||||
description: error instanceof Error ? error.message : FALLBACK_ERROR_MESSAGE,
|
||||
variant: "error",
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ export class WorkflowSettingsPage {
|
||||
readonly createDialog: Locator;
|
||||
readonly workflowNameInput: Locator;
|
||||
readonly confirmCreateButton: Locator;
|
||||
readonly cycleGuardDialog: Locator;
|
||||
|
||||
constructor(page: Page) {
|
||||
this.page = page;
|
||||
@@ -13,6 +14,7 @@ export class WorkflowSettingsPage {
|
||||
this.createDialog = page.getByTestId("create-workflow-dialog");
|
||||
this.workflowNameInput = page.getByTestId("workflow-name-input");
|
||||
this.confirmCreateButton = page.getByTestId("confirm-create-workflow");
|
||||
this.cycleGuardDialog = page.getByTestId("workflow-cycle-guard-dialog");
|
||||
}
|
||||
|
||||
async goto(workspaceId: string) {
|
||||
@@ -62,6 +64,52 @@ export class WorkflowSettingsPage {
|
||||
return card.locator(".group.relative").filter({ hasText: stepName });
|
||||
}
|
||||
|
||||
/** A replay-cycle diagnostic rendered inside a workflow card or guard dialog. */
|
||||
cycleDiagnostic(container: Locator, autoStartStepId: string): Locator {
|
||||
return container.getByTestId(`workflow-cycle-diagnostic-${autoStartStepId}`);
|
||||
}
|
||||
|
||||
/** Select a step and return its configuration panel. */
|
||||
async selectStep(card: Locator, stepName: string, touch = false): Promise<Locator> {
|
||||
const currentName = card.getByPlaceholder("Step name");
|
||||
const alreadySelected =
|
||||
(await currentName.isVisible().catch(() => false)) &&
|
||||
(await currentName.inputValue().catch(() => "")) === stepName;
|
||||
if (!alreadySelected) {
|
||||
await this.activate(this.stepNodeByName(card, stepName), touch);
|
||||
}
|
||||
await expect(currentName).toHaveValue(stepName);
|
||||
return currentName.locator(
|
||||
"xpath=ancestor::div[contains(concat(' ', normalize-space(@class), ' '), ' rounded-lg ')][1]",
|
||||
);
|
||||
}
|
||||
|
||||
/** Toggle auto-start for a step through the visible configuration panel. */
|
||||
async setAutoStart(card: Locator, stepName: string, enabled: boolean, touch = false) {
|
||||
const panel = await this.selectStep(card, stepName, touch);
|
||||
const checkbox = panel.getByRole("checkbox", { name: "Auto-start agent" });
|
||||
if ((await checkbox.isChecked()) !== enabled) {
|
||||
await this.activate(checkbox, touch);
|
||||
}
|
||||
if (enabled) await expect(checkbox).toBeChecked();
|
||||
else await expect(checkbox).not.toBeChecked();
|
||||
}
|
||||
|
||||
/** Set the On Turn Complete transition in a step's configuration panel. */
|
||||
async setTurnCompleteTransition(
|
||||
card: Locator,
|
||||
stepName: string,
|
||||
optionName: string,
|
||||
touch = false,
|
||||
) {
|
||||
const panel = await this.selectStep(card, stepName, touch);
|
||||
const transitionSection = panel
|
||||
.getByText("On Turn Complete", { exact: true })
|
||||
.locator("xpath=../..");
|
||||
await this.activate(transitionSection.getByRole("combobox"), touch);
|
||||
await this.activate(this.page.getByRole("option", { name: optionName }), touch);
|
||||
}
|
||||
|
||||
/** The add-step (+) button within a workflow card. */
|
||||
addStepButton(card: Locator): Locator {
|
||||
return card.getByTestId("add-step-button");
|
||||
@@ -100,21 +148,25 @@ export class WorkflowSettingsPage {
|
||||
}
|
||||
|
||||
/** Open the "Add Workflow" dialog and create a workflow. */
|
||||
async createWorkflow(name: string, templateName?: string) {
|
||||
await this.addWorkflowButton.click();
|
||||
async createWorkflow(name: string, templateName?: string, touch = false) {
|
||||
await this.activate(this.addWorkflowButton, touch);
|
||||
await expect(this.createDialog).toBeVisible();
|
||||
|
||||
if (name) {
|
||||
if (touch) await this.workflowNameInput.tap();
|
||||
await this.workflowNameInput.fill(name);
|
||||
}
|
||||
|
||||
if (templateName === "Custom") {
|
||||
await this.createDialog.locator('label[for="custom"]').click();
|
||||
await this.activate(this.createDialog.locator('label[for="custom"]'), touch);
|
||||
} else if (templateName) {
|
||||
await this.createDialog.getByText(templateName, { exact: false }).first().click();
|
||||
await this.activate(
|
||||
this.createDialog.getByRole("radio", { name: templateName, exact: false }),
|
||||
touch,
|
||||
);
|
||||
}
|
||||
|
||||
await this.confirmCreateButton.click();
|
||||
await this.activate(this.confirmCreateButton, touch);
|
||||
await expect(this.createDialog).not.toBeVisible();
|
||||
}
|
||||
|
||||
@@ -137,4 +189,9 @@ export class WorkflowSettingsPage {
|
||||
.filter({ has: this.page.locator(".tabler-icon-trash") })
|
||||
.click();
|
||||
}
|
||||
|
||||
private async activate(locator: Locator, touch: boolean) {
|
||||
if (touch) await locator.tap();
|
||||
else await locator.click();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { WorkflowSettingsPage } from "../../pages/workflow-settings-page";
|
||||
|
||||
test.describe("Workflow cycle guardrails on mobile", () => {
|
||||
test("keeps a blocking cycle readable above the dialog action", async ({
|
||||
testPage,
|
||||
seedData,
|
||||
}) => {
|
||||
const settings = new WorkflowSettingsPage(testPage);
|
||||
await settings.goto(seedData.workspaceId);
|
||||
await settings.createWorkflow("Mobile blocked draft", "Custom", true);
|
||||
const card = await settings.findWorkflowCard("Mobile blocked draft");
|
||||
|
||||
await settings.setAutoStart(card, "Todo", true, true);
|
||||
await settings.setTurnCompleteTransition(card, "Todo", "Move to next step", true);
|
||||
await settings.setAutoStart(card, "In Progress", true, true);
|
||||
await settings.setTurnCompleteTransition(card, "In Progress", "Move to previous step", true);
|
||||
await settings.saveButton(card).tap();
|
||||
|
||||
const dialog = settings.cycleGuardDialog;
|
||||
await expect(dialog.getByRole("heading", { name: "Workflow cycle blocked" })).toBeVisible();
|
||||
await expect(dialog.getByText("Automatic workflow cycle")).toHaveCount(2);
|
||||
|
||||
const transitionLabels = dialog.getByText(/^(On turn complete|Move to (next|previous) step)$/);
|
||||
await expect(transitionLabels).toHaveCount(8);
|
||||
const labelSizes = await transitionLabels.evaluateAll((labels) =>
|
||||
labels.map((label) => {
|
||||
const box = label.getBoundingClientRect();
|
||||
return { width: box.width, height: box.height };
|
||||
}),
|
||||
);
|
||||
for (const label of labelSizes) expect(label.width).toBeGreaterThan(label.height);
|
||||
|
||||
const finalExplanation = dialog
|
||||
.getByText('"In Progress" has no step prompt, so re-entering it sends the task description.')
|
||||
.last();
|
||||
const returnButton = dialog.getByRole("button", { name: "Return to workflow" });
|
||||
await finalExplanation.scrollIntoViewIfNeeded();
|
||||
const [explanationBox, buttonBox] = await Promise.all([
|
||||
finalExplanation.boundingBox(),
|
||||
returnButton.boundingBox(),
|
||||
]);
|
||||
expect(explanationBox).not.toBeNull();
|
||||
expect(buttonBox).not.toBeNull();
|
||||
expect(explanationBox!.y + explanationBox!.height).toBeLessThanOrEqual(buttonBox!.y);
|
||||
|
||||
await returnButton.tap();
|
||||
await expect(dialog).not.toBeVisible();
|
||||
});
|
||||
|
||||
test("reviews and confirms a repeated agent run by touch without horizontal overflow", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const workflowName = "Mobile warning draft";
|
||||
const settings = new WorkflowSettingsPage(testPage);
|
||||
await settings.goto(seedData.workspaceId);
|
||||
await settings.createWorkflow(workflowName, "Custom", true);
|
||||
const card = await settings.findWorkflowCard(workflowName);
|
||||
|
||||
await settings.setAutoStart(card, "Todo", true, true);
|
||||
await settings.setTurnCompleteTransition(card, "Todo", "Move to next step", true);
|
||||
await settings.setTurnCompleteTransition(card, "In Progress", "Move to previous step", true);
|
||||
await settings.saveButton(card).tap();
|
||||
|
||||
const dialog = settings.cycleGuardDialog;
|
||||
await expect(dialog.getByRole("heading", { name: "Confirm workflow cycle" })).toBeVisible();
|
||||
await expect(dialog.getByText("Potential repeated agent run")).toBeVisible();
|
||||
await expect(
|
||||
dialog.getByRole("list", { name: "Replay path for Todo" }).getByRole("listitem"),
|
||||
).toHaveCount(2);
|
||||
await expect(
|
||||
dialog.getByText('"Todo" has no step prompt, so re-entering it sends the task description.'),
|
||||
).toBeVisible();
|
||||
|
||||
const actionSizes = await dialog.getByRole("button").evaluateAll((buttons) =>
|
||||
buttons.map((button) => ({
|
||||
name: button.textContent?.trim(),
|
||||
height: button.getBoundingClientRect().height,
|
||||
})),
|
||||
);
|
||||
expect(
|
||||
actionSizes.filter((action) => ["Cancel", "Create anyway"].includes(action.name ?? "")),
|
||||
).toEqual([
|
||||
expect.objectContaining({ name: "Cancel", height: expect.any(Number) }),
|
||||
expect.objectContaining({ name: "Create anyway", height: expect.any(Number) }),
|
||||
]);
|
||||
for (const action of actionSizes.filter((item) =>
|
||||
["Cancel", "Create anyway"].includes(item.name ?? ""),
|
||||
)) {
|
||||
expect(action.height).toBeGreaterThanOrEqual(44);
|
||||
}
|
||||
|
||||
const overflow = await testPage.evaluate(() => {
|
||||
const guard = document.querySelector<HTMLElement>(
|
||||
'[data-testid="workflow-cycle-guard-dialog"]',
|
||||
);
|
||||
return {
|
||||
document: document.documentElement.scrollWidth - document.documentElement.clientWidth,
|
||||
dialog: guard ? guard.scrollWidth - guard.clientWidth : Number.POSITIVE_INFINITY,
|
||||
};
|
||||
});
|
||||
expect(overflow.document).toBeLessThanOrEqual(1);
|
||||
expect(overflow.dialog).toBeLessThanOrEqual(1);
|
||||
|
||||
await dialog.getByRole("button", { name: "Create anyway" }).tap();
|
||||
await expect(dialog).not.toBeVisible();
|
||||
await expect
|
||||
.poll(async () => (await apiClient.listWorkflows(seedData.workspaceId)).workflows)
|
||||
.toContainEqual(expect.objectContaining({ name: workflowName }));
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,124 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { WorkflowSettingsPage } from "../../pages/workflow-settings-page";
|
||||
|
||||
const WARNING_WORKFLOW_NAME = "Guardrail warning workflow";
|
||||
|
||||
test.describe("Workflow cycle guardrails", () => {
|
||||
test("warns before persisting a repeated agent run and keeps its diagnostic after reload", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const workflow = await apiClient.createWorkflow(seedData.workspaceId, WARNING_WORKFLOW_NAME);
|
||||
const work = await apiClient.createWorkflowStep(workflow.id, "Work", 0, {
|
||||
is_start_step: true,
|
||||
});
|
||||
const review = await apiClient.createWorkflowStep(workflow.id, "Review", 1);
|
||||
await apiClient.updateWorkflowStep(work.id, {
|
||||
events: {
|
||||
on_enter: [{ type: "auto_start_agent" }],
|
||||
on_turn_complete: [{ type: "move_to_next" }],
|
||||
},
|
||||
});
|
||||
|
||||
const settings = new WorkflowSettingsPage(testPage);
|
||||
await settings.goto(seedData.workspaceId);
|
||||
const card = await settings.findWorkflowCard(WARNING_WORKFLOW_NAME);
|
||||
await settings.setTurnCompleteTransition(card, "Review", "Move to previous step");
|
||||
|
||||
const dialog = settings.cycleGuardDialog;
|
||||
await expect(dialog).toBeVisible();
|
||||
await expect(dialog.getByRole("heading", { name: "Confirm workflow cycle" })).toBeVisible();
|
||||
const diagnostic = settings.cycleDiagnostic(dialog, work.id);
|
||||
await expect(diagnostic.getByText("Potential repeated agent run")).toBeVisible();
|
||||
const trace = diagnostic
|
||||
.getByRole("list", { name: "Replay path for Work" })
|
||||
.getByRole("listitem");
|
||||
await expect(trace).toHaveCount(2);
|
||||
await expect(trace.nth(0)).toContainText(
|
||||
/Work\s*Review\s*On turn complete\s*Move to next step/,
|
||||
);
|
||||
await expect(trace.nth(1)).toContainText(
|
||||
/Review\s*Work\s*On turn complete\s*Move to previous step/,
|
||||
);
|
||||
await expect(trace.nth(1).getByText("User action required")).toBeVisible();
|
||||
await expect(
|
||||
diagnostic.getByText(
|
||||
'"Work" has no step prompt, so re-entering it sends the task description.',
|
||||
),
|
||||
).toBeVisible();
|
||||
|
||||
await dialog.getByRole("button", { name: "Cancel" }).click();
|
||||
await expect(dialog).not.toBeVisible();
|
||||
let persistedSteps = await apiClient.listWorkflowSteps(workflow.id);
|
||||
expect(
|
||||
persistedSteps.steps.find((step) => step.id === review.id)?.events?.on_turn_complete,
|
||||
).toBeFalsy();
|
||||
|
||||
await settings.setTurnCompleteTransition(card, "Review", "Move to previous step");
|
||||
await dialog.getByRole("button", { name: "Apply anyway" }).click();
|
||||
await expect(dialog).not.toBeVisible();
|
||||
await expect
|
||||
.poll(async () => {
|
||||
persistedSteps = await apiClient.listWorkflowSteps(workflow.id);
|
||||
return persistedSteps.steps.find((step) => step.id === review.id)?.events?.on_turn_complete;
|
||||
})
|
||||
.toEqual([{ type: "move_to_previous" }]);
|
||||
|
||||
await expect(settings.cycleDiagnostic(card, work.id)).toBeVisible();
|
||||
await expect(card.getByLabel("Work is part of a replay cycle")).toBeVisible();
|
||||
await expect(card.getByLabel("Review is part of a replay cycle")).toBeVisible();
|
||||
|
||||
await settings.goto(seedData.workspaceId);
|
||||
const reloadedCard = await settings.findWorkflowCard(WARNING_WORKFLOW_NAME);
|
||||
await expect(settings.cycleDiagnostic(reloadedCard, work.id)).toBeVisible();
|
||||
});
|
||||
|
||||
test("blocks creation of a draft with a fully automatic cycle", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const settings = new WorkflowSettingsPage(testPage);
|
||||
await settings.goto(seedData.workspaceId);
|
||||
await settings.createWorkflow("Blocked automatic draft", "Custom");
|
||||
const card = await settings.findWorkflowCard("Blocked automatic draft");
|
||||
|
||||
await settings.setAutoStart(card, "Todo", true);
|
||||
await settings.setTurnCompleteTransition(card, "Todo", "Move to next step");
|
||||
await settings.setAutoStart(card, "In Progress", true);
|
||||
await settings.setTurnCompleteTransition(card, "In Progress", "Move to previous step");
|
||||
await settings.saveButton(card).click();
|
||||
|
||||
const dialog = settings.cycleGuardDialog;
|
||||
await expect(dialog.getByRole("heading", { name: "Workflow cycle blocked" })).toBeVisible();
|
||||
await expect(dialog.getByText("Automatic workflow cycle")).toHaveCount(2);
|
||||
await expect(dialog.getByRole("button", { name: /anyway/i })).toHaveCount(0);
|
||||
await expect(dialog.getByRole("button", { name: "Return to workflow" })).toBeVisible();
|
||||
await expect
|
||||
.poll(async () => (await apiClient.listWorkflows(seedData.workspaceId)).workflows)
|
||||
.not.toContainEqual(expect.objectContaining({ name: "Blocked automatic draft" }));
|
||||
});
|
||||
|
||||
test("allows a generic cycle that cannot replay an auto-start step", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const workflowName = "Allowed generic cycle";
|
||||
const settings = new WorkflowSettingsPage(testPage);
|
||||
await settings.goto(seedData.workspaceId);
|
||||
await settings.createWorkflow(workflowName, "Custom");
|
||||
const card = await settings.findWorkflowCard(workflowName);
|
||||
|
||||
await settings.setTurnCompleteTransition(card, "Todo", "Move to next step");
|
||||
await settings.setTurnCompleteTransition(card, "In Progress", "Move to previous step");
|
||||
await expect(card.locator('[data-testid^="workflow-cycle-diagnostic-"]')).toHaveCount(0);
|
||||
await settings.saveButton(card).click();
|
||||
|
||||
await expect(settings.cycleGuardDialog).not.toBeVisible();
|
||||
await expect
|
||||
.poll(async () => (await apiClient.listWorkflows(seedData.workspaceId)).workflows)
|
||||
.toContainEqual(expect.objectContaining({ name: workflowName }));
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,554 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { workflowId } from "@/lib/types/ids";
|
||||
import type { WorkflowStep } from "@/lib/types/http";
|
||||
import type {
|
||||
OnTurnCompleteAction,
|
||||
OnTurnStartAction,
|
||||
TransitionConfig,
|
||||
} from "@/lib/types/workflow-actions";
|
||||
import {
|
||||
analyzeIntroducedWorkflowReplayCycles,
|
||||
analyzeWorkflowReplayCycles,
|
||||
} from "./replay-cycle-analysis";
|
||||
|
||||
const NOW = "2026-07-15T00:00:00.000Z";
|
||||
const SHORT_RETURN_STEP_ID = "short-return";
|
||||
const WORK_STEP_ID = "work";
|
||||
const EXISTING_STEP_ID = "a-existing";
|
||||
const ALTERNATE_STEP_ID = "z-existing";
|
||||
|
||||
function step(
|
||||
id: string,
|
||||
position: number,
|
||||
options: {
|
||||
autoStart?: boolean;
|
||||
name?: string;
|
||||
prompt?: string;
|
||||
onTurnStart?: OnTurnStartAction[];
|
||||
onTurnComplete?: OnTurnCompleteAction[];
|
||||
} = {},
|
||||
): WorkflowStep {
|
||||
return {
|
||||
id,
|
||||
workflow_id: workflowId("workflow-1"),
|
||||
name: options.name ?? id,
|
||||
position,
|
||||
color: "bg-neutral-400",
|
||||
prompt: options.prompt,
|
||||
events: {
|
||||
on_enter: options.autoStart ? [{ type: "auto_start_agent" }] : [],
|
||||
on_turn_start: options.onTurnStart,
|
||||
on_turn_complete: options.onTurnComplete,
|
||||
},
|
||||
created_at: NOW,
|
||||
updated_at: NOW,
|
||||
};
|
||||
}
|
||||
|
||||
function moveToStep(target: string, config: TransitionConfig = {}): OnTurnCompleteAction {
|
||||
return { type: "move_to_step", config: { ...config, step_id: target } };
|
||||
}
|
||||
|
||||
function guardedMoveToStep(target: string): OnTurnCompleteAction {
|
||||
return moveToStep(target, {
|
||||
if: { wait_for_quorum: { role: "reviewer", threshold: "all_approve" } },
|
||||
});
|
||||
}
|
||||
|
||||
describe("replay cycle detection", () => {
|
||||
it("reports a fully automatic next/previous replay cycle as blocking", () => {
|
||||
const diagnostics = analyzeWorkflowReplayCycles([
|
||||
step("build", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostics).toHaveLength(2);
|
||||
expect(diagnostics[0]).toEqual({
|
||||
identity: expect.any(String),
|
||||
severity: "blocking",
|
||||
autoStartStepId: "build",
|
||||
autoStartStepName: "build",
|
||||
affectedStepIds: ["build", "review"],
|
||||
trace: [
|
||||
{
|
||||
sourceStepId: "build",
|
||||
sourceStepName: "build",
|
||||
trigger: "on_turn_complete",
|
||||
actionKind: "move_to_next",
|
||||
destinationStepId: "review",
|
||||
destinationStepName: "review",
|
||||
requiresUserInvolvement: false,
|
||||
},
|
||||
{
|
||||
sourceStepId: "review",
|
||||
sourceStepName: "review",
|
||||
trigger: "on_turn_complete",
|
||||
actionKind: "move_to_previous",
|
||||
destinationStepId: "build",
|
||||
destinationStepName: "build",
|
||||
requiresUserInvolvement: false,
|
||||
},
|
||||
],
|
||||
promptSource: "task_description",
|
||||
});
|
||||
});
|
||||
|
||||
it("treats on_turn_start from an auto-start step as automatic", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnStart: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("blocking");
|
||||
expect(diagnostic.trace.map((hop) => hop.requiresUserInvolvement)).toEqual([false, false]);
|
||||
});
|
||||
|
||||
it("marks on_turn_start from a non-auto-start step as user-mediated", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", 1, { onTurnStart: [{ type: "move_to_next" }] }),
|
||||
step("finish", 2, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("warning");
|
||||
expect(diagnostic.trace.map((hop) => hop.requiresUserInvolvement)).toEqual([
|
||||
false,
|
||||
true,
|
||||
false,
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not report the built-in Kanban return because on_turn_start skips on_enter", () => {
|
||||
const diagnostics = analyzeWorkflowReplayCycles([
|
||||
step("backlog", 0, {
|
||||
onTurnStart: [{ type: "move_to_next" }],
|
||||
onTurnComplete: [moveToStep("review")],
|
||||
}),
|
||||
step("in-progress", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep("review")],
|
||||
}),
|
||||
step("review", 2, { onTurnStart: [{ type: "move_to_previous" }] }),
|
||||
step("done", 3, {
|
||||
onTurnStart: [{ type: "move_to_step", config: { step_id: "in-progress" } }],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostics).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("replay cycle severity", () => {
|
||||
it("marks an on_turn_complete hop from a non-auto-start source as user-mediated", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", 1, { onTurnComplete: [{ type: "move_to_previous" }] }),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("warning");
|
||||
expect(diagnostic.trace.map((hop) => hop.requiresUserInvolvement)).toEqual([false, true]);
|
||||
});
|
||||
|
||||
it("marks an approval-gated hop as user-mediated", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next", config: { requires_approval: true } }],
|
||||
}),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("warning");
|
||||
expect(diagnostic.trace.map((hop) => hop.requiresUserInvolvement)).toEqual([true, false]);
|
||||
});
|
||||
|
||||
it("treats guarded transitions as possible automatic paths", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [
|
||||
{
|
||||
type: "move_to_next",
|
||||
config: { if: { wait_for_quorum: { role: "reviewer", threshold: "all_approve" } } },
|
||||
},
|
||||
],
|
||||
}),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("blocking");
|
||||
});
|
||||
});
|
||||
|
||||
describe("replay transition resolution", () => {
|
||||
it("ignores lower-priority moves after an unconditional transition", () => {
|
||||
const diagnostics = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }, moveToStep("work")],
|
||||
}),
|
||||
step("done", 1),
|
||||
]);
|
||||
|
||||
expect(diagnostics).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps fallback moves after a guarded transition", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [
|
||||
{
|
||||
type: "move_to_next",
|
||||
config: { if: { wait_for_quorum: { role: "reviewer", threshold: "all_approve" } } },
|
||||
},
|
||||
moveToStep("work"),
|
||||
],
|
||||
}),
|
||||
step("done", 1),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("blocking");
|
||||
expect(diagnostic.trace).toHaveLength(1);
|
||||
expect(diagnostic.trace[0].actionKind).toBe("move_to_step");
|
||||
});
|
||||
|
||||
it("resolves explicit targets and ignores dangling targets", () => {
|
||||
const valid = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, { autoStart: true, onTurnComplete: [moveToStep("review")] }),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
const dangling = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, { autoStart: true, onTurnComplete: [moveToStep("missing")] }),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(valid).toHaveLength(2);
|
||||
expect(dangling).toEqual([]);
|
||||
});
|
||||
|
||||
it("ignores malformed explicit targets without throwing", () => {
|
||||
const malformed = { type: "move_to_step" } as OnTurnCompleteAction;
|
||||
|
||||
expect(
|
||||
analyzeWorkflowReplayCycles([
|
||||
step("work", 0, { autoStart: true, onTurnComplete: [malformed] }),
|
||||
]),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("uses position order for relative transitions after a reorder", () => {
|
||||
const before = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
step("done", 2),
|
||||
]);
|
||||
const after = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_next" }],
|
||||
}),
|
||||
step("review", 2, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [{ type: "move_to_previous" }],
|
||||
}),
|
||||
step("done", 1),
|
||||
]);
|
||||
|
||||
expect(before.map((diagnostic) => diagnostic.autoStartStepId)).toEqual(["work", "review"]);
|
||||
expect(after).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not report cycles that never re-enter an auto-start step", () => {
|
||||
const diagnostics = analyzeWorkflowReplayCycles([
|
||||
step("backlog", 0, { onTurnComplete: [{ type: "move_to_next" }] }),
|
||||
step("review", 1, { onTurnComplete: [{ type: "move_to_previous" }] }),
|
||||
]);
|
||||
|
||||
expect(diagnostics).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("replay cycle search bounds", () => {
|
||||
it("selects deterministically without enumerating every path in a dense branching graph", () => {
|
||||
const layerCount = 16;
|
||||
const branchCount = 3;
|
||||
const layers = Array.from({ length: layerCount }, (_, layerIndex) =>
|
||||
Array.from(
|
||||
{ length: branchCount },
|
||||
(_, branchIndex) => `layer-${String(layerIndex).padStart(2, "0")}-${branchIndex}`,
|
||||
),
|
||||
);
|
||||
const denseSteps = [
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [...layers[0]].reverse().map(guardedMoveToStep),
|
||||
}),
|
||||
...layers.flatMap((layer, layerIndex) =>
|
||||
layer.map((id, branchIndex) =>
|
||||
step(id, layerIndex * branchCount + branchIndex + 1, {
|
||||
onTurnComplete:
|
||||
layerIndex === layerCount - 1
|
||||
? [moveToStep("work")]
|
||||
: [...layers[layerIndex + 1]].reverse().map(guardedMoveToStep),
|
||||
}),
|
||||
),
|
||||
),
|
||||
];
|
||||
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles(denseSteps);
|
||||
|
||||
expect(diagnostic.trace.map((hop) => hop.destinationStepId)).toEqual([
|
||||
...layers.map(([lexicallyFirst]) => lexicallyFirst),
|
||||
"work",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("replay diagnostic selection", () => {
|
||||
it("selects a blocking trace before a warning trace", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnStart: [{ type: "move_to_step", config: { step_id: "manual-return" } }],
|
||||
onTurnComplete: [moveToStep(SHORT_RETURN_STEP_ID)],
|
||||
}),
|
||||
step("manual-return", 1, { onTurnComplete: [moveToStep("work")] }),
|
||||
step(SHORT_RETURN_STEP_ID, 2, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("blocking");
|
||||
expect(diagnostic.trace.map((hop) => hop.destinationStepId)).toEqual([
|
||||
SHORT_RETURN_STEP_ID,
|
||||
"work",
|
||||
]);
|
||||
});
|
||||
|
||||
it("selects the shortest trace when candidates have equal severity", () => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
onTurnStart: [{ type: "move_to_step", config: { step_id: SHORT_RETURN_STEP_ID } }],
|
||||
onTurnComplete: [moveToStep("long-a")],
|
||||
}),
|
||||
step(SHORT_RETURN_STEP_ID, 1, { onTurnComplete: [moveToStep("work")] }),
|
||||
step("long-a", 2, { onTurnComplete: [moveToStep("long-b")] }),
|
||||
step("long-b", 3, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostic.severity).toBe("warning");
|
||||
expect(diagnostic.trace.map((hop) => hop.destinationStepId)).toEqual([
|
||||
SHORT_RETURN_STEP_ID,
|
||||
"work",
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps identity stable across names and prompt changes", () => {
|
||||
const original = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, { autoStart: true, onTurnComplete: [moveToStep("review")] }),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
const renamed = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
name: "Implementation",
|
||||
prompt: "Use {{task_prompt}} carefully",
|
||||
onTurnComplete: [moveToStep("review")],
|
||||
}),
|
||||
step("review", 1, {
|
||||
autoStart: true,
|
||||
name: "Code review",
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(renamed.map((diagnostic) => diagnostic.identity)).toEqual(
|
||||
original.map((diagnostic) => diagnostic.identity),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
[undefined, "task_description"],
|
||||
["", "task_description"],
|
||||
["Before\n{{task_prompt}}\nAfter", "step_prompt_with_task_description"],
|
||||
["Review the implementation", "step_prompt"],
|
||||
] as const)("classifies prompt %j as %s", (prompt, expected) => {
|
||||
const [diagnostic] = analyzeWorkflowReplayCycles([
|
||||
step("work", 0, {
|
||||
autoStart: true,
|
||||
prompt,
|
||||
onTurnComplete: [moveToStep("work")],
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(diagnostic.promptSource).toBe(expected);
|
||||
});
|
||||
|
||||
it("uses input order to break equal-position ties and returns diagnostics in that order", () => {
|
||||
const steps = [
|
||||
step("second", 0, { autoStart: true, onTurnComplete: [moveToStep("second")] }),
|
||||
step("first", 0, { autoStart: true, onTurnComplete: [moveToStep("first")] }),
|
||||
];
|
||||
|
||||
expect(
|
||||
analyzeWorkflowReplayCycles(steps).map((diagnostic) => diagnostic.autoStartStepId),
|
||||
).toEqual(["second", "first"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("introduced replay cycle inventory", () => {
|
||||
it("detects a new alternate loop when the preferred trace is unchanged", () => {
|
||||
const baseline = [
|
||||
step(WORK_STEP_ID, 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [guardedMoveToStep(EXISTING_STEP_ID)],
|
||||
}),
|
||||
step(EXISTING_STEP_ID, 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep(WORK_STEP_ID)],
|
||||
}),
|
||||
step("z-new", 2, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep(WORK_STEP_ID)],
|
||||
}),
|
||||
];
|
||||
const proposed = [
|
||||
{
|
||||
...baseline[0],
|
||||
events: {
|
||||
...baseline[0].events,
|
||||
on_turn_complete: [guardedMoveToStep(EXISTING_STEP_ID), guardedMoveToStep("z-new")],
|
||||
},
|
||||
},
|
||||
...baseline.slice(1),
|
||||
];
|
||||
|
||||
expect(analyzeWorkflowReplayCycles(proposed)[0].trace[0].destinationStepId).toBe(
|
||||
EXISTING_STEP_ID,
|
||||
);
|
||||
expect(
|
||||
analyzeIntroducedWorkflowReplayCycles(baseline, proposed)[0].trace.map(
|
||||
(hop) => hop.destinationStepId,
|
||||
),
|
||||
).toEqual(["z-new", WORK_STEP_ID]);
|
||||
});
|
||||
|
||||
it("does not reclassify an existing alternate loop when the preferred trace is removed", () => {
|
||||
const baseline = [
|
||||
step(WORK_STEP_ID, 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [guardedMoveToStep("a-preferred"), guardedMoveToStep(ALTERNATE_STEP_ID)],
|
||||
}),
|
||||
step("a-preferred", 1, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep(WORK_STEP_ID)],
|
||||
}),
|
||||
step(ALTERNATE_STEP_ID, 2, {
|
||||
autoStart: true,
|
||||
onTurnComplete: [moveToStep(WORK_STEP_ID)],
|
||||
}),
|
||||
];
|
||||
const proposed = [
|
||||
{
|
||||
...baseline[0],
|
||||
events: {
|
||||
...baseline[0].events,
|
||||
on_turn_complete: [guardedMoveToStep(ALTERNATE_STEP_ID)],
|
||||
},
|
||||
},
|
||||
...baseline.slice(1),
|
||||
];
|
||||
|
||||
expect(analyzeWorkflowReplayCycles(proposed)[0].trace[0].destinationStepId).toBe(
|
||||
ALTERNATE_STEP_ID,
|
||||
);
|
||||
expect(analyzeIntroducedWorkflowReplayCycles(baseline, proposed)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("bounded replay cycle inventory", () => {
|
||||
it("fails conservatively when alternate cycle inventory is truncated", () => {
|
||||
const existingStepIds = Array.from(
|
||||
{ length: 257 },
|
||||
(_, index) => `existing-${index.toString().padStart(3, "0")}`,
|
||||
);
|
||||
const baseline = [
|
||||
step(WORK_STEP_ID, 0, {
|
||||
autoStart: true,
|
||||
onTurnComplete: existingStepIds.map(guardedMoveToStep),
|
||||
}),
|
||||
...existingStepIds.map((stepId, index) =>
|
||||
step(stepId, index + 1, {
|
||||
onTurnComplete: [moveToStep(WORK_STEP_ID)],
|
||||
}),
|
||||
),
|
||||
];
|
||||
const newStepId = "new-beyond-inventory-cap";
|
||||
const proposed = [
|
||||
{
|
||||
...baseline[0],
|
||||
events: {
|
||||
...baseline[0].events,
|
||||
on_turn_complete: [
|
||||
...(baseline[0].events?.on_turn_complete ?? []),
|
||||
guardedMoveToStep(newStepId),
|
||||
],
|
||||
},
|
||||
},
|
||||
...baseline.slice(1),
|
||||
step(newStepId, baseline.length, {
|
||||
onTurnComplete: [moveToStep(WORK_STEP_ID)],
|
||||
}),
|
||||
];
|
||||
|
||||
expect(analyzeIntroducedWorkflowReplayCycles(baseline, proposed)).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,503 @@
|
||||
import { stepHasOnEnterAction, type WorkflowStep } from "@/lib/types/http";
|
||||
import type {
|
||||
OnTurnCompleteAction,
|
||||
OnTurnCompleteActionType,
|
||||
OnTurnStartAction,
|
||||
OnTurnStartActionType,
|
||||
} from "@/lib/types/workflow-actions";
|
||||
|
||||
export type WorkflowReplayCycleSeverity = "blocking" | "warning";
|
||||
export type WorkflowReplayCycleTrigger = "on_turn_start" | "on_turn_complete";
|
||||
export type WorkflowReplayCycleActionKind = Extract<
|
||||
OnTurnStartActionType | OnTurnCompleteActionType,
|
||||
"move_to_next" | "move_to_previous" | "move_to_step"
|
||||
>;
|
||||
export type WorkflowReplayPromptSource =
|
||||
| "task_description"
|
||||
| "step_prompt_with_task_description"
|
||||
| "step_prompt";
|
||||
|
||||
export type WorkflowReplayCycleHop = {
|
||||
sourceStepId: string;
|
||||
sourceStepName: string;
|
||||
trigger: WorkflowReplayCycleTrigger;
|
||||
actionKind: WorkflowReplayCycleActionKind;
|
||||
destinationStepId: string;
|
||||
destinationStepName: string;
|
||||
requiresUserInvolvement: boolean;
|
||||
};
|
||||
|
||||
export type WorkflowReplayCycleDiagnostic = {
|
||||
identity: string;
|
||||
severity: WorkflowReplayCycleSeverity;
|
||||
autoStartStepId: string;
|
||||
autoStartStepName: string;
|
||||
affectedStepIds: string[];
|
||||
trace: WorkflowReplayCycleHop[];
|
||||
promptSource: WorkflowReplayPromptSource;
|
||||
};
|
||||
|
||||
type TransitionAction = OnTurnStartAction | OnTurnCompleteAction;
|
||||
|
||||
type ReplayGraphEdge = {
|
||||
source: WorkflowStep;
|
||||
destination: WorkflowStep;
|
||||
trigger: WorkflowReplayCycleTrigger;
|
||||
actionKind: WorkflowReplayCycleActionKind;
|
||||
requiresUserInvolvement: boolean;
|
||||
};
|
||||
|
||||
type StepResolutionContext = {
|
||||
orderedSteps: WorkflowStep[];
|
||||
stepsById: Map<string, WorkflowStep>;
|
||||
};
|
||||
|
||||
type CycleCandidate = {
|
||||
identity: string;
|
||||
severity: WorkflowReplayCycleSeverity;
|
||||
trace: WorkflowReplayCycleHop[];
|
||||
};
|
||||
|
||||
type ReplayAnalysisContext = {
|
||||
orderedSteps: WorkflowStep[];
|
||||
graph: Map<string, ReplayGraphEdge[]>;
|
||||
};
|
||||
|
||||
type CycleInventory = {
|
||||
candidates: CycleCandidate[];
|
||||
truncated: boolean;
|
||||
};
|
||||
|
||||
const MAX_CYCLE_INVENTORY_SIZE = 256;
|
||||
const MAX_CYCLE_INVENTORY_STATES = 4096;
|
||||
|
||||
export function analyzeWorkflowReplayCycles(
|
||||
steps: WorkflowStep[],
|
||||
): WorkflowReplayCycleDiagnostic[] {
|
||||
const { orderedSteps, graph } = buildAnalysisContext(steps);
|
||||
|
||||
return orderedSteps.flatMap((autoStartStep) => {
|
||||
if (!stepHasOnEnterAction(autoStartStep, "auto_start_agent")) return [];
|
||||
const selected = findPreferredCycleCandidate(autoStartStep, graph);
|
||||
return selected ? [toDiagnostic(autoStartStep, selected)] : [];
|
||||
});
|
||||
}
|
||||
|
||||
export function analyzeIntroducedWorkflowReplayCycles(
|
||||
baselineSteps: WorkflowStep[],
|
||||
proposedSteps: WorkflowStep[],
|
||||
): WorkflowReplayCycleDiagnostic[] {
|
||||
const baseline = buildAnalysisContext(baselineSteps);
|
||||
const existingIdentities = new Set(
|
||||
baseline.orderedSteps.flatMap((step) =>
|
||||
stepHasOnEnterAction(step, "auto_start_agent")
|
||||
? collectCycleInventory(step, baseline.graph).candidates.map(
|
||||
(candidate) => candidate.identity,
|
||||
)
|
||||
: [],
|
||||
),
|
||||
);
|
||||
const proposed = buildAnalysisContext(proposedSteps);
|
||||
|
||||
return proposed.orderedSteps.flatMap((autoStartStep) => {
|
||||
if (!stepHasOnEnterAction(autoStartStep, "auto_start_agent")) return [];
|
||||
const inventory = collectCycleInventory(autoStartStep, proposed.graph);
|
||||
const selected = inventory.candidates
|
||||
.filter((candidate) => !existingIdentities.has(candidate.identity))
|
||||
.sort(compareCandidates)[0];
|
||||
const fallback = inventory.truncated ? inventory.candidates[0] : undefined;
|
||||
const diagnosticCandidate = selected ?? fallback;
|
||||
return diagnosticCandidate ? [toDiagnostic(autoStartStep, diagnosticCandidate)] : [];
|
||||
});
|
||||
}
|
||||
|
||||
function buildAnalysisContext(steps: WorkflowStep[]): ReplayAnalysisContext {
|
||||
const orderedSteps = steps
|
||||
.map((step, inputIndex) => ({ step, inputIndex }))
|
||||
.sort(
|
||||
(left, right) =>
|
||||
left.step.position - right.step.position || left.inputIndex - right.inputIndex,
|
||||
)
|
||||
.map(({ step }) => step);
|
||||
return { orderedSteps, graph: buildReplayGraph(orderedSteps) };
|
||||
}
|
||||
|
||||
function toDiagnostic(
|
||||
autoStartStep: WorkflowStep,
|
||||
candidate: CycleCandidate,
|
||||
): WorkflowReplayCycleDiagnostic {
|
||||
return {
|
||||
identity: candidate.identity,
|
||||
severity: candidate.severity,
|
||||
autoStartStepId: autoStartStep.id,
|
||||
autoStartStepName: autoStartStep.name,
|
||||
affectedStepIds: affectedStepIds(autoStartStep.id, candidate.trace),
|
||||
trace: candidate.trace,
|
||||
promptSource: classifyPromptSource(autoStartStep.prompt),
|
||||
};
|
||||
}
|
||||
|
||||
function buildReplayGraph(steps: WorkflowStep[]): Map<string, ReplayGraphEdge[]> {
|
||||
const graph = new Map<string, ReplayGraphEdge[]>();
|
||||
const stepsById = new Map(steps.map((step) => [step.id, step]));
|
||||
const resolution = { orderedSteps: steps, stepsById };
|
||||
|
||||
steps.forEach((source, sourceIndex) => {
|
||||
const edges = [
|
||||
...resolveTriggerEdges(
|
||||
source,
|
||||
sourceIndex,
|
||||
"on_turn_start",
|
||||
source.events?.on_turn_start ?? [],
|
||||
resolution,
|
||||
),
|
||||
...resolveTriggerEdges(
|
||||
source,
|
||||
sourceIndex,
|
||||
"on_turn_complete",
|
||||
source.events?.on_turn_complete ?? [],
|
||||
resolution,
|
||||
),
|
||||
].sort(compareGraphEdges);
|
||||
graph.set(source.id, edges);
|
||||
});
|
||||
|
||||
return graph;
|
||||
}
|
||||
|
||||
function resolveTriggerEdges(
|
||||
source: WorkflowStep,
|
||||
sourceIndex: number,
|
||||
trigger: WorkflowReplayCycleTrigger,
|
||||
actions: TransitionAction[],
|
||||
resolution: StepResolutionContext,
|
||||
): ReplayGraphEdge[] {
|
||||
const edges: ReplayGraphEdge[] = [];
|
||||
for (const action of actions) {
|
||||
if (!isMoveAction(action)) continue;
|
||||
const destination = resolveDestination(action, sourceIndex, resolution);
|
||||
if (destination) {
|
||||
edges.push({
|
||||
source,
|
||||
destination,
|
||||
trigger,
|
||||
actionKind: action.type,
|
||||
requiresUserInvolvement:
|
||||
!stepHasOnEnterAction(source, "auto_start_agent") ||
|
||||
action.config?.requires_approval === true,
|
||||
});
|
||||
}
|
||||
if (isAlwaysEligibleTransition(action)) break;
|
||||
}
|
||||
return edges;
|
||||
}
|
||||
|
||||
function isMoveAction(
|
||||
action: TransitionAction,
|
||||
): action is Extract<TransitionAction, { type: WorkflowReplayCycleActionKind }> {
|
||||
return ["move_to_next", "move_to_previous", "move_to_step"].includes(action.type);
|
||||
}
|
||||
|
||||
function isAlwaysEligibleTransition(
|
||||
action: Extract<TransitionAction, { type: WorkflowReplayCycleActionKind }>,
|
||||
): boolean {
|
||||
return action.config?.requires_approval !== true && !hasValidTransitionGuard(action.config);
|
||||
}
|
||||
|
||||
function hasValidTransitionGuard(
|
||||
config: Extract<TransitionAction, { type: WorkflowReplayCycleActionKind }>["config"],
|
||||
): boolean {
|
||||
const legacyConfig = config as
|
||||
| (typeof config & {
|
||||
wait_for_quorum?: { role?: unknown; threshold?: unknown };
|
||||
})
|
||||
| undefined;
|
||||
const quorum = legacyConfig?.wait_for_quorum ?? config?.if?.wait_for_quorum;
|
||||
return (
|
||||
typeof quorum?.role === "string" &&
|
||||
quorum.role !== "" &&
|
||||
typeof quorum.threshold === "string" &&
|
||||
quorum.threshold !== ""
|
||||
);
|
||||
}
|
||||
|
||||
function resolveDestination(
|
||||
action: Extract<TransitionAction, { type: WorkflowReplayCycleActionKind }>,
|
||||
sourceIndex: number,
|
||||
resolution: StepResolutionContext,
|
||||
): WorkflowStep | undefined {
|
||||
if (action.type === "move_to_next") return resolution.orderedSteps[sourceIndex + 1];
|
||||
if (action.type === "move_to_previous") return resolution.orderedSteps[sourceIndex - 1];
|
||||
return resolution.stepsById.get(action.config?.step_id ?? "");
|
||||
}
|
||||
|
||||
type CycleSearchState = {
|
||||
stepId: string;
|
||||
hasUserInvolvement: boolean;
|
||||
trace: WorkflowReplayCycleHop[];
|
||||
};
|
||||
|
||||
type CycleSearchExpansion =
|
||||
| { kind: "closing"; candidate: CycleCandidate }
|
||||
| { kind: "next"; state: CycleSearchState };
|
||||
|
||||
function findPreferredCycleCandidate(
|
||||
autoStartStep: WorkflowStep,
|
||||
graph: Map<string, ReplayGraphEdge[]>,
|
||||
): CycleCandidate | undefined {
|
||||
return (
|
||||
findShortestCycleCandidate(autoStartStep, graph, "blocking") ??
|
||||
findShortestCycleCandidate(autoStartStep, graph, "warning")
|
||||
);
|
||||
}
|
||||
|
||||
type CycleInventorySearchState = {
|
||||
stepId: string;
|
||||
trace: WorkflowReplayCycleHop[];
|
||||
visitedStepIds: Set<string>;
|
||||
};
|
||||
|
||||
function collectCycleInventory(
|
||||
autoStartStep: WorkflowStep,
|
||||
graph: Map<string, ReplayGraphEdge[]>,
|
||||
): CycleInventory {
|
||||
const candidates = new Map<string, CycleCandidate>();
|
||||
let truncated = false;
|
||||
const preferred = findPreferredCycleCandidate(autoStartStep, graph);
|
||||
if (preferred) candidates.set(preferred.identity, preferred);
|
||||
|
||||
const queue: CycleInventorySearchState[] = [
|
||||
{
|
||||
stepId: autoStartStep.id,
|
||||
trace: [],
|
||||
visitedStepIds: new Set([autoStartStep.id]),
|
||||
},
|
||||
];
|
||||
|
||||
for (
|
||||
let index = 0;
|
||||
index < queue.length &&
|
||||
index < MAX_CYCLE_INVENTORY_STATES &&
|
||||
candidates.size < MAX_CYCLE_INVENTORY_SIZE;
|
||||
index += 1
|
||||
) {
|
||||
const state = queue[index];
|
||||
for (const edge of graph.get(state.stepId) ?? []) {
|
||||
truncated =
|
||||
expandCycleInventoryEdge(autoStartStep.id, state, edge, queue, candidates) || truncated;
|
||||
if (candidates.size >= MAX_CYCLE_INVENTORY_SIZE) break;
|
||||
}
|
||||
}
|
||||
|
||||
return { candidates: [...candidates.values()].sort(compareCandidates), truncated };
|
||||
}
|
||||
|
||||
function expandCycleInventoryEdge(
|
||||
autoStartStepId: string,
|
||||
state: CycleInventorySearchState,
|
||||
edge: ReplayGraphEdge,
|
||||
queue: CycleInventorySearchState[],
|
||||
candidates: Map<string, CycleCandidate>,
|
||||
): boolean {
|
||||
const trace = [...state.trace, toTraceHop(edge)];
|
||||
if (edge.destination.id === autoStartStepId) {
|
||||
if (edge.trigger !== "on_turn_complete") return false;
|
||||
const candidate = toCandidate(autoStartStepId, trace);
|
||||
candidates.set(candidate.identity, candidate);
|
||||
return candidates.size >= MAX_CYCLE_INVENTORY_SIZE;
|
||||
}
|
||||
if (state.visitedStepIds.has(edge.destination.id)) return false;
|
||||
if (queue.length >= MAX_CYCLE_INVENTORY_STATES) return true;
|
||||
queue.push({
|
||||
stepId: edge.destination.id,
|
||||
trace,
|
||||
visitedStepIds: new Set([...state.visitedStepIds, edge.destination.id]),
|
||||
});
|
||||
return false;
|
||||
}
|
||||
|
||||
function findShortestCycleCandidate(
|
||||
autoStartStep: WorkflowStep,
|
||||
graph: Map<string, ReplayGraphEdge[]>,
|
||||
severity: WorkflowReplayCycleSeverity,
|
||||
): CycleCandidate | undefined {
|
||||
let frontier: CycleSearchState[] = [
|
||||
{ stepId: autoStartStep.id, hasUserInvolvement: false, trace: [] },
|
||||
];
|
||||
const visitedStates = new Set([cycleSearchStateKey(autoStartStep.id, false)]);
|
||||
|
||||
while (frontier.length > 0) {
|
||||
const { closingCandidates, nextFrontier } = expandCycleSearchLevel(
|
||||
autoStartStep.id,
|
||||
graph,
|
||||
severity,
|
||||
frontier,
|
||||
visitedStates,
|
||||
);
|
||||
|
||||
if (closingCandidates.length > 0) {
|
||||
return closingCandidates.sort(compareCandidates)[0];
|
||||
}
|
||||
|
||||
frontier = [...nextFrontier.values()].sort((left, right) =>
|
||||
compareTraceIdentity(autoStartStep.id, left.trace, right.trace),
|
||||
);
|
||||
for (const state of frontier) {
|
||||
visitedStates.add(cycleSearchStateKey(state.stepId, state.hasUserInvolvement));
|
||||
}
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function expandCycleSearchLevel(
|
||||
autoStartStepId: string,
|
||||
graph: Map<string, ReplayGraphEdge[]>,
|
||||
severity: WorkflowReplayCycleSeverity,
|
||||
frontier: CycleSearchState[],
|
||||
visitedStates: Set<string>,
|
||||
): {
|
||||
closingCandidates: CycleCandidate[];
|
||||
nextFrontier: Map<string, CycleSearchState>;
|
||||
} {
|
||||
const closingCandidates: CycleCandidate[] = [];
|
||||
const nextFrontier = new Map<string, CycleSearchState>();
|
||||
|
||||
for (const state of frontier) {
|
||||
for (const edge of graph.get(state.stepId) ?? []) {
|
||||
const expansion = expandCycleSearchEdge(autoStartStepId, severity, state, edge);
|
||||
if (!expansion) continue;
|
||||
if (expansion.kind === "closing") {
|
||||
closingCandidates.push(expansion.candidate);
|
||||
continue;
|
||||
}
|
||||
retainPreferredSearchState(autoStartStepId, expansion.state, visitedStates, nextFrontier);
|
||||
}
|
||||
}
|
||||
|
||||
return { closingCandidates, nextFrontier };
|
||||
}
|
||||
|
||||
function expandCycleSearchEdge(
|
||||
autoStartStepId: string,
|
||||
severity: WorkflowReplayCycleSeverity,
|
||||
state: CycleSearchState,
|
||||
edge: ReplayGraphEdge,
|
||||
): CycleSearchExpansion | undefined {
|
||||
const hasUserInvolvement = state.hasUserInvolvement || edge.requiresUserInvolvement;
|
||||
if (severity === "blocking" && hasUserInvolvement) return undefined;
|
||||
|
||||
const trace = [...state.trace, toTraceHop(edge)];
|
||||
if (edge.destination.id !== autoStartStepId) {
|
||||
return {
|
||||
kind: "next",
|
||||
state: { stepId: edge.destination.id, hasUserInvolvement, trace },
|
||||
};
|
||||
}
|
||||
if (edge.trigger !== "on_turn_complete") return undefined;
|
||||
if ((severity === "warning") !== hasUserInvolvement) return undefined;
|
||||
return { kind: "closing", candidate: toCandidate(autoStartStepId, trace) };
|
||||
}
|
||||
|
||||
function retainPreferredSearchState(
|
||||
autoStartStepId: string,
|
||||
nextState: CycleSearchState,
|
||||
visitedStates: Set<string>,
|
||||
nextFrontier: Map<string, CycleSearchState>,
|
||||
): void {
|
||||
const stateKey = cycleSearchStateKey(nextState.stepId, nextState.hasUserInvolvement);
|
||||
if (visitedStates.has(stateKey)) return;
|
||||
|
||||
const currentState = nextFrontier.get(stateKey);
|
||||
if (
|
||||
!currentState ||
|
||||
compareTraceIdentity(autoStartStepId, nextState.trace, currentState.trace) < 0
|
||||
) {
|
||||
nextFrontier.set(stateKey, nextState);
|
||||
}
|
||||
}
|
||||
|
||||
function toTraceHop(edge: ReplayGraphEdge): WorkflowReplayCycleHop {
|
||||
return {
|
||||
sourceStepId: edge.source.id,
|
||||
sourceStepName: edge.source.name,
|
||||
trigger: edge.trigger,
|
||||
actionKind: edge.actionKind,
|
||||
destinationStepId: edge.destination.id,
|
||||
destinationStepName: edge.destination.name,
|
||||
requiresUserInvolvement: edge.requiresUserInvolvement,
|
||||
};
|
||||
}
|
||||
|
||||
function toCandidate(autoStartStepId: string, trace: WorkflowReplayCycleHop[]): CycleCandidate {
|
||||
const severity = trace.some((hop) => hop.requiresUserInvolvement) ? "warning" : "blocking";
|
||||
return {
|
||||
identity: traceIdentity(autoStartStepId, trace),
|
||||
severity,
|
||||
trace,
|
||||
};
|
||||
}
|
||||
|
||||
function traceIdentity(autoStartStepId: string, trace: WorkflowReplayCycleHop[]): string {
|
||||
const identityParts = trace.map((hop) => [
|
||||
hop.sourceStepId,
|
||||
hop.trigger,
|
||||
hop.actionKind,
|
||||
hop.destinationStepId,
|
||||
hop.requiresUserInvolvement,
|
||||
]);
|
||||
return `workflow-replay-cycle:${JSON.stringify([autoStartStepId, identityParts])}`;
|
||||
}
|
||||
|
||||
function compareTraceIdentity(
|
||||
autoStartStepId: string,
|
||||
left: WorkflowReplayCycleHop[],
|
||||
right: WorkflowReplayCycleHop[],
|
||||
): number {
|
||||
return compareLexical(
|
||||
traceIdentity(autoStartStepId, left),
|
||||
traceIdentity(autoStartStepId, right),
|
||||
);
|
||||
}
|
||||
|
||||
function cycleSearchStateKey(stepId: string, hasUserInvolvement: boolean): string {
|
||||
return JSON.stringify([stepId, hasUserInvolvement]);
|
||||
}
|
||||
|
||||
function compareGraphEdges(left: ReplayGraphEdge, right: ReplayGraphEdge): number {
|
||||
return compareLexical(graphEdgeIdentity(left), graphEdgeIdentity(right));
|
||||
}
|
||||
|
||||
function graphEdgeIdentity(edge: ReplayGraphEdge): string {
|
||||
return JSON.stringify([
|
||||
edge.source.id,
|
||||
edge.trigger,
|
||||
edge.actionKind,
|
||||
edge.destination.id,
|
||||
edge.requiresUserInvolvement,
|
||||
]);
|
||||
}
|
||||
|
||||
function compareLexical(left: string, right: string): number {
|
||||
if (left === right) return 0;
|
||||
return left < right ? -1 : 1;
|
||||
}
|
||||
|
||||
function compareCandidates(left: CycleCandidate, right: CycleCandidate): number {
|
||||
if (left.severity !== right.severity) return left.severity === "blocking" ? -1 : 1;
|
||||
const lengthDifference = left.trace.length - right.trace.length;
|
||||
if (lengthDifference !== 0) return lengthDifference;
|
||||
if (left.identity === right.identity) return 0;
|
||||
return left.identity < right.identity ? -1 : 1;
|
||||
}
|
||||
|
||||
function affectedStepIds(autoStartStepId: string, trace: WorkflowReplayCycleHop[]): string[] {
|
||||
const affected = new Set([autoStartStepId]);
|
||||
trace.forEach((hop) => affected.add(hop.destinationStepId));
|
||||
return [...affected];
|
||||
}
|
||||
|
||||
function classifyPromptSource(prompt: string | undefined): WorkflowReplayPromptSource {
|
||||
if (!prompt) return "task_description";
|
||||
return prompt.includes("{{task_prompt}}") ? "step_prompt_with_task_description" : "step_prompt";
|
||||
}
|
||||
@@ -0,0 +1,277 @@
|
||||
---
|
||||
spec: docs/specs/workflow-cycle-guardrails/spec.md
|
||||
created: 2026-07-15
|
||||
status: complete
|
||||
---
|
||||
|
||||
# Implementation Plan: Workflow Cycle Guardrails
|
||||
|
||||
## Overview
|
||||
|
||||
Add a deterministic frontend graph analyzer first, then place a shared mutation
|
||||
guard in the workflow card so both draft creation and existing immediate step
|
||||
mutations use the same proposed-shape decision. Build the trace UI on that
|
||||
contract and finish with desktop and mobile Playwright coverage. The existing
|
||||
workflow HTTP contracts and runtime engine remain unchanged.
|
||||
|
||||
---
|
||||
|
||||
## Backend
|
||||
|
||||
No backend changes are planned. The feature is an authoring-time guard over the
|
||||
complete client-visible `WorkflowStep[]`; it intentionally does not make replay
|
||||
cycles a persistence invariant or alter `POST /api/v1/workflow/steps`,
|
||||
`PUT /api/v1/workflow/steps/:id`, step deletion/reorder, or workflow import.
|
||||
|
||||
Relevant contracts verified during planning:
|
||||
|
||||
- `apps/backend/internal/workflow/models/models.go` defines `on_enter`,
|
||||
`on_turn_start`, and `on_turn_complete` action semantics.
|
||||
- `apps/backend/internal/workflow/engine/types.go` resolves the same three move
|
||||
action kinds and applies first-eligible-transition behavior.
|
||||
- `apps/backend/internal/orchestrator/event_handlers_workflow.go` confirms that
|
||||
`on_turn_start` is user-message driven and an entered `auto_start_agent` step
|
||||
starts the workflow prompt.
|
||||
- `apps/backend/internal/orchestrator/task_operations.go` defines the three
|
||||
prompt-source cases that the diagnostic describes.
|
||||
|
||||
---
|
||||
|
||||
## Frontend
|
||||
|
||||
### Replay-cycle analyzer
|
||||
|
||||
Files:
|
||||
|
||||
- `apps/web/lib/workflows/replay-cycle-analysis.ts` (new)
|
||||
- `apps/web/lib/workflows/replay-cycle-analysis.test.ts` (new)
|
||||
|
||||
Changes:
|
||||
|
||||
- Add `analyzeWorkflowReplayCycles(steps: WorkflowStep[])` and exported
|
||||
diagnostic/trace types matching the spec's Analysis Contract.
|
||||
- Normalize order by `position`, resolve `move_to_next`,
|
||||
`move_to_previous`, and valid `move_to_step` targets for both kanban
|
||||
triggers, and find paths that return to each `on_enter.auto_start_agent`
|
||||
step through an `on_turn_complete` replay edge. Treat `on_turn_start` entry
|
||||
into an auto-start step as safe because the runtime skips destination
|
||||
`on_enter` while dispatching the user's message.
|
||||
- Classify a hop as automatic when its source auto-starts on entry and the
|
||||
transition does not require approval. This includes `on_turn_start`
|
||||
transitions fired by an auto-started prompt. A cycle is blocking only when
|
||||
all of its hops are automatic.
|
||||
- Produce stable diagnostic identities, affected step IDs, a deterministic
|
||||
highest-severity/shortest trace per auto-start step, and the prompt-source
|
||||
category.
|
||||
- Ignore unresolved targets and cycles without an auto-start re-entry.
|
||||
|
||||
### Proposed-shape mutation guard
|
||||
|
||||
Files:
|
||||
|
||||
- `apps/web/components/settings/workflow-card-actions.ts`
|
||||
- `apps/web/components/settings/workflow-card-actions.test.ts`
|
||||
- `apps/web/components/settings/workflow-card.tsx`
|
||||
|
||||
Changes:
|
||||
|
||||
- Introduce a workflow-card guard state that compares analyzer diagnostics for
|
||||
the current server-backed shape and a proposed shape.
|
||||
- Route existing step update, add, remove, and reorder handlers through a
|
||||
single guard before their current API call. Build proposed arrays with the
|
||||
same step update/start-step demotion, add, delete/reindex, and reorder
|
||||
semantics already used by the handlers.
|
||||
- For a new draft, run the guard in `handleSaveWorkflow` before
|
||||
`createWorkflowAction` so cancellation sends neither the workflow create nor
|
||||
any follow-up step mutations.
|
||||
- Hold one pending user-mediated operation until confirmation; execute it once
|
||||
on **Apply anyway** or **Create anyway**, and discard it on cancel.
|
||||
- Reject fully automatic proposals without exposing an override. Compare a
|
||||
bounded identity inventory so alternate cycles are not hidden by the
|
||||
preferred display trace, and fall back conservatively when that inventory is
|
||||
exhausted. Always allow changes that remove diagnostics.
|
||||
- Preserve current request errors, temp-step remapping, optimistic reorder
|
||||
feedback, and workflow creation sequencing. Reconcile successful remote
|
||||
mutations from their authoritative responses and roll failed reorders back.
|
||||
|
||||
### Diagnostic presentation
|
||||
|
||||
Files:
|
||||
|
||||
- `apps/web/components/settings/workflow-cycle-diagnostic.tsx` (new)
|
||||
- `apps/web/components/settings/workflow-pipeline-editor.tsx`
|
||||
- `apps/web/components/settings/workflow-card.tsx`
|
||||
|
||||
Changes:
|
||||
|
||||
- Add a reusable diagnostic view for the inline alert and the confirmation or
|
||||
blocking dialog. Render severity, ordered trace, user-mediated hops, and the
|
||||
prompt-source explanation without rendering prompt contents.
|
||||
- Place the inline alert above the pipeline's horizontal scroll area and pass
|
||||
affected step IDs into pipeline nodes for icon/text plus border treatment,
|
||||
so affected steps are not communicated by color alone.
|
||||
- Use an `AlertDialog` for a pending proposal. Blocking diagnostics provide
|
||||
only a return action; warning diagnostics provide Cancel plus the context
|
||||
appropriate **Apply anyway** or **Create anyway** action.
|
||||
- Keep dialog actions and the full trace keyboard accessible. At narrow widths,
|
||||
stack trace hops and actions, wrap long names, maintain touch-sized controls,
|
||||
and prevent horizontal page overflow while leaving the pipeline's contained
|
||||
horizontal scrolling intact.
|
||||
|
||||
No API-client or Zustand state changes are required; workflow cards already own
|
||||
their loaded step arrays and mutation callbacks.
|
||||
|
||||
---
|
||||
|
||||
## Tests
|
||||
|
||||
- **What:** transition resolution for next, previous, and explicit targets on
|
||||
both triggers, including reordered positions and dangling targets.
|
||||
**File:** `apps/web/lib/workflows/replay-cycle-analysis.test.ts`
|
||||
**How:** table-driven pure unit tests.
|
||||
|
||||
- **What:** fully automatic cycles are blocking only when every source step
|
||||
auto-starts and every hop is unapproved `on_turn_complete`.
|
||||
**File:** `apps/web/lib/workflows/replay-cycle-analysis.test.ts`
|
||||
**How:** graph fixtures that vary auto-start, trigger, and approval config.
|
||||
|
||||
- **What:** user-mediated cycles, generic cycles without auto-start, stable
|
||||
diagnostic identities, deterministic trace selection, and all three prompt
|
||||
sources.
|
||||
**File:** `apps/web/lib/workflows/replay-cycle-analysis.test.ts`
|
||||
**How:** focused unit fixtures.
|
||||
|
||||
- **What:** the built-in Review `on_turn_start` feedback edge into In Progress
|
||||
does not warn or replay auto-start behavior.
|
||||
**File:** `apps/web/lib/workflows/replay-cycle-analysis.test.ts`
|
||||
**How:** regression fixture matching the built-in Kanban transition shape.
|
||||
|
||||
- **What:** an existing step edit that introduces a blocking cycle does not call
|
||||
`updateWorkflowStepAction`; a warning holds the call until confirmation and
|
||||
calls it exactly once; cancel does not call it.
|
||||
**File:** `apps/web/components/settings/workflow-card-actions.test.ts`
|
||||
**How:** mocked action tests around the guard/controller helper extracted from
|
||||
`useWorkflowStepActions`.
|
||||
|
||||
- **What:** add, remove, and reorder use proposed full shapes and existing
|
||||
diagnostics are not repeatedly confirmed.
|
||||
**File:** `apps/web/components/settings/workflow-card-actions.test.ts`
|
||||
**How:** mocked action tests with baseline/proposed diagnostic identities.
|
||||
|
||||
- **What:** draft creation is blocked before `createWorkflowAction`, warning
|
||||
cancellation sends no requests, and confirmation runs the existing creation
|
||||
sequence once.
|
||||
**File:** `apps/web/components/settings/workflow-card-actions.test.ts`
|
||||
**How:** extend the current new-workflow save tests.
|
||||
|
||||
- **What:** the inline and dialog variants render the exact trace, prompt source,
|
||||
affected-step accessibility text, and correct action labels.
|
||||
**File:** `apps/web/components/settings/workflow-cycle-diagnostic.test.tsx`
|
||||
**How:** Vitest/Testing Library component tests.
|
||||
|
||||
---
|
||||
|
||||
## E2E Tests
|
||||
|
||||
- **Scenario:** a persisted existing-workflow edit introduces a user-mediated
|
||||
return path to an auto-start step; no PUT is observable until **Apply
|
||||
anyway**, and the inline alert remains after persistence.
|
||||
**File:** `apps/web/e2e/tests/workflow/workflow-cycle-guardrails.spec.ts`
|
||||
**What to verify:** exact step/trigger trace, task-description prompt source,
|
||||
cancellation behavior, confirmation, and persisted events through the API
|
||||
helper.
|
||||
|
||||
- **Scenario:** a draft workflow contains a fully automatic cycle; saving is
|
||||
blocked and no workflow is created.
|
||||
**File:** `apps/web/e2e/tests/workflow/workflow-cycle-guardrails.spec.ts`
|
||||
**What to verify:** blocking severity, no override action, and workflow absent
|
||||
after reload/API list.
|
||||
|
||||
- **Scenario:** a cycle without auto-start remains allowed.
|
||||
**File:** `apps/web/e2e/tests/workflow/workflow-cycle-guardrails.spec.ts`
|
||||
**What to verify:** no diagnostic and normal persistence.
|
||||
|
||||
- **Scenario:** at a mobile viewport, a user-mediated draft warning exposes the
|
||||
complete trace and prompt source and can be cancelled/confirmed by touch
|
||||
without horizontal page overflow.
|
||||
**File:** `apps/web/e2e/tests/workflow/mobile-workflow-cycle-guardrails.spec.ts`
|
||||
**What to verify:** touch-reachable dialog actions, wrapped/visible trace,
|
||||
successful confirmation, and document width not exceeding viewport width.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Waves
|
||||
|
||||
Wave 1:
|
||||
|
||||
- [x] [task-01-cycle-analyzer](task-01-cycle-analyzer.md) (done)
|
||||
|
||||
Wave 2:
|
||||
|
||||
- [x] [task-02-mutation-guard](task-02-mutation-guard.md) (done)
|
||||
|
||||
Wave 3:
|
||||
|
||||
- [x] [task-03-diagnostic-ui](task-03-diagnostic-ui.md) (done)
|
||||
|
||||
Wave 4:
|
||||
|
||||
- [x] [task-04-e2e-verification](task-04-e2e-verification.md) (done)
|
||||
|
||||
---
|
||||
|
||||
## Verification Commands
|
||||
|
||||
Format first:
|
||||
|
||||
```bash
|
||||
make fmt
|
||||
```
|
||||
|
||||
Focused frontend unit tests:
|
||||
|
||||
```bash
|
||||
(cd apps && pnpm --filter @kandev/web test -- lib/workflows/replay-cycle-analysis.test.ts components/settings/workflow-card-actions.test.ts components/settings/workflow-cycle-diagnostic.test.tsx)
|
||||
```
|
||||
|
||||
Frontend typecheck and lint:
|
||||
|
||||
```bash
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps && pnpm --filter @kandev/web lint)
|
||||
```
|
||||
|
||||
Focused desktop and mobile E2E (rebuilds the production web/backend artifacts
|
||||
through the repo E2E harness):
|
||||
|
||||
```bash
|
||||
(cd apps/web && pnpm e2e:run --host tests/workflow/workflow-cycle-guardrails.spec.ts)
|
||||
(cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome tests/workflow/mobile-workflow-cycle-guardrails.spec.ts)
|
||||
```
|
||||
|
||||
Full repository verification:
|
||||
|
||||
```bash
|
||||
make typecheck test lint
|
||||
```
|
||||
|
||||
## Risks
|
||||
|
||||
- `move_to_next` and `move_to_previous` change targets when steps are reordered;
|
||||
every topology-changing handler must analyze its proposed array, not only
|
||||
event updates.
|
||||
- Existing step controls issue remote mutations immediately and some maintain
|
||||
local debounced input. The pending-operation guard must avoid optimistic UI
|
||||
that disagrees with the server after cancel or failure.
|
||||
- The backend engine supports additional generic triggers and configurations.
|
||||
The diagnostic must state its two-trigger scope and tests must prevent the UI
|
||||
from implying that every possible workflow event has been analyzed.
|
||||
- New workflow persistence is a multi-request sequence. Preflighting before the
|
||||
first request is required so a cancelled warning cannot leave a partial
|
||||
workflow.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None for this implementation. The first iteration deliberately guards the
|
||||
kanban-era `on_turn_start` and `on_turn_complete` paths agreed in the feature
|
||||
spec and leaves direct API/import enforcement for separate work.
|
||||
@@ -0,0 +1,54 @@
|
||||
---
|
||||
id: "01-cycle-analyzer"
|
||||
title: "Workflow replay cycle analyzer"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/workflow-cycle-guardrails/spec.md"
|
||||
---
|
||||
|
||||
# Task 01: Workflow replay cycle analyzer
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A pure deterministic analyzer returns the diagnostic contract from the spec
|
||||
for cycles over `on_turn_start` and `on_turn_complete` move actions.
|
||||
- Fully automatic versus user-mediated classification accounts for source-step
|
||||
auto-start and approval, including automatic `on_turn_start` transitions,
|
||||
and ignores cycles without auto-start
|
||||
re-entry through `on_turn_complete`. An `on_turn_start` edge into an
|
||||
auto-start step is covered as a safe non-replay regression.
|
||||
- Trace choice, bounded identity inventory with conservative truncation,
|
||||
affected steps, dangling targets, reorder behavior, and all three
|
||||
prompt-source categories have focused unit coverage.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps && pnpm --filter @kandev/web test -- lib/workflows/replay-cycle-analysis.test.ts)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `apps/web/lib/workflows/replay-cycle-analysis.ts`
|
||||
- `apps/web/lib/workflows/replay-cycle-analysis.test.ts`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Spec sections `What`, `Analysis Contract`, and `Scenarios`.
|
||||
- `apps/web/lib/types/workflow-actions.ts` for action shapes.
|
||||
- `apps/backend/internal/workflow/engine/types.go` for transition resolution
|
||||
and trigger semantics.
|
||||
- `apps/backend/internal/orchestrator/event_handlers_workflow.go` for the
|
||||
auto-start/user-turn distinction.
|
||||
|
||||
## Output Contract
|
||||
|
||||
When finished, change this task's `status` to `done`, check it in `plan.md`, and
|
||||
report the analyzer API, files changed, tests run, blockers, and residual risks.
|
||||
@@ -0,0 +1,54 @@
|
||||
---
|
||||
id: "02-mutation-guard"
|
||||
title: "Workflow mutation guard"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["01-cycle-analyzer"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/workflow-cycle-guardrails/spec.md"
|
||||
---
|
||||
|
||||
# Task 02: Workflow mutation guard
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Existing update/add/delete/reorder handlers analyze their complete proposed
|
||||
shape before sending the current API mutation.
|
||||
- Blocking proposals send no request; warning proposals execute exactly once
|
||||
only after confirmation; existing diagnostic identities and cycle-removing
|
||||
edits are not gated unless identity comparison is conservatively truncated.
|
||||
- Successful remote mutations reconcile from their authoritative responses;
|
||||
reorder remains optimistic while pending and rolls back on request failure.
|
||||
- Draft workflow save preflights before `createWorkflowAction`, with cancel
|
||||
leaving the draft and sending no create or step requests.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps && pnpm --filter @kandev/web test -- components/settings/workflow-card-actions.test.ts lib/workflows/replay-cycle-analysis.test.ts)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `apps/web/components/settings/workflow-card-actions.ts`
|
||||
- `apps/web/components/settings/workflow-card-actions.test.ts`
|
||||
- `apps/web/components/settings/workflow-card.tsx`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Spec scenarios for existing mutations, baseline/proposed comparison, and
|
||||
draft creation.
|
||||
- Analyzer and diagnostic identity contract from Task 01.
|
||||
- Existing `useWorkflowStepActions` and `useWorkflowSaveActions` behavior,
|
||||
including temp-step remapping and request-error feedback.
|
||||
|
||||
## Output Contract
|
||||
|
||||
When finished, change this task's `status` to `done`, check it in `plan.md`, and
|
||||
report guarded operations, request behavior tests, files changed, blockers, and
|
||||
residual risks.
|
||||
@@ -0,0 +1,55 @@
|
||||
---
|
||||
id: "03-diagnostic-ui"
|
||||
title: "Workflow cycle diagnostic UI"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on: ["02-mutation-guard"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/workflow-cycle-guardrails/spec.md"
|
||||
---
|
||||
|
||||
# Task 03: Workflow cycle diagnostic UI
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Existing and proposed diagnostics render severity, exact ordered trigger
|
||||
trace, user-required hops, and prompt source through one reusable component.
|
||||
- Blocking dialogs expose no override; warning dialogs use **Apply anyway** or
|
||||
**Create anyway**, and affected pipeline steps are identified without color
|
||||
alone.
|
||||
- Desktop and narrow layouts keep text/actions usable, keyboard/touch
|
||||
accessible, and page overflow contained.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
make fmt
|
||||
(cd apps && pnpm --filter @kandev/web test -- components/settings/workflow-cycle-diagnostic.test.tsx components/settings/workflow-card-actions.test.ts)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps && pnpm --filter @kandev/web lint)
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `apps/web/components/settings/workflow-cycle-diagnostic.tsx`
|
||||
- `apps/web/components/settings/workflow-cycle-diagnostic.test.tsx`
|
||||
- `apps/web/components/settings/workflow-pipeline-editor.tsx`
|
||||
- `apps/web/components/settings/workflow-card.tsx`
|
||||
- `apps/web/components/settings/workflow-card-dialogs.tsx`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 02.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Spec prompt-source language, trace requirements, and mobile behavior.
|
||||
- Pending-operation state from Task 02.
|
||||
- Existing `AlertDialog`, workflow pipeline scroll area, and workflow-card
|
||||
dialog patterns.
|
||||
|
||||
## Output Contract
|
||||
|
||||
When finished, change this task's `status` to `done`, check it in `plan.md`, and
|
||||
report responsive behavior, accessibility treatment, files changed, tests run,
|
||||
blockers, and residual risks.
|
||||
@@ -0,0 +1,55 @@
|
||||
---
|
||||
id: "04-e2e-verification"
|
||||
title: "Workflow cycle guardrail E2E verification"
|
||||
status: done
|
||||
wave: 4
|
||||
depends_on: ["03-diagnostic-ui"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/workflow-cycle-guardrails/spec.md"
|
||||
---
|
||||
|
||||
# Task 04: Workflow cycle guardrail E2E verification
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Desktop Playwright covers warning cancel/confirm, blocking draft creation,
|
||||
exact trace/prompt source, persisted diagnostics, and an allowed generic
|
||||
cycle without auto-start.
|
||||
- A `mobile-*.spec.ts` test completes the warning confirmation flow by touch
|
||||
and asserts the trace/actions fit without horizontal page overflow.
|
||||
- Focused and repository verification commands run after formatting, with any
|
||||
environment-only blocker reported exactly.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
make fmt
|
||||
(cd apps/web && pnpm e2e:run --host tests/workflow/workflow-cycle-guardrails.spec.ts)
|
||||
(cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome tests/workflow/mobile-workflow-cycle-guardrails.spec.ts)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps && pnpm --filter @kandev/web lint)
|
||||
make typecheck test lint
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `apps/web/e2e/pages/workflow-settings-page.ts`
|
||||
- `apps/web/e2e/tests/workflow/workflow-cycle-guardrails.spec.ts`
|
||||
- `apps/web/e2e/tests/workflow/mobile-workflow-cycle-guardrails.spec.ts`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 03.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Every user-visible spec scenario.
|
||||
- Existing workflow settings page object and API helper patterns.
|
||||
- Mobile parity requirement that mobile tests use the `mobile-` filename
|
||||
convention and real touch-accessible actions.
|
||||
|
||||
## Output Contract
|
||||
|
||||
When finished, change this task's `status` to `done`, check it in `plan.md`, and
|
||||
report desktop/mobile scenarios, files changed, exact commands/results,
|
||||
environment blockers, and residual risks.
|
||||
@@ -124,6 +124,7 @@ System pages (Radarr/Sonarr-style) for status, disk usage, database maintenance,
|
||||
|
||||
| Spec | Status |
|
||||
|---|---|
|
||||
| [workflow-cycle-guardrails](workflow-cycle-guardrails/spec.md) | building |
|
||||
| [improve-kandev](improve-kandev/spec.md) | draft |
|
||||
| [homebrew-core](homebrew-core/spec.md) | draft |
|
||||
| [native-kandev-cli](native-kandev-cli/spec.md) | draft |
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
---
|
||||
status: building
|
||||
created: 2026-07-15
|
||||
owner: kandev
|
||||
---
|
||||
|
||||
# Workflow Cycle Guardrails
|
||||
|
||||
## Why
|
||||
|
||||
Workflow authors can unintentionally connect transition actions into a cycle
|
||||
that re-enters an auto-start step. The repeated agent prompt looks like a user
|
||||
message and may start work the author did not intend, while the current linear
|
||||
pipeline view makes the complete trigger path easy to miss.
|
||||
|
||||
## What
|
||||
|
||||
- The workflow settings editor analyzes `on_turn_start` and
|
||||
`on_turn_complete` transition actions whenever it displays or proposes a
|
||||
workflow shape.
|
||||
- The analysis resolves `move_to_next`, `move_to_previous`, and `move_to_step`
|
||||
against the workflow's ordered steps and reports only cycles whose
|
||||
`on_turn_complete` edge re-enters a step whose `on_enter` actions include
|
||||
`auto_start_agent`. An `on_turn_start` edge into that step does not replay
|
||||
automation because the runtime deliberately skips destination `on_enter`
|
||||
actions while dispatching the user's prompt.
|
||||
- A cycle is **fully automatic** when every hop can occur without a user action:
|
||||
every transition starts from a step that auto-starts an agent on entry, and
|
||||
no hop requires approval. This includes `on_turn_start` transitions fired by
|
||||
an auto-started prompt. Creating or applying a newly introduced fully
|
||||
automatic cycle is blocked.
|
||||
- A cycle is **user-mediated** when its `on_turn_complete` replay edge re-enters
|
||||
an auto-start step but at least one hop needs a user action: a transition
|
||||
from a step that does not auto-start its own turn, or a transition that
|
||||
requires approval. Creating or applying a newly introduced user-mediated
|
||||
cycle requires explicit confirmation.
|
||||
- Transition guards are treated conservatively as possible paths. A guard does
|
||||
not make a cycle safe merely because its condition is not currently met.
|
||||
- Intentional cycles that do not re-enter an `auto_start_agent` step remain
|
||||
valid and do not produce a warning.
|
||||
- The diagnostic shows an ordered trace beginning and ending at the affected
|
||||
auto-start step. Every hop names the source step, trigger, transition action,
|
||||
and destination step, and indicates where a user action is required.
|
||||
- The diagnostic identifies what the re-entered auto-start step sends:
|
||||
- an empty step prompt replays the task description;
|
||||
- a step prompt containing `{{task_prompt}}` sends the rendered step prompt
|
||||
including the task description;
|
||||
- any other non-empty step prompt replaces the task description.
|
||||
- If several cycles re-enter the same auto-start step, the editor shows the
|
||||
highest-severity diagnostic first, then the shortest trace. Diagnostics for
|
||||
distinct auto-start steps remain separately identifiable.
|
||||
- Existing persisted workflows with a replay cycle display the diagnostic as
|
||||
soon as their steps load. This visibility does not retroactively stop active
|
||||
tasks or prevent edits that remove the cycle.
|
||||
- Existing workflows keep their current immediate step persistence. Before a
|
||||
step update, add, delete, or reorder request is sent, the editor analyzes the
|
||||
proposed complete shape:
|
||||
- a newly introduced fully automatic cycle cancels the request and offers no
|
||||
override;
|
||||
- a newly introduced user-mediated cycle holds the request until the user
|
||||
chooses **Apply anyway** or cancels it;
|
||||
- a change that removes a cycle proceeds without confirmation.
|
||||
- A mutation is considered newly introduced only when its resulting diagnostic
|
||||
is absent from the currently persisted shape. Unrelated edits to a workflow
|
||||
that already has a diagnostic do not repeatedly ask for confirmation.
|
||||
- A draft workflow is analyzed locally before its first persistence. Fully
|
||||
automatic cycles disable creation and explain the blocking trace;
|
||||
user-mediated cycles require **Create anyway** confirmation. Cancelling keeps
|
||||
the draft and sends no workflow or step request.
|
||||
- The same diagnostic component is used for the inline workflow alert and the
|
||||
blocking/confirmation dialog. Affected steps are visually identified in the
|
||||
pipeline without relying on color alone.
|
||||
- On mobile, the alert and trace stack vertically, dialog actions remain
|
||||
reachable by touch, transition triggers and actions use human-readable labels,
|
||||
long step names and prompt-source text wrap, and the page does not gain
|
||||
horizontal overflow. Diagnostic content scrolls independently from the action
|
||||
footer so every explanation can be read without being covered. The existing
|
||||
pipeline may retain its own horizontal scrolling region.
|
||||
- The guardrail is advisory for user-mediated cycles and blocking for fully
|
||||
automatic cycles only in the workflow settings authoring surface. It does
|
||||
not change workflow runtime execution or persisted workflow schemas.
|
||||
|
||||
## Analysis Contract
|
||||
|
||||
The analyzer accepts the complete proposed `WorkflowStep[]` and returns zero or
|
||||
more diagnostics. Step order is ascending `position`, with input order as the
|
||||
stable tie-breaker. A transition that cannot resolve to a step in the same
|
||||
array is ignored by cycle analysis and remains subject to existing validation.
|
||||
|
||||
Each diagnostic contains:
|
||||
|
||||
- `severity`: `blocking` or `warning`;
|
||||
- the re-entered auto-start step ID and name;
|
||||
- the ordered affected step IDs;
|
||||
- trace hops containing source ID/name, `on_turn_start` or
|
||||
`on_turn_complete`, action kind, destination ID/name, and whether that hop
|
||||
requires user involvement;
|
||||
- prompt source: `task_description`, `step_prompt_with_task_description`, or
|
||||
`step_prompt`.
|
||||
|
||||
Diagnostics have stable identities derived from the auto-start step and the
|
||||
ordered trace. The authoring guard compares a bounded inventory of these
|
||||
identities between current and proposed shapes so it can distinguish every
|
||||
discovered introduced cycle from an existing one, even when the preferred
|
||||
display trace is unchanged. The UI still presents one deterministic preferred
|
||||
trace per auto-start step. If the bounded inventory is exhausted, the guard
|
||||
conservatively presents that preferred diagnostic instead of allowing the
|
||||
mutation based on an incomplete identity comparison. The analysis is
|
||||
deterministic and has no persisted state.
|
||||
|
||||
## Failure Modes
|
||||
|
||||
- If workflow steps fail to load, the editor keeps its existing load error and
|
||||
does not claim that the workflow is safe.
|
||||
- If a confirmed remote mutation fails, the existing error toast remains the
|
||||
source of truth and the editor retains its last server-backed shape. A
|
||||
successful add, update, or reorder applies the authoritative mutation
|
||||
response before the guard accepts another topology edit. Confirmation never
|
||||
implies persistence succeeded.
|
||||
- If a workflow already contains a fully automatic cycle, opening settings
|
||||
surfaces the blocking diagnostic but does not interrupt running tasks. The
|
||||
author must remove the cycle to prevent future re-entry.
|
||||
- Invalid or dangling transition targets do not crash the editor and do not
|
||||
create a false replay trace.
|
||||
|
||||
## Scenarios
|
||||
|
||||
- **GIVEN** `Build` and `Review` both auto-start on entry and each moves to the
|
||||
other on turn completion, **WHEN** an author attempts to save the second
|
||||
transition, **THEN** the request is blocked and the trace shows
|
||||
`Build --on_turn_complete--> Review --on_turn_complete--> Build`.
|
||||
- **GIVEN** `In Progress` auto-starts, a path includes an `on_turn_start`
|
||||
transition from a step that does not auto-start, and its final
|
||||
`on_turn_complete` edge returns to `In Progress`, **WHEN** an author applies
|
||||
the final edit, **THEN** the editor requires **Apply anyway** and sends no
|
||||
mutation before confirmation.
|
||||
- **GIVEN** every source in the same path auto-starts, **WHEN** an
|
||||
`on_turn_start` transition is fired by an auto-started prompt, **THEN** that
|
||||
hop is automatic and a newly introduced fully automatic replay cycle is
|
||||
blocked.
|
||||
- **GIVEN** `Review` moves to auto-starting `In Progress` on `on_turn_start`,
|
||||
**WHEN** that feedback cycle is analyzed, **THEN** the editor shows no replay
|
||||
warning because the runtime sends the user's prompt without executing
|
||||
`In Progress.on_enter`.
|
||||
- **GIVEN** a cycle contains only `on_turn_complete` transitions but one source
|
||||
step does not auto-start, **WHEN** the cycle is introduced, **THEN** it is a
|
||||
user-mediated warning because a user must start that step's turn.
|
||||
- **GIVEN** any transition in the cycle, including the replay edge, requires
|
||||
approval, **WHEN** the cycle is introduced, **THEN** it is a user-mediated
|
||||
warning rather than a fully automatic block.
|
||||
- **GIVEN** a draft workflow has a user-mediated replay cycle, **WHEN** the user
|
||||
clicks Save and then cancels the confirmation, **THEN** the draft remains and
|
||||
no workflow or step creation request is sent.
|
||||
- **GIVEN** a draft workflow has a user-mediated replay cycle, **WHEN** the user
|
||||
confirms **Create anyway**, **THEN** the existing workflow creation sequence
|
||||
proceeds once.
|
||||
- **GIVEN** an existing workflow already has a replay-cycle diagnostic, **WHEN**
|
||||
the settings page loads, **THEN** the inline alert shows the exact trace and
|
||||
prompt source before the author edits it.
|
||||
- **GIVEN** an existing workflow already has a replay-cycle diagnostic, **WHEN**
|
||||
the author changes only its name or prompt without changing the diagnostic
|
||||
identity, **THEN** the mutation is not reconfirmed and the inline alert
|
||||
remains accurate.
|
||||
- **GIVEN** an existing replay cycle, **WHEN** an edit removes the return path or
|
||||
removes `auto_start_agent`, **THEN** the edit persists without confirmation
|
||||
and the diagnostic disappears after refresh.
|
||||
- **GIVEN** a workflow cycle that never re-enters an auto-start step, **WHEN**
|
||||
the author creates or edits it, **THEN** the editor shows no replay warning and
|
||||
does not block the mutation.
|
||||
- **GIVEN** a re-entered auto-start step has no prompt, **WHEN** its diagnostic
|
||||
renders, **THEN** the prompt source states that the task description will be
|
||||
replayed.
|
||||
- **GIVEN** a re-entered auto-start step prompt contains `{{task_prompt}}`,
|
||||
**WHEN** its diagnostic renders, **THEN** the prompt source states that the
|
||||
rendered step prompt includes the task description.
|
||||
- **GIVEN** the warning is displayed at a mobile viewport, **WHEN** the user
|
||||
reviews and confirms a user-mediated cycle, **THEN** the full trace and both
|
||||
dialog actions are usable without horizontal page scrolling or hover.
|
||||
- **GIVEN** multiple blocking diagnostics exceed a mobile viewport, **WHEN** the
|
||||
user scrolls through them, **THEN** transition labels remain readable and the
|
||||
final explanation can scroll fully above the always-reachable return action.
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Changing workflow runtime transition or auto-start behavior.
|
||||
- Stopping tasks already cycling when the settings page opens.
|
||||
- Rejecting workflow writes made directly through HTTP, WebSocket, YAML import,
|
||||
seed configuration, or another client.
|
||||
- Analyzing generic event-driven transitions such as
|
||||
`on_children_completed`, `on_heartbeat`, or `on_agent_error` in this first
|
||||
iteration.
|
||||
- Warning about cycles that contain no auto-start replay.
|
||||
- Displaying the full task description or fully rendered prompt in settings.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
[Implementation plan](../../plans/workflow-cycle-guardrails/plan.md)
|
||||
Reference in New Issue
Block a user