fix(kanban): NFC-normalize worktree path identity checks
macOS hands back DECOMPOSED path strings (NFD) for names stored in composed form (NFC), so raw Path equality in kanban_db_workspace reported a real repo root as 'not inside a git repo' and worktree dispatch died with ValueError for every task on such a board. Add _path_key() (NFC-normalized identity key) and route the four path-identity comparisons on the dispatch path through it: _resolve_worktree_workspace, _ensure_git_worktree, _cleanup_worktree_workspace, and the fallback branch. Patch authored by the issue reporter; independently verified against main by @KeyArgo (RED/GREEN + collateral). The reporter's test file runs green locally on macOS including both macos_only end-to-end rows (3 passed). Fixes #115080
This commit is contained in:
@@ -12,6 +12,7 @@ import shutil
|
||||
import sqlite3
|
||||
import subprocess
|
||||
import time
|
||||
import unicodedata
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
from typing import TYPE_CHECKING
|
||||
@@ -24,6 +25,19 @@ if TYPE_CHECKING:
|
||||
|
||||
_REMOVABLE_KINDS = ("scratch", "worktree")
|
||||
|
||||
|
||||
def _path_key(path: Path | str | None) -> str:
|
||||
"""Unicode-form-insensitive identity for a filesystem path.
|
||||
|
||||
macOS hands back DECOMPOSED path strings (NFD: ``o`` + U+0308) for names the
|
||||
user typed in composed form (NFC: ``ö``) — a OneDrive/FileProvider path like
|
||||
``OneDrive-Persönlich`` round-trips through ``git rev-parse --show-toplevel``
|
||||
as NFD while the DB row holds NFC. Raw ``Path`` equality then reports a real
|
||||
repo root as "not a repo" purely on Unicode form, so every path identity
|
||||
check here goes through this key.
|
||||
"""
|
||||
return unicodedata.normalize("NFC", str(path)) if path is not None else ""
|
||||
|
||||
# Statuses after which a child no longer needs its parent's workspace artifacts.
|
||||
_ACTIVE_CHILDREN_SQL = (
|
||||
"SELECT 1 FROM task_links l "
|
||||
@@ -195,7 +209,7 @@ def _cleanup_worktree_workspace(
|
||||
if common is None or common.name != ".git":
|
||||
return # not a linked worktree of a normal repo — never guess
|
||||
repo_root = common.parent
|
||||
if wp.resolve(strict=False) == repo_root.resolve(strict=False):
|
||||
if _path_key(wp.resolve(strict=False)) == _path_key(repo_root.resolve(strict=False)):
|
||||
return # never remove the main checkout
|
||||
if _worktree_is_dirty(str(wp)) or _worktree_has_unpushed_commits(str(wp)):
|
||||
_kb._log.info(
|
||||
@@ -428,7 +442,7 @@ def _ensure_git_worktree(repo_root: Path, target: Path, branch_name: str) -> Non
|
||||
"""Materialize ``target`` as a linked git worktree under ``repo_root``."""
|
||||
target = target.expanduser()
|
||||
repo_common = _git_common_dir(repo_root)
|
||||
if target.exists() and repo_common is not None and _git_common_dir(target) == repo_common:
|
||||
if target.exists() and repo_common is not None and _path_key(_git_common_dir(target)) == _path_key(repo_common):
|
||||
return
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
if _git_branch_exists(repo_root, branch_name):
|
||||
@@ -501,7 +515,7 @@ def _resolve_worktree_workspace(task: Task, *, board: Optional[str] = None) -> t
|
||||
fallback_root = _repo_root_for_worktree_target(requested.parent)
|
||||
if fallback_root is not None:
|
||||
fallback = fallback_root / ".worktrees" / task.id
|
||||
if fallback.resolve(strict=False) != requested_resolved:
|
||||
if _path_key(fallback.resolve(strict=False)) != _path_key(requested_resolved):
|
||||
_ensure_git_worktree(fallback_root, fallback, branch_name)
|
||||
return fallback.resolve(strict=False), branch_name
|
||||
# No repo to anchor a fallback on (or the occupied path IS this task's
|
||||
@@ -509,7 +523,7 @@ def _resolve_worktree_workspace(task: Task, *, board: Optional[str] = None) -> t
|
||||
return requested_resolved, actual_branch or branch_name
|
||||
|
||||
repo_root = _git_toplevel(requested)
|
||||
if repo_root is not None and requested_resolved == repo_root:
|
||||
if repo_root is not None and _path_key(requested_resolved) == _path_key(repo_root):
|
||||
return _anchored_worktree(repo_root, task.id, branch_name)
|
||||
|
||||
repo_root = _repo_root_for_worktree_target(requested.parent)
|
||||
|
||||
97
tests/hermes_cli/test_kanban_worktree_unicode_path.py
Normal file
97
tests/hermes_cli/test_kanban_worktree_unicode_path.py
Normal file
@@ -0,0 +1,97 @@
|
||||
"""Regression: a composed (NFC) workspace path must resolve against its repo root.
|
||||
|
||||
macOS hands back DECOMPOSED path strings (NFD: ``o`` + U+0308) for names the user
|
||||
typed in composed form (NFC: ``ö``). A OneDrive/FileProvider root such as
|
||||
``OneDrive-Persönlich`` therefore arrives from ``git rev-parse --show-toplevel``
|
||||
as NFD while the kanban DB row holds NFC. The raw ``Path`` comparisons in
|
||||
``_resolve_worktree_workspace`` / ``_ensure_git_worktree`` /
|
||||
``_cleanup_worktree_workspace`` then reported a real repo root as "not inside a
|
||||
git repo" and dispatch died with a ``ValueError`` for every task on that board.
|
||||
|
||||
The identity comparison goes through ``_path_key`` (NFC-normalized), so Unicode
|
||||
form can never decide a path identity question again.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import unicodedata
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import kanban_db_workspace as kbw
|
||||
|
||||
|
||||
def _nfd(text: str) -> str:
|
||||
return unicodedata.normalize("NFD", text)
|
||||
|
||||
|
||||
def _nfc(text: str) -> str:
|
||||
return unicodedata.normalize("NFC", text)
|
||||
|
||||
|
||||
def _git(repo: Path, *args: str) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(
|
||||
["git", "-C", str(repo), *args],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=60,
|
||||
)
|
||||
|
||||
|
||||
def test_path_key_ignores_unicode_form():
|
||||
"""The identity key is form-insensitive but still distinguishes paths."""
|
||||
composed = "/tmp/repo-Persönlich"
|
||||
decomposed = _nfd(composed)
|
||||
|
||||
assert composed != decomposed # the two forms really are different strings
|
||||
assert kbw._path_key(composed) == kbw._path_key(decomposed)
|
||||
assert kbw._path_key(None) == ""
|
||||
assert kbw._path_key("/tmp/repo-Personlich") != kbw._path_key(composed)
|
||||
|
||||
|
||||
@pytest.mark.macos_only
|
||||
def test_nfc_workspace_path_resolves_against_nfd_repo_root(tmp_path):
|
||||
"""A task row in NFC form resolves on macOS, where git reports NFD."""
|
||||
repo = tmp_path / _nfd("repo-Persönlich")
|
||||
repo.mkdir()
|
||||
assert _git(repo.parent, "init", "-b", "main", str(repo)).returncode == 0
|
||||
(repo / "README.md").write_text("x\n", encoding="utf-8")
|
||||
assert _git(repo, "add", "README.md").returncode == 0
|
||||
assert _git(repo, "commit", "-m", "init").returncode == 0
|
||||
|
||||
composed = Path(_nfc(str(repo)))
|
||||
if str(composed) == str(repo):
|
||||
pytest.skip("filesystem does not round-trip the name in NFD here")
|
||||
|
||||
class _Task:
|
||||
id = "t_unicode_probe"
|
||||
workspace_kind = "worktree"
|
||||
workspace_path = str(composed)
|
||||
branch_name = None
|
||||
|
||||
resolved, _branch = kbw._resolve_worktree_workspace(_Task())
|
||||
|
||||
expected = repo / ".worktrees" / "t_unicode_probe"
|
||||
assert expected.is_dir()
|
||||
assert _nfc(str(Path(resolved).resolve(strict=False))) == _nfc(
|
||||
str(expected.resolve(strict=False))
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.macos_only
|
||||
def test_repo_root_treated_as_repo_root_in_other_unicode_form(tmp_path):
|
||||
"""A root passed in NFC form must take the anchored-worktree path, not fail."""
|
||||
repo = tmp_path / _nfd("root-Persönlich")
|
||||
repo.mkdir()
|
||||
assert _git(repo.parent, "init", "-b", "main", str(repo)).returncode == 0
|
||||
(repo / "README.md").write_text("x\n", encoding="utf-8")
|
||||
assert _git(repo, "add", "README.md").returncode == 0
|
||||
assert _git(repo, "commit", "-m", "init").returncode == 0
|
||||
|
||||
composed_root = _nfc(str(repo))
|
||||
if composed_root == str(repo):
|
||||
pytest.skip("filesystem does not round-trip the name in NFD here")
|
||||
|
||||
assert kbw._path_key(kbw._git_toplevel(Path(composed_root))) == kbw._path_key(repo)
|
||||
Reference in New Issue
Block a user