From b675fb2b3e19117889c337cc2ab12e238c604e77 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:11:00 +0530 Subject: [PATCH] docs: correct os.replace claim and complete the hand-rolled site list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 review findings: (1) _commit_staged_replacements' docstring cited os.replace while the code uses os.rename — the atomicity claim holds (same- filesystem rename is atomic on POSIX and NTFS) but named the wrong function. (2) venv_bin_dir's remaining hand-rolled site list missed agent/lsp/servers.py:270. --- hermes_cli/update_cmd.py | 7 ++++--- hermes_constants.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index c1331475d6a9a..1405283381960 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -665,9 +665,10 @@ def _commit_staged_replacements(staged) -> None: This covers plain files as well as directories: the repo root holds 20 first-party modules (``run_agent.py``, ``cli.py``, ``hermes_constants.py`` …), so a files-only failure reproduces exactly the bug class we are - closing. ``os.replace`` is atomic on POSIX and maps to - ``MoveFileEx(REPLACE_EXISTING)`` on Windows, so a file swap can never - leave a half-written module the way ``copy2`` onto a live path can. + closing. Every swap is an ``os.rename`` onto a path that was just moved + aside — a same-filesystem rename is atomic on POSIX and NTFS alike, so a + file swap can never leave a half-written module the way ``copy2`` onto a + live path can. Splitting stage-all-then-swap-all shrinks the failure window from "the duration of a full tree copy" to "the duration of N renames", and makes diff --git a/hermes_constants.py b/hermes_constants.py index c0718c69b51a4..ac93c8da351a0 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -1261,8 +1261,8 @@ def venv_bin_dir(venv_dir, *, windows: bool | None = None) -> Path: each new call site had to re-derive it, and #76091 shipped an eighth copy because the correct behaviour lived 2400 lines away in another function. A few sites outside ``hermes_cli`` (``tools/code_execution_tool.py``, - ``agent/lsp/install.py``) still hand-roll it — convert them as they are - touched. + ``agent/lsp/install.py``, ``agent/lsp/servers.py``) still hand-roll it — + convert them as they are touched. *windows* lets a caller pass its own platform verdict. Several callers resolve this through predicates the test-suite patches to exercise