fix: address review findings from PR #1383 Windows path separator fix

[dyoshikawa]

Summary

• .replace(/\\/g, "/") パターンを toPosixPath() ユーティリティ関数に抽出（src/utils/file.ts）
• ai-file.ts, ai-dir.ts, fetch.ts の3箇所で toPosixPath を使用するようリファクタリング
• getRelativePathFromCwd() に正規化戦略の違いを説明するコードコメントを追加
• fetch.test.ts を it.each で Windows パス入力もテストするよう改善
• coding-guidelines を更新: ファイルシステムパスと非ファイルシステムパスの使い分けを明記
• タイポ修正: condsider → consider

Test plan

• 全 4608 テスト通過
• pnpm cicheck (lint, type check, tests, cspell, secretlint) 全通過
• Windows スタイルのバックスラッシュ入力が正しく正規化されることをテストで確認

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR follows up on PR #1383 to clarify and harden Windows/posix path handling across Rulesync, especially where forward-slash “semantic” paths are required (GitHub API paths, generated references, gitignore-style patterns).

Changes:

• Added explanatory comments in AiFile/AiDir explaining why backslash normalization uses path.join(...).replace(/\\/g, "/").
• Normalized fetch’s resolved repository subdirectory path for GitHub API compatibility and expanded tests to cover POSIX vs Windows-style inputs.
• Updated internal coding guidelines to distinguish filesystem paths (path.join) from semantic/API paths (path.posix.join or backslash replacement).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/types/ai-file.ts	Adds rationale comment for POSIX normalization strategy in display/semantic paths.
src/types/ai-dir.ts	Adds rationale comment for POSIX normalization strategy in display/semantic paths.
src/lib/fetch.ts	Normalizes backslashes in the repo subdirectory path used for GitHub API requests.
src/lib/fetch.test.ts	Parameterizes the GitHub API path assertion across POSIX and Windows-style inputs.
.rulesync/rules/coding-guidelines.md	Updates repo guidelines to distinguish filesystem vs non-filesystem path handling.
.github/instructions/coding-guidelines.instructions.md	Mirrors the guideline update for GitHub instruction consumption.

[Copilot]

resolvedPath is normalized by replacing backslashes, but it can still end up with leading/trailing / (e.g. user passes packages\\shared\\ or packages/shared/). collectFeatureFiles() later derives relativePath via file.path.substring(basePath.length + 1), which assumes basePath has no trailing slash; a trailing / will drop the first character of the relative path (e.g. ules/overview.md). Consider additionally trimming leading/trailing slashes from resolvedPath (and/or computing the relative path using a safer prefix check / posix.relative) so basePath is in a consistent normalized form.

Suggested change

	const resolvedPath = (options.path ?? parsed.path ?? ".").replace(/\\/g, "/");
	let resolvedPath = (options.path ?? parsed.path ?? ".").replace(/\\/g, "/");
	// Trim leading/trailing slashes so base paths are in a consistent form.
	resolvedPath = resolvedPath.replace(/^\/+|\/+$/g, "");
	if (resolvedPath === "") {
	resolvedPath = ".";
	}

[Copilot]

The new comment mentions (relativeDirPath, dirName) may already contain backslashes on Windows, but dirName is explicitly validated in the constructor to reject any / or \\. To keep the comment accurate (and avoid implying dirName might contain separators), it should refer only to relativeDirPath (or otherwise clarify which segment can contain backslashes).

Suggested change

	// Use path.join + replace instead of path.posix.join because input segments
	// (relativeDirPath, dirName) may already contain backslashes on Windows.
	// path.posix.join does not normalize backslashes, so we must use path.join
	// (which handles OS-native separators) and then normalize to POSIX format.
	// Use path.join + replace instead of path.posix.join because relativeDirPath
	// may already contain backslashes on Windows. dirName is validated to be a
	// simple directory name without separators. path.posix.join does not normalize
	// backslashes, so we must use path.join (which handles OS-native separators)
	// and then normalize to POSIX format.

[Copilot]

The parameterized test covers backslashes in the source path, but it doesn’t cover trailing separators (e.g. owner/repo:packages\\shared\\ / owner/repo:packages/shared/). Those inputs are plausible (especially from Windows shells) and currently interact badly with basePath.length + 1 relative-path slicing. Consider adding trailing-slash/backslash variants here so the normalization logic is exercised and guarded against regressions.

Suggested change

	["Windows style input (backslashes in subdir)", "owner/repo:packages\\shared"],
	["Windows style input (backslashes in subdir)", "owner/repo:packages\\shared"],
	["POSIX style input with trailing slash", "owner/repo:packages/shared/"],
	["Windows style input with trailing backslash", "owner/repo:packages\\shared\\"],

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/lib/fetch.test.ts:686

• This mock’s branching logic (path.endsWith("/rules") / !path.includes(".")) is quite permissive and no longer validates that fetchFiles is calling the GitHub API with the expected normalized directory (e.g. it would accept any directory path without a dot). To keep the test meaningful, consider deriving the expected base path from source and asserting exact matches (or a small explicit set of allowed paths) rather than using heuristics.

        if (path.endsWith("/rules")) {
          return Promise.resolve([
            {
              name: "overview.md",
              path: `${path}/overview.md`,
              type: "file",
              sha: "abc",
              size: 100,
              download_url: "https://example.com",
            },
          ]);
        }
        if (!path.endsWith("/rules") && !path.includes(".")) {
          return Promise.resolve([
            {
              name: "mcp.json",
              path: `${path}/mcp.json`,
              type: "file",
              sha: "def",
              size: 50,
              download_url: "https://example.com",
            },
          ]);
        }

[Copilot]

toPosixPath() currently does a 1:1 backslash replacement, which turns consecutive backslashes into doubled forward slashes (e.g. "packages\\\\shared" -> "packages//shared"). For API paths (GitHub contents API) and generated config paths, empty path segments can change the meaning and may cause lookups to fail. Consider collapsing runs of separators so the output never contains // (e.g. replace one-or-more backslashes and also collapse multiple /).

Suggested change

	* Converts OS-native path separators to POSIX forward slashes.
	* Use this instead of `path.posix.join` when input segments may already
	* contain backslashes (e.g., on Windows), because `path.posix.join` does
	* not normalize backslashes.
	*/
	export function toPosixPath(p: string): string {
	return p.replace(/\\/g, "/");
	* Converts OS-native path separators to normalized POSIX forward slashes.
	* Collapses any run of `\` or `/` into a single `/`.
	* Use this instead of `path.posix.join` when input segments may already
	* contain backslashes (e.g., on Windows), because `path.posix.join` does
	* not normalize backslashes.
	*/
	export function toPosixPath(p: string): string {
	return p.replace(/[\\/]+/g, "/");

[Copilot]

The toPosixPath test case for "consecutive backslashes" asserts a double-slash result (packages//shared). This bakes in behavior that’s likely undesirable for API/config paths (empty segment) and would mask regressions if toPosixPath is improved to normalize separators. Suggest updating the expectation to a single slash and aligning the helper implementation accordingly.

Suggested change

	["consecutive backslashes", "packages\\\\shared", "packages//shared"],
	["consecutive backslashes", "packages\\\\shared", "packages/shared"],

[Copilot]

"ballel" appears to be a typo; did you mean "barrel" (as in barrel files)? It’s copied into this new doc, so fixing it here would prevent propagating the misspelling.

Suggested change

	- Don't create ballel files. Please always direct import the implementation file.
	- The maintainer thinks that ballel files are harmful to tree-shaking and import path transparency.
	- Don't create barrel files. Please always direct import the implementation file.
	- The maintainer thinks that barrel files are harmful to tree-shaking and import path transparency.

[Copilot]

"ballel" appears to be a typo; did you mean "barrel" (as in barrel files)? Fixing it here would avoid spreading the misspelling across tool-specific copies of the guidelines.

Suggested change

	- Don't create ballel files. Please always direct import the implementation file.
	- The maintainer thinks that ballel files are harmful to tree-shaking and import path transparency.
	- Don't create barrel files. Please always direct import the implementation file.
	- The maintainer thinks that barrel files are harmful to tree-shaking and import path transparency.

[dyoshikawa]

@dyoshikawa Thank you!