fix(upstream-delta): discovery is git's index, not the filesystem (#200)
All checks were successful
CI / test (pull_request) Successful in 3m9s
CI / release-exercise (pull_request) Successful in 11s
CI / self-guards (pull_request) Successful in 6s
CI / action-exercise (pull_request) Successful in 6s
CI / docs-sync-exercise (pull_request) Successful in 6s
Refs guard / refs-not-closing (pull_request) Has been skipped
labels / labels (pull_request) Successful in 8s

@codex-reviewer-andresmgsl reproduced it again, with one file: scanned_paths()
said "tracked" and used `find`, which walks the working directory and knows
nothing about the index.

Not pedantry — ci.yml extracts shellcheck.tar.xz, actionlint.tar.gz and their
binaries INTO the checkout before the suite runs, and any developer cache sits
there too. Today none happens to carry a matching marker; that is luck, not a
property, and a false red on a downloaded tarball would be indistinguishable
from a real finding.

`git ls-files -z` makes "tracked" executable rather than prose.

The fixtures become tiny git repositories, because a fixture that is only a
directory is invisible to ls-files and every must-fail below it would have
passed vacuously — the same trap as the earlier teeth that never invoked the
guard. Plus the negative case he asked for: an untracked marker-bearing cache
file is ignored, and the moment it is TRACKED the guard sees it.

Reverting discovery to find reds three.

Branch updated from merged main (e236318, now carrying !206) before verifying:
upstream-delta 28/28, test/run.sh 29/29, shellcheck 0.10.0 clean.

Refs #200
This commit is contained in:
cluade-reviewer-andresmgsl 2026-08-05 14:10:56 +00:00
parent f8f318c634
commit a48cc719a4
2 changed files with 50 additions and 20 deletions

View file

@ -32,6 +32,10 @@
so a composite `action.yml` or a `.yaml` workflow is seen without anyone
remembering to add a glob (#200).
- Discovery is git's, not the filesystem's: `ls-files`, so the tarballs `ci.yml`
extracts into the checkout and any developer cache are not parsed as source
(#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

View file

@ -122,7 +122,6 @@ scan_root() { printf '%s' "${SCAN_ROOT:-$ROOT}"; }
# 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
@ -130,14 +129,21 @@ scan_root() { printf '%s' "${SCAN_ROOT:-$ROOT}"; }
# 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.
#
# DISCOVERY IS GIT'S, NOT THE FILESYSTEM'S. An earlier head said "tracked" and
# used `find`, which walks the working directory and knows nothing about the
# index. That is not pedantry: `ci.yml` extracts shellcheck and actionlint
# tarballs INTO the checkout before the suite runs, and any developer cache
# sits there too. @codex-reviewer-andresmgsl reproduced a false red with one
# untracked file. `git ls-files -z` makes "tracked" executable rather than
# prose.
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
git -C "$root" ls-files -z 2>/dev/null \
| tr '\0' '\n' \
| grep -vE '^(test/|changelog\.d/)' \
| grep -vE '\.md$' \
| sort
}
forge_specific_files() {
@ -203,15 +209,20 @@ check "every forge-deciding file is named in the inventory" 0 "" no_unlisted
TMP="$(mktemp -d)"
trap 'rm -rf "$TMP"' EXIT
fixture_tree() { # -> a tree the scan walks, with no forge-deciding files
# A tiny GIT repository, because discovery is git's now: a fixture that is only
# a directory would be invisible to `ls-files` and every must-fail below would
# pass vacuously.
fixture_tree() { # -> a tracked tree 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"
mkdir -p "$t/lib" "$t/.github/workflows"
printf '#!/usr/bin/env bash\necho hello\n' >"$t/lib/plain.sh"
printf 'name: ci\non: [push]\n' >"$t/.github/workflows/plain.yml"
git -C "$t" init -q 2>/dev/null
git -C "$t" add -A 2>/dev/null
printf '%s' "$t"
}
track() { git -C "$root" add -A 2>/dev/null; }
root="$(fixture_tree)"
clean_tree_passes() { SCAN_ROOT="$root" no_unlisted; }
@ -221,10 +232,10 @@ check "the guard is green on a tree with no forge decisions" 0 "" clean_tree_pas
# 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' \
>"$root/lib/scattered.sh"
scattered_shell() { SCAN_ROOT="$root" no_unlisted; }
track; 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"
rm -f "$root/lib/scattered.sh"; track
# 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.
@ -232,10 +243,10 @@ rm -f "$root/lib/scattered.sh"
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; }
track; 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"
rm -f "$root/.github/workflows/scattered.yml"; track
# @codex-reviewer-andresmgsl's two reproductions, verbatim as fixtures. Both
# passed the hand-picked-glob version 21/21, which is why discovery is derived
@ -244,10 +255,10 @@ 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; }
track; 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"
rm -rf "$root/actions/unlisted-forge-decision"; track
# ...and a workflow written .yaml rather than .yml — `*.yml` was never a
# complete workflow surface.
@ -255,22 +266,37 @@ rm -rf "$root/actions/unlisted-forge-decision"
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; }
track; 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"
rm -f "$root/.github/workflows/unlisted-forge-decision.yaml"; track
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"
printf '%s\n' '#!/usr/bin/env bash' 'CEREMONY_FORGE_CLIENT=gh' >"$root/lib/facts.sh"
declared_not_exempt() { SCAN_ROOT="$root" no_unlisted; }
track; 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"
rm -f "$root/lib/facts.sh"; track
track
check "...and the tree is green again once it is gone" 0 "" clean_tree_passes
# UNTRACKED input is not source. ci.yml extracts shellcheck and actionlint
# tarballs into the checkout before the suite runs, and a developer cache sits
# there too; parsing either is a false red on something outside the repository
# property (@codex-reviewer-andresmgsl, #200 review — reproduced with one file).
printf 'CEREMONY_FORGE_CLIENT=gh\n' >"$root/local-tool-cache.txt"
check "an UNTRACKED marker-bearing file is ignored" 0 "" clean_tree_passes
check "...and is still ignored once it carries a decision" 0 "" clean_tree_passes
track
check "...but the moment it is TRACKED the guard sees it" 1 "local-tool-cache.txt" \
clean_tree_passes
git -C "$root" rm -q --cached local-tool-cache.txt 2>/dev/null
rm -f "$root/local-tool-cache.txt"
check "...and removing it restores green" 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