Skip to content

test(windows): add red regressions for platform-specific failures#639

Merged
DeusData merged 4 commits into
DeusData:mainfrom
Flipper1994:windows-red-tests-20260627
Jul 4, 2026
Merged

test(windows): add red regressions for platform-specific failures#639
DeusData merged 4 commits into
DeusData:mainfrom
Flipper1994:windows-red-tests-20260627

Conversation

@Flipper1994

@Flipper1994 Flipper1994 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Windows-only integration tests that drive the product surface (a real
codebase-memory-mcp.exe over 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 four
originally-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 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 via the roots field); test rewritten
test_cli_non_ascii_arg.py #423 / #20 RED (open)main() is still narrow-argv, no wide command line

Re-verified on this Windows host (11 Pro 26200, drives C:/D:/E:): the three
guards pass green, the keeper stays red.

What changed since the first review

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/Folder nodes for non-ASCII
paths (the pass readers fopen()'d a UTF-8 path in the ANSI code page); #700
routed 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 emit hookSpecificOutput
for a drive-letter cwd. Before #619 it bailed on cwd[0] != '/'; #619 added
cbm_is_walkable_abs_path (accepts X:/). 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_browse
now appends "roots":["C:/","D:/",...] (via GetLogicalDrives) to every
/api/browse response. 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 / #20

cli index_repository rejects a non-ASCII repo_path ("repo_path is required")
because main() (src/main.c) is int main(int argc, char **argv) with no
wmain / GetCommandLineW, so on Windows argv arrives in the ANSI code page.
Opt-in only; never gates CI. Real MCP clients pass repo_path inside byte-clean
JSON-RPC over stdio, so this affects the documented cli entrypoint and the
hook/install flows that shell out to it.

How to run

pwsh -File scripts/test-windows.ps1            # build (with UI) + guards + known-reds
pwsh -File scripts/test-windows.ps1 -GuardsOnly # only the green guards (the CI gate)

Each test exits 0 (green), 1 (red), or 2 (precondition/setup). Full
environment, commands, ruled-out seed areas, and fix references are in
tests/windows/RED_TEST_ANALYSIS.md.

Runner scope / isolation

  • The three green guards are wired into Windows CI (the test-windows-guards job)
    so the landed fixes stay enforced; the opt-in known-red is never run in CI.
  • No production changes — the diff is limited to tests/windows/,
    scripts/test-windows.ps1, and the CI job in .github/workflows/_test.yml.
  • The harness launches only the locally built codebase-memory-mcp.exe (JSON-RPC
    stdio / 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.

@DeusData

Copy link
Copy Markdown
Owner

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.

@DeusData DeusData added bug Something isn't working windows Windows-specific issues priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jun 29, 2026
@DeusData

DeusData commented Jul 1, 2026

Copy link
Copy Markdown
Owner

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.

@Flipper1994

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback — both points are addressed:

  • Attribution: removed the generated/session footer from the PR description.
  • Runner scope: added a "Runner scope / isolation" section to the description spelling out that the runner is opt-in (scripts/test-windows.ps1 only), is referenced by no Makefile / make test target / .github/ workflow, ships no production changes, and only launches the locally built binary and the project's own loopback HTTP UI (no network, temp-only writes).

Happy to adjust the runner further if you'd like it gated or relocated differently.

@DeusData

DeusData commented Jul 3, 2026

Copy link
Copy Markdown
Owner

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!

Flipper1994 and others added 4 commits July 4, 2026 12:59
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>
@Flipper1994 Flipper1994 force-pushed the windows-red-tests-20260627 branch from 1b65aa2 to 0eb2c58 Compare July 4, 2026 11:23
@Flipper1994 Flipper1994 requested a review from DeusData as a code owner July 4, 2026 11:23
@Flipper1994

Copy link
Copy Markdown
Contributor Author

Thanks — this was exactly right, and I've reworked the PR accordingly.

Rebased onto current main (only .gitignore conflicted). Re-ran on Windows (11 Pro, drives C:/D:/E:) and confirmed your read on all four:

One implementation note: the runner sets CBM_INDEX_SUPERVISOR=0 so the guards index in-process — the passes under test (#700's cbm_fopen) run in-process either way, and it keeps the guards independent of the index-supervisor's separate worker process. Happy to switch to the supervised path if you'd prefer.

Refreshed RED_TEST_ANALYSIS.md to record the landed fixes. Thanks again for the careful review.

@DeusData DeusData merged commit 7388921 into DeusData:main Jul 4, 2026
15 checks passed
@DeusData

DeusData commented Jul 4, 2026

Copy link
Copy Markdown
Owner

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority/normal Standard review queue; useful PR with ordinary maintainer urgency. windows Windows-specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: File parsing fails silently on Windows when project path contains accented/non-ASCII characters

2 participants