refactor: collapse mcp surface to ledgerr tools#27
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the MCP server’s advertised tool surface into a reduced set of 7 top-level ledgerr_* tools with action-based subcommands, while keeping legacy l3dg3rr_*/proxy tool names as hidden compatibility aliases.
Changes:
- Collapses the MCP tool catalog to 7
ledgerr_*capability families and routes calls viaactiondispatch in the adapter/server. - Updates e2e tests and demo scripts to use the new
ledgerr_*tool names andactionarguments. - Refreshes operator docs/runbooks to reflect the reduced catalog and new invocation patterns.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/mcp_e2e.sh | Updates the e2e script to run tests from the renamed crate/package. |
| scripts/mcp_cli_demo.sh | Updates demo calls to the new ledgerr_* tools and action-based arguments. |
| docs/mcp-capability-contract.md | Rewrites MCP surface documentation to reflect the reduced 7-tool catalog and compatibility notes. |
| docs/claude-cowork-plugin-marketplace.md | Updates marketplace/runtime docs and example tools/call usage for the new surface. |
| docs/agent-mcp-runbook.md | Simplifies the runbook and updates examples/test commands for the new tool surface. |
| crates/ledgerr-mcp/tests/* | Updates MCP e2e/contract tests to validate the reduced advertised catalog and new tool names. |
| crates/ledgerr-mcp/src/mcp_adapter.rs | Implements grouped tools (ledgerr_*) with action dispatch and updates tool catalog/schema generation. |
| crates/ledgerr-mcp/src/bin/mcp-outcome-test.rs | Updates the outcome-test flow to the new tool names/action arguments. |
| crates/ledgerr-mcp/src/bin/ledgerr-mcp-server.rs | Routes tools/call to the new grouped tools while preserving legacy aliases. |
| README.md | Updates README language to describe the reduced 7-tool MCP surface. |
| AGENTS.md | Documents the new “published surface rule” and updates canonical contract path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WORKFLOW_TOOL => json!({ | ||
| "type": "object", | ||
| "required": ["state_marker"], | ||
| "required": ["action"], | ||
| "properties": { | ||
| "state_marker": { "type": "string" } | ||
| "action": { |
There was a problem hiding this comment.
ledgerr_workflow schema requires only action, but transition and resume calls require additional mandatory fields (target_state/target_substate and state_marker respectively). Please make the schema action-sensitive (e.g., oneOf per action) so callers can discover the true required arguments via tools/list.
| - Marketplace catalog: [marketplace.json](/home/brianh/promptexecution/mbse/l3dg3rr/.claude-plugin/marketplace.json) | ||
| - Plugin manifest: [plugin.json](/home/brianh/promptexecution/mbse/l3dg3rr/plugins/l3dg3rr-plugin-create/.claude-plugin/plugin.json) | ||
| - Plugin skill: [SKILL.md](/home/brianh/promptexecution/mbse/l3dg3rr/plugins/l3dg3rr-plugin-create/skills/plugin-create-for-l3dg3rr/SKILL.md) | ||
| - MCP server entrypoint: [turbo-mcp-server.rs](/home/brianh/promptexecution/mbse/l3dg3rr/crates/turbo-mcp/src/bin/turbo-mcp-server.rs) | ||
| - MCP server entrypoint: [ledgerr-mcp-server.rs](/mnt/c/users/wendy/l3dg3rr/crates/ledgerr-mcp/src/bin/ledgerr-mcp-server.rs) | ||
| - Runtime helper commands: [Justfile](/home/brianh/promptexecution/mbse/l3dg3rr/Justfile) |
There was a problem hiding this comment.
This markdown link points to a developer-specific absolute path (/mnt/c/users/wendy/...), which will be broken for other users and in GitHub rendering. Please switch to a repository-relative link (e.g., crates/ledgerr-mcp/src/bin/ledgerr-mcp-server.rs) like the other docs in the repo.
| DOCUMENTS_TOOL => json!({ | ||
| "type": "object", | ||
| "required": ["pdf_path", "journal_path", "workbook_path"], | ||
| "required": ["action"], | ||
| "properties": { | ||
| "action": { | ||
| "type": "string", | ||
| "enum": ["list_accounts", "get_raw_context", "pipeline_status", "validate_filename", "ingest_pdf", "ingest_rows"] | ||
| }, |
There was a problem hiding this comment.
tool_input_schema for ledgerr_documents only requires action, but the handler/parsers require additional fields depending on the action (e.g., ingest_pdf needs pdf_path/journal_path/workbook_path; ingest_rows needs rows). Please update the schema to reflect action-specific required arguments (e.g., via oneOf branches per action) so tools/list is code-aligned and clients can validate inputs before calling.
| "items": { | ||
| "type": "object", | ||
| "required": ["account_id", "date", "amount", "description", "source_ref"], | ||
| "required": ["date", "amount", "description", "source_ref"], |
There was a problem hiding this comment.
Schema for rows items does not require account_id/account, but parse_rows will error unless one of those fields is present. The schema should enforce this (e.g., require account_id or use anyOf to require either account_id or account) to avoid advertising an input shape that is guaranteed to fail at runtime.
| "required": ["date", "amount", "description", "source_ref"], | |
| "required": ["date", "amount", "description", "source_ref"], | |
| "anyOf": [ | |
| { "required": ["account_id"] }, | |
| { "required": ["account"] } | |
| ], |
| TAX_TOOL => json!({ | ||
| "type": "object", | ||
| "required": ["ontology_path", "from_entity_id", "reconciliation"], | ||
| "required": ["action"], | ||
| "properties": { |
There was a problem hiding this comment.
ledgerr_tax schema only requires action, but assist/ambiguity_review parsing requires ontology_path, from_entity_id, and a nested reconciliation object. This schema/API mismatch will cause clients using tools/list schemas to produce invalid calls. Consider using oneOf with action-specific required fields (and making reconciliation required for the actions that need it).
| "type": "object", | ||
| "required": ["path", "entities"], | ||
| "required": ["action", "ontology_path"], | ||
| "properties": { | ||
| "path": { "type": "string" }, | ||
| "action": { "type": "string", "enum": ["query_path", "export_snapshot", "upsert_entities", "upsert_edges"] }, | ||
| "ontology_path": { "type": "string" }, |
There was a problem hiding this comment.
ledgerr_ontology schema globally requires ontology_path but does not require from_entity_id for the query_path action, even though parse_ontology_query_path_request requires it. This should be reflected in the schema (e.g., oneOf per action) to prevent clients from generating calls that will deterministically fail.
…cture Merges origin/main (PRs #27-#30: MCP surface collapse, codegen contract, workbook export, persist state) into codex-issue-23-document-queue. Resolution strategy: - Keep origin/main's 7-tool contract architecture throughout - Retain document inventory helpers (lib.rs) and service call path - Add DocumentInventory as a new action variant in DocumentsArgs so document_inventory is now reachable as ledgerr_documents + action - Add handle_document_inventory + parse helpers to mcp_adapter.rs - Update PUBLISHED_TOOLS actions list and mcp-capability-contract.md to include document_inventory in the ledgerr_documents row All 41 test suites pass including contract_codegen schema/doc drift checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Collapse the advertised MCP surface from 28 tools to 7 top-level
ledgerr_*capabilities while keeping the major domain areas visible:ledgerr_documentsledgerr_reviewledgerr_reconciliationledgerr_workflowledgerr_auditledgerr_taxledgerr_ontologyWhat changed
l3dg3rr_*toledgerr_*ledgerr_workflowviaaction: "plugin_info"l3dg3rr_*and proxy-style names as hidden compatibility aliases in server dispatchAGENTS.mdValidation
cargo test -p ledgerr-mcpjust testCloses #19
Related follow-on gaps: