From b0d6d08239d22b70e77386b35c44a925509d57e7 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Sun, 16 Aug 2026 19:49:15 +0000 Subject: [PATCH] test: pin the nine ADR-0020 gate defects that shipped untested MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every defect fixed in f7cc279 was reachable because nothing asserted against it. The gate had 43 assertions and none of them covered a consumer repo, a non-string description, an unclosed fence, or the two spec ceilings. Each case below fails against the pre-fix code and passes against the current one; every one was proved non-vacuous by mutating a scratch copy of the script and watching the test go red, independently twice. The two that mattered most had no fixture anywhere. A consumer repo WITH .git is the shape the resolver exists to serve, and only the no-.git case had ever been tested, which is exactly why the blocker was invisible. And ADR-0020 says the walk-up runs in two passes specifically so a nested .git cannot beat a plugins/ root further up — no fixture had ever placed a .git inside a plugin. test-adr0020-differential.sh loses _non_adr_hook_error(). It excluded MAX_LINES and MAX_WORDS from the cross-script comparison on the untested assumption that awk and splitlines() agree. They do not, and the divergence stayed invisible for exactly as long as the exclusion stood. The ceilings are now compared like any other rule. Two existing assertions were repairs, not additions. The skill-improve probe had been fixed by this very branch, so its iteration permanently took an assertion-free SKIP that still counted as a pass; both branches now fail loudly and each names the other file's pin so the two stay in step. And the yaml-none fixture emitted `---/---`, which never matched the frontmatter pattern at all — it passed on the bare word "frontmatter", present in both messages, while never reaching the branch it was named for. Needles throughout that file now name their branch. Refs: #99 ADR: 0020 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015W3iwF9ncfRZddGBxsMCYi --- .../skills/agent-audit/tests/validate.bats | 133 +++++++++++++ tests/test-adr0020-body-checks.sh | 70 +++++++ tests/test-adr0020-differential.sh | 135 ++++++++++--- tests/test-adr0020-frontmatter.sh | 170 ++++++++++++++++- tests/test-adr0020-targets.sh | 177 ++++++++++++++++++ tests/test-skill-size-check.sh | 30 ++- 6 files changed, 673 insertions(+), 42 deletions(-) diff --git a/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats b/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats index fbe1572..b58033e 100644 --- a/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats +++ b/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats @@ -810,3 +810,136 @@ EOF assert_success refute_output --partial "hooks" } + +# --------------------------------------------------------------------------- +# tools: — both YAML spellings +# --------------------------------------------------------------------------- +# The subagent-unavailable-tool SUGGESTION is read off the `tools` field, and +# `tools` has two legal spellings: an inline scalar and a block sequence. The +# field used to be pulled out with a line regex whose capture is newline-bounded +# on purpose, so a block sequence captured NOTHING and the check silently +# stopped firing — on the shape Copilot agent files actually use, which is to say +# on the files it was written for. Both spellings are pinned, and they are pinned +# together: the inline case alone was green throughout. + +# make_pair — a project-scope CC + Copilot pair +# carrying the same `tools` value in both files. `tools` is on neither the +# claude-code-only nor the copilot-only list, so it is legal in both and the pair +# stays otherwise clean; the description carries a boundary clause so the only +# SUGGESTION that can fire is the one under test. +make_tools_pair() { + local root="$1" tools="$2" + mkdir -p "$root/.git" "$root/.claude/agents" "$root/.github/agents" + local f + for f in "$root/.claude/agents/my-agent.md" "$root/.github/agents/my-agent.agent.md"; do + { + echo "---" + echo "name: my-agent" + echo "description: A valid agent description. Do not use for anything else." + echo "$tools" + echo "---" + echo "" + echo "You are a test agent. When invoked, do the thing." + } > "$f" + done +} + +@test "a subagent-unavailable tool in an INLINE tools scalar raises a SUGGESTION" { + make_tools_pair "$TMPDIR/inline" "tools: Read ExitPlanMode" + run bash "$SCRIPT" "$TMPDIR/inline/.claude/agents/my-agent.md" + assert_success + assert_output --partial "'ExitPlanMode' is listed in tools but is never available to subagents" +} + +@test "a subagent-unavailable tool in a BLOCK SEQUENCE tools field raises the same SUGGESTION" { + make_tools_pair "$TMPDIR/block" "$(printf 'tools:\n - Read\n - ExitPlanMode')" + run bash "$SCRIPT" "$TMPDIR/block/.claude/agents/my-agent.md" + assert_success + assert_output --partial "'ExitPlanMode' is listed in tools but is never available to subagents" +} + +@test "a tools list with no subagent-unavailable tool stays silent in both spellings" { + # The control. Without it both cases above are satisfied by a check that + # fires on every tools field it can see, which would be the opposite defect. + make_tools_pair "$TMPDIR/inline-clean" "tools: Read Edit" + run bash "$SCRIPT" "$TMPDIR/inline-clean/.claude/agents/my-agent.md" + assert_success + refute_output --partial "never available to subagents" + + make_tools_pair "$TMPDIR/block-clean" "$(printf 'tools:\n - Read\n - Edit')" + run bash "$SCRIPT" "$TMPDIR/block-clean/.claude/agents/my-agent.md" + assert_success + refute_output --partial "never available to subagents" +} + +# --------------------------------------------------------------------------- +# A file that cannot be read +# --------------------------------------------------------------------------- +# scripts/check-apm-agents-valid.sh derives its expected agent-file set from +# `git ls-files`, so it hands this script paths that are tracked but absent from +# the worktree — a real and expected state, not a corner case. That used to exit +# 1 with a bare FileNotFoundError traceback and no FAIL line at all: non-zero, so +# the gate blocked, but with an interpreter stack instead of a diagnostic naming +# the file. Both scope paths are covered because they are separate call sites +# (check_apm_agent_file and check_file) and each needed its own handler. +# +# `is-a-dir.agent.md` is a DIRECTORY rather than a chmod 000 file on purpose: +# these tests run as root in CI, where mode bits do not deny anything and a +# permissions fixture would be silently readable and prove nothing. + +@test "a nonexistent plugin/APM-scope agent file gets a FAIL naming the path, not a traceback" { + local root="$TMPDIR/pkg" + mkdir -p "$root/.apm/agents" + cat > "$root/apm.yml" < "$root/apm.yml" < "$root/.github/agents/my-agent.agent.md" <> "$FX/gotchas-fraction/SKILL.md" +# The agentskills.io spec ceilings, measured over Unicode whitespace. +# +# These two are in the comparison at all because they used to be excluded from +# it — `_non_adr_hook_error()` waved a spec-ceiling exit through as "not a +# disagreement", and that exclusion is exactly why the divergence below stayed +# invisible. The hook counted lines and words in a single awk pass (NR / NF) +# while skill-audit counted them with Python's splitlines() / split(). The two +# primitives do not agree: splitlines() also breaks on U+2028, U+2029, \x0b, +# \x0c, \x1c-\x1e and \x85, and split() breaks on every Unicode space. Same +# constants, same file, different verdict — hook green, audit FAIL, which is the +# precise failure mode ("passes its own audit, blocked by the commit hook", +# inverted) this whole suite exists to catch. +# +# One fixture per primitive, each sitting just past its ceiling on the Python +# measurement and nowhere near it on the awk one. +python3 - "$FX" <<'PY' +import os +import sys + +fx = sys.argv[1] +# Spelled as escapes, never as literals. An invisible separator pasted into a +# source file is unreviewable and one editor round-trip away from becoming an +# ordinary space, which would silently turn both fixtures into nothing. +SEP_LINE = '\u2028' # LINE SEPARATOR: splitlines() breaks on it, awk's NR does not +SEP_WORD = '\u00a0' # NO-BREAK SPACE: split() breaks on it, awk's NF does not +head = ('---\nname: %s\n' + 'description: Use when doing the thing. Do not use for anything else.\n' + '---\n\n') +# 600 U+2028-separated segments: 605 lines to splitlines(), 6 to awk's NR. +# Word count stays far below the 2,770 ceiling, so this fixture isolates lines. +cases = { + 'spec-lines-u2028': SEP_LINE.join(['word'] * 600), + # 2,800 U+00A0-separated words: 2,816 words to split(), 17 to awk's NF. + 'spec-words-u00a0': SEP_WORD.join(['word'] * 2800), +} +for name, body in cases.items(): + d = os.path.join(fx, name) + os.makedirs(d, exist_ok=True) + with open(os.path.join(d, 'SKILL.md'), 'w', encoding='utf-8') as fh: + fh.write(head % name + body + '\n') +PY + # Empty description — the shape that used to exit 0 in silence. mkdir -p "$FX/empty-desc" printf -- '---\nname: empty-desc\ndescription:\nmodel: sonnet\n---\n\nDo the thing.\n' \ @@ -219,6 +267,18 @@ RULES = ( ('NO_BOUNDARY_CLAUSE', re.compile(r'(description has no boundary clause)')), ('RESOLUTION_DECLINED', re.compile(r'(boundary-target resolution DID NOT RUN)')), ('DESC_EMPTY', re.compile(r'(description field is missing or empty)')), + # The agentskills.io spec ceilings. These were EXCLUDED from the comparison + # until the awk/Python divergence shipped, on the reasoning that "the hook + # checks whole-file lines and words" and the auditor did not. It does — with + # the same two constants — so the exclusion was never a scope decision, only + # an untested assumption, and it hid a real disagreement. Both scripts spell + # the finding differently, so the patterns match either wording and capture + # only the MEASUREMENT: + # hook: "... has 605 lines, exceeding the 500-line ceiling ..." + # audit: "SKILL.md line count 605 — exceeds 500-line limit" + ('SPEC_LINES', re.compile(r'(?:has|line count) (\d+)(?: lines,)? (?:exceeding|—)')), + ('SPEC_WORDS', re.compile( + r'(?:has|word count) (\d+)(?: words \(proxy for tokens\),)? (?:exceeding|—)')), ) @@ -227,9 +287,15 @@ def verdict(output): Lines that match no rule are dropped rather than compared: the two scripts legitimately check different things outside ADR-0020 (name/directory - agreement, script executability, the 1024-char spec backstop, whole-file - line and word ceilings), and forcing those into the comparison would report - a difference that is not a disagreement. + agreement, script executability, the 1024-char spec backstop), and forcing + those into the comparison would report a difference that is not a + disagreement. + + The whole-file line and word ceilings are NOT in that list. They were + excluded once, on the untested assumption that awk and splitlines() agree; + they do not, and the divergence was invisible for exactly as long as the + exclusion stood. SPEC_LINES/SPEC_WORDS are compared like any other rule — + see the file header. Do not re-add an exclusion for them. """ found = set() for raw in output.splitlines(): @@ -278,9 +344,15 @@ def compare(label, skill_dir): problems.append('the hook reported an ADR-0020 ERROR but exited 0') if audit_err and audit_rc == 0: problems.append('skill-audit reported an ADR-0020 FAIL but exited 0') - if not hook_err and hook_rc != 0 and not _non_adr_hook_error(hook_out): - problems.append('the hook exited %d with no ADR-0020 ERROR and no spec-ceiling ERROR' - % hook_rc) + # No escape hatch here any more. There used to be one — a + # `_non_adr_hook_error()` helper that waved through a non-zero hook exit + # explained by MAX_LINES / MAX_WORDS, on the grounds that those two were + # outside the comparison. They are inside it now (see SPEC_LINES / + # SPEC_WORDS in RULES), so every ERROR the hook can raise is a token this + # comparison holds both scripts to. + if not hook_err and hook_rc != 0: + problems.append('the hook exited %d with no compared ERROR at all — it has an ' + 'ERROR source this comparison does not know about' % hook_rc) if problems: bad('%s: %s' % (label, '; '.join(problems))) @@ -289,16 +361,6 @@ def compare(label, skill_dir): return False -def _non_adr_hook_error(output): - """True if the hook failed on a spec ceiling rather than an ADR-0020 gate. - - MAX_LINES / MAX_WORDS are the hook's other ERROR sources and are outside - this comparison, so a non-zero exit explained by one of them is not a - disagreement. - """ - return bool(re.search(r'ERROR: .*(-line ceiling|-word ceiling \(~5,000 tokens)', output)) - - # --- The real corpus ------------------------------------------------------- corpus = sorted(glob.glob(os.path.join(repo_root, 'plugins', '*', '.apm', 'skills', '*'))) corpus = [d for d in corpus if os.path.isfile(os.path.join(d, 'SKILL.md'))] @@ -359,6 +421,31 @@ else: ok('every one of the %d compared axes was exercised by at least one fixture' % len(expected_tokens)) +# --- The Unicode-whitespace fixtures, named and asserted directly ----------- +# The two comparisons above would catch this divergence, but only as "fixture +# spec-lines-u2028 disagreed" — one line among 65. Spelled out here so the +# failure names the primitive, and so the ceiling is asserted to FIRE in both +# scripts rather than merely to be reported the same way by both. +print("") +print("--- both scripts break the spec ceilings on the same Unicode whitespace ---") +for name, token, expected in (('spec-lines-u2028', 'SPEC_LINES', '605'), + ('spec-words-u00a0', 'SPEC_WORDS', '2816')): + skill_dir = os.path.join(fixture_dir, name) + _, h_out = run(['bash', hook, os.path.join(skill_dir, 'SKILL.md')]) + _, a_out = run(['bash', validate, skill_dir]) + want = ('ERROR', token, expected) + missing = [who for who, v in (('the hook', verdict(h_out)), + ('skill-audit', verdict(a_out))) + if want not in v] + if missing: + bad('%s: %s did not report %s=%s. The two scripts must count with the ' + 'same primitive — Python splitlines()/split(), not awk NR/NF, which ' + 'does not break on this character' % (name, ' and '.join(missing), + token, expected)) + else: + ok('%s: both scripts measure %s=%s and raise the ceiling ERROR' + % (name, token, expected)) + print("") print("Results: %d passed, %d failed" % (passes, failures)) sys.exit(1 if failures else 0) diff --git a/tests/test-adr0020-frontmatter.sh b/tests/test-adr0020-frontmatter.sh index 8344e1f..07cf878 100755 --- a/tests/test-adr0020-frontmatter.sh +++ b/tests/test-adr0020-frontmatter.sh @@ -19,7 +19,22 @@ # NEXT key. The value then looked present (so "missing or empty" never # fired) and was empty once folded (so every ADR-0020 gate early-returned). # An agent file with one exited 0 with zero output through a BLOCKING -# pre-push gate. All five spellings of "no value" are pinned here. +# pre-push gate. All five spellings of "no value" are pinned here, plus the +# three shapes where the value is present but is not TEXT — a list, a +# mapping, a bool. Those used to be `str()`-coerced and then measured as a +# Python repr, so `description: true` was the four-character "True" and +# passed the 400-character gate. +# +# 3. THE INDENTED CLOSING MARKER. The mirror image of (1): content the pattern +# was too LOOSE to reject. `\r?\n[ \t]*---` matched an indented `---` inside +# a `>`-folded description, truncating the frontmatter mid-value — the +# description gate then measured a fragment and the body gate measured the +# discarded description text. +# +# Every needle names the specific branch or measurement the case is about. A +# needle loose enough to match two branches is how the yaml-none fixture spent +# its life asserting the wrong one: it emitted `---\n---\n`, which never matched +# the frontmatter pattern at all, and passed on the bare word "frontmatter". # # Both fixtures carry an over-ceiling description AND an over-ceiling body on # purpose: asserting a non-zero exit alone would be satisfied by the "cannot @@ -81,7 +96,48 @@ elif kind == 'yaml-list': elif kind == 'yaml-string': fm_lines = ['just a bare scalar, not a mapping'] elif kind == 'yaml-none': + # A comment-only block, NOT an empty one. `---\n---\n` does not match + # FRONTMATTER_RE at all (the pattern needs a `\n` between the markers), so + # it lands on the "no parseable frontmatter" branch and never reaches the + # `data is None` -> "not a YAML mapping" branch this fixture is named for. + # It passed anyway because the needle used to be the bare word + # "frontmatter", which both messages contain. A comment is real frontmatter + # text that yaml.safe_load() returns None for, which is the branch. + fm_lines = ['# nothing but a comment'] +elif kind == 'yaml-empty-block': + # The shape the fixture above USED to have, kept as its own case so the + # "no parseable frontmatter block" branch is covered on purpose rather than + # by accident. fm_lines = [] +elif kind == 'desc-folded-indented': + # A `>`-folded description whose CONTENT contains an indented `---` line. + # YAML block-scalar content must be indented deeper than its key, so this is + # a value, not a document marker — but the closing pattern used to be + # `\r?\n[ \t]*---`, which matched it, truncated the frontmatter mid-value + # and silently reclassified the rest of the description as body. Both halves + # of that are vacuous greens: the description gate measured a fragment, and + # the body gate measured description text. + # + # The value is padded to exactly desc_chars AFTER folding, and the boundary + # clause naming a target sits in the part the truncation used to discard. + head = 'Use when doing the thing. ' + tail = ' Do not use for improvements — use no-such-folded-target instead.' + span = int(desc_chars) - len(head) - len(tail) - len(' --- ') + if span < 2: + raise SystemExit('desc_chars too small for the folded fixture') + fm_lines = [ + 'name: ' + name, + 'description: >', + ' ' + head + 'x' * (span // 2), + ' ---', + ' ' + 'x' * (span - span // 2) + tail, + ] +elif kind == 'desc-list': + fm_lines = ['name: ' + name, 'description:', ' - one', ' - two'] +elif kind == 'desc-mapping': + fm_lines = ['name: ' + name, 'description:', ' text: a description'] +elif kind == 'desc-bool': + fm_lines = ['name: ' + name, 'description: true'] elif kind == 'yaml-malformed': fm_lines = ['name: ' + name, 'description: "unterminated', 'tabs:\t- a'] elif kind == 'desc-no-value': @@ -222,24 +278,58 @@ probe_all "trailing whitespace after either --- marker does not hide the finding probe_all "CRLF line endings do not hide the findings" \ crlf "description is $DESC_CHARS char" "@skills:body is $BODY_WORDS words" +# --------------------------------------------------------------------------- +# 1a-bis. An indented `---` inside a block scalar is CONTENT, not a marker +# --------------------------------------------------------------------------- +# The mirror image of the four shapes above. Those were markers the pattern was +# too strict to accept; this is content the pattern was too loose to reject. The +# closing marker used to be `\r?\n[ \t]*---`, so an indented `---` inside a +# `>`-folded description ended the frontmatter early: the description gate then +# measured a truncated fragment (under every ceiling, so silent) and the body +# gate measured the discarded description text as body. Measured on the fixture +# below, the old code exited 0 with nothing but a spurious "no boundary clause" +# SUGGESTION — the clause is in the half it threw away. +# +# The needle is the full-value length, so a script that merely rejected the file +# would not satisfy it. +echo "" +echo "--- an indented --- inside a >-folded description is content, not the end of the frontmatter ---" +build_subjects desc-folded-indented +probe_all "a folded description containing an indented '---' is measured whole" \ + desc-folded-indented "description is $DESC_CHARS char" + # --------------------------------------------------------------------------- # 1b. Unparseable frontmatter is a hard ERROR, never a quiet skip # --------------------------------------------------------------------------- echo "" echo "--- genuinely unparseable frontmatter exits non-zero with a message, rather than passing quietly ---" -for kind in no-close yaml-list yaml-string yaml-none yaml-malformed; do +# Each needle names the BRANCH the fixture is supposed to reach, not the word +# "frontmatter" — which every one of these messages contains, and which is why +# the yaml-none fixture below passed for years while landing on the wrong branch +# entirely. +for kind in no-close yaml-list yaml-string yaml-none yaml-empty-block yaml-malformed; do build_subjects "$kind" done probe_all "frontmatter with no closing --- is reported, not skipped" \ - no-close "frontmatter" + no-close "parseable YAML frontmatter block" probe_all "frontmatter that parses to a LIST is reported, not skipped" \ - yaml-list "frontmatter" + yaml-list "frontmatter is not a YAML mapping" probe_all "frontmatter that parses to a STRING is reported, not skipped" \ - yaml-string "frontmatter" -probe_all "frontmatter that parses to None (empty block) is reported, not skipped" \ - yaml-none "frontmatter" + yaml-string "frontmatter is not a YAML mapping" +probe_all "frontmatter that parses to None (a comment-only block) is reported, not skipped" \ + yaml-none "frontmatter is not a YAML mapping" +probe_all "a completely empty '---/---' block is reported, not skipped" \ + yaml-empty-block "parseable YAML frontmatter block" +# Two needles, both naming the SYNTAX branch specifically. "frontmatter is not +# valid YAML" is now exclusive to it — the wrong-typed-description failures reach +# the same wrapper and no longer borrow that phrase (see 2c below) — and the +# scanner context proves the parser's own diagnostic survives the wrapper rather +# than being replaced by a generic one. Do not needle the tail of PyYAML's +# message: an earlier attempt used "could not find expected", which PyYAML 6.0.3 +# does not emit for this fixture at all, so the case failed on the assertion +# rather than on the behaviour. probe_all "malformed YAML in the frontmatter is reported, not skipped" \ - yaml-malformed "frontmatter" + yaml-malformed "frontmatter is not valid YAML" "while scanning a quoted scalar" # --------------------------------------------------------------------------- # 2. A valueless description is a hard FAIL in all three scripts @@ -265,6 +355,70 @@ probe_all "'description: \"\"' FAILs" \ probe_all "'description: >' with nothing folded under it FAILs" \ desc-empty-fold "description field is missing or empty" +# --------------------------------------------------------------------------- +# 2b. A description that is not a STRING is a parse failure, not a measurement +# --------------------------------------------------------------------------- +# The other half of the same family, and the reason it belongs beside the five +# above: all eight shapes are "the description is not a description", and seven +# of them used to be handled while this one was silently coerced. A non-string +# value went through `str()` and was then measured as a Python repr — +# `description: true` became the four-character "True" and sailed through the +# 400-character gate, a list became "['one', 'two']", a mapping its dict repr. +# None of those is text a host can preload, so measuring one is a green verdict +# on a file that was never measured. +echo "" +echo "--- a description that is a list, a mapping or a bool hard-FAILs in all three scripts ---" +for kind in desc-list desc-mapping desc-bool; do + build_subjects "$kind" +done +probe_all "a LIST description FAILs rather than being measured as its repr" \ + desc-list "description is a list, not a string" +probe_all "a MAPPING description FAILs rather than being measured as its repr" \ + desc-mapping "description is a dict, not a string" +probe_all "a BOOL description FAILs rather than being measured as the 4-char 'True'" \ + desc-bool "description is a bool, not a string" + +# --------------------------------------------------------------------------- +# 2c. The FAILURE CLASS reported has to be the one that happened +# --------------------------------------------------------------------------- +# The three fixtures above reach the same wrapper as a genuine YAML syntax +# error, and that wrapper used to prefix a hard-coded "frontmatter is not valid +# YAML (...)" onto all of them. For a non-string description that is false: the +# block parses, only the field's TYPE is wrong. On a blocking gate with no +# baseline it sent the author hunting for a syntax error that is not there. The +# assertion runs in both directions, because fixing it by dropping the phrase +# everywhere would trade one wrong diagnosis for another. +echo "" +echo "--- 'not valid YAML' is said for a syntax error and NOT for a wrong-typed description ---" +YAML_CLASS_PROBLEMS="" +for spec in "yaml-malformed|yes" "desc-list|no" "desc-mapping|no" "desc-bool|no"; do + kind="${spec%%|*}" + want="${spec#*|}" + build_subjects "$kind" + for target in \ + "hook|$HOOK|$TMPDIR_T/$kind/skill/my-skill/SKILL.md" \ + "skill-audit|$SKILL_VALIDATE|$TMPDIR_T/$kind/skill/my-skill" \ + "agent-audit|$AGENT_VALIDATE|$TMPDIR_T/$kind/agent/.apm/agents/my-agent.agent.md" + do + who="${target%%|*}"; rest="${target#*|}" + script="${rest%%|*}"; arg="${rest#*|}" + set +e + out="$(bash "$script" "$arg" 2>&1)" + set -e + if [[ "$want" == yes && "$out" != *"frontmatter is not valid YAML"* ]]; then + YAML_CLASS_PROBLEMS="$YAML_CLASS_PROBLEMS [$who did not call $kind a YAML syntax error: $out]" + fi + if [[ "$want" == no && "$out" == *"not valid YAML"* ]]; then + YAML_CLASS_PROBLEMS="$YAML_CLASS_PROBLEMS [$who called $kind invalid YAML, but the frontmatter parsed: $out]" + fi + done +done +if [[ -z "$YAML_CLASS_PROBLEMS" ]]; then + pass "a type error is reported as a type error and a syntax error as a syntax error" +else + fail "wrong failure class reported —$YAML_CLASS_PROBLEMS" +fi + # The specific regression, spelled out: the valueless-description agent file must # not merely fail — it must not be SILENT. Zero output on a blocking gate is what # made this un-diagnosable, so the output is asserted non-empty independently. diff --git a/tests/test-adr0020-targets.sh b/tests/test-adr0020-targets.sh index 364e6fa..1bd355e 100755 --- a/tests/test-adr0020-targets.sh +++ b/tests/test-adr0020-targets.sh @@ -13,6 +13,14 @@ # identical with and without a deployed tree — on a synthetic fixture AND on # the real 39-skill corpus. # +# Three further ways the universe can be built out of the wrong directory, +# each of which shipped: a `.git` at the CONSUMER root (the fallback is +# truthy in any git repo, which made the deployed-tree branch dead code), a +# `.git` INSIDE a plugin (the walk-up is two passes precisely so this cannot +# capture the root), and glob metacharacters in the checkout path (which +# turned the directory name into a character class matching nothing, and the +# resolver into a no-op that still reported green). +# # 2. THE BARE-TARGET GRAMMAR RULE. A hyphenated token used as a compound # MODIFIER ("pre-commit hooks", "pull-request template") is prose, not a # route; a terminal one is a real target. Getting this wrong in either @@ -138,6 +146,175 @@ else fail "the consumer path did not resolve through the deployed tree (exit $CONSUMER_RC): ${CONSUMER_OUT:-}" fi +# --------------------------------------------------------------------------- +# 1a-bis. The consumer case with the one thing every real consumer has: .git +# --------------------------------------------------------------------------- +# The fixture immediately above has no .git, and that is precisely why it could +# never catch this. _authoring_root() falls back to the nearest .git ancestor, so +# it returns truthy in ANY git repo — a consumer checkout included. The branch +# that reads the deployed trees was guarded by `else`, so in every consumer +# checkout the fallback won, _collect_authoring_root() contributed nothing +# (there is no plugins/ directory to collect), and _deployed_roots() was dead +# code in exactly the case it exists for. +# +# The pair below is the whole test: the SAME tree, once with .git and once +# without. Old behaviour was rc=1 with .git and rc=0 without; a test covering +# only the no-.git shape reports green on both. +# +# `deployed-only-agent` lives ONLY in .agents/agents/, so it can be reached +# through no route but _deployed_roots(). `sibling-skill` sits in .claude/skills/ +# beside the subject, which the sibling-collection block above reaches on its own +# — it is the corroborator that makes the dangling target BLOCKING rather than a +# SUGGESTION, so the old failure shows up in the exit code and not only in prose. +echo "" +echo "--- a consumer checkout resolves through its deployed trees even though it is a git repo ---" +build_consumer() { + local root="$1" + mkdir -p "$root/.agents/agents" + write_skill "$root/.claude/skills/sibling-skill" sibling-skill \ + "Use when doing the other thing. Do not use for anything else." + write_skill "$root/.claude/skills/my-skill" my-skill \ + "Use when doing the thing. Do not use for the other thing — use sibling-skill or deployed-only-agent instead." + : > "$root/.agents/agents/deployed-only-agent.agent.md" +} +build_consumer "$TMPDIR_T/consumer-git" +mkdir -p "$TMPDIR_T/consumer-git/.git" +build_consumer "$TMPDIR_T/consumer-nogit" + +# consumer_case