Skip to content

fix(wiki): POSIX wiki paths and byte-exact decision fixtures on Windows (#692, class 8) - #896

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/692-crlf-and-posix-wiki-paths
Nov 6, 2026
Merged

fix(wiki): POSIX wiki paths and byte-exact decision fixtures on Windows (#692, class 8)#896
github-actions[bot] merged 1 commit into
mainfrom
fix/692-crlf-and-posix-wiki-paths

Conversation

@cdeust

@cdeust cdeust commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Summary

Class 8 of #692 (23 Windows failures listed as "suspected CRLF"). The Windows log of run 38008915302 (job 114084097957) shows two separate root causes:

  • Separators (14 tests, test_wiki_bulk_migrate 5 + test_wiki_rebucket_file_docs 9): a product defect. plan() in both scripts recorded str(md.relative_to(wiki_root)), which uses \ on Windows. The detectors split on /, so timestamp-slug and the file-doc notes were not seen, and the stub's redirect_to stored adr\_general\2234-zero-dependencies.md. Both now use rel.as_posix().
  • Newlines (9 tests, test_migrate_decision_blocks): a test defect. The apply helper wrote the fixture with write_text, which turns \n into \r\n on Windows, while apply_selected hashes the file's bytes. The block id taken from the LF source then named no block: selection must identify exactly one unchanged candidate. The fixture is now written byte for byte. The product already handles CRLF sources (test_crlf_source_keeps_line_endings_and_content_address).

Refs #692 (one class out of several; it does not close the issue).

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test plan

  • pytest tests_py/scripts/test_migrate_decision_blocks.py tests_py/scripts/test_wiki_bulk_migrate.py tests_py/scripts/test_wiki_rebucket_file_docs.py: 55 passed (Linux, Python 3.11).
  • pytest tests_py/scripts: 1804 passed, 15 skipped.
  • Windows simulated on Linux, with a pytest plugin outside the repo:
    • Newlines: Path.write_text forced to newline="\r\n". On main, test_migrate_decision_blocks fails 9 tests, the same 9 ids as the Windows run. On this branch: 14 passed.
    • Separators: Path.relative_to returns a PureWindowsPath. On main, 15 failures: the 14 ids of the Windows run, plus test_apply_is_idempotent. On this branch: 41 passed.
  • ruff check / ruff format --check on the changed files: OK. python scripts/check_craftsmanship.py: OK.
  • Mutation: the mutation gate (ADR-1122) only covers mcp_server. These two scripts are outside it.

Audit notes

  • Not verified: nothing ran on real Windows. The Windows re-measure from Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692 is still to be done.
  • Same str(relative_to(...)) pattern spotted elsewhere, outside class 8 and not fixed here: scripts/wiki_pilot_migration.py:454, mcp_server/infrastructure/wiki_decision_mirror.py:50-53, mcp_server/handlers/wiki_purge.py:284, mcp_server/handlers/consolidation/page_io.py:204, mcp_server/handlers/codebase_analyze.py:463. Whether each one is a defect has not been diagnosed.

Coding-standards compliance

  • §2.2 Layers unchanged (scripts and tests only).
  • §4.1 / §4.2 No file or function lengthened (inline comment only).
  • §8 No new constant.

Breaking changes

None. On Linux/macOS, as_posix() gives the same thing as str().

Reviewer checklist

  • CHANGELOG.md updated (Unreleased › Fixed).
  • No secrets / credentials / PII in the diff.

Generated by Claude Code

@claude
fix(wiki): POSIX wiki paths and byte-exact decision fixtures on Windo…
d18493a 
…ws (#692, class 8)

wiki_bulk_migrate.plan and wiki_rebucket_file_docs.plan took str() of the
page's relative path, which uses "\" on Windows. The detectors split on "/"
and the stub's redirect_to stored the backslash form (Windows run 38008915302:
'adr\_general\2234-zero-dependencies.md'). Both now use as_posix().

test_migrate_decision_blocks wrote its fixture with write_text, which
translates "\n" to "\r\n" on Windows, while apply_selected hashes the file's
bytes: the block id taken from the LF source named no block ("selection must
identify exactly one unchanged candidate", 9 tests). The fixture is now
written byte for byte; the product already handles CRLF sources
(test_crlf_source_keeps_line_endings_and_content_address).

Refs #692

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYs9LU5DzBCETpKyqtH
@cdeust cdeust mentioned this pull request Nov 5, 2026
Open
@cdeust
cdeust marked this pull request as ready for review November 6, 2026 14:19
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T14:20:46.095126Z d18493a Draft marked ready
🔒 Security Review ✅ Completed 2026-11-06T14:22:23.024735Z d18493a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @cdeust's task in 1m 43s —— View job

PR #896 review (1/3)

  • Gather context and diff
  • Run five reviews: code quality, performance, test coverage, documentation accuracy, security
  • Check each finding against the code
  • Post the review

The review is in this comment. It has no Blockers and three Notes, none of which needs action before merge:

  • test_migrate_decision_blocks.py:80 and :91 still use write_text, so on Windows the "appears twice" case passes without being tested.
  • The CHANGELOG and the test comment say the block id is a hash of the file's bytes. It is actually a hash of the block's lines.
  • Linux CI would not catch a revert to str(rel).

Verdict: approve

@claude

claude Bot commented Nov 6, 2026

Copy link
Copy Markdown

Review 1/3: PR #896

I ran five reviews: code quality, performance, test coverage, documentation accuracy and security. I checked every finding myself against d18493a.

Blockers: none. In both scripts, every downstream use of rel_str now gets /: the _stem split, _FILE_NOTE_PATH_RE, _disambiguate, wiki_root / item.rel_path and the rename handler. Neither script has another str(relative_to(...)). The path change does not touch containment: relative_to still works on files rglob found under wiki_root, and wiki_store still runs its realpath checks.

Notes, no action before merge

  • tests_py/scripts/test_migrate_decision_blocks.py:80 and :91 still use write_text. On Windows, line 80 gets 0 matches instead of the 2 it means to test. It still passes, because _selected (scripts/migrate_decision_blocks.py:109) raises the same error for any count other than 1, so the "appears twice" case goes untested there. Using write_bytes(... .encode("utf-8")) would fix it.
  • CHANGELOG.md:91 and the comment at test_migrate_decision_blocks.py:16-17 say the block id is "a hash of the file's bytes". It is actually a SHA-256 of the block's lines with their line endings (migrate_decision_blocks.py:49,55). The CRLF explanation is still right.
  • Linux CI would not catch a revert to str(rel), because both give the same string on POSIX. A test with PureWindowsPath would pin the fix.

Verdict: approve

@github-actions
github-actions Bot merged commit 8bb81f6 into main Nov 6, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cdeust @claude