Skip to content

ci: abandon the pre-push hook, detect the orphan instead (C-340, #426) #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Polichinel wants to merge 5 commits into from
Closed

ci: abandon the pre-push hook, detect the orphan instead (C-340, #426) #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0ddec67
ci: abandon the pre-push hook, detect the orphan instead (C-340, #426)
Polichinel Aug 12, 2026
4465850
ci: the detector could not answer, and said nothing (C-347, C-348, C-…
Polichinel Aug 12, 2026
4ef1f37
fix: the detector was never going to run, and my own guards could not…
Polichinel Aug 12, 2026
3e1fa37
ci: ask one question instead of inferring orphan-ness (C-340, C-350)
Polichinel Aug 12, 2026
5124a4a
docs: correct three false claims, including the one the design rested on
Polichinel Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 168 additions & 6 deletions .github/workflows/release-topology.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ on:
permissions:
contents: read
issues: write
# `pull-requests: read` is load-bearing, not tidiness. Once a `permissions:`
# block exists, every scope NOT listed is set to `none` — so omitting this
# revokes it. The orphan detector below is built entirely on `gh pr list`,
# and its calls are written `... 2>/dev/null) || continue`, which makes a
# 403 indistinguishable from "no PR found": every branch would be skipped
# and the step would report "No orphaned branches" forever, green, on a
# daily cron. It was nearly shipped that way — the drill that verified the
# detector ran under the operator's own `gh` credentials, which carry full
# scope, so it could not have caught this (C-347).
pull-requests: read

concurrency:
group: release-topology
Expand Down Expand Up @@ -129,6 +139,119 @@ jobs:
# TestF7ProductPlanCurrency reads only files and already runs in the
# normal PR suite; it needs no scheduled runner.

- name: Look for a branch that outlived its pull request
id: orphans
# C-340 mechanism 2. A PR auto-merges the instant CI goes green; a
# follow-up commit is then pushed to a branch that no longer has an
# open PR, and the work is simply not on the base branch. `git push`
# reports success. It happened on #416 and again on #437.
#
# THIS ASKS ONE QUESTION, AND THAT IS THE WHOLE DESIGN.
# `delete_branch_on_merge` is on, so GitHub removes a branch when its
# PR merges. A remote branch that still exists and has NO OPEN PR is
# therefore already anomalous, whatever the reason. Report it; let a
# human look.
#
# An earlier version tried to establish that work was *definitely*
# orphaned — matching the merged PR, comparing its head SHA, counting
# commits beyond it. Two review rounds found fifteen defects in that
# version, each from a different property of git, `gh`, or GitHub:
# branch names are reused so the name match was wrong; `--limit 1`
# orders by createdAt not mergedAt; `git fetch <name>` resolves
# `refs/tags/` before `refs/heads/`; `merge-base` exits 128 for a
# missing object and that was booked as a definite answer; and the
# recovery it printed could never clear its own alert. Four versions
# of a pre-push hook died the same way before it. The lesson is not
# any one defect: **precision here requires inferring state we cannot
# see, and every attempt to infer it acquires a new special case.**
# This version cannot be wrong about what it observed, because it
# observes almost nothing.
#
# A branch pushed before its PR is opened is reported. That is a
# false positive by the strict definition and not one in practice —
# it is a branch with work on it and no route to `development`, which
# is the thing worth knowing.
#
# Needs `pull-requests: read` — see the permissions block.
env:
GH_TOKEN: ${{ github.token }}
run: |
set -uo pipefail

# Capture and check, rather than iterating the substitution directly.
# `for x in $(cmd)` swallows a failure of `cmd` completely: GitHub
# runs `bash -e {0}` and `set -uo pipefail` does NOT remove `-e`, but
# a failing command substitution in a `for` word list is exempt from
# it. Verified: `bash -e -c 'for x in $(false | sed s/a/b/); do echo
# BODY; done; echo END'` prints only END. One transient `ls-remote`
# failure would give an empty loop and a confident all-clear.
branches=$(git ls-remote --heads origin | sed 's|.*refs/heads/||') || {
echo "::error::git ls-remote failed — cannot enumerate branches."
exit 1
}
# A repository always has at least `main` and `development`. Empty
# here means the command succeeded and told us nothing, which is not
# the same as "no branches" and must not read as a clean result.
[ -n "$branches" ] || {
echo "::error::git ls-remote returned no branches at all."
exit 1
}

# "Could not ask" is NOT "asked and it is fine". A rate limit or a
# 502 removes a branch from the check silently, and if enough fire
# the step reports a confident all-clear having observed nothing.
found=""
answered=0
unanswered=0
for branch in $branches; do
case "$branch" in main|development) continue ;; esac

open=$(gh pr list --head "$branch" --state open --json number \
--jq '.[0].number // empty' 2>/dev/null) || {
unanswered=$((unanswered + 1)); continue; }
answered=$((answered + 1))
[ -n "$open" ] && continue

found="${found}
- \`${branch}\`"
done

echo "Checked ${answered} branch(es); ${unanswered} could not be answered for."

# Answering for NOTHING while reporting clean is the defect itself,
# not a degraded mode of it.
if [ "$answered" -eq 0 ] && [ "$unanswered" -gt 0 ]; then
echo "::error::Could not answer for any of ${unanswered} branch(es) — reporting clean here would assert nothing."
exit 1
fi

# A partial scan must not write an unqualified all-clear: the close
# step keys on this output, and would close a live orphan issue on
# the strength of a scan that skipped the branch it was about.
[ "$unanswered" -gt 0 ] && echo "::warning::${unanswered} branch(es) could not be checked; this result is partial."
if [ -n "$found" ]; then
# A finding is a finding even from a degraded scan.
echo "orphans=true" >> "$GITHUB_OUTPUT"
elif [ "$unanswered" -gt 0 ]; then
# Nothing found, but we did not look everywhere. NOT 'false':
# the close step keys on 'false' and would close a live orphan
# issue on the strength of a scan that skipped its branch.
echo "orphans=partial" >> "$GITHUB_OUTPUT"
else
echo "orphans=false" >> "$GITHUB_OUTPUT"
echo "No branches without an open pull request."
fi

if [ -n "$found" ]; then
DELIM="ORPHANS_$(openssl rand -hex 8)"
{
echo "list<<${DELIM}"
printf '%s\n' "$found"
echo "${DELIM}"
} >> "$GITHUB_OUTPUT"
echo "::warning::A remote branch has no open pull request."
fi

- name: Summarise the gate result
id: gateresult
run: |
Expand All @@ -153,9 +276,11 @@ jobs:
fi

- name: Open or update the tracking issue
if: steps.check.outputs.diverged == 'true' || steps.gateresult.outputs.failed == 'true'
if: steps.check.outputs.diverged == 'true' || steps.gateresult.outputs.failed == 'true' || steps.orphans.outputs.orphans == 'true'
env:
GH_TOKEN: ${{ github.token }}
ORPHANS: ${{ steps.orphans.outputs.orphans }}
ORPHAN_LIST: ${{ steps.orphans.outputs.list }}
DIVERGED: ${{ steps.check.outputs.diverged }}
COMMITS: ${{ steps.check.outputs.commits }}
SHAS: ${{ steps.check.outputs.shas }}
Expand Down Expand Up @@ -211,10 +336,39 @@ jobs:
)
fi
set -euo pipefail
TITLE="Release hygiene: one or more deploy gates are failing"
TITLE="Release hygiene: one or more checks are failing"
ORPHAN_SECTION=""
if [ "${ORPHANS:-false}" = "true" ]; then
ORPHAN_SECTION=$(cat <<ORPH
## A remote branch has no open pull request

\`delete_branch_on_merge\` is on, so a branch normally disappears when its pull request
merges. These still exist and have **no open pull request**, so nothing is carrying
their work to \`development\`:
${ORPHAN_LIST}

That is C-340 mechanism 2 — a commit pushed to a branch after its PR merged goes
nowhere and \`git push\` reports success (#416, #437). It is also what a branch pushed
before its pull request was opened looks like, which is worth knowing too.

**Either give it a route or remove it.** Both clear this check:

\`\`\`bash
gh pr create --base development --head <branch>
\`\`\`
\`\`\`bash
git push origin --delete <branch>
\`\`\`

If the branch is already gone but its work never reached \`development\`, recover it the
way #417 did — cherry-pick onto a new branch and open a pull request.
ORPH
)
fi
BODY=$(cat <<EOF
${TOPOLOGY_SECTION}
${GATES_SECTION}
${ORPHAN_SECTION}

Detected by [release-hygiene](${RUN_URL}). This issue is reused rather than duplicated,
and closes itself once every check passes again.
Expand All @@ -223,21 +377,29 @@ jobs:
EXISTING=$(gh issue list --state open --search "$TITLE in:title" \
--json number --jq '.[0].number // empty')
if [ -n "$EXISTING" ]; then
gh issue comment "$EXISTING" --body "Still failing as of $(date -u +%Y-%m-%dT%H:%MZ) — diverged=${DIVERGED}, gates_failed=${GATES_FAILED}. [run](${RUN_URL})"
# Include the orphan section. The previous version commented only
# "diverged=false, gates_failed=false" — on a red run, naming no
# branch, so the one actionable fact never reached the reader.
gh issue comment "$EXISTING" --body "$(printf '%s\n\n%s' \
"Still failing as of $(date -u +%Y-%m-%dT%H:%MZ) — diverged=${DIVERGED}, gates_failed=${GATES_FAILED}, orphans=${ORPHANS:-false}. [run](${RUN_URL})" \
"${ORPHAN_SECTION}")"
echo "Updated existing issue #${EXISTING}"
else
gh issue create --title "$TITLE" --body "$BODY"
fi

- name: Close the tracking issue if every check passes
if: steps.check.outputs.diverged == 'false' && steps.gateresult.outputs.failed == 'false'
if: steps.check.outputs.diverged == 'false' && steps.gateresult.outputs.failed == 'false' && steps.orphans.outputs.orphans == 'false'
env:
GH_TOKEN: ${{ github.token }}
run: |
set -euo pipefail
# Both titles: the pre-2026-08-11 one so an issue opened by the
# narrower "Release topology" workflow still closes cleanly.
for TITLE in "Release hygiene: one or more deploy gates are failing" \
# Every title this workflow has opened an issue under, so a rename
# never strands an open issue that nobody closes.
for TITLE in "Release hygiene: one or more checks are failing" \
"Release hygiene: one or more deploy gates are failing" \
"main and development have diverged — back-merge needed"; do
EXISTING=$(gh issue list --state open --search "$TITLE in:title" \
--json number --jq '.[0].number // empty')
Expand All @@ -249,7 +411,7 @@ jobs:
done

- name: Fail the run so the problem is visible in the Actions tab
if: steps.check.outputs.diverged == 'true' || steps.gateresult.outputs.failed == 'true'
if: steps.check.outputs.diverged == 'true' || steps.gateresult.outputs.failed == 'true' || steps.orphans.outputs.orphans == 'true'
run: |
echo "::error::A release-hygiene check failed — see the tracking issue."
exit 1
53 changes: 42 additions & 11 deletions docs/guides/publishing_to_pypi.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,23 +115,54 @@ gh api -X PUT repos/views-platform/views-datafactory/branches/<branch>/protectio

---

## One-time setup — the pre-push hook
## Work orphaned on a merged branch — detected, not prevented

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 simply not on `development`.
That is C-340 mechanism 2, and it has happened twice — #416, and again on #437 while #437 was
fixing it.

**There is no client-side guard, deliberately.** A pre-push hook was written four times and
abandoned; each version was defeated by a different property of the environment. A client check
races GitHub's asynchronous branch deletion, which is a property of the system rather than a bug to
iterate out. The register entry for C-340 records all four so nobody rebuilds them.

**It is not yet running.** GitHub executes a `schedule` trigger from the **default branch only**, and
this repository's default branch is `main`. The check lands on `development`, so the 06:00 cron goes
on running `main`'s copy until the next release promotion carries it across (C-350). Until then,
trigger it by hand:

```bash
git config core.hooksPath scripts/git-hooks
gh workflow run release-topology.yml --ref development
```

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.
**`--ref` is not optional here.** Without it `gh` dispatches the *default branch's* copy — `main`, which has no
detector — and the run goes green having executed none of it, which reads exactly like a clean result.

**What it checks, once live.** `delete_branch_on_merge` is on, so a branch normally disappears when
its pull request merges. `release-topology.yml` therefore asks one question of every remote branch:
**is there an open pull request for it?** If not, it says so, and folds the branch into the same
tracking issue as the other release-hygiene checks.

That is deliberately blunter than "is this work orphaned". A first version tried to establish
orphan-ness properly — matching the merged pull request, comparing head SHAs, counting commits
beyond it — and two review rounds found fifteen defects in it, each from a different property of
git, `gh`, or GitHub. Precision there requires inferring state we cannot see. This version reports a
branch you pushed before opening its PR, which is not really a false positive: it is a branch with
work on it and no route to `development`.

**Clearing it takes one command, either way:**

`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
gh pr create --base development --head <branch>
```

```bash
git push origin --delete <branch>
```

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`.
If a branch is already gone but its work never reached `development`, recover it the way #417 did —
cherry-pick onto a new branch and open a pull request. twice in ~440 PRs — and the recovery figure this entry first cited was wrong. **Measured:** #416 merged 2026-08-03T11:24:10Z and its recovery #417 was not opened until 2026-08-04T01:08:49Z, **13h 44m later**; #437's recovery #438 was closed unmerged and that work never landed at all (it became moot when the hook was deleted). The claim *"both recovered by cherry-pick inside the hour"* was repeated in this register, the changelog and the guide, and it was the only quantitative basis offered for abandoning the guard. Corrected 2026-08-12 by `/code-review max`. It cuts both ways and both are worth stating: 14 hours to notice makes a daily detector comparable rather than clearly worse, **and** it removes the "cheap to recover" premise the abandonment argument leaned on.

## Arming auto-merge — use the script, not `gh pr merge`

Expand Down
Loading
Loading