Skip to content

fix(ui): save notes from tmux Ctrl-S input#350

Merged
benvinegar merged 2 commits into
mainfrom
fix/tmux-ctrl-s-save-note
May 23, 2026
Merged

fix(ui): save notes from tmux Ctrl-S input#350
benvinegar merged 2 commits into
mainfrom
fix/tmux-ctrl-s-save-note

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • Normalize Ctrl-S note-saving input across raw control, CSI-u, and tmux control-mode encodings
  • Prevent handled save/cancel shortcut keys from reaching the focused textarea
  • Add regression coverage for CSI-u Ctrl-S input

Fixes #345

Testing

  • bun run typecheck
  • bun test
  • tmux smoke: raw C-s saved a draft note without inserting 0x13
  • tmux smoke: literal CSI-u ESC [ 115 ; 5 u saved a draft note without inserting 0x13 or 115;5u

This PR description was generated by Pi using OpenAI GPT-5 Codex

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 23, 2026

Greptile Summary

Fixes Ctrl-S note saving when tmux forwards keyboard input using CSI-u encoding (\u001b[115;5u) by extracting a isSaveDraftNoteKey helper that normalises all three encodings (raw \u0013, key.ctrl + name, and CSI-u), and by adding explicit preventDefault/stopPropagation calls so the event never leaks into the focused textarea.

  • keyboard.ts — new isSaveDraftNoteKey covers raw control, key.ctrl flag, and CSI-u sequence; also matches the literal string \"0x13\" via CTRL_S_HEX_LABEL which appears to be dead code.
  • AgentInlineNote.tsx + useAppKeyboardShortcuts.ts — both now consume the key on match, preventing stray characters from reaching the textarea.
  • AppHost.interactions.test.tsx — regression test added for the CSI-u path.

Confidence Score: 4/5

The change is safe to merge; the core CSI-u fix is correct and the new test exercises the added path end-to-end.

The CTRL_S_HEX_LABEL = "0x13" constant matches the 4-character text string "0x13" rather than any known terminal encoding, and the actual control character is already covered by CTRL_S. This is most likely dead code, but the misleading name and unexplained purpose leave a small gap in confidence.

src/ui/lib/keyboard.ts — the CTRL_S_HEX_LABEL clauses warrant a second look to confirm intent or remove them.

Important Files Changed

Filename Overview
src/ui/lib/keyboard.ts New isSaveDraftNoteKey correctly adds CSI-u detection, but also matches "0x13" as a literal string via CTRL_S_HEX_LABEL — the textual label of the control character, not any known terminal encoding — appears to be dead code or an accidental match.
src/ui/hooks/useAppKeyboardShortcuts.ts Extracts inline Ctrl-S check into isSaveDraftNoteKey, adds consumeKey helper, and applies preventDefault/stopPropagation to both Escape and Ctrl-S note shortcuts — clean refactor with correct intent.
src/ui/components/panes/AgentInlineNote.tsx Adds a isSaveDraftNoteKey guard inside the textarea key handler, consuming the event before it reaches the textarea so CSI-u Ctrl-S no longer inserts 115;5u into the note text.
src/ui/AppHost.interactions.test.tsx Adds regression test for the CSI-u Ctrl-S path — verifies the note is saved and the draft label disappears without inserting stray characters.
CHANGELOG.md Single changelog entry added for the tmux CSI-u Ctrl-S fix.

Sequence Diagram

sequenceDiagram
    participant T as Terminal/tmux
    participant AI as AgentInlineNote keyHandler
    participant KS as useAppKeyboardShortcuts
    participant TA as Textarea

    T->>AI: KeyEvent (raw 0x13 or CSI-u)
    AI->>AI: isSaveDraftNoteKey check
    alt Ctrl-S recognized in component
        AI->>AI: preventDefault + stopPropagation
        AI->>AI: draft.onSave()
        note over TA: event never reaches textarea
    else Escape in component
        AI->>AI: preventDefault + stopPropagation
        AI->>AI: draft.onCancel()
    else other key
        AI->>KS: propagate
        KS->>KS: isSaveDraftNoteKey check
        alt Ctrl-S in app shortcuts
            KS->>KS: consumeKey + saveDraftNote
        else Escape in app shortcuts
            KS->>KS: consumeKey + cancelDraftNote
        else other
            KS->>TA: propagate to textarea
        end
    end
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/ui/lib/keyboard.ts:28-29
`CTRL_S_HEX_LABEL` matches the literal 4-character ASCII string `"0x13"`, not the control character. The control character (`\u0013`) is already matched by `CTRL_S` on lines 24–25, so these two clauses are dead code unless some terminal or tmux mode actually emits the text string `"0x13"` as a key sequence. If no such encoding exists, these lines should be removed to avoid confusion; if they were added to handle a specific case, a comment explaining which encoding produces this string would prevent the next reader from assuming it is a typo.

Reviews (1): Last reviewed commit: "fix(ui): save notes from tmux Ctrl-S inp..." | Re-trigger Greptile

Comment thread src/ui/lib/keyboard.ts Outdated
Comment on lines +28 to +29
sequence === CTRL_S_HEX_LABEL ||
raw === CTRL_S_HEX_LABEL
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 CTRL_S_HEX_LABEL matches the literal 4-character ASCII string "0x13", not the control character. The control character (\u0013) is already matched by CTRL_S on lines 24–25, so these two clauses are dead code unless some terminal or tmux mode actually emits the text string "0x13" as a key sequence. If no such encoding exists, these lines should be removed to avoid confusion; if they were added to handle a specific case, a comment explaining which encoding produces this string would prevent the next reader from assuming it is a typo.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/lib/keyboard.ts
Line: 28-29

Comment:
`CTRL_S_HEX_LABEL` matches the literal 4-character ASCII string `"0x13"`, not the control character. The control character (`\u0013`) is already matched by `CTRL_S` on lines 24–25, so these two clauses are dead code unless some terminal or tmux mode actually emits the text string `"0x13"` as a key sequence. If no such encoding exists, these lines should be removed to avoid confusion; if they were added to handle a specific case, a comment explaining which encoding produces this string would prevent the next reader from assuming it is a typo.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch — the literal 0x13 fallback was speculative and not backed by the tmux smoke test. I removed it, leaving only raw Ctrl-S and CSI-u handling.

This comment was generated by Pi using OpenAI GPT-5 Codex

@benvinegar benvinegar merged commit fac3c50 into main May 23, 2026
3 of 4 checks passed
@benvinegar benvinegar deleted the fix/tmux-ctrl-s-save-note branch May 23, 2026 16:47
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.

Keyboard command that include control (eg control + s) do not successfully trigger in tmux

1 participant