Skip to content

Document get_balance MCP account selectors#891

Open
RuliSlim wants to merge 2 commits into
ramimbo:mainfrom
RuliSlim:fix/mcp-conformance-usability
Open

Document get_balance MCP account selectors#891
RuliSlim wants to merge 2 commits into
ramimbo:mainfrom
RuliSlim:fix/mcp-conformance-usability

Conversation

@RuliSlim
Copy link
Copy Markdown

@RuliSlim RuliSlim commented Jun 4, 2026

Summary

  • Document the supported get_balance MCP account selector formats in the tool description.
  • Add regression coverage so tools/list keeps exposing selector guidance for github:<login>, mrwk1..., treasury:mrwk, and reserve:bounty:<id>.

Test Plan

  • ./.venv/bin/python -m pytest tests/test_api_mcp.py
  • ./.venv/bin/python -m pytest
  • ./.venv/bin/python -m ruff check app/mcp.py tests/test_api_mcp.py
  • ./.venv/bin/python -m mypy app/mcp.py

Bounty

Caveats / Scope

  • Description/test-only MCP usability change.
  • No ledger, wallet, treasury, payout, admin, bridge, exchange, cash-out, or MRWK price behavior changed.
  • This does not duplicate Add structured MCP balance result #885's structured get_balance response work; it only documents the existing selector formats exposed through tools/list.

This fix was solved by Hermes, Rulislim's autonomous coding agent, on behalf of Rulislim.

Summary by CodeRabbit

  • Documentation

    • Expanded the balance lookup tool description to include detailed examples of supported account selector formats (GitHub logins, address-style IDs, treasury accounts, and bounty reserve selectors).
  • Tests

    • Added a test to confirm the tool listing exposes the updated description and includes the account selector examples.

The get_balance MCP tool description was just "Get an account balance",
giving clients no guidance on the account selector format. Document the
four formats normalized_account accepts: github:<login>, mrwk1 wallet
addresses, treasury:mrwk, and reserve:bounty:<id>.

Description-only change; no ledger/wallet/treasury behavior is altered.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 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: 130da980-3d36-413b-8c1f-6a446b8a6f64

📥 Commits

Reviewing files that changed from the base of the PR and between 5f79b75 and 6f5af6c.

📒 Files selected for processing (1)
  • tests/test_api_mcp.py

📝 Walkthrough

Walkthrough

The PR expands the MCP tool description for get_balance to document supported account selector formats (github:<login>, mrwk1..., treasury:mrwk, reserve:bounty:<id>), replacing a generic one-liner. A new test validates the expanded description is exposed via the MCP tools/list endpoint.

Changes

get_balance tool documentation

Layer / File(s) Summary
Tool description and validation
app/mcp.py, tests/test_api_mcp.py
The get_balance tool description in MCP_TOOLS is expanded to document account selector formats. A new test validates the description includes all expected selector examples when queried via /mcp tools/list.
  • Possibly related PRs:
    • ramimbo/mergework#329: Both touch app/mcp.py’s MCP_TOOLS definition; #329 restructures the MCP tool registry/handler while this PR updates the get_balance entry and adds a validation test.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concretely names the changed surface (get_balance MCP tool description) and the specific action (documenting account selectors), matching the requirement.
Description check ✅ Passed The description covers summary, test plan, and bounty references, though it omits the template's structured Evidence section with specific subsections.
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 contains no investment claims, price claims, cash-out/off-ramp claims, fabricated payout claims, or private security details. Changes only document functional account selector formats.
Bounty Pr Focus ✅ Passed PR diff matches stated files (app/mcp.py, tests/test_api_mcp.py), documents all four required selector formats, includes regression test evidence, and avoids unrelated scope creep.

✏️ 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: 8e0e1276-a9bf-44c2-abb0-27c65fd06b53

📥 Commits

Reviewing files that changed from the base of the PR and between d4d0e48 and 5f79b75.

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

Comment thread tests/test_api_mcp.py
Comment on lines +298 to +301
assert "github:" in description
assert "mrwk1" in description
assert "treasury:mrwk" in description
assert "reserve:bounty:" in description
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Strengthen selector-format assertions to match the documented contract.

Line 298 and Line 301 currently assert only prefixes ("github:", "reserve:bounty:"), so the test still passes if <login> or <id> guidance regresses. Assert the full documented tokens.

As per coding guidelines, "tests/**/*.py: ... focus on whether tests prove the changed behavior and include ... regression cases where relevant."

Proposed test tightening
-    assert "github:" in description
+    assert "github:<login>" in description
     assert "mrwk1" in description
     assert "treasury:mrwk" in description
-    assert "reserve:bounty:" in description
+    assert "reserve:bounty:<id>" in description

Copy link
Copy Markdown

@xiefuzheng713-alt xiefuzheng713-alt left a comment

Choose a reason for hiding this comment

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

Reviewed current head 5f79b753213da8bb61809225d7527d317051edc2 as a non-author.

The change itself is narrow and the new selector-description assertion passes, but this head still needs a formatting fix before it is merge-ready. The hosted Quality check is failing, and I reproduced the local formatter failure on the changed test file:

  • uv run --python 3.12 --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py -> fails because tests/test_api_mcp.py would be reformatted.
  • uv run --python 3.12 --extra dev ruff format --diff tests/test_api_mcp.py shows the new balance_tool = next(...) expression should be on one line.

Other validation I ran:

  • uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_get_balance_description_documents_account_selectors -q -> 1 passed, 1 existing warning.
  • uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py -q -> 106 passed, 1 existing warning.
  • uv run --python 3.12 --extra dev ruff check app/mcp.py tests/test_api_mcp.py -> passed.
  • uv run --python 3.12 --extra dev mypy app/mcp.py -> success.
  • uv run --python 3.12 --extra dev python scripts/docs_smoke.py -> docs smoke ok.
  • git diff --check origin/main...HEAD -> clean.
  • git merge-tree --write-tree origin/main HEAD -> clean tree b70268e7d972b0275c35dbc2844d150e3dc904b9.

Requesting changes only for the formatting failure; no MCP behavior blocker found.

Copy link
Copy Markdown

@xiefuzheng713-alt xiefuzheng713-alt left a comment

Choose a reason for hiding this comment

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

Reviewed current head 6f5af6c191202f1638acd0cb0b9d2f9c49770af0 as a non-author after the formatting follow-up.

Evidence checked:

  • inspected app/mcp.py and tests/test_api_mcp.py;
  • confirmed get_balance now documents the supported account selector formats: github:<login>, mrwk1..., treasury:mrwk, and reserve:bounty:<id>;
  • confirmed the new tools/list regression covers those selector hints without changing balance lookup behavior;
  • confirmed the previous formatter failure from head 5f79b753213da8bb61809225d7527d317051edc2 is fixed on this head;
  • verified GitHub reports mergeStateStatus=CLEAN, hosted Quality/readiness/docs/image success, and CodeRabbit success.

Validation run on this exact head:

  • uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py -q -> 106 passed, 1 existing warning.
  • uv run --python 3.12 --extra dev ruff check app/mcp.py tests/test_api_mcp.py -> passed.
  • uv run --python 3.12 --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py -> 2 files already formatted.
  • uv run --python 3.12 --extra dev mypy app/mcp.py -> success.
  • uv run --python 3.12 --extra dev python scripts/docs_smoke.py -> docs smoke ok.
  • git diff --check origin/main...HEAD -> clean.
  • git merge-tree --write-tree origin/main HEAD -> clean tree 6007e446d909290da0404eeb4fa7a78050039781.

No blocker found. Scope is MCP tool-list description/test coverage only; no ledger, wallet, treasury, payout, admin-token, private data, secrets, exchange, bridge, cash-out, or MRWK price behavior changed.

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