Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
62 changes: 62 additions & 0 deletions docs/spec/company-brain/approvals.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,68 @@ card can appear live. It is deliberately thin — an id, a dotted kind, a thread
because the effect's payload is redacted in exactly one place and must not
acquire a second. A reader re-reads the approvals feed for the rest.

### One turn is asked about once (issue #842)

A research turn that reaches `espn.com`, `bbc.com` and `theguardian.com` parks
three approvals, and asking three times is the same fact told badly: it is one
piece of work, and every interruption costs a re-dispatch cycle that can
dead-end. So the parks a single turn raised are **surfaced as one request**.

The grouping key is not new. Issue #469 already journals the parking cycle, so
that a turn blocked on four decisions is continued exactly once when the last
one lands. `ApprovalSummary.batch` projects that same key, which is what makes
the two agree by construction: the batch an operator is asked about in one card
is precisely the batch the runtime holds a single continuation for. It is opaque
— an equality key, never an ordering, a count, or anything to show an operator.

**The grant model does not change at all.** There is no batch entity on the
host, no batch resolve on the wire, and nothing new in how a grant is minted,
stored or revoked. Each approval keeps its own id, its own verdict and — on
approve — its own host-scoped grant, so approving three fetches still leaves
three independently revocable rows under `Standing permissions`, one per host,
each with its own expiry. Batching the *asking* is not batching the *granting*,
and widening a grant to save a click would be exactly the leak `grants.md`
exists to prevent.

Two renderings over that one state, divided by what each surface is **for**:

- **Chat is the fast path: all-or-nothing.** One card per turn, listing the
hosts it covers, with a single Approve/Decline and the ordinary scope
control. Approve grants every call in the batch; Decline grants none. The
operator is mid-conversation and wants one decision, not a form. It answers
every item it is still asking about, because the turn stays blocked until each
parked call has a verdict — a decision that left one open would hold the turn
while looking as though it had resolved the card.
- **The Approvals page is the granular path: itemised.** One row per gated
call, approved or declined on its own, matching how `Standing permissions`
lists one revocable row per grant. It is where an operator goes for precision,
or to clean up after the fact. A row says how many others came from the same
turn, so someone arriving from the toast can tell one batch from an unrelated
queue.

Granular control in *both* places would be redundant, and would double the state
that has to stay in step between two surfaces — so it lives in one.

**A decision that does not land is named, not swallowed.** One click fans out to
one resolve per item, so a failure on the third leaves two effects authorised
and one not. A toast is the wrong home for that — it does not say *which*, and
it is gone by the time the operator looks back at the card — so the row that
failed says so itself, the card counts the failures honestly (never "nothing was
recorded" about a click that authorised two of three), and the buttons stay live,
because a retry is the way out. A retry re-resolves only what is still pending.

The two must not drift, and do not, because neither owns any state: both render
the same feed, and both react to the `approval_resolved` frame. Deciding a row
on the page settles that item on the chat card without a reload, and the card
reports a partial state (`1 of 3 decided`) rather than going on claiming three
things are pending.

An approval with **no** batch — a workflow node, a scheduler tick, a park
journaled before #469 — is never grouped, not even with another one like it.
Absent means "the host did not say which turn this came from", and folding two
unknowns together would invent a batch out of a shared silence. Each is shown
alone, exactly as before this existed.

## Delegation levels (standing rules)

Prosumers adjust the fence in plain language, which compiles to policy:
Expand Down
21 changes: 21 additions & 0 deletions frontend/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,27 @@ export interface ApprovalSummary {
* alone, exactly as every approval did before this shipped.
*/
thread?: string | null;
/**
* Which turn's gated calls this one belongs to (#842) — an opaque key shared
* by every approval a single agent turn parked.
*
* **A display grouping, never a decision.** One research turn that reaches
* three sites parks three approvals, and each stays its own record with its
* own id, its own approve/decline and — on approve — its own host-scoped
* grant (#739). The conversation consolidates them into one card so it
* interrupts once instead of three times; the Approvals page deliberately
* keeps one row per approval, matching how `Standing permissions` lists one
* revocable row per grant. Resolving is per id on both surfaces.
*
* Never compare it for anything but equality, and never show it: it is a
* runtime identifier, which the glossary rule keeps off an operator's screen.
*
* Absent for an approval no turn raised (a workflow node, a scheduler tick)
* and against a host that predates the field. Both are grouped alone, which
* is exactly the pre-#842 rendering — so an old host still produces a card
* that can be decided.
*/
batch?: string | null;
}

/**
Expand Down
36 changes: 36 additions & 0 deletions frontend/src/components/app-shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,21 @@ export function AppShell({
() => new Map(),
);
const [decidedApprovals, setDecidedApprovals] = useState<Record<string, DecidedApproval>>({});
/**
* Decisions that did **not** land, per approval id (#842 review).
*
* A third map, and it earns its keep because of consolidation. Deciding three
* cards separately, a failure belongs to the one card just clicked and the
* toast is beside it. Deciding one card that covers three, a failure on the
* third leaves two effects authorised and one not — and an item that simply
* drops back to its pending look reads as "still working", not "this one did
* not take". The operator clicked once and got two thirds of what they asked
* for, with nothing on screen saying which third.
*
* Cleared when that item is decided again, so a retry starts from a clean
* state rather than showing the previous attempt's error under a live one.
*/
const [failedApprovals, setFailedApprovals] = useState<Record<string, string>>({});

const pending = feed.status.pending_approvals;

Expand Down Expand Up @@ -442,6 +457,7 @@ export function AppShell({
// must not survive the switch as a ghost in the new company's channels.
setDecidedApprovals({});
setDecidingApprovals(new Map());
setFailedApprovals({});

const hydrate = (threadId: string) => {
client
Expand Down Expand Up @@ -893,6 +909,16 @@ export function AppShell({
});
}, []);

/** Drops a recorded failure — a retry is starting, or the item is gone. */
const clearFailure = useCallback((id: string) => {
setFailedApprovals((prev) => {
if (!(id in prev)) return prev;
const next = { ...prev };
delete next[id];
return next;
});
}, []);

/**
* Decide an approval from inside the conversation it was raised in (#379).
*
Expand Down Expand Up @@ -922,6 +948,9 @@ export function AppShell({
) => {
if (decidingApprovals.has(approval.id)) return;
markDeciding(approval.id, verdict);
// A retry starts clean: the previous attempt's error must not sit under a
// live one, or the operator cannot tell which attempt it belongs to.
clearFailure(approval.id);
try {
await client.resolveApproval(approval.id, verdict, undefined, company, {
detach: true,
Expand All @@ -943,6 +972,12 @@ export function AppShell({
const msg = err instanceof ApiError ? err.message : "something went wrong";
toast.error(`Couldn't record your decision — ${msg}`);
noteInChannel(approval.thread, `Couldn't record your decision — ${msg}`);
// On the card as well as in a toast, and keyed to the item that failed.
// A toast is the wrong and only home for this once one click covers
// several calls: it says a decision failed without saying *which*, and it
// is gone by the time the operator looks back at the card. The row that
// did not take has to say so itself.
setFailedApprovals((prev) => ({ ...prev, [approval.id]: msg }));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} finally {
markDeciding(approval.id, null);
void feed.refresh();
Expand Down Expand Up @@ -1117,6 +1152,7 @@ export function AppShell({
}
decidingApprovals={decidingApprovals}
decidedApprovals={decidedApprovals}
failedApprovals={failedApprovals}
/>
)}
{view === "conversation" && (
Expand Down
42 changes: 41 additions & 1 deletion frontend/src/views/ApprovalsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,29 @@ export function ApprovalsView({ client, company, feed, onResolved, onGoToConvers
const { approvals, now } = feed;
const askerNames = useAskerNames(client, company, approvals);
const { grants, granterNames, refreshGrants } = useStandingGrants(client, company);
/**
* How many rows each turn's batch still has waiting (#842).
*
* The page stays **itemised** — one row per gated call, each independently
* approvable, exactly as `Standing permissions` below lists one revocable row
* per grant. This is the one thing it borrows from the conversation's
* consolidated card: a row says how many others were asked for alongside it,
* so an operator who arrives here from the toast can tell "this is one of
* three from one turn" from "these are three unrelated requests" — which is
* the difference between deciding the batch and deciding a queue.
*
* Counted over what is still pending rather than over the whole batch, so the
* number shrinks as rows are decided instead of promising a fourth row that
* has already been signed off.
*/
const batchTotals = useMemo(() => {
const counts = new Map<string, number>();
for (const a of approvals) {
if (!a.batch) continue;
counts.set(a.batch, (counts.get(a.batch) ?? 0) + 1);
}
return counts;
}, [approvals]);

const markInFlight = (id: string, verdict: Verdict | null) =>
setInFlight((prev) => {
Expand Down Expand Up @@ -207,6 +230,7 @@ export function ApprovalsView({ client, company, feed, onResolved, onGoToConvers
now={now}
askerNames={askerNames}
deciding={inFlight.get(a.id) ?? null}
batchTotal={batchTotals.get(a.batch ?? "") ?? 1}
onDecide={(verdict, scope) => void decide(a, verdict, scope)}
/>
))}
Expand Down Expand Up @@ -477,13 +501,19 @@ function ApprovalCard({
now,
askerNames,
deciding,
batchTotal,
onDecide,
}: {
approval: ApprovalSummary;
now: number;
askerNames: Map<string, string>;
/** The verdict this card is waiting on, or `null` when it is idle (#373). */
deciding: Verdict | null;
/**
* How many rows this turn's batch still has waiting, including this one
* (#842). `1` — the default for an approval with no batch — says nothing.
*/
batchTotal: number;
onDecide: (verdict: Verdict, scope: GrantScope) => void;
}) {
// Per-card, like the in-flight verdict and for the same reason: two cards can
Expand Down Expand Up @@ -559,7 +589,17 @@ function ApprovalCard({
approve is not done when the button stops spinning, it is handed
to the agent. A decline IS terminal, so it only has to record. */
status={
deciding ? (deciding === "approve" ? "Waiting for the agent…" : "Recording…") : undefined
deciding
? deciding === "approve"
? "Waiting for the agent…"
: "Recording…"
: batchTotal > 1
? // Deliberately a count and not a link: the row is decided
// here, on its own, and pointing at the others would imply a
// batch decision this page does not offer. The conversation's
// card is where one Approve covers all of them (#842).
`1 of ${batchTotal} from the same turn`
: undefined
}
/>
</CardContent>
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/views/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ interface Props {
/** The verdict each card is waiting on, and the ones already witnessed. */
decidingApprovals?: ReadonlyMap<string, Verdict>;
decidedApprovals?: Record<string, DecidedApproval>;
/**
* Decisions that did not land, per approval id (#842) — the message to show
* on that item. Owned by the shell, like the two maps above, because a
* failure has to outlive this view unmounting: the operator's next move after
* one is often to open the Approvals page and come back.
*/
failedApprovals?: Record<string, string>;
}

/**
Expand Down Expand Up @@ -161,6 +168,7 @@ export function ChatView({
onDecideApproval,
decidingApprovals,
decidedApprovals,
failedApprovals,
}: Props) {
// Which (connection, company) this subtree's browser-local state belongs to.
const scope = useLocalScope();
Expand Down Expand Up @@ -827,6 +835,7 @@ export function ChatView({
now={now}
askerNames={askerNames}
decidingApprovals={decidingApprovals}
failedApprovals={failedApprovals}
onDecideApproval={onDecideApproval}
/>
{consoleOnlyMember && (
Expand Down
Loading
Loading