Reject control-padded MCP work-proof format

[ramimbo]

Summary

• Reject raw control characters in the MCP submit_work_proof format argument before trimming/lowercasing.
• Add JSON-RPC and direct dispatcher regression coverage for C1 control-padded format values.

Evidence

• Confusion, missing step, stale example, or bug this addresses: format previously normalized a C1-control-padded JSON format value to json, unlike neighboring MCP string helpers that reject raw control characters before normalization.
• Bounty capacity and active attempts/open PRs checked: no open PR found for Proposed work: reject control-padded MCP work-proof format #819; this is a maintainer PR and does not request MRWK.
• Intended files or surfaces: app/mcp_tools.py, tests/test_api_mcp.py, tests/test_mcp_tools.py.
• Expected PR size: small.
• Out of scope: no ledger, wallet, treasury, bounty lifecycle, payout, proof, docs, or valid text/json behavior changes.

Test Evidence

• python scripts/check_agents.py
• pytest tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_invalid_bounty_selectors -q (red before fix, green after fix)
• pytest tests/test_api_mcp.py tests/test_mcp_tools.py -q
• pytest
• ruff format --check .
• ruff check .
• mypy app
• python scripts/docs_smoke.py
• git diff --check

MRWK

Related bounty or issue: Refs #819.

Maintainer PR; no MRWK bounty requested.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds control-character validation to the format argument of call_mcp_tool's MCP tool submission flow. The implementation validates the raw format input before normalizing and type-checking, rejecting values containing control characters with a ValueError. Test coverage includes both unit and integration tests that verify rejection of C1 control-character-prefixed format values.

Changes

Control-Character Validation for MCP Format Argument

Layer / File(s)

Summary

Control-character validation for format argument app/mcp_tools.py, tests/test_mcp_tools.py, tests/test_api_mcp.py

The output_format_arg() function adds a control-character check to the format input before normalizing and enforcing "text" or "json" values. Unit test test_call_mcp_tool_rejects_c1_work_proof_format_before_normalizing verifies the rejection with the exact error message. Integration test test_mcp_submit_work_proof_rejects_invalid_bounty_selectors includes a parametrized case with a control-character-prefixed format value.

Possibly related issues

• Proposed work: reject control-padded MCP work-proof format #819: Duplicates the same control-character validation fix for the format argument in output_format_arg() with matching tests.

Possibly related PRs

• ramimbo/mergework#398: Extracts and refactors the call_mcp_tool dispatcher that this PR modifies.
• ramimbo/mergework#659: Implements similar C1 control-character rejection in call_mcp_tool for other MCP argument fields before normalization.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Bounty Pr Focus	⚠️ Warning	PR references "Refs #819" but adds entire repository (162 files, 40K lines) instead of stated +14 line feature change to three files.	Verify if initial repository commit is intentional; if this is a feature PR, scope includes unrelated infrastructure and setup files far exceeding stated changes.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly names the main change: rejecting control characters in MCP work-proof format arguments, which is the core of the changeset.
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	No prohibited MRWK claims found. PR contains technical validation changes only. README correctly describes MRWK as native project coin with future snapshots/bridges discussion-based.
Description check	✅ Passed	The PR description comprehensively addresses the template requirements with clear evidence, test results, scope boundaries, and proper linking to issue #819.

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

[jakerated-r]

Reviewed current head 5b5a88c936bb6175e28bece75b177c3c55662b49 for Bounty #654.

Evidence checked:

• Inspected the three-file diff: app/mcp_tools.py, tests/test_api_mcp.py, tests/test_mcp_tools.py.
• Confirmed submit_work_proof now rejects raw C1/control-padded format values before strip().lower(), matching the neighboring MCP string-validation behavior.
• Confirmed clean format: "text" still returns the normal submission guidance path.
• Local focused tests: 110 passed, 1 warning with pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q.
• Local quality checks: ruff format --check ., ruff check ., mypy app, and scripts/docs_smoke.py all passed.
• git diff --check origin/main...HEAD passed.
• git merge-tree $(git merge-base HEAD origin/main) origin/main HEAD produced a clean merge result.
• Hosted GitHub checks are passing; CodeRabbit reports pass.

No blocker found. The change is small, scoped, and covered at both dispatcher and JSON-RPC API levels.

[ramimbo]

Scope clarification for the Bounty PR Focus warning: GitHub compare currently shows this PR as one commit with three changed files and 14 additions: app/mcp_tools.py, tests/test_api_mcp.py, and tests/test_mcp_tools.py.

Refs #819 is included as source/proposed-work context for the maintainer PR. This PR is not requesting MRWK and does not include a bounty claim.

[ramimbo]

Maintainer queue status: held. Current head is clean and has one useful current-head human approval; the queue gate requires two useful current-head human reviews before merge. No MRWK requested.

[MolhamHamwi]

Reviewed current head 5b5a88c936bb6175e28bece75b177c3c55662b49 as a non-author.

Evidence checked:

• Inspected app/mcp_tools.py, tests/test_api_mcp.py, and tests/test_mcp_tools.py in the PR diff.
• Confirmed output_format_arg() now rejects C0/C1 control characters before strip().lower(), matching the existing clean-selector pattern used for repo/status-style MCP arguments.
• Confirmed both dispatcher-level coverage (call_mcp_tool(..., {"format": "\u0085json"})) and JSON-RPC invalid-params coverage were added for submit_work_proof.
• Verified GitHub reports the PR as mergeStateStatus=CLEAN with hosted checks green.
• Ran locally on the PR checkout: python -m pytest tests/test_api_mcp.py tests/test_mcp_tools.py -q → 110 passed, 1 warning.

No blocker found; this is a focused validation hardening change.

[aglichandrap]

Review evidence (head SHA: current):

• ✅ Scope: Rejects C1 control characters in format arg before normalization in output_format_arg().
• ✅ Reuse: Uses existing contains_control_character helper — consistent with other validation.
• ✅ Tests: test_api_mcp.py adds "\x85json" to parametrized rejection cases. test_mcp_tools.py adds dedicated test_call_mcp_tool_rejects_c1_work_proof_format_before_normalizing.
• ✅ Files: 3 files, +14/-0 — minimal.
• ✅ Mergeable: state=clean.

No blockers. Approve.