Unify return code framework within the multipass CLI #4567
Conversation
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
Change the return codes of the shell and exec commands to the following paradigm: if there is an internal failure in multipass, the return code is 255, else the return code from the internal VM is returned.
Moved the ssh_channel_get_exit_state function call inside the SSH::throw_on_error wrapper to follow existing standards.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4567 +/- ##
==========================================
+ Coverage 87.18% 87.21% +0.03%
==========================================
Files 247 248 +1
Lines 14273 14290 +17
==========================================
+ Hits 12443 12462 +19
+ Misses 1830 1828 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ricab
reviewed
Dec 12, 2025
| }; | ||
| // Only implicitly int-convertible types should be used here. If at least 2 of the same type are | ||
| // used, index-based get<> and holds_alternative<> are needed. | ||
| using ReturnCodeVariant = std::variant<ReturnCode, VMReturnCode>; |
Collaborator
There was a problem hiding this comment.
That's it, that's what we need! Thank you so much for taking care of it 👍
Collaborator
There was a problem hiding this comment.
PS: not an actual review, just had a light peek
Contributor
There was a problem hiding this comment.
Pull request overview
This PR unifies the return code framework within the multipass CLI by introducing a variant-based return type system that distinguishes between CLI-side failures (multipass errors) and VM-side return codes. The key changes include:
- Introduction of
ReturnCodeVariantto handle bothReturnCodeandVMReturnCodetypes - Addition of
ShellExecFail(value 255) to distinguish multipass failures from VM exit codes - Migration of
ssh_channel_get_exit_statusto the existing libssh error handling framework - Consistent return type updates across all CLI commands and test files
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| include/multipass/cli/return_codes.h | Introduces VMReturnCode enum and ReturnCodeVariant, adds ShellExecFail code (255), and helper function for variant comparison |
| include/multipass/cli/command.h | Updates command interface to return ReturnCodeVariant instead of ReturnCode |
| include/multipass/ssh/ssh_client.h | Removes unused exec(vector<string>) overload and updates ssh_channel_get_exit_status signature |
| src/ssh/ssh_client.cpp | Refactors to use libssh error framework and returns VM exit codes as VMReturnCode |
| src/client/cli/main.cpp | Converts ReturnCodeVariant to int for process exit code |
| src/client/cli/client.h | Updates run method to return ReturnCodeVariant |
| src/client/cli/cmd/shell.cpp | Returns VMReturnCode for shell exit codes and ShellExecFail for exceptions |
| src/client/cli/cmd/exec.cpp | Returns VMReturnCode for command exit codes and ShellExecFail for exceptions |
| src/client/cli/cmd/*.cpp (multiple) | Updates all command implementations to return ReturnCodeVariant with explicit return type annotations in lambdas |
| src/client/cli/cmd/common_cli.cpp | Updates helper functions for variant-based return codes |
| tests/test_ssh_client.cpp | Updates test expectations for new API signature |
| tests/test_cli_client.cpp | Updates tests for new return codes and variant conversions |
| tests/daemon_test_fixture.cpp | Updates test commands with explicit ReturnCodeVariant return types |
| tests/test_daemon.cpp | Removes unused includes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c856668 to
03a365a
Compare
sharder996
approved these changes
Dec 15, 2025
Collaborator
sharder996
left a comment
There was a problem hiding this comment.
LGTM! Thanks for tackling this @tobe2098
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Related Issue(s)
Closes #4428
Closes #3054
Testing
Unit tests
Manual testing steps:
Checklist
MULTI-2412