fix(skills): normalize backslashes in compacted skill paths on Windows#52200
fix(skills): normalize backslashes in compacted skill paths on Windows#52200chienchandler wants to merge 2 commits intoopenclaw:mainfrom
Conversation
On Windows, compactSkillPaths() replaces the home directory prefix with ~/ but leaves remaining path segments with backslashes, producing hybrid paths like ~/AppData\Roaming\...\SKILL.md. Models fail to resolve these malformed paths and return ENOENT errors. Normalize all backslashes to forward slashes after path compaction on Windows so models receive consistent POSIX-style paths. Fixes openclaw#52022
Greptile SummaryThis PR fixes a Windows-only bug where The fix is a one-liner
Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/skills.compact-skill-paths.test.ts
Line: 63-66
Comment:
**Conditional assertion may silently pass without testing anything**
The `if (locationMatch)` guard means the `expect` only runs when the prompt actually contains a `<location>` tag. If the prompt format changes or `buildWorkspaceSkillsPrompt` returns something unexpected, the test still reports green — providing a false sense of coverage.
Consider asserting unconditionally that a `<location>` tag exists first, then checking its content:
```suggestion
// The prompt should contain a location tag
const locationMatch = prompt.match(/<location>([^<]+)<\/location>/);
expect(locationMatch).not.toBeNull();
expect(locationMatch![1]).not.toContain("\\");
```
This way the test fails loudly if the output format ever changes, rather than silently skipping the backslash assertion.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(skills): normalize backslashes in co..." | Re-trigger Greptile |
Address greptile review: replace silent conditional guard with explicit assertion so the test fails loudly if prompt format changes.
|
CI note: Both failing checks (node test shard 2/2 and windows test shard 6/6) fail on the same unrelated test — |
|
Codex review: needs changes before merge. Summary Reproducibility: yes. Source inspection gives a high-confidence path: on Windows, Next step before merge Security Review findings
Review detailsBest possible solution: Normalize prompt-facing Windows skill locations at the current compaction helper boundary, add focused regression coverage, add a single-line Do we have a high-confidence way to reproduce the issue? Yes. Source inspection gives a high-confidence path: on Windows, Is this the best way to solve the issue? No, not as-is. The normalization direction is the narrow maintainable fix, but it should be adapted to current main's Full review comments:
Overall correctness: patch is correct Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 06056926a099. |
What
On Windows,
compactSkillPaths()produces hybrid paths like~/AppData\Roaming\npm\...\SKILL.md— the home prefix gets replaced with~/but remaining segments keep backslashes. Models can't resolve these and return ENOENT.Fixes #52022
How
Added backslash-to-forward-slash normalization after the
~/prefix replacement, gated behindpath.sep === "\\"so it only runs on Windows. Same pattern asrefresh.ts:81.Testing
compactSkillPathstests pass (including new test for backslash normalization)AI-assisted (Claude Code), reviewed and tested by author.