Windows native port: pipe-transport CLI daemon, JSON tool surface, cleanup pass

[nsxdavid]

What

Native Windows port branch plus a quality/cleanup pass and new test coverage:

• Windows port: named-pipe CLI daemon transport, cio platform shims (mmap/locks/env/spawn), per-user central cache via cio.userHome
• Cleanup pass: deduped arg validation, conn IO helpers, errdefer call-graph ownership, JSON emit helpers consolidated
• JSON wire fixes: format=json error envelopes for codedb_search/codedb_symbol were invalid JSON (trailing diagnostic text after the closing brace); now schema-stable with tests that parse the real dispatch output
• Windows pid fix: cio.processId() returned 0 on Windows; now GetCurrentProcessId
• Repo hygiene: ignore local .codex-tools/, drop committed machine-specific codedb.snapshot

Known-failing test (intentional)

test_mcp.zig contains issue-XX: spawnDetached command line round-trips argv with trailing backslashes — a deliberately failing test documenting a real quoting bug in cio.windowsCommandLine (args ending in \, e.g. drive-root project paths, fuse with the following arg under CommandLineToArgvW rules). Fix to follow in a separate change.

Verification

• zig build test: 738/746 pass, 7 skipped, 1 intentional failure (above)
• E2E on Windows: built exe, drove CLI (tree/hot/status/search/symbol + arg-rejection probes), MCP stdio with format=json (valid payloads + all three error envelopes), warm daemon proxy over named pipe, clean exit on stdin EOF

Summary by CodeRabbit

• New Features

  • Call-path querying with graph-based ranking, a callpath tool/CLI, richer symbol search filters, and structured JSON output for tools.

• Bug Fixes

  • Restored search/recall for snapshot-loaded files; fixes to dependency/import parsing and search/indexing edge cases; stricter CLI argument validation and error handling.

• Documentation

  • Updated release notes and agent/testing docs for 0.2.5825.

• Chores

  • Windows portability, installer/daemon robustness, temp-indexing opt-in, and safer startup/daemon behaviors.

• Tests

  • Large expansion of regression tests and new validator scripts.

[coderabbitai]

Actionable comments posted: 19

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/mcp.zig (1)

1593-1600: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Return JSON errors for invalid max_results when format=json is requested.

codedb_search still emits plain-text (error: ...) for max_results <= 0, which breaks the format=json response contract for structured clients.

💡 Minimal fix

@@
-    if (getInt(args, "max_results")) |n| {
+    const json_fmt = wantsJsonFormat(args);
+    if (getInt(args, "max_results")) |n| {
         if (n <= 0) {
-            const w_err = cio.listWriter(out, alloc);
-            w_err.print("error: max_results ({d}) must be >= 1", .{n}) catch {};
+            if (json_fmt) {
+                var msg_buf: [96]u8 = undefined;
+                const msg = std.fmt.bufPrint(&msg_buf, "max_results ({d}) must be >= 1", .{n}) catch "max_results must be >= 1";
+                writeJsonToolError(out, alloc, "codedb_search", "invalid_max_results", msg);
+            } else {
+                const w_err = cio.listWriter(out, alloc);
+                w_err.print("error: max_results ({d}) must be >= 1", .{n}) catch {};
+            }
             return;
         }
     }
@@
-    const json_fmt = wantsJsonFormat(args);

Also applies to: 1623-1623

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mcp.zig` around lines 1593 - 1600, The plain-text error emitted when
getInt(args, "max_results") returns n <= 0 should instead return a JSON error
when the client requested JSON output: detect the requested format (check args
for "format" == "json" or use the same format-detection logic elsewhere in
codedb_search), and when true write a JSON error object (e.g.
{"error":"max_results must be >= 1","details":n}) to out using the same JSON
writer used by codedb_search; otherwise keep the existing plain-text
cio.listWriter path. Apply the identical change for the second occurrence noted
around the other max_results check (the block near line ~1623) so both places
respect the format=json contract.

src/explore.zig (1)

2212-2223: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard intermediate dupes until ownership transfers into the array.

Both allocation sequences build owned strings before the final append, but the cleanup only covers elements that are already in the list. If a later dupe or the append itself fails, the earlier temporary allocations leak. Add local errdefer cleanup for each newly-duped string until the append succeeds.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/explore.zig` around lines 2212 - 2223, The code dupes multiple strings
(path, sym.name, sym.detail) before calling list_ptr.append, but if any
subsequent dup or the append fails those earlier allocations leak; wrap each
alloc.dupe call with an errdefer that frees that duplicated buffer (using the
same allocator) so the temp is cleaned up on error, and cancel the corresponding
errdefer(s) after list_ptr.append succeeds; specifically, when creating the
temporary duped values used to build the appended element (the duped path, the
duped name, and the optional duped detail), add an errdefer cleanup for each and
call the defer cancel/disable right after append returns successfully to
transfer ownership into list_ptr.append (refer to Dedup.contains, alloc.dupe
calls and list_ptr.append and the symbol fields sym.name/sym.detail).

src/index.zig (1)

2455-2463: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Treat a failed base/overlay candidate build as null, not a partial result.

mergeOverlayCandidates currently returns whichever side succeeded when the other side returned null. In overlay mode, null from MmapTrigramIndex.candidates* or TrigramIndex.candidates* is the allocation-failure signal, so this converts OOM into a partial candidate set and can drop valid matches instead of forcing the caller back to the full scan.

💡 Minimal fix

     fn mergeOverlayCandidates(
         mo: *const MmapOverlay,
         base: ?[]const []const u8,
         over: ?[]const []const u8,
         allocator: std.mem.Allocator,
     ) ?[]const []const u8 {
-        if (base == null and over == null) return null;
-        if (base == null) return over;
+        _ = mo;
+        if (base == null or over == null) {
+            if (base) |b| allocator.free(b);
+            if (over) |o| allocator.free(o);
+            return null;
+        }
         var merged = std.StringHashMap(void).init(allocator);
         defer merged.deinit();
         var failed = false;
         for (base.?) |p| {
-            if (mo.masked.contains(p)) continue;
+            if (mo.masked.contains(p)) continue;
             merged.put(p, {}) catch {
                 failed = true;
                 break;
             };
         }

Also applies to: 2467-2507, 2509-2517

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/index.zig` around lines 2455 - 2463, When building candidates in overlay
mode (AnyTrigramIndex.candidates -> .mmap_overlay) do not treat a null return
from mo.base.candidates or mo.overlay.candidates as a partial success; instead,
if either base or over is null (allocation failure), return null immediately and
only call mergeOverlayCandidates when both succeeded. Update the .mmap_overlay
branch and the similar blocks handled around lines 2467-2507 and 2509-2517 so
they check both results and propagate null on any failure before attempting to
merge.

🧹 Nitpick comments (1)

src/test_mcp.zig (1)

2071-2073: ⚡ Quick win

Use an arena allocator for new Explorer-based tests.

These new Explorer.init(...) test blocks use testing.allocator; in src/test_*.zig, Explorer tests should use std.heap.ArenaAllocator and defer arena.deinit().

As per coding guidelines: src/test_*.zig: "Use std.heap.ArenaAllocator for Explorer tests in Zig".

Also applies to: 2105-2108, 2148-2151, 2190-2193, 2229-2232, 2355-2361

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test_mcp.zig` around lines 2071 - 2073, Replace uses of testing.allocator
when initializing Explorer with a std.heap.ArenaAllocator: create an arena via
std.heap.ArenaAllocator.init(&testing.allocator) (or similar), pass
arena.allocator() into Explorer.init instead of testing.allocator, and add defer
arena.deinit() before deferring explorer.deinit(); update the Explorer.init
calls referenced (e.g., Explorer.init(...)) and corresponding defer lines so
each test uses the arena allocator and deinitializes the arena.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bench/swe-lite/validators/validate_bandit.py`:
- Around line 33-46: The temp directory created by tempfile.mkdtemp() (variable
d) is never removed, causing temp file leaks; change the _scan() flow to ensure
the directory is cleaned up after use (e.g., wrap the
write/discover/run/get_issue sequence in a try/finally or switch to
tempfile.TemporaryDirectory) and call shutil.rmtree(d) (or rely on the context
manager) after mgr.get_issue_list()/metrics access; reference variables and
functions: d, p, mgr.discover_files(...), mgr.run_tests(), mgr.get_issue_list(),
and totals = mgr.metrics.data["_totals"] so the cleanup always runs even if
run_tests() or get_issue_list() raises.

In `@bench/swe-lite/validators/validate_httpx.py`:
- Around line 241-244: The test currently passes a generator to httpx.Response
via content= which causes HTTPX to buffer the body at Response initialization;
change the constructor to use stream= with an httpx.ByteStream wrapper so the
chunks from _streaming_body are consumed only when iterating (e.g.,
httpx.Response(..., stream=httpx.ByteStream(_streaming_body([...]))), ensuring
iter_json() exercises streaming and that a subsequent iteration raises
httpx.StreamConsumed as expected; update the instantiation that currently calls
httpx.Response(200, headers={...}, content=stream) to use stream= instead.

In `@bench/swe-lite/validators/validate_kombu.py`:
- Around line 412-413: The code is calling Kombu private internals
ch._inplace_augment_message and _size and mutating
msg["properties"]["delivery_info"]["queue"]; replace those with public APIs:
construct the test message as a kombu.message.Message (or use
kombu.message.Message(body, delivery_info=..., properties=...)) and set the
queue name in its public delivery_info/properties instead of mutating internals,
and compute message byte-size using a public serializer (e.g.,
kombu.serialization.dumps followed by len(...) of the serialized payload) rather
than calling _size; update usages around _inplace_augment_message,
msg["properties"]["delivery_info"]["queue"], and _size to use these public
alternatives.

In `@bench/swe-lite/validators/validate_sqliteutils.py`:
- Around line 272-276: The validator currently treats any non-dict return from
db.import_csv(..., safe_mode=True, strict=False) and db.import_json(...,
safe_mode=True, strict=False) as success; change the logic so you require the
documented safe-mode envelope: for the db.import_csv result (variable ok_dict)
only set ok_dict = True when isinstance(res, dict) and res.get("success") is
True, otherwise False; apply the same strict check to the db.import_json result
(variable ok_json) so legacy/non-compliant non-dict returns do not pass
validation.
- Around line 47-56: The current get_exc() silently falls back to Exception
which hides missing API types; change get_exc to fail loudly when none of the
requested names exist (e.g., raise ImportError or RuntimeError with a clear
message referencing the requested names) so that assigning
SafeImportNotEnabledError, CheckpointNotActiveError, and CheckpointNotFoundError
will error if the implementation doesn't provide them; update references to
get_exc("SafeImportNotEnabledError"), get_exc("CheckpointNotActiveError"), and
get_exc("CheckpointNotFoundError") to rely on the new behavior (or perform an
explicit None check and raise) so the validator fails instead of silently using
Exception.
- Around line 327-337: The test currently only checks exit code and presence of
generic words; update it to also assert the failing invariant's identifier is
printed by capturing the id returned from d.add_import_invariant (call to
d.add_import_invariant("t", "SELECT COUNT(*) = 999") — store its return value,
e.g. inv_id) and then after running runner.invoke(cli_mod.cli,
["validate-import-invariants", dbpath, "t"]) check that (r.output or "").lower()
contains that inv_id (or a formatted form like f"{inv_id}" or f"{inv_id}"
prefixed by table if CLI prints "t:{id}"), and incorporate that boolean into
results["checks"]["cli_validate_invariants_exit0"] so the test fails if the CLI
omits the failing-invariant identifier.

In `@bench/swe-lite/validators/validate_vulture.py`:
- Around line 177-200: The tests only assert "at least one" scanned/reused path
but must require exact path coverage; update the checks around Vulture.scavenge
and _cache_stats so that stats1["scanned"] == set(paths) and stats1["reused"] ==
set() for the first run, and for the second run assert stats2["reused"] ==
set(paths) and stats2["scanned"] == set() (apply the same exact-set assertions
to the later analogous block at lines ~299-308); keep using getattr(vN,
"_cache_stats", None), ensure ok_stats_shape/ok2 still validate types before
comparing equality, and set C["first_run_scans_files"] and
C["second_run_reuses_unchanged"] based on those exact-set comparisons.
- Around line 255-256: The validator currently sets C["corrupt_cache_warns"] by
searching a combined variable (combined = (r.stderr or "") + (r.stdout or "")),
which can hide whether the message was actually written to stderr; change the
check to look only at r.stderr (e.g. test "cache is corrupted or unreadable" in
r.stderr or (r.stderr or "")) when assigning C["corrupt_cache_warns"]; apply the
same change to the other occurrence that sets C["corrupt_cache_warns"] on lines
~279-280 so both checks validate stderr only.

In `@src/explore.zig`:
- Around line 1580-1602: ensureSymbolIndex currently flips symbol_index_complete
back to true and calls rebuildSymbolIndexFor(..., false) which only appends,
leaving old entries and causing duplicates/stale results; before iterating
outlines in ensureSymbolIndex, acquire the same exclusive lock and clear the
existing symbol index so the rebuild starts fresh (e.g., reset or call the
symbol_index clear/reset routine) and then call rebuildSymbolIndexFor for each
outline; reference functions/fields: ensureSymbolIndex,
markSymbolIndexIncomplete, symbol_index, symbol_index_complete, and
rebuildSymbolIndexFor.
- Around line 818-823: setRoot currently overwrites self.root_dir and self.io
without closing any existing Dir, leaking the previous directory handle; modify
setRoot (in Explorer) to, before assigning the new root_dir/io, check if
self.root_dir != null and call close on that Dir handle (handle or ignore the
error), set it to null, then open the new dir and update self.io; keep the
existing logic that frees/dupes self.root_path and ensure deinit still closes
whatever root_dir remains.
- Around line 7059-7074: The function appendPythonModuleDep currently silently
truncates long mod_path into the fixed 512-byte stack buffer and still appends
".py", corrupting dependency keys; instead, detect when the input path
(converted with '.'->'/') plus the ".py" suffix would exceed the buffer and fail
explicitly (return an error) or allocate a sufficiently-sized buffer before
writing. Update appendPythonModuleDep to check if (mod_path.len + 3) > buf.len
(or compute required length after replacing '.' with '/') and if so return a
descriptive error instead of truncating; keep the call to
appendImportPath(outline, buf[0..pos]) unchanged when the path fits. Ensure the
error propagates from appendPythonModuleDep (it already returns !void) so
callers can handle the failure.

In `@src/main.zig`:
- Around line 1321-1350: The early "status" fast path bypasses the argument
validation in runQuery and rejectExtraArgs, letting "codedb status typo"
succeed; ensure the zero-arg contract by invoking the same validation before
taking the fast return: call rejectExtraArgs (the same function used by
runQuery) for the "status" branch (or otherwise validate there that no extra
args exist) before calling getDataDir/index_mod.readStatusMeta and exiting, so
the shortcut enforces the same no-arg rule as the normal status handler.
- Around line 2224-2228: The command parser misclassifies "bench-engine" because
isCommand() builds its commands list from cli_query_cmds plus an explicit array
that currently omits "bench-engine", so parsePositional() never treats args[1]
as a command; fix by adding "bench-engine" to the explicit array in isCommand()
(or ensure cli_query_cmds includes it) so that isCommand("bench-engine") returns
true and parsePositional() dispatches to the bench-engine handler.
- Around line 798-805: daemonLockTryAcquire currently constructs an ANSI byte
path and calls CreateFileA on Windows which fails for non-ASCII user paths;
change the Windows branch to convert the UTF-8 path (built from data_dir) to a
null-terminated UTF-16 (wide) buffer and call CreateFileW with that wide pointer
(then check for INVALID_HANDLE_VALUE and return the handle as before), or
explicitly document and perform an ANSI code-page conversion if you
intentionally want CreateFileA; refer to the function daemonLockTryAcquire and
the Windows calls CreateFileA/CreateFileW when making the change.

In `@src/snapshot.zig`:
- Around line 913-921: The rollback defer is calling explorer.removeFile(path)
where path is a borrowed slice that can alias the heap_section/CMS CONTENT
buffer which is freed earlier by the heap_section defer; fix by ensuring
inserted_paths stores owned copies instead of borrowed slices (or move the
rollback defer to run before heap_section defer). Specifically, change the code
paths that append into inserted_paths to duplicate the path (e.g.,
allocate/duplicate the string into the snapshot/explorer allocator) so that
entries in inserted_paths outlive heap_section/CONTENT, and keep the existing
rollback defer that calls explorer.removeFile(path) and the symbols load_ok,
inserted_paths, heap_section, explorer.removeFile, releaseOutlineSectionsFrom
and releaseContentSectionsFrom intact.
- Around line 1331-1373: isSensitivePath currently only splits on '/' and only
searches for UNIX-style directory tokens, which lets Windows paths (backslashes)
bypass checks; update isSensitivePath to treat both '/' and '\\' as path
separators when computing basename (e.g., use lastIndexOf for both or a helper
that finds the last of '/' or '\\'), and update directory-pattern checks
(std.mem.indexOf(u8, path, ".ssh/") etc.) to also look for the backslash
variants (".ssh\\", ".gnupg\\", ".aws\\") or check for the token followed by
either separator; keep the existing basename exact-name and extension checks but
run them on the normalized basename extracted using the new separator logic so
Windows paths are correctly flagged.

In `@src/store.zig`:
- Around line 99-117: appendVersion treats data_log as a multi-process file
(uses log.lock and re-stat) but compactDataLog currently rewrites/truncates
using only this process's self.files and can therefore drop bytes other
processes referenced; fix compactDataLog to acquire the same advisory lock on
data_log (same lock(...) path used by appendVersion) and while holding that
exclusive lock re-stat the file, scan all live version records across self.files
to compute the true highest referenced data_offset/length (not just this
process's view), rebuild the compacted log into a temporary file and atomically
replace/truncate the original only after verifying offsets against the
locked+re-statted end_pos, and update self.data_log_pos and any in-process
metadata consistently while still holding the lock so no concurrent append can
make offsets stale; ensure symbols touched include compactDataLog,
appendVersion, data_log, self.files, data_log_pos and log.lock/log.unlock.

In `@src/test_search.zig`:
- Around line 1815-1816: Replace direct use of testing.allocator when
initializing Explorer with a std.heap.ArenaAllocator instance: create an
ArenaAllocator (e.g. via std.heap.ArenaAllocator.init(testing.allocator)), pass
the arena's allocator to Explorer.init (instead of testing.allocator) for the
Explorer.DEFAULT_CONTENT_CACHE_CAPACITY calls, and ensure you deinit the arena
after the Explorer (mirror Explorer.deinit with arena.deinit/destroy as
appropriate). Update the three sites that call Explorer.init to use the arena
allocator and add the corresponding arena lifetime management adjacent to those
tests.
- Around line 1115-1132: Before you call
cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", "0") save the current environment
value (e.g., via cio.posixGetenv or std.os.getenv) into a local optional string,
and then in a defer restore it: if an original value existed call
cio.posixSetenv with that value, otherwise call cio.posixUnsetenv; do the same
pattern for the later block that calls cio.posixUnsetenv so the test always
restores the pre-test state of CODEDB_LEX_FREQ_PENALTY rather than permanently
unsetting it.

---

Outside diff comments:
In `@src/explore.zig`:
- Around line 2212-2223: The code dupes multiple strings (path, sym.name,
sym.detail) before calling list_ptr.append, but if any subsequent dup or the
append fails those earlier allocations leak; wrap each alloc.dupe call with an
errdefer that frees that duplicated buffer (using the same allocator) so the
temp is cleaned up on error, and cancel the corresponding errdefer(s) after
list_ptr.append succeeds; specifically, when creating the temporary duped values
used to build the appended element (the duped path, the duped name, and the
optional duped detail), add an errdefer cleanup for each and call the defer
cancel/disable right after append returns successfully to transfer ownership
into list_ptr.append (refer to Dedup.contains, alloc.dupe calls and
list_ptr.append and the symbol fields sym.name/sym.detail).

In `@src/index.zig`:
- Around line 2455-2463: When building candidates in overlay mode
(AnyTrigramIndex.candidates -> .mmap_overlay) do not treat a null return from
mo.base.candidates or mo.overlay.candidates as a partial success; instead, if
either base or over is null (allocation failure), return null immediately and
only call mergeOverlayCandidates when both succeeded. Update the .mmap_overlay
branch and the similar blocks handled around lines 2467-2507 and 2509-2517 so
they check both results and propagate null on any failure before attempting to
merge.

In `@src/mcp.zig`:
- Around line 1593-1600: The plain-text error emitted when getInt(args,
"max_results") returns n <= 0 should instead return a JSON error when the client
requested JSON output: detect the requested format (check args for "format" ==
"json" or use the same format-detection logic elsewhere in codedb_search), and
when true write a JSON error object (e.g. {"error":"max_results must be >=
1","details":n}) to out using the same JSON writer used by codedb_search;
otherwise keep the existing plain-text cio.listWriter path. Apply the identical
change for the second occurrence noted around the other max_results check (the
block near line ~1623) so both places respect the format=json contract.

---

Nitpick comments:
In `@src/test_mcp.zig`:
- Around line 2071-2073: Replace uses of testing.allocator when initializing
Explorer with a std.heap.ArenaAllocator: create an arena via
std.heap.ArenaAllocator.init(&testing.allocator) (or similar), pass
arena.allocator() into Explorer.init instead of testing.allocator, and add defer
arena.deinit() before deferring explorer.deinit(); update the Explorer.init
calls referenced (e.g., Explorer.init(...)) and corresponding defer lines so
each test uses the arena allocator and deinitializes the arena.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 89289cf1-087f-4e74-a955-d239bebd3e54

📥 Commits

Reviewing files that changed from the base of the PR and between 0d345ef and 185f8c3.

⛔ Files ignored due to path filters (1)

• experiments/ranking/scratch/direct.out is excluded by !**/*.out

📒 Files selected for processing (80)

• .gitignore
• CHANGELOG.md
• CLAUDE.md
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_baseline.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_codedb.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_codegraph.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_graphify.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_leanctx.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_baseline.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_codedb.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_codegraph.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_graphify.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_leanctx.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_baseline.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_codedb.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_codegraph.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_graphify.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_leanctx.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_baseline.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_codedb.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_codegraph.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_graphify.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_leanctx.json
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_baseline.json
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_codedb.json
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_codegraph.json
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_graphify.json
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_leanctx.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_baseline.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_codedb.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_codegraph.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_graphify.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_leanctx.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_baseline.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_codedb.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_codegraph.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_graphify.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_leanctx.json
• bench/swe-lite/validators/validate_bandit.py
• bench/swe-lite/validators/validate_cattrs.py
• bench/swe-lite/validators/validate_httpx.py
• bench/swe-lite/validators/validate_kombu.py
• bench/swe-lite/validators/validate_narwhals.py
• bench/swe-lite/validators/validate_sqliteutils.py
• bench/swe-lite/validators/validate_vulture.py
• codedb.snapshot
• docs/agents.md
• experiments/ranking/failed.md
• experiments/ranking/research.md
• experiments/ranking/scratch/direct.err
• experiments/ranking/scratch/step4a_out.txt
• experiments/ranking/scratch/step4a_time.txt
• experiments/ranking/scratch/step4c_out.txt
• experiments/ranking/scratch/step4c_time.txt
• experiments/ranking/todo.md
• src/cio.zig
• src/codegraph.zig
• src/explore.zig
• src/git.zig
• src/hot_cache.zig
• src/index.zig
• src/linter_pref.zig
• src/main.zig
• src/mcp.zig
• src/nuke.zig
• src/root_policy.zig
• src/snapshot.zig
• src/store.zig
• src/test_bench.zig
• src/test_core.zig
• src/test_explore.zig
• src/test_index.zig
• src/test_mcp.zig
• src/test_parser.zig
• src/test_query.zig
• src/test_search.zig
• src/test_snapshot.zig
• src/tests.zig
• src/update.zig
• src/watcher.zig

✅ Files skipped from review due to trivial changes (43)

• bench/swe-lite/validators/results/narwhals-rolling-window-suite_leanctx.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_codedb.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_leanctx.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_leanctx.json
• experiments/ranking/scratch/step4a_out.txt
• experiments/ranking/scratch/step4a_time.txt
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_graphify.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_leanctx.json
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_graphify.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_baseline.json
• src/test_bench.zig
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_baseline.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_graphify.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_codegraph.json
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_graphify.json
• experiments/ranking/scratch/step4c_time.txt
• experiments/ranking/scratch/direct.err
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_graphify.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_codegraph.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_baseline.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_graphify.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_codedb.json
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_baseline.json
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_codegraph.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_leanctx.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_leanctx.json
• CLAUDE.md
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_codegraph.json
• experiments/ranking/scratch/step4c_out.txt
• bench/swe-lite/validators/results/httpx-streaming-json-iteration_codegraph.json
• docs/agents.md
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_baseline.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_codedb.json
• bench/swe-lite/validators/results/vulture-persistent-analysis-cache_graphify.json
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_codegraph.json
• bench/swe-lite/validators/results/bandit-structured-nosec-directives_leanctx.json
• CHANGELOG.md
• bench/swe-lite/validators/results/kombu-virtual-queue-dead-lettering_codedb.json
• bench/swe-lite/validators/results/cattrs-partial-structuring-recovery_codedb.json
• .gitignore
• bench/swe-lite/validators/results/sqlite-utils-safe-import-checkpoints_codedb.json
• experiments/ranking/todo.md
• bench/swe-lite/validators/results/narwhals-rolling-window-suite_codedb.json

🚧 Files skipped from review as they are similar to previous changes (6)

• src/update.zig
• src/linter_pref.zig
• src/nuke.zig
• src/root_policy.zig
• src/codegraph.zig
• src/cio.zig

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clean up the temporary scan directory.

_scan() creates a fresh directory on every probe run and never removes it, so repeated validator runs will keep leaking files under the system temp root.

💡 Minimal fix

 def _scan(src, ignore_nosec=False):
     """Run a real bandit scan over `src`. Returns (issue_list, totals_dict)."""
     from bandit.core import config as b_config
     from bandit.core import manager as b_manager
     from bandit.core import test_set as b_test_set

-    d = tempfile.mkdtemp()
-    p = os.path.join(d, "probe.py")
-    with open(p, "w") as fh:
-        fh.write(src)
-
-    b_conf = b_config.BanditConfig()
-    mgr = b_manager.BanditManager(b_conf, "file")
-    mgr.b_ts = b_test_set.BanditTestSet(config=b_conf)
-    mgr.ignore_nosec = ignore_nosec
-    mgr.discover_files([p], True)
-    mgr.run_tests()
-    issues = mgr.get_issue_list()
-    totals = mgr.metrics.data["_totals"]
-    return issues, totals
+    with tempfile.TemporaryDirectory() as d:
+        p = os.path.join(d, "probe.py")
+        with open(p, "w", encoding="utf-8") as fh:
+            fh.write(src)
+
+        b_conf = b_config.BanditConfig()
+        mgr = b_manager.BanditManager(b_conf, "file")
+        mgr.b_ts = b_test_set.BanditTestSet(config=b_conf)
+        mgr.ignore_nosec = ignore_nosec
+        mgr.discover_files([p], True)
+        mgr.run_tests()
+        issues = mgr.get_issue_list()
+        totals = dict(mgr.metrics.data["_totals"])
+        return issues, totals

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	d = tempfile.mkdtemp()
	p = os.path.join(d, "probe.py")
	with open(p, "w") as fh:
	fh.write(src)
	b_conf = b_config.BanditConfig()
	mgr = b_manager.BanditManager(b_conf, "file")
	mgr.b_ts = b_test_set.BanditTestSet(config=b_conf)
	mgr.ignore_nosec = ignore_nosec
	mgr.discover_files([p], True)
	mgr.run_tests()
	issues = mgr.get_issue_list()
	totals = mgr.metrics.data["_totals"]
	return issues, totals
	with tempfile.TemporaryDirectory() as d:
	p = os.path.join(d, "probe.py")
	with open(p, "w", encoding="utf-8") as fh:
	fh.write(src)
	b_conf = b_config.BanditConfig()
	mgr = b_manager.BanditManager(b_conf, "file")
	mgr.b_ts = b_test_set.BanditTestSet(config=b_conf)
	mgr.ignore_nosec = ignore_nosec
	mgr.discover_files([p], True)
	mgr.run_tests()
	issues = mgr.get_issue_list()
	totals = dict(mgr.metrics.data["_totals"])
	return issues, totals

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bench/swe-lite/validators/validate_bandit.py` around lines 33 - 46, The temp
directory created by tempfile.mkdtemp() (variable d) is never removed, causing
temp file leaks; change the _scan() flow to ensure the directory is cleaned up
after use (e.g., wrap the write/discover/run/get_issue sequence in a try/finally
or switch to tempfile.TemporaryDirectory) and call shutil.rmtree(d) (or rely on
the context manager) after mgr.get_issue_list()/metrics access; reference
variables and functions: d, p, mgr.discover_files(...), mgr.run_tests(),
mgr.get_issue_list(), and totals = mgr.metrics.data["_totals"] so the cleanup
always runs even if run_tests() or get_issue_list() raises.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In current HTTPX versions, does httpx.Response(..., content=)create a true streaming response, or must streaming bodies be provided viastream= / a streaming byte-stream type?

💡 Result:

In current versions of HTTPX, providing a generator to httpx.Response(content=<generator>) does not create a true streaming response that remains unbuffered [1]. When you initialize an httpx.Response object with the content parameter, the library automatically processes and buffers the body [1]. Specifically, if the provided content is a generator (or any byte iterator), HTTPX converts it into a stream but then immediately calls self.read() on that stream, which loads the entire response body into memory [1]. This ensures that the response.content and response.text properties are immediately available after initialization [2][1]. Conversely, the stream parameter in httpx.Response is intended for low-level usage where you are manually passing an existing ByteStream (e.g., received from a transport layer) [1]. Using stream directly does not trigger the automatic read() call, allowing the body to remain unbuffered [1]. However, the official documentation notes that end-users generally do not need to use stream= directly; instead, they should use the httpx.stream() interface or client.stream() for streaming requests, which are designed to handle response streaming properly without manual byte-stream management [3][4][1].

Citations:

• 1: https://github.com/encode/httpx/blob/f13ab4d288d0b790f6f1c515a6c0ea45e9615748/httpx/_models.py
• 2: https://github.com/encode/httpx/blob/def4778d/tests/models/test_responses.py
• 3: https://github.com/encode/httpx/blob/master/docs/compatibility.md
• 4: https://www.python-httpx.org/compatibility/

---

Fix streaming test to avoid buffering: use stream= instead of content= (bench/swe-lite/validators/validate_httpx.py, lines 241-244)

Passing a generator via httpx.Response(..., content=generator) causes HTTPX to buffer/read the body during Response initialization, so the test may not exercise the real streaming-consumption path that iter_json() is intended to validate. Switch this to the unbuffered path by constructing Response with stream= using an httpx.ByteStream(...) wrapper around the chunk generator, so the stream is consumed on iteration and subsequent iteration raises StreamConsumed as expected.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bench/swe-lite/validators/validate_httpx.py` around lines 241 - 244, The test
currently passes a generator to httpx.Response via content= which causes HTTPX
to buffer the body at Response initialization; change the constructor to use
stream= with an httpx.ByteStream wrapper so the chunks from _streaming_body are
consumed only when iterating (e.g., httpx.Response(...,
stream=httpx.ByteStream(_streaming_body([...]))), ensuring iter_json() exercises
streaming and that a subsequent iteration raises httpx.StreamConsumed as
expected; update the instantiation that currently calls httpx.Response(200,
headers={...}, content=stream) to use stream= instead.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Are Kombu channel methods like _inplace_augment_messageand_size considered private implementation details that may vary across versions/transports, or are they supported/public APIs?

💡 Result:

In Kombu, method names prefixed with an underscore (such as _inplace_augment_message and _size) are strictly considered private implementation details [1][2][3]. They are not supported public APIs and are intended for internal use within the library [4][2]. Key points regarding these methods include: 1. Internal Naming Convention: Following standard Python practices, methods starting with an underscore are designated as internal. Their signatures, behaviors, and existence may change, be refactored, or be removed between versions or across different transport implementations (e.g., SQLAlchemy, Redis, RabbitMQ) without warning [2][3]. 2. Transport-Specific Implementation: Methods like _size are often implemented individually by specific transport classes to handle the unique storage mechanisms of different message brokers [2][3]. Because each transport has different capabilities, there is no guarantee that a _size method exists or behaves consistently across all Kombu transports [3]. 3. Public Alternatives: If you need to interact with queues or messages, you should rely on the documented public API, such as kombu.Consumer or kombu.SimpleQueue [5][6][7]. For instance, to check the size of a queue, the public SimpleQueue.qsize() or the equivalent queue_declare with passive=True (which returns the message count) is the appropriate approach [5]. Relying on underscore-prefixed methods makes your code fragile and likely to break during Kombu library updates [2][8]. [2][3]

Citations:

• 1: https://kombu.readthedocs.io/en/latest/_modules/kombu/messaging.html
• 2: celery/kombu@b6e4747
• 3: https://docs.celeryq.dev/projects/kombu/en/v4.5.0/%5Fmodules/kombu/transport/mongodb.html
• 4: https://docs.celeryq.dev/projects/kombu/en/stable/reference/kombu.transport.base.html
• 5: https://kombu.readthedocs.io/en/latest/%5Fmodules/kombu/simple.html
• 6: https://docs.celeryq.dev/projects/kombu/en/latest/%5Fsources/reference/kombu.rst.txt
• 7: https://docs.celeryq.dev/projects/kombu/en/latest/reference/kombu.html
• 8: Remove nested query from sqlalchemy _size celery/kombu#2315

---

Avoid private Kombu Channel internals in the validator.

_inplace_augment_message (lines 412-413) and _size (481-485) are underscore-prefixed Kombu internals (not supported/public APIs), so this validator can break across Kombu versions/transports even when the public dead-letter/max-length behavior it’s testing is correct.

ch._inplace_augment_message(msg, "exS", "rkS")
msg["properties"]["delivery_info"]["queue"] = "qS"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bench/swe-lite/validators/validate_kombu.py` around lines 412 - 413, The code
is calling Kombu private internals ch._inplace_augment_message and _size and
mutating msg["properties"]["delivery_info"]["queue"]; replace those with public
APIs: construct the test message as a kombu.message.Message (or use
kombu.message.Message(body, delivery_info=..., properties=...)) and set the
queue name in its public delivery_info/properties instead of mutating internals,
and compute message byte-size using a public serializer (e.g.,
kombu.serialization.dumps followed by len(...) of the serialized payload) rather
than calling _size; update usages around _inplace_augment_message,
msg["properties"]["delivery_info"]["queue"], and _size to use these public
alternatives.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail the validator when the named checkpoint exceptions are missing.

get_exc() falls back to Exception, so checks that are supposed to verify CheckpointNotActiveError and CheckpointNotFoundError will also pass if the implementation raises the wrong type. That turns a spec violation into a false green.

🛠️ Minimal fix

     def get_exc(*names):
         for n in names:
             c = getattr(dbmod, n, None)
             if c is not None:
                 return c
-        return Exception
+        raise AttributeError(f"Missing expected exception class: {', '.join(names)}")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def get_exc(*names):
	for n in names:
	c = getattr(dbmod, n, None)
	if c is not None:
	return c
	return Exception
	SafeImportNotEnabledError = get_exc("SafeImportNotEnabledError")
	CheckpointNotActiveError = get_exc("CheckpointNotActiveError")
	CheckpointNotFoundError = get_exc("CheckpointNotFoundError")
	def get_exc(*names):
	for n in names:
	c = getattr(dbmod, n, None)
	if c is not None:
	return c
	raise AttributeError(f"Missing expected exception class: {', '.join(names)}")
	SafeImportNotEnabledError = get_exc("SafeImportNotEnabledError")
	CheckpointNotActiveError = get_exc("CheckpointNotActiveError")
	CheckpointNotFoundError = get_exc("CheckpointNotFoundError")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bench/swe-lite/validators/validate_sqliteutils.py` around lines 47 - 56, The
current get_exc() silently falls back to Exception which hides missing API
types; change get_exc to fail loudly when none of the requested names exist
(e.g., raise ImportError or RuntimeError with a clear message referencing the
requested names) so that assigning SafeImportNotEnabledError,
CheckpointNotActiveError, and CheckpointNotFoundError will error if the
implementation doesn't provide them; update references to
get_exc("SafeImportNotEnabledError"), get_exc("CheckpointNotActiveError"), and
get_exc("CheckpointNotFoundError") to rely on the new behavior (or perform an
explicit None check and raise) so the validator fails instead of silently using
Exception.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Require the documented non-strict safe-mode return envelope.

These branches currently treat any non-dict return as success. The supplied task spec says import_csv(..., safe_mode=True, strict=False) and import_json(..., safe_mode=True, strict=False) return structured {success: ...} envelopes, so this validator would incorrectly pass legacy or non-compliant behavior.

🛠️ Minimal fix

         res = db.import_csv("people", io.StringIO(csv_text), safe_mode=True, strict=False)
-        if isinstance(res, dict):
-            ok_dict = res.get("success") is True
-        else:
-            ok_dict = True
+        ok_dict = isinstance(res, dict) and res.get("success") is True
         present = "people" in db.table_names() and db["people"].count == 2
         results["checks"]["import_csv_safe_mode_commits"] = present and ok_dict
@@
         res = db.import_json("jt", data, safe_mode=True, strict=False)
-        if isinstance(res, dict):
-            ok_dict = res.get("success") is True
-        else:
-            ok_dict = True
+        ok_dict = isinstance(res, dict) and res.get("success") is True
         present = "jt" in db.table_names() and db["jt"].count == 2
         results["checks"]["import_json_safe_mode_commits"] = present and ok_dict

Also applies to: 287-291

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bench/swe-lite/validators/validate_sqliteutils.py` around lines 272 - 276,
The validator currently treats any non-dict return from db.import_csv(...,
safe_mode=True, strict=False) and db.import_json(..., safe_mode=True,
strict=False) as success; change the logic so you require the documented
safe-mode envelope: for the db.import_csv result (variable ok_dict) only set
ok_dict = True when isinstance(res, dict) and res.get("success") is True,
otherwise False; apply the same strict check to the db.import_json result
(variable ok_json) so legacy/non-compliant non-dict returns do not pass
validation.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Rollback keeps borrowed path slices past the CONTENT buffer lifetime.

In the heap-read fallback, path aliases heap_section, but the heap_section defer runs before the !load_ok rollback defer. That makes the stale/outline-only branches append freed slices into inserted_paths, and explorer.removeFile(path) can read dangling memory on any later return false.

💡 Minimal fix

         if (disk_content) |dc| {
             word_index_can_load_from_disk = false;
             if (outline_states.fetchRemove(path)) |removed| {
                 allocator.free(removed.key);
                 var stale_outline = removed.value;
                 stale_outline.deinit();
             }
             explorer.indexFile(path, dc) catch continue;
-            inserted_paths.appendAssumeCapacity(path);
+            const stable = explorer.outlines.getEntry(path) orelse continue;
+            inserted_paths.appendAssumeCapacity(stable.key_ptr.*);
             const hash = std.hash.Wyhash.hash(0, dc);
             _ = store.recordSnapshot(path, dc.len, hash) catch {};
         } else if (outline_states.fetchRemove(path)) |removed| {
             const t_ins: i128 = if (prof) cio.nanoTimestamp() else 0;
             insertRestoredFile(explorer, removed.key, content, removed.value, allocator, content_borrowed) catch {
@@
         } else {
             word_index_can_load_from_disk = false;
             explorer.indexFileOutlineOnly(path, content) catch continue;
-            inserted_paths.appendAssumeCapacity(path);
+            const stable = explorer.outlines.getEntry(path) orelse continue;
+            inserted_paths.appendAssumeCapacity(stable.key_ptr.*);
             const hash = record.stored_hash orelse std.hash.Wyhash.hash(0, content);
             _ = store.recordSnapshot(path, content.len, hash) catch {};
         }

Also applies to: 991-1017, 1146-1174

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/snapshot.zig` around lines 913 - 921, The rollback defer is calling
explorer.removeFile(path) where path is a borrowed slice that can alias the
heap_section/CMS CONTENT buffer which is freed earlier by the heap_section
defer; fix by ensuring inserted_paths stores owned copies instead of borrowed
slices (or move the rollback defer to run before heap_section defer).
Specifically, change the code paths that append into inserted_paths to duplicate
the path (e.g., allocate/duplicate the string into the snapshot/explorer
allocator) so that entries in inserted_paths outlive heap_section/CONTENT, and
keep the existing rollback defer that calls explorer.removeFile(path) and the
symbols load_ok, inserted_paths, heap_section, explorer.removeFile,
releaseOutlineSectionsFrom and releaseContentSectionsFrom intact.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Windows paths bypass the sensitive-file filter.

isSensitivePath only splits on '/' and only checks ".ssh/", ".gnupg/", and ".aws/". On the new Windows port, paths like C:\Users\alice\.ssh\id_rsa or ...\ .aws\credentials keep the full path as basename, so the exact-name checks miss and those secrets can still be snapshotted or live-indexed.

💡 Minimal fix

-    const basename = if (std.mem.lastIndexOfScalar(u8, path, '/')) |sep| path[sep + 1 ..] else path;
+    const basename = if (std.mem.lastIndexOfAny(u8, path, "/\\")) |sep| path[sep + 1 ..] else path;
@@
-        if (std.mem.indexOf(u8, path, ".ssh/") != null or
-            std.mem.indexOf(u8, path, ".gnupg/") != null or
-            std.mem.indexOf(u8, path, ".aws/") != null) return true;
+        if (std.mem.indexOf(u8, path, ".ssh/") != null or
+            std.mem.indexOf(u8, path, ".ssh\\") != null or
+            std.mem.indexOf(u8, path, ".gnupg/") != null or
+            std.mem.indexOf(u8, path, ".gnupg\\") != null or
+            std.mem.indexOf(u8, path, ".aws/") != null or
+            std.mem.indexOf(u8, path, ".aws\\") != null) return true;
@@
-    if (std.mem.indexOf(u8, path, ".ssh/") != null or
-        std.mem.indexOf(u8, path, ".gnupg/") != null or
-        std.mem.indexOf(u8, path, ".aws/") != null) return true;
+    if (std.mem.indexOf(u8, path, ".ssh/") != null or
+        std.mem.indexOf(u8, path, ".ssh\\") != null or
+        std.mem.indexOf(u8, path, ".gnupg/") != null or
+        std.mem.indexOf(u8, path, ".gnupg\\") != null or
+        std.mem.indexOf(u8, path, ".aws/") != null or
+        std.mem.indexOf(u8, path, ".aws\\") != null) return true;

This risk is amplified by the Windows-native port scope in this PR.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	pub fn isSensitivePath(path: []const u8) bool {
	const basename = if (std.mem.lastIndexOfScalar(u8, path, '/')) |sep| path[sep + 1 ..] else path;
	// Fast path: most source files have extensions like .zig, .ts, .py — none start with '.'
	// or match sensitive patterns. Skip the full check for common cases.
	if (basename.len == 0) return false;
	const first = basename[0];
	// Only check sensitive names if basename starts with '.', 'c', 's', 'i' or has key/cert extension
	if (first != '.' and first != 'c' and first != 's' and first != 'i') {
	// Still need to check extensions and directory patterns
	if (std.mem.endsWith(u8, basename, ".env") or
	std.mem.endsWith(u8, basename, ".pem") or
	std.mem.endsWith(u8, basename, ".key") or
	std.mem.endsWith(u8, basename, ".p12") or
	std.mem.endsWith(u8, basename, ".pfx") or
	std.mem.endsWith(u8, basename, ".jks")) return true;
	if (std.mem.indexOf(u8, path, ".ssh/") != null or
	std.mem.indexOf(u8, path, ".gnupg/") != null or
	std.mem.indexOf(u8, path, ".aws/") != null) return true;
	return false;
	}
	// .env, .env.<token>; do NOT match .envoy, .envrc, .environment, etc.
	if (basename.len >= 4 and std.mem.eql(u8, basename[0..4], ".env") and
	(basename.len == 4 or basename[4] == '.' or basename[4] == '-' or basename[4] == '_')) return true;
	// Exact matches
	const sensitive_names = [_][]const u8{
	".env",
	".env.local",
	".env.production",
	".env.development",
	".env.staging",
	".env.test",
	".dev.vars",
	"credentials.json",
	"service-account.json",
	"secrets.json",
	"secrets.yaml",
	"secrets.yml",
	".npmrc",
	".pypirc",
	".netrc",
	"id_rsa",
	"id_ed25519",
	".pem",
	".dev.vars", ".npmrc", ".pypirc", ".netrc",
	"credentials.json", "service-account.json", "secrets.json", "secrets.yaml",
	"secrets.yml", "id_rsa", "id_ed25519", ".git-credentials",
	"id_ecdsa", "id_dsa", "id_ecdsa_sk", "id_ed25519_sk",
	};
	// Check exact filename (basename)
	const basename = if (std.mem.lastIndexOfScalar(u8, path, '/')) |sep| path[sep + 1 ..] else path;
	for (sensitive_names) |name| {
	if (std.mem.eql(u8, basename, name)) return true;
	}
	// Catch .env, .env.anything; do NOT match .envoy, .envrc, .environment, etc.
	if (basename.len >= 4 and std.mem.eql(u8, basename[0..4], ".env") and
	(basename.len == 4 or basename[4] == '.' or basename[4] == '-' or basename[4] == '_')) return true;
	// Check extensions
	if (endsWith(basename, ".pem")) return true;
	if (endsWith(basename, ".key")) return true;
	if (endsWith(basename, ".p12")) return true;
	if (endsWith(basename, ".pfx")) return true;
	if (endsWith(basename, ".jks")) return true;
	// Check directory patterns
	if (std.mem.indexOf(u8, path, ".ssh/") != null) return true;
	if (std.mem.indexOf(u8, path, ".gnupg/") != null) return true;
	if (std.mem.indexOf(u8, path, ".aws/") != null) return true;
	if (std.mem.endsWith(u8, basename, ".env") or
	std.mem.endsWith(u8, basename, ".pem") or
	std.mem.endsWith(u8, basename, ".key") or
	std.mem.endsWith(u8, basename, ".p12") or
	std.mem.endsWith(u8, basename, ".pfx") or
	std.mem.endsWith(u8, basename, ".jks")) return true;
	if (std.mem.indexOf(u8, path, ".ssh/") != null or
	std.mem.indexOf(u8, path, ".gnupg/") != null or
	std.mem.indexOf(u8, path, ".aws/") != null) return true;
	return false;
	pub fn isSensitivePath(path: []const u8) bool {
	const basename = if (std.mem.lastIndexOfAny(u8, path, "/\\")) |sep| path[sep + 1 ..] else path;
	// Fast path: most source files have extensions like .zig, .ts, .py — none start with '.'
	// or match sensitive patterns. Skip the full check for common cases.
	if (basename.len == 0) return false;
	const first = basename[0];
	// Only check sensitive names if basename starts with '.', 'c', 's', 'i' or has key/cert extension
	if (first != '.' and first != 'c' and first != 's' and first != 'i') {
	// Still need to check extensions and directory patterns
	if (std.mem.endsWith(u8, basename, ".env") or
	std.mem.endsWith(u8, basename, ".pem") or
	std.mem.endsWith(u8, basename, ".key") or
	std.mem.endsWith(u8, basename, ".p12") or
	std.mem.endsWith(u8, basename, ".pfx") or
	std.mem.endsWith(u8, basename, ".jks")) return true;
	if (std.mem.indexOf(u8, path, ".ssh/") != null or
	std.mem.indexOf(u8, path, ".ssh\\") != null or
	std.mem.indexOf(u8, path, ".gnupg/") != null or
	std.mem.indexOf(u8, path, ".gnupg\\") != null or
	std.mem.indexOf(u8, path, ".aws/") != null or
	std.mem.indexOf(u8, path, ".aws\\") != null) return true;
	return false;
	}
	// .env, .env.<token>; do NOT match .envoy, .envrc, .environment, etc.
	if (basename.len >= 4 and std.mem.eql(u8, basename[0..4], ".env") and
	(basename.len == 4 or basename[4] == '.' or basename[4] == '-' or basename[4] == '_')) return true;
	// Exact matches
	const sensitive_names = [_][]const u8{
	".dev.vars", ".npmrc", ".pypirc", ".netrc",
	"credentials.json", "service-account.json", "secrets.json", "secrets.yaml",
	"secrets.yml", "id_rsa", "id_ed25519", ".git-credentials",
	"id_ecdsa", "id_dsa", "id_ecdsa_sk", "id_ed25519_sk",
	};
	for (sensitive_names) |name| {
	if (std.mem.eql(u8, basename, name)) return true;
	}
	if (std.mem.endsWith(u8, basename, ".env") or
	std.mem.endsWith(u8, basename, ".pem") or
	std.mem.endsWith(u8, basename, ".key") or
	std.mem.endsWith(u8, basename, ".p12") or
	std.mem.endsWith(u8, basename, ".pfx") or
	std.mem.endsWith(u8, basename, ".jks")) return true;
	if (std.mem.indexOf(u8, path, ".ssh/") != null or
	std.mem.indexOf(u8, path, ".ssh\\") != null or
	std.mem.indexOf(u8, path, ".gnupg/") != null or
	std.mem.indexOf(u8, path, ".gnupg\\") != null or
	std.mem.indexOf(u8, path, ".aws/") != null or
	std.mem.indexOf(u8, path, ".aws\\") != null) return true;
	return false;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/snapshot.zig` around lines 1331 - 1373, isSensitivePath currently only
splits on '/' and only searches for UNIX-style directory tokens, which lets
Windows paths (backslashes) bypass checks; update isSensitivePath to treat both
'/' and '\\' as path separators when computing basename (e.g., use lastIndexOf
for both or a helper that finds the last of '/' or '\\'), and update
directory-pattern checks (std.mem.indexOf(u8, path, ".ssh/") etc.) to also look
for the backslash variants (".ssh\\", ".gnupg\\", ".aws\\") or check for the
token followed by either separator; keep the existing basename exact-name and
extension checks but run them on the normalized basename extracted using the new
separator logic so Windows paths are correctly flagged.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Compaction is not safe with the shared cross-process diff log.

appendVersion treats data_log as a multi-process file (lock(...) + re-stat current end before write), but compactDataLog rewrites and truncates using only this process's self.files. Any diffs appended by another process are invisible here and can be truncated as "orphaned" bytes even though that process still has live data_offsets pointing at them.

Also applies to: 147-215

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/store.zig` around lines 99 - 117, appendVersion treats data_log as a
multi-process file (uses log.lock and re-stat) but compactDataLog currently
rewrites/truncates using only this process's self.files and can therefore drop
bytes other processes referenced; fix compactDataLog to acquire the same
advisory lock on data_log (same lock(...) path used by appendVersion) and while
holding that exclusive lock re-stat the file, scan all live version records
across self.files to compute the true highest referenced data_offset/length (not
just this process's view), rebuild the compacted log into a temporary file and
atomically replace/truncate the original only after verifying offsets against
the locked+re-statted end_pos, and update self.data_log_pos and any in-process
metadata consistently while still holding the lock so no concurrent append can
make offsets stale; ensure symbols touched include compactDataLog,
appendVersion, data_log, self.files, data_log_pos and log.lock/log.unlock.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Restore pre-existing env var state to prevent cross-test leakage.

Line 1115 and Line 1131 mutate a process-global env var but only unset it. If CODEDB_LEX_FREQ_PENALTY was already set before this test, the original value is lost, making later tests order-dependent.

Suggested minimal fix

-    cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", "0");
-    defer cio.posixUnsetenv("CODEDB_LEX_FREQ_PENALTY");
+    const prev = std.process.getEnvVarOwned(testing.allocator, "CODEDB_LEX_FREQ_PENALTY") catch null;
+    defer if (prev) |v| {
+        defer testing.allocator.free(v);
+        cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", v);
+    } else {
+        cio.posixUnsetenv("CODEDB_LEX_FREQ_PENALTY");
+    };
+    cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", "0");

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", "0");
	defer cio.posixUnsetenv("CODEDB_LEX_FREQ_PENALTY");
	{
	const results = try explorer.searchContent("evt", testing.allocator, 50);
	defer {
	for (results) |r| {
	testing.allocator.free(r.path);
	testing.allocator.free(r.line_text);
	}
	testing.allocator.free(results);
	}
	try testing.expect(results.len >= 2);
	try testing.expectEqualStrings("src/dispatcher.zig", results[0].path);
	}
	// Default (on): dispatcher.zig saturates the query → demoted below handler.zig.
	cio.posixUnsetenv("CODEDB_LEX_FREQ_PENALTY");
	{
	const prev = std.process.getEnvVarOwned(testing.allocator, "CODEDB_LEX_FREQ_PENALTY") catch null;
	defer if (prev) |v| {
	defer testing.allocator.free(v);
	cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", v);
	} else {
	cio.posixUnsetenv("CODEDB_LEX_FREQ_PENALTY");
	};
	cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", "0");
	{
	const results = try explorer.searchContent("evt", testing.allocator, 50);
	defer {
	for (results) |r| {
	testing.allocator.free(r.path);
	testing.allocator.free(r.line_text);
	}
	testing.allocator.free(results);
	}
	try testing.expect(results.len >= 2);
	try testing.expectEqualStrings("src/dispatcher.zig", results[0].path);
	}
	// Default (on): dispatcher.zig saturates the query → demoted below handler.zig.
	cio.posixUnsetenv("CODEDB_LEX_FREQ_PENALTY");
	{

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test_search.zig` around lines 1115 - 1132, Before you call
cio.posixSetenv("CODEDB_LEX_FREQ_PENALTY", "0") save the current environment
value (e.g., via cio.posixGetenv or std.os.getenv) into a local optional string,
and then in a defer restore it: if an original value existed call
cio.posixSetenv with that value, otherwise call cio.posixUnsetenv; do the same
pattern for the later block that calls cio.posixUnsetenv so the test always
restores the pre-test state of CODEDB_LEX_FREQ_PENALTY rather than permanently
unsetting it.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use ArenaAllocator for Explorer tests in this file.

Line 1815, Line 1903, and Line 1948 initialize Explorer with testing.allocator. In src/test_*.zig, these should use std.heap.ArenaAllocator per repo guideline.

Suggested pattern

-    var explorer = Explorer.init(testing.allocator, Explorer.DEFAULT_CONTENT_CACHE_CAPACITY);
-    defer explorer.deinit();
+    var arena = std.heap.ArenaAllocator.init(testing.allocator);
+    defer arena.deinit();
+    var explorer = Explorer.init(arena.allocator(), Explorer.DEFAULT_CONTENT_CACHE_CAPACITY);

As per coding guidelines, src/test_*.zig: “Use std.heap.ArenaAllocator for Explorer tests in Zig”.

Also applies to: 1903-1904, 1948-1949

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test_search.zig` around lines 1815 - 1816, Replace direct use of
testing.allocator when initializing Explorer with a std.heap.ArenaAllocator
instance: create an ArenaAllocator (e.g. via
std.heap.ArenaAllocator.init(testing.allocator)), pass the arena's allocator to
Explorer.init (instead of testing.allocator) for the
Explorer.DEFAULT_CONTENT_CACHE_CAPACITY calls, and ensure you deinit the arena
after the Explorer (mirror Explorer.deinit with arena.deinit/destroy as
appropriate). Update the three sites that call Explorer.init to use the arena
allocator and add the corresponding arena lifetime management adjacent to those
tests.

Source: Coding guidelines

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/root_policy.zig`:
- Around line 59-63: The Windows-root checker is missing normalization for Win32
device-prefix paths (e.g. "\\?\\C:\\..." and "\\.\\C:\\...") so functions like
isNativeWindowsSystemRoot bypass the drive-letter checks; update
isNativeWindowsSystemRoot to detect and strip the "\\?\" and "\\.\" UNC device
prefixes (and any equivalent case variants) before the existing drive-letter and
path-separator logic, then apply the same normalization change to the other
Windows classifier referenced around lines 172-185 (the other Windows
root-checking function), and add/adjust unit tests to cover "\\?\\C:\\...",
"\\.\\C:\\...", and ordinary "C:\\..." forms to ensure deny logic triggers
consistently.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d632d33e-665f-45f5-9758-f54c395f90a1

📥 Commits

Reviewing files that changed from the base of the PR and between 185f8c3 and 3b422e5.

📒 Files selected for processing (1)

• src/root_policy.zig

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize Win32 device-prefix paths before Windows root checks.

Line 60 assumes the drive letter is at byte 0, so valid absolute forms like \\?\C:\Windows\System32 and \\.\C:\ProgramData bypass the deny logic and remain indexable. Normalize those prefixes before both Windows classifiers, and add coverage for them.

Suggested targeted fix

+fn normalizeWindowsDrivePath(path_in: []const u8) []const u8 {
+    if (path_in.len >= 7 and
+        isPathSep(path_in[0]) and isPathSep(path_in[1]) and
+        (path_in[2] == '?' or path_in[2] == '.') and
+        isPathSep(path_in[3]) and
+        std.ascii.isAlphabetic(path_in[4]) and
+        path_in[5] == ':' and
+        isPathSep(path_in[6]))
+    {
+        return path_in[4..];
+    }
+    return path_in;
+}
+
-fn isNativeWindowsTempRoot(path: []const u8) bool {
+fn isNativeWindowsTempRoot(path_in: []const u8) bool {
+    const path = normalizeWindowsDrivePath(path_in);
     if (path.len < 3 or path[1] != ':' or !std.ascii.isAlphabetic(path[0]) or !isPathSep(path[2])) return false;
@@
-fn isNativeWindowsSystemRoot(path: []const u8) bool {
+fn isNativeWindowsSystemRoot(path_in: []const u8) bool {
+    const path = normalizeWindowsDrivePath(path_in);
     if (path.len < 2 or path[1] != ':' or !std.ascii.isAlphabetic(path[0])) return false;
@@
 test "windows system and drive roots are denied" {
@@
     try testing.expect(!isIndexableRoot("C:\\ProgramData\\Vendor"));
+    try testing.expect(!isIndexableRoot("\\\\?\\C:\\Windows\\System32"));
+    try testing.expect(!isIndexableRoot("\\\\.\\C:\\ProgramData\\Vendor"));
     try testing.expect(isIndexableRoot("C:\\Users\\dev\\projects\\repo"));

Also applies to: 172-185

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/root_policy.zig` around lines 59 - 63, The Windows-root checker is
missing normalization for Win32 device-prefix paths (e.g. "\\?\\C:\\..." and
"\\.\\C:\\...") so functions like isNativeWindowsSystemRoot bypass the
drive-letter checks; update isNativeWindowsSystemRoot to detect and strip the
"\\?\" and "\\.\" UNC device prefixes (and any equivalent case variants) before
the existing drive-letter and path-separator logic, then apply the same
normalization change to the other Windows classifier referenced around lines
172-185 (the other Windows root-checking function), and add/adjust unit tests to
cover "\\?\\C:\\...", "\\.\\C:\\...", and ordinary "C:\\..." forms to ensure
deny logic triggers consistently.