Skip to content

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

Merged
LukasGold merged 3 commits into
mainfrom
fix/cli-mcp-followups
Sep 22, 2026
Merged

LukasGold merged 3 commits into
mainfrom
fix/cli-mcp-followups

Conversation

@LukasGold

@LukasGold LukasGold commented Sep 21, 2026

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 keeps it as typed; the MCP server resolves it at startup, see the A relative OSW_CRED_FILEPATH resolves against the MCP client's working directory #197 section below.
  • 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.

#197 - a relative OSW_CRED_FILEPATH depended on the MCP client's working directory

Implements the behavior described in #197 (comment): resolve at startup, reject if missing, store the resolved path.

  • _resolve_cred_file makes a relative path absolute with os.path.abspath while implicit discovery is disabled, which is the MCP server's setting. load() already checks existence on that return value and passes it to Settings, so the check, the source report, and every later read (Context, available_iris) use the full path.
  • The MCP server is recognized by the existing _discover_env_file flag, which load() already uses for its MCP-only stdio hint. A process that embeds osw.service without calling set_env_file_discovery gets the same resolution.
  • os.path.abspath rather than Path.resolve(), so a symlink stays as configured. Path.is_absolute() rather than os.path.isabs, so oo and C:foo on Windows count as relative.
  • An absolute path is left exactly as configured. The CLI keeps a relative value, because its working directory is the one the user typed the command in.
  • The "does not exist" rejection names the relative value it was resolved from, because the full path alone shows a directory the user never typed.
  • 3 tests in tests/test_service_config.py; the 2 MCP tests fail on the unfixed source, the CLI test guards the unchanged CLI behavior.

Verification

  • Full unit suite: 671 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.
  • Started the installed osw-mcp console script with OSW_CRED_FILEPATH=accounts.pwd.yaml: with the file missing it exits 1 and names the full path; with a dummy file it starts and the report shows the full path.
  • Run locally on Windows only.

Closes #192, closes #193, closes #194, closes #197

- 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

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Release preview

No version bump from the current commits (stays at v2.6.0). Use conventional commit types (feat, fix, ...) to trigger a release.

Changelog preview (truncated)

Preview via python-semantic-release and conventional commits.

- the MCP client chooses the working directory, so store the full path
- load() checks existence on the resolved path and rejects a missing one
- the rejection names the relative value it was resolved from
- the CLI keeps a relative value
- closes #197
@LukasGold LukasGold changed the title fix(cli,mcp): UTF-8 stderr, a stdout guard, and ~ in OSW_CRED_FILEPATH fix(cli,mcp): UTF-8 stderr, a stdout guard, and OSW_CRED_FILEPATH paths Sep 22, 2026
@LukasGold LukasGold self-assigned this Sep 22, 2026
@LukasGold
LukasGold merged commit 0d0b2b6 into main Sep 22, 2026
12 checks passed
@LukasGold
LukasGold deleted the fix/cli-mcp-followups branch September 22, 2026 09:43
@LukasGold
LukasGold restored the fix/cli-mcp-followups branch September 22, 2026 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment