From b3adcb18ef762d8652272e2ea68c0f416f33c740 Mon Sep 17 00:00:00 2001 From: okwn Date: Thu, 21 May 2026 10:16:09 +0000 Subject: [PATCH 1/2] fix: honor explicit -n N limit for git log on merge commits When user runs 'git log -1 --format='%H' HEAD' where HEAD is a merge commit, rtk was adding --no-merges which filtered out the merge commit itself and returned the second parent instead. This made 'git log -1' return wrong SHAs for merge commits. Fix: don't add --no-merges when user explicitly passes -n N or --max-count=N. When a user specifies an exact count they expect exactly that many commits, not filtered results. Also skip --no-merges if user already passed --merges or --no-merges explicitly. Fixes rtk-ai/rtk#2009. --- src/cmds/git/git.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index 6f42d20e8..95bba3729 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -522,8 +522,10 @@ fn run_log( // Only add --no-merges if user didn't explicitly request merge commits let wants_merges = args .iter() - .any(|arg| arg == "--merges" || arg == "--min-parents=2"); - if !wants_merges { + .any(|arg| arg == "--merges" || arg == "--min-parents=2" || arg == "--no-merges"); + // Don't add --no-merges if user explicitly requested merges or an exact count (-n N / --max-count) + // When user passes -1 they want 1 commit regardless of whether it's a merge + if !wants_merges && !has_limit_flag { cmd.arg("--no-merges"); } From 3f24599363737f1e4e2dc6ebc764e123f4ae3412 Mon Sep 17 00:00:00 2001 From: okwn Date: Thu, 21 May 2026 13:45:36 +0000 Subject: [PATCH 2/2] test(permissions): add regression tests for deny/allow mixed chain precedence Add 3 tests covering the missing deny/allow mixed chain scenario: - test_chain_deny_short_circuits_allow: single denied segment short-circuits - test_chain_deny_wins_over_partial_allow: deny wins in multi-segment chain - test_chain_deny_wins_over_ask: deny > ask precedence in compound commands These complement existing #1213 tests (which cover allow-only chains) by covering the deny precedence edge cases that were not tested. Fixes: #1213 (precedence documentation gap) --- src/hooks/permissions.rs | 53 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 71536b0e1..9249d5339 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -594,6 +594,59 @@ mod tests { // grant Allow to the entire chain. Every non-empty segment must match // independently. + // --- Mixed deny/allow chain tests --- + // Deny short-circuits the entire chain regardless of allow rules on other segments. + // This tests that a single denied segment takes priority over a permitted one. + + #[test] + fn test_chain_deny_short_circuits_allow() { + // Deny takes precedence: `git reset` is denied, `git status` is allowed. + // Even though `git status` matches allow, the denied `git reset` must + // short-circuit the entire chain to Deny (not Default/Allow/Ask). + let deny = vec!["git reset".to_string()]; + let allow = vec!["git status".to_string(), "git *".to_string()]; + + // Deny segment present → entire chain is Deny + assert_eq!( + check_command_with_rules("git status && git reset --hard", &deny, &[], &allow), + PermissionVerdict::Deny, + "denied segment must short-circuit even with allowed segments present" + ); + } + + #[test] + fn test_chain_deny_wins_over_partial_allow() { + // Three-segment chain: first allowed, second denied, third allowed. + // Deny short-circuits; result is Deny (not Allow, not Default). + let deny = vec!["git push".to_string()]; + let allow = vec!["git status".to_string(), "git log".to_string()]; + + assert_eq!( + check_command_with_rules( + "git status && git push origin main && git log --oneline -5", + &deny, + &[], + &allow + ), + PermissionVerdict::Deny, + "deny must win even when allow rules exist for other segments" + ); + } + + #[test] + fn test_chain_deny_wins_over_ask() { + // Deny also wins over Ask: `git commit` denied, `git status` would be Ask. + // Precedence is Deny > Ask > Allow > Default. + let deny = vec!["git commit".to_string()]; + let ask = vec!["git status".to_string()]; + + assert_eq!( + check_command_with_rules("git status && git commit -m foo", &deny, &ask, &[]), + PermissionVerdict::Deny, + "deny must win over ask even in compound commands" + ); + } + #[test] fn test_compound_allow_requires_every_segment() { // Reproduces #1213: `git status` is allowed but `git add .` is not.