Skip to content

feat(mcp): add MCP resources for sample pipelines#384

Merged
streamer45 merged 3 commits into
mainfrom
devin/1777138112-mcp-resources
Apr 25, 2026
Merged

feat(mcp): add MCP resources for sample pipelines#384
streamer45 merged 3 commits into
mainfrom
devin/1777138112-mcp-resources

Conversation

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor

@staging-devin-ai-integration staging-devin-ai-integration Bot commented Apr 25, 2026

Summary

Enable the MCP resources capability and expose sample/template pipelines as browsable MCP resources. LLM agents can now discover and read curated pipeline templates via resources/list, resources/read, and resources/templates/list.

Changes

  • apps/skit/src/mcp/mod.rs: Enable resources capability in ServerCapabilities builder. Override list_resources (enumerates all sample pipelines with streamkit://samples/{mode}/{id} URIs), read_resource (returns pipeline YAML by URI with defensive URI parsing and path traversal validation), and list_resource_templates (returns a single URI template describing available modes).
  • apps/skit/src/samples.rs: Make list_samples, get_sample, and SamplesError pub for cross-module access from the MCP handler. Add # Errors doc sections to satisfy clippy.
  • apps/skit/tests/mcp_integration_test.rs: Add 5 integration tests with a dedicated start_mcp_server_with_samples helper that creates temp sample files:
    • mcp_list_resources_returns_samples
    • mcp_read_resource_returns_yaml
    • mcp_read_resource_not_found
    • mcp_list_resource_templates
    • mcp_read_resource_rejects_path_traversal

Security

  • read_resource checks perms.read_samples (matching the HTTP handler's access control)
  • Resource IDs are validated against path traversal characters (.., /, \)

Review & Testing Checklist for Human

  • Verify read_resource permission check uses read_samples (not list_samples) — mirrors get_sample_handler in samples.rs:294
  • Verify path traversal validation rejects .., /, and \ in resource IDs
  • Test resources/list returns correct URIs and metadata for actual sample pipelines
  • Test resources/read with a valid URI returns the expected YAML content
  • Recommended: start just skit, connect an MCP client, and exercise resources/list → pick a URI → resources/read

Notes

  • The plugin_integration_test failures are pre-existing on main (native plugin tests) and unrelated to this change.
  • list_resource_templates intentionally skips auth — it returns only static schema metadata (URI pattern, description, MIME type), not sample data.

Link to Devin session: https://staging.itsdev.in/sessions/d635c6317c6640348fbaecb4b0cf1184
Requested by: @streamer45

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

staging-devin-ai-integration Bot commented Apr 25, 2026

✅ Reviewed on efa1f0d

View review

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review (Staging)
Debug

Playground

Comment thread apps/skit/src/mcp/mod.rs Outdated
Comment thread apps/skit/src/mcp/mod.rs
Comment thread apps/skit/src/mcp/mod.rs
Comment on lines +815 to +829
async fn list_resource_templates(
&self,
_request: Option<PaginatedRequestParams>,
_ctx: RequestContext<RoleServer>,
) -> Result<ListResourceTemplatesResult, McpError> {
let raw = RawResourceTemplate::new("streamkit://samples/{mode}/{id}", "Sample Pipeline")
.with_description(
"A curated sample pipeline template. Modes: 'oneshot' (batch processing), \
'dynamic' (real-time streaming), 'demo', or 'user'.",
)
.with_mime_type("application/x-yaml");

let template = Annotated::new(raw, None);

Ok(ListResourceTemplatesResult::with_all_items(vec![template]))
Copy link
Copy Markdown
Contributor Author

@staging-devin-ai-integration staging-devin-ai-integration Bot Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: list_resource_templates skips auth, unlike sibling resource handlers

list_resources (apps/skit/src/mcp/mod.rs:732) checks perms.list_samples and read_resource (apps/skit/src/mcp/mod.rs:768) checks perms.read_samples, but list_resource_templates (apps/skit/src/mcp/mod.rs:822-837) has no auth or permission check at all. This is consistent with get_info() which also returns static metadata without auth, and the template only reveals the URI pattern (streamkit://samples/{mode}/{id}) — not actual sample names or content. So this is defensible, but worth noting the asymmetry in case the team prefers uniform auth enforcement across all resource-related handlers.

Open in Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional — the template is static schema metadata (URI pattern, description, MIME type) that doesn't expose any sample data. It's useful for MCP clients to discover the resource URI pattern even before authenticating. Adding an auth check here would prevent agents from knowing how to request resources.

staging-devin-ai-integration[bot]

This comment was marked as resolved.

staging-devin-ai-integration Bot pushed a commit that referenced this pull request Apr 25, 2026
The list_samples tool mode filter only accepted 'oneshot' and 'dynamic',
but the sample system also supports 'demo' and 'user' modes. This
updates the filter to accept all four modes, matching the modes used
in samples.rs:list_samples and the resource handler in PR #384.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
streamkit-devin and others added 3 commits April 25, 2026 18:44
Enable the MCP resources capability and expose sample/template
pipelines as browsable MCP resources.

- Enable `resources` capability in ServerCapabilities builder
- Override `list_resources` to enumerate all sample pipelines
  (oneshot, dynamic, demo, user) with streamkit:// URIs
- Override `read_resource` to return pipeline YAML by URI
- Override `list_resource_templates` with a single URI template
- Make `list_samples`, `get_sample`, and `SamplesError` pub for
  cross-module use from the MCP handler
- Add 4 integration tests: list_resources, read_resource,
  read_resource_not_found, list_resource_templates

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
…ad_resource

- Check perms.read_samples (not list_samples) in read_resource,
  matching the HTTP handler's access control
- Validate resource ID against path traversal characters (.., /, \)
- Add mcp_read_resource_rejects_path_traversal integration test

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Defense-in-depth: validate the filename portion of the sample ID
directly in get_sample so future callers cannot skip validation.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
@staging-devin-ai-integration staging-devin-ai-integration Bot force-pushed the devin/1777138112-mcp-resources branch from 978a937 to efa1f0d Compare April 25, 2026 18:44
@streamer45 streamer45 enabled auto-merge (squash) April 25, 2026 18:48
@streamer45 streamer45 merged commit cbaf928 into main Apr 25, 2026
17 checks passed
@streamer45 streamer45 deleted the devin/1777138112-mcp-resources branch April 25, 2026 18:59
@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

MCP Comprehensive Test Report

Date: 2026-04-25 | Branch: test/mcp-combined (all 4 PRs merged onto main) | Build: 93a34e2

Summary

Phase Pass Fail Total
Integration Tests 39 0 39
Live HTTP Tests 42 0 42
Security Tests 5 0 5
STDIO Transport 3 0 3
Total 89 0 89

Overall: ALL 89 TESTS PASS

PR #384 Specific Results (MCP Resources)

Test Status Notes
mcp_list_resources_returns_samples PASS
mcp_read_resource_returns_yaml PASS
mcp_read_resource_not_found PASS
mcp_list_resource_templates PASS
mcp_read_resource_rejects_path_traversal PASS
Live: resources/list PASS 2 resources, MIME=application/x-yaml
Live: resources/read (valid URI) PASS YAML content returned
Live: resources/read (not found) PASS Error returned
Live: path traversal ../../etc/passwd PASS Rejected
Live: encoded traversal ..%2F PASS Rejected
Live: nested slash sub/nested PASS Rejected
Live: resources/templates/list PASS 1 template with URI pattern

Security: All path traversal vectors properly blocked. No regressions. Full report: https://staging.itsdev.in/sessions/c0e3bffbfefe401daa6aeeb33d9c2e89

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.

2 participants