Skip to content

Commit 70f6de8

Browse files
authored
Merge pull request #2822 from divakantpandey/fix/2722-install-managed-binary-path
fix(cli): use managed binary path for agent configs when externally managed (#2722)
2 parents 9faa3e5 + 4787012 commit 70f6de8

3 files changed

Lines changed: 117 additions & 20 deletions

File tree

‎src/cli/cli.c‎

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10988,7 +10988,20 @@ void cbm_cli_set_client_selection_for_testing(const char *spec) {
1098810988
/* Detect the running binary's path at runtime. The return value distinguishes
1098910989
* OS-reported identity (safe ownership evidence) from the ~/.local/bin fallback
1099010990
* used only as an install copy source. */
10991+
#ifdef CBM_CLI_ENABLE_TEST_API
10992+
static const char *g_cli_self_path_override_for_test = NULL;
10993+
void cbm_cli_set_self_path_override_for_test(const char *self_path) {
10994+
g_cli_self_path_override_for_test = self_path;
10995+
}
10996+
#endif
10997+
1099110998
static bool cbm_detect_self_path(char *buf, size_t buf_sz, const char *home) {
10999+
#ifdef CBM_CLI_ENABLE_TEST_API
11000+
if (g_cli_self_path_override_for_test) {
11001+
snprintf(buf, buf_sz, "%s", g_cli_self_path_override_for_test);
11002+
return true;
11003+
}
11004+
#endif
1099211005
buf[0] = '\0';
1099311006
bool exact = false;
1099411007
#ifdef _WIN32
@@ -11250,6 +11263,7 @@ char *cbm_build_install_plan_json(const char *home, const char *binary_path) {
1125011263

1125111264
typedef struct {
1125211265
const char *bin_target;
11266+
const char *config_binary;
1125311267
const char *bin_dir;
1125411268
const char *home;
1125511269
const char *shell_rc;
@@ -11274,6 +11288,8 @@ static int cli_install_activate(void *opaque) {
1127411288
!activation->shell_rc) {
1127511289
return CLI_TRUE;
1127611290
}
11291+
const char *config_binary =
11292+
activation->config_binary ? activation->config_binary : activation->bin_target;
1127711293
char previous_managed_binary[CLI_BUF_1K] = {0};
1127811294
bool previous_managed_binary_exact = cbm_detect_self_path(
1127911295
previous_managed_binary, sizeof(previous_managed_binary), activation->home);
@@ -11322,10 +11338,13 @@ static int cli_install_activate(void *opaque) {
1132211338
* including same-binary and non-force installs. */
1132311339
int agent_config_rc = CLI_OK;
1132411340
if (!activation->skip_config) {
11341+
const char *prev = previous_managed_binary_exact ? previous_managed_binary : NULL;
11342+
if (config_binary != activation->bin_target &&
11343+
(!prev || strcmp(prev, config_binary) == 0)) {
11344+
prev = activation->bin_target;
11345+
}
1132511346
agent_config_rc = cbm_install_agent_configs_with_previous(
11326-
activation->home, activation->bin_target,
11327-
previous_managed_binary_exact ? previous_managed_binary : NULL, activation->force,
11328-
activation->dry_run);
11347+
activation->home, config_binary, prev, activation->force, activation->dry_run);
1132911348
}
1133011349
if (agent_config_rc != CLI_OK) {
1133111350
cli_activation_transaction_finalize_committed_or_fail_stop(
@@ -11472,11 +11491,40 @@ int cbm_cmd_install(int argc, char **argv) {
1147211491
return CLI_TRUE;
1147311492
}
1147411493

11494+
char self_path[CLI_BUF_1K] = {0};
11495+
bool self_path_exact = cbm_detect_self_path(self_path, sizeof(self_path), home);
11496+
11497+
/* #1566: a binary owned by mise/Homebrew/nix is not ours to relocate. Infer
11498+
* it, and let either flag override the inference in either direction. */
11499+
bool externally_managed =
11500+
!force_binary && cli_binary_is_externally_managed(self_path, self_path_exact, bin_dir);
11501+
bool explicit_skip_binary = skip_binary;
11502+
if (externally_managed) {
11503+
skip_binary = true;
11504+
}
11505+
11506+
/* NOT stat(): on Windows it goes through the ANSI code page, so an
11507+
* extended-length or non-ASCII target reports "absent" and a non-force
11508+
* install silently overwrites bytes the user asked to keep. */
11509+
cbm_path_info_t target_status;
11510+
bool target_exists = cbm_path_info_utf8(bin_target, &target_status) == 0;
11511+
bool same_binary = cbm_same_file(self_path, bin_target);
11512+
11513+
/* #2722: when a binary is managed externally (mise, brew, nix, etc.),
11514+
* do not write the non-existent default target into agent configurations.
11515+
* Use the running managed binary path instead. */
11516+
const char *config_binary = bin_target;
11517+
if (externally_managed && self_path_exact && self_path[0]) {
11518+
config_binary = self_path;
11519+
} else if (skip_binary && !target_exists && self_path_exact && self_path[0]) {
11520+
config_binary = self_path;
11521+
}
11522+
1147511523
/* --plan: emit the machine-readable install receipt and exit WITHOUT
1147611524
* mutating anything (no config writes, no index deletion, no network) so
1147711525
* an agent can inspect exactly what install would touch first (#388). */
1147811526
if (plan) {
11479-
char *json = cbm_build_install_plan_json_options(home, bin_target, skip_config);
11527+
char *json = cbm_build_install_plan_json_options(home, config_binary, skip_config);
1148011528
if (!json) {
1148111529
(void)fprintf(stderr, "error: failed to build install plan\n");
1148211530
return CLI_TRUE;
@@ -11498,32 +11546,18 @@ int cbm_cmd_install(int argc, char **argv) {
1149811546

1149911547
printf("codebase-memory-mcp install %s\n\n", CBM_VERSION);
1150011548

11501-
char self_path[CLI_BUF_1K] = {0};
11502-
bool self_path_exact = cbm_detect_self_path(self_path, sizeof(self_path), home);
11503-
11504-
/* #1566: a binary owned by mise/Homebrew/nix is not ours to relocate. Infer
11505-
* it, and let either flag override the inference in either direction. */
11506-
bool externally_managed =
11507-
!force_binary && cli_binary_is_externally_managed(self_path, self_path_exact, bin_dir);
11508-
if (externally_managed || skip_binary) {
11549+
if (externally_managed || explicit_skip_binary) {
1150911550
const char *manager = cli_external_manager_name(self_path);
11510-
if (skip_binary) {
11551+
if (explicit_skip_binary) {
1151111552
printf("Skipping binary placement (--skip-binary); configuring agents only.\n\n");
1151211553
} else {
1151311554
printf("Binary is managed elsewhere%s%s:\n %s\n"
1151411555
"Leaving it and your PATH untouched; configuring agents only.\n"
1151511556
" (use --force-binary to install a copy into %s anyway)\n\n",
1151611557
manager ? " by " : "", manager ? manager : "", self_path, bin_dir);
1151711558
}
11518-
skip_binary = true;
1151911559
}
1152011560

11521-
/* NOT stat(): on Windows it goes through the ANSI code page, so an
11522-
* extended-length or non-ASCII target reports "absent" and a non-force
11523-
* install silently overwrites bytes the user asked to keep. */
11524-
cbm_path_info_t target_status;
11525-
bool target_exists = cbm_path_info_utf8(bin_target, &target_status) == 0;
11526-
bool same_binary = cbm_same_file(self_path, bin_target);
1152711561
bool do_copy = !skip_binary && !same_binary && (!target_exists || force);
1152811562

1152911563
/* (#607) Default: preserve existing indexes. `--reset-indexes` opts into
@@ -11690,6 +11724,7 @@ int cbm_cmd_install(int argc, char **argv) {
1169011724
#endif
1169111725
cli_install_activation_t activation = {
1169211726
.bin_target = bin_target,
11727+
.config_binary = config_binary,
1169311728
.bin_dir = bin_dir,
1169411729
.home = home,
1169511730
.shell_rc = shell_rc,

‎src/cli/cli.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ void cbm_cli_set_client_selection_for_testing(const char *spec);
250250
* not recognisably foreign. Detection must be positive evidence — see the
251251
* implementation comment. */
252252
const char *cbm_cli_external_manager_name_for_testing(const char *self_path);
253+
/* #2722: override runtime binary self-path detection for testing. */
254+
void cbm_cli_set_self_path_override_for_test(const char *self_path);
253255
/* #1558: expose the config-key table so a test can prove a key is discoverable
254256
* (`config list`/`set` both walk it). */
255257
size_t cbm_cli_config_key_count_for_testing(void);

‎tests/test_cli.c‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18673,6 +18673,65 @@ TEST(cli_external_manager_detection_needs_positive_evidence_issue1566) {
1867318673
PASS();
1867418674
}
1867518675

18676+
/* #2722: when a binary is managed externally (e.g. mise, brew, nix), `install`
18677+
* skips placing a binary at ~/.local/bin, but must write the running managed
18678+
* binary path into agent configuration files instead of the non-existent
18679+
* default target. */
18680+
TEST(cli_install_managed_binary_config_path_issue2722) {
18681+
char tmpdir[256];
18682+
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-install-managed-XXXXXX");
18683+
if (!cbm_mkdtemp(tmpdir)) {
18684+
FAIL("cbm_mkdtemp failed");
18685+
}
18686+
char *old_home = NULL;
18687+
char *old_cache = NULL;
18688+
cli_activation_save_env(&old_home, &old_cache);
18689+
cbm_setenv("HOME", tmpdir, 1);
18690+
char cache_dir[512];
18691+
snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir);
18692+
cbm_setenv("CBM_CACHE_DIR", cache_dir, 1);
18693+
18694+
test_mark_claude_code_installed(tmpdir);
18695+
18696+
const char *managed_path =
18697+
"/Users/x/.local/share/mise/installs/cbm/0.12.1/bin/codebase-memory-mcp";
18698+
cbm_cli_set_self_path_override_for_test(managed_path);
18699+
18700+
cli_activation_fake_t fake = {
18701+
.mutation_reserve_result = 1,
18702+
};
18703+
cbm_cli_activation_ops_t ops = cli_activation_fake_ops(&fake);
18704+
cbm_cli_set_activation_ops_for_test(&ops);
18705+
18706+
char *argv[] = {"--clients=claude"};
18707+
int rc = cli_test_cmd_install(1, argv);
18708+
18709+
cbm_cli_set_activation_ops_for_test(NULL);
18710+
cbm_cli_set_self_path_override_for_test(NULL);
18711+
18712+
char bin_target[512];
18713+
snprintf(bin_target, sizeof(bin_target), "%s/.local/bin/codebase-memory-mcp", tmpdir);
18714+
struct stat target_st;
18715+
bool target_created = stat(bin_target, &target_st) == 0;
18716+
18717+
char claude_json_path[512];
18718+
snprintf(claude_json_path, sizeof(claude_json_path), "%s/.claude.json", tmpdir);
18719+
char *claude_json = read_test_file_alloc(claude_json_path);
18720+
bool has_managed_path = claude_json && strstr(claude_json, managed_path) != NULL;
18721+
bool has_target_path =
18722+
claude_json && strstr(claude_json, ".local/bin/codebase-memory-mcp") != NULL;
18723+
18724+
free(claude_json);
18725+
cli_activation_restore_env(old_home, old_cache);
18726+
test_rmdir_r(tmpdir);
18727+
18728+
ASSERT_EQ(rc, 0);
18729+
ASSERT_FALSE(target_created);
18730+
ASSERT_TRUE(has_managed_path);
18731+
ASSERT_FALSE(has_target_path);
18732+
PASS();
18733+
}
18734+
1867618735
/* #1558: `install` configured EVERY detected client, so a user who wanted
1867718736
* Claude and Codex had to revert OpenCode and Cursor by hand — and the next
1867818737
* install silently recreated them. The selector restricts it.
@@ -19090,6 +19149,7 @@ SUITE(cli) {
1909019149
RUN_TEST(cli_ui_config_keys_are_discoverable_and_settable_issue1558);
1909119150
RUN_TEST(cli_skill_frontmatter_scalars_with_colons_are_quoted_issue1554);
1909219151
RUN_TEST(cli_external_manager_detection_needs_positive_evidence_issue1566);
19152+
RUN_TEST(cli_install_managed_binary_config_path_issue2722);
1909319153
RUN_TEST(cli_clients_selector_vocabulary_is_complete_and_strict_issue1558);
1909419154
RUN_TEST(cli_clients_selector_filters_registry_installs_issue1798);
1909519155
RUN_TEST(cli_update_accepts_retired_variant_flags_issue1544);

0 commit comments

Comments
 (0)