fix(cli,mcp): UTF-8 stderr, a stdout guard, and OSW_CRED_FILEPATH paths - #196
Merged
Merged
Conversation
- 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
Contributor
Release previewNo version bump from the current commits (stays at 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups to #133, now that it has merged.
#194 - a leading
~inOSW_CRED_FILEPATHwas not expandedSettings._validate_cred_filepathexpands~, as_validate_state_diralready does._resolve_cred_fileexpands it too. The issue's suggested fix alone is not enough:load()callsPath(cred_filepath).is_file()on_resolve_cred_file's return value (src/osw/service/config.py) long before it buildsSettings, so the field validator runs too late.~/accounts.pwd.yamlwas reported as missing while the file was there._cred_file_varis determined, because that lookup matches the raw value againstos.getenv.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()raisesRuntimeErrorwhen no home directory can be found. Both call sites catch it and name the variable, so the message is actionable.load()'sRaisesdocstring lists the new case.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
src/osw/service/streams.pywithforce_utf8(*streams). The CLI had this logic inline in_force_utf8_output; it now lives in the shared core, becauseosw.mcpmay never importosw.cli(tests/test_no_paths_on_mcp_surface.py).main()insrc/osw/mcp/server.pycalls 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.stdio_serverre-wraps the binary buffer as UTF-8 itself and claims file descriptor 1 while doing it.reconfigure()is passederrors=explicitly. It silently resets the handler to strict otherwise, and a strict stderr would raise while reporting a failure.#192 - no test held stdout free of log records during a tool call
tests/test_mcp_server.pynow calls the realstatustool throughmcp.call_tool("status", {})and asserts stdout is empty while the record appears on stderr.statusis used because it logs a warning from insidectx.guard()when the connection check fails (src/osw/service/ops/status.py).enable_loggingdefaults its handler tosys.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 whensrc/osw/__init__.pyis changed tosys.stdout.enable_loggingin a context manager, not a fixture, because pytest attaches its log-capture handler after fixtures run. Mirrorsplain_loggingintests/test_logging_setup.py.#197 - a relative
OSW_CRED_FILEPATHdepended on the MCP client's working directoryImplements the behavior described in #197 (comment): resolve at startup, reject if missing, store the resolved path.
_resolve_cred_filemakes a relative path absolute withos.path.abspathwhile implicit discovery is disabled, which is the MCP server's setting.load()already checks existence on that return value and passes it toSettings, so the check, the source report, and every later read (Context,available_iris) use the full path._discover_env_fileflag, whichload()already uses for its MCP-only stdio hint. A process that embedsosw.servicewithout callingset_env_file_discoverygets the same resolution.os.path.abspathrather thanPath.resolve(), so a symlink stays as configured.Path.is_absolute()rather thanos.path.isabs, soooandC:fooon Windows count as relative.tests/test_service_config.py; the 2 MCP tests fail on the unfixed source, the CLI test guards the unchanged CLI behavior.Verification
make check: lock file, pre-commit,ty,deptryall pass.osw-mcpconsole script withOSW_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.Closes #192, closes #193, closes #194, closes #197