Clean up MCP bounty selector reuse

[manav8498]

Refs #846

Summary

• Reuses the existing MCP bounty selector helper for submit_work_proof instead of maintaining a second selector path.
• Preserves the generic no-selector guidance and the selected-bounty / unknown-bounty behavior.
• Adds dispatcher-level regression coverage for id, issue+repo, generic, unknown, and invalid selector paths.

Validation

• uv run --extra dev python -m pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q -> 116 passed, 1 existing Starlette/httpx warning
• uv run --extra dev python -m pytest -q -> 794 passed, 1 existing Starlette/httpx warning
• uv run --extra dev ruff format --check app/mcp_tools.py tests/test_mcp_tools.py -> 2 files already formatted
• uv run --extra dev ruff check app/mcp_tools.py tests/test_mcp_tools.py -> All checks passed
• uv run --extra dev mypy app -> Success: no issues found in 42 source files
• git diff --check -> clean

No private data, credentials, wallet material, production mutation, proposal execution, payout/proof/ledger behavior, balance, exchange, bridge, cash-out, price behavior, or fabricated payout claims used.

Summary by CodeRabbit

• Bug Fixes

  • Unified bounty selection behavior so guidance vs "bounty not found" is returned consistently (returns generic guidance when no selector supplied; reports "bounty not found" only when a selector was provided).
  • Strengthened argument validation to reject conflicting or incomplete parameter combinations.

• Tests

  • Added tests covering selector equivalence (ID vs issue number), case-insensitive repo matching, generic guidance path, and expanded validation scenarios.

[coderabbitai]

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: 506ada26-8bbc-46e4-814e-4877c05ebbd2

📥 Commits

Reviewing files that changed from the base of the PR and between d1f918f and b74c491.

📒 Files selected for processing (2)

• app/mcp_tools.py
• tests/test_mcp_tools.py

---

📝 Walkthrough

Walkthrough

Refactors submit_work_proof bounty selector handling: adds has_bounty_selector(), extends selected_bounty(require_selector=False) to return None, unifies submit_work_proof resolution via selected_bounty("bounty_id", require_selector=False), and adds tests for selector parity and validation.

Changes

submit_work_proof bounty selector refactor

Layer / File(s)	Summary
Bounty selector helpers app/mcp_tools.py	Adds has_bounty_selector(internal_id_field) and extends selected_bounty(..., require_selector: bool = True) so callers can request None instead of a required-selector error.
submit_work_proof selector resolution app/mcp_tools.py	Refactors submit_work_proof to call selected_bounty("bounty_id", require_selector=False) for both bounty_id and issue_number paths; returns guidance when resolved, "bounty not found" when a selector was provided but unresolved, otherwise generic guidance.
Selector parity and guidance tests tests/test_mcp_tools.py	Adds test asserting bounty_id vs issue_number+repo produce identical submit_work_proof responses (case-insensitive repo), and test for generic guidance when no identifiers vs "bounty not found" for unknown issue_number.
Argument validation for selector combinations tests/test_mcp_tools.py	Extends invalid-argument parametrized tests to reject both bounty_id and issue_number together and to reject repo without issue_number.

Possibly related issues

• Proposed work: add issue-number selectors to MCP bounty lookup tools #711: Directly overlaps request to add issue-number selector parity for MCP bounty lookups in the same module.

Possibly related PRs

• ramimbo/mergework#732: Refactors bounty selector handling for submit_work_proof with centralized issue_number/bounty_id resolution paths.
• ramimbo/mergework#398: Modifies submit_work_proof argument-selection flow including how bounty_id/issue_number and repo selectors are handled.
• ramimbo/mergework#287: Changes submit_work_proof argument-selection logic for bounty_id vs issue_number selector handling and "bounty not found" cases.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately names the changed surface and main objective: refactoring MCP bounty selector logic for code reuse.
Description check	✅ Passed	Description includes summary, validation results, and issue reference. All critical sections are present and complete.
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, price, cash-out, or payout claims. Code excludes price claims from submissions; MRWK described as native to ledger without off-ramps.
Bounty Pr Focus	✅ Passed	Changes match stated scope: app/mcp_tools.py refactored submit_work_proof logic, tests/test_mcp_tools.py added selector and validation tests. Issue #846 focused, no unrelated additions verified.

_{✏️ 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.}

[xiefuzheng713-alt]

Approved for current head d1f918f0a4d7978d1fc9b55bf01c8dca3ee029b2.

I reviewed app/mcp_tools.py, tests/test_mcp_tools.py, and the existing submit-work-proof coverage in tests/test_api_mcp.py. The refactor reuses the shared selected_bounty() selector for submit_work_proof while keeping the generic no-selector path separate via require_selector=False. The existing selector validation is still applied for mixed bounty_id/issue_number, repo-without-issue, repo scoping, ambiguous issue numbers, non-positive ids, and unknown bounty selectors.

Validation I ran locally:

• uv run --python 3.12 --extra dev python -m pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q -> 116 passed, 1 existing Starlette/httpx warning.
• uv run --python 3.12 --extra dev ruff check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> passed.
• uv run --python 3.12 --extra dev ruff format --check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> 3 files already formatted.
• uv run --python 3.12 --extra dev mypy app/mcp_tools.py -> success.
• git diff --check -> clean.
• git merge-tree --write-tree origin/main HEAD -> clean tree 9b6cf0918a1954c3ae3910443c2deaf2110e36df.

I also rechecked GitHub state before approving: this is not my PR, there were no prior human reviews on this head, mergeStateStatus=CLEAN, and the hosted Quality/readiness/docs/image check is successful. Scope is limited to MCP bounty selector reuse and regression coverage; I did not see ledger, wallet, treasury, payout, admin-token, exchange/bridge/cash-out, private-data, or MRWK price behavior changes.

[alan747271363-art]

Reviewed current head d1f918f0a4d7978d1fc9b55bf01c8dca3ee029b2 again after the PR changed from the earlier clean approval state to current mergeStateStatus=DIRTY / mergeable=CONFLICTING.

Requesting changes because the branch-local MCP bounty selector reuse still validates, but the PR is now blocked by a current-main content conflict in app/mcp_tools.py. The hosted checks are green on the PR head, but they predate the current-main conflict and do not make the branch mergeable now.

Validation on this exact head:

• .\.venv\Scripts\python.exe -m pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q -> 116 passed, 1 existing Starlette/httpx warning.
• .\.venv\Scripts\python.exe -m ruff check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> passed.
• .\.venv\Scripts\python.exe -m ruff format --check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> 3 files already formatted.
• .\.venv\Scripts\python.exe -m mypy app/mcp_tools.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 $(git rev-parse origin/main) HEAD -> failed with a content conflict in app/mcp_tools.py; tests/test_mcp_tools.py auto-merged.

Suggested next step: rebase or merge current main, resolve app/mcp_tools.py, then rerun the MCP API/tool tests plus ruff/mypy/docs smoke checks before merge.

Scope reviewed: MCP bounty selector reuse in app/mcp_tools.py and focused tool/API coverage only. No wallet material, ledger mutation, treasury/proposal execution, payout execution, admin-token behavior, private data, credentials, bridge/exchange/cash-out behavior, or MRWK price behavior was used.

[manav8498]

Resolved the current-main conflict reported in the changes-requested review by merging origin/main into this branch and resolving app/mcp_tools.py so both behaviors are preserved: upstream internal-id aliases for bounty selectors and this PR’s optional no-selector path for submit_work_proof generic guidance.

Validation on head b74c491:

• uv run --python 3.12 --extra dev python -m pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q -> 135 passed, 1 existing Starlette/httpx warning.
• uv run --python 3.12 --extra dev ruff check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> passed.
• uv run --python 3.12 --extra dev ruff format --check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> 3 files already formatted.
• uv run --python 3.12 --extra dev mypy app/mcp_tools.py -> success.
• uv run --python 3.12 --extra dev python scripts/docs_smoke.py -> docs smoke ok.
• git diff --check -> clean.
• git diff --check origin/main...HEAD -> clean.
• git merge-tree --write-tree origin/main HEAD -> clean tree d7936c4541ddd7762f67ad3bab62c2deaf2110e36df.

Hosted CI is green on the new head; CodeRabbit was still pending when checked.

[mauricemohr88-debug]

Reviewed current head b74c491285db04a889b0966e0e17e2917c6d300a.

I inspected the submit_work_proof selector refactor in app/mcp_tools.py and the focused regression coverage in tests/test_mcp_tools.py. The change removes the duplicated bounty-selection path and reuses selected_bounty("bounty_id", require_selector=False) so bounty_id, issue_number, optional repo, selector conflicts, repo-without-issue, and unknown bounty behavior stay centralized. The separate has_bounty_selector() check preserves the intended split between generic no-selector guidance and explicit-but-unknown selectors returning bounty not found.

Validation I ran locally on this head:

• uv run --python 3.12 --extra dev python -m pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q -> 135 passed, 1 existing Starlette/httpx warning.
• uv run --python 3.12 --extra dev ruff check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> passed.
• uv run --python 3.12 --extra dev ruff format --check app/mcp_tools.py tests/test_mcp_tools.py tests/test_api_mcp.py -> 3 files already formatted.
• uv run --python 3.12 --extra dev mypy app/mcp_tools.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 d7936c4541ddd7762f67ad3bab62c2deaf2110e36df.

GitHub state checked before review: this is not my PR, current head has no human review yet, hosted Quality/readiness/docs/image check is successful, and CodeRabbit is still pending, which appears to be why mergeStateStatus remains UNSTABLE. I do not see a code blocker from local review, but I would still wait for the pending CodeRabbit status before merge.

Scope reviewed: MCP bounty selector reuse and focused API/tool regression coverage only. No wallet material, ledger mutation, treasury mutation, payout execution, admin-token behavior, private data, credentials, exchange, bridge, cash-out, or MRWK price behavior was used.

[heickerv1001-dev]

Bounty #838 finding on PR #882: submit_work_proof no longer rejects mixed selectors. In app/mcp_tools.py, the old if has_bounty_id and has_issue_number: raise ValueError("use bounty_id or issue_number, not both") check was removed, but the new selected_bounty(..., require_selector=False) path now returns the bounty_id match immediately when both bounty_id and issue_number are present. That means issue_number is silently ignored instead of surfacing the validation error that tests/test_mcp_tools.py still expects. This is a real behavior regression, not just a docs mismatch.

[manav8498]

Rechecked current head b74c491285db04a889b0966e0e17e2917c6d300a after the mixed-selector concern.

The submit_work_proof path still routes through selected_bounty("bounty_id", require_selector=False), and selected_bounty() validates before returning a bounty:

• if both an internal id field and issue_number are present, it raises use bounty_id or issue_number, not both;
• the session.get(Bounty, ...) return path only runs after that guard.

Validation on current head:

• uv run --python 3.12 --extra dev python -m pytest tests/test_mcp_tools.py::test_call_mcp_tool_preserves_argument_validation_errors -q -> 5 passed.
• uv run --python 3.12 --extra dev python -m pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q -> 135 passed, 1 existing Starlette/httpx warning.
• git diff --check -> clean.

So I do not see the claimed silent-ignore regression on the current PR head.

[Errordog2]

Reviewed the MCP bounty selector reuse cleanup.

Validation performed locally:

• .venv\Scripts\python.exe -m pytest tests/test_mcp_tools.py -> 13 passed
• .venv\Scripts\python.exe -m pytest tests/test_mcp_work_proof.py -> 8 passed
• .venv\Scripts\python.exe -m ruff check app/mcp_tools.py tests/test_mcp_tools.py -> passed
• .venv\Scripts\python.exe -m ruff format --check app/mcp_tools.py tests/test_mcp_tools.py -> passed
• .venv\Scripts\python.exe -m mypy app/mcp_tools.py app/mcp_work_proof.py -> passed
• git diff --check -> passed
• git merge-tree $(git merge-base origin/main HEAD) origin/main HEAD -> clean merge

The refactor keeps the generic submit_work_proof path intact, returns bounty not found when a provided selector misses, and centralizes the id/issue/repo validation through the shared selector helper. Looks good to me.

[ramimbo]

Closing this unmerged for the current queue pass.

The referenced bounty #846 is filled and closed, so this PR is not payable from that round. The successor cleanup round #936 is still a pending create_bounty proposal and is not claimable until execution creates the public bounty row and GitHub finalization.