Repository navigation
Add Skills as Resources reference implementation #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
b1d7b7f
Add Skills as Resources reference implementation
olaservo 0ad4f46
Align skills-as-resources with skillsdotnet conventions
olaservo e8105f4
Remove Python skills-as-resources implementation
olaservo 29a3bef
Remove Python references from README
olaservo af48856
Add resources/subscribe + resources/unsubscribe support
olaservo 09882d3
Update README to reflect resource subscription support
olaservo 1425e12
Fix skillsdotnet attribution to correct author (PederHP)
olaservo 225a9cf
Add file watching for dynamic skill discovery (resources/listChanged)
olaservo 4eaaf86
Remove stale "Intentionally Omits" section from README
olaservo 6a5ff31
Apply suggestions from code review
olaservo 581587b
Add MCP resource annotations (audience, priority, lastModified)
olaservo b186b9d
Add tool annotations to load_skill and bump SDK to 1.27.0
olaservo 1ce3df9
Bump @modelcontextprotocol/sdk to 1.27.1
olaservo 048ae19
Enhance README with detailed descriptions for supporting files and dy…
olaservo ee33c77
Add comparison section to README highlighting differences with FastMC…
olaservo 9060a51
Add smoke test script for Skills as Resources MCP server
olaservo cd92e59
Bump sdk in package lock
olaservo b7fbb44
Merge remote-tracking branch 'origin/main' into add-skills-as-resourc…
olaservo 726b92a
Remove load_skill tool; adopt resources-only server architecture
olaservo d3691cd
Add @ext-modelcontextprotocol/skills SDK; refactor example to consume it
olaservo 825cf6b
Update example README to reference SDK instead of pseudocode sketch
olaservo e187e3e
Merge remote-tracking branch 'origin/main' into add-skills-as-resourc…
olaservo 04a88a8
Refactor SDK to enhance resource handling and update documentation
olaservo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,11 @@ | ||
| .claude/settings.local.json | ||
|
|
||
| # Local reference repos and planning docs | ||
| TODO.md | ||
|
|
||
| # Build artifacts | ||
| node_modules/ | ||
| dist/ | ||
| __pycache__/ | ||
| *.egg-info/ | ||
| .eggs/ |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| # Skills as Resources — Reference Implementation | ||
|
|
||
| > **Experimental** — This is a reference implementation for evaluation by the Skills Over MCP Interest Group. | ||
|
|
||
| ## Pattern Overview | ||
|
|
||
| This example demonstrates the **Resources approach** from [`docs/approaches.md`](../../docs/approaches.md): exposing agent skills via MCP resources using the `skill://` URI scheme, combined with a `load_skill` tool for model-controlled progressive disclosure. | ||
|
|
||
| An MCP server scans a directory for SKILL.md files and exposes them as resources and tools: | ||
|
|
||
| | Type | Name / URI | MIME Type | Purpose | | ||
| | :--- | :--- | :--- | :--- | | ||
| | Resource | `skill://{name}/SKILL.md` | `text/markdown` | Full SKILL.md content (listed) | | ||
| | Resource | `skill://{name}/_manifest` | `application/json` | File inventory with SHA256 hashes (listed) | | ||
| | Resource | `skill://{name}/{+path}` | varies | Supporting file (template, not listed) | | ||
| | Resource | `skill://prompt-xml` | `application/xml` | XML for system prompt injection (optional) | | ||
| | Tool | `load_skill` | — | Model-controlled skill loading | | ||
|
|
||
| The URI scheme is aligned with the [SkillsDotNet](https://github.com/bradwilson/skillsdotnet) conventions, enabling interoperability between TypeScript and C# implementations. Clients can discover skills by scanning `resources/list` for URIs matching `skill://*/SKILL.md`. | ||
|
|
||
| This is a **hybrid** approach: resources provide **application-controlled** access (the host/client decides when to read), while the `load_skill` tool provides **model-controlled** access (the LLM decides when to invoke). See [Open Question #9](../../docs/open-questions.md) for the control model discussion. | ||
|
|
||
| ## How It Works | ||
|
|
||
| ``` | ||
| ┌─────────────┐ ┌──────────────────────┐ ┌──────────────┐ | ||
| │ MCP Client │────▶│ Skills as Resources │────▶│ Skill Files │ | ||
| │ (e.g. Claude│◀────│ MCP Server │◀────│ (SKILL.md) │ | ||
| │ Code) │ └──────────────────────┘ └──────────────┘ | ||
| └─────────────┘ | ||
| ``` | ||
|
|
||
| 1. **Startup**: Server scans the configured skills directory for `*/SKILL.md` files and supplementary documents; computes SHA256 hashes and builds file manifests | ||
| 2. **Discovery**: Parses YAML frontmatter to extract `name` and `description` | ||
| 3. **Registration**: Registers static resources (`SKILL.md` + `_manifest`) for each skill, a `ResourceTemplate` for supporting files, and a `load_skill` tool | ||
| 4. **Progressive disclosure** (two paths): | ||
| - **Application-controlled** (via resources): `resources/list` → scan for `skill://*/SKILL.md` → read `skill://{name}/SKILL.md` on demand → read `skill://{name}/_manifest` for file inventory → read `skill://{name}/{path}` for supporting files | ||
| - **Model-controlled** (via tool): `tools/list` → discover `load_skill` with available skill names in description → call `load_skill("code-review")` to get full content | ||
| 5. **System prompt injection**: `skill://prompt-xml` provides XML that hosts can inject into system prompts | ||
| 6. **Capability declaration**: Server declares `resources.listChanged` and `resources.subscribe` capabilities | ||
| 7. **Resource subscriptions**: Clients can call `resources/subscribe` on any `skill://` URI to receive `notifications/resources/updated` when the underlying file(s) change on disk. Watchers are created on-demand via [chokidar](https://github.com/paulmillr/chokidar) and cleaned up on unsubscribe. | ||
|
|
||
| ## Implementations | ||
|
|
||
| ### TypeScript | ||
|
|
||
| **Prerequisites**: Node.js >= 18, npm | ||
|
|
||
| ```bash | ||
| cd typescript | ||
| npm install | ||
| npm run build | ||
| ``` | ||
|
|
||
| **Run with MCP Inspector**: | ||
| ```bash | ||
| npx @modelcontextprotocol/inspector node dist/index.js ../sample-skills | ||
| ``` | ||
|
|
||
| **Development mode** (no build step): | ||
| ```bash | ||
| npm run dev -- ../sample-skills | ||
| ``` | ||
|
|
||
| ## Security Features | ||
|
|
||
| The implementation includes: | ||
|
|
||
| - **Path traversal protection** — Resolved paths are checked against the skills directory boundary using `realpathSync`. Symlink escapes are detected. | ||
| - **Skill name validation** — Resources and the `load_skill` tool look up names by key in the discovered skills map. User input is never used to construct file paths. | ||
| - **File path validation** — Paths containing `..` are rejected. All paths are verified to be within the skills directory. | ||
| - **File size limits** — Files larger than 1MB are skipped during discovery and rejected on read. | ||
| - **Safe YAML parsing** — Uses the `yaml` package which is safe by default. | ||
| - **Content integrity** — The `_manifest` resource includes SHA256 hashes for all files, enabling clients to verify downloaded content. | ||
|
|
||
| ## Sample Skills | ||
|
|
||
| Two sample skills are included in `sample-skills/` for testing: | ||
|
|
||
| | Skill | Description | Documents | Notes | | ||
| | :--- | :--- | :--- | :--- | | ||
| | `code-review` | Structured code review methodology | `references/REFERENCE.md` | Tests document scanning, `_manifest` with hashes, and `ResourceTemplate` | | ||
| | `git-commit-review` | Review commits for quality and conventional format | None | Tests basic skill resource with no supporting files | | ||
|
|
||
| ## Key Design Decisions | ||
|
|
||
| - **Hybrid approach (resources + tool)**: Resources provide application-controlled access for hosts that want to manage context. The `load_skill` tool provides model-controlled access for progressive disclosure. Experimental findings show models reliably use tools but tend to ignore resources (see [`docs/experimental-findings.md`](../../docs/experimental-findings.md)), making the hybrid approach more practical than resources alone. | ||
| - **URI scheme aligned with skillsdotnet**: The `skill://{name}/SKILL.md` and `skill://{name}/_manifest` URI conventions match the [SkillsDotNet](https://github.com/bradwilson/skillsdotnet) C# implementation. This enables cross-implementation interoperability — a client-side `SkillCatalog` can discover skills from either implementation by scanning `resources/list` for `skill://*/SKILL.md` URIs. | ||
| - **`_manifest` with SHA256 hashes**: Pre-computed at startup with file sizes and content hashes. Enables download/sync workflows and cache invalidation without re-reading files on each request. | ||
| - **Listed resources for skills, template for supporting files**: Each skill's `SKILL.md` and `_manifest` are concrete resources visible in `resources/list`. Supporting files are accessed via a `ResourceTemplate` (`skill://{name}/{+path}`) and are discoverable through the `_manifest` — keeping `resources/list` clean. | ||
| - **`skill://prompt-xml` for injection**: Optional convenience resource that allows hosts to inject skill awareness into system prompts using the resources primitive. | ||
|
|
||
| ## How This Differs from Skills as Tools | ||
|
|
||
| | Aspect | Skills as Tools | Skills as Resources (this example) | | ||
| | :--- | :--- | :--- | | ||
| | Control model | Model-controlled only | Hybrid: application-controlled (resources) + model-controlled (`load_skill` tool) | | ||
| | MCP Primitive | Tools | Resources + Tools | | ||
| | Discovery | Tool description + `list_skills` call | `resources/list` scan for `skill://*/SKILL.md` + `load_skill` tool description | | ||
| | Loading | `read_skill(name)` tool call | `resources/read` on `skill://{name}/SKILL.md` or `load_skill(name)` tool call | | ||
| | File inventory | Not demonstrated | `skill://{name}/_manifest` with SHA256 hashes | | ||
| | System prompt | Via tool description embedding | Via `skill://prompt-xml` resource or `load_skill` tool description | | ||
| | Input validation | Zod schema on tool parameters | URI template matching (resources) + Zod schema (`load_skill` tool) | | ||
| | Supporting files | Not demonstrated | `ResourceTemplate` for files, `_manifest` for discovery | | ||
|
|
||
| ## Answers to Open Question #12 | ||
|
|
||
| > "Why not just use resources?" | ||
|
|
||
| This implementation shows that resources **do work** for skill delivery, and that combining them with a `load_skill` tool creates a more practical system. Key findings for evaluation: | ||
|
|
||
| - **Discovery**: Skills appear in `resources/list` as `skill://*/SKILL.md`, making them immediately visible to any MCP-aware client. The `load_skill` tool description also lists available skills for model discovery. | ||
| - **Progressive disclosure**: The URI hierarchy (`SKILL.md` → `_manifest` → `{path}`) provides layered loading. The `load_skill` tool provides an alternative on-demand loading path. | ||
| - **System prompt injection**: `skill://prompt-xml` provides a clean mechanism for hosts to inject skill awareness | ||
| - **Control model**: The hybrid approach gives both the host (via resources) and the model (via `load_skill` tool) agency over when skill content gets loaded | ||
| - **Interoperability**: The `skill://` URI convention is shared with skillsdotnet, enabling cross-implementation discovery | ||
|
|
||
| ## Relationship to Other Approaches | ||
|
|
||
| | Approach | How it differs | | ||
| | :--- | :--- | | ||
| | **1. Skills as Primitives** (SEP-2076) | Uses dedicated `skills/list` and `skills/get` protocol methods instead of resources | | ||
| | **3. Skills as Tools** (sibling example) | Uses MCP tools only (model-controlled) instead of the hybrid resources + tool approach | | ||
| | **5. Server Instructions** | Uses server instructions to point to resources instead of exposing resources directly | | ||
| | **6. Convention** | This example could become part of a documented convention pattern | | ||
|
|
||
| ## Convergence with SkillsDotNet | ||
|
|
||
| The URI scheme in this implementation is aligned with [SkillsDotNet](https://github.com/bradwilson/skillsdotnet), a C# implementation of the same pattern. Both implementations use: | ||
|
olaservo marked this conversation as resolved.
Resolved
|
||
|
|
||
| - `skill://{name}/SKILL.md` — listed resource for skill content | ||
| - `skill://{name}/_manifest` — listed resource for file inventory (with SHA256 hashes) | ||
| - `skill://{name}/{+path}` — resource template for supporting files | ||
|
|
||
| This convergence enables a future client-side `SkillCatalog` to discover and load skills from either implementation without knowing which language the server is written in. The key client-side pattern (from skillsdotnet) is: | ||
|
|
||
| 1. Scan `resources/list` for URIs matching `skill://*/SKILL.md` | ||
| 2. Read each SKILL.md to extract frontmatter (name + description) | ||
| 3. Build compact context summaries for the system prompt (~50-100 tokens per skill) | ||
| 4. Expose a `load_skill` tool/function for on-demand full content loading | ||
|
|
||
| ## Inspirations and Attribution | ||
|
|
||
| This reference implementation is original code inspired by patterns from: | ||
|
|
||
| - **[skills-over-mcp](https://github.com/keithagroves/skills-over-mcp)** by [Keith Groves](https://github.com/keithagroves) — Resource-based skill exposure, `skill://` URI scheme, JSON index, XML prompt injection, document templates | ||
| - **[skilljack-mcp](https://github.com/olaservo/skilljack-mcp)** by [Ola Hungerford](https://github.com/olaservo) — Resource template patterns, subscription architecture, path security | ||
| - **[skillsdotnet](https://github.com/PederHP/skillsdotnet)** by [Peder HP](https://github.com/PederHP) — `_manifest` resource with file hashes, `load_skill` tool, `SkillCatalog` client-side pattern, URI scheme conventions | ||
47 changes: 47 additions & 0 deletions
47
examples/skills-as-resources/sample-skills/code-review/SKILL.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| --- | ||
| name: code-review | ||
| description: Perform structured code reviews focusing on correctness, readability, and maintainability. Use when asked to review code changes or pull requests. | ||
| metadata: | ||
| author: skills-over-mcp-ig | ||
| version: "0.1" | ||
| --- | ||
|
|
||
| # Code Review | ||
|
|
||
| Perform structured code reviews using a consistent methodology. | ||
|
|
||
| ## When to Use | ||
|
|
||
| - User asks you to review code, a diff, or a pull request | ||
| - User asks for feedback on code quality | ||
| - You are evaluating code changes before merge | ||
|
|
||
| ## Process | ||
|
|
||
| 1. **Understand the context** — read the PR description or ask what the change is trying to accomplish | ||
| 2. **Review for correctness** — does the code do what it claims? Are there logic errors, off-by-one bugs, or unhandled edge cases? | ||
| 3. **Review for security** — check for injection vulnerabilities, improper input validation, hardcoded secrets, and OWASP top 10 issues | ||
| 4. **Review for readability** — are names clear? Is the structure easy to follow? Is there unnecessary complexity? | ||
| 5. **Review for maintainability** — is the code testable? Are dependencies reasonable? Will this be easy to change later? | ||
| 6. **Check the tests** — are there tests? Do they cover the important cases? Are they testing behavior, not implementation? | ||
|
|
||
| ## Severity Levels | ||
|
|
||
| - **Blocker**: Must fix before merge (security issues, data loss risk, broken functionality) | ||
| - **Major**: Should fix before merge (logic errors, missing edge cases, poor error handling) | ||
| - **Minor**: Nice to fix (naming, style, minor simplifications) | ||
| - **Nit**: Optional (personal preference, cosmetic) | ||
|
|
||
| ## Reference | ||
|
|
||
| For a detailed checklist, see `references/REFERENCE.md` in this skill's directory. | ||
|
|
||
| ## Output Format | ||
|
|
||
| For each finding: | ||
| - **File and line**: Where the issue is | ||
| - **Severity**: Blocker / Major / Minor / Nit | ||
| - **Issue**: What's wrong | ||
| - **Suggestion**: How to fix it | ||
|
|
||
| End with an overall summary: approve, request changes, or comment. |
36 changes: 36 additions & 0 deletions
36
examples/skills-as-resources/sample-skills/code-review/references/REFERENCE.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # Code Review Checklist | ||
|
|
||
| ## Correctness | ||
| - [ ] Logic matches the stated intent | ||
| - [ ] Edge cases handled (null, empty, boundary values) | ||
| - [ ] Error paths return meaningful messages | ||
| - [ ] Async operations properly awaited | ||
| - [ ] Resources cleaned up (connections, file handles, timers) | ||
|
|
||
| ## Security | ||
| - [ ] User input validated and sanitized | ||
| - [ ] No SQL injection, XSS, or command injection vectors | ||
| - [ ] No hardcoded secrets or credentials | ||
| - [ ] Authentication/authorization checks in place | ||
| - [ ] Sensitive data not logged or exposed in errors | ||
|
|
||
| ## Readability | ||
| - [ ] Names describe purpose (not implementation) | ||
| - [ ] Functions do one thing | ||
| - [ ] No deeply nested conditionals (max 3 levels) | ||
| - [ ] Comments explain "why", not "what" | ||
| - [ ] Consistent formatting with project style | ||
|
|
||
| ## Maintainability | ||
| - [ ] No code duplication (DRY where appropriate) | ||
| - [ ] Dependencies are justified | ||
| - [ ] Configuration externalized (not hardcoded) | ||
| - [ ] Backward compatibility considered | ||
| - [ ] Migration path documented if breaking | ||
|
|
||
| ## Testing | ||
| - [ ] Tests exist for new/changed behavior | ||
| - [ ] Tests cover happy path and error cases | ||
| - [ ] Tests are independent (no shared mutable state) | ||
| - [ ] Test names describe the scenario | ||
| - [ ] No flaky tests (timing, ordering, external dependencies) |
42 changes: 42 additions & 0 deletions
42
examples/skills-as-resources/sample-skills/git-commit-review/SKILL.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| --- | ||
| name: git-commit-review | ||
| description: Review git commits for quality, conventional commit format compliance, and potential issues. Use when asked to review commits or improve commit messages. | ||
| metadata: | ||
| author: skills-over-mcp-ig | ||
| version: "0.1" | ||
| --- | ||
|
|
||
| # Git Commit Review | ||
|
|
||
| Review git commits against conventional commit standards and common quality issues. | ||
|
|
||
| ## When to Use | ||
|
|
||
| - User asks you to review a commit or commit message | ||
| - User asks for help improving commit quality | ||
| - You are reviewing a PR and want to assess commit hygiene | ||
|
|
||
| ## Process | ||
|
|
||
| 1. **Read the commit message** — check for conventional commit format: `type(scope): description` | ||
| 2. **Verify the type** — must be one of: `feat`, `fix`, `docs`, `style`, `refactor`, `test`, `chore`, `ci`, `build`, `perf` | ||
| 3. **Check the description** — should be imperative mood, lowercase, no period at end, under 72 characters | ||
| 4. **Review the body** (if present) — should explain *why* not *what*, wrapped at 72 characters | ||
| 5. **Check for breaking changes** — must include `BREAKING CHANGE:` footer or `!` after type/scope | ||
| 6. **Assess the diff** — does the commit message accurately describe the changes? | ||
|
|
||
| ## Common Issues | ||
|
|
||
| - Vague messages ("fix stuff", "update code", "wip") | ||
| - Type mismatch (using `feat` for a bug fix) | ||
| - Scope too broad (single commit touching unrelated files) | ||
| - Missing breaking change annotation | ||
| - Commit contains unrelated changes that should be separate commits | ||
|
|
||
| ## Output Format | ||
|
|
||
| Provide a structured review: | ||
| - **Format**: Pass/Fail with specific issues | ||
| - **Message quality**: Rating and suggestions | ||
| - **Scope assessment**: Whether changes match the stated scope | ||
| - **Recommendations**: Concrete improvements |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't clients just have their own
read_resourcetool (not exposed by the MCP server, but just maps internally to a MCP client SDK resource read)? e.g. Claude Code has this if it wants to read MCP resources.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benefit of using resources directly is that you can take advantage of the semantics of resources, e.g. caching.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly because that's simply nonstandard - they could, but I was actually quite surprised to learn CC has that as I've not seen it anywhere else, and Claude also never used it until I started prompting for it more explicitly in server instructions.
It would be great if clients did this! But they don't, and don't know that it might be a good idea.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put differently: Server authors need to build with the capabilities of their target clients in mind, and if those clients include some that don't expose resource-loading directly to the model, they can't fully rely on using resources in a particular idiomatic (or non-idiomatic) way.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading through this again, I'm thinking that we want to include 3 things in our reference implementations in this repo:
In this PR, we're doing a mix of (1) and (3). I think in our documentation and future PRs we could clarify which parts of our implementation handles (1) vs (3), and add in examples of (2) so that clients have a reference point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I'd be convinced of a need for (3) if there were some arguments for why supporting resource reads more generally was challenging for some clients. That many clients don't support it today I think is not enough justification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is our intent to show how servers can do skills as resources today, or to show how we would like that to be done in an ideal world? In today's landscape, we can't assume there's going to be a builtin resource-reading tool in any given client. Given that this is a reference implementation and not a spec proposal in and of itself, I don't think whether or not we're convinced a particular approach is ideal is relevant in the former case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to demonstrate the latter, then I agree with @pja-ant's premises FWIW - it's just not clear what our intent is to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the question is what the reference implementation intends to show. If it's a quick demo to show something working end-to-end, but doesn't represent what we'd actually do as an extension/SEP then fair enough. If the goal is to prototype what we'd end up proposing then I think maybe the best way to do that would be to fork some open source LLM client and do a proper end-to-end integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding at least was that this was intended to demonstrate how servers can robustly expose skills to clients in the current ecosystem, which means considering what the clients of today do and don't support. That would then (hopefully) reveal what problems that approach has and what assumptions we can and cannot rely on, which would then (also hopefully) point us towards ideal solutions from there.