mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-26 06:04:14 +00:00
Merge pull request #1044 from fabro-sh/codex/restore-run-retry
Some checks are pending
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Rustdoc (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Sandbox providers (Docker) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
Rust / Process titles (musl) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
TypeScript / Build (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Some checks are pending
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Rustdoc (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Sandbox providers (Docker) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
Rust / Process titles (musl) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
TypeScript / Build (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Restore Retry for terminal runs in the web UI
This commit is contained in:
commit
7a876b5444
8 changed files with 229 additions and 4 deletions
64
apps/fabro-web/app/components/retry-run-dialog.tsx
Normal file
64
apps/fabro-web/app/components/retry-run-dialog.tsx
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
import { useRef, useState } from "react";
|
||||
import { useNavigate } from "react-router";
|
||||
import { useSWRConfig } from "swr";
|
||||
|
||||
import { mutateRunListCaches } from "../lib/board-cache";
|
||||
import { queryKeys } from "../lib/query-keys";
|
||||
import { mapError, retryRun } from "../lib/run-actions";
|
||||
import { ConfirmDialog, ErrorMessage } from "./ui";
|
||||
|
||||
export function RetryRunDialog({
|
||||
runId,
|
||||
onClose,
|
||||
}: {
|
||||
runId: string;
|
||||
onClose: () => void;
|
||||
}) {
|
||||
const navigate = useNavigate();
|
||||
const { mutate } = useSWRConfig();
|
||||
const inFlight = useRef(false);
|
||||
const [pending, setPending] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
async function handleRetry() {
|
||||
// Lock synchronously: two clicks can arrive before React renders pending.
|
||||
if (inFlight.current) return;
|
||||
inFlight.current = true;
|
||||
setPending(true);
|
||||
setError(null);
|
||||
try {
|
||||
const run = await retryRun(runId);
|
||||
// Retry returns a different run; never replace the original's detail cache.
|
||||
void mutate(queryKeys.runs.detail(run.id), run, { revalidate: false });
|
||||
mutateRunListCaches(mutate);
|
||||
onClose();
|
||||
void navigate(`/runs/${encodeURIComponent(run.id)}`);
|
||||
} catch (error) {
|
||||
setError(mapError(error, "retry"));
|
||||
} finally {
|
||||
inFlight.current = false;
|
||||
setPending(false);
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<ConfirmDialog
|
||||
open
|
||||
title="Retry this run?"
|
||||
description={
|
||||
<div className="space-y-3">
|
||||
<p>Retry creates a new run from the saved specification and starts from the beginning.</p>
|
||||
{error && <ErrorMessage message={error} />}
|
||||
</div>
|
||||
}
|
||||
confirmLabel="Retry run"
|
||||
pendingLabel="Creating run…"
|
||||
tone="primary"
|
||||
pending={pending}
|
||||
onConfirm={() => void handleRetry()}
|
||||
onCancel={() => {
|
||||
if (!inFlight.current) onClose();
|
||||
}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -11,6 +11,7 @@ import {
|
|||
canArchive,
|
||||
canCancel,
|
||||
canDelete,
|
||||
canRetry,
|
||||
canUnarchive,
|
||||
cancellationActionLabel,
|
||||
cancellationSuccessMessage,
|
||||
|
|
@ -24,6 +25,7 @@ import {
|
|||
} from "../../lib/run-actions";
|
||||
import type { LifecycleAction } from "../../lib/run-actions";
|
||||
import { useToast } from "../toast";
|
||||
import { RetryRunDialog } from "../retry-run-dialog";
|
||||
import { ConfirmDialog } from "../ui";
|
||||
|
||||
const MENU_ITEM_CLASS =
|
||||
|
|
@ -37,12 +39,14 @@ export function RowActionsMenu({ run }: { run: RunWithStatus }) {
|
|||
const { push } = useToast();
|
||||
const [pendingAction, setPendingAction] = useState<LifecycleAction | "delete" | null>(null);
|
||||
const [optimisticallyCancelled, setOptimisticallyCancelled] = useState(false);
|
||||
const [retryDialogOpen, setRetryDialogOpen] = useState(false);
|
||||
const [deleteDialogOpen, setDeleteDialogOpen] = useState(false);
|
||||
const [idCopied, setIdCopied] = useState(false);
|
||||
|
||||
const status = run.lifecycleStatus;
|
||||
const showApprove = run.pendingApproval === true;
|
||||
const showDeny = run.pendingApproval === true;
|
||||
const showRetry = canRetry(status);
|
||||
const showArchive = canArchive(status);
|
||||
const showUnarchive = canUnarchive(status);
|
||||
const showCancel = canCancel(status);
|
||||
|
|
@ -54,7 +58,7 @@ export function RowActionsMenu({ run }: { run: RunWithStatus }) {
|
|||
);
|
||||
const pending = pendingAction !== null || cancellationPending;
|
||||
|
||||
const hasLifecycle = showArchive || showUnarchive;
|
||||
const hasLifecycle = showRetry || showArchive || showUnarchive;
|
||||
const hasDestructive = showDeny || showCancel || showDelete;
|
||||
|
||||
async function runAction<T>(
|
||||
|
|
@ -160,6 +164,18 @@ export function RowActionsMenu({ run }: { run: RunWithStatus }) {
|
|||
</button>
|
||||
</MenuItem>
|
||||
)}
|
||||
{showRetry && (
|
||||
<MenuItem>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setRetryDialogOpen(true)}
|
||||
disabled={pending}
|
||||
className={MENU_ITEM_CLASS}
|
||||
>
|
||||
Retry
|
||||
</button>
|
||||
</MenuItem>
|
||||
)}
|
||||
{showArchive && (
|
||||
<MenuItem>
|
||||
<button
|
||||
|
|
@ -233,6 +249,9 @@ export function RowActionsMenu({ run }: { run: RunWithStatus }) {
|
|||
)}
|
||||
</MenuItems>
|
||||
</Menu>
|
||||
{retryDialogOpen && (
|
||||
<RetryRunDialog runId={run.id} onClose={() => setRetryDialogOpen(false)} />
|
||||
)}
|
||||
<ConfirmDialog
|
||||
open={deleteDialogOpen}
|
||||
title="Delete this run?"
|
||||
|
|
|
|||
|
|
@ -85,6 +85,7 @@ export function ConfirmDialog({
|
|||
confirmLabel,
|
||||
pendingLabel,
|
||||
cancelLabel = "Cancel",
|
||||
tone = "danger",
|
||||
pending = false,
|
||||
onConfirm,
|
||||
onCancel,
|
||||
|
|
@ -95,6 +96,7 @@ export function ConfirmDialog({
|
|||
confirmLabel: string;
|
||||
pendingLabel?: string;
|
||||
cancelLabel?: string;
|
||||
tone?: "primary" | "danger";
|
||||
pending?: boolean;
|
||||
onConfirm: () => void;
|
||||
onCancel: () => void;
|
||||
|
|
@ -126,7 +128,7 @@ export function ConfirmDialog({
|
|||
type="button"
|
||||
onClick={onConfirm}
|
||||
disabled={pending}
|
||||
className={DANGER_BUTTON_CLASS}
|
||||
className={tone === "danger" ? DANGER_BUTTON_CLASS : PRIMARY_BUTTON_CLASS}
|
||||
>
|
||||
{pending ? (pendingLabel ?? `${confirmLabel}…`) : confirmLabel}
|
||||
</button>
|
||||
|
|
|
|||
|
|
@ -11,6 +11,8 @@ import {
|
|||
archiveRun,
|
||||
archiveRuns,
|
||||
canArchive,
|
||||
canRetry,
|
||||
retryRun,
|
||||
canApprove,
|
||||
canCancel,
|
||||
canUnarchive,
|
||||
|
|
@ -460,3 +462,32 @@ describe("run lifecycle actions", () => {
|
|||
expect(cancellationActionLabel(false)).toBe("Cancel");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe("retryRun", () => {
|
||||
test("posts to retry and returns the new run", async () => {
|
||||
const run = { ...makeRun({ kind: "pending" }), id: "new-run" };
|
||||
const { requests } = stubGeneratedAxiosOnce({ status: 201, body: run });
|
||||
expect(await retryRun("run-1")).toEqual(run);
|
||||
expect(requests).toEqual([{ url: "/api/v1/runs/run-1/retry", method: "post", data: undefined }]);
|
||||
});
|
||||
|
||||
test("preserves API errors for the retry dialog", async () => {
|
||||
const errors = [{ status: "422", title: "Invalid run", detail: "Saved environment is unavailable." }];
|
||||
stubGeneratedAxiosOnce({ status: 422, body: { errors } });
|
||||
await expect(retryRun("run-1")).rejects.toEqual({ status: 422, errors });
|
||||
});
|
||||
|
||||
test("eligibility includes all terminal states and excludes archived or unknown states", () => {
|
||||
for (const status of ["succeeded", "failed", "dead"]) expect(canRetry(status)).toBe(true);
|
||||
for (const status of ["archived", "submitted", "pending", "runnable", "starting", "running", "paused", "blocked", "unknown", null, undefined]) {
|
||||
expect(canRetry(status)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test("maps retry failures with actionable fallback messages", () => {
|
||||
expect(mapError({ status: 404, errors: [] }, "retry")).toBe("This run no longer exists.");
|
||||
expect(mapError({ status: 409, errors: [] }, "retry")).toBe("Only terminal, non-archived runs can be retried.");
|
||||
expect(mapError(new Error("offline"), "retry")).toBe("Couldn't retry the run right now. Try again.");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -46,6 +46,18 @@ const TERMINAL_RUN_STATUSES = new Set<RunStatus>([
|
|||
"dead",
|
||||
]);
|
||||
|
||||
export async function retryRun(id: string): Promise<Run> {
|
||||
try {
|
||||
return await apiData(() => runsApi.retryRun(id));
|
||||
} catch (error) {
|
||||
throw lifecycleActionErrorFromError(error);
|
||||
}
|
||||
}
|
||||
|
||||
export function canRetry(status: string | null | undefined): boolean {
|
||||
return isTerminalRunStatus(status);
|
||||
}
|
||||
|
||||
export async function cancelRun(id: string, request?: Request): Promise<Run> {
|
||||
return runLifecycleAction(id, "cancel", request);
|
||||
}
|
||||
|
|
@ -197,13 +209,15 @@ export function deleteErrorMessage(error: unknown): string {
|
|||
return "Couldn't delete the run right now. Try again.";
|
||||
}
|
||||
|
||||
export function mapError(error: unknown, action: LifecycleAction): string {
|
||||
export function mapError(error: unknown, action: LifecycleAction | "retry"): string {
|
||||
if (isLifecycleActionError(error)) {
|
||||
if (error.status === 404) {
|
||||
return "This run no longer exists.";
|
||||
}
|
||||
if (error.status === 409) {
|
||||
switch (action) {
|
||||
case "retry":
|
||||
return "Only terminal, non-archived runs can be retried.";
|
||||
case "cancel":
|
||||
return "This run can no longer be cancelled.";
|
||||
case "approve":
|
||||
|
|
@ -223,6 +237,8 @@ export function mapError(error: unknown, action: LifecycleAction): string {
|
|||
}
|
||||
|
||||
switch (action) {
|
||||
case "retry":
|
||||
return "Couldn't retry the run right now. Try again.";
|
||||
case "cancel":
|
||||
return "Couldn't cancel the run right now. Try again.";
|
||||
case "approve":
|
||||
|
|
|
|||
|
|
@ -20,6 +20,10 @@ let currentRunSummary: any = null;
|
|||
let currentRunState: any = null;
|
||||
let currentQuestions: any[] = [];
|
||||
let currentBoardRuns: any[] = [];
|
||||
let retryRunApiResult: Promise<unknown> | null = null;
|
||||
const retryRunApiMock = mock((_id: string) =>
|
||||
retryRunApiResult ?? Promise.resolve({ data: null }),
|
||||
);
|
||||
let deleteRunApiResult: Promise<unknown> | null = null;
|
||||
const mountedRenderers: TestRenderer.ReactTestRenderer[] = [];
|
||||
|
||||
|
|
@ -110,6 +114,7 @@ mock.module("../lib/api-client", () => ({
|
|||
requestSignalOptions: () => undefined,
|
||||
runsApi: {
|
||||
deleteRun: deleteRunApiMock,
|
||||
retryRun: retryRunApiMock,
|
||||
},
|
||||
ApiError: class ApiError extends Error {
|
||||
readonly status: number;
|
||||
|
|
@ -200,6 +205,8 @@ const {
|
|||
default: RunDetail,
|
||||
resolveDockClearance,
|
||||
} = await import("./run-detail");
|
||||
const { RowActionsMenu } = await import("../components/runs-list/row-actions-menu");
|
||||
const { toRunWithStatus } = await import("../data/runs");
|
||||
const { default: Runs } = await import("./runs");
|
||||
mock.restore();
|
||||
type LifecycleToastState = import("./run-detail/lifecycle-toasts").LifecycleToastState;
|
||||
|
|
@ -346,6 +353,10 @@ async function renderRunDetailHarness({
|
|||
|
||||
const router = createMemoryRouter(
|
||||
[
|
||||
{
|
||||
path: "/row",
|
||||
element: h(RowActionsMenu, { run: toRunWithStatus(currentRunSummary) }),
|
||||
},
|
||||
{
|
||||
path: "/runs",
|
||||
element: h(Runs),
|
||||
|
|
@ -638,7 +649,7 @@ describe("handleLifecycleToastResult", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("RunDetail full-height child routes", () => {
|
||||
describe("RunDetail and run actions", () => {
|
||||
afterEach(() => {
|
||||
act(() => {
|
||||
for (const renderer of mountedRenderers.splice(0)) {
|
||||
|
|
@ -653,6 +664,8 @@ describe("RunDetail full-height child routes", () => {
|
|||
questionQueryMock.mockClear();
|
||||
submitAnswerMock.mockClear();
|
||||
submitAnswerHookMock.mockClear();
|
||||
retryRunApiResult = null;
|
||||
retryRunApiMock.mockClear();
|
||||
deleteRunApiResult = null;
|
||||
deleteRunApiMock.mockClear();
|
||||
mutateRunListCachesMock.mockClear();
|
||||
|
|
@ -660,6 +673,68 @@ describe("RunDetail full-height child routes", () => {
|
|||
delete (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT;
|
||||
});
|
||||
|
||||
for (const surface of ["row", "header"] as const) {
|
||||
const initialEntry = surface === "row" ? "/row" : "/runs/run_1";
|
||||
for (const status of ["submitted", "pending", "runnable", "starting", "running", "paused", "blocked", "succeeded", "failed", "dead", "archived"]) {
|
||||
test(`${surface} offers Retry only for terminal, non-archived runs: ${status}`, async () => {
|
||||
const { renderer } = await renderRunDetailHarness({ initialEntry, status });
|
||||
expect(findButtonsByText(renderer, "Retry")).toHaveLength(
|
||||
["succeeded", "failed", "dead"].includes(status) ? 1 : 0,
|
||||
);
|
||||
if (["succeeded", "failed", "dead"].includes(status)) {
|
||||
expect(findButtonByText(renderer, "Archive")).toBeDefined();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
test(`${surface} retries once while pending and opens the returned new run`, async () => {
|
||||
const response = deferred<unknown>();
|
||||
retryRunApiResult = response.promise;
|
||||
const { renderer, router } = await renderRunDetailHarness({ initialEntry, status: "failed" });
|
||||
await act(async () => { findButtonByText(renderer, "Retry")!.props.onClick(); });
|
||||
expect(textFromNode(renderer.toJSON())).toContain(
|
||||
"Retry creates a new run from the saved specification and starts from the beginning.",
|
||||
);
|
||||
const confirm = findButtonByText(renderer, "Retry run")!;
|
||||
await act(async () => {
|
||||
confirm.props.onClick();
|
||||
confirm.props.onClick();
|
||||
});
|
||||
expect(retryRunApiMock).toHaveBeenCalledTimes(1);
|
||||
expect(retryRunApiMock.mock.calls[0]?.[0]).toBe("run_1");
|
||||
expect(findButtonByText(renderer, "Creating run…")!.props.disabled).toBe(true);
|
||||
expect(findButtonsByText(renderer, "Cancel").at(-1)!.props.disabled).toBe(true);
|
||||
expect(router.state.location.pathname).toBe(initialEntry);
|
||||
const newRun = { ...makeRunSummary({ status: "pending" }), id: "new-run" };
|
||||
await act(async () => { response.resolve({ data: newRun }); });
|
||||
expect(router.state.location.pathname).toBe("/runs/new-run");
|
||||
expect(mutateRunListCachesMock).toHaveBeenCalledTimes(1);
|
||||
expect(swrMutateMock.mock.calls).toContainEqual([
|
||||
["runs", "detail", "new-run"], newRun, { revalidate: false },
|
||||
]);
|
||||
expect(swrMutateMock.mock.calls.some(([key]) =>
|
||||
JSON.stringify(key) === JSON.stringify(["runs", "detail", "run_1"]),
|
||||
)).toBe(false);
|
||||
});
|
||||
|
||||
test(`${surface} shows retry errors, stays on the source run, and allows recovery`, async () => {
|
||||
const response = deferred<unknown>();
|
||||
retryRunApiResult = response.promise;
|
||||
const { renderer, router } = await renderRunDetailHarness({ initialEntry, status: "succeeded" });
|
||||
await act(async () => { findButtonByText(renderer, "Retry")!.props.onClick(); });
|
||||
await act(async () => { findButtonByText(renderer, "Retry run")!.props.onClick(); });
|
||||
await act(async () => { response.reject({ status: 422, errors: [{ detail: "Saved environment is unavailable." }] }); });
|
||||
expect(textFromTestNode(renderer.root.findByProps({ role: "alert" }))).toBe("Saved environment is unavailable.");
|
||||
expect(router.state.location.pathname).toBe(initialEntry);
|
||||
expect(mutateRunListCachesMock).not.toHaveBeenCalled();
|
||||
expect(findButtonByText(renderer, "Retry run")!.props.disabled).toBe(false);
|
||||
retryRunApiResult = Promise.resolve({ data: { ...makeRunSummary(), id: "recovered-run" } });
|
||||
await act(async () => { findButtonByText(renderer, "Retry run")!.props.onClick(); });
|
||||
expect(retryRunApiMock).toHaveBeenCalledTimes(2);
|
||||
expect(router.state.location.pathname).toBe("/runs/recovered-run");
|
||||
});
|
||||
}
|
||||
|
||||
test("Answer Question on a blocked board card opens that run's pending interview", async () => {
|
||||
currentBoardRuns = [
|
||||
{ ...makeRunSummary(), id: "other-run" },
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import {
|
|||
import { AgentSessionActivity } from "@qltysh/fabro-api-client";
|
||||
|
||||
import { type SteerBarHandle } from "../components/steer-bar";
|
||||
import { RetryRunDialog } from "../components/retry-run-dialog";
|
||||
import { ErrorState } from "../components/state";
|
||||
import { useToast } from "../components/toast";
|
||||
import {
|
||||
|
|
@ -114,6 +115,7 @@ export default function RunDetail({ params }: { params: { id: string } }) {
|
|||
const interruptMutation = useInterruptRun(params.id);
|
||||
const navigate = useNavigate();
|
||||
const { mutate } = useSWRConfig();
|
||||
const [retryDialogOpen, setRetryDialogOpen] = useState(false);
|
||||
const [deleteDialogOpen, setDeleteDialogOpen] = useState(false);
|
||||
const [deletePending, setDeletePending] = useState(false);
|
||||
const [dockMeasurement, setDockMeasurement] =
|
||||
|
|
@ -279,6 +281,13 @@ export default function RunDetail({ params }: { params: { id: string } }) {
|
|||
},
|
||||
],
|
||||
lifecycle: [
|
||||
...(visibility.showRetry
|
||||
? [{
|
||||
key: "retry",
|
||||
label: "Retry",
|
||||
onSelect: () => setRetryDialogOpen(true),
|
||||
}]
|
||||
: []),
|
||||
...(visibility.showArchive
|
||||
? [{
|
||||
key: "archive",
|
||||
|
|
@ -373,6 +382,13 @@ export default function RunDetail({ params }: { params: { id: string } }) {
|
|||
}}
|
||||
/>
|
||||
|
||||
{retryDialogOpen && (
|
||||
<RetryRunDialog
|
||||
key={params.id}
|
||||
runId={params.id}
|
||||
onClose={() => setRetryDialogOpen(false)}
|
||||
/>
|
||||
)}
|
||||
<ConfirmDialog
|
||||
open={deleteDialogOpen}
|
||||
title="Delete this run?"
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import {
|
|||
canArchive,
|
||||
canCancel,
|
||||
canDelete,
|
||||
canRetry,
|
||||
canUnarchive,
|
||||
cancellationSuccessMessage,
|
||||
mapError,
|
||||
|
|
@ -55,6 +56,7 @@ export function updateLifecycleToastState(
|
|||
export function lifecycleActionVisibility(status: string | null | undefined) {
|
||||
return {
|
||||
showPrimaryCancel: canCancel(status),
|
||||
showRetry: canRetry(status),
|
||||
showArchive: canArchive(status),
|
||||
showUnarchive: canUnarchive(status),
|
||||
showDelete: canDelete(status),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue