Refs #406: Harden submission gate API bounty parsing#478
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughHardens the API bounty loader: adds API_BOUNTY_SAFETY_CAP, sends explicit limit, enforces cap and payload-type checks, tightens per-row validation and awards_remaining normalization (raising RuntimeError on malformed rows), adds UnicodeDecodeError handling, and extends tests for UTF-8 and malformed-payload edge cases. ChangesAPI Bounty Loader Hardening
Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed PR #478 at current head 3efeac3745abde5b7ec7858ff568e3c18ecfbdff.
I would hold this because it closes the malformed UTF-8 fallback for the bounty-list API, but the same failure mode still exists in the active-attempts API path used by the live submission gate.
Evidence:
_load_api_bounties()now catchesUnicodeDecodeErrorand converts bad API bytes into aRuntimeError, which_load_live_context()can turn into a warning._load_api_attempts()still decodes the attempts response withresponse.read().decode("utf-8")but only catchesHTTPError,OSError,URLError, andjson.JSONDecodeError._load_live_context()calls_load_api_attempts()inside anexcept RuntimeErrorblock only. If/api/v1/bounties/{id}/attemptsreturns malformed UTF-8, theUnicodeDecodeErrorescapes and the gate crashes instead of marking active-attempt lookup unverified.
Minimal local repro on this head:
from scripts import submission_quality_gate as gate
class BadUtf8Response:
def __enter__(self): return self
def __exit__(self, *args): return None
def read(self): return b"\xff\xfe"
gate.urlopen = lambda url, timeout: BadUtf8Response()
gate._load_api_attempts("https://api.example.test", 66)Observed result:
UnicodeDecodeError
'utf-8' codec can't decode byte 0xff in position 0: invalid start byte
I also reproduced the full live-context shape by returning a valid bounty-list row for #406 and malformed UTF-8 for /api/v1/bounties/66/attempts; _load_live_context() raised the same UnicodeDecodeError. Expected behavior should match the rest of this PR: wrap attempts decode failures in RuntimeError so the caller can emit the existing active_attempts unavailable warning.
Validation run locally:
uv run --extra dev python -m pytest tests/test_submission_quality_gate.py -q->27 passeduv run --extra dev python -m pytest tests/test_pr_queue_health.py -q->9 passeduv run --extra dev python -m pytest -q->418 passeduv run --extra dev ruff check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py-> passeduv run --extra dev ruff format --check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py->2 files already formatteduv run --extra dev python -m mypy scripts/submission_quality_gate.py-> successuv run --extra dev python scripts/docs_smoke.py-> docs smoke okgit diff --check origin/main...HEAD-> cleangitleaks detect --no-banner --redact --source . --log-opts origin/main..HEAD --exit-code 1-> no leaks found- Live gate smoke for a
Refs #406draft returnedwarnonly because the current PR itself is a similar open PR; bounty payability, maintainer activity, and active-attempt checks passed.
No wallet material, private keys, private data, signatures, price claims, exchange claims, liquidity claims, or off-ramp promises were used.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ddcd564c-b17f-4611-830d-a38f902f2e83
📒 Files selected for processing (2)
scripts/submission_quality_gate.pytests/test_submission_quality_gate.py
|
Thanks, agreed. I pushed What changed:
Fresh validation:
|
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Re-reviewed current head aa16b142fc316bd36e62288f0a0d9c60d91f5083 after the author follow-up.
The blocker from my previous review is fixed. _load_api_attempts() now catches UnicodeDecodeError and wraps it in RuntimeError, so _load_live_context() can mark active attempts as unverified and emit the existing warning instead of crashing. The added regression covers the full live-context path with a valid bounty-list row plus malformed /api/v1/bounties/{id}/attempts bytes.
Validation run locally on the updated head:
uv run --extra dev python -m pytest tests/test_submission_quality_gate.py -q->28 passeduv run --extra dev python -m pytest tests/test_pr_queue_health.py -q->9 passeduv run --extra dev python -m pytest -q->419 passeduv run --extra dev ruff check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py-> passeduv run --extra dev ruff format --check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py->2 files already formatteduv run --extra dev python -m mypy scripts/submission_quality_gate.py-> successuv run --extra dev python scripts/docs_smoke.py-> docs smoke okgit diff --check origin/main...HEAD-> cleangitleaks detect --no-banner --redact --source . --log-opts origin/main..HEAD --exit-code 1-> no leaks found
GitHub quality check is successful. CodeRabbit was still processing the updated head at the time of this review, so I checked the changed surface locally. This looks ready from my review.
|
Reviewed current head I would hold this because PR #478 validates API Concrete repro on this head: mock Observed repro output: {"active_attempts": [], "active_attempts_verified": true, "awards_remaining": 1, "id": 11, "maintainer_activity_verified": false, "max_maintainer_age_days": 14, "number": 319, "payability_verified": true, "state": "CLOSED", "title": "MRWK bounty: gate"}
[{"message": "referenced bounty #319 is closed or exhausted", "name": "bounty_payable", "status": "fail"}]Expected fix: when an API bounty row is present, propagate Validation: |
|
Follow-up for CodeRabbit's API status propagation warning is pushed in What changed:
Validation:
|
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Re-reviewed current head 27089896ed0c6d13f4be227820b0daf8b91ec926.
This resolves the prior API-status propagation blocker. _load_live_context() now carries the validated MergeWork API bounty state into the evaluated bounty, so _bounty_is_payable() no longer falls back to the GitHub issue state when API bounty data is available. The new regression covers an API-closed bounty with the GitHub issue still open and confirms the gate fails it as not payable.
Validation run locally:
uv run --extra dev python -m pytest tests/test_submission_quality_gate.py -q->29 passed in 1.14s.uv run --extra dev python -m pytest -q->420 passed in 87.10s.uv run --extra dev python scripts/docs_smoke.py->docs smoke ok.uv run --extra dev python -m mypy app scripts/submission_quality_gate.py->Success: no issues found in 31 source files.uv run --extra dev ruff check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py->All checks passed!.uv run --extra dev ruff format --check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py->2 files already formatted.git diff --check origin/main...HEAD-> clean.git diff origin/main...HEAD | gitleaks stdin --no-banner --redact --exit-code 1-> no leaks found.
Live readback: PR is open/non-draft, mergeable CLEAN, CI successful, and CodeRabbit reports no actionable comments on the current head.
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Re-reviewed current head 27089896ed0c6d13f4be227820b0daf8b91ec926 after the API status propagation follow-up.
I do not see a blocker on this head. The earlier malformed-attempts UTF-8 issue remains fixed, and the new state propagation change makes _load_live_context() carry the validated API bounty state into the evaluated bounty row with api_bounty.get("state", issue.get("state")). That resolves the prior false-fail path where the API row says the bounty is open/payable while the GitHub issue state is closed.
Validation run locally on the reviewed head:
/home/kali/money/mergework/.venv/bin/python -m pytest tests/test_submission_quality_gate.py -q->29 passed/home/kali/money/mergework/.venv/bin/python -m pytest tests/test_pr_queue_health.py -q->9 passed/home/kali/money/mergework/.venv/bin/python -m pytest -q->420 passed/home/kali/money/mergework/.venv/bin/python -m ruff check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py-> passed/home/kali/money/mergework/.venv/bin/python -m ruff format --check scripts/submission_quality_gate.py tests/test_submission_quality_gate.py->2 files already formatted/home/kali/money/mergework/.venv/bin/python -m mypy scripts/submission_quality_gate.py-> success, no issues in 1 source file/home/kali/money/mergework/.venv/bin/python scripts/docs_smoke.py-> docs smoke okgit diff --check upstream/main...HEAD-> cleangitleaks detect --no-banner --redact --source . --log-opts upstream/main..HEAD --exit-code 1-> no leaks found- Local explicit API-open/GitHub-closed repro now preserves API state and does not emit the closed/exhausted false fail.
GitHub CI and CodeRabbit are successful on the reviewed head.
3 participants
Summary
scripts/submission_quality_gate.pylive API bounty parsing against malformed UTF-8, malformed rows, missing authoritative fields, and full API pagesissue_number, non-emptystatus, and integer-coercibleawards_remainingbefore the gate treats API payability as verifiedRefs #406.
Evidence
The submission quality gate is a preflight tool for bounty PRs. Its live path already warns when API bounty payability cannot be verified, but
_load_api_bounties()could still let malformed API data through: invalid UTF-8 escaped theRuntimeErrorfallback boundary, missingstatusdefaulted to open, missingawards_remainingcould be surfaced as an incomplete authoritative row, and a full API page was not treated as a truncation risk. This patch keeps the existing fallback behavior while making the API boundary explicit and test-covered.Live API sanity check:
https://api.mrwk.ltclab.site/api/v1/bounties?status=open&limit=200returned a list with 7 open bounty rows, including expectedid,repo,issue_number,status, andawards_remainingfields.Validation
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ~/.local/bin/uv run --extra dev python -m pytest tests/test_submission_quality_gate.py -q->27 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ~/.local/bin/uv run --extra dev python -m pytest -q->418 passed~/.local/bin/uv run --extra dev ruff check .-> passed~/.local/bin/uv run --extra dev ruff format --check .->79 files already formatted~/.local/bin/uv run --extra dev mypy app scripts/submission_quality_gate.py-> successgit diff --check-> cleanNote: pytest plugin autoload is disabled because this workstation has an unrelated ROS Humble pytest plugin on the global path.
Summary by CodeRabbit
Bug Fixes
Tests