Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion apps/web/app/api/v1/approvals/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ export async function GET(request: Request) {
const traceId = requestTraceId(request);
try {
return Response.json({
data: await new ApprovalDomainService().list(await apiSubject(request)),
data: await new ApprovalDomainService().list(
await apiSubject(request),
traceId,
),
traceId,
});
} catch (error) {
Expand Down
41 changes: 27 additions & 14 deletions apps/web/features/approvals/governance-inbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,14 @@ function ApprovalDetail({
}) {
const severity = riskSeverity(approval.riskSummary);
const highImpact = severity === "critical";
const pending = approval.status === "pending";
// A row can still read as pending until the next inbox load expires it, so
// trust the deadline rather than the stored status for what is offerable.
const overdue = new Date(approval.expiresAt) <= new Date();
const pending = approval.status === "pending" && !overdue;
// Includes rows already stored as `expired` by the inbox's lazy sweep, not
// just ones that are still nominally pending.
const closable =
approval.status === "expired" || (approval.status === "pending" && overdue);

return (
<div>
Expand Down Expand Up @@ -277,9 +284,15 @@ function ApprovalDetail({
</div>
</dl>

{pending ? (
{pending || closable ? (
<div className="space-y-3 rounded-md border border-border bg-[var(--color-paper)] p-3">
<label className="block text-xs font-semibold" htmlFor="decision-reason">
{closable ? (
<p className="rounded-md border border-[var(--color-warning)]/40 bg-[var(--color-warning-soft)] p-2 text-sm text-[var(--color-warning)]">
This request passed its deadline, so it can no longer be
approved. Reject it to close it out with a recorded reason.
</p>
) : null}
<label className="block text-sm font-semibold" htmlFor="decision-reason">
Decision reason (required)
</label>
<textarea
Expand Down Expand Up @@ -308,28 +321,28 @@ function ApprovalDetail({
</label>
) : null}
<div className="flex flex-wrap gap-2">
{closable ? null : (
<Button type="button" disabled={busy} onClick={onApprove}>
<Check className="size-4" />
Approve
</Button>
)}
<Button
type="button"
disabled={busy}
onClick={onApprove}
>
<Check className="size-4" />
Approve
</Button>
<Button
type="button"
variant="outline"
variant={closable ? "default" : "outline"}
disabled={busy}
onClick={onReject}
>
<X className="size-4" />
Reject
{closable ? "Reject and close" : "Reject"}
</Button>
</div>
</div>
) : (
<p className="text-sm text-muted-foreground">
This approval is no longer pending.
{approval.status === "expired"
? "This approval expired without a decision. Nothing was executed."
: "This approval is no longer pending."}
{approval.reason ? ` Reason: ${approval.reason}` : ""}
</p>
)}
Expand Down
28 changes: 27 additions & 1 deletion apps/web/features/operations/operations-view.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe("Operations board", () => {
expect(view).toContain("dispatchBlockedReason");
// Every refusal path returns operator-readable text.
expect(view).toContain("Assign this task to an agent to dispatch it.");
expect(view).toContain("already has an active agent run");
expect(view).toContain("A run is already in flight.");
expect(view).toContain("assigneeReadinessReason");
});

Expand Down Expand Up @@ -103,3 +103,29 @@ describe("Task composer", () => {
expect(composer).not.toMatch(/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-/);
});
});

describe("Stuck and failed agent work", () => {
it("offers cancel while a run is in flight", async () => {
const view = await source("./operations-view.tsx");
expect(view).toContain("useCancelTaskRun");
expect(view).toContain("Cancel run");
// Every status the server treats as in-flight must be escapable, not just
// queued/running — awaiting_approval wedges a task just as hard.
expect(view).toContain('"awaiting_approval"');
expect(view).toContain('"waiting_sources"');
});

it("labels a re-dispatch as a retry and says why", async () => {
const view = await source("./operations-view.tsx");
expect(view).toContain("function isRetry");
expect(view).toContain("Retry dispatch");
expect(view).toContain("Previous run");
});

it("points a blocked dispatch at the way out", async () => {
const view = await source("./operations-view.tsx");
expect(view).toContain(
"A run is already in flight. Cancel it before dispatching again.",
);
});
});
82 changes: 68 additions & 14 deletions apps/web/features/operations/operations-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
type ComposerSeed,
} from "@/features/operations/task-composer";
import {
useCancelTaskRun,
useDelegateTask,
useTasks,
useUpdateTask,
Expand Down Expand Up @@ -162,19 +163,38 @@ function taskToBoardItem(task: RawTask): BoardItem {
};
}

/** A run that is already queued or running must not be dispatched again. */
/** Statuses the server treats as an in-flight run; cancel is the only exit. */
const ACTIVE_RUN_STATUSES = [
"queued",
"running",
"awaiting_approval",
"waiting_sources",
];

function hasActiveRun(item: BoardItem): boolean {
return ACTIVE_RUN_STATUSES.includes(item.agentRunStatus ?? "");
}

/** A run already in flight must not be dispatched again. */
function dispatchBlockedReason(item: BoardItem): string | null {
if (!item.assigneeIsAgent)
return "Assign this task to an agent to dispatch it.";
if (item.agentRunStatus === "queued" || item.agentRunStatus === "running")
return "This task already has an active agent run.";
if (hasActiveRun(item))
return "A run is already in flight. Cancel it before dispatching again.";
if (item.assigneeReadiness !== "ready")
return (
item.assigneeReadinessReason ?? "Assigned agent is not ready for work."
);
return null;
}

/** A settled run can be handed back to the agent; label it as a retry. */
function isRetry(item: BoardItem): boolean {
return (
item.agentRunStatus === "failed" || item.agentRunStatus === "cancelled"
);
}

export function OperationsView() {
const tasks = useTasks();
const updateTask = useUpdateTask();
Expand Down Expand Up @@ -473,6 +493,7 @@ export function OperationsView() {

function DetailDrawer({ item }: { item: BoardItem | null }) {
const delegateTask = useDelegateTask();
const cancelRun = useCancelTaskRun();
const [error, setError] = useState<string | null>(null);
const [notice, setNotice] = useState<string | null>(null);

Expand Down Expand Up @@ -501,6 +522,20 @@ function DetailDrawer({ item }: { item: BoardItem | null }) {
}
}

async function cancel() {
if (!item) return;
setError(null);
setNotice(null);
try {
await cancelRun.mutateAsync(item.id);
setNotice("Run cancelled. You can dispatch it again.");
} catch (caught) {
setError(
caught instanceof Error ? caught.message : "Could not cancel the run.",
);
}
}

return (
<aside className="rounded-md border border-border bg-card p-4">
<h2 className="text-sm font-semibold">{item.title}</h2>
Expand All @@ -518,19 +553,38 @@ function DetailDrawer({ item }: { item: BoardItem | null }) {
)}
{item.ownerName ?? "Unassigned"}
</p>
<Button
type="button"
size="sm"
disabled={Boolean(blocked) || delegateTask.isPending}
title={blocked ?? undefined}
onClick={() => void dispatch()}
>
{delegateTask.isPending ? "Dispatching…" : "Dispatch to agent"}
</Button>
<div className="flex flex-wrap gap-2">
{hasActiveRun(item) ? (
<Button
type="button"
size="sm"
variant="outline"
disabled={cancelRun.isPending}
onClick={() => void cancel()}
>
{cancelRun.isPending ? "Cancelling…" : "Cancel run"}
</Button>
) : null}
<Button
type="button"
size="sm"
disabled={Boolean(blocked) || delegateTask.isPending}
title={blocked ?? undefined}
onClick={() => void dispatch()}
>
{delegateTask.isPending
? "Dispatching…"
: isRetry(item)
? "Retry dispatch"
: "Dispatch to agent"}
</Button>
</div>
</div>
<p className="mt-1.5 text-xs text-muted-foreground">
<p className="mt-1.5 text-sm text-muted-foreground">
{blocked ??
"Runs under the agent's governed capability envelope. External writes stay approval-gated."}
(isRetry(item)
? `Previous run ${item.agentRunStatus}. Dispatching again starts a fresh run under the agent's governed capability envelope.`
: "Runs under the agent's governed capability envelope. External writes stay approval-gated.")}
</p>
{error ? (
<p role="alert" className="mt-1.5 text-xs text-[var(--color-error)]">
Expand Down
76 changes: 76 additions & 0 deletions apps/web/lib/approval-expiry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { readFile } from "node:fs/promises";
import { describe, expect, it } from "vitest";

async function source(name: string) {
return readFile(new URL(name, import.meta.url), "utf8");
}

/**
* An expired approval used to be a permanent dead row: `decide` refused both
* verdicts, and nothing ever moved it out of `pending`.
*/
describe("approval expiry", () => {
it("refuses approval past the deadline but allows rejection", async () => {
const domain = await source("./integration-action-domain.ts");
expect(domain).toContain(
'approval.expiresAt <= new Date() && decision.status !== "rejected"',
);
expect(domain).toContain("Reject it to close it out.");
});

it("transitions overdue rows to expired with an audit event", async () => {
const domain = await source("./integration-action-domain.ts");
expect(domain).toContain("async expireOverdue(");
expect(domain).toContain('set({ status: "expired"');
expect(domain).toContain("lte(schema.approvals.expiresAt, new Date())");
expect(domain).toContain('action: "workflow.approval.expired"');
// Expiry must run before the inbox is read, or the UI keeps offering
// Approve on a request that can no longer be approved.
expect(domain).toContain("await this.expireOverdue(subject.organisationId");
});

it("never counts an overdue approval as actionable on Command", async () => {
const summary = await source("./command-summary-domain.ts");
expect(summary).toContain("gt(schema.approvals.expiresAt, new Date())");
});
});

describe("approval inbox controls", () => {
it("drops Approve and offers a closing Reject once overdue", async () => {
const view = await source("../features/approvals/governance-inbox.tsx");
expect(view).toContain("const overdue = new Date(approval.expiresAt)");
expect(view).toContain('(approval.status === "pending" && overdue)');
expect(view).toContain("{closable ? null : (");
expect(view).toContain('{closable ? "Reject and close" : "Reject"}');
expect(view).toContain("can no longer be");
});

it("explains an expired outcome rather than saying only 'not pending'", async () => {
const view = await source("../features/approvals/governance-inbox.tsx");
expect(view).toContain('approval.status === "expired"');
expect(view).toContain("Nothing was executed.");
});
});

/**
* The lazy sweep and the decision path have to agree. If listing the inbox
* moves a row to `expired` and `decide` then treats `expired` as terminal,
* opening Approvals is what destroys the only way to close the row.
*/
describe("expired rows stay closable after the lazy sweep", () => {
it("does not short-circuit a rejection on a stored expired row", async () => {
const domain = await source("./integration-action-domain.ts");
expect(domain).toContain("const closingExpired =");
expect(domain).toContain(
'approval.status === "expired" && decision.status === "rejected"',
);
expect(domain).toContain(
'if (approval.status !== "pending" && !closingExpired)',
);
});

it("still offers the closing control once the row reads expired", async () => {
const view = await source("../features/approvals/governance-inbox.tsx");
expect(view).toContain('approval.status === "expired" ||');
});
});
5 changes: 4 additions & 1 deletion apps/web/lib/command-summary-domain.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { and, desc, eq, inArray, isNull } from "drizzle-orm";
import { and, desc, eq, gt, inArray, isNull } from "drizzle-orm";
import {
hasCapability,
type AuthorisationSubject,
Expand Down Expand Up @@ -74,6 +74,9 @@ export async function getCommandSummary(
and(
eq(schema.approvals.organisationId, subject.organisationId),
eq(schema.approvals.status, "pending"),
// An overdue row is still stored as pending until something reads
// the inbox and expires it. Never count it as actionable here.
gt(schema.approvals.expiresAt, new Date()),
),
)
.orderBy(desc(schema.approvals.requestedAt))
Expand Down
Loading
Loading