Windows native port submission prep - #6
Open
nsxdavid wants to merge 26 commits into
Open
Conversation
Teach the 0.16 compat shim to back every primitive with the native API on Windows while POSIX keeps its existing libc/pthread paths: - argv: windowsArgv flattens WTF-16 command lines into the posix-shaped C-string argv the rest of the program uses - raw io: writeRaw/readRaw dispatch to _write/_read (msvcrt) vs write/read - locks: Mutex/RwLock select SRWLOCK on Windows (zero-init, real shared/exclusive semantics) and pthread on POSIX - time: GetSystemTimeAsFileTime wall clock; QueryPerformanceCounter monotonic clock with the constant frequency queried once and cached - env: getenv/_putenv_s with a HOME -> USERPROFILE fallback, plus a shared userHome() helper for the per-user cache root - subprocess: std.process.run capture path and a CreateProcessW DETACHED_PROCESS + CREATE_NO_WINDOW detached spawn for the cli-daemon - mapFileRead/unmapFileRead: mmap on POSIX, aligned heap read on Windows
main() converts the WTF-16 command line once via cio.windowsArgv; everything downstream keeps consuming posix-shaped argv. The warm CLI daemon gets a Windows transport equivalent to the per-project Unix socket: a per-project, per-user named pipe. One CliConn type (HANDLE on Windows, fd on POSIX) backs shared framing, serve, respond, and proxy paths, so the wire protocol lives in exactly one place; only connect/accept/close differ per platform. Pipe endpoint hardening, matching the socket path's chmod 0600 + stale-node handling: - owner-only protected DACL (SDDL D:P(A;;GA;;;OW)), fail closed - FILE_FLAG_FIRST_PIPE_INSTANCE so a daemon that lost the creation race exits instead of serving alongside the winner - clients pass SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION so a pipe server can identify but never impersonate them - username hashed into the pipe name (parity with uid in the socket path) The MCP idle watchdog skips the POSIX stdin POLLHUP probe on Windows and relies on the timed loop; socket unlink-on-exit is POSIX-only since a named pipe is a kernel object with no filesystem node.
…tcher - Replace direct std.posix.mmap/munmap with cio.mapFileRead/unmapFileRead (mmap on POSIX, aligned heap read on Windows); snapshot content sections allocate with the Explorer allocator that owns the adopted section and unmap with that same allocator on the rejection path - Open the hot-log/notify files with read access in one createFile call: Windows requires read access on the handle for length(), and .truncate = false opens-or-creates without the close/reopen dance - Honor native path separators where realPathFile returns them, keeping '/' in indexed relative paths for cross-platform codedb output - Route remaining timestamps and env access through cio
Project data dirs and the central snapshot cache derive their root from cio.userHome() (HOME, with the USERPROFILE fallback on native Windows) instead of reading HOME directly, so the per-user cache works from stock Windows shells that never set HOME. Central-cache tests share one setTestHome helper to point HOME inside the tmp dir.
- Skip getGitHead and symlink-cycle walker tests on native Windows: the former shells out to git (recurses through this checkout's tool plumbing and exhausts the DebugAllocator under the Windows test runner), the latter needs a reparse-point-aware walker; POSIX keeps both covered - Heap-allocate the large watcher EventQueue in tests so the smaller default Windows test-thread stack exercises the same queue behavior - test_mcp subprocess builds pin --global-cache-dir .zig-global-cache and use platform-correct exe paths/line endings; the unicode-root test exercises the named-pipe daemon end to end
Rollback inserted snapshot entries before releasing adopted backing buffers and invalidate cached call graph state when outlines mutate.
Keep plain search on the fast trigram path when the word index is incomplete, while reserving space for restored skip-trigram files. Also block native Windows temp roots unless temp indexing is explicitly allowed.
processId() returned a stub 0 on Windows, silently breaking nuke's self-pid guard and randU64's entropy mix; use GetCurrentProcessId. Move the HOME->USERPROFILE fallback out of posixGetenv into userHome() and switch all HOME readers (linter_pref, nuke, root_policy, update, mcp projects, main getDataDir) to it. Dedupe env-var NUL-termination into a zTerm helper.
format=json callers of codedb_search/codedb_symbol got plain-text
errors (and a stray arg-keys diagnostic in the JSON path); emit the
ok=false/tool/error{code,message} envelope instead. Consolidate the
per-type appendJsonKey{Usize,U8,F32} writers into appendJsonKeyNum,
extract searchResultVisible, and add round-trip schema tests that run
the real dispatch path and parse the responses.
Extract rejectExtraArgs for the arity-zero commands (tree/hot/status), fold the per-platform read/write bodies of cliReadFull/cliWriteFull into connRead/connWrite, and drop the hand-rolled SECURITY_ATTRIBUTES, CloseHandle, GetLastError, and ERROR_PIPE_CONNECTED declarations in favor of std.os.windows equivalents.
Replace the manual free-everything-on-each-failure ladder in the call graph build with an errdefer-based inner function, hoist the symbol max_results cap checks to loop tops, and reuse std.fs.path.extension / resolveRelativeImportPath in pathExtension and resolveDartImport.
The issue-537/537b/539/539b load-path tests each hand-rolled the same tmp-dir snapshot write + load sequence; share one helper.
Extract the CreateProcessW command-line builder from spawnDetachedWindows into pub cio.windowsCommandLine so it is testable, and add a FAILING test: an arg ending in a backslash (any drive-root project path like D:\) is emitted as "D:\" whose trailing \" escapes the closing quote under CommandLineToArgvW rules, fusing the arg with whatever follows in the spawned child's argv. Known-failing on Windows until the quoting bug is fixed; rename issue-XX once the issue is filed.
This reverts commit 5ab20bd.
This reverts commit cb455e0.
…CommandLine Double backslashes before a closing quote (2N) and before embedded quotes (2N+1) so args ending in '\' (e.g. drive-root paths like D:\) no longer fuse with the following argument. Extend the round-trip test with embedded quotes, internal double backslashes, and trailing backslash runs, and promote it from issue-XX to a named windows test.
… envelopes
- Add toolOutputIsError so telemetry, bundle fail counts, and CLI exit
codes recognize the {"ok":false,...} envelope, not just "error:" text
- Emit writeJsonToolError envelopes on regex and plain-search failures
when format=json was requested
- Skip the renderPlainSearch fast path under format=json so results
come back as JSON instead of plain text
- Include the symbol body in codedb_symbol JSON output when body=true
…geRank edge weights - searchSymbols: collect all candidates before sorting instead of capping during hash-map iteration, so the cap keeps the best-scored results; free the entries trimmed after the sort - findCallPath: dupe step path/name into the caller's allocator since the call graph's backing strings can be invalidated once the shared lock is released; free them at the callpath handler - pageRank: count out_weight only for edges with valid destinations, matching the propagation loop, so valid targets aren't under-weighted
Returning null on a malformed word/trigram index skipped the errdefer that unmaps the file, leaking the mapping; return error.MalformedIndex instead. Also move loadSnapshotFast's call-graph invalidation past header/section validation, so an early-rejected snapshot leaves the existing graph intact while the first real mutation still invalidates.
… --allow-temp Cold `search` (no snapshot) now enables the word index during the initial scan and marks it complete, so Tier-0 BM25 recall works on the first run instead of only after `index`. Add the missing `callpath` subcommand and `--allow-temp` option to the usage text.
…help tests Add EnvVarGuard/TestHomeGuard so tests that override HOME, USERPROFILE, CODEDB_ALLOW_TEMP, or CODEDB_CLI_DAEMON_IDLE_MS restore the original values instead of unsetting them for the rest of the test process. Use builtCodedbExe() in the --help/-h tests so they run on Windows. Reword a CHANGELOG line to avoid a bare trailing `import ` in backticks.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nsxdavid
marked this pull request as ready for review
June 12, 2026 01:48
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 185f8c39eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
nsxdavid
pushed a commit
that referenced
this pull request
Jul 12, 2026
#3 #6) Three behavior-preserving optimizations on the per-query search/rank path, identified by the perf audit and adversarially verified: #1 (mcp.zig, appendSearchSymbolNudge): the bare-identifier nudge did a full O(U+S) symbol scan (searchSymbols scans the whole symbol_index AND outlines every call) just to test existence. Switched to findAllSymbols — the same helper codedb_symbol uses: ensureSymbolIndex + O(1) symbol_index.get, with the outline scan only when !symbol_index_complete. Output byte-identical (the nudge only checks results.len and prints the query string). #3 (explore.zig, BM25 posting loop): dropped the doc_tf hashmap. doc_best_line already accumulates .count identically per posting, so df = doc_best_line.count() and tf = entry.value_ptr.count — one map instead of two, and the redundant per-doc .get() re-lookup is gone. Values bit-identical. #6 (explore.zig, tier-1 sort): the comparator hashed 4 strings + touched 2 atomics per comparison. Precompute (count, len) per candidate once in O(C) (mirroring tier0_order), then sort a struct-key array — identical predicate (count desc, len asc) and input order, so ranking is unchanged. Also stops the comparator corrupting ContentCache hit/miss stats. zig build test: full suite green (0 failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
zig build --global-cache-dir .zig-global-cache test --summary allPYTHONIOENCODING=utf-8 python scripts/e2e_mcp_test.py --binary zig-out/bin/codedb.exe --project .Notes
This PR targets a fork-local base branch,
codex/release-0.2.5825-base, which mirrorsupstream/release/0.2.5825. It is intended as a CodeRabbit/submission-prep review surface before opening anything upstream.