From 955f36faf1330297f9e681d970027a577242d806 Mon Sep 17 00:00:00 2001 From: longcoding Date: Thu, 23 Jul 2026 16:54:57 +0800 Subject: [PATCH] fix(logging): isolate Windows process log files --- src/kimi_cli/app.py | 10 ++-- src/kimi_cli/cli/__init__.py | 4 +- src/kimi_cli/utils/logging.py | 14 +++++ tests/e2e/test_cli_error_output.py | 8 +-- tests/utils/test_logging.py | 85 ++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 tests/utils/test_logging.py diff --git a/src/kimi_cli/app.py b/src/kimi_cli/app.py index a3d4d0a87a..18bef3a451 100644 --- a/src/kimi_cli/app.py +++ b/src/kimi_cli/app.py @@ -22,7 +22,6 @@ from kimi_cli.constant import VERSION from kimi_cli.llm import augment_provider_with_env_vars, create_llm, model_display_name from kimi_cli.session import Session -from kimi_cli.share import get_share_dir from kimi_cli.soul import RunCancelled, run_soul from kimi_cli.soul.agent import Runtime, load_agent from kimi_cli.soul.context import Context @@ -30,7 +29,12 @@ from kimi_cli.soul.toolset import KimiToolset from kimi_cli.utils.aioqueue import QueueShutDown from kimi_cli.utils.envvar import get_env_bool -from kimi_cli.utils.logging import logger, open_original_stderr, redirect_stderr_to_logger +from kimi_cli.utils.logging import ( + get_log_file_path, + logger, + open_original_stderr, + redirect_stderr_to_logger, +) from kimi_cli.utils.path import shorten_home from kimi_cli.wire import Wire, WireUISide from kimi_cli.wire.types import ApprovalRequest, ApprovalResponse, ContentPart, WireMessage @@ -59,7 +63,7 @@ def enable_logging(debug: bool = False, *, redirect_stderr: bool = True) -> None if debug: logger.enable("kosong") logger.add( - get_share_dir() / "logs" / "kimi.log", + get_log_file_path(), # FIXME: configure level for different modules level="TRACE" if debug else "INFO", format=( diff --git a/src/kimi_cli/cli/__init__.py b/src/kimi_cli/cli/__init__.py index b9aafb7a87..9e97b9d11c 100644 --- a/src/kimi_cli/cli/__init__.py +++ b/src/kimi_cli/cli/__init__.py @@ -864,9 +864,9 @@ async def _pick_session() -> str: # In debug mode, show full traceback for quick diagnosis. _emit_fatal_error(traceback.format_exc()) else: - from kimi_cli.share import get_share_dir + from kimi_cli.utils.logging import get_log_file_path - log_path = get_share_dir() / "logs" / "kimi.log" + log_path = get_log_file_path() # In non-debug mode, print a concise error and point users to logs. _emit_fatal_error( f"{exc}\n" diff --git a/src/kimi_cli/utils/logging.py b/src/kimi_cli/utils/logging.py index f1fbfbadbd..ded7092276 100644 --- a/src/kimi_cli/utils/logging.py +++ b/src/kimi_cli/utils/logging.py @@ -7,9 +7,23 @@ import sys import threading from collections.abc import Iterator +from pathlib import Path from typing import IO from kimi_cli import logger +from kimi_cli.share import get_share_dir + + +def get_log_file_path() -> Path: + """Return a log path that is safe for concurrent Windows processes. + + Windows does not allow Loguru to rename a file for rotation while another + process has it open. Other platforms retain the existing shared log. + """ + logs_dir = get_share_dir() / "logs" + if sys.platform == "win32": + return logs_dir / f"kimi.{os.getpid()}.log" + return logs_dir / "kimi.log" class StderrRedirector: diff --git a/tests/e2e/test_cli_error_output.py b/tests/e2e/test_cli_error_output.py index 2de830c272..2e1414e9f8 100644 --- a/tests/e2e/test_cli_error_output.py +++ b/tests/e2e/test_cli_error_output.py @@ -3,6 +3,7 @@ from __future__ import annotations import os +import re import subprocess import sys from pathlib import Path @@ -35,17 +36,18 @@ def _run_kimi(args: list[str], *, share_dir: Path) -> subprocess.CompletedProces def _normalize_cli_error_output(text: str) -> str: """Normalize Rich/Click error boxes across platforms for snapshot tests.""" text = text.replace("\r\n", "\n") + text = re.sub(r"kimi\.\d+\.log", "kimi.log", text) lines: list[str] = [] in_box = False for line in text.splitlines(): - if line.startswith(("╭", "┌")) and "Error" in line: + if line.startswith(("╭", "┌", "+-")) and "Error" in line: in_box = True lines.append("Error:") continue - if in_box and line.startswith(("╰", "└")): + if in_box and line.startswith(("╰", "└", "+-")): in_box = False continue - if in_box and line.startswith(("│", "┃")) and line.endswith(("│", "┃")): + if in_box and line.startswith(("│", "┃", "|")) and line.endswith(("│", "┃", "|")): inner = line[1:-1].strip() if inner: lines.append(inner) diff --git a/tests/utils/test_logging.py b/tests/utils/test_logging.py new file mode 100644 index 0000000000..a29a77ffbf --- /dev/null +++ b/tests/utils/test_logging.py @@ -0,0 +1,85 @@ +from __future__ import annotations + +import os +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +from kimi_cli.utils import logging as logging_utils + + +def test_get_log_file_path_keeps_shared_log_off_windows(monkeypatch, tmp_path: Path): + monkeypatch.setenv("KIMI_SHARE_DIR", str(tmp_path)) + monkeypatch.setattr(logging_utils.sys, "platform", "linux") + + assert logging_utils.get_log_file_path() == tmp_path / "logs" / "kimi.log" + + +def test_get_log_file_path_uses_process_log_on_windows(monkeypatch, tmp_path: Path): + monkeypatch.setenv("KIMI_SHARE_DIR", str(tmp_path)) + monkeypatch.setattr(logging_utils.sys, "platform", "win32") + monkeypatch.setattr(logging_utils.os, "getpid", lambda: 12345) + + assert logging_utils.get_log_file_path() == tmp_path / "logs" / "kimi.12345.log" + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows file locking regression") +def test_windows_processes_can_rotate_logs_concurrently(tmp_path: Path): + """Exercise Loguru's real Windows file handles in separate processes.""" + ready = tmp_path / "ready" + release = tmp_path / "release" + env = os.environ.copy() + env["KIMI_SHARE_DIR"] = str(tmp_path) + + holder_code = """ +import os, time +from loguru import logger +from kimi_cli.utils.logging import get_log_file_path +logger.remove() +logger.add(get_log_file_path(), rotation='1 B') +logger.info('holder') +open(os.environ['READY'], 'w').close() +while not os.path.exists(os.environ['RELEASE']): + time.sleep(0.01) +""" + env["READY"] = str(ready) + env["RELEASE"] = str(release) + holder = subprocess.Popen( + [sys.executable, "-c", holder_code], + env=env, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + for _ in range(500): + if ready.exists(): + break + if holder.poll() is not None: + break + time.sleep(0.01) + assert ready.exists(), holder.communicate(timeout=1)[1] + + writer_code = """ +from loguru import logger +from kimi_cli.utils.logging import get_log_file_path +logger.remove() +logger.add(get_log_file_path(), rotation='1 B') +logger.info('writer') +""" + writer = subprocess.run( + [sys.executable, "-c", writer_code], + env=env, + capture_output=True, + text=True, + timeout=10, + ) + assert writer.returncode == 0 + assert "PermissionError" not in writer.stderr + assert len(list((tmp_path / "logs").glob("kimi.*.log"))) >= 2 + finally: + release.touch() + holder.communicate(timeout=10)