fix(verification): neutralize .NET/PowerShell-flavored examples in agnostic skills#492
Conversation
…nostic skills The confirm/measure skills are sold as ecosystem-agnostic, but their illustrative examples leaned .NET/C#/PowerShell and one assumed a Unix shell (`wc -l`) against the cross-platform "never assume Bash" contract. Neutralize them so a non-.NET consumer isn't handed a stack-specific illustration as the universal path: generic reproduction-test name and failure, import/dependency-declaration counting instead of `using`/`ProjectReference`, "your test runner"/"your benchmark harness" instead of `dotnet test`/BenchmarkDotNet, and shell-neutral line counting (`wc -l` on POSIX/Git Bash, `Measure-Object -Line` in PowerShell). The named marketplace-plugin evidence pointers (`dotnet-*`) are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011V31qpAHP3jfs76B9d5Rfo
|
Claude finished @kyle-sexton's task in 1m 49s —— View job PR Review — fix(verification): neutralize .NET/PowerShell-flavored examples in agnostic skills
Verdict: Approve — clean, focused, well-reasoned patch. The changes are correct and do exactly what the PR description says they do. No blocking issues. SummaryThis is a documentation-only patch (5 files, 26 additions / 11 deletions) that neutralizes .NET/C#/PowerShell-flavored illustrative examples in the ✅ What's done well
Dual-shell pattern is applied consistently (metrics.md L14, performance.md L14) Marketplace plugin evidence pointers left intact (metrics.md L69–73, performance.md L61–66) Version and CHANGELOG are consistent 🟡 Nit — parenthetical inside code fenceEmbedding 🟡 Nit — table cell verbosity in
|
…t name Address claude[bot] review nit: the "(name it per your framework's convention)" parenthetical inside the code fence could be misread as part of the test name. Move it to a trailing `#` comment delimiter so the meta-note is unambiguously commentary, not the identifier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 26s —— View job PR Review — fix(verification): neutralize .NET/PowerShell-flavored examples in agnostic skills
Verdict: Approve — the follow-up commit cleanly resolves the one nit from the prior review, and the full patch is well-scoped and correct. What changed since the last reviewCommit -Reproduction test: returns_null_when_user_is_deleted (name it per your framework's convention)
+Reproduction test: returns_null_when_user_is_deleted # name it per your framework's conventionThe Full patch — residual observationsCHANGELOG date field (
REVIEW.md blocking criteria — all six inapplicable to a docs-only patch: no object-level authorization, no tenant-scoped data, no secrets, no injection surface, no audit-log requirements, no multi-file atomic write. SummaryBoth commits are clean. The original neutralization was well-reasoned; the follow-up precisely resolves the single nit. Ready to merge. |
|
Thanks for the review — verdict noted as Approve. Classification of the two 🟡 nits:
Head is now |
…ls (#531) (#609) ## Summary Skills declared ecosystem/forge/tracker-agnostic ship bare hardcoded stack/forge/branch/tracker defaults because agnosticism was asserted in prose and never enforced mechanically — the dominant review-churn class, re-caught by the external reviewer PR after PR (branch/remote hardcodes, seam-conditional eval, .NET-flavored examples). This adds a fail-closed CI lane that mechanically detects that coupling instead of paying for it at review time again and again. Detection only — this PR builds the gate. Fixing the existing violations stays with the member issues below. ## Fix A new `portability-lint` lane (`.github/workflows/ci.yml`) wired into the required `ci-status` aggregate, plus a repo-local detector: - **`scripts/check-skill-portability.sh`** — scans skill files for coupling tokens. On a PR it scans only the skill files the change **touches** (mirroring the `skill-quality-gate` changed-diff pattern), so enabling a token class prevents **new** coupling without red-lining pre-existing violations — main's push event scans nothing (self-test is the push path), and existing hits wait for their owning follow-up fix or the file's next edit. - **`scripts/skill-portability-tokens.txt`** — the token list as external, extensible **data** (not logic buried in bash), so a reviewer re-catch is a one-line data edit. Seeded with **one active class** — the branch/default-branch hardcode (`origin/main` / `origin/master`) — per the ratified one-token-class-at- a-time rollout; further classes (`dotnet`/`Clean Arch`, `raw.githubusercontent`, bare `gh` tracker calls) are staged as commented entries with enable-triggers. - **Never-skip shape**: the job is unconditional; only the PR-diff step is event-gated, and a self-test step runs first so a broken detector can never mask a real violation behind a green gate. **Design decisions** (resolved per the ratified plan, not invented): - *How a skill declares agnosticism scope* — **no new frontmatter field**. A skill is agnostic by default (the Design boundary already binds every plugin), so the gated set is the files a change touches. A hit is excused three ways, all reviewer-visible comments (reusing the silent-skip gate's annotated-exemption shape): an auto-recognized detection-first / presence-gated use; a per-site `portability-ok: <reason>`; or a whole-file `portability-scope: <reason>` declaring an inherent narrower boundary (the forge-locked-under-a-neutral-name case). This distinguishes **guarded** refs (fine) from **bare** ones, as the guarded-forward-ref disposition requires. - `docs/PLUGIN-PHILOSOPHY.md` gains one doctrine sentence extending the existing declared-narrower-boundary allowance from OS platform to the forge/ecosystem/tracker axis. No `plugins/<name>/` directory is touched, so no version bump / CHANGELOG entry is needed (matching the CI-gate precedent). ## Verification - **Self-test suite** (`scripts/check-skill-portability.test.sh`, runs in CI): `PASS=12 FAIL=0` — covers bare-token FAIL with file:line, detection-ladder pass, same-line and comment-block-above `portability-ok`, annotation non-leak past intervening code, whole-file `portability-scope`, staged tokens staying inactive under the shipped list, fail-closed exit 2 on missing token list / invalid base ref, the vendor/evals/`*.test.sh` exclusion set, and empty scope. - **Catches a real violation, passes a real legit use** (the bare-vs-guarded discrimination that drives this class), on live corpus files: ``` $ scripts/check-skill-portability.sh --paths plugins/work-items/skills/track/actions/start.md COUPLING: plugins/work-items/skills/track/actions/start.md:61: origin/(main|master) -> ... git checkout -b <type>/<N>-<slug> origin/main ... COUPLING: plugins/work-items/skills/track/actions/start.md:65: origin/(main|master) -> ... git checkout -b <type>/<N>-<slug> origin/main ... (exit 1) $ scripts/check-skill-portability.sh --paths plugins/review/skills/fanout/SKILL.md No unexcused coupling tokens in 1 skill file(s). # detection-first origin/HEAD ladder → guarded (exit 0) ``` - **Calibrated against the full corpus** (`--all`): the active branch class flags exactly 3 genuine bare hardcodes in 2 files (owned by member issues; changed-file scoping keeps them off main) and correctly passes the detection-ladder uses in `review/*` and excludes evals/test fixtures. - **Changed-file CI path proven** against a real base ref (16 changed skills gated, all clean). Local checks green: `shellcheck` (repo `.shellcheckrc`), `shfmt`, `actionlint`, comment-residue detector (T1=T2=T3=0), `typos`. Closes #620 Part of #531 (umbrella stays open — stages remain; child #620 carries this PR's shipped scope per the #603 pattern). ## Related - Member coupling instances (this is prevention; it does not block them): #404 #405 #406 #408 #410 #412 #415 #416 #418 #421 #422 #423 #428 #429 #432 #438 #439 #441 #442 - #445 — sibling CI-gate backlog (mechanical conformance only; explicitly not coupling) - #412 — guarded-forward-ref disposition the gate honors (guarded ≠ bare) - #441 — declared narrower-scope exemption the `portability-scope` mechanism serves - #467 — the branch/remote review-churn the seed active class targets - #453 — seam-conditional eval re-catch of this class - #491 #492 — .NET-flavored-example hand-fixes a future staged class would obviate - #611 — post-green review finding (annotation-carry pending_annot doesn't distinguish inline vs block HTML comments), deferred out of this PR's scope 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KyLj6oaFVTE6xFuoYCC2KC --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
The
verificationplugin'sconfirmandmeasureskills are sold as ecosystem-agnostic, but their illustrative examples leaned .NET/C#/PowerShell, and two "how to check" cells assumed a Unix shell (wc -l) against the cross-platform "never assume Bash" contract. A non-.NET consumer was handed a stack-specific illustration as the universal path. This patch neutralizes the examples while keeping the guidance concrete and useful.Fix
skills/confirm/context/fix.md— reproduction-test example now uses a generic descriptive name (returns_null_when_user_is_deleted, "name it per your framework's convention") and a generic failure ("the reported failure (e.g. a null-dereference crash)") instead ofUserService_ShouldReturnNull_WhenUserIsDeleted/NullReferenceException.skills/measure/context/metrics.md— coupling row counts "import/dependency declarations (import/require/using, package or project references)" instead ofusing/ProjectReference; coverage row points to "your test runner's output" instead ofdotnet test; bothwc -lsites are now stated shell-neutrally (wc -lon POSIX/Git Bash,Measure-Object -Linein PowerShell), mirroring the both-forms pattern established in fix: state shell requirements with a Windows path across 8 plugins #331.skills/measure/context/performance.md— metric "how to measure" cells point to "your test runner", "your build tool", "your platform's profiler", and "your benchmark harness", with the shell timer given for both shells (timeon POSIX/Git Bash,Measure-Commandin PowerShell) instead ofdotnet test/dotnet-counters/BenchmarkDotNet as the only path.dotnet-*:*) at the bottom of bothmeasurefiles are unchanged — those are real named plugins used as evidence sources, not universal-path illustrations, and verification: .NET/C#/PowerShell-flavored examples and Unix-shell assumptions in stack-agnostic skills (low severity) #424 deliberately did not flag them.verificationbumped0.2.2→0.2.3(docs = patch) with a CHANGELOG entry.Verification
Gates run in this Windows environment against the changed markdown; all clean, nothing suppressed:
markdownlint-cli2 v0.23.0 (markdownlint v0.41.0)(repo-pinned) on the four changed.mdfiles —Summary: 0 error(s).typos-cli 1.44.0on the four changed.mdfiles — exit 0, no output.editorconfig-checker v3.8.0on the changed.mdfiles plusplugin.json— exit 0, no findings.plugin.jsonre-parsed as valid JSON; version reads0.2.3.Related
Closes #424. Sibling stack-bias issues in the same work-readiness sweep, same pattern: #430, #429, #428 (all
testingplugin).🤖 Generated with Claude Code