Skip to content

fix(tui): escape during approval prompt denies instead of quitting - #1760

Merged
Aaronontheweb merged 5 commits into
devfrom
fix/1757-escape-denies-approval
Aug 5, 2026
Merged

fix(tui): escape during approval prompt denies instead of quitting#1760
Aaronontheweb merged 5 commits into
devfrom
fix/1757-escape-denies-approval

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Fixes #1757.

Problem

Pressing Escape while an approval prompt is up in netclaw chat calls RequestAppShutdown() and tears down the whole session. The pending interaction dies with the process — neither approved nor denied.

Fix

In ChatPage.HandleKeyPress, when HasPendingInteraction is true, Escape now routes to the first-class Deny option instead of quitting:

  • ChatViewModel.DenyPendingInteractionAsync() resolves the ApprovalOptionKeys.Deny option and submits it through the existing interaction-response path.
  • Bare Escape (no prompt, not generating) still quits, preserving the historical behavior.
  • Generation-cancel remains a separate TODO (unchanged).

This matches Termina's default "cancel" semantics and the convention every other NetClaw page follows (Escape = back/cancel, never quit).

Tests

  • Escape_WithPendingInteraction_DeniesInsteadOfQuitting — proves Escape submits deny and only the explicit Ctrl+Q shuts down (ordered event log).
  • Escape_WithNoPendingInteraction_StillQuits — bare Escape keeps quitting.
  • DenyPendingInteraction_NoDenyOption_DoesNotSubmit — a prompt without a deny option is a safe no-op, no quit, no bogus submit.

Full Netclaw.Cli.Tests suite: 1286/1286 passing. Build: 0 warnings, 0 errors.

Pressing Escape while an approval prompt is up called RequestAppShutdown()
and tore down the whole chat session, killing the pending interaction
without approving or denying it (#1757).

Escape now routes to the first-class Deny option when a pending
interaction exists, matching Termina's default cancel semantics and the
convention every other NetClaw page follows. Bare Escape only quits when
no prompt is active; generation-cancel remains a separate TODO.

Adds ChatViewModel.DenyPendingInteractionAsync() which resolves the
ApprovalOptionKeys.Deny option and submits it via the existing
interaction-response path. Regression tests cover: Escape denies + app
stays alive, bare Escape still quits, and a no-deny-option interaction
is a safe no-op.
Addresses the pr-review-specialist findings on #1760:

- Reorder HandleKeyPress so HasPendingInteraction is checked BEFORE
  IsGenerating. A pending approval prompt always wins over the
  generation-cancel TODO branch, closing the stale-flag race where
  Escape got swallowed and the user had to press it twice.
- Escape during generation now shows a status message instead of
  silently eating the key (generation cancel remains a separate TODO).
- Add an in-flight guard in SubmitInteractionSelectionAsync so
  back-to-back Escape + Enter can't dispatch two responses for the
  same interaction (daemon already nacks expired prompts; this is
  defensive).
- Narrow DenyPendingInteractionAsync from public to internal virtual.
- Strengthen the no-deny-option test with a deny:called lifecycle
  marker so it positively proves the no-op branch ran instead of
  passing vacuously. Add tests for the reorder and the generation
  status message (16 ChatPageTests total, 1288 CLI tests green).
@Aaronontheweb
Aaronontheweb force-pushed the fix/1757-escape-denies-approval branch from 4d77fd5 to 8f6774e Compare August 5, 2026 14:44
@Aaronontheweb

Copy link
Copy Markdown
Collaborator Author

Addressed the adversarial review findings in 8f6774e (rebased onto current dev):

Fixed in this PR:

  • Key-ordering race (MEDIUM 2): HasPendingInteraction is now checked before IsGenerating in HandleKeyPress — a pending prompt always wins over the generation-cancel TODO branch, so a stale generating flag can't swallow Escape. New test: Escape_WithPendingInteractionAndStaleGenerating_Denies.
  • Escape during generation (MEDIUM 1, TUI half): no longer a silent no-op — shows "Cancel generation is not supported yet." status message. New test: Escape_WhileGenerating_ShowsStatusInsteadOfQuitting. (Actual cancel capability is a daemon/API change — tracked separately.)
  • Double-submit guard (LOW 4, TUI half): _isSubmittingInteraction in-flight flag in SubmitInteractionSelectionAsync so back-to-back Escape + Enter can't send two responses for one interaction. Defensive — the daemon already nacks expired prompts.
  • Visibility (LOW 5): DenyPendingInteractionAsync narrowed to internal virtual.
  • No-deny test gap (LOW 6): the test VM now records a deny:called lifecycle marker, positively proving the no-op branch runs instead of passing vacuously.

Deferred as design work (daemon/API side, out of scope for a TUI fix): generation cancellation as a first-class API capability, interaction lifecycle as pushed client events, session-history semantics for cancelled turns — the substrate a future web/desktop client (NetClaw HTTP + WebSocket API) would build on.

Verification: 16/16 ChatPageTests, 1288/1288 full Netclaw.Cli.Tests, 0 warnings/0 errors.

@Aaronontheweb Aaronontheweb added tui Terminal UI (Termina) issues UX/DX UI / UX / DX friction issue or user-facing annoyances. labels Aug 5, 2026
Idle Escape no longer calls RequestAppShutdown(). Escape now means
cancel in every state: denies a pending approval, shows a status
message while generating, and is a no-op when idle. The status bar
already advertises [Ctrl+Q] Quit as the quit affordance.

Updates Escape_WithNoPendingInteraction_StillQuits to assert Escape
is a no-op and only an explicit Ctrl+Q shuts the app down.

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Aaronontheweb merged commit f7d868f into dev Aug 5, 2026
21 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/1757-escape-denies-approval branch August 5, 2026 17:51
@Aaronontheweb Aaronontheweb mentioned this pull request Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tui Terminal UI (Termina) issues UX/DX UI / UX / DX friction issue or user-facing annoyances.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Escape during approval prompt quits netclaw chat instead of denying

1 participant