Port check-leaks and the pre-push installer to agentrepo

Problem

The leak scanner (.devtools/scripts/check-leaks.sh, 881 physical lines), its working-tree skip loader (leak-skip.sh, 16 lines) and the pre-push installer (install-pre-push.sh, 119 lines) are complex shell that ships to every consumer, and the pre-push path runs on every push. The shell is frozen by #588, which directs that new behaviour is written as tested Go and that the shell is removed by a later issue. This issue is the Go port of the installer and scanner for that direction.

The scanner alone has 176 Bats cases behind it and doesn't fit one pull request. This issue therefore delivers the first PR-sized slice, the Go pre-push installer, and lists the remaining scanner slices and the task-body cutover as proposed sub-issues under Scope and non-goals.

Port rule, from #588: new behaviour lives under internal/ and cmd/ with Go tests, reuses the existing agentrepo command and its configuration code, adds no new shell, and leaves the frozen shell files untouched.

Parity contract: the shell installer is the reference. It rewrites the wrapper and sets its mode on every run, including a repeat run, so parity here means the same resulting wrapper contents and file mode after a first install and after a repeat install. It doesn't mean the Go installer avoids the write.

Acceptance criteria

AC1: Given a repository whose pre-push hook is prek's generated hook, when the installer runs, then prek's hook is kept as pre-push.prek, pre-push holds the devtools wrapper, it is executable, and the wrapper text and file mode are byte-identical to what the shell installer produces for the same repository.
AC2: Given the wrapper is already installed in front of prek's hook, when the installer runs again, then it exits 0, prints the success message, and leaves pre-push and pre-push.prek with the same contents and file mode as after the first run, matching the shell installer's result on a repeat run. Non-functional: idempotence of the resulting state.
AC3: Given prek install has parked the wrapper as pre-push.legacy, when the installer runs, then pre-push.legacy is removed and the wrapper is back in front of prek's hook.
AC4: Given a foreign pre-push, a foreign or non-prek pre-push.prek, or no hooks directory, when the installer runs, then it exits 1 with the same message as the shell installer and leaves every hook file untouched.
AC5: Given the wrapper text differs from the expected one, prek's hook is missing, or the wrapper sits in pre-push.legacy, when the installer runs with --check, then it exits 1 with the same message as the shell installer. When the installed wrapper matches the expected text, it exits 0. Parity includes the shell's comparison rule: it reads both texts through command substitution, which drops trailing newlines, so a wrapper differing only in trailing newlines passes in both installers.
AC6: Given --check fails, when DEVTOOLS_LEAK_HOOK_CHECK=warn is set, then it prints a warning and exits 0. Given CI is set, --check exits 0 without reading the hooks directory.
AC7: Given .pre-commit-config.yaml contains the commit-msg-pre-push hook id, when the wrapper is written, then it enforces commit-lint and fails with the install hint when commit-lint is missing. Without that id, the wrapper has no commit-lint step.
AC8: Given a push that adds a private literal, when it runs through the wrapper the Go installer wrote, then the push is blocked with the same exit status and findings as through the wrapper the shell installer wrote. Given a clean push, the downstream prek hook receives exactly Git's original arguments and exactly the ref lines Git wrote to standard input, which a stub downstream hook records and the test compares. Non-functional: leak-scan behaviour stays byte-compatible with the shell original and is never weakened.
AC9: Given the Windows, macOS and Linux CI jobs, when the installer tests run, then they pass on all three. Non-functional: portability.

Scope and non-goals

In scope: the installer and its --check mode, as a Go subcommand of agentrepo with Go tests ported from the Bats cases that cover the installer.

Non-goal: switching the task bodies. The existing task names devtools:install-pre-push-hook and the pre-push check task keep calling the shell installer in this PR. The task-body switch is a follow-up that waits for a release carrying the binary, because consumers resolve the tasks from the vendored payload and need the binary available first.

Non-goal: changing what the scanner finds, or its exit statuses, output or messages. Leak-scan behaviour stays byte-compatible and is never weakened in any slice.

Non-goal: editing or deleting the frozen shell files, the scanner Bats suite, leak-private-strings.py or the gitleaks task. A later #588 issue removes them.

Non-goal: the shared-hooks fix for worktrees. That belongs to #277 and this port keeps the current behaviour.

Proposed follow-up sub-issues of this one, each a single PR within the limit:

Scan core: private-literal loading (including the Python helper's behaviour), base patterns, .leak-patterns, .leak-allow, opaque locations, and the working-tree and FILE... modes with .leak-skip. Size M.
Staged and range modes: whole-object reads, completeness checks and the exit 2 rules. Size M.
Pre-push, pre-push-hook and publish modes, including outgoing tags and the git ls-remote destination lookup. Size M.
Cut over the scanner task bodies and devtools:gitleaks to the Go scanner after parity runs against the shell on shared fixtures. Size S.
Switch the two installer task bodies to the Go installer after a release carries the binary. Size S.

Approach

Add an installer subcommand to the existing agentrepo command, next to its other subcommands, reusing its configuration and repository helpers instead of a new binary. The wrapper script text is a fixed template kept byte-identical to the shell version, because --check compares it exactly and installed hooks must not churn on upgrade. Tests run against scratch Git repositories, isolated from global and system Git configuration as the repository's test helpers do.

Port the installer's Bats cases first as failing Go tests, then implement. Run the shell installer and the Go subcommand on the same fixtures and compare exit status, both output streams, and the resulting hook file contents and modes after a first install and after a repeat install.

Dependencies

Native blockers: none.
Replace complex shell with tested Go, starting with gates and merging #588 (open): the Go port direction this issue follows.
Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277 (open): the shared-hooks fix. It isn't a blocker, since this port preserves current behaviour, and either order works.
Rollout prerequisite, not a blocker of this PR: Move reusable agent tooling into released Go binaries #174 (open), the plan to ship reusable agent tooling as released Go binaries, which decides how agentrepo reaches consumers. It gates only the task-body switch in follow-up 5. Building, testing and merging the installer needs no distribution decision, because the installer lives in the existing agentrepo command and the shell installer keeps serving consumers until that follow-up.
Rollout prerequisite, not a blocker of this PR: build(agents): protect replacement Go enforcement paths #753 (open), golden-file protection for replacement enforcement code. It must cover the new installer files before the task-body switch, and that coverage requires a change to .agents/agent-skills.conf, which is owner-tier and handled in build(agents): protect replacement Go enforcement paths #753 itself. This PR touches no owner-tier path.
Both prerequisites are prose and not native blocked-by links because neither blocks building or merging this PR.
feat(gh-post): run the leak review inside every post #525 (open): folding the leak review into gh-post. Unrelated to this slice and listed to avoid overlap.

Size

S. The shell original is 119 lines. Estimated about 250 lines of Go and about 450 lines of Go tests, within one pull request under 800 code and 2000 prose lines. The full scanner port is larger than one pull request, hence the proposed split above.

Verify plan

AC1: Go test installs into a scratch repository with a prek hook and compares the wrapper bytes and mode with the shell installer's output on the same repository.
AC2: Go test runs the installer twice and compares the contents and modes of pre-push and pre-push.prek after each run with the shell installer's results.
AC3: Go test builds the pre-push.legacy layout and checks the result.
AC4: table-driven Go test over the foreign, non-prek and missing-directory layouts, asserting message, exit status and untouched files.
AC5: table-driven Go test over mismatching text, missing prek hook and legacy wrapper, plus the matching case and a case with extra trailing newlines, each compared with the shell installer's exit status and message.
AC6: Go tests with DEVTOOLS_LEAK_HOOK_CHECK=warn and with CI set.
AC7: Go tests with and without the hook id in .pre-commit-config.yaml, comparing against the shell installer's wrapper text.
AC8: integration test pushes to a local bare remote through the installed wrapper with the existing shell scanner, once with a private literal and once clean. The blocked push is compared with the same push through the shell-installed wrapper. The clean push uses a stub downstream hook that writes its arguments and standard input to a file, and the test asserts both equal what Git passed.
AC9: the Windows, macOS and Linux CI test jobs.

Risks

A wrapper that differs by one byte makes every consumer's --check fail until the installer reruns. The comparisons in AC1 and AC5 guard this. A wrong hook swap could leave a push unscanned or pass altered arguments to prek, which the AC8 integration test covers. Undo: revert the PR. Nothing calls the Go subcommand until the task-body switch, so a revert changes nothing for consumers.

Operator authority

None

Refinement: NOT READY — prerequisite issues are listed only in prose

Checked

The checkout’s origin is https://github.com/tbhb-dev/devtools.git. I reviewed the live issue body, label and all comments once; there are no earlier refinement comments, so this is round 1.
Compared the plan with the installer at .devtools/scripts/install-pre-push.sh:20 and the agentrepo command structure at internal/agentrepo/command.go:19.
Replace complex shell with tested Go, starting with gates and merging #588, Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277, Move reusable agent tooling into released Go binaries #174, build(agents): protect replacement Go enforcement paths #753 and feat(gh-post): run the leak review inside every post #525 are all open. The Go-port direction matches Replace complex shell with tested Go, starting with gates and merging #588.
The proposed changes do not name paths in the specified owner-tier set. However, build(agents): protect replacement Go enforcement paths #753 requires a change to .agents/agent-skills.conf, which is owner-tier.
The listed acceptance criteria each describe observable outcomes, and the Verify plan pairs each AC ID with a corresponding check. The installer and task-body slice fits one PR, and the undo is stated.

Gaps

Major — Dependencies: “Native blockers: none” conflicts with the stated rollout prerequisites. build(agents): protect replacement Go enforcement paths #753 must cover the replacement enforcement code before operational rollout, and Move reusable agent tooling into released Go binaries #174’s distribution decision affects how consumers receive agentrepo. Add native blocked-by links for prerequisites that block this issue’s rollout, or clarify why they do not block it. .agents/agent-skills.conf is owner-tier under the requested review policy.
Minor — Definition of done: No base D1–D10 checklist is provided in the issue or repository material I checked, so I cannot assess its planning coverage item by item.

Split

Keep the installer, --check mode and two task-body changes together. The scanner slices are already identified as separate proposed issues; no further split is needed.

Definition of done

Planning coverage only; checked boxes indicate that the ACs and Verify plan cover the item, not that work is complete.

Round: 1 of 3

Refinement: NOT READY — rollout prerequisites still lack native blockers

Checked

origin is https://github.com/tbhb-dev/devtools.git. Read the live issue body, label and comments once. There is one earlier “Refinement: NOT READY” comment, so this is round 2.
Confirmed the prior dependency gap remains: the body says “Native blockers: none,” while the round‑1 comment identifies build(agents): protect replacement Go enforcement paths #753 and Move reusable agent tooling into released Go binaries #174 as rollout prerequisites. Round‑1 also noted that build(agents): protect replacement Go enforcement paths #753 requires an owner-tier change to .agents/agent-skills.conf.
Compared the proposed Go port with internal/agentrepo/command.go:19 and the existing installer at .devtools/scripts/install-pre-push.sh:1. The proposed direction follows the existing agentrepo command structure and does not introduce new shell logic.
The prior review found the ACs observable and paired with checks in the Verify plan, the scope fits one PR, and the undo is stated. The current body does not resolve the dependency gap.

Gaps

Major — Dependencies: build(agents): protect replacement Go enforcement paths #753 and Move reusable agent tooling into released Go binaries #174 are stated rollout prerequisites but are not represented as native blocked-by links. Add links if they block rollout, or clarify why they do not. The build(agents): protect replacement Go enforcement paths #753 prerequisite also involves an owner-tier path.

Split

Keep the installer, --check mode and two task-body changes together. The scanner work is already split into follow-up issues; no additional split is indicated.

Definition of done

Planning coverage only: checked boxes mean the ACs and Verify plan cover the item, not that the work is complete.

Round: 2 of 3

Refinement: NOT READY — parity and cutover scope remain inconsistent

Checked

Verified origin identifies tbhb-dev/devtools. Read the live body, labels and both comments once. Two earlier NOT READY comments make this round 3.
Compared every AC with origin/main and with the other ACs. AC1–AC7 describe installer behavior; AC8 covers wrapper execution; AC9 covers task callers; AC10 matches the existing platform matrix at .github/workflows/ci.yml:175.
The approach reuses agentrepo, consistent with internal/agentrepo/command.go:19 and Replace complex shell with tested Go, starting with gates and merging #588. It adds no shell logic or unnecessary guard, configuration knob or layer.
Replace complex shell with tested Go, starting with gates and merging #588, Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277, Move reusable agent tooling into released Go binaries #174, build(agents): protect replacement Go enforcement paths #753 and feat(gh-post): run the leak review inside every post #525 remain open. The stated reason for keeping rollout prerequisites in prose is acceptable; missing native links are not a gap.
Idempotence, compatibility and portability are addressed, and the risks include an undo.
The body names .agents/agent-skills.conf, an owner-tier path, solely for work assigned to build(agents): protect replacement Go enforcement paths #753. No proposed implementation path otherwise requires the specified owner-tier authority.

Gaps

Major — AC2 conflicts with the parity contract. Its verification requires unchanged modification state, but the shell installer always rewrites and chmods the wrapper, including on repeat installation (.devtools/scripts/install-pre-push.sh:117). Clarify whether parity means unchanged contents or whether avoiding writes is an intentional behavior change; reconcile that choice with AC9 and the old/new comparison.
Major — The task cutover has two conflicting delivery points. Scope and AC9 require this PR to switch both task bodies to Go (.devtools/tasks/git-hooks.toml:30). Dependencies say the shell installer keeps serving consumers until a later cutover. Specify whether this PR delivers that caller switch or only the installer. This concerns the deliverable, not whether rollout prerequisites need native links.
Major — AC8’s check does not prove argument and input preservation. Comparing push status and output cannot establish that prek received Git’s original arguments and ref lines. Add explicit assertions capturing both at the downstream hook; forwarding them is existing behavior at .devtools/scripts/install-pre-push.sh:77.

Split

The installer and its checks fit one PR within the stated estimate. Retain the proposed scanner slices. Resolve the caller-cutover boundary before implementation; no additional split is otherwise needed.

Definition of done

Planning coverage only: checked boxes indicate coverage by the ACs and Verify plan, not completed work.

Round: 3 of 3

Refinement: NOT READY — AC5 still differs from the shell’s check behavior

Checked

Verified origin identifies tbhb-dev/devtools. Read the live body, refinement:active label and all three comments once. Applied the explicit redesigned-body restart.
Compared every live AC with the shell installer on origin/main and with the other ACs:
- AC1–AC4 match installation, repeat-install state, legacy recovery and refusal behavior at .devtools/scripts/install-pre-push.sh:97.
- AC5 has the parity discrepancy below at .devtools/scripts/install-pre-push.sh:88.
- AC6 matches warning and CI handling at .devtools/scripts/install-pre-push.sh:27.
- AC7 matches conditional commit linting at .devtools/scripts/install-pre-push.sh:23 and .devtools/scripts/install-pre-push.sh:57.
- AC8 matches scanner execution and downstream argument/input forwarding at .devtools/scripts/install-pre-push.sh:65. Its revised recording assertions prove forwarding.
- AC9 matches the existing platform matrix at .github/workflows/ci.yml:175.
Every AC ID appears beside a relevant verification check. Resulting-state parity resolves the earlier repeat-install conflict, and the task-body switch is consistently deferred.
The approach follows Replace complex shell with tested Go, starting with gates and merging #588, extends the existing command at internal/agentrepo/command.go:19, and adds no new shell logic, unnecessary guard, configuration knob or layer.
Replace complex shell with tested Go, starting with gates and merging #588, Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277, Move reusable agent tooling into released Go binaries #174, build(agents): protect replacement Go enforcement paths #753 and feat(gh-post): run the leak review inside every post #525 remain open. The stated rollout-only reasons justify keeping Move reusable agent tooling into released Go binaries #174 and build(agents): protect replacement Go enforcement paths #753 in prose; neither requires a native blocker for this PR.
.agents/agent-skills.conf is owner-tier, but the body assigns that change to build(agents): protect replacement Go enforcement paths #753. No scoped implementation change requires owner-tier authority.
Compatibility, idempotence and portability are addressed. Risks include reverting the PR.

Gaps

Major — AC5 requires stricter comparison than the parity reference. The shell captures both expected and installed text through command substitution, which removes trailing newlines before comparison (.devtools/scripts/install-pre-push.sh:88 and .devtools/scripts/install-pre-push.sh:91). Consequently, an otherwise valid wrapper with extra trailing newlines passes the shell check, while AC5 requires rejection because its bytes differ. Reconcile AC5 and its verification with the shell’s existing behavior, and include a trailing-newline comparison case.

Split

The installer and its tests fit one PR. Retain the proposed scanner slices and deferred task-body switch; no additional split is needed.

Definition of done

Planning coverage only: checked boxes indicate coverage by the ACs and Verify plan, not completed work.

Round: 1 of 3

Refinement: READY — AC5 now preserves the shell’s trailing-newline comparison

Checked

Verified origin identifies tbhb-dev/devtools. Read the live body, refinement:active label and every comment once. Applied the explicit restart-round instruction.
Confirmed every live AC against the shell installer on origin/main and against the other ACs:
- AC1–AC4 match installation, repeat-install state, legacy recovery and refusal behavior at .devtools/scripts/install-pre-push.sh:97.
- AC5 now matches command substitution’s trailing-newline removal at .devtools/scripts/install-pre-push.sh:88 and .devtools/scripts/install-pre-push.sh:91. Its Verify entry includes extra trailing newlines. The previous Major gap is closed.
- AC6 matches warning and CI handling at .devtools/scripts/install-pre-push.sh:27.
- AC7 matches conditional commit linting at .devtools/scripts/install-pre-push.sh:23 and .devtools/scripts/install-pre-push.sh:57.
- AC8 matches scanner execution and downstream forwarding at .devtools/scripts/install-pre-push.sh:65; recording arguments and ref lines proves preservation.
- AC9 matches the existing platform matrix at .github/workflows/ci.yml:175.
Every AC has a relevant verification check. Resulting-state parity, deferred task cutover and forwarding assertions remain consistent with the preceding review. No additional defect was found.
The approach follows Replace complex shell with tested Go, starting with gates and merging #588 and extends agentrepo at internal/agentrepo/command.go:19. It adds no new shell logic or unnecessary guard, configuration knob or layer.
Replace complex shell with tested Go, starting with gates and merging #588, Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277, Move reusable agent tooling into released Go binaries #174, build(agents): protect replacement Go enforcement paths #753 and feat(gh-post): run the leak review inside every post #525 remain open. The stated rollout-only reasons justify prose prerequisites for Move reusable agent tooling into released Go binaries #174 and build(agents): protect replacement Go enforcement paths #753; neither blocks building or merging this PR.
.agents/agent-skills.conf is owner-tier, but its change belongs to build(agents): protect replacement Go enforcement paths #753. No scoped implementation change requires the specified owner-tier authority.
Compatibility, idempotence and portability are covered. Risks include reverting the PR.

Gaps

None.

Split

The installer and its tests fit one PR. Retain the proposed scanner slices and deferred task-body switch; no additional split is needed.

Definition of done

Planning coverage only: checked boxes indicate coverage by the ACs and Verify plan, not completed work.

Round: 2 of 3

Port check-leaks and the pre-push installer to agentrepo #329

Description

Problem

Acceptance criteria

Scope and non-goals

Approach

Dependencies

Size

Verify plan

Risks

Operator authority

Activity

tbhb-agent commented on Oct 31, 2026

Checked

Gaps

Split

Definition of done

tbhb-agent commented on Oct 31, 2026

Checked

Gaps

Split

Definition of done

tbhb-agent commented on Oct 31, 2026

Checked

Gaps

Split

Definition of done

tbhb-agent commented on Oct 31, 2026

Checked

Gaps

Split

Definition of done

tbhb-agent commented on Oct 31, 2026

Checked

Gaps

Split

Definition of done

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions