Skip to content

fix(tui): correct Ctrl+C abort handling#44223

Open
Crpdim wants to merge 10 commits intoopenclaw:mainfrom
Crpdim:fit/tui_crtl_c
Open

fix(tui): correct Ctrl+C abort handling#44223
Crpdim wants to merge 10 commits intoopenclaw:mainfrom
Crpdim:fit/tui_crtl_c

Conversation

@Crpdim
Copy link
Copy Markdown
Contributor

@Crpdim Crpdim commented Mar 12, 2026

Summary

  • Problem: Ctrl+C in TUI only aborted active runs when the editor was empty, and abort handling could leave stale exit-confirmation state armed.
  • Why it matters: during streaming runs, Ctrl+C could clear the drafted next prompt instead of interrupting the run, or make a later single Ctrl+C exit unexpectedly.
  • What changed: TUI now prioritizes aborting active runs over clearing draft input, clears stale exit-confirmation state when aborting, and preserves partial assistant text on aborted runs.
  • What did NOT change (scope boundary): no gateway/protocol behavior changed; this is limited to TUI interaction handling and tests.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Closes #
  • Related #

User-visible / Behavior Changes

  • Ctrl+C now aborts an active TUI run even if the draft editor already contains the next prompt.
  • After aborting a run, exit confirmation is reset so a later single Ctrl+C shows the normal warning instead of exiting immediately.
  • Aborted runs keep any partial assistant text already streamed instead of clearing the response placeholder.

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: Linux
  • Runtime/container: local Node/Vitest
  • Model/provider: N/A
  • Integration/channel (if any): TUI
  • Relevant config (redacted): default local dev config

Steps

  1. Start openclaw tui and submit a prompt so the run enters running/streaming.
  2. Type a follow-up prompt while the current run is still active, then press Ctrl+C.
  3. After the abort completes, press Ctrl+C again once without an active run.

Expected

  • Step 2 aborts the active run and keeps the drafted follow-up text intact.
  • Step 3 only shows the exit warning; it does not exit immediately.

Actual

  • Before this fix, step 2 could clear the draft instead of aborting.
  • Before this fix, step 3 could exit immediately if a previous exit warning was still armed.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: exercised the TUI Ctrl+C decision logic with focused Vitest coverage for active-run abort, stale exit-confirmation reset, and partial-text preservation on abort.
  • Edge cases checked: active run with non-empty draft input; active run after a prior exit warning; aborted event with streamed partial assistant text.
  • What you did not verify: manual interactive TUI behavior against a live gateway session.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: revert commit 54241ad1c.
  • Files/config to restore: src/tui/tui.ts, src/tui/tui-event-handlers.ts, src/tui/tui.test.ts, src/tui/tui-event-handlers.test.ts
  • Known bad symptoms reviewers should watch for: Ctrl+C clearing draft input during active runs, or immediate TUI exit after a prior abort.

Risks and Mitigations

  • Risk: changing Ctrl+C precedence could alter existing editor-only expectations.
  • Mitigation: explicit unit coverage now locks the intended priority order for active run abort, draft clear, and exit confirmation.

@Crpdim Crpdim closed this Mar 12, 2026
@Crpdim Crpdim reopened this Mar 12, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 12, 2026 •
edited
Loading

Greptile Summary

This PR fixes three related Ctrl+C interaction bugs in the TUI: abort is now prioritized over draft-input clearing when a run is active, stale exit-confirmation state (lastCtrlCAt) is reset to zero on abort so a subsequent Ctrl+C won't exit immediately, and the aborted event handler now preserves any partial assistant text already streamed rather than discarding it.

Key changes across the six files:

  • tui.ts — resolveCtrlCAction gains an "abort" priority branch checked before "clear" or "warn"/"exit". A new abortPendingRunId variable tracks in-flight abort RPCs so a second Ctrl+C (while waiting for the server) correctly falls back to the normal clear/exit path. The activeChatRunId setter auto-clears the pending state when the run pointer changes.
  • tui-session-actions.ts — abortActive now captures runId before the await to avoid a race condition, handles the aborted: false (stale-run) response by resetting local state to "idle", and clears local state on RPC failure so Ctrl+C cannot loop forever.
  • tui-event-handlers.ts — The aborted state branch calls streamAssembler.finalize() before terminateRun, preserving partial streamed text.
  • Tests — All three changed behaviour scenarios are covered by new focused unit tests in tui.test.ts, tui-session-actions.test.ts, and tui-event-handlers.test.ts.

Confidence Score: 4/5

  • Safe to merge; the changes are limited to TUI interaction handling with no gateway/protocol impact and are well-covered by unit tests.
  • All three bug scenarios (abort priority, exit-confirmation reset, partial-text preservation) have explicit unit coverage and the logic is correct. The one caveat is that if the gateway confirms an abort RPC (aborted: true) but then fails to deliver the aborted event (e.g., a server-side bug), activeChatRunId and abortPendingRunId would remain set until a disconnect/reconnect clears them — leaving the user unable to trigger another abort. This is an unlikely edge case and is mitigated by disconnect handling, but it was not manually tested against a live gateway session (as the author noted).
  • No files require special attention; src/tui/tui.ts contains the most interconnected state logic and is worth a quick read-through.

Last reviewed commit: 44bb8db

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Mar 12, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54241ad1c8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/tui/tui.ts Outdated
@katoue

This comment was marked as spam.

Sign in to view
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Mar 12, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b2eaf9bfb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/tui/tui-session-actions.ts Outdated
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Mar 12, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61f18e1ee0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/tui/tui.ts Outdated
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed89a59095

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/tui/tui.ts Outdated
@Crpdim Crpdim closed this Mar 13, 2026
@Crpdim Crpdim reopened this Mar 13, 2026
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d142d19a2d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/tui/tui-session-actions.ts Outdated
@Crpdim Crpdim closed this Mar 13, 2026
@Crpdim Crpdim reopened this Mar 13, 2026
@BingqingLyu BingqingLyu mentioned this pull request Apr 27, 2026
Open
20 tasks
@openclaw-barnacle
Copy link
Copy Markdown

openclaw-barnacle Bot commented Apr 28, 2026

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Apr 28, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 28, 2026 •
edited
Loading

Codex review: found issues before merge.

Summary
The PR changes TUI Ctrl+C handling to abort active runs before clear/exit handling, manage pending/stale abort state, preserve aborted partial assistant text, and add focused tests.

Reproducibility: yes. Source inspection on current main shows Ctrl+C only evaluates editor text and exit timing, so with an active run and non-empty draft it takes the clear-input path instead of aborting.

Next step before merge
Maintainers need to choose the intended TUI Ctrl+C contract before automation patches docs/changelog or updates this branch.

Security
Cleared: The diff is limited to TUI interaction state and tests, with no new dependency, workflow, permission, secret-handling, package-resolution, or command-execution surface.

Review findings

  • [P2] Update the TUI shortcut docs — src/tui/tui.ts:292-305
  • [P2] Add the required changelog entry — src/tui/tui.ts:292-305
Review details

Best possible solution:

Maintain one explicit TUI shortcut contract: either land abort-first Ctrl+C with matching docs, changelog, and focused validation, or keep Ctrl+C as clear/exit and resolve the confusion through the hint-only path in #38501/#38502.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows Ctrl+C only evaluates editor text and exit timing, so with an active run and non-empty draft it takes the clear-input path instead of aborting.

Is this the best way to solve the issue?

Unclear. The patch is a narrow implementation if maintainers want Ctrl+C to abort active runs, but it is not clearly the best solution until the shortcut contract is chosen against the open hint-only alternative.

Full review comments:

  • [P2] Update the TUI shortcut docs — src/tui/tui.ts:292-305
    This changes Ctrl+C so an active run can be aborted before the clear/exit path, but docs/web/tui.md still documents Esc as the abort shortcut and Ctrl+C only as clear input / double-press exit. If this behavior lands, the public shortcut docs need the active-run exception.
    Confidence: 0.92
  • [P2] Add the required changelog entry — src/tui/tui.ts:292-305
    This is a user-visible TUI keyboard behavior change, but the PR file list does not include CHANGELOG.md. OpenClaw policy requires changelog coverage for user-facing fixes.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.86

What I checked:

  • current-main-ctrl-c-resolver: Current main's Ctrl+C resolver only accepts editor input/timing state and can return clear, warn, or exit; it has no active-run abort branch. (src/tui/tui.ts:272, a90be474f441)
  • current-main-keybinding-contract: Esc invokes abortActive, while Ctrl+C calls resolveCtrlCAction and then clears input, exits, or warns; Ctrl+C does not call abortActive on main. (src/tui/tui.ts:1058, a90be474f441)
  • public-docs-contract: The public TUI docs document Esc as the active-run abort shortcut and Ctrl+C as clear input / double-press exit, with no active-run abort exception. Public docs: docs/web/tui.md. (docs/web/tui.md:88, a90be474f441)
  • abort-rpc-stale-run-contract: chat.abort can return aborted:false when the supplied runId is not active, so stale local active-run handling is a real edge case for this PR's abort-first behavior. (src/gateway/server-methods/chat.ts:1839, a90be474f441)
  • current-main-aborted-partial-handling: Current main's aborted-event path adds a system message and terminates the run; terminateRun drops stream assembler state, so the PR's partial-text preservation is not implemented on main. (src/tui/tui-event-handlers.ts:441, a90be474f441)
  • pr-file-list-gap: The provided PR file list changes TUI source/tests only and does not include docs/web/tui.md or CHANGELOG.md, leaving user-facing shortcut docs and release notes out of sync if this behavior lands. (19ee68241369)

Likely related people:

  • vignesh07: Available history and prior ClawSweeper review context connect Vignesh to the Ctrl+C resolver/keybinding area and active-run TUI flow, including the current optimistic-message active-run work. (role: introduced behavior and adjacent TUI flow owner; confidence: high; commits: b4cdffc7a429, 61a0b0293104; files: src/tui/tui.ts, src/tui/tui.test.ts, src/tui/tui-command-handlers.ts)
  • advaitpaliwal: Introduced the Gateway/Control UI partial-output-on-abort behavior that this PR mirrors for TUI aborted runs. (role: adjacent abort-partial behavior owner; confidence: medium; commits: 14fb2c05b133; files: src/gateway/server-methods/chat.ts, docs/web/control-ui.md, docs/web/webchat.md)
  • katoue: Commented positively on this PR's Ctrl+C abort precedence and stale exit-state reset, making them useful for deciding the intended TUI shortcut contract. (role: reviewer and routing candidate; confidence: medium; files: src/tui/tui.ts, src/tui/tui-session-actions.ts)

Remaining risk / open question:

Codex review notes: model gpt-5.5, reasoning high; reviewed against a90be474f441.

@mwfj
Copy link
Copy Markdown

mwfj commented Apr 28, 2026

What is going on for this?

This was referenced Apr 28, 2026
Closed
Closed
Closed
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Apr 29, 2026
@RomneyDa RomneyDa mentioned this pull request May 4, 2026
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

size: M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Crpdim @katoue @mwfj