Repository navigation
refactor(plugins): delete the 10 external-tool check and check-prerequisites skills (*:check wave 3) - #12770
refactor(plugins): delete the 10 external-tool check and check-prerequisites skills (*:check wave 3)#12770kyle-sexton wants to merge 22 commits into
Conversation
…, part B) Delete skills/check for harness-ops, testing, session-flow, context-budget, desktop-notification, guardrails, eol-normalizer, source-control and autonomy, with their probe files. The node-missing notice now names /harness-ops:prerequisites, and each plugin's notice ends with its own bundled checker command. Rewrite the eval cases so the prompt carries no consent and quotes the text a real notice prints; each carries no-install, no-edit and no-write graders. Refs: #12486 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
Judge calibration passed "jq is installed" as a result and failed a named "Both PASS". The rubrics now say what counts; all four llm graders agree with their labels on every calibration run. Refs: #12486 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
The reference check reads a slash reference as a capability that must resolve, and these skills are gone. Each fragment already sits under its plugin. Refs: #12486 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
…-delete-check-skills
The drift check fails on three samples the 12486 eval cases share byte for byte. Register them beside the existing ck-notice entries. Refs: #12486 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
Deleting the check skills leaves that column empty for those plugins, and the skill told agents it always names a skill to run. Refs: #12486 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
…k wave 2) Delete skills/check for actionlint, bash-format, biome-format, go-format, markdown-format, powershell-format, ruff-format and typos-format, with their evals and actionlint's probe set. plugins/*/skills/check goes from 17 to 9. Each plugin's session notice already names its bundled checker. Rewrite the 3 actionlint baseline cases and add one plugin-level case for each of the other 7: the notice alone, no consent, no install, no edit, no write. Refs: #12487 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
Refs: #12487 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
merge=union kept main's owner line beside this branch's edited one, so check-skill-leaf-names failed with check registered twice. Refs: #12487 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
…h tests ck-passing-no-successor matched a client name and verb anywhere in the command, so a grep, an echo or a curl -sI failed a run that installed nothing. It now uses the command-position matcher its sibling cases use. The fake-plugin rename left the session-latch tests calling one plugin twice; a latch keyed per plugin passed them. Refs: #12487 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
…ck wave 3) Delete `check` in context7, disk-hygiene, explainer-video, knowledge, playwright and speech, and `check-prerequisites` in animation, instruction-placement, skill-quality and toolchain, with their probes. Notices and skills now name the bundled checker, or speech's check.py and explainer-video's pydeps.py check where the checker cannot see the state. context7 declares its API key and MCP server; disk-hygiene declares Python. Refs: #12488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
…check-skills' into refactor/12488-delete-external-tool-check-skills
Two after-arm legs went INVALID when the run tried to Read a guessed absolute path for the checker. With the command in the prompt it runs the checker instead. Refs: #12488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
context7 ck-notice-alone-installs-nothing takes ck-read-only's no-install matcher, so it catches npx ctx7, with that as a fail sample. disk-hygiene's no-interpreter and guard messages end in the shared "diagnose with:" form and no longer name /disk-hygiene:setup, which the prerequisites contract says a notice does not name. Refs: #12488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd
…-external-tool-check-skills
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32da30a17a
ℹ️ 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".
|
claude-security-review reviewed Chunk 1 of 4 (33 files)Security review, chunk 1 of 4 (PR #12770 at
|
|
claude-review reviewed Chunk 1 of 4 (33 files)Code review: chunk 1 of 4 (33 files), no findingsI read every hunk in chunk 1. That covers 11 changelog fragments, What I checked in the repository at head
Closing issue. Author-claimed, not verified by me. The PR's paired eval verdict, the list of gates it says pass ( Spend, not a hold. The new and renamed eval cases use prepaid API credits whenever someone runs them (the PR reports about 6.36 USD for its own runs). No billing setting or billed resource changes. There are no prior findings from this lane, so the round cap does not apply. I saw no embedded instructions in the PR context. Chunk 2 of 4 (30 files)Code review: chunk 2 of 4 (context7, disk-hygiene, explainer-video)I read all 30 files in this chunk in full. For context I also read the bundled checker ( 1 finding (advisory, inline): the new
Checked, no issue:
Manifest (
Closing issue: the PR refs #12488 but closes no issue in the PR context file, so I had no acceptance criteria to check. Verification in the PR body (paired eval runs, gate results) is author-claimed; I did not run any of it. Spend (not a hold): the new and changed eval cases spend subscription/API usage when run (up to 12 turns and 300 s per case). No billable change. Prior findings: none (round 0 of a cap of 3). Chunk 3 of 4 (30 files)Code review: chunk 3 of 4 (30 files)No findings in this chunk. I read every hunk of the chunk's 30 files: harness-ops probes, instruction-placement, knowledge, playwright, skill-quality, and the first part of speech. That count includes the deleted What I checked
Plugin manifests (
|
…ment The registry policy test reads each comment block as belonging to the one entry that follows it, so the eight ck-notice entries after the first had no "No dedicated check" / "Canonical copy:" annotation and failed check-cross-plugin-source-drift.test.sh. Give each entry its own block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vcupom3BKF6QuC5U3sRJPj
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KeBVgcHpe4YpFuL5b24Ft2
|
CI fix pushed by PR Lane Automation, on top of 32da30a (no commits of yours were rewritten).
Local: the drift test passes 15/15 and I did not touch the five open review threads (the |
- explainer-video produce: keep the decision not to launch through py and point at the Python docs section instead of restating what a py launch does. - explainer-video produce and speech narrate: name /harness-ops:prerequisites, the fleet prerequisites report, as the successor in ## Next now that the per-plugin check skills are gone. - disk-hygiene prerequisites.json: the checker judges only the first interpreter found (detect.any is first-match-wins in the prerequisites contract), while the guard tries python3, python, then py -3 against its floor. The degrade text now says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vcupom3BKF6QuC5U3sRJPj
|
Review threads settled by PR Lane Automation in fde3192, pushed on top of f566bbb (no force push).
Local checks: |
Refs: #12488
Summary
Wave 3 of
*:check. It deletescheckin context7, disk-hygiene, explainer-video, knowledge, playwright and speech, andcheck-prerequisitesin animation, instruction-placement, skill-quality and toolchain, with 5 probe files.plugins/*/skills/checkgoes from 9 to 3,plugins/*/skills/check-prerequisitesfrom 4 to 0, and both of the issue's done-when greps print nothing. This is 259,485 bytes (git diff --no-renames) against the 262,144 cap, measured against merge base 54f1af6a56. It replaces the 260,828 figure: that comment says the 4 playwright samples moved out; they are back in, and the 10 control-skill READMEs moved out instead.Fix
Notices and skills now name the bundled checker. speech names
scripts/check.pyand explainer-video namesscripts/pydeps.py check, because the checker cannot see their packages or model. context7 declaresCONTEXT7_API_KEY(env) and its MCP server (mcp). disk-hygiene declares Python, so the checker no longer reports PASS without looking at it. Its launcher, guard and monitor messages end in the shareddiagnose with:form and do not name/disk-hygiene:setup, which the prerequisites contract says a notice does not name.Behaviour change: the model no longer has a skill for these:
ctx7now FAILs even when MCP is live), thenpm viewlatest-version comparison, the MSYS row, and a run ofctx7 --version. A smoke run saw the checker PASS on a mise shim that does not run./disk-hygiene:setup check, which the user types./playwright:setup check.Shipped this way because these are read-only reports the user can still run and the point-of-use failures still fire. The eval below does not exercise them.
Which plugins emit a live notice:
run-python-hook.shdeny textnarrate.pyexit-2 textrender.pyexit-2 text, names no checkerpydeps.py checkNone calls
hook::requireorhook::prereq_notice_to. instruction-placement declaressession_probebut has nohook:entry.Eval case changes:
ck-install-routedis nowck-notice-alone-installs-nothing, because the prompt no longer asks for an install.ck-notice-reads-checkerin 7 plugins, andck-notice-runs-own-reportin speech and explainer-video. All use the anchoredno-installfrom wave 2'sck-passing-no-successor, not the unanchored regex in test(evals): anchor no-install in the 7 formatter ck-notice cases #12645. Both context7 cases add context7's extra runners, withnpx ctx7as a fail sample.version-checked,routes-to-install-cli,status-table, all of which needed the deleted skill or an install request.ck-read-onlygainsreports-auth-and-mcp.no-install,no-editandno-write, with Write and Edit granted. No grader is judged.Verification
Read this first: the paired run shows the after arm handles notice text. It cannot show "no loss". The restored skill was listed in all 11 before-arm traces. It was called in 1 of 33 runs in the final before-arm files, on disk-hygiene, plus 2 of 3 in a replaced disk-hygiene attempt. The losses above come from reading the deleted SKILL.md files.
Same 11 cases on both arms, Sonnet subject, 3 runs per case,
--scaffold. The before arm is the after tree with the skill directory restored (from the merge base for the 2 re-run cases, from wave 2's tree for the other 9), not main. Judged withrun-validity.pyandnoise-report.pyfrom an export ofplugins/evalsat the merge base,--scaffolded,--margin 0.05. 3.49 USD for the final paired runs, about 6.36 USD with replaced legs and smoke.Verdict, 10 of 11 cases: change +0.00, 95% interval +0.00 to +0.00, non-inferior at margin 0.05. 21 of the 22 result files are VALID. disk-hygiene is left out of the verdict because its before arm is INVALID.
no-install,no-editandno-write: 0 failures in 99 grader results per arm, that leg included. In the final files no grader missed on either arm.disk-hygiene: after the deny text moved to the
diagnose with:form, the after arm ran the checker in 3 of 3 runs, up from 0 of 3 with the old text. The before arm was run twice under a stopping rule fixed in advance: one further attempt, then leave the leg out. Both attempts are INVALID, 3 denials and then 1. Each Bash denial is the same shape: the restored skill sends the model to the setup check, it builds a compound command naming the engine script, and the plugin's ownBash(*hygiene.py*)guard denies it. The first attempt also had one Glob denied under dontAsk. Every denial came from a run that called the skill. In the first attempt both such runs missedran-checker; in the second the one such run reached it. The skill fired in 0 of 3 before-arm runs with the old text; why it fires now is UNVERIFIED. No grader or gate was changed to make this leg count, and its issue done-when item (non-inferiority across all 11) is met only for the other 10.Limits:
install-python-deps.shwas stubbed toexit 0in both arms' scratch copies for animation, speech and explainer-video, so runs did not install their packages.Gates:
validate-caseson the 10 suites,check-matchers(no case here has amatchers.json, so theno-installsamples were also graded under node separately, 0 wrong),check-cross-plugin-source-drift --check,check-skill-leaf-names --check,validate-plugins,sync-shared-copies --check-bump origin/main,check-changelog-fragments --check-required origin/main,check-skill-count-claims --check,generate-cheatsheet --check,check-declared-prerequisites,lib/prerequisites.test.sh,scripts/node-notice-rows.test.sh, the disk-hygiene and speech tests. All pass except one strace census assertion inrun-python-hook.test.sh, which fails the same way on the merge base.shfmt -don the launcher and its test reports the same counts as the merge base.Post-merge probe is the orchestrator's: the skill picker lists none of these 10 skills, and one hidden tool produces a notice whose command runs.
Left out: a live missing-tool leg, models other than Sonnet.
Related
no-installregex.Remaining
Issue 12488 closes by hand after this merges and its merge probe is posted. Moved to a follow-up to fit the cap: the
hook::prerequisite_fields_tofallback and its 20 copies, and the control-skill bullets for these 10 skills.🤖 Generated with Claude Code
https://claude.ai/code/session_01VQb9Q4XhhsHcqrFVqRfStd