Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
48 changes: 47 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,46 @@ 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`. A leading
/// `~/` is expanded against the home directory, since a variable set outside
/// a shell reaches us unexpanded.
fn codex_config_dir() -> Option<PathBuf> {
Comment thread
worktrunk-bot marked this conversation as resolved.
Resolved
if let Ok(dir) = std::env::var("CODEX_HOME")
&& !dir.is_empty()
{
if let Some(rest) = dir.strip_prefix("~/") {
return worktrunk::path::home_dir().map(|home| home.join(rest));
}
return Some(PathBuf::from(dir));
}
worktrunk::path::home_dir().map(|home| home.join(".codex"))
}

/// Whether the worktrunk marketplace is configured in Codex.
///
/// `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`.
pub(super) fn is_marketplace_configured() -> bool {
let Some(config_dir) = codex_config_dir() else {
return false;
};

let Ok(content) = std::fs::read_to_string(config_dir.join("config.toml")) else {
return false;
};

let Ok(config) = content.parse::<toml::Table>() else {
return false;
};

config
.get("marketplaces")
.and_then(|m| m.get(MARKETPLACE_NAME))
.is_some()
}
23 changes: 23 additions & 0 deletions src/commands/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,29 @@ 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.
///
/// 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() -> bool,
) -> anyhow::Result<()> {
match run_plugin_cli(program, args) {
Ok(()) => Ok(()),
Err(err) if still_configured() => Err(err),
Err(_) => Ok(()),
}
}

#[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
24 changes: 24 additions & 0 deletions src/commands/config/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,30 @@ pub(super) fn is_plugin_installed() -> bool {
.is_some()
}

/// Check if the worktrunk marketplace is configured in Claude Code
///
/// 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.
pub(super) fn is_marketplace_configured() -> bool {
Comment thread
max-sixty marked this conversation as resolved.
Resolved
let Some(config_dir) = claude_config_dir() else {
return false;
};

let marketplaces_file = config_dir.join("plugins/known_marketplaces.json");
let Ok(content) = std::fs::read_to_string(&marketplaces_file) else {
return false;
};

let Ok(json) = serde_json::from_str::<serde_json::Value>(&content) else {
return false;
};

json.get("worktrunk").is_some()
}

/// Whether Claude Code's statusline runs worktrunk's.
///
/// The question is which subcommand the configured command invokes, so it's
Expand Down
83 changes: 83 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,34 @@ impl TestRepo {
.unwrap();
}

/// Record the worktrunk marketplace as configured in Claude Code
///
/// `known_marketplaces.json` is keyed by marketplace name, which is what
/// `is_marketplace_configured` reads.
pub fn setup_claude_marketplace_configured(temp_home: &std::path::Path) {
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"),
r#"{"worktrunk":{"source":{"source":"github","repo":"max-sixty/worktrunk"}}}"#,
)
.unwrap();
}

/// Record the worktrunk marketplace as configured in Codex
///
/// Codex keeps each one as a `[marketplaces.<name>]` table in
/// `config.toml`.
pub fn setup_codex_marketplace_configured(temp_home: &std::path::Path) {
let codex_dir = temp_home.join(".codex");
std::fs::create_dir_all(&codex_dir).unwrap();
std::fs::write(
codex_dir.join("config.toml"),
"[marketplaces.worktrunk]\nsource_type = \"git\"\nsource = \"https://github.com/max-sixty/worktrunk.git\"\n",
)
.unwrap();
}

/// 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 +2393,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
72 changes: 72 additions & 0 deletions tests/integration_tests/config_show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4341,6 +4341,74 @@ fn test_plugins_codex_uninstall_plugin_remove_fails(mut repo: TestRepo, temp_hom
});
}

/// The marketplace removal's goal is that the marketplace is gone, so the
/// harness reporting it was never configured is that goal already met. Running
/// `uninstall` a second time hits exactly this, and used to fail with nothing
/// left to do.
#[rstest]
fn test_plugins_claude_uninstall_tolerates_absent_marketplace(
mut repo: TestRepo,
temp_home: TempDir,
) {
repo.setup_mock_ci_tools_unauthenticated();
repo.setup_mock_claude_with_marketplace_remove_failing();
TestRepo::setup_plugin_installed(temp_home.path());
// No `known_marketplaces.json`, so the marketplace the removal failed on
Comment thread
max-sixty marked this conversation as resolved.
Resolved
// is already gone.

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 Codex counterpart: a second `uninstall` succeeds.
#[rstest]
fn test_plugins_codex_uninstall_tolerates_absent_marketplace(
mut repo: TestRepo,
temp_home: TempDir,
) {
repo.setup_mock_ci_tools_unauthenticated();
repo.setup_mock_codex_with_marketplace_remove_failing();
// No `config.toml`, so codex has no marketplace left to remove.

let settings = setup_snapshot_settings_with_home(&repo, &temp_home);
settings.bind(|| {
let mut cmd = repo.wt_command();
cmd.args(["config", "plugins", "codex", "uninstall", "--yes"])
.current_dir(repo.root_path());
set_temp_home_env(&mut cmd, temp_home.path());

assert_cmd_snapshot!(cmd);
});
}

/// The Codex counterpart: a marketplace that survives the removal still errors.
#[rstest]
fn test_plugins_codex_uninstall_surfaces_marketplace_remove_failure(
mut repo: TestRepo,
temp_home: TempDir,
) {
repo.setup_mock_ci_tools_unauthenticated();
repo.setup_mock_codex_with_marketplace_remove_failing();
TestRepo::setup_codex_marketplace_configured(temp_home.path());

let settings = setup_snapshot_settings_with_home(&repo, &temp_home);
settings.bind(|| {
let mut cmd = repo.wt_command();
cmd.args(["config", "plugins", "codex", "uninstall", "--yes"])
.current_dir(repo.root_path());
set_temp_home_env(&mut cmd, temp_home.path());

assert_cmd_snapshot!(cmd);
});
}

#[test]
fn test_codex_plugin_metadata_is_valid_json() {
let project_root = std::path::Path::new(env!("CARGO_MANIFEST_DIR"));
Expand Down Expand Up @@ -5846,6 +5914,10 @@ fn test_plugins_claude_uninstall_second_step_fails(mut repo: TestRepo, temp_home
repo.setup_mock_ci_tools_unauthenticated();
repo.setup_mock_claude_installed();
TestRepo::setup_plugin_installed(temp_home.path());
// The marketplace is still configured after the removal fails, which is
// what makes this a genuine failure rather than the already-gone state
// `test_plugins_claude_uninstall_tolerates_absent_marketplace` covers.
TestRepo::setup_claude_marketplace_configured(temp_home.path());

// Plugin uninstall succeeds and only the marketplace removal that follows
// it fails, so the error the command surfaces can come from nothing else.
Expand Down
Loading
Loading