diff --git a/plugins/kyberforge/.apm/skills/agent-audit/references/validation-scripts.md b/plugins/kyberforge/.apm/skills/agent-audit/references/validation-scripts.md index d6bc4ac..55e949c 100644 --- a/plugins/kyberforge/.apm/skills/agent-audit/references/validation-scripts.md +++ b/plugins/kyberforge/.apm/skills/agent-audit/references/validation-scripts.md @@ -34,8 +34,14 @@ first of these: `plugin.json` and no `apm.yml` falls through to project or user scope. `validate-provenance.sh` exits 0 silently when that walk does not land on a package root, and again -when the package has no provenance data. Silence from it is a pass, not a skip you need to -investigate. +when the package has no provenance data. Check the exit code before you believe the silence: + +- **0** — a pass, not a skip you need to investigate. Both silent cases above land here. +- **1** — real findings, on stdout with Why and Fix. +- **2** — the check never ran. A missing, doubled, non-file or wrongly-named argument, an + undecodable `apm.yml`, or an absent `python3`, each with a diagnostic on stderr and no findings + at all. Report the `### Provenance` dimension as unverified and quote the reason. An exit 2 is + never a clean pass: empty stdout there means nothing was checked, not that nothing was wrong. ## Manual fallback diff --git a/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate-provenance.sh b/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate-provenance.sh index 4b07ae3..1dcc003 100755 --- a/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate-provenance.sh +++ b/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate-provenance.sh @@ -16,7 +16,30 @@ Arguments: Exit codes: 0 All checks passed (or nothing to validate, or not plugin scope) 1 One or more checks failed - 2 Script error (unrecognized file extension — expected .md or .agent.md) + 2 Usage error, or the argument is not an agent file this script can read + +An exit code of 2 is NOT a finding. SKILL.md tells the auditor to surface a +non-zero exit as findings, so a usage error leaving exit 1 with nothing on +stdout was indistinguishable from a clean-but-failing run. Environment and +argument problems exit 2; only real findings exit 1. + +Exit 2 and the silent exit 0 answer two DIFFERENT questions, and neither may +be spelled with the other's code: + + exit 2 the argument is not something this script can audit at all — it is + missing, doubled, not a file, or not named .md / .agent.md. Decided + before the scope walk-up runs, from the argument alone. + exit 0 the argument IS a readable agent file, and the scope walk-up found + no type:-bearing apm.yml above it before hitting the \$HOME, .git or + filesystem-root boundary. That is a real verdict about a real file — + "this agent is user or project scope, so plugin-scope provenance + does not apply to it" — not a rejected input. + +scripts/check-scope-walkup-sync.sh's fixture 6 pins the second: a real agent +file under a \$HOME with a type-bearing apm.yml ABOVE it must exit 0 with empty +output. Widening exit 2 to cover "the walk-up found no package" would break +that fixture AND would be wrong on its own terms, because new-agent.sh happily +scaffolds exactly that layout. Checks performed: 0 source_keys present in agent pair but sources.md absent @@ -28,6 +51,13 @@ Checks performed: not run, never skipped silently. 4 Contributing files back-reference the parent slug in their source_keys 5 Research doc field present and not placeholder + +This script has no counterpart to skill-audit's checks 6, 7 and 8 (Research +doc field / upstream forward / upstream reverse are numbered 6, 7, 8 there and +5 here): an agent at plugin scope is a single file with a plugin-root +sources.md, so there is no references/ tree to walk and no upstream research +source index to cross-check. parse_status() and the sources.md-basename gate +that those checks need exist only in the skill-audit copy. EOF } @@ -36,26 +66,131 @@ if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then exit 0 fi +# Usage and environment problems exit 2, findings exit 1. See the usage text +# above for why the two must not share a code, and for why "not plugin scope" +# is neither of them. This is a deliberate divergence from validate.sh, which +# has no 2 tier for content: 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 echo "Error: agent-file is required." >&2 echo "" >&2 usage >&2 - exit 1 + exit 2 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 + echo "" >&2 + usage >&2 + exit 2 +fi + +# 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 +# rather than the missing dependency. +if ! command -v python3 > /dev/null 2>&1; then + echo "Error: python3 is required but was not found on PATH." >&2 + echo " Why: skipping the provenance checks entirely would be a vacuous pass." >&2 + echo " Fix: install python3 (pre-commit itself is a Python application, so it is almost certainly already present)." >&2 + exit 2 +fi + +# A path that does not exist, or exists but is not a regular file, used to reach +# the Python body, get os.path.dirname()'d into some ancestor directory and then +# either report a silent exit 0 (no package above it) or — worse — audit a +# DIFFERENT agent's package while naming the typo'd path. A typo'd target was +# indistinguishable from a clean agent. vale-wrap.sh hard-errors on a +# nonexistent path for exactly this reason. +# +# This is decided from the argument alone, before any walk-up runs, so it cannot +# collide with the not-plugin-scope exit 0: that verdict is only ever reached by +# a file that got past here. +if [[ ! -e "$1" ]]; then + echo "Error: no such file: $1" >&2 + echo " Why: a nonexistent target would otherwise report a silent pass." >&2 + echo " Fix: pass the path of the agent file to validate." >&2 + exit 2 +fi + +if [[ ! -f "$1" ]]; then + echo "Error: not a regular file: $1" >&2 + echo " Why: this script audits one agent file, not a directory of them, and reporting a directory as a pass hides the wrong-target mistake." >&2 + echo " Fix: pass the agent file itself — .apm/agents/.agent.md — not its parent directory." >&2 + exit 2 +fi + +# The extension check used to live inside the Python body. It stays exit 2 and +# keeps its wording; it moves up here so that every "this argument is not +# auditable" verdict is reached in one place, before the interpreter starts and +# before the scope walk-up can turn a bad argument into a silent exit 0. +case "$1" in + *.agent.md | *.md) ;; + *) + echo "Error: unrecognized extension '$(basename "$1")' — expected .md or .agent.md" >&2 + exit 2 + ;; +esac + python3 -u - "$1" <<'PYTHON' import sys import os import re +# 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. +# Pinning only the reads moved the crash from the read to the write — a +# UnicodeEncodeError inside print_findings(), which loses the whole report +# after all the checks have already run. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + agent_file = os.path.abspath(sys.argv[1]) -fname = os.path.basename(agent_file) agent_dir = os.path.dirname(agent_file) -# --- Sanity-check extension (single vendor-neutral .agent.md file at plugin/APM scope) --- -if not (fname.endswith('.agent.md') or fname.endswith('.md')): - print(f"Error: unrecognized extension '{fname}' — expected .md or .agent.md", file=sys.stderr) - sys.exit(2) +# --- Input ---------------------------------------------------------------- +# Ported from the skill-audit copy, where the same two problems were already +# fixed. +# +# read_text() pins UTF-8 explicitly instead of inheriting +# locale.getpreferredencoding(), which is ASCII under LC_ALL=C — an ordinary em +# dash in an agent file or in sources.md then aborted the run with a bare +# UnicodeDecodeError traceback, or, at the one call site that wrapped its read +# in `except Exception: return []`, reported the unreadable file as having no +# source_keys and therefore as clean. A file that genuinely is not UTF-8 still +# fails; it just says which file and why. +# +# strip_bom() runs on every read because a leading BOM defeats +# parse_frontmatter()'s `^---` anchor, which silently disabled check 2 on a +# BOM-prefixed agent file: no frontmatter parsed means no source_keys parsed +# means nothing to validate. + + +class EncodingError(Exception): + pass + + +def strip_bom(text): + return text[1:] if text.startswith(u'\ufeff') else text + + +def read_text(path): + """File contents as text, UTF-8 and BOM-free, with a diagnostic instead of a traceback.""" + try: + with open(path, encoding='utf-8') as fh: + return strip_bom(fh.read()) + except UnicodeDecodeError as exc: + raise EncodingError( + "not valid UTF-8 (%s at byte %d) — re-save the file as UTF-8; " + "this gate does not guess at other encodings" + % (exc.reason, exc.start)) # Matches a top-level `type:` line whose value is exactly one of the four # package content types — identical to validate.sh's APM_TYPE_RE. Group 1's @@ -70,15 +205,31 @@ TYPE_RE = re.compile(r"^type:\s*(['\"]?)(instructions|skill|hybrid|prompts)\1(?: # keep walking. Stop at a $HOME boundary, a .git boundary, or the filesystem # root: none of these is plugin/APM scope, so this script has nothing to # check there. +# +# Returning None here means NOT PLUGIN SCOPE, which is a verdict, not an error: +# the caller exits 0 silently, and scripts/check-scope-walkup-sync.sh fixture 6 +# pins that. It is deliberately NOT folded into the exit-2 tier above. def find_plugin_root(start_dir): home = os.path.expanduser('~') current = os.path.abspath(start_dir) while True: apm_yml = os.path.join(current, 'apm.yml') if os.path.isfile(apm_yml): - with open(apm_yml) as f: - if any(TYPE_RE.match(line) for line in f): - return current + # An apm.yml is a manifest this script must be able to READ to + # classify scope at all. Under LC_ALL=C the old bare open() decoded + # as ASCII, so a manifest with an accented author name raised + # UnicodeDecodeError mid-walk and killed the run with a traceback. + # It is an environment problem, not a finding, so it exits 2 rather + # than being swallowed into a silent "no package here". + try: + content = read_text(apm_yml) + except EncodingError as exc: + print( + "Error: %s is %s" % (apm_yml, exc), + file=sys.stderr) + sys.exit(2) + if any(TYPE_RE.match(line) for line in content.splitlines()): + return current # $HOME is a non-plugin-scope boundary — checked before the .git test # below (mirrors validate.sh's detect_scope ordering), so a # dotfiles-managed $HOME (yadm, chezmoi bare-repo, etc.) can't shadow @@ -104,7 +255,14 @@ if plugin_root is None: sources_md_path = os.path.join(plugin_root, 'sources.md') # --- Helpers --- -PLACEHOLDER_RE = re.compile(r'(? 1: + emit_info( + f"Duplicate '## {_slug}' entry in sources.md — only the first block is checked", + f"sources.md (## {_slug})", + f"'## {_slug}' appears {_count} times. Every field parser here takes the first match, so the " + f"second and later blocks' Contributing files and Research doc are never validated — " + f"checks 3, 4 and 5 did not run for them. " + f"Merge the blocks into one entry, or give each a distinct slug and reference it from source_keys." + ) + +for slug in unique_slugs: # Checks 3 and 4: Contributing files exist (paths relative to plugin root), # and back-reference the slug. `[]` and None are NOT the same answer here. # `[]` is the author writing "(none)" — there is nothing to check and the @@ -370,8 +585,11 @@ for slug in parse_h2_slugs(sources_content): ) else: # Check 4: Bidirectional — file should list slug in its source_keys - with open(cf_abs) as f: - cf_content = f.read() + try: + cf_content = read_text(cf_abs) + except EncodingError as exc: + emit_unreadable(cf_rel, exc) + continue cf_fm, _ = parse_frontmatter(cf_content) cf_keys = parse_source_keys(cf_fm) if slug not in cf_keys: @@ -383,7 +601,17 @@ for slug in parse_h2_slugs(sources_content): ) # Check 5: Research doc field required - rd_value = parse_research_doc(sources_content, slug) + rd_values = parse_research_docs(sources_content, slug) + if len(rd_values) > 1: + emit_info( + f"Multiple '- **Research doc:**' lines for '{slug}' — only the first is used", + f"sources.md (## {slug})", + f"The '## {slug}' entry has {len(rd_values)} Research doc lines; check 5 ran against the first " + f"('{rd_values[0]}') and never looked at the rest. " + f"Keep one Research doc line per entry — if a slug genuinely came from two documents, split it into two slugs, " + f"or name the extra document inside the first value's annotation where it is at least visible." + ) + rd_value = rd_values[0] if rd_values else None if rd_value is None: emit_fail( "Research doc field missing", diff --git a/plugins/kyberforge/.apm/skills/agent-audit/tests/validate-provenance.bats b/plugins/kyberforge/.apm/skills/agent-audit/tests/validate-provenance.bats index 186d484..7ae7edf 100644 --- a/plugins/kyberforge/.apm/skills/agent-audit/tests/validate-provenance.bats +++ b/plugins/kyberforge/.apm/skills/agent-audit/tests/validate-provenance.bats @@ -570,3 +570,340 @@ EOF assert_output --partial "INFO Contributing-file checks skipped for 'ghost-source'" assert_output --partial "Note:" } + +# --------------------------------------------------------------------------- +# The exit-2 tier, and its boundary with the silent exit 0 +# +# Ported from the skill-audit sibling, which had already split usage and +# environment errors (exit 2) away from findings (exit 1). SKILL.md tells the +# auditor to surface a non-zero exit, so a usage error leaving exit 1 with +# nothing on stdout was indistinguishable from a clean-but-failing run. +# +# The reconciliation this script needs and the sibling does not: "the walk-up +# found no type:-bearing apm.yml" is NOT bad input. It is a verdict about a +# real, readable agent file — user or project scope, where plugin-scope +# provenance does not apply — and scripts/check-scope-walkup-sync.sh fixture 6 +# pins it as exit 0 with empty output. Every exit-2 gate is therefore decided +# from the ARGUMENT ALONE, before the walk-up runs, so the two can never +# collide. The two tests at the end of this block assert both halves. +# --------------------------------------------------------------------------- + +@test "exit 2: no arguments is a usage error, not a finding" { + run bash "$SCRIPT" + [ "$status" -eq 2 ] + assert_output --partial "agent-file is required" +} + +@test "exit 2: a second positional argument is rejected instead of silently dropped" { + local root="$TMPDIR/package" + make_package "$root" + make_clean_agent "$root" + run bash "$SCRIPT" "$root/.apm/agents/my-agent.agent.md" --some-typo + [ "$status" -eq 2 ] + assert_output --partial "expected exactly one argument" +} + +@test "exit 2: a nonexistent path is an error, not a silent pass" { + run bash "$SCRIPT" "$TMPDIR/no-such-agent.agent.md" + [ "$status" -eq 2 ] + assert_output --partial "no such file" +} + +@test "exit 2: a directory is not an agent file" { + local root="$TMPDIR/package" + make_package "$root" + run bash "$SCRIPT" "$root/.apm/agents" + [ "$status" -eq 2 ] + assert_output --partial "not a regular file" +} + +@test "exit 2: an unrecognized extension is rejected before the walk-up runs" { + local root="$TMPDIR/package" + make_package "$root" + echo "not an agent" > "$root/.apm/agents/my-agent.txt" + run bash "$SCRIPT" "$root/.apm/agents/my-agent.txt" + [ "$status" -eq 2 ] + assert_output --partial "unrecognized extension" +} + +@test "exit 2: a PATH with no python3 names the missing dependency instead of exiting 127" { + local root="$TMPDIR/package" + make_package "$root" + make_clean_agent "$root" + local emptybin="$TMPDIR/emptybin" + mkdir -p "$emptybin" + local bash_bin + bash_bin="$(command -v bash)" + run env -i PATH="$emptybin" HOME="$HOME" "$bash_bin" "$SCRIPT" "$root/.apm/agents/my-agent.agent.md" + [ "$status" -eq 2 ] + # The needle is the DIAGNOSTIC, not the bare word: with no preflight, bash's + # own "python3: command not found" would satisfy a bare-word match. + assert_output --partial "python3 is required" +} + +@test "reconciliation: a REAL agent file at non-plugin scope still exits 0 silently, never 2" { + # scripts/check-scope-walkup-sync.sh fixture 6 in miniature. The exit-2 tier + # must not widen to cover "find_plugin_root returned None": the file exists, + # is readable and is correctly named — it is simply user/project scope. + local dir="$TMPDIR/anc" + mkdir -p "$dir" + cat > "$dir/apm.yml" < "$fake_home/.apm/agents/my-agent.agent.md" < "$root/sources.md" < "$root/sources.md" < "$root/.apm/agents/my-agent.agent.md" + make_sources_md "$root" "my-source" "(none)" + run bash "$SCRIPT" "$root/.apm/agents/my-agent.agent.md" + assert_failure + assert_output --partial "not valid UTF-8" + refute_output --partial "Traceback" +} + +@test "FAIL: an undecodable contributing file is reported, not a traceback" { + local root="$TMPDIR/package" + make_package "$root" + make_agent_with_source_keys "$root" + printf '\xff\xfe---\nname: other\n---\n' > "$root/.apm/agents/other.agent.md" + make_sources_md "$root" "my-source" ".apm/agents/other.agent.md" + run bash "$SCRIPT" "$root/.apm/agents/my-agent.agent.md" + assert_failure + assert_output --partial "not valid UTF-8" + refute_output --partial "Traceback" +} + +@test "exit 2: an undecodable apm.yml names the file instead of dying mid walk-up" { + local root="$TMPDIR/package" + make_package "$root" + make_clean_agent "$root" + printf 'name: t\nversion: 0.1.0\ntype: skill\n# \xff\xfe\n' > "$root/apm.yml" + run bash "$SCRIPT" "$root/.apm/agents/my-agent.agent.md" + [ "$status" -eq 2 ] + assert_output --partial "not valid UTF-8" + refute_output --partial "Traceback" +} + +@test "a BOM-prefixed agent file still has its source_keys read (check 2 runs)" { + # A leading BOM defeats parse_frontmatter()'s ^--- anchor, so no frontmatter + # parsed means no source_keys parsed means nothing to validate — check 2 + # went silently missing on exactly the file it was pointed at. + local root="$TMPDIR/package" + make_package "$root" + printf '\xef\xbb\xbf---\nname: my-agent\ndescription: A valid agent description.\nsource_keys:\n - ghost-source\n---\n\nYou are a test agent.\n' \ + > "$root/.apm/agents/my-agent.agent.md" + make_sources_md "$root" "my-source" "(none)" + run bash "$SCRIPT" "$root/.apm/agents/my-agent.agent.md" + assert_failure + assert_output --partial "source_keys slug 'ghost-source' not found in sources.md" +} + +@test "under LC_ALL=C a sources.md carrying an em dash is read, not a UnicodeDecodeError" { + local root="$TMPDIR/package" + make_package "$root" + make_agent_with_source_keys "$root" + cat > "$root/sources.md" < "$root/sources.md" < "$root/sources.md" < "$root/.apm/agents/my-agent.agent.md" + make_sources_md "$root" "my-source" ".apm/agents/my-agent.agent.md" + run bash "$SCRIPT" "$root/.apm/agents/my-agent.agent.md" + assert_failure + local count + count="$(printf '%s\n' "$output" | grep -c "^FAIL File is not valid UTF-8" || true)" + [ "$count" -eq 1 ] +} 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 89c7788..a1ddb24 100644 --- a/plugins/kyberforge/.apm/skills/skill-audit/references/validation-scripts.md +++ b/plugins/kyberforge/.apm/skills/skill-audit/references/validation-scripts.md @@ -99,9 +99,12 @@ Three ways to read the result wrong: ## Script-specific failures -- **`validate-provenance.sh` printed nothing.** That is a pass, not a skip. It also exits 0 - silently when the skill has no `source_keys` and no `references/sources.md` — nothing to - validate is not a finding. +- **`validate-provenance.sh` printed nothing *and exited 0*.** That is a pass, not a skip — it + exits 0 silently when the skill has no `source_keys` and no `references/sources.md`, and nothing + to validate is not a finding. Check the exit code before you believe the silence: a target that + 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. - **`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 2e8cca9..4ed3629 100755 --- a/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate-provenance.sh +++ b/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate-provenance.sh @@ -13,6 +13,12 @@ Arguments: Exit codes: 0 All checks passed (or nothing to validate) 1 One or more checks failed + 2 Usage error, or the argument is not a skill directory + +An exit code of 2 is NOT a finding. SKILL.md tells the auditor to surface a +non-zero exit as findings, so a usage error leaving exit 1 with nothing on +stdout was indistinguishable from a clean-but-failing run. Environment and +argument problems exit 2; only real findings exit 1. Checks performed: 0 source_keys present but references/sources.md absent @@ -32,6 +38,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 + + 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 + slugs. A Research doc pointing at a topic document is reported as an INFO + saying the two checks are not applicable, and every other reason they do not + run is announced the same way. EOF } @@ -40,11 +52,57 @@ if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then exit 0 fi +# 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 echo "Error: skill-dir is required." >&2 echo "" >&2 usage >&2 - exit 1 + exit 2 +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 + echo "" >&2 + usage >&2 + exit 2 +fi + +# 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 +# rather than the missing dependency. +if ! command -v python3 > /dev/null 2>&1; then + echo "Error: python3 is required but was not found on PATH." >&2 + echo " Why: skipping the provenance checks entirely would be a vacuous pass." >&2 + echo " Fix: install python3 (pre-commit itself is a Python application, so it is almost certainly already present)." >&2 + exit 2 +fi + +# A path that is not a directory, or a directory that is not a skill, used to +# reach the Python body, find no sources.md and no source_keys, take the +# "nothing to validate" early exit and report exit 0 with no output — which +# 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 + 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 + 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' @@ -52,13 +110,67 @@ import sys import os import re +# 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. +# Pinning only the reads moved the crash from the read to the write — a +# UnicodeEncodeError inside print_findings(), which loses the whole report +# after all the checks have already run. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + 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") # --- Helpers --- -PLACEHOLDER_RE = re.compile(r'(?/ was collected there — and then never validated here, +# because the flat listdir could not see it. The two halves of the same check +# disagreed about which files exist. if os.path.isdir(refs_dir): - for fname in sorted(os.listdir(refs_dir)): - if not fname.endswith('.md'): - continue - if fname == "sources.md": - continue - fpath = os.path.join(refs_dir, fname) + ref_paths = [] + for root, dirs, files in os.walk(refs_dir): + dirs[:] = sorted(d for d in dirs if not d.startswith('.')) + for fname in sorted(files): + if not fname.endswith('.md'): + continue + fpath = os.path.join(root, fname) + if os.path.relpath(fpath, refs_dir) == "sources.md": + continue + ref_paths.append(fpath) + for fpath in ref_paths: rel = os.path.relpath(fpath, skill_dir) - with open(fpath) as f: - ref_content = f.read() + try: + ref_content = read_text(fpath) + except EncodingError as exc: + emit_unreadable(rel, exc) + continue ref_fm, _ = parse_frontmatter(ref_content) ref_keys = parse_source_keys(ref_fm) if not ref_keys: @@ -442,9 +639,32 @@ if os.path.isdir(refs_dir): repo_root = find_repo_root(skill_dir) # Collect all research doc paths we'll check (for Check 8) -research_docs_seen = {} # abs_path → set of slugs in sources.md that reference it +research_docs_seen = {} # abs_path → (rel_path, slugs referencing it, content) -for slug in parse_h2_slugs(sources_content): +# Every per-slug parser below — parse_contributing_files, parse_research_docs, +# parse_status — locates its block with pattern.search(), so a slug written +# twice resolves to the FIRST block every time. Iterating the raw heading list +# therefore checked the first block's fields twice and the second block's +# never: a duplicated slug is half-validated, and looked fully validated. The +# duplicate is announced and the repeat visit dropped. +all_slugs = parse_h2_slugs(sources_content) +unique_slugs = [] +for _slug in all_slugs: + if _slug in unique_slugs: + continue + unique_slugs.append(_slug) + _count = all_slugs.count(_slug) + if _count > 1: + emit_info( + f"Duplicate '## {_slug}' entry in sources.md — only the first block is checked", + f"references/sources.md (## {_slug})", + f"'## {_slug}' appears {_count} times. Every field parser here takes the first match, so the " + f"second and later blocks' Contributing files, Research doc and Status are never validated — " + f"checks 4, 5, 6, 7 and 8 did not run for them. " + f"Merge the blocks into one entry, or give each a distinct slug and reference it from source_keys." + ) + +for slug in unique_slugs: # Checks 4 and 5: Contributing files exist, and back-reference the slug. # `[]` and None are NOT the same answer here. `[]` is the author writing # "(none)" — there is nothing to check and the skip is correct. None is a @@ -478,8 +698,11 @@ for slug in parse_h2_slugs(sources_content): # Skip sources.md itself if cf_rel == "references/sources.md": continue - with open(cf_abs) as f: - cf_content = f.read() + try: + cf_content = read_text(cf_abs) + except EncodingError as exc: + emit_unreadable(cf_rel, exc) + continue cf_fm, _ = parse_frontmatter(cf_content) cf_keys = parse_source_keys(cf_fm) if slug not in cf_keys: @@ -491,7 +714,17 @@ for slug in parse_h2_slugs(sources_content): ) # Check 6: Research doc field required - rd_value = parse_research_doc(sources_content, slug) + rd_values = parse_research_docs(sources_content, slug) + if len(rd_values) > 1: + emit_info( + f"Multiple '- **Research doc:**' lines for '{slug}' — only the first is used", + f"references/sources.md (## {slug})", + f"The '## {slug}' entry has {len(rd_values)} Research doc lines; checks 7 and 8 ran against the first " + f"('{rd_values[0]}') and never looked at the rest. " + f"Keep one Research doc line per entry — if a slug genuinely came from two documents, split it into two slugs, " + f"or name the extra document inside the first value's annotation where it is at least visible." + ) + rd_value = rd_values[0] if rd_values else None if rd_value is None: emit_fail( f"Research doc field missing", @@ -537,9 +770,41 @@ for slug in parse_h2_slugs(sources_content): f"Point the value at one existing file — a brace expansion, a comma-separated list of paths, or a bare section title does not resolve — " f"or record '(none)' if no research doc backs this entry." ) + elif os.path.basename(rd_path) != "sources.md": + # Checks 7 and 8 both assume the Research doc is a research + # SOURCE INDEX — a sources.md whose H2 headings ARE source + # slugs. 30 of the 121 corpus entries point instead at a TOPIC + # DOCUMENT (remotes.md, gitflow.md, api-reference.md), whose + # H2s are headings like '## Core Philosophy'. A slug can never + # match one, so check 7 reported all 30 as "slug not found" — + # every one a false positive — and check 8, aimed at documents + # that carry no '- **Status:**' line at all, was saved from a + # matching flood of false FAILs only by an UNANNOUNCED skip on + # that missing status. The premise, not the corpus, was wrong. + # + # A topic-document reference is a legitimate, useful value; it + # just is not something these two checks can verify. Say that + # once, out loud, instead of failing 30 entries for it. + emit_info( + f"Upstream checks not applicable for '{slug}' — research doc '{rd_path}' is a topic document, not a source index", + f"references/sources.md (## {slug})", + f"Checks 7 and 8 match slugs against the H2 headings of a research source index — a file named 'sources.md', " + f"where each H2 IS a source slug. '{os.path.basename(rd_path)}' is a topic document, so its H2s are section " + f"headings and no slug will ever match one. Checks 7 and 8 did not run for this slug. " + f"This needs no fix: point the value at the research corpus's own sources.md only if you want the " + f"provenance link machine-verified." + ) else: - with open(rd_abs) as f: - rd_content = f.read() + try: + rd_content = read_text(rd_abs) + except EncodingError as exc: + emit_info( + f"Upstream checks skipped for '{slug}' — research doc '{rd_path}' is {exc}", + f"references/sources.md (## {slug})", + f"'{rd_path}' could not be decoded, so checks 7 and 8 did not run for this slug. " + f"Re-save the research doc as UTF-8." + ) + continue rd_slugs = set(parse_h2_slugs(rd_content)) if slug not in rd_slugs: emit_info( @@ -548,15 +813,15 @@ for slug in parse_h2_slugs(sources_content): f"The research doc '{rd_path}' does not have a '## {slug}' heading. " f"The provenance link may be imprecise — the slug name in sources.md may differ from the research doc's heading." ) - # Track for Check 8 + # Track for Check 8. The content is carried with the entry so + # check 8 reuses this read rather than decoding the file a + # second time, with a second chance to fail differently. if rd_abs not in research_docs_seen: - research_docs_seen[rd_abs] = (rd_path, set()) + research_docs_seen[rd_abs] = (rd_path, set(), rd_content) research_docs_seen[rd_abs][1].add(slug) # --- Check 8: Upstream reverse --- -for rd_abs, (rd_rel, known_slugs) in research_docs_seen.items(): - with open(rd_abs) as f: - rd_content = f.read() +for rd_abs, (rd_rel, known_slugs, rd_content) in research_docs_seen.items(): for rd_slug in parse_h2_slugs(rd_content): # Parse this slug's Contributing files and Status in the research doc rd_cf = parse_contributing_files(rd_content, rd_slug) @@ -564,8 +829,25 @@ for rd_abs, (rd_rel, known_slugs) in research_docs_seen.items(): # Skip if the research doc explicitly records no contributing files if rd_cf == []: continue - # Skip if status is not `extracted` - if rd_status != "`extracted`": + # Skip if status is not `extracted` — and say so when the skip is what + # kept the slug out of the FAIL below. A status of `referenced` or + # `no content extracted` is a real reason not to demand the slug, but + # it was applied in silence, so an entry that should have been in + # sources.md and a status line nobody had updated produced the same + # output: nothing. Only a MATERIAL skip is announced; when the slug is + # already in sources.md the check passes either way and there is no + # fail-open to disclose. + if rd_status != "extracted": + if rd_slug not in sources_slugs: + shown = f"`{rd_status}`" if rd_status else "absent" + emit_info( + f"Check 8 skipped for research-doc slug '{rd_slug}' — its Status is {shown}, not `extracted`", + f"{rd_rel} (## {rd_slug})", + f"'{rd_rel}' has '## {rd_slug}' with contributing files but Status {shown}, and this skill's " + f"sources.md has no '## {rd_slug}' entry. Check 8 only demands an entry for an `extracted` slug, " + f"so it did not run here. If that status is stale — the content was extracted and the line was never " + f"updated — this skill is missing a source entry; if it is accurate, nothing needs doing." + ) continue # This slug should be in sources.md if rd_slug not in sources_slugs: 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 aef8d83..c5f7d95 100644 --- a/plugins/kyberforge/.apm/skills/skill-audit/tests/validate-provenance.bats +++ b/plugins/kyberforge/.apm/skills/skill-audit/tests/validate-provenance.bats @@ -71,10 +71,10 @@ EOF # 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/my-research.md". + # written per test into "$repo/docs/research/sources.md". make_upstream_skill() { local repo="$1" - local research="${2:-docs/research/my-research.md}" + local research="${2:-docs/research/sources.md}" local skill="$repo/my-skill" mkdir -p "$skill/references" "$repo/docs/research" touch "$repo/.git" @@ -375,7 +375,7 @@ EOF # Create a research doc that does NOT have the slug local research_dir="$TMPDIR/research" mkdir -p "$research_dir" - cat > "$research_dir/my-research.md" < "$research_dir/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/my-research.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/remotes.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/sources.md" < "$fake_repo/docs/research/sources.md" < "$skill/SKILL.md" + cat >> "$skill/SKILL.md" < "$skill/references/sources.md" < "$skill/references/topic.md" < "$skill/references/topic.md" + + run bash "$SCRIPT" "$skill" + assert_failure + assert_output --partial "not valid UTF-8" + assert_output --partial "references/topic.md" +} + +# --------------------------------------------------------------------------- +# Cycle 24 — G4: check 3 walks references/ recursively +# --------------------------------------------------------------------------- + +@test "G4: a source_keys file in references// is validated, not skipped" { + local skill="$TMPDIR/my-skill" + make_skill_with_source_keys "$skill" + make_sources_md "$skill" + mkdir -p "$skill/references/nested" + cat > "$skill/references/nested/topic.md" < "$skill/references/sources.md" < "$skill/references/sources.md" < "$skill/references/sources.md" < "$skill/references/topic.md" + + run bash "$SCRIPT" "$skill" + assert_failure + assert_output --partial "not valid UTF-8" +} diff --git a/plugins/kyberforge/skills/agent-audit/references/validation-scripts.md b/plugins/kyberforge/skills/agent-audit/references/validation-scripts.md index d6bc4ac..55e949c 100644 --- a/plugins/kyberforge/skills/agent-audit/references/validation-scripts.md +++ b/plugins/kyberforge/skills/agent-audit/references/validation-scripts.md @@ -34,8 +34,14 @@ first of these: `plugin.json` and no `apm.yml` falls through to project or user scope. `validate-provenance.sh` exits 0 silently when that walk does not land on a package root, and again -when the package has no provenance data. Silence from it is a pass, not a skip you need to -investigate. +when the package has no provenance data. Check the exit code before you believe the silence: + +- **0** — a pass, not a skip you need to investigate. Both silent cases above land here. +- **1** — real findings, on stdout with Why and Fix. +- **2** — the check never ran. A missing, doubled, non-file or wrongly-named argument, an + undecodable `apm.yml`, or an absent `python3`, each with a diagnostic on stderr and no findings + at all. Report the `### Provenance` dimension as unverified and quote the reason. An exit 2 is + never a clean pass: empty stdout there means nothing was checked, not that nothing was wrong. ## Manual fallback diff --git a/plugins/kyberforge/skills/agent-audit/scripts/validate-provenance.sh b/plugins/kyberforge/skills/agent-audit/scripts/validate-provenance.sh index 4b07ae3..1dcc003 100755 --- a/plugins/kyberforge/skills/agent-audit/scripts/validate-provenance.sh +++ b/plugins/kyberforge/skills/agent-audit/scripts/validate-provenance.sh @@ -16,7 +16,30 @@ Arguments: Exit codes: 0 All checks passed (or nothing to validate, or not plugin scope) 1 One or more checks failed - 2 Script error (unrecognized file extension — expected .md or .agent.md) + 2 Usage error, or the argument is not an agent file this script can read + +An exit code of 2 is NOT a finding. SKILL.md tells the auditor to surface a +non-zero exit as findings, so a usage error leaving exit 1 with nothing on +stdout was indistinguishable from a clean-but-failing run. Environment and +argument problems exit 2; only real findings exit 1. + +Exit 2 and the silent exit 0 answer two DIFFERENT questions, and neither may +be spelled with the other's code: + + exit 2 the argument is not something this script can audit at all — it is + missing, doubled, not a file, or not named .md / .agent.md. Decided + before the scope walk-up runs, from the argument alone. + exit 0 the argument IS a readable agent file, and the scope walk-up found + no type:-bearing apm.yml above it before hitting the \$HOME, .git or + filesystem-root boundary. That is a real verdict about a real file — + "this agent is user or project scope, so plugin-scope provenance + does not apply to it" — not a rejected input. + +scripts/check-scope-walkup-sync.sh's fixture 6 pins the second: a real agent +file under a \$HOME with a type-bearing apm.yml ABOVE it must exit 0 with empty +output. Widening exit 2 to cover "the walk-up found no package" would break +that fixture AND would be wrong on its own terms, because new-agent.sh happily +scaffolds exactly that layout. Checks performed: 0 source_keys present in agent pair but sources.md absent @@ -28,6 +51,13 @@ Checks performed: not run, never skipped silently. 4 Contributing files back-reference the parent slug in their source_keys 5 Research doc field present and not placeholder + +This script has no counterpart to skill-audit's checks 6, 7 and 8 (Research +doc field / upstream forward / upstream reverse are numbered 6, 7, 8 there and +5 here): an agent at plugin scope is a single file with a plugin-root +sources.md, so there is no references/ tree to walk and no upstream research +source index to cross-check. parse_status() and the sources.md-basename gate +that those checks need exist only in the skill-audit copy. EOF } @@ -36,26 +66,131 @@ if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then exit 0 fi +# Usage and environment problems exit 2, findings exit 1. See the usage text +# above for why the two must not share a code, and for why "not plugin scope" +# is neither of them. This is a deliberate divergence from validate.sh, which +# has no 2 tier for content: 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 echo "Error: agent-file is required." >&2 echo "" >&2 usage >&2 - exit 1 + exit 2 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 + echo "" >&2 + usage >&2 + exit 2 +fi + +# 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 +# rather than the missing dependency. +if ! command -v python3 > /dev/null 2>&1; then + echo "Error: python3 is required but was not found on PATH." >&2 + echo " Why: skipping the provenance checks entirely would be a vacuous pass." >&2 + echo " Fix: install python3 (pre-commit itself is a Python application, so it is almost certainly already present)." >&2 + exit 2 +fi + +# A path that does not exist, or exists but is not a regular file, used to reach +# the Python body, get os.path.dirname()'d into some ancestor directory and then +# either report a silent exit 0 (no package above it) or — worse — audit a +# DIFFERENT agent's package while naming the typo'd path. A typo'd target was +# indistinguishable from a clean agent. vale-wrap.sh hard-errors on a +# nonexistent path for exactly this reason. +# +# This is decided from the argument alone, before any walk-up runs, so it cannot +# collide with the not-plugin-scope exit 0: that verdict is only ever reached by +# a file that got past here. +if [[ ! -e "$1" ]]; then + echo "Error: no such file: $1" >&2 + echo " Why: a nonexistent target would otherwise report a silent pass." >&2 + echo " Fix: pass the path of the agent file to validate." >&2 + exit 2 +fi + +if [[ ! -f "$1" ]]; then + echo "Error: not a regular file: $1" >&2 + echo " Why: this script audits one agent file, not a directory of them, and reporting a directory as a pass hides the wrong-target mistake." >&2 + echo " Fix: pass the agent file itself — .apm/agents/.agent.md — not its parent directory." >&2 + exit 2 +fi + +# The extension check used to live inside the Python body. It stays exit 2 and +# keeps its wording; it moves up here so that every "this argument is not +# auditable" verdict is reached in one place, before the interpreter starts and +# before the scope walk-up can turn a bad argument into a silent exit 0. +case "$1" in + *.agent.md | *.md) ;; + *) + echo "Error: unrecognized extension '$(basename "$1")' — expected .md or .agent.md" >&2 + exit 2 + ;; +esac + python3 -u - "$1" <<'PYTHON' import sys import os import re +# 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. +# Pinning only the reads moved the crash from the read to the write — a +# UnicodeEncodeError inside print_findings(), which loses the whole report +# after all the checks have already run. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + agent_file = os.path.abspath(sys.argv[1]) -fname = os.path.basename(agent_file) agent_dir = os.path.dirname(agent_file) -# --- Sanity-check extension (single vendor-neutral .agent.md file at plugin/APM scope) --- -if not (fname.endswith('.agent.md') or fname.endswith('.md')): - print(f"Error: unrecognized extension '{fname}' — expected .md or .agent.md", file=sys.stderr) - sys.exit(2) +# --- Input ---------------------------------------------------------------- +# Ported from the skill-audit copy, where the same two problems were already +# fixed. +# +# read_text() pins UTF-8 explicitly instead of inheriting +# locale.getpreferredencoding(), which is ASCII under LC_ALL=C — an ordinary em +# dash in an agent file or in sources.md then aborted the run with a bare +# UnicodeDecodeError traceback, or, at the one call site that wrapped its read +# in `except Exception: return []`, reported the unreadable file as having no +# source_keys and therefore as clean. A file that genuinely is not UTF-8 still +# fails; it just says which file and why. +# +# strip_bom() runs on every read because a leading BOM defeats +# parse_frontmatter()'s `^---` anchor, which silently disabled check 2 on a +# BOM-prefixed agent file: no frontmatter parsed means no source_keys parsed +# means nothing to validate. + + +class EncodingError(Exception): + pass + + +def strip_bom(text): + return text[1:] if text.startswith(u'\ufeff') else text + + +def read_text(path): + """File contents as text, UTF-8 and BOM-free, with a diagnostic instead of a traceback.""" + try: + with open(path, encoding='utf-8') as fh: + return strip_bom(fh.read()) + except UnicodeDecodeError as exc: + raise EncodingError( + "not valid UTF-8 (%s at byte %d) — re-save the file as UTF-8; " + "this gate does not guess at other encodings" + % (exc.reason, exc.start)) # Matches a top-level `type:` line whose value is exactly one of the four # package content types — identical to validate.sh's APM_TYPE_RE. Group 1's @@ -70,15 +205,31 @@ TYPE_RE = re.compile(r"^type:\s*(['\"]?)(instructions|skill|hybrid|prompts)\1(?: # keep walking. Stop at a $HOME boundary, a .git boundary, or the filesystem # root: none of these is plugin/APM scope, so this script has nothing to # check there. +# +# Returning None here means NOT PLUGIN SCOPE, which is a verdict, not an error: +# the caller exits 0 silently, and scripts/check-scope-walkup-sync.sh fixture 6 +# pins that. It is deliberately NOT folded into the exit-2 tier above. def find_plugin_root(start_dir): home = os.path.expanduser('~') current = os.path.abspath(start_dir) while True: apm_yml = os.path.join(current, 'apm.yml') if os.path.isfile(apm_yml): - with open(apm_yml) as f: - if any(TYPE_RE.match(line) for line in f): - return current + # An apm.yml is a manifest this script must be able to READ to + # classify scope at all. Under LC_ALL=C the old bare open() decoded + # as ASCII, so a manifest with an accented author name raised + # UnicodeDecodeError mid-walk and killed the run with a traceback. + # It is an environment problem, not a finding, so it exits 2 rather + # than being swallowed into a silent "no package here". + try: + content = read_text(apm_yml) + except EncodingError as exc: + print( + "Error: %s is %s" % (apm_yml, exc), + file=sys.stderr) + sys.exit(2) + if any(TYPE_RE.match(line) for line in content.splitlines()): + return current # $HOME is a non-plugin-scope boundary — checked before the .git test # below (mirrors validate.sh's detect_scope ordering), so a # dotfiles-managed $HOME (yadm, chezmoi bare-repo, etc.) can't shadow @@ -104,7 +255,14 @@ if plugin_root is None: sources_md_path = os.path.join(plugin_root, 'sources.md') # --- Helpers --- -PLACEHOLDER_RE = re.compile(r'(? 1: + emit_info( + f"Duplicate '## {_slug}' entry in sources.md — only the first block is checked", + f"sources.md (## {_slug})", + f"'## {_slug}' appears {_count} times. Every field parser here takes the first match, so the " + f"second and later blocks' Contributing files and Research doc are never validated — " + f"checks 3, 4 and 5 did not run for them. " + f"Merge the blocks into one entry, or give each a distinct slug and reference it from source_keys." + ) + +for slug in unique_slugs: # Checks 3 and 4: Contributing files exist (paths relative to plugin root), # and back-reference the slug. `[]` and None are NOT the same answer here. # `[]` is the author writing "(none)" — there is nothing to check and the @@ -370,8 +585,11 @@ for slug in parse_h2_slugs(sources_content): ) else: # Check 4: Bidirectional — file should list slug in its source_keys - with open(cf_abs) as f: - cf_content = f.read() + try: + cf_content = read_text(cf_abs) + except EncodingError as exc: + emit_unreadable(cf_rel, exc) + continue cf_fm, _ = parse_frontmatter(cf_content) cf_keys = parse_source_keys(cf_fm) if slug not in cf_keys: @@ -383,7 +601,17 @@ for slug in parse_h2_slugs(sources_content): ) # Check 5: Research doc field required - rd_value = parse_research_doc(sources_content, slug) + rd_values = parse_research_docs(sources_content, slug) + if len(rd_values) > 1: + emit_info( + f"Multiple '- **Research doc:**' lines for '{slug}' — only the first is used", + f"sources.md (## {slug})", + f"The '## {slug}' entry has {len(rd_values)} Research doc lines; check 5 ran against the first " + f"('{rd_values[0]}') and never looked at the rest. " + f"Keep one Research doc line per entry — if a slug genuinely came from two documents, split it into two slugs, " + f"or name the extra document inside the first value's annotation where it is at least visible." + ) + rd_value = rd_values[0] if rd_values else None if rd_value is None: emit_fail( "Research doc field missing", diff --git a/plugins/kyberforge/skills/skill-audit/references/validation-scripts.md b/plugins/kyberforge/skills/skill-audit/references/validation-scripts.md index 89c7788..a1ddb24 100644 --- a/plugins/kyberforge/skills/skill-audit/references/validation-scripts.md +++ b/plugins/kyberforge/skills/skill-audit/references/validation-scripts.md @@ -99,9 +99,12 @@ Three ways to read the result wrong: ## Script-specific failures -- **`validate-provenance.sh` printed nothing.** That is a pass, not a skip. It also exits 0 - silently when the skill has no `source_keys` and no `references/sources.md` — nothing to - validate is not a finding. +- **`validate-provenance.sh` printed nothing *and exited 0*.** That is a pass, not a skip — it + exits 0 silently when the skill has no `source_keys` and no `references/sources.md`, and nothing + to validate is not a finding. Check the exit code before you believe the silence: a target that + 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. - **`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/skills/skill-audit/scripts/validate-provenance.sh b/plugins/kyberforge/skills/skill-audit/scripts/validate-provenance.sh index 2e8cca9..4ed3629 100755 --- a/plugins/kyberforge/skills/skill-audit/scripts/validate-provenance.sh +++ b/plugins/kyberforge/skills/skill-audit/scripts/validate-provenance.sh @@ -13,6 +13,12 @@ Arguments: Exit codes: 0 All checks passed (or nothing to validate) 1 One or more checks failed + 2 Usage error, or the argument is not a skill directory + +An exit code of 2 is NOT a finding. SKILL.md tells the auditor to surface a +non-zero exit as findings, so a usage error leaving exit 1 with nothing on +stdout was indistinguishable from a clean-but-failing run. Environment and +argument problems exit 2; only real findings exit 1. Checks performed: 0 source_keys present but references/sources.md absent @@ -32,6 +38,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 + + 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 + slugs. A Research doc pointing at a topic document is reported as an INFO + saying the two checks are not applicable, and every other reason they do not + run is announced the same way. EOF } @@ -40,11 +52,57 @@ if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then exit 0 fi +# 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 echo "Error: skill-dir is required." >&2 echo "" >&2 usage >&2 - exit 1 + exit 2 +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 + echo "" >&2 + usage >&2 + exit 2 +fi + +# 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 +# rather than the missing dependency. +if ! command -v python3 > /dev/null 2>&1; then + echo "Error: python3 is required but was not found on PATH." >&2 + echo " Why: skipping the provenance checks entirely would be a vacuous pass." >&2 + echo " Fix: install python3 (pre-commit itself is a Python application, so it is almost certainly already present)." >&2 + exit 2 +fi + +# A path that is not a directory, or a directory that is not a skill, used to +# reach the Python body, find no sources.md and no source_keys, take the +# "nothing to validate" early exit and report exit 0 with no output — which +# 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 + 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 + 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' @@ -52,13 +110,67 @@ import sys import os import re +# 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. +# Pinning only the reads moved the crash from the read to the write — a +# UnicodeEncodeError inside print_findings(), which loses the whole report +# after all the checks have already run. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + 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") # --- Helpers --- -PLACEHOLDER_RE = re.compile(r'(?/ was collected there — and then never validated here, +# because the flat listdir could not see it. The two halves of the same check +# disagreed about which files exist. if os.path.isdir(refs_dir): - for fname in sorted(os.listdir(refs_dir)): - if not fname.endswith('.md'): - continue - if fname == "sources.md": - continue - fpath = os.path.join(refs_dir, fname) + ref_paths = [] + for root, dirs, files in os.walk(refs_dir): + dirs[:] = sorted(d for d in dirs if not d.startswith('.')) + for fname in sorted(files): + if not fname.endswith('.md'): + continue + fpath = os.path.join(root, fname) + if os.path.relpath(fpath, refs_dir) == "sources.md": + continue + ref_paths.append(fpath) + for fpath in ref_paths: rel = os.path.relpath(fpath, skill_dir) - with open(fpath) as f: - ref_content = f.read() + try: + ref_content = read_text(fpath) + except EncodingError as exc: + emit_unreadable(rel, exc) + continue ref_fm, _ = parse_frontmatter(ref_content) ref_keys = parse_source_keys(ref_fm) if not ref_keys: @@ -442,9 +639,32 @@ if os.path.isdir(refs_dir): repo_root = find_repo_root(skill_dir) # Collect all research doc paths we'll check (for Check 8) -research_docs_seen = {} # abs_path → set of slugs in sources.md that reference it +research_docs_seen = {} # abs_path → (rel_path, slugs referencing it, content) -for slug in parse_h2_slugs(sources_content): +# Every per-slug parser below — parse_contributing_files, parse_research_docs, +# parse_status — locates its block with pattern.search(), so a slug written +# twice resolves to the FIRST block every time. Iterating the raw heading list +# therefore checked the first block's fields twice and the second block's +# never: a duplicated slug is half-validated, and looked fully validated. The +# duplicate is announced and the repeat visit dropped. +all_slugs = parse_h2_slugs(sources_content) +unique_slugs = [] +for _slug in all_slugs: + if _slug in unique_slugs: + continue + unique_slugs.append(_slug) + _count = all_slugs.count(_slug) + if _count > 1: + emit_info( + f"Duplicate '## {_slug}' entry in sources.md — only the first block is checked", + f"references/sources.md (## {_slug})", + f"'## {_slug}' appears {_count} times. Every field parser here takes the first match, so the " + f"second and later blocks' Contributing files, Research doc and Status are never validated — " + f"checks 4, 5, 6, 7 and 8 did not run for them. " + f"Merge the blocks into one entry, or give each a distinct slug and reference it from source_keys." + ) + +for slug in unique_slugs: # Checks 4 and 5: Contributing files exist, and back-reference the slug. # `[]` and None are NOT the same answer here. `[]` is the author writing # "(none)" — there is nothing to check and the skip is correct. None is a @@ -478,8 +698,11 @@ for slug in parse_h2_slugs(sources_content): # Skip sources.md itself if cf_rel == "references/sources.md": continue - with open(cf_abs) as f: - cf_content = f.read() + try: + cf_content = read_text(cf_abs) + except EncodingError as exc: + emit_unreadable(cf_rel, exc) + continue cf_fm, _ = parse_frontmatter(cf_content) cf_keys = parse_source_keys(cf_fm) if slug not in cf_keys: @@ -491,7 +714,17 @@ for slug in parse_h2_slugs(sources_content): ) # Check 6: Research doc field required - rd_value = parse_research_doc(sources_content, slug) + rd_values = parse_research_docs(sources_content, slug) + if len(rd_values) > 1: + emit_info( + f"Multiple '- **Research doc:**' lines for '{slug}' — only the first is used", + f"references/sources.md (## {slug})", + f"The '## {slug}' entry has {len(rd_values)} Research doc lines; checks 7 and 8 ran against the first " + f"('{rd_values[0]}') and never looked at the rest. " + f"Keep one Research doc line per entry — if a slug genuinely came from two documents, split it into two slugs, " + f"or name the extra document inside the first value's annotation where it is at least visible." + ) + rd_value = rd_values[0] if rd_values else None if rd_value is None: emit_fail( f"Research doc field missing", @@ -537,9 +770,41 @@ for slug in parse_h2_slugs(sources_content): f"Point the value at one existing file — a brace expansion, a comma-separated list of paths, or a bare section title does not resolve — " f"or record '(none)' if no research doc backs this entry." ) + elif os.path.basename(rd_path) != "sources.md": + # Checks 7 and 8 both assume the Research doc is a research + # SOURCE INDEX — a sources.md whose H2 headings ARE source + # slugs. 30 of the 121 corpus entries point instead at a TOPIC + # DOCUMENT (remotes.md, gitflow.md, api-reference.md), whose + # H2s are headings like '## Core Philosophy'. A slug can never + # match one, so check 7 reported all 30 as "slug not found" — + # every one a false positive — and check 8, aimed at documents + # that carry no '- **Status:**' line at all, was saved from a + # matching flood of false FAILs only by an UNANNOUNCED skip on + # that missing status. The premise, not the corpus, was wrong. + # + # A topic-document reference is a legitimate, useful value; it + # just is not something these two checks can verify. Say that + # once, out loud, instead of failing 30 entries for it. + emit_info( + f"Upstream checks not applicable for '{slug}' — research doc '{rd_path}' is a topic document, not a source index", + f"references/sources.md (## {slug})", + f"Checks 7 and 8 match slugs against the H2 headings of a research source index — a file named 'sources.md', " + f"where each H2 IS a source slug. '{os.path.basename(rd_path)}' is a topic document, so its H2s are section " + f"headings and no slug will ever match one. Checks 7 and 8 did not run for this slug. " + f"This needs no fix: point the value at the research corpus's own sources.md only if you want the " + f"provenance link machine-verified." + ) else: - with open(rd_abs) as f: - rd_content = f.read() + try: + rd_content = read_text(rd_abs) + except EncodingError as exc: + emit_info( + f"Upstream checks skipped for '{slug}' — research doc '{rd_path}' is {exc}", + f"references/sources.md (## {slug})", + f"'{rd_path}' could not be decoded, so checks 7 and 8 did not run for this slug. " + f"Re-save the research doc as UTF-8." + ) + continue rd_slugs = set(parse_h2_slugs(rd_content)) if slug not in rd_slugs: emit_info( @@ -548,15 +813,15 @@ for slug in parse_h2_slugs(sources_content): f"The research doc '{rd_path}' does not have a '## {slug}' heading. " f"The provenance link may be imprecise — the slug name in sources.md may differ from the research doc's heading." ) - # Track for Check 8 + # Track for Check 8. The content is carried with the entry so + # check 8 reuses this read rather than decoding the file a + # second time, with a second chance to fail differently. if rd_abs not in research_docs_seen: - research_docs_seen[rd_abs] = (rd_path, set()) + research_docs_seen[rd_abs] = (rd_path, set(), rd_content) research_docs_seen[rd_abs][1].add(slug) # --- Check 8: Upstream reverse --- -for rd_abs, (rd_rel, known_slugs) in research_docs_seen.items(): - with open(rd_abs) as f: - rd_content = f.read() +for rd_abs, (rd_rel, known_slugs, rd_content) in research_docs_seen.items(): for rd_slug in parse_h2_slugs(rd_content): # Parse this slug's Contributing files and Status in the research doc rd_cf = parse_contributing_files(rd_content, rd_slug) @@ -564,8 +829,25 @@ for rd_abs, (rd_rel, known_slugs) in research_docs_seen.items(): # Skip if the research doc explicitly records no contributing files if rd_cf == []: continue - # Skip if status is not `extracted` - if rd_status != "`extracted`": + # Skip if status is not `extracted` — and say so when the skip is what + # kept the slug out of the FAIL below. A status of `referenced` or + # `no content extracted` is a real reason not to demand the slug, but + # it was applied in silence, so an entry that should have been in + # sources.md and a status line nobody had updated produced the same + # output: nothing. Only a MATERIAL skip is announced; when the slug is + # already in sources.md the check passes either way and there is no + # fail-open to disclose. + if rd_status != "extracted": + if rd_slug not in sources_slugs: + shown = f"`{rd_status}`" if rd_status else "absent" + emit_info( + f"Check 8 skipped for research-doc slug '{rd_slug}' — its Status is {shown}, not `extracted`", + f"{rd_rel} (## {rd_slug})", + f"'{rd_rel}' has '## {rd_slug}' with contributing files but Status {shown}, and this skill's " + f"sources.md has no '## {rd_slug}' entry. Check 8 only demands an entry for an `extracted` slug, " + f"so it did not run here. If that status is stale — the content was extracted and the line was never " + f"updated — this skill is missing a source entry; if it is accurate, nothing needs doing." + ) continue # This slug should be in sources.md if rd_slug not in sources_slugs: