Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions flocks/cli/service_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,8 @@ def get_node_major_version() -> int | None:
check=False,
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
)
except OSError:
return None
Expand Down Expand Up @@ -528,6 +530,8 @@ def _windows_tasklist_process_name(pid: int) -> str | None:
check=False,
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
)
if completed.returncode != 0:
return None
Expand Down Expand Up @@ -946,6 +950,8 @@ def _process_list_pids() -> list[int]:
check=False,
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
)
else:
completed = subprocess.run(
Expand Down Expand Up @@ -2166,6 +2172,8 @@ def _run_windows_netstat(port: int) -> str:
check=False,
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
)
if completed.returncode != 0:
return ""
Expand Down
21 changes: 21 additions & 0 deletions flocks/contracts/webui/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,8 @@ def _iter_page_dirs(self, root: Path) -> list[tuple[Path, str]]:
page_dir = manifest_path.parent
if page_dir == root:
continue
if self._is_hidden_relative(page_dir, root):
continue
page_id = self._manifest_page_id_at(manifest_path)
if page_id is None:
continue
Expand All @@ -701,6 +703,8 @@ def _iter_workspace_dirs(self, root: Path) -> list[tuple[Path, WebUIWorkspaceMan
workspace_dir = manifest_path.parent
if workspace_dir == root:
continue
if self._is_hidden_relative(workspace_dir, root):
continue
manifest = self._read_workspace_manifest_at(workspace_dir)
if manifest is None:
continue
Expand Down Expand Up @@ -764,6 +768,23 @@ def _page_dir_in_root(self, root: Path, page_id: str) -> Path:
self._assert_inside_root(page_path, root)
return page_path

@staticmethod
def _is_hidden_relative(path: Path, root: Path) -> bool:
"""True when *path* lives under a dot-prefixed dir relative to *root*.

The Hub installer stages WebUI packages into sibling scratch dirs
named ``.<plugin>.<rand>`` / ``.<plugin>.bak`` inside this very root
before the atomic swap. On Windows that swap can fail (WinError 5)
while the page watcher holds the tree open, leaving those dot-dirs
behind. Scans must skip them so half-written or stale copies never
surface as real pages (and duplicate the live ones).
"""
try:
rel = path.relative_to(root)
except ValueError:
return False
return any(part.startswith(".") for part in rel.parts)

@staticmethod
def _assert_inside_root(path: Path, root: Path) -> None:
try:
Expand Down
8 changes: 8 additions & 0 deletions flocks/contracts/webui/watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ def _classify_event(
return None
if not rel.parts:
return None
# Ignore events under dot-prefixed dirs. The Hub installer stages
# WebUI packages into sibling scratch dirs (``.<plugin>.<rand>`` /
# ``.<plugin>.bak``) inside this watched root before its atomic
# swap. Reacting to those writes would race the installer's own
# build — on Windows the watcher's file handles block the swap
# with a WinError 5 access-denied — and surface half-built pages.
if any(part.startswith(".") for part in rel.parts):
return None

if rel.name == WORKSPACE_MANIFEST_FILE:
workspace_id = self._store.workspace_id_for_path(src)
Expand Down
65 changes: 54 additions & 11 deletions flocks/hub/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

import asyncio
import shutil
import sys
import tempfile
import time
from pathlib import Path
from typing import Awaitable, Callable

Expand Down Expand Up @@ -93,18 +95,11 @@ def _resolve_install_destination(
def _copy_package(src: Path, dst: Path) -> None:
parent = dst.parent
parent.mkdir(parents=True, exist_ok=True)
_purge_stale_scratch(parent, dst.name)
tmp = Path(tempfile.mkdtemp(prefix=f".{dst.name}.", dir=str(parent)))
try:
_copy_package_contents(src, tmp)
backup = None
if dst.exists():
backup = parent / f".{dst.name}.bak"
if backup.exists():
shutil.rmtree(backup)
dst.replace(backup)
tmp.replace(dst)
if backup and backup.exists():
shutil.rmtree(backup)
_replace_dir(tmp, dst)
except Exception:
if tmp.exists():
shutil.rmtree(tmp, ignore_errors=True)
Expand All @@ -122,15 +117,62 @@ def _copy_package_contents(src: Path, dst: Path) -> None:
shutil.copy2(item, target)


def _purge_stale_scratch(parent: Path, name: str) -> None:
"""Remove leftover ``.<name>.<rand>`` / ``.<name>.bak`` staging dirs.

A failed atomic swap (see :func:`_replace_dir`) can leave scratch and
backup dirs behind next to *parent*/*name*. They are never valid
installs, but on Windows a lingering ``.<name>.bak`` blocks the next
swap, so we clear both before staging a fresh copy.
"""
if not parent.is_dir():
return
for entry in parent.iterdir():
stale = entry.name.startswith(f".{name}.") or entry.name == f".{name}.bak"
if not stale:
continue
try:
if entry.is_dir() and not entry.is_symlink():
shutil.rmtree(entry, ignore_errors=True)
else:
entry.unlink()
except OSError:
pass


def _replace_with_retry(src: Path, dst: Path) -> None:
"""``src.replace(dst)`` with a Windows access-denied backoff.

On Windows an antivirus scan or a directory watcher (e.g. the WebUI
page watcher over ``~/.flocks/plugins/contracts/webui``) can hold a
transient handle on the freshly written tree, making the atomic swap
fail with ``PermissionError`` (WinError 5 / 32). Elsewhere the rename
is atomic and never needs retrying.
"""
if sys.platform != "win32":
src.replace(dst)
return
delay = 0.1
for attempt in range(6):
try:
src.replace(dst)
return
except PermissionError:
if attempt == 5:
raise
time.sleep(delay)
delay = min(delay * 2, 1.0)


def _replace_dir(src: Path, dst: Path) -> None:
parent = dst.parent
backup = None
if dst.exists():
backup = parent / f".{dst.name}.bak"
if backup.exists():
shutil.rmtree(backup)
dst.replace(backup)
src.replace(dst)
_replace_with_retry(dst, backup)
_replace_with_retry(src, dst)
if backup and backup.exists():
shutil.rmtree(backup)

Expand Down Expand Up @@ -185,6 +227,7 @@ def _build_webui_pages(plugin_id: str, install_dir: Path) -> None:
def _copy_webui_package_with_build(plugin_id: str, src: Path, dst: Path) -> None:
parent = dst.parent
parent.mkdir(parents=True, exist_ok=True)
_purge_stale_scratch(parent, dst.name)
tmp = Path(tempfile.mkdtemp(prefix=f".{dst.name}.", dir=str(parent)))
try:
_copy_package_contents(src, tmp)
Expand Down
81 changes: 81 additions & 0 deletions tests/cli/test_service_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,87 @@ def test_run_windows_netstat_handles_missing_stdout(monkeypatch) -> None:
assert service_manager._run_windows_netstat(5173) == ""


def test_run_windows_netstat_decodes_with_replacement(monkeypatch) -> None:
"""netstat output is decoded as utf-8/replace so GBK console output on
Chinese Windows never raises UnicodeDecodeError in the reader thread."""
seen: dict[str, object] = {}

def fake_run(*_args, **kwargs):
seen.update(kwargs)
return SimpleNamespace(returncode=0, stdout=" TCP 127.0.0.1:5173 0.0.0.0:0 LISTENING 42\n")

monkeypatch.setattr(service_manager.subprocess, "run", fake_run)

assert "42" in service_manager._run_windows_netstat(5173)
assert seen.get("encoding") == "utf-8"
assert seen.get("errors") == "replace"


def test_windows_tasklist_process_name_decodes_with_replacement(monkeypatch) -> None:
monkeypatch.setattr(service_manager.sys, "platform", "win32")
seen: dict[str, object] = {}

def fake_run(*_args, **kwargs):
seen.update(kwargs)
return SimpleNamespace(returncode=0, stdout='"python.exe","9436"\n')

monkeypatch.setattr(service_manager.subprocess, "run", fake_run)

assert service_manager._windows_tasklist_process_name(9436) == "python.exe"
assert seen.get("encoding") == "utf-8"
assert seen.get("errors") == "replace"


def test_process_list_pids_windows_decodes_with_replacement(monkeypatch) -> None:
monkeypatch.setattr(service_manager.sys, "platform", "win32")
seen: dict[str, object] = {}

def fake_run(*_args, **kwargs):
seen.update(kwargs)
return SimpleNamespace(returncode=0, stdout="9436\n36056\n")

monkeypatch.setattr(service_manager.subprocess, "run", fake_run)

assert service_manager._process_list_pids() == [9436, 36056]
assert seen.get("encoding") == "utf-8"
assert seen.get("errors") == "replace"


def test_get_node_major_version_decodes_with_replacement(monkeypatch) -> None:
monkeypatch.setattr(service_manager, "resolve_node_executable", lambda: "node")
seen: dict[str, object] = {}

def fake_run(*_args, **kwargs):
seen.update(kwargs)
return SimpleNamespace(returncode=0, stdout="v20.11.1\n")

monkeypatch.setattr(service_manager.subprocess, "run", fake_run)

assert service_manager.get_node_major_version() == 20
assert seen.get("encoding") == "utf-8"
assert seen.get("errors") == "replace"


def test_windows_process_probes_survive_gbk_bytes(monkeypatch) -> None:
"""End-to-end: real GBK-encoded bytes flowing through the decode path
must not raise UnicodeDecodeError (the restart-time reader-thread crash)."""
monkeypatch.setattr(service_manager.sys, "platform", "win32")
# 0xbb is the byte that crashed utf-8 decode in the original bug report.
gbk_stdout = "映像名称: python.exe 拒绝访问".encode("gbk").decode("utf-8", errors="replace")

def fake_run(*_args, **kwargs):
assert kwargs.get("encoding") == "utf-8"
assert kwargs.get("errors") == "replace"
return SimpleNamespace(returncode=0, stdout=gbk_stdout)

monkeypatch.setattr(service_manager.subprocess, "run", fake_run)

# None of these should raise, even with mojibake stdout.
service_manager._windows_tasklist_process_name(123)
service_manager._run_windows_netstat(5173)
service_manager._process_list_pids()


def test_port_owner_pids_warns_when_no_tool_found(monkeypatch) -> None:
monkeypatch.setattr(service_manager.sys, "platform", "linux")
monkeypatch.setattr(service_manager, "which", lambda _name: None)
Expand Down
49 changes: 49 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""

import os
from pathlib import Path

import pytest

Expand Down Expand Up @@ -37,3 +38,51 @@ def pytest_runtest_setup(item):
for marker_name, (env_var, reason) in _API_KEY_MARKERS.items():
if item.get_closest_marker(marker_name) and not os.getenv(env_var):
pytest.skip(reason)


@pytest.fixture(autouse=True)
def _home_honors_env(monkeypatch):
"""Make ``Path.home()`` follow the ``HOME`` env var on every platform.

Many suites isolate their filesystem by ``monkeypatch.setenv("HOME", ...)``
and then rely on production code that resolves ``Path.home() / ".flocks"``
(hub installs, plugin roots, the WebUI contract store, ...). That works on
Linux CI, where ``Path.home()`` honors ``$HOME`` — but on Windows
``Path.home()`` reads ``%USERPROFILE%`` and ignores ``HOME`` entirely, so
those tests silently leak into (and assert against) the *real* user home.

This autouse fixture patches ``pathlib.Path.home`` to honor ``HOME`` (then
``USERPROFILE``) at call time, aligning Windows with Linux CI. It is
behavior-preserving when ``HOME`` already points at the real home (the
default), so tests that never touch ``HOME`` see no change.
"""
def _home() -> Path:
candidate = os.environ.get("HOME") or os.environ.get("USERPROFILE")
if candidate:
return Path(candidate)
return Path(os.path.expanduser("~"))

monkeypatch.setattr(Path, "home", staticmethod(_home))

# ``Path.home()`` covers explicit home lookups, but ``~``/``~user`` tilde
# expansion goes through ``os.path.expanduser`` (and ``Path.expanduser``),
# which on Windows keys off ``%USERPROFILE%``/``%HOMEDRIVE%%HOMEPATH%`` and
# likewise ignores ``HOME``. Route a bare ``~`` / ``~/...`` through the same
# ``HOME``-aware root so tilde-based tests are hermetic too. Anything else
# (``~otheruser``) falls back to the real implementation untouched.
_real_expanduser = os.path.expanduser

def _expanduser(path):
text = os.fspath(path)
# Only rewrite plain ``str`` tilde paths; leave bytes and ``~user``
# forms to the real implementation so we never change its type
# contract or raise on a bytes path.
if isinstance(text, str):
home = os.environ.get("HOME") or os.environ.get("USERPROFILE")
if home and (text == "~" or text.startswith("~/") or text.startswith("~\\")):
return home + text[1:]
return _real_expanduser(path)

monkeypatch.setattr(os.path, "expanduser", _expanduser)


34 changes: 34 additions & 0 deletions tests/contracts/webui/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,40 @@ def test_list_pages_scans_user_and_project_roots_with_user_priority(tmp_path):
assert store.page_dir("project-page").is_relative_to(project_root)


def test_list_pages_skips_installer_scratch_dirs(tmp_path):
"""Pages that exist only under the Hub installer's ``.<name>.<rand>`` /
``.<name>.bak`` scratch dirs must never surface as real pages.

Reproduces the Windows WinError 5 aftermath where a failed atomic swap
left the SOC pages' manifests stuck inside scratch dirs while the real
install was incomplete.
"""
user_root = tmp_path / "user" / "contracts" / "webui"
_write_page(user_root, "real-page", "Real Page")
# Manifests stranded in leftover scratch/backup dirs (no real install).
_write_page_at(user_root, ".soc_ui.abc123/soc_overview", "soc-overview", "Stranded Overview")
_write_page_at(user_root, ".soc_ui.bak/soc_dashboard", "soc-dashboard", "Stranded Dashboard")

store = WebUIPagesStore(root=user_root, project_root=None, legacy_root=None)

pages = store.list_pages()
assert [page.id for page in pages] == ["real-page"]
assert pages[0].title == "Real Page"


def test_list_workspaces_skips_installer_scratch_dirs(tmp_path):
user_root = tmp_path / "user" / "contracts" / "webui"
_write_workspace(user_root, "real_ws", "Real Workspace")
# A workspace manifest stranded under a scratch dir must be ignored.
_write_workspace(user_root / ".soc_ui.abc123", "soc_ui", "Stranded Workspace")

store = WebUIPagesStore(root=user_root, project_root=None, legacy_root=None)

workspaces = store.list_workspaces()
assert [workspace.id for workspace in workspaces] == ["real_ws"]
assert workspaces[0].title == "Real Workspace"


def test_grouped_page_directory_uses_manifest_id_for_lookup(tmp_path):
user_root = tmp_path / "user" / "contracts" / "webui"
_write_workspace(user_root, "scene_workspace", "场景工作区")
Expand Down
Loading