ceos/fastmcp
1
0
Fork 0
mirror of https://github.com/PrefectHQ/fastmcp.git synced 2026-08-09 07:09:11 +02:00
Code Issues Projects Releases Packages Wiki Activity

Add contract gate to review-issue skill (#4214)

Browse source
This commit is contained in:
Jeremiah Lowin 2026-05-22 20:08:48 -04:00 • committed by GitHub
commit 8382569027
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 17 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

18
.claude/skills/review-issue/SKILL.md
View file

@ -56,7 +56,7 @@ gh pr view <pr> --repo PrefectHQ/fastmcp --json number,title,body,labels,files,a
gh pr view <pr> --repo PrefectHQ/fastmcp --comments
```
## Step 2 — Classify the issue (is it valid?)
## Step 2 — Classify the issue (is it valid AND a real bug?)
- Is there a real, reproducible problem? For bugs, demand an MRE that shows FastMCP misbehaving
— not user config error, not a question, not an upstream-SDK issue.
@ -64,6 +64,22 @@ gh pr view <pr> --repo PrefectHQ/fastmcp --comments
- If the issue itself is weak, **stop here and decline** — don't evaluate the PR. A good PR
attached to a bad issue is still declined.
**A reproducible MRE is not the same as a bug.** This is the trap that produces wrong verdicts:
an MRE can demonstrate real, observable behavior that is nonetheless *not a bug*, because it
violates no contract the framework intends to hold. The decisive question is not "does this
reproduce?" but "does the demonstrated behavior violate the intended contract for this API?" A
shared-mutable-state MRE only matters if callers are *supposed* to mutate that state; an
ordering/timing MRE only matters if the framework promises an order; a "wrong" value only matters
relative to what the API guarantees. An MRE that has to reach past the supported surface to
trigger the behavior (mutating a field meant to be set only at construction, depending on an
internal that isn't part of the public contract) is showing you a property, not a defect.
You usually cannot read the intended contract off the code — the code shows what it *does*, not
what it *promises*. **The maintainer is often the only authoritative source for the contract, so
stopping to ask is legitimate and expected here.** Ask "is X a supported pattern / does this API
promise Y?" before sinking time into investigating a fix. If the behavior is in-contract correct,
decline — no matter how cleanly the PR fixes it, and no matter how real the MRE looks.
## Step 3 — Investigate the PR (mandatory; do NOT skip if a PR exists)
The most common failure of this skill is judging a PR from the diff hunk and the PR description