fix(cli): drop the POSIX probe-counter store that Clang 23 rejects - #2027
Merged
Conversation
`g_mcp_command_path_probe_counter` is only ever read inside `cbm_json_mcp_probe_command_path`, which is compiled for Windows alone. On POSIX test builds the setter stored a pointer nothing read, and Clang 23 (Homebrew LLVM 23.1.0, which `brew install llvm` now pours on the macOS LSan leg) flags that as `-Wunused-but-set-global`; with `-Werror` the whole test-runner build dies at `src/cli/cli.c:1768:21`. Keep the counter where a probe can happen (Windows) and make the POSIX setter an explicit no-op with the reason spelled out. The three tests that install a counter keep asserting it stays at zero on every platform, which on POSIX is exactly what "no classifier" means. Verified in the failing arena: `origin/main` `src/cli/cli.c` fails under clang 23.1.1 with the CI diagnostic; with this change it compiles clean, as do all 105 built `src` TUs and all 155 test + extraction TUs under the same flags. `cli` suite: 292 passed with Homebrew LLVM 22. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added
bug
birhantprkc
pushed a commit
to birhantprkc/codebase-memory-mcp
that referenced
this pull request
Sep 4, 2026
The test-lsan-macos leg installs the unversioned `llvm` formula, which tracks Homebrew's current stable. That moved from 22.1.8 to 23.1.0 today, so the leg's compiler now depends on which bottle the runner image draws: run 33745916176 (PR DeusData#1811) poured 23.1.0 and died at compile on a new Clang 23 diagnostic (-Wunused-but-set-global under -Werror); the run an hour earlier poured 22.1.8 and built fine. Same code, two verdicts. Pin the formula (and the `brew --prefix` lookup) to llvm@22 — the version every green run of this leg has used so far. Moving to a newer major becomes a deliberate edit of these two lines instead of an accident of the runner image. The diagnostic itself is fixed independently in DeusData#2027 so main compiles under both majors; this change is about determinism of the leg, not the warning. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What breaks
Since today,
brew install llvmon the macOS LSan leg pours LLVM 23.1.0 (the formula's new stable) instead of 22.1.8 — which bottle a runner gets depends on its Homebrew snapshot, so the leg is now red-or-green by draw. Under Clang 23 the test-runner build dies at:First seen on #1811's run 33745916176 (reported by @pcristin — thank you). Any PR whose LSan runner draws the 23.1.0 bottle hits it.
Why
g_mcp_command_path_probe_counteris only read insidecbm_json_mcp_probe_command_path, which is compiled for Windows alone (cli.c:1968-2014). On POSIX test builds the setter stored a pointer nothing ever read. Clang 23 is right: on POSIX the store is dead.Fix
Keep the counter where a probe can happen (Windows); make the POSIX setter an explicit no-op with the reason in place. The three
test_cli.ctests that install a counter keep asserting it stays at zero on every platform — on POSIX that is exactly what "there is no classifier" means, so the assertion keeps its meaning rather than being relaxed.Verification
origin/mainsrc/cli/cli.ccompiled with clang 23.1.1 and the Makefile'sCFLAGS_TEST→ the CI diagnostic verbatim. Same compile with this change → clean.srcTUs and all 155 test + extraction TUs compile clean under clang 23 with the same flags.clisuite with Homebrew LLVM 22.1.8: 292 passed.clang-format --dry-run --Werror: clean.Not touched here: whether
_test.ymlshould pin the formula (llvm@22) rather than float — that is a CI-scope choice and separate from making the code correct under the newer compiler.