[codex] Fix path-like filename search#2
Conversation
WalkthroughWindows and WSL filename search adapters were refactored to improve path-like query handling and normalize path comparisons. Windows searches now match against full item paths ( ChangesCross-Platform Filename Search Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds WSL UNC-path parsing, direct-path fast-path logic for exact WSL path queries, hidden-file and directory support in both WSL and Windows filename scans, and routes path-like Windows profile searches through the filesystem scanner instead of the Windows Search index. Two issues worth addressing before merge:
Confidence Score: 3/5Two confirmed defects on the changed paths: the WSL direct-path fast-path is non-functional due to dollar-sign over-escaping, and OleDb filename searches now silently do full-path matching for all queries including simple word searches. The buildDirectPathPrefix function, introduced specifically to short-circuit exact WSL path queries, never fires because escapeWslCommandArgument escapes the variable references inside the already-single-quoted script body. Additionally, the OleDb query semantic shift from System.FileName to System.ItemPathDisplay affects every profile-root filename search, not only the path-like case it was intended to fix. src/search/wslSearchAdapter.js (escaping interaction between escapeWslCommandArgument and buildDirectPathPrefix) and src/search/windowsSearchAdapter.js (OleDb query column change and its effect on simple-word searches). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[WSL filename search input] --> B{parseWslUncPath?}
B -- yes --> C[Extract distro + path from UNC]
B -- no --> D[Use raw query + input distro]
C --> E[validateSearchInput]
D --> E
E --> F[buildDirectPathPrefix BROKEN]
F --> G{rg available?}
G -- yes --> H[rg files + find dirs piped through grep]
G -- no --> I[find files+dirs piped through grep]
H --> J[head results]
I --> J
K[Windows filename search input] --> L[validateWindowsSearchInput]
L --> M{directFilePathResult now includes dirs}
M -- exact path hit --> N[Return result]
M -- no hit --> O{shouldUseFilesystemFilenameSearch}
O -- all-drives OR path-like OR non-profile --> P[runFilenameSearch via rg full-path match]
O -- profile root simple query --> Q[OleDb ItemPathDisplay LIKE full-path match ALL queries]
P --> R[parseWindowsSearchOutput]
Q --> R
Reviews (12): Last reviewed commit: "Fix path-like filename search" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function escapeWslCommandArgument(value) { | ||
| return String(value).replace(/\$/g, "\\$"); | ||
| } |
There was a problem hiding this comment.
escapeWslCommandArgument breaks buildDirectPathPrefix variable expansion
escapeWslCommandArgument replaces every $ in the already-shellQuoted script — including the $root_path, $candidate, and $under_root references inside the single-quoted inner-bash script body. Those references end up as \$root_path / \$candidate. Inside single quotes, they are passed literally to the inner bash. In double-quoted contexts like [ "\$root_path" = "/" ], bash interprets \$ as a literal $ character, not a variable. root_path and candidate are assigned but never read. The [ -e "$candidate" ] guard tests for a file literally named $candidate, which never exists, so the fast-path exit 0 never fires and every path-like WSL query falls through to a full rg/find scan.
Smallest correct fix: apply escapeWslCommandArgument only before shellQuote on user-controlled input if needed, not on the entire already-quoted script. The outer script's own $var references must remain as unexpanded dollar signs reaching the inner bash.
| "foreach ($term in $terms) {", | ||
| " $whereClauses += \"System.FileName LIKE '%\" + (Escape-LikeLiteral $term) + \"%'\"", | ||
| " $pathTerm = $term.Replace('/', [string][char]92)", | ||
| " $whereClauses += \"System.ItemPathDisplay LIKE '%\" + (Escape-LikeLiteral $pathTerm) + \"%'\"", | ||
| "}", |
There was a problem hiding this comment.
System.ItemPathDisplay expands result set for simple word searches
Switching from System.FileName LIKE '%term%' to System.ItemPathDisplay LIKE '%term%' for all OleDb-backed filename searches causes full-path matching for every term, including simple non-path-like queries (e.g. photos, budget). Before this change, searching for photos returned files whose name contained photos. Now it returns every file whose full path contains photos — for example, every file under C:\Users\Me\Photos\ — which can produce a large false-positive set.
Path-like queries are now routed to runFilenameSearch (rg), so System.ItemPathDisplay is only reached for simple, non-path-like, profile-root queries. rg's matchesFilenameSearch also does full-path matching, so the two branches are now consistent — but users who expect filename-only matching for simple terms will see significantly more results. If full-path matching for all queries is intentional, a comment explaining the deliberate semantic shift would prevent future regressions back to System.FileName.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/search/wslSearchAdapter.js`:
- Around line 18-20: The wrapper currently double-escapes dollar signs in
escapeWslCommandArgument which alters what the inner bash -lc sees and prevents
expansion of variables like $root_path and $candidate; remove the post-quoting
replacement of `$` (i.e., stop turning $ into \$) so that searchCommand—already
protected by shellQuote(...)—is passed through intact to the bash -lc wrapper;
update the escapeWslCommandArgument function (and any similar escaping in the
WSL script wrapper) to no longer escape `$`, ensuring variable expansion and
literal dollar-containing filenames/queries work correctly.
In `@test/windowsSearch.test.js`:
- Around line 272-304: The test "runSearch uses filename filesystem search for
path-like profile filename searches" is only guarded by HAS_USERPROFILE but
should also require IS_WINDOWS because adapter.runSearch validates Windows
absolute paths; update the test's skip condition to skip when not
(HAS_USERPROFILE && IS_WINDOWS) (or equivalent) so it only runs on Windows
environments; locate the test by its title and the symbols HAS_USERPROFILE,
IS_WINDOWS, and createWindowsSearchAdapter/adapter.runSearch to modify the skip
flag accordingly.
🪄 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: 27ca5f93-45d0-490f-b87e-8dc4ac1bf1d4
📒 Files selected for processing (4)
src/search/windowsSearchAdapter.jssrc/search/wslSearchAdapter.jstest/search.test.jstest/windowsSearch.test.js
| const child = spawn( | ||
| resolveRipgrepPath(), | ||
| [ | ||
| "--files", | ||
| "-0", | ||
| "--hidden", | ||
| "--glob", | ||
| "!.git", | ||
| "--glob", | ||
| "!node_modules", | ||
| "--glob", | ||
| "!.cache", | ||
| "--glob", | ||
| "!.venv", | ||
| "--glob", | ||
| "!__pycache__", | ||
| "--glob", | ||
| "!dist", | ||
| ...roots, | ||
| ], | ||
| { windowsHide: true } | ||
| ); |
There was a problem hiding this comment.
Filesystem fallback still drops directory matches.
shouldUseFilesystemFilenameSearch() now sends .codex/foo/bar filename queries through runFilenameSearch(), but this helper still shells out to rg --files, which never emits directory entries. That means a relative directory query like .codex/plans under %USERPROFILE% still can't return the directory itself unless the caller provides an exact absolute path and hits directFilePathResult(). The new Windows path-like flow is therefore still incomplete for directory searches.
Also applies to: 481-485
| function escapeWslCommandArgument(value) { | ||
| return String(value).replace(/\$/g, "\\$"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
search_command='candidate=/tmp/demo; query='\''price$list.txt'\''; printf "candidate=%s\nquery=%s\n" "$candidate" "$query"'
wrapped_without_escape="timeout 1s bash -lc '$search_command'"
wrapped_with_escape="timeout 1s bash -lc '${search_command//$/\\$}'"
echo "without post-escape:"
bash -lc "$wrapped_without_escape"
echo
echo "with post-escape:"
bash -lc "$wrapped_with_escape"Repository: future3OOO/wsl-search-tool
Length of output: 251
🏁 Script executed:
# First, locate and examine the file
find . -name "wslSearchAdapter.js" -type fRepository: future3OOO/wsl-search-tool
Length of output: 103
🏁 Script executed:
# Read the entire file to understand the context
wc -l ./src/search/wslSearchAdapter.jsRepository: future3OOO/wsl-search-tool
Length of output: 107
🏁 Script executed:
# Read the critical sections
echo "=== Lines 15-25 (function definition) ==="
sed -n '15,25p' ./src/search/wslSearchAdapter.js
echo
echo "=== Lines 139-160 (buildDirectPathPrefix and variable references) ==="
sed -n '139,160p' ./src/search/wslSearchAdapter.js
echo
echo "=== Lines 185-200 (call sites) ==="
sed -n '185,200p' ./src/search/wslSearchAdapter.jsRepository: future3OOO/wsl-search-tool
Length of output: 2933
🏁 Script executed:
# Check how buildSearchScript is used (where it's called)
echo "=== Searching for buildSearchScript calls ==="
rg "buildSearchScript" ./src/search/wslSearchAdapter.js -B 2 -A 2
echo
echo "=== Check for execSync or similar execution ==="
rg "execSync|spawn|exec" ./src/search/wslSearchAdapter.js -B 2 -A 2Repository: future3OOO/wsl-search-tool
Length of output: 1018
🏁 Script executed:
# Check how the result of buildSearchScript is used
echo "=== Full buildSearchCommand function ==="
sed -n '173,180p' ./src/search/wslSearchAdapter.js
echo
echo "=== Look for test files ==="
find . -name "*test*" -o -name "*.test.js" -o -name "*.spec.js" | head -20Repository: future3OOO/wsl-search-tool
Length of output: 685
🏁 Script executed:
# Get the complete buildSearchCommand function
sed -n '160,205p' ./src/search/wslSearchAdapter.jsRepository: future3OOO/wsl-search-tool
Length of output: 2698
🏁 Script executed:
# Check if there are tests for the functions being modified
grep -r "buildSearchScript\|buildSearchCommand\|escapeWslCommandArgument" ./test/ -lRepository: future3OOO/wsl-search-tool
Length of output: 92
🏁 Script executed:
# Check the search test file for relevant tests
grep -n "buildSearchScript\|buildSearchCommand\|escapeWslCommandArgument" ./test/search.test.js -B 2 -A 5Repository: future3OOO/wsl-search-tool
Length of output: 2929
🏁 Script executed:
# Read test cases to understand expected behavior
sed -n '36,51p' ./test/search.test.js
echo "---"
sed -n '104,116p' ./test/search.test.js
echo "---"
sed -n '145,157p' ./test/search.test.jsRepository: future3OOO/wsl-search-tool
Length of output: 1590
🏁 Script executed:
# Let's verify what the current output actually produces
# Look at the full test for the exact-path scenario
sed -n '145,157p' ./test/search.test.js
echo "---"
echo "Let's examine what escapeWslCommandArgument is actually doing to the shell command:"
echo "Looking at lines 18-20 again:"
sed -n '18,20p' ./src/search/wslSearchAdapter.jsRepository: future3OOO/wsl-search-tool
Length of output: 778
🏁 Script executed:
# Check if there are any tests that actually execute the shell commands
grep -n "spawn\|execSync\|exec" ./test/search.test.js
echo "---"
echo "Let's understand the quoting flow:"
echo "The test expects escaped dollars. Let's trace what buildSearchScript actually produces."
echo "---"
# Let's run a Node snippet to see what buildSearchScript produces
node <<'EOF'
const { buildSearchCommand } = require('./src/search/wslSearch');
const command = buildSearchCommand({
distro: "OpenCockLab",
root: "/home",
query: "/home/test.txt",
mode: "filename",
maxResults: 10,
});
console.log("Full command array:");
command.forEach((arg, i) => {
if (arg.length > 100) {
console.log(`[${i}]: ${arg.substring(0, 100)}...`);
} else {
console.log(`[${i}]: ${arg}`);
}
});
console.log("\n=== Command[6] (the bash script) ===");
console.log(command[6]);
console.log("\n=== Looking for $candidate or \\$candidate ===");
if (command[6].includes('$candidate')) {
console.log("Found: $candidate (unescaped)");
}
if (command[6].includes('\\$candidate')) {
console.log("Found: \\$candidate (escaped)");
}
EOFRepository: future3OOO/wsl-search-tool
Length of output: 1090
🏁 Script executed:
# Check what's exported from wslSearchAdapter
grep -n "^module.exports\|^exports" ./src/search/wslSearchAdapter.js
echo "---"
echo "Check if there's a wslSearch.js file:"
find . -name "wslSearch.js" -type f
echo "---"
echo "Check the test imports:"
head -20 ./test/search.test.jsRepository: future3OOO/wsl-search-tool
Length of output: 682
🏁 Script executed:
# Check the root wslSearch.js file to understand exports
head -30 ./wslSearch.js
echo "---"
echo "Now let's run the actual test to see if it passes or fails"
cd ./test && npm test -- search.test.js 2>&1 | head -100Repository: future3OOO/wsl-search-tool
Length of output: 284
🏁 Script executed:
# Run the test from the root
npm test 2>&1 | grep -A 50 "search.test.js\|buildSearchCommand"Repository: future3OOO/wsl-search-tool
Length of output: 4563
🏁 Script executed:
# Let's manually trace through what buildSearchScript produces with the exact-path query
node <<'EOF'
const fs = require('fs');
const path = require('path');
// Read and eval the adapter to get the functions
const adapterCode = fs.readFileSync('./src/search/wslSearchAdapter.js', 'utf8');
// Create a context with necessary functions
const module = { exports: {} };
const require_local = (name) => {
if (name === 'node:child_process') return {};
return {};
};
// Execute only the relevant parts
eval(adapterCode.replace('require("node:child_process")', '{}'));
const functions = module.exports;
// Test the exact-path scenario
const result = functions.buildSearchCommand({
distro: "OpenCockLab",
root: "/home",
query: "/home/test.txt",
mode: "filename",
maxResults: 10,
});
console.log("=== buildSearchCommand result ===");
console.log("Command[6] (the bash -lc argument):");
const bashArg = result[6];
console.log(bashArg.substring(0, 200) + "...");
console.log("\n=== Checking for variable references ===");
if (bashArg.includes('$candidate')) {
console.log("✓ Found unescaped: $candidate");
} else if (bashArg.includes('\\$candidate')) {
console.log("✗ Found escaped: \\$candidate");
} else {
console.log("? Neither found");
}
if (bashArg.includes('$root_path')) {
console.log("✓ Found unescaped: $root_path");
} else if (bashArg.includes('\\$root_path')) {
console.log("✗ Found escaped: \\$root_path");
}
EOFRepository: future3OOO/wsl-search-tool
Length of output: 446
🏁 Script executed:
# Let's check the complete bash script to understand the variable expansion issue
node <<'EOF'
const fs = require('fs');
const adapterCode = fs.readFileSync('./src/search/wslSearchAdapter.js', 'utf8');
const module = { exports: {} };
eval(adapterCode.replace('require("node:child_process")', '{}'));
const functions = module.exports;
// Test the exact-path scenario
const result = functions.buildSearchCommand({
distro: "OpenCockLab",
root: "/home",
query: "/home/test.txt",
mode: "filename",
maxResults: 10,
});
const bashArg = result[6];
console.log("=== Full bash -lc argument ===");
console.log(bashArg);
console.log("\n=== Key observation ===");
console.log("In bash, \\$root_path means literal $root_path (backslash escapes the dollar)");
console.log("So when bash interprets the command, [ \"\\$root_path\" = \"/\" ] compares");
console.log("the literal string '$root_path' instead of expanding the variable.");
EOFRepository: future3OOO/wsl-search-tool
Length of output: 1430
Remove the post-quoting $ escaping from the WSL script wrapper.
searchCommand is already protected by shellQuote(...). Escaping every $ afterwards changes what the inner bash -lc sees: Line 153's variable references like $root_path and $candidate stop expanding (bash interprets \$ as a literal dollar sign within double quotes), and literal queries or paths containing $ are matched as \$... instead of the original text. This breaks the exact-path fast path and any filename/content search for dollar-containing names.
Suggested fix
-function escapeWslCommandArgument(value) {
- return String(value).replace(/\$/g, "\\$");
-}
-
...
- return escapeWslCommandArgument(`timeout ${WSL_SEARCH_TIMEOUT_SECONDS}s bash -lc ${shellQuote(searchCommand)}`);
+ return `timeout ${WSL_SEARCH_TIMEOUT_SECONDS}s bash -lc ${shellQuote(searchCommand)}`;
...
- return escapeWslCommandArgument(`timeout ${WSL_SEARCH_TIMEOUT_SECONDS}s bash -lc ${shellQuote(searchCommand)}`);
+ return `timeout ${WSL_SEARCH_TIMEOUT_SECONDS}s bash -lc ${shellQuote(searchCommand)}`;
...
- return escapeWslCommandArgument(`timeout ${WSL_SEARCH_TIMEOUT_SECONDS}s bash -lc ${shellQuote(searchCommand)}`);
+ return `timeout ${WSL_SEARCH_TIMEOUT_SECONDS}s bash -lc ${shellQuote(searchCommand)}`;Also applies to: Lines 139–154, 188–198
🤖 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/search/wslSearchAdapter.js` around lines 18 - 20, The wrapper currently
double-escapes dollar signs in escapeWslCommandArgument which alters what the
inner bash -lc sees and prevents expansion of variables like $root_path and
$candidate; remove the post-quoting replacement of `$` (i.e., stop turning $
into \$) so that searchCommand—already protected by shellQuote(...)—is passed
through intact to the bash -lc wrapper; update the escapeWslCommandArgument
function (and any similar escaping in the WSL script wrapper) to no longer
escape `$`, ensuring variable expansion and literal dollar-containing
filenames/queries work correctly.
| test("runSearch uses filename filesystem search for path-like profile filename searches", { skip: !HAS_USERPROFILE }, async () => { | ||
| const commandCalls = []; | ||
| const filenameCalls = []; | ||
| const profileRoot = process.env.USERPROFILE.replace(/\//g, "\\"); | ||
| const adapter = createWindowsSearchAdapter({ | ||
| runCommand: async (command) => { | ||
| commandCalls.push(command); | ||
| return "indexed search should not run"; | ||
| }, | ||
| runFilenameSearch: async (search) => { | ||
| filenameCalls.push(search); | ||
| return `${profileRoot}\\.codex\\plans\\tdd-seam-and-deep-modules-prd-v2.md\0`; | ||
| }, | ||
| }); | ||
|
|
||
| assert.deepEqual( | ||
| await adapter.runSearch({ | ||
| query: ".codex/plans/tdd-seam-and-deep-modules-prd-v2.md", | ||
| mode: "filename", | ||
| root: profileRoot, | ||
| maxResults: "10", | ||
| }), | ||
| [ | ||
| { | ||
| source: "windows", | ||
| target: driveTargetForTestPath(profileRoot), | ||
| path: `${profileRoot}\\.codex\\plans\\tdd-seam-and-deep-modules-prd-v2.md`, | ||
| }, | ||
| ] | ||
| ); | ||
| assert.deepEqual(commandCalls, []); | ||
| assert.deepEqual(filenameCalls[0].roots, [profileRoot]); | ||
| }); |
There was a problem hiding this comment.
Guard this test on IS_WINDOWS too.
This case currently only checks HAS_USERPROFILE, but runSearch() still validates root as a Windows absolute path. If a non-Windows environment happens to set USERPROFILE, the test will fail before it reaches the routing behavior you're trying to assert.
🧪 Suggested fix
-test("runSearch uses filename filesystem search for path-like profile filename searches", { skip: !HAS_USERPROFILE }, async () => {
+test(
+ "runSearch uses filename filesystem search for path-like profile filename searches",
+ { skip: !(IS_WINDOWS && HAS_USERPROFILE) },
+ async () => {🤖 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 `@test/windowsSearch.test.js` around lines 272 - 304, The test "runSearch uses
filename filesystem search for path-like profile filename searches" is only
guarded by HAS_USERPROFILE but should also require IS_WINDOWS because
adapter.runSearch validates Windows absolute paths; update the test's skip
condition to skip when not (HAS_USERPROFILE && IS_WINDOWS) (or equivalent) so it
only runs on Windows environments; locate the test by its title and the symbols
HAS_USERPROFILE, IS_WINDOWS, and createWindowsSearchAdapter/adapter.runSearch to
modify the skip flag accordingly.
1 participant
Summary
Root Cause
Filename search treated path-like input as ordinary name fragments. WSL also only enumerated files and had a direct-path command escaping issue through
wsl.exe; Windows profile filename search relied on indexed filename matching for queries that need literal full-path matching.Validation
npm testpassed: 85/85.C:\Users\Property Partner\Downloads\tdd-seam-and-deep-modules-prd-final.md./home/prop_/.codexinOpenCockLab.Summary by cubic
Fixes filename search when queries look like paths on Windows and WSL. Supports exact file and directory paths, WSL UNC inputs, hidden dot paths, slashes, and hyphens for more reliable results.
System.ItemPathDisplay(full path) and detect path-like queries; route those to a filesystem scan with hidden files enabled and noisy dirs pruned.\\wsl.localhost\<distro>\..., infer distro/path, and escape$in shell args.Written for commit cc0abd7. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
New Features