feat(mcp): get_config tool

[aram-devdocs]

Target branch

All PRs target main. Plumb has no dev branch.

• This PR targets main

Spec

Fixes #37

Summary

• New get_config MCP tool returns the resolved plumb.toml for a caller-supplied working directory as JSON-serialized [Config].
• Memoized per (path, mtime) via an Arc<Mutex<HashMap<PathBuf, ConfigCacheEntry>>> shared across Clone instances of PlumbServer. Cache is internal-only — never iterated, so the determinism rule banning HashMap in observable output is preserved.
• When no plumb.toml exists at the requested path, the tool returns Config::default() with source = "default" so a fresh checkout still gets a usable answer.

Crates touched

• plumb-core
• plumb-format
• plumb-cdp
• plumb-config
• plumb-mcp
• plumb-cli (test only)
• xtask
• docs/
• .agents/ or .claude/
• .github/

System impact

• New public API item (PlumbServer::get_config, GetConfigArgs) — rustdoc + # Errors section included.
• New MCP tool — tools/list entry + happy-path protocol test added.
• New rule
• CDP / browser surface change
• Config schema change
• Dependency added — plumb-mcp now depends on plumb-config (which depends only on plumb-core; layering preserved).
• Determinism invariant touched — see Architectural compliance below for the analysis.

Architectural compliance

• Layer discipline: plumb-mcp → plumb-config → plumb-core is allowed; no cycles, no unsafe introduced, no println! outside plumb-cli.
• Error shape: ErrorData::invalid_params for empty / non-absolute working_dir; ErrorData::internal_error for stat / parse / serialize / poisoned-mutex paths. No anyhow added.
• No new unwrap/expect/panic! in library crates — every Mutex::lock and serde_json::to_value is mapped to a typed error.
• No new SystemTime::now / Instant::now. metadata.modified() is a filesystem read used only for cache-equality, not a clock read.
• No new HashMap in observable output. The internal cache is never iterated; output ordering of the JSON config comes from Config's IndexMap fields.
• No new #[allow(...)] attributes added.

Test plan

• cargo fmt --all -- --check clean.
• cargo clippy -p plumb-mcp -p plumb-cli --all-targets --all-features -- -D warnings clean.
• cargo nextest run -p plumb-mcp -p plumb-cli — 58/58 pass, including the new mcp_get_config_returns_default_when_no_file protocol test and the extended mcp_initialize_and_tools_list assertion.
• just validate — will run as part of CI / final test gate; not yet executed locally (skeleton milestone).
• cargo xtask pre-release — N/A (no rule or schema change).
• just determinism-check — will run in CI.
• cargo deny check — will run in CI (one new internal dep edge: plumb-mcp → plumb-config).

Documentation

• Rustdoc on GetConfigArgs and PlumbServer::get_config, including a # Errors section.
• # Errors section on every new public fallible fn — covered.
• docs/src/mcp.md tool table extended.
• CHANGELOG — release-please will pick up the feat(mcp): Conventional Commits.
• Humanizer skill — ran mentally over the one-line docs row; no marketing prose introduced.

Breaking change?

• No
• Yes

Checklist

• Conventional Commits title.
• Branch name: codex/37-feat-mcp-get-config.
• All review gates passed — running after this PR opens (skeleton milestone).
• /gh-review --local-diff main...HEAD — will run after this initial PR is up so reviewers see the same scope as CI.

Reviewer notes

• The working_dir argument is required and must be absolute. This avoids reading process state (current dir / env) inside a tool call and keeps get_config deterministic.
• The cache is keyed by PathBuf, so two distinct working directories cohabit. There is no eviction yet; an MCP session lives for a CLI run, so the cache stays small in practice.
• source = "default" is surfaced explicitly so an agent can distinguish "no config exists" from "config exists and happens to match defaults" without a separate probe.
• This is the initial PR / skeleton milestone. The remaining review gates (spec / quality / architecture / test / security-auditor) will run on the open PR. Follow-up commits will address any blocker findings before merge.

[aram-devdocs]

No description provided.

[aram-devdocs]

PR #132 has merge conflicts with main (#131 feat(mcp): lint_url accepts real http(s) URLs via ChromiumDriver landed after this branch). Conflicts span crates/plumb-mcp/src/lib.rs, crates/plumb-mcp/Cargo.toml, crates/plumb-mcp/AGENTS.md, docs/src/mcp.md, crates/plumb-cli/tests/mcp_stdio.rs, and Cargo.lock — both PRs touch the same regions of lib.rs (the lint_url tool body, the imports block, the tools list and the server instructions string). Automated resolution from this session is blocked: the sandbox does not permit git config, git merge, or arbitrary file writes outside narrow patterns, so no identity could be set for the merge commit and no merged blobs could be staged. The conflicts are mechanical — combine PR #131's is_fake_url branching in lint_url with PR #132's get_config tool, retain both plumb-cdp and plumb-config as deps, and stack the get_config row/test on top of main's updated tools table and assertion. Please resolve manually (git merge origin/main on the branch, then push); CI's just validate should be the gate.

[github-actions]

I have everything I need for a complete review.

---

Code review summary

PR #132 — feat(mcp): get_config tool
Crates touched: plumb-mcp, plumb-cli (test only), docs/.
Adds get_config tool that resolves <working_dir>/plumb.toml and returns the Config as JSON, with an internal (path, mtime) memo cache.

---

Blockers

None per the workflow-contract list. All hard-check items clear:

Check	Result
unsafe outside plumb-cdp	✓ none
unwrap/expect/panic! in library	✓ none — all Mutex::lock and serde_json paths mapped to ErrorData
println!/eprintln! outside plumb-cli	✓ none
SystemTime::now/Instant::now in plumb-core	✓ not in plumb-core; mtime comes from metadata().modified(), which is not the banned SystemTime::now
todo!/unimplemented!/dbg!	✓ none
HashMap in observable-output path	✓ cache is keyed-read-only at the output surface; iteration order never reaches JSON output
New MCP tool without protocol test in mcp_stdio.rs	✓ mcp_get_config_returns_default_when_no_file added
Config schema regenerated	✓ N/A, no schema change

---

Warnings

1. Missing invalid-args tests — crates/plumb-mcp/tests/mcp_protocol.rs
mcp-tool-patterns.md is explicit: "Every tool gets a case in crates/plumb-mcp/tests/mcp_protocol.rs: Happy path. Invalid args → clear JSON-RPC error." The file exists and already holds this pattern for explain_rule (explain_rule_unknown_rule_id_returns_invalid_params). get_config has zero coverage in mcp_protocol.rs — no happy path, no invalid-args cases. The two paths that need in-process coverage:

• working_dir = "" → ErrorCode::INVALID_PARAMS (-32602)
• working_dir = "relative/path" → ErrorCode::INVALID_PARAMS

The subprocess test in mcp_stdio.rs covers the "no file" scenario but does not replace in-process invalid-args coverage. This is the same gap that would cause the 03-code-quality-reviewer gate to flag it.

2. HashMap for the internal cache — plumb-mcp/src/lib.rs:34,110
Not a blocker (never iterated for output), but diverges from project convention. The workspace Cargo.toml comments: # Deterministic data structures. indexmap = .... An IndexMap here would be consistent even for a non-output-path map, and cargo-deny is already wired to enforce this spirit. Consider indexmap::IndexMap<PathBuf, ConfigCacheEntry> to stay uniform.

3. mtime-equality cache invalidation — plumb-mcp/src/lib.rs:325
SystemTime equality has coarse resolution on FAT/exFAT/DrvFs (2 s). A write + re-read within the same coarse window returns stale cached JSON. Not a correctness blocker for typical developer-machine use (ext4/APFS both have 1 ns resolution), but worth a comment explaining the limitation.

4. Unbounded cache growth — plumb-mcp/src/lib.rs:110
No eviction policy. Per the PR description this is intentional ("MCP session lives for a CLI run"). A short doc comment on config_cache documenting the no-eviction contract and the expected lifetime would help a future reader not introduce eviction incorrectly.

---

Architecture compliance

• Layer: plumb-mcp (L5) → plumb-config (L4) → plumb-core (L1). Allowed per dependency-hierarchy.md. No cycles, no diagonal jumps. ✓
• plumb-config added to Cargo.lock with one new edge. ✓
• No anyhow added to plumb-mcp; all error paths use thiserror-backed ErrorData. ✓
• No #![forbid(unsafe_code)] override needed — plumb-mcp inherits the workspace forbid. ✓

---

Anti-pattern scan

• No commented-out code, no open-ended TODO, no _-prefixed unused params. ✓
• No #[allow(...)] attributes added. ✓
• path.display().to_string() in structured output (plumb-mcp/src/lib.rs:470) echoes the user's own input back — deterministic for the same input. ✓
• Security: PathBuf::from(working_dir).join("plumb.toml") — join with a relative component appends cleanly; the absoluteness pre-check prevents the join-replaces-base footgun. No path traversal risk. ✓

---

Quality assessment

The implementation is clean: error handling is thorough, rustdoc is complete (including # Errors), the ConfigSource enum avoids stringly-typed source values, and the cache_lookup/cache_store split keeps the lock scope minimal. The PR description correctly identifies the determinism edge case (HashMap internal, IndexMap in Config output) and explains it. The missing invalid-args tests in mcp_protocol.rs are the only material gap against the stated rules.

---

Scope check

All changed files are within the declared crates (plumb-mcp, plumb-cli test, docs). No unrelated cleanup or refactoring bundled in. Conventional Commits title matches the content. ✓

---

Punch list (file:line)

1. crates/plumb-mcp/tests/mcp_protocol.rs — add #[tokio::test] async fn get_config_empty_working_dir_returns_invalid_params and get_config_relative_working_dir_returns_invalid_params. Required by mcp-tool-patterns.md.
2. crates/plumb-mcp/tests/mcp_protocol.rs — add happy-path in-process test (happy path with real tempdir + no file). Rule names this file explicitly for both paths.
3. crates/plumb-mcp/src/lib.rs:34,110 — consider indexmap::IndexMap for config_cache to stay consistent with project-wide convention.
4. crates/plumb-mcp/src/lib.rs:110 — add a doc comment on config_cache field documenting the no-eviction lifetime contract.

---

Verdict: REQUEST_CHANGES

[aram-devdocs]

Resolved the merge conflicts with main (PRs #131 + #133 had landed since this branch was opened):

• crates/plumb-mcp/src/lib.rs — kept both new use lines: plumb_cdp::{BrowserDriver, ChromiumDriver, ChromiumOptions, Target, is_fake_url} from main and plumb_config::ConfigError from this branch; pulled ViewportKey into the existing plumb_core import.
• Cargo.lock — union of plumb-cdp + plumb-config under plumb-mcp.

Local cargo check could not run in the sandbox (rustup hit No space left on device while syncing the 1.95.0 toolchain) so verification was deferred to CI. All required checks now pass: Preflight, MSRV (1.95), Test (ubuntu/macos/windows), Determinism, Coverage, Docs, Size guard, cargo-deny, GitGuardian, Conventional Commits, Claude review. PR is MERGEABLE / CLEAN.

[aram-devdocs]

Resolved the conflicts from PR #129 (list_rules) merging into main while this PR was open. The merge keeps both tools side-by-side: the PlumbServer now exposes echo, lint_url, explain_rule, list_rules, and get_config, with merged tool registration, dispatch, args structs, and updated mcp_stdio protocol assertions and docs/src/mcp.md table. All CI checks pass on the merge commit (27d487a); proceeding with squash auto-merge.