mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
fix(config): reach the marketplace when the plugin is already gone (#4034)
`wt config plugins claude uninstall` cannot finish a half-done uninstall. It returns early on `!is_plugin_installed()`, so once the plugin is gone the command stops there — and the state where the plugin is gone but the marketplace is not is exactly what a first run leaves behind when the plugin uninstall succeeds and the marketplace removal then fails. Re-running prints `Plugin not installed` and exits 0 with the marketplace still configured, and nothing in `wt` will remove it. The early return now requires both halves to be gone, and takes only a confident `Some(false)` for the marketplace, matching what `run_plugin_removal` is willing to trust. The plugin step becomes conditional, and the `?` preview lists it only when it will run, so the preview stays equal to the set of commands the uninstall spawns — the property `test_plugins_claude_prompt_previews_commands` pins. The marketplace removal still runs unconditionally. Gating it on the config reading `Some(false)` was the first thing I tried, and it is wrong: it makes `run_plugin_removal`'s tolerance unreachable in the cases that exercise it, so `test_plugins_claude_uninstall_tolerates_absent_marketplace` and its siblings would pass while never attempting the removal they exist to tolerate. Leaving the removal unconditional keeps that tolerance the thing that decides whether an absent marketplace is a failure. `test_plugins_claude_uninstall_removes_marketplace_left_without_plugin` covers the recovered state: no `installed_plugins.json`, a `known_marketplaces.json` holding worktrunk, and a run that skips the plugin step and removes the marketplace. Every other uninstall snapshot is unchanged, which is what confirms this reaches only the state that was previously unreachable. `wt config plugins claude uninstall --help` promised the opposite of the new behavior — "Skips gracefully if the plugin is not installed, leaving the marketplace in place" is precisely the state the command no longer skips — so it now states the real condition. Nothing caught that: the string is in no generated mirror and no snapshot, so `test_docs_are_in_sync` and the help snapshots both stayed green while the sentence described behavior that was gone. Both uninstall pages are `test_help` cases now, since what each says about skipping depends on what the harness's config still holds. The Codex uninstall has no equivalent stranded state, which is measurable rather than a judgment call: `codex plugin remove` exits 0 on an absent plugin, so its first step never blocks the re-run from reaching the marketplace step. `claude plugin uninstall` exits 1 there, which is what made the `is_plugin_installed` guard necessary on that side and gave it a state to strand. Raised as a non-blocking observation on #4033. It is pre-existing rather than introduced there, but it is in the function that PR was editing. > _This was written by Claude Code on behalf of max-sixty_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Maximilian Roos
+4
-2
@@ -442,12 +442,14 @@ Requires `claude` CLI. Skips gracefully if already installed."#
|
||||
|
||||
/// Remove the Worktrunk plugin
|
||||
#[command(
|
||||
after_long_help = r#"Uninstalls the Worktrunk plugin from Claude Code and removes its marketplace. Skips gracefully if the plugin is not installed, leaving the marketplace in place. Equivalent to:
|
||||
after_long_help = r#"Uninstalls the Worktrunk plugin from Claude Code and removes its marketplace. Equivalent to:
|
||||
|
||||
```console
|
||||
$ claude plugin uninstall worktrunk@worktrunk
|
||||
$ claude plugin marketplace remove worktrunk
|
||||
```"#
|
||||
```
|
||||
|
||||
Requires `claude` CLI. Skips gracefully only when both are already gone. An uninstall that removed the plugin and then failed leaves the marketplace behind; running it again removes that half."#
|
||||
)]
|
||||
Uninstall,
|
||||
|
||||
|
||||
@@ -52,17 +52,36 @@ pub fn handle_claude_install(yes: bool) -> anyhow::Result<()> {
|
||||
pub fn handle_claude_uninstall(yes: bool) -> anyhow::Result<()> {
|
||||
require_claude_cli()?;
|
||||
|
||||
if !is_plugin_installed() {
|
||||
// The marketplace can outlive the plugin: an uninstall that removed the
|
||||
// plugin and then failed on the marketplace leaves exactly that. Asking
|
||||
// only about the plugin would report "not installed" and exit 0 with the
|
||||
// marketplace still there and no way left to finish the job, so the early
|
||||
// return needs both halves gone. Only a confident `Some(false)` counts as
|
||||
// gone, for the reason `run_plugin_removal` gives.
|
||||
let plugin_installed = is_plugin_installed();
|
||||
if !plugin_installed && is_marketplace_configured() == Some(false) {
|
||||
eprintln!("{}", info_message("Plugin not installed"));
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// The marketplace removal always runs, so its own tolerance decides
|
||||
// whether an absent marketplace is a failure. Only the plugin step is
|
||||
// conditional, and the preview says so rather than naming a command that
|
||||
// will not run.
|
||||
let mut commands = Vec::new();
|
||||
if plugin_installed {
|
||||
commands.push("claude plugin uninstall worktrunk@worktrunk");
|
||||
}
|
||||
commands.push("claude plugin marketplace remove worktrunk");
|
||||
|
||||
if !yes {
|
||||
match prompt_yes_no_preview(
|
||||
&cformat!("Uninstall Worktrunk plugin from <bold>Claude Code</>?"),
|
||||
|| {
|
||||
let commands = "claude plugin uninstall worktrunk@worktrunk\nclaude plugin marketplace remove worktrunk";
|
||||
eprintln!("{}", worktrunk::styling::format_bash_with_gutter(commands));
|
||||
eprintln!(
|
||||
"{}",
|
||||
worktrunk::styling::format_bash_with_gutter(&commands.join("\n"))
|
||||
);
|
||||
},
|
||||
)? {
|
||||
PromptResponse::Accepted => {}
|
||||
@@ -70,8 +89,10 @@ pub fn handle_claude_uninstall(yes: bool) -> anyhow::Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
eprintln!("{}", progress_message("Uninstalling plugin..."));
|
||||
super::run_plugin_cli("claude", &["plugin", "uninstall", "worktrunk@worktrunk"])?;
|
||||
if plugin_installed {
|
||||
eprintln!("{}", progress_message("Uninstalling plugin..."));
|
||||
super::run_plugin_cli("claude", &["plugin", "uninstall", "worktrunk@worktrunk"])?;
|
||||
}
|
||||
|
||||
eprintln!(
|
||||
"{}",
|
||||
|
||||
@@ -4367,6 +4367,35 @@ fn test_plugins_claude_uninstall_tolerates_absent_marketplace(
|
||||
});
|
||||
}
|
||||
|
||||
/// A first `uninstall` that removed the plugin and then failed on the
|
||||
/// marketplace leaves the marketplace behind. Re-running has to finish that
|
||||
/// job rather than reporting "Plugin not installed" and exiting 0 with the
|
||||
/// marketplace still configured.
|
||||
#[rstest]
|
||||
fn test_plugins_claude_uninstall_removes_marketplace_left_without_plugin(
|
||||
mut repo: TestRepo,
|
||||
temp_home: TempDir,
|
||||
) {
|
||||
repo.setup_mock_ci_tools_unauthenticated();
|
||||
repo.setup_mock_claude_with_plugins();
|
||||
// No `installed_plugins.json`: the plugin is already gone, and only the
|
||||
// marketplace is left to remove.
|
||||
TestRepo::setup_claude_marketplaces(
|
||||
temp_home.path(),
|
||||
TestRepo::CLAUDE_MARKETPLACES_WITH_WORKTRUNK,
|
||||
);
|
||||
|
||||
let settings = setup_snapshot_settings_with_home(&repo, &temp_home);
|
||||
settings.bind(|| {
|
||||
let mut cmd = repo.wt_command();
|
||||
cmd.args(["config", "plugins", "claude", "uninstall", "--yes"])
|
||||
.current_dir(repo.root_path());
|
||||
set_temp_home_env(&mut cmd, temp_home.path());
|
||||
|
||||
assert_cmd_snapshot!(cmd);
|
||||
});
|
||||
}
|
||||
|
||||
/// The Claude counterpart of the key-absent case: `known_marketplaces.json`
|
||||
/// exists because the user has other marketplaces, and worktrunk's entry is
|
||||
/// gone.
|
||||
|
||||
@@ -110,6 +110,18 @@ fn test_merge_help_describes_exact_shape_no_rebase() {
|
||||
"help_config_plugins_codex_install",
|
||||
"config plugins codex install --help"
|
||||
)]
|
||||
// The uninstall pages describe when each command skips, which depends on what
|
||||
// the harness's config still holds. Snapshot them so that text cannot drift
|
||||
// away from the handlers the way it did when the marketplace removal became
|
||||
// reachable with the plugin already gone.
|
||||
#[case(
|
||||
"help_config_plugins_claude_uninstall",
|
||||
"config plugins claude uninstall --help"
|
||||
)]
|
||||
#[case(
|
||||
"help_config_plugins_codex_uninstall",
|
||||
"config plugins codex uninstall --help"
|
||||
)]
|
||||
#[case("help_config_state", "config state --help")]
|
||||
#[case("help_config_state_cache", "config state cache --help")]
|
||||
#[case(
|
||||
|
||||
+66
@@ -0,0 +1,66 @@
|
||||
---
|
||||
source: tests/integration_tests/config_show.rs
|
||||
info:
|
||||
program: wt
|
||||
args:
|
||||
- config
|
||||
- plugins
|
||||
- claude
|
||||
- uninstall
|
||||
- "--yes"
|
||||
env:
|
||||
APPDATA: "[TEST_CONFIG_HOME]"
|
||||
CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]"
|
||||
CLICOLOR_FORCE: "1"
|
||||
COLUMNS: "500"
|
||||
GIT_ALLOW_PROTOCOL: file
|
||||
GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z"
|
||||
GIT_AUTHOR_EMAIL: test@example.com
|
||||
GIT_AUTHOR_NAME: Test User
|
||||
GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z"
|
||||
GIT_COMMITTER_EMAIL: test@example.com
|
||||
GIT_COMMITTER_NAME: Test User
|
||||
GIT_CONFIG_COUNT: "2"
|
||||
GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig
|
||||
GIT_CONFIG_KEY_0: user.useConfigOnly
|
||||
GIT_CONFIG_KEY_1: rerere.enabled
|
||||
GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig
|
||||
GIT_CONFIG_VALUE_0: "true"
|
||||
GIT_CONFIG_VALUE_1: "false"
|
||||
GIT_TERMINAL_PROMPT: "0"
|
||||
HOME: "[TEST_HOME]"
|
||||
LANG: C
|
||||
LC_ALL: C
|
||||
LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]"
|
||||
OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]"
|
||||
PATH: "[PATH]"
|
||||
TERM: alacritty
|
||||
USERPROFILE: "[TEST_HOME]"
|
||||
WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]"
|
||||
WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]"
|
||||
WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]"
|
||||
WORKTRUNK_TEST_BASH_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_CLAUDE_INSTALLED: "1"
|
||||
WORKTRUNK_TEST_CODEX_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1"
|
||||
WORKTRUNK_TEST_EPOCH: "1735776000"
|
||||
WORKTRUNK_TEST_FISH_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_GEMINI_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]"
|
||||
WORKTRUNK_TEST_NUSHELL_ENV: "0"
|
||||
WORKTRUNK_TEST_OPENCODE_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_PARENT_SHELL: ""
|
||||
WORKTRUNK_TEST_POWERSHELL_ENV: "0"
|
||||
WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000"
|
||||
WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1"
|
||||
WORKTRUNK_TEST_ZSH_INSTALLED: "0"
|
||||
XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]"
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
|
||||
----- stderr -----
|
||||
[36m◎[39m [36mRemoving Claude Code plugin marketplace...[39m
|
||||
[32m✓[39m [32mPlugin & marketplace removed[39m
|
||||
+79
@@ -0,0 +1,79 @@
|
||||
---
|
||||
source: tests/integration_tests/help.rs
|
||||
info:
|
||||
program: wt
|
||||
args:
|
||||
- config
|
||||
- plugins
|
||||
- claude
|
||||
- uninstall
|
||||
- "--help"
|
||||
env:
|
||||
CLICOLOR_FORCE: "1"
|
||||
COLUMNS: "500"
|
||||
GIT_ALLOW_PROTOCOL: file
|
||||
GIT_CONFIG_COUNT: "2"
|
||||
GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig
|
||||
GIT_CONFIG_KEY_0: user.useConfigOnly
|
||||
GIT_CONFIG_KEY_1: rerere.enabled
|
||||
GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig
|
||||
GIT_CONFIG_VALUE_0: "true"
|
||||
GIT_CONFIG_VALUE_1: "false"
|
||||
LANG: C
|
||||
LC_ALL: C
|
||||
LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]"
|
||||
TERM: alacritty
|
||||
WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]"
|
||||
WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]"
|
||||
WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]"
|
||||
WORKTRUNK_TEST_BASH_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_CLAUDE_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_CODEX_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1"
|
||||
WORKTRUNK_TEST_EPOCH: "1735776000"
|
||||
WORKTRUNK_TEST_FISH_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_GEMINI_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_NUSHELL_ENV: "0"
|
||||
WORKTRUNK_TEST_OPENCODE_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_PARENT_SHELL: ""
|
||||
WORKTRUNK_TEST_POWERSHELL_ENV: "0"
|
||||
WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000"
|
||||
WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1"
|
||||
WORKTRUNK_TEST_ZSH_INSTALLED: "0"
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
wt config plugins claude uninstall - Remove the Worktrunk plugin
|
||||
|
||||
Usage: [1m[36mwt config plugins claude uninstall[0m [36m[OPTIONS][0m
|
||||
|
||||
[1m[32mOptions:[0m
|
||||
[1m[36m-h[0m, [1m[36m--help[0m
|
||||
Print help (see a summary with '-h')
|
||||
|
||||
[1m[32mGlobal Options:[0m
|
||||
[1m[36m-C[0m[36m [0m[36m<path>[0m
|
||||
Working directory for this command
|
||||
|
||||
[1m[36m--config[0m[36m [0m[36m<path>[0m
|
||||
User config file path
|
||||
|
||||
[1m[36m--config-set[0m[36m [0m[36m<toml>[0m
|
||||
Override config with inline TOML, e.g. --config-set list.full=true (repeatable)
|
||||
|
||||
[1m[36m-v[0m, [1m[36m--verbose[0m[36m...[0m
|
||||
Verbose output (-v: info logs + hook/alias template variables on stderr; -vv: also debug logs and raw subprocess output written to .git/wt/logs/). Set WORKTRUNK_VERBOSE=0|1|2 to apply the same level everywhere — including shell completion, which no flag can reach
|
||||
|
||||
[1m[36m-y[0m, [1m[36m--yes[0m
|
||||
Skip approval prompts
|
||||
|
||||
Uninstalls the Worktrunk plugin from Claude Code and removes its marketplace. Equivalent to:
|
||||
|
||||
[107m [0m [2m[0m[2m[34mclaude[0m[2m plugin uninstall worktrunk@worktrunk[0m
|
||||
[107m [0m [2m[0m[2m[34mclaude[0m[2m plugin marketplace remove worktrunk[0m
|
||||
|
||||
Requires [2mclaude[0m CLI. Skips gracefully only when both are already gone. An uninstall that removed the plugin and then failed leaves the marketplace behind; running it again removes that half.
|
||||
|
||||
----- stderr -----
|
||||
+79
@@ -0,0 +1,79 @@
|
||||
---
|
||||
source: tests/integration_tests/help.rs
|
||||
info:
|
||||
program: wt
|
||||
args:
|
||||
- config
|
||||
- plugins
|
||||
- codex
|
||||
- uninstall
|
||||
- "--help"
|
||||
env:
|
||||
CLICOLOR_FORCE: "1"
|
||||
COLUMNS: "500"
|
||||
GIT_ALLOW_PROTOCOL: file
|
||||
GIT_CONFIG_COUNT: "2"
|
||||
GIT_CONFIG_GLOBAL: /nonexistent/wt/gitconfig
|
||||
GIT_CONFIG_KEY_0: user.useConfigOnly
|
||||
GIT_CONFIG_KEY_1: rerere.enabled
|
||||
GIT_CONFIG_SYSTEM: /nonexistent/wt/gitconfig
|
||||
GIT_CONFIG_VALUE_0: "true"
|
||||
GIT_CONFIG_VALUE_1: "false"
|
||||
LANG: C
|
||||
LC_ALL: C
|
||||
LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]"
|
||||
TERM: alacritty
|
||||
WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]"
|
||||
WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]"
|
||||
WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]"
|
||||
WORKTRUNK_TEST_BASH_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_CLAUDE_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_CODEX_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1"
|
||||
WORKTRUNK_TEST_EPOCH: "1735776000"
|
||||
WORKTRUNK_TEST_FISH_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_GEMINI_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_NUSHELL_ENV: "0"
|
||||
WORKTRUNK_TEST_OPENCODE_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_PARENT_SHELL: ""
|
||||
WORKTRUNK_TEST_POWERSHELL_ENV: "0"
|
||||
WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0"
|
||||
WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000"
|
||||
WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1"
|
||||
WORKTRUNK_TEST_ZSH_INSTALLED: "0"
|
||||
---
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
wt config plugins codex uninstall - Remove the Worktrunk plugin
|
||||
|
||||
Usage: [1m[36mwt config plugins codex uninstall[0m [36m[OPTIONS][0m
|
||||
|
||||
[1m[32mOptions:[0m
|
||||
[1m[36m-h[0m, [1m[36m--help[0m
|
||||
Print help (see a summary with '-h')
|
||||
|
||||
[1m[32mGlobal Options:[0m
|
||||
[1m[36m-C[0m[36m [0m[36m<path>[0m
|
||||
Working directory for this command
|
||||
|
||||
[1m[36m--config[0m[36m [0m[36m<path>[0m
|
||||
User config file path
|
||||
|
||||
[1m[36m--config-set[0m[36m [0m[36m<toml>[0m
|
||||
Override config with inline TOML, e.g. --config-set list.full=true (repeatable)
|
||||
|
||||
[1m[36m-v[0m, [1m[36m--verbose[0m[36m...[0m
|
||||
Verbose output (-v: info logs + hook/alias template variables on stderr; -vv: also debug logs and raw subprocess output written to .git/wt/logs/). Set WORKTRUNK_VERBOSE=0|1|2 to apply the same level everywhere — including shell completion, which no flag can reach
|
||||
|
||||
[1m[36m-y[0m, [1m[36m--yes[0m
|
||||
Skip approval prompts
|
||||
|
||||
Uninstalls the Worktrunk plugin from Codex and removes its marketplace. Equivalent to:
|
||||
|
||||
[107m [0m [2m[0m[2m[34mcodex[0m[2m plugin remove worktrunk@worktrunk[0m
|
||||
[107m [0m [2m[0m[2m[34mcodex[0m[2m plugin marketplace remove worktrunk[0m
|
||||
|
||||
Requires [2mcodex[0m CLI.
|
||||
|
||||
----- stderr -----
|
||||
Reference in New Issue
Block a user