fix(use): prompt to install when target version missing#24
Merged
Fahl-Design merged 4 commits intomainfrom May 8, 2026
Merged
Conversation
Previously 'pvm use <ver>' bailed with an error if <ver> was not installed locally, leaving users (notably the 'pvm init' flow) stuck. Now the command offers to run the install dialog directly. Adds a hidden '--silent' flag for the cd hooks so a missing .php-version never blocks the shell prompt with an interactive question.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds silent mode support to the ChangesSilent use command
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 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 docstrings
🧪 Generate unit tests (beta)
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 |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/cli.rs (1)
128-140: ⚡ Quick winStrengthen assertions to match the documented silent-mode contract
The comment at line 138 states "exits 0 with no output and no env file written", but only the exit code is verified. The
predicatescrate is already imported; adding stdout/stderr checks is a one-liner.✅ Proposed fix
- // Silent mode: missing version exits 0 with no output and no env file written. - cmd.assert().success(); + // Silent mode: missing version exits 0 silently. + cmd.assert() + .success() + .stdout(predicate::str::is_empty()) + .stderr(predicate::str::is_empty()); + + // Env file should not have been written + assert!(!temp_dir.path().join("pvm_env_update").exists());As per coding guidelines: "Use
assert_cmdandpredicatescrates for CLI output and exit code verification in integration tests."🤖 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 `@tests/cli.rs` around lines 128 - 140, The test test_use_silent_skips_missing_version currently only asserts success; update it to also assert that both stdout and stderr are empty by chaining .stdout(predicate::str::is_empty()) and .stderr(predicate::str::is_empty()) on cmd.assert(), and after running the command verify that no env file was written in the temporary PVM_DIR (e.g., check temp_dir.path() has no new env file by asserting the directory has no entries or that the specific env filename is absent) so the test enforces "no output and no env file written".
🤖 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 `@src/commands/use_cmd.rs`:
- Around line 23-48: The branch that handles a missing local version returns
immediately after calling crate::commands::install::execute_install(v). Instead,
after a successful install, call fs::try_resolve_local_version(v) again and
assign its Some(resolved) to the same resolved/version binding so execution
falls through to the existing activation logic (the env-file write,
PVM_MULTISHELL_PATH assignment and PATH update) rather than returning early;
update the Some(ref v) match arm in use_cmd.rs to re-resolve post-install and
proceed to the activation path when resolution succeeds, handling the silent and
cancel cases as before.
In `@src/interactive.rs`:
- Around line 51-57: Match arm 3 is dispatching to the wrong command: it
constructs commands::use_cmd::Use instead of the list command, causing the "List
(View locally installed versions)" menu option to open the version-switcher.
Replace the construction in the arm with the list command (e.g., construct and
call commands::list_cmd::List with the appropriate fields or empty struct if
none) so the arm invokes List::call().await rather than Use::call().await;
update the match arm that currently references commands::use_cmd::Use to
reference commands::list_cmd::List.
---
Nitpick comments:
In `@tests/cli.rs`:
- Around line 128-140: The test test_use_silent_skips_missing_version currently
only asserts success; update it to also assert that both stdout and stderr are
empty by chaining .stdout(predicate::str::is_empty()) and
.stderr(predicate::str::is_empty()) on cmd.assert(), and after running the
command verify that no env file was written in the temporary PVM_DIR (e.g.,
check temp_dir.path() has no new env file by asserting the directory has no
entries or that the specific env filename is absent) so the test enforces "no
output and no env file written".
🪄 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: CHILL
Plan: Pro
Run ID: 2851a3d0-2c9e-461f-9594-94cbbf92b88a
📒 Files selected for processing (6)
src/commands/init.rssrc/commands/use_cmd.rssrc/fs.rssrc/interactive.rssrc/shell.rstests/cli.rs
…u correctly Addresses CodeRabbit review on #24: - use_cmd: after the install prompt accepts and the install runs, fall through to the activation path with the freshly resolved version instead of returning early. Otherwise the user's shell was never switched and they had to run 'pvm use <ver>' a second time. - install: 'execute_install_with(version, prompt_activation)' returns the resolved version so callers can chain activation themselves; legacy 'execute_install' kept as a thin wrapper for the standalone 'pvm install' command. - interactive: menu index 3 ('List') now dispatches to ls::Ls; it was previously wired to use_cmd::Use, opening the version-switcher.
actions/create-github-app-token v3 deprecated the 'app-id' input in favor of 'client-id'. The new input still accepts an App ID value, so the existing BOT_APP_ID secret continues to work unchanged. This silences the eight deprecation warnings emitted on every workflow run.
Strengthen test_use_silent_skips_missing_version per CodeRabbit feedback: with PVM_ENV_UPDATE_PATH pointed at a known path inside the tempdir, assert stdout/stderr are empty and that the env file was never created when the requested version is absent.
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
webproject-bot Bot
pushed a commit
that referenced
this pull request
May 8, 2026
## [1.2.1](v1.2.0...v1.2.1) (2026-05-08) ### Bug Fixes * **use:** prompt to install when target version missing ([#24](#24)) ([ceec711](ceec711))
|
🎉 This PR is included in version 1.2.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Fahl-Design
added a commit
that referenced
this pull request
May 8, 2026
Refactor the monolithic fpm-readme.sh into: tests/e2e/run.sh driver tests/e2e/_lib.sh shared helpers (run_under_expect, fcgi_call) tests/e2e/cases/NN_*.sh one file per case The driver runs each case in a fresh bash subprocess, so state from one case can no longer hide bugs in the next (the previous monolith silently passed because pvm's 24h .update_check_guard suppressed the patch-update prompt for any case after the first one to use it). Coverage now includes (Linux, ubuntu-latest): 01 real `pvm install` flow with interactive MultiSelect (expect) 02 pvm ls discovers [cli, fpm] 03 pvm use through shell wrapper switches PATH; which php-fpm 04 pvm current after pvm use 05 .php-version cd-hook auto-switch 06 pvm use <missing-version> → install prompt + decline (#24) 07 patch-update detection on pvm use <minor> 08 README php-fpm.conf + pool.d/{www,sock}.conf passes -t 09 fpm -F listens on TCP :9000 + unix /tmp/php-fpm-www.sock 10 FastCGI roundtrip over TCP returns FCGI_OK 11 FastCGI roundtrip over unix socket returns FCGI_OK 12 -c php.ini effective inside worker (memory_limit, expose_php) 13 pid file + error log written 14 SIGQUIT clean shutdown 15 pvm uninstall removes the version directory CI job in release.yml renamed to "e2e tests - linux", chained between the existing `tests` and `release` jobs. apt-get installs expect and libfcgi-bin (for cgi-fcgi) on top of netcat-openbsd. The suite targets PHP 8.5 by default (PVM_VERSION_MAJOR_MINOR=8.5), overridable per env.
6 tasks
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
Repro
Reported in 'docker run --rm -it ubuntu:24.04' after fresh install:
```
$ pvm init
✓ Wrote 8.3 to .php-version
✔ Do you want to run `pvm use 8.3` now? · yes
Error: PHP 8.3 is not installed locally.
```
After this PR the user is asked whether to install, then the existing install flow runs (which also handles the "use now" prompt).
Implementation
Test plan
Summary by CodeRabbit