Skip to content

feat(mcp): expose resolved config resource - #159

Merged
aram-devdocs merged 3 commits into
mainfrom
gh-issue-42-config-resource
Apr 29, 2026
Merged

feat(mcp): expose resolved config resource#159
aram-devdocs merged 3 commits into
mainfrom
gh-issue-42-config-resource

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

  • expose plumb://config as an MCP resource for the server working directory
  • advertise MCP resource capability and cover the resource in protocol tests
  • document the MCP resource surface and note it in the changelog

Fixes #42

Tests Run

  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo check --no-default-features
  • cargo test -p plumb-mcp
  • cargo test -p plumb-cli --test mcp_stdio

@github-actions

github-actions Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

I have everything I need. Here is the review.


PR #159feat(mcp): expose resolved config resource

File buckets touched

  • plumb-clicommands/mcp.rs (CLI boundary only)
  • plumb-mcpsrc/lib.rs (MCP server implementation)
  • plumb-mcptests/mcp_protocol.rs (in-process protocol tests)
  • plumb-clitests/mcp_stdio.rs (end-to-end stdio tests)
  • docsdocs/src/mcp.md, CHANGELOG.md

1. Determinism

No violations introduced.

  • SystemTime (lib.rs:38, ConfigCacheEntry.mtime) is pre-existing and used only as a cache key — it never appears in any response payload.
  • HashMap (lib.rs:34, config_cache) is pre-existing and internal; iteration order never escapes to observable output.
  • serialize_config (lib.rs:647–659) produces { config, source, path } — all fields are deterministic functions of the config + path. No timestamps leak.
  • read_resource_impl serialises resolved.value with serde_json::to_string — deterministic given the same Config.

2. Workspace layering

No violations.

  • std::env::current_dir() is called in plumb-cli/src/commands/mcp.rs:8 — correct layer (CLI is the only crate permitted to read env). The resolved PathBuf is passed into plumb-mcp, which never calls env itself.
  • plumb-mcp acquires no new internal crate dependencies.
  • #![forbid(unsafe_code)] is not touched.

3. Error handling

No violations.

  • resolve_config_for_dir (lib.rs) uses map_err + ? throughout; all branches return Result<ResolvedConfig, ErrorData>.
  • read_resource_impl maps every fallible call: serde_json::to_stringErrorData::internal_error; unknown URI → ErrorData::resource_not_found.
  • unwrap_or_default() at lib.rs:525 is Option::unwrap_or_default, not unwrap_used — pre-existing and clean.
  • expect / unwrap in the mod tests block (lib.rs:700–768) are pre-existing; the PR does not add any.

4. Test coverage

Adequate.

  • mcp_stdio.rs: mcp_read_config_resource_returns_resolved_config_json — exercises the happy path over a real subprocess with a temp plumb.toml; asserts source, path (canonicalised comparison, not snapshot), and config values. ✅
  • mcp_stdio.rs: mcp_read_unknown_resource_returns_jsonrpc_error — verifies JSON-RPC error -32002 and the error message string. ✅
  • mcp_protocol.rs: server_info_declares_resource_capability — in-process check that resources capability is advertised. ✅
  • mcp_initialize_and_tools_list extended to assert resources/list response and plumb://config entry shape. ✅
  • Deleted default_is_equivalent_to_new correctly pairs with Default impl removal — no orphaned test. ✅

5. Documentation

Two minor gaps — not blockers because missing_docs only fires on absent doc comments, not missing param descriptions.

  • run_stdio (lib.rs:677): the doc comment (lines 665–676) does not mention the new cwd parameter. The # Errors section is present and accurate.
  • PlumbServer::new (lib.rs:156–162): doc says "Construct a new server." — cwd's role (sets the server working directory used by plumb://config) is undocumented.

docs/src/mcp.md resource table is present and accurate. CHANGELOG entry is correct.

Security note (MCP surface)

read_resource_impl only accepts the literal URI "plumb://config" and reads from self.cwd, which is set once at server start from env::current_dir() in the CLI. There is no user-controlled path in the resource endpoint. No path-traversal or injection surface is introduced.


Punch list

# Severity Location Issue
1 Warning lib.rs:665–676 / lib.rs:677 run_stdio doc doesn't describe the cwd parameter.
2 Warning lib.rs:155–162 PlumbServer::new doc doesn't describe cwd's purpose.

Neither is a CI blocker. Both should be cleaned up in a follow-up or same-PR amendment before the docs are regenerated for docs.rs.


Verdict: APPROVE

@aram-devdocs
aram-devdocs merged commit 222a82a into main Apr 29, 2026
14 checks passed
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.

feat(mcp): MCP resource exposing resolved plumb.toml

1 participant