From 09eea5e7ab9b22dcb64802eba4683cacf2ac1cd8 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Mon, 7 Sep 2026 20:37:03 +0000 Subject: [PATCH] fix(skill-audit): flag a changed provenance claim, not just its shape validate-provenance.sh checked that a sources.md entry was internally consistent -- slugs resolve, Contributing files exist, back-references match -- but never whether the asserted contribution was true. A retrofit once turned an honest hedge into a false confident claim and every existing check passed it silently. A literal-filename cross-check (flag a description naming a .md file absent from Contributing files) was tried and rejected: 3/95 flagged against the real corpus, all three false positives, and it would not have caught the actual bug -- the bad description never named a literal filename. No bash script can verify semantic truth, so the fix uses what git can reliably detect -- a changed field -- purely as a trigger for what can verify semantics: the auditor reading the files. New check 9 flags (INFO only, never FAIL) any Description or Contributing-files text change against a base ref (default: merge-base with origin/main, overridable via --base-ref). A slug absent at the base ref is a creation, not a change, and is not flagged. skill-audit's rubric now tells the auditor a check-9 INFO means open the named files and verify by reading, not just relay it. skill-author's retrofit checklist gained a matching authoring-time guardrail: don't upgrade a hedge into a confident claim without re-reading the source first. 8 new bats tests (73 total, 0 failures). Fixes: #118 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EeH8SCbcrCAQrtymkNuhKP --- .../references/validation-scripts.md | 13 + .../scripts/validate-provenance.sh | 222 +++++++++++++++++- .../tests/validate-provenance.bats | 175 +++++++++++++- .../skill-author/references/retrofit.md | 10 + 4 files changed, 404 insertions(+), 16 deletions(-) diff --git a/plugins/kyberforge/.apm/skills/skill-audit/references/validation-scripts.md b/plugins/kyberforge/.apm/skills/skill-audit/references/validation-scripts.md index a1ddb24..1c50e36 100644 --- a/plugins/kyberforge/.apm/skills/skill-audit/references/validation-scripts.md +++ b/plugins/kyberforge/.apm/skills/skill-audit/references/validation-scripts.md @@ -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 — `'' changed for '' since ` — 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. diff --git a/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate-provenance.sh b/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate-provenance.sh index 4ed3629..9cec436 100755 --- a/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate-provenance.sh +++ b/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate-provenance.sh @@ -3,12 +3,19 @@ set -euo pipefail usage() { cat < +Usage: validate-provenance.sh [--base-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= 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 ` 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 '## ' 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 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=, 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 '## ' 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 diff --git a/plugins/kyberforge/.apm/skills/skill-audit/tests/validate-provenance.bats b/plugins/kyberforge/.apm/skills/skill-audit/tests/validate-provenance.bats index c5f7d95..36dd144 100644 --- a/plugins/kyberforge/.apm/skills/skill-audit/tests/validate-provenance.bats +++ b/plugins/kyberforge/.apm/skills/skill-audit/tests/validate-provenance.bats @@ -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" <> "$skill/references/sources.md" </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" +} diff --git a/plugins/kyberforge/.apm/skills/skill-author/references/retrofit.md b/plugins/kyberforge/.apm/skills/skill-author/references/retrofit.md index 37fa55d..337bb1d 100644 --- a/plugins/kyberforge/.apm/skills/skill-author/references/retrofit.md +++ b/plugins/kyberforge/.apm/skills/skill-author/references/retrofit.md @@ -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