diff --git a/changelog.d/200.md b/changelog.d/200.md index 16add90..52ca386 100644 --- a/changelog.d/200.md +++ b/changelog.d/200.md @@ -27,9 +27,10 @@ - `.upstream-ref` records the upstream commit this tree carries, in 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 — across shell, workflows, `labels.conf` - and `drills/`, not shell alone (#200). +- `test/upstream-delta.test.sh` fails the PR that scatters a forge decision + into a file the inventory does not name. Discovery is derived from the tree, + so a composite `action.yml` or a `.yaml` workflow is seen without anyone + remembering to add a glob (#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` diff --git a/docs/UPSTREAM-SYNC.md b/docs/UPSTREAM-SYNC.md index 8377e42..e111fe3 100644 --- a/docs/UPSTREAM-SYNC.md +++ b/docs/UPSTREAM-SYNC.md @@ -227,6 +227,20 @@ Forge-specific behaviour is confined to the files below. Keeping it there is what makes each sync cost 18 hunks instead of hundreds, and `test/upstream-delta.test.sh` fails the PR that scatters it into a new file. +**What that guard actually checks**, stated precisely so the table is not read +as a stronger promise than it is: it walks every tracked file except prose +(`*.md`), the test harness and `changelog.d/`, and flags any that **decides** +the forge — the selector's verbs, `CEREMONY_FORGE*`, or a server-URL comparison +written inline. Discovery is derived from the tree rather than from a list of +directories and extensions, so a composite `action.yml` or a `.yaml` workflow +is seen without anyone remembering to add it. + +It is a check on *forge decisions in executable and configuration files*. It is +**not** a diff against upstream, so it cannot see a file that differs from +upstream for some other forge-specific reason — `drills/` and +`.github/labels.conf` are in the table for that kind of reason and are listed +by judgement, not by scan. + | file | what is forge-specific about it | |---|---| | `lib/forge.sh` | the selector: `forge_detect`, `forge_client`, `forge_preflight` | diff --git a/test/upstream-delta.test.sh b/test/upstream-delta.test.sh index 4299f38..a9a1c96 100644 --- a/test/upstream-delta.test.sh +++ b/test/upstream-delta.test.sh @@ -110,17 +110,43 @@ FORGE_MARKERS='forge_detect|forge_select|forge_preflight|forge_client|CEREMONY_F # invoked no_unlisted — the guard could have been `return 0` and still passed. scan_root() { printf '%s' "${SCAN_ROOT:-$ROOT}"; } +# DISCOVERY IS DERIVED FROM THE TREE, not from a list of directories, depths +# and extensions. The earlier version hand-picked five globs and therefore +# could not see `actions/*/action.yml` — this repository's normal composite +# structure, where a client declaration is exactly a forge decision — or a +# workflow written `.yaml` rather than `.yml`. @codex-reviewer-andresmgsl +# constructed both and the guard stayed 21/21 green, which is the whole +# argument against maintaining a glob list. +# +# So: walk everything, then EXCLUDE by class, and let content classify the +# rest. Excluding is safer than including because a new file type arrives +# scanned rather than invisible. +# +# .git/ not source +# test/ the harness stubs and asserts these tokens by design +# changelog.d/ prose fragments +# *.md prose. `drills/` stays in the INVENTORY because its records +# are forge-specific by content, but a record mentioning a +# selector verb in prose is not a decision, and scanning prose +# for decisions is the mistake this guard's own comment +# handling exists to avoid. +scanned_paths() { + local root; root="$(scan_root)" + find "$root" -type f \ + -not -path "$root/.git/*" \ + -not -path "$root/test/*" \ + -not -path "$root/changelog.d/*" \ + -not -name '*.md' \ + 2>/dev/null | sed "s|^$root/||" | sort +} + forge_specific_files() { - 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"/}" - sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$f" \ + local root rel; root="$(scan_root)" + while IFS= read -r rel; do + [ -n "$rel" ] || continue + sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$root/$rel" 2>/dev/null \ | grep -qE "$FORGE_MARKERS" && printf '%s\n' "$rel" - done + done < <(scanned_paths) return 0 } @@ -211,6 +237,30 @@ 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" +# @codex-reviewer-andresmgsl's two reproductions, verbatim as fixtures. Both +# passed the hand-picked-glob version 21/21, which is why discovery is derived +# from the tree now. Deleting a discovery class must make these red. +mkdir -p "$root/actions/unlisted-forge-decision" +printf '%s\n' 'name: x' 'runs:' ' using: composite' ' steps:' \ + ' - shell: bash' ' env:' ' CEREMONY_FORGE_CLIENT: gh' \ + ' run: true' >"$root/actions/unlisted-forge-decision/action.yml" +composite_action_seen() { SCAN_ROOT="$root" no_unlisted; } +check "a forge declaration in actions/*/action.yml fails the guard" 1 \ + "actions/unlisted-forge-decision/action.yml" composite_action_seen +rm -rf "$root/actions/unlisted-forge-decision" + +# ...and a workflow written .yaml rather than .yml — `*.yml` was never a +# complete workflow surface. +# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold +printf '%s\n' 'name: x' 'on: [push]' 'jobs:' ' j:' \ + " if: github.server_url == 'https://github.com'" ' steps: []' \ + >"$root/.github/workflows/unlisted-forge-decision.yaml" +yaml_workflow_seen() { SCAN_ROOT="$root" no_unlisted; } +check "...and one in a .yaml workflow does too" 1 \ + ".github/workflows/unlisted-forge-decision.yaml" yaml_workflow_seen +rm -f "$root/.github/workflows/unlisted-forge-decision.yaml" +check "...leaving the fixture tree green again" 0 "" clean_tree_passes + # 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"