fix: surface a failing bootstrap_command instead of implying success#34
Merged
Merged
Conversation
plugin:install discarded the bootstrap subprocess ProcessResult and only checked the notResolvable case, so a chained bootstrap_command that exited non-zero (stale fast-CLI bundle, unknown command, scaffold error) was reported to the operator as if it had completed. A from-scratch install QA hit this: the chained starter:install failed silently and the manifest post_install message still claimed the scaffold succeeded. BootstrapCommandRunner.run now returns a BootstrapRunResult carrying the subprocess exit code and captured stderr. plugin:install warns with the exit code + stderr and prints the manual bootstrap hint when the chained command exits non-zero; the success path stays quiet. Tests: runner exit-code/stderr propagation + succeeded() on zero exit; a non-zero bootstrap exit warns + hints without failing the install.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR fixes plugin:install reporting a chained bootstrap_command as if it succeeded even when the spawned dispatcher subprocess exits non-zero, by preserving and surfacing the subprocess result (exit code and stderr) to the operator.
Changes:
- Introduce
BootstrapRunResultto carryexitCodeand capturedstderrfromBootstrapCommandRunner.run. - Update
plugin:installto warn (exit code + stderr) and print the manual bootstrap hint when the chained command exits non-zero, while keeping install best-effort. - Add/adjust unit tests to cover exit-code/stderr propagation and the non-zero bootstrap warning path; update
CHANGELOG.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/src/commands/helpers/bootstrap_command_runner.dart | Adds BootstrapRunResult and returns subprocess exit code/stderr from run. |
| lib/src/commands/plugin_install_command.dart | Warns and hints when the bootstrap subprocess exits non-zero. |
| test/commands/helpers/bootstrap_command_runner_test.dart | Extends tests to assert exit code/stderr propagation and succeeded semantics. |
| test/commands/plugin_install_command_test.dart | Adds coverage for non-zero bootstrap exit warning + hint behavior. |
| CHANGELOG.md | Documents the fix under [Unreleased]. |
… code Addresses review feedback: new public types follow the final class convention, and an assert invariant prevents constructing an invoked result without an exit code (which could surface as 'exited with code null').
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
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
plugin:installdiscarded the bootstrap subprocessProcessResultand only handled thenotResolvablecase, so a chainedbootstrap_commandthat exited non-zero (stale fast-CLI bundle, unknown command, scaffold error) was reported to the operator as if it had completed.A from-scratch install QA hit exactly this: the chained
starter:installfailed silently and the manifestpost_installmessage still claimed the scaffold had succeeded, leavinglib/config/magic_starter.dartabsent with no signal to the user.Change
BootstrapCommandRunner.runnow returns aBootstrapRunResultcarrying the subprocessexitCodeand capturedstderr(previously it returned a bareBootstrapRunOutcomeenum and threw theProcessResultaway).plugin:installwarns with the exit code + stderr and prints the manual bootstrap hint when the chained command exits non-zero. The success path stays quiet; a failure to resolve a dispatcher still falls back to the hint as before.Tests
succeededtrue on zero exit.plugin:install: a non-zero bootstrap exit warns (exited with code N+ stderr) and hints, without failing the install.dart test: 1185 tests green;dart analyze+dart formatclean.Author: Anılcan Çakır anilcan.cakir@gmail.com