Skip to content

docs(retrofit-audit): grade all shipped plugins against extensibility contract v2.1 - #99

Merged
kyle-sexton merged 6 commits into
mainfrom
chore/audit-extensibility-contract-v21
Jul 12, 2026
Merged

docs(retrofit-audit): grade all shipped plugins against extensibility contract v2.1#99
kyle-sexton merged 6 commits into
mainfrom
chore/audit-extensibility-contract-v21

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Grades all 41 shipped plugins against the extensibility contract v2.1 (four seams, setup-action requirement, convention-resolution ladder, baked-assumption check, MCP-carry, evals) and records the snapshot as a new audit doc docs/extensibility-contract-grading.md, indexed from the README alongside the smoke-tests record.

Findings

  • 36 of 41 need no net-new work — 9 hook plugins, 5 pure-reference plugins, and 22 behavior plugins are compliant as shipped or their outstanding work is already owned by an existing issue.
  • 5 net-new setup-action gapsbug-report, discovery, planning, code-tidying, claude-ops each expose a config seam but ship no setup/configure action. One retrofit issue filed per plugin, sub-issue-linked under wave-2 map:
    • bug-report → melodic-software/medley#1428
    • discovery → melodic-software/medley#1429
    • planning → melodic-software/medley#1430
    • code-tidying → melodic-software/medley#1431
    • claude-ops → melodic-software/medley#1432
  • 0 baked repo assumptions at runtime; eval coverage deferred to melodic-software/medley#1396; hook migration to melodic-software/medley#1391.

Doc-only change (no marketplace.json / catalog-table touch) — conflict-free with sibling PRs.

Refs melodic-software/medley#1388


Note

Low Risk
Documentation-only; no plugin code, marketplace catalog, or runtime behavior changes.

Overview
Adds docs/extensibility-contract-grading.md, a dated audit snapshot that grades all 41 marketplace plugins against extensibility contract v2.1 (class, config seams, setup actions, convention ladder, baked assumptions, MCP-carry, evals).

The doc concludes 34 plugins need no net-new work, flags 7 behavior plugins with missing or partial setup for their config seams (bug-report, claude-ops, code-tidying, discovery, planning, songwriting, work-items), records 0 baked repo assumptions, and links seven new retrofit issues (#1428#1435) under wave-2 #1369. It also notes reclassifying songwriting from pure-reference to behavior and defers eval backfill to #1396.

README indexes the new grading doc next to the existing smoke-tests contract doc.

Reviewed by Cursor Bugbot for commit 1981c5c. Bugbot is set up for automated code reviews on this repo. Configure here.

… contract v2.1

Point-in-time grade of all 41 shipped plugins against the four seams, setup-action
requirement, convention-resolution ladder, baked-assumption check, MCP-carry, and evals.
36 compliant/owned; 5 net-new setup-action gaps emitted as retrofit issues.

Refs melodic-software/medley#1388
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_4a59b2ef-2e47-4b56-8525-c72a32676150)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Gather context and read changed files
  • Review docs/extensibility-contract-grading.md for accuracy and completeness
  • Review README.md changes
  • Check alignment with CLAUDE.md guidelines
  • Post final review findings

Doc-only change grading 41 plugins against extensibility contract v2.1. Counts and cross-references are internally consistent. No security concerns. A few minor editorial issues noted below.


Correctness check

All numerical claims verified against .claude-plugin/marketplace.json (41 plugins) and the grading table:

Claim Verified
41 total plugins ✓ — 41 "name" entries in marketplace.json
9 hook + 5 pure-reference + 27 behavior = 41 ✓ — counts add up
9 + 5 + 22 = 36 "no net-new work" ✓ — 22 behavior = 27 total − 5 net-new gaps
5 net-new setup-action gaps ✓ — bug-report, claude-ops, code-tidying, discovery, planning
Issue numbers match PR body ✓ — #1428–1432 consistent throughout
docs/extensibility-contract-smoke-tests.md exists (README ref)
Behavior table has 27 rows ✓ — counted individually

Findings

1. Unexplained bold/plain "missing" convention — potential reader confusion
docs/extensibility-contract-grading.md L94

The implementation row uses plain missing while the 5 net-new gaps use missing (bold). This is a useful signal — bold = net-new issue filed, plain = already owned — but it's never documented in the "Grading dimensions" key. A reader scanning the table could easily treat implementation as compliant. Worth adding a note in the Setup dimension or a table footnote: **missing** = net-new gap (issue filed this session); missing = existing tracked gap.

Fix this →


2. discovery MCP-carry verdict doesn't map to the SHIP/STAY/DROP taxonomy
docs/extensibility-contract-grading.md L92

The intro paragraph defines MCP-carry verdicts as SHIP 0 / STAY 14 / DROP 0. The discovery row reads perplexity/ref/microsoft-learn declared, not shipped (rule 3) which doesn't map to any of those three verdicts. All other MCP-carry cells use the taxonomy or a dash. A label like n/a — declared not shipped or DROP (rule 3, never shipped) would be consistent with the rest of the table.

Fix this →


3. Cross-file anchor links don't resolve to specific sections
docs/extensibility-contract-grading.md L4, L13

Links like ["Extensibility contract v2.1 — the four seams"](MIGRATION-PLAYBOOK.md) point to the file root, not the specific section. GitHub resolves #heading-slug fragments across relative links; e.g. MIGRATION-PLAYBOOK.md#extensibility-contract-v21--the-four-seams would land readers at the correct section. Minor quality issue — clicking currently drops the reader at the top of a long playbook.


4. Behavior plugins heading vs summary could confuse scanners (informational)
docs/extensibility-contract-grading.md L85

## Behavior plugins (27) combined with "22 behavior plugins are compliant" in the Verdict summary is accurate but could mislead a quick reader into thinking 5 are uncounted. The existing wording is logically correct once implementation's existing-owned gap is factored in; no change required, but consider (27 total) or a brief parenthetical if readers flag confusion.


Summary

The audit document is accurate, well-structured, and follows the CLAUDE.md "audit snapshot, not durable policy" principle correctly. The date-stamped, Tier-0 fact sourcing, "empirical claims decay" disclaimer, and clear issue tracking are all good practice. The README addition correctly places this alongside the smoke-tests record. The five net-new setup-action gaps and their issues are clearly attributed. Ready to merge; findings above are editorial with no correctness impact.

@cursor cursor 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.

Stale comment

Risk: low. Did not approve because Cursor Bugbot completed with skip status (usage limit reached), so the required automated review did not complete. Human review is needed before merge; no reviewers were assigned because the PR author is the sole repository contributor.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@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: 2e5fcc4999

ℹ️ 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 docs/extensibility-contract-grading.md Outdated
Its setup skill persists the catalog overlay but defers the report_dir userConfig to
/plugin configure; knowledge and skill-quality setups interview+persist theirs. Route the
gap to #1419. Addresses Codex PR-review finding.

Refs melodic-software/medley#1388
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_a715cc0a-c6f7-4741-9c68-cc83632fcc5f)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Gather context and read changed files
  • Review docs/extensibility-contract-grading.md for accuracy and completeness
  • Review README.md changes
  • Check alignment with CLAUDE.md guidelines and prior review comments
  • Investigate Codex finding on machine-health
  • Post final review findings

Doc-only change; the Codex finding has been addressed. Three minor editorial issues remain open from the prior Claude review plus one new one introduced by the correction commit. No correctness or security concerns.


Codex finding — addressed ✓

Codex flagged that machine-health was incorrectly marked present / compliant in commit 2e5fcc4. The correction commit 657ddf5 fixes this: the row now reads partial with the verdict "setup ships (catalog overlay) but does not interview/persist report_dir — completeness gap + behavior fixes owned by #1419." The Verdict Summary likewise calls machine-health "the lone setup-completeness outlier." Finding resolved.


New finding introduced by the correction commit

1. partial grade is undocumented in the grading-dimensions key
docs/extensibility-contract-grading.md L26

The Setup action dimension ends with: Present / missing / n/a (no config seam → not required). The correction commit added a fourth grade, partial, for machine-health (line 100), but the key was not updated to document it. Combined with the still-undocumented bold **missing** (net-new gap) vs. plain missing (existing tracked gap) distinction from the prior review, the key now lists three grades while the table uses five distinct values. A reader relying solely on the key will be confused.

Suggested update to line 26:

Present / missing (net-new gap, issue filed this session) / missing (existing tracked gap, issue already owns it) / partial (setup ships but does not cover all userConfig keys — gap noted, existing issue owns) / n/a (no config seam → not required).

Fix this →


Carried-forward findings (from prior Claude review — still unaddressed)

2. discovery MCP-carry cell doesn't map to the SHIP/STAY/DROP taxonomy
docs/extensibility-contract-grading.md L95

The intro establishes SHIP 0 / STAY 14 / DROP 0 as the MCP-carry vocabulary. The discovery row reads perplexity/ref/microsoft-learn declared, not shipped (rule 3) — no SHIP/STAY/DROP label. Every other row uses the taxonomy or a dash. A consistent label like n/a — declared not shipped (rule 3) would align it.

Fix this →

3. Cross-file links point to file root, not the specific section
docs/extensibility-contract-grading.md L4

Links to MIGRATION-PLAYBOOK.md have no fragment anchor. GitHub supports #heading-slug fragments on relative links; adding them (e.g., MIGRATION-PLAYBOOK.md#extensibility-contract-v21--the-four-seams) would drop readers at the right section rather than the top of a long document.


Summary

The document is accurate and the numerical claims are internally consistent. The Codex correctness finding was properly addressed before this re-review. All three remaining findings are editorial with no correctness impact. Ready to merge as-is; the three items above are nice-to-have polish.

@cursor cursor 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.

Stale comment

Risk: low. Did not approve because Cursor Bugbot completed with skip status (usage limit reached), so the required automated review did not complete. Human review is needed before merge; no reviewers were assigned because the PR author is the sole human repository contributor.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@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: 657ddf5f13

ℹ️ 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 docs/extensibility-contract-grading.md Outdated
…issue

songwriting has 9 action skills + a songwriting/templates/ override seam and persists
output under CLAUDE_PROJECT_DIR — not knowledge-only. Moved from pure-reference to behavior
with a setup-action gap (parity with code-tidying's tidy-lanes); filed #1433. Counts:
pure-reference 5->4, behavior 27->28, net-new 5->6. Addresses Codex PR-review finding.

Refs melodic-software/medley#1388
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_6fea968b-3233-4545-927e-b56e0ace7cef)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Read changed files and latest commit
  • Review docs/extensibility-contract-grading.md for accuracy and completeness
  • Check alignment with CLAUDE.md guidelines
  • Verify prior review findings are addressed
  • Post final review findings

Three commits in; the Codex songwriting finding has been addressed. Numerical claims are internally consistent after the reclassification. Three prior editorial findings remain open. No correctness or security concerns. Ready to merge.


Codex finding — addressed ✓

Commit 265938b reclassifies songwriting from pure-reference to behavior (9 action skills, template-override seam, ${CLAUDE_PROJECT_DIR}/songwriting/ output), emits setup issue melodic-software/medley#1433, and updates all counts accordingly. Finding resolved.


Numerical accuracy check (post-reclassification)

All claims verified against the doc at HEAD:

Claim Verified
41 total plugins ✓ — 9 hook + 4 pure-reference + 28 behavior
35 of 41 no net-new work ✓ — 9 + 4 + 22 = 35
6 net-new setup-action gaps ✓ — bug-report, claude-ops, code-tidying, discovery, planning, songwriting
Behavior table has 28 rows ✓ — counted individually
Issue numbers #1428–1433 ✓ — present in net-new issues table
machine-health partial grade ✓ — defers report_dir to /plugin configure, gap owned by #1419

Findings

1. partial grade still undocumented in the grading-dimensions key
docs/extensibility-contract-grading.md L26

The Setup action dimension lists Present / missing / n/a (no config seam → not required) — three values. The table now uses five distinct values: present, bold **missing** (net-new gap), plain missing (existing tracked gap), partial, and n/a. A reader relying solely on the key will be confused. Suggested replacement for line 26:

Present / missing (net-new gap, issue filed this session) / missing (existing tracked gap, existing issue owns it) / partial (setup ships but does not cover all userConfig keys — gap noted, existing issue owns) / n/a (no config seam → not required).

Fix this →


2. discovery MCP-carry cell doesn't use the SHIP/STAY/DROP taxonomy
docs/extensibility-contract-grading.md L98

The intro establishes SHIP 0 / STAY 14 / DROP 0 as the vocabulary for MCP-carry verdicts. The discovery row reads perplexity/ref/microsoft-learn declared, not shipped (rule 3) — no SHIP/STAY/DROP label. Every other row uses the taxonomy or a dash. A label like n/a — declared not shipped (rule 3) would make it consistent.

Fix this →


3. Cross-file links point to file root, not the specific section (minor)
docs/extensibility-contract-grading.md L4

Both MIGRATION-PLAYBOOK.md links at L4–5 lack fragment anchors. GitHub resolves #heading-slug fragments on relative links — adding e.g. MIGRATION-PLAYBOOK.md#extensibility-contract-v21--the-four-seams would land readers at the relevant section instead of the top of a long document.


4. PR body description is now stale (informational)

The PR body still says "36 of 41 need no net-new work — 9 hook plugins, 5 pure-reference plugins" and "5 net-new setup-action gaps." After the songwriting reclassification these are 35/41, 4 pure-reference, and 6 gaps. No correctness impact to the shipped doc, but worth a body edit if you want the PR description to match.


Summary

The audit document is accurate after three corrective commits. Codex's songwriting finding is resolved. The three editorial findings above are carry-forwards with no correctness impact. Ready to merge as-is; findings 1–3 are nice-to-have polish before or after merge.

@cursor cursor 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.

Stale comment

Risk: low. Did not approve because Cursor Bugbot completed with skip status (usage limit reached), so the required automated review did not complete. Human review is needed before merge; no reviewers were assigned because the PR author is the sole human repository contributor.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@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: 265938bee8

ℹ️ 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 docs/extensibility-contract-grading.md Outdated
…fy seam-vs-seam-3

work-items carries an optional .github/recurring-schedule.json tracked-config seam with only
per-item scaffolding and no dedicated setup; filed #1435. Added the config-seam vs seam-3
distinction so 'none needed' rows (reading the consumer's pre-existing CLAUDE.md / REVIEW.md /
tool configs) are defensibly separated from plugin-defined config seams. Net-new 6->7.
Addresses Codex PR-review finding.

Refs melodic-software/medley#1388
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_22720093-f53e-4ef4-b965-3f036f786523)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Gather context and read changed files
  • Check latest commit (8d800f1) against Codex work-items finding
  • Verify prior editorial findings status
  • Post final review findings

Fourth commit addresses the Codex work-items finding and adds a valuable seam-3 clarification. Numerical claims are internally consistent. Three prior editorial findings remain open. Ready to merge.


Codex finding — addressed ✓

Commit 8d800f1 adds the work-items seam-2 analysis: the row now shows **partial** setup, **GAP → net-new setup issue** verdict, and issue melodic-software/medley#1435 in the net-new table. The seam-3 clarification added to the Setup action dimension (lines 26–32) is an excellent addition — it clearly distinguishes plugin-defined config files (which require a setup action) from pre-existing consumer conventions read via seam 3 (which do not). This pre-empts similar misclassifications for review-toolkit, teach, claude-config-audit, etc.


Numerical accuracy check (post-work-items)

Claim Verified
41 total plugins ✓ — 9 hook + 4 pure-reference + 28 behavior
34 of 41 no net-new work ✓ — 9 + 4 + 21 = 34 (21 behavior = compliant-or-existing-owned)
7 net-new setup-action gaps ✓ — bug-report, claude-ops, code-tidying, discovery, planning, songwriting, work-items
Behavior table has 28 rows ✓ — counted individually
Net-new issues table has 7 rows ✓ — #1428#1435 (with gaps from concurrent issue filing)
work-items uses **partial** (bold=net-new), machine-health uses plain partial (existing gap) ✓ — consistent with the bold/plain convention used for **missing** vs missing

Findings (all carry-forward editorial, no correctness impact)

1. Grade vocabulary at line 26 still only lists three values — table uses six
docs/extensibility-contract-grading.md L26

The key ends with Present / missing / n/a (no config seam → not required). The table now uses six distinct values: Present, bold **missing** (net-new gap), plain missing (existing tracked gap), bold **partial** (net-new partial — work-items), plain partial (existing partial — machine-health), and n/a. The bold/plain convention is internally consistent and well-designed, but a reader relying solely on the key will not know how to decode partial or the bold distinction.

Suggested replacement for the key line:

Present / missing (net-new gap, issue filed this session) / missing (existing tracked gap, existing issue owns it) / partial (setup ships but is incomplete — net-new issue filed) / partial (existing partial gap, existing issue owns it) / n/a (no config seam → not required).

Fix this →


2. discovery MCP-carry cell doesn't use the SHIP/STAY/DROP taxonomy
docs/extensibility-contract-grading.md L106

The intro establishes SHIP 0 / STAY 14 / DROP 0 as the vocabulary for MCP-carry verdicts. The discovery row reads perplexity/ref/microsoft-learn declared, not shipped (rule 3) — no SHIP/STAY/DROP label. Every other row uses the taxonomy or a dash. A label like n/a — declared not shipped (rule 3) would make it consistent.

Fix this →


3. Cross-file links point to file root, not the specific section (minor)
docs/extensibility-contract-grading.md L4–5

Both MIGRATION-PLAYBOOK.md links lack fragment anchors. GitHub resolves #heading-slug fragments on relative links — adding e.g. MIGRATION-PLAYBOOK.md#extensibility-contract-v21--the-four-seams would land readers at the right section rather than the top of a long document.


Summary

All three Codex correctness findings have been addressed across four commits. The seam-3 clarification added in this latest commit is a meaningful improvement to the grading key. Numerical claims are internally consistent throughout. The three remaining findings above are all editorial with no correctness impact. Ready to merge; findings 1–3 are nice-to-have polish.

@cursor cursor 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.

Stale comment

Risk: low. Did not approve because Cursor Bugbot completed with skip status (usage limit reached), so the required automated review did not complete. Human review is needed before merge; no reviewers were assigned because the PR author is the sole human repository contributor.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_02ffb689-b5e4-48d0-9c19-8336b92258ff)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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


PR Review

  • Read changed files at HEAD
  • Check what the merge commit from main introduced
  • Verify status of three open editorial findings from prior review
  • Identify new staleness issues from sibling PRs
  • Post final review findings

Merge commit dd042c6 synced to dc5aaf7 (PR #98 / review-toolkit), not to the current tip of main (6ad0148, PR #100 / implementation). Two rows in the grading doc are now factually stale against what will be on main at merge time. Three prior editorial findings remain open.


New findings (correctness — pre-merge)

1. implementation row will be immediately stale at merge
docs/extensibility-contract-grading.md L108

The row reads missing (plain, existing-tracked gap owned by #1420). PR #100 (6ad0148) is already on main and resolves #1420: it bumps implementation to v0.2.0 and ships /implementation:setup (the re-runnable ladder writer for .claude/ecosystems/*.yaml). The merge commit dd042c6 incorporated main only up to dc5aaf7; PR #100 landed after that point. When this PR merges into main, the grading doc will describe a setup gap that has already shipped.

Suggested row correction:

Plugin Config seam Setup Ladder MCP-carry Evals Verdict / owner
implementation userConfig notes_dir + tracked .claude/ecosystems/<eco>.yaml (seam 2, ecosystem-commands contract) present consumer ecosystems file → inference → ask → bundled default present compliant; #1420 resolved by PR #100 (v0.2.0, /implementation:setup)

Fix this →


2. review-toolkit row carries stale issue reference
docs/extensibility-contract-grading.md L119

The row ends "compliant; ecosystem-commands retrofit owned by #1421." PR #98 (dc5aaf7) is a parent of the merge commit dd042c6, so the ecosystem-commands retrofit is already in this branch's working tree: review-toolkit is at v0.2.0, ecosystem-specialist.md has the command-truth resolution ladder, and #1421 is resolved. The qualifier "owned by #1421" should be removed or marked as resolved.

Suggested update: compliant; #1421 resolved by PR #98 (v0.2.0, ecosystem-commands contract)

Fix this →


Carried-forward findings (editorial, no correctness impact)

3. Grade vocabulary at line 26 only lists three values — table uses six
docs/extensibility-contract-grading.md L26

The key lists Present / missing / n/a. The table uses six distinct values: Present, bold **missing** (net-new gap), plain missing (existing tracked gap), bold **partial** (net-new partial — work-items), plain partial (existing partial — machine-health), and n/a. Suggested replacement:

Present / missing (net-new gap, issue filed this session) / missing (existing tracked gap, existing issue owns it) / partial (setup ships but is incomplete — net-new issue filed) / partial (existing partial gap, existing issue owns it) / n/a (no config seam → not required).

4. discovery MCP-carry cell doesn't use the SHIP/STAY/DROP taxonomy
docs/extensibility-contract-grading.md L106

Every other MCP-carry cell uses the established SHIP/STAY/DROP taxonomy or a dash. The discovery row reads perplexity/ref/microsoft-learn declared, not shipped (rule 3) — no taxonomy label. A label like n/a — declared not shipped (rule 3) would align it.

5. Cross-file links point to file root, not the specific section
docs/extensibility-contract-grading.md L4–5

Both MIGRATION-PLAYBOOK.md links lack fragment anchors. GitHub resolves #heading-slug fragments on relative links; adding them (e.g. MIGRATION-PLAYBOOK.md#extensibility-contract-v21--the-four-seams) would land readers at the correct section.


Summary

The audit document has been accurate through four corrective commit rounds. The merge from main introduced a gap: the implementation row (finding 1) is factually wrong at merge time because PR #100 has already shipped /implementation:setup on main, and the review-toolkit row (finding 2) carries a stale outstanding-issue reference that was resolved by PR #98 before the merge commit. Both are one-line fixes. The three carry-forward items (3–5) remain editorial with no correctness impact. Recommend addressing findings 1–2 before merging.

@cursor cursor 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.

Stale comment

Risk: low. Did not approve because Cursor Bugbot completed with skip status (usage limit reached), so the required automated review did not complete. Human review is needed before merge; no reviewers were assigned because the PR author is the sole human repository contributor.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_1f091614-d195-43d1-a57f-102f084a4ae7)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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


PR Review


The merge commit at 1981c5c pulled in PR #100 (6ad0148, implementation v0.2.0), which resolves the two correctness issues the previous review raised. I've prepared and committed the fixes locally but cannot push due to a 403 (write access not granted for this bot). The five changes below are ready to apply.


Correctness findings (pre-merge staleness — apply before merging)

1. implementation row is stale — setup shipped in PR #100
docs/extensibility-contract-grading.md L108

The row still reads missing for Setup and "owned by #1420." PR #100 ships plugins/implementation/skills/setup/SKILL.md and bumps the plugin to v0.2.0. The config seam also expanded — /implementation:setup writes .claude/ecosystems/<eco>.yaml (seam 2, ecosystem-commands contract) in addition to userConfig notes_dir.

Replace the implementation row with:

| implementation | `userConfig` `notes_dir` + tracked `.claude/ecosystems/<eco>.yaml` (seam 2, ecosystem-commands contract) | present | consumer ecosystems file → inference → ask → bundled default | — | present | compliant; #1420 resolved by PR #100 (v0.2.0, `/implementation:setup`) |

Fix this →


2. review-toolkit row carries resolved issue reference
docs/extensibility-contract-grading.md L119

"ecosystem-commands retrofit owned by #1421" — PR #98 (dc5aaf7) resolved #1421 before even the first merge commit.

Replace with: compliant; #1421 resolved by PR #98 (v0.2.0, ecosystem-commands contract)


3. Verdict Summary names #1420 and #1421 as "already filed" — both are now resolved
docs/extensibility-contract-grading.md L61–65

Update the "Per-slug retrofits already filed" sentence to annotate those two as resolved:

~~`#1420` (implementation)~~ resolved by PR #100 (v0.2.0, `/implementation:setup`), ~~`#1421` (review-toolkit)~~ resolved by PR #98 (v0.2.0, ecosystem-commands contract).

Editorial findings (carried forward — four review rounds open)

4. Grade vocabulary at line 26 only lists three values; table uses five
docs/extensibility-contract-grading.md L26

The key says Present / missing / n/a. The table uses five distinct values: present, bold **missing** (net-new gap), bold **partial** (work-items net-new), plain partial (machine-health existing), and n/a. Suggested replacement:

Present / missing (net-new gap, issue filed this session) / partial (setup ships but is incomplete — net-new issue filed) / partial (existing partial gap, existing issue owns it) / n/a (no config seam → not required).

Fix this →


5. discovery MCP-carry cell doesn't use SHIP/STAY/DROP taxonomy
docs/extensibility-contract-grading.md L106

Every other row uses the taxonomy or a dash. Change perplexity/ref/microsoft-learn declared, not shipped (rule 3) to n/a — perplexity/ref/microsoft-learn declared, not shipped (rule 3).


Summary

All five Codex correctness findings (machine-health, songwriting, work-items) were addressed in prior commits. The two new correctness issues introduced when PR #100 merged into this branch have been prepared as commit 1430c92 locally — the changes are confirmed correct against plugins/implementation/skills/setup/SKILL.md and both plugin.json files (v0.2.0). Push was blocked by a 403; Kyle can cherry-pick or manually apply the three correctness changes (findings 1–3) before merging. Findings 4–5 remain editorial polish with no correctness impact.

@cursor cursor 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.

Risk: low. Did not approve because Cursor Bugbot completed with skip status (usage limit reached), so the required automated review did not complete. Human review is needed before merge; no reviewers were assigned because the PR author is the sole human repository contributor.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit f0aa5af into main Jul 12, 2026
41 of 43 checks passed
@kyle-sexton
kyle-sexton deleted the chore/audit-extensibility-contract-v21 branch July 12, 2026 16:55
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…ributions (#2843)

Closes #2837. Closes #2833.

## Summary

Three defects in the silent-revert canary, plus one unfiled
harness-safety fix
in a file this change already owns. Branched off `2de57a379` (#2832),
which had
already merged.

## Fix

## 1. `declares_removal()` could not read the only revert subject that
merges here (#2837)

The detector accepted three intent forms. This repo is squash-only with
`squash_merge_commit_title: PR_TITLE`, so the squash subject is the PR
title,
and `.github/workflows/pr-title.yml` gates every title through a
required
Conventional-Commits check whose default type list is all-lowercase — it
admits
`revert:` and contains nothing a `Revert "…"` subject could match. So a
deliberate revert reached `main` wearing a subject the detector could
not read.

`declares_removal()` now also accepts the Conventional-Commits revert
type,
anchored at the start of the **subject**, requiring the literal
lowercase token,
its optional `(scope)` and/or `!`, its colon, and a non-empty
description:

```
^revert(\([^()]+\))?!?:[[:space:]]*[^[:space:]]
```

Never a substring search for "revert" — kept exactly as constrained as
the three
forms beside it.

### Before / after on the repo's one real deliberate revert

`1d1fca6e8` — `revert: remove Cursor dual-target marketplace manifests
(#1835) (#1839)`

Before (at `2de57a379`, shipped detector, thresholds unmodified):

```
$ bash scripts/check-silent-revert.sh --commit 1d1fca6

SILENT REVERT SUSPECTED

  removed by   1d1fca6  revert: remove Cursor dual-target marketplace manifests (#1835) (#1839)
               2026-07-30 19:41:47 -0400
  content from b6c4b58  feat: add Cursor dual-target marketplace manifests (#1835)
               2026-07-30 18:17:36 -0400  (1 commit(s) earlier on main)
  lines lost   3361  (threshold 200, window 40 commits)
  [... 102 files, sample block and "What to do" block elided; 130 lines total ...]

EXIT=1
```

After:

```
$ bash scripts/check-silent-revert.sh --commit 1d1fca6
declared 1d1fca6 revert: remove Cursor dual-target marketplace manifests (#1835) (#1839)
         removal is declared: the subject carries the Conventional-Commits revert type
EXIT=0
```

### Suppression is not widened over anything the corpus records

- No recorded incident has a `revert`-prefixed subject — `f603880da`
`fix(disk-hygiene): …`, `9239f1541` `feat(disk-hygiene): …`, `cc58cbc53`
`fix(repo-fleet-hygiene): …`, `c8470efd0` `docs(conventions): …`. All
four rows
  still hold (green run below).
- Neither acknowledgment-file commit is revert-prefixed either —
`6f0a31109`
`fix(repo-fleet-hygiene): …`, `91e77fc16` `fix(hook-utils): …` — so no
ack row
goes dead now that `declares_removal()` short-circuits ahead of
`ack_reason()`.
- The header's "fires on 5 commits — 1%" calibration figure is therefore
  unchanged; none of those five is revert-prefixed.

New tests pin both directions. `revert:`, `revert(scope):`, `revert!:`
and
`revert(scope)!:` suppress; `feat: do not revert the alpha guard (#99)`,
`reverted: drop the alpha guard (#99)`, `Revert: drop the alpha guard
(#99)` and
a bare `revert:` with no description all still fire. The pre-existing
case only
covered a *body* mention of "revert"; the subject is what the new form
reads, so
that is where a substring bug would widen.

## 2. `verify_known_incidents` asserted only "something fired" (#2833)

A `fires` row passed on `scan_commit`'s exit status while the note
beside it
named a specific culprit and a specific line count that nothing checked.
On
`cc58cbc53`, whose deletions trace to two culprits, losing the
`eda5ae5ed`
attribution entirely would still have printed `ok` on the surviving
`bfb66beb8`
finding — the canary announcing a reproduction it did not perform.

A `fires` row may now carry a bracketed attribution expectation after
its sha:

```
fires <sha> [<culprit-full-sha>=<blamed-lines>,<culprit-full-sha>=<blamed-lines>] <note>
```

and the replay asserts the run's findings are **exactly** that set —
same
culprits, same per-culprit counts, no extras, no omissions. Full
40-character
culprit shas only, the same discipline `silent-revert-acknowledged.txt`
uses.

Counts come from a new `FINDINGS_SINK` file that `report_finding`
appends
`<full-culprit-sha> <count>` to, not from scraping the human report —
the report
prints a 9-character abbreviation, which is not enough sha to assert on.
Nothing
else sets `FINDINGS_SINK`, so `--commit` and range mode are
byte-identical.

Every recorded figure was **measured, not transcribed from the notes** —
the
sink was wired first and the observed values recorded:

```
f603880 -> a95f240 346
9239f15 -> f603880 451
cc58cbc -> bfb66be 853
             eda5ae5 298
```

Three of the four agree with the notes #2832 corrected; the fourth is
298 rather
than 301, for the reason in section 5. A malformed field is exit 2
(cannot
run), never a FAIL and never a pass — a silently misread expectation is
the same
false green this file exists to remove. The field is optional so a row
can be
pinned before its attribution is measured, but a new well-formedness
assertion
requires every *shipped* `fires` row to carry one.

## 3. The workflow header stated a reason that was not true (unfiled)

`.github/workflows/silent-revert-canary.yml` asserted *"There is no
`pull_request` trigger, so it can never gate a PR"* — while its own
`on:` block
has a paths-filtered `pull_request` trigger, added by #2808 to run the
detector's
unit tests, and explained at length 20 lines further down in the same
file. The
conclusion holds for a different reason: both scan steps are gated
`if: github.event_name != 'pull_request'`, and the lane sits outside
`ci.yml`
and its `ci-status` aggregate. Wording corrected to the actual reason.

**No trigger and no `if:` changed.** `git diff` on that file is comment
lines
only — one hunk, `@@ -13,6 +13,9 @@`, entirely inside the `#` header.

## 4. The detector's line counts depended on ambient git config
(unfiled, found by #2833's new assertion)

The first CI run of this branch went red, and the failure is the most
valuable
thing in this PR. The runner reported the `eda5ae5ed` attribution as
**298**
lines where my machine measured **301**:

```
FAIL cc58cbc fires, but NOT as recorded
     recorded attribution:   eda5ae5…  301
     what the detector reported:  eda5ae5…  298
```

Cause: `attribute_file` called bare `git diff --unified=0`, so it
inherited
whatever `diff.algorithm` the caller's config carried. I have
`diff.algorithm = histogram` set globally; CI has nothing set and
therefore uses
git's default `myers`. The algorithm changes which lines a hunk calls
deleted,
so it changes the per-culprit counts this canary **thresholds on**.
Reproduced
directly:

```
$ GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=diff.algorithm GIT_CONFIG_VALUE_0=myers \
    FINDINGS_SINK=… scripts/check-silent-revert.sh --commit cc58cbcbfb66be…  853
eda5ae5…  298      # 301 under histogram
```

Three lines is harmless in itself. The principle is not: the same drift
can
carry a count across the 200-line threshold, so a commit could fire on
one
machine and stay silent on another, and the header's "fires on 5 commits
over
500 — 1%" calibration only ever described one algorithm.

`attribute_file` now pins `--diff-algorithm=myers -M` explicitly, and
the file
enumeration pins `-M` too. **Both are git's defaults, so this does not
change
what CI detects today** — CI already had no `diff.algorithm` set. It
makes a
local run match CI, not the reverse. `-M` covers the same exposure for
rename
detection, which the existing header calls "load-bearing rather than
incidental": `diff.renames = false` in a developer's config would
decompose a
`git mv` into delete + add and make relocating a large recent file fire.

The recorded figure is now **298**, and the prose figures in the script
header
and the corpus are corrected with the refutation attached, so nobody
re-measuring on a histogram machine "corrects" it back to 301.

Worth stating plainly: nothing asked for this. #2833's exact-count
assertion
turned a silent, config-dependent divergence into a red build on its
first run —
which is precisely the argument for asserting attributions instead of
exit
status.

## 5. The test harness could commit the developer's work as `test
<t@t.test>` (unfiled)

Found the hard way while developing this. `mk_repo` is called as
`repo="$(mk_repo)"`, so a `return 1` inside the command substitution
cannot abort
the suite — the caller just gets `""`. And `""` is not inert: `git -C
""` is
documented as a no-op, so the next `add -A` + `commit` staged and
committed my
uncommitted work into the checkout, authored `test <t@t.test>`. Such a
commit
cannot be pushed here — it fails `required_signatures` with `no_user`.

`mk_repo` now yields a path derived from `SELF_DIR` that does not exist,
so every
git call against it fails loudly and the assertions go red —
fail-closed, which
is what a harness that cannot build its fixture should do. Scoped to
this file
only; if the same `repo="$(mk_repo)"` shape exists in sibling harnesses
that is a
separate follow-up.

## Verification

`scripts/check-silent-revert.sh --verify-known-incidents`, run on a
machine with
`diff.algorithm = histogram` set globally — it now agrees with CI
exactly,
because the flags are pinned:

```
ok   f603880 fires as recorded, 1 attribution(s) reproduced exactly  (#2639 dropped #2635 (346 blamed lines), 13 minutes later)
ok   9239f15 fires as recorded, 1 attribution(s) reproduced exactly  (#2641 dropped #2639 (451 blamed lines), 10 minutes later)
ok   cc58cbc fires as recorded, 2 attribution(s) reproduced exactly  (#2633 dropped #2644's rollups (853 blamed lines) and #2642's GraphQL merge evidence (298); already recorded in #2656)
ok   c8470ef stays clean as recorded  (docs(conventions) rewrote 129 lines of a doc #2679 had just added)

Canary reproduces every recorded incident at the shipped settings.
```

Injected failure against the **shipped** corpus (`853` changed to `852`
in a
copy) — the mechanism is proven on real rows, not only on synthetic
fixtures:

```
$ SILENT_REVERT_INCIDENTS=<copy with 298 -> 297> scripts/check-silent-revert.sh --verify-known-incidents
ok   f603880 fires as recorded, 1 attribution(s) reproduced exactly  (...)
ok   9239f15 fires as recorded, 1 attribution(s) reproduced exactly  (...)
FAIL cc58cbc fires, but NOT as recorded  (#2633 dropped #2644's rollups (853 blamed lines) and #2642's GraphQL merge evidence (298); already recorded in #2656)
     recorded attribution:
       bfb66be 853
       eda5ae5 297
     what the detector reported:
       bfb66be 853
       eda5ae5 298
     A row that fires for the wrong reason is not a reproduction.
     Do NOT edit the row to match; find out why the attribution moved.
ok   c8470ef stays clean as recorded  (...)

The canary no longer reproduces the incidents it was built for.
Do not relax the recorded expectations to make this pass.
EXIT=1
```

The commit still fires — exit status alone would have passed this row.
Note the
row is red on a **one-line** discrepancy in one of two attributions,
which is
exactly the regression #2833 describes.

`scripts/check-silent-revert.test.sh`: **49 passed, 0 failed**,
including
`replay fails when the finding is attributed to a different culprit`,
`replay fails when the recorded line count no longer reproduces`, and
`replay fails when one of two recorded attributions stops reproducing`.
So the
assertion is proven by permanent tests, not only by a one-off injection.

## 6. Review follow-up: an unterminated attribution field read as
*absent*

Both automated review lanes independently flagged the same real gap, and
they
were right. A `fires` row whose field opened with `[` but never closed
it failed
the `[[ "$rest" == \[*\]* ]]` glob, so `attribution` stayed empty, the
remainder
became free-text `note`, and the row fell back to passing on exit status
alone —
reintroducing the exact pre-#2833 gap by the one route nobody would look
at, and
contradicting the contract documented directly above it.

A leading `[` now COMMITS the row to carrying an attribution;
unterminated takes
the malformed path. Reproduced against the real corpus with the closing
bracket
stripped from the `f603880da` row:

```
check-silent-revert: unterminated attribution field for f603880… (no closing ']'): [a95f240…=346 #2639 dropped #2635 …
EXIT=2
```

The four malformed shapes already pinned all carried a closing `]`, so
this one
was untested; `[<sha>=40` and a bare `[` are now pinned too. The shipped
corpus
was never at risk — `t_shipped_data_files_are_wellformed`'s regex covers
it —
but that is a separate layer and does not hold for a custom
`SILENT_REVERT_INCIDENTS`.

## 7. Verification follow-up: `git blame` was still ambient-config
dependent

Fresh-context verification of section 4 found that fix was only half of
one.
Pinning the diff flags left `attribute_file`'s `git blame` call bare, so
`blame.ignoreRevsFile` — an ordinary setting in any repo carrying a
bulk-reformat commit — still decided the per-culprit counts the replay
now
asserts on. On the real corpus, with that setting naming `bfb66beb8`:

| culprit | pinned | with `blame.ignoreRevsFile` |
| --- | ---: | ---: |
| `bfb66beb8…` | 853 | **259** |
| `eda5ae5ed…` | 298 | **322** |

On a synthetic fixture it is worse than a wrong number. With the pin
absent and
that config present, the detector reports **no finding at all** on a
genuine
silent revert:

| | clean config | hostile config |
| --- | --- | --- |
| pinned | `culprit 40` | `culprit 40` |
| unpinned | `culprit 40` | **(nothing — the canary goes silent)** |

That is a false green reached through the developer's own gitconfig —
the
precise failure this canary exists to remove.

**The obvious fix does not work, and the comment says so.**
`-c blame.ignoreRevsFile=` does *not* clear it: the documented "an empty
file
name resets the list" applies to the **option**, and the `-c` form was
measured
leaving the hostile value fully in effect (853 → 259 with the reset
supposedly
applied). Only `--no-ignore-revs-file` actually resets. The symmetry
with the
`-c` pins above is wrong here and is deliberately not used.

`t_counts_are_immune_to_ambient_git_config` pins the property: the same
fixture
scanned twice, once under a hostile `GIT_CONFIG_GLOBAL` setting
`blame.ignoreRevsFile`, `diff.algorithm` and `diff.renames`, asserting
identical
exit status and identical per-culprit findings. **Confirmed
discriminating** —
with the blame pin stripped it fails, and it fails because the hostile
run
reports nothing at all.

`shellcheck`, `actionlint`, `typos`, `bash -n` and
`scripts/check-shell-portability.sh` all pass.

## Test plan

Run from a clean checkout of this branch, with `unset GIT_DIR
GIT_WORK_TREE`:

1. `bash scripts/check-silent-revert.test.sh` — the detector's own unit
suite.
Expect **49 passed, 0 failed**. Covers all four `revert:` spellings, the
   four negative subject cases (`feat: do not revert ...`, `reverted:`,
`Revert:`, bare `revert:`), the wrong-culprit / wrong-count / missing-
   attribution replay regressions, all six malformed-field shapes, and
   `t_counts_are_immune_to_ambient_git_config`.
2. `bash scripts/check-silent-revert.sh --verify-known-incidents` — the
real
   corpus. Expect exit 0 with all three `fires` rows reporting
`attribution(s) reproduced exactly` and the `clean` row staying clean.
3. Negative control for step 2: edit
`scripts/silent-revert-incidents.txt` to
inject a wrong culprit sha (leaving the count correct, so the commit
still
   fires) and, separately, a wrong line count. Each must exit **1** with
   `fires, but NOT as recorded`. Restore the file afterwards.
4. Set `diff.algorithm = histogram` in global git config and repeat
steps 1-2.
   The numbers must not move — that is what the new flag pins buy.
5. `git diff origin/main...HEAD --
.github/workflows/silent-revert-canary.yml`
   must show comment-only changes; no executable YAML line may differ.

## Related

- Refs #2808 — the PR that merged the canary and its three intent forms.
- Refs #2832 / #2831 — the corpus-attribution correction this branches
off;
  #2833 was raised in its review.
- Refs #2691 — the original silent-revert audit the corpus is built
from.
- Refs #1839 — the deliberate revert (`1d1fca6e8`) used as the
real-history
  fixture for #2837.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant