Skip to content

fix(cli): use managed binary path for agent configs when externally managed (#2722) - #2822

Merged
DeusData merged 6 commits into
DeusData:mainfrom
divakantpandey:fix/2722-install-managed-binary-path
Oct 31, 2026
Merged

DeusData merged 6 commits into
DeusData:mainfrom
divakantpandey:fix/2722-install-managed-binary-path

Conversation

@divakantpandey

Copy link
Copy Markdown
Contributor

Fixes #2722

Problem

When the binary is managed externally (e.g. mise, Homebrew, nix, asdf, cargo), codebase-memory-mcp install recognizes that the binary is managed elsewhere and skips copying the binary to $HOME/.local/bin/codebase-memory-mcp. However, it still passed bin_target into cbm_build_install_plan_json_options() and cbm_install_agent_configs_with_previous(), causing agent configurations (Claude Code, OpenCode, VS Code, Zed, Copilot CLI, Junie, etc.) and --plan receipts to reference the non-existent default binary path instead of the detected managed path.

Solution

  • Evaluate self_path and external management detection prior to --plan and agent configuration.
  • Use self_path as config_binary when the binary is externally managed (or when --skip-binary is passed and bin_target does not exist).
  • In cli_install_activate(), pass config_binary to cbm_install_agent_configs_with_previous(). If config_binary differs from bin_target, pass bin_target as the previous binary to ensure pre-existing configs pointing to ~/.local/bin are recognized and migrated.
  • Exposed cbm_cli_set_self_path_override_for_test under CBM_CLI_ENABLE_TEST_API to deterministically verify managed binary installation behavior in test suites.

Verification

  • Added cli_install_managed_binary_config_path_issue2722 in tests/test_cli.c.
  • ./scripts/test.sh --suites cli: 371 passed, 0 failed.
  • ./scripts/test.sh --contracts-only: All static contracts passed.
  • ./scripts/check-dco.sh upstream/main..HEAD: Valid DCO sign-off verified.
  • ./scripts/build.sh: Clean build.

…anaged (DeusData#2722)

Signed-off-by: Divakant Pandey <divakant01@gmail.com>
@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

…ta#2722)

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData

Copy link
Copy Markdown
Owner

Thank you so much, @divakantpandey - and thanks again for the Pi adapter fix in #2807. This one is a really good catch: install already recognised that the binary belongs to mise/Homebrew/nix and said so, and then still wrote the default ~/.local/bin path into every agent config, a path that does not exist on those setups. Moving the detection ahead of --plan so the receipt and the real install agree, migrating entries that still point at the old default target, and adding a test that checks the path actually written into the config (not just that no binary was placed) is exactly the right shape.

We pushed one small commit (113f24f) on top of your branch:

  1. style(cli): clang-format the managed-binary agent-config call (#2722) - joins the cbm_install_agent_configs_with_previous(...) arguments in cli_install_activate() onto one line. That was the only thing CI's clang-format check flagged (src/cli/cli.c:11338), and because the lint job gates the test matrix, none of the test legs had run yet. With it in, the full matrix, including your new cli_install_managed_binary_config_path_issue2722 test, runs on all three platforms.

There is no change to your logic. If you push to this branch again, please pull first so the extra commit is not lost.

Thanks again for taking this on - it fixes a real day-one problem for everyone who installs through a package manager.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData

Copy link
Copy Markdown
Owner

Thank you for your patience, @divakantpandey. A pull request that merged into main a few minutes ago (#2290) changes src/cli/cli.c, which this pull request also changes, so the CI run in progress here could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, 4a85a26; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

@divakantpandey

Copy link
Copy Markdown
Contributor Author

Pulled the branch locally to stay in sync. Thanks for handling the merge and the review!

@divakantpandey

Copy link
Copy Markdown
Contributor Author

Thank you for the detailed review and kind feedback, Martin! Really glad this helps make package-managed installs seamless. Also appreciate you taking care of the clang-format adjustment and keeping the branch in sync with main.

@DeusData

Copy link
Copy Markdown
Owner

Thank you for your patience, @divakantpandey. Main has moved since the last CI run on this branch, and some of the new commits change src/cli/cli.c and tests/test_cli.c, which this pull request also changes, so that run can no longer vouch for today's main. We merged current main into your branch (one merge commit, cf77fb8; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData

Copy link
Copy Markdown
Owner

Thank you for your patience, @divakantpandey. The macOS legs of the last CI run here were cancelled by GitHub itself: it is retiring its macOS 14 runners on November 2 and ran scheduled brownouts beforehand, so those jobs never got a runner. Nothing in your change caused it. Main now runs CI on macOS 15 (#2849), so we merged current main into your branch (one merge commit, 4787012; it merged cleanly and your own change is untouched) to get a run on the new runners. Please pull before pushing again.

@DeusData
DeusData merged commit 70f6de8 into DeusData:main Oct 31, 2026
34 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged as 70f6de8 - thank you, @divakantpandey! When the binary is managed by mise, Homebrew, nix, asdf or cargo, install now writes the path the running binary really has into the agent configs instead of a ~/.local/bin path that does not exist on those setups, the plan receipt and the real install agree, and old entries that pointed at our previous default target are migrated. Thank you for the careful fix and the test that checks the path actually written. We opened #2856 as a follow-up for what happens to those entries across a package-manager upgrade and on uninstall, so that part is not lost.

@DeusData DeusData mentioned this pull request Oct 31, 2026
2 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong Binary Path Used during Installation

2 participants