Skip to content

fix(console): a paused card names the approvals blocking it, and Resume stops re-running them (#883) #945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
senamakel merged 1 commit into from
Aug 17, 2026
Merged

fix(console): a paused card names the approvals blocking it, and Resume stops re-running them (#883) #945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions frontend/src/components/app-shell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1250,13 +1250,24 @@ export function AppShell({
// counter the chat's in-flight strip reads, so a card opened from
// chat lands on the board without a reload.
taskEventTick={taskEventTick}
// Issue #883: a paused card is blocked until every approval its
// turn parked is decided, and the board's own read carries none
// of them. This is the feed the sidebar badge already polls, so
// the card says what it is waiting on without a second request.
approvals={feed.approvals}
now={feed.now}
// Issue #246: the card → chat half of the round trip. A card
// opened from a conversation remembers which one, so its detail
// screen can put the operator back in that thread.
onOpenThread={(threadId) => {
setActiveThreadId(threadId);
setView("conversation");
}}
// Issue #883: "Review" on a blocked card opens the queue narrowed
// to that card. Through `navigate` rather than `setView` so the
// filter lands in the hash and survives a refresh and the Back
// button, like every other sub-page.
onReviewApprovals={(taskId) => navigate("approvals", encodeURIComponent(taskId))}
/>
)}
{view === "ledgers" && (
Expand Down Expand Up @@ -1317,6 +1328,13 @@ export function AppShell({
client={client}
company={company}
feed={feed}
// Issue #883: `#/approvals/<taskId>` narrows the queue to one
// card, so "Review" on a blocked card lands on its approvals
// rather than on a page the operator has to search. Same
// unvalidated second segment every other sub-page gets — only
// this view knows whether the id matches anything parked, so it
// does that check itself and says so when it does not.
sub={sub}
onResolved={noteSystem}
onGoToConversation={() => setView("chat")}
/>
Expand Down
71 changes: 71 additions & 0 deletions frontend/src/lib/task-approvals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { TaskApproval } from "@/api/tasks";
import type { ApprovalSummary } from "@/api/types";

/**
* What the task card says about approvals (issue #468).
Expand Down Expand Up @@ -52,3 +53,73 @@ export function pendingApprovalWait(
if (count === 0) return null;
return { count, since, waited: Math.max(0, now - since) };
}

/**
* The parked approvals the board's own poll says belong to one card (#883).
*
* The board reads `…/tasks`, whose card projection carries no approvals, and
* opening every card to find out would be N reads per 4s poll — the cost
* `TaskCard::output` is documented as existing to avoid. So the join happens
* here instead, against the approvals feed the shell **already** polls for the
* sidebar badge. No new request, no new wire field, and the board and the
* Approvals page are reading one list rather than two that can disagree.
*
* `GET …/approvals` returns the parked queue and nothing else, so every entry is
* pending by construction — unlike {@link pendingApprovalWait}, which filters a
* task-detail projection that includes resolutions.
*
* **Only `{link: "task"}` matches, and that is the whole ownership rule here.**
* `CycleHostImpl::park` stamps the link on every park path, so a card-dispatched
* approval always carries it. The host's own `approval_owner` has a first rule
* this cannot reach — the attempt-level `run_id`, which separates two *runs of
* the same card* — but that distinction does not change the answer to "is this
* card blocked", which is the only question asked here. `{link: "unlinked"}`
* (a workflow delivery, an operator-chat turn, a scheduler tick) and an absent
* link (a park predating #333) both belong to no card and are skipped rather
* than guessed at: the host keeps a run-window heuristic for the ambiguous case
* and the board has no window to apply it against.
*/
export function approvalsForTask(
approvals: readonly ApprovalSummary[],
taskId: string,
): ApprovalSummary[] {
return approvals.filter((a) => a.task?.link === "task" && a.task.id === taskId);
}

/** Why a card is stopped — read by its board card and by its Resume button (#883). */
export interface TaskApprovalBlock {
/** The still-parked approvals for this card, oldest park first. */
approvals: ApprovalSummary[];
/** How many there are — the number the card reports. */
count: number;
/** Epoch-millis the *oldest* of them parked — what the card measures from. */
since: number;
}

/**
* What is blocking one card right now — `null` when nothing is (#883).
*
* This is the derivation behind both halves of the fix, and it is one function
* rather than two so they cannot disagree: the line that says *why* the card is
* stopped and the `disabled` that stops Resume re-running it read the same
* result. A card that said "blocked on 4 approvals" beside a Resume button that
* dispatched anyway would be a worse surface than the silent one it replaces.
*
* Ordered oldest-first, and `since` is the oldest park — same reasoning as
* {@link pendingApprovalWait}: it is how long the card has really been stopped,
* and taking the newest would reset a clock that should be climbing every time
* a second effect parks behind the first.
*
* No `waited` counterpart, unlike {@link pendingApprovalWait}: this block's only
* reader renders the span through `timeAgo`, which takes the instant and clamps
* its own negative — so a second copy of that arithmetic here would be a field
* with no consumer to keep it honest.
*/
export function taskApprovalBlock(
approvals: readonly ApprovalSummary[],
taskId: string,
): TaskApprovalBlock | null {
const mine = approvalsForTask(approvals, taskId).sort((a, b) => a.at_millis - b.at_millis);
if (mine.length === 0) return null;
return { approvals: mine, count: mine.length, since: mine[0].at_millis };
}
103 changes: 97 additions & 6 deletions frontend/src/views/ApprovalsView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { Card, CardContent } from "@/components/ui/card";
import type { CompanyFeed } from "@/hooks/use-company";
import { approvedByRuntimeLine, approvedLine } from "@/lib/approval-wording";
import { approvalSummary, grantHeadline, timeAgo, toolAction } from "@/lib/language";
import { approvalsForTask } from "@/lib/task-approvals";
import { startVisiblePolling } from "@/lib/visible-poll";
import { isRecord, parseNodeMessages } from "@/views/workflows/run-output";

Expand Down Expand Up @@ -76,12 +77,30 @@ interface Props {
client: OpenCompanyClient;
company: string | null;
feed: CompanyFeed;
/**
* `#/approvals/<taskId>` — narrow the queue to one board card (issue #883).
*
* Arrives unvalidated, as every hash sub-page does. An id matching nothing
* parked is a legitimate state rather than an error: the operator followed a
* blocked card's Review link and the last of its approvals was decided on the
* way, which is the flow working. It renders as "this card is clear" with a
* way back to the whole queue, never as the generic "nothing needs you" —
* those are different facts and only one of them is about the whole company.
*/
sub?: string | null;
onResolved: (systemLine: string) => void;
onGoToConversation: () => void;
}

/** The approvals inbox: the few things the company parked for the operator. */
export function ApprovalsView({ client, company, feed, onResolved, onGoToConversation }: Props) {
export function ApprovalsView({
client,
company,
feed,
sub,
onResolved,
onGoToConversation,
}: Props) {
// Issue #373: in-flight state is per approval, not a single module-wide slot.
//
// Approving is not a quick write — the host mints a grant and re-dispatches
Expand All @@ -96,6 +115,32 @@ export function ApprovalsView({ client, company, feed, onResolved, onGoToConvers
// is doing it" vs "recorded"), and the card says which one it is waiting on.
const [inFlight, setInFlight] = useState<ReadonlyMap<string, Verdict>>(() => new Map());
const { approvals, now } = feed;
/**
* The card the queue is narrowed to, or `null` for the whole queue (#883).
*
* Decoded because the shell percent-encodes the id into the hash; a malformed
* escape throws `URIError`, and a broken link must fall back to the full queue
* rather than blank the page.
*/
const focusTaskId = useMemo(() => {
if (!sub) return null;
try {
return decodeURIComponent(sub);
} catch {
return null;
}
}, [sub]);
/**
* The rows on screen. Every other derivation below — the batch totals, the
* asker names, the decide path — deliberately stays on the **full** queue:
* a batch's "1 of 3 from this turn" counts what the turn parked, and filtering
* the count with the view would turn a batch of three into a batch of one and
* tell the operator the opposite of the truth.
*/
const visible = useMemo(
() => (focusTaskId === null ? approvals : approvalsForTask(approvals, focusTaskId)),
[approvals, focusTaskId],
);
const askerNames = useAskerNames(client, company, approvals);
const { grants, granterNames, refreshGrants } = useStandingGrants(client, company);
/**
Expand Down Expand Up @@ -217,19 +262,39 @@ export function ApprovalsView({ client, company, feed, onResolved, onGoToConvers
return (
<div className="flex-1 overflow-y-auto">
<div className="mx-auto w-full max-w-3xl px-4 py-6">
{approvals.length === 0 ? (
<EmptyApprovals onGoToConversation={onGoToConversation} />
{/* Issue #883: the filter says so, and offers the way out of itself.
A narrowed queue that looked identical to the whole one would make a
decided-elsewhere approval look like it had vanished. */}
{focusTaskId !== null && (
<div className="mb-4 flex items-center justify-between gap-3 rounded-lg border bg-muted/40 px-3 py-2 text-xs">
<span className="min-w-0 text-muted-foreground">
Showing only what one board card is waiting on.
</span>
<a
href="#/approvals"
className="shrink-0 font-medium underline-offset-2 hover:underline"
>
Show all
</a>
</div>
)}
{visible.length === 0 ? (
focusTaskId !== null ? (
<ClearedForTask />
) : (
<EmptyApprovals onGoToConversation={onGoToConversation} />
)
) : (
<>
<div className="mb-4 flex items-baseline justify-between">
<h2 className="text-sm font-medium text-muted-foreground">
{approvals.length === 1
{visible.length === 1
? "1 thing needs your approval"
: `${approvals.length} things need your approval`}
: `${visible.length} things need your approval`}
</h2>
</div>
<div className="flex flex-col gap-3">
{approvals.map((a) => (
{visible.map((a) => (
<ApprovalCard
key={a.id}
approval={a}
Expand Down Expand Up @@ -655,6 +720,32 @@ function WorkflowContentReview({ approval }: { approval: ApprovalSummary }) {
);
}

/**
* A filtered queue with nothing left in it (issue #883).
*
* Deliberately not {@link EmptyApprovals}. That one says "nothing is waiting on
* you", which is a claim about the *whole company* — and here it would be said
* while other cards' approvals sit one click away, unread. The two states also
* mean opposite things to the operator who arrived from a blocked card: this
* one says the card is free to resume, which is the answer they came for.
*/
function ClearedForTask() {
return (
<div className="mt-16 flex flex-col items-center gap-3 text-center">
<div className="flex size-12 items-center justify-center rounded-2xl bg-status-done-soft text-status-done-text">
<ShieldCheck className="size-6" />
</div>
<div className="space-y-1">
<p className="font-medium">This card is clear</p>
<p className="max-w-sm text-sm text-muted-foreground">
Nothing it parked is still waiting on you. Other cards may still have
approvals of their own — use <span className="font-medium">Show all</span> to see them.
</p>
</div>
</div>
);
}

function EmptyApprovals({ onGoToConversation }: { onGoToConversation: () => void }) {
return (
<div className="mt-16 flex flex-col items-center gap-3 text-center">
Expand Down
70 changes: 63 additions & 7 deletions frontend/src/views/TaskDetailView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import {
AWAITING_APPROVAL_LABEL,
ApiError,
STEP_FAILURE_LABEL,
type ApprovalSummary,
} from "@/api/types";
import type { OpenCompanyClient } from "@/api/client";
import { hasFocus, type TaskFocus } from "@/lib/task-output";
Expand Down Expand Up @@ -110,7 +111,7 @@ import {
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
import { Textarea } from "@/components/ui/textarea";
import { cn } from "@/lib/utils";
import { effectDone } from "@/lib/language";
import { approvalAction, effectDone } from "@/lib/language";

import { labelFor, PRIORITY_STYLES, type TaskColumn } from "@/lib/board-columns";
import { useBoardColumns } from "@/hooks/use-board-columns";
Expand Down Expand Up @@ -204,6 +205,13 @@ export function waitingBandHeight(millis: number): number {
return Math.round(Math.min(112, Math.max(12, raw)));
}

/**
* A stable empty default for the `parked` prop (issue #883). A `[]` literal in
* the parameter list is a new array every render, which would re-run the memo
* that reads it on a screen with a 1s clock.
*/
const EMPTY_PARKED: readonly ApprovalSummary[] = [];

/** `1h 04m 09s` / `4m 09s` / `9s`. */
function formatDuration(millis: number): string {
const s = Math.floor(millis / 1000);
Expand Down Expand Up @@ -265,6 +273,7 @@ export function TaskDetailView({
company,
taskId,
focus,
parked = EMPTY_PARKED,
onBack,
onNavigate,
onOpenThread,
Expand All @@ -274,6 +283,12 @@ export function TaskDetailView({
client: OpenCompanyClient;
company: string | null;
taskId: string;
/**
* The company's parked approvals, for naming what this card is waiting on
* (issue #883). Optional and defaulting to empty, which renders the pre-#883
* row — this screen's own read is what decides *whether* it is waiting.
*/
parked?: readonly ApprovalSummary[];
/**
* What the address asked this screen to open (issue #339): a pinned artifact
* or an attempt's trace. Empty — the ordinary "open the card" navigation —
Expand Down Expand Up @@ -491,7 +506,12 @@ export function TaskDetailView({
columns={columns}
/>

<AwaitingApprovalRow approvals={detail.approvals} now={now} />
<AwaitingApprovalRow
approvals={detail.approvals}
parked={parked}
taskId={detail.task.id}
now={now}
/>

{/* Issue #580: the built workflow awaiting approval, shown only while
the card sits In Review with a proposal. Apply creates the
Expand Down Expand Up @@ -1836,22 +1856,58 @@ function RunDrawer({
* Renders nothing when nothing is pending — an always-present "no approvals"
* line is the clutter the tab was removed for.
*/
function AwaitingApprovalRow({ approvals, now }: { approvals: TaskApproval[]; now: number }) {
function AwaitingApprovalRow({
approvals,
parked,
taskId,
now,
}: {
approvals: TaskApproval[];
/**
* The company's parked queue, for naming (issue #883). Deliberately **not**
* the source of truth for whether the card is waiting: `approvals` is, because
* the host computed it with an ownership rule (`approval_owner`) that has an
* attempt-level key this side cannot see. These rows are matched into it by
* id, so an approval the host counts and the feed has not caught up on is
* still counted — it just goes unnamed for one poll rather than disappearing.
*/
parked: readonly ApprovalSummary[];
taskId: string;
now: number;
}) {
const pending = pendingApprovalWait(approvals, now);
const named = useMemo(() => {
if (!pending) return null;
const ids = new Set(approvals.filter((a) => a.status === "pending").map((a) => a.id));
const hits = parked.filter((p) => ids.has(p.id));
// Only when *every* pending row is named, and there is exactly one. Naming
// "the" blocked call while a second one the feed has not delivered sits
// beside it would tell the operator one decision clears the card when two
// do — the precise mistake issue #883 is about.
return hits.length === 1 && pending.count === 1 ? hits[0] : null;
}, [approvals, parked, pending]);
if (!pending) return null;
const { waited } = pending;
const href = `#/approvals/${encodeURIComponent(taskId)}`;

return (
<div className="flex items-center gap-2 rounded-lg border border-status-blocked/30 bg-status-blocked-soft px-3 py-2 text-xs">
<Hourglass className="size-3.5 shrink-0 text-status-blocked-text" />
<span className="min-w-0 flex-1 text-status-blocked-text">
{pending.count === 1
? "Waiting on an approval"
: `Waiting on ${pending.count} approvals`}{" "}
{/* Issue #883: name the call, not the mechanism. "Waiting on an
approval" is true of every one of these rows and therefore answers
nothing — the same complaint #372 made of the chat card and #846 of
the workflow one. `approvalAction` is the function both of those were
fixed with, so all three surfaces say one thing about one approval. */}
{named
? `Waiting on your approval — ${approvalAction(named).toLowerCase()}`
: pending.count === 1
? "Waiting on an approval"
: `Waiting on ${pending.count} approvals`}{" "}
<span className="tabular-nums">for {formatDuration(waited)}</span>
</span>
<a
href="#/approvals"
href={href}
className="shrink-0 font-medium text-status-blocked-text underline-offset-2 hover:underline"
aria-label={
pending.count === 1
Expand Down
Loading
Loading