diff --git a/cloud/apps/web/src/app/(app)/exceptions/page.tsx b/cloud/apps/web/src/app/(app)/exceptions/page.tsx new file mode 100644 index 0000000..e851b89 --- /dev/null +++ b/cloud/apps/web/src/app/(app)/exceptions/page.tsx @@ -0,0 +1,268 @@ +import Link from "next/link"; +import { auth } from "@/auth"; +import { prisma } from "@/lib/db"; +import { Card, CardBody } from "@/components/ui/card"; +import { parseWorkflowSteps } from "@ghost/core/schema/step"; +import { + classifyException, + dispositionForKind, + duplicateRiskFor, + type ExceptionKind, + type ExceptionOwner, +} from "@ghost/core/classifier/exception"; +import { ExceptionAssignee } from "@/components/exception-assignee"; + +export const dynamic = "force-dynamic"; + +/** + * The exception queue — the work an ops team actually does. + * + * Ghost's promise is that it involves a human only when necessary. That promise + * is only kept if the necessary involvement is *findable*: before this page an + * incident could only be reached by knowing a run's id, which meant the failure + * mode of a 40-invoice batch was twelve parked runs nobody knew about. + * + * Grouped by owner rather than by workflow or time, because the owner is the + * routing decision: a changed selector is the workflow author's problem, an + * expired credential is an administrator's, and a rejected value is the + * operator's. Sorting by anything else makes every reader re-triage the list. + */ + +const OWNER_COPY: Record = { + operator: { + title: "For an operator", + blurb: + "Needs a judgement call about the work itself, or a look at the target system.", + }, + author: { + title: "For the workflow author", + blurb: + "The workflow no longer matches the software it drives. Retrying will fail the same way.", + }, + administrator: { + title: "For an administrator", + blurb: + "Credentials, permissions, or approvals need attention before these can proceed.", + }, +}; + +const OWNER_ORDER: ExceptionOwner[] = ["operator", "author", "administrator"]; + +function kindTone(kind: ExceptionKind): string { + // OUTCOME_UNKNOWN is the only kind that carries a risk of repeating an effect, + // so it is the only one styled as a danger rather than a warning. If + // everything is red, the one that matters stops standing out. + if (kind === "OUTCOME_UNKNOWN") return "text-[var(--color-danger)]"; + if (kind === "TARGET_MISSING" || kind === "AUTH") + return "text-[var(--color-warning)]"; + return "text-[var(--color-muted)]"; +} + +function age(from: Date): string { + const mins = Math.floor((Date.now() - from.getTime()) / 60_000); + if (mins < 1) return "just now"; + if (mins < 60) return `${mins}m`; + const hours = Math.floor(mins / 60); + if (hours < 24) return `${hours}h`; + return `${Math.floor(hours / 24)}d`; +} + +export default async function ExceptionsPage() { + const session = await auth(); + const orgId = session?.user.orgId; + + const runs = orgId + ? await prisma.run.findMany({ + where: { orgId, status: "INCIDENT" }, + // Longest-parked first, on when the incident was raised — a run's own age + // says nothing about how long it has been waiting for someone. + orderBy: [{ incidentRaisedAt: "asc" }, { createdAt: "asc" }], + take: 200, + include: { + workflowVersion: { + include: { workflow: { select: { name: true } } }, + }, + incidentAssignee: { select: { id: true, name: true, email: true } }, + }, + }) + : []; + + // One batched lookup for the stopped-on step of every parked run, rather than + // a query per row. + const stepRows = + runs.length > 0 + ? await prisma.runStep.findMany({ + // Paired, not two independent `IN`s — that is a cross product. See + // the exceptions API route. + where: { OR: runs.map((r) => ({ runId: r.id, index: r.cursor })) }, + select: { + runId: true, + index: true, + status: true, + label: true, + endedAt: true, + }, + }) + : []; + const stopped = new Map(stepRows.map((s) => [`${s.runId}:${s.index}`, s])); + + const members = orgId + ? await prisma.membership.findMany({ + where: { orgId }, + select: { user: { select: { id: true, name: true, email: true } } }, + }) + : []; + const assignable = members.map((m) => m.user); + + const rows = runs.map((run) => { + let step; + try { + step = parseWorkflowSteps(run.workflowVersion.steps)[run.cursor]; + } catch { + step = undefined; + } + const recorded = stopped.get(`${run.id}:${run.cursor}`) ?? null; + const disposition = classifyException({ + reason: run.error ?? "", + step, + recordedOutcome: + recorded?.status === "UNKNOWN" + ? "UNKNOWN" + : recorded?.status === "FAILED" + ? "FAILED" + : null, + }); + // Stored kind is what the engine decided at the time and is what an auditor + // should see; fall back to the live computation only for runs raised before + // the column existed. Duplicate risk always takes the cautious answer. + const kind = (run.incidentKind as ExceptionKind | null) ?? disposition.kind; + // Display fields come from the kind actually shown, never a mix of stored + // label and live classification — see the exceptions API route. + const shown = dispositionForKind(kind); + return { + run, + kind, + owner: shown.owner, + headline: shown.headline, + guidance: shown.guidance, + retryMayDuplicate: duplicateRiskFor({ + disposition, + storedKind: kind, + recordedOutcome: recorded?.status === "UNKNOWN" ? "UNKNOWN" : null, + step, + }), + stepLabel: recorded?.label ?? step?.label ?? step?.type ?? null, + // When the incident was raised. `Run.endedAt` stays null for an INCIDENT + // (it is not terminal), and `createdAt` is the run's age, not its wait. + stoppedAt: run.incidentRaisedAt ?? recorded?.endedAt ?? run.createdAt, + }; + }); + + return ( +
+
+

Exceptions

+

+ Runs that stopped and need a person. Oldest first, grouped by who can + resolve them. +

+
+ + {rows.length === 0 ? ( + + +

Nothing waiting

+

+ No run is parked. Exceptions appear here the moment one stops and + cannot safely continue on its own. +

+ + + ) : ( + OWNER_ORDER.map((owner) => { + const group = rows.filter((r) => r.owner === owner); + if (group.length === 0) return null; + const copy = OWNER_COPY[owner]; + return ( +
+
+

+ {copy.title} + + {group.length} + +

+

+ {copy.blurb} +

+
+ + {group.map( + ({ + run, + kind, + headline, + guidance, + retryMayDuplicate, + stepLabel, + stoppedAt, + }) => ( + + +
+ + {run.workflowVersion.workflow.name} + + v{run.workflowVersion.version} + + + + stopped {age(stoppedAt)} ago + +
+ +
+ {headline} + + {" — step "} + {run.cursor + 1} + {stepLabel ? ` · ${stepLabel}` : ""} + +
+ +

+ {guidance} +

+ + {retryMayDuplicate && ( +

+ This step's effect may already have happened. + Check the target system before retrying. +

+ )} + + {run.error && ( + 
+                          {run.error}
+
+ )} + + + + + ), + )} +
+ ); + }) + )} +
+ ); +} diff --git a/cloud/apps/web/src/app/api/exceptions/route.ts b/cloud/apps/web/src/app/api/exceptions/route.ts new file mode 100644 index 0000000..fcee4c6 --- /dev/null +++ b/cloud/apps/web/src/app/api/exceptions/route.ts @@ -0,0 +1,249 @@ +import { NextResponse } from "next/server"; +import { auth } from "@/auth"; +import { prisma } from "@/lib/db"; +import { parseWorkflowSteps } from "@ghost/core/schema/step"; +import { + classifyException, + dispositionForKind, + duplicateRiskFor, + kindsForOwner, + EXCEPTION_KINDS, + type ExceptionKind, + type ExceptionOwner, +} from "@ghost/core/classifier/exception"; + +/** + * The exception queue: every run in this org waiting on a human, oldest first. + * + * This is the surface that makes incidents a *product* rather than a state. + * Before it, an incident was reachable only by knowing a run's id and opening + * that run — fine when you triggered the run yourself thirty seconds ago, + * useless as the way an ops team finds the twelve things that broke overnight. + * + * Oldest-first is deliberate and not a UI preference: an exception is work + * someone is waiting on, and the run that has been parked longest is the one + * most likely to have gone stale — a portal session that will need + * re-authenticating, a batch whose downstream deadline is closest. Newest-first + * would bury exactly the rows that need attention. + * + * ## Reads only + * + * Nothing here mutates or resumes anything. Resolution stays in + * `POST /api/runs/[id]/incident`, which re-checks the run's state under its own + * authorization. A queue that could also act would need every one of those + * checks duplicated here. + */ + +/** Cap on rows returned, so one org's bad night cannot fetch unboundedly. */ +const MAX_ROWS = 200; + +export interface ExceptionRow { + runId: string; + workflowName: string; + workflowVersion: number; + stepIndex: number; + stepLabel: string | null; + kind: ExceptionKind; + owner: ExceptionOwner; + headline: string; + guidance: string; + retryUseful: boolean; + retryMayDuplicate: boolean; + error: string | null; + /** + * When the run stopped, taken from the stopped-on step. + * + * NOT `Run.endedAt`: an INCIDENT is not terminal, so that column is still null + * — reading it here produced a field that was always null. Falls back to the + * run's creation time when the step row carries no end (a run that halted + * before the step was ever written). + */ + stoppedAt: string | null; + assignee: { id: string; name: string | null; email: string | null } | null; + triggeredBy: { name: string | null; email: string | null } | null; +} + +export async function GET(req: Request) { + const session = await auth(); + if (!session?.user?.orgId) { + return NextResponse.json({ error: "unauthorized" }, { status: 401 }); + } + const orgId = session.user.orgId; + + const url = new URL(req.url); + // `mine=1` narrows to the caller's own assignments — the "my work" view. + const mine = url.searchParams.get("mine") === "1"; + const kindFilter = url.searchParams.get("kind"); + const ownerFilter = url.searchParams.get("owner") as ExceptionOwner | null; + + // Both filters are pushed into SQL, and both must admit `incidentKind: null`. + // + // Two bugs live here otherwise. Filtering `incidentKind = 'X'` in SQL silently + // drops every incident whose kind was never stored — rows from before the + // column existed — so those never reach the fallback classification below and + // `?kind=UNKNOWN` omits exactly the incidents it should return. And applying an + // *owner* filter after `take` means the cap selects the 200 longest-parked + // incidents first: if those are all operator-owned, `?owner=author` returns + // nothing while author-owned exceptions sit just past the cap, reporting a + // `total` of 0 that reads as "none exist". + // + // `owner` is a pure function of `kind` (see kindsForOwner), so it becomes an IN + // list rather than needing a stored owner column. Null-kind rows are admitted + // here and filtered after classification. + const wantedKinds: ExceptionKind[] | null = ownerFilter + ? kindsForOwner(ownerFilter).filter((k) => !kindFilter || k === kindFilter) + : kindFilter && (EXCEPTION_KINDS as readonly string[]).includes(kindFilter) + ? [kindFilter as ExceptionKind] + : null; + + const runs = await prisma.run.findMany({ + where: { + orgId, + status: "INCIDENT", + ...(mine && session.user.id + ? { incidentAssigneeId: session.user.id } + : {}), + ...(wantedKinds + ? { OR: [{ incidentKind: { in: wantedKinds } }, { incidentKind: null }] } + : {}), + }, + // Longest-parked first, on when the incident was raised rather than when the + // run was created — see the module comment. + orderBy: [{ incidentRaisedAt: "asc" }, { createdAt: "asc" }], + take: MAX_ROWS, + include: { + workflowVersion: { include: { workflow: { select: { name: true } } } }, + incidentAssignee: { select: { id: true, name: true, email: true } }, + triggeredBy: { select: { name: true, email: true } }, + }, + }); + + // The stopped-on step for every run in the page, in one query rather than one + // per row. Prisma cannot filter a relation by a column of the parent row + // (`index = run.cursor`), so the pairing is done here instead of in SQL — but + // it is still a single round trip, which is the part that matters. + // + // Paired, not two independent `IN`s: `runId IN (…) AND index IN (…)` is a + // cross product, so 200 runs stopped at 200 distinct cursors would fetch up to + // 40,000 rows to shape 200 exceptions. + const stepRows = await prisma.runStep.findMany({ + where: { + OR: runs.map((r) => ({ runId: r.id, index: r.cursor })), + }, + select: { + runId: true, + index: true, + status: true, + label: true, + endedAt: true, + }, + }); + const stoppedStep = new Map( + stepRows.map((s) => [`${s.runId}:${s.index}`, s]), + ); + + const rows: ExceptionRow[] = []; + + for (const run of runs) { + // The stored `incidentKind` is the classification made when the run + // stopped, and it is what an auditor should see. Re-classify only when it is + // absent — a run that predates this column, or one whose incident was + // raised by an older worker. Never overwrite the stored value on read. + let kind = run.incidentKind as ExceptionKind | null; + + // The step is needed for the disposition text either way, and for duplicate + // risk when re-classifying. A malformed stored version must not take the + // whole queue down with it, so parse defensively. + let step; + try { + step = parseWorkflowSteps(run.workflowVersion.steps)[run.cursor]; + } catch { + step = undefined; + } + + const recorded = stoppedStep.get(`${run.id}:${run.cursor}`) ?? null; + + const disposition = classifyException({ + reason: run.error ?? "", + step, + recordedOutcome: + recorded?.status === "UNKNOWN" + ? "UNKNOWN" + : recorded?.status === "FAILED" + ? "FAILED" + : null, + }); + + // The stored kind is what the engine decided when the run stopped, and it is + // what an auditor should see — so every *other* displayed field has to come + // from that same kind. Mixing a stored label with a freshly-classified + // owner/headline/guidance produced visible nonsense as soon as the two + // disagreed, which is immediately for compensation incidents: their kind is + // asserted at the call site while their reason text carries no prefix, so + // live classification says UNKNOWN and the row read "OUTCOME_UNKNOWN" beside + // "Unclassified failure". + kind = kind ?? disposition.kind; + const shown = dispositionForKind(kind); + const retryMayDuplicate = duplicateRiskFor({ + disposition, + storedKind: kind, + recordedOutcome: recorded?.status === "UNKNOWN" ? "UNKNOWN" : null, + step, + }); + + const shaped: ExceptionRow = { + runId: run.id, + workflowName: run.workflowVersion.workflow.name, + workflowVersion: run.workflowVersion.version, + stepIndex: run.cursor, + stepLabel: recorded?.label ?? step?.label ?? step?.type ?? null, + kind, + owner: shown.owner, + headline: shown.headline, + guidance: shown.guidance, + retryUseful: shown.retryUseful, + retryMayDuplicate, + error: run.error, + // Prefer the recorded incident time; fall back to the step's end, then the + // run's creation for rows predating `incidentRaisedAt`. + stoppedAt: ( + run.incidentRaisedAt ?? + recorded?.endedAt ?? + run.createdAt + ).toISOString(), + assignee: run.incidentAssignee + ? { + id: run.incidentAssignee.id, + name: run.incidentAssignee.name, + email: run.incidentAssignee.email, + } + : null, + triggeredBy: run.triggeredBy + ? { name: run.triggeredBy.name, email: run.triggeredBy.email } + : null, + }; + + // Post-filter, needed only for the null-kind rows admitted above: their kind + // — and therefore their owner — is not known until classification. + if (ownerFilter && shaped.owner !== ownerFilter) continue; + if (kindFilter && shaped.kind !== kindFilter) continue; + rows.push(shaped); + } + + // Counts are computed over the returned rows, so they describe exactly what + // the caller is looking at rather than a different, unfiltered population. + const byOwner: Record = {}; + const byKind: Record = {}; + for (const r of rows) { + byOwner[r.owner] = (byOwner[r.owner] ?? 0) + 1; + byKind[r.kind] = (byKind[r.kind] ?? 0) + 1; + } + + return NextResponse.json({ + total: rows.length, + truncated: runs.length === MAX_ROWS, + byOwner, + byKind, + exceptions: rows, + }); +} diff --git a/cloud/apps/web/src/app/api/runs/[id]/incident/incident-routing.test.ts b/cloud/apps/web/src/app/api/runs/[id]/incident/incident-routing.test.ts new file mode 100644 index 0000000..7065900 --- /dev/null +++ b/cloud/apps/web/src/app/api/runs/[id]/incident/incident-routing.test.ts @@ -0,0 +1,471 @@ +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; +import { prisma } from "@ghost/core/db"; +import type { WorkflowSteps } from "@ghost/core/schema/step"; + +/** + * Exception routing on the incident route: assignment, and the acknowledgement + * a risky retry now requires. + * + * The behaviour under test is narrow but load-bearing. `OUTCOME_UNKNOWN` means + * "this step may already have taken effect" — the engine deliberately lets a + * human retry it anyway (see journal.ts on clearing `inFlight`), because only a + * person can check the target system. What it must not do is let that happen as + * a one-click accident indistinguishable from retrying a network blip, or leave + * no record that the warning was ever shown. + * + * Requires DATABASE_URL; skips cleanly without one. + */ + +const hasDb = Boolean(process.env.DATABASE_URL); + +const enqueueRunWorkflow = vi.hoisted(() => + vi.fn(async (_data: unknown) => undefined), +); +vi.mock("@/lib/queue", () => ({ + enqueueRunWorkflow, + enqueueCompensateRun: async () => undefined, +})); + +const session = vi.hoisted(() => ({ + current: null as null | { user: { id: string; orgId: string } }, +})); +vi.mock("@/auth", () => ({ auth: async () => session.current })); + +const steps: WorkflowSteps = [ + { id: "nav", type: "navigate", url: "https://example.com" }, + { + id: "pay", + type: "click", + selector: { role: "button", name: "Pay invoice" }, + }, +]; + +describe.skipIf(!hasDb)("incident routing (Postgres)", () => { + let orgId: string; + let userId: string; + let otherOrgId: string; + let outsiderId: string; + const slug = `exc-${Date.now()}`; + + beforeAll(async () => { + const org = await prisma.organization.create({ + data: { name: "Exc", slug }, + }); + orgId = org.id; + const user = await prisma.user.create({ + data: { + email: `${slug}@example.com`, + memberships: { create: { orgId, role: "OWNER" } }, + }, + }); + userId = user.id; + + // A second tenant, to prove assignment cannot cross an org boundary. + const other = await prisma.organization.create({ + data: { name: "Other", slug: `${slug}-other` }, + }); + otherOrgId = other.id; + const outsider = await prisma.user.create({ + data: { + email: `${slug}-outsider@example.com`, + memberships: { create: { orgId: otherOrgId, role: "OWNER" } }, + }, + }); + outsiderId = outsider.id; + }); + + afterAll(async () => { + for (const id of [orgId, otherOrgId]) { + await prisma.organization + .delete({ where: { id } }) + .catch(() => undefined); + } + for (const id of [userId, outsiderId]) { + await prisma.user.delete({ where: { id } }).catch(() => undefined); + } + }); + + beforeEach(() => { + session.current = { user: { id: userId, orgId } }; + enqueueRunWorkflow.mockClear(); + }); + + /** + * An INCIDENT run parked on step 1 (the click), with the recorded step status + * controlling whether its effect is known. + */ + async function incidentRun(opts: { + stepStatus: "FAILED" | "UNKNOWN"; + error: string; + incidentKind?: string; + }) { + const wf = await prisma.workflow.create({ + data: { + orgId, + name: `wf-${Math.random().toString(36).slice(2, 8)}`, + versions: { create: { version: 1, steps: steps as never } }, + }, + include: { versions: true }, + }); + const run = await prisma.run.create({ + data: { + orgId, + workflowVersionId: wf.versions[0]!.id, + status: "INCIDENT", + cursor: 1, + error: opts.error, + incidentKind: opts.incidentKind, + incidentRaisedAt: new Date(), + triggeredById: userId, + steps: { + create: { + index: 1, + type: "click", + status: opts.stepStatus, + label: "Pay invoice", + error: opts.error, + }, + }, + }, + }); + return run.id; + } + + async function post(runId: string, body: unknown) { + const { POST } = await import("./route"); + return POST( + new Request(`http://localhost/api/runs/${runId}/incident`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }), + { params: Promise.resolve({ id: runId }) }, + ); + } + + // ---- The acknowledgement gate ---------------------------------------- + + it("refuses an unacknowledged retry when the step's effect may already have happened", async () => { + const runId = await incidentRun({ + stepStatus: "UNKNOWN", + error: + "OUTCOME_UNKNOWN: step 1 may or may not have taken effect — socket hang up", + }); + + const res = await post(runId, { action: "retry" }); + expect(res.status).toBe(409); + const body = (await res.json()) as { + requiresAcknowledgement?: boolean; + kind?: string; + }; + expect(body.requiresAcknowledgement).toBe(true); + expect(body.kind).toBe("OUTCOME_UNKNOWN"); + + // The run must not have moved, and nothing may have been enqueued. + const run = await prisma.run.findUnique({ where: { id: runId } }); + expect(run?.status).toBe("INCIDENT"); + expect(enqueueRunWorkflow).not.toHaveBeenCalled(); + }); + + it("allows the same retry once the risk is acknowledged, and records that it was", async () => { + const runId = await incidentRun({ + stepStatus: "UNKNOWN", + error: + "OUTCOME_UNKNOWN: step 1 may or may not have taken effect — socket hang up", + }); + + const res = await post(runId, { + action: "retry", + acknowledgeDuplicateRisk: true, + }); + expect(res.status).toBe(200); + + const run = await prisma.run.findUnique({ where: { id: runId } }); + expect(run?.status).toBe("QUEUED"); + expect(enqueueRunWorkflow).toHaveBeenCalledOnce(); + + // The acknowledgement is in the run journal, so the decision is attributable + // to a person and sealed into the hash chain like any other event. + const event = await prisma.runEvent.findFirst({ + where: { runId, type: "step.retry_requested" }, + orderBy: { seq: "desc" }, + }); + const payload = event?.payload as { + acknowledgedDuplicateRisk?: boolean; + kind?: string; + } | null; + expect(payload?.acknowledgedDuplicateRisk).toBe(true); + expect(payload?.kind).toBe("OUTCOME_UNKNOWN"); + + // And in the org-wide audit log. + const audit = await prisma.auditEvent.findFirst({ + where: { orgId, entityId: runId, action: "run.incident_retried" }, + orderBy: { createdAt: "desc" }, + }); + const meta = audit?.metadata as { + acknowledgedDuplicateRisk?: boolean; + } | null; + expect(meta?.acknowledgedDuplicateRisk).toBe(true); + }); + + it("does not demand acknowledgement for an ordinary failure", async () => { + // A plain recorded FAILED on a transient network error: retry is the normal, + // safe, one-click path and must stay that way. If this test ever needs an + // acknowledgement, the gate has become noise and will be clicked through. + const runId = await incidentRun({ + stepStatus: "FAILED", + error: "net::ERR_CONNECTION_RESET at https://example.com", + incidentKind: "TRANSIENT", + }); + + const res = await post(runId, { action: "retry" }); + expect(res.status).toBe(200); + expect(enqueueRunWorkflow).toHaveBeenCalledOnce(); + + const event = await prisma.runEvent.findFirst({ + where: { runId, type: "step.retry_requested" }, + orderBy: { seq: "desc" }, + }); + const payload = event?.payload as { + acknowledgedDuplicateRisk?: boolean; + } | null; + // Absent, not false: the flag's presence in the chain is the evidence. + expect(payload?.acknowledgedDuplicateRisk).toBeUndefined(); + }); + + it("ignores a stale stored kind that would understate the risk", async () => { + // The stored label says TRANSIENT, but the recorded step outcome says the + // effect is unknown. The authoritative signal must win, or writing a wrong + // `incidentKind` would be a way to bypass the gate. + const runId = await incidentRun({ + stepStatus: "UNKNOWN", + error: "Timeout 30000ms exceeded", + incidentKind: "TRANSIENT", + }); + + const res = await post(runId, { action: "retry" }); + expect(res.status).toBe(409); + }); + + // ---- Resolution clears the queue ------------------------------------- + + it("clears routing fields when the incident resolves", async () => { + const runId = await incidentRun({ + stepStatus: "FAILED", + error: "net::ERR_CONNECTION_RESET", + incidentKind: "TRANSIENT", + }); + await prisma.run.update({ + where: { id: runId }, + data: { incidentAssigneeId: userId }, + }); + + await post(runId, { action: "retry" }); + + const run = await prisma.run.findUnique({ where: { id: runId } }); + // A resolved exception must leave the queue, and must not stay "assigned" to + // someone who has finished with it. + expect(run?.incidentKind).toBeNull(); + expect(run?.incidentAssigneeId).toBeNull(); + }); + + // ---- Assignment ------------------------------------------------------- + + it("assigns an exception to a member and records it", async () => { + const runId = await incidentRun({ stepStatus: "FAILED", error: "boom" }); + + const res = await post(runId, { action: "assign", assigneeId: userId }); + expect(res.status).toBe(200); + + const run = await prisma.run.findUnique({ where: { id: runId } }); + expect(run?.incidentAssigneeId).toBe(userId); + // Assignment resumes nothing. + expect(run?.status).toBe("INCIDENT"); + expect(enqueueRunWorkflow).not.toHaveBeenCalled(); + + const audit = await prisma.auditEvent.findFirst({ + where: { orgId, entityId: runId, action: "run.incident_assigned" }, + }); + expect(audit).not.toBeNull(); + }); + + it("unassigns on a null assignee", async () => { + const runId = await incidentRun({ stepStatus: "FAILED", error: "boom" }); + await post(runId, { action: "assign", assigneeId: userId }); + + const res = await post(runId, { action: "assign", assigneeId: null }); + expect(res.status).toBe(200); + const run = await prisma.run.findUnique({ where: { id: runId } }); + expect(run?.incidentAssigneeId).toBeNull(); + }); + + it("refuses to assign an exception to a user outside the org", async () => { + // Tenant isolation. Without the membership check this would accept any user + // id, putting another org's user on this org's queue and confirming that + // account exists. + const runId = await incidentRun({ stepStatus: "FAILED", error: "boom" }); + + const res = await post(runId, { action: "assign", assigneeId: outsiderId }); + expect(res.status).toBe(404); + const run = await prisma.run.findUnique({ where: { id: runId } }); + expect(run?.incidentAssigneeId).toBeNull(); + }); + + it("does not warn about duplicate effects for an indeterminate read", async () => { + // A `verify` whose outcome is unknown is a read: repeating it costs nothing. + // Demanding acknowledgement here would train operators to click through the + // prompt that exists for payments. + const wf = await prisma.workflow.create({ + data: { + orgId, + name: `wf-${Math.random().toString(36).slice(2, 8)}`, + versions: { + create: { + version: 1, + steps: [ + { id: "nav", type: "navigate", url: "https://example.com" }, + { + id: "chk", + type: "verify", + assertion: { kind: "textPresent", expected: "Paid" }, + }, + ] as never, + }, + }, + }, + include: { versions: true }, + }); + const run = await prisma.run.create({ + data: { + orgId, + workflowVersionId: wf.versions[0]!.id, + status: "INCIDENT", + cursor: 1, + error: "OUTCOME_UNKNOWN: step 1 may or may not have taken effect", + incidentKind: "OUTCOME_UNKNOWN", + incidentRaisedAt: new Date(), + triggeredById: userId, + steps: { + create: { + index: 1, + type: "verify", + status: "UNKNOWN", + label: "Paid?", + }, + }, + }, + }); + + const res = await post(run.id, { action: "retry" }); + expect(res.status).toBe(200); + }); + + it("clears incidentRaisedAt when the incident resolves", async () => { + const runId = await incidentRun({ + stepStatus: "FAILED", + error: "net::ERR_CONNECTION_RESET", + incidentKind: "TRANSIENT", + }); + const before = await prisma.run.findUnique({ where: { id: runId } }); + expect(before?.incidentRaisedAt).not.toBeNull(); + + await post(runId, { action: "retry" }); + + // Leaving INCIDENT clears the parked-since stamp, or the next incident on + // this run would inherit an age from the previous one. + const after = await prisma.run.findUnique({ where: { id: runId } }); + expect(after?.incidentRaisedAt).toBeNull(); + }); + + it("refuses an acknowledgement raised for a different incident", async () => { + // The confirmation must be about the incident the human actually read. If + // someone else resolves it and the run parks on a new risky step, a stale + // open dialog must not be able to acknowledge the new one. + const runId = await incidentRun({ + stepStatus: "UNKNOWN", + error: "OUTCOME_UNKNOWN: step 1 may or may not have taken effect", + }); + + const stale = await post(runId, { + action: "retry", + acknowledgeDuplicateRisk: true, + expectStepIndex: 0, // the run is parked on step 1 + }); + expect(stale.status).toBe(409); + const body = (await stale.json()) as { staleAcknowledgement?: boolean }; + expect(body.staleAcknowledgement).toBe(true); + expect(enqueueRunWorkflow).not.toHaveBeenCalled(); + + // The same call naming the incident actually on screen is accepted. + const run = await prisma.run.findUnique({ where: { id: runId } }); + const ok = await post(runId, { + action: "retry", + acknowledgeDuplicateRisk: true, + expectStepIndex: 1, + expectIncidentRaisedAt: run!.incidentRaisedAt!.toISOString(), + }); + expect(ok.status).toBe(200); + }); + + it("refuses an acknowledgement whose incident timestamp has moved on", async () => { + const runId = await incidentRun({ + stepStatus: "UNKNOWN", + error: "OUTCOME_UNKNOWN: step 1 may or may not have taken effect", + }); + + const res = await post(runId, { + action: "retry", + acknowledgeDuplicateRisk: true, + expectStepIndex: 1, + expectIncidentRaisedAt: new Date(Date.now() - 86_400_000).toISOString(), + }); + expect(res.status).toBe(409); + expect(enqueueRunWorkflow).not.toHaveBeenCalled(); + }); + + it("records the retry audit in the same transaction as the state change", async () => { + const runId = await incidentRun({ + stepStatus: "FAILED", + error: "net::ERR_CONNECTION_RESET", + incidentKind: "TRANSIENT", + }); + await post(runId, { action: "retry" }); + + // Both must exist together: a run that left INCIDENT with no org audit + // record would let the reclaimer drive the retry unaccounted for. + const run = await prisma.run.findUnique({ where: { id: runId } }); + expect(run?.status).toBe("QUEUED"); + const audit = await prisma.auditEvent.findFirst({ + where: { orgId, entityId: runId, action: "run.incident_retried" }, + }); + expect(audit).not.toBeNull(); + }); + + it("still audits a skip", async () => { + const runId = await incidentRun({ stepStatus: "FAILED", error: "boom" }); + // Step 1 is a click on "Pay invoice" — sensitive, so skip is refused. Use + // step 0 (navigate) instead by parking the run there. + await prisma.run.update({ where: { id: runId }, data: { cursor: 0 } }); + + const res = await post(runId, { action: "skip" }); + expect(res.status).toBe(200); + const audit = await prisma.auditEvent.findFirst({ + where: { orgId, entityId: runId, action: "run.incident_skipped" }, + }); + expect(audit).not.toBeNull(); + }); + + it("rejects an unknown action", async () => { + const runId = await incidentRun({ stepStatus: "FAILED", error: "boom" }); + const res = await post(runId, { action: "resolve" }); + expect(res.status).toBe(400); + }); +}); diff --git a/cloud/apps/web/src/app/api/runs/[id]/incident/route.ts b/cloud/apps/web/src/app/api/runs/[id]/incident/route.ts index 68878ba..d6d43e9 100644 --- a/cloud/apps/web/src/app/api/runs/[id]/incident/route.ts +++ b/cloud/apps/web/src/app/api/runs/[id]/incident/route.ts @@ -6,6 +6,7 @@ import { appendAuditEvent, appendRunEvent } from "@ghost/core/audit-log"; import { RUN_EVENT_TYPES } from "@ghost/core/run-events"; import { parseWorkflowSteps } from "@ghost/core/schema/step"; import { classifyStep } from "@ghost/core/classifier"; +import { classifyException, duplicateRiskFor } from "@ghost/core/classifier/exception"; /** * Resolve an incident: retry the failed step, or skip it. @@ -46,9 +47,17 @@ export async function POST(req: Request, { params }: { params: Promise<{ id: str const userId = session.user.id ?? null; const { id } = await params; - const body = (await req.json().catch(() => ({}))) as { action?: string }; - if (body.action !== "retry" && body.action !== "skip") { - return NextResponse.json({ error: "action must be retry|skip" }, { status: 400 }); + const body = (await req.json().catch(() => ({}))) as { + action?: string; + assigneeId?: string | null; + acknowledgeDuplicateRisk?: boolean; + // Identity of the incident the caller was looking at when they confirmed. + // See the acknowledgement check below. + expectStepIndex?: number; + expectIncidentRaisedAt?: string; + }; + if (body.action !== "retry" && body.action !== "skip" && body.action !== "assign") { + return NextResponse.json({ error: "action must be retry|skip|assign" }, { status: 400 }); } const run = await prisma.run.findFirst({ @@ -57,6 +66,92 @@ export async function POST(req: Request, { params }: { params: Promise<{ id: str }); if (!run) return NextResponse.json({ error: "no incident on this run" }, { status: 404 }); + // ---- Assign ----------------------------------------------------------- + // Handled before the compensation guard below, because assignment is the one + // control that is always appropriate: a failed *reversal* is exactly the kind + // of incident that needs a named owner, even though retry and skip are refused + // on it. + // + // Assignment changes no run state and resumes nothing, so it is open to any + // member. Deciding who looks at a problem is not authorizing the action that + // caused it. + if (body.action === "assign") { + let assigneeId: string | null = null; + if (body.assigneeId != null) { + if (typeof body.assigneeId !== "string") { + return NextResponse.json({ error: "assigneeId must be a string or null" }, { status: 400 }); + } + // Tenant isolation: an exception may only be assigned to a member of the + // org that owns the run. Without this check any user id would be accepted, + // leaking the existence of accounts across tenants and putting another + // org's user on this org's queue. + const member = await prisma.membership.findFirst({ + where: { orgId, userId: body.assigneeId }, + select: { userId: true }, + }); + if (!member) { + return NextResponse.json({ error: "assignee is not a member of this org" }, { status: 404 }); + } + assigneeId = member.userId; + } + + // One transaction: a routing mutation that applied but was never recorded + // would leave the queue showing an owner with no audit trail explaining how + // they got it, while the caller was told the request failed. Either both + // land or neither does. `appendAuditEvent` takes the tx for exactly this. + // Both the membership check and the run state are re-verified *inside* the + // transaction below. The lookup above is only an early, friendly 404: member + // removal runs its own unassignment cleanup in a transaction of its own, and + // between that read and this write the membership can vanish — the foreign + // key references `User`, not `Membership`, so the write would still succeed + // and re-attach a non-member to this org's queue. + // + // Conditional on the run still being an INCIDENT, inside the transaction. + // The `findFirst` above is a read: a retry, skip, cancel or undo can resolve + // the incident between it and this write, and an id-only update would then + // assign an owner to a run that is no longer an exception — reporting + // success, and leaving a stale assignee to surface on the run's *next* + // incident. + let assigned = 0; + await prisma.$transaction(async (tx) => { + if (assigneeId !== null) { + const stillMember = await tx.membership.findFirst({ + where: { orgId, userId: assigneeId }, + select: { userId: true }, + }); + if (!stillMember) return; + } + const res = await tx.run.updateMany({ + where: { id, orgId, status: "INCIDENT" }, + data: { incidentAssigneeId: assigneeId }, + }); + assigned = res.count; + if (assigned !== 1) return; + await appendAuditEvent( + orgId, + userId, + { + action: assigneeId ? "run.incident_assigned" : "run.incident_unassigned", + entityType: "Run", + entityId: id, + metadata: { stepIndex: run.cursor, assigneeId }, + }, + tx, + ); + }); + if (assigned !== 1) { + return NextResponse.json( + { + error: + "could not assign: the run is no longer an open exception, or that " + + "person is no longer a member of this organization", + }, + { status: 409 }, + ); + } + return NextResponse.json({ ok: true, assigneeId }); + } + // A compensation that failed also stops as INCIDENT, and these controls are // forward-recovery controls. Letting them run on a reversal would append // forward-step recovery events to a run whose steps are all already complete: @@ -116,7 +211,18 @@ export async function POST(req: Request, { params }: { params: Promise<{ id: str }); await tx.run.update({ where: { id }, - data: { status: "QUEUED", error: null, cursor: index + 1 }, + // Clear the routing fields: this run is no longer an open exception, so + // it must leave the queue and must not stay assigned to someone who has + // finished with it. A later failure raises a fresh, re-classified + // incident. + data: { + status: "QUEUED", + error: null, + cursor: index + 1, + incidentKind: null, + incidentAssigneeId: null, + incidentRaisedAt: null, + }, }); const { seq } = await appendRunEvent( id, @@ -128,8 +234,112 @@ export async function POST(req: Request, { params }: { params: Promise<{ id: str tx, ); resumeSeq = seq; + // Same atomicity as the retry branch: the state change, the journal + // event and the org audit record commit together or not at all. + await appendAuditEvent( + orgId, + userId, + { + action: "run.incident_skipped", + entityType: "Run", + entityId: id, + metadata: { stepIndex: index, kind: run.incidentKind }, + }, + tx, + ); }); } else { + // Retrying a step whose effect may already have happened is a decision the + // engine deliberately leaves to a human (see journal.ts on clearing + // `inFlight`) — but it must be a decision, not a mis-click. The disposition + // is recomputed here rather than read from `Run.incidentKind`, so a stale + // stored label cannot wave a risky retry through. + // + // This adds no prohibition: the retry still happens, on the same terms as + // before, for any caller that says it understands the risk. What changes is + // that it cannot happen *accidentally*, and the record shows the warning was + // shown. + // Deliberately NOT `.catch(() => null)`. The recorded outcome is the signal + // that decides whether this retry needs an acknowledgement, so swallowing a + // transient database error would turn "I could not find out" into "there is + // nothing to worry about" — the least restrictive answer available, on the + // one query where being wrong repeats a payment. A lookup failure means the + // gate cannot be evaluated, so the retry is refused rather than allowed. + let recorded: { status: string } | null; + try { + recorded = await prisma.runStep.findUnique({ + where: { runId_index: { runId: id, index } }, + select: { status: true }, + }); + } catch { + return NextResponse.json( + { + error: + "could not determine whether this step's effect is known, so the retry was refused. " + + "This is a transient storage error — try again.", + }, + { status: 503 }, + ); + } + + const disposition = classifyException({ + reason: run.error ?? "", + step, + recordedOutcome: + recorded?.status === "UNKNOWN" ? "UNKNOWN" : recorded?.status === "FAILED" ? "FAILED" : null, + }); + + // Union of the live verdict and the stored label, still gated on the step + // actually reaching outside the browser. See duplicateRiskFor. + const mayDuplicate = duplicateRiskFor({ + disposition, + storedKind: run.incidentKind, + recordedOutcome: recorded?.status === "UNKNOWN" ? "UNKNOWN" : null, + step, + }); + + // An acknowledgement has to be *about something*. A bare boolean can + // outlive the incident it was shown for: if another operator resolves this + // incident while the confirmation is open and the run then parks on a new + // risky step, the still-open dialog would acknowledge a step its clicker + // never looked at. So the caller states which incident it was shown — step + // index and raised-at — and a mismatch is refused. Same principle as + // approving the *resolved* action rather than the template: the human must + // be confirming the thing that actually runs. + if (mayDuplicate && body.acknowledgeDuplicateRisk === true) { + const sameStep = + body.expectStepIndex === undefined || body.expectStepIndex === index; + const sameIncident = + body.expectIncidentRaisedAt === undefined || + (run.incidentRaisedAt !== null && + new Date(body.expectIncidentRaisedAt).getTime() === run.incidentRaisedAt.getTime()); + if (!sameStep || !sameIncident) { + return NextResponse.json( + { + error: + "this run has stopped on a different step since that confirmation was shown. " + + "Re-read the current exception before retrying.", + requiresAcknowledgement: true, + staleAcknowledgement: true, + }, + { status: 409 }, + ); + } + } + + if (mayDuplicate && body.acknowledgeDuplicateRisk !== true) { + return NextResponse.json( + { + error: + "this step may already have taken effect, so retrying it could repeat that effect. " + + "Confirm in the target system first, then retry with acknowledgeDuplicateRisk: true.", + kind: disposition.kind, + guidance: disposition.guidance, + requiresAcknowledgement: true, + }, + { status: 409 }, + ); + } // Retry: clear the recorded failure so the journal fold stops reporting it. // The step's own `step.started`/`step.failed` history stays in the chain — // this appends, it never rewrites. @@ -138,27 +348,59 @@ export async function POST(req: Request, { params }: { params: Promise<{ id: str where: { runId: id, index }, data: { status: "PENDING", error: null }, }); - await tx.run.update({ where: { id }, data: { status: "QUEUED", error: null } }); + await tx.run.update({ + where: { id }, + // See the skip branch: leaving INCIDENT clears the routing fields. + data: { + status: "QUEUED", + error: null, + incidentKind: null, + incidentAssigneeId: null, + incidentRaisedAt: null, + }, + }); const { seq } = await appendRunEvent( id, { type: RUN_EVENT_TYPES.stepRetryRequested, stepIndex: index, - payload: { phase: "incident", retriedById: userId }, + payload: { + phase: "incident", + retriedById: userId, + kind: disposition.kind, + // Present only when the retry carried duplicate risk, so its + // presence in the chain is itself the evidence of an informed + // decision rather than a flag that is always there. + ...(mayDuplicate ? { acknowledgedDuplicateRisk: true } : {}), + }, }, tx, ); resumeSeq = seq; + // Inside the transaction, with the state change and the journal event. + // Appended after them so it is ordered last in the chain, but committed + // atomically: if this were left outside and failed, the run would already + // have left INCIDENT carrying an acknowledgement recorded only in the run + // journal, and the stalled-run reclaimer could later drive the retry with + // no org-level audit record of who accepted the duplicate risk. + await appendAuditEvent( + orgId, + userId, + { + action: "run.incident_retried", + entityType: "Run", + entityId: id, + metadata: { + stepIndex: index, + kind: run.incidentKind, + ...(mayDuplicate ? { acknowledgedDuplicateRisk: true } : {}), + }, + }, + tx, + ); }); } - await appendAuditEvent(orgId, userId, { - action: body.action === "skip" ? "run.incident_skipped" : "run.incident_retried", - entityType: "Run", - entityId: id, - metadata: { stepIndex: index }, - }); - await enqueueRunWorkflow({ runId: id, orgId, diff --git a/cloud/apps/web/src/app/api/runs/[id]/undo/route.ts b/cloud/apps/web/src/app/api/runs/[id]/undo/route.ts index 04dbe3b..d011aea 100644 --- a/cloud/apps/web/src/app/api/runs/[id]/undo/route.ts +++ b/cloud/apps/web/src/app/api/runs/[id]/undo/route.ts @@ -4,6 +4,7 @@ import { prisma } from "@/lib/db"; import { enqueueCompensateRun } from "@/lib/queue"; import { appendAuditEvent } from "@ghost/core/audit-log"; import { parseWorkflowSteps } from "@ghost/core/schema/step"; +import { duplicateRiskFor } from "@ghost/core/classifier/exception"; import { describeActions, planCompensation, remainingEntries } from "@ghost/core/compensate"; import { journalFromEvents, journalFromLegacyRunSteps } from "@ghost/core/journal"; import { RUN_EVENT_TYPES } from "@ghost/core/run-events"; @@ -96,7 +97,7 @@ export async function GET(_req: Request, { params }: { params: Promise<{ id: str }); } -export async function POST(_req: Request, { params }: { params: Promise<{ id: string }> }) { +export async function POST(req: Request, { params }: { params: Promise<{ id: string }> }) { const session = await auth(); if (!session?.user?.orgId) { return NextResponse.json({ error: "unauthorized" }, { status: 401 }); @@ -105,6 +106,13 @@ export async function POST(_req: Request, { params }: { params: Promise<{ id: st const userId = session.user.id ?? null; const { id } = await params; + // Undo took no body until the duplicate-risk gate below needed one. Tolerant + // of an absent or malformed body so an existing caller that sends nothing + // still works — it simply will not be treated as having acknowledged. + const body = (await req.json().catch(() => ({}))) as { + acknowledgeDuplicateRisk?: boolean; + }; + const loaded = await loadPlan(id, orgId); if (!loaded) return NextResponse.json({ error: "not found" }, { status: 404 }); @@ -141,10 +149,63 @@ export async function POST(_req: Request, { params }: { params: Promise<{ id: st ); } + // Snapshot the routing fields before clearing them. The catch below promises + // to give the claim back if scheduling fails, and a run returned to INCIDENT + // without its classification and owner has silently lost work — the exception + // reappears in the queue unassigned and labelled "Unclassified failure". + // A reversal that already failed indeterminately must not be re-scheduled on + // a single click. + // + // `compensateRun` records OUTCOME_UNKNOWN when a reversal action was in + // flight — a Cancel or Refund that timed out may well have reached the target. + // Storing that classification is worthless without an enforcement path, and + // this route was the gap: it accepted the resulting INCIDENT and rescheduled + // the same remaining reversal, under a compensation approval that is still + // APPROVED, with no acknowledgement. That is the forward gate's exact failure + // mode reached from the reversal side, so it gets the same treatment. + // + // The step is not passed to `duplicateRiskFor` deliberately: the risky effect + // here is the *reversal's* action, not the forward step at this cursor, and + // omitting it fails closed. + if ( + run.status === "INCIDENT" && + duplicateRiskFor({ storedKind: run.incidentKind }) && + body.acknowledgeDuplicateRisk !== true + ) { + return NextResponse.json( + { + error: + "the previous reversal of this run may already have taken effect before it failed, " + + "so reversing again could repeat it. Confirm in the target system first, then retry " + + "with acknowledgeDuplicateRisk: true.", + kind: run.incidentKind, + requiresAcknowledgement: true, + }, + { status: 409 }, + ); + } + + const priorRouting = { + incidentKind: run.incidentKind, + incidentAssigneeId: run.incidentAssigneeId, + incidentRaisedAt: run.incidentRaisedAt, + }; + // Conditional update: a double-clicked Undo enqueues once. const { count } = await prisma.run.updateMany({ where: { id, orgId, status: { in: [...REVERSIBLE_STATES] as never } }, - data: { status: "COMPENSATING", error: null, endedAt: null }, + // Clear the exception routing along with the error. A run reversing is no + // longer an open exception, so it must leave the queue; and if the reversal + // itself later fails, compensateRun raises a *fresh* incident with its own + // kind, owner and timestamp rather than silently inheriting these. + data: { + status: "COMPENSATING", + error: null, + endedAt: null, + incidentKind: null, + incidentAssigneeId: null, + incidentRaisedAt: null, + }, }); if (count !== 1) { return NextResponse.json({ error: "run is already being reversed" }, { status: 409 }); @@ -181,7 +242,16 @@ export async function POST(_req: Request, { params }: { params: Promise<{ id: st await prisma.run .updateMany({ where: { id, orgId, status: "COMPENSATING" }, - data: { status: run.status, error: run.error, endedAt: run.endedAt }, + // Including the routing fields the update above cleared. Restoring only + // status/error/endedAt returned an INCIDENT run to the queue stripped of + // its classification and its owner — "restore exactly what was there" + // has to mean all of it. + data: { + status: run.status, + error: run.error, + endedAt: run.endedAt, + ...priorRouting, + }, }) .catch(() => undefined); return NextResponse.json( diff --git a/cloud/apps/web/src/app/api/settings/members/[userId]/route.ts b/cloud/apps/web/src/app/api/settings/members/[userId]/route.ts index 863996c..3a9d207 100644 --- a/cloud/apps/web/src/app/api/settings/members/[userId]/route.ts +++ b/cloud/apps/web/src/app/api/settings/members/[userId]/route.ts @@ -124,6 +124,16 @@ export async function DELETE(_req: Request, { params }: Params): Promise(null); + const [value, setValue] = useState(assignee?.id ?? ""); + // `pending` from useTransition only becomes true *after* the fetch resolves, + // so it leaves the select enabled for the whole request. Two quick changes + // then race, and the slower one can win on the server while the control shows + // the faster one. This tracks the request itself, and a version counter makes + // a superseded response a no-op rather than something that rewrites state. + const [sending, setSending] = useState(false); + const latest = useRef(0); + + async function assign(next: string) { + const ticket = ++latest.current; + setError(null); + setValue(next); + setSending(true); + const res = await fetch(`/api/runs/${runId}/incident`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + action: "assign", + assigneeId: next === "" ? null : next, + }), + }); + // A response that has been overtaken must not touch state at all — not the + // value, not the error. + if (ticket !== latest.current) return; + setSending(false); + + if (!res.ok) { + const body = (await res.json().catch(() => ({}))) as { error?: string }; + if (ticket !== latest.current) return; + setError(body.error ?? "could not assign"); + // Put the control back to the truth, so it never shows an assignment the + // server rejected. + setValue(assignee?.id ?? ""); + return; + } + startTransition(() => router.refresh()); + } + + return ( +
+ + + {error && {error}} +
+ ); +} diff --git a/cloud/apps/web/src/components/run-timeline.tsx b/cloud/apps/web/src/components/run-timeline.tsx index cbe0796..4ee5674 100644 --- a/cloud/apps/web/src/components/run-timeline.tsx +++ b/cloud/apps/web/src/components/run-timeline.tsx @@ -44,6 +44,17 @@ interface RunView { * this exists so nobody discovers the rule by having a click rejected. */ canApprove: boolean; + /** Deterministic disposition of the stop. Non-null only for INCIDENT. */ + exception: { + kind: string; + owner: string; + headline: string; + guidance: string; + retryUseful: boolean; + retryMayDuplicate: boolean; + /** When this incident was raised — echoed back to bind the confirmation. */ + raisedAt: string | null; + } | null; } interface ChainView { org: { intact: boolean; count: number }; @@ -105,6 +116,18 @@ export function RunTimeline({ runId }: { runId: string }) { const [undo, setUndo] = useState(null); const [busy, setBusy] = useState(false); const [notice, setNotice] = useState(null); + // Two-step confirmation for a retry whose effect may already have happened. + // + // Holds the *identity* of the incident it was opened for, not just a boolean. + // The page is fed by SSE, so `run` changes underneath an open dialog: if + // another operator resolves this incident and the run then parks on a new + // risky step, a bare flag would let the still-open button acknowledge a step + // its clicker never read. The route refuses a mismatch; this keeps the UI + // honest too, by closing the dialog the moment it stops describing what is + // actually on screen. + const [confirmRetry, setConfirmRetry] = useState< + { stepIndex: number; raisedAt: string | null } | null + >(null); const load = useCallback(async () => { const res = await fetch(`/api/runs/${runId}`, { cache: "no-store" }); @@ -306,12 +329,19 @@ export function RunTimeline({ runId }: { runId: string }) { {incidentStep?.label ? ` — ${incidentStep.label}` : ""} - {incidentStep?.status === "UNKNOWN" && ( -

- This step started but never reported an outcome, and its effect cannot safely be - repeated. Check the target system before retrying — Ghost will not guess whether it - took effect. -

+ {run.exception && ( +
+

+ {run.exception.headline} +

+

{run.exception.guidance}

+
)} {run.restoreScreenshotUrl && (
@@ -328,23 +358,85 @@ export function RunTimeline({ runId }: { runId: string }) { />
)} -
- - -
+ {/* + Retry is de-emphasised when the classifier says it cannot help (a + changed selector fails identically) and becomes a two-step + confirmation when it could repeat an effect that already happened. + The route enforces the same rule — it refuses an unacknowledged + risky retry with a 409 — so this is the honest affordance for a + decision the server will demand anyway, not a client-side guard + standing in for one. + */} + {confirmRetry && + confirmRetry.stepIndex === run.cursor && + confirmRetry.raisedAt === (run.exception?.raisedAt ?? null) ? ( +
+

+ Retry anyway? This step may already have taken effect, and retrying could repeat + it. Confirm in the target system first. +

+
+ + +
+
+ ) : ( +
+ + +
+ )} )} diff --git a/cloud/apps/web/src/lib/run-view.ts b/cloud/apps/web/src/lib/run-view.ts index 590e0d6..04f663b 100644 --- a/cloud/apps/web/src/lib/run-view.ts +++ b/cloud/apps/web/src/lib/run-view.ts @@ -2,6 +2,13 @@ import { prisma } from "@/lib/db"; import { canApproveRun } from "@ghost/core/roles"; import { throttleReason } from "@ghost/core/concurrency"; import { loadActor } from "@/lib/members"; +import { parseWorkflowSteps } from "@ghost/core/schema/step"; +import { + classifyException, + dispositionForKind, + duplicateRiskFor, + type ExceptionKind, +} from "@ghost/core/classifier/exception"; /** * Build the run detail view: status, ordered steps, pending approvals, and @@ -107,8 +114,68 @@ export async function buildRunView(orgId: string, viewerId: string, runId: strin } } + // Exception disposition, for an INCIDENT run only. Computed here so the + // timeline can lead with what kind of problem this is and whether retrying + // risks repeating an effect, instead of a raw driver error and two + // equal-weight buttons. + // + // The stored `incidentKind` is what the engine decided when it stopped and is + // what an auditor should see; the live computation supplies the guidance text + // and — always — the cautious answer on duplicate risk, so a stale stored + // label can never downgrade a warning. + let exception: { + kind: ExceptionKind; + owner: string; + headline: string; + guidance: string; + retryUseful: boolean; + retryMayDuplicate: boolean; + raisedAt: string | null; + } | null = null; + + if (run.status === "INCIDENT") { + let stoppedStep; + try { + stoppedStep = parseWorkflowSteps(run.workflowVersion.steps)[run.cursor]; + } catch { + stoppedStep = undefined; + } + const recorded = run.steps.find((s) => s.index === run.cursor); + const d = classifyException({ + reason: run.error ?? "", + step: stoppedStep, + recordedOutcome: + recorded?.status === "UNKNOWN" ? "UNKNOWN" : recorded?.status === "FAILED" ? "FAILED" : null, + }); + const kind = (run.incidentKind as ExceptionKind | null) ?? d.kind; + // Every displayed field comes from the kind actually shown — see the + // exceptions route for why mixing the two sources produced contradictions. + const shown = dispositionForKind(kind); + exception = { + kind, + owner: shown.owner, + headline: shown.headline, + guidance: shown.guidance, + retryUseful: shown.retryUseful, + // Shared helper rather than an inline OR: forcing this true whenever the + // kind or recorded status is UNKNOWN also fired it for an indeterminate + // `verify` or `extract`, which are reads that cost nothing to repeat. The + // confirmation prompt has to stay rare to stay meaningful. + // Identity of this incident, echoed back by the confirm dialog so the + // server can refuse an acknowledgement raised for a different one. + raisedAt: run.incidentRaisedAt?.toISOString() ?? null, + retryMayDuplicate: duplicateRiskFor({ + disposition: d, + storedKind: kind, + recordedOutcome: recorded?.status === "UNKNOWN" ? "UNKNOWN" : null, + step: stoppedStep, + }), + }; + } + return { throttle, + exception, id: run.id, status: run.status, error: run.error, diff --git a/cloud/apps/web/src/middleware.ts b/cloud/apps/web/src/middleware.ts index 291edb2..62b9faa 100644 --- a/cloud/apps/web/src/middleware.ts +++ b/cloud/apps/web/src/middleware.ts @@ -71,6 +71,7 @@ export const config = { matcher: [ "/audit/:path*", "/dashboard/:path*", + "/exceptions/:path*", "/recordings/:path*", "/runs/:path*", "/settings/:path*", diff --git a/cloud/apps/worker/src/jobs/compensateRun.ts b/cloud/apps/worker/src/jobs/compensateRun.ts index a173dbb..e0abf23 100644 --- a/cloud/apps/worker/src/jobs/compensateRun.ts +++ b/cloud/apps/worker/src/jobs/compensateRun.ts @@ -21,6 +21,37 @@ import { resolveStep, ExpressionError, type ExpressionScope } from "@ghost/core/ import { BrowserSession, applyStep, verifyStep } from "../browser/driver.js"; import { artifactStore, compensationScreenshotKey } from "../storage/artifacts.js"; import { claimSlot, recordThrottle, releaseSlot } from "../runtime/slots.js"; +import { classifyException, type ExceptionKind } from "@ghost/core/classifier/exception"; + +/** + * Routing fields for an incident raised by a *failed reversal*. + * + * A reversal that stops is a new exception, and the run may already be carrying + * `incidentKind` / `incidentAssigneeId` from whatever stopped it in the forward + * direction. Without overwriting all three fields here, the failed undo inherits + * that classification and owner — so "refusing to reverse a run whose journal is + * broken" could land in the queue labelled as a changed selector, on the desk of + * whoever happened to own the earlier problem, with a "parked since" time from + * hours ago. + * + * The assignee is deliberately cleared rather than kept: a reversal failure is a + * different problem from the one that person accepted, so it goes back to the + * unassigned queue to be triaged on its own terms. + * + * No step is passed to the classifier. These reasons are Ghost's own prose about + * the reversal itself, not a driver error about a step, and duplicate risk for + * the one indeterminate case is asserted by passing `OUTCOME_UNKNOWN` directly + * at the call site. + */ +function freshIncidentRouting( + input: { kind: ExceptionKind } | { reason: string }, +): { incidentKind: string; incidentAssigneeId: null; incidentRaisedAt: Date } { + return { + incidentKind: "kind" in input ? input.kind : classifyException({ reason: input.reason }).kind, + incidentAssigneeId: null, + incidentRaisedAt: new Date(), + }; +} /** * Reverse a run's completed side effects — the BPMN saga, over the run journal. @@ -120,6 +151,15 @@ export async function compensateRunJob(job: Job): Promise undefined); @@ -143,6 +183,9 @@ export async function compensateRunJob(job: Job): Promise undefined); @@ -189,6 +232,9 @@ export async function compensateRunJob(job: Job): Promise): Promise { : "JOURNAL_TAMPERED: run journal tail does not match its expected head"; // Appending here would move journalHead to the forged tail and erase // the evidence. Quarantine the run and use the independent org chain. - await prisma.run.update({ where: { id: runId }, data: { status: "INCIDENT", error: reason } }); + await prisma.run.update({ + where: { id: runId }, + // Stamped like every other incident transition, so a quarantined run + // is visible in the exception queue rather than sorted past the cap + // by a null timestamp. UNKNOWN: a broken journal is not a step + // failure and none of the kinds names it. + data: { + status: "INCIDENT", + error: reason, + incidentKind: "UNKNOWN", + incidentAssigneeId: null, + incidentRaisedAt: new Date(), + }, + }); await appendAuditEvent(run.orgId, run.triggeredById, { action: "run.journal_tampered", entityType: "Run", @@ -346,7 +360,14 @@ export async function runWorkflowJob(job: Job): Promise { // approval or a broken expression is not recoverable — retrying cannot // change either — so those stay terminal. if (action.recoverable) { - await raiseIncident(runId, run.orgId, run.triggeredById, action.index, action.reason); + await raiseIncident( + runId, + run.orgId, + run.triggeredById, + action.index, + action.reason, + steps[action.index], + ); } else { await failRun(runId, run.orgId, run.triggeredById, action.index, action.reason); } @@ -381,6 +402,7 @@ export async function runWorkflowJob(job: Job): Promise { run.triggeredById, action.index, `OUTCOME_UNKNOWN: ${action.reason}`, + action.step, ); return; } @@ -572,6 +594,7 @@ async function executeStep(args: ExecuteArgs): Promise { actorId, index, `RESTORE_UNSAFE: ${restored.reason}`, + step, ); return { kind: "halt" }; } @@ -663,7 +686,14 @@ async function executeStep(args: ExecuteArgs): Promise { }); if (!passed) { - await raiseIncident(runId, orgId, actorId, index, `verification failed at step ${index}`); + await raiseIncident( + runId, + orgId, + actorId, + index, + `verification failed at step ${index}`, + step, + ); return { kind: "halt" }; } @@ -691,6 +721,7 @@ async function executeStep(args: ExecuteArgs): Promise { actorId, index, `OUTCOME_UNKNOWN: step ${index} may or may not have taken effect — ${message}`, + step, ); return { kind: "halt" }; } @@ -720,12 +751,19 @@ async function executeStep(args: ExecuteArgs): Promise { entityId: `${runId}:${index}`, metadata: { error: message }, }); - await raiseIncident(runId, orgId, actorId, index, message); + await raiseIncident(runId, orgId, actorId, index, message, step); return { kind: "halt" }; } } - await raiseIncident(runId, orgId, actorId, index, lastError || "step exhausted its retries"); + await raiseIncident( + runId, + orgId, + actorId, + index, + lastError || "step exhausted its retries", + step, + ); return { kind: "halt" }; } @@ -916,6 +954,17 @@ async function failRun( * A failed or unresumable step stops the run and waits for a human, rather than * killing it. Borrowed from Camunda incidents: an ops person watching a * 40-invoice batch wants to fix the one broken step, not restart the batch. + * + * The incident is *classified* here, when it is raised, rather than on read. Two + * reasons: the queue can then filter and sort in SQL instead of re-classifying + * every open incident on every page load, and `Run.error` can be overwritten by + * a later failure on the same run — so a disposition computed on read might not + * be the one the engine actually stopped on. The audit event records the + * classification next to the reason, which is what lets someone later ask "what + * did Ghost think this was at the time?" and get an answer that cannot drift. + * + * Classification never changes control flow. It decides what a human is *shown* + * and whose desk the work lands on; the state transition is identical either way. */ async function raiseIncident( runId: string, @@ -923,16 +972,45 @@ async function raiseIncident( actorId: string | null, index: number, reason: string, + step?: WorkflowStep, ): Promise { + // The recorded step outcome outranks the reason text: `UNKNOWN` is written + // only after the engine has decided an effect is genuinely indeterminate, and + // that judgement must survive a reason string that reads like a plain timeout. + const recorded = await prisma.runStep + .findUnique({ where: { runId_index: { runId, index } }, select: { status: true } }) + .catch(() => null); + const recordedOutcome = + recorded?.status === "UNKNOWN" ? "UNKNOWN" : recorded?.status === "FAILED" ? "FAILED" : null; + const disposition = classifyException({ reason, step, recordedOutcome }); + await prisma.run.update({ where: { id: runId }, - data: { status: "INCIDENT", error: reason, cursor: index }, + data: { + status: "INCIDENT", + error: reason, + cursor: index, + incidentKind: disposition.kind, + incidentRaisedAt: new Date(), + // A newly raised incident is unassigned. Resolution already clears the + // assignee, so in the normal path this is a no-op; it closes the window + // where an assignment raced a resolution and left an owner attached to a + // run that then stopped again for an unrelated reason. + incidentAssigneeId: null, + }, }); await appendAuditEvent(orgId, actorId, { action: "run.incident", entityType: "Run", entityId: runId, - metadata: { stepIndex: index, reason, runChainHead: await runChainHead(runId) }, + metadata: { + stepIndex: index, + reason, + kind: disposition.kind, + owner: disposition.owner, + retryMayDuplicate: disposition.retryMayDuplicate, + runChainHead: await runChainHead(runId), + }, }); } diff --git a/cloud/docs/CURSOR_HANDOFF.md b/cloud/docs/CURSOR_HANDOFF.md index c1141e5..d8d9763 100644 --- a/cloud/docs/CURSOR_HANDOFF.md +++ b/cloud/docs/CURSOR_HANDOFF.md @@ -124,6 +124,43 @@ hash-chained `AuditEvent`. - MCP: `pnpm --filter @ghost/mcp exec tsx src/index.ts` - Doc: `cloud/docs/AGENT_PLUGIN.md` +## Exception routing (done) + +Incidents are now a product surface, not just a status. `classifyException` +(`packages/core/src/classifier/exception.ts`) is a third deterministic classifier +alongside `sensitive.ts` and `replay.ts`; `raiseIncident` runs it when the +incident is raised and stores the verdict on `Run.incidentKind`, with +`Run.incidentRaisedAt` recording when the run parked and `Run.incidentAssigneeId` +its owner. + +- **Queue:** `GET /api/exceptions` (read-only) and `/exceptions`. Query params + `mine=1`, `kind=`, `owner=`. Both filters are pushed into SQL — `owner` + translates to an `incidentKind IN (…)` list via `kindsForOwner`, since owner is + a pure function of kind — and both admit `incidentKind: null` so rows predating + the column still classify on read. Ordered by `incidentRaisedAt`, **not** + `createdAt`: a week-old run that fails now has been parked for a minute. +- **Resolution:** unchanged route, `POST /api/runs/[id]/incident`, now taking + `assign` (with `assigneeId`, tenant-checked, audited in one transaction with + the state change) alongside `retry` / `skip`. +- **Risky retry:** a retry whose effect may already have happened is refused with + **409** unless the caller passes `acknowledgeDuplicateRisk: true`; the + acknowledgement lands in the run journal and the audit log. The outcome lookup + that decides this deliberately does **not** swallow database errors — a failed + lookup returns 503 rather than defaulting to the permissive answer. +- **Duplicate risk** is `duplicateRiskFor`, shared by all three read paths. It + unions the live verdict with the stored label *and* requires the step to be + mutating per `replaySafety`, so an indeterminate `verify`/`extract` is not + flagged. +- **Lifecycle:** routing fields clear whenever a run leaves `INCIDENT` (retry, + skip, undo). A failed reversal is re-routed from scratch in `compensateRun` — + it must not inherit the forward incident's kind, owner or wait time. Removing a + member unassigns their open exceptions (the FK's `SET NULL` fires only on + *account* deletion, not membership removal). + +Not built: any notification. The queue is pull-only, so nobody is told an +exception arrived — see "Remaining Phase 1 work". `DATA` exceptions are named but +not fixable; retry re-sends the same rejected value. + ## Phase 1.3 — durable execution (done) Run position is now a fold over an append-only, hash-chained `RunEvent` journal diff --git a/cloud/packages/core/package.json b/cloud/packages/core/package.json index dd2f7be..852238f 100644 --- a/cloud/packages/core/package.json +++ b/cloud/packages/core/package.json @@ -10,6 +10,7 @@ "./schema/step": "./src/schema/step.ts", "./classifier": "./src/classifier/sensitive.ts", "./classifier/replay": "./src/classifier/replay.ts", + "./classifier/exception": "./src/classifier/exception.ts", "./queue": "./src/queue.ts", "./audit": "./src/audit.ts", "./audit-log": "./src/auditLog.ts", diff --git a/cloud/packages/core/prisma/migrations/20260817170000_run_exception_routing/migration.sql b/cloud/packages/core/prisma/migrations/20260817170000_run_exception_routing/migration.sql new file mode 100644 index 0000000..a7fa818 --- /dev/null +++ b/cloud/packages/core/prisma/migrations/20260817170000_run_exception_routing/migration.sql @@ -0,0 +1,22 @@ +-- Exception routing: classify an incident when it is raised, and let a human own it. +-- +-- Both columns are nullable with no default: every existing row is either not an +-- incident (so they are correctly null) or is a pre-existing incident that was +-- never classified. The queue renders an unclassified incident as UNKNOWN rather +-- than backfilling a guess, because the reason string it would have to guess from +-- may since have been overwritten by a later failure on the same run. + +-- AlterTable +ALTER TABLE "Run" ADD COLUMN "incidentKind" TEXT, +ADD COLUMN "incidentAssigneeId" TEXT; + +-- CreateIndex +CREATE INDEX "Run_orgId_status_createdAt_idx" ON "Run"("orgId", "status", "createdAt"); + +-- CreateIndex +CREATE INDEX "Run_incidentAssigneeId_idx" ON "Run"("incidentAssigneeId"); + +-- AddForeignKey +-- ON DELETE SET NULL: removing a person from the org returns their open +-- exceptions to the unassigned queue. It must never cascade a run away. +ALTER TABLE "Run" ADD CONSTRAINT "Run_incidentAssigneeId_fkey" FOREIGN KEY ("incidentAssigneeId") REFERENCES "User"("id") ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/cloud/packages/core/prisma/migrations/20260817175000_incident_raised_at/migration.sql b/cloud/packages/core/prisma/migrations/20260817175000_incident_raised_at/migration.sql new file mode 100644 index 0000000..4fa15f5 --- /dev/null +++ b/cloud/packages/core/prisma/migrations/20260817175000_incident_raised_at/migration.sql @@ -0,0 +1,18 @@ +-- Track when a run entered INCIDENT, so the exception queue can order by how +-- long something has actually been parked rather than by how old the run is. +-- +-- Backfilled to NULL, not to `createdAt`: guessing would assert that a run +-- created last week has been waiting since last week, which is the exact +-- misreading this column exists to remove. The queue falls back to `createdAt` +-- for these pre-existing rows and says so, rather than inventing a timestamp. + +-- AlterTable +ALTER TABLE "Run" ADD COLUMN "incidentRaisedAt" TIMESTAMP(3); + +-- The previous index was on (orgId, status, createdAt); the queue no longer +-- orders on createdAt, so it is replaced rather than added to. +-- DropIndex +DROP INDEX IF EXISTS "Run_orgId_status_createdAt_idx"; + +-- CreateIndex +CREATE INDEX "Run_orgId_status_incidentRaisedAt_idx" ON "Run"("orgId", "status", "incidentRaisedAt"); diff --git a/cloud/packages/core/prisma/schema.prisma b/cloud/packages/core/prisma/schema.prisma index ad0e72a..2b72b24 100644 --- a/cloud/packages/core/prisma/schema.prisma +++ b/cloud/packages/core/prisma/schema.prisma @@ -39,6 +39,7 @@ model User { memberships Membership[] createdFlows Workflow[] @relation("WorkflowCreatedBy") triggeredRuns Run[] @relation("RunTriggeredBy") + assignedIncidents Run[] @relation("RunIncidentAssignee") resolvedApprovals Approval[] @relation("ApprovalResolvedBy") agentCredentials AgentCredential[] invitationsSent Invitation[] @relation("InvitationCreatedBy") @@ -422,9 +423,42 @@ model Run { // retention window. artifactsPurgedAt DateTime? + // Exception routing. Both are denormalized caches for the queue view, in the + // same sense as `cursor`: written in the same transaction that raises the + // incident, never the source of truth for a control-flow decision. + // + // `incidentKind` is the deterministic classifier's verdict + // (packages/core/src/classifier/exception.ts) recorded at the moment the run + // stopped, so the queue filters and sorts in SQL instead of re-classifying + // every row on read. Stored rather than derived because the reason string it + // was computed from can be superseded by a later failure on the same run, and + // "what did Ghost think this was when it stopped?" deserves the answer from + // that moment. + // + // Both are cleared when the run leaves INCIDENT — a resolved exception must + // not linger in the queue, nor stay assigned to whoever finished with it. + incidentKind String? + incidentAssigneeId String? + + // When this run entered INCIDENT. + // + // The queue orders by this, NOT by `createdAt`: those differ, and the + // difference is the whole point of the ordering. A run created a week ago that + // fails in the last minute has been *parked* for a minute, and sorting it + // ahead of yesterday's stuck run — which is what `createdAt` does — buries + // exactly the row that has been waiting. It is also what the UI's "stopped Nm + // ago" reads, so an age of "6d" no longer means "this run is 6 days old." + // + // Rewritten on every fresh incident, including one raised by a failed + // reversal, so an inherited timestamp cannot make new work look stale. + incidentRaisedAt DateTime? + org Organization @relation(fields: [orgId], references: [id], onDelete: Cascade) workflowVersion WorkflowVersion @relation(fields: [workflowVersionId], references: [id], onDelete: Cascade) triggeredBy User? @relation("RunTriggeredBy", fields: [triggeredById], references: [id], onDelete: SetNull) + // SetNull, not Cascade: removing a person from the org must never delete the + // run they were looking at. The exception returns to unassigned. + incidentAssignee User? @relation("RunIncidentAssignee", fields: [incidentAssigneeId], references: [id], onDelete: SetNull) steps RunStep[] approvals Approval[] events RunEvent[] @@ -432,6 +466,10 @@ model Run { @@index([orgId]) @@index([workflowVersionId]) @@index([status, leaseExpiresAt]) + // The exception queue's own query: this org's open incidents, longest-parked + // first. Ordered on incidentRaisedAt, not createdAt — see that field. + @@index([orgId, status, incidentRaisedAt]) + @@index([incidentAssigneeId]) // Admission counts a workflow's in-flight runs on every start and resume. @@index([workflowVersionId, status]) @@index([workflowVersionId, slotHeldAt]) diff --git a/cloud/packages/core/src/classifier/exception.test.ts b/cloud/packages/core/src/classifier/exception.test.ts new file mode 100644 index 0000000..e7eb01b --- /dev/null +++ b/cloud/packages/core/src/classifier/exception.test.ts @@ -0,0 +1,381 @@ +import { describe, expect, it } from "vitest"; +import { + EXCEPTION_KINDS, + classifyException, + dispositionForKind, + duplicateRiskFor, + kindsForOwner, + needsAuthoring, + type ExceptionKind, +} from "./exception.js"; +import type { WorkflowStep } from "../schema/step.js"; + +const click: WorkflowStep = { + id: "s1", + type: "click", + selector: { css: "#pay" }, +}; + +const verify: WorkflowStep = { + id: "s2", + type: "verify", + assertion: { kind: "textPresent", expected: "Paid" }, +}; + +function kindOf(reason: string, step?: WorkflowStep): ExceptionKind { + return classifyException({ reason, step }).kind; +} + +describe("classifyException — authoritative signals win", () => { + it("treats a recorded UNKNOWN outcome as OUTCOME_UNKNOWN regardless of the reason text", () => { + // The reason here looks like a plain transient timeout. The recorded outcome + // is the engine's own considered judgement and must override it, or a + // possibly-completed payment would be offered a one-click retry. + const d = classifyException({ + reason: "Timeout 30000ms exceeded", + step: click, + recordedOutcome: "UNKNOWN", + }); + expect(d.kind).toBe("OUTCOME_UNKNOWN"); + expect(d.retryMayDuplicate).toBe(true); + }); + + it("reads Ghost's own structured prefixes", () => { + expect( + kindOf("OUTCOME_UNKNOWN: step 4 may or may not have taken effect"), + ).toBe("OUTCOME_UNKNOWN"); + expect( + kindOf("RESTORE_UNSAFE: captured session digest did not match"), + ).toBe("RESTORE_UNSAFE"); + expect(kindOf("RUN_TIMEOUT: exceeded wall-clock budget of 600000ms")).toBe( + "TRANSIENT", + ); + }); + + it("reads the unprefixed phrases the engine writes", () => { + expect(kindOf("approval for step 3 expired before it was used")).toBe( + "APPROVAL_EXPIRED", + ); + expect(kindOf("verification failed at step 7")).toBe("VERIFICATION"); + }); + + it("does not sweep up unrelated errors that merely mention approval", () => { + // Whole-phrase anchoring, not keyword matching: this is a missing button on + // an approvals page, which is an authoring problem, not an expired gate. + expect(kindOf('waiting for locator("#approval-banner")')).toBe( + "TARGET_MISSING", + ); + }); +}); + +describe("classifyException — best-effort text rules", () => { + it("files a Playwright locator timeout as a target change, not a blip", () => { + // Regression guard for rule ordering: this string contains "Timeout", so a + // generic timeout rule placed first would misfile every changed selector as + // transient and retry it forever. + const reason = + 'Timeout 30000ms exceeded.\nCall log:\n - waiting for locator("#submit-order")'; + expect(kindOf(reason)).toBe("TARGET_MISSING"); + }); + + it("files strict-mode violations with the author too", () => { + expect( + kindOf("strict mode violation: locator resolved to 3 elements"), + ).toBe("TARGET_MISSING"); + }); + + it("recognizes auth refusals", () => { + expect(kindOf("Request failed with status 403")).toBe("AUTH"); + expect(kindOf("Your session has expired, please sign in again")).toBe( + "AUTH", + ); + }); + + it("recognizes rejected values", () => { + expect(kindOf("Invalid value for field 'amount'")).toBe("DATA"); + expect(kindOf("Purchase order number is required")).toBe("DATA"); + }); + + it("recognizes network and lifecycle noise as transient", () => { + expect( + kindOf("net::ERR_CONNECTION_RESET at https://portal.example.com"), + ).toBe("TRANSIENT"); + expect(kindOf("Target page, context or browser has been closed")).toBe( + "TRANSIENT", + ); + expect(kindOf("Request failed with status 503")).toBe("TRANSIENT"); + }); + + it("falls back to a bare generic timeout as transient", () => { + expect(kindOf("Timeout 5000ms exceeded")).toBe("TRANSIENT"); + }); +}); + +describe("semantic Playwright locators route to the author, not to retry", () => { + // apps/worker/src/browser/selector.ts resolves every *preferred* selector + // through getByRole/getByTestId/getByText, and Playwright's call log then says + // "waiting for getByRole(...)" — the word "locator" never appears. Matching + // only /locator|selector/ missed exactly the selectors Ghost prefers and + // routed a changed page to an operator as a transient blip. + it.each([ + ['Timeout 30000ms exceeded.\nCall log:\n - waiting for getByRole(\'button\', { name: \'Pay\' })', "getByRole"], + ['Timeout 30000ms exceeded.\nCall log:\n - waiting for getByTestId(\'submit\')', "getByTestId"], + ['Timeout 30000ms exceeded.\nCall log:\n - waiting for getByText(\'Continue\')', "getByText"], + ['Timeout 5000ms exceeded.\nCall log:\n - waiting for getByLabel(\'Amount\')', "getByLabel"], + ['Timeout 5000ms exceeded.\nCall log:\n - waiting for getByPlaceholder(\'Search\')', "getByPlaceholder"], + ])("files a %s timeout as TARGET_MISSING", (reason) => { + expect(kindOf(reason, click)).toBe("TARGET_MISSING"); + }); + + it("routes those to the author rather than telling an operator to retry", () => { + const d = classifyException({ + reason: "Timeout 30000ms exceeded.\nCall log:\n - waiting for getByRole('button')", + step: click, + }); + expect(d.owner).toBe("author"); + expect(d.retryUseful).toBe(false); + }); +}); + +describe("duplicateRiskFor", () => { + it("is false when nothing is indeterminate", () => { + expect( + duplicateRiskFor({ + disposition: classifyException({ reason: "net::ERR_CONNECTION_RESET", step: click }), + storedKind: "TRANSIENT", + recordedOutcome: "FAILED", + step: click, + }), + ).toBe(false); + }); + + it("is true for an indeterminate mutating step", () => { + expect( + duplicateRiskFor({ storedKind: "OUTCOME_UNKNOWN", step: click }), + ).toBe(true); + }); + + it("stays FALSE for an indeterminate read, however the signal arrives", () => { + // The regression this helper exists for: three call sites each wrote + // `kind === "OUTCOME_UNKNOWN" || recorded === "UNKNOWN"`, which forced the + // duplicate-effect prompt on for a `verify` — a read that costs nothing to + // repeat. A prompt that fires on reads gets clicked through. + expect(duplicateRiskFor({ storedKind: "OUTCOME_UNKNOWN", step: verify })).toBe(false); + expect(duplicateRiskFor({ recordedOutcome: "UNKNOWN", step: verify })).toBe(false); + expect( + duplicateRiskFor({ + disposition: classifyException({ reason: "OUTCOME_UNKNOWN: ?", step: verify }), + step: verify, + }), + ).toBe(false); + }); + + it("lets a stale stored kind raise, but never lower, the risk", () => { + // Stored label says benign, recorded outcome says indeterminate: the union + // must still warn. + expect( + duplicateRiskFor({ storedKind: "TRANSIENT", recordedOutcome: "UNKNOWN", step: click }), + ).toBe(true); + // And the reverse — a stale OUTCOME_UNKNOWN label on a read stays quiet, + // because the step, not the label, decides whether an effect could repeat. + expect( + duplicateRiskFor({ storedKind: "OUTCOME_UNKNOWN", recordedOutcome: "FAILED", step: verify }), + ).toBe(false); + }); + + it("assumes risk when the step is unknown", () => { + expect(duplicateRiskFor({ storedKind: "OUTCOME_UNKNOWN" })).toBe(true); + }); +}); + +describe("a failed verification on a mutating step is duplicate risk", () => { + // The action ran — that is *why* there was something to assert — and the + // incident route's retry resets the step to PENDING and re-executes the whole + // step under the original approval. For a click on Pay that is a second + // payment, and nothing was warning about it. Stronger than uncertainty: here + // the effect is known to have landed. + it("flags a mutating step whose verification failed", () => { + expect( + duplicateRiskFor({ + disposition: classifyException({ reason: "verification failed at step 3", step: click }), + step: click, + }), + ).toBe(true); + }); + + it("does not flag a verification failure on a read", () => { + expect( + duplicateRiskFor({ + disposition: classifyException({ reason: "verification failed at step 3", step: verify }), + step: verify, + }), + ).toBe(false); + }); + + it("flags it from the stored kind too", () => { + expect(duplicateRiskFor({ storedKind: "VERIFICATION", step: click })).toBe(true); + }); + + it("still leaves genuinely safe kinds unflagged", () => { + for (const kind of ["TRANSIENT", "TARGET_MISSING", "AUTH", "DATA", "UNKNOWN"] as const) { + expect(duplicateRiskFor({ storedKind: kind, step: click })).toBe(false); + } + }); +}); + +describe("dispositionForKind", () => { + it("returns that kind's own owner, headline and guidance", () => { + const d = dispositionForKind("OUTCOME_UNKNOWN"); + expect(d.kind).toBe("OUTCOME_UNKNOWN"); + expect(d.owner).toBe("operator"); + expect(d.headline).toMatch(/already/i); + expect(d.retryUseful).toBe(false); + }); + + it("never mixes one kind's label with another's guidance", () => { + // The display bug this exists to prevent: a compensation incident stores an + // explicit kind whose reason text carries no classifier prefix, so a live + // re-classification returns UNKNOWN and the row showed "OUTCOME_UNKNOWN" + // beside "Unclassified failure". + for (const kind of EXCEPTION_KINDS) { + const d = dispositionForKind(kind); + expect(d).toEqual( + expect.objectContaining({ kind, ...({} as Record) }), + ); + expect(d.headline).toBe(dispositionForKind(kind).headline); + expect(d.guidance).not.toBe(""); + } + expect(dispositionForKind("UNKNOWN").headline).not.toBe( + dispositionForKind("OUTCOME_UNKNOWN").headline, + ); + }); +}); + +describe("kindsForOwner", () => { + it("partitions every kind across the three desks exactly once", () => { + const all = (["operator", "author", "administrator"] as const).flatMap((o) => kindsForOwner(o)); + expect(all.sort()).toEqual([...EXCEPTION_KINDS].sort()); + }); + + it("puts target changes on the author's desk", () => { + expect(kindsForOwner("author")).toContain("TARGET_MISSING"); + expect(kindsForOwner("administrator")).toEqual( + expect.arrayContaining(["AUTH", "APPROVAL_EXPIRED"]), + ); + }); +}); + +describe("classifyException — fails closed", () => { + it("leaves an unrecognized reason UNKNOWN rather than guessing", () => { + const d = classifyException({ + reason: "the frobnicator disagreed", + step: click, + }); + expect(d.kind).toBe("UNKNOWN"); + expect(d.retryUseful).toBe(false); + }); + + it("does not let the bare retry-exhausted wrapper imply a category", () => { + // This string carries no information about cause. Classifying it as + // transient would offer a retry on a step that has already exhausted its + // retries for an unknown reason. + expect(kindOf("step exhausted its retries")).toBe("UNKNOWN"); + }); + + it("treats an empty reason as UNKNOWN", () => { + expect(kindOf("")).toBe("UNKNOWN"); + }); + + it("assumes duplicate risk when the step is not known", () => { + // No step means the classifier cannot prove the effect was confined to the + // browser, so it must assume the dangerous case. + const d = classifyException({ reason: "OUTCOME_UNKNOWN: gone dark" }); + expect(d.retryMayDuplicate).toBe(true); + }); +}); + +describe("retryMayDuplicate is the conjunction, not a synonym for the kind", () => { + it("warns on an indeterminate mutating step", () => { + const d = classifyException({ reason: "OUTCOME_UNKNOWN: ?", step: click }); + expect(d.retryMayDuplicate).toBe(true); + }); + + it("does not warn on an indeterminate read", () => { + // Repeating a `verify` costs nothing. If this warned, the warning would + // appear often enough to be ignored when it matters. + const d = classifyException({ reason: "OUTCOME_UNKNOWN: ?", step: verify }); + expect(d.retryMayDuplicate).toBe(false); + }); + + it("never warns for kinds other than OUTCOME_UNKNOWN", () => { + for (const reason of [ + "net::ERR_CONNECTION_RESET", + 'waiting for locator("#x")', + "verification failed at step 2", + "Request failed with status 403", + ]) { + expect(classifyException({ reason, step: click }).retryMayDuplicate).toBe( + false, + ); + } + }); +}); + +describe("dispositions are complete and coherent", () => { + it("gives every kind an owner, headline, and guidance", () => { + for (const kind of EXCEPTION_KINDS) { + // Reach each kind through the classifier where a reason exists for it, + // and assert the table itself is filled in for all of them. + const d = classifyException({ reason: `${kind}:` }); + expect(d.headline.length).toBeGreaterThan(0); + expect(d.guidance.length).toBeGreaterThan(0); + expect(["operator", "author", "administrator"]).toContain(d.owner); + } + }); + + it("routes target changes to the author and auth to the administrator", () => { + expect(needsAuthoring("TARGET_MISSING")).toBe(true); + expect(needsAuthoring("TRANSIENT")).toBe(false); + expect(classifyException({ reason: "403 Forbidden" }).owner).toBe( + "administrator", + ); + expect( + classifyException({ reason: "approval for step 1 expired" }).owner, + ).toBe("administrator"); + }); + + it("only claims retry is useful for transient failures", () => { + // Every other kind recurs identically on retry. If this list grows, the + // guidance text for the new kind needs to justify it. + const useful = EXCEPTION_KINDS.filter( + (k) => classifyException({ reason: syntheticReasonFor(k) }).kind === k, + ).filter( + (k) => classifyException({ reason: syntheticReasonFor(k) }).retryUseful, + ); + expect(useful).toEqual(["TRANSIENT"]); + }); +}); + +/** A reason string that classifies to the given kind, for table-driven tests. */ +function syntheticReasonFor(kind: ExceptionKind): string { + switch (kind) { + case "TRANSIENT": + return "RUN_TIMEOUT: exceeded budget"; + case "TARGET_MISSING": + return 'waiting for locator("#x")'; + case "AUTH": + return "403 Forbidden"; + case "VERIFICATION": + return "verification failed at step 1"; + case "OUTCOME_UNKNOWN": + return "OUTCOME_UNKNOWN: dark"; + case "APPROVAL_EXPIRED": + return "approval for step 1 expired before it was used"; + case "RESTORE_UNSAFE": + return "RESTORE_UNSAFE: digest mismatch"; + case "DATA": + return "Invalid value for field 'x'"; + case "UNKNOWN": + return "no idea"; + } +} diff --git a/cloud/packages/core/src/classifier/exception.ts b/cloud/packages/core/src/classifier/exception.ts new file mode 100644 index 0000000..0493b57 --- /dev/null +++ b/cloud/packages/core/src/classifier/exception.ts @@ -0,0 +1,403 @@ +import type { WorkflowStep } from "../schema/step.js"; +import { replaySafety } from "./replay.js"; + +/** + * Deterministic exception classifier — the routing half of Camunda incidents. + * + * Third sibling of `classifyStep` (sensitive.ts) and `replaySafety` (replay.ts), + * held to the same discipline: pure, rule-based, exhaustive, fail-closed. The + * three answer different questions about the same step: + * + * | Classifier | Question | + * |---|---| + * | `classifyStep` | Must a human approve this before it runs? | + * | `replaySafety` | Is it safe to re-apply silently while restoring state? | + * | `classifyException` | This stopped. Who fixes it, and is retrying safe? | + * + * ## Why this is not a model call + * + * Engineering rule 1 — AI may propose, deterministic code executes. An incident + * disposition decides whether a human is *offered* a one-click retry on a step + * that may already have charged a customer. A model that classifies that wrong + * once has caused a double payment, so the mapping is a rule table that can be + * read, tested, and argued with. + * + * ## Two tiers of signal, and why the order matters + * + * 1. **Authoritative.** The recorded step outcome, and the structured prefixes + * Ghost itself emits (`OUTCOME_UNKNOWN:`, `RESTORE_UNSAFE:`, `RUN_TIMEOUT:`). + * These mean exactly what they say. + * 2. **Best-effort.** Raw driver text from Playwright or a target system. Useful + * — "waiting for locator" really does mean the page changed — but it is a + * third party's prose and may be reworded by an upgrade at any time. + * + * Tier 1 is checked first and wins outright. Tier 2 only ever refines an + * otherwise-`UNKNOWN` result, and an unrecognized reason stays `UNKNOWN` rather + * than being forced into the nearest-looking bucket. A wrong-but-confident label + * on an incident is worse than no label, because the whole point is telling a + * human where to look. + */ + +export type ExceptionKind = + /** Infrastructure or timing. Nothing is wrong with the workflow. */ + | "TRANSIENT" + /** The element is gone. The target UI changed under a recorded workflow. */ + | "TARGET_MISSING" + /** Credentials, session, or permission at the target system. */ + | "AUTH" + /** The step ran and the outcome was not what the workflow asserted. */ + | "VERIFICATION" + /** The step may or may not have taken effect. Retrying may repeat it. */ + | "OUTCOME_UNKNOWN" + /** An approval was granted but expired before the run could consume it. */ + | "APPROVAL_EXPIRED" + /** Page state could not be provably rebuilt, so the run refused to guess. */ + | "RESTORE_UNSAFE" + /** A value was rejected by the target system. */ + | "DATA" + /** Unrecognized. Needs human judgement — never assumed benign. */ + | "UNKNOWN"; + +/** + * Which desk an exception belongs on. + * + * This is the actual routing decision, and it is why this file exists rather + * than a label on a status badge. "A run stopped" is not actionable; "the portal + * changed and whoever maintains this workflow needs to re-record step 4" is. + */ +export type ExceptionOwner = + /** The person running the work. Can decide, retry, or escalate. */ + | "operator" + /** Whoever maintains this workflow. The steps themselves need changing. */ + | "author" + /** Whoever holds credentials and grants approvals for this org. */ + | "administrator"; + +export interface ExceptionDisposition { + kind: ExceptionKind; + /** Which desk this belongs on. */ + owner: ExceptionOwner; + /** One line naming the class of problem, safe to show in a queue row. */ + headline: string; + /** What the owner should actually do. */ + guidance: string; + /** + * Would retrying the same step plausibly change the outcome? + * + * False does not mean "forbidden" — a human may always retry, and the engine + * deliberately lets them (see journal.ts on clearing `inFlight`). It means the + * UI should not lead with retry, because the same failure will recur. + */ + retryUseful: boolean; + /** + * Could retrying repeat an effect that has already happened? + * + * The one field with teeth. When true, a retry needs explicit human + * acknowledgement rather than a one-click button, and that acknowledgement is + * recorded — the difference between an informed decision and an accident. + */ + retryMayDuplicate: boolean; +} + +/** Authoritative prefixes emitted by Ghost's own engine. */ +const PREFIX_KINDS: ReadonlyArray = [ + ["OUTCOME_UNKNOWN:", "OUTCOME_UNKNOWN"], + ["RESTORE_UNSAFE:", "RESTORE_UNSAFE"], + ["RUN_TIMEOUT:", "TRANSIENT"], +]; + +/** + * Best-effort patterns over third-party error text, most specific first. + * + * Ordering is load-bearing: a Playwright locator timeout contains the word + * "Timeout", so `TARGET_MISSING` must be tested before the generic timeout rule + * or every changed selector would be misfiled as a transient blip and retried + * forever. + */ +const TEXT_RULES: ReadonlyArray = [ + // Playwright's locator failures. "strict mode violation" is the opposite + // problem — too many matches — but lands on the same desk: the selector no + // longer identifies one thing, so the workflow needs re-authoring. + // + // The `getBy*` forms are not optional extras: apps/worker/src/browser/selector.ts + // resolves every *preferred* selector through `getByRole`/`getByTestId`/ + // `getByText`, and Playwright's call log then reads "waiting for + // getByRole(...)" — the word "locator" never appears. Matching only + // `locator|selector` therefore missed exactly the selectors Ghost prefers, + // dropping them through to the generic timeout rule below and routing a + // changed page to an operator as a transient blip to retry forever. The rest + // of the family is listed too, so a workflow that starts using them is not a + // silent regression. + [ + /waiting for (?:locator|selector|getBy(?:Role|TestId|Text|Label|Placeholder|AltText|Title))|strict mode violation|no element matches/i, + "TARGET_MISSING", + ], + [ + /element is not (?:visible|attached|enabled)|not attached to the DOM/i, + "TARGET_MISSING", + ], + + // Auth before generic HTTP, since a 403 body often also mentions the URL. + [/\b(?:401|403)\b|unauthorized|forbidden|access denied/i, "AUTH"], + [ + /session (?:has )?expired|please (?:sign|log) ?in|login required|invalid credentials/i, + "AUTH", + ], + + // Value rejected by the target, as opposed to the field not being found. + [/\b(?:invalid|malformed)\b.*\b(?:value|format|input|field)\b/i, "DATA"], + [ + /is required\b|must be (?:a|an|at least|no more)|does not match the expected/i, + "DATA", + ], + + // Network and browser-lifecycle noise. Genuinely transient. + [ + /net::ERR_|ECONNREFUSED|ECONNRESET|ETIMEDOUT|EAI_AGAIN|socket hang up/i, + "TRANSIENT", + ], + [ + /target (?:page|closed|crashed)|browser has been closed|page crashed/i, + "TRANSIENT", + ], + [ + /\b(?:429|502|503|504)\b|too many requests|service unavailable|bad gateway/i, + "TRANSIENT", + ], + + // Generic timeout last, after every specific timeout shape above. + [/timeout .* exceeded|timed out/i, "TRANSIENT"], +]; + +const DISPOSITIONS: Record< + ExceptionKind, + Omit +> = { + TRANSIENT: { + owner: "operator", + headline: "Temporary failure", + guidance: + "Infrastructure or timing, not the workflow. Retry the step; if it fails the same way twice, treat it as a target change instead.", + retryUseful: true, + }, + TARGET_MISSING: { + owner: "author", + headline: "Target element not found", + guidance: + "The page no longer matches what was recorded. Retrying will fail identically — re-record or edit the step's selector, then start a new run.", + retryUseful: false, + }, + AUTH: { + owner: "administrator", + headline: "Authentication or permission refused", + guidance: + "The target system rejected Ghost's session or permissions. Refresh the stored credential or widen its scope, then retry.", + retryUseful: false, + }, + VERIFICATION: { + owner: "operator", + headline: "Outcome did not match expectation", + guidance: + "The action ran but the result was not what the workflow asserted — so the effect has already happened, and retrying re-runs the action rather than just the check. Confirm in the target system: if the outcome is acceptable, skip the check; if not, reverse the run.", + retryUseful: false, + }, + OUTCOME_UNKNOWN: { + owner: "operator", + headline: "Effect unknown — may already have happened", + guidance: + "The step started and never reported back. Ghost will not guess. Confirm in the target system whether it took effect before deciding: retrying could repeat it.", + retryUseful: false, + }, + APPROVAL_EXPIRED: { + owner: "administrator", + headline: "Approval expired before use", + guidance: + "Someone approved this step but the run could not consume it in time. A fresh approval is required — the expired one cannot be reused.", + retryUseful: false, + }, + RESTORE_UNSAFE: { + owner: "operator", + headline: "Could not rebuild page state", + guidance: + "Resuming would have meant guessing at browser state, so the run stopped instead. Start a fresh run rather than retrying from here.", + retryUseful: false, + }, + DATA: { + owner: "operator", + headline: "Value rejected by the target", + guidance: + "The target system refused a value this run supplied. Retrying sends the same value — correct the input or the extracting step first.", + retryUseful: false, + }, + UNKNOWN: { + owner: "operator", + headline: "Unclassified failure", + guidance: + "Ghost could not categorize this failure. Read the error and the screenshot before acting; treat the step's effect as uncertain.", + retryUseful: false, + }, +}; + +export interface ClassifyExceptionInput { + /** The incident reason recorded on the run. */ + reason: string; + /** The step the run stopped on, when it is known. */ + step?: WorkflowStep; + /** + * The recorded outcome of that step, when known. + * + * `"UNKNOWN"` is authoritative and overrides the reason text: the engine only + * writes it after deciding an action's effect is genuinely indeterminate. + */ + recordedOutcome?: "UNKNOWN" | "FAILED" | null; +} + +export function classifyException( + input: ClassifyExceptionInput, +): ExceptionDisposition { + const kind = classifyKind(input); + const base = DISPOSITIONS[kind]; + + // Duplicate risk is the conjunction of two independent facts: the engine does + // not know whether the effect happened, AND the step is one whose effect + // reaches outside the browser. An indeterminate `verify` or `extract` is a + // read — repeating it costs nothing — so it must not carry the same warning + // as an indeterminate payment, or the warning stops meaning anything. + // + // Composing `replaySafety` here rather than re-listing step types keeps one + // definition of "mutating" across restoration and incident recovery. + const mutating = input.step ? replaySafety(input.step) === "mutating" : true; + const retryMayDuplicate = kind === "OUTCOME_UNKNOWN" && mutating; + + return { kind, ...base, retryMayDuplicate }; +} + +function classifyKind(input: ClassifyExceptionInput): ExceptionKind { + // Tier 1a: the recorded outcome. The engine already made this judgement with + // more context than a string match will ever have. + if (input.recordedOutcome === "UNKNOWN") return "OUTCOME_UNKNOWN"; + + const reason = input.reason ?? ""; + + // Tier 1b: Ghost's own structured prefixes. + for (const [prefix, kind] of PREFIX_KINDS) { + if (reason.startsWith(prefix)) return kind; + } + + // Phrases the engine writes without a prefix. Matched as whole phrases rather + // than keywords so an unrelated error mentioning "approval" is not swept up. + if (/^approval for step \d+ expired/i.test(reason)) return "APPROVAL_EXPIRED"; + if (/^verification failed at step \d+/i.test(reason)) return "VERIFICATION"; + + // "exhausted its retries" is the wrapper the step loop uses when it has no + // better message; on its own it says nothing about *why*, so it must not + // short-circuit the text rules below — the underlying driver error, when the + // loop managed to capture one, is the informative part. + const bare = /^step exhausted its retries$/i.test(reason); + if (bare) return "UNKNOWN"; + + // Tier 2: best-effort over third-party text. + for (const [pattern, kind] of TEXT_RULES) { + if (pattern.test(reason)) return kind; + } + + return "UNKNOWN"; +} + +/** + * Duplicate-effect risk for a stopped step, as a standalone decision. + * + * Exported because three read paths (the retry gate, the run timeline, and the + * exception queue) all need it, and each had been re-deriving it as + * `disposition.retryMayDuplicate || kind === "OUTCOME_UNKNOWN" || recorded === + * "UNKNOWN"`. That looked like belt-and-braces caution and was actually a bug: + * it forced the warning on for an indeterminate **read** — a `verify` or + * `extract` — which `classifyException` deliberately reports as safe, because + * repeating a read costs nothing. A confirmation prompt that fires on reads is + * one operators learn to click through, which is precisely how the prompt stops + * protecting the payment it exists for. + * + * The caution those callers wanted is real, though: a *stored* `incidentKind` + * must never be able to talk the risk down. So this takes the union of the + * live verdict and the stored label, and then — the part the callers dropped — + * still requires the step to actually reach outside the browser. + * + * Fails closed: an unknown step is assumed mutating. + */ +/** + * Kinds where a completed effect may already exist for the stopped step. + * + * `OUTCOME_UNKNOWN` is the obvious one: the step started and never reported. + * + * `VERIFICATION` is the subtle one, and it is *stronger* than uncertainty. A + * verification failure means the action ran and its assertion did not hold — + * there was something to check precisely because the click or fill went through. + * The worker is careful about this within one attempt (it re-runs the assertion + * only, never the action), but an incident retry resets the step to PENDING and + * re-executes the whole thing under the original approval. For a mutating step + * that is a second Pay, and nothing was warning about it. + */ +const EFFECT_MAY_HAVE_LANDED: ReadonlySet = new Set([ + "OUTCOME_UNKNOWN", + "VERIFICATION", +]); + +export function duplicateRiskFor(input: { + /** Live classifier verdict, when already computed. */ + disposition?: ExceptionDisposition; + /** The stored `Run.incidentKind`, which may be stale or absent. */ + storedKind?: ExceptionKind | string | null; + /** Recorded outcome of the stopped step. */ + recordedOutcome?: "UNKNOWN" | "FAILED" | null; + /** The stopped-on step, when known. */ + step?: WorkflowStep; +}): boolean { + const landed = + (input.disposition && EFFECT_MAY_HAVE_LANDED.has(input.disposition.kind)) || + (typeof input.storedKind === "string" && + EFFECT_MAY_HAVE_LANDED.has(input.storedKind as ExceptionKind)) || + input.recordedOutcome === "UNKNOWN"; + if (!landed) return false; + return input.step ? replaySafety(input.step) === "mutating" : true; +} + +/** + * The full disposition for an explicitly known kind. + * + * Read paths store the engine's verdict in `Run.incidentKind` and show that as + * the label, but they also need the owner, headline, guidance and retry + * recommendation that belong *to that kind*. Re-deriving those from a fresh + * `classifyException` over the same reason text produces a mismatch whenever the + * two disagree — and they disagree immediately for compensation incidents, whose + * kind is asserted directly at the call site while their reason text carries no + * classifier prefix. The queue then showed `OUTCOME_UNKNOWN` next to + * "Unclassified failure" guidance, which is worse than either alone. + * + * `retryMayDuplicate` still needs the step, so it is not decided here — callers + * pass the result to `duplicateRiskFor`. + */ +export function dispositionForKind(kind: ExceptionKind): ExceptionDisposition { + return { kind, ...DISPOSITIONS[kind], retryMayDuplicate: false }; +} + +/** Every kind, for exhaustive UI rendering and tests. */ +export const EXCEPTION_KINDS = Object.keys( + DISPOSITIONS, +) as ReadonlyArray; + +/** True when this kind is a defect in the workflow rather than in the world. */ +export function needsAuthoring(kind: ExceptionKind): boolean { + return DISPOSITIONS[kind].owner === "author"; +} + +/** + * Every kind that routes to a given desk. + * + * `owner` is a pure function of `kind`, so an owner filter can be pushed into + * SQL as `incidentKind IN (...)` instead of being applied to an + * already-capped page of rows — which silently returned "no author-owned + * exceptions" whenever the oldest N happened to be operator-owned. + */ +export function kindsForOwner(owner: ExceptionOwner): ExceptionKind[] { + return EXCEPTION_KINDS.filter((k) => DISPOSITIONS[k].owner === owner); +} diff --git a/docs/README.md b/docs/README.md index 36988ea..0717290 100644 --- a/docs/README.md +++ b/docs/README.md @@ -21,6 +21,7 @@ If a doc contradicts this index or `cloud/README.md`, treat the cloud docs as tr | [`product-direction.md`](product-direction.md) | Category, ICP, non-goals | | [`business-model.md`](business-model.md) | How we make money | | [`audiences.md`](audiences.md) | Who we sell to first | +| [`competitive-landscape.md`](competitive-landscape.md) | Commercial competitors — who's adjacent, what to adopt and refuse | | [`trust-pipeline.md`](trust-pipeline.md) | Approval / verify / audit principles | | [`why-deterministic-gates.md`](why-deterministic-gates.md) | Why the gate is a pure function, not a model call — and what the audit chain does not prove | | [`../CONTRIBUTING.md`](../CONTRIBUTING.md) | Setup, the two env traps, trust invariants | diff --git a/docs/competitive-landscape.md b/docs/competitive-landscape.md new file mode 100644 index 0000000..95e55fe --- /dev/null +++ b/docs/competitive-landscape.md @@ -0,0 +1,243 @@ +# Competitive landscape + +Ghost sells **governed execution**: a human-approved, verified, audited pipeline over +software that was never designed to be automated. That is a narrow category, and the +products it gets compared to mostly sit in *adjacent* categories — context layers, +assistants, RPA, and workflow orchestrators. + +`cloud/docs/PRIOR_ART.md` covers the open-source orchestrators (Temporal, Camunda, +Windmill, n8n, …) as engineering prior art. This document covers **commercial +competitors** — who a buyer might put next to Ghost on a shortlist, and what to take +or refuse from each. + +The map: + +| Category | Example | Relationship to Ghost | +|---|---|---| +| **Ambient context layer** | [Littlebird](#littlebird) | Adjacent above — analyzed below | +| Governed execution / trust runtime | *(no named direct competitor yet)* | Ghost's category | +| Agent frameworks | Claude, Cursor, Codex | **Clients**, not competitors — they propose, Ghost gates | +| No-code automation | Zapier, Make, n8n Cloud | Connectors without a trust pipeline — explicit non-goal | +| Enterprise RPA | UiPath, Automation Anywhere | Same problem, coordinate-first, no approval-gate story | +| Screen-recall / memory | Rewind, Windows Recall | Same layer as Littlebird, weaker on privacy stance | + +Only Littlebird is analyzed in depth so far. The rest are named to keep the map honest, +not because a survey of each has been done. + +--- + +## Littlebird + +> **Sourcing caveat.** `littlebird.ai` is blocked by this environment's network egress +> proxy, so nothing here is quoted from the vendor's own site. Everything below comes +> from press coverage, review sites, and search summaries (see [Sources](#sources)). +> Treat feature specifics as **secondary-source**, and re-verify against the product +> before any of this reaches a pitch, a comparison page, or a roadmap commitment. + +### What it actually is + +A macOS-native, always-on assistant that builds a **private memory of your work**. It +reads *structured text* from the active window across every app you have open — not +screenshots — transcribes your meetings, and stores that context as local text on the +Mac. You then ask it questions, and it answers with the context already in hand. +"Routines" run a saved prompt on a recurring schedule (daily briefing, weekly activity +summary). Connectors to email and calendar pull in further context and, per one review, +can "take actions on your behalf." + +The company: founded 2024 by Alap Shah, Naman Shah, and Alexander Green — the Shah +brothers previously founded Sentieo (sold to AlphaSense). **$11M seed**, announced March +2026. Product Hunt #1 product of the day. **SOC 2 certified**, GDPR/CCPA compliant, +AES-256 at rest, TLS 1.3 in transit, no training on user data. + +Pricing: **free** Basic tier; Plus at **$17/mo** annual or $20 monthly; higher tiers +reported up to ~$100/mo; 14-day trial on paid plans. + +### The finding: this is not a head-on competitor + +Littlebird is a **context layer**, not an execution platform. The clearest statement of +this comes from a competitor's own comparison page, which is where positioning is +usually least flattering and most accurate: Littlebird "is a smart observer on your +machine; unlike agents … that do the work itself, Littlebird gives you better answers +about your work." Another review puts it as "the context layer that feeds into your +other tools." + +Set against Ghost's pipeline, the overlap is thin and the gap is structural: + +| Capability | Littlebird | Ghost Cloud | +|---|---|---| +| Ambient cross-app context capture | **Yes** — all macOS apps, always on | No — and refused, see below | +| Meeting transcription | **Yes** | No | +| Persistent work memory + Q&A | **Yes**, local | No | +| Scheduled recurring prompts | **Yes** ("Routines") | **No trigger surface at all** | +| Records a demonstrated workflow | Observes, doesn't compile to steps | Yes — Phase 2 Chrome extension → typed steps | +| Executes multi-step work across apps | Limited, via email/calendar connectors | **Browser only** — typed steps; `apiCall`/`sendEmail` are declared but unimplemented ([`driver.ts`](../cloud/apps/worker/src/browser/driver.ts) `UNIMPLEMENTED_ACTION_TYPES`) | +| Deny-by-default gate on send/pay/delete | Not evidenced | **Yes** — deterministic `classifyStep` | +| Human approval before sensitive actions | Not evidenced | **Yes**, with expiry, single-use | +| Per-step outcome verification | No | **Yes** | +| Hash-chained audit log | No | **Yes**, two-level | +| Incidents / undo / durable resume | No | **Yes** — undo ships as approval-gated compensation, with documented limits (reversals run in a fresh unauthenticated context) | +| Multi-tenant org isolation, RBAC | Single-user Mac app | **Yes** | +| SOC 2 | **Yes** | **No** | +| Self-serve free tier | **Yes** | **No** — implementation-led | +| Platform | macOS only | Cloud (browser; API execution is future work) | + +Ghost wins every row that describes *doing the work under controls*. Littlebird wins +every row that describes *knowing about the work* — plus, notably, the two rows that are +about being a real company you can buy from today: SOC 2 and a free tier. + +One correction to Ghost's own column, because rule 10 cuts both ways. Ghost's execution +surface today is **the browser only**. `apiCall` and `sendEmail` exist in the step schema, +are classified by the approval gate, and are classified for replay safety — but their +executors are no-ops listed in `UNIMPLEMENTED_ACTION_TYPES`, and a guard keeps them out +of the step editor so nobody can author a step that silently does nothing. So Ghost +cannot currently send an email or call an API, and the strategy's "APIs first, then +browser" ordering describes the intended preference, not shipped behavior. That gap is +the single largest one in this comparison, and it is not in Littlebird's favour — it is +in the favour of any connector-backed automation platform. + +They are also not competing for the same dollar. $17/mo personal productivity versus +$500–2,000/mo team plus $2.5k–15k implementation are different budgets, different +buyers, and different sales motions. + +### So what is the actual threat? + +Three of them, and none is "Littlebird takes Ghost's customers." + +**1. Narrative capture.** Littlebird has $11M, a shipping consumer product, a #1 Product +Hunt launch, and press on the story "AI that already knows what you're working on." +Ghost has a stronger engine and no self-serve on-ramp. The risk is that Littlebird +defines the category in the buyer's head first, and Ghost gets evaluated as *"Littlebird +but harder to set up"* — judged on a rubric written by a product that doesn't do +governed execution and doesn't need to. + +**2. They own the layer above Ghost's hardest unsolved problem.** Ghost's Phase 2 answer +to "which workflow should we automate?" is: record one in Chrome, compile it to typed +steps. Littlebird already watches *every app on the machine, continuously*. It is much +better positioned to know which work is repetitive — because it sees all of it, and +Ghost sees only what someone chose to record in a browser tab. If Littlebird adds +execution, it descends into Ghost's layer carrying context Ghost cannot get. + +**3. Asymmetric compliance urgency.** Littlebird has SOC 2 as a $17/mo consumer app, +where it's a nice-to-have. Ghost sells approval-gated audit trails to regulated ops +teams — bookkeeping, healthcare admin, financial ops — where SOC 2 is a **procurement +blocker**, and Ghost doesn't have it. A product whose entire pitch is auditability being +outrun on third-party attestation by a personal-productivity app is a positioning +problem, not just a checklist gap. + +### The tripwires + +Littlebird becomes a direct competitor the moment it does any of these. Each is a step +down into Ghost's layer: + +1. **Write-execution beyond email/calendar** — especially browser automation. +2. **A team/org tier** with shared memory and admin controls. +3. **An approval concept** — any "confirm before it sends" gate. +4. **Beyond macOS**, i.e. server-side or cross-platform execution. +5. **Compiling observed activity into a reusable, editable routine** rather than a + recurring prompt. This is the one to watch hardest — it is Ghost's Phase 2, reached + from ambient capture instead of from an extension. + +If none of these ship, Littlebird stays a complement — plausibly even a context source +Ghost consumes. + +### An uncomfortable note for the record + +Ghost's legacy desktop app **already built Littlebird's wedge and deprioritized it**. +`src-tauri/src/core/atlas.rs` + `storage/atlas.rs` are a local, offline semantic-memory +graph over the user's work. `commands/experimental.rs` includes observer mode. +`core/ocr.rs` does on-device OCR with no network. Local-first, no-cloud, never-delete. + +Littlebird raised $11M taking approximately that wedge to market as a $17/mo macOS app +with meeting notes and a good privacy story. + +The correct reading is **not** "go back to the desktop app." The wedge was abandoned for +sound reasons recorded in `CLAUDE.md`: the local-first delivery model doesn't carry +forward, and ambient observation is against Ghost's own trust boundary. The correct +reading is narrower and more useful: **the wedge is fundable, and the market rewards +products that visibly know the user's work.** Ghost's version of that has to be built +out of run history, not surveillance. + +--- + +## What Ghost should adopt + +Framed the way `PRIOR_ART.md` frames its steals — the refusals matter as much. + +**Adopt: a trigger surface, and treat it as competitive, not backlog.** +Littlebird ships scheduled recurring execution at $17/mo. `PRIOR_ART.md` lists "Triggers +and schedules" as deferred with the note *"no trigger surface exists at all."* That +reads differently now. It also has a genuine product blocker recorded alongside it — +scheduling *unattended* runs of an approval-gated workflow needs an answer for who gets +notified — which leads directly to the next item. + +**Adopt: a notification surface. This is the highest-leverage unblock in the repo.** +It currently blocks three separately-deferred things: multiple approvers, Slack/email +approval routing (both from Windmill), and scheduled runs. One piece of infrastructure, +three features, and one of them is now a competitive gap. Build this first. + +**Adopt: workflow intelligence from run history, not from observation.** +Littlebird's real insight is that a product feels indispensable when it already knows +your work. Ghost can have a scoped version of this without watching anything: it already +stores every run, every step, every approval, every verification. It should be able to +tell a customer *"this workflow ran 43 times last month, 91% of its steps never needed +approval, and step 7 has failed 4 times"* — which is a better automation-ROI argument +than ambient capture can make, because it's measured on work Ghost actually executed. +No new capture surface, no new trust boundary, uses data that exists. + +**Adopt: state the screenshot stance as loudly as they state theirs.** +Littlebird made "we don't take screenshots" a headline privacy claim. Ghost **does** +capture per-step screenshots — and that's defensible: they are evidence for an auditor, +scoped to a run a human triggered, on a workflow a human approved, not ambient recording +of a person's day. That distinction is real and currently under-stated. Rule 10 forbids +overclaiming; it does not forbid claiming what is true. Write it down in +`trust-pipeline.md` with the same clarity Littlebird writes theirs. + +**Adopt: SOC 2, with urgency.** See the asymmetry above. For Ghost's buyer this is a +blocker, not a badge. + +**Adopt: one self-serve path that completes.** Not a free tier, and not a reversal of +the implementation-led model in `business-model.md` — that ordering is deliberate. But +"$2,500 minimum before you see it work" loses every evaluation where the buyer wants to +try before they talk. One templated workflow a prospect can run end-to-end — gate, +approve, verify, audit log, all of it — is the demo the engine has earned and doesn't +currently have a front door for. + +## What Ghost should refuse + +**Always-on ambient capture across all applications.** This violates a stated trust +boundary in `CLAUDE.md` — *"No monitoring the customer hasn't asked for"* — and it is a +different product with a different buyer. Ghost's capture is scoped, per-workflow, and +initiated by a human who is demonstrating a task on purpose. Do not blur that to match a +competitor's feature list. + +**A local-first / on-device trust story.** Already decided and documented. Ghost's trust +story is least privilege + approval + verification + tamper-evident audit. Littlebird's +strong local-privacy story is an argument for stating Ghost's story better, not for +changing it. + +**Meeting transcription, personal memory, and "ask me about your day."** Adjacent +product, adjacent buyer, and `product-direction.md` already lists "not a chatbot or 'AI +coworker'" as a non-goal. + +**macOS-native as the primary surface.** Ghost executes in the cloud, against browsers +today and APIs once `apiCall` is implemented. Desktop automation stays where the +strategy puts it: after browser, before vision. + +--- + +## Sources + +- [Littlebird raises $11 Million — PR Newswire](https://www.prnewswire.com/news-releases/littlebird-raises-11-million-to-launch-the-only-ai-that-already-knows-what-youre-working-on-302721664.html) +- [Littlebird raises $11M for its AI-assisted 'recall' tool — TechCrunch](https://techcrunch.com/2026/03/23/littlebird-raises-11m-to-capture-context-from-your-computer-so-you-can-query-your-data/) *(egress-blocked; cited from search summary)* +- [Littlebird — Product Hunt](https://www.producthunt.com/products/littlebird) +- [Littlebird pricing](https://littlebird.ai/pricing) *(egress-blocked)* +- [Littlebird Review 2026 — Efficient App](https://efficient.app/apps/littlebird) *(egress-blocked; cited from search summary)* +- [Littlebird Review — Agent Finder](https://agent-finder.co/reviews/littlebird) +- [6 Best Littlebird Alternatives — Carly](https://www.usecarly.com/blog/littlebird-alternatives/) — source of the "smart observer … gives you better answers about your work" characterization +- [Littlebird AI Review — Toolworthy](https://www.toolworthy.ai/tool/littlebird-ai) +- [Littlebird AI — App Store](https://apps.apple.com/us/app/littlebird-ai/id6737920045) + +Internal: [`product-direction.md`](product-direction.md) · +[`business-model.md`](business-model.md) · [`trust-pipeline.md`](trust-pipeline.md) · +[`../cloud/docs/PRIOR_ART.md`](../cloud/docs/PRIOR_ART.md) · +[`../cloud/docs/CURSOR_HANDOFF.md`](../cloud/docs/CURSOR_HANDOFF.md) diff --git a/docs/trust-pipeline.md b/docs/trust-pipeline.md index 5766eab..847ff42 100644 --- a/docs/trust-pipeline.md +++ b/docs/trust-pipeline.md @@ -48,6 +48,51 @@ Compensation has known gaps, listed in `cloud/docs/CURSOR_HANDOFF.md` under "Known gaps in compensation". The one that limits what it is useful for today: reversals run in a fresh, unauthenticated browser context. +## Exception routing + +A run that stops does not just sit in `INCIDENT` waiting to be found by run id. +The stop is **classified when it is raised** by a third deterministic classifier, +`classifyException` — sibling to `classifyStep` (must a human approve this?) and +`replaySafety` (is it safe to re-apply silently?). It answers a different +question: *this stopped — whose desk, and is retrying safe?* + +Nine kinds each route to one **owner**, and the owner is the point: a changed +selector (`TARGET_MISSING`) belongs to whoever maintains the workflow, an expired +credential (`AUTH`) or a lapsed gate (`APPROVAL_EXPIRED`) to an administrator, a +rejected value (`DATA`) or a wrong outcome (`VERIFICATION`) to the operator +running the work. "A run stopped" is not actionable; "the portal changed and step +4 needs re-recording" is. + +Like its siblings it is a rule table, not a model call, and it fails closed: an +unrecognized reason stays `UNKNOWN` rather than being forced into the nearest +bucket, because a confidently wrong label defeats the purpose of routing. + +Two properties carry the trust weight: + +- **Classification never changes how a run *stops*.** The worker's transition + into `INCIDENT` is identical whatever the verdict — the kind decides what a + human is shown and whose desk the work lands on, not whether the run halts. + It does, deliberately, shape *recovery*: the disposition feeds + `duplicateRiskFor`, which is what makes the route below refuse an + unacknowledged retry. Naming that plainly matters, because a reader who + believed classification were inert would not think to check it when a retry + is refused. +- **`OUTCOME_UNKNOWN` retries require acknowledgement.** A step that started and + never reported back may already have taken effect. The engine still lets a + human retry it — only a person can check the target system — but the route + refuses an unacknowledged retry and records the acknowledgement in the run + journal and the audit log. Duplicate risk additionally requires the step to + reach *outside the browser*: an indeterminate `verify` is a read, and a prompt + that fires on reads is one operators learn to click through. + +Resolution (`retry` / `skip` / `assign`) stays on the run's own incident route +under its own authorization; the queue at `/exceptions` and `GET /api/exceptions` +is read-only. Assignment is open to any member — deciding who looks at a problem +is not authorizing the action that caused it — and is tenant-scoped to members of +the org that owns the run. Routing fields are cleared whenever a run leaves +`INCIDENT`, and a failed *reversal* is re-routed from scratch rather than +inheriting the forward incident's kind, owner, or waiting time. + ## Execution limits A workflow may cap how many of its runs are in flight at once