diff --git a/docs/guides/publishing_to_pypi.md b/docs/guides/publishing_to_pypi.md index 0f4ba1d..ffd409b 100644 --- a/docs/guides/publishing_to_pypi.md +++ b/docs/guides/publishing_to_pypi.md @@ -115,23 +115,26 @@ gh api -X PUT repos/views-platform/views-datafactory/branches//protectio --- -## One-time setup — the pre-push hook +## Pushing to a branch whose pull request already merged -```bash -git config core.hooksPath scripts/git-hooks -``` +A pull request auto-merges the instant CI goes green. Push a follow-up commit and it lands on a +branch with no open PR: `git push` reports success and the work is not on `development`. This has +happened twice in ~440 pull requests (#416, #437). + +**There is no guard, deliberately.** A pre-push hook was written four times and a scheduled detector +three times; every version was defeated by a different property of git, `gh` or GitHub — reused +branch names, merge-commit ancestry, `delete_branch_on_merge` removing the branch before the next +push, `git fetch` resolving `refs/tags/` first. The guards cost far more than the failures they +prevent. C-340 records the attempts so nobody rebuilds them. -Run this once per clone. `core.hooksPath` is per-clone config that git does **not** version, so a -fresh clone has no hooks until you set it. +**If it happens, recover it:** -`scripts/git-hooks/pre-push` **refuses a push to a branch whose pull request has already merged.** -That is C-340 mechanism 2, and it happened: #416 merged the instant CI went green, a follow-up -commit was pushed to that branch, and two pieces of work were simply not on `development`. -`git push` reported success. The only signal was a PR showing one commit when two had been pushed. +```bash +git checkout -b origin/development +git cherry-pick +``` -It **allows** the push whenever it cannot answer — `gh` absent, unauthenticated, or offline — and -says why. A hook that blocks work when it does not know gets uninstalled within a day, and then it -guards nothing. Bypass with `git push --no-verify`. +Both real incidents were recovered this way. ## Arming auto-merge — use the script, not `gh pr merge` diff --git a/scripts/git-hooks/pre-push b/scripts/git-hooks/pre-push deleted file mode 100755 index c710a0d..0000000 --- a/scripts/git-hooks/pre-push +++ /dev/null @@ -1,72 +0,0 @@ -#!/usr/bin/env bash -# Refuse a push to a branch whose pull request has already merged (C-340). -# -# WHY THIS IS A HOOK AND NOT ADVICE -# #416 merged the instant CI went green. A follow-up commit was then pushed -# to that branch, which by then had no open PR. Two pieces of work were -# simply not on `development`. `git push` reported success; the only signal -# was a PR showing one commit when two had been pushed. -# -# Everything else in epic #421 is a test or a workflow — it runs whether or -# not anyone remembers. A checklist item would be the same class of defect -# this epic exists to remove, so this is a hook: it cannot be forgotten. -# -# WHY IT ALLOWS THE PUSH WHEN IT CANNOT ANSWER -# No `gh`, no auth, no network -> allow. A hook that blocks work when it -# does not know is worse than the problem it solves, and it would be -# uninstalled within a day. Same idiom as the deploy gates (C-320): skip -# where the environment cannot answer, and say why. -# -# Install: git config core.hooksPath scripts/git-hooks -# Bypass: git push --no-verify - -set -uo pipefail # deliberately NOT -e: every failure path below must fall - # through to "allow", never abort the push by accident. - -branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null) || exit 0 -[ -n "$branch" ] && [ "$branch" != "HEAD" ] || exit 0 - -# Long-lived branches are pushed to constantly and never have their own PR. -case "$branch" in - main|development) exit 0 ;; -esac - -command -v gh >/dev/null 2>&1 || { - echo "pre-push: gh not installed — cannot check for a merged PR, allowing." >&2 - exit 0 -} - -merged=$(gh pr list --head "$branch" --state merged \ - --json number,mergedAt --limit 1 2>/dev/null) || { - echo "pre-push: gh could not answer (auth or network) — allowing." >&2 - exit 0 -} - -# Empty list, or anything unparseable, means "no merged PR" — allow. -case "$merged" in - ""|"[]") exit 0 ;; -esac - -number=$(printf '%s' "$merged" | sed -n 's/.*"number":\([0-9]*\).*/\1/p') -when=$(printf '%s' "$merged" | sed -n 's/.*"mergedAt":"\([^"]*\)".*/\1/p') -[ -n "$number" ] || exit 0 # could not parse -> do not block - -cat >&2 < - git push -u origin - - If you are certain you want this push: git push --no-verify - -EOF -exit 1 diff --git a/tests/test_git_hooks.py b/tests/test_git_hooks.py deleted file mode 100644 index f0665e1..0000000 --- a/tests/test_git_hooks.py +++ /dev/null @@ -1,69 +0,0 @@ -"""Guard: the pre-push hook stays installable and stays fail-open. - -The hook itself is the machinery for C-340 mechanism 2 — it runs on every -push and cannot be forgotten. What a test can add is narrow but real: - -**It must remain executable.** Git silently ignores a hook without the -executable bit. No error, no warning — the protection just stops, and the -next push to a merged branch orphans a commit exactly as #416's did. A -mode lost to a rebase, a patch, or a filesystem copy is invisible. - -**It must remain fail-open.** Every path where the hook cannot answer — -no ``gh``, no auth, no network — has to allow the push. A hook that -blocks work when it does not know gets uninstalled within a day, and -then it protects nothing at all. This asserts the escape hatches are -still present rather than that they still work; the behaviour is drilled -separately, in the PR. - -**The install step must stay documented.** ``core.hooksPath`` is per-clone -config that git does not version, so a hook nobody installs is a file -nobody runs. -""" - -from __future__ import annotations - -import os -from pathlib import Path - -REPO = Path(__file__).resolve().parents[1] -HOOK = REPO / "scripts" / "git-hooks" / "pre-push" -ARM = REPO / "scripts" / "arm_automerge.sh" -GUIDE = REPO / "docs" / "guides" / "publishing_to_pypi.md" - - -class TestHookStaysUsable: - def test_hook_exists_and_is_executable(self) -> None: - assert HOOK.is_file(), f"{HOOK} is missing — C-340 mechanism 2 is unguarded" - assert os.access(HOOK, os.X_OK), ( - f"{HOOK} has lost its executable bit. Git ignores a non-executable " - f"hook SILENTLY — no error, no warning, the protection simply " - f"stops. Restore with `chmod +x`." - ) - - def test_arm_helper_exists_and_is_executable(self) -> None: - assert ARM.is_file(), f"{ARM} is missing" - assert os.access(ARM, os.X_OK), f"{ARM} has lost its executable bit" - - def test_hook_still_fails_open(self) -> None: - """Every 'cannot answer' path must allow the push.""" - text = HOOK.read_text() - for needle, why in [ - ("command -v gh", "must check gh exists before using it"), - ("gh not installed", "must say why it is allowing when gh is absent"), - ("could not answer", "must say why it is allowing when gh cannot auth"), - ("--no-verify", "must tell the operator the escape hatch"), - ]: - assert needle in text, ( - f"pre-push hook no longer contains {needle!r} — it {why}. " - f"A hook that blocks work when it cannot answer is worse than " - f"the problem it solves; it gets uninstalled, and then it " - f"guards nothing (C-320's lesson applied to a hook)." - ) - - def test_install_step_is_documented(self) -> None: - assert "core.hooksPath" in GUIDE.read_text(), ( - "publishing_to_pypi.md no longer documents " - "`git config core.hooksPath scripts/git-hooks`. That setting is " - "per-clone and git does not version it, so an undocumented hook " - "is a file nobody installs." - )