fix: use uv for modal deployment

[Bnowako]

running the deployment with raw modal, could run with wrong python env and lead to failed deployment. This change guarantees running with correct python environment

Summary by CodeRabbit

• Chores
  • Added preflight validation to the deployment process to verify required system tooling and project configuration before running a deployment.
  • Standardized how the deployment command is invoked across deployment modes while preserving existing branching and failure behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f8bcff3b-ac63-4f5f-af79-2016979a5213

📥 Commits

Reviewing files that changed from the base of the PR and between 08a991f and 842035d.

📒 Files selected for processing (1)

• terraform/modules/modal-app/scripts/deploy.sh

🚧 Files skipped from review as they are similar to previous changes (1)

• terraform/modules/modal-app/scripts/deploy.sh

---

📝 Walkthrough

Walkthrough

Added preflight checks to the deployment script to verify uv is in PATH and that pyproject.toml exists in DEPLOY_PATH. Replaced direct modal deploy ... invocations with a MODAL_CMD=(uv run modal) wrapper while keeping existing deploy-mode branching and error handling.

Changes

Cohort / File(s)	Summary
Deployment Script terraform/modules/modal-app/scripts/deploy.sh	Added checks for uv availability and presence of pyproject.toml in DEPLOY_PATH. Introduced MODAL_CMD=(uv run modal) and replaced all modal deploy ... calls with ${MODAL_CMD[@]} deploy .... Preserved branching for DEPLOY_MODULE values (deploy, src, others) and existing failure messages/exit codes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I nudge the script with a twitchy nose,
I check the tools where the deployment goes,
I bundle uv in a tidy command,
Then hop aside as the modal takes the land,
Soft thumps of code where safe deploys grow.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: introducing uv for modal deployment with preflight validation checks.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

terraform/modules/modal-app/scripts/deploy.sh (2)

43-43: Consider uv run --frozen for deterministic deploys.

Using uv run --frozen modal ensures deployments use exactly the locked environment and fails fast if uv.lock is out of sync, rather than implicitly re-resolving during a production deploy. Since Terraform invokes this from CI/local-exec where uv sync --frozen is already the documented workflow, aligning MODAL_CMD with --frozen avoids surprise resolutions.

-MODAL_CMD=(uv run modal)
+MODAL_CMD=(uv run --frozen modal)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/modal-app/scripts/deploy.sh` at line 43, Update the
MODAL_CMD definition in deploy.sh to include the --frozen flag so deployments
use the exact locked environment; specifically change the MODAL_CMD variable
(MODAL_CMD=(uv run modal)) to include --frozen (i.e., add the --frozen token to
the array) so Terraform/local-exec invokes uv run --frozen modal and fails if
uv.lock is out of sync.

---

33-36: Hardcoded path in error message.

Line 34's guidance references packages/modal-infra even though this script is a generic Terraform module driven by DEPLOY_PATH. Prefer ${DEPLOY_PATH} so the message stays accurate for any caller.

Suggested diff

-    echo "Error: uv is required to deploy packages/modal-infra. Install uv, then run 'cd packages/modal-infra && uv sync --frozen --extra dev'."
+    echo "Error: uv is required to deploy ${APP_NAME}. Install uv, then run 'cd ${DEPLOY_PATH} && uv sync --frozen --extra dev'."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/modal-app/scripts/deploy.sh` around lines 33 - 36, The
error message in the uv check hardcodes "packages/modal-infra" which is
incorrect for a generic module; update the echo in the uv existence branch to
reference the DEPLOY_PATH variable (e.g., use ${DEPLOY_PATH}) so the guidance
reflects the actual caller path used by this script (look for the uv check block
in deploy.sh that uses command -v uv and the echo/exit sequence).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@terraform/modules/modal-app/scripts/deploy.sh`:
- Around line 45-48: The preflight check unconditionally tests "import
sandbox_runtime" which is specific to modal-infra and will fail other
deployments; change the check in deploy.sh to either only run when DEPLOY_MODULE
(or DEPLOY_PATH) corresponds to the modal-infra case or remove the hard check
and rely on "uv run" to surface import errors; additionally update the error
text to reference ${DEPLOY_PATH} instead of the hardcoded packages/modal-infra
so the message is correct when DEPLOY_PATH points elsewhere (look for the "if !
uv run python -c \"import sandbox_runtime\"" block and the echo using
packages/modal-infra to modify).

---

Nitpick comments:
In `@terraform/modules/modal-app/scripts/deploy.sh`:
- Line 43: Update the MODAL_CMD definition in deploy.sh to include the --frozen
flag so deployments use the exact locked environment; specifically change the
MODAL_CMD variable (MODAL_CMD=(uv run modal)) to include --frozen (i.e., add the
--frozen token to the array) so Terraform/local-exec invokes uv run --frozen
modal and fails if uv.lock is out of sync.
- Around line 33-36: The error message in the uv check hardcodes
"packages/modal-infra" which is incorrect for a generic module; update the echo
in the uv existence branch to reference the DEPLOY_PATH variable (e.g., use
${DEPLOY_PATH}) so the guidance reflects the actual caller path used by this
script (look for the uv check block in deploy.sh that uses command -v uv and the
echo/exit sequence).

🪄 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: c9e817c9-74bb-4b1c-8022-296a76de80bc

📥 Commits

Reviewing files that changed from the base of the PR and between c6f0ea4 and 08a991f.

📒 Files selected for processing (1)

• terraform/modules/modal-app/scripts/deploy.sh

[ColeMurray]

Thanks for the contribution @Bnowako — the direction here is right and we want to standardize on uv for Modal deployment.

We couldn't push directly to your branch (maintainerCanModify is disabled on your fork), so we opened #527 which builds on your changes with a few additions needed to keep CI working:

1. CI workflow update — the terraform apply job installed Modal via pip install modal, so merging your deploy.sh change alone would have broken CI (no uv on PATH). fix: use uv for modal deployment #527 replaces the pip setup with astral-sh/setup-uv + uv sync --frozen.
2. uv sync --frozen before deploy — ensures sandbox-runtime (the sibling package from Errors during initial deployment #521) is installed before modal deploy runs, which is the actual root cause of the error in Errors during initial deployment #521.
3. Error message fix — replaced the hardcoded packages/modal-infra reference with ${APP_NAME}/${DEPLOY_PATH} so the message stays correct for any caller of the Terraform module.
4. Docs update — removed pipx install modal from GETTING_STARTED.md (redundant now that uv sync installs it), added the uv sync step to the setup flow, and added a troubleshooting entry for the sandbox_runtime error.

We'll close this PR once #527 is merged. Thanks again for flagging the issue!