Extract bounty attempt helpers#363
Conversation
|
Warning Review limit reached
More reviews will be available in 11 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR extracts bounty-attempt helper logic from app/main.py into a dedicated module, adds focused unit tests for the new helpers, and includes a small hardening update to _safe_next_path plus minor typing cleanup in ledger service.
Changes:
- Added
app/bounty_attempts.pyto host attempt serialization, status/warnings, and expiration helpers. - Updated
app/main.pyto use the extracted helpers and improved_safe_next_pathvalidation by checking decoded input. - Added
tests/test_bounty_attempt_helpers.pyto cover expiration/status/warnings behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_bounty_attempt_helpers.py | Adds tests validating attempt serialization/status and warnings/expiration flows. |
| app/main.py | Switches to extracted bounty-attempt helpers and strengthens next path validation using decoding. |
| app/ledger/service.py | Removes unnecessary typing casts around session.execute(...update...) results. |
| app/bounty_attempts.py | New standalone module for attempt helper functions previously embedded in app/main.py. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return value.astimezone(UTC) | ||
|
|
||
|
|
||
| def attempt_effective_status(attempt: BountyAttempt, now: datetime) -> str: |
| def _safe_next_path(next_path: str | None) -> str: | ||
| decoded_next_path = unquote(next_path) if next_path else "" | ||
| if ( | ||
| not next_path | ||
| or not next_path.startswith("/") | ||
| or next_path.startswith("//") | ||
| or len(next_path) > 2048 | ||
| or "\\" in next_path | ||
| or decoded_next_path.startswith("//") | ||
| or "\\" in decoded_next_path | ||
| or any(ord(char) < 32 or 127 <= ord(char) < 160 for char in next_path) | ||
| or any(ord(char) < 32 or 127 <= ord(char) < 160 for char in decoded_next_path) | ||
| ): | ||
| return "/me" | ||
| return next_path |
Refs #320
Summary
app/main.pyintoapp/bounty_attempts.py.mypy appand by sanitizing percent-decoded OAuthnextpaths covered by the existing OAuth regression test.Complexity reduced
app/main.pyno longer owns bounty-attempt serialization and query helper logic used by REST routes and MCP dispatch. Attempt behavior now lives in a small module with direct tests, leaving route handlers focused on auth, request parsing, and response status.Tests
python -m pytest -q-> 337 passedpython -m pytest tests/test_bounty_attempt_helpers.py tests/test_bounty_attempts.py tests/test_api_mcp.py::test_mcp_list_bounty_attempts_reports_active_and_expired tests/test_ledger.py -q-> 36 passedpython -m ruff format --check app/main.py app/ledger/service.py app/bounty_attempts.py tests/test_bounty_attempt_helpers.pypython -m ruff check app/main.py app/ledger/service.py app/bounty_attempts.py tests/test_bounty_attempt_helpers.pypython -m mypy app