From b641639e425bfd26dbe3edbd113d8749384cbf40 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 22 Apr 2026 15:22:33 -0500 Subject: [PATCH] fix(debug): distinguish empty-log from missing-log in report placeholder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot on #14138 flagged that the share report says '(file not found)' when the log exists but is empty (either because the primary is empty and no .1 rotation exists, or in the rare race where the file is truncated between _resolve_log_path() and stat()). - Split _primary_log_path() out of _resolve_log_path so both can share the LOG_FILES/home math without duplication. - _capture_log_snapshot now reports '(file empty)' when the primary path exists on disk with zero bytes, and keeps '(file not found)' for the truly-missing case. Tests: rename test_returns_none_for_empty → test_empty_primary_reports_file_empty with the new assertion, plus a race-path test that monkeypatches _resolve_log_path to exercise the size==0 branch directly. --- hermes_cli/debug.py | 32 ++++++++++++++++++++------------ tests/hermes_cli/test_debug.py | 19 ++++++++++++++++--- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/hermes_cli/debug.py b/hermes_cli/debug.py index 2d7fbd2071..d5947be822 100644 --- a/hermes_cli/debug.py +++ b/hermes_cli/debug.py @@ -332,24 +332,29 @@ class LogSnapshot: tail_text: str full_text: Optional[str] -def _resolve_log_path(log_name: str) -> Optional[Path]: - """Find the log file for *log_name*, falling back to the .1 rotation. - - Returns the path if found, or None. - """ +def _primary_log_path(log_name: str) -> Optional[Path]: + """Where *log_name* would live if present. Doesn't check existence.""" from hermes_cli.logs import LOG_FILES filename = LOG_FILES.get(log_name) - if not filename: + return (get_hermes_home() / "logs" / filename) if filename else None + + +def _resolve_log_path(log_name: str) -> Optional[Path]: + """Find the log file for *log_name*, falling back to the .1 rotation. + + Returns the first non-empty candidate (primary, then .1), or None. + Callers distinguish 'empty primary' from 'truly missing' via + :func:`_primary_log_path`. + """ + primary = _primary_log_path(log_name) + if primary is None: return None - log_dir = get_hermes_home() / "logs" - primary = log_dir / filename if primary.exists() and primary.stat().st_size > 0: return primary - # Fall back to the most recent rotated file (.1). - rotated = log_dir / f"{filename}.1" + rotated = primary.parent / f"{primary.name}.1" if rotated.exists() and rotated.stat().st_size > 0: return rotated @@ -370,12 +375,15 @@ def _capture_log_snapshot( """ log_path = _resolve_log_path(log_name) if log_path is None: - return LogSnapshot(path=None, tail_text="(file not found)", full_text=None) + primary = _primary_log_path(log_name) + tail = "(file empty)" if primary and primary.exists() else "(file not found)" + return LogSnapshot(path=None, tail_text=tail, full_text=None) try: size = log_path.stat().st_size if size == 0: - return LogSnapshot(path=log_path, tail_text="(file not found)", full_text=None) + # race: file was truncated between _resolve_log_path and stat + return LogSnapshot(path=log_path, tail_text="(file empty)", full_text=None) with open(log_path, "rb") as f: if size <= max_bytes: diff --git a/tests/hermes_cli/test_debug.py b/tests/hermes_cli/test_debug.py index 91795151bf..4bba56867e 100644 --- a/tests/hermes_cli/test_debug.py +++ b/tests/hermes_cli/test_debug.py @@ -158,14 +158,27 @@ class TestCaptureLogSnapshot: assert snap.full_text is None assert snap.tail_text == "(file not found)" - def test_returns_none_for_empty(self, hermes_home): - # Truncate agent.log to empty + def test_empty_primary_reports_file_empty(self, hermes_home): + """Empty primary (no .1 fallback) surfaces as '(file empty)', not missing.""" (hermes_home / "logs" / "agent.log").write_text("") from hermes_cli.debug import _capture_log_snapshot snap = _capture_log_snapshot("agent", tail_lines=10) assert snap.full_text is None - assert snap.tail_text == "(file not found)" + assert snap.tail_text == "(file empty)" + + def test_race_truncate_after_resolve_reports_empty(self, hermes_home, monkeypatch): + """If the log is truncated between resolve and stat, say 'empty', not 'missing'.""" + log_path = hermes_home / "logs" / "agent.log" + from hermes_cli import debug + + monkeypatch.setattr(debug, "_resolve_log_path", lambda _name: log_path) + log_path.write_text("") + + snap = debug._capture_log_snapshot("agent", tail_lines=10) + assert snap.path == log_path + assert snap.full_text is None + assert snap.tail_text == "(file empty)" def test_truncates_large_file(self, hermes_home): """Files larger than max_bytes get tail-truncated."""