Skip to content

Conversation

@jtcorbett
Copy link
Contributor

@jtcorbett jtcorbett commented Sep 15, 2025

TL;DR

Added retry functionality to the MCP Agent deployment process to improve reliability.

What changed?

  • Added a new --retry-count option to the deploy command with a default of 3 retries
  • Implemented exponential backoff retry logic for deployment operations
  • Split the deployment process into bundling and deployment phases for better error handling
  • Added progress indicators showing the current attempt number
  • Enhanced error messages to indicate whether errors are retriable or not
  • Added visual feedback during retry attempts with warning messages

How to test?

  1. Deploy an MCP agent with the default retry settings:

    mcp-agent deploy config ./my-config
    
  2. Test with a custom retry count:

    mcp-agent deploy config ./my-config --retry-count 5
    
  3. Force a deployment failure to verify retry behavior (e.g., by temporarily disrupting network connectivity)

Summary by CodeRabbit

  • New Features

    • Added deployment retry support via --retry-count (default 3, range 1–10).
    • Enhanced deployment progress feedback and final output (App ID, App URL, App Status).
  • Bug Fixes

    • Improved error handling during deployment: non-retriable errors surfaced immediately and verbose traces preserved when enabled.
    • Properly handles cancellation during retries to avoid unintended reattempts.
  • Tests

    • Updated tests to cover the new retry-count option.

@coderabbitai
Copy link

coderabbitai bot commented Sep 15, 2025

Caution

Review failed

The pull request is closed.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds retry-enabled deployment to the deploy CLI with a new --retry-count option and an internal async retry flow. Updates the retry utility to re-raise asyncio.CancelledError. Adjusts a test to pass retry_count. The deploy path now bundles, deploys via mcp_app_client, and retries with exponential backoff on failure.

Changes

Cohort / File(s) Summary
Deploy CLI: retry-enabled flow
src/mcp_agent/cli/cloud/commands/deploy/main.py
Introduces _deploy_with_retry using exponential backoff; integrates bundling (wrangler_deploy) then API deploy; surfaces per-attempt progress; adds --retry-count (1–10, default 3); marks certain CLI errors non-retriable; updates deploy_config signature to include retry_count.
Retry utility: cancellation propagation
src/mcp_agent/cli/utils/retry.py
retry_async_with_exponential_backoff now re-raises asyncio.CancelledError immediately; non-cancelled exceptions follow existing retry/backoff logic; no API signature changes.
Tests: deploy invocation updated
tests/cli/commands/test_deploy_command.py
Updates test_deploy_with_secrets_file to pass retry_count=3 to deploy_config; no other behavioral changes in tests.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CLI as Deploy CLI (deploy_config)
  participant Bundler as wrangler_deploy
  participant AppAPI as mcp_app_client.deploy_app
  participant Retry as retry_async_with_exponential_backoff

  User->>CLI: deploy --retry-count=N
  CLI->>Retry: attempt deploy (max N)
  rect rgb(240,248,255)
    note over CLI,Retry: Single attempt flow
    Retry->>Bundler: bundle artifacts
    Bundler-->>Retry: bundle result or error
    Retry->>AppAPI: deploy bundle
    AppAPI-->>Retry: app info or error
  end
  alt Success
    Retry-->>CLI: app info
    CLI-->>User: Print App ID/URL/Status
  else Non-retriable error
    Retry-->>CLI: raise CLIError(retriable=False)
    CLI-->>User: Error (no further retries)
  else Retriable error
    Retry-->>Retry: exponential backoff then retry
  end

  %% Cancellation path
  User--x CLI: cancel
  CLI->>Retry: cancel in-flight attempt
  Retry-->>CLI: re-raise asyncio.CancelledError
  CLI-->>User: Aborted
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • rholinshead
  • petersonbill64
  • saqadri

Poem

I hop through logs, a vigilant sprite,
Retrying the clouds by day and night.
If storms arise, I wait—then leap!
Exponential dreams, not too deep.
Cancel the wind? I’ll swiftly bail—
Yet try, try again, with a twinkling tail. 🐇⚙️

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 09-15-update_deploy_to_retry_if_deployment_failed

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe2567c and 4311918.

📒 Files selected for processing (3)
  • src/mcp_agent/cli/cloud/commands/deploy/main.py (7 hunks)
  • src/mcp_agent/cli/utils/retry.py (1 hunks)
  • tests/cli/commands/test_deploy_command.py (1 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor Author

jtcorbett commented Sep 15, 2025

@jtcorbett jtcorbett marked this pull request as ready for review September 15, 2025 19:10
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from c76d1e8 to a532d02 Compare September 15, 2025 19:18
@jtcorbett jtcorbett force-pushed the 09-15-create_retry_utility_and_mark_certain_errors_as_non-retriable branch from 65929ef to bd0f3de Compare September 15, 2025 19:18
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from a532d02 to e748862 Compare September 15, 2025 19:20
@jtcorbett jtcorbett force-pushed the 09-15-create_retry_utility_and_mark_certain_errors_as_non-retriable branch from bd0f3de to ddb1062 Compare September 15, 2025 19:20
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from e748862 to b630e23 Compare September 15, 2025 19:26
@jtcorbett jtcorbett force-pushed the 09-15-create_retry_utility_and_mark_certain_errors_as_non-retriable branch from ddb1062 to 79b1e41 Compare September 15, 2025 19:26
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from b630e23 to 01a0d8b Compare September 15, 2025 19:31
@jtcorbett jtcorbett force-pushed the 09-15-create_retry_utility_and_mark_certain_errors_as_non-retriable branch from 79b1e41 to a3ed6b7 Compare September 15, 2025 20:51
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from 01a0d8b to 9e5aeed Compare September 15, 2025 20:51
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from 9e5aeed to a02b521 Compare September 15, 2025 21:02
Copy link
Collaborator

@saqadri saqadri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, that comment from graphite PR bot seems legit

@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from ddd950f to 41d9904 Compare September 15, 2025 21:50
@jtcorbett jtcorbett force-pushed the 09-15-create_retry_utility_and_mark_certain_errors_as_non-retriable branch from a3ed6b7 to 17040ab Compare September 15, 2025 21:50
@jtcorbett jtcorbett mentioned this pull request Sep 15, 2025
@jtcorbett jtcorbett force-pushed the 09-15-create_retry_utility_and_mark_certain_errors_as_non-retriable branch from 17040ab to 5b8c790 Compare September 15, 2025 22:00
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from 41d9904 to 5b1ced8 Compare September 15, 2025 22:00
Copy link
Contributor Author

jtcorbett commented Sep 15, 2025

Merge activity

  • Sep 15, 10:43 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Sep 15, 10:45 PM UTC: Graphite couldn't merge this PR because it failed for an unknown reason (Your push would publish a private email address. You can make your email public or disable this protection on GitHub).

@jtcorbett jtcorbett changed the base branch from 09-15-create_retry_utility_and_mark_certain_errors_as_non-retriable to graphite-base/477 September 15, 2025 22:43
@jtcorbett jtcorbett changed the base branch from graphite-base/477 to main September 15, 2025 22:43
@jtcorbett jtcorbett force-pushed the 09-15-update_deploy_to_retry_if_deployment_failed branch from f1f34d6 to 4311918 Compare September 15, 2025 22:49
@jtcorbett jtcorbett merged commit 9991128 into main Sep 15, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants