fix: resolve Windows path separator inconsistency and make tests OS-independent#1
Merged
fix: resolve Windows path separator inconsistency and make tests OS-independent#1
Conversation
flanny7
force-pushed
the
fix/path-backslash
branch
from
74eea7f to
02a51e4
Compare
GitHub API and .gitignore entries require forward slashes, but path.join() produces backslashes on Windows. This caused fetch command failures with subdirectory paths and gitignore entries with mixed separators. Changes: - src/constants/rulesync-paths.ts: Use path.posix.join to ensure constants always contain forward slashes. When used in filesystem operations (e.g., join(cwd, constant)), path.join automatically normalizes to platform separators. - src/lib/fetch.ts: Use posix.join for GitHub API path construction instead of platform-dependent join. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Add tests verifying that: - rulesync-paths constants always use forward slashes - .gitignore entries use only forward slashes - GitHub API paths never contain backslashes These tests catch Windows path separator issues that would break fetch and gitignore commands on Windows systems. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Normalize backslashes to forward slashes in AiFile.getRelativePathFromCwd() and AiDir.getRelativePathFromCwd(). These methods produce paths that are written into generated rule file content (e.g., @.cursor/rules/my-rule.md) via rules-processor.ts. On Windows, when rules are in subdirectories, path.relative() returns backslash-separated paths, causing getRelativePathFromCwd() to produce paths with backslashes—invalid syntax for rule files. The fix applies .replace(/\\/g, "/") to normalize the output, ensuring cross-platform consistency regardless of the host OS. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Add tests to verify that getRelativePathFromCwd() normalizes backslashes to forward slashes: - tool-file.test.ts: Test AiFile.getRelativePathFromCwd() with backslash in relativeFilePath (simulating Windows path.relative() output) - ai-dir.test.ts (new): Test AiDir.getRelativePathFromCwd() with backslash in relativeDirPath Both tests verify that the return value never contains backslashes, ensuring paths written into rule file content are always cross-platform compatible. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Replaced host-OS-dependent path tests with `it.each` blocks that explicitly test both Windows-style and POSIX-style path inputs. This prevents false positives on macOS/Linux CI runners and ensures path sanitization logic is robust without destructively mocking `node:path`.
Fix it.each test in tool-file.test.ts that declared relativeDirPath and relativeFilePath parameters but hardcoded values in the TestToolFile constructor, making the Windows-style path case never actually exercised. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
flanny7
force-pushed
the
fix/path-backslash
branch
from
697d957 to
28a23fe
Compare
flanny7
pushed a commit
that referenced
this pull request
Apr 2, 2026
…arator fix - Add explanatory comments in ai-file.ts and ai-dir.ts for why .replace() is used instead of path.posix.join (#1) - Improve fetch.test.ts with parameterized Windows-style backslash path test inputs via it.each (#2) - Normalize backslashes in fetch.ts resolvedPath for API compatibility - Update coding-guidelines.md to distinguish filesystem paths (path.join) from semantic/API paths (path.posix.join) (dyoshikawa#3) Closes dyoshikawa#1394 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
flanny7
pushed a commit
that referenced
this pull request
Apr 2, 2026
…d gitignore sync - Enforce both-or-neither opts in buildDeletionRulesFromPaths (#1) - Rename destructured param in fromRootFile for clarity (#2) - Remove redundant optional chaining after nonRoot guard (dyoshikawa#3) - Remove duplicate **/.rovodev/ gitignore entry (#4) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
flanny7
pushed a commit
that referenced
this pull request
Apr 22, 2026
- #1: use static import for resetDeprecationWarningForTests in tests - #2: document why mutual-exclusivity is runtime-enforced, not a discriminated union - dyoshikawa#3: stop emitting the deprecation warning from the Config constructor; the ConfigResolver is now the single emission point - #4: cache validated ToolTarget[] for object-form targets in the constructor so getTargets() no longer rebuilds the ALL_TOOL_TARGETS set per call - #5: fix misleading schema comment that claimed unknown-target warnings (the runtime path actually throws) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
flanny7
pushed a commit
that referenced
this pull request
Apr 22, 2026
…nd match-all bypasses - Reject imported rules whose toolName maps to __proto__, constructor, or prototype to prevent prototype pollution when round-tripping untrusted TOML; use Object.hasOwn for lookups to avoid hitting inherited accessors. (Sec #1) - Stop translating glob character classes to regex classes; emit '[' and ']' as literals so that negated ([^a]) or wide-range ([!-~]) classes cannot bypass the JSON field-boundary guard. (Sec #2) - Skip empty patterns ('') with a warning (would match every bash invocation or nothing for other tools). Skip bash '*' and '**' with allow/deny decisions because they would silently grant or revoke every shell command; 'ask' remains supported. (Sec dyoshikawa#3) - Update docs to reflect the new guardrails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
flanny7
pushed a commit
that referenced
this pull request
Apr 22, 2026
Address Round 2 review findings for PR dyoshikawa#1526: - HIGH-R2-#1: guard the stale-file cleanup loop in apm-install.ts against path traversal. Attacker-controlled deployed_files entries with ".." segments or absolute paths are now rejected by shape and via checkPathTraversal, with a warn log per offending entry, so a hostile lockfile cannot drive arbitrary removeFile calls. - MID-R2-#2: make lockfile ordering deterministic for failed deps. The per-dep worker now returns the preserved prior entry via its result object, and the sequential post-loop pushes successes or preserved entries strictly in manifest order, not in promise-completion order. - MID-R2-dyoshikawa#3: preserve top-level loose fields (mcp_servers and any looseObject extras) across lockfile rewrites by carrying forward existingLock through createEmptyApmLock. - MID-R2-#4: relax the content_hash schema to accept arbitrary strings on parse so a lockfile produced by the upstream apm CLI does not break readApmLock. The --frozen integrity check now only compares hashes whose shape matches RULESYNC_CONTENT_HASH_REGEX and skips comparison otherwise (commit SHA pin still enforces integrity). Tests added for each finding, including a two-dep ordering regression and a frozen-mode interop check with a legacy content_hash value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello,
I always appreciate your work on rulesync.
Following up on Issue dyoshikawa#1251 and related Windows path behaviors, I have created a Pull Request to resolve the path separator inconsistency and make the tests OS-independent.
Description
Closes dyoshikawa#1251
This PR fixes the Windows path separator inconsistency that caused backslashes (
\) to appear in contexts requiring forward slashes (/), and implements OS-independent parameterized tests to ensure cross-platform reliability.Root Cause
path.join()fromnode:pathuses the platform separator — backslash on Windows. The codebase usedpath.join()in four non-filesystem contexts that require forward slashes:.gitignoreentries — The gitignore spec requires forward slashes. On Windows,RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATHresolved to.rulesync\skills\.curated, producing an invalid gitignore entry.fetch.tsusedpath.join(basePath, featurePath)to build repository paths sent to the GitHub Contents API. On Windows this produced paths likepackages\shared\rules, which the API cannot resolve.@-references —AiFile.getRelativePathFromCwd()usedpath.join()to produce display paths that are written verbatim into generated rule files (e.g.,@.cursor/rules/sub/my-rule.mdin TOON format and Claude Code legacy format). On Windows, when a rule file is in a subdirectory,path.relative()returnssub\my-rule.md, causinggetRelativePathFromCwd()to produce@.cursor/rules/sub\my-rule.md— broken syntax.AiDir.getRelativePathFromCwd()had the same issue (used for dry-run logging and changed-path reporting).Affected Components
gitignorecommand.gitignorecontained backslash entries on Windowsfetchcommand@-references in generated root rule files contained backslashes on WindowsChanges Made
src/constants/rulesync-paths.ts: Changedpath.jointopath.posix.joinso constants always use forward slashes. When these constants are later used in filesystem operations (e.g.,path.join(process.cwd(), constant)),path.joinnormalizes them to the platform separator automatically.src/lib/fetch.ts: Changedjoin(basePath, featurePath)toposix.join(basePath, featurePath)for the GitHub API path construction.src/types/ai-file.ts&src/types/ai-dir.ts: Added.replace(/\\/g, "/")togetRelativePathFromCwd()so paths written into rule file content always use forward slashes, regardless of whatpath.join()produces on the host OS.Testing Strategy & Added Tests
To prevent false positives on macOS/Linux CI runners (where
path.joinnatively returns/), the tests have been designed to be strictly OS-independent:src/types/tool-file.test.ts,src/types/ai-dir.test.ts,src/lib/fetch.test.ts): Instead of destructively mockingnode:path, we useit.eachto explicitly inject both Windows-style (\\) and POSIX-style (/) path strings. This guarantees that the path sanitization logic works consistently across all platforms without breaking native OS testing capabilities.src/constants/rulesync-paths.test.ts,src/cli/commands/gitignore.test.ts): Added assertions to verify that dynamically generated constants and gitignore entries never contain backslashes (.not.toContain("\\")). (Any regressions in base paths will be caught by Windows runners in the CI matrix).Please let me know if there are any formatting issues or if I missed anything in the contribution guidelines. I would be happy to fix them.
There is no rush at all for the review. Your time and consideration are greatly appreciated.