Improve deploy status and retry UX#26
Merged
Merged
Conversation
Summary by CodeRabbit
WalkthroughAdds phase-tracking state and phase-aware progress callbacks to preview deploy, writes local binding on first deploy, maps deploy failures to humanLines-aware CliError, simplifies deploy success output (duration + URL + logs row), updates related docs, and expands tests for output and failure scenarios. ChangesApp Deploy Progress and Error Handling
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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.
Actionable comments posted: 3
🤖 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 `@packages/cli/src/controllers/app.ts`:
- Around line 2497-2521: The current CliError always uses the runtime-focused
headline and human message ("Runtime failed after the build completed.") even
when the failure occurred during deploy/upload/start; update the CliError
construction (the return new CliError({...}) block) to pick a phase-accurate
headline and humanLines first line based on progress.containerLive (or the same
phase logic used for meta.phase): e.g., if progress.containerLive is true keep
"Runtime failed after the build completed.", otherwise use "Deploy failed after
the build completed." Make the summary field and the first element of humanLines
conditional so the displayed message matches meta.phase.
In `@packages/cli/src/shell/errors.ts`:
- Line 49: Normalize the humanLines assignment so empty arrays become null and
non-empty arrays are cloned: replace the direct assignment to this.humanLines
from options.humanLines with logic that sets this.humanLines = null when
options.humanLines is falsy or has length === 0, otherwise set this.humanLines
to a shallow clone (e.g., [...options.humanLines]); update the
constructor/assignment site that currently uses "this.humanLines =
options.humanLines ?? null" to use this new normalization so downstream
rendering will fall back correctly when no human lines are provided.
In `@packages/cli/src/shell/output.ts`:
- Around line 71-81: The current conditional treats an empty array in
error.humanLines as truthy and can cause no output; change the guard in the
block that uses error.humanLines (the branch that builds lines, appends trace
via options.trace and error.debug, calls renderNextSteps and writeHumanLines) to
only run when humanLines is a non-empty array (e.g., check
Array.isArray(error.humanLines) && error.humanLines.length > 0); leaving the
rest of the logic (trace handling, renderNextSteps(error.nextSteps),
writeHumanLines(output, lines)) intact so errors with humanLines: [] fall
through to default formatting.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 32d8022d-a1b4-4986-a294-db8a5ad7b692
📒 Files selected for processing (10)
docs/product/command-spec.mddocs/product/error-conventions.mddocs/product/output-conventions.mdpackages/cli/src/controllers/app.tspackages/cli/src/lib/app/preview-progress.tspackages/cli/src/presenters/app.tspackages/cli/src/shell/errors.tspackages/cli/src/shell/output.tspackages/cli/src/types/app.tspackages/cli/tests/app-controller.test.ts
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 `@packages/cli/src/controllers/app.ts`:
- Around line 2483-2485: The recovery copy uses an inconsistent CLI prefix;
update the command string(s) that currently read "prisma app logs --deployment
${progress.versionId}" to use the canonical prefix "prisma-cli" instead (so
"prisma-cli app logs --deployment ${progress.versionId}"). Edit the
recoveryLines definition (the const recoveryLines that depends on
progress.versionId) and any other occurrences in the same failure-path block
(the similar strings around lines 2510-2512) to replace "prisma " with
"prisma-cli" so all recovery messages use the same CLI invocation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e71e34ef-b54a-427b-a688-46cef978caeb
📒 Files selected for processing (11)
docs/product/command-spec.mddocs/product/error-conventions.mddocs/product/output-conventions.mdpackages/cli/src/controllers/app.tspackages/cli/src/lib/app/deploy-output.tspackages/cli/src/lib/app/preview-progress.tspackages/cli/src/presenters/app.tspackages/cli/src/shell/errors.tspackages/cli/src/shell/output.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/output.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/presenters/app.ts (1)
50-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the success action labeled as
Runtime logs.The rest of this deploy flow is now phase-aware, but this row falls back to a generic
Logslabel. Renaming it keeps the success block aligned with the new build/deploy/runtime wording in this PR.Suggested fix
- { label: "Logs", value: "prisma-cli app logs" }, + { label: "Runtime logs", value: "prisma-cli app logs" },🤖 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 `@packages/cli/src/presenters/app.ts` around lines 50 - 56, Update the success action label in the deploy presenter: in the array assigned to lines (in packages/cli/src/presenters/app.ts) change the deploy output row entry passed to renderDeployOutputRows from { label: "Logs", value: "prisma-cli app logs" } to use the new label "Runtime logs" so the success block is phase-aware and consistent with build/deploy/runtime terminology.
🤖 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.
Outside diff comments:
In `@packages/cli/src/presenters/app.ts`:
- Around line 50-56: Update the success action label in the deploy presenter: in
the array assigned to lines (in packages/cli/src/presenters/app.ts) change the
deploy output row entry passed to renderDeployOutputRows from { label: "Logs",
value: "prisma-cli app logs" } to use the new label "Runtime logs" so the
success block is phase-aware and consistent with build/deploy/runtime
terminology.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad974083-c002-4642-978b-6a0bc5000a0d
📒 Files selected for processing (6)
docs/product/command-spec.mddocs/product/error-conventions.mddocs/product/output-conventions.mdpackages/cli/src/controllers/app.tspackages/cli/src/presenters/app.tspackages/cli/tests/app-controller.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.prisma/local.jsonbefore build/deploy and using a compact retry header afterward.Runtime logs.Tests
pnpm --filter @prisma/cli exec vitest run tests/app-controller.test.ts tests/output.test.tspnpm --filter @prisma/cli buildpnpm --filter @prisma/cli test