diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 53ecb358..bf4dff21 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -127,8 +127,9 @@ jobs: # over stdio / CLI / HTTP and fail if a Windows bug already fixed on main comes # back -- non-ASCII repo paths dropping definitions (#636/#357, fixed by #700), # the PreToolUse hook augmenter no-op on drive-letter cwd (#618, fixed by #619), - # and the UI directory picker not enumerating drives (#548, roots field). The - # still-open narrow-argv repro (#423/#20) is opt-in and excluded via -GuardsOnly. + # the UI directory picker not enumerating drives (#548, roots field), and non-ASCII + # CLI arguments being mangled by the narrow-argv main() (#423/#20, fixed by the + # wide-argv entrypoint that reads GetCommandLineW). test-windows-guards: runs-on: windows-latest timeout-minutes: 60 @@ -155,9 +156,10 @@ jobs: # guard's HTTP UI is available. Functional gate only (no sanitizers). run: scripts/build.sh --with-ui CC=clang CXX=clang++ - - name: Windows regression guards (#636/#357, #618, #548) + - name: Windows regression guards (#636/#357, #618, #548, #423/#20) shell: pwsh - # -GuardsOnly runs the three green guards and gates on them; the runner - # runs indexing in-process (CBM_INDEX_SUPERVISOR=0) so a guard reflects the - # path/hook/drive fix under test, not the orthogonal index-worker spawn. + # -GuardsOnly runs the four green guards and gates on them. The runner sets + # CBM_INDEX_SUPERVISOR=0 for determinism, but the non-ASCII CLI guard drops that + # override so it exercises the real supervisor->worker spawn (where #423/#20's + # second half lives); the other guards test path/hook/drive fixes in-process. run: ./scripts/test-windows.ps1 -GuardsOnly -Binary build/c/codebase-memory-mcp.exe diff --git a/Makefile.cbm b/Makefile.cbm index 299cfcde..4e8130a9 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -72,13 +72,14 @@ CFLAGS_TSAN = $(CFLAGS_COMMON) -g -O1 \ CXXFLAGS_TSAN = $(CXXFLAGS_COMMON) -g -O1 \ -fsanitize=thread -fno-omit-frame-pointer -# Windows needs ws2_32 (Winsock), psapi (GetProcessMemoryInfo), and +# Windows needs ws2_32 (Winsock), psapi (GetProcessMemoryInfo), shell32 +# (CommandLineToArgvW — the UTF-8 argv path in main.c, #423/#20), and # --allow-multiple-definition (MinGW CRT symbol clashes). # Auto-detected via compiler; no manual override needed. IS_MINGW := $(shell echo | $(CC) -dM -E - 2>/dev/null | grep -q 'define _WIN32 ' && echo yes || echo no) WIN32_LIBS := ifeq ($(IS_MINGW),yes) -WIN32_LIBS := -lws2_32 -lpsapi -Wl,--allow-multiple-definition -Wl,--stack,8388608 -static +WIN32_LIBS := -lws2_32 -lpsapi -lshell32 -Wl,--allow-multiple-definition -Wl,--stack,8388608 -static endif # STATIC=1 produces a fully static binary (for Alpine/musl portable builds) diff --git a/scripts/test-windows.ps1 b/scripts/test-windows.ps1 index 89600921..2c0cf344 100644 --- a/scripts/test-windows.ps1 +++ b/scripts/test-windows.ps1 @@ -16,19 +16,20 @@ * test_non_ascii_path.py guards #636/#357 (fixed by #700) * test_hook_augment.py guards #618 (fixed by #619) * test_ui_drive_listing.py guards #548 (roots field) + * test_cli_non_ascii_arg.py guards #423/#20 (wide-argv main()) KNOWN REDS - genuine, still-open Windows bugs reproduced at the product surface. They are EXPECTED to be RED (exit 1) and are opt-in (never gate CI). If one turns GREEN the underlying bug was fixed and it should be promoted to a guard. - * test_cli_non_ascii_arg.py reproduces #423/#20 (narrow - argv main() - no wide command line) + * (none currently - test_cli_non_ascii_arg.py was promoted to a + guard when the wide-argv fix for #423/#20 landed) - Determinism: indexing runs in-process (CBM_INDEX_SUPERVISOR=0). These tests - exercise path / hook / drive handling, not the index-supervisor subprocess - path; the pass-level readers (#700's cbm_fopen routing) run in-process either - way, so the guard coverage is identical while results stay independent of the - local toolchain's worker-spawn behavior. + Determinism: the runner sets CBM_INDEX_SUPERVISOR=0 so the path / hook / drive + guards index in-process (the pass-level readers under test, e.g. #700's cbm_fopen + routing, run in-process either way). The non-ASCII CLI guard is the exception - it + drops that override to cross the real supervisor -> worker spawn, where the second + half of #423/#20 lives (CreateProcessW delivering the wide command line). On native Windows the MinGW/LLVM toolchain ships no libasan/libubsan, so the build disables sanitizers (SANITIZE=). Where the toolchain provides @@ -104,14 +105,14 @@ $env:CBM_INDEX_SUPERVISOR = "0" # in-process indexing (see .DESCRIPTION) $guards = @( "tests\windows\test_non_ascii_path.py", "tests\windows\test_hook_augment.py", - "tests\windows\test_ui_drive_listing.py" -) - -# Opt-in known-red repros - EXPECTED red (exit 1); never gate CI. -$knownReds = @( + "tests\windows\test_ui_drive_listing.py", "tests\windows\test_cli_non_ascii_arg.py" ) +# Opt-in known-red repros - EXPECTED red (exit 1); never gate CI. Currently empty: +# test_cli_non_ascii_arg.py was promoted to a guard when #423/#20's wide-argv fix landed. +$knownReds = @() + $guardFailures = @() $guardSkips = @() $fixedKeepers = @() @@ -159,7 +160,7 @@ if ($fixedKeepers.Count -gt 0) { } if ($guardFailures.Count -gt 0) { Write-Host ("REGRESSION: {0} green guard(s) went red: {1}" -f $guardFailures.Count, ($guardFailures -join ", ")) -ForegroundColor Red - Write-Host "A previously-fixed Windows bug is broken again. See tests/windows/RED_TEST_ANALYSIS.md." -ForegroundColor Red + Write-Host "A previously-fixed Windows bug is broken again (see the guard's docstring and its referenced issue)." -ForegroundColor Red exit 1 } Write-Host "All Windows green guards passed." -ForegroundColor Green diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index 61730c24..4f272649 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -13,6 +13,9 @@ #ifdef _WIN32 #include +#include "win_utf8.h" /* cbm_utf8_to_wide — spawn the worker with a wide command line so a + * non-ASCII repo path survives CreateProcess (#423/#20) */ +#include /* free */ #else #include #include @@ -234,9 +237,20 @@ static int cbm_run_win(const cbm_proc_opts_t *opts, cbm_proc_result_t *out) { out->term_signal = 0; return -1; } + /* Spawn via CreateProcessW with a WIDE command line. CreateProcessA would + * re-interpret our UTF-8 cmdline bytes through the ANSI code page (CP_ACP), + * re-mangling a non-ASCII repo path at the parent->worker boundary — so the + * worker's own wide-argv read could never recover it (#423/#20). */ + wchar_t *wcmd = cbm_utf8_to_wide(cmdline); + if (!wcmd) { + out->outcome = CBM_PROC_SPAWN_FAILED; + out->exit_code = -1; + out->term_signal = 0; + return -1; + } HANDLE hlog = INVALID_HANDLE_VALUE; - STARTUPINFOA si = {.cb = sizeof(si)}; + STARTUPINFOW si = {.cb = sizeof(si)}; if (opts->log_file) { hlog = CreateFileA(opts->log_file, GENERIC_WRITE, FILE_SHARE_READ, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); @@ -248,7 +262,8 @@ static int cbm_run_win(const cbm_proc_opts_t *opts, cbm_proc_result_t *out) { } PROCESS_INFORMATION pi = {0}; - BOOL ok = CreateProcessA(NULL, cmdline, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); + BOOL ok = CreateProcessW(NULL, wcmd, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); + free(wcmd); if (hlog != INVALID_HANDLE_VALUE) { CloseHandle(hlog); } diff --git a/src/main.c b/src/main.c index 97adaf15..d533b224 100644 --- a/src/main.c +++ b/src/main.c @@ -40,6 +40,10 @@ enum { #include "foundation/compat_thread.h" #include "foundation/mem.h" #include "foundation/profile.h" +#include "foundation/win_utf8.h" /* cbm_wide_to_utf8 — Windows UTF-8 argv (#423/#20); no-op on POSIX */ +#ifdef _WIN32 +#include /* CommandLineToArgvW — not pulled in by windows.h under WIN32_LEAN_AND_MEAN */ +#endif #include "ui/config.h" #include "ui/http_server.h" #include "ui/embedded_assets.h" @@ -593,6 +597,48 @@ static void setup_signal_handlers(void) { #endif } +#ifdef _WIN32 +/* On Windows the CRT hands main() an argv encoded in the active ANSI code page, so a + * non-ASCII CLI argument (e.g. a repo path like café_日本語_repo) is mangled before the + * program ever sees it — the documented `cli index_repository ""` then fails with + * "repo_path is required" (#423/#20). Rebuild argv from the wide command line + * (GetCommandLineW → CommandLineToArgvW) and convert each element to UTF-8 so the rest + * of the program receives the same UTF-8 bytes it gets on POSIX. Returns a + * NULL-terminated argv and sets *out_argc, or NULL on any failure (caller then keeps + * the original narrow argv). The returned block lives for the whole process (argv must + * stay valid until exit), so it is intentionally never freed. */ +static char **cbm_win_utf8_argv(int *out_argc) { + int wargc = 0; + LPWSTR *wargv = CommandLineToArgvW(GetCommandLineW(), &wargc); + if (!wargv) { + return NULL; + } + if (wargc <= 0) { + LocalFree(wargv); + return NULL; + } + char **u8argv = (char **)calloc((size_t)wargc + 1, sizeof(char *)); + if (!u8argv) { + LocalFree(wargv); + return NULL; + } + for (int i = 0; i < wargc; i++) { + u8argv[i] = cbm_wide_to_utf8(wargv[i]); + if (!u8argv[i]) { + for (int j = 0; j < i; j++) { + free(u8argv[j]); + } + free(u8argv); + LocalFree(wargv); + return NULL; + } + } + LocalFree(wargv); + *out_argc = wargc; + return u8argv; /* NULL-terminated (calloc'd wargc+1) */ +} +#endif /* _WIN32 */ + int main(int argc, char **argv) { /* Defense-in-depth: bind tree-sitter and sqlite3 to mimalloc so a * correct binary does not rely on the fragile MI_OVERRIDE symbol override @@ -601,6 +647,20 @@ int main(int argc, char **argv) { * below opens sqlite early), else sqlite3_config returns SQLITE_MISUSE and * the bind is silently ignored. No-op in the test build. */ cbm_alloc_init(); +#ifdef _WIN32 + /* Replace the ANSI-code-page argv the CRT handed us with a UTF-8 argv rebuilt from + * the wide command line, so non-ASCII CLI arguments survive (#423/#20). Falls back + * to the original argv if the wide rebuild fails. Done after cbm_alloc_init (which + * must stay the very first statement) but before argv is first read below. */ + { + int win_argc = 0; + char **win_argv = cbm_win_utf8_argv(&win_argc); + if (win_argv) { + argc = win_argc; + argv = win_argv; + } + } +#endif /* #845: mark this process as the REAL binary so the index supervisor may * wrap index_repository in a worker subprocess. Must run before any * subcommand dispatch so MCP-server, CLI, and HTTP paths are all covered. diff --git a/src/ui/http_server.c b/src/ui/http_server.c index aef72aef..f23e36f4 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -34,6 +34,7 @@ #include "foundation/str_util.h" #include "foundation/compat_thread.h" #include "foundation/subprocess.h" /* cbm_build_win_cmdline — shared MS-CRT arg quoting */ +#include "foundation/win_utf8.h" /* cbm_utf8_to_wide — CreateProcessW wide cmdline (#423/#20) */ #include #include @@ -972,19 +973,30 @@ static void *index_thread_fn(void *arg) { atomic_store(&job->status, 3); return NULL; } + /* Wide command line: CreateProcessA would re-mangle the UTF-8 repo path through the + * ANSI code page at the spawn boundary, so a non-ASCII repo path never reaches the + * worker intact (#423/#20). Convert and spawn via CreateProcessW. */ + wchar_t *wcmd = cbm_utf8_to_wide(cmdline); + if (!wcmd) { + snprintf(job->error_msg, sizeof(job->error_msg), "index command line conversion failed"); + atomic_store(&job->status, 3); + return NULL; + } cbm_log_info("ui.index.spawn", "bin", bin, "log", log_file); HANDLE hlog = CreateFileA(log_file, GENERIC_WRITE, FILE_SHARE_READ, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); - STARTUPINFOA si_proc = {.cb = sizeof(si_proc)}; + STARTUPINFOW si_proc = {.cb = sizeof(si_proc)}; if (hlog != INVALID_HANDLE_VALUE) { si_proc.dwFlags = STARTF_USESTDHANDLES; si_proc.hStdError = hlog; si_proc.hStdOutput = hlog; } PROCESS_INFORMATION pi = {0}; - if (!CreateProcessA(NULL, cmdline, NULL, NULL, TRUE, 0, NULL, NULL, &si_proc, &pi)) { + BOOL spawned = CreateProcessW(NULL, wcmd, NULL, NULL, TRUE, 0, NULL, NULL, &si_proc, &pi); + free(wcmd); + if (!spawned) { snprintf(job->error_msg, sizeof(job->error_msg), "CreateProcess failed"); atomic_store(&job->status, 3); if (hlog != INVALID_HANDLE_VALUE) diff --git a/tests/windows/RED_TEST_ANALYSIS.md b/tests/windows/RED_TEST_ANALYSIS.md deleted file mode 100644 index 8c06f655..00000000 --- a/tests/windows/RED_TEST_ANALYSIS.md +++ /dev/null @@ -1,322 +0,0 @@ -# Windows Test Analysis - -Deterministic, Windows-only integration tests found during a native-Windows -red-test campaign. They drive the product surface (a real `codebase-memory-mcp.exe` -over stdio / CLI / HTTP UI, real SQLite DB) and pass on Linux/macOS. - -Three of the four originally-red findings have since been fixed on `main` and are -kept here as **green regression guards** (they go red again only if the fix -regresses). The fourth is a genuine, still-open Windows bug kept as a **known red**. - -| Test | Issue | Status | -|---|---|---| -| `test_non_ascii_path.py` | #636 / #357 | GREEN guard - fixed by #700 (`cbm_fopen` routing in the pass readers) | -| `test_hook_augment.py` | #618 | GREEN guard - fixed by #619 (`cbm_is_walkable_abs_path` accepts `X:/`) | -| `test_ui_drive_listing.py` | #548 | GREEN guard - fixed (drives exposed via the `roots` field) | -| `test_cli_non_ascii_arg.py` | #423 / #20 | RED (open) - `main()` is still narrow-argv, no wide command line | - -The three green guards are wired into CI via the `test-windows-guards` job in -`.github/workflows/_test.yml` (build the product+UI binary, run the guards with -`-GuardsOnly`) so #700 / #619 / #548 stay enforced. The known red is opt-in and -never gates CI. **This PR contains no production fixes.** - -## Environment - -- OS: Microsoft Windows 11 Pro, build 10.0.26200 -- Source build: MinGW-w64 GCC 15.2.0 (MSYS2), `make -f Makefile.cbm cbm-with-ui` -- Filesystem: NTFS, code page 65001 (UTF-8 console); drives `C:`, `D:`, `E:` -- Shells/launchers exercised: PowerShell 5.1 (5.1.26100), `cmd.exe`, - Git Bash (MSYS2), direct Win32 process launch, Python `subprocess.Popen`, - Python stdio (line-delimited JSON-RPC) transport -- Findings first captured at `b075f05`; re-verified after rebasing onto current - `main` (this is where the three now-fixed cases were confirmed green) -- Binary: `build/c/codebase-memory-mcp.exe` (product build, with embedded UI) - -### Determinism note (index supervisor) - -The guards drive indexing in-process via the `CBM_INDEX_SUPERVISOR=0` kill switch -(set by `scripts/test-windows.ps1`). The passes under test (`#700`'s `cbm_fopen` -routing) run in-process either way, so guard coverage is identical, while results -stay independent of the index-supervisor's separate worker process (whose spawn -behavior varies by local toolchain). The drive-picker guard does not index at all. - -### Sanitizer note - -The MinGW/LLVM toolchain available on this machine ships **no** `libasan` / -`libubsan`, so an AddressSanitizer/UBSan build is not possible natively. These are -product-level integration tests that drive a real `codebase-memory-mcp.exe`; the -sanitizer C suite is a separate concern (the `test-windows` CI job / `scripts/test.sh`). - -## How to run - -```powershell -# Builds build/c/codebase-memory-mcp.exe (with UI) if missing, then runs the suite. -pwsh -File scripts/test-windows.ps1 -# only the green guards (the CI gate): -pwsh -File scripts/test-windows.ps1 -GuardsOnly -# or, against an installed/relocated binary: -pwsh -File scripts/test-windows.ps1 -Binary "C:\path\to\codebase-memory-mcp.exe" -``` - -Each test exits `0` (green), `1` (red), or `2` (precondition/setup). A guard that -exits `1` fails the runner (regression); a known red that exits `0` is flagged for -promotion. Standard-library Python 3 only. - ---- - -## windows_non_ascii_repo_path_preserves_definitions - -**Status: GREEN guard - fixed by #700.** The text below describes the original -red (at `b075f05`); the closing note records the landed fix. The "Actual" counts -are the pre-fix observation. - -- Class: integration (green regression guard) -- Test: `tests/windows/test_non_ascii_path.py` -- Related issues: #636, #357, #571 (naming), #530 -- Environment: Windows 11 26200, PowerShell 5.1 / Python stdio, NTFS, CP 65001 -- Fixture: byte-identical 2-file TypeScript repo (`src/math.ts`, `src/main.ts`), - copied to an ASCII parent path and to four non-ASCII parent paths - (Latin-1 accents `café`, Cyrillic `проект`, CJK `日本語`, Greek `Ωμέγα`) -- Expected: each non-ASCII copy produces the same graph counts as the ASCII - baseline (12 nodes / 20 edges / 5 definition nodes) -- Actual: every non-ASCII copy produces **5 nodes / 4 edges / 0 definition - nodes** — only `File`/`Folder` nodes; zero `Function`/`Class`/`Method` -- Command: `python tests/windows/test_non_ascii_path.py build\c\codebase-memory-mcp.exe` -- Minimal failure output: - - ``` - baseline (ASCII): nodes=12 edges=20 definitions=5 - [FAIL] non-ascii/latin1_accents nodes=5 edges=4 definitions=0 (baseline 12/20/5) - [FAIL] non-ascii/cyrillic nodes=5 edges=4 definitions=0 (baseline 12/20/5) - [FAIL] non-ascii/cjk nodes=5 edges=4 definitions=0 (baseline 12/20/5) - [FAIL] non-ascii/greek nodes=5 edges=4 definitions=0 (baseline 12/20/5) - ``` - -- Suspected implementation area: the per-pass source readers - `read_file()` in `src/pipeline/pass_definitions.c`, `pass_calls.c`, - `pass_parallel.c`, `pass_semantic.c` (and the `k8s`/`lsp_cross`/`pkgmap` - variants) open files with plain `fopen(path, "rb")`. On Windows `fopen` - interprets the UTF-8 path in the active **ANSI code page**, so a path with - non-ASCII bytes cannot be opened and the tree-sitter parser receives no bytes. - Directory discovery already uses the wide API - (`cbm_utf8_to_wide` + `FindFirstFileW` in `src/foundation/compat_fs.c`, - `src/foundation/platform.c`), which is why `File`/`Folder` nodes still appear - while all definitions vanish. Fix direction: route the pass-level reads through - the wide layer (`cbm_utf8_to_wide` + `_wfopen`), or add a shared - UTF-8-aware file reader and use it from every pass. - -Verified with `_wfopen` vs `fopen` on a non-ASCII path: `fopen(utf8, "rb")` -returns `NULL`, `_wfopen(cbm_utf8_to_wide(utf8), L"rb")` opens the same file. - -**Fix landed (#700):** the per-pass readers now go through `cbm_fopen`, which on -Windows converts the UTF-8 path to wide and calls `_wfopen` (`src/foundation/compat_fs.c`). -Re-verified green on current `main`: every non-ASCII variant now matches the ASCII -baseline (12 nodes / 22 edges / 5 definitions). This invariant also holds on -Linux/macOS (byte-transparent UTF-8 filesystem). - ---- - -## windows_cli_non_ascii_repo_path_is_honored - -**Status: RED (still open) - the keeper.** Re-verified on current `main`: -`main()` (`src/main.c`) is still `int main(int argc, char **argv)` with no -`wmain` / `GetCommandLineW`, so this remains genuinely red. Opt-in; not a CI gate. - -- Class: integration (known red) -- Test: `tests/windows/test_cli_non_ascii_arg.py` -- Related issues: #636, #423, #20 -- Environment: Windows 11 26200, `cli` argv path, NTFS, CP 65001 -- Fixture: a TypeScript repo under a non-ASCII directory (`café_日本語_repo`), - created with the OS wide API so it genuinely exists; an ASCII control repo -- Expected: `codebase-memory-mcp cli index_repository '{"repo_path":""}'` - indexes the directory (ASCII control proves the CLI path works) -- Actual: the ASCII control indexes; the non-ASCII invocation fails with - `repo_path is required` (the mangled, now-invalid-UTF-8 JSON argument is - rejected) and exits non-zero -- Command: `python tests/windows/test_cli_non_ascii_arg.py build\c\codebase-memory-mcp.exe` -- Minimal failure output: - - ``` - ASCII control: indexed OK - non-ASCII argv: rc=1 - stderr: ... repo_path is required - ``` - -- Suspected implementation area: `int main(int argc, char **argv)` in - `src/main.c` does not use `wmain` / `GetCommandLineW`, so on Windows the C - runtime delivers `argv` in the ANSI code page. The non-ASCII bytes in the JSON - argument are corrupted before `yyjson` parses them. Fix direction: read the - wide command line on Windows (`GetCommandLineW` + `CommandLineToArgvW`, or a - `wmain` entrypoint) and convert each argument to UTF-8. - -Real MCP clients pass `repo_path` inside a JSON-RPC message over stdio (which is -byte-clean), so this affects the documented `cli` entrypoint and the hook/install -flows that shell out to it, not the stdio server path. Holds on Linux/macOS -(argv is UTF-8 bytes). - ---- - -## windows_hook_augment_emits_context - -**Status: GREEN guard - fixed by #619.** The text below describes the original -red (at `b075f05`); the closing note records the landed fix. - -- Class: integration (green regression guard) -- Test: `tests/windows/test_hook_augment.py` -- Related issues: #618 -- Environment: Windows 11 26200, `hook-augment` CLI subcommand -- Fixture: a repo with a known function `someIndexedSymbol`, indexed; a realistic - Claude Code PreToolUse Grep payload with a Windows drive-letter `cwd` -- Expected: `codebase-memory-mcp hook-augment` emits a `hookSpecificOutput` with - `additionalContext` listing the matching graph symbol (the control - `search_graph` finds the symbol, so the index and project name are fine) -- Actual: `hook-augment` emits **empty stdout** for every payload -- Command: `python tests/windows/test_hook_augment.py build\c\codebase-memory-mcp.exe` -- Minimal failure output: - - ``` - control: search_graph finds someIndexedSymbol in project C-...-repo - hook-augment rc=0 stdout='' - ``` - -- Suspected implementation area: `src/cli/hook_augment.c` has two POSIX-only path - guards. `cbm_cmd_hook_augment` (`_WIN32` branch, ~L330): - `if (!cwd || cwd[0] != '/') { ...; return 0; }` and the `ha_resolve_and_query` - walk-up loop (~L254): `for (...; dir[0] == '/'; ...)`. A Windows `cwd` is a - drive-letter path (`C:\...` / `C:/...`), so `cwd[0]` is never `'/'`; the - augmenter bails before it queries the graph. The PreToolUse Grep/Glob graph - augmentation therefore never fires on Windows. - -**Fix landed (#619):** `hook_augment.c` now uses `cbm_is_walkable_abs_path`, which -accepts a drive-letter root (`X:/`) in addition to POSIX `/`, and the walk-up loop -climbs it. Re-verified green on current `main`: `hook-augment` emits the -`hookSpecificOutput` / `additionalContext` payload for a drive-letter `cwd`. Also -holds on Linux/macOS (`cwd` starts with `/`). - ---- - -## windows_ui_picker_reaches_all_drives - -**Status: GREEN guard - fixed.** The original red asserted drives appear in the -`dirs` array; the landed fix intentionally exposes them via a separate `roots` -field, so that assertion would stay red against fixed code. The test was -**rewritten** to guard the real invariant (every fixed drive is advertised in -`roots` and is browsable). The text below describes the original red; the closing -note records the fix and the rewrite. - -- Class: integration (green regression guard) -- Test: `tests/windows/test_ui_drive_listing.py` -- Related issues: #548 -- Environment: Windows 11 26200 with drives `C:\`, `D:\`, `E:\`; UI build - (`make -f Makefile.cbm cbm-with-ui`); embedded HTTP server on a local port -- Fixture: none — exercises the live `GET /api/browse` endpoint -- Expected: browsing the filesystem root (`/api/browse?path=/`) lets the user - reach every fixed drive (`D:\`, `E:\`), so a project on a non-system drive can - be selected -- Actual: the control browse of an explicit directory returns entries (endpoint - works), but `browse('/')` returns **0 entries** and no drive letters — `D:\` - and `E:\` are unreachable from the picker root -- Command: `python tests/windows/test_ui_drive_listing.py build\c\codebase-memory-mcp.exe` -- Minimal failure output: - - ``` - control browse('C:/Users/jacob') -> dirs(23) - browse('/') -> path='/' dirs(0)=[] - RED: drives ['D:\\', 'E:\\'] are not reachable from the UI root picker - ``` - -- Suspected implementation area: `handle_browse` in `src/ui/http_server.c` did - `opendir(path)` for the requested path and, for the root, listed only the - current drive's contents with no logical-drive enumeration. - -**Fix landed (#548):** `handle_browse` now calls `append_roots_json`, which on -Windows enumerates `GetLogicalDrives()` into a `"roots":["C:/","D:/",...]` array -appended to every `/api/browse` response (POSIX emits `"/"`). The rewritten guard -asserts every fixed drive is present in `roots` and that `GET /api/browse?path=X:/` -returns for it. Re-verified green on current `main` (drives `C:`/`D:`/`E:`): -`roots=['C:/','D:/','E:/']`, all reachable. - -This test requires a UI build because the HTTP server only starts when the -frontend is embedded; against a non-UI binary it reports a precondition (exit 2). -The `roots` check is meaningful even on a single-drive machine (the system drive -must be advertised and browsable), so it also gates on single-drive CI runners. -Holds on Linux/macOS (a single `/` root). - ---- - -## Seed areas revisited and ruled out (green on native Windows) - -Each was reproduced as a concrete attempt against the production binary and -behaved correctly — recorded as green and **not** included as a red test: - -| Area | Seed | Result on Windows | -|---|---|---| -| stdio `initialize` returns before stdin EOF; stdout flushes before EOF | #513, #530.1, #635 | green | -| `tools/list` non-empty; all 14 tools return valid JSON-RPC | #530 | green | -| `get_code_snippet` on a CP949 file emits valid UTF-8 (invalid bytes → U+FFFD) | #530.3 | green | -| Indexing a mapped (subst) drive `W:\` — no `bad_root_path`/`store.corrupt`, DB kept | #227, #367 | green (subst; real SMB not testable here) | -| Client exit terminates the server process (no residual `.exe`) | #185, #406 | green | -| `--help` / `--version` exit 0 in PowerShell, cmd, Git Bash | — | green | -| `search_code` works without bash/GNU grep (PowerShell `Select-String`) | #422, #348 | green | -| `.gitignore` and `.cbmignore` honored | #274 | green | -| `detect_changes` reports real changed files across commits | #371, #137 | green | -| `query_graph` shapes (counts, paths, labels) — no crash/disconnect | #627 | green | -| Paths with spaces, `&`, `()`, `[]`, `#`, `%`, `!`, apostrophe | #272 | green | -| Mixed slash/backslash and lower-case drive letters | #133 | green | -| Non-UTF-8 (CP949) source file emits valid UTF-8 JSON; no crash | #511 | green | -| Re-index is idempotent (counts stable, single project) | #140 | green | -| Index never escapes the selected root | #331 | green | -| Every JSON-RPC response decodes as strict UTF-8 | invariant | green | - -## Observed but intentionally out of scope for this PR - -- **Project-name collision for non-ASCII paths (#571/#20).** Two distinct repos - (`проект`, `日本語`) under the same parent derive the *same* project name, - because `cbm_project_name_from_path` (`src/pipeline/fqn.c`) maps every - non-`[A-Za-z0-9._-]` byte to `-` and then trims. This is a real bug but it is - **not Windows-specific** — `cbm_project_name_from_path` is platform-independent - and collides identically on Linux. Per the campaign rules it is recorded here - and left for a cross-platform PR. -- **Paths longer than 260 characters.** This machine has - `HKLM\SYSTEM\CurrentControlSet\Control\FileSystem\LongPathsEnabled = 0`, so - paths over `MAX_PATH` are unreachable by every application, not just CBM. - CBM could opt in via the `\\?\` prefix + wide APIs, but the failure is gated by - a machine-wide policy rather than a clean CBM-only defect, so it is excluded. -- **Cascading nested `.gitignore` (#530.2) and `.git/info/exclude` (#530.5).** - `try_load_nested_gitignore` in `src/discover/discover.c` skips nested - `.gitignore` files once a parent ignore is loaded, and discovery never reads - `.git/info/exclude`. Both are real, but the discovery logic is - platform-independent and reproduces identically on Linux, so they are out of - scope for a Windows-only PR. -- **libgit2 1.8+ build break (#530.4).** `git_allocator` moved to - ``; cross-platform compile issue, not a Windows runtime bug. -- **Windows umbrella tracker (#394).** This is a meta-issue ("8 bugs"); its - remaining open children are the mapped/SMB-drive class (#227, #367), covered in - the ruled-out table above (a `subst` mapped drive indexes and keeps its DB; a - real SMB share is not available here). Its other children (#221, #266, #274, - #331, #347, #348) are already marked fixed upstream, so no new test is shipped. -- **Memory growth over hours (#581).** Requires a multi-hour soak to surface and - is not deterministic in a unit/integration test; the existing - `scripts/soak-test.sh` RSS-trend harness is the right vehicle and is not - reproduced as a red test here. -- **C `test-runner` failures on Windows.** The in-process C suite reports many - extraction-count failures concentrated in `test_grammar_probe_*`, - `test_node_creation_probe`, `test_edge_*`, `test_matrix_*`, and - `test_integration.c` (e.g. `integ_index_has_files` finds 0 files even for an - **ASCII** fixture). The production binary indexes those same ASCII/CRLF cases - correctly (CRLF vs LF source files were verified to extract identically), so - these look like in-process test-harness issues rather than user-facing product - regressions. Distinguishing genuine Windows-only product regressions from - fixture/harness sensitivity requires a Linux baseline of the same commit and is - left as a follow-up; they are deliberately **not** converted into red tests - here to avoid shipping undiagnosed assertions. - -## Stop-condition coverage - -- Shells/launchers covered: PowerShell 5.1, `cmd.exe`, Git Bash, direct Win32, - Python `subprocess`, Python stdio JSON-RPC (>= 3 required). -- Classes covered in the green streak: smoke, integration, unit (the passing - `build/c/test-runner` cases), invariant. -- Seed areas (Unicode paths, mapped-drive/UNC, stdio, `search_code`, - install/update, watcher/ignore, query, memory/process lifecycle) were each - revisited or explicitly ruled out above. diff --git a/tests/windows/test_cli_non_ascii_arg.py b/tests/windows/test_cli_non_ascii_arg.py index 1aafad18..959ab747 100644 --- a/tests/windows/test_cli_non_ascii_arg.py +++ b/tests/windows/test_cli_non_ascii_arg.py @@ -1,23 +1,24 @@ -"""RED integration test — `cli index_repository` rejects a non-ASCII repo_path. +"""GREEN regression guard — `cli index_repository` honors a non-ASCII repo_path. -Reproduces the CLI-argv half of issue #636 / #423 / #20 on native Windows. +Guards the CLI-argv fix for issue #636 / #423 / #20 on native Windows. The documented entrypoint `codebase-memory-mcp cli index_repository ''` -receives its JSON argument through argv. main() is declared as -`int main(int argc, char **argv)` (src/main.c) — it does not use wmain / -GetCommandLineW — so on Windows the C runtime hands it argv in the active ANSI -code page. A repo_path containing non-ASCII characters is therefore mangled (or, -when yyjson rejects the now-invalid UTF-8, the whole argument is discarded), and -the command fails with "repo_path is required" / "Pipeline failed" instead of -indexing the real directory. +receives its JSON argument through argv. main() used to take only the narrow +`int main(int argc, char **argv)` (src/main.c), so on Windows the C runtime handed +it argv in the active ANSI code page: a repo_path containing non-ASCII characters +was mangled (or, when yyjson rejected the now-invalid UTF-8, the whole argument was +discarded), and the command failed with "repo_path is required" / "Pipeline failed" +instead of indexing the real directory. + +Fixed: on Windows main() now rebuilds argv from the wide command line +(GetCommandLineW + CommandLineToArgvW) and converts each element to UTF-8, so a +non-ASCII repo path survives. This test asserts that fix stays in place — it was RED +before it and is GREEN after. (It is inherently green on Linux/macOS, where argv is +already UTF-8 bytes.) The directory itself is created with the Windows wide API (Python uses CreateFileW/_wmkdir under the hood), so it genuinely exists on disk; only the -argv path delivery is lossy. - -Passes on Linux/macOS (argv is UTF-8 bytes). Fails on native Windows until the -CLI reads the wide command line (GetCommandLineW + CommandLineToArgvW, or a -wmain entrypoint) and converts to UTF-8. +argv path delivery was lossy. Exit code: 0 == honored (green), 1 == rejected/mangled (red), 2 == setup error. @@ -80,6 +81,13 @@ def main(): env2 = dict(os.environ) env2["CBM_CACHE_DIR"] = cache + # Exercise the DEFAULT (supervisor-enabled) path, not in-process. The non-ASCII + # repo path must survive BOTH the argv read (main() wide command line) AND the + # supervisor -> worker spawn (CreateProcessW). The suite runner sets + # CBM_INDEX_SUPERVISOR=0 for determinism across the other guards; forcing it OFF + # here would run in-process and mask the spawn-boundary half of #423/#20, so we + # drop the override and let the supervisor wrap the worker as it does for users. + env2.pop("CBM_INDEX_SUPERVISOR", None) arg = json.dumps({"repo_path": repo}, ensure_ascii=False) p = subprocess.run([binary, "cli", "index_repository", arg], capture_output=True, timeout=120, env=env2) @@ -87,14 +95,15 @@ def main(): err = (p.stderr or b"").decode("utf-8", "replace") honored = '"nodes"' in out and '"nodes":0' not in out.replace(" ", "") print("ASCII control: indexed OK") - print("non-ASCII argv: rc=%d" % p.returncode) + print("non-ASCII argv (supervised): rc=%d" % p.returncode) print(" stdout: %s" % out[:200].replace("\n", " ")) print(" stderr: %s" % err[-200:].replace("\n", " ")) if honored: - print("\nGREEN: CLI honored the non-ASCII repo_path.") + print("\nGREEN: CLI honored the non-ASCII repo_path (argv + worker spawn).") return 0 - print("\nRED: CLI did not index the non-ASCII repo_path (argv delivered " - "in the ANSI code page; main() does not read the wide command line).") + print("\nRED: CLI did not index the non-ASCII repo_path — the path was mangled " + "either in the argv read (narrow main()) or re-mangled at the " + "supervisor->worker CreateProcess boundary (ANSI code page).") return 1 finally: shutil.rmtree(work, ignore_errors=True)