refactor(bin): retrofit research to the ADR-0020 context contract
Description 583 -> 231 chars and body 854 -> 519 words. Deletes the
neuledge-context boundary clause outright: commit 6146120 deleted that
skill and no skill has owned MCP-server installation since. That was the
last dangling routing target in the corpus.
Removes META.md, which file-structure.md:20 forbids at a skill root. Its
when: field duplicated the description and its references: entry pointed
at .agents/skills/context7-mcp/SKILL.md, which does not exist.
Restores two rules a clean-context audit found had lost their force: the
starting-URLs branch in step 3, which the retrofit had reduced to a
condition with no behaviour, and the references/file-format.md pointer at
step 6. The second matters downstream -- validate-provenance.sh parses
sources.md with anchored regexes and check 8 short-circuits silently when
the Status field is absent, so a sources.md written from step 6 alone
broke the provenance chain with no error anywhere.
Rewrites steps 4-5 as serial WebFetch reads. They mandated spawning
subagents that allowed-tools never granted; no tool was added because the
name differs across the three compile targets. Tracked as #116.
Updates the two test pins and the eval case that asserted the dead route.
Refs #99
This commit is contained in:
@@ -362,35 +362,47 @@ cp -R "$REPO_ROOT/plugins" "$FRESH_ROOT/plugins"
|
||||
[[ -f "$REPO_ROOT/apm.yml" ]] && cp "$REPO_ROOT/apm.yml" "$FRESH_ROOT/apm.yml"
|
||||
FRESH_DANGLING="$(dangling_set "$FRESH_ROOT/plugins")"
|
||||
|
||||
DEPLOYED_ROOT="$TMPDIR_T/deployed-clone"
|
||||
mkdir -p "$DEPLOYED_ROOT/.claude/skills" "$DEPLOYED_ROOT/.claude/agents"
|
||||
cp -R "$REPO_ROOT/plugins" "$DEPLOYED_ROOT/plugins"
|
||||
[[ -f "$REPO_ROOT/apm.yml" ]] && cp "$REPO_ROOT/apm.yml" "$DEPLOYED_ROOT/apm.yml"
|
||||
# Deploy exactly the names that currently dangle. That is the strongest possible
|
||||
# bait: if the deployed tree were consulted, every one of them would resolve and
|
||||
# the dangling set would collapse to empty.
|
||||
DEPLOY_COUNT=0
|
||||
while IFS= read -r name; do
|
||||
[[ -n "$name" ]] || continue
|
||||
mkdir -p "$DEPLOYED_ROOT/.claude/skills/$name"
|
||||
DEPLOY_COUNT=$((DEPLOY_COUNT + 1))
|
||||
done <<< "$FRESH_DANGLING"
|
||||
DEPLOYED_DANGLING="$(dangling_set "$DEPLOYED_ROOT/plugins")"
|
||||
# The bait used to be DERIVED from the corpus: deploy exactly the names that
|
||||
# currently dangle. That was the strongest bait available while the corpus had
|
||||
# dangling names — and it silently became vacuous the moment issue #99 fixed the
|
||||
# last one, because a corpus reporting nothing gives nothing to deploy. A test of
|
||||
# "deployed trees do not leak" must not depend on the corpus staying broken.
|
||||
#
|
||||
# So the bait is now EXPLICIT. Both contrast copies get one synthetic skill whose
|
||||
# boundary clause routes to a name guaranteed absent from the monorepo, and only
|
||||
# the deployed copy gets that name planted in .claude/skills/. If deployed trees
|
||||
# leaked into the resolver's universe, the deployed copy would resolve it and
|
||||
# report an empty set while the fresh copy reported one. The A/B now distinguishes
|
||||
# something on every run, forever, whatever the corpus does.
|
||||
BAIT_NAME="no-such-deployed-only-skill"
|
||||
BAIT_FRESH="$TMPDIR_T/bait-fresh"
|
||||
BAIT_DEPLOYED="$TMPDIR_T/bait-deployed"
|
||||
for bait_root in "$BAIT_FRESH" "$BAIT_DEPLOYED"; do
|
||||
mkdir -p "$bait_root"
|
||||
cp -R "$REPO_ROOT/plugins" "$bait_root/plugins"
|
||||
[[ -f "$REPO_ROOT/apm.yml" ]] && cp "$REPO_ROOT/apm.yml" "$bait_root/apm.yml"
|
||||
write_skill "$bait_root/plugins/bin/.apm/skills/deployed-tree-probe" deployed-tree-probe \
|
||||
"Use when doing the probe thing. Do not use for the other thing — use $BAIT_NAME instead."
|
||||
done
|
||||
# Only the deployed copy gets the name planted where `apm install` would put it.
|
||||
mkdir -p "$BAIT_DEPLOYED/.claude/skills/$BAIT_NAME" "$BAIT_DEPLOYED/.claude/agents"
|
||||
BAIT_FRESH_DANGLING="$(dangling_set "$BAIT_FRESH/plugins")"
|
||||
BAIT_DEPLOYED_DANGLING="$(dangling_set "$BAIT_DEPLOYED/plugins")"
|
||||
|
||||
if [[ "$DEPLOY_COUNT" -gt 0 ]]; then
|
||||
pass "precondition: $DEPLOY_COUNT dangling name(s) deployed into the contrast tree's .claude/skills/, so the A/B has something to distinguish"
|
||||
if [[ "$BAIT_FRESH_DANGLING" == *"$BAIT_NAME"* ]]; then
|
||||
pass "precondition: the bait target dangles in the un-deployed copy, so the A/B has something to distinguish"
|
||||
else
|
||||
fail "no dangling names to deploy — the corpus reports none, so this A/B distinguishes nothing. Deploy a known-absent name explicitly instead of deriving one."
|
||||
fail "the bait target '$BAIT_NAME' does not dangle even without a deployed tree — the fixture is broken, so the contrast below proves nothing. Got: [$(echo "$BAIT_FRESH_DANGLING" | tr '\n' ' ')]"
|
||||
fi
|
||||
if [[ ! -d "$FRESH_ROOT/.claude" && ! -d "$FRESH_ROOT/.agents" ]]; then
|
||||
if [[ ! -d "$BAIT_FRESH/.claude" && ! -d "$BAIT_FRESH/.agents" ]]; then
|
||||
pass "precondition: the fresh-clone copy has no deployed tree of its own"
|
||||
else
|
||||
fail "the fresh-clone copy picked up a deployed tree — it is not a fresh-clone fixture"
|
||||
fi
|
||||
if [[ "$FRESH_DANGLING" == "$DEPLOYED_DANGLING" ]]; then
|
||||
pass "deploying every dangling name into .claude/skills/ changes nothing: $(echo "$FRESH_DANGLING" | tr '\n' ' ')"
|
||||
if [[ "$BAIT_FRESH_DANGLING" == "$BAIT_DEPLOYED_DANGLING" ]]; then
|
||||
pass "planting the dangling name in .claude/skills/ changes nothing: $(echo "$BAIT_DEPLOYED_DANGLING" | tr '\n' ' ')"
|
||||
else
|
||||
fail "the corpus verdict depends on whether apm install has been run — fresh clone: [$(echo "$FRESH_DANGLING" | tr '\n' ' ')] with a deployed tree: [$(echo "$DEPLOYED_DANGLING" | tr '\n' ' ')]"
|
||||
fail "the verdict depends on whether apm install has been run — fresh clone: [$(echo "$BAIT_FRESH_DANGLING" | tr '\n' ' ')] with a deployed tree: [$(echo "$BAIT_DEPLOYED_DANGLING" | tr '\n' ' ')]. A deployed .claude/skills/ tree is leaking into the resolver's universe."
|
||||
fi
|
||||
# Third data point: whatever state THIS machine happens to be in, the live tree
|
||||
# must agree with a bare copy of the same plugins/. No precondition on that state
|
||||
@@ -417,37 +429,33 @@ fi
|
||||
# terminal and therefore danglable. The issue #99 retrofit cut that composition
|
||||
# sentence and the dangling target went with it, so the set is down to one.
|
||||
#
|
||||
# WHEN `research` IS RETROFITTED: drop neuledge-context and leave the set empty.
|
||||
# Do not delete the assertion — an empty expected set is fine and still pins
|
||||
# that no NEW dangling target appeared.
|
||||
# `neuledge-context` was the last one. The issue #99 wave-3 retrofit deleted that
|
||||
# boundary clause outright — commit `6146120` had already deleted the skill it
|
||||
# named, and nothing has owned MCP-server installation since — so the corpus
|
||||
# dangling set is now EMPTY.
|
||||
#
|
||||
# The assertion stays, and it is not vacuous now that it expects nothing: it is
|
||||
# the only thing standing between a newly-authored boundary clause naming a
|
||||
# non-existent target and a green suite. An empty expected set pins "no NEW
|
||||
# dangling target appeared", which is the property that actually matters from
|
||||
# here on.
|
||||
#
|
||||
# The per-target probe loop that used to sit below is GONE, not emptied. Its job
|
||||
# was to prove the check detects each live dangling target individually, and with
|
||||
# no live targets left there is nothing to point it at. A loop over an empty list
|
||||
# is an assertion-free result counted in the totals — exactly the vacuous-pass
|
||||
# shape the comment above rejects. Detection is still covered, and covered
|
||||
# better, by the synthetic fixtures in section 2 below, which build a real plugin
|
||||
# tree and assert the resolver fires. Do not reinstate the loop unless a real
|
||||
# dangling target reappears in the corpus.
|
||||
echo ""
|
||||
echo "--- the live dangling targets in the corpus are exactly the ADR-0020 records still open ---"
|
||||
EXPECTED_DANGLING="$(printf '%s\n' neuledge-context)"
|
||||
echo "--- no skill in the corpus routes to a target that does not resolve ---"
|
||||
EXPECTED_DANGLING=""
|
||||
if [[ "$LIVE_DANGLING" == "$EXPECTED_DANGLING" ]]; then
|
||||
pass "the corpus dangling set is exactly {neuledge-context}"
|
||||
pass "the corpus dangling set is empty"
|
||||
else
|
||||
fail "the corpus dangling set changed — expected [$(echo "$EXPECTED_DANGLING" | tr '\n' ' ')], got [$(echo "$LIVE_DANGLING" | tr '\n' ' ')]. If a retrofit fixed one, update EXPECTED_DANGLING; if a false-positive fix silently deleted one, that is the regression this asserts."
|
||||
fail "a dangling routing target appeared in the corpus — expected none, got [$(echo "$LIVE_DANGLING" | tr '\n' ' ')]. A boundary clause names a skill or agent that does not resolve; fix the clause or the target. This assertion is the corpus-wide backstop, so do not relax it to make a new skill pass."
|
||||
fi
|
||||
# shellcheck disable=SC2043 # one probe left by design -- the list shrinks as
|
||||
# each fixture is retrofitted and reaches zero when `research` lands. Keeping the
|
||||
# loop means removing the last entry is a one-line edit, not a restructure.
|
||||
for probe in \
|
||||
"plugins/bin/.apm/skills/research/SKILL.md:neuledge-context"; do
|
||||
probe_file="$REPO_ROOT/${probe%%:*}"
|
||||
probe_name="${probe##*:}"
|
||||
if [[ ! -f "$probe_file" ]]; then
|
||||
fail "the true-positive fixture ${probe%%:*} no longer exists — this pin has become vacuous"
|
||||
continue
|
||||
fi
|
||||
set +e
|
||||
probe_out="$(bash "$HOOK" "$probe_file" 2>&1)"
|
||||
set -e
|
||||
if [[ "$probe_out" == *"routes to '$probe_name'"* ]]; then
|
||||
pass "detects the dangling '$probe_name' target in ${probe%%:*}"
|
||||
else
|
||||
fail "did not detect the dangling '$probe_name' target in ${probe%%:*} — a false-positive fix has taken a true positive with it: $probe_out"
|
||||
fi
|
||||
done
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. The bare-target grammar rule
|
||||
|
||||
@@ -534,10 +534,12 @@ expect_gate "a fixture with no authoring root reports DID NOT RUN and exits 0" \
|
||||
"Use when doing the thing. Do not use for improvements — use some-other-skill instead." 10)" \
|
||||
"Unchecked target(s): some-other-skill"
|
||||
|
||||
echo ""
|
||||
echo "--- the live dangling routing targets are caught (issue #100) ---"
|
||||
# NOTE: this section prints no header and runs no assertions any more — see why
|
||||
# below. The commentary is kept because it records why probes are removed rather
|
||||
# than skipped, which is the rule the next person to touch this file needs.
|
||||
#
|
||||
# ADR-0020 records the broken routing targets and splits fixing them into its own
|
||||
# issue. This asserts the gate actually sees them rather than the check being
|
||||
# issue. This asserted the gate actually sees them rather than the check being
|
||||
# vacuous in the corpus it was written against.
|
||||
#
|
||||
# There used to be a third probe here, for `skill-improve` in skill-audit's
|
||||
@@ -556,29 +558,26 @@ echo "--- the live dangling routing targets are caught (issue #100) ---"
|
||||
# The gitea-labels probe was dropped when the issue #99 retrofit cut the
|
||||
# composition sentence whose YAML fold produced that target. Per the rule above
|
||||
# it is removed, not skipped.
|
||||
# shellcheck disable=SC2043 # one probe left by design -- the list shrinks as
|
||||
# each fixture is retrofitted and reaches zero when `research` lands. Keeping the
|
||||
# loop means removing the last entry is a one-line edit, not a restructure.
|
||||
for probe in \
|
||||
"plugins/bin/.apm/skills/research/SKILL.md:neuledge-context"; do
|
||||
probe_file="$REPO_ROOT/${probe%%:*}"
|
||||
probe_name="${probe##*:}"
|
||||
if [[ ! -f "$probe_file" ]]; then
|
||||
fail "the probe fixture ${probe%%:*} no longer exists — this pin has become vacuous; update it and EXPECTED_DANGLING in tests/test-adr0020-targets.sh together"
|
||||
continue
|
||||
fi
|
||||
# Captured, not piped: the script exits non-zero on these files and
|
||||
# `set -o pipefail` would make the whole pipeline non-zero regardless of what
|
||||
# grep found.
|
||||
set +e
|
||||
probe_out="$("$SCRIPT" "$probe_file" 2>&1)"
|
||||
set -e
|
||||
if [[ "$probe_out" == *"routes to '$probe_name'"* ]]; then
|
||||
pass "detects the dangling '$probe_name' target in ${probe%%:*}"
|
||||
else
|
||||
fail "did not detect the dangling '$probe_name' target in ${probe%%:*}. If issue #100 retrofitted it, drop this probe and update EXPECTED_DANGLING in tests/test-adr0020-targets.sh; if a false-positive fix took a true positive with it, that is the regression this asserts."
|
||||
fi
|
||||
done
|
||||
#
|
||||
# The `neuledge-context` probe — the last one — went the same way in wave 3 of
|
||||
# that retrofit, which deleted the boundary clause naming it. **The corpus now
|
||||
# has zero dangling targets**, so this loop is removed entirely rather than left
|
||||
# to iterate over an empty list.
|
||||
#
|
||||
# That is deliberate and follows the rule stated above. A loop over no probes
|
||||
# produces no assertion while still returning success, which is the vacuous-pass
|
||||
# shape this comment block exists to reject — it would make the suite look one
|
||||
# test stronger than it is, exactly the complaint levelled at the old
|
||||
# `skill-improve` SKIP branch.
|
||||
#
|
||||
# Nothing is lost. This file only ever checked that each member of the live
|
||||
# dangling set is individually detected; tests/test-adr0020-targets.sh remains
|
||||
# the authority on the set itself, and now pins it as EMPTY, which is what
|
||||
# catches a newly-authored clause naming a target that does not resolve. That
|
||||
# file also carries synthetic fixtures built inside a real plugin tree, which
|
||||
# exercise the detection path without depending on the corpus staying broken.
|
||||
#
|
||||
# If a real dangling target ever reappears, add its probe back here.
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
|
||||
Reference in New Issue
Block a user