Read the push shim's refspecs off the positions the matcher found

[HackingGate]

Closes #84.

VALUE_OPTIONS taught reading git's global grammar, and that is what makes
git -c user.name=x push match a push:* table at all. collect_git_refs
was never given the same grammar: it skipped argv[0] as the subcommand and
the next non-option word as the remote, so that invocation read user.name=x
as the remote and push as a refspec. The branch going onto the forge was
collected nowhere, and the fallback that reads it off HEAD did not run
either, because a name had been collected.

The shim matched, a checker ran, it approved a word nobody was publishing,
and the command exec'd. Exit 0.

What changed

before	after
words walks argv and stops at two positional words	scan walks argv and stops after stop_after of them
collect_git_refs reads argv.iter().skip(1) and the first non-option word after it	collect_git_refs reads self.positional(argv).iter().skip(2)

One grammar in one place: the matcher takes the first two positional words
out of scan, a positional collector takes all of them. Nothing else in the
walk moved -- the -- terminator, the inline --flag=value split and the
unclear accounting are the same code they were.

Measured

A tree whose policy refuses the branch name it is standing on, 1.8.0 as
installed vs this branch:

invocation	1.8.0	here
uphold shim git push	refused	refused
uphold shim git -c user.name=x push	published	refused
uphold shim git -C elsewhere push	published	refused
uphold shim git --git-dir X push	published	refused
uphold shim git -c user.name=x push origin ordinary	ran	ran

What is still approximate

An option this grammar cannot classify shifts the positions by one. The words
after it are read regardless, and a shift that leaves no refspec at all falls
back to HEAD -- so git push -o ci.skip origin topic collects origin as
well as topic. An extra subject rather than a missing one, which is the
direction this seam errs in everywhere else.

Tests

Both fail on the old collector and pass here, which was checked by putting
the old body back:

• shim::tests::a_global_option_does_not_shift_which_word_the_branch_is --
  four spellings of a global option, one expected subject each.
• shim_cli::a_global_option_does_not_shift_which_word_the_branch_is -- real
  invocations against a stub git on PATH, including the form where the name
  appears nowhere in argv and has to come off HEAD. Asserts the command
  never ran, and that a clean name past the same option still reaches it.

Verification

• prek run --all-files and prek run --all-files --hook-stage manual --
  every hook passed, coverage floor included
• cargo test 566 pass, cargo clippy --all-targets, cargo fmt --check

One thing found on the way, and not fixed here: the git hooks installed in
this checkout are lefthook's, and lefthook is not on PATH, so every git commit and git push prints Can't find lefthook in PATH and proceeds.
The gates above were run by hand for that reason.

Summary by CodeRabbit

• Bug Fixes

  • Git reference collection now correctly handles global options placed before subcommands.
  • Branch names and refspecs are identified accurately when invoking Git commands.
  • Rejected pushes are blocked before Git runs, while valid pushes continue normally.

• Tests

  • Added coverage for branch validation, HEAD handling, global Git options, rejected pushes, and successful push forwarding.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3a216e72-3eb2-4861-a137-d7d5b4ef5bac

📥 Commits

Reviewing files that changed from the base of the PR and between 07dedf2 and fb65633.

📒 Files selected for processing (2)

• src/shim.rs
• tests/shim_cli.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The shim now shares grammar-aware argv scanning between command matching and Git ref collection. Git pushes with leading global options identify the correct remote and refspecs. Regression tests cover branch rejection, HEAD fallback, and forwarding behavior.

Changes

Git ref collection

Layer / File(s)	Summary
Shared argv scanner src/shim.rs	The Scanned result combines positional arguments with unknown-option metadata. Subcommand matching and positional collection use bounded or unbounded scans.
Git ref policy and CLI coverage src/shim.rs, tests/shim_cli.rs	collect_git_refs reads grammar-aware positions. Tests cover -c, -C, --git-dir, HEAD fallback, rejected pushes, and forwarded clean pushes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to fb656

This change keeps Git options from shifting the remote and refspec positions used by the push shim, with targeted tests and project checks reported passing; no actionable merge-blocking risk remains beyond normal review.

Sequence Diagram(s)

sequenceDiagram
  participant GitCLI
  participant Shim
  participant ArgvScanner
  participant GitRefCollector
  participant Git
  GitCLI->>Shim: invoke git push with global options
  Shim->>ArgvScanner: parse argv with Git option grammar
  ArgvScanner-->>GitRefCollector: return positional arguments
  GitRefCollector-->>Shim: return push refs
  alt policy rejects a branch
    Shim-->>GitCLI: reject push
  else policy allows the branch
    Shim->>Git: execute git push
    Git-->>GitCLI: return Git result
  end

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: collecting push refspecs from matcher-identified positional arguments.
Linked Issues check	✅ Passed	The changes address issue #84 by sharing option grammar, handling global options, collecting correct refs, and adding regression coverage.
Out of Scope Changes check	✅ Passed	The shim refactor and Git CLI tests directly support the linked issue and stated objectives; no unrelated changes are evident.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch read-the-refspecs-off-the-positions

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.59%. Comparing base (07dedf2) to head (fb65633).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
+ Coverage   90.51%   90.59%   +0.07%     
==========================================
  Files          35       35              
  Lines       11339    11370      +31     
==========================================
+ Hits        10264    10301      +37     
+ Misses       1075     1069       -6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.