Skip to content

Add a contribution guide - #6

Merged
mgrabina merged 2 commits into
mainfrom
contributing-guide
Sep 7, 2026
Merged

mgrabina merged 2 commits into
mainfrom
contributing-guide

Conversation

@mgrabina

@mgrabina mgrabina commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Adds CONTRIBUTING.md.

What a proposal needs

A skill is judged on whether it changes what an agent does, so the guide asks for exactly that: the prompt, the sequence of MCP tools the agent called without the skill, the sequence with it, and one sentence on why the difference matters. It also asks for more than one attempt per arm, since a single run says very little about a change in behaviour.

The two most common reasons a proposal does not land are stated up front: a skill that restates what the tool descriptions already say costs context on every turn and buys nothing, because every description loads whether or not the skill fires; and a skill written for a flow the tools already handle is a fix for a problem that is not there.

There is also a short section on when the right change is a tool description in aave/mcp instead, where every client gets it and nothing is paid for it on turns where it does not apply.

On the renames key

An earlier push removed "renames": { "mcp": "aave-mcp" } from the marketplace manifest, because
claude plugin validate . rejected it. That was wrong and is no longer part of this branch.

The key is valid. It was added in Claude Code 2.1.193 and the CLI I ran was 2.1.114, so the grep of
that binary was accurate and the conclusion drawn from it was not. Only plugin validate is strict
about unknown keys; every runtime path strips them, which is why marketplace add and install
worked throughout. Confirmed both ways since: on 2.1.260 locally, and by #7's CI, which installs the
current CLI and passes claude plugin validate . against main unchanged.

Keeping the map matters. main shipped "name": "mcp" for 17 days, and the README in that window
published /plugin install mcp@aave along with a committable enabledPlugins: { "mcp@aave": true }
snippet that still sits in other people's repos. Without the entry those resolve to
plugin-not-found. The docs say to treat it as append-only history.

Also

The README's Contributing section now points at the guide instead of carrying the instructions itself.

Licence in the guide is stated as MIT, matching LICENSE. Flagging that the bsl-license branch is still open and unmerged, so if the intent is to relicense, it is easier to settle before contributions arrive than after.

@andreaszig

Copy link
Copy Markdown
Contributor

Good guide — it states the rule we ended up applying. A few additions from running that rule 200-odd times over the last two weeks; each one cost us a wrong verdict at some point.

  • Run both arms on a small model too, not only the one you use. Our first pass on one model dropped yield-analysis and tx-confirmation; on Haiku 4.5 both turn a wrong answer into a right one (Haiku never reads the APY history bare, and omits chainId on the v3 activity feed so a wallet with a live Base position comes back as inactive). The plugin ships to every client regardless of model, so the small-model run is the one that decides.
  • Strip the harness of everything except the server. No shell, no file tools, no user-level skills in either arm. Two of our early "wins" were artefacts: one skill only won because the bare agent had curl, another was confounded by a local wallet CLI. A skill that only wins when the bare agent has a shell has not won.
  • Same sequence, same answer → drop, whatever the cost number says. The guide names redundancy as the main rejection reason; worth stating the test. 10 of our 18 candidates had an identical tool sequence at +8% to +152% cost. Cost is recorded, not decisive.
  • A skill that fires and changes nothing is also a fail. Sonnet 5 invoked our deleverage skill 3/3 and still didn't simulate a route. The check is the tool sequence, not the Skill call.
  • Re-run the shipped skills after every server release, and expect to delete some. Within a week the server's descriptions absorbed three of our winning lines (prepare_set_collateral now simulates and refuses itself, get_apy_history says read both sides, get_swap_quote says check coverage first). The set should shrink over time; if the guide doesn't say so, nobody deletes anything.
  • File the bare-arm failure against aave/mcp alongside the skill. Your "belongs in the server" section covers this in spirit — I'd make it a checklist item: the sequence the bare agent got wrong is a tool-description bug first, skill evidence second.
  • Anchor the description to the tool, not only the topic. Three of our skills didn't fire until the description named the tool ("before any get_swap_quote call"). Adds to the "write around what a user would ask" line.
  • Use a wallet with a live position. Against an empty wallet, "no position" is the right answer to everything and every skill passes trivially — and the bare agent invented a placeholder address (0x…0001) twice when none was given.

Method and per-cell results behind these are in the description of #5.

mgrabina added a commit that referenced this pull request Aug 27, 2026
From review on #6: require a small-model pair and a bare, live-position
harness so the comparison measures the skill and nothing else; state the
redundancy test (same sequence, same answer, drop it) and that a skill
firing without changing the sequence also fails; have descriptions name
the tool they precede; re-run shipped skills after server releases and
delete the ones the server absorbed, filing bare-arm failures against
aave/mcp.
@mgrabina

Copy link
Copy Markdown
Contributor Author

All eight are in, folded into the existing sections rather than appended. The re-run-and-delete point got its own section, since it changes what maintainers do rather than what contributors send. Agreed on filing the bare-arm sequences against aave/mcp; the kept lines from #5 are the first batch.

CONTRIBUTING.md sets out what a skill proposal needs: the prompt, the tool
sequence the agent produced with the skill and without it, and why the
difference matters. A skill is judged on whether it changes what the agent
does, so that is what a proposal has to show.

The README's Contributing section now points at the guide rather than carrying
the instructions itself.
From review on #6: require a small-model pair and a bare, live-position
harness so the comparison measures the skill and nothing else; state the
redundancy test (same sequence, same answer, drop it) and that a skill
firing without changing the sequence also fails; have descriptions name
the tool they precede; re-run shipped skills after server releases and
delete the ones the server absorbed, filing bare-arm failures against
aave/mcp.
@mgrabina mgrabina changed the title Add a contribution guide, and make its first instruction work Add a contribution guide Sep 4, 2026
@mgrabina
mgrabina merged commit 82746bc into main Sep 7, 2026
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.

3 participants