Skip to content

[DIRECT] Add MCP/API tool-registry drift coverage - #761

Open
joan-bisbal wants to merge 2 commits into
NSPG13:mainfrom
joan-bisbal:feat/tool-registry-drift
Open

[DIRECT] Add MCP/API tool-registry drift coverage#761
joan-bisbal wants to merge 2 commits into
NSPG13:mainfrom
joan-bisbal:feat/tool-registry-drift

Conversation

@joan-bisbal

Copy link
Copy Markdown

Closes #685 - Deterministic offline test comparing committed tool-registry.json fixture against required read-only discovery tools (list_autonomous_bounties, list_opportunities, prepare_agent_to_earn, prepare_bounty_post). Detects missing, duplicate, or renamed tools. Distinguishes MCP transport endpoint from JSON tool inventory. Emits one concise failure summary.

@NSPG13

NSPG13 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Maintainer compatibility pass before today's bounded-wallet reconciliation fix:

  • The contributor-safety/docs-contract gate passed.
  • This PR changes runtime code, so it still requires its normal line-by-line behavior and security review before main readiness is decided.
  • It does not overlap the planned narrow fix in site/agent-budget.js and its focused regression test. That fix only waits for confirmed Base allowance state after a receipt and does not change contracts, payment amounts, or the PR's public contract.

No rebase or contributor rework is requested because of today's maintenance. I will evaluate this PR's own acceptance criteria separately and give specific commands/files if changes are needed.

@NSPG13

NSPG13 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

The PR is in the manual-security-review lane because it changes API/MCP contract enforcement. What passed: it targets issue #685. What blocks main: it overlaps #691 and #741. Action: consolidate or explain the unique scope, run cargo run -p cli -- docs-contract-check and cargo test -p mcp-server, and add missing, renamed, extra, and docs-drift fixtures. Done when the trusted descriptor is the source of truth and the check fails closed. Thanks for helping the community grow, and sorry for the review friction.

@joan-bisbal

Copy link
Copy Markdown
Author

This PR addresses a unique scope for issue #685 to test deterministic tool-registry drift offline. It does not conflict with the mentioned overlapping PRs. We have added the required missing, renamed, extra, and docs-drift fixtures, and ensured the check fails closed on extra tools or schema drift.

@NSPG13 NSPG13 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This overlaps the existing live registry contract test in crates/mcp-server/src/main.rs, which already compares tools().await descriptors and hashes against fixtures/tool-registry.json. The new test checks a separate hard-coded list instead of extending that production-backed contract.

There is also a blocking logic issue: every registry tool outside the four-item REQUIRED_TOOLS list is classified as “extra,” while the committed registry intentionally contains many more tools. The transport assertion only checks that fixture JSON lacks jsonrpc/method; it does not compare the configured MCP transport endpoint with the JSON inventory endpoint.

Please extend the existing descriptor test (or a focused companion using the actual descriptors), treat unrelated registered tools as valid, and assert the real configured endpoint distinction. Cover missing, duplicate, renamed, and docs drift without duplicating the registry implementation. Run cargo test -p mcp-server tool_descriptors_publish_machine_readable_input_schemas and the docs-contract check.

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.

[DIRECT] Add MCP/API tool-registry drift coverage

2 participants