fix(tui): accept uppercase approval shortcuts

## Summary
- accept uppercase Y/A/N/D approval shortcuts in addition to lowercase and numeric input
- keep destructive approvals on the existing two-step confirmation path
- leave #1199 open because the Windows ConHost rendering report is broader than shortcut casing

## Test plan
- cargo test -p deepseek-tui benign_y_one_step_approves --locked
- cargo test -p deepseek-tui benign_a_two_approves_for_session --locked
- cargo test -p deepseek-tui benign_n_d_three_all_deny --locked
- cargo test -p deepseek-tui destructive_y_first_press_stages_then_second_commits --locked
- cargo test -p deepseek-tui destructive_a_first_press_stages_then_second_commits_session --locked
- cargo test -p deepseek-tui destructive_deny_does_not_require_confirmation --locked
- cargo fmt --all -- --check
- git diff --check
- GitHub CI: lint, version drift, ubuntu/macos/windows tests, npm wrapper smoke, GitGuardian
This commit is contained in:
Hunter Bown
2026-05-08 10:04:30 -05:00
committed by GitHub
parent c0fe2e3360
commit cd8f247fa1
+97 -69
View File
@@ -636,15 +636,17 @@ impl ModalView for ApprovalView {
KeyCode::Enter => self.commit_or_stage(self.current_option()),
// Direct shortcuts; '1' / '2' map to the first two options
// so a numeric pad still works for benign approve flows.
KeyCode::Char('y') | KeyCode::Char('1') => {
KeyCode::Char('y') | KeyCode::Char('Y') | KeyCode::Char('1') => {
self.commit_or_stage(ApprovalOption::ApproveOnce)
}
KeyCode::Char('a') | KeyCode::Char('2') => {
KeyCode::Char('a') | KeyCode::Char('A') | KeyCode::Char('2') => {
self.commit_or_stage(ApprovalOption::ApproveAlways)
}
KeyCode::Char('n') | KeyCode::Char('d') | KeyCode::Char('3') => {
self.commit_or_stage(ApprovalOption::Deny)
}
KeyCode::Char('n')
| KeyCode::Char('N')
| KeyCode::Char('d')
| KeyCode::Char('D')
| KeyCode::Char('3') => self.commit_or_stage(ApprovalOption::Deny),
KeyCode::Char('v') | KeyCode::Char('V') => {
self.pending_confirm = None;
self.emit_params_pager()
@@ -1173,15 +1175,20 @@ mod tests {
#[test]
fn benign_y_one_step_approves() {
let mut view = ApprovalView::new(benign_request());
let action = view.handle_key(create_key_event(KeyCode::Char('y')));
assert!(matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::Approved,
..
})
));
for code in [KeyCode::Char('y'), KeyCode::Char('Y')] {
let mut view = ApprovalView::new(benign_request());
let action = view.handle_key(create_key_event(code));
assert!(
matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::Approved,
..
})
),
"expected Approved for {code:?}"
);
}
}
#[test]
@@ -1212,30 +1219,31 @@ mod tests {
#[test]
fn benign_a_two_approves_for_session() {
let mut view = ApprovalView::new(benign_request());
let action = view.handle_key(create_key_event(KeyCode::Char('a')));
assert!(matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::ApprovedForSession,
..
})
));
let mut view = ApprovalView::new(benign_request());
let action = view.handle_key(create_key_event(KeyCode::Char('2')));
assert!(matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::ApprovedForSession,
..
})
));
for code in [KeyCode::Char('a'), KeyCode::Char('A'), KeyCode::Char('2')] {
let mut view = ApprovalView::new(benign_request());
let action = view.handle_key(create_key_event(code));
assert!(
matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::ApprovedForSession,
..
})
),
"expected ApprovedForSession for {code:?}"
);
}
}
#[test]
fn benign_n_d_three_all_deny() {
for code in [KeyCode::Char('n'), KeyCode::Char('d'), KeyCode::Char('3')] {
for code in [
KeyCode::Char('n'),
KeyCode::Char('N'),
KeyCode::Char('d'),
KeyCode::Char('D'),
KeyCode::Char('3'),
] {
let mut view = ApprovalView::new(benign_request());
let action = view.handle_key(create_key_event(code));
assert!(
@@ -1343,22 +1351,27 @@ mod tests {
#[test]
fn destructive_y_first_press_stages_then_second_commits() {
let mut view = ApprovalView::new(destructive_request());
for code in [KeyCode::Char('y'), KeyCode::Char('Y')] {
let mut view = ApprovalView::new(destructive_request());
// First press stages — no decision emitted yet.
let action = view.handle_key(create_key_event(KeyCode::Char('y')));
assert!(matches!(action, ViewAction::None));
assert_eq!(view.pending_confirm(), Some(ApprovalOption::ApproveOnce));
// First press stages — no decision emitted yet.
let action = view.handle_key(create_key_event(code));
assert!(matches!(action, ViewAction::None));
assert_eq!(view.pending_confirm(), Some(ApprovalOption::ApproveOnce));
// Second press of the same key commits.
let action = view.handle_key(create_key_event(KeyCode::Char('y')));
assert!(matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::Approved,
..
})
));
// Second press of the same key commits.
let action = view.handle_key(create_key_event(code));
assert!(
matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::Approved,
..
})
),
"expected Approved for {code:?}"
);
}
}
#[test]
@@ -1408,20 +1421,25 @@ mod tests {
#[test]
fn destructive_a_first_press_stages_then_second_commits_session() {
let mut view = ApprovalView::new(destructive_request());
for code in [KeyCode::Char('a'), KeyCode::Char('A')] {
let mut view = ApprovalView::new(destructive_request());
let action = view.handle_key(create_key_event(KeyCode::Char('a')));
assert!(matches!(action, ViewAction::None));
assert_eq!(view.pending_confirm(), Some(ApprovalOption::ApproveAlways));
let action = view.handle_key(create_key_event(code));
assert!(matches!(action, ViewAction::None));
assert_eq!(view.pending_confirm(), Some(ApprovalOption::ApproveAlways));
let action = view.handle_key(create_key_event(KeyCode::Char('a')));
assert!(matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::ApprovedForSession,
..
})
));
let action = view.handle_key(create_key_event(code));
assert!(
matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::ApprovedForSession,
..
})
),
"expected ApprovedForSession for {code:?}"
);
}
}
#[test]
@@ -1442,15 +1460,25 @@ mod tests {
#[test]
fn destructive_deny_does_not_require_confirmation() {
// Deny / Abort skip the two-key dance — the user is bailing.
let mut view = ApprovalView::new(destructive_request());
let action = view.handle_key(create_key_event(KeyCode::Char('n')));
assert!(matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::Denied,
..
})
));
for code in [
KeyCode::Char('n'),
KeyCode::Char('N'),
KeyCode::Char('d'),
KeyCode::Char('D'),
] {
let mut view = ApprovalView::new(destructive_request());
let action = view.handle_key(create_key_event(code));
assert!(
matches!(
action,
ViewAction::EmitAndClose(ViewEvent::ApprovalDecision {
decision: ReviewDecision::Denied,
..
})
),
"expected Denied for {code:?}"
);
}
}
#[test]