feat(dashboard): show what a session is waiting for, not just that it is (DRC-4015) - #124
Merged
Conversation
… is (DRC-4015)
Knowing a session is blocked is worth less than knowing what on. "Force push to
main?" is a decision a person can make from the board. "open question
(AskUserQuestion)" is a reason to go and look, which is the trip the row exists
to save.
Two changes, because the board note's "render what is already parsed" was half
right and the wrong half is where the work was.
The parse threw the text away. A pending input tool was kept as {name, ts}, and
the question lives in the tool_use block's `input`, which was dropped. It is now
summarised at parse time rather than kept raw: `ExitPlanMode` carries a whole
plan, sometimes thousands of words, and a plan has no business in a session row
or in the caches a row is built from. A plan becomes its first line, which is its
own title in practice; a question becomes the first question with a count of the
rest. Both bounded, both scrubbed, because a transcript is a file Cargento does
not write and this text lands in the DOM.
An agreeing overlay then blanked it anyway. No overlay constructor sets `detail`,
so every needs-input patch carried None and applying it wrote that over whatever
the collector had found. On a default install, with the bundled hook present,
that happened every time. The collector's detail now survives, narrowly: only
when the row was already Needs input and stays Needs input, which is the case
where the overlay agrees about the state and contradicts nothing.
Working and Idle still clear the field. A working detail such as `running Bash`
is true of a running session and false of one stopped at a gate, so it must not
follow the row into a wait, and an answered question must not outlive the overlay
that retired it, which is DRC-4095 and DRC-4097.
The text still comes from the transcript alone, so it is there when the record
reached disk and absent when it did not (N-4). The row says a question is open
either way and can only sometimes say which. SKILL.md says so rather than
smoothing it over: a field that is usually present teaches a reader to trust it
and then fails silently on the sessions that matter.
Rationale in docs/design-needs-input.md (N-7).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Kent Huang <kent@infuseai.io>
Contributor
CoverageThreshold: |
There was a problem hiding this comment.
Pull request overview
This PR improves the dashboard’s “Needs input” rows by showing what a session is waiting for (when available from the transcript), and by preventing an agreeing overlay from erasing that detail.
Changes:
- Parse and persist a bounded, scrubbed
askssummary for pendingAskUserQuestion/ExitPlanModetool uses (rather than discarding tool input). - Render the row using the summarized question/plan title when present, falling back to the tool name when not.
- Preserve an existing
state_detailwhen an overlay keeps a session inneeds_inputbut carriesdetail=None, preventing accidental blanking.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/design-needs-input.md | Documents the two-part change (parse-time summarization + overlay non-blanking) and the intentional boundary behavior. |
| cargento/skills/cargento/tests/test_observation.py | Adds regression tests ensuring agreeing needs-input overlays do not wipe collector-provided wait detail. |
| cargento/skills/cargento/tests/test_claude.py | Adds tests for input_summary shape handling, caps, scrubbing, and end-to-end transcript carry-through. |
| cargento/skills/cargento/SKILL.md | Updates user-facing documentation to reflect that the row may show the question/plan title when present on disk. |
| cargento/skills/cargento/cargento_runtime/config.py | Introduces input_summary_cap_chars to bound summarized wait-detail text. |
| cargento/skills/cargento/cargento_runtime/collectors/claude.py | Renders needs-input detail using asks when present, otherwise falls back to the prior tool-name-based phrasing. |
| cargento/skills/cargento/cargento_runtime/claude_data.py | Implements input_summary and stores asks in pending_input_tool during transcript analysis. |
| cargento/skills/cargento/cargento_runtime/aggregate.py | Applies _keep_wait_detail to prevent overlays from blanking an existing wait detail when state remains needs_input. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…what is unmeasured Two review findings, both about the writing outrunning the evidence rather than about the code. N-7 documented one reason the question can be missing, the record not having reached disk, and missed the ordinary one. For up to `overlay_working_ttl_sec` after a turn starts, a live working overlay overrules the collector outright: the row reads Working, the detail is cleared, and the question is invisible even though the parse has it. `_keep_wait_detail` cannot help, because it only holds when the patch agrees the row is waiting. N-7's own argument about a field that is usually there applies to that window word for word, so it now names it, and points at N-6's recorder as the thing that counts it. SKILL.md gains the same caveat. "On a default install the erasure happened every time" was asserted. It needs an `input_requested` overlay, which only `PermissionRequest` mints, and `captures/` holds no Claude `PermissionRequest` record: the only capture of that name is Codex's, and it is a negative result. Whether the hook fires for `AskUserQuestion` and `ExitPlanMode` is an adapter claim this repository requires a capture for, so the paragraph now says the erasure is reachable and its frequency unknown, and names what would settle it. Four smaller things from the same pass: `safe_text` stripped C0 and DEL and passed the zero-width, bidi-embedding and isolate ranges. Those reorder how the text after them renders, so a record could make a row read as something it does not say. Shared function, so this covers every untrusted string on a row, not only the new one. A plan now summarises to its first *usable* line. A plan opening with a blank line, a fence or a bullet gave "```, waiting 2m", which is a worse row than the tool's name it replaced. The seam between the parse and the row had no test: `InputSummaryTest` stopped at `analyze_transcript` and `WaitDetailTest` started at hand-built rows. Two collector-level tests now pin both readings, the question present and absent. SECURITY.md's rationale for keeping `state_detail` out of dispute records said it can carry a permission prompt's text. That is now also an open question's and a plan's first line, which makes the exclusion better justified rather than different. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Kent Huang <kent@infuseai.io>
The scrub I widened a commit ago took U+200B to U+200F as a range, which sweeps in ZWNJ and ZWJ. ZWNJ is orthographic in Persian and several Indic scripts and sits inside words; ZWJ is what composes an emoji sequence. The substitution replaces with a space, so a Persian title would have been broken mid-word. This is a shared function, so the blast radius was every untrusted string on a row: titles, prompts, model names, not only the question this branch adds. My own check passed because CJK does not use joiners. Neither joiner can reorder text, so keeping them costs no protection. The spoofing vector is the directional marks and the embedding, override and isolate ranges, and those stay covered. The range is now listed character by character across that span with the reason written beside it, since a range is what caused this. Tests both ways: what must be stripped, and what must survive. Escapes rather than literals in the fixture, for the same reason ruff bans literals in source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Kent Huang <kent@infuseai.io>
kentwelcome
added a commit
that referenced
this pull request
Aug 18, 2026
* feat(dashboard): batch the gates into one queue (DRC-4018) Attention has a fixed cost per switch, so three separate pings cost more than one list of three. The blocked sessions were already a list on screen. What they were not was a queue: the band rendered them in session id order, which is an arbitrary order to be stopped in, and gave you nothing to act with — you read a session id off the screen by eye and went hunting for its terminal. Four changes, and none of them is new data. Order. Needs-input rows are published longest-blocked first. The gate that has held you up longest is the one still costing something and the one you are least likely to have remembered; the one that just opened is the one you already know about. It ranks on `blocked_since`, a fixed timestamp rather than an elapsed time, so the queue does not reshuffle under you while every row in it waits longer. Calm's `attention` ordering makes the same call for the same rows — its needs group was newest-first, which is right for a row you are watching and wrong for one waiting on you. `recent` is untouched and still genuinely newest-first for every state. Position. Rows are numbered from the top and the band head says how many are waiting, so the list reads as a queue and you can watch it shrink. A handle. Each row carries `copy id`, the control calm's rows already had, routed through the same action channel so there is one copy implementation rather than two that report success differently. A pass. `j`/`k` step the queue, Enter takes the row you are on, and `g` reaches the queue from either mode. Nothing marks a gate handled: Cargento does not write to a session, and a mark it did not measure would hide a gate that is still open. You answer in the terminal, the server stops calling the session blocked, the row leaves on the next refresh and the cursor inherits the new head. The queue clears itself out of measurements. One thing that is not interface. DRC-4015 found the per-row text reaches disk only sometimes. A row without it used to render an empty div — the same blank a row with nothing to add would leave — so the one row you cannot triage was also the one row that looked ordinary. It now says so, in both views. Scope note: D7 (DRC-4030) stays separate. This orders the gate queue; ranking working cards and the idle list against each other is a different question. Signed-off-by: Kent Huang <kent@infuseai.io> * fix(dashboard): review findings on the gate queue (DRC-4018) Two of these were bugs, and both were bugs of the same kind: a binding that looked right in calm mode and was wrong in the view it was copied into. The regular view took Space and the arrow keys. Calm can bind those because its ledger scrolls inside its own frame under its own cursor; the regular view is an ordinary long page, so preventDefault there removed paging and line-scrolling — and removed them only while something was blocked, since the branch returns early on an empty queue. Scroll keys that work or not depending on the payload are worse than no bindings. `j`/`k` and Enter only, now. `g` into calm narrowed to the blocked rows without setting the ordering. From `recent` that renders the queue exactly backwards with the cursor parked on the last row; from `fastest` it files the gates under "not working now". SKILL.md asserted both modes show the same queue in the same order, which was true only under `attention`. It sets the ordering too, so the doc is now true as written. Also from the review: The band's `copy id` dropped keyboard focus on the render it triggered. Calm solves this with calmFocusKey/calmRestoreFocus; the regular path called neither, so activating the button from the keyboard sent focus to <body> and the next Tab restarted at the top of the document. Calm's `attention` ranked the queue on `waitSec`, the rendered elapsed time, which floors at zero. Two rows carrying implausibly future stamps would tie there while the server still separated them — the one way the two views could name a different gate at the head. It ranks on the raw timestamp now, as aggregate.py does. `gateCursorKey` outlived an emptied queue, so a session that blocked, was answered, and blocked again later inherited a cursor that belonged on whichever gate had waited longest. test_the_order_does_not_move_as_the_wait_lengthens could not fail: sorted() over one list with one key is deterministic by construction, and flipping the key's sign left it green. Replaced with an assertion on the key itself. Two gaps closed: no test clicked the band's copy button, so the data-calm reuse — the riskiest part — was only ever exercised through the keyboard, which does not go through the global listener; and nothing covered the focus hand-off. Both new tests were mutation-checked against the un-fixed code. Docs: the calm-mode controls paragraph had ended up inside the gate-queue section though it owns sorting, filtering and the `u` key, so it moves back under Display modes. The queue's key list said `⏎` copies, which is regular mode only — in calm it expands. The "nothing marks a gate handled" argument was being restated in five places; it moves to docs/design-needs-input.md as N-8, which already owns the needs-input reasoning, and the comments become pointers. Comment trimming throughout, on the same standard: the paragraph in main.js restated the three lines under it, and row_order carried an 18-line docstring over a 4-line function while leaving the one branch a reader would stop at — the last_activity fallback — unexplained. Signed-off-by: Kent Huang <kent@infuseai.io> * docs(sync): reconcile the docs with the gate queue (DRC-4018) The module ownership table in the architecture doc did not say where the queue lives. It is split three ways on purpose: the queue itself is shared page state because both views read it, the band's cursor belongs to the regular view, and the listeners that route both views' controls have always been in calm.js and are the page's rather than calm's. A reader checking the inward-only rule against that table would have read the last one as drift. The N-8 entry added in the previous commit carried two em dashes, which the voice standard bans in docs/design-*.md. Nothing in CI checks that. The sync marker was stamped at 8f859f8 and four PRs have merged since: #121 through #124. Their docs are current, so this is the orphaned-marker failure the skill warns about rather than a real backlog, and the new marker is the merge base so the next squash cannot orphan it again. Signed-off-by: Kent Huang <kent@infuseai.io> --------- Signed-off-by: Kent Huang <kent@infuseai.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Real output from the parse-render-overlay path end to end, not a mockup.
Knowing a session is blocked is worth less than knowing what on. The first line is a reason to go and look, which is the trip the row exists to save.
Two changes, because the ticket's premise was half wrong
The board note called this "render what is already parsed". The audit on the issue found otherwise, and both halves needed doing.
The parse threw the text away. A pending input tool was kept as
{name, ts}; the question lives in thetool_useblock'sinput, dropped on the floor. Now summarised at parse time rather than kept raw, which is the deliberate part:ExitPlanModecarries a whole plan, sometimes thousands of words, and a plan has no business in a session row or in the caches a row is built from.AskUserQuestion(+N more)ExitPlanModeBoth bounded (
input_summary_cap_chars, 160) and scrubbed throughrecords.safe_text, because a transcript is a file Cargento does not write and this text lands in the DOM. There is a test for control characters specifically.An agreeing overlay then blanked it anyway. No overlay constructor sets
detail, so every needs-input patch carriedNoneandapply_patchwrote that over whatever the collector had found. On a default install, with the bundled hook present, that erasure happened every time.The fix is narrow on purpose. The collector's detail survives only when the row was already Needs input and stays Needs input, which is the case where the overlay agrees about the state and contradicts nothing.
What must keep clearing, and why
Working and Idle still blank the field, and that is not an oversight:
running Bashis true of a session that is running and false of one stopped at a gate, so a working detail must not follow the row into a wait;Three of the four new tests in
WaitDetailTestpin those boundaries rather than the feature. They pass with the fix reverted, deliberately: they exist to stop the rule widening later, and the fourth is the one that fails without it.What this does not do
The text still comes from the transcript alone, so it is there when the record reached disk and absent when it did not (N-4 measured that as neither guaranteed nor rate-known). The row says a question is open either way and can only sometimes say which.
SKILL.mdsays so rather than smoothing it over. A field that is usually present teaches a reader to trust it and then fails silently on exactly the sessions that matter.Making it reliable means sourcing the text from the event path, and that is not a rendering change: the envelope drops the tool name and tool input at the hook, deliberately, and
SECURITY.mdstates that as a property. Reopening it is a security decision and is step 3 on the issue, not this PR.Test plan
AGENTS.md§ Pre-PR Checks against a venv pinned torequirements-dev.txt, cleanfail_underthresholdWaitDetailTestfails with the fix reverted; the other three are boundary pinsruntime-floorjob covers the floor/sync-docs; N-7 added,SKILL.mdupdated in two placesgit commit -s, DCO)Closes DRC-4015