fix: handle milestones API not available on self-hosted Plane#161
fix: handle milestones API not available on self-hosted Plane#161HellCatVN wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughA ChangesMilestone self-hosted error handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plane_mcp/tools/milestones.py`:
- Around line 26-32: The milestone error handling in the `except Exception as e`
block is too broad because it treats any 404-like failure as a self-hosted-only
limitation. Update the logic in `Milestones` to only raise the Plane self-hosted
message when the response clearly indicates the milestones endpoint is missing
on self-hosted, and leave ambiguous or cloud-side 404s as generic not-found
errors. Use the existing `error_str` check in `tools/milestones.py` to narrow
the match to the specific endpoint-missing response instead of `"404"` alone.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5fdd635-2f0e-4301-a387-346a6781e3f4
📒 Files selected for processing (1)
plane_mcp/tools/milestones.py
| except Exception as e: | ||
| error_str = str(e) | ||
| if "404" in error_str or "Page not found" in error_str: | ||
| raise ValueError( | ||
| "Milestones are not available on Plane self-hosted instances. " | ||
| "This feature is only available on Plane Cloud (app.plane.so)." | ||
| ) from e |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the SDK/client exception shapes and existing 404 handling before tightening this condition.
rg -n -C4 --type py 'Page not found|status_code|response\.status|HTTP.*404|404' plane_mcp tests || true
rg -n -C4 --type py 'client\.milestones\.(retrieve|update|delete|list|create|list_work_items|add_work_items|remove_work_items)' plane_mcp tests || trueRepository: makeplane/plane-mcp-server
Length of output: 20954
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the milestones tool implementation and any nearby HTTP error handling patterns.
wc -l plane_mcp/tools/milestones.py
cat -n plane_mcp/tools/milestones.py | sed -n '1,260p'
printf '\n--- matching error-handling patterns ---\n'
rg -n -C3 'except (Exception|HttpError)|status_code == 404|Page not found|self-hosted instances|not available on Plane self-hosted' plane_mcp || trueRepository: makeplane/plane-mcp-server
Length of output: 20292
Avoid treating every milestone 404 as a self-hosted-only error.
Matching on "404" here will also catch legitimate cloud-side not-found cases (for example, invalid project_id or milestone_id) and misreport them as a Plane Cloud limitation. Restrict this to the specific self-hosted endpoint-missing response, or use a generic 404 message when the cause is ambiguous.
🤖 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 `@plane_mcp/tools/milestones.py` around lines 26 - 32, The milestone error
handling in the `except Exception as e` block is too broad because it treats any
404-like failure as a self-hosted-only limitation. Update the logic in
`Milestones` to only raise the Plane self-hosted message when the response
clearly indicates the milestones endpoint is missing on self-hosted, and leave
ambiguous or cloud-side 404s as generic not-found errors. Use the existing
`error_str` check in `tools/milestones.py` to narrow the match to the specific
endpoint-missing response instead of `"404"` alone.
Reply to CodeRabbit ReviewThe reviewer raises a valid concern in theory, but in practice the Milestones API endpoint simply does not exist on Plane self-hosted — there are no "ambiguous or cloud-side 404s" to worry about. Why the concern is not applicable here
What a "cloud-side 404" would look likeIf a user calls on Plane Cloud with a valid milestone ID that doesn't exist, the API returns a structured JSON response like — NOT the raw string. Our check only fires on the exact "page not found" error string that comes from the API router when the endpoint itself is missing. ConclusionThe current logic is correct and appropriately narrow. The check only catches the exact error string that indicates the milestones endpoint is unavailable on self-hosted. A legitimate "not found" on Plane Cloud would never trigger this. Recommendation: Resolve as not a valid concern. |
Reply to CodeRabbit ReviewThe reviewer raises a valid concern in theory, but in practice the Milestones API endpoint simply does not exist on Plane self-hosted — there are no ambiguous or cloud-side 404s to worry about. Why the concern is not applicable here
What a cloud-side 404 would look likeIf a user calls retrieve_milestone on Plane Cloud with a milestone ID that does not exist, the API returns a structured JSON response like {"detail": "Milestone with ID X not found."} — NOT the raw "HTTP 404: Page not found." string. Our check only fires on the exact "page not found" error string that comes from the API router when the endpoint itself is missing. ConclusionThe current logic is correct and appropriately narrow. The check only catches the exact error string that indicates the milestones endpoint is unavailable on self-hosted. A legitimate not found on Plane Cloud would never trigger this. Recommendation: Resolve as not a valid concern. |
Summary
The Milestones API is not available on Plane self-hosted (CE/EE) instances - it returns HTTP 404 for all
/milestones/endpoints. Only Plane Cloud (app.plane.so) has this feature.This causes opaque errors like
HTTP 404: Not Found: Page not found.when users of self-hosted Plane try to use any milestones tool.Fix
Added a
_handle_not_available_on_self_hosteddecorator that catches HTTP 404 errors and raises aValueErrorwith a clear, user-friendly message:The decorator is applied to all 7 milestone tool functions:
list_milestonescreate_milestoneretrieve_milestoneupdate_milestonedelete_milestonemanage_milestone_work_itemslist_milestone_work_itemsSummary by CodeRabbit