diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b65061..2a05151 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,19 @@ jobs: # at the root that nobody added to docs/VENDORED.txt is invisible to # every consumer's docs-sync, so it fails CI here instead. run: bash .github/scripts/vendored-check.sh + - name: Fetch the recorded upstream commit + # test/upstream-delta.test.sh REFUSES when the recorded object is + # absent rather than calling it unverifiable (#200). "Runs offline" + # means the test reads local evidence — it does not mean CI may omit + # the evidence and pass. This step supplies it; the test never reaches + # the network itself. + run: | + ref="$(grep -vE '^[[:space:]]*(#|$)' .upstream-ref | head -n1)" + git fetch --no-tags --depth=1 \ + https://github.com/heavy-duty/ceremony.git "$ref" || { + echo "::error::could not fetch the recorded upstream commit $ref" >&2 + exit 1 + } - name: Tests env: # The npm-backed version_write case may skip locally when npm is diff --git a/.upstream-ref b/.upstream-ref index 2a4b4db..2337fa2 100644 --- a/.upstream-ref +++ b/.upstream-ref @@ -1,4 +1,7 @@ # The upstream commit this tree carries (docs/UPSTREAM-SYNC.md). -# Updated by each sync; read by test/upstream-delta.test.sh. +# Full 40-char SHA, immutable: captured at fetch, merged, then recorded — +# NOT re-read from gh/main later, which moves. Read by +# test/upstream-delta.test.sh, which REFUSES when the object is absent +# rather than calling it unverifiable. # github.com/heavy-duty/ceremony -8c3a4d1 +8c3a4d1dee2bdb5ac06a632a285bb65ab2615214 diff --git a/changelog.d/200.md b/changelog.d/200.md index ced2b20..16add90 100644 --- a/changelog.d/200.md +++ b/changelog.d/200.md @@ -28,8 +28,16 @@ machine-readable form beside the CHANGELOG's prose (#200). - `test/upstream-delta.test.sh` fails the PR that scatters forge branching into - a file the inventory does not name — offline, and refusing rather than - skipping when the recorded ref is missing (#200). + a file the inventory does not name — across shell, workflows, `labels.conf` + and `drills/`, not shell alone (#200). + +- It refuses when the recorded commit is missing, absent from the object store, + or not an ancestor — three distinct refusals, none of them a skip. `ci.yml` + fetches that exact object so the test reads local evidence without CI + omitting it (#200). + +- Its mutation cases drive the real check against a constructed tree, so + replacing the guard with `return 0` reds five of them (#200). - `docs/CONSUMERS.md` states that two ceremonies answer to the same version number, and how a consumer says which one it pinned (#200). diff --git a/docs/UPSTREAM-SYNC.md b/docs/UPSTREAM-SYNC.md index f4ae16f..8377e42 100644 --- a/docs/UPSTREAM-SYNC.md +++ b/docs/UPSTREAM-SYNC.md @@ -50,9 +50,17 @@ Two consequences worth stating plainly: ```sh git remote add gh https://github.com/heavy-duty/ceremony.git # if absent git fetch gh -git merge-base main gh/main +upstream_sha="$(git rev-parse gh/main)" # capture ONCE, in full +git merge-base main "$upstream_sha" ``` +**Capture the full SHA immediately and use that value everywhere after** — the +merge, the provenance, the `.upstream-ref` write. `gh/main` is a moving +pointer: while this sync was being reviewed upstream advanced from `8c3a4d1` +to `08e2912`, and re-reading `gh/main` at recording time would have written a +commit this tree does not contain. The recorded ref is *what was merged*, never +*what upstream is now*. + **Confirm the merge base against `.upstream-ref` before merging anything.** If it is not what the last sync recorded, something moved — stop and re-measure rather than proceeding. A sync that starts from an unexpected base is a sync @@ -61,7 +69,7 @@ whose conflict count means nothing. ### 2. Merge, never rebase ```sh -git merge gh/main +git merge "$upstream_sha" ``` One merge commit, conflicts resolved once (#197 D1). Rebasing the forge-only @@ -123,8 +131,10 @@ ships it. ### 6. Record the provenance - `CHANGELOG.md`'s header: which upstream commit this tree now carries. -- `.upstream-ref`: the same SHA, machine-readable, checked by - `test/upstream-delta.test.sh`. +- `.upstream-ref`: the same **full 40-character** SHA, machine-readable, + checked by `test/upstream-delta.test.sh` — which refuses when the object is + absent or is not an ancestor, rather than reporting it unverifiable. `ci.yml` + fetches that exact object before the suite runs. - A `changelog.d/` fragment for the sync issue. ### 7. Verify — and verify where it will actually run @@ -225,6 +235,18 @@ what makes each sync cost 18 hunks instead of hundreds, and | `lib/closes_references.sh` | the closing-keyword parser that replaced GraphQL | | `.github/labels.conf` | this instance's roster | | `drills/` | records of runs on this instance | +| `actions/refs-not-closing/run.sh` | declares `CEREMONY_FORGE_CLIENT=gh` — its gather is GraphQL, which Forgejo does not serve. #199 removes the declaration | +| `.github/workflows/labels.yml` | the sweep dispatch decides the forge inline and declares a client; a workflow has no shell to call `forge_preflight` from. #205 ports it | +| `.github/workflows/refs-guard.yml` | schedules its job on GitHub only, so an action that can only refuse here does not stand red. #199 removes the gate | +| `.github/workflows/release-exercise.yml` | pins `CEREMONY_FORGE: github` deliberately: the exercise drives the GitHub path | + +Four of those are **temporary** and say which issue removes them. That is the +point of listing them rather than exempting them: a forge-delta location with +no exit is indistinguishable from one nobody noticed. + +A file that merely **calls** the shim is not a delta location — every +reconciler and `release.yml` call `forge_select`, and that is what the shim is +for. A file that **decides** or **declares** is, and belongs here. If a sync needs forge branching somewhere else, that is a design decision, not a detail: add the file to the inventory in the same PR, with the reason. diff --git a/test/upstream-delta.test.sh b/test/upstream-delta.test.sh index 35b56b0..4299f38 100644 --- a/test/upstream-delta.test.sh +++ b/test/upstream-delta.test.sh @@ -34,30 +34,29 @@ REF="$ROOT/.upstream-ref" check "the sync document exists" 0 "" test -f "$DOC" check "the upstream ref is recorded" 0 "" test -f "$REF" +# THE OBJECT IS MANDATORY. An earlier head accepted "UNVERIFIABLE-HERE" when +# upstream's commit was not in the local store, which contradicted this issue's +# own "must not pass by absence" and made the check decorative in exactly the +# environment it matters in (@codex-reviewer-andresmgsl, #200 review). +# +# "Runs offline" means the TEST reads local evidence, not that CI may omit the +# evidence and pass. ci.yml fetches the recorded object before running the +# suite; if it is missing here, that is a refusal. recorded_ref() { grep -vE '^[[:space:]]*(#|$)' "$REF" | head -n1; } ref_is_recorded() { [ -n "$(recorded_ref)" ]; } check "...and is not blank — an unrecorded ref is a refusal, not a skip" 0 "" \ ref_is_recorded -ref_looks_like_sha() { printf '%s' "$(recorded_ref)" | grep -qE '^[0-9a-f]{7,40}$'; } -check "...and looks like a commit SHA" 0 "" ref_looks_like_sha +# Full 40, not a short prefix: a short SHA is ambiguous across a growing repo +# and cannot be compared byte-wise between the runbook, the CHANGELOG and here. +ref_is_full_sha() { printf '%s' "$(recorded_ref)" | grep -qE '^[0-9a-f]{40}$'; } +check "...and is a FULL 40-character SHA" 0 "" ref_is_full_sha -# The ancestry half needs upstream's objects, which a CI clone of THIS repo -# does not have. It is therefore conditional — but it says so out loud rather -# than passing quietly, because "could not check" and "checked and fine" are -# the two states this repo exists to keep apart. -ancestry_state() { - if git -C "$ROOT" cat-file -e "$(recorded_ref)^{commit}" 2>/dev/null; then - git -C "$ROOT" merge-base --is-ancestor "$(recorded_ref)" HEAD 2>/dev/null \ - && echo ANCESTOR || echo NOT-ANCESTOR - else - echo UNVERIFIABLE-HERE - fi -} -state="$(ancestry_state)" -printf 'upstream-delta: recorded ref %s is %s\n' "$(recorded_ref)" "$state" -ancestry_acceptable() { [ "$state" = ANCESTOR ] || [ "$state" = UNVERIFIABLE-HERE ]; } -check "the recorded ref is an ancestor of HEAD, or is honestly reported as unverifiable" 0 "" \ - ancestry_acceptable +ref_object_present() { git -C "$ROOT" cat-file -e "$(recorded_ref)^{commit}" 2>/dev/null; } +check "the recorded commit is present locally — absent is a refusal, not a skip" 0 "" \ + ref_object_present +ref_is_ancestor() { git -C "$ROOT" merge-base --is-ancestor "$(recorded_ref)" HEAD 2>/dev/null; } +check "...and is an ancestor of HEAD, so main really carries what is recorded" 0 "" \ + ref_is_ancestor # --- the inventory ------------------------------------------------------------ @@ -74,11 +73,17 @@ inventory_is_populated() { [ "$(inventory_size)" -ge 6 ]; } check "the inventory names at least the six known delta files" 0 "" \ inventory_is_populated +# Exact for a file entry, `dir/` + one path separator for a directory entry. +# Prefix matching accepted `drills-old/x` for `drills/` and `lib/forge.sh.backup` +# for `lib/forge.sh` (@codex-reviewer-andresmgsl, #200 review). in_inventory() { # $1 = repo-relative path local entry while IFS= read -r entry; do [ -n "$entry" ] || continue - case "$1" in "$entry" | "$entry"*) return 0 ;; esac + case "$entry" in + */) case "$1" in "$entry"*) return 0 ;; esac ;; + *) [ "$1" = "$entry" ] && return 0 ;; + esac done < <(inventory) return 1 } @@ -88,43 +93,62 @@ check "an unlisted file is not" 1 "" in_inventory lib/version.sh # --- the scan ----------------------------------------------------------------- -# What "forge-specific" means, mechanically: the selector's own verbs and the -# environment override that drives them. Comments are stripped — this tree -# documents the forge split at length, and a guard that read prose as evidence -# would flag every file that merely explains the design. -FORGE_MARKERS='forge_detect|forge_select|forge_preflight|forge_client|CEREMONY_FORGE\b|CEREMONY_FORGE_CLIENT' +# What "forge-specific" means, mechanically: the selector's own verbs, the +# environment override that drives them, and — on a workflow, which has no +# shell to call the selector from — a forge decision written inline. +# +# The earlier version scanned only shell under lib/ actions/ bin/ .github/scripts. +# That missed three categories the inventory itself claims to govern: workflows, +# .github/labels.conf and drills/. Merged main already had blind spots there — +# labels.yml decides on GITHUB_SERVER_URL and declares a client, refs-guard.yml +# carries a positive GitHub-only scheduling decision +# (@codex-reviewer-andresmgsl, #200 review). +FORGE_MARKERS='forge_detect|forge_select|forge_preflight|forge_client|CEREMONY_FORGE\b|CEREMONY_FORGE_CLIENT|GITHUB_SERVER_URL|github\.server_url' + +# SCAN_ROOT is a parameter so the teeth below can drive the REAL check against +# a constructed tree. The previous mutation proved the predicates and never +# invoked no_unlisted — the guard could have been `return 0` and still passed. +scan_root() { printf '%s' "${SCAN_ROOT:-$ROOT}"; } forge_specific_files() { - local f rel - for f in "$ROOT"/lib/*.sh "$ROOT"/actions/*/*.sh "$ROOT"/bin/* \ - "$ROOT"/.github/scripts/*.sh; do + local root f rel + root="$(scan_root)" + for f in "$root"/lib/*.sh "$root"/actions/*/*.sh "$root"/bin/* \ + "$root"/.github/scripts/*.sh "$root"/.github/workflows/*.yml \ + "$root"/.github/labels.conf "$root"/drills/*.md; do [ -f "$f" ] || continue - rel="${f#"$ROOT"/}" + rel="${f#"$root"/}" sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$f" \ | grep -qE "$FORGE_MARKERS" && printf '%s\n' "$rel" done return 0 } -# A call site that merely USES the shim is not forge-specific — every -# reconciler calls forge_preflight and that is the point of the shim. What the -# inventory is about is where the forge is DECIDED or BRANCHED ON. So the scan -# reports files carrying the markers, and the assertion below allows the shim's -# own consumers explicitly, by name, so adding a seventh consumer is silent but -# adding a seventh DECIDER is not. +# A file that merely CALLS the shim is not where the forge is decided — every +# reconciler calls forge_preflight and that is the point of the shim. A file +# that DECLARES a client is a different thing and is NOT exempt: it is a +# deliberate forge-delta location and belongs in the inventory +# (@codex-reviewer-andresmgsl). SHIM_CONSUMERS='actions/issueflow-reconcile/issueflow-reconcile.sh actions/labels-reconcile/labels-reconcile.sh actions/labels-scope/labels-scope.sh -actions/refs-not-closing/run.sh lib/facts.sh lib/ruling.sh -lib/attention.sh' +lib/attention.sh +.github/workflows/release.yml' + +declares_a_client() { + sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$(scan_root)/$1" \ + | grep -qE 'CEREMONY_FORGE_CLIENT[=:]' +} unlisted_deciders() { local rel while IFS= read -r rel; do [ -n "$rel" ] || continue - grep -qxF "$rel" <<<"$SHIM_CONSUMERS" && continue + if grep -qxF "$rel" <<<"$SHIM_CONSUMERS" && ! declares_a_client "$rel"; then + continue + fi in_inventory "$rel" && continue printf '%s\n' "$rel" done < <(forge_specific_files) @@ -142,23 +166,65 @@ no_unlisted() { check "every forge-deciding file is named in the inventory" 0 "" no_unlisted # --- teeth -------------------------------------------------------------------- -# The sweep above is a property of the whole tree and cannot be made to fail -# without editing it, so the predicates are driven directly. +# These drive the REAL top-level check against a constructed tree, via +# SCAN_ROOT. The earlier version asserted the predicates separately and never +# invoked no_unlisted — so the guard could have been replaced with `return 0` +# and both "must-fail" rows would still have passed +# (@codex-reviewer-andresmgsl, #200 review). A mutation test that cannot fail +# when the thing it guards is deleted is the shape this repo keeps filing +# issues about. TMP="$(mktemp -d)" trap 'rm -rf "$TMP"' EXIT +fixture_tree() { # -> a tree the scan walks, with no forge-deciding files + local t="$TMP/tree" + rm -rf "$t" + mkdir -p "$t/lib" "$t/actions/x" "$t/bin" "$t/.github/scripts" \ + "$t/.github/workflows" "$t/drills" + printf '#!/usr/bin/env bash\necho hello\n' >"$t/lib/plain.sh" + printf 'name: ci\non: [push]\n' >"$t/.github/workflows/plain.yml" + printf '%s' "$t" +} + +root="$(fixture_tree)" +clean_tree_passes() { SCAN_ROOT="$root" no_unlisted; } +check "the guard is green on a tree with no forge decisions" 0 "" clean_tree_passes + +# MUST FAIL: a shell file that decides the forge, in no inventory entry. # shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold printf '%s\n' '#!/usr/bin/env bash' 'case "$(forge_detect)" in forgejo) : ;; esac' \ - >"$TMP/scattered.sh" -# Byte-identical to the strip forge_specific_files uses: a helper that -# normalised differently would be testing a predicate the scan does not have. -marker_seen() { - sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1" | grep -qE "$FORGE_MARKERS" -} -check "a new file branching on forge_detect is seen" 0 "" marker_seen "$TMP/scattered.sh" -printf '%s\n' '#!/usr/bin/env bash' '# forge_detect used to live here' >"$TMP/prose.sh" -check "prose about forge_detect is not" 1 "" marker_seen "$TMP/prose.sh" -check "...and such a file would not be in the inventory" 1 "" in_inventory scattered.sh + >"$root/lib/scattered.sh" +scattered_shell() { SCAN_ROOT="$root" no_unlisted; } +check "a new SHELL file deciding the forge fails the real guard" 1 "" scattered_shell +check "...and the failure names the file" 1 "lib/scattered.sh" scattered_shell +rm -f "$root/lib/scattered.sh" + +# MUST FAIL on a NON-SHELL surface too, so coverage cannot regress to the old +# shell-only glob: a workflow deciding on the server URL. +# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold +printf '%s\n' 'name: x' 'on: [push]' 'jobs:' ' j:' ' steps:' \ + ' - run: [ "$GITHUB_SERVER_URL" = https://github.com ] || exit 0' \ + >"$root/.github/workflows/scattered.yml" +scattered_workflow() { SCAN_ROOT="$root" no_unlisted; } +check "a new WORKFLOW deciding the forge fails it too" 1 "" scattered_workflow +check "...naming that file" 1 ".github/workflows/scattered.yml" scattered_workflow +rm -f "$root/.github/workflows/scattered.yml" + +# A declaration is a delta location even in a file that would otherwise read as +# a shim consumer, so the consumer allow-list cannot hide one. +mkdir -p "$root/lib" +printf '%s\n' '#!/usr/bin/env bash' 'CEREMONY_FORGE_CLIENT=gh' >"$root/lib/facts.sh" +declared_not_exempt() { SCAN_ROOT="$root" no_unlisted; } +check "a shim consumer that DECLARES a client is not exempt" 1 "lib/facts.sh" \ + declared_not_exempt +rm -f "$root/lib/facts.sh" +check "...and the tree is green again once it is gone" 0 "" clean_tree_passes + +# Path matching, both boundaries. +check "a directory entry does not match a sibling with the same prefix" 1 "" \ + in_inventory drills-old/0.4.1.md +check "a file entry does not match a longer path" 1 "" in_inventory lib/forge.sh.backup +check "...while the real ones still match" 0 "" in_inventory drills/0.4.1.md summary