[wcy12378]

Why

Issue #221 (install cannot wire up opencode on Windows 11) was reopened after the
PATHEXT fix because two further root causes remain (maintainer diagnosis, 2026-09-26):

1. The installer's PATH search copied %PATH% into a fixed 4 KB stack buffer
   (char path_copy[CLI_BUF_4K] in find_in_path). On Windows dev machines whose
   process PATH is >= 4096 characters, cbm_safe_getenv cannot fit the value and the
   search skips the whole PATH, so opencode (present via mise/npm/scoop) is invisible.
2. The cbm_find_cli home-directory fallbacks were POSIX-only
   (/usr/local/bin, ~/.npm/bin, ~/.local/bin, ~/.cargo/bin, /opt/homebrew/bin).
   None of these exist on Windows; the Windows-shaped locations
   (%APPDATA%\npm, %LOCALAPPDATA%\mise\shims, %USERPROFILE%\scoop\shims) were never probed.

Summary

• find_in_path now duplicates PATH into a heap buffer sized to the real value
  (cbm_dup_env_path), so long PATHs are searched in full instead of being truncated.
• cbm_find_cli builds a platform-aware fallback list. On _WIN32 it probes
  %APPDATA%\npm, %LOCALAPPDATA%\mise\shims and %USERPROFILE%\scoop\shims, and
  also tries the PATHEXT variants (.exe/.cmd/.bat/.ps1) for those fallback locations
  so the extension-bearing shims resolve. POSIX behaviour is unchanged.

How to Test

Build on Windows and run the CLI suite (the existing repro_issue221_opencode_pathext_lookup
regression guard must stay green; a long-PATH (>4096 char) + npm/mise/scoop shim case is the
additional scenario this addresses). cbm_find_cli("opencode", home) should now resolve a
shim that lives on a long PATH or under one of the Windows install dirs.

Closes #221

[github-actions]

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.

[DeusData]

Thank you so much, @wcy12378, for picking up #221 within a day of our root-cause note and implementing both halves of it: the full-length PATH copy, and Windows-aware npm/mise/scoop fallbacks. You read the diagnosis carefully, and the PR body explains the why clearly. This is the right fix direction and we'd love to merge it. A few things need to change first; here is each one and why:

1. find_in_path leaks the PATH copy on success. The two return true; statements inside the loop skip free(path_copy), so every detected CLI leaks a full copy of PATH, and agent detection runs this about 25 times per scan. Please free before every return, for example with a single found flag and one exit.
2. Please allocate through the memory core. This repo routes all allocation through src/foundation/mem_core.h (cbm_alloc/cbm_calloc/cbm_free). scripts/lint-memory-core.py is a ratchet that fails when a file gains raw malloc/calloc/free sites. This PR adds 8 to src/cli/cli.c (173 → 181), so CI will go red. The ratchet is how we keep memory accounting and leak detection honest across the whole binary.
3. Please reuse the platform env helper instead of re-implementing it in cli.c. cbm_safe_getenv (src/foundation/platform.c) already does the wide read, the UTF-8 conversion and the missing-vs-empty handling. The cleanest shape is a small allocating sibling there that returns a memory-core-owned UTF-8 copy via cbm_wide_to_utf8, called from find_in_path. That keeps #include <windows.h> out of the middle of cli.c and gives both call sites one tested code path.
4. Please keep the existing Windows fallbacks and add the new ones. %USERPROFILE%/.local/bin and %USERPROFILE%/.cargo/bin were also probed on Windows before this change, and both are real there: pipx installs to ~\.local\bin by default, and cargo to ~\.cargo\bin. Replacing the list would make pipx- and cargo-installed agents disappear. Please also build these paths from the home_dir argument rather than cbm_get_home_dir(), so the function honours its contract and can be tested with a temporary home. / separators, as the rest of the file uses, are fine on Windows.
5. Please add a test that fails without the change. We merge bug fixes with a regression test that is red on main and green with the fix, so the bug can't quietly return. The 4 KB PATH limit affects POSIX too, so a portable test is easy: build a PATH longer than 4096 characters whose last entry is a temp directory holding a uniquely named executable, and assert cbm_find_cli resolves it. That is also the case that covers the original #221 reporter's layout. For the npm/mise/scoop fallbacks, a Windows-only test that points APPDATA, LOCALAPPDATA and USERPROFILE at temp directories containing a name.cmd shim would be ideal. tests/repro/repro_issue221.c is a natural home for both.
6. Small style points. Please add braces to the one-line if/for statements (clang-format and our style require them), and hoist the duplicated win_exts[] array into one file-scope constant.
7. DCO sign-off. The commit has no Signed-off-by: trailer, and CI enforces DCO on every commit. git commit --amend -s followed by a force-push fixes it.

Thanks again. This will make install work for a lot of Windows developers with long PATHs, and we really appreciate you taking it on. We're happy to help if any of the above is unclear.

[DeusData]

Thank you so much, @wcy12378! This picks up exactly the two causes we described on #221, and the heap-sized PATH copy is the right shape. A few changes are needed before it can land:

1. Allocation through the memory core. Please use the project's memory core (src/foundation/mem_core.h: cbm_calloc / cbm_alloc / cbm_free / cbm_mem_strdup) instead of raw calloc/malloc/free. The lint job has a memory-core ratchet that fails when raw allocation counts grow in a file. On Windows, src/watcher/watcher.c already reads PATH with GetEnvironmentVariableW sizing, so a small shared helper could serve both.
2. Keep the home_dir argument. Please use the home_dir argument, not cbm_get_home_dir(), for the scoop fallback. Also keep <home_dir>/.local/bin on Windows: cbm installs itself there, and tests and dry-run scans pass a synthetic home.
3. Loop bound. The fallback loop should run to NUM_PATHS, not NUM_RETRIES. Both are 5 today, so the bug is latent.
4. Formatting. Please add braces to the one-line ifs, run clang-format (Homebrew LLVM) on the file, and move the windows.h include up to the top-of-file includes.
5. Tests. In the cli suite, please add:
   • a PATH longer than 4,096 characters, with the binary's directory near the end, that must be found (this also fails on macOS/Linux before your change);
   • the fallback dirs exercised through a HOME/env seam.
6. DCO. The DCO check needs a sign-off: git commit --amend -s and force-push.

Thanks again, and happy to help if any of this is unclear!

[wcy12378]

Thanks for the detailed review — all seven points are addressed. The branch is also rebased onto today's main (80eb92a7) and is now a single signed commit, so the diff you review is the diff you get.

1. find_in_path leak. The scan now has one exit: a found flag, a while (dir != NULL && !found) loop, and a single cbm_free(CBM_MEM_CLASS_OTHER, path_copy) before return found. No return inside the loop any more.

2. Allocation through the memory core. The PATH copy is now allocated by the new cbm_env_dup in src/foundation/platform.c, which uses cbm_calloc (wide staging) and cbm_alloc (UTF-8 result) and is released with cbm_free. src/ gains no raw allocator site at all, so the ratchet is untouched — I ran scripts/lint-memory-core.py locally against the merged tree: 3801 raw sites across 85 files, none grew (exit 0). src/cli/cli.c is back to its baseline of 170.

3. Reuse the platform env helper. cbm_env_dup is the allocating sibling of cbm_safe_getenv, declared in platform.h next to it, and find_in_path just calls it. <windows.h> is gone from the middle of cli.c.

One deliberate deviation to flag: it does not call cbm_wide_to_utf8. That helper returns a raw-malloced buffer, and the free() needed to release it would itself be a new raw site in platform.c, which the ratchet would then reject. So the sibling performs the same WideCharToMultiByte call with the same WC_ERR_INVALID_CHARS flags, but allocates its output through the core. Happy to route it through cbm_wide_to_utf8 instead if you would rather have that shared code path and accept a baseline bump or a small migration in the same change.

4. Existing Windows fallbacks kept. %USERPROFILE%/.local/bin (pipx) and %USERPROFILE%/.cargo/bin (cargo) are still probed and are now built from the home_dir argument, not cbm_get_home_dir(), so a synthetic home is honoured. Added: %APPDATA%/npm, %LOCALAPPDATA%/mise/shims, <home_dir>/scoop/shims. All five use / separators. The list is exactly NUM_PATHS = 5, and the loop is now bounded by NUM_PATHS rather than NUM_RETRIES. I dropped /usr/local/bin and <home>/.npm/bin from the Windows list because neither is a real Windows location — say the word if you want them kept for MSYS2/MinGW shells.

5. Regression test. Both cases live in tests/repro/repro_issue221.c, extending the existing #221 cluster:

• repro_issue221_long_path_beyond_four_kb — builds a 4668-character PATH whose last entry is a temp dir holding the fixture, so any truncation hides exactly the directory under test. The fixture name is unique, and home_dir is a non-existent path so the fallbacks cannot satisfy it.
• repro_issue221_windows_fallback_shims — Windows-only; drives one shim at a time through APPDATA, LOCALAPPDATA and the home_dir argument, asserting each entry resolves in its own directory. All the environment swaps are restored on every exit path.

I could not run the repo's own test binary here (no gcc/clang on this box), so I extracted the three functions verbatim, compiled them with MSVC, and ran both cases against each revision. Against the current main code the long-PATH case fails and all five fallback cases fail; with the change applied all six pass. That second result is also a small finding: the fallback directories probe only the bare name today, so a .cmd shim is invisible even in ~/.local/bin — the PATHEXT probe is needed on the fallback list too, not just on PATH entries.

6. Style. Braces on every one-line if/for, win_exts hoisted to a single file-scope WIN_EXEC_EXTS constant, and clang-format --dry-run --Werror is clean on cli.c, platform.c and platform.h. (The formatter here is LLVM 19.1.5 with your .clang-format; I checked it reproduces your files byte-for-byte, so that run is not a false pass.)

7. DCO. Single commit, Signed-off-by: Kris <2040188027@qq.com>, and scripts/check-dco.sh passes locally against the real range. The rebase was force-pushed, so the previous head (9d8f51d7) is superseded rather than amended.

One thing worth your call: I kept the tests in tests/repro/ as you suggested, which means they are not in ALL_TEST_SRCS and so do not gate ci-ok. If you would rather have the long-PATH case enforced by the gating cli suite as well, I am happy to mirror it into tests/test_cli.c.

[wcy12378]

The failing Windows shard stops in the parallel scheduler infrastructure self-test, before the CLI suite: suite 'hang_after_summary' taskkill could not prove process-tree cleanup (exit 2). This then leaves shard 2's manifest absent, causing shard-completeness and ci-ok to fail as well.

Evidence: https://github.com/DeusData/codebase-memory-mcp/actions/runs/36737825537/job/109990817112 (log at 2026-09-30T16:45:05Z).

This PR does not modify the scheduler self-test. The latest main cache-warm run passed, but it is a different workflow, so I cannot use it as proof that this particular self-test passes on main. I attempted to rerun the failed jobs; GitHub refused because repository admin rights are required. Could a maintainer rerun this job when convenient? I will follow up on any reproducible failure in the changed CLI/platform code.

[wcy12378]

Re-checking CI on the current head (48f2559): the only red contexts are test / test-windows (CLANG64, 2/2) and its downstream shard-completeness + ci-ok. That Windows shard dies in the parallel-scheduler self-test hang_after_summary (taskkill could not prove the process was killed) — i.e. before the CLI suite runs and before build/c/test-logs/shard-manifest.txt is ever written, which is why shard-completeness reports the manifest missing. It is an infra flake, not the CLI change.

Everything that actually exercises the code is green: lint (cppcheck + clang-format-20), ubuntu (3/3 shards), and macos (both arches) all pass, and the two regression tests in tests/repro/repro_issue221.c exercise the long-PATH and npm/mise/scoop-shim paths directly.

Could a maintainer please re-run the failed Windows shard (or the whole test job) to clear ci-ok? The code is complete and rebased onto today's main; all seven review points from #221 are addressed in the single signed commit. Happy to help triage the hang_after_summary flake separately if you'd like.

[wcy12378]

The requested changes are present in head 48f2559: ind_in_path owns/frees its PATH copy through the memory core and has one cleanup exit; PATH and fallback lookups share the Windows extension list; existing pipx/cargo locations are retained; Windows npm/mise/scoop paths and test fixtures use the documented env/home seams; the long-PATH regression and style changes are included. The commit also has the required Signed-off-by: Kris 2040188027@qq.com trailer.

The remaining red CI is infrastructure-only: every Windows shard task passed, but the Windows CLANG64 2/2 runner then failed to prove process-tree cleanup for hang_after_summary ( askkill could not prove process-tree cleanup), so it emitted no shard manifest and shard-completeness failed downstream. I attempted gh run rerun ... --failed, but GitHub denied it because rerunning requires repository admin rights. Could a maintainer rerun the failed Windows job when convenient? No source change appears warranted by this log.

[DeusData]

Thanks for the detailed response to the review and the repeated Windows-runner evidence. I have read the request for a maintainer rerun and the question about gating the regression cases. Those remain on our side; this sweep has not rerun CI or independently attributed the cleanup failure. No new source change is requested here.

[DeusData]

Thank you so much, @wcy12378, and sorry for the long wait on our side. Your second round answered every point of our review, and it was done with real care: the single-exit find_in_path that can no longer leak the PATH copy, cbm_env_dup allocating through the memory core so the ratchet stays untouched, the pipx and cargo fallbacks kept and built from the home_dir argument, and a long-PATH test that puts the fixture in the LAST entry so any truncation hides exactly the directory under test. The finding that the fallback directories needed the PATHEXT probe too was a genuinely good catch.

Since main had moved a long way, we took the remaining small steps ourselves, as new commits on top of yours (your commit is unchanged):

1. Merged current main. The only conflict was in src/foundation/platform.h: main added declarations for a settings environment resolver right next to cbm_safe_getenv, where your cbm_env_dup declaration sits. Both are kept.
2. cbm_env_dup now asks that resolver first (src/foundation/platform.c). After your rebase, main changed cbm_safe_getenv to consult a startup settings resolver before the process environment. Without this, the two helpers could give different answers for the same name. The resolver only serves CBM_* settings today, so PATH lookups behave exactly as in your version. platform_env_dup_honours_the_settings_resolver in tests/test_platform.c pins it.
3. Moved your two regression tests into the gating cli suite (tests/test_cli.c, next to the other cbm_find_cli tests), unchanged apart from their names. To answer your question: yes, we want them to gate. tests/repro/ builds only into the non-gating bug-repro runner, and the Windows-only case would also have shown up there as a skipped case on macOS and Linux, which that board treats as incomplete. In the cli suite, SKIP_PLATFORM is the normal way to mark a Windows-only case, and the suite runs on every CI leg. tests/repro/repro_issue221.c is back to its content on main.

On your other two questions:

• Doing the WideCharToMultiByte conversion in cbm_env_dup instead of going through cbm_wide_to_utf8 is the right call. It keeps every allocation in the memory core, so no change is needed.
• Dropping /usr/local/bin and ~/.npm/bin from the Windows list is fine too: neither is a real location for a native Windows binary, and npm's global bin is the %APPDATA%\npm entry you added.

The red Windows shard you reported was not caused by your change, and CI runs fresh on the new head. Thanks again for sticking with this through several rounds. It will make install work for a lot of Windows developers with long PATHs.

[wcy12378]

Hi @DeusData maintainers — following up on the 7 CHANGES_REQUESTED items from the review. All points were addressed and the branch was rebased onto the current main tip (the diff is now MERGEABLE). Specifically: single-exit PATH handling, all new code routes through the memory core (no new raw allocations), the new cbm_env_dup helper lives in platform.c, pipx/cargo fallbacks preserved with npm/mise/scoop added, two regression tests added, clang-format clean, and DCO signed. Could you please re-review when convenient? Thanks!

[wcy12378]

Maintainers — reporting on the red CI on the current head 079fb20 (the branch tip after your merge/test-move commits; our commit is unchanged underneath).

The failing shards fail in repo-wide documentation contract checks, not in the CLI-detection code this PR touches:

• test / test-unix (ubuntu-latest, gcc, g++, unix-x86, 1/1) (job 118928439585) ends with 5 language-count contract violations: docs/index.html, docs/llms.txt, pkg/npm/README.md, scripts/package-release.sh, and server.json are registered as language-count surfaces but state no count. None of these files are touched by this PR (the diff is limited to src/cli/cli.c, src/foundation/platform.{c,h}, tests/test_cli.c, tests/test_platform.c).
• The same failure family reproduces on current main independent of this branch: PR run 40024734482 ("feat: add agentty agent support to the install CLI", 2026-10-23) fails the identical shard set — test-unix (ubuntu-x86, macos-14, macos-15-intel, arm64), test-windows, test-lsan-macos, ci-ok — with e.g. cli_supported_agent_surfaces_match_installers failing on "README must describe all 46 automatic/conditional client surfaces accurately" (job 120671043564).

So both the unix-shard and windows-shard reds on this head reflect the same repo-wide docs-contract drift on main, consistent with your earlier finding that the Windows shard failure was not caused by this change. Happy to help fix the docs contracts if that is wanted from contributors; otherwise this looks maintainer-side. The CLI/platform code and its gating cli-suite regression tests remain green in the passing shards (pr-smoke, test-package-wrappers, test-ui, test-windows-guards, lint/security gates).

[wcy12378]

Branch updated onto current main (a2227157 → 10ab613d), which clears the red CI this PR has been carrying.

The previous failures were not from this diff. Every failing lane (test-unix ×5, test-windows, test-lsan) was tripping the same repo-wide contract:

FAIL: docs/index.html is registered as a language-count surface but states no count
FAIL: docs/llms.txt is registered as a language-count surface but states no count
FAIL: pkg/npm/README.md is registered as a language-count surface but states no count
FAIL: scripts/package-release.sh is registered as a language-count surface but states no count
FAIL: server.json is registered as a language-count surface but states no count
FAIL: 5 language-count contract violation(s)

None of those five paths is in this PR, which touches only src/cli/cli.c, src/foundation/platform.{c,h}, tests/test_cli.c, tests/test_platform.c. The contract was broken on main and repaired upstream by #2732 (fix(ci): repair language count and Windows CodeQL retrace contracts, merged 2026-10-21 20:00) — after the CI run this PR was sitting on.

On the new base the earlier blockers are already green: lint / lint-mem (the memory-core allocation ratchet, none grew), dco, security / security-static. Test lanes are re-running now.

On the seven review items from 2026-09-27, all are addressed in 079fb20 and the per-item reply above still stands; the one deliberate deviation is that cbm_env_dup does not route through cbm_wide_to_utf8, because that helper returns a raw malloc and freeing it would add a new raw allocation site and trip the ratchet.

Could you re-review when the new run settles? Happy to split or reshape anything that is still awkward.

[wcy12378]

The lanes that were re-running when I last reported have now settled on head 7d01cae — the full PR run (40052896282) and the DCO run (40052896244) completed green: every test shard (unix x86 + arm64, macos both arches, tsan, lsan-macos, windows CLANG64, windows-guards, package-wrappers, test-ui), all three pr-smoke legs, lint (cppcheck + clang-format-20 + lint-mem), security-static, license-gate, CodeQL, shard-completeness and ci-ok all pass, and GitHub reports the branch MERGEABLE / CLEAN.

No new commits were needed for this round; the head is unchanged at 7d01cae. For convenience, the per-item status against the 2026-09-27 review:

1. find_in_path PATH-copy leak — fixed with a single found flag and one exit; the copy is released once via cbm_free(CBM_MEM_CLASS_OTHER, path_copy) on every path out of the loop (src/cli/cli.c).
2. Allocation through the memory core — every new allocation goes through cbm_alloc/cbm_calloc/cbm_mem_strdup; scripts/lint-memory-core.py passes ("3708 raw sites across 82 files, none grew"), so the ratchet never moved.
3. Platform env helper — cbm_env_dup() lives in src/foundation/platform.c (declaration in platform.h), returns a memory-core-owned UTF-8 copy from the wide read, and is the only PATH source find_in_path uses; #include <windows.h> stays out of cli.c. The one deliberate deviation remains the one you endorsed on Oct 20: converting inline with WideCharToMultiByte instead of routing through cbm_wide_to_utf8, since that helper returns a raw malloc and freeing it would add a raw free site and trip the ratchet. Pinned by platform_env_dup_honours_the_settings_resolver in tests/test_platform.c.
4. Fallbacks kept and extended — %USERPROFILE%/.local/bin (pipx) and %USERPROFILE%/.cargo/bin (cargo) are still probed first on Windows, with npm (%APPDATA%\npm), mise (%LOCALAPPDATA%\mise\shims) and scoop (%USERPROFILE%\scoop\shims) added after; every home-derived entry is built from the home_dir argument with / separators.
5. Regression tests — both live in the gating cli suite (tests/test_cli.c): cli_find_cli_long_path_beyond_four_kb_issue221 (portable: PATH built past 4096 characters with the uniquely named fixture directory as the LAST entry) and cli_find_cli_windows_fallback_shims_issue221 (Windows-only: APPDATA/LOCALAPPDATA/USERPROFILE pointed at temp roots holding a name.cmd shim in each of the five fallback locations). Both are RED against the pre-fix code (the 4 KB copy is refused outright; bare-name probing never matches a .cmd shim) and green here.
6. Style — the duplicated win_exts[] is hoisted to the file-scope WIN_EXEC_EXTS constant; all one-line if/for bodies are braced; the clang-format-20 lane is green.
7. DCO — 48f2559 carries Signed-off-by: Kris <2040188027@qq.com> matching the commit author identity, your follow-up commits are signed, and the DCO lane is green on 7d01cae.

Could you re-review when convenient? Happy to split or reshape anything that is still awkward.

[DeusData]

Thank you for the detailed October 25 update and the reported green runs on 7d01cae. The earlier review items and gating-test placement have been addressed according to the thread; the requested re-review is now on us. Sorry for the delay—I’ll follow up soon.

[DeusData]

Merged - thank you so much, @wcy12378! This fixes a long-standing Windows install problem, and you carried it through every round with real care: the PATH copy sized to the real value, the single-exit find_in_path that can no longer leak, cbm_env_dup allocating through the memory core, the pipx and cargo fallbacks kept and built from the home_dir argument, and a long-PATH test that puts the fixture in the last entry so any truncation hides exactly the directory under test. Your finding that the fallback directories needed the PATHEXT probe too was a genuinely good catch, and your careful CI write-ups saved us real time. Both regressions now gate in the cli suite on every CI leg, Windows included. Thanks again for sticking with this - it will make install work for a lot of Windows developers.