Skip to content

test: close affected-tests blind spots for seven unmapped scripts across five plugins - #3459

Merged
kyle-sexton merged 2 commits into
mainfrom
claude/repo-improvement-discovery-y2wg3r
Aug 28, 2026
Merged

test: close affected-tests blind spots for seven unmapped scripts across five plugins#3459
kyle-sexton merged 2 commits into
mainfrom
claude/repo-improvement-discovery-y2wg3r

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

No linked issue

Summary

A repo-wide /improvement:find scan surfaced seven executable scripts across five plugins that mapped to zero test suites under scripts/affected-tests.sh. The repo contract (AGENTS.md) treats a zero-suite mapping as an error, and this is the same gap class that let the kindle-dedrm firewall truthiness defect (#3396) ship undetected: scripts whose only "coverage" was accidental substring selection of unrelated suites. Writing the tests also forced out two latent defects in the instruction-placement eval harness, fixed here.

Fix

One sibling test suite per unmapped script, plus the two defect fixes:

  • wizard 0.2.4: new template.test.sh (144 assertions) covering the wizard library's fail-closed prompt gates, _drain_tty paste bypass, write_env quoting/atomicity/permissions, open_url https-only dispatch, gh secret/variable helpers (secret never on argv), stage framing, and the EXIT trap.
  • songwriting 1.4.15: new datamuse.test.sh (93 assertions), fully offline via a curl PATH-shim stub that fails hard if it does not shadow real curl. Covers the mode-to-relation table, TSV contract, family merge/dedupe/re-sort, LIMIT handling, and transport failures.
  • prototype 0.9.6: sibling suites for the canonical detect-ecosystems.sh and both skill wrappers (91 assertions): marker detection, glob-order pinning, near-miss plus non-marker manifests (so widening the marker list fails), symlink/anchoring/CLAUDE_PROJECT_DIR semantics, and wrapper delegation contracts. The scripts themselves are unchanged.
  • kindle-dedrm 0.7.9: new sync-prep.test.sh (73 assertions) proving the script is print-only: PATH shims shadow rm/pwsh/netsh with recording no-ops; "printed the command, ran nothing, deleted nothing" is asserted over a call log plus tree snapshot; the dry-run/live guard is tested in both directions (the firewall-bug class); the printed firewall rule name is checked against firewall.ps1's actual $RuleName.
  • instruction-placement 0.11.11: new adherence-experiment.test.sh (49 cases via a stub CLI) plus two fixes the tests forced out: the underscore scoring criterion was a constant (it grepped the whole edited file, which always contains the seeded _unitPrice, so the published underscore/both columns measured nothing; scoring now scans only the InvoiceTotal body with an under-crediting fallback), and the --filler flag was undocumented. adherence-results.md carries a correction note; the experiment's conclusion is unchanged (it rests on the correctly-scored sealed column).

Verification

  • Every previously-unmapped script now selects its sibling suite under scripts/affected-tests.sh --explain (each was UNMAPPED, exit 1, before), and --run over the full diff passes (7 suites).
  • shellcheck (repo rcfile), shfmt -d, and scripts/check-shell-portability.sh clean on all new suites; changelog parity (--check, --check-bump, --check-preserved) green; fixture-git-isolation, silent-skips, discriminating-skips, orphaned-fixtures, and purged-em-dashes gates green.
  • Each plugin's work was reviewed by an independent fresh-context verifier: diff review, gate re-runs, adversarial assertion audit, and independent mutation testing with byte-identity restoration proofs (implementer + verifier mutation batteries: wizard 33+9, songwriting 23+10, prototype 19+10, kindle-dedrm 12+9). All verifier findings, including one MEDIUM (the family-dedupe assertion could not distinguish unique from unique_by(.word)) and several LOW precision gaps, were closed with mutation-kill confirmation.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01XGLX1xYgy27JiRqLjoiH8T


Generated by Claude Code

…oss five plugins

Seven executable scripts mapped to zero test suites under
scripts/affected-tests.sh, the gap class behind the kindle-dedrm firewall
defect (#3396). Each now has real sibling coverage, independently verified
with adversarial review and mutation testing:

- wizard 0.2.4: template.test.sh, 144 assertions over the wizard library
- songwriting 1.4.15: datamuse.test.sh, 93 assertions, offline curl stub
- prototype 0.9.6: three detect-ecosystems suites, 91 assertions
- kindle-dedrm 0.7.9: sync-prep.test.sh, 73 assertions, print-only proof
- instruction-placement 0.11.11: adherence-experiment.test.sh, 49 cases,
  plus fixes for a constant underscore-scoring criterion (published
  results column corrected) and an undocumented --filler flag

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XGLX1xYgy27JiRqLjoiH8T
@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 28, 2026 06:25
@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 29s —— View job


Claude is working on this

  • Gather context (PR diff, files changed)
  • Run /review:security-review per custom instructions
  • Post findings

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76beda05b6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/instruction-placement/evals/adherence-experiment.sh Outdated
Comment thread plugins/instruction-placement/evals/adherence-experiment.sh Outdated
@github-actions github-actions Bot deleted a comment from claude Bot Aug 28, 2026
@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

…nt-discovery-y2wg3r

# Conflicts:
#	plugins/kindle-dedrm/CHANGELOG.md
#	plugins/prototype/.claude-plugin/plugin.json
#	plugins/prototype/CHANGELOG.md
#	plugins/songwriting/.claude-plugin/plugin.json
#	plugins/songwriting/CHANGELOG.md
#	plugins/wizard/CHANGELOG.md
@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 30s —— View job


Claude is working on this

  • Gather context (PR diff, files changed)
  • Run /review:security-review per custom instructions
  • Post findings

Security review

No security issues found. Skip gate applies: this change has no security-relevant surface.

What changed (verified via git diff origin/main...HEAD): 19 files, entirely test infrastructure — five new/expanded *.test.sh suites (wizard, songwriting, prototype ×3, kindle-dedrm), one modified eval harness (plugins/instruction-placement/evals/adherence-experiment.sh), plugin.json version bumps, and CHANGELOG/results-doc updates. No .github/workflows/** files are touched.

Specifically checked:

  • sync-prep.test.sh (kindle-dedrm): uses mktemp -d sandboxes and PATH-shimmed rm/pwsh/powershell.exe/netsh/icacls recorders that perform nothing — asserts the script under test never runs a real destructive/elevated command. No real firewall or filesystem state outside the sandbox is reachable.
  • datamuse.test.sh (songwriting): curl is shadowed via a PATH stub with an explicit self-check (command -v curl must resolve to the stub or the suite fails), so no live network call to api.datamuse.com is possible.
  • template.test.sh (wizard): the new assertions actively verify good security properties in the library under test — secrets are sent over stdin and never appear on argv or in printed output (set_secret/ask_secret), the env file is tightened to 0600, and open_url refuses non-https URLs. No new exposure introduced.
  • detect-ecosystems.test.sh (prototype, ×3): pure marker-detection/glob-order fixtures; no script changes, no network or credential surface.
  • adherence-experiment.sh scoring fix (instruction-placement): tightens two regexes used to grade locally-generated eval transcripts (anchoring the class-name match, requiring a field-declaration terminator for the underscore check). This is a measurement-correctness fix in an internal eval harness, not a trust-boundary or injection-relevant change — no untrusted external input reaches these regexes.

No secrets, injection surfaces, authz logic, or GitHub Actions permissions/triggers are touched by this diff.

(Note: the two Codex review comments on this PR concern eval-scoring precision, not security — out of scope for this lane; /review:code-review is the right lane for those.)

@github-actions

Copy link
Copy Markdown

Last security-reviewed head: f2116e79374e25522f2460909b253c9210a1c6a3. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 7m 43s —— View job


Claude finished the code review

  • Gather context (PR diff, files changed)
  • Run /review:code-review per custom instructions
  • Post findings

Note: the review:code-review skill failed to execute in this environment (returned an infrastructure error on every invocation attempt), so this review was conducted manually following the same lane's scope: high-signal correctness/maintainability findings only, security out of scope (separate lane).

Summary

Reviewed the full diff (git diff origin/main...HEAD, 19 files, +2818/-11) via a dedicated static-review pass plus direct verification of the two logic changes in adherence-experiment.sh. No new correctness or maintainability findings.

On the two open Codex review comments

Codex's two P2 comments (unanchored InvoiceTotal class match, and the underscore check not requiring a real field declarator) describe defects that are already fixed in this exact commit (76beda0, unchanged by the later merge commit). I verified both directly against the current code:

  • Class-name anchoring (adherence-experiment.sh#L254): the pattern is InvoiceTotal([^A-Za-z0-9_]|$). Tested against sealed class InvoiceTotals { ... } — does not match, correctly rejected.
  • Field-declarator constraint (adherence-experiment.sh#L303): the pattern requires the match to end in ; or =[^>]. Tested against private decimal _total { get; set; }, private decimal _GetTotal() => total;, and private decimal _Total => x; — none match (auto-property, expression-bodied method, and expression-bodied property all correctly rejected); private readonly decimal _unitPrice; still matches as expected.

Both appear to be exactly the two defects the PR body says were fixed ("the underscore scoring criterion was a constant... scoring now scans only the InvoiceTotal body"). No action needed — flagging here so the thread doesn't read as unresolved.

Scope covered

  • adherence-experiment.sh's score_trial() awk body-extraction logic (decoy-class, primary-constructor, unbalanced-brace edge cases) — sound.
  • sync-prep.test.sh's PATH-shim stubs for rm/pwsh/netsh — command-scoped PATH= prefix, can't leak into the suite's own cleanup trap; sync-prep.sh itself never invokes those binaries, so the "touched nothing" assertions are real regression guards, not tautologies.
  • datamuse.test.sh's curl stub — self-verifies with command -v curl before any assertion runs, so it can't silently fall through to the network.
  • The three detect-ecosystems.test.sh copies (top-level + two skill wrappers) — near-identical by design, each wrapper asserts against its sibling and byte-diffs to catch drift; not flagging the duplication since it's a documented, low-drift-risk pattern.
  • wizard/skills/generate/template.test.sh (764 lines, read in full) — the fd-3 seam rewrite, fail-closed guards, write_env quoting/atomicity, open_url https-only dispatch, gh secret/variable helpers (stdin-only, never argv), and the EXIT trap under set -e all check out; the shipped-file contract assertions (single exec 3</dev/tty, single STAGES marker, TOTAL_STAGES consistency) correctly pin the seam so it can't silently drift onto executing a different code path.

@kyle-sexton
kyle-sexton merged commit 94c2abe into main Aug 28, 2026
58 checks passed
@kyle-sexton
kyle-sexton deleted the claude/repo-improvement-discovery-y2wg3r branch August 28, 2026 11:14
@github-actions

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

kyle-sexton added a commit that referenced this pull request Aug 28, 2026
… it found in the rubric (#3472)

Rubric v3 shipped in #3471 with an obligation attached: its own rule blocks
every precision figure, and with it every class's fix eligibility, until the
golden set is re-scored against v3. That re-score is done here.

- The re-score reproduces the recorded table exactly: 8 tp / 0 fp / 0 fn /
  2 tn, thirty judges in independent processes, three per case, cases
  relabelled so no path reached a judge. Every panel unanimous, no verdict
  moved. No class becomes fix-eligible, because every class sits below
  min_n_per_class 10 -- the class-size block stands after the
  rubric-version one is lifted.
- The rubric carried its own answer key. Its version-history paragraph
  recorded the expected tally, the panel size, and which case turns on which
  criterion, and the pipeline inlines the whole rubric into every judge
  prompt. All thirty judges read the prediction before grading, and the run
  withdrew its claim of a blind panel. The prediction now lives in the
  changelog; the rubric says a judge should be able to read all of it and
  still not know the answer.
- A vendored-snapshot basis gains a tier rule: caps at
  source-fetched-similar, records source.route with each failed live fetch,
  never fix-eligible, because fix eligibility rests on current upstream state
  and a snapshot cannot establish it.
- The not-found searched-surfaces listing is recorded as prose-only and
  unenforced, because emit-findings.sh carries no field for it.
- The modal "may" is no longer read as a month name. Three sites, including
  the classifier a single-site fix would miss.

Three reviewers then found that the "may" fix had traded over-reporting for
under-reporting: a digitless "Verified this May" vanished upstream of the
declined bucket, from both audit paths. Fixed by reading the capital M from
the original line, which both scripts had been discarding via tolower(). The
suites could not have caught it -- they asserted the two scripts agree, which
passes when both are equally wrong -- so the new cases pin non-zero expected
counts on each side.

Every substantive change was checked by an independent fresh-context
verifier. Two returned FAIL and were acted on. One, nested two tiers down and
never briefed by this session, noticed the branch was 9 commits ahead rather
than 1 -- which was hiding that the branch had never been reset after #3471
merged and would have deleted seven test suites added by #3459.

Limits recorded rather than left to be discovered: the re-score panel was not
blind, for the reason above; a second answer-key leak sits in a fixture and is
deliberately unfixed because that line is inside the text the fingerprint
compares, so the fix and a re-score are one atomic change; and a latent
under-report from may_form()'s branch order is documented at the rule rather
than chased, since the obvious one-line reorder only mirrors it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants