Skip to content

Git-safety parity: exercise both the native git-hooks layer and Claude Code PreToolUse gates against one harness set #331

Description

@arndvs

Summary

Two independent git-safety enforcement stacks ship in this repo and nothing verifies they agree. Native git hooks (git-hooks/, installed via core.hooksPath -> ~/dotfiles/git-hooks) enforce public-push and direct-to-main guards; the Claude Code PreToolUse hook (hooks/git-workflow-gate.sh) blocks commit-to-main and forced-branch writes through a per-repo .ctrlshft config that is re-parsed independently. The two layers share no contract test.

Today a bypass in one layer (any agent whose PreToolUse hooks never fire e.g. VS Code / Copilot run_in_terminal) is silent until a bad push reaches the remote — which is exactly the wrong-base-merge (~125-file) incident git-hooks/pre-push + bin/verify-pr-base.sh were authored to fix.

Why it's a deepening, not cleanup

  • Two authorities for the same invariant, each unmeasured against the other.
  • ADR-007 (executable-doc-policy, Accepted): every enforced convention earns a CI check. The repo has no CI harness on the native git-hooks/ layer — only the intra-repo byte-identity check in test/git-hooks-drift.sh. That check enforces copy-coupling, not semantic parity with the PreToolUse hook.
  • Single source of truth is claimed (.ctrlshft) but re-written ad hoc in 3+ places (_commit_types(), _protected_branches(), per-layer CTRL_ALLOW_*). hooks/git-workflow-gate.sh has zero reference to the shared remote-URL matchers (is_public/private_ctrlshft_remote_url in bin/_lib.sh), and bin/verify-pr-base.sh has exactly one caller (the native pre-push).

Proposed

  1. A parity harness, not new fixer logic. Stand up a tabular test corpus (one row per protected invariant): e.g. push to public ctrlshft remote without CTRL_ALLOW_PUBLIC_PUSH=1 -> safe at pre-push; a wrong-base-merge branch push -> unsafe at pre-push; commit to main with conventional-format -> blocked/handled at both layers. Each case drives the native dispatcher AND the PreToolUse handler with the analogous input, asserting accept/deny agreement.
  2. Reuse the pipeline's canonical contract: route verify-pr-base.sh and the .ctrlshft parsers under the same declaration (pipeline-states.ts + a small artifact renderer) so branch/base rules are generated, not typed in two syntaxes.
  3. Factor the CLI secrets gate: make git-workflow-gate.sh compute the push guard from the same remote-URL matchers that git-hooks/pre-push and bin/validate-remotes.sh already share — remove the current path where the Claude Code layer is blind to ctrlshft remotes.
  4. Wire the harness into CI so a drift of either layer fails the eval.

Out of scope:

  • Removing either layer (both serve different runtime shapes — per the comment trail in hooks/README.md).
  • Re-architecting two-into-one single hook.

Acceptance

  • The harness runs in CI and returns green on the eval base.
  • A matrix row covering a known per-layer gap (e.g. a native-path commit to main, a PreToolUse-only force-push) is registered and enforced at the opposite layer as needed.
  • The remote-URL matcher helpers and protection rules are single-sourced in bin/_lib.sh (or generated from pipeline-states.ts), none re-parsed ad hoc.

Reviewed ADRs

ADR-007 (executable-doc policy) directly supports this. ADR-001/002/003/004/005/006 not bound. CODING_STANDARDS.md shell sourcing and set -euo pipefail convention respected.

Activity

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

    source:architecture-reviewPRDs proposed by the automated architecture-review workflow

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions