Skip to content

test(ai-slop): back the audit evals with committed fixtures (0.3.0) - #3063

Merged
kyle-sexton merged 8 commits into
mainfrom
claude/ai-slop-unslop-integration-l8gr0k
Aug 21, 2026
Merged

test(ai-slop): back the audit evals with committed fixtures (0.3.0)#3063
kyle-sexton merged 8 commits into
mainfrom
claude/ai-slop-unslop-integration-l8gr0k

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Closes #3041

Summary

ai-slop's audit eval cases described their input in prose inside the prompt string. Nothing checked that the described input produced the finding the case graded, and it disagreed with the detector three times in one PR — each time a golden answer the scenario could not produce. Seven of the nine cases now name a committed fixture instead.

Fix

  • Six fixtures ship under plugins/ai-slop/skills/audit/evals/fixtures/, referenced from each case's files array:

    Fixture What the detector actually emits
    report-only.md 4 findings across 4 rules (filler L3, significance-inflation L6, em-dash + chatbot-artifacts L8)
    fix-guarded-rewrite.md 2 em dashes (L3, L7) + 2 filler phrases (L3, L4)
    rubric-boundary.md exactly 1 script finding (em dash L7); the promotional register sits outside the mechanical vocabulary on purpose
    em-dash-substitution.md exactly 1 em dash (L5)
    triads.md rule-rule-of-three, density 43.5/1000 (3 hits in 69 words, threshold 3.0)
    knowledge-cutoff-prose.md 3 rule-knowledge-cutoff-disclaimer findings (L1, L3, L5), all in the recorded false-positive class
  • Every expected_output names the rules, lines and fired thresholds the detector emits, measured rather than asserted. Case 7's three triads are all single-token, so the load-bearing one genuinely reaches the fix flow — the third of the three original defects was a fix whose multi-word items never matched the rule's ERE.

  • A fixture is named the way every sibling suite names one: files[] plus a prose prompt pointing at evals/fixtures/<name>.md relative to the skill directory, the same form mcp-tools:audit and docs-hygiene:compress use. Eighteen other fixture-backed suites do this and none builds a repository to audit in.

  • Three fix cases carry a graded copy instruction. fix rewrites in place, so cases 2, 6 and 7 say to work on a copy, and each asserts the committed fixture is left byte-identical after the run — graded on the outcome, not on how the copy is made.

  • Cases 4 and 5 state their tracked-file premise ("treat the audited file as tracked in the repo under audit") rather than constructing repository state for it. SKILL.md gates findings-file persistence on the audit having "examined tracked files", so trackedness is a precondition for reaching the branch, not the claim under test — what those cases grade is the decision: that persistence applies, that the producer contract is fetched before anything is written, that the findings file is written, and that a refusal is attributed to an unreachable contract rather than to the target being out of tracked space.

  • Cases 3 and 9 keep narration: true: they grade repo-wide flow and consuming-repo config, not file content, so there is nothing for a fixture to pin.

  • The 0.1.0 no-fixtures decision is reversed, in the header that recorded it. It still holds for the unit suite (fixtures stay inline in a tmpdir); it does not survive the eval suite, where a case is graded downstream of a deterministic detector run and its scenario has to satisfy an ERE the eval author cannot see.

  • Exclusion via excluded_paths, not an in-file marker. A file marker declines unconditionally — including under the empty HOME + CLAUDE_PROJECT_DIR isolation the detector's own tests use — so detect.sh <fixture> would print nothing and the eval author would be back to trusting prose. An excluded_paths entry is a config layer, and that isolation lifts it. This repo's .claude/ai-slop.json carries the glob as the worked example, with the reasoning in _comment.

  • Consuming repos need no exclusion of their own: the audit scans git ls-files '*.md', and an installed plugin's files are not tracked in the repo that installs it.

Why the prompts are a specification and not a script

Three intermediate commits built staging into the prompt strings — git init a scratch repo, an identity-scoped commit, pinned CLAUDE_PROJECT_DIR and HOME, unset GIT_DIR. That apparatus is gone, and the reason is worth recording so it is not rebuilt.

claude plugin eval --help runs <eval dir>/**/case.yaml or prompt.md + graders/*.md. This repo has zero of that layout against 200 evals.json files, no plugin manifest declares experimental.evals, and the only things that read evals.json are lint scripts. Nothing executes these prompts. Every defect the staging apparatus accumulated — a missing git identity, a leaked config cascade, $CLAUDE_SKILL_DIR not being an exported variable, an unspecified working directory, GIT_DIR surviving into the graded commands, and a VAR=… detect.sh form that cannot match the skill's own literal-prefix allowed-tools grant — was a property of executing something nothing executes.

If this repo ever adopts that layout, staging has a documented home: a case's scaffold_script, run under --scaffold. It never belonged inside a prompt string.

Verification

Every fixture was measured against the shipped defaults (HOME and CLAUDE_PROJECT_DIR pinned to an empty dir, confirmed via --show-config reporting (none; bundled defaults)) before its case was written — the table above is that output, not a description of it.

Check Result
detect.sh <fixture> × 6, isolated config matches each case's expected_output exactly
triads.md figures recomputed from source 3 hits, 69 words, 43.5/1000, threshold 3.0
detect.sh evals/fixtures/ with this repo's config 0 findings, 6 files declined (cause=excluded-glob)
scripts/check-orphaned-fixtures.sh --check exit 0 — all 6 report CONSUMED
check-evals-quality.sh on this file PASS, 0 warnings
check-jsonschema vs evals.schema.json ok
detect.test.sh all 92 cases passed
check-changelog-parity.sh --check-bump origin/main exit 0 (0.2.2 → 0.3.0)
check-changed-skills.sh origin/main PASS, 0 errors (3 pre-existing warnings, untouched surfaces)
shellcheck detect.test.sh clean
apparatus residue (git init, mktemp, unset GIT, CLAUDE_*_DIR, scratch, docs/, …) 21 tokens, all grep count 0

Fresh-context adversarial passes reviewed each head with instructions to refute rather than confirm, and found the defects the automated lanes did not. The final pass confirms the apparatus is gone with no residue, all six fixtures reconcile figure-for-figure, nothing was weakened against the pre-apparatus baseline, and every CHANGELOG claim re-derives.

Related

The audit eval cases described their input in prose, and nothing checked that
the described input produced the finding the case graded. It drifted from the
detector three times in one PR — each time a golden answer the scenario could
not produce.

Seven of the nine cases now name a committed fixture under
skills/audit/evals/fixtures/ via files[], and every expected_output names the
rules, lines and fired thresholds the detector actually emits, measured with
detect.sh under an empty HOME and CLAUDE_PROJECT_DIR rather than asserted.
Cases 3 and 9 keep narration: true — they grade repo-wide flow and
consuming-repo config, not file content.

This reverses 0.1.0's no-fixtures decision, which was recorded in
detect.test.sh's header and is updated in place. The decision still holds for
the unit suite, whose fixtures stay inline in a tmpdir; it does not survive the
eval suite, where a case is graded against a deterministic detector run and its
scenario has to satisfy an ERE the eval author cannot see.

The fixtures carry real tells, so this repo's own audit declines them through
an excluded_paths glob rather than an in-file marker: a file marker declines
unconditionally, including under the config isolation the detector's tests use,
which would leave detect.sh <fixture> printing nothing and the eval author back
to trusting prose.

Closes #3041

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 4m 15s —— View job


Code review — PR #3063

Lane: /review:code-review (correctness/maintainability). This repo carries .github/workflows/claude-security-review.yml, so security is scoped to that lane and out of this one.

Verification method: this lane's Bash access is restricted to gh pr */Read/Glob/Grep — detect.sh couldn't be executed here, so the PR's shell-command verification table (detect.sh runs, check-*.sh, shellcheck) is author-claimed and unverified by me. Instead I hand-checked every new fixture against detect.sh's actual rule EREs (lines 57–80) and cross-referenced reference/catalog.md's calibration notes:

Fixture Claim in evals.json Static check
report-only.md filler L3, significance-inflation L6, em-dash + chatbot-artifacts L8 matches — "In order to" (L3), "crucial role" (L6), em dash + "I hope this helps" (L8)
fix-guarded-rewrite.md 2 em dashes (L3, L7), 2 filler (L3, L4) matches
rubric-boundary.md exactly 1 em dash (L7), promotional words outside script vocab matches — rule-promotional-language catalog entry (L123) names "breathtaking"/"must-visit" as deliberately excluded from the script vocab — the fixture uses those exact words
em-dash-substitution.md exactly 1 em dash (L5) matches
triads.md rule-of-three, 3 hits in 69 words, 43.5/1000 matches exactly — counted 69 whitespace-delimited tokens via Grep -o '\S+' and 3 ERE-matching triads; 3×1000/69 = 43.5
knowledge-cutoff-prose.md 3 findings (L1, L3, L5) matches — catalog's recorded false-positive class (L59-63)

Also checked: evals.schema.json compliance (cases keep files+drop narration correctly; cases 3/9 correctly retain narration: true since they stay files: []), the excluded_paths glob's bash case-pattern semantics (** behaves as * here, correctly matches the fixtures dir), and that check-orphaned-fixtures.sh/check-evals-quality.sh/check-changelog-parity.sh/check-changed-skills.sh referenced in the PR description all exist in the repo.

Result: no findings. Every expected_output I could statically verify matches the detector's actual regex behavior, the schema stays valid, and the exclusion mechanism (excluded_paths over an in-file marker) is correctly reasoned given the config-isolation the eval suite relies on. No inline comments posted.
· branch claude/ai-slop-unslop-integration-l8gr0k

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 59s —— View job


Security Review

  • Gather context (PR diff, changed files)
  • Check skip gate
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

This PR's changed surface is entirely test/config data, not executable logic:

  • plugins/ai-slop/skills/audit/evals/fixtures/*.md — six new inert-prose fixtures (retry-helper docs, cache-invalidation notes, a metrics-dashboard blurb, parser release notes, a config-cascade doc, and a knowledge-cutoff explainer). None contain shell metacharacters, script tags, or content that would behave differently than inert text when passed through detect.sh's ERE-based grep matching.
  • plugins/ai-slop/skills/audit/evals/evals.json — eval case prompts/expectations updated to point at the new fixtures instead of prose descriptions; no code path changes.
  • .claude/ai-slop.json — adds plugins/ai-slop/skills/audit/evals/fixtures/** to excluded_paths (a config-only glob, not a code-execution or file-marker mechanism) so the fixtures aren't flagged by this repo's own audit; reasoning is documented in _comment.
  • plugins/ai-slop/skills/audit/scripts/detect.test.sh — comment-only change explaining the exclusion.
  • plugins/ai-slop/.claude-plugin/plugin.json, CHANGELOG.md — version bump and changelog entry only.

No GitHub Actions workflows, authentication/authorization logic, injection surfaces, or secret-handling code are touched by this PR.

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

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

@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: dc5ca9ade3

ℹ️ 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/ai-slop/skills/audit/evals/evals.json Outdated
Comment thread plugins/ai-slop/skills/audit/evals/evals.json
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

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

…g them

The fixture-backed cases pointed the invocation straight at the committed
fixture, which reintroduced the failure class #3041 was meant to close.

The fix cases (2, 6, 7) rewrite each flagged line in place, so the first run
remediated the committed fixture and the second run graded already-fixed
input, where the declared findings no longer fire.

The persistence expectations (2, 4, 5) grade a branch conditioned on the audit
having examined tracked files. An installed plugin's fixture is not tracked in
the consuming repo, so the branch was unreachable, and case 4's contract-fetch
refusal passed for the wrong reason: nothing was written because nothing was
owed, not because the fetch failed.

Every fixture-backed case now stages its target first — git init a temp
directory, copy the fixture in as docs/<name>.md, commit it, audit the copy.
The rewrite lands on a throwaway file and the target is tracked, so the
persistence branch is genuinely reached. Staged copies re-measured: identical
rules, lines and densities at the new path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 59s —— View job


Security Review

  • Gather context (PR diff, changed files)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Skip gate: the previous run on this PR (head dc5ca9a) already reviewed the full fixture/eval change set and found no security-relevant surface. This trigger is for a new commit, 1c06107 ("stage eval fixtures into a scratch repo before auditing them"), so I reviewed just the delta between those two heads:

git diff dc5ca9ade367685423d853f4894f89081bc73556..1c06107669e6558ca494f145c77daf6fd845e8aa --stat
 plugins/ai-slop/CHANGELOG.md                  | 11 ++++++
 plugins/ai-slop/skills/audit/evals/evals.json | 50 +++++++++++++++------------

Both files are non-executable:

  • plugins/ai-slop/skills/audit/evals/evals.json — this commit rewrites 7 eval-case prompts to describe staging each fixture into a fresh scratch git init directory (as docs/<name>.md) before auditing it, per the fix-mutates-the-committed-fixture and persistence-requires-tracked-files findings raised in review by chatgpt-codex-connector and confirmed by the PR author. These are natural-language instructions consumed by a human/agent running the eval manually — not code, and not a workflow step that executes automatically in CI (this eval suite is lint-checked, not executed, per the PR's own verification table). No injection surface: the "scratch git repo" is described in prose, not built from unsanitized input, and each case still names a fixed, repo-relative fixture path.
  • plugins/ai-slop/CHANGELOG.md — changelog prose only.

No scripts, GitHub Actions workflows, config/permission surfaces, or detector logic (detect.sh, detect.test.sh) changed in this delta. This confirms and extends the prior "no security issues" result to the current head.

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 50s —— View job


Code review — PR #3063

Lane: /review:code-review (correctness/maintainability). Security is scoped to /review:security-review.

  • Check skip gate (new commit 1c06107 since the lane's prior review — not a repeat)
  • Gather context (gh pr diff, focused on what 1c06107 changed since the previously-reviewed SHA dc5ca9a)
  • Review changed files for correctness/maintainability
  • Post findings

Scope of this pass: the new commit (1c06107, "stage eval fixtures into a scratch repo before auditing them") answers the Codex and @kyle-sexton review threads on the prior commit by adding a staging preamble to 7 of 9 eval prompts (git init a temp dir, copy the fixture in as docs/<name>.md, commit it, target the copy). I re-verified the fixture-content claims from the earlier commit were unchanged (no fixture files touched by 1c06107) and focused on whether the new staging mechanism itself is sound.

1 finding posted inline (plugins/ai-slop/skills/audit/evals/evals.json:7, recurring at lines 20, 46, 59, 72, 85, 98): the staging preamble's git commit step sets no git identity. Nothing in these prompts, the schema, or elsewhere in the repo guarantees a ~/.gitconfig for whoever executes them, and this same commit's own detect.test.sh comment (plugins/ai-slop/skills/audit/scripts/detect.test.sh:29-30) documents pointing HOME at an empty tmpdir for isolation — under that same convention, git commit fails with "Please tell me who you are" before the audit runs, which breaks exactly the reliability property this PR exists to add. detect.sh resolves tracked files via git ls-files (detect.sh:256,268), which already reports staged-but-uncommitted files as tracked, so dropping the commit step in favor of git add closes the gap with fewer moving parts than adding -c user.name=/-c user.email= flags.

Everything else in 1c06107's diff (the evals.json prompt/expectation rewording, the .claude/ai-slop.json comment expansion, the CHANGELOG.md entry, the detect.test.sh comment) is prose/config-only and consistent with what it describes; no other correctness issues found.
· branch claude/ai-slop-unslop-integration-l8gr0k

Comment thread plugins/ai-slop/skills/audit/evals/evals.json Outdated
The staging preamble said "git init a temp directory ... and commit it", but
git init inherits user.name/user.email from ~/.gitconfig or the GIT_* env, and
neither is guaranteed. This suite's own unit tests pin HOME to an empty tmpdir
for isolation (detect.test.sh), and a fresh container has no seeded identity.

Under that condition the preamble dies at git commit with "Please tell me who
you are" (exit 128) BEFORE the audit runs, so all seven staged cases error out
instead of grading anything - the same "scenario cannot produce the graded
input" failure class this release exists to remove.

All seven preambles now commit with the identity inline:

  git -c user.name=eval -c user.email=eval@local commit -m stage

Staging still stops at a commit rather than at git add. git ls-files already
reports a staged file as tracked, so either form satisfies the persistence
gate, but these cases model a consuming repo and a real one has history: the
repo-wide ordering reads git log --since=90.days --name-only for change
frequency, which returns nothing in a repo with no commits. Harmless for
today's single-file cases, and invisible in every current expectation, which
is why it is worth not building in.

Verified by executing each preamble as written under an empty HOME with
GIT_AUTHOR_*/GIT_COMMITTER_* unset: all seven commit, leave a HEAD, report the
target tracked, and emit their declared finding counts (4, 4, 4, 1, 1, 1, 3).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review

  • Gather context (PR diff since last reviewed head)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Skip gate: the previous run on this PR (head 1c06107) already covered the full fixture/eval/staging change set with no security-relevant surface. This trigger is for the new commit, 5010437 ("give the eval staging commit an explicit git identity"), so I reviewed just the delta between those two heads:

git diff 1c06107669e6558ca494f145c77daf6fd845e8aa..50104376455886728c6600966f75d3533d67507a --stat
 plugins/ai-slop/CHANGELOG.md                  | 14 ++++++++++++++
 plugins/ai-slop/skills/audit/evals/evals.json | 14 +++++++-------

Both changed files are non-executable, and neither introduces new security-relevant surface:

  • plugins/ai-slop/skills/audit/evals/evals.json — this commit adds git -c user.name=eval -c user.email=eval@local commit -m stage to the staging preamble in 7 eval-case prompts, closing the git-identity gap flagged by the prior code-review pass (missing identity would make git commit fail under an isolated HOME). This is natural-language instruction text for a manually-run eval, not code executed in CI (the eval suite is lint-checked in this pipeline, not executed — confirmed against the PR's verification table). The literal string eval@local is a fixed placeholder, not derived from any untrusted input, so there's no injection surface even if a human/agent later executes it verbatim.
  • plugins/ai-slop/CHANGELOG.md — changelog prose only, documenting the same fix.

No scripts, GitHub Actions workflows, config/permission surfaces, or detector logic (detect.sh, detect.test.sh) changed in this delta. This confirms and extends the prior "no security issues" result to the current head.
· branch claude/ai-slop-unslop-integration-l8gr0k

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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


Code review — PR #3063

Lane: /review:code-review (correctness/maintainability). Security is scoped to /review:security-review.

  • Check skip gate (new commit 5010437 since the lane's prior review — not a repeat)
  • Gather context (diff between the previously-reviewed head 1c06107 and current head 5010437)
  • Review changed files for correctness/maintainability
  • Post findings

Scope of this pass: the new commit (5010437, "give the eval staging commit an explicit git identity") answers this lane's own prior finding on 1c06107 — that the staging preamble's git commit step had no git identity and would fail with "Please tell me who you are" under an empty HOME (the same isolation convention detect.test.sh uses).

Verification:

  • All 7 staging preambles (cases 1, 2, 4, 5, 6, 7, 8) now read git -c user.name=eval -c user.email=eval@local commit -m stage. Confirmed by count: grep -c "git -c user.name=eval" and grep -c "git init" both return 7 in evals.json — no case was missed.
  • The syntax (git -c key=value ... commit) is valid one-off config scoping and correctly resolves the identity error without touching global/user config.
  • CHANGELOG.md amends the existing unreleased 0.3.0 entry (correct — no version bump needed for an unreleased version) and gives a reasoned justification for keeping commit over the lighter-weight git add alternative I'd flagged as an option: these cases model a consuming repo, and git add alone would leave the scratch repo with zero commits, which would make a real repo-wide op (git log --since=90.days) return empty — the doc calls this out explicitly as a considered, harmless-today tradeoff rather than an oversight.
  • No other files in the diff (git diff 1c06107..HEAD --stat shows only evals.json and CHANGELOG.md); fixtures, schema, .claude/ai-slop.json, and detect.test.sh are unchanged since the previous review of this lane.

Result: no findings. The P2 finding from the prior review pass is resolved correctly and consistently across all 7 affected cases; nothing new introduced by this commit.
· branch claude/ai-slop-unslop-integration-l8gr0k

…eamble

The staging preamble isolated git identity but not the config cascade.
detect.sh resolves REPO_ROOT from CLAUDE_PROJECT_DIR before falling back to
git rev-parse --show-toplevel, then loads that root's .claude/ai-slop.json,
and rule disablement is not path-scoped. A maintainer running these evals
from a session rooted at this repo therefore inherits its disabled_rules -
rule-em-dash, rule-curly-artifacts, rule-emoji-formatting - and those apply
to the /tmp scratch repo too.

Measured with only CLAUDE_PROJECT_DIR changed:

  case 1: 4 -> 3 findings   (em dash L8 vanishes)
  case 2: 4 -> 2            (both em-dash findings vanish)
  case 4: 4 -> 3
  case 5: 1 -> 0            (the em dash IS the single script finding)
  case 6: 1 -> 0            (the case is entirely that one em dash)
  cases 7, 8: unaffected

Cases 5 and 6 fail silently: nothing is found, nothing is asserted, and the
expectations pass vacuously. Same golden-answer-the-scenario-cannot-produce
class as the rest of this release, arriving through config rather than prose.

All seven fixture-backed prompts now point CLAUDE_PROJECT_DIR at the scratch
directory and HOME at an empty one, so the cascade resolves to shipped
defaults. Applied to 7 and 8 as well: their rules are not disabled today, but
disabled_rules can change and uniformity is cheaper than a per-case rule.
The knowledge that this isolation is what keeps the fixtures measurable was
already recorded in detect.test.sh's header comment; it now lives in the
artifact that needs it.

Prompts only - no expected_output, files, expectations, narration, or fixture
byte changed, and cases 3 and 9 stay byte-identical to dc5ca9a. Re-ran all
seven preambles literally under the pinned cascade: every rule id, line and
density figure still matches its expected_output, case 7 included.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review

  • Gather context (PR diff since last reviewed head)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Skip gate: the previous run on this PR (head 5010437) already covered the fixture/eval/staging change set with no security-relevant surface. This trigger is for the new commit, 0f5fec4 ("pin the detector config cascade in the eval staging preamble"), so I reviewed just the delta between those two heads:

git diff 50104376455886728c6600966f75d3533d67507a..0f5fec477b59049efb0e17808095f6027f8146b1 --stat
 plugins/ai-slop/CHANGELOG.md                  | 10 ++++++++++
 plugins/ai-slop/skills/audit/evals/evals.json | 14 +++++++-------

Both changed files are non-executable, and neither introduces new security-relevant surface:

  • plugins/ai-slop/skills/audit/evals/evals.json — this commit extends the staging preamble in all 7 fixture-backed cases with point CLAUDE_PROJECT_DIR at that directory and HOME at an empty one, so the eval's detector run resolves the shipped-default config rather than this repo's own .claude/ai-slop.json (which disables rule-em-dash and would silently drop findings the cases grade). This is natural-language instruction text describing environment variables for a manually-run eval — not code executed in CI (the eval suite is lint-checked, not executed, consistent with every prior review of this PR). CLAUDE_PROJECT_DIR and HOME are pointed at fixed, freshly-created scratch paths described in prose, not built from unsanitized input, so there's no injection or path-traversal surface.
  • plugins/ai-slop/CHANGELOG.md — amends the existing unreleased 0.3.0 entry with the same explanation (config-cascade isolation gap, measured finding-count deltas). Changelog prose only.

No scripts, GitHub Actions workflows, config/permission surfaces, or detector logic (detect.sh, detect.test.sh) changed in this delta. This confirms and extends the prior "no security issues" result to the current head.

Note: my working tree shows an uncommitted, unrelated local modification to .claude/ai-slop.json (reverting its fixtures excluded_paths entry). It is not part of this PR's commit history and I left it untouched — flagging only in case it's unexpected on your end.

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 37s —— View job


Code review — PR #3063

Lane: /review:code-review (correctness/maintainability). Security is scoped to /review:security-review.

  • Check skip gate (new commit 0f5fec4 since the lane's prior review of 5010437 — not a repeat)
  • Gather context (git diff 5010437..0f5fec4, focused on what changed since the previously-reviewed head)
  • Review changed files for correctness/maintainability
  • Post findings

Scope of this pass: the new commit (0f5fec4, "pin the detector config cascade in the eval staging preamble") answers a bug in the staging preamble added two commits ago: detect.sh resolves its config root from CLAUDE_PROJECT_DIR before falling back to git rev-parse --show-toplevel (detect.sh#L150-L152). A git init'd scratch directory doesn't clear an already-set CLAUDE_PROJECT_DIR — if the eval is run from a session rooted at this repo, the scratch audit still inherits this repo's .claude/ai-slop.json, which disables rule-em-dash. The fix adds an explicit CLAUDE_PROJECT_DIR-at-scratch-dir instruction to all 7 fixture-backed prompts (cases 1, 2, 4, 5, 6, 7, 8), alongside the already-established empty-HOME pin.

Verification, checked against the actual fixtures and detector code rather than taken on claim:

Fixture (case) Em-dash findings if rule-em-dash leaked disabled Changelog's claimed drop Match
report-only.md (1, 4) L8 removed → 4→3 "case 1 drops 4 to 3, case 4 drops 4 to 3"
fix-guarded-rewrite.md (2) L3, L7 removed → 4→2 "case 2 drops 4 to 2"
rubric-boundary.md (5) L7 was the only finding → 1→0 "case 5 drops to zero"
em-dash-substitution.md (6) L5 was the only finding → 1→0 "case 6 drops to zero"
triads.md (7), knowledge-cutoff-prose.md (8) no em dash present correctly omitted from the drop list

Also confirmed: the config-cascade ordering description in the CHANGELOG entry (CLAUDE_PROJECT_DIRHOME's ~/.claude/ai-slop.json → repo root's .claude/ai-slop.json) matches detect.sh:150-157 exactly, disabled_rules still includes rule-em-dash at this repo's root config, and the diff only touches the 7 prompt strings plus the changelog — no expected_output values, schema, or fixtures changed, so the fix is additive isolation rather than a rewrite of what's graded.

Result: no findings. This closes a real, subtle bug (an ambient session env var silently deleting the graded finding from an otherwise-isolated scratch repo) with a correct, narrowly-scoped fix, consistent with the pattern of the prior three review rounds on this PR.
· branch claude/ai-slop-unslop-integration-l8gr0k

…uence

Three residual defects, all the same class: the preamble described staging in
prose and left the mechanism to the reader.

A. The cascade pin was phrased as a one-time export, but the harness
   re-supplies CLAUDE_PROJECT_DIR on every tool call and shell state does not
   survive between them. A model that staged in one call and ran the detector
   in the next got the disabled rule back and measured 0 findings on case 5.
   The preamble now runs in ONE invocation, says why, and instructs setting
   CLAUDE_PROJECT_DIR/HOME on every later command rather than exporting once.

B. The preamble never issued git add, so a literal executor produced an
   untracked target - inverting the tracked-file branch cases 4 and 5 exist to
   grade. The sequence now stages before committing.

C. The scratch repo was built without clearing inherited git environment,
   violating the invariant scripts/check-fixture-git-isolation.sh already
   enforces for *.sh/*.py fixtures. Under an ambient GIT_DIR the init and the
   stage commit landed in the CALLER's repository and the scratch directory got
   no .git at all. That gate cannot see prose inside evals.json, so the eval
   suite now honors the rule itself: the sequence opens with
   unset GIT_DIR GIT_WORK_TREE GIT_CONFIG.

Also repairs a hyphenated compound broken across a line wrap in the 0.3.0
entry, which rendered as "golden-answer-the- scenario-cannot-produce".

Verified by extracting each command verbatim from the committed JSON and
running it under a hostile environment - HOME holding a config that disables
rule-em-dash, CLAUDE_PROJECT_DIR pointed at this repo, and an ambient GIT_DIR
pointed at this repo's .git. All seven: commit exit 0, HEAD created, target
tracked, scratch dir owns its .git, caller HEAD unchanged, and the declared
finding counts (4, 4, 4, 1, 1, 1, 3). Detector re-run in a separate tool call
from staging still reports them; relying on ambient env instead reports 0,
which is the failure A describes.

Prompts only - no expected_output, expectations, files, narration, name or
skill_name changed, cases 3 and 9 stay byte-identical to dc5ca9a, and no
fixture byte moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review

  • Gather context (PR diff since last reviewed head)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Skip gate: the previous run on this PR (head 0f5fec4) already covered the full fixture/eval/staging change set with no security-relevant surface. This trigger is for the newest commit, 1adb33a ("make the eval staging preamble an executable shell sequence"), so I reviewed just the delta between those two heads:

git diff 0f5fec4..1adb33a --stat
 plugins/ai-slop/CHANGELOG.md                  | 18 +++++++++++++++---
 plugins/ai-slop/skills/audit/evals/evals.json | 14 +++++++-------

Both changed files are non-executable, and neither introduces new security-relevant surface:

  • plugins/ai-slop/skills/audit/evals/evals.json — rewrites the staging preamble in all 7 fixture-backed cases from loose prose into one literal shell one-liner (unset GIT_DIR GIT_WORK_TREE GIT_CONFIG; d=$(mktemp -d); cd "$d"; git init -q; mkdir -p docs home; cp "$CLAUDE_SKILL_DIR/evals/fixtures/<name>.md" docs/<name>.md; git add docs/<name>.md; git -c user.name=eval -c user.email=eval@local commit -qm stage; echo "$d"). I checked this both as text and as a shell command:
    • Still not executed automatically. evals.json is schema-validated (check-jsonschema against evals.schema.json) and quality-linted (check-evals-quality.sh) in .github/workflows/ci.yml:1282-1296; nothing in CI parses or runs the prompt string. It remains instruction text for a manually-run/agent-run eval, consistent with every prior review pass on this PR.
    • No injection surface. $CLAUDE_SKILL_DIR is a fixed runtime-supplied path (not attacker/user-controlled input), used quoted throughout; $d comes from mktemp -d and is quoted at every use; the literal -c user.name=eval -c user.email=eval@local values are fixed strings, not interpolated from anything external.
    • The isolation claim in the changelog checks out. I read scripts/check-fixture-git-isolation.sh directly: it scans git ls-files '*.sh' '*.py' only (line 318), so it structurally cannot see a fixture-building git init sequence embedded as a JSON string inside evals.json — the changelog's "that gate cannot see prose inside evals.json" claim is accurate, not asserted. The new preamble's leading unset GIT_DIR GIT_WORK_TREE GIT_CONFIG matches exactly the invariant that script enforces for .sh/.py fixture suites (its own header, lines 35-38), applied here by convention since the gate has no reach into this file.
    • Net effect is a hardening, not a new risk. Folding the sequence into one shell invocation (with an explicit rationale that env exports don't survive between separate tool calls) and adding git add before commit closes gaps the previous prose left to interpretation — same conclusion as the last four review passes on this PR's staging mechanism.
  • plugins/ai-slop/CHANGELOG.md — changelog prose only, documenting the same change and citing the isolation gate's actual scope.

No scripts, GitHub Actions workflows, config/permission surfaces, fixtures, or detector logic (detect.sh, detect.test.sh) changed in this delta. This confirms and extends the prior "no security issues" result to the current head.
· branch claude/ai-slop-unslop-integration-l8gr0k

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 22s —— View job


Code review — PR #3063

Lane: /review:code-review (correctness/maintainability). Security is scoped to /review:security-review.

  • Check skip gate (new commit 1adb33a since the lane's last review of 0f5fec4 — not a repeat)
  • Gather context (git diff 0f5fec4..1adb33a, focused on what this commit changed)
  • Review changed files for correctness/maintainability
  • Post findings

Scope of this pass: the new commit (1adb33a, "make the eval staging preamble an executable shell sequence") rewrites the staging preamble in all 7 fixture-backed eval prompts (cases 1, 2, 4, 5, 6, 7, 8) from a prose description into a single literal shell one-liner, per the CHANGELOG's own framing: "The preamble is an explicit shell sequence, not prose." Only evals.json and CHANGELOG.md changed in this delta.

1 finding posted inline (plugins/ai-slop/skills/audit/evals/evals.json:7, recurring identically at lines 20, 46, 59, 72, 85, 98): the new one-liner's cp step reads cp "$CLAUDE_SKILL_DIR/evals/fixtures/report-only.md" docs/report-only.md, but ${CLAUDE_SKILL_DIR} is a Claude Code text substitution scoped to two places only — a skill's own SKILL.md markdown content, and Bash rules in allowed-tools frontmatter (per the skills reference, and this repo's own docs/MIGRATION-PLAYBOOK.md:177-180, which explicitly contrasts it with ${CLAUDE_PROJECT_DIR} — "substituted ... and exported to subprocesses" — a claim ${CLAUDE_SKILL_DIR}'s entry does not make). It is not exported as an environment variable to Bash subprocesses, hooks, MCP, or LSP servers. evals.json's prompt string is eval-case data, not one of the two places substitution applies, so when an executor runs this "ONE shell invocation" literally as instructed, $CLAUDE_SKILL_DIR expands to empty, the cp fails ("No such file or directory"), and every step after it (git add, the identity-scoped commit) fails too — the case errors out before the audit ever runs. This directly undercuts the commit's own stated goal of making the preamble literally executable, and reintroduces the "case errors before grading" failure class the git-identity fix (5010437) closed two commits ago. The prior wording ("copy evals/fixtures/report-only.md (path relative to the skill directory)") avoided this by leaving path resolution to the executor rather than asserting a shell token; the fix should use an explicit fill-in placeholder instead, consistent with how <the printed path> is already handled later in the very same prompt.

Everything else in the diff (the CLAUDE_PROJECT_DIR/HOME env-pinning moved onto each later command instead of a one-time export, the added git add before commit, the unset GIT_DIR GIT_WORK_TREE GIT_CONFIG guard, and the CHANGELOG entry describing all three) is sound and correctly reasoned given the harness's per-tool-call shell-state reset.
· branch claude/ai-slop-unslop-integration-l8gr0k

Comment thread plugins/ai-slop/skills/audit/evals/evals.json Outdated
…tion

The staging preamble solved a problem this repo does not have. These eval
suites are not executable by `claude plugin eval`: the CLI consumes
`<eval dir>/**/case.yaml` or `prompt.md` + `graders/*.md`, and this repo has
200 evals.json files, zero of that layout, no manifest declaring
experimental.evals, and only lint scripts reading evals.json. Nothing runs a
prompt. The prompts are a written specification of expected skill behavior.

So every environment property the preamble controlled - config cascade, git
identity, `git add`, GIT_DIR isolation, the skill-dir path - was a property of
executing a prompt that nothing executes. Six failure modes in five review
rounds came from that machinery, in a shape none of the eighteen sibling
fixture-backed suites uses.

All seven cases now name the fixture through files[] and in prose, matching
mcp-tools:audit and docs-hygiene:compress: "evals/fixtures/<name>.md relative
to the skill directory".

One constraint survives independent of any harness question, and it is the
original Codex finding: the fix flow rewrites in place, so cases 2, 6 and 7
say to work on a copy and leave the committed fixture byte-identical. Prose,
no mechanism.

Cases 4 and 5 state their persistence premise rather than constructing it, and
their expectations now grade the skill's decision.

expected_output for cases 1, 2, 6, 7 and 8 is restored verbatim to dc5ca9a -
the measured text from before the apparatus. Fixtures are untouched and
re-measured under the shipped-defaults condition detect.test.sh itself uses:
every rule id, line and density figure still matches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review

  • Gather context (PR diff since last reviewed head)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Skip gate: the previous run on this PR (head 1adb33a) already covered the full fixture/eval/staging change set with no security-relevant surface. This trigger is for the newest commit, d5448e4 ("drop the eval staging apparatus for the sibling convention"), so I reviewed just the delta between those two heads:

git diff 1adb33a..d5448e4 --stat
 plugins/ai-slop/CHANGELOG.md                  | 68 +++++++++------------------
 plugins/ai-slop/skills/audit/evals/evals.json | 53 ++++++++++-----------

Both changed files are non-executable, and this commit removes rather than adds any shell-executable content:

  • plugins/ai-slop/skills/audit/evals/evals.json — reverts the multi-commit staging apparatus (the git init/mktemp -d/unset GIT_DIR/scoped-identity git commit one-liner flagged in the prior code-review pass for its $CLAUDE_SKILL_DIR non-export bug) back to plain prose: /ai-slop:audit evals/fixtures/<name>.md relative to the skill directory, matching the convention the CHANGELOG cites in mcp-tools:audit and docs-hygiene:compress. This is a net reduction in security-relevant surface — there is no longer any literal shell sequence embedded in the JSON string for an executor to run at all. As with every prior review of this PR, nothing in CI parses or executes evals.json's prompt field (check-evals-quality.sh and check-jsonschema only lint it); it remains instruction text read by a human or agent, not a script.
    • Cases 2, 6, 7 (fix cases) now say only "Work on a copy so the committed fixture stays byte-identical" with no mechanism specified — this is a correctness gap (the eval no longer specifies how to avoid mutating the committed fixture), not a security one, and is out of scope for this lane per /review:security-review's remit (style/quality findings belong to /review:code-review).
    • Cases 4/5 now state their tracked-file premise as an instruction ("Treat the audited file as tracked in the repo under audit") rather than constructing real git state — again a grading-fidelity question, not a security concern.
  • plugins/ai-slop/CHANGELOG.md — changelog prose only, documenting the reversal and its rationale (200 evals.json files repo-wide, none of which are executed by claude plugin eval's case.yaml/graders/ format).

No scripts, GitHub Actions workflows, config/permission surfaces, fixtures, or detector logic (detect.sh, detect.test.sh) changed in this delta — confirmed git diff 1adb33a..d5448e4 touches only the two files above.

Note (unrelated to this PR): the working tree at review time carries an uncommitted local modification to .claude/ai-slop.json (removing the fixtures excluded_paths glob and the _comment paragraph explaining it). It is not part of this PR's commit history — consistent with the same observation made in an earlier review pass on this PR — so I left it untouched and am only flagging it in case it's unexpected.

Cases 2, 6 and 7 told the reader to work on a copy and then checked nothing.
An instruction a case states but never checks is the same defect this release
removes, in miniature.

Each now carries one expectation asserting the outcome: the committed fixture
under evals/fixtures/ is byte-identical after the run, and the rewrites land
on a copy. How the copy gets made is the reader's business - pinning the
mechanism is what the earlier staging preamble did wrong.

Expectations only. No prompt, expected_output, files, narration, name or
fixture byte changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 49s —— View job


Security Review

  • Gather context (PR diff since last reviewed head)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Skip gate: the previous run on this PR (head d5448e4) already covered the full fixture/eval/staging change set with no security-relevant surface. This trigger is for the newest commit, b263df3 ("grade the work-on-a-copy instruction in the fix cases"), so I reviewed just the delta between those two heads:

git diff d5448e4..b263df3 --stat
 plugins/ai-slop/CHANGELOG.md                  | 5 ++++-
 plugins/ai-slop/skills/audit/evals/evals.json | 9 ++++++---

Both changed files are non-executable, and this commit is purely additive to grading criteria — no new mechanism, no new executable content:

  • plugins/ai-slop/skills/audit/evals/evals.json — adds one expectations entry to each of the three fix cases (2, 6, 7 — L27, L81, L94): Leaves evals/fixtures/<name>.md byte-identical after the run — the rewrites land on a copy. This checks the outcome of the prior commit's "work on a copy" instruction rather than adding any new mechanism, path, or executable step. As with every prior review of this PR, evals.json's prompt/expectations strings are lint-checked (check-evals-quality.sh, check-jsonschema) in CI, never parsed or executed — no injection or code-execution surface is introduced.
  • plugins/ai-slop/CHANGELOG.md — amends the existing unreleased 0.3.0 entry with the same explanation. Changelog prose only.

No scripts, GitHub Actions workflows, config/permission surfaces, fixtures, or detector logic (detect.sh, detect.test.sh) changed in this delta — confirmed git diff d5448e4..b263df3 touches only the two files above, and git status shows no other tracked changes.

Note (unrelated to this PR, unchanged from prior reviews): the working tree at review time still carries an uncommitted local modification to .claude/ai-slop.json (removing the fixtures excluded_paths glob and its _comment paragraph). It is not part of this PR's commit history, so I left it untouched — flagging again only in case it's unexpected on your end.
· branch claude/ai-slop-unslop-integration-l8gr0k

Case 5's remaining persistence expectations were negative or procedural: no
rubric finding in the file, and persistence treated as applicable. Neither
requires a file to exist. context/persist-findings.md permits reporting
without writing when the destination cannot be proven outside tracked space,
so a run could call persistence applicable, write nothing, and satisfy both -
green while asserting nothing, which is the shape this release removes.

Adds the positive half: the findings file is written, and a run that reports
the findings and persists nothing does not satisfy the case. The rubric
boundary now constrains a file that must exist.

Not a regression from dc5ca9a, which was also negative-only, but the
assertion is gradeable under the specification design and so was worth having.

Case 5 expectations only. Every prompt and expected_output in the file is
byte-identical to b263df3, and cases 1-4 and 6-9 are untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqwX2njhiLMWGrMg3QNt4g
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review

  • Gather context (PR diff since last reviewed head)
  • Invoke /review:security-review and follow its instructions
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Skip gate: the previous run on this PR (head b263df3) already covered the full fixture/eval/staging change set with no security-relevant surface. This trigger is for the newest commit, 0793689 ("assert case 5 actually writes the findings file"), so I reviewed just the delta between those two heads:

git diff b263df3..0793689 --stat
 plugins/ai-slop/CHANGELOG.md                  | 5 ++++-
 plugins/ai-slop/skills/audit/evals/evals.json | 1 +

Both changed files are non-executable, and this commit only strengthens an existing grading expectation — no new mechanism, path, or executable content:

  • plugins/ai-slop/skills/audit/evals/evals.json — adds one expectations entry to case 5: "Writes the findings file, on the stated premise that the audited file is tracked — a run that reports the findings and persists nothing does not satisfy this case." This closes a gap where the case's existing negative assertion ("puts NO rubric finding in the findings file") could be vacuously satisfied by a run that writes no findings file at all. As with every prior review of this PR, evals.json's prompt/expectations strings are lint-checked (check-evals-quality.sh, check-jsonschema) in CI, never parsed or executed by anything — no injection or code-execution surface exists here.
  • plugins/ai-slop/CHANGELOG.md — amends the existing unreleased 0.3.0 entry with the same explanation. Changelog prose only.

No scripts, GitHub Actions workflows, config/permission surfaces, fixtures, or detector logic (detect.sh, detect.test.sh) changed in this delta — confirmed git diff b263df3..0793689 touches only the two files above, and git status shows no other tracked changes on this commit.

Note (unrelated to this PR, unchanged from prior reviews): the working tree at review time still carries an uncommitted local modification to .claude/ai-slop.json (removing the fixtures excluded_paths glob and its _comment paragraph). It is not part of this PR's commit history, so I left it untouched — flagging again only in case it's unexpected on your end.
· branch claude/ai-slop-unslop-integration-l8gr0k

@kyle-sexton
kyle-sexton merged commit 25c79e5 into main Aug 21, 2026
49 checks passed
@kyle-sexton
kyle-sexton deleted the claude/ai-slop-unslop-integration-l8gr0k branch August 21, 2026 07:09
kyle-sexton pushed a commit that referenced this pull request Aug 21, 2026
…tes them

T2 (Codex P2, agreed). The audit skill is the normal entry point that
recommends remediation after a run, and it still told users to keep this
plugin's prose findings away from `review:fanout fix` because routing them
there "retires the findings without fixing them". That is now flatly false:
the crosswalk declares `/ai-slop:audit fix` as the remediation owner for the
other fourteen rules, so the relay hands those rows over instead of retiring
them. Leaving step 6 in place would have made the new route unreachable
through the documented flow while the contract advertised it, and handed the
model directly contradictory instructions.

Both stale passages corrected:

- Step 6 now recommends `review:fanout fix` for the whole file when the
  operator is already running a fix pass, and this skill's own `fix` directly
  when they are not, and names the one condition that changes the answer --
  the relay can only hand the rows over when /ai-slop:audit is available in
  that session, and surfaces them otherwise.
- The Purpose paragraph drew a real distinction in the wrong place. The
  narrowness is about what the relay APPLIES, not what it ROUTES:
  rule-utm-params is still the only row the relay applies itself, and the
  other fourteen are now routed rather than left unrouted. It keeps the true
  half -- the cleanup route never loads this skill's rewrite guide, which is
  exactly why the declaration exists.

No behavior, script, rule, or eval changed: this is the producer half of a
claim the consumer now honors. ai-slop 0.3.1 (0.3.0 is published on main via
#3063, so amending it was not available). review stays 0.26.0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 21, 2026
… skill (0.26.0) (#3068)

Closes #3033

## Summary

The `detector-findings` convention lets a producer emit a findings file
that `review:fanout`'s `fix`
action consumes. The relay then decided **how** to remediate — and for
`ai-slop:audit` that decision
was wrong for 14 of its 15 rules.

Those rows are prose-style findings. They classify as **cleanup** by
content, and `fix-pass-mode.md`
Step 4 hands the whole cleanup class to `/simplify` — a
**code**-simplification skill that, in its
own words, "rediscovers cleanups from the working-tree diff — it does
NOT read the findings files",
and which never loads
`plugins/ai-slop/skills/audit/reference/rewrite-guide.md` where that
plugin's
rewrite discipline lives. Step 5 then retired the file anyway. A relay
pass over an `ai-slop:audit`
findings file reported a clean run while applying at most
`rule-utm-params`, the one genuinely
auto-applicable rule. #3031 narrowed the plugin's own claim to say
exactly that, which was honest and
left the gap.

This teaches the contract to let a producer name the skill that owns its
findings' remediation, and
teaches the relay to honor it — **without a new field, and without any
producer changing what it
emits.**

### The question settled first: does an existing disposition already
cover this?

Asked before anything was written, because a third disposition an
existing one already covers is
pure cost. **It does not**, on two independent grounds:

1. **Off-site is a statement about the SITE, and these repairs are at
`Location`.**
`docs/conventions/detector-findings/README.md` "When the remediation is
not at `Location`" binds
its producer obligation to "a rule whose remediation can lie **outside
`Location`'s file**", and
both limbs of the consumer's trigger in `fix-pass-mode.md` "Step 2" are
site limbs — the `Action`
names a different file, or the contract declares the rule off-site.
`testing:audit`'s own adopter
row states the contrast: its `Location` "IS the remediation site".
Claiming off-site to reach the
disposition would assert something false about where the fix goes, the
same defect the doc already
forbids in retargeting `Location`. And it routes to **surface-only**,
which trades a wrong apply
   for no apply rather than closing the gap.

2. **`Auto-applicable: No` has no path to the route that actually
misapplies these rows.** Step 4's
surface-instead-of-auto-applying fence sits under its
**correctness-class** heading. Cleanup-class
rows go to `/simplify` wholesale, consulting no crosswalk cell. The
column can already say a rule
is not auto-applicable and still not stop the apply. The fresh-context
verifier confirmed this
independently: before this change, `grep -n "Auto-applicable"
fix-pass-mode.md` returned **zero
   hits** — the relay read that column nowhere.

Evidence went toward a new **disposition**, not a new **field**. No
column was added to the crosswalk
and nothing changed in the findings-file shape (`default-mode.md`
"Findings-file shape").

## Fix

**Producer side** — `docs/conventions/detector-findings/README.md`
2.4.0, new section "When the
remediation is owned by the producer's own skill". A rule whose repair
is contained to `Location` but
safe only under discipline the producer owns **leads its crosswalk
`Auto-applicable` cell with**
``No, remediated by `<invocation>` `` — the invocation written as a code
span, whose delimiters a
consumer strips before matching.

The declaration is **per rule and lives only in the crosswalk** — this
contract's own settle-once
rule applied rather than restated. "Auto-applicability is settled per
rule, at contract time" already
says a rule's remediation shape does not vary run to run, and who owns
the repair is exactly such a
fact. Requiring every emitted row to carry a copy would be the
per-finding restatement that section
forbids, and would make conformance a property of a producer's *emitter*
rather than of its rule set.

**Consumer side** — `review` 0.26.0, `fix-pass-mode.md`:

- **Step 2** gains one classification rule. It resolves the declaration
through the qualified rule id
every conforming row already leads its `Finding` cell with, against the
crosswalk registry that id
"resolves against by exact match" — the contract's own pre-existing
words. **The crosswalk
declaration is NECESSARY**: an `Action` cell leading with ``Remediate
with `<invocation>` ``
corroborates it and can never substitute for it, and a rule with no
crosswalk declaration takes its
ordinary class however its `Action` reads. Off-site is decided first,
and an unresolvable contract
is the no-declaration case — never a licence to fall back to the
`Action` cell.
- **Step 4** gains the route, with **no direct-apply fallback** — the
asymmetry with `/simplify` is
the point. Only an invocation already available in the session is
invoked; nothing is installed,
fetched, or loosely name-matched, because Step 1 already establishes
that nothing authenticates the
writer of a findings file. An unavailable or unrecognized invocation
**surfaces** its rows, naming
  what the producer asked for.
- **Steps 3 and 5** count and report the route, and the counts
**partition** rather than overlap: a
row routed to a producer-owned surface is counted there and never on the
cleanup line, mirroring
the existing correctness/surface-only rule in both the plan block and
the report block.
- Three evals added: `fix-pass-producer-owned-routes-to-named-surface`,
`fix-pass-producer-owned-surface-unavailable`, and
`fix-pass-action-cell-alone-never-routes`.

**Producer adoption** — `ai-slop` 0.3.1. The 14 non-`rule-utm-params`
rows now carry the declaration
and `rule-utm-params` stays auto-applicable, with **zero change to what
the producer emits** —
`ai-slop:audit` already leads every emitted `Finding` cell with its
qualified rule id, which is why
siting the declaration in the crosswalk closes the gap on its own.
`plugins/ai-slop/skills/audit/SKILL.md`
is corrected to match: it previously said the relay "retires the
findings without fixing them" and to
recommend `review:fanout fix` only for `rule-utm-params`, which this
change makes false. It now draws
the distinction where it actually falls — **what the relay APPLIES is
narrow; what it ROUTES is not** —
and states the one condition that changes the answer, that the relay can
only hand rows over when
`/ai-slop:audit` is available in that session, and surfaces them
otherwise.

**Neither other adopter changes, and neither was touched.**
`mutation-testing:audit` declares no
owner and is off-site, which Step 2 decides first. `testing:audit`
declares no owner because no skill
owns choosing the assertion a behavior deserves; its rows keep the plain
`No — <reason>` form and are
surfaced by Step 4's judgment fence exactly as before. All three adopter
rows now state their
disposition explicitly.

## Verification

| Gate | Result |
|---|---|
| `check-detector-findings-crosswalk.sh --check` + its self-test | PASS
— 22 rule rows, every disposition argued |
| `check-changelog-parity.sh` `--check` / `--check-bump origin/main` /
`--check-order` / `--check-preserved origin/main` | PASS (83 changelogs
newest-first; 64 headings compared) |
| `check-changed-skills.sh origin/main` | `audit: PASS — 0 errors`,
`fanout: PASS — 0 errors` |
| `check-stale-base-overlap.sh --check origin/main` | up to date with
`origin/main` |
| `check-contract-clause-coverage.py` | PASS |
| `check-contract-slice-prune.sh --check` / `--check-diff origin/main` |
PASS |
| `check-cross-plugin-source-drift.sh --check` | PASS |
| `check-evals-quality.sh` on both changed eval sets | PASS — 0 warnings
|
| `markdownlint-cli2` on every changed markdown file | PASS — 0 issues |
| `detect.test.sh` (regression check) | all 92 cases passed |
| `node scripts/generate-catalog.mjs` | Catalog already in sync;
`docs/CATALOG.md` unchanged |

### Fresh-context verification

Separate opus verifiers, in fresh context and told to judge the final
state rather than any
description of it, reviewed this. The first found **three** real
defects, all fixed: the
`mutation-testing:audit` adopter row misdescribed two of its own
`Auto-applicable` cells; the
declared lead was written as a code span in the shipped cells but bare
in the contract, so a
consumer implementing it literally would match nothing; and the
contract-reachability argument was
evidence about the **producer's** session rather than the consumer's.
Its full verdict is on

[#3033](#3033 (comment)).

### Review findings, all fixed

- **P1 — routing on an `Action` cell alone.** The original text let the
untrusted half of the pair
authorize a route by itself. Since nothing authenticates a findings
file's writer, any component
able to write one could have named any already-installed skill and
handed it arbitrary rows, with
effects bounded neither by `Location` nor by Step 2. Now the crosswalk
declaration is necessary and
the `Action` cell only corroborates — the trust boundary being that the
crosswalk lives in the
consuming repo's own docs, **outside** the artifact being consumed,
while the `Action` cell is
  inside it. **Availability is not authentication.**
- **P2 — ambiguous plan counts.** The cleanup and producer-owned lines
now partition rows explicitly,
in both the plan and the report, so a plan can no longer print
`Cleanup-class (14) → /simplify` for
  rows that never reach `/simplify`.
- **P2 — the producer steering operators off the new route.**
`ai-slop`'s `SKILL.md`, corrected above.

## Related

- #3031 narrowed `ai-slop`'s relay claim to what was then true, and left
this gap open on purpose.
- #3041 / #3063 backed the `ai-slop` audit evals with committed
fixtures; `ai-slop` 0.3.1 here builds
  on the 0.3.0 that shipped there.
- `docs/conventions/detector-findings/README.md` — "The severity
crosswalk", the `Auto-applicable`
column, "When the remediation is not at `Location`" (the precedent this
is argued against, whose
rejected remediation-target column is why no column was added here), and
the Adopters table.
- `plugins/review/skills/fanout/context/fix-pass-mode.md` — the consumer
algorithm this teaches to
  honor the declaration.

### Follow-up, deliberately not in this PR

Two `ai-slop` items are tracked separately and nothing here depends on
either. `emit-findings.sh`
writes `date: …T05-45-10Z` — hyphens where ISO-8601 needs colons in the
time portion; Step 1 classes
that UNREADABLE and fails **open**, so it costs at most one extra pass
and drops nothing. And
`docs/upstream/cursor-pstack.md` carries no `unslop` row, although it
calls itself the single source
of truth for everything derived from that upstream collection and its
recheck trigger only covers
rows its table names.

Merge ordering, disclosed rather than closed: the relay's only contract
URL points at `main`, so the
new crosswalk cells are not visible to a live relay until this merges.

---------

Co-authored-by: Claude <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 21, 2026
…on gaps (#3097)

Four corrections surfaced by adversarial verification on #3063 and #3068.

emit-findings.sh wrote date: with hyphens where ISO-8601 needs colons in the
time portion, so the value parsed nowhere. The consumer classes an unparseable
date as UNREADABLE and fails open, so it cost one extra pass and dropped
nothing, but the staleness check degraded silently. The bug was guarded by a
passing test asserting the hyphenated form; the replacement is discriminating,
failing against the old emitter output.

Two live passages described the relay's cleanup route as handing the class
wholesale to /simplify, omitting the branch that applies rows directly when
/simplify is absent. Two further occurrences sit in published changelog entries
and are left as written, with the correction noted in the new entry instead.

A contract fence sentence read unconditionally while binding the correctness
class only; now scoped, with the class-independent half preserved verbatim. It
predates #3068 by six days.

ai-slop eval case 2 grades findings-file persistence but lacked the tracked-file
premise its siblings carry. Premise added. Whether tracked-ness is a substantive
gate on the fix flow's re-emit step is recorded as an open question with the
seam named, rather than closed by an argument the cited text does not support.

ai-slop 0.3.3, detector-findings 2.4.1.
kyle-sexton pushed a commit that referenced this pull request Aug 21, 2026
…enerally

The paragraph asserted that nothing executes an eval prompt, and told authors
not to assume a runner would stage anything. Both overreach. This repo's own
surfaces are scoped more carefully: plugins/evals/skills/design/SKILL.md and
that plugin's README say no MARKETPLACE command runs model-graded evals, and
name Anthropic's skill-creator as a runner a consumer may have installed — the
runner belonging to this very format.

Left as written, the guidance would have pushed authors toward cases that are
incompatible with skill-creator's staging, which is the opposite of the intent.

The corrected text keeps what is true and load-bearing — nothing here runs
these, so a case must be followable by hand — and replaces the false half with
the actual rule the #3063 apparatus violated: declare fixtures in files[] and
reference them by their documented path, rather than hand-rolling staging in the
prompt string. A prompt that builds its own workspace is neither followable by
hand nor compatible with the runner that would otherwise stage it.

Found by review on #3102.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 21, 2026
… eval`'s (#3102)

The Evals section already stated that no first-party command executes
model-graded evals, but never said which format the 203 shipped suites are in or
how it relates to the `claude plugin eval` command — so a reader meeting that
command reasonably reads them as drift. They are not: the shipped format is
skill-creator's, which is why the schema notes upstream's `assertions` naming,
and it is the ecosystem-wide shape. The command consumes a different layout and
is early access, refusing to run, so adopting it would trade a corpus CI checks
for one nobody here can execute. Adoption stays deferred behind the same tracker
as the runner.

The authoring consequence is scoped precisely, after review caught an overclaim
in the first draft: nothing in THIS marketplace or THIS CI executes a prompt, so
a case must be followable by hand — but a consumer with skill-creator installed
can run these suites, and it stages a case's files[]. So declare fixtures in
files[] rather than hand-rolling staging in the prompt string. A prompt that
builds its own workspace is neither followable by hand nor compatible with the
runner that would otherwise stage it.

Also reconciles write-for-agents-brief.md, whose acceptance criterion named a
`claude plugin eval` suite as its settling artifact while the skill shipped with
the house format.

Found while diagnosing #3063, where three rounds of work built machinery to make
an eval prompt literally executable before establishing what actually runs one.
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.

ai-slop: back the audit evals with real fixture files instead of prose scenarios

2 participants