From 2bcde9b38b762b7f108405132f47cd31f96f9da1 Mon Sep 17 00:00:00 2001 From: Ken Tobias <634380+l1a@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:03:17 -0700 Subject: [PATCH 1/2] Gate merge-pr on CI; check the triad merge-pr went straight from the branch check to gh pr merge --squash --delete-branch, with no inspection of the status rollup. gh pr merge happily merges a red PR when there is no branch protection, so every merge in this repo has been ungated -- safe only because whoever merged happened to look first. rusticprofile added this in v0.1.5 after a PR went in with a leg red, and extended it in 0.2.1 after an EMPTY rollup passed vacuously. Neither reached here. Three refusals now: a failing check, an empty rollup, and checks still running. The empty state is compared as a string rather than via jq -e length, because an external jq is not on a default Windows PATH and a gate that degrades where its dependency is missing is the thing being fixed. gate_conformance.py (template v3) is vendored and run by standard-check, so the guards cannot vanish again. It is structural, not behavioural, and says so. Verified safely: on a branch with no PR the rollup is empty, so merge-pr refuses before reaching gh pr merge. Assisted-By: Claude Opus 5 --- Cargo.toml | 2 +- Justfile | 41 +++++- NOTES.md | 32 ++++- docs/retch.1 | 2 +- scripts/gate_conformance.py | 241 +++++++++++++++++++++++++++++++++ templates/justfile-common.just | 27 +++- 6 files changed, 334 insertions(+), 11 deletions(-) create mode 100644 scripts/gate_conformance.py diff --git a/Cargo.toml b/Cargo.toml index e633cfe..b0fe776 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ members = [ [package] name = "retch-cli" -version = "0.6.22" +version = "0.6.23" edition = "2021" authors = ["Ken Tobias"] description = "A fast, feature-rich system information fetcher written in Rust (similar to fastfetch or neofetch)" diff --git a/Justfile b/Justfile index 475fbca..5204bae 100644 --- a/Justfile +++ b/Justfile @@ -17,7 +17,7 @@ MAN_PAGES := "docs/retch.1" # vendored helpers, bump their versions, and propagate to the sibling repos in their own PRs. # `just standard-check` runs the helpers' self-tests and `just check` depends on it, so a # violation fails the build rather than being discovered years later. -# >>> COMMON (template v2) +# >>> COMMON (template v3) # The interpreter is resolved ONCE per line, and a missing one is a hard error. The # `python3 … 2>/dev/null || python …` idiom is deliberately NOT used: it retries on ANY # failure, so a real error inside the script gets re-run and reported as if the @@ -88,6 +88,8 @@ standard-check: [ "{{PY}}" != "PYTHON-NOT-FOUND" ] || { echo "error: no python3/python on PATH" >&2; exit 1; } "{{PY}}" scripts/install_completions.py --self-test "{{PY}}" scripts/install_man.py --self-test + "{{PY}}" scripts/gate_conformance.py --self-test + "{{PY}}" scripts/gate_conformance.py "{{justfile()}}" # <<< COMMON # ===== PROJECT-SPECIFIC — everything below is this repo's own ===== @@ -297,6 +299,43 @@ merge-pr: echo "Error: You are already on main." exit 1 fi + # Refuse to merge over a failing check. + # + # `gh pr merge` happily merges a red PR when the repository has no branch protection, and + # "wait for the checks to settle" is not "wait for them to pass". rusticprofile added this + # after PR #19 went in with a leg red; retch never had it, so every merge here has been + # ungated -- safe only because whoever merged happened to look first. + echo "Checking CI on this branch..." + STATES=$(gh pr view --json statusCheckRollup --jq '[.statusCheckRollup[]? | select(.conclusion != "SKIPPED") | .conclusion]' 2>/dev/null || echo '[]') + + # NO checks at all is not "green", and the arm below cannot tell the difference: an empty + # rollup matches neither "" nor FAILURE, so without this the recipe prints "CI is green." + # and merges a commit CI has never seen. That is not hypothetical -- it happened in + # rusticprofile on 2026-08-06, when GitHub stopped creating runs for pushed commits. + # + # Compared as a string rather than piped through `jq -e length`: `gh --jq` is gh's BUILT-IN + # jq, but an external `jq` is not on a default Windows PATH, and a gate that silently + # degrades where its dependency is missing is the thing being fixed, not a way to fix it. + if [ "$(printf '%s' "$STATES" | tr -d '[:space:]')" = "[]" ]; then + echo "Error: no checks have reported for this commit at all." + echo " That is not the same as passing. GitHub sometimes fails to create a run;" + echo " force one with: gh workflow run rust.yml --ref $BRANCH" + exit 1 + fi + + if echo "$STATES" | grep -q '""'; then + echo "Error: checks are still running. Wait for them, or merge deliberately with gh." + exit 1 + fi + + if echo "$STATES" | grep -qE 'FAILURE|TIMED_OUT|CANCELLED|ACTION_REQUIRED'; then + echo "Error: CI is not green on this branch:" + gh pr view --json statusCheckRollup --jq '.statusCheckRollup[]? | select(.conclusion != "SKIPPED" and .conclusion != "SUCCESS") | " \(.conclusion) \(.name)"' + echo "Fix it, or merge deliberately with gh if you have a reason." + exit 1 + fi + echo "CI is green." + echo "Merging PR for branch $BRANCH..." gh pr merge --squash --delete-branch echo "Switching to main and pulling..." diff --git a/NOTES.md b/NOTES.md index 09eaccf..c0de725 100644 --- a/NOTES.md +++ b/NOTES.md @@ -96,7 +96,37 @@ The `retch-sysinfo` crate can be used independently as a library for cross-platf --- -## Current State (v0.6.22) +## Current State (v0.6.23) +- **v0.6.23 — `just merge-pr` had no CI gate, and now the triad is checked** (tooling only; no + runtime change, `retch-sysinfo` unchanged at `0.1.53`). + - **`merge-pr` went straight from the branch check to `gh pr merge --squash --delete-branch`.** + No inspection of the status rollup, in any form. `gh pr merge` will happily merge a red PR when + the repository has no branch protection, and "wait for the checks to settle" is not "wait for + them to pass". **Every merge in this repo has been ungated** — safe only because whoever merged + happened to look at CI first. + - `rusticprofile` added this gate in its `v0.1.5` after a PR went in with a leg red, and extended + it in `0.2.1` after an **empty** rollup passed vacuously — printing "CI is green." over a commit + CI had never seen, which happened for real when GitHub stopped creating runs for pushed commits. + Neither fix reached here. Same cross-repo staleness as the nushell completion path, this time on + the recipe that performs the irreversible act. + - Three refusals now: a failing check, an **empty** rollup (`nothing ran` is not `everything + passed`), and checks still running rather than racing them. The empty-rollup state is compared + as a **string** rather than through `jq -e length`, because `gh --jq` is gh's built-in jq while + an external `jq` is not on a default Windows PATH — and a gate that silently degrades where its + dependency is missing is the thing being fixed, not a way to fix it. + - **`scripts/gate_conformance.py` (template v3) is vendored, and `standard-check` runs it**, so + the guards cannot quietly disappear again. It asserts nine of them across `pr`, `open-pr` and + `merge-pr`, with **comments stripped first** — a comment explaining a guard must not satisfy the + check for a recipe that lost it. + - **It is structural, not behavioural**, and says so: it proves a guard is present, not that it + works. The install helpers are pure functions their self-test can call; these recipes run the + suite, push branches and merge PRs, so executing them from `check` would be slow and + destructive. + - **Verified by running it, safely:** on a branch with no PR the rollup is empty, so `merge-pr` + refuses and exits *before* reaching `gh pr merge` — which tests the gate without merging + anything. The jq expression was also checked by asking gh's own jq to parse it rather than + reasoning about backslash layers. + - `retch-cli` → 0.6.23. Patch bump. - **v0.6.22 — the manual Claude review was available and inert** (CI configuration only; one line removed, no runtime change, `retch-sysinfo` unchanged at `0.1.53`). - `v0.6.17` disabled automatic review by commenting out the `pull_request` trigger **and** diff --git a/docs/retch.1 b/docs/retch.1 index 56bf593..539601f 100644 --- a/docs/retch.1 +++ b/docs/retch.1 @@ -1,4 +1,4 @@ -.TH "RETCH" "1" "August 2026" "retch 0.6.22" "System Information Fetcher" +.TH "RETCH" "1" "August 2026" "retch 0.6.23" "System Information Fetcher" .SH "NAME" .PP diff --git a/scripts/gate_conformance.py b/scripts/gate_conformance.py new file mode 100644 index 0000000..caa6420 --- /dev/null +++ b/scripts/gate_conformance.py @@ -0,0 +1,241 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-3.0-or-later +# Copyright (C) 2026 l1a +"""Assert the pr / open-pr / merge-pr triad still carries its required guards. + +TEMPLATE v3 — vendored verbatim in rusticprofile, retch and etr. Change it here, bump +TEMPLATE_VERSION, and propagate in each repo's own PR. Run by `just standard-check`. + +WHAT THIS IS, AND HONESTLY WHAT IT IS NOT +----------------------------------------- +This is a **structural** check. It reads the justfile and asserts each guard is present. +It does NOT prove the guards work — a recipe could satisfy every assertion here and still +be wrong. + +That limitation is deliberate rather than lazy, and the reason is worth stating: the install +helpers could be checked behaviourally because their logic is pure functions over a path and +an environment, so `--self-test` can call them. The gate recipes are not: `pr` runs the whole +test suite, regenerates the man page and touches git; `open-pr` pushes and opens a PR; +`merge-pr` merges. Executing them from a `check` dependency would be slow, side-effecting and +occasionally destructive. So this checks that the guards are *there*, and the guards +themselves are exercised for real on every PR — which is the strongest thing available +without making `just check` open pull requests. + +It would have caught every one of the drift items found by hand in August 2026: +PR_CONFIRM missing in two repos, open-pr not pushing in one, and merge-pr having no CI gate +at all in two. + +WHY THESE RECIPES ARE NOT SHARED VERBATIM LIKE THE INSTALL FAMILY +----------------------------------------------------------------- +They legitimately differ: `cargo clippy` is bare in rusticprofile, `--workspace` in retch and +`--all-targets` in etr; the NOTES header is `## Current State (vX)` in two and +`## Current state: vX` in the third; retch's checklist has wiki and tldr items, etr's has +PROTOCOL.md, rusticprofile's has neither. Forcing one body would mean changing what each gate +*does*, which is a behaviour change per repo rather than a copy. So what is standardised is +their **behaviour**, and this file is what stops that behaviour drifting apart again. +""" + +import re +import sys +from pathlib import Path + +TEMPLATE_VERSION = 3 + + +def recipe_body(text, name): + """The lines of one recipe, from its header to the next top-level construct. + + Deliberately not a YAML/justfile parser: a recipe body is every line after the header + that is indented or blank, which is the one structural rule just guarantees. + """ + lines = text.splitlines() + start = None + for i, l in enumerate(lines): + if re.match(rf"^{re.escape(name)}(\s+[*+]?\w+.*)?:", l): + start = i + break + if start is None: + return None + body = [] + for l in lines[start + 1:]: + if l.strip() and not l.startswith((" ", "\t")): + break + body.append(l) + return "\n".join(body) + + +def code_of(body): + """Recipe body with comment-only lines removed. + + Load-bearing: every assertion below must run against CODE, never against prose. A comment + explaining a guard would otherwise satisfy the check for a recipe that lost it — which is + exactly how `if: false` inside a comment once read as an active guard during this work. + """ + out = [] + for l in body.splitlines(): + s = l.strip() + if s.startswith("#") or s.startswith("@#"): + continue + out.append(l) + return "\n".join(out) + + +# (recipe, key, human description, predicate over the recipe's CODE) +RULES = [ + ("pr", "confirm-env", + "`pr` READS PR_CONFIRM, so a script or agent can satisfy the gate. Naming it in help " + "text is not honouring it -- the rule requires a parameter expansion.", + lambda c: re.search(r"\$\{?PR_CONFIRM", c) is not None), + ("pr", "confirm-refuses", + "`pr` REFUSES rather than defaulting to yes when there is no terminal and no stdin", + lambda c: re.search(r"PR_CONFIRM", c) and re.search(r"exit 1|fail ", c)), + ("pr", "confirm-explicit-y", + "`pr` still requires an explicit y — widening who can answer, not what counts", + lambda c: re.search(r'CONFIRM"?\s*=\s*"?y', c, re.I) is not None), + ("open-pr", "gate-first", + "`open-pr` runs the gate before creating the PR", + lambda c: re.search(r"just\s+pr\b", c) is not None), + ("open-pr", "creates-pr", + "`open-pr` is the call site that actually creates the PR", + lambda c: "gh pr create" in c), + ("open-pr", "push-if-no-upstream", + "`open-pr` pushes when the branch has no upstream, and only then", + lambda c: "@{upstream}" in c and re.search(r"git push", c) is not None), + ("merge-pr", "refuse-red", + "`merge-pr` refuses to merge over a failing check", + lambda c: re.search(r"FAILURE|TIMED_OUT|CANCELLED", c) is not None), + ("merge-pr", "refuse-empty", + "`merge-pr` refuses an EMPTY status rollup — 'nothing ran' is not 'everything passed'", + lambda c: re.search(r"statusCheckRollup", c) is not None + and re.search(r"\[\]|empty|no checks", c, re.I) is not None), + ("merge-pr", "refuse-pending", + "`merge-pr` refuses while checks are still running rather than racing them", + lambda c: re.search(r"still running|in progress|pending", c, re.I) is not None), +] + + +def check(justfile: Path): + text = justfile.read_text(encoding="utf-8") + failures = [] + for recipe, key, desc, pred in RULES: + body = recipe_body(text, recipe) + if body is None: + failures.append((f"{recipe}:{key}", f"recipe `{recipe}` does not exist")) + continue + if not pred(code_of(body)): + failures.append((f"{recipe}:{key}", desc)) + return failures + + +CONFORMANT = """ +pr: + #!/usr/bin/env bash + if [ -n "${PR_CONFIRM:-}" ]; then CONFIRM="$PR_CONFIRM" + elif [ -t 0 ]; then read -r CONFIRM + else read -r -t 10 CONFIRM || CONFIRM="" + [ -n "$CONFIRM" ] || { echo "set PR_CONFIRM=y"; exit 1; } + fi + [ "$CONFIRM" = "y" ] || exit 1 + +open-pr *ARGS: + #!/usr/bin/env bash + just pr + if ! git rev-parse '@{upstream}' >/dev/null 2>&1; then git push -u origin "$B"; fi + gh pr create + +merge-pr: + #!/usr/bin/env bash + STATES=$(gh pr view --json statusCheckRollup --jq '...') + if [ "$STATES" = "[]" ]; then echo "no checks have reported"; exit 1; fi + if echo "$STATES" | grep -q '""'; then echo "still running"; exit 1; fi + if echo "$STATES" | grep -qE 'FAILURE|TIMED_OUT'; then exit 1; fi + gh pr merge --squash +""" + + +def self_test(): + """Prove the checker passes a conformant justfile and FAILS each guard's removal. + + A conformance checker nobody has watched fail is exactly the thing it exists to catch, so + every rule is verified to fire when its guard is deleted — not merely to be satisfied. + """ + import tempfile + problems = [] + + with tempfile.TemporaryDirectory() as d: + good = Path(d) / "Justfile" + good.write_text(CONFORMANT, encoding="utf-8") + got = check(good) + if got: + problems.append(f" conformant fixture should pass, but failed: {[k for k, _ in got]}") + + # Removing any single guard must be caught by that guard's own rule. + breaks = { + "pr:confirm-env": ("PR_CONFIRM", "NOPE"), + "open-pr:gate-first": ("just pr", "echo skipped"), + "open-pr:creates-pr": ("gh pr create", "echo nope"), + "open-pr:push-if-no-upstream": ("@{upstream}", "@{nothing}"), + "merge-pr:refuse-red": ("FAILURE|TIMED_OUT", "NOTHING"), + "merge-pr:refuse-empty": ("statusCheckRollup", "somethingElse"), + "merge-pr:refuse-pending": ("still running", "quiet"), + } + for key, (needle, repl) in breaks.items(): + broken = Path(d) / "Broken" + broken.write_text(CONFORMANT.replace(needle, repl), encoding="utf-8") + keys = [k for k, _ in check(broken)] + if key not in keys: + problems.append(f" removing {needle!r} should trip {key}, but tripped {keys}") + + # A comment must NOT satisfy a rule. This is the `if: false`-in-a-comment trap. + commented = Path(d) / "Commented" + commented.write_text( + CONFORMANT.replace('if [ -n "${PR_CONFIRM:-}" ]; then CONFIRM="$PR_CONFIRM"', + '# PR_CONFIRM is handled elsewhere\n if false; then CONFIRM=x'), + encoding="utf-8") + if "pr:confirm-env" not in [k for k, _ in check(commented)]: + problems.append(" a COMMENT mentioning PR_CONFIRM satisfied pr:confirm-env") + + # A missing recipe must be a failure, not a pass by absence. + missing = Path(d) / "Missing" + missing.write_text(CONFORMANT.replace("open-pr *ARGS:", "unrelated:"), encoding="utf-8") + if not any(k.startswith("open-pr") for k, _ in check(missing)): + problems.append(" a MISSING open-pr recipe did not fail the check") + + if problems: + print(f"gate_conformance self-test FAILED (template v{TEMPLATE_VERSION}):", file=sys.stderr) + print("\n".join(problems), file=sys.stderr) + return 1 + print(f"gate_conformance.py self-test passed (template v{TEMPLATE_VERSION})") + return 0 + + +def main(argv): + if "--self-test" in argv: + return self_test() + + args = [a for a in argv if not a.startswith("-")] + here = Path(__file__).resolve().parent.parent + candidates = [Path(args[0])] if args else [here / "Justfile", here / "justfile"] + justfile = next((c for c in candidates if c.is_file()), None) + if justfile is None: + print(f"error: no justfile found (tried {[str(c) for c in candidates]})", file=sys.stderr) + return 1 + + failures = check(justfile) + if failures: + print(f"gate conformance FAILED for {justfile.name} " + f"(template v{TEMPLATE_VERSION}):", file=sys.stderr) + for key, desc in failures: + print(f" [{key}] {desc}", file=sys.stderr) + print("", file=sys.stderr) + print(" These guards are shared behaviour across rusticprofile, retch and etr. Each one", file=sys.stderr) + print(" exists because it was missing once and something bad followed: a PR merged over", file=sys.stderr) + print(" a red leg, a merge over a rollup nothing had reported into, a gate no script", file=sys.stderr) + print(" could answer, an open-pr that printed 'Gate passed' and then failed.", file=sys.stderr) + return 1 + print(f"gate conformance ok: pr / open-pr / merge-pr (template v{TEMPLATE_VERSION})") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/templates/justfile-common.just b/templates/justfile-common.just index 7698cba..15f772c 100644 --- a/templates/justfile-common.just +++ b/templates/justfile-common.just @@ -2,11 +2,11 @@ # Copyright (C) 2026 l1a # # ============================================================================ -# CANONICAL COMMON JUSTFILE BLOCK — template v2 +# CANONICAL COMMON JUSTFILE BLOCK — template v3 # ============================================================================ # # The text below `---- BEGIN CANONICAL ----` is the reference for the block -# delimited by `# >>> COMMON (template v2)` and `# <<< COMMON` in a project's +# delimited by `# >>> COMMON (template v3)` and `# <<< COMMON` in a project's # Justfile. Vendored in: rusticprofile, retch, etr. # # It is a THIN layer. The real work lives in two vendored Python helpers, @@ -86,11 +86,22 @@ # # SCOPE, STATED SO THE GAPS ARE NOT READ AS OVERSIGHTS # ---------------------------------------------------- -# Covers the install/man/completions family and `standard-check`. Does NOT cover -# `check`/`lint`/`test`/`pr`/`open-pr`/`merge-pr`: those legitimately differ today -# (`--workspace` in retch, `--all-targets` in etr, bare in rusticprofile) and -# reconciling them is a behaviour change per repo rather than a copy. `man` stays -# project-specific — one repo commits its page, one gitignores it, one builds two. +# Covers the install/man/completions family, and — since v3 — the BEHAVIOUR of the +# `pr`/`open-pr`/`merge-pr` triad, via `scripts/gate_conformance.py`. +# +# The triad's *bodies* are still not shared, and that is deliberate: `cargo clippy` is +# bare in rusticprofile, `--workspace` in retch and `--all-targets` in etr; the NOTES +# header differs; each checklist asks for different things. Forcing one body would +# change what each gate DOES. So what is standardised is the set of guards each recipe +# must carry, and `gate_conformance.py` asserts they are present. +# +# That check is STRUCTURAL, not behavioural, and its own docstring says so: it proves a +# guard exists, not that it works. The install helpers can be checked behaviourally +# because they are pure functions; the gate recipes run test suites, push branches and +# merge PRs, so executing them from `check` would be slow and destructive. +# +# Still not covered: `check`/`lint`/`test` bodies, and `man` — one repo commits its +# page, one gitignores it, one builds two. # # Known divergences left alone, for whoever extends this: # * `Justfile` (rusticprofile, retch) vs `justfile` (etr) @@ -170,3 +181,5 @@ standard-check: [ "{{PY}}" != "PYTHON-NOT-FOUND" ] || { echo "error: no python3/python on PATH" >&2; exit 1; } "{{PY}}" scripts/install_completions.py --self-test "{{PY}}" scripts/install_man.py --self-test + "{{PY}}" scripts/gate_conformance.py --self-test + "{{PY}}" scripts/gate_conformance.py "{{justfile()}}" From c7b1386c068f9769e8c264a570f7055f8a6277ce Mon Sep 17 00:00:00 2001 From: Ken Tobias <634380+l1a@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:04:41 -0700 Subject: [PATCH 2/2] Commit the Cargo.lock version bump Assisted-By: Claude Opus 5 --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index e7f2ea6..a5e77d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1551,7 +1551,7 @@ checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" [[package]] name = "retch-cli" -version = "0.6.22" +version = "0.6.23" dependencies = [ "anyhow", "base64",