forked from heavy-duty/ceremony
fix(upstream-delta): discovery derives from the tree, not from a glob list (#200)
@codex-reviewer-andresmgsl did not argue this one, he reproduced it: an `actions/*/action.yml` declaring CEREMONY_FORGE_CLIENT and a workflow written `.yaml` rather than `.yml`, both invisible to the hand-picked globs, guard still 21/21 green. The first is not an edge case — `actions/*/action.yml` is this repository's normal composite structure and a client declaration there IS a forge decision. The second shows `*.yml` was never a complete workflow surface. So discovery walks the tree and EXCLUDES by class rather than enumerating directories, depths and extensions. Excluding is the safer default: a new file type arrives scanned rather than invisible. Out of scope are .git/, test/ (whose harness asserts these tokens by design), changelog.d/ and *.md — prose, including drills/, which stays in the inventory because its records are forge-specific by CONTENT while a record mentioning a selector verb in prose is not a decision. Both of his reproductions are now fixtures driving the real no_unlisted, and restricting discovery back to *.sh reds four cases. The documentation claim is aligned with what the guard does rather than what the table implies: it checks forge DECISIONS in executable and configuration files; it is not a diff against upstream, so drills/ and labels.conf are listed by judgement rather than found by scan. Saying otherwise made labels.conf and drills/ look like evidence of completeness while action.yml was invisible. upstream-delta 24/24; test/run.sh 29/29; shellcheck 0.10.0, actionlint and changelog-armed clean. Refs #200
This commit is contained in:
parent
634e7a3528
commit
f3a1336d42
3 changed files with 77 additions and 12 deletions
|
|
@ -27,9 +27,10 @@
|
||||||
- `.upstream-ref` records the upstream commit this tree carries, in
|
- `.upstream-ref` records the upstream commit this tree carries, in
|
||||||
machine-readable form beside the CHANGELOG's prose (#200).
|
machine-readable form beside the CHANGELOG's prose (#200).
|
||||||
|
|
||||||
- `test/upstream-delta.test.sh` fails the PR that scatters forge branching into
|
- `test/upstream-delta.test.sh` fails the PR that scatters a forge decision
|
||||||
a file the inventory does not name — across shell, workflows, `labels.conf`
|
into a file the inventory does not name. Discovery is derived from the tree,
|
||||||
and `drills/`, not shell alone (#200).
|
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,
|
- 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`
|
or not an ancestor — three distinct refusals, none of them a skip. `ci.yml`
|
||||||
|
|
|
||||||
|
|
@ -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
|
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.
|
`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 |
|
| file | what is forge-specific about it |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `lib/forge.sh` | the selector: `forge_detect`, `forge_client`, `forge_preflight` |
|
| `lib/forge.sh` | the selector: `forge_detect`, `forge_client`, `forge_preflight` |
|
||||||
|
|
|
||||||
|
|
@ -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.
|
# invoked no_unlisted — the guard could have been `return 0` and still passed.
|
||||||
scan_root() { printf '%s' "${SCAN_ROOT:-$ROOT}"; }
|
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() {
|
forge_specific_files() {
|
||||||
local root f rel
|
local root rel; root="$(scan_root)"
|
||||||
root="$(scan_root)"
|
while IFS= read -r rel; do
|
||||||
for f in "$root"/lib/*.sh "$root"/actions/*/*.sh "$root"/bin/* \
|
[ -n "$rel" ] || continue
|
||||||
"$root"/.github/scripts/*.sh "$root"/.github/workflows/*.yml \
|
sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$root/$rel" 2>/dev/null \
|
||||||
"$root"/.github/labels.conf "$root"/drills/*.md; do
|
|
||||||
[ -f "$f" ] || continue
|
|
||||||
rel="${f#"$root"/}"
|
|
||||||
sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$f" \
|
|
||||||
| grep -qE "$FORGE_MARKERS" && printf '%s\n' "$rel"
|
| grep -qE "$FORGE_MARKERS" && printf '%s\n' "$rel"
|
||||||
done
|
done < <(scanned_paths)
|
||||||
return 0
|
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
|
check "...naming that file" 1 ".github/workflows/scattered.yml" scattered_workflow
|
||||||
rm -f "$root/.github/workflows/scattered.yml"
|
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 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.
|
# a shim consumer, so the consumer allow-list cannot hide one.
|
||||||
mkdir -p "$root/lib"
|
mkdir -p "$root/lib"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue