fix(console): a paused card names the approvals blocking it, and Resume stops re-running them (#883) - #945
Conversation
…me stops re-running them A paused card carried a Resume button and no reason, so "decided one of five, still waiting on four" and "wedged" were the same pixels — and Resume was the natural next click from both. It is the wrong click from the first: a turn continues once, when the last decision it parked lands (#469), so re-dispatching re-runs the agent from the top, parks the same calls again, and grows the queue. The card now names what it is waiting on, through the same `approvalAction` the Approvals page and the chat card label their rows with, and disables Resume while any of its approvals are undecided. "Review" opens the queue narrowed to that card via `#/approvals/<taskId>`. Derived from the approvals feed the shell already polls for the sidebar badge, so the board makes no new request and no wire field changes. Closes #883
|
@coderabbitai review |
|
|
Warning Review limit reached
Next review available in: 58 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
How this change flows3 changed behaviours across 2 relationships. The code graph does not know these behaviours yet — normal for newly added code, and a cold index otherwise. 11 further behaviours left out to keep the diagram readable. flowchart LR
n0["AppShell<br/>changed"]:::changed
n1["ApprovalsView<br/>changed"]:::changed
n2["Props<br/>changed"]:::changed
n0 -->|uses| n1
n1 -->|uses| n2
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
tinysweeper
Bot
added
the
priority: p3
2 participants
Summary
A paused card carried a Resume button and nothing about why it was paused. The card is blocked
until every approval its turn parked has been decided — the #469 continuation rule: a turn is
continued once, when the last decision it was blocked on lands. Neither the count nor the identity of
those approvals reached the card.
That makes two different states look identical:
Resume is the natural next click from both, and it is the wrong click from the first: it
re-dispatches, the agent re-runs from the top, parks the same calls again, and the queue grows. The
issue's own reproduction is that loop.
1. The card names the blocked call — through the function every other surface already uses
This is the part worth reading closely, because it is a cross-surface consistency change rather
than "the card now shows some text".
approvalActioninfrontend/src/lib/language.tsis the one place that answers "what is thisapproval asking for". It is what #372/#375 fixed the chat card with when it was showing raw
runtime identifiers ("Glob", "Shell"), and what PR #854 extended when the workflow card was
labelled
Continue a paused workflow— true of every one of those cards and therefore an answer tonothing. The board card was the third surface with the same defect and no fix.
So the blocked row calls the same function. All three now say "Fetch a web page" about the same
approval, instead of three different things — and a glossary entry added for one of them lands on all
three. Nothing new was written here; the wording layer is reused as-is.
Blocked on 4 approvals), because five tool names is not something toread on a Kanban card. The count plus a Review link is, and the page it links to lists them.
2. Resume is disabled, not hidden — with the reason on it
Stated explicitly because it is a deliberate choice a reviewer will otherwise ask about.
Hiding the button would re-create the bug. The ambiguity being fixed is a card that gives the
operator no way to tell "blocked, and here is on what" from "stuck". A card whose only action
silently disappeared answers neither — it just moves the confusion from which click to where did
the button go. The operator has to be able to see that Resume exists and is the wrong click right
now.
So the button stays,
disabled, carrying its reason intitle("Blocked — decide its approvals first; resuming re-runs the work from the start") for a pointer,
with the blocked row above it carrying the same reason for everyone else.
The rule lives in exactly one place —
taskApprovalBlock— read by the button'sdisabledandby the row that explains it, so the card cannot say "blocked on 4 approvals" beside a button that
dispatches anyway. There is deliberately no second guard inside
resume():onResumeis wiredonly to that button, so a copy of the rule there would be a branch nothing can execute and therefore
a branch nothing keeps true.
3. The queue can be narrowed to one card
#/approvals/<taskId>, using the unvalidated second hash segment every other view already takes(
#/workflows/<id>,#/team/<agentId>). Linkable, survives a refresh, honours Back.A filtered queue with nothing left in it renders "This card is clear", deliberately not the
existing "Nothing is waiting on you" — that one is a claim about the whole company, and it would
be said here while other cards' approvals sit one click away unread. It is also the wrong answer to
the question the operator arrived with.
4. The detail row names the call too
AwaitingApprovalRowalready said that the card was waiting and for how long (#468). It now alsosays what, which is what the MVP spec asks for: "clicking the task shows what approval is
needed."
Its source of truth is unchanged:
detail.approvals, the host's own read, still decides whetherthe card is waiting — because
approval_ownerresolves ownership with an attempt-levelrun_idkeythis side cannot see. The feed only supplies the words, matched in by approval id. An approval the
host counts and the feed has not caught up on is still counted; it goes unnamed for one poll rather
than disappearing. It also only names the call when there is exactly one and every pending row was
matched — naming "the" blocked call while a second sits beside it would say one decision clears the
card when two do, which is the precise mistake this issue is about.
Where the data comes from — and why there is no new wire field
The board reads
…/tasks, whose card projection carries no approvals, and opening every card to findout would be N reads per 4s poll — the cost
TaskCard::output's own docs record as the reason itrides the board read.
Neither was needed. The shell already polls
GET …/approvalsevery 5s for the sidebar badge, andsince #333 every entry carries
task: {link: "task", id}.CycleHostImpl::parkstamps that link onevery park path, so a card-dispatched approval always has it. The board joins against the feed it
was already paying for.
The consequence worth noting: the board and the Approvals page now read one list, not two that can
disagree.
Two states are skipped rather than guessed at, and both are deliberate:
{link: "unlinked"}— a workflow delivery, an operator-chat turn, a scheduler tick. Counting onewould put "blocked on 1 approval" on a card that is not blocked, then disable its Resume forever,
since deciding that approval is not something the operator would ever connect to this card.
the board has no window to apply it against, so it skips.
approval_owner's first rule — the attempt-levelrun_id— is unreachable from here. It separatestwo runs of the same card, which does not change the answer to "is this card blocked", the only
question the board asks.
What this does not fix — stated rather than left implicit
Resume still re-runs. This stops the operator walking into the loop; it does not make a resume
resumable. That is engine work in the vendored pause primitive, and it is already filed twice:
NodeControl::Interruptdiscards the activation's state update, so the node re-runs from the top;finish_agent_runalready refuses with "resuming a paused agent is not supported yet". #846'sPAYLOAD_PERFORMEDledger keys on atool_callnode's static slug and args, and an agent's internal calls are chosen mid-turn, so there is nothing to key against.shell/curl/git_operationsstill repeatEffectGroup::Other, which is useless as a replay predicate — the host cannot tell agit logfrom agit push. #846 refused to fold it in on the grounds that replaying on a guess is a bigger behavioural change than the bug.So: the issue's two halves land in different places. "A paused card never says which approvals block
it" is fixed here, completely. "Resume re-parks it" is contained here — the operator can no
longer start it from a blocked card — and fixed in #899 / #850, neither of which this touches.
A card sitting in Paused with no approvals outstanding — one moved there by hand, or a
RunStatus::Paused"resumed, not approved" card whose approvals have since been decided — keeps itsenabled Resume and its existing behaviour exactly.
Also untouched by design: batching (#842 / PR #848) — nothing here groups or consolidates cards — and
approval-card.tsx, which PR #854 deliberately stayed out of for the same reason.API Or Behavior Changes
None on the wire. No route, no HTTP/GraphQL type, no Rust file, no
Cargo.lockline. Every newprop is optional and defaults to the pre-#883 render, so a console built from this against an older
host behaves exactly as before.
Behaviour:
#/approvals/<taskId>, the queue narrowed to that card, with Show all back.Tests
cargo fmt --all -- --check— N/A: no Rust file is touched.git diff --name-only upstream/mainis seven paths, all underfrontend/.cargo clippy --all-targets -- -D warnings— N/A: no Rust file is touched, and no featuregate is involved, so neither the default nor the
openhuman,tinycortexlane has anything tolint here.
cargo build --all-targets— N/A: no Rust file is touched;git diff --stat Cargo.lockisempty.
cargo test— N/A as above. Ran instead, as the console's CI lane does:npm run typecheck(tsc -b --noEmit) — cleannpm run typecheck:unit— cleannpm run typecheck:e2e— cleannpm run build— cleannpx vitest run— 828 passed, 79 files, 0 failed. No pre-existing failures in this repo;13 of those tests are new.
Not run:
prettier. CI does not run it, and untouched files (src/views/InboxView.tsx,src/hooks/use-company.ts) failprettier --checkonmain— so running it would have producedreformatting noise across files this change has nothing to do with.
Every new test was proven to fail with its fix reverted — 12 of 13
Each fix reverted individually, the suite re-run, the named test observed failing:
sincetakes the newest park instead of the oldestanchors the wait to the oldest park, whatever order the feed is insortis droppedanchors the wait to the oldest park…,orders the approvals oldest park firstapprovalsForTaskstops filtering on the task linkapprovalsForTaskcases, bothtaskApprovalBlockownership cases,is not blocked by another card's approvals(6 in total)BlockedRowis not renderednames the one call it is blocked on…,counts them when there is more than one…disabled={block !== null}→disabled={false}disables Resume, so the re-dispatch loop cannot be started from heretitlereason is removedstill shows the Resume button, with the reason on itThe 13th is a control case —
renders no blocked row and an enabled Resume, the unblocked path —and has no meaningful revert: any change that makes it fail also fails one of the tests above.
Recorded rather than counted as coverage.
One test file earns an exception to
vitest.config.ts's pure-functions rule, on the same groundsapproval-batch-card.test.tsearns it: the claim under test is "Resume is disabled", which existsonly at the rendered button.
taskApprovalBlockis unit-tested next door and decides whether thecard is blocked; it cannot reach whether the button the operator's hand lands on is actually stopped,
and that is the half that breaks the loop.
TaskItemis exported for it, with the reason recorded atthe export.
What is NOT proven, stated rather than claimed as coverage
against the
marketingstaging tenant. The claims are from the code and the suite.approvalsForTask, thesame one the board uses, deliberately not a second copy — is unit-tested and revert-proven, but the
filter banner, the Show all link and the
ClearedForTaskstate are JSX with no component-testharness in this repo for that view. Same gap PR fix(workflows): an approval must not repeat an outward call, finish a run that is waiting, or hide what it is approving (#846) #854 recorded for
RunHistoryPanel's branch.(a live host plus the mock brain), which this change did not add a spec to.
Documentation
No spec doc changes. The reasoning lives at the code it governs:
frontend/src/lib/task-approvals.tscarries the ownership rule, why the two skipped link states are skipped, and why the host's
attempt-level key is out of reach;
TasksView'sBlockedRowcarries the disabled-not-hiddenargument and the
approvalActionreuse.Closes #883