Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion src/commands/config/codex.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Codex plugin and marketplace management.

use std::path::PathBuf;

use anyhow::{Result, bail};
use color_print::cformat;
use worktrunk::styling::{eprintln, hint_message, progress_message, success_message};
Expand Down Expand Up @@ -81,9 +83,10 @@ pub fn handle_codex_uninstall(yes: bool) -> Result<()> {
"{}",
progress_message("Removing Codex plugin marketplace...")
);
super::run_plugin_cli(
super::run_plugin_removal(
"codex",
&["plugin", "marketplace", "remove", MARKETPLACE_NAME],
is_marketplace_configured,
)?;

eprintln!("{}", success_message("Codex plugin & marketplace removed"));
Expand All @@ -98,3 +101,51 @@ fn require_codex_cli() -> Result<()> {

bail!("codex CLI not found. Install Codex first: https://developers.openai.com/codex/cli/");
}

/// Codex's config root.
///
/// Honors `CODEX_HOME`, which relocates the whole tree away from `~/.codex`,
/// the same way `claude_config_dir` honors `CLAUDE_CONFIG_DIR`. They diverge
/// on one point deliberately: a leading `~/` is taken literally here rather
/// than expanded against the home directory, because a shell expands it before
/// the variable is set and nothing sets `CODEX_HOME` from a non-shell context.
fn codex_config_dir() -> Option<PathBuf> {
Comment thread
worktrunk-bot marked this conversation as resolved.
match std::env::var("CODEX_HOME") {
Ok(dir) if !dir.is_empty() => Some(PathBuf::from(dir)),
_ => worktrunk::path::home_dir().map(|home| home.join(".codex")),
}
}

/// Whether the worktrunk marketplace is configured in Codex, or `None` where
/// the config cannot answer.
///
/// `codex plugin marketplace remove` exits non-zero when the marketplace is
/// not configured, so uninstall needs to tell that from a removal that
/// genuinely failed. Codex records each one as a `[marketplaces.<name>]`
/// table in `config.toml`.
///
/// `None` is the same "cannot tell" the Claude reader returns: a `config.toml`
/// that will not read or parse, or whose `marketplaces` is not a table, cannot
/// say worktrunk's entry is gone.
///
/// It claims less than the Claude reader does, because less is available here.
/// A fresh `config.toml` legitimately has no `marketplaces` key at all, so a
/// key that was renamed or relocated is indistinguishable from a user who has
/// configured no marketplaces, and reads as a confident absence.
pub(super) fn is_marketplace_configured() -> Option<bool> {
let path = codex_config_dir()?.join("config.toml");
// `try_exists` so a directory we lack permission to stat is unknown rather
// than the `false` that `exists` reports for it.
if !path.try_exists().ok()? {
return Some(false);
}

let content = std::fs::read_to_string(&path).ok()?;
let config = content.parse::<toml::Table>().ok()?;
match config.get("marketplaces") {
None => Some(false),
Some(marketplaces) => marketplaces
.as_table()
.map(|table| table.contains_key(MARKETPLACE_NAME)),
}
}
32 changes: 32 additions & 0 deletions src/commands/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,38 @@ fn run_plugin_cli(program: &str, args: &[&str]) -> anyhow::Result<()> {
Ok(())
}

/// Run a removal whose goal is that the target is gone.
///
/// Both harnesses' `plugin marketplace remove` exits non-zero when the
/// marketplace is not configured, which is the state the removal is trying to
/// reach, so running `uninstall` twice would otherwise fail with nothing left
/// to do. Asking `still_configured` after the attempt tells that apart from a
/// removal that genuinely failed, which still surfaces the harness's own
/// stderr in the gutter.
///
/// Only a confident `Some(false)` drops the error. `None` — no home directory,
/// or a config that will not read, parse, or match the shape the reader knows
/// — keeps it, because a reader that cannot see the marketplace has not
/// established that the removal worked. Reporting success there would fail open
/// on exactly the silent case: a config whose shape changed under us would
/// swallow every genuine failure and print `Plugin & marketplace removed`.
///
/// The command runs either way, so the `?` preview lists what the uninstall
/// actually invokes.
fn run_plugin_removal(
program: &str,
args: &[&str],
still_configured: impl Fn() -> Option<bool>,
) -> anyhow::Result<()> {
match run_plugin_cli(program, args) {
Ok(()) => Ok(()),
Err(err) => match still_configured() {
Some(false) => Ok(()),
_ => Err(err),
},
}
}

#[cfg(test)]
mod tests {
use insta::assert_snapshot;
Expand Down
9 changes: 7 additions & 2 deletions src/commands/config/plugins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use color_print::cformat;
use worktrunk::styling::{eprintln, info_message, progress_message, success_message};

use super::show::{
claude_config_dir, is_claude_available, is_plugin_installed, is_statusline_configured,
claude_config_dir, is_claude_available, is_marketplace_configured, is_plugin_installed,
is_statusline_configured,
};
use crate::output::prompt::{PromptResponse, prompt_yes_no_preview};

Expand Down Expand Up @@ -76,7 +77,11 @@ pub fn handle_claude_uninstall(yes: bool) -> anyhow::Result<()> {
"{}",
progress_message("Removing Claude Code plugin marketplace...")
);
super::run_plugin_cli("claude", &["plugin", "marketplace", "remove", "worktrunk"])?;
super::run_plugin_removal(
"claude",
&["plugin", "marketplace", "remove", "worktrunk"],
is_marketplace_configured,
)?;

eprintln!("{}", success_message("Plugin & marketplace removed"));

Expand Down
37 changes: 37 additions & 0 deletions src/commands/config/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,43 @@ pub(super) fn is_plugin_installed() -> bool {
.is_some()
}

/// Whether the worktrunk marketplace is configured in Claude Code, or `None`
/// where the config cannot answer
///
/// Read from the same config tree as `is_plugin_installed`, and asked for the
/// same reason: `claude plugin marketplace remove` exits non-zero when the
/// marketplace is not there, so uninstall needs to tell that from a removal
/// that genuinely failed. `known_marketplaces.json` is keyed by marketplace
/// name.
///
/// The three-way answer is what keeps that safe. A file Claude Code has never
/// written records no marketplaces, so its absence is a confident no. A file
/// that exists and cannot be read, parsed, or recognized leaves the question
/// open, and `run_plugin_removal` keeps the harness's error rather than
/// reporting a removal it cannot confirm.
///
/// Recognizing the shape is the part that earns its keep. A key lookup alone
/// answers "absent" for any JSON that simply lacks it, so a file 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. Requiring every value to be a marketplace
/// object turns that reshape into `None` instead.
pub(super) fn is_marketplace_configured() -> Option<bool> {
let path = claude_config_dir()?.join("plugins/known_marketplaces.json");
// `try_exists` so a directory we lack permission to stat is unknown rather
// than the `false` that `exists` reports for it.
if !path.try_exists().ok()? {
return Some(false);
}

let content = std::fs::read_to_string(&path).ok()?;
let json = serde_json::from_str::<serde_json::Value>(&content).ok()?;
let map = json.as_object()?;
map.values()
.all(serde_json::Value::is_object)
.then(|| map.contains_key("worktrunk"))
}

/// Whether Claude Code's statusline runs worktrunk's.
///
/// The question is which subcommand the configured command invokes, so it's
Expand Down
99 changes: 99 additions & 0 deletions src/testing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,11 @@ pub fn set_temp_home_env(cmd: &mut Command, home: &Path) {
"PI_PROFILE",
"PI_CONFIG_DIR",
"PI_CODING_AGENT_DIR",
// Codex resolves its config root from CODEX_HOME, falling back to
// `$HOME/.codex` — which the temp home already pins. Dropping an
// ambient value is all hermeticity needs, and unlike setting one it
// keeps the variable out of every snapshot's env block.
"CODEX_HOME",
] {
cmd.env_remove(var);
}
Expand Down Expand Up @@ -2237,6 +2242,50 @@ impl TestRepo {
.unwrap();
}

/// Write Claude Code's marketplace record verbatim
///
/// `known_marketplaces.json` is keyed by marketplace name, which is what
/// `is_marketplace_configured` reads. The body is the caller's, since the
/// three states that reader distinguishes — worktrunk present, a different
/// marketplace present, and a file that will not parse — differ only in
/// what this file holds.
pub fn setup_claude_marketplaces(temp_home: &std::path::Path, body: &str) {
let plugins_dir = temp_home.join(".claude/plugins");
std::fs::create_dir_all(&plugins_dir).unwrap();
std::fs::write(plugins_dir.join("known_marketplaces.json"), body).unwrap();
}

/// `known_marketplaces.json` holding the worktrunk marketplace
pub const CLAUDE_MARKETPLACES_WITH_WORKTRUNK: &'static str =
r#"{"worktrunk":{"source":{"source":"github","repo":"max-sixty/worktrunk"}}}"#;

/// `known_marketplaces.json` holding some other marketplace
///
/// The state a second `uninstall` lands in: the file exists because the
/// user has other marketplaces, and worktrunk's entry is simply gone.
pub const CLAUDE_MARKETPLACES_WITHOUT_WORKTRUNK: &'static str =
r#"{"other":{"source":{"source":"github","repo":"someone/other"}}}"#;

/// Write Codex's `config.toml` verbatim
///
/// Codex keeps each marketplace as a `[marketplaces.<name>]` table there.
/// Takes the config root rather than the home directory, since
/// `CODEX_HOME` can move it off `~/.codex`.
pub fn setup_codex_config(codex_dir: &std::path::Path, body: &str) {
std::fs::create_dir_all(codex_dir).unwrap();
std::fs::write(codex_dir.join("config.toml"), body).unwrap();
}

/// `config.toml` holding the worktrunk marketplace
pub const CODEX_CONFIG_WITH_WORKTRUNK: &'static str = "[marketplaces.worktrunk]\nsource_type = \"git\"\nsource = \"https://github.com/max-sixty/worktrunk.git\"\n";

/// `config.toml` a Codex user has after the marketplace is removed
///
/// Codex keeps its model and other settings in this file, so it outlives
/// the marketplace table — which is why a second `uninstall` reads a
/// present config with no worktrunk entry rather than no config at all.
pub const CODEX_CONFIG_WITHOUT_WORKTRUNK: &'static str = "model = \"gpt-5.5\"\n\n[marketplaces.other]\nsource_type = \"git\"\nsource = \"https://github.com/someone/other.git\"\n";

/// Setup the statusline as configured in Claude Code settings
///
/// Creates the settings.json file with the wt statusline command.
Expand Down Expand Up @@ -2360,6 +2409,56 @@ impl TestRepo {
self.codex_installed = true;
}

/// Setup mock `claude` CLI whose only failing command is the marketplace
/// removal
///
/// Uninstall reads the config to tell "the marketplace was already gone"
/// from a removal that genuinely failed, so the two cases need a mock that
/// fails the removal and leaves every other step alone. Must call
/// `setup_mock_ci_tools_unauthenticated()` first.
pub fn setup_mock_claude_with_marketplace_remove_failing(&mut self) {
let mock_bin = self
.mock_bin_path
.as_ref()
.expect("call setup_mock_ci_tools_unauthenticated() first");

MockConfig::new("claude")
.command("plugin marketplace add", MockResponse::exit(0))
.command(
"plugin marketplace remove",
MockResponse::exit(1).with_stderr("error: marketplace remove failed\n"),
)
.command("plugin install", MockResponse::exit(0))
.command("plugin uninstall", MockResponse::exit(0))
.write(mock_bin);

self.claude_installed = true;
}

/// Setup mock `codex` CLI whose only failing command is the marketplace
/// removal
///
/// The Codex counterpart of
/// `setup_mock_claude_with_marketplace_remove_failing`.
pub fn setup_mock_codex_with_marketplace_remove_failing(&mut self) {
let mock_bin = self
.mock_bin_path
.as_ref()
.expect("call setup_mock_ci_tools_unauthenticated() first");

MockConfig::new("codex")
.command("plugin marketplace add", MockResponse::exit(0))
.command(
"plugin marketplace remove",
MockResponse::exit(1).with_stderr("error: marketplace remove failed\n"),
)
.command("plugin add", MockResponse::exit(0))
.command("plugin remove", MockResponse::exit(0))
.write(mock_bin);

self.codex_installed = true;
}

/// Setup mock `codex` CLI whose marketplace commands succeed and whose
/// plugin commands fail
///
Expand Down
Loading
Loading