Pre-Push Hook Review and Fixes

Trail #2630 (https://entire.com/gh/entireio/-/trails/2630) has five open findings. cli/1704 has none; two are on cli/1707 (followups) and three on cli/1712 (first-push question, which is built on top of 1707). I read the code behind each one and all five are real.

cli/1707: muskan/checkpoint-holds-followups

1. Medium: no warning about an old hook when Entire is off for the branch (hooks_git_cmd.go:235, finding 01M646N7A6GP)

• Problem: When hooks are off (enabled:false, no .entire/, or settings that don't load), refuseOuterCheckpointPushWithHooksOff only runs CheckOuterPushHeldCheckpoints. The old-hook warning (warnOuterPushUncheckable) is only called from prePush. An old hook passes no ref list, so a git push --mirror sends held checkpoints and the user sees nothing.
• Fix: Also run the warning on the hooks-off path. Since it's unexported in strategy, I'd add one exported entry point that runs the check and then the warning, and use it from both prePush and the hooks-off path. Plus a test for the disabled case.

2. Low: slow pre-push in repos with many tags (checkpoint_push_policy.go:164, finding 01M646N8TTRZ)

• Problem: checkOuterPushOfHeldCommits looks up a tag object for every pushed line, even when no checkpoints are waiting. The finding measured 3.2s for 20k tags.
• Fix: Return early when nothing is waiting. Compare the raw hash first, and only look up the tag on refs/tags/ lines that don't match directly.

cli/1712: muskan/checkpoint-holds-first-push-question

3. Low: lost-race path skips the old-hook warning (manual_commit_push.go:440, finding 01M646NE0Y8C)

• Problem: When a stored policy wins the race against the question, this branch re-runs the check but never the warning.
• Fix: Call warnOuterPushUncheckable(ctx) there. It already checks for itself whether checkpoints are held and waiting.

4. Low: a manual answer lets an unchecked push through (checkpoint_push_policy.go:112, finding 01M646NCGK9W)

• Problem: If the hook couldn't save the ref list, refuseOuterPushAfterManualAnswer checks an empty list and passes. So the push where the user picks manual could be a --mirror that sends held checkpoints.
• Fix: Have checkOuterPushUnchecked take the reason as a parameter, and call it from the manual-answer path when prePushRefsUnsaved.

5. Low: the prompt timeout doesn't work on macOS (manual_commit_hooks.go:82, finding 01M646NAPBHB)

• Problem: I checked this one directly. Opening /dev/tty and calling SetReadDeadline on this Mac returns file type does not support deadline. So readPromptAnswer falls back to a goroutine read that stays blocked after the timeout and can take the next line typed into the terminal, such as a git credential prompt during the upload. The current test passes only because it opens the pty by name (/dev/ttysNNN), which does support deadlines.
• Fix options:
  • (a) Fix it (my recommendation). On darwin, wait for input with unix.Select with a timeout before reading. Select works on /dev/tty even though deadlines don't. Change the test so it reads through a real /dev/tty whose controlling terminal is the pty. That means darwin-specific code and a fiddlier test.
  • (b) Resolve with a note. This only matters when nobody answers within 15s and git then prompts in the same terminal.

Plan

• Fixes 1 and 2 go on the followups branch, which I'll then merge into first-push-question. Fixes 3 to 5 go on first-push-question.
• I'll write failing tests first, then run mise run check, push, and resolve each finding with a short note.

Should I go ahead, and do you want (a) or (b) for finding 5?

I'm waiting on mise run check for the 1707 branch. The background job will notify me when it finishes, so I don't need a monitor.

Where things stand:

• 1707: fixes 1 and 2 are done and tested, waiting on the full check before I commit and push.
• 1712: fix 5 is done and tested. Tests for 3 and 4 are written and failing as expected. Once 1707 is committed, I'll merge it into 1712; that merge also brings fix 3, because the warning now runs inside CheckOuterPushHeldCheckpoints. Then I'll add fix 4.

The CI tests are rerunning in the background with the full log saved; I'll find the failure when it finishes. Nothing is committed yet.

The tree is identical to the backup (empty diff). Deleting the backup branch and running the full mise run check on 1712.