Close four small ADR-0020-retrofit follow-ups (#125, #127, #113, #118) #130

Merged
Defame1297 merged 12 commits from fix/adr0020-followups into main 2026-09-09 18:23:00 +00:00
4 changed files with 404 additions and 16 deletions
Showing only changes of commit 09eea5e7ab - Show all commits

View File

@@ -105,6 +105,19 @@ Three ways to read the result wrong:
is not a directory, a directory holding no `SKILL.md`, a missing or extra argument, and an absent
`python3` all exit **2** with a message on stderr. Exit 2 means the script never ran — report it
as an unaudited dimension, never as a pass and never as a finding. Exit 1 is findings.
- **A check-9 INFO — `'<field>' changed for '<slug>' since <ref>` — means go read, not just relay.**
Check 9 diffs the current `references/sources.md` against a base ref (default: the merge base with
`origin/main`) and flags a slug whose `Description` or `Contributing files` text differs. It is
structurally incapable of telling you whether the new wording is still *true* — it only detects
that the text changed — so when this INFO fires, open the Contributing files it names and the
document named in that slug's `Research doc:` field, and confirm by reading whether the (possibly
strengthened) claim genuinely holds. This is the one provenance finding this script cannot verify
for you: every other check here is a structural fact you can relay as-is, but check 9's job is
only to tell you *where* to spend that reading effort, not to replace it. Acknowledging the INFO
without opening those files is not auditing it. A single INFO naming "no base ref could be
resolved" or "no repo root above the skill directory" is the same graceful-skip pattern as every
other check here that cannot run — treat it as an unaudited dimension for that reason, not as a
finding about the skill.
- **`vale` reports `0 files`.** Treat the pass as NOT RUN, not as clean, and fall back to full
Step 3 judgment for the dimensions it would have covered. The bundled `Kyberforge` style is
scoped by glob in `assets/vale/.vale.ini`; a file outside those globs is silently not linted.

View File

@@ -3,12 +3,19 @@ set -euo pipefail
usage() {
cat <<EOF
Usage: validate-provenance.sh <skill-dir>
Usage: validate-provenance.sh <skill-dir> [--base-ref=<ref>]
Validate that a skill's sources provenance chain is complete and internally consistent.
Arguments:
skill-dir Path to the skill directory to validate.
skill-dir Path to the skill directory to validate.
--base-ref=REF Git ref to diff references/sources.md against for check 9.
Defaults to \`git merge-base HEAD origin/main\`. Override this
when origin/main is not the right comparison point (a fork,
a long-lived branch, a mirror with a different remote name).
The VALIDATE_PROVENANCE_BASE_REF environment variable is an
equivalent, lower-precedence way to set it — the flag wins
if both are given.
Exit codes:
0 All checks passed (or nothing to validate)
@@ -38,6 +45,12 @@ Checks performed:
resolved; a path that still does not resolve is reported as an INFO saying
checks 7 and 8 did not run, never skipped silently.
8 Extracted non-(none) slug in research doc present in sources.md
9 Description or Contributing files text changed since --base-ref (INFO
only — a bash script cannot verify the claim is still TRUE, only that it
changed; the auditor reads the named files to check that). A slug absent
at the base ref is a creation, not a change, and is not flagged. When the
base ref cannot be resolved at all, this is announced as ONE INFO for the
whole check, never a silent skip.
Checks 7 and 8 apply ONLY when the Research doc value names a research SOURCE
INDEX — a file whose basename is sources.md, whose H2 headings ARE source
@@ -52,13 +65,31 @@ if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then
exit 0
fi
# --base-ref=<ref> is the only recognised flag, for check 9's base-ref
# override. It is pulled out before the positional-count checks below so it
# never counts against them — a caller passing it alongside skill-dir sees
# the same argument-count behaviour as one who does not pass it at all, and a
# genuinely extra positional argument is still rejected.
declare -a POSITIONAL_ARGS=()
BASE_REF_OVERRIDE=""
for arg in "$@"; do
case "$arg" in
--base-ref=*)
BASE_REF_OVERRIDE="${arg#--base-ref=}"
;;
*)
POSITIONAL_ARGS+=("$arg")
;;
esac
done
# Usage and environment problems exit 2, findings exit 1. See the usage text
# above for why the two must not share a code. This is a deliberate divergence
# from validate.sh, which has no 2 tier: validate.sh always prints PASS lines,
# so a usage error there is visibly not a findings report. This script prints
# NOTHING on a clean run, so exit 1 plus empty stdout was the only signal a
# caller got either way.
if [[ $# -lt 1 ]]; then
if [[ ${#POSITIONAL_ARGS[@]} -lt 1 ]]; then
echo "Error: skill-dir is required." >&2
echo "" >&2
usage >&2
@@ -67,13 +98,20 @@ fi
# Extra positional arguments were silently dropped, so a typo'd flag or a second
# path looked like it had been honoured.
if [[ $# -gt 1 ]]; then
echo "Error: expected exactly one argument, got $#: $*" >&2
if [[ ${#POSITIONAL_ARGS[@]} -gt 1 ]]; then
echo "Error: expected exactly one argument, got ${#POSITIONAL_ARGS[@]}: ${POSITIONAL_ARGS[*]}" >&2
echo "" >&2
usage >&2
exit 2
fi
SKILL_DIR_ARG="${POSITIONAL_ARGS[0]}"
# The flag wins over the environment variable when both are given; either is
# empty-string when unset, and an empty string tells the Python body to fall
# back to `git merge-base HEAD origin/main`.
BASE_REF="${BASE_REF_OVERRIDE:-${VALIDATE_PROVENANCE_BASE_REF:-}}"
# python3 is a HARD dependency. Without this preflight a missing interpreter
# produced 'line NN: python3: command not found' and exit 127 — an exit code no
# caller maps to anything, from a message that names this script's line number
@@ -91,24 +129,25 @@ fi
# references/validation-scripts.md explicitly told the auditor to read as a
# pass. A typo'd target was therefore indistinguishable from a clean skill.
# vale-wrap.sh hard-errors on a nonexistent path for exactly this reason.
if [[ ! -d "$1" ]]; then
echo "Error: not a directory: $1" >&2
if [[ ! -d "$SKILL_DIR_ARG" ]]; then
echo "Error: not a directory: $SKILL_DIR_ARG" >&2
echo " Why: a nonexistent target would otherwise report a silent pass." >&2
echo " Fix: pass the path of the skill directory to validate." >&2
exit 2
fi
if [[ ! -f "$1/SKILL.md" ]]; then
echo "Error: not a skill directory (no SKILL.md): $1" >&2
if [[ ! -f "$SKILL_DIR_ARG/SKILL.md" ]]; then
echo "Error: not a skill directory (no SKILL.md): $SKILL_DIR_ARG" >&2
echo " Why: a directory with no SKILL.md has no provenance chain to validate, and reporting that as a pass hides the wrong-target mistake." >&2
echo " Fix: pass the skill directory itself, not its parent or its references/ subdirectory." >&2
exit 2
fi
python3 -u - "$1" <<'PYTHON'
python3 -u - "$SKILL_DIR_ARG" "$BASE_REF" <<'PYTHON'
import sys
import os
import re
import subprocess
# Output is UTF-8 for the same reason input is: under LC_ALL=C the streams
# default to ASCII, and every finding this script prints contains an em dash.
@@ -125,6 +164,11 @@ skill_dir = os.path.abspath(sys.argv[1])
sources_md_path = os.path.join(skill_dir, "references", "sources.md")
refs_dir = os.path.join(skill_dir, "references")
# Empty string (the shell side passes "" when neither --base-ref nor
# VALIDATE_PROVENANCE_BASE_REF was given) means: resolve the default via
# `git merge-base HEAD origin/main` at check-9 time, below.
base_ref_override = sys.argv[2] if len(sys.argv) > 2 else ""
# --- Helpers ---
# The trailing character class used to be CONSUMING — `[^`\n]` — so a
@@ -444,6 +488,81 @@ def find_repo_root(start_dir):
return None
current = parent
# --- Check 9 helpers ---------------------------------------------------
# Check 9 needs a raw field VALUE (as text, to diff against an earlier
# version), not the parsed structure parse_contributing_files() and
# parse_status() return — a Contributing files list that reordered its
# entries without changing them is not what this check is looking for, but
# neither is normalizing so hard that a genuine rewrite disappears. Raw text,
# whitespace-normalized, is the middle ground.
def run_git(args, cwd):
"""Run `git <args>` in cwd. Returns (returncode, stdout, stderr) — never
raises, so a missing git binary or an unexpected OSError is just another
non-zero result the caller folds into "could not run", not a crash."""
try:
result = subprocess.run(
["git"] + args, cwd=cwd, capture_output=True, text=True,
encoding="utf-8", errors="replace"
)
return result.returncode, result.stdout, result.stderr
except OSError as exc:
return 1, "", str(exc)
def ref_exists(ref, cwd):
"""True when ref resolves to a commit in the repo at cwd."""
rc, _out, _err = run_git(["rev-parse", "--verify", "--quiet", ref + "^{commit}"], cwd)
return rc == 0
def find_slug_block(content, slug):
"""The raw text of a '## <slug>' entry's body, or None if no such H2."""
pattern = re.compile(
r'^## ' + re.escape(slug) + r'\s*\n(.*?)(?=^## |\Z)',
re.MULTILINE | re.DOTALL
)
m = pattern.search(content)
return m.group(1) if m else None
def parse_field_raw(content, slug, field_name):
"""Raw text of a '**<field_name>:**' field under a slug H2.
Mirrors the two authored shapes parse_contributing_files() and
parse_status() already handle (inline value on the same line, or a
bare heading followed by '- ' bullets), but returns text rather than a
parsed structure, because check 9 diffs wording, not semantics.
Returns None when the H2 itself is absent (the slug did not exist at
this content's revision) or the field is absent — both read as "no
earlier claim to compare against" to the caller, which is deliberate:
a field appearing for the first time is a creation, not a change.
"""
block = find_slug_block(content, slug)
if block is None:
return None
inline_re = re.compile(r'^\- \*\*' + re.escape(field_name) + r':\*\* (.+)$', re.MULTILINE)
im = inline_re.search(block)
if im:
return im.group(1).strip()
heading_re = re.compile(r'^\*\*' + re.escape(field_name) + r':\*\*\s*$', re.MULTILINE)
hm = heading_re.search(block)
if not hm:
return None
lines = []
for line in block[hm.end():].splitlines():
line = line.strip()
if not line:
if lines:
break
continue
if not line.startswith("- "):
break
lines.append(line[2:].strip())
return ", ".join(lines) if lines else None
def normalize_field_text(value):
"""Collapse whitespace so reformatting alone never registers as a change."""
return re.sub(r'\s+', ' ', value).strip()
findings = []
has_fail = False
@@ -859,6 +978,89 @@ for rd_abs, (rd_rel, known_slugs, rd_content) in research_docs_seen.items():
f"Add '## {rd_slug}' to references/sources.md or mark it as '(none)' in the research doc's Contributing files."
)
# --- Check 9: Description / Contributing files changed since --base-ref ---
# A structural fact — the field's TEXT differs from an earlier revision — is
# all git can tell us. Whether the (possibly stronger) new wording is still
# TRUE is a semantic question no parser here can answer; that is what sent
# the earlier literal-text approaches (flagging a named-but-missing filename)
# to 3/3 false positives against the real corpus without even catching the
# bug that motivated this check. So check 9 does the one thing git reliably
# can: detect the change, and hand the auditor the slug and field to go read,
# never a verdict on the claim itself. Always INFO, never FAIL.
if repo_root is None:
emit_info(
"Check 9 skipped — no repo root above the skill directory",
"references/sources.md",
"No ancestor of the skill directory contains a .git entry, so there is no git history to diff "
"references/sources.md against. Check 9 did not run for any slug in this skill. "
"Run this script against a skill inside a checkout to get this check."
)
else:
resolved_base_ref = base_ref_override.strip()
resolve_error = None
if not resolved_base_ref:
rc, mb_out, mb_err = run_git(["merge-base", "HEAD", "origin/main"], repo_root)
if rc == 0 and mb_out.strip():
resolved_base_ref = mb_out.strip()
else:
resolve_error = (
"`git merge-base HEAD origin/main` could not resolve a base ref"
+ (f" ({mb_err.strip()})" if mb_err.strip() else "")
+ " — there may be no origin/main remote, HEAD may be detached, or the clone may be shallow."
)
elif not ref_exists(resolved_base_ref, repo_root):
resolve_error = f"--base-ref value '{resolved_base_ref}' does not resolve to a commit in this repository."
if resolve_error:
emit_info(
"Check 9 skipped — no base ref could be resolved",
"references/sources.md",
resolve_error + " Check 9 did not run for any slug in this skill. "
"Pass --base-ref=<ref>, or set the VALIDATE_PROVENANCE_BASE_REF environment variable, "
"to compare against something other than origin/main."
)
else:
sources_md_relpath = os.path.relpath(sources_md_path, repo_root)
rc, old_sources_content, show_err = run_git(
["show", f"{resolved_base_ref}:{sources_md_relpath}"], repo_root
)
if rc != 0:
# The base ref resolved fine, but references/sources.md did not
# exist there at all — the whole file is new. Every entry in it
# is therefore a creation, not a change: nothing to flag, and
# this is not a structural failure of the check, so no INFO
# either. Same reasoning applies per-slug below when the ref
# resolved but a given '## <slug>' heading did not exist yet.
old_sources_content = None
if old_sources_content is not None:
for slug in unique_slugs:
changed_fields = []
for field_name in ("Description", "Contributing files"):
old_value = parse_field_raw(old_sources_content, slug, field_name)
new_value = parse_field_raw(sources_content, slug, field_name)
if old_value is None or new_value is None:
# No earlier claim to compare against — a brand-new
# entry, or a field that did not exist yet at the
# base ref. That is a creation, not a change, and is
# never flagged.
continue
if normalize_field_text(old_value) != normalize_field_text(new_value):
changed_fields.append(field_name)
if changed_fields:
field_list = " and ".join(changed_fields)
emit_info(
f"'{field_list}' changed for '{slug}' since {resolved_base_ref}",
f"references/sources.md (## {slug})",
f"The '## {slug}' entry's {field_list} text differs from the version at "
f"{resolved_base_ref}. This script can confirm the entry is internally "
f"consistent, but it cannot verify whether the claim itself is still true — a "
f"retrofit once turned an honest hedge into an unsupported confident claim and "
f"every structural check here passed it silently. Re-read the upstream research "
f"doc named in this entry's Research doc field and the Contributing files it "
f"lists, and confirm by hand that the wording still holds."
)
print_findings()
sys.exit(1 if has_fail else 0)
PYTHON

View File

@@ -67,17 +67,36 @@ EOF
EOF
}
# Helper: create a fake repo (a .git marker makes find_repo_root stop there)
# holding one skill whose single sources.md slug points at the given
# Research doc value. Checks 7 and 8 only run for a skill inside a checkout,
# so every upstream case needs this shape; the research doc itself is
# written per test into "$repo/docs/research/sources.md".
# Helper: turn dir into a real git repo with one commit of its current
# contents, and a refs/remotes/origin/main pointing at that same commit.
# Check 9 diffs the skill's references/sources.md against `git merge-base
# HEAD origin/main` by default; this makes that resolve to a commit whose
# sources.md is byte-identical to the working tree, so check 9 has
# nothing to report there — exactly what a real repo looks like the
# instant after a clean commit. Fixtures that go on to test something
# else entirely (checks 3, 4, 5, 7, 8...) call this once, at the point
# their skill's own sources.md is in its final state, so a completely
# clean run stays completely clean.
commit_as_base() {
local dir="$1"
git -C "$dir" init -q >/dev/null 2>&1
git -C "$dir" -c user.email=test@example.com -c user.name=test add -A >/dev/null 2>&1
git -C "$dir" -c user.email=test@example.com -c user.name=test commit -q -m base >/dev/null 2>&1
git -C "$dir" update-ref refs/remotes/origin/main HEAD >/dev/null 2>&1
}
# Helper: create a fake repo (a real git repo, one commit, makes
# find_repo_root stop there) holding one skill whose single sources.md
# slug points at the given Research doc value. Checks 7 and 8 only run
# for a skill inside a checkout, so every upstream case needs this shape;
# the research doc itself is written per test into
# "$repo/docs/research/sources.md" — which check 9 does not examine, so
# a test overwriting it after this helper runs does not disturb check 9.
make_upstream_skill() {
local repo="$1"
local research="${2:-docs/research/sources.md}"
local skill="$repo/my-skill"
mkdir -p "$skill/references" "$repo/docs/research"
touch "$repo/.git"
cat > "$skill/SKILL.md" <<EOF
---
name: my-skill
@@ -102,6 +121,7 @@ EOF
- **Research doc:** ${research}
- **Status:** \`extracted\`
EOF
commit_as_base "$repo"
}
}
@@ -790,6 +810,7 @@ source_keys: []
House-authored, no external source.
EOF
commit_as_base "$skill"
run bash "$SCRIPT" "$skill"
assert_success
assert_output ""
@@ -809,6 +830,7 @@ metadata:
House-authored, no external source.
EOF
commit_as_base "$skill"
run bash "$SCRIPT" "$skill"
assert_success
assert_output ""
@@ -1046,6 +1068,7 @@ EOF
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill" "my-source" "(none — not used directly)"
commit_as_base "$skill"
run bash "$SCRIPT" "$skill"
assert_success
assert_output ""
@@ -1494,3 +1517,143 @@ EOF
assert_failure
assert_output --partial "not valid UTF-8"
}
# ---------------------------------------------------------------------------
# Cycle 28 — Check 9: a changed Description or Contributing files claim is an
# INFO, never a FAIL — the script can tell the text changed, not whether the
# (possibly stronger) new wording is still true.
# ---------------------------------------------------------------------------
@test "check 9: Description text changed since base ref fires an INFO naming the slug and field" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
commit_as_base "$skill"
# Rewritten in the working tree only, never committed — exactly the
# shape of the bug check 9 exists to flag: a hedge upgraded to a
# confident claim with nothing else in the entry touched.
sed -i 's/^- \*\*Description:\*\* A test source\.$/- **Description:** A test source that Grounds the dispatch table directly./' \
"$skill/references/sources.md"
run bash "$SCRIPT" "$skill"
assert_success
assert_output --partial "INFO"
assert_output --partial "'Description' changed for 'my-source'"
}
@test "check 9: Contributing files text changed since base ref fires an INFO naming the slug and field" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
commit_as_base "$skill"
# Only an annotation is added. strip_note() in parse_contributing_files()
# removes it before checks 4 and 5 compare paths, so those stay clean —
# check 9 compares the raw field text, not the parsed path list, and
# this is still a real wording change worth a human re-reading it.
sed -i 's/^- \*\*Contributing files:\*\* SKILL\.md$/- **Contributing files:** SKILL.md (the dispatch table)/' \
"$skill/references/sources.md"
run bash "$SCRIPT" "$skill"
assert_success
assert_output --partial "INFO"
assert_output --partial "'Contributing files' changed for 'my-source'"
refute_output --partial "does not exist"
refute_output --partial "does not list"
}
@test "check 9: an entry unchanged since base ref produces no check-9 finding" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
commit_as_base "$skill"
run bash "$SCRIPT" "$skill"
assert_success
assert_output ""
}
@test "check 9: a brand-new entry absent at the base ref is a creation, not a change" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
commit_as_base "$skill"
# Added in the working tree only, after the commit above. It has no
# earlier revision to diff against, so check 9 must stay silent about
# it — a brand-new entry is a creation, not a rewrite of an existing
# claim, and flagging it would be exactly the false-positive shape the
# rejected literal-text approaches produced.
cat >> "$skill/references/sources.md" <<EOF
## new-source
- **URL:** https://example.com/new-source
- **Description:** A brand-new source, never committed.
- **Contributing files:** (none)
- **Research doc:** (none)
- **Status:** \`extracted\`
EOF
run bash "$SCRIPT" "$skill"
assert_success
assert_output ""
}
@test "check 9: no repo root emits the one graceful INFO, not a silent skip or a crash" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
# Deliberately no commit_as_base — $TMPDIR has no ancestor .git, so
# find_repo_root() returns None.
run bash "$SCRIPT" "$skill"
assert_success
assert_output --partial "INFO"
assert_output --partial "Check 9 skipped — no repo root above the skill directory"
}
@test "check 9: an unresolvable base ref emits the one graceful INFO, not a silent skip or a crash" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
commit_as_base "$skill"
# A repo with no origin/main and no override: `git merge-base HEAD
# origin/main` fails outright.
git -C "$skill" update-ref -d refs/remotes/origin/main >/dev/null 2>&1
run bash "$SCRIPT" "$skill"
assert_success
assert_output --partial "INFO"
assert_output --partial "Check 9 skipped — no base ref could be resolved"
}
@test "check 9: --base-ref overrides the default origin/main resolution" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
commit_as_base "$skill"
local base_sha
base_sha="$(git -C "$skill" rev-parse HEAD)"
git -C "$skill" update-ref -d refs/remotes/origin/main >/dev/null 2>&1
sed -i 's/^- \*\*Description:\*\* A test source\.$/- **Description:** A rewritten claim./' \
"$skill/references/sources.md"
run bash "$SCRIPT" "$skill" "--base-ref=$base_sha"
assert_success
assert_output --partial "'Description' changed for 'my-source'"
}
@test "check 9: an invalid --base-ref value is reported as unresolvable, not a crash" {
local skill="$TMPDIR/my-skill"
make_skill_with_source_keys "$skill"
make_sources_md "$skill"
commit_as_base "$skill"
run bash "$SCRIPT" "$skill" "--base-ref=not-a-real-ref"
assert_success
assert_output --partial "Check 9 skipped — no base ref could be resolved"
assert_output --partial "not-a-real-ref"
}

View File

@@ -105,6 +105,16 @@ them for you. After every retrofit that adds, removes or renames a file:
zero.
- [ ] Re-run `/skill-audit` and confirm its `### Provenance` dimension does not report the new
file as missing `source_keys`.
- [ ] **Compression must not add authority the source text didn't have.** The bullet above is
about a `sources.md` entry going *stale* — Contributing files left uncited after content
moves. This is a distinct failure: a compression or rewrite pass that upgrades an honest
hedge in a Description into an unsupported confident claim, without the underlying source
having changed at all — "no forge-specific content drawn directly from it beyond that"
quietly becoming "Grounds Step 2's dispatch table." Nothing in `/skill-audit`'s structural
checks catches this; a bash script can verify an entry is internally consistent, never
whether the claim is *true*. If a retrofit strengthens or otherwise changes the wording of a
provenance claim, re-read the upstream research doc first and confirm the stronger wording
is actually still true before committing it.
## Worked example — a description retrofit