Skip to content

feat: add implementation plugin (10 implementation-stage skills) - #78

Merged
kyle-sexton merged 5 commits into
mainfrom
feat/publish-implementation
Jul 11, 2026
Merged

feat: add implementation plugin (10 implementation-stage skills)#78
kyle-sexton merged 5 commits into
mainfrom
feat/publish-implementation

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Contributor

Refs melodic-software/medley#1281

What

New implementation plugin bundling ten skills as one implementation-stage capability:

  • /implementation:implement — inline plan execution: mode detection (feature/fix/refactor/config), TDD-by-default cadence, build+test after each logical block, green-checkpoint commits, divergence detection routing back to planning, phase-boundary handoffs
  • /implementation:implement-dispatch — orchestrated execution variant: scope-fenced worker briefs, subagent dispatch, evidence-verified returns, main-side builds, conservative-option deviations log
  • /implementation:build — build+test+lint for changed files, auto-detecting affected ecosystems (.NET, Python, TypeScript, Bash, PowerShell, Markdown); the command-table SSOT sibling skills compose
  • /implementation:lint — lint/format-only pass with --fix mode, honoring each tool's config-file opt-in
  • /implementation:test-write / test-plan / test-diagnose / test-e2e — test authoring, coverage-gap analysis, failure diagnosis loop, live E2E evidence capture (incl. non-UI smoke playbook)
  • /implementation:verify-changes — mechanical prerequisite gate + outcome verification with change-type-detected criterion (feature / fix / refactor)
  • /implementation:verify-improvement — baseline-vs-after measurable-improvement verification; no baseline → honest "cannot quantify"

Marketplace entry: category: development, tags implementation, build, lint, testing, tdd, e2e, verification, skill. plugin.json pins version: 0.1.0 with one userConfig option (notes_dir, default .claude/notes, consumer-convention-wins).

De-coupling from the source repo

  • Medley .work/<slug>/ slice machinery replaced with ${user_config.notes_dir}/<topic-slug>/ artifacts + generic inline guidance; medley rule citations inlined as generic principles.
  • Medley ecosystem-command SSOTs replaced with portable per-tool defaults (reference/ecosystem-config.md in build and lint) that the consuming project's own documented commands override ("consumer conventions win").
  • Medley CI-parity checks generalized to project-declared gates read from the consumer's own conventions — the plugin ships none of its own.
  • Cross-plugin refs qualified with graceful degradation: /tdd:tdd, /playwright:playwright, /discovery:research, /planning:architect, /session-flow:handoff, /session-flow:retro, /diagnose:diagnose — each "when installed", with an inline fallback. Sibling refs inside the plugin stay bare per the locked issue decision.
  • Descriptions ≤2 tight sentences each (10 skills, ~3.9k chars total).

Gate evidence

  • claude plugin validate --strict ./plugins/implementation — PASS
  • claude plugin validate --strict . (catalog manifest) — PASS
  • claude plugin details (via --plugin-dir): always-on ~1,409 tok (build ~140 / implement ~150 / implement-dispatch ~130 / lint ~130 / test-diagnose ~120 / test-e2e ~130 / test-plan ~120 / test-write ~140 / verify-changes ~170 / verify-improvement ~180); on-invoke ~950–7.4k per skill
  • --plugin-dir smoke test in a clean non-medley repo (fresh git init scratch dir): /implementation:test-plan invoked headless; replied SMOKE-OK implementation:test-plan — correct plugin namespace
  • typos over the plugin + shared files: clean. markdownlint / editorconfig-checker not runnable locally on this worker (npm not installed on the machine after reboot) — deferring those two lanes to this repo's CI gates.
  • Full baseline/parity apparatus intentionally skipped per the issue's speed-adjusted gate (non-destructive skill content; medley originals remain until the cutover issue).

Security review (plugin-acceptance)

  1. Code execution — no hooks, no scripts; markdown-only skill content (plus per-skill evals/evals.json test fixtures, an established pattern in this marketplace).
  2. MCP servers — none.
  3. userConfig — one non-sensitive string option (notes_dir); no secrets anywhere.
  4. Cache isolation — no ../ reach-outs, no absolute paths; all references relative inside the plugin; consumer artifacts written only under the consumer-configured notes directory.
  5. Data egress — none; no outbound network calls.
  6. Provenance — first-party authored, lifted from melodic-software/medley and de-coupled per the migration playbook.

🤖 Generated with Claude Code


Note

Low Risk
Markdown-only plugin content and catalog wiring; no hooks, MCP, or secrets. Skills may instruct running consumer-repo CLI in other projects, consistent with sibling plugins.

Overview
Adds a new implementation marketplace plugin (plugins/implementation/, version 0.1.0) and registers it in .claude-plugin/marketplace.json plus the root README catalog.

The plugin ships ten user-invocable skills as markdown workflows: implement / implement-dispatch (inline vs worker orchestration, divergence and phase handoffs), build / lint (polyglot verification with shared reference/ecosystem-config.md and per-ecosystem context), test-write, test-plan, test-diagnose, test-e2e, verify-changes, and verify-improvement. Cross-plugin calls (e.g. tdd, planning, playwright) are optional with inline fallbacks; consumer CLAUDE.md / rules override default commands. Artifacts default to notes_dir (.claude/notes).

Several skills include evals/evals.json fixtures. Content is lifted from the medley source and generalized (no medley-specific paths or CI gates in the bundle).

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

De-coupled from the medley source repo per docs/MIGRATION-PLAYBOOK.md:
consumer conventions override bundled ecosystem defaults; cross-plugin
references use qualified /<plugin>:<skill> form with graceful degradation;
artifacts route to the notes_dir userConfig option.

Refs melodic-software/medley#1281

Co-Authored-By: Claude <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 11, 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_201134ee-73e6-4979-ae01-4ece5d64d2fa)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat: add implementation plugin (10 implementation-stage skills)

  • Gather context and understand request
  • Fetch fresh plugin docs per CLAUDE.md mandate
  • Review plugin manifest and marketplace entry
  • Review skill files for correctness and repo-agnostic design
  • Check security concerns (egress, isolation, secrets)
  • Check cross-plugin references and degradation fallbacks
  • Post comprehensive review feedback

Overall Assessment

This is a high-quality, well-structured plugin addition. The skill content is coherent, the SSOT decomposition (/build/lint/verify-changes) is logically sound, and the plugin correctly follows the repo's established patterns for repo-agnostic design, consumer-convention-wins, and cross-plugin graceful degradation. The security posture is clean: markdown-only skills, no hooks, no MCP, no egress, no hardcoded paths.

Four findings follow — one unrelated bundled change, two cross-marketplace reference patterns worth acknowledging, and one cosmetic eval-gap.


Finding 1 — Unrelated .editorconfig change bundled in the PR

Severity: Minor

The PR removes the Go language section from the repo-wide .editorconfig, but the PR description doesn't mention this and the commit message covers only the plugin addition. The repo has no Go files (confirmed), so the removal is harmless and arguably correct cleanup — but it's a cross-cutting repo config change mixed into a plugin PR.

-# PowerShell, config/serialization (JSON/YAML/TOML), JS/TS, Go, Windows batch, git
+# PowerShell, config/serialization (JSON/YAML/TOML), JS/TS, Windows batch, git
-[*.go]
-indent_style = unset
-indent_size = unset

Permalink: /.editorconfig

Suggestion: Either note this removal in the PR description, or split it into its own commit/PR so the implementation plugin addition stays atomic.


Finding 2 — External marketplace plugin references in skill content

Severity: Low

Several skills reference plugins from other marketplaces by name, with the "when installed" graceful fallback pattern:

The "when installed" + inline-fallback pattern is applied correctly everywhere — the skill degrades gracefully if those plugins are absent. The safety concern is low.

The main friction is discoverability: a consumer installing implementation from this marketplace may not know they can also install andrej-karpathy-skills from a separate marketplace, or where to find it. As the marketplace grows, this could create unverifiable cross-marketplace trust chains.

Suggestion: This pattern is acceptable for cross-plugin references within the same marketplace (the other sibling references like /tdd:tdd, /planning:architect are fine). For references pointing to fully external marketplaces, consider either: (a) adding a brief "find it at <url>" note in the skill so consumers can locate the dependency, or (b) a ## Optional enhancers section in the plugin's README.md listing external plugins that augment it with install pointers.


Finding 3 — Eval IDs 5 and 6 are absent in verify-changes/evals/evals.json

Severity: Cosmetic

The file contains IDs 1–4, then jumps to 7–10. IDs 5 and 6 are missing, suggesting evals were removed during development. The existing evals are well-written and cover the key routing behaviors.

Permalink: verify-changes/evals/evals.json

Suggestion: Either renumber IDs sequentially (1–8) for cleanliness, or leave as-is if the IDs are intentionally sparse (e.g., reserved for later additions). No functional impact.


Finding 4 — notes_dir description implies CLAUDE.md override that the config mechanism doesn't enforce

Severity: Minor / documentation clarity

The notes_dir userConfig description says:

"A working-notes convention declared in your own project's CLAUDE.md or rules takes precedence."

Permalink: plugin.json L16

In practice, skills read ${user_config.notes_dir} — the actual config value. Whether the model honors a CLAUDE.md convention over the config value depends on the model's judgment, not a technical override. A consumer setting notes_dir: ".claude/notes" in their plugin config but declaring a different path in CLAUDE.md may see inconsistent behavior.

Suggestion: Tighten the description to: "Project-relative directory for implementation artifacts. Override this value to match your project's working-notes convention." This removes the ambiguous "takes precedence" claim and gives consumers clear, actionable guidance.


What looks good

  • Plugin manifest (plugin.json): valid schema, explicit version: 0.1.0, correct author/license/keywords, single userConfig option with a sensible default. No issues.
  • Marketplace entry: category: development, tags match the skill content, JSON placement is clean.
  • Repo-agnostic design: all artifact paths use ${user_config.notes_dir}/<topic-slug>/, all build/test/lint commands defer to consumer CLAUDE.md with "consumer conventions win" stated explicitly in both build and lint reference configs.
  • SSOT decomposition: /build owns ecosystem detection and CLI commands; /lint owns lint-only; /verify-changes orchestrates without reimplementing the mechanical pass. The delegation chain is coherent and consistently followed.
  • Graceful degradation: every cross-plugin reference (12 sibling + 7 external) uses "when [plugin] is installed" with a documented fallback. The disable-model-invocation field follows the repo's established pattern across 56+ skills.
  • Security: no hooks, no MCP servers, no ../ reach-outs, no absolute paths, no egress, no secrets, no PII.
  • Evals: the verify-changes evals are particularly strong — they cover the mode-routing, delegation boundaries, and redirect behaviors that matter for this skill's correctness.
  • Token budget: ~1,409 tokens always-on for 10 skills is documented and validated. The per-skill on-invoke range (~950–7.4k) is appropriate for orchestration-level skills.

@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: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed; no reviewers were assigned because no non-author human maintainers are available on this repository.

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: 9ac7bf8dc3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/implementation/skills/implement/SKILL.md
Comment thread plugins/implementation/skills/build/reference/ecosystem-config.md Outdated
Comment thread plugins/implementation/skills/lint/reference/ecosystem-config.md Outdated
TypeScript change-detection globs now include manifests and lockfiles
(package.json, lockfiles, tsconfig*.json) so dependency/config-only
changes trigger verification; actionlint runs in no-argument discovery
mode so .yaml workflows are linted alongside .yml.

Refs melodic-software/medley#1281

Co-Authored-By: Claude <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 11, 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_b159dd39-787e-4786-9ae1-3d6b6db5291a)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat: add implementation plugin (10 implementation-stage skills)

  • Gather context and understand request
  • Review new commit (fix: address Codex review findings on ecosystem detection)
  • Check ecosystem detection changes in build/lint reference files
  • Verify fixes address the Codex findings correctly
  • Post comprehensive review feedback

New Commit Review — fix: address Codex review findings on ecosystem detection

This commit addresses the two Codex P2 findings from the prior automated review. Both fixes are correct and well-targeted.


Fix 1 — TypeScript change-detection globs now include manifests and lockfiles ✅

build/reference/ecosystem-config.md L40

-    globs: ["*.ts", "*.tsx", "*.js", "*.jsx", "*.mjs"]
+    globs: ["*.ts", "*.tsx", "*.js", "*.jsx", "*.mjs", "package.json", "package-lock.json", "pnpm-lock.yaml", "yarn.lock", "tsconfig*.json"]

Correct fix. A dependency-only PR (lockfile bump, package.json script change, tsconfig path tweak) can break tsc --noEmit without touching any .ts file. The added globs ensure those changes route through TypeScript verification. The tsconfig*.json wildcard also catches project-specific variants like tsconfig.build.json.

One observation: the lint/reference/ecosystem-config.md TypeScript section still has only the narrower source-file globs (*.ts, *.tsx, *.js, *.jsx, *.mjs). This is intentional and correct — changing package.json or a lockfile doesn't require re-linting source; only build/test verification is meaningful there.


Fix 2 — actionlint uses no-argument discovery mode ✅

lint/reference/ecosystem-config.md L67

-      actionlint .github/workflows/*.yml
+      actionlint  # no args — discovers all workflow files itself (.yml and .yaml)

Correct fix. Per actionlint's documented behavior, with no argument it discovers all workflow files under .github/workflows/ regardless of extension (.yml or .yaml). The prior form would silently miss any repo using .yaml extensions. The inline comment makes the intent explicit.


Outstanding items from the earlier review

These were raised in the previous review pass and were not addressed in this commit — all are minor/cosmetic:

# Finding Severity Status
1 .editorconfig Go section removal bundled without mention in PR description Minor Open
2 External marketplace plugin references lack install pointers for consumers Low Open
3 verify-changes/evals/evals.json IDs skip 5 and 6 Cosmetic Open
4 notes_dir description implies CLAUDE.md override that the config mechanism doesn't enforce Minor Open

None of these block the PR — the plugin is functionally correct, securely scoped (markdown-only, no hooks, no MCP, no egress), and the two substantive Codex findings are now properly resolved.


Overall

The implementation plugin is in good shape. The two P2 Codex findings were correctly addressed: TypeScript verification now triggers on manifest/config-only changes, and actionlint no longer silently skips .yaml workflow files. No new issues introduced by this commit.

@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: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor

cursor Bot commented Jul 11, 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_adfdecdc-163f-4a2c-b34a-38383820932b)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat: add implementation plugin (10 implementation-stage skills)

  • Gather context and understand request
  • Fetch fresh plugin docs per CLAUDE.md mandate
  • Review PR diff and changed files
  • Check plugin manifest and marketplace entry
  • Review skill files for correctness and repo-agnostic design
  • Verify security posture
  • Check cross-plugin references and degradation fallbacks
  • Post comprehensive review feedback

Trigger: Merge sync commit 54cf325

This review covers the merge commit (54cf325 Merge remote-tracking branch 'origin/main' into update-merge) that synced the branch with main. The merge changed only .editorconfig (8 lines added, 1 changed) — restoring the Go section that the 9ac7bf8 feature commit had removed. The full PR diff (git diff origin/main...HEAD) now shows zero .editorconfig delta vs. main.

Finding 1 from the prior review (Go section removal bundled without mention) is fully resolved by this merge.


Status of all open findings

# Finding Prior severity Status
1 .editorconfig Go section removed without mention Minor Resolved — merge 54cf325 restored it; PR now clean vs. main
2 External marketplace refs (andrej-karpathy-skills, dotnet-ai, dotnet-template-engine, dotnet-test, document-skills) lack install pointers Low Open
3 verify-changes/evals/evals.json IDs skip 5 and 6 (sequence is 1–4, 7–10) Cosmetic Open
4 notes_dir description claims CLAUDE.md "takes precedence" — mechanism doesn't enforce that Minor Open

Verification of prior P2 Codex fixes (commit 2305865)

Both fixes confirmed correct and still present:

TypeScript globs now include manifests and lockfilesbuild/reference/ecosystem-config.md L40:

globs: ["*.ts", "*.tsx", "*.js", "*.jsx", "*.mjs", "package.json", "package-lock.json", "pnpm-lock.yaml", "yarn.lock", "tsconfig*.json"]

lint/reference/ecosystem-config.md TypeScript globs intentionally remain narrower (source files only — correct, changing a lockfile doesn't require re-linting source).

actionlint in no-arg discovery modelint/reference/ecosystem-config.md L67:

check-cmd: |
  actionlint  # no args — discovers all workflow files itself (.yml and .yaml)

Residual observations (no blockers introduced by the merge)

notes_dir description wording (plugin.json L16): still reads "A working-notes convention declared in your own project's CLAUDE.md or rules takes precedence." Per the fresh docs, ${user_config.notes_dir} is the actual interpolated value — the CLAUDE.md convention doesn't technically override it, the consumer just sets the config option to match. The description is technically misleading. Suggested tightening: "Project-relative directory for implementation artifacts. Set this to match your project's working-notes convention."

Eval ID gap (verify-changes/evals/evals.json): IDs sequence is 1–4, then 7–10. IDs 5–6 absent. Cosmetic only — the eight eval cases that exist are solid.


Overall

The plugin is functionally correct, securely scoped (markdown-only, no hooks, no MCP, no egress, no secrets), and all substantive review findings are now resolved. The merge commit cleanly eliminates the last structural concern. None of the three remaining open items (external-plugin discoverability note, eval ID gap, config description wording) block 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: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed; no reviewers were assigned because no non-author human maintainers are available on this repository.

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: 54cf325bb7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/implementation/skills/build/SKILL.md Outdated
/build and /lint change detection previously saw only git status
--porcelain, so checkpoint-committed work was skipped at pre-PR
verification time. On a clean tree both skills now classify the
branch diff vs the default-branch merge-base; an explicit caller
file list still overrides both paths.

Refs melodic-software/medley#1281

Co-Authored-By: Claude <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 11, 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_9f496efe-7c0b-4ac3-a19f-c57f110f26bc)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat: add implementation plugin (10 implementation-stage skills)

  • Gather context and identify new commit
  • Review new commit diff vs prior state
  • Verify the Codex P2 finding is addressed
  • Check for any new issues introduced
  • Post comprehensive review

New Commit Review — fix: fall back to branch diff when working tree is clean

This commit addresses the Codex P2 finding posted on the previous push. The fix is correct and consistently applied to both affected skills.


Codex P2 — committed branch work now classified on clean tree ✅

build/SKILL.md L57 | lint/SKILL.md L59

-If no changes detected and no `$ARGUMENTS`: report "No uncommitted changes found..."
+If the working tree is clean, fall back to the branch diff —
+  `git diff --name-only $(git merge-base <default-branch> HEAD)..HEAD`
+— so checkpoint-committed work still gets classified...
+If neither path yields changes and no `$ARGUMENTS`: report "No changes found
+(working tree clean, no branch diff vs the default branch)..."

The fix is correct: git merge-base <default-branch> HEAD gives the true branch-vs-main boundary, matching files that were committed on the feature branch but not yet merged. The three-level resolution order — explicit caller list → dirty tree → branch diff — is logically sound and the caller-override clause correctly insulates /verify-changes from having its explicit file list replaced by branch-diff discovery.

The updated exit message is strictly better: "working tree clean, no branch diff vs the default branch" is more diagnostic than "No uncommitted changes found."


One observation — <default-branch> is left to model inference

The git merge-base <default-branch> HEAD command uses <default-branch> as a symbolic placeholder, not a literal git token. The model must infer whether to use main, master, or whatever the remote default is. This is consistent with the skill's general approach (consumer conventions win, context-aware inference), and it will work well in the common main case.

For repos with non-standard default branches (e.g., develop, trunk), the skill would benefit from a one-line hint on how to resolve it — e.g., "detect via git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||' or default to main". That said, this is a refinement, not a correctness problem: a model reading the skill in context of a non-main repo will typically infer the correct branch name from its CLAUDE.md or git remote config.

Refine this →


Minor — argument-hint frontmatter still says "auto-detect from git status"

build/SKILL.md L5

argument-hint: "... — default: auto-detect from git status"

The argument-hint is user-visible at invocation time, but now understates the detection logic (it's git-status + branch-diff fallback). Could be tightened to "auto-detect from changed files" or "auto-detect from git status / branch diff" for consistency with the updated body. Cosmetic only.


All findings — final status

# Finding Severity Status
1 .editorconfig Go section removal bundled without mention Minor ✅ Resolved — merge 54cf325 restored it
2 External marketplace refs lack install pointers Low Open
3 verify-changes/evals IDs skip 5–6 (sequence 1–4, 7–10) Cosmetic Open
4 notes_dir description claims CLAUDE.md "takes precedence" — mechanism doesn't enforce it Minor Open
Codex P2 Build/lint skips committed changes on clean worktree P2 ✅ Resolved — 5db736b
Codex P1 Bare sibling-skill refs break in plugin form P1 Author-declined (locked decision per #1281; smoke-tested)

Overall

The plugin is in good shape. The clean-tree branch-diff fallback correctly resolves the most substantive outstanding finding, and both affected skills were updated consistently. The three remaining open items are cosmetic or documentation-clarity level — none block merging. Security posture is unchanged: markdown-only skills, no hooks, no MCP, no egress.

@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: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor

cursor Bot commented Jul 11, 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_536b0eeb-2cf0-4f57-a71d-1b2424340023)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat: add implementation plugin (10 implementation-stage skills)

  • Gather context and understand request
  • Review latest merge commit and current PR diff vs origin/main
  • Review plugin manifest and key skill files
  • Assess outstanding findings from prior review rounds
  • Post comprehensive review feedback

Trigger: Merge sync commit 023f13c

This merge synced the branch with origin/main to pick up commit 47cfa52 (session-flow: add --bg background-agent launch option to handoff — PR #76). The merge touched only plugins/session-flow/ files. The full PR diff (git diff origin/main...HEAD) remains a clean 43-file add, 2898 insertions, 0 deletions — no delta to any pre-existing file vs main. Nothing in this merge affects the implementation plugin.


Final status of all findings

# Finding Severity Status
1 .editorconfig Go section removed without mention Minor ✅ Resolved — merge 54cf325 restored it; confirmed still clean vs origin/main
Codex P2a TypeScript globs excluded manifests/lockfiles P2 ✅ Resolved — 2305865
Codex P2b actionlint missed .yaml workflows P2 ✅ Resolved — 2305865
Codex P2c Build/lint skips committed changes on clean worktree P2 ✅ Resolved — 5db736b
Codex P1 Bare sibling-skill refs break in plugin form P1 Author-declined per locked decision in medley#1281; smoke-tested ✓
2 External marketplace refs lack install pointers Low Open
3 verify-changes/evals IDs skip 5–6 (sequence 1–4, 7–10) Cosmetic Open
4 notes_dir description claims CLAUDE.md "takes precedence" Minor Open
5 <default-branch> placeholder in git merge-base has no resolution hint Observation Open
6 argument-hint says "auto-detect from git status" — understates the branch-diff fallback Cosmetic Open

Detail on remaining open items

Finding 4 — notes_dir description wording (plugin.json L16)

The description still reads: "A working-notes convention declared in your own project's CLAUDE.md or rules takes precedence."

${user_config.notes_dir} is the interpolated value the skills actually use — a CLAUDE.md convention doesn't technically override it, the consumer just sets the config option to match. The "takes precedence" claim is technically ambiguous.

Suggested tightening: "Project-relative directory for implementation artifacts. Set this to match your project's working-notes convention." Fix this →

Finding 5 — <default-branch> placeholder (build/SKILL.md L57, lint/SKILL.md L59)

Both skills use git diff --name-only $(git merge-base <default-branch> HEAD)..HEAD with <default-branch> as a symbolic placeholder. This works well for main-based repos. For repos using develop, trunk, or another default, the model infers it from context — which is usually correct but is silent about how to resolve it.

A brief parenthetical would make the intent explicit: $(git merge-base <default-branch> HEAD)$(git merge-base <default-branch> HEAD) # detect via: git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||'; default to main. Not a blocker — the skill's consumer-conventions-win framing means a project's CLAUDE.md will typically supply this context. Fix this →

Finding 6 — argument-hint understates detection (build/SKILL.md L5)

argument-hint: "... — default: auto-detect from git status"

This user-visible hint no longer fully describes the detection logic (git status → branch diff fallback). A small tightening: "default: auto-detect from changed files" would be accurate for both detection paths without being verbose. Cosmetic only. Fix this →


What's confirmed correct

  • Manifest (plugin.json): valid schema, explicit version: 0.1.0, correct author/license/keywords, notes_dir with sensible default.
  • Marketplace entry: correct category: development, tags match skill content, JSON placement clean.
  • Repo-agnostic design: all artifact paths use ${user_config.notes_dir}/<topic-slug>/; no hardcoded paths or project-specific values.
  • SSOT decomposition: /build owns detection + commands; /lint owns lint-only; /verify-changes delegates without reimplementing. Chain is coherent.
  • Consumer-conventions-win: explicitly stated in both build and lint reference configs; portable defaults are clearly labeled as such.
  • Graceful degradation: all cross-plugin references use "when [plugin] is installed" + inline fallback — confirmed for sibling refs (/tdd:tdd, /planning:architect, etc.) and external refs (/andrej-karpathy-skills:karpathy-guidelines, dotnet-ai:*, etc.).
  • Security: markdown-only skills, no hooks, no MCP servers, no ../ reach-outs, no absolute paths, no egress, no secrets, no PII.
  • Evals: verify-changes evals cover the key routing behaviors (mode detection, Stage 1 gate, redirect to /verify-improvement, live-app delegation). test-diagnose, test-e2e, test-write, verify-improvement evals are also solid.
  • TypeScript globs: now include package.json, package-lock.json, pnpm-lock.yaml, yarn.lock, tsconfig*.json — dependency/config-only PRs correctly trigger TypeScript verification.
  • actionlint: no-argument discovery mode; covers both .yml and .yaml workflow files.
  • Clean-tree fallback: both build and lint now fall back to branch diff on a clean worktree — checkpoint-committed work gets classified at pre-PR verify time.

Overall

The implementation plugin is merge-ready. All substantive findings across three review rounds are resolved. The five remaining open items are Minor/Cosmetic/documentation-clarity level and none block merging. The latest merge sync is clean — session-flow only, no interference with the implementation plugin.

@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: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit 65f5446 into main Jul 11, 2026
22 checks passed
@kyle-sexton
kyle-sexton deleted the feat/publish-implementation branch July 11, 2026 19:52

@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: 023f13c085

ℹ️ 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".

## Pre-computed context

Working tree status: !`git status --porcelain 2>/dev/null || echo ""`
Changed files (vs HEAD): !`git diff --name-only HEAD 2>/dev/null || echo ""`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include branch-diff files in outcome targeting

In the clean-tree pre-PR flow produced by /implement checkpoint commits, this precomputed list is empty, but Stage 2 later decides E2E triggers and the implementation inventory by “inspect changed files.” Fresh evidence beyond the earlier /build finding is that /verify-changes itself still only exposes git diff --name-only HEAD, so committed API/UI changes can pass the mechanical branch-diff fallback while skipping runtime/outcome checks that depend on the PR diff.

Useful? React with 👍 / 👎.


python:
enabled: true
globs: ["*.py", "pyproject.toml", "uv.lock"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add Python dependency manifests to build detection

For Python dependency/config-only changes such as requirements.txt, poetry.lock, setup.cfg, or pytest.ini, these globs do not classify the Python ecosystem unless a .py, pyproject.toml, or uv.lock file also changed. In those PRs /implementation:build can report no affected ecosystem and skip tests/lint even though the dependency or test/lint configuration change can break the project.

Useful? React with 👍 / 👎.

kyle-sexton added a commit that referenced this pull request Jul 13, 2026
## What

- **Collapse 4 selector jobs into 1**: `ci.yml` ran four identical
per-lane `select-runner.yml` preflights (`select-hygiene`,
`select-hook-utils-sync`, `select-plugin-gate`, `select-miro-plugin`).
They are now one `select-runner` job with the exact same inputs and
observer secret; every lane (`hygiene`, `zizmor`, `hook-utils-sync`,
`plugin-gate`, `miro-plugin`, `runner-policy`) rewires to it with its
existing `!cancelled() && result == 'success'` gate and `outputs.runner
|| 'ubuntu-24.04'` fallback unchanged. `ci-status` and
triggers/concurrency are untouched.
- **Pin bump de50a08 → 3415de3** in `ci.yml` and `pr-title.yml`: the
liveness selector routes on fleet liveness (any online managed runner
keeps the workload self-hosted; GitHub queues on a busy fleet) and drops
the rerun-to-hosted branch (melodic-software/ci-workflows#86). The SHA
is allowlisted owner-scoped for melodic-software in the synced runner
policy (melodic-software/standards#100).
- **Docs**: `docs/CI-RUNNER-ROUTING.md` now describes the
one-preflight-per-workflow contract instead of one selector per
workload.

## Not changed

- `zizmor.yml@de50a08` stays pinned: de50a08 is the only zizmor contract
SHA approved in the standards-distributed `policy.json`; bumping it
would fail the runner-policy gate.

## Verification

- `GITHUB_REPOSITORY=melodic-software/claude-code-plugins node
.github/standards/runner-policy/runner-policy.mjs --root .` → `Runner
policy passed.`
- `actionlint` and `markdownlint-cli2` clean on the changed files.

Part of melodic-software/github-iac#79, epic #78.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01EPDbXgonTuFwFwdTtHaCmw

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes how every CI lane picks runners and bumps the governed
selector contract; mis-routing or selector failure would block
workloads, though gates and fallbacks are unchanged.
> 
> **Overview**
> **Consolidates CI runner selection** so `ci.yml` runs one shared
`select-runner` preflight instead of four duplicate per-lane selector
jobs; all lanes (`hygiene`, `zizmor`, `hook-utils-sync`, `plugin-gate`,
`miro-plugin`, `runner-policy`) still gate on selector success and use
the same `outputs.runner || 'ubuntu-24.04'` expression.
> 
> **Bumps** the pinned `select-runner.yml` reusable workflow from
`de50a08` to `3415de3` in `ci.yml` and `pr-title.yml` (liveness-based
routing; reruns can reuse a prior successful selector result).
**`zizmor.yml` stays on `de50a08`** per runner-policy allowlisting.
> 
> **Updates** `docs/CI-RUNNER-ROUTING.md` to document one selector per
workflow rather than one per workload.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
600d4b5. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Co-authored-by: Claude Fable 5 <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