fix(system): survive a rollover racing the read, and stop trusting the scan

Three review findings, all applied.

Greptile P1, read race: a rollover can rename a candidate between the
existence check and the open, and the handler exposes no lock a route can
take. Per-file OSError now skips that file instead of 500ing the whole panel
-- which is what the single-file version did in the same situation, so this is
strictly better than before rather than a new guarantee. A roll landing
mid-walk can still shift which chunk a file holds, so a tail taken at that
instant may repeat or miss a block; the panel re-polls every 5s and the next
read is clean. Buying strict consistency would mean reaching into logging's
internals from a route.

Greptile P1, clear race: enumerating first left a window where a rollover
created a backup after the scan and its history survived a Clear that
reported success. Clear now works off the fixed name set -- every name the
handler can write is known up front, so there is nothing to enumerate and no
snapshot to go stale.

CodeRabbit: the CHANGELOG lines ended in (#1782), which reads as "this fixes
#1782" when the desktop path defect that thread is about is untouched. Now
(#1920).

Two tests added, both red before: a candidate vanishing mid-walk still fills
the request from the next file, and a Clear whose scan reported nothing still
empties the backups.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Chang-Jin-Lee <ckdwls525@gmail.com>

CHANGELOG.md

@@ -50,8 +50,8 @@ the frozen-backend fallback mirror it for their toolchains.
 
 ### Fixed
 
-- The Backend log tab keeps showing history across a log rollover, instead of going nearly empty until new lines arrive (#1782)
-- Clearing the logs now empties the rotated log files too, so it frees the space it appears to (#1782)
+- The Backend log tab keeps showing history across a log rollover, instead of going nearly empty until new lines arrive (#1920)
+- Clearing the logs now empties the rotated log files too, so it frees the space it appears to (#1920)
 - Install documentation help now prints correctly on Windows consoles using legacy encodings (#1815) — thanks @dajiaohuang!
 - Saved transcriptions with missing or invalid timestamps now remain readable (#1799) — thanks @yunaremaia and @tvbht!
 - Copying a saved transcription now uses the shared clipboard helper and reports failed copies accurately (#1803) — thanks @tvbht!

backend/api/routers/system.py

@@ -330,7 +330,20 @@ def _tail_rolling(base: str, tail: int):
     for path in candidates:
         if remaining <= 0:
             break
-        lines, count = _tail_file(path, remaining)
+        try:
+            lines, count = _tail_file(path, remaining)
+        except OSError:
+            # A rollover can rename a candidate between the existence check
+            # above and this open, and the handler holds no lock we can take
+            # from a route. Skip the file rather than 500 the whole panel over
+            # one member of the set — the previous single-file version failed
+            # the request outright in the same situation.
+            #
+            # A roll landing mid-walk can also shift which chunk a file holds,
+            # so a tail taken at that instant may repeat or miss a block. The
+            # panel re-polls every 5s and the next read is clean; buying strict
+            # consistency here would mean reaching into logging's internals.
+            continue
         if count == 0:
             continue
         chunks.append(lines)
@@ -533,7 +546,15 @@ async def clear_system_logs():
     reaches into those files — would have looked like it did nothing at all.
     """
     cleared_any = False
-    targets = [LOG_PATH, *_rotated_log_paths(LOG_PATH), CRASH_LOG_PATH]
+    # The full fixed name set rather than a snapshot of what exists: enumerating
+    # first leaves a window where a rollover creates a backup after the scan and
+    # its history survives a Clear that reported success. Names the handler can
+    # ever write are known up front, so there is nothing to enumerate.
+    targets = [
+        LOG_PATH,
+        *(f"{LOG_PATH}.{i}" for i in range(1, _LOG_BACKUP_COUNT + 1)),
+        CRASH_LOG_PATH,
+    ]
     for p in targets:
         if os.path.exists(p):
             try:

tests/backend/api/routers/test_system_logs_rotation.py

@@ -102,6 +102,50 @@ def test_no_log_at_all_still_reports_absent(system_mod, tmp_path, monkeypatch):
     assert res == {"lines": [], "path": str(tmp_path / "omnivoice.log"), "exists": False}
 
 
+def test_a_file_that_vanishes_mid_walk_does_not_fail_the_panel(
+    system_mod, rolling, monkeypatch
+):
+    """A rollover can rename a candidate between the scan and the open.
+
+    The handler exposes no lock a route can take, so the walk skips the file
+    instead of failing the request. The single-file version 500'd the whole
+    panel in the same situation, so this is strictly better than before.
+    """
+    real_tail = system_mod._tail_file
+
+    def flaky(path, tail):
+        if path.endswith("omnivoice.log.1"):
+            raise FileNotFoundError(path)
+        return real_tail(path, tail)
+
+    monkeypatch.setattr(system_mod, "_tail_file", flaky)
+
+    res = asyncio.run(system_mod.system_logs(tail=200))
+
+    # .1 is gone, so the walk falls through to .2 and still fills the request.
+    assert res["exists"] is True
+    assert len(res["lines"]) == 200
+    assert [os.path.basename(p) for p in res["paths"]] == ["omnivoice.log.2", "omnivoice.log"]
+
+
+def test_clear_covers_a_backup_created_after_the_scan(system_mod, rolling, monkeypatch):
+    """Clear works off the fixed name set, not a snapshot of what exists.
+
+    Enumerating first left a window where a rollover created a backup after the
+    scan and its history survived a Clear that reported success.
+    """
+    monkeypatch.setattr(system_mod, "prefs_delete", lambda _key: None, raising=False)
+    # Stand in for the race: a scan that ran before the rollover would have
+    # reported no backups at all, and the version that trusted it truncated
+    # only the current file while .1 and .2 kept their history.
+    monkeypatch.setattr(system_mod, "_rotated_log_paths", lambda _base: [])
+
+    asyncio.run(system_mod.clear_system_logs())
+
+    for name in ("omnivoice.log", "omnivoice.log.1", "omnivoice.log.2"):
+        assert (rolling / name).stat().st_size == 0, f"{name} survived a Clear that trusted a stale scan"
+
+
 def test_clear_empties_the_backups_too(system_mod, rolling, monkeypatch):
     monkeypatch.setattr(system_mod, "prefs_delete", lambda _key: None, raising=False)