Skip to content

feat: replace server instructions with skill resources#2374

Draft
SamMorrowDrums wants to merge 7 commits intomainfrom
sammorrowdrums/skills-over-mcp-refactor
Draft

feat: replace server instructions with skill resources#2374
SamMorrowDrums wants to merge 7 commits intomainfrom
sammorrowdrums/skills-over-mcp-refactor

Conversation

@SamMorrowDrums
Copy link
Copy Markdown
Collaborator

Summary

Replaces the server instructions system with MCP skill resources that follow the skill:// URI convention, enabling MCP clients (like pi-mcp-agent) to discover and load domain-specific guidance on demand.

Related: https://github.com/SamMorrowDrums/pi-mcp-agent/issues/2

What changed

Added

  • pkg/github/skill_resources.go — 16 skill resources covering all toolsets, registered as skill://github/{name}/SKILL.md static resources
  • pkg/github/skill_resources_test.go — tests verifying all tools are covered, content format, URI uniqueness, and registration

Removed

  • Server instructionsInstructionsFunc on ToolsetMetadata, generateInstructions(), WithServerInstructions() builder method, and the Instructions field from MCP server options
  • pkg/github/toolset_instructions.go — instruction generator functions
  • pkg/inventory/instructions.go — the generateInstructions function
  • pkg/inventory/instructions_test.go — associated tests

Skills created (16 total)

Skill Tools covered
context get_me, get_teams, get_team_members
repos search_repositories, get_file_contents, list_commits, search_code, etc.
issues issue_read, search_issues, list_issues, issue_write, etc.
pull-requests pull_request_read, create_pull_request, merge_pull_request, etc.
code-security code scanning, secret scanning, dependabot alerts
actions actions_list, actions_get, actions_run_trigger, get_job_logs
discussions list_discussions, get_discussion, etc.
projects projects_list, projects_get, projects_write
notifications list_notifications, dismiss_notification, etc.
gists list_gists, get_gist, create_gist, update_gist
users-orgs search_users, search_orgs
security-advisories global and repository security advisories
labels list_label, list_labels, label_write
git get_repository_tree
stargazers list_starred_repositories, star/unstar
copilot assign_copilot_to_issue, request_copilot_review

How it works

  1. Skills are registered as static MCP resources with skill://github/{name}/SKILL.md URIs
  2. MCP clients discover skills via resources/list (filtering for skill:// URIs)
  3. Each skill has YAML frontmatter with name, description, and allowed-tools
  4. The markdown body contains usage guidance (previously in server instructions)
  5. All tools remain available by default — skills provide context, not gating

Testing

  • script/lint
  • script/test ✅ (all tests pass)
  • New tests verify coverage, content format, and registration

Replace the server instructions system with MCP skill resources that
follow the skill:// URI convention for discovery by MCP clients.

Each skill resource covers a domain of tools and provides YAML
frontmatter (name, description, allowed-tools) plus markdown guidance.

All tools remain available by default (all tools mode). Skills provide
contextual guidance that clients can discover via resources/list.

16 skills covering all toolsets: context, repos, issues, pull-requests,
code-security, actions, discussions, projects, notifications, gists,
users-orgs, security-advisories, labels, git, stargazers, copilot.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums SamMorrowDrums force-pushed the sammorrowdrums/skills-over-mcp-refactor branch from 118f25c to 2140dd3 Compare April 23, 2026 20:35
SamMorrowDrums and others added 6 commits April 24, 2026 10:50
Update all 16 skill definitions to use explicit backtick-formatted
tool names with '## Available Tools' sections instead of bold markdown
references. This makes tool names more reliably parseable by models
that consume the SKILL.md content via load_skill.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…kills

Replace 16 toolset-oriented skills (repos, issues, pull-requests, etc.)
with 27 workflow-oriented skills that map to real user intents:

PR workflows: create-pr, review-pr, self-review-pr, address-pr-feedback, merge-pr
Issue workflows: triage-issues, create-issue, manage-sub-issues
CI/CD: debug-ci, trigger-workflow
Security: security-audit, fix-dependabot, research-vulnerability
Code exploration: explore-repo, search-code, trace-history
Projects: manage-project
Notifications: handle-notifications
Releases: prepare-release
Repository management: manage-repo, manage-labels
Collaboration: contribute-oss, browse-discussions, delegate-to-copilot
Discovery: discover-github, share-snippet, get-context

Each skill focuses on a specific workflow with non-obvious guidance,
anti-patterns, and identity-aware instructions (e.g., reviewing someone
else's PR vs self-reviewing your own). Tools overlap across skills
based on workflow needs rather than API domain.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update all 27 skill descriptions to include both what the skill does
AND when to use it, following the agent skills specification guidance
that descriptions are the primary triggering mechanism. Descriptions
now explicitly list user intents that should activate each skill.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Skills now only include tools that are actually registered with the
MCP server, based on the active toolset configuration. Skills with
no available tools are skipped entirely. This prevents skills from
referencing non-existent tools (e.g., gist tools when the gists
toolset isn't enabled).

RegisterSkillResources now accepts a map of available tool names
built from the inventory at startup time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When skills handle progressive disclosure, the server should expose
all tools via tools/list regardless of toolset configuration. The
client decides tool visibility through skill allowedTools, not the
server.

This replaces the previous filtering approach — now inv.AllTools()
registers every tool with the MCP server, and skills reference the
full tool surface.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
All toolsets are now enabled by default (Default: true). This ensures
every tool is registered out of the box, which is required for skills
to work — skills handle progressive tool discovery, so the server
should expose the full surface. Users can still restrict with
--toolsets if needed.

Reverts the inv.AllTools() registration workaround from server.go
since the normal RegisterAll path now picks up everything.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
olaservo added a commit to olaservo/github-mcp-server that referenced this pull request Apr 28, 2026
Expands the Agent Skills surface from 2 to 28 bundled skills, adds a
per-repo resource template covering arbitrary GitHub repositories, and
introduces a model-facing discovery tool to bridge the gap left by the
SEP's UI-only completion mechanism.

- Import 25 workflow-oriented skills from github#2374
  as standalone SKILL.md files (skip review-pr and handle-notifications
  due to overlap with our pull-requests / inbox-triage). Migrate URI
  shape to skill://github/<name>/SKILL.md to match github#2374's prefix style.

- Remove the legacy Instructions / InstructionsFunc / WithServerInstructions
  machinery (~300 lines). Skills are now the only guidance surface.

- Add a SEP-aligned per-repo resource template
  skill://{owner}/{repo}/{skill_name}/{+file_path} (from PR github#2129, with
  the non-SEP _manifest endpoint dropped). Gated on a new non-default
  `skills` toolset. Index now advertises both SEP entry types
  (skill-md and mcp-resource-template).

- Add list_repo_skills tool that wraps the existing discoverSkills() and
  returns each discovered skill plus a skill:// URL ready for
  resources/read. Workaround for autonomous-agent discovery on unbounded
  template namespaces — documented as such.

- Add discover-mcp-skills meta-skill (always-on) that teaches the model
  the discover-then-read workflow for both bundled and repo-hosted
  surfaces.

Verified end-to-end against anthropics/skills via stdio JSON-RPC: full
loop from index read through SKILL.md to relative-file resolution works.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
olaservo added a commit to olaservo/github-mcp-server that referenced this pull request May 3, 2026
…ll gating

Renames the two original skills to the names Sam uses in
github#2374, and makes their load behavior consistent
with the 25 skills imported from that PR.

- pull-requests → review-pr
- inbox-triage  → handle-notifications

Both renames preserve our richer content (parameter-level workflow detail
for review-pr; High/Medium/Low priority taxonomy + read/done semantics +
discover-mcp-skills cross-link for handle-notifications). Only the names
change, plus the cross-reference inside handle-notifications/SKILL.md.

Load consistency:
- Drop the per-skill `Enabled func() bool` closures. All bundled skills
  now register unconditionally, matching the 25 imported skills and
  Sam's github#2374 semantics.
- Drop the per-skill `Icons` field (octicons.Icons("light-bulb") didn't
  resolve in this fork's icon set anyway, and the imported skills set
  no icons).
- The Registry's gating mechanism stays in place for future use; just
  no shipped skill currently exercises it. Test_DeclareSkillsExtensionIfEnabled's
  "empty registry" subtest covers the gating-off path directly.

Tests:
- Test_PullRequestsSkill_EmbeddedContent → Test_ReviewPRSkill_EmbeddedContent
- Test_InboxTriageSkill_EmbeddedContent → Test_HandleNotificationsSkill_EmbeddedContent
- Test_BundledSkills_Registration (toolset-gating subtests) collapsed into
  Test_BundledSkills_RegisterRegardlessOfToolset
- Test_DeclareSkillsExtensionIfEnabled simplified: 4 subtests → 3, since
  the toolset-specific subtests are now redundant with always-on semantics

Caveat for downstream consumers: any client hardcoding the old URIs
(skill://github/pull-requests/SKILL.md, skill://github/inbox-triage/SKILL.md)
needs to update to the new ones.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
olaservo added a commit to olaservo/github-mcp-server that referenced this pull request May 3, 2026
…ions machinery

Skills are the only guidance surface going forward; the per-toolset
inline instruction system is no longer needed. This removes ~308 lines
of toolset instruction generation.

- Delete pkg/github/toolset_instructions.go (5 generate*ToolsetInstructions
  helpers), pkg/inventory/instructions.go (generateInstructions aggregator),
  and pkg/inventory/instructions_test.go (~265 lines of tests).
- Remove the InstructionsFunc field from inventory.ToolsetMetadata, and
  drop it from the 5 toolsets that used it (Context, Issues, PullRequests,
  Discussions, Projects).
- Remove the generateInstructions field, WithServerInstructions() method,
  and the conditional in Builder.Build() that called generateInstructions(r).
- Remove the instructions field and Instructions() method from Inventory.
- Drop the .WithServerInstructions() call from NewStdioMCPServer
  (internal/ghmcp/server.go) and from DefaultInventoryFactory (pkg/http/handler.go).
- Drop `Instructions: inv.Instructions(),` from the serverOpts assembled
  in NewMCPServer (pkg/github/server.go).

Inspired by the cleanup in github#2374.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
olaservo added a commit to olaservo/github-mcp-server that referenced this pull request May 3, 2026
Per SEP-2640, the bundled-skill URI structure is
skill://<skill-path>/<file-path>, where preceding segments before the
final name are an organizational prefix chosen by the server. This
commit moves from the flat skill://<name>/SKILL.md form to the
prefixed skill://github/<name>/SKILL.md form, matching the convention
used in github#2374.

Single-line change to skills.Bundled.URI(). Tests already build URIs
through this helper, so they pick up the change automatically — no
test edits needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
olaservo added a commit to olaservo/github-mcp-server that referenced this pull request May 3, 2026
…ll gating

Renames the two original skills to the names Sam uses in
github#2374, and switches them from toolset-gated
loading to always-on, matching how upstream's skills load.

- pull-requests → review-pr
- inbox-triage  → handle-notifications

Both renames preserve the original content (parameter-level workflow
detail for review-pr; High/Medium/Low priority taxonomy + read/done
semantics for handle-notifications). Only the names change, plus the
cross-reference inside handle-notifications/SKILL.md (was → review-pr).

Load consistency:
- Drop the per-skill `Enabled func() bool` closures so both skills
  register unconditionally, matching upstream's "everything always
  available" semantics.
- Drop the per-skill `Icons` field. The `light-bulb` octicon used by
  pull-requests didn't resolve in this fork's icon set anyway, and
  shipping no icons is the simpler, more consistent default.
- The Registry's `Enabled` closure mechanism stays in place for future
  use; just no shipped skill currently exercises it. A new
  Test_DeclareSkillsExtensionIfEnabled subtest covers the
  empty-registry path directly.

Tests:
- Test_PullRequestsSkill_EmbeddedContent → Test_ReviewPRSkill_EmbeddedContent
- Test_InboxTriageSkill_EmbeddedContent → Test_HandleNotificationsSkill_EmbeddedContent
- Test_BundledSkills_Registration (toolset-gating subtests) collapsed into
  Test_BundledSkills_RegisterRegardlessOfToolset.
- Test_DeclareSkillsExtensionIfEnabled simplified from 4 subtests to 3.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
olaservo added a commit to olaservo/github-mcp-server that referenced this pull request May 3, 2026
Imports the workflow-oriented skill catalogue from
github#2374 as standalone SKILL.md files served via
the existing Registry. Each skill covers a discrete GitHub workflow
the model can pull on demand rather than loading all guidance up-front.

Imported skills (25): get-context, explore-repo, search-code,
trace-history, create-pr, self-review-pr, address-pr-feedback,
merge-pr, triage-issues, create-issue, manage-sub-issues, debug-ci,
trigger-workflow, security-audit, fix-dependabot, research-vulnerability,
manage-project, prepare-release, manage-repo, manage-labels,
contribute-oss, browse-discussions, delegate-to-copilot,
discover-github, share-snippet.

Skipped from github#2374 (2): review-pr and handle-notifications — those
names are now used by the renamed-and-deepened skills from the prior
commit, so the catalogue ends up with github#2374's names + this fork's
deeper content for those two.

Each skill is a `skills/<name>/SKILL.md` file embedded via //go:embed,
plus a single Registry.Add() call in pkg/github/bundled_skills.go.
The `allowed-tools` frontmatter list is included verbatim from github#2374
as advisory metadata (not enforced; not surfaced via resource _meta).

Tests: adds Test_BundledSkills_NoDuplicateURIs,
Test_BundledSkills_AllFrontmatterValid, and
Test_BundledSkills_AllReadable — these scale across the full catalogue
and protect against regressions when adding more skills.

Co-Authored-By: Claude Opus 4.7 (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