Fix deployment page link id resolution#6365
Conversation
📝 WalkthroughWalkthroughDeploy command URL resolution now distinguishes numeric deployment IDs from UUID-only payloads, extracts crew UUIDs from deployment/status responses, and falls back to a status lookup before opening the deployment page. Tests cover the new UUID-only and failure paths. ChangesUUID-only deployment page resolution
Sequence Diagram(s)sequenceDiagram
participant DeployCommand
participant plus_api_client
participant browser
DeployCommand->>plus_api_client: crew_status_by_uuid(crew_uuid)
alt status lookup succeeds
plus_api_client-->>DeployCommand: status_response
DeployCommand->>DeployCommand: _deployment_identifier_from_status(status_response)
DeployCommand->>browser: open(deployment_page_url)
else status lookup fails
plus_api_client-->>DeployCommand: failure / invalid payload
end
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/cli/src/crewai_cli/deploy/main.py (1)
261-278: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfusing name collision with swapped argument order.
The instance method
_deployment_page_url(self, json_response, base_url)shares its name with the module-level_deployment_page_url(base_url, json_response)but reverses the parameter order. The current call on Line 267 is correct, but this collision is fragile: a future edit that accidentally swaps the arguments (or prefixes the call withself.) would silently build a wrong URL or recurse. Consider renaming one of them (e.g., the module helper to_build_deployment_page_url) and aligning parameter order.🤖 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 `@lib/cli/src/crewai_cli/deploy/main.py` around lines 261 - 278, The deployment URL helpers have a confusing name collision: the instance method _deployment_page_url(self, json_response, base_url) and the module-level _deployment_page_url(base_url, json_response) use opposite argument order, which is easy to misuse later. Rename one of them, such as the module helper to _build_deployment_page_url, and make the parameter order consistent at the call site in the _deployment_page_url method so the logic remains unambiguous and avoids accidental recursion or malformed URLs.
🤖 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.
Nitpick comments:
In `@lib/cli/src/crewai_cli/deploy/main.py`:
- Around line 261-278: The deployment URL helpers have a confusing name
collision: the instance method _deployment_page_url(self, json_response,
base_url) and the module-level _deployment_page_url(base_url, json_response) use
opposite argument order, which is easy to misuse later. Rename one of them, such
as the module helper to _build_deployment_page_url, and make the parameter order
consistent at the call site in the _deployment_page_url method so the logic
remains unambiguous and avoids accidental recursion or malformed URLs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 95f4a1f2-80fa-4b70-8dfc-94e04d0a9407
📒 Files selected for processing (2)
lib/cli/src/crewai_cli/deploy/main.pylib/cli/tests/deploy/test_deploy_main.py
alex-clawd
left a comment
alex-clawd
left a comment
There was a problem hiding this comment.
Clean fix — proper separation of crew UUID vs numeric deployment ID, graceful degradation when status lookup fails, good test coverage on both paths.
joaomdmoura
force-pushed
the
codex/fix-deployment-page-id
branch
from
ae7392f to
19f4cd6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/cli/src/crewai_cli/deploy/main.py (1)
242-259: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMethod shares a name with the module-level helper, which is easy to misread.
The instance method
_deployment_page_urland the module-level function_deployment_page_urlhave the same name with a swapped argument order. It works because the bare call on Line 248 resolves to the module global (class scope isn't visible in method bodies), but the collision plus the reversed(base_url, json_response)vs(json_response, base_url)signatures is a readability trap. Consider renaming one (e.g.,_resolve_deployment_page_url) to make the delegation explicit.🤖 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 `@lib/cli/src/crewai_cli/deploy/main.py` around lines 242 - 259, The instance method and module-level helper share the same _deployment_page_url name, and the swapped argument order makes the delegation hard to read. Rename one of them, preferably the instance method in the Deploy CLI class, to something like _resolve_deployment_page_url, and update the internal call so it is explicit which helper is being used. Keep the module-level helper’s behavior unchanged, but make the method-to-helper delegation in deploy/main.py unambiguous.
🤖 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.
Nitpick comments:
In `@lib/cli/src/crewai_cli/deploy/main.py`:
- Around line 242-259: The instance method and module-level helper share the
same _deployment_page_url name, and the swapped argument order makes the
delegation hard to read. Rename one of them, preferably the instance method in
the Deploy CLI class, to something like _resolve_deployment_page_url, and update
the internal call so it is explicit which helper is being used. Keep the
module-level helper’s behavior unchanged, but make the method-to-helper
delegation in deploy/main.py unambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c1b3148-aed2-4829-8697-73b5eb350ef2
📒 Files selected for processing (2)
lib/cli/src/crewai_cli/deploy/main.pylib/cli/tests/deploy/test_deploy_main.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/cli/tests/deploy/test_deploy_main.py
alex-clawd
left a comment
alex-clawd
left a comment
There was a problem hiding this comment.
Simplified nicely — direct uuid lookup instead of the helper abstraction. Clean and sufficient.
2 participants
Summary
Tests
Note
Low Risk
Scoped to CLI post-deploy browser URL building and identifier parsing; failures are swallowed without affecting deploy success.
Overview
Fixes broken deployment links after create/deploy when the API response only includes a crew UUID, not the numeric deployment id the
/crewai_plus/deployments/:idroute expects.Identifier resolution no longer treats
uuidas a fallback deployment id (including nesteddeployment.uuid).DeployCommand._deployment_page_urlstill builds a URL fromdeployment_id/idwhen present; otherwise it callscrew_status_by_uuidand uses the status payload’s id. If that lookup fails, the CLI does not open the browser with a UUID URL.Reviewed by Cursor Bugbot for commit 19f4cd6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit