[codex] Structure MCP work-proof selector errors#374
Open
rounaksalim95 wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe PR adds structured JSON error responses to the MCP ChangesStructured JSON Error Handling for Bounty Selection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
ayskobtw-lil
left a comment
ayskobtw-lil
left a comment
There was a problem hiding this comment.
No blockers found in this focused structured-error guidance change.
Evidence checked:
- Inspected
app/main.pyaroundsubmit_work_proofand the newwork_proof_selection_error_json()helper. - Verified
format: "json"now returnsstructuredContentplus matching JSON text for unknownbounty_id/issue_numberselectors instead of only plain text. - Verified ambiguous issue-number JSON responses include machine-readable
error.code,issue_number,can_submit: false, and amatching_bountiessummary with actual bounty ids, repos, issue URLs, status, and remaining awards so agents can retry with a concretebounty_id. - Checked that existing text-mode behavior is preserved for unknown bounty selectors, and invalid argument combinations still use the existing JSON-RPC invalid-argument path.
- Inspected the small
docs/agent-guide.mdaddition and the targeted MCP regressions intests/test_api_mcp.py.
Validation on PR head 2d54ef5:
python -m pytest tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_unknown_bounty_selector tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_ambiguous_issue_number tests/test_api_mcp.py::test_mcp_submit_work_proof_reports_unknown_bounty tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_ambiguous_issue_number -q-> 4 passedpython -m ruff check app/main.py tests/test_api_mcp.py docs/agent-guide.md-> passedpython -m mypy app/main.py-> passedgit diff --check origin/main...HEAD-> clean- Hosted Quality/readiness/docs/image check is green; CodeRabbit was still pending when I reviewed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/main.py`:
- Around line 1824-1829: The query that builds bounties uses a hard limit of 2
(session.scalars(select(Bounty).where(Bounty.issue_number ==
issue_number).order_by(Bounty.id.desc()).limit(2)).all()), which can hide valid
ambiguous candidates; remove the .limit(2) so the full set of matching rows for
the given issue_number is returned (i.e., call
session.scalars(select(Bounty).where(Bounty.issue_number ==
issue_number).order_by(Bounty.id.desc())).all()) and then build
matching_bounties from that complete result set to ensure all ambiguous matches
are included in JSON responses.
In `@tests/test_api_mcp.py`:
- Around line 1545-1591: Add a companion test case in
test_mcp_submit_work_proof_structures_unknown_bounty_selector that posts the
same JSON-RPC call but with "arguments": {"bounty_id": <missing_id>, "format":
"json"} (e.g., 999) instead of issue_number, using the same
TestClient/create_app setup; extract response =
client.post(...).json()["result"], check response["structuredContent"] and
response["content"][0]["text"] parsed as JSON, and assert the structured dict
mirrors the existing expected payload for a missing bounty (status
"bounty_not_found", availability "unknown_without_bounty", error
{"code":"bounty_not_found",...}, matching_bounties empty, etc.) to lock the
bounty_id-not-found JSON branch. Ensure you reference the existing test function
name when adding the companion assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb048370-f83c-4834-8b6c-b8845d64a853
📒 Files selected for processing (3)
app/main.pydocs/agent-guide.mdtests/test_api_mcp.py
Comment on lines
1824
to
1829
| bounties = session.scalars( | ||
| select(Bounty) | ||
| .where(Bounty.issue_number == positive_int_arg("issue_number")) | ||
| .where(Bounty.issue_number == issue_number) | ||
| .order_by(Bounty.id.desc()) | ||
| .limit(2) | ||
| ).all() |
There was a problem hiding this comment.
Return all ambiguous matches for JSON responses.
The query is capped at 2 rows before building matching_bounties, so ambiguous JSON responses can hide valid candidate bounty_ids when more than two matches exist.
💡 Suggested adjustment
- bounties = session.scalars(
- select(Bounty)
- .where(Bounty.issue_number == issue_number)
- .order_by(Bounty.id.desc())
- .limit(2)
- ).all()
+ base_query = (
+ select(Bounty)
+ .where(Bounty.issue_number == issue_number)
+ .order_by(Bounty.id.desc())
+ )
+ bounties = session.scalars(
+ base_query if output_format == "json" else base_query.limit(2)
+ ).all()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/main.py` around lines 1824 - 1829, The query that builds bounties uses a
hard limit of 2 (session.scalars(select(Bounty).where(Bounty.issue_number ==
issue_number).order_by(Bounty.id.desc()).limit(2)).all()), which can hide valid
ambiguous candidates; remove the .limit(2) so the full set of matching rows for
the given issue_number is returned (i.e., call
session.scalars(select(Bounty).where(Bounty.issue_number ==
issue_number).order_by(Bounty.id.desc())).all()) and then build
matching_bounties from that complete result set to ensure all ambiguous matches
are included in JSON responses.
Comment on lines
+1545
to
+1591
| def test_mcp_submit_work_proof_structures_unknown_bounty_selector( | ||
| sqlite_url: str, | ||
| ) -> None: | ||
| create_schema(sqlite_url) | ||
| with session_scope(sqlite_url) as session: | ||
| ensure_genesis(session) | ||
|
|
||
| client = TestClient(create_app(database_url=sqlite_url, webhook_secret="secret")) | ||
|
|
||
| response = client.post( | ||
| "/mcp", | ||
| json={ | ||
| "jsonrpc": "2.0", | ||
| "id": 1, | ||
| "method": "tools/call", | ||
| "params": { | ||
| "name": "submit_work_proof", | ||
| "arguments": {"issue_number": 999, "format": "json"}, | ||
| }, | ||
| }, | ||
| ).json()["result"] | ||
|
|
||
| structured = response["structuredContent"] | ||
| assert json.loads(response["content"][0]["text"]) == structured | ||
| assert structured == { | ||
| "bounty_id": None, | ||
| "issue_number": 999, | ||
| "status": "bounty_not_found", | ||
| "availability": "unknown_without_bounty", | ||
| "can_submit": False, | ||
| "availability_warnings": ["bounty not found"], | ||
| "awards_remaining": None, | ||
| "reward_mrwk": None, | ||
| "repository": None, | ||
| "issue_url": None, | ||
| "acceptance": None, | ||
| "error": {"code": "bounty_not_found", "message": "bounty not found"}, | ||
| "matching_bounties": [], | ||
| "submission_format": ( | ||
| "Retry with an unambiguous bounty_id before preparing work-proof evidence." | ||
| ), | ||
| "safety_rules": [ | ||
| "Do not include private keys, seed material, secrets, deployment " | ||
| "credentials, private vulnerability details, or price claims." | ||
| ], | ||
| } | ||
|
|
There was a problem hiding this comment.
Add JSON coverage for unknown bounty_id selection.
This covers the issue_number not-found JSON branch, but the PR also changed the bounty_id not-found JSON branch and it isn’t asserted yet. Add a companion case for {"bounty_id": <missing>, "format": "json"} to lock that contract.
As per coding guidelines, tests/**/*.py: Add or update tests for changed behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_api_mcp.py` around lines 1545 - 1591, Add a companion test case in
test_mcp_submit_work_proof_structures_unknown_bounty_selector that posts the
same JSON-RPC call but with "arguments": {"bounty_id": <missing_id>, "format":
"json"} (e.g., 999) instead of issue_number, using the same
TestClient/create_app setup; extract response =
client.post(...).json()["result"], check response["structuredContent"] and
response["content"][0]["text"] parsed as JSON, and assert the structured dict
mirrors the existing expected payload for a missing bounty (status
"bounty_not_found", availability "unknown_without_bounty", error
{"code":"bounty_not_found",...}, matching_bounties empty, etc.) to lock the
bounty_id-not-found JSON branch. Ensure you reference the existing test function
name when adding the companion assertions.
tolga-tom-nook
suggested changes
May 26, 2026
Contributor
tolga-tom-nook
left a comment
tolga-tom-nook
left a comment
There was a problem hiding this comment.
Requesting one focused change before this is safe to merge.
I inspected current head 2d54ef5 for the #315 structured MCP work-proof selector path, specifically app/main.py, tests/test_api_mcp.py, and docs/agent-guide.md.
Evidence checked:
uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_unknown_bounty_selector tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_ambiguous_issue_number -q-> 2 passed.uv run --extra dev python -m ruff check app/main.py tests/test_api_mcp.py-> passed.
Blocker:
- The JSON ambiguous-selector branch still builds
matching_bountiesfrom a query capped with.limit(2)inapp/main.py. The new structured response tells agents to retry with an unambiguousbounty_id, but when more than two bounties share anissue_number, it can omit valid candidate IDs frommatching_bounties. That makes the new machine-readable retry guidance incomplete for exactly the ambiguity case it is meant to solve. - The added ambiguity test only creates two matches, so it cannot catch the truncation. Please either remove/raise the cap for the structured JSON path or make the response explicitly indicate truncation/next lookup, and add a three-match regression asserting agents can see all intended candidates (or know the list is truncated).
The unknown-selector JSON behavior itself looks consistent; this is limited to the ambiguous issue-number JSON response contract.
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
structuredContentforsubmit_work_proof(format: "json")when a bounty selector is unknown or ambiguous.bounty_idinstead of scraping prose.Why this is distinct
Bounty #315 already has accepted work for the core JSON guidance path and availability fields. This PR targets the remaining selector failure path: JSON callers can now handle missing or ambiguous bounty selectors with structured fields instead of plain text or generic tool errors.
Validation
uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_unknown_bounty_selector tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_ambiguous_issue_number tests/test_api_mcp.py::test_mcp_submit_work_proof_reports_unknown_bounty tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_ambiguous_issue_number -q-> 4 passed.uv run --extra dev python -m pytest tests/test_api_mcp.py -q-> 78 passed.uv run --extra dev python -m pytest -q-> 337 passed.uv run --extra dev ruff check .-> passed.uv run --extra dev ruff format --check .-> 48 files already formatted.uv run --extra dev python -m mypy app-> success.uv run --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check-> clean.Bounty #315
No private keys, seed material, secrets, deployment credentials, private vulnerability details, payout credentials, or MRWK price claims are included.
Summary by CodeRabbit
New Features
Documentation
Tests