Skip to content

fix(cli,mcp): UTF-8 stderr, a stdout guard, and ~ in OSW_CRED_FILEPATH - #196

Open
LukasGold wants to merge 2 commits into
mainfrom
fix/cli-mcp-followups
Open

LukasGold wants to merge 2 commits into
mainfrom
fix/cli-mcp-followups

Conversation

@LukasGold

Copy link
Copy Markdown
Contributor

Follow-ups to #133, now that it has merged.

#194 - a leading ~ in OSW_CRED_FILEPATH was not expanded

  • Settings._validate_cred_filepath expands ~, as _validate_state_dir already does.
  • _resolve_cred_file expands it too. The issue's suggested fix alone is not enough: load() calls Path(cred_filepath).is_file() on _resolve_cred_file's return value (src/osw/service/config.py) long before it builds Settings, so the field validator runs too late. ~/accounts.pwd.yaml was reported as missing while the file was there.
  • The expansion happens after _cred_file_var is determined, because that lookup matches the raw value against os.getenv.
  • A relative path stays accepted, unlike state_dir. The CLI resolves accounts.pwd.yaml against the working directory on purpose.
  • expanduser() raises RuntimeError when no home directory can be found. Both call sites catch it and name the variable, so the message is actionable. load()'s Raises docstring lists the new case.
  • 5 tests in tests/test_service_config.py; 4 of them fail on the unfixed source.

#193 - the osw-mcp startup report was written with the locale encoding

  • New src/osw/service/streams.py with force_utf8(*streams). The CLI had this logic inline in _force_utf8_output; it now lives in the shared core, because osw.mcp may never import osw.cli (tests/test_no_paths_on_mcp_surface.py).
  • main() in src/osw/mcp/server.py calls it on stderr only. An MCP client starts the server with stderr on a pipe, so Python picks the locale encoding, cp1252 on a German Windows system. The report carries the credential file path and the env file path.
  • 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.
  • reconfigure() is passed errors= explicitly. It silently resets the handler to strict otherwise, and a strict stderr would raise while reporting a failure.
  • Reconfiguring in place also covers osw's own log handler, which holds the same stream object.

#192 - no test held stdout free of log records during a tool call

  • tests/test_mcp_server.py now calls the real status tool through mcp.call_tool("status", {}) and asserts stdout is empty while the record appears on stderr.
  • status is used because it logs a warning from inside ctx.guard() when the connection check fails (src/osw/service/ops/status.py).
  • Two mechanisms keep stdout clean and only one is osw's own code: enable_logging defaults its handler to sys.stderr, and the SDK claims file descriptor 1. A single edit to that default would undo the first, which is what the test holds. It fails when src/osw/__init__.py is changed to sys.stdout.
  • The test resets the root handlers and re-runs enable_logging in a context manager, not a fixture, because pytest attaches its log-capture handler after fixtures run. Mirrors plain_logging in tests/test_logging_setup.py.

Verification

  • Full unit suite: 668 passed.
  • make check: lock file, pre-commit, ty, deptry all pass.
  • Each fix was proven by reverting the source change and watching the new test fail.
  • Run on Windows only.

- expand in _resolve_cred_file as well: load() checks the path before
  Settings is built, so the field validator alone runs too late
- a relative path stays accepted, unlike state_dir
- name the variable when no home directory can be determined
- closes #194
- move the mechanism to osw.service.streams.force_utf8, shared by both
  adapters without osw.mcp importing osw.cli
- reconfigure stderr only: the SDK re-wraps stdout as UTF-8 itself
- add a test that a log record during a tool call leaves stdout empty
- closes #193, closes #192
@github-actions

Copy link
Copy Markdown
Contributor

Release preview

Merging this PR would release v2.6.1 (current: v2.6.0).

Changelog preview (truncated)
## v2.6.1 (2026-09-21)

### Bug Fixes

- **config**: Expand a leading ~ in OSW_CRED_FILEPATH
  ([`eb35ba7`](https://github.com/OpenSemanticLab/osw-python/commit/eb35ba7bdf5be6e9a10f108fcc5f7c10133d6515))

- **mcp**: Encode the osw-mcp startup report as UTF-8
  ([`979ad81`](https://github.com/OpenSemanticLab/osw-python/commit/979ad81ea20e2c0b85444c8f49d3815c5029ff3a))

Preview via python-semantic-release and conventional commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant