Skip to content

Repository navigation

refactor(hooks): port pre-push installation and execution to Go #819

New issue
New issue
Open
Feature
Open
refactor(hooks): port pre-push installation and execution to Go#819
Feature
Labels
change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

Description

@tbhb-agent
tbhb-agent
opened on Oct 26, 2026

Problem

Part of #588. This is the pre-push installer and installed-hook slice of #329.

At origin/main (a0347a715fab580ece45edfcb3d13509620459a8), the installer recognizes and preserves prek hooks, checks the installed wrapper and emits a runtime that replays Git's ref input (.devtools/scripts/install-pre-push.sh:20, .devtools/scripts/install-pre-push.sh:50, .devtools/scripts/install-pre-push.sh:86). This remains shell logic on every push. The shared-worktree correction in #277 is required by #329 and must survive this port.

Acceptance criteria

  1. AC1: Installing or refreshing places the devtools checks before prek, remains idempotent across a prek reinstall and the recognized legacy upgrade, and never overwrites an unrelated hook or removes unrelated legacy content.
  2. AC2: --check retains its installed/missing/mismatched/read-failure outcomes, stderr diagnostics, warning-only behavior only for DEVTOOLS_LEAK_HOOK_CHECK=warn, and successful CI bypass before inspecting local hooks, subject to Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277's version-aware correction.
  3. AC3: When commit-message pre-push checking is configured, the installed hook checks the required linter before reading stdin and retains the current actionable missing-linter diagnostic; without that configuration it does not require that linter.
  4. AC4: Commit-message checking, optional leak checking and prek receive the same original ref-input bytes in the current order, with their current arguments; failure stops subsequent checks and blocks the push.
  5. AC5: A branch without the leak checker retains its warning-and-continue behavior, and consumers without leak tasks retain their existing prek-only installation behavior; the installer adds no new scanner invocation.
  6. AC6: Installation, checking and hook execution work from linked worktrees and the configured hooks location, without embedding an installing worktree's path into behavior used by another worktree.
  7. AC7: Failed hook reads, stdin buffering, temporary storage, permission changes or hook writes return nonzero rather than presenting an incomplete hook as successfully installed or a failed scan as clean.
  8. AC8: Non-functional: the installer, check command and installed runtime end with thin shell entry points that probe their Go subcommand without writes or consuming Git input, use the frozen shell when binary or capability is absent, and preserve supported execution's streams and status with no fallback after failure or partially completed installation.
  9. AC9: The version-aware shared-hook outcomes required by Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277 remain true: newer recognized wrappers pass checking and cannot be downgraded, older wrappers require upgrade, and displaced-wrapper repair and shared-worktree notices remain available.
  10. AC10: Non-functional: published task names, compatibility aliases and argument boundaries remain unchanged on N-2 consumers.
  11. AC11: Consumer documentation describes installation after prek, shared-hook behavior, the fallback window, repair and rollback without requiring new consumer configuration.
  12. AC12: Non-functional: the Go behavior works on Linux, macOS and Windows.

Scope and non-goals

In scope: installation, check mode, the emitted hook's runtime behavior and existing task entry points. Evidence: .devtools/scripts/install-pre-push.sh:23, .devtools/scripts/install-pre-push.sh:36, .devtools/scripts/install-pre-push.sh:41, .devtools/scripts/install-pre-push.sh:57, .devtools/scripts/install-pre-push.sh:62, .devtools/scripts/install-pre-push.sh:97, .devtools/tasks/git-hooks.toml:26, .devtools/tasks/git-hooks.toml:30 and .devtools/tasks/leaks.toml:15.

Coverage: installer/runtime cases at .devtools/scripts/check-leaks.bats:960 through .devtools/scripts/check-leaks.bats:1332, stdin failure at .devtools/scripts/check-leaks.bats:1414, foreign-hook protection at .devtools/scripts/check-leaks.bats:1452, and bootstrap ordering at tests/tasks/bootstrap.bats:10. Retain a small integration suite using disposable local Git destinations; scanner findings themselves belong to the other #329 slices.

Overlap: AC4 preserves the leak-gate invocation boundary used by #501; no new source-loading or provisioning AC from #501 is absorbed. Those remain #501. AC5 preserves scanner composition while #551 owns gitleaks modes and caller selection; no #551 scan-selection or coverage-reduction AC is absorbed. Neither overlap issue closes here.

Non-goals: reimplementing either scanner or the commit-message linter, changing Git's shared-hook architecture, automatically configuring per-worktree hooks, changing hook stages, adding gitleaks to pre-push or preventing deliberate hook bypass. AC9 is an inherited dependency requirement, not a claim that #277's correction is already present in the evidence baseline. Freeze installer parity against the landed #277 behavior before this port. Fallback retirement requires a separate follow-up under D9 after the N-2 window permits it.

Approach

Move the scoped behavior to tested Go, with failing cases first. The mechanism is the implementer's choice within these outcomes. Keep the fallback frozen at the required predecessor's behavior.

Dependencies

Blocked by #277 for the shared-hook correction and by #753 before activating replacement enforcement. The implementation PR must cite both merged prerequisites. #329 tracks the migration. #501 and #551 retain the excluded scanner changes; #661 governs compatibility and #333 supplies distribution without raising this port's binary floor.

Size

M. One pull request for the installer and the runtime it installs.

Verify plan

Write failing Go cases for lost ref input, an unrelated saved hook and an attempted shared-hook downgrade first. Port all scoped Bats cases before reducing shell coverage.

  • AC1: initial install, repeat install, prek reinstall, recognized older marker, stale devtools legacy wrapper, foreign main hook, foreign saved prek hook and foreign legacy content; compare hook contents, modes, statuses and notices with the frozen predecessor.
  • AC2: valid, missing and mismatched wrappers, absent hook directory, installed content emitted before a failing read, warning value set/unset/other, and nonempty CI; compare stdout and stderr separately and prove CI bypass performs no hook inspection.
  • AC3: configured and unconfigured commit-message checking, absent/unreadable configuration, commented or extended ID text that the frozen check recognizes, missing linter and a recording linter; compare the enabled/disabled result with shell. Assert the diagnostic and no stdin read before the required-tool refusal.
  • AC4, AC5: recording commit checker, leak task and prek with multiple ref lines and both arguments. Assert byte-identical input to each, exact order, immediate stop after either check fails, final prek status propagation and no duplicate invocation. Cover absent checker with warning, no-leak-task consumer, deletion-only and zero-line input, quiet task arguments and each destination in a multi-URL push.
  • AC6: main and two linked worktrees, hooks paths containing spaces and a configured hooks location; install in one and execute/check from the others, with no worktree-specific path leakage.
  • AC7: injected read, copy, temporary-file, permission and write failures; assert nonzero, no downstream checker/prek after input failure, preservation of unrelated files and no success report for an incomplete install.
  • AC8: missing binary, old binary without capability, supported success, and supported output-then-failure before and after an installation mutation. Exercise old/new installer, checker and runtime combinations, including a Go-installed hook executed with an older binary and from a branch predating the leak payload. Assert frozen behavior remains available, probe consumes no ref bytes and performs no mutation, supported failure never triggers shell replay or a second write, and probe errors refuse instead of guessing absence.
  • AC9: carry each Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277 fixture into Go: newer-wrapper check success, downgrade refusal with unchanged hook, older-wrapper upgrade refusal/warning, recognized legacy forms, displaced wrapper repair and shared/unmerged-worktree notice. Confirm this uses the merged Shared worktree hooks: an unmerged branch's installer breaks pushes from every other worktree #277 contract rather than the older baseline's exact-text check.
  • AC10: compare all primary and compatibility task names and arguments; run the bootstrap-order regression and unchanged N-2 fallback fixtures.
  • AC11: verify documented installation, repair and rollback in disposable fixtures and compare help with the actual entry points.
  • AC12: native platform tests and shell parity where available. Record 20 warm installer/check invocations and hook invocations with recording tools against the same shell baseline; investigate regressions.
  • Run the branch binary through mise run test-leaks, mise run test-hooks tests/tasks/bootstrap.bats, then mise run check, fresh coverage and both coverage floors. Verify build(agents): protect replacement Go enforcement paths #753 protects replacement enforcement and registration before activation. Apply ordered generation and generated-output checks if maintained skill sources change.
  • Definition of done: D1 each AC is verified at the merged head, with the commands and results cited in the pull request's Verification. D2 an independent review approves the exact merged head, and every finding has a recorded disposition. D3 required checks pass with no gate loosened. D4 the leak scans the repository specifies are clean, and every post made for this work (comments, descriptions) passes the per-post leak review before it is posted. D5 every commit is signed. D6 the documentation matches, or the description says No doc impact with the reason. D7 every commit carries an Assisted-by trailer and the pull request carries the provenance:agent-assisted label. D8 the change stays within the pull request size limits. D9 follow-up work, including deferred review findings, is filed as issues, each with a Problem section and numbered acceptance criteria and linking this issue; the pull request's Related section lists them, and any code that defers work links its issue. D10 the merge closes this issue through a closing reference, the project board shows the state Done, and a final Status: comment names the merge commit.

Risks

Shared hooks affect every worktree, and a repeated install after partial failure can destroy useful hook content. Version, foreign-hook and no-replay cases are essential. Older binaries deliberately retain their frozen predecessor behavior. Undo: revert this PR and reinstall the previously supported wrapper after prek in a disposable verification fixture before documenting the consumer repair.

Operator authority

standing rule names the operator: replacement enforcement requires #753's protection and its exact-head owner-review requirements before activation; this port changes no identity or approval authority.

Activity

  1. tbhb-agent
    added
    change:featPR adds or changes a feature (Conventional Commit type feat)
    on Oct 26, 2026
  2. github-project-automation
    added this to devtoolson Oct 26, 2026
  3. github-project-automation
    moved this to Backlog in devtoolson Oct 26, 2026
  4. tbhb-agent
    added the
    Feature
    issue type on Oct 26, 2026
  5. tbhb-agent
    added a parent issue on Oct 26, 2026

  6. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    tbhb-agent
    on Oct 26, 2026
    ContributorAuthor

    Refinement: READY — pre-push installer and hook runtime Go port requirements, #277/#753 blockers, and verification plan are complete
    Model: gemini-4-argon-high

    Checked

    • Live issue and referenced issues: Read live tbhb-dev/devtools#819 (0 comments) and referenced issues #588, #329, #277, #501, #551, #753, #661, #333, plus sibling #329 slices #817 and #818. Verified against origin/main at a0347a715fab580ece45edfcb3d13509620459a8.
    • Problem and baseline evidence: Confirmed all origin/main citations in .devtools/scripts/install-pre-push.sh:20 (wrapper markers), .devtools/scripts/install-pre-push.sh:50 (wrapper() emitting the shell pre-push runtime), and .devtools/scripts/install-pre-push.sh:86 (--check and install flow).
    • Acceptance criteria (AC1–AC12):
      • AC1 covers idempotent wrapper installation before prek (.devtools/scripts/install-pre-push.sh:97-119), v0.12 legacy wrapper cleanup (.devtools/scripts/install-pre-push.sh:21, .devtools/scripts/install-pre-push.sh:106-108), and refusal to overwrite foreign pre-push or pre-push.prek hooks or delete foreign pre-push.legacy files (.devtools/scripts/install-pre-push.sh:99-102, .devtools/scripts/install-pre-push.sh:109-116, .devtools/scripts/check-leaks.bats:1184-1192, .devtools/scripts/check-leaks.bats:1324-1330, .devtools/scripts/check-leaks.bats:1452-1462).
      • AC2 covers --check outcomes, stderr diagnostics, DEVTOOLS_LEAK_HOOK_CHECK=warn warning-only behavior (.devtools/scripts/install-pre-push.sh:27-34, .devtools/scripts/check-leaks.bats:1274-1294), CI bypass before hook inspection (.devtools/scripts/install-pre-push.sh:36-39, .devtools/scripts/check-leaks.bats:1264-1272), and read-failure rejection (.devtools/scripts/install-pre-push.sh:88-91, .devtools/scripts/check-leaks.bats:1312-1322), qualified by #277's version-aware comparison.
      • AC3 covers conditional commit-msg-pre-push detection in .pre-commit-config.yaml (.devtools/scripts/install-pre-push.sh:22-25) and the pre-stdin commit-lint availability check and diagnostic (.devtools/scripts/install-pre-push.sh:57-60, .devtools/scripts/check-leaks.bats:1236-1262).
      • AC4 and AC5 cover buffering and replaying identical stdin ref lines and arguments in order across commit-lint --pre-push "${1-}", mise run --quiet devtools:lint-leaks -- --pre-push-hook "$@", and "$hooks/pre-push.prek" "$@" (.devtools/scripts/install-pre-push.sh:65-77, .devtools/scripts/check-leaks.bats:960-986, .devtools/scripts/check-leaks.bats:1203-1234), warning-and-continue when .devtools/scripts/check-leaks.sh is absent on an older branch (.devtools/scripts/install-pre-push.sh:72-76, .devtools/scripts/check-leaks.bats:1102-1113), and preserving prek-only installation for consumers without leak tasks (.devtools/scripts/check-leaks.bats:1136-1146).
      • AC6 covers linked worktrees and core.hooksPath resolution via git rev-parse --git-path hooks and runtime dirname "$0" resolution (.devtools/scripts/install-pre-push.sh:41-48, .devtools/scripts/install-pre-push.sh:61, .devtools/scripts/check-leaks.bats:1296-1310, .devtools/scripts/check-leaks.bats:1332-1342).
      • AC7 covers fail-closed nonzero returns on hook read, mktemp, stdin copy (cat >"$input"), permission, or write errors (.devtools/scripts/install-pre-push.sh:62-68, .devtools/scripts/install-pre-push.sh:88-91, .devtools/scripts/install-pre-push.sh:117-118, .devtools/scripts/check-leaks.bats:1312-1322, .devtools/scripts/check-leaks.bats:1414-1427).
      • AC8, AC10, and AC12 state the non-functional requirements required by #588, #329, and #661: thin shell entry points that probe Go support without writes or consuming stdin, frozen-shell fallback when binary or capability is absent, no fallback or duplicate write after supported execution failure, unchanged task names/aliases (devtools:prek-install at .devtools/tasks/git-hooks.toml:26, devtools:check-pre-push-hook at .devtools/tasks/git-hooks.toml:30, devtools:install-pre-push-hook at .devtools/tasks/git-hooks.toml:35, devtools:lint-leaks at .devtools/tasks/leaks.toml:2, devtools:check-leak-hook at .devtools/tasks/leaks.toml:16, devtools:install-leak-hook at .devtools/tasks/leaks.toml:21, and bootstrap ordering at mise.toml:146-154 / tests/tasks/bootstrap.bats:10), and Linux/macOS/Windows support.
      • AC9 and #277 dependency: #819 must depend on #277, and the live body states this dependency accurately. On origin/main, .devtools/scripts/install-pre-push.sh:88-95 still requires exact wrapper string equality and .devtools/scripts/install-pre-push.sh:117 overwrites .git/hooks/pre-push without downgrade protection, displaced-wrapper repair diagnostics, or shared-worktree notices. Because #329 requires #277's shared-hooks fix and #819 freezes the predecessor shell fallback (AC8) while porting the installer to Go (AC9), #277 must land before #819 freezes parity. #819 properly lists #277 as a blocking dependency in ## Dependencies, scopes AC2 and AC9 to #277's version-aware outcomes, notes in ## Scope and non-goals that #277 is not yet in the origin/main baseline so parity must freeze against the landed #277 contract, and verifies all #277 fixtures in ## Verify plan.
      • AC11 covers consumer documentation (README.md:281-285, docs/commit-lint.md:40-44).
    • Requirements-only rule: Each AC specifies observable outcomes and non-functional constraints without prescribing internal Go package layout, file structure, or library choices; ## Approach explicitly leaves mechanism to the implementer.
    • Scope, non-goals, and dependencies: Scope citations (.devtools/scripts/install-pre-push.sh:23, :36, :41, :57, :62, :97; .devtools/tasks/git-hooks.toml:26, :30; .devtools/tasks/leaks.toml:15; .devtools/scripts/check-leaks.bats:960-1332, :1414, :1452; tests/tasks/bootstrap.bats:10) match origin/main. Boundaries with #817 and #818 (which own check-leaks.sh scanning modes while #819 keeps the devtools:lint-leaks -- --pre-push-hook invocation boundary), #501 (worktree private-string loading), #551 (devtools:gitleaks mode scoping), #753 (replacement enforcement path protection in .agents/agent-skills.conf:4-9), #661 (docs/compatibility.md:1-20), and #333 (release binary assets) are all accurate.
    • Verify plan, risks, undo, and operator authority: Every AC (AC1–AC12) has concrete verification checks proving its outcome, risks and undo steps are actionable, and ## Operator authority names #753's exact-head owner-review requirement for replacement enforcement protection.

    Gaps

    None

    Split

    None needed. Size M fits a single pull request covering the .devtools/scripts/install-pre-push.sh (119 lines) port, its Go unit/integration tests, task/wrapper entry points, and consumer documentation, with scanner implementations already separated into #817 and #818 and the shared-hook shell fix separated into #277.

    Definition of done

    The definition of done for #819 (Feature). Tick each item and give its evidence. (Each ticked box below records planning coverage in the issue's criteria and verify plan, not completed implementation work.)

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link. — Planning coverage: ## Verify plan maps AC1 through AC12 to concrete test fixtures and commands (mise run test-leaks, mise run test-hooks tests/tasks/bootstrap.bats, mise run check, coverage-floor tasks) and requires citing each AC ID's command and result on the merged head under D1.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. — Planning coverage: ## Verify plan (Definition of done D2) requires independent review approval on the exact merged head with recorded dispositions for every finding.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. — Planning coverage: ## Verify plan requires mise run check, fresh coverage, both coverage floors, and D3 required checks passing with no loosened gate; ## Operator authority requires #753's protection and exact-head owner review before replacement enforcement activation.
    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post. — Planning coverage: ## Verify plan runs mise run test-leaks and mise run check and specifies under D4 that repository leak scans and per-post leak reviews are clean before posting.
    • D5 The author signed each commit, the squash included. — Planning coverage: ## Verify plan (Definition of done D5) requires every commit, including the squash commit, to be signed.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Planning coverage: AC11 and its ## Verify plan item require updating and verifying consumer documentation for installation after prek, shared-hook behavior, fallback window, repair, and rollback, plus D6 verification.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. — Planning coverage: ## Verify plan (Definition of done D7) requires Assisted-by trailers on every commit and the provenance:agent-assisted label on the pull request.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. — Planning coverage: ## Size scopes the work to a single M pull request limited to the installer and installed hook runtime, and ## Verify plan (Definition of done D8) requires staying within the repository's PR size limits.
    • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue. — Planning coverage: ## Scope and non-goals specifies a separate follow-up issue under D9 for retiring the frozen shell fallback after the N-2 window, and ## Verify plan (Definition of done D9) requires filing follow-ups and deferred findings with Problem and numbered ACs linked in the PR's Related section and code comments.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. — Planning coverage: ## Verify plan (Definition of done D10) requires closing the issue via merge reference, verifying the project board shows Done, and posting the final Status: comment naming the merge commit.

    Round: 1 of 3

  7. tbhb-agent
    added
    refinement:readyPassed the Definition of Ready; may be picked up
    on Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

    Type

    Feature

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • devtools
      Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions