Skip to content

fix(hitl): the attention card offers one button per terminal option (V42) - #6280

Open
liususan091219 wants to merge 2 commits into
mainfrom
fix/v42-card-options-match-terminal
Open

liususan091219 wants to merge 2 commits into
mainfrom
fix/v42-card-options-match-terminal

Conversation

@liususan091219

Copy link
Copy Markdown
Collaborator

What the owner saw

A terminal offering three options, and a "needs your attention" card offering two — the first missing, the rest renumbered.

Two independent causes

Both measured against the real Claude Code permission dialog, not a hypothetical:

1. A wrapped option label split the block. option_blocks() ended a block at any non-option line, so a hard-wrapped label broke the run in two. parse_options() then returned only the trailing block, dropping every option before the wrap:

  1. Yes, and allow this command for the rest of
     this session          <- ends the block here
  2. Yes
  3. No, and tell Claude what to do differently

→ before: ['Yes', 'No, and tell Claude what to do differently'] · after: all three, label rejoined.

2. Any yes+no dialog collapsed to two buttons. requirement_for() rendered No/Yes whenever it found a yes and a no, regardless of what else was offered. Claude Code's actual permission dialog has three — Yes, Yes, and don't ask again…, No — so the middle option had no button, and no keystroke could reach it.

A dialog that genuinely offers two still renders No/Yes. Three or more now render one button per option in the terminal's own order, and keys_for() walks the caret to the clicked row.

Reviewer note — this reverses a prior deliberate choice

test_a_permission_dialog_is_a_no_yes_card_in_the_owners_words pinned the No/Yes shape for the 3-option dialog, by earlier owner preference. V36/V42 (owner, 2026-11-06) supersede it for 3+ options. I replaced that test rather than deleting it, and added a new one pinning the two-option case it was protecting, so the old intent is still covered where it still applies.

Regression evidence

Regression test: tests/hitl-tui-gate.test.py

Before fix: FAIL — reverting the collapse guard alone leaves 8 failures; reverting the wrap fix alone leaves 3. Run: python3 tests/hitl-tui-gate.test.py

After fix: PASS — 37/37. Same command.

Each fix was reverted independently to confirm both are load-bearing rather than one masking the other.

Not in this PR

V36 (card copy) and V44 (one status after resolve) are separate — V44 overlaps the web-side card owned by another agent and is waiting on that coordination.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ByiYdfob1ZYnq5PHYxo8qb

…V42)

The owner saw a terminal with three options and a card with two, the first
missing and the rest renumbered. Two independent causes, both measured:

1. option_blocks() ended a block at any non-option line, so a hard-wrapped
   option label split the run. parse_options() then returned only the trailing
   block: the options before the wrap vanished and the rest were renumbered.

2. requirement_for() collapsed any dialog containing a yes and a no into a
   two-button No/Yes card, whatever else it offered. Claude Code's real
   permission dialog has three ("Yes", "Yes, and don't ask again...", "No"),
   so the middle option had no button and no keystroke could reach it.

A dialog that genuinely offers two still renders No/Yes; three or more now
render one button per option, in the terminal's own order, and keys_for()
walks the caret to the row that was clicked.

This supersedes the earlier no-yes-in-the-owner's-words shape for 3+ option
dialogs. That was a deliberate choice, so the test that pinned it is replaced
rather than deleted, and the two-option case it was protecting is now pinned
by its own test.

Verified by reverting each fix alone against the suite: the collapse guard
alone accounts for 8 failures, the wrap fix for 3; together 37/37 pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ByiYdfob1ZYnq5PHYxo8qb
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Susan Xueqing Liu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

…(V42)

CI caught what my first sweep missed: two more suites build cards from the same
three-option permission fixture, so they still expected the No/Yes collapse.

- core-input-watch.test.py: the styled (SGR) caret-row dialog is the same three
  options, so it is a choice now. Its real subject — that the styled caret row
  stays dialog evidence, or every click goes stale — is kept and widened: the
  middle option is now clicked too.
- core-input-watch-chat-escalation.test.py: 2 failures and 7 errors, the errors
  from clicks naming `allow`/`deny`, which the card no longer carries. Those
  tests are about driving a click into the pane (partial sends, staleness,
  per-session routing), not about labels, so they now click opt1/opt3 and keep
  asserting the same keystrokes.

The two fixtures that build their own Action("allow", ...) are left alone: they
never came from a parsed dialog.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ByiYdfob1ZYnq5PHYxo8qb
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 93%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/hitl/tui_gate.py (100%)

Summary

  • Total: 8 lines
  • Missing: 0 lines
  • Coverage: 100%

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants