fix(office): routine create/save persists every field and can arm a cron schedule (#3759)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -0,0 +1,91 @@
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { TooltipProvider } from "@kandev/ui/tooltip";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { defaultOfficeState } from "@/lib/state/slices/office/office-slice";
|
||||
import type { Routine } from "@/lib/state/slices/office/types";
|
||||
import { CoordinatorRoutineHint } from "./layout";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
const WORKSPACE_ID = "ws-1";
|
||||
const AGENT_ID = "agent-1";
|
||||
const TIMESTAMP = "2026-05-04T00:00:00Z";
|
||||
const HINT_LABEL = "No scheduled wake-ups; manage routines";
|
||||
|
||||
function baseRoutine(overrides: Partial<Routine> = {}): Routine {
|
||||
return {
|
||||
id: "routine-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
name: "Daily standup",
|
||||
taskTemplate: {},
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
assigneeAgentProfileId: AGENT_ID,
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function renderHint(routines: Routine[], agentRole = "ceo") {
|
||||
return render(
|
||||
<StateProvider
|
||||
initialState={{
|
||||
workspaces: { activeId: WORKSPACE_ID, items: [] },
|
||||
office: { ...defaultOfficeState.office, routines },
|
||||
}}
|
||||
>
|
||||
<TooltipProvider>
|
||||
<CoordinatorRoutineHint agentId={AGENT_ID} agentRole={agentRole} />
|
||||
</TooltipProvider>
|
||||
</StateProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("CoordinatorRoutineHint", () => {
|
||||
it("shows no hint for a coordinator with a routine whose status is active", () => {
|
||||
renderHint([baseRoutine({ status: "active" })]);
|
||||
expect(screen.queryByLabelText(HINT_LABEL)).toBeNull();
|
||||
});
|
||||
|
||||
it("shows no hint for a coordinator with a routine whose status is the empty string (no writer set one)", () => {
|
||||
renderHint([baseRoutine({ status: "" })]);
|
||||
expect(screen.queryByLabelText(HINT_LABEL)).toBeNull();
|
||||
});
|
||||
|
||||
it("shows the hint for a coordinator with a paused routine", () => {
|
||||
renderHint([baseRoutine({ status: "paused" })]);
|
||||
expect(screen.getByLabelText(HINT_LABEL)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows the hint for a coordinator with no routines at all", () => {
|
||||
renderHint([]);
|
||||
expect(screen.getByLabelText(HINT_LABEL)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows the hint when the only routine targeting this agent is archived", () => {
|
||||
renderHint([baseRoutine({ status: "archived" })]);
|
||||
expect(screen.getByLabelText(HINT_LABEL)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows no hint when a routine is active but a different one targeting the agent is paused", () => {
|
||||
renderHint([
|
||||
baseRoutine({ id: "routine-1", status: "paused" }),
|
||||
baseRoutine({ id: "routine-2", status: "active" }),
|
||||
]);
|
||||
expect(screen.queryByLabelText(HINT_LABEL)).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores an active routine assigned to a different agent", () => {
|
||||
renderHint([baseRoutine({ assigneeAgentProfileId: "agent-2", status: "active" })]);
|
||||
expect(screen.getByLabelText(HINT_LABEL)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("renders nothing for a non-coordinator role, regardless of routines", () => {
|
||||
renderHint([], "worker");
|
||||
expect(screen.queryByLabelText(HINT_LABEL)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,7 @@ import { AgentRoleBadge } from "../components/agent-role-badge";
|
||||
import { BudgetGauge } from "../components/budget-gauge";
|
||||
import { AgentRouteStrip } from "./components/agent-route-strip";
|
||||
import { AgentRecoveryControl } from "./components/agent-recovery-control";
|
||||
import { isRoutineFiring } from "../../lib/routine-status";
|
||||
import { Trans, useTranslation } from "react-i18next";
|
||||
|
||||
type AgentDetailLayoutProps = {
|
||||
@@ -137,12 +138,18 @@ function activeSlugFromPath(pathname: string | null, agentId: string): string {
|
||||
* navigates to /office/routines to install one. Workers / specialists
|
||||
* don't get this hint since they only run on assignment, not schedule.
|
||||
*/
|
||||
function CoordinatorRoutineHint({ agentId, agentRole }: { agentId: string; agentRole: string }) {
|
||||
export function CoordinatorRoutineHint({
|
||||
agentId,
|
||||
agentRole,
|
||||
}: {
|
||||
agentId: string;
|
||||
agentRole: string;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
const routines = useAppStore((s) => s.office.routines);
|
||||
if (agentRole !== "ceo") return null;
|
||||
const hasActive = routines.some(
|
||||
(r) => r.assigneeAgentProfileId === agentId && r.status === "active",
|
||||
(r) => r.assigneeAgentProfileId === agentId && isRoutineFiring(r.status),
|
||||
);
|
||||
if (hasActive) return null;
|
||||
return (
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { coerceCatchUpMax } from "./catch-up-max";
|
||||
|
||||
describe("coerceCatchUpMax", () => {
|
||||
it("passes through a valid positive number", () => {
|
||||
expect(coerceCatchUpMax(10)).toBe(10);
|
||||
});
|
||||
|
||||
it("passes through a valid numeric string", () => {
|
||||
expect(coerceCatchUpMax("10")).toBe(10);
|
||||
});
|
||||
|
||||
it("coerces zero to 25", () => {
|
||||
expect(coerceCatchUpMax(0)).toBe(25);
|
||||
});
|
||||
|
||||
it("coerces a negative number to 25", () => {
|
||||
expect(coerceCatchUpMax(-5)).toBe(25);
|
||||
});
|
||||
|
||||
it("coerces an unparsable string to 25", () => {
|
||||
expect(coerceCatchUpMax("abc")).toBe(25);
|
||||
});
|
||||
|
||||
it("coerces undefined to 25", () => {
|
||||
expect(coerceCatchUpMax(undefined)).toBe(25);
|
||||
});
|
||||
|
||||
it("coerces an empty string to 25", () => {
|
||||
expect(coerceCatchUpMax("")).toBe(25);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,10 @@
|
||||
/**
|
||||
* Coerces a routine catch-up max to a valid value for display or form submission.
|
||||
* Any value below 1 (including NaN from an unparsable string) becomes the
|
||||
* default of 25, mirroring the backend's own normalization
|
||||
* (apps/backend/internal/office/models: NormaliseCatchUpMax).
|
||||
*/
|
||||
export function coerceCatchUpMax(value: number | string | undefined): number {
|
||||
const numeric = typeof value === "number" ? value : Number(value);
|
||||
return Number.isFinite(numeric) && numeric >= 1 ? numeric : 25;
|
||||
}
|
||||
@@ -0,0 +1,221 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import type { RoutineTrigger } from "@/lib/state/slices/office/types";
|
||||
import { reconcileCronTrigger } from "./cron-reconcile";
|
||||
import {
|
||||
createRoutineTrigger,
|
||||
deleteRoutineTrigger,
|
||||
listRoutineTriggers,
|
||||
} from "@/lib/api/domains/office-api";
|
||||
|
||||
vi.mock("@/lib/api/domains/office-api", () => ({
|
||||
createRoutineTrigger: vi.fn(),
|
||||
deleteRoutineTrigger: vi.fn(),
|
||||
listRoutineTriggers: vi.fn(),
|
||||
}));
|
||||
|
||||
const createRoutineTriggerMock = vi.mocked(createRoutineTrigger);
|
||||
const deleteRoutineTriggerMock = vi.mocked(deleteRoutineTrigger);
|
||||
const listRoutineTriggersMock = vi.mocked(listRoutineTriggers);
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
const TIMESTAMP = "2026-05-04T00:00:00Z";
|
||||
const CRON_EXPR = "*/5 * * * *";
|
||||
const NEW_TRIGGER_ID = "new-trigger";
|
||||
const DELETE_REJECTED_MESSAGE = "delete rejected";
|
||||
|
||||
function makeTrigger(overrides: Partial<RoutineTrigger> = {}): RoutineTrigger {
|
||||
return {
|
||||
id: "trigger-1",
|
||||
routineId: "routine-1",
|
||||
kind: "cron",
|
||||
cronExpression: CRON_EXPR,
|
||||
timezone: "UTC",
|
||||
enabled: true,
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("reconcileCronTrigger: no-op branches", () => {
|
||||
it("does nothing for a webhook draft", async () => {
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "webhook", cronExpression: "", timezone: "UTC" },
|
||||
[],
|
||||
);
|
||||
expect(outcome).toEqual({ kind: "unchanged" });
|
||||
expect(createRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does nothing when the cron expression is empty after trimming", async () => {
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: " ", timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
expect(outcome).toEqual({ kind: "unchanged" });
|
||||
expect(createRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
expect(deleteRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does nothing when the expression and effective timezone are unchanged", async () => {
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: CRON_EXPR, timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
expect(outcome).toEqual({ kind: "unchanged" });
|
||||
expect(createRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("treats an absent stored timezone and an explicit UTC draft as the same effective timezone", async () => {
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: CRON_EXPR, timezone: "UTC" },
|
||||
[makeTrigger({ timezone: undefined })],
|
||||
);
|
||||
expect(outcome).toEqual({ kind: "unchanged" });
|
||||
});
|
||||
|
||||
it("treats a stored UTC timezone and an unset draft timezone as the same effective timezone", async () => {
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: CRON_EXPR, timezone: "" },
|
||||
[makeTrigger({ timezone: "UTC" })],
|
||||
);
|
||||
expect(outcome).toEqual({ kind: "unchanged" });
|
||||
expect(createRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("reconcileCronTrigger: create/delete orchestration", () => {
|
||||
it("creates a trigger and skips delete when the routine has no cron trigger yet", async () => {
|
||||
createRoutineTriggerMock.mockResolvedValue(makeTrigger({ id: NEW_TRIGGER_ID }));
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [makeTrigger({ id: NEW_TRIGGER_ID })] });
|
||||
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: CRON_EXPR, timezone: "UTC" },
|
||||
[],
|
||||
);
|
||||
|
||||
expect(createRoutineTriggerMock).toHaveBeenCalledWith("routine-1", {
|
||||
kind: "cron",
|
||||
cronExpression: CRON_EXPR,
|
||||
timezone: "UTC",
|
||||
});
|
||||
expect(deleteRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
expect(outcome).toEqual({ kind: "success", triggers: [makeTrigger({ id: NEW_TRIGGER_ID })] });
|
||||
});
|
||||
|
||||
it("creates the replacement before deleting the existing trigger, then re-lists", async () => {
|
||||
const calls: string[] = [];
|
||||
createRoutineTriggerMock.mockImplementation(async () => {
|
||||
calls.push("create");
|
||||
return makeTrigger({ id: NEW_TRIGGER_ID });
|
||||
});
|
||||
deleteRoutineTriggerMock.mockImplementation(async () => {
|
||||
calls.push("delete");
|
||||
});
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [makeTrigger({ id: NEW_TRIGGER_ID })] });
|
||||
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: "0 * * * *", timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
|
||||
expect(calls).toEqual(["create", "delete"]);
|
||||
expect(deleteRoutineTriggerMock).toHaveBeenCalledWith("trigger-1");
|
||||
expect(outcome).toEqual({ kind: "success", triggers: [makeTrigger({ id: NEW_TRIGGER_ID })] });
|
||||
});
|
||||
|
||||
it("leaves the existing trigger in place and issues no delete when create is rejected", async () => {
|
||||
createRoutineTriggerMock.mockRejectedValue(new Error("bad cron"));
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [makeTrigger()] });
|
||||
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: "bad", timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
|
||||
expect(deleteRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
expect(outcome).toEqual({
|
||||
kind: "create-failed",
|
||||
message: "bad cron",
|
||||
triggers: [makeTrigger()],
|
||||
});
|
||||
});
|
||||
|
||||
it("reports create-failed with unknown triggers when the re-list after a create failure also fails", async () => {
|
||||
createRoutineTriggerMock.mockRejectedValue(new Error("bad cron"));
|
||||
listRoutineTriggersMock.mockRejectedValue(new Error("network down"));
|
||||
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: "bad", timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
|
||||
expect(outcome).toEqual({ kind: "create-failed", message: "bad cron", triggers: null });
|
||||
});
|
||||
});
|
||||
|
||||
describe("reconcileCronTrigger: delete-failure and stale re-list outcomes", () => {
|
||||
it("reports delete-failed with the re-listed triggers when delete fails after a successful create", async () => {
|
||||
createRoutineTriggerMock.mockResolvedValue(makeTrigger({ id: NEW_TRIGGER_ID }));
|
||||
deleteRoutineTriggerMock.mockRejectedValue(new Error(DELETE_REJECTED_MESSAGE));
|
||||
listRoutineTriggersMock.mockResolvedValue({
|
||||
triggers: [makeTrigger(), makeTrigger({ id: NEW_TRIGGER_ID })],
|
||||
});
|
||||
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: "0 * * * *", timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
|
||||
expect(outcome).toEqual({
|
||||
kind: "delete-failed",
|
||||
message: DELETE_REJECTED_MESSAGE,
|
||||
triggers: [makeTrigger(), makeTrigger({ id: NEW_TRIGGER_ID })],
|
||||
});
|
||||
});
|
||||
|
||||
it("reports delete-failed with unknown triggers when the re-list after a delete failure also fails", async () => {
|
||||
createRoutineTriggerMock.mockResolvedValue(makeTrigger({ id: NEW_TRIGGER_ID }));
|
||||
deleteRoutineTriggerMock.mockRejectedValue(new Error(DELETE_REJECTED_MESSAGE));
|
||||
listRoutineTriggersMock.mockRejectedValue(new Error("network down"));
|
||||
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: "0 * * * *", timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
|
||||
expect(outcome).toEqual({
|
||||
kind: "delete-failed",
|
||||
message: DELETE_REJECTED_MESSAGE,
|
||||
triggers: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("reports success with null triggers (stale) when the re-list fails after create and delete both succeed", async () => {
|
||||
createRoutineTriggerMock.mockResolvedValue(makeTrigger({ id: NEW_TRIGGER_ID }));
|
||||
deleteRoutineTriggerMock.mockResolvedValue(undefined);
|
||||
listRoutineTriggersMock.mockRejectedValue(new Error("network down"));
|
||||
|
||||
const outcome = await reconcileCronTrigger(
|
||||
"routine-1",
|
||||
{ triggerKind: "cron", cronExpression: "0 * * * *", timezone: "UTC" },
|
||||
[makeTrigger()],
|
||||
);
|
||||
|
||||
expect(outcome).toEqual({ kind: "success", triggers: null });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,92 @@
|
||||
import {
|
||||
createRoutineTrigger,
|
||||
deleteRoutineTrigger,
|
||||
listRoutineTriggers,
|
||||
} from "@/lib/api/domains/office-api";
|
||||
import type { RoutineTrigger } from "@/lib/state/slices/office/types";
|
||||
|
||||
export type CronDraft = {
|
||||
triggerKind: string;
|
||||
cronExpression: string;
|
||||
timezone: string;
|
||||
};
|
||||
|
||||
export type CronReconcileOutcome =
|
||||
| { kind: "unchanged" }
|
||||
| { kind: "success"; triggers: RoutineTrigger[] | null }
|
||||
| { kind: "create-failed"; message: string; triggers: RoutineTrigger[] | null }
|
||||
| { kind: "delete-failed"; message: string; triggers: RoutineTrigger[] | null };
|
||||
|
||||
function effectiveTimezone(timezone: string | undefined): string {
|
||||
const trimmed = timezone?.trim();
|
||||
return trimmed ? trimmed : "UTC";
|
||||
}
|
||||
|
||||
function cronMatches(existing: RoutineTrigger, cronExpression: string, timezone: string): boolean {
|
||||
return (
|
||||
existing.cronExpression === cronExpression &&
|
||||
effectiveTimezone(existing.timezone) === effectiveTimezone(timezone)
|
||||
);
|
||||
}
|
||||
|
||||
async function relistOrNull(routineId: string): Promise<RoutineTrigger[] | null> {
|
||||
try {
|
||||
const res = await listRoutineTriggers(routineId);
|
||||
return res.triggers;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function errorMessage(err: unknown): string {
|
||||
return err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reconciles the routine's cron trigger with the draft's expression, timezone,
|
||||
* and kind. Creates the replacement trigger before deleting the old one, and
|
||||
* always re-lists from the server afterward rather than splicing the local
|
||||
* array, per docs/specs/office/system-design/routine-wire-contract.md.
|
||||
*/
|
||||
export async function reconcileCronTrigger(
|
||||
routineId: string,
|
||||
draft: CronDraft,
|
||||
triggers: RoutineTrigger[],
|
||||
): Promise<CronReconcileOutcome> {
|
||||
if (draft.triggerKind !== "cron") return { kind: "unchanged" };
|
||||
const cronExpression = draft.cronExpression.trim();
|
||||
if (!cronExpression) return { kind: "unchanged" };
|
||||
|
||||
const existing = triggers.find((t) => t.kind === "cron") ?? null;
|
||||
if (existing && cronMatches(existing, cronExpression, draft.timezone)) {
|
||||
return { kind: "unchanged" };
|
||||
}
|
||||
|
||||
try {
|
||||
await createRoutineTrigger(routineId, {
|
||||
kind: "cron",
|
||||
cronExpression,
|
||||
timezone: draft.timezone,
|
||||
});
|
||||
} catch (err) {
|
||||
return {
|
||||
kind: "create-failed",
|
||||
message: errorMessage(err),
|
||||
triggers: await relistOrNull(routineId),
|
||||
};
|
||||
}
|
||||
|
||||
if (existing) {
|
||||
try {
|
||||
await deleteRoutineTrigger(existing.id);
|
||||
} catch (err) {
|
||||
return {
|
||||
kind: "delete-failed",
|
||||
message: errorMessage(err),
|
||||
triggers: await relistOrNull(routineId),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return { kind: "success", triggers: await relistOrNull(routineId) };
|
||||
}
|
||||
@@ -16,15 +16,17 @@ export default async function RoutineDetailPage({ params }: Props) {
|
||||
let routine: Routine | null = null;
|
||||
let triggers: RoutineTrigger[] = [];
|
||||
try {
|
||||
// A failed trigger list must not silently become "no trigger exists":
|
||||
// the client's save-time reconciliation (cron-reconcile.ts) trusts this
|
||||
// initial list to detect an already-armed cron trigger, so swallowing a
|
||||
// failure here would make a later Save create a duplicate schedule
|
||||
// instead of replacing the one that failed to load. Let it fail the
|
||||
// whole page load instead, same as a `getRoutine` failure below.
|
||||
const [routineRes, triggersRes] = await Promise.all([
|
||||
getRoutine(id, { cache: "no-store" }),
|
||||
listRoutineTriggers(id, { cache: "no-store" }).catch(() => ({
|
||||
triggers: [] as RoutineTrigger[],
|
||||
})),
|
||||
listRoutineTriggers(id, { cache: "no-store" }),
|
||||
]);
|
||||
routine =
|
||||
(routineRes as unknown as { routine?: Routine }).routine ??
|
||||
(routineRes as unknown as Routine);
|
||||
routine = routineRes;
|
||||
triggers = triggersRes.triggers ?? [];
|
||||
} catch {
|
||||
routine = null;
|
||||
|
||||
@@ -1,14 +1,23 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, fireEvent, render, screen, within } from "@testing-library/react";
|
||||
import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { defaultOfficeState } from "@/lib/state/slices/office/office-slice";
|
||||
import type { Routine, RoutineTrigger } from "@/lib/state/slices/office/types";
|
||||
import { RoutineDetailView } from "./routine-detail-view";
|
||||
import { reconcileCronTrigger } from "./cron-reconcile";
|
||||
import {
|
||||
OfficeTopbarChromeProvider,
|
||||
useOfficeTopbarChrome,
|
||||
} from "../../components/office-topbar-context";
|
||||
|
||||
const refreshMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@/lib/routing/client-router", () => ({
|
||||
useRouter: () => ({ refresh: vi.fn(), push: vi.fn(), replace: vi.fn() }),
|
||||
useRouter: () => ({ refresh: refreshMock, push: vi.fn(), replace: vi.fn() }),
|
||||
}));
|
||||
|
||||
vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
|
||||
|
||||
vi.mock("@/lib/api/domains/office-api", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/lib/api/domains/office-api")>(
|
||||
"@/lib/api/domains/office-api",
|
||||
@@ -20,6 +29,18 @@ vi.mock("@/lib/api/domains/office-api", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("./cron-reconcile", async () => {
|
||||
const actual = await vi.importActual<typeof import("./cron-reconcile")>("./cron-reconcile");
|
||||
return {
|
||||
...actual,
|
||||
reconcileCronTrigger: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
import { toast } from "sonner";
|
||||
|
||||
const reconcileCronTriggerMock = vi.mocked(reconcileCronTrigger);
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
@@ -51,8 +72,29 @@ const cronTrigger: RoutineTrigger = {
|
||||
updatedAt: TIMESTAMP,
|
||||
};
|
||||
|
||||
const relistedCronTrigger: RoutineTrigger = {
|
||||
id: "trigger-2",
|
||||
routineId: "routine-1",
|
||||
kind: "cron",
|
||||
cronExpression: "*/10 * * * *",
|
||||
timezone: "UTC",
|
||||
nextRunAt: "2026-06-06T00:00:00Z",
|
||||
enabled: true,
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
};
|
||||
|
||||
const NO_TRIGGERS: RoutineTrigger[] = [];
|
||||
|
||||
// Save/Run Now are contributed through useOfficeTopbar's actions slot (a
|
||||
// context the office shell's topbar renders elsewhere), not RoutineDetailView's
|
||||
// own return tree, so a direct render needs the provider plus a consumer that
|
||||
// actually displays the slot for the buttons to be reachable at all.
|
||||
function TopbarActions() {
|
||||
const chrome = useOfficeTopbarChrome();
|
||||
return <>{chrome?.actions}</>;
|
||||
}
|
||||
|
||||
function renderDetailView(routine: Routine, triggers: RoutineTrigger[]) {
|
||||
return render(
|
||||
<StateProvider
|
||||
@@ -61,7 +103,10 @@ function renderDetailView(routine: Routine, triggers: RoutineTrigger[]) {
|
||||
office: { ...defaultOfficeState.office },
|
||||
}}
|
||||
>
|
||||
<RoutineDetailView initialRoutine={routine} initialTriggers={triggers} />
|
||||
<OfficeTopbarChromeProvider>
|
||||
<RoutineDetailView initialRoutine={routine} initialTriggers={triggers} />
|
||||
<TopbarActions />
|
||||
</OfficeTopbarChromeProvider>
|
||||
</StateProvider>,
|
||||
);
|
||||
}
|
||||
@@ -90,6 +135,12 @@ describe("RoutineDetailView next-fire display", () => {
|
||||
renderDetailView({ ...baseRoutine, status: "paused" }, [cronTrigger]);
|
||||
expect(screen.getByText("Next fire: -")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows no next-fire countdown for a routine loaded with an unrecognized status", () => {
|
||||
renderDetailView({ ...baseRoutine, status: "draft" }, [cronTrigger]);
|
||||
expect(screen.getByText("Next fire: -")).toBeTruthy();
|
||||
expect(screen.queryByText(/Next fire: 5\/5\/2026/)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// AC-OFFICE-ROUTINE-CATCHUP-003.8: the catch_up_max control renders under
|
||||
@@ -143,3 +194,252 @@ describe("RoutineDetailView catch-up policy labeling (AC-003.6)", () => {
|
||||
expect(option.textContent).toMatch(/once|single/i);
|
||||
});
|
||||
});
|
||||
|
||||
describe("RoutineDetailView catch-up max editing", () => {
|
||||
it("keeps the raw value while editing and coerces it at save time", async () => {
|
||||
const { updateRoutine } = await import("@/lib/api/domains/office-api");
|
||||
reconcileCronTriggerMock.mockResolvedValue({ kind: "unchanged" });
|
||||
renderDetailView(
|
||||
{ ...baseRoutine, catchUpPolicy: "summarize_missed", catchUpMax: 25 },
|
||||
NO_TRIGGERS,
|
||||
);
|
||||
|
||||
const input = screen.getByRole("spinbutton") as HTMLInputElement;
|
||||
fireEvent.change(input, { target: { value: "" } });
|
||||
expect(input.value).toBe("");
|
||||
fireEvent.change(input, { target: { value: "7" } });
|
||||
expect(input.value).toBe("7");
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /save/i }));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(updateRoutine).toHaveBeenCalledWith(
|
||||
"routine-1",
|
||||
expect.objectContaining({ catchUpMax: 7 }),
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// AC-003.17: a stored status outside the three selectable options renders no
|
||||
// selection (the control does not invent a value) and a save omits the field
|
||||
// rather than sending an invented one.
|
||||
describe("RoutineDetailView draft status normalization (AC-003.17)", () => {
|
||||
it("shows no selected status option when the stored status is not one of the three known values", () => {
|
||||
renderDetailView({ ...baseRoutine, status: "draft" }, NO_TRIGGERS);
|
||||
const statusField = screen.getByText("Status").closest("div") as HTMLElement;
|
||||
const statusCombobox = within(statusField).getByRole("combobox");
|
||||
expect(statusCombobox.textContent).toBe("");
|
||||
});
|
||||
|
||||
it("keeps the known status selected when it is one of the three options", () => {
|
||||
renderDetailView({ ...baseRoutine, status: "paused" }, NO_TRIGGERS);
|
||||
const statusField = screen.getByText("Status").closest("div") as HTMLElement;
|
||||
expect(within(statusField).getByText("Paused")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("omits status from the save patch when the stored status was unrecognized", async () => {
|
||||
const { updateRoutine } = await import("@/lib/api/domains/office-api");
|
||||
reconcileCronTriggerMock.mockResolvedValue({ kind: "unchanged" });
|
||||
renderDetailView({ ...baseRoutine, status: "draft" }, NO_TRIGGERS);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /save/i }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(updateRoutine).toHaveBeenCalledWith(
|
||||
"routine-1",
|
||||
expect.objectContaining({ status: undefined }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// TS-005: a rejected updateRoutine must not fall through into a cron
|
||||
// reconcile attempt against a routine whose own field save just failed, and
|
||||
// must re-enable the Save button so the operator can retry.
|
||||
describe("RoutineDetailView save failure (TS-005)", () => {
|
||||
it("shows the server error and does not attempt a cron reconcile when updateRoutine rejects", async () => {
|
||||
const { updateRoutine } = await import("@/lib/api/domains/office-api");
|
||||
vi.mocked(updateRoutine).mockRejectedValueOnce(new Error("name already exists"));
|
||||
renderDetailView(baseRoutine, NO_TRIGGERS);
|
||||
|
||||
const saveButton = screen.getByRole("button", { name: /save/i });
|
||||
fireEvent.click(saveButton);
|
||||
|
||||
await waitFor(() => expect(toast.error).toHaveBeenCalledWith("name already exists"));
|
||||
expect(reconcileCronTriggerMock).not.toHaveBeenCalled();
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
expect((saveButton as HTMLButtonElement).disabled).toBe(false);
|
||||
expect(screen.getByRole("button", { name: /save/i }).textContent).not.toMatch(/saving/i);
|
||||
});
|
||||
|
||||
it("shows an error toast and re-enables Save if reconcileCronTrigger throws instead of resolving with a failure outcome", async () => {
|
||||
// reconcileCronTrigger is designed to always resolve (every internal call
|
||||
// is its own try/catch), but nothing in its type enforces that. Save must
|
||||
// still catch it, toast, and clear the saving flag if a future change
|
||||
// breaks that invariant, rather than leaving an unhandled rejection and a
|
||||
// permanently-disabled button.
|
||||
reconcileCronTriggerMock.mockRejectedValueOnce(new Error("unexpected"));
|
||||
renderDetailView(baseRoutine, NO_TRIGGERS);
|
||||
|
||||
const saveButton = screen.getByRole("button", { name: /save/i });
|
||||
fireEvent.click(saveButton);
|
||||
|
||||
await waitFor(() => expect(toast.error).toHaveBeenCalledWith("unexpected"));
|
||||
expect((screen.getByRole("button", { name: /save/i }) as HTMLButtonElement).disabled).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// AC-002.9 / SRF-34: describeCronOutcome (through handleSave) must map every
|
||||
// CronReconcileOutcome branch to the correct toast kind, message, and
|
||||
// refresh decision — including the SRF-34 uncovered cell (a failed trigger
|
||||
// mutation whose re-list also fails), which must show the fate-unknown
|
||||
// message rather than AC-002.9's "saved but schedule was not" text.
|
||||
describe("RoutineDetailView stale schedule state", () => {
|
||||
it("blocks another save after a successful schedule mutation has an unknown trigger state", async () => {
|
||||
const { updateRoutine } = await import("@/lib/api/domains/office-api");
|
||||
reconcileCronTriggerMock.mockResolvedValueOnce({ kind: "success", triggers: null });
|
||||
renderDetailView(baseRoutine, [cronTrigger]);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /save/i }));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.success).toHaveBeenCalledWith(
|
||||
"The routine was saved. The displayed schedule may be stale until you reload the page.",
|
||||
),
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /save/i }));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
"The routine was saved, but the schedule's fate is unknown. Reload the page to see the current state.",
|
||||
),
|
||||
);
|
||||
expect(reconcileCronTriggerMock).toHaveBeenCalledTimes(1);
|
||||
expect(updateRoutine).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("RoutineDetailView save outcome mapping (AC-002.9, SRF-34)", () => {
|
||||
function clickSave() {
|
||||
fireEvent.click(screen.getByRole("button", { name: /save/i }));
|
||||
}
|
||||
|
||||
it("shows the plain saved toast and refreshes when the cron draft is unchanged", async () => {
|
||||
reconcileCronTriggerMock.mockResolvedValue({ kind: "unchanged" });
|
||||
renderDetailView(baseRoutine, NO_TRIGGERS);
|
||||
|
||||
clickSave();
|
||||
|
||||
await waitFor(() => expect(toast.success).toHaveBeenCalledWith("Routine saved"));
|
||||
expect(toast.error).not.toHaveBeenCalled();
|
||||
expect(refreshMock).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows the plain saved toast and refreshes on a successful reconcile with a fresh trigger list", async () => {
|
||||
reconcileCronTriggerMock.mockResolvedValue({ kind: "success", triggers: [] });
|
||||
renderDetailView(baseRoutine, NO_TRIGGERS);
|
||||
|
||||
clickSave();
|
||||
|
||||
await waitFor(() => expect(toast.success).toHaveBeenCalledWith("Routine saved"));
|
||||
expect(refreshMock).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows the stale-schedule toast and does not refresh when a successful reconcile's re-list fails", async () => {
|
||||
reconcileCronTriggerMock.mockResolvedValue({ kind: "success", triggers: null });
|
||||
renderDetailView(baseRoutine, NO_TRIGGERS);
|
||||
|
||||
clickSave();
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.success).toHaveBeenCalledWith(
|
||||
"The routine was saved. The displayed schedule may be stale until you reload the page.",
|
||||
),
|
||||
);
|
||||
expect(refreshMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows the fate-unknown toast, not AC-002.9's saved-but-not text, when a failed create's re-list also fails (SRF-34)", async () => {
|
||||
reconcileCronTriggerMock.mockResolvedValue({
|
||||
kind: "create-failed",
|
||||
message: "invalid cron expression",
|
||||
triggers: null,
|
||||
});
|
||||
renderDetailView(baseRoutine, NO_TRIGGERS);
|
||||
|
||||
clickSave();
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
"The routine was saved, but the schedule's fate is unknown. Reload the page to see the current state.",
|
||||
),
|
||||
);
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
expect(refreshMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows the fate-unknown toast when a failed delete's re-list also fails (SRF-34)", async () => {
|
||||
reconcileCronTriggerMock.mockResolvedValue({
|
||||
kind: "delete-failed",
|
||||
message: "server error",
|
||||
triggers: null,
|
||||
});
|
||||
renderDetailView(baseRoutine, NO_TRIGGERS);
|
||||
|
||||
clickSave();
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
"The routine was saved, but the schedule's fate is unknown. Reload the page to see the current state.",
|
||||
),
|
||||
);
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("names the create failure, does not refresh, and renders the re-listed schedule when the re-list succeeds", async () => {
|
||||
reconcileCronTriggerMock.mockResolvedValue({
|
||||
kind: "create-failed",
|
||||
message: "invalid cron expression",
|
||||
triggers: [relistedCronTrigger],
|
||||
});
|
||||
renderDetailView(baseRoutine, [cronTrigger]);
|
||||
|
||||
clickSave();
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
"The routine was saved, but the schedule change failed: invalid cron expression",
|
||||
),
|
||||
);
|
||||
expect(refreshMock).not.toHaveBeenCalled();
|
||||
// Proves setTriggers(outcome.triggers) actually reaches the DOM: the
|
||||
// Schedule card must show the re-listed trigger's next fire, not the
|
||||
// pre-save trigger it replaced.
|
||||
expect(screen.getByText(/Next fire: 6\/6\/2026/)).toBeTruthy();
|
||||
expect(screen.queryByText(/Next fire: 5\/5\/2026/)).toBeNull();
|
||||
});
|
||||
|
||||
it("names the delete failure, the surviving cron count, and renders the re-listed schedule when the re-list succeeds", async () => {
|
||||
reconcileCronTriggerMock.mockResolvedValue({
|
||||
kind: "delete-failed",
|
||||
message: "server error",
|
||||
triggers: [relistedCronTrigger],
|
||||
});
|
||||
renderDetailView(baseRoutine, [cronTrigger]);
|
||||
|
||||
clickSave();
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
"The routine was saved, but removing the old schedule failed: server error. 1 cron schedule is now active.",
|
||||
),
|
||||
);
|
||||
expect(refreshMock).not.toHaveBeenCalled();
|
||||
expect(screen.getByText(/Next fire: 6\/6\/2026/)).toBeTruthy();
|
||||
expect(screen.queryByText(/Next fire: 5\/5\/2026/)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -12,18 +12,21 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@
|
||||
import { toast } from "@/lib/toast/sonner";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { selectOfficeAgentProfiles } from "@/lib/state/slices/office/selectors";
|
||||
import {
|
||||
updateRoutine,
|
||||
runRoutine,
|
||||
createRoutineTrigger,
|
||||
deleteRoutineTrigger,
|
||||
} from "@/lib/api/domains/office-api";
|
||||
import type { Routine, RoutineTrigger } from "@/lib/state/slices/office/types";
|
||||
import { updateRoutine, runRoutine } from "@/lib/api/domains/office-api";
|
||||
import type {
|
||||
Routine,
|
||||
RoutineStatus,
|
||||
RoutineTrigger,
|
||||
UpdateRoutinePatch,
|
||||
} from "@/lib/state/slices/office/types";
|
||||
import { timeAgo } from "@/lib/utils/time";
|
||||
import { useOfficeTopbar } from "../../components/office-topbar-context";
|
||||
import { isRoutineFiring } from "../../lib/routine-status";
|
||||
import { routineNotFiringMessage } from "../../lib/routine-not-firing";
|
||||
import { coerceCatchUpMax } from "../../lib/catch-up-max";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import type { TFunction } from "i18next";
|
||||
import { reconcileCronTrigger, type CronReconcileOutcome } from "./cron-reconcile";
|
||||
|
||||
// Lift the form state out of the component so the file stays under the
|
||||
// 100-line per-function ceiling and the helpers can render typed slices
|
||||
@@ -31,16 +34,24 @@ import { useTranslation } from "react-i18next";
|
||||
type DraftState = {
|
||||
name: string;
|
||||
description: string;
|
||||
status: "active" | "paused" | "archived";
|
||||
// Undefined means the stored status is not one of the three selectable
|
||||
// options; the control shows no selection and a save omits the field.
|
||||
status: RoutineStatus | undefined;
|
||||
assigneeAgentProfileId: string;
|
||||
concurrencyPolicy: string;
|
||||
catchUpPolicy: string;
|
||||
catchUpMax: number;
|
||||
catchUpMax: string;
|
||||
triggerKind: "cron" | "webhook";
|
||||
cronExpression: string;
|
||||
timezone: string;
|
||||
};
|
||||
|
||||
const ROUTINE_STATUSES: RoutineStatus[] = ["active", "paused", "archived"];
|
||||
|
||||
function normalizeDraftStatus(status: string): RoutineStatus | undefined {
|
||||
return (ROUTINE_STATUSES as string[]).includes(status) ? (status as RoutineStatus) : undefined;
|
||||
}
|
||||
|
||||
function pickTriggerKind(triggers: RoutineTrigger[]): "cron" | "webhook" {
|
||||
const cron = triggers.find((t) => t.kind === "cron");
|
||||
if (cron) return "cron";
|
||||
@@ -55,17 +66,63 @@ function buildDraft(routine: Routine, triggers: RoutineTrigger[]): DraftState {
|
||||
return {
|
||||
name: routine.name,
|
||||
description: routine.description ?? "",
|
||||
status: (routine.status as DraftState["status"]) ?? "active",
|
||||
status: normalizeDraftStatus(routine.status),
|
||||
assigneeAgentProfileId: routine.assigneeAgentProfileId ?? "",
|
||||
concurrencyPolicy: routine.concurrencyPolicy ?? "coalesce_if_active",
|
||||
catchUpPolicy: routine.catchUpPolicy ?? "summarize_missed",
|
||||
catchUpMax: routine.catchUpMax ?? 25,
|
||||
catchUpMax: String(coerceCatchUpMax(routine.catchUpMax)),
|
||||
triggerKind,
|
||||
cronExpression: cron?.cronExpression ?? "",
|
||||
timezone: cron?.timezone ?? "UTC",
|
||||
};
|
||||
}
|
||||
|
||||
function buildUpdatePatch(draft: DraftState): UpdateRoutinePatch {
|
||||
return {
|
||||
name: draft.name,
|
||||
description: draft.description,
|
||||
status: draft.status,
|
||||
assigneeAgentProfileId: draft.assigneeAgentProfileId,
|
||||
concurrencyPolicy: draft.concurrencyPolicy,
|
||||
catchUpPolicy: draft.catchUpPolicy,
|
||||
catchUpMax: coerceCatchUpMax(draft.catchUpMax),
|
||||
};
|
||||
}
|
||||
|
||||
function describeCronOutcome(
|
||||
outcome: CronReconcileOutcome,
|
||||
t: TFunction,
|
||||
): { toastKind: "success" | "error"; message: string; refresh: boolean } {
|
||||
if (outcome.kind === "unchanged") {
|
||||
return { toastKind: "success", message: t("office:routineSaved"), refresh: true };
|
||||
}
|
||||
if (outcome.kind === "success") {
|
||||
return outcome.triggers === null
|
||||
? {
|
||||
toastKind: "success",
|
||||
message: t("office:routineSavedScheduleMayBeStale"),
|
||||
refresh: false,
|
||||
}
|
||||
: { toastKind: "success", message: t("office:routineSaved"), refresh: true };
|
||||
}
|
||||
if (outcome.triggers === null) {
|
||||
return { toastKind: "error", message: t("office:routineScheduleFateUnknown"), refresh: false };
|
||||
}
|
||||
if (outcome.kind === "create-failed") {
|
||||
return {
|
||||
toastKind: "error",
|
||||
message: t("office:routineSavedScheduleChangeFailed", { error: outcome.message }),
|
||||
refresh: false,
|
||||
};
|
||||
}
|
||||
const count = outcome.triggers.filter((trigger) => trigger.kind === "cron").length;
|
||||
return {
|
||||
toastKind: "error",
|
||||
message: t("office:routineSavedScheduleDeleteFailed", { error: outcome.message, count }),
|
||||
refresh: false,
|
||||
};
|
||||
}
|
||||
|
||||
type RoutineDetailViewProps = {
|
||||
initialRoutine: Routine;
|
||||
initialTriggers: RoutineTrigger[];
|
||||
@@ -76,7 +133,7 @@ export function RoutineDetailView({ initialRoutine, initialTriggers }: RoutineDe
|
||||
const router = useRouter();
|
||||
const agents = useAppStore(selectOfficeAgentProfiles);
|
||||
const [routine] = useState(initialRoutine);
|
||||
const [triggers, setTriggers] = useState(initialTriggers);
|
||||
const [triggers, setTriggers] = useState<RoutineTrigger[] | null>(initialTriggers);
|
||||
const [draft, setDraft] = useState<DraftState>(buildDraft(initialRoutine, initialTriggers));
|
||||
const [saving, setSaving] = useState(false);
|
||||
const update = useCallback(
|
||||
@@ -84,31 +141,44 @@ export function RoutineDetailView({ initialRoutine, initialTriggers }: RoutineDe
|
||||
[],
|
||||
);
|
||||
|
||||
const cronTrigger = triggers.find((t) => t.kind === "cron");
|
||||
const cronTrigger = triggers?.find((t) => t.kind === "cron");
|
||||
const lastFired = cronTrigger?.lastFiredAt ?? null;
|
||||
|
||||
const handleSave = useCallback(async () => {
|
||||
if (triggers === null) {
|
||||
toast.error(t("office:routineScheduleFateUnknown"));
|
||||
return;
|
||||
}
|
||||
setSaving(true);
|
||||
try {
|
||||
await updateRoutine(routine.id, {
|
||||
name: draft.name,
|
||||
description: draft.description,
|
||||
status: draft.status,
|
||||
assigneeAgentProfileId: draft.assigneeAgentProfileId,
|
||||
concurrencyPolicy: draft.concurrencyPolicy,
|
||||
catchUpPolicy: draft.catchUpPolicy,
|
||||
catchUpMax: draft.catchUpMax,
|
||||
} as Record<string, unknown>);
|
||||
const nextTriggers = await syncCronTrigger(routine.id, draft, triggers);
|
||||
setTriggers(nextTriggers);
|
||||
toast.success(t("office:routineSaved"));
|
||||
router.refresh();
|
||||
await updateRoutine(routine.id, buildUpdatePatch(draft));
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : t("office:failedToSaveRoutine"));
|
||||
setSaving(false);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const outcome = await reconcileCronTrigger(routine.id, draft, triggers);
|
||||
if (outcome.kind !== "unchanged") setTriggers(outcome.triggers);
|
||||
const result = describeCronOutcome(outcome, t);
|
||||
if (result.toastKind === "success") {
|
||||
toast.success(result.message);
|
||||
} else {
|
||||
toast.error(result.message);
|
||||
}
|
||||
if (result.refresh) router.refresh();
|
||||
} catch (err) {
|
||||
// reconcileCronTrigger is designed to always resolve (every internal
|
||||
// call is its own try/catch) rather than throw, but nothing enforces
|
||||
// that contract. Catching here — not just the `finally` below — keeps
|
||||
// a future violation from becoming an unhandled rejection out of an
|
||||
// unawaited click handler on top of a stuck Save button.
|
||||
toast.error(err instanceof Error ? err.message : t("office:failedToSaveRoutine"));
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
}, [routine.id, draft, triggers, router]);
|
||||
}, [routine.id, draft, triggers, router, t]);
|
||||
|
||||
const handleRunNow = useCallback(async () => {
|
||||
try {
|
||||
@@ -143,7 +213,9 @@ export function RoutineDetailView({ initialRoutine, initialTriggers }: RoutineDe
|
||||
<DetailTriggerCard draft={draft} update={update} />
|
||||
<DetailReadOnlyCard
|
||||
lastFiredAt={lastFired}
|
||||
nextRunAt={isRoutineFiring(draft.status) ? (cronTrigger?.nextRunAt ?? null) : null}
|
||||
nextRunAt={
|
||||
isRoutineFiring(draft.status ?? routine.status) ? (cronTrigger?.nextRunAt ?? null) : null
|
||||
}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
@@ -174,7 +246,8 @@ function DetailGeneralCard({
|
||||
type="number"
|
||||
min={1}
|
||||
value={draft.catchUpMax}
|
||||
onChange={(e) => update({ catchUpMax: Number(e.target.value) || 25 })}
|
||||
onChange={(e) => update({ catchUpMax: e.target.value })}
|
||||
onBlur={(e) => update({ catchUpMax: String(coerceCatchUpMax(e.target.value)) })}
|
||||
/>
|
||||
</Field>
|
||||
)}
|
||||
@@ -221,7 +294,7 @@ function StatusAndAssigneeFields({
|
||||
<div className="grid grid-cols-2 gap-4">
|
||||
<Field label={t("common:status")}>
|
||||
<Select
|
||||
value={draft.status}
|
||||
value={draft.status ?? ""}
|
||||
onValueChange={(v) => update({ status: v as DraftState["status"] })}
|
||||
>
|
||||
<SelectTrigger className="cursor-pointer">
|
||||
@@ -404,42 +477,3 @@ function Field({ label, children }: { label: string; children: React.ReactNode }
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// syncCronTrigger reconciles the routine's cron trigger with the
|
||||
// draft's expression / timezone / kind. Two paths:
|
||||
// - Draft.kind === "cron" with a non-empty expression → ensure a
|
||||
// matching trigger exists (delete the old one + create a new one
|
||||
// when the expression changes; the trigger model has no PATCH
|
||||
// endpoint today and the cron-expression is the schedule's
|
||||
// identity, so a delete + create is the simplest path).
|
||||
// - Draft.kind === "webhook" or empty cron → leave triggers alone for
|
||||
// now. A future iteration can add explicit webhook config.
|
||||
async function syncCronTrigger(
|
||||
routineId: string,
|
||||
draft: DraftState,
|
||||
triggers: RoutineTrigger[],
|
||||
): Promise<RoutineTrigger[]> {
|
||||
if (draft.triggerKind !== "cron" || !draft.cronExpression.trim()) {
|
||||
return triggers;
|
||||
}
|
||||
const existing = triggers.find((t) => t.kind === "cron");
|
||||
if (
|
||||
existing &&
|
||||
existing.cronExpression === draft.cronExpression &&
|
||||
(existing.timezone ?? "UTC") === draft.timezone
|
||||
) {
|
||||
return triggers;
|
||||
}
|
||||
if (existing) {
|
||||
await deleteRoutineTrigger(existing.id);
|
||||
}
|
||||
const res = await createRoutineTrigger(routineId, {
|
||||
kind: "cron",
|
||||
cronExpression: draft.cronExpression,
|
||||
timezone: draft.timezone,
|
||||
});
|
||||
const created = (res as unknown as { trigger?: RoutineTrigger }).trigger ?? null;
|
||||
const next = triggers.filter((t) => t.id !== existing?.id);
|
||||
if (created) next.push(created);
|
||||
return next;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, render, screen, fireEvent, within } from "@testing-library/react";
|
||||
import { act, cleanup, render, screen, fireEvent, waitFor, within } from "@testing-library/react";
|
||||
import type { AgentProfile } from "@/lib/state/slices/office/types";
|
||||
|
||||
import { CreateRoutineDialog } from "./create-routine-dialog";
|
||||
@@ -83,6 +83,28 @@ describe("CreateRoutineDialog catch-up max control (AC-003.8)", () => {
|
||||
expect.objectContaining({ catchUpPolicy: "summarize_missed" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the raw value while editing so a cleared field can be retyped", () => {
|
||||
const onSubmit = vi.fn();
|
||||
render(
|
||||
<CreateRoutineDialog open onOpenChange={vi.fn()} agents={[AGENT]} onSubmit={onSubmit} />,
|
||||
);
|
||||
goToScheduleStep();
|
||||
|
||||
const input = screen.getByRole("spinbutton") as HTMLInputElement;
|
||||
fireEvent.change(input, { target: { value: "" } });
|
||||
expect(input.value).toBe("");
|
||||
|
||||
fireEvent.change(input, { target: { value: "7" } });
|
||||
expect(input.value).toBe("7");
|
||||
|
||||
fireEvent.change(screen.getByPlaceholderText("0 9 * * *"), {
|
||||
target: { value: "0 9 * * *" },
|
||||
});
|
||||
fireEvent.click(screen.getByRole("button", { name: /create/i }));
|
||||
|
||||
expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ catchUpMax: 7 }));
|
||||
});
|
||||
});
|
||||
|
||||
// AC-OFFICE-ROUTINE-CATCHUP-003.6: the summarizing policy's own label must
|
||||
@@ -103,3 +125,58 @@ describe("CreateRoutineDialog catch-up policy labeling (AC-003.6)", () => {
|
||||
expect(option.textContent).toMatch(/once|single/i);
|
||||
});
|
||||
});
|
||||
|
||||
// The backend has no create idempotency/uniqueness guard, so a second
|
||||
// concurrent submission (double-click, or a repeated Enter activation per
|
||||
// this repo's dialog Enter-to-confirm behavior) while the first is still
|
||||
// in flight would otherwise persist a duplicate routine and arm a second
|
||||
// schedule for it.
|
||||
describe("CreateRoutineDialog concurrent submission guard", () => {
|
||||
it("does not call onSubmit again while the first create is still in flight", async () => {
|
||||
let resolveSubmit: (value: boolean) => void = () => {};
|
||||
const onSubmit = vi.fn(
|
||||
() =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
resolveSubmit = resolve;
|
||||
}),
|
||||
);
|
||||
render(
|
||||
<CreateRoutineDialog open onOpenChange={vi.fn()} agents={[AGENT]} onSubmit={onSubmit} />,
|
||||
);
|
||||
goToScheduleStep();
|
||||
fireEvent.change(screen.getByPlaceholderText("0 9 * * *"), { target: { value: "0 9 * * *" } });
|
||||
|
||||
const createButton = screen.getByRole("button", { name: /create/i });
|
||||
fireEvent.click(createButton);
|
||||
fireEvent.click(createButton);
|
||||
fireEvent.click(createButton);
|
||||
|
||||
expect(onSubmit).toHaveBeenCalledTimes(1);
|
||||
expect((createButton as HTMLButtonElement).disabled).toBe(true);
|
||||
|
||||
await act(async () => {
|
||||
resolveSubmit(true);
|
||||
await Promise.resolve();
|
||||
});
|
||||
// Still only ever called once, even after settling.
|
||||
expect(onSubmit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("re-enables Create for a retry after a rejected create resolves", async () => {
|
||||
const onSubmit = vi.fn().mockResolvedValue(false);
|
||||
render(
|
||||
<CreateRoutineDialog open onOpenChange={vi.fn()} agents={[AGENT]} onSubmit={onSubmit} />,
|
||||
);
|
||||
goToScheduleStep();
|
||||
fireEvent.change(screen.getByPlaceholderText("0 9 * * *"), { target: { value: "0 9 * * *" } });
|
||||
|
||||
const createButton = screen.getByRole("button", { name: /create/i });
|
||||
fireEvent.click(createButton);
|
||||
|
||||
await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1));
|
||||
await waitFor(() => expect((createButton as HTMLButtonElement).disabled).toBe(false));
|
||||
|
||||
fireEvent.click(createButton);
|
||||
await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(2));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,6 +9,7 @@ import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogFooter } from "
|
||||
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select";
|
||||
import { IconArrowLeft, IconArrowRight } from "@tabler/icons-react";
|
||||
import type { AgentProfile } from "@/lib/state/slices/office/types";
|
||||
import { coerceCatchUpMax } from "../lib/catch-up-max";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
type CreateRoutineDialogProps = {
|
||||
@@ -27,7 +28,7 @@ type CreateRoutineDialogProps = {
|
||||
triggerKind: string;
|
||||
cronExpression: string;
|
||||
timezone: string;
|
||||
}) => void;
|
||||
}) => Promise<boolean>;
|
||||
};
|
||||
|
||||
type RoutineFormState = {
|
||||
@@ -38,7 +39,7 @@ type RoutineFormState = {
|
||||
assignee: string;
|
||||
concurrency: string;
|
||||
catchUpPolicy: string;
|
||||
catchUpMax: number;
|
||||
catchUpMax: string;
|
||||
triggerKind: string;
|
||||
cronExpr: string;
|
||||
timezone: string;
|
||||
@@ -52,7 +53,7 @@ const INITIAL_ROUTINE_STATE: RoutineFormState = {
|
||||
assignee: "",
|
||||
concurrency: "coalesce_if_active",
|
||||
catchUpPolicy: "summarize_missed",
|
||||
catchUpMax: 25,
|
||||
catchUpMax: "25",
|
||||
triggerKind: "cron",
|
||||
cronExpr: "",
|
||||
timezone: "UTC",
|
||||
@@ -306,7 +307,8 @@ function PolicyFields({
|
||||
type="number"
|
||||
min={1}
|
||||
value={state.catchUpMax}
|
||||
onChange={(e) => onUpdate({ catchUpMax: Number(e.target.value) || 25 })}
|
||||
onChange={(e) => onUpdate({ catchUpMax: e.target.value })}
|
||||
onBlur={(e) => onUpdate({ catchUpMax: String(coerceCatchUpMax(e.target.value)) })}
|
||||
className="mt-1.5"
|
||||
/>
|
||||
<p className="text-xs text-muted-foreground mt-1.5">
|
||||
@@ -339,6 +341,62 @@ function canAdvance(step: number, state: RoutineFormState): boolean {
|
||||
return true;
|
||||
}
|
||||
|
||||
function buildSubmitPayload(state: RoutineFormState) {
|
||||
return {
|
||||
name: state.name,
|
||||
description: state.description,
|
||||
taskTitle: state.taskTitle,
|
||||
taskDescription: state.taskDesc,
|
||||
assigneeAgentProfileId: state.assignee,
|
||||
concurrencyPolicy: state.concurrency,
|
||||
catchUpPolicy: state.catchUpPolicy,
|
||||
catchUpMax: coerceCatchUpMax(state.catchUpMax),
|
||||
triggerKind: state.triggerKind,
|
||||
cronExpression: state.cronExpr,
|
||||
timezone: state.timezone,
|
||||
};
|
||||
}
|
||||
|
||||
// Owns the dialog's step/form state plus submission, keeping
|
||||
// CreateRoutineDialog itself under the per-function line ceiling.
|
||||
function useCreateRoutineDialogState(
|
||||
onOpenChange: (open: boolean) => void,
|
||||
onSubmit: CreateRoutineDialogProps["onSubmit"],
|
||||
) {
|
||||
const [step, setStep] = useState(0);
|
||||
const [state, setState] = useState<RoutineFormState>(INITIAL_ROUTINE_STATE);
|
||||
// Guards against a second `handleSubmit` firing (double-click, or a
|
||||
// repeated Enter activation per the dialog's Enter-to-confirm behavior)
|
||||
// while the first `onSubmit` call is still in flight — the backend has no
|
||||
// create idempotency guard, so two concurrent submits persist two routines.
|
||||
const [submitting, setSubmitting] = useState(false);
|
||||
const update = (patch: Partial<RoutineFormState>) => setState((prev) => ({ ...prev, ...patch }));
|
||||
|
||||
function handleOpenChange(next: boolean) {
|
||||
if (!next) {
|
||||
setState(INITIAL_ROUTINE_STATE);
|
||||
setStep(0);
|
||||
}
|
||||
onOpenChange(next);
|
||||
}
|
||||
|
||||
async function handleSubmit() {
|
||||
if (submitting) return;
|
||||
setSubmitting(true);
|
||||
try {
|
||||
const succeeded = await onSubmit(buildSubmitPayload(state));
|
||||
// A rejected create leaves the dialog open (per onSubmit's contract) for
|
||||
// the user to correct and retry; resetting the form on that path would
|
||||
// silently discard what they just typed.
|
||||
if (succeeded) handleOpenChange(false);
|
||||
} finally {
|
||||
setSubmitting(false);
|
||||
}
|
||||
}
|
||||
|
||||
return { step, setStep, state, update, submitting, handleOpenChange, handleSubmit };
|
||||
}
|
||||
|
||||
function StepContent({
|
||||
step,
|
||||
state,
|
||||
@@ -362,36 +420,8 @@ export function CreateRoutineDialog({
|
||||
onSubmit,
|
||||
}: CreateRoutineDialogProps) {
|
||||
const { t } = useTranslation();
|
||||
const [step, setStep] = useState(0);
|
||||
const [state, setState] = useState<RoutineFormState>(INITIAL_ROUTINE_STATE);
|
||||
const update = (patch: Partial<RoutineFormState>) => setState((prev) => ({ ...prev, ...patch }));
|
||||
|
||||
function reset() {
|
||||
setState(INITIAL_ROUTINE_STATE);
|
||||
setStep(0);
|
||||
}
|
||||
|
||||
function handleOpenChange(next: boolean) {
|
||||
if (!next) reset();
|
||||
onOpenChange(next);
|
||||
}
|
||||
|
||||
function handleSubmit() {
|
||||
onSubmit({
|
||||
name: state.name,
|
||||
description: state.description,
|
||||
taskTitle: state.taskTitle,
|
||||
taskDescription: state.taskDesc,
|
||||
assigneeAgentProfileId: state.assignee,
|
||||
concurrencyPolicy: state.concurrency,
|
||||
catchUpPolicy: state.catchUpPolicy,
|
||||
catchUpMax: state.catchUpMax,
|
||||
triggerKind: state.triggerKind,
|
||||
cronExpression: state.cronExpr,
|
||||
timezone: state.timezone,
|
||||
});
|
||||
reset();
|
||||
}
|
||||
const { step, setStep, state, update, submitting, handleOpenChange, handleSubmit } =
|
||||
useCreateRoutineDialogState(onOpenChange, onSubmit);
|
||||
|
||||
const isLast = step === STEP_COUNT - 1;
|
||||
const advanceEnabled = canAdvance(step, state);
|
||||
@@ -435,7 +465,11 @@ export function CreateRoutineDialog({
|
||||
{t("common:cancel")}
|
||||
</Button>
|
||||
{isLast ? (
|
||||
<Button onClick={handleSubmit} disabled={!advanceEnabled} className="cursor-pointer">
|
||||
<Button
|
||||
onClick={handleSubmit}
|
||||
disabled={!advanceEnabled || submitting}
|
||||
className="cursor-pointer"
|
||||
>
|
||||
{t("office:create")}
|
||||
</Button>
|
||||
) : (
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { formatVariableValue, templateText } from "./routine-row";
|
||||
|
||||
describe("formatVariableValue", () => {
|
||||
it("renders a declared variable's default string, not the wrapping object", () => {
|
||||
expect(formatVariableValue({ default: "prod" })).toBe("prod");
|
||||
});
|
||||
|
||||
it("passes a bare string value through unchanged", () => {
|
||||
expect(formatVariableValue("prod")).toBe("prod");
|
||||
});
|
||||
|
||||
it("falls back to an empty string when default is missing or not a string", () => {
|
||||
expect(formatVariableValue({})).toBe("");
|
||||
expect(formatVariableValue({ default: 5 })).toBe("");
|
||||
expect(formatVariableValue(null)).toBe("");
|
||||
expect(formatVariableValue(undefined)).toBe("");
|
||||
});
|
||||
});
|
||||
|
||||
describe("templateText", () => {
|
||||
it("passes a string value through unchanged", () => {
|
||||
expect(templateText("Nightly summary")).toBe("Nightly summary");
|
||||
});
|
||||
|
||||
it("falls back to an empty string for a non-string value instead of rendering it", () => {
|
||||
// task_template is an unvalidated backend string; a malformed value like
|
||||
// {"title":{"nested":1}} must not reach a JSX text node as an object.
|
||||
expect(templateText({ nested: 1 })).toBe("");
|
||||
expect(templateText(5)).toBe("");
|
||||
expect(templateText(null)).toBe("");
|
||||
expect(templateText(undefined)).toBe("");
|
||||
});
|
||||
});
|
||||
@@ -34,6 +34,25 @@ function concurrencyLabel(t: TFunction, policy: string): string {
|
||||
return key ? t(key) : policy;
|
||||
}
|
||||
|
||||
/** A declared variable's wire value is `{ default: string }`; a bare string passes through unchanged. */
|
||||
export function formatVariableValue(value: unknown): string {
|
||||
if (typeof value === "string") return value;
|
||||
if (value && typeof value === "object" && "default" in value) {
|
||||
const declaredDefault = (value as { default?: unknown }).default;
|
||||
if (typeof declaredDefault === "string") return declaredDefault;
|
||||
}
|
||||
return "";
|
||||
}
|
||||
|
||||
/**
|
||||
* `task_template` is an unvalidated wire string decoded to `Record<string, unknown>`
|
||||
* (`parseOptionalJSONObject`); a non-string `title`/`description` must not reach a JSX
|
||||
* text node, so this coerces anything else to "" instead of casting.
|
||||
*/
|
||||
export function templateText(value: unknown): string {
|
||||
return typeof value === "string" ? value : "";
|
||||
}
|
||||
|
||||
type RoutineRowProps = {
|
||||
routine: Routine;
|
||||
agents: AgentProfile[];
|
||||
@@ -75,16 +94,13 @@ export function RoutineRow({
|
||||
onClick,
|
||||
}: RoutineRowProps) {
|
||||
const { t } = useTranslation();
|
||||
// The API may return snake_case fields (assignee_agent_profile_id, concurrency_policy)
|
||||
// before any mapping layer converts them. Use both camelCase and snake_case lookups.
|
||||
const routineRaw = routine as unknown as Record<string, unknown>;
|
||||
const assigneeId =
|
||||
routine.assigneeAgentProfileId ?? (routineRaw.assignee_agent_profile_id as string | undefined);
|
||||
const concurrencyPolicy =
|
||||
routine.concurrencyPolicy ?? (routineRaw.concurrency_policy as string | undefined) ?? "";
|
||||
const assignee = agents.find((a) => a.id === assigneeId);
|
||||
const assignee = agents.find((a) => a.id === routine.assigneeAgentProfileId);
|
||||
const isActive = isRoutineFiring(routine.status);
|
||||
const template = routine.taskTemplate as { title?: string; description?: string } | undefined;
|
||||
const rawTemplate = routine.taskTemplate as Record<string, unknown> | undefined;
|
||||
const template = {
|
||||
title: templateText(rawTemplate?.title),
|
||||
description: templateText(rawTemplate?.description),
|
||||
};
|
||||
const cronTrigger = triggers.find((t) => t.kind === "cron");
|
||||
const nextFire = isActive ? nextFireText(t, triggers) : "";
|
||||
|
||||
@@ -113,7 +129,7 @@ export function RoutineRow({
|
||||
)}
|
||||
{nextFire && <span>{t("office:nextIn", { when: nextFire })}</span>}
|
||||
<span>{routine.lastRunAt ? timeAgo(routine.lastRunAt) : t("office:neverRun")}</span>
|
||||
<span>{concurrencyLabel(t, concurrencyPolicy)}</span>
|
||||
<span>{concurrencyLabel(t, routine.concurrencyPolicy)}</span>
|
||||
</div>
|
||||
</div>
|
||||
<Badge variant={isActive ? "default" : "secondary"}>
|
||||
@@ -206,19 +222,16 @@ function RoutineExpandedDetail({
|
||||
}: {
|
||||
routine: Routine;
|
||||
assignee: AgentProfile | undefined;
|
||||
template: { title?: string; description?: string } | undefined;
|
||||
template: { title: string; description: string };
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
const routineRaw = routine as unknown as Record<string, unknown>;
|
||||
const concurrencyPolicy =
|
||||
routine.concurrencyPolicy ?? (routineRaw.concurrency_policy as string | undefined) ?? "";
|
||||
return (
|
||||
<div className="px-4 pb-3 pt-1 ml-7 border-t border-border/50 space-y-2 text-sm">
|
||||
{routine.description && (
|
||||
<DetailField label={t("office:description")} value={routine.description} />
|
||||
)}
|
||||
{template?.title && <DetailField label={t("office:taskTitle")} value={template.title} />}
|
||||
{template?.description && (
|
||||
{template.title && <DetailField label={t("office:taskTitle")} value={template.title} />}
|
||||
{template.description && (
|
||||
<DetailField label={t("office:taskDescription")} value={template.description} />
|
||||
)}
|
||||
<DetailField label={t("office:assignee")} value={assignee?.name ?? t("office:unassigned")} />
|
||||
@@ -226,14 +239,17 @@ function RoutineExpandedDetail({
|
||||
label={t("office:lastRun")}
|
||||
value={routine.lastRunAt ? timeAgo(routine.lastRunAt) : t("office:lastRunNever")}
|
||||
/>
|
||||
<DetailField label={t("office:concurrency")} value={concurrencyLabel(t, concurrencyPolicy)} />
|
||||
<DetailField
|
||||
label={t("office:concurrency")}
|
||||
value={concurrencyLabel(t, routine.concurrencyPolicy)}
|
||||
/>
|
||||
{routine.variables && Object.keys(routine.variables).length > 0 && (
|
||||
<div>
|
||||
<span className="text-xs font-medium text-muted-foreground">{t("office:variables")}</span>
|
||||
<div className="mt-1 space-y-0.5">
|
||||
{Object.entries(routine.variables).map(([key, val]) => (
|
||||
<div key={key} className="text-xs font-mono text-muted-foreground">
|
||||
{key}: {String(val)}
|
||||
{key}: {formatVariableValue(val)}
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
import { readdirSync, readFileSync, statSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
const here = path.dirname(fileURLToPath(import.meta.url));
|
||||
const APP_ROOT = path.join(here, "../../..");
|
||||
|
||||
/**
|
||||
* The left columns of the routine, trigger and run mapping tables in
|
||||
* docs/specs/office/system-design/routine-wire-contract.md, restricted to
|
||||
* keys containing an underscore. `id`, `name`, `status`, `kind`, `timezone`,
|
||||
* `enabled`, `variables` and `source` are deliberately excluded: they are
|
||||
* legitimate model-shape spellings that appear throughout these files, so
|
||||
* including them would fail correct code.
|
||||
*/
|
||||
const WIRE_KEYS = [
|
||||
"workspace_id",
|
||||
"task_template",
|
||||
"assignee_agent_profile_id",
|
||||
"concurrency_policy",
|
||||
"catch_up_policy",
|
||||
"catch_up_max",
|
||||
"last_run_at",
|
||||
"created_at",
|
||||
"updated_at",
|
||||
"routine_id",
|
||||
"cron_expression",
|
||||
"public_id",
|
||||
"signing_mode",
|
||||
"next_run_at",
|
||||
"last_fired_at",
|
||||
"trigger_id",
|
||||
"trigger_payload",
|
||||
"linked_task_id",
|
||||
"coalesced_into_run_id",
|
||||
"dispatch_fingerprint",
|
||||
"catch_up_missed_ticks",
|
||||
"catch_up_first_missed_at",
|
||||
"catch_up_truncated",
|
||||
"started_at",
|
||||
"completed_at",
|
||||
"skip_reason",
|
||||
"pause_id",
|
||||
"causation_id",
|
||||
];
|
||||
|
||||
const WIRE_KEY_PATTERN = new RegExp(`\\b(${WIRE_KEYS.join("|")})\\b`);
|
||||
const AS_UNKNOWN_AS_PATTERN = /\bas unknown as\b/;
|
||||
|
||||
function listSourceFiles(dir: string): string[] {
|
||||
const entries = readdirSync(dir);
|
||||
const files: string[] = [];
|
||||
for (const entry of entries) {
|
||||
const full = path.join(dir, entry);
|
||||
const stat = statSync(full);
|
||||
if (stat.isDirectory()) {
|
||||
files.push(...listSourceFiles(full));
|
||||
continue;
|
||||
}
|
||||
if (/\.tsx?$/.test(entry) && !/\.test\.tsx?$/.test(entry)) {
|
||||
files.push(full);
|
||||
}
|
||||
}
|
||||
return files;
|
||||
}
|
||||
|
||||
function isExemptLine(line: string): boolean {
|
||||
const trimmed = line.trimStart();
|
||||
return trimmed.startsWith("//") || trimmed.startsWith("*");
|
||||
}
|
||||
|
||||
function findMatches(files: string[], pattern: RegExp): string[] {
|
||||
const hits: string[] = [];
|
||||
for (const file of files) {
|
||||
const lines = readFileSync(file, "utf8").split("\n");
|
||||
lines.forEach((line, index) => {
|
||||
if (isExemptLine(line)) return;
|
||||
if (pattern.test(line)) {
|
||||
hits.push(`${path.relative(APP_ROOT, file)}:${index + 1}: ${line.trim()}`);
|
||||
}
|
||||
});
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
||||
const SCANNED_FILES = [
|
||||
...listSourceFiles(path.join(APP_ROOT, "app/office/routines")),
|
||||
path.join(APP_ROOT, "src/office-routine-client-routes.tsx"),
|
||||
];
|
||||
|
||||
describe("routine wire contract scan (AC-OFFICE-ROUTINE-WIRE-003.10)", () => {
|
||||
it("contains no snake_case wire key outside comments and tests", () => {
|
||||
expect(findMatches(SCANNED_FILES, WIRE_KEY_PATTERN)).toEqual([]);
|
||||
});
|
||||
|
||||
it("contains no `as unknown as` cast", () => {
|
||||
expect(findMatches(SCANNED_FILES, AS_UNKNOWN_AS_PATTERN)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("s.office.routines consumer set (AC-OFFICE-ROUTINE-WIRE-003.18)", () => {
|
||||
it("is exactly the three known files under apps/web/app/**", () => {
|
||||
const appFiles = listSourceFiles(path.join(APP_ROOT, "app"));
|
||||
const consumers = appFiles
|
||||
.filter((file) => {
|
||||
const lines = readFileSync(file, "utf8").split("\n");
|
||||
return lines.some((line) => !isExemptLine(line) && line.includes("s.office.routines"));
|
||||
})
|
||||
.map((file) => path.relative(APP_ROOT, file))
|
||||
.sort();
|
||||
|
||||
expect(consumers).toEqual(
|
||||
[
|
||||
"app/office/agents/[id]/layout.tsx",
|
||||
"app/office/agents/[id]/runs/runs-list-view.tsx",
|
||||
"app/office/routines/routines-content.tsx",
|
||||
].sort(),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,301 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { defaultOfficeState } from "@/lib/state/slices/office/office-slice";
|
||||
import type { AgentProfile } from "@/lib/state/slices/office/types";
|
||||
import { RoutinesContent } from "./routines-content";
|
||||
import {
|
||||
createRoutine,
|
||||
createRoutineTrigger,
|
||||
listAllRoutineRuns,
|
||||
listRoutines,
|
||||
listRoutineTriggers,
|
||||
} from "@/lib/api/domains/office-api";
|
||||
|
||||
vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
|
||||
vi.mock("@/lib/api/domains/office-api", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/lib/api/domains/office-api")>(
|
||||
"@/lib/api/domains/office-api",
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
listRoutines: vi.fn(),
|
||||
listAllRoutineRuns: vi.fn(),
|
||||
listRoutineTriggers: vi.fn(),
|
||||
createRoutine: vi.fn(),
|
||||
createRoutineTrigger: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
import { toast } from "sonner";
|
||||
|
||||
const listRoutinesMock = vi.mocked(listRoutines);
|
||||
const listAllRoutineRunsMock = vi.mocked(listAllRoutineRuns);
|
||||
const listRoutineTriggersMock = vi.mocked(listRoutineTriggers);
|
||||
const createRoutineMock = vi.mocked(createRoutine);
|
||||
const createRoutineTriggerMock = vi.mocked(createRoutineTrigger);
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
const WORKSPACE_ID = "ws-1";
|
||||
const TIMESTAMP = "2026-05-04T00:00:00Z";
|
||||
const ROUTINE_NAME = "Nightly digest";
|
||||
const CRON_EXPRESSION_LABEL = "Cron Expression";
|
||||
|
||||
const AGENT: AgentProfile = {
|
||||
id: "agent-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
name: "Worker",
|
||||
role: "worker",
|
||||
status: "idle",
|
||||
agentProfileId: "profile-1",
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
permissions: {},
|
||||
pauseReason: "",
|
||||
budgetMonthlyCents: 0,
|
||||
maxConcurrentSessions: 1,
|
||||
} as AgentProfile;
|
||||
|
||||
function renderContent() {
|
||||
return render(
|
||||
<StateProvider
|
||||
initialState={{
|
||||
workspaces: { activeId: WORKSPACE_ID, items: [] },
|
||||
office: {
|
||||
...defaultOfficeState.office,
|
||||
agentProfilesByWorkspaceId: { [WORKSPACE_ID]: [AGENT] },
|
||||
routines: [],
|
||||
},
|
||||
}}
|
||||
>
|
||||
<RoutinesContent />
|
||||
</StateProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
async function openCreateDialogToScheduleStep() {
|
||||
fireEvent.click(screen.getByRole("button", { name: /new routine/i }));
|
||||
fireEvent.change(await screen.findByLabelText("Name"), {
|
||||
target: { value: ROUTINE_NAME },
|
||||
});
|
||||
fireEvent.click(screen.getAllByRole("combobox")[0]);
|
||||
fireEvent.click(within(screen.getByRole("listbox")).getByRole("option", { name: "Worker" }));
|
||||
fireEvent.click(screen.getByRole("button", { name: /next/i }));
|
||||
fireEvent.click(screen.getByRole("button", { name: /next/i }));
|
||||
}
|
||||
|
||||
describe("RoutinesContent create-routine cron arm (AC-002.2, AC-002.10)", () => {
|
||||
it("arms a cron trigger after the routine is created and shows one success toast", async () => {
|
||||
listRoutinesMock.mockResolvedValue({ routines: [] });
|
||||
listAllRoutineRunsMock.mockResolvedValue({ runs: [] });
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [] });
|
||||
createRoutineMock.mockResolvedValue({
|
||||
id: "routine-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
name: ROUTINE_NAME,
|
||||
taskTemplate: {},
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
});
|
||||
createRoutineTriggerMock.mockResolvedValue({
|
||||
id: "trigger-1",
|
||||
routineId: "routine-1",
|
||||
kind: "cron",
|
||||
cronExpression: "0 9 * * *",
|
||||
timezone: "UTC",
|
||||
enabled: true,
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
});
|
||||
|
||||
renderContent();
|
||||
await openCreateDialogToScheduleStep();
|
||||
fireEvent.change(screen.getByLabelText(CRON_EXPRESSION_LABEL), {
|
||||
target: { value: "0 9 * * *" },
|
||||
});
|
||||
fireEvent.click(screen.getByRole("button", { name: /create/i }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(createRoutineTriggerMock).toHaveBeenCalledWith("routine-1", {
|
||||
kind: "cron",
|
||||
cronExpression: "0 9 * * *",
|
||||
timezone: "UTC",
|
||||
});
|
||||
});
|
||||
expect(toast.success).toHaveBeenCalledWith("Routine created");
|
||||
expect(toast.error).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("disables Create for a cron expression that is only whitespace, so no trigger can be armed", async () => {
|
||||
renderContent();
|
||||
await openCreateDialogToScheduleStep();
|
||||
fireEvent.change(screen.getByLabelText(CRON_EXPRESSION_LABEL), {
|
||||
target: { value: " " },
|
||||
});
|
||||
|
||||
expect((screen.getByRole("button", { name: /create/i }) as HTMLButtonElement).disabled).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it("shows an error toast naming the trigger failure and no success toast when the cron trigger create fails", async () => {
|
||||
listRoutinesMock.mockResolvedValue({ routines: [] });
|
||||
listAllRoutineRunsMock.mockResolvedValue({ runs: [] });
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [] });
|
||||
createRoutineMock.mockResolvedValue({
|
||||
id: "routine-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
name: ROUTINE_NAME,
|
||||
taskTemplate: {},
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
});
|
||||
createRoutineTriggerMock.mockRejectedValue(new Error("invalid cron expression"));
|
||||
|
||||
renderContent();
|
||||
await openCreateDialogToScheduleStep();
|
||||
fireEvent.change(screen.getByLabelText(CRON_EXPRESSION_LABEL), {
|
||||
target: { value: "bad cron" },
|
||||
});
|
||||
fireEvent.click(screen.getByRole("button", { name: /create/i }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
"The routine was created without a schedule: invalid cron expression",
|
||||
);
|
||||
});
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
// TS-004: the failure branch must still close the dialog and refetch,
|
||||
// not just toast.
|
||||
expect(screen.queryByRole("button", { name: /^create$/i })).toBeNull();
|
||||
expect(listRoutinesMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
// TS-006: a rejected createRoutine call must leave the dialog open for the
|
||||
// user to correct and retry, and must not attempt to arm a trigger or
|
||||
// refetch the routine list for a routine that was never created.
|
||||
describe("RoutinesContent create failure (TS-006)", () => {
|
||||
it("keeps the dialog open and shows the create error without calling fetchRoutines or arming a trigger when createRoutine rejects", async () => {
|
||||
listRoutinesMock.mockResolvedValue({ routines: [] });
|
||||
listAllRoutineRunsMock.mockResolvedValue({ runs: [] });
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [] });
|
||||
createRoutineMock.mockRejectedValue(new Error("duplicate name"));
|
||||
|
||||
renderContent();
|
||||
await openCreateDialogToScheduleStep();
|
||||
fireEvent.change(screen.getByLabelText(CRON_EXPRESSION_LABEL), {
|
||||
target: { value: "0 9 * * *" },
|
||||
});
|
||||
const callsBeforeCreate = listRoutinesMock.mock.calls.length;
|
||||
fireEvent.click(screen.getByRole("button", { name: /create/i }));
|
||||
|
||||
await waitFor(() => expect(toast.error).toHaveBeenCalledWith("duplicate name"));
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
expect(createRoutineTriggerMock).not.toHaveBeenCalled();
|
||||
expect(listRoutinesMock.mock.calls.length).toBe(callsBeforeCreate);
|
||||
expect(screen.getByRole("button", { name: /^create$/i })).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// Regression coverage: handleCreate's refactor into two try/catch blocks
|
||||
// (create, then optional cron-arm) left the post-mutation `fetchRoutines()`
|
||||
// calls unguarded. A refetch failure there must not swallow the toast
|
||||
// reporting the create/trigger outcome, and must not become an unhandled
|
||||
// promise rejection.
|
||||
describe("RoutinesContent post-create refetch failure", () => {
|
||||
// The mount effect also calls fetchRoutines, so a fixed call-count
|
||||
// assumption ("call 2 is the post-create refetch") is fragile. Instead,
|
||||
// resolve every call until the test arms failNextFetch right before the
|
||||
// action whose refetch should fail, so the rejection lands on that call
|
||||
// regardless of how many mount-time calls preceded it.
|
||||
function mockListRoutinesFailingNextCallAfterArmed() {
|
||||
let failNext = false;
|
||||
listRoutinesMock.mockImplementation(async () => {
|
||||
if (failNext) {
|
||||
failNext = false;
|
||||
throw new Error("network down");
|
||||
}
|
||||
return { routines: [] };
|
||||
});
|
||||
return () => {
|
||||
failNext = true;
|
||||
};
|
||||
}
|
||||
|
||||
it("still shows the created toast when the post-success refetch fails", async () => {
|
||||
const armFailure = mockListRoutinesFailingNextCallAfterArmed();
|
||||
listAllRoutineRunsMock.mockResolvedValue({ runs: [] });
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [] });
|
||||
createRoutineMock.mockResolvedValue({
|
||||
id: "routine-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
name: ROUTINE_NAME,
|
||||
taskTemplate: {},
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
});
|
||||
|
||||
renderContent();
|
||||
fireEvent.click(screen.getByRole("button", { name: /new routine/i }));
|
||||
fireEvent.change(await screen.findByLabelText("Name"), {
|
||||
target: { value: ROUTINE_NAME },
|
||||
});
|
||||
fireEvent.click(screen.getAllByRole("combobox")[0]);
|
||||
fireEvent.click(within(screen.getByRole("listbox")).getByRole("option", { name: "Worker" }));
|
||||
fireEvent.click(screen.getByRole("button", { name: /next/i }));
|
||||
fireEvent.click(screen.getByRole("button", { name: /next/i }));
|
||||
// Switch off the default "cron" trigger kind (which disables Create
|
||||
// until a cron expression is entered) to exercise the plain
|
||||
// no-trigger create path.
|
||||
fireEvent.click(screen.getAllByRole("combobox")[0]);
|
||||
fireEvent.click(within(screen.getByRole("listbox")).getByRole("option", { name: "Webhook" }));
|
||||
armFailure();
|
||||
fireEvent.click(screen.getByRole("button", { name: /create/i }));
|
||||
|
||||
await waitFor(() => expect(toast.success).toHaveBeenCalledWith("Routine created"));
|
||||
expect(toast.error).toHaveBeenCalledWith("Failed to load");
|
||||
});
|
||||
|
||||
it("still shows the trigger-failure toast when that branch's refetch also fails", async () => {
|
||||
const armFailure = mockListRoutinesFailingNextCallAfterArmed();
|
||||
listAllRoutineRunsMock.mockResolvedValue({ runs: [] });
|
||||
listRoutineTriggersMock.mockResolvedValue({ triggers: [] });
|
||||
createRoutineMock.mockResolvedValue({
|
||||
id: "routine-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
name: ROUTINE_NAME,
|
||||
taskTemplate: {},
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
});
|
||||
createRoutineTriggerMock.mockRejectedValue(new Error("invalid cron expression"));
|
||||
|
||||
renderContent();
|
||||
await openCreateDialogToScheduleStep();
|
||||
fireEvent.change(screen.getByLabelText(CRON_EXPRESSION_LABEL), {
|
||||
target: { value: "bad cron" },
|
||||
});
|
||||
armFailure();
|
||||
fireEvent.click(screen.getByRole("button", { name: /create/i }));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
"The routine was created without a schedule: invalid cron expression",
|
||||
),
|
||||
);
|
||||
expect(toast.error).toHaveBeenCalledWith("Failed to load");
|
||||
});
|
||||
});
|
||||
@@ -22,6 +22,7 @@ import type {
|
||||
AgentProfile,
|
||||
RoutineRun,
|
||||
RoutineTrigger,
|
||||
CreateRoutineInput,
|
||||
} from "@/lib/state/slices/office/types";
|
||||
import { RoutineRow } from "./routine-row";
|
||||
import { RunRow } from "./run-row";
|
||||
@@ -29,6 +30,7 @@ import { CreateRoutineDialog } from "./create-routine-dialog";
|
||||
import { EmptyState } from "../components/shared/empty-state";
|
||||
import { routineNotFiringMessage } from "../lib/routine-not-firing";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import type { TFunction } from "i18next";
|
||||
|
||||
type RoutineFormData = {
|
||||
name: string;
|
||||
@@ -44,15 +46,38 @@ type RoutineFormData = {
|
||||
timezone: string;
|
||||
};
|
||||
|
||||
// A refetch failure after a create/trigger call must not swallow the toast
|
||||
// reporting that call's own outcome, and must not become an unhandled
|
||||
// rejection; it is reported as its own toast instead.
|
||||
async function refreshRoutinesOrReportFailure(
|
||||
fetchRoutines: () => Promise<void>,
|
||||
t: TFunction,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await fetchRoutines();
|
||||
} catch {
|
||||
toast.error(t("office:failedToLoad"));
|
||||
}
|
||||
}
|
||||
|
||||
function buildCreateRoutineInput(data: RoutineFormData): CreateRoutineInput {
|
||||
return {
|
||||
name: data.name,
|
||||
description: data.description,
|
||||
taskTemplate: { title: data.taskTitle, description: data.taskDescription },
|
||||
assigneeAgentProfileId: data.assigneeAgentProfileId,
|
||||
concurrencyPolicy: data.concurrencyPolicy,
|
||||
catchUpPolicy: data.catchUpPolicy,
|
||||
catchUpMax: data.catchUpMax,
|
||||
};
|
||||
}
|
||||
|
||||
function useRoutineActions(workspaceId: string | null, fetchRoutines: () => Promise<void>) {
|
||||
const { t } = useTranslation();
|
||||
const handleToggle = useCallback(
|
||||
async (id: string, active: boolean) => {
|
||||
try {
|
||||
await updateRoutine(id, { status: active ? "active" : "paused" } as Record<
|
||||
string,
|
||||
unknown
|
||||
>);
|
||||
await updateRoutine(id, { status: active ? "active" : "paused" });
|
||||
await fetchRoutines();
|
||||
toast.success(active ? t("office:routineActivated") : t("office:routinePaused"));
|
||||
} catch (err) {
|
||||
@@ -75,40 +100,44 @@ function useRoutineActions(workspaceId: string | null, fetchRoutines: () => Prom
|
||||
[fetchRoutines],
|
||||
);
|
||||
|
||||
// Returns whether the routine itself was created, which is also whether
|
||||
// the create dialog should close and reset its form: true covers both the
|
||||
// full-success and trigger-create-failed cases (a real routine now
|
||||
// exists), false only when createRoutine itself rejected (nothing to
|
||||
// close or reset — the user's input needs to stay for a retry).
|
||||
const handleCreate = useCallback(
|
||||
async (data: RoutineFormData, onDone: () => void) => {
|
||||
if (!workspaceId) return;
|
||||
async (data: RoutineFormData): Promise<boolean> => {
|
||||
if (!workspaceId) return false;
|
||||
let routine: Routine;
|
||||
try {
|
||||
const template = JSON.stringify({
|
||||
title: data.taskTitle,
|
||||
description: data.taskDescription,
|
||||
});
|
||||
const res = await createRoutine(workspaceId, {
|
||||
name: data.name,
|
||||
description: data.description,
|
||||
taskTemplate: JSON.parse(template),
|
||||
assigneeAgentProfileId: data.assigneeAgentProfileId,
|
||||
concurrencyPolicy: data.concurrencyPolicy,
|
||||
catchUpPolicy: data.catchUpPolicy,
|
||||
catchUpMax: data.catchUpMax,
|
||||
} as Record<string, unknown>);
|
||||
if (data.triggerKind === "cron" && data.cronExpression && res) {
|
||||
const routineObj = res as unknown as { routine?: { id: string } };
|
||||
const routineId = routineObj.routine?.id ?? (res as unknown as { id: string }).id;
|
||||
if (routineId) {
|
||||
await createRoutineTrigger(routineId, {
|
||||
kind: data.triggerKind as "cron",
|
||||
cronExpression: data.cronExpression,
|
||||
timezone: data.timezone,
|
||||
});
|
||||
}
|
||||
}
|
||||
onDone();
|
||||
await fetchRoutines();
|
||||
toast.success(t("office:routineCreated"));
|
||||
routine = await createRoutine(workspaceId, buildCreateRoutineInput(data));
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : t("office:failedToCreateRoutine"));
|
||||
return false;
|
||||
}
|
||||
|
||||
const cronExpression = data.cronExpression.trim();
|
||||
if (data.triggerKind === "cron" && cronExpression) {
|
||||
try {
|
||||
await createRoutineTrigger(routine.id, {
|
||||
kind: "cron",
|
||||
cronExpression,
|
||||
timezone: data.timezone,
|
||||
});
|
||||
} catch (err) {
|
||||
await refreshRoutinesOrReportFailure(fetchRoutines, t);
|
||||
toast.error(
|
||||
t("office:routineCreatedWithoutSchedule", {
|
||||
error: err instanceof Error ? err.message : t("office:failedToCreateRoutine"),
|
||||
}),
|
||||
);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
await refreshRoutinesOrReportFailure(fetchRoutines, t);
|
||||
toast.success(t("office:routineCreated"));
|
||||
return true;
|
||||
},
|
||||
[workspaceId, fetchRoutines],
|
||||
);
|
||||
@@ -249,7 +278,7 @@ export function RoutinesContent() {
|
||||
open={showCreate}
|
||||
onOpenChange={setShowCreate}
|
||||
agents={agents}
|
||||
onSubmit={(data) => handleCreate(data, () => setShowCreate(false))}
|
||||
onSubmit={handleCreate}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -565,7 +565,16 @@ export class OfficeApiClient {
|
||||
|
||||
async createRoutine(
|
||||
wsId: string,
|
||||
data: { name: string; description?: string; catch_up_policy?: string },
|
||||
data: {
|
||||
name: string;
|
||||
description?: string;
|
||||
catch_up_policy?: string;
|
||||
assignee_agent_profile_id?: string;
|
||||
concurrency_policy?: string;
|
||||
catch_up_max?: number;
|
||||
task_template?: string;
|
||||
variables?: string;
|
||||
},
|
||||
): Promise<Record<string, unknown>> {
|
||||
const res = await this.request<{ routine: Record<string, unknown> }>(
|
||||
"POST",
|
||||
@@ -585,6 +594,14 @@ export class OfficeApiClient {
|
||||
return this.rawRequest("POST", `/routines/${routineId}/run`);
|
||||
}
|
||||
|
||||
async listRoutineTriggers(routineId: string): Promise<Record<string, unknown>[]> {
|
||||
const res = await this.request<{ triggers: Record<string, unknown>[] | null }>(
|
||||
"GET",
|
||||
`/routines/${routineId}/triggers`,
|
||||
);
|
||||
return res.triggers ?? [];
|
||||
}
|
||||
|
||||
// --- Workspace pause (kill switch) ---
|
||||
|
||||
async getWorkspacePause(wsId: string): Promise<{
|
||||
|
||||
@@ -6,24 +6,18 @@ import { test, expect } from "../../fixtures/office-fixture";
|
||||
* `summarize_missed`, with the old value accepted forever as a deprecated
|
||||
* alias on write and normalized away on read.
|
||||
*
|
||||
* NOTE ON SCOPE: `apps/web/lib/state/slices/office/types.ts`'s `Routine`
|
||||
* type (pre-existing, unchanged by this diff) declares every multi-word
|
||||
* field in camelCase (`catchUpPolicy`, `catchUpMax`, `concurrencyPolicy`,
|
||||
* `assigneeAgentProfileId`, `taskTemplate`, `workspaceId`), but the backend
|
||||
* (`apps/backend/internal/office/models/models.go`) serializes and binds
|
||||
* all of them in snake_case. Neither the Create Routine dialog's submit
|
||||
* nor the detail view's Save round-trips any of those fields — they're
|
||||
* silently dropped on write and read back as `undefined` (falling through
|
||||
* to each field's hardcoded UI default) regardless of what's actually
|
||||
* persisted. This is pre-existing (present unchanged at the merge base)
|
||||
* and spans the whole Routines feature, not just catch-up policy — filed
|
||||
* as a follow-up, not fixed here. Consequence for this spec: the create
|
||||
* dialog's and detail view's catch-up-policy controls can only be proven
|
||||
* correct through pure client-side interaction (matching AC-003.8's own
|
||||
* "component test, not a flow" framing) — a real "seed via API, load the
|
||||
* page, expect the persisted policy to render" round trip is not currently
|
||||
* possible for either surface. The alias-normalization contract itself
|
||||
* (AC-003.1) is still verified for real over HTTP, via the API layer.
|
||||
* The catch-up-policy controls below are still proven through pure
|
||||
* client-side interaction (matching AC-003.8's own "component test, not a
|
||||
* flow" framing) — that needs no server round trip and stays as-is.
|
||||
*
|
||||
* HISTORICAL NOTE, now closed: `apps/web/lib/state/slices/office/types.ts`'s
|
||||
* `Routine` type declared every multi-word field in camelCase while the
|
||||
* backend bound and serialized all of them snake_case, so neither the
|
||||
* Create Routine dialog's submit nor the detail view's Save round-tripped
|
||||
* any of those fields, and a real "seed via API, load the page, expect the
|
||||
* persisted policy to render" test was not possible for either surface.
|
||||
* `docs/specs/office/requirements/routine-wire-contract.md` fixed the wire
|
||||
* boundary; the last test below is that round trip, now that it exists.
|
||||
*/
|
||||
|
||||
const RETIRED_LABEL = /enqueue.?missed.?with.?cap/i;
|
||||
@@ -128,4 +122,45 @@ test.describe("Routine catch-up policy UI", () => {
|
||||
await expect(testPage.getByText("E2E Catch-up Legacy Alias")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(testPage.getByText(RETIRED_LABEL)).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("detail view: seeded non-default catch-up policy renders and Save persists a change", async ({
|
||||
officeApi,
|
||||
officeSeed,
|
||||
testPage,
|
||||
}) => {
|
||||
// AC-OFFICE-ROUTINE-WIRE-003.1: seeded via the raw snake_case wire key,
|
||||
// read back through the fixed adapter rather than the hardcoded
|
||||
// `summarize_missed` UI default every routine rendered as before.
|
||||
const routine = (await officeApi.createRoutine(officeSeed.workspaceId, {
|
||||
name: "E2E Catch-up Wire Round Trip",
|
||||
catch_up_policy: "skip_missed",
|
||||
})) as { id: string };
|
||||
expect(routine.id).toBeTruthy();
|
||||
|
||||
await testPage.goto(`/office/routines/${routine.id}`);
|
||||
await expect(testPage.getByText("E2E Catch-up Wire Round Trip")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(catchUpPolicyCombobox(testPage)).toHaveText("Skip missed");
|
||||
await expect(testPage.getByText("Catch-up max", { exact: true })).toHaveCount(0);
|
||||
|
||||
// AC-OFFICE-ROUTINE-WIRE-001.4: Save round-trips the change back to the
|
||||
// server under the wire's snake_case key.
|
||||
await catchUpPolicyCombobox(testPage).click();
|
||||
await testPage.getByRole("option", { name: SUMMARIZE_MISSED_LABEL }).click();
|
||||
await testPage.getByRole("button", { name: "Save" }).click();
|
||||
// A successful save calls `router.refresh()` (`window.location.reload()`
|
||||
// in this SPA — pre-existing, unchanged by this capability), which races
|
||||
// the success toast off the page before it can be observed. Wait for the
|
||||
// reload and read the freshly-seeded form instead, which also proves the
|
||||
// read path round-trips what this save just wrote.
|
||||
await testPage.waitForLoadState("load");
|
||||
await expect(testPage.getByText("E2E Catch-up Wire Round Trip")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(catchUpPolicyCombobox(testPage)).toHaveText(SUMMARIZE_MISSED_LABEL);
|
||||
|
||||
const stored = await officeApi.getRoutine(routine.id);
|
||||
expect(stored["catch_up_policy"]).toBe("summarize_missed");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,29 @@
|
||||
import { type Page } from "@playwright/test";
|
||||
import { test, expect } from "../../fixtures/office-fixture";
|
||||
import { waitForHttp } from "../../helpers/causal-waits";
|
||||
|
||||
/**
|
||||
* Wire contract round trips (docs/specs/office/requirements/routine-wire-contract.md).
|
||||
* The web client and the Go DTOs disagreed on key case for every multi-word
|
||||
* routine/trigger field; these specs drive the real Create/Save UI against the
|
||||
* running backend and read the raw (snake_case) wire response back, which is
|
||||
* the honest round trip `routine-catch-up-policy-ui.spec.ts` could not do
|
||||
* before this capability landed.
|
||||
*/
|
||||
|
||||
// Field wrapper in routine-detail-view.tsx renders <Label> then the control as
|
||||
// a sibling, not a `for`/`id` pair, so `getByLabel` cannot resolve it there
|
||||
// (the create dialog's steps 0-1 controls do have real `id`s and use
|
||||
// `getByLabel` directly). This mirrors routine-catch-up-policy-ui.spec.ts's
|
||||
// own `catchUpPolicyCombobox` helper, generalized to any Field-wrapped control.
|
||||
function comboboxNear(page: Page, label: string) {
|
||||
return page.getByText(label, { exact: true }).locator("..").getByRole("combobox");
|
||||
}
|
||||
|
||||
function textboxNear(page: Page, label: string) {
|
||||
return page.getByText(label, { exact: true }).locator("..").getByRole("textbox");
|
||||
}
|
||||
|
||||
test.describe("Routines UI", () => {
|
||||
test("routine created via API appears in page", async ({ testPage, officeApi, officeSeed }) => {
|
||||
await officeApi.createRoutine(officeSeed.workspaceId, {
|
||||
@@ -64,4 +87,313 @@ test.describe("Routines UI", () => {
|
||||
timeout: 10_000,
|
||||
});
|
||||
});
|
||||
|
||||
test("create dialog persists assignee, policies, task template and arms a cron trigger", async ({
|
||||
testPage,
|
||||
officeApi,
|
||||
officeSeed,
|
||||
prCapture,
|
||||
}) => {
|
||||
const name = "E2E Wire Contract Create";
|
||||
await testPage.goto("/office/routines");
|
||||
await testPage.getByRole("button", { name: "New Routine" }).click();
|
||||
|
||||
await testPage.getByLabel("Name").fill(name);
|
||||
await testPage
|
||||
.getByText("Assignee", { exact: true })
|
||||
.locator("..")
|
||||
.getByRole("combobox")
|
||||
.click();
|
||||
await testPage.getByRole("option", { name: "CEO" }).click();
|
||||
await testPage.getByRole("button", { name: "Next" }).click();
|
||||
|
||||
await testPage.getByLabel("Task Title Template").fill("{{name}} wire check");
|
||||
await testPage.getByLabel("Task Description Template").fill("Verify the wire contract.");
|
||||
await testPage.getByRole("button", { name: "Next" }).click();
|
||||
|
||||
await comboboxNear(testPage, "Concurrency").click();
|
||||
await testPage.getByRole("option", { name: "Always create" }).click();
|
||||
await comboboxNear(testPage, "Catch-up policy").click();
|
||||
await testPage.getByRole("option", { name: "Skip missed" }).click();
|
||||
await testPage.getByLabel("Cron Expression").fill("*/5 * * * *");
|
||||
await testPage.getByLabel("Timezone").fill("America/New_York");
|
||||
|
||||
await prCapture.screenshot("create-dialog-policies-and-cron", {
|
||||
caption:
|
||||
"Create Routine dialog with assignee, concurrency/catch-up policy, task template and cron schedule filled in",
|
||||
});
|
||||
|
||||
// AC-OFFICE-ROUTINE-WIRE-002.1/.2: before this capability, the trigger
|
||||
// create call this arms was rejected outright (`cronExpression` bound to
|
||||
// "" -> `ErrInvalidTrigger` -> 400), so a cron schedule could not be
|
||||
// armed from the UI at all.
|
||||
const routineCreated = waitForHttp(testPage, "POST", /\/workspaces\/[^/]+\/routines$/);
|
||||
const triggerCreated = waitForHttp(testPage, "POST", /\/routines\/[^/]+\/triggers$/);
|
||||
await testPage.getByRole("button", { name: "Create" }).click();
|
||||
await routineCreated;
|
||||
await triggerCreated;
|
||||
await expect(testPage.getByText(name)).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
const listed = (await officeApi.listRoutines(officeSeed.workspaceId)) as {
|
||||
routines: Record<string, unknown>[];
|
||||
};
|
||||
const routine = listed.routines.find((r) => r.name === name);
|
||||
expect(routine).toBeTruthy();
|
||||
// AC-OFFICE-ROUTINE-WIRE-001.1: every multi-word key round-trips under
|
||||
// its snake_case wire spelling.
|
||||
expect(routine?.assignee_agent_profile_id).toBe(officeSeed.agentId);
|
||||
expect(routine?.concurrency_policy).toBe("always_create");
|
||||
expect(routine?.catch_up_policy).toBe("skip_missed");
|
||||
expect(JSON.parse(routine?.task_template as string)).toEqual({
|
||||
title: "{{name}} wire check",
|
||||
description: "Verify the wire contract.",
|
||||
});
|
||||
|
||||
const triggers = await officeApi.listRoutineTriggers(routine?.id as string);
|
||||
const cron = triggers.find((t) => t.kind === "cron");
|
||||
expect(cron).toBeTruthy();
|
||||
expect(cron?.cron_expression).toBe("*/5 * * * *");
|
||||
expect(cron?.timezone).toBe("America/New_York");
|
||||
expect(cron?.next_run_at).toBeTruthy();
|
||||
});
|
||||
|
||||
test("detail view save persists assignee, policies and arms a cron schedule", async ({
|
||||
testPage,
|
||||
officeApi,
|
||||
officeSeed,
|
||||
prCapture,
|
||||
}) => {
|
||||
const name = "E2E Wire Contract Detail Save";
|
||||
const routine = (await officeApi.createRoutine(officeSeed.workspaceId, { name })) as {
|
||||
id: string;
|
||||
};
|
||||
expect(routine.id).toBeTruthy();
|
||||
|
||||
await testPage.goto(`/office/routines/${routine.id}`);
|
||||
await expect(testPage.getByText(name)).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
await comboboxNear(testPage, "Assignee").click();
|
||||
await testPage.getByRole("option", { name: "CEO" }).click();
|
||||
await comboboxNear(testPage, "Concurrency policy").click();
|
||||
await testPage.getByRole("option", { name: "Always create" }).click();
|
||||
await comboboxNear(testPage, "Catch-up policy").click();
|
||||
await testPage.getByRole("option", { name: "Skip missed" }).click();
|
||||
await textboxNear(testPage, "Cron expression").fill("15 3 * * *");
|
||||
await textboxNear(testPage, "Timezone").fill("Europe/London");
|
||||
|
||||
const routineUpdated = waitForHttp(testPage, "PATCH", /\/routines\/[^/]+$/);
|
||||
const triggerCreated = waitForHttp(testPage, "POST", /\/routines\/[^/]+\/triggers$/);
|
||||
await testPage.getByRole("button", { name: "Save" }).click();
|
||||
await routineUpdated;
|
||||
await triggerCreated;
|
||||
// A successful save with a trigger change calls `router.refresh()`
|
||||
// (`window.location.reload()` in this SPA — pre-existing, unchanged by
|
||||
// this capability), which races the success toast off the page before
|
||||
// Playwright can observe it. Wait for that reload instead of the toast,
|
||||
// then read the freshly-seeded form: this is a stronger assertion than
|
||||
// the toast anyway, since it also proves the read path round-trips the
|
||||
// values this same save just wrote.
|
||||
await testPage.waitForLoadState("load");
|
||||
await expect(testPage.getByText(name)).toBeVisible({ timeout: 10_000 });
|
||||
await expect(comboboxNear(testPage, "Assignee")).toHaveText("CEO");
|
||||
await expect(comboboxNear(testPage, "Concurrency policy")).toHaveText("Always create");
|
||||
await expect(comboboxNear(testPage, "Catch-up policy")).toHaveText("Skip missed");
|
||||
await expect(textboxNear(testPage, "Cron expression")).toHaveValue("15 3 * * *");
|
||||
await expect(textboxNear(testPage, "Timezone")).toHaveValue("Europe/London");
|
||||
|
||||
await prCapture.screenshot("detail-view-persisted-schedule", {
|
||||
caption:
|
||||
"Routine detail view after save, showing the persisted assignee, policies and armed cron schedule",
|
||||
});
|
||||
|
||||
// AC-OFFICE-ROUTINE-WIRE-001.4: an update sends exactly the caller's
|
||||
// patch fields under their wire spelling, and the server stores them.
|
||||
const stored = await officeApi.getRoutine(routine.id);
|
||||
expect(stored.assignee_agent_profile_id).toBe(officeSeed.agentId);
|
||||
expect(stored.concurrency_policy).toBe("always_create");
|
||||
expect(stored.catch_up_policy).toBe("skip_missed");
|
||||
|
||||
// AC-OFFICE-ROUTINE-WIRE-002.5: the routine had no cron trigger yet, so
|
||||
// save creates one and deletes nothing.
|
||||
const triggers = await officeApi.listRoutineTriggers(routine.id);
|
||||
expect(triggers).toHaveLength(1);
|
||||
const cron = triggers.find((t) => t.kind === "cron");
|
||||
expect(cron).toBeTruthy();
|
||||
expect(cron?.cron_expression).toBe("15 3 * * *");
|
||||
expect(cron?.timezone).toBe("Europe/London");
|
||||
expect(cron?.next_run_at).toBeTruthy();
|
||||
});
|
||||
|
||||
test("list row and detail view render persisted assignee, policy and variables", async ({
|
||||
testPage,
|
||||
officeApi,
|
||||
officeSeed,
|
||||
prCapture,
|
||||
}) => {
|
||||
const name = "E2E Wire Contract Read";
|
||||
const routine = (await officeApi.createRoutine(officeSeed.workspaceId, {
|
||||
name,
|
||||
assignee_agent_profile_id: officeSeed.agentId,
|
||||
concurrency_policy: "always_create",
|
||||
catch_up_policy: "skip_missed",
|
||||
// Declared-variable wire shape (backend `parseDeclaredDefaults`,
|
||||
// apps/backend/internal/office/routines/service.go:1273): each entry
|
||||
// is `{ default: string }`, not a bare scalar.
|
||||
variables: JSON.stringify({ region: { default: "us-east" }, tier: { default: "gold" } }),
|
||||
})) as { id: string };
|
||||
expect(routine.id).toBeTruthy();
|
||||
|
||||
await testPage.goto("/office/routines");
|
||||
const row = testPage.getByTestId(`routine-row-${routine.id}`);
|
||||
await expect(row).toBeVisible({ timeout: 10_000 });
|
||||
// AC-OFFICE-ROUTINE-WIRE-003.1/.6: routine-row.tsx's own snake_case
|
||||
// fallbacks are gone; these now render through the normalized model.
|
||||
await expect(row.getByText("CEO", { exact: true })).toBeVisible();
|
||||
await expect(row.getByText("Always create", { exact: true })).toBeVisible();
|
||||
|
||||
// AC-OFFICE-ROUTINE-WIRE-003.11: one entry per declared variable name,
|
||||
// not one entry per character of the encoded JSON string. Also proves
|
||||
// Build round 3's fix: a declared variable's `{ default }` object
|
||||
// renders its default value, not the literal string "[object Object]".
|
||||
await row.click();
|
||||
await expect(testPage.getByText("region: us-east")).toBeVisible();
|
||||
await expect(testPage.getByText("tier: gold")).toBeVisible();
|
||||
await expect(testPage.getByText("[object Object]")).toHaveCount(0);
|
||||
await prCapture.screenshot("list-row-expanded-variables", {
|
||||
caption:
|
||||
"Routines list with an expanded row rendering the persisted assignee, policy and declared variables",
|
||||
});
|
||||
|
||||
await testPage.goto(`/office/routines/${routine.id}`);
|
||||
await expect(testPage.getByText(name)).toBeVisible({ timeout: 10_000 });
|
||||
await expect(comboboxNear(testPage, "Assignee")).toHaveText("CEO");
|
||||
await expect(comboboxNear(testPage, "Concurrency policy")).toHaveText("Always create");
|
||||
await expect(comboboxNear(testPage, "Catch-up policy")).toHaveText("Skip missed");
|
||||
});
|
||||
|
||||
// Review round 1 (Codex-2): handleCreate's post-create refetch used to be
|
||||
// unguarded, so a refetch failure became an unhandled rejection that
|
||||
// swallowed the create outcome toast instead of surfacing its own. Build
|
||||
// round 2 wrapped it in `refreshRoutinesOrReportFailure`; this proves the
|
||||
// fix end to end against the real component, not just the unit mock.
|
||||
test("a post-create refetch failure reports its own toast and still closes the dialog", async ({
|
||||
testPage,
|
||||
}) => {
|
||||
const name = "E2E Refetch Failure Routine";
|
||||
// React StrictMode double-invokes the page-mount effect in dev, so the
|
||||
// list endpoint sees two GET calls before any user interaction. Rather
|
||||
// than count calls, arm the failure right before the action whose
|
||||
// refetch this test targets, so it lands on that call regardless of how
|
||||
// many preceded it.
|
||||
let failNextList = false;
|
||||
await testPage.route("**/api/v1/office/workspaces/*/routines", async (route) => {
|
||||
const request = route.request();
|
||||
if (request.method() === "GET" && failNextList) {
|
||||
failNextList = false;
|
||||
await route.fulfill({ status: 500, contentType: "application/json", body: "{}" });
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
await testPage.goto("/office/routines");
|
||||
await testPage.getByRole("button", { name: "New Routine" }).click();
|
||||
|
||||
await testPage.getByLabel("Name").fill(name);
|
||||
await testPage
|
||||
.getByText("Assignee", { exact: true })
|
||||
.locator("..")
|
||||
.getByRole("combobox")
|
||||
.click();
|
||||
await testPage.getByRole("option", { name: "CEO" }).click();
|
||||
await testPage.getByRole("button", { name: "Next" }).click();
|
||||
await testPage.getByRole("button", { name: "Next" }).click();
|
||||
|
||||
// Webhook has no required fields at this step, so the create call carries
|
||||
// no trigger and this test isolates the routine-list refetch alone.
|
||||
await comboboxNear(testPage, "Trigger Type").click();
|
||||
await testPage.getByRole("option", { name: "Webhook" }).click();
|
||||
|
||||
const routineCreated = waitForHttp(testPage, "POST", /\/workspaces\/[^/]+\/routines$/);
|
||||
failNextList = true;
|
||||
const listFailed = waitForHttp(testPage, "GET", /\/workspaces\/[^/]+\/routines$/, {
|
||||
predicate: (r) => r.status() === 500,
|
||||
});
|
||||
await testPage.getByRole("button", { name: "Create" }).click();
|
||||
await routineCreated;
|
||||
await listFailed;
|
||||
|
||||
await expect(testPage.getByText("Routine created")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(testPage.getByText("Failed to load")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(testPage.getByRole("dialog")).toHaveCount(0);
|
||||
expect(failNextList).toBe(false);
|
||||
});
|
||||
|
||||
test("manual fire renders the real creation time in the Runs tab", async ({
|
||||
testPage,
|
||||
officeApi,
|
||||
officeSeed,
|
||||
}) => {
|
||||
const routine = (await officeApi.createRoutine(officeSeed.workspaceId, {
|
||||
name: "E2E Wire Contract Run",
|
||||
})) as { id: string };
|
||||
expect(routine.id).toBeTruthy();
|
||||
|
||||
const fireResponse = await officeApi.runRoutine(routine.id);
|
||||
expect(fireResponse.ok).toBe(true);
|
||||
const fired = (await fireResponse.json()) as { run: { id: string; created_at: string } };
|
||||
expect(fired.run.created_at).toBeTruthy();
|
||||
|
||||
await testPage.goto("/office/routines");
|
||||
await testPage.getByRole("tab", { name: "Runs" }).click();
|
||||
// officeSeed's workspace is reset per test, so exactly one run exists.
|
||||
const runsList = testPage.locator(".rounded-lg.divide-y > div");
|
||||
await expect(runsList).toHaveCount(1, { timeout: 10_000 });
|
||||
// AC-OFFICE-ROUTINE-WIRE-004.1/.3: `created_at` now reaches the model,
|
||||
// so run-row.tsx's `formatTime` renders the real timestamp instead of
|
||||
// the "--" placeholder it showed for every run before this capability.
|
||||
await expect(runsList).toContainText(new Date(fired.run.created_at).toLocaleString());
|
||||
});
|
||||
|
||||
// Review round 3 (Codex-1): before Build round 4's fix,
|
||||
// `isRoutineFiring(draft.status ?? "")` treated any unrecognized
|
||||
// persisted status the same as the backend's genuinely empty "no writer
|
||||
// set one" default, showing a next-fire countdown the routine can never
|
||||
// reach. `UpdateRoutineRequest.Status` has no enum validation
|
||||
// (`handler.go:353-354`: `routine.Status = *req.Status`), so an
|
||||
// unrecognized value is reachable through the real API, not just a
|
||||
// hand-built object.
|
||||
test("an unrecognized persisted status hides the next-fire countdown", async ({
|
||||
testPage,
|
||||
officeApi,
|
||||
officeSeed,
|
||||
}) => {
|
||||
const name = "E2E Wire Contract Unrecognized Status";
|
||||
const routine = (await officeApi.createRoutine(officeSeed.workspaceId, { name })) as {
|
||||
id: string;
|
||||
};
|
||||
expect(routine.id).toBeTruthy();
|
||||
|
||||
const triggerCreated = await officeApi.rawRequest("POST", `/routines/${routine.id}/triggers`, {
|
||||
kind: "cron",
|
||||
cron_expression: "*/5 * * * *",
|
||||
timezone: "UTC",
|
||||
});
|
||||
expect(triggerCreated.ok).toBe(true);
|
||||
const triggers = await officeApi.listRoutineTriggers(routine.id);
|
||||
const cron = triggers.find((t) => t.kind === "cron") as { next_run_at?: string } | undefined;
|
||||
expect(cron?.next_run_at).toBeTruthy();
|
||||
|
||||
const statusUpdated = await officeApi.rawRequest("PATCH", `/routines/${routine.id}`, {
|
||||
status: "draft",
|
||||
});
|
||||
expect(statusUpdated.ok).toBe(true);
|
||||
|
||||
await testPage.goto(`/office/routines/${routine.id}`);
|
||||
await expect(testPage.getByText(name)).toBeVisible({ timeout: 10_000 });
|
||||
await expect(testPage.getByText("Next fire: -")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(
|
||||
testPage.getByText(new Date(cron!.next_run_at as string).toLocaleString()),
|
||||
).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,9 +4,6 @@ import type {
|
||||
AgentProfile,
|
||||
Project,
|
||||
CostSummary,
|
||||
Routine,
|
||||
RoutineTrigger,
|
||||
RoutineRun,
|
||||
Approval,
|
||||
InboxItem,
|
||||
OfficeMeta,
|
||||
@@ -64,6 +61,19 @@ export {
|
||||
gitPush,
|
||||
} from "./office-extended-api";
|
||||
export { listBudgets, createBudget, updateBudget, deleteBudget } from "./office-budget-api";
|
||||
export {
|
||||
listRoutines,
|
||||
createRoutine,
|
||||
getRoutine,
|
||||
updateRoutine,
|
||||
deleteRoutine,
|
||||
runRoutine,
|
||||
listRoutineTriggers,
|
||||
createRoutineTrigger,
|
||||
deleteRoutineTrigger,
|
||||
listRoutineRuns,
|
||||
listAllRoutineRuns,
|
||||
} from "./office-routine-api";
|
||||
export type {
|
||||
ImportDiff,
|
||||
ImportPreview,
|
||||
@@ -452,92 +462,6 @@ export function setDefaultCeiling(
|
||||
});
|
||||
}
|
||||
|
||||
// --- Routines ---
|
||||
|
||||
export function listRoutines(workspaceId: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<{ routines: Routine[] }>(`${BASE}/workspaces/${workspaceId}/routines`, options);
|
||||
}
|
||||
|
||||
export function createRoutine(
|
||||
workspaceId: string,
|
||||
data: Partial<Routine>,
|
||||
options?: ApiRequestOptions,
|
||||
) {
|
||||
return fetchJson<Routine>(`${BASE}/workspaces/${workspaceId}/routines`, {
|
||||
...options,
|
||||
init: { method: "POST", body: JSON.stringify(data), ...options?.init },
|
||||
});
|
||||
}
|
||||
|
||||
export function getRoutine(id: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<Routine>(`${BASE}/routines/${id}`, options);
|
||||
}
|
||||
|
||||
export function updateRoutine(id: string, data: Partial<Routine>, options?: ApiRequestOptions) {
|
||||
return fetchJson<Routine>(`${BASE}/routines/${id}`, {
|
||||
...options,
|
||||
init: { method: "PATCH", body: JSON.stringify(data), ...options?.init },
|
||||
});
|
||||
}
|
||||
|
||||
export function deleteRoutine(id: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<void>(`${BASE}/routines/${id}`, {
|
||||
...options,
|
||||
init: { method: "DELETE", ...options?.init },
|
||||
});
|
||||
}
|
||||
|
||||
export function runRoutine(
|
||||
id: string,
|
||||
variables?: Record<string, string>,
|
||||
options?: ApiRequestOptions,
|
||||
) {
|
||||
return fetchJson<{ run: RoutineRun }>(`${BASE}/routines/${id}/run`, {
|
||||
...options,
|
||||
init: {
|
||||
method: "POST",
|
||||
body: variables ? JSON.stringify({ variables }) : undefined,
|
||||
...options?.init,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
export function listRoutineTriggers(routineId: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<{ triggers: RoutineTrigger[] }>(
|
||||
`${BASE}/routines/${routineId}/triggers`,
|
||||
options,
|
||||
);
|
||||
}
|
||||
|
||||
export function createRoutineTrigger(
|
||||
routineId: string,
|
||||
data: Partial<RoutineTrigger>,
|
||||
options?: ApiRequestOptions,
|
||||
) {
|
||||
return fetchJson<{ trigger: RoutineTrigger }>(`${BASE}/routines/${routineId}/triggers`, {
|
||||
...options,
|
||||
init: { method: "POST", body: JSON.stringify(data), ...options?.init },
|
||||
});
|
||||
}
|
||||
|
||||
export function deleteRoutineTrigger(triggerId: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<void>(`${BASE}/routine-triggers/${triggerId}`, {
|
||||
...options,
|
||||
init: { method: "DELETE", ...options?.init },
|
||||
});
|
||||
}
|
||||
|
||||
export function listRoutineRuns(routineId: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<{ runs: RoutineRun[] }>(`${BASE}/routines/${routineId}/runs`, options);
|
||||
}
|
||||
|
||||
export function listAllRoutineRuns(workspaceId: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<{ runs: RoutineRun[] }>(
|
||||
`${BASE}/workspaces/${workspaceId}/routine-runs`,
|
||||
options,
|
||||
);
|
||||
}
|
||||
|
||||
// --- Approvals ---
|
||||
|
||||
export function listApprovals(workspaceId: string, options?: ApiRequestOptions) {
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { getRoutine, listRoutines } from "./office-routine-api";
|
||||
|
||||
const fetchSpy = vi.fn<typeof fetch>();
|
||||
const API_BASE_URL = "http://api.test";
|
||||
const WORKSPACE_ID = "workspace-1";
|
||||
const ROUTINE_ID = "routine-1";
|
||||
const MALFORMED_ROUTINE_ERROR = 'office API: malformed "routine" response';
|
||||
|
||||
function jsonResponse(body: unknown, status = 200) {
|
||||
return new Response(JSON.stringify(body), {
|
||||
status,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy.mockReset();
|
||||
vi.stubGlobal("fetch", fetchSpy);
|
||||
});
|
||||
|
||||
afterEach(() => vi.unstubAllGlobals());
|
||||
|
||||
// AC-003.5: unwrapEnvelopeList takes the missing/null/non-array directions to
|
||||
// an empty list rather than throwing, so a malformed collection envelope
|
||||
// degrades to "nothing to show" instead of crashing the page.
|
||||
describe("listRoutines envelope unwrap (AC-003.5)", () => {
|
||||
it("returns an empty list when the envelope key is missing", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({}));
|
||||
await expect(listRoutines(WORKSPACE_ID, { baseUrl: API_BASE_URL })).resolves.toEqual({
|
||||
routines: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns an empty list when the envelope key is null", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routines: null }));
|
||||
await expect(listRoutines(WORKSPACE_ID, { baseUrl: API_BASE_URL })).resolves.toEqual({
|
||||
routines: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns an empty list when the envelope key is not an array", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routines: "not-an-array" }));
|
||||
await expect(listRoutines(WORKSPACE_ID, { baseUrl: API_BASE_URL })).resolves.toEqual({
|
||||
routines: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns an empty list when the response body itself is not an object", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse(null));
|
||||
await expect(listRoutines(WORKSPACE_ID, { baseUrl: API_BASE_URL })).resolves.toEqual({
|
||||
routines: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("passes a well-formed list through to normalization", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routines: [{ id: "r1", name: "Sweep" }] }));
|
||||
const result = await listRoutines(WORKSPACE_ID, { baseUrl: API_BASE_URL });
|
||||
expect(result.routines).toHaveLength(1);
|
||||
expect(result.routines[0].id).toBe("r1");
|
||||
});
|
||||
});
|
||||
|
||||
// AC-003.15: unwrapEnvelopeResource takes the missing/null/non-object/no-id
|
||||
// directions to a thrown error instead of returning a value the caller would
|
||||
// silently skip past — this is the exact silent-skip AC-003.15 exists to
|
||||
// prevent.
|
||||
describe("getRoutine envelope unwrap (AC-003.15)", () => {
|
||||
it("throws when the envelope key is missing", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({}));
|
||||
await expect(getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL })).rejects.toThrow(
|
||||
MALFORMED_ROUTINE_ERROR,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the envelope key is null", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routine: null }));
|
||||
await expect(getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL })).rejects.toThrow(
|
||||
MALFORMED_ROUTINE_ERROR,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the envelope key is not an object", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routine: "not-an-object" }));
|
||||
await expect(getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL })).rejects.toThrow(
|
||||
MALFORMED_ROUTINE_ERROR,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the envelope key is an array", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routine: [] }));
|
||||
await expect(getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL })).rejects.toThrow(
|
||||
MALFORMED_ROUTINE_ERROR,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the resource has no id field", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routine: { name: "Sweep" } }));
|
||||
await expect(getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL })).rejects.toThrow(
|
||||
MALFORMED_ROUTINE_ERROR,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the resource's id is an empty string", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routine: { id: "" } }));
|
||||
await expect(getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL })).rejects.toThrow(
|
||||
MALFORMED_ROUTINE_ERROR,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the resource's id is not a string", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routine: { id: 123 } }));
|
||||
await expect(getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL })).rejects.toThrow(
|
||||
MALFORMED_ROUTINE_ERROR,
|
||||
);
|
||||
});
|
||||
|
||||
it("returns the normalized resource when it has a non-empty string id", async () => {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse({ routine: { id: "r1", name: "Sweep" } }));
|
||||
const result = await getRoutine(ROUTINE_ID, { baseUrl: API_BASE_URL });
|
||||
expect(result.id).toBe("r1");
|
||||
expect(result.name).toBe("Sweep");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,154 @@
|
||||
import { fetchJson, type ApiRequestOptions } from "../client";
|
||||
import type {
|
||||
Routine,
|
||||
RoutineTrigger,
|
||||
RoutineRun,
|
||||
CreateRoutineInput,
|
||||
UpdateRoutinePatch,
|
||||
CreateTriggerInput,
|
||||
} from "@/lib/state/slices/office/types";
|
||||
import {
|
||||
buildCreateRoutineBody,
|
||||
buildCreateTriggerBody,
|
||||
buildUpdateRoutineBody,
|
||||
normalizeRoutine,
|
||||
normalizeRoutineList,
|
||||
normalizeRoutineRun,
|
||||
normalizeRoutineRunList,
|
||||
normalizeRoutineTrigger,
|
||||
normalizeRoutineTriggerList,
|
||||
} from "./office-routine-normalize";
|
||||
|
||||
const BASE = "/api/v1/office";
|
||||
|
||||
/** A body that is not a plain object has no envelope key and takes the same branch as a missing one. */
|
||||
function unwrapEnvelopeList(body: unknown, key: string): unknown[] {
|
||||
if (!body || typeof body !== "object" || Array.isArray(body)) return [];
|
||||
const value = (body as Record<string, unknown>)[key];
|
||||
return Array.isArray(value) ? value : [];
|
||||
}
|
||||
|
||||
// i18n-exempt: defensive diagnostic for a response shape the documented
|
||||
// routine wire contract guarantees will not occur; not a message a normal
|
||||
// user flow can trigger.
|
||||
function unwrapEnvelopeResource(body: unknown, key: string): Record<string, unknown> {
|
||||
const container =
|
||||
body && typeof body === "object" && !Array.isArray(body)
|
||||
? (body as Record<string, unknown>)
|
||||
: undefined;
|
||||
const value = container?.[key];
|
||||
const isObject =
|
||||
value !== undefined && value !== null && typeof value === "object" && !Array.isArray(value);
|
||||
const id = isObject ? (value as Record<string, unknown>).id : undefined;
|
||||
if (!isObject || typeof id !== "string" || id === "") {
|
||||
throw new Error(`office API: malformed "${key}" response`);
|
||||
}
|
||||
return value as Record<string, unknown>;
|
||||
}
|
||||
|
||||
export async function listRoutines(
|
||||
workspaceId: string,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<{ routines: Routine[] }> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/workspaces/${workspaceId}/routines`, options);
|
||||
return { routines: normalizeRoutineList(unwrapEnvelopeList(res, "routines")) };
|
||||
}
|
||||
|
||||
export async function createRoutine(
|
||||
workspaceId: string,
|
||||
data: CreateRoutineInput,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<Routine> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/workspaces/${workspaceId}/routines`, {
|
||||
...options,
|
||||
init: { method: "POST", body: JSON.stringify(buildCreateRoutineBody(data)), ...options?.init },
|
||||
});
|
||||
return normalizeRoutine(unwrapEnvelopeResource(res, "routine"));
|
||||
}
|
||||
|
||||
export async function getRoutine(id: string, options?: ApiRequestOptions): Promise<Routine> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/routines/${id}`, options);
|
||||
return normalizeRoutine(unwrapEnvelopeResource(res, "routine"));
|
||||
}
|
||||
|
||||
export async function updateRoutine(
|
||||
id: string,
|
||||
patch: UpdateRoutinePatch,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<Routine> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/routines/${id}`, {
|
||||
...options,
|
||||
init: {
|
||||
method: "PATCH",
|
||||
body: JSON.stringify(buildUpdateRoutineBody(patch)),
|
||||
...options?.init,
|
||||
},
|
||||
});
|
||||
return normalizeRoutine(unwrapEnvelopeResource(res, "routine"));
|
||||
}
|
||||
|
||||
export function deleteRoutine(id: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<void>(`${BASE}/routines/${id}`, {
|
||||
...options,
|
||||
init: { method: "DELETE", ...options?.init },
|
||||
});
|
||||
}
|
||||
|
||||
export async function runRoutine(
|
||||
id: string,
|
||||
variables?: Record<string, string>,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<RoutineRun> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/routines/${id}/run`, {
|
||||
...options,
|
||||
init: {
|
||||
method: "POST",
|
||||
body: variables ? JSON.stringify({ variables }) : undefined,
|
||||
...options?.init,
|
||||
},
|
||||
});
|
||||
return normalizeRoutineRun(unwrapEnvelopeResource(res, "run"));
|
||||
}
|
||||
|
||||
export async function listRoutineTriggers(
|
||||
routineId: string,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<{ triggers: RoutineTrigger[] }> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/routines/${routineId}/triggers`, options);
|
||||
return { triggers: normalizeRoutineTriggerList(unwrapEnvelopeList(res, "triggers")) };
|
||||
}
|
||||
|
||||
export async function createRoutineTrigger(
|
||||
routineId: string,
|
||||
data: CreateTriggerInput,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<RoutineTrigger> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/routines/${routineId}/triggers`, {
|
||||
...options,
|
||||
init: { method: "POST", body: JSON.stringify(buildCreateTriggerBody(data)), ...options?.init },
|
||||
});
|
||||
return normalizeRoutineTrigger(unwrapEnvelopeResource(res, "trigger"));
|
||||
}
|
||||
|
||||
export function deleteRoutineTrigger(triggerId: string, options?: ApiRequestOptions) {
|
||||
return fetchJson<void>(`${BASE}/routine-triggers/${triggerId}`, {
|
||||
...options,
|
||||
init: { method: "DELETE", ...options?.init },
|
||||
});
|
||||
}
|
||||
|
||||
export async function listRoutineRuns(
|
||||
routineId: string,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<{ runs: RoutineRun[] }> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/routines/${routineId}/runs`, options);
|
||||
return { runs: normalizeRoutineRunList(unwrapEnvelopeList(res, "runs")) };
|
||||
}
|
||||
|
||||
export async function listAllRoutineRuns(
|
||||
workspaceId: string,
|
||||
options?: ApiRequestOptions,
|
||||
): Promise<{ runs: RoutineRun[] }> {
|
||||
const res = await fetchJson<unknown>(`${BASE}/workspaces/${workspaceId}/routine-runs`, options);
|
||||
return { runs: normalizeRoutineRunList(unwrapEnvelopeList(res, "runs")) };
|
||||
}
|
||||
@@ -0,0 +1,331 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
buildCreateRoutineBody,
|
||||
buildCreateTriggerBody,
|
||||
buildUpdateRoutineBody,
|
||||
normalizeRoutine,
|
||||
normalizeRoutineList,
|
||||
normalizeRoutineRun,
|
||||
normalizeRoutineRunList,
|
||||
normalizeRoutineTrigger,
|
||||
normalizeRoutineTriggerList,
|
||||
} from "./office-routine-normalize";
|
||||
|
||||
const ROUTINE_NAME = "Daily sweep";
|
||||
const LAST_RUN_AT = "2026-09-16T00:00:00Z";
|
||||
const CREATED_AT = "2026-09-01T00:00:00Z";
|
||||
const UPDATED_AT = "2026-09-02T00:00:00Z";
|
||||
const RUN_TIMESTAMP = "2026-09-17T00:00:00Z";
|
||||
|
||||
describe("normalizeRoutine", () => {
|
||||
it("maps the backend routine wire shape to the web model", () => {
|
||||
expect(
|
||||
normalizeRoutine({
|
||||
id: "r1",
|
||||
workspace_id: "w1",
|
||||
name: ROUTINE_NAME,
|
||||
description: "runs every day",
|
||||
task_template: '{"title":"Sweep"}',
|
||||
assignee_agent_profile_id: "agent-1",
|
||||
status: "active",
|
||||
concurrency_policy: "coalesce_if_active",
|
||||
catch_up_policy: "skip_if_active",
|
||||
catch_up_max: 10,
|
||||
variables: '{"env":"prod"}',
|
||||
last_run_at: LAST_RUN_AT,
|
||||
created_at: CREATED_AT,
|
||||
updated_at: UPDATED_AT,
|
||||
}),
|
||||
).toEqual({
|
||||
id: "r1",
|
||||
workspaceId: "w1",
|
||||
name: ROUTINE_NAME,
|
||||
description: "runs every day",
|
||||
taskTemplate: { title: "Sweep" },
|
||||
assigneeAgentProfileId: "agent-1",
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
catchUpPolicy: "skip_if_active",
|
||||
catchUpMax: 10,
|
||||
variables: { env: "prod" },
|
||||
lastRunAt: LAST_RUN_AT,
|
||||
createdAt: CREATED_AT,
|
||||
updatedAt: UPDATED_AT,
|
||||
});
|
||||
});
|
||||
|
||||
it("produces empty string for required strings and undefined for optional fields when absent", () => {
|
||||
const routine = normalizeRoutine({ id: "r1" });
|
||||
expect(routine.name).toBe("");
|
||||
expect(routine.status).toBe("");
|
||||
expect(routine.concurrencyPolicy).toBe("");
|
||||
expect(routine.description).toBeUndefined();
|
||||
expect(routine.assigneeAgentProfileId).toBeUndefined();
|
||||
expect(routine.catchUpPolicy).toBeUndefined();
|
||||
expect(routine.catchUpMax).toBeUndefined();
|
||||
expect(routine.variables).toBeUndefined();
|
||||
expect(routine.lastRunAt).toBeUndefined();
|
||||
});
|
||||
|
||||
it("produces an empty object for task_template when absent, null, or malformed", () => {
|
||||
expect(normalizeRoutine({}).taskTemplate).toEqual({});
|
||||
expect(normalizeRoutine({ task_template: null }).taskTemplate).toEqual({});
|
||||
expect(normalizeRoutine({ task_template: "" }).taskTemplate).toEqual({});
|
||||
expect(normalizeRoutine({ task_template: "not json" }).taskTemplate).toEqual({});
|
||||
expect(normalizeRoutine({ task_template: "[1,2]" }).taskTemplate).toEqual({});
|
||||
expect(normalizeRoutine({ task_template: "3" }).taskTemplate).toEqual({});
|
||||
});
|
||||
|
||||
it("never clamps a catch_up_max the server sent, even below 1", () => {
|
||||
expect(normalizeRoutine({ catch_up_max: 0 }).catchUpMax).toBe(0);
|
||||
expect(normalizeRoutine({ catch_up_max: -5 }).catchUpMax).toBe(-5);
|
||||
});
|
||||
|
||||
it("does not substitute a policy value the server did not send", () => {
|
||||
const routine = normalizeRoutine({ concurrency_policy: undefined, catch_up_policy: undefined });
|
||||
expect(routine.concurrencyPolicy).toBe("");
|
||||
expect(routine.catchUpPolicy).toBeUndefined();
|
||||
});
|
||||
|
||||
it("produces an empty object for a present-but-malformed variables string, undefined only when absent", () => {
|
||||
expect(normalizeRoutine({ variables: "not json" }).variables).toEqual({});
|
||||
expect(normalizeRoutine({ variables: "" }).variables).toEqual({});
|
||||
expect(normalizeRoutine({}).variables).toBeUndefined();
|
||||
expect(normalizeRoutine({ variables: null }).variables).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeRoutineTrigger", () => {
|
||||
it("maps the backend trigger wire shape to the web model, excluding secret", () => {
|
||||
expect(
|
||||
normalizeRoutineTrigger({
|
||||
id: "t1",
|
||||
routine_id: "r1",
|
||||
kind: "cron",
|
||||
cron_expression: "0 * * * *",
|
||||
timezone: "UTC",
|
||||
public_id: "pub-1",
|
||||
signing_mode: "hmac",
|
||||
secret: "super-secret",
|
||||
next_run_at: "2026-09-17T01:00:00Z",
|
||||
last_fired_at: "2026-09-16T01:00:00Z",
|
||||
enabled: true,
|
||||
created_at: CREATED_AT,
|
||||
updated_at: UPDATED_AT,
|
||||
}),
|
||||
).toEqual({
|
||||
id: "t1",
|
||||
routineId: "r1",
|
||||
kind: "cron",
|
||||
cronExpression: "0 * * * *",
|
||||
timezone: "UTC",
|
||||
publicId: "pub-1",
|
||||
signingMode: "hmac",
|
||||
nextRunAt: "2026-09-17T01:00:00Z",
|
||||
lastFiredAt: "2026-09-16T01:00:00Z",
|
||||
enabled: true,
|
||||
createdAt: CREATED_AT,
|
||||
updatedAt: UPDATED_AT,
|
||||
});
|
||||
});
|
||||
|
||||
it("defaults enabled to false and leaves optional fields undefined when absent", () => {
|
||||
const trigger = normalizeRoutineTrigger({ id: "t1" });
|
||||
expect(trigger.enabled).toBe(false);
|
||||
expect(trigger.cronExpression).toBeUndefined();
|
||||
expect(trigger.timezone).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeRoutineRun", () => {
|
||||
it("maps the backend run wire shape to the web model", () => {
|
||||
expect(
|
||||
normalizeRoutineRun({
|
||||
id: "run1",
|
||||
routine_id: "r1",
|
||||
trigger_id: "t1",
|
||||
source: "cron",
|
||||
status: "done",
|
||||
trigger_payload: "{}",
|
||||
linked_task_id: "task-1",
|
||||
coalesced_into_run_id: "run0",
|
||||
dispatch_fingerprint: "fp-1",
|
||||
catch_up_missed_ticks: 3,
|
||||
catch_up_first_missed_at: LAST_RUN_AT,
|
||||
catch_up_truncated: true,
|
||||
started_at: RUN_TIMESTAMP,
|
||||
completed_at: "2026-09-17T00:01:00Z",
|
||||
created_at: RUN_TIMESTAMP,
|
||||
}),
|
||||
).toEqual({
|
||||
id: "run1",
|
||||
routineId: "r1",
|
||||
triggerId: "t1",
|
||||
source: "cron",
|
||||
status: "done",
|
||||
triggerPayload: "{}",
|
||||
linkedTaskId: "task-1",
|
||||
coalescedIntoRunId: "run0",
|
||||
dispatchFingerprint: "fp-1",
|
||||
catchUpMissedTicks: 3,
|
||||
catchUpFirstMissedAt: LAST_RUN_AT,
|
||||
catchUpTruncated: true,
|
||||
startedAt: RUN_TIMESTAMP,
|
||||
completedAt: "2026-09-17T00:01:00Z",
|
||||
createdAt: RUN_TIMESTAMP,
|
||||
});
|
||||
});
|
||||
|
||||
it("leaves catch_up_missed_ticks undefined rather than 0 when absent", () => {
|
||||
const run = normalizeRoutineRun({ id: "run1" });
|
||||
expect(run.catchUpMissedTicks).toBeUndefined();
|
||||
expect(run.catchUpTruncated).toBeUndefined();
|
||||
});
|
||||
|
||||
it("normalizes an absent status and source to an empty string, not a cast", () => {
|
||||
const run = normalizeRoutineRun({ id: "run1" });
|
||||
expect(run.status).toBe("");
|
||||
expect(run.source).toBe("");
|
||||
});
|
||||
});
|
||||
|
||||
describe("list normalizers", () => {
|
||||
it("drop non-object elements and preserve order", () => {
|
||||
const raw = [{ id: "a" }, null, "garbage", 42, { id: "b" }];
|
||||
expect(normalizeRoutineList(raw).map((r) => r.id)).toEqual(["a", "b"]);
|
||||
expect(normalizeRoutineTriggerList(raw).map((r) => r.id)).toEqual(["a", "b"]);
|
||||
expect(normalizeRoutineRunList(raw).map((r) => r.id)).toEqual(["a", "b"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildCreateRoutineBody", () => {
|
||||
it("sends only the supplied fields, JSON-encoding taskTemplate and variables", () => {
|
||||
expect(
|
||||
buildCreateRoutineBody({
|
||||
name: ROUTINE_NAME,
|
||||
taskTemplate: { title: "Sweep" },
|
||||
}),
|
||||
).toEqual({
|
||||
name: ROUTINE_NAME,
|
||||
task_template: '{"title":"Sweep"}',
|
||||
});
|
||||
});
|
||||
|
||||
it("omits concurrency_policy and catch_up_policy when not supplied", () => {
|
||||
const body = buildCreateRoutineBody({ name: "n" });
|
||||
expect(body).not.toHaveProperty("concurrency_policy");
|
||||
expect(body).not.toHaveProperty("catch_up_policy");
|
||||
});
|
||||
|
||||
it("omits a catch_up_max below 1 rather than sending it", () => {
|
||||
expect(buildCreateRoutineBody({ name: "n", catchUpMax: 0 })).not.toHaveProperty("catch_up_max");
|
||||
expect(buildCreateRoutineBody({ name: "n", catchUpMax: -3 })).not.toHaveProperty(
|
||||
"catch_up_max",
|
||||
);
|
||||
expect(buildCreateRoutineBody({ name: "n", catchUpMax: 10 })).toHaveProperty(
|
||||
"catch_up_max",
|
||||
10,
|
||||
);
|
||||
});
|
||||
|
||||
it("does not send workspace_id or status", () => {
|
||||
const body = buildCreateRoutineBody({ name: "n" });
|
||||
expect(body).not.toHaveProperty("workspace_id");
|
||||
expect(body).not.toHaveProperty("status");
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildUpdateRoutineBody", () => {
|
||||
it("omits a key the caller did not supply", () => {
|
||||
const body = buildUpdateRoutineBody({ name: "renamed" });
|
||||
expect(body).toEqual({ name: "renamed" });
|
||||
});
|
||||
|
||||
it("transmits an empty string for name, description, taskTemplate, assigneeAgentProfileId, and variables to clear them", () => {
|
||||
const body = buildUpdateRoutineBody({
|
||||
name: "",
|
||||
description: "",
|
||||
taskTemplate: "",
|
||||
assigneeAgentProfileId: "",
|
||||
variables: "",
|
||||
});
|
||||
expect(body).toEqual({
|
||||
name: "",
|
||||
description: "",
|
||||
task_template: "",
|
||||
assignee_agent_profile_id: "",
|
||||
variables: "",
|
||||
});
|
||||
});
|
||||
|
||||
it("never sends an empty status", () => {
|
||||
expect(buildUpdateRoutineBody({ status: "" })).not.toHaveProperty("status");
|
||||
expect(buildUpdateRoutineBody({})).not.toHaveProperty("status");
|
||||
expect(buildUpdateRoutineBody({ status: "paused" })).toHaveProperty("status", "paused");
|
||||
});
|
||||
|
||||
it("omits an empty concurrencyPolicy or catchUpPolicy rather than clearing it", () => {
|
||||
const body = buildUpdateRoutineBody({ concurrencyPolicy: "", catchUpPolicy: "" });
|
||||
expect(body).not.toHaveProperty("concurrency_policy");
|
||||
expect(body).not.toHaveProperty("catch_up_policy");
|
||||
});
|
||||
|
||||
it("omits a catch_up_max below 1 rather than sending it", () => {
|
||||
expect(buildUpdateRoutineBody({ catchUpMax: 0 })).not.toHaveProperty("catch_up_max");
|
||||
expect(buildUpdateRoutineBody({ catchUpMax: 25 })).toHaveProperty("catch_up_max", 25);
|
||||
});
|
||||
|
||||
it("sends only the fields status admits, plus the general set", () => {
|
||||
const body = buildUpdateRoutineBody({
|
||||
name: "n",
|
||||
description: "d",
|
||||
taskTemplate: { a: 1 },
|
||||
assigneeAgentProfileId: "agent-1",
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
catchUpPolicy: "skip_if_active",
|
||||
catchUpMax: 10,
|
||||
variables: { env: "prod" },
|
||||
});
|
||||
expect(Object.keys(body).sort()).toEqual(
|
||||
[
|
||||
"name",
|
||||
"description",
|
||||
"task_template",
|
||||
"assignee_agent_profile_id",
|
||||
"status",
|
||||
"concurrency_policy",
|
||||
"catch_up_policy",
|
||||
"catch_up_max",
|
||||
"variables",
|
||||
].sort(),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildCreateTriggerBody", () => {
|
||||
it("sends kind and the supplied optional fields under their wire keys", () => {
|
||||
expect(
|
||||
buildCreateTriggerBody({
|
||||
kind: "cron",
|
||||
cronExpression: "0 * * * *",
|
||||
timezone: "UTC",
|
||||
}),
|
||||
).toEqual({
|
||||
kind: "cron",
|
||||
cron_expression: "0 * * * *",
|
||||
timezone: "UTC",
|
||||
});
|
||||
});
|
||||
|
||||
it("trims the cron expression before sending it", () => {
|
||||
expect(buildCreateTriggerBody({ kind: "cron", cronExpression: " 0 * * * * " })).toEqual({
|
||||
kind: "cron",
|
||||
cron_expression: "0 * * * *",
|
||||
});
|
||||
});
|
||||
|
||||
it("omits fields the caller did not supply", () => {
|
||||
expect(buildCreateTriggerBody({ kind: "webhook" })).toEqual({ kind: "webhook" });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,182 @@
|
||||
import type {
|
||||
CreateRoutineInput,
|
||||
CreateTriggerInput,
|
||||
Routine,
|
||||
RoutineRun,
|
||||
RoutineTrigger,
|
||||
UpdateRoutinePatch,
|
||||
} from "@/lib/state/slices/office/types";
|
||||
|
||||
type WireRecord = Record<string, unknown>;
|
||||
|
||||
function asRecord(raw: unknown): WireRecord {
|
||||
return raw && typeof raw === "object" && !Array.isArray(raw) ? (raw as WireRecord) : {};
|
||||
}
|
||||
|
||||
function str(record: WireRecord, key: string): string {
|
||||
const value = record[key];
|
||||
return typeof value === "string" ? value : "";
|
||||
}
|
||||
|
||||
function optStr(record: WireRecord, key: string): string | undefined {
|
||||
const value = record[key];
|
||||
return typeof value === "string" ? value : undefined;
|
||||
}
|
||||
|
||||
function optNum(record: WireRecord, key: string): number | undefined {
|
||||
const value = record[key];
|
||||
return typeof value === "number" ? value : undefined;
|
||||
}
|
||||
|
||||
function optBool(record: WireRecord, key: string): boolean | undefined {
|
||||
const value = record[key];
|
||||
return typeof value === "boolean" ? value : undefined;
|
||||
}
|
||||
|
||||
function isJSONObject(value: unknown): value is WireRecord {
|
||||
return value !== null && typeof value === "object" && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/** Parses a required JSON-object-encoded field: absent, malformed, or non-object decodes to {}. */
|
||||
function parseRequiredJSONObject(value: unknown): Record<string, unknown> {
|
||||
if (typeof value !== "string" || !value) return {};
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(value);
|
||||
return isJSONObject(parsed) ? parsed : {};
|
||||
} catch {
|
||||
return {};
|
||||
}
|
||||
}
|
||||
|
||||
/** Parses an optional JSON-object-encoded field: absent/null is undefined; a present string follows AC-003.4. */
|
||||
function parseOptionalJSONObject(value: unknown): Record<string, unknown> | undefined {
|
||||
if (value === undefined || value === null) return undefined;
|
||||
return parseRequiredJSONObject(value);
|
||||
}
|
||||
|
||||
/** Serializes a write-side task_template/variables value: object -> JSON text, string passes through, undefined stays undefined. */
|
||||
function stringifyJSONField(
|
||||
value: Record<string, unknown> | string | undefined,
|
||||
): string | undefined {
|
||||
if (value === undefined) return undefined;
|
||||
return typeof value === "string" ? value : JSON.stringify(value);
|
||||
}
|
||||
|
||||
function assignIfDefined(body: WireRecord, key: string, value: unknown): void {
|
||||
if (value !== undefined) body[key] = value;
|
||||
}
|
||||
|
||||
export function normalizeRoutine(raw: unknown): Routine {
|
||||
const r = asRecord(raw);
|
||||
return {
|
||||
id: str(r, "id"),
|
||||
workspaceId: str(r, "workspace_id"),
|
||||
name: str(r, "name"),
|
||||
description: optStr(r, "description"),
|
||||
taskTemplate: parseRequiredJSONObject(r.task_template),
|
||||
assigneeAgentProfileId: optStr(r, "assignee_agent_profile_id"),
|
||||
status: str(r, "status"),
|
||||
concurrencyPolicy: str(r, "concurrency_policy"),
|
||||
catchUpPolicy: optStr(r, "catch_up_policy"),
|
||||
catchUpMax: optNum(r, "catch_up_max"),
|
||||
variables: parseOptionalJSONObject(r.variables),
|
||||
lastRunAt: optStr(r, "last_run_at"),
|
||||
createdAt: str(r, "created_at"),
|
||||
updatedAt: str(r, "updated_at"),
|
||||
};
|
||||
}
|
||||
|
||||
export function normalizeRoutineTrigger(raw: unknown): RoutineTrigger {
|
||||
const r = asRecord(raw);
|
||||
return {
|
||||
id: str(r, "id"),
|
||||
routineId: str(r, "routine_id"),
|
||||
kind: str(r, "kind"),
|
||||
cronExpression: optStr(r, "cron_expression"),
|
||||
timezone: optStr(r, "timezone"),
|
||||
publicId: optStr(r, "public_id"),
|
||||
signingMode: optStr(r, "signing_mode"),
|
||||
nextRunAt: optStr(r, "next_run_at"),
|
||||
lastFiredAt: optStr(r, "last_fired_at"),
|
||||
enabled: Boolean(r.enabled === true),
|
||||
createdAt: str(r, "created_at"),
|
||||
updatedAt: str(r, "updated_at"),
|
||||
};
|
||||
}
|
||||
|
||||
export function normalizeRoutineRun(raw: unknown): RoutineRun {
|
||||
const r = asRecord(raw);
|
||||
return {
|
||||
id: str(r, "id"),
|
||||
routineId: str(r, "routine_id"),
|
||||
triggerId: optStr(r, "trigger_id"),
|
||||
source: str(r, "source"),
|
||||
status: str(r, "status"),
|
||||
triggerPayload: optStr(r, "trigger_payload"),
|
||||
linkedTaskId: optStr(r, "linked_task_id"),
|
||||
coalescedIntoRunId: optStr(r, "coalesced_into_run_id"),
|
||||
dispatchFingerprint: optStr(r, "dispatch_fingerprint"),
|
||||
catchUpMissedTicks: optNum(r, "catch_up_missed_ticks"),
|
||||
catchUpFirstMissedAt: optStr(r, "catch_up_first_missed_at"),
|
||||
catchUpTruncated: optBool(r, "catch_up_truncated"),
|
||||
startedAt: optStr(r, "started_at"),
|
||||
completedAt: optStr(r, "completed_at"),
|
||||
createdAt: str(r, "created_at"),
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeList<T>(raw: unknown[], normalize: (raw: unknown) => T): T[] {
|
||||
return raw.filter(isJSONObject).map(normalize);
|
||||
}
|
||||
|
||||
export function normalizeRoutineList(raw: unknown[]): Routine[] {
|
||||
return normalizeList(raw, normalizeRoutine);
|
||||
}
|
||||
|
||||
export function normalizeRoutineTriggerList(raw: unknown[]): RoutineTrigger[] {
|
||||
return normalizeList(raw, normalizeRoutineTrigger);
|
||||
}
|
||||
|
||||
export function normalizeRoutineRunList(raw: unknown[]): RoutineRun[] {
|
||||
return normalizeList(raw, normalizeRoutineRun);
|
||||
}
|
||||
|
||||
export function buildCreateRoutineBody(input: CreateRoutineInput): WireRecord {
|
||||
const body: WireRecord = { name: input.name };
|
||||
assignIfDefined(body, "description", input.description);
|
||||
assignIfDefined(body, "task_template", stringifyJSONField(input.taskTemplate));
|
||||
assignIfDefined(body, "assignee_agent_profile_id", input.assigneeAgentProfileId);
|
||||
assignIfDefined(body, "concurrency_policy", input.concurrencyPolicy);
|
||||
assignIfDefined(body, "catch_up_policy", input.catchUpPolicy);
|
||||
if (input.catchUpMax !== undefined && input.catchUpMax >= 1) {
|
||||
body.catch_up_max = input.catchUpMax;
|
||||
}
|
||||
assignIfDefined(body, "variables", stringifyJSONField(input.variables));
|
||||
return body;
|
||||
}
|
||||
|
||||
export function buildUpdateRoutineBody(patch: UpdateRoutinePatch): WireRecord {
|
||||
const body: WireRecord = {};
|
||||
assignIfDefined(body, "name", patch.name);
|
||||
assignIfDefined(body, "description", patch.description);
|
||||
assignIfDefined(body, "task_template", stringifyJSONField(patch.taskTemplate));
|
||||
assignIfDefined(body, "assignee_agent_profile_id", patch.assigneeAgentProfileId);
|
||||
if (patch.status) body.status = patch.status;
|
||||
assignIfDefined(body, "variables", stringifyJSONField(patch.variables));
|
||||
if (patch.concurrencyPolicy) body.concurrency_policy = patch.concurrencyPolicy;
|
||||
if (patch.catchUpPolicy) body.catch_up_policy = patch.catchUpPolicy;
|
||||
if (patch.catchUpMax !== undefined && patch.catchUpMax >= 1) {
|
||||
body.catch_up_max = patch.catchUpMax;
|
||||
}
|
||||
return body;
|
||||
}
|
||||
|
||||
export function buildCreateTriggerBody(input: CreateTriggerInput): WireRecord {
|
||||
const body: WireRecord = { kind: input.kind };
|
||||
assignIfDefined(body, "cron_expression", input.cronExpression?.trim());
|
||||
assignIfDefined(body, "timezone", input.timezone);
|
||||
assignIfDefined(body, "public_id", input.publicId);
|
||||
assignIfDefined(body, "signing_mode", input.signingMode);
|
||||
assignIfDefined(body, "secret", input.secret);
|
||||
return body;
|
||||
}
|
||||
@@ -157,7 +157,9 @@ export type Routine = {
|
||||
description?: string;
|
||||
taskTemplate: Record<string, unknown>;
|
||||
assigneeAgentProfileId?: string;
|
||||
status: RoutineStatus;
|
||||
// The server can store any status string; only the three named
|
||||
// RoutineStatus values are selectable from the detail form.
|
||||
status: string;
|
||||
concurrencyPolicy: string;
|
||||
catchUpPolicy?: string;
|
||||
catchUpMax?: number;
|
||||
@@ -180,7 +182,8 @@ export type RoutineRunStatus =
|
||||
export type RoutineTrigger = {
|
||||
id: string;
|
||||
routineId: string;
|
||||
kind: RoutineTriggerKind;
|
||||
// The server can store any trigger kind; only "cron" is creatable from the UI today.
|
||||
kind: string;
|
||||
cronExpression?: string;
|
||||
timezone?: string;
|
||||
publicId?: string;
|
||||
@@ -197,7 +200,8 @@ export type RoutineRun = {
|
||||
routineId: string;
|
||||
triggerId?: string;
|
||||
source: string;
|
||||
status: RoutineRunStatus;
|
||||
// The server can store any run status string.
|
||||
status: string;
|
||||
triggerPayload?: string;
|
||||
linkedTaskId?: string;
|
||||
coalescedIntoRunId?: string;
|
||||
@@ -213,6 +217,46 @@ export type RoutineRun = {
|
||||
createdAt: string;
|
||||
};
|
||||
|
||||
/** Fields the client can supply when creating a routine (POST body, camelCase). */
|
||||
export type CreateRoutineInput = {
|
||||
name: string;
|
||||
description?: string;
|
||||
taskTemplate?: Record<string, unknown> | string;
|
||||
assigneeAgentProfileId?: string;
|
||||
concurrencyPolicy?: string;
|
||||
catchUpPolicy?: string;
|
||||
catchUpMax?: number;
|
||||
variables?: Record<string, unknown> | string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Fields the client can patch on a routine (PATCH body, camelCase). Absent
|
||||
* keys are omitted from the request and preserve the stored value; empty
|
||||
* strings clear the field, except concurrencyPolicy/catchUpPolicy (omitted
|
||||
* rather than cleared) and status (never sent empty).
|
||||
*/
|
||||
export type UpdateRoutinePatch = {
|
||||
name?: string;
|
||||
description?: string;
|
||||
taskTemplate?: Record<string, unknown> | string;
|
||||
assigneeAgentProfileId?: string;
|
||||
status?: RoutineStatus | string;
|
||||
concurrencyPolicy?: string;
|
||||
catchUpPolicy?: string;
|
||||
catchUpMax?: number;
|
||||
variables?: Record<string, unknown> | string;
|
||||
};
|
||||
|
||||
/** Fields the client can supply when creating a trigger (POST body, camelCase). */
|
||||
export type CreateTriggerInput = {
|
||||
kind: RoutineTriggerKind;
|
||||
cronExpression?: string;
|
||||
timezone?: string;
|
||||
publicId?: string;
|
||||
signingMode?: string;
|
||||
secret?: string;
|
||||
};
|
||||
|
||||
export type InboxItemType =
|
||||
| "approval"
|
||||
| "budget_alert"
|
||||
|
||||
@@ -722,6 +722,7 @@
|
||||
"routeOutcomeSkippedUserAction": "skipped user action",
|
||||
"routineActivated": "Routine activated",
|
||||
"routineCreated": "Routine created",
|
||||
"routineCreatedWithoutSchedule": "The routine was created without a schedule: {{error}}",
|
||||
"routineDeleted": "Routine deleted",
|
||||
"routineFired": "Routine fired",
|
||||
"routineNotFiring": "Cannot run: routine status is {{status}}",
|
||||
@@ -729,6 +730,11 @@
|
||||
"routines": "Routines",
|
||||
"routinesAutomaticallyCreateTasksOnA": "Routines automatically create tasks on a schedule or webhook trigger.",
|
||||
"routineSaved": "Routine saved",
|
||||
"routineSavedScheduleChangeFailed": "The routine was saved, but the schedule change failed: {{error}}",
|
||||
"routineSavedScheduleDeleteFailed_one": "The routine was saved, but removing the old schedule failed: {{error}}. {{count}} cron schedule is now active.",
|
||||
"routineSavedScheduleDeleteFailed_other": "The routine was saved, but removing the old schedule failed: {{error}}. {{count}} cron schedules are now active.",
|
||||
"routineSavedScheduleMayBeStale": "The routine was saved. The displayed schedule may be stale until you reload the page.",
|
||||
"routineScheduleFateUnknown": "The routine was saved, but the schedule's fate is unknown. Reload the page to see the current state.",
|
||||
"routineStarted": "Routine started",
|
||||
"routineStatusActive": "Active",
|
||||
"routineStatusArchived": "Archived",
|
||||
|
||||
@@ -722,6 +722,7 @@
|
||||
"routeOutcomeSkippedUserAction": "śķĩƥƥēď ũśēŕ àćţĩōń",
|
||||
"routineActivated": "Ŕōũţĩńē àćţĩvàţēď",
|
||||
"routineCreated": "Ŕōũţĩńē ćŕēàţēď",
|
||||
"routineCreatedWithoutSchedule": "Ţĥē ŕōũţĩńē ŵàś ćŕēàţēď ŵĩţĥōũţ à śćĥēďũĺē: {{error}}",
|
||||
"routineDeleted": "Ŕōũţĩńē ďēĺēţēď",
|
||||
"routineFired": "Ŕōũţĩńē ƒĩŕēď",
|
||||
"routineNotFiring": "Ćàńńōţ ŕũń: ŕōũţĩńē śţàţũś ĩś {{status}}",
|
||||
@@ -729,6 +730,11 @@
|
||||
"routines": "Ŕōũţĩńēś",
|
||||
"routinesAutomaticallyCreateTasksOnA": "Ŕōũţĩńēś àũţōḿàţĩćàĺĺŷ ćŕēàţē ţàśķś ōń à śćĥēďũĺē ōŕ ŵēƀĥōōķ ţŕĩĝĝēŕ.",
|
||||
"routineSaved": "Ŕōũţĩńē śàvēď",
|
||||
"routineSavedScheduleChangeFailed": "Ţĥē ŕōũţĩńē ŵàś śàvēď, ƀũţ ţĥē śćĥēďũĺē ćĥàńĝē ƒàĩĺēď: {{error}}",
|
||||
"routineSavedScheduleDeleteFailed_one": "Ţĥē ŕōũţĩńē ŵàś śàvēď, ƀũţ ŕēḿōvĩńĝ ţĥē ōĺď śćĥēďũĺē ƒàĩĺēď: {{error}}. {{count}} ćŕōń śćĥēďũĺē ĩś ńōŵ àćţĩvē.",
|
||||
"routineSavedScheduleDeleteFailed_other": "Ţĥē ŕōũţĩńē ŵàś śàvēď, ƀũţ ŕēḿōvĩńĝ ţĥē ōĺď śćĥēďũĺē ƒàĩĺēď: {{error}}. {{count}} ćŕōń śćĥēďũĺēś àŕē ńōŵ àćţĩvē.",
|
||||
"routineSavedScheduleMayBeStale": "Ţĥē ŕōũţĩńē ŵàś śàvēď. Ţĥē ďĩśƥĺàŷēď śćĥēďũĺē ḿàŷ ƀē śţàĺē ũńţĩĺ ŷōũ ŕēĺōàď ţĥē ƥàĝē.",
|
||||
"routineScheduleFateUnknown": "Ţĥē ŕōũţĩńē ŵàś śàvēď, ƀũţ ţĥē śćĥēďũĺē'ś ƒàţē ĩś ũńķńōŵń. Ŕēĺōàď ţĥē ƥàĝē ţō śēē ţĥē ćũŕŕēńţ śţàţē.",
|
||||
"routineStarted": "Ŕōũţĩńē śţàŕţēď",
|
||||
"routineStatusActive": "Àćţĩvē",
|
||||
"routineStatusArchived": "Àŕćĥĩvēď",
|
||||
|
||||
@@ -722,6 +722,7 @@
|
||||
"routeOutcomeSkippedUserAction": "ignorado por ação do utilizador",
|
||||
"routineActivated": "Rotina ativada",
|
||||
"routineCreated": "Rotina criada",
|
||||
"routineCreatedWithoutSchedule": "A rotina foi criada sem agendamento: {{error}}",
|
||||
"routineDeleted": "Rotina eliminada",
|
||||
"routineFired": "Rotina disparada",
|
||||
"routineNotFiring": "Não é possível executar: o estado da rotina é {{status}}",
|
||||
@@ -729,6 +730,11 @@
|
||||
"routines": "Rotinas",
|
||||
"routinesAutomaticallyCreateTasksOnA": "As rotinas criam tarefas automaticamente com base num agendamento ou num acionador de webhook.",
|
||||
"routineSaved": "Rotina guardada",
|
||||
"routineSavedScheduleChangeFailed": "A rotina foi guardada, mas a alteração do agendamento falhou: {{error}}",
|
||||
"routineSavedScheduleDeleteFailed_one": "A rotina foi guardada, mas a remoção do agendamento anterior falhou: {{error}}. {{count}} agendamento cron está agora ativo.",
|
||||
"routineSavedScheduleDeleteFailed_other": "A rotina foi guardada, mas a remoção do agendamento anterior falhou: {{error}}. {{count}} agendamentos cron estão agora ativos.",
|
||||
"routineSavedScheduleMayBeStale": "A rotina foi guardada. O agendamento apresentado pode estar desatualizado até recarregar a página.",
|
||||
"routineScheduleFateUnknown": "A rotina foi guardada, mas o estado do agendamento é desconhecido. Recarregue a página para ver o estado atual.",
|
||||
"routineStarted": "Rotina iniciada",
|
||||
"routineStatusActive": "Ativa",
|
||||
"routineStatusArchived": "Arquivada",
|
||||
|
||||
@@ -722,6 +722,7 @@
|
||||
"routeOutcomeSkippedUserAction": "已跳过(用户操作)",
|
||||
"routineActivated": "例行程序已激活",
|
||||
"routineCreated": "例行程序已创建",
|
||||
"routineCreatedWithoutSchedule": "例行程序已创建,但未设置计划:{{error}}",
|
||||
"routineDeleted": "例行程序已删除",
|
||||
"routineFired": "例行程序已触发",
|
||||
"routineNotFiring": "无法运行:例行程序状态为 {{status}}",
|
||||
@@ -729,6 +730,11 @@
|
||||
"routines": "例行程序",
|
||||
"routinesAutomaticallyCreateTasksOnA": "例行程序按计划或 webhook 触发自动创建任务。",
|
||||
"routineSaved": "例行程序已保存",
|
||||
"routineSavedScheduleChangeFailed": "例行程序已保存,但计划变更失败:{{error}}",
|
||||
"routineSavedScheduleDeleteFailed_one": "例行程序已保存,但删除旧计划失败:{{error}}。目前有 {{count}} 个 cron 计划处于活动状态。",
|
||||
"routineSavedScheduleDeleteFailed_other": "例行程序已保存,但删除旧计划失败:{{error}}。目前有 {{count}} 个 cron 计划处于活动状态。",
|
||||
"routineSavedScheduleMayBeStale": "例行程序已保存。显示的计划可能已过期,请重新加载页面。",
|
||||
"routineScheduleFateUnknown": "例行程序已保存,但计划的状态未知。请重新加载页面以查看当前状态。",
|
||||
"routineStarted": "例行程序已启动",
|
||||
"routineStatusActive": "活动",
|
||||
"routineStatusArchived": "已归档",
|
||||
|
||||
@@ -722,6 +722,7 @@
|
||||
"routeOutcomeSkippedUserAction": "已跳過(用戶操作)",
|
||||
"routineActivated": "例行程序已激活",
|
||||
"routineCreated": "例行程序已建立",
|
||||
"routineCreatedWithoutSchedule": "例行程序已建立,但未設定計劃:{{error}}",
|
||||
"routineDeleted": "例行程序已刪除",
|
||||
"routineFired": "例行程序已觸發",
|
||||
"routineNotFiring": "無法執行:例行程序狀態為 {{status}}",
|
||||
@@ -729,6 +730,11 @@
|
||||
"routines": "例行程序",
|
||||
"routinesAutomaticallyCreateTasksOnA": "例行程序按計劃或 webhook 觸發自動建立任務。",
|
||||
"routineSaved": "例行程序已儲存",
|
||||
"routineSavedScheduleChangeFailed": "例行程序已儲存,但計劃變更失敗:{{error}}",
|
||||
"routineSavedScheduleDeleteFailed_one": "例行程序已儲存,但刪除舊計劃失敗:{{error}}。目前有 {{count}} 個 cron 計劃處於活動狀態。",
|
||||
"routineSavedScheduleDeleteFailed_other": "例行程序已儲存,但刪除舊計劃失敗:{{error}}。目前有 {{count}} 個 cron 計劃處於活動狀態。",
|
||||
"routineSavedScheduleMayBeStale": "例行程序已儲存。顯示的計劃可能已過期,請重新載入頁面。",
|
||||
"routineScheduleFateUnknown": "例行程序已儲存,但計劃的狀態未知。請重新載入頁面以查看目前狀態。",
|
||||
"routineStarted": "例行程序已啓動",
|
||||
"routineStatusActive": "活動",
|
||||
"routineStatusArchived": "已歸檔",
|
||||
|
||||
@@ -722,6 +722,7 @@
|
||||
"routeOutcomeSkippedUserAction": "已跳過(使用者操作)",
|
||||
"routineActivated": "例行程式已啟用",
|
||||
"routineCreated": "例行程式已建立",
|
||||
"routineCreatedWithoutSchedule": "例行程式已建立,但未設定計劃:{{error}}",
|
||||
"routineDeleted": "例行程式已刪除",
|
||||
"routineFired": "例行程式已觸發",
|
||||
"routineNotFiring": "無法執行:例行程式狀態為 {{status}}",
|
||||
@@ -729,6 +730,11 @@
|
||||
"routines": "例行程式",
|
||||
"routinesAutomaticallyCreateTasksOnA": "例行程式按計劃或 webhook 觸發自動建立任務。",
|
||||
"routineSaved": "例行程式已儲存",
|
||||
"routineSavedScheduleChangeFailed": "例行程式已儲存,但計劃變更失敗:{{error}}",
|
||||
"routineSavedScheduleDeleteFailed_one": "例行程式已儲存,但刪除舊計劃失敗:{{error}}。目前有 {{count}} 個 cron 計劃處於活動狀態。",
|
||||
"routineSavedScheduleDeleteFailed_other": "例行程式已儲存,但刪除舊計劃失敗:{{error}}。目前有 {{count}} 個 cron 計劃處於活動狀態。",
|
||||
"routineSavedScheduleMayBeStale": "例行程式已儲存。顯示的計劃可能已過期,請重新載入頁面。",
|
||||
"routineScheduleFateUnknown": "例行程式已儲存,但計劃的狀態未知。請重新載入頁面以檢視目前狀態。",
|
||||
"routineStarted": "例行程式已啟動",
|
||||
"routineStatusActive": "活動",
|
||||
"routineStatusArchived": "已歸檔",
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, render, screen, waitFor } from "@testing-library/react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { defaultOfficeState } from "@/lib/state/slices/office/office-slice";
|
||||
import type { Routine, RoutineTrigger } from "@/lib/state/slices/office/types";
|
||||
import { RoutineDetailRoute } from "./office-routine-client-routes";
|
||||
|
||||
vi.mock("@/lib/routing/client-router", () => ({
|
||||
useRouter: () => ({ refresh: vi.fn(), push: vi.fn(), replace: vi.fn() }),
|
||||
}));
|
||||
|
||||
vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
|
||||
|
||||
const { getRoutine, listRoutineTriggers } = vi.hoisted(() => ({
|
||||
getRoutine: vi.fn(),
|
||||
listRoutineTriggers: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/domains/office-api", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/lib/api/domains/office-api")>(
|
||||
"@/lib/api/domains/office-api",
|
||||
);
|
||||
return { ...actual, getRoutine, listRoutineTriggers };
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
const WORKSPACE_ID = "ws-1";
|
||||
const TIMESTAMP = "2026-05-04T00:00:00Z";
|
||||
const ROUTINE_ID = "routine-1";
|
||||
|
||||
const routine: Routine = {
|
||||
id: ROUTINE_ID,
|
||||
workspaceId: WORKSPACE_ID,
|
||||
name: "Nightly sync",
|
||||
taskTemplate: {},
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
};
|
||||
|
||||
const armedCronTrigger: RoutineTrigger = {
|
||||
id: "trigger-1",
|
||||
routineId: ROUTINE_ID,
|
||||
kind: "cron",
|
||||
cronExpression: "*/5 * * * *",
|
||||
timezone: "UTC",
|
||||
nextRunAt: "2026-05-05T00:00:00Z",
|
||||
enabled: true,
|
||||
createdAt: TIMESTAMP,
|
||||
updatedAt: TIMESTAMP,
|
||||
};
|
||||
|
||||
function renderRoute() {
|
||||
return render(
|
||||
<StateProvider
|
||||
initialState={{
|
||||
workspaces: { activeId: WORKSPACE_ID, items: [] },
|
||||
office: { ...defaultOfficeState.office },
|
||||
}}
|
||||
>
|
||||
<RoutineDetailRoute routineId={ROUTINE_ID} />
|
||||
</StateProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("RoutineDetailRoute — trigger-list load failure", () => {
|
||||
it("does not silently render as if the routine had no trigger when the trigger list fails to load", async () => {
|
||||
getRoutine.mockResolvedValue(routine);
|
||||
listRoutineTriggers.mockRejectedValue(new Error("network down"));
|
||||
|
||||
renderRoute();
|
||||
|
||||
await waitFor(() => expect(screen.getByText("network down")).toBeTruthy());
|
||||
// The routine's own detail form (and its trigger-reconciliation state,
|
||||
// which would otherwise treat the already-armed cron trigger as absent
|
||||
// and let a later Save create a duplicate) must never mount on this path.
|
||||
expect(screen.queryByDisplayValue(routine.name)).toBeNull();
|
||||
});
|
||||
|
||||
it("still renders the routine's real trigger when both loads succeed", async () => {
|
||||
getRoutine.mockResolvedValue(routine);
|
||||
listRoutineTriggers.mockResolvedValue({ triggers: [armedCronTrigger] });
|
||||
|
||||
renderRoute();
|
||||
|
||||
await waitFor(() => expect(screen.getByDisplayValue(routine.name)).toBeTruthy());
|
||||
expect(screen.getByDisplayValue(armedCronTrigger.cronExpression ?? "")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
@@ -19,16 +19,17 @@ export function RoutineDetailRoute({ routineId }: { routineId: string }) {
|
||||
setState({ status: "loading" });
|
||||
|
||||
async function loadRoutineDetail(): Promise<RoutineDetailData> {
|
||||
// A failed trigger list must not silently become "no trigger exists":
|
||||
// save-time reconciliation (cron-reconcile.ts) trusts this initial list
|
||||
// to detect an already-armed cron trigger, so swallowing a failure here
|
||||
// would make a later Save create a duplicate schedule instead of
|
||||
// replacing the one that failed to load. Let it fail the whole load
|
||||
// instead, same as a `getRoutine` failure below.
|
||||
const [routineResponse, triggersResponse] = await Promise.all([
|
||||
getRoutine(routineId, { cache: "no-store" }),
|
||||
listRoutineTriggers(routineId, { cache: "no-store" }).catch(() => ({
|
||||
triggers: [] as RoutineTrigger[],
|
||||
})),
|
||||
listRoutineTriggers(routineId, { cache: "no-store" }),
|
||||
]);
|
||||
const routine =
|
||||
(routineResponse as unknown as { routine?: Routine }).routine ??
|
||||
(routineResponse as unknown as Routine);
|
||||
return { routine, triggers: triggersResponse.triggers ?? [] };
|
||||
return { routine: routineResponse, triggers: triggersResponse.triggers ?? [] };
|
||||
}
|
||||
|
||||
loadRoutineDetail()
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
---
|
||||
created: 2026-09-17
|
||||
status: done
|
||||
requirements:
|
||||
- REQ-OFFICE-ROUTINE-WIRE-001
|
||||
- REQ-OFFICE-ROUTINE-WIRE-002
|
||||
- REQ-OFFICE-ROUTINE-WIRE-003
|
||||
- REQ-OFFICE-ROUTINE-WIRE-004
|
||||
system_design:
|
||||
- ../../specs/office/system-design/routine-wire-contract.md
|
||||
---
|
||||
|
||||
# Implementation plan: Office routine wire contract (camelCase/snake_case adapter)
|
||||
|
||||
## Overview
|
||||
|
||||
The Office routine HTTP contract is snake_case in both directions
|
||||
(`apps/backend/internal/office/routines/dto.go`), while the web client sent
|
||||
and read camelCase. Go's `encoding/json` matches case-insensitively but does
|
||||
not fold underscores, so every multi-word key was silently discarded on
|
||||
write, a cron trigger could not be armed at all, and reads rendered blanks,
|
||||
placeholders, or the characters of a JSON-encoded string one per line. This
|
||||
plan closes that gap with a single adapter at the client's API boundary that
|
||||
translates in both directions, so no HTTP contract, Go struct tag, database
|
||||
column, or scheduler behavior changes.
|
||||
|
||||
## Scope
|
||||
|
||||
In scope: a create/update/trigger request builder that emits the exact
|
||||
snake_case keys the backend binds, JSON-encoding `task_template` and
|
||||
`variables` as strings (REQ-001); a create-routine and detail-view control
|
||||
flow that arms, replaces, and reports on a cron trigger correctly (REQ-002);
|
||||
a response adapter that unwraps envelopes, decodes JSON-encoded string
|
||||
fields, and produces camelCase models with no snake_case fallback casts and
|
||||
no `as unknown as` (REQ-003); and the same normalization applied to routine
|
||||
runs, including the previously-missing `dispatchFingerprint`, `linkedTaskId`
|
||||
and `createdAt` (REQ-004).
|
||||
|
||||
Out of scope: any HTTP contract, Go struct tag, database column, or
|
||||
scheduler behavior change; clearing an armed cron schedule; enqueueing more
|
||||
than one run per resumed tick; and the pre-existing `AgentRunSummary` wire
|
||||
shape consumed by `app/office/agents/[id]/`, which this capability does not
|
||||
own (system design, [Out of scope](../../specs/office/system-design/routine-wire-contract.md#out-of-scope)).
|
||||
|
||||
## Technical approach
|
||||
|
||||
`apps/web/lib/api/domains/office-routine-api.ts` builds every create/update/
|
||||
trigger request body from typed inputs, emitting only the contract's
|
||||
snake_case keys and JSON-encoding `task_template`/`variables` before they
|
||||
leave the client. `apps/web/lib/api/domains/office-routine-normalize.ts` is
|
||||
the single response-side adapter: it unwraps the envelope, decodes the two
|
||||
JSON-encoded string fields back into objects (falling back to `{}` for
|
||||
anything that isn't a JSON object per the requirement's definition), and
|
||||
maps every wire field to its camelCase model field with no
|
||||
`as`/`as unknown as` cast. `apps/web/lib/api/domains/office-api.ts` now calls
|
||||
through these two modules instead of shipping ad hoc request/response
|
||||
shapes. `apps/web/app/office/routines/[id]/cron-reconcile.ts` centralizes the
|
||||
create-before-delete replacement logic for a changed cron schedule (REQ-002:
|
||||
AC-002.4 through AC-002.11), including the two-cron-trigger and
|
||||
failed-relist error messages. `routine-row.tsx` and `routine-detail-view.tsx`
|
||||
were changed to consume model-shape fields exclusively; `routine-row.tsx`
|
||||
also gained a defensive `templateText` guard (a non-string `task_template`
|
||||
title/description, reachable only because this branch is the first time that
|
||||
field's content reaches JSX, would otherwise crash React's renderer).
|
||||
`create-routine-dialog.tsx`'s `onSubmit` now returns `Promise<boolean>` so a
|
||||
rejected `createRoutine` leaves the dialog open with the user's input intact
|
||||
instead of silently resetting the form. `routine-wire-scan.test.ts` is the
|
||||
static scan required by AC-003.10 and AC-003.18: it asserts no snake_case
|
||||
wire key and no `as unknown as` appear under
|
||||
`apps/web/app/office/routines/**` plus
|
||||
`apps/web/src/office-routine-client-routes.tsx`, and that the set of files
|
||||
outside that scope reading `s.office.routines` is exactly the two enumerated
|
||||
in the system design's Testing section.
|
||||
|
||||
## Implementation waves and parallel candidates
|
||||
|
||||
- [x] [Task 01: camelCase/snake_case adapter and cron-reconcile control flow](task-01-wire-adapter.md) (`done`) — REQ-001, REQ-002, REQ-003, REQ-004
|
||||
|
||||
Single wave: the request builder, response adapter, and the UI components
|
||||
that consume them all change together behind one contract, so there is no
|
||||
useful task split.
|
||||
|
||||
## Verification results
|
||||
|
||||
`pnpm --filter @kandev/web run typecheck`, `pnpm --filter @kandev/web run lint`
|
||||
and `pnpm --filter @kandev/web run test` (vitest, full `app/office/routines`
|
||||
and `lib/api/domains/office-routine-*` suites) all green. `pnpm run
|
||||
i18n:ratchet` and `pnpm run i18n:check` clean across English, `pt-pt`,
|
||||
`zh-cn`, `zh-hk`, `zh-tw` and pseudo. `pnpm run build` (Vite production
|
||||
build) succeeds. `apps/web/e2e` Playwright run green, including
|
||||
`routines-ui.spec.ts` (create dialog persists assignee/policies/task
|
||||
template and arms a cron trigger; detail view save persists a changed
|
||||
schedule; list/detail render persisted assignee, policy and declared
|
||||
variables; an unrecognized persisted status hides the next-fire countdown; a
|
||||
post-create refetch failure reports its own toast and still closes the
|
||||
dialog), `routine-catch-up-policy-ui.spec.ts`, `routine-fire.spec.ts`,
|
||||
`agentctl-cli.spec.ts`'s routines coverage, and the i18n pseudo-locale
|
||||
coverage check for the routines pages. Four independent review rounds
|
||||
(code-reviewer, security-reviewer, test-supervisor, plus a cross-vendor
|
||||
outside voice) converged with no residual defect; the final round's one
|
||||
verified finding (a React crash on a non-string `task_template` field) and
|
||||
one pre-existing regression (the create dialog silently discarding form
|
||||
input on a rejected create) were both fixed and covered by regression tests
|
||||
in the same round.
|
||||
|
||||
## Risks and out of scope
|
||||
|
||||
- The client cannot validate that the server actually stored what it sent;
|
||||
it can only prove it sent the contract's exact keys and shapes. Any future
|
||||
DTO change on the backend needs a matching adapter change, not the reverse.
|
||||
- `app/office/agents/[id]/` reads `s.office.routines` for `AgentRunSummary`
|
||||
data this capability does not own; those two files are deliberately
|
||||
excluded from the AC-003.10/AC-003.18 static scan rather than migrated.
|
||||
- Clearing an already-armed cron schedule from the UI remains unbuilt
|
||||
(system design, Out of scope); AC-002.13 only covers leaving it untouched
|
||||
when the submitted expression is empty after trimming.
|
||||
@@ -0,0 +1,157 @@
|
||||
---
|
||||
id: "01-wire-adapter"
|
||||
title: "camelCase/snake_case adapter and cron-reconcile control flow"
|
||||
status: done
|
||||
wave: 0
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-ROUTINE-WIRE-001
|
||||
- REQ-OFFICE-ROUTINE-WIRE-002
|
||||
- REQ-OFFICE-ROUTINE-WIRE-003
|
||||
- REQ-OFFICE-ROUTINE-WIRE-004
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.1
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.2
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.3
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.4
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.5
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.6
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.7
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.8
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.9
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.10
|
||||
- AC-OFFICE-ROUTINE-WIRE-001.11
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.1
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.2
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.3
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.4
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.5
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.6
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.7
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.8
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.9
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.10
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.11
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.12
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.13
|
||||
- AC-OFFICE-ROUTINE-WIRE-002.14
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.1
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.2
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.3
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.4
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.5
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.6
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.7
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.8
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.9
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.10
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.11
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.12
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.13
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.14
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.15
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.16
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.17
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.18
|
||||
- AC-OFFICE-ROUTINE-WIRE-003.19
|
||||
- AC-OFFICE-ROUTINE-WIRE-004.1
|
||||
- AC-OFFICE-ROUTINE-WIRE-004.2
|
||||
- AC-OFFICE-ROUTINE-WIRE-004.3
|
||||
- AC-OFFICE-ROUTINE-WIRE-004.4
|
||||
- AC-OFFICE-ROUTINE-WIRE-004.5
|
||||
system_design:
|
||||
- ../../specs/office/system-design/routine-wire-contract.md
|
||||
---
|
||||
|
||||
# Task 01: camelCase/snake_case adapter and cron-reconcile control flow
|
||||
|
||||
## Summary
|
||||
|
||||
The routines UI sent and read camelCase against a snake_case backend
|
||||
contract, so multi-word fields (assignee, policies, task template, cron
|
||||
schedule, declared variables) were silently discarded on write and rendered
|
||||
as blanks, placeholders, or one character per line on read, and a cron
|
||||
trigger could not be armed at all. This task adds a single adapter pair at
|
||||
the client's API boundary — a request builder that emits the contract's
|
||||
exact snake_case keys and a response normalizer that produces camelCase
|
||||
models with no snake_case fallback and no unchecked cast — plus the
|
||||
cron-reconcile control flow that replaces an armed trigger correctly when
|
||||
its schedule changes.
|
||||
|
||||
## In scope
|
||||
|
||||
- `apps/web/lib/api/domains/office-routine-api.ts`: request builders for
|
||||
create/update routine and create routine trigger, emitting only the keys
|
||||
and shapes `apps/backend/internal/office/routines/dto.go` binds.
|
||||
- `apps/web/lib/api/domains/office-routine-normalize.ts`: the response-side
|
||||
adapter — envelope unwrap, JSON-string decode for `task_template` and
|
||||
`variables`, and the routine/trigger/run camelCase mapping.
|
||||
- `apps/web/lib/api/domains/office-api.ts`: wires the two modules above into
|
||||
the existing exported API surface.
|
||||
- `apps/web/app/office/routines/[id]/cron-reconcile.ts`: create-before-delete
|
||||
trigger replacement, the two-cron-trigger warning, and the failed-relist
|
||||
message.
|
||||
- `apps/web/app/office/routines/{routine-row.tsx, routines-content.tsx,
|
||||
create-routine-dialog.tsx, [id]/routine-detail-view.tsx, [id]/page.tsx}`,
|
||||
`apps/web/app/office/agents/[id]/layout.tsx`,
|
||||
`apps/web/app/office/lib/catch-up-max.ts`,
|
||||
`apps/web/src/office-routine-client-routes.tsx`: consume model-shape
|
||||
fields exclusively; `create-routine-dialog.tsx`'s `onSubmit` returns
|
||||
`Promise<boolean>` so a rejected create leaves the dialog open.
|
||||
- `apps/web/app/office/routines/routine-wire-scan.test.ts`: the static
|
||||
no-snake_case / no-`as unknown as` scan required by AC-003.10, and the
|
||||
closed-set assertion required by AC-003.18.
|
||||
- `apps/web/src/locales/{en,pt-pt,zh-cn,zh-hk,zh-tw,pseudo}/office.json`: the
|
||||
new trigger-replacement and stale-schedule toast copy (AC-002.12).
|
||||
- `apps/web/e2e/tests/office/routines-ui.spec.ts` and the routine
|
||||
catch-up/fire/agentctl-cli E2E suites touched by the model-shape change.
|
||||
|
||||
## Acceptance
|
||||
|
||||
Every acceptance criterion under REQ-OFFICE-ROUTINE-WIRE-001 through
|
||||
REQ-OFFICE-ROUTINE-WIRE-004 in
|
||||
[the requirements](../../specs/office/requirements/routine-wire-contract.md),
|
||||
listed in this file's frontmatter.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/web
|
||||
pnpm run typecheck
|
||||
pnpm run lint
|
||||
pnpm run test -- app/office/routines lib/api/domains/office-routine-api.test.ts lib/api/domains/office-routine-normalize.test.ts
|
||||
pnpm run i18n:ratchet
|
||||
pnpm run i18n:check
|
||||
pnpm run build
|
||||
pnpm run e2e:run -- routines-ui routine-catch-up-policy-ui routine-fire
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
See `## In scope` above; full diff stat against the branch's merge-base:
|
||||
`git diff --stat $(git merge-base HEAD origin/main)..HEAD`.
|
||||
|
||||
## Dependencies
|
||||
|
||||
None — single task, single wave.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Not applicable: the request builder, response adapter, and the components
|
||||
that consume them change together behind one contract.
|
||||
|
||||
## Results
|
||||
|
||||
Shipped on `feature/routines-ui-camelcas-rf9`. Four independent review
|
||||
rounds (code-reviewer, security-reviewer, test-supervisor, and a
|
||||
cross-vendor outside voice) converged with no residual defect. The final
|
||||
round's one verified new finding — a React crash on a non-string
|
||||
`task_template.title`/`.description`, reachable only because this branch is
|
||||
the first time that field's content reaches JSX — was fixed with a
|
||||
`templateText` guard and regression test. A second, pre-existing
|
||||
finding surfaced by that same round's test work — the create dialog
|
||||
resetting and losing the operator's input on a rejected `createRoutine` call
|
||||
— was fixed by making `onSubmit` report success/failure and only
|
||||
closing/resetting on success, with regression coverage in
|
||||
`routines-content.test.tsx`.
|
||||
@@ -0,0 +1,339 @@
|
||||
---
|
||||
status: draft
|
||||
system: office
|
||||
created: 2026-09-17
|
||||
owners:
|
||||
- kandev
|
||||
---
|
||||
|
||||
# Office Routine Wire Contract Requirements
|
||||
|
||||
## Overview
|
||||
|
||||
The Office routine HTTP contract is snake_case in both directions:
|
||||
`CreateRoutineRequest`, `UpdateRoutineRequest` and `CreateTriggerRequest`
|
||||
(`apps/backend/internal/office/routines/dto.go`) bind keys like `task_template`
|
||||
and `cron_expression`, and `models.Routine`, `models.RoutineTrigger` and
|
||||
`models.RoutineRun` serialize the same keys. The web client sends and reads
|
||||
camelCase. Go's `encoding/json` matches case-insensitively but does not fold
|
||||
underscores, so every multi-word key crosses the boundary and is discarded.
|
||||
|
||||
This is measured, not inferred: binding the real DTOs through `gin`
|
||||
`ShouldBindJSON`, a camelCase routine body binds `name` alone, a camelCase
|
||||
`cronExpression` is rejected as a 400, and a `task_template` sent as a JSON
|
||||
object fails to bind.
|
||||
|
||||
Both consequences are live. On write, a routine's assignee, policies, catch-up
|
||||
max and task template are silently dropped, so the UI reports success and
|
||||
persists defaults. On the trigger path nothing is silent: a cron schedule cannot
|
||||
be armed at all, removing the product's only path to an unattended routine. On read, only `routine-row.tsx` has snake_case fallbacks
|
||||
and only for two fields, so the detail form, the expanded row, the Schedule
|
||||
card and the Runs list render blanks.
|
||||
|
||||
This capability makes the web client speak the contract the backend publishes.
|
||||
It changes no HTTP contract, no Go struct tag, no database column and no
|
||||
scheduler behavior.
|
||||
|
||||
## Terminology
|
||||
|
||||
Wire shape, model shape, adapter, envelope, arm, patch field, effective
|
||||
timezone and trimmed expression are defined in the system design's
|
||||
[Terminology](../system-design/routine-wire-contract.md#terminology).
|
||||
|
||||
## Requirements
|
||||
|
||||
### REQ-OFFICE-ROUTINE-WIRE-001: Routine configuration written from the UI is persisted
|
||||
|
||||
**Intent:** A create or save that reports success must have persisted what the
|
||||
operator chose. Today every multi-word field is discarded and the operator is
|
||||
told it worked.
|
||||
|
||||
|
||||
**User story:** As an operator, I want the assignee, policies and task template
|
||||
I pick to be saved, so that a routine does what I configured it to do.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.1:** When the client creates a routine, the
|
||||
system shall send each supplied value AC-001.11 admits under its contract
|
||||
key: `name`,
|
||||
`description`, `task_template`, `assignee_agent_profile_id`,
|
||||
`concurrency_policy`, `catch_up_policy`, `catch_up_max`, `variables`.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.2:** When the client creates a routine, the
|
||||
system shall send no key outside AC-001.1's set, and in particular no
|
||||
camelCase spelling of any of them.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.3:** When the client sends `task_template` or
|
||||
`variables`, the system shall send a JSON-encoded string, never a JSON object,
|
||||
because the bound fields are Go strings and an object is rejected as a 400.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.4:** When the client updates a routine, the
|
||||
system shall include exactly the patch fields AC-001.10 and AC-001.11 admit,
|
||||
so a field the caller did not supply is absent from the request body and the
|
||||
stored value is unchanged.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.5:** When a caller supplies an empty string for
|
||||
`assignee_agent_profile_id`, `description` or `task_template` on update, the
|
||||
system shall send it, because an empty string clears and an absent key
|
||||
preserves (AC-001.11).
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.6:** When the client creates a routine, the
|
||||
system shall not send `workspace_id` or `status`, because the handler takes
|
||||
the workspace from the route and forces `active`.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.7:** When a caller does not supply
|
||||
`concurrency_policy` or `catch_up_policy`, the system shall omit that key and
|
||||
shall not substitute a client-side default, so the server defaults
|
||||
(`skip_if_active`, `summarize_missed`) apply.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.8:** When the routine list toggle pauses or
|
||||
resumes a routine, the system shall send `status` alone and shall not widen
|
||||
the patch to any other field.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.9:** When the client updates a routine, the
|
||||
system shall send no key outside AC-001.1's set plus `status`, and in
|
||||
particular no camelCase spelling of any of them. `status` is an update key
|
||||
and not a create key (AC-001.6).
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.10:** When a caller supplies an empty
|
||||
`concurrency_policy` or `catch_up_policy` on update, the system shall omit
|
||||
that key rather than send the empty string, because `Valid()`
|
||||
(`models/enums.go`) rejects it with a 400. Those two keys are the only ones
|
||||
where empty means omit; every other update key transmits its empty string and
|
||||
clears (AC-001.11).
|
||||
- **AC-OFFICE-ROUTINE-WIRE-001.11:** When the system builds an update body, the
|
||||
clearable set shall be `name`, `description`, `task_template`,
|
||||
`assignee_agent_profile_id`, `status` and `variables`, not AC-001.5's three:
|
||||
`UpdateRoutineRequest` types all six as pointers and `applyRoutineUpdates`
|
||||
(`routines/handler.go`) assigns each one unvalidated, so an empty string
|
||||
supplied for any of the six is stored rather than ignored. The system shall
|
||||
not send an empty `status` (AC-003.17), and shall omit a `catch_up_max` below 1
|
||||
from a create or an update body rather than send it, because `*int` stores
|
||||
what is sent (AC-003.13).
|
||||
|
||||
### REQ-OFFICE-ROUTINE-WIRE-002: A cron schedule can be armed and changed from the UI
|
||||
|
||||
**Intent:** A routine with no cron trigger never fires unattended. The trigger
|
||||
create call is currently rejected outright, so the UI cannot arm one.
|
||||
|
||||
**User story:** As an operator, I want to set a routine's schedule in the UI,
|
||||
so that the routine runs without me firing it by hand.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.1:** When the client creates a routine trigger,
|
||||
the system shall send `kind`, `cron_expression`, `timezone`, `public_id`,
|
||||
`signing_mode` and `secret` under those keys, omitting any the caller did not
|
||||
supply.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.2:** When the create-routine flow is submitted
|
||||
with trigger kind `cron` and a non-empty trimmed expression, the system shall
|
||||
arm a cron trigger on the new routine, and the created trigger's `next_run_at`
|
||||
shall be non-null. A next fire time is a trigger field, not a routine field.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.3:** When the detail view opens for a routine
|
||||
that has a cron trigger, the system shall seed the form with that trigger's
|
||||
stored expression and timezone.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.4:** When the detail view is saved, the draft's
|
||||
trimmed expression matches the stored trigger's expression exactly, and the
|
||||
draft's effective timezone matches the stored trigger's effective timezone,
|
||||
the system shall issue no trigger create and no trigger delete.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.5:** When the detail view is saved with a changed
|
||||
expression or timezone, the system shall create the replacement trigger before
|
||||
deleting the superseded one. When the routine has no cron trigger yet, the
|
||||
system shall create one and delete nothing.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.6:** When that replacement create is rejected,
|
||||
the system shall leave the existing trigger in place, shall issue no delete,
|
||||
shall show the server's error message in an error toast, and shall show no
|
||||
success toast.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.7:** When the replacement create succeeds and the
|
||||
subsequent delete fails, the system shall re-list the routine's triggers and
|
||||
show an error toast naming the delete failure and stating how many cron
|
||||
triggers that re-list found, because two cron triggers both fire. When the
|
||||
re-list itself fails, the toast shall instead state that the
|
||||
trigger's fate is unknown and the page needs a reload.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.8:** When a routine has more than one cron
|
||||
trigger, the system shall seed, replace and display the first cron trigger in
|
||||
the order the list endpoint returned (`ORDER BY created_at`), and shall leave
|
||||
the others untouched. That order is always read from an endpoint response
|
||||
(AC-002.11).
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.9:** When a save updates the routine and its
|
||||
trigger create or delete then fails, the system shall show an error toast
|
||||
naming the trigger error and stating that the routine's own fields were saved
|
||||
and the schedule was not, shall show no success toast, and shall re-list the
|
||||
routine's triggers. A failed re-list is AC-002.11's case, not this one.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.10:** When the create-routine flow creates the
|
||||
routine and its cron trigger create then fails, the system shall close the
|
||||
create dialog, refresh the routine list so the new routine is visible, and
|
||||
show an error toast naming the trigger error and stating that the routine was
|
||||
created without a schedule. The routine is not deleted.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.11:** When the system creates or deletes a
|
||||
trigger, it shall render the routine's triggers from a fresh list response
|
||||
rather than from a locally spliced array. When that re-list fails after the
|
||||
create and delete both succeeded, the system shall report the save as
|
||||
succeeded and state that the displayed schedule may be stale until a reload.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.12:** When AC-002.7, AC-002.9, AC-002.10 or
|
||||
AC-002.11 shows a message today's code has no string for, that message shall
|
||||
be a translated `office`-namespace key present in English, `pt-pt`, `zh-cn`,
|
||||
`zh-hk` and `zh-tw`.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.13:** When the detail view is saved with a cron
|
||||
expression that is empty after trimming, the system shall issue no trigger
|
||||
create and no trigger delete, leaving any existing trigger armed. Clearing a
|
||||
schedule is out of scope below.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-002.14:** When the system decides whether a cron
|
||||
expression is empty, it shall decide on the trimmed expression, and when it
|
||||
sends one it shall send that trimmed string, so whitespace alone is empty at
|
||||
every gate and never reaches the server. The three live gates disagree today
|
||||
(system design, [Control flow](../system-design/routine-wire-contract.md#control-flow)).
|
||||
|
||||
### REQ-OFFICE-ROUTINE-WIRE-003: Routine and trigger reads render persisted values
|
||||
|
||||
**Intent:** The list, the expanded row, the detail form and the Schedule card
|
||||
must show what is stored. Today they show blanks, placeholders and, for declared
|
||||
variables, the characters of a JSON string one per line.
|
||||
|
||||
**User story:** As an operator, I want the routine pages to show the stored
|
||||
configuration, so that I can see what a routine will actually do.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.1:** When a routine endpoint returns a routine,
|
||||
the system shall produce a model whose `taskTemplate`,
|
||||
`assigneeAgentProfileId`, `concurrencyPolicy`, `catchUpPolicy`, `catchUpMax`,
|
||||
`variables`, `lastRunAt`, `workspaceId`, `createdAt` and `updatedAt` carry the
|
||||
persisted values.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.2:** When a routine endpoint returns a trigger,
|
||||
the system shall produce a model whose `routineId`, `cronExpression`,
|
||||
`timezone`, `publicId`, `signingMode`, `nextRunAt`, `lastFiredAt`, `enabled`,
|
||||
`createdAt` and `updatedAt` carry the persisted values. `created_at` and
|
||||
`updated_at` are mapped, not direct: the model declares them camelCase.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.3:** When the wire carries `task_template` or
|
||||
`variables` as a JSON-encoded string, the system shall parse it into an object
|
||||
before it reaches a component.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.4:** When that string is empty, is not valid
|
||||
JSON, or does not decode to a JSON object, the system shall produce an empty
|
||||
object rather than raising or passing the raw string through. A JSON object
|
||||
here means a non-null, non-array object, so `null`, `[]`, `[{"a":1}]`, `3` and
|
||||
`"text"` all produce the empty object.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.5:** When a response is enveloped, the system
|
||||
shall unwrap it by reading the envelope key alone, so a caller receives the
|
||||
resource or the array and never a wrapper object. On a list endpoint a missing
|
||||
key, a `null` payload and a non-array payload all produce the empty array. On
|
||||
a single-resource endpoint the same three cases are an error (AC-003.15), not
|
||||
an empty model. A body that is not an object has no envelope key and takes the
|
||||
same branch as a missing one.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.6:** When a routine or trigger field is absent or
|
||||
`null`, the system shall produce the empty string for a field the model
|
||||
declares as a required string, `{}` for one it declares as a required object
|
||||
(`taskTemplate`), `undefined` for a field the model declares optional, and
|
||||
`false` for `enabled`, and shall not substitute a policy value the server did
|
||||
not send. `status` and `kind` are required strings and take the
|
||||
same rule as any other (AC-003.16).
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.7:** When a list response contains an element
|
||||
that is not a JSON object in AC-003.4's sense, the system shall drop that
|
||||
element and keep the rest.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.8:** When a list response is returned, the system
|
||||
shall preserve the server's element order and shall not re-sort.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.9:** When a trigger is normalized, the system
|
||||
shall map only the fields the model declares, so the wire's `secret` key does
|
||||
not enter the client model or the store.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.10:** When a component renders a routine, trigger
|
||||
or run, it shall read only model-shape fields. The claim is checked by a scan
|
||||
over `apps/web/app/office/routines/**` plus
|
||||
`apps/web/src/office-routine-client-routes.tsx` asserting two negatives: no
|
||||
snake_case wire key from the three mapping tables, and no `as unknown as`.
|
||||
That removes `routine-row.tsx`'s two snake_case fallback casts and all four
|
||||
envelope casts. The fourth is `syncCronTrigger`'s, which alone has no
|
||||
bare-resource fallback and would silently drop the created trigger if it
|
||||
survived the unwrap (system design,
|
||||
[Components and responsibilities](../system-design/routine-wire-contract.md#components-and-responsibilities)).
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.11:** When a routine declares variables, the
|
||||
expanded row shall render one entry per declared variable name, not one entry
|
||||
per character of the encoded string.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.12:** When the wire carries `catch_up_max`, the
|
||||
system shall carry the stored number into the model unchanged and shall not
|
||||
clamp it, because the server normalizes at use
|
||||
(`models.NormaliseCatchUpMax`, ceiling 1000) and the model must report what is
|
||||
stored.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.13:** When a routine form seeds a catch-up max
|
||||
below 1, or the operator enters one below 1, the system shall use 25, because
|
||||
the server treats a sub-1 stored value as 25. `min={1}` enforces nothing here:
|
||||
no routine form saves through a form submit.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.14:** When the wire carries an empty `status`,
|
||||
the system shall carry the empty string into the model unchanged, because the
|
||||
empty string is a firing status the scheduler and `isRoutineFiring` both
|
||||
honour, and substituting `active` would assert a value no writer set.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.15:** When a single-resource endpoint returns a
|
||||
body whose resource is missing, `null`, not a JSON object in AC-003.4's sense,
|
||||
or an object carrying no non-empty `id`, the system shall reject the call with
|
||||
an error rather than produce a model built from nothing. The create-routine flow reads the new routine's `id`
|
||||
to arm its trigger, so an empty model would skip arming silently.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.16:** When the adapter produces a routine,
|
||||
trigger or run model, it shall do so without a type assertion, so
|
||||
`Routine.status`, `RoutineRun.status` and `RoutineTrigger.kind` shall each
|
||||
admit any string the server stored. None is validated on write, so `""`
|
||||
(AC-003.14, AC-004.5) and an arbitrary `unknown` are both reachable.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.17:** When the detail form seeds from a routine
|
||||
whose `status` is not one of the control's three options, `""` included, the
|
||||
control shall show no selected option, and a save that left it untouched shall
|
||||
omit `status` from the patch rather than write back a value no writer set
|
||||
(AC-001.11). A `""` routine shall still read as firing wherever firing state
|
||||
is shown (AC-003.14). No `unset` option is added.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.18:** When a file outside AC-003.10's scanned set
|
||||
reads `s.office.routines`, it shall consume model shape and shall test firing
|
||||
state through `isRoutineFiring` rather than comparing `status` to a literal.
|
||||
Two such files exist, both under `app/office/agents/[id]/`, and both are
|
||||
outside the scan because they also consume `AgentRunSummary`, a wire shape
|
||||
this contract does not own. The scan shall assert that the set of files
|
||||
reading `s.office.routines` under `apps/web/app/**` is exactly the three that
|
||||
do today, enumerated in the system design's
|
||||
[Testing](../system-design/routine-wire-contract.md#testing), so a new
|
||||
consumer fails the check rather than escaping the bound. AC-003.10's set is
|
||||
larger: most of its files take routines as props.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-003.19:** When `CoordinatorRoutineHint` renders for
|
||||
a CEO agent with a firing assigned routine present in `s.office.routines`, it
|
||||
shall not show the "no scheduled wake-ups" hint. The criterion is the
|
||||
component, not `/office/agents/:id`: nothing on that route populates the
|
||||
store, so the hint shows there regardless (system design, Out of scope). This
|
||||
capability owns the change: `assigneeAgentProfileId` is `undefined` for every
|
||||
routine today, so the hint is unconditional until the assignee transmits.
|
||||
|
||||
### REQ-OFFICE-ROUTINE-WIRE-004: Routine run reads render persisted values
|
||||
|
||||
**Intent:** The Runs tab reads the same boundary and is broken the same way:
|
||||
`dispatch_fingerprint`, `linked_task_id` and `created_at` never reach the model,
|
||||
so every run shows placeholders and no linked task.
|
||||
|
||||
**User story:** As an operator, I want the Runs list to show when each run
|
||||
happened and which task it created, so that I can tell whether a routine is
|
||||
working.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-OFFICE-ROUTINE-WIRE-004.1:** When a runs endpoint returns runs, the
|
||||
system shall produce models whose `id`, `source`, `status`, `routineId`,
|
||||
`triggerId`, `triggerPayload`, `linkedTaskId`, `coalescedIntoRunId`,
|
||||
`dispatchFingerprint`, `catchUpMissedTicks`, `catchUpFirstMissedAt`,
|
||||
`catchUpTruncated`, `startedAt`, `completedAt` and `createdAt` carry the
|
||||
persisted values. `id`, `source` and `status` are named even though their
|
||||
spellings match, because the adapter maps declared fields explicitly and
|
||||
never spreads.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-004.2:** When a run's `catch_up_missed_ticks` is
|
||||
absent, the system shall produce `undefined` and shall not produce `0`,
|
||||
because absence is never represented as a stored zero (`routine-catch-up.md`
|
||||
AC-002.3).
|
||||
- **AC-OFFICE-ROUTINE-WIRE-004.3:** When a run row renders, it shall show the
|
||||
run's creation time and, when the run created a task, the linked task id.
|
||||
`run-row.tsx` already renders both; this verifies the values stop being
|
||||
placeholders.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-004.4:** When the manual fire endpoint is called, the
|
||||
system shall continue to send `{"variables": ...}` unchanged, which already
|
||||
matches the contract.
|
||||
- **AC-OFFICE-ROUTINE-WIRE-004.5:** When a run field is absent or `null`, the
|
||||
system shall apply AC-003.6's rule to it. Absence is ordinary: a manually
|
||||
fired run has no `trigger_id`, and a `received` run has neither `started_at`
|
||||
nor `completed_at`. An empty or unrecognized `status` renders the unlabeled
|
||||
badge `run-row.tsx` already produces, so no new copy is added.
|
||||
|
||||
## Concurrency, retry and ordering
|
||||
|
||||
Stated in the system design's
|
||||
[Concurrency, retry and ordering](../system-design/routine-wire-contract.md#concurrency-retry-and-ordering):
|
||||
they are properties of the server contract, not client choices, and AC-001.4,
|
||||
AC-002.8, AC-002.11 and AC-003.8 depend on them.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Listed in the system design's
|
||||
[Out of scope](../system-design/routine-wire-contract.md#out-of-scope). Each
|
||||
names what is not built here and what a later capability would have to settle.
|
||||
@@ -0,0 +1,554 @@
|
||||
---
|
||||
status: draft
|
||||
system: office
|
||||
requirements:
|
||||
- REQ-OFFICE-ROUTINE-WIRE-001
|
||||
- REQ-OFFICE-ROUTINE-WIRE-002
|
||||
- REQ-OFFICE-ROUTINE-WIRE-003
|
||||
- REQ-OFFICE-ROUTINE-WIRE-004
|
||||
---
|
||||
|
||||
# Office Routine Wire Contract System Design
|
||||
|
||||
## Purpose and boundaries
|
||||
|
||||
The Office system publishes the routine HTTP contract, so it owns the
|
||||
translation between that contract and the web client's model shape. This
|
||||
design places that translation in one adapter at the client's API boundary and
|
||||
forbids it anywhere else.
|
||||
|
||||
Adjacent contracts this design uses but does not own: the routine HTTP
|
||||
endpoints and DTOs (`apps/backend/internal/office/routines`), the routine
|
||||
models (`apps/backend/internal/office/models`), and the shared fetch helpers
|
||||
(`apps/web/lib/api/client.ts`). None of them change.
|
||||
|
||||
## Terminology
|
||||
|
||||
These terms are used throughout, here and in the requirements, which reference
|
||||
this section rather than restating them.
|
||||
|
||||
- **Wire shape:** the JSON object the backend emits or binds, snake_case, with
|
||||
`task_template` and `variables` carried as JSON-encoded strings.
|
||||
- **Model shape:** the camelCase TypeScript types in
|
||||
`apps/web/lib/state/slices/office/types.ts` that components consume.
|
||||
- **Adapter:** the single translation point between wire shape and model
|
||||
shape, in both directions.
|
||||
- **Envelope:** the single-key object a routine endpoint wraps a resource in
|
||||
(`{"routine": ...}`, `{"trigger": ...}`, `{"run": ...}`, `{"routines": [...]}`).
|
||||
- **Arm:** create a cron trigger with a satisfiable expression, so the routine
|
||||
will fire unattended.
|
||||
- **Patch field:** a field the caller supplied to an update call; one the
|
||||
caller did not supply is not a patch field.
|
||||
- **Effective timezone:** a trigger's `timezone` with absent, `null` and the
|
||||
empty string all read as `UTC`, matching `CreateRoutineTrigger`, which
|
||||
rewrites an empty `Timezone` to `UTC` before storing it. Two effective
|
||||
timezones are equal when their strings match exactly: no case folding, no
|
||||
alias resolution, no offset comparison.
|
||||
- **Trimmed expression:** a cron expression with leading and trailing
|
||||
whitespace removed (AC-002.14).
|
||||
|
||||
## Requirement mapping
|
||||
|
||||
| Requirement | Design section |
|
||||
| --- | --- |
|
||||
| `REQ-OFFICE-ROUTINE-WIRE-001` | [Data and contracts](#data-and-contracts), [Control flow](#control-flow) |
|
||||
| `REQ-OFFICE-ROUTINE-WIRE-002` | [Control flow](#control-flow), [Failure and recovery](#failure-and-recovery) |
|
||||
| `REQ-OFFICE-ROUTINE-WIRE-003` | [Data and contracts](#data-and-contracts), [Components and responsibilities](#components-and-responsibilities) |
|
||||
| `REQ-OFFICE-ROUTINE-WIRE-004` | [Data and contracts](#data-and-contracts) |
|
||||
|
||||
## Components and responsibilities
|
||||
|
||||
- **Routine wire adapter** (`apps/web/lib/api/domains/`): the only code that
|
||||
knows a routine key is spelled `catch_up_policy`. Read direction normalizes
|
||||
wire to model; write direction builds the request body from the model. It
|
||||
maps declared fields explicitly and never spreads the raw object, so an
|
||||
unmapped wire key such as `secret` cannot reach the store.
|
||||
- **Routine API functions** (`office-api.ts`): call the adapter on the way out
|
||||
and on the way back, and unwrap the response envelope so callers receive a
|
||||
resource or an array.
|
||||
- **Routine components** (`apps/web/app/office/routines/`): consume model shape
|
||||
only. `routine-row.tsx`'s two snake_case fallback casts and four envelope
|
||||
casts are removed, because the adapter now guarantees what they were
|
||||
defending against. The envelope casts are in `routines-content.tsx`,
|
||||
`app/office/routines/[id]/page.tsx`, `src/office-routine-client-routes.tsx`
|
||||
and `[id]/routine-detail-view.tsx`. The fourth is the one that fails silently
|
||||
if it is missed: `syncCronTrigger` ends `.trigger ?? null` where the other
|
||||
three end in a bare-resource fallback (`?? (x as Resource)`), so against an
|
||||
unwrapped return it yields `null`, the created trigger is dropped from local
|
||||
state, and the page shows no schedule after an arm that succeeded. AC-003.10's
|
||||
second negative is what catches it.
|
||||
- **Office store slice**: unchanged. `setRoutines` is the only ingress and its
|
||||
only feeder is `listRoutines`; the Go boot payload ships `"routines": []`
|
||||
and no WebSocket event carries a routine, so the adapter has no bypass.
|
||||
|
||||
This mirrors the agent profile boundary in the same file (`normalizeAgent`,
|
||||
`agentPayload`, `stringifyJSONField`) and `normalizeProject`. It follows
|
||||
ADR-0036: a protocol shape is normalized
|
||||
at the adapter and the frontend renders only the normalized contract.
|
||||
|
||||
## Data and contracts
|
||||
|
||||
Read direction. Left is the wire key, right is the model field.
|
||||
|
||||
| Routine | Model | Note |
|
||||
| --- | --- | --- |
|
||||
| `id`, `name`, `description`, `status` | same | single-word; `status` carries `""` unchanged (AC-003.14) |
|
||||
| `workspace_id` | `workspaceId` | |
|
||||
| `task_template` | `taskTemplate` | JSON string, parsed to object |
|
||||
| `assignee_agent_profile_id` | `assigneeAgentProfileId` | |
|
||||
| `concurrency_policy` | `concurrencyPolicy` | passthrough, no default substituted |
|
||||
| `catch_up_policy` | `catchUpPolicy` | passthrough |
|
||||
| `catch_up_max` | `catchUpMax` | number, `undefined` when absent, never clamped client-side |
|
||||
| `variables` | `variables` | JSON string, parsed to object |
|
||||
| `last_run_at` | `lastRunAt` | `null` maps to `undefined` |
|
||||
| `created_at`, `updated_at` | `createdAt`, `updatedAt` | |
|
||||
|
||||
| Trigger | Model |
|
||||
| --- | --- |
|
||||
| `routine_id` | `routineId` |
|
||||
| `cron_expression` | `cronExpression` |
|
||||
| `public_id` | `publicId` |
|
||||
| `signing_mode` | `signingMode` |
|
||||
| `next_run_at` | `nextRunAt` |
|
||||
| `last_fired_at` | `lastFiredAt` |
|
||||
| `secret` | not mapped |
|
||||
| `created_at`, `updated_at` | `createdAt`, `updatedAt` |
|
||||
| `kind`, `timezone`, `enabled`, `id` | direct |
|
||||
|
||||
| Run | Model |
|
||||
| --- | --- |
|
||||
| `id`, `source`, `status` | direct |
|
||||
| `routine_id`, `trigger_id` | `routineId`, `triggerId` |
|
||||
| `trigger_payload` | `triggerPayload` |
|
||||
| `linked_task_id` | `linkedTaskId` |
|
||||
| `coalesced_into_run_id` | `coalescedIntoRunId` |
|
||||
| `dispatch_fingerprint` | `dispatchFingerprint` |
|
||||
| `catch_up_missed_ticks` | `catchUpMissedTicks`, `undefined` when absent |
|
||||
| `catch_up_first_missed_at` | `catchUpFirstMissedAt` |
|
||||
| `catch_up_truncated` | `catchUpTruncated` |
|
||||
| `started_at`, `completed_at`, `created_at` | `startedAt`, `completedAt`, `createdAt` |
|
||||
| `skip_reason`, `pause_id`, `causation_id` | not mapped, not rendered |
|
||||
|
||||
Model type changes (`types.ts`). The read mapping above cannot be written
|
||||
without a cast against today's types, and AC-003.16 forbids the cast, so three
|
||||
declarations widen to `string`. The server validates none of the three on
|
||||
write, so each closed union is a claim the wire does not honour (AC-003.16):
|
||||
|
||||
- `Routine.status` becomes `string`. `models.Routine.Status` is a plain Go
|
||||
string, `applyRoutineUpdates` assigns the patch pointer unguarded, and
|
||||
`CanFire()` treats `""` as firing, so both `""` (AC-003.14) and an arbitrary
|
||||
`unknown` written by the API, agentctl or config sync are reachable.
|
||||
`RoutineStatus` stays as the union the client writes (`UpdateRoutinePatch`),
|
||||
the direction the client does control. `Routine.status` has three consumers,
|
||||
not one, and two of them need a change:
|
||||
- `isRoutineFiring(status: string)` already takes a plain string and is
|
||||
correct as written.
|
||||
- `CoordinatorRoutineHint` (`app/office/agents/[id]/layout.tsx`) tests
|
||||
`r.status === "active"` directly, bypassing `isRoutineFiring`, so a firing
|
||||
`""` routine would read as not-firing. It moves to `isRoutineFiring`
|
||||
(AC-003.18). That surface changes whether we touch it or not: the same
|
||||
expression tests `r.assigneeAgentProfileId === agentId`, `undefined` for
|
||||
every routine today (AC-003.19).
|
||||
- `buildDraft` (`routine-detail-view.tsx`) narrows with
|
||||
`(routine.status as DraftState["status"]) ?? "active"`, and `??` catches
|
||||
neither. The cast goes; AC-003.17's unset control replaces it.
|
||||
- `RoutineRun.status` becomes `string` the same way (AC-004.5). Blast radius is
|
||||
nil: `RoutineRunStatus` has one reference, and `run-row.tsx`'s two lookup
|
||||
tables are typed `Record<string, string>` with an `?? run.status` fallback, so
|
||||
an unrecognized status renders as an unlabeled badge and needs no new copy.
|
||||
- `RoutineTrigger.kind` becomes `string`. `CreateRoutineTrigger`
|
||||
(`routines/service.go`) validates `cron_expression` for a cron trigger and
|
||||
never validates `kind`. `RoutineTriggerKind` stays as the union the client
|
||||
writes (`CreateTriggerInput.kind`). The `kind === "cron"` comparisons in `routine-detail-view.tsx` are unaffected.
|
||||
|
||||
No widening changes a rendered value; each makes the declared type match what
|
||||
the wire can carry. No exhaustive `switch` exists over any of the three unions.
|
||||
Widening rather than casting is the point: a cast at the adapter would defeat
|
||||
AC-003.9's explicit-mapping discipline, which is what keeps `secret` out of the
|
||||
store.
|
||||
|
||||
Write-side input types (`types.ts`, next to the model types). The write mapper
|
||||
takes these rather than `Partial<Routine>`: the model's `taskTemplate` is a
|
||||
`Record<string, unknown>` and cannot express AC-001.5's clearing empty string,
|
||||
and `RoutineTrigger` does not declare the `secret` AC-002.1 requires:
|
||||
|
||||
- `CreateRoutineInput`: `name`, and optional `description`, `taskTemplate`,
|
||||
`assigneeAgentProfileId`, `concurrencyPolicy`, `catchUpPolicy`, `catchUpMax`,
|
||||
`variables`. No `status` and no `workspaceId` (AC-001.6).
|
||||
- `UpdateRoutinePatch`: every `CreateRoutineInput` field optional, plus
|
||||
`status`. This is the key set AC-001.9 fixes.
|
||||
- `CreateTriggerInput`: `kind`, and optional `cronExpression`, `timezone`,
|
||||
`publicId`, `signingMode`, `secret`.
|
||||
|
||||
`taskTemplate` and `variables` are typed `Record<string, unknown> | string` on
|
||||
both input types, so an operator-cleared template is an expressible `""` rather
|
||||
than a cast.
|
||||
|
||||
Write direction. A field the caller did not supply is absent from the body;
|
||||
`JSON.stringify` drops an `undefined` value, so an explicitly supplied empty
|
||||
string still serializes and still clears the stored value. `task_template` and
|
||||
`variables` are encoded with the existing `stringifyJSONField` shape: an object
|
||||
becomes its JSON text, a string passes through, `undefined` stays `undefined`.
|
||||
An empty `concurrencyPolicy` or `catchUpPolicy` is dropped rather than sent,
|
||||
because `Valid()` rejects it with a 400 (AC-001.10); those two are the only
|
||||
fields where empty means omit rather than clear.
|
||||
|
||||
Everything else `UpdateRoutineRequest` types as a pointer clears on an empty
|
||||
string, and that is six fields rather than AC-001.5's three: `name`,
|
||||
`description`, `task_template`, `assignee_agent_profile_id`, `status` and
|
||||
`variables` (`dto.go`, applied by `applyRoutineUpdates` as an unguarded
|
||||
nil-check-then-assign). `status` is the consequential one: an empty status is stored unvalidated and
|
||||
`CanFire()` treats it as firing, so AC-001.11 forbids sending one and AC-003.17
|
||||
keeps an unset control from producing one. `catch_up_max` is a `*int`, so `0`
|
||||
and negatives are storable and distinct from absent; the client sends neither
|
||||
(AC-001.11, and [Failure and recovery](#failure-and-recovery) for why the
|
||||
control is not the mechanism).
|
||||
|
||||
Envelopes. `listRoutines` unwraps `routines`; `getRoutine`, `createRoutine` and
|
||||
`updateRoutine` unwrap `routine`; `createRoutineTrigger` unwraps `trigger`;
|
||||
`listRoutineTriggers` unwraps `triggers`; `runRoutine` unwraps `run`;
|
||||
`listRoutineRuns` and `listAllRoutineRuns` unwrap `runs`. The unwrap reads the
|
||||
named key and nothing else. There is no bare-resource tolerance: a body that is
|
||||
already the resource carries no envelope key, so it takes the missing-key
|
||||
branch. Tolerating one would need a shape discriminator to tell a bare routine
|
||||
from a malformed body, since neither has the key, and getting it wrong is the
|
||||
same silent skip the next paragraph closes. Nothing on this contract sends a
|
||||
bare resource: every routine response is enveloped by `RoutineResponse`,
|
||||
`RoutineListResponse`, `TriggerResponse`, `TriggerListResponse`,
|
||||
`RoutineRunResponse` or `RunListResponse` (`routines/dto.go`).
|
||||
|
||||
The two directions fail differently. A list unwrap that finds a missing key, a
|
||||
`null` payload or a non-array payload yields the empty array, which every list
|
||||
caller already renders (AC-003.5). A single-resource unwrap throws on those same
|
||||
three cases and on an object carrying no non-empty `id`, because its callers
|
||||
read fields off the result: `createRoutine`'s caller reads the new routine's
|
||||
`id` to arm the trigger, and `{}` would otherwise pass the object check, produce
|
||||
`id: ""` and arm `/routines//triggers` - the silent skip AC-003.15 exists to
|
||||
prevent.
|
||||
|
||||
Return types change from the enveloped shape to the resource shape, so the
|
||||
`as unknown as` casts at the call sites listed above disappear. This is a
|
||||
compile-time change inside the web app; no other package imports these
|
||||
functions.
|
||||
|
||||
## Control flow
|
||||
|
||||
Create routine: dialog state, then the write mapper, then `POST
|
||||
/workspaces/:wsId/routines`, then the read normalizer on the 201 body, then the
|
||||
routine id for the optional trigger step. Workspace comes from the path and
|
||||
status is forced server-side, so neither is in the body.
|
||||
|
||||
Save routine: the write mapper over the patch fields only, then `PATCH
|
||||
/routines/:id`, then the cron reconciliation below. The list toggle is the same
|
||||
path with a one-key patch (AC-001.8), which AC-001.9's key set admits.
|
||||
|
||||
Trigger state after reconciliation comes from `listRoutineTriggers`, not from
|
||||
splicing the local array (AC-002.11). That is what makes AC-002.8's "the order
|
||||
the list endpoint returned" true after a replacement, and how AC-002.7 learns
|
||||
whether a failed delete left the superseded trigger behind. It costs one extra
|
||||
GET per reconciliation that changed something; an unchanged save issues none.
|
||||
|
||||
Trigger equality for AC-002.4 compares the expression exactly and the effective
|
||||
timezone, where absent, `null` and `""` all read as `UTC`. The server normalizes
|
||||
the same way on the way in, so a stored `UTC` and an unset draft are the same
|
||||
schedule and must not churn the trigger.
|
||||
|
||||
Cron emptiness is decided on the trimmed expression, and the trimmed string is
|
||||
what is sent (AC-002.14). Three live sites disagree today:
|
||||
`create-routine-dialog.tsx`'s step gate trims, `routines-content.tsx`'s arm step
|
||||
gates on raw truthiness, and `syncCronTrigger` trims the gate but transmits
|
||||
`draft.cronExpression` untrimmed. That disagreement is invisible while the key
|
||||
is dropped and observable once it transmits: `" "` becomes a silent no-op on
|
||||
one path and a server 400 toast on another, and an untrimmed stored expression
|
||||
compares unequal to the trimmed draft on every later save (AC-002.4). No stored
|
||||
expression can carry stray whitespace from this UI today; one written by the API
|
||||
or by config is replaced on the next save, which is correct rather than churn. One trim at the boundary settles all three sites.
|
||||
|
||||
Status seeding. `buildDraft` seeds the status control from `routine.status`,
|
||||
where `?? "active"` catches neither `""` nor an unrepresentable stored value.
|
||||
Either leaves the control unselected and contributes no `status` patch field
|
||||
unless the operator picks one, which is AC-001.4's rule applied rather than a
|
||||
new one (AC-003.17). A `""` routine still reads as firing everywhere firing
|
||||
state is shown, because those read `isRoutineFiring`. The `<SelectValue />`
|
||||
gains no placeholder text: neither value is reachable through this UI, and
|
||||
inventing a placeholder would mean a new key in five locales for a state the
|
||||
operator cannot create. Coercing the display to `active` instead would write
|
||||
`active` back on the next save and commit a value no writer set, turning a
|
||||
read-only visit into a mutation.
|
||||
|
||||
Cron reconciliation replaces its current delete-then-create order with
|
||||
create-then-delete. The reason is a new failure mode this fix introduces: once
|
||||
`cron_expression` actually transmits, the server validates it with
|
||||
`shared.NextCronTime` and answers 400 on a bad expression. Under the old order
|
||||
the superseded trigger is already gone by then, so a typo costs the operator a
|
||||
working schedule. Under the new order a rejected create leaves the existing
|
||||
trigger armed. The cost is a window in which both triggers exist, which a
|
||||
delete failure leaves open and AC-002.7 requires be reported rather than
|
||||
swallowed. That toast is the only signal, which is why it carries the count:
|
||||
the page shows neither the duplicate nor a way out, because AC-002.8 displays
|
||||
the first cron trigger by `created_at` - after a failed delete, the superseded
|
||||
one - and `deleteRoutineTrigger`'s only call site is this reconciliation, which
|
||||
swaps which trigger is orphaned rather than removing one. The routine fires
|
||||
twice per tick until the extra is deleted through the API. Still the right
|
||||
trade: a duplicate fire is recoverable, a lost schedule is not.
|
||||
|
||||
Reads: server component loader, client route loader and list fetch all call the
|
||||
same API functions, so all three get model shape with no per-caller unwrap.
|
||||
|
||||
## Failure and recovery
|
||||
|
||||
- A non-2xx response rejects through the existing fetch helper. Callers keep
|
||||
their `try`/`catch` and their toasts. The adapter swallows nothing.
|
||||
- A malformed `task_template` or `variables` string yields an empty object.
|
||||
A routine with corrupt template JSON renders with empty template fields
|
||||
instead of failing the page.
|
||||
- A trigger create rejected by `ErrInvalidTrigger` surfaces the server's
|
||||
message, which names the bad expression. No local cron parser is added; the
|
||||
server owns cron validity.
|
||||
- `listRoutineTriggers` already has a per-routine `.catch` returning an empty
|
||||
list. That stays: one routine's trigger fetch failing must not blank the list.
|
||||
- The create-routine flow is two calls. A trigger failure after a successful
|
||||
routine create leaves an unscheduled routine, reported rather than undone
|
||||
(AC-002.10). The same split applies to the detail save (AC-002.9).
|
||||
- The four partial-failure paths each have a named observable, because
|
||||
"surface the error" is not one. A rejected replacement create (AC-002.6) and
|
||||
a rejected create-flow trigger (AC-002.10) show the server's message, and
|
||||
AC-002.10 also closes the dialog and refreshes the list rather than inviting a
|
||||
second create. A failed delete (AC-002.7) and a failed trigger step after a committed routine
|
||||
update (AC-002.9) both re-list first and report what that list shows. None of
|
||||
the four shows a success toast.
|
||||
- AC-002.7, AC-002.9 and AC-002.10 need copy that does not exist today: each
|
||||
path is `toast.error(err.message)` now, and the server's message cannot say
|
||||
which half of a two-call flow succeeded. That is new `office`-namespace copy,
|
||||
so it carries the five-locale obligation (`pt-pt`, `zh-cn`, `zh-hk`, `zh-tw`
|
||||
alongside English, `pnpm run i18n:zh-hant` for the Traditional pair) and the
|
||||
new-code ratchet enforces it. AC-002.12 states it so the build does not meet
|
||||
it as a surprise.
|
||||
- AC-003.15's rejection throws from the adapter, a plain `.ts` file, where
|
||||
`scripts/check-nonjsx-copy.mjs` scans internal error strings as copy. The
|
||||
message is either translated or carries an `// i18n-exempt:` line comment; a
|
||||
bare literal fails the ratchet rather than review.
|
||||
- `catch_up_max` is normalized by the server at use, not by the client. The
|
||||
control's `min={1}` is not what keeps a sub-1 value off the wire. The detail
|
||||
view and the create dialog both carry this control, neither saves through a
|
||||
form submit, and both spell the handler `Number(e.target.value) || 25`, which
|
||||
catches only `0` and `NaN`. Each form supplies the coercion, using 25 below 1
|
||||
(AC-003.13), and the write mapper omits a below-1 value (AC-001.11).
|
||||
|
||||
## Concurrency, retry and ordering
|
||||
|
||||
These are properties of the server contract the client cannot change, stated so
|
||||
no implementer has to invent them; each names the residual it leaves rather
|
||||
than a mitigation.
|
||||
|
||||
- **Retry:** the adapter adds no retry. Routine calls keep `fetchJson`, not
|
||||
`fetchJsonWithRetry`.
|
||||
- **Create is not idempotent:** the create endpoints carry no idempotency key,
|
||||
so a retried create produces a second routine or a second trigger. The
|
||||
adapter does not add one.
|
||||
- **Last write wins, over the whole row:** routine update has no
|
||||
optimistic-concurrency token, and the server's update is read-modify-write
|
||||
rather than a column-scoped patch: `doUpdateRoutine` (`routines/handler.go`)
|
||||
reads the routine, applies the patch in memory, and calls `UpdateRoutine`,
|
||||
whose statement (`repository/sqlite/routines.go`) writes every routine column
|
||||
from that struct. Two concurrent savers therefore overwrite each other's
|
||||
fields even when their patches are disjoint, and what survives is whichever
|
||||
database write lands second, not whichever response arrives second. AC-001.4
|
||||
narrows the request body, not the write. Removing the residual needs the
|
||||
optimistic concurrency the requirements exclude.
|
||||
- **Concurrent trigger creates:** two savers replacing the same cron trigger
|
||||
can leave two triggers. This is the same residual as AC-002.7 and is
|
||||
surfaced, not prevented.
|
||||
- **Server ordering:** routines are `ORDER BY name`, triggers `ORDER BY
|
||||
created_at`, runs `ORDER BY created_at DESC`. None carries a tiebreak column,
|
||||
so two rows with an equal sort key have an order this contract does not
|
||||
define. The client preserves whatever order it receives (AC-003.8). For
|
||||
triggers the consequence is stronger than display order: AC-002.8 replaces the
|
||||
first cron trigger, so two stored in the same clock tick leave which one gets
|
||||
replaced undefined. Closing that needs the tiebreak column the requirements
|
||||
exclude; AC-002.11 confines it to a real endpoint response, not a local array.
|
||||
|
||||
## Persistence
|
||||
|
||||
None. This is a client-side serialization boundary: no schema, no migration, no
|
||||
stored state change. The observable difference is that values the operator
|
||||
already intended to store now arrive.
|
||||
|
||||
## Security
|
||||
|
||||
`secret` is redacted to `""` by `redactTriggerSecrets` before any list
|
||||
response, and the model does not declare it. Explicit field mapping, rather
|
||||
than an object spread, is what keeps that true if the redaction ever regresses.
|
||||
No routine field is a credential; no new field is exposed.
|
||||
|
||||
## Observability
|
||||
|
||||
No new metrics or logs. The existing `routing_*` and routine counters are
|
||||
backend-side and unaffected.
|
||||
|
||||
## Testing
|
||||
|
||||
Vitest at the adapter is where the contract is pinned, because that is the
|
||||
boundary the defect lives at:
|
||||
|
||||
- serialized request body keys for routine create, routine update and trigger
|
||||
create, asserting the exact key set and that no camelCase key is present
|
||||
- update omits an unsupplied field and transmits an explicitly empty string
|
||||
- `task_template` and `variables` serialize as strings, never objects
|
||||
- read normalization for routine, trigger and run, including JSON-string
|
||||
parsing, the malformed-JSON fallback, `null` handling, element order,
|
||||
dropped non-object elements, and `secret` exclusion
|
||||
- the two unwrap failure directions: a list envelope that is missing, `null` or
|
||||
non-array yields `[]`; a single-resource envelope that is missing, `null`,
|
||||
non-object, or an object with no non-empty `id` throws (AC-003.5, AC-003.15)
|
||||
- an array-valued `variables` or `task_template` string yields `{}`, not an
|
||||
index-keyed object (AC-003.4)
|
||||
- run normalization carries `id`, `source` and `status` (AC-004.1), absent
|
||||
optional run fields stay `undefined`, and an absent run `status` normalizes to
|
||||
`""` without a cast (AC-004.5, AC-003.16)
|
||||
- an update body built from a patch that clears `variables` or `description`
|
||||
transmits the empty string, and one with an empty policy omits the key
|
||||
(AC-001.10, AC-001.11)
|
||||
- a cron expression is trimmed before it is sent (AC-002.14)
|
||||
|
||||
The REQ-002 orchestration behaviors are pinned by component tests over
|
||||
`routine-detail-view` and the create dialog with the API module mocked, next to
|
||||
the existing `create-routine-dialog.test.tsx`. Playwright cannot drive a failed
|
||||
DELETE or a rejected create without a fault-injection surface the app does not
|
||||
have, and these are the criteria a builder would otherwise have to invent a test
|
||||
shape for:
|
||||
|
||||
- an unchanged save issues no trigger create and no trigger delete, including
|
||||
when the stored timezone is `UTC` and the draft's is unset (AC-002.4)
|
||||
- a changed expression creates before it deletes (AC-002.5)
|
||||
- a rejected create issues no delete and leaves the stored expression displayed
|
||||
(AC-002.6)
|
||||
- a failed delete re-lists and reports the trigger count it found; a re-list
|
||||
that also fails reports the state as unknown (AC-002.7)
|
||||
- a routine update that succeeds followed by a failing trigger step reports both
|
||||
halves and shows no success toast (AC-002.9)
|
||||
- a create-flow trigger failure closes the dialog, refreshes the list, and
|
||||
reports the routine as created without a schedule (AC-002.10)
|
||||
- reconciliation renders the re-listed triggers, not a spliced array (AC-002.11)
|
||||
- a whitespace-only expression arms nothing in the create flow and issues
|
||||
neither create nor delete on the detail save (AC-002.14)
|
||||
- a `""`-status routine leaves the status control unselected and its save patch
|
||||
omits `status`, while the page still reads the routine as firing (AC-003.17)
|
||||
|
||||
`CoordinatorRoutineHint` is tested where it lives, next to the agent layout,
|
||||
with the store seeded: the hint is hidden for a CEO with a firing assigned
|
||||
routine, including one whose status is `""`, and shown when no assigned routine
|
||||
is firing (AC-003.19). The seeding is required, not convenient: the agents route
|
||||
never populates `s.office.routines` (Out of scope), so this cannot honestly be
|
||||
an E2E check. It is the one behavior change outside the routines directory, so it is the one place a passing routines suite
|
||||
would not have covered.
|
||||
|
||||
AC-003.10 is checked by a scan, not by reading. A vitest case reads
|
||||
`apps/web/app/office/routines/**` plus
|
||||
`apps/web/src/office-routine-client-routes.tsx` and asserts three things.
|
||||
|
||||
- **No snake_case wire key appears.** The key set is the left columns of the
|
||||
three mapping tables above, restricted to keys containing an underscore. The
|
||||
restriction is load-bearing rather than cosmetic: those columns also hold
|
||||
`id`, `name`, `description`, `status`, `kind`, `timezone`, `enabled`,
|
||||
`variables` and `source`, which are legitimate model-shape property names
|
||||
appearing throughout these files, so a scan over the unrestricted key set must
|
||||
fail correct code and could never pass. An underscore-bearing key has no
|
||||
model-shape spelling, so a match is always a violation. Two exclusions are
|
||||
required for the scan to be able to pass, and both are decidable line by line:
|
||||
`*.test.ts`/`*.test.tsx` files, whose comments cite `catch_up_max` when they
|
||||
name the catch-up criteria they cover, and lines whose first non-whitespace
|
||||
characters are `//` or `*`, because `routine-row.tsx` explains `next_run_at`
|
||||
in prose that survives this change. A comment cannot make a component read a
|
||||
wire field, so excluding them narrows nothing the criterion claims. Run over
|
||||
the tree today with those exclusions it reports exactly the three
|
||||
`routine-row.tsx` fallback lines this capability deletes, which is what makes
|
||||
it a passing assertion afterwards rather than a permanent red.
|
||||
- **No `as unknown as` appears.** This is the half no key scan can reach:
|
||||
`syncCronTrigger`'s cast contains no wire key. The bounded set holds nine
|
||||
occurrences across six sites today: two fallback casts in `routine-row.tsx`
|
||||
and four envelope unwraps, three of which spell the cast twice because they
|
||||
carry a bare-resource fallback. All nine go, so the assertion is zero rather
|
||||
than a count.
|
||||
- **The consumer set of `s.office.routines` has not grown.** Scanned over
|
||||
`apps/web/app/**`, which excludes `lib/state/slices/office/office-slice.ts`,
|
||||
the slice that declares the array rather than consuming it. The set must be
|
||||
exactly the three files that read it today:
|
||||
`app/office/routines/routines-content.tsx`,
|
||||
`app/office/agents/[id]/layout.tsx` and
|
||||
`app/office/agents/[id]/runs/runs-list-view.tsx` (AC-003.18). Asserting
|
||||
AC-003.10's whole set instead could never pass: most of those files take
|
||||
routines as props. Those two are
|
||||
excluded from the key scan deliberately, not by oversight: both also consume
|
||||
`AgentRunSummary`, a snake_case wire shape owned by a different contract, so
|
||||
`run.routine_id` and `run.task_id` are correct code there and no key scan can
|
||||
tell them from a violation. Asserting the consumer set instead keeps the
|
||||
exclusion honest and self-maintaining, because a third consumer fails this
|
||||
case rather than escaping the bound in silence.
|
||||
|
||||
The file bound makes the negative decidable where a repo-wide claim would not
|
||||
be, and the key set comes from the tables, so a key added to the contract later
|
||||
is added to the scan by the same edit.
|
||||
|
||||
Playwright covers the round trip the wire bug currently makes impossible to
|
||||
write honestly: seed a routine through the API, drive the real create and save
|
||||
flows, and read the persisted value back. The user-visible surfaces are
|
||||
`/office/routines` (list row, expanded detail, Runs tab), `/office/routines/:id`
|
||||
(form seeding, save, schedule card) and the create dialog. Existing specs to
|
||||
extend rather than duplicate: `routines-ui.spec.ts`,
|
||||
`routine-catch-up-policy-ui.spec.ts` and its mobile sibling
|
||||
`mobile-routine-catch-up-policy-ui.spec.ts`. No new component, layout or
|
||||
interaction pattern is introduced, so the same adapter feeds both and the
|
||||
mobile spec remains the check that it does.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- **Changing the HTTP contract.** The backend keeps snake_case JSON tags. An
|
||||
existing wire spelling is the public contract and stays: the same keys are
|
||||
consumed by the agentctl CLI, the YAML config loader, config import/export
|
||||
and config sync. Fixing the client is the safer change.
|
||||
- **Populating `s.office.routines` anywhere but `/office/routines`.** Only that
|
||||
route's two page files call `setRoutines`, so a direct visit to
|
||||
`/office/agents/:id` renders `CoordinatorRoutineHint` against an empty array
|
||||
and shows the hint whatever the assignee is. AC-003.19 is scoped to the
|
||||
component for that reason. A fix needs a second store feeder, falsifying the
|
||||
single-ingress property above, or a per-page fetch with its own loading and
|
||||
error contract. Neither is a wire-contract change.
|
||||
- **Adding a tiebreak column to any routine list query.** Changing it is a
|
||||
backend ordering decision with its own compatibility surface.
|
||||
- **Optimistic concurrency for routine update.** An `expected_*` guard is a
|
||||
contract change to the update DTO.
|
||||
- **Making the routine update and its trigger replacement atomic.** No endpoint
|
||||
accepts both. AC-002.9 makes the split outcome explicit instead.
|
||||
- **Rejecting an empty routine `name`.** `name` is single-word, so it already
|
||||
transmits and already clears; this capability neither introduces nor worsens
|
||||
it. Guarding it is a form-validation change with its own copy.
|
||||
- **Switching a routine from cron to webhook in the detail view.** Selecting
|
||||
`webhook` leaves the existing cron trigger armed, so the routine keeps firing.
|
||||
This is pre-existing and not worsened by the wire fix. It needs a
|
||||
trigger-kind transition contract of its own: what happens to the cron trigger,
|
||||
whether a webhook trigger is minted, and what `public_id` and `signing_mode`
|
||||
it gets.
|
||||
- **Creating or configuring webhook triggers from the UI.** `public_id`,
|
||||
`signing_mode` and `secret` have no controls. AC-002.1 makes the adapter carry
|
||||
them so a future surface needs no boundary change; no surface is added here.
|
||||
- **Clearing a routine's schedule from the detail view.** Emptying the cron
|
||||
expression leaves the trigger armed (AC-002.13), so the routine keeps firing.
|
||||
This is pre-existing. Removing a schedule needs its own contract: whether
|
||||
empty means delete, whether an operator can disarm without deleting, and what
|
||||
`enabled` is for. Guessing that empty means delete would let a cleared field
|
||||
destroy a working schedule silently.
|
||||
- **Clearing a routine's assignee from the detail view.** The assignee Select
|
||||
has no unassigned option. AC-001.5 makes the boundary able to express the
|
||||
clear; adding the control is a UI change this capability excludes.
|
||||
- **Surfacing `skip_reason`, `pause_id` and `causation_id` on runs.** They are
|
||||
on the wire and absent from the run model. Nothing renders them, so they stay
|
||||
unmapped.
|
||||
- **The divergence between the create dialog's `coalesce_if_active` default and
|
||||
the server's `skip_if_active`.** Once the field transmits, the dialog's choice
|
||||
is what persists. Which default is correct is a product decision, not a
|
||||
serialization one.
|
||||
|
||||
## Related decisions
|
||||
|
||||
- [ADR-0036: Normalize ACP shell output at the adapter boundary](../../../decisions/0036-normalize-acp-shell-output-at-adapter-boundary.md)
|
||||
Reference in New Issue
Block a user