Repository navigation
fix(app): surface uninstall failures instead of reporting false success - #1973
Merged
Merged
Conversation
DeleteBaseCommand caught errors thrown by deleteResource() but then still called process.complete(), which made the SimpleProcessRenderer print "Process completed successfully" even though the deletion had failed. The command therefore reported success (and appeared to exit 0) when e.g. `app uninstall` was rejected with HTTP 412. The catch branch now calls process.error() so the failure is surfaced, and keeps ux.exit(1) to guarantee a non-zero exit code for scripts. Additionally, `app uninstall` now maps the HTTP 412 that the API returns for an app with a linked primary database to an actionable message instead of a raw Axios error, while passing all other errors through unchanged. Closes #1970
gandie
approved these changes
Jul 2, 2026
martin-helmich
marked this pull request as draft
July 2, 2026 19:44
martin-helmich
commented
Jul 2, 2026
gandie
marked this pull request as ready for review
July 23, 2026 09:34
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
2 participants
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.
Problem
mw app uninstall <id> --forceprinted both an error andProcess completed successfully, and appeared to exit0, even when the uninstall actually failed with HTTP412(the app has a linked primary database). Scripts could not detect the failure, and the reason was not surfaced.Root cause
DeleteBaseCommand.exec()caught errors thrown bydeleteResource()and marked the step as failed, but then still calledprocess.complete(...). TheSimpleProcessRenderer(used in non-TTY/CI contexts) unconditionally writesCompleted: Process completed successfullyincomplete(), so a failed deletion was reported as a success.Fix
DeleteBaseCommandnow callsprocess.error(err)in the catch branch instead ofprocess.complete(...), so the failure is surfaced. It keepsux.exit(1)to guarantee a non-zero exit code. This corrects all delete commands.app uninstallnow maps the HTTP412(linked primary database) to an actionable message — "cannot uninstall: app has a linked primary database — unlink or repurpose it first ..." — while passing every other error through unchanged (with the original error attached ascause).Tests
src/lib/basecommands/DeleteBaseCommand.test.tsx— asserts that on a failing deletion the command does not report success, surfaces the error viaprocess.error, and exits non-zero (ExitErrorwithoclif.exit === 1); and that a successful deletion still reports success and does not exit non-zero.src/commands/app/uninstall.test.ts— covers the412→ primary-database message mapping and pass-through of other errors.Closes #1970
🤖 Generated with Claude Code