Skip to content

Add vMCP anti-pattern review skill#4172

Open
jerm-dro wants to merge 3 commits intomainfrom
jerm/2026-03-16-vmcp-anti-pattern-review-skill
Open

Add vMCP anti-pattern review skill#4172
jerm-dro wants to merge 3 commits intomainfrom
jerm/2026-03-16-vmcp-anti-pattern-review-skill

Conversation

@jerm-dro
Copy link
Contributor

@jerm-dro jerm-dro commented Mar 17, 2026

Summary

  • vMCP code reviews lack a shared vocabulary for structural issues that make the codebase harder to understand and more brittle. This adds a Claude Code skill (/vmcp-anti-pattern-review) that codifies 9 anti-patterns identified through codebase analysis, each with detection guidance and concrete alternatives.
  • The catalog covers: context variable coupling, repeated body read/restore, god object, middleware overuse, string-based error classification, silent error swallowing, SDK coupling leakage, config sprawl, and mutable shared state through context.

Type of change

  • Other (describe): Developer tooling — Claude Code skill for code review

Test plan

  • Manual testing (describe below)

Invoked /vmcp-anti-pattern-review in Claude Code and verified it loads the skill and anti-pattern catalog correctly.

Changes

File Change
.claude/skills/vmcp-anti-pattern-review/SKILL.md Skill definition with instructions for scoping, checking, classifying, and presenting findings
.claude/skills/vmcp-anti-pattern-review/ANTI-PATTERNS.md Catalog of 9 anti-patterns with definitions, detection heuristics, known instances, and recommended alternatives

Does this introduce a user-facing change?

No. This is internal developer tooling for AI-assisted code review.

Special notes for reviewers

The anti-patterns were identified through analysis of the current vMCP codebase (pkg/vmcp/, cmd/vmcp/). The "known instances" sections reference real code locations. The catalog is intended to evolve — new anti-patterns can be added as they're identified, and existing entries updated as the codebase improves.

Key design decisions in the alternatives:

  • Anti-pattern #1 (context coupling) recommends pushing data onto MultiSession as the primary alternative, since it's a well-typed domain object that's 1:1 with requests and decoupled from protocol concerns.
  • Anti-pattern #4 (middleware overuse) similarly recommends domain objects over middleware for non-cross-cutting concerns.
  • Anti-pattern #7 (SDK coupling) accepts the two-phase session pattern as necessary but emphasizes containing it at the adapter boundary.

Generated with Claude Code

@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Mar 17, 2026
@codecov
Copy link

codecov bot commented Mar 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.85%. Comparing base (29a2c67) to head (96cbb59).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4172      +/-   ##
==========================================
- Coverage   68.85%   68.85%   -0.01%     
==========================================
  Files         467      467              
  Lines       46983    46983              
==========================================
- Hits        32349    32348       -1     
- Misses      11974    11977       +3     
+ Partials     2660     2658       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jerm-dro jerm-dro requested review from amirejaz and yrobla March 17, 2026 00:54
@JAORMX
Copy link
Collaborator

JAORMX commented Mar 17, 2026

@jerm-dro is generating an anti-pattern skill the way to go? What about having a code-review skill for vmcp in general and writing in the skill that it must watch for antipatterns. Those antipatterns could then go in a reference in the skill. wdyt?

@aponcedeleonch
Copy link
Member

aponcedeleonch commented Mar 17, 2026

I would recommend:

Ideally, since vMCP is part of the same CC project that toolhive, I would try to consolidate in the existing Agent code-reviewer

I am proposing: #4189
It's just a coincidence that I started to work on this on Friday. But now I have a pretty decent idea on how each of our CC resources are distributed

@jerm-dro
Copy link
Contributor Author

  • Besides listing anti-patterns (negative), also list what would be what it should do instead (positive)

The current PR has What to Do Instead sections for each anti-pattern. Did you have something else in mind? It's hard to be too prescriptive, since the ideal change depends largely on the goal of the change and the state of the code at the time of writing.

Nice, I'll change this to also use a rule.

Ideally, since vMCP is part of the same CC project that toolhive, I would try to consolidate in the existing Agent code-reviewer

I'll also update the code-reviewer 🫡

jerm-dro and others added 2 commits March 17, 2026 12:06
Introduces a Claude Code skill that reviews vMCP code for 9 known
anti-patterns: context variable coupling, repeated body read/restore,
god object, middleware overuse, string-based error classification,
silent error swallowing, SDK coupling leakage, config sprawl, and
mutable shared state through context. Each anti-pattern includes
detection guidance and concrete alternatives.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move anti-patterns catalog to docs/vmcp-anti-patterns.md as a single
source of truth. Create a glob-scoped rule for automatic loading when
editing vMCP code, rename the skill to vmcp-review, and integrate it
into the code-reviewer agent as an additional check for vMCP changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jerm-dro jerm-dro force-pushed the jerm/2026-03-16-vmcp-anti-pattern-review-skill branch from 05f039e to b2800c1 Compare March 17, 2026 19:07
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 17, 2026
@jerm-dro
Copy link
Contributor Author

@JAORMX @aponcedeleonch Thanks for the review. I've addressed your comments. PTAL!

@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 17, 2026
@jerm-dro jerm-dro requested a review from JAORMX March 17, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants