Skip to content

Fix hook binary resolution for managed installs#760

Open
mariusvniekerk wants to merge 2 commits into
mainfrom
codex/hook-binary-resolution
Open

Fix hook binary resolution for managed installs#760
mariusvniekerk wants to merge 2 commits into
mainfrom
codex/hook-binary-resolution

Conversation

@mariusvniekerk
Copy link
Copy Markdown
Collaborator

Summary

  • add --binary to roborev init and install-hook so users can explicitly choose the roborev executable baked into git hooks
  • resolve stable shim/symlink paths for empirically verified mise and Homebrew installs instead of pinning hooks to versioned binaries
  • cover mise Go backend, mise GitHub Releases backend, Homebrew/Linuxbrew, explicit binary overrides, and unsupported-manager non-guesses in tests

Validation

  • go fmt ./...
  • go test ./internal/githook
  • go vet ./...
  • go test ./...
  • make lint
  • Docker probes for mise Go backend, mise GitHub backend, Homebrew/Linuxbrew, rtx, and asdf

roborev init previously baked the running executable into git hooks, which is brittle when the command was launched through a version manager. mise and Homebrew both expose stable shim or bin paths while keeping the actual executable under versioned install directories, so hooks could be pinned to a binary that later disappeared after an update.

This adds an explicit hook binary override and makes the default resolver prefer empirically verified stable paths for mise and Homebrew installs. It intentionally avoids guessing rtx/asdf layouts because container probes and current docs did not show a standard roborev install path there; users can still pass --binary for custom setups.

Validation: go fmt ./...; go test ./internal/githook; go vet ./...; go test ./...; make lint; Docker probes for mise Go backend, mise GitHub backend, Homebrew/Linuxbrew, rtx, and asdf.

🤖 Generated with [OpenAI Codex](https://openai.com/codex)
Co-authored-by: OpenAI Codex <noreply@openai.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (6b9668f)

Medium: Current managed hooks can keep a stale ROBOREV= binary path after roborev init --binary ....

Medium

  • internal/githook/githook.go:466
    Existing hooks with the current version marker are skipped before verifying whether the embedded ROBOREV= path matches the newly resolved shim or explicit --binary value. Users with current v4/v2 hooks pointing at an old versioned mise/Homebrew binary can run roborev init --binary ... and still keep the stale path.
    Fix: When a current hook is detected, update it if the embedded binary differs from the resolved path, or bump the hook version markers so managed-install users are upgraded.

Synthesized from 2 reviews (agents: codex | types: default, security)

The managed-install resolver only helps if existing current-version hooks can move off an old baked binary path. Without this, rerunning roborev init or install-hook with a stable shim leaves v4/v2 hooks untouched because the version marker already matches.

This treats a current hook as reusable only when its generated ROBOREV assignment matches the resolved or explicit binary path. Otherwise the roborev block is removed and re-rendered, preserving surrounding user hook content for embedded installs.

Validation: go test ./internal/githook; go vet ./...; go test ./...; make lint

🤖 Generated with [OpenAI Codex](https://openai.com/codex)
Co-authored-by: OpenAI Codex <noreply@openai.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (487a623)

No Medium, High, or Critical findings were reported.

All reported issues were below the requested severity threshold, and the security review found no issues.


Synthesized from 2 reviews (agents: codex | types: default, security)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant