Skip to content

fix: Escape key reliably interrupts agent chat sessions#130

Open
danshapiro wants to merge 6 commits intomainfrom
fix/escape-interrupt
Open

fix: Escape key reliably interrupts agent chat sessions#130
danshapiro wants to merge 6 commits intomainfrom
fix/escape-interrupt

Conversation

@danshapiro
Copy link
Owner

Summary

  • Moved the Escape-to-interrupt handler from the <textarea> onKeyDown in ChatComposer up to a container-level onKeyDown on the AgentChatView wrapper <div> with tabIndex={-1}
  • Escape now interrupts a running agent chat session regardless of which element inside the pane has focus (message area, code blocks, etc.)
  • Removed the now-redundant Escape handler from ChatComposer to avoid double-firing; Stop button still works via onInterrupt prop

Test plan

  • New tests in AgentChatView-interrupt.test.tsx verify container-level Escape sends sdk.interrupt when running, does not fire when idle, and ignores non-Escape keys
  • Removed 2 obsolete Escape tests from ChatComposer.test.tsx (7 tests remain, including Stop button click)
  • Full test suite: no new failures (all failures are pre-existing act() environment issue)
  • Lint: no new errors
  • Manual: open Freshclaude, send a prompt, click in the message area (away from textarea), press Escape — generation stops

🤖 Generated with Claude Code

Dan Shapiro and others added 4 commits February 28, 2026 11:06
…d picker pane warning

The skill previously said FRESHELL_TOKEN="<auth-token>" with no hint
where to find it — now reads it from .env where it actually lives.
Also documents picker panes as ephemeral and warns against targeting
them for layout mutations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… of focus

Move the Escape-to-interrupt handler from the <textarea> onKeyDown in
ChatComposer up to a container-level onKeyDown on the AgentChatView
wrapper <div> with tabIndex={-1}. This ensures Escape works regardless
of which element inside the agent-chat pane has focus.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Now that AgentChatView handles Escape at the container level, the
textarea-level Escape handler in ChatComposer is redundant. Remove it
to avoid double-firing. The Stop button still uses onInterrupt.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@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: c0df0adace

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +308 to +310
if (e.key === 'Escape' && isRunning) {
e.preventDefault()
handleInterrupt()

Choose a reason for hiding this comment

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

P2 Badge Ignore Escape from nested dialogs before interrupting session

This handler interrupts any running session for every bubbled Escape keydown inside the pane, including when the user is trying to dismiss the settings popover or close a select/menu. AgentChatSettings already binds Escape-to-close behavior, but the container onKeyDown runs earlier in the bubble path, so pressing Escape while settings are open will now also send sdk.interrupt and stop generation unexpectedly. Please scope this shortcut to contexts where Escape is intended to mean “interrupt” (e.g., composer/message area) or explicitly skip dialog/form controls.

Useful? React with 👍 / 👎.

Dan Shapiro and others added 2 commits February 28, 2026 19:14
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ailures

Root cause: Freshell runs as a production server (NODE_ENV=production).
When tests are invoked from within Freshell, Vitest inherits this env var.
React's index.js checks process.env.NODE_ENV at runtime — when it's
'production', it loads react.production.min.js which disables act(),
breaking all component tests (1048 failures).

Additionally, NODE_ENV=production causes npm install to skip devDependencies
in worktrees, leading to dual React instances (worktree has react but not
vitest/react-dom, which fall through to main repo's node_modules).

Fix: Override process.env.NODE_ENV from 'production' to 'test' at the
top of both vitest config files, before any imports resolve. This ensures
React loads its development build and npm install includes devDependencies.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant