Skip to content

Commit 5db4b18

Browse files
baudouin-ivclaude
andcommitted
fix: harden leader chord handling for paste, modifiers, and validation
- Cancel the armed prefix when a paste arrives so the PREFIX indicator cannot get stuck and the next key is no longer consumed as a follow-up. - Match the leader chord on exact Ctrl/Alt/Shift so Ctrl+Alt+<leader> and Ctrl+Shift+<leader> pass through to the PTY instead of being swallowed. - Restrict parse_leader to ascii letters: digits and punctuation have no single control-byte encoding, so they broke literal <leader><leader> pass-through. Update the architecture doc to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 322de34 commit 5db4b18

4 files changed

Lines changed: 91 additions & 12 deletions

File tree

docs/architecture.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ trait TerminalBackend {
109109

110110
라우팅은 leader(prefix) 모델을 따른다. 1순위 사용자는 패널에서 LLM CLI를 굴리는 cockpit 사용자이므로, `Ctrl+W`/`Ctrl+L` 같은 프롬프트 편집 Ctrl 키가 nightcrow에 가로채이지 않고 PTY로 통과해야 한다. 앱 전역 명령은 leader 뒤에 한 키를 눌러야만 실행된다.
111111

112-
- **Leader (prefix)**: 기본값 `Ctrl+G`(tmux 자체 prefix인 `Ctrl+B`와 겹치지 않아 tmux 안에서도 사용 가능), `[input] leader`로 변경 가능(`config.rs::parse_leader``ctrl+<ascii>`만 허용하고 예약키는 거부). leader를 누르면 `App.prefix_armed` 플래그가 켜지고, 다음 키 한 개가 앱 명령(`input::prefix_action`)으로 해석된다. **타임아웃은 없다** — armed 상태는 follow-up 키나 `Esc`/`Ctrl+C`로만 해제된다. 해제 경로는 셋뿐이다: 매핑된 키 → Action 실행 후 해제, 미매핑 키 → 소비 후 해제, `Esc`/`Ctrl+C` → 취소. `<L> <L>`는 terminal focus에서 leader를 `encode_key`로 리터럴 PTY 전송한다. prefix 매핑: `t`=NewPane, `w`=ClosePane, `l`=ToggleLogView, `f`=ToggleFullscreen, `o`=ChangeRepo, `p`=CycleTheme, `q`=Quit, `1``7`=SwitchPane.
112+
- **Leader (prefix)**: 기본값 `Ctrl+G`(tmux 자체 prefix인 `Ctrl+B`와 겹치지 않아 tmux 안에서도 사용 가능), `[input] leader`로 변경 가능(`config.rs::parse_leader``ctrl+<letter>`만 허용하고 예약키·인코딩 불가 chord는 거부). leader를 누르면 `App.prefix_armed` 플래그가 켜지고, 다음 키 한 개가 앱 명령(`input::prefix_action`)으로 해석된다. **타임아웃은 없다** — armed 상태는 follow-up 키나 `Esc`/`Ctrl+C`로만 해제된다. 해제 경로는 셋뿐이다: 매핑된 키 → Action 실행 후 해제, 미매핑 키 → 소비 후 해제, `Esc`/`Ctrl+C` → 취소. `<L> <L>`는 terminal focus에서 leader를 `encode_key`로 리터럴 PTY 전송한다. prefix 매핑: `t`=NewPane, `w`=ClosePane, `l`=ToggleLogView, `f`=ToggleFullscreen, `o`=ChangeRepo, `p`=CycleTheme, `q`=Quit, `1``7`=SwitchPane.
113113
- **No-prefix 예약키**: `F1`/`F2`(focus jump), `F3``F9`(pane jump), `Shift+←/→`(focus cycle), `Shift+↑/↓`·`Shift+PgUp/PgDn`(터미널 스크롤)는 leader 없이 항상 앱이 먼저 처리한다. modifier 또는 F-key라서 프롬프트 텍스트와 혼동되지 않는다.
114114
- **Upper panel focused**: leader 명령과 no-prefix 예약키를 제외한 나머지는 로컬 네비게이션(`j`/`k`, `/`, `v`, `n`/`N`, `Enter`, `Esc`, 화살표, `PgUp`/`PgDn`)으로 처리된다. `j`/`k`는 upper-pane handler 내부에서 vim navigation으로 변환되며, `map_key`는 plain character로 통과시켜 terminal focus에서 PTY로 그대로 전달되게 한다.
115115
- **Lower panel focused (terminal)**: leader/예약키가 아닌 모든 키는 active backend의 stdin으로 직접 통과한다(`encode_key`가 화살표/F-key/제어문자를 VT100 시퀀스로 인코딩). 단독 `Ctrl+T/W/L/F/O/P/Q`도 더 이상 앱 명령이 아니므로 control byte로 PTY에 전달된다.

src/app.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,14 @@ impl App {
182182
}
183183
}
184184

185-
/// True when `key` matches the configured leader chord.
185+
/// True when `key` matches the configured leader chord. Only Ctrl/Alt/Shift
186+
/// distinguish a chord, so we compare exactly those: a bare `Ctrl+<leader>`
187+
/// matches, while `Ctrl+Alt+<leader>` / `Ctrl+Shift+<leader>` do not and
188+
/// pass straight through to the PTY instead of being swallowed.
186189
pub fn is_leader_key(&self, key: KeyEvent) -> bool {
190+
let relevant = KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SHIFT;
187191
key.code == self.leader.code
188-
&& key.modifiers.contains(KeyModifiers::CONTROL)
189-
== self.leader.modifiers.contains(KeyModifiers::CONTROL)
192+
&& (key.modifiers & relevant) == (self.leader.modifiers & relevant)
190193
}
191194
}
192195

src/config.rs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ pub fn parse_leader(spec: &str) -> Result<KeyEvent> {
5959
let rest = normalized.strip_prefix("ctrl+").ok_or_else(|| {
6060
anyhow::anyhow!(
6161
"input.leader \"{spec}\" must be a ctrl chord like \"ctrl+b\" \
62-
(only ctrl+<letter/ascii> leaders are supported)"
62+
(only ctrl+<letter> leaders are supported)"
6363
)
6464
})?;
6565
let mut chars = rest.chars();
@@ -71,14 +71,15 @@ pub fn parse_leader(spec: &str) -> Result<KeyEvent> {
7171
unreachable!()
7272
};
7373
anyhow::ensure!(
74-
c.is_ascii_graphic(),
75-
"input.leader \"{spec}\" must use a printable ascii character after ctrl+ \
76-
(e.g. ctrl+b, ctrl+a, ctrl+space is not allowed)"
74+
c.is_ascii_alphabetic(),
75+
"input.leader \"{spec}\" must use an ascii letter after ctrl+ \
76+
(e.g. ctrl+b; ctrl+1, ctrl+-, ctrl+space are not allowed)"
7777
);
78-
// A control chord that encode_key cannot turn into a single control byte
79-
// would make `<L><L>` literal pass-through impossible. The ascii-graphic
80-
// gate above already excludes space/control inputs, so every accepted
81-
// char maps to a control byte via the xterm convention.
78+
// Restricting to letters guarantees `<L><L>` literal pass-through works:
79+
// `encode_key` maps Ctrl+A..Ctrl+Z to control bytes 1..26 via the xterm
80+
// convention. Digits and punctuation (e.g. ctrl+1) have no single-control-
81+
// byte encoding, so encode_key would send the literal char instead and the
82+
// pass-through would break — hence they are rejected above.
8283
Ok(KeyEvent::new(KeyCode::Char(c), KeyModifiers::CONTROL))
8384
}
8485

@@ -738,6 +739,24 @@ command = "cargo test"
738739
assert!(leader.modifiers.contains(KeyModifiers::CONTROL));
739740
}
740741

742+
#[test]
743+
fn parse_leader_rejects_unencodable_ctrl_chords() {
744+
// Digits and punctuation have no single control-byte encoding, so they
745+
// would break `<L><L>` literal pass-through and must be rejected.
746+
for spec in ["ctrl+1", "ctrl+-", "ctrl+/", "ctrl+@"] {
747+
assert!(
748+
parse_leader(spec).is_err(),
749+
"{spec} must be rejected as a leader"
750+
);
751+
}
752+
}
753+
754+
#[test]
755+
fn parse_leader_rejects_non_ctrl_and_multichar() {
756+
assert!(parse_leader("g").is_err(), "bare key is not a ctrl chord");
757+
assert!(parse_leader("ctrl+ab").is_err(), "leader is a single key");
758+
}
759+
741760
#[test]
742761
fn input_leader_parses_from_toml() {
743762
let toml = r#"

src/main.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,11 @@ fn main_loop(
260260
/// paste from interactive input (crossterm consumes the outer markers when
261261
/// surfacing `Event::Paste`).
262262
fn handle_paste(app: &mut App, text: &str) {
263+
// A paste arriving while the prefix is armed would otherwise leave the
264+
// PREFIX indicator stuck and make the next key resolve as a follow-up.
265+
// Resolve the prefix first (tmux treats a non-command event as a cancel),
266+
// then route the paste normally.
267+
app.cancel_prefix();
263268
if app.repo_input.active {
264269
for ch in text.chars().filter(|c| !c.is_control()) {
265270
app.repo_input_push(ch);
@@ -755,6 +760,58 @@ mod tests {
755760
assert!(!app.prefix_armed(), "Esc must cancel the armed prefix");
756761
}
757762

763+
#[test]
764+
fn handle_key_leader_ctrl_c_cancels() {
765+
let mut app = app_with_terminal_pane();
766+
let _ = handle_key(&mut app, leader());
767+
assert!(app.prefix_armed());
768+
769+
let outcome = handle_key(&mut app, press(KeyCode::Char('c'), KeyModifiers::CONTROL));
770+
assert!(matches!(outcome, KeyOutcome::Continue));
771+
assert!(!app.prefix_armed(), "Ctrl+C must cancel the armed prefix");
772+
// The cancel is consumed, never leaked to the PTY.
773+
assert!(
774+
backend_payloads(&app).is_empty(),
775+
"Ctrl+C cancel must not send bytes to the PTY"
776+
);
777+
}
778+
779+
#[test]
780+
fn handle_key_ctrl_alt_leader_passes_through() {
781+
// Ctrl+Alt+<leader> carries an extra modifier, so it is NOT the leader
782+
// chord — it must reach the PTY rather than arm the prefix.
783+
let mut app = app_with_terminal_pane();
784+
785+
let outcome = handle_key(
786+
&mut app,
787+
press(KeyCode::Char('g'), KeyModifiers::CONTROL | KeyModifiers::ALT),
788+
);
789+
790+
assert!(matches!(outcome, KeyOutcome::Continue));
791+
assert!(
792+
!app.prefix_armed(),
793+
"Ctrl+Alt+leader must not arm the prefix"
794+
);
795+
assert!(
796+
!backend_payloads(&app).is_empty(),
797+
"Ctrl+Alt+leader must pass through to the PTY"
798+
);
799+
}
800+
801+
#[test]
802+
fn paste_while_prefix_armed_cancels_prefix() {
803+
let mut app = app_with_terminal_pane();
804+
let _ = handle_key(&mut app, leader());
805+
assert!(app.prefix_armed());
806+
807+
handle_paste(&mut app, "hello");
808+
809+
assert!(
810+
!app.prefix_armed(),
811+
"a paste must resolve (cancel) the armed prefix"
812+
);
813+
}
814+
758815
#[test]
759816
fn handle_key_leader_unmapped_followup_cancels() {
760817
let mut app = app_with_terminal_pane();

0 commit comments

Comments
 (0)