Sammorrowdrums/sandman readme rewrite - #34
Merged
Merged
Conversation
Rewrite README as a Neil Gaiman Sandman-inspired narrative introducing the three tiers of progressive MCP tool discovery: - The Skill Dealer (Tier 1): self-referential skill-gated tool visibility - The Nuclear Football (Tier 2): shell-native CLI progressive discovery - Codey C. Maude (Tier 3): sandboxed code mode for read-only tools Add character and banner artwork in images/. Technical details are accurate — the narrative weaves around the actual architecture of skill registries, JSON-RPC tool-cli, and isolated-vm V8 sandboxes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GitHub renders mermaid code blocks natively, so these diagrams will display properly regardless of font or viewer. Replaces the three ASCII box-drawing diagrams (skill flow, tool-cli architecture, and overall tier architecture) with equivalent mermaid flowcharts and sequence diagrams. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Skill invocation pushes tools to the model (unsolicited tool definitions via setActiveTools) rather than having the model pull them via search. Both solve cache invalidation from large tool lists, but from opposite directions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Guide for MCP server developers covering: - skill:// resource format (frontmatter, allowed-tools, SKILL.md body) - Registering skill resources with the MCP SDK - Best practices for grouping tools into skills - Tool annotations (readOnlyHint) and outputSchema for Code Mode - Returning structuredContent for typed sandbox access - Complete working example with all three tiers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add npm badges for mcpi, mcpi-ext, and tool-cli packages at the top. Rework the Nuclear Football section to include the dual-lock metaphor: agent holds the briefcase (reach), harness holds the launch authority (safety). Every call routes through the harness for observability and HITL gating. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve merge conflicts by accepting main's significantly reworked README, then re-applying this branch's three additions: - npm badges for mcpi, mcpi-ext, and tool-cli - Anthropic tool search reference (push vs pull) in Skill Dealer section - Dual-lock framing in Nuclear Football section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR rewrites the project README to present the “tiered progressive discovery” architecture (Skills, tool-cli, Code Mode) with updated onboarding instructions, and adds a new guide for MCP server authors explaining how to integrate with the extension’s discovery/gating model.
Changes:
- Major README rewrite with architecture narrative, diagrams, and updated Quick Start commands.
- Added
docs/server-developer-guide.mddescribing skill resources, tool annotations, andoutputSchema/structuredContentexpectations for Code Mode. - Added/embedded new image assets and npm package badges in README.
Show a summary per file
| File | Description |
|---|---|
| README.md | Replaces the minimal README with a detailed architecture + onboarding doc (Skills/tool-cli/Code Mode), diagrams, images, and updated Quick Start. |
| docs/server-developer-guide.md | New documentation for MCP server developers on skills, gating, annotations, and structured outputs for progressive discovery/code mode. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (4)
README.md:25
- The markdown table syntax here uses a double leading pipe (
|| ...). That renders as an empty first column (or breaks table rendering depending on the renderer). Use single leading pipes (| ...) for each row and the separator line.
> _They will tell you that MCP has a context problem. That the protocol gives too many tools, that the model drowns in schemas it doesn't need, that the cost of knowing everything is losing the ability to do anything well._
>
> _They are wrong._
>
> _MCP doesn't have a context problem. It has an imagination problem. The protocol already contains everything you need — `skill://` resources, tool annotations, `outputSchema`, progressive discovery. The pieces are all there, lying in the open like runes on a hillside. You just have to read them._
README.md:149
- This markdown table also uses
||at the start of each row, which introduces an unintended empty first column. Switch these rows to standard|-prefixed table syntax so the table renders consistently.
The harness controls what the model sees. MCP servers just expose their tools and skills. The extension decides _when_ and _how_ to reveal them.
### Every call flows through the harness
README.md:186
- Quick Start now recommends
npm install, but the repo still includes anaube-lock.yamlat the root. Ifaubeis no longer part of the supported workflow, consider removing/archiving that lockfile (or calling out in the README that it’s legacy) to avoid confusing new contributors.
"ghcr.io/github/github-mcp-server:skill-discovery",
"stdio"
],
"env": {
README.md:41
- The README describes skill-gated tools as being hidden until
load_skill, but the implementation notes this requirespi >= 0.70.0for dynamic tool refresh. It would help to document the minimum pi version (or the fallback behavior on older pi versions) so users don’t get surprised.
| 1 — Skills | **The Skill Dealer** | `skill://` resources gate tools via `allowed-tools` |
| 2 — tool-cli | **The Nuclear Football** | CLI progressive discovery via shell |
| 3 — Code Mode | **Codey C. Maude** | Sandboxed JS over read-only tools with `outputSchema` |
---
## I. The Skill Dealer
- Files reviewed: 2/2 changed files
- Comments generated: 3
Comment on lines
+9
to
+13
| | What you provide | Tier | What happens | | ||
| |------------------|------|--------------| | ||
| | `skill://` resources with `allowed-tools` | **Tier 1 -- Skills** | Tools hidden until the model loads the skill | | ||
| | Nothing special | **Tier 2 -- tool-cli** | Tools discoverable via CLI progressive exploration | | ||
| | `readOnlyHint: true` + `outputSchema` | **Tier 3 -- Code Mode** | Tools callable from sandboxed JavaScript | |
Comment on lines
+56
to
+60
| | Field | Required | Description | | ||
| |-------|----------|-------------| | ||
| | `name` | Yes | Skill identifier. The model uses this with `load_skill("name")`. Keep it short and descriptive. | | ||
| | `description` | Yes | One-line summary shown in the skill catalog. Helps the model decide which skill to load. | | ||
| | `allowed-tools` | Yes | Array of tool names this skill gates. These tools are hidden until the skill is loaded. | |
Comment on lines
+28
to
+30
| 4. Tools listed in `allowed-tools` are hidden from the model until `load_skill` is called | ||
| 5. When the model calls `load_skill("your-skill")`, the tools appear and the SKILL.md body is returned as instructions | ||
|
|
Add the skills-as-groups proposal frontmatter format alongside the current allowed-tools format. Document both with examples showing metadata.io.modelcontextprotocol/tools (space-separated) vs allowed-tools (YAML array). Add Further Reading section linking to the spec proposal, progressive discovery docs, and mechanism docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Parse both frontmatter formats for skill tool declarations: - Proposed: metadata.io.modelcontextprotocol/tools (space-separated) - Current: allowed-tools (YAML array) Prefers the proposed format when both are present. This enables mcpi-ext to work with servers adopting the skills-as-groups proposal (github/github-mcp-server#2465) while maintaining backward compat with the current allowed-tools format. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.