Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions .github/workflows/_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
5 changes: 3 additions & 2 deletions Makefile.cbm
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 14 additions & 13 deletions scripts/test-windows.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = @()
Expand Down Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions src/foundation/subprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

#ifdef _WIN32
#include <windows.h>
#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 <stdlib.h> /* free */
#else
#include <errno.h>
#include <fcntl.h>
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
60 changes: 60 additions & 0 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 <shellapi.h> /* 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"
Expand Down Expand Up @@ -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 "<json>"` 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
Expand All @@ -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.
Expand Down
16 changes: 14 additions & 2 deletions src/ui/http_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 <sqlite3/sqlite3.h>
#include <yyjson/yyjson.h>
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading