Skip to content

Advertise MCP wallet output schema#1013

Open
elianguitarra wants to merge 2 commits into
ramimbo:mainfrom
elianguitarra:codex/get-wallet-mcp-output-schema-946
Open

Advertise MCP wallet output schema#1013
elianguitarra wants to merge 2 commits into
ramimbo:mainfrom
elianguitarra:codex/get-wallet-mcp-output-schema-946

Conversation

@elianguitarra
Copy link
Copy Markdown

@elianguitarra elianguitarra commented Jun 6, 2026

Bounty #946

Summary

  • Adds an MCP outputSchema for the read-only get_wallet tool, matching the wallet object already returned in result.structuredContent.
  • Captures the stable wallet fields agents need: canonical address, public key, optional label/GitHub login, balance, nonce, next nonce, and creation timestamp.
  • Keeps runtime text and JSON behavior unchanged; this only improves tools/list metadata and tests.
  • Distinct from Advertise MCP schemas for wallet write tools #989, which advertises input schemas for wallet write tools. This PR is focused on the get_wallet structured output contract.

Validation

  • .venv\Scripts\python.exe -m pytest tests\test_api_mcp.py::test_mcp_tools_list_and_call tests\test_api_mcp.py::test_mcp_can_register_and_fetch_wallet -> 2 passed, 1 existing Starlette/httpx warning
  • .venv\Scripts\python.exe -m pytest tests\test_api_mcp.py tests\test_mcp_tools.py -> 131 passed, 1 existing Starlette/httpx warning
  • .venv\Scripts\python.exe scripts\docs_smoke.py -> docs smoke ok
  • .venv\Scripts\ruff.exe check app\mcp.py tests\test_api_mcp.py -> All checks passed
  • .venv\Scripts\ruff.exe format --check app\mcp.py tests\test_api_mcp.py -> 2 files already formatted
  • git diff --check -> clean, Windows CRLF warning only for docs

Touched MCP surface: tools/list metadata for get_wallet plus focused tests/docs.

Summary by CodeRabbit

  • Documentation

    • Clarified the agent guide to describe the tool's structured wallet output schema.
  • Tests

    • Strengthened wallet API tests to assert exact required fields, disable extra properties, and validate field formats and constraints for wallet responses.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 6, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f7d04e9e-0167-4161-be5c-d80b1369b3f0

📥 Commits

Reviewing files that changed from the base of the PR and between 34ae779 and 364ca87.

📒 Files selected for processing (2)
  • app/mcp.py
  • tests/test_api_mcp.py

📝 Walkthrough

Walkthrough

Adds MCP_WALLET_OUTPUT_SCHEMA and wires it as the get_wallet tool's outputSchema; tests assert the schema shape and exact wallet output keys; docs note that tools/list advertises the structured wallet output schema.

Changes

Wallet Output Schema

Layer / File(s) Summary
Wallet schema definition and MCP tool integration
app/mcp.py
MCP_WALLET_OUTPUT_SCHEMA is added (address, public_key_hex, optional label/github_login, balances, nonce/next_nonce, created_at) with regex/pattern/maxLength constraints, required fields, and additionalProperties: False. The get_wallet entry in MCP_TOOLS is updated to include outputSchema: MCP_WALLET_OUTPUT_SCHEMA.
Schema assertions and wallet output tests
tests/test_api_mcp.py
Tests now assert get_wallet's outputSchema disallows additional properties, lists the exact required output keys, and verifies property constraints (address/public_key_hex patterns and lengths, label maxlength, balance pattern, created_at format presence/absence). test_mcp_can_register_and_fetch_wallet asserts fetched wallet keys exactly match the required set.
Agent guide documentation
docs/agent-guide.md
Adds a parenthetical note clarifying that tools/list advertises the structured wallet output schema for get_wallet.

Possibly related PRs

  • ramimbo/mergework#424: Touches MCP get_wallet schema/response expectations and adds response-shape examples and tests.
  • ramimbo/mergework#926: Modifies MCP_TOOLS["get_wallet"]; earlier work added inputSchema while this PR adds outputSchema.
  • ramimbo/mergework#345: Updates MCP tool schema definitions and corresponding tests for tools surfaced via tools/list.
🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Bounty Pr Focus ❓ Inconclusive PR references issue #946 via branch name but lacks explicit "Bounty #N" in commits. Unclear if check applies without bounty label. Clarify: Does issue #946 qualify as a Bounty issue? PR scope is focused (app/mcp.py, docs, tests) with matching changes if bounty status is confirmed.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Advertise MCP wallet output schema' clearly and concretely names the primary changed surface—the addition and advertisement of a wallet output schema for the MCP get_wallet tool.
Description check ✅ Passed The description covers the main objective, validation results, and MCP surface changes. However, the template sections 'Confusion, missing step, stale example, or bug this addresses', 'Bounty capacity and active attempts/open PRs checked', and 'Expected PR size' are not explicitly addressed in the provided description.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Mergework Public Artifact Hygiene ✅ Passed PR adds only technical wallet schema with factual descriptions; no investment, price, cash-out, or payout claims. Existing docs correctly describe MRWK per guidelines.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 48f7f23c-4e2d-4ecb-90cc-8870c7f5e78a

📥 Commits

Reviewing files that changed from the base of the PR and between 8bdd282 and 34ae779.

📒 Files selected for processing (3)
  • app/mcp.py
  • docs/agent-guide.md
  • tests/test_api_mcp.py

Comment thread app/mcp.py Outdated
@elianguitarra
Copy link
Copy Markdown
Author

Follow-up for CodeRabbit timestamp schema comment: updated get_wallet outputSchema created_at from JSON Schema date-time to the timezone-naive ISO pattern emitted by the serializer.\n\nValidation after fix:\n- .venv\Scripts\python.exe -m pytest tests\test_api_mcp.py::test_mcp_tools_list_and_call tests\test_api_mcp.py::test_mcp_can_register_and_fetch_wallet -> 2 passed\n- .venv\Scripts\ruff.exe check app\mcp.py tests\test_api_mcp.py -> All checks passed\n- .venv\Scripts\ruff.exe format --check app\mcp.py tests\test_api_mcp.py -> 2 files already formatted

Copy link
Copy Markdown

@modelsbridgeaicom-ship-it modelsbridgeaicom-ship-it left a comment

Choose a reason for hiding this comment

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

Requesting changes on the current head 364ca87f25d0ffbf6d26dae8594acb7568b14cbf.

The focused MCP wallet output-schema change validates cleanly on its own branch, but the PR is not merge-ready against current origin/main because app/mcp.py has a content conflict. GitHub also reports the PR as mergeable=CONFLICTING / mergeStateStatus=DIRTY, so the branch needs a rebase or conflict resolution before maintainers can merge it.

Files inspected:

  • app/mcp.py
  • tests/test_api_mcp.py
  • docs/agent-guide.md

Validation run locally:

  • .\.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py::test_mcp_tools_list_and_call tests\test_api_mcp.py::test_mcp_can_register_and_fetch_wallet -q -> 2 passed, 1 existing Starlette/httpx warning.
  • .\.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py tests\test_mcp_tools.py -q -> 131 passed, 1 existing Starlette/httpx warning.
  • .\.venv\Scripts\ruff.exe check app\mcp.py tests\test_api_mcp.py docs\agent-guide.md -> All checks passed.
  • .\.venv\Scripts\ruff.exe format --check app\mcp.py tests\test_api_mcp.py -> 2 files already formatted.
  • .\.venv\Scripts\python.exe -m mypy app\mcp.py app\mcp_tools.py app\wallet_api.py -> success.
  • .\.venv\Scripts\python.exe scripts\docs_smoke.py -> docs smoke ok.
  • git diff --check origin/main...HEAD -> clean.
  • git merge-tree --write-tree origin/main HEAD -> content conflict in app/mcp.py.

Verdict: changes requested until the app/mcp.py conflict is rebased/resolved. No runtime MCP behavior, ledger mutation, wallet custody/signing, treasury/proposal execution, payout execution, admin-token behavior, credentials, private data, exchange, bridge, cash-out, MRWK price behavior, or fabricated payout claim was used.

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.

2 participants