Skip to content

feat(mcp): add lint_url full detail mode - #158

Merged
aram-devdocs merged 2 commits into
mainfrom
gh-issue-40-lint-url-detail-full-09a39d0
Apr 29, 2026
Merged

feat(mcp): add lint_url full detail mode#158
aram-devdocs merged 2 commits into
mainfrom
gh-issue-40-lint-url-detail-full-09a39d0

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Fixes #40

Summary

  • Adds optional detail argument to lint_url, defaulting to compact output.
  • Adds detail: "full" structured content using deterministic full JSON violation payloads.
  • Enforces the 50 KB response cap for full-detail payloads and documents the new option.

Validation

  • cargo test -p plumb-mcp — passed
  • cargo test -p plumb-cli --test mcp_stdio — passed
  • cargo check --no-default-features — passed

Note

The original remote branch gh-issue-40-lint-url-detail-full already existed at divergent commit 61ea2e5; this PR uses gh-issue-40-lint-url-detail-full-09a39d0 to avoid force-pushing over it.

@github-actions

github-actions Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

I have everything I need. Here is the review.


PR Review: feat(mcp): add lint_url full detail mode + fix(mcp): address lint_url review feedback

Scope: 2 commits, 3 files changed (net +192 lines): crates/plumb-mcp/src/lib.rs, crates/plumb-cli/tests/mcp_stdio.rs, docs/src/mcp.md.


1. Determinism — PASS

No new nondeterminism introduced. The new code paths call plumb_format::json, which: sorts violations by sort_key(), builds a stable serde_json::Map, and derives run_id from a SHA-256 hash of the canonical serialization — no wall-clock, no map iteration leaking.

The pre-existing SystemTime (line 38) and HashMap (line 34) are scoped to the config-file mtime cache and do not flow into any tool response. Those are not new in this PR.

clippy.toml bans SystemTime::now and Instant::now (not SystemTime field storage), so the existing usage is conformant.


2. Workspace layering — PASS

plumb-mcp already depends on plumb-format. The new use plumb_format::{json as full_json, mcp_compact} at line 43 is within that pre-approved dependency edge. No diagonal or reverse dependencies introduced.


3. Error handling — PASS

build_lint_url_result and build_full_lint_payload are private fns returning Result<_, ErrorData>. No unwrap/expect in library code. expect calls in mcp_stdio.rs tests are in #[cfg(test)] context and are explicitly allowed by clippy.toml (allow-expect-in-tests = true).


4. Test coverage — PASS

Four new integration tests in mcp_stdio.rs:

  • mcp_lint_url_explicit_compact_matches_default — parity between default and explicit compact.
  • mcp_lint_url_full_returns_json_envelope — happy path for full mode; checks plumb_version, run_id prefix, summary.total, doc_url.
  • mcp_lint_url_invalid_detail_returns_jsonrpc_error-32602 error code and serde message for unknown variant.
  • Schema assertion in mcp_initialize_and_tools_list$defs.LintUrlDetail.oneOf exposes exactly ["compact", "full"].

Three new unit tests in lib.rs's #[cfg(test)] block covering build_full_lint_payload (happy path + cap rejection) and build_lint_url_result (cap rejection in Full mode).


5. Documentation — PASS

LintUrlDetail and its variants are doc-commented. LintUrlArgs::detail is doc-commented. CHANGELOG.md updated. docs/src/mcp.md updated with the new detail argument and the 50 KB cap. No public items are undocumented.


Punch list (warnings, no blockers)

crates/plumb-mcp/src/lib.rs:391–403 — WARNING: unnecessary serialize→parse round-trip.
build_full_lint_payload calls full_json(violations) to get a pretty-printed String, checks byte length, then calls serde_json::from_str(&payload) to convert back to a Value. Functionally correct, but this double-parse is forced by plumb_format::json's string-only API. Not a blocker, but if plumb_format::json ever grows a -> Value variant, this is the first refactor target.

crates/plumb-mcp/src/lib.rs:380 — WARNING: mcp_compact always called in Full mode.
build_lint_url_result always calls mcp_compact(violations), gets (text, compact_structured), and then discards compact_structured when detail == Full. The text is legitimately needed for the shared text block in both modes, so the call itself is correct. The wasted allocation of compact_structured is minor; flagging so it's a known tradeoff rather than an oversight.

crates/plumb-mcp/src/lib.rs:409 — NIT: &'static str overly restrictive.
enforce_response_cap(…, error_message: &'static str) could be &str with no functional impact; it's a private function with one call site. Stack as a nit; doesn't matter in practice.


Verdict: APPROVE

@aram-devdocs
aram-devdocs merged commit 46fe12a into main Apr 29, 2026
14 of 15 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,format): lint_url detail=full mode

1 participant