Skip to content

fix(windows): stat paths through the long-path-aware converter - #1445

Open
mlandolfi90 wants to merge 1 commit into
DeusData:mainfrom
mlandolfi90:fix/path-info-long-paths
Open

fix(windows): stat paths through the long-path-aware converter#1445
mlandolfi90 wants to merge 1 commit into
DeusData:mainfrom
mlandolfi90:fix/path-info-long-paths

Conversation

@mlandolfi90

Copy link
Copy Markdown

cbm_path_info_utf8 converts with cbm_utf8_to_wide, the plain UTF-8 converter, while every other filesystem entry point in the same file — cbm_opendir, cbm_fopen, cbm_rename_replace, cbm_canonical_path — uses cbm_path_to_wide, which canonicalises and applies the \\?\ prefix past the legacy limit.

The mismatch is observable within a single walk: cbm_opendir successfully enumerates a deep directory because it prefixes, then the per-entry stat fails because it does not. GetFileAttributesExW gets a bare path and returns ERROR_PATH_NOT_FOUND.

Consequences, both in the semantic-manifest code:

  • semantic_manifest_walk_controls — one unstattable entry aborts the entire walk (CBM_NOT_FOUND).
  • semantic_manifest_hash_file — returns CBM_NOT_FOUND, so the file is silently dropped from the manifest.

Either way the index degrades with no error surfaced. Machines with LongPathsEnabled=0 are the exposed case, and a mingw-cross PE carries no longPathAware manifest, so flipping that registry policy is not a workaround there.

One-word change, consistent with the convention the rest of the file already follows.

🤖 Generated with Claude Code

cbm_path_info_utf8 converted with cbm_utf8_to_wide, the plain UTF-8
converter, while every other filesystem entry point in this file
(cbm_opendir, cbm_fopen, cbm_rename_replace, cbm_canonical_path) uses
cbm_path_to_wide, which canonicalises and applies the \?\ prefix past
the legacy limit.

The mismatch is observable: cbm_opendir enumerates a deep directory
because it prefixes, then the per-entry stat fails because it does not.
On a machine with LongPathsEnabled=0 that makes
semantic_manifest_walk_controls abort the whole walk (CBM_NOT_FOUND) and
semantic_manifest_hash_file silently drop files from the manifest, so
the index degrades with no error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: mlandolfi90 <mlandolfi90@users.noreply.github.com>
@mlandolfi90
mlandolfi90 requested a review from DeusData as a code owner August 4, 2026 19:08
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@mlandolfi90

Copy link
Copy Markdown
Author

On CI, since the acknowledgement asks for green or an explanation:

test / test-windows-guards — I believe this is a flake, not this change. test_non_ascii_path.py went red on exactly one of its four variants:

[PASS] non-ascii/latin1_accents nodes=12 edges=21 definitions=5 (baseline 12/21/5)
[FAIL] non-ascii/cyrillic       nodes=None edges=None definitions=None
[PASS] non-ascii/cjk            nodes=12 edges=21 definitions=5
[PASS] non-ascii/greek          nodes=12 edges=21 definitions=5

Two reasons I do not think the diff can explain it:

  1. This change is a no-op for the path under test. cbm_path_to_wide returns exactly what cbm_utf8_to_wide returns unless the path is drive-absolute and wcslen >= 240; below the threshold it is the same early return. The guard's path (C:\Users\runneradmin\cbm-windows-guards-root-<32 hex>\cbm_win_nonascii_<8>\проект_repo\src\math.ts) is roughly 126 wide chars, so the branch this PR touches is never taken. If it did regress non-ASCII handling, it would also have to explain why the Cyrillic variant differs from the Greek and CJK ones, which take the identical code path.

  2. The failure shape is wrong for this bug. Windows: tree-sitter definitions pass fails on ALL files when repo_path contains non-ASCII (CJK / Cyrillic / Arabic / accented Latin / etc.) characters #636 produced File/Folder nodes with zero definitions. Here the counts are None — the tool call returned nothing at all, which is a server/startup failure, not an extraction one.

A re-run would settle it. Happy to be shown wrong if it reproduces.

security / codeql-gate timed out with the analysis still queued after 90 polls (BLOCKED: CodeQL timeout) — no alerts, just never scheduled. I opened five PRs in quick succession, so this may well be queue pressure I caused; apologies if so.

🤖 Generated with Claude Code

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.

1 participant