fix(desktop): preserve source freshness and console diagnostics

Remove metadata-only source caching so mapped edits and transient read failures cannot leave stale rebuild decisions. Keep directory pruning, lazy npm lookup, renderer manifests and the COM scheduler query. Separate stdout and stderr logging, decode bounded logical lines before redaction, and route desktop records to the GUI log. Add real-child, mapped-write and temporary-read-lock regressions. Verified by independent review, targeted Windows/Linux suites, renderer tests and packaged Windows manual checks. Existing Windows logging failures reproduce on the parent; no end-to-end startup percentage is claimed for this corrected version.
2026-09-16 23:13:20 -04:00
parent db63214c7b
commit 2094265047
6 changed files with 134 additions and 139 deletions
--- a/hermes_cli/desktop_console.py
+++ b/hermes_cli/desktop_console.py
@@ -4,6 +4,7 @@ import logging
 import os
 import sys
 import threading
+import time
 from contextlib import contextmanager

 logger = logging.getLogger("hermes_cli.desktop")
@@ -31,19 +32,39 @@ def desktop_console_output(*, source_mode: bool):
        yield {}
        return

-    import subprocess
-
-    read_fd, write_fd = os.pipe()
-
-    def drain():
+    def drain(read_fd, level):
        with os.fdopen(read_fd, "rb") as stream:
-            while data := stream.readline(8192):
-                logger.info("[desktop] %s", data.decode("utf-8", errors="replace").rstrip())
+            # Decode/redact complete records, not arbitrary pipe-read fragments.
+            # Discard an oversized line in full: even its prefix may be a secret.
+            limit = 128 * 1024
+            while data := stream.readline(limit + 1):
+                if len(data) > limit:
+                    while data and not data.endswith(b"\n"):
+                        data = stream.readline(8192)
+                    logger.log(level, "[desktop] [oversized line omitted]")
+                else:
+                    logger.log(level, "[desktop] %s", data.decode("utf-8", errors="replace").rstrip())

-    reader = threading.Thread(target=drain, name="desktop-console", daemon=True)
-    reader.start()
+    streams = {}
+    readers = []
    try:
-        yield {"stdout": write_fd, "stderr": subprocess.STDOUT}
+        # Keep diagnostics visible even when ordinary INFO output is disabled.
+        for name, level in (("stdout", logging.INFO), ("stderr", logging.ERROR)):
+            read_fd, write_fd = os.pipe()
+            streams[name] = write_fd
+            reader = threading.Thread(
+                target=drain, args=(read_fd, level), name=f"desktop-console-{name}", daemon=True,
+            )
+            try:
+                reader.start()
+            except BaseException:
+                os.close(read_fd)
+                raise
+            readers.append(reader)
+        yield streams
    finally:
-        os.close(write_fd)
-        reader.join(timeout=1)
+        for write_fd in streams.values():
+            os.close(write_fd)
+        deadline = time.monotonic() + 1
+        for reader in readers:
+            reader.join(timeout=max(0, deadline - time.monotonic()))
--- a/hermes_cli/desktop_source_cache.py
+++ b/hermes_cli/desktop_source_cache.py
@@ -1,111 +0,0 @@
-"""Reuse the desktop content hash while all source metadata is unchanged.
-
-This is a build-freshness cache, not an integrity/security check. Normal launches
-must not read thousands of source/test files (and trigger on-access AV scans).
-Changed metadata always falls back to the existing content hash, so checkout
-mtime churn alone still does not force a rebuild.
-"""
-
-import ctypes
-import contextlib
-import hashlib
-import json
-import os
-import tempfile
-from functools import lru_cache
-from pathlib import Path
-from typing import Callable
-
-
-@lru_cache(maxsize=1)
-def _windows_file_api():
-    from ctypes import wintypes
-
-    class FileBasicInfo(ctypes.Structure):
-        _fields_ = [(name, ctypes.c_longlong) for name in (
-            "creation", "access", "write", "change"
-        )] + [("attributes", wintypes.DWORD)]
-
-    kernel = ctypes.WinDLL("kernel32", use_last_error=True)
-    kernel.CreateFileW.argtypes = [wintypes.LPCWSTR, wintypes.DWORD, wintypes.DWORD,
-                                  ctypes.c_void_p, wintypes.DWORD, wintypes.DWORD, wintypes.HANDLE]
-    kernel.CreateFileW.restype = wintypes.HANDLE
-    kernel.GetFileInformationByHandleEx.argtypes = [wintypes.HANDLE, ctypes.c_int,
-                                                   ctypes.c_void_p, wintypes.DWORD]
-    kernel.GetFileInformationByHandleEx.restype = wintypes.BOOL
-    kernel.CloseHandle.argtypes = [wintypes.HANDLE]
-    kernel.CloseHandle.restype = wintypes.BOOL
-    return kernel, FileBasicInfo
-
-
-def _change_time(path: Path, st: os.stat_result) -> int:
-    if os.name != "nt":
-        return st.st_ctime_ns
-    # Windows stat().st_ctime is CREATION time on supported Python versions.
-    # FILE_BASIC_INFO.ChangeTime also catches same-size edits with restored mtime.
-    # Request metadata only: never FILE_READ_DATA or a read of source contents.
-    kernel, info_type = _windows_file_api()
-    handle = kernel.CreateFileW(str(path), 0x80, 7, None, 3, 0x02000000, None)
-    if handle == ctypes.c_void_p(-1).value:
-        raise ctypes.WinError(ctypes.get_last_error())
-    try:
-        info = info_type()
-        if not kernel.GetFileInformationByHandleEx(handle, 0, ctypes.byref(info), ctypes.sizeof(info)):
-            raise ctypes.WinError(ctypes.get_last_error())
-        return info.change
-    finally:
-        kernel.CloseHandle(handle)
-
-
-def _source_metadata(project_root: Path, tree_dir: Path) -> str:
-    from hermes_cli.main_web_build import _source_tree_files
-
-    digest = hashlib.sha256()
-    for path in _source_tree_files(project_root, tree_dir):
-        st = path.stat()
-        record = (str(path.relative_to(project_root)), st.st_dev, st.st_ino,
-                  st.st_size, st.st_mtime_ns, _change_time(path, st))
-        digest.update(json.dumps(record, ensure_ascii=True).encode())
-        digest.update(b"\0")
-    return digest.hexdigest()
-
-
-def cached_desktop_source_hash(project_root: Path, compute: Callable[[], str]) -> str:
-    from hermes_constants import get_hermes_home
-
-    tree_dir = project_root / "apps" / "desktop"
-    cache_file = get_hermes_home() / "desktop-source-cache.json"
-    scope = str(project_root.resolve())
-    try:
-        before = _source_metadata(project_root, tree_dir)
-    except OSError:
-        return compute()  # Metadata unavailable: keep the full content check.
-    try:
-        cache = json.loads(cache_file.read_text(encoding="utf-8"))
-        if (isinstance(cache, dict) and cache.get("version") == 1
-                and cache.get("root") == scope and cache.get("metadata") == before
-                and isinstance(cache.get("contentHash"), str) and len(cache["contentHash"]) == 64):
-            return cache["contentHash"]
-    except (OSError, ValueError):
-        pass
-
-    content_hash = compute()
-    temporary = None
-    try:
-        # Do not cache a digest if an editor/update changed inputs while we read.
-        if before != _source_metadata(project_root, tree_dir):
-            return content_hash
-        cache_file.parent.mkdir(parents=True, exist_ok=True)
-        with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8", dir=cache_file.parent,
-                                         prefix=".desktop-source-", delete=False) as stream:
-            temporary = Path(stream.name)
-            json.dump({"version": 1, "root": scope, "metadata": before,
-                       "contentHash": content_hash}, stream)
-        temporary.replace(cache_file)
-    except OSError:
-        pass  # Read-only cache locations must not prevent building/launching.
-    finally:
-        if temporary is not None:
-            with contextlib.suppress(OSError):
-                temporary.unlink(missing_ok=True)
-    return content_hash
--- a/hermes_cli/main_desktop.py
+++ b/hermes_cli/main_desktop.py
@@ -37,9 +37,7 @@ def _desktop_dist_exists(desktop_dir: Path) -> bool:

 def _compute_desktop_content_hash(project_root: Path) -> str:
    """SHA-256 of ``apps/desktop/`` (minus .gitignore matches) plus root workspace config."""
-    from hermes_cli.desktop_source_cache import cached_desktop_source_hash
-    return cached_desktop_source_hash(
-        project_root, lambda: _hash_source_tree(project_root, project_root / "apps" / "desktop"))
+    return _hash_source_tree(project_root, project_root / "apps" / "desktop")


 def _desktop_stamp_path() -> Path:
--- a/hermes_logging.py
+++ b/hermes_logging.py
@@ -165,7 +165,7 @@ COMPONENT_PREFIXES = {
    "tools": ("tools",),
    "cli": ("hermes_cli", "cli"),
    "cron": ("cron",),
-    "gui": ("hermes_cli.web_server", "hermes_cli.pty_bridge", "tui_gateway", "uvicorn"),
+    "gui": ("hermes_cli.web_server", "hermes_cli.pty_bridge", "hermes_cli.desktop", "tui_gateway", "uvicorn"),
 }


--- a/tests/hermes_cli/test_desktop_console.py
+++ b/tests/hermes_cli/test_desktop_console.py
@@ -30,6 +30,66 @@ def test_packaged_console_output_drains_both_streams(caplog, monkeypatch):
    assert any("diagnostic" in message for message in messages)


+@pytest.mark.windows_only
+@pytest.mark.parametrize("level", [logging.WARNING, logging.ERROR])
+def test_stderr_survives_logging_threshold(caplog, level):
+    caplog.set_level(level, logger="hermes_cli.desktop")
+    with desktop_console_output(source_mode=False) as streams:
+        result = subprocess.run(
+            [sys.executable, "-c", "import sys; print('ordinary-output'); "
+             "sys.stderr.write('fatal-diagnostic\\n'); sys.exit(7)"],
+            timeout=15, check=False, **streams,
+        )
+    assert result.returncode == 7
+    messages = [record.getMessage() for record in caplog.records]
+    assert any("fatal-diagnostic" in message for message in messages)
+    assert not any("ordinary-output" in message for message in messages)
+
+
+@pytest.mark.windows_only
+@pytest.mark.parametrize("stream", ["stdout", "stderr"])
+def test_complete_records_reach_redacted_logs(tmp_path, monkeypatch, caplog, stream):
+    import hermes_logging
+
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+    hermes_logging._reset_queued_handlers()
+    caplog.set_level(logging.INFO)
+    caplog.set_level(logging.INFO, logger="hermes_cli.desktop")
+    secret = "synthetic-boundary-credential"
+    payload = (
+        b" " * (8192 - len(b"API_KEY=")) + b"API_KEY=" + secret.encode() + b"\n"
+        + b"u" * 8191 + "\u20ac\n".encode()
+        + b"oversized-start API_KEY=" + secret.encode() + b"z" * 262144
+        + b"oversized-end\nrecovered\ntail " + "\u20ac".encode()
+    )
+    payload_path = tmp_path / "child-output.bin"
+    payload_path.write_bytes(payload)
+    try:
+        log_dir = hermes_logging.setup_logging(hermes_home=tmp_path, log_level="INFO", mode="gui")
+        with desktop_console_output(source_mode=False) as streams:
+            result = subprocess.run(
+                [sys.executable, "-c", "import pathlib, sys; "
+                 "data = pathlib.Path(sys.argv[1]).read_bytes(); "
+                 "out = getattr(sys, sys.argv[2]).buffer; "
+                 "[(out.write(data[i:i+997]), out.flush()) for i in range(0, len(data), 997)]",
+                 str(payload_path), stream],
+                timeout=15, check=False, **streams,
+            )
+        assert result.returncode == 0
+        hermes_logging.flush_log_queue()
+        text = (log_dir / "agent.log").read_text(encoding="utf-8")
+        assert secret not in text
+        assert "u" * 8191 + "\u20ac" in text
+        assert "\ufffd" not in text
+        assert "oversized-start" not in text and "oversized-end" not in text
+        assert "recovered" in text and "tail \u20ac" in text
+        assert "omitted" in text.lower()
+        gui_text = (log_dir / "gui.log").read_text(encoding="utf-8")
+        assert "recovered" in gui_text and secret not in gui_text
+    finally:
+        hermes_logging._reset_queued_handlers()
+
+
 def test_source_launch_keeps_interactive_streams():
    with desktop_console_output(source_mode=True) as streams:
        assert streams == {}
--- a/tests/hermes_cli/test_desktop_startup_cost.py
+++ b/tests/hermes_cli/test_desktop_startup_cost.py
@@ -1,7 +1,6 @@
 """Startup regressions exercised against real source trees on the current host."""

 import argparse
-import builtins
 import os
 import subprocess
 from pathlib import Path
@@ -25,28 +24,21 @@ def _tree(tmp_path):
    return root, app, source


-def test_unchanged_desktop_hash_does_not_reopen_sources(tmp_path):
-    root, app, source = _tree(tmp_path)
+def test_desktop_hash_prunes_ignored_build_directories(tmp_path):
+    root, app, _ = _tree(tmp_path)
    ignored = app / "dist"
    ignored.mkdir()
    (ignored / "bundle.js").write_text("built output")
    expected = desktop._compute_desktop_content_hash(root)
-    original_open = builtins.open
    original_scandir = os.scandir
-    reads = []
-
-    def track(file, *args, **kwargs):
-        if isinstance(file, (str, os.PathLike)) and Path(file) == source:
-            reads.append(file)
-        return original_open(file, *args, **kwargs)

    def scan(directory):
        assert Path(directory) != ignored, "ignored build output must be pruned before traversal"
        return original_scandir(directory)

-    with patch("builtins.open", side_effect=track), patch("os.scandir", side_effect=scan):
+    (ignored / "bundle.js").write_text("different build output")
+    with patch("os.scandir", side_effect=scan):
        assert desktop._compute_desktop_content_hash(root) == expected
-    assert reads == [], "launch should use source metadata, not read every source file again"


 def test_desktop_hash_invalidates_for_edits_even_with_restored_mtime(tmp_path):
@@ -67,6 +59,41 @@ def test_desktop_hash_invalidates_for_edits_even_with_restored_mtime(tmp_path):
    assert desktop._compute_desktop_content_hash(root) != original


+@pytest.mark.windows_only
+def test_desktop_hash_detects_memory_mapped_edits(tmp_path):
+    import mmap
+
+    root, _, source = _tree(tmp_path)
+    original = desktop._compute_desktop_content_hash(root)
+    with source.open("r+b") as stream:
+        with mmap.mmap(stream.fileno(), 0, access=mmap.ACCESS_WRITE) as view:
+            view[-1:] = b"2"
+            view.flush()
+    assert source.read_bytes().endswith(b"2")
+    assert desktop._compute_desktop_content_hash(root) != original
+
+
+@pytest.mark.windows_only
+def test_desktop_hash_recovers_after_a_temporary_read_lock(tmp_path):
+    import msvcrt
+
+    root, _, source = _tree(tmp_path)
+    source.write_bytes(b"")
+    original = desktop._compute_desktop_content_hash(root)
+    payload = b"export const x = 2"
+    source.write_bytes(payload)
+    with source.open("r+b") as locked:
+        msvcrt.locking(locked.fileno(), msvcrt.LK_NBLCK, len(payload))
+        try:
+            with pytest.raises(OSError):
+                source.read_bytes()
+            desktop._compute_desktop_content_hash(root)
+        finally:
+            locked.seek(0)
+            msvcrt.locking(locked.fileno(), msvcrt.LK_UNLCK, len(payload))
+    assert desktop._compute_desktop_content_hash(root) != original
+
+
 def test_current_packaged_launch_does_not_require_npm(tmp_path, monkeypatch):
    root, app, _ = _tree(tmp_path)
    monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)