Skip to content

fix: normalize single Windows path separators in protectedFilePatterns - #403

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-16_fix-windows-path-normalization
Sep 16, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-16_fix-windows-path-normalization

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Summary

Fixes #402 — ported from upstream DCP commit 5f8f33b.

normalizePath() in lib/protected-patterns.ts did replaceAll("\\\\", "/"). In source, "\\\\" is the two-character string \\, so only doubled backslashes were ever replaced. Real Windows paths contain single separators, making the normalization a no-op on the only platform it exists for — protectedFilePatterns silently failed to protect Windows paths at all four isFilePathProtected call sites (protected-content, sweep, deduplication, purge-errors). Only patterns like **/*.ts accidentally worked, because * compiles to [^/]*, which spans backslashes freely.

Changes

  • lib/protected-patterns.ts:2 — replace each single backslash with / (both input path and pattern are normalized identically, so cross-style matching stays consistent and * still compiles to [^/]* without crossing directory boundaries)
  • tests/protected-patterns.test.ts (NEW) — 8 regression tests: Windows path vs forward-slash pattern, pattern written with Windows separators, wildcard boundary semantics, read/multiedit/apply_patch parameter shapes, unmatched-path negatives, isToolNameProtected non-regression, regex metacharacter escaping. Backslashes are built via String.fromCharCode(92) so no future reformatting can reintroduce an escape mistake.

Verification

  • New suite: 8/8 pass on fixed code; 5/8 fail against pre-fix code (verified by temporarily stashing the fix)
  • Typecheck: clean (tsc --noEmit)
  • Full suite: 1271 tests, 0 failures
  • Prettier: changed files clean (repo-wide format:check reports 452 pre-existing unformatted files unrelated to this change; left untouched to keep the diff minimal)

No state-format, config-schema, or API changes. No version bump (content fix).

normalizePath() searched for the two-character string \\(source literal
"\\\\") instead of each single backslash, so real Windows paths were
never normalized and protectedFilePatterns silently failed to protect them.
Ported from upstream DCP commit 5f8f33b. Adds tests/protected-patterns.test.ts
(5 of 8 tests fail against pre-fix code). Fixes #402
@github-actions

Copy link
Copy Markdown
Contributor

📦 Built Plugin Artifact

Branch: 2026-09-16_fix-windows-path-normalization (5035d36)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-403 --global

Each push to this PR publishes a new version under the pr-403 npm tag.

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-09-16_fix-windows-path-normalization" --global

Option C — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf opencode-acp-pr403.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
  1. Restart opencode to pick up changes.

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Reviewing this PR (mine / another agent's prior work — verifying rather than redoing).

Diff reviewed (origin/master...5035d36): clean and focused — 4 files, all on-topic.

  • lib/protected-patterns.ts:2 — normalizePath changed from replaceAll("\\\\", "/") (a two-char string) to replaceAll("\\", "/") (a one-char string). Root-cause analysis holds: the old literal only ever matched doubled backslashes, which real Windows paths don't contain, so the normalisation was a no-op on the only platform it exists for. Both the input path and the pattern are normalised identically, and * still compiles to [^/]*, so a single-segment wildcard does not start crossing directory boundaries. Confirmed correct by reading the full glob→regex compiler.
  • tests/protected-patterns.test.ts (new) — 8 tests importing from real source; covers Windows-vs-forward-slash matching, patterns written with Windows separators, wildcard boundary semantics, read/multiedit/apply_patch param shapes, unmatched negatives, isToolNameProtected non-regression, and regex-metachar escaping. Backslashes built via String.fromCharCode(92) so a future reformat can't reintroduce the exact escape bug being pinned.

CI: pr-validation ✅ and build-artifact ✅ already green; test (22) / test (24) still running.

Independently running typecheck + the new suite + full suite locally to confirm the 8/8-pass / 5-of-8-fail-pre-fix claims. Will report back with the conclusion.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Verification complete — PR #403 is ready for human merge. (I did not redo the work; I fetched the branch and independently verified it.)

Independent local run (detached worktree at head 5035d36, deps reused from a lock-identical sibling):

  • Typecheck: clean (tsc --noEmit, exit 0)
  • New suite tests/protected-patterns.test.ts: 8/8 pass on the fixed code
  • Regression validity: swapped in the pre-fix (origin/master) source and re-ran the same suite → exactly 5/8 fail (tests 1,2,3,4,6; only the 3 negative/non-regression/metacharacter tests pass). Confirms the suite genuinely pins the bug rather than asserting tautologies.
  • Full suite: 1271 tests, 0 failures
  • Prettier: both changed files clean

Code review: the one-line change is correct and minimal — normalizePath goes from replaceAll("\\\\","/") (a two-char string, so it only ever matched doubled backslashes) to replaceAll("\\","/") (one char). Input path and pattern are normalized identically, and * still compiles to [^/]*, so a single-segment wildcard does not start crossing directory boundaries. Faithful port of upstream DCP 5f8f33b.

Diff cleanliness: 4 files, all on-topic — the fix, the new test file, and the required devlog (REQ.md + WORKLOG.md; DESIGN.md correctly omitted since this is a single-function bug fix with no architecture/data-flow/module-boundary impact). No lockfile churn, no version bump (content fix), no unrelated files.

CI (all green): pr-validation ✅ · build ✅ · test (22) ✅ · test (24) ✅ · build-artifact ✅ · e2e ✅. PR is mergeable (no conflicts).

Issue linkage: PR body carries Fixes #402; #402 is the valid originating bug report with correct triage already done. Compliant with §5.1.3.

Nothing to change. Per AGENTS.md §5.1.1.2, merging is a human-only operation — please merge when ready: #403

中文摘要:独立复核了本 PR(非重做)——单行修复正确(归一化从只替换双反斜杠改为替换每个单反斜杠,路径与模式同向归一、* 仍编译为 [^/]* 不跨目录),新增 8 条回归测试在预修复代码上恰好 5/8 失败、修复后 8/8 通过,typecheck 干净、全量 1271 测试 0 失败、改动文件 Prettier 干净,CI 六项全绿且可合并;diff 仅含修复+测试+devlog 共 4 个相关文件、无 version 变更。可以合并(合并操作按规范由人工执行)。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: normalize single Windows path separators in protected file patterns

1 participant