Fix #2247: allow git checkout HEAD -- <path> in pipeline sessions

[jwbron]

Summary

• is_branch_switch (gateway/git_client.py) classified any positional arg before -- as a branch ref, so git checkout HEAD -- file.txt (and <sha> / <branch> variants) were rejected in pipeline sessions, contradicting the function's docstring. Agents fell back to multi-step recovery chains (status/awk/stash/diff) when one command would have done.
• When -- is present, the leading positional is a commit-ish source for a file restore (HEAD, HEAD~N, sha, branch name) and does not switch the active branch. Return False in that case, refresh the docstring, and add HEAD~1 / <sha> regression coverage.
• Flipped the two tests that pinned the buggy behavior (test_checkout_branch_with_double_dash_and_files, test_checkout_head_double_dash_file).

Fixes #2247.

Test plan

• make test (or make test-all) passes locally on the gateway suite.
• git checkout HEAD -- file.txt succeeds inside a pipeline session.
• git checkout main is still blocked in a pipeline session.

[egg-reviewer]

Review

Targeted fix for the documented is_branch_switch false positive in #2247. The new logic correctly classifies git checkout [<commit-ish>] -- <pathspec> as a file restore, brings the helper into parity with the already-correct is_branch_switching_checkout (gateway/git_client.py:1161), and the docstring matches the implementation. No blocking issues.

Correctness

• The semantic claim is right: when -- is present, anything before it in git checkout is a tree-ish source for the restore, never a branch switch. git checkout main -- file.txt does not move HEAD; it stages/working-tree-writes file.txt from main. Allowing this in a pipeline session does not violate the branch lock — the agent's HEAD stays on the assigned branch, and any commit they make still lands there.
• Behavior parity with is_branch_switching_checkout (git_client.py:1161-1230) is preserved. Both now return False for ["HEAD", "--", "file.txt"], ["main", "--", "file.txt"], etc.
• Branch-switching forms are still caught:
  • ["main"], ["origin/main"] → True (no --, positional present)
  • ["-b", "new"], ["-B", "x"], ["--orphan", "x"] → True (early return on the flag check, before the -- short-circuit)
  • ["--detach", "HEAD"] via switch → True (operation == "switch")
• --orphan/-b/-B short-circuit before the -- handler runs, so a hypothetical git checkout -b new -- file (invalid syntax in real git) still classifies as a branch switch — the safe default.
• ["-p", "main"] still returns False (file flag short-circuit), preserving the existing patch-mode handling.

Tests

• Tests now cover the three commit-ish flavors the issue calls out: HEAD, HEAD~1, <sha>. Good regression coverage.
• The two pinning tests (test_checkout_branch_with_double_dash_and_files, test_checkout_head_double_dash_file) are correctly flipped, with updated docstrings describing the new behavior.
• Existing tests for the still-blocked cases (test_checkout_branch_name, test_checkout_remote_branch, test_checkout_unknown_flags_ignored) are unchanged and still pass under the new logic — no silent loosening of the lock.

Non-blocking suggestions

1. Test placement. test_checkout_branch_with_double_dash_and_files lives in TestIsBranchSwitchCheckoutBranch (the class for cases that are branch switches) but now asserts is False. Consider moving it to TestIsBranchSwitchCheckoutFiles or TestIsBranchSwitchEdgeCases so the class taxonomy stays meaningful. (gateway/tests/test_branch_switch.py:73-75)

2. Error-message hint. The block message at gateway/gateway.py:2310 still says only Use 'git checkout -- <file>' to restore files instead. Now that git checkout HEAD -- <file> (and <sha> / <branch> variants) work, expanding the hint to mention the commit-ish form would save the next agent a round-trip. E.g. Use 'git checkout [-- | <commit-ish> --] <file>' to restore files instead.

3. De-duplication follow-up. The issue's optional cleanup — routing the gateway.py:2291 check through is_branch_switching_checkout and retiring is_branch_switch — is still on the table. The two helpers now agree behaviorally, but maintaining two parallel implementations of the same heuristic is a drift risk. Worth a separate PR.

— Authored by egg

[egg-reviewer]

Re-Review

Re-reviewing the delta since 8dd50fb. Two-file follow-up commit (54a3db1) addressing both non-blocking suggestions from the prior review. Clean and correct.

Delta

gateway/gateway.py:2310 — Error hint expanded as suggested.

• Old: Use 'git checkout -- <file>' to restore files instead.
• New: Use 'git checkout [<commit-ish>] -- <file>' to restore files instead (e.g. 'git checkout HEAD -- <file>' or 'git checkout <sha> -- <file>').

The bracket syntax [<commit-ish>] correctly conveys that the commit-ish is optional, and the two examples cover the cases agents will actually reach for. Saves the next agent a round-trip, as intended.

gateway/tests/test_branch_switch.py — test_checkout_branch_with_double_dash_and_files relocated from TestIsBranchSwitchCheckoutBranch (which is for cases that are branch switches) into TestIsBranchSwitchEdgeCases. The class taxonomy is now consistent again, and grouping it next to test_checkout_head_double_dash_file / test_checkout_head_relative_double_dash_file / test_checkout_sha_double_dash_file (already in TestIsBranchSwitchEdgeCases) puts the four "commit-ish before --" cases together. Good landing spot.

Verification

• All 33 tests in gateway/tests/test_branch_switch.py pass locally.
• The behavioral change from the original fix is unchanged — no logic touched, only the error string and a class label.
• The third suggestion (de-duplicating is_branch_switch and is_branch_switching_checkout) is correctly deferred to a separate PR per my earlier note.

No new issues. Approving.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.