fix(workspace-changes): record symlink targets without the verbatim prefix (#5250)

* fix(workspace-changes): record symlink targets without the verbatim prefix

os.readlink on Windows reports absolute targets in extended-length
form (\?\C:\... or \?\UNC\server\share). The scanner stored that raw
spelling, so workspace-change events showed \?\-prefixed targets that
do not match ordinary Windows paths. Strip the prefix when recording;
POSIX readlink output is unchanged. Skills projection/review readlink
sites are untouched — they have no user-facing contract pinned on the
spelling.

* fix(workspace-changes): gate symlink target normalization to Windows

Review follow-up on #5250:

- Gate _normalize_symlink_target on os.name == "nt". readlink(2) on
  POSIX returns the literal string the link was created with, and
  backslash is a valid filename byte on Linux, so a target that starts
  with the extended-length prefix there must be recorded verbatim. The
  docstring's POSIX claim is now provably true.
- Commit the unit checks the PR body previously described as ad-hoc:
  drive and UNC prefix stripping, relative and plain POSIX targets,
  mid-string prefix left verbatim, and an off-Windows identity case, so
  the new branch has real coverage on every platform instead of relying
  on a Windows host with symlink privilege.

* test(workspace-changes): force Windows platform in prefix-strip unit tests

The os.name gate added in the previous commit makes
_normalize_symlink_target a verbatim identity off-Windows, so the two
prefix-strip assertions failed on the ubuntu-only unit CI. Force
os.name to "nt" via monkeypatch in both, mirroring the off-Windows
identity test, so every case pins exactly one platform's contract and
the suite is green on every host.

* fix(workspace-changes): strip only extended drive-letter prefixes

Review follow-up on #5250: the catch-all branch also stripped the
extended-length prefix from volume-GUID targets (\?\Volume{...}\...),
leaving a relative-looking path that loses the target's namespace.
Restrict the branch to extended drive-letter paths (letter, colon,
separator) and keep every other \?\ namespace form verbatim; add the
volume-GUID regression plus degenerate-prefix cases.
This commit is contained in:
Shxiao 2026-09-08 15:51:57 +09:00 committed by GitHub
parent 9ad79baf97
commit 48bbea6df3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 78 additions and 1 deletions

View File

@ -259,6 +259,33 @@ def _snapshot_file(
)
def _normalize_symlink_target(target: str) -> str:
"""Strip the Windows extended-length prefix from a symlink target.
``os.readlink`` on Windows reports absolute targets in extended-length
form (``\\\\?\\C:\\...`` or ``\\\\?\\UNC\\server\\share``). Recorded targets
are surfaced in workspace-change events and compared against ordinary
paths, so keep the plain spelling.
On POSIX this is a provable identity: the strip only applies on Windows
hosts. ``readlink(2)`` returns the literal string the link was created
with, and backslash is a valid filename byte on Linux a target string
that merely starts with ``\\\\?\\`` there must be recorded verbatim.
On Windows, only extended *drive-letter* paths are stripped. Other
``\\\\?\\`` namespace forms (volume-GUID paths, device paths) are kept
verbatim: stripping them would leave a relative-looking remainder that
no longer names the target's namespace.
"""
if os.name != "nt":
return target
if target.startswith("\\\\?\\UNC\\"):
return "\\\\" + target[len("\\\\?\\UNC\\") :]
if target.startswith("\\\\?\\") and len(target) >= 7 and target[4].isascii() and target[4].isalpha() and target[5] == ":" and target[6] in "\\/":
return target[4:]
return target
def _snapshot_symlink(root: WorkspaceRoot, host_file: Path) -> FileSnapshot | None:
# Deliberately never follows the link (no read_bytes()/open() on the target):
# the target may point anywhere on the host, including outside the scanned
@ -278,6 +305,8 @@ def _snapshot_symlink(root: WorkspaceRoot, host_file: Path) -> FileSnapshot | No
target = os.readlink(host_file)
except OSError:
target = None
else:
target = _normalize_symlink_target(target)
return FileSnapshot(
path=virtual_path,

View File

@ -1,5 +1,6 @@
from __future__ import annotations
import os
from pathlib import Path
import pytest
@ -19,7 +20,11 @@ from deerflow.workspace_changes import (
scan_workspace_roots,
)
from deerflow.workspace_changes.api import get_workspace_changes_response
from deerflow.workspace_changes.scanner import SAMPLE_BYTES, is_sensitive_workspace_path
from deerflow.workspace_changes.scanner import (
SAMPLE_BYTES,
_normalize_symlink_target,
is_sensitive_workspace_path,
)
def _roots(tmp_path):
@ -741,3 +746,46 @@ async def test_workspace_changes_route_forwards_include_files_flag():
assert response["available"] is True
assert response["files"] == []
assert calls["event_types"] == ["workspace_changes"]
def test_normalize_symlink_target_strips_extended_length_drive_prefix(monkeypatch):
# The strip only applies on Windows hosts, so force the platform here;
# this test runs on the ubuntu-only CI too.
monkeypatch.setattr(os, "name", "nt")
assert _normalize_symlink_target(r"\\?\C:\Users\u1\target.txt") == r"C:\Users\u1\target.txt"
def test_normalize_symlink_target_strips_extended_length_unc_prefix(monkeypatch):
monkeypatch.setattr(os, "name", "nt")
assert _normalize_symlink_target(r"\\?\UNC\server\share\a.txt") == r"\\server\share\a.txt"
def test_normalize_symlink_target_preserves_volume_guid_and_degenerate_prefixes(monkeypatch):
monkeypatch.setattr(os, "name", "nt")
# Volume-GUID paths are absolute Windows targets in their own namespace;
# stripping the prefix would leave a relative-looking path.
target = r"\\?\Volume{01234567-89ab-cdef-0123-456789abcdef}\folder\target.txt"
assert _normalize_symlink_target(target) == target
# Only a drive letter followed by a colon and a separator is a drive path.
for degenerate in (r"\\?\C:", r"\\?\1:\x", r"\\?\:"):
assert _normalize_symlink_target(degenerate) == degenerate
def test_normalize_symlink_target_leaves_relative_and_plain_posix_targets_verbatim():
assert _normalize_symlink_target("relative/target.txt") == "relative/target.txt"
assert _normalize_symlink_target("/tmp/target.txt") == "/tmp/target.txt"
def test_normalize_symlink_target_leaves_mid_string_prefix_verbatim():
# Backslash is a legal filename byte on POSIX, so only a *leading*
# extended-length prefix may ever be stripped.
for target in (r"/data/\\?\weird-target.txt", r"C:\data\\?\nested.txt"):
assert _normalize_symlink_target(target) == target
def test_normalize_symlink_target_is_identity_off_windows(monkeypatch):
# The strip is gated on Windows hosts: readlink(2) on POSIX returns the
# literal string the link was created with, so a target that starts with
# "\\?\" there must be recorded verbatim.
monkeypatch.setattr(os, "name", "posix")
assert _normalize_symlink_target(r"\\?\C:\Users\u1\target.txt") == r"\\?\C:\Users\u1\target.txt"