fix(config): let uninstall succeed when the marketplace is already gone

[max-sixty]

wt config plugins codex uninstall fails when run twice. #4019 made both harnesses' uninstall remove the marketplace after the plugin, and plugin marketplace remove exits non-zero when the marketplace is not configured — which is the state uninstall is trying to reach. So the second run reports a failure having nothing left to do. The Claude side has the same shape and is only masked by its is_plugin_installed early return, which returns before the marketplace step.

Measured against the real CLIs, each in a throwaway config dir:

command	target absent	exit
claude plugin marketplace remove worktrunk	Marketplace 'worktrunk' not found	1
claude plugin uninstall worktrunk@worktrunk	not found in installed plugins	1
codex plugin marketplace remove worktrunk	is not configured or installed	1
codex plugin remove worktrunk@worktrunk	reports removed	0

codex plugin remove tolerating an absent plugin is what #4019's "Superseded during review" note established; the marketplace removal beside it does not, which is what this fixes.

run_plugin_removal runs the removal and, when it fails, asks the harness's own config whether the marketplace is still there. The answer is three-way. A config the harness has never written records no marketplaces, so its absence is a confident Some(false) and the command succeeds. A marketplace still present is Some(true), a genuine failure, and the harness's stderr surfaces in the gutter exactly as before. A config that exists and will not read or parse is None, and the error stands there too: a reader that cannot see the marketplace has not established that the removal worked.

That third answer is what keeps the fix from failing open, and it has to rest on the file's shape rather than on parsing alone. A key lookup answers "absent" for any JSON that simply lacks the key, so known_marketplaces.json reshaped the way the installed_plugins.json beside it wraps its map in a version key would parse, miss, and report a confident no — every genuine failure silently reported as success for as long as the shape held. The Claude reader therefore requires every value to be a marketplace object; the Codex reader rules out the one shape available to it, a marketplaces that is not a table, and claims nothing more, since a fresh config.toml legitimately has no such key. Both ask try_exists rather than exists, which reports false for a directory it cannot stat.

The removal runs in every case, so the ? preview stays an accurate list of the commands the uninstall spawns, which test_plugins_claude_prompt_previews_commands pins.

is_marketplace_configured reads known_marketplaces.json for Claude, beside the installed_plugins.json that is_plugin_installed already reads, and Codex's [marketplaces.<name>] table in config.toml.

Tests, and one existing test whose setup changed

Each harness gets five cases, differing only in what the config holds: no config file, a config holding a different marketplace, a config holding worktrunk, a config that will not parse, and a config reshaped so it parses but no longer means what the reader expects. The middle one is the state a second uninstall actually lands in — Codex keeps its model and other settings in config.toml, and a Claude user with any other marketplace still has a known_marketplaces.json — so it, not the missing-file case, is the branch this fix exists for. Mock helpers fail only the marketplace removal, so the cases differ in config state alone, and the fixtures take the file body.

test_plugins_claude_uninstall_second_step_fails needed a configured marketplace added to its setup. Without one it had become the already-absent case, so it would have asserted success while its doc comment says the error surfaces. With the fixture its snapshot is unchanged from before this branch.

Tests drop an ambient CODEX_HOME rather than pinning one, so the Codex config lookup stays inside the temp home the way CLAUDE_CONFIG_DIR already does. Removing it keeps the variable out of every snapshot's env block, where pinning it would have churned each one and needed a new redaction. A separate test then points CODEX_HOME at a directory holding the only copy of the marketplace record, so the lookup has to follow the variable to find it; it asserts on the exit status rather than a snapshot, for that same reason.

The lookups carry no unreachable arms. codex_config_dir does not expand a leading ~/, which was mirrored from claude_config_dir — where the comment explains a literal ~ only arrives when the variable is set outside a shell — and nothing sets Codex's that way here. Both readers fold their early returns into the paths the tests drive: resolving the config dir and reading the file are one and_then, so the missing-file case covers the else arm, and a malformed file falls out of is_ok_and rather than a let ... else of its own.

Local gate: cargo fmt --check, cargo clippy --all-targets --all-features, cargo test --lib --bins, cargo test --test integration (2090 passed), and pre-commit run --all-files.

This was written by Claude Code on behalf of max-sixty

🤖 Generated with Claude Code

[worktrunk-bot]

The two-step shape — run the removal, then ask the harness's own config whether the goal state was reached — is the right call, and it's a real improvement over a blanket best-effort. Three things I'd want resolved before this lands.

codecov/patch is red at 90.63% (9 of 96 patch lines missed), and six of those are one branch nothing can reach. The misses are src/commands/config/codex.rs 113 and 115–118 (the whole CODEX_HOME arm of codex_config_dir), 131 and 139 (the no-home and TOML-parse arms), and src/commands/config/show.rs 251 and 260 (the no-home and JSON-parse arms). The CODEX_HOME block is the interesting one: set_temp_home_env now env_removes CODEX_HOME, so by construction no integration test can enter that branch. The PR body argues for removing over pinning on snapshot-churn grounds, which is fair for the existing tests — but it also means the newly added env-var support ships with nothing exercising it. One test that sets CODEX_HOME to a relocated dir (and one that sets it to a ~/-prefixed value) would both close most of the gap and be the only check that the branch works at all. Per CLAUDE.md → Coverage, this gates the merge either way.

The predicate answers "is it configured?" with false for both "definitely not" and "I couldn't tell", and run_plugin_removal turns the second into a reported success. Detail inline on show.rs.

Smaller: codex_config_dir is a near-verbatim copy of claude_config_dir (env var, empty-check, ~/ expansion, home-joined default), differing only in the two names — and, less visibly, in which home_dir it calls. claude_config_dir uses show::home_dir, which prefers HOME then USERPROFILE; the new copy uses worktrunk::path::home_dir. On Windows with a Git-Bash-style HOME that differs from %USERPROFILE% those resolve differently. I think the new one is actually the better match for Codex, which resolves its own home the same way — but that makes it worth stating rather than leaving as an artifact of the copy. A shared config_dir_from_env(var, default_subdir) would collapse both and keep the two harnesses from drifting again.

[worktrunk-bot]

The CODEX_HOME gap is closed properly: test_plugins_codex_uninstall_reads_codex_home records the marketplace under the relocated root and nowhere else, so a lookup that ignored the variable would read the failed removal as "already gone" and exit 0 — the assertion discriminates in the right direction, and dropping the ~/ expansion removes surface that had no caller rather than papering over it.

One thing the commit changed that the code doesn't say: codex_config_dir's docstring still claims parity with claude_config_dir, which this commit is exactly what broke. Inline.

[worktrunk-bot]

The Option<bool> shape is the right answer to the last round, and the four-state-per-harness test matrix pins it properly. One thing the new code doesn't actually do, though, is the thing its docstring says it does — inline on show.rs.

Separately, on whether the Claude half reaches the case the PR is named for. handle_claude_uninstall returns early on !is_plugin_installed(), so a genuine second wt config plugins claude uninstall prints Plugin not installed and never gets to the marketplace step; what the new tolerance covers on Claude is the narrower state where the plugin is still recorded but the marketplace was removed by hand. That leaves the partial-failure path with no way out: claude plugin uninstall succeeds, the marketplace removal fails for a transient reason, wt exits 1 — and every re-run now short-circuits at the early return, so the marketplace stays configured and no wt command will remove it. The user's recourse is claude plugin marketplace remove worktrunk by hand. Pre-existing and outside this diff, so not something I'd hold the PR on, but it's the state this fix's own failure path drops users into, and gating the early return on plugin-or-marketplace rather than plugin alone would close it.