Skip to content

feat(install): CBM_PACKAGE_MANAGED guard + FreeBSD self-path detection - #1331

Open
ocochard wants to merge 1 commit into
DeusData:mainfrom
ocochard:main
Open

feat(install): CBM_PACKAGE_MANAGED guard + FreeBSD self-path detection#1331
ocochard wants to merge 1 commit into
DeusData:mainfrom
ocochard:main

Conversation

@ocochard

@ocochard ocochard commented Jul 29, 2026

Copy link
Copy Markdown

Problems

When installed by a package manager (FreeBSD ports, Homebrew, distro pkg), codebase-memory-mcp install misbehaves in two ways:

  1. Mutates $HOME — copies the running binary into ~/.local/bin (a second, unmanaged copy of a file the package manager already owns under its prefix) and appends export PATH="~/.local/bin:$PATH" to the user's shell rc. Both are redundant: the binary is already on PATH via the manager's prefix.

  2. Writes the wrong binary path into agent configs on FreeBSD. cbm_detect_self_path() used readlink("/proc/self/exe"), but FreeBSD has no /proc mounted by default, so detection failed and silently fell back to ~/.local/bin/codebase-memory-mcp. That bogus path was then written as the MCP command into .claude.json / .mcp.json, so the agent could never launch the server.

Fix

Add a compile-time guard CBM_PACKAGE_MANAGED (off by default; follows the existing -DCBM_* build-flag convention). When set, install:

  • skips the ~/.local/bin binary copy (do_copy = false),
  • skips the PATH step (no shell-rc edit; completion message drops the source <rc> hint), and
  • points bin_target — hence the MCP command written into every agent config — at the running executable instead of the hardcoded ~/.local/bin path.

Separately (all platforms, not gated): add a FreeBSD branch to cbm_detect_self_path() using sysctl KERN_PROC_PATHNAME, which resolves the running executable without /proc. Non-FreeBSD detection is unchanged.

Verification

Built on FreeBSD with -DCBM_PACKAGE_MANAGED (via the devel/codebase-memory-mcp port, clean poudriere jail, -Werror, no warnings). Running codebase-memory-mcp install against a seeded Claude Code config now writes:

{ "mcpServers": { "codebase-memory-mcp": { "command": "/usr/local/bin/codebase-memory-mcp" } } }

No ~/.local/bin path anywhere, no shell rc touched. Source installs (scripts/build.sh, flag off) are unaffected.

@ocochard
ocochard requested a review from DeusData as a code owner July 29, 2026 06:03
…path

When built with -DCBM_PACKAGE_MANAGED, `install` no longer mutates the
user's $HOME and wires agent configs to the real installed binary:

- skips copying the running binary into ~/.local/bin (the package manager
  already owns it under its prefix), and
- skips appending `export PATH=...` to the shell rc, and
- points bin_target (hence the MCP `command` written into .claude.json,
  .mcp.json, etc.) at the running executable instead of the hardcoded
  ~/.local/bin path.

Also add a FreeBSD self-path branch: cbm_detect_self_path() used
readlink("/proc/self/exe"), but FreeBSD has no /proc by default, so it
silently fell back to ~/.local/bin — writing that wrong path into every
agent config. Use sysctl KERN_PROC_PATHNAME, which needs no /proc.

The flag is off by default; source installs (scripts/build.sh) are
unchanged. Non-FreeBSD self-path detection is unchanged.
@ocochard ocochard changed the title feat(install): add CBM_PACKAGE_MANAGED build guard feat(install): CBM_PACKAGE_MANAGED guard + FreeBSD self-path detection Jul 29, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thank you for this — the diagnosis is correct and the flag design is genuinely careful. Three things: one you can fix now, one that is not your fault, and one that needs the maintainer.

Your FreeBSD diagnosis is exactly right. FreeBSD mounts no /proc by default, so the existing readlink("/proc/self/exe") silently failed and a bogus ~/.local/bin path got written into .claude.json and .mcp.json. sysctl(KERN_PROC_PATHNAME) with pid = -1 is the correct fix, and thank you for verifying it in a clean poudriere jail with -Werror and showing the resulting config.

The flag design is genuinely additive, and I verified that rather than taking it on trust. With CBM_PACKAGE_MANAGED unset, the only delta is capturing a previously-discarded return value and (void)-ing it — byte-for-byte identical behaviour for every existing user on every shipped platform. The FreeBSD branch is #elif defined(__FreeBSD__) inside the existing chain, so it is preprocessor-inert on Linux, macOS and Windows. I also confirmed the empty-shell_rc mechanism correctly short-circuits the PATH step in cli_install_activate.

One thing to fix now: the DCO check is a genuine failure. Commit dfd2ab22 has no Signed-off-by trailer matching its author:

git commit --amend -s
git push --force-with-lease

The sign-off email needs to match your commit author email. That alone blocks the merge regardless of everything else.

One failure that is not yours: test / test-unix (macos-15-intel) failed on lock_registry — a known transient-window flake on our slowest runner that asserts a 500 ms observation window. An install-command diff cannot touch lock registry code. I have flagged those tests separately for a determinism fix.

One defect worth fixing while you are in there. Under CBM_PACKAGE_MANAGED, if self-path detection fails, bin_target silently stays ~/.local/bin/... while the copy step is skipped — so the agent configs would point at a binary that was never installed there. That is a broken config written confidently. It should fail loudly instead.

Minor style note: #include <sys/sysctl.h> before <sys/types.h> — FreeBSD convention is types first, and it only builds because cli.c already pulls types in earlier.

What needs the maintainer, and why I am not deciding it: CBM_PACKAGE_MANAGED would be a permanent build-flag surface. Once ports or Homebrew build with it, we support it indefinitely. And the FreeBSD path raises the same question we have open on #1342 (NetBSD): do we accept code for a platform we cannot build, test, or regression-guard in CI? Both deserve one consistent answer, so I have put them together rather than answering one by accident.

Relatedly, and worth knowing: the CBM_PACKAGE_MANAGED path is compiled by no CI leg today, so all three guarded behaviours would be untested from CI's perspective. If it is accepted, a Linux compile leg with -DCBM_PACKAGE_MANAGED would be the minimum guard.

The work itself is tidy and the packaging motivation is legitimate — this is a policy question, not a quality one.

@DeusData DeusData added enhancement New feature or request editor/integration Editor compatibility and CLI integration ux/behavior Display bugs, docs, adoption UX labels Aug 3, 2026
@DeusData DeusData added this to the 0.9.2-rc milestone Aug 3, 2026
@DeusData DeusData added the priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens. label Aug 3, 2026
@DeusData

DeusData commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Thank you for this PR — the problem is real and well worth fixing: self-update clobbering a package-manager-owned binary and editing the user's shell rc is exactly the wrong behavior for a ports/package install, and FreeBSD reaching cbm through ports would be a genuinely nice distribution win. The sysctl(KERN_PROC_PATHNAME) self-path detection is also the right primitive there.

Before we can take it, though, our review turned up a few functional gaps we'd like your read on:

  1. macOS/Homebrew defeats the guard: once bin_target is repointed at the running executable, sign_binary = do_copy || target_exists is always true — so the install path still stages, ad-hoc-signs, and re-commits onto the package-manager-owned binary in its own prefix (or hard-fails on a root-owned prefix). That's the exact clobber the flag is meant to prevent, on one of the platforms a packager would build for.
  2. Detection-failure fallback reintroduces the bug: with do_copy forced false but bin_target only repointed if (self_path_exact), a failed self-path detection means nothing is copied yet a bogus ~/.local/bin path is still written into the agent configs — silently.
  3. Partial suppression: bin_dir is still created (~/.local/bin), the Windows PATH edit isn't gated (the suppression is POSIX-only), and --dir gets silently ignored under the guard.
  4. Testability: as a compile-time macro that no build we gate ever defines, the branch is never compiled, linted, or tested by any CI leg on any OS — and several existing --dir install tests would fail under the flag as written.

Our main question is about the layer: would a runtime decision work for your use case — e.g. "the running binary is not under bin_dir (or bin_dir isn't ours) → don't copy, don't touch rc, point configs at the running exe"? That reaches the same outcome for ports, needs no build variant, keeps one binary behavior everywhere, and is testable by the existing suite. If there's a packaging constraint that genuinely needs the compile-time gate (e.g. ports policy about runtime heuristics), we'd like to understand it before deciding the direction.

Also fyi: resolve_self_executable() in src/ui/http_server.c:908 still uses readlink("/proc/self/exe"), so the FreeBSD path fix would want to cover that call site too.

No action needed on the defects yet — we'd like your feedback on the runtime-vs-compile-time question first, then we can converge on the shape. Thanks again for bringing FreeBSD packaging to the table!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editor/integration Editor compatibility and CLI integration enhancement New feature or request priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants