diff --git a/plugins/instruction-placement/.claude-plugin/plugin.json b/plugins/instruction-placement/.claude-plugin/plugin.json index c14c189907..aeac0383cd 100644 --- a/plugins/instruction-placement/.claude-plugin/plugin.json +++ b/plugins/instruction-placement/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "instruction-placement", - "version": "0.11.10", + "version": "0.11.11", "description": "Routes agent-instruction content to the surface that loads it at the right moment. The audit skill sweeps a repository's instruction layer and its ordinary markdown for content whose scope is narrower than the surface carrying it \u2014 conventions keyed to one file type or one subtree sitting in an always-loaded CLAUDE.md or AGENTS.md \u2014 and for normative conventions stranded in documentation Claude never loads at all, then classifies each against a routing rubric and proposes a destination whose `paths:` glob is machine-validated before it is ever offered. Safety-class content (irreversible actions, secrets, data integrity, external publication, compliance, agent authority) is hard-denied from demotion and reported as held back rather than proposed, because demotion trades guaranteed presence for conditional presence and deferred surfaces are invisible inside subagents and absent after compaction until re-triggered. Every accepted move regenerates an always-loaded index of deferred surfaces, which is what keeps a demoted rule reachable from a subagent that never receives its injection. The audit is read-only and emits a diffable findings artifact; realignment is a separate skill gated per item with no blanket-approve path; a deterministic check skill gates that every rule glob still resolves and the index is current; and a setup skill verifies the one thing no other gate can see \u2014 that the index target is a file Claude Code will actually read, since it reads CLAUDE.md and not AGENTS.md.", "author": { "name": "Melodic Software", diff --git a/plugins/instruction-placement/CHANGELOG.md b/plugins/instruction-placement/CHANGELOG.md index 9cc95508c2..eaa8285962 100644 --- a/plugins/instruction-placement/CHANGELOG.md +++ b/plugins/instruction-placement/CHANGELOG.md @@ -3,6 +3,46 @@ All notable changes to the `instruction-placement` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.11.11] + +### Fixed + +- **`adherence-experiment.sh` scored its underscore criterion against the whole file.** The seeded + `Billing.cs` already declares `private readonly decimal _unitPrice`, so the check returned 1 + before the model had written anything and no run could ever fail it. The check is now scoped to + the body of the class the task asks for, by a scanner that biases toward under-crediting: a + body-less primary-constructor declaration, a brace inside a string or comment, and any other + shape that leaves the body undelimited all score 0 rather than running on into a later class and + crediting a field the task never asked for. For an instrument whose numbers get published, a + false 1 is the unrecoverable error. `adherence-results.md` carries a correction noting that the + recorded run's `underscore` and `both` columns were constants, not measurements; the conclusion + is unchanged, since it rests on the `sealed` criterion, which was scored correctly. +- **Both class matches now require a non-identifier boundary after the name.** `InvoiceTotal` is a + prefix of `InvoiceTotals` and `InvoiceTotalizer`, so a trial that produced a differently named + class scored full compliance on both criteria. The boundary still admits the three shapes that + occur here: the bare name, `InvoiceTotal {`, and `InvoiceTotal(decimal seed);`. +- **The underscore criterion credits a FIELD, not any member carrying an underscore name.** The + convention is about field naming, but the match accepted an auto-property + (`private decimal _total { get; set; }`), an expression-bodied property (`private decimal _Total + => x;`) and a method (`private decimal _GetTotal() => x;`). The name must now be followed by a + `;` or by an initializer `=` that is not the `=>` of an expression-bodied member. A + multi-declarator line credits on its first name only, per the same under-crediting bias. +- **The usage banner documents `--filler`.** The flag has always been parsed, and + `adherence-results.md` gives `--filler 120` as the re-run command, but neither the banner nor the + header comment listed it. + +### Added + +- **`adherence-experiment.test.sh` covers the experiment harness.** 52 cases driving the harness + through a stub CLI, so every trial's output is controlled and nothing is skipped: argument + validation and the exit-3 unmeasurable path, arm construction (identical filler and identical + seed file in both arms, the convention delivered inline in one and as a `**/*.cs` path-scoped + rule in the other), scoring asserted in both directions, against the three C# shapes that decide + whether a field from another class is credited and the three near misses that carry the right + characters in the wrong construct, per-trial reset, the ERROR row for a failed trial and its + exclusion from the arm's `n`, and fixture cleanup. The file previously mapped to no suite under + `scripts/affected-tests.sh`. + ## [0.11.10] ### Changed diff --git a/plugins/instruction-placement/evals/adherence-experiment.sh b/plugins/instruction-placement/evals/adherence-experiment.sh index 10c65f0799..b20211eeb5 100755 --- a/plugins/instruction-placement/evals/adherence-experiment.sh +++ b/plugins/instruction-placement/evals/adherence-experiment.sh @@ -34,7 +34,7 @@ # * Order is interleaved so drift in service conditions hits both arms alike. # # Usage: -# adherence-experiment.sh [--trials N] [--claude ] [--keep] +# adherence-experiment.sh [--trials N] [--filler N] [--claude ] [--keep] # # Exit: 0 the experiment ran; 2 usage error; 3 could not measure. @@ -53,9 +53,12 @@ usage() { adherence-experiment.sh — measure whether path-scoping improves adherence. Usage: - adherence-experiment.sh [--trials N] [--claude ] [--keep] + adherence-experiment.sh [--trials N] [--filler N] [--claude ] [--keep] --trials N runs per arm (default 6; 2 model calls per trial pair) + --filler N filler sections per half of the control file (default 24, + giving a ~250-line AGENTS.md); raise it to test for an + adherence effect that appears only at much greater bloat --claude PATH Claude Code CLI to drive --keep leave the fixture and transcripts on disk for inspection @@ -216,11 +219,90 @@ TASK='Add a public class named InvoiceTotal to src/Billing.cs. It needs a privat # Compliance, defined before any run. # sealed: the produced class is declared sealed # underscore: the new private field uses a leading-underscore name +# +# The underscore check is SCOPED to the InvoiceTotal body. The seeded file +# already declares `private readonly decimal _unitPrice`, so a whole-file search +# scores 1 before the model has written anything: the criterion becomes a +# constant, and a run reports a number that is not a measurement. +# +# The scanner below biases toward UNDER-crediting. Every way of failing to +# delimit the body ends at the declaration line alone, scoring 0, rather than +# running on into a later class and crediting a field that is not the one the +# task asked for. For an instrument whose numbers get published, a false 1 is +# the unrecoverable error and a false 0 is a visible one. +# +# Three shapes a model can plausibly produce, and what happens to each: +# `class InvoiceTotal(decimal x);` a body-less primary constructor. No brace +# body opens before the next declaration, so +# the region stops at the declaration line. +# `"unbalanced { brace"` a brace inside a string, char literal or +# line comment. Stripped before counting, so +# the depth counter stays in sync. +# anything else that desyncs a block comment holding a lone brace, say. +# Depth never returns to zero, the body is +# treated as undelimited, and the region +# stops at the declaration line. +# +# Both class matches are ANCHORED on a non-identifier boundary. `InvoiceTotal` +# is a prefix of `InvoiceTotals` and of `InvoiceTotalizer`, so an unanchored +# match scores full compliance for a trial that produced the wrong class. The +# boundary is `[^A-Za-z0-9_]` or end of line, which still admits the three +# shapes that occur here: `InvoiceTotal` alone, `InvoiceTotal {`, and +# `InvoiceTotal(decimal seed);`. score_trial() { - local file="$1" sealed=0 underscore=0 - grep -qE 'sealed[[:space:]]+class[[:space:]]+InvoiceTotal' "$file" 2>/dev/null && sealed=1 - # A private field declaration in the new class whose name starts with `_`. - grep -qE 'private[^;]*[[:space:]]_[A-Za-z][A-Za-z0-9]*' "$file" 2>/dev/null && underscore=1 + local file="$1" sealed=0 underscore=0 body + grep -qE 'sealed[[:space:]]+class[[:space:]]+InvoiceTotal([^A-Za-z0-9_]|$)' \ + "$file" 2>/dev/null && sealed=1 + # The new class, from its declaration to its matching closing brace. + body="$(awk ' + # Brace characters that are data, not structure. + function scrub(s, q) { + q = sprintf("%c", 39) + gsub(/"[^"]*"/, "", s) + gsub(q "[^" q "]*" q, "", s) + sub(/\/\/.*/, "", s) + return s + } + # A line that declares some class. Rejects comment lines, which cannot. + function declares_class(s) { + return (s ~ /^[[:space:]]*[A-Za-z]/ && + s ~ /(^|[[:space:]])class[[:space:]]+[A-Za-z_]/) + } + !seen && /class[[:space:]]+InvoiceTotal([^[:alnum:]_]|$)/ { seen = 1; decl = NR } + seen && !finished { + # A second declaration before any brace opened means InvoiceTotal has no + # brace body of its own, and the lines that follow belong to that class. + if (!entered && NR > decl && declares_class($0)) { finished = 1; next } + buf[++n] = $0 + code = scrub($0) + opens = gsub(/\{/, "{", code) + closes = gsub(/\}/, "}", code) + depth += opens - closes + if (opens > 0) entered = 1 + if (entered && depth <= 0) { finished = 1; closed = 1 } + } + END { + if (!seen) exit + if (!closed) n = 1 + for (i = 1; i <= n; i++) print buf[i] + } + ' "$file" 2>/dev/null)" + # A private FIELD declaration in the new class whose name starts with `_`. + # + # "Field" is the load-bearing word. The convention is about field naming, so + # a member that merely carries an underscore name does not satisfy it: an + # auto-property (`private decimal _total { get; set; }`), an expression-bodied + # property (`private decimal _Total => x;`) and a method + # (`private decimal _GetTotal() => x;`) all have to score 0. The trailing + # group is what separates them: a field declaration ends at `;` or continues + # into an initializer `=`, and `=[^>]` rejects the `=>` of an + # expression-bodied member. The leading `[^;{(=]*` cannot cross a `{`, a `(` + # or an `=`, so no later token on the line can be reached to stand in for the + # field. A multi-declarator line (`private decimal _a, _b;`) credits on its + # first name only, which is the documented under-crediting bias. + printf '%s\n' "$body" | + grep -qE 'private[^;{(=]*[[:space:]]_[A-Za-z][A-Za-z0-9]*[[:space:]]*(;|=[^>])' && + underscore=1 printf '%d\t%d' "$sealed" "$underscore" } diff --git a/plugins/instruction-placement/evals/adherence-experiment.test.sh b/plugins/instruction-placement/evals/adherence-experiment.test.sh new file mode 100755 index 0000000000..4ed78202ea --- /dev/null +++ b/plugins/instruction-placement/evals/adherence-experiment.test.sh @@ -0,0 +1,452 @@ +#!/usr/bin/env bash +# Regression tests for adherence-experiment.sh. +# +# WHAT IS ACTUALLY UNDER TEST. This harness produces a NUMBER that a published +# document (adherence-results.md) reasons from, so the defect that matters is +# not a crash, it is a number that looks like a measurement and is not one. The +# suite therefore drives the harness through a STUB CLI that produces a known +# output per trial and asserts the reported cell, in both directions: a +# compliant trial must score 1, and a non-compliant trial must score 0. A +# scoring rule that can only ever say 1 passes a one-directional suite and +# still reports a constant as a result. +# +# The stub is what makes that possible. The real CLI is a live model and cannot +# be asked for a controlled non-compliant answer, so every case here is +# deterministic and nothing is skipped. +# +# fixture-isolation-scope: the harness under test builds git fixtures, so this +# suite clears the inherited git environment itself rather than sourcing a +# harness, keeping the plugin self-contained outside this marketplace. +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_COMMON_DIR GIT_PREFIX GIT_OBJECT_DIRECTORY GIT_CONFIG + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$SCRIPT_DIR/adherence-experiment.sh" + +FAILED=0 +CASE_NUM=0 + +pass() { + CASE_NUM=$((CASE_NUM + 1)) + printf 'PASS: %s\n' "$1" +} +fail() { + CASE_NUM=$((CASE_NUM + 1)) + FAILED=$((FAILED + 1)) + printf 'FAIL: %s\n expected: %s\n actual: %s\n' "$1" "$2" "$3" >&2 +} +assert_eq() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "$2" "$3"; fi +} +assert_contains() { + if [[ "$2" == *"$3"* ]]; then pass "$1"; else fail "$1" "contains: $3" "$2"; fi +} +assert_lacks() { + if [[ "$2" != *"$3"* ]]; then pass "$1"; else fail "$1" "does NOT contain: $3" "$2"; fi +} + +SANDBOX="$(mktemp -d)" +trap 'rm -rf "$SANDBOX"' EXIT + +# --------------------------------------------------------------------------- +# The stub CLI. +# +# Stands in for the Claude Code CLI. Each invocation consumes the next line of +# a plan file, so the suite decides exactly what every trial "produces" and in +# what order. Trial order is control then treatment per round, which is what +# lets a plan line be mapped back to one reported cell. +# +# Directives: +# compliant sealed class, leading-underscore field +# bare neither +# sealed-only sealed class, plain field name +# underscore-only unsealed class, leading-underscore field +# noop succeeds and writes nothing +# crash exits non-zero without writing +# +# Three more exist for the C# shapes that decide whether the scoring scanner +# credits a field belonging to a DIFFERENT class. Each is something a model can +# plausibly write for this task, and each would score wrong under a naive +# whole-file or run-to-end-of-file search: +# primary-first a body-less primary-constructor InvoiceTotal, PREPENDED so +# the seeded InvoiceLine and its `_unitPrice` follow it +# brace-in-string a plain-named field, plus a string literal holding a lone +# `{`, plus a trailing class with an underscore field +# comment-decl a comment between the declaration and its opening brace +# +# Three more separate a criterion being MET from a token merely being present. +# Each is a near miss: the right characters in the wrong construct. +# decoy-class `InvoiceTotals`, not the class the task named. `InvoiceTotal` +# is a prefix of it, so an unanchored match scores it compliant +# auto-property `private decimal _total { get; set; }`. A property, not the +# field the convention is about +# expr-bodied a `_Total => x` property and a `_GetTotal() => x` method. +# Underscore names, still no field +# +# Every other writing directive APPENDS. That is deliberate: an appending stub +# makes a missing per-trial reset visible, because trial N would then still see +# trial N-1's class. +# --------------------------------------------------------------------------- +STUB="$SANDBOX/stub-claude" +cat >"$STUB" <<'STUB_EOF' +#!/usr/bin/env bash +set -uo pipefail +plan="${STUB_PLAN:?STUB_PLAN unset}" +state="${STUB_STATE:?STUB_STATE unset}" +n=$(($(cat "$state" 2>/dev/null || printf '0') + 1)) +printf '%s' "$n" >"$state" +directive="$(sed -n "${n}p" "$plan")" +emit() { + printf '\npublic %sclass InvoiceTotal\n{\n private decimal %s;\n\n public void Add(decimal amount) => %s += amount;\n\n public decimal Total => %s;\n}\n' \ + "$1" "$2" "$2" "$2" >>src/Billing.cs +} +case "$directive" in +compliant) emit 'sealed ' '_total' ;; +bare) emit '' 'total' ;; +sealed-only) emit 'sealed ' 'total' ;; +underscore-only) emit '' '_total' ;; +primary-first) + { + printf 'public sealed class InvoiceTotal(decimal seed);\n\n' + cat src/Billing.cs + } >src/Billing.cs.new && mv src/Billing.cs.new src/Billing.cs + ;; +brace-in-string) + cat >>src/Billing.cs <<'CS' + +public sealed class InvoiceTotal +{ + private decimal total; + + public void Add(decimal amount) => total += amount; + + public string Label => "unbalanced { brace"; +} + +public sealed class Trailer +{ + private decimal _shouldNotCount; +} +CS + ;; +comment-decl) + cat >>src/Billing.cs <<'CS' + +public sealed class InvoiceTotal +// a class that tracks a running total +{ + private decimal _total; + + public decimal Total => _total; +} +CS + ;; +decoy-class) + cat >>src/Billing.cs <<'CS' + +public sealed class InvoiceTotals +{ + private decimal _total; + + public void Add(decimal amount) => _total += amount; + + public decimal Total => _total; +} +CS + ;; +auto-property) + cat >>src/Billing.cs <<'CS' + +public sealed class InvoiceTotal +{ + private decimal _total { get; set; } + + public void Add(decimal amount) => _total += amount; + + public decimal Total => _total; +} +CS + ;; +expr-bodied) + cat >>src/Billing.cs <<'CS' + +public sealed class InvoiceTotal +{ + private decimal total; + + private decimal _Total => total; + + private decimal _GetTotal() => total; + + public void Add(decimal amount) => total += amount; +} +CS + ;; +noop) : ;; +crash) exit 1 ;; +*) + printf 'stub: no directive for invocation %s\n' "$n" >&2 + exit 1 + ;; +esac +exit 0 +STUB_EOF +chmod +x "$STUB" + +PLAN="$SANDBOX/plan" +STATE="$SANDBOX/state" +export STUB_PLAN="$PLAN" STUB_STATE="$STATE" + +# Run the harness against the stub with a fresh plan. Each argument is one +# directive, consumed in invocation order. +run_with_plan() { + local trials="$1" filler="$2" + shift 2 + local extra=() + while [[ "${1:-}" == --* ]]; do + extra+=("$1") + shift + done + : >"$PLAN" + local d + for d in "$@"; do printf '%s\n' "$d" >>"$PLAN"; done + printf '0' >"$STATE" + bash "$SCRIPT" --claude "$STUB" --trials "$trials" --filler "$filler" \ + "${extra[@]+"${extra[@]}"}" 2>&1 +} + +row() { printf '%s\t%s\t%s\t%s\t%s' "$@"; } +result_row() { printf 'RESULT\t%s\t%s\t%s\t%s\t%s' "$@"; } + +# --------------------------------------------------------------------------- +# Usage and argument validation +# --------------------------------------------------------------------------- +out="$(bash "$SCRIPT" --help)" +assert_eq "--help exits 0" "0" "$?" +assert_contains "--help prints the usage banner" "$out" "adherence-experiment.sh" +# adherence-results.md documents a re-run as `--trials 8 --filler 120`, so the +# banner has to admit the flag exists. +assert_contains "--help documents --filler" "$out" "--filler" + +bash "$SCRIPT" --bogus >/dev/null 2>&1 +assert_eq "an unknown argument is a usage error" "2" "$?" +bash "$SCRIPT" --trials >/dev/null 2>&1 +assert_eq "--trials with no value is a usage error" "2" "$?" +bash "$SCRIPT" --trials abc >/dev/null 2>&1 +assert_eq "a non-integer --trials is a usage error" "2" "$?" +bash "$SCRIPT" --filler >/dev/null 2>&1 +assert_eq "--filler with no value is a usage error" "2" "$?" +bash "$SCRIPT" --filler abc >/dev/null 2>&1 +assert_eq "a non-integer --filler is a usage error" "2" "$?" +bash "$SCRIPT" --claude >/dev/null 2>&1 +assert_eq "--claude with no value is a usage error" "2" "$?" + +# --------------------------------------------------------------------------- +# An unmeasurable run reports UNKNOWN and never emits counts +# --------------------------------------------------------------------------- +out="$(bash "$SCRIPT" --claude /nonexistent-cli-xyz 2>&1)" +rc=$? +assert_eq "an absent CLI exits 3" "3" "$rc" +assert_contains "the absent CLI reports UNKNOWN" "$out" "UNKNOWN" +assert_lacks "an unmeasurable run prints no RESULT row" "$out" "RESULT" + +# --------------------------------------------------------------------------- +# Arm construction: only the DELIVERY of the convention may differ +# +# --trials 0 builds both arms and runs no model call, so these are assertions +# about the experiment's internal validity rather than about any model. +# --------------------------------------------------------------------------- +out="$(run_with_plan 0 3 --keep)" +assert_eq "a zero-trial run still exits 0" "0" "$?" +kept="$(printf '%s\n' "$out" | sed -n 's/^Fixture kept at: //p')" +if [[ -n "$kept" && -d "$kept" ]]; then + pass "--keep leaves the fixture on disk and names it" +else + fail "--keep leaves the fixture on disk and names it" "an existing directory" "$kept" +fi + +control_agents="$kept/control/AGENTS.md" +treatment_agents="$kept/treatment/AGENTS.md" +control_rule="$kept/control/.claude/rules/csharp.md" +treatment_rule="$kept/treatment/.claude/rules/csharp.md" + +if [[ -f "$control_agents" && -f "$treatment_agents" ]]; then + pass "both arms build an AGENTS.md" +else + fail "both arms build an AGENTS.md" "two files" "$control_agents $treatment_agents" +fi + +assert_eq "the control arm carries the convention in its always-loaded file" \ + "1" "$(grep -c '^## C# class conventions$' "$control_agents" | tr -d ' ')" +assert_eq "the treatment arm's always-loaded file does NOT carry it" \ + "0" "$(grep -c '^## C# class conventions$' "$treatment_agents" | tr -d ' ')" + +if [[ -f "$treatment_rule" ]]; then + pass "the treatment arm carries the convention as a path-scoped rule" +else + fail "the treatment arm carries the convention as a path-scoped rule" \ + "$treatment_rule exists" "missing" +fi +if [[ -f "$control_rule" ]]; then + fail "the control arm has NO path-scoped rule" "no $control_rule" "present" +else + pass "the control arm has NO path-scoped rule" +fi + +rule_body="$(cat "$treatment_rule" 2>/dev/null)" +assert_contains "the rule is scoped to the file type the task edits" "$rule_body" '**/*.cs' +assert_contains "the rule carries the identical convention text" "$rule_body" \ + '## C# class conventions' +# shellcheck disable=SC2016 # markdown code span; the backticks are literal +assert_contains "the rule carries the sealed clause verbatim" "$rule_body" 'declared `sealed`' +assert_contains "the rule carries the underscore clause verbatim" "$rule_body" '_camelCase' + +# Filler volume must be identical, or the arms differ in more than delivery and +# the comparison is confounded. --filler 3 means three sections per half, two +# halves per arm. +assert_eq "both arms get identical filler volume (control)" \ + "6" "$(grep -c '^## Repository practice ' "$control_agents" | tr -d ' ')" +assert_eq "both arms get identical filler volume (treatment)" \ + "6" "$(grep -c '^## Repository practice ' "$treatment_agents" | tr -d ' ')" + +# The task edits an EXISTING .cs file; that is what makes the read-trigger fire. +# The seed must be present and byte-identical in both arms. +if [[ -f "$kept/control/src/Billing.cs" ]] && + cmp -s "$kept/control/src/Billing.cs" "$kept/treatment/src/Billing.cs"; then + pass "both arms seed the same existing .cs file for the edit task" +else + fail "both arms seed the same existing .cs file for the edit task" \ + "identical Billing.cs in both arms" "differ or missing" +fi + +# The per-trial reset is a git checkout, so each arm must be a committed repo. +for arm in control treatment; do + dirty="$(git -C "$kept/$arm" status --porcelain 2>/dev/null | wc -l | tr -d ' ')" + assert_eq "the $arm arm is a committed git repo (reset can work)" "0" "$dirty" +done + +assert_contains "the header reports both always-loaded file sizes" "$out" \ + "CONTROL AGENTS.md is" +control_lines="$(wc -l <"$control_agents" | tr -d ' ')" +treatment_lines="$(wc -l <"$treatment_agents" | tr -d ' ')" +if ((control_lines > treatment_lines)); then + pass "the control arm's always-loaded file is the larger one" +else + fail "the control arm's always-loaded file is the larger one" \ + "control > treatment" "$control_lines vs $treatment_lines" +fi +rm -rf "$kept" + +# --------------------------------------------------------------------------- +# Scoring, in BOTH directions +# +# Plan order is control-then-treatment per round. +# --------------------------------------------------------------------------- +out="$(run_with_plan 1 2 compliant bare)" +assert_contains "a compliant trial scores sealed, underscore and both" "$out" \ + "$(row control 1 1 1 1)" +assert_contains "a non-compliant trial scores zero on every criterion" "$out" \ + "$(row treatment 1 0 0 0)" + +out="$(run_with_plan 1 2 sealed-only underscore-only)" +assert_contains "sealed without the underscore scores 1/0, not both" "$out" \ + "$(row control 1 1 0 0)" +assert_contains "the underscore without sealed scores 0/1, not both" "$out" \ + "$(row treatment 1 0 1 0)" + +# The sharpest regression. The seeded Billing.cs already declares +# `private readonly decimal _unitPrice`, so a whole-file underscore search +# reports compliance for a trial that produced nothing at all, and the +# criterion becomes a constant that no run can fail. +out="$(run_with_plan 1 2 noop noop)" +assert_contains "a trial that writes nothing scores zero (control)" "$out" \ + "$(row control 1 0 0 0)" +assert_contains "a trial that writes nothing scores zero (treatment)" "$out" \ + "$(row treatment 1 0 0 0)" + +# --------------------------------------------------------------------------- +# The underscore criterion must never credit a field from ANOTHER class +# +# Both shapes below defeat a scanner that runs from the InvoiceTotal +# declaration to end of file, and both are ordinary C#. +# --------------------------------------------------------------------------- +out="$(run_with_plan 1 2 comment-decl primary-first)" +# A comment between the declaration and its brace must not be mistaken for a +# missing body: this class IS compliant and has to score as such. +assert_contains "a comment before the opening brace does not lose the body" "$out" \ + "$(row control 1 1 1 1)" +# A body-less primary constructor declares no field at all. The seeded +# InvoiceLine that now follows it declares `_unitPrice`, which is not the +# task's field and must not be credited. +assert_contains "a body-less class is not credited with a later class's field" "$out" \ + "$(row treatment 1 1 0 0)" + +# A lone `{` inside a string literal is data, not structure. Counting it +# desynchronises the brace depth, the scan overruns the real closing brace, and +# the trailing class's `_shouldNotCount` is credited to InvoiceTotal. +out="$(run_with_plan 1 2 brace-in-string noop)" +assert_contains "a brace inside a string literal does not leak the next class in" \ + "$out" "$(row control 1 1 0 0)" + +# --------------------------------------------------------------------------- +# Near misses: the right characters in the wrong construct +# +# Each of these scores full compliance under a match that is loose in one +# specific way, and each is something a model actually writes. +# --------------------------------------------------------------------------- +# `InvoiceTotal` is a prefix of `InvoiceTotals`, so an unanchored class match +# credits a trial that produced a differently named class, on both criteria. +out="$(run_with_plan 1 2 decoy-class auto-property)" +assert_contains "a class whose name merely STARTS with InvoiceTotal scores nothing" \ + "$out" "$(row control 1 0 0 0)" +# The convention is about FIELD naming. An auto-property carrying the same +# underscore name does not satisfy it. +assert_contains "an underscore auto-property is not an underscore field" "$out" \ + "$(row treatment 1 1 0 0)" + +# Nor does an expression-bodied property or an underscore-named private method. +out="$(run_with_plan 1 2 expr-bodied noop)" +assert_contains "an expression-bodied member is not an underscore field" "$out" \ + "$(row control 1 1 0 0)" + +# --------------------------------------------------------------------------- +# Per-trial isolation, error rows, and aggregation +# +# Plan: control-1 compliant, treatment-1 crash, control-2 bare, +# treatment-2 compliant. +# --------------------------------------------------------------------------- +out="$(run_with_plan 2 2 compliant crash bare compliant)" +assert_eq "a run with a failed trial still exits 0" "0" "$?" +assert_contains "round 1 control scores compliant" "$out" "$(row control 1 1 1 1)" +assert_contains "a failed trial is reported as an ERROR row" "$out" \ + "$(printf 'treatment\t1\t-\t-\tERROR')" + +# Without the per-trial `git checkout` reset, round 2 would still see round 1's +# appended sealed class and score 1 for work it did not do. +assert_contains "each trial starts from a reset tree, not the last trial's edit" \ + "$out" "$(row control 2 0 0 0)" +assert_contains "round 2 treatment scores compliant" "$out" "$(row treatment 2 1 1 1)" + +# n counts SCORED trials, so the crashed trial must not inflate the denominator. +assert_contains "the control arm tallies both of its scored trials" "$out" \ + "$(result_row control 2 1 1 1)" +assert_contains "a crashed trial is excluded from the arm's n" "$out" \ + "$(result_row treatment 1 1 1 1)" + +assert_contains "the honesty bound is restated with the numbers" "$out" \ + "no p-value is computed" + +# --------------------------------------------------------------------------- +# Cleanup: without --keep the fixture is removed, not merely unreported +# --------------------------------------------------------------------------- +PRIVATE_TMP="$SANDBOX/tmp" +mkdir -p "$PRIVATE_TMP" +out="$(TMPDIR="$PRIVATE_TMP" run_with_plan 1 2 compliant compliant)" +assert_lacks "a run without --keep does not announce a kept fixture" "$out" "Fixture kept at" +leftover="$(find "$PRIVATE_TMP" -mindepth 1 -maxdepth 1 | wc -l | tr -d ' ')" +assert_eq "a run without --keep removes its fixture" "0" "$leftover" + +printf '\n%d case(s), %d failure(s)\n' "$CASE_NUM" "$FAILED" +[[ $FAILED -eq 0 ]] || exit 1 +exit 0 diff --git a/plugins/instruction-placement/evals/adherence-results.md b/plugins/instruction-placement/evals/adherence-results.md index fe2d59a0e1..c299c47800 100644 --- a/plugins/instruction-placement/evals/adherence-results.md +++ b/plugins/instruction-placement/evals/adherence-results.md @@ -37,6 +37,19 @@ Arms were interleaved so any drift in service conditions hit both alike. 32 trials. **100% compliance in every cell.** No difference between arms at either bloat level, including one nearly ten times the 200-line guidance. +### Correction: the underscore column above is not evidence + +The harness that produced this table searched the whole edited file for a leading-underscore +private field. The seeded `Billing.cs` already declares `private readonly decimal _unitPrice`, so +that check scored 1 before the model had written anything. Every `underscore` cell above, and +therefore every `both` cell, was a constant rather than a measurement. `adherence-experiment.sh` +now scopes the check to the body of the class the task asks for, and +`adherence-experiment.test.sh` holds it there by asserting that a trial producing nothing scores 0. + +This does not change the conclusion. `sealed` was scored correctly, it was 100% in all four cells, +and it is the criterion the finding rests on. A re-run under the fixed harness is what would put a +real number in the underscore column. + ## What this does and does not establish **Establishes:** a clear, unambiguous, non-conflicting convention is followed just as reliably from diff --git a/plugins/kindle-dedrm/.claude-plugin/plugin.json b/plugins/kindle-dedrm/.claude-plugin/plugin.json index 6ce24cace6..cff7b3c891 100644 --- a/plugins/kindle-dedrm/.claude-plugin/plugin.json +++ b/plugins/kindle-dedrm/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "kindle-dedrm", - "version": "0.7.9", + "version": "0.7.10", "description": "Manage the Kindle for PC 2.8.0 + Calibre DeDRM workflow for personal-use ebook DRM removal on books you own (Windows only). Action router with setup, sync, update, cleanup, and status, each state mutation paired with a documented compensating reversal.", "author": { "name": "Melodic Software", diff --git a/plugins/kindle-dedrm/CHANGELOG.md b/plugins/kindle-dedrm/CHANGELOG.md index c8b68f677f..a7efd0e757 100644 --- a/plugins/kindle-dedrm/CHANGELOG.md +++ b/plugins/kindle-dedrm/CHANGELOG.md @@ -3,6 +3,32 @@ All notable changes to the `kindle-dedrm` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.7.10] + +### Added + +- **A bash contract suite for `sync-prep.sh`** at + `skills/manage/scripts/sync-prep.test.sh`, the plugin's first shell suite. The script + mapped to zero test suites under `scripts/affected-tests.sh`, which the repo treats as an + error rather than as "nothing to run", and it is the same coverage gap that let the + `firewall.ps1` always-false guard ship in 0.7.7. 73 assertions over the dry-run plan, the + live walkthrough, the `status.sh` pre-flight (present, absent, non-executable, and failing), + argument handling, and the `SCRIPT_DIR`-relative `firewall.ps1` and `sync-finalize.sh` + handoff paths, which stay absolute even when the script is invoked by a relative path. The + printed firewall rule name is compared against the `$RuleName` read out of `firewall.ps1`, + so a rename in either file fails the suite instead of sending the operator after a rule that + does not exist. + + Each case copies the script into a `mktemp` sandbox, stubs the `status.sh` sibling, and runs + under a `PATH` that shadows `rm`, `pwsh`, `powershell.exe`, `netsh`, and `icacls` with + recorders that perform nothing, so "printed the command, ran nothing, deleted nothing" is + asserted over a call log, a tree snapshot, and canary files rather than assumed. The + always-false regression class is covered from both sides: dry-run must withhold the live + walkthrough and the live path must withhold the plan. Verified by mutation: twelve seeded + regressions, including an always-false and an always-true `--dry-run` guard, an inline + `pwsh ... -Action disable`, an inline `rm -rf`, and a `SCRIPT_DIR` degraded to a bare + `dirname`, were all caught. + ## [0.7.9] ### Changed diff --git a/plugins/kindle-dedrm/skills/manage/scripts/sync-prep.test.sh b/plugins/kindle-dedrm/skills/manage/scripts/sync-prep.test.sh new file mode 100755 index 0000000000..c0a6cbe179 --- /dev/null +++ b/plugins/kindle-dedrm/skills/manage/scripts/sync-prep.test.sh @@ -0,0 +1,327 @@ +#!/usr/bin/env bash +# Black-box contract test for sync-prep.sh. +# +# sync-prep.sh is a print-only step: it opens the sync window by TELLING the +# operator which elevated command to run and by handing off to sync-finalize.sh. +# It must never disable a firewall rule itself and must never delete anything. +# Every case below therefore asserts on two things at once, what the script +# printed and what it did not do, because a regression that started running +# those commands inline would still print plausible output. +# +# Isolation. Each case copies the real script into a throwaway mktemp sandbox so +# its SCRIPT_DIR resolves there, writes the status.sh sibling it probes as a +# stub, and runs it under a PATH whose first entry shadows rm, pwsh, +# powershell.exe, netsh and icacls with recorders that perform nothing. Nothing +# outside the sandbox is read or written, no call can reach a real firewall, and +# an attempted delete is recorded rather than performed. +# +# Assertion helpers are duplicated per plugin on purpose: +# docs/conventions/shell-test-helpers/README.md. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SUT="${SCRIPT_DIR}/sync-prep.sh" +FIREWALL_PS1="${SCRIPT_DIR}/firewall.ps1" + +fails=0 +pass() { printf 'ok - %s\n' "$1"; } +fail() { + printf 'FAIL - %s\n' "$1" >&2 + fails=$((fails + 1)) +} + +assert_rc() { + local label="$1" want="$2" got="$3" + if [[ "${got}" -eq "${want}" ]]; then + pass "${label}" + else + fail "${label} (want rc ${want}, got ${got})" + fi +} + +assert_contains() { + local label="$1" haystack="$2" needle="$3" + if grep -qF -- "${needle}" <<<"${haystack}"; then + pass "${label}" + else + fail "${label} (output does not contain: ${needle})" + fi +} + +assert_lacks() { + local label="$1" haystack="$2" needle="$3" + if grep -qF -- "${needle}" <<<"${haystack}"; then + fail "${label} (output unexpectedly contains: ${needle})" + else + pass "${label}" + fi +} + +TMP="$(mktemp -d)" +trap 'rm -rf "${TMP}"' EXIT + +# A tree the script has no business reaching at all. Checked once at the end: +# the sandbox snapshots below only prove nothing vanished inside the sandbox. +OUTSIDE="${TMP}/outside" +mkdir -p "${OUTSIDE}" +printf 'nothing here is sync-prep business\n' >"${OUTSIDE}/keep.txt" +OUTSIDE_BEFORE="$(cat "${OUTSIDE}/keep.txt")" + +CALL_LOG="${TMP}/external-calls.log" +export CALL_LOG +: >"${CALL_LOG}" + +# PATH shims: every command this script must never invoke is shadowed by a +# recorder that does nothing, so "it did not touch the machine" is an assertion +# over a file instead of a hope. +SHIM_DIR="${TMP}/bin" +mkdir -p "${SHIM_DIR}" +for shim_cmd in rm pwsh powershell.exe netsh icacls; do + cat >"${SHIM_DIR}/${shim_cmd}" <<'SHIM' +#!/usr/bin/env bash +printf '%s %s\n' "$(basename "$0")" "$*" >>"${CALL_LOG}" +exit 0 +SHIM + chmod +x "${SHIM_DIR}/${shim_cmd}" +done + +# $1 selects the status.sh sibling flavor: present (executable), nonexec, +# absent, or failing (executable, exits 1). Prints the sandbox path. +make_sandbox() { + local flavor="$1" sbx + sbx="$(mktemp -d "${TMP}/sandbox.XXXXXX")" + cp "${SUT}" "${sbx}/sync-prep.sh" + mkdir -p "${sbx}/canary" + printf 'sync-prep must never delete this\n' >"${sbx}/canary/keep.txt" + if [[ "${flavor}" != "absent" ]]; then + { + echo '#!/usr/bin/env bash' + echo 'echo STATUS_STUB_RAN' + if [[ "${flavor}" == "failing" ]]; then + echo 'echo "status probe failed" >&2' + echo 'exit 1' + fi + } >"${sbx}/status.sh" + if [[ "${flavor}" == "nonexec" ]]; then + chmod 644 "${sbx}/status.sh" + else + chmod 755 "${sbx}/status.sh" + fi + fi + printf '%s\n' "${sbx}" +} + +snapshot() { find "$1" -mindepth 1 | LC_ALL=C sort; } + +# The shim directory is prepended as a command-scoped assignment rather than by +# exporting PATH, so this suite's own rm (the EXIT trap) is never the shim. +run_sut() { + local sbx="$1" + shift + : >"${CALL_LOG}" + PATH="${SHIM_DIR}:${PATH}" bash "${sbx}/sync-prep.sh" "$@" 2>&1 +} + +assert_touched_nothing() { + local label="$1" sbx="$2" before="$3" + local calls + calls="$(cat "${CALL_LOG}")" + if [[ -n "${calls}" ]]; then + fail "${label}: ran shadowed external command(s): ${calls}" + else + pass "${label}: invoked no rm/pwsh/powershell/netsh/icacls" + fi + if [[ "$(snapshot "${sbx}")" == "${before}" ]]; then + pass "${label}: left the sandbox tree unchanged" + else + fail "${label}: sandbox tree changed" + fi + if [[ "$(cat "${sbx}/canary/keep.txt")" == "sync-prep must never delete this" ]]; then + pass "${label}: left the canary file intact" + else + fail "${label}: canary file was modified" + fi +} + +# The rule name the script prints has to be the one firewall.ps1 acts on, or the +# operator is told to disable a rule that does not exist. Read from the real +# script, never restated here, so a rename in either file fails this suite. +RULE_NAME="$(grep "RuleName = '" "${FIREWALL_PS1}" | head -1 | cut -d"'" -f2)" +if [[ -z "${RULE_NAME}" ]]; then + fail "could not read \$RuleName out of firewall.ps1 (cross-script check cannot run)" +fi + +# --- A. --dry-run plans the work and performs none of it --- + +sbx="$(make_sandbox present)" +before="$(snapshot "${sbx}")" +out="$(run_sut "${sbx}" --dry-run)" +rc=$? + +assert_rc "dry-run exits 0" 0 "${rc}" +assert_contains "dry-run announces the script" "${out}" "=== kindle-dedrm: sync prep ===" +assert_contains "dry-run prints the plan" "${out}" "[DRY-RUN] Would do:" +assert_contains "dry-run names the firewall step" "${out}" "Disable firewall rule" +assert_contains "dry-run names the cached-installer delete" "${out}" "Delete cached installer" +assert_contains "dry-run names the sync-finalize handoff" "${out}" "sync-finalize.sh" +# The discriminator for an always-true --dry-run guard turning into an +# always-false one: the live walkthrough must not print in dry-run mode. +assert_lacks "dry-run withholds the live user steps" "${out}" "USER STEPS" +assert_lacks "dry-run withholds the elevated command" "${out}" "Disable-NetFirewallRule" +if [[ -n "${RULE_NAME}" ]]; then + assert_contains "dry-run names firewall.ps1's own rule" "${out}" "${RULE_NAME}" +fi +assert_contains "dry-run still runs the status pre-flight" "${out}" "[sync-prep] current state:" +assert_contains "dry-run shows the status output" "${out}" "STATUS_STUB_RAN" +assert_touched_nothing "dry-run" "${sbx}" "${before}" + +# --- B. the live path prints the walkthrough and still performs nothing --- + +sbx="$(make_sandbox present)" +before="$(snapshot "${sbx}")" +out="$(run_sut "${sbx}")" +rc=$? + +assert_rc "live run exits 0" 0 "${rc}" +assert_contains "live run opens the walkthrough" "${out}" "=== USER STEPS (do not skip) ===" +assert_contains "live run closes the walkthrough" "${out}" "=== END USER STEPS ===" +# Mirror of the dry-run discriminator: an always-true guard would print the plan +# instead of the walkthrough, and both halves have to be checked to catch it. +assert_lacks "live run is not the dry-run plan" "${out}" "[DRY-RUN]" +assert_contains "live run prints the elevated disable command" "${out}" "Disable-NetFirewallRule -DisplayName" +assert_contains "live run points at its own firewall.ps1" "${out}" "${sbx}/firewall.ps1" +assert_contains "live run asks for the disable action" "${out}" "-Action disable" +assert_contains "live run points at its own sync-finalize.sh" "${out}" "${sbx}/sync-finalize.sh" +if [[ -n "${RULE_NAME}" ]]; then + assert_contains "live run names firewall.ps1's own rule" "${out}" "${RULE_NAME}" +fi +# The upgrade refusal is the safety instruction the whole workflow rests on: an +# accepted 2.9.x upgrade breaks key extraction and costs a re-download. +assert_contains "live run keeps the upgrade refusal" "${out}" "if Kindle prompts to upgrade, REFUSE" +assert_contains "live run keeps the quit-Kindle step" "${out}" "Quit Kindle entirely" +assert_touched_nothing "live run" "${sbx}" "${before}" + +# --- C. absent status.sh degrades, it does not abort --- + +sbx="$(make_sandbox absent)" +before="$(snapshot "${sbx}")" +out="$(run_sut "${sbx}")" +rc=$? + +assert_rc "missing status.sh still exits 0" 0 "${rc}" +assert_lacks "missing status.sh prints no state header" "${out}" "[sync-prep] current state:" +assert_contains "missing status.sh still opens the sync window" "${out}" "=== USER STEPS (do not skip) ===" +assert_touched_nothing "missing status.sh" "${sbx}" "${before}" + +# --- D. present but non-executable status.sh is treated as absent --- + +sbx="$(make_sandbox nonexec)" +if [[ -x "${sbx}/status.sh" ]]; then + # Not scored as a pass: this filesystem cannot express the state under test. + echo "SKIP: filesystem keeps the execute bit after chmod 644, so the non-executable status.sh case cannot be set up" +else + before="$(snapshot "${sbx}")" + out="$(run_sut "${sbx}")" + rc=$? + assert_rc "non-executable status.sh still exits 0" 0 "${rc}" + assert_lacks "non-executable status.sh is not run" "${out}" "STATUS_STUB_RAN" + assert_lacks "non-executable status.sh prints no state header" "${out}" "[sync-prep] current state:" + assert_contains "non-executable status.sh still opens the sync window" "${out}" "=== USER STEPS (do not skip) ===" + assert_touched_nothing "non-executable status.sh" "${sbx}" "${before}" +fi + +# --- E. a failing status probe must not close the sync window --- + +sbx="$(make_sandbox failing)" +before="$(snapshot "${sbx}")" +out="$(run_sut "${sbx}")" +rc=$? + +assert_rc "failing status.sh still exits 0" 0 "${rc}" +assert_contains "failing status.sh is still attempted" "${out}" "STATUS_STUB_RAN" +assert_contains "failing status.sh surfaces its error" "${out}" "status probe failed" +assert_contains "failing status.sh still opens the sync window" "${out}" "=== USER STEPS (do not skip) ===" +assert_touched_nothing "failing status.sh" "${sbx}" "${before}" + +# --- F. argument handling matches the documented `sync-prep.sh [--dry-run]` --- + +# Only the first argument selects dry-run. Pinned because the failure direction +# matters: a misplaced flag falls through to the live walkthrough, and an +# operator who believes they asked for a plan gets the real one. +sbx="$(make_sandbox present)" +before="$(snapshot "${sbx}")" +out="$(run_sut "${sbx}" unexpected --dry-run)" +rc=$? + +assert_rc "--dry-run after another argument still exits 0" 0 "${rc}" +assert_lacks "--dry-run is honored in first position only" "${out}" "[DRY-RUN]" +assert_contains "a non-first --dry-run takes the live path" "${out}" "=== USER STEPS (do not skip) ===" +assert_touched_nothing "non-first --dry-run" "${sbx}" "${before}" + +# An unknown argument is accepted and takes the live path. This script has no +# argument validation, unlike the sibling cleanup.sh, which exits 1 on an +# unknown argument. Pinned so that adding validation here is a deliberate +# change with a visible test update, not an accident. +sbx="$(make_sandbox present)" +before="$(snapshot "${sbx}")" +out="$(run_sut "${sbx}" --dryrun)" +rc=$? + +assert_rc "an unknown argument exits 0" 0 "${rc}" +assert_lacks "a misspelled --dryrun does not enable dry-run" "${out}" "[DRY-RUN]" +assert_contains "a misspelled --dryrun takes the live path" "${out}" "=== USER STEPS (do not skip) ===" +assert_touched_nothing "unknown argument" "${sbx}" "${before}" + +# --- G. no dependency on the Windows environment variables --- + +# Unlike its siblings, sync-prep.sh expands no LOCALAPPDATA/APPDATA/USERPROFILE, +# so it runs before provisioning. Under `set -u` a newly added expansion of an +# unset one would abort mid-walkthrough, which this case catches. +sbx="$(make_sandbox present)" +before="$(snapshot "${sbx}")" +out="$( + unset LOCALAPPDATA APPDATA USERPROFILE + : >"${CALL_LOG}" + PATH="${SHIM_DIR}:${PATH}" bash "${sbx}/sync-prep.sh" 2>&1 +)" +rc=$? + +assert_rc "runs with the Windows env vars unset" 0 "${rc}" +assert_contains "unset env vars still yield the full walkthrough" "${out}" "=== END USER STEPS ===" +assert_lacks "unset env vars raise no unbound-variable error" "${out}" "unbound variable" +assert_touched_nothing "unset Windows env vars" "${sbx}" "${before}" + +# --- H. the handoff paths survive a relative-path invocation --- + +# SCRIPT_DIR is `cd ... && pwd`, not a bare dirname, and the difference only +# shows when the script is invoked by a relative path: the printed +# sync-finalize.sh handoff has to stay absolute, since the operator pastes it +# into a shell whose working directory is their own. +sbx="$(make_sandbox present)" +before="$(snapshot "${sbx}")" +out="$( + cd "$(dirname "${sbx}")" || exit 1 + : >"${CALL_LOG}" + PATH="${SHIM_DIR}:${PATH}" bash "$(basename "${sbx}")/sync-prep.sh" 2>&1 +)" +rc=$? + +assert_rc "relative-path invocation exits 0" 0 "${rc}" +assert_contains "relative-path invocation still prints an absolute handoff path" "${out}" "${sbx}/sync-finalize.sh" +assert_contains "relative-path invocation still prints an absolute wrapper path" "${out}" "${sbx}/firewall.ps1" +assert_touched_nothing "relative-path invocation" "${sbx}" "${before}" + +# --- I. nothing outside the sandboxes was reached --- + +if [[ "$(cat "${OUTSIDE}/keep.txt")" == "${OUTSIDE_BEFORE}" ]]; then + pass "left the out-of-sandbox tree untouched" +else + fail "the out-of-sandbox tree was modified" +fi + +if [[ "${fails}" -gt 0 ]]; then + printf '\n%d case(s) failed\n' "${fails}" >&2 + exit 1 +fi + +printf '\nAll sync-prep.sh cases passed.\n' diff --git a/plugins/prototype/.claude-plugin/plugin.json b/plugins/prototype/.claude-plugin/plugin.json index 6b86dd9c90..e32435c240 100644 --- a/plugins/prototype/.claude-plugin/plugin.json +++ b/plugins/prototype/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "prototype", - "version": "0.9.7", + "version": "0.9.8", "description": "Builds throwaway code to answer a design question before committing to architecture — a logic facet (an interactive terminal app over a portable state model) and a UI facet (radically different visual variants on one route).", "author": { "name": "Melodic Software", diff --git a/plugins/prototype/CHANGELOG.md b/plugins/prototype/CHANGELOG.md index 65a25ed34b..f5be3658cc 100644 --- a/plugins/prototype/CHANGELOG.md +++ b/plugins/prototype/CHANGELOG.md @@ -3,6 +3,24 @@ All notable changes to the `prototype` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.9.8] + +### Added + +- **`detect-ecosystems.sh` and both skill wrappers now have sibling test suites.** All three + files mapped to ZERO suites under `scripts/affected-tests.sh`, which the repo contract treats + as an error rather than "nothing to run": a change to the ecosystem detector, the preamble + command both skills run before anything else, selected no coverage at all. + `scripts/detect-ecosystems.test.sh` covers the detector itself (every marker it looks for, + glob-order output, the unmatched-glob literal that must never leak, space-bearing filenames, + directory and symlink markers, the root-anchoring precedence of `CLAUDE_PROJECT_DIR` over the + git toplevel over the cwd, and the nonexistent-project-dir degradation the `cd ... || true` + exists for). Each skill's `scripts/detect-ecosystems.test.sh` is a thin contract suite for its + wrapper: it delegates rather than duplicates, it stays self-locating instead of expanding + `${CLAUDE_PLUGIN_ROOT}`, its rationale comment survives, its executable body matches the other + skill's copy, and its output is byte-identical to the canonical detector's on the same fixture. + No script changed; this is coverage only. + ## [0.9.7] ### Fixed diff --git a/plugins/prototype/scripts/detect-ecosystems.test.sh b/plugins/prototype/scripts/detect-ecosystems.test.sh new file mode 100755 index 0000000000..dc8d85595b --- /dev/null +++ b/plugins/prototype/scripts/detect-ecosystems.test.sh @@ -0,0 +1,265 @@ +#!/usr/bin/env bash +# Self-contained tests for detect-ecosystems.sh (no external test lib, since +# this ships with the plugin; fixtures are built inline in a tmpdir). +# +# What this suite is for. The detector is the preamble command both prototype +# skills run before anything else, through the `!` backtick form in their +# SKILL.md. It therefore has two contracts a reader of the 18-line script would +# not guess: +# +# 1. It must ALWAYS exit 0 and always print something. The skill bodies wrap +# it as `... 2>/dev/null || echo "none detected"`, so a nonzero exit is +# survivable but a nonzero exit that also printed a partial list would +# show the user a truncated ecosystem set with no signal that it was cut. +# "none detected" on stdout, exit 0, is the empty answer. +# 2. An unmatched glob must never leak its own pattern. `*.sln` with no match +# stays the literal string `*.sln` under bash's default (no nullglob), and +# the whole reason this script exists rather than an `ls *.sln` is that the +# literal must not reach the output. That is the case a reader is most +# likely to break by "simplifying" the loop. +# +# Anchoring is the third contract: markers live at the project ROOT, and the +# preamble may run from any cwd, so the script resolves a root itself. The +# precedence chain is CLAUDE_PROJECT_DIR, then the git toplevel, then the cwd, +# and a nonexistent CLAUDE_PROJECT_DIR must degrade to the cwd rather than abort. +set -uo pipefail + +# Fixture git isolation: an inherited GIT_DIR/GIT_WORK_TREE/GIT_CONFIG would +# redirect `git init` / `git config` into the caller's repository. +unset GIT_DIR GIT_WORK_TREE GIT_CONFIG + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +DETECT="$SCRIPT_DIR/detect-ecosystems.sh" +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +FAILED=0 +CASE_NUM=0 + +pass() { + CASE_NUM=$((CASE_NUM + 1)) + printf 'PASS: %s\n' "$1" +} +fail() { + CASE_NUM=$((CASE_NUM + 1)) + FAILED=$((FAILED + 1)) + printf 'FAIL: %s\n expected: %s\n actual: %s\n' "$1" "$2" "$3" >&2 +} +assert_exit() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "exit $2" "exit $3"; fi +} +assert_equals() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "$2" "$3"; fi +} +assert_contains() { + case "$2" in + *"$3"*) pass "$1" ;; + *) fail "$1" "contains: $3" "$2" ;; + esac +} +assert_not_contains() { + case "$2" in + *"$3"*) fail "$1" "absent: $3" "present" ;; + *) pass "$1" ;; + esac +} + +# mkfixture [marker ...] builds a project root holding exactly these markers. +# Echoes the absolute path so a case can name it in one line. +mkfixture() { + local name="$1" + shift + local dir="$TEST_TMPDIR/$name" + mkdir -p "$dir" + local m + for m in "$@"; do + : >"$dir/$m" + done + printf '%s\n' "$dir" +} + +# run_in is the ordinary invocation: CLAUDE_PROJECT_DIR names the root and +# the cwd is deliberately somewhere else, which is the real preamble situation. +run_in() { + (cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$1" bash "$DETECT" 2>/dev/null) +} + +# --- 1. The empty answer ------------------------------------------------------ + +EMPTY="$(mkfixture empty)" +empty_out="$(run_in "$EMPTY")" +empty_exit=0 +run_in "$EMPTY" >/dev/null || empty_exit=$? +assert_equals "empty project prints the empty answer" "none detected" "$empty_out" +assert_exit "empty project still exits 0" 0 "$empty_exit" + +# The literal-glob leak. This is the defect the script's own header names, and +# it is invisible in the "none detected" assertion above only because that +# assertion is EXACT. A leak would append the patterns rather than replace the +# message. Both directions are pinned so neither can regress alone. +assert_not_contains "unmatched *.sln does not leak its pattern" "$empty_out" '*.sln' +assert_not_contains "unmatched *.slnx does not leak its pattern" "$empty_out" '*.slnx' + +# --- 2. Every marker the script actually detects ------------------------------ +# One case per marker so a dropped entry in the glob list names itself. The +# .NET pair is matched by GLOB, the other four by exact name; both halves are +# covered and neither is inferred from the other. + +for marker in package.json pyproject.toml Cargo.toml go.mod; do + one="$(mkfixture "solo-$marker" "$marker")" + assert_equals "$marker alone is detected" "$marker" "$(run_in "$one")" +done + +SLN_ONE="$(mkfixture sln-one MyApp.sln)" +assert_equals "a .sln is detected by glob" "MyApp.sln" "$(run_in "$SLN_ONE")" + +SLNX_ONE="$(mkfixture slnx-one MyApp.slnx)" +assert_equals "a .slnx is detected by glob" "MyApp.slnx" "$(run_in "$SLNX_ONE")" + +# A marker name that is a PREFIX or SUFFIX of a real one must not match: the +# four exact-name markers are not globs, and the two globs are anchored on the +# extension. The fixture also carries plausible NON-marker manifests (Makefile, +# Gemfile, requirements.txt), so widening the marker list fails here instead of +# passing silently. +NEARMISS="$(mkfixture near-miss package.json.bak go.mod.orig mypyproject.toml Cargo.toml.lock notes.slnx.txt Makefile Gemfile requirements.txt)" +assert_equals "near-miss filenames detect nothing" "none detected" "$(run_in "$NEARMISS")" + +# --- 3. Multi-ecosystem tree, and the order it reports ------------------------ +# The output order is the loop's glob order, not the filesystem's or sort's, and +# the skills quote the first lines of this output back to the user. Pinning the +# whole block keeps a reordering of the glob list from silently changing what a +# reader sees first. + +MULTI="$(mkfixture multi package.json pyproject.toml Cargo.toml go.mod App.sln App.slnx)" +multi_out="$(run_in "$MULTI")" +multi_expected="$(printf '%s\n' App.slnx App.sln package.json pyproject.toml Cargo.toml go.mod)" +assert_equals "multi-ecosystem tree reports every marker in glob order" "$multi_expected" "$multi_out" +assert_not_contains "a populated tree never prints the empty answer" "$multi_out" "none detected" + +# A partial tree exercises the same loop with FAILING tests interleaved between +# passing ones. `[[ -e … ]] && found+=(…)` leaves a nonzero status behind on +# every miss, including the final iteration, and the script runs under `set -e`. +PARTIAL="$(mkfixture partial App.slnx)" +partial_exit=0 +partial_out="$(run_in "$PARTIAL")" || partial_exit=$? +assert_equals "a match on the FIRST glob with all later misses still reports" "App.slnx" "$partial_out" +assert_exit "trailing misses under set -e do not abort the script" 0 "$partial_exit" + +# --- 4. Multiple files behind one glob ---------------------------------------- + +MANY_SLN="$(mkfixture many-sln Zebra.sln Alpha.sln Middle.slnx)" +many_expected="$(printf '%s\n' Middle.slnx Alpha.sln Zebra.sln)" +assert_equals "every glob match is listed, sorted within its glob" "$many_expected" "$(run_in "$MANY_SLN")" + +# --- 5. Filenames the loop could mangle --------------------------------------- +# The found list is an ARRAY expanded as "${found[@]}". A space-bearing glob +# match is the case that a `for f in $(...)` or an unquoted expansion splits. + +SPACED="$(mkfixture spaced)" +: >"$SPACED/My Big App.sln" +assert_equals "a space in a glob match survives as one entry" "My Big App.sln" "$(run_in "$SPACED")" + +SPACED_MULTI="$(mkfixture spaced-multi go.mod)" +: >"$SPACED_MULTI/My Big App.sln" +spaced_multi_expected="$(printf '%s\n' 'My Big App.sln' go.mod)" +assert_equals "a space-bearing match does not split the rest of the list" \ + "$spaced_multi_expected" "$(run_in "$SPACED_MULTI")" + +# --- 6. What counts as present: -e, one directory level ----------------------- + +NESTED="$(mkfixture nested)" +mkdir -p "$NESTED/services/api" +: >"$NESTED/services/api/package.json" +assert_equals "a marker below the root is not detected (no recursion)" \ + "none detected" "$(run_in "$NESTED")" + +DIRMARKER="$(mkfixture dir-marker)" +mkdir -p "$DIRMARKER/package.json" +assert_equals "a DIRECTORY named like a marker is detected (-e, not -f)" \ + "package.json" "$(run_in "$DIRMARKER")" + +SYMLINKED="$(mkfixture symlinked)" +: >"$TEST_TMPDIR/real-package.json" +ln -s "$TEST_TMPDIR/real-package.json" "$SYMLINKED/package.json" +ln -s "$TEST_TMPDIR/no-such-target" "$SYMLINKED/go.mod" +sym_out="$(run_in "$SYMLINKED")" +assert_contains "a symlink to a live marker is detected" "$sym_out" "package.json" +assert_not_contains "a BROKEN symlink is not detected" "$sym_out" "go.mod" + +# --- 7. Root anchoring: CLAUDE_PROJECT_DIR wins ------------------------------- + +WINS="$(mkfixture anchor-wins Cargo.toml)" +DECOY="$(mkfixture anchor-decoy package.json)" +anchor_out="$(cd "$DECOY" && CLAUDE_PROJECT_DIR="$WINS" bash "$DETECT" 2>/dev/null)" +assert_equals "CLAUDE_PROJECT_DIR outranks the cwd" "Cargo.toml" "$anchor_out" + +# An EMPTY CLAUDE_PROJECT_DIR is the unset case: the script uses `:-`, so an +# exported-but-blank variable falls through to the git/cwd chain instead of +# anchoring on the filesystem root. +blank_out="$(cd "$DECOY" && env CLAUDE_PROJECT_DIR= GIT_CEILING_DIRECTORIES="$TEST_TMPDIR" bash "$DETECT" 2>/dev/null)" +assert_equals "an empty CLAUDE_PROJECT_DIR is treated as unset" "package.json" "$blank_out" + +# --- 8. Root anchoring: the git toplevel --------------------------------------- + +GITREPO="$TEST_TMPDIR/gitrepo" +mkdir -p "$GITREPO/deep/nested" +git -C "$GITREPO" init -q +: >"$GITREPO/Cargo.toml" +: >"$GITREPO/deep/nested/package.json" +git_out="$(cd "$GITREPO/deep/nested" && env -u CLAUDE_PROJECT_DIR bash "$DETECT" 2>/dev/null)" +assert_equals "without CLAUDE_PROJECT_DIR the git toplevel is the root" "Cargo.toml" "$git_out" +assert_not_contains "the cwd's own marker is not reported from a subdirectory" "$git_out" "package.json" + +# --- 9. Root anchoring: the cwd fallback --------------------------------------- +# GIT_CEILING_DIRECTORIES makes this deterministic: without it the case would +# pass or fail on whether the machine's TMPDIR happens to sit inside a checkout. + +NOGIT="$(mkfixture nogit pyproject.toml)" +nogit_out="$(cd "$NOGIT" && env -u CLAUDE_PROJECT_DIR GIT_CEILING_DIRECTORIES="$TEST_TMPDIR" bash "$DETECT" 2>/dev/null)" +assert_equals "outside a repo and with no override, the cwd is the root" "pyproject.toml" "$nogit_out" + +# --- 10. A CLAUDE_PROJECT_DIR that does not exist ------------------------------- +# The `cd … || true` is load-bearing: under `set -e` a failed cd would abort with +# no output at all, and the skill preamble would show the user nothing. The +# documented degradation is "stay in the cwd and answer from there", quietly. + +GHOST="$(mkfixture ghost-cwd go.mod)" +ghost_exit=0 +ghost_out="$(cd "$GHOST" && CLAUDE_PROJECT_DIR="$TEST_TMPDIR/does-not-exist" bash "$DETECT" 2>/dev/null)" || ghost_exit=$? +assert_equals "a nonexistent CLAUDE_PROJECT_DIR degrades to the cwd" "go.mod" "$ghost_out" +assert_exit "a nonexistent CLAUDE_PROJECT_DIR does not abort" 0 "$ghost_exit" + +# The cd failure must also stay off stderr. The skills redirect stderr away, so +# a leak is not user-visible there, but it is visible to every other caller. +ghost_err="$(cd "$GHOST" && CLAUDE_PROJECT_DIR="$TEST_TMPDIR/does-not-exist" bash "$DETECT" 2>&1 >/dev/null)" +assert_equals "the failed cd prints nothing on stderr" "" "$ghost_err" + +# --- 11. Arguments ------------------------------------------------------------- +# The script reads no arguments, but the skill grants are `…/detect-ecosystems.sh:*` +# and the wrappers forward "$@", so anything a caller appends lands here. It must +# be inert rather than fatal. + +args_exit=0 +args_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" bash "$DETECT" --bogus extra 2>/dev/null)" || args_exit=$? +assert_equals "unread arguments do not change the answer" "$multi_expected" "$args_out" +assert_exit "unread arguments do not fail the script" 0 "$args_exit" + +# --- 12. Output shape ---------------------------------------------------------- +# One marker per line, newline-terminated, nothing else on stdout. A consumer +# pipes this into `head`, so a trailing-newline regression would join the last +# marker to whatever follows. + +raw="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$SLN_ONE" bash "$DETECT" 2>/dev/null | od -c | tr -s ' ')" +assert_contains "output is newline-terminated" "$raw" 'M y A p p . s l n \n' + +line_count="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" bash "$DETECT" 2>/dev/null | wc -l | tr -d ' ')" +assert_equals "six markers print on exactly six lines" "6" "$line_count" + +clean_err="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" bash "$DETECT" 2>&1 >/dev/null)" +assert_equals "a normal run prints nothing on stderr" "" "$clean_err" + +# --- Report -------------------------------------------------------------------- + +printf '\n%d case(s), %d failure(s)\n' "$CASE_NUM" "$FAILED" +[[ $FAILED -eq 0 ]] || exit 1 +echo "All detect-ecosystems.sh checks passed." diff --git a/plugins/prototype/skills/explore-directions/scripts/detect-ecosystems.test.sh b/plugins/prototype/skills/explore-directions/scripts/detect-ecosystems.test.sh new file mode 100755 index 0000000000..8ff6370153 --- /dev/null +++ b/plugins/prototype/skills/explore-directions/scripts/detect-ecosystems.test.sh @@ -0,0 +1,201 @@ +#!/usr/bin/env bash +# Contract tests for this skill's detect-ecosystems.sh WRAPPER. +# +# The wrapper is not a second detector. It is the ${CLAUDE_SKILL_DIR}-addressable +# handle for the one at plugins/prototype/scripts/detect-ecosystems.sh, and it +# exists only because an `allowed-tools` Bash rule can substitute +# ${CLAUDE_SKILL_DIR} but leaves ${CLAUDE_PLUGIN_ROOT} a literal string, which +# makes any grant naming the plugin root inert. So the grant has to name a path +# under the skill's own directory, and this file is that path. +# +# Two ways that arrangement degrades, both of which look harmless in review: +# +# 1. Someone "removes the indirection" by pasting the detector's body in here. +# The skill keeps working, and the two copies then drift apart silently. +# The single-source assertions below fail that edit. +# 2. Someone tidies the header comment away, or swaps the self-locating +# dirname walk for a ${CLAUDE_PLUGIN_ROOT} expansion, which is empty in the +# Bash tool's environment. The grant goes inert and the preamble prints +# nothing. The rationale and resolution assertions below fail that edit. +# +# The detector's own behavior is covered by its sibling suite at the plugin +# root; this suite proves delegation, not detection, plus one end-to-end smoke +# check that the delegation actually reaches a working detector. +# +# SC2016 is disabled file-wide on purpose, for the same reason the sibling +# allowed-tools-pairing suite disables it. Every single-quoted `${…}` below is a +# fixed string searched for VERBATIM in frontmatter or in the wrapper's source +# text. Letting the shell expand one would make the assertion match nothing. +# shellcheck disable=SC2016 +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WRAPPER="$SCRIPT_DIR/detect-ecosystems.sh" +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" +CANONICAL="$PLUGIN_ROOT/scripts/detect-ecosystems.sh" +SIBLING="$PLUGIN_ROOT/skills/pressure-test/scripts/detect-ecosystems.sh" +SKILL_MD="$(cd "$SCRIPT_DIR/.." && pwd)/SKILL.md" +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +FAILED=0 +CASE_NUM=0 + +pass() { + CASE_NUM=$((CASE_NUM + 1)) + printf 'PASS: %s\n' "$1" +} +fail() { + CASE_NUM=$((CASE_NUM + 1)) + FAILED=$((FAILED + 1)) + printf 'FAIL: %s\n expected: %s\n actual: %s\n' "$1" "$2" "$3" >&2 +} +assert_exit() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "exit $2" "exit $3"; fi +} +assert_equals() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "$2" "$3"; fi +} +assert_contains() { + case "$2" in + *"$3"*) pass "$1" ;; + *) fail "$1" "contains: $3" "$2" ;; + esac +} +assert_not_contains() { + case "$2" in + *"$3"*) fail "$1" "absent: $3" "present" ;; + *) pass "$1" ;; + esac +} + +# Executable lines only: the header comment is prose that differs on purpose +# between the two skill copies (each names the OTHER skill as the sibling). +body() { grep -v -e '^[[:space:]]*#' -e '^[[:space:]]*$' "$1"; } + +# --- 1. The file is reachable the way the grant reaches it --------------------- + +if [[ -x "$WRAPPER" ]]; then + pass "wrapper exists and is executable" +else + fail "wrapper exists and is executable" "executable file" "$WRAPPER" +fi + +assert_equals "wrapper sits at the \${CLAUDE_SKILL_DIR}-addressable path" \ + "$PLUGIN_ROOT/skills/explore-directions/scripts/detect-ecosystems.sh" "$WRAPPER" + +# The grant in the skill body is what makes this path load-bearing. If the +# frontmatter ever stops naming it, this file is dead weight rather than a +# handle, and the pairing gate's own failure would be the first hint. +assert_contains "SKILL.md grants this exact skill-relative path" \ + "$(cat "$SKILL_MD")" 'Bash(${CLAUDE_SKILL_DIR}/scripts/detect-ecosystems.sh:*)' + +# --- 2. It delegates rather than duplicates ------------------------------------ + +wrapper_body="$(body "$WRAPPER")" +assert_contains "wrapper hands off with exec" "$wrapper_body" "exec " +assert_contains "wrapper forwards its arguments" "$wrapper_body" '"$@"' +assert_contains "wrapper targets the plugin-root detector" "$wrapper_body" '/scripts/detect-ecosystems.sh' + +# Single-sourcing. These are the detector's own internals; none of them may +# appear here, or there are two detectors to keep in step instead of one. +assert_not_contains "wrapper does not carry the marker list" "$wrapper_body" "pyproject.toml" +assert_not_contains "wrapper does not carry the detector's accumulator" "$wrapper_body" "found+=(" +assert_not_contains "wrapper does not carry the empty answer" "$wrapper_body" "none detected" + +# --- 3. It resolves the canonical script, and resolves it self-locatingly ------ + +assert_contains "wrapper walks up from its own BASH_SOURCE" "$wrapper_body" 'dirname "${BASH_SOURCE[0]}"' +assert_contains "wrapper walks the three levels to the plugin root" "$wrapper_body" '/../../..' + +# ${CLAUDE_PLUGIN_ROOT} is not exported into the Bash tool's environment, so a +# shell expansion of it here yields an empty string and the exec target becomes +# /scripts/detect-ecosystems.sh. The name may appear in the rationale comment; +# it must never appear in an executable line. +assert_not_contains "no live \${CLAUDE_PLUGIN_ROOT} expansion in the body" "$wrapper_body" "CLAUDE_PLUGIN_ROOT" + +if [[ -x "$CANONICAL" ]]; then + pass "the resolved target exists and is executable" +else + fail "the resolved target exists and is executable" "executable file" "$CANONICAL" +fi + +# --- 4. The rationale survives ------------------------------------------------- +# This comment is the only record of WHY a one-line exec file exists. A tidying +# pass that deletes it leaves the next reader with an obvious-looking deletion. + +header="$(grep -e '^[[:space:]]*#' "$WRAPPER")" +assert_contains "rationale names the allowed-tools constraint" "$header" "allowed-tools" +assert_contains "rationale names the token that IS substituted" "$header" 'CLAUDE_SKILL_DIR' +assert_contains "rationale names the token that is NOT substituted" "$header" 'CLAUDE_PLUGIN_ROOT' +assert_contains "rationale records that the detector stays single-sourced" "$header" "single-source" +assert_contains "rationale explains the self-locating form" "$header" "not exported" + +# --- 5. The two skill copies stay in lockstep ----------------------------------- +# Same plugin, same handle, one detector. The prose headers differ by design +# (each names the other skill), so only the executable lines are compared. + +if [[ -f "$SIBLING" ]]; then + if diff <(body "$WRAPPER") <(body "$SIBLING") >/dev/null; then + pass "the sibling skill's wrapper has a byte-identical body" + else + fail "the sibling skill's wrapper has a byte-identical body" \ + "identical executable lines" "$(diff <(body "$WRAPPER") <(body "$SIBLING") | tr '\n' ' ')" + fi +else + fail "the sibling skill's wrapper is present" "$SIBLING" "missing" +fi + +# --- 6. Behavioral smoke: the delegation actually reaches a working detector ---- +# Byte-for-byte against the canonical script on the same fixture, so the wrapper +# cannot pass by execing something that merely also exits 0. + +MULTI="$TEST_TMPDIR/multi" +mkdir -p "$MULTI" +: >"$MULTI/package.json" +: >"$MULTI/go.mod" +: >"$MULTI/App.sln" + +EMPTY="$TEST_TMPDIR/empty" +mkdir -p "$EMPTY" + +for fixture in "$MULTI" "$EMPTY"; do + label="$(basename "$fixture")" + + wrap_exit=0 + wrap_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$fixture" bash "$WRAPPER" 2>/dev/null)" || wrap_exit=$? + canon_exit=0 + canon_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$fixture" bash "$CANONICAL" 2>/dev/null)" || canon_exit=$? + + assert_equals "$label: wrapper output matches the canonical detector" "$canon_out" "$wrap_out" + assert_exit "$label: wrapper exit status matches the canonical detector" "$canon_exit" "$wrap_exit" +done + +assert_equals "the multi fixture is a non-trivial comparison" \ + "$(printf '%s\n' App.sln package.json go.mod)" \ + "$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" bash "$WRAPPER" 2>/dev/null)" + +# Self-locating means the cwd is irrelevant. `/` is the harshest cwd available +# and is the one a preamble can genuinely land in. +root_out="$(cd / && CLAUDE_PROJECT_DIR="$MULTI" bash "$WRAPPER" 2>/dev/null)" +assert_equals "wrapper resolves its target from an unrelated cwd" \ + "$(printf '%s\n' App.sln package.json go.mod)" "$root_out" + +# Direct invocation, no `bash` interpreter prefix: this is the form the paired +# grant permits, and the only one the skill body is allowed to use. +direct_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" "$WRAPPER" 2>/dev/null)" +assert_equals "direct (non-interpreter-led) invocation works" \ + "$(printf '%s\n' App.sln package.json go.mod)" "$direct_out" + +# Forwarded arguments reach a script that reads none. Inert, never fatal. +args_exit=0 +args_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" bash "$WRAPPER" --bogus extra 2>/dev/null)" || args_exit=$? +assert_equals "forwarded arguments do not change the answer" \ + "$(printf '%s\n' App.sln package.json go.mod)" "$args_out" +assert_exit "forwarded arguments do not fail the wrapper" 0 "$args_exit" + +# --- Report --------------------------------------------------------------------- + +printf '\n%d case(s), %d failure(s)\n' "$CASE_NUM" "$FAILED" +[[ $FAILED -eq 0 ]] || exit 1 +echo "All explore-directions detect-ecosystems.sh wrapper checks passed." diff --git a/plugins/prototype/skills/pressure-test/scripts/detect-ecosystems.test.sh b/plugins/prototype/skills/pressure-test/scripts/detect-ecosystems.test.sh new file mode 100755 index 0000000000..662078a6ee --- /dev/null +++ b/plugins/prototype/skills/pressure-test/scripts/detect-ecosystems.test.sh @@ -0,0 +1,201 @@ +#!/usr/bin/env bash +# Contract tests for this skill's detect-ecosystems.sh WRAPPER. +# +# The wrapper is not a second detector. It is the ${CLAUDE_SKILL_DIR}-addressable +# handle for the one at plugins/prototype/scripts/detect-ecosystems.sh, and it +# exists only because an `allowed-tools` Bash rule can substitute +# ${CLAUDE_SKILL_DIR} but leaves ${CLAUDE_PLUGIN_ROOT} a literal string, which +# makes any grant naming the plugin root inert. So the grant has to name a path +# under the skill's own directory, and this file is that path. +# +# Two ways that arrangement degrades, both of which look harmless in review: +# +# 1. Someone "removes the indirection" by pasting the detector's body in here. +# The skill keeps working, and the two copies then drift apart silently. +# The single-source assertions below fail that edit. +# 2. Someone tidies the header comment away, or swaps the self-locating +# dirname walk for a ${CLAUDE_PLUGIN_ROOT} expansion, which is empty in the +# Bash tool's environment. The grant goes inert and the preamble prints +# nothing. The rationale and resolution assertions below fail that edit. +# +# The detector's own behavior is covered by its sibling suite at the plugin +# root; this suite proves delegation, not detection, plus one end-to-end smoke +# check that the delegation actually reaches a working detector. +# +# SC2016 is disabled file-wide on purpose, for the same reason the sibling +# allowed-tools-pairing suite disables it. Every single-quoted `${…}` below is a +# fixed string searched for VERBATIM in frontmatter or in the wrapper's source +# text. Letting the shell expand one would make the assertion match nothing. +# shellcheck disable=SC2016 +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WRAPPER="$SCRIPT_DIR/detect-ecosystems.sh" +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" +CANONICAL="$PLUGIN_ROOT/scripts/detect-ecosystems.sh" +SIBLING="$PLUGIN_ROOT/skills/explore-directions/scripts/detect-ecosystems.sh" +SKILL_MD="$(cd "$SCRIPT_DIR/.." && pwd)/SKILL.md" +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +FAILED=0 +CASE_NUM=0 + +pass() { + CASE_NUM=$((CASE_NUM + 1)) + printf 'PASS: %s\n' "$1" +} +fail() { + CASE_NUM=$((CASE_NUM + 1)) + FAILED=$((FAILED + 1)) + printf 'FAIL: %s\n expected: %s\n actual: %s\n' "$1" "$2" "$3" >&2 +} +assert_exit() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "exit $2" "exit $3"; fi +} +assert_equals() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "$2" "$3"; fi +} +assert_contains() { + case "$2" in + *"$3"*) pass "$1" ;; + *) fail "$1" "contains: $3" "$2" ;; + esac +} +assert_not_contains() { + case "$2" in + *"$3"*) fail "$1" "absent: $3" "present" ;; + *) pass "$1" ;; + esac +} + +# Executable lines only: the header comment is prose that differs on purpose +# between the two skill copies (each names the OTHER skill as the sibling). +body() { grep -v -e '^[[:space:]]*#' -e '^[[:space:]]*$' "$1"; } + +# --- 1. The file is reachable the way the grant reaches it --------------------- + +if [[ -x "$WRAPPER" ]]; then + pass "wrapper exists and is executable" +else + fail "wrapper exists and is executable" "executable file" "$WRAPPER" +fi + +assert_equals "wrapper sits at the \${CLAUDE_SKILL_DIR}-addressable path" \ + "$PLUGIN_ROOT/skills/pressure-test/scripts/detect-ecosystems.sh" "$WRAPPER" + +# The grant in the skill body is what makes this path load-bearing. If the +# frontmatter ever stops naming it, this file is dead weight rather than a +# handle, and the pairing gate's own failure would be the first hint. +assert_contains "SKILL.md grants this exact skill-relative path" \ + "$(cat "$SKILL_MD")" 'Bash(${CLAUDE_SKILL_DIR}/scripts/detect-ecosystems.sh:*)' + +# --- 2. It delegates rather than duplicates ------------------------------------ + +wrapper_body="$(body "$WRAPPER")" +assert_contains "wrapper hands off with exec" "$wrapper_body" "exec " +assert_contains "wrapper forwards its arguments" "$wrapper_body" '"$@"' +assert_contains "wrapper targets the plugin-root detector" "$wrapper_body" '/scripts/detect-ecosystems.sh' + +# Single-sourcing. These are the detector's own internals; none of them may +# appear here, or there are two detectors to keep in step instead of one. +assert_not_contains "wrapper does not carry the marker list" "$wrapper_body" "pyproject.toml" +assert_not_contains "wrapper does not carry the detector's accumulator" "$wrapper_body" "found+=(" +assert_not_contains "wrapper does not carry the empty answer" "$wrapper_body" "none detected" + +# --- 3. It resolves the canonical script, and resolves it self-locatingly ------ + +assert_contains "wrapper walks up from its own BASH_SOURCE" "$wrapper_body" 'dirname "${BASH_SOURCE[0]}"' +assert_contains "wrapper walks the three levels to the plugin root" "$wrapper_body" '/../../..' + +# ${CLAUDE_PLUGIN_ROOT} is not exported into the Bash tool's environment, so a +# shell expansion of it here yields an empty string and the exec target becomes +# /scripts/detect-ecosystems.sh. The name may appear in the rationale comment; +# it must never appear in an executable line. +assert_not_contains "no live \${CLAUDE_PLUGIN_ROOT} expansion in the body" "$wrapper_body" "CLAUDE_PLUGIN_ROOT" + +if [[ -x "$CANONICAL" ]]; then + pass "the resolved target exists and is executable" +else + fail "the resolved target exists and is executable" "executable file" "$CANONICAL" +fi + +# --- 4. The rationale survives ------------------------------------------------- +# This comment is the only record of WHY a one-line exec file exists. A tidying +# pass that deletes it leaves the next reader with an obvious-looking deletion. + +header="$(grep -e '^[[:space:]]*#' "$WRAPPER")" +assert_contains "rationale names the allowed-tools constraint" "$header" "allowed-tools" +assert_contains "rationale names the token that IS substituted" "$header" 'CLAUDE_SKILL_DIR' +assert_contains "rationale names the token that is NOT substituted" "$header" 'CLAUDE_PLUGIN_ROOT' +assert_contains "rationale records that the detector stays single-sourced" "$header" "single-source" +assert_contains "rationale explains the self-locating form" "$header" "not exported" + +# --- 5. The two skill copies stay in lockstep ----------------------------------- +# Same plugin, same handle, one detector. The prose headers differ by design +# (each names the other skill), so only the executable lines are compared. + +if [[ -f "$SIBLING" ]]; then + if diff <(body "$WRAPPER") <(body "$SIBLING") >/dev/null; then + pass "the sibling skill's wrapper has a byte-identical body" + else + fail "the sibling skill's wrapper has a byte-identical body" \ + "identical executable lines" "$(diff <(body "$WRAPPER") <(body "$SIBLING") | tr '\n' ' ')" + fi +else + fail "the sibling skill's wrapper is present" "$SIBLING" "missing" +fi + +# --- 6. Behavioral smoke: the delegation actually reaches a working detector ---- +# Byte-for-byte against the canonical script on the same fixture, so the wrapper +# cannot pass by execing something that merely also exits 0. + +MULTI="$TEST_TMPDIR/multi" +mkdir -p "$MULTI" +: >"$MULTI/package.json" +: >"$MULTI/go.mod" +: >"$MULTI/App.sln" + +EMPTY="$TEST_TMPDIR/empty" +mkdir -p "$EMPTY" + +for fixture in "$MULTI" "$EMPTY"; do + label="$(basename "$fixture")" + + wrap_exit=0 + wrap_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$fixture" bash "$WRAPPER" 2>/dev/null)" || wrap_exit=$? + canon_exit=0 + canon_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$fixture" bash "$CANONICAL" 2>/dev/null)" || canon_exit=$? + + assert_equals "$label: wrapper output matches the canonical detector" "$canon_out" "$wrap_out" + assert_exit "$label: wrapper exit status matches the canonical detector" "$canon_exit" "$wrap_exit" +done + +assert_equals "the multi fixture is a non-trivial comparison" \ + "$(printf '%s\n' App.sln package.json go.mod)" \ + "$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" bash "$WRAPPER" 2>/dev/null)" + +# Self-locating means the cwd is irrelevant. `/` is the harshest cwd available +# and is the one a preamble can genuinely land in. +root_out="$(cd / && CLAUDE_PROJECT_DIR="$MULTI" bash "$WRAPPER" 2>/dev/null)" +assert_equals "wrapper resolves its target from an unrelated cwd" \ + "$(printf '%s\n' App.sln package.json go.mod)" "$root_out" + +# Direct invocation, no `bash` interpreter prefix: this is the form the paired +# grant permits, and the only one the skill body is allowed to use. +direct_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" "$WRAPPER" 2>/dev/null)" +assert_equals "direct (non-interpreter-led) invocation works" \ + "$(printf '%s\n' App.sln package.json go.mod)" "$direct_out" + +# Forwarded arguments reach a script that reads none. Inert, never fatal. +args_exit=0 +args_out="$(cd "$TEST_TMPDIR" && CLAUDE_PROJECT_DIR="$MULTI" bash "$WRAPPER" --bogus extra 2>/dev/null)" || args_exit=$? +assert_equals "forwarded arguments do not change the answer" \ + "$(printf '%s\n' App.sln package.json go.mod)" "$args_out" +assert_exit "forwarded arguments do not fail the wrapper" 0 "$args_exit" + +# --- Report --------------------------------------------------------------------- + +printf '\n%d case(s), %d failure(s)\n' "$CASE_NUM" "$FAILED" +[[ $FAILED -eq 0 ]] || exit 1 +echo "All pressure-test detect-ecosystems.sh wrapper checks passed." diff --git a/plugins/songwriting/.claude-plugin/plugin.json b/plugins/songwriting/.claude-plugin/plugin.json index 050e913174..242197f984 100644 --- a/plugins/songwriting/.claude-plugin/plugin.json +++ b/plugins/songwriting/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "songwriting", - "version": "1.4.16", + "version": "1.4.17", "description": "Songwriting craft companion — nine concern-scoped lyric-craft skills (workflow router, rhyme, object-writing, metaphor, meter-prosody, song-form, co-write, diagnose, practice) applying Pat Pattison's methods, with an object-writing agent that performs the sensory exercise itself and per-skill emission boundaries that route generation to the skill that owns it, plus Suno v5.5 prompt engineering (style prompts, tagged lyrics, genre templates, troubleshooting).", "author": { "name": "Melodic Software", diff --git a/plugins/songwriting/CHANGELOG.md b/plugins/songwriting/CHANGELOG.md index bf2c05d0f7..d08ca3b071 100644 --- a/plugins/songwriting/CHANGELOG.md +++ b/plugins/songwriting/CHANGELOG.md @@ -3,6 +3,28 @@ All notable changes to the `songwriting` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [1.4.17] + +### Added + +- **`datamuse.sh` has an offline contract suite.** The Datamuse helper that `rhyme` operates is the + plugin's only executable script, and nothing covered it: `scripts/affected-tests.sh` mapped it to + zero suites, which this repo's validation contract calls an error rather than "nothing to run". + The co-located `datamuse.test.sh` closes that gap with 93 cases and replaces `curl` with a + PATH-stub that records the whole request argv and serves a canned body, so the suite never reaches + api.datamuse.com and is deterministic with no network. It covers argument validation (no mode, no + word, unknown mode) exiting 1 with the usage banner and no request issued, response parsing into + the four-field TSV with tags comma-joined and missing fields defaulted rather than emitted as + `null`, the preserved API result order, the mode-to-relation table where a typo would silently + return the wrong relation, the `LIMIT` default and override plus the `max=5` pin on `syllables`, + a multi-word argument reaching the query `+`-joined, empty results as exit 0, a nonzero `curl` + exit propagating through the pipe with no rows on stdout, malformed JSON failing loudly instead + of emitting partial TSV, and `family` merging near rhymes with consonance across two requests + that each carry `md=s`, deduped by word rather than by whole object and re-sorted by score over + both sources. Twenty-three seeded mutations of `datamuse.sh`, across the jq filter, the query + table, the curl flags, the limits, the family merge and the usage banner, were each caught by the + suite. + ## [1.4.16] ### Changed diff --git a/plugins/songwriting/context/pat-pattison/scripts/datamuse.test.sh b/plugins/songwriting/context/pat-pattison/scripts/datamuse.test.sh new file mode 100755 index 0000000000..e4eb50ad81 --- /dev/null +++ b/plugins/songwriting/context/pat-pattison/scripts/datamuse.test.sh @@ -0,0 +1,378 @@ +#!/usr/bin/env bash +# Contract tests for datamuse.sh. Fully offline: `curl` is replaced by a +# PATH-stub that records the full request argv and serves a canned body, so +# nothing here reaches api.datamuse.com and the suite is deterministic on a +# laptop with no network. +# +# Coverage: +# - argument validation (no mode, no word, unknown mode) exits 1, prints the +# usage banner, and issues NO request +# - the usage banner still spans the modes and the LIMIT override, so +# truncating the `sed` range that prints it is caught +# - curl is invoked with the flags the script documents (-sS), since silent +# mode without -S swallows the transport error this suite relies on +# - happy-path response parsing: one TSV row per result, four tab-separated +# fields, tags joined with commas, missing score/numSyllables defaulting to +# 0 and missing tags to an empty field +# - API result order is preserved (the non-family modes do not re-sort) +# - the mode -> query-parameter table (rel_rhy, rel_nry, rel_cns, rel_syn, +# rel_ant, rel_trg, rel_jja, rel_jjb, ml, sl, sp), which is where a typo +# silently returns the wrong relation +# - LIMIT default 25, LIMIT override, and `syllables` pinning max=5 +# - a multi-word argument reaching the query as '+'-joined +# - empty results are exit 0 with no output, never an error +# - a nonzero curl exit propagates through the pipe (pipefail) with no rows +# on stdout +# - malformed JSON fails loudly instead of emitting partial TSV +# - `family` merges near-rhyme and consonance: two requests, EACH carrying +# md=s, deduped by word rather than by whole object, re-sorted by score +# across BOTH sources +# +# Prerequisite: jq, which datamuse.sh itself requires. Absent, this suite fails +# loudly rather than skipping, because every parsing assertion below would be +# vacuous without it. +# +# Self-contained assertion helpers, per +# docs/conventions/shell-test-helpers/README.md: per-plugin duplication of this +# shape is the accepted default, not an opt-in. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$SCRIPT_DIR/datamuse.sh" + +# The suite drives LIMIT explicitly per case; an inherited one would silently +# rewrite the default-limit assertions. +unset LIMIT + +FAILED=0 +CASE_NUM=0 +pass() { + CASE_NUM=$((CASE_NUM + 1)) + printf 'PASS: [%d] %s\n' "$CASE_NUM" "$1" +} +fail() { + CASE_NUM=$((CASE_NUM + 1)) + printf 'FAIL: [%d] %s - expected %q got %q\n' "$CASE_NUM" "$1" "$2" "$3" >&2 + FAILED=$((FAILED + 1)) +} +assert_eq() { if [[ "$3" == "$2" ]]; then pass "$1"; else fail "$1" "$2" "$3"; fi; } +assert_contains() { if [[ "$2" == *"$3"* ]]; then pass "$1"; else fail "$1" "contains: $3" "$2"; fi; } +assert_not_contains() { if [[ "$2" != *"$3"* ]]; then pass "$1"; else fail "$1" "absent: $3" "$2"; fi; } +assert_nonzero() { if [[ "$2" -ne 0 ]]; then pass "$1"; else fail "$1" "nonzero exit" "exit $2"; fi; } + +if [[ ! -f "$SCRIPT" ]]; then + printf 'FAIL: datamuse.sh not found at %s\n' "$SCRIPT" >&2 + exit 1 +fi +if ! command -v jq >/dev/null 2>&1; then + printf 'FAIL: jq is required to test datamuse.sh (the script itself depends on it)\n' >&2 + exit 1 +fi + +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +STUB_BIN="$TEST_TMPDIR/stub-bin" +STUB_DATA="$TEST_TMPDIR/stub-data" +REQUEST_LOG="$STUB_DATA/requests.log" +STDOUT_FILE="$TEST_TMPDIR/stdout" +STDERR_FILE="$TEST_TMPDIR/stderr" +mkdir -p "$STUB_BIN" + +# --- curl stub ------------------------------------------------------------- +# Logs one line per invocation carrying the WHOLE argv, flags included, so the +# suite can pin the flags as well as the URL; then serves a per-relation canned +# body. Selecting the body by relation (rather than by call order) is what lets +# `family`'s two calls return different payloads. +cat >"$STUB_BIN/curl" <<'STUB' +#!/usr/bin/env bash +url="" +for arg in "$@"; do + case "$arg" in + http*) url="$arg" ;; + *) ;; + esac +done +printf '%s\n' "$*" >>"$DATAMUSE_STUB_DATA/requests.log" +if [[ "${DATAMUSE_STUB_EXIT:-0}" -ne 0 ]]; then + printf 'curl: (%s) stubbed transport failure\n' "$DATAMUSE_STUB_EXIT" >&2 + exit "$DATAMUSE_STUB_EXIT" +fi +body="$DATAMUSE_STUB_DATA/default.json" +case "$url" in + *rel_nry=*) + if [[ -f "$DATAMUSE_STUB_DATA/near.json" ]]; then body="$DATAMUSE_STUB_DATA/near.json"; fi + ;; + *rel_cns=*) + if [[ -f "$DATAMUSE_STUB_DATA/cons.json" ]]; then body="$DATAMUSE_STUB_DATA/cons.json"; fi + ;; + *) ;; +esac +cat "$body" +STUB +chmod +x "$STUB_BIN/curl" + +if ! PATH="$STUB_BIN:$PATH" command -v curl | grep -q "^$STUB_BIN/curl$"; then + printf 'FAIL: the curl stub does not shadow the real curl; the suite would hit the network\n' >&2 + exit 1 +fi + +STUB_EXIT=0 + +# reset_stub [default-body-json] - fresh request log and a fresh body set. +reset_stub() { + rm -rf "$STUB_DATA" + mkdir -p "$STUB_DATA" + : >"$REQUEST_LOG" + STUB_EXIT=0 + printf '%s' "${1:-[]}" >"$STUB_DATA/default.json" +} + +# set_body +set_body() { printf '%s' "$2" >"$STUB_DATA/$1.json"; } + +RC=0 +OUT="" +ERR="" + +# run_datamuse - runs the script against the stub with LIMIT unset. +run_datamuse() { + RC=0 + PATH="$STUB_BIN:$PATH" DATAMUSE_STUB_DATA="$STUB_DATA" DATAMUSE_STUB_EXIT="$STUB_EXIT" \ + bash "$SCRIPT" "$@" >"$STDOUT_FILE" 2>"$STDERR_FILE" || RC=$? + OUT="$(cat "$STDOUT_FILE")" + ERR="$(cat "$STDERR_FILE")" +} + +# run_datamuse_limit - same, with LIMIT exported. +run_datamuse_limit() { + local limit="$1" + shift + RC=0 + PATH="$STUB_BIN:$PATH" DATAMUSE_STUB_DATA="$STUB_DATA" DATAMUSE_STUB_EXIT="$STUB_EXIT" \ + LIMIT="$limit" bash "$SCRIPT" "$@" >"$STDOUT_FILE" 2>"$STDERR_FILE" || RC=$? + OUT="$(cat "$STDOUT_FILE")" + ERR="$(cat "$STDERR_FILE")" +} + +requests() { cat "$REQUEST_LOG"; } +request_count() { grep -c . "$REQUEST_LOG"; } +# How many logged requests carry individually. A substring check +# over the whole log is satisfied by ONE matching request, which is the wrong +# question when the script issues several. +requests_matching() { grep -c -F -e "$1" "$REQUEST_LOG"; } +row() { sed -n "${1}p" "$STDOUT_FILE"; } +row_count() { grep -c . "$STDOUT_FILE"; } +# Number of rows whose tab-separated field count is not 4. +malformed_rows() { awk -F'\t' 'NF != 4 { n++ } END { print n + 0 }' "$STDOUT_FILE"; } + +TAB=$'\t' + +# --- argument validation: no request may be issued -------------------------- + +reset_stub +run_datamuse +assert_eq "no arguments -> exit 1" "1" "$RC" +assert_contains "no arguments prints the usage banner" "$ERR" "datamuse.sh rhyme" +assert_contains "usage banner names the API" "$ERR" "https://www.datamuse.com/api/" +# The banner is a line RANGE out of the script's own header, so it is pinned at +# both ends: `family` is the last mode listed and the LIMIT note sits below it. +# Narrowing the range drops one or both. +assert_contains "usage banner reaches the last mode listed" "$ERR" "datamuse.sh family" +assert_contains "usage banner documents the LIMIT override" "$ERR" "LIMIT=" +assert_eq "no arguments issues no request" "0" "$(request_count)" +assert_eq "no arguments writes nothing to stdout" "" "$OUT" + +reset_stub +run_datamuse rhyme +assert_eq "mode without a word -> exit 1" "1" "$RC" +assert_contains "mode without a word prints the usage banner" "$ERR" "datamuse.sh rhyme" +assert_eq "mode without a word issues no request" "0" "$(request_count)" + +reset_stub +run_datamuse rhyme "" +assert_eq "mode with an empty word -> exit 1" "1" "$RC" +assert_eq "empty word issues no request" "0" "$(request_count)" + +reset_stub +run_datamuse bogus lonely +assert_eq "unknown mode -> exit 1" "1" "$RC" +assert_contains "unknown mode names the mode" "$ERR" "unknown mode: bogus" +assert_contains "unknown mode also prints the usage banner" "$ERR" "datamuse.sh rhyme" +assert_eq "unknown mode issues no request" "0" "$(request_count)" + +# --- happy path: response parsing and TSV shape ----------------------------- + +# Realistic md=s payload, plus one sparse entry: Datamuse omits `tags` for some +# results, and the script's own contract is to default the numeric fields to 0 +# and the tag field to empty rather than to emit `null`. +RHYME_JSON='[ + {"word":"only","score":1097,"numSyllables":2,"tags":["adv","adj"]}, + {"word":"solely","score":1074,"numSyllables":2,"tags":["adv"]}, + {"word":"slowly"} +]' + +reset_stub "$RHYME_JSON" +run_datamuse rhyme lonely +assert_eq "rhyme -> exit 0" "0" "$RC" +assert_eq "one row per result" "3" "$(row_count)" +assert_eq "every row has four tab-separated fields" "0" "$(malformed_rows)" +assert_eq "row 1 carries word, score, syllables, comma-joined tags" \ + "only${TAB}1097${TAB}2${TAB}adv,adj" "$(row 1)" +assert_eq "row 2 keeps a single tag unjoined" \ + "solely${TAB}1074${TAB}2${TAB}adv" "$(row 2)" +assert_eq "a sparse entry defaults score and syllables to 0 and tags to empty" \ + "slowly${TAB}0${TAB}0${TAB}" "$(row 3)" +assert_not_contains "no JSON null leaks into the TSV" "$OUT" "null" +assert_eq "one request per invocation" "1" "$(request_count)" +# -sS, not -s: silent WITHOUT show-error swallows the transport diagnostic the +# caller needs, and this suite asserts that diagnostic further down. +assert_eq "curl is invoked with -sS" "1" "$(requests_matching '-sS')" +assert_contains "the rhyme relation is requested" "$(requests)" "rel_rhy=lonely" +assert_contains "metadata flag md=s is requested" "$(requests)" "md=s" +assert_contains "the default limit is 25" "$(requests)" "max=25" +assert_contains "the request targets the Datamuse words endpoint" \ + "$(requests)" "https://api.datamuse.com/words?" + +# The API returns its own ranking; the non-family modes must not re-sort it. +# A score-ascending payload makes a stray sort_by visible. +reset_stub '[{"word":"beta","score":10,"numSyllables":2,"tags":["n"]},{"word":"alpha","score":900,"numSyllables":2,"tags":["n"]}]' +run_datamuse near lonely +assert_eq "API order is preserved, not re-sorted" "beta" "$(row 1 | cut -f1)" +assert_eq "and the second row is the higher-scoring one" "alpha" "$(row 2 | cut -f1)" + +# --- mode -> query-parameter table ------------------------------------------ + +MODE_QUERIES=( + "rhyme rel_rhy" + "near rel_nry" + "cons rel_cns" + "syn rel_syn" + "ant rel_ant" + "trg rel_trg" + "jja rel_jja" + "jjb rel_jjb" + "means ml" + "sounds sl" + "pattern sp" +) +for pair in "${MODE_QUERIES[@]}"; do + read -r mode key <<<"$pair" + reset_stub "$RHYME_JSON" + run_datamuse "$mode" winter + assert_eq "$mode -> exit 0" "0" "$RC" + assert_contains "$mode requests $key" "$(requests)" "$key=winter" +done + +# --- limits ----------------------------------------------------------------- + +reset_stub "$RHYME_JSON" +run_datamuse_limit 50 near grief +assert_eq "LIMIT override -> exit 0" "0" "$RC" +assert_contains "LIMIT=50 reaches the query" "$(requests)" "max=50" +assert_not_contains "and the default is not also sent" "$(requests)" "max=25" + +reset_stub "$RHYME_JSON" +run_datamuse syllables disappointment +assert_contains "syllables pins max=5" "$(requests)" "max=5" +assert_contains "syllables searches by letter pattern" "$(requests)" "sp=disappointment" + +reset_stub "$RHYME_JSON" +run_datamuse_limit 50 syllables disappointment +assert_contains "syllables pins max=5 even under a LIMIT override" "$(requests)" "max=5" + +# --- multi-word arguments --------------------------------------------------- + +reset_stub "$RHYME_JSON" +run_datamuse means cold winter morning +assert_eq "multi-word argument -> exit 0" "0" "$RC" +assert_contains "spaces are joined with '+' in the query" "$(requests)" "ml=cold+winter+morning" +assert_not_contains "no raw space survives into the URL" "$(requests)" "cold winter" + +# --- empty results are not an error ----------------------------------------- + +reset_stub '[]' +run_datamuse rhyme zzzzz +assert_eq "empty result set -> exit 0" "0" "$RC" +assert_eq "empty result set writes no rows" "0" "$(row_count)" +assert_eq "empty result set writes nothing to stdout" "" "$OUT" + +# --- transport failure ------------------------------------------------------ + +reset_stub "$RHYME_JSON" +STUB_EXIT=6 +run_datamuse rhyme lonely +assert_eq "a nonzero curl exit propagates through the pipe" "6" "$RC" +assert_eq "and no rows reach stdout" "0" "$(row_count)" +assert_contains "the transport error reaches stderr" "$ERR" "stubbed transport failure" + +reset_stub +STUB_EXIT=7 +set_body near '[]' +set_body cons '[]' +run_datamuse family stranger +assert_nonzero "family fails when its first request fails" "$RC" +assert_eq "and family emits no rows" "0" "$(row_count)" + +# --- malformed JSON --------------------------------------------------------- + +reset_stub 'not json at all' +run_datamuse rhyme lonely +assert_nonzero "malformed JSON fails loudly" "$RC" +assert_eq "malformed JSON emits no partial TSV" "0" "$(row_count)" +assert_contains "the parser error reaches stderr" "$ERR" "jq" + +reset_stub '{"word":"only","score":1097}' +run_datamuse rhyme lonely +assert_nonzero "a JSON object where an array is expected fails" "$RC" +assert_eq "and emits no rows" "0" "$(row_count)" + +# --- family: merge, dedupe, re-sort ----------------------------------------- + +# The duplicate word carries a DIFFERENT score and tag set in each payload, so +# the two entries are distinct objects: deduping on the whole object would keep +# both and yield four rows, which is what separates unique_by(.word) from a +# plain unique. Interleaved scores are what proves the merge re-sorts across +# BOTH sources instead of concatenating them. +reset_stub +set_body near '[ + {"word":"stranger","score":900,"numSyllables":2,"tags":["n"]}, + {"word":"danger","score":500,"numSyllables":2,"tags":["n"]} +]' +set_body cons '[ + {"word":"stranger","score":850,"numSyllables":2,"tags":["n","adj"]}, + {"word":"stringer","score":700,"numSyllables":2,"tags":["n"]} +]' +run_datamuse family stranger +assert_eq "family -> exit 0" "0" "$RC" +assert_eq "family issues exactly two requests" "2" "$(request_count)" +assert_contains "family requests near rhymes" "$(requests)" "rel_nry=stranger" +assert_contains "family requests consonance" "$(requests)" "rel_cns=stranger" +assert_eq "EACH family request sends md=s" "2" "$(requests_matching 'md=s')" +assert_eq "family dedupes by word, not by whole object" "3" "$(row_count)" +assert_eq "the surviving duplicate is the near-rhyme copy" "900" "$(row 1 | cut -f2)" +assert_eq "family rows keep the four-field shape" "0" "$(malformed_rows)" +assert_eq "family sorts by score across both sources (1)" "stranger" "$(row 1 | cut -f1)" +assert_eq "family sorts by score across both sources (2)" "stringer" "$(row 2 | cut -f1)" +assert_eq "family sorts by score across both sources (3)" "danger" "$(row 3 | cut -f1)" +assert_eq "the deduped row keeps its tags" "n" "$(row 1 | cut -f4)" + +reset_stub +set_body near '[]' +set_body cons '[]' +run_datamuse family nothingrhymeswiththis +assert_eq "family with two empty result sets -> exit 0" "0" "$RC" +assert_eq "family with two empty result sets writes no rows" "0" "$(row_count)" + +reset_stub +set_body near '[{"word":"danger","score":500,"numSyllables":2,"tags":["n"]}]' +set_body cons '[]' +run_datamuse_limit 40 family stranger +assert_eq "family under a LIMIT override -> exit 0" "0" "$RC" +assert_contains "family passes LIMIT to the near-rhyme request" "$(requests)" "rel_nry=stranger&md=s&max=40" +assert_contains "family passes LIMIT to the consonance request" "$(requests)" "rel_cns=stranger&md=s&max=40" +assert_eq "one side empty still yields the other side's rows" "1" "$(row_count)" + +[[ $FAILED -eq 0 ]] || exit 1 +echo "All cases passed ($CASE_NUM)." diff --git a/plugins/wizard/.claude-plugin/plugin.json b/plugins/wizard/.claude-plugin/plugin.json index 6be1779750..77aed25c51 100644 --- a/plugins/wizard/.claude-plugin/plugin.json +++ b/plugins/wizard/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "wizard", - "version": "0.2.4", + "version": "0.2.5", "description": "Generate an interactive bash wizard that walks a human, step by step, through the manual procedures an agent cannot perform — provisioning infrastructure or credentials, setting CI secrets, clicking through third-party dashboards, one-off migrations and cutovers. One skill, generate (/wizard:generate): the agent scopes the stages from the repo (reading key NAMES only from a live .env, never values), authors them onto a fixed hardened library (TTY-only fail-closed prompts, https-only URL opening, hidden secret entry, single-quoted 0600 .env upserts with a gitignore check, repo-confirmed gh secret/variable writes over stdin, names-only summary), prints the full STAGES block for explicit human approval BEFORE the script is made executable, and never runs the wizard itself — the human does, in their own terminal. Ephemeral by default: built for one run, committed only when the setup path should live in the repo. The generated script requires bash (Windows: Git Bash or WSL); gh is optional — CI-secret stages degrade to a visible warning plus a closing-summary entry when it is absent.", "author": { "name": "Melodic Software", diff --git a/plugins/wizard/CHANGELOG.md b/plugins/wizard/CHANGELOG.md index 79eb51eb71..7275efad81 100644 --- a/plugins/wizard/CHANGELOG.md +++ b/plugins/wizard/CHANGELOG.md @@ -3,6 +3,24 @@ All notable changes to the `wizard` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.2.5] + +### Added + +- **`template.sh` gets a contract suite.** `plugins/wizard/skills/generate/template.test.sh`, 144 + assertions over the hardened wizard library: fail-closed prompts at EOF, the `_drain_tty` paste + defense (with a control case proving the fixture really does carry a bypass payload), key-name + validation, `.env` upsert and quote round-tripping through a real shell read, owner-only file + mode, the once-only gitignore warning, https-only `open_url`, `gh` secret and variable values + travelling over stdin and never argv, resolve-the-repo-once, every `gh` degradation path, the + names-only closing summary, and the `_cleanup` if-form that keeps a clean run exiting 0 under + `set -e`. The template is a runnable wizard, so each case extracts the library half above the + `STAGES` marker and rewrites its one `exec 3&2 +} +assert_eq() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "$2" "$3"; fi +} +assert_exit() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "exit $2" "exit $3"; fi +} +assert_contains() { + case "$2" in + *"$3"*) pass "$1" ;; + *) fail "$1" "contains: $3" "$2" ;; + esac +} +assert_not_contains() { + case "$2" in + *"$3"*) fail "$1" "absent: $3" "present in: $2" ;; + *) pass "$1" ;; + esac +} +# skip_case . Skip one optional case without exiting. Named to match the +# house helper so scripts/check-discriminating-test-skips.sh can see the branch. +# Only ever used where another case already carries the discriminating proof. +skip_case() { + SKIPPED_N=$((SKIPPED_N + 1)) + printf 'SKIP: %s\n' "$1" >&2 +} + +if [[ ! -f "$TEMPLATE" ]]; then + printf 'FAIL: template.sh not found at %s\n' "$TEMPLATE" >&2 + exit 1 +fi + +# --- The library extraction and its fd-3 seam ------------------------------- + +LIB="$TEST_TMPDIR/wizard-lib.sh" +awk '/^# STAGES/ { exit } { print }' "$TEMPLATE" | + sed 's|exec 3"$LIB" + +STUB_BIN="$TEST_TMPDIR/bin" +mkdir -p "$STUB_BIN" +export STUB_BIN + +# gh stub. Records every invocation's argv and, for a write, its stdin, so a case +# can prove a secret value travelled over stdin and never over the command line. +cat >"$STUB_BIN/gh" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"$GH_CAPTURE/argv" +case "${1:-}" in +auth) + exit "${GH_STUB_AUTH_RC:-0}" + ;; +repo) + if [[ "${GH_STUB_REPO_RC:-0}" -ne 0 ]]; then + printf 'stub: no repo here\n' >&2 + exit "${GH_STUB_REPO_RC}" + fi + printf '%s\n' "${GH_STUB_REPO:-acme/widgets}" + ;; +secret | variable) + cat >>"$GH_CAPTURE/stdin" + if [[ "${GH_STUB_SET_RC:-0}" -ne 0 ]]; then + printf 'stub: gh %s set refused\n' "$1" >&2 + exit "${GH_STUB_SET_RC}" + fi + ;; +*) + exit 127 + ;; +esac +STUB +chmod +x "$STUB_BIN/gh" + +# Browser-opener stub. open_url tries wslview first, so stubbing that name makes +# the dispatch deterministic on any host, and the capture file records exactly +# what was handed to it (nothing, for a refused URL). +cat >"$STUB_BIN/wslview" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "$1" >>"$OPEN_CAPTURE" +STUB +chmod +x "$STUB_BIN/wslview" + +# case_build [strict]. Read a case body from stdin, prepend the library, and +# print the path of the runnable case script. `strict` keeps the library's +# `set -e` in force; the default relaxes it so a case can inspect a return code. +case_build() { + local mode="${1:-relaxed}" script + script="$(mktemp "$TEST_TMPDIR/case.XXXXXX")" + { + printf 'source %q\n' "$LIB" + if [[ "$mode" != strict ]]; then printf 'set +e\n'; fi + cat + } >"$script" + printf '%s' "$script" +} + +# case_exec