fix(verification): presence-gate dotnet/cloudflare cross-plugin refs in measure contexts - #526
Conversation
…in measure contexts The `measure` skill's `metrics` and `performance` contexts named `dotnet-*` and `cloudflare` marketplace-plugin skills as evidence sources with no presence gate and no fallback — the bare unguarded cross-plugin reference `docs/PLUGIN-PHILOSOPHY.md` names as a defect. None of these plugins exist in this marketplace; other refs in the same plugin (`/toolchain:*`, `/testing:run-e2e`, `/review:quality-gate`) are already gated. Both lists now carry the `## Marketplace plugin skills (invoke only when installed)` guard heading (mirroring the `testing` plugin's gated lists, #491) plus a stack-qualifier lead-in that frames the `dotnet-*` skills as .NET-only and `cloudflare:web-perf` as web-frontend-only, each invoked only when its plugin is installed and otherwise falling back to the project's own tooling — the same install-gate + fallback idiom `confirm/SKILL.md` already uses. No hard dependencies; every reference stays optional. Closes #422 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 38s —— View job PR Review
Overall: Correct fix, clean execution. Ready to merge with one minor note. This PR does exactly what Guard conformanceThe seam-phrasing convention requires three elements at each instructed invocation: gate, fallback, and ownership framing. Both sites deliver all three:
This is strictly more complete than the testing sibling (which uses Correctness of the mixed-stack section in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cfce709c2
ℹ️ 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".
…e contexts The presence-gated marketplace-skill lists in `measure`'s metrics and performance contexts shared one generic lead-in fallback, but two bullets have evidence types that generic fallback cannot supply: - `dotnet-data:optimizing-ef-core-queries` (N+1 / query plans) — complexity / coverage tooling reveals neither; point to query logging, database profiling, or ORM diagnostics instead. - `cloudflare:web-perf` (Core Web Vitals) — a benchmark/profiling harness targets code perf, not web vitals; point to Lighthouse / PageSpeed Insights / a CI web-perf runner instead. Each divergent bullet now carries a tailored per-bullet fallback; the generic lead-in still covers the bullets it fits. CHANGELOG 0.2.4 entry updated to describe the per-bullet fallbacks. Addresses codex P2 (EF-query fallback) and the claude[bot] web-vitals precision note on #526. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 22s —— View job PR Review
Overall: All prior review notes addressed. Ready to merge. This update adds a second commit ( What changed in the follow-up commit
CHANGELOG updated in the same commit to describe the per-bullet fallbacks alongside the guard addition, so 0.2.4 accurately represents the final state (not the intermediate state after only the first commit). Guard completeness — final state
Every bullet that has an evidence type the lead-in fallback cannot cover now has a dedicated inline fallback. The bullets where the lead-in fallback is sufficient (CRAP scores, test anti-patterns, code-level perf, microbenchmarks, build perf) inherit the lead-in without redundancy. This is the right granularity. No concerns
|
|
Addressing the review — thanks for the thorough pass.
Fixed in e8d938e. The generic benchmark/profiling lead-in fallback is retained for the Note: this same commit also addresses the codex P2 EF-query fallback (replied on that inline thread). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8d938ebdc
ℹ️ 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".
The `dotnet-test:test-anti-patterns` bullet still deferred to the generic complexity/coverage lead-in fallback, but that tool category cannot surface the test smells this evidence source assesses — over-mocking, flakiness, and tautological tests all persist while coverage and complexity improve. The bullet now carries its own fallback: the project's own test-quality analyzer or an explicit test-smell review checklist. CRAP scores remains on the generic lead-in fallback (it genuinely is complexity + coverage). CHANGELOG 0.2.4 entry updated. Addresses codex P2 (test-quality fallback) on #526. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 10s —— View job PR Review
Overall: All review notes addressed. Ready to merge. This commit ( What changed in dd93761
The explicit enumeration of failure modes (over-mocking, flakiness, tautological tests) is exactly right — it rules out the false-positive escape hatch where coverage and complexity both improve while the tests remain worthless. CRAP scores deliberately retains the generic lead-in fallback — the commit message explains the reasoning: CRAP = cyclomatic complexity + coverage, so the generic "complexity/coverage tooling" fallback IS equivalent evidence for that bullet. This is a correct judgment call, not an oversight. CHANGELOG updated to add Final guard completeness —
|
| Bullet | Fallback strategy | Correct? |
|---|---|---|
| CRAP scores | Generic lead-in (complexity + coverage) | ✓ — CRAP is literally that |
| Test quality | Per-bullet: test-quality analyzer / test-smell checklist | ✓ — generic can't surface smells |
| EF Core queries | Per-bullet: query logging / DB profiling / ORM diagnostics | ✓ — fixed in e8d938e |
All three bullets now have fallbacks matched to the actual evidence type the plugin skill would have supplied. No bullet relies on a fallback that cannot substitute for it.
No concerns
- CHANGELOG describes the final state accurately.
- Version
0.2.3 → 0.2.4remains the correct tier. - No dependencies added; no
cross-plugin-source-registry.txtentries affected.
…efs, keep tool names (#405) Rework per the control-tower ruling relay (2026-07-20) on this PR: the ratified #412 disposition governing #405 requires KEEPING the dotnet-* tool names and the presence gate, presence-gating the forward references, and adding stack-qualified framing — not genericizing the references away. The prior commit on this branch genericized the three mode-context marketplace sections into ecosystem-neutral capability descriptions, which is the OPPOSITE of the disposition. Restore each context file's `## Marketplace plugin skills (invoke only when installed)` heading and its exact dotnet-* bullets, and add a lead-in that frames those skills as .NET-ecosystem forward references — invoked only when your stack is .NET and the plugin is installed — with an explicit fallback to the project's own tooling so the generic path stays first-class. This matches the conforming testing (#491) and verification (#526) pattern. No reference removal; every reference stays optional and installed-gated. CHANGELOG 0.7.3 entry rewritten to describe the stack-qualification rework; version bump 0.7.2 -> 0.7.3 (from current origin/main) unchanged. Sibling PR #583 (toolchain, issue #412) is reworked in parallel under the same ruling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ard references (#412) Reverses this branch's earlier genericized approach, which implemented the OPPOSITE of the ratified #412 disposition. Per the maintainer ruling (relayed 2026-07-20): the `dotnet-msbuild:*` build-diagnostics skills in `plugins/toolchain/skills/check/context/dotnet.md` are optional FORWARD references to the maintainer's planned `dotnet-*` plugin family — keep the tool names, presence-gate them, add stack framing, and never remove a reference. Restore every `dotnet-msbuild:*` skill name and the `## Marketplace plugin skills for build diagnostics (invoke only when installed)` heading, and add a lead-in that frames the list as .NET-ecosystem skills applicable when the stack is .NET and as forward references to the planned family — invoked only when the plugin is installed, otherwise falling back to the section's own prose remediation and binlog gotcha (the generic path stays first-class). This matches the presence-gated forward-reference convention the merged `testing` (#491) and `verification` (#526) siblings adopted. Framing only: no skill reference is removed, renamed, or genericized, and no command string is altered. The CHANGELOG entry is rewritten to a `### Changed` note describing the reframing; version stays at the 0.4.3 bump. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… as .NET forward references (#412) (#583) ## Summary `plugins/toolchain/skills/check/context/dotnet.md` lists `dotnet-msbuild:*` build-diagnostics plugin skills (`build-perf-diagnostics`, `binlog-failure-analysis`, `msbuild-antipatterns`, and others) under a `## Marketplace plugin skills for build diagnostics (invoke only when installed)` presence gate. No `dotnet-*` plugin ships in this marketplace today — but per the **ratified #412 disposition** (maintainer answer 2026-07-19, control-tower relay 2026-07-20) these are **optional FORWARD references** to the maintainer's planned `dotnet-*` plugin family (`.NET` is the primary application stack), not stale references to remove. ## Fix Add a lead-in to the presence-gated list that frames it as .NET-ecosystem skills — applicable when the stack is .NET — and as forward references to the planned `dotnet-*` family: invoke each only when its plugin is installed, otherwise fall back to the section's own prose remediation and binlog gotcha (the generic path stays first-class). **Framing only** — the ratified disposition is *keep tool names + stack framing, presence-gate forward references, no reference removal*: - Every `dotnet-msbuild:*` skill name is kept verbatim. - Nothing is removed, renamed, or genericized. - No command string is altered (`dotnet build /bl:{}` unchanged). This matches the presence-gated forward-reference convention the merged siblings **#491 (`testing`)** and **#526 (`verification`, issue #422)** adopted — each added a ".NET-ecosystem plugin skills — applicable when your stack is .NET" framing line over the same `dotnet-*`/`dotnet-diag:*` lists. Bumps `toolchain` 0.4.2 → 0.4.3 (patch) with a matching `### Changed` CHANGELOG entry. ### Rework note An earlier revision of this branch took the OPPOSITE approach — it genericized the `dotnet-msbuild:*` names into ecosystem-neutral capability descriptions, reading #412's original "replace with capability descriptions" fix-direction literally. The control-tower ruling relayed on this PR (2026-07-20) identified that as the inverse of the ratified disposition and required rework to the #491/#526 keep-tool-names pattern. This revision implements that rework. Sibling PR #580 (issue #405, `implementation`) is governed by the same ruling. ## Verification Cumulative branch diff vs `main` is exactly: the framing lead-in sentence, the version bump, and the CHANGELOG entry — no other change to `dotnet.md`. Validators run on the merged tree (branch rebased current via a `main` merge), all clean, nothing suppressed: ``` markdownlint-cli2 (dotnet.md, CHANGELOG.md) 0 error(s) typos (3 touched files) exit 0 editorconfig-checker (3 touched files) exit 0 check-skill-portability.sh origin/main No unexcused coupling tokens in 1 skill file(s) validate-plugins.sh All plugin manifests and the catalog validated validate-plugin-contracts.mjs 33 setup skills and 1758 plugin files checked ``` Closes #412 ## Related - Closes #412 (source issue — `dotnet-msbuild:*` forward references in `toolchain`'s `dotnet.md`). - #405 (`implementation`) / PR #580 — sibling finding, same pattern, same ruling, fixed in parallel. - #491 (`testing`) and #526 (`verification`, issue #422) — merged siblings whose keep-tool-names + stack-framing pattern this PR now conforms to. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…or refs ecosystem-neutral (#580) ## Summary Reworked per the ratified #412 disposition (which governs #405): the implementation plugin's mode-context files KEEP their `dotnet-*` marketplace-skill names and the `## Marketplace plugin skills (invoke only when installed)` presence gate, and gain a stack-qualified lead-in with a generic-path fallback — the #491/#526 conforming pattern. An earlier revision of this branch genericized/renamed those references; that approach was rejected by the maintainer ruling and has been fully reverted (commit 08f8e2c). ## Fix - feature.md / bugfix.md / refactor.md: all six `dotnet-*` skill references retained verbatim under the intact presence-gated heading; a .NET-ecosystem forward-reference lead-in added ("stack-qualified, invoke only when installed, otherwise the generic path applies"). No reference removal, no renames, no command-string changes. - CHANGELOG [0.7.3]: describes the stack-qualification accurately ("retain their dotnet-* names… No reference removal"). - plugin.json 0.7.2 → 0.7.3 (patch, from current main). ## Verification - CI green (all checks incl. portability-lint); the review bot independently re-reviewed the reworked diff and confirmed conformance to the #491/#526 pattern and the #412 disposition. - Zero review threads; `git diff origin/main` on the three context files shows only heading-preserved + lead-in-added. Closes #405 ## Related - #412 (the governing maintainer disposition; sibling PR #583 applies the same ruling to toolchain/dotnet.md) - #491 / #526 (merged precedent pattern for presence-gated stack-qualified references) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…producer (#546) (#568) ## Summary `block-hook-bypass.sh` (PreToolUse:Bash) exists to block Bash file-write workarounds that circumvent the Write/Edit hook gates. Its `echo > file` detector matched an `echo`/`>` **token co-occurrence anywhere in the command string** and conflated it with the `echo "content" > file` content-authoring bypass it is meant to catch. That over-broad heuristic blocked legitimate commands that redirect a **subprocess's stdout** to a scratchpad data file while an unrelated `echo` status line lives elsewhere in the same compound call — and any command whose **quoted string arguments merely mention** the tokens (e.g. a `gh issue create --body "…"` whose text describes the bug). Observed while PR-babysitting #526. ## Fix Two root-cause changes to `plugins/guardrails/hooks/block-hook-bypass.sh`: 1. **Producer-scoped redirect detection** (`producer_redirect_bypass`). The anywhere-co-occurrence (`_echo_redir` present AND `_echo_file_out` present) is replaced by segment analysis: the literal-stripped command is split into simple-command segments on shell separators (`; | & ( )` and newlines), and a segment is flagged only when its **command word is `echo`/`printf` AND that same segment redirects stdout into a real file**. The redirect's producer must therefore *be* the echo/printf — not a co-located but unrelated one. Leading compound-command keywords (`do`/`then`/`else`/`{`) are peeled first, so a producer inside a loop, conditional, or brace-group body is still caught (`for …; do echo x > f; done`). The existing fd/stderr (`2>…`, `2>&1`, `&>`) and `/dev/null` exemptions are preserved unchanged. 2. **Multi-line quote-aware literal strip** (`strip_literals`). The per-line `sed` quote strip is replaced by a char-by-char scan that **carries an open quote across physical lines**, honoring bash quoting (single quotes take no escapes; inside double quotes a backslash escapes the next char). A `--body "…"` argument spanning newlines is now inert end-to-end instead of leaking its `echo`/`>` tokens from the second line on — the exact form that forced the reporter to fall back to `--body-file`. Heredoc-body handling and all its regression cases are untouched (heredoc opener detection is now gated to run only outside a quoted span). `printf … > file` content-authoring is now caught alongside `echo … > file` (both are the producer set named in the issue's suggested fix direction). The telemetry `form` stays `echo-redirect`, so the logged payload shape is unchanged — the schema/example files are left alone. This is a narrowing of the guard's match, not a disabling of it: every true-positive content-authoring form still blocks (see Verification). ## Verification Both actually run in this worktree. **Test suite** — `bash plugins/guardrails/hooks/block-hook-bypass.test.sh` → **`PASS=48 FAIL=0`** (37 pre-existing + 11 new). New cases: Allowed (were false-positives): - `script stdout capture + echo status (allowed)` — `bash fetch-all-pr-comments.sh 526 > pr526.json && echo "EXIT: $?"` - `script stdout capture; echo status semicolon (allowed)` - `poll-loop redirect + echo summary (allowed)` — redirect + status `echo` in a loop body - `gh issue create --body mentioning echo > file, single line (allowed)` - `gh issue create --body mentioning echo > file, multi-line (allowed)` Still blocked (true positives, unchanged): - `echo content > file still blocked`, `printf content > file (blocked)` - `echo > file after an unrelated command (blocked)` - `echo > file in for-loop body (blocked)`, `… in if-then body (blocked)`, `… in brace group (blocked)` All prior heredoc / here-string / stderr-redirect / kill-switch / telemetry regression cases remain green. **Linters** (repo-pinned, no suppressions added beyond the existing `SC2016` command-under-test convention): - `shellcheck --rcfile .shellcheckrc` on `block-hook-bypass.sh` and `block-hook-bypass.test.sh` → clean (exit 0). - `markdownlint-cli2` on `CHANGELOG.md` → `Summary: 0 error(s)`. - `typos` on the changed shell + markdown → exit 0. - `editorconfig-checker` on all four changed files → exit 0. - `scripts/validate-plugins.sh` (schema + version-bump validation) → `All plugin manifests and the catalog validated.` Closes #546 ## Related - Source: #546 (the false-fire self-observation this fix closes). - #545 — the sibling `flag-commit-pr-skill-bypass` advisory false-fire of the same class. **Not fixed here** (out of scope; it has its own separate detection logic and does not share `strip_literals`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
The
verificationplugin'smeasureskill billeddotnet-*andcloudflaremarketplace-plugin skills as evidence sources in two measure contexts with no presence gate and no fallback — the exact "defect"docs/PLUGIN-PHILOSOPHY.mdnames (every cross-plugin reference must be declared or presence-gated). None of the referenced plugins exist in this marketplace, while every other cross-plugin ref in the same plugin (/toolchain:*,/testing:run-e2e,/review:quality-gate) is already gated. This PR brings the two stray lists under the same guard and reframes them as stack-specific, keeping every reference optional and installed-gated — no hard dependencies added.Fix
Guard heading adopted verbatim from the
testingsibling (#491) on both lists:## Marketplace plugin skills (invoke only when installed). Each list gains one stack-qualifier lead-in ending in a colon, carrying the install-gate + fallback idiomconfirm/SKILL.mdalready uses ("when the plugin is installed … else the project's own tooling").plugins/verification/skills/measure/context/metrics.md:68(all .NET:dotnet-test:crap-score,dotnet-test:test-anti-patterns,dotnet-data:optimizing-ef-core-queries) — lead-in:These are .NET-ecosystem plugin skills — invoke each only when your stack is .NET and its plugin is installed; otherwise draw the same evidence from the project's own complexity/coverage tooling:plugins/verification/skills/measure/context/performance.md:61(mixed:dotnet-diag:analyzing-dotnet-performance,dotnet-diag:microbenchmarking,dotnet-msbuild:build-perf-baseline,cloudflare:web-perf) — lead-in:These enrichment skills are stack-specific — the \dotnet-*` skills apply when your stack is .NET, `cloudflare:web-perf` when you ship a web frontend; invoke each only when its plugin is installed, otherwise draw the same evidence from the project's own benchmark/profiling harness:(deliberately **not** a blanket .NET label, sincecloudflare:web-perf` is not .NET).verification0.2.3 → 0.2.4(plugin.json) with a matching## [0.2.4] / ### ChangedCHANGELOG entry that explicitly supersedes 0.2.3's "thedotnet-*evidence pointers are unchanged" note.A completeness grep for
dotnet-|cloudflareacrossplugins/verification/confirms these two## Marketplace plugin skillsheadings are the only occurrences — no stray inline references (the trap #491 hit inwrite.md).Verification
Ran on the changed files, all green, nothing suppressed:
markdownlint-cli2(repo config.markdownlint-cli2.jsonc) on the 3 changed md files —Linting: 3 file(s) / Summary: 0 error(s).editorconfig-checkeron the 3 md files +plugin.json— exit 0, no findings.typosonplugins/verification/— exit 0, clean.scripts/validate-plugins.sh—✔ Validation passed(manifest + catalog).scripts/validate-plugin-contracts.mjs—33 setup skills and 1586 plugin files checked, pass.scripts/cross-plugin-source-registry.txt, so no upstream sync is involved.Related
dotnet-*/cloudflarecross-plugin refs inverification'smeasurecontexts).dotnet-*/cloudflare:*unguarded/ecosystem-coupled pattern: implementation: .NET-only marketplace-skill references in every mode context (origin coupling, low severity) #405 (implementation), toolchain: dotnet.md references dotnet-msbuild:* plugin skills that do not exist in this marketplace #412 (toolchain), testing: unguarded inline dotnet-test:* cross-plugin ref; .NET-only enrichment lists presented as the universal path #430 (testing, fixed in fix(testing): guard unguarded dotnet cross-plugin ref, frame .NET enrichment lists as stack-specific #491 — the guard pattern this PR mirrors).🤖 Generated with Claude Code