Skip to content

refactor(cosh): unify escape key handling in appcontainer#437

Open
kongld-mq wants to merge 4 commits into
alibaba:mainfrom
kongld-mq:feat/cosh/unify-escape_key
Open

refactor(cosh): unify escape key handling in appcontainer#437
kongld-mq wants to merge 4 commits into
alibaba:mainfrom
kongld-mq:feat/cosh/unify-escape_key

Conversation

@kongld-mq
Copy link
Copy Markdown
Collaborator

@kongld-mq kongld-mq commented May 6, 2026

Description

Consolidate Escape key behavior to improve UX and prevent state conflicts:

  • Move Escape handling from useGeminiStream.ts to AppContainer.tsx
  • Remove ESC double-press clear logic from InputPrompt.tsx (preserve only specific context handling: search, completions, shell mode)
  • Remove onEscapePromptChange callback mechanism (no longer needed since AppContainer directly manages state)
  • Implement unified two-tier behavior in AppContainer:
    • Input with content: double-press Escape to clear input content
    • Empty input + responding: single-press immediately cancels ongoing request
  • Preserve embeddedShellFocused check to allow embedded shells to handle their own Escape behavior
  • Update tests to call cancelOngoingRequest directly instead of simulating keypress events

This refactoring resolves state conflicts caused by having two independent ESC handling systems (escapePressedOnce in AppContainer and escPressCount in InputPrompt). All ESC double-press clear logic is now centralized in AppContainer, providing a cleaner architecture and more predictable behavior.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional change)
  • Performance improvement
  • CI/CD or build changes

Scope

  • cosh (copilot-shell)
  • sec-core (agent-sec-core)
  • skill (os-skills)
  • sight (agentsight)
  • tokenless (tokenless)
  • Multiple / Project-wide

Checklist

  • I have read the Contributing Guide
  • My code follows the project's code style
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly
  • For cosh: Lint passes, type check passes, and tests pass
  • For sec-core (Rust): cargo clippy -- -D warnings and cargo fmt --check pass
  • For sec-core (Python): Ruff format and pytest pass
  • For skill: Skill directory structure is valid and shell scripts pass syntax check
  • For sight: cargo clippy -- -D warnings and cargo fmt --check pass
  • For tokenless: cargo clippy -- -D warnings and cargo fmt --check pass
  • Lock files are up to date (package-lock.json / Cargo.lock)

Testing

cd src/copilot-shell,npm run format,npm run lint,npm run test

有输入时按 ESC 的行为

步骤 操作 预期结果
1 发送请求等待响应开始 正在响应状态
2 输入文本如 "hello world" 输入框显示文本
3 按 ESC 第一次 底部显示 "再次按 Esc 清除",已发起的请求未取消
4 500ms 内按 ESC 第二次 输入清空,提示消失,已发起的请求未取消
5 等待超过 500ms 后再按 ESC 提示消失,内容保留,重新开始双击流程

无输入时按 ESC 的行为

步骤 操作 预期结果
1 发送请求等待响应开始 正在响应状态
2 确保输入框为空 输入框无内容
3 按 ESC 单击即可取消请求
4 输入框为空且非响应状态 按 ESC 无操作

Additional Notes

Signed-off-by: konglidong <kld02270359@alibaba-inc.com>
@gemini-code-assist
Copy link
Copy Markdown

Important

Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services.

@github-actions github-actions Bot added the component:cosh src/copilot-shell/ label May 6, 2026
Copy link
Copy Markdown
Collaborator

@samchu-zsl samchu-zsl left a comment

Choose a reason for hiding this comment

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

Summary

重构方向正确(把分散的 ESC 处理集中到 AppContainer),但实现存在一个阻塞性缺陷:useKeypress 基于 KeypressContext 广播订阅(for (const handler of subscribers) handler(key),无 stopPropagation),ESC 会被 AppContainer 与 InputPrompt 两个订阅者并行处理。InputPrompt 在 shellMode / completion / shellCompletion / reverseSearch / commandSearch 等特定上下文分支里 return,只是结束自身 handler,并不能阻止 AppContainer 同一事件里看到 buffer.text.length > 0 而进入 double-press clear 流程。结果用户在这些场景按一次 ESC 退出 context 后,会看到「Press Esc again to clear」提示,再按一次 ESC 时输入被误清空。

🔴 Blockers

  • packages/cli/src/ui/AppContainer.tsx:1287-1338 — 新 ESC handler 未感知 InputPrompt 的特殊 context 状态,双订阅者冲突导致退出 shellMode / 关闭 completion / 取消 reverseSearch 后再按 ESC 会误清空输入。

🟡 Major

  • packages/cli/src/ui/AppContainer.tsx:1315 — PR description 写「500ms 内按 ESC 第二次」,但实际 CTRL_EXIT_PROMPT_DURATION_MS = 1000,双击窗口扩大了一倍,与文档预期不符。
  • packages/cli/src/ui/components/InputPrompt.test.tsx:1629 — 移除了 InputPrompt 侧两个双击清空测试,但 AppContainer 侧未补回等效单测;新逻辑(双击清空 / 空 buffer 单击取消 / timer 超时 / embeddedShellFocused 跳过)全部未覆盖。

验证依据

  • packages/cli/src/ui/contexts/KeypressContext.tsx:385-387:按键事件以 for (const handler of subscribers) 方式广播到所有订阅者,handler 返回值不被消费,不存在 stopPropagation 语义
  • packages/cli/src/ui/hooks/useKeypress.ts:订阅/取消订阅,纯 subscriber 模式。
  • packages/cli/src/ui/AppContainer.tsx:1419useKeypress(handleGlobalKeypress, { isActive: true }) — AppContainer 始终激活。
  • packages/cli/src/ui/components/InputPrompt.tsx:745useKeypress(handleInput, { isActive: !isEmbeddedShellFocused }) — 非嵌入 shell 时激活;与 AppContainer 在普通输入场景下并行。
  • packages/cli/src/ui/components/InputPrompt.tsx:388-424:ESC 分支 reverseSearch / commandSearch / shellMode / shellCompletion / completion 只 return,未、也无法阻止 AppContainer 的 handler 执行。

建议修复方向

  1. 把 InputPrompt 内剩余的 ESC 特殊 context 处理一并迁到 AppContainer(通过 UIState/UIActions context 暴露 reverseSearchActive / commandSearchActive / completion.showSuggestions / shellCompletion.showSuggestions 及对应 reset 动作),让 AppContainer 的 ESC handler 先判断是否命中特殊 context,命中则调用对应 reset 并 return,否则走统一双层语义。
  2. 或反向:在 AppContainer 的 ESC handler 起始处读取这些 uiState 标志,命中任一则直接 return,把这些场景的处理权显式让回 InputPrompt。
  3. 任选其一后补齐 AppContainer 的 ESC 单测,并对齐 PR description 的双击窗口(500 vs 1000ms,二选一)。

Scope 说明

变更范围与 PR title 一致,均集中在 packages/cli/src/ui/ 下 7 个文件;无夹带改动。CHANGELOG / PR description Type of Change 等工程纪律项已按作者约定豁免,不在本次评论中提出。

Comment thread src/copilot-shell/packages/cli/src/ui/AppContainer.tsx
Comment thread src/copilot-shell/packages/cli/src/ui/AppContainer.tsx Outdated
Comment thread src/copilot-shell/packages/cli/src/ui/components/InputPrompt.test.tsx Outdated
@kongld-mq kongld-mq force-pushed the feat/cosh/unify-escape_key branch from 51ccc8c to 83642db Compare May 13, 2026 06:13
kongld-mq added 3 commits May 13, 2026 17:20
…rchitecture

- UIStateContext.tsx: add reverseSearchActive, commandSearchActive, completionShowSuggestions, shellCompletionShowSuggestions
- UIActionsContext.tsx: add set/reset/register functions for state management
- AppContainer.tsx: unified ESC handler for special context states
- InputPrompt.tsx: consume state from UIState/UIActions context, remove local ESC handler, register reset callbacks
- InputPrompt.test.tsx: update mocks, simplify ESC-related tests
- test-utils/render.tsx: add UIStateContext/UIActionsContext mock providers

ESC handler priority order: reverseSearch → commandSearch → shellMode → completion → shellCompletion → double-press clear

Resolves ESC multiple-trigger issue caused by KeypressContext broadcast mechanism
…riginal design

- Introduce ESC_DOUBLE_PRESS_WINDOW_MS constant (500ms) separate from CTRL_EXIT_PROMPT_DURATION_MS (1000ms)
- Export constants for test verification
- Add tests to verify ESC window stays at 500ms, preventing accidental regression to 1000ms

The ESC double press window should be 500ms per qwen-code original design, not 1000ms like Ctrl+C/D exit prompt.
This fixes the inconsistency between PR description (500ms) and actual implementation (1000ms).
Add test coverage for ESC double-press clear behavior:
1. Verify ESC_DOUBLE_PRESS_WINDOW_MS is 500ms (matching qwen-code original design)
2. Document expected ESC handler behavior:
   - First ESC: show prompt, set escapePressedOnce, start timer
   - Second ESC within window: clear buffer, reset state
   - ESC after timeout: restart first-press flow
   - ESC with empty buffer + streaming: cancel ongoing request
   - ESC with embeddedShellFocused: no action
   - ESC with special contexts (shellMode/reverseSearch/completion): handle context first, not double-press clear
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:cosh src/copilot-shell/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants