fix(cli,mcp): UTF-8 stderr, a stdout guard, and OSW_CRED_FILEPATH paths

docs/tools/configuration.md

@@ -103,7 +103,7 @@ set wins:
 | `OSW_DOMAIN` | `OSL_DOMAIN` | Instance to connect to. A bare host (`wiki.example.org`) or a full URL (`https://wiki.example.org/w/`); the host is taken from either, and a value no host can be read from (`https://`, `/w/index.php`) is rejected at startup |
 | `OSW_USERNAME` | `OSL_USERNAME` | Login user |
 | `OSW_PASSWORD` | `OSL_PASSWORD` | Login password |
-| `OSW_CRED_FILEPATH` | `OSW_MCP_CRED_FILEPATH`, `OSL_CRED_FILEPATH` | YAML credential file, keyed by iri (falls back to `accounts.pwd.yaml` in the working directory, CLI only) |
+| `OSW_CRED_FILEPATH` | `OSW_MCP_CRED_FILEPATH`, `OSL_CRED_FILEPATH` | YAML credential file, keyed by iri (falls back to `accounts.pwd.yaml` in the working directory, CLI only). A leading `~` is expanded. A relative path is accepted; the MCP server resolves it at startup against the working directory its client chose, and the source report shows the full path |
 | `OSW_ENV_FILE` | `OSW_MCP_ENV_FILE` | `.env` file to load |
 | `OSW_READ_ONLY` | `OSW_MCP_READ_ONLY` | `true` refuses every write |
 | `OSW_SPARQL_ENDPOINT` | | Endpoint for `sparql` queries |

src/osw/cli/main.py

@@ -30,6 +30,7 @@
 from osw.service.errors import OpError
 from osw.service.params import json_value
 from osw.service.registry import Operation, bind, iter_operations
+from osw.service.streams import force_utf8
 from osw.wtsite import SLOTS
 
 from .render import render
@@ -40,14 +41,8 @@
 def _force_utf8_output() -> None:
     """Encode stdout and stderr as UTF-8, whatever the locale asks for.
 
-    Python encodes a redirected stream with the locale encoding, which on a
-    German Windows system is cp1252. A non-ASCII label then reaches the
-    consumer as bytes no JSON parser can read, and a character cp1252 has no
-    code point for -- Japanese, Greek, Cyrillic -- raises UnicodeEncodeError
-    and ends the command. A Windows console stream is UTF-8 already, so on
-    Windows only redirected output changes. Elsewhere a terminal uses the
-    locale encoding, so this overrides a deliberate non-UTF-8 LANG or
-    PYTHONIOENCODING too. stderr is covered as well as stdout, because
+    The mechanism lives in :func:`osw.service.streams.force_utf8`, which the
+    osw-mcp server uses as well. stderr is covered as well as stdout, because
     ``Context.guard`` sends captured stdout to stderr under ``--json``.
 
     Called from the app callback, so it covers every command. Click prints
@@ -60,16 +55,7 @@ def _force_utf8_output() -> None:
     after the command is fine, because click resolves the command, runs this
     callback, and only then parses the command's own arguments.
     """
-    for stream in (sys.stdout, sys.stderr):
-        reconfigure = getattr(stream, "reconfigure", None)
-        errors = getattr(stream, "errors", None)
-        # A stream a test harness or host application substituted may have
-        # neither, and then decides its own encoding. Both are required:
-        # errors= must be passed, because reconfigure() silently resets the
-        # handler to strict otherwise, which would let stderr raise while
-        # reporting a failure. Passing errors=None does exactly that too.
-        if reconfigure is not None and errors is not None:
-            reconfigure(encoding="utf-8", errors=errors)
+    force_utf8(sys.stdout, sys.stderr)
 
 
 @app.callback()

src/osw/mcp/server.py

@@ -22,6 +22,7 @@
 from osw.service.config import Settings
 from osw.service.context import Context, Policy
 from osw.service.registry import Operation, bind, iter_operations
+from osw.service.streams import force_utf8
 
 INSTRUCTIONS = """\
 This server is pinned to exactly one OpenSemanticLab (OSL) instance for its
@@ -156,6 +157,18 @@ def create_server() -> MCPServer:
 
 def main() -> None:
     """Console-script entry point: build the server and serve over stdio."""
+    # Before any write below. An MCP client starts this server with stderr on
+    # a pipe, so Python encodes it with the locale encoding, cp1252 on a
+    # German Windows system. The report holds the credential file path and the
+    # env file path, so a directory named "Muller" with an umlaut is enough to
+    # reach the client's log mangled. Reconfiguring in place also covers osw's
+    # own log handler, which holds this same stream object.
+    #
+    # stdout is deliberately left alone. The SDK's stdio_server re-wraps the
+    # binary buffer as UTF-8 itself, and claims file descriptor 1 while doing
+    # it, so the JSON-RPC channel does not depend on this and changing it here
+    # would only add a way to interfere.
+    force_utf8(sys.stderr)
     # See _build_server for why this is set here too.
     config.set_log_prefix("osw-mcp")
     report = io.StringIO()

src/osw/service/config.py

@@ -176,6 +176,21 @@ def _validate_cred_filepath(cls, value: Optional[str]) -> Optional[str]:
             return value
         if not value.strip():
             raise ValueError("must not be empty or whitespace-only")
+        # Same rewrite as _validate_state_dir, for the same reason: load()
+        # checks Path(cred_filepath).is_file() and _cred_file_iris() opens the
+        # path, and neither expands a leading ~, so "~/accounts.pwd.yaml" was
+        # reported as missing while the file was there. A relative path is
+        # still accepted, unlike for state_dir: the CLI resolves
+        # "accounts.pwd.yaml" against the working directory on purpose. For the
+        # MCP server, _resolve_cred_file has already made it absolute.
+        if value.startswith("~"):
+            try:
+                value = str(Path(value).expanduser())
+            except RuntimeError as exc:
+                raise ValueError(
+                    f"starts with '~' but the home directory cannot be "
+                    f"determined ({exc})"
+                ) from exc
         return value
 
     def redacted(self) -> dict:
@@ -288,6 +303,10 @@ def _verify_cred_file_has_domain(cred_filepath: str, domain: str) -> None:
 _cred_file_path: Optional[str] = None
 _cred_file_origin: str = "not searched"
 _cred_file_var: Optional[str] = None
+# The relative value _resolve_cred_file() made absolute, or None. Only for
+# load()'s "does not exist" message, which otherwise shows a directory the
+# user never typed.
+_cred_file_relative: Optional[str] = None
 
 # The adapter name every "[name] ..." message this module (and the rest of
 # osw.service) prints. "osw" is the default, covering a process that embeds
@@ -328,7 +347,9 @@ def set_env_file_discovery(enabled: bool) -> None:
     depend on the working directory the process happens to run in, so both
     are gated by the same flag: the CLI's working directory is the one the
     user typed the command in, while the MCP server's is chosen by the MCP
-    client, which it does not control.
+    client, which it does not control. For the same reason, a relative
+    ``OSW_CRED_FILEPATH`` is made absolute while discovery is disabled, see
+    :func:`_resolve_cred_file`.
 
     Must be called before settings are first loaded, since the file is read
     exactly once per process; a call that would *change* the setting after
@@ -406,8 +427,10 @@ def _resolve_cred_file() -> Optional[str]:
     1. An explicitly configured ``OSW_CRED_FILEPATH`` (or its
        ``OSW_MCP_CRED_FILEPATH`` / ``OSL_CRED_FILEPATH`` aliases) always wins,
        whether it came from the real environment or from a ``.env`` file.
-       Existence is not checked here; ``load()`` already reports a missing
-       configured file with a specific error message.
+       A leading ``~`` is expanded. While implicit discovery is disabled (the
+       MCP server), a relative path is made absolute against the working
+       directory. Existence is not checked here; ``load()`` checks the
+       returned path and reports a missing file with a specific error message.
     2. Otherwise, if implicit discovery is disabled (the MCP server; see
        :func:`set_env_file_discovery`), nothing is resolved: origin
        ``"not searched"``.
@@ -435,7 +458,8 @@ def _resolve_cred_file() -> Optional[str]:
     username/password already configured. Safe to call more than once, like
     :func:`_load_env_file`, which this assumes has already run.
     """
-    global _cred_file_path, _cred_file_origin, _cred_file_var
+    global _cred_file_path, _cred_file_origin, _cred_file_var, _cred_file_relative
+    _cred_file_relative = None
     path = _first_env(ENV_CRED_FILEPATH)
     if path:
         _cred_file_var = next(
@@ -444,6 +468,29 @@ def _resolve_cred_file() -> Optional[str]:
         _cred_file_origin = (
             "env file" if _cred_file_var in _env_file_supplied else "environment"
         )
+        # Expanded here, not only in Settings._validate_cred_filepath, because
+        # load() calls Path(cred_filepath).is_file() on this return value long
+        # before it constructs Settings. Determined after _cred_file_var above,
+        # which matches the raw value against os.getenv.
+        if path.startswith("~"):
+            try:
+                path = str(Path(path).expanduser())
+            except RuntimeError as exc:
+                raise RuntimeError(
+                    f"{_cred_file_var} starts with '~' but the home directory "
+                    f"cannot be determined ({exc}). Set it to a full path."
+                ) from exc
+        # With discovery disabled, the working directory is the one the MCP
+        # client chose, so a relative path names a file the user cannot predict.
+        # It is resolved once, here, so that load()'s existence check, the
+        # source report and the stored Settings all name the same full path,
+        # and no later read depends on the working directory. The CLI keeps
+        # the relative value: its working directory is the one the user typed
+        # the command in. os.path.abspath rather than Path.resolve(), which
+        # would also replace a symlink with its target.
+        if not _discover_env_file and not Path(path).is_absolute():
+            _cred_file_relative = path
+            path = os.path.abspath(path)
         _cred_file_path = path
         return path
     if not _discover_env_file:
@@ -618,9 +665,11 @@ def load(strict: bool = True) -> Settings:
         If domain is missing and no usable credential file is configured, if
         neither a usable credential file nor username/password are
         configured (only when ``strict`` is ``True``), if a configured
-        credential file does not exist, or if a configured credential file has
-        no entry matching a configured domain. This keeps the osw interactive
-        credential prompt from ever being reached.
+        credential file does not exist, if a configured credential file has
+        no entry matching a configured domain, or if a configured credential
+        file path starts with ``~`` and no home directory can be determined.
+        This keeps the osw interactive credential prompt from ever being
+        reached.
     """
     _load_env_file()
 
@@ -632,9 +681,17 @@ def load(strict: bool = True) -> Settings:
     cred_file_usable = False
     if cred_filepath:
         if not Path(cred_filepath).is_file():
+            relative_hint = (
+                f"{_cred_file_var} is the relative path '{_cred_file_relative}', "
+                "resolved against the working directory, which the MCP client "
+                "chooses. "
+                if _cred_file_relative
+                else ""
+            )
             raise RuntimeError(
                 f"Configured credential file '{cred_filepath}' does not exist. "
-                "Set OSW_CRED_FILEPATH (or its OSW_MCP_CRED_FILEPATH / "
+                + relative_hint
+                + "Set OSW_CRED_FILEPATH (or its OSW_MCP_CRED_FILEPATH / "
                 "OSL_CRED_FILEPATH aliases) to a valid path, or remove it and "
                 "configure OSW_USERNAME/OSW_PASSWORD instead."
                 + _escape_hint(cred_filepath)
@@ -744,7 +801,7 @@ def reset() -> None:
     """Drop cached settings and the active-instance selection (used by tests)."""
     global _settings, _active_iri, _active_resolved
     global _discover_env_file, _env_file_path, _env_file_origin, _env_file_supplied
-    global _cred_file_path, _cred_file_origin, _cred_file_var
+    global _cred_file_path, _cred_file_origin, _cred_file_var, _cred_file_relative
     _settings = None
     _active_iri = None
     _active_resolved = False
@@ -755,6 +812,7 @@ def reset() -> None:
     _cred_file_path = None
     _cred_file_origin = "not searched"
     _cred_file_var = None
+    _cred_file_relative = None
 
 
 # -- active-instance state ---------------------------------------------------

src/osw/service/streams.py

@@ -0,0 +1,39 @@
+"""Output stream setup shared by the osw CLI and the osw-mcp server.
+
+Neither adapter may import the other, and both write text that the locale
+encoding may not be able to represent. The single helper here is what they
+share; which streams to apply it to is the caller's decision, because the two
+adapters differ there. See :func:`force_utf8`.
+"""
+
+from __future__ import annotations
+
+from typing import TextIO
+
+
+def force_utf8(*streams: TextIO) -> None:
+    """Encode each given stream as UTF-8, whatever the locale asks for.
+
+    Python encodes a redirected stream with the locale encoding, which on a
+    German Windows system is cp1252. A non-ASCII label then reaches the
+    consumer as bytes no JSON parser can read, and a character cp1252 has no
+    code point for -- Japanese, Greek, Cyrillic -- raises UnicodeEncodeError
+    and ends the command. A Windows console stream is UTF-8 already, so on
+    Windows only redirected output changes. Elsewhere a terminal uses the
+    locale encoding, so this overrides a deliberate non-UTF-8 LANG or
+    PYTHONIOENCODING too.
+
+    Each stream is reconfigured in place. A ``logging.StreamHandler`` built
+    earlier holds the stream object itself, not a name, so it writes UTF-8
+    from here on as well.
+    """
+    for stream in streams:
+        reconfigure = getattr(stream, "reconfigure", None)
+        errors = getattr(stream, "errors", None)
+        # A stream a test harness or host application substituted may have
+        # neither, and then decides its own encoding. Both are required:
+        # errors= must be passed, because reconfigure() silently resets the
+        # handler to strict otherwise, which would let stderr raise while
+        # reporting a failure. Passing errors=None does exactly that too.
+        if reconfigure is not None and errors is not None:
+            reconfigure(encoding="utf-8", errors=errors)

tests/test_mcp_server.py

@@ -11,12 +11,17 @@
 
 import asyncio
 import io
+import logging
+import sys
+from contextlib import contextmanager
 
 import pytest
 import yaml
 
+import osw
 from osw.mcp import server
 from osw.service import config
+from osw.service.context import Context
 from osw.service.registry import iter_operations
 
 _ALL_VARS = [
@@ -237,3 +242,97 @@ def test_main_prints_the_report_when_startup_fails(monkeypatch, tmp_path, capsys
     err = capsys.readouterr().err
     assert "[osw-mcp] " in err
     assert "failed to start" in err
+
+
+def test_main_forces_utf8_on_stderr_and_leaves_stdout_alone(monkeypatch):
+    """An MCP client puts stderr on a pipe, so Python picks the locale encoding.
+
+    The startup report carries the credential file path and the env file path
+    (src/osw/service/config.py), so a directory or account name outside ASCII
+    reaches the client's log mangled.
+
+    stdout is left alone on purpose. The SDK's ``stdio_server`` re-wraps the
+    binary buffer as UTF-8 itself and claims file descriptor 1 while doing it,
+    so the JSON-RPC channel does not depend on this.
+    """
+    _configure(monkeypatch)
+    _serve_without_blocking(monkeypatch)
+    out = io.TextIOWrapper(io.BytesIO(), encoding="cp1252", errors="strict")
+    err = io.TextIOWrapper(io.BytesIO(), encoding="cp1252", errors="backslashreplace")
+    monkeypatch.setattr(sys, "stdout", out)
+    monkeypatch.setattr(sys, "stderr", err)
+
+    server.main()
+
+    assert err.encoding == "utf-8"
+    # reconfigure() resets errors to strict unless it is passed as well, and a
+    # strict stderr would raise while reporting a failure.
+    assert err.errors == "backslashreplace"
+    assert out.encoding == "cp1252"
+
+
+@contextmanager
+def _osw_logging_on_the_captured_stream():
+    """osw's own handler, writing to the stream pytest has in place right now.
+
+    Two resets are needed. ``enable_logging`` resolves ``sys.stderr`` once,
+    when it builds the handler (src/osw/__init__.py:149), so the handler
+    attached when conftest imported osw still holds the stderr from before
+    capsys replaced it. And that handler steps aside as soon as an ancestor
+    logger has a handler of its own (src/osw/__init__.py:84-88), which
+    pytest's log capture puts on the root logger.
+
+    A context manager rather than a fixture, because pytest attaches those
+    root handlers after the fixtures have run. Mirrors ``osw_logger`` and
+    ``plain_logging`` in tests/test_logging_setup.py.
+    """
+    root, osw_logger = logging.getLogger(), logging.getLogger("osw")
+    saved_root = root.handlers[:]
+    saved = (osw_logger.handlers[:], osw_logger.level, osw._level_is_ours)
+    root.handlers = []
+    try:
+        osw.enable_logging()
+        yield
+    finally:
+        root.handlers = saved_root
+        osw_logger.handlers, osw._level_is_ours = saved[0], saved[2]
+        osw_logger.setLevel(saved[1])
+
+
+def _no_connection(self, iri):
+    raise RuntimeError("offline test: no connection is made")
+
+
+def test_a_log_record_during_a_tool_call_never_reaches_stdout(
+    monkeypatch, tmp_path, capsys
+):
+    """stdout is the JSON-RPC channel, so one log line there breaks the client.
+
+    Two mechanisms keep it clean and only one of them is osw's own code:
+    ``enable_logging`` defaults its handler to ``sys.stderr``, and the MCP SDK
+    claims file descriptor 1 for the wire. A single edit to that default would
+    undo the first, which is what this holds.
+
+    The status operation is used because it logs a warning from inside
+    ``ctx.guard()`` when the connection check fails
+    (src/osw/service/ops/status.py:63). ``guard()`` rebinds ``sys.stdout`` to
+    ``sys.stderr`` for the call's duration, and a handler built earlier does
+    not follow that rebinding, so the record goes to the handler's own stream.
+    That is the stream under test here.
+    """
+    _configure(monkeypatch)
+    monkeypatch.setenv("OSW_STATE_DIR", str(tmp_path / "state"))
+    # Makes the connection check fail without a network, which is what gets
+    # status to log while the tool call is running.
+    monkeypatch.setattr(Context, "osw_for", _no_connection)
+    config.reset()
+    mcp = server.create_server()
+
+    with _osw_logging_on_the_captured_stream():
+        asyncio.run(mcp.call_tool("status", {}))
+
+    captured = capsys.readouterr()
+    # First, so a run that emits no record at all fails here rather than
+    # passing the stdout assertion without having observed anything.
+    assert "status connection check failed" in captured.err
+    assert captured.out == ""