From 53aec0e28934d5fbb07cfbfd0f3de7f5cb5fdd5a Mon Sep 17 00:00:00 2001 From: KoNit-K Date: Fri, 11 Sep 2026 20:43:50 +0800 Subject: [PATCH 01/13] fix(desktop): persist project cwd for new sessions --- cron/scheduler.py | 19 ++- cron/scheduler_detached_worker.py | 5 +- tests/cron/test_cleanup_timeout.py | 3 +- tests/cron/test_cron_session_workdir.py | 84 +++++++++++++ ...t_desktop_session_workspace_persistence.py | 111 ++++++++++++++++++ tui_gateway/methods_session.py | 21 +++- 6 files changed, 233 insertions(+), 10 deletions(-) create mode 100644 tests/cron/test_cron_session_workdir.py create mode 100644 tests/tui_gateway/test_desktop_session_workspace_persistence.py diff --git a/cron/scheduler.py b/cron/scheduler.py index 3268a250f2..592dca42eb 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -2159,7 +2159,8 @@ def _final_response_from_result(result: dict, job_id: str, job_name: str, AIAgen return final_response -def _finalize_cron_session(session_db, agent, job_id: str, job_name: str, cron_session_id: str) -> None: +def _finalize_cron_session(session_db, agent, job_id: str, job_name: str, cron_session_id: str, + workdir: Optional[str] = None) -> None: """Title, classify, end and release the cron session after the agent turn has returned.""" # Bound every DB op so storage failure cannot hold the dispatch guard. _session_db = _BoundedCronSessionDB(session_db, job_id) @@ -2229,6 +2230,16 @@ def _finalize_cron_session(session_db, agent, job_id: str, job_name: str, cron_s job_id, _lifecycle, _end_reason) except (Exception, KeyboardInterrupt) as e: logger.debug("Job '%s': session lifecycle classification failed: %s", job_id, e) + # Stamp the job's workdir on the row BEFORE end_session (same before-close ordering as the + # title write, #50536): the sidebar groups sessions by cwd prefix, and a NULL cwd filed every + # cron run under __no_project__ even when the job ran inside a repo (#108205). The workdir is + # the session's authoritative cwd — _launch_cwd_for_session records none for cron source, so + # nothing else writes one. Best-effort like every finalize step. + if workdir: + try: + _session_db.update_session_cwd(_final_cron_session_id, workdir) + except (Exception, KeyboardInterrupt) as e: + logger.debug("Job '%s': failed to stamp workdir on session row: %s", job_id, e) try: _session_db.end_session(_final_cron_session_id, _end_reason) # The scheduler owns cron-session finalization. AIAgent.close() also @@ -2682,10 +2693,12 @@ def run_job( finally: from cron.scheduler_detached_worker import defer_teardown_to_running_worker _worker_teardown_deferred = defer_teardown_to_running_worker( - _worker_state.get("future"), _session_db, agent, job_id, job_name, _cron_session_id) + _worker_state.get("future"), _session_db, agent, job_id, job_name, _cron_session_id, + workdir=scope.workdir) scope.exit() if _session_db and not _worker_teardown_deferred: - _finalize_cron_session(_session_db, agent, job_id, job_name, _cron_session_id) + _finalize_cron_session(_session_db, agent, job_id, job_name, _cron_session_id, + workdir=scope.workdir) # Tear down the ephemeral agent or the gateway leaks fds per tick (EMFILE). With deferred # teardown, hand the live agent back: delivery needs a live async client. # Release subprocesses, terminal sandboxes, browser daemons, and the main OpenAI/httpx client held diff --git a/cron/scheduler_detached_worker.py b/cron/scheduler_detached_worker.py index 39cdf72328..db403b4f6c 100644 --- a/cron/scheduler_detached_worker.py +++ b/cron/scheduler_detached_worker.py @@ -16,7 +16,7 @@ from typing import Optional def defer_teardown_to_running_worker( future: Optional[concurrent.futures.Future], session_db, agent, job_id: str, job_name: str, - cron_session_id: str, + cron_session_id: str, workdir: Optional[str] = None, ) -> bool: """Return True when the worker is still running and its Future will finalize the session and tear the agent down on completion; False when the caller must do it now.""" @@ -27,7 +27,8 @@ def defer_teardown_to_running_worker( def _finish(_future) -> None: try: if session_db: - _finalize_cron_session(session_db, agent, job_id, job_name, cron_session_id) + _finalize_cron_session(session_db, agent, job_id, job_name, cron_session_id, + workdir=workdir) finally: _teardown_cron_agent(agent, job_id) diff --git a/tests/cron/test_cleanup_timeout.py b/tests/cron/test_cleanup_timeout.py index 9fcc97b43c..0cd2c92ed1 100644 --- a/tests/cron/test_cleanup_timeout.py +++ b/tests/cron/test_cleanup_timeout.py @@ -111,7 +111,8 @@ def test_detached_worker_teardown_waits_for_future(): future.set_result({"final_response": "late"}) - finalize.assert_called_once_with(fake_db, agent, "detached-worker", "detached worker", "cron_detached-worker") + finalize.assert_called_once_with(fake_db, agent, "detached-worker", "detached worker", + "cron_detached-worker", workdir=None) teardown_agent.assert_called_once_with(agent, "detached-worker") assert defer_teardown_to_running_worker( future, fake_db, agent, "detached-worker", "detached worker", "cron_detached-worker") is False diff --git a/tests/cron/test_cron_session_workdir.py b/tests/cron/test_cron_session_workdir.py new file mode 100644 index 0000000000..8e17897756 --- /dev/null +++ b/tests/cron/test_cron_session_workdir.py @@ -0,0 +1,84 @@ +"""Cron sessions stamp the job's workdir on their session row (#108205). + +The sidebar groups sessions by cwd prefix; a cron run's row was left NULL even when the +job ran inside a repo workdir (``_launch_cwd_for_session`` records no cwd for cron +source), so every cron session filed under __no_project__. The scheduler — the owner of +cron-session finalization — stamps the job's workdir before closing the session, with the +same before-end_session ordering as the title write (#50536). +""" + +from __future__ import annotations + +from unittest.mock import patch + +from cron.scheduler import run_job +from hermes_state import SessionDB + +_RUNTIME = { + "api_key": "test-key", + "base_url": "https://example.invalid/v1", + "provider": "openrouter", + "api_mode": "chat_completions", +} + + +class _FakeCronAgent: + """Stand-in for the cron AIAgent: creates its session row on the first turn exactly + like AIAgent's lazy create (source 'cron', no cwd — _launch_cwd_for_session records + none for cron source).""" + + def __init__(self, *args, session_id=None, session_db=None, **kwargs): + self.session_id = session_id + self.session_db = session_db + + def run_conversation(self, user_message, conversation_history=None, task_id=None): + if self.session_db is not None: + self.session_db.create_session(self.session_id, source="cron") + return {"final_response": "ok"} + + +def _run_job_with_real_db(job, db, tmp_path): + with patch("cron.scheduler._hermes_home", tmp_path), \ + patch("cron.scheduler_delivery._resolve_origin", return_value=None), \ + patch("hermes_cli.env_loader.load_hermes_dotenv"), \ + patch("hermes_cli.env_loader.reset_secret_source_cache"), \ + patch("hermes_state_registry.acquire", return_value=db), \ + patch("hermes_cli.runtime_provider.resolve_runtime_provider", return_value=_RUNTIME), \ + patch("run_agent.AIAgent", _FakeCronAgent): + return run_job(job) + + +def _cron_rows(db): + return [dict(r) for r in db._read_all("SELECT * FROM sessions WHERE id LIKE 'cron_%'")] + + +def test_run_job_stamps_workdir_on_cron_session_row(tmp_path): + db = SessionDB(db_path=tmp_path / "state.db") + repo = tmp_path / "repo" + repo.mkdir() + job = {"id": "workdir-job", "name": "workdir job", "prompt": "hello", "workdir": str(repo)} + + try: + success, _output, _final, error = _run_job_with_real_db(job, db, tmp_path) + + assert success is True, error + rows = _cron_rows(db) + assert len(rows) == 1 + assert rows[0]["cwd"] == str(repo) + finally: + db.close() + + +def test_run_job_without_workdir_leaves_cron_session_cwd_null(tmp_path): + db = SessionDB(db_path=tmp_path / "state.db") + job = {"id": "no-workdir-job", "name": "no workdir", "prompt": "hello"} + + try: + success, _output, _final, error = _run_job_with_real_db(job, db, tmp_path) + + assert success is True, error + rows = _cron_rows(db) + assert len(rows) == 1 + assert rows[0]["cwd"] is None + finally: + db.close() diff --git a/tests/tui_gateway/test_desktop_session_workspace_persistence.py b/tests/tui_gateway/test_desktop_session_workspace_persistence.py new file mode 100644 index 0000000000..5c2e16f7f6 --- /dev/null +++ b/tests/tui_gateway/test_desktop_session_workspace_persistence.py @@ -0,0 +1,111 @@ +"""Desktop-provided project workspaces remain durable session metadata (#108205). + +A desktop client names a workspace its gateway host cannot ``isdir``-probe (Docker/remote +backend topology). The failed probe is a topology artifact, not a verdict on the path, so a +nonblank desktop-sourced cwd with the client's own provenance (``cwd_explicit``, #52589) +persists to the session row; an inherited workspace only persists when the cwd resolution +actually adopted it, and a launch-dir fallback still persists nothing. +""" + +import os + +from hermes_state import SessionDB +from tui_gateway import server + + +def _gateway_with_db(monkeypatch, tmp_path): + db = SessionDB(db_path=tmp_path / "state.db") + monkeypatch.setattr(server, "_get_db", lambda: db) + monkeypatch.setattr(server, "_schedule_agent_build", lambda _sid: None) + monkeypatch.setattr(server, "_schedule_session_cap_enforcement", lambda: None) + monkeypatch.setattr(server, "_register_session_cwd", lambda _session: None) + return db + + +def _create_desktop_session(params): + response = server.handle_request( + {"id": "create", "method": "session.create", + "params": {"source": "desktop", **params}} + ) + assert "result" in response, response + return response["result"] + + +def test_desktop_explicit_cwd_persists_when_local_probe_cannot_see_it(monkeypatch, tmp_path): + db = _gateway_with_db(monkeypatch, tmp_path) + workspace = tmp_path / "desktop-only-workspace" + assert not workspace.exists() + + sid = None + try: + result = _create_desktop_session({"cwd": str(workspace), "cwd_explicit": True}) + sid, stored_id = result["session_id"], result["stored_session_id"] + + session = server._sessions[sid] + assert session["cwd"] == str(workspace) + assert session["explicit_cwd"] is True + + assert server._persist_session_row_for_submit("rid", session) is None + assert db.get_session(stored_id)["cwd"] == str(workspace) + finally: + if sid: + server._sessions.pop(sid, None) + db.close() + + +def test_desktop_inherited_cwd_persists_only_when_the_resolution_adopted_it( + monkeypatch, tmp_path +): + """An inherited (cwd_explicit=false) workspace under a container backend: the cwd + resolution adopts the client path (no host isdir on a container path), so it is the + session's workspace and persists — while still yielding to a named profile's + configured terminal.cwd inside _completion_cwd (#52589).""" + db = _gateway_with_db(monkeypatch, tmp_path) + container_cwd = "/workspace/repo" + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setattr(server, "_launch_configured_cwd", lambda: None) + monkeypatch.delenv("TERMINAL_CWD", raising=False) + + sid = None + try: + result = _create_desktop_session({"cwd": container_cwd}) + sid, stored_id = result["session_id"], result["stored_session_id"] + + session = server._sessions[sid] + assert session["cwd"] == container_cwd + assert session["explicit_cwd"] is True + + assert server._persist_session_row_for_submit("rid", session) is None + assert db.get_session(stored_id)["cwd"] == container_cwd + finally: + if sid: + server._sessions.pop(sid, None) + db.close() + + +def test_desktop_launch_fallback_cwd_still_persists_nothing(monkeypatch, tmp_path): + """The inherited cwd resolves to the gateway's launch directory (os.getcwd) — an + artifact of how the app started, not a picked workspace: the row stays NULL and the + sidebar keeps filing it under "No workspace" (the #98924 rule).""" + db = _gateway_with_db(monkeypatch, tmp_path) + launch_dir = tmp_path / "gateway-launch-dir" + launch_dir.mkdir() + monkeypatch.chdir(launch_dir) + monkeypatch.delenv("TERMINAL_CWD", raising=False) + assert os.path.realpath(server._completion_cwd({})) == os.path.realpath(launch_dir) + + sid = None + try: + result = _create_desktop_session({"cwd": "relative-inherited-notebook"}) + sid, stored_id = result["session_id"], result["stored_session_id"] + + session = server._sessions[sid] + assert session["explicit_cwd"] is False + + assert server._persist_session_row_for_submit("rid", session) is None + assert db.get_session(stored_id)["cwd"] is None + finally: + if sid: + server._sessions.pop(sid, None) + db.close() diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index fd2730fbf1..e7abcbcb90 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -400,14 +400,27 @@ def _create_session(rid, params: dict, *, copy_parent_history: bool = False) -> return _err(rid, 4008, f"nothing to branch — {exc}") history = _visible_branch_history(display_history) if not history: - return _err(rid, 4008, "nothing to branch — send a message first") - # Only an explicitly chosen existing workspace persists as cwd; the launch-dir fallback is "No workspace". - explicit_cwd = False + return _err(rid, 4008, "send a message first") + # Only a chosen workspace persists as cwd; the launch-dir fallback is "No workspace". raw_cwd = _str_param(params, "cwd") # unguarded, as on BASE: only the path check is best-effort # An ssh profile's cwd lives on the remote host, where the host isdir check cannot vouch for it. remote_cwd = bool(raw_cwd) and _is_remote_cwd_shape(raw_cwd) and _cwd_is_remote(profile_home) + explicit_cwd = False with contextlib.suppress(Exception): explicit_cwd = bool(raw_cwd) and (remote_cwd or os.path.isdir(os.path.abspath(os.path.expanduser(raw_cwd)))) + session_cwd = _completion_cwd(params) + if raw_cwd and not explicit_cwd and source == "desktop": + # The desktop client names a workspace its gateway host cannot probe (Docker/remote backend + # topology — #108205): a failed host-side isdir is a topology artifact, not a verdict on the + # client's path. The CLIENT vouches instead, with #52589 provenance: a deliberate pick + # (cwd_explicit) adopts the raw path outright; an inherited app-global workspace only marks + # the workspace explicit when the resolution above actually adopted it, so a launch-dir + # fallback still persists nothing and a named profile's terminal.cwd keeps winning. + if _flag(params, "cwd_explicit"): + explicit_cwd = True + session_cwd = raw_cwd + elif session_cwd and session_cwd == os.path.abspath(os.path.expanduser(raw_cwd)): + explicit_cwd = True _enable_gateway_prompts() from .methods_session_model_guard import create_overrides try: @@ -431,7 +444,7 @@ def _create_session(rid, params: dict, *, copy_parent_history: bool = False) -> "explicit_cwd": explicit_cwd, "history": history, "history_lock": threading.Lock(), "history_version": 0, "image_counter": 0, "seeded": bool(history), # gates _persist_branch_seed: only create-time history is unpersisted - "cwd": _completion_cwd(params), "inflight_turn": None, "last_active": now, + "cwd": session_cwd, "inflight_turn": None, "last_active": now, "model_override": session_model_override, "composer_override_profile": composer_override_profile, "create_reasoning_override": create_reasoning_override, From 59a3866ea5a07290afd9a1d137d52d679b77f3ab Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 6 Oct 2026 14:43:35 -0500 Subject: [PATCH 02/13] fix(desktop): persist project cwd for new sessions (WIP continuation) --- cron/scheduler.py | 25 ++++--------------------- cron/scheduler_detached_worker.py | 16 ++++++++++++++++ tui_gateway/methods_session.py | 23 +++-------------------- tui_gateway/session_workdir.py | 29 +++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 41 deletions(-) diff --git a/cron/scheduler.py b/cron/scheduler.py index 592dca42eb..db29cde441 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -55,21 +55,6 @@ from agent.turn_failure_copy import is_max_iteration_handoff logger = logging.getLogger(__name__) -def _close_late_session_db_result(future: "concurrent.futures.Future") -> None: - """Done-callback: close a SessionDB whose constructor finished after run_job's init timeout - (worker abandoned via ``shutdown(wait=False)``), else its .db/WAL/SHM handles leak to EMFILE. - - If the constructor later completes inside that abandoned worker, the Future's result — an open SessionDB - holding .db / WAL / SHM file handles — would be orphaned and never closed, leaking descriptors until - EMFILE (#72782). This callback retrieves and closes that eventual late result. - """ - with contextlib.suppress(Exception): - db = future.result() - if db is not None: - from hermes_state_registry import release_or_close - release_or_close(db) - - def _set_cron_session_title(session_db, session_id, base_title): """Persist a non-blank, unique title for a finished cron session; returns it (None if unset). Runs BEFORE end_session()/close() so no write races the close. Duplicate title (unique-index @@ -1949,6 +1934,7 @@ def _open_cron_session_db(job: dict): # inside it, the future's result would be orphaned and its SQLite FDs (.db, WAL, SHM) leak until # process exit. Register a done-callback that retrieves and closes any eventual late result # (#72782). + from cron.scheduler_detached_worker import _close_late_session_db_result _session_db_future.add_done_callback(_close_late_session_db_result) raise finally: @@ -2230,16 +2216,13 @@ def _finalize_cron_session(session_db, agent, job_id: str, job_name: str, cron_s job_id, _lifecycle, _end_reason) except (Exception, KeyboardInterrupt) as e: logger.debug("Job '%s': session lifecycle classification failed: %s", job_id, e) - # Stamp the job's workdir on the row BEFORE end_session (same before-close ordering as the - # title write, #50536): the sidebar groups sessions by cwd prefix, and a NULL cwd filed every - # cron run under __no_project__ even when the job ran inside a repo (#108205). The workdir is - # the session's authoritative cwd — _launch_cwd_for_session records none for cron source, so - # nothing else writes one. Best-effort like every finalize step. + # Stamp the job's workdir on the row BEFORE end_session (title-write ordering, #50536): the + # sidebar groups by cwd prefix and nothing else writes a cron row's cwd (#108205). if workdir: try: _session_db.update_session_cwd(_final_cron_session_id, workdir) except (Exception, KeyboardInterrupt) as e: - logger.debug("Job '%s': failed to stamp workdir on session row: %s", job_id, e) + logger.debug("Job '%s': failed to stamp workdir on session row: %s", job_id, e, exc_info=True) try: _session_db.end_session(_final_cron_session_id, _end_reason) # The scheduler owns cron-session finalization. AIAgent.close() also diff --git a/cron/scheduler_detached_worker.py b/cron/scheduler_detached_worker.py index db403b4f6c..66cbbccf47 100644 --- a/cron/scheduler_detached_worker.py +++ b/cron/scheduler_detached_worker.py @@ -9,11 +9,27 @@ overlap behind #102827. The worker's Future owns the teardown instead. from __future__ import annotations import concurrent.futures +import contextlib import subprocess import threading from typing import Optional +def _close_late_session_db_result(future: "concurrent.futures.Future") -> None: + """Done-callback: close a SessionDB whose constructor finished after run_job's init timeout + (worker abandoned via ``shutdown(wait=False)``), else its .db/WAL/SHM handles leak to EMFILE. + + If the constructor later completes inside that abandoned worker, the Future's result — an open + SessionDB holding .db / WAL / SHM file handles — would be orphaned and never closed, leaking + descriptors until EMFILE (#72782). This callback retrieves and closes that eventual late result. + """ + with contextlib.suppress(Exception): + db = future.result() + if db is not None: + from hermes_state_registry import release_or_close + release_or_close(db) + + def defer_teardown_to_running_worker( future: Optional[concurrent.futures.Future], session_db, agent, job_id: str, job_name: str, cron_session_id: str, workdir: Optional[str] = None, diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index e7abcbcb90..c11f7d4575 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -401,26 +401,9 @@ def _create_session(rid, params: dict, *, copy_parent_history: bool = False) -> history = _visible_branch_history(display_history) if not history: return _err(rid, 4008, "send a message first") - # Only a chosen workspace persists as cwd; the launch-dir fallback is "No workspace". - raw_cwd = _str_param(params, "cwd") # unguarded, as on BASE: only the path check is best-effort - # An ssh profile's cwd lives on the remote host, where the host isdir check cannot vouch for it. - remote_cwd = bool(raw_cwd) and _is_remote_cwd_shape(raw_cwd) and _cwd_is_remote(profile_home) - explicit_cwd = False - with contextlib.suppress(Exception): - explicit_cwd = bool(raw_cwd) and (remote_cwd or os.path.isdir(os.path.abspath(os.path.expanduser(raw_cwd)))) - session_cwd = _completion_cwd(params) - if raw_cwd and not explicit_cwd and source == "desktop": - # The desktop client names a workspace its gateway host cannot probe (Docker/remote backend - # topology — #108205): a failed host-side isdir is a topology artifact, not a verdict on the - # client's path. The CLIENT vouches instead, with #52589 provenance: a deliberate pick - # (cwd_explicit) adopts the raw path outright; an inherited app-global workspace only marks - # the workspace explicit when the resolution above actually adopted it, so a launch-dir - # fallback still persists nothing and a named profile's terminal.cwd keeps winning. - if _flag(params, "cwd_explicit"): - explicit_cwd = True - session_cwd = raw_cwd - elif session_cwd and session_cwd == os.path.abspath(os.path.expanduser(raw_cwd)): - explicit_cwd = True + # Only a chosen workspace persists as cwd; the launch-dir fallback is "No workspace" + # (#108205: the desktop arm lets the client vouch for a host-invisible path, #52589 provenance). + explicit_cwd, session_cwd, remote_cwd = _resolve_create_cwd(params, source, profile_home) _enable_gateway_prompts() from .methods_session_model_guard import create_overrides try: diff --git a/tui_gateway/session_workdir.py b/tui_gateway/session_workdir.py index 86493462d7..586c199c2a 100644 --- a/tui_gateway/session_workdir.py +++ b/tui_gateway/session_workdir.py @@ -224,6 +224,35 @@ def _context_cwd_is_launch_artifact(session: dict | None) -> bool: return bool(session and not session.get("explicit_cwd") and _session_source(session) in _LAUNCH_CWD_NOT_A_WORKSPACE) +def _resolve_create_cwd(params: dict, source: str, profile_home) -> tuple[bool, str, bool]: + """``(explicit_cwd, session_cwd, remote_cwd)`` for a freshly created session. + + Only a chosen workspace persists as cwd; the launch-dir fallback is "No workspace". A + chosen workspace is one the gateway host can ``isdir``-probe, an ssh-shaped cwd on a remote + profile, or — the #108205 desktop arm — a nonblank desktop-sourced cwd the host probe could + not vouch for: the client names a workspace its gateway host cannot see (Docker/remote + backend topology), and a failed host-side isdir is a topology artifact, not a verdict on + the client's path. The CLIENT vouches instead, with #52589 provenance: a deliberate pick + (``cwd_explicit``) adopts the raw path outright; an inherited app-global workspace only + counts once the completion resolution actually adopted it, so a launch-dir fallback still + persists nothing and a named profile's configured ``terminal.cwd`` keeps winning. + """ + raw_cwd = str(params.get("cwd") or "").strip() + remote_cwd = bool(raw_cwd) and _is_remote_cwd_shape(raw_cwd) and _cwd_is_remote(profile_home) + explicit_cwd = False + with contextlib.suppress(Exception): + explicit_cwd = bool(raw_cwd) and ( + remote_cwd or os.path.isdir(os.path.abspath(os.path.expanduser(raw_cwd)))) + session_cwd = _completion_cwd(params) + if raw_cwd and not explicit_cwd and source == "desktop": + if params.get("cwd_explicit"): + explicit_cwd = True + session_cwd = raw_cwd + elif session_cwd and session_cwd == os.path.abspath(os.path.expanduser(raw_cwd)): + explicit_cwd = True + return explicit_cwd, session_cwd, remote_cwd + + def _persisted_session_cwd(session: dict) -> str | None: """The cwd to stamp on the session's DB row, or None to leave it unset (launch-dir rule: ``_ensure_session_db_row``).""" if session.get("explicit_cwd"): From 9b38eb1432d8363fe0f01056d03b0fc123014946 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 6 Oct 2026 13:11:02 -0700 Subject: [PATCH 03/13] test(update): a failed pause-record test no longer orphans its draining gateway The _DRAINING child traps SIGTERM and then polls for a flag file every 10 ms, and each test released it only in a finally that begins after its first assert. Any earlier failure (the "up"/"asked"/"stopping" reads, the updater spawn) skipped that block: the gateway was reparented to init and spun forever. Two such orphans from red-on-base runs were still alive on a dev host three days later; a CI red leaks the same way onto its runner. _child now records every process it spawns and an autouse fixture SIGKILLs any survivor once the test is over, so the whole file is covered, including the updater children (time.sleep(120)) and any test added later. Sabotaging the "stopping" assert: origin/main leaves 1 draining gateway alive after the failed test, this change leaves 0; the file passes 42/42. --- tests/hermes_cli/test_update_pause_record.py | 21 ++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/hermes_cli/test_update_pause_record.py b/tests/hermes_cli/test_update_pause_record.py index 86d78a6846..4bc17484d5 100644 --- a/tests/hermes_cli/test_update_pause_record.py +++ b/tests/hermes_cli/test_update_pause_record.py @@ -35,14 +35,31 @@ _lock.checkout_lock_path = checkout_lock_path """ +_SPAWNED: list[subprocess.Popen] = [] + + +@pytest.fixture(autouse=True) +def _reap_children(): + """A failed assert skips a test's own release, and a draining gateway ignores SIGTERM and + polls for its flag forever: SIGKILL every child still alive once the test is over.""" + yield + while _SPAWNED: + child = _SPAWNED.pop() + if child.poll() is None: + child.kill() # windows-footgun: ok — module skips on Windows + child.wait(timeout=10) + + def _child(code: str, *argv: str, env: dict) -> subprocess.Popen: shim = Path(env["HERMES_HOME"]) / "checkout-lock-shim" shim.mkdir(exist_ok=True) (shim / "sitecustomize.py").write_text(_PRIVATE_CHECKOUT_LOCK, encoding="utf-8") env = {**os.environ, "PYTHONPATH": os.pathsep.join((str(shim), str(REPO))), "HERMES_TEST_CHECKOUT_LOCK": str(shim / "hermes-update.lock"), **env} - return subprocess.Popen([sys.executable, "-c", textwrap.dedent(code), *argv], cwd=REPO, stdin=subprocess.DEVNULL, - stdout=subprocess.PIPE, text=True, env=env) + child = subprocess.Popen([sys.executable, "-c", textwrap.dedent(code), *argv], cwd=REPO, + stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, text=True, env=env) + _SPAWNED.append(child) + return child def _orphaned_profiles(home: Path) -> dict | None: From 53fefae5d74312a534063e1b1ef932c0cf98e1e9 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Wed, 7 Oct 2026 05:24:53 +0530 Subject: [PATCH 04/13] fix(local-runtime): hermes update from v2026.9.24 no longer crashes importing local_runtime After the pull, the v2026.9.24 updater lazily imports hermes_cli.local_runtime.processes (in _subprocess_compat.bounded_probe_run) from the new checkout. Its process still holds the old hermes_platform.host.facts. The package __init__ eagerly imported every submodule, so hardware.py ran `from hermes_platform.host.facts import gpu_class` (added in 827f4ff1d1) against the old facts module, and the update died with ImportError. This broke e2e-upgrade (core) on main and on every PR. The package-level re-exports now resolve on first access through a module __getattr__, so importing one submodule runs only that submodule. Callers of `hermes_cli.local_runtime.` are unchanged. --- hermes_cli/local_runtime/__init__.py | 49 +++++++++++++------ .../test_local_runtime_package_import.py | 30 ++++++++++++ 2 files changed, 63 insertions(+), 16 deletions(-) create mode 100644 tests/hermes_cli/test_local_runtime_package_import.py diff --git a/hermes_cli/local_runtime/__init__.py b/hermes_cli/local_runtime/__init__.py index 6aafeb1e07..6c29b9089e 100644 --- a/hermes_cli/local_runtime/__init__.py +++ b/hermes_cli/local_runtime/__init__.py @@ -6,21 +6,38 @@ engine is installed (the engine bytes live in pm's machine-wide store); download; ``supervisor`` spawns and supervises one llama-server in router mode (readiness is a touch generation, never health-200 alone); ``detect`` finds an already-running llama-server (external or ours). + +The package-level names resolve on first access. Shipped updaters import +``hermes_cli.local_runtime.processes`` from the NEW checkout after the pull, +while their process still holds the OLD ``hermes_platform`` and other +first-party modules. An eager re-export here would run every submodule +against those stale modules and crash the update on any name added since. """ -from hermes_cli.local_runtime.binaries import ( # noqa: F401 - BACKEND_PACKAGES, BinaryResolutionError, Engine, ensure_engine, - installed_engine, pinned_tag, resolve_backend, select_backend) -from hermes_cli.local_runtime.bootstrap import ensure_local_runtime, shutdown_local_runtime # noqa: F401 -from hermes_cli.local_runtime.context_policy import ( # noqa: F401 - FLOOR, growth_decision, initial_window, ladder, launch_args) -from hermes_cli.local_runtime.growth import ( # noqa: F401 - clear_window_override, load_window_overrides, maybe_grow_window, save_window_override) -from hermes_cli.local_runtime.detect import detect_server # noqa: F401 -from hermes_cli.local_runtime.endpoint import resolve_llamacpp_endpoint # noqa: F401 -from hermes_cli.local_runtime.estimator import ( # noqa: F401 - HardwareBudget, ctx_bytes, physics_check, profile_from_gguf) -from hermes_cli.local_runtime.gguf import read_gguf_header # noqa: F401 -from hermes_cli.local_runtime.hardware import probe_budget # noqa: F401 -from hermes_cli.local_runtime.presets import generate_presets # noqa: F401 -from hermes_cli.local_runtime.supervisor import LlamaServerSupervisor # noqa: F401 +from importlib import import_module + +_EXPORTS = { + "binaries": ("BACKEND_PACKAGES", "BinaryResolutionError", "Engine", "ensure_engine", + "installed_engine", "pinned_tag", "resolve_backend", "select_backend"), + "bootstrap": ("ensure_local_runtime", "shutdown_local_runtime"), + "context_policy": ("FLOOR", "growth_decision", "initial_window", "ladder", "launch_args"), + "growth": ("clear_window_override", "load_window_overrides", "maybe_grow_window", + "save_window_override"), + "detect": ("detect_server",), + "endpoint": ("resolve_llamacpp_endpoint",), + "estimator": ("HardwareBudget", "ctx_bytes", "physics_check", "profile_from_gguf"), + "gguf": ("read_gguf_header",), + "hardware": ("probe_budget",), + "presets": ("generate_presets",), + "supervisor": ("LlamaServerSupervisor",), +} +_SOURCE = {name: module for module, names in _EXPORTS.items() for name in names} + + +def __getattr__(name: str): + module = _SOURCE.get(name) + if module is None: + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + value = getattr(import_module(f"{__name__}.{module}"), name) + globals()[name] = value + return value diff --git a/tests/hermes_cli/test_local_runtime_package_import.py b/tests/hermes_cli/test_local_runtime_package_import.py new file mode 100644 index 0000000000..5313150c36 --- /dev/null +++ b/tests/hermes_cli/test_local_runtime_package_import.py @@ -0,0 +1,30 @@ +"""Shipped updaters import ``hermes_cli.local_runtime.processes`` from the new tree mid-update. + +v2026.9.24's ``_subprocess_compat.bounded_probe_run`` lazily imports +``spawn_server`` after the pull, inside a process that still holds the old +``hermes_platform`` and other first-party modules. Importing that one +submodule must not run the rest of the package, or any name added to an +already-loaded module since the release crashes ``hermes update``. +""" + +import subprocess +import sys + +_PROBE = ( + "import sys\n" + "import hermes_cli.local_runtime.processes\n" + "loaded = sorted(m for m in sys.modules if m.startswith(('hermes_platform', 'hermes_cli.local_runtime.')))\n" + "print('\\n'.join(loaded))\n" +) + + +def test_processes_import_runs_no_other_local_runtime_module(): + out = subprocess.run([sys.executable, "-c", _PROBE], capture_output=True, text=True, check=True).stdout + assert out.split() == ["hermes_cli.local_runtime.processes"] + + +def test_package_exports_still_resolve(): + import hermes_cli.local_runtime as local_runtime + from hermes_cli.local_runtime.hardware import probe_budget + + assert local_runtime.probe_budget is probe_budget From cbffbeecf4ebb7f24e17241f8738923bfa069fd2 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Wed, 7 Oct 2026 05:48:02 +0530 Subject: [PATCH 05/13] ci: package __init__ files on the update path start the update E2Es Importing a/b/c.py runs a/__init__.py and a/b/__init__.py first. The update-lane routing listed hermes_cli/local_runtime/processes.py (the v2026.9.24 updater imports it after the pull) but not the package init that import runs. #134116 changed what that init loads, its PR never ran e2e-upgrade, and main went red until #134242. classify_changes now routes the package inits above every module the e2e_upgrade and e2e_desktop_update lanes own, and above each shared hub, because a hub's inits run whenever the hub is imported. An init that is itself a hub stays out. The routing guard's import walker counts the package inits a module's imports run, so the next unrouted one fails it. This routes eight inits that were missing: local_runtime, gateway, cron, tools, tools/environments, plugins, hermes_platform, hermes_cli/observability. --- scripts/ci/classify_changes.py | 16 ++++++++++++++++ tests/ci/test_update_ci_routing.py | 2 ++ tests/ci/test_update_transitive_routing.py | 3 +++ 3 files changed, 21 insertions(+) diff --git a/scripts/ci/classify_changes.py b/scripts/ci/classify_changes.py index 70d69c5553..7200475a2e 100644 --- a/scripts/ci/classify_changes.py +++ b/scripts/ci/classify_changes.py @@ -451,6 +451,22 @@ _E2E_LANES: dict[str, tuple[str, ...]] = { "hermes_cli/web_routers/status.py", ), } + + +def _with_package_inits(paths: tuple[str, ...]) -> tuple[str, ...]: + """Importing a/b/c.py runs a/__init__.py and a/b/__init__.py first, so a routed module + routes the package inits above it. A shared hub's inits run whenever the hub is imported, so + they are routed too; only an init that is itself a hub stays out.""" + root = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + inits = [f"{'/'.join(parts[:i])}/__init__.py" + for p in (*paths, *sorted(_SHARED_HUBS)) if p.endswith(".py") + for parts in [p.split("/")[:-1]] for i in range(1, len(parts) + 1)] + return tuple(dict.fromkeys([*paths, *(i for i in inits if i not in _SHARED_HUBS + and os.path.isfile(os.path.join(root, i)))])) + + +for _lane in ("e2e_upgrade", "e2e_desktop_update"): + _E2E_LANES[_lane] = _with_package_inits(_E2E_LANES[_lane]) # The upgrade journeys are their own lane; editing one does not start ``e2e``. # The update suite shares apps/desktop/e2e/ with the core suite but not its specs. _E2E_LANE_EXCLUDES = { diff --git a/tests/ci/test_update_ci_routing.py b/tests/ci/test_update_ci_routing.py index 1fa3e06180..7863d9a79a 100644 --- a/tests/ci/test_update_ci_routing.py +++ b/tests/ci/test_update_ci_routing.py @@ -650,6 +650,8 @@ def _imports(path: Path) -> frozenset[str]: base = ".".join([*parts, base] if base else parts) names.add(base) names.update(f"{base}.{a.name}" for a in node.names) + # Importing a.b.c runs a/__init__.py and a/b/__init__.py first. + names |= {".".join(n.split(".")[:i]) for n in names for i in range(1, n.count(".") + 1)} found = set() for name in names: target = _module_file(name) diff --git a/tests/ci/test_update_transitive_routing.py b/tests/ci/test_update_transitive_routing.py index 6aeab38360..e2d21445c0 100644 --- a/tests/ci/test_update_transitive_routing.py +++ b/tests/ci/test_update_transitive_routing.py @@ -17,6 +17,9 @@ from tests.ci.test_update_ci_routing import _REPO, _ci_run, _consumers_reached, # backup is the update transaction's pre-build step, not a build dependency. @pytest.mark.parametrize("path", [ "hermes_cli/local_runtime/processes.py", + # Importing processes runs the package init: v2026.9.24's update died there on a name + # the init's eager imports needed from an already-loaded module. + "hermes_cli/local_runtime/__init__.py", "agent/deadline.py", "agent/memory_provider.py", "pm/plugins_state.py", From 855997b7d7e90a28ec6259a5160a92ea8e062fe9 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 1 Oct 2026 16:47:57 +0530 Subject: [PATCH 06/13] fix(pm): give Store a long-path root on Windows Windows cuts plain file calls at 260 characters when LongPathsEnabled is 0, which is the default. PM's copy, walk, digest, rename and delete calls under the store then fail or silently skip files (#130242). Store now spells its root in the extended-length form (\\?\C:\...). Every path built from it with entry() or / inherits that spelling, so the file calls beneath the root no longer hit the limit. Relative paths, and so digests, are unchanged. On POSIX the root is untouched. native() returns the ordinary spelling for text that leaves PM's own file calls (argv, environment values, hashed identities, records and messages). The call sites adopt it in follow-up commits. --- pm/filesystem.py | 32 ++++++++++++++++++++++++++++++++ pm/store.py | 4 ++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/pm/filesystem.py b/pm/filesystem.py index 37d3e04de5..eb829818e3 100644 --- a/pm/filesystem.py +++ b/pm/filesystem.py @@ -7,6 +7,7 @@ from __future__ import annotations import errno import hashlib +import ntpath import os from pathlib import Path import shutil @@ -23,6 +24,37 @@ def is_junction(path: Path) -> bool: return os.name == "nt" and path.lstat().st_reparse_tag == stat.IO_REPARSE_TAG_MOUNT_POINT +_VERBATIM = "\\\\?\\" +_VERBATIM_UNC = "\\\\?\\UNC\\" + + +def _long_form(text: str) -> str: + if text.startswith(_VERBATIM): + return text + # Windows stops normalising a verbatim path, so it must already be absolute, + # backslashed and free of `..` before the prefix goes on. + text = ntpath.abspath(text) + if text.startswith("\\\\"): + return _VERBATIM_UNC + text[2:] + return _VERBATIM + text + + +def long_root(path: Path) -> Path: + """Spell a PM-owned root so Windows file calls beneath it are not cut at + MAX_PATH when LongPathsEnabled is 0 (the default). Every path joined onto the + result inherits the spelling; relative paths and digests are unchanged.""" + return Path(_long_form(os.fspath(path))) if os.name == "nt" else path + + +def native(path: str | os.PathLike[str]) -> str: + """The ordinary spelling, for text that leaves PM's own file calls: argv, cwd, + environment values, hashed identities, records and messages.""" + text = os.fspath(path) + if text.startswith(_VERBATIM_UNC): + return "\\\\" + text[len(_VERBATIM_UNC):] + return text[len(_VERBATIM):] if text.startswith(_VERBATIM) else text + + def lock_fd(fd: int, *, wait: bool, timeout: float | None = None) -> bool: """Take the byte lock; ``timeout`` bounds the retry loop (None waits forever, 0 tries once).""" deadline = None if timeout is None else time.monotonic() + timeout diff --git a/pm/store.py b/pm/store.py index b6754651e6..ac0d63ce52 100644 --- a/pm/store.py +++ b/pm/store.py @@ -14,7 +14,7 @@ from contextlib import contextmanager from pathlib import Path from typing import IO -from pm.filesystem import is_junction, retry_held +from pm.filesystem import is_junction, long_root, retry_held @@ -348,7 +348,7 @@ class Store: Hash-keyed archives survive failed installs. Publication releases them.""" def __init__(self, root: Path): - self.root = root + self.root = long_root(root) def entry(self, name: str) -> Path: return self.root / name From af60718b03f874ec0b76a4ae76cfb27a9e4c7714 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 1 Oct 2026 16:55:35 +0530 Subject: [PATCH 07/13] test(pm): live Windows proof for PM with LongPathsEnabled=0 (#130242) Drives a real Store through scratch, the bundled-copy copytree, tree_digest, publish and removal over a tree with files and a directory beyond MAX_PATH, then checks the facts record and composed child env carry no \\?\ prefix. The test first asserts RtlAreLongPathsEnabled() is false so it cannot pass as a false proof. The wine2e lane sets LongPathsEnabled=0 in a separate, final step and selects the test with -m 'platforms and integration'. --- .github/workflows/windows-venv-e2e.yml | 20 +++ tests/pm/test_long_paths_windows_live.py | 169 +++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 tests/pm/test_long_paths_windows_live.py diff --git a/.github/workflows/windows-venv-e2e.yml b/.github/workflows/windows-venv-e2e.yml index 5ec4ab0892..1c5abca933 100644 --- a/.github/workflows/windows-venv-e2e.yml +++ b/.github/workflows/windows-venv-e2e.yml @@ -80,3 +80,23 @@ jobs: run: | set -uo pipefail bash scripts/run_tests.sh tests/tools/test_process_registry_windows_live.py + + - name: Turn Windows long paths off for later steps (#130242) + # The runner image turns LongPathsEnabled on; 0 is the Windows default + # most users have. A process reads the value when it starts, so only + # later steps see it. Keep this step after every lane that expects the + # image's setting. + shell: pwsh + run: | + reg add 'HKLM\SYSTEM\CurrentControlSet\Control\FileSystem' /v LongPathsEnabled /t REG_DWORD /d 0 /f + if ($LASTEXITCODE) { exit $LASTEXITCODE } + (Get-ItemProperty 'HKLM:\SYSTEM\CurrentControlSet\Control\FileSystem').LongPathsEnabled + + - name: Run PM long-path live E2E with long paths off (#130242) + # The test first asserts RtlAreLongPathsEnabled() is false in its own + # process, so this step cannot pass if the step above did not take. + # ``-m`` replaces pyproject's addopts, which deselect ``integration``. + shell: bash + run: | + set -uo pipefail + bash scripts/run_tests.sh tests/pm/test_long_paths_windows_live.py -- -m "platforms and integration" -v diff --git a/tests/pm/test_long_paths_windows_live.py b/tests/pm/test_long_paths_windows_live.py new file mode 100644 index 0000000000..7fc922d1c0 --- /dev/null +++ b/tests/pm/test_long_paths_windows_live.py @@ -0,0 +1,169 @@ +"""Live Windows proof for PM under ``LongPathsEnabled = 0`` (#130242, #130232). + +Runs ONLY on a real Windows host whose registry value +``HKLM\\SYSTEM\\CurrentControlSet\\Control\\FileSystem\\LongPathsEnabled`` is 0 +(the Windows default). The on-demand ``wine2e/**`` lane +(``.github/workflows/windows-venv-e2e.yml``) sets it in an earlier step, so this +test's process starts with long paths off, and selects the file with +``-m integration`` (the marker keeps it out of the ordinary OS lane, whose runner +has long paths on and could not make the claim). + +Nothing is mocked: a ``Store`` rooted under a temp directory drives the real +scratch / copy / digest / publish / removal path a bundled-copy install takes +(``pm.install._copy_verified_source`` plus ``_install``), over a tree whose files +and one directory lie beyond MAX_PATH. The test reaches its own fixtures through +``\\\\?\\`` spellings it builds itself, so a check never trusts the very calls +under test (plain ``Path.exists`` silently answers False beyond 260 characters). +""" + +from __future__ import annotations + +import json +import ntpath +import os +import shutil +import sys +from pathlib import Path + +import pytest + +pytestmark = [ + pytest.mark.platforms("windows"), # live long-path behaviour exists only on Windows + # Requires a host configured with long paths off; the wine2e lane selects it. + pytest.mark.integration, +] + +MAX_PATH = 260 +VERBATIM = "\\\\?\\" +SEGMENT = "s" * 60 + + +def _long_paths_enabled() -> bool: + import ctypes + + ntdll = ctypes.WinDLL("ntdll") + ntdll.RtlAreLongPathsEnabled.restype = ctypes.c_ubyte + return bool(ntdll.RtlAreLongPathsEnabled()) + + +def _verbatim(path: Path | str) -> Path: + """The test's own independent spelling for checks beyond MAX_PATH.""" + text = ntpath.abspath(os.fspath(path)) + return Path(text if text.startswith(VERBATIM) else VERBATIM + text) + + +def _plain(path: Path | str) -> str: + text = os.fspath(path) + return text[len(VERBATIM):] if text.startswith(VERBATIM) else text + + +def _strings(value): + if isinstance(value, str): + yield value + elif isinstance(value, dict): + for key, item in value.items(): + yield key + yield from _strings(item) + elif isinstance(value, list): + for item in value: + yield from _strings(item) + + +def _build_tree(entry: Path, plain_base_length: int) -> dict[str, bytes]: + """Files beyond MAX_PATH, one directory beyond MAX_PATH, and a short binary. + + ``plain_base_length`` is the plain length of the shortest root the tree + will sit under, so every deep path exceeds MAX_PATH wherever it lands. + """ + deep = "" + while plain_base_length + len(deep) <= MAX_PATH + 10: + deep = f"{deep}/{SEGMENT}" if deep else SEGMENT + files = { + "bin/tool.exe": b"not a real binary", + "short.txt": b"short", + # File whose full path crosses MAX_PATH inside a directory that does not. + f"{deep.rsplit('/', 1)[0]}/{'f' * 80}.txt": b"file beyond MAX_PATH", + # Directory whose own path is beyond MAX_PATH, holding two files. + f"{deep}/inner/a.txt": b"inside a directory beyond MAX_PATH", + f"{deep}/inner/b.py": b"print('deep')\n", + } + for rel, data in files.items(): + target = _verbatim(entry) / Path(rel) + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(data) + return files + + +def _assert_tree(entry: Path, files: dict[str, bytes]) -> None: + root = _verbatim(entry) + for rel, data in files.items(): + assert (root / Path(rel)).read_bytes() == data, rel + found = sorted(p.relative_to(root).as_posix() for p in root.rglob("*") if p.is_file()) + assert found == sorted(files) + + +def test_store_lifecycle_beyond_max_path_with_long_paths_disabled(tmp_path): + # A green run must not be a false proof: long paths really are off here. + assert sys.platform == "win32" + assert not _long_paths_enabled(), ( + "LongPathsEnabled is on for this process; set HKLM\\SYSTEM\\CurrentControlSet\\" + "Control\\FileSystem LongPathsEnabled=0 before the interpreter starts") + probe = _verbatim(tmp_path) / ("p" * 120) / ("q" * 120) / "probe.txt" + probe.parent.mkdir(parents=True) + probe.write_bytes(b"x") + assert len(_plain(probe)) > MAX_PATH + with pytest.raises(OSError): + os.stat(_plain(probe)) # the host really cuts plain spellings at MAX_PATH + shutil.rmtree(_verbatim(tmp_path) / ("p" * 120)) + + from pm.install import _remove_entry + from pm.lock import Facts + from pm.package import Package, compose_env + from pm.store import Store, current_target, tree_digest + + class Tool(Package): + name = "longpath-probe" + + def binary(self, entry: Path, target: str) -> Path: + return entry / "bin" / "tool.exe" + + package, target, version = Tool(), current_target(), "1.0.0" + entry_name = package.store_entry(version, target) + source_root, store_root = tmp_path / "src", tmp_path / "dst" + source, store = Store(source_root), Store(store_root) + + files = _build_tree(source_root / entry_name, + len(_plain(tmp_path / "dst" / entry_name))) + expected = tree_digest(_verbatim(source_root / entry_name)) + + with store.scratch() as scratch: + staged = scratch / "staged" + # The copy _copy_verified_source performs, then its digest check. + shutil.copytree(source.entry(entry_name), staged, symlinks=True) + assert tree_digest(staged) == expected + assert tree_digest(source.entry(entry_name)) == expected + _assert_tree(staged, files) + # Bytes scratch() must clean up itself (an abandoned extraction). + abandoned = scratch / "abandoned" + shutil.copytree(source.entry(entry_name), abandoned, symlinks=True) + published = store.publish(staged, entry_name) + assert not _verbatim(scratch).exists(), "scratch() left bytes behind beyond MAX_PATH" + assert not [p for p in _verbatim(store_root).iterdir() if p.name.startswith(".staging-")] + + _assert_tree(store_root / entry_name, files) + assert tree_digest(published) == expected + + # Records and child environments are exits: they carry the ordinary spelling. + facts = Facts(tmp_path / "facts.json") + facts.record(package.name, version, entry_name, package.env(published, target), store.root, + target=target, artifacts=["0" * 64], digest=tree_digest(published)) + assert facts.installed(package.name, version, store.root, (target, ("0" * 64,))) + on_disk = json.loads((tmp_path / "facts.json").read_text(encoding="utf-8")) + assert not [s for s in _strings(on_disk) if VERBATIM in s] + child_env = compose_env([facts.env_for(package.name, store.root)], base={}) + assert not [v for v in child_env.values() if VERBATIM in v], child_env + assert ntpath.normcase(child_env["PATH"]) == ntpath.normcase( + _plain(store_root / entry_name / "bin")) + + _remove_entry(store, entry_name) + assert not _verbatim(store_root / entry_name).exists(), "removal left bytes beyond MAX_PATH" From 953a598860e60920bbc7c8cd8d947609df118a36 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 1 Oct 2026 17:02:41 +0530 Subject: [PATCH 08/13] fix(pm): give text that leaves PM the ordinary spelling of store paths Store now spells its root \\?\C:\... on Windows so PM's own file calls under it pass MAX_PATH. That spelling must not escape: the same folder spelled two ways hashes and compares differently, and children such as cmd.exe, gpg or uv refuse or record the prefix (uv would write it into pyvenv.cfg). native() now converts store-derived paths where their text leaves PM: - process argv and cwd: uv via PythonEnvironment._run (its one exit), binary version probes, npm self-install, the PortableGit extractor, gpg provenance checks, the plugin-eviction interpreter probe; - environment values: compose_env (PATH and package env diffs), the activation PATH dirs, HERMES_RUNTIME_DIR; - hashed identities: the PM runtime inputs and isolated-environment identity, so existing records still match and nothing rebuilds; - API results callers execute or compare: installed_package, uv_launcher, verified_tools entries; - user-facing verification messages. Facts records already store {STORE}-templated env, so no absolute path reaches them. Tests pin that identities are spelling-independent and that records, composed env and uv invocations from a verbatim root carry no prefix (both fail on e8c97320ac). --- pm/build_operations.py | 7 +++-- pm/environment.py | 13 +++++--- pm/install.py | 13 +++++--- pm/operations.py | 5 +-- pm/package.py | 17 ++++++---- pm/packages.py | 23 +++++++------- pm/plugin_eviction.py | 4 +-- pm/runtime.py | 6 ++-- pm/security_packages.py | 5 +-- tests/pm/test_long_path_exits.py | 54 ++++++++++++++++++++++++++++++++ 10 files changed, 110 insertions(+), 37 deletions(-) create mode 100644 tests/pm/test_long_path_exits.py diff --git a/pm/build_operations.py b/pm/build_operations.py index c468a4d481..a41b49d6b0 100644 --- a/pm/build_operations.py +++ b/pm/build_operations.py @@ -7,6 +7,7 @@ from pathlib import Path import sys from types import MappingProxyType +from pm.filesystem import native from pm.install import InstalledPackage from pm.lock import Lockfile from pm.package import InstallError @@ -127,7 +128,7 @@ def _copy_links(package, entry: Path) -> None: if path.is_symlink() or is_junction(path): if (Path(os.readlink(path)).is_absolute() or not path.exists() or not path.resolve().is_relative_to(entry)): - raise InstallError(package.name, f"tool copy link escapes entry: {path}") + raise InstallError(package.name, f"tool copy link escapes entry: {native(path)}") @dataclass(frozen=True) @@ -187,7 +188,9 @@ def verified_tools(names: Sequence[str], *, source_store: Path, target: str, if ((binary is not None and (not binary.is_file() or not binary.resolve().is_relative_to(entry))) or facts.env_for(package.name, source_store) != env): raise InstallError(package.name, "tool source failed verification: binary or environment") - entries[package.name] = InstalledPackage(entry, version, binary) + # The selection leaves PM (bundle scripts compare and execute it): ordinary spelling. + entries[package.name] = InstalledPackage(Path(native(entry)), version, + Path(native(binary)) if binary is not None else None) envs.append(env) return VerifiedTools(MappingProxyType(entries), tuple(envs)) diff --git a/pm/environment.py b/pm/environment.py index 076ebd8c9b..ea2f856517 100644 --- a/pm/environment.py +++ b/pm/environment.py @@ -19,6 +19,7 @@ import tempfile import time from typing import TextIO +from pm.filesystem import native from pm.package import InstallError from pm.progress import LiveTail, TextSink, verbose_output @@ -142,7 +143,7 @@ def _run_streaming(command: list[str], *, cwd: Path, env: dict[str, str], timeout: int, output: TextSink) -> subprocess.CompletedProcess: """Keep CI progress live, a bounded diagnostic tail, and a wall-clock timeout.""" deadline = time.monotonic() + timeout - proc = subprocess.Popen(command, cwd=str(cwd), env=env, stdout=subprocess.PIPE, + proc = subprocess.Popen(command, cwd=native(cwd), env=env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, encoding="utf-8", errors="replace", bufsize=0) pipe = proc.stdout assert isinstance(pipe, io.TextIOWrapper) # Popen was given stdout=PIPE and text=True. @@ -284,11 +285,13 @@ class PythonEnvironment: from pm.index_config import is_index_redirect env = {key: value for key, value in env.items() if not is_index_redirect(key)} - env.update(UV_PYTHON=str(self.python), UV_PROJECT_ENVIRONMENT=str(self.destination), - UV_CACHE_DIR=str(self.cache), UV_PYTHON_DOWNLOADS="never") + # uv is a child: it gets the ordinary spelling of store paths (pm.filesystem.native), + # or the venv it writes would record the extended-length one in pyvenv.cfg. + env.update(UV_PYTHON=native(self.python), UV_PROJECT_ENVIRONMENT=native(self.destination), + UV_CACHE_DIR=native(self.cache), UV_PYTHON_DOWNLOADS="never") with tempfile.TemporaryDirectory(prefix="pm-uv-config-") as config: env.update(XDG_CONFIG_HOME=config, XDG_CONFIG_DIRS=config) - command = [str(self.uv), *args] + command = [native(self.uv), *map(native, args)] if self.no_config and "--no-config" not in command: command.append("--no-config") if self.offline: @@ -314,7 +317,7 @@ class PythonEnvironment: raise tail.close(result.returncode == 0) return result - return subprocess.run(command, cwd=str(cwd), env=env, capture_output=True, + return subprocess.run(command, cwd=native(cwd), env=env, capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=timeout) except subprocess.TimeoutExpired as exc: from pm.index_config import TIMEOUT_HINT diff --git a/pm/install.py b/pm/install.py index fe83afe86a..e4648feabd 100644 --- a/pm/install.py +++ b/pm/install.py @@ -15,7 +15,7 @@ from typing import Optional from pm import paths from pm.downloader import DownloadPaused, ProgressFn -from pm.filesystem import remove_tree, retry_held +from pm.filesystem import native, remove_tree, retry_held from pm.lock import Facts, Lockfile from pm.package import InstallError, Package, Runner, StatePackage, compose_env from pm.plugin_inputs import Candidates, Members, PluginInput, Selection, StagedUpdate @@ -128,7 +128,10 @@ def installed_package(name: str, *, allow_outdated: bool = False) -> InstalledPa facts, store = location fact = facts.get(name) entry = store.entry(fact["entry"]) - return InstalledPackage(entry, fact["version"], package.binary(entry, target)) + binary = package.binary(entry, target) + # Callers execute and compare these paths outside PM: the ordinary spelling, not the store's. + return InstalledPackage(Path(native(entry)), fact["version"], + Path(native(binary)) if binary is not None else None) def uv_launcher(name: str) -> Path | None: @@ -145,7 +148,7 @@ def uv_launcher(name: str) -> Path | None: facts, store = location binary = package.binary(store.entry(facts.get("uv")["entry"]), target) launcher = binary.with_name(name + binary.suffix) if binary is not None else None - return launcher if launcher is not None and launcher.is_file() else None + return Path(native(launcher)) if launcher is not None and launcher.is_file() else None def _identity(lockfile: Lockfile, name: str, target: str): @@ -936,8 +939,8 @@ def _store_path_dirs() -> list[str]: if isinstance(path_dirs, str): path_dirs = [path_dirs] for directory in path_dirs: - if directory and directory not in dirs: - dirs.append(str(directory)) + if directory and native(directory) not in dirs: + dirs.append(native(directory)) return dirs diff --git a/pm/operations.py b/pm/operations.py index f25421b7cc..f92507c4ed 100644 --- a/pm/operations.py +++ b/pm/operations.py @@ -254,7 +254,7 @@ def _ensure_generation( Build at the final path: Windows launchers and scripts embed that path. The prior generation survives both successful replacement and failed builds. """ - from pm.filesystem import lock_fd + from pm.filesystem import lock_fd, native from pm._uv import _toolchain from pm.install import _refuse_lazy, lazy_installs_allowed from pm.lock import Lockfile, _write @@ -273,7 +273,8 @@ def _ensure_generation( def identity(base_python: Path) -> str: # The same pinned artifact can live in different stores. A venv's # pyvenv.cfg keeps the original interpreter path, not just its version. - return hashlib.sha256(json.dumps({**inputs, "interpreter": str(base_python)}, + # The store spells its root extended-length on Windows; the identity must not. + return hashlib.sha256(json.dumps({**inputs, "interpreter": native(base_python)}, sort_keys=True).encode()).hexdigest() def current(base_python: Path | None) -> Path | None: diff --git a/pm/package.py b/pm/package.py index 600cb1f461..da459d963f 100644 --- a/pm/package.py +++ b/pm/package.py @@ -8,6 +8,8 @@ import subprocess from pathlib import Path, PurePosixPath from typing import TYPE_CHECKING, Optional +from pm.filesystem import native + if TYPE_CHECKING: from pm.store import Store @@ -24,18 +26,21 @@ def compose_env(diffs: list[dict], base: Optional[dict] = None) -> dict[str, str """Dependents win over their dependencies for every key: diffs arrive deps-first, later ones take precedence — npm's pinned shim must shadow the npm bundled inside node, and a package's exports beat inherited env. - 'PATH' values are lists of dirs, prepended.""" + 'PATH' values are lists of dirs, prepended. + + Diff values are paths under a store root, which PM spells extended-length + on Windows; a child environment gets their ordinary spelling.""" env = dict(os.environ if base is None else base) path_dirs: list[str] = [] for diff in reversed(diffs): for key, value in diff.items(): if key == "PATH": dirs = value if isinstance(value, list) else [value] - path_dirs.extend(str(d) for d in dirs if str(d) not in path_dirs) + path_dirs.extend(native(d) for d in dirs if native(d) not in path_dirs) for diff in diffs: for key, value in diff.items(): if key != "PATH": - env[key] = str(value) + env[key] = native(str(value)) if path_dirs: key = next((k for k in env if k.upper() == "PATH"), "PATH") existing = env.get(key, "") @@ -209,7 +214,7 @@ class DebPackage(Package): the digest already proved the bytes.""" expected = entry / self.prefix_rel / self.main_rel(target) if not expected.is_file() and not expected.is_symlink(): - return f"{expected.relative_to(entry)} missing under {entry}" + return f"{expected.relative_to(entry)} missing under {native(entry)}" return "" def main_rel(self, target: str) -> str: @@ -355,14 +360,14 @@ def _missing_reason(binary: Path, entry: Path) -> str: """Why a package's expected binary is not where it should be — the diagnosis that tells you whether the pin's layout is wrong.""" rel = binary.relative_to(entry).as_posix() - return f"{rel} missing under {entry}; {_entry_listing(entry)}" + return f"{rel} missing under {native(entry)}; {_entry_listing(entry)}" def _probe_reason(binary: Path, proc: "subprocess.CompletedProcess") -> str: """Why a --version probe failed: the exit code plus output tail.""" out = (proc.stdout or b"") + (proc.stderr or b"") tail = out.decode(errors="replace").strip()[-300:] - return f"{binary} --version exited {proc.returncode}" + (f": {tail}" if tail else "") + return f"{native(binary)} --version exited {proc.returncode}" + (f": {tail}" if tail else "") class Runner: diff --git a/pm/packages.py b/pm/packages.py index 6ea498ad2a..e6308ee51e 100644 --- a/pm/packages.py +++ b/pm/packages.py @@ -12,6 +12,7 @@ import tempfile from pathlib import Path from typing import Optional +from pm.filesystem import native from pm.package import ( DebPackage, InstallError, @@ -101,16 +102,16 @@ class BinaryPackage(Package): return "" try: proc = subprocess.run( - [str(binary), *self.probe_args], + [native(binary), *self.probe_args], capture_output=True, timeout=60, - cwd=str(binary.parent) if self.probe_cwd else None, + cwd=native(binary.parent) if self.probe_cwd else None, env=self._probe_env(), ) except OSError as e: - return f"could not exec {binary} {' '.join(self.probe_args)}: {e}" + return f"could not exec {native(binary)} {' '.join(self.probe_args)}: {e}" except subprocess.TimeoutExpired: - return f"{binary} {' '.join(self.probe_args)} timed out after 60s" + return f"{native(binary)} {' '.join(self.probe_args)} timed out after 60s" if proc.returncode != 0: return _probe_reason(binary, proc) return "" @@ -614,9 +615,9 @@ class Npm(BinaryPackage): with tempfile.TemporaryDirectory(prefix="hermes-npm-cache-", ignore_cleanup_errors=True) as cache: proc = subprocess.run( [ - str(node_bin), str(bundled_cli), "install", "--global", - "--prefix", str(staged), "--offline", "--ignore-scripts", - "--no-audit", "--no-fund", str(archive), + native(node_bin), native(bundled_cli), "install", "--global", + "--prefix", native(staged), "--offline", "--ignore-scripts", + "--no-audit", "--no-fund", native(archive), ], capture_output=True, text=True, @@ -699,7 +700,7 @@ class Git(BinaryPackage): # open past the stub's exit. Under -y the stub prints nothing anyway. try: proc = subprocess.run( - [str(exe), f"-o{staged}", "-y"], + [native(exe), f"-o{native(staged)}", "-y"], stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, @@ -1040,16 +1041,16 @@ class Chromium(Package): def verify(self, entry: Path, target: str) -> str: marker = entry / "INSTALLATION_COMPLETE" if not marker.is_file(): - return f"INSTALLATION_COMPLETE missing under {entry}; {_entry_listing(entry)}" + return f"INSTALLATION_COMPLETE missing under {native(entry)}; {_entry_listing(entry)}" binary = self.binary(entry, target) if binary is None: - return f"Chromium executable missing under {entry}" + return f"Chromium executable missing under {native(entry)}" return self._binary_reason(binary, entry, target) def env(self, entry: Path, target: str) -> dict: binary = self.binary(entry, target) if binary is None: - raise InstallError(self.name, f"Chromium executable missing under {entry}") + raise InstallError(self.name, f"Chromium executable missing under {native(entry)}") return { "PLAYWRIGHT_BROWSERS_PATH": str(entry.parent), "AGENT_BROWSER_EXECUTABLE_PATH": str(binary), diff --git a/pm/plugin_eviction.py b/pm/plugin_eviction.py index 512b029f19..093ea88c90 100644 --- a/pm/plugin_eviction.py +++ b/pm/plugin_eviction.py @@ -27,7 +27,7 @@ from pathlib import Path from pm.environment import BuildFailure, ResolutionConflict from pm.environments import install_state_dir, runtime_facts_path -from pm.filesystem import durable_write_bytes, file_digest, read_bytes_or_none +from pm.filesystem import durable_write_bytes, file_digest, native, read_bytes_or_none from pm.package import InstallError Entry = tuple[Path, str, Path] # (home plugins dir, selection key, plugin dir) @@ -40,7 +40,7 @@ def _interpreter_version() -> str: tools = _toolchain(explicit=True) if tools is None: raise InstallError("venv", "PM's pinned toolchain is unavailable") - probe = subprocess.run([str(tools[1]), "-I", "-c", "import platform; print(platform.python_version())"], + probe = subprocess.run([native(tools[1]), "-I", "-c", "import platform; print(platform.python_version())"], capture_output=True, text=True, check=True, timeout=60) return probe.stdout.strip() diff --git a/pm/runtime.py b/pm/runtime.py index ec8205d07f..e1e5399221 100644 --- a/pm/runtime.py +++ b/pm/runtime.py @@ -15,6 +15,7 @@ import sys from typing import Callable import uuid +from pm.filesystem import native from pm.package import InstallError @@ -27,7 +28,7 @@ def runtime_environment() -> dict[str, str]: env = _base_environment() env["HERMES_HOME"] = str(get_hermes_home()) - env["HERMES_RUNTIME_DIR"] = str(store_root()) + env["HERMES_RUNTIME_DIR"] = native(store_root()) return env @@ -46,7 +47,8 @@ def _inputs(project: Path, python: Path, *, as_spelled: bool = False) -> str: # the same one reached through a symlinked store (a per-task HERMES_HOME whose # tools/ links back) must. Only the directories resolve: the pinned # bin/python3 is itself a symlink, so resolving it would re-key every install. - python = python.absolute() + # A store spells its root extended-length on Windows; the identity must not. + python = Path(native(python)).absolute() digest.update(str(python if as_spelled else python.parent.resolve() / python.name).encode()) return digest.hexdigest() diff --git a/pm/security_packages.py b/pm/security_packages.py index 871e5c0b2d..aa58ca4838 100644 --- a/pm/security_packages.py +++ b/pm/security_packages.py @@ -9,6 +9,7 @@ from pathlib import Path from urllib.parse import urlparse from pm import paths +from pm.filesystem import native from pm.lock import Lockfile from pm.package import InstallError from pm.packages import BinaryPackage, _RUST_TRIPLE @@ -120,12 +121,12 @@ class IronProxy(_SignedBinary): key = directory / "public-key.asc" if not signature.is_file() or not key.is_file(): raise InstallError(self.name, "pinned signature assets missing") - imported = subprocess.run([*args, "--import", str(key)], stdin=subprocess.DEVNULL, + imported = subprocess.run([*args, "--import", native(key)], stdin=subprocess.DEVNULL, capture_output=True, timeout=60, check=False) if imported.returncode: logging.getLogger(__name__).warning("Could not import iron-proxy signing key; archive checksum remains enforced") return - verified = subprocess.run([*args, "--verify", str(signature), str(directory / "checksums.txt")], + verified = subprocess.run([*args, "--verify", native(signature), native(directory / "checksums.txt")], stdin=subprocess.DEVNULL, capture_output=True, timeout=60, check=False) if verified.returncode: raise InstallError(self.name, "GPG signature verification failed") \ No newline at end of file diff --git a/tests/pm/test_long_path_exits.py b/tests/pm/test_long_path_exits.py new file mode 100644 index 0000000000..e5a80c960e --- /dev/null +++ b/tests/pm/test_long_path_exits.py @@ -0,0 +1,54 @@ +"""A store root spelled in Windows' extended-length form (``\\\\?\\C:\\...``) stays inside PM. + +The spelling exists only so PM's own file calls beneath the root pass MAX_PATH. Text that +leaves PM must carry the ordinary spelling: the same folder spelled two ways hashes and +compares differently, and child tools (cmd.exe, gpg, installers) may refuse the prefix. +The roots below are plain text, so both invariants hold on every OS. +""" +import subprocess +from pathlib import Path + +from pm import environment as environment_module +from pm.environment import PythonEnvironment +from pm.lock import Facts +from pm.package import compose_env +from pm.runtime import _inputs +from pm.store import Store + +VERBATIM = "\\\\?\\" +ROOT = Path(VERBATIM + r"C:\Program Files\WindowsApps\Hermes\agent-payload\tools") + + +def test_identity_does_not_depend_on_the_root_spelling(tmp_path): + for name in ("pyproject.toml", "uv.lock"): + (tmp_path / name).write_text(name, encoding="utf-8") + plain = Path(r"C:\Program Files\WindowsApps\Hermes\agent-payload\tools\python-3.12\python.exe") + + assert _inputs(tmp_path, Path(VERBATIM + str(plain))) == _inputs(tmp_path, plain) + + +def test_no_verbatim_spelling_leaves_through_records_environment_or_uv(tmp_path, monkeypatch): + store = Store(ROOT) + facts = Facts(tmp_path / "facts.json") + facts.record("git", "1", "git-1", {"PATH": [str(store.entry("git-1") / "cmd")]}, store.root) + facts.record("chromium", "1", "chromium-1", + {"AGENT_BROWSER_EXECUTABLE_PATH": str(store.entry("chromium-1") / "chrome.exe")}, store.root) + record = (tmp_path / "facts.json").read_text(encoding="utf-8") + + env = compose_env([facts.env_for("git", store.root), facts.env_for("chromium", store.root)], base={}) + + calls = [] + + def run(command, **kwargs): + calls.append((command, kwargs)) + return subprocess.CompletedProcess(command, 0, "", "") + + monkeypatch.setattr(environment_module.subprocess, "run", run) + PythonEnvironment(uv=store.entry("uv-1") / "uv.exe", python=store.entry("python-1") / "python.exe", + destination=tmp_path / "venv", cache=tmp_path / "cache", + env={}).sync(store.entry("source")) + (command, kwargs), = calls + + leaving = [record, *env.values(), *command, kwargs["cwd"], *kwargs["env"].values()] + assert [text for text in leaving if VERBATIM in text] == [] + assert env["AGENT_BROWSER_EXECUTABLE_PATH"].endswith("chrome.exe") From a1461d11abb009d7d4067ffeec75697788c4c06f Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 1 Oct 2026 17:04:01 +0530 Subject: [PATCH 09/13] fix(pm): spell store roots PM opens without Store like Store does Some PM code reaches a store root from paths.store_root() or writable_store_root() directly instead of through Store, so its file calls kept the ordinary spelling and stay cut at MAX_PATH on Windows with LongPathsEnabled=0. Worse, its comparisons against a Store root (now \\?\-spelled) never matched. Each such root now gets long_root() once, where the variable is born: - Facts.refresh_digests digests whole payload tool trees; - verified_tools walks and digests the source store's tool trees; - the uv cache seed copies the payload's deep uv-cache tree out; - _toolchain's writable store, so `location.root != writable` compares like with like and the copy store is built from the same spelling; - _installed_location passes store.root to Facts.installed; - ensure() picks the shipped facts by comparing two Store roots; - the corrupt-facts guard compares both sides spelled long. Returned and exported paths stay ordinary (previous commit). --- pm/_uv.py | 4 +++- pm/build_operations.py | 4 +++- pm/install.py | 6 +++--- pm/lock.py | 10 ++++++++-- pm/packages.py | 11 +++++++---- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/pm/_uv.py b/pm/_uv.py index 76ba629c6d..b70477463e 100644 --- a/pm/_uv.py +++ b/pm/_uv.py @@ -4,6 +4,7 @@ from __future__ import annotations from pathlib import Path from pm import paths +from pm.filesystem import long_root from pm.lock import Facts from pm.package import InstallError from pm.registry import get_package @@ -35,7 +36,8 @@ def _toolchain(*, realize: bool = True, explicit: bool = False) -> tuple[Path, P return None raise InstallError(name, "pinned tool is unavailable", "run `hermes pm install`") if name == "python" and target.startswith("win32") and sealed(): - writable = paths.writable_store_root() + # Spelled like Store spells its root, so the comparison and the copy's file calls agree. + writable = long_root(paths.writable_store_root()) if location[1].root != writable: copied = _installed_location(package, lockfile, target, verify=explicit, roots=(writable,)) if copied is None: diff --git a/pm/build_operations.py b/pm/build_operations.py index a41b49d6b0..6a4b5cc980 100644 --- a/pm/build_operations.py +++ b/pm/build_operations.py @@ -7,7 +7,7 @@ from pathlib import Path import sys from types import MappingProxyType -from pm.filesystem import native +from pm.filesystem import long_root, native from pm.install import InstalledPackage from pm.lock import Lockfile from pm.package import InstallError @@ -164,6 +164,8 @@ def verified_tools(names: Sequence[str], *, source_store: Path, target: str, source_store = Path(source_store).resolve() if not (source_store / "facts.json").is_file(): raise InstallError("tools", f"source facts missing: {source_store}") + # Admission walks and digests whole tool trees: spell the root like Store does. + source_store = long_root(source_store) facts = Facts(source_store / "facts.json", strict=True) lock = lock if lock is not None else _lockfile() entries, envs = {}, [] diff --git a/pm/install.py b/pm/install.py index e4648feabd..89b5c210d6 100644 --- a/pm/install.py +++ b/pm/install.py @@ -91,7 +91,7 @@ def _installed_location(package: Package, lockfile: Lockfile, target: str, *, store = Store(root) facts = _facts() if root == paths.store_root() else Facts(root / "facts.json") fact = facts.get(package.name) - if not fact or not facts.installed(package.name, None, root): + if not fact or not facts.installed(package.name, None, store.root): continue binary = package.binary(store.entry(fact["entry"]), target) if binary is not None and not binary.is_file(): @@ -100,7 +100,7 @@ def _installed_location(package: Package, lockfile: Lockfile, target: str, *, continue if verify and not _entry_verified(package, fact, store, target): continue - if facts.installed(package.name, lockfile.version(package.name), root, + if facts.installed(package.name, lockfile.version(package.name), store.root, _identity(lockfile, package.name, target)): return facts, store if (allow_outdated and fact.get("target") == target @@ -564,7 +564,7 @@ def ensure( raise _refuse_lazy(name, ", ".join(p.name for p in missing)) if missing: store = _operation.lock() if _operation is not None else Store(paths.writable_store_root()) - facts = _facts() if store.root == paths.store_root() else Facts(store.root / "facts.json") + facts = _facts() if store.root == _store().root else Facts(store.root / "facts.json") for package in missing: # Publication may change entries; do not carry observations across it. checked.clear() diff --git a/pm/lock.py b/pm/lock.py index e13773ea7a..4dce1e5f82 100644 --- a/pm/lock.py +++ b/pm/lock.py @@ -52,8 +52,11 @@ def _read(path: Path, *, strict: bool = False) -> dict: # the next _write would silently discard every installed-state # record. Keep the bytes for post-mortem. try: + from pm.filesystem import long_root from pm.paths import store_root - if path.parent == store_root() and (store_root().parent / "manifest.json").is_file(): + # A Store-built facts path carries the store's long spelling; compare like with like. + if (long_root(path.parent) == long_root(store_root()) + and (store_root().parent / "manifest.json").is_file()): import logging logging.getLogger(__name__).warning("invalid shipped state file: %s", path) else: @@ -160,8 +163,11 @@ class Facts: """Publish all tool digests after packaging finishes changing their bytes.""" from pm.store import tree_digest + from pm.filesystem import long_root + packages = _read(self.path, strict=True)["packages"] - root = store_root.resolve() + # Digests walk whole tool trees: the long spelling keeps them past MAX_PATH. + root = long_root(store_root.resolve()) count = 0 for name, fact in packages.items(): if not isinstance(fact, dict): diff --git a/pm/packages.py b/pm/packages.py index e6308ee51e..0f59caa1e1 100644 --- a/pm/packages.py +++ b/pm/packages.py @@ -12,7 +12,7 @@ import tempfile from pathlib import Path from typing import Optional -from pm.filesystem import native +from pm.filesystem import long_root, native from pm.package import ( DebPackage, InstallError, @@ -324,13 +324,16 @@ def uv_cache_dir() -> Path: try: from pm.paths import store_root - payload_cache = store_root().parent / "uv-cache" + # uv's cache trees run deep; both roots get the long spelling so + # the copy is not cut at MAX_PATH. The returned path stays ordinary. + payload_cache = long_root(store_root().parent / "uv-cache") if payload_cache.is_dir(): - machine_cache.mkdir(parents=True, exist_ok=True) + seeded = long_root(machine_cache) + seeded.mkdir(parents=True, exist_ok=True) for entry in payload_cache.iterdir(): if entry.name == ".seeded": continue - dest = machine_cache / entry.name + dest = seeded / entry.name if not dest.exists(): ( shutil.copytree(entry, dest) From 7ab657a06b76a1b905a77f75fa45371c31548d8e Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 1 Oct 2026 18:07:16 +0530 Subject: [PATCH 10/13] ci(wine2e): run the long-paths-off PM proof even when an earlier lane fails The PM long-path steps do not use anything the earlier live lanes set up. Gating them on those lanes hid the proof whenever an unrelated lane went red. The test step still requires the long-paths-off step to succeed, so it never runs with the runner image's setting. --- .github/workflows/windows-venv-e2e.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/windows-venv-e2e.yml b/.github/workflows/windows-venv-e2e.yml index 1c5abca933..2afe568129 100644 --- a/.github/workflows/windows-venv-e2e.yml +++ b/.github/workflows/windows-venv-e2e.yml @@ -85,7 +85,9 @@ jobs: # The runner image turns LongPathsEnabled on; 0 is the Windows default # most users have. A process reads the value when it starts, so only # later steps see it. Keep this step after every lane that expects the - # image's setting. + # image's setting. It does not depend on those lanes passing. + id: long_paths_off + if: ${{ !cancelled() }} shell: pwsh run: | reg add 'HKLM\SYSTEM\CurrentControlSet\Control\FileSystem' /v LongPathsEnabled /t REG_DWORD /d 0 /f @@ -96,6 +98,7 @@ jobs: # The test first asserts RtlAreLongPathsEnabled() is false in its own # process, so this step cannot pass if the step above did not take. # ``-m`` replaces pyproject's addopts, which deselect ``integration``. + if: ${{ !cancelled() && steps.long_paths_off.outcome == 'success' }} shell: bash run: | set -uo pipefail From 8d7d6334b0a0b6a1ecce9804d5a0f75561efe9d7 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Fri, 2 Oct 2026 15:40:11 +0530 Subject: [PATCH 11/13] fix(pm): _toolchain hands callers the ordinary spelling of uv and Python The long-path commits now on onboarding/next spell PM's store root extended-length (\\?\) on Windows, and _toolchain returned uv and Python under that root. Its callers run and compare those paths outside PM's own file calls, and on main they got the ordinary path. tests/pm/test_uv_python.py::test_bundled_uv_uses_a_verified_writable_python_without_changing_runtime fails on both Windows lanes for that reason. Return native() spellings; the existence checks inside still use the store's. (cherry picked from commit acad756e3378ffbb3b23616bb0b5647e251fc895) --- pm/_uv.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pm/_uv.py b/pm/_uv.py index b70477463e..a5ec8429d0 100644 --- a/pm/_uv.py +++ b/pm/_uv.py @@ -4,7 +4,7 @@ from __future__ import annotations from pathlib import Path from pm import paths -from pm.filesystem import long_root +from pm.filesystem import long_root, native from pm.lock import Facts from pm.package import InstallError from pm.registry import get_package @@ -57,4 +57,5 @@ def _toolchain(*, realize: bool = True, explicit: bool = False) -> tuple[Path, P return None raise InstallError(name, "installed binary is missing", "run `hermes pm install`") binaries[name] = binary - return binaries["uv"], binaries["python"] + # Callers execute and compare these outside PM's own file calls: the ordinary spelling, not the store's. + return Path(native(binaries["uv"])), Path(native(binaries["python"])) From 1eb77a2154c8279540eebfaa829d2144d1dcebaf Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Wed, 7 Oct 2026 02:26:34 +0530 Subject: [PATCH 12/13] fix(pm): the long-paths live test runs the real bundled-copy install (review) test_long_paths_windows_live calls _install(copy_from=...) with facts.json inside each store instead of copying and publishing by hand. --- tests/pm/test_long_paths_windows_live.py | 40 +++++++++++------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/pm/test_long_paths_windows_live.py b/tests/pm/test_long_paths_windows_live.py index 7fc922d1c0..c915e33846 100644 --- a/tests/pm/test_long_paths_windows_live.py +++ b/tests/pm/test_long_paths_windows_live.py @@ -116,8 +116,8 @@ def test_store_lifecycle_beyond_max_path_with_long_paths_disabled(tmp_path): os.stat(_plain(probe)) # the host really cuts plain spellings at MAX_PATH shutil.rmtree(_verbatim(tmp_path) / ("p" * 120)) - from pm.install import _remove_entry - from pm.lock import Facts + from pm.install import _install, _remove_entry + from pm.lock import Facts, Lockfile from pm.package import Package, compose_env from pm.store import Store, current_target, tree_digest @@ -127,38 +127,36 @@ def test_store_lifecycle_beyond_max_path_with_long_paths_disabled(tmp_path): def binary(self, entry: Path, target: str) -> Path: return entry / "bin" / "tool.exe" - package, target, version = Tool(), current_target(), "1.0.0" + package, target, version, pin = Tool(), current_target(), "1.0.0", "0" * 64 entry_name = package.store_entry(version, target) source_root, store_root = tmp_path / "src", tmp_path / "dst" source, store = Store(source_root), Store(store_root) + lockfile = Lockfile(tmp_path / "lock.json") + lockfile.set_pin(package.name, version, {target: {"sha256": pin}}) files = _build_tree(source_root / entry_name, len(_plain(tmp_path / "dst" / entry_name))) expected = tree_digest(_verbatim(source_root / entry_name)) + # The bundled source as a sealed install records it: facts.json inside its store. + source_facts = Facts(source_root / "facts.json") + source_facts.record(package.name, version, entry_name, package.env(source.entry(entry_name), target), + source.root, target=target, artifacts=[pin], digest=expected) - with store.scratch() as scratch: - staged = scratch / "staged" - # The copy _copy_verified_source performs, then its digest check. - shutil.copytree(source.entry(entry_name), staged, symlinks=True) - assert tree_digest(staged) == expected - assert tree_digest(source.entry(entry_name)) == expected - _assert_tree(staged, files) - # Bytes scratch() must clean up itself (an abandoned extraction). - abandoned = scratch / "abandoned" - shutil.copytree(source.entry(entry_name), abandoned, symlinks=True) - published = store.publish(staged, entry_name) - assert not _verbatim(scratch).exists(), "scratch() left bytes behind beyond MAX_PATH" + # The real bundled-copy install: _copy_verified_source, verification, publish, facts commit. + facts = Facts(store_root / "facts.json") + published = _install(package, lockfile, facts, store, target, copy_from=(source_facts, source)) assert not [p for p in _verbatim(store_root).iterdir() if p.name.startswith(".staging-")] - _assert_tree(store_root / entry_name, files) assert tree_digest(published) == expected + # Bytes scratch() must clean up itself (an abandoned extraction). + with store.scratch() as scratch: + shutil.copytree(source.entry(entry_name), scratch / "abandoned", symlinks=True) + assert not _verbatim(scratch).exists(), "scratch() left bytes behind beyond MAX_PATH" + # Records and child environments are exits: they carry the ordinary spelling. - facts = Facts(tmp_path / "facts.json") - facts.record(package.name, version, entry_name, package.env(published, target), store.root, - target=target, artifacts=["0" * 64], digest=tree_digest(published)) - assert facts.installed(package.name, version, store.root, (target, ("0" * 64,))) - on_disk = json.loads((tmp_path / "facts.json").read_text(encoding="utf-8")) + assert facts.installed(package.name, version, store.root, (target, (pin,))) + on_disk = json.loads((store_root / "facts.json").read_text(encoding="utf-8")) assert not [s for s in _strings(on_disk) if VERBATIM in s] child_env = compose_env([facts.env_for(package.name, store.root)], base={}) assert not [v for v in child_env.values() if VERBATIM in v], child_env From ee8dd6c886326db4b468c2ba18d9409f123da827 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 7 Oct 2026 01:12:55 +0530 Subject: [PATCH 13/13] fix(agent): a credit-limited 402 retries once with the affordable output cap instead of failing as billing OpenRouter answers a request whose max_tokens exceeds the remaining balance with "402 ... You requested up to 65536 tokens, but can only afford 56272". The account has credit; only the cap was too large. The main loop classified it as terminal billing and failed the turn (or rotated off a credential that still had credit), so a request that needed a few hundred tokens was dropped (#49769). recover_after_classification now retries once with the output cap lowered to the affordable budget before pool rotation, reusing the auxiliary client's _affordable_max_tokens_from_error parse (same 64-token margin and 512-token floor) and the existing one-shot _ephemeral_max_output_tokens seam. A second credit-limited 402 on the same call, or a body with no parseable budget, keeps today's billing handling. Classification is unchanged. Approach from #49785 by @tcconnally (closed unmerged), applied at the recovery layer. Co-authored-by: Thomas Connally <51974392+tcconnally@users.noreply.github.com> --- agent/turn_recovery.py | 31 ++++++++++-- agent/turn_retry_state.py | 2 + tests/agent/test_affordable_402_main_loop.py | 53 ++++++++++++++++++++ 3 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 tests/agent/test_affordable_402_main_loop.py diff --git a/agent/turn_recovery.py b/agent/turn_recovery.py index 95442a89b8..201d08c737 100644 --- a/agent/turn_recovery.py +++ b/agent/turn_recovery.py @@ -640,6 +640,27 @@ def _recover_welcome_tier(agent: Any, classified: Any, _retry: TurnRetryState) - return False +def _clamp_to_affordable_budget(agent: Any, api_error: Exception, classified: Any, _retry: TurnRetryState) -> bool: + """A credit-limited 402 (OpenRouter: "...requested up to 65536 tokens, but can only afford + 56272") means the account has credit and the output cap was too large: retry ONCE with the + cap lowered to the affordable budget instead of abandoning the provider as billing (#49769). + Same budget parse as the auxiliary client's retry. The 402 itself says the request's cap + exceeded the budget (which may be a boosted continuation cap above ``agent.max_tokens``), + so a second credit-limited 402 on this call is genuine exhaustion.""" + if classified.reason != FailoverReason.billing or _retry.affordable_402_clamp_attempted: + return False + from agent.auxiliary_client import _affordable_max_tokens_from_error + + affordable = _affordable_max_tokens_from_error(api_error) + if affordable is None: + return False + _retry.affordable_402_clamp_attempted = True + agent._ephemeral_max_output_tokens = affordable + _vlines(agent, f"💳 Credit balance covers fewer output tokens — retrying with max_tokens={affordable:,}...") + logger.info("%sCredit-limited 402: retrying once with max_tokens=%d", agent.log_prefix, affordable) + return True + + def recover_after_classification( agent: Any, api_error: Exception, classified: Any, _retry: TurnRetryState, *, status_code: Optional[int], error_context: Any, messages: List[Dict[str, Any]], @@ -647,14 +668,18 @@ def recover_after_classification( ) -> Tuple[bool, bool]: """One-shot recovery chain that runs AFTER ``classify_api_error`` and before the generic retry path. Order is load-bearing (each branch may ``return`` early): - Nous paid-entitlement refresh → Codex stale-reasoning strip on 401 ``token_expired`` → - credential-pool rotation → image shrink → multimodal-tool-content strip → corrupt-image + welcome-tier repair → credit-limited 402 output-cap clamp → Nous paid-entitlement refresh → + Codex stale-reasoning strip on 401 ``token_expired`` → credential-pool rotation → image shrink → multimodal-tool-content strip → corrupt-image strip → Anthropic OAuth 1M-beta disable → per-provider 401 credential refresh → format-recovery strips. Returns ``(retry_now, recovered_with_pool)``; the latter feeds the Nous rate-limit guard.""" from agent.conversation_loop import _is_nous_inference_route - if _recover_welcome_tier(agent, classified, _retry): + # The credit-limited 402 clamp runs before pool rotation, which would bench a credential + # that still has credit. + if _recover_welcome_tier(agent, classified, _retry) or _clamp_to_affordable_budget( + agent, api_error, classified, _retry + ): return True, False # 401 ``token_expired`` while the transcript still carries ``codex_reasoning_items`` is a diff --git a/agent/turn_retry_state.py b/agent/turn_retry_state.py index 76752ae61c..e00297d095 100644 --- a/agent/turn_retry_state.py +++ b/agent/turn_retry_state.py @@ -42,6 +42,8 @@ class TurnRetryState: # Transport / rate-limit recovery primary_recovery_attempted: bool = False has_retried_429: bool = False + # Credit-limited 402 ("can only afford N tokens") already retried with a lowered output cap. + affordable_402_clamp_attempted: bool = False # Persistent 401/403 already escalated to the fallback chain once this attempt. auth_failover_attempted: bool = False # Post-exhaustion auto-recovery cycles spent on this API call (agent.auto_recovery_cycles caps it). diff --git a/tests/agent/test_affordable_402_main_loop.py b/tests/agent/test_affordable_402_main_loop.py new file mode 100644 index 0000000000..f8b50d6ec9 --- /dev/null +++ b/tests/agent/test_affordable_402_main_loop.py @@ -0,0 +1,53 @@ +"""Main-loop credit-limited 402: retry once with the affordable output cap (#49769). + +OpenRouter answers a request whose ``max_tokens`` exceeds the remaining balance with +``402 ... You requested up to 65536 tokens, but can only afford 56272``. The account has +credit; the cap was too large. A real ``AIAgent`` turn against a recording fake provider must +retry the same provider once with the cap lowered to the affordable budget (the auxiliary +client's ``_affordable_max_tokens_from_error`` parse) and answer — and a second 402 must +still end the turn as billing, without a retry loop. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from tests.e2e.core.history._helpers import NO_BACKGROUND_REVIEW, OFFLINE_CONFIG +from tests.fakes.fake_llm_provider import Error, FakeLLMServer, Text, write_hermes_home + +_CREDIT_LIMITED_402 = ( + "This request requires more credits, or fewer max_tokens. You requested up to 65536 tokens, " + "but can only afford 56272. To increase, visit https://openrouter.ai/settings/credits and add more credits" +) + + +def _turn(script): + from run_agent import AIAgent + + home = Path(os.environ["HERMES_HOME"]) + with FakeLLMServer(script) as srv: + write_hermes_home(home, srv.base_url, extra_config=OFFLINE_CONFIG + NO_BACKGROUND_REVIEW) + agent = AIAgent(provider="custom", base_url=srv.base_url, api_key="sk-fake", model="fake-model", + session_id="affordable-402", quiet_mode=True, platform="cli", skip_memory=True, + max_tokens=65536) + try: + result = agent.run_conversation("hi", conversation_history=[], task_id="t") + finally: + agent.close() + caps = [r.get("max_tokens") or r.get("max_completion_tokens") for r in srv.main_requests()] + return result, caps + + +@pytest.mark.parametrize("repeat_402", [False, True], ids=["retry-answers", "second-402-is-billing"]) +def test_credit_limited_402_retries_once_with_the_affordable_cap(repeat_402): + script = [Error(402, _CREDIT_LIMITED_402)] * (2 if repeat_402 else 1) + [Text("answered")] + result, caps = _turn(script) + + assert caps == [65536, 56272 - 64] + if repeat_402: + assert result.get("failed") and "Billing" in (result.get("final_response") or ""), result + else: + assert result.get("completed") and result.get("final_response") == "answered", result