Repository navigation
fix(conditions): put a refused keystroke where a modal cannot cover it - #1110
Conversation
#1070 moved a refused STRUCTURED condition answer onto the global toast, because every condition that can refuse one is a Radix modal and the pane toast is an in-flow sibling with no z-index, painted under a 1100-z 85%-opaque scrim inside the subtree Radix marks aria-hidden. The keystroke arm was left on the pane toast. That is worse than either surface on its own: one click produced a readable message or an invisible one depending on which arm handled it — and the keystroke arm is the one a TRUST DIALOG uses, which is the case where a modal is guaranteed to be up. Both refusals in `sendConditionKey` now go to the global toast at the same 6000ms the refusal reporter uses, because both ask the user to look at something and decide, which the 2500ms default does not allow. The vanished-prompt branch moves too, though its modal has usually unmounted by then and the pane toast would often have been visible. A single click landing on two different surfaces depending on which check failed first is the inconsistency; keeping one of them where it happened to work is not a saving. Three tests, all failing first: a refused keystroke is readable and is NOT also sent to the pane toast (a message sent to both would pass a text-only assertion), a vanished prompt lands on the same surface, and a keystroke that landed stays silent. Three mutations measured individually, all killed: each message routed back to the pane toast, and the refusal branch inverted. Closes #711 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on #1110. The WHY block said "every condition that reaches this arm is a Radix modal". That is false and the review proved it: `codex.approval` and `claude.resume-prompt` are in-flow strips with no scrim, and there is a fifth entry point with no condition surface at all — `useComposerKeybinds` routes composer ArrowUp/Down here whenever `hasActionCondition` is true. The comment enumerated two of four kinds and called that "every". The conclusion survives, for a reason now written down: the modal cases force the move (and a modal from a RETAINED pane portals to `body` while its pane toast sits inside a `display: none` subtree, so it is invisible outright), while for the strip cases the global toast is the lesser fit and that cost is accepted rather than branching the surface on layout — which would put the same click on two surfaces again. Every path here is focus-safe, so the toast always describes the pane the user is looking at; the review checked all three. Also corrected: the vanished-prompt branch is not "usually reached after the modal unmounted", it is not reachable at all today — the outlet is fed `normalizedConditions` and renders nothing for a null snapshot. Moving it costs nothing, which makes the consistency argument cheap and correct for a different reason than the one I gave. The 6000ms was unpinned: dropping both duration arguments passed all 1013 renderer tests. The comment calls that duration load-bearing, so it now has the same fake-timer test the sibling reporter has (still there at 4s, gone by 6.1s), and the mutation dies 1/8. Added the reporter's console breadcrumb to this arm — the toast carries no session identity, and in a 4-pane grid that leaves the user with an unattributed message. Two comments in dispatch.ts and dispatchRefusal.renderer.test.tsx still described this arm as showing a pane toast. Still true in substance, wrong about the surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review round one — all four findings fixed in Medium — the premise was false. I wrote "every condition that reaches this arm is a Radix modal: the trust dialogs and the permission prompt", having enumerated two of four kinds. The conclusion survives, and the reason is now written down rather than assumed: the modal cases force the move — including the case you found, a modal from a retained pane, where Radix portals to Low — the vanished-prompt branch. Your correction is better than my rationale: it is not "usually reached after the modal unmounted", it is not reachable at all today — the outlet is fed Low — the 6000ms was not pinned. Confirmed: dropping both duration arguments passed all 1013 renderer tests. That is a real gap given the comment calls the duration load-bearing, and the fix was four lines the file next door already had. Added, with the same fake-timer shape as the #1099 reporter (present at 4s, gone by 6.1s) — the mutation now dies 1/8. Low — two stale comments. Also taken, from your Medium finding: the toast carries no session identity, so I mirrored the reporter's Not taken, recorded so it is a decision and not an oversight: the residual split you named — Renderer project: 1014/1014. |
…usal-visible # Conflicts: # src/shared/conditions-core/dispatch.ts
Merging main broke every test in the file, not just the keystroke ones: #1177 renamed the outlet's pty callback from onSend to onPtyAction, so the mock's probe read undefined and mount's guard failed. The guard did its job; the probe now reads the real prop and adapts it back to a keystroke. Mutation-checked: routing a refused keystroke back to the pane toast turns 2 of the 8 tests red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes #711 (item 1). Found while auditing the open-issue table against main — this is the half of #1070 that was left behind, and it was left in the worse of the two states.
The inconsistency
#1070 (merged today as #1099) moved a refused structured condition answer onto
GlobalToastatz-[1200], because every condition that can refuse one is a Radix modal: the dialog overlay isz-[1100]and 85% opaque, and Radix marks the rest of the subtreearia-hidden. A pane toast is an in-flow sibling inside the pane with no z-index, so it was painted under the scrim, in the region screen readers are told to ignore.sendConditionKey— the keystroke arm — stayed on the pane toast. So the same click produced a readable message or an invisible one depending on which arm handled it. And the keystroke arm is the one a trust dialog uses, which is exactly the case where the modal is guaranteed to be up.Both of its refusals now go to the global toast, at the same 6000ms the refusal reporter uses — these ask the user to look at something and decide, which the 2500ms default does not allow.
Why the second branch moved too
The vanished-prompt branch (
'That prompt is no longer live.') fires when the condition cleared between paint and click. Its modal has usually unmounted by then, so the pane toast would often have been visible.It moved anyway. A single click landing on two different surfaces depending on which check failed first is the inconsistency; keeping one of them where it happens to work is not a saving, it is the bug in miniature.
Tests
Three, all failing first against the old code:
showPaneToastcall and requiring the array to be empty. A message sent to both surfaces would pass a text-only assertion, which is why the negative is asserted explicitly.The probe outlet from #1099 now hands back
onSendas well asonConditionRefused, so the realsendConditionKeyis exercised rather than a stand-in.Three mutations, each applied individually against a verified-clean tree, all killed:
if (ok))npx tsc -bclean on Node 24; renderer project 1013/1013.What is still open on #711
Item 2 of that issue is untouched and stays open: a condition click on a session with no live backend is still a silent no-op —
sendConditionKeydoes not callensureSessionLive, andSessionManager.write()returnsfalsebeforerecordInputWritewhen a delivery holds the write reservation or there is no registry entry. That is a different fix with a real design question behind it (the twofalsereasons are indistinguishable from the renderer), so it is not bundled here.🤖 Generated with Claude Code