test(windows): add red regressions for platform-specific failures#639
Conversation
|
Huge thanks for opening this PR and for the work you put into it. The maintainer shop is currently full, so this may sit for a bit before it gets a proper review. We will come back to this as soon as possible with real feedback; I wanted to make sure it did not sit unacknowledged in the meantime. |
|
Thanks for putting these Windows repros together. Before this can proceed, please remove generated/session attribution from the PR text and make sure the red-test runner remains outside the normal green test path. Since the harness launches local binaries and an HTTP server, maintainers will review the runner scope carefully. |
|
Thanks for the feedback — both points are addressed:
Happy to adjust the runner further if you'd like it gated or relocated differently. |
|
Thank you for this — the mcp_stdio.py harness fills a real gap (we've wanted a Windows product-binary harness for a while), and the tests are deterministic and well-engineered. The catch: the branch was cut at b075f05 and main has moved — 3 of the 4 reds are stale: the hook-augment case was fixed by #619 (issue #618 is closed), the non-ASCII pass readers were fixed by #700 (cbm_fopen everywhere), and the drive-listing test asserts drives appear in the dirs array while the landed fix intentionally exposes them via the roots field — so it stays red against fixed code, which is as misleading as a false green. The keeper is test_cli_non_ascii_arg.py: main() is still narrow argv with no wide entry point, so that red is genuine. Could you: rebase + re-run on Windows; convert the two now-green tests into permanent green guards (ideally wired into the windows CI job so #619/#700 stay enforced); rewrite the drive-listing test against the roots-field/user-level invariant; and refresh or drop the analysis doc (it snapshots claims that are no longer true of main)? With that, this becomes a great addition. Thanks! |
Adds Windows-only red tests and analysis for native Windows failures found during a Windows red-test campaign. This change contains no production fixes. - windows_non_ascii_repo_path_preserves_definitions (integration): byte-identical TypeScript fixtures indexed under non-ASCII parent paths (Latin-1 accents, Cyrillic, CJK, Greek) extract zero definitions and only File/Folder nodes (5 nodes / 4 edges) versus the ASCII baseline (12 nodes / 20 edges / 5 definitions). The pipeline source readers open files with fopen() on a UTF-8 path, which the Windows CRT interprets in the active ANSI code page; directory discovery already uses the wide API, so files are listed but never parsed. - windows_cli_non_ascii_repo_path_is_honored (integration): the documented `cli index_repository` entrypoint rejects a non-ASCII repo_path because main() does not read the wide command line, so argv arrives in the ANSI code page. Both reproduce at the product surface (real MCP process, real stdio, real SQLite DB), are deterministic, and pass on Linux/macOS. A PowerShell runner builds the binary and runs the suite; standard-library Python only. See tests/windows/RED_TEST_ANALYSIS.md for environment, commands, ruled-out seed areas, and suspected fix locations. Signed-off-by: Flipper <jacobphilipp@ymail.com>
…cker Extends the Windows red-test suite with two more reproduced, Windows-specific failures. No production fixes. - windows_hook_augment_emits_context (integration, DeusData#618): the PreToolUse Grep/Glob augmenter `hook-augment` emits empty stdout for every payload on Windows. src/cli/hook_augment.c gates on POSIX-style absolute paths (cwd[0] == '/' and the walk-up loop's dir[0] == '/'), which a Windows drive-letter cwd never satisfies, so the graph augmentation never fires. A control search_graph confirms the symbol is indexed. - windows_ui_picker_reaches_all_drives (integration, DeusData#548): the UI directory picker's GET /api/browse?path=/ returns no entries and never enumerates logical drives, so drives other than the system drive (D:\, E:\) cannot be selected. handle_browse in src/ui/http_server.c uses opendir without a GetLogicalDriveStrings root case. Needs a UI build (cbm-with-ui) and >1 drive; otherwise it reports a precondition (exit 2). Also records additional ruled-out seed areas (get_code_snippet sanitizes non-UTF-8 to U+FFFD DeusData#530.3; stdio handshake/flush works DeusData#513/DeusData#530.1/DeusData#635; mapped subst-drive indexing keeps the DB DeusData#227/DeusData#367) and cross-platform items left out of this Windows-only PR (DeusData#530.2 nested gitignore, DeusData#530.5 .git/info/exclude, DeusData#530.4 libgit2 build, DeusData#581 memory soak). Signed-off-by: Flipper <jacobphilipp@ymail.com>
Reference the Windows umbrella issue DeusData#394 in the analysis so every open Windows-relevant issue is accounted for: its open children (DeusData#227/DeusData#367, the mapped/SMB-drive class) are in the ruled-out table and its other children are already fixed upstream. No test or production change. Signed-off-by: Flipper <jacobphilipp@ymail.com>
…rive test Rebased onto current main and reworked in response to review. Three of the four Windows reds were fixed upstream since the branch was cut at b075f05, so they are now green regression guards; the fourth stays a genuine known-red. - test_non_ascii_path.py (DeusData#636/DeusData#357): green guard - fixed by DeusData#700 (per-pass readers now route through cbm_fopen -> _wfopen). Re-verified green on main. - test_hook_augment.py (DeusData#618): green guard - fixed by DeusData#619 (cbm_is_walkable_abs_path accepts drive-letter X:/ cwd). Re-verified green on main. - test_ui_drive_listing.py (DeusData#548): rewritten. The fix exposes drives via a new roots[] field, not the dirs[] array the old test asserted (which would stay red against fixed code). Now asserts every fixed drive is in roots and browsable. Re-verified green on main (drives C:/D:/E:). - test_cli_non_ascii_arg.py (DeusData#423/DeusData#20): unchanged - main() is still narrow-argv with no wide command line, so this remains genuinely red (the keeper). - scripts/test-windows.ps1: split green guards (gate CI) from opt-in known-reds; add -GuardsOnly; run indexing in-process (CBM_INDEX_SUPERVISOR=0) so a guard reflects the path/hook/drive fix under test, not the index-worker spawn path. - .github/workflows/_test.yml: new test-windows-guards job builds the product+UI binary (scripts/build.sh --with-ui) and runs the guards with -GuardsOnly so DeusData#700/DeusData#619/DeusData#548 stay enforced on Windows CI. - RED_TEST_ANALYSIS.md: refreshed to record the landed fixes and current status. Signed-off-by: Flipper <jacobphilipp@ymail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1b65aa2 to
0eb2c58
Compare
|
Thanks — this was exactly right, and I've reworked the PR accordingly. Rebased onto current
One implementation note: the runner sets Refreshed |
|
Thank you — and thanks especially for the rebase: converting the three now-fixed reds into green regression guards (non-ASCII paths via #703, hook-augment via #619, drive-listing via the roots API) while keeping the CLI-argv case as an opt-in known-red is exactly the right shape for a platform-guard board. Verified each against main. Merged as 7388921; the new Windows-guards CI job is a welcome addition. Two small follow-ups on our side: the non-ASCII fix is #703 (not #700), and #423/#20 need reopening since the CLI narrow-argv bug your known-red guards is genuinely still present. Appreciated! |
Summary
Windows-only integration tests that drive the product surface (a real
codebase-memory-mcp.exeover JSON-RPC stdio / CLI / HTTP UI, real SQLite DB),found during a native-Windows red-test campaign. Deterministic, standard-library
Python 3 only, and pass on Linux/macOS.
Updated after review + rebased onto current
main. Three of the fouroriginally-red findings were fixed upstream since the branch was cut at
b075f05,so they are now green regression guards; the fourth is a genuine, still-open
Windows bug kept as a known red. This PR still contains no production fixes.
test_non_ascii_path.pycbm_fopenrouting in the pass readers)test_hook_augment.pycbm_is_walkable_abs_pathacceptsX:/)test_ui_drive_listing.pyrootsfield); test rewrittentest_cli_non_ascii_arg.pymain()is still narrow-argv, no wide command lineRe-verified on this Windows host (11 Pro 26200, drives
C:/D:/E:): the threeguards pass green, the keeper stays red.
What changed since the first review
main(the only conflict was.gitignore; resolved).test_non_ascii_path.py/test_hook_augment.pykept as-is functionally(they already pass on fixed code) and re-framed as green guards for Bug: File parsing fails silently on Windows when project path contains accented/non-ASCII characters #700 / fix(hook): make hook-augment work on Windows drive-letter paths #619.
test_ui_drive_listing.pyrewritten. The D:\ drive and custom path cannot be selected in server UI #548 fix intentionally exposesdrives through a new
rootsarray, not thedirsarray the original testasserted — so the old assertion would stay red against fixed code. It now asserts
every fixed drive appears in
rootsand is browsable (GET /api/browse?path=X:/).Meaningful on a single-drive machine too, so it also gates single-drive CI.
test_cli_non_ascii_arg.pyunchanged — still genuinely red (main()narrow argv).scripts/test-windows.ps1now splits green guards (must stay green, gateCI) from opt-in known-reds (expected red, never gate CI); adds
-GuardsOnly.Indexing runs in-process (
CBM_INDEX_SUPERVISOR=0) so a guard reflects thepath/hook/drive fix under test, not the orthogonal index-supervisor worker path.
.github/workflows/_test.yml): newtest-windows-guardsjob buildsthe product+UI binary (
scripts/build.sh --with-ui) and runs the guards with-GuardsOnly, so Bug: File parsing fails silently on Windows when project path contains accented/non-ASCII characters #700 / fix(hook): make hook-augment work on Windows drive-letter paths #619 / D:\ drive and custom path cannot be selected in server UI #548 stay enforced on Windows. This is separatefrom the existing
test-windowssanitizer C-suite job.RED_TEST_ANALYSIS.mdrefreshed to record the landed fixes and status.Guards (fixed on main, enforced here)
test_non_ascii_path.py— #636 / #357 (fixed by #700)Byte-identical TypeScript fixtures under non-ASCII parent paths (Latin-1 accents,
Cyrillic, CJK, Greek) must extract the same definitions as the ASCII baseline.
Before #700, native Windows extracted only
File/Foldernodes for non-ASCIIpaths (the pass readers
fopen()'d a UTF-8 path in the ANSI code page); #700routed those reads through
cbm_fopen→_wfopen. Guard fails if that regresses.test_hook_augment.py— #618 (fixed by #619)hook-augment(the PreToolUse Grep/Glob augmenter) must emithookSpecificOutputfor a drive-letter
cwd. Before #619 it bailed oncwd[0] != '/'; #619 addedcbm_is_walkable_abs_path(acceptsX:/). Guard fails if that regresses.test_ui_drive_listing.py— #548 (fixed)The UI directory picker must let the user reach every logical drive.
handle_browsenow appends
"roots":["C:/","D:/",...](viaGetLogicalDrives) to every/api/browseresponse. Needs a UI build (cbm-with-ui), which the CI job builds.Known red (still open, opt-in)
test_cli_non_ascii_arg.py— #423 / #20cli index_repositoryrejects a non-ASCIIrepo_path("repo_path is required")because
main()(src/main.c) isint main(int argc, char **argv)with nowmain/GetCommandLineW, so on Windows argv arrives in the ANSI code page.Opt-in only; never gates CI. Real MCP clients pass
repo_pathinside byte-cleanJSON-RPC over stdio, so this affects the documented
clientrypoint and thehook/install flows that shell out to it.
How to run
Each test exits
0(green),1(red), or2(precondition/setup). Fullenvironment, commands, ruled-out seed areas, and fix references are in
tests/windows/RED_TEST_ANALYSIS.md.Runner scope / isolation
test-windows-guardsjob)so the landed fixes stay enforced; the opt-in known-red is never run in CI.
tests/windows/,scripts/test-windows.ps1, and the CI job in.github/workflows/_test.yml.codebase-memory-mcp.exe(JSON-RPCstdio / CLI) and, for the drive-picker guard, the project's own embedded HTTP UI
bound to loopback; it makes no network calls and writes only to temp dirs.
Related issues
Guards fixes: #700 (→ #636/#357), #619 (→ #618), #548. Reproduces (open): #423,
#20. Also documents ruled-out seed areas (#513/#530.x/#635, #422/#348, #274,
#371/#137, #627, #272, #511, #331, #185/#406, #227/#367) and cross-platform
out-of-scope items (#571, #530.2/.4/.5, #394 umbrella) in the analysis.