diff --git a/.github/workflows/windows-venv-e2e.yml b/.github/workflows/windows-venv-e2e.yml index 5ec4ab0892..2afe568129 100644 --- a/.github/workflows/windows-venv-e2e.yml +++ b/.github/workflows/windows-venv-e2e.yml @@ -80,3 +80,26 @@ 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. 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 + 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``. + if: ${{ !cancelled() && steps.long_paths_off.outcome == 'success' }} + 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/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/cron/scheduler.py b/cron/scheduler.py index 3268a250f2..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: @@ -2159,7 +2145,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 +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 (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, exc_info=True) try: _session_db.end_session(_final_cron_session_id, _end_reason) # The scheduler owns cron-session finalization. AIAgent.close() also @@ -2682,10 +2676,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..66cbbccf47 100644 --- a/cron/scheduler_detached_worker.py +++ b/cron/scheduler_detached_worker.py @@ -9,14 +9,30 @@ 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, + 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 +43,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/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/pm/_uv.py b/pm/_uv.py index 76ba629c6d..a5ec8429d0 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, native 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: @@ -55,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"])) diff --git a/pm/build_operations.py b/pm/build_operations.py index c468a4d481..6a4b5cc980 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 long_root, 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) @@ -163,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 = {}, [] @@ -187,7 +190,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/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/install.py b/pm/install.py index fe83afe86a..89b5c210d6 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 @@ -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 @@ -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): @@ -561,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() @@ -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/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/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..0f59caa1e1 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 long_root, 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 "" @@ -323,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) @@ -614,9 +618,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 +703,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 +1044,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/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 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/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 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", 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/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 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: 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") 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..c915e33846 --- /dev/null +++ b/tests/pm/test_long_paths_windows_live.py @@ -0,0 +1,167 @@ +"""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 _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 + + class Tool(Package): + name = "longpath-probe" + + def binary(self, entry: Path, target: str) -> Path: + return entry / "bin" / "tool.exe" + + 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) + + # 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. + 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 + 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" 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..c11f7d4575 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -400,14 +400,10 @@ 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 - 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) - with contextlib.suppress(Exception): - explicit_cwd = bool(raw_cwd) and (remote_cwd or os.path.isdir(os.path.abspath(os.path.expanduser(raw_cwd)))) + return _err(rid, 4008, "send a message first") + # 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: @@ -431,7 +427,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, 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"):