Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
54c3f11
feat(console): a stacked ApprovalRow variant for a board card (#1891)
sanil-23 Aug 27, 2026
878a977
feat(console): a blocked board card shows and decides its approvals (…
sanil-23 Aug 27, 2026
ee2186c
feat(console): the task detail decides its approvals, and says what i…
sanil-23 Aug 27, 2026
bf2180e
fix(console): scope witnessed verdicts to the live queue, and hold Re…
sanil-23 Aug 27, 2026
7a394f5
fix(console): bound the witnessed fold to open batches, and let the h…
sanil-23 Aug 27, 2026
50be7a1
fix(console): a board card is not a batch — group its approvals per t…
sanil-23 Aug 27, 2026
2981a93
Merge remote-tracking branch 'refs/remotes/upstream/main' into pr/1895
senamakel Aug 28, 2026
664e764
chore(api): update API type definitions
senamakel Aug 28, 2026
d88adb8
chore(runtime): update types
senamakel Aug 28, 2026
0ed3871
chore: update runtime implementation
senamakel Aug 28, 2026
7061616
chore: files changed src/company/runtime.rs
senamakel Aug 28, 2026
61d0b65
chore(runtime): update runtime implementation
senamakel Aug 28, 2026
ebff908
chore(runtime): update runtime implementation
senamakel Aug 28, 2026
fbf875b
chore(runtime): update runtime module
senamakel Aug 28, 2026
119b8c8
chore(api): update api type definitions
senamakel Aug 28, 2026
2d20ca0
chore(runtime): update runtime implementation
senamakel Aug 28, 2026
491ea8f
chore: files changed src/server/ops/tasks.rs
senamakel Aug 28, 2026
6a22444
chore(ops): update task operations
senamakel Aug 28, 2026
3f91f0d
chore: update task API
senamakel Aug 28, 2026
e09274a
chore: update ledgers view
senamakel Aug 28, 2026
ea97487
chore: files changed frontend/src/views/LedgersView.tsx
senamakel Aug 28, 2026
ba52776
chore(ledgers): update ledger view
senamakel Aug 28, 2026
5327320
fix(ledgers): use authoritative approvals for board entries
senamakel Aug 28, 2026
b755e5d
chore(ops): update task operations
senamakel Aug 28, 2026
14c1bbf
feat(tasks): group pending approvals by task
senamakel Aug 28, 2026
60d3804
chore: files changed frontend/src/api/tasks.ts
senamakel Aug 28, 2026
c9ace7c
fix(ledgers): load authoritative approvals directly
senamakel Aug 28, 2026
17b1c43
Merge remote-tracking branch 'refs/remotes/upstream/main' into pr/1895
senamakel Aug 28, 2026
007b5b5
chore: update task card
senamakel Aug 28, 2026
192fcc2
chore(frontend): update task card
senamakel Aug 28, 2026
10f836a
chore(frontend): update ledgers view
senamakel Aug 28, 2026
e702645
chore(frontend): update ledgers view
senamakel Aug 28, 2026
f2a8ace
chore: update ledgers view
senamakel Aug 28, 2026
f1a991e
chore(app-shell): update application shell
senamakel Aug 28, 2026
b0d8734
chore(frontend): update app shell
senamakel Aug 28, 2026
15589c5
chore: files changed frontend/src/components/app-shell.tsx
senamakel Aug 28, 2026
9525446
chore(frontend): update app shell
senamakel Aug 28, 2026
74fc6e2
chore: update app shell
senamakel Aug 28, 2026
990e1a7
chore: files changed frontend/src/components/app-shell.tsx
senamakel Aug 28, 2026
15cfc93
chore: update api types
senamakel Aug 28, 2026
e17d57a
chore(task-approvals): update task approval handling
senamakel Aug 28, 2026
f666bb7
chore(frontend): update task card
senamakel Aug 28, 2026
6c187ee
fix(runtime): resolve approval ownership in the queue projection (#1891)
sanil-23 Aug 28, 2026
f71141d
fix(runtime): resolve ownership only for parks whose run id is an att…
sanil-23 Aug 28, 2026
86bf801
fix(runtime): a failed owner lookup must not unlink a parked approval…
sanil-23 Aug 28, 2026
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
1 change: 0 additions & 1 deletion frontend/src/api/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,6 @@ export function taskStatusesById(
return statuses;
}

/** The steer body. `redirect` requires `instruction`; `cancel` requires `confirm`. */
export interface SteerInput {
action: SteerAction;
instruction?: string;
Expand Down
19 changes: 17 additions & 2 deletions frontend/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,16 +491,31 @@ export interface ApprovalSummary {
*/
group?: "spend" | "send" | "sign" | "publish" | "hire" | "identity" | "other";
/**
* Which board task this approval was parked for (#333). Mirrors `TaskLink` in
* `src/runtime/journal.rs`.
* Which board task **owns** this approval (#333, resolved by #1891).
*
* Three states, deliberately: `{link: "task"}` is owned by that card,
* `{link: "unlinked"}` is owned by no card (a workflow delivery, an
* operator-chat turn, a scheduler tick), and *absent* means the park predates
* the field. Only the last one is ambiguous — the server keeps a run-window
* heuristic for it, and for nothing else.
*
* **The host's answer, not the park's stamp.** Until #1891 this mirrored the
* raw `TaskLink` the parking cycle wrote, which is only the *fallback* half
* of the host's ownership rule: the attempt behind a park outranks the card
* it was stamped with wherever there is one, and the task detail read has
* always applied that (`approval_owner`). So an approval parked under one
* card's attempt and stamped with another's arrived here under the stamp, and
* a client joining on it put the row on the wrong card. `pending_approvals_resolved`
* now applies the same rule before serialising, so this field and
* `…/tasks/{id}`'s `approvals` cannot disagree.
*
* That is what makes joining on it safe enough to hang a decision off — see
* `approvalsForTask` in `@/lib/task-approvals`, which is how the board card
* finds what it is blocked on and what it is offering to resolve.
*/
task?: { link: "task"; id: string } | { link: "unlinked" };
/** The host-resolved task owner, when an authoritative task-detail projection provides it. */
ownerTaskId?: string;
/**
* The roster teammate whose blocked tool call this is (#372). Mirrors
* `Effect::agent`: present exactly when the effect came from a harness tool
Expand Down
37 changes: 32 additions & 5 deletions frontend/src/components/app-shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2861,6 +2861,19 @@ export function AppShell({
// than only counting it. The feed the sidebar badge already polls,
// so the screen says what it is waiting on with no second request.
parked={feed.approvals}
// Issue #1891: and decided here too, not only named. The same
// bundle the board and the run drawer get, so a verdict given on
// any of the three settles on the others with no reload. Named
// as this route's own props rather than the `…Approvals` suffix
// the section views take: it is a thin wrapper whose props mirror
// `TaskDetailView`'s, which has no other kind of decision to
// qualify these against.
deciding={decidingApprovals}
decided={decidedApprovals}
failed={failedApprovals}
onDecide={(approval, verdict, scope) =>
void decideApproval(approval, verdict, scope)
}
// 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.
Expand Down Expand Up @@ -2933,11 +2946,25 @@ export function AppShell({
// second request.
approvals={feed.approvals}
now={feed.now}
// 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))}
// Issue #1891: a blocked card decides in place rather than only
// reporting that it is blocked. The same four maps the run drawer
// receives, owned here for the same reason — an operator who
// decides on the board, steps over to Approvals and comes back
// must not find a card that forgot what they did. `decided` is
// fed by the `approval_resolved` frame as well as by this
// console's own resolves, so a decision taken on the page settles
// on the board with no reload.
//
// This replaces `onReviewApprovals`: the card's own "View
// details" is an `href` built with `withHostParam`, which lands
// the same `#/approvals/<taskId>` in the hash — surviving a
// refresh and the Back button — without a callback to route it.
decidingApprovals={decidingApprovals}
decidedApprovals={decidedApprovals}
failedApprovals={failedApprovals}
onDecideApproval={(approval, verdict, scope) =>
void decideApproval(approval, verdict, scope)
}
// The switcher's in-place wizard declared a new list — re-read
// the shared list so it shows up in the menu (and Manage
// Lists, which reads the same instance) with no reload.
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/components/approval-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,15 @@ function workflowIdForApproval(approval: ApprovalSummary): string | null {
* returns nothing at all and inherits the meta line's muted grey, which keeps
* the quiet case exactly as it shipped — the emphasis is only worth anything if
* most cards do not have it.
*
* Exported for the board card (#1891), which paints the deadline without the
* rest of {@link ApprovalMeta}: a `w-65` column has no room for the origin
* pills, and "Open the card" on the card you are already looking at is the same
* redundancy `OutputLinkRow` exists to avoid. The *tone* is not the board's to
* re-decide, though — a surface that invented its own amber would be the one
* place a passing deadline looked routine.
*/
function deadlineToneClass(tone: DeadlineTone): string | undefined {
export function deadlineToneClass(tone: DeadlineTone): string | undefined {
if (tone === "passed") return "font-medium text-status-failed-text";
if (tone === "soon") return "font-medium text-status-blocked-text";
return undefined;
Expand Down
185 changes: 173 additions & 12 deletions frontend/src/lib/task-approvals.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { TaskApproval } from "@/api/tasks";
import type { ApprovalSummary } from "@/api/types";
import type { ApprovalSummary, Verdict } from "@/api/types";
import type { DecidedApproval } from "@/views/chat/model";

/**
* What the task card says about approvals (issue #468).
Expand Down Expand Up @@ -68,22 +69,36 @@ export function pendingApprovalWait(
* 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.
* **Only `{link: "task"}` matches, and since #1891 that is the host's own
* ownership answer rather than an approximation of it.** The field used to
* carry the raw link `CycleHostImpl::park` stamped, which is only the fallback
* half of `approval_owner`: the attempt behind a park outranks the card it was
* stamped with wherever there is one. This join could not reach that rule — no
* attempt id is on the wire, and it would need every card's attempt ids anyway
* — so an approval parked under one card's attempt and stamped with another's
* landed on the wrong card here.
*
* That was a wrong label while the row was read-only. It stopped being
* survivable when the card grew Approve and Decline, because the operator would
* have been resolving another card's request, so the resolution moved to the
* host: `pending_approvals_resolved` applies the rule before serialising and
* this reads the answer. See `ApprovalSummary.task`.
*
* `{link: "unlinked"}` (a workflow delivery, an operator-chat turn, a scheduler
* tick, or an attempt no card claims) 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);
return approvals.filter(
(a) =>
(a.ownerTaskId === taskId ||
(a.ownerTaskId === undefined && a.task?.link === "task" && a.task.id === taskId)),
);
}

/** Why a card is stopped — read by its board card and by its Resume button (#883). */
Expand Down Expand Up @@ -123,3 +138,149 @@ export function taskApprovalBlock(
if (mine.length === 0) return null;
return { approvals: mine, count: mine.length, since: mine[0].at_millis };
}

/**
* One approval this card is (or was) held on, and what has become of it (#1891).
*
* The `verdict` half is why this exists rather than the card rendering
* {@link TaskApprovalBlock.approvals} straight: a resolve's answer and the
* feed's next poll are two moments, and for that gap a decided row must read as
* decided rather than offering its buttons a second time.
*/
export interface TaskApprovalRow {
approval: ApprovalSummary;
/**
* The verdict already witnessed for it — from this card, from the Approvals
* page, or from another operator's console over the event stream — or `null`
* while it is still waiting on somebody.
*/
verdict: Verdict | null;
}

/**
* Every approval one card is blocked on, decidable, in a stable order (#1891).
*
* The task-board answer to the problem the run drawer had first (#1002): it
* *told* an operator their run had parked and gave them nowhere to act, and the
* board is that fix arriving at the surface an operator actually works from.
*
* **The live queue decides which rows exist; `decided` may only extend one of
* its open batches.** That is the difference from `runApprovals` in
* `@/views/workflows/run-approvals`, which this otherwise mirrors, and copying
* its second pass wholesale was wrong (#1895 review): a workflow *run* is
* one-shot, so every verdict the console has ever witnessed for it belongs to
* it, while a **task is re-dispatched**. The shell holds `decided` for the whole
* company session — cleared on a company switch, never pruned — so folding in
* every historical verdict for a task id meant a card parking one new approval
* next week reporting "3 of 4 decided", the count growing with every repeat
* run. A card that looks completely normal while saying the wrong thing.
*
* Dropping the fold entirely was the over-correction, and it cost the thing the
* board's one-click batch depends on (#1895 review, second pass). That click
* resolves each id separately, and `decideApproval` refreshes the feed on each
* one — so when two of three succeed and the third fails, the successes leave
* the queue and a live-only projection collapses to a single-item card reading
* "Not recorded — try again". The operator authorised two effects and the card
* no longer says so; `ApprovalRow`'s whole partial-failure disclosure ("2 of 3
* weren't recorded") needs the original `total` to be able to speak.
*
* So the bound is the **batch** — `ApprovalSummary.batch`, the opaque key every
* approval one agent turn parked shares. A witnessed verdict is re-attached
* only when some approval of that same batch is *still* in the queue for this
* card. That keeps a batch whole while it is settling, releases it entirely
* once the host has taken the last of it, and cannot reach across runs: a
* re-dispatch parks a new turn, and a new turn is a new batch. An approval with
* no batch (a scheduler tick, an older host) is never re-attached, which is the
* safe direction — it degrades to the live queue.
*
* The annotation on a row still in the queue is separate and unconditional: for
* the moment between the resolve's answer and the feed's next poll the queue
* still holds a settled approval, and a row that offered its buttons again
* there would invite a second decision on it.
*
* Ordered oldest park first, then by id — stable across a refresh, because the
* queue hands back a fresh array on every poll and a list that reordered under
* a pointer mid-click would be its own bug. Same ordering
* {@link taskApprovalBlock} uses, so the row the card measures its wait from is
* the row it draws first.
*/
export function taskApprovalRows(
approvals: readonly ApprovalSummary[],
decided: Readonly<Record<string, DecidedApproval>>,
taskId: string,
): TaskApprovalRow[] {
const queued = approvalsForTask(approvals, taskId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify authoritative ownership before enabling board decisions

When an approval's task link names card A but its run_id belongs to card B, this link-only join includes it on A; the_attempt_id_outranks_the_card_link_when_both_are_present in src/server/ops/write_test.rs explicitly proves that the host excludes this case from A's authoritative task detail. Unlike the previous read-only blocker display, the new board path passes these rows to ApprovalRow with onDecide, so an operator can now approve or decline B's request from A's card. The board needs an authoritative ownership projection (or equivalent host-provided filtering) before exposing decision controls rather than relying solely on approvalsForTask.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, and this contradicts an argument I made on the sibling thread, so let me withdraw that explicitly rather than quietly.

I fixed the ownership asymmetry on the detail screen and said the board could keep the looser join because "the cost of getting it wrong is lower: a card that says it is blocked and disables Resume, rather than a surface offering a decision". That was true of #883's read-only row. It stopped being true the moment this PR put Approve on the card, which is the whole change — so the justification I gave was for the code I was replacing, not the code I was writing.

I checked the test you cite rather than take it on trust. the_attempt_id_outranks_the_card_link_when_both_are_present (src/server/ops/write_test.rs:6278) parks appr-elsewhere under run-c — an attempt at t-other — stamped TaskLink::Task { id: "t-1" }, and asserts it must not appear on t-1. approval_owner (src/server/ops/tasks.rs:1862) resolves the attempt first and falls back to the link only when there is no run_id. So approvalsForTask really can attribute an approval to the wrong card, and this PR makes that a resolvable request rather than a misleading label.

The console cannot close this on its own. ApprovalSummary carries no attempt id for a task-linked park — workflow_run_id is stamped only for unlinked parks precisely because Effect::run_id also carries task-attempt ids that must not be read as workflow runs — and approval_owner needs task_runs, which is per-card state the board does not have. Re-reading task detail per card is the N-reads-per-poll cost #883 exists to avoid.

What would close it is the queue projection resolving the owner itself: for a parked approval carrying a run_id, look up that run's card (RunRecord names one) and project the resolved owner in ApprovalSummary.task instead of the raw park link. Then approvalsForTask becomes authoritative for every consumer, and #883's existing blocked row — which has the same latent misattribution today, read-only — gets correct as a side effect.

That is a host change and this PR is frontend-only, so I am checking on scope before expanding it rather than either shipping the decision surface as it stands or quietly dropping the feature. I will follow up here with which way it goes.

const byId = new Map<string, TaskApprovalRow>(
queued.map((approval) => [
approval.id,
{ approval, verdict: decided[approval.id]?.verdict ?? null },
]),
);
// The batches this card still has something parked under. Empty when the
// host has taken everything, which is what makes the fold below terminate.
const open = new Set(
queued.map((a) => a.batch).filter((b): b is string => typeof b === "string" && b !== ""),
);
if (open.size > 0) {
for (const [id, d] of Object.entries(decided)) {
if (byId.has(id)) continue;
const link = d.approval.task;
if (link?.link !== "task" || link.id !== taskId) continue;
if (!d.approval.batch || !open.has(d.approval.batch)) continue;
byId.set(id, { approval: d.approval, verdict: d.verdict });
}
}
return [...byId.values()].sort(
(a, b) =>
a.approval.at_millis - b.approval.at_millis ||
(a.approval.id < b.approval.id ? -1 : a.approval.id > b.approval.id ? 1 : 0),
);
}

/** The subset still waiting on somebody — what the card is actually stopped behind. */
export function blockingTaskApprovals(rows: readonly TaskApprovalRow[]): TaskApprovalRow[] {
return rows.filter((r) => r.verdict === null);
}

/** Stable empties, so an idle card's props do not churn on every board poll. */
const NO_VERDICTS: Record<string, Verdict> = {};
const NO_DECIDING: ReadonlyMap<string, Verdict> = new Map();

/**
* The verdicts already witnessed for one card's rows (#1891).
*
* What `ApprovalRow` means by `decided`: the map it subtracts from its batch to
* work out what an Approve still covers. Handing it the console-wide map would
* be nearly the same thing and quietly wrong on the count — a decision on some
* other card's approval would be subtracted from this card's total.
*/
export function taskApprovalVerdicts(rows: readonly TaskApprovalRow[]): Record<string, Verdict> {
const verdicts: Record<string, Verdict> = {};
for (const row of rows) if (row.verdict) verdicts[row.approval.id] = row.verdict;
return Object.keys(verdicts).length === 0 ? NO_VERDICTS : verdicts;
}

/**
* The decisions in flight on **this** card, as their own map (#1891).
*
* Narrowed for the reason #373 established and `RunResultPanel` repeats: the
* shell's `deciding` map is console-wide, and `ApprovalRow` reads `size > 0` as
* "I am busy". Passed whole, one operator click would grey out the buttons on
* every blocked card on the board at once.
*
* Per *card* rather than per row, unlike the run drawer's version: the card
* renders one batch with one Approve, so every id it covers is genuinely in
* flight together and each should show it.
*/
export function decidingForTask(
rows: readonly TaskApprovalRow[],
deciding: ReadonlyMap<string, Verdict>,
): ReadonlyMap<string, Verdict> {
if (deciding.size === 0) return NO_DECIDING;
const mine = new Map<string, Verdict>();
for (const row of rows) {
const verdict = deciding.get(row.approval.id);
if (verdict) mine.set(row.approval.id, verdict);
}
return mine.size === 0 ? NO_DECIDING : mine;
}
Loading